Re: [PATCH v2 3/6] iio: ad7949: Support configuration read-back

[Adam Michaelis <adam.michaelis@collins.com>]

The configuration read-back feature is being maintained from the
original version of this driver. Before adding the device tree entry,
there was no way to change this setting other than debugfs raw access
to the SPI interface, and there is still no access to the returned
configuration data should the feature be enabled. I would be willing
to remove the feature altogether, but wanted to tread softly on
existing features.

On Sun, May 5, 2019 at 9:42 AM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Thu,  2 May 2019 11:14:29 -0500
> Adam Michaelis <adam.michaelis@rockwellcollins.com> wrote:
>
> > Adds device tree parameter to set the configuration read-back bit
> > in the configuration register to tell the AD7949 to include the value of
> > the configuration register at the time the current sample was acquired
> > when reading from the part.
> >
> > Further work must be done to make read-back information available to
> > consumer.
>
> This needs some explanation of why it is useful at all. I'm certainly unclear
> on why it would be useful to configure this at boot time.
>
> Code looks fine.
>
> Jonathan
>
> >
> > Signed-off-by: Adam Michaelis <adam.michaelis@rockwellcollins.com>
> > ---
> >       V2: Add some defines to reduce use of magic numbers.
> > ---
> >  drivers/iio/adc/ad7949.c | 12 +++++++++++-
> >  1 file changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/iio/adc/ad7949.c b/drivers/iio/adc/ad7949.c
> > index afc1361af5fb..7820e1097787 100644
> > --- a/drivers/iio/adc/ad7949.c
> > +++ b/drivers/iio/adc/ad7949.c
> > @@ -69,6 +69,7 @@ struct ad7949_adc_spec {
> >   * @iio_dev: reference to iio structure
> >   * @spi: reference to spi structure
> >   * @ref_sel: selected reference voltage source
> > + * @cfg_readback: whether reads will include configuration data
> >   * @resolution: resolution of the chip
> >   * @cfg: copy of the configuration register
> >   * @current_channel: current channel in use
> > @@ -80,6 +81,7 @@ struct ad7949_adc_chip {
> >       struct iio_dev *indio_dev;
> >       struct spi_device *spi;
> >       enum ad7949_ref_sel ref_sel;
> > +     bool cfg_readback;
> >       u8 resolution;
> >       u16 cfg;
> >       unsigned int current_channel;
> > @@ -283,7 +285,11 @@ static int ad7949_spi_init(struct ad7949_adc_chip *ad7949_adc)
> >                       AD7949_CFG_REF_SEL_MASK;
> >       adc_config |= (AD7949_CFG_SEQ_DISABLED << AD7949_CFG_SEQ_SHIFT) &
> >                       AD7949_CFG_SEQ_MASK;
> > -     adc_config |= AD7949_CFG_READBACK_DIS;
> > +
> > +     if (ad7949_adc->cfg_readback)
> > +             adc_config |= AD7949_CFG_READBACK_EN;
> > +     else
> > +             adc_config |= AD7949_CFG_READBACK_DIS;
> >
> >       ret = ad7949_spi_write_cfg(ad7949_adc,
> >                       adc_config,
> > @@ -331,6 +337,10 @@ static int ad7949_spi_probe(struct spi_device *spi)
> >       indio_dev->num_channels = spec->num_channels;
> >       ad7949_adc->resolution = spec->resolution;
> >
> > +     ad7949_adc->cfg_readback = of_property_read_bool(
> > +                     ad7949_adc->indio_dev->dev.of_node,
> > +                     "adi,cfg-readback");
> > +
> >       ret = of_property_read_u32(ad7949_adc->indio_dev->dev.of_node,
> >                       "adi,reference-select",
> >                       &temp);
>


-- 

Adam Michaelis | Sr Software Engineer | Comm Engineering | Mission Systems

COLLINS AEROSPACE

400 Collins Road, Cedar Rapids, IA 52498 U.S.A.

Tel: +1 319 295 4102

adam.michaelis@collins.com | collinsaerospace.com



CONFIDENTIALITY WARNING: This message may contain proprietary and/or
privileged information of Collins Aerospace and its affiliated
companies. If you are not the intended recipient, please 1) Do not
disclose, copy, distribute or use this message or its contents. 2)
Advise the sender by return email. 3) Delete all copies (including all
attachments) from your computer. Your cooperation is greatly
appreciated.