Re: [PATCH] iio: adc: ti-ads7950: add GPIO support

[Linus Walleij <linus.walleij@xxxxxxxxxx>]

Hi Justin,

thanks for your patch!

On Fri, Feb 8, 2019 at 8:25 PM <justinpopo6@xxxxxxxxx> wrote:

> From: Justin Chen <justinpopo6@xxxxxxxxx>
>
> The ADS79XX has GPIO pins that can be used. Add support for the GPIO
> pins using the GPIO chip framework.
>
> Signed-off-by: Justin Chen <justinpopo6@xxxxxxxxx>
(...)

> @@ -56,11 +61,17 @@ struct ti_ads7950_state {
>         struct spi_message      ring_msg;
>         struct spi_message      scan_single_msg;
>
> +       struct iio_dev          *indio_dev;
> +       struct gpio_chip        *chip;

Why use a pointer here? Correct me if wrong by a struct ti_ads7950_state is
always allocated for each instance of the hardware right?
So just struct gpio_chip chip; should be fine, then just fill in that
struct.

> +       /* Add GPIO chip */
> +       st->chip->label = dev_name(&st->spi->dev);

So it would be st->chip.label = ... etc.

> +       chip = devm_kzalloc(&spi->dev, sizeof(*chip), GFP_KERNEL);
> +       if (!chip)
> +               return -ENOMEM;

And no need to do this.

Apart from that it looks OK to me, but there are some locking comments
I see.

Yours,
Linus Walleij