[00/11] OpenSBI compile-time C arrays

Message ID	20220503033759.544156-1-apatel@ventanamicro.com
Headers	show Return-Path: <opensbi-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org> From: Anup Patel <apatel@ventanamicro.com> To: Atish Patra <atishp@atishpatra.org>, Alistair Francis <Alistair.Francis@wdc.com> Cc: Anup Patel <anup@brainfault.org>, opensbi@lists.infradead.org, Anup Patel <apatel@ventanamicro.com> Subject: [PATCH 00/11] OpenSBI compile-time C arrays Date: Tue, 3 May 2022 09:07:48 +0530 Message-Id: <20220503033759.544156-1-apatel@ventanamicro.com> MIME-Version: 1.0 preview: 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 li [...] Content analysis details: (-0.2 points, 5.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- -0.0 RCVD_IN_DNSWL_NONE RBL: Sender listed at https://www.dnswl.org/, no trust [2607:f8b0:4864:20:0:0:0:630 listed in] [list.dnswl.org] -0.0 SPF_PASS SPF: sender matches SPF record 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.1 DKIM_VALID_EF Message has a valid DKIM or DK signature from envelope-from domain Precedence: list Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "opensbi" <opensbi-bounces@lists.infradead.org> Errors-To: opensbi-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org
Series	OpenSBI compile-time C arrays \| expand [00/11] OpenSBI compile-time C arrays [01/11] Makefile: Allow generated C source to be anywhere in build directory [02/11] Makefile: Add support for generating C array at compile time [03/11] lib: utils/reset: Generate FDT reset driver list at compile-time [04/11] lib: utils/serial: Generate FDT serial driver list at compile-time [05/11] lib: utils/timer: Generate FDT timer driver list at compile-time [06/11] lib: utils/irqchip: Generate FDT irqchip driver list at compile-time [07/11] lib: utils/ipi: Generate FDT ipi driver list at compile-time [08/11] lib: utils/i2c: Generate FDT i2c adapter driver list at compile-time [09/11] lib: utils/gpio: Generate FDT gpio driver list at compile-time [10/11] platform: generic: Generate platform override module list at compile-time [11/11] platform: generic: Move Sifive platform overrides into own directory

Anup Patel May 3, 2022, 3:37 a.m. UTC

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

Jessica Clarke May 3, 2022, 3:42 a.m. UTC | #1

On 3 May 2022, at 04:38, Anup Patel <apatel@ventanamicro.com> wrote:
> 
> 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

> 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@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi

Anup Patel May 3, 2022, 3:52 a.m. UTC | #2

On Tue, May 3, 2022 at 9:12 AM Jessica Clarke <jrtc27@jrtc27.com> wrote:
>
> On 3 May 2022, at 04:38, Anup Patel <apatel@ventanamicro.com> wrote:
> >
> > 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

>
> Jess
>
> > 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@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/opensbi

Jessica Clarke May 3, 2022, 3:54 a.m. UTC | #3

[resent cc’ing list…]

On 3 May 2022, at 04:52, Anup Patel <apatel@ventanamicro.com> wrote:
> 
> On Tue, May 3, 2022 at 9:12 AM Jessica Clarke <jrtc27@jrtc27.com> wrote:
>> 
>>> On 3 May 2022, at 04:38, Anup Patel <apatel@ventanamicro.com> wrote:
>>> 
>>> 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.


They require no build system support. Unlike your patch.

Jess

> Regards,
> Anup
> 
>> 
>> Jess
>> 
>>> 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@lists.infradead.org
>>> http://lists.infradead.org/mailman/listinfo/opensbi

Anup Patel May 3, 2022, 4:04 a.m. UTC | #4

On Tue, May 3, 2022 at 9:24 AM Jessica Clarke <jrtc27@jrtc27.com> wrote:
>
> [resent cc’ing list…]
>
> On 3 May 2022, at 04:52, Anup Patel <apatel@ventanamicro.com> wrote:
> >
> > On Tue, May 3, 2022 at 9:12 AM Jessica Clarke <jrtc27@jrtc27.com> wrote:
> >>
> >>> On 3 May 2022, at 04:38, Anup Patel <apatel@ventanamicro.com> wrote:
> >>>
> >>> 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.
>
>
> 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

"




>
> Jess
>
> > Regards,
> > Anup
> >
> >>
> >> Jess
> >>
> >>> 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@lists.infradead.org
> >>> http://lists.infradead.org/mailman/listinfo/opensbi

Jessica Clarke May 3, 2022, 4:13 a.m. UTC | #5

On 3 May 2022, at 05:04, Anup Patel <apatel@ventanamicro.com> wrote:
> 
> On Tue, May 3, 2022 at 9:24 AM Jessica Clarke <jrtc27@jrtc27.com> wrote:
>> 
>> [resent cc’ing list…]
>> 
>>> On 3 May 2022, at 04:52, Anup Patel <apatel@ventanamicro.com> wrote:
>>> 
>>> On Tue, May 3, 2022 at 9:12 AM Jessica Clarke <jrtc27@jrtc27.com> wrote:
>>>> 
>>>>> On 3 May 2022, at 04:38, Anup Patel <apatel@ventanamicro.com> wrote:
>>>>> 
>>>>> 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.
>> 
>> 
>> 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.

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

> Regards,
> Anup
> 
> "
> 
> 
> 
> 
>> 
>> Jess
>> 
>>> Regards,
>>> Anup
>>> 
>>>> 
>>>> Jess
>>>> 
>>>>> 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@lists.infradead.org
>>>>> http://lists.infradead.org/mailman/listinfo/opensbi

Anup Patel May 3, 2022, 4:17 a.m. UTC | #6

On Tue, May 3, 2022 at 9:43 AM Jessica Clarke <jrtc27@jrtc27.com> wrote:
>
> On 3 May 2022, at 05:04, Anup Patel <apatel@ventanamicro.com> wrote:
> >
> > On Tue, May 3, 2022 at 9:24 AM Jessica Clarke <jrtc27@jrtc27.com> wrote:
> >>
> >> [resent cc’ing list…]
> >>
> >>> On 3 May 2022, at 04:52, Anup Patel <apatel@ventanamicro.com> wrote:
> >>>
> >>> On Tue, May 3, 2022 at 9:12 AM Jessica Clarke <jrtc27@jrtc27.com> wrote:
> >>>>
> >>>>> On 3 May 2022, at 04:38, Anup Patel <apatel@ventanamicro.com> wrote:
> >>>>>
> >>>>> 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.
> >>
> >>
> >> 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.
>
> 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

>
> Jess
>
> > Regards,
> > Anup
> >
> > "
> >
> >
> >
> >
> >>
> >> Jess
> >>
> >>> Regards,
> >>> Anup
> >>>
> >>>>
> >>>> Jess
> >>>>
> >>>>> 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@lists.infradead.org
> >>>>> http://lists.infradead.org/mailman/listinfo/opensbi

Jessica Clarke May 3, 2022, 4:23 a.m. UTC | #7

On 3 May 2022, at 05:17, Anup Patel <apatel@ventanamicro.com> wrote:
> 
> On Tue, May 3, 2022 at 9:43 AM Jessica Clarke <jrtc27@jrtc27.com> wrote:
>> 
>> On 3 May 2022, at 05:04, Anup Patel <apatel@ventanamicro.com> wrote:
>>> 
>>> On Tue, May 3, 2022 at 9:24 AM Jessica Clarke <jrtc27@jrtc27.com> wrote:
>>>> 
>>>> [resent cc’ing list…]
>>>> 
>>>>> On 3 May 2022, at 04:52, Anup Patel <apatel@ventanamicro.com> wrote:
>>>>> 
>>>>> On Tue, May 3, 2022 at 9:12 AM Jessica Clarke <jrtc27@jrtc27.com> wrote:
>>>>>> 
>>>>>>> On 3 May 2022, at 04:38, Anup Patel <apatel@ventanamicro.com> wrote:
>>>>>>> 
>>>>>>> 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.
>>>> 
>>>> 
>>>> 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.
>> 
>> 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.

> 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

> Regards,
> Anup
> 
>> 
>> Jess
>> 
>>> Regards,
>>> Anup
>>> 
>>> "
>>> 
>>> 
>>> 
>>> 
>>>> 
>>>> Jess
>>>> 
>>>>> Regards,
>>>>> Anup
>>>>> 
>>>>>> 
>>>>>> Jess
>>>>>> 
>>>>>>> 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@lists.infradead.org
>>>>>>> http://lists.infradead.org/mailman/listinfo/opensbi

Anup Patel May 3, 2022, 4:30 a.m. UTC | #8

On Tue, May 3, 2022 at 9:53 AM Jessica Clarke <jrtc27@jrtc27.com> wrote:
>
> On 3 May 2022, at 05:17, Anup Patel <apatel@ventanamicro.com> wrote:
> >
> > On Tue, May 3, 2022 at 9:43 AM Jessica Clarke <jrtc27@jrtc27.com> wrote:
> >>
> >> On 3 May 2022, at 05:04, Anup Patel <apatel@ventanamicro.com> wrote:
> >>>
> >>> On Tue, May 3, 2022 at 9:24 AM Jessica Clarke <jrtc27@jrtc27.com> wrote:
> >>>>
> >>>> [resent cc’ing list…]
> >>>>
> >>>>> On 3 May 2022, at 04:52, Anup Patel <apatel@ventanamicro.com> wrote:
> >>>>>
> >>>>> On Tue, May 3, 2022 at 9:12 AM Jessica Clarke <jrtc27@jrtc27.com> wrote:
> >>>>>>
> >>>>>>> On 3 May 2022, at 04:38, Anup Patel <apatel@ventanamicro.com> wrote:
> >>>>>>>
> >>>>>>> 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.
> >>>>
> >>>>
> >>>> 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.
> >>
> >> 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.

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.

>
> > 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

>
> Jess
>
> > Regards,
> > Anup
> >
> >>
> >> Jess
> >>
> >>> Regards,
> >>> Anup
> >>>
> >>> "
> >>>
> >>>
> >>>
> >>>
> >>>>
> >>>> Jess
> >>>>
> >>>>> Regards,
> >>>>> Anup
> >>>>>
> >>>>>>
> >>>>>> Jess
> >>>>>>
> >>>>>>> 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@lists.infradead.org
> >>>>>>> http://lists.infradead.org/mailman/listinfo/opensbi
>

Jessica Clarke May 3, 2022, 4:45 a.m. UTC | #9

On 3 May 2022, at 05:30, Anup Patel <anup@brainfault.org> wrote:
> 
> On Tue, May 3, 2022 at 9:53 AM Jessica Clarke <jrtc27@jrtc27.com> wrote:
>> 
>> On 3 May 2022, at 05:17, Anup Patel <apatel@ventanamicro.com> wrote:
>>> 
>>> On Tue, May 3, 2022 at 9:43 AM Jessica Clarke <jrtc27@jrtc27.com> wrote:
>>>> 
>>>> On 3 May 2022, at 05:04, Anup Patel <apatel@ventanamicro.com> wrote:
>>>>> 
>>>>> On Tue, May 3, 2022 at 9:24 AM Jessica Clarke <jrtc27@jrtc27.com> wrote:
>>>>>> 
>>>>>> [resent cc’ing list…]
>>>>>> 
>>>>>>> On 3 May 2022, at 04:52, Anup Patel <apatel@ventanamicro.com> wrote:
>>>>>>> 
>>>>>>> On Tue, May 3, 2022 at 9:12 AM Jessica Clarke <jrtc27@jrtc27.com> wrote:
>>>>>>>> 
>>>>>>>>> On 3 May 2022, at 04:38, Anup Patel <apatel@ventanamicro.com> wrote:
>>>>>>>>> 
>>>>>>>>> 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.
>>>>>> 
>>>>>> 
>>>>>> 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.
>>>> 
>>>> 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.
> 
> 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.

> 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.

>>> 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.

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

> Regards,
> Anup
> 
>> 
>> Jess
>> 
>>> Regards,
>>> Anup
>>> 
>>>> 
>>>> Jess
>>>> 
>>>>> Regards,
>>>>> Anup
>>>>> 
>>>>> "
>>>>> 
>>>>> 
>>>>> 
>>>>> 
>>>>>> 
>>>>>> Jess
>>>>>> 
>>>>>>> Regards,
>>>>>>> Anup
>>>>>>> 
>>>>>>>> 
>>>>>>>> Jess
>>>>>>>> 
>>>>>>>>> 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@lists.infradead.org
>>>>>>>>> http://lists.infradead.org/mailman/listinfo/opensbi

Anup Patel May 3, 2022, 4:47 a.m. UTC | #10

On Tue, May 3, 2022 at 10:15 AM Jessica Clarke <jrtc27@jrtc27.com> wrote:
>
> On 3 May 2022, at 05:30, Anup Patel <anup@brainfault.org> wrote:
> >
> > On Tue, May 3, 2022 at 9:53 AM Jessica Clarke <jrtc27@jrtc27.com> wrote:
> >>
> >> On 3 May 2022, at 05:17, Anup Patel <apatel@ventanamicro.com> wrote:
> >>>
> >>> On Tue, May 3, 2022 at 9:43 AM Jessica Clarke <jrtc27@jrtc27.com> wrote:
> >>>>
> >>>> On 3 May 2022, at 05:04, Anup Patel <apatel@ventanamicro.com> wrote:
> >>>>>
> >>>>> On Tue, May 3, 2022 at 9:24 AM Jessica Clarke <jrtc27@jrtc27.com> wrote:
> >>>>>>
> >>>>>> [resent cc’ing list…]
> >>>>>>
> >>>>>>> On 3 May 2022, at 04:52, Anup Patel <apatel@ventanamicro.com> wrote:
> >>>>>>>
> >>>>>>> On Tue, May 3, 2022 at 9:12 AM Jessica Clarke <jrtc27@jrtc27.com> wrote:
> >>>>>>>>
> >>>>>>>>> On 3 May 2022, at 04:38, Anup Patel <apatel@ventanamicro.com> wrote:
> >>>>>>>>>
> >>>>>>>>> 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.
> >>>>>>
> >>>>>>
> >>>>>> 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.
> >>>>
> >>>> 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.
> >
> > 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.
>
> > 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.
>
> >>> 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.
>
> 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

>
> Jess
>
> > Regards,
> > Anup
> >
> >>
> >> Jess
> >>
> >>> Regards,
> >>> Anup
> >>>
> >>>>
> >>>> Jess
> >>>>
> >>>>> Regards,
> >>>>> Anup
> >>>>>
> >>>>> "
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>>>
> >>>>>> Jess
> >>>>>>
> >>>>>>> Regards,
> >>>>>>> Anup
> >>>>>>>
> >>>>>>>>
> >>>>>>>> Jess
> >>>>>>>>
> >>>>>>>>> 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@lists.infradead.org
> >>>>>>>>> http://lists.infradead.org/mailman/listinfo/opensbi
>

Xiang W May 3, 2022, 3:35 p.m. UTC | #11

在 2022-05-03星期二的 09:07 +0530，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.

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

> 
> 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
> 
>

Anup Patel May 3, 2022, 4:24 p.m. UTC | #12

On Tue, May 3, 2022 at 9:07 PM Xiang W <wxjstz@126.com> wrote:
>
> 在 2022-05-03星期二的 09:07 +0530，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.
>
> 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

>
> Regards,
> Xiang W
>
> >
> > 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@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi

Atish Patra May 9, 2022, 5:29 p.m. UTC | #13

On Mon, May 2, 2022 at 8:38 PM Anup Patel <apatel@ventanamicro.com> wrote:
>
> 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
>
> --
> 2.34.1
>

LGTM. For the remaining patches,
Reviewed-by: Atish Patra <atishp@rivosinc.com>

Anup Patel May 13, 2022, 4:03 a.m. UTC | #14

On Mon, May 9, 2022 at 10:59 PM Atish Patra <atishp@atishpatra.org> wrote:
>
> On Mon, May 2, 2022 at 8:38 PM Anup Patel <apatel@ventanamicro.com> wrote:
> >
> > 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
> >
> > --
> > 2.34.1
> >
>
> LGTM. For the remaining patches,
> Reviewed-by: Atish Patra <atishp@rivosinc.com>

Applied this series to the riscv/opensbi repo

Thanks,
Anup

>
> --
> Regards,
> Atish

[00/11] OpenSBI compile-time C arrays

Message

Comments