Hi Justin, thanks for your patch!
On Fri, Feb 8, 2019 at 8:25 PM <[email protected]> wrote: > From: Justin Chen <[email protected]> > > 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 <[email protected]> (...) > @@ -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
