Merge pull request #682 from hlyi/remove-custom-init

replacing machine_init.c with individual init c files

Makefile

@@ -15,11 +15,10 @@ SUBDIRS := tools
 SUBDIRSCLEAN=$(addsuffix clean,$(SUBDIRS))
 
 # $MACHINE selects machine/$(MACHINE).c as the board definition and is passed
-# on as -DMACHINE_$(MACHINE), which the rest of the firmware tests with
+# on as -DMACHINE_NAME, which machine.h uses to include machine/$(MACHINE).h.
-# #if defined(MACHINE_...), and as -DMACHINE_NAME, which machine.h uses to
+# Board selection itself needs no MACHINE_* symbol for #if chains, so the
-# include machine/$(MACHINE).h. Both take the same value, so they cannot drift.
+# definition is all the compiler has to know.
 ifneq ($(MACHINE),)
-	CC_FLAGS += -DMACHINE_$(MACHINE)
 	CC_FLAGS += -DMACHINE_NAME=$(MACHINE)
 endif
 # Health instrumentation and the "health" console command: HEALTH=1 gmake ...
@@ -34,6 +33,12 @@ endif
 # The board definition being built: one file per MACHINE_* symbol in machine.h.
 MACHINE_SRC = machine/$(MACHINE).c
 
+MACHINE_INIT_DEFAULT := machine/default_machine_init.c
+MACHINE_INIT_SRC := $(wildcard machine/$(MACHINE)_init.c)
+ifeq ($(MACHINE_INIT_SRC),)
+	MACHINE_INIT_SRC := $(MACHINE_INIT_DEFAULT)
+endif
+
 ifndef BUILDDIR
 	BUILDDIR = output/$(MACHINE)
 endif
@@ -87,7 +92,7 @@ $(MACHINE_SRC):
 # invalid $MACHINE value
 SRCS = \
 	$(MACHINE_SRC) \
-	machine_init.c \
+	$(MACHINE_INIT_SRC) \
 	cmd_editor.c \
 	cmd_parser.c \
 	dhcp.c \
@@ -209,21 +214,25 @@ $(BUILDDIR)/rtlplayground-$(FILENAME_EXTENSION).bin: $(BUILDDIR)/rtlplayground.i
 .PHONY: clean distclean all $(SUBDIRS) $(SUBDIRSCLEAN) create_build_dir
 
 # Every machine/ definition is compiled, so a machine added to machine/ is
-# covered without registering it anywhere. The machine-specific defines are
+# covered without registering it anywhere. The _init.c files are excluded here:
+# they are the boot hooks, compiled separately below (the board's own if it
+# exists, otherwise default_machine_init.c). The machine-specific defines are
 # filtered out of $(CC_FLAGS) and supplied per iteration instead, so this also
 # works as "make machine_check MACHINE=...".
-MACHINE_DEFS := $(sort $(patsubst machine/%.c,%,$(wildcard machine/*.c)))
+MACHINE_DEFS := $(sort $(patsubst machine/%.c,%,$(filter-out machine/%_init.c,$(wildcard machine/*.c))))
 MACHINE_LESS_CC_FLAGS = $(filter-out -DMACHINE_%,$(CC_FLAGS))
 
-.PHONY:
+.PHONY: machine_check
 machine_check:
 	@mkdir -p $(BUILDDIR)/tmp
 	@set -eo pipefail; \
 	for MACHINE in $(MACHINE_DEFS); \
 	do \
 	echo "Checking $${MACHINE}"; \
-	$(CC) $(MACHINE_LESS_CC_FLAGS) -DMACHINE_$${MACHINE} -DMACHINE_NAME=$${MACHINE} -MMD -o $(BUILDDIR)/tmp/machine_check -c machine/$${MACHINE}.c; \
+	$(CC) $(MACHINE_LESS_CC_FLAGS) -DMACHINE_NAME=$${MACHINE} -MMD -o $(BUILDDIR)/tmp/machine_check -c machine/$${MACHINE}.c; \
-	$(CC) $(MACHINE_LESS_CC_FLAGS) -DMACHINE_$${MACHINE} -DMACHINE_NAME=$${MACHINE} -MMD -o $(BUILDDIR)/tmp/machine_check -c machine_init.c; \
+	INIT=machine/$${MACHINE}_init.c; \
+	if [ ! -e $$INIT ] && [ ! -L $$INIT ]; then INIT=$(MACHINE_INIT_DEFAULT); fi; \
+	$(CC) $(MACHINE_LESS_CC_FLAGS) -DMACHINE_NAME=$${MACHINE} -MMD -o $(BUILDDIR)/tmp/machine_check -c $$INIT; \
 	done
 	@rm -rf $(BUILDDIR)/tmp
 

doc/adding-a-device.md

@@ -15,6 +15,7 @@ make MACHINE=<NAME>
 |------|---------|
 | `machine/<NAME>.c` | the `struct machine machine` board definition |
 | `machine/<NAME>.h` | `MACHINE_MODEL` (the model string shown in the UI and the CLI) and `MACHINE_CHIPSET` (`RTL8372` or `RTL8373`) |
+| `machine/<NAME>_init.c` | optional `machine_custom_init()` boot hook, for registers the machine definition cannot express |
 | `doc/devices/<name>.md` + `doc/supported_devices.md` | (only if the board is verified and you want to support it publicly) user-facing documentation |
 
 `<NAME>` is the machine symbol without its `MACHINE_` prefix, so `MACHINE` is
@@ -147,6 +148,20 @@ and the six `KP_9000_*` revision variants.
 Two files that differ only in whitespace or comments are duplicates and should
 be collapsed this way.
 
+### Optional custom init hook
+
+If the board needs custom initialization, add
+`machine/<NAME>_init.c` defining `machine_custom_init()`. The Makefile picks the
+file up by name and compiles it.
+
+Boards that need the same hook share one file: keep the code in a single
+`machine/<NAME>_init.c` whose `machine/<NAME>.c` is a real file (not a symlink),
+and make the other boards' `_init.c` symlinks to it, for example:
+
+```
+ln -s KP_9000_6XH_X2_init.c machine/MY_BOARD_1_0_init.c
+```
+
 ## Step 3: build and verify
 
 ```
@@ -160,7 +175,7 @@ file, before anything is compiled.
 Then run the checks CI runs:
 
 ```
-make machine_check    # compiles machine/<NAME>.c and machine_init.c for every machine
+make machine_check    # compiles machine/<NAME>.c and its optional _init.c for every machine
 make -C test          # host-side unit tests
 ```
 

machine/2G5F4_10G2_V1_01.c

@@ -41,8 +41,8 @@ __code const struct machine machine = {
 	 * at 2.5G only) and LED-ID2 carries the SFP single LED. The "solid"
 	 * conditions (LINK/speed) barely drive the LEDs here and ACT is what
 	 * actually lights them, which is why the white LED used to glow faintly at
-	 * non-2.5G speeds; the fix for that is LED_GLB_IO_EN bit 6, set in
+	 * non-2.5G speeds; the fix for that is LED_GLB_IO_EN bit 6, set in the
-	 * machine_init.c. */
+	 * shared machine_custom_init() hook (machine/2G5F4_10G2_V1_01_init.c). */
 	.led_sets = { { LEDS_2G5 | LEDS_1G | LEDS_100M | LEDS_10M | LEDS_LINK | LEDS_ACT | LEDS_10G,
 			LEDS_2G5 | LEDS_LINK | LEDS_10G,
 			LEDS_1G | LEDS_LINK,

machine/2G5F4_10G2_V1_01_init.c

@@ -0,0 +1 @@
+KP_9000_6XH_X2_init.c

machine/FG_4GT_2SX_V2_0_init.c

@@ -0,0 +1,6 @@
+#include "machine_init_defs.h"
+
+void machine_custom_init(void) __banked
+{
+	REG_SET(RTL837X_REG_LED_GLB_IO_EN, 0x7624155b);
+}

machine/FNS1200P_init.c

@@ -0,0 +1 @@
+KP_9000_6XH_X2_init.c

machine/HI_K0402WS_init.c

@@ -0,0 +1 @@
+KP_9000_6XH_X2_init.c

machine/KP_9000_6XHML_X2_V2_1_init.c

@@ -0,0 +1 @@
+KP_9000_6XH_X2_init.c

machine/KP_9000_6XH_X2_V2_1_init.c

@@ -0,0 +1 @@
+KP_9000_6XH_X2_init.c

machine/KP_9000_6XH_X2_init.c

@@ -0,0 +1,6 @@
+#include "machine_init_defs.h"
+
+void machine_custom_init(void) __banked
+{
+	reg_bit_set(RTL837X_REG_LED_GLB_IO_EN, 6);
+}

machine/PCB_K0402WS_V2_init.c

@@ -0,0 +1 @@
+KP_9000_6XH_X2_init.c

machine/PCB_K0402WS_V3_init.c

@@ -0,0 +1 @@
+KP_9000_6XH_X2_init.c

machine/PCB_SWTG018AS_V2_1_0_init.c

@@ -0,0 +1,18 @@
+#include "machine_init_defs.h"
+
+static __code const struct { uint16_t reg; uint32_t val; } custom_init_regs[] = {
+	{ 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
+{
+	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);
+	}
+}

machine/PCB_SWTG024AS_A_2_0_1_init.c

@@ -0,0 +1,9 @@
+#include "machine_init_defs.h"
+
+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);
+}

machine/SWTG024AS_A_2_0_1_5C_1SFP_init.c

@@ -0,0 +1 @@
+SWTG024AS_V2_0_init.c

machine/SWTG024AS_V2_0_init.c

@@ -0,0 +1,20 @@
+#include "machine_init_defs.h"
+
+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);
+}

machine/ZX310S_4T2XT_init.c

@@ -0,0 +1,8 @@
+#include "machine_init_defs.h"
+
+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);
+}

machine/default_machine_init.c

@@ -0,0 +1,6 @@
+/*
+ * Fallback boot hook for boards without a machine/<MACHINE>_init.c.
+ */
+#include "machine_init_defs.h"
+
+void machine_custom_init(void) __banked { }

machine/machine_init_defs.h

@@ -0,0 +1,10 @@
+#ifndef _MACHINE_INIT_DEFS_H_
+#define _MACHINE_INIT_DEFS_H_
+
+
+#include "machine_defs.h"
+
+#pragma codeseg BANK2
+#pragma constseg BANK2
+
+#endif

machine_init.c

@@ -1,96 +0,0 @@
-/*
- * Per-machine one-shot boot hooks, hosted in BANK2 so board-specific
- * tables and code do not consume the common bank.
- */
-#include <stdint.h>
-#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) || \
-	defined(MACHINE_KP_9000_6XH_X2_V2_1) || \
-	defined(MACHINE_KP_9000_6XHML_X2_V2_1) || \
-	defined(MACHINE_HI_K0402WS) || \
-	defined(MACHINE_PCB_K0402WS_V2) || \
-	defined(MACHINE_PCB_K0402WS_V3) || \
-	defined(MACHINE_2G5F4_10G2_V1_01) || \
-	defined(MACHINE_FNS1200P)
-void machine_custom_init(void) __banked
-{
-	reg_bit_set(RTL837X_REG_LED_GLB_IO_EN, 6);
-}
-
-#elif defined MACHINE_PCB_SWTG018AS_V2_1_0
-// 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[] = {
-	{ 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
-{
-	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_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) || \
-      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