[RFC PATCH] dm: fpga: Introduce new uclass

Alexander Dahl post at lespocky.de
Fri Sep 30 14:03:43 CEST 2022


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 at 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 at amd.com>
> > Suggested-by: Simon Glass <sjg at chromium.org>
> > Signed-off-by: Alexander Dahl <post at 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 at 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 at 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 at 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 at 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 at 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

-- 
/"\ ASCII RIBBON | »With the first link, the chain is forged. The first
\ / CAMPAIGN     | speech censured, the first thought forbidden, the
 X  AGAINST      | first freedom denied, chains us all irrevocably.«
/ \ HTML MAIL    | (Jean-Luc Picard, quoting Judge Aaron Satie)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20220930/506ec035/attachment.sig>


More information about the U-Boot mailing list