[RFC PATCH] dm: fpga: Introduce new uclass

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

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 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,
> +};
> 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