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

[Michal Simek <michal.simek@amd.com>]

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