Received-SPF: pass (google.com: domain of linux-kernel+bounces-81115-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Mon, 26 Feb 2024 17:50:38 +0800
From: Xu Yilun <yilun.xu@linux.intel.com>
To: Charles Perry <charles.perry@savoirfairelinux.com>
Cc: mdf@kernel.org, avandiver@markem-imaje.com, bcody@markem-imaje.com,
	Wu Hao <hao.wu@intel.com>, Xu Yilun <yilun.xu@intel.com>,
	Tom Rix <trix@redhat.com>, Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Michal Simek <michal.simek@amd.com>, linux-fpga@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v4 3/3] fpga: xilinx-selectmap: add new driver
Message-ID: <Zdxe7mvBbHDHzbiS@yilunxu-OptiPlex-7050>
References: <20240221195058.1281973-1-charles.perry@savoirfairelinux.com>
 <20240221195058.1281973-4-charles.perry@savoirfairelinux.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240221195058.1281973-4-charles.perry@savoirfairelinux.com>

On Wed, Feb 21, 2024 at 02:50:49PM -0500, Charles Perry wrote:
> Xilinx 7 series FPGA can be programmed using a parallel port named
> the SelectMAP interface in the datasheet. This interface is compatible
> with the i.MX6 EIM bus controller but other types of external memory
> mapped parallel bus might work.
> 
> xilinx-selectmap currently only supports the x8 mode where data is loaded
> at one byte per rising edge of the clock, with the MSb of each byte
> presented to the D0 pin.
> 
> Signed-off-by: Charles Perry <charles.perry@savoirfairelinux.com>
> ---
>  drivers/fpga/Kconfig            |  8 +++
>  drivers/fpga/Makefile           |  1 +
>  drivers/fpga/xilinx-selectmap.c | 97 +++++++++++++++++++++++++++++++++
>  3 files changed, 106 insertions(+)
>  create mode 100644 drivers/fpga/xilinx-selectmap.c
> 
> diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> index d27a1ebf40838..37b35f58f0dfb 100644
> --- a/drivers/fpga/Kconfig
> +++ b/drivers/fpga/Kconfig
> @@ -67,6 +67,14 @@ config FPGA_MGR_STRATIX10_SOC
>  config FPGA_MGR_XILINX_CORE
>  	tristate
>  
> +config FPGA_MGR_XILINX_SELECTMAP
> +	tristate "Xilinx Configuration over SelectMAP"
> +	depends on HAS_IOMEM
> +	select FPGA_MGR_XILINX_CORE
> +	help
> +	  FPGA manager driver support for Xilinx FPGA configuration
> +	  over SelectMAP interface.
> +
>  config FPGA_MGR_XILINX_SPI
>  	tristate "Xilinx Configuration over Slave Serial (SPI)"
>  	depends on SPI
> diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> index 7ec795b6a5a70..aeb89bb13517e 100644
> --- a/drivers/fpga/Makefile
> +++ b/drivers/fpga/Makefile
> @@ -16,6 +16,7 @@ obj-$(CONFIG_FPGA_MGR_SOCFPGA_A10)	+= socfpga-a10.o
>  obj-$(CONFIG_FPGA_MGR_STRATIX10_SOC)	+= stratix10-soc.o
>  obj-$(CONFIG_FPGA_MGR_TS73XX)		+= ts73xx-fpga.o
>  obj-$(CONFIG_FPGA_MGR_XILINX_CORE)	+= xilinx-core.o
> +obj-$(CONFIG_FPGA_MGR_XILINX_SELECTMAP)	+= xilinx-selectmap.o
>  obj-$(CONFIG_FPGA_MGR_XILINX_SPI)	+= xilinx-spi.o
>  obj-$(CONFIG_FPGA_MGR_ZYNQ_FPGA)	+= zynq-fpga.o
>  obj-$(CONFIG_FPGA_MGR_ZYNQMP_FPGA)	+= zynqmp-fpga.o
> diff --git a/drivers/fpga/xilinx-selectmap.c b/drivers/fpga/xilinx-selectmap.c
> new file mode 100644
> index 0000000000000..b63f4623f8b2c
> --- /dev/null
> +++ b/drivers/fpga/xilinx-selectmap.c
> @@ -0,0 +1,97 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Xilinx Spartan6 and 7 Series SelectMAP interface driver
> + *
> + * (C) 2024 Charles Perry <charles.perry@savoirfairelinux.com>
> + *
> + * Manage Xilinx FPGA firmware loaded over the SelectMAP configuration
> + * interface.
> + */
> +
> +#include "xilinx-core.h"
> +
> +#include <linux/platform_device.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/module.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/of.h>
> +#include <linux/io.h>
> +
> +struct xilinx_selectmap_conf {
> +	struct xilinx_fpga_core core;
> +	void __iomem *base;
> +};
> +
> +#define to_xilinx_selectmap_conf(obj) \
> +	container_of(obj, struct xilinx_selectmap_conf, core)
> +
> +static int xilinx_selectmap_write(struct xilinx_fpga_core *core,
> +				  const char *buf, size_t count)
> +{
> +	struct xilinx_selectmap_conf *conf = to_xilinx_selectmap_conf(core);
> +	u32 i;
> +
> +	for (i = 0; i < count; ++i)
> +		writeb(buf[i], conf->base);
> +
> +	return 0;
> +}
> +
> +static int xilinx_selectmap_probe(struct platform_device *pdev)
> +{
> +	struct xilinx_selectmap_conf *conf;
> +	struct resource *r;
> +	void __iomem *base;
> +	struct gpio_desc *csi_b;
> +	struct gpio_desc *rdwr_b;
> +
> +	conf = devm_kzalloc(&pdev->dev, sizeof(*conf), GFP_KERNEL);
> +	if (!conf)
> +		return -ENOMEM;
> +
> +	conf->core.dev = &pdev->dev;
> +	conf->core.write = xilinx_selectmap_write;
> +
> +	base = devm_platform_get_and_ioremap_resource(pdev, 0, &r);
> +	if (IS_ERR(base))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(base),
> +				     "ioremap error\n");
> +	conf->base = base;
> +
> +	/* CSI_B is active low */
> +	csi_b = devm_gpiod_get_optional(&pdev->dev, "csi", GPIOD_OUT_HIGH);
> +	if (IS_ERR(csi_b))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(csi_b),
> +				     "Failed to get CSI_B gpio\n");
> +
> +	/* RDWR_B is active low */
> +	rdwr_b = devm_gpiod_get_optional(&pdev->dev, "rdwr", GPIOD_OUT_HIGH);
> +	if (IS_ERR(rdwr_b))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(rdwr_b),
> +				     "Failed to get RDWR_B gpio\n");
> +
> +	return xilinx_core_probe(&conf->core);
> +}
> +
> +static const struct of_device_id xlnx_selectmap_of_match[] = {
> +	{ .compatible = "xlnx,fpga-xc7s-selectmap", }, // Spartan-7
> +	{ .compatible = "xlnx,fpga-xc7a-selectmap", }, // Artix-7
> +	{ .compatible = "xlnx,fpga-xc7k-selectmap", }, // Kintex-7
> +	{ .compatible = "xlnx,fpga-xc7v-selectmap", }, // Virtex-7
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, xlnx_selectmap_of_match);

Does the driver have to be used with OF or not?

If yes, please specify the reason and enforce in Kconfig.
If no, please ensure it decently compiles without CONFIG_OF.

Thanks,
Yilun

> +
> +static struct platform_driver xilinx_selectmap_driver = {
> +	.driver = {
> +		.name = "xilinx-selectmap",
> +		.of_match_table = xlnx_selectmap_of_match,
> +	},
> +	.probe  = xilinx_selectmap_probe,
> +};
> +
> +module_platform_driver(xilinx_selectmap_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Charles Perry <charles.perry@savoirfairelinux.com>");
> +MODULE_DESCRIPTION("Load Xilinx FPGA firmware over SelectMap");
> -- 
> 2.43.0
> 
>