Re: [PATCH v2 1/3] iio: accel: adxl372: Fix/remove limitation for FIFO samples
On Du, 2019-09-15 at 14:27 +0100, Jonathan Cameron wrote:
>
>
>
> On Tue, 10 Sep 2019 17:43:32 +0300
> Stefan Popa wrote:
>
> >
> >
> >
> >
> > Currently, the driver sets the FIFO_SAMPLES register with the number of
> > sample sets (maximum of 170 for 3 axis data, 256 for 2-axis and 512 for
> > single axis). However, the FIFO_SAMPLES register should store the
> > number
> > of samples, regardless of how the FIFO format is configured.
> >
> > Signed-off-by: Stefan Popa
Hi Jonathan,
>
>
>
> Fixes tags? I think it's
> Fixes: f4f55ce38e5f ("iio:adxl372: Add FIFO and interrupts support")
>
> Check I got that right though.
Yes, that's right! Thank you!
-Stefan
>
>
>
>
> One trivial inline that I have tidied up whilst applying.
>
> Applied to the fixes-togreg branch of iio.git.
>
> Thanks,
>
> Jonathan
>
>
> >
> >
> >
> >
> > ---
> > Changes in v2:
> > - st->watermark needs to store the number of sample sets,
> > the total number of samples is computed in
> > adxl372_configure_fifo() func.
> >
> > drivers/iio/accel/adxl372.c | 11 ---
> > 1 file changed, 8 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/iio/accel/adxl372.c b/drivers/iio/accel/adxl372.c
> > index 055227cb..7de5e1b 100644
> > --- a/drivers/iio/accel/adxl372.c
> > +++ b/drivers/iio/accel/adxl372.c
> > @@ -474,12 +474,17 @@ static int adxl372_configure_fifo(struct
> > adxl372_state *st)
> > if (ret < 0)
> > return ret;
> >
> > - fifo_samples = st->watermark & 0xFF;
> > + /*
> > + * watermak stores the number of sets; we need to write the
> > FIFO
> watermark
>
> >
> >
> >
> >
> > + * registers with the number of samples
> > + */
> > + fifo_samples = (st->watermark * st->fifo_set_size);
> > fifo_ctl = ADXL372_FIFO_CTL_FORMAT_MODE(st->fifo_format) |
> > ADXL372_FIFO_CTL_MODE_MODE(st->fifo_mode) |
> > - ADXL372_FIFO_CTL_SAMPLES_MODE(st->watermark);
> > + ADXL372_FIFO_CTL_SAMPLES_MODE(fifo_samples);
> >
> > - ret = regmap_write(st->regmap, ADXL372_FIFO_SAMPLES,
> > fifo_samples);
> > + ret = regmap_write(st->regmap,
> > + ADXL372_FIFO_SAMPLES, fifo_samples & 0xFF);
> > if (ret < 0)
> > return ret;
> >
Re: [PATCH v5 2/2] dt-bindings: iio: frequency: Add docs for ADF4371 PLL
Hi Rob,
I'm sorry I forgot to include your Reviewed-by in this patch series.
However, I realized this mistake and added it to v6 which in the meantime
Jonathan has applied.
I will fix your remark bellow regarding clock-names in the next patch
series for this device.
Thank you,
-Stefan
On Ma, 2019-06-11 at 15:59 -0600, Rob Herring wrote:
>
>
> On Tue, Jun 04, 2019 at 04:08:17PM +0300, Stefan Popa wrote:
> >
> > Document support for Analog Devices ADF4371 SPI Wideband Synthesizer.
> >
> > Signed-off-by: Stefan Popa
> > ---
> > Changes in v2:
> > - Nothing changed.
> > Changes in v3:
> > - Nothing changed.
> > Changes in v4:
> > - Nothing changed.
> > Changes in v5:
> > - Nothing changed.
> Please add acks/reviewed-bys when posting new versions.
>
> But something else I noticed:
>
> >
> >
> > .../devicetree/bindings/iio/frequency/adf4371.yaml | 54
> > ++
> > 1 file changed, 54 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/iio/frequency/adf4371.yaml
> >
> > diff --git
> > a/Documentation/devicetree/bindings/iio/frequency/adf4371.yaml
> > b/Documentation/devicetree/bindings/iio/frequency/adf4371.yaml
> > new file mode 100644
> > index 000..d7adf074
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/frequency/adf4371.yaml
> > @@ -0,0 +1,54 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/iio/frequency/adf4371.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Analog Devices ADF4371 Wideband Synthesizer
> > +
> > +maintainers:
> > + - Popa Stefan
> > +
> > +description: |
> > + Analog Devices ADF4371 SPI Wideband Synthesizer
> > + https://www.analog.com/media/en/technical-documentation/data-sheets/
> > adf4371.pdf
> > +
> > +properties:
> > + compatible:
> > +enum:
> > + - adi,adf4371
> > +
> > + reg:
> > +maxItems: 1
> > +
> > + clocks:
> > +description:
> > + Definition of the external clock (see clock/clock-bindings.txt)
> > +maxItems: 1
> > +
> > + clock-names:
> > +description:
> > + Must be "clkin"
> This can be a schema:
>
> clock-names:
> items:
> - clkin
>
> >
> > +maxItems: 1
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - clocks
> > + - clock-names
> > +
> > +examples:
> > + - |
> > +spi0 {
> > +#address-cells = <1>;
> > +#size-cells = <0>;
> > +
> > +frequency@0 {
> > +compatible = "adi,adf4371";
> > +reg = <0>;
> > +spi-max-frequency = <100>;
> > +clocks = <&adf4371_clkin>;
> > +clock-names = "clkin";
> > +};
> > +};
> > +...
> > --
> > 2.7.4
> >
Re: [PATCH] dt-bindings: iio: accel: adxl372: switch to YAML bindings
On Sb, 2019-05-18 at 18:55 -0300, Lucas Oshiro wrote: > [External] > > > Convert the old device tree documentation to yaml format. > > Signed-off-by: Lucas Oshiro > Signed-off-by: Rodrigo Ribeiro > Co-developed-by: Rodrigo Ribeiro > --- > > Hello, > We've added Stefan Popa as maintainer of the yaml documentation of this > driver > because we found through git that he was the author of the older > documentation. Acked-by: Stefan Popa
Re: [PATCH] iio: adc: fix indentation issue, remove extra tab
On Jo, 2019-03-14 at 23:26 +, Colin King wrote: > [External] > > > From: Colin Ian King > > A return statement is indented one level too deeply; clean this > up by removing a tab. > > Signed-off-by: Colin Ian King Acked-by: Stefan Popa Thanks! > --- > drivers/iio/adc/ad7124.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/iio/adc/ad7124.c b/drivers/iio/adc/ad7124.c > index 7d5e5311d8de..659ef37d5fe8 100644 > --- a/drivers/iio/adc/ad7124.c > +++ b/drivers/iio/adc/ad7124.c > @@ -411,7 +411,7 @@ static int ad7124_init_channel_vref(struct > ad7124_state *st, > dev_err(&st->sd.spi->dev, > "Error, trying to use external voltage > reference without a %s regulator.\n", > ad7124_ref_names[refsel]); > - return PTR_ERR(st->vref[refsel]); > + return PTR_ERR(st->vref[refsel]); > } > st->channel_config[channel_number].vref_mv = > regulator_get_voltage(st->vref[refsel]); > -- > 2.20.1 >
Re: [PATCH 2/6] iio: imu: adis16480: Add support for configurable drdy indicator
On Mi, 2019-02-20 at 10:29 +, Jonathan Cameron wrote:
> [External]
>
>
> On Tue, 19 Feb 2019 19:12:14 +0200
> Stefan Popa wrote:
>
> >
> > The FNCTIO_CTRL register provides configuration control for each I/O
> > pin
> > (DIO1, DIO2, DIO3 and DIO4).
> >
> > This patch adds the option to configure each DIOx pin as data ready
> > indicator with positive or negative polarity by reading the
> > 'interrupts'
> > and 'interrupt-names' properties from the devicetree. The
> > 'interrupt-names' property is optional, if it is not specified, then
> > the
> > factory default DIO2 data ready signal is used.
> >
> > Signed-off-by: Stefan Popa
> Other than follow on from the previous patch change of the default, this
> looks fine to me.
>
> One question inline.
>
> Jonathan
Hi Jonathan,
Thank you for the review!
I will fall back on the 'wrong' default in the previous patch.
Answer inline.
-Stefan
> >
> > ---
> > drivers/iio/imu/adis16480.c | 76
> > +
> > 1 file changed, 76 insertions(+)
> >
> > diff --git a/drivers/iio/imu/adis16480.c b/drivers/iio/imu/adis16480.c
> > index d222188..38ba0c1 100644
> > --- a/drivers/iio/imu/adis16480.c
> > +++ b/drivers/iio/imu/adis16480.c
> > @@ -10,6 +10,7 @@
> > */
> >
> > #include
> > +#include
> > #include
> > #include
> > #include
> > @@ -109,6 +110,10 @@
> > #define
> > ADIS16480_FIR_COEF_D(x) ADIS16480_FIR_COEF(0x0B,
> > (x))
> >
> > /* ADIS16480_REG_FNCTIO_CTRL */
> > +#define ADIS16480_DRDY_SEL_MSK GENMASK(1, 0)
> > +#define
> > ADIS16480_DRDY_SEL(x)FIELD_PREP(ADIS16480_DRDY_SEL_MSK,
> > x)
> > +#define ADIS16480_DRDY_POL_MSK BIT(2)
> > +#define
> > ADIS16480_DRDY_POL(x)FIELD_PREP(ADIS16480_DRDY_POL_MSK,
> > x)
> > #define ADIS16480_DRDY_EN_MSKBIT(3)
> > #define ADIS16480_DRDY_EN(x) FIELD_PREP(ADIS16480_DRDY_EN_MSK,
> > x)
> >
> > @@ -121,12 +126,26 @@ struct adis16480_chip_info {
> > unsigned int accel_max_scale;
> > };
> >
> > +enum adis16480_int_pin {
> > + ADIS16480_PIN_DIO1,
> > + ADIS16480_PIN_DIO2,
> > + ADIS16480_PIN_DIO3,
> > + ADIS16480_PIN_DIO4
> > +};
> > +
> > struct adis16480 {
> > const struct adis16480_chip_info *chip_info;
> >
> > struct adis adis;
> > };
> >
> > +static const char * const adis16480_int_pin_names[4] = {
> > + [ADIS16480_PIN_DIO1] = "DIO1",
> > + [ADIS16480_PIN_DIO2] = "DIO2",
> > + [ADIS16480_PIN_DIO3] = "DIO3",
> > + [ADIS16480_PIN_DIO4] = "DIO4",
> > +};
> > +
> > #ifdef CONFIG_DEBUG_FS
> >
> > static ssize_t adis16480_show_firmware_revision(struct file *file,
> > @@ -840,6 +859,59 @@ static const struct adis_data adis16480_data = {
> > .enable_irq = adis16480_enable_irq,
> > };
> >
> > +static int adis16480_config_irq_pin(struct device_node *of_node,
> > + struct adis16480 *st)
> > +{
> > + struct irq_data *desc;
> > + enum adis16480_int_pin pin;
> > + unsigned int irq_type;
> > + uint16_t val;
> > + int i, irq = 0;
> > +
> > + desc = irq_get_irq_data(st->adis.spi->irq);
> > + if (!desc) {
> > + dev_err(&st->adis.spi->dev, "Could not find IRQ %d\n",
> > irq);
> > + return -EINVAL;
> > + }
> > +
> > + /* Disable data ready */
> > + val = ADIS16480_DRDY_EN(0);
> Does it default to on after reset?
> That's a little unusual and nasty. If not, why are you disabling here?
Yes, the default after reset is on.
> >
> > +
> > + /*
> > + * Get the interrupt from the devicetre by reading the
> > + * interrupt-names property. If it is not specified, use
> > + * the default interrupt on DIO2 pin.
> > + */
> > + pin = ADIS16480_PIN_DIO2;
> > + for (i = 0; i < ARRAY_SIZE(adis16480_int_pin_names); i++) {
> > + irq = of_irq_get_byname(of_node,
> > adis16480_int_pin_names[i]);
> > + if (irq > 0) {
> > + pin = i;
> > + break;
> > + }
> > + }
> > +
> > + val |= ADIS16480_DRDY_SEL(pin);
> > +
> > + /*
> > + * Get the interrupt line behaviour. The data ready polarity can
> > be
> > + * configured as positive or negative, corresponding to
> > + * IRQF_TRIGGER_RISING or IRQF_TRIGGER_FALLING respectively.
> > + */
> > + irq_type = irqd_get_trigger_type(desc);
> > + if (irq_type == IRQF_TRIGGER_RISING) { /* Default */
> > + val |= ADIS16480_DRDY_POL(1);
> > + } else if (irq_type == IRQF_TRIGGER_FALLING) {
> > + val |= ADIS16480_DRDY_POL(0);
> > + } else {
> > + dev_err(&st->adis.spi->dev,
> > + "Invalid interrupt type 0x%x specified\n",
> > irq_type);
> > + return -EINVAL;
> > + }
> > + /* Write the data ready configuration to the FNCTIO_CTRL register
> > */
> > + return adis_write_reg_16(&st
Re: [PATCH] staging:iio:ad7152: Rename misspelled RESEVERD -> RESERVED
On Mi, 2019-02-13 at 22:25 -0200, Rodrigo Ribeiro wrote: > [External] > > > Em ter, 29 de jan de 2019 às 07:10, Alexandru Ardelean l.com> escreveu: > > > > On Sat, Jan 26, 2019 at 8:13 PM Jonathan Cameron > wrote: > > > > > > On Fri, 25 Jan 2019 22:14:32 -0200 > > > Rodrigo Ribeiro wrote: > > > > > > > Em sex, 25 de jan de 2019 às 21:46, Rodrigo Ribeiro > > > > escreveu: > > > > > > > > > > Em sex, 25 de jan de 2019 às 06:20, Alexandru Ardelean > > > > > escreveu: > > > > > > > > > > > > On Thu, Jan 24, 2019 at 9:35 PM Rodrigo Ribeiro ail.com> wrote: > > > > > > > > > > > > > > Remove the checkpatch.pl check: > > > > > > > > > > > > > > CHECK: 'RESEVERD' may be misspelled - perhaps 'RESERVED'? > > > > > > > > > > > > Hey, > > > > > > > > > > Hi, > > > > > > > > > > Thanks for answering. > > > > > > > > > > > A bit curios about this one. > > > > > > Are you using this chip/driver ? > > > > > > > > > > No, I am just a student at USP (University of São Paulo) starting > in Linux > > > > > Kernel and a new member of the USP Linux Kernel group that has > contributed > > > > > for a few months. > > > > > > > > > > > Thing is: the part is nearing EOL, and it could be an idea to > be > > > > > > marked for removal (since it's still in staging). > > > > > > But if there are users for this driver, it could be left around > for a while. > > > > > > > > > > This is my first patch in Linux Kernel, but if the driver will be > removed, I > > > > > can send a patch for another driver. Is there any driver that I > can > > > > > fix a style warning? > > > > > > > > Maybe, one checkstyle patch is enough, right? Which drivers can I > truly > > > > contribute to? > > > > > > How about the ad7150? That one is still listed as production. > > > What do you think Alex, you probably have better visibility on the > > > status of these parts and their drivers than I do! > > > > > > (note I haven't even opened that one for a few years so no idea > > > what state the driver is in!) > > > > > > > ad7150 is a good alternative. > > At a first glance over the driver it looks like it could do with some > > polish/conversions to newer IIO constructs (like IIO triggers, maybe > > some newer event handling mechanisms?). > > I'll sync with Stefan [Popa] to see about this stuff at a later point > in time. > > > > I'd also add here the adxl345 driver. > > This is mostly informational for anyone who'd find this interesting. > > There are 2 drivers for this chip, one in IIO > > [drivers/iio/accel/adxl345] and another one in > > "drivers/misc/adxl34x.c" as part of the input sub-system. > > What would be interesting here is to finalize the IIO driver [ I think > > some features are lacking behind the input driver], and make the input > > driver a consumer of the IIO driver. > > > > From my side, both these variants are fine to take on. > > The ad7150 is a good idea as a starter project, and the adxl one is > > more of a long-term medium-level project. > > > > Thanks > > Alex > > > > Hi Alex, thanks for suggestions. > > I read the IIO trigger documentation > (https://www.kernel.org/doc/html/v4.12/driver-api/iio/triggers.html) and > ask one question: What is the difference between events and triggers? > They are sounds me same things. > > I am trying to understand how to implement a IIO trigger by reading > the IIO Simple Dummy code but this driver does not implements IIO > triggers > but only IIO events. Is there a didactic example like IIO Simple Dummy > that implements IIO triggers? > > Thanks > Rodrigo > Hi Rodrigo, From what I know, IIO Events are not used for passing readings from devices to userspace, but rather out of bounds information such as crossing of voltage thresholds, proximity detection, motion detection, etc. Triggers are typically used to determine when valid data can be read from a device which is then stored in the buffer. After a quick look over the AD7150, I think that using IIO events, might be the correct approach, since it is a proximity sensing device. -Stefan > > > Jonathan > > > > > > > > > > > > Thanks > > > > > > > > > > > > > > > Em sex, 25 de jan de 2019 às 06:20, Alexandru Ardelean > > > > > escreveu: > > > > > > > > > > > > On Thu, Jan 24, 2019 at 9:35 PM Rodrigo Ribeiro ail.com> wrote: > > > > > > > > > > > > > > Remove the checkpatch.pl check: > > > > > > > > > > > > > > CHECK: 'RESEVERD' may be misspelled - perhaps 'RESERVED'? > > > > > > > > > > > > Hey, > > > > > > > > > > > > A bit curios about this one. > > > > > > Are you using this chip/driver ? > > > > > > > > > > > > Thing is: the part is nearing EOL, and it could be an idea to > be > > > > > > marked for removal (since it's still in staging). > > > > > > But if there are users for this driver, it could be left around > for a while. > > > > > > > > > > > > Thanks > > > > > > Alex > > > > > > > > > > > > > > > > > > > > Signed-off-by: Rodrigo Ribeiro > > > > > > > Signed-off-by: Rafael Tsuha > > > > > > > --- > > > > > > > This macr
Re: [PATCH v2 1/2] staging: iio: ad7780: add gain & filter gpio support
On Vi, 2019-02-01 at 12:55 -0200, Renato Lui Geh wrote:
> On 01/30, Popa, Stefan Serban wrote:
> >
> > On Du, 2019-01-27 at 18:30 -0200, 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.
> > >
> > > Signed-off-by: Renato Lui Geh
> > > Signed-off-by: Giuliano Belinassi
> > > Co-developed-by: Giuliano Belinassi
> > > ---
> > > Changes in v2:
> > > - Filter reading changed to mHz
> > > - Storing filter, gain and voltage to chip_infoHi,
> Hi Stefan,
>
> Thanks for the review. Comments inline.
>
> Renato
> >
> >
> > Comments inline.
> >
> > Regards,
> > Stefan
> >
> > >
> > >
> > > drivers/staging/iio/adc/ad7780.c | 103 ++-
> > > --
> > > include/linux/iio/adc/ad_sigma_delta.h | 5 ++
> > > 2 files changed, 99 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/drivers/staging/iio/adc/ad7780.c
> > > b/drivers/staging/iio/adc/ad7780.c
> > > index c4a85789c2db..82394e31b168 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_filter[2] = { 1, 16700 };
> > I would name this array ad778x_odr_avail or something like that.
> Sure
> >
> > We should also consider adding the option to read the available
> > sampling frequencies from user space, but let's leave this for another
> > commit.
> Do you mean returning 10 and 16.7 Hz?
>
> Should this be done before sending the driver to the main tree, or is it
> ok to do something like that after it has left staging?
I think, we should leave it for a future commit.
> >
> >
> > >
> > > +
> > > struct ad7780_chip_info {
> > > struct iio_chan_specchannel;
> > > unsigned intpattern_mask;
> > > @@ -50,7 +59,11 @@ struct ad7780_state {
> > > const struct ad7780_chip_info *chip_info;
> > > struct regulator*reg;
> > > struct gpio_desc*powerdown_gpio;
> > > - unsigned intgain;
> > > + struct gpio_desc*gain_gpio;
> > > + struct gpio_desc*filter_gpio;
> > > + unsigned intgain;
> > > + unsigned intfilter;
> > Also, this could be renamed as odr.
> >
> > >
> > > + unsigned intint_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->filter;
> > > + 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
Re: [PATCH v2 1/2] staging: iio: ad7780: add gain & filter gpio support
On Du, 2019-01-27 at 18:30 -0200, 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.
>
> Signed-off-by: Renato Lui Geh
> Signed-off-by: Giuliano Belinassi
> Co-developed-by: Giuliano Belinassi
> ---
> Changes in v2:
> - Filter reading changed to mHz
> - Storing filter, gain and voltage to chip_info
Hi,
Comments inline.
Regards,
Stefan
>
> drivers/staging/iio/adc/ad7780.c | 103 ++---
> include/linux/iio/adc/ad_sigma_delta.h | 5 ++
> 2 files changed, 99 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/staging/iio/adc/ad7780.c
> b/drivers/staging/iio/adc/ad7780.c
> index c4a85789c2db..82394e31b168 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_filter[2] = { 1, 16700 };
I would name this array ad778x_odr_avail or something like that. We should
also consider adding the option to read the available sampling frequencies
from user space, but let's leave this for another commit.
> +
> struct ad7780_chip_info {
> struct iio_chan_specchannel;
> unsigned intpattern_mask;
> @@ -50,7 +59,11 @@ struct ad7780_state {
> const struct ad7780_chip_info *chip_info;
> struct regulator*reg;
> struct gpio_desc*powerdown_gpio;
> - unsigned intgain;
> + struct gpio_desc*gain_gpio;
> + struct gpio_desc*filter_gpio;
> + unsigned intgain;
> + unsigned intfilter;
Also, this could be renamed as odr.
> + unsigned intint_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->filter;
> + 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;
> + int vref, gain;
> + unsigned int full_scale;
> +
> + if (!chip_info->is_ad778x)
> + return 0;
> +
> + switch (m) {
> + case IIO_CHAN_INFO_SCALE:
> + if (val != 0)
> + return -EINVAL;
> +
> + vref = st->int_vref_mv * 100LL;
From the datasheet, the VREF is ±5 V, therefore your vref variable will
overflow. You probably need unsigned long long.
> + full_scale = 1 << (chip_info->channel.scane_type.realbis
> - 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->filter = ad778x_filter[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_
Re: [PATCH v2 1/2] iio: adc: Add AD7768-1 ADC basic support
On Sb, 2019-01-19 at 17:55 +, Jonathan Cameron wrote: > Use caution when clicking links or attachments as this email originated > outside of Analog Devices. > > > On Tue, 15 Jan 2019 14:33:11 +0200 > Stefan Popa wrote: > > > > > The ad7768-1 is a single channel, precision 24-bit analog to digital > > converter (ADC). > > > > This basic patch configures the device in fast mode, with 32 kSPS and > > leaves the default sinc5 filter. > > > > Two data conversion modes are made available. When data is retrieved by > > using the read_raw attribute, one shot single conversion mode is set. > > The continuous conversion mode is enabled when the triggered buffer > > mechanism is used. To assure correct data retrieval, the driver waits > > for the interrupt triggered by the low to high transition of the DRDY > > pin. > > > > Datasheets: > > Link: https://www.analog.com/media/en/technical-documentation/data-shee > > ts/ad7768-1.pdf > > > > Signed-off-by: Stefan Popa > Hi Stefan, > > Driver is fine, but the licensing needs clarification. > "GPL" in module license means GPL v2 or later, but the SPDX is GPL v2 > only. > > I had a question out to Michael for some of his older drivers on > exactly this. We need to fix this one and tidy those up to > be clear one way or the other. > If you just let me know which one is right, I'll fix it up rather than > you having > to send a v3. > > Thanks, > > Jonathan > Hi Jonathan, Thank you for your review. Michael informed me that our drivers are typically GPL v2 only. I will check the other drivers that we've sent upstream and provide a patch to fix the wrong licenses. -Stefan > > > > --- > > Changes in v2: > > - Added values to all the elements of ad7768_pwrmode enum. > > - Removed the ad7768_ids enum, as the driver supports only one > > device. > > - Added a new data union which is part of the ad7768_state > > struct. This > > union, now includes a d8 field. > > - Used spi_write_then_read() in ad7768_spi_reg_read(). > > - Called spi_read() instead of spi_sync_transfer() in > > ad7768_trigger_handler(). > > - Used the devm_request_irq() instead of > > devm_request_threaded_irq(); called > > iio_trigger_poll() instead of iio_trigger_poll_chained(). > > > > MAINTAINERS| 7 + > > drivers/iio/adc/Kconfig| 13 ++ > > drivers/iio/adc/Makefile | 1 + > > drivers/iio/adc/ad7768-1.c | 459 > > + > > 4 files changed, 480 insertions(+) > > create mode 100644 drivers/iio/adc/ad7768-1.c > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index d039f66..3ba3811 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -862,6 +862,13 @@ S: Supported > > F: drivers/iio/adc/ad7606.c > > F: Documentation/devicetree/bindings/iio/adc/ad7606.txt > > > > +ANALOG DEVICES INC AD7768-1 DRIVER > > +M: Stefan Popa > > +L: linux-...@vger.kernel.org > > +W: http://ez.analog.com/community/linux-device-drivers > > +S: Supported > > +F: drivers/iio/adc/ad7768-1.c > > + > > ANALOG DEVICES INC AD9389B DRIVER > > M: Hans Verkuil > > L: linux-me...@vger.kernel.org > > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig > > index f3cc7a3..6c19dfe 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 AD7768_1 > > + tristate "Analog Devices AD7768-1 ADC driver" > > + depends on SPI > > + select IIO_BUFFER > > + select IIO_TRIGGER > > + select IIO_TRIGGERED_BUFFER > > + help > > + Say yes here to build support for Analog Devices AD7768-1 SPI > > + simultaneously sampling sigma-delta analog to digital converter > > (ADC). > > + > > + To compile this driver as a module, choose M here: the module > > will be > > + called ad7768-1. > > + > > config AD7791 > > tristate "Analog Devices AD7791 ADC driver" > > depends on SPI > > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile > > index ea50313..9d50f7b 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_AD7768_1) += ad7768-1.o > > obj-$(CONFIG_AD7791) += ad7791.o > > obj-$(CONFIG_AD7793) += ad7793.o > > obj-$(CONFIG_AD7887) += ad7887.o > > diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c > > new file mode 100644 > > index 000..fdcb966 > > --- /dev/null > > +++ b/drivers/iio/adc/ad7768-1.c > > @@ -0,0 +1,459 @@ > > +// SPDX-License-Identifier: GPL-2.0 > This is GPl v2 only. The MODULE_LICENSE is GPL v2 or later. > > > > > +/* > > + * Analog Devices AD7768-1
Re: [PATCH 05/11] staging: iio: adc: ad7606: Add support for threaded irq
On Du, 2018-12-16 at 13:49 +, Jonathan Cameron wrote:
> On Thu, 13 Dec 2018 14:46:17 +0200
> Stefan Popa wrote:
>
> >
> > This patch replaces the use of a polling ring buffer with a threaded
> > interrupt.
> >
> > Enabling the buffer sets the CONVST signal to high. When the rising
> > edge
> > of the CONVST is applied, BUSY signal goes logic high and transitions
> > low
> > at the end of the entire conversion process. The falling edge of the
> > BUSY
> > signal triggers the interrupt.
> >
> > ad7606_trigger_handler() is used as bottom half of the poll function.
> > It reads data from the device and stores it in the internal buffer.
> >
> > Signed-off-by: Stefan Popa
> I'd like a little more info as comments in this one. See below.
> Which is another way of saying I have no idea what is going on :)
>
> Thanks,
>
> Jonathan.
>
Hi Jonathan,
Thank you for the review. It turns out that there is no reason to trigger a
conversion before disabling the buffer. I will remove it in v2.
Thank you!
-Stefan
> >
> > ---
> > drivers/staging/iio/adc/ad7606.c | 116 +
> > --
> > drivers/staging/iio/adc/ad7606.h | 6 +-
> > 2 files changed, 89 insertions(+), 33 deletions(-)
> >
> > diff --git a/drivers/staging/iio/adc/ad7606.c
> > b/drivers/staging/iio/adc/ad7606.c
> > index 7191d51..13aeeec 100644
> > --- a/drivers/staging/iio/adc/ad7606.c
> > +++ b/drivers/staging/iio/adc/ad7606.c
> > @@ -21,6 +21,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> >
> > #include "ad7606.h"
> > @@ -81,36 +82,24 @@ static int ad7606_read_samples(struct ad7606_state
> > *st)
> > static irqreturn_t ad7606_trigger_handler(int irq, void *p)
> > {
> > struct iio_poll_func *pf = p;
> > - struct ad7606_state *st = iio_priv(pf->indio_dev);
> > -
> > - gpiod_set_value(st->gpio_convst, 1);
> > -
> > - return IRQ_HANDLED;
> > -}
> > -
> > -/**
> > - * ad7606_poll_bh_to_ring() bh of trigger launched polling to ring
> > buffer
> > - * @work_s:the work struct through which this was scheduled
> > - *
> > - * Currently there is no option in this driver to disable the saving
> > of
> > - * timestamps within the ring.
> > - * I think the one copy of this at a time was to avoid problems if the
> > - * trigger was set far too high and the reads then locked up the
> > computer.
> > - **/
> > -static void ad7606_poll_bh_to_ring(struct work_struct *work_s)
> > -{
> > - struct ad7606_state *st = container_of(work_s, struct
> > ad7606_state,
> > - poll_work);
> > - struct iio_dev *indio_dev = iio_priv_to_dev(st);
> > + struct iio_dev *indio_dev = pf->indio_dev;
> > + struct ad7606_state *st = iio_priv(indio_dev);
> > int ret;
> >
> > + mutex_lock(&st->lock);
> > +
> > ret = ad7606_read_samples(st);
> > if (ret == 0)
> > iio_push_to_buffers_with_timestamp(indio_dev, st-
> > >data,
> > iio_get_time_ns(ind
> > io_dev));
> >
> > - gpiod_set_value(st->gpio_convst, 0);
> > iio_trigger_notify_done(indio_dev->trig);
> > + /* The rising edge of the CONVST signal starts a new
> > conversion. */
> > + gpiod_set_value(st->gpio_convst, 1);
> > +
> > + mutex_unlock(&st->lock);
> > +
> > + return IRQ_HANDLED;
> > }
> >
> > static int ad7606_scan_direct(struct iio_dev *indio_dev, unsigned int
> > ch)
> > @@ -378,8 +367,11 @@ static int ad7606_request_gpios(struct
> > ad7606_state *st)
> > return PTR_ERR_OR_ZERO(st->gpio_os);
> > }
> >
> > -/**
> > - * Interrupt handler
> > +/*
> > + * The BUSY signal indicates when conversions are in progress, so when
> > a rising
> > + * edge of CONVST is applied, BUSY goes logic high and transitions low
> > at the
> > + * end of the entire conversion process. The falling edge of the BUSY
> > signal
> > + * triggers this interrupt.
> > */
> > static irqreturn_t ad7606_interrupt(int irq, void *dev_id)
> > {
> > @@ -387,7 +379,8 @@ static irqreturn_t ad7606_interrupt(int irq, void
> > *dev_id)
> > struct ad7606_state *st = iio_priv(indio_dev);
> >
> > if (iio_buffer_enabled(indio_dev)) {
> > - schedule_work(&st->poll_work);
> > + gpiod_set_value(st->gpio_convst, 0);
> > + iio_trigger_poll_chained(st->trig);
> > } else {
> > complete(&st->completion);
> > }
> > @@ -395,26 +388,74 @@ static irqreturn_t ad7606_interrupt(int irq, void
> > *dev_id)
> > return IRQ_HANDLED;
> > };
> >
> > +static int ad7606_validate_trigger(struct iio_dev *indio_dev,
> > + struct iio_trigger *trig)
> > +{
> > + struct ad7606_state *st = iio_priv(indio_dev);
> > +
> > + if (st->trig != trig)
> > + return -EINVAL;
> > +
> > + return 0;
> > +}
> > +
> > +static int ad7606_buffer_postenable(struct iio_dev *indio_dev)
> > +{
> > + struct ad7606_state *st = iio_priv(indio_dev);
> > +
> > + iio_trigger
Re: [PATCH 1/2] staging: iio: ad7780: Add gain & filter gpio support
On Mi, 2018-11-28 at 16:15 -0200, Giuliano Belinassi 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: Giuliano Belinassi
> ---
> Changes in v2:
> - Now this patch is part of the patchset that aims to remove ad7780
> out of staging. https://marc.info/?l=linux-iio&m=154282349808890&w=2
> - Also, now it reads voltage and filter values from the userspace
> instead of gpio pin states.
Hello,
Please see bellow.
>
> drivers/staging/iio/adc/ad7780.c | 78 --
> include/linux/iio/adc/ad_sigma_delta.h | 5 ++
> 2 files changed, 79 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/iio/adc/ad7780.c
> b/drivers/staging/iio/adc/ad7780.c
> index c4a85789c2db..05979a79fda3 100644
> --- a/drivers/staging/iio/adc/ad7780.c
> +++ b/drivers/staging/iio/adc/ad7780.c
> @@ -39,6 +39,12 @@
> #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
> +
> struct ad7780_chip_info {
> struct iio_chan_specchannel;
> unsigned intpattern_mask;
> @@ -50,6 +56,8 @@ 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 intgain;
>
> struct ad_sigma_delta sd;
> @@ -115,18 +123,65 @@ static int ad7780_read_raw(struct iio_dev
> *indio_dev,
> 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;
> + int uvref, gain;
> + unsigned int full_scale;
> +
> + if (!chip_info->is_ad778x)
> + return 0;
> +
> + switch (m) {
> + case IIO_CHAN_INFO_SCALE:
> + if (val != 0)
> + return -EINVAL;
> +
> + uvref = regulator_get_voltage(st->reg);
regulator_get_voltage() has already been called in the probe function and
the result is stored in st->int_vref_mv.
My suggestion would be to use a local vref variable declared as unsigned
int. It is my fault that I haven't explained correctly in the previous
email, but you need to multiply vref_mv with 100LL in order to get the
right precision: vref = st->int_vref_mv * 100LL. Afterwards you will be
able to perform the divisions.
> +
> + if (uvref < 0)
> + return uvref;
> +
> + full_scale = 1 << (chip_info->channel.scan_type.realbits
> - 1);
> + gain = DIV_ROUND_CLOSEST(uvref, full_scale);
> + gain = DIV_ROUND_CLOSEST(gain, val2);
> +
> + gpiod_set_value(st->gain_gpio, gain <
> AD7780_GAIN_MIDPOINT ? 0 : 1);
Once the gain is set, you can store it in st->gain variable.
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + if (val2 != 0)
> + return -EINVAL;
> +
> + gpiod_set_value(st->filter_gpio, val <
> AD7780_FILTER_MIDPOINT ? 0 : 1);
This is probably fine, although I am not a big fan of the ternary operator.
A simple if else statement would do. However, I don't feel strongly about
it, so feel free to disagree.
> + 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;
> + int val;
>
> if ((raw_sample & AD7780_ERR) ||
> ((raw_sample & chip_info->pattern_mask) != chip_info-
> >pattern))
> return -EIO;
>
> if (chip_info->is_ad778x) {
> - if (raw_sample & AD7780_GAIN)
> + val = raw_sample & AD7780_GAIN;
> +
> + if (val != gpiod_get_value(st->gain_gpio))
> + return -EIO;
It is not obvious to me what is the point of this check. Maybe you can add
a comment?
> +
> + if (val)
> st->gain = 1;
> else
> st->gain = 128;
Do we still need this? I am not convinced.
> @@ -141,18 +196,20 @@ static const struct ad_sigma_delta_info
> ad7780_sigma_delta_info = {
> .has_registers = false,
> };
>
> -#define AD7780_CHANNEL(bits, wordsize) \
> +#define AD7170_CHANNEL(bits, word
Re: [PATCH] staging: iio: ad7780: Add gain & filter gpio support
On Lu, 2018-11-26 at 17:24 -0200, Giuliano Belinassi wrote: Hi, please see bellow > Hi, thank you for the review > > > > > On Thu, 22 Nov 2018 11:01:00 + > > "Popa, Stefan Serban" wrote: > > > > > > I think that instead of setting the gain directly, we should use > > > the IIO_CHAN_INFO_SCALE attribute. At page 12 of the ad7780 datasheet > > > there > > > is a formula from which the output code can be calculated: > > > Code = 2^(N − 1) > > > × [(AIN × Gain /VREF) + 1]. So, by setting the scale from user space, > > > the > > > driver can calculate the correct gain by using the formula above. > > > Also, it > > > would be useful to introduce scale available. > > > Furthermore, there is a new > > > ad7124 adc driver which does this exact thing. Take a look here: http > > > s://gi > > > thub.com/analogdevicesinc/linux/blob/master/drivers/iio/adc/ad7124.c# > > > L337. > We have some questions about the code you provided to us: > 1-) What is exactly the inputs for the write_raw function? In your write_raw() function you need to add IIO_CHAN_INFO_SCALE case. Setting the scale from user space looks something like this: root:/sys/bus/iio/devices/iio:device0> echo 0.000298 > in_voltage_scale . Furthermore, in your write_raw() function, val=0 and val2=298. Knowing that full_scale_voltage = Vref / (gain * scale), we can calculate the gain = Vref / (full_scale_voltage * scale). We only support two gains (1 and 128), so we need to determine which one fits better with the desired scale. Finally, all we have left to do is to set the gain. > 2-) Could you give more details about the math around lines 346-348? > Is it correct to assume that the multiplication at line 346 won't > overflow? (vref is an uint) It is correct that Vref is in microvolts, so for example, Vref of 2.5V = 25uV. It won't overflow since we use the Vref as nominator, while full_scale_voltage and scale are the denominators. > > And regarding our code: > 1-) The val in our write_raw function should be, in case of GAIN, a > number that best approximate the actual gain value of 1 or 128? For > instance, if the user inputs 126, we should default to 128? We should not allow the the user to input the gain, he needs to input the scale (see the mail from Jonathan and the above explanation). However, if the calculated gain is one that is not supported, such as 126, we will set the closest matching value, in this case 128. > 2-) In the case of FILTER, is it the same? Is the user sending the > value in mHz (milihertz)? Yes, it is the same with the FILTER. You need to add a IIO_CHAN_INFO_SAMP_FREQ case in your write_raw() function. From user space, the input value should be in Hz, something like this: root:/sys/bus/iio/devices/iio:device0> echo 10 > in_voltage_sampling_frequency > > Thank you >
Re: [PATCH] staging: iio: ad7780: Add gain & filter gpio support
On Mi, 2018-11-21 at 16:04 -0200, Giuliano Belinassi wrote:
> Previously, the AD7780 driver only supported gpio for the 'powerdown'
> pin. This commit adds suppport for the 'gain' and 'filter' pin.
Hey,
Comments inline.
>
> Signed-off-by: Giuliano Belinassi
> ---
> drivers/staging/iio/adc/ad7780.c | 61 --
> include/linux/iio/adc/ad_sigma_delta.h | 5 +++
> 2 files changed, 62 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/iio/adc/ad7780.c
> b/drivers/staging/iio/adc/ad7780.c
> index c4a85789c2db..69794f06dbcd 100644
> --- a/drivers/staging/iio/adc/ad7780.c
> +++ b/drivers/staging/iio/adc/ad7780.c
> @@ -39,6 +39,9 @@
> #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
> +
> struct ad7780_chip_info {
> struct iio_chan_specchannel;
> unsigned intpattern_mask;
> @@ -50,6 +53,8 @@ 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 intgain;
>
> struct ad_sigma_delta sd;
> @@ -115,18 +120,51 @@ static int ad7780_read_raw(struct iio_dev
> *indio_dev,
> 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);
> +
> + if (m != IIO_CHAN_INFO_RAW)
> + return -EINVAL;
> +
> + if (st->chip_info->is_ad778x) {
> + switch(val) {
> + case AD7780_GAIN_GPIO:
I think that instead of setting the gain directly, we should use
the IIO_CHAN_INFO_SCALE attribute. At page 12 of the ad7780 datasheet there
is a formula from which the output code can be calculated:
Code = 2^(N − 1)
× [(AIN × Gain /VREF) + 1]. So, by setting the scale from user space, the
driver can calculate the correct gain by using the formula above. Also, it
would be useful to introduce scale available.
Furthermore, there is a new
ad7124 adc driver which does this exact thing. Take a look here: https://gi
thub.com/analogdevicesinc/linux/blob/master/drivers/iio/adc/ad7124.c#L337.
> + gpiod_set_value(st->gain_gpio, val2);
> + break;
> + case AD7780_FILTER_GPIO:
The attribute that should be used to configure the filter gpio is
IIO_CHAN_INFO_SAMP_FREQ. So, we should have 10 Hz and 16.7 Hz available
sampling frequencies. If from user space the 10 Hz sampling freq is
requested, then we set the FILTER pin high, while for 16.7 Hz the FILTER
pin will be low.
> + gpiod_set_value(st->filter_gpio, val2);
> + break;
> + default:
> + return -EINVAL;
> + }
> + }
> +
> + 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;
> + int val;
>
> if ((raw_sample & AD7780_ERR) ||
> ((raw_sample & chip_info->pattern_mask) != chip_info-
> >pattern))
> return -EIO;
>
> if (chip_info->is_ad778x) {
> - if (raw_sample & AD7780_GAIN)
> + val = raw_sample & AD7780_GAIN;
> +
> + if (val != gpiod_get_value(st->gain_gpio))
> + return -EIO;
> +
> + if (val)
> st->gain = 1;
> else
> st->gain = 128;
> @@ -141,18 +179,20 @@ static const struct ad_sigma_delta_info
> ad7780_sigma_delta_info = {
> .has_registers = false,
> };
>
> -#define AD7780_CHANNEL(bits, wordsize) \
> +#define AD7170_CHANNEL(bits, wordsize) \
> AD_SD_CHANNEL_NO_SAMP_FREQ(1, 0, 0, bits, 32, wordsize - bits)
> +#define AD7780_CHANNEL(bits, wordsize) \
> + AD_SD_CHANNEL_GAIN_FILTER(1, 0, 0, bits, 32, wordsize - bits)
>
> static const struct ad7780_chip_info ad7780_chip_info_tbl[] = {
> [ID_AD7170] = {
> - .channel = AD7780_CHANNEL(12, 24),
> + .channel = AD7170_CHANNEL(12, 24),
> .pattern = AD7170_PATTERN,
> .pattern_mask = AD7170_PATTERN_MASK,
> .is_ad778x = false,
> },
> [ID_AD7171] = {
> - .channel = AD7780_CHANNEL(16, 24),
> + .channel = AD7170_CHANNEL(16, 24),
> .pattern = AD7170_PATTERN,
> .pattern_mask = AD7170_PATTERN_MASK,
>
Re: [PATCH v4 4/4] dt-bindings: iio: adc: Add docs for ad7124
On Du, 2018-11-11 at 12:19 +, Jonathan Cameron wrote:
>
> On Fri, 9 Nov 2018 17:43:00 +0200
> Stefan Popa wrote:
>
> >
> >
> > Add support for Analog Devices AD7124 4-channels and 8-channels ADC.
> >
> > Signed-off-by: Stefan Popa
> Your example still includes the other things that I think you have now
> dropped.
> gain and odr.
>
You are right! I am sorry about this.
>
> >
> >
> > ---
> > Changes in v2:
> > - Nothing changed.
> > Changes in v3:
> > - Removed the "adi,channels" property.
> > - Used the "reg" property to get the channel number and "adi,diff-
> > channels"
> > for the differential pins. The "adi,channel-number" property was
> > removed.
> > - adi,bipolar is of boolean type.
> > Changes in v4:
> > - Used the bipolar and diff-channels properties defined in the new
> > adc.txt doc.
> >
> > .../devicetree/bindings/iio/adc/adi,ad7124.txt | 81
> > ++
> > MAINTAINERS| 1 +
> > 2 files changed, 82 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/iio/adc/adi,ad7124.txt
> >
> > diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7124.txt
> > b/Documentation/devicetree/bindings/iio/adc/adi,ad7124.txt
> > new file mode 100644
> > index 000..fa0c43b
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7124.txt
> > @@ -0,0 +1,81 @@
> > +Analog Devices AD7124 ADC device driver
> > +
> > +Required properties for the AD7124:
> > + - compatible: Must be one of "adi,ad7124-4" or "adi,ad7124-8"
> > + - reg: SPI chip select number for the device
> > + - spi-max-frequency: Max SPI frequency to use
> > + see: Documentation/devicetree/bindings/spi/spi-bus.txt
> > + - clocks: phandle to the master clock (mclk)
> > + see: Documentation/devicetree/bindings/clock/clock-
> > bindings.txt
> > + - clock-names: Must be "mclk".
> > + - interrupts: IRQ line for the ADC
> > + see: Documentation/devicetree/bindings/interrupt-
> > controller/interrupts.txt
> Given the driver doesn't currently use it, should perhaps be optional?
>
The interrupt is actually required by ad_sigma_delta.
>
> >
> >
> > +
> > + Required properties:
> > + * #address-cells: Must be 1.
> > + * #size-cells: Must be 0.
> > +
> > + Subnode(s) represent the external channels which are
> > connected to the ADC.
> > + Each subnode represents one channel and has the following
> > properties:
> > + Required properties:
> > + * reg: The channel number. It can have up to 4
> > channels on ad7124-4
> > + and 8 channels on ad7124-8, numbered from 0
> > to 15.
> > + * diff-channels: see:
> > Documentation/devicetree/bindings/iio/adc/adc.txt
> > +
> > + Optional properties:
> > + * bipolar: see:
> > Documentation/devicetree/bindings/iio/adc/adc.txt
> > + * adi,reference-select: Select the reference
> > source to use when
> > + converting on the the specific channel.
> > Valid values are:
> > + 0: REFIN1(+)/REFIN1(−).
> > + 1: REFIN2(+)/REFIN2(−).
> > + 3: AVDD
> > + If this field is left empty, internal
> > reference is selected.
> > +
> > +Optional properties:
> > + - refin1-supply: refin1 supply can be used as reference for
> > conversion.
> > + - refin2-supply: refin2 supply can be used as reference for
> > conversion.
> > + - avdd-supply: avdd supply can be used as reference for
> > conversion.
> > +
> > +Example:
> > + adc@0 {
> > + compatible = "adi,ad7124-4";
> > + reg = <0>;
> > + spi-max-frequency = <500>;
> > + interrupts = <25 2>;
> > + interrupt-parent = <&gpio>;
> > + refin1-supply = <&adc_vref>;
> > + clocks = <&ad7124_mclk>;
> > + clock-names = "mclk";
> > +
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + channel@0 {
> > + reg = <0>;
> > + adi,diff-channels = <0 1>;
> > + adi,reference-select = <0>;
> > + adi,gain = <2>;
> > + adi,odr-hz = <10>;
> I think you have dropped these two..
>
> >
> >
> > + };
> > +
> > + channel@1 {
> > + reg = <1>;
> > + adi,bipolar;
> > + adi,diff-channels = <2 3>;
> > + adi,reference-select = <0>;
> > + adi,gain = <4>;
> > + adi,odr-hz = <50>;
> > + };
> > +
> > + channel@2 {
> > + reg = <2>;
> > + adi,diff-channels = <4 5>;
> > + adi,gain = <128>;
> > + adi,odr-hz = <19200>;
> > + };
> > +
> > + channel@3
Re: [PATCH v3 2/3] iio: adc: Add ad7124 support
On Jo, 2018-11-08 at 10:34 -0600, Rob Herring wrote: > On Thu, Nov 8, 2018 at 9:02 AM Popa, Stefan Serban > wrote: > > > > > > On Sb, 2018-11-03 at 12:16 +, Jonathan Cameron wrote: > > > > > > On Mon, 29 Oct 2018 18:38:31 +0200 > > > Stefan Popa wrote: > > > > > > > > > > > > > > > The ad7124-4 and ad7124-8 are a family of 4 and 8 channel sigma- > > > > delta > > > > ADCs > > > > with 24-bit precision and reference. > > > > > > > > Three power modes are available which in turn affect the output > > > > data > > > > rate: > > > > * Full power: 9.38 SPS to 19,200 SPS > > > > * Mid power: 2.34 SPS to 4800 SPS > > > > * Low power: 1.17 SPS to 2400 SPS > > > > > > > > The ad7124-4 can be configured to have four differential inputs, > > > > while > > > > ad7124-8 can have 8. Moreover, ad7124 also supports per channel > > > > configuration. Each configuration consists of gain, reference > > > > source, > > > > output data rate and bipolar/unipolar configuration. > > > > > > > > Datasheets: > > > > Link: http://www.analog.com/media/en/technical-documentation/data-s > > > > heet > > > > s/AD7124-4.pdf > > > > Link: http://www.analog.com/media/en/technical-documentation/data-s > > > > heet > > > > s/ad7124-8.pdf > > > > > > > > Signed-off-by: Stefan Popa > > > Hi Stefan, > > > > > > The discussion around the DT binding has gotten me looking at bit > > > more closely at that for this version. > > > > > > Some most comments in that section. Also a really minor ordering > > > issue > > > in > > > remove which I'd just have fixed if we weren't going around again for > > > the binding. > > > > > > Main binding thing is I don't think the odr value belongs in DT. > > > Gain is more marginal (unless the part can actually be damaged by > > > a wrong value - which I hope it can't!). I'm not that fussed > > > as there are definitely reasons to specify a default scale to > > > cover the reasonable range on a pin. > > > > > > Thanks, > > > > > > Jonathan > > Hi Jonathan, > > > > Thank you for the review! So, how should I proceed? > > > > First, we need an adc.txt file where "bipolar" and something like > > "diff- > > channels" should be documented. Should the file be placed under > > Documentation/devicetree/bindings/iio/adc? > Yes. > > > > > Regarding the "odr-hz" property, it totally makes sense to remove it > > from > > the DT. How about the "gain"? Should we leave it in the DT and also add > > the > > possibility to be configured from user space? > Look at other bindings. I think there are others having gain. If not, > then it probably should only be user space configurable. If so, then > can it be a common property too. > > Rob > Hi Rob, I found only a couple of examples using gain in other bindings, so I guess it's not common practice. I will remove the gain as well from the DT and set it with the default of 1. @Jonathan: I think that IIO_CHAN_INFO_HARDWAREGAIN is the attribute that can be used in user space? Thank you! -Stefan
Re: [PATCH v3 2/3] iio: adc: Add ad7124 support
On Sb, 2018-11-03 at 12:16 +, Jonathan Cameron wrote: > On Mon, 29 Oct 2018 18:38:31 +0200 > Stefan Popa wrote: > > > > > The ad7124-4 and ad7124-8 are a family of 4 and 8 channel sigma-delta > > ADCs > > with 24-bit precision and reference. > > > > Three power modes are available which in turn affect the output data > > rate: > > * Full power: 9.38 SPS to 19,200 SPS > > * Mid power: 2.34 SPS to 4800 SPS > > * Low power: 1.17 SPS to 2400 SPS > > > > The ad7124-4 can be configured to have four differential inputs, while > > ad7124-8 can have 8. Moreover, ad7124 also supports per channel > > configuration. Each configuration consists of gain, reference source, > > output data rate and bipolar/unipolar configuration. > > > > Datasheets: > > Link: http://www.analog.com/media/en/technical-documentation/data-sheet > > s/AD7124-4.pdf > > Link: http://www.analog.com/media/en/technical-documentation/data-sheet > > s/ad7124-8.pdf > > > > Signed-off-by: Stefan Popa > Hi Stefan, > > The discussion around the DT binding has gotten me looking at bit > more closely at that for this version. > > Some most comments in that section. Also a really minor ordering issue > in > remove which I'd just have fixed if we weren't going around again for > the binding. > > Main binding thing is I don't think the odr value belongs in DT. > Gain is more marginal (unless the part can actually be damaged by > a wrong value - which I hope it can't!). I'm not that fussed > as there are definitely reasons to specify a default scale to > cover the reasonable range on a pin. > > Thanks, > > Jonathan Hi Jonathan, Thank you for the review! So, how should I proceed? First, we need an adc.txt file where "bipolar" and something like "diff- channels" should be documented. Should the file be placed under Documentation/devicetree/bindings/iio/adc? Regarding the "odr-hz" property, it totally makes sense to remove it from the DT. How about the "gain"? Should we leave it in the DT and also add the possibility to be configured from user space? Regards, -Stefan > > > > --- > > Changes in v2: > > - Added this commit. > > Changes in v3: > > - Removed channel, address, scan_index and shift fields from > > ad7124_channel_template. > > - Added a sanity check for val2 in ad7124_write_raw(). > > - Used the "reg" property to get the channel address and "adi,diff- > > channels" > > for the differential pins. The "adi,channel-number" property was > > removed. > > - When calling regulator_get_optional, the probe is given up in > > case of error, > > but continues in case of -ENODEV. > > - clk_disable_unprepare() is called before > > ad_sd_cleanup_buffer_and_trigger > > in ad7124_remove(). > > > > MAINTAINERS | 7 + > > drivers/iio/adc/Kconfig | 11 + > > drivers/iio/adc/Makefile | 1 + > > drivers/iio/adc/ad7124.c | 648 > > +++ > > 4 files changed, 667 insertions(+) > > create mode 100644 drivers/iio/adc/ad7124.c > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index f642044..3a1bfcb 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -839,6 +839,13 @@ S: Supported > > F: drivers/iio/dac/ad5758.c > > F: Documentation/devicetree/bindings/iio/dac/ad5758.txt > > > > +ANALOG DEVICES INC AD7124 DRIVER > > +M: Stefan Popa > > +L: linux-...@vger.kernel.org > > +W: http://ez.analog.com/community/linux-device-drivers > > +S: Supported > > +F: drivers/iio/adc/ad7124.c > > + > > ANALOG DEVICES INC AD9389B DRIVER > > M: Hans Verkuil > > L: linux-me...@vger.kernel.org > > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig > > index a52fea8..148a10f 100644 > > --- a/drivers/iio/adc/Kconfig > > +++ b/drivers/iio/adc/Kconfig > > @@ -10,6 +10,17 @@ config AD_SIGMA_DELTA > > select IIO_BUFFER > > select IIO_TRIGGERED_BUFFER > > > > +config AD7124 > > + tristate "Analog Devices AD7124 and similar sigma-delta ADCs > > driver" > > + depends on SPI_MASTER > > + select AD_SIGMA_DELTA > > + help > > + Say yes here to build support for Analog Devices AD7124-4 > > and AD7124-8 > > + SPI analog to digital converters (ADC). > > + > > + To compile this driver as a module, choose M here: the > > module will be > > + called ad7124. > > + > > config AD7266 > > tristate "Analog Devices AD7265/AD7266 ADC driver" > > depends on SPI_MASTER > > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile > > index a6e6a0b..76168b2 100644 > > --- a/drivers/iio/adc/Makefile > > +++ b/drivers/iio/adc/Makefile > > @@ -5,6 +5,7 @@ > > > > # When adding new entries keep the list in alphabetical order > > obj-$(CONFIG_AD_SIGMA_DELTA) += ad_sigma_delta.o > > +obj-$(CONFIG_AD7124) += ad7124.o > > obj-$(CONFIG_AD7266) += ad7266.o > > obj-$(CONFIG_AD7291) += ad7291.o > > obj-$(CONFIG_AD7298) += ad7298.o > > diff --git a/drivers/iio/adc/ad7124.c b/drivers/iio/adc/ad7124.c > > new file mode
Re: [PATCH v4 3/6] regmap: Add regmap_noinc_read API
On Lu, 2018-08-06 at 15:39 +0100, Mark Brown wrote:
> On Mon, Aug 06, 2018 at 03:04:44PM +0300, Stefan Popa wrote:
>
> >
> > + if (!regmap_volatile(map, reg) || !regmap_readable(map, reg))
> > {
> > + ret = -EINVAL;
> > + goto out_unlock;
> > + }
> I suggested having an explicit operation to check if a register supports
> this mode of operation when I reviewed an earlier version, I didn't
> notice a reply on that?
Hi Mark,
Sorry, I got confused about what needs to be done. Should I add a new field
to the regmap_config struct which will be checked during the function call?
Something like can_multi_write? How do you suggest it should be called? Is
readable_noinc_reg ok?
Thank you!
Stefan
Re: [PATCH v2 1/6] iio: adxl372: New driver for Analog Devices ADXL372 Accelerometer
On Jo, 2018-08-02 at 14:05 +0100, Jonathan Cameron wrote: > On Wed, 1 Aug 2018 18:13:09 +0300 > Stefan Popa wrote: > > > > > This patch adds basic support for Analog Devices ADXL372 SPI-Bus > > Three-Axis Digital Accelerometer. > > > > The device is probed and configured the with some initial default > > values. With this basic driver, it is possible to read raw acceleration > > data. > > > > Datasheet: > > http://www.analog.com/media/en/technical-documentation/data-sheets/ADXL > > 372.pdf > > > > Signed-off-by: Stefan Popa > Hi Stefan, > > I would have preferred a cover letter (if nothing else it keeps them > together in > a thread in people's email clients) > > I've just reviewed all of these and I think the only real outstanding > issue > is Mark's requests for small changes in the regmap stuff. > > The open question on what to do long term about 'peak' and similar > derived > channels can probably wait for another day. > > Thanks, > Hi Jonathan, Thank you very much for your review! If you agree, let's leave the 'peak' mode out of this first version of the driver. -Stefan > > > > > --- > > Changes in v2: > > - removed ADXL372_RD_FLAG_MSK and ADXL372_WR_FLAG_MSK macros. > > - handled regmap read/write by setting reg_bits and pad_bits > > fields in regmap_config struct. > > - removed the buffer specifications when defining the channels. > > - changed the activity and inactivity thresholds. > > - added two new functions for setting the activity and inactivity > > timers: adxl372_set_inactivity_time_ms() and > > adxl372_set_activity_time_ms(). > > > > MAINTAINERS | 6 + > > drivers/iio/accel/Kconfig | 11 + > > drivers/iio/accel/Makefile | 1 + > > drivers/iio/accel/adxl372.c | 530 > > > > 4 files changed, 548 insertions(+) > > create mode 100644 drivers/iio/accel/adxl372.c > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index 60b1028..2ba47bb 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -543,6 +543,12 @@ W: http://ez.analog.com/community/linux-dev > > ice-drivers > > S: Supported > > F: drivers/input/misc/adxl34x.c > > > > +ADXL372 THREE-AXIS DIGITAL ACCELEROMETER DRIVER > > +M: Stefan Popa > > +W: http://ez.analog.com/community/linux-device-drivers > > +S: Supported > > +F: drivers/iio/accel/adxl372.c > > + > > AF9013 MEDIA DRIVER > > M: Antti Palosaari > > L: linux-me...@vger.kernel.org > > diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig > > index 62ae7e5..1b496ef 100644 > > --- a/drivers/iio/accel/Kconfig > > +++ b/drivers/iio/accel/Kconfig > > @@ -60,6 +60,17 @@ config ADXL345_SPI > > will be called adxl345_spi and you will also get > > adxl345_core > > for the core module. > > > > +config ADXL372 > > + tristate "Analog Devices ADXL372 3-Axis Accelerometer Driver" > > + depends on SPI > > + select IIO_BUFFER > > + select IIO_TRIGGERED_BUFFER > > + help > > + Say yes here to add support for the Analog Devices ADXL372 > > triaxial > > + acceleration sensor. > > + To compile this driver as a module, choose M here: the > > + module will be called adxl372. > > + > > config BMA180 > > tristate "Bosch BMA180/BMA250 3-Axis Accelerometer Driver" > > depends on I2C > > diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile > > index 636d4d1..5758ffc 100644 > > --- a/drivers/iio/accel/Makefile > > +++ b/drivers/iio/accel/Makefile > > @@ -9,6 +9,7 @@ obj-$(CONFIG_ADIS16209) += adis16209.o > > obj-$(CONFIG_ADXL345) += adxl345_core.o > > obj-$(CONFIG_ADXL345_I2C) += adxl345_i2c.o > > obj-$(CONFIG_ADXL345_SPI) += adxl345_spi.o > > +obj-$(CONFIG_ADXL372) += adxl372.o > > obj-$(CONFIG_BMA180) += bma180.o > > obj-$(CONFIG_BMA220) += bma220_spi.o > > obj-$(CONFIG_BMC150_ACCEL) += bmc150-accel-core.o > > diff --git a/drivers/iio/accel/adxl372.c b/drivers/iio/accel/adxl372.c > > new file mode 100644 > > index 000..7d5092d > > --- /dev/null > > +++ b/drivers/iio/accel/adxl372.c > > @@ -0,0 +1,530 @@ > > +// SPDX-License-Identifier: GPL-2.0+ > > +/* > > + * ADXL372 3-Axis Digital Accelerometer SPI driver > > + * > > + * Copyright 2018 Analog Devices Inc. > > + */ > > + > > +#include > > +#include > > +#include > > +#include > > + > > +#include > > +#include > > + > > +/* ADXL372 registers definition */ > > +#define ADXL372_DEVID 0x00 > > +#define ADXL372_DEVID_MST 0x01 > > +#define ADXL372_PARTID 0x02 > > +#define ADXL372_REVID 0x03 > > +#define ADXL372_STATUS_1 0x04 > > +#define ADXL372_STATUS_2 0x05 > > +#define ADXL372_FIFO_ENTRIES_2 0x06 > > +#define ADXL372_FIFO_ENTRIES_1 0x07 > > +#define ADXL372_X_DATA_H 0x08 > > +#define ADXL372_X_DATA_L 0x09 > > +#define ADXL372_Y_DATA_H 0x0A > > +#define ADXL372_Y_DATA_L 0x0B
Re: [PATCH v4 1/2] iio: dac: Add AD5758 support
On Du, 2018-07-01 at 00:26 +0300, Andy Shevchenko wrote:
> On Fri, Jun 29, 2018 at 11:38 AM, Stefan Popa wrote:
> >
> > The AD5758 is a single channel DAC with 16-bit precision which uses the
> > SPI interface that operates at clock rates up to 50MHz.
> >
> > The output can be configured as voltage or current and is available on a
> > single terminal.
> >
> > Datasheet:
> > http://www.analog.com/media/en/technical-documentation/data-sheets/ad5758.pdf
> Thanks for an update.
> Few comments below.
>
> >
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> Perhaps keep them ordered?
>
> >
> > +
> > +#include
> > +#include
> > +
> >
> > +#include
> ASM? Hmm...
>
> >
> > +static int cmpfunc(const void *a, const void *b)
> > +{
> > + return (*(int *)a - *(int *)b);
> Surrounding parens are not needed.
>
> >
> > +}
> > +
> >
> > +static int ad5758_find_closest_match(const int *array,
> > +unsigned int size, int val)
> > +{
> > + int i;
> > +
> > + for (i = 0; i < size; i++) {
> > + if (val <= array[i])
> > + return i;
> > + }
> > +
> > + return size - 1;
> > +}
> Isn't it what bsearch() covers as well?
>
bsearch() looks for an item in an array and returns NULL if it is not found.
This function returns the index of the closest value to val even if there is
no exact match.
> >
> > + do {
> > + ret = ad5758_spi_reg_read(st, reg);
> > + if (ret < 0)
> > + return ret;
> > +
> > + if (!(ret & mask))
> > + return 0;
> > +
> >
> > + udelay(100);
> If it's not called from atomic context, perhaps switch to usleep_range() ?
>
> >
> > + } while (--timeout);
>
> >
> > +static int ad5758_soft_reset(struct ad5758_state *st)
> > +{
> > + int ret;
> > +
> > + ret = ad5758_spi_reg_write(st, AD5758_KEY, AD5758_KEY_CODE_RESET_1);
> > + if (ret < 0)
> > + return ret;
> > +
> > + ret = ad5758_spi_reg_write(st, AD5758_KEY, AD5758_KEY_CODE_RESET_2);
> > +
> >
> > + /* Perform a software reset and wait 100us */
> > + udelay(100);
> Ditto.
>
> >
> > +
> > + return ret;
> > +}
> >
> > +static int ad5758_find_out_range(struct ad5758_state *st,
> > +const struct ad5758_range *range,
> > +unsigned int size,
> > +int min, int max)
> > +{
> > + int i;
> > +
> > + for (i = 0; i < size; i++) {
> > + if ((min == range[i].min) && (max == range[i].max)) {
> > + st->out_range.reg = range[i].reg;
> > + st->out_range.min = range[i].min;
> > + st->out_range.max = range[i].max;
> > +
> > + return 0;
> > + }
> > + }
> One more candidate to use bsearch().
>
I am not sure if bsearch() will work in this case since it requires that the
given
array is sorted. Moreover, both ad5758_voltage_range[] and
ad5758_current_range[]
contain duplicate elements.
> >
> > +
> > + return -EINVAL;
> > +}
> >
> > + index = (int *) bsearch(&tmp, ad5758_dc_dc_ilim,
> > + ARRAY_SIZE(ad5758_dc_dc_ilim),
> > + sizeof(int), cmpfunc);
> I'm not sure you need that casting.
>
