| Message ID | 20200403132717.24682-1-lars@metafoo.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/5] iio: xilinx-xadc: Fix ADC-B powerdown | expand |
On Fri, 3 Apr 2020 15:27:13 +0200 Lars-Peter Clausen <lars@metafoo.de> wrote: > The check for shutting down the second ADC is inverted. This causes it to > be powered down when it should be enabled. As a result channels that are > supposed to be handled by the second ADC return invalid conversion results. > > Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> Fixes tag? Definitely sounds like something we should be backporting! Jonathan > --- > drivers/iio/adc/xilinx-xadc-core.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/iio/adc/xilinx-xadc-core.c b/drivers/iio/adc/xilinx-xadc-core.c > index 2d6505a66511..4fcf1729341f 100644 > --- a/drivers/iio/adc/xilinx-xadc-core.c > +++ b/drivers/iio/adc/xilinx-xadc-core.c > @@ -722,13 +722,14 @@ static int xadc_power_adc_b(struct xadc *xadc, unsigned int seq_mode) > { > uint16_t val; > > + /* Powerdown the ADC-B when it is not needed. */ > switch (seq_mode) { > case XADC_CONF1_SEQ_SIMULTANEOUS: > case XADC_CONF1_SEQ_INDEPENDENT: > - val = XADC_CONF2_PD_ADC_B; > + val = 0; > break; > default: > - val = 0; > + val = XADC_CONF2_PD_ADC_B; > break; > } >
On 4/5/20 2:10 PM, Jonathan Cameron wrote: > On Fri, 3 Apr 2020 15:27:13 +0200 > Lars-Peter Clausen <lars@metafoo.de> wrote: > >> The check for shutting down the second ADC is inverted. This causes it to >> be powered down when it should be enabled. As a result channels that are >> supposed to be handled by the second ADC return invalid conversion results. >> >> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> > Fixes tag? Definitely sounds like something we should be backporting! Fixes: bdc8cda1d010 ("iio:adc: Add Xilinx XADC driver") > > Jonathan > >> --- >> drivers/iio/adc/xilinx-xadc-core.c | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/iio/adc/xilinx-xadc-core.c b/drivers/iio/adc/xilinx-xadc-core.c >> index 2d6505a66511..4fcf1729341f 100644 >> --- a/drivers/iio/adc/xilinx-xadc-core.c >> +++ b/drivers/iio/adc/xilinx-xadc-core.c >> @@ -722,13 +722,14 @@ static int xadc_power_adc_b(struct xadc *xadc, unsigned int seq_mode) >> { >> uint16_t val; >> >> + /* Powerdown the ADC-B when it is not needed. */ >> switch (seq_mode) { >> case XADC_CONF1_SEQ_SIMULTANEOUS: >> case XADC_CONF1_SEQ_INDEPENDENT: >> - val = XADC_CONF2_PD_ADC_B; >> + val = 0; >> break; >> default: >> - val = 0; >> + val = XADC_CONF2_PD_ADC_B; >> break; >> } >>
On Sun, 5 Apr 2020 14:13:32 +0200 Lars-Peter Clausen <lars@metafoo.de> wrote: > On 4/5/20 2:10 PM, Jonathan Cameron wrote: > > On Fri, 3 Apr 2020 15:27:13 +0200 > > Lars-Peter Clausen <lars@metafoo.de> wrote: > > > >> The check for shutting down the second ADC is inverted. This causes it to > >> be powered down when it should be enabled. As a result channels that are > >> supposed to be handled by the second ADC return invalid conversion results. > >> > >> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> > > Fixes tag? Definitely sounds like something we should be backporting! > > Fixes: bdc8cda1d010 ("iio:adc: Add Xilinx XADC driver") For all of them? (just checking) Jonathan > > > > > Jonathan > > > >> --- > >> drivers/iio/adc/xilinx-xadc-core.c | 5 +++-- > >> 1 file changed, 3 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/iio/adc/xilinx-xadc-core.c b/drivers/iio/adc/xilinx-xadc-core.c > >> index 2d6505a66511..4fcf1729341f 100644 > >> --- a/drivers/iio/adc/xilinx-xadc-core.c > >> +++ b/drivers/iio/adc/xilinx-xadc-core.c > >> @@ -722,13 +722,14 @@ static int xadc_power_adc_b(struct xadc *xadc, unsigned int seq_mode) > >> { > >> uint16_t val; > >> > >> + /* Powerdown the ADC-B when it is not needed. */ > >> switch (seq_mode) { > >> case XADC_CONF1_SEQ_SIMULTANEOUS: > >> case XADC_CONF1_SEQ_INDEPENDENT: > >> - val = XADC_CONF2_PD_ADC_B; > >> + val = 0; > >> break; > >> default: > >> - val = 0; > >> + val = XADC_CONF2_PD_ADC_B; > >> break; > >> } > >> > >
On 4/5/20 2:26 PM, Jonathan Cameron wrote: > On Sun, 5 Apr 2020 14:13:32 +0200 > Lars-Peter Clausen <lars@metafoo.de> wrote: > >> On 4/5/20 2:10 PM, Jonathan Cameron wrote: >>> On Fri, 3 Apr 2020 15:27:13 +0200 >>> Lars-Peter Clausen <lars@metafoo.de> wrote: >>> >>>> The check for shutting down the second ADC is inverted. This causes it to >>>> be powered down when it should be enabled. As a result channels that are >>>> supposed to be handled by the second ADC return invalid conversion results. >>>> >>>> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> >>> Fixes tag? Definitely sounds like something we should be backporting! >> Fixes: bdc8cda1d010 ("iio:adc: Add Xilinx XADC driver") > For all of them? (just checking) Yes, took 6 years for somebody to notice :)
On Sun, 5 Apr 2020 14:29:00 +0200 Lars-Peter Clausen <lars@metafoo.de> wrote: > On 4/5/20 2:26 PM, Jonathan Cameron wrote: > > On Sun, 5 Apr 2020 14:13:32 +0200 > > Lars-Peter Clausen <lars@metafoo.de> wrote: > > > >> On 4/5/20 2:10 PM, Jonathan Cameron wrote: > >>> On Fri, 3 Apr 2020 15:27:13 +0200 > >>> Lars-Peter Clausen <lars@metafoo.de> wrote: > >>> > >>>> The check for shutting down the second ADC is inverted. This causes it to > >>>> be powered down when it should be enabled. As a result channels that are > >>>> supposed to be handled by the second ADC return invalid conversion results. > >>>> > >>>> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> > >>> Fixes tag? Definitely sounds like something we should be backporting! > >> Fixes: bdc8cda1d010 ("iio:adc: Add Xilinx XADC driver") > > For all of them? (just checking) > Yes, took 6 years for somebody to notice :) We've had ones that took longer :) Applied to the fixes-togreg branch of iio.git and marked for stable. Thanks, Jonathan
diff --git a/drivers/iio/adc/xilinx-xadc-core.c b/drivers/iio/adc/xilinx-xadc-core.c index 2d6505a66511..4fcf1729341f 100644 --- a/drivers/iio/adc/xilinx-xadc-core.c +++ b/drivers/iio/adc/xilinx-xadc-core.c @@ -722,13 +722,14 @@ static int xadc_power_adc_b(struct xadc *xadc, unsigned int seq_mode) { uint16_t val; + /* Powerdown the ADC-B when it is not needed. */ switch (seq_mode) { case XADC_CONF1_SEQ_SIMULTANEOUS: case XADC_CONF1_SEQ_INDEPENDENT: - val = XADC_CONF2_PD_ADC_B; + val = 0; break; default: - val = 0; + val = XADC_CONF2_PD_ADC_B; break; }
The check for shutting down the second ADC is inverted. This causes it to be powered down when it should be enabled. As a result channels that are supposed to be handled by the second ADC return invalid conversion results. Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> --- drivers/iio/adc/xilinx-xadc-core.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)