On Sat, Nov 21, 2020 at 5:38 PM Jonathan Cameron <ji...@kernel.org> wrote: > > On Thu, 19 Nov 2020 12:07:45 +0200 > Alexandru Ardelean <alexandru.ardel...@analog.com> wrote: > > > This change converts the configuration of the dual-channel mode from the > > old platform-data, to the device_property_present() function, which > > supports both device-tree and ACPI configuration setups. > > > > With this change the old platform_data include of the driver can be > > removed. > > > > Signed-off-by: Alexandru Ardelean <alexandru.ardel...@analog.com> > > Hi Alexandru, > > The set looks good in general, but there is a an oddity in the driver. > If we don't provide the vref regulator, then the scale is set to reflect > the internal 2.5V reference. Fiven the external reference only > works by overdriving the 2.5V (must be higher than that to have > an effect) I guess we could in theory clamp it to a minimum of > 2.5V but anyone wiring up less than that would have built a crazy board > so we can probably ignore it. > > However, as I read the datasheet in dual channel mode it should be set > to VDD not 2.5V. Right now you could make it work in a DT file > by setting VREF==VDD regulator but that's inelegant. > > If you agree with my logic, perhaps a follow up patch?
I haven't managed to get around to this one. Will try next week. > > Jonathan > > > > --- > > > > I'm wondering if this changeset is what was in mind here: > > > > https://lore.kernel.org/linux-iio/CA+U=DsqF5tu8Be9KXeyCWD2uHvV688Nc3n=z_xi2j6h6dfj...@mail.gmail.com/T/#mbe72e4da3acea3899d0d35402ea81e52a9bc34e6 > > This driver could have been simplified/reduced a whole lot more, but I'm > > not sure about it. It's a bit of patch-noise, and later > > > > Changelog v1 -> v2: > > * dropped patch 'iio: adc: ad7887: convert driver to full DT probing' > > not adding the device_get_match_data() logic anymore > > * added patch 'iio: adc: ad7887: remove matching code from driver' > > hooking the chip info directly to AD7887 > > * added patch 'iio: adc: ad7887: add OF match table' > > this just adds an OF table for DT and ACPI > > > > drivers/iio/adc/ad7887.c | 10 +++++----- > > include/linux/platform_data/ad7887.h | 21 --------------------- > > 2 files changed, 5 insertions(+), 26 deletions(-) > > delete mode 100644 include/linux/platform_data/ad7887.h > > > > diff --git a/drivers/iio/adc/ad7887.c b/drivers/iio/adc/ad7887.c > > index 4f6f0e0e03ee..06f684c053a0 100644 > > --- a/drivers/iio/adc/ad7887.c > > +++ b/drivers/iio/adc/ad7887.c > > @@ -23,8 +23,6 @@ > > #include <linux/iio/trigger_consumer.h> > > #include <linux/iio/triggered_buffer.h> > > > > -#include <linux/platform_data/ad7887.h> > > - > > #define AD7887_REF_DIS BIT(5) /* on-chip reference disable > > */ > > #define AD7887_DUAL BIT(4) /* dual-channel mode */ > > #define AD7887_CH_AIN1 BIT(3) /* convert on channel 1, > > DUAL=1 */ > > @@ -241,9 +239,9 @@ static void ad7887_reg_disable(void *data) > > > > static int ad7887_probe(struct spi_device *spi) > > { > > - struct ad7887_platform_data *pdata = spi->dev.platform_data; > > struct ad7887_state *st; > > struct iio_dev *indio_dev; > > + bool dual_mode; > > uint8_t mode; > > int ret; > > > > @@ -286,7 +284,9 @@ static int ad7887_probe(struct spi_device *spi) > > mode = AD7887_PM_MODE4; > > if (!st->reg) > > mode |= AD7887_REF_DIS; > > - if (pdata && pdata->en_dual) > > + > > + dual_mode = device_property_present(&spi->dev, > > "adi,dual-channel-mode"); > > + if (dual_mode) > > mode |= AD7887_DUAL; > > > > st->tx_cmd_buf[0] = AD7887_CH_AIN0 | mode; > > @@ -298,7 +298,7 @@ static int ad7887_probe(struct spi_device *spi) > > spi_message_init(&st->msg[AD7887_CH0]); > > spi_message_add_tail(&st->xfer[0], &st->msg[AD7887_CH0]); > > > > - if (pdata && pdata->en_dual) { > > + if (dual_mode) { > > st->tx_cmd_buf[2] = AD7887_CH_AIN1 | mode; > > > > st->xfer[1].rx_buf = &st->data[0]; > > diff --git a/include/linux/platform_data/ad7887.h > > b/include/linux/platform_data/ad7887.h > > deleted file mode 100644 > > index 9b4dca6ae70b..000000000000 > > --- a/include/linux/platform_data/ad7887.h > > +++ /dev/null > > @@ -1,21 +0,0 @@ > > -/* SPDX-License-Identifier: GPL-2.0-or-later */ > > -/* > > - * AD7887 SPI ADC driver > > - * > > - * Copyright 2010 Analog Devices Inc. > > - */ > > -#ifndef IIO_ADC_AD7887_H_ > > -#define IIO_ADC_AD7887_H_ > > - > > -/** > > - * struct ad7887_platform_data - AD7887 ADC driver platform data > > - * @en_dual: Whether to use dual channel mode. If set to true AIN1 becomes > > the > > - * second input channel, and Vref is internally connected to Vdd. If set > > to > > - * false the device is used in single channel mode and AIN1/Vref is used > > as > > - * VREF input. > > - */ > > -struct ad7887_platform_data { > > - bool en_dual; > > -}; > > - > > -#endif /* IIO_ADC_AD7887_H_ */ >