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 <[email protected]>
---
Changes since v4: (from Yilun review)
* xilinx-core: select between prog/init and prog_b/init-b
drivers/fpga/Kconfig | 8 +++
drivers/fpga/Makefile | 1 +
drivers/fpga/xilinx-core.c | 29 +++++++++-
drivers/fpga/xilinx-selectmap.c | 97 +++++++++++++++++++++++++++++++++
4 files changed, 133 insertions(+), 2 deletions(-)
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-core.c b/drivers/fpga/xilinx-core.c
index a35c43382dd5f..ccdeb45eba4ee 100644
--- a/drivers/fpga/xilinx-core.c
+++ b/drivers/fpga/xilinx-core.c
@@ -171,6 +171,29 @@ static int xilinx_core_write_complete(struct fpga_manager *mgr,
return -ETIMEDOUT;
}
+/**
+ * xilinx_core_devm_gpiod_get - Obtain a resource-managed GPIO using a
+ * legacy consumer name fallback.
+ *
+ * @dev: Device managing the GPIO
+ * @con_id: Consumer id
+ * @legacy_con_id: Legacy consumer id
+ * @flags: optional GPIO initialization flags
+ */
+static inline struct gpio_desc *
+xilinx_core_devm_gpiod_get(struct device *dev, const char *con_id,
+ const char *legacy_con_id, enum gpiod_flags flags)
+{
+ struct gpio_desc *desc;
+
+ desc = devm_gpiod_get(dev, con_id, flags);
+ if (IS_ERR(desc) && PTR_ERR(desc) == -ENOENT &&
+ of_device_is_compatible(dev->of_node, "xlnx,fpga-slave-serial"))
+ desc = devm_gpiod_get(dev, legacy_con_id, flags);
+
+ return desc;
+}
+
static const struct fpga_manager_ops xilinx_core_ops = {
.state = xilinx_core_state,
.write_init = xilinx_core_write_init,
@@ -186,12 +209,14 @@ int xilinx_core_probe(struct xilinx_fpga_core *core)
return -EINVAL;
/* PROGRAM_B is active low */
- core->prog_b = devm_gpiod_get(core->dev, "prog_b", GPIOD_OUT_LOW);
+ core->prog_b = xilinx_core_devm_gpiod_get(core->dev, "prog", "prog_b",
+ GPIOD_OUT_LOW);
if (IS_ERR(core->prog_b))
return dev_err_probe(core->dev, PTR_ERR(core->prog_b),
"Failed to get PROGRAM_B gpio\n");
- core->init_b = devm_gpiod_get_optional(core->dev, "init-b", GPIOD_IN);
+ core->init_b = xilinx_core_devm_gpiod_get(core->dev, "init", "init-b",
+ GPIOD_IN);
if (IS_ERR(core->init_b))
return dev_err_probe(core->dev, PTR_ERR(core->init_b),
"Failed to get INIT_B gpio\n");
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 <[email protected]>
+ *
+ * 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);
+
+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 <[email protected]>");
+MODULE_DESCRIPTION("Load Xilinx FPGA firmware over SelectMap");
--
2.43.0
On Wed, Mar 13, 2024 at 06:57:37PM -0400, 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 <[email protected]>
> ---
> Changes since v4: (from Yilun review)
> * xilinx-core: select between prog/init and prog_b/init-b
>
> drivers/fpga/Kconfig | 8 +++
> drivers/fpga/Makefile | 1 +
> drivers/fpga/xilinx-core.c | 29 +++++++++-
> drivers/fpga/xilinx-selectmap.c | 97 +++++++++++++++++++++++++++++++++
> 4 files changed, 133 insertions(+), 2 deletions(-)
> 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-core.c b/drivers/fpga/xilinx-core.c
> index a35c43382dd5f..ccdeb45eba4ee 100644
> --- a/drivers/fpga/xilinx-core.c
> +++ b/drivers/fpga/xilinx-core.c
> @@ -171,6 +171,29 @@ static int xilinx_core_write_complete(struct fpga_manager *mgr,
> return -ETIMEDOUT;
> }
>
> +/**
> + * xilinx_core_devm_gpiod_get - Obtain a resource-managed GPIO using a
> + * legacy consumer name fallback.
> + *
> + * @dev: Device managing the GPIO
> + * @con_id: Consumer id
> + * @legacy_con_id: Legacy consumer id
> + * @flags: optional GPIO initialization flags
> + */
No need to have kernel doc comments for internal functions.
> +static inline struct gpio_desc *
> +xilinx_core_devm_gpiod_get(struct device *dev, const char *con_id,
> + const char *legacy_con_id, enum gpiod_flags flags)
> +{
> + struct gpio_desc *desc;
> +
> + desc = devm_gpiod_get(dev, con_id, flags);
> + if (IS_ERR(desc) && PTR_ERR(desc) == -ENOENT &&
> + of_device_is_compatible(dev->of_node, "xlnx,fpga-slave-serial"))
> + desc = devm_gpiod_get(dev, legacy_con_id, flags);
> +
> + return desc;
> +}
> +
> static const struct fpga_manager_ops xilinx_core_ops = {
> .state = xilinx_core_state,
> .write_init = xilinx_core_write_init,
> @@ -186,12 +209,14 @@ int xilinx_core_probe(struct xilinx_fpga_core *core)
> return -EINVAL;
>
> /* PROGRAM_B is active low */
> - core->prog_b = devm_gpiod_get(core->dev, "prog_b", GPIOD_OUT_LOW);
> + core->prog_b = xilinx_core_devm_gpiod_get(core->dev, "prog", "prog_b",
> + GPIOD_OUT_LOW);
> if (IS_ERR(core->prog_b))
> return dev_err_probe(core->dev, PTR_ERR(core->prog_b),
> "Failed to get PROGRAM_B gpio\n");
>
> - core->init_b = devm_gpiod_get_optional(core->dev, "init-b", GPIOD_IN);
> + core->init_b = xilinx_core_devm_gpiod_get(core->dev, "init", "init-b",
> + GPIOD_IN);
> if (IS_ERR(core->init_b))
> return dev_err_probe(core->dev, PTR_ERR(core->init_b),
> "Failed to get INIT_B gpio\n");
Please make a separate patch for the naming change. This give a chance
to explain why the change and how to correctly use the GPIO names.
> 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 <[email protected]>
> + *
> + * 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>
alphabetical order please.
> +
> +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;
One gpio_desc is enough, is it? We don't use these gpio_desc anywhere
else.
BTW, reverse Xmas tree is not strictly required, but please do it when it is easy to follow.
> +
> + 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);
I can't find where 'r' is used.
Thanks,
Yilun
> + 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);
> +
> +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 <[email protected]>");
> +MODULE_DESCRIPTION("Load Xilinx FPGA firmware over SelectMap");
> --
> 2.43.0
>
On Mar 19, 2024, at 2:35 AM, Xu Yilun [email protected] wrote:
> On Wed, Mar 13, 2024 at 06:57:37PM -0400, 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 <[email protected]>
>> ---
>> Changes since v4: (from Yilun review)
>> * xilinx-core: select between prog/init and prog_b/init-b
>>
>> drivers/fpga/Kconfig | 8 +++
>> drivers/fpga/Makefile | 1 +
>> drivers/fpga/xilinx-core.c | 29 +++++++++-
>> drivers/fpga/xilinx-selectmap.c | 97 +++++++++++++++++++++++++++++++++
>> 4 files changed, 133 insertions(+), 2 deletions(-)
>> 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-core.c b/drivers/fpga/xilinx-core.c
>> index a35c43382dd5f..ccdeb45eba4ee 100644
>> --- a/drivers/fpga/xilinx-core.c
>> +++ b/drivers/fpga/xilinx-core.c
>> @@ -171,6 +171,29 @@ static int xilinx_core_write_complete(struct fpga_manager
>> *mgr,
>> return -ETIMEDOUT;
>> }
>>
>> +/**
>> + * xilinx_core_devm_gpiod_get - Obtain a resource-managed GPIO using a
>> + * legacy consumer name fallback.
>> + *
>> + * @dev: Device managing the GPIO
>> + * @con_id: Consumer id
>> + * @legacy_con_id: Legacy consumer id
>> + * @flags: optional GPIO initialization flags
>> + */
>
> No need to have kernel doc comments for internal functions.
>
Ok, will remove.
>> +static inline struct gpio_desc *
>> +xilinx_core_devm_gpiod_get(struct device *dev, const char *con_id,
>> + const char *legacy_con_id, enum gpiod_flags flags)
>> +{
>> + struct gpio_desc *desc;
>> +
>> + desc = devm_gpiod_get(dev, con_id, flags);
>> + if (IS_ERR(desc) && PTR_ERR(desc) == -ENOENT &&
>> + of_device_is_compatible(dev->of_node, "xlnx,fpga-slave-serial"))
>> + desc = devm_gpiod_get(dev, legacy_con_id, flags);
>> +
>> + return desc;
>> +}
>> +
>> static const struct fpga_manager_ops xilinx_core_ops = {
>> .state = xilinx_core_state,
>> .write_init = xilinx_core_write_init,
>> @@ -186,12 +209,14 @@ int xilinx_core_probe(struct xilinx_fpga_core *core)
>> return -EINVAL;
>>
>> /* PROGRAM_B is active low */
>> - core->prog_b = devm_gpiod_get(core->dev, "prog_b", GPIOD_OUT_LOW);
>> + core->prog_b = xilinx_core_devm_gpiod_get(core->dev, "prog", "prog_b",
>> + GPIOD_OUT_LOW);
>> if (IS_ERR(core->prog_b))
>> return dev_err_probe(core->dev, PTR_ERR(core->prog_b),
>> "Failed to get PROGRAM_B gpio\n");
>>
>> - core->init_b = devm_gpiod_get_optional(core->dev, "init-b", GPIOD_IN);
>> + core->init_b = xilinx_core_devm_gpiod_get(core->dev, "init", "init-b",
>> + GPIOD_IN);
>> if (IS_ERR(core->init_b))
>> return dev_err_probe(core->dev, PTR_ERR(core->init_b),
>> "Failed to get INIT_B gpio\n");
>
> Please make a separate patch for the naming change. This give a chance
> to explain why the change and how to correctly use the GPIO names.
>
Ok
>> 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 <[email protected]>
>> + *
>> + * 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>
>
> alphabetical order please.
>
Ok
>> +
>> +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;
>
> One gpio_desc is enough, is it? We don't use these gpio_desc anywhere
> else.
>
Ok
> BTW, reverse Xmas tree is not strictly required, but please do it when it is
> easy to follow.
>
Ok
>> +
>> + 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);
>
> I can't find where 'r' is used.
>
> Thanks,
> Yilun
>
I'm guilty of keeping things around in case I need them in the future
here! Ok, will use NULL.
Regards,
Charles
>> + 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);
>> +
>> +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 <[email protected]>");
>> +MODULE_DESCRIPTION("Load Xilinx FPGA firmware over SelectMap");
>> --
>> 2.43.0