[PATCH v2 0/2] Xilinx XADC fixes

[PATCH v2 0/2] Xilinx XADC fixes

[Robert Hancock]

Fixes for a couple of issues in the Xilinx XADC driver: one where
preconfigured temperature/voltage thresholds were being clobbered and
potentially breaking overtemperature shutdown, and another for inaccurate
temperature readings on UltraScale family devices.

Changed since v2: Updated to also remove disabling XADC alarm bits.

Robert Hancock (2):
  iio: adc: xilinx-xadc: Don't clobber preset voltage/temperature
    thresholds
  iio: adc: xilinx-xadc: Correct temperature offset/scale for UltraScale

 drivers/iio/adc/xilinx-xadc-core.c | 39 +++++++++++-------------------
 drivers/iio/adc/xilinx-xadc.h      |  2 ++
 2 files changed, 16 insertions(+), 25 deletions(-)

-- 
2.41.0

[PATCH v2 1/2] iio: adc: xilinx-xadc: Don't clobber preset voltage/temperature thresholds

[Robert Hancock]

In the probe function, the driver was reading out the thresholds already
set in the core, which can be configured by the user in the Vivado tools
when the FPGA image is built. However, it later clobbered those values
with zero or maximum values. In particular, the overtemperature shutdown
threshold register was overwritten with the max value, which effectively
prevents the FPGA from shutting down when the desired threshold was
eached, potentially risking hardware damage in that case.

Remove this code to leave the preconfigured default threshold values
intact.

The code was also disabling all alarms regardless of what enable state
they were left in by the FPGA image, including the overtemperature
shutdown feature. Leave these bits in their original state so they are
not unconditionally disabled.

Fixes: bdc8cda1d010 ("iio:adc: Add Xilinx XADC driver")
Signed-off-by: Robert Hancock <robert.hancock@calian.com>
---
 drivers/iio/adc/xilinx-xadc-core.c | 22 ----------------------
 1 file changed, 22 deletions(-)

diff --git a/drivers/iio/adc/xilinx-xadc-core.c b/drivers/iio/adc/xilinx-xadc-core.c
index dba73300f894..d4d0d184a172 100644
--- a/drivers/iio/adc/xilinx-xadc-core.c
+++ b/drivers/iio/adc/xilinx-xadc-core.c
@@ -1423,28 +1423,6 @@ static int xadc_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	/* Disable all alarms */
-	ret = xadc_update_adc_reg(xadc, XADC_REG_CONF1, XADC_CONF1_ALARM_MASK,
-				  XADC_CONF1_ALARM_MASK);
-	if (ret)
-		return ret;
-
-	/* Set thresholds to min/max */
-	for (i = 0; i < 16; i++) {
-		/*
-		 * Set max voltage threshold and both temperature thresholds to
-		 * 0xffff, min voltage threshold to 0.
-		 */
-		if (i % 8 < 4 || i == 7)
-			xadc->threshold[i] = 0xffff;
-		else
-			xadc->threshold[i] = 0;
-		ret = xadc_write_adc_reg(xadc, XADC_REG_THRESHOLD(i),
-			xadc->threshold[i]);
-		if (ret)
-			return ret;
-	}
-
 	/* Go to non-buffered mode */
 	xadc_postdisable(indio_dev);
 
-- 
2.41.0

[PATCH v2 2/2] iio: adc: xilinx-xadc: Correct temperature offset/scale for UltraScale

[Robert Hancock]

The driver was previously using offset and scale values for the
temperature sensor readings which were only valid for 7-series devices.
Add per-device-type values for offset and scale and set them appropriately
for each device type.

Note that the values used for the UltraScale family are for UltraScale+
(i.e. the SYSMONE4 primitive) using the internal reference, as that seems
to be the most common configuration and the device tree values Xilinx's
device tree generator produces don't seem to give us anything to tell us
which configuration is used. However, the differences within the UltraScale
family seem fairly minor and it's closer than using the 7-series values
instead in any case.

Fixes: c2b7720a7905 ("iio: xilinx-xadc: Add basic support for Ultrascale System Monitor")
Signed-off-by: Robert Hancock <robert.hancock@calian.com>
---
 drivers/iio/adc/xilinx-xadc-core.c | 17 ++++++++++++++---
 drivers/iio/adc/xilinx-xadc.h      |  2 ++
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/adc/xilinx-xadc-core.c b/drivers/iio/adc/xilinx-xadc-core.c
index d4d0d184a172..564c0cad0fc7 100644
--- a/drivers/iio/adc/xilinx-xadc-core.c
+++ b/drivers/iio/adc/xilinx-xadc-core.c
@@ -456,6 +456,9 @@ static const struct xadc_ops xadc_zynq_ops = {
 	.interrupt_handler = xadc_zynq_interrupt_handler,
 	.update_alarm = xadc_zynq_update_alarm,
 	.type = XADC_TYPE_S7,
+	/* Temp in C = (val * 503.975) / 2**bits - 273.15 */
+	.temp_scale = 503975,
+	.temp_offset = 273150,
 };
 
 static const unsigned int xadc_axi_reg_offsets[] = {
@@ -566,6 +569,9 @@ static const struct xadc_ops xadc_7s_axi_ops = {
 	.interrupt_handler = xadc_axi_interrupt_handler,
 	.flags = XADC_FLAGS_BUFFERED | XADC_FLAGS_IRQ_OPTIONAL,
 	.type = XADC_TYPE_S7,
+	/* Temp in C = (val * 503.975) / 2**bits - 273.15 */
+	.temp_scale = 503975,
+	.temp_offset = 273150,
 };
 
 static const struct xadc_ops xadc_us_axi_ops = {
@@ -577,6 +583,12 @@ static const struct xadc_ops xadc_us_axi_ops = {
 	.interrupt_handler = xadc_axi_interrupt_handler,
 	.flags = XADC_FLAGS_BUFFERED | XADC_FLAGS_IRQ_OPTIONAL,
 	.type = XADC_TYPE_US,
+	/**
+	 * Values below are for UltraScale+ (SYSMONE4) using internal reference.
+	 * See https://docs.xilinx.com/v/u/en-US/ug580-ultrascale-sysmon
+	 */
+	.temp_scale = 509314,
+	.temp_offset = 280231,
 };
 
 static int _xadc_update_adc_reg(struct xadc *xadc, unsigned int reg,
@@ -945,8 +957,7 @@ static int xadc_read_raw(struct iio_dev *indio_dev,
 			*val2 = bits;
 			return IIO_VAL_FRACTIONAL_LOG2;
 		case IIO_TEMP:
-			/* Temp in C = (val * 503.975) / 2**bits - 273.15 */
-			*val = 503975;
+			*val = xadc->ops->temp_scale;
 			*val2 = bits;
 			return IIO_VAL_FRACTIONAL_LOG2;
 		default:
@@ -954,7 +965,7 @@ static int xadc_read_raw(struct iio_dev *indio_dev,
 		}
 	case IIO_CHAN_INFO_OFFSET:
 		/* Only the temperature channel has an offset */
-		*val = -((273150 << bits) / 503975);
+		*val = -((xadc->ops->temp_offset << bits) / xadc->ops->temp_scale);
 		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_SAMP_FREQ:
 		ret = xadc_read_samplerate(xadc);
diff --git a/drivers/iio/adc/xilinx-xadc.h b/drivers/iio/adc/xilinx-xadc.h
index 7d78ce698967..3036f4d613ff 100644
--- a/drivers/iio/adc/xilinx-xadc.h
+++ b/drivers/iio/adc/xilinx-xadc.h
@@ -85,6 +85,8 @@ struct xadc_ops {
 
 	unsigned int flags;
 	enum xadc_type type;
+	int temp_scale;
+	int temp_offset;
 };
 
 static inline int _xadc_read_adc_reg(struct xadc *xadc, unsigned int reg,
-- 
2.41.0

Re: [PATCH v2 0/2] Xilinx XADC fixes

[Michal Simek]

+Conall

On 9/15/23 02:10, Robert Hancock wrote:
> Fixes for a couple of issues in the Xilinx XADC driver: one where
> preconfigured temperature/voltage thresholds were being clobbered and
> potentially breaking overtemperature shutdown, and another for inaccurate
> temperature readings on UltraScale family devices.
> 
> Changed since v2: Updated to also remove disabling XADC alarm bits.
> 
> Robert Hancock (2):
>    iio: adc: xilinx-xadc: Don't clobber preset voltage/temperature
>      thresholds
>    iio: adc: xilinx-xadc: Correct temperature offset/scale for UltraScale
> 
>   drivers/iio/adc/xilinx-xadc-core.c | 39 +++++++++++-------------------
>   drivers/iio/adc/xilinx-xadc.h      |  2 ++
>   2 files changed, 16 insertions(+), 25 deletions(-)
> 

Conall: Please test and review.

Thanks,
Michal

Re: [PATCH v2 0/2] Xilinx XADC fixes

[Jonathan Cameron]

On Fri, 15 Sep 2023 08:52:49 +0200
Michal Simek <michal.simek@amd.com> wrote:

> +Conall
> 
> On 9/15/23 02:10, Robert Hancock wrote:
> > Fixes for a couple of issues in the Xilinx XADC driver: one where
> > preconfigured temperature/voltage thresholds were being clobbered and
> > potentially breaking overtemperature shutdown, and another for inaccurate
> > temperature readings on UltraScale family devices.
> > 
> > Changed since v2: Updated to also remove disabling XADC alarm bits.
> > 
> > Robert Hancock (2):
> >    iio: adc: xilinx-xadc: Don't clobber preset voltage/temperature
> >      thresholds
> >    iio: adc: xilinx-xadc: Correct temperature offset/scale for UltraScale
> > 
> >   drivers/iio/adc/xilinx-xadc-core.c | 39 +++++++++++-------------------
> >   drivers/iio/adc/xilinx-xadc.h      |  2 ++
> >   2 files changed, 16 insertions(+), 25 deletions(-)
> >   
> 
> Conall: Please test and review.

I'm sitting on this one until I hear back.   No huge rush, but if you
can estimate when you'll get to this I know to leave you alone until after
that!

Jonathan

> 
> Thanks,
> Michal

Re: [PATCH v2 0/2] Xilinx XADC fixes

[Jonathan Cameron]

On Sun, 24 Sep 2023 17:32:10 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Fri, 15 Sep 2023 08:52:49 +0200
> Michal Simek <michal.simek@amd.com> wrote:
> 
> > +Conall
> > 
> > On 9/15/23 02:10, Robert Hancock wrote:  
> > > Fixes for a couple of issues in the Xilinx XADC driver: one where
> > > preconfigured temperature/voltage thresholds were being clobbered and
> > > potentially breaking overtemperature shutdown, and another for inaccurate
> > > temperature readings on UltraScale family devices.
> > > 
> > > Changed since v2: Updated to also remove disabling XADC alarm bits.
> > > 
> > > Robert Hancock (2):
> > >    iio: adc: xilinx-xadc: Don't clobber preset voltage/temperature
> > >      thresholds
> > >    iio: adc: xilinx-xadc: Correct temperature offset/scale for UltraScale
> > > 
> > >   drivers/iio/adc/xilinx-xadc-core.c | 39 +++++++++++-------------------
> > >   drivers/iio/adc/xilinx-xadc.h      |  2 ++
> > >   2 files changed, 16 insertions(+), 25 deletions(-)
> > >     
> > 
> > Conall: Please test and review.  
> 
> I'm sitting on this one until I hear back.   No huge rush, but if you
> can estimate when you'll get to this I know to leave you alone until after
> that!

I'll ask one more time, then probably just assume these are fine and apply.

So Conall, have you had a chance to look at these?

Thanks,

Jonathan

> 
> Jonathan
> 
> > 
> > Thanks,
> > Michal  
> 

RE: [PATCH v2 0/2] Xilinx XADC fixes

[O'Griofa, Conall]

[AMD Official Use Only - General]

Acked-by: O'Griofa, Conall <conall.ogriofa@amd.com>
Tested-by: O'Griofa, Conall <conall.ogriofa@amd.com>

Hi Jonathan,

Thanks for the patches, changes look good, I have tested on board and not seen any issues from my testing.

Cheers,
Conall.
> -----Original Message-----
> From: Jonathan Cameron <jic23@kernel.org>
> Sent: Tuesday, October 10, 2023 5:19 PM
> To: Simek, Michal <michal.simek@amd.com>
> Cc: Robert Hancock <robert.hancock@calian.com>; O'Griofa, Conall
> <conall.ogriofa@amd.com>; Lars-Peter Clausen <lars@metafoo.de>; linux-
> iio@vger.kernel.org; linux-arm-kernel@lists.infradead.org
> Subject: Re: [PATCH v2 0/2] Xilinx XADC fixes
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> On Sun, 24 Sep 2023 17:32:10 +0100
> Jonathan Cameron <jic23@kernel.org> wrote:
>
> > On Fri, 15 Sep 2023 08:52:49 +0200
> > Michal Simek <michal.simek@amd.com> wrote:
> >
> > > +Conall
> > >
> > > On 9/15/23 02:10, Robert Hancock wrote:
> > > > Fixes for a couple of issues in the Xilinx XADC driver: one where
> > > > preconfigured temperature/voltage thresholds were being clobbered
> > > > and potentially breaking overtemperature shutdown, and another for
> > > > inaccurate temperature readings on UltraScale family devices.
> > > >
> > > > Changed since v2: Updated to also remove disabling XADC alarm bits.
> > > >
> > > > Robert Hancock (2):
> > > >    iio: adc: xilinx-xadc: Don't clobber preset voltage/temperature
> > > >      thresholds
> > > >    iio: adc: xilinx-xadc: Correct temperature offset/scale for
> > > > UltraScale
> > > >
> > > >   drivers/iio/adc/xilinx-xadc-core.c | 39 +++++++++++-------------------
> > > >   drivers/iio/adc/xilinx-xadc.h      |  2 ++
> > > >   2 files changed, 16 insertions(+), 25 deletions(-)
> > > >
> > >
> > > Conall: Please test and review.
> >
> > I'm sitting on this one until I hear back.   No huge rush, but if you
> > can estimate when you'll get to this I know to leave you alone until
> > after that!
>
> I'll ask one more time, then probably just assume these are fine and apply.
>
> So Conall, have you had a chance to look at these?
>
> Thanks,
>
> Jonathan
>
> >
> > Jonathan
> >
> > >
> > > Thanks,
> > > Michal
> >

Re: [PATCH v2 0/2] Xilinx XADC fixes

[Jonathan Cameron]

On Thu, 12 Oct 2023 09:53:44 +0000
"O'Griofa, Conall" <conall.ogriofa@amd.com> wrote:

> [AMD Official Use Only - General]
> 
> Acked-by: O'Griofa, Conall <conall.ogriofa@amd.com>
> Tested-by: O'Griofa, Conall <conall.ogriofa@amd.com>
> 
> Hi Jonathan,
> 
> Thanks for the patches, changes look good, I have tested on board and not seen any issues from my testing.
Great. Applied to the fixes-togreg branch of iio.git.

thanks,

Jonathan

> 
> Cheers,
> Conall.
> > -----Original Message-----
> > From: Jonathan Cameron <jic23@kernel.org>
> > Sent: Tuesday, October 10, 2023 5:19 PM
> > To: Simek, Michal <michal.simek@amd.com>
> > Cc: Robert Hancock <robert.hancock@calian.com>; O'Griofa, Conall
> > <conall.ogriofa@amd.com>; Lars-Peter Clausen <lars@metafoo.de>; linux-
> > iio@vger.kernel.org; linux-arm-kernel@lists.infradead.org
> > Subject: Re: [PATCH v2 0/2] Xilinx XADC fixes
> >
> > Caution: This message originated from an External Source. Use proper caution
> > when opening attachments, clicking links, or responding.
> >
> >
> > On Sun, 24 Sep 2023 17:32:10 +0100
> > Jonathan Cameron <jic23@kernel.org> wrote:
> >  
> > > On Fri, 15 Sep 2023 08:52:49 +0200
> > > Michal Simek <michal.simek@amd.com> wrote:
> > >  
> > > > +Conall
> > > >
> > > > On 9/15/23 02:10, Robert Hancock wrote:  
> > > > > Fixes for a couple of issues in the Xilinx XADC driver: one where
> > > > > preconfigured temperature/voltage thresholds were being clobbered
> > > > > and potentially breaking overtemperature shutdown, and another for
> > > > > inaccurate temperature readings on UltraScale family devices.
> > > > >
> > > > > Changed since v2: Updated to also remove disabling XADC alarm bits.
> > > > >
> > > > > Robert Hancock (2):
> > > > >    iio: adc: xilinx-xadc: Don't clobber preset voltage/temperature
> > > > >      thresholds
> > > > >    iio: adc: xilinx-xadc: Correct temperature offset/scale for
> > > > > UltraScale
> > > > >
> > > > >   drivers/iio/adc/xilinx-xadc-core.c | 39 +++++++++++-------------------
> > > > >   drivers/iio/adc/xilinx-xadc.h      |  2 ++
> > > > >   2 files changed, 16 insertions(+), 25 deletions(-)
> > > > >  
> > > >
> > > > Conall: Please test and review.  
> > >
> > > I'm sitting on this one until I hear back.   No huge rush, but if you
> > > can estimate when you'll get to this I know to leave you alone until
> > > after that!  
> >
> > I'll ask one more time, then probably just assume these are fine and apply.
> >
> > So Conall, have you had a chance to look at these?
> >
> > Thanks,
> >
> > Jonathan
> >  
> > >
> > > Jonathan
> > >  
> > > >
> > > > Thanks,
> > > > Michal  
> > >  
>