* [PATCH v3 0/4] staging: iio: ad7780: move out of staging @ 2019-02-05 17:12 Renato Lui Geh 2019-02-05 17:13 ` [PATCH v3 1/4] staging: iio: ad7780: add gain & filter gpio support Renato Lui Geh ` (3 more replies) 0 siblings, 4 replies; 15+ messages in thread From: Renato Lui Geh @ 2019-02-05 17:12 UTC (permalink / raw) To: lars, Michael.Hennerich, jic23, knaack.h, pmeerw, gregkh, stefan.popa, alexandru.Ardelean, giuliano.belinassi Cc: linux-iio, devel, linux-kernel, kernel-usp This series of patches adds user input to ad7780 'gain' & 'filter' gpio pins, moves regulator initialization to after gpio initialization to maintain consistency between probe and remove, adds SPDX to the driver's license, and moves the ad7780 to the mainline. Renato Lui Geh (4): staging: iio: ad7780: add gain & filter gpio support staging: iio: ad7780: move regulator to after GPIO init staging: iio: ad7780: add SPDX identifier staging: iio: ad7780: moving ad7780 out of staging Changelog: *v3 - SPDX and regulator init as patches - Renamed filter to odr and ad778x_filter to ad778x_odr_avail - Removed unnecessary regulator disabling - Removed unnecessary AD_SD_CHANNEL macro - Changed unsigned int to unsigned long long to avoid overflow drivers/iio/adc/Kconfig | 13 ++ drivers/iio/adc/Makefile | 1 + drivers/iio/adc/ad7780.c | 359 +++++++++++++++++++++++++++++++ drivers/staging/iio/adc/Kconfig | 13 -- drivers/staging/iio/adc/Makefile | 1 - drivers/staging/iio/adc/ad7780.c | 277 ------------------------ 6 files changed, 373 insertions(+), 291 deletions(-) create mode 100644 drivers/iio/adc/ad7780.c delete mode 100644 drivers/staging/iio/adc/ad7780.c -- 2.20.1 ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v3 1/4] staging: iio: ad7780: add gain & filter gpio support 2019-02-05 17:12 [PATCH v3 0/4] staging: iio: ad7780: move out of staging Renato Lui Geh @ 2019-02-05 17:13 ` Renato Lui Geh 2019-02-05 19:58 ` Peter Meerwald-Stadler 2019-02-09 16:25 ` Jonathan Cameron 2019-02-05 17:13 ` [PATCH v3 2/4] staging: iio: ad7780: move regulator to after GPIO init Renato Lui Geh ` (2 subsequent siblings) 3 siblings, 2 replies; 15+ messages in thread From: Renato Lui Geh @ 2019-02-05 17:13 UTC (permalink / raw) To: lars, Michael.Hennerich, jic23, knaack.h, pmeerw, gregkh, stefan.popa, alexandru.Ardelean, giuliano.belinassi Cc: linux-iio, devel, linux-kernel, kernel-usp Previously, the AD7780 driver only supported gpio for the 'powerdown' pin. This commit adds suppport for the 'gain' and 'filter' pin. Signed-off-by: Renato Lui Geh <renatogeh@gmail.com> Signed-off-by: Giuliano Belinassi <giuliano.belinassi@usp.br> Co-developed-by: Giuliano Belinassi <giuliano.belinassi@usp.br> --- Changes in v3: - Renamed ad7780_chip_info's filter to odr - Renamed ad778x_filter to ad778x_odr_avail - Changed vref variable from unsigned int to unsigned long long to avoid overflow - Removed unnecessary AD_SD_CHANNEL macro drivers/staging/iio/adc/ad7780.c | 95 ++++++++++++++++++++++++++++++-- 1 file changed, 89 insertions(+), 6 deletions(-) diff --git a/drivers/staging/iio/adc/ad7780.c b/drivers/staging/iio/adc/ad7780.c index c4a85789c2db..6e4357800d31 100644 --- a/drivers/staging/iio/adc/ad7780.c +++ b/drivers/staging/iio/adc/ad7780.c @@ -39,6 +39,15 @@ #define AD7170_PATTERN (AD7780_PAT0 | AD7170_PAT2) #define AD7170_PATTERN_MASK (AD7780_PAT0 | AD7780_PAT1 | AD7170_PAT2) +#define AD7780_GAIN_GPIO 0 +#define AD7780_FILTER_GPIO 1 + +#define AD7780_GAIN_MIDPOINT 64 +#define AD7780_FILTER_MIDPOINT 13350 + +static const unsigned int ad778x_gain[2] = { 1, 128 }; +static const unsigned int ad778x_odr_avail[2] = { 10000, 16700 }; + struct ad7780_chip_info { struct iio_chan_spec channel; unsigned int pattern_mask; @@ -50,7 +59,11 @@ struct ad7780_state { const struct ad7780_chip_info *chip_info; struct regulator *reg; struct gpio_desc *powerdown_gpio; - unsigned int gain; + struct gpio_desc *gain_gpio; + struct gpio_desc *filter_gpio; + unsigned int gain; + unsigned int odr; + unsigned int int_vref_mv; struct ad_sigma_delta sd; }; @@ -104,17 +117,65 @@ static int ad7780_read_raw(struct iio_dev *indio_dev, voltage_uv = regulator_get_voltage(st->reg); if (voltage_uv < 0) return voltage_uv; - *val = (voltage_uv / 1000) * st->gain; + voltage_uv /= 1000; + *val = voltage_uv * st->gain; *val2 = chan->scan_type.realbits - 1; + st->int_vref_mv = voltage_uv; return IIO_VAL_FRACTIONAL_LOG2; case IIO_CHAN_INFO_OFFSET: *val = -(1 << (chan->scan_type.realbits - 1)); return IIO_VAL_INT; + case IIO_CHAN_INFO_SAMP_FREQ: + *val = st->odr; + return IIO_VAL_INT; } return -EINVAL; } +static int ad7780_write_raw(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, + int val, + int val2, + long m) +{ + struct ad7780_state *st = iio_priv(indio_dev); + const struct ad7780_chip_info *chip_info = st->chip_info; + unsigned long long vref; + unsigned int full_scale, gain; + + if (!chip_info->is_ad778x) + return 0; + + switch (m) { + case IIO_CHAN_INFO_SCALE: + if (val != 0) + return -EINVAL; + + vref = st->int_vref_mv * 1000000LL; + full_scale = 1 << (chip_info->channel.scan_type.realbits - 1); + gain = DIV_ROUND_CLOSEST(vref, full_scale); + gain = DIV_ROUND_CLOSEST(gain, val2); + st->gain = gain; + if (gain < AD7780_GAIN_MIDPOINT) + gain = 0; + else + gain = 1; + gpiod_set_value(st->gain_gpio, gain); + break; + case IIO_CHAN_INFO_SAMP_FREQ: + if (1000*val + val2/1000 < AD7780_FILTER_MIDPOINT) + val = 0; + else + val = 1; + st->odr = ad778x_odr_avail[val]; + gpiod_set_value(st->filter_gpio, val); + break; + } + + return 0; +} + static int ad7780_postprocess_sample(struct ad_sigma_delta *sigma_delta, unsigned int raw_sample) { @@ -126,10 +187,8 @@ static int ad7780_postprocess_sample(struct ad_sigma_delta *sigma_delta, return -EIO; if (chip_info->is_ad778x) { - if (raw_sample & AD7780_GAIN) - st->gain = 1; - else - st->gain = 128; + st->gain = ad778x_gain[raw_sample & AD7780_GAIN]; + st->odr = ad778x_odr_avail[raw_sample & AD7780_FILTER]; } return 0; @@ -173,6 +232,7 @@ static const struct ad7780_chip_info ad7780_chip_info_tbl[] = { static const struct iio_info ad7780_info = { .read_raw = ad7780_read_raw, + .write_raw = ad7780_write_raw, }; static int ad7780_probe(struct spi_device *spi) @@ -222,6 +282,29 @@ static int ad7780_probe(struct spi_device *spi) goto error_disable_reg; } + if (st->chip_info->is_ad778x) { + st->gain_gpio = devm_gpiod_get_optional(&spi->dev, + "gain", + GPIOD_OUT_HIGH); + if (IS_ERR(st->gain_gpio)) { + ret = PTR_ERR(st->gain_gpio); + dev_err(&spi->dev, "Failed to request gain GPIO: %d\n", + ret); + goto error_disable_reg; + } + + st->filter_gpio = devm_gpiod_get_optional(&spi->dev, + "filter", + GPIOD_OUT_HIGH); + if (IS_ERR(st->filter_gpio)) { + ret = PTR_ERR(st->filter_gpio); + dev_err(&spi->dev, + "Failed to request filter GPIO: %d\n", + ret); + goto error_disable_reg; + } + } + ret = ad_sd_setup_buffer_and_trigger(indio_dev); if (ret) goto error_disable_reg; -- 2.20.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v3 1/4] staging: iio: ad7780: add gain & filter gpio support 2019-02-05 17:13 ` [PATCH v3 1/4] staging: iio: ad7780: add gain & filter gpio support Renato Lui Geh @ 2019-02-05 19:58 ` Peter Meerwald-Stadler 2019-02-05 20:21 ` Renato Lui Geh 2019-02-06 7:27 ` Linus Torvalds 2019-02-09 16:25 ` Jonathan Cameron 1 sibling, 2 replies; 15+ messages in thread From: Peter Meerwald-Stadler @ 2019-02-05 19:58 UTC (permalink / raw) To: Renato Lui Geh Cc: lars, Michael.Hennerich, jic23, stefan.popa, alexandru.Ardelean, giuliano.belinassi, linux-iio, linux-kernel, kernel-usp On Tue, 5 Feb 2019, Renato Lui Geh wrote: > Previously, the AD7780 driver only supported gpio for the 'powerdown' > pin. This commit adds suppport for the 'gain' and 'filter' pin. comments below > Signed-off-by: Renato Lui Geh <renatogeh@gmail.com> > Signed-off-by: Giuliano Belinassi <giuliano.belinassi@usp.br> > Co-developed-by: Giuliano Belinassi <giuliano.belinassi@usp.br> > --- > Changes in v3: > - Renamed ad7780_chip_info's filter to odr > - Renamed ad778x_filter to ad778x_odr_avail > - Changed vref variable from unsigned int to unsigned long long to > avoid overflow > - Removed unnecessary AD_SD_CHANNEL macro > > drivers/staging/iio/adc/ad7780.c | 95 ++++++++++++++++++++++++++++++-- > 1 file changed, 89 insertions(+), 6 deletions(-) > > diff --git a/drivers/staging/iio/adc/ad7780.c > b/drivers/staging/iio/adc/ad7780.c > index c4a85789c2db..6e4357800d31 100644 > --- a/drivers/staging/iio/adc/ad7780.c > +++ b/drivers/staging/iio/adc/ad7780.c > @@ -39,6 +39,15 @@ > #define AD7170_PATTERN (AD7780_PAT0 | AD7170_PAT2) > #define AD7170_PATTERN_MASK (AD7780_PAT0 | AD7780_PAT1 | AD7170_PAT2) > > +#define AD7780_GAIN_GPIO 0 > +#define AD7780_FILTER_GPIO 1 > + > +#define AD7780_GAIN_MIDPOINT 64 > +#define AD7780_FILTER_MIDPOINT 13350 > + > +static const unsigned int ad778x_gain[2] = { 1, 128 }; > +static const unsigned int ad778x_odr_avail[2] = { 10000, 16700 }; > + > struct ad7780_chip_info { > struct iio_chan_spec channel; > unsigned int pattern_mask; > @@ -50,7 +59,11 @@ struct ad7780_state { > const struct ad7780_chip_info *chip_info; > struct regulator *reg; > struct gpio_desc *powerdown_gpio; > - unsigned int gain; > + struct gpio_desc *gain_gpio; > + struct gpio_desc *filter_gpio; > + unsigned int gain; > + unsigned int odr; > + unsigned int int_vref_mv; > > struct ad_sigma_delta sd; > }; > @@ -104,17 +117,65 @@ static int ad7780_read_raw(struct iio_dev *indio_dev, > voltage_uv = regulator_get_voltage(st->reg); > if (voltage_uv < 0) > return voltage_uv; > - *val = (voltage_uv / 1000) * st->gain; > + voltage_uv /= 1000; > + *val = voltage_uv * st->gain; > *val2 = chan->scan_type.realbits - 1; > + st->int_vref_mv = voltage_uv; > return IIO_VAL_FRACTIONAL_LOG2; > case IIO_CHAN_INFO_OFFSET: > *val = -(1 << (chan->scan_type.realbits - 1)); > return IIO_VAL_INT; > + case IIO_CHAN_INFO_SAMP_FREQ: was this missing before? this is not covered by the patch description and seems unrelated needs to be added to the channel spec? > + *val = st->odr; > + return IIO_VAL_INT; > } > > return -EINVAL; > } > > +static int ad7780_write_raw(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, > + int val, > + int val2, > + long m) > +{ > + struct ad7780_state *st = iio_priv(indio_dev); > + const struct ad7780_chip_info *chip_info = st->chip_info; > + unsigned long long vref; > + unsigned int full_scale, gain; > + > + if (!chip_info->is_ad778x) > + return 0; > + > + switch (m) { > + case IIO_CHAN_INFO_SCALE: > + if (val != 0) > + return -EINVAL; > + > + vref = st->int_vref_mv * 1000000LL; > + full_scale = 1 << (chip_info->channel.scan_type.realbits - 1); > + gain = DIV_ROUND_CLOSEST(vref, full_scale); > + gain = DIV_ROUND_CLOSEST(gain, val2); > + st->gain = gain; > + if (gain < AD7780_GAIN_MIDPOINT) > + gain = 0; > + else > + gain = 1; > + gpiod_set_value(st->gain_gpio, gain); > + break; indentation with the other, previous statements > + case IIO_CHAN_INFO_SAMP_FREQ: > + if (1000*val + val2/1000 < AD7780_FILTER_MIDPOINT) space around operators missing > + val = 0; > + else > + val = 1; > + st->odr = ad778x_odr_avail[val]; > + gpiod_set_value(st->filter_gpio, val); > + break; > + } > + > + return 0; > +} > + > static int ad7780_postprocess_sample(struct ad_sigma_delta *sigma_delta, > unsigned int raw_sample) > { > @@ -126,10 +187,8 @@ static int ad7780_postprocess_sample(struct > ad_sigma_delta *sigma_delta, > return -EIO; > > if (chip_info->is_ad778x) { > - if (raw_sample & AD7780_GAIN) > - st->gain = 1; > - else > - st->gain = 128; > + st->gain = ad778x_gain[raw_sample & AD7780_GAIN]; > + st->odr = ad778x_odr_avail[raw_sample & AD7780_FILTER]; > } > > return 0; > @@ -173,6 +232,7 @@ static const struct ad7780_chip_info > ad7780_chip_info_tbl[] = { > > static const struct iio_info ad7780_info = { > .read_raw = ad7780_read_raw, > + .write_raw = ad7780_write_raw, > }; > > static int ad7780_probe(struct spi_device *spi) > @@ -222,6 +282,29 @@ static int ad7780_probe(struct spi_device *spi) > goto error_disable_reg; > } > > + if (st->chip_info->is_ad778x) { > + st->gain_gpio = devm_gpiod_get_optional(&spi->dev, > + "gain", > + GPIOD_OUT_HIGH); > + if (IS_ERR(st->gain_gpio)) { > + ret = PTR_ERR(st->gain_gpio); > + dev_err(&spi->dev, "Failed to request gain GPIO: > %d\n", > + ret); > + goto error_disable_reg; > + } > + > + st->filter_gpio = devm_gpiod_get_optional(&spi->dev, > + "filter", > + GPIOD_OUT_HIGH); > + if (IS_ERR(st->filter_gpio)) { > + ret = PTR_ERR(st->filter_gpio); > + dev_err(&spi->dev, > + "Failed to request filter GPIO: %d\n", > + ret); > + goto error_disable_reg; > + } > + } > + > ret = ad_sd_setup_buffer_and_trigger(indio_dev); > if (ret) > goto error_disable_reg; > -- Peter Meerwald-Stadler Mobile: +43 664 24 44 418 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 1/4] staging: iio: ad7780: add gain & filter gpio support 2019-02-05 19:58 ` Peter Meerwald-Stadler @ 2019-02-05 20:21 ` Renato Lui Geh 2019-02-06 7:27 ` Linus Torvalds 1 sibling, 0 replies; 15+ messages in thread From: Renato Lui Geh @ 2019-02-05 20:21 UTC (permalink / raw) To: Peter Meerwald-Stadler Cc: Renato Lui Geh, lars, Michael.Hennerich, jic23, stefan.popa, alexandru.Ardelean, giuliano.belinassi, linux-iio, linux-kernel, kernel-usp Hi Peter, Thank you for the review! Comments inline. Renato On 02/05, Peter Meerwald-Stadler wrote: >On Tue, 5 Feb 2019, Renato Lui Geh wrote: > >> Previously, the AD7780 driver only supported gpio for the 'powerdown' >> pin. This commit adds suppport for the 'gain' and 'filter' pin. > >comments below > >> Signed-off-by: Renato Lui Geh <renatogeh@gmail.com> >> Signed-off-by: Giuliano Belinassi <giuliano.belinassi@usp.br> >> Co-developed-by: Giuliano Belinassi <giuliano.belinassi@usp.br> >> --- >> Changes in v3: >> - Renamed ad7780_chip_info's filter to odr >> - Renamed ad778x_filter to ad778x_odr_avail >> - Changed vref variable from unsigned int to unsigned long long to >> avoid overflow >> - Removed unnecessary AD_SD_CHANNEL macro >> >> drivers/staging/iio/adc/ad7780.c | 95 ++++++++++++++++++++++++++++++-- >> 1 file changed, 89 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/staging/iio/adc/ad7780.c >> b/drivers/staging/iio/adc/ad7780.c >> index c4a85789c2db..6e4357800d31 100644 >> --- a/drivers/staging/iio/adc/ad7780.c >> +++ b/drivers/staging/iio/adc/ad7780.c >> @@ -39,6 +39,15 @@ >> #define AD7170_PATTERN (AD7780_PAT0 | AD7170_PAT2) >> #define AD7170_PATTERN_MASK (AD7780_PAT0 | AD7780_PAT1 | AD7170_PAT2) >> >> +#define AD7780_GAIN_GPIO 0 >> +#define AD7780_FILTER_GPIO 1 >> + >> +#define AD7780_GAIN_MIDPOINT 64 >> +#define AD7780_FILTER_MIDPOINT 13350 >> + >> +static const unsigned int ad778x_gain[2] = { 1, 128 }; >> +static const unsigned int ad778x_odr_avail[2] = { 10000, 16700 }; >> + >> struct ad7780_chip_info { >> struct iio_chan_spec channel; >> unsigned int pattern_mask; >> @@ -50,7 +59,11 @@ struct ad7780_state { >> const struct ad7780_chip_info *chip_info; >> struct regulator *reg; >> struct gpio_desc *powerdown_gpio; >> - unsigned int gain; >> + struct gpio_desc *gain_gpio; >> + struct gpio_desc *filter_gpio; >> + unsigned int gain; >> + unsigned int odr; >> + unsigned int int_vref_mv; >> >> struct ad_sigma_delta sd; >> }; >> @@ -104,17 +117,65 @@ static int ad7780_read_raw(struct iio_dev *indio_dev, >> voltage_uv = regulator_get_voltage(st->reg); >> if (voltage_uv < 0) >> return voltage_uv; >> - *val = (voltage_uv / 1000) * st->gain; >> + voltage_uv /= 1000; >> + *val = voltage_uv * st->gain; >> *val2 = chan->scan_type.realbits - 1; >> + st->int_vref_mv = voltage_uv; >> return IIO_VAL_FRACTIONAL_LOG2; >> case IIO_CHAN_INFO_OFFSET: >> *val = -(1 << (chan->scan_type.realbits - 1)); >> return IIO_VAL_INT; >> + case IIO_CHAN_INFO_SAMP_FREQ: > >was this missing before? >this is not covered by the patch description and seems unrelated >needs to be added to the channel spec? Yes, this is a new feature. Should I have this chunk sent as a separate patch? What is the channel spec? How would I add this to the channel spec? > >> + *val = st->odr; >> + return IIO_VAL_INT; >> } >> >> return -EINVAL; >> } >> >> +static int ad7780_write_raw(struct iio_dev *indio_dev, >> + struct iio_chan_spec const *chan, >> + int val, >> + int val2, >> + long m) >> +{ >> + struct ad7780_state *st = iio_priv(indio_dev); >> + const struct ad7780_chip_info *chip_info = st->chip_info; >> + unsigned long long vref; >> + unsigned int full_scale, gain; >> + >> + if (!chip_info->is_ad778x) >> + return 0; >> + >> + switch (m) { >> + case IIO_CHAN_INFO_SCALE: >> + if (val != 0) >> + return -EINVAL; >> + >> + vref = st->int_vref_mv * 1000000LL; >> + full_scale = 1 << (chip_info->channel.scan_type.realbits - 1); >> + gain = DIV_ROUND_CLOSEST(vref, full_scale); >> + gain = DIV_ROUND_CLOSEST(gain, val2); >> + st->gain = gain; >> + if (gain < AD7780_GAIN_MIDPOINT) >> + gain = 0; >> + else >> + gain = 1; >> + gpiod_set_value(st->gain_gpio, gain); >> + break; > >indentation with the other, previous statements > >> + case IIO_CHAN_INFO_SAMP_FREQ: >> + if (1000*val + val2/1000 < AD7780_FILTER_MIDPOINT) > >space around operators missing > >> + val = 0; >> + else >> + val = 1; >> + st->odr = ad778x_odr_avail[val]; >> + gpiod_set_value(st->filter_gpio, val); >> + break; >> + } >> + >> + return 0; >> +} >> + >> static int ad7780_postprocess_sample(struct ad_sigma_delta *sigma_delta, >> unsigned int raw_sample) >> { >> @@ -126,10 +187,8 @@ static int ad7780_postprocess_sample(struct >> ad_sigma_delta *sigma_delta, >> return -EIO; >> >> if (chip_info->is_ad778x) { >> - if (raw_sample & AD7780_GAIN) >> - st->gain = 1; >> - else >> - st->gain = 128; >> + st->gain = ad778x_gain[raw_sample & AD7780_GAIN]; >> + st->odr = ad778x_odr_avail[raw_sample & AD7780_FILTER]; >> } >> >> return 0; >> @@ -173,6 +232,7 @@ static const struct ad7780_chip_info >> ad7780_chip_info_tbl[] = { >> >> static const struct iio_info ad7780_info = { >> .read_raw = ad7780_read_raw, >> + .write_raw = ad7780_write_raw, >> }; >> >> static int ad7780_probe(struct spi_device *spi) >> @@ -222,6 +282,29 @@ static int ad7780_probe(struct spi_device *spi) >> goto error_disable_reg; >> } >> >> + if (st->chip_info->is_ad778x) { >> + st->gain_gpio = devm_gpiod_get_optional(&spi->dev, >> + "gain", >> + GPIOD_OUT_HIGH); >> + if (IS_ERR(st->gain_gpio)) { >> + ret = PTR_ERR(st->gain_gpio); >> + dev_err(&spi->dev, "Failed to request gain GPIO: >> %d\n", >> + ret); >> + goto error_disable_reg; >> + } >> + >> + st->filter_gpio = devm_gpiod_get_optional(&spi->dev, >> + "filter", >> + GPIOD_OUT_HIGH); >> + if (IS_ERR(st->filter_gpio)) { >> + ret = PTR_ERR(st->filter_gpio); >> + dev_err(&spi->dev, >> + "Failed to request filter GPIO: %d\n", >> + ret); >> + goto error_disable_reg; >> + } >> + } >> + >> ret = ad_sd_setup_buffer_and_trigger(indio_dev); >> if (ret) >> goto error_disable_reg; >> > >-- > >Peter Meerwald-Stadler >Mobile: +43 664 24 44 418 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 1/4] staging: iio: ad7780: add gain & filter gpio support 2019-02-05 19:58 ` Peter Meerwald-Stadler 2019-02-05 20:21 ` Renato Lui Geh @ 2019-02-06 7:27 ` Linus Torvalds 1 sibling, 0 replies; 15+ messages in thread From: Linus Torvalds @ 2019-02-06 7:27 UTC (permalink / raw) To: Peter Meerwald-Stadler; +Cc: lkml Peter, this email was marked as spam for me (and probably others) because of this DKIM signature: DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=pmeerw.net; s=mail; .... where the problem is that when the message goes through the vger.kernel.org mailing list machinery, the header whitespace will be modified. As a result, the DKIM signature no longer matched, and at least gmail will consider the email to be spam. That's arguably a bug in the vger mail server setup, but equally arguably the "c=simple/simple" model of hashing in DKIM is just broken, and shouldn't be used. Whitespace in SMTP headers is simply not meaningful, and checking whitespace in them (like simple/simple does) is misguided. If you have control over the DKIM setup of pmeerw.net, may I suggest changing the DKIM setup to use "c=relaxed/relaxed", which doesn't invalidate the signature just for whitespace changes in the headers, and which is the proper DKIM model to use. The simple canonicalization really is too simple-minded for email. Linus On Tue, Feb 5, 2019 at 7:58 PM Peter Meerwald-Stadler <pmeerw@pmeerw.net> wrote: > > On Tue, 5 Feb 2019, Renato Lui Geh wrote: > > > Previously, the AD7780 driver only supported gpio for the 'powerdown' > > pin. This commit adds suppport for the 'gain' and 'filter' pin. > > comments below [...] ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 1/4] staging: iio: ad7780: add gain & filter gpio support 2019-02-05 17:13 ` [PATCH v3 1/4] staging: iio: ad7780: add gain & filter gpio support Renato Lui Geh 2019-02-05 19:58 ` Peter Meerwald-Stadler @ 2019-02-09 16:25 ` Jonathan Cameron 2019-02-14 20:31 ` Renato Lui Geh 1 sibling, 1 reply; 15+ messages in thread From: Jonathan Cameron @ 2019-02-09 16:25 UTC (permalink / raw) To: Renato Lui Geh Cc: lars, Michael.Hennerich, knaack.h, pmeerw, gregkh, stefan.popa, alexandru.Ardelean, giuliano.belinassi, linux-iio, devel, linux-kernel, kernel-usp On Tue, 5 Feb 2019 15:13:00 -0200 Renato Lui Geh <renatogeh@gmail.com> wrote: > Previously, the AD7780 driver only supported gpio for the 'powerdown' > pin. This commit adds suppport for the 'gain' and 'filter' pin. > > Signed-off-by: Renato Lui Geh <renatogeh@gmail.com> > Signed-off-by: Giuliano Belinassi <giuliano.belinassi@usp.br> > Co-developed-by: Giuliano Belinassi <giuliano.belinassi@usp.br> Comments inline. > --- > Changes in v3: > - Renamed ad7780_chip_info's filter to odr > - Renamed ad778x_filter to ad778x_odr_avail > - Changed vref variable from unsigned int to unsigned long long to > avoid overflow > - Removed unnecessary AD_SD_CHANNEL macro > > drivers/staging/iio/adc/ad7780.c | 95 ++++++++++++++++++++++++++++++-- > 1 file changed, 89 insertions(+), 6 deletions(-) > > diff --git a/drivers/staging/iio/adc/ad7780.c b/drivers/staging/iio/adc/ad7780.c > index c4a85789c2db..6e4357800d31 100644 > --- a/drivers/staging/iio/adc/ad7780.c > +++ b/drivers/staging/iio/adc/ad7780.c > @@ -39,6 +39,15 @@ > #define AD7170_PATTERN (AD7780_PAT0 | AD7170_PAT2) > #define AD7170_PATTERN_MASK (AD7780_PAT0 | AD7780_PAT1 | AD7170_PAT2) > > +#define AD7780_GAIN_GPIO 0 > +#define AD7780_FILTER_GPIO 1 What are these for? > + > +#define AD7780_GAIN_MIDPOINT 64 > +#define AD7780_FILTER_MIDPOINT 13350 > + > +static const unsigned int ad778x_gain[2] = { 1, 128 }; > +static const unsigned int ad778x_odr_avail[2] = { 10000, 16700 }; > + > struct ad7780_chip_info { > struct iio_chan_spec channel; > unsigned int pattern_mask; > @@ -50,7 +59,11 @@ struct ad7780_state { > const struct ad7780_chip_info *chip_info; > struct regulator *reg; > struct gpio_desc *powerdown_gpio; > - unsigned int gain; > + struct gpio_desc *gain_gpio; > + struct gpio_desc *filter_gpio; > + unsigned int gain; > + unsigned int odr; > + unsigned int int_vref_mv; > > struct ad_sigma_delta sd; > }; > @@ -104,17 +117,65 @@ static int ad7780_read_raw(struct iio_dev *indio_dev, > voltage_uv = regulator_get_voltage(st->reg); > if (voltage_uv < 0) > return voltage_uv; > - *val = (voltage_uv / 1000) * st->gain; > + voltage_uv /= 1000; > + *val = voltage_uv * st->gain; > *val2 = chan->scan_type.realbits - 1; > + st->int_vref_mv = voltage_uv; > return IIO_VAL_FRACTIONAL_LOG2; > case IIO_CHAN_INFO_OFFSET: > *val = -(1 << (chan->scan_type.realbits - 1)); > return IIO_VAL_INT; > + case IIO_CHAN_INFO_SAMP_FREQ: > + *val = st->odr; > + return IIO_VAL_INT; > } > > return -EINVAL; > } > > +static int ad7780_write_raw(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, > + int val, > + int val2, > + long m) > +{ > + struct ad7780_state *st = iio_priv(indio_dev); > + const struct ad7780_chip_info *chip_info = st->chip_info; > + unsigned long long vref; > + unsigned int full_scale, gain; > + > + if (!chip_info->is_ad778x) > + return 0; > + > + switch (m) { > + case IIO_CHAN_INFO_SCALE: > + if (val != 0) > + return -EINVAL; > + > + vref = st->int_vref_mv * 1000000LL; > + full_scale = 1 << (chip_info->channel.scan_type.realbits - 1); > + gain = DIV_ROUND_CLOSEST(vref, full_scale); > + gain = DIV_ROUND_CLOSEST(gain, val2); > + st->gain = gain; > + if (gain < AD7780_GAIN_MIDPOINT) > + gain = 0; > + else > + gain = 1; > + gpiod_set_value(st->gain_gpio, gain); > + break; > + case IIO_CHAN_INFO_SAMP_FREQ: > + if (1000*val + val2/1000 < AD7780_FILTER_MIDPOINT) > + val = 0; > + else > + val = 1; > + st->odr = ad778x_odr_avail[val]; > + gpiod_set_value(st->filter_gpio, val); > + break; > + } > + > + return 0; > +} > + > static int ad7780_postprocess_sample(struct ad_sigma_delta *sigma_delta, > unsigned int raw_sample) > { > @@ -126,10 +187,8 @@ static int ad7780_postprocess_sample(struct ad_sigma_delta *sigma_delta, > return -EIO; > > if (chip_info->is_ad778x) { > - if (raw_sample & AD7780_GAIN) > - st->gain = 1; > - else > - st->gain = 128; > + st->gain = ad778x_gain[raw_sample & AD7780_GAIN]; > + st->odr = ad778x_odr_avail[raw_sample & AD7780_FILTER]; > } > > return 0; > @@ -173,6 +232,7 @@ static const struct ad7780_chip_info ad7780_chip_info_tbl[] = { > > static const struct iio_info ad7780_info = { > .read_raw = ad7780_read_raw, > + .write_raw = ad7780_write_raw, > }; > > static int ad7780_probe(struct spi_device *spi) > @@ -222,6 +282,29 @@ static int ad7780_probe(struct spi_device *spi) > goto error_disable_reg; > } > > + if (st->chip_info->is_ad778x) { > + st->gain_gpio = devm_gpiod_get_optional(&spi->dev, > + "gain", These are not particularly standard names (basically not "reset"), so they should be vendor prefixed, so that people know to go look at the device specific binding. > + GPIOD_OUT_HIGH); > + if (IS_ERR(st->gain_gpio)) { > + ret = PTR_ERR(st->gain_gpio); > + dev_err(&spi->dev, "Failed to request gain GPIO: %d\n", > + ret); > + goto error_disable_reg; > + } > + > + st->filter_gpio = devm_gpiod_get_optional(&spi->dev, > + "filter", > + GPIOD_OUT_HIGH); > + if (IS_ERR(st->filter_gpio)) { > + ret = PTR_ERR(st->filter_gpio); > + dev_err(&spi->dev, > + "Failed to request filter GPIO: %d\n", > + ret); > + goto error_disable_reg; > + } > + } > + > ret = ad_sd_setup_buffer_and_trigger(indio_dev); > if (ret) > goto error_disable_reg; ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 1/4] staging: iio: ad7780: add gain & filter gpio support 2019-02-09 16:25 ` Jonathan Cameron @ 2019-02-14 20:31 ` Renato Lui Geh 2019-02-18 14:48 ` Jonathan Cameron 0 siblings, 1 reply; 15+ messages in thread From: Renato Lui Geh @ 2019-02-14 20:31 UTC (permalink / raw) To: Jonathan Cameron Cc: Renato Lui Geh, lars, Michael.Hennerich, knaack.h, pmeerw, gregkh, stefan.popa, alexandru.Ardelean, giuliano.belinassi, linux-iio, devel, linux-kernel, kernel-usp Hi Jonathan, Thanks for the review. Comments inline. Renato On 02/09, Jonathan Cameron wrote: >On Tue, 5 Feb 2019 15:13:00 -0200 >Renato Lui Geh <renatogeh@gmail.com> wrote: > >> Previously, the AD7780 driver only supported gpio for the 'powerdown' >> pin. This commit adds suppport for the 'gain' and 'filter' pin. >> >> Signed-off-by: Renato Lui Geh <renatogeh@gmail.com> >> Signed-off-by: Giuliano Belinassi <giuliano.belinassi@usp.br> >> Co-developed-by: Giuliano Belinassi <giuliano.belinassi@usp.br> >Comments inline. > >> --- >> Changes in v3: >> - Renamed ad7780_chip_info's filter to odr >> - Renamed ad778x_filter to ad778x_odr_avail >> - Changed vref variable from unsigned int to unsigned long long to >> avoid overflow >> - Removed unnecessary AD_SD_CHANNEL macro >> >> drivers/staging/iio/adc/ad7780.c | 95 ++++++++++++++++++++++++++++++-- >> 1 file changed, 89 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/staging/iio/adc/ad7780.c b/drivers/staging/iio/adc/ad7780.c >> index c4a85789c2db..6e4357800d31 100644 >> --- a/drivers/staging/iio/adc/ad7780.c >> +++ b/drivers/staging/iio/adc/ad7780.c >> @@ -39,6 +39,15 @@ >> #define AD7170_PATTERN (AD7780_PAT0 | AD7170_PAT2) >> #define AD7170_PATTERN_MASK (AD7780_PAT0 | AD7780_PAT1 | AD7170_PAT2) >> >> +#define AD7780_GAIN_GPIO 0 >> +#define AD7780_FILTER_GPIO 1 >What are these for? Sorry about that. That's leftover from a previous attempt. > >> + >> +#define AD7780_GAIN_MIDPOINT 64 >> +#define AD7780_FILTER_MIDPOINT 13350 >> + >> +static const unsigned int ad778x_gain[2] = { 1, 128 }; >> +static const unsigned int ad778x_odr_avail[2] = { 10000, 16700 }; >> + >> struct ad7780_chip_info { >> struct iio_chan_spec channel; >> unsigned int pattern_mask; >> @@ -50,7 +59,11 @@ struct ad7780_state { >> const struct ad7780_chip_info *chip_info; >> struct regulator *reg; >> struct gpio_desc *powerdown_gpio; >> - unsigned int gain; >> + struct gpio_desc *gain_gpio; >> + struct gpio_desc *filter_gpio; >> + unsigned int gain; >> + unsigned int odr; >> + unsigned int int_vref_mv; >> >> struct ad_sigma_delta sd; >> }; >> @@ -104,17 +117,65 @@ static int ad7780_read_raw(struct iio_dev *indio_dev, >> voltage_uv = regulator_get_voltage(st->reg); >> if (voltage_uv < 0) >> return voltage_uv; >> - *val = (voltage_uv / 1000) * st->gain; >> + voltage_uv /= 1000; >> + *val = voltage_uv * st->gain; >> *val2 = chan->scan_type.realbits - 1; >> + st->int_vref_mv = voltage_uv; >> return IIO_VAL_FRACTIONAL_LOG2; >> case IIO_CHAN_INFO_OFFSET: >> *val = -(1 << (chan->scan_type.realbits - 1)); >> return IIO_VAL_INT; >> + case IIO_CHAN_INFO_SAMP_FREQ: >> + *val = st->odr; >> + return IIO_VAL_INT; >> } >> >> return -EINVAL; >> } >> >> +static int ad7780_write_raw(struct iio_dev *indio_dev, >> + struct iio_chan_spec const *chan, >> + int val, >> + int val2, >> + long m) >> +{ >> + struct ad7780_state *st = iio_priv(indio_dev); >> + const struct ad7780_chip_info *chip_info = st->chip_info; >> + unsigned long long vref; >> + unsigned int full_scale, gain; >> + >> + if (!chip_info->is_ad778x) >> + return 0; >> + >> + switch (m) { >> + case IIO_CHAN_INFO_SCALE: >> + if (val != 0) >> + return -EINVAL; >> + >> + vref = st->int_vref_mv * 1000000LL; >> + full_scale = 1 << (chip_info->channel.scan_type.realbits - 1); >> + gain = DIV_ROUND_CLOSEST(vref, full_scale); >> + gain = DIV_ROUND_CLOSEST(gain, val2); >> + st->gain = gain; >> + if (gain < AD7780_GAIN_MIDPOINT) >> + gain = 0; >> + else >> + gain = 1; >> + gpiod_set_value(st->gain_gpio, gain); >> + break; >> + case IIO_CHAN_INFO_SAMP_FREQ: >> + if (1000*val + val2/1000 < AD7780_FILTER_MIDPOINT) >> + val = 0; >> + else >> + val = 1; >> + st->odr = ad778x_odr_avail[val]; >> + gpiod_set_value(st->filter_gpio, val); >> + break; >> + } >> + >> + return 0; >> +} >> + >> static int ad7780_postprocess_sample(struct ad_sigma_delta *sigma_delta, >> unsigned int raw_sample) >> { >> @@ -126,10 +187,8 @@ static int ad7780_postprocess_sample(struct ad_sigma_delta *sigma_delta, >> return -EIO; >> >> if (chip_info->is_ad778x) { >> - if (raw_sample & AD7780_GAIN) >> - st->gain = 1; >> - else >> - st->gain = 128; >> + st->gain = ad778x_gain[raw_sample & AD7780_GAIN]; >> + st->odr = ad778x_odr_avail[raw_sample & AD7780_FILTER]; >> } >> >> return 0; >> @@ -173,6 +232,7 @@ static const struct ad7780_chip_info ad7780_chip_info_tbl[] = { >> >> static const struct iio_info ad7780_info = { >> .read_raw = ad7780_read_raw, >> + .write_raw = ad7780_write_raw, >> }; >> >> static int ad7780_probe(struct spi_device *spi) >> @@ -222,6 +282,29 @@ static int ad7780_probe(struct spi_device *spi) >> goto error_disable_reg; >> } >> >> + if (st->chip_info->is_ad778x) { >> + st->gain_gpio = devm_gpiod_get_optional(&spi->dev, >> + "gain", > >These are not particularly standard names (basically not "reset"), >so they should be vendor prefixed, so that people know to go >look at the device specific binding. I see. Should they be something like "adi,gain" and "adi,filter"? Am I correct to assume that I'll have to somehow mention these in the dt-binding? > >> + GPIOD_OUT_HIGH); >> + if (IS_ERR(st->gain_gpio)) { >> + ret = PTR_ERR(st->gain_gpio); >> + dev_err(&spi->dev, "Failed to request gain GPIO: %d\n", >> + ret); >> + goto error_disable_reg; >> + } >> + >> + st->filter_gpio = devm_gpiod_get_optional(&spi->dev, >> + "filter", >> + GPIOD_OUT_HIGH); >> + if (IS_ERR(st->filter_gpio)) { >> + ret = PTR_ERR(st->filter_gpio); >> + dev_err(&spi->dev, >> + "Failed to request filter GPIO: %d\n", >> + ret); >> + goto error_disable_reg; >> + } >> + } >> + >> ret = ad_sd_setup_buffer_and_trigger(indio_dev); >> if (ret) >> goto error_disable_reg; > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 1/4] staging: iio: ad7780: add gain & filter gpio support 2019-02-14 20:31 ` Renato Lui Geh @ 2019-02-18 14:48 ` Jonathan Cameron 0 siblings, 0 replies; 15+ messages in thread From: Jonathan Cameron @ 2019-02-18 14:48 UTC (permalink / raw) To: Renato Lui Geh Cc: Jonathan Cameron, lars, Michael.Hennerich, knaack.h, pmeerw, gregkh, stefan.popa, alexandru.Ardelean, giuliano.belinassi, linux-iio, devel, linux-kernel, kernel-usp On Thu, 14 Feb 2019 18:31:12 -0200 Renato Lui Geh <renatogeh@gmail.com> wrote: > Hi Jonathan, > > Thanks for the review. Comments inline. > > Renato > > On 02/09, Jonathan Cameron wrote: > >On Tue, 5 Feb 2019 15:13:00 -0200 > >Renato Lui Geh <renatogeh@gmail.com> wrote: > > > >> Previously, the AD7780 driver only supported gpio for the 'powerdown' > >> pin. This commit adds suppport for the 'gain' and 'filter' pin. > >> > >> Signed-off-by: Renato Lui Geh <renatogeh@gmail.com> > >> Signed-off-by: Giuliano Belinassi <giuliano.belinassi@usp.br> > >> Co-developed-by: Giuliano Belinassi <giuliano.belinassi@usp.br> > >Comments inline. > > > >> --- > >> Changes in v3: > >> - Renamed ad7780_chip_info's filter to odr > >> - Renamed ad778x_filter to ad778x_odr_avail > >> - Changed vref variable from unsigned int to unsigned long long to > >> avoid overflow > >> - Removed unnecessary AD_SD_CHANNEL macro > >> > >> drivers/staging/iio/adc/ad7780.c | 95 ++++++++++++++++++++++++++++++-- > >> 1 file changed, 89 insertions(+), 6 deletions(-) > >> > >> diff --git a/drivers/staging/iio/adc/ad7780.c b/drivers/staging/iio/adc/ad7780.c > >> index c4a85789c2db..6e4357800d31 100644 > >> --- a/drivers/staging/iio/adc/ad7780.c > >> +++ b/drivers/staging/iio/adc/ad7780.c > >> @@ -39,6 +39,15 @@ > >> #define AD7170_PATTERN (AD7780_PAT0 | AD7170_PAT2) > >> #define AD7170_PATTERN_MASK (AD7780_PAT0 | AD7780_PAT1 | AD7170_PAT2) > >> > >> +#define AD7780_GAIN_GPIO 0 > >> +#define AD7780_FILTER_GPIO 1 > >What are these for? > > Sorry about that. That's leftover from a previous attempt. > > > >> + > >> +#define AD7780_GAIN_MIDPOINT 64 > >> +#define AD7780_FILTER_MIDPOINT 13350 > >> + > >> +static const unsigned int ad778x_gain[2] = { 1, 128 }; > >> +static const unsigned int ad778x_odr_avail[2] = { 10000, 16700 }; > >> + > >> struct ad7780_chip_info { > >> struct iio_chan_spec channel; > >> unsigned int pattern_mask; > >> @@ -50,7 +59,11 @@ struct ad7780_state { > >> const struct ad7780_chip_info *chip_info; > >> struct regulator *reg; > >> struct gpio_desc *powerdown_gpio; > >> - unsigned int gain; > >> + struct gpio_desc *gain_gpio; > >> + struct gpio_desc *filter_gpio; > >> + unsigned int gain; > >> + unsigned int odr; > >> + unsigned int int_vref_mv; > >> > >> struct ad_sigma_delta sd; > >> }; > >> @@ -104,17 +117,65 @@ static int ad7780_read_raw(struct iio_dev *indio_dev, > >> voltage_uv = regulator_get_voltage(st->reg); > >> if (voltage_uv < 0) > >> return voltage_uv; > >> - *val = (voltage_uv / 1000) * st->gain; > >> + voltage_uv /= 1000; > >> + *val = voltage_uv * st->gain; > >> *val2 = chan->scan_type.realbits - 1; > >> + st->int_vref_mv = voltage_uv; > >> return IIO_VAL_FRACTIONAL_LOG2; > >> case IIO_CHAN_INFO_OFFSET: > >> *val = -(1 << (chan->scan_type.realbits - 1)); > >> return IIO_VAL_INT; > >> + case IIO_CHAN_INFO_SAMP_FREQ: > >> + *val = st->odr; > >> + return IIO_VAL_INT; > >> } > >> > >> return -EINVAL; > >> } > >> > >> +static int ad7780_write_raw(struct iio_dev *indio_dev, > >> + struct iio_chan_spec const *chan, > >> + int val, > >> + int val2, > >> + long m) > >> +{ > >> + struct ad7780_state *st = iio_priv(indio_dev); > >> + const struct ad7780_chip_info *chip_info = st->chip_info; > >> + unsigned long long vref; > >> + unsigned int full_scale, gain; > >> + > >> + if (!chip_info->is_ad778x) > >> + return 0; > >> + > >> + switch (m) { > >> + case IIO_CHAN_INFO_SCALE: > >> + if (val != 0) > >> + return -EINVAL; > >> + > >> + vref = st->int_vref_mv * 1000000LL; > >> + full_scale = 1 << (chip_info->channel.scan_type.realbits - 1); > >> + gain = DIV_ROUND_CLOSEST(vref, full_scale); > >> + gain = DIV_ROUND_CLOSEST(gain, val2); > >> + st->gain = gain; > >> + if (gain < AD7780_GAIN_MIDPOINT) > >> + gain = 0; > >> + else > >> + gain = 1; > >> + gpiod_set_value(st->gain_gpio, gain); > >> + break; > >> + case IIO_CHAN_INFO_SAMP_FREQ: > >> + if (1000*val + val2/1000 < AD7780_FILTER_MIDPOINT) > >> + val = 0; > >> + else > >> + val = 1; > >> + st->odr = ad778x_odr_avail[val]; > >> + gpiod_set_value(st->filter_gpio, val); > >> + break; > >> + } > >> + > >> + return 0; > >> +} > >> + > >> static int ad7780_postprocess_sample(struct ad_sigma_delta *sigma_delta, > >> unsigned int raw_sample) > >> { > >> @@ -126,10 +187,8 @@ static int ad7780_postprocess_sample(struct ad_sigma_delta *sigma_delta, > >> return -EIO; > >> > >> if (chip_info->is_ad778x) { > >> - if (raw_sample & AD7780_GAIN) > >> - st->gain = 1; > >> - else > >> - st->gain = 128; > >> + st->gain = ad778x_gain[raw_sample & AD7780_GAIN]; > >> + st->odr = ad778x_odr_avail[raw_sample & AD7780_FILTER]; > >> } > >> > >> return 0; > >> @@ -173,6 +232,7 @@ static const struct ad7780_chip_info ad7780_chip_info_tbl[] = { > >> > >> static const struct iio_info ad7780_info = { > >> .read_raw = ad7780_read_raw, > >> + .write_raw = ad7780_write_raw, > >> }; > >> > >> static int ad7780_probe(struct spi_device *spi) > >> @@ -222,6 +282,29 @@ static int ad7780_probe(struct spi_device *spi) > >> goto error_disable_reg; > >> } > >> > >> + if (st->chip_info->is_ad778x) { > >> + st->gain_gpio = devm_gpiod_get_optional(&spi->dev, > >> + "gain", > > > >These are not particularly standard names (basically not "reset"), > >so they should be vendor prefixed, so that people know to go > >look at the device specific binding. > > I see. Should they be something like "adi,gain" and "adi,filter"? Am I > correct to assume that I'll have to somehow mention these in the > dt-binding? yes and yes - name is just adi,gain-gpios rather than gain-gpios. Take a look at the other drivers doing the same thing. We used to be more lax on this so there are drivers without the prefixes, but can't fix them now. Jonathan > > > >> + GPIOD_OUT_HIGH); > >> + if (IS_ERR(st->gain_gpio)) { > >> + ret = PTR_ERR(st->gain_gpio); > >> + dev_err(&spi->dev, "Failed to request gain GPIO: %d\n", > >> + ret); > >> + goto error_disable_reg; > >> + } > >> + > >> + st->filter_gpio = devm_gpiod_get_optional(&spi->dev, > >> + "filter", > >> + GPIOD_OUT_HIGH); > >> + if (IS_ERR(st->filter_gpio)) { > >> + ret = PTR_ERR(st->filter_gpio); > >> + dev_err(&spi->dev, > >> + "Failed to request filter GPIO: %d\n", > >> + ret); > >> + goto error_disable_reg; > >> + } > >> + } > >> + > >> ret = ad_sd_setup_buffer_and_trigger(indio_dev); > >> if (ret) > >> goto error_disable_reg; > > ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v3 2/4] staging: iio: ad7780: move regulator to after GPIO init 2019-02-05 17:12 [PATCH v3 0/4] staging: iio: ad7780: move out of staging Renato Lui Geh 2019-02-05 17:13 ` [PATCH v3 1/4] staging: iio: ad7780: add gain & filter gpio support Renato Lui Geh @ 2019-02-05 17:13 ` Renato Lui Geh 2019-02-09 16:26 ` Jonathan Cameron 2019-02-05 17:13 ` [PATCH v3 3/4] staging: iio: ad7780: add SPDX identifier Renato Lui Geh 2019-02-05 17:14 ` [PATCH v3 4/4] staging: iio: ad7780: moving ad7780 out of staging Renato Lui Geh 3 siblings, 1 reply; 15+ messages in thread From: Renato Lui Geh @ 2019-02-05 17:13 UTC (permalink / raw) To: lars, Michael.Hennerich, jic23, knaack.h, pmeerw, gregkh, stefan.popa, alexandru.Ardelean, giuliano.belinassi Cc: linux-iio, devel, linux-kernel, kernel-usp To maintain consistency between ad7780_probe and ad7780_remove orders, regulator initialization has been moved to after GPIO initializations. Signed-off-by: Renato Lui Geh <renatogeh@gmail.com> --- drivers/staging/iio/adc/ad7780.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/drivers/staging/iio/adc/ad7780.c b/drivers/staging/iio/adc/ad7780.c index 6e4357800d31..7804cd2b273e 100644 --- a/drivers/staging/iio/adc/ad7780.c +++ b/drivers/staging/iio/adc/ad7780.c @@ -250,16 +250,6 @@ static int ad7780_probe(struct spi_device *spi) ad_sd_init(&st->sd, indio_dev, spi, &ad7780_sigma_delta_info); - st->reg = devm_regulator_get(&spi->dev, "avdd"); - if (IS_ERR(st->reg)) - return PTR_ERR(st->reg); - - ret = regulator_enable(st->reg); - if (ret) { - dev_err(&spi->dev, "Failed to enable specified AVdd supply\n"); - return ret; - } - st->chip_info = &ad7780_chip_info_tbl[spi_get_device_id(spi)->driver_data]; @@ -279,7 +269,7 @@ static int ad7780_probe(struct spi_device *spi) ret = PTR_ERR(st->powerdown_gpio); dev_err(&spi->dev, "Failed to request powerdown GPIO: %d\n", ret); - goto error_disable_reg; + return ret; } if (st->chip_info->is_ad778x) { @@ -290,7 +280,7 @@ static int ad7780_probe(struct spi_device *spi) ret = PTR_ERR(st->gain_gpio); dev_err(&spi->dev, "Failed to request gain GPIO: %d\n", ret); - goto error_disable_reg; + return ret; } st->filter_gpio = devm_gpiod_get_optional(&spi->dev, @@ -301,10 +291,20 @@ static int ad7780_probe(struct spi_device *spi) dev_err(&spi->dev, "Failed to request filter GPIO: %d\n", ret); - goto error_disable_reg; + return ret; } } + st->reg = devm_regulator_get(&spi->dev, "avdd"); + if (IS_ERR(st->reg)) + return PTR_ERR(st->reg); + + ret = regulator_enable(st->reg); + if (ret) { + dev_err(&spi->dev, "Failed to enable specified AVdd supply\n"); + return ret; + } + ret = ad_sd_setup_buffer_and_trigger(indio_dev); if (ret) goto error_disable_reg; -- 2.20.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v3 2/4] staging: iio: ad7780: move regulator to after GPIO init 2019-02-05 17:13 ` [PATCH v3 2/4] staging: iio: ad7780: move regulator to after GPIO init Renato Lui Geh @ 2019-02-09 16:26 ` Jonathan Cameron 0 siblings, 0 replies; 15+ messages in thread From: Jonathan Cameron @ 2019-02-09 16:26 UTC (permalink / raw) To: Renato Lui Geh Cc: lars, Michael.Hennerich, knaack.h, pmeerw, gregkh, stefan.popa, alexandru.Ardelean, giuliano.belinassi, linux-iio, devel, linux-kernel, kernel-usp On Tue, 5 Feb 2019 15:13:21 -0200 Renato Lui Geh <renatogeh@gmail.com> wrote: > To maintain consistency between ad7780_probe and ad7780_remove orders, > regulator initialization has been moved to after GPIO initializations. > > Signed-off-by: Renato Lui Geh <renatogeh@gmail.com> This looks fine, will pick up with the earlier patches when ready. Thanks, Jonathan > --- > drivers/staging/iio/adc/ad7780.c | 26 +++++++++++++------------- > 1 file changed, 13 insertions(+), 13 deletions(-) > > diff --git a/drivers/staging/iio/adc/ad7780.c b/drivers/staging/iio/adc/ad7780.c > index 6e4357800d31..7804cd2b273e 100644 > --- a/drivers/staging/iio/adc/ad7780.c > +++ b/drivers/staging/iio/adc/ad7780.c > @@ -250,16 +250,6 @@ static int ad7780_probe(struct spi_device *spi) > > ad_sd_init(&st->sd, indio_dev, spi, &ad7780_sigma_delta_info); > > - st->reg = devm_regulator_get(&spi->dev, "avdd"); > - if (IS_ERR(st->reg)) > - return PTR_ERR(st->reg); > - > - ret = regulator_enable(st->reg); > - if (ret) { > - dev_err(&spi->dev, "Failed to enable specified AVdd supply\n"); > - return ret; > - } > - > st->chip_info = > &ad7780_chip_info_tbl[spi_get_device_id(spi)->driver_data]; > > @@ -279,7 +269,7 @@ static int ad7780_probe(struct spi_device *spi) > ret = PTR_ERR(st->powerdown_gpio); > dev_err(&spi->dev, "Failed to request powerdown GPIO: %d\n", > ret); > - goto error_disable_reg; > + return ret; > } > > if (st->chip_info->is_ad778x) { > @@ -290,7 +280,7 @@ static int ad7780_probe(struct spi_device *spi) > ret = PTR_ERR(st->gain_gpio); > dev_err(&spi->dev, "Failed to request gain GPIO: %d\n", > ret); > - goto error_disable_reg; > + return ret; > } > > st->filter_gpio = devm_gpiod_get_optional(&spi->dev, > @@ -301,10 +291,20 @@ static int ad7780_probe(struct spi_device *spi) > dev_err(&spi->dev, > "Failed to request filter GPIO: %d\n", > ret); > - goto error_disable_reg; > + return ret; > } > } > > + st->reg = devm_regulator_get(&spi->dev, "avdd"); > + if (IS_ERR(st->reg)) > + return PTR_ERR(st->reg); > + > + ret = regulator_enable(st->reg); > + if (ret) { > + dev_err(&spi->dev, "Failed to enable specified AVdd supply\n"); > + return ret; > + } > + > ret = ad_sd_setup_buffer_and_trigger(indio_dev); > if (ret) > goto error_disable_reg; ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v3 3/4] staging: iio: ad7780: add SPDX identifier 2019-02-05 17:12 [PATCH v3 0/4] staging: iio: ad7780: move out of staging Renato Lui Geh 2019-02-05 17:13 ` [PATCH v3 1/4] staging: iio: ad7780: add gain & filter gpio support Renato Lui Geh 2019-02-05 17:13 ` [PATCH v3 2/4] staging: iio: ad7780: move regulator to after GPIO init Renato Lui Geh @ 2019-02-05 17:13 ` Renato Lui Geh 2019-02-05 17:14 ` [PATCH v3 4/4] staging: iio: ad7780: moving ad7780 out of staging Renato Lui Geh 3 siblings, 0 replies; 15+ messages in thread From: Renato Lui Geh @ 2019-02-05 17:13 UTC (permalink / raw) To: lars, Michael.Hennerich, jic23, knaack.h, pmeerw, gregkh, stefan.popa, alexandru.Ardelean, giuliano.belinassi Cc: linux-iio, devel, linux-kernel, kernel-usp Add SPDX identifier (GPL-2.0) to the AD7780 driver. Signed-off-by: Renato Lui Geh <renatogeh@gmail.com> --- drivers/staging/iio/adc/ad7780.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/staging/iio/adc/ad7780.c b/drivers/staging/iio/adc/ad7780.c index 7804cd2b273e..163e3c983598 100644 --- a/drivers/staging/iio/adc/ad7780.c +++ b/drivers/staging/iio/adc/ad7780.c @@ -1,9 +1,8 @@ +// SPDX-License-Identifier: GPL-2.0 /* * AD7170/AD7171 and AD7780/AD7781 SPI ADC driver * * Copyright 2011 Analog Devices Inc. - * - * Licensed under the GPL-2. */ #include <linux/interrupt.h> -- 2.20.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v3 4/4] staging: iio: ad7780: moving ad7780 out of staging 2019-02-05 17:12 [PATCH v3 0/4] staging: iio: ad7780: move out of staging Renato Lui Geh ` (2 preceding siblings ...) 2019-02-05 17:13 ` [PATCH v3 3/4] staging: iio: ad7780: add SPDX identifier Renato Lui Geh @ 2019-02-05 17:14 ` Renato Lui Geh 2019-02-09 16:37 ` Jonathan Cameron 3 siblings, 1 reply; 15+ messages in thread From: Renato Lui Geh @ 2019-02-05 17:14 UTC (permalink / raw) To: lars, Michael.Hennerich, jic23, knaack.h, pmeerw, gregkh, stefan.popa, alexandru.Ardelean, giuliano.belinassi Cc: linux-iio, devel, linux-kernel, kernel-usp Move ad7780 ADC driver out of staging and into the mainline. The ad7780 is a sigma-delta analog to digital converter. This driver provides reading voltage values and status bits from both the ad778x and ad717x series. Its interface also allows writing on the FILTER and GAIN GPIO pins on the ad778x. Signed-off-by: Renato Lui Geh <renatogeh@gmail.com> Signed-off-by: Giuliano Belinassi <giuliano.belinassi@usp.br> Co-developed-by: Giuliano Belinassi <giuliano.belinassi@usp.br> --- Changes in v3: - Changes unrelated to moving the driver to main tree were resent as individual patches drivers/iio/adc/Kconfig | 13 ++ drivers/iio/adc/Makefile | 1 + drivers/iio/adc/ad7780.c | 359 +++++++++++++++++++++++++++++++ drivers/staging/iio/adc/Kconfig | 13 -- drivers/staging/iio/adc/Makefile | 1 - drivers/staging/iio/adc/ad7780.c | 359 ------------------------------- 6 files changed, 373 insertions(+), 373 deletions(-) create mode 100644 drivers/iio/adc/ad7780.c delete mode 100644 drivers/staging/iio/adc/ad7780.c diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig index f3cc7a31bce5..2cdee166d0e9 100644 --- a/drivers/iio/adc/Kconfig +++ b/drivers/iio/adc/Kconfig @@ -108,6 +108,19 @@ config AD7766 To compile this driver as a module, choose M here: the module will be called ad7766. +config AD7780 + tristate "Analog Devices AD7780 and similar ADCs driver" + depends on SPI + depends on GPIOLIB || COMPILE_TEST + select AD_SIGMA_DELTA + help + Say yes here to build support for Analog Devices AD7170, AD7171, + AD7780 and AD7781 SPI analog to digital converters (ADC). + If unsure, say N (but it's safe to say "Y"). + + To compile this driver as a module, choose M here: the + module will be called ad7780. + config AD7791 tristate "Analog Devices AD7791 ADC driver" depends on SPI diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile index ea5031348052..b48852157115 100644 --- a/drivers/iio/adc/Makefile +++ b/drivers/iio/adc/Makefile @@ -15,6 +15,7 @@ obj-$(CONFIG_AD7606_IFACE_PARALLEL) += ad7606_par.o obj-$(CONFIG_AD7606_IFACE_SPI) += ad7606_spi.o obj-$(CONFIG_AD7606) += ad7606.o obj-$(CONFIG_AD7766) += ad7766.o +obj-$(CONFIG_AD7780) += ad7780.o obj-$(CONFIG_AD7791) += ad7791.o obj-$(CONFIG_AD7793) += ad7793.o obj-$(CONFIG_AD7887) += ad7887.o diff --git a/drivers/iio/adc/ad7780.c b/drivers/iio/adc/ad7780.c new file mode 100644 index 000000000000..163e3c983598 --- /dev/null +++ b/drivers/iio/adc/ad7780.c @@ -0,0 +1,359 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * AD7170/AD7171 and AD7780/AD7781 SPI ADC driver + * + * Copyright 2011 Analog Devices Inc. + */ + +#include <linux/interrupt.h> +#include <linux/device.h> +#include <linux/kernel.h> +#include <linux/slab.h> +#include <linux/sysfs.h> +#include <linux/spi/spi.h> +#include <linux/regulator/consumer.h> +#include <linux/err.h> +#include <linux/sched.h> +#include <linux/gpio/consumer.h> +#include <linux/module.h> + +#include <linux/iio/iio.h> +#include <linux/iio/sysfs.h> +#include <linux/iio/adc/ad_sigma_delta.h> + +#define AD7780_RDY BIT(7) +#define AD7780_FILTER BIT(6) +#define AD7780_ERR BIT(5) +#define AD7780_ID1 BIT(4) +#define AD7780_ID0 BIT(3) +#define AD7780_GAIN BIT(2) +#define AD7780_PAT1 BIT(1) +#define AD7780_PAT0 BIT(0) + +#define AD7780_PATTERN (AD7780_PAT0) +#define AD7780_PATTERN_MASK (AD7780_PAT0 | AD7780_PAT1) + +#define AD7170_PAT2 BIT(2) + +#define AD7170_PATTERN (AD7780_PAT0 | AD7170_PAT2) +#define AD7170_PATTERN_MASK (AD7780_PAT0 | AD7780_PAT1 | AD7170_PAT2) + +#define AD7780_GAIN_GPIO 0 +#define AD7780_FILTER_GPIO 1 + +#define AD7780_GAIN_MIDPOINT 64 +#define AD7780_FILTER_MIDPOINT 13350 + +static const unsigned int ad778x_gain[2] = { 1, 128 }; +static const unsigned int ad778x_odr_avail[2] = { 10000, 16700 }; + +struct ad7780_chip_info { + struct iio_chan_spec channel; + unsigned int pattern_mask; + unsigned int pattern; + bool is_ad778x; +}; + +struct ad7780_state { + const struct ad7780_chip_info *chip_info; + struct regulator *reg; + struct gpio_desc *powerdown_gpio; + struct gpio_desc *gain_gpio; + struct gpio_desc *filter_gpio; + unsigned int gain; + unsigned int odr; + unsigned int int_vref_mv; + + struct ad_sigma_delta sd; +}; + +enum ad7780_supported_device_ids { + ID_AD7170, + ID_AD7171, + ID_AD7780, + ID_AD7781, +}; + +static struct ad7780_state *ad_sigma_delta_to_ad7780(struct ad_sigma_delta *sd) +{ + return container_of(sd, struct ad7780_state, sd); +} + +static int ad7780_set_mode(struct ad_sigma_delta *sigma_delta, + enum ad_sigma_delta_mode mode) +{ + struct ad7780_state *st = ad_sigma_delta_to_ad7780(sigma_delta); + unsigned int val; + + switch (mode) { + case AD_SD_MODE_SINGLE: + case AD_SD_MODE_CONTINUOUS: + val = 1; + break; + default: + val = 0; + break; + } + + gpiod_set_value(st->powerdown_gpio, val); + + return 0; +} + +static int ad7780_read_raw(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, + int *val, + int *val2, + long m) +{ + struct ad7780_state *st = iio_priv(indio_dev); + int voltage_uv; + + switch (m) { + case IIO_CHAN_INFO_RAW: + return ad_sigma_delta_single_conversion(indio_dev, chan, val); + case IIO_CHAN_INFO_SCALE: + voltage_uv = regulator_get_voltage(st->reg); + if (voltage_uv < 0) + return voltage_uv; + voltage_uv /= 1000; + *val = voltage_uv * st->gain; + *val2 = chan->scan_type.realbits - 1; + st->int_vref_mv = voltage_uv; + return IIO_VAL_FRACTIONAL_LOG2; + case IIO_CHAN_INFO_OFFSET: + *val = -(1 << (chan->scan_type.realbits - 1)); + return IIO_VAL_INT; + case IIO_CHAN_INFO_SAMP_FREQ: + *val = st->odr; + return IIO_VAL_INT; + } + + return -EINVAL; +} + +static int ad7780_write_raw(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, + int val, + int val2, + long m) +{ + struct ad7780_state *st = iio_priv(indio_dev); + const struct ad7780_chip_info *chip_info = st->chip_info; + unsigned long long vref; + unsigned int full_scale, gain; + + if (!chip_info->is_ad778x) + return 0; + + switch (m) { + case IIO_CHAN_INFO_SCALE: + if (val != 0) + return -EINVAL; + + vref = st->int_vref_mv * 1000000LL; + full_scale = 1 << (chip_info->channel.scan_type.realbits - 1); + gain = DIV_ROUND_CLOSEST(vref, full_scale); + gain = DIV_ROUND_CLOSEST(gain, val2); + st->gain = gain; + if (gain < AD7780_GAIN_MIDPOINT) + gain = 0; + else + gain = 1; + gpiod_set_value(st->gain_gpio, gain); + break; + case IIO_CHAN_INFO_SAMP_FREQ: + if (1000*val + val2/1000 < AD7780_FILTER_MIDPOINT) + val = 0; + else + val = 1; + st->odr = ad778x_odr_avail[val]; + gpiod_set_value(st->filter_gpio, val); + break; + } + + return 0; +} + +static int ad7780_postprocess_sample(struct ad_sigma_delta *sigma_delta, + unsigned int raw_sample) +{ + struct ad7780_state *st = ad_sigma_delta_to_ad7780(sigma_delta); + const struct ad7780_chip_info *chip_info = st->chip_info; + + if ((raw_sample & AD7780_ERR) || + ((raw_sample & chip_info->pattern_mask) != chip_info->pattern)) + return -EIO; + + if (chip_info->is_ad778x) { + st->gain = ad778x_gain[raw_sample & AD7780_GAIN]; + st->odr = ad778x_odr_avail[raw_sample & AD7780_FILTER]; + } + + return 0; +} + +static const struct ad_sigma_delta_info ad7780_sigma_delta_info = { + .set_mode = ad7780_set_mode, + .postprocess_sample = ad7780_postprocess_sample, + .has_registers = false, +}; + +#define AD7780_CHANNEL(bits, wordsize) \ + AD_SD_CHANNEL_NO_SAMP_FREQ(1, 0, 0, bits, 32, wordsize - bits) + +static const struct ad7780_chip_info ad7780_chip_info_tbl[] = { + [ID_AD7170] = { + .channel = AD7780_CHANNEL(12, 24), + .pattern = AD7170_PATTERN, + .pattern_mask = AD7170_PATTERN_MASK, + .is_ad778x = false, + }, + [ID_AD7171] = { + .channel = AD7780_CHANNEL(16, 24), + .pattern = AD7170_PATTERN, + .pattern_mask = AD7170_PATTERN_MASK, + .is_ad778x = false, + }, + [ID_AD7780] = { + .channel = AD7780_CHANNEL(24, 32), + .pattern = AD7780_PATTERN, + .pattern_mask = AD7780_PATTERN_MASK, + .is_ad778x = true, + }, + [ID_AD7781] = { + .channel = AD7780_CHANNEL(20, 32), + .pattern = AD7780_PATTERN, + .pattern_mask = AD7780_PATTERN_MASK, + .is_ad778x = true, + }, +}; + +static const struct iio_info ad7780_info = { + .read_raw = ad7780_read_raw, + .write_raw = ad7780_write_raw, +}; + +static int ad7780_probe(struct spi_device *spi) +{ + struct ad7780_state *st; + struct iio_dev *indio_dev; + int ret; + + indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st)); + if (!indio_dev) + return -ENOMEM; + + st = iio_priv(indio_dev); + st->gain = 1; + + ad_sd_init(&st->sd, indio_dev, spi, &ad7780_sigma_delta_info); + + st->chip_info = + &ad7780_chip_info_tbl[spi_get_device_id(spi)->driver_data]; + + spi_set_drvdata(spi, indio_dev); + + indio_dev->dev.parent = &spi->dev; + indio_dev->name = spi_get_device_id(spi)->name; + indio_dev->modes = INDIO_DIRECT_MODE; + indio_dev->channels = &st->chip_info->channel; + indio_dev->num_channels = 1; + indio_dev->info = &ad7780_info; + + st->powerdown_gpio = devm_gpiod_get_optional(&spi->dev, + "powerdown", + GPIOD_OUT_LOW); + if (IS_ERR(st->powerdown_gpio)) { + ret = PTR_ERR(st->powerdown_gpio); + dev_err(&spi->dev, "Failed to request powerdown GPIO: %d\n", + ret); + return ret; + } + + if (st->chip_info->is_ad778x) { + st->gain_gpio = devm_gpiod_get_optional(&spi->dev, + "gain", + GPIOD_OUT_HIGH); + if (IS_ERR(st->gain_gpio)) { + ret = PTR_ERR(st->gain_gpio); + dev_err(&spi->dev, "Failed to request gain GPIO: %d\n", + ret); + return ret; + } + + st->filter_gpio = devm_gpiod_get_optional(&spi->dev, + "filter", + GPIOD_OUT_HIGH); + if (IS_ERR(st->filter_gpio)) { + ret = PTR_ERR(st->filter_gpio); + dev_err(&spi->dev, + "Failed to request filter GPIO: %d\n", + ret); + return ret; + } + } + + st->reg = devm_regulator_get(&spi->dev, "avdd"); + if (IS_ERR(st->reg)) + return PTR_ERR(st->reg); + + ret = regulator_enable(st->reg); + if (ret) { + dev_err(&spi->dev, "Failed to enable specified AVdd supply\n"); + return ret; + } + + ret = ad_sd_setup_buffer_and_trigger(indio_dev); + if (ret) + goto error_disable_reg; + + ret = iio_device_register(indio_dev); + if (ret) + goto error_cleanup_buffer_and_trigger; + + return 0; + +error_cleanup_buffer_and_trigger: + ad_sd_cleanup_buffer_and_trigger(indio_dev); +error_disable_reg: + regulator_disable(st->reg); + + return ret; +} + +static int ad7780_remove(struct spi_device *spi) +{ + struct iio_dev *indio_dev = spi_get_drvdata(spi); + struct ad7780_state *st = iio_priv(indio_dev); + + iio_device_unregister(indio_dev); + ad_sd_cleanup_buffer_and_trigger(indio_dev); + + regulator_disable(st->reg); + + return 0; +} + +static const struct spi_device_id ad7780_id[] = { + {"ad7170", ID_AD7170}, + {"ad7171", ID_AD7171}, + {"ad7780", ID_AD7780}, + {"ad7781", ID_AD7781}, + {} +}; +MODULE_DEVICE_TABLE(spi, ad7780_id); + +static struct spi_driver ad7780_driver = { + .driver = { + .name = "ad7780", + }, + .probe = ad7780_probe, + .remove = ad7780_remove, + .id_table = ad7780_id, +}; +module_spi_driver(ad7780_driver); + +MODULE_AUTHOR("Michael Hennerich <michael.hennerich@analog.com>"); +MODULE_DESCRIPTION("Analog Devices AD7780 and similar ADCs"); +MODULE_LICENSE("GPL v2"); diff --git a/drivers/staging/iio/adc/Kconfig b/drivers/staging/iio/adc/Kconfig index 7a93d3a5c113..404a53c743a6 100644 --- a/drivers/staging/iio/adc/Kconfig +++ b/drivers/staging/iio/adc/Kconfig @@ -3,19 +3,6 @@ # menu "Analog to digital converters" -config AD7780 - tristate "Analog Devices AD7780 and similar ADCs driver" - depends on SPI - depends on GPIOLIB || COMPILE_TEST - select AD_SIGMA_DELTA - help - Say yes here to build support for Analog Devices AD7170, AD7171, - AD7780 and AD7781 SPI analog to digital converters (ADC). - If unsure, say N (but it's safe to say "Y"). - - To compile this driver as a module, choose M here: the - module will be called ad7780. - config AD7816 tristate "Analog Devices AD7816/7/8 temperature sensor and ADC driver" depends on SPI diff --git a/drivers/staging/iio/adc/Makefile b/drivers/staging/iio/adc/Makefile index 7a421088ff82..4b76769b32bc 100644 --- a/drivers/staging/iio/adc/Makefile +++ b/drivers/staging/iio/adc/Makefile @@ -3,7 +3,6 @@ # Makefile for industrial I/O ADC drivers # -obj-$(CONFIG_AD7780) += ad7780.o obj-$(CONFIG_AD7816) += ad7816.o obj-$(CONFIG_AD7192) += ad7192.o obj-$(CONFIG_AD7280) += ad7280a.o diff --git a/drivers/staging/iio/adc/ad7780.c b/drivers/staging/iio/adc/ad7780.c deleted file mode 100644 index 163e3c983598..000000000000 --- a/drivers/staging/iio/adc/ad7780.c +++ /dev/null @@ -1,359 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0 -/* - * AD7170/AD7171 and AD7780/AD7781 SPI ADC driver - * - * Copyright 2011 Analog Devices Inc. - */ - -#include <linux/interrupt.h> -#include <linux/device.h> -#include <linux/kernel.h> -#include <linux/slab.h> -#include <linux/sysfs.h> -#include <linux/spi/spi.h> -#include <linux/regulator/consumer.h> -#include <linux/err.h> -#include <linux/sched.h> -#include <linux/gpio/consumer.h> -#include <linux/module.h> - -#include <linux/iio/iio.h> -#include <linux/iio/sysfs.h> -#include <linux/iio/adc/ad_sigma_delta.h> - -#define AD7780_RDY BIT(7) -#define AD7780_FILTER BIT(6) -#define AD7780_ERR BIT(5) -#define AD7780_ID1 BIT(4) -#define AD7780_ID0 BIT(3) -#define AD7780_GAIN BIT(2) -#define AD7780_PAT1 BIT(1) -#define AD7780_PAT0 BIT(0) - -#define AD7780_PATTERN (AD7780_PAT0) -#define AD7780_PATTERN_MASK (AD7780_PAT0 | AD7780_PAT1) - -#define AD7170_PAT2 BIT(2) - -#define AD7170_PATTERN (AD7780_PAT0 | AD7170_PAT2) -#define AD7170_PATTERN_MASK (AD7780_PAT0 | AD7780_PAT1 | AD7170_PAT2) - -#define AD7780_GAIN_GPIO 0 -#define AD7780_FILTER_GPIO 1 - -#define AD7780_GAIN_MIDPOINT 64 -#define AD7780_FILTER_MIDPOINT 13350 - -static const unsigned int ad778x_gain[2] = { 1, 128 }; -static const unsigned int ad778x_odr_avail[2] = { 10000, 16700 }; - -struct ad7780_chip_info { - struct iio_chan_spec channel; - unsigned int pattern_mask; - unsigned int pattern; - bool is_ad778x; -}; - -struct ad7780_state { - const struct ad7780_chip_info *chip_info; - struct regulator *reg; - struct gpio_desc *powerdown_gpio; - struct gpio_desc *gain_gpio; - struct gpio_desc *filter_gpio; - unsigned int gain; - unsigned int odr; - unsigned int int_vref_mv; - - struct ad_sigma_delta sd; -}; - -enum ad7780_supported_device_ids { - ID_AD7170, - ID_AD7171, - ID_AD7780, - ID_AD7781, -}; - -static struct ad7780_state *ad_sigma_delta_to_ad7780(struct ad_sigma_delta *sd) -{ - return container_of(sd, struct ad7780_state, sd); -} - -static int ad7780_set_mode(struct ad_sigma_delta *sigma_delta, - enum ad_sigma_delta_mode mode) -{ - struct ad7780_state *st = ad_sigma_delta_to_ad7780(sigma_delta); - unsigned int val; - - switch (mode) { - case AD_SD_MODE_SINGLE: - case AD_SD_MODE_CONTINUOUS: - val = 1; - break; - default: - val = 0; - break; - } - - gpiod_set_value(st->powerdown_gpio, val); - - return 0; -} - -static int ad7780_read_raw(struct iio_dev *indio_dev, - struct iio_chan_spec const *chan, - int *val, - int *val2, - long m) -{ - struct ad7780_state *st = iio_priv(indio_dev); - int voltage_uv; - - switch (m) { - case IIO_CHAN_INFO_RAW: - return ad_sigma_delta_single_conversion(indio_dev, chan, val); - case IIO_CHAN_INFO_SCALE: - voltage_uv = regulator_get_voltage(st->reg); - if (voltage_uv < 0) - return voltage_uv; - voltage_uv /= 1000; - *val = voltage_uv * st->gain; - *val2 = chan->scan_type.realbits - 1; - st->int_vref_mv = voltage_uv; - return IIO_VAL_FRACTIONAL_LOG2; - case IIO_CHAN_INFO_OFFSET: - *val = -(1 << (chan->scan_type.realbits - 1)); - return IIO_VAL_INT; - case IIO_CHAN_INFO_SAMP_FREQ: - *val = st->odr; - return IIO_VAL_INT; - } - - return -EINVAL; -} - -static int ad7780_write_raw(struct iio_dev *indio_dev, - struct iio_chan_spec const *chan, - int val, - int val2, - long m) -{ - struct ad7780_state *st = iio_priv(indio_dev); - const struct ad7780_chip_info *chip_info = st->chip_info; - unsigned long long vref; - unsigned int full_scale, gain; - - if (!chip_info->is_ad778x) - return 0; - - switch (m) { - case IIO_CHAN_INFO_SCALE: - if (val != 0) - return -EINVAL; - - vref = st->int_vref_mv * 1000000LL; - full_scale = 1 << (chip_info->channel.scan_type.realbits - 1); - gain = DIV_ROUND_CLOSEST(vref, full_scale); - gain = DIV_ROUND_CLOSEST(gain, val2); - st->gain = gain; - if (gain < AD7780_GAIN_MIDPOINT) - gain = 0; - else - gain = 1; - gpiod_set_value(st->gain_gpio, gain); - break; - case IIO_CHAN_INFO_SAMP_FREQ: - if (1000*val + val2/1000 < AD7780_FILTER_MIDPOINT) - val = 0; - else - val = 1; - st->odr = ad778x_odr_avail[val]; - gpiod_set_value(st->filter_gpio, val); - break; - } - - return 0; -} - -static int ad7780_postprocess_sample(struct ad_sigma_delta *sigma_delta, - unsigned int raw_sample) -{ - struct ad7780_state *st = ad_sigma_delta_to_ad7780(sigma_delta); - const struct ad7780_chip_info *chip_info = st->chip_info; - - if ((raw_sample & AD7780_ERR) || - ((raw_sample & chip_info->pattern_mask) != chip_info->pattern)) - return -EIO; - - if (chip_info->is_ad778x) { - st->gain = ad778x_gain[raw_sample & AD7780_GAIN]; - st->odr = ad778x_odr_avail[raw_sample & AD7780_FILTER]; - } - - return 0; -} - -static const struct ad_sigma_delta_info ad7780_sigma_delta_info = { - .set_mode = ad7780_set_mode, - .postprocess_sample = ad7780_postprocess_sample, - .has_registers = false, -}; - -#define AD7780_CHANNEL(bits, wordsize) \ - AD_SD_CHANNEL_NO_SAMP_FREQ(1, 0, 0, bits, 32, wordsize - bits) - -static const struct ad7780_chip_info ad7780_chip_info_tbl[] = { - [ID_AD7170] = { - .channel = AD7780_CHANNEL(12, 24), - .pattern = AD7170_PATTERN, - .pattern_mask = AD7170_PATTERN_MASK, - .is_ad778x = false, - }, - [ID_AD7171] = { - .channel = AD7780_CHANNEL(16, 24), - .pattern = AD7170_PATTERN, - .pattern_mask = AD7170_PATTERN_MASK, - .is_ad778x = false, - }, - [ID_AD7780] = { - .channel = AD7780_CHANNEL(24, 32), - .pattern = AD7780_PATTERN, - .pattern_mask = AD7780_PATTERN_MASK, - .is_ad778x = true, - }, - [ID_AD7781] = { - .channel = AD7780_CHANNEL(20, 32), - .pattern = AD7780_PATTERN, - .pattern_mask = AD7780_PATTERN_MASK, - .is_ad778x = true, - }, -}; - -static const struct iio_info ad7780_info = { - .read_raw = ad7780_read_raw, - .write_raw = ad7780_write_raw, -}; - -static int ad7780_probe(struct spi_device *spi) -{ - struct ad7780_state *st; - struct iio_dev *indio_dev; - int ret; - - indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st)); - if (!indio_dev) - return -ENOMEM; - - st = iio_priv(indio_dev); - st->gain = 1; - - ad_sd_init(&st->sd, indio_dev, spi, &ad7780_sigma_delta_info); - - st->chip_info = - &ad7780_chip_info_tbl[spi_get_device_id(spi)->driver_data]; - - spi_set_drvdata(spi, indio_dev); - - indio_dev->dev.parent = &spi->dev; - indio_dev->name = spi_get_device_id(spi)->name; - indio_dev->modes = INDIO_DIRECT_MODE; - indio_dev->channels = &st->chip_info->channel; - indio_dev->num_channels = 1; - indio_dev->info = &ad7780_info; - - st->powerdown_gpio = devm_gpiod_get_optional(&spi->dev, - "powerdown", - GPIOD_OUT_LOW); - if (IS_ERR(st->powerdown_gpio)) { - ret = PTR_ERR(st->powerdown_gpio); - dev_err(&spi->dev, "Failed to request powerdown GPIO: %d\n", - ret); - return ret; - } - - if (st->chip_info->is_ad778x) { - st->gain_gpio = devm_gpiod_get_optional(&spi->dev, - "gain", - GPIOD_OUT_HIGH); - if (IS_ERR(st->gain_gpio)) { - ret = PTR_ERR(st->gain_gpio); - dev_err(&spi->dev, "Failed to request gain GPIO: %d\n", - ret); - return ret; - } - - st->filter_gpio = devm_gpiod_get_optional(&spi->dev, - "filter", - GPIOD_OUT_HIGH); - if (IS_ERR(st->filter_gpio)) { - ret = PTR_ERR(st->filter_gpio); - dev_err(&spi->dev, - "Failed to request filter GPIO: %d\n", - ret); - return ret; - } - } - - st->reg = devm_regulator_get(&spi->dev, "avdd"); - if (IS_ERR(st->reg)) - return PTR_ERR(st->reg); - - ret = regulator_enable(st->reg); - if (ret) { - dev_err(&spi->dev, "Failed to enable specified AVdd supply\n"); - return ret; - } - - ret = ad_sd_setup_buffer_and_trigger(indio_dev); - if (ret) - goto error_disable_reg; - - ret = iio_device_register(indio_dev); - if (ret) - goto error_cleanup_buffer_and_trigger; - - return 0; - -error_cleanup_buffer_and_trigger: - ad_sd_cleanup_buffer_and_trigger(indio_dev); -error_disable_reg: - regulator_disable(st->reg); - - return ret; -} - -static int ad7780_remove(struct spi_device *spi) -{ - struct iio_dev *indio_dev = spi_get_drvdata(spi); - struct ad7780_state *st = iio_priv(indio_dev); - - iio_device_unregister(indio_dev); - ad_sd_cleanup_buffer_and_trigger(indio_dev); - - regulator_disable(st->reg); - - return 0; -} - -static const struct spi_device_id ad7780_id[] = { - {"ad7170", ID_AD7170}, - {"ad7171", ID_AD7171}, - {"ad7780", ID_AD7780}, - {"ad7781", ID_AD7781}, - {} -}; -MODULE_DEVICE_TABLE(spi, ad7780_id); - -static struct spi_driver ad7780_driver = { - .driver = { - .name = "ad7780", - }, - .probe = ad7780_probe, - .remove = ad7780_remove, - .id_table = ad7780_id, -}; -module_spi_driver(ad7780_driver); - -MODULE_AUTHOR("Michael Hennerich <michael.hennerich@analog.com>"); -MODULE_DESCRIPTION("Analog Devices AD7780 and similar ADCs"); -MODULE_LICENSE("GPL v2"); -- 2.20.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v3 4/4] staging: iio: ad7780: moving ad7780 out of staging 2019-02-05 17:14 ` [PATCH v3 4/4] staging: iio: ad7780: moving ad7780 out of staging Renato Lui Geh @ 2019-02-09 16:37 ` Jonathan Cameron 2019-02-14 20:48 ` Renato Lui Geh 0 siblings, 1 reply; 15+ messages in thread From: Jonathan Cameron @ 2019-02-09 16:37 UTC (permalink / raw) To: Renato Lui Geh Cc: lars, Michael.Hennerich, knaack.h, pmeerw, gregkh, stefan.popa, alexandru.Ardelean, giuliano.belinassi, linux-iio, devel, linux-kernel, kernel-usp On Tue, 5 Feb 2019 15:14:03 -0200 Renato Lui Geh <renatogeh@gmail.com> wrote: > Move ad7780 ADC driver out of staging and into the mainline. > > The ad7780 is a sigma-delta analog to digital converter. This driver provides > reading voltage values and status bits from both the ad778x and ad717x series. > Its interface also allows writing on the FILTER and GAIN GPIO pins on the > ad778x. > > Signed-off-by: Renato Lui Geh <renatogeh@gmail.com> > Signed-off-by: Giuliano Belinassi <giuliano.belinassi@usp.br> > Co-developed-by: Giuliano Belinassi <giuliano.belinassi@usp.br> This needs a device tree binding doc which should be reviewed before we move the driver out of staging. Make sure to cc the dt-binding maintainers and list. Doesn't really matter if that patch is before or after this one in the series but needs to be in the same series. There are a few more minor tidy ups that would be nice to have inline given you are doing a v4. Stuff like this could have been cleaned up after moving out of staging (nothing wrong with improving non staging drivers after all) but always better to do it whilst we remember! > --- > Changes in v3: > - Changes unrelated to moving the driver to main tree were resent as > individual patches > > drivers/iio/adc/Kconfig | 13 ++ > drivers/iio/adc/Makefile | 1 + > drivers/iio/adc/ad7780.c | 359 +++++++++++++++++++++++++++++++ > drivers/staging/iio/adc/Kconfig | 13 -- > drivers/staging/iio/adc/Makefile | 1 - > drivers/staging/iio/adc/ad7780.c | 359 ------------------------------- > 6 files changed, 373 insertions(+), 373 deletions(-) > create mode 100644 drivers/iio/adc/ad7780.c > delete mode 100644 drivers/staging/iio/adc/ad7780.c > > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig > index f3cc7a31bce5..2cdee166d0e9 100644 > --- a/drivers/iio/adc/Kconfig > +++ b/drivers/iio/adc/Kconfig > @@ -108,6 +108,19 @@ config AD7766 > To compile this driver as a module, choose M here: the module will be > called ad7766. > > +config AD7780 > + tristate "Analog Devices AD7780 and similar ADCs driver" > + depends on SPI > + depends on GPIOLIB || COMPILE_TEST > + select AD_SIGMA_DELTA > + help > + Say yes here to build support for Analog Devices AD7170, AD7171, > + AD7780 and AD7781 SPI analog to digital converters (ADC). > + If unsure, say N (but it's safe to say "Y"). I wouldn't bother with this statement, doesn't add any real info! > + > + To compile this driver as a module, choose M here: the > + module will be called ad7780. > + > config AD7791 > tristate "Analog Devices AD7791 ADC driver" > depends on SPI > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile > index ea5031348052..b48852157115 100644 > --- a/drivers/iio/adc/Makefile > +++ b/drivers/iio/adc/Makefile > @@ -15,6 +15,7 @@ obj-$(CONFIG_AD7606_IFACE_PARALLEL) += ad7606_par.o > obj-$(CONFIG_AD7606_IFACE_SPI) += ad7606_spi.o > obj-$(CONFIG_AD7606) += ad7606.o > obj-$(CONFIG_AD7766) += ad7766.o > +obj-$(CONFIG_AD7780) += ad7780.o > obj-$(CONFIG_AD7791) += ad7791.o > obj-$(CONFIG_AD7793) += ad7793.o > obj-$(CONFIG_AD7887) += ad7887.o > diff --git a/drivers/iio/adc/ad7780.c b/drivers/iio/adc/ad7780.c > new file mode 100644 > index 000000000000..163e3c983598 > --- /dev/null > +++ b/drivers/iio/adc/ad7780.c > @@ -0,0 +1,359 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * AD7170/AD7171 and AD7780/AD7781 SPI ADC driver > + * > + * Copyright 2011 Analog Devices Inc. I think you have done more than enough to this driver to add an additional copyright line if you want to! > + */ > + > +#include <linux/interrupt.h> > +#include <linux/device.h> > +#include <linux/kernel.h> > +#include <linux/slab.h> > +#include <linux/sysfs.h> > +#include <linux/spi/spi.h> > +#include <linux/regulator/consumer.h> > +#include <linux/err.h> > +#include <linux/sched.h> > +#include <linux/gpio/consumer.h> > +#include <linux/module.h> > + > +#include <linux/iio/iio.h> > +#include <linux/iio/sysfs.h> > +#include <linux/iio/adc/ad_sigma_delta.h> > + > +#define AD7780_RDY BIT(7) > +#define AD7780_FILTER BIT(6) > +#define AD7780_ERR BIT(5) > +#define AD7780_ID1 BIT(4) > +#define AD7780_ID0 BIT(3) > +#define AD7780_GAIN BIT(2) > +#define AD7780_PAT1 BIT(1) > +#define AD7780_PAT0 BIT(0) These two bits of pattern don't really add anything. I'd drop them in favour of something like #define AD7780_PATTERN_GOOD 1 #define AD7780_PATTERN_MASK GENMASK(1, 0) Same for ID for that matter. These aren't one bit fields, so we shouldn't ever present them as such (though the datasheet confusingly sort of does so!) > + > +#define AD7780_PATTERN (AD7780_PAT0) > +#define AD7780_PATTERN_MASK (AD7780_PAT0 | AD7780_PAT1) > + > +#define AD7170_PAT2 BIT(2) > + > +#define AD7170_PATTERN (AD7780_PAT0 | AD7170_PAT2) > +#define AD7170_PATTERN_MASK (AD7780_PAT0 | AD7780_PAT1 | AD7170_PAT2) I'd use a value for the pattern directly and GENMASK for the mask. > + > +#define AD7780_GAIN_GPIO 0 > +#define AD7780_FILTER_GPIO 1 > + > +#define AD7780_GAIN_MIDPOINT 64 > +#define AD7780_FILTER_MIDPOINT 13350 > + > +static const unsigned int ad778x_gain[2] = { 1, 128 }; > +static const unsigned int ad778x_odr_avail[2] = { 10000, 16700 }; > + > +struct ad7780_chip_info { > + struct iio_chan_spec channel; > + unsigned int pattern_mask; > + unsigned int pattern; > + bool is_ad778x; > +}; > + > +struct ad7780_state { > + const struct ad7780_chip_info *chip_info; > + struct regulator *reg; > + struct gpio_desc *powerdown_gpio; > + struct gpio_desc *gain_gpio; > + struct gpio_desc *filter_gpio; > + unsigned int gain; > + unsigned int odr; > + unsigned int int_vref_mv; > + > + struct ad_sigma_delta sd; > +}; > + > +enum ad7780_supported_device_ids { > + ID_AD7170, > + ID_AD7171, > + ID_AD7780, > + ID_AD7781, > +}; > + > +static struct ad7780_state *ad_sigma_delta_to_ad7780(struct ad_sigma_delta *sd) > +{ > + return container_of(sd, struct ad7780_state, sd); > +} > + > +static int ad7780_set_mode(struct ad_sigma_delta *sigma_delta, > + enum ad_sigma_delta_mode mode) > +{ > + struct ad7780_state *st = ad_sigma_delta_to_ad7780(sigma_delta); > + unsigned int val; > + > + switch (mode) { > + case AD_SD_MODE_SINGLE: > + case AD_SD_MODE_CONTINUOUS: > + val = 1; > + break; > + default: > + val = 0; > + break; > + } > + > + gpiod_set_value(st->powerdown_gpio, val); > + > + return 0; > +} > + > +static int ad7780_read_raw(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, > + int *val, > + int *val2, > + long m) > +{ > + struct ad7780_state *st = iio_priv(indio_dev); > + int voltage_uv; > + > + switch (m) { > + case IIO_CHAN_INFO_RAW: > + return ad_sigma_delta_single_conversion(indio_dev, chan, val); > + case IIO_CHAN_INFO_SCALE: > + voltage_uv = regulator_get_voltage(st->reg); > + if (voltage_uv < 0) > + return voltage_uv; > + voltage_uv /= 1000; > + *val = voltage_uv * st->gain; > + *val2 = chan->scan_type.realbits - 1; > + st->int_vref_mv = voltage_uv; > + return IIO_VAL_FRACTIONAL_LOG2; > + case IIO_CHAN_INFO_OFFSET: > + *val = -(1 << (chan->scan_type.realbits - 1)); > + return IIO_VAL_INT; > + case IIO_CHAN_INFO_SAMP_FREQ: > + *val = st->odr; > + return IIO_VAL_INT; > + } > + > + return -EINVAL; > +} > + > +static int ad7780_write_raw(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, > + int val, > + int val2, > + long m) > +{ > + struct ad7780_state *st = iio_priv(indio_dev); > + const struct ad7780_chip_info *chip_info = st->chip_info; > + unsigned long long vref; > + unsigned int full_scale, gain; > + > + if (!chip_info->is_ad778x) > + return 0; > + > + switch (m) { > + case IIO_CHAN_INFO_SCALE: > + if (val != 0) > + return -EINVAL; > + > + vref = st->int_vref_mv * 1000000LL; > + full_scale = 1 << (chip_info->channel.scan_type.realbits - 1); > + gain = DIV_ROUND_CLOSEST(vref, full_scale); > + gain = DIV_ROUND_CLOSEST(gain, val2); > + st->gain = gain; > + if (gain < AD7780_GAIN_MIDPOINT) > + gain = 0; > + else > + gain = 1; > + gpiod_set_value(st->gain_gpio, gain); > + break; > + case IIO_CHAN_INFO_SAMP_FREQ: > + if (1000*val + val2/1000 < AD7780_FILTER_MIDPOINT) > + val = 0; > + else > + val = 1; > + st->odr = ad778x_odr_avail[val]; > + gpiod_set_value(st->filter_gpio, val); > + break; We'll get a warning here due to the lack of a default handler. It's pointless except to suppress the warning, but best to add one. > + } > + > + return 0; > +} > + > +static int ad7780_postprocess_sample(struct ad_sigma_delta *sigma_delta, > + unsigned int raw_sample) > +{ > + struct ad7780_state *st = ad_sigma_delta_to_ad7780(sigma_delta); > + const struct ad7780_chip_info *chip_info = st->chip_info; > + > + if ((raw_sample & AD7780_ERR) || > + ((raw_sample & chip_info->pattern_mask) != chip_info->pattern)) > + return -EIO; > + > + if (chip_info->is_ad778x) { > + st->gain = ad778x_gain[raw_sample & AD7780_GAIN]; > + st->odr = ad778x_odr_avail[raw_sample & AD7780_FILTER]; > + } > + > + return 0; > +} > + > +static const struct ad_sigma_delta_info ad7780_sigma_delta_info = { > + .set_mode = ad7780_set_mode, > + .postprocess_sample = ad7780_postprocess_sample, > + .has_registers = false, > +}; > + > +#define AD7780_CHANNEL(bits, wordsize) \ > + AD_SD_CHANNEL_NO_SAMP_FREQ(1, 0, 0, bits, 32, wordsize - bits) > + > +static const struct ad7780_chip_info ad7780_chip_info_tbl[] = { > + [ID_AD7170] = { > + .channel = AD7780_CHANNEL(12, 24), > + .pattern = AD7170_PATTERN, > + .pattern_mask = AD7170_PATTERN_MASK, > + .is_ad778x = false, > + }, > + [ID_AD7171] = { > + .channel = AD7780_CHANNEL(16, 24), > + .pattern = AD7170_PATTERN, > + .pattern_mask = AD7170_PATTERN_MASK, > + .is_ad778x = false, > + }, > + [ID_AD7780] = { > + .channel = AD7780_CHANNEL(24, 32), > + .pattern = AD7780_PATTERN, > + .pattern_mask = AD7780_PATTERN_MASK, > + .is_ad778x = true, > + }, > + [ID_AD7781] = { > + .channel = AD7780_CHANNEL(20, 32), > + .pattern = AD7780_PATTERN, > + .pattern_mask = AD7780_PATTERN_MASK, > + .is_ad778x = true, > + }, > +}; > + > +static const struct iio_info ad7780_info = { > + .read_raw = ad7780_read_raw, > + .write_raw = ad7780_write_raw, > +}; > + > +static int ad7780_probe(struct spi_device *spi) > +{ > + struct ad7780_state *st; > + struct iio_dev *indio_dev; > + int ret; > + > + indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st)); > + if (!indio_dev) > + return -ENOMEM; > + > + st = iio_priv(indio_dev); > + st->gain = 1; > + > + ad_sd_init(&st->sd, indio_dev, spi, &ad7780_sigma_delta_info); > + > + st->chip_info = > + &ad7780_chip_info_tbl[spi_get_device_id(spi)->driver_data]; > + > + spi_set_drvdata(spi, indio_dev); > + > + indio_dev->dev.parent = &spi->dev; > + indio_dev->name = spi_get_device_id(spi)->name; > + indio_dev->modes = INDIO_DIRECT_MODE; > + indio_dev->channels = &st->chip_info->channel; > + indio_dev->num_channels = 1; > + indio_dev->info = &ad7780_info; > + > + st->powerdown_gpio = devm_gpiod_get_optional(&spi->dev, > + "powerdown", > + GPIOD_OUT_LOW); > + if (IS_ERR(st->powerdown_gpio)) { > + ret = PTR_ERR(st->powerdown_gpio); > + dev_err(&spi->dev, "Failed to request powerdown GPIO: %d\n", > + ret); > + return ret; > + } > + > + if (st->chip_info->is_ad778x) { > + st->gain_gpio = devm_gpiod_get_optional(&spi->dev, > + "gain", > + GPIOD_OUT_HIGH); > + if (IS_ERR(st->gain_gpio)) { > + ret = PTR_ERR(st->gain_gpio); > + dev_err(&spi->dev, "Failed to request gain GPIO: %d\n", > + ret); > + return ret; > + } > + > + st->filter_gpio = devm_gpiod_get_optional(&spi->dev, > + "filter", > + GPIOD_OUT_HIGH); > + if (IS_ERR(st->filter_gpio)) { > + ret = PTR_ERR(st->filter_gpio); > + dev_err(&spi->dev, > + "Failed to request filter GPIO: %d\n", > + ret); > + return ret; > + } > + } > + > + st->reg = devm_regulator_get(&spi->dev, "avdd"); > + if (IS_ERR(st->reg)) > + return PTR_ERR(st->reg); > + > + ret = regulator_enable(st->reg); > + if (ret) { > + dev_err(&spi->dev, "Failed to enable specified AVdd supply\n"); > + return ret; > + } > + > + ret = ad_sd_setup_buffer_and_trigger(indio_dev); > + if (ret) > + goto error_disable_reg; > + > + ret = iio_device_register(indio_dev); > + if (ret) > + goto error_cleanup_buffer_and_trigger; > + > + return 0; > + > +error_cleanup_buffer_and_trigger: > + ad_sd_cleanup_buffer_and_trigger(indio_dev); > +error_disable_reg: > + regulator_disable(st->reg); > + > + return ret; > +} > + > +static int ad7780_remove(struct spi_device *spi) > +{ > + struct iio_dev *indio_dev = spi_get_drvdata(spi); > + struct ad7780_state *st = iio_priv(indio_dev); > + > + iio_device_unregister(indio_dev); > + ad_sd_cleanup_buffer_and_trigger(indio_dev); > + > + regulator_disable(st->reg); > + > + return 0; > +} > + > +static const struct spi_device_id ad7780_id[] = { > + {"ad7170", ID_AD7170}, > + {"ad7171", ID_AD7171}, > + {"ad7780", ID_AD7780}, > + {"ad7781", ID_AD7781}, > + {} > +}; > +MODULE_DEVICE_TABLE(spi, ad7780_id); > + > +static struct spi_driver ad7780_driver = { > + .driver = { > + .name = "ad7780", > + }, > + .probe = ad7780_probe, > + .remove = ad7780_remove, > + .id_table = ad7780_id, > +}; > +module_spi_driver(ad7780_driver); > + > +MODULE_AUTHOR("Michael Hennerich <michael.hennerich@analog.com>"); > +MODULE_DESCRIPTION("Analog Devices AD7780 and similar ADCs"); > +MODULE_LICENSE("GPL v2"); ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 4/4] staging: iio: ad7780: moving ad7780 out of staging 2019-02-09 16:37 ` Jonathan Cameron @ 2019-02-14 20:48 ` Renato Lui Geh 2019-02-18 14:50 ` Jonathan Cameron 0 siblings, 1 reply; 15+ messages in thread From: Renato Lui Geh @ 2019-02-14 20:48 UTC (permalink / raw) To: Jonathan Cameron Cc: Renato Lui Geh, lars, Michael.Hennerich, knaack.h, pmeerw, gregkh, stefan.popa, alexandru.Ardelean, giuliano.belinassi, linux-iio, devel, linux-kernel, kernel-usp On 02/09, Jonathan Cameron wrote: >On Tue, 5 Feb 2019 15:14:03 -0200 >Renato Lui Geh <renatogeh@gmail.com> wrote: > >> Move ad7780 ADC driver out of staging and into the mainline. >> >> The ad7780 is a sigma-delta analog to digital converter. This driver provides >> reading voltage values and status bits from both the ad778x and ad717x series. >> Its interface also allows writing on the FILTER and GAIN GPIO pins on the >> ad778x. >> >> Signed-off-by: Renato Lui Geh <renatogeh@gmail.com> >> Signed-off-by: Giuliano Belinassi <giuliano.belinassi@usp.br> >> Co-developed-by: Giuliano Belinassi <giuliano.belinassi@usp.br> > >This needs a device tree binding doc which should be reviewed before we move >the driver out of staging. Make sure to cc the dt-binding maintainers and >list. Doesn't really matter if that patch is before or after this one >in the series but needs to be in the same series. Ok! I see that some Analog dt-bindings are prefixed by adi and some are not. Should I follow any naming standard? > >There are a few more minor tidy ups that would be nice to have inline >given you are doing a v4. Stuff like this could have been cleaned up >after moving out of staging (nothing wrong with improving non staging >drivers after all) but always better to do it whilst we remember! > >> --- >> Changes in v3: >> - Changes unrelated to moving the driver to main tree were resent as >> individual patches >> >> drivers/iio/adc/Kconfig | 13 ++ >> drivers/iio/adc/Makefile | 1 + >> drivers/iio/adc/ad7780.c | 359 +++++++++++++++++++++++++++++++ >> drivers/staging/iio/adc/Kconfig | 13 -- >> drivers/staging/iio/adc/Makefile | 1 - >> drivers/staging/iio/adc/ad7780.c | 359 ------------------------------- >> 6 files changed, 373 insertions(+), 373 deletions(-) >> create mode 100644 drivers/iio/adc/ad7780.c >> delete mode 100644 drivers/staging/iio/adc/ad7780.c >> >> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig >> index f3cc7a31bce5..2cdee166d0e9 100644 >> --- a/drivers/iio/adc/Kconfig >> +++ b/drivers/iio/adc/Kconfig >> @@ -108,6 +108,19 @@ config AD7766 >> To compile this driver as a module, choose M here: the module will be >> called ad7766. >> >> +config AD7780 >> + tristate "Analog Devices AD7780 and similar ADCs driver" >> + depends on SPI >> + depends on GPIOLIB || COMPILE_TEST >> + select AD_SIGMA_DELTA >> + help >> + Say yes here to build support for Analog Devices AD7170, AD7171, >> + AD7780 and AD7781 SPI analog to digital converters (ADC). >> + If unsure, say N (but it's safe to say "Y"). > >I wouldn't bother with this statement, doesn't add any real info! > >> + >> + To compile this driver as a module, choose M here: the >> + module will be called ad7780. >> + >> config AD7791 >> tristate "Analog Devices AD7791 ADC driver" >> depends on SPI >> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile >> index ea5031348052..b48852157115 100644 >> --- a/drivers/iio/adc/Makefile >> +++ b/drivers/iio/adc/Makefile >> @@ -15,6 +15,7 @@ obj-$(CONFIG_AD7606_IFACE_PARALLEL) += ad7606_par.o >> obj-$(CONFIG_AD7606_IFACE_SPI) += ad7606_spi.o >> obj-$(CONFIG_AD7606) += ad7606.o >> obj-$(CONFIG_AD7766) += ad7766.o >> +obj-$(CONFIG_AD7780) += ad7780.o >> obj-$(CONFIG_AD7791) += ad7791.o >> obj-$(CONFIG_AD7793) += ad7793.o >> obj-$(CONFIG_AD7887) += ad7887.o >> diff --git a/drivers/iio/adc/ad7780.c b/drivers/iio/adc/ad7780.c >> new file mode 100644 >> index 000000000000..163e3c983598 >> --- /dev/null >> +++ b/drivers/iio/adc/ad7780.c >> @@ -0,0 +1,359 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * AD7170/AD7171 and AD7780/AD7781 SPI ADC driver >> + * >> + * Copyright 2011 Analog Devices Inc. > >I think you have done more than enough to this driver to add >an additional copyright line if you want to! Oh wow, that'd be awesome! Thanks! Should I just add my full name there? > >> + */ >> + >> +#include <linux/interrupt.h> >> +#include <linux/device.h> >> +#include <linux/kernel.h> >> +#include <linux/slab.h> >> +#include <linux/sysfs.h> >> +#include <linux/spi/spi.h> >> +#include <linux/regulator/consumer.h> >> +#include <linux/err.h> >> +#include <linux/sched.h> >> +#include <linux/gpio/consumer.h> >> +#include <linux/module.h> >> + >> +#include <linux/iio/iio.h> >> +#include <linux/iio/sysfs.h> >> +#include <linux/iio/adc/ad_sigma_delta.h> >> + >> +#define AD7780_RDY BIT(7) >> +#define AD7780_FILTER BIT(6) >> +#define AD7780_ERR BIT(5) >> +#define AD7780_ID1 BIT(4) >> +#define AD7780_ID0 BIT(3) >> +#define AD7780_GAIN BIT(2) >> +#define AD7780_PAT1 BIT(1) >> +#define AD7780_PAT0 BIT(0) >These two bits of pattern don't really add anything. I'd drop them in >favour of something like > >#define AD7780_PATTERN_GOOD 1 >#define AD7780_PATTERN_MASK GENMASK(1, 0) > >Same for ID for that matter. These aren't one bit fields, so we shouldn't >ever present them as such (though the datasheet confusingly sort of does >so!) > >> + >> +#define AD7780_PATTERN (AD7780_PAT0) >> +#define AD7780_PATTERN_MASK (AD7780_PAT0 | AD7780_PAT1) >> + >> +#define AD7170_PAT2 BIT(2) >> + >> +#define AD7170_PATTERN (AD7780_PAT0 | AD7170_PAT2) >> +#define AD7170_PATTERN_MASK (AD7780_PAT0 | AD7780_PAT1 | AD7170_PAT2) >I'd use a value for the pattern directly and >GENMASK for the mask. > >> + >> +#define AD7780_GAIN_GPIO 0 >> +#define AD7780_FILTER_GPIO 1 >> + >> +#define AD7780_GAIN_MIDPOINT 64 >> +#define AD7780_FILTER_MIDPOINT 13350 >> + >> +static const unsigned int ad778x_gain[2] = { 1, 128 }; >> +static const unsigned int ad778x_odr_avail[2] = { 10000, 16700 }; >> + >> +struct ad7780_chip_info { >> + struct iio_chan_spec channel; >> + unsigned int pattern_mask; >> + unsigned int pattern; >> + bool is_ad778x; >> +}; >> + >> +struct ad7780_state { >> + const struct ad7780_chip_info *chip_info; >> + struct regulator *reg; >> + struct gpio_desc *powerdown_gpio; >> + struct gpio_desc *gain_gpio; >> + struct gpio_desc *filter_gpio; >> + unsigned int gain; >> + unsigned int odr; >> + unsigned int int_vref_mv; >> + >> + struct ad_sigma_delta sd; >> +}; >> + >> +enum ad7780_supported_device_ids { >> + ID_AD7170, >> + ID_AD7171, >> + ID_AD7780, >> + ID_AD7781, >> +}; >> + >> +static struct ad7780_state *ad_sigma_delta_to_ad7780(struct ad_sigma_delta *sd) >> +{ >> + return container_of(sd, struct ad7780_state, sd); >> +} >> + >> +static int ad7780_set_mode(struct ad_sigma_delta *sigma_delta, >> + enum ad_sigma_delta_mode mode) >> +{ >> + struct ad7780_state *st = ad_sigma_delta_to_ad7780(sigma_delta); >> + unsigned int val; >> + >> + switch (mode) { >> + case AD_SD_MODE_SINGLE: >> + case AD_SD_MODE_CONTINUOUS: >> + val = 1; >> + break; >> + default: >> + val = 0; >> + break; >> + } >> + >> + gpiod_set_value(st->powerdown_gpio, val); >> + >> + return 0; >> +} >> + >> +static int ad7780_read_raw(struct iio_dev *indio_dev, >> + struct iio_chan_spec const *chan, >> + int *val, >> + int *val2, >> + long m) >> +{ >> + struct ad7780_state *st = iio_priv(indio_dev); >> + int voltage_uv; >> + >> + switch (m) { >> + case IIO_CHAN_INFO_RAW: >> + return ad_sigma_delta_single_conversion(indio_dev, chan, val); >> + case IIO_CHAN_INFO_SCALE: >> + voltage_uv = regulator_get_voltage(st->reg); >> + if (voltage_uv < 0) >> + return voltage_uv; >> + voltage_uv /= 1000; >> + *val = voltage_uv * st->gain; >> + *val2 = chan->scan_type.realbits - 1; >> + st->int_vref_mv = voltage_uv; >> + return IIO_VAL_FRACTIONAL_LOG2; >> + case IIO_CHAN_INFO_OFFSET: >> + *val = -(1 << (chan->scan_type.realbits - 1)); >> + return IIO_VAL_INT; >> + case IIO_CHAN_INFO_SAMP_FREQ: >> + *val = st->odr; >> + return IIO_VAL_INT; >> + } >> + >> + return -EINVAL; >> +} >> + >> +static int ad7780_write_raw(struct iio_dev *indio_dev, >> + struct iio_chan_spec const *chan, >> + int val, >> + int val2, >> + long m) >> +{ >> + struct ad7780_state *st = iio_priv(indio_dev); >> + const struct ad7780_chip_info *chip_info = st->chip_info; >> + unsigned long long vref; >> + unsigned int full_scale, gain; >> + >> + if (!chip_info->is_ad778x) >> + return 0; >> + >> + switch (m) { >> + case IIO_CHAN_INFO_SCALE: >> + if (val != 0) >> + return -EINVAL; >> + >> + vref = st->int_vref_mv * 1000000LL; >> + full_scale = 1 << (chip_info->channel.scan_type.realbits - 1); >> + gain = DIV_ROUND_CLOSEST(vref, full_scale); >> + gain = DIV_ROUND_CLOSEST(gain, val2); >> + st->gain = gain; >> + if (gain < AD7780_GAIN_MIDPOINT) >> + gain = 0; >> + else >> + gain = 1; >> + gpiod_set_value(st->gain_gpio, gain); >> + break; >> + case IIO_CHAN_INFO_SAMP_FREQ: >> + if (1000*val + val2/1000 < AD7780_FILTER_MIDPOINT) >> + val = 0; >> + else >> + val = 1; >> + st->odr = ad778x_odr_avail[val]; >> + gpiod_set_value(st->filter_gpio, val); >> + break; >We'll get a warning here due to the lack of a default handler. >It's pointless except to suppress the warning, but best to add one. > >> + } >> + >> + return 0; >> +} >> + >> +static int ad7780_postprocess_sample(struct ad_sigma_delta *sigma_delta, >> + unsigned int raw_sample) >> +{ >> + struct ad7780_state *st = ad_sigma_delta_to_ad7780(sigma_delta); >> + const struct ad7780_chip_info *chip_info = st->chip_info; >> + >> + if ((raw_sample & AD7780_ERR) || >> + ((raw_sample & chip_info->pattern_mask) != chip_info->pattern)) >> + return -EIO; >> + >> + if (chip_info->is_ad778x) { >> + st->gain = ad778x_gain[raw_sample & AD7780_GAIN]; >> + st->odr = ad778x_odr_avail[raw_sample & AD7780_FILTER]; >> + } >> + >> + return 0; >> +} >> + >> +static const struct ad_sigma_delta_info ad7780_sigma_delta_info = { >> + .set_mode = ad7780_set_mode, >> + .postprocess_sample = ad7780_postprocess_sample, >> + .has_registers = false, >> +}; >> + >> +#define AD7780_CHANNEL(bits, wordsize) \ >> + AD_SD_CHANNEL_NO_SAMP_FREQ(1, 0, 0, bits, 32, wordsize - bits) >> + >> +static const struct ad7780_chip_info ad7780_chip_info_tbl[] = { >> + [ID_AD7170] = { >> + .channel = AD7780_CHANNEL(12, 24), >> + .pattern = AD7170_PATTERN, >> + .pattern_mask = AD7170_PATTERN_MASK, >> + .is_ad778x = false, >> + }, >> + [ID_AD7171] = { >> + .channel = AD7780_CHANNEL(16, 24), >> + .pattern = AD7170_PATTERN, >> + .pattern_mask = AD7170_PATTERN_MASK, >> + .is_ad778x = false, >> + }, >> + [ID_AD7780] = { >> + .channel = AD7780_CHANNEL(24, 32), >> + .pattern = AD7780_PATTERN, >> + .pattern_mask = AD7780_PATTERN_MASK, >> + .is_ad778x = true, >> + }, >> + [ID_AD7781] = { >> + .channel = AD7780_CHANNEL(20, 32), >> + .pattern = AD7780_PATTERN, >> + .pattern_mask = AD7780_PATTERN_MASK, >> + .is_ad778x = true, >> + }, >> +}; >> + >> +static const struct iio_info ad7780_info = { >> + .read_raw = ad7780_read_raw, >> + .write_raw = ad7780_write_raw, >> +}; >> + >> +static int ad7780_probe(struct spi_device *spi) >> +{ >> + struct ad7780_state *st; >> + struct iio_dev *indio_dev; >> + int ret; >> + >> + indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st)); >> + if (!indio_dev) >> + return -ENOMEM; >> + >> + st = iio_priv(indio_dev); >> + st->gain = 1; >> + >> + ad_sd_init(&st->sd, indio_dev, spi, &ad7780_sigma_delta_info); >> + >> + st->chip_info = >> + &ad7780_chip_info_tbl[spi_get_device_id(spi)->driver_data]; >> + >> + spi_set_drvdata(spi, indio_dev); >> + >> + indio_dev->dev.parent = &spi->dev; >> + indio_dev->name = spi_get_device_id(spi)->name; >> + indio_dev->modes = INDIO_DIRECT_MODE; >> + indio_dev->channels = &st->chip_info->channel; >> + indio_dev->num_channels = 1; >> + indio_dev->info = &ad7780_info; >> + >> + st->powerdown_gpio = devm_gpiod_get_optional(&spi->dev, >> + "powerdown", >> + GPIOD_OUT_LOW); >> + if (IS_ERR(st->powerdown_gpio)) { >> + ret = PTR_ERR(st->powerdown_gpio); >> + dev_err(&spi->dev, "Failed to request powerdown GPIO: %d\n", >> + ret); >> + return ret; >> + } >> + >> + if (st->chip_info->is_ad778x) { >> + st->gain_gpio = devm_gpiod_get_optional(&spi->dev, >> + "gain", >> + GPIOD_OUT_HIGH); >> + if (IS_ERR(st->gain_gpio)) { >> + ret = PTR_ERR(st->gain_gpio); >> + dev_err(&spi->dev, "Failed to request gain GPIO: %d\n", >> + ret); >> + return ret; >> + } >> + >> + st->filter_gpio = devm_gpiod_get_optional(&spi->dev, >> + "filter", >> + GPIOD_OUT_HIGH); >> + if (IS_ERR(st->filter_gpio)) { >> + ret = PTR_ERR(st->filter_gpio); >> + dev_err(&spi->dev, >> + "Failed to request filter GPIO: %d\n", >> + ret); >> + return ret; >> + } >> + } >> + >> + st->reg = devm_regulator_get(&spi->dev, "avdd"); >> + if (IS_ERR(st->reg)) >> + return PTR_ERR(st->reg); >> + >> + ret = regulator_enable(st->reg); >> + if (ret) { >> + dev_err(&spi->dev, "Failed to enable specified AVdd supply\n"); >> + return ret; >> + } >> + >> + ret = ad_sd_setup_buffer_and_trigger(indio_dev); >> + if (ret) >> + goto error_disable_reg; >> + >> + ret = iio_device_register(indio_dev); >> + if (ret) >> + goto error_cleanup_buffer_and_trigger; >> + >> + return 0; >> + >> +error_cleanup_buffer_and_trigger: >> + ad_sd_cleanup_buffer_and_trigger(indio_dev); >> +error_disable_reg: >> + regulator_disable(st->reg); >> + >> + return ret; >> +} >> + >> +static int ad7780_remove(struct spi_device *spi) >> +{ >> + struct iio_dev *indio_dev = spi_get_drvdata(spi); >> + struct ad7780_state *st = iio_priv(indio_dev); >> + >> + iio_device_unregister(indio_dev); >> + ad_sd_cleanup_buffer_and_trigger(indio_dev); >> + >> + regulator_disable(st->reg); >> + >> + return 0; >> +} >> + >> +static const struct spi_device_id ad7780_id[] = { >> + {"ad7170", ID_AD7170}, >> + {"ad7171", ID_AD7171}, >> + {"ad7780", ID_AD7780}, >> + {"ad7781", ID_AD7781}, >> + {} >> +}; >> +MODULE_DEVICE_TABLE(spi, ad7780_id); >> + >> +static struct spi_driver ad7780_driver = { >> + .driver = { >> + .name = "ad7780", >> + }, >> + .probe = ad7780_probe, >> + .remove = ad7780_remove, >> + .id_table = ad7780_id, >> +}; >> +module_spi_driver(ad7780_driver); >> + >> +MODULE_AUTHOR("Michael Hennerich <michael.hennerich@analog.com>"); >> +MODULE_DESCRIPTION("Analog Devices AD7780 and similar ADCs"); >> +MODULE_LICENSE("GPL v2"); ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 4/4] staging: iio: ad7780: moving ad7780 out of staging 2019-02-14 20:48 ` Renato Lui Geh @ 2019-02-18 14:50 ` Jonathan Cameron 0 siblings, 0 replies; 15+ messages in thread From: Jonathan Cameron @ 2019-02-18 14:50 UTC (permalink / raw) To: Renato Lui Geh Cc: Jonathan Cameron, lars, Michael.Hennerich, knaack.h, pmeerw, gregkh, stefan.popa, alexandru.Ardelean, giuliano.belinassi, linux-iio, devel, linux-kernel, kernel-usp On Thu, 14 Feb 2019 18:48:15 -0200 Renato Lui Geh <renatogeh@gmail.com> wrote: > On 02/09, Jonathan Cameron wrote: > >On Tue, 5 Feb 2019 15:14:03 -0200 > >Renato Lui Geh <renatogeh@gmail.com> wrote: > > > >> Move ad7780 ADC driver out of staging and into the mainline. > >> > >> The ad7780 is a sigma-delta analog to digital converter. This driver provides > >> reading voltage values and status bits from both the ad778x and ad717x series. > >> Its interface also allows writing on the FILTER and GAIN GPIO pins on the > >> ad778x. > >> > >> Signed-off-by: Renato Lui Geh <renatogeh@gmail.com> > >> Signed-off-by: Giuliano Belinassi <giuliano.belinassi@usp.br> > >> Co-developed-by: Giuliano Belinassi <giuliano.belinassi@usp.br> > > > >This needs a device tree binding doc which should be reviewed before we move > >the driver out of staging. Make sure to cc the dt-binding maintainers and > >list. Doesn't really matter if that patch is before or after this one > >in the series but needs to be in the same series. > > Ok! I see that some Analog dt-bindings are prefixed by adi and some are > not. Should I follow any naming standard? Yeah, comes of history and the fact that we can't 'fix' a binding that is in the wild if the driver is already out of staging. Even in the staging case we won't generally do it if we know of real devices out there using the defacto binding. They should be prefixed if they aren't 'generic'. Exactly where that lines tends to be a little unclear unfortunately. > > > >There are a few more minor tidy ups that would be nice to have inline > >given you are doing a v4. Stuff like this could have been cleaned up > >after moving out of staging (nothing wrong with improving non staging > >drivers after all) but always better to do it whilst we remember! > > > >> --- > >> Changes in v3: > >> - Changes unrelated to moving the driver to main tree were resent as > >> individual patches > >> > >> drivers/iio/adc/Kconfig | 13 ++ > >> drivers/iio/adc/Makefile | 1 + > >> drivers/iio/adc/ad7780.c | 359 +++++++++++++++++++++++++++++++ > >> drivers/staging/iio/adc/Kconfig | 13 -- > >> drivers/staging/iio/adc/Makefile | 1 - > >> drivers/staging/iio/adc/ad7780.c | 359 ------------------------------- > >> 6 files changed, 373 insertions(+), 373 deletions(-) > >> create mode 100644 drivers/iio/adc/ad7780.c > >> delete mode 100644 drivers/staging/iio/adc/ad7780.c > >> > >> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig > >> index f3cc7a31bce5..2cdee166d0e9 100644 > >> --- a/drivers/iio/adc/Kconfig > >> +++ b/drivers/iio/adc/Kconfig > >> @@ -108,6 +108,19 @@ config AD7766 > >> To compile this driver as a module, choose M here: the module will be > >> called ad7766. > >> > >> +config AD7780 > >> + tristate "Analog Devices AD7780 and similar ADCs driver" > >> + depends on SPI > >> + depends on GPIOLIB || COMPILE_TEST > >> + select AD_SIGMA_DELTA > >> + help > >> + Say yes here to build support for Analog Devices AD7170, AD7171, > >> + AD7780 and AD7781 SPI analog to digital converters (ADC). > >> + If unsure, say N (but it's safe to say "Y"). > > > >I wouldn't bother with this statement, doesn't add any real info! > > > >> + > >> + To compile this driver as a module, choose M here: the > >> + module will be called ad7780. > >> + > >> config AD7791 > >> tristate "Analog Devices AD7791 ADC driver" > >> depends on SPI > >> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile > >> index ea5031348052..b48852157115 100644 > >> --- a/drivers/iio/adc/Makefile > >> +++ b/drivers/iio/adc/Makefile > >> @@ -15,6 +15,7 @@ obj-$(CONFIG_AD7606_IFACE_PARALLEL) += ad7606_par.o > >> obj-$(CONFIG_AD7606_IFACE_SPI) += ad7606_spi.o > >> obj-$(CONFIG_AD7606) += ad7606.o > >> obj-$(CONFIG_AD7766) += ad7766.o > >> +obj-$(CONFIG_AD7780) += ad7780.o > >> obj-$(CONFIG_AD7791) += ad7791.o > >> obj-$(CONFIG_AD7793) += ad7793.o > >> obj-$(CONFIG_AD7887) += ad7887.o > >> diff --git a/drivers/iio/adc/ad7780.c b/drivers/iio/adc/ad7780.c > >> new file mode 100644 > >> index 000000000000..163e3c983598 > >> --- /dev/null > >> +++ b/drivers/iio/adc/ad7780.c > >> @@ -0,0 +1,359 @@ > >> +// SPDX-License-Identifier: GPL-2.0 > >> +/* > >> + * AD7170/AD7171 and AD7780/AD7781 SPI ADC driver > >> + * > >> + * Copyright 2011 Analog Devices Inc. > > > >I think you have done more than enough to this driver to add > >an additional copyright line if you want to! > > Oh wow, that'd be awesome! Thanks! Should I just add my full name there? Yes. > > > >> + */ > >> + > >> +#include <linux/interrupt.h> > >> +#include <linux/device.h> > >> +#include <linux/kernel.h> > >> +#include <linux/slab.h> > >> +#include <linux/sysfs.h> > >> +#include <linux/spi/spi.h> > >> +#include <linux/regulator/consumer.h> > >> +#include <linux/err.h> > >> +#include <linux/sched.h> > >> +#include <linux/gpio/consumer.h> > >> +#include <linux/module.h> > >> + > >> +#include <linux/iio/iio.h> > >> +#include <linux/iio/sysfs.h> > >> +#include <linux/iio/adc/ad_sigma_delta.h> > >> + > >> +#define AD7780_RDY BIT(7) > >> +#define AD7780_FILTER BIT(6) > >> +#define AD7780_ERR BIT(5) > >> +#define AD7780_ID1 BIT(4) > >> +#define AD7780_ID0 BIT(3) > >> +#define AD7780_GAIN BIT(2) > >> +#define AD7780_PAT1 BIT(1) > >> +#define AD7780_PAT0 BIT(0) > >These two bits of pattern don't really add anything. I'd drop them in > >favour of something like > > > >#define AD7780_PATTERN_GOOD 1 > >#define AD7780_PATTERN_MASK GENMASK(1, 0) > > > >Same for ID for that matter. These aren't one bit fields, so we shouldn't > >ever present them as such (though the datasheet confusingly sort of does > >so!) > > > >> + > >> +#define AD7780_PATTERN (AD7780_PAT0) > >> +#define AD7780_PATTERN_MASK (AD7780_PAT0 | AD7780_PAT1) > >> + > >> +#define AD7170_PAT2 BIT(2) > >> + > >> +#define AD7170_PATTERN (AD7780_PAT0 | AD7170_PAT2) > >> +#define AD7170_PATTERN_MASK (AD7780_PAT0 | AD7780_PAT1 | AD7170_PAT2) > >I'd use a value for the pattern directly and > >GENMASK for the mask. > > > >> + > >> +#define AD7780_GAIN_GPIO 0 > >> +#define AD7780_FILTER_GPIO 1 > >> + > >> +#define AD7780_GAIN_MIDPOINT 64 > >> +#define AD7780_FILTER_MIDPOINT 13350 > >> + > >> +static const unsigned int ad778x_gain[2] = { 1, 128 }; > >> +static const unsigned int ad778x_odr_avail[2] = { 10000, 16700 }; > >> + > >> +struct ad7780_chip_info { > >> + struct iio_chan_spec channel; > >> + unsigned int pattern_mask; > >> + unsigned int pattern; > >> + bool is_ad778x; > >> +}; > >> + > >> +struct ad7780_state { > >> + const struct ad7780_chip_info *chip_info; > >> + struct regulator *reg; > >> + struct gpio_desc *powerdown_gpio; > >> + struct gpio_desc *gain_gpio; > >> + struct gpio_desc *filter_gpio; > >> + unsigned int gain; > >> + unsigned int odr; > >> + unsigned int int_vref_mv; > >> + > >> + struct ad_sigma_delta sd; > >> +}; > >> + > >> +enum ad7780_supported_device_ids { > >> + ID_AD7170, > >> + ID_AD7171, > >> + ID_AD7780, > >> + ID_AD7781, > >> +}; > >> + > >> +static struct ad7780_state *ad_sigma_delta_to_ad7780(struct ad_sigma_delta *sd) > >> +{ > >> + return container_of(sd, struct ad7780_state, sd); > >> +} > >> + > >> +static int ad7780_set_mode(struct ad_sigma_delta *sigma_delta, > >> + enum ad_sigma_delta_mode mode) > >> +{ > >> + struct ad7780_state *st = ad_sigma_delta_to_ad7780(sigma_delta); > >> + unsigned int val; > >> + > >> + switch (mode) { > >> + case AD_SD_MODE_SINGLE: > >> + case AD_SD_MODE_CONTINUOUS: > >> + val = 1; > >> + break; > >> + default: > >> + val = 0; > >> + break; > >> + } > >> + > >> + gpiod_set_value(st->powerdown_gpio, val); > >> + > >> + return 0; > >> +} > >> + > >> +static int ad7780_read_raw(struct iio_dev *indio_dev, > >> + struct iio_chan_spec const *chan, > >> + int *val, > >> + int *val2, > >> + long m) > >> +{ > >> + struct ad7780_state *st = iio_priv(indio_dev); > >> + int voltage_uv; > >> + > >> + switch (m) { > >> + case IIO_CHAN_INFO_RAW: > >> + return ad_sigma_delta_single_conversion(indio_dev, chan, val); > >> + case IIO_CHAN_INFO_SCALE: > >> + voltage_uv = regulator_get_voltage(st->reg); > >> + if (voltage_uv < 0) > >> + return voltage_uv; > >> + voltage_uv /= 1000; > >> + *val = voltage_uv * st->gain; > >> + *val2 = chan->scan_type.realbits - 1; > >> + st->int_vref_mv = voltage_uv; > >> + return IIO_VAL_FRACTIONAL_LOG2; > >> + case IIO_CHAN_INFO_OFFSET: > >> + *val = -(1 << (chan->scan_type.realbits - 1)); > >> + return IIO_VAL_INT; > >> + case IIO_CHAN_INFO_SAMP_FREQ: > >> + *val = st->odr; > >> + return IIO_VAL_INT; > >> + } > >> + > >> + return -EINVAL; > >> +} > >> + > >> +static int ad7780_write_raw(struct iio_dev *indio_dev, > >> + struct iio_chan_spec const *chan, > >> + int val, > >> + int val2, > >> + long m) > >> +{ > >> + struct ad7780_state *st = iio_priv(indio_dev); > >> + const struct ad7780_chip_info *chip_info = st->chip_info; > >> + unsigned long long vref; > >> + unsigned int full_scale, gain; > >> + > >> + if (!chip_info->is_ad778x) > >> + return 0; > >> + > >> + switch (m) { > >> + case IIO_CHAN_INFO_SCALE: > >> + if (val != 0) > >> + return -EINVAL; > >> + > >> + vref = st->int_vref_mv * 1000000LL; > >> + full_scale = 1 << (chip_info->channel.scan_type.realbits - 1); > >> + gain = DIV_ROUND_CLOSEST(vref, full_scale); > >> + gain = DIV_ROUND_CLOSEST(gain, val2); > >> + st->gain = gain; > >> + if (gain < AD7780_GAIN_MIDPOINT) > >> + gain = 0; > >> + else > >> + gain = 1; > >> + gpiod_set_value(st->gain_gpio, gain); > >> + break; > >> + case IIO_CHAN_INFO_SAMP_FREQ: > >> + if (1000*val + val2/1000 < AD7780_FILTER_MIDPOINT) > >> + val = 0; > >> + else > >> + val = 1; > >> + st->odr = ad778x_odr_avail[val]; > >> + gpiod_set_value(st->filter_gpio, val); > >> + break; > >We'll get a warning here due to the lack of a default handler. > >It's pointless except to suppress the warning, but best to add one. > > > >> + } > >> + > >> + return 0; > >> +} > >> + > >> +static int ad7780_postprocess_sample(struct ad_sigma_delta *sigma_delta, > >> + unsigned int raw_sample) > >> +{ > >> + struct ad7780_state *st = ad_sigma_delta_to_ad7780(sigma_delta); > >> + const struct ad7780_chip_info *chip_info = st->chip_info; > >> + > >> + if ((raw_sample & AD7780_ERR) || > >> + ((raw_sample & chip_info->pattern_mask) != chip_info->pattern)) > >> + return -EIO; > >> + > >> + if (chip_info->is_ad778x) { > >> + st->gain = ad778x_gain[raw_sample & AD7780_GAIN]; > >> + st->odr = ad778x_odr_avail[raw_sample & AD7780_FILTER]; > >> + } > >> + > >> + return 0; > >> +} > >> + > >> +static const struct ad_sigma_delta_info ad7780_sigma_delta_info = { > >> + .set_mode = ad7780_set_mode, > >> + .postprocess_sample = ad7780_postprocess_sample, > >> + .has_registers = false, > >> +}; > >> + > >> +#define AD7780_CHANNEL(bits, wordsize) \ > >> + AD_SD_CHANNEL_NO_SAMP_FREQ(1, 0, 0, bits, 32, wordsize - bits) > >> + > >> +static const struct ad7780_chip_info ad7780_chip_info_tbl[] = { > >> + [ID_AD7170] = { > >> + .channel = AD7780_CHANNEL(12, 24), > >> + .pattern = AD7170_PATTERN, > >> + .pattern_mask = AD7170_PATTERN_MASK, > >> + .is_ad778x = false, > >> + }, > >> + [ID_AD7171] = { > >> + .channel = AD7780_CHANNEL(16, 24), > >> + .pattern = AD7170_PATTERN, > >> + .pattern_mask = AD7170_PATTERN_MASK, > >> + .is_ad778x = false, > >> + }, > >> + [ID_AD7780] = { > >> + .channel = AD7780_CHANNEL(24, 32), > >> + .pattern = AD7780_PATTERN, > >> + .pattern_mask = AD7780_PATTERN_MASK, > >> + .is_ad778x = true, > >> + }, > >> + [ID_AD7781] = { > >> + .channel = AD7780_CHANNEL(20, 32), > >> + .pattern = AD7780_PATTERN, > >> + .pattern_mask = AD7780_PATTERN_MASK, > >> + .is_ad778x = true, > >> + }, > >> +}; > >> + > >> +static const struct iio_info ad7780_info = { > >> + .read_raw = ad7780_read_raw, > >> + .write_raw = ad7780_write_raw, > >> +}; > >> + > >> +static int ad7780_probe(struct spi_device *spi) > >> +{ > >> + struct ad7780_state *st; > >> + struct iio_dev *indio_dev; > >> + int ret; > >> + > >> + indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st)); > >> + if (!indio_dev) > >> + return -ENOMEM; > >> + > >> + st = iio_priv(indio_dev); > >> + st->gain = 1; > >> + > >> + ad_sd_init(&st->sd, indio_dev, spi, &ad7780_sigma_delta_info); > >> + > >> + st->chip_info = > >> + &ad7780_chip_info_tbl[spi_get_device_id(spi)->driver_data]; > >> + > >> + spi_set_drvdata(spi, indio_dev); > >> + > >> + indio_dev->dev.parent = &spi->dev; > >> + indio_dev->name = spi_get_device_id(spi)->name; > >> + indio_dev->modes = INDIO_DIRECT_MODE; > >> + indio_dev->channels = &st->chip_info->channel; > >> + indio_dev->num_channels = 1; > >> + indio_dev->info = &ad7780_info; > >> + > >> + st->powerdown_gpio = devm_gpiod_get_optional(&spi->dev, > >> + "powerdown", > >> + GPIOD_OUT_LOW); > >> + if (IS_ERR(st->powerdown_gpio)) { > >> + ret = PTR_ERR(st->powerdown_gpio); > >> + dev_err(&spi->dev, "Failed to request powerdown GPIO: %d\n", > >> + ret); > >> + return ret; > >> + } > >> + > >> + if (st->chip_info->is_ad778x) { > >> + st->gain_gpio = devm_gpiod_get_optional(&spi->dev, > >> + "gain", > >> + GPIOD_OUT_HIGH); > >> + if (IS_ERR(st->gain_gpio)) { > >> + ret = PTR_ERR(st->gain_gpio); > >> + dev_err(&spi->dev, "Failed to request gain GPIO: %d\n", > >> + ret); > >> + return ret; > >> + } > >> + > >> + st->filter_gpio = devm_gpiod_get_optional(&spi->dev, > >> + "filter", > >> + GPIOD_OUT_HIGH); > >> + if (IS_ERR(st->filter_gpio)) { > >> + ret = PTR_ERR(st->filter_gpio); > >> + dev_err(&spi->dev, > >> + "Failed to request filter GPIO: %d\n", > >> + ret); > >> + return ret; > >> + } > >> + } > >> + > >> + st->reg = devm_regulator_get(&spi->dev, "avdd"); > >> + if (IS_ERR(st->reg)) > >> + return PTR_ERR(st->reg); > >> + > >> + ret = regulator_enable(st->reg); > >> + if (ret) { > >> + dev_err(&spi->dev, "Failed to enable specified AVdd supply\n"); > >> + return ret; > >> + } > >> + > >> + ret = ad_sd_setup_buffer_and_trigger(indio_dev); > >> + if (ret) > >> + goto error_disable_reg; > >> + > >> + ret = iio_device_register(indio_dev); > >> + if (ret) > >> + goto error_cleanup_buffer_and_trigger; > >> + > >> + return 0; > >> + > >> +error_cleanup_buffer_and_trigger: > >> + ad_sd_cleanup_buffer_and_trigger(indio_dev); > >> +error_disable_reg: > >> + regulator_disable(st->reg); > >> + > >> + return ret; > >> +} > >> + > >> +static int ad7780_remove(struct spi_device *spi) > >> +{ > >> + struct iio_dev *indio_dev = spi_get_drvdata(spi); > >> + struct ad7780_state *st = iio_priv(indio_dev); > >> + > >> + iio_device_unregister(indio_dev); > >> + ad_sd_cleanup_buffer_and_trigger(indio_dev); > >> + > >> + regulator_disable(st->reg); > >> + > >> + return 0; > >> +} > >> + > >> +static const struct spi_device_id ad7780_id[] = { > >> + {"ad7170", ID_AD7170}, > >> + {"ad7171", ID_AD7171}, > >> + {"ad7780", ID_AD7780}, > >> + {"ad7781", ID_AD7781}, > >> + {} > >> +}; > >> +MODULE_DEVICE_TABLE(spi, ad7780_id); > >> + > >> +static struct spi_driver ad7780_driver = { > >> + .driver = { > >> + .name = "ad7780", > >> + }, > >> + .probe = ad7780_probe, > >> + .remove = ad7780_remove, > >> + .id_table = ad7780_id, > >> +}; > >> +module_spi_driver(ad7780_driver); > >> + > >> +MODULE_AUTHOR("Michael Hennerich <michael.hennerich@analog.com>"); > >> +MODULE_DESCRIPTION("Analog Devices AD7780 and similar ADCs"); > >> +MODULE_LICENSE("GPL v2"); ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2019-02-18 14:50 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-02-05 17:12 [PATCH v3 0/4] staging: iio: ad7780: move out of staging Renato Lui Geh 2019-02-05 17:13 ` [PATCH v3 1/4] staging: iio: ad7780: add gain & filter gpio support Renato Lui Geh 2019-02-05 19:58 ` Peter Meerwald-Stadler 2019-02-05 20:21 ` Renato Lui Geh 2019-02-06 7:27 ` Linus Torvalds 2019-02-09 16:25 ` Jonathan Cameron 2019-02-14 20:31 ` Renato Lui Geh 2019-02-18 14:48 ` Jonathan Cameron 2019-02-05 17:13 ` [PATCH v3 2/4] staging: iio: ad7780: move regulator to after GPIO init Renato Lui Geh 2019-02-09 16:26 ` Jonathan Cameron 2019-02-05 17:13 ` [PATCH v3 3/4] staging: iio: ad7780: add SPDX identifier Renato Lui Geh 2019-02-05 17:14 ` [PATCH v3 4/4] staging: iio: ad7780: moving ad7780 out of staging Renato Lui Geh 2019-02-09 16:37 ` Jonathan Cameron 2019-02-14 20:48 ` Renato Lui Geh 2019-02-18 14:50 ` Jonathan Cameron
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.