OpenSBI compile-time C arrays

PATCH New Anup Patel 5 participants 29 messages May 3, 2022 – May 13, 2022

Subsystems (2)

Kbuild SiFive drivers

lore

Share & tools

lore

Anup Patel

author

2022-05-03 03:37 UTC #0 lore raw

cover OpenSBI compile-time C arrays

This series aims at removing hard-coded C arrays for drivers and modules in OpenSBI sources and replace it with dynamically generated arrays at compile-time. External firmwares which use OpenSBI as library will have to create these arrays separately because they typically don't use OpenSBI build system.

These patches can also be found in generated_carray_v1 branch at:
https://github.com/avpatel/opensbi.git

Anup Patel (11):
Makefile: Allow generated C source to be anywhere in build directory
Makefile: Add support for generating C array at compile time
lib: utils/reset: Generate FDT reset driver list at compile-time
lib: utils/serial: Generate FDT serial driver list at compile-time
lib: utils/timer: Generate FDT timer driver list at compile-time
lib: utils/irqchip: Generate FDT irqchip driver list at compile-time
lib: utils/ipi: Generate FDT ipi driver list at compile-time
lib: utils/i2c: Generate FDT i2c adapter driver list at compile-time
lib: utils/gpio: Generate FDT gpio driver list at compile-time
platform: generic: Generate platform override module list at
compile-time
platform: generic: Move Sifive platform overrides into own directory

 Makefile                                      | 17 +++-
 include/sbi_utils/gpio/fdt_gpio.h             |  2 +
 lib/utils/gpio/fdt_gpio.c                     | 18 ++---
 lib/utils/gpio/fdt_gpio_drivers.carray        |  3 +
 lib/utils/gpio/objects.mk                     |  4 +
 lib/utils/i2c/fdt_i2c.c                       | 14 ++--
 lib/utils/i2c/fdt_i2c_adapter_drivers.carray  |  3 +
 lib/utils/i2c/objects.mk                      |  4 +
 lib/utils/ipi/fdt_ipi.c                       | 12 ++-
 lib/utils/ipi/fdt_ipi_drivers.carray          |  3 +
 lib/utils/ipi/objects.mk                      |  4 +
 lib/utils/irqchip/fdt_irqchip.c               | 16 ++--
 lib/utils/irqchip/fdt_irqchip_drivers.carray  |  3 +
 lib/utils/irqchip/objects.mk                  |  8 ++
 lib/utils/reset/fdt_reset.c                   | 22 ++----
 lib/utils/reset/fdt_reset_drivers.carray      |  3 +
 lib/utils/reset/objects.mk                    | 12 +++
 lib/utils/serial/fdt_serial.c                 | 28 ++-----
 lib/utils/serial/fdt_serial_drivers.carray    |  3 +
 lib/utils/serial/objects.mk                   | 16 ++++
 lib/utils/timer/fdt_timer.c                   | 12 ++-
 lib/utils/timer/fdt_timer_drivers.carray      |  3 +
 lib/utils/timer/objects.mk                    |  4 +
 platform/generic/objects.mk                   |  3 +-
 platform/generic/platform.c                   | 14 ++--
 .../generic/platform_override_modules.carray  |  3 +
 .../{sifive_fu540.c => sifive/fu540.c}        |  0
 .../{sifive_fu740.c => sifive/fu740.c}        |  0
 platform/generic/sifive/objects.mk            |  9 +++
 scripts/carray.sh                             | 77 +++++++++++++++++++
 30 files changed, 224 insertions(+), 96 deletions(-)
 create mode 100644 lib/utils/gpio/fdt_gpio_drivers.carray
 create mode 100644 lib/utils/i2c/fdt_i2c_adapter_drivers.carray
 create mode 100644 lib/utils/ipi/fdt_ipi_drivers.carray
 create mode 100644 lib/utils/irqchip/fdt_irqchip_drivers.carray
 create mode 100644 lib/utils/reset/fdt_reset_drivers.carray
 create mode 100644 lib/utils/serial/fdt_serial_drivers.carray
 create mode 100644 lib/utils/timer/fdt_timer_drivers.carray
 create mode 100644 platform/generic/platform_override_modules.carray
 rename platform/generic/{sifive_fu540.c => sifive/fu540.c} (100%)
 rename platform/generic/{sifive_fu740.c => sifive/fu740.c} (100%)
 create mode 100644 platform/generic/sifive/objects.mk
 create mode 100755 scripts/carray.sh

Signature · 1 line

2.34.1

Anup Patel

author

2022-05-03 03:37 UTC #1 lore raw

PATCH 1/11 Makefile: Allow generated C source to be anywhere in build directory

The generated C source could be anywhere within build directory so let us update the make rule to comple generated C source accordingly.

Signed-off-by Anup Patel <[email protected]>

---
 Makefile | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Makefile +3 -3

index fc1ea13..30f7c46 100644
@@ -437,6 +437,9 @@ $(build_dir)/%.dep: $(src_dir)/%.c
 $(build_dir)/%.o: $(src_dir)/%.c
 	$(call compile_cc,$@,$<)
 
+$(build_dir)/%.o: $(build_dir)/%.c
+	$(call compile_cc,$@,$<)
+
 ifeq ($(BUILD_INFO),y)
 $(build_dir)/lib/sbi/sbi_init.o: $(libsbi_dir)/sbi_init.c FORCE
 	$(call compile_cc,$@,$<)

@@ -463,9 +466,6 @@ $(platform_build_dir)/%.dep: $(platform_src_dir)/%.c
 $(platform_build_dir)/%.o: $(platform_src_dir)/%.c
 	$(call compile_cc,$@,$<)
 
-$(platform_build_dir)/%.o: $(platform_build_dir)/%.c
-	$(call compile_cc,$@,$<)
-
 $(platform_build_dir)/%.dep: $(platform_src_dir)/%.S
 	$(call compile_as_dep,$@,$<)

Signature · 1 line

2.34.1

Atish Patra

2022-05-09 17:24 UTC #2 lore raw

PATCH 1/11 Makefile: Allow generated C source to be anywhere in build directory

Quoted 34 lines · Anup Patel

The generated C source could be anywhere within build directory so let us update the make rule to comple generated C source accordingly.

Signed-off-by Anup Patel <[email protected]>

---
 Makefile | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Makefile +3 -4

index fc1ea13..30f7c46 100644
@@ -437,6 +437,9 @@ $(build_dir)/%.dep: $(src_dir)/%.c
 $(build_dir)/%.o: $(src_dir)/%.c
        $(call compile_cc,$@,$<)
 
+$(build_dir)/%.o: $(build_dir)/%.c
+       $(call compile_cc,$@,$<)
+
 ifeq ($(BUILD_INFO),y)
 $(build_dir)/lib/sbi/sbi_init.o: $(libsbi_dir)/sbi_init.c FORCE
        $(call compile_cc,$@,$<)

@@ -463,9 +466,6 @@ $(platform_build_dir)/%.dep: $(platform_src_dir)/%.c
 $(platform_build_dir)/%.o: $(platform_src_dir)/%.c
        $(call compile_cc,$@,$<)
 
-$(platform_build_dir)/%.o: $(platform_build_dir)/%.c
-       $(call compile_cc,$@,$<)
-
 $(platform_build_dir)/%.dep: $(platform_src_dir)/%.S
        $(call compile_as_dep,$@,$<)
 
--

Quoted 2 lines

2.34.1

Reviewed-by Atish Patra <[email protected]>

Signature · 2 lines

Regards,
Atish

Anup Patel

author

2022-05-03 03:37 UTC #3 lore raw

PATCH 2/11 Makefile: Add support for generating C array at compile time

In reply to Anup Patel

Generating C array at compile time based on details provided by objects.mk is a very useful feature which will help us compile only a subset of drivers or modules.

We add a bash script (carray.sh) which takes array details and object/variable list from command-line to generate a C source containing array of object/variable pointers. We also extend top-level makefile to use carray.sh whenever specified through objects.mk.

Signed-off-by Anup Patel <[email protected]>

---
 Makefile          | 11 +++++++
 scripts/carray.sh | 77 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 88 insertions(+)
 create mode 100755 scripts/carray.sh

Makefile +11 -0

index 30f7c46..f619ef7 100644
@@ -404,6 +404,10 @@ compile_d2c = $(CMD_PREFIX)mkdir -p `dirname $(1)`; \
 	     $(if $($(2)-varprefix-$(3)),$(eval D2C_NAME_PREFIX := $($(2)-varprefix-$(3))),$(eval D2C_NAME_PREFIX := $(5))) \
 	     $(if $($(2)-padding-$(3)),$(eval D2C_PADDING_BYTES := $($(2)-padding-$(3))),$(eval D2C_PADDING_BYTES := 0)) \
 	     $(src_dir)/scripts/d2c.sh -i $(6) -a $(D2C_ALIGN_BYTES) -p $(D2C_NAME_PREFIX) -t $(D2C_PADDING_BYTES) > $(1)
+compile_carray = $(CMD_PREFIX)mkdir -p `dirname $(1)`; \
+	     echo " CARRAY    $(subst $(build_dir)/,,$(1))"; \
+	     $(eval CARRAY_VAR_LIST := $(carray-$(subst .c,,$(shell basename $(1)))-y)) \
+	     $(src_dir)/scripts/carray.sh -i $(2) -l "$(CARRAY_VAR_LIST)" > $(1)
 compile_gen_dep = $(CMD_PREFIX)mkdir -p `dirname $(1)`; \
 	     echo " GEN-DEP   $(subst $(build_dir)/,,$(1))"; \
 	     echo "$(1:.dep=$(2)): $(3)" >> $(1)

@@ -451,6 +455,13 @@ $(build_dir)/%.dep: $(src_dir)/%.S
 $(build_dir)/%.o: $(src_dir)/%.S
 	$(call compile_as,$@,$<)
 
+$(build_dir)/%.dep: $(src_dir)/%.carray
+	$(call compile_gen_dep,$@,.c,$<)
+	$(call compile_gen_dep,$@,.o,$(@:.dep=.c))
+
+$(build_dir)/%.c: $(src_dir)/%.carray
+	$(call compile_carray,$@,$<)
+
 $(platform_build_dir)/%.bin: $(platform_build_dir)/%.elf
 	$(call compile_objcopy,$@,$<)

scripts/carray.sh new file +77 -0

77 lines not on this page · raw message

Signature · 1 line

2.34.1

Atish Patra

2022-05-09 17:26 UTC #4 lore raw

PATCH 2/11 Makefile: Add support for generating C array at compile time

Quoted 131 lines · Anup Patel

Generating C array at compile time based on details provided by objects.mk is a very useful feature which will help us compile only a subset of drivers or modules.

Signed-off-by Anup Patel <[email protected]>

---
 Makefile          | 11 +++++++
 scripts/carray.sh | 77 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 88 insertions(+)
 create mode 100755 scripts/carray.sh

Makefile +11 -0

index 30f7c46..f619ef7 100644
@@ -404,6 +404,10 @@ compile_d2c = $(CMD_PREFIX)mkdir -p `dirname $(1)`; \
             $(if $($(2)-varprefix-$(3)),$(eval D2C_NAME_PREFIX := $($(2)-varprefix-$(3))),$(eval D2C_NAME_PREFIX := $(5))) \
             $(if $($(2)-padding-$(3)),$(eval D2C_PADDING_BYTES := $($(2)-padding-$(3))),$(eval D2C_PADDING_BYTES := 0)) \
             $(src_dir)/scripts/d2c.sh -i $(6) -a $(D2C_ALIGN_BYTES) -p $(D2C_NAME_PREFIX) -t $(D2C_PADDING_BYTES) > $(1)
+compile_carray = $(CMD_PREFIX)mkdir -p `dirname $(1)`; \
+            echo " CARRAY    $(subst $(build_dir)/,,$(1))"; \
+            $(eval CARRAY_VAR_LIST := $(carray-$(subst .c,,$(shell basename $(1)))-y)) \
+            $(src_dir)/scripts/carray.sh -i $(2) -l "$(CARRAY_VAR_LIST)" > $(1)
 compile_gen_dep = $(CMD_PREFIX)mkdir -p `dirname $(1)`; \
             echo " GEN-DEP   $(subst $(build_dir)/,,$(1))"; \
             echo "$(1:.dep=$(2)): $(3)" >> $(1)

@@ -451,6 +455,13 @@ $(build_dir)/%.dep: $(src_dir)/%.S
 $(build_dir)/%.o: $(src_dir)/%.S
        $(call compile_as,$@,$<)
 
+$(build_dir)/%.dep: $(src_dir)/%.carray
+       $(call compile_gen_dep,$@,.c,$<)
+       $(call compile_gen_dep,$@,.o,$(@:.dep=.c))
+
+$(build_dir)/%.c: $(src_dir)/%.carray
+       $(call compile_carray,$@,$<)
+
 $(platform_build_dir)/%.bin: $(platform_build_dir)/%.elf
        $(call compile_objcopy,$@,$<)

scripts/carray.sh new file +77 -1

new file mode 100755
index 0000000..0c52bd6
@@ -0,0 +1,77 @@
+#!/bin/bash
+
+function usage()
+{
+       echo "Usage:"
+       echo " $0 [options]"
+       echo "Options:"
+       echo "     -h                   Display help or usage"
+       echo "     -i <input_config>    Input config file"
+       echo "     -l <variable_list>   List of variables in the array (Optional)"
+       exit 1;
+}
+
+# Command line options
+CONFIG_FILE=""
+VAR_LIST=""
+
+while getopts "hi:l:" o; do
+       case "${o}" in
+       h)
+               usage
+               ;;
+       i)
+               CONFIG_FILE=${OPTARG}
+               ;;
+       l)
+               VAR_LIST=${OPTARG}
+               ;;
+       *)
+               usage
+               ;;
+       esac
+done
+shift $((OPTIND-1))
+
+if [ -z "${CONFIG_FILE}" ]; then
+       echo "Must specify input config file"
+       usage
+fi
+
+if [ ! -f "${CONFIG_FILE}" ]; then
+       echo "The input config file should be a present"
+       usage
+fi
+
+TYPE_HEADER=`cat ${CONFIG_FILE} | awk '{ if ($1 == "HEADER:") { printf $2; exit 0; } }'`
+if [ -z "${TYPE_HEADER}" ]; then
+       echo "Must specify HEADER: in input config file"
+       usage
+fi
+
+TYPE_NAME=`cat ${CONFIG_FILE} | awk '{ if ($1 == "TYPE:") { printf $2; for (i=3; i<=NF; i++) printf " %s", $i; exit 0; } }'`
+if [ -z "${TYPE_NAME}" ]; then
+       echo "Must specify TYPE: in input config file"
+       usage
+fi
+
+ARRAY_NAME=`cat ${CONFIG_FILE} | awk '{ if ($1 == "NAME:") { printf $2; exit 0; } }'`
+if [ -z "${ARRAY_NAME}" ]; then
+       echo "Must specify NAME: in input config file"
+       usage
+fi
+
+printf "#include <%s>\n\n" "${TYPE_HEADER}"
+
+for VAR in ${VAR_LIST}; do
+       printf "extern %s %s;\n" "${TYPE_NAME}" "${VAR}"
+done
+printf "\n"
+
+printf "%s *%s[] = {\n" "${TYPE_NAME}" "${ARRAY_NAME}"
+for VAR in ${VAR_LIST}; do
+       printf "\t&%s,\n" "${VAR}"
+done
+printf "};\n\n"
+
+printf "unsigned long %s_size = sizeof(%s) / sizeof(%s *);\n" "${ARRAY_NAME}" "${ARRAY_NAME}" "${TYPE_NAME}"
--

Quoted 2 lines

2.34.1

Reviewed-by Atish Patra <[email protected]>

Signature · 2 lines

Regards,
Atish

Anup Patel

author

2022-05-03 03:37 UTC #5 lore raw

PATCH 3/11 lib: utils/reset: Generate FDT reset driver list at compile-time

In reply to Anup Patel

Instead of having FDT reset driver list hard-coded in the C source, we generate it using carray.sh at compile-time.

Signed-off-by Anup Patel <[email protected]>

---
 lib/utils/reset/fdt_reset.c              | 22 +++++-----------------
 lib/utils/reset/fdt_reset_drivers.carray |  3 +++
 lib/utils/reset/objects.mk               | 12 ++++++++++++
 3 files changed, 20 insertions(+), 17 deletions(-)
 create mode 100644 lib/utils/reset/fdt_reset_drivers.carray

lib/utils/reset/fdt_reset.c +5 ~~-17~~

index 66281cb..4334586 100644
@@ -13,21 +13,9 @@
 #include <sbi_utils/fdt/fdt_helper.h>
 #include <sbi_utils/reset/fdt_reset.h>
 
-extern struct fdt_reset fdt_poweroff_gpio;
-extern struct fdt_reset fdt_reset_gpio;
-extern struct fdt_reset fdt_reset_htif;
-extern struct fdt_reset fdt_reset_sifive_test;
-extern struct fdt_reset fdt_reset_sunxi_wdt;
-extern struct fdt_reset fdt_reset_thead;
-
-static struct fdt_reset *reset_drivers[] = {
-	&fdt_poweroff_gpio,
-	&fdt_reset_gpio,
-	&fdt_reset_htif,
-	&fdt_reset_sifive_test,
-	&fdt_reset_sunxi_wdt,
-	&fdt_reset_thead,
-};
+/* List of FDT reset drivers generated at compile time */
+extern struct fdt_reset *fdt_reset_drivers[];
+extern unsigned long fdt_reset_drivers_size;
 
 int fdt_reset_driver_init(void *fdt, struct fdt_reset *drv)
 {

@@ -54,6 +42,6 @@ void fdt_reset_init(void)
 	int pos;
 	void *fdt = fdt_get_address();
 
-	for (pos = 0; pos < array_size(reset_drivers); pos++)
-		fdt_reset_driver_init(fdt, reset_drivers[pos]);
+	for (pos = 0; pos < fdt_reset_drivers_size; pos++)
+		fdt_reset_driver_init(fdt, fdt_reset_drivers[pos]);
 }

lib/utils/reset/fdt_reset_drivers.carray new file +3 -0

new file mode 100644
index 0000000..6ff799c
@@ -0,0 +1,3 @@
+HEADER: sbi_utils/reset/fdt_reset.h
+TYPE: struct fdt_reset
+NAME: fdt_reset_drivers

lib/utils/reset/objects.mk +12 -0

index 6c95db3..8cddcdf 100644
@@ -8,9 +8,21 @@
 #
 
 libsbiutils-objs-y += reset/fdt_reset.o
+libsbiutils-objs-y += reset/fdt_reset_drivers.o
+
+carray-fdt_reset_drivers-y += fdt_poweroff_gpio
+carray-fdt_reset_drivers-y += fdt_reset_gpio
 libsbiutils-objs-y += reset/fdt_reset_gpio.o
+
+carray-fdt_reset_drivers-y += fdt_reset_htif
 libsbiutils-objs-y += reset/fdt_reset_htif.o
+
+carray-fdt_reset_drivers-y += fdt_reset_sifive_test
 libsbiutils-objs-y += reset/fdt_reset_sifive_test.o
+
+carray-fdt_reset_drivers-y += fdt_reset_sunxi_wdt
 libsbiutils-objs-y += reset/fdt_reset_sunxi_wdt.o
+
+carray-fdt_reset_drivers-y += fdt_reset_thead
 libsbiutils-objs-y += reset/fdt_reset_thead.o
 libsbiutils-objs-y += reset/fdt_reset_thead_asm.o

Signature · 1 line

2.34.1

Anup Patel

author

2022-05-03 03:37 UTC #6 lore raw

PATCH 4/11 lib: utils/serial: Generate FDT serial driver list at compile-time

In reply to Anup Patel

Instead of having FDT serial driver list hard-coded in the C source, we generate it using carray.sh at compile-time.

Signed-off-by Anup Patel <[email protected]>

---
 lib/utils/serial/fdt_serial.c              | 28 ++++++----------------
 lib/utils/serial/fdt_serial_drivers.carray |  3 +++
 lib/utils/serial/objects.mk                | 16 +++++++++++++
 3 files changed, 26 insertions(+), 21 deletions(-)
 create mode 100644 lib/utils/serial/fdt_serial_drivers.carray

lib/utils/serial/fdt_serial.c +7 ~~-21~~

index 45aea4a..1a4bf9e 100644
@@ -13,23 +13,9 @@
 #include <sbi_utils/fdt/fdt_helper.h>
 #include <sbi_utils/serial/fdt_serial.h>
 
-extern struct fdt_serial fdt_serial_uart8250;
-extern struct fdt_serial fdt_serial_sifive;
-extern struct fdt_serial fdt_serial_litex;
-extern struct fdt_serial fdt_serial_htif;
-extern struct fdt_serial fdt_serial_shakti;
-extern struct fdt_serial fdt_serial_gaisler;
-extern struct fdt_serial fdt_serial_xlnx_uartlite;
-
-static struct fdt_serial *serial_drivers[] = {
-	&fdt_serial_uart8250,
-	&fdt_serial_sifive,
-	&fdt_serial_litex,
-	&fdt_serial_htif,
-	&fdt_serial_shakti,
-	&fdt_serial_gaisler,
-	&fdt_serial_xlnx_uartlite,
-};
+/* List of FDT serial drivers generated at compile time */
+extern struct fdt_serial *fdt_serial_drivers[];
+extern unsigned long fdt_serial_drivers_size;
 
 static struct fdt_serial dummy = {
 	.match_table = NULL,

@@ -64,8 +50,8 @@ int fdt_serial_init(void)
 	}
 
 	/* First check DT node pointed by stdout-path */
-	for (pos = 0; pos < array_size(serial_drivers) && -1 < noff; pos++) {
-		drv = serial_drivers[pos];
+	for (pos = 0; pos < fdt_serial_drivers_size && -1 < noff; pos++) {
+		drv = fdt_serial_drivers[pos];
 
 		match = fdt_match_node(fdt, noff, drv->match_table);
 		if (!match)

@@ -87,8 +73,8 @@ int fdt_serial_init(void)
 		goto done;
 
 	/* Lastly check all DT nodes */
-	for (pos = 0; pos < array_size(serial_drivers); pos++) {
-		drv = serial_drivers[pos];
+	for (pos = 0; pos < fdt_serial_drivers_size; pos++) {
+		drv = fdt_serial_drivers[pos];
 
 		noff = fdt_find_match(fdt, -1, drv->match_table, &match);
 		if (noff < 0)

lib/utils/serial/fdt_serial_drivers.carray new file +3 -0

new file mode 100644
index 0000000..3517b2c
@@ -0,0 +1,3 @@
+HEADER: sbi_utils/serial/fdt_serial.h
+TYPE: struct fdt_serial
+NAME: fdt_serial_drivers

lib/utils/serial/objects.mk +16 -0

index 4fb9627..d26a74e 100644
@@ -8,13 +8,29 @@
 #
 
 libsbiutils-objs-y += serial/fdt_serial.o
+libsbiutils-objs-y += serial/fdt_serial_drivers.o
+
+carray-fdt_serial_drivers-y += fdt_serial_gaisler
 libsbiutils-objs-y += serial/fdt_serial_gaisler.o
+
+carray-fdt_serial_drivers-y += fdt_serial_htif
 libsbiutils-objs-y += serial/fdt_serial_htif.o
+
+carray-fdt_serial_drivers-y += fdt_serial_shakti
 libsbiutils-objs-y += serial/fdt_serial_shakti.o
+
+carray-fdt_serial_drivers-y += fdt_serial_sifive
 libsbiutils-objs-y += serial/fdt_serial_sifive.o
+
+carray-fdt_serial_drivers-y += fdt_serial_litex
 libsbiutils-objs-y += serial/fdt_serial_litex.o
+
+carray-fdt_serial_drivers-y += fdt_serial_uart8250
 libsbiutils-objs-y += serial/fdt_serial_uart8250.o
+
+carray-fdt_serial_drivers-y += fdt_serial_xlnx_uartlite
 libsbiutils-objs-y += serial/fdt_serial_xlnx_uartlite.o
+
 libsbiutils-objs-y += serial/gaisler-uart.o
 libsbiutils-objs-y += serial/shakti-uart.o
 libsbiutils-objs-y += serial/sifive-uart.o

Signature · 1 line

2.34.1

Anup Patel

author

2022-05-03 03:37 UTC #7 lore raw

PATCH 5/11 lib: utils/timer: Generate FDT timer driver list at compile-time

In reply to Anup Patel

Instead of having FDT timer driver list hard-coded in the C source, we generate it using carray.sh at compile-time.

Signed-off-by Anup Patel <[email protected]>

---
 lib/utils/timer/fdt_timer.c              | 12 +++++-------
 lib/utils/timer/fdt_timer_drivers.carray |  3 +++
 lib/utils/timer/objects.mk               |  4 ++++
 3 files changed, 12 insertions(+), 7 deletions(-)
 create mode 100644 lib/utils/timer/fdt_timer_drivers.carray

lib/utils/timer/fdt_timer.c +5 -7

index 781bb63..4695c0f 100644
@@ -12,11 +12,9 @@
 #include <sbi_utils/fdt/fdt_helper.h>
 #include <sbi_utils/timer/fdt_timer.h>
 
-extern struct fdt_timer fdt_timer_mtimer;
-
-static struct fdt_timer *timer_drivers[] = {
-	&fdt_timer_mtimer
-};
+/* List of FDT timer drivers generated at compile time */
+extern struct fdt_timer *fdt_timer_drivers[];
+extern unsigned long fdt_timer_drivers_size;
 
 static struct fdt_timer dummy = {
 	.match_table = NULL,

@@ -47,8 +45,8 @@ static int fdt_timer_cold_init(void)
 	const struct fdt_match *match;
 	void *fdt = fdt_get_address();
 
-	for (pos = 0; pos < array_size(timer_drivers); pos++) {
-		drv = timer_drivers[pos];
+	for (pos = 0; pos < fdt_timer_drivers_size; pos++) {
+		drv = fdt_timer_drivers[pos];
 
 		noff = -1;
 		while ((noff = fdt_find_match(fdt, noff,

lib/utils/timer/fdt_timer_drivers.carray new file +3 -0

new file mode 100644
index 0000000..c62ee73
@@ -0,0 +1,3 @@
+HEADER: sbi_utils/timer/fdt_timer.h
+TYPE: struct fdt_timer
+NAME: fdt_timer_drivers

lib/utils/timer/objects.mk +4 -0

index 12cffcf..bc4073d 100644
@@ -8,5 +8,9 @@
 #
 
 libsbiutils-objs-y += timer/aclint_mtimer.o
+
 libsbiutils-objs-y += timer/fdt_timer.o
+libsbiutils-objs-y += timer/fdt_timer_drivers.o
+
+carray-fdt_timer_drivers-y += fdt_timer_mtimer
 libsbiutils-objs-y += timer/fdt_timer_mtimer.o

Signature · 1 line

2.34.1

Anup Patel

author

2022-05-03 03:37 UTC #8 lore raw

PATCH 6/11 lib: utils/irqchip: Generate FDT irqchip driver list at compile-time

In reply to Anup Patel

Instead of having FDT irqchip driver list hard-coded in the C source, we generate it using carray.sh at compile-time.

Signed-off-by Anup Patel <[email protected]>

---
 lib/utils/irqchip/fdt_irqchip.c              | 16 +++++-----------
 lib/utils/irqchip/fdt_irqchip_drivers.carray |  3 +++
 lib/utils/irqchip/objects.mk                 |  8 ++++++++
 3 files changed, 16 insertions(+), 11 deletions(-)
 create mode 100644 lib/utils/irqchip/fdt_irqchip_drivers.carray

lib/utils/irqchip/fdt_irqchip.c +5 ~~-11~~

index 6007755..1b6b674 100644
@@ -12,15 +12,9 @@
 #include <sbi_utils/fdt/fdt_helper.h>
 #include <sbi_utils/irqchip/fdt_irqchip.h>
 
-extern struct fdt_irqchip fdt_irqchip_aplic;
-extern struct fdt_irqchip fdt_irqchip_imsic;
-extern struct fdt_irqchip fdt_irqchip_plic;
-
-static struct fdt_irqchip *irqchip_drivers[] = {
-	&fdt_irqchip_aplic,
-	&fdt_irqchip_imsic,
-	&fdt_irqchip_plic
-};
+/* List of FDT irqchip drivers generated at compile time */
+extern struct fdt_irqchip *fdt_irqchip_drivers[];
+extern unsigned long fdt_irqchip_drivers_size;
 
 #define FDT_IRQCHIP_MAX_DRIVERS	8

@@ -61,8 +55,8 @@ static int fdt_irqchip_cold_init(void)
 	const struct fdt_match *match;
 	void *fdt = fdt_get_address();
 
-	for (pos = 0; pos < array_size(irqchip_drivers); pos++) {
-		drv = irqchip_drivers[pos];
+	for (pos = 0; pos < fdt_irqchip_drivers_size; pos++) {
+		drv = fdt_irqchip_drivers[pos];
 
 		noff = -1;
 		drv_added = false;

lib/utils/irqchip/fdt_irqchip_drivers.carray new file +3 -0

new file mode 100644
index 0000000..b373be5
@@ -0,0 +1,3 @@
+HEADER: sbi_utils/irqchip/fdt_irqchip.h
+TYPE: struct fdt_irqchip
+NAME: fdt_irqchip_drivers

lib/utils/irqchip/objects.mk +8 -0

index fad4344..7775bc4 100644
@@ -8,9 +8,17 @@
 #
 
 libsbiutils-objs-y += irqchip/fdt_irqchip.o
+libsbiutils-objs-y += irqchip/fdt_irqchip_drivers.o
+
+carray-fdt_irqchip_drivers-y += fdt_irqchip_aplic
 libsbiutils-objs-y += irqchip/fdt_irqchip_aplic.o
+
+carray-fdt_irqchip_drivers-y += fdt_irqchip_imsic
 libsbiutils-objs-y += irqchip/fdt_irqchip_imsic.o
+
+carray-fdt_irqchip_drivers-y += fdt_irqchip_plic
 libsbiutils-objs-y += irqchip/fdt_irqchip_plic.o
+
 libsbiutils-objs-y += irqchip/aplic.o
 libsbiutils-objs-y += irqchip/imsic.o
 libsbiutils-objs-y += irqchip/plic.o

Signature · 1 line

2.34.1

Anup Patel

author

2022-05-03 03:37 UTC #9 lore raw

PATCH 7/11 lib: utils/ipi: Generate FDT ipi driver list at compile-time

In reply to Anup Patel

Instead of having FDT ipi driver list hard-coded in the C source, we generate it using carray.sh at compile-time.

Signed-off-by Anup Patel <[email protected]>

---
 lib/utils/ipi/fdt_ipi.c              | 12 +++++-------
 lib/utils/ipi/fdt_ipi_drivers.carray |  3 +++
 lib/utils/ipi/objects.mk             |  4 ++++
 3 files changed, 12 insertions(+), 7 deletions(-)
 create mode 100644 lib/utils/ipi/fdt_ipi_drivers.carray

lib/utils/ipi/fdt_ipi.c +5 -7

index 91f116f..66dc510 100644
@@ -12,11 +12,9 @@
 #include <sbi_utils/fdt/fdt_helper.h>
 #include <sbi_utils/ipi/fdt_ipi.h>
 
-extern struct fdt_ipi fdt_ipi_mswi;
-
-static struct fdt_ipi *ipi_drivers[] = {
-	&fdt_ipi_mswi
-};
+/* List of FDT ipi drivers generated at compile time */
+extern struct fdt_ipi *fdt_ipi_drivers[];
+extern unsigned long fdt_ipi_drivers_size;
 
 static struct fdt_ipi dummy = {
 	.match_table = NULL,

@@ -47,8 +45,8 @@ static int fdt_ipi_cold_init(void)
 	const struct fdt_match *match;
 	void *fdt = fdt_get_address();
 
-	for (pos = 0; pos < array_size(ipi_drivers); pos++) {
-		drv = ipi_drivers[pos];
+	for (pos = 0; pos < fdt_ipi_drivers_size; pos++) {
+		drv = fdt_ipi_drivers[pos];
 
 		noff = -1;
 		while ((noff = fdt_find_match(fdt, noff,

lib/utils/ipi/fdt_ipi_drivers.carray new file +3 -0

new file mode 100644
index 0000000..275fa9c
@@ -0,0 +1,3 @@
+HEADER: sbi_utils/ipi/fdt_ipi.h
+TYPE: struct fdt_ipi
+NAME: fdt_ipi_drivers

lib/utils/ipi/objects.mk +4 -0

index 129eea8..0b0bc2d 100644
@@ -8,5 +8,9 @@
 #
 
 libsbiutils-objs-y += ipi/aclint_mswi.o
+
 libsbiutils-objs-y += ipi/fdt_ipi.o
+libsbiutils-objs-y += ipi/fdt_ipi_drivers.o
+
+carray-fdt_ipi_drivers-y += fdt_ipi_mswi
 libsbiutils-objs-y += ipi/fdt_ipi_mswi.o

Signature · 1 line

2.34.1

Anup Patel

author

2022-05-03 03:37 UTC #10 lore raw

PATCH 8/11 lib: utils/i2c: Generate FDT i2c adapter driver list at compile-time

In reply to Anup Patel

Instead of having FDT i2c adapter driver list hard-coded in the C source, we generate it using carray.sh at compile-time.

Signed-off-by Anup Patel <[email protected]>

---
 lib/utils/i2c/fdt_i2c.c                      | 14 +++++---------
 lib/utils/i2c/fdt_i2c_adapter_drivers.carray |  3 +++
 lib/utils/i2c/objects.mk                     |  4 ++++
 3 files changed, 12 insertions(+), 9 deletions(-)
 create mode 100644 lib/utils/i2c/fdt_i2c_adapter_drivers.carray

lib/utils/i2c/fdt_i2c.c +5 -9

index be8e506..6fd35dd 100644
@@ -16,13 +16,9 @@
 #include <sbi_utils/fdt/fdt_helper.h>
 #include <sbi_utils/i2c/fdt_i2c.h>
 
-#include <sbi/sbi_console.h>
-
-extern struct fdt_i2c_adapter fdt_i2c_adapter_sifive;
-
-static struct fdt_i2c_adapter *i2c_adapter_drivers[] = {
-	&fdt_i2c_adapter_sifive
-};
+/* List of FDT i2c adapter drivers generated at compile time */
+extern struct fdt_i2c_adapter *fdt_i2c_adapter_drivers[];
+extern unsigned long fdt_i2c_adapter_drivers_size;
 
 static int fdt_i2c_adapter_init(void *fdt, int nodeoff)
 {

@@ -31,8 +27,8 @@ static int fdt_i2c_adapter_init(void *fdt, int nodeoff)
 	const struct fdt_match *match;
 
 	/* Try all I2C drivers one-by-one */
-	for (pos = 0; pos < array_size(i2c_adapter_drivers); pos++) {
-		drv = i2c_adapter_drivers[pos];
+	for (pos = 0; pos < fdt_i2c_adapter_drivers_size; pos++) {
+		drv = fdt_i2c_adapter_drivers[pos];
 		match = fdt_match_node(fdt, nodeoff, drv->match_table);
 		if (match && drv->init) {
 			rc = drv->init(fdt, nodeoff, match);

lib/utils/i2c/fdt_i2c_adapter_drivers.carray new file +3 -0

new file mode 100644
index 0000000..fd51ae1
@@ -0,0 +1,3 @@
+HEADER: sbi_utils/i2c/fdt_i2c.h
+TYPE: struct fdt_i2c_adapter
+NAME: fdt_i2c_adapter_drivers

lib/utils/i2c/objects.mk +4 -0

index a748c48..bb0d6d5 100644
@@ -8,5 +8,9 @@
 #
 
 libsbiutils-objs-y += i2c/i2c.o
+
 libsbiutils-objs-y += i2c/fdt_i2c.o
+libsbiutils-objs-y += i2c/fdt_i2c_adapter_drivers.o
+
+carray-fdt_i2c_adapter_drivers-y += fdt_i2c_adapter_sifive
 libsbiutils-objs-y += i2c/fdt_i2c_sifive.o

Signature · 1 line

2.34.1

Anup Patel

author

2022-05-03 03:37 UTC #11 lore raw

PATCH 9/11 lib: utils/gpio: Generate FDT gpio driver list at compile-time

In reply to Anup Patel

Instead of having FDT gpio driver list hard-coded in the C source, we generate it using carray.sh at compile-time.

Signed-off-by Anup Patel <[email protected]>

---
 include/sbi_utils/gpio/fdt_gpio.h      |  2 ++
 lib/utils/gpio/fdt_gpio.c              | 18 ++++++++----------
 lib/utils/gpio/fdt_gpio_drivers.carray |  3 +++
 lib/utils/gpio/objects.mk              |  4 ++++
 4 files changed, 17 insertions(+), 10 deletions(-)
 create mode 100644 lib/utils/gpio/fdt_gpio_drivers.carray

include/sbi_utils/gpio/fdt_gpio.h +2 -0

index 19e1b58..ccbf2a1 100644
@@ -12,6 +12,8 @@
 
 #include <sbi_utils/gpio/gpio.h>
 
+struct fdt_phandle_args;
+
 /** FDT based GPIO driver */
 struct fdt_gpio {
 	const struct fdt_match *match_table;

lib/utils/gpio/fdt_gpio.c +8 ~~-10~~

index 297b248..7258128 100644
@@ -12,11 +12,9 @@
 #include <sbi_utils/fdt/fdt_helper.h>
 #include <sbi_utils/gpio/fdt_gpio.h>
 
-extern struct fdt_gpio fdt_gpio_sifive;
-
-static struct fdt_gpio *gpio_drivers[] = {
-	&fdt_gpio_sifive
-};
+/* List of FDT gpio drivers generated at compile time */
+extern struct fdt_gpio *fdt_gpio_drivers[];
+extern unsigned long fdt_gpio_drivers_size;
 
 static struct fdt_gpio *fdt_gpio_driver(struct gpio_chip *chip)
 {

@@ -25,9 +23,9 @@ static struct fdt_gpio *fdt_gpio_driver(struct gpio_chip *chip)
 	if (!chip)
 		return NULL;
 
-	for (pos = 0; pos < array_size(gpio_drivers); pos++) {
-		if (chip->driver == gpio_drivers[pos])
-			return gpio_drivers[pos];
+	for (pos = 0; pos < fdt_gpio_drivers_size; pos++) {
+		if (chip->driver == fdt_gpio_drivers[pos])
+			return fdt_gpio_drivers[pos];
 	}
 
 	return NULL;

@@ -49,8 +47,8 @@ static int fdt_gpio_init(void *fdt, u32 phandle)
 		return SBI_EINVAL;
 
 	/* Try all GPIO drivers one-by-one */
-	for (pos = 0; pos < array_size(gpio_drivers); pos++) {
-		drv = gpio_drivers[pos];
+	for (pos = 0; pos < fdt_gpio_drivers_size; pos++) {
+		drv = fdt_gpio_drivers[pos];
 
 		match = fdt_match_node(fdt, nodeoff, drv->match_table);
 		if (match && drv->init) {

lib/utils/gpio/fdt_gpio_drivers.carray new file +3 -0

new file mode 100644
index 0000000..e863f1c
@@ -0,0 +1,3 @@
+HEADER: sbi_utils/gpio/fdt_gpio.h
+TYPE: struct fdt_gpio
+NAME: fdt_gpio_drivers

lib/utils/gpio/objects.mk +4 -0

index 8eb7736..a5e131b 100644
@@ -8,5 +8,9 @@
 #
 
 libsbiutils-objs-y += gpio/fdt_gpio.o
+libsbiutils-objs-y += gpio/fdt_gpio_drivers.o
+
+carray-fdt_gpio_drivers-y += fdt_gpio_sifive
 libsbiutils-objs-y += gpio/fdt_gpio_sifive.o
+
 libsbiutils-objs-y += gpio/gpio.o

Signature · 1 line

2.34.1

Anup Patel

author

2022-05-03 03:37 UTC #12 lore raw

PATCH 10/11 platform: generic: Generate platform override module list at compile-time

In reply to Anup Patel

Instead of having platform override module list hard-coded in the C source, we generate it using carray.sh at compile-time.

Signed-off-by Anup Patel <[email protected]>

---
 platform/generic/objects.mk                       |  5 +++++
 platform/generic/platform.c                       | 14 +++++---------
 platform/generic/platform_override_modules.carray |  3 +++
 3 files changed, 13 insertions(+), 9 deletions(-)
 create mode 100644 platform/generic/platform_override_modules.carray

platform/generic/objects.mk +5 -0

index cb15a18..6f63e69 100644
@@ -8,5 +8,10 @@
 #
 
 platform-objs-y += platform.o
+platform-objs-y += platform_override_modules.o
+
+carray-platform_override_modules-y += sifive_fu540
 platform-objs-y += sifive_fu540.o
+
+carray-platform_override_modules-y += sifive_fu740
 platform-objs-y += sifive_fu740.o

platform/generic/platform.c +5 -9

index 8a4fb70..35c5ee4 100644
@@ -24,13 +24,9 @@
 #include <sbi_utils/ipi/fdt_ipi.h>
 #include <sbi_utils/reset/fdt_reset.h>
 
-extern const struct platform_override sifive_fu540;
-extern const struct platform_override sifive_fu740;
-
-static const struct platform_override *special_platforms[] = {
-	&sifive_fu540,
-	&sifive_fu740,
-};
+/* List of platform override modules generated at compile time */
+extern const struct platform_override *platform_override_modules[];
+extern unsigned long platform_override_modules_size;
 
 static const struct platform_override *generic_plat = NULL;
 static const struct fdt_match *generic_plat_match = NULL;

@@ -41,8 +37,8 @@ static void fw_platform_lookup_special(void *fdt, int root_offset)
 	const struct platform_override *plat;
 	const struct fdt_match *match;
 
-	for (pos = 0; pos < array_size(special_platforms); pos++) {
-		plat = special_platforms[pos];
+	for (pos = 0; pos < platform_override_modules_size; pos++) {
+		plat = platform_override_modules[pos];
 		if (!plat->match_table)
 			continue;

platform/generic/platform_override_modules.carray new file +3 -0

new file mode 100644
index 0000000..ab9d47f
@@ -0,0 +1,3 @@
+HEADER: platform_override.h
+TYPE: const struct platform_override
+NAME: platform_override_modules

Signature · 1 line

2.34.1

Anup Patel

author

2022-05-03 03:37 UTC #13 lore raw

PATCH 11/11 platform: generic: Move Sifive platform overrides into own directory

In reply to Anup Patel

Let us move SiFive platform overrides for FU540 and FU740 into a separate directory so better maintainability. Other SoC vendors can also create their own directory under platform/generic.

Signed-off-by Anup Patel <[email protected]>

---
 platform/generic/objects.mk                         | 6 ------
 platform/generic/{sifive_fu540.c => sifive/fu540.c} | 0
 platform/generic/{sifive_fu740.c => sifive/fu740.c} | 0
 platform/generic/sifive/objects.mk                  | 9 +++++++++
 4 files changed, 9 insertions(+), 6 deletions(-)
 rename platform/generic/{sifive_fu540.c => sifive/fu540.c} (100%)
 rename platform/generic/{sifive_fu740.c => sifive/fu740.c} (100%)
 create mode 100644 platform/generic/sifive/objects.mk

platform/generic/objects.mk +0 -6

index 6f63e69..4907754 100644
@@ -9,9 +9,3 @@
 
 platform-objs-y += platform.o
 platform-objs-y += platform_override_modules.o
-
-carray-platform_override_modules-y += sifive_fu540
-platform-objs-y += sifive_fu540.o
-
-carray-platform_override_modules-y += sifive_fu740
-platform-objs-y += sifive_fu740.o

platform/generic/sifive/fu540.c renamed from platform/generic/sifive_fu540.c +0 -0

similarity index 100%
rename from platform/generic/sifive_fu540.c
rename to platform/generic/sifive/fu540.c

platform/generic/sifive/fu740.c renamed from platform/generic/sifive_fu740.c +0 -0

similarity index 100%
rename from platform/generic/sifive_fu740.c
rename to platform/generic/sifive/fu740.c

platform/generic/sifive/objects.mk new file +9 -0

new file mode 100644
index 0000000..c17e2df
@@ -0,0 +1,9 @@
+#
+# SPDX-License-Identifier: BSD-2-Clause
+#
+
+carray-platform_override_modules-y += sifive_fu540
+platform-objs-y += sifive/fu540.o
+
+carray-platform_override_modules-y += sifive_fu740
+platform-objs-y += sifive/fu740.o

Signature · 1 line

2.34.1

Atish Patra

2022-05-09 17:28 UTC #14 lore raw

PATCH 11/11 platform: generic: Move Sifive platform overrides into own directory

Quoted 54 lines · Anup Patel

Let us move SiFive platform overrides for FU540 and FU740 into a separate directory so better maintainability. Other SoC vendors can also create their own directory under platform/generic.

Signed-off-by Anup Patel <[email protected]>

---
 platform/generic/objects.mk                         | 6 ------
 platform/generic/{sifive_fu540.c => sifive/fu540.c} | 0
 platform/generic/{sifive_fu740.c => sifive/fu740.c} | 0
 platform/generic/sifive/objects.mk                  | 9 +++++++++
 4 files changed, 9 insertions(+), 6 deletions(-)
 rename platform/generic/{sifive_fu540.c => sifive/fu540.c} (100%)
 rename platform/generic/{sifive_fu740.c => sifive/fu740.c} (100%)
 create mode 100644 platform/generic/sifive/objects.mk

platform/generic/objects.mk +0 -6

index 6f63e69..4907754 100644
@@ -9,9 +9,3 @@
 
 platform-objs-y += platform.o
 platform-objs-y += platform_override_modules.o
-
-carray-platform_override_modules-y += sifive_fu540
-platform-objs-y += sifive_fu540.o
-
-carray-platform_override_modules-y += sifive_fu740
-platform-objs-y += sifive_fu740.o

platform/generic/sifive/fu540.c renamed from platform/generic/sifive_fu540.c +0 -0

similarity index 100%
rename from platform/generic/sifive_fu540.c
rename to platform/generic/sifive/fu540.c

platform/generic/sifive/fu740.c renamed from platform/generic/sifive_fu740.c +0 -0

similarity index 100%
rename from platform/generic/sifive_fu740.c
rename to platform/generic/sifive/fu740.c

platform/generic/sifive/objects.mk new file +9 -1

new file mode 100644
index 0000000..c17e2df
@@ -0,0 +1,9 @@
+#
+# SPDX-License-Identifier: BSD-2-Clause
+#
+
+carray-platform_override_modules-y += sifive_fu540
+platform-objs-y += sifive/fu540.o
+
+carray-platform_override_modules-y += sifive_fu740
+platform-objs-y += sifive/fu740.o
--

Quoted 2 lines

2.34.1

Reviewed-by Atish Patra <[email protected]>

Signature · 2 lines

Regards,
Atish

Jessica Clarke

2022-05-03 03:42 UTC #15 lore raw

cover OpenSBI compile-time C arrays

In reply to Anup Patel

Quoted 6 lines · Anup Patel

?This series aims at removing hard-coded C arrays for drivers and modules in OpenSBI sources and replace it with dynamically generated arrays at compile-time. External firmwares which use OpenSBI as library will have to create these arrays separately because they typically don't use OpenSBI build system.

Why not use linker sets? No scripts needed.

Jess

Quoted 69 lines

These patches can also be found in generated_carray_v1 branch at:
https://github.com/avpatel/opensbi.git
Anup Patel (11):
Makefile: Allow generated C source to be anywhere in build directory
Makefile: Add support for generating C array at compile time
lib: utils/reset: Generate FDT reset driver list at compile-time
lib: utils/serial: Generate FDT serial driver list at compile-time
lib: utils/timer: Generate FDT timer driver list at compile-time
lib: utils/irqchip: Generate FDT irqchip driver list at compile-time
lib: utils/ipi: Generate FDT ipi driver list at compile-time
lib: utils/i2c: Generate FDT i2c adapter driver list at compile-time
lib: utils/gpio: Generate FDT gpio driver list at compile-time
platform: generic: Generate platform override module list at
compile-time
platform: generic: Move Sifive platform overrides into own directory
Makefile                                      | 17 +++-
include/sbi_utils/gpio/fdt_gpio.h             |  2 +
lib/utils/gpio/fdt_gpio.c                     | 18 ++---
lib/utils/gpio/fdt_gpio_drivers.carray        |  3 +
lib/utils/gpio/objects.mk                     |  4 +
lib/utils/i2c/fdt_i2c.c                       | 14 ++--
lib/utils/i2c/fdt_i2c_adapter_drivers.carray  |  3 +
lib/utils/i2c/objects.mk                      |  4 +
lib/utils/ipi/fdt_ipi.c                       | 12 ++-
lib/utils/ipi/fdt_ipi_drivers.carray          |  3 +
lib/utils/ipi/objects.mk                      |  4 +
lib/utils/irqchip/fdt_irqchip.c               | 16 ++--
lib/utils/irqchip/fdt_irqchip_drivers.carray  |  3 +
lib/utils/irqchip/objects.mk                  |  8 ++
lib/utils/reset/fdt_reset.c                   | 22 ++----
lib/utils/reset/fdt_reset_drivers.carray      |  3 +
lib/utils/reset/objects.mk                    | 12 +++
lib/utils/serial/fdt_serial.c                 | 28 ++-----
lib/utils/serial/fdt_serial_drivers.carray    |  3 +
lib/utils/serial/objects.mk                   | 16 ++++
lib/utils/timer/fdt_timer.c                   | 12 ++-
lib/utils/timer/fdt_timer_drivers.carray      |  3 +
lib/utils/timer/objects.mk                    |  4 +
platform/generic/objects.mk                   |  3 +-
platform/generic/platform.c                   | 14 ++--
.../generic/platform_override_modules.carray  |  3 +
.../{sifive_fu540.c => sifive/fu540.c}        |  0
.../{sifive_fu740.c => sifive/fu740.c}        |  0
platform/generic/sifive/objects.mk            |  9 +++
scripts/carray.sh                             | 77 +++++++++++++++++++
30 files changed, 224 insertions(+), 96 deletions(-)
create mode 100644 lib/utils/gpio/fdt_gpio_drivers.carray create mode 100644 lib/utils/i2c/fdt_i2c_adapter_drivers.carray create mode 100644 lib/utils/ipi/fdt_ipi_drivers.carray create mode 100644 lib/utils/irqchip/fdt_irqchip_drivers.carray create mode 100644 lib/utils/reset/fdt_reset_drivers.carray create mode 100644 lib/utils/serial/fdt_serial_drivers.carray create mode 100644 lib/utils/timer/fdt_timer_drivers.carray create mode 100644 platform/generic/platform_override_modules.carray rename platform/generic/{sifive_fu540.c => sifive/fu540.c} (100%) rename platform/generic/{sifive_fu740.c => sifive/fu740.c} (100%) create mode 100644 platform/generic/sifive/objects.mk create mode 100755 scripts/carray.sh
Signature · 7 lines
2.34.1


-- 
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

Anup Patel

author

2022-05-03 03:52 UTC #16 lore raw

cover OpenSBI compile-time C arrays

Quoted 10 lines · Jessica Clarke

Quoted 6 lines · Anup Patel
?This series aims at removing hard-coded C arrays for drivers and modules in OpenSBI sources and replace it with dynamically generated arrays at compile-time. External firmwares which use OpenSBI as library will have to create these arrays separately because they typically don't use OpenSBI build system.
Why not use linker sets? No scripts needed.

The problem is we can't assume anything about the build system of external firmware which use OpenSBI as library.

Regards,
Anup

Quoted 72 lines

Jess
Quoted 69 lines
These patches can also be found in generated_carray_v1 branch at:
https://github.com/avpatel/opensbi.git
Anup Patel (11):
Makefile: Allow generated C source to be anywhere in build directory
Makefile: Add support for generating C array at compile time
lib: utils/reset: Generate FDT reset driver list at compile-time
lib: utils/serial: Generate FDT serial driver list at compile-time
lib: utils/timer: Generate FDT timer driver list at compile-time
lib: utils/irqchip: Generate FDT irqchip driver list at compile-time
lib: utils/ipi: Generate FDT ipi driver list at compile-time
lib: utils/i2c: Generate FDT i2c adapter driver list at compile-time
lib: utils/gpio: Generate FDT gpio driver list at compile-time
platform: generic: Generate platform override module list at
compile-time
platform: generic: Move Sifive platform overrides into own directory
Makefile                                      | 17 +++-
include/sbi_utils/gpio/fdt_gpio.h             |  2 +
lib/utils/gpio/fdt_gpio.c                     | 18 ++---
lib/utils/gpio/fdt_gpio_drivers.carray        |  3 +
lib/utils/gpio/objects.mk                     |  4 +
lib/utils/i2c/fdt_i2c.c                       | 14 ++--
lib/utils/i2c/fdt_i2c_adapter_drivers.carray  |  3 +
lib/utils/i2c/objects.mk                      |  4 +
lib/utils/ipi/fdt_ipi.c                       | 12 ++-
lib/utils/ipi/fdt_ipi_drivers.carray          |  3 +
lib/utils/ipi/objects.mk                      |  4 +
lib/utils/irqchip/fdt_irqchip.c               | 16 ++--
lib/utils/irqchip/fdt_irqchip_drivers.carray  |  3 +
lib/utils/irqchip/objects.mk                  |  8 ++
lib/utils/reset/fdt_reset.c                   | 22 ++----
lib/utils/reset/fdt_reset_drivers.carray      |  3 +
lib/utils/reset/objects.mk                    | 12 +++
lib/utils/serial/fdt_serial.c                 | 28 ++-----
lib/utils/serial/fdt_serial_drivers.carray    |  3 +
lib/utils/serial/objects.mk                   | 16 ++++
lib/utils/timer/fdt_timer.c                   | 12 ++-
lib/utils/timer/fdt_timer_drivers.carray      |  3 +
lib/utils/timer/objects.mk                    |  4 +
platform/generic/objects.mk                   |  3 +-
platform/generic/platform.c                   | 14 ++--
.../generic/platform_override_modules.carray  |  3 +
.../{sifive_fu540.c => sifive/fu540.c}        |  0
.../{sifive_fu740.c => sifive/fu740.c}        |  0
platform/generic/sifive/objects.mk            |  9 +++
scripts/carray.sh                             | 77 +++++++++++++++++++
30 files changed, 224 insertions(+), 96 deletions(-)
create mode 100644 lib/utils/gpio/fdt_gpio_drivers.carray create mode 100644 lib/utils/i2c/fdt_i2c_adapter_drivers.carray create mode 100644 lib/utils/ipi/fdt_ipi_drivers.carray create mode 100644 lib/utils/irqchip/fdt_irqchip_drivers.carray create mode 100644 lib/utils/reset/fdt_reset_drivers.carray create mode 100644 lib/utils/serial/fdt_serial_drivers.carray create mode 100644 lib/utils/timer/fdt_timer_drivers.carray create mode 100644 platform/generic/platform_override_modules.carray rename platform/generic/{sifive_fu540.c => sifive/fu540.c} (100%) rename platform/generic/{sifive_fu740.c => sifive/fu740.c} (100%) create mode 100644 platform/generic/sifive/objects.mk create mode 100755 scripts/carray.sh
--
2.34.1
--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

Jessica Clarke

2022-05-03 03:54 UTC #17 lore raw

cover OpenSBI compile-time C arrays

[resent cc?ing list?]

Quoted 11 lines · Anup Patel

Quoted 9 lines · Jessica Clarke
Quoted 8 lines · Anup Patel
?This series aims at removing hard-coded C arrays for drivers and modules in OpenSBI sources and replace it with dynamically generated arrays at compile-time. External firmwares which use OpenSBI as library will have to create these arrays separately because they typically don't use OpenSBI build system.

Quoted 4 lines

Quoted 1 line
Why not use linker sets? No scripts needed.
The problem is we can't assume anything about the build system of external firmware which use OpenSBI as library.

They require no build system support. Unlike your patch.

Jess

Quoted 75 lines

Regards,
Anup
Quoted 72 lines
Jess
Quoted 69 lines
These patches can also be found in generated_carray_v1 branch at:
https://github.com/avpatel/opensbi.git
Anup Patel (11):
Makefile: Allow generated C source to be anywhere in build directory
Makefile: Add support for generating C array at compile time
lib: utils/reset: Generate FDT reset driver list at compile-time
lib: utils/serial: Generate FDT serial driver list at compile-time
lib: utils/timer: Generate FDT timer driver list at compile-time
lib: utils/irqchip: Generate FDT irqchip driver list at compile-time
lib: utils/ipi: Generate FDT ipi driver list at compile-time
lib: utils/i2c: Generate FDT i2c adapter driver list at compile-time
lib: utils/gpio: Generate FDT gpio driver list at compile-time
platform: generic: Generate platform override module list at
compile-time
platform: generic: Move Sifive platform overrides into own directory
Makefile                                      | 17 +++-
include/sbi_utils/gpio/fdt_gpio.h             |  2 +
lib/utils/gpio/fdt_gpio.c                     | 18 ++---
lib/utils/gpio/fdt_gpio_drivers.carray        |  3 +
lib/utils/gpio/objects.mk                     |  4 +
lib/utils/i2c/fdt_i2c.c                       | 14 ++--
lib/utils/i2c/fdt_i2c_adapter_drivers.carray  |  3 +
lib/utils/i2c/objects.mk                      |  4 +
lib/utils/ipi/fdt_ipi.c                       | 12 ++-
lib/utils/ipi/fdt_ipi_drivers.carray          |  3 +
lib/utils/ipi/objects.mk                      |  4 +
lib/utils/irqchip/fdt_irqchip.c               | 16 ++--
lib/utils/irqchip/fdt_irqchip_drivers.carray  |  3 +
lib/utils/irqchip/objects.mk                  |  8 ++
lib/utils/reset/fdt_reset.c                   | 22 ++----
lib/utils/reset/fdt_reset_drivers.carray      |  3 +
lib/utils/reset/objects.mk                    | 12 +++
lib/utils/serial/fdt_serial.c                 | 28 ++-----
lib/utils/serial/fdt_serial_drivers.carray    |  3 +
lib/utils/serial/objects.mk                   | 16 ++++
lib/utils/timer/fdt_timer.c                   | 12 ++-
lib/utils/timer/fdt_timer_drivers.carray      |  3 +
lib/utils/timer/objects.mk                    |  4 +
platform/generic/objects.mk                   |  3 +-
platform/generic/platform.c                   | 14 ++--
.../generic/platform_override_modules.carray  |  3 +
.../{sifive_fu540.c => sifive/fu540.c}        |  0
.../{sifive_fu740.c => sifive/fu740.c}        |  0
platform/generic/sifive/objects.mk            |  9 +++
scripts/carray.sh                             | 77 +++++++++++++++++++
30 files changed, 224 insertions(+), 96 deletions(-)
create mode 100644 lib/utils/gpio/fdt_gpio_drivers.carray create mode 100644 lib/utils/i2c/fdt_i2c_adapter_drivers.carray create mode 100644 lib/utils/ipi/fdt_ipi_drivers.carray create mode 100644 lib/utils/irqchip/fdt_irqchip_drivers.carray create mode 100644 lib/utils/reset/fdt_reset_drivers.carray create mode 100644 lib/utils/serial/fdt_serial_drivers.carray create mode 100644 lib/utils/timer/fdt_timer_drivers.carray create mode 100644 platform/generic/platform_override_modules.carray rename platform/generic/{sifive_fu540.c => sifive/fu540.c} (100%) rename platform/generic/{sifive_fu740.c => sifive/fu740.c} (100%) create mode 100644 platform/generic/sifive/objects.mk create mode 100755 scripts/carray.sh
--
2.34.1
--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

Anup Patel

author

2022-05-03 04:04 UTC #18 lore raw

cover OpenSBI compile-time C arrays

Quoted 15 lines · Jessica Clarke

[resent cc?ing list?]
Quoted 11 lines · Anup Patel
Quoted 9 lines · Jessica Clarke
Quoted 8 lines · Anup Patel
?This series aims at removing hard-coded C arrays for drivers and modules in OpenSBI sources and replace it with dynamically generated arrays at compile-time. External firmwares which use OpenSBI as library will have to create these arrays separately because they typically don't use OpenSBI build system.

Quoted 7 lines

Quoted 4 lines
Quoted 1 line
Why not use linker sets? No scripts needed.
The problem is we can't assume anything about the build system of external firmware which use OpenSBI as library.
They require no build system support. Unlike your patch.

With this series, they don't need to change anything in their build system. For example:
1) If they want to use fdt_serial_uart8250.c then they directly use "struct fdt_serial fdt_serial_uart8250"
2) If they want to use fdt_serial.c then they can create custom "struct fdt_serial *fdt_serial_drivers[]" with their desired drivers

If we go with a separate linker section then external firmwares will also require a separate linker section. They will also need to take care of relocations and dynamic linking for this special linker section if the external firmware uses -fPIC.

Regards,
Anup

Quoted 78 lines

Jess
Quoted 75 lines
Regards,
Anup
Quoted 72 lines
Jess
Quoted 69 lines
These patches can also be found in generated_carray_v1 branch at:
https://github.com/avpatel/opensbi.git
Anup Patel (11):
Makefile: Allow generated C source to be anywhere in build directory
Makefile: Add support for generating C array at compile time
lib: utils/reset: Generate FDT reset driver list at compile-time
lib: utils/serial: Generate FDT serial driver list at compile-time
lib: utils/timer: Generate FDT timer driver list at compile-time
lib: utils/irqchip: Generate FDT irqchip driver list at compile-time
lib: utils/ipi: Generate FDT ipi driver list at compile-time
lib: utils/i2c: Generate FDT i2c adapter driver list at compile-time
lib: utils/gpio: Generate FDT gpio driver list at compile-time
platform: generic: Generate platform override module list at
compile-time
platform: generic: Move Sifive platform overrides into own directory
Makefile                                      | 17 +++-
include/sbi_utils/gpio/fdt_gpio.h             |  2 +
lib/utils/gpio/fdt_gpio.c                     | 18 ++---
lib/utils/gpio/fdt_gpio_drivers.carray        |  3 +
lib/utils/gpio/objects.mk                     |  4 +
lib/utils/i2c/fdt_i2c.c                       | 14 ++--
lib/utils/i2c/fdt_i2c_adapter_drivers.carray  |  3 +
lib/utils/i2c/objects.mk                      |  4 +
lib/utils/ipi/fdt_ipi.c                       | 12 ++-
lib/utils/ipi/fdt_ipi_drivers.carray          |  3 +
lib/utils/ipi/objects.mk                      |  4 +
lib/utils/irqchip/fdt_irqchip.c               | 16 ++--
lib/utils/irqchip/fdt_irqchip_drivers.carray  |  3 +
lib/utils/irqchip/objects.mk                  |  8 ++
lib/utils/reset/fdt_reset.c                   | 22 ++----
lib/utils/reset/fdt_reset_drivers.carray      |  3 +
lib/utils/reset/objects.mk                    | 12 +++
lib/utils/serial/fdt_serial.c                 | 28 ++-----
lib/utils/serial/fdt_serial_drivers.carray    |  3 +
lib/utils/serial/objects.mk                   | 16 ++++
lib/utils/timer/fdt_timer.c                   | 12 ++-
lib/utils/timer/fdt_timer_drivers.carray      |  3 +
lib/utils/timer/objects.mk                    |  4 +
platform/generic/objects.mk                   |  3 +-
platform/generic/platform.c                   | 14 ++--
.../generic/platform_override_modules.carray  |  3 +
.../{sifive_fu540.c => sifive/fu540.c}        |  0
.../{sifive_fu740.c => sifive/fu740.c}        |  0
platform/generic/sifive/objects.mk            |  9 +++
scripts/carray.sh                             | 77 +++++++++++++++++++
30 files changed, 224 insertions(+), 96 deletions(-)
create mode 100644 lib/utils/gpio/fdt_gpio_drivers.carray create mode 100644 lib/utils/i2c/fdt_i2c_adapter_drivers.carray create mode 100644 lib/utils/ipi/fdt_ipi_drivers.carray create mode 100644 lib/utils/irqchip/fdt_irqchip_drivers.carray create mode 100644 lib/utils/reset/fdt_reset_drivers.carray create mode 100644 lib/utils/serial/fdt_serial_drivers.carray create mode 100644 lib/utils/timer/fdt_timer_drivers.carray create mode 100644 platform/generic/platform_override_modules.carray rename platform/generic/{sifive_fu540.c => sifive/fu540.c} (100%) rename platform/generic/{sifive_fu740.c => sifive/fu740.c} (100%) create mode 100644 platform/generic/sifive/objects.mk create mode 100755 scripts/carray.sh
--
2.34.1
--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

Jessica Clarke

2022-05-03 04:13 UTC #19 lore raw

cover OpenSBI compile-time C arrays

Quoted 32 lines · Anup Patel

Quoted 15 lines · Jessica Clarke
[resent cc?ing list?]
Quoted 12 lines · Anup Patel
Quoted 9 lines · Jessica Clarke
Quoted 8 lines · Anup Patel
?This series aims at removing hard-coded C arrays for drivers and modules in OpenSBI sources and replace it with dynamically generated arrays at compile-time. External firmwares which use OpenSBI as library will have to create these arrays separately because they typically don't use OpenSBI build system.
Quoted 7 lines
Quoted 4 lines
Quoted 1 line
Why not use linker sets? No scripts needed.
The problem is we can't assume anything about the build system of external firmware which use OpenSBI as library.
They require no build system support. Unlike your patch.
With this series, they don't need to change anything in their build system. For example:
1) If they want to use fdt_serial_uart8250.c then they directly use "struct fdt_serial fdt_serial_uart8250"
2) If they want to use fdt_serial.c then they can create custom "struct fdt_serial *fdt_serial_drivers[]" with their desired drivers

Quoted 4 lines

If we go with a separate linker section then external firmwares will also require a separate linker section. They will also need to take care of relocations and dynamic linking for this special linker section if the external firmware uses -fPIC.

No they don?t. They just need to link in the object file like normal and it just works. Do you actually know what a linker set is and how it is used? Because it sounds like you haven?t encountered them before, or have and misunderstand them. They?re really quite convenient and perfect for this kind of thing. It?s what FreeBSD uses for all its drivers, and I assume Linux too.

Jess

Quoted 86 lines

Regards,
Anup
"
Quoted 78 lines
Jess
Quoted 75 lines
Regards,
Anup
Quoted 72 lines
Jess
Quoted 69 lines
These patches can also be found in generated_carray_v1 branch at:
https://github.com/avpatel/opensbi.git
Anup Patel (11):
Makefile: Allow generated C source to be anywhere in build directory
Makefile: Add support for generating C array at compile time
lib: utils/reset: Generate FDT reset driver list at compile-time
lib: utils/serial: Generate FDT serial driver list at compile-time
lib: utils/timer: Generate FDT timer driver list at compile-time
lib: utils/irqchip: Generate FDT irqchip driver list at compile-time
lib: utils/ipi: Generate FDT ipi driver list at compile-time
lib: utils/i2c: Generate FDT i2c adapter driver list at compile-time
lib: utils/gpio: Generate FDT gpio driver list at compile-time
platform: generic: Generate platform override module list at
compile-time
platform: generic: Move Sifive platform overrides into own directory
Makefile                                      | 17 +++-
include/sbi_utils/gpio/fdt_gpio.h             |  2 +
lib/utils/gpio/fdt_gpio.c                     | 18 ++---
lib/utils/gpio/fdt_gpio_drivers.carray        |  3 +
lib/utils/gpio/objects.mk                     |  4 +
lib/utils/i2c/fdt_i2c.c                       | 14 ++--
lib/utils/i2c/fdt_i2c_adapter_drivers.carray  |  3 +
lib/utils/i2c/objects.mk                      |  4 +
lib/utils/ipi/fdt_ipi.c                       | 12 ++-
lib/utils/ipi/fdt_ipi_drivers.carray          |  3 +
lib/utils/ipi/objects.mk                      |  4 +
lib/utils/irqchip/fdt_irqchip.c               | 16 ++--
lib/utils/irqchip/fdt_irqchip_drivers.carray  |  3 +
lib/utils/irqchip/objects.mk                  |  8 ++
lib/utils/reset/fdt_reset.c                   | 22 ++----
lib/utils/reset/fdt_reset_drivers.carray      |  3 +
lib/utils/reset/objects.mk                    | 12 +++
lib/utils/serial/fdt_serial.c                 | 28 ++-----
lib/utils/serial/fdt_serial_drivers.carray    |  3 +
lib/utils/serial/objects.mk                   | 16 ++++
lib/utils/timer/fdt_timer.c                   | 12 ++-
lib/utils/timer/fdt_timer_drivers.carray      |  3 +
lib/utils/timer/objects.mk                    |  4 +
platform/generic/objects.mk                   |  3 +-
platform/generic/platform.c                   | 14 ++--
.../generic/platform_override_modules.carray  |  3 +
.../{sifive_fu540.c => sifive/fu540.c}        |  0
.../{sifive_fu740.c => sifive/fu740.c}        |  0
platform/generic/sifive/objects.mk            |  9 +++
scripts/carray.sh                             | 77 +++++++++++++++++++
30 files changed, 224 insertions(+), 96 deletions(-)
create mode 100644 lib/utils/gpio/fdt_gpio_drivers.carray create mode 100644 lib/utils/i2c/fdt_i2c_adapter_drivers.carray create mode 100644 lib/utils/ipi/fdt_ipi_drivers.carray create mode 100644 lib/utils/irqchip/fdt_irqchip_drivers.carray create mode 100644 lib/utils/reset/fdt_reset_drivers.carray create mode 100644 lib/utils/serial/fdt_serial_drivers.carray create mode 100644 lib/utils/timer/fdt_timer_drivers.carray create mode 100644 platform/generic/platform_override_modules.carray rename platform/generic/{sifive_fu540.c => sifive/fu540.c} (100%) rename platform/generic/{sifive_fu740.c => sifive/fu740.c} (100%) create mode 100644 platform/generic/sifive/objects.mk create mode 100755 scripts/carray.sh
--
2.34.1
--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

Anup Patel

author

2022-05-03 04:17 UTC #20 lore raw

cover OpenSBI compile-time C arrays

Quoted 34 lines · Jessica Clarke

Quoted 32 lines · Anup Patel
Quoted 15 lines · Jessica Clarke
[resent cc?ing list?]
Quoted 12 lines · Anup Patel
Quoted 9 lines · Jessica Clarke
Quoted 8 lines · Anup Patel
?This series aims at removing hard-coded C arrays for drivers and modules in OpenSBI sources and replace it with dynamically generated arrays at compile-time. External firmwares which use OpenSBI as library will have to create these arrays separately because they typically don't use OpenSBI build system.
Quoted 7 lines
Quoted 4 lines
Quoted 1 line
Why not use linker sets? No scripts needed.
The problem is we can't assume anything about the build system of external firmware which use OpenSBI as library.
They require no build system support. Unlike your patch.
With this series, they don't need to change anything in their build system. For example:
1) If they want to use fdt_serial_uart8250.c then they directly use "struct fdt_serial fdt_serial_uart8250"
2) If they want to use fdt_serial.c then they can create custom "struct fdt_serial *fdt_serial_drivers[]" with their desired drivers

Quoted 6 lines

Quoted 4 lines
If we go with a separate linker section then external firmwares will also require a separate linker section. They will also need to take care of relocations and dynamic linking for this special linker section if the external firmware uses -fPIC.
No they don?t. They just need to link in the object file like normal and it just works. Do you actually know what a linker set is and how it is used? Because it sounds like you haven?t encountered them before, or have and misunderstand them. They?re really quite convenient and perfect for this kind of thing. It?s what FreeBSD uses for all its drivers, and I assume Linux too.

You are missing my point. The is about not breaking things for external firmware which use OpenSBI as library. It's not about whether we can add special linker section for OpenSBI firmwares.

You are welcome to send alternate patch series with special linker section.

Regards,
Anup

Quoted 89 lines

Jess
Quoted 86 lines
Regards,
Anup
"
Quoted 78 lines
Jess
Quoted 75 lines
Regards,
Anup
Quoted 72 lines
Jess
Quoted 69 lines
These patches can also be found in generated_carray_v1 branch at:
https://github.com/avpatel/opensbi.git
Anup Patel (11):
Makefile: Allow generated C source to be anywhere in build directory
Makefile: Add support for generating C array at compile time
lib: utils/reset: Generate FDT reset driver list at compile-time
lib: utils/serial: Generate FDT serial driver list at compile-time
lib: utils/timer: Generate FDT timer driver list at compile-time
lib: utils/irqchip: Generate FDT irqchip driver list at compile-time
lib: utils/ipi: Generate FDT ipi driver list at compile-time
lib: utils/i2c: Generate FDT i2c adapter driver list at compile-time
lib: utils/gpio: Generate FDT gpio driver list at compile-time
platform: generic: Generate platform override module list at
compile-time
platform: generic: Move Sifive platform overrides into own directory
Makefile                                      | 17 +++-
include/sbi_utils/gpio/fdt_gpio.h             |  2 +
lib/utils/gpio/fdt_gpio.c                     | 18 ++---
lib/utils/gpio/fdt_gpio_drivers.carray        |  3 +
lib/utils/gpio/objects.mk                     |  4 +
lib/utils/i2c/fdt_i2c.c                       | 14 ++--
lib/utils/i2c/fdt_i2c_adapter_drivers.carray  |  3 +
lib/utils/i2c/objects.mk                      |  4 +
lib/utils/ipi/fdt_ipi.c                       | 12 ++-
lib/utils/ipi/fdt_ipi_drivers.carray          |  3 +
lib/utils/ipi/objects.mk                      |  4 +
lib/utils/irqchip/fdt_irqchip.c               | 16 ++--
lib/utils/irqchip/fdt_irqchip_drivers.carray  |  3 +
lib/utils/irqchip/objects.mk                  |  8 ++
lib/utils/reset/fdt_reset.c                   | 22 ++----
lib/utils/reset/fdt_reset_drivers.carray      |  3 +
lib/utils/reset/objects.mk                    | 12 +++
lib/utils/serial/fdt_serial.c                 | 28 ++-----
lib/utils/serial/fdt_serial_drivers.carray    |  3 +
lib/utils/serial/objects.mk                   | 16 ++++
lib/utils/timer/fdt_timer.c                   | 12 ++-
lib/utils/timer/fdt_timer_drivers.carray      |  3 +
lib/utils/timer/objects.mk                    |  4 +
platform/generic/objects.mk                   |  3 +-
platform/generic/platform.c                   | 14 ++--
.../generic/platform_override_modules.carray  |  3 +
.../{sifive_fu540.c => sifive/fu540.c}        |  0
.../{sifive_fu740.c => sifive/fu740.c}        |  0
platform/generic/sifive/objects.mk            |  9 +++
scripts/carray.sh                             | 77 +++++++++++++++++++
30 files changed, 224 insertions(+), 96 deletions(-)
create mode 100644 lib/utils/gpio/fdt_gpio_drivers.carray create mode 100644 lib/utils/i2c/fdt_i2c_adapter_drivers.carray create mode 100644 lib/utils/ipi/fdt_ipi_drivers.carray create mode 100644 lib/utils/irqchip/fdt_irqchip_drivers.carray create mode 100644 lib/utils/reset/fdt_reset_drivers.carray create mode 100644 lib/utils/serial/fdt_serial_drivers.carray create mode 100644 lib/utils/timer/fdt_timer_drivers.carray create mode 100644 platform/generic/platform_override_modules.carray rename platform/generic/{sifive_fu540.c => sifive/fu540.c} (100%) rename platform/generic/{sifive_fu740.c => sifive/fu740.c} (100%) create mode 100644 platform/generic/sifive/objects.mk create mode 100755 scripts/carray.sh
--
2.34.1
--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

Jessica Clarke

2022-05-03 04:23 UTC #21 lore raw

cover OpenSBI compile-time C arrays

Quoted 41 lines · Anup Patel

Quoted 39 lines · Jessica Clarke
Quoted 36 lines · Anup Patel
Quoted 15 lines · Jessica Clarke
[resent cc?ing list?]
Quoted 12 lines · Anup Patel
Quoted 9 lines · Jessica Clarke
Quoted 8 lines · Anup Patel
?This series aims at removing hard-coded C arrays for drivers and modules in OpenSBI sources and replace it with dynamically generated arrays at compile-time. External firmwares which use OpenSBI as library will have to create these arrays separately because they typically don't use OpenSBI build system.
Quoted 7 lines
Quoted 4 lines
Quoted 1 line
Why not use linker sets? No scripts needed.
The problem is we can't assume anything about the build system of external firmware which use OpenSBI as library.
They require no build system support. Unlike your patch.
With this series, they don't need to change anything in their build system. For example:
1) If they want to use fdt_serial_uart8250.c then they directly use "struct fdt_serial fdt_serial_uart8250"
2) If they want to use fdt_serial.c then they can create custom "struct fdt_serial *fdt_serial_drivers[]" with their desired drivers
If we go with a separate linker section then external firmwares will also require a separate linker section. They will also need to take care of relocations and dynamic linking for this special linker section if the external firmware uses -fPIC.

Quoted 5 lines

Quoted 1 line
No they don?t. They just need to link in the object file like normal and it just works. Do you actually know what a linker set is and how it is used? Because it sounds like you haven?t encountered them before, or have and misunderstand them. They?re really quite convenient and perfect for this kind of thing. It?s what FreeBSD uses for all its drivers, and I assume Linux too.
You are missing my point. The is about not breaking things for external firmware which use OpenSBI as library. It's not about whether we can add special linker section for OpenSBI firmwares.

I don?t understand what the relevance of that is. It?s an implementation detail consumers, including of libsbi, don?t need to care about.

Quoted 2 lines

You are welcome to send alternate patch series with special linker section.

Sure, but that requires a lot more time that I don?t have than pointing out a better (in my opinion, at least) way to achieve your goals than your patch series. FreeBSD has a BSD-2-clause licensed header file at sys/sys/linker_set.h that provides macros for declaring, adding to and iterating over linker sets, for what it?s worth.

Jess

Quoted 92 lines

Regards,
Anup
Quoted 89 lines
Jess
Quoted 86 lines
Regards,
Anup
"
Quoted 78 lines
Jess
Quoted 75 lines
Regards,
Anup
Quoted 72 lines
Jess
Quoted 69 lines
These patches can also be found in generated_carray_v1 branch at:
https://github.com/avpatel/opensbi.git
Anup Patel (11):
Makefile: Allow generated C source to be anywhere in build directory
Makefile: Add support for generating C array at compile time
lib: utils/reset: Generate FDT reset driver list at compile-time
lib: utils/serial: Generate FDT serial driver list at compile-time
lib: utils/timer: Generate FDT timer driver list at compile-time
lib: utils/irqchip: Generate FDT irqchip driver list at compile-time
lib: utils/ipi: Generate FDT ipi driver list at compile-time
lib: utils/i2c: Generate FDT i2c adapter driver list at compile-time
lib: utils/gpio: Generate FDT gpio driver list at compile-time
platform: generic: Generate platform override module list at compile-time
platform: generic: Move Sifive platform overrides into own directory
Makefile | 17 +++- include/sbi_utils/gpio/fdt_gpio.h | 2 + lib/utils/gpio/fdt_gpio.c | 18 ++--- lib/utils/gpio/fdt_gpio_drivers.carray | 3 + lib/utils/gpio/objects.mk | 4 + lib/utils/i2c/fdt_i2c.c | 14 ++-- lib/utils/i2c/fdt_i2c_adapter_drivers.carray | 3 + lib/utils/i2c/objects.mk | 4 + lib/utils/ipi/fdt_ipi.c | 12 ++- lib/utils/ipi/fdt_ipi_drivers.carray | 3 + lib/utils/ipi/objects.mk | 4 + lib/utils/irqchip/fdt_irqchip.c | 16 ++-- lib/utils/irqchip/fdt_irqchip_drivers.carray | 3 + lib/utils/irqchip/objects.mk | 8 ++ lib/utils/reset/fdt_reset.c | 22 ++---- lib/utils/reset/fdt_reset_drivers.carray | 3 + lib/utils/reset/objects.mk | 12 +++ lib/utils/serial/fdt_serial.c | 28 ++----- lib/utils/serial/fdt_serial_drivers.carray | 3 + lib/utils/serial/objects.mk | 16 ++++ lib/utils/timer/fdt_timer.c | 12 ++- lib/utils/timer/fdt_timer_drivers.carray | 3 + lib/utils/timer/objects.mk | 4 + platform/generic/objects.mk | 3 +- platform/generic/platform.c | 14 ++-- .../generic/platform_override_modules.carray | 3 + .../{sifive_fu540.c => sifive/fu540.c} | 0 .../{sifive_fu740.c => sifive/fu740.c} | 0 platform/generic/sifive/objects.mk | 9 +++ scripts/carray.sh | 77 +++++++++++++++++++
30 files changed, 224 insertions(+), 96 deletions(-)
create mode 100644 lib/utils/gpio/fdt_gpio_drivers.carray create mode 100644 lib/utils/i2c/fdt_i2c_adapter_drivers.carray create mode 100644 lib/utils/ipi/fdt_ipi_drivers.carray create mode 100644 lib/utils/irqchip/fdt_irqchip_drivers.carray create mode 100644 lib/utils/reset/fdt_reset_drivers.carray create mode 100644 lib/utils/serial/fdt_serial_drivers.carray create mode 100644 lib/utils/timer/fdt_timer_drivers.carray create mode 100644 platform/generic/platform_override_modules.carray rename platform/generic/{sifive_fu540.c => sifive/fu540.c} (100%) rename platform/generic/{sifive_fu740.c => sifive/fu740.c} (100%) create mode 100644 platform/generic/sifive/objects.mk create mode 100755 scripts/carray.sh
--
2.34.1
--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

Anup Patel

2022-05-03 04:30 UTC #22 lore raw

cover OpenSBI compile-time C arrays

Quoted 45 lines · Jessica Clarke

Quoted 43 lines · Anup Patel
Quoted 40 lines · Jessica Clarke
Quoted 32 lines · Anup Patel
Quoted 15 lines · Jessica Clarke
[resent cc?ing list?]
Quoted 12 lines · Anup Patel
Quoted 9 lines · Jessica Clarke
Quoted 8 lines · Anup Patel
?This series aims at removing hard-coded C arrays for drivers and modules in OpenSBI sources and replace it with dynamically generated arrays at compile-time. External firmwares which use OpenSBI as library will have to create these arrays separately because they typically don't use OpenSBI build system.
Quoted 7 lines
Quoted 4 lines
Quoted 1 line
Why not use linker sets? No scripts needed.
The problem is we can't assume anything about the build system of external firmware which use OpenSBI as library.
They require no build system support. Unlike your patch.
With this series, they don't need to change anything in their build system. For example:
1) If they want to use fdt_serial_uart8250.c then they directly use "struct fdt_serial fdt_serial_uart8250"
2) If they want to use fdt_serial.c then they can create custom "struct fdt_serial *fdt_serial_drivers[]" with their desired drivers
Quoted 4 lines
If we go with a separate linker section then external firmwares will also require a separate linker section. They will also need to take care of relocations and dynamic linking for this special linker section if the external firmware uses -fPIC.
No they don?t. They just need to link in the object file like normal and it just works. Do you actually know what a linker set is and how it is used? Because it sounds like you haven?t encountered them before, or have and misunderstand them. They?re really quite convenient and perfect for this kind of thing. It?s what FreeBSD uses for all its drivers, and I assume Linux too.

Quoted 7 lines

Quoted 3 lines
You are missing my point. The is about not breaking things for external firmware which use OpenSBI as library. It's not about whether we can add special linker section for OpenSBI firmwares.
I don?t understand what the relevance of that is. It?s an implementation detail consumers, including of libsbi, don?t need to care about.

You don't care about breaking things for external (non-OpenSBI) firmwares but as maintainer I have to care about it.

External firmware use not just lib/sbi but they also use lib/utils which contain various drivers.

Quoted 9 lines

Quoted 3 lines
You are welcome to send alternate patch series with special linker section.
Sure, but that requires a lot more time that I don?t have than pointing out a better (in my opinion, at least) way to achieve your goals than your patch series. FreeBSD has a BSD-2-clause licensed header file at sys/sys/linker_set.h that provides macros for declaring, adding to and iterating over linker sets, for what it?s worth.

In past, I have written linker based modules and dynamic module loading from scratch in Xvisor. I am well aware how linker scripts work so be careful when making judgmental statements about me. Try to understand the rationale of any series before commenting. It is also amazing to see that you always have time to comment/argue on the mailing list but never have time to send patches to this project or other RISC-V specs.

Regards,
Anup

Quoted 96 lines

Jess
Quoted 92 lines
Regards,
Anup
Quoted 89 lines
Jess
Quoted 86 lines
Regards,
Anup
"
Quoted 78 lines
Jess
Quoted 75 lines
Regards,
Anup
Quoted 72 lines
Jess
Quoted 69 lines
These patches can also be found in generated_carray_v1 branch at:
https://github.com/avpatel/opensbi.git
Anup Patel (11):
Makefile: Allow generated C source to be anywhere in build directory
Makefile: Add support for generating C array at compile time
lib: utils/reset: Generate FDT reset driver list at compile-time
lib: utils/serial: Generate FDT serial driver list at compile-time
lib: utils/timer: Generate FDT timer driver list at compile-time
lib: utils/irqchip: Generate FDT irqchip driver list at compile-time
lib: utils/ipi: Generate FDT ipi driver list at compile-time
lib: utils/i2c: Generate FDT i2c adapter driver list at compile-time
lib: utils/gpio: Generate FDT gpio driver list at compile-time
platform: generic: Generate platform override module list at compile-time
platform: generic: Move Sifive platform overrides into own directory
Makefile | 17 +++- include/sbi_utils/gpio/fdt_gpio.h | 2 + lib/utils/gpio/fdt_gpio.c | 18 ++--- lib/utils/gpio/fdt_gpio_drivers.carray | 3 + lib/utils/gpio/objects.mk | 4 + lib/utils/i2c/fdt_i2c.c | 14 ++-- lib/utils/i2c/fdt_i2c_adapter_drivers.carray | 3 + lib/utils/i2c/objects.mk | 4 + lib/utils/ipi/fdt_ipi.c | 12 ++- lib/utils/ipi/fdt_ipi_drivers.carray | 3 + lib/utils/ipi/objects.mk | 4 + lib/utils/irqchip/fdt_irqchip.c | 16 ++-- lib/utils/irqchip/fdt_irqchip_drivers.carray | 3 + lib/utils/irqchip/objects.mk | 8 ++ lib/utils/reset/fdt_reset.c | 22 ++---- lib/utils/reset/fdt_reset_drivers.carray | 3 + lib/utils/reset/objects.mk | 12 +++ lib/utils/serial/fdt_serial.c | 28 ++----- lib/utils/serial/fdt_serial_drivers.carray | 3 + lib/utils/serial/objects.mk | 16 ++++ lib/utils/timer/fdt_timer.c | 12 ++- lib/utils/timer/fdt_timer_drivers.carray | 3 + lib/utils/timer/objects.mk | 4 + platform/generic/objects.mk | 3 +- platform/generic/platform.c | 14 ++-- .../generic/platform_override_modules.carray | 3 + .../{sifive_fu540.c => sifive/fu540.c} | 0 .../{sifive_fu740.c => sifive/fu740.c} | 0 platform/generic/sifive/objects.mk | 9 +++ scripts/carray.sh | 77 +++++++++++++++++++
30 files changed, 224 insertions(+), 96 deletions(-)
create mode 100644 lib/utils/gpio/fdt_gpio_drivers.carray create mode 100644 lib/utils/i2c/fdt_i2c_adapter_drivers.carray create mode 100644 lib/utils/ipi/fdt_ipi_drivers.carray create mode 100644 lib/utils/irqchip/fdt_irqchip_drivers.carray create mode 100644 lib/utils/reset/fdt_reset_drivers.carray create mode 100644 lib/utils/serial/fdt_serial_drivers.carray create mode 100644 lib/utils/timer/fdt_timer_drivers.carray create mode 100644 platform/generic/platform_override_modules.carray rename platform/generic/{sifive_fu540.c => sifive/fu540.c} (100%) rename platform/generic/{sifive_fu740.c => sifive/fu740.c} (100%) create mode 100644 platform/generic/sifive/objects.mk create mode 100755 scripts/carray.sh
--
2.34.1
--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

Jessica Clarke

2022-05-03 04:45 UTC #23 lore raw

cover OpenSBI compile-time C arrays

Quoted 51 lines · Anup Patel

Quoted 49 lines · Jessica Clarke
Quoted 46 lines · Anup Patel
Quoted 34 lines · Jessica Clarke
Quoted 32 lines · Anup Patel
Quoted 15 lines · Jessica Clarke
[resent cc?ing list?]
Quoted 12 lines · Anup Patel
Quoted 9 lines · Jessica Clarke
Quoted 8 lines · Anup Patel
?This series aims at removing hard-coded C arrays for drivers and modules in OpenSBI sources and replace it with dynamically generated arrays at compile-time. External firmwares which use OpenSBI as library will have to create these arrays separately because they typically don't use OpenSBI build system.
Quoted 7 lines
Quoted 4 lines
Quoted 1 line
Why not use linker sets? No scripts needed.
The problem is we can't assume anything about the build system of external firmware which use OpenSBI as library.
They require no build system support. Unlike your patch.
With this series, they don't need to change anything in their build system. For example:
1) If they want to use fdt_serial_uart8250.c then they directly use "struct fdt_serial fdt_serial_uart8250"
2) If they want to use fdt_serial.c then they can create custom "struct fdt_serial *fdt_serial_drivers[]" with their desired drivers
Quoted 6 lines
Quoted 4 lines
If we go with a separate linker section then external firmwares will also require a separate linker section. They will also need to take care of relocations and dynamic linking for this special linker section if the external firmware uses -fPIC.
No they don?t. They just need to link in the object file like normal and it just works. Do you actually know what a linker set is and how it is used? Because it sounds like you haven?t encountered them before, or have and misunderstand them. They?re really quite convenient and perfect for this kind of thing. It?s what FreeBSD uses for all its drivers, and I assume Linux too.
You are missing my point. The is about not breaking things for external firmware which use OpenSBI as library. It's not about whether we can add special linker section for OpenSBI firmwares.

Quoted 6 lines

Quoted 3 lines
I don?t understand what the relevance of that is. It?s an implementation detail consumers, including of libsbi, don?t need to care about.
You don't care about breaking things for external (non-OpenSBI) firmwares but as maintainer I have to care about it.

I do care about it and am not suggesting breaking things either. I just fail to understand what the breakage you see is; this is more disruptive in that it introduces new object files, whereas linker sets would not.

Quoted 2 lines

External firmware use not just lib/sbi but they also use lib/utils which contain various drivers.

I don?t see what difference this makes. With a linker set, anything that used fdt_serial.o for example would continue to function the same, just via a different implementation, and anything that manually used a specific driver would be unaffected as it wouldn?t use those code paths.

Quoted 9 lines

Quoted 8 lines
Quoted 2 lines
You are welcome to send alternate patch series with special linker section.
Sure, but that requires a lot more time that I don?t have than pointing out a better (in my opinion, at least) way to achieve your goals than your patch series. FreeBSD has a BSD-2-clause licensed header file at sys/sys/linker_set.h that provides macros for declaring, adding to and iterating over linker sets, for what it?s worth.

Quoted 7 lines

In past, I have written linker based modules and dynamic module loading from scratch in Xvisor. I am well aware how linker scripts work so be careful when making judgmental statements about me. Try to understand the rationale of any series before commenting. It is also amazing to see that you always have time to comment/argue on the mailing list but never have time to send patches to this project or other RISC-V specs.

Ok, I only asked because you seemed to be making factually inaccurate statements about how linker sets work and their requirements. Yes, I make more comments than patches for OpenSBI, it?s orders of magnitude quicker to do that, but ?never? is inaccurate, I have sent patches in the past to OpenSBI. I?m also RISC-V psABI co-chair so have my fair share of RISC-V spec contributions there. Not that that should matter in any way. So please let?s keep this respectful rather than resorting to personal attacks, whether accurate or (in this case) not.

Jess

Quoted 98 lines

Regards,
Anup
Quoted 95 lines
Jess
Quoted 92 lines
Regards,
Anup
Quoted 89 lines
Jess
Quoted 86 lines
Regards,
Anup
"
Quoted 78 lines
Jess
Quoted 75 lines
Regards,
Anup
Quoted 72 lines
Jess
Quoted 69 lines
These patches can also be found in generated_carray_v1 branch at:
https://github.com/avpatel/opensbi.git
Anup Patel (11):
Makefile: Allow generated C source to be anywhere in build directory
Makefile: Add support for generating C array at compile time
lib: utils/reset: Generate FDT reset driver list at compile-time
lib: utils/serial: Generate FDT serial driver list at compile-time
lib: utils/timer: Generate FDT timer driver list at compile-time
lib: utils/irqchip: Generate FDT irqchip driver list at compile-time
lib: utils/ipi: Generate FDT ipi driver list at compile-time
lib: utils/i2c: Generate FDT i2c adapter driver list at compile-time
lib: utils/gpio: Generate FDT gpio driver list at compile-time
platform: generic: Generate platform override module list at compile-time
platform: generic: Move Sifive platform overrides into own directory
Makefile | 17 +++- include/sbi_utils/gpio/fdt_gpio.h | 2 + lib/utils/gpio/fdt_gpio.c | 18 ++--- lib/utils/gpio/fdt_gpio_drivers.carray | 3 + lib/utils/gpio/objects.mk | 4 + lib/utils/i2c/fdt_i2c.c | 14 ++-- lib/utils/i2c/fdt_i2c_adapter_drivers.carray | 3 + lib/utils/i2c/objects.mk | 4 + lib/utils/ipi/fdt_ipi.c | 12 ++- lib/utils/ipi/fdt_ipi_drivers.carray | 3 + lib/utils/ipi/objects.mk | 4 + lib/utils/irqchip/fdt_irqchip.c | 16 ++-- lib/utils/irqchip/fdt_irqchip_drivers.carray | 3 + lib/utils/irqchip/objects.mk | 8 ++ lib/utils/reset/fdt_reset.c | 22 ++---- lib/utils/reset/fdt_reset_drivers.carray | 3 + lib/utils/reset/objects.mk | 12 +++ lib/utils/serial/fdt_serial.c | 28 ++----- lib/utils/serial/fdt_serial_drivers.carray | 3 + lib/utils/serial/objects.mk | 16 ++++ lib/utils/timer/fdt_timer.c | 12 ++- lib/utils/timer/fdt_timer_drivers.carray | 3 + lib/utils/timer/objects.mk | 4 + platform/generic/objects.mk | 3 +- platform/generic/platform.c | 14 ++-- .../generic/platform_override_modules.carray | 3 + .../{sifive_fu540.c => sifive/fu540.c} | 0 .../{sifive_fu740.c => sifive/fu740.c} | 0 platform/generic/sifive/objects.mk | 9 +++ scripts/carray.sh | 77 +++++++++++++++++++
30 files changed, 224 insertions(+), 96 deletions(-)
create mode 100644 lib/utils/gpio/fdt_gpio_drivers.carray create mode 100644 lib/utils/i2c/fdt_i2c_adapter_drivers.carray create mode 100644 lib/utils/ipi/fdt_ipi_drivers.carray create mode 100644 lib/utils/irqchip/fdt_irqchip_drivers.carray create mode 100644 lib/utils/reset/fdt_reset_drivers.carray create mode 100644 lib/utils/serial/fdt_serial_drivers.carray create mode 100644 lib/utils/timer/fdt_timer_drivers.carray create mode 100644 platform/generic/platform_override_modules.carray rename platform/generic/{sifive_fu540.c => sifive/fu540.c} (100%) rename platform/generic/{sifive_fu740.c => sifive/fu740.c} (100%) create mode 100644 platform/generic/sifive/objects.mk create mode 100755 scripts/carray.sh
--
2.34.1
--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

Anup Patel

author

2022-05-03 04:47 UTC #24 lore raw

cover OpenSBI compile-time C arrays

Quoted 90 lines · Jessica Clarke

Quoted 51 lines · Anup Patel
Quoted 49 lines · Jessica Clarke
Quoted 46 lines · Anup Patel
Quoted 34 lines · Jessica Clarke
Quoted 32 lines · Anup Patel
Quoted 15 lines · Jessica Clarke
[resent cc?ing list?]
Quoted 12 lines · Anup Patel
Quoted 9 lines · Jessica Clarke
Quoted 8 lines · Anup Patel
?This series aims at removing hard-coded C arrays for drivers and modules in OpenSBI sources and replace it with dynamically generated arrays at compile-time. External firmwares which use OpenSBI as library will have to create these arrays separately because they typically don't use OpenSBI build system.
Quoted 7 lines
Quoted 4 lines
Quoted 1 line
Why not use linker sets? No scripts needed.
The problem is we can't assume anything about the build system of external firmware which use OpenSBI as library.
They require no build system support. Unlike your patch.
With this series, they don't need to change anything in their build system. For example:
1) If they want to use fdt_serial_uart8250.c then they directly use "struct fdt_serial fdt_serial_uart8250"
2) If they want to use fdt_serial.c then they can create custom "struct fdt_serial *fdt_serial_drivers[]" with their desired drivers
Quoted 6 lines
Quoted 4 lines
If we go with a separate linker section then external firmwares will also require a separate linker section. They will also need to take care of relocations and dynamic linking for this special linker section if the external firmware uses -fPIC.
No they don?t. They just need to link in the object file like normal and it just works. Do you actually know what a linker set is and how it is used? Because it sounds like you haven?t encountered them before, or have and misunderstand them. They?re really quite convenient and perfect for this kind of thing. It?s what FreeBSD uses for all its drivers, and I assume Linux too.
You are missing my point. The is about not breaking things for external firmware which use OpenSBI as library. It's not about whether we can add special linker section for OpenSBI firmwares.
Quoted 6 lines
Quoted 3 lines
I don?t understand what the relevance of that is. It?s an implementation detail consumers, including of libsbi, don?t need to care about.
You don't care about breaking things for external (non-OpenSBI) firmwares but as maintainer I have to care about it.
I do care about it and am not suggesting breaking things either. I just fail to understand what the breakage you see is; this is more disruptive in that it introduces new object files, whereas linker sets would not.
Quoted 2 lines
External firmware use not just lib/sbi but they also use lib/utils which contain various drivers.
I don?t see what difference this makes. With a linker set, anything that used fdt_serial.o for example would continue to function the same, just via a different implementation, and anything that manually used a specific driver would be unaffected as it wouldn?t use those code paths.
Quoted 9 lines
Quoted 8 lines
Quoted 2 lines
You are welcome to send alternate patch series with special linker section.
Sure, but that requires a lot more time that I don?t have than pointing out a better (in my opinion, at least) way to achieve your goals than your patch series. FreeBSD has a BSD-2-clause licensed header file at sys/sys/linker_set.h that provides macros for declaring, adding to and iterating over linker sets, for what it?s worth.
Quoted 7 lines
In past, I have written linker based modules and dynamic module loading from scratch in Xvisor. I am well aware how linker scripts work so be careful when making judgmental statements about me. Try to understand the rationale of any series before commenting. It is also amazing to see that you always have time to comment/argue on the mailing list but never have time to send patches to this project or other RISC-V specs.

Quoted 8 lines

Ok, I only asked because you seemed to be making factually inaccurate statements about how linker sets work and their requirements. Yes, I make more comments than patches for OpenSBI, it?s orders of magnitude quicker to do that, but ?never? is inaccurate, I have sent patches in the past to OpenSBI. I?m also RISC-V psABI co-chair so have my fair share of RISC-V spec contributions there. Not that that should matter in any way. So please let?s keep this respectful rather than resorting to personal attacks, whether accurate or (in this case) not.

Same applies to you as well. Keep things respectful rather than making judgemental comments or personal attacks.

Regards,
Anup

Quoted 102 lines

Jess
Quoted 98 lines
Regards,
Anup
Quoted 95 lines
Jess
Quoted 92 lines
Regards,
Anup
Quoted 89 lines
Jess
Quoted 86 lines
Regards,
Anup
"
Quoted 78 lines
Jess
Quoted 75 lines
Regards,
Anup
Quoted 72 lines
Jess
Quoted 69 lines
These patches can also be found in generated_carray_v1 branch at:
https://github.com/avpatel/opensbi.git
Anup Patel (11):
Makefile: Allow generated C source to be anywhere in build directory
Makefile: Add support for generating C array at compile time
lib: utils/reset: Generate FDT reset driver list at compile-time
lib: utils/serial: Generate FDT serial driver list at compile-time
lib: utils/timer: Generate FDT timer driver list at compile-time
lib: utils/irqchip: Generate FDT irqchip driver list at compile-time
lib: utils/ipi: Generate FDT ipi driver list at compile-time
lib: utils/i2c: Generate FDT i2c adapter driver list at compile-time
lib: utils/gpio: Generate FDT gpio driver list at compile-time
platform: generic: Generate platform override module list at compile-time
platform: generic: Move Sifive platform overrides into own directory
Makefile | 17 +++- include/sbi_utils/gpio/fdt_gpio.h | 2 + lib/utils/gpio/fdt_gpio.c | 18 ++--- lib/utils/gpio/fdt_gpio_drivers.carray | 3 + lib/utils/gpio/objects.mk | 4 + lib/utils/i2c/fdt_i2c.c | 14 ++-- lib/utils/i2c/fdt_i2c_adapter_drivers.carray | 3 + lib/utils/i2c/objects.mk | 4 + lib/utils/ipi/fdt_ipi.c | 12 ++- lib/utils/ipi/fdt_ipi_drivers.carray | 3 + lib/utils/ipi/objects.mk | 4 + lib/utils/irqchip/fdt_irqchip.c | 16 ++-- lib/utils/irqchip/fdt_irqchip_drivers.carray | 3 + lib/utils/irqchip/objects.mk | 8 ++ lib/utils/reset/fdt_reset.c | 22 ++---- lib/utils/reset/fdt_reset_drivers.carray | 3 + lib/utils/reset/objects.mk | 12 +++ lib/utils/serial/fdt_serial.c | 28 ++----- lib/utils/serial/fdt_serial_drivers.carray | 3 + lib/utils/serial/objects.mk | 16 ++++ lib/utils/timer/fdt_timer.c | 12 ++- lib/utils/timer/fdt_timer_drivers.carray | 3 + lib/utils/timer/objects.mk | 4 + platform/generic/objects.mk | 3 +- platform/generic/platform.c | 14 ++-- .../generic/platform_override_modules.carray | 3 + .../{sifive_fu540.c => sifive/fu540.c} | 0 .../{sifive_fu740.c => sifive/fu740.c} | 0 platform/generic/sifive/objects.mk | 9 +++ scripts/carray.sh | 77 +++++++++++++++++++
30 files changed, 224 insertions(+), 96 deletions(-)
create mode 100644 lib/utils/gpio/fdt_gpio_drivers.carray create mode 100644 lib/utils/i2c/fdt_i2c_adapter_drivers.carray create mode 100644 lib/utils/ipi/fdt_ipi_drivers.carray create mode 100644 lib/utils/irqchip/fdt_irqchip_drivers.carray create mode 100644 lib/utils/reset/fdt_reset_drivers.carray create mode 100644 lib/utils/serial/fdt_serial_drivers.carray create mode 100644 lib/utils/timer/fdt_timer_drivers.carray create mode 100644 platform/generic/platform_override_modules.carray rename platform/generic/{sifive_fu540.c => sifive/fu540.c} (100%) rename platform/generic/{sifive_fu740.c => sifive/fu740.c} (100%) create mode 100644 platform/generic/sifive/objects.mk create mode 100755 scripts/carray.sh
--
2.34.1
--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

Xiang W

2022-05-03 15:35 UTC #25 lore raw

cover OpenSBI compile-time C arrays

In reply to Anup Patel

? 2022-05-03???? 09:07 +0530?Anup Patel???

Quoted 5 lines

This series aims at removing hard-coded C arrays for drivers and modules in OpenSBI sources and replace it with dynamically generated arrays at compile-time. External firmwares which use OpenSBI as library will have to create these arrays separately because they typically don't use OpenSBI build system.

The purpose of removing hardcoded arrays is to make it easier for users of the library to add their own drivers and modules? But this patch makes it necessary for end users to produce their own array of drivers and modules. We can implement this by simply putting these arrays into separate C files and distributing them with the library.

Regards,
Xiang W

Quoted 66 lines

These patches can also be found in generated_carray_v1 branch at:
https://github.com/avpatel/opensbi.git
Anup Patel (11):
? Makefile: Allow generated C source to be anywhere in build directory ? Makefile: Add support for generating C array at compile time ? lib: utils/reset: Generate FDT reset driver list at compile-time ? lib: utils/serial: Generate FDT serial driver list at compile-time ? lib: utils/timer: Generate FDT timer driver list at compile-time ? lib: utils/irqchip: Generate FDT irqchip driver list at compile-time ? lib: utils/ipi: Generate FDT ipi driver list at compile-time ? lib: utils/i2c: Generate FDT i2c adapter driver list at compile-time ? lib: utils/gpio: Generate FDT gpio driver list at compile-time ? platform: generic: Generate platform override module list at ??? compile-time
? platform: generic: Move Sifive platform overrides into own directory
?Makefile????????????????????????????????????? | 17 +++-
?include/sbi_utils/gpio/fdt_gpio.h???????????? |? 2 +
?lib/utils/gpio/fdt_gpio.c???????????????????? | 18 ++---
?lib/utils/gpio/fdt_gpio_drivers.carray??????? |? 3 +
?lib/utils/gpio/objects.mk???????????????????? |? 4 +
?lib/utils/i2c/fdt_i2c.c?????????????????????? | 14 ++--
?lib/utils/i2c/fdt_i2c_adapter_drivers.carray? |? 3 +
?lib/utils/i2c/objects.mk????????????????????? |? 4 +
?lib/utils/ipi/fdt_ipi.c?????????????????????? | 12 ++-
?lib/utils/ipi/fdt_ipi_drivers.carray????????? |? 3 +
?lib/utils/ipi/objects.mk????????????????????? |? 4 +
?lib/utils/irqchip/fdt_irqchip.c?????????????? | 16 ++--
?lib/utils/irqchip/fdt_irqchip_drivers.carray? |? 3 +
?lib/utils/irqchip/objects.mk????????????????? |? 8 ++
?lib/utils/reset/fdt_reset.c?????????????????? | 22 ++----
?lib/utils/reset/fdt_reset_drivers.carray????? |? 3 +
?lib/utils/reset/objects.mk??????????????????? | 12 +++
?lib/utils/serial/fdt_serial.c???????????????? | 28 ++-----
?lib/utils/serial/fdt_serial_drivers.carray??? |? 3 +
?lib/utils/serial/objects.mk?????????????????? | 16 ++++
?lib/utils/timer/fdt_timer.c?????????????????? | 12 ++-
?lib/utils/timer/fdt_timer_drivers.carray????? |? 3 +
?lib/utils/timer/objects.mk??????????????????? |? 4 +
?platform/generic/objects.mk?????????????????? |? 3 +-
?platform/generic/platform.c?????????????????? | 14 ++--
?.../generic/platform_override_modules.carray? |? 3 +
?.../{sifive_fu540.c => sifive/fu540.c}??????? |? 0
?.../{sifive_fu740.c => sifive/fu740.c}??????? |? 0
?platform/generic/sifive/objects.mk??????????? |? 9 +++
?scripts/carray.sh???????????????????????????? | 77 +++++++++++++++++++
?30 files changed, 224 insertions(+), 96 deletions(-) ?create mode 100644 lib/utils/gpio/fdt_gpio_drivers.carray ?create mode 100644 lib/utils/i2c/fdt_i2c_adapter_drivers.carray ?create mode 100644 lib/utils/ipi/fdt_ipi_drivers.carray ?create mode 100644 lib/utils/irqchip/fdt_irqchip_drivers.carray ?create mode 100644 lib/utils/reset/fdt_reset_drivers.carray ?create mode 100644 lib/utils/serial/fdt_serial_drivers.carray ?create mode 100644 lib/utils/timer/fdt_timer_drivers.carray ?create mode 100644 platform/generic/platform_override_modules.carray ?rename platform/generic/{sifive_fu540.c => sifive/fu540.c} (100%) ?rename platform/generic/{sifive_fu740.c => sifive/fu740.c} (100%) ?create mode 100644 platform/generic/sifive/objects.mk ?create mode 100755 scripts/carray.sh
Signature · 1 line
2.34.1

Anup Patel

author

2022-05-03 16:24 UTC #26 lore raw

cover OpenSBI compile-time C arrays

Quoted 8 lines · Xiang W

? 2022-05-03???? 09:07 +0530?Anup Patel???
Quoted 5 lines
This series aims at removing hard-coded C arrays for drivers and modules in OpenSBI sources and replace it with dynamically generated arrays at compile-time. External firmwares which use OpenSBI as library will have to create these arrays separately because they typically don't use OpenSBI build system.

Quoted 5 lines

The purpose of removing hardcoded arrays is to make it easier for users of the library to add their own drivers and modules? But this patch makes it necessary for end users to produce their own array of drivers and modules. We can implement this by simply putting these arrays into separate C files and distributing them with the library.

Only external firmware (which use OpenSBI as library) need to provide their own array of drivers. These external firmwares have their own build system and they generally compile only few required drivers or modules.

For all OpenSBI firmware users, these arrays are generated at compile time by the OpenSBI build system. Adding a new driver or module to OpenSBI build system is easy and only need changes in objects.mk.

Regards,
Anup

Quoted 77 lines

Regards,
Xiang W

Quoted 62 lines

These patches can also be found in generated_carray_v1 branch at:
https://github.com/avpatel/opensbi.git

 Makefile                                      | 17 +++-
 include/sbi_utils/gpio/fdt_gpio.h             |  2 +
 lib/utils/gpio/fdt_gpio.c                     | 18 ++---
 lib/utils/gpio/fdt_gpio_drivers.carray        |  3 +
 lib/utils/gpio/objects.mk                     |  4 +
 lib/utils/i2c/fdt_i2c.c                       | 14 ++--
 lib/utils/i2c/fdt_i2c_adapter_drivers.carray  |  3 +
 lib/utils/i2c/objects.mk                      |  4 +
 lib/utils/ipi/fdt_ipi.c                       | 12 ++-
 lib/utils/ipi/fdt_ipi_drivers.carray          |  3 +
 lib/utils/ipi/objects.mk                      |  4 +
 lib/utils/irqchip/fdt_irqchip.c               | 16 ++--
 lib/utils/irqchip/fdt_irqchip_drivers.carray  |  3 +
 lib/utils/irqchip/objects.mk                  |  8 ++
 lib/utils/reset/fdt_reset.c                   | 22 ++----
 lib/utils/reset/fdt_reset_drivers.carray      |  3 +
 lib/utils/reset/objects.mk                    | 12 +++
 lib/utils/serial/fdt_serial.c                 | 28 ++-----
 lib/utils/serial/fdt_serial_drivers.carray    |  3 +
 lib/utils/serial/objects.mk                   | 16 ++++
 lib/utils/timer/fdt_timer.c                   | 12 ++-
 lib/utils/timer/fdt_timer_drivers.carray      |  3 +
 lib/utils/timer/objects.mk                    |  4 +
 platform/generic/objects.mk                   |  3 +-
 platform/generic/platform.c                   | 14 ++--
 .../generic/platform_override_modules.carray  |  3 +
 .../{sifive_fu540.c => sifive/fu540.c}        |  0
 .../{sifive_fu740.c => sifive/fu740.c}        |  0
 platform/generic/sifive/objects.mk            |  9 +++
 scripts/carray.sh                             | 77 +++++++++++++++++++
 30 files changed, 224 insertions(+), 96 deletions(-)
 create mode 100644 lib/utils/gpio/fdt_gpio_drivers.carray
 create mode 100644 lib/utils/i2c/fdt_i2c_adapter_drivers.carray
 create mode 100644 lib/utils/ipi/fdt_ipi_drivers.carray
 create mode 100644 lib/utils/irqchip/fdt_irqchip_drivers.carray
 create mode 100644 lib/utils/reset/fdt_reset_drivers.carray
 create mode 100644 lib/utils/serial/fdt_serial_drivers.carray
 create mode 100644 lib/utils/timer/fdt_timer_drivers.carray
 create mode 100644 platform/generic/platform_override_modules.carray
 rename platform/generic/{sifive_fu540.c => sifive/fu540.c} (100%)
 rename platform/generic/{sifive_fu740.c => sifive/fu740.c} (100%)
 create mode 100644 platform/generic/sifive/objects.mk
 create mode 100755 scripts/carray.sh

Quoted 4 lines

--
2.34.1

--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

Atish Patra

2022-05-09 17:29 UTC #27 lore raw

cover OpenSBI compile-time C arrays

In reply to Anup Patel

Quoted 68 lines · Anup Patel

This series aims at removing hard-coded C arrays for drivers and modules in OpenSBI sources and replace it with dynamically generated arrays at compile-time. External firmwares which use OpenSBI as library will have to create these arrays separately because they typically don't use OpenSBI build system.
These patches can also be found in generated_carray_v1 branch at:
https://github.com/avpatel/opensbi.git
Anup Patel (11):
Makefile: Allow generated C source to be anywhere in build directory
Makefile: Add support for generating C array at compile time
lib: utils/reset: Generate FDT reset driver list at compile-time
lib: utils/serial: Generate FDT serial driver list at compile-time
lib: utils/timer: Generate FDT timer driver list at compile-time
lib: utils/irqchip: Generate FDT irqchip driver list at compile-time
lib: utils/ipi: Generate FDT ipi driver list at compile-time
lib: utils/i2c: Generate FDT i2c adapter driver list at compile-time
lib: utils/gpio: Generate FDT gpio driver list at compile-time
platform: generic: Generate platform override module list at
compile-time
platform: generic: Move Sifive platform overrides into own directory
 Makefile                                      | 17 +++-
 include/sbi_utils/gpio/fdt_gpio.h             |  2 +
 lib/utils/gpio/fdt_gpio.c                     | 18 ++---
 lib/utils/gpio/fdt_gpio_drivers.carray        |  3 +
 lib/utils/gpio/objects.mk                     |  4 +
 lib/utils/i2c/fdt_i2c.c                       | 14 ++--
 lib/utils/i2c/fdt_i2c_adapter_drivers.carray  |  3 +
 lib/utils/i2c/objects.mk                      |  4 +
 lib/utils/ipi/fdt_ipi.c                       | 12 ++-
 lib/utils/ipi/fdt_ipi_drivers.carray          |  3 +
 lib/utils/ipi/objects.mk                      |  4 +
 lib/utils/irqchip/fdt_irqchip.c               | 16 ++--
 lib/utils/irqchip/fdt_irqchip_drivers.carray  |  3 +
 lib/utils/irqchip/objects.mk                  |  8 ++
 lib/utils/reset/fdt_reset.c                   | 22 ++----
 lib/utils/reset/fdt_reset_drivers.carray      |  3 +
 lib/utils/reset/objects.mk                    | 12 +++
 lib/utils/serial/fdt_serial.c                 | 28 ++-----
 lib/utils/serial/fdt_serial_drivers.carray    |  3 +
 lib/utils/serial/objects.mk                   | 16 ++++
 lib/utils/timer/fdt_timer.c                   | 12 ++-
 lib/utils/timer/fdt_timer_drivers.carray      |  3 +
 lib/utils/timer/objects.mk                    |  4 +
 platform/generic/objects.mk                   |  3 +-
 platform/generic/platform.c                   | 14 ++--
 .../generic/platform_override_modules.carray  |  3 +
 .../{sifive_fu540.c => sifive/fu540.c}        |  0
 .../{sifive_fu740.c => sifive/fu740.c}        |  0
 platform/generic/sifive/objects.mk            |  9 +++
 scripts/carray.sh                             | 77 +++++++++++++++++++
 30 files changed, 224 insertions(+), 96 deletions(-)
 create mode 100644 lib/utils/gpio/fdt_gpio_drivers.carray
 create mode 100644 lib/utils/i2c/fdt_i2c_adapter_drivers.carray
 create mode 100644 lib/utils/ipi/fdt_ipi_drivers.carray
 create mode 100644 lib/utils/irqchip/fdt_irqchip_drivers.carray
 create mode 100644 lib/utils/reset/fdt_reset_drivers.carray
 create mode 100644 lib/utils/serial/fdt_serial_drivers.carray
 create mode 100644 lib/utils/timer/fdt_timer_drivers.carray
 create mode 100644 platform/generic/platform_override_modules.carray
 rename platform/generic/{sifive_fu540.c => sifive/fu540.c} (100%)
 rename platform/generic/{sifive_fu740.c => sifive/fu740.c} (100%)
 create mode 100644 platform/generic/sifive/objects.mk
 create mode 100755 scripts/carray.sh

Quoted 3 lines

--
2.34.1

LGTM. For the remaining patches,

Reviewed-by Atish Patra <[email protected]>

Signature · 2 lines

Regards,
Atish

Anup Patel

Applied

2022-05-13 04:03 UTC #28 lore raw

cover OpenSBI compile-time C arrays

Quoted 70 lines · Atish Patra

Quoted 68 lines · Anup Patel
This series aims at removing hard-coded C arrays for drivers and modules in OpenSBI sources and replace it with dynamically generated arrays at compile-time. External firmwares which use OpenSBI as library will have to create these arrays separately because they typically don't use OpenSBI build system.
These patches can also be found in generated_carray_v1 branch at:
https://github.com/avpatel/opensbi.git
Anup Patel (11):
Makefile: Allow generated C source to be anywhere in build directory
Makefile: Add support for generating C array at compile time
lib: utils/reset: Generate FDT reset driver list at compile-time
lib: utils/serial: Generate FDT serial driver list at compile-time
lib: utils/timer: Generate FDT timer driver list at compile-time
lib: utils/irqchip: Generate FDT irqchip driver list at compile-time
lib: utils/ipi: Generate FDT ipi driver list at compile-time
lib: utils/i2c: Generate FDT i2c adapter driver list at compile-time
lib: utils/gpio: Generate FDT gpio driver list at compile-time
platform: generic: Generate platform override module list at
compile-time
platform: generic: Move Sifive platform overrides into own directory
 Makefile                                      | 17 +++-
 include/sbi_utils/gpio/fdt_gpio.h             |  2 +
 lib/utils/gpio/fdt_gpio.c                     | 18 ++---
 lib/utils/gpio/fdt_gpio_drivers.carray        |  3 +
 lib/utils/gpio/objects.mk                     |  4 +
 lib/utils/i2c/fdt_i2c.c                       | 14 ++--
 lib/utils/i2c/fdt_i2c_adapter_drivers.carray  |  3 +
 lib/utils/i2c/objects.mk                      |  4 +
 lib/utils/ipi/fdt_ipi.c                       | 12 ++-
 lib/utils/ipi/fdt_ipi_drivers.carray          |  3 +
 lib/utils/ipi/objects.mk                      |  4 +
 lib/utils/irqchip/fdt_irqchip.c               | 16 ++--
 lib/utils/irqchip/fdt_irqchip_drivers.carray  |  3 +
 lib/utils/irqchip/objects.mk                  |  8 ++
 lib/utils/reset/fdt_reset.c                   | 22 ++----
 lib/utils/reset/fdt_reset_drivers.carray      |  3 +
 lib/utils/reset/objects.mk                    | 12 +++
 lib/utils/serial/fdt_serial.c                 | 28 ++-----
 lib/utils/serial/fdt_serial_drivers.carray    |  3 +
 lib/utils/serial/objects.mk                   | 16 ++++
 lib/utils/timer/fdt_timer.c                   | 12 ++-
 lib/utils/timer/fdt_timer_drivers.carray      |  3 +
 lib/utils/timer/objects.mk                    |  4 +
 platform/generic/objects.mk                   |  3 +-
 platform/generic/platform.c                   | 14 ++--
 .../generic/platform_override_modules.carray  |  3 +
 .../{sifive_fu540.c => sifive/fu540.c}        |  0
 .../{sifive_fu740.c => sifive/fu740.c}        |  0
 platform/generic/sifive/objects.mk            |  9 +++
 scripts/carray.sh                             | 77 +++++++++++++++++++
 30 files changed, 224 insertions(+), 96 deletions(-)
 create mode 100644 lib/utils/gpio/fdt_gpio_drivers.carray
 create mode 100644 lib/utils/i2c/fdt_i2c_adapter_drivers.carray
 create mode 100644 lib/utils/ipi/fdt_ipi_drivers.carray
 create mode 100644 lib/utils/irqchip/fdt_irqchip_drivers.carray
 create mode 100644 lib/utils/reset/fdt_reset_drivers.carray
 create mode 100644 lib/utils/serial/fdt_serial_drivers.carray
 create mode 100644 lib/utils/timer/fdt_timer_drivers.carray
 create mode 100644 platform/generic/platform_override_modules.carray
 rename platform/generic/{sifive_fu540.c => sifive/fu540.c} (100%)
 rename platform/generic/{sifive_fu740.c => sifive/fu740.c} (100%)
 create mode 100644 platform/generic/sifive/objects.mk
 create mode 100755 scripts/carray.sh

Quoted 6 lines

Quoted 3 lines
--
2.34.1
LGTM. For the remaining patches,
Reviewed-by Atish Patra <[email protected]>

Applied this series to the riscv/opensbi repo

Thanks,
Anup

Quoted 4 lines

--
Regards,
Atish