Re: [PATCH 17/21] nfc: marvell: convert to gpio descriptors

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Fri, Aug 08, 2025 at 05:18:01PM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> The only reason this driver seems to still use the legacy gpio
> numbers is for the module parameter that lets users pass a different
> reset gpio.
> 
> Since the fixed numbers are on their way out, and none of the platforms
> this driver is used on would have them any more, remove the module
> parameter and instead just use the reset information from firmware.

This patch is my love in the series, thanks for doing it!
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

But note some comments below.

...

> -	if (gpio_is_valid(priv->config.reset_n_io)) {
> -		rc = gpio_request_one(priv->config.reset_n_io,
> -				      GPIOF_OUT_INIT_LOW,
> -				      "nfcmrvl_reset_n");
> -		if (rc < 0) {
> -			priv->config.reset_n_io = -EINVAL;
> -			nfc_err(dev, "failed to request reset_n io\n");
> -		}
> +	priv->reset_n_io = gpiod_get_optional(dev, "reset-n-io", GPIOD_OUT_LOW);
> +	if (IS_ERR(priv->reset_n_io)) {
> +		nfc_err(dev, "failed to get reset_n gpio\n");
> +		return ERR_CAST(priv->reset_n_io);
>  	}

This also needs a call to gpiod_set_consumer_name(), IIRC the API name.

...

> -	if (gpio_is_valid(priv->config.reset_n_io)) {
> -		nfc_info(priv->dev, "reset the chip\n");
> -		gpio_set_value(priv->config.reset_n_io, 0);
> -		usleep_range(5000, 10000);
> -		gpio_set_value(priv->config.reset_n_io, 1);
> -	} else
> -		nfc_info(priv->dev, "no reset available on this interface\n");
> +	nfc_info(priv->dev, "reset the chip\n");
> +	gpiod_set_value(priv->reset_n_io, 0);
> +	usleep_range(5000, 10000);

Side note, this would be nice to use fsleep(), but I see that's just a
copy'n'paste of the original piece.

> +	gpiod_set_value(priv->reset_n_io, 1);

...

>  void nfcmrvl_chip_halt(struct nfcmrvl_private *priv)
>  {
> -	if (gpio_is_valid(priv->config.reset_n_io))
> -		gpio_set_value(priv->config.reset_n_io, 0);
> +	if (priv->reset_n_io)

Not sure why we need this dup check.

> +		gpiod_set_value(priv->reset_n_io, 0);
>  }

-- 
With Best Regards,
Andy Shevchenko