| Message ID | 20220120010246.3794962-4-robert.hancock@calian.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Xilinx AMS fixes | expand |
On Wed, 19 Jan 2022 19:02:45 -0600, Robert Hancock wrote: > Register settings used for the sequencer configuration register > were incorrect, causing some inputs to not be read properly. > > Fixes: d5c70627a794 ("iio: adc: Add Xilinx AMS driver") > Signed-off-by: Robert Hancock <robert.hancock@calian.com> > --- > drivers/iio/adc/xilinx-ams.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/iio/adc/xilinx-ams.c b/drivers/iio/adc/xilinx-ams.c > index b93864362dac..199027c93cdc 100644 > --- a/drivers/iio/adc/xilinx-ams.c > +++ b/drivers/iio/adc/xilinx-ams.c > @@ -91,8 +91,8 @@ > > #define AMS_CONF1_SEQ_MASK GENMASK(15, 12) > #define AMS_CONF1_SEQ_DEFAULT FIELD_PREP(AMS_CONF1_SEQ_MASK, 0) > -#define AMS_CONF1_SEQ_CONTINUOUS FIELD_PREP(AMS_CONF1_SEQ_MASK, 1) > -#define AMS_CONF1_SEQ_SINGLE_CHANNEL FIELD_PREP(AMS_CONF1_SEQ_MASK, 2) > +#define AMS_CONF1_SEQ_CONTINUOUS FIELD_PREP(AMS_CONF1_SEQ_MASK, 2) > +#define AMS_CONF1_SEQ_SINGLE_CHANNEL FIELD_PREP(AMS_CONF1_SEQ_MASK, 3) The TRM states that Continuous Loop Mode is 2, but Single Pass Sequence Mode is 1, not 3. Is there a reason, why you need to set both bits? Michael > > #define AMS_REG_SEQ0_MASK GENMASK(15, 0) > #define AMS_REG_SEQ2_MASK GENMASK(21, 16) > -- > 2.31.1 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >
On Tue, 2022-01-25 at 09:21 +0100, Michael Tretter wrote: > On Wed, 19 Jan 2022 19:02:45 -0600, Robert Hancock wrote: > > Register settings used for the sequencer configuration register > > were incorrect, causing some inputs to not be read properly. > > > > Fixes: d5c70627a794 ("iio: adc: Add Xilinx AMS driver") > > Signed-off-by: Robert Hancock <robert.hancock@calian.com> > > --- > > drivers/iio/adc/xilinx-ams.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/iio/adc/xilinx-ams.c b/drivers/iio/adc/xilinx-ams.c > > index b93864362dac..199027c93cdc 100644 > > --- a/drivers/iio/adc/xilinx-ams.c > > +++ b/drivers/iio/adc/xilinx-ams.c > > @@ -91,8 +91,8 @@ > > > > #define AMS_CONF1_SEQ_MASK GENMASK(15, 12) > > #define AMS_CONF1_SEQ_DEFAULT FIELD_PREP(AMS_CONF1_SEQ_MASK, > > 0) > > -#define AMS_CONF1_SEQ_CONTINUOUS FIELD_PREP(AMS_CONF1_SEQ_MASK, 1) > > -#define AMS_CONF1_SEQ_SINGLE_CHANNEL FIELD_PREP(AMS_CONF1_SEQ_MASK, > > 2) > > +#define AMS_CONF1_SEQ_CONTINUOUS FIELD_PREP(AMS_CONF1_SEQ_MASK, 2) > > +#define AMS_CONF1_SEQ_SINGLE_CHANNEL FIELD_PREP(AMS_CONF1_SEQ_MASK, > > 3) > > The TRM states that Continuous Loop Mode is 2, but Single Pass Sequence Mode > is 1, not 3. Is there a reason, why you need to set both bits? Single pass sequence mode (1) just runs the same sequence only once. To read these values it needs to switch to single channel mode (3). The register bits are defined in Table 3-8 of https://www.xilinx.com/support/documentation/user_guides/ug580-ultrascale-sysmon.pdf . > > Michael > > > > > #define AMS_REG_SEQ0_MASK GENMASK(15, 0) > > #define AMS_REG_SEQ2_MASK GENMASK(21, 16) > > -- > > 2.31.1 > > > > > > _______________________________________________ > > linux-arm-kernel mailing list > > linux-arm-kernel@lists.infradead.org > > https://urldefense.com/v3/__http://lists.infradead.org/mailman/listinfo/linux-arm-kernel__;!!IOGos0k!yGFEjSC1BL20lwurby914len0HCLXyzarwxKJP9Jx30qv_qrERSkRJUiVo_2MdusMVA$ > >
On Tue, 25 Jan 2022 16:15:05 +0000, Robert Hancock wrote: > On Tue, 2022-01-25 at 09:21 +0100, Michael Tretter wrote: > > On Wed, 19 Jan 2022 19:02:45 -0600, Robert Hancock wrote: > > > Register settings used for the sequencer configuration register > > > were incorrect, causing some inputs to not be read properly. > > > > > > Fixes: d5c70627a794 ("iio: adc: Add Xilinx AMS driver") > > > Signed-off-by: Robert Hancock <robert.hancock@calian.com> > > > --- > > > drivers/iio/adc/xilinx-ams.c | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/iio/adc/xilinx-ams.c b/drivers/iio/adc/xilinx-ams.c > > > index b93864362dac..199027c93cdc 100644 > > > --- a/drivers/iio/adc/xilinx-ams.c > > > +++ b/drivers/iio/adc/xilinx-ams.c > > > @@ -91,8 +91,8 @@ > > > > > > #define AMS_CONF1_SEQ_MASK GENMASK(15, 12) > > > #define AMS_CONF1_SEQ_DEFAULT FIELD_PREP(AMS_CONF1_SEQ_MASK, > > > 0) > > > -#define AMS_CONF1_SEQ_CONTINUOUS FIELD_PREP(AMS_CONF1_SEQ_MASK, 1) > > > -#define AMS_CONF1_SEQ_SINGLE_CHANNEL FIELD_PREP(AMS_CONF1_SEQ_MASK, > > > 2) > > > +#define AMS_CONF1_SEQ_CONTINUOUS FIELD_PREP(AMS_CONF1_SEQ_MASK, 2) > > > +#define AMS_CONF1_SEQ_SINGLE_CHANNEL FIELD_PREP(AMS_CONF1_SEQ_MASK, > > > 3) > > > > The TRM states that Continuous Loop Mode is 2, but Single Pass Sequence Mode > > is 1, not 3. Is there a reason, why you need to set both bits? > > Single pass sequence mode (1) just runs the same sequence only once. To read > these values it needs to switch to single channel mode (3). > > The register bits are defined in Table 3-8 of > https://www.xilinx.com/support/documentation/user_guides/ug580-ultrascale-sysmon.pdf > . Thanks for the clarification. Reviewed-by: Michael Tretter <m.tretter@pengutronix.de> > > > > > Michael > > > > > > > > #define AMS_REG_SEQ0_MASK GENMASK(15, 0) > > > #define AMS_REG_SEQ2_MASK GENMASK(21, 16) > > > -- > > > 2.31.1 > > > > > > > > > _______________________________________________ > > > linux-arm-kernel mailing list > > > linux-arm-kernel@lists.infradead.org > > > https://urldefense.com/v3/__http://lists.infradead.org/mailman/listinfo/linux-arm-kernel__;!!IOGos0k!yGFEjSC1BL20lwurby914len0HCLXyzarwxKJP9Jx30qv_qrERSkRJUiVo_2MdusMVA$ > > > > -- > Robert Hancock > Senior Hardware Designer, Calian Advanced Technologies > www.calian.com
On Wed, 26 Jan 2022 10:12:50 +0100 Michael Tretter <m.tretter@pengutronix.de> wrote: > On Tue, 25 Jan 2022 16:15:05 +0000, Robert Hancock wrote: > > On Tue, 2022-01-25 at 09:21 +0100, Michael Tretter wrote: > > > On Wed, 19 Jan 2022 19:02:45 -0600, Robert Hancock wrote: > > > > Register settings used for the sequencer configuration register > > > > were incorrect, causing some inputs to not be read properly. > > > > > > > > Fixes: d5c70627a794 ("iio: adc: Add Xilinx AMS driver") > > > > Signed-off-by: Robert Hancock <robert.hancock@calian.com> > > > > --- > > > > drivers/iio/adc/xilinx-ams.c | 4 ++-- > > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/drivers/iio/adc/xilinx-ams.c b/drivers/iio/adc/xilinx-ams.c > > > > index b93864362dac..199027c93cdc 100644 > > > > --- a/drivers/iio/adc/xilinx-ams.c > > > > +++ b/drivers/iio/adc/xilinx-ams.c > > > > @@ -91,8 +91,8 @@ > > > > > > > > #define AMS_CONF1_SEQ_MASK GENMASK(15, 12) > > > > #define AMS_CONF1_SEQ_DEFAULT FIELD_PREP(AMS_CONF1_SEQ_MASK, > > > > 0) > > > > -#define AMS_CONF1_SEQ_CONTINUOUS FIELD_PREP(AMS_CONF1_SEQ_MASK, 1) > > > > -#define AMS_CONF1_SEQ_SINGLE_CHANNEL FIELD_PREP(AMS_CONF1_SEQ_MASK, > > > > 2) > > > > +#define AMS_CONF1_SEQ_CONTINUOUS FIELD_PREP(AMS_CONF1_SEQ_MASK, 2) > > > > +#define AMS_CONF1_SEQ_SINGLE_CHANNEL FIELD_PREP(AMS_CONF1_SEQ_MASK, > > > > 3) > > > > > > The TRM states that Continuous Loop Mode is 2, but Single Pass Sequence Mode > > > is 1, not 3. Is there a reason, why you need to set both bits? > > > > Single pass sequence mode (1) just runs the same sequence only once. To read > > these values it needs to switch to single channel mode (3). > > > > The register bits are defined in Table 3-8 of > > https://www.xilinx.com/support/documentation/user_guides/ug580-ultrascale-sysmon.pdf > > . > > Thanks for the clarification. > > Reviewed-by: Michael Tretter <m.tretter@pengutronix.de> Applied to the fixes-togreg branch of iio.git Thanks, Jonathan
diff --git a/drivers/iio/adc/xilinx-ams.c b/drivers/iio/adc/xilinx-ams.c index b93864362dac..199027c93cdc 100644 --- a/drivers/iio/adc/xilinx-ams.c +++ b/drivers/iio/adc/xilinx-ams.c @@ -91,8 +91,8 @@ #define AMS_CONF1_SEQ_MASK GENMASK(15, 12) #define AMS_CONF1_SEQ_DEFAULT FIELD_PREP(AMS_CONF1_SEQ_MASK, 0) -#define AMS_CONF1_SEQ_CONTINUOUS FIELD_PREP(AMS_CONF1_SEQ_MASK, 1) -#define AMS_CONF1_SEQ_SINGLE_CHANNEL FIELD_PREP(AMS_CONF1_SEQ_MASK, 2) +#define AMS_CONF1_SEQ_CONTINUOUS FIELD_PREP(AMS_CONF1_SEQ_MASK, 2) +#define AMS_CONF1_SEQ_SINGLE_CHANNEL FIELD_PREP(AMS_CONF1_SEQ_MASK, 3) #define AMS_REG_SEQ0_MASK GENMASK(15, 0) #define AMS_REG_SEQ2_MASK GENMASK(21, 16)
Register settings used for the sequencer configuration register were incorrect, causing some inputs to not be read properly. Fixes: d5c70627a794 ("iio: adc: Add Xilinx AMS driver") Signed-off-by: Robert Hancock <robert.hancock@calian.com> --- drivers/iio/adc/xilinx-ams.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)