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
