Re: [PATCH] staging: iio: ad7816: Switch to the gpio descriptor interface

From: Nishad Kamdar
Date: Wed Oct 17 2018 - 10:43:21 EST


On Tue, Oct 16, 2018 at 05:10:14PM +0200, Lars-Peter Clausen wrote:
> On 10/16/2018 04:46 PM, Nishad Kamdar wrote:
> > Use the gpiod interface for rdwr_pin, convert_pin and busy_pin
> > instead of the deprecated old non-descriptor interface.
> >
> > Signed-off-by: Nishad Kamdar <nishadkamdar@xxxxxxxxx>
>
> Hi,
>
> Thanks for the patch, this looks good.
>
> One thing about the error messages though.
>
> > + chip->rdwr_pin = devm_gpiod_get(&spi_dev->dev, "rdwr", GPIOD_IN);
> > + if (IS_ERR(chip->rdwr_pin)) {
> > + ret = PTR_ERR(chip->rdwr_pin);
> > dev_err(&spi_dev->dev, "Fail to request rdwr gpio PIN %d.\n",
> > - chip->rdwr_pin);
> > + ret);
>
> This previously showed the pin number which has now been replaced with the
> error code. The message doesn't make that much sense semantically anymore.
> Maybe replace it with something like
>
> "Failed to request rdwr GPIO: %d\n", ret
>
> > return ret;

Hello,

Ok, I'll do that.

Thanks for the review.

regards,
Nishad