| Message ID | 20190912144310.7458-2-andrea.merello@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fixes for ad7949 | expand |
On Thu, 2019-09-12 at 16:43 +0200, Andrea Merello wrote: > [External] > > The device could be configured to spit out also the configuration word > while reading the AD result value (in the same SPI xfer) - this is called > "readback" in the device datasheet. > > The driver checks if readback is enabled and it eventually adjusts the SPI > xfer length and it applies proper shifts to still get the data, discarding > the configuration word. > > The readback option is actually never enabled (the driver disables it), so > the said checks do not serve for any purpose. > > Since enabling the readback option seems not to provide any advantage (the > driver entirely sets the configuration word without relying on any default > value), just kill the said, unused, code. Reviewed-by: Alexandru Ardelean <alexandru.ardelean@analog.com> > > Signed-off-by: Andrea Merello <andrea.merello@gmail.com> > --- > drivers/iio/adc/ad7949.c | 27 +++------------------------ > 1 file changed, 3 insertions(+), 24 deletions(-) > > diff --git a/drivers/iio/adc/ad7949.c b/drivers/iio/adc/ad7949.c > index ac0ffff6c5ae..518044c31a73 100644 > --- a/drivers/iio/adc/ad7949.c > +++ b/drivers/iio/adc/ad7949.c > @@ -57,29 +57,11 @@ struct ad7949_adc_chip { > u32 buffer ____cacheline_aligned; > }; > > -static bool ad7949_spi_cfg_is_read_back(struct ad7949_adc_chip *ad7949_adc) > -{ > - if (!(ad7949_adc->cfg & AD7949_CFG_READ_BACK)) > - return true; > - > - return false; > -} > - > -static int ad7949_spi_bits_per_word(struct ad7949_adc_chip *ad7949_adc) > -{ > - int ret = ad7949_adc->resolution; > - > - if (ad7949_spi_cfg_is_read_back(ad7949_adc)) > - ret += AD7949_CFG_REG_SIZE_BITS; > - > - return ret; > -} > - > static int ad7949_spi_write_cfg(struct ad7949_adc_chip *ad7949_adc, u16 val, > u16 mask) > { > int ret; > - int bits_per_word = ad7949_spi_bits_per_word(ad7949_adc); > + int bits_per_word = ad7949_adc->resolution; > int shift = bits_per_word - AD7949_CFG_REG_SIZE_BITS; > struct spi_message msg; > struct spi_transfer tx[] = { > @@ -107,7 +89,7 @@ static int ad7949_spi_read_channel(struct ad7949_adc_chip *ad7949_adc, int *val, > unsigned int channel) > { > int ret; > - int bits_per_word = ad7949_spi_bits_per_word(ad7949_adc); > + int bits_per_word = ad7949_adc->resolution; > int mask = GENMASK(ad7949_adc->resolution, 0); > struct spi_message msg; > struct spi_transfer tx[] = { > @@ -138,10 +120,7 @@ static int ad7949_spi_read_channel(struct ad7949_adc_chip *ad7949_adc, int *val, > > ad7949_adc->current_channel = channel; > > - if (ad7949_spi_cfg_is_read_back(ad7949_adc)) > - *val = (ad7949_adc->buffer >> AD7949_CFG_REG_SIZE_BITS) & mask; > - else > - *val = ad7949_adc->buffer & mask; > + *val = ad7949_adc->buffer & mask; > > return 0; > }
On Fri, 13 Sep 2019 06:37:17 +0000 "Ardelean, Alexandru" <alexandru.Ardelean@analog.com> wrote: > On Thu, 2019-09-12 at 16:43 +0200, Andrea Merello wrote: > > [External] > > > > The device could be configured to spit out also the configuration word > > while reading the AD result value (in the same SPI xfer) - this is called > > "readback" in the device datasheet. > > > > The driver checks if readback is enabled and it eventually adjusts the SPI > > xfer length and it applies proper shifts to still get the data, discarding > > the configuration word. > > > > The readback option is actually never enabled (the driver disables it), so > > the said checks do not serve for any purpose. > > > > Since enabling the readback option seems not to provide any advantage (the > > driver entirely sets the configuration word without relying on any default > > value), just kill the said, unused, code. > > Reviewed-by: Alexandru Ardelean <alexandru.ardelean@analog.com> Applied to the togreg branch of iio.git and pushed out as testing for the autobuilders to poke at it. Thanks, Jonathan > > > > > Signed-off-by: Andrea Merello <andrea.merello@gmail.com> > > --- > > drivers/iio/adc/ad7949.c | 27 +++------------------------ > > 1 file changed, 3 insertions(+), 24 deletions(-) > > > > diff --git a/drivers/iio/adc/ad7949.c b/drivers/iio/adc/ad7949.c > > index ac0ffff6c5ae..518044c31a73 100644 > > --- a/drivers/iio/adc/ad7949.c > > +++ b/drivers/iio/adc/ad7949.c > > @@ -57,29 +57,11 @@ struct ad7949_adc_chip { > > u32 buffer ____cacheline_aligned; > > }; > > > > -static bool ad7949_spi_cfg_is_read_back(struct ad7949_adc_chip *ad7949_adc) > > -{ > > - if (!(ad7949_adc->cfg & AD7949_CFG_READ_BACK)) > > - return true; > > - > > - return false; > > -} > > - > > -static int ad7949_spi_bits_per_word(struct ad7949_adc_chip *ad7949_adc) > > -{ > > - int ret = ad7949_adc->resolution; > > - > > - if (ad7949_spi_cfg_is_read_back(ad7949_adc)) > > - ret += AD7949_CFG_REG_SIZE_BITS; > > - > > - return ret; > > -} > > - > > static int ad7949_spi_write_cfg(struct ad7949_adc_chip *ad7949_adc, u16 val, > > u16 mask) > > { > > int ret; > > - int bits_per_word = ad7949_spi_bits_per_word(ad7949_adc); > > + int bits_per_word = ad7949_adc->resolution; > > int shift = bits_per_word - AD7949_CFG_REG_SIZE_BITS; > > struct spi_message msg; > > struct spi_transfer tx[] = { > > @@ -107,7 +89,7 @@ static int ad7949_spi_read_channel(struct ad7949_adc_chip *ad7949_adc, int *val, > > unsigned int channel) > > { > > int ret; > > - int bits_per_word = ad7949_spi_bits_per_word(ad7949_adc); > > + int bits_per_word = ad7949_adc->resolution; > > int mask = GENMASK(ad7949_adc->resolution, 0); > > struct spi_message msg; > > struct spi_transfer tx[] = { > > @@ -138,10 +120,7 @@ static int ad7949_spi_read_channel(struct ad7949_adc_chip *ad7949_adc, int *val, > > > > ad7949_adc->current_channel = channel; > > > > - if (ad7949_spi_cfg_is_read_back(ad7949_adc)) > > - *val = (ad7949_adc->buffer >> AD7949_CFG_REG_SIZE_BITS) & mask; > > - else > > - *val = ad7949_adc->buffer & mask; > > + *val = ad7949_adc->buffer & mask; > > > > return 0; > > }
diff --git a/drivers/iio/adc/ad7949.c b/drivers/iio/adc/ad7949.c index ac0ffff6c5ae..518044c31a73 100644 --- a/drivers/iio/adc/ad7949.c +++ b/drivers/iio/adc/ad7949.c @@ -57,29 +57,11 @@ struct ad7949_adc_chip { u32 buffer ____cacheline_aligned; }; -static bool ad7949_spi_cfg_is_read_back(struct ad7949_adc_chip *ad7949_adc) -{ - if (!(ad7949_adc->cfg & AD7949_CFG_READ_BACK)) - return true; - - return false; -} - -static int ad7949_spi_bits_per_word(struct ad7949_adc_chip *ad7949_adc) -{ - int ret = ad7949_adc->resolution; - - if (ad7949_spi_cfg_is_read_back(ad7949_adc)) - ret += AD7949_CFG_REG_SIZE_BITS; - - return ret; -} - static int ad7949_spi_write_cfg(struct ad7949_adc_chip *ad7949_adc, u16 val, u16 mask) { int ret; - int bits_per_word = ad7949_spi_bits_per_word(ad7949_adc); + int bits_per_word = ad7949_adc->resolution; int shift = bits_per_word - AD7949_CFG_REG_SIZE_BITS; struct spi_message msg; struct spi_transfer tx[] = { @@ -107,7 +89,7 @@ static int ad7949_spi_read_channel(struct ad7949_adc_chip *ad7949_adc, int *val, unsigned int channel) { int ret; - int bits_per_word = ad7949_spi_bits_per_word(ad7949_adc); + int bits_per_word = ad7949_adc->resolution; int mask = GENMASK(ad7949_adc->resolution, 0); struct spi_message msg; struct spi_transfer tx[] = { @@ -138,10 +120,7 @@ static int ad7949_spi_read_channel(struct ad7949_adc_chip *ad7949_adc, int *val, ad7949_adc->current_channel = channel; - if (ad7949_spi_cfg_is_read_back(ad7949_adc)) - *val = (ad7949_adc->buffer >> AD7949_CFG_REG_SIZE_BITS) & mask; - else - *val = ad7949_adc->buffer & mask; + *val = ad7949_adc->buffer & mask; return 0; }
The device could be configured to spit out also the configuration word while reading the AD result value (in the same SPI xfer) - this is called "readback" in the device datasheet. The driver checks if readback is enabled and it eventually adjusts the SPI xfer length and it applies proper shifts to still get the data, discarding the configuration word. The readback option is actually never enabled (the driver disables it), so the said checks do not serve for any purpose. Since enabling the readback option seems not to provide any advantage (the driver entirely sets the configuration word without relying on any default value), just kill the said, unused, code. Signed-off-by: Andrea Merello <andrea.merello@gmail.com> --- drivers/iio/adc/ad7949.c | 27 +++------------------------ 1 file changed, 3 insertions(+), 24 deletions(-)