[v2] driver: fpga: xilinx-selectmap: add csi and rdwr support

[PATCH v2] driver: fpga: xilinx-selectmap: add csi and rdwr support

Posted by Heiko Schocher 3 days, 12 hours ago

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

Changes in v2:
- add comments from Michal
  - skip check if gpio descriptor variables csi_b/rdwr_b are valid,
    as validate_desc() checks this in gpiod_set_value() call.
  - initialize the gpio variables csi_b/rdwr_b immediately with
    the return value from devm_gpiod_get_optional(), so we can
    drop local gpio variable at all

 drivers/fpga/xilinx-selectmap.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/fpga/xilinx-selectmap.c b/drivers/fpga/xilinx-selectmap.c
index d0cbb5fdfe3a..d5175f9430e4 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,16 +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;
 
+	gpiod_set_value(conf->csi_b, GPIOD_OUT_HIGH);
+	gpiod_set_value(conf->rdwr_b, GPIOD_OUT_HIGH);
+
 	for (i = 0; i < count; ++i)
 		writeb(buf[i], conf->base);
 
+	gpiod_set_value(conf->rdwr_b, GPIOD_OUT_LOW);
+	gpiod_set_value(conf->csi_b, GPIOD_OUT_LOW);
+
 	return 0;
 }
 
 static int xilinx_selectmap_probe(struct platform_device *pdev)
 {
 	struct xilinx_selectmap_conf *conf;
-	struct gpio_desc *gpio;
 	void __iomem *base;
 
 	conf = devm_kzalloc(&pdev->dev, sizeof(*conf), GFP_KERNEL);
@@ -56,15 +63,17 @@ 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);
-	if (IS_ERR(gpio))
-		return dev_err_probe(&pdev->dev, PTR_ERR(gpio),
+	conf->csi_b = devm_gpiod_get_optional(&pdev->dev, "csi",
+					      GPIOD_OUT_LOW);
+	if (IS_ERR(conf->csi_b))
+		return dev_err_probe(&pdev->dev, PTR_ERR(conf->csi_b),
 				     "Failed to get CSI_B gpio\n");
 
 	/* RDWR_B is active low */
-	gpio = devm_gpiod_get_optional(&pdev->dev, "rdwr", GPIOD_OUT_HIGH);
-	if (IS_ERR(gpio))
-		return dev_err_probe(&pdev->dev, PTR_ERR(gpio),
+	conf->rdwr_b = devm_gpiod_get_optional(&pdev->dev, "rdwr",
+					       GPIOD_OUT_LOW);
+	if (IS_ERR(conf->rdwr_b))
+		return dev_err_probe(&pdev->dev, PTR_ERR(conf->rdwr_b),
 				     "Failed to get RDWR_B gpio\n");
 
 	return xilinx_core_probe(&conf->core);
---
base-commit: 58717b2a1365d06c8c64b72aa948541b53fe31eb

-- 
2.55.0

Re: [PATCH v2] driver: fpga: xilinx-selectmap: add csi and rdwr support

Posted by Xu Yilun 1 day, 19 hours ago

On Tue, Jul 21, 2026 at 01:42:52PM +0200, 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.

Please help specify in changelog: what do csi_b, rdwr pins do? Why they
now work with one FPGA but not with 2 FPGAs? How do you select which FPGA
to reprogram? thanks.

Thanks,
Yilun

Re: [PATCH v2] driver: fpga: xilinx-selectmap: add csi and rdwr support

Posted by Heiko Schocher 1 day, 19 hours ago

Hello Yilun,

On 23.07.26 06:42, Xu Yilun wrote:
> On Tue, Jul 21, 2026 at 01:42:52PM +0200, 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.
> 
> Please help specify in changelog: what do csi_b, rdwr pins do? Why they
> now work with one FPGA but not with 2 FPGAs? How do you select which FPGA
> to reprogram? thanks.

I will try to add this. You find some hint here:

Documentation/devicetree/bindings/fpga/xlnx,fpga-selectmap.yaml

Thanks!

bye,
Heiko
> 
> Thanks,
> Yilun
> 

-- 
Nabla Software Engineering
HRB 40522 Augsburg
Phone: +49 821 45592596
E-Mail: office@nabladev.com
Geschäftsführer : Stefano Babic

Re: [PATCH v2] driver: fpga: xilinx-selectmap: add csi and rdwr support

Posted by Michal Simek 2 days, 17 hours ago


On 7/21/26 13:42, 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>
> ---
> 
> Changes in v2:
> - add comments from Michal
>    - skip check if gpio descriptor variables csi_b/rdwr_b are valid,
>      as validate_desc() checks this in gpiod_set_value() call.
>    - initialize the gpio variables csi_b/rdwr_b immediately with
>      the return value from devm_gpiod_get_optional(), so we can
>      drop local gpio variable at all
> 
>   drivers/fpga/xilinx-selectmap.c | 23 ++++++++++++++++-------
>   1 file changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/fpga/xilinx-selectmap.c b/drivers/fpga/xilinx-selectmap.c
> index d0cbb5fdfe3a..d5175f9430e4 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,16 +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;
>   
> +	gpiod_set_value(conf->csi_b, GPIOD_OUT_HIGH);
> +	gpiod_set_value(conf->rdwr_b, GPIOD_OUT_HIGH);

I think GPIOD_OUT_HIGH/LOW is wrong in this context.
If you do "git grep gpiod_set_value" you will see that values are 0/1

Thanks,
Michal

Re: [PATCH v2] driver: fpga: xilinx-selectmap: add csi and rdwr support

Posted by Heiko Schocher 2 days, 9 hours ago

Hello Michal,

On 22.07.26 08:55, Michal Simek wrote:
> 
> 
> On 7/21/26 13:42, 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>
>> ---
>>
>> Changes in v2:
>> - add comments from Michal
>>    - skip check if gpio descriptor variables csi_b/rdwr_b are valid,
>>      as validate_desc() checks this in gpiod_set_value() call.
>>    - initialize the gpio variables csi_b/rdwr_b immediately with
>>      the return value from devm_gpiod_get_optional(), so we can
>>      drop local gpio variable at all
>>
>>   drivers/fpga/xilinx-selectmap.c | 23 ++++++++++++++++-------
>>   1 file changed, 16 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/fpga/xilinx-selectmap.c b/drivers/fpga/xilinx-selectmap.c
>> index d0cbb5fdfe3a..d5175f9430e4 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,16 +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;
>> +    gpiod_set_value(conf->csi_b, GPIOD_OUT_HIGH);
>> +    gpiod_set_value(conf->rdwr_b, GPIOD_OUT_HIGH);
> 
> I think GPIOD_OUT_HIGH/LOW is wrong in this context.
> If you do "git grep gpiod_set_value" you will see that values are 0/1
I will change it!

Hmm.. while thinking about it... CSI_B and RDWR_B signals
are defined as active low...

Doesn't it make more sense to use here

gpiod_set_raw_value()

instead, which sets the physical levels?

Thanks!

bye,
Heiko
> 
> Thanks,
> Michal

-- 
Nabla Software Engineering
HRB 40522 Augsburg
Phone: +49 821 45592596
E-Mail: office@nabladev.com
Geschäftsführer : Stefano Babic

Re: [PATCH v2] driver: fpga: xilinx-selectmap: add csi and rdwr support

Posted by Michal Simek 1 day, 18 hours ago


On 7/22/26 16:45, Heiko Schocher wrote:
> Hello Michal,
> 
> On 22.07.26 08:55, Michal Simek wrote:
>>
>>
>> On 7/21/26 13:42, 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>
>>> ---
>>>
>>> Changes in v2:
>>> - add comments from Michal
>>>    - skip check if gpio descriptor variables csi_b/rdwr_b are valid,
>>>      as validate_desc() checks this in gpiod_set_value() call.
>>>    - initialize the gpio variables csi_b/rdwr_b immediately with
>>>      the return value from devm_gpiod_get_optional(), so we can
>>>      drop local gpio variable at all
>>>
>>>   drivers/fpga/xilinx-selectmap.c | 23 ++++++++++++++++-------
>>>   1 file changed, 16 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/fpga/xilinx-selectmap.c b/drivers/fpga/xilinx-selectmap.c
>>> index d0cbb5fdfe3a..d5175f9430e4 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,16 +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;
>>> +    gpiod_set_value(conf->csi_b, GPIOD_OUT_HIGH);
>>> +    gpiod_set_value(conf->rdwr_b, GPIOD_OUT_HIGH);
>>
>> I think GPIOD_OUT_HIGH/LOW is wrong in this context.
>> If you do "git grep gpiod_set_value" you will see that values are 0/1
> I will change it!
> 
> Hmm.. while thinking about it... CSI_B and RDWR_B signals
> are defined as active low...

I have never worked with this driver but values here should be 0 or 1. And if 
that means active low/high is said via DT.

GPIOD_OUT_LOW below should mean that you are starting in deassert state (if it 
is low/high is coming from DT). And before write you assert that line and 
deassert after it.

Thanks,
Michal

Re: [PATCH v2] driver: fpga: xilinx-selectmap: add csi and rdwr support

Posted by Heiko Schocher 1 day, 18 hours ago

Hello Michal,

On 23.07.26 07:44, Michal Simek wrote:
> 
> 
> On 7/22/26 16:45, Heiko Schocher wrote:
>> Hello Michal,
>>
>> On 22.07.26 08:55, Michal Simek wrote:
>>>
>>>
>>> On 7/21/26 13:42, 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>
>>>> ---
>>>>
>>>> Changes in v2:
>>>> - add comments from Michal
>>>>    - skip check if gpio descriptor variables csi_b/rdwr_b are valid,
>>>>      as validate_desc() checks this in gpiod_set_value() call.
>>>>    - initialize the gpio variables csi_b/rdwr_b immediately with
>>>>      the return value from devm_gpiod_get_optional(), so we can
>>>>      drop local gpio variable at all
>>>>
>>>>   drivers/fpga/xilinx-selectmap.c | 23 ++++++++++++++++-------
>>>>   1 file changed, 16 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/drivers/fpga/xilinx-selectmap.c b/drivers/fpga/xilinx-selectmap.c
>>>> index d0cbb5fdfe3a..d5175f9430e4 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,16 +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;
>>>> +    gpiod_set_value(conf->csi_b, GPIOD_OUT_HIGH);
>>>> +    gpiod_set_value(conf->rdwr_b, GPIOD_OUT_HIGH);
>>>
>>> I think GPIOD_OUT_HIGH/LOW is wrong in this context.
>>> If you do "git grep gpiod_set_value" you will see that values are 0/1
>> I will change it!
>>
>> Hmm.. while thinking about it... CSI_B and RDWR_B signals
>> are defined as active low...
> 
> I have never worked with this driver but values here should be 0 or 1. And if that means active 
> low/high is said via DT.
> 
> GPIOD_OUT_LOW below should mean that you are starting in deassert state (if it is low/high is coming 
> from DT). And before write you assert that line and deassert after it.

Fine for me, I rework it, thanks!

bye,
Heiko
> 
> Thanks,
> Michal
> 
> 

-- 
Nabla Software Engineering
HRB 40522 Augsburg
Phone: +49 821 45592596
E-Mail: office@nabladev.com
Geschäftsführer : Stefano Babic