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

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

On Mon, Aug 11, 2025 at 02:43:51PM -0700, Dmitry Torokhov wrote:
> On Sat, Aug 09, 2025 at 01:09:47PM +0300, Andy Shevchenko wrote:
> > On Fri, Aug 08, 2025 at 05:18:01PM +0200, Arnd Bergmann wrote:

...

> > > -	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);
> 
> No, this should be "reset". gpiolib-of.c has a quirk to resolve to naked
> "reset-n-io", otherwise this will resolve to "reset-n-io-gpios" in the
> bowels of gpiolib.

Good point.

> > > +	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.
> 
> It does not have to... I am not sure who pays attention to names.

It goes to user space, isn't it?
In any case it will give 1:1 transition from the look&fell perspective.

...

> > >  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.
> 
> I personally feel very uneasy when dealing with optional GPIO and not
> checking if it exists or not, even though gpiod_set_value() handles
> this. I think check makes logic clearer.

I disagree with the duplicate. It doesn't make any additional clearness as I
read it. When one reads the code the "here we set GPIO to the X state" without
any conditional is fine as one may check later in DT schema if the GPIO is
optional or not.

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



-- 
With Best Regards,
Andy Shevchenko