[PATCH v2] staging: iio: addac: adt7316: check SPI write errors in probe

[PATCH v2] staging: iio: addac: adt7316: check SPI write errors in probe

[Hungyu Lin]

The probe function issues three SPI writes to switch the device
from the default I2C protocol to SPI protocol, but ignores their
return values.

Switch the repeated SPI writes to a loop and return an error
if any of them fail, as failing to switch the protocol should
abort the probe.

Signed-off-by: Hungyu Lin <dennylin0707@gmail.com>
---
Changes in v2:
- Return error instead of just warning
- Clarify that repeated SPI writes are converted to a loop
---
 drivers/staging/iio/addac/adt7316-spi.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/iio/addac/adt7316-spi.c b/drivers/staging/iio/addac/adt7316-spi.c
index f91325d11394..056a4505bf30 100644
--- a/drivers/staging/iio/addac/adt7316-spi.c
+++ b/drivers/staging/iio/addac/adt7316-spi.c
@@ -98,6 +98,7 @@ static int adt7316_spi_probe(struct spi_device *spi_dev)
 		.multi_read = adt7316_spi_multi_read,
 		.multi_write = adt7316_spi_multi_write,
 	};
+	int i, ret;
 
 	/* don't exceed max specified SPI CLK frequency */
 	if (spi_dev->max_speed_hz > ADT7316_SPI_MAX_FREQ_HZ) {
@@ -107,9 +108,11 @@ static int adt7316_spi_probe(struct spi_device *spi_dev)
 	}
 
 	/* switch from default I2C protocol to SPI protocol */
-	adt7316_spi_write(spi_dev, 0, 0);
-	adt7316_spi_write(spi_dev, 0, 0);
-	adt7316_spi_write(spi_dev, 0, 0);
+	for (i = 0; i < 3; i++) {
+		ret = adt7316_spi_write(spi_dev, 0, 0);
+		if (ret)
+			return ret;
+	}
 
 	return adt7316_probe(&spi_dev->dev, &bus, spi_dev->modalias);
 }
-- 
2.34.1

Re: [PATCH v2] staging: iio: addac: adt7316: check SPI write errors in probe

[Maxwell Doose]

On Sat, May 9, 2026 at 4:03 AM Hungyu Lin <dennylin0707@gmail.com> wrote:
>
> The probe function issues three SPI writes to switch the device
> from the default I2C protocol to SPI protocol, but ignores their
> return values.
>
> Switch the repeated SPI writes to a loop and return an error
> if any of them fail, as failing to switch the protocol should
> abort the probe.
>
> Signed-off-by: Hungyu Lin <dennylin0707@gmail.com>
> ---
> Changes in v2:
> - Return error instead of just warning
> - Clarify that repeated SPI writes are converted to a loop
> ---
>  drivers/staging/iio/addac/adt7316-spi.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
[snip]

Hmmm...has this been tested? Normally patches like these don't get
merged unless tested since we need to know how the hardware would
respond when we hit an error case and quit sending data, and also some
of the timing changes introduced by writing, checking if its an error,
then jumping back. I've personally tried this patch before and it got
rejected so I'll link it below since the questions that you might get
asked will likely be similar.

Link: https://lore.kernel.org/linux-iio/20260426205039.125818-1-m32285159@gmail.com/

best regards,
max

Re: [PATCH v2] staging: iio: addac: adt7316: check SPI write errors in probe

[Denny Lin]

Hi Nuno, Maxwell,

Thanks for the feedback.

I agree that just warning is not very useful, and we should bail out on
errors if we check them. However, I currently cannot test the failure
case and I'm not sure how the device behaves if the switching sequence
is interrupted.

I've reached out to Analog Devices to clarify this. For now, I think it's
safer not to change the behavior until we have confirmation.

I'll follow up once I have more information.

Thanks,
Hungyu

On Sat, May 9, 2026 at 1:26 PM Maxwell Doose <m32285159@gmail.com> wrote:
>
> On Sat, May 9, 2026 at 4:03 AM Hungyu Lin <dennylin0707@gmail.com> wrote:
> >
> > The probe function issues three SPI writes to switch the device
> > from the default I2C protocol to SPI protocol, but ignores their
> > return values.
> >
> > Switch the repeated SPI writes to a loop and return an error
> > if any of them fail, as failing to switch the protocol should
> > abort the probe.
> >
> > Signed-off-by: Hungyu Lin <dennylin0707@gmail.com>
> > ---
> > Changes in v2:
> > - Return error instead of just warning
> > - Clarify that repeated SPI writes are converted to a loop
> > ---
> >  drivers/staging/iio/addac/adt7316-spi.c | 9 ++++++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
> >
> [snip]
>
> Hmmm...has this been tested? Normally patches like these don't get
> merged unless tested since we need to know how the hardware would
> respond when we hit an error case and quit sending data, and also some
> of the timing changes introduced by writing, checking if its an error,
> then jumping back. I've personally tried this patch before and it got
> rejected so I'll link it below since the questions that you might get
> asked will likely be similar.
>
> Link: https://lore.kernel.org/linux-iio/20260426205039.125818-1-m32285159@gmail.com/
>
> best regards,
> max

Re: [PATCH v2] staging: iio: addac: adt7316: check SPI write errors in probe

[Maxwell Doose]

On Sat, May 9, 2026 at 9:00 PM Denny Lin <dennylin0707@gmail.com> wrote:
>
> Hi Nuno, Maxwell,
>
> Thanks for the feedback.
>
> I agree that just warning is not very useful, and we should bail out on
> errors if we check them. However, I currently cannot test the failure
> case and I'm not sure how the device behaves if the switching sequence
> is interrupted.
>
> I've reached out to Analog Devices to clarify this. For now, I think it's
> safer not to change the behavior until we have confirmation.
>
> I'll follow up once I have more information.
>
> Thanks,
> Hungyu
>

Two things. One, please don't top-post, and two, from what I've heard
the switching sequence tends to be very error-prone since (I think)
we're essentially spamming the bus with spi data while we're still in
i2c mode, so we may want to ignore error codes anyways. Just a
heads-up.

best regards,
max

Re: [PATCH v2] staging: iio: addac: adt7316: check SPI write errors in probe

[Andy Shevchenko]

On Sat, May 09, 2026 at 09:03:20AM +0000, Hungyu Lin wrote:
> The probe function issues three SPI writes to switch the device
> from the default I2C protocol to SPI protocol, but ignores their
> return values.
> 
> Switch the repeated SPI writes to a loop and return an error
> if any of them fail, as failing to switch the protocol should
> abort the probe.

...

>  	/* switch from default I2C protocol to SPI protocol */
> -	adt7316_spi_write(spi_dev, 0, 0);
> -	adt7316_spi_write(spi_dev, 0, 0);
> -	adt7316_spi_write(spi_dev, 0, 0);
> +	for (i = 0; i < 3; i++) {
> +		ret = adt7316_spi_write(spi_dev, 0, 0);
> +		if (ret)
> +			return ret;
> +	}

There was an attempt in the past to do the same and it was NAKed, so is this now.
NAK.

Instead, add a better comment, if you wish, with the citing from the datasheet
to explain the magic here and why we don't check for the error codes.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2] staging: iio: addac: adt7316: check SPI write errors in probe

[Maxwell Doose]

On Sun, May 10, 2026 at 1:23 AM Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
> On Sat, May 09, 2026 at 09:03:20AM +0000, Hungyu Lin wrote:
[snip]
>
> There was an attempt in the past to do the same and it was NAKed, so is this now.
> NAK.
>
> Instead, add a better comment, if you wish, with the citing from the datasheet
> to explain the magic here and why we don't check for the error codes.
>

Agree with Andy here on the comment, I'm personally not a fan of how
undocumented this "spam data on the bus and see if it works" stuff is.
Make sure to mention:

1. We're in the transitional state between I2C and SPI
2. Facts from the data sheet about why this is what we're doing
3. Why we shouldn't check the return values from the spi_write() calls

best regards,
max




> --
> With Best Regards,
> Andy Shevchenko
>
>
>