From 0998b7381fd8eeb0e69ec969c06f2c3a33bc8fcc Mon Sep 17 00:00:00 2001 From: bloqaudio Date: Thu, 13 Aug 2026 15:58:55 -0500 Subject: [PATCH 01/36] machine: add PCB SWTG018AS-V2.1.0 variant of the SWTGW218AS The SWTGW218AS label covers more than one PCB. On boards with the SWTG018AS-V2.1.0 silkscreen the SFP module-detect is GPIO38 (no LOS pin wired) and the LED block is wired differently, including a bi-color SFP LED (green up to 2.5G, blue at 10G) that the LED-set encoding cannot express. With the existing MACHINE_SWTGW218AS definition these boards never detect an SFP module, so an SFP-uplinked switch comes up with no working uplink. Add the variant as its own machine define named by the PCB marking, leaving MACHINE_SWTGW218AS unchanged for boards that match its wiring. The LED register values are taken from the stock firmware; the PIN_MUX_0 write routes the blue-LED pin to the LED controller, without which no LED register value can drive it. PIN_MUX_1/2 stay untouched so SFP detect and i2c remain GPIOs. --- machine.c | 81 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ machine.h | 1 + 2 files changed, 82 insertions(+) diff --git a/machine.c b/machine.c index c27b92a..d7d4823 100644 --- a/machine.c +++ b/machine.c @@ -444,6 +444,87 @@ __code const struct machine machine = { }; void machine_custom_init(void) { } + +#elif defined MACHINE_PCB_SWTG018AS_V2_1_0 // Sold as Sodola SL902 / Horaco "SWTGW218AS"; the SWTGW218AS label also covers other PCBs with different SFP and LED wiring (see MACHINE_SWTGW218AS) + +__code const struct machine machine = { + .machine_name = "SWTGW218AS (SWTG018AS-V2.1.0)", + .isRTL8373 = 1, + .mac_flash_offset = 0x1FC000, + .min_port = 0, + .max_port = 8, + .n_sfp = 1, + .log_to_phys_port = {1, 2, 3, 4, 5, 6, 7, 8, 9}, + .phys_to_log_port = {0, 1, 2, 3, 4, 5, 6, 7, 8}, + .is_sfp = {0, 0, 0, 0, 0, 0, 0, 0, 1}, + .sfp_port[0].pin_detect = GPIO38, // pulled low on module insert + .sfp_port[0].pin_los = GPIO_NA, // no LOS pin wired + .sfp_port[0].pin_tx_disable = GPIO_NA, + .sfp_port[0].sds = 1, + .sfp_port[0].i2c = { .sda = GPIO39_I2C_SDA4, .scl = GPIO40_I2C_SCL3_MDC1 }, + .reset_pin = GPIO54_ACL_BIT2_EN, + .high_leds = { .mux = LED_27 | LED_28_SYS | LED_29, .enable = LED_28_SYS | LED_29 }, + .port_led_set = { 0, 0, 0, 0, 0, 0, 0, 0, 1}, + // LED wiring matches the SWTG018AS-A V2.0 (same PCB family) + .led_sets = { + { /* RJ45: First LED, yellow, second LED: green */ + LEDS_2G5 | LEDS_LINK, + LEDS_2G5 | LEDS_1G | LEDS_100M | LEDS_10M | LEDS_LINK | LEDS_ACT, + 0, + 0, + }, { /* SFP set (superseded by the raw register override in machine_custom_init) */ + LEDS_2G5 | LEDS_1G | LEDS_100M | LEDS_10M | LEDS_LINK | LEDS_ACT | LEDS_10G, + 0, + 0, + 0, + }}, + .led_mux_custom = 1, + .led_mux = { 0x00, 0x01, 0x04, 0x05, 0x08, // 65e0 + 0x09, 0x0c, 0x09, 0x0d, 0x10, // 65e4 + 0x11, 0x0e, 0x14, 0x11, 0x12, // 65e8 + 0x15, 0x15, 0x16, 0x18, 0x19, // 65ec + 0x1a, 0x19, 0x1d, 0x1e, 0x1c, // 65f0 + 0x1d, 0x20, 0x21 }, +}; + +// The LED-set encoding cannot express this board's bi-color SFP LED (green <= 2.5G, +// blue at 10G), so program the LED register block with the values the stock firmware +// uses. Runs after leds_setup() and overrides the values computed there. +// The final entry routes the blue-LED pin to the LED controller via PIN_MUX_0; +// as a GPIO (the default) no LED register can light it. PIN_MUX_1/2 stay +// untouched so SFP detect (GPIO38) and i2c remain GPIOs. +static __code const struct { uint16_t reg; uint32_t val; } custom_init_regs[] = { + { 0x6520, 0x0023e430UL }, // LED_MODE + { 0x6524, 0xff001400UL }, // LED3_0_SET3 + { 0x6528, 0x00100000UL }, // LED3_0_SET1 + { 0x652c, 0x007f013fUL }, // LED3_2_SET3 + { 0x6530, 0x02000400UL }, // LED1_0_SET3 + { 0x6534, 0x01400141UL }, // LED3_2_SET2 + { 0x6538, 0x01440170UL }, // LED1_0_SET2 + { 0x653c, 0x18000041UL }, // LED3_2_SET1 + { 0x6540, 0x01400155UL }, // LED1_0_SET1 + { 0x6544, 0x01411000UL }, // LED3_2_SET0 + { 0x6548, 0x01740141UL }, // LED1_0_SET0 + { 0x654c, 0x00010000UL }, // LED_PORT_SET_SEL + { 0x65d8, 0x3ffb6dffUL }, // LED_GLB_ACTIVE + { 0x65dc, 0x7f24977fUL }, // LED_GLB_IO_EN + { 0x65e0, 0x08144040UL }, // LED_GLB_MUX_1 + { 0x65e4, 0x10349309UL }, // LED_GLB_MUX_2 + { 0x65e8, 0x12454391UL }, // LED_GLB_MUX_3 + { 0x65ec, 0x19616555UL }, // LED_GLB_MUX_4 + { 0x65f0, 0x1c79d65aUL }, // LED_GLB_MUX_5 + { 0x65f4, 0x0002181dUL }, // LED_GLB_MUX_6 + { 0x7f8c, 0x20db6880UL }, // PIN_MUX_0 +}; + +void machine_custom_init(void) { + uint8_t i; + // REG_SET is a multi-statement macro without a do-while wrapper: braces required + for (i = 0; i < sizeof(custom_init_regs) / sizeof(custom_init_regs[0]); i++) { + REG_SET(custom_init_regs[i].reg, custom_init_regs[i].val); + } +} + #elif defined MACHINE_LIANGUO_ZX_SWTGW215AS // Has PCB branded PCB-SWTG115AS-V2.0 but is labeled and reports as a ZX-SWTGW215AS, seems to be identical to the "real" ZX-SWTGW215AS except for the LEDs __code const struct machine machine = { .machine_name = "Lianguo ZX-SWTGW215AS", diff --git a/machine.h b/machine.h index 523beea..29fe39f 100644 --- a/machine.h +++ b/machine.h @@ -18,6 +18,7 @@ // #define MACHINE_HG0402XG_V1_1 // #define MACHINE_SWTG018AS_A_V_2_0 // #define MACHINE_SWTGW218AS +// #define MACHINE_PCB_SWTG018AS_V2_1_0 // #define MACHINE_PCB_K0402WS_V3 // #define MACHINE_K0501W_V2_0 // #define MACHINE_LIANGUO_ZX_SWTGW215AS From 1d1e33f4d5008d645df4b5906beff7a61f66a193 Mon Sep 17 00:00:00 2001 From: d00f <8052722+DrDoof@users.noreply.github.com> Date: Sat, 15 Aug 2026 17:25:51 +0200 Subject: [PATCH 02/36] sfp: notice when an I2C read fails sfp_read_reg() waited for the transfer to finish and then read the output register whatever the outcome, so an address nothing acknowledged came back as an ordinary byte and no caller could tell it apart from data. The vendor SDK looks at bit 1 of the control register for exactly this, and we did not. A failure now sets sfp_i2c_fail and the read returns 0xff, which is already the value sfp_apply_quirks() reads as either a failed transfer or a voltage the spec does not allow, so that test starts being true when it should be. The insertion path and the sfp command clear the flag first and say so afterwards, rather than presenting the bytes as though they came from the module. What this deliberately does not do is act on the failure. Skipping sds_config() when the rate read failed is the obvious next step, but a module that raises the bit spuriously would then never be configured at all, which is worse than what happens today, and I have no way to judge how often the bit is right. That decision belongs with someone holding the board. It also leaves the other half of the rewrite alone, reading and writing up to sixteen bytes per transaction. doc/sfp.md describes only the single byte path and does not name a length field, and guessing at a register I cannot test is how the last attempt at this function went wrong. 40 bytes of the common segment, 51 of BANK2 and 1 of xdata, nothing in BANK1 or internal RAM. Built for SWTGW218AS and KP_9000_6XHML_X2 on sdcc 4.5.0. Not tested on hardware: shorting the clock line, as in #342, should now print the failure line instead of a plausible looking byte. --- cmd_parser.c | 3 +++ rtl837x_common.h | 1 + rtlplayground.c | 12 ++++++++++++ 3 files changed, 16 insertions(+) diff --git a/cmd_parser.c b/cmd_parser.c index 9e2be41..2b13edd 100644 --- a/cmd_parser.c +++ b/cmd_parser.c @@ -777,11 +777,14 @@ void parse_sfp(void) print_string(" - empty\n"); continue; } + sfp_i2c_fail = 0; print_string(" - Rate: "); print_byte(sfp_read_reg(slot, 12)); print_string(" Encoding: "); print_byte(sfp_read_reg(slot, 11)); write_char('\n'); sfp_print_info(slot); sfp_print_measurements(slot); + if (sfp_i2c_fail) + print_string("I2C read failed on this slot\n"); } return; } diff --git a/rtl837x_common.h b/rtl837x_common.h index 66df88a..98202b8 100644 --- a/rtl837x_common.h +++ b/rtl837x_common.h @@ -150,6 +150,7 @@ void write_char_no_syslog(char c); void write_char(char c); void print_reg(uint16_t reg); uint8_t sfp_read_reg(uint8_t slot, uint8_t reg); +extern __xdata uint8_t sfp_i2c_fail; void reg_bit_set(uint16_t reg_addr, char bit); void reg_bit_clear(uint16_t reg_addr, char bit); uint8_t reg_bit_test(uint16_t reg_addr, char bit); diff --git a/rtlplayground.c b/rtlplayground.c index bdb8ab3..f8cd721 100644 --- a/rtlplayground.c +++ b/rtlplayground.c @@ -139,6 +139,7 @@ __xdata char sfp_module_vendor[2][17]; __xdata char sfp_module_model[2][17]; __xdata char sfp_module_serial[2][17]; __xdata uint8_t sfp_options[2]; +__xdata uint8_t sfp_i2c_fail; /* set by sfp_read_reg() when the controller flags a failed transfer */ __xdata uint8_t sfp_speed[2]; __xdata uint8_t sfp_quirks[2]; __xdata bool button_last; @@ -1061,6 +1062,14 @@ uint8_t sfp_read_reg(uint8_t slot, uint8_t reg) reg_read_m(RTL837X_REG_I2C_CTRL); } while (sfr_data[3] & 0x1); + /* Bit 1 is the controller's own failure indication, which the vendor SDK + * looks at and this did not. Without it an unacknowledged address comes + * back as an ordinary byte and the caller cannot tell it from data. */ + if (sfr_data[3] & 0x2) { + sfp_i2c_fail = 1; + return 0xff; + } + reg_read_m(RTL837X_REG_I2C_OUT); return sfr_data[3]; } @@ -1300,6 +1309,7 @@ void handle_sfp(void) // Read Reg 11: Encoding, see SFF-8472 and SFF-8024 // Read Reg 12: Signalling rate (including overhead) in 100Mbit: 0xd: 1Gbit, 0x67:10Gbit delay(100); // Delay, because some modules need time to wake up + sfp_i2c_fail = 0; uint8_t rate = sfp_read_reg(sfp, 12); if (sfp_speed[sfp] == SFP_SPEED_100M) rate = 0x1; @@ -1316,6 +1326,8 @@ void handle_sfp(void) sfp_options[sfp] = sfp_read_reg(sfp, 92); sfp_get_info(sfp); sfp_apply_quirks(sfp); + if (sfp_i2c_fail) + print_string("SFP: an I2C read failed, the module data above may be wrong\n"); sds_config(machine.sfp_port[sfp].sds, sfp_rate_to_sds_config(rate)); } } else { From 772e9dc52608a22eb8cad94abf8205724356b404 Mon Sep 17 00:00:00 2001 From: bloqaudio Date: Tue, 18 Aug 2026 14:14:14 -0500 Subject: [PATCH 03/36] httpd: parse a configuration upload once the whole request has arrived Saving the configuration works in Chrome and fails in Firefox, and the difference is only how each browser splits the request. The handler erased the config sector as soon as the request started and then parsed the multipart body one TCP segment at a time, which requires every boundary and every part header to fall inside a single segment. Firefox splits inside a part header, so the parser lost its place and the erased sector was left holding a truncated body or nothing at all. A single-burst scripted post lost the whole body the same way. The configuration is limited to two kilobytes, so the whole request body now accumulates in xdata and is parsed only after the closing boundary has arrived. The parts are walked in one pass, the part carrying a filename is written to a freshly erased sector, and the client receives a 200 instead of the previous silent close. No segmentation can confuse this, since the parser only ever sees a complete body. Locating the closing boundary first also bounds the walk over the parts, since none can lie beyond it, so the length of the buffer is no longer the bound and the test for the two trailing dashes is unnecessary. The walk matches at offsets inside a buffer that is not terminated, so neither existing helper fits: strcmp() goes on to compare the byte after the match and is_word_x() demands a separator there. Add strstart() and strstart_x() for that case, and use strlen_x() for the boundary length. The firmware upload path still streams, since a megabyte cannot be buffered, and is untouched. --- httpd/httpd.c | 100 ++++++++++++++++++++++++++++++++++++++++++++--- rtl837x_common.h | 2 + rtlplayground.c | 26 ++++++++++++ 3 files changed, 123 insertions(+), 5 deletions(-) diff --git a/httpd/httpd.c b/httpd/httpd.c index d856ccd..3a54aca 100644 --- a/httpd/httpd.c +++ b/httpd/httpd.c @@ -41,6 +41,14 @@ __xdata uint32_t cont_addr; // HTTP header properties __xdata uint8_t boundary[72]; + +// a client may split the request anywhere, including inside a boundary or a +// part header, so a configuration upload is parsed only once it is complete +#define CONFIG_UPLOAD_BUF 2560 +__xdata uint8_t config_upload; +__xdata uint8_t config_buf[CONFIG_UPLOAD_BUF]; +__xdata uint16_t cfg_pos, cfg_hdr, cfg_body, cfg_end, cfg_last; +__xdata uint8_t cfg_bl; __xdata uint8_t *content_type = 0; __xdata uint8_t *session = 0; @@ -77,6 +85,7 @@ inline uint8_t is_separator(uint8_t c) void httpd_init(void) __banked { + config_upload = 0; // xdata is not zeroed by the startup code __xdata struct httpd_state * __xdata s = &(uip_conn->appstate); // Start listening to port 80 uip_listen(HTONS(80)); @@ -315,6 +324,62 @@ void gen_random_bytes(__xdata uint8_t *b, uint8_t bytes) } +/* 0: body incomplete, 1: configuration stored, 2: malformed */ +static uint8_t config_take(void) +{ + cfg_bl = strlen_x(boundary); + + // the body is complete once the closing boundary has arrived + cfg_last = 0; + while (1) { + if (cfg_last + cfg_bl + 1 >= write_len) + return 0; + if (strstart_x(&config_buf[cfg_last], boundary) + && strstart(&config_buf[cfg_last + cfg_bl], "--")) + break; + cfg_last++; + } + + // every part lies ahead of the closing boundary, so it bounds the walk + cfg_pos = 0; + while (cfg_pos < cfg_last) { + if (!strstart_x(&config_buf[cfg_pos], boundary)) { + cfg_pos++; + continue; + } + cfg_hdr = cfg_pos + cfg_bl; + cfg_body = cfg_hdr; + while (1) { + if (cfg_body + 3 >= cfg_last) + return 2; + if (strstart(&config_buf[cfg_body], "\r\n\r\n")) + break; + cfg_body++; + } + cfg_end = cfg_body; + cfg_body += 4; + // reaching cfg_last is a match: the last part ends at the closing boundary + while (cfg_end < cfg_last && !strstart_x(&config_buf[cfg_end], boundary)) + cfg_end++; + while (cfg_hdr + 8 < cfg_body) { + // the part carrying a filename holds the configuration + if (strstart(&config_buf[cfg_hdr], "filename")) { + config_buf[cfg_end] = 0; + flash_region.addr = CONFIG_START; + flash_sector_erase(); + flash_region.addr = CONFIG_START; + flash_region.len = cfg_end - cfg_body + 1; + flash_write_bytes(config_buf + cfg_body); + return 1; + } + cfg_hdr++; + } + cfg_pos = cfg_end; + } + return 2; +} + + /* * Reads post data from the http stream and writes it into flash memory * Input: the current position in the TCP buffer (uip_appdata) @@ -437,6 +502,7 @@ void handle_post(void) return; } print_string("Firmware upload started."); + config_upload = 0; uptr = FIRMWARE_UPLOAD_START; verify_crc = 1; max_upload = 1024576; @@ -445,12 +511,10 @@ void handle_post(void) send_unauthorized(); return; } - dbg_string("Configuration upload, erasing config mem!\n"); - uptr = CONFIG_START; + dbg_string("Configuration upload\n"); verify_crc = 0; - max_upload = 2048; - flash_region.addr = CONFIG_START; - flash_sector_erase(); + config_upload = 1; + write_len = 0; } // Check for other POST requests, which are not multipart, below } else { @@ -504,6 +568,32 @@ void handle_post(void) send_bad_request(); return; } + if (config_upload) { + cfg_pos = uip_len - (p - uip_appdata); + if (write_len + cfg_pos >= CONFIG_UPLOAD_BUF) { + print_string("Configuration too large, aborting.\n"); + config_upload = 0; + s->tstate = TSTATE_NONE; + send_bad_request(); + return; + } + memcpy(config_buf + write_len, p, cfg_pos); + write_len += cfg_pos; + uint8_t taken = config_take(); + + if (!taken) { + s->tstate = TSTATE_MULTIPART; + return; + } + config_upload = 0; + s->tstate = TSTATE_NONE; + if (taken == 2) { + send_bad_request(); + return; + } + slen = strtox(outbuf, "HTTP/1.1 200 OK\r\nConnection: close\r\n\r\n"); + return; + } // We skip the intial parts as part of the header do { p = skip_boundary(p); diff --git a/rtl837x_common.h b/rtl837x_common.h index 66df88a..44e06d0 100644 --- a/rtl837x_common.h +++ b/rtl837x_common.h @@ -164,6 +164,8 @@ uint16_t strlen_x(register __xdata const char *s); uint16_t strtox(register __xdata uint8_t *dst, register __code const char *s); uint16_t strcpy(register __xdata uint8_t *dst, register const char *s); char strcmp(register __xdata const uint8_t *a, register __code const uint8_t *b); +bool strstart(__xdata const uint8_t *a, __code const uint8_t *b); +bool strstart_x(__xdata const uint8_t *a, __xdata const uint8_t *b); void tcpip_output(void); uint8_t read_flash(uint8_t bank, __code uint8_t *addr); void get_random_32(void); diff --git a/rtlplayground.c b/rtlplayground.c index b9cc05d..e08c09a 100644 --- a/rtlplayground.c +++ b/rtlplayground.c @@ -360,6 +360,32 @@ char strcmp(register __xdata const uint8_t *a, register __code const uint8_t *b) } +/* + * True when b is a prefix of a. Unlike strcmp() the byte after the match is not + * compared, and unlike is_word_x() it need not be a separator. + */ +bool strstart(__xdata const uint8_t *a, __code const uint8_t *b) +{ + uint8_t i = 0; + + while (b[i] && (b[i] == a[i])) + i++; + + return !b[i]; +} + + +bool strstart_x(__xdata const uint8_t *a, __xdata const uint8_t *b) +{ + uint8_t i = 0; + + while (b[i] && (b[i] == a[i])) + i++; + + return !b[i]; +} + + void print_short(uint16_t a) { // allocating the registers first improves the sdcc code here From 9a96c80af230ac825942eefcb6a527e25f89139b Mon Sep 17 00:00:00 2001 From: d00f <8052722+DrDoof@users.noreply.github.com> Date: Mon, 24 Aug 2026 02:34:39 +0200 Subject: [PATCH 04/36] sfp: move the I2C transfer to the banked pins module sfp_read_reg() sat in rtlplayground.c, so it occupied the common 16 KB window that every bank shares, even though nothing outside the SFP paths calls it. That window is the tightest resource in the image: SWTG024AS_V2_0 and SWTG024AS_A_2_0_1_5C_1SFP had 54 bytes left in it. rtl837x_pins.c is already in BANK2 and already holds the I2C bus helpers this function calls, so the transfer belongs there. The function moves verbatim and becomes __banked; the prototype in rtl837x_common.h says so, which is what keeps the callers in BANK1 and BANK2 honest. No behaviour change. The common segment gains 200 bytes on every machine: 159 to 359 free on SWTGW218AS, 54 to 254 on the two variants above. BANK1 +6 bytes, BANK2 +336. Built on sdcc 4.5.0. --- rtl837x_common.h | 2 +- rtl837x_pins.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ rtlplayground.c | 39 --------------------------------------- 3 files changed, 45 insertions(+), 40 deletions(-) diff --git a/rtl837x_common.h b/rtl837x_common.h index 98202b8..5432343 100644 --- a/rtl837x_common.h +++ b/rtl837x_common.h @@ -149,7 +149,7 @@ void sleep(uint16_t t); void write_char_no_syslog(char c); void write_char(char c); void print_reg(uint16_t reg); -uint8_t sfp_read_reg(uint8_t slot, uint8_t reg); +uint8_t sfp_read_reg(uint8_t slot, uint8_t reg) __banked; extern __xdata uint8_t sfp_i2c_fail; void reg_bit_set(uint16_t reg_addr, char bit); void reg_bit_clear(uint16_t reg_addr, char bit); diff --git a/rtl837x_pins.c b/rtl837x_pins.c index 90015b7..ac0cea3 100644 --- a/rtl837x_pins.c +++ b/rtl837x_pins.c @@ -1,6 +1,11 @@ #include "rtl837x_pins.h" #include "rtl837x_common.h" +#include "rtl837x_sfr.h" #include "rtl837x_regs.h" +#include "machine.h" + +extern __code const struct machine machine; +extern __xdata uint8_t sfr_data[4]; #pragma codeseg BANK2 #pragma constseg BANK2 @@ -121,3 +126,42 @@ void gpio_output_setup(uint8_t pin, __xdata uint8_t initial_val) __banked{ reg_bit_set(gpio_direction_reg(pin), (pin % 32)); } + + +/* + * Read a register of the EEPROM via I2C + */ +uint8_t sfp_read_reg(uint8_t slot, uint8_t reg) __banked +{ + if (reg & 0x80) { // Configure SFP readings address (0x51) as I2C device address + reg &= 0x7f; + REG_WRITE(RTL837X_REG_I2C_CTRL, 0x00, 0x1 << (I2C_MEM_ADDR_WIDTH-16) | 0, 0x51 >> 5, (0x51 << 3) & 0xff); + } else { + REG_WRITE(RTL837X_REG_I2C_CTRL, 0x00, 0x1 << (I2C_MEM_ADDR_WIDTH-16) | 0, 0x50 >> 5, (0x50 << 3) & 0xff); + } + + reg_read_m(RTL837X_REG_I2C_CTRL); + sfr_mask_data(1, 0xfc, i2c_bus_from_scl_pin(machine.sfp_port[slot].i2c.scl) << 5 | i2c_bus_from_sda_pin(machine.sfp_port[slot].i2c.sda) << 2); + reg_write_m(RTL837X_REG_I2C_CTRL); + + REG_WRITE(RTL837X_REG_I2C_IN, 0, 0, 0, reg); + + // Execute I2C Read + reg_bit_set(RTL837X_REG_I2C_CTRL, 0); + + // Wait for execution to finish + do { + reg_read_m(RTL837X_REG_I2C_CTRL); + } while (sfr_data[3] & 0x1); + + /* Bit 1 is the controller's own failure indication, which the vendor SDK + * looks at and this did not. Without it an unacknowledged address comes + * back as an ordinary byte and the caller cannot tell it from data. */ + if (sfr_data[3] & 0x2) { + sfp_i2c_fail = 1; + return 0xff; + } + + reg_read_m(RTL837X_REG_I2C_OUT); + return sfr_data[3]; +} diff --git a/rtlplayground.c b/rtlplayground.c index f8cd721..6926ae5 100644 --- a/rtlplayground.c +++ b/rtlplayground.c @@ -1036,45 +1036,6 @@ void sds_config(uint8_t sds, uint8_t mode) } -/* - * Read a register of the EEPROM via I2C - */ -uint8_t sfp_read_reg(uint8_t slot, uint8_t reg) -{ - if (reg & 0x80) { // Configure SFP readings address (0x51) as I2C device address - reg &= 0x7f; - REG_WRITE(RTL837X_REG_I2C_CTRL, 0x00, 0x1 << (I2C_MEM_ADDR_WIDTH-16) | 0, 0x51 >> 5, (0x51 << 3) & 0xff); - } else { - REG_WRITE(RTL837X_REG_I2C_CTRL, 0x00, 0x1 << (I2C_MEM_ADDR_WIDTH-16) | 0, 0x50 >> 5, (0x50 << 3) & 0xff); - } - - reg_read_m(RTL837X_REG_I2C_CTRL); - sfr_mask_data(1, 0xfc, i2c_bus_from_scl_pin(machine.sfp_port[slot].i2c.scl) << 5 | i2c_bus_from_sda_pin(machine.sfp_port[slot].i2c.sda) << 2); - reg_write_m(RTL837X_REG_I2C_CTRL); - - REG_WRITE(RTL837X_REG_I2C_IN, 0, 0, 0, reg); - - // Execute I2C Read - reg_bit_set(RTL837X_REG_I2C_CTRL, 0); - - // Wait for execution to finish - do { - reg_read_m(RTL837X_REG_I2C_CTRL); - } while (sfr_data[3] & 0x1); - - /* Bit 1 is the controller's own failure indication, which the vendor SDK - * looks at and this did not. Without it an unacknowledged address comes - * back as an ordinary byte and the caller cannot tell it from data. */ - if (sfr_data[3] & 0x2) { - sfp_i2c_fail = 1; - return 0xff; - } - - reg_read_m(RTL837X_REG_I2C_OUT); - return sfr_data[3]; -} - - /* * Adds TX Header to uip_buf and calls nic_tx_packet to send the packet * over the wire From e8e7fcbe7fb2a46a5d8dc445a657b8d035e40789 Mon Sep 17 00:00:00 2001 From: bloqaudio Date: Mon, 24 Aug 2026 15:45:25 -0500 Subject: [PATCH 05/36] httpd: send the config tail from the config sector, not a bare offset send_config() streams a configuration larger than the TCP output buffer through the cont_addr/cont_len continuation, but set cont_addr to the offset within the config instead of a flash address, so the tail was read from code space. The file server sets cont_addr absolute; do the same by adding CONFIG_START. The bug was unreachable while CONFIG_UPLOAD_BUF capped uploads below TCP_OUTBUF_SIZE, but a stored config near the full sector exposes it. --- httpd/page_impl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/httpd/page_impl.c b/httpd/page_impl.c index bf3e268..3e2e696 100644 --- a/httpd/page_impl.c +++ b/httpd/page_impl.c @@ -822,7 +822,7 @@ found_end: if (valid_len > (TCP_OUTBUF_SIZE - slen)) { cont_len = valid_len - (TCP_OUTBUF_SIZE - slen); valid_len = TCP_OUTBUF_SIZE - slen; - cont_addr = valid_len; + cont_addr = CONFIG_START + valid_len; } flash_region.addr = CONFIG_START; From cefe48fe5125fca1c1712af8c5dd541084fe3231 Mon Sep 17 00:00:00 2001 From: bloqaudio Date: Mon, 24 Aug 2026 15:45:25 -0500 Subject: [PATCH 06/36] httpd: size the config upload buffer for a full config sector CONFIG_UPLOAD_BUF at 2560 capped a configuration upload at about 2.2K while the config sector holds 4K. Size the buffer as CONFIG_LEN plus room for the multipart framing so the whole sector is usable. config_take() wrote the payload without checking it against the sector: safe before only because the buffer could not hold an oversized one. Reject a payload that does not fit CONFIG_LEN, terminator included, instead of writing past the erased sector. --- httpd/httpd.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/httpd/httpd.c b/httpd/httpd.c index 3a54aca..ff4eff9 100644 --- a/httpd/httpd.c +++ b/httpd/httpd.c @@ -43,8 +43,9 @@ __xdata uint32_t cont_addr; __xdata uint8_t boundary[72]; // a client may split the request anywhere, including inside a boundary or a -// part header, so a configuration upload is parsed only once it is complete -#define CONFIG_UPLOAD_BUF 2560 +// part header, so a configuration upload is parsed only once it is complete; +// sized for a full config sector plus the multipart framing around it +#define CONFIG_UPLOAD_BUF (CONFIG_LEN + 384) __xdata uint8_t config_upload; __xdata uint8_t config_buf[CONFIG_UPLOAD_BUF]; __xdata uint16_t cfg_pos, cfg_hdr, cfg_body, cfg_end, cfg_last; @@ -364,6 +365,9 @@ static uint8_t config_take(void) while (cfg_hdr + 8 < cfg_body) { // the part carrying a filename holds the configuration if (strstart(&config_buf[cfg_hdr], "filename")) { + // the payload plus its terminator must fit the sector + if (cfg_end - cfg_body + 1 > CONFIG_LEN) + return 2; config_buf[cfg_end] = 0; flash_region.addr = CONFIG_START; flash_sector_erase(); From a0628c7df5b1ef856402f6c2c83ea7ad48cf07b5 Mon Sep 17 00:00:00 2001 From: d00f <8052722+DrDoof@users.noreply.github.com> Date: Mon, 24 Aug 2026 02:35:02 +0200 Subject: [PATCH 07/36] sfp: read the EEPROM in blocks instead of a byte at a time The I2C controller transfers up to sixteen bytes per transaction and page_impl.c already used that for sfp_send_data(), while sfp_read_reg() asked for one byte and every caller looped. Reading a module therefore cost one address phase per byte: 87 transactions when a module is inserted, 52 for the sfp command, 36 for the vendor block in status.json. sfp_read_block() replaces sfp_read_reg() and the callers that already wanted a run of registers ask for it once: the vendor fields as three 16 byte pages, the diagnostics as one transfer, rate and encoding together. That drops the three paths above to 8, 6 and 3 transactions, and sfp_send_data() loses its copy of the transfer. The vendor loops now run over 16..63 rather than 20..59 so the page base is a multiple of sixteen and the index into the buffer is a single AND. The four extra bytes at each end are read and discarded. The diagnostics read asks for 16 bytes rather than the 15 it uses, because 16 is a width the shipped firmware already exercises and 15 is not. The device address, the bus selection and the start bit go into the control register in one write now that the memory address is written first, so a transfer touches that register once instead of three times. The register reads take their result from the SFRs directly rather than through the sfr_data mirror. The result is a bool and the destination is sfp_buf, so a caller that cares about a failed transfer looks at the return value instead of a flag. Every caller gives up on the first failed read rather than carrying a flag to the end, which is why the module read moved out of handle_sfp into a function of its own. A module whose read fails is left marked as absent, so the next poll retries it instead of configuring the SerDes from bytes that never arrived. BANK1 -194 bytes, BANK2 +382, common segment +44, xdata +15 for the buffer, and one byte more of internal RAM free than before the series. Built for all 25 machine definitions on sdcc 4.5.0; the tightest common segment is 98 bytes free on SWTG024AS_V2_0, against 54 before this series. --- cmd_parser.c | 40 +++++++++++------- httpd/page_impl.c | 36 ++++------------ rtl837x_common.h | 6 +-- rtl837x_pins.c | 62 ++++++++++++++++----------- rtlplayground.c | 104 +++++++++++++++++++++++++++++----------------- 5 files changed, 140 insertions(+), 108 deletions(-) diff --git a/cmd_parser.c b/cmd_parser.c index 2b13edd..d1c23b6 100644 --- a/cmd_parser.c +++ b/cmd_parser.c @@ -748,18 +748,25 @@ void parse_mtu(void) write_char('\n'); } -void sfp_print_measurements(uint8_t sfp) +bool sfp_print_measurements(uint8_t sfp) { - print_string("Options: "); print_byte(sfp_read_reg(sfp, 92)); write_char('\n'); + if (!sfp_read_block(sfp, 92, 1)) + return false; + + print_string("Options: "); print_byte(sfp_buf[0]); write_char('\n'); if (!(sfp_options[sfp] & 0x40)) - return; - print_string("Temp: "); print_byte(sfp_read_reg(sfp, 224)); print_byte(sfp_read_reg(sfp, 225)); write_char('\n'); - print_string("Vcc: "); print_byte(sfp_read_reg(sfp, 226)); print_byte(sfp_read_reg(sfp, 227)); write_char('\n'); - print_string("TX Bias: "); print_byte(sfp_read_reg(sfp, 228)); print_byte(sfp_read_reg(sfp, 229)); write_char('\n'); - print_string("TX Power: "); print_byte(sfp_read_reg(sfp, 230)); print_byte(sfp_read_reg(sfp, 231)); write_char('\n'); - print_string("RX Power: "); print_byte(sfp_read_reg(sfp, 232)); print_byte(sfp_read_reg(sfp, 233)); write_char('\n'); - print_string("Laser: "); print_byte(sfp_read_reg(sfp, 234)); print_byte(sfp_read_reg(sfp, 235)); write_char('\n'); - print_string("State: "); print_byte(sfp_read_reg(sfp, 238)); write_char('\n'); + return true; + if (!sfp_read_block(sfp, 224, 16)) + return false; + print_string("Temp: "); print_byte(sfp_buf[0]); print_byte(sfp_buf[1]); write_char('\n'); + print_string("Vcc: "); print_byte(sfp_buf[2]); print_byte(sfp_buf[3]); write_char('\n'); + print_string("TX Bias: "); print_byte(sfp_buf[4]); print_byte(sfp_buf[5]); write_char('\n'); + print_string("TX Power: "); print_byte(sfp_buf[6]); print_byte(sfp_buf[7]); write_char('\n'); + print_string("RX Power: "); print_byte(sfp_buf[8]); print_byte(sfp_buf[9]); write_char('\n'); + print_string("Laser: "); print_byte(sfp_buf[10]); print_byte(sfp_buf[11]); write_char('\n'); + print_string("State: "); print_byte(sfp_buf[14]); write_char('\n'); + + return true; } @@ -777,13 +784,14 @@ void parse_sfp(void) print_string(" - empty\n"); continue; } - sfp_i2c_fail = 0; - print_string(" - Rate: "); print_byte(sfp_read_reg(slot, 12)); - print_string(" Encoding: "); print_byte(sfp_read_reg(slot, 11)); + if (!sfp_read_block(slot, 11, 2)) { + print_string(" - I2C read failed on this slot\n"); + continue; + } + print_string(" - Rate: "); print_byte(sfp_buf[1]); + print_string(" Encoding: "); print_byte(sfp_buf[0]); write_char('\n'); - sfp_print_info(slot); - sfp_print_measurements(slot); - if (sfp_i2c_fail) + if (!sfp_print_info(slot) || !sfp_print_measurements(slot)) print_string("I2C read failed on this slot\n"); } return; diff --git a/httpd/page_impl.c b/httpd/page_impl.c index 19796ed..4101031 100644 --- a/httpd/page_impl.c +++ b/httpd/page_impl.c @@ -177,10 +177,12 @@ void reg_to_html_long(register uint16_t reg) void send_sfp_info(uint8_t sfp) { // This loops over the Vendor-name, Vendor OUI, Vendor PN and Vendor rev ASCII fields - for (uint8_t i = 20; i < 60; i++) { - if (i >= 36 && i < 40) // Skip Non-ASCII codes + for (uint8_t i = 16; i < 64; i++) { + if (!(i & 0xf)) + sfp_read_block(sfp, i, 16); + if (i < 20 || i >= 60 || (i >= 36 && i < 40)) // Skip Non-ASCII codes continue; - uint8_t c = sfp_read_reg(sfp, i); + uint8_t c = sfp_buf[i & 0xf]; if (c && c != 0xa0) // a0 is the byte read from a non-existant I2C EEPROM char_to_html(c); } @@ -193,32 +195,10 @@ void sfp_send_data(uint8_t slot, uint8_t reg, uint8_t len) if (len > 16) return; - if (reg & 0x80) { // Configure SFP readings address (0x51) as I2C device address - reg &= 0x7f; - REG_WRITE(RTL837X_REG_I2C_CTRL, 0x00, 0x1 << (I2C_MEM_ADDR_WIDTH-16) | (len - 1) & 0xf, 0x51 >> 5, (0x51 << 3) & 0xff); - } else { - REG_WRITE(RTL837X_REG_I2C_CTRL, 0x00, 0x1 << (I2C_MEM_ADDR_WIDTH-16) | (len - 1) & 0xf, 0x50 >> 5, (0x50 << 3) & 0xff); - } + sfp_read_block(slot, reg, len); - reg_read_m(RTL837X_REG_I2C_CTRL); - sfr_mask_data(1, 0xfc, i2c_bus_from_scl_pin(machine.sfp_port[slot].i2c.scl) << 5 | i2c_bus_from_sda_pin(machine.sfp_port[slot].i2c.sda) << 2); - reg_write_m(RTL837X_REG_I2C_CTRL); - - REG_WRITE(RTL837X_REG_I2C_IN, 0, 0, 0, reg); - - // Execute I2C Read - reg_bit_set(RTL837X_REG_I2C_CTRL, 0); - - // Wait for execution to finish - do { - reg_read_m(RTL837X_REG_I2C_CTRL); - } while (sfr_data[3] & 0x1); - - for (uint8_t i = 0; i < len; i++) { - if (!(i & 0x3)) - reg_read_m(RTL837X_REG_I2C_OUT + i); - byte_to_html(sfr_data[3 - (i & 0x3)]); - } + for (uint8_t i = 0; i < len; i++) + byte_to_html(sfp_buf[i]); } diff --git a/rtl837x_common.h b/rtl837x_common.h index 5432343..9fe2c0c 100644 --- a/rtl837x_common.h +++ b/rtl837x_common.h @@ -149,8 +149,8 @@ void sleep(uint16_t t); void write_char_no_syslog(char c); void write_char(char c); void print_reg(uint16_t reg); -uint8_t sfp_read_reg(uint8_t slot, uint8_t reg) __banked; -extern __xdata uint8_t sfp_i2c_fail; +bool sfp_read_block(uint8_t slot, uint8_t reg, uint8_t len) __banked __reentrant; +extern __xdata uint8_t sfp_buf[16]; void reg_bit_set(uint16_t reg_addr, char bit); void reg_bit_clear(uint16_t reg_addr, char bit); uint8_t reg_bit_test(uint16_t reg_addr, char bit); @@ -169,7 +169,7 @@ void tcpip_output(void); uint8_t read_flash(uint8_t bank, __code uint8_t *addr); void get_random_32(void); void read_reg_timer(__xdata uint32_t * tmr); -void sfp_print_info(uint8_t sfp); +bool sfp_print_info(uint8_t sfp); bool gpio_pin_test(uint8_t pin); void set_sys_led_state(uint8_t state); void sds_read(uint8_t sds_id, uint8_t page, uint8_t reg); diff --git a/rtl837x_pins.c b/rtl837x_pins.c index ac0cea3..bee9818 100644 --- a/rtl837x_pins.c +++ b/rtl837x_pins.c @@ -129,39 +129,53 @@ void gpio_output_setup(uint8_t pin, __xdata uint8_t initial_val) __banked{ /* - * Read a register of the EEPROM via I2C + * Read up to 16 consecutive registers of the EEPROM via I2C into sfp_buf */ -uint8_t sfp_read_reg(uint8_t slot, uint8_t reg) __banked +bool sfp_read_block(uint8_t slot, uint8_t reg, uint8_t len) __banked __reentrant { - if (reg & 0x80) { // Configure SFP readings address (0x51) as I2C device address - reg &= 0x7f; - REG_WRITE(RTL837X_REG_I2C_CTRL, 0x00, 0x1 << (I2C_MEM_ADDR_WIDTH-16) | 0, 0x51 >> 5, (0x51 << 3) & 0xff); - } else { - REG_WRITE(RTL837X_REG_I2C_CTRL, 0x00, 0x1 << (I2C_MEM_ADDR_WIDTH-16) | 0, 0x50 >> 5, (0x50 << 3) & 0xff); - } + uint8_t dev; + uint8_t val; - reg_read_m(RTL837X_REG_I2C_CTRL); - sfr_mask_data(1, 0xfc, i2c_bus_from_scl_pin(machine.sfp_port[slot].i2c.scl) << 5 | i2c_bus_from_sda_pin(machine.sfp_port[slot].i2c.sda) << 2); - reg_write_m(RTL837X_REG_I2C_CTRL); + len--; + if (len > 15) + return false; + + dev = (reg & 0x80) ? 0x51 : 0x50; // 0x51 holds the diagnostics, 0x50 the module data + reg &= 0x7f; REG_WRITE(RTL837X_REG_I2C_IN, 0, 0, 0, reg); - // Execute I2C Read - reg_bit_set(RTL837X_REG_I2C_CTRL, 0); + REG_WRITE(RTL837X_REG_I2C_CTRL, 0x00, + 0x1 << (I2C_MEM_ADDR_WIDTH - 16) | len, + (dev >> 5) | i2c_bus_from_scl_pin(machine.sfp_port[slot].i2c.scl) << 5 + | i2c_bus_from_sda_pin(machine.sfp_port[slot].i2c.sda) << 2, + ((dev << 3) & 0xff) | 0x1); - // Wait for execution to finish do { - reg_read_m(RTL837X_REG_I2C_CTRL); - } while (sfr_data[3] & 0x1); + reg_read(RTL837X_REG_I2C_CTRL); + } while (SFR_DATA_0 & 0x1); - /* Bit 1 is the controller's own failure indication, which the vendor SDK - * looks at and this did not. Without it an unacknowledged address comes - * back as an ordinary byte and the caller cannot tell it from data. */ - if (sfr_data[3] & 0x2) { - sfp_i2c_fail = 1; - return 0xff; + if (SFR_DATA_0 & 0x2) + return false; + + for (uint8_t i = 0; i <= len; i++) { + switch (i & 0x3) { + case 0: + reg_read(RTL837X_REG_I2C_OUT + i); + val = SFR_DATA_0; + break; + case 1: + val = SFR_DATA_8; + break; + case 2: + val = SFR_DATA_16; + break; + default: + val = SFR_DATA_24; + break; + } + sfp_buf[i] = val; } - reg_read_m(RTL837X_REG_I2C_OUT); - return sfr_data[3]; + return true; } diff --git a/rtlplayground.c b/rtlplayground.c index 6926ae5..420ec9e 100644 --- a/rtlplayground.c +++ b/rtlplayground.c @@ -139,7 +139,7 @@ __xdata char sfp_module_vendor[2][17]; __xdata char sfp_module_model[2][17]; __xdata char sfp_module_serial[2][17]; __xdata uint8_t sfp_options[2]; -__xdata uint8_t sfp_i2c_fail; /* set by sfp_read_reg() when the controller flags a failed transfer */ +__xdata uint8_t sfp_buf[16]; /* scratch for one I2C transaction, the controller reads at most 16 bytes */ __xdata uint8_t sfp_speed[2]; __xdata uint8_t sfp_quirks[2]; __xdata bool button_last; @@ -1188,36 +1188,46 @@ static inline uint8_t sfp_rate_to_sds_config(register uint8_t rate) } -void sfp_print_info(uint8_t sfp) +bool sfp_print_info(uint8_t sfp) { // This loops over the Vendor-name, Vendor OUI, Vendor PN and Vendor rev ASCII fields - for (uint8_t i = 20; i < 60; i++) { - if (i >= 36 && i < 40) // Skip Non-ASCII codes + for (uint8_t i = 16; i < 64; i++) { + if (!(i & 0xf) && !sfp_read_block(sfp, i, 16)) + return false; + if (i < 20 || i >= 60 || (i >= 36 && i < 40)) // Skip Non-ASCII codes continue; - uint8_t c = sfp_read_reg(sfp, i); + uint8_t c = sfp_buf[i & 0xf]; if (c) write_char(c); } print_string("\n"); + + return true; } // Normalize strings from EEPROM by removing any trailing spaces; this allows simpler comparisons -void sfp_read_field(__xdata char *dst, uint8_t sfp, uint8_t start, uint8_t length) __reentrant +bool sfp_read_field(__xdata char *dst, uint8_t sfp, uint8_t start, uint8_t length) __reentrant { - dst[length] = '\0'; + if (!sfp_read_block(sfp, start, length)) + return false; - for (uint8_t i = 0; i < length; i++) - dst[i] = sfp_read_reg(sfp, start + i); + dst[length] = '\0'; + memcpy(dst, sfp_buf, length); while (length > 0 && dst[--length] == ' ') dst[length] = '\0'; + + return true; } -void sfp_get_info(uint8_t sfp) +bool sfp_get_info(uint8_t sfp) { - sfp_read_field(sfp_module_vendor[sfp], sfp, 20, 16); - sfp_read_field(sfp_module_model[sfp], sfp, 40, 16); - sfp_read_field(sfp_module_serial[sfp], sfp, 68, 16); + if (!sfp_read_field(sfp_module_vendor[sfp], sfp, 20, 16)) + return false; + if (!sfp_read_field(sfp_module_model[sfp], sfp, 40, 16)) + return false; + + return sfp_read_field(sfp_module_serial[sfp], sfp, 68, 16); } void sfp_apply_quirks(uint8_t sfp) __reentrant @@ -1236,7 +1246,7 @@ void sfp_apply_quirks(uint8_t sfp) __reentrant if (!(sfp_options[sfp] & 0x40)) { // The module reports that DDM is not implemented, but try a dummy read to confirm // 0xff would mean a failed I2C read or an impossible (per spec) voltage greater than 6.5V - if (sfp_read_reg(sfp, 226) != 0xff) { + if (sfp_read_block(sfp, 226, 1) && sfp_buf[0] != 0xff) { sfp_options[sfp] |= 0x40; } } @@ -1260,6 +1270,45 @@ void setup_sfp_gpio(void) } } +static bool sfp_module_read(uint8_t sfp) +{ + uint8_t rate; + + // Read Reg 11: Encoding, see SFF-8472 and SFF-8024 + // Read Reg 12: Signalling rate (including overhead) in 100Mbit: 0xd: 1Gbit, 0x67:10Gbit + delay(100); // Delay, because some modules need time to wake up + if (!sfp_read_block(sfp, 11, 2)) + return false; + + rate = sfp_buf[1]; + if (sfp_speed[sfp] == SFP_SPEED_100M) + rate = 0x1; + else if (sfp_speed[sfp] == SFP_SPEED_1G) + rate = 0xc; + else if (sfp_speed[sfp] == SFP_SPEED_2G5) + rate = 0x19; + else if (sfp_speed[sfp] == SFP_SPEED_10G) + rate = 0x69; + print_string(" Rate: "); print_byte(rate); // Normally 1, but 0 for DAC, can be ignored? + print_string(" Encoding: "); print_byte(sfp_buf[0]); + print_string(" Module: "); + if (!sfp_print_info(sfp)) + return false; + print_string("\n"); + + if (!sfp_read_block(sfp, 92, 1)) + return false; + sfp_options[sfp] = sfp_buf[0]; + if (!sfp_get_info(sfp)) + return false; + + sfp_apply_quirks(sfp); + sds_config(machine.sfp_port[sfp].sds, sfp_rate_to_sds_config(rate)); + + return true; +} + + void handle_sfp(void) { for (uint8_t sfp = 0; sfp < machine.n_sfp; sfp++) { @@ -1267,29 +1316,10 @@ void handle_sfp(void) if (sfp_pins_last & (0x1 << (sfp << 2))) { sfp_pins_last &= ~(0x01 << (sfp << 2)); print_string("\n Slot: "); write_char('1' + sfp); - // Read Reg 11: Encoding, see SFF-8472 and SFF-8024 - // Read Reg 12: Signalling rate (including overhead) in 100Mbit: 0xd: 1Gbit, 0x67:10Gbit - delay(100); // Delay, because some modules need time to wake up - sfp_i2c_fail = 0; - uint8_t rate = sfp_read_reg(sfp, 12); - if (sfp_speed[sfp] == SFP_SPEED_100M) - rate = 0x1; - else if (sfp_speed[sfp] == SFP_SPEED_1G) - rate = 0xc; - else if (sfp_speed[sfp] == SFP_SPEED_2G5) - rate = 0x19; - else if (sfp_speed[sfp] == SFP_SPEED_10G) - rate = 0x69; - print_string(" Rate: "); print_byte(rate); // Normally 1, but 0 for DAC, can be ignored? - print_string(" Encoding: "); print_byte(sfp_read_reg(sfp, 11)); - print_string(" Module: "); sfp_print_info(sfp); - print_string("\n"); - sfp_options[sfp] = sfp_read_reg(sfp, 92); - sfp_get_info(sfp); - sfp_apply_quirks(sfp); - if (sfp_i2c_fail) - print_string("SFP: an I2C read failed, the module data above may be wrong\n"); - sds_config(machine.sfp_port[sfp].sds, sfp_rate_to_sds_config(rate)); + if (!sfp_module_read(sfp)) { + print_string("SFP: an I2C read failed, retrying on the next poll\n"); + sfp_pins_last |= 0x01 << (sfp << 2); + } } } else { if (!(sfp_pins_last & (0x1 << (sfp << 2)))) { From 425283b77456734119fb8c20c0cc50cf36f2d05a Mon Sep 17 00:00:00 2001 From: bloqaudio Date: Mon, 24 Aug 2026 23:36:17 -0500 Subject: [PATCH 08/36] machine: host the per-machine custom init hooks in BANK2 machine_custom_init() runs once at boot, but its code and any tables it uses were compiled into machine.c and so into the common bank. On the SWTG018AS-V2.1.0 variant, whose init carries a 21-entry LED register table, that overflows bank 0 by 0x66 bytes and main no longer links for MACHINE_PCB_SWTG018AS_V2_1_0; any machine whose init grows can hit the same wall. SDCC segment pragmas apply file-wide, so the hooks move to a new machine_init.c compiled into BANK2, and the prototype becomes banked. machine_check also compiles the new file per machine so the hooks keep CI syntax coverage. --- Makefile | 2 + machine.c | 141 ------------------------------------------------- machine.h | 2 +- machine_init.c | 139 ++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 142 insertions(+), 142 deletions(-) create mode 100644 machine_init.c diff --git a/Makefile b/Makefile index ec66eb6..301f4f0 100644 --- a/Makefile +++ b/Makefile @@ -53,6 +53,7 @@ create_build_dir: # Keep machine.c in first position to fail immediately on invalid $MACHINE value SRCS = \ machine.c \ + machine_init.c \ cmd_editor.c \ cmd_parser.c \ dhcp.c \ @@ -142,6 +143,7 @@ machine_check: do \ echo "Checking $${MACHINE}"; \ $(CC) $(CC_FLAGS) -DMACHINE_$${MACHINE} -MMD -o $(BUILDDIR)/tmp/machine_check -c machine.c; \ + $(CC) $(CC_FLAGS) -DMACHINE_$${MACHINE} -MMD -o $(BUILDDIR)/tmp/machine_check -c machine_init.c; \ done @rm -rf $(BUILDDIR)/tmp diff --git a/machine.c b/machine.c index d7d4823..fcdb007 100644 --- a/machine.c +++ b/machine.c @@ -42,8 +42,6 @@ __code const struct machine machine = { }, }; -void machine_custom_init(void) { } - #elif defined MACHINE_KP_9000_6XH_X __code const struct machine machine = { .machine_name = "keepLink KP-9000-6XH-X", @@ -72,8 +70,6 @@ __code const struct machine machine = { }, }; -void machine_custom_init(void) { } - #elif defined MACHINE_KP_9000_6XH_X2 __code const struct machine machine = { .machine_name = "keepLink KP-9000-6XH-X2", @@ -122,10 +118,6 @@ __code const struct machine machine = { }, }; -void machine_custom_init(void) { - reg_bit_set(RTL837X_REG_LED_GLB_IO_EN, 6); -} - #elif defined MACHINE_KP_9000_9XH_X_EU __code const struct machine machine = { .machine_name = "keepLink KP-9000-9XH-X-EU", @@ -151,8 +143,6 @@ __code const struct machine machine = { }, }; -void machine_custom_init(void) { } - #elif defined MACHINE_KP_9000_9XHML_X_V2_2 __code const struct machine machine = { .machine_name = "keepLink KP-9000-9XHML-X V2.2", @@ -204,8 +194,6 @@ __code const struct machine machine = { }, }; -void machine_custom_init(void) { } - #elif defined MACHINE_KP_9000_9XHML_X_V3_1 __code const struct machine machine = { .machine_name = "keepLink KP-9000-9XHML-X V3.1", @@ -241,8 +229,6 @@ __code const struct machine machine = { 0x1a, 0x19, 0x1d, 0x1e, 0x1c, 0x1d, 0x20, 0x21}, }; -void machine_custom_init(void) { } - #elif defined MACHINE_SWGT024_V2_0_MANAGED __code const struct machine machine = { .machine_name = "SWGT024 V2.0 Managed", @@ -284,8 +270,6 @@ __code const struct machine machine = { }, }; -void machine_custom_init(void) { } - #elif defined MACHINE_SWGT024_V2_0_UNMANAGED __code const struct machine machine = { .machine_name = "SWGT024 V2.0 Unmanaged", @@ -327,8 +311,6 @@ __code const struct machine machine = { }, }; -void machine_custom_init(void) { } - #elif defined MACHINE_SWTG018AS_A_V_2_0 __code const struct machine machine = { .machine_name = "SWTG018AS-A V2.0", @@ -368,8 +350,6 @@ __code const struct machine machine = { 0x1d, 0x20, 0x21 }, }; -void machine_custom_init(void) { } - #elif defined MACHINE_HG0402XG_V1_1 __code const struct machine machine = { .machine_name = "HG0402XG V1.1", @@ -408,8 +388,6 @@ __code const struct machine machine = { }, }; -void machine_custom_init(void) { } - #elif defined MACHINE_SWTGW218AS __code const struct machine machine = { @@ -443,8 +421,6 @@ __code const struct machine machine = { }, }; -void machine_custom_init(void) { } - #elif defined MACHINE_PCB_SWTG018AS_V2_1_0 // Sold as Sodola SL902 / Horaco "SWTGW218AS"; the SWTGW218AS label also covers other PCBs with different SFP and LED wiring (see MACHINE_SWTGW218AS) __code const struct machine machine = { @@ -487,44 +463,6 @@ __code const struct machine machine = { 0x1d, 0x20, 0x21 }, }; -// The LED-set encoding cannot express this board's bi-color SFP LED (green <= 2.5G, -// blue at 10G), so program the LED register block with the values the stock firmware -// uses. Runs after leds_setup() and overrides the values computed there. -// The final entry routes the blue-LED pin to the LED controller via PIN_MUX_0; -// as a GPIO (the default) no LED register can light it. PIN_MUX_1/2 stay -// untouched so SFP detect (GPIO38) and i2c remain GPIOs. -static __code const struct { uint16_t reg; uint32_t val; } custom_init_regs[] = { - { 0x6520, 0x0023e430UL }, // LED_MODE - { 0x6524, 0xff001400UL }, // LED3_0_SET3 - { 0x6528, 0x00100000UL }, // LED3_0_SET1 - { 0x652c, 0x007f013fUL }, // LED3_2_SET3 - { 0x6530, 0x02000400UL }, // LED1_0_SET3 - { 0x6534, 0x01400141UL }, // LED3_2_SET2 - { 0x6538, 0x01440170UL }, // LED1_0_SET2 - { 0x653c, 0x18000041UL }, // LED3_2_SET1 - { 0x6540, 0x01400155UL }, // LED1_0_SET1 - { 0x6544, 0x01411000UL }, // LED3_2_SET0 - { 0x6548, 0x01740141UL }, // LED1_0_SET0 - { 0x654c, 0x00010000UL }, // LED_PORT_SET_SEL - { 0x65d8, 0x3ffb6dffUL }, // LED_GLB_ACTIVE - { 0x65dc, 0x7f24977fUL }, // LED_GLB_IO_EN - { 0x65e0, 0x08144040UL }, // LED_GLB_MUX_1 - { 0x65e4, 0x10349309UL }, // LED_GLB_MUX_2 - { 0x65e8, 0x12454391UL }, // LED_GLB_MUX_3 - { 0x65ec, 0x19616555UL }, // LED_GLB_MUX_4 - { 0x65f0, 0x1c79d65aUL }, // LED_GLB_MUX_5 - { 0x65f4, 0x0002181dUL }, // LED_GLB_MUX_6 - { 0x7f8c, 0x20db6880UL }, // PIN_MUX_0 -}; - -void machine_custom_init(void) { - uint8_t i; - // REG_SET is a multi-statement macro without a do-while wrapper: braces required - for (i = 0; i < sizeof(custom_init_regs) / sizeof(custom_init_regs[0]); i++) { - REG_SET(custom_init_regs[i].reg, custom_init_regs[i].val); - } -} - #elif defined MACHINE_LIANGUO_ZX_SWTGW215AS // Has PCB branded PCB-SWTG115AS-V2.0 but is labeled and reports as a ZX-SWTGW215AS, seems to be identical to the "real" ZX-SWTGW215AS except for the LEDs __code const struct machine machine = { .machine_name = "Lianguo ZX-SWTGW215AS", @@ -557,8 +495,6 @@ __code const struct machine machine = { .led_mux_custom = 0, }; -void machine_custom_init(void) { } - #elif defined MACHINE_DEFAULT_8C_1SFP __code const struct machine machine = { .machine_name = "8+1 SFP Port Switch", @@ -584,8 +520,6 @@ __code const struct machine machine = { }, }; -void machine_custom_init(void) { } - #elif defined MACHINE_TRENDNET_TEG_S562 __code const struct machine machine = { .machine_name = "Trendnet TEG-S562", @@ -625,8 +559,6 @@ __code const struct machine machine = { }; -void machine_custom_init(void) { } - #elif defined(MACHINE_PCB_K0402WS_V3) || defined(MACHINE_HI_K0402WS) // Sold as a variety of devices, see doc/ __code const struct machine machine = { .machine_name = "PCB-K0402WS-V3.0", @@ -673,10 +605,6 @@ __code const struct machine machine = { }, }; -void machine_custom_init(void) { - reg_bit_set(RTL837X_REG_LED_GLB_IO_EN, 6); -} - #elif defined MACHINE_K0501W_V2_0 __code const struct machine machine = { .machine_name = "K0501W V2.0", @@ -711,8 +639,6 @@ __code const struct machine machine = { }, }; -void machine_custom_init(void) { } - #elif defined MACHINE_ZX310S_4T2XH __code const struct machine machine = { .machine_name = "ZX310S-4T2XH", @@ -756,8 +682,6 @@ __code const struct machine machine = { }, }; -void machine_custom_init(void) { } - #elif defined MACHINE_STEAMEMO_IG204_V1 __code const struct machine machine = { .machine_name = "Steamemo IG204 V1", @@ -808,8 +732,6 @@ __code const struct machine machine = { }, }; -void machine_custom_init(void) { } - #elif defined MACHINE_HI_K0801WS __code const struct machine machine = { .machine_name = "Hi-Source HI-k0801WS", @@ -859,8 +781,6 @@ __code const struct machine machine = { }, }; -void machine_custom_init(void) { } - #elif defined MACHINE_FNS1200P __code const struct machine machine = { .machine_name = "FNS-1200P", @@ -916,11 +836,6 @@ __code const struct machine machine = { }, }; -void machine_custom_init(void) -{ - reg_bit_set(RTL837X_REG_LED_GLB_IO_EN, 6); -} - #elif defined MACHINE_PCB_SWTG024AS_A_2_0_1 __code const struct machine machine = { @@ -970,14 +885,6 @@ __code const struct machine machine = { }, }; -void machine_custom_init(void) -{ - reg_bit_set(RTL837X_REG_LED_GLB_IO_EN, 6); - reg_bit_set(RTL837X_REG_LED_MODE, 17); - reg_bit_clear(RTL837X_REG_LED_MODE, 9); - reg_bit_clear(RTL837X_REG_LED_MODE, 7); -} - #elif defined MACHINE_SWTG024AS_A_2_0_1_5C_1SFP __code const struct machine machine = { .machine_name = "SWTG024AS-A-V2.0.1-5C-1SFP", @@ -1021,25 +928,6 @@ __code const struct machine machine = { }, }; -void machine_custom_init(void) -{ - uint16_t pval; - - reg_bit_set(RTL837X_REG_LED_GLB_IO_EN, 6); - reg_bit_set(RTL837X_REG_LED_MODE, 17); - reg_bit_clear(RTL837X_REG_LED_MODE, 9); - reg_bit_clear(RTL837X_REG_LED_MODE, 7); - - // OEM firmware sets these companion SDS0 polarity bits for the RTL8221B. - sds_read(0, 0, 0); - pval = SFR_DATA_U16; - sds_write_v(0, 0, 0, pval | 0x100); - - sds_read(0, 6, 2); - pval = SFR_DATA_U16; - sds_write_v(0, 6, 2, pval | 0x4000); -} - #elif defined MACHINE_SWTG024AS_V2_0 __code const struct machine machine = { .machine_name = "SWTG024AS-V2.0", @@ -1083,25 +971,6 @@ __code const struct machine machine = { }, }; -void machine_custom_init(void) -{ - uint16_t pval; - - reg_bit_set(RTL837X_REG_LED_GLB_IO_EN, 6); - reg_bit_set(RTL837X_REG_LED_MODE, 17); - reg_bit_clear(RTL837X_REG_LED_MODE, 9); - reg_bit_clear(RTL837X_REG_LED_MODE, 7); - - // OEM firmware sets these companion SDS0 polarity bits for the RTL8221B. - sds_read(0, 0, 0); - pval = SFR_DATA_U16; - sds_write_v(0, 0, 0, pval | 0x100); - - sds_read(0, 6, 2); - pval = SFR_DATA_U16; - sds_write_v(0, 6, 2, pval | 0x4000); -} - #elif defined MACHINE_ZX310S_4T2XT __code const struct machine machine = { .machine_name = "ZX310S_4T2XT", @@ -1139,12 +1008,6 @@ __code const struct machine machine = { }, }; -void machine_custom_init(void) { - // For this device, the reset value of RTL837X_PIN_MUX_0 is 0x30000000, - // which would disables all LEDS, enable them manually: - REG_SET(RTL837X_PIN_MUX_0, 0x30db68bf); -} - #elif defined MACHINE_FG_4GT_2SX_V2_0 __code const struct machine machine = { .machine_name = "FG-4GT-2SX_V2.0", @@ -1211,10 +1074,6 @@ __code const struct machine machine = { }, }; -void machine_custom_init(void) { - REG_SET(RTL837X_REG_LED_GLB_IO_EN, 0x7624155b); -} - #else #error "Please select a machine type in machine.h" #endif diff --git a/machine.h b/machine.h index 29fe39f..ae467cb 100644 --- a/machine.h +++ b/machine.h @@ -95,6 +95,6 @@ typedef struct machine_runtime uint8_t isN : 1; }; -void machine_custom_init(void); +void machine_custom_init(void) __banked; #endif diff --git a/machine_init.c b/machine_init.c new file mode 100644 index 0000000..e5669ed --- /dev/null +++ b/machine_init.c @@ -0,0 +1,139 @@ +/* + * Per-machine one-shot boot hooks, hosted in BANK2 so board-specific + * tables and code do not consume the common bank. + */ +#include +#include "machine.h" +#include "rtl837x_pins.h" +#include "rtl837x_leds.h" +#include "rtl837x_sfr.h" +#include "rtl837x_regs.h" +#include "rtl837x_common.h" + +#pragma codeseg BANK2 +#pragma constseg BANK2 + +#if defined MACHINE_KP_9000_6XH_X2 +void machine_custom_init(void) __banked +{ + reg_bit_set(RTL837X_REG_LED_GLB_IO_EN, 6); +} + +#elif defined MACHINE_PCB_SWTG018AS_V2_1_0 +// The LED-set encoding cannot express this board's bi-color SFP LED (green <= 2.5G, +// blue at 10G), so program the LED register block with the values the stock firmware +// uses. Runs after leds_setup() and overrides the values computed there. +// The final entry routes the blue-LED pin to the LED controller via PIN_MUX_0; +// as a GPIO (the default) no LED register can drive it. PIN_MUX_1/2 stay +// untouched so SFP detect (GPIO38) and i2c remain GPIOs. +static __code const struct { uint16_t reg; uint32_t val; } custom_init_regs[] = { + { 0x6520, 0x0023e430UL }, // LED_MODE + { 0x6524, 0xff001400UL }, // LED3_0_SET3 + { 0x6528, 0x00100000UL }, // LED3_0_SET1 + { 0x652c, 0x007f013fUL }, // LED3_2_SET3 + { 0x6530, 0x02000400UL }, // LED1_0_SET3 + { 0x6534, 0x01400141UL }, // LED3_2_SET2 + { 0x6538, 0x01440170UL }, // LED1_0_SET2 + { 0x653c, 0x18000041UL }, // LED3_2_SET1 + { 0x6540, 0x01400155UL }, // LED1_0_SET1 + { 0x6544, 0x01411000UL }, // LED3_2_SET0 + { 0x6548, 0x01740141UL }, // LED1_0_SET0 + { 0x654c, 0x00010000UL }, // LED_PORT_SET_SEL + { 0x65d8, 0x3ffb6dffUL }, // LED_GLB_ACTIVE + { 0x65dc, 0x7f24977fUL }, // LED_GLB_IO_EN + { 0x65e0, 0x08144040UL }, // LED_GLB_MUX_1 + { 0x65e4, 0x10349309UL }, // LED_GLB_MUX_2 + { 0x65e8, 0x12454391UL }, // LED_GLB_MUX_3 + { 0x65ec, 0x19616555UL }, // LED_GLB_MUX_4 + { 0x65f0, 0x1c79d65aUL }, // LED_GLB_MUX_5 + { 0x65f4, 0x0002181dUL }, // LED_GLB_MUX_6 + { 0x7f8c, 0x20db6880UL }, // PIN_MUX_0 +}; + +void machine_custom_init(void) __banked +{ + uint8_t i; + // REG_SET is a multi-statement macro without a do-while wrapper: braces required + for (i = 0; i < sizeof(custom_init_regs) / sizeof(custom_init_regs[0]); i++) { + REG_SET(custom_init_regs[i].reg, custom_init_regs[i].val); + } +} + +#elif defined(MACHINE_PCB_K0402WS_V3) || defined(MACHINE_HI_K0402WS) +void machine_custom_init(void) __banked +{ + reg_bit_set(RTL837X_REG_LED_GLB_IO_EN, 6); +} + +#elif defined MACHINE_FNS1200P +void machine_custom_init(void) __banked +{ + reg_bit_set(RTL837X_REG_LED_GLB_IO_EN, 6); +} + +#elif defined MACHINE_PCB_SWTG024AS_A_2_0_1 +void machine_custom_init(void) __banked +{ + reg_bit_set(RTL837X_REG_LED_GLB_IO_EN, 6); + reg_bit_set(RTL837X_REG_LED_MODE, 17); + reg_bit_clear(RTL837X_REG_LED_MODE, 9); + reg_bit_clear(RTL837X_REG_LED_MODE, 7); +} + +#elif defined MACHINE_SWTG024AS_A_2_0_1_5C_1SFP +void machine_custom_init(void) __banked +{ + uint16_t pval; + + reg_bit_set(RTL837X_REG_LED_GLB_IO_EN, 6); + reg_bit_set(RTL837X_REG_LED_MODE, 17); + reg_bit_clear(RTL837X_REG_LED_MODE, 9); + reg_bit_clear(RTL837X_REG_LED_MODE, 7); + + // OEM firmware sets these companion SDS0 polarity bits for the RTL8221B. + sds_read(0, 0, 0); + pval = SFR_DATA_U16; + sds_write_v(0, 0, 0, pval | 0x100); + + sds_read(0, 6, 2); + pval = SFR_DATA_U16; + sds_write_v(0, 6, 2, pval | 0x4000); +} + +#elif defined MACHINE_SWTG024AS_V2_0 +void machine_custom_init(void) __banked +{ + uint16_t pval; + + reg_bit_set(RTL837X_REG_LED_GLB_IO_EN, 6); + reg_bit_set(RTL837X_REG_LED_MODE, 17); + reg_bit_clear(RTL837X_REG_LED_MODE, 9); + reg_bit_clear(RTL837X_REG_LED_MODE, 7); + + // OEM firmware sets these companion SDS0 polarity bits for the RTL8221B. + sds_read(0, 0, 0); + pval = SFR_DATA_U16; + sds_write_v(0, 0, 0, pval | 0x100); + + sds_read(0, 6, 2); + pval = SFR_DATA_U16; + sds_write_v(0, 6, 2, pval | 0x4000); +} + +#elif defined MACHINE_ZX310S_4T2XT +void machine_custom_init(void) __banked +{ + // For this device, the reset value of RTL837X_PIN_MUX_0 is 0x30000000, + // which would disables all LEDS, enable them manually: + REG_SET(RTL837X_PIN_MUX_0, 0x30db68bf); +} + +#elif defined MACHINE_FG_4GT_2SX_V2_0 +void machine_custom_init(void) __banked +{ + REG_SET(RTL837X_REG_LED_GLB_IO_EN, 0x7624155b); +} + +#else +void machine_custom_init(void) __banked { } +#endif From 1a743b3889316761b71d8df5e26c0d0c2d22df8d Mon Sep 17 00:00:00 2001 From: bloqaudio Date: Mon, 24 Aug 2026 23:36:51 -0500 Subject: [PATCH 09/36] leds: reduce the SWTG018AS-V2.1.0 custom init to the effective registers Measured against the generic leds_setup() output on the board: eight of the 21 stock values are identical to what leds_setup() computes from the machine's led_mux table, and eight more revert to the generic values with no change in LED behaviour in any tested link state (copper 2.5G, SFP 2.5G, SFP 10G). Keep the five with a measurable effect: 6528 selects blue over green at 10G, 6540/6548 carry the SFP and copper LED set behaviour, 65dc enables the LED outputs, and PIN_MUX_0 routes the blue pin. Verified from a clean boot on a SWTG018AS-V2.1.0 board: copper solid green with activity blink at 2.5G on two ports, SFP green at 2.5G, SFP blue at 10G, LEDs off on link down. --- machine_init.c | 35 ++++++++--------------------------- 1 file changed, 8 insertions(+), 27 deletions(-) diff --git a/machine_init.c b/machine_init.c index e5669ed..215c21d 100644 --- a/machine_init.c +++ b/machine_init.c @@ -20,34 +20,15 @@ void machine_custom_init(void) __banked } #elif defined MACHINE_PCB_SWTG018AS_V2_1_0 -// The LED-set encoding cannot express this board's bi-color SFP LED (green <= 2.5G, -// blue at 10G), so program the LED register block with the values the stock firmware -// uses. Runs after leds_setup() and overrides the values computed there. -// The final entry routes the blue-LED pin to the LED controller via PIN_MUX_0; -// as a GPIO (the default) no LED register can drive it. PIN_MUX_1/2 stay -// untouched so SFP detect (GPIO38) and i2c remain GPIOs. +// Stock-firmware values for what the LED-set encoding cannot express: the +// bi-color SFP LED (blue pin at 10G) and the PIN_MUX_0 routing of that pin +// to the LED controller. Runs after leds_setup(), which covers the rest. static __code const struct { uint16_t reg; uint32_t val; } custom_init_regs[] = { - { 0x6520, 0x0023e430UL }, // LED_MODE - { 0x6524, 0xff001400UL }, // LED3_0_SET3 - { 0x6528, 0x00100000UL }, // LED3_0_SET1 - { 0x652c, 0x007f013fUL }, // LED3_2_SET3 - { 0x6530, 0x02000400UL }, // LED1_0_SET3 - { 0x6534, 0x01400141UL }, // LED3_2_SET2 - { 0x6538, 0x01440170UL }, // LED1_0_SET2 - { 0x653c, 0x18000041UL }, // LED3_2_SET1 - { 0x6540, 0x01400155UL }, // LED1_0_SET1 - { 0x6544, 0x01411000UL }, // LED3_2_SET0 - { 0x6548, 0x01740141UL }, // LED1_0_SET0 - { 0x654c, 0x00010000UL }, // LED_PORT_SET_SEL - { 0x65d8, 0x3ffb6dffUL }, // LED_GLB_ACTIVE - { 0x65dc, 0x7f24977fUL }, // LED_GLB_IO_EN - { 0x65e0, 0x08144040UL }, // LED_GLB_MUX_1 - { 0x65e4, 0x10349309UL }, // LED_GLB_MUX_2 - { 0x65e8, 0x12454391UL }, // LED_GLB_MUX_3 - { 0x65ec, 0x19616555UL }, // LED_GLB_MUX_4 - { 0x65f0, 0x1c79d65aUL }, // LED_GLB_MUX_5 - { 0x65f4, 0x0002181dUL }, // LED_GLB_MUX_6 - { 0x7f8c, 0x20db6880UL }, // PIN_MUX_0 + { RTL837X_REG_LED3_0_SET1, 0x00100000UL }, + { RTL837X_REG_LED1_0_SET1, 0x01400155UL }, + { RTL837X_REG_LED1_0_SET0, 0x01740141UL }, + { RTL837X_REG_LED_GLB_IO_EN, 0x7f24977fUL }, + { RTL837X_PIN_MUX_0, 0x20db6880UL }, }; void machine_custom_init(void) __banked From 4937618684a29e1395e501f6fcc56907352bd2b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20van=20Dorst?= Date: Tue, 11 Aug 2026 20:50:36 +0200 Subject: [PATCH 10/36] Change atoi_short() so it returns number of bytes consumed. --- cmd_parser.c | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/cmd_parser.c b/cmd_parser.c index 842d3f5..7a46a52 100644 --- a/cmd_parser.c +++ b/cmd_parser.c @@ -80,6 +80,8 @@ __xdata uint16_t cmd_history_ptr; // Error set by commands __xdata uint8_t err_status; +__xdata uint16_t atoi_results_short; + inline uint8_t isletter(uint8_t l) { // return (l >= 'a' && l <= 'z') || (l >= 'A' && l <= 'Z'); @@ -203,22 +205,26 @@ uint8_t atoi_byte(__xdata uint8_t *out, uint8_t idx) return err; } - -uint8_t atoi_short(__xdata uint16_t *vlan, uint8_t idx) +// return 0 on error or non-zero number of number-byte taken for the conversion. +// Stops at any non-digit '0'-'9' char or bytes is more then 5. +uint8_t atoi_short(uint8_t idx) { - uint8_t err = 1; - *vlan = 0; + uint8_t cnt = 0; + atoi_results_short = 0; - while (isnumber(cmd_buffer[idx])) { - err = 0; - uint8_t val = cmd_buffer[idx] - '0'; - if (*vlan > 6553 || (*vlan == 6553 && val > 5)) - return 1; - *vlan = (*vlan * 10) + val; - idx++; + uint8_t *ptr = &cmd_buffer[idx]; + + while (1) { + uint8_t val = *ptr++ - '0'; + if (val > 9) + break; + if (atoi_results_short > 6553 || (atoi_results_short == 6553 && val > 5) || cnt >= 5) + return 0; + atoi_results_short = (atoi_results_short * 10) + val; + cnt++; } - return err; + return cnt; } From c265a20d8380d6d31e60412cb1fe64965f580c38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20van=20Dorst?= Date: Tue, 11 Aug 2026 21:12:07 +0200 Subject: [PATCH 11/36] Refactor the code for the new atoi_short() Free-up 1 SRAM-byte --- cmd_parser.c | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/cmd_parser.c b/cmd_parser.c index 7a46a52..8c9db08 100644 --- a/cmd_parser.c +++ b/cmd_parser.c @@ -355,11 +355,16 @@ void parse_vlan(void) vlan_settings.vlan = 0; vlan_settings.members = 0; vlan_settings.tagged = 0; + if (cmd_words_len < 2) goto err; - if (!atoi_short(&vlan_settings.vlan, cmd_words_b[1])) { + + uint8_t ret = atoi_short(cmd_words_b[1]); + if (ret) { if (cmd_words_len == 3 && cmd_buffer[cmd_words_b[2]] == 'd') { - vlan_delete(vlan_settings.vlan); + if (atoi_results_short > 4094) + goto err; + vlan_delete(atoi_results_short); return; } if (cmd_compare(2, "mgmt")) { @@ -376,7 +381,7 @@ void parse_vlan(void) goto err; uint8_t w = 2; if (cmd_words_len > w && isletter(cmd_buffer[cmd_words_b[w]])) { - register uint8_t i = 0; + uint8_t i = 0; vlan_name_remove(vlan_settings.vlan); vlan_names[vlan_ptr++] = hex[(vlan_settings.vlan >> 8) & 0xf]; vlan_names[vlan_ptr++] = hex[(vlan_settings.vlan >> 4) & 0xf] ; @@ -737,7 +742,7 @@ void parse_port(void) void parse_mtu(void) { __xdata uint16_t mtu; - uint8_t p; + uint8_t p, ret; if (cmd_compare(1, "show")) { for (p = machine.min_port; p <= machine.max_port; p++) { @@ -756,12 +761,15 @@ void parse_mtu(void) } p = machine.phys_to_log_port[cmd_buffer[cmd_words_b[1]] - '1']; print_byte(p); - if (atoi_short(&mtu, cmd_words_b[2]) || mtu < 64 || mtu > 0x3fff) { + + ret = atoi_short(cmd_words_b[2]); + + if (!ret || atoi_results_short < 64 || atoi_results_short > 0x3fff) { print_string("MTU must be 64..16383\n"); return; } - REG_WRITE(RTL8373_REG_MAC_L2_PORT_MAX_LEN + ((uint16_t) p << 8), (mtu >> 10) & 0xf, (mtu >> 2) & 0xff, - ((mtu & 0x3) << 6) | ((mtu >> 8) & 0x3f), mtu & 0xff); + REG_WRITE(RTL8373_REG_MAC_L2_PORT_MAX_LEN + ((uint16_t) p << 8), (atoi_results_short >> 10) & 0xf, (atoi_results_short >> 2) & 0xff, + ((atoi_results_short & 0x3) << 6) | ((atoi_results_short >> 8) & 0x3f), atoi_results_short & 0xff); write_char('\n'); } @@ -1597,12 +1605,12 @@ void cmd_parser(void) __banked stpEnabled = 0; } } else if (cmd_compare(0, "pvid") && cmd_words_len == 3) { - __xdata uint16_t pvid; + uint8_t ret; if (cmd_buffer[cmd_words_b[1]] >= '1' && cmd_buffer[cmd_words_b[1]] <= '9' && cmd_buffer[cmd_words_b[1] + 1] <= ' ' - && !atoi_short(&pvid, cmd_words_b[2]) && pvid && pvid <= 4094) - port_pvid_set(machine.phys_to_log_port[cmd_buffer[cmd_words_b[1]] - '1'], pvid); + && atoi_short(cmd_words_b[2]) && atoi_results_short && atoi_results_short <= 4094) + port_pvid_set(machine.phys_to_log_port[cmd_buffer[cmd_words_b[1]] - '1'], atoi_results_short); else print_string("Error: pvid <1-4094>\n"); } else if (cmd_compare(0, "vlan")) { From 80f97a50e0ed151dbd79e0b7bf4e867ba7aecd21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20van=20Dorst?= Date: Tue, 11 Aug 2026 21:15:48 +0200 Subject: [PATCH 12/36] Refactor atoi_byte() same as atoi_short() --- cmd_parser.c | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/cmd_parser.c b/cmd_parser.c index 8c9db08..c09d752 100644 --- a/cmd_parser.c +++ b/cmd_parser.c @@ -81,6 +81,7 @@ __xdata uint16_t cmd_history_ptr; __xdata uint8_t err_status; __xdata uint16_t atoi_results_short; +__xdata uint8_t atoi_results_u8; inline uint8_t isletter(uint8_t l) { @@ -187,24 +188,30 @@ uint8_t atoi_hex(uint8_t idx) } -uint8_t atoi_byte(__xdata uint8_t *out, uint8_t idx) +// return 0 on error or non-zero number of number-byte taken for the conversion. +// Stops at any non-digit '0'-'9' char or bytes is more then 3. +uint8_t atoi_byte(uint8_t idx) { - uint8_t err = 1; + uint8_t cnt = 0; uint8_t num = 0; - while (isnumber(cmd_buffer[idx])) { - uint8_t val = cmd_buffer[idx] - '0'; - err = 0; - if (num > 25 || (num == 25 && val > 5)) - return 1; + uint8_t * ptr = &cmd_buffer[idx]; + + while (1) { + uint8_t val = *ptr++ - '0'; + if (val > 9) + break; + if (num > 25 || (num == 25 && val > 5) || cnt >= 3) + return 0; num = (num * 10) + val; - idx++; + cnt++; } - *out = num; - return err; + atoi_results_u8 = num; + return cnt; } + // return 0 on error or non-zero number of number-byte taken for the conversion. // Stops at any non-digit '0'-'9' char or bytes is more then 5. uint8_t atoi_short(uint8_t idx) From 57061161b6898fde94e45c6af21fa80c5b758c74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20van=20Dorst?= Date: Tue, 11 Aug 2026 21:32:04 +0200 Subject: [PATCH 13/36] Refactor code to make use of new atoi_byte() --- cmd_parser.c | 89 ++++++++++++++++++++++++++++++++++------------------ 1 file changed, 58 insertions(+), 31 deletions(-) diff --git a/cmd_parser.c b/cmd_parser.c index c09d752..269a6d8 100644 --- a/cmd_parser.c +++ b/cmd_parser.c @@ -235,22 +235,32 @@ uint8_t atoi_short(uint8_t idx) } -uint8_t parse_ip(uint8_t idx) +int8_t parse_ip(uint8_t idx) { - __xdata uint8_t b; + uint8_t b = 0; + uint8_t ret; - for (b = 0; b < 4; b++) { - ip[b] = 0; - while (isnumber(cmd_buffer[idx])) { - ip[b] = (ip[b] * 10) + cmd_buffer[idx] - '0'; - idx++; + while(1) { + ret = atoi_byte(idx); + if (ret == 0) { + goto err; } - if (b < 3 && cmd_buffer[idx++] != '.') { - print_string("Error in IP format, expecting '.'\n"); - return -1; + idx += ret; + ip[b++] = atoi_results_u8; + + if (b == 4) { + break; + } + + if (cmd_buffer[idx++] != '.') { + goto err; } } return 0; + +err: + print_string("Error in IP format\n"); + return -1; } @@ -258,6 +268,7 @@ void parse_lag(void) { __xdata uint8_t group; __xdata uint16_t members = 0; + uint8_t ret; if (cmd_compare(1, "show")) { print_string("LAG status:\n"); @@ -284,30 +295,35 @@ void parse_lag(void) return; } - if (cmd_words_len < 2 || !isnumber(cmd_buffer[cmd_words_b[1]])) + if (cmd_words_len < 2) goto err; - group = cmd_buffer[cmd_words_b[1]] - '1'; + + // Parse group, expect only one number 0-9. + ret = atoi_byte(cmd_words_b[1]); + if (ret != 1) { + goto err; + } + group = atoi_results_u8 - 1; + if (group > 3) /* '0' wraps well past three, so one test does both ends */ goto err; + uint8_t w = 2; while (w < cmd_words_len) { // write_char('|'); print_byte(w); write_char(':'); write_char(cmd_buffer[cmd_words_b[w]]); write_char('-'); - uint8_t port; - if (isnumber(cmd_buffer[cmd_words_b[w]])) { - port = cmd_buffer[cmd_words_b[w]] - '1'; - if (isnumber(cmd_buffer[cmd_words_b[w] + 1])) - port = (port + 1) * 10 + cmd_buffer[cmd_words_b[w] + 1] - '1'; - if (port > 8) /* phys_to_log_port holds nine entries */ - goto err; - port = machine.phys_to_log_port[port]; - } else { + + // Parse port. + ret = atoi_byte(cmd_words_b[w++]); + if (ret != 1) goto err; - } + + uint8_t port = atoi_results_u8; + port = machine.phys_to_log_port[port]; + if (port > machine.max_port) goto err; members |= ((uint16_t)1) << port; - w++; } port_lag_members_set(group, members); return; @@ -828,11 +844,14 @@ void parse_sfp(void) } return; } - if (cmd_buffer[cmd_words_b[1]] < '1' || cmd_buffer[cmd_words_b[1]] > '2' || cmd_buffer[cmd_words_b[1] + 1] != ' ' ) { + uint8_t idx = cmd_words_b[1]; + uint8_t ret = atoi_byte(idx); + idx += ret; + slot = atoi_results_u8 - 1; + if (ret == 0 || cmd_buffer[idx] != ' ' || slot > 1) { print_string("Illegal SFP slot number\n"); return; } - slot = cmd_buffer[cmd_words_b[1]] - '1'; if (slot >= machine.n_sfp) { print_string("SFP slot not present\n"); return; @@ -955,9 +974,10 @@ void parse_sdsget(void) goto err; } - if (atoi_byte(&sds_id, cmd_words_b[1])) { + if (!atoi_byte(cmd_words_b[1])) { goto err; } + sds_id = atoi_results_u8; hex_size = atoi_hex(cmd_words_b[2]); if (hex_size != 1) { @@ -999,9 +1019,10 @@ void parse_sdsset(void) goto err; } - if (atoi_byte(&sds_id, cmd_words_b[1])) { + if (!atoi_byte(cmd_words_b[1])) { goto err; } + sds_id = atoi_results_u8; hex_size = atoi_hex(cmd_words_b[2]); if (hex_size != 1) { @@ -1054,13 +1075,16 @@ void parse_phyget(void) goto err; } - if (atoi_byte(&phy_id, cmd_words_b[1])) { + if (!atoi_byte(cmd_words_b[1])) { goto err; } + phy_id = atoi_results_u8; - if (atoi_byte(&dev_id, cmd_words_b[2])) { + + if (!atoi_byte(cmd_words_b[2])) { goto err; } + dev_id = atoi_results_u8; hex_size = atoi_hex(cmd_words_b[3]); if (hex_size == 0 || hex_size > 2) { @@ -1100,13 +1124,16 @@ void parse_physet(void) goto err; } - if (atoi_byte(&phy_id, cmd_words_b[1])) { + if (!atoi_byte(cmd_words_b[1])) { goto err; } + phy_id = atoi_results_u8; - if (atoi_byte(&dev_id, cmd_words_b[2])) { + + if (!atoi_byte(cmd_words_b[2])) { goto err; } + dev_id = atoi_results_u8; hex_size = atoi_hex(cmd_words_b[3]); if (hex_size == 0 || hex_size > 2) { From 175f3bb1856ecf3456124d5ea92f430262c80e87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20van=20Dorst?= Date: Tue, 11 Aug 2026 21:44:10 +0200 Subject: [PATCH 14/36] Add cmd_parse_port() and cmd_parse_port_space() helper. A lot of places we manual parse and translate the port. These helper functions will do that for us. --- cmd_parser.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/cmd_parser.c b/cmd_parser.c index 269a6d8..9d627e7 100644 --- a/cmd_parser.c +++ b/cmd_parser.c @@ -234,6 +234,55 @@ uint8_t atoi_short(uint8_t idx) return cnt; } +/* Parse, validate and translate phys_to_log_port port argument + * cpu_is_valid tells the parser that port 10 is valid input. + * returns 0 when on parser error or invalid value. + * return non-zero number of bytes consumed. + * Store the value in atoi_results_u8. + */ +uint8_t cmd_parse_port(uint8_t idx, __bit cpu_is_valid) { + uint8_t ret = atoi_byte(idx); + uint8_t port = 0; + if (ret != 0) { + port = atoi_results_u8 - 1; + + if (cpu_is_valid && port == 9) { + // CPU port is valid + } else if (port < 9) { + port = machine.phys_to_log_port[port]; + if (port < machine.min_port || port > machine.max_port) { + ret = 0; + } + } else { + ret = 0; + } + } + + atoi_results_u8 = port; + return ret; +} + + +// Same as cmd_parse_port() addition to check the trailing space. +// returns 0 when on parser error or invalid value or no space. +// return non-zero number of bytes consumed including the space. +uint8_t cmd_parse_port_space(uint8_t idx, __bit cpu_is_valid) { + uint8_t ret = cmd_parse_port(idx, cpu_is_valid); + if (ret != 0) { + idx += ret; + ret++; + if (cmd_buffer[idx] != ' ') + ret = 0; + } + return ret; +} + + +// check if the cmd_buffer[idx] is a space. +__bit cmd_is_space(uint8_t idx) { + return cmd_buffer[idx] == ' '; +} + int8_t parse_ip(uint8_t idx) { From 73df006d392ed8e489332652a0da8d48f4b29ad3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20van=20Dorst?= Date: Tue, 11 Aug 2026 22:55:07 +0200 Subject: [PATCH 15/36] Replace all manual port parsing with cmd_parse_port_space() or cmd_parse_port(). Saves no SRAM but around 1k code size --- cmd_parser.c | 247 ++++++++++++++++++++++----------------------------- 1 file changed, 107 insertions(+), 140 deletions(-) diff --git a/cmd_parser.c b/cmd_parser.c index 9d627e7..296741b 100644 --- a/cmd_parser.c +++ b/cmd_parser.c @@ -349,30 +349,22 @@ void parse_lag(void) // Parse group, expect only one number 0-9. ret = atoi_byte(cmd_words_b[1]); - if (ret != 1) { - goto err; - } - group = atoi_results_u8 - 1; - - if (group > 3) /* '0' wraps well past three, so one test does both ends */ + if (ret != 1) goto err; + group = atoi_results_u8 - 1; + if (group > 3) /* '0' wraps well past three, so one test does both ends */ + goto err; uint8_t w = 2; while (w < cmd_words_len) { // write_char('|'); print_byte(w); write_char(':'); write_char(cmd_buffer[cmd_words_b[w]]); write_char('-'); // Parse port. - ret = atoi_byte(cmd_words_b[w++]); - if (ret != 1) + if (cmd_parse_port(cmd_words_b[w++], false) == 0) goto err; - uint8_t port = atoi_results_u8; - port = machine.phys_to_log_port[port]; - - if (port > machine.max_port) - goto err; - members |= ((uint16_t)1) << port; + members |= ((uint16_t)1) << atoi_results_u8; } port_lag_members_set(group, members); return; @@ -431,17 +423,13 @@ void parse_vlan(void) if (cmd_words_len < 2) goto err; - uint8_t ret = atoi_short(cmd_words_b[1]); - if (ret) { - if (cmd_words_len == 3 && cmd_buffer[cmd_words_b[2]] == 'd') { - if (atoi_results_short > 4094) - goto err; - vlan_delete(atoi_results_short); - return; - } + // Parse the VLAN number + if (atoi_short(cmd_words_b[1]) != 0) { + if (atoi_results_short > 4094) + goto err; + vlan_settings.vlan = atoi_results_short; + if (cmd_compare(2, "mgmt")) { - if (vlan_settings.vlan > 4094) - goto err; management_vlan = vlan_settings.vlan; if (!vlan_settings.vlan) print_string("Management VLAN disabled\n"); @@ -449,8 +437,16 @@ void parse_vlan(void) print_string("Management VLAN set to "); print_short(management_vlan); write_char('\n'); return; } - if (!vlan_settings.vlan || vlan_settings.vlan > 4094) + + // Other commands vlan 0 is invalid + if (!vlan_settings.vlan) goto err; + + if (cmd_words_len == 3 && cmd_buffer[cmd_words_b[2]] == 'd') { + vlan_delete(vlan_settings.vlan); + return; + } + uint8_t w = 2; if (cmd_words_len > w && isletter(cmd_buffer[cmd_words_b[w]])) { uint8_t i = 0; @@ -466,24 +462,21 @@ void parse_vlan(void) w++; print_string("<\n"); } + + uint8_t ret; + uint8_t idx; while (cmd_words_len > w) { - __xdata uint8_t port; - if (isnumber(cmd_buffer[cmd_words_b[w]])) { - port = cmd_buffer[cmd_words_b[w]] - '1'; - if (isnumber(cmd_buffer[cmd_words_b[w] + 1])) { - port = (port + 1) * 10 + cmd_buffer[cmd_words_b[w] + 1] - '1'; - if (cmd_buffer[cmd_words_b[w] + 2] == 't') - vlan_settings.tagged |= ((uint16_t)1) << port; - } else { - port = machine.phys_to_log_port[port]; - if (cmd_buffer[cmd_words_b[w] + 1] == 't') - vlan_settings.tagged |= ((uint16_t)1) << port; - } - if (port > machine.max_port) - goto err; - vlan_settings.members |= ((uint16_t)1) << port; - } - w++; + idx = cmd_words_b[w++]; + ret = cmd_parse_port(idx, false); + if (ret == 0) + goto err; + + idx += ret; + uint16_t pmask = ((uint16_t)1) << atoi_results_u8; + vlan_settings.members |= pmask; + + if (cmd_buffer[idx] == 't') + vlan_settings.tagged |= pmask; } vlan_create(); } else if (cmd_compare(1, "show")) { @@ -511,12 +504,9 @@ void parse_isolate(void) print_string("\nISOLATE "); - if (!isnumber(cmd_buffer[cmd_words_b[1]]) || cmd_buffer[cmd_words_b[1]] == '0' - || isnumber(cmd_buffer[cmd_words_b[1] + 1])) - goto err; - __xdata uint8_t port_configured = machine.phys_to_log_port[cmd_buffer[cmd_words_b[1]] - '1']; - if (port_configured < machine.min_port || port_configured > machine.max_port) + if (cmd_parse_port_space(cmd_words_b[1], false) == 0) goto err; + uint8_t port_configured = atoi_results_u8; print_byte(port_configured); write_char('\n'); @@ -545,22 +535,12 @@ void parse_isolate(void) uint8_t w = 2; while (w < cmd_words_len) { - __xdata uint8_t port; - if (isnumber(cmd_buffer[cmd_words_b[w]])) { - port = cmd_buffer[cmd_words_b[w]] - '1'; - if (isnumber(cmd_buffer[cmd_words_b[w] + 1])) { - port = (port + 1) * 10 + cmd_buffer[cmd_words_b[w] + 1] - '1'; // logical port - if (port != 9) // CPU port is logical port 9 - goto err; - } else { - port = machine.phys_to_log_port[port]; - if (port < machine.min_port || port > machine.max_port) - goto err; - } - members |= ((uint16_t)1) << port; - } - w++; + if (cmd_parse_port(cmd_words_b[w++], false) == 0) + goto err; + uint8_t port = atoi_results_u8; + members |= ((uint16_t)1) << port; } + port_isolate(port_configured, members); return; @@ -605,19 +585,19 @@ void parse_ingress(void) print_string("All ports ingress filter set to: "); print_port_ingress_filter_mode(mode); write_char('\n'); } - return; } else { for(uint8_t w = 1; w < cmd_words_len; w++) { - uint8_t p = cmd_buffer[cmd_words_b[w]]; - if (!isnumber(p)) { + uint8_t idx = cmd_words_b[w]; + char p = cmd_buffer[idx]; + uint8_t ret = cmd_parse_port_space(idx, false); + if (ret == 0) { + print_string("Invalid physical port number\n"); continue; } - if (p < '1') { - print_string("Invalid physical port number: "); write_char(p); write_char('\n'); - continue; - } - log_port = machine.phys_to_log_port[p - '1']; - if (!vlan_ingress_mode_parse(cmd_buffer[cmd_words_b[w] + 1], &mode)) { + log_port = atoi_results_u8; + idx += ret; + + if (!vlan_ingress_mode_parse(cmd_buffer[idx], &mode)) { print_string("Invalid ingress mode for port "); write_char(p); print_string(" in ingress command\n"); goto err; } @@ -629,15 +609,15 @@ void parse_ingress(void) print_string(" ingress filter set to: "); print_port_ingress_filter_mode(mode); write_char('\n'); } - return; } + return; err: print_string("Error: ingress [p]... \n"); } void parse_mirror(void) { - __xdata uint8_t mirroring_port; + __xdata uint8_t mirroring_port = 0; __xdata uint16_t rx_pmask = 0; __xdata uint16_t tx_pmask = 0; @@ -667,48 +647,38 @@ void parse_mirror(void) return; } - if (cmd_words_len < 2 || !isnumber(cmd_buffer[cmd_words_b[1]])) { - print_string("Port/command missing: mirror [status/off/ [port][t/r]]...\n"); - return; - } - - mirroring_port = cmd_buffer[cmd_words_b[1]] - '1'; - if (isnumber(cmd_buffer[cmd_words_b[1] + 1])) - mirroring_port = (mirroring_port + 1) * 10 + cmd_buffer[cmd_words_b[1] + 1] - '1'; - mirroring_port = machine.phys_to_log_port[mirroring_port]; - + if (cmd_words_len < 2) + goto err; uint8_t w = 2; + uint8_t port; + uint8_t ret; while (w < cmd_words_len) { - uint8_t port; - if (isnumber(cmd_buffer[cmd_words_b[w]])) { - port = cmd_buffer[cmd_words_b[w]] - '1'; - if (isnumber(cmd_buffer[cmd_words_b[w] + 1])) { - port = (port + 1) * 10 + cmd_buffer[cmd_words_b[w] + 1] - '1'; - port = machine.phys_to_log_port[port]; - if (cmd_buffer[cmd_words_b[w] + 2] == 'r') - rx_pmask |= ((uint16_t)1) << port; - else if (cmd_buffer[cmd_words_b[w] + 2] == 't') - tx_pmask |= ((uint16_t)1) << port; - else { - rx_pmask |= ((uint16_t)1) << port; - tx_pmask |= ((uint16_t)1) << port; - } - } else { - port = machine.phys_to_log_port[port]; - if (cmd_buffer[cmd_words_b[w] + 1] == 'r') - rx_pmask |= ((uint16_t)1) << port; - else if (cmd_buffer[cmd_words_b[w] + 1] == 't') - tx_pmask |= ((uint16_t)1) << port; - else { - rx_pmask |= ((uint16_t)1) << port; - tx_pmask |= ((uint16_t)1) << port; - } - } - } - w++; + uint8_t idx = cmd_words_b[w++]; + ret = cmd_parse_port(idx, false); + if (ret == 0) + goto err; + + idx += ret; + port = atoi_results_u8; + + // Use the first port argument as mirroring_port + if (w == 2) + mirroring_port = port; + + ret = cmd_buffer[idx]; + uint16_t pmask = ((uint16_t)1) << port; + if (ret != 't') + rx_pmask |= pmask; + if (ret != 'r') + tx_pmask |= pmask; } port_mirror_set(mirroring_port, rx_pmask, tx_pmask); + return; + +err: + print_string("Port/command missing: mirror [status/off/ [port][t/r]]...\n"); + return; } @@ -722,16 +692,11 @@ void parse_port(void) return; } - if (cmd_buffer[cmd_words_b[1]] < '1' || cmd_buffer[cmd_words_b[1]] > '9' || cmd_buffer[cmd_words_b[1] + 1] != ' ' ) { - print_string("Illegal port number\n"); - return; - } - phy_settings.port = cmd_buffer[cmd_words_b[1]] - '1'; - phy_settings.port = machine.phys_to_log_port[phy_settings.port]; - if (phy_settings.port > machine.max_port || phy_settings.port < machine.min_port) { - print_string("This machine has no port with the specified number\n"); + if (cmd_parse_port_space(cmd_words_b[1], false) == 0) { + print_string("Invalid port number\n"); return; } + phy_settings.port = atoi_results_u8; print_string("Logical Port: "); print_byte(phy_settings.port); write_char('\n'); phy_settings.duplex = PHY_DUPLEX_BOTH; @@ -813,25 +778,24 @@ void parse_port(void) void parse_mtu(void) { - __xdata uint16_t mtu; uint8_t p, ret; if (cmd_compare(1, "show")) { for (p = machine.min_port; p <= machine.max_port; p++) { reg_read_m(RTL8373_REG_MAC_L2_PORT_MAX_LEN + ((uint16_t) p << 8)); - mtu = SFR_DATA_U16 & 0x3fff; + uint16_t mtu = SFR_DATA_U16 & 0x3fff; print_string("Port "); print_byte(machine.log_to_phys_port[p]); write_char(' '); print_short(mtu); write_char('\n'); } return; } - if (cmd_words_len != 3 || cmd_buffer[cmd_words_b[1]] < '1' - || cmd_buffer[cmd_words_b[1]] > '9' - || cmd_buffer[cmd_words_b[1] + 1] > ' ') { - print_string("mtu [port] [size]\n"); - return; - } - p = machine.phys_to_log_port[cmd_buffer[cmd_words_b[1]] - '1']; + if (cmd_words_len != 3) + goto err; + + if (cmd_parse_port_space(cmd_words_b[1], false) == 0) + goto err; + + p = atoi_results_u8; print_byte(p); ret = atoi_short(cmd_words_b[2]); @@ -843,6 +807,11 @@ void parse_mtu(void) REG_WRITE(RTL8373_REG_MAC_L2_PORT_MAX_LEN + ((uint16_t) p << 8), (atoi_results_short >> 10) & 0xf, (atoi_results_short >> 2) & 0xff, ((atoi_results_short & 0x3) << 6) | ((atoi_results_short >> 8) & 0x3f), atoi_results_short & 0xff); write_char('\n'); + return; + +err: + print_string("mtu [port] [size]\n"); + return; } bool sfp_print_measurements(uint8_t sfp) @@ -897,7 +866,7 @@ void parse_sfp(void) uint8_t ret = atoi_byte(idx); idx += ret; slot = atoi_results_u8 - 1; - if (ret == 0 || cmd_buffer[idx] != ' ' || slot > 1) { + if (ret == 0 || !cmd_is_space(idx) || slot > 1) { print_string("Illegal SFP slot number\n"); return; } @@ -1277,8 +1246,11 @@ void parse_eee(void) speed_word = 2; } else if (cmd_buffer[idx] == ' ' || cmd_buffer[idx] == '\0') { // Word 2 is a port number - port = cmd_buffer[cmd_words_b[2]] - '1'; - port = machine.phys_to_log_port[port]; + if (cmd_parse_port_space(idx, false) == 0) { + print_string("Speed word invalid, use: [100m|1g|2g5]\n"); + return; + } + port = atoi_results_u8; // Check if word 3 is a speed if (cmd_words_len >= 4) speed_word = 3; @@ -1321,17 +1293,15 @@ void parse_eee(void) void parse_bw(void) { - __xdata uint8_t port; + uint8_t port; __xdata uint32_t bw = 0; if (cmd_words_len < 2) // Check for at least 2 arguments goto err; - port = cmd_buffer[cmd_words_b[2]] - '1'; - if (port > 9) + if (cmd_parse_port_space(cmd_words_b[2], false) == 0) goto err; - - port = machine.phys_to_log_port[port]; + port = atoi_results_u8; if (cmd_compare(1, "status")) { bandwidth_status(port); @@ -1688,12 +1658,9 @@ void cmd_parser(void) __banked stpEnabled = 0; } } else if (cmd_compare(0, "pvid") && cmd_words_len == 3) { - uint8_t ret; - if (cmd_buffer[cmd_words_b[1]] >= '1' - && cmd_buffer[cmd_words_b[1]] <= '9' - && cmd_buffer[cmd_words_b[1] + 1] <= ' ' + if (cmd_parse_port_space(cmd_words_b[1], false) != 0 && atoi_results_u8 && atoi_short(cmd_words_b[2]) && atoi_results_short && atoi_results_short <= 4094) - port_pvid_set(machine.phys_to_log_port[cmd_buffer[cmd_words_b[1]] - '1'], atoi_results_short); + port_pvid_set(atoi_results_u8, atoi_results_short); else print_string("Error: pvid <1-4094>\n"); } else if (cmd_compare(0, "vlan")) { From 3eb2dfe0c9151dba53b5b9b886f93c5b46318c52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20van=20Dorst?= Date: Tue, 11 Aug 2026 23:44:32 +0200 Subject: [PATCH 16/36] Change parse_ip() --- cmd_parser.c | 70 ++++++++++++++++++++++++++++++---------------------- 1 file changed, 41 insertions(+), 29 deletions(-) diff --git a/cmd_parser.c b/cmd_parser.c index 296741b..98ebc61 100644 --- a/cmd_parser.c +++ b/cmd_parser.c @@ -283,33 +283,43 @@ __bit cmd_is_space(uint8_t idx) { return cmd_buffer[idx] == ' '; } - -int8_t parse_ip(uint8_t idx) +// returns 0 when on parser error or invalid value or no space. +// return non-zero number of bytes consumed including the space. +// Stops at a space or NULL. +uint8_t parse_ip(uint8_t idx) { uint8_t b = 0; uint8_t ret; + uint8_t idx_start = idx; while(1) { ret = atoi_byte(idx); - if (ret == 0) { + if (ret == 0) goto err; - } + idx += ret; ip[b++] = atoi_results_u8; + ret = cmd_buffer[idx]; if (b == 4) { - break; - } - - if (cmd_buffer[idx++] != '.') { + if (ret == ' ') { + idx++; + break; + } + if (ret == '\0') + break; goto err; } + idx++; + + if (ret != '.') + goto err; } - return 0; + return idx - idx_start; err: print_string("Error in IP format\n"); - return -1; + return 0; } @@ -1388,7 +1398,7 @@ void parse_syslog(void) itoa(syslog_state.server_ip[0]); write_char('.'); itoa(syslog_state.server_ip[1]); write_char('.'); itoa(syslog_state.server_ip[2]); write_char('.'); itoa(syslog_state.server_ip[3]); return; - } else if (!parse_ip(cmd_words_b[2])) { + } else if (parse_ip(cmd_words_b[2]) != 0) { uint8_t was_enabled = syslog_state.enabled; if (was_enabled) syslog_stop(); @@ -1568,46 +1578,48 @@ void cmd_parser(void) __banked } else { if (dhcp_state.state) dhcp_stop(); - if (!parse_ip(cmd_words_b[1])) { + if (parse_ip(cmd_words_b[1]) != 0) { uip_ipaddr(&uip_hostaddr, ip[0], ip[1], ip[2], ip[3]); print_string("Setting ip: "); itoa(ip[0]); write_char('.'); itoa(ip[1]); write_char('.'); itoa(ip[2]); write_char('.'); itoa(ip[3]); write_char('\n'); } else { - print_string("Invalid IP address\n"); - print_string("Error: ip [|dhcp]\n"); - print_string(" The dhcp option enables the dhcp client, calling ip without options prints the current IP\n"); - print_string(" Calling with a valid IP address will stop any ongoing dhcp client and set the IP address\n"); + print_string("Invalid IP address\n" \ + "Error: ip [|dhcp]\n" \ + " The dhcp option enables the dhcp client, calling ip without options prints the current IP\n" \ + " Calling with a valid IP address will stop any ongoing dhcp client and set the IP address\n"); } } } else if (cmd_compare(0, "gw")) { if (cmd_words_len == 1) { print_string("Current gw: "); itoa(uip_draddr[0]); write_char('.'); itoa(uip_draddr[0] >> 8); write_char('.'); - itoa(uip_draddr[1]); write_char('.'); itoa(uip_draddr[1] >> 8); + itoa(uip_draddr[1]); write_char('.'); itoa(uip_draddr[1] >> 8); write_char('\n'); } else { - if (!parse_ip(cmd_words_b[1])) + if (parse_ip(cmd_words_b[1]) != 0) { uip_ipaddr(&uip_draddr, ip[0], ip[1], ip[2], ip[3]); - else - print_string("Invalid IP address\n"); - print_string("Setting gw: "); - itoa(ip[0]); write_char('.'); itoa(ip[1]); write_char('.'); - itoa(ip[2]); write_char('.'); itoa(ip[3]); + print_string("Setting gw: "); + itoa(ip[0]); write_char('.'); itoa(ip[1]); write_char('.'); + itoa(ip[2]); write_char('.'); itoa(ip[3]); write_char('\n'); + } else { + print_string("Invalid IP address\n" \ + "Error: gw \n"); + } } - write_char('\n'); } else if (cmd_compare(0, "netmask")) { if (cmd_words_len == 1) { print_string("Current netmask: "); itoa(uip_netmask[0]); write_char('.'); itoa(uip_netmask[0] >> 8); write_char('.'); itoa(uip_netmask[1]); write_char('.'); itoa(uip_netmask[1] >> 8); } else { - if (!parse_ip(cmd_words_b[1])) + if (parse_ip(cmd_words_b[1]) != 0) { uip_ipaddr(&uip_netmask, ip[0], ip[1], ip[2], ip[3]); - else + print_string("Setting netmask: "); + itoa(ip[0]); write_char('.'); itoa(ip[1]); write_char('.'); + itoa(ip[2]); write_char('.'); itoa(ip[3]); + } else { print_string("Invalid IP address\n"); - print_string("Setting netmask: "); - itoa(ip[0]); write_char('.'); itoa(ip[1]); write_char('.'); - itoa(ip[2]); write_char('.'); itoa(ip[3]); + } } write_char('\n'); } else if (cmd_compare(0, "l2")) { From 724fd1e061b32edb7522cdb13f04840435d830d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20van=20Dorst?= Date: Wed, 12 Aug 2026 01:08:47 +0200 Subject: [PATCH 17/36] Added print_ip() to print IPv4 addresses. --- cmd_parser.c | 40 ++++++++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/cmd_parser.c b/cmd_parser.c index 98ebc61..ef0bf04 100644 --- a/cmd_parser.c +++ b/cmd_parser.c @@ -322,6 +322,22 @@ err: return 0; } +// Print a IPv4 adress. +void print_ip(__xdata uint8_t * ptr) +{ + uint8_t idx = 0; + uint8_t num; + + while(1) { + num = *ptr++; + itoa(num); + if (++idx == 4) + break; + + write_char('.'); + } +} + void parse_lag(void) { @@ -1379,8 +1395,7 @@ void parse_syslog(void) print_string("Current syslog status: "); if (syslog_state.enabled) { print_string("enabled, sending to "); - itoa(syslog_state.server_ip[0]); write_char('.'); itoa(syslog_state.server_ip[1]); write_char('.'); - itoa(syslog_state.server_ip[2]); write_char('.'); itoa(syslog_state.server_ip[3]); + print_ip(syslog_state.server_ip); write_char('\n'); } else { print_string("disabled\n"); @@ -1395,8 +1410,7 @@ void parse_syslog(void) } else if (cmd_compare(1, "ip")) { if (cmd_words_len < 3) { // no additional arguemnt -> print current ip print_string("Current syslog IP: "); - itoa(syslog_state.server_ip[0]); write_char('.'); itoa(syslog_state.server_ip[1]); write_char('.'); - itoa(syslog_state.server_ip[2]); write_char('.'); itoa(syslog_state.server_ip[3]); + print_ip(syslog_state.server_ip); return; } else if (parse_ip(cmd_words_b[2]) != 0) { uint8_t was_enabled = syslog_state.enabled; @@ -1565,8 +1579,7 @@ void cmd_parser(void) __banked dhcp_start(); } else if (cmd_words_len == 1) { print_string("Current IP: "); - itoa(uip_hostaddr[0]); write_char('.'); itoa(uip_hostaddr[0] >> 8); write_char('.'); - itoa(uip_hostaddr[1]); write_char('.'); itoa(uip_hostaddr[1] >> 8); + print_ip(uip_hostaddr); if (dhcp_state.state == DHCP_LEASING) { print_string(" (dhcp, renewal in sec: "); print_short(dhcp_state.dhcp_timer); @@ -1581,8 +1594,7 @@ void cmd_parser(void) __banked if (parse_ip(cmd_words_b[1]) != 0) { uip_ipaddr(&uip_hostaddr, ip[0], ip[1], ip[2], ip[3]); print_string("Setting ip: "); - itoa(ip[0]); write_char('.'); itoa(ip[1]); write_char('.'); - itoa(ip[2]); write_char('.'); itoa(ip[3]); write_char('\n'); + print_ip(ip); write_char('\n'); } else { print_string("Invalid IP address\n" \ "Error: ip [|dhcp]\n" \ @@ -1593,14 +1605,12 @@ void cmd_parser(void) __banked } else if (cmd_compare(0, "gw")) { if (cmd_words_len == 1) { print_string("Current gw: "); - itoa(uip_draddr[0]); write_char('.'); itoa(uip_draddr[0] >> 8); write_char('.'); - itoa(uip_draddr[1]); write_char('.'); itoa(uip_draddr[1] >> 8); write_char('\n'); + print_ip(uip_draddr); write_char('\n'); } else { if (parse_ip(cmd_words_b[1]) != 0) { uip_ipaddr(&uip_draddr, ip[0], ip[1], ip[2], ip[3]); print_string("Setting gw: "); - itoa(ip[0]); write_char('.'); itoa(ip[1]); write_char('.'); - itoa(ip[2]); write_char('.'); itoa(ip[3]); write_char('\n'); + print_ip(ip); write_char('\n'); } else { print_string("Invalid IP address\n" \ "Error: gw \n"); @@ -1609,14 +1619,12 @@ void cmd_parser(void) __banked } else if (cmd_compare(0, "netmask")) { if (cmd_words_len == 1) { print_string("Current netmask: "); - itoa(uip_netmask[0]); write_char('.'); itoa(uip_netmask[0] >> 8); write_char('.'); - itoa(uip_netmask[1]); write_char('.'); itoa(uip_netmask[1] >> 8); + print_ip(uip_netmask); write_char('\n'); } else { if (parse_ip(cmd_words_b[1]) != 0) { uip_ipaddr(&uip_netmask, ip[0], ip[1], ip[2], ip[3]); print_string("Setting netmask: "); - itoa(ip[0]); write_char('.'); itoa(ip[1]); write_char('.'); - itoa(ip[2]); write_char('.'); itoa(ip[3]); + print_ip(ip); write_char('\n'); } else { print_string("Invalid IP address\n"); } From 17642699d4a3e666bda72e91b71b9a295dd4d4c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20van=20Dorst?= Date: Fri, 14 Aug 2026 19:56:23 +0200 Subject: [PATCH 18/36] Improve parse_ingress() --- cmd_parser.c | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/cmd_parser.c b/cmd_parser.c index ef0bf04..2a78936 100644 --- a/cmd_parser.c +++ b/cmd_parser.c @@ -283,6 +283,12 @@ __bit cmd_is_space(uint8_t idx) { return cmd_buffer[idx] == ' '; } +// check if the cmd_buffer[idx] is a space or null. +__bit cmd_is_space_or_null(uint8_t idx) { + uint8_t c = cmd_buffer[idx]; + return c == ' ' || c == '\0'; +} + // returns 0 when on parser error or invalid value or no space. // return non-zero number of bytes consumed including the space. // Stops at a space or NULL. @@ -598,10 +604,13 @@ void parse_ingress(void) if (cmd_words_len < 2) { goto err; } - __xdata uint8_t log_port = 0; + uint8_t log_port = 0; __xdata vlan_ingress_mode_t mode = VLAN_INVALID; + uint8_t idx = cmd_words_b[1]; - if (vlan_ingress_mode_parse(cmd_buffer[cmd_words_b[1]], &mode)) { + if (vlan_ingress_mode_parse(cmd_buffer[idx++], &mode)) { + if (!cmd_is_space_or_null(idx)) + goto err; // Setting mode for all ports at once for (log_port = machine.min_port; log_port <= machine.max_port; log_port++) { if (!port_ingress_filter(log_port, mode)) { @@ -613,17 +622,17 @@ void parse_ingress(void) } } else { for(uint8_t w = 1; w < cmd_words_len; w++) { - uint8_t idx = cmd_words_b[w]; + idx = cmd_words_b[w]; char p = cmd_buffer[idx]; - uint8_t ret = cmd_parse_port_space(idx, false); - if (ret == 0) { + uint8_t ret = cmd_parse_port(idx, false); + if (ret != 1) { print_string("Invalid physical port number\n"); continue; } log_port = atoi_results_u8; idx += ret; - if (!vlan_ingress_mode_parse(cmd_buffer[idx], &mode)) { + if (!vlan_ingress_mode_parse(cmd_buffer[idx++], &mode) || !cmd_is_space_or_null(idx)) { print_string("Invalid ingress mode for port "); write_char(p); print_string(" in ingress command\n"); goto err; } @@ -638,7 +647,7 @@ void parse_ingress(void) } return; err: - print_string("Error: ingress [p]... \n"); + print_string("Error: ingress [p]...\n"); } void parse_mirror(void) From 42f7b4fba1bc396607ba5159f8233c9e65e95d23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20van=20Dorst?= Date: Fri, 14 Aug 2026 22:06:19 +0200 Subject: [PATCH 19/36] Fix parse_isolate(), ensure spaces between arguments --- cmd_parser.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/cmd_parser.c b/cmd_parser.c index 2a78936..59fca82 100644 --- a/cmd_parser.c +++ b/cmd_parser.c @@ -529,7 +529,7 @@ err: void parse_isolate(void) { - __xdata uint16_t members = 0; + uint16_t members = 0; if (cmd_words_len < 3) goto err; @@ -555,7 +555,7 @@ void parse_isolate(void) members >>= 1; } return; - } + } if (cmd_compare(2, "off")) { for (uint8_t i = machine.min_port; i < machine.max_port; i++) @@ -567,7 +567,10 @@ void parse_isolate(void) uint8_t w = 2; while (w < cmd_words_len) { - if (cmd_parse_port(cmd_words_b[w++], false) == 0) + uint8_t idx = cmd_words_b[w++]; + uint8_t ret = cmd_parse_port(idx, false); + idx += ret; + if (ret == 0 || !cmd_is_space_or_null(idx)) goto err; uint8_t port = atoi_results_u8; members |= ((uint16_t)1) << port; From 149dc8537ffc2dbd50ab7cec1e24fa8136e9baf3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20van=20Dorst?= Date: Mon, 17 Aug 2026 22:37:41 +0200 Subject: [PATCH 20/36] Rename cmd_parse_port_space() to cmd_parse_port_separator() cmd_parse_port_separator() parse the full port number and checks that the number end with a NUL of space. Now this function can also be used to parse the last argument number because this ends with a NUL. Also refactor code. --- cmd_parser.c | 46 +++++++++++++++++++++++++++------------------- 1 file changed, 27 insertions(+), 19 deletions(-) diff --git a/cmd_parser.c b/cmd_parser.c index 59fca82..0d85aa7 100644 --- a/cmd_parser.c +++ b/cmd_parser.c @@ -264,14 +264,15 @@ uint8_t cmd_parse_port(uint8_t idx, __bit cpu_is_valid) { // Same as cmd_parse_port() addition to check the trailing space. -// returns 0 when on parser error or invalid value or no space. +// returns 0 when on parser error or invalid value or no space or no NUL. // return non-zero number of bytes consumed including the space. -uint8_t cmd_parse_port_space(uint8_t idx, __bit cpu_is_valid) { +uint8_t cmd_parse_port_separator(uint8_t idx, __bit cpu_is_valid) { uint8_t ret = cmd_parse_port(idx, cpu_is_valid); if (ret != 0) { idx += ret; ret++; - if (cmd_buffer[idx] != ' ') + uint8_t c = cmd_buffer[idx]; + if (c != ' ' && c != '\0') ret = 0; } return ret; @@ -385,7 +386,7 @@ void parse_lag(void) goto err; group = atoi_results_u8 - 1; - if (group > 3) /* '0' wraps well past three, so one test does both ends */ + if (group > 3) goto err; uint8_t w = 2; @@ -393,7 +394,7 @@ void parse_lag(void) // write_char('|'); print_byte(w); write_char(':'); write_char(cmd_buffer[cmd_words_b[w]]); write_char('-'); // Parse port. - if (cmd_parse_port(cmd_words_b[w++], false) == 0) + if (cmd_parse_port_separator(cmd_words_b[w++], false) == 0) goto err; members |= ((uint16_t)1) << atoi_results_u8; @@ -407,13 +408,16 @@ err: void parse_lag_hash(void) { - __xdata uint8_t group; __xdata uint8_t hash = 0; - if (cmd_words_len < 2 || !isnumber(cmd_buffer[cmd_words_b[1]])) + if (cmd_words_len < 2) goto err; - group = cmd_buffer[cmd_words_b[1]] - '1'; - if (group > 3) /* '0' wraps well past three, so one test does both ends */ + + if (cmd_parse_port_separator(cmd_buffer[cmd_words_b[1]], false) == 0) + goto err; + + uint8_t group = atoi_results_u8 - 1; + if (group > 3) goto err; uint8_t w = 2; @@ -507,8 +511,13 @@ void parse_vlan(void) uint16_t pmask = ((uint16_t)1) << atoi_results_u8; vlan_settings.members |= pmask; - if (cmd_buffer[idx] == 't') + if (cmd_buffer[idx] == 't') { vlan_settings.tagged |= pmask; + idx++; + } + + if (!cmd_is_space_or_null(idx)) + goto err; } vlan_create(); } else if (cmd_compare(1, "show")) { @@ -536,7 +545,7 @@ void parse_isolate(void) print_string("\nISOLATE "); - if (cmd_parse_port_space(cmd_words_b[1], false) == 0) + if (cmd_parse_port_separator(cmd_words_b[1], false) == 0) goto err; uint8_t port_configured = atoi_results_u8; @@ -568,9 +577,8 @@ void parse_isolate(void) uint8_t w = 2; while (w < cmd_words_len) { uint8_t idx = cmd_words_b[w++]; - uint8_t ret = cmd_parse_port(idx, false); - idx += ret; - if (ret == 0 || !cmd_is_space_or_null(idx)) + uint8_t ret = cmd_parse_port_separator(idx, false); + if (ret == 0) goto err; uint8_t port = atoi_results_u8; members |= ((uint16_t)1) << port; @@ -730,7 +738,7 @@ void parse_port(void) return; } - if (cmd_parse_port_space(cmd_words_b[1], false) == 0) { + if (cmd_parse_port_separator(cmd_words_b[1], false) == 0) { print_string("Invalid port number\n"); return; } @@ -830,7 +838,7 @@ void parse_mtu(void) if (cmd_words_len != 3) goto err; - if (cmd_parse_port_space(cmd_words_b[1], false) == 0) + if (cmd_parse_port_separator(cmd_words_b[1], false) == 0) goto err; p = atoi_results_u8; @@ -1284,7 +1292,7 @@ void parse_eee(void) speed_word = 2; } else if (cmd_buffer[idx] == ' ' || cmd_buffer[idx] == '\0') { // Word 2 is a port number - if (cmd_parse_port_space(idx, false) == 0) { + if (cmd_parse_port_separator(idx, false) == 0) { print_string("Speed word invalid, use: [100m|1g|2g5]\n"); return; } @@ -1337,7 +1345,7 @@ void parse_bw(void) if (cmd_words_len < 2) // Check for at least 2 arguments goto err; - if (cmd_parse_port_space(cmd_words_b[2], false) == 0) + if (cmd_parse_port_separator(cmd_words_b[2], false) == 0) goto err; port = atoi_results_u8; @@ -1690,7 +1698,7 @@ void cmd_parser(void) __banked stpEnabled = 0; } } else if (cmd_compare(0, "pvid") && cmd_words_len == 3) { - if (cmd_parse_port_space(cmd_words_b[1], false) != 0 && atoi_results_u8 + if (cmd_parse_port_separator(cmd_words_b[1], false) != 0 && atoi_results_u8 && atoi_short(cmd_words_b[2]) && atoi_results_short && atoi_results_short <= 4094) port_pvid_set(atoi_results_u8, atoi_results_short); else From dca92d7f45c785591ee54b838c9804e5edc3990d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20van=20Dorst?= Date: Mon, 17 Aug 2026 22:53:27 +0200 Subject: [PATCH 21/36] cmd_parse_port_separator() cmd_parse_port() remove CPU-port support Removing the CPU-port support, As stated in #334, it is not needed to manual add the CPU-port to any command. Because the CPU-port should be added automatilly when CPU-port is needed. Refactor cmd_parse_port() because port number is only 1 byte. This simplifies the parsing code. Refactor cmd_parse_port_separator() to ensure the return size is correct. --- cmd_parser.c | 69 ++++++++++++++++++++++------------------------------ 1 file changed, 29 insertions(+), 40 deletions(-) diff --git a/cmd_parser.c b/cmd_parser.c index 0d85aa7..05f1f26 100644 --- a/cmd_parser.c +++ b/cmd_parser.c @@ -234,45 +234,37 @@ uint8_t atoi_short(uint8_t idx) return cnt; } -/* Parse, validate and translate phys_to_log_port port argument - * cpu_is_valid tells the parser that port 10 is valid input. +/* Parse, validate and translate phys_to_log_port physical port argument. + * So CPU-port 10 is not valid. * returns 0 when on parser error or invalid value. * return non-zero number of bytes consumed. * Store the value in atoi_results_u8. */ -uint8_t cmd_parse_port(uint8_t idx, __bit cpu_is_valid) { - uint8_t ret = atoi_byte(idx); - uint8_t port = 0; - if (ret != 0) { - port = atoi_results_u8 - 1; +uint8_t cmd_parse_port(uint8_t idx) { + uint8_t port = cmd_buffer[idx] - '0' - 1; + if (port > 8) + return 0; - if (cpu_is_valid && port == 9) { - // CPU port is valid - } else if (port < 9) { - port = machine.phys_to_log_port[port]; - if (port < machine.min_port || port > machine.max_port) { - ret = 0; - } - } else { - ret = 0; - } - } + port = machine.phys_to_log_port[port]; + if (port < machine.min_port || port > machine.max_port) + return 0; atoi_results_u8 = port; - return ret; + return 1; } -// Same as cmd_parse_port() addition to check the trailing space. +// Same as cmd_parse_port() addition to check the trailing space or NUL. // returns 0 when on parser error or invalid value or no space or no NUL. // return non-zero number of bytes consumed including the space. -uint8_t cmd_parse_port_separator(uint8_t idx, __bit cpu_is_valid) { - uint8_t ret = cmd_parse_port(idx, cpu_is_valid); +uint8_t cmd_parse_port_separator(uint8_t idx) { + uint8_t ret = cmd_parse_port(idx); if (ret != 0) { idx += ret; - ret++; uint8_t c = cmd_buffer[idx]; - if (c != ' ' && c != '\0') + if (c == ' ') { + ret++; + } else if (c != '\0') ret = 0; } return ret; @@ -394,7 +386,7 @@ void parse_lag(void) // write_char('|'); print_byte(w); write_char(':'); write_char(cmd_buffer[cmd_words_b[w]]); write_char('-'); // Parse port. - if (cmd_parse_port_separator(cmd_words_b[w++], false) == 0) + if (cmd_parse_port_separator(cmd_words_b[w++]) == 0) goto err; members |= ((uint16_t)1) << atoi_results_u8; @@ -413,7 +405,7 @@ void parse_lag_hash(void) if (cmd_words_len < 2) goto err; - if (cmd_parse_port_separator(cmd_buffer[cmd_words_b[1]], false) == 0) + if (cmd_parse_port_separator(cmd_buffer[cmd_words_b[1]]) == 0) goto err; uint8_t group = atoi_results_u8 - 1; @@ -503,7 +495,7 @@ void parse_vlan(void) uint8_t idx; while (cmd_words_len > w) { idx = cmd_words_b[w++]; - ret = cmd_parse_port(idx, false); + ret = cmd_parse_port(idx); if (ret == 0) goto err; @@ -545,7 +537,7 @@ void parse_isolate(void) print_string("\nISOLATE "); - if (cmd_parse_port_separator(cmd_words_b[1], false) == 0) + if (cmd_parse_port_separator(cmd_words_b[1]) == 0) goto err; uint8_t port_configured = atoi_results_u8; @@ -576,9 +568,7 @@ void parse_isolate(void) uint8_t w = 2; while (w < cmd_words_len) { - uint8_t idx = cmd_words_b[w++]; - uint8_t ret = cmd_parse_port_separator(idx, false); - if (ret == 0) + if (cmd_parse_port_separator(cmd_words_b[w++]) == 0) goto err; uint8_t port = atoi_results_u8; members |= ((uint16_t)1) << port; @@ -635,7 +625,7 @@ void parse_ingress(void) for(uint8_t w = 1; w < cmd_words_len; w++) { idx = cmd_words_b[w]; char p = cmd_buffer[idx]; - uint8_t ret = cmd_parse_port(idx, false); + uint8_t ret = cmd_parse_port(idx); if (ret != 1) { print_string("Invalid physical port number\n"); continue; @@ -701,7 +691,7 @@ void parse_mirror(void) uint8_t ret; while (w < cmd_words_len) { uint8_t idx = cmd_words_b[w++]; - ret = cmd_parse_port(idx, false); + ret = cmd_parse_port(idx); if (ret == 0) goto err; @@ -738,7 +728,7 @@ void parse_port(void) return; } - if (cmd_parse_port_separator(cmd_words_b[1], false) == 0) { + if (cmd_parse_port_separator(cmd_words_b[1]) == 0) { print_string("Invalid port number\n"); return; } @@ -838,7 +828,7 @@ void parse_mtu(void) if (cmd_words_len != 3) goto err; - if (cmd_parse_port_separator(cmd_words_b[1], false) == 0) + if (cmd_parse_port_separator(cmd_words_b[1]) == 0) goto err; p = atoi_results_u8; @@ -1292,7 +1282,7 @@ void parse_eee(void) speed_word = 2; } else if (cmd_buffer[idx] == ' ' || cmd_buffer[idx] == '\0') { // Word 2 is a port number - if (cmd_parse_port_separator(idx, false) == 0) { + if (cmd_parse_port_separator(idx) == 0) { print_string("Speed word invalid, use: [100m|1g|2g5]\n"); return; } @@ -1339,15 +1329,14 @@ void parse_eee(void) void parse_bw(void) { - uint8_t port; __xdata uint32_t bw = 0; if (cmd_words_len < 2) // Check for at least 2 arguments goto err; - if (cmd_parse_port_separator(cmd_words_b[2], false) == 0) + if (cmd_parse_port_separator(cmd_words_b[2]) == 0) goto err; - port = atoi_results_u8; + uint8_t port = atoi_results_u8; if (cmd_compare(1, "status")) { bandwidth_status(port); @@ -1698,7 +1687,7 @@ void cmd_parser(void) __banked stpEnabled = 0; } } else if (cmd_compare(0, "pvid") && cmd_words_len == 3) { - if (cmd_parse_port_separator(cmd_words_b[1], false) != 0 && atoi_results_u8 + if (cmd_parse_port_separator(cmd_words_b[1]) != 0 && atoi_results_u8 && atoi_short(cmd_words_b[2]) && atoi_results_short && atoi_results_short <= 4094) port_pvid_set(atoi_results_u8, atoi_results_short); else From f62755900519eb2affb9ab4b624d0bc89da220c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20van=20Dorst?= Date: Tue, 18 Aug 2026 18:53:28 +0200 Subject: [PATCH 22/36] fix CMD: pvid: remove atoi_results_u8 from check --- cmd_parser.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd_parser.c b/cmd_parser.c index 05f1f26..fe33db0 100644 --- a/cmd_parser.c +++ b/cmd_parser.c @@ -1686,8 +1686,8 @@ void cmd_parser(void) __banked stp_off(); stpEnabled = 0; } - } else if (cmd_compare(0, "pvid") && cmd_words_len == 3) { - if (cmd_parse_port_separator(cmd_words_b[1]) != 0 && atoi_results_u8 + } else if (cmd_compare(0, "pvid")) { + if (cmd_words_len == 3 && cmd_parse_port_separator(cmd_words_b[1]) != 0 && atoi_short(cmd_words_b[2]) && atoi_results_short && atoi_results_short <= 4094) port_pvid_set(atoi_results_u8, atoi_results_short); else From 5ddec9710ad61a027f326e3f72bd06fe09b73417 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20van=20Dorst?= Date: Tue, 18 Aug 2026 19:07:13 +0200 Subject: [PATCH 23/36] CMD: Fix parse_lag_hash() --- cmd_parser.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/cmd_parser.c b/cmd_parser.c index fe33db0..83bae49 100644 --- a/cmd_parser.c +++ b/cmd_parser.c @@ -400,18 +400,15 @@ err: void parse_lag_hash(void) { - __xdata uint8_t hash = 0; - - if (cmd_words_len < 2) - goto err; - - if (cmd_parse_port_separator(cmd_buffer[cmd_words_b[1]]) == 0) + // Parse group, expect only one number 0-9. + if (cmd_words_len < 3 || atoi_byte(cmd_words_b[1]) != 1) goto err; uint8_t group = atoi_results_u8 - 1; if (group > 3) goto err; + uint8_t hash = 0; uint8_t w = 2; while (w < cmd_words_len) { if (cmd_compare(w, "spa")) @@ -432,13 +429,14 @@ void parse_lag_hash(void) print_string("Error: invalid hash type:"); print_string_x(&cmd_buffer[cmd_words_b[w]]); write_char('\n'); + goto err; } w++; } port_lag_hash_set(group, hash); return; err: - print_string("Error: lag hash <1-4> [type]...\n"); + print_string("Error: laghash <1-4> [smac|dmac|sip|dip|sport|dport]\n"); } From 3776f12f5c986c2a05cdb10c10ee2d4f73b2579a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20van=20Dorst?= Date: Tue, 18 Aug 2026 20:11:59 +0200 Subject: [PATCH 24/36] Fix and refactor parse_bw() --- cmd_parser.c | 37 ++++++++++++++++++------------------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/cmd_parser.c b/cmd_parser.c index 83bae49..fe76721 100644 --- a/cmd_parser.c +++ b/cmd_parser.c @@ -1327,9 +1327,7 @@ void parse_eee(void) void parse_bw(void) { - __xdata uint32_t bw = 0; - - if (cmd_words_len < 2) // Check for at least 2 arguments + if (cmd_words_len < 3) // Check for at least 3 arguments goto err; if (cmd_parse_port_separator(cmd_words_b[2]) == 0) @@ -1344,8 +1342,13 @@ void parse_bw(void) if (cmd_words_len < 4) // Check for at least 4 arguments goto err; + // Ensure first argument is only `in` or `out`. + __bit is_in = cmd_compare(1, "in") != 0; + if (!(is_in || cmd_compare(1, "out"))) + goto err; + if (cmd_compare(3, "drop")) { - if (cmd_compare(1, "in")) { + if (is_in) { bandwidth_ingress_drop(port); return; } @@ -1353,7 +1356,7 @@ void parse_bw(void) } if (cmd_compare(3, "fc")) { - if (cmd_compare(1, "in")) { + if (is_in) { bandwidth_ingress_fc(port); return; } @@ -1361,34 +1364,30 @@ void parse_bw(void) } if (cmd_compare(3, "off")) { - if (cmd_compare(1, "in")) { + if (is_in) { bandwidth_ingress_disable(port); - return; - } else if (cmd_compare(1, "out")) { + } else { bandwidth_egress_disable(port); - return; } - goto err; + return; } + __xdata uint32_t bw = 0; uint8_t hex_size = atoi_hex(cmd_words_b[3]); - if (hex_size == 0 || hex_size > 4) { + if (hex_size == 0 || hex_size > 4) goto err; - } + uint8_t i = 0; - while (hex_size) { + do { hex_size--; *(((uint8_t *) &bw) + hex_size) = hexvalue[i++]; - } + } while (hex_size); - if (cmd_compare(1, "in")) { + if (is_in) { bandwidth_ingress_set(port, bw); - } else if (cmd_compare(1, "out")) { - bandwidth_egress_set(port, bw); } else { - goto err; + bandwidth_egress_set(port, bw); } - return; err: From 3c183f73f8e72f28cb68d836d9f3a0832b107694 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20van=20Dorst?= Date: Tue, 18 Aug 2026 20:55:57 +0200 Subject: [PATCH 25/36] Added print_port() helper to print physical port number. --- cmd_parser.c | 21 ++++++++------------- rtl837x_common.h | 1 + rtl837x_phy.c | 4 ++-- rtl837x_port.c | 21 ++++----------------- rtlplayground.c | 13 +++++++++++++ 5 files changed, 28 insertions(+), 32 deletions(-) diff --git a/cmd_parser.c b/cmd_parser.c index fe76721..c3582d7 100644 --- a/cmd_parser.c +++ b/cmd_parser.c @@ -27,7 +27,6 @@ extern __code struct machine machine; extern __xdata uint8_t stpEnabled; -extern __code uint8_t log_to_phys_port[9]; extern volatile __xdata uint32_t ticks; extern volatile __xdata uint8_t sfr_data[4]; @@ -356,7 +355,7 @@ void parse_lag(void) print_string(" member ports: "); for (uint8_t j = 0; j < 10; j++) { if (members & 1) { - write_char('0' + machine.log_to_phys_port[j]); + print_port(j); write_char(' '); } members >>= 1; @@ -545,10 +544,7 @@ void parse_isolate(void) members = port_isolation_get(port_configured); for (uint8_t i = 0; i < 10; i++) { if (members & 1) { - if (i < 9) - write_char(machine.log_to_phys_port[i] + '0'); - else - print_string("CPU"); + print_port(i); write_char(' '); } members >>= 1; @@ -613,7 +609,7 @@ void parse_ingress(void) // Setting mode for all ports at once for (log_port = machine.min_port; log_port <= machine.max_port; log_port++) { if (!port_ingress_filter(log_port, mode)) { - print_string("Error setting ingress filter for port "); print_byte(machine.log_to_phys_port[log_port]); write_char('\n'); + print_string("Error setting ingress filter for port "); print_port(log_port); write_char('\n'); return; } print_string("All ports ingress filter set to: "); @@ -622,7 +618,6 @@ void parse_ingress(void) } else { for(uint8_t w = 1; w < cmd_words_len; w++) { idx = cmd_words_b[w]; - char p = cmd_buffer[idx]; uint8_t ret = cmd_parse_port(idx); if (ret != 1) { print_string("Invalid physical port number\n"); @@ -632,14 +627,14 @@ void parse_ingress(void) idx += ret; if (!vlan_ingress_mode_parse(cmd_buffer[idx++], &mode) || !cmd_is_space_or_null(idx)) { - print_string("Invalid ingress mode for port "); write_char(p); print_string(" in ingress command\n"); + print_string("Invalid ingress mode for port "); print_port(log_port); print_string(" in ingress command\n"); goto err; } if (!port_ingress_filter(log_port, mode)) { - print_string("Error setting ingress filter for port "); write_char(p); write_char('\n'); + print_string("Error setting ingress filter for port "); print_port(log_port); write_char('\n'); return; } - print_string("Port "); write_char(p); + print_string("Port "); print_port(log_port); print_string(" ingress filter set to: "); print_port_ingress_filter_mode(mode); write_char('\n'); } @@ -664,7 +659,7 @@ void parse_mirror(void) print_string("NOT Enabled: "); } print_string("Mirroring port: "); - write_char('0' + machine.log_to_phys_port[mPort >> 1]); + print_port(mirroring_port); reg_read_m(RTL837x_MIRROR_CONF); uint16_t m = sfr_data[0]; m = (m << 8) | sfr_data[1]; @@ -818,7 +813,7 @@ void parse_mtu(void) for (p = machine.min_port; p <= machine.max_port; p++) { reg_read_m(RTL8373_REG_MAC_L2_PORT_MAX_LEN + ((uint16_t) p << 8)); uint16_t mtu = SFR_DATA_U16 & 0x3fff; - print_string("Port "); print_byte(machine.log_to_phys_port[p]); + print_string("Port "); print_port(p); write_char(' '); print_short(mtu); write_char('\n'); } return; diff --git a/rtl837x_common.h b/rtl837x_common.h index 861c1e6..27d9449 100644 --- a/rtl837x_common.h +++ b/rtl837x_common.h @@ -135,6 +135,7 @@ void itoa(uint8_t v); void print_sfr_data(void); void print_phy_data(void); void print_cmd_prompt(void); +void print_port(uint8_t port); void phy_write_mask(uint16_t phy_mask, uint8_t dev_id, uint16_t reg, uint16_t v); void phy_write(uint8_t phy_id, uint8_t dev_id, uint16_t reg, uint16_t v); void phy_read(uint8_t phy_id, uint8_t dev_id, uint16_t reg); diff --git a/rtl837x_phy.c b/rtl837x_phy.c index 16fd214..5512755 100644 --- a/rtl837x_phy.c +++ b/rtl837x_phy.c @@ -262,7 +262,7 @@ void phy_set_speed(void) __banked { uint16_t v; - print_string("Setting port "); write_char(machine.log_to_phys_port[phy_settings.port] + '0'); + print_string("Setting port "); print_port(phy_settings.port); if (machine.n_10g && phy_settings.port == 3) phy_settings.is10g_port = 1; if (machine.n_10g == 2 && phy_settings.port == 8) @@ -381,7 +381,7 @@ void phy_set_duplex(void) __banked { uint16_t v; - print_string("Setting port "); write_char(machine.log_to_phys_port[phy_settings.port] + '0'); + print_string("Setting port "); print_port(phy_settings.port); if (phy_settings.duplex) print_string(" to full duplex"); else diff --git a/rtl837x_port.c b/rtl837x_port.c index a85e90d..bc073d0 100644 --- a/rtl837x_port.c +++ b/rtl837x_port.c @@ -390,10 +390,7 @@ void port_l2_learned(void) __banked print_string("\tlearned\t"); port |= (sfr_data[3] & 0x3) << 2; - if (port < 9) - write_char(machine.log_to_phys_port[port] + '0'); - else - print_string("CPU"); + print_port(port); } entry++; @@ -431,7 +428,7 @@ void port_stats_print(void) __banked { print_string("\nPort\tState\tLink\tTxGood\t\tTxBad\t\tRxGood\t\tRxBad\n"); for (uint8_t i = machine.min_port; i <= machine.max_port; i++) { - write_char('0' + machine.log_to_phys_port[i]); write_char('\t'); + print_port(i); write_char('\t'); if (!machine.is_sfp[i]) { phy_read(i, PHY_MMD31, 0xa610); @@ -606,7 +603,7 @@ void port_eee_disable(uint8_t port) __banked void port_eee_status(uint8_t port) __banked { - print_string("Port: "); write_char('0' + machine.log_to_phys_port[port]); + print_string("Port: "); print_port(port); print_string(": "); if (machine.is_sfp[port]) { print_string("SFP\n"); @@ -801,19 +798,9 @@ void print_port_ingress_filter_mode(vlan_ingress_mode_t mode) __banked } } -static void print_phys_port(uint8_t port) __banked -{ - if (port >= machine.min_port && port <= machine.max_port) - write_char(machine.log_to_phys_port[port] + '0'); - else if (port == 9) - write_char('9'); - else - write_char('?'); -} - void print_vlan_ingress_port(uint8_t log_port) __banked { - print_phys_port(log_port);write_char('\t'); + print_port(log_port);write_char('\t'); print_short(port_pvid_get(log_port));write_char('\t'); print_port_ingress_filter_mode(port_ingress_filter_get(log_port));write_char('\t'); port_ingress_vlan_filter_get(log_port) ? print_string("Enabled") : print_string("Disabled"); diff --git a/rtlplayground.c b/rtlplayground.c index d7146e3..d94d0fb 100644 --- a/rtlplayground.c +++ b/rtlplayground.c @@ -813,6 +813,19 @@ void print_reg(uint16_t reg) print_sfr_data(); } +// Print the phy port for a log port number. +void print_port(uint8_t port) +{ + if (port < 9) + write_char(machine.log_to_phys_port[port] + '0'); + else if (port == 9) + print_string("CPU"); + else { + print_string("UNKNOWN "); + write_char(port + '0'); + } +} + /* // TODO: This uses 2 DSEG bytes and is not used! From 5d42e8843ef28068e56e3145447266a674a2e91e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20van=20Dorst?= Date: Tue, 18 Aug 2026 22:05:26 +0200 Subject: [PATCH 26/36] Rename cmd_is_space_or_null to cmd_is_space_or_nul --- cmd_parser.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/cmd_parser.c b/cmd_parser.c index c3582d7..1f4da2d 100644 --- a/cmd_parser.c +++ b/cmd_parser.c @@ -275,15 +275,15 @@ __bit cmd_is_space(uint8_t idx) { return cmd_buffer[idx] == ' '; } -// check if the cmd_buffer[idx] is a space or null. -__bit cmd_is_space_or_null(uint8_t idx) { +// check if the cmd_buffer[idx] is a space or NULL. +__bit cmd_is_space_or_nul(uint8_t idx) { uint8_t c = cmd_buffer[idx]; return c == ' ' || c == '\0'; } // returns 0 when on parser error or invalid value or no space. // return non-zero number of bytes consumed including the space. -// Stops at a space or NULL. +// Stops at a space or NUL. uint8_t parse_ip(uint8_t idx) { uint8_t b = 0; @@ -479,7 +479,7 @@ void parse_vlan(void) vlan_names[vlan_ptr++] = hex[(vlan_settings.vlan >> 8) & 0xf]; vlan_names[vlan_ptr++] = hex[(vlan_settings.vlan >> 4) & 0xf] ; vlan_names[vlan_ptr++] = hex[vlan_settings.vlan & 0xf]; - while(cmd_buffer[cmd_words_b[w] + i] != ' ' && cmd_buffer[cmd_words_b[w] + i] != '\0') { + while(!cmd_is_space_or_nul(cmd_words_b[w] + i)) { write_char(cmd_buffer[cmd_words_b[w] + i]); vlan_names[vlan_ptr++] = cmd_buffer[cmd_words_b[w] + i++]; } @@ -505,7 +505,7 @@ void parse_vlan(void) idx++; } - if (!cmd_is_space_or_null(idx)) + if (!cmd_is_space_or_nul(idx)) goto err; } vlan_create(); @@ -604,7 +604,7 @@ void parse_ingress(void) uint8_t idx = cmd_words_b[1]; if (vlan_ingress_mode_parse(cmd_buffer[idx++], &mode)) { - if (!cmd_is_space_or_null(idx)) + if (!cmd_is_space_or_nul(idx)) goto err; // Setting mode for all ports at once for (log_port = machine.min_port; log_port <= machine.max_port; log_port++) { @@ -626,7 +626,7 @@ void parse_ingress(void) log_port = atoi_results_u8; idx += ret; - if (!vlan_ingress_mode_parse(cmd_buffer[idx++], &mode) || !cmd_is_space_or_null(idx)) { + if (!vlan_ingress_mode_parse(cmd_buffer[idx++], &mode) || !cmd_is_space_or_nul(idx)) { print_string("Invalid ingress mode for port "); print_port(log_port); print_string(" in ingress command\n"); goto err; } @@ -1273,7 +1273,7 @@ void parse_eee(void) if (cmd_buffer[idx] == 'g' || cmd_buffer[idx] == 'm') { // Word 2 is a speed (e.g., "2g5", "100m", "1g") speed_word = 2; - } else if (cmd_buffer[idx] == ' ' || cmd_buffer[idx] == '\0') { + } else if (cmd_is_space_or_nul(idx)) { // Word 2 is a port number if (cmd_parse_port_separator(idx) == 0) { print_string("Speed word invalid, use: [100m|1g|2g5]\n"); From 2ff418f80f936f1d46b41cb6bfe831aacc7fe6e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20van=20Dorst?= Date: Tue, 18 Aug 2026 22:10:31 +0200 Subject: [PATCH 27/36] Replace '\0' to NUL to make it more clear that it is a NUL-terminated string. --- cmd_editor.c | 2 +- cmd_parser.c | 38 +++++++++++++++++++------------------- httpd/httpd.c | 20 ++++++++++---------- rtl837x_common.h | 1 + rtlplayground.c | 8 ++++---- 5 files changed, 35 insertions(+), 34 deletions(-) diff --git a/cmd_editor.c b/cmd_editor.c index 866cf40..aebfe7f 100644 --- a/cmd_editor.c +++ b/cmd_editor.c @@ -193,7 +193,7 @@ void cmd_edit(void) __banked // Check whether return was pressed: if (sbuf[l] == '\n' || sbuf[l] == '\r') { write_char('\n'); - cmd_buffer[cmd_line_len] = '\0'; + cmd_buffer[cmd_line_len] = NUL; // write_char('>'); print_string_x(cmd_buffer); write_char('<'); // If there is a command we print the prompt after execution // otherwise immediately because there is nothing to execute diff --git a/cmd_parser.c b/cmd_parser.c index 1f4da2d..d75233f 100644 --- a/cmd_parser.c +++ b/cmd_parser.c @@ -114,8 +114,8 @@ uint8_t cmd_compare(uint8_t start, __code uint8_t * cmd) uint8_t b = cmd_buffer[i]; // cmd is garanteerd to be NULL-terminated. - if (c == '\0') { - if ((b == ' ') || (b == '\0')) { + if (c == NUL) { + if ((b == ' ') || (b == NUL)) { // Match return 1; } @@ -147,7 +147,7 @@ uint8_t atoi_hex(uint8_t idx) while(1) { c = cmd_buffer[idx]; - if (c == '\0' || c == ' ') { + if (c == NUL || c == ' ') { break; } @@ -263,7 +263,7 @@ uint8_t cmd_parse_port_separator(uint8_t idx) { uint8_t c = cmd_buffer[idx]; if (c == ' ') { ret++; - } else if (c != '\0') + } else if (c != NUL) ret = 0; } return ret; @@ -278,7 +278,7 @@ __bit cmd_is_space(uint8_t idx) { // check if the cmd_buffer[idx] is a space or NULL. __bit cmd_is_space_or_nul(uint8_t idx) { uint8_t c = cmd_buffer[idx]; - return c == ' ' || c == '\0'; + return c == ' ' || c == NUL; } // returns 0 when on parser error or invalid value or no space. @@ -304,7 +304,7 @@ uint8_t parse_ip(uint8_t idx) idx++; break; } - if (ret == '\0') + if (ret == NUL) break; goto err; } @@ -483,7 +483,7 @@ void parse_vlan(void) write_char(cmd_buffer[cmd_words_b[w] + i]); vlan_names[vlan_ptr++] = cmd_buffer[cmd_words_b[w] + i++]; } - vlan_names[vlan_ptr++] = ' '; vlan_names[vlan_ptr] = '\0'; + vlan_names[vlan_ptr++] = ' '; vlan_names[vlan_ptr] = NUL; w++; print_string("<\n"); } @@ -738,11 +738,11 @@ void parse_port(void) } } else if (cmd_compare(2, "name")) { uint8_t i = 0; - while ( (i < PORT_NAME_SIZE-1) && (cmd_buffer[cmd_words_b[3] + i] != '\0') ) { + while ( (i < PORT_NAME_SIZE-1) && (cmd_buffer[cmd_words_b[3] + i] != NUL) ) { port_names[phy_settings.port][i] = cmd_buffer[cmd_words_b[3] + i]; i++; } - port_names[phy_settings.port][i] = '\0'; + port_names[phy_settings.port][i] = NUL; print_string("\nName set to: \""); print_string_x(port_names[phy_settings.port]); print_string("\"\n"); @@ -1248,8 +1248,8 @@ void parse_passwd(void) do { c = cmd_buffer[i++]; passwd[j++] = c; - } while (c != '\0' && j < 20); - passwd[j] = '\0'; + } while (c != NUL && j < 20); + passwd[j] = NUL; return; } print_string("Missing password\n"); @@ -1453,7 +1453,7 @@ void cmd_tokenize(void) __banked while(1) { c = cmd_buffer[line_ptr]; - if (c == '\0') { + if (c == NUL) { // Store the word count cmd_words_len = word; break; @@ -1658,13 +1658,13 @@ void cmd_parser(void) __banked __xdata char *dst = hostname; for (uint8_t hn = 0; hn < sizeof(hostname) - 1; hn++) { uint8_t c = *hp++; - if (c == '\0' || c == '\r' || c == '\n') + if (c == NUL || c == '\r' || c == '\n') break; if (c < 0x20 || c > 0x7e || c == '"' || c == '\\') c = '.'; *dst++ = c; } - *dst = '\0'; + *dst = NUL; } else { print_string("Error: hostname [name] - the name must not contain spaces\n"); } @@ -1752,7 +1752,7 @@ void cmd_parser(void) __banked uint8_t i = cmd_words_b[cmd_words_len - 1]; do { i++; - } while(cmd_buffer[i] != '\0'); + } while(cmd_buffer[i] != NUL); // Copy last cmd-buffer to history. cmd_history_ptr = (cmd_history_ptr + i) & CMD_HISTORY_MASK; @@ -1800,7 +1800,7 @@ void execute_config(void) __banked uint8_t c = 0; do { if (cmd_idx >= (CMD_BUF_SIZE - 1)) { - cmd_buffer[cmd_idx] = '\0'; + cmd_buffer[cmd_idx] = NUL; print_string("ERROR: Command too long: "); print_string_x(cmd_buffer); write_char('\n'); @@ -1809,7 +1809,7 @@ void execute_config(void) __banked } c = flash_buf[cfg_idx++]; if (c == 0 || c == '\n') { - cmd_buffer[cmd_idx] = '\0'; + cmd_buffer[cmd_idx] = NUL; if (cmd_idx) { cmd_tokenize(); if (err_status != ERR_OK) @@ -1845,7 +1845,7 @@ void execute_commands(__xdata uint8_t *p) __banked { while (1) { if (*p == 0 || *p == '\n' || *p == '\r') { if (cmd_idx) { - cmd_buffer[cmd_idx] = '\0'; + cmd_buffer[cmd_idx] = NUL; cmd_tokenize(); if (err_status != ERR_OK) return; @@ -1858,7 +1858,7 @@ void execute_commands(__xdata uint8_t *p) __banked { if (cmd_idx < (CMD_BUF_SIZE - 1)) { cmd_buffer[cmd_idx++] = *p; } else { - cmd_buffer[CMD_BUF_SIZE - 1] = '\0'; + cmd_buffer[CMD_BUF_SIZE - 1] = NUL; print_string("ERROR: Command too long: "); print_string_x(cmd_buffer); write_char('\n'); diff --git a/httpd/httpd.c b/httpd/httpd.c index ff4eff9..8f52631 100644 --- a/httpd/httpd.c +++ b/httpd/httpd.c @@ -119,8 +119,8 @@ bool is_word(__xdata uint8_t *xdata_str_p, __code uint8_t * __xdata code_str_p) u = *xdata_str_p++; c = *code_str_p++; - if (c == '\0') { - if (u != '\0' && u != ' ' && u != '\t' && u != ':' && u != '?' && u != '=' && u != '\n' && u != '\r') + if (c == NUL) { + if (u != NUL && u != ' ' && u != '\t' && u != ':' && u != '?' && u != '=' && u != '\n' && u != '\r') return false; return true; } @@ -140,8 +140,8 @@ bool is_url_word_x(__xdata uint8_t *uri_str_p, __xdata uint8_t *src_str_p) u = *uri_str_p++; s = *src_str_p++; - if (s == '\0') { - if (u != '\0' && u != ' ' && u != '\t' && u != ':' && u != '?' && u != '=' && u != '\n' && u != '\r') + if (s == NUL) { + if (u != NUL && u != ' ' && u != '\t' && u != ':' && u != '?' && u != '=' && u != '\n' && u != '\r') return false; return true; } @@ -183,9 +183,9 @@ bool is_word_x(__xdata uint8_t *lhs_str_p, __xdata uint8_t *rhs_str_p) u = *lhs_str_p++; c = *rhs_str_p++; - if (c == '\0') { + if (c == NUL) { /* ';' separates cookies in a Cookie header, so it ends a value too. */ - if (u != '\0' && u != ' ' && u != '\t' && u != ':' && u != '?' && u != '=' && u != '\n' && u != '\r' && u != ';') + if (u != NUL && u != ' ' && u != '\t' && u != ':' && u != '?' && u != '=' && u != '\n' && u != '\r' && u != ';') return false; return true; } @@ -487,10 +487,10 @@ void handle_post(void) // Find end of request path while (*p && !is_separator(*p)) p++; - *p++ = '\0'; + *p++ = NUL; // Find end of request header - boundary[0] ='\0'; + boundary[0] =NUL; p = scan_header(p); dbg_string("Boundary: >"); dbg_string_x(boundary); dbg_string("<\n"); if (!*p || !content_type) { @@ -550,7 +550,7 @@ void handle_post(void) dbg_string("Password accepted!\n"); read_reg_timer(&last_session_use); gen_random_bytes(session_id, SESSION_ID_LENGTH); - session_id[SESSION_ID_LENGTH] = '\0'; + session_id[SESSION_ID_LENGTH] = NUL; slen = strtox(outbuf, "HTTP/1.1 302 Found\r\nConnection: close\r\nLocation: index.html\r\n" \ "Set-Cookie: session="); for (register uint8_t i = 0; i < SESSION_ID_LENGTH; i++) @@ -736,7 +736,7 @@ void httpd_appcall(void) __xdata uint8_t *q = p; while (*p && !is_separator(*p)) p++; - *p = '\0'; + *p = NUL; dbg_string_x(q); dbg_char('\n'); diff --git a/rtl837x_common.h b/rtl837x_common.h index 27d9449..8f7bf1b 100644 --- a/rtl837x_common.h +++ b/rtl837x_common.h @@ -8,6 +8,7 @@ #define SYS_TICK_HZ 200 #define CPU_PORT 9 +#define NUL '\0' // Define Port-masks for 9-port devices and 6-port devices #define PMASK_9 0x1ff diff --git a/rtlplayground.c b/rtlplayground.c index d94d0fb..a7f1dc5 100644 --- a/rtlplayground.c +++ b/rtlplayground.c @@ -1257,11 +1257,11 @@ bool sfp_read_field(__xdata char *dst, uint8_t sfp, uint8_t start, uint8_t lengt if (!sfp_read_block(sfp, start, length)) return false; - dst[length] = '\0'; + dst[length] = NUL; memcpy(dst, sfp_buf, length); while (length > 0 && dst[--length] == ' ') - dst[length] = '\0'; + dst[length] = NUL; return true; } @@ -2069,7 +2069,7 @@ void check_and_flash_update_image(void) * because itohex() is inline and brings its own frame. */ void set_hostname_default(void) { - if (hostname[0] != '\0') + if (hostname[0] != NUL) return; strcpy((__xdata uint8_t *)hostname, "RTLPlayground-"); @@ -2079,7 +2079,7 @@ void set_hostname_default(void) hostname[17] = hex[uip_ethaddr.addr[4] & 0xf]; hostname[18] = hex[uip_ethaddr.addr[5] >> 4]; hostname[19] = hex[uip_ethaddr.addr[5] & 0xf]; - hostname[20] = '\0'; + hostname[20] = NUL; } From e62e0fc8bd45969528d369086da8e45556d5a7c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20van=20Dorst?= Date: Tue, 18 Aug 2026 23:06:12 +0200 Subject: [PATCH 28/36] CMD: Isolate allow the CPU-port as a member. --- cmd_parser.c | 30 +++++++++++++++++++++++++++++- rtlplayground.c | 4 ++-- 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/cmd_parser.c b/cmd_parser.c index d75233f..2d64708 100644 --- a/cmd_parser.c +++ b/cmd_parser.c @@ -269,6 +269,34 @@ uint8_t cmd_parse_port_separator(uint8_t idx) { return ret; } +// Same as cmd_parse_port_separator() addition to allow the CPU-port. +// returns 0 when on parser error or invalid value or no space or no NUL. +// return non-zero number of bytes consumed including the space. +uint8_t cmd_parse_port_cpu_separator(uint8_t idx) { + uint8_t ret = atoi_byte(idx); + // Check valid con + if (ret != 0) { + // port number 1-10 -> 0-9 + atoi_results_u8 -= 1; + if(atoi_results_u8 < CPU_PORT) + // Phy port, translate it. + ret = cmd_parse_port(idx); + // Check is not + else if (atoi_results_u8 > CPU_PORT) + ret = 0; + } + + if (ret != 0) { + idx += ret; + uint8_t c = cmd_buffer[idx]; + if (c == ' ') { + ret++; + } else if (c != NUL) + ret = 0; + } + return ret; +} + // check if the cmd_buffer[idx] is a space. __bit cmd_is_space(uint8_t idx) { @@ -562,7 +590,7 @@ void parse_isolate(void) uint8_t w = 2; while (w < cmd_words_len) { - if (cmd_parse_port_separator(cmd_words_b[w++]) == 0) + if (cmd_parse_port_cpu_separator(cmd_words_b[w++]) == 0) goto err; uint8_t port = atoi_results_u8; members |= ((uint16_t)1) << port; diff --git a/rtlplayground.c b/rtlplayground.c index a7f1dc5..63a9c39 100644 --- a/rtlplayground.c +++ b/rtlplayground.c @@ -816,9 +816,9 @@ void print_reg(uint16_t reg) // Print the phy port for a log port number. void print_port(uint8_t port) { - if (port < 9) + if (port < CPU_PORT) write_char(machine.log_to_phys_port[port] + '0'); - else if (port == 9) + else if (port == CPU_PORT) print_string("CPU"); else { print_string("UNKNOWN "); From 86532160479d1e9d3c1700ba01a673d6992cace4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20van=20Dorst?= Date: Wed, 19 Aug 2026 19:23:51 +0200 Subject: [PATCH 29/36] CMD: Fix some comments --- cmd_parser.c | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/cmd_parser.c b/cmd_parser.c index 2d64708..3ef2fe3 100644 --- a/cmd_parser.c +++ b/cmd_parser.c @@ -113,7 +113,7 @@ uint8_t cmd_compare(uint8_t start, __code uint8_t * cmd) uint8_t c = cmd[j]; uint8_t b = cmd_buffer[i]; - // cmd is garanteerd to be NULL-terminated. + // cmd is garanteerd to be NUL-terminated. if (c == NUL) { if ((b == ' ') || (b == NUL)) { // Match @@ -253,9 +253,9 @@ uint8_t cmd_parse_port(uint8_t idx) { } -// Same as cmd_parse_port() addition to check the trailing space or NUL. -// returns 0 when on parser error or invalid value or no space or no NUL. -// return non-zero number of bytes consumed including the space. +// Same as cmd_parse_port() addition to check the trailing SPACE or NUL. +// returns 0 when on parser error or invalid value or no SPACE or no NUL. +// return non-zero number of bytes consumed including the SPACE. uint8_t cmd_parse_port_separator(uint8_t idx) { uint8_t ret = cmd_parse_port(idx); if (ret != 0) { @@ -270,18 +270,17 @@ uint8_t cmd_parse_port_separator(uint8_t idx) { } // Same as cmd_parse_port_separator() addition to allow the CPU-port. -// returns 0 when on parser error or invalid value or no space or no NUL. -// return non-zero number of bytes consumed including the space. +// returns 0 when on parser error or invalid value or no SPACE or no NUL. +// return non-zero number of bytes consumed including the SPACE. uint8_t cmd_parse_port_cpu_separator(uint8_t idx) { uint8_t ret = atoi_byte(idx); - // Check valid con + // Check valid conversion if (ret != 0) { // port number 1-10 -> 0-9 atoi_results_u8 -= 1; if(atoi_results_u8 < CPU_PORT) // Phy port, translate it. ret = cmd_parse_port(idx); - // Check is not else if (atoi_results_u8 > CPU_PORT) ret = 0; } @@ -298,20 +297,20 @@ uint8_t cmd_parse_port_cpu_separator(uint8_t idx) { } -// check if the cmd_buffer[idx] is a space. +// check if the cmd_buffer[idx] is a SPACE. __bit cmd_is_space(uint8_t idx) { return cmd_buffer[idx] == ' '; } -// check if the cmd_buffer[idx] is a space or NULL. +// check if the cmd_buffer[idx] is a SPACE or NUL. __bit cmd_is_space_or_nul(uint8_t idx) { uint8_t c = cmd_buffer[idx]; return c == ' ' || c == NUL; } -// returns 0 when on parser error or invalid value or no space. -// return non-zero number of bytes consumed including the space. -// Stops at a space or NUL. +// returns 0 when on parser error or invalid value or no SPACE. +// return non-zero number of bytes consumed including the SPACE. +// Stops at a SPACE or NUL. uint8_t parse_ip(uint8_t idx) { uint8_t b = 0; @@ -1268,7 +1267,7 @@ void parse_rnd(void) void parse_passwd(void) { - // cmd_words_len can be more then 2 if a space in the password. + // cmd_words_len can be more then 2 if a SPACE in the password. if (cmd_words_len >= 2) { uint8_t i = cmd_words_b[1]; uint8_t c = 0; @@ -1776,7 +1775,7 @@ void cmd_parser(void) __banked if (save_cmd && cmd_words_len) { - // Find end of the cmd-buffer, looking for the NULL-byte. + // Find end of the cmd-buffer, looking for the NUL-byte. uint8_t i = cmd_words_b[cmd_words_len - 1]; do { i++; From f511453f79997e4e247cd3577661de40c106dcdd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20van=20Dorst?= Date: Wed, 19 Aug 2026 19:31:21 +0200 Subject: [PATCH 30/36] CMD: refactor code --- cmd_parser.c | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/cmd_parser.c b/cmd_parser.c index 3ef2fe3..d0ba703 100644 --- a/cmd_parser.c +++ b/cmd_parser.c @@ -368,7 +368,6 @@ void parse_lag(void) { __xdata uint8_t group; __xdata uint16_t members = 0; - uint8_t ret; if (cmd_compare(1, "show")) { print_string("LAG status:\n"); @@ -398,9 +397,8 @@ void parse_lag(void) if (cmd_words_len < 2) goto err; - // Parse group, expect only one number 0-9. - ret = atoi_byte(cmd_words_b[1]); - if (ret != 1) + // Parse group, expect only one number 0-9; + if (atoi_byte(cmd_words_b[1]) != 1) goto err; group = atoi_results_u8 - 1; @@ -834,7 +832,7 @@ void parse_port(void) void parse_mtu(void) { - uint8_t p, ret; + uint8_t p; if (cmd_compare(1, "show")) { for (p = machine.min_port; p <= machine.max_port; p++) { @@ -854,9 +852,7 @@ void parse_mtu(void) p = atoi_results_u8; print_byte(p); - ret = atoi_short(cmd_words_b[2]); - - if (!ret || atoi_results_short < 64 || atoi_results_short > 0x3fff) { + if (atoi_short(cmd_words_b[2]) == 0 || atoi_results_short < 64 || atoi_results_short > 0x3fff) { print_string("MTU must be 64..16383\n"); return; } @@ -959,8 +955,6 @@ err: void parse_regget(void) { - uint16_t reg = 0; - if (cmd_words_len != 2) { goto err; } @@ -971,7 +965,7 @@ void parse_regget(void) goto err; } - reg = hexvalue[0]; + uint16_t reg = hexvalue[0]; if (hex_size == 2) { reg <<= 8; reg |= hexvalue[1]; @@ -994,8 +988,6 @@ err: void parse_regset(void) { - uint16_t reg = 0; - if (cmd_words_len != 3) { goto err; } @@ -1005,7 +997,7 @@ void parse_regset(void) goto err; } - reg = hexvalue[0]; + uint16_t reg = hexvalue[0]; if (hex_size == 2) { reg <<= 8; reg |= hexvalue[1]; From 95b3bde82276bd081fcf6b3296acfff1f6d695fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20van=20Dorst?= Date: Wed, 19 Aug 2026 19:34:34 +0200 Subject: [PATCH 31/36] Rename print_port() to print_phys_port() --- cmd_parser.c | 16 ++++++++-------- rtl837x_common.h | 2 +- rtl837x_phy.c | 4 ++-- rtl837x_port.c | 8 ++++---- rtlplayground.c | 4 ++-- 5 files changed, 17 insertions(+), 17 deletions(-) diff --git a/cmd_parser.c b/cmd_parser.c index d0ba703..848a74f 100644 --- a/cmd_parser.c +++ b/cmd_parser.c @@ -381,7 +381,7 @@ void parse_lag(void) print_string(" member ports: "); for (uint8_t j = 0; j < 10; j++) { if (members & 1) { - print_port(j); + print_phys_port(j); write_char(' '); } members >>= 1; @@ -569,7 +569,7 @@ void parse_isolate(void) members = port_isolation_get(port_configured); for (uint8_t i = 0; i < 10; i++) { if (members & 1) { - print_port(i); + print_phys_port(i); write_char(' '); } members >>= 1; @@ -634,7 +634,7 @@ void parse_ingress(void) // Setting mode for all ports at once for (log_port = machine.min_port; log_port <= machine.max_port; log_port++) { if (!port_ingress_filter(log_port, mode)) { - print_string("Error setting ingress filter for port "); print_port(log_port); write_char('\n'); + print_string("Error setting ingress filter for port "); print_phys_port(log_port); write_char('\n'); return; } print_string("All ports ingress filter set to: "); @@ -652,14 +652,14 @@ void parse_ingress(void) idx += ret; if (!vlan_ingress_mode_parse(cmd_buffer[idx++], &mode) || !cmd_is_space_or_nul(idx)) { - print_string("Invalid ingress mode for port "); print_port(log_port); print_string(" in ingress command\n"); + print_string("Invalid ingress mode for port "); print_phys_port(log_port); print_string(" in ingress command\n"); goto err; } if (!port_ingress_filter(log_port, mode)) { - print_string("Error setting ingress filter for port "); print_port(log_port); write_char('\n'); + print_string("Error setting ingress filter for port "); print_phys_port(log_port); write_char('\n'); return; } - print_string("Port "); print_port(log_port); + print_string("Port "); print_phys_port(log_port); print_string(" ingress filter set to: "); print_port_ingress_filter_mode(mode); write_char('\n'); } @@ -684,7 +684,7 @@ void parse_mirror(void) print_string("NOT Enabled: "); } print_string("Mirroring port: "); - print_port(mirroring_port); + print_phys_port(mirroring_port); reg_read_m(RTL837x_MIRROR_CONF); uint16_t m = sfr_data[0]; m = (m << 8) | sfr_data[1]; @@ -838,7 +838,7 @@ void parse_mtu(void) for (p = machine.min_port; p <= machine.max_port; p++) { reg_read_m(RTL8373_REG_MAC_L2_PORT_MAX_LEN + ((uint16_t) p << 8)); uint16_t mtu = SFR_DATA_U16 & 0x3fff; - print_string("Port "); print_port(p); + print_string("Port "); print_phys_port(p); write_char(' '); print_short(mtu); write_char('\n'); } return; diff --git a/rtl837x_common.h b/rtl837x_common.h index 8f7bf1b..432260e 100644 --- a/rtl837x_common.h +++ b/rtl837x_common.h @@ -136,7 +136,7 @@ void itoa(uint8_t v); void print_sfr_data(void); void print_phy_data(void); void print_cmd_prompt(void); -void print_port(uint8_t port); +void print_phys_port(uint8_t port); void phy_write_mask(uint16_t phy_mask, uint8_t dev_id, uint16_t reg, uint16_t v); void phy_write(uint8_t phy_id, uint8_t dev_id, uint16_t reg, uint16_t v); void phy_read(uint8_t phy_id, uint8_t dev_id, uint16_t reg); diff --git a/rtl837x_phy.c b/rtl837x_phy.c index 5512755..240d1d0 100644 --- a/rtl837x_phy.c +++ b/rtl837x_phy.c @@ -262,7 +262,7 @@ void phy_set_speed(void) __banked { uint16_t v; - print_string("Setting port "); print_port(phy_settings.port); + print_string("Setting port "); print_phys_port(phy_settings.port); if (machine.n_10g && phy_settings.port == 3) phy_settings.is10g_port = 1; if (machine.n_10g == 2 && phy_settings.port == 8) @@ -381,7 +381,7 @@ void phy_set_duplex(void) __banked { uint16_t v; - print_string("Setting port "); print_port(phy_settings.port); + print_string("Setting port "); print_phys_port(phy_settings.port); if (phy_settings.duplex) print_string(" to full duplex"); else diff --git a/rtl837x_port.c b/rtl837x_port.c index bc073d0..103ac4e 100644 --- a/rtl837x_port.c +++ b/rtl837x_port.c @@ -390,7 +390,7 @@ void port_l2_learned(void) __banked print_string("\tlearned\t"); port |= (sfr_data[3] & 0x3) << 2; - print_port(port); + print_phys_port(port); } entry++; @@ -428,7 +428,7 @@ void port_stats_print(void) __banked { print_string("\nPort\tState\tLink\tTxGood\t\tTxBad\t\tRxGood\t\tRxBad\n"); for (uint8_t i = machine.min_port; i <= machine.max_port; i++) { - print_port(i); write_char('\t'); + print_phys_port(i); write_char('\t'); if (!machine.is_sfp[i]) { phy_read(i, PHY_MMD31, 0xa610); @@ -603,7 +603,7 @@ void port_eee_disable(uint8_t port) __banked void port_eee_status(uint8_t port) __banked { - print_string("Port: "); print_port(port); + print_string("Port: "); print_phys_port(port); print_string(": "); if (machine.is_sfp[port]) { print_string("SFP\n"); @@ -800,7 +800,7 @@ void print_port_ingress_filter_mode(vlan_ingress_mode_t mode) __banked void print_vlan_ingress_port(uint8_t log_port) __banked { - print_port(log_port);write_char('\t'); + print_phys_port(log_port);write_char('\t'); print_short(port_pvid_get(log_port));write_char('\t'); print_port_ingress_filter_mode(port_ingress_filter_get(log_port));write_char('\t'); port_ingress_vlan_filter_get(log_port) ? print_string("Enabled") : print_string("Disabled"); diff --git a/rtlplayground.c b/rtlplayground.c index 63a9c39..f0a3292 100644 --- a/rtlplayground.c +++ b/rtlplayground.c @@ -813,8 +813,8 @@ void print_reg(uint16_t reg) print_sfr_data(); } -// Print the phy port for a log port number. -void print_port(uint8_t port) +// Print the physical port of a logical port number. +void print_phys_port(uint8_t port) { if (port < CPU_PORT) write_char(machine.log_to_phys_port[port] + '0'); From 1212def7992db3496add2679dc172b724a34f5f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20van=20Dorst?= Date: Thu, 20 Aug 2026 19:37:34 +0200 Subject: [PATCH 32/36] CMD: fixes many comments --- cmd_parser.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/cmd_parser.c b/cmd_parser.c index 848a74f..7273582 100644 --- a/cmd_parser.c +++ b/cmd_parser.c @@ -113,7 +113,7 @@ uint8_t cmd_compare(uint8_t start, __code uint8_t * cmd) uint8_t c = cmd[j]; uint8_t b = cmd_buffer[i]; - // cmd is garanteerd to be NUL-terminated. + // cmd is guaranteed to be NUL-terminated. if (c == NUL) { if ((b == ' ') || (b == NUL)) { // Match @@ -187,8 +187,8 @@ uint8_t atoi_hex(uint8_t idx) } -// return 0 on error or non-zero number of number-byte taken for the conversion. -// Stops at any non-digit '0'-'9' char or bytes is more then 3. +// Returns 0 or the number of digits taken into account for conversion. +// Stops at any non-digit '0'-'9' char or more than 3 bytes. uint8_t atoi_byte(uint8_t idx) { uint8_t cnt = 0; @@ -211,7 +211,7 @@ uint8_t atoi_byte(uint8_t idx) } -// return 0 on error or non-zero number of number-byte taken for the conversion. +// Returns 0 or the number of digits taken into account for conversion. // Stops at any non-digit '0'-'9' char or bytes is more then 5. uint8_t atoi_short(uint8_t idx) { @@ -234,9 +234,9 @@ uint8_t atoi_short(uint8_t idx) } /* Parse, validate and translate phys_to_log_port physical port argument. - * So CPU-port 10 is not valid. + * The CPU-port, i.e. port 10, is not a valid argument. * returns 0 when on parser error or invalid value. - * return non-zero number of bytes consumed. + * returns non-zero number of characters consumed. * Store the value in atoi_results_u8. */ uint8_t cmd_parse_port(uint8_t idx) { @@ -253,9 +253,9 @@ uint8_t cmd_parse_port(uint8_t idx) { } -// Same as cmd_parse_port() addition to check the trailing SPACE or NUL. +// Same as cmd_parse_port() but additionally check for trailing SPACE or NUL. // returns 0 when on parser error or invalid value or no SPACE or no NUL. -// return non-zero number of bytes consumed including the SPACE. +// returns non-zero number of characters consumed including the SPACE. uint8_t cmd_parse_port_separator(uint8_t idx) { uint8_t ret = cmd_parse_port(idx); if (ret != 0) { @@ -269,9 +269,9 @@ uint8_t cmd_parse_port_separator(uint8_t idx) { return ret; } -// Same as cmd_parse_port_separator() addition to allow the CPU-port. +// Same as cmd_parse_port_separator() but additionally allow CPU-port. // returns 0 when on parser error or invalid value or no SPACE or no NUL. -// return non-zero number of bytes consumed including the SPACE. +// returns number of characters consumed including the SPACE. uint8_t cmd_parse_port_cpu_separator(uint8_t idx) { uint8_t ret = atoi_byte(idx); // Check valid conversion @@ -308,9 +308,9 @@ __bit cmd_is_space_or_nul(uint8_t idx) { return c == ' ' || c == NUL; } -// returns 0 when on parser error or invalid value or no SPACE. -// return non-zero number of bytes consumed including the SPACE. -// Stops at a SPACE or NUL. +// Parse an IPv4 address +// returns 0 when on parse error or invalid value or it don't ends with SPACE or NUL. +// returns non-zero number of characters consumed including the SPACE. uint8_t parse_ip(uint8_t idx) { uint8_t b = 0; @@ -347,7 +347,7 @@ err: return 0; } -// Print a IPv4 adress. +// Prints an IPv4 address. void print_ip(__xdata uint8_t * ptr) { uint8_t idx = 0; From beb14deba56dbbda1e1b93ba07514c6ffc120d5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20van=20Dorst?= Date: Sun, 23 Aug 2026 14:46:02 +0200 Subject: [PATCH 33/36] httpd: send_counter(): Validate phys_port_idx and better error handling --- httpd/httpd.c | 9 +-------- httpd/page_impl.c | 30 ++++++++++++++++++++++-------- httpd/page_impl.h | 4 +++- 3 files changed, 26 insertions(+), 17 deletions(-) diff --git a/httpd/httpd.c b/httpd/httpd.c index 8f52631..411ba0c 100644 --- a/httpd/httpd.c +++ b/httpd/httpd.c @@ -758,16 +758,9 @@ void httpd_appcall(void) parse_short(q + 15); send_vlan(short_parsed); } else if (is_word(q, "/counters.json")) { - /* The port is one raw character of the request line and - * indexes a nine entry table, so bound it here instead - * of trusting the client to have sent a digit. Anything - * below '0' wraps well past eight, so the one test - * covers both ends. */ uint8_t cport = q[20] - '0'; - if (cport > 8) + if (send_counters(cport)) send_bad_request(); - else - send_counters(cport); } else if (is_word(q, "/eee.json")) { send_eee(); } else if (is_word(q, "/bandwidth.json")) { diff --git a/httpd/page_impl.c b/httpd/page_impl.c index 92aeb1a..4bcf497 100644 --- a/httpd/page_impl.c +++ b/httpd/page_impl.c @@ -289,17 +289,25 @@ void send_vlan(uint16_t vlan) slen += strtox(outbuf + slen, "\"}"); } - -void send_counters(char port) +/* Send counters + * Only accepts physical port idx to 0-8. + * Returns an error if the port physical don't exists. + */ +bool send_counters(uint8_t phys_port_idx) { - dbg_string("send_counters called: "); dbg_byte(port); dbg_char('\n'); + if (phys_port_idx > 8) + goto err; + uint8_t log_port = machine.phys_to_log_port[phys_port_idx]; + if (log_port == 0) + goto err; + + dbg_string("send_counters called: "); dbg_byte(phys_port_idx); dbg_char('\n'); slen = strtox(outbuf, HTTP_RESPONCE_JSON); - dbg_string("sending counters\n"); - dbg_byte(port); - uint8_t i = machine.phys_to_log_port[port]; - slen += strtox(outbuf + slen, "["); + dbg_string("sending counters\n"); dbg_byte(phys_port_idx); + + char_to_html('['); for (uint8_t counter = 0; counter < 0x37; counter++) { - STAT_GET(counter, i); + STAT_GET(counter, log_port); slen += strtox(outbuf + slen, "\"0x"); reg_to_html(RTL837X_STAT_V_HIGH); reg_to_html_long(RTL837X_STAT_V_LOW); @@ -308,6 +316,12 @@ void send_counters(char port) char_to_html(','); } char_to_html(']'); + + return false; + +err: + dbg_string("Error: counters: phy_port_idx don't exists\n"); + return true; } diff --git a/httpd/page_impl.h b/httpd/page_impl.h index 7907285..1cbd94e 100644 --- a/httpd/page_impl.h +++ b/httpd/page_impl.h @@ -1,7 +1,9 @@ #ifndef __PAGE_IMPL_H__ #define __PAGE_IMPL_H__ -void send_counters(char port); +#include + +bool send_counters(uint8_t phys_port_idx); void send_status(void); void send_vlan(uint16_t vlan); void send_basic_info(void); From 74013bb14fec8754c991671b5a2845f335d1de70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20van=20Dorst?= Date: Sun, 23 Aug 2026 15:33:04 +0200 Subject: [PATCH 34/36] CMD: Treat port zero as CPU-port --- cmd_parser.c | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/cmd_parser.c b/cmd_parser.c index 7273582..66b05bd 100644 --- a/cmd_parser.c +++ b/cmd_parser.c @@ -234,7 +234,7 @@ uint8_t atoi_short(uint8_t idx) } /* Parse, validate and translate phys_to_log_port physical port argument. - * The CPU-port, i.e. port 10, is not a valid argument. + * The CPU-port, i.e. port 0, is not a valid argument. * returns 0 when on parser error or invalid value. * returns non-zero number of characters consumed. * Store the value in atoi_results_u8. @@ -273,16 +273,11 @@ uint8_t cmd_parse_port_separator(uint8_t idx) { // returns 0 when on parser error or invalid value or no SPACE or no NUL. // returns number of characters consumed including the SPACE. uint8_t cmd_parse_port_cpu_separator(uint8_t idx) { - uint8_t ret = atoi_byte(idx); - // Check valid conversion - if (ret != 0) { - // port number 1-10 -> 0-9 - atoi_results_u8 -= 1; - if(atoi_results_u8 < CPU_PORT) - // Phy port, translate it. - ret = cmd_parse_port(idx); - else if (atoi_results_u8 > CPU_PORT) - ret = 0; + uint8_t ret = cmd_parse_port(idx); + + if (ret == 0 && cmd_buffer[idx] == '0') { + ret = 1; + atoi_results_u8 = CPU_PORT; } if (ret != 0) { From ee05cd400088b8adc42e4b0bccee98543bd4f1a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20van=20Dorst?= Date: Mon, 24 Aug 2026 22:03:50 +0200 Subject: [PATCH 35/36] Fix: `send_counters()`, `port`-argument have to be physical port number. Not the physical port index! --- html/stat.js | 42 +++++++++++++++++++++--------------------- httpd/page_impl.c | 5 +++-- httpd/page_impl.h | 2 +- 3 files changed, 25 insertions(+), 24 deletions(-) diff --git a/html/stat.js b/html/stat.js index 2709eeb..0944d96 100644 --- a/html/stat.js +++ b/html/stat.js @@ -109,7 +109,7 @@ const mib_counters = [ function getCounters(port) { var xhttp = new XMLHttpRequest(); const popup = document.getElementById('popup'); - xhttp.onreadystatechange = function() { + xhttp.onreadystatechange = function () { if (this.readyState == 4 && this.status == 200) { const s = JSON.parse(xhttp.responseText); console.log("Counters: ", JSON.stringify(s)); @@ -118,16 +118,16 @@ function getCounters(port) { console.log("Counter 0: ", BigInt(s[0]).toString(), " length: ", s.length); var c = 0; for (i = 0; i < mib_counters.length; i += 4) { - console.log(i, " ", mib_counters[i], ": ", mib_counters[i+1]); + console.log(i, " ", mib_counters[i], ": ", mib_counters[i + 1]); if (mib_counters[i] == "" && mib_counters[i + 1] == 8) { console.log("c " + i + ": continue"); continue; } - var count = BigInt(s[i/4]); - if (mib_counters[i+1] == 8) { + var count = BigInt(s[i / 4]); + if (mib_counters[i + 1] == 8) { tableHtml += "" + mib_counters[i] + "" + count.toString() + ""; c += 1; - } else if (mib_counters[i+1] == 4) { + } else if (mib_counters[i + 1] == 4) { if (mib_counters[i] != "") { tableHtml += "" + mib_counters[i] + "" + (count >> 32n).toString() + ""; c += 1; @@ -136,8 +136,8 @@ function getCounters(port) { tableHtml += " "; c = 0; } - if (mib_counters[i+2] != "") { - tableHtml += "" + mib_counters[i+2] + "" + (count & 4294967295n).toString() + ""; + if (mib_counters[i + 2] != "") { + tableHtml += "" + mib_counters[i + 2] + "" + (count & 4294967295n).toString() + ""; c += 1; } } @@ -161,26 +161,26 @@ function fillStats() { return; if (tbl.rows.length > 1) { for (let i = 0; i < numPorts; i++) { - console.log("Table Update row: " + i + " state " + pState[i] + " is " + linkS[pState[i] +1]); - tbl.rows[i+1].cells[2].innerHTML = linkText(pState[i]+1); - tbl.rows[i+1].cells[3].innerHTML = `${txG[i]}` + t('common_pkts'); - tbl.rows[i+1].cells[4].innerHTML = `${txB[i]}` + t('common_pkts'); - tbl.rows[i+1].cells[5].innerHTML = `${rxG[i]}` + t('common_pkts'); - tbl.rows[i+1].cells[6].innerHTML = `${rxB[i]}` + t('common_pkts'); + console.log("Table Update row: " + i + " state " + pState[i] + " is " + linkS[pState[i] + 1]); + tbl.rows[i + 1].cells[2].innerHTML = linkText(pState[i] + 1); + tbl.rows[i + 1].cells[3].innerHTML = `${txG[i]}` + t('common_pkts'); + tbl.rows[i + 1].cells[4].innerHTML = `${txB[i]}` + t('common_pkts'); + tbl.rows[i + 1].cells[5].innerHTML = `${rxG[i]}` + t('common_pkts'); + tbl.rows[i + 1].cells[6].innerHTML = `${rxB[i]}` + t('common_pkts'); } } else { for (let i = 0; i < numPorts; i++) { console.log("Table row: " + i); const tr = tbl.insertRow(); - let td = tr.insertCell(); td.appendChild(document.createTextNode(t('common_port') + (i+1))); + let td = tr.insertCell(); td.appendChild(document.createTextNode(t('common_port') + (i + 1))); let portName = portNames[physToLogPort[i]] || ''; td = tr.insertCell(); td.appendChild(document.createTextNode(portName)); - td = tr.insertCell(); td.appendChild(document.createTextNode(linkText(pState[i]+1))); + td = tr.insertCell(); td.appendChild(document.createTextNode(linkText(pState[i] + 1))); td = tr.insertCell(); td.appendChild(document.createTextNode(`${txG[i]}` + t('common_pkts'))); - td = tr.insertCell();td.appendChild(document.createTextNode(`${txB[i]}` + t('common_pkts'))); - td = tr.insertCell();td.appendChild(document.createTextNode(`${rxG[i]}` + t('common_pkts'))); - td = tr.insertCell();td.appendChild(document.createTextNode(`${rxB[i]}` + t('common_pkts'))); - var button = ''; + td = tr.insertCell(); td.appendChild(document.createTextNode(`${txB[i]}` + t('common_pkts'))); + td = tr.insertCell(); td.appendChild(document.createTextNode(`${rxG[i]}` + t('common_pkts'))); + td = tr.insertCell(); td.appendChild(document.createTextNode(`${rxB[i]}` + t('common_pkts'))); + var button = ''; td = tr.insertCell(); td.innerHTML = button; } } @@ -197,8 +197,8 @@ window.addEventListener('click', (event) => { } }); -window.addEventListener("load", function() { - update( () => { +window.addEventListener("load", function () { + update(() => { update(); fillStats(); const stat = setInterval(fillStats, 1000); diff --git a/httpd/page_impl.c b/httpd/page_impl.c index 4bcf497..d55d4fe 100644 --- a/httpd/page_impl.c +++ b/httpd/page_impl.c @@ -290,11 +290,12 @@ void send_vlan(uint16_t vlan) } /* Send counters - * Only accepts physical port idx to 0-8. + * Only accepts physical port 1..9. * Returns an error if the port physical don't exists. */ -bool send_counters(uint8_t phys_port_idx) +bool send_counters(uint8_t phys_port) { + uint8_t phys_port_idx = phys_port - 1; if (phys_port_idx > 8) goto err; uint8_t log_port = machine.phys_to_log_port[phys_port_idx]; diff --git a/httpd/page_impl.h b/httpd/page_impl.h index 1cbd94e..02ed4eb 100644 --- a/httpd/page_impl.h +++ b/httpd/page_impl.h @@ -3,7 +3,7 @@ #include -bool send_counters(uint8_t phys_port_idx); +bool send_counters(uint8_t phys_port); void send_status(void); void send_vlan(uint16_t vlan); void send_basic_info(void); From 801150431b4ea0e3e3659fa61661834b2e88972d Mon Sep 17 00:00:00 2001 From: bloqaudio Date: Tue, 25 Aug 2026 09:28:54 -0500 Subject: [PATCH 36/36] stp: host the spanning tree module in BANK2 All entry points are already __banked and none of the code runs in interrupt context, so the module can leave the resident bank. This relieves pressure on bank 0, which no longer fits a machine with a custom init table. --- rtl837x_stp.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/rtl837x_stp.c b/rtl837x_stp.c index 2eb4f92..116c64c 100644 --- a/rtl837x_stp.c +++ b/rtl837x_stp.c @@ -14,6 +14,11 @@ #include "uip.h" #include "machine.h" +// All entry points are __banked and nothing here runs from an interrupt, +// so the module does not need to stay in the resident bank +#pragma codeseg BANK2 +#pragma constseg BANK2 + extern __code struct machine machine; extern __xdata uint8_t sfr_data[4];