drivers/fpga/xilinx-selectmap.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-)
Add csi_b and rdwr pin support for the xilinx,selectmap driver.
In current driver the pins are configured from DTS and set to
GPIOD_OUT_HIGH. This works in case you have one FPGA.
Extend this to really implement csi_b and rdwr pin function in
driver, so it works with more than one FPGA.
Tested on AM625 based board with 2 FPGAs connected to GPMC.
Signed-off-by: Heiko Schocher <hs@nabladev.com>
---
drivers/fpga/xilinx-selectmap.c | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/drivers/fpga/xilinx-selectmap.c b/drivers/fpga/xilinx-selectmap.c
index d0cbb5fdfe3a..b5fbdedf8aec 100644
--- a/drivers/fpga/xilinx-selectmap.c
+++ b/drivers/fpga/xilinx-selectmap.c
@@ -19,6 +19,8 @@
struct xilinx_selectmap_conf {
struct xilinx_fpga_core core;
void __iomem *base;
+ struct gpio_desc *csi_b;
+ struct gpio_desc *rdwr_b;
};
#define to_xilinx_selectmap_conf(obj) \
@@ -30,9 +32,21 @@ static int xilinx_selectmap_write(struct xilinx_fpga_core *core,
struct xilinx_selectmap_conf *conf = to_xilinx_selectmap_conf(core);
size_t i;
+ if (conf->csi_b)
+ gpiod_set_value(conf->csi_b, GPIOD_OUT_HIGH);
+
+ if (conf->rdwr_b)
+ gpiod_set_value(conf->rdwr_b, GPIOD_OUT_HIGH);
+
for (i = 0; i < count; ++i)
writeb(buf[i], conf->base);
+ if (conf->rdwr_b)
+ gpiod_set_value(conf->rdwr_b, GPIOD_OUT_LOW);
+
+ if (conf->csi_b)
+ gpiod_set_value(conf->csi_b, GPIOD_OUT_LOW);
+
return 0;
}
@@ -56,16 +70,18 @@ static int xilinx_selectmap_probe(struct platform_device *pdev)
conf->base = base;
/* CSI_B is active low */
- gpio = devm_gpiod_get_optional(&pdev->dev, "csi", GPIOD_OUT_HIGH);
+ gpio = devm_gpiod_get_optional(&pdev->dev, "csi", GPIOD_OUT_LOW);
if (IS_ERR(gpio))
return dev_err_probe(&pdev->dev, PTR_ERR(gpio),
"Failed to get CSI_B gpio\n");
+ conf->csi_b = gpio;
/* RDWR_B is active low */
- gpio = devm_gpiod_get_optional(&pdev->dev, "rdwr", GPIOD_OUT_HIGH);
+ gpio = devm_gpiod_get_optional(&pdev->dev, "rdwr", GPIOD_OUT_LOW);
if (IS_ERR(gpio))
return dev_err_probe(&pdev->dev, PTR_ERR(gpio),
"Failed to get RDWR_B gpio\n");
+ conf->rdwr_b = gpio;
return xilinx_core_probe(&conf->core);
}
---
base-commit: 58717b2a1365d06c8c64b72aa948541b53fe31eb
--
2.55.0
On 7/16/26 09:00, Heiko Schocher wrote:
> Add csi_b and rdwr pin support for the xilinx,selectmap driver.
>
> In current driver the pins are configured from DTS and set to
> GPIOD_OUT_HIGH. This works in case you have one FPGA.
>
> Extend this to really implement csi_b and rdwr pin function in
> driver, so it works with more than one FPGA.
>
> Tested on AM625 based board with 2 FPGAs connected to GPMC.
>
> Signed-off-by: Heiko Schocher <hs@nabladev.com>
> ---
>
> drivers/fpga/xilinx-selectmap.c | 20 ++++++++++++++++++--
> 1 file changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/fpga/xilinx-selectmap.c b/drivers/fpga/xilinx-selectmap.c
> index d0cbb5fdfe3a..b5fbdedf8aec 100644
> --- a/drivers/fpga/xilinx-selectmap.c
> +++ b/drivers/fpga/xilinx-selectmap.c
> @@ -19,6 +19,8 @@
> struct xilinx_selectmap_conf {
> struct xilinx_fpga_core core;
> void __iomem *base;
> + struct gpio_desc *csi_b;
> + struct gpio_desc *rdwr_b;
> };
>
> #define to_xilinx_selectmap_conf(obj) \
> @@ -30,9 +32,21 @@ static int xilinx_selectmap_write(struct xilinx_fpga_core *core,
> struct xilinx_selectmap_conf *conf = to_xilinx_selectmap_conf(core);
> size_t i;
>
> + if (conf->csi_b)
Is this really needed?
4023 int gpiod_set_value(struct gpio_desc *desc, int value)
4024 {
4025 VALIDATE_DESC(desc);
4026 /* Should be using gpiod_set_value_cansleep() */
4027 WARN_ON(desc->gdev->can_sleep);
4028 return gpiod_set_value_nocheck(desc, value);
4029 }
4030 EXPORT_SYMBOL_GPL(gpiod_set_value);
398 #define VALIDATE_DESC(desc) do { \
399 int __valid = validate_desc(desc, __func__); \
400 if (__valid <= 0) \
401 return __valid; \
402 } while (0)
385 static int validate_desc(const struct gpio_desc *desc, const char *func)
386 {
387 if (!desc)
388 return 0;
389
390 if (IS_ERR(desc)) {
391 pr_warn("%s: invalid GPIO (errorpointer: %pe)\n", func, desc);
392 return PTR_ERR(desc);
393 }
394
395 return 1;
396 }
397
You should be able to just skip it because validate_desc is checking it for you.
> + gpiod_set_value(conf->csi_b, GPIOD_OUT_HIGH);
> +
> + if (conf->rdwr_b)
> + gpiod_set_value(conf->rdwr_b, GPIOD_OUT_HIGH);
> +
> for (i = 0; i < count; ++i)
> writeb(buf[i], conf->base);
>
> + if (conf->rdwr_b)
> + gpiod_set_value(conf->rdwr_b, GPIOD_OUT_LOW);
> +
> + if (conf->csi_b)
> + gpiod_set_value(conf->csi_b, GPIOD_OUT_LOW);
> +
> return 0;
> }
>
> @@ -56,16 +70,18 @@ static int xilinx_selectmap_probe(struct platform_device *pdev)
> conf->base = base;
>
> /* CSI_B is active low */
> - gpio = devm_gpiod_get_optional(&pdev->dev, "csi", GPIOD_OUT_HIGH);
> + gpio = devm_gpiod_get_optional(&pdev->dev, "csi", GPIOD_OUT_LOW);
> if (IS_ERR(gpio))
> return dev_err_probe(&pdev->dev, PTR_ERR(gpio),
> "Failed to get CSI_B gpio\n");
> + conf->csi_b = gpio;
Isn't it easier to simply call to avoid it?
conf->csi_b = devm_gpiod_get_optional(&pdev->dev, "csi", GPIOD_OUT_LOW);
Thanks,
Michal
Hello Michal,
On 20.07.26 14:40, Michal Simek wrote:
>
>
> On 7/16/26 09:00, Heiko Schocher wrote:
>> Add csi_b and rdwr pin support for the xilinx,selectmap driver.
>>
>> In current driver the pins are configured from DTS and set to
>> GPIOD_OUT_HIGH. This works in case you have one FPGA.
>>
>> Extend this to really implement csi_b and rdwr pin function in
>> driver, so it works with more than one FPGA.
>>
>> Tested on AM625 based board with 2 FPGAs connected to GPMC.
>>
>> Signed-off-by: Heiko Schocher <hs@nabladev.com>
>> ---
>>
>> drivers/fpga/xilinx-selectmap.c | 20 ++++++++++++++++++--
>> 1 file changed, 18 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/fpga/xilinx-selectmap.c b/drivers/fpga/xilinx-selectmap.c
>> index d0cbb5fdfe3a..b5fbdedf8aec 100644
>> --- a/drivers/fpga/xilinx-selectmap.c
>> +++ b/drivers/fpga/xilinx-selectmap.c
>> @@ -19,6 +19,8 @@
>> struct xilinx_selectmap_conf {
>> struct xilinx_fpga_core core;
>> void __iomem *base;
>> + struct gpio_desc *csi_b;
>> + struct gpio_desc *rdwr_b;
>> };
>> #define to_xilinx_selectmap_conf(obj) \
>> @@ -30,9 +32,21 @@ static int xilinx_selectmap_write(struct xilinx_fpga_core *core,
>> struct xilinx_selectmap_conf *conf = to_xilinx_selectmap_conf(core);
>> size_t i;
>> + if (conf->csi_b)
>
> Is this really needed?
>
> 4023 int gpiod_set_value(struct gpio_desc *desc, int value)
> 4024 {
> 4025 VALIDATE_DESC(desc);
> 4026 /* Should be using gpiod_set_value_cansleep() */
> 4027 WARN_ON(desc->gdev->can_sleep);
> 4028 return gpiod_set_value_nocheck(desc, value);
> 4029 }
> 4030 EXPORT_SYMBOL_GPL(gpiod_set_value);
>
> 398 #define VALIDATE_DESC(desc) do { \
> 399 int __valid = validate_desc(desc, __func__); \
> 400 if (__valid <= 0) \
> 401 return __valid; \
> 402 } while (0)
>
> 385 static int validate_desc(const struct gpio_desc *desc, const char *func)
> 386 {
> 387 if (!desc)
> 388 return 0;
> 389
> 390 if (IS_ERR(desc)) {
> 391 pr_warn("%s: invalid GPIO (errorpointer: %pe)\n", func, desc);
> 392 return PTR_ERR(desc);
> 393 }
> 394
> 395 return 1;
> 396 }
> 397
>
> You should be able to just skip it because validate_desc is checking it for you.
It seems this is possible yes, so I drop it!
>> + gpiod_set_value(conf->csi_b, GPIOD_OUT_HIGH);
>> +
>> + if (conf->rdwr_b)
>> + gpiod_set_value(conf->rdwr_b, GPIOD_OUT_HIGH);
>> +
>> for (i = 0; i < count; ++i)
>> writeb(buf[i], conf->base);
>> + if (conf->rdwr_b)
>> + gpiod_set_value(conf->rdwr_b, GPIOD_OUT_LOW);
>> +
>> + if (conf->csi_b)
>> + gpiod_set_value(conf->csi_b, GPIOD_OUT_LOW);
>> +
>> return 0;
>> }
>> @@ -56,16 +70,18 @@ static int xilinx_selectmap_probe(struct platform_device *pdev)
>> conf->base = base;
>> /* CSI_B is active low */
>> - gpio = devm_gpiod_get_optional(&pdev->dev, "csi", GPIOD_OUT_HIGH);
>> + gpio = devm_gpiod_get_optional(&pdev->dev, "csi", GPIOD_OUT_LOW);
>> if (IS_ERR(gpio))
>> return dev_err_probe(&pdev->dev, PTR_ERR(gpio),
>> "Failed to get CSI_B gpio\n");
>> + conf->csi_b = gpio;
>
> Isn't it easier to simply call to avoid it?
>
> conf->csi_b = devm_gpiod_get_optional(&pdev->dev, "csi", GPIOD_OUT_LOW);
Of course, will send a v2 and drop the local gpio variable as not
longer needed.
Thanks for the review!
bye,
Heiko
>
> Thanks,
> Michal
--
Nabla Software Engineering
HRB 40522 Augsburg
Phone: +49 821 45592596
E-Mail: office@nabladev.com
Geschäftsführer : Stefano Babic
© 2016 - 2026 Red Hat, Inc.