| Message ID | 20190709223649.25630-2-alistair.francis@wdc.com |
|---|---|
| State | Rejected |
| Headers | show |
| Series | None | expand |
Hello Alistair, On Tue, 9 Jul 2019 15:36:49 -0700 Alistair Francis <alistair.francis@wdc.com> wrote: > Instead of requiring users to look at the OpenSBI source code and > determine the platform string let's use a choice option to allow them to > specify the platform they would like to target. We still allow them to > specify a custom string if they want to. > > Signed-off-by: Alistair Francis <alistair.francis@wdc.com> I have not applied this patch, because we don't do this anywhere in Buildroot. For things like U-Boot, Linux or Barebox, the name of the platform/configuration is always a free-form string. Having an explicit list of platform is difficult to maintain as the list of platforms can be huge. Also the list of supported platforms can differ from one version to the other (in the case where the version itself is configurable). In addition, how do you draw the line between platforms that should be explicitly listed in the Config.in choice, and the ones for which users should use the free-form string option. But again, the main reason for rejecting is that in exactly this situation in other packages in Buildroot, we use a simple free-form string. Best regards, Thomas
On Sat, Jul 20, 2019 at 12:50 PM Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote: > > Hello Alistair, > > On Tue, 9 Jul 2019 15:36:49 -0700 > Alistair Francis <alistair.francis@wdc.com> wrote: > > > Instead of requiring users to look at the OpenSBI source code and > > determine the platform string let's use a choice option to allow them to > > specify the platform they would like to target. We still allow them to > > specify a custom string if they want to. > > > > Signed-off-by: Alistair Francis <alistair.francis@wdc.com> > > I have not applied this patch, because we don't do this anywhere in > Buildroot. For things like U-Boot, Linux or Barebox, the name of the > platform/configuration is always a free-form string. Having an explicit > list of platform is difficult to maintain as the list of platforms can > be huge. Also the list of supported platforms can differ from one > version to the other (in the case where the version itself is > configurable). In addition, how do you draw the line between platforms > that should be explicitly listed in the Config.in choice, and the ones > for which users should use the free-form string option. > > But again, the main reason for rejecting is that in exactly this > situation in other packages in Buildroot, we use a simple free-form > string. No worries. As that was the main problem Mark had with the HiFive Unleashed patches these should be good to merge. I just sent a v2 with an updated readme to the list. Alistair > > Best regards, > > Thomas > -- > Thomas Petazzoni, CTO, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com > _______________________________________________ > buildroot mailing list > buildroot@busybox.net > http://lists.busybox.net/mailman/listinfo/buildroot
diff --git a/boot/opensbi/Config.in b/boot/opensbi/Config.in index 5f3cc13312..997c79f9b0 100644 --- a/boot/opensbi/Config.in +++ b/boot/opensbi/Config.in @@ -13,13 +13,32 @@ config BR2_TARGET_OPENSBI https://github.com/riscv/opensbi.git if BR2_TARGET_OPENSBI -config BR2_TARGET_OPENSBI_PLAT - string "OpenSBI Platform" - default "" + +choice + prompt "OpenSBI Platform" help Specifies the OpenSBI platform to build. If no platform is specified only the OpenSBI platform independent static library libsbi.a is built. If a platform is specified then the platform specific static library libplatsbi.a and firmware examples are built. + +config BR2_TARGET_OPENSBI_LIBRARY_ONLY + bool "Library Only" + +config BR2_TARGET_OPENSBI_QEMU_VIRT + bool "QEMU Virt" + +config BR2_TARGET_OPENSBI_QEMU_SIFIVE_U + bool "QEMU SiFive U" + +config BR2_TARGET_OPENSBI_CUSTOM_PLATFORM + bool "Custom Platform" + +endchoice + +config BR2_TARGET_OPENSBI_CUSTOM_PLATFORM_VALUE + string "OpenSBI Platform String" + depends on BR2_TARGET_OPENSBI_CUSTOM_PLATFORM + endif diff --git a/boot/opensbi/opensbi.mk b/boot/opensbi/opensbi.mk index 83552a5442..78da22aa1f 100644 --- a/boot/opensbi/opensbi.mk +++ b/boot/opensbi/opensbi.mk @@ -14,7 +14,16 @@ OPENSBI_INSTALL_STAGING = YES OPENSBI_MAKE_ENV = \ CROSS_COMPILE=$(TARGET_CROSS) -OPENSBI_PLAT = $(call qstrip,$(BR2_TARGET_OPENSBI_PLAT)) +ifeq ($(BR2_TARGET_OPENSBI_QEMU_VIRT),y) +OPENSBI_PLAT = qemu/virt +else ifeq ($(BR2_TARGET_OPENSBI_QEMU_SIFIVE_U),y) +OPENSBI_PLAT = qemu/sifive_u +else ifeq ($(BR2_TARGET_OPENSBI_SIFIVE_FU540),y) +OPENSBI_PLAT = sifive/fu540 +else ifeq ($(BR2_TARGET_OPENSBI_CUSTOM_PLATFORM),y) +OPENSBI_PLAT = $(call qstrip,$(BR2_TARGET_OPENSBI_CUSTOM_PLATFORM_VALUE)) +endif + ifneq ($(OPENSBI_PLAT),) OPENSBI_MAKE_ENV += PLATFORM=$(OPENSBI_PLAT) endif diff --git a/configs/qemu_riscv32_virt_defconfig b/configs/qemu_riscv32_virt_defconfig index a1a8c5fd20..ebe3e72135 100644 --- a/configs/qemu_riscv32_virt_defconfig +++ b/configs/qemu_riscv32_virt_defconfig @@ -24,4 +24,4 @@ BR2_LINUX_KERNEL_IMAGE=y # Bootloader BR2_TARGET_OPENSBI=y BR2_TARGET_OPENSBI_USE_PLAT=y -BR2_TARGET_OPENSBI_PLAT="qemu/virt" +BR2_TARGET_OPENSBI_QEMU_VIRT=y diff --git a/configs/qemu_riscv64_virt_defconfig b/configs/qemu_riscv64_virt_defconfig index c0b1a43925..16e0e332f7 100644 --- a/configs/qemu_riscv64_virt_defconfig +++ b/configs/qemu_riscv64_virt_defconfig @@ -23,4 +23,4 @@ BR2_LINUX_KERNEL_IMAGE=y # Bootloader BR2_TARGET_OPENSBI=y BR2_TARGET_OPENSBI_USE_PLAT=y -BR2_TARGET_OPENSBI_PLAT="qemu/virt" +BR2_TARGET_OPENSBI_QEMU_VIRT=y
Instead of requiring users to look at the OpenSBI source code and determine the platform string let's use a choice option to allow them to specify the platform they would like to target. We still allow them to specify a custom string if they want to. Signed-off-by: Alistair Francis <alistair.francis@wdc.com> --- boot/opensbi/Config.in | 25 ++++++++++++++++++++++--- boot/opensbi/opensbi.mk | 11 ++++++++++- configs/qemu_riscv32_virt_defconfig | 2 +- configs/qemu_riscv64_virt_defconfig | 2 +- 4 files changed, 34 insertions(+), 6 deletions(-)