On Tue, 10 Jan 2023 19:50:42 +0100 Christophe JAILLET <christophe.jaillet@xxxxxxxxxx> wrote: > Le 10/01/2023 à 17:01, Hugo Villeneuve a écrit : > > From: Hugo Villeneuve <hvilleneuve@xxxxxxxxxxxx> > > > > The Texas Instruments ADS7924 is a 4 channels, 12-bit analog to > > digital converter (ADC) with an I2C interface. > > > > Datasheet: https://www.ti.com/lit/gpn/ads7924 > > > > Signed-off-by: Hugo Villeneuve <hvilleneuve@xxxxxxxxxxxx> > > --- > > Hi, > > should there be a v3, a few nits below. Hi, there will definitely be a V3 (missing include), thanks for the input, I will incorporate these changes into V3. Hugo. > > CJ > > > MAINTAINERS | 7 + > > drivers/iio/adc/Kconfig | 11 + > > drivers/iio/adc/Makefile | 1 + > > drivers/iio/adc/ti-ads7924.c | 492 +++++++++++++++++++++++++++++++++++ > > 4 files changed, 511 insertions(+) > > create mode 100644 drivers/iio/adc/ti-ads7924.c > > > > [...] > > > +static int ads7924_get_channels_config(struct i2c_client *client, > > + struct iio_dev *indio_dev) > > +{ > > + struct ads7924_data *priv = iio_priv(indio_dev); > > + struct device *dev = priv->dev; > > + struct fwnode_handle *node; > > + int num_channels = 0; > > + > > + device_for_each_child_node(dev, node) { > > + u32 pval; > > + unsigned int channel; > > + > > + if (fwnode_property_read_u32(node, "reg", &pval)) { > > + dev_err(dev, "invalid reg on %pfw\n", node); > > + continue; > > + } > > + > > + channel = pval; > > + if (channel >= ADS7924_CHANNELS) { > > + dev_err(dev, "invalid channel index %d on %pfw\n", > > + channel, node); > > + continue; > > + } > > + > > + num_channels++; > > + } > > + > > + if (num_channels > 0) { > > + dev_dbg(dev, "found %d ADC channels\n", num_channels); > > + return 0; > > + } else { > > + return -EINVAL; > > + } > > if (num_channels <= 0) > return -EINVAL; > > dev_dbg(dev, "found %d ADC channels\n", num_channels); > return 0; > > is much more usual. > > > +} > > + > > [...] > > > +static int ads7924_reset(struct iio_dev *indio_dev) > > +{ > > + struct ads7924_data *data = iio_priv(indio_dev); > > + > > + if (data->reset_gpio) { > > + gpiod_set_value(data->reset_gpio, 1); /* Assert. */ > > + /* Educated guess: assert time not specified in datasheet... */ > > + mdelay(100); > > + gpiod_set_value(data->reset_gpio, 0); /* Deassert. */ > > + } else { > > + int ret; > > having 'ret' near 'struct ads7924_data *data' is more usual and saves 1 LoC. > > > + > > + /* > > + * A write of 10101010 to this register will generate a > > + * software reset of the ADS7924. > > + */ > > + ret = regmap_write(data->regmap, ADS7924_RESET_REG, 0b10101010); > > + if (ret) > > + return ret; > > + } > > + > > + return 0; > > +}; > > -- Hugo Villeneuve <hugo@xxxxxxxxxxx>
