| Message ID | 20220926204805.80286-1-post@lespocky.de |
|---|---|
| State | RFC |
| Delegated to: | Michal Simek |
| Headers | show |
| Series | [RFC] dm: fpga: Introduce new uclass | expand |
Hi Alexander, On Mon, 26 Sept 2022 at 14:48, Alexander Dahl <post@lespocky.de> wrote: > > For future DM based FPGA drivers and for now to have a meaningful > logging class for old FPGA drivers. > > Suggested-by: Michal Simek <michal.simek@amd.com> > Suggested-by: Simon Glass <sjg@chromium.org> > Signed-off-by: Alexander Dahl <post@lespocky.de> > --- > arch/sandbox/dts/test.dts | 4 ++++ > drivers/fpga/Kconfig | 14 ++++++++++++++ > drivers/fpga/Makefile | 3 +++ > drivers/fpga/fpga-uclass.c | 11 +++++++++++ > drivers/fpga/sandbox.c | 11 +++++++++++ > include/dm/uclass-id.h | 1 + > test/dm/Makefile | 1 + > test/dm/fpga.c | 20 ++++++++++++++++++++ > 8 files changed, 65 insertions(+) > create mode 100644 drivers/fpga/fpga-uclass.c > create mode 100644 drivers/fpga/sandbox.c > create mode 100644 test/dm/fpga.c Reviewed-by: Simon Glass <sjg@chromium.org> nits below > > diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts > index 2761588f0d..3b9cc8cd7c 100644 > --- a/arch/sandbox/dts/test.dts > +++ b/arch/sandbox/dts/test.dts > @@ -650,6 +650,10 @@ > }; > }; > > + fpga { > + compatible = "sandbox,fpga"; > + }; > + > pinctrl-gpio { > compatible = "sandbox,pinctrl-gpio"; > > diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig > index e07a9cf80e..2ad1ff60b6 100644 > --- a/drivers/fpga/Kconfig > +++ b/drivers/fpga/Kconfig > @@ -118,4 +118,18 @@ config SPL_FPGA_LOAD_SECURE > Enables the fpga loads() functions that are used to load secure > (authenticated or encrypted or both) bitstreams on to FPGA. > > +config DM_FPGA > + bool "Enable Driver Model for FPGA drivers" > + depends on DM this is the default I think so you can drop this > + select FPGA Normally we would have 'depends on FPGA' here. With this you will just need to enable DM_FPGA and will get FPGA automatically. If that is what you want, that is fine. > + help > + Enable driver model for FPGA. > + For now this is uclass only without a real driver using it. What is an FPGA? What sort of features does the uclass support (none yet). > + > +config SANDBOX_FPGA > + bool "Enable sandbox FPGA driver" > + depends on SANDBOX && DM_FPGA > + help > + tbd Add help here, explaining that for now it is only a driver with no uclass methods. > + > endmenu > diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile > index 83243fb107..610c168fc3 100644 > --- a/drivers/fpga/Makefile > +++ b/drivers/fpga/Makefile > @@ -4,6 +4,9 @@ > # Wolfgang Denk, DENX Software Engineering, wd@denx.de. > > obj-y += fpga.o > +obj-$(CONFIG_DM_FPGA) += fpga-uclass.o > +obj-$(CONFIG_SANDBOX_FPGA) += sandbox.o > + > obj-$(CONFIG_FPGA_SPARTAN2) += spartan2.o > obj-$(CONFIG_FPGA_SPARTAN3) += spartan3.o > obj-$(CONFIG_FPGA_VERSALPL) += versalpl.o > diff --git a/drivers/fpga/fpga-uclass.c b/drivers/fpga/fpga-uclass.c > new file mode 100644 > index 0000000000..4278ec28e5 > --- /dev/null > +++ b/drivers/fpga/fpga-uclass.c > @@ -0,0 +1,11 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Copyright 2022 Alexander Dahl <post@lespocky.de> > + */ > + > +#include <dm.h> > + > +UCLASS_DRIVER(fpga) = { > + .name = "fpga", > + .id = UCLASS_FPGA, > +}; > diff --git a/drivers/fpga/sandbox.c b/drivers/fpga/sandbox.c > new file mode 100644 > index 0000000000..5687efccb1 > --- /dev/null > +++ b/drivers/fpga/sandbox.c > @@ -0,0 +1,11 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Copyright 2022 Alexander Dahl <post@lespocky.de> > + */ > + > +#include <dm.h> > + > +U_BOOT_DRIVER(sandbox_fpga) = { > + .name = "sandbox_fpga", > + .id = UCLASS_FPGA, > +}; > diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h > index a432e43871..c2b15881ba 100644 > --- a/include/dm/uclass-id.h > +++ b/include/dm/uclass-id.h > @@ -56,6 +56,7 @@ enum uclass_id { > UCLASS_ETH, /* Ethernet device */ > UCLASS_ETH_PHY, /* Ethernet PHY device */ > UCLASS_FIRMWARE, /* Firmware */ > + UCLASS_FPGA, /* FPGA device */ > UCLASS_FUZZING_ENGINE, /* Fuzzing engine */ > UCLASS_FS_FIRMWARE_LOADER, /* Generic loader */ > UCLASS_GPIO, /* Bank of general-purpose I/O pins */ > diff --git a/test/dm/Makefile b/test/dm/Makefile > index 7543df8823..666c85f10a 100644 > --- a/test/dm/Makefile > +++ b/test/dm/Makefile > @@ -47,6 +47,7 @@ ifneq ($(CONFIG_EFI_PARTITION),) > obj-$(CONFIG_FASTBOOT_FLASH_MMC) += fastboot.o > endif > obj-$(CONFIG_FIRMWARE) += firmware.o > +obj-$(CONFIG_DM_FPGA) += fpga.o > obj-$(CONFIG_DM_HWSPINLOCK) += hwspinlock.o > obj-$(CONFIG_DM_I2C) += i2c.o > obj-$(CONFIG_SOUND) += i2s.o > diff --git a/test/dm/fpga.c b/test/dm/fpga.c > new file mode 100644 > index 0000000000..8d29c8f159 > --- /dev/null > +++ b/test/dm/fpga.c > @@ -0,0 +1,20 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Copyright 2022 Alexander Dahl <post@lespocky.de> > + */ > + > +#include <dm.h> > +#include <dm/test.h> > +#include <test/test.h> > +#include <test/ut.h> > + > +static int dm_test_fpga(struct unit_test_state *uts) > +{ > + struct udevice *dev; > + > + ut_assertok(uclass_get_device(UCLASS_FPGA, 0, &dev)); > + > + return 0; > +} > + > +DM_TEST(dm_test_fpga, UT_TESTF_SCAN_FDT); > > base-commit: f117c54cc83e3c519883edb5a48062644d38c443 > -- > 2.30.2 > Regards, Simon
Hello Simon, first of all, thanks for your reviewi(s), it was very helpful. Remarks below. Am Tue, Sep 27, 2022 at 07:55:00PM -0600 schrieb Simon Glass: > Hi Alexander, > > On Mon, 26 Sept 2022 at 14:48, Alexander Dahl <post@lespocky.de> wrote: > > > > For future DM based FPGA drivers and for now to have a meaningful > > logging class for old FPGA drivers. > > > > Suggested-by: Michal Simek <michal.simek@amd.com> > > Suggested-by: Simon Glass <sjg@chromium.org> > > Signed-off-by: Alexander Dahl <post@lespocky.de> > > --- > > arch/sandbox/dts/test.dts | 4 ++++ > > drivers/fpga/Kconfig | 14 ++++++++++++++ > > drivers/fpga/Makefile | 3 +++ > > drivers/fpga/fpga-uclass.c | 11 +++++++++++ > > drivers/fpga/sandbox.c | 11 +++++++++++ > > include/dm/uclass-id.h | 1 + > > test/dm/Makefile | 1 + > > test/dm/fpga.c | 20 ++++++++++++++++++++ > > 8 files changed, 65 insertions(+) > > create mode 100644 drivers/fpga/fpga-uclass.c > > create mode 100644 drivers/fpga/sandbox.c > > create mode 100644 test/dm/fpga.c > > Reviewed-by: Simon Glass <sjg@chromium.org> I did not carry that to v2 (which I will send soon), because I felt I changed too much. > > nits below > > > > > diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts > > index 2761588f0d..3b9cc8cd7c 100644 > > --- a/arch/sandbox/dts/test.dts > > +++ b/arch/sandbox/dts/test.dts > > @@ -650,6 +650,10 @@ > > }; > > }; > > > > + fpga { > > + compatible = "sandbox,fpga"; > > + }; > > + > > pinctrl-gpio { > > compatible = "sandbox,pinctrl-gpio"; > > > > diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig > > index e07a9cf80e..2ad1ff60b6 100644 > > --- a/drivers/fpga/Kconfig > > +++ b/drivers/fpga/Kconfig > > @@ -118,4 +118,18 @@ config SPL_FPGA_LOAD_SECURE > > Enables the fpga loads() functions that are used to load secure > > (authenticated or encrypted or both) bitstreams on to FPGA. > > > > +config DM_FPGA > > + bool "Enable Driver Model for FPGA drivers" > > + depends on DM > > this is the default I think so you can drop this > > > + select FPGA > > Normally we would have 'depends on FPGA' here. With this you will just > need to enable DM_FPGA and will get FPGA automatically. If that is > what you want, that is fine. If you look at drivers/fpga/Kconfig you'll see CONFIG_FPGA is currently not user selectable, so I did what CONFIG_FPGA_ALTERA and CONFIG_FPGA_XILINX do. I guess this might be from the old days and I saw it looks different for other driver subsystems, but I did not want to restructure FPGA here at this point. Actually, if I dropped this, and CONFIG_FPGA was not 'y', then neither the new fpga uclass nor the sandbox driver was built, although selected. > > + help > > + Enable driver model for FPGA. > > + For now this is uclass only without a real driver using it. > > What is an FPGA? What sort of features does the uclass support (none yet). > > > + > > +config SANDBOX_FPGA > > + bool "Enable sandbox FPGA driver" > > + depends on SANDBOX && DM_FPGA > > + help > > + tbd > > Add help here, explaining that for now it is only a driver with no > uclass methods. Added some more help in v2. > > + > > endmenu > > diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile > > index 83243fb107..610c168fc3 100644 > > --- a/drivers/fpga/Makefile > > +++ b/drivers/fpga/Makefile > > @@ -4,6 +4,9 @@ > > # Wolfgang Denk, DENX Software Engineering, wd@denx.de. > > > > obj-y += fpga.o > > +obj-$(CONFIG_DM_FPGA) += fpga-uclass.o > > +obj-$(CONFIG_SANDBOX_FPGA) += sandbox.o > > + > > obj-$(CONFIG_FPGA_SPARTAN2) += spartan2.o > > obj-$(CONFIG_FPGA_SPARTAN3) += spartan3.o > > obj-$(CONFIG_FPGA_VERSALPL) += versalpl.o > > diff --git a/drivers/fpga/fpga-uclass.c b/drivers/fpga/fpga-uclass.c > > new file mode 100644 > > index 0000000000..4278ec28e5 > > --- /dev/null > > +++ b/drivers/fpga/fpga-uclass.c > > @@ -0,0 +1,11 @@ > > +// SPDX-License-Identifier: GPL-2.0+ > > +/* > > + * Copyright 2022 Alexander Dahl <post@lespocky.de> > > + */ > > + > > +#include <dm.h> > > + > > +UCLASS_DRIVER(fpga) = { > > + .name = "fpga", > > + .id = UCLASS_FPGA, > > +}; > > diff --git a/drivers/fpga/sandbox.c b/drivers/fpga/sandbox.c > > new file mode 100644 > > index 0000000000..5687efccb1 > > --- /dev/null > > +++ b/drivers/fpga/sandbox.c > > @@ -0,0 +1,11 @@ > > +// SPDX-License-Identifier: GPL-2.0+ > > +/* > > + * Copyright 2022 Alexander Dahl <post@lespocky.de> > > + */ > > + > > +#include <dm.h> > > + > > +U_BOOT_DRIVER(sandbox_fpga) = { > > + .name = "sandbox_fpga", > > + .id = UCLASS_FPGA, > > +}; This is not enough to probe the driver, I added something for .of_match in v2. Greets Alex > > diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h > > index a432e43871..c2b15881ba 100644 > > --- a/include/dm/uclass-id.h > > +++ b/include/dm/uclass-id.h > > @@ -56,6 +56,7 @@ enum uclass_id { > > UCLASS_ETH, /* Ethernet device */ > > UCLASS_ETH_PHY, /* Ethernet PHY device */ > > UCLASS_FIRMWARE, /* Firmware */ > > + UCLASS_FPGA, /* FPGA device */ > > UCLASS_FUZZING_ENGINE, /* Fuzzing engine */ > > UCLASS_FS_FIRMWARE_LOADER, /* Generic loader */ > > UCLASS_GPIO, /* Bank of general-purpose I/O pins */ > > diff --git a/test/dm/Makefile b/test/dm/Makefile > > index 7543df8823..666c85f10a 100644 > > --- a/test/dm/Makefile > > +++ b/test/dm/Makefile > > @@ -47,6 +47,7 @@ ifneq ($(CONFIG_EFI_PARTITION),) > > obj-$(CONFIG_FASTBOOT_FLASH_MMC) += fastboot.o > > endif > > obj-$(CONFIG_FIRMWARE) += firmware.o > > +obj-$(CONFIG_DM_FPGA) += fpga.o > > obj-$(CONFIG_DM_HWSPINLOCK) += hwspinlock.o > > obj-$(CONFIG_DM_I2C) += i2c.o > > obj-$(CONFIG_SOUND) += i2s.o > > diff --git a/test/dm/fpga.c b/test/dm/fpga.c > > new file mode 100644 > > index 0000000000..8d29c8f159 > > --- /dev/null > > +++ b/test/dm/fpga.c > > @@ -0,0 +1,20 @@ > > +// SPDX-License-Identifier: GPL-2.0+ > > +/* > > + * Copyright 2022 Alexander Dahl <post@lespocky.de> > > + */ > > + > > +#include <dm.h> > > +#include <dm/test.h> > > +#include <test/test.h> > > +#include <test/ut.h> > > + > > +static int dm_test_fpga(struct unit_test_state *uts) > > +{ > > + struct udevice *dev; > > + > > + ut_assertok(uclass_get_device(UCLASS_FPGA, 0, &dev)); > > + > > + return 0; > > +} > > + > > +DM_TEST(dm_test_fpga, UT_TESTF_SCAN_FDT); > > > > base-commit: f117c54cc83e3c519883edb5a48062644d38c443 > > -- > > 2.30.2 > > > > Regards, > Simon
diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts index 2761588f0d..3b9cc8cd7c 100644 --- a/arch/sandbox/dts/test.dts +++ b/arch/sandbox/dts/test.dts @@ -650,6 +650,10 @@ }; }; + fpga { + compatible = "sandbox,fpga"; + }; + pinctrl-gpio { compatible = "sandbox,pinctrl-gpio"; diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig index e07a9cf80e..2ad1ff60b6 100644 --- a/drivers/fpga/Kconfig +++ b/drivers/fpga/Kconfig @@ -118,4 +118,18 @@ config SPL_FPGA_LOAD_SECURE Enables the fpga loads() functions that are used to load secure (authenticated or encrypted or both) bitstreams on to FPGA. +config DM_FPGA + bool "Enable Driver Model for FPGA drivers" + depends on DM + select FPGA + help + Enable driver model for FPGA. + For now this is uclass only without a real driver using it. + +config SANDBOX_FPGA + bool "Enable sandbox FPGA driver" + depends on SANDBOX && DM_FPGA + help + tbd + endmenu diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile index 83243fb107..610c168fc3 100644 --- a/drivers/fpga/Makefile +++ b/drivers/fpga/Makefile @@ -4,6 +4,9 @@ # Wolfgang Denk, DENX Software Engineering, wd@denx.de. obj-y += fpga.o +obj-$(CONFIG_DM_FPGA) += fpga-uclass.o +obj-$(CONFIG_SANDBOX_FPGA) += sandbox.o + obj-$(CONFIG_FPGA_SPARTAN2) += spartan2.o obj-$(CONFIG_FPGA_SPARTAN3) += spartan3.o obj-$(CONFIG_FPGA_VERSALPL) += versalpl.o diff --git a/drivers/fpga/fpga-uclass.c b/drivers/fpga/fpga-uclass.c new file mode 100644 index 0000000000..4278ec28e5 --- /dev/null +++ b/drivers/fpga/fpga-uclass.c @@ -0,0 +1,11 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright 2022 Alexander Dahl <post@lespocky.de> + */ + +#include <dm.h> + +UCLASS_DRIVER(fpga) = { + .name = "fpga", + .id = UCLASS_FPGA, +}; diff --git a/drivers/fpga/sandbox.c b/drivers/fpga/sandbox.c new file mode 100644 index 0000000000..5687efccb1 --- /dev/null +++ b/drivers/fpga/sandbox.c @@ -0,0 +1,11 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright 2022 Alexander Dahl <post@lespocky.de> + */ + +#include <dm.h> + +U_BOOT_DRIVER(sandbox_fpga) = { + .name = "sandbox_fpga", + .id = UCLASS_FPGA, +}; diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h index a432e43871..c2b15881ba 100644 --- a/include/dm/uclass-id.h +++ b/include/dm/uclass-id.h @@ -56,6 +56,7 @@ enum uclass_id { UCLASS_ETH, /* Ethernet device */ UCLASS_ETH_PHY, /* Ethernet PHY device */ UCLASS_FIRMWARE, /* Firmware */ + UCLASS_FPGA, /* FPGA device */ UCLASS_FUZZING_ENGINE, /* Fuzzing engine */ UCLASS_FS_FIRMWARE_LOADER, /* Generic loader */ UCLASS_GPIO, /* Bank of general-purpose I/O pins */ diff --git a/test/dm/Makefile b/test/dm/Makefile index 7543df8823..666c85f10a 100644 --- a/test/dm/Makefile +++ b/test/dm/Makefile @@ -47,6 +47,7 @@ ifneq ($(CONFIG_EFI_PARTITION),) obj-$(CONFIG_FASTBOOT_FLASH_MMC) += fastboot.o endif obj-$(CONFIG_FIRMWARE) += firmware.o +obj-$(CONFIG_DM_FPGA) += fpga.o obj-$(CONFIG_DM_HWSPINLOCK) += hwspinlock.o obj-$(CONFIG_DM_I2C) += i2c.o obj-$(CONFIG_SOUND) += i2s.o diff --git a/test/dm/fpga.c b/test/dm/fpga.c new file mode 100644 index 0000000000..8d29c8f159 --- /dev/null +++ b/test/dm/fpga.c @@ -0,0 +1,20 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright 2022 Alexander Dahl <post@lespocky.de> + */ + +#include <dm.h> +#include <dm/test.h> +#include <test/test.h> +#include <test/ut.h> + +static int dm_test_fpga(struct unit_test_state *uts) +{ + struct udevice *dev; + + ut_assertok(uclass_get_device(UCLASS_FPGA, 0, &dev)); + + return 0; +} + +DM_TEST(dm_test_fpga, UT_TESTF_SCAN_FDT);
For future DM based FPGA drivers and for now to have a meaningful logging class for old FPGA drivers. Suggested-by: Michal Simek <michal.simek@amd.com> Suggested-by: Simon Glass <sjg@chromium.org> Signed-off-by: Alexander Dahl <post@lespocky.de> --- arch/sandbox/dts/test.dts | 4 ++++ drivers/fpga/Kconfig | 14 ++++++++++++++ drivers/fpga/Makefile | 3 +++ drivers/fpga/fpga-uclass.c | 11 +++++++++++ drivers/fpga/sandbox.c | 11 +++++++++++ include/dm/uclass-id.h | 1 + test/dm/Makefile | 1 + test/dm/fpga.c | 20 ++++++++++++++++++++ 8 files changed, 65 insertions(+) create mode 100644 drivers/fpga/fpga-uclass.c create mode 100644 drivers/fpga/sandbox.c create mode 100644 test/dm/fpga.c base-commit: f117c54cc83e3c519883edb5a48062644d38c443