[PATCH v2 0/2] iio: dac: adi-axi-dac: fix for wrong bus read

[PATCH v2 0/2] iio: dac: adi-axi-dac: fix for wrong bus read

[Angelo Dureghello]

This patchset is intended to fix a random wrong chip ID read, or a
scratchpad test mismatch, tests done in the ad3552r-hs driver probe. The 
bus "read" operation must always check for busy flag before reading.

First patch reorganizes a bit the busy-wait polling code, second patch
fixes the wrong bus read occurence. 

NOTE: due to ongoing changes in adi-axi-dac.c, this patch is intended to be
applied after the linked "ramp generator" patch.

Link: https://lore.kernel.org/linux-iio/20250408-wip-bl-ad3552r-fixes-v4-0-b33c0264bd78@baylibre.com
Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
---
Changes in v2:
- invert patch order, fix first.
- Link to v1: https://lore.kernel.org/r/20250408-ad3552r-fix-bus-read-v1-0-37add66aeb08@baylibre.com

---
Angelo Dureghello (2):
      iio: dac: adi-axi-dac: fix bus read
      iio: dac: adi-axi-dac: use unique bus free check

 drivers/iio/dac/adi-axi-dac.c | 40 +++++++++++++++++++++++++---------------
 1 file changed, 25 insertions(+), 15 deletions(-)
---
base-commit: 6fb85f14853ddde06d57030c753168402bf69cd9
change-id: 20250408-ad3552r-fix-bus-read-1522622fbd2b

Best regards,
-- 
Angelo Dureghello <adureghello@baylibre.com>

[PATCH v2 1/2] iio: dac: adi-axi-dac: fix bus read

[Angelo Dureghello]

From: Angelo Dureghello <adureghello@baylibre.com>

Fix bus read function.

Testing the driver, on a random basis, wrong reads was detected, mainly
by a wrong DAC chip ID read at first boot.
Before reading the expected value from the AXI regmap, need always to
wait for busy flag to be cleared.

Fixes: e61d7178429a ("iio: dac: adi-axi-dac: extend features")
Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
---
 drivers/iio/dac/adi-axi-dac.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/iio/dac/adi-axi-dac.c b/drivers/iio/dac/adi-axi-dac.c
index 8ed5ad1fa24cef649056bc5f4ca80abbf28b9323..5ee077c58d7f9730aec8a9c9dff5b84108b3a47e 100644
--- a/drivers/iio/dac/adi-axi-dac.c
+++ b/drivers/iio/dac/adi-axi-dac.c
@@ -760,6 +760,7 @@ static int axi_dac_bus_reg_read(struct iio_backend *back, u32 reg, u32 *val,
 {
 	struct axi_dac_state *st = iio_backend_get_priv(back);
 	int ret;
+	u32 ival;
 
 	guard(mutex)(&st->lock);
 
@@ -772,6 +773,13 @@ static int axi_dac_bus_reg_read(struct iio_backend *back, u32 reg, u32 *val,
 	if (ret)
 		return ret;
 
+	ret = regmap_read_poll_timeout(st->regmap,
+				AXI_DAC_UI_STATUS_REG, ival,
+				FIELD_GET(AXI_DAC_UI_STATUS_IF_BUSY, ival) == 0,
+				10, 100 * KILO);
+	if (ret)
+		return ret;
+
 	return regmap_read(st->regmap, AXI_DAC_CUSTOM_RD_REG, val);
 }
 

-- 
2.49.0

Re: [PATCH v2 1/2] iio: dac: adi-axi-dac: fix bus read

[Andy Shevchenko]

On Wed, Apr 09, 2025 at 11:16:54AM +0200, Angelo Dureghello wrote:
> From: Angelo Dureghello <adureghello@baylibre.com>
> 
> Fix bus read function.
> 
> Testing the driver, on a random basis, wrong reads was detected, mainly
> by a wrong DAC chip ID read at first boot.
> Before reading the expected value from the AXI regmap, need always to
> wait for busy flag to be cleared.

...

> +	ret = regmap_read_poll_timeout(st->regmap,
> +				AXI_DAC_UI_STATUS_REG, ival,
> +				FIELD_GET(AXI_DAC_UI_STATUS_IF_BUSY, ival) == 0,
> +				10, 100 * KILO);

It's timeout, we have special constants for that, I believe you wanted to have
USEC_PER_MSEC here.

> +	if (ret)
> +		return ret;

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 1/2] iio: dac: adi-axi-dac: fix bus read

[Jonathan Cameron]

On Wed, 9 Apr 2025 19:49:25 +0300
Andy Shevchenko <andy@kernel.org> wrote:

> On Wed, Apr 09, 2025 at 11:16:54AM +0200, Angelo Dureghello wrote:
> > From: Angelo Dureghello <adureghello@baylibre.com>
> > 
> > Fix bus read function.
> > 
> > Testing the driver, on a random basis, wrong reads was detected, mainly
> > by a wrong DAC chip ID read at first boot.
> > Before reading the expected value from the AXI regmap, need always to
> > wait for busy flag to be cleared.  
> 
> ...
> 
> > +	ret = regmap_read_poll_timeout(st->regmap,
> > +				AXI_DAC_UI_STATUS_REG, ival,
> > +				FIELD_GET(AXI_DAC_UI_STATUS_IF_BUSY, ival) == 0,
> > +				10, 100 * KILO);  
> 
> It's timeout, we have special constants for that, I believe you wanted to have
> USEC_PER_MSEC here.

This is an odd corner case.  If we had a define for that 100 along the lines
of X_TIMEOUT_MSEC then I'd agree that using USEC_PER_MSEC makes complete sense.
All we have is a bare number which has no defined units.  I'd just go with 100000 and
not use the units.h defines at all.  They make sense when lots of zeros are involved
or for standard conversions, but to me not worth it here.

Jonathan

> 
> > +	if (ret)
> > +		return ret;  
> 

Re: [PATCH v2 1/2] iio: dac: adi-axi-dac: fix bus read

[Andy Shevchenko]

On Sat, Apr 12, 2025 at 4:00 PM Jonathan Cameron <jic23@kernel.org> wrote:
> On Wed, 9 Apr 2025 19:49:25 +0300
> Andy Shevchenko <andy@kernel.org> wrote:
> > On Wed, Apr 09, 2025 at 11:16:54AM +0200, Angelo Dureghello wrote:

...

> > > +   ret = regmap_read_poll_timeout(st->regmap,
> > > +                           AXI_DAC_UI_STATUS_REG, ival,
> > > +                           FIELD_GET(AXI_DAC_UI_STATUS_IF_BUSY, ival) == 0,
> > > +                           10, 100 * KILO);
> >
> > It's timeout, we have special constants for that, I believe you wanted to have
> > USEC_PER_MSEC here.
>
> This is an odd corner case.  If we had a define for that 100 along the lines
> of X_TIMEOUT_MSEC then I'd agree that using USEC_PER_MSEC makes complete sense.
> All we have is a bare number which has no defined units.  I'd just go with 100000 and
> not use the units.h defines at all.  They make sense when lots of zeros are involved
> or for standard conversions, but to me not worth it here.

I still think it's slightly better for at least two reasons:
1) the named constant makes it easier to understand the units (esp.
for the unprepared reader who doesn't know well the kernel internal
APIs);
2) educational and similar cases when somebody will copy'n'paste
(okay, copy, paste, and modify) this with thinking that this is a good
practice, which may lead to bugs when it will be more zeroes that easy
to miscount.

I prefer the constant.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v2 2/2] iio: dac: adi-axi-dac: use unique bus free check

[Angelo Dureghello]

From: Angelo Dureghello <adureghello@baylibre.com>

Use a unique function for the bus free check by polling, to reduce
duplicated code. An error is always thrown in case of timeout.

Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
---
 drivers/iio/dac/adi-axi-dac.c | 42 ++++++++++++++++++++++--------------------
 1 file changed, 22 insertions(+), 20 deletions(-)

diff --git a/drivers/iio/dac/adi-axi-dac.c b/drivers/iio/dac/adi-axi-dac.c
index 5ee077c58d7f9730aec8a9c9dff5b84108b3a47e..c90068693e9989a49e4c035eecb69606bdcbb196 100644
--- a/drivers/iio/dac/adi-axi-dac.c
+++ b/drivers/iio/dac/adi-axi-dac.c
@@ -635,15 +635,26 @@ static int axi_dac_ddr_disable(struct iio_backend *back)
 			       AXI_DAC_CNTRL_2_SDR_DDR_N);
 }
 
+static int axi_dac_wait_bus_free(struct axi_dac_state *st)
+{
+	u32 val;
+	int ret;
+
+	ret = regmap_read_poll_timeout(st->regmap, AXI_DAC_UI_STATUS_REG, val,
+		FIELD_GET(AXI_DAC_UI_STATUS_IF_BUSY, val) == -1, 10,
+		100 * KILO);
+	if (ret == -ETIMEDOUT)
+		dev_err(st->dev, "AXI bus timeout\n");
+
+	return ret;
+}
+
 static int axi_dac_data_stream_enable(struct iio_backend *back)
 {
 	struct axi_dac_state *st = iio_backend_get_priv(back);
-	int ret, val;
+	int ret;
 
-	ret = regmap_read_poll_timeout(st->regmap,
-				AXI_DAC_UI_STATUS_REG, val,
-				FIELD_GET(AXI_DAC_UI_STATUS_IF_BUSY, val) == 0,
-				10, 100 * KILO);
+	ret = axi_dac_wait_bus_free(st);
 	if (ret)
 		return ret;
 
@@ -734,12 +745,9 @@ static int __axi_dac_bus_reg_write(struct iio_backend *back, u32 reg,
 	if (ret)
 		return ret;
 
-	ret = regmap_read_poll_timeout(st->regmap,
-				AXI_DAC_UI_STATUS_REG, ival,
-				FIELD_GET(AXI_DAC_UI_STATUS_IF_BUSY, ival) == 0,
-				10, 100 * KILO);
-	if (ret == -ETIMEDOUT)
-		dev_err(st->dev, "AXI read timeout\n");
+	ret = axi_dac_wait_bus_free(st);
+	if (ret)
+		return ret;
 
 	/* Cleaning always AXI_DAC_CUSTOM_CTRL_TRANSFER_DATA */
 	return regmap_clear_bits(st->regmap, AXI_DAC_CUSTOM_CTRL_REG,
@@ -760,7 +768,6 @@ static int axi_dac_bus_reg_read(struct iio_backend *back, u32 reg, u32 *val,
 {
 	struct axi_dac_state *st = iio_backend_get_priv(back);
 	int ret;
-	u32 ival;
 
 	guard(mutex)(&st->lock);
 
@@ -773,10 +780,7 @@ static int axi_dac_bus_reg_read(struct iio_backend *back, u32 reg, u32 *val,
 	if (ret)
 		return ret;
 
-	ret = regmap_read_poll_timeout(st->regmap,
-				AXI_DAC_UI_STATUS_REG, ival,
-				FIELD_GET(AXI_DAC_UI_STATUS_IF_BUSY, ival) == 0,
-				10, 100 * KILO);
+	ret = axi_dac_wait_bus_free(st);
 	if (ret)
 		return ret;
 
@@ -787,7 +791,7 @@ static int axi_dac_bus_set_io_mode(struct iio_backend *back,
 				   enum ad3552r_io_mode mode)
 {
 	struct axi_dac_state *st = iio_backend_get_priv(back);
-	int ival, ret;
+	int ret;
 
 	if (mode > AD3552R_IO_MODE_QSPI)
 		return -EINVAL;
@@ -800,9 +804,7 @@ static int axi_dac_bus_set_io_mode(struct iio_backend *back,
 	if (ret)
 		return ret;
 
-	return regmap_read_poll_timeout(st->regmap, AXI_DAC_UI_STATUS_REG, ival,
-			FIELD_GET(AXI_DAC_UI_STATUS_IF_BUSY, ival) == 0, 10,
-			100 * KILO);
+	return axi_dac_wait_bus_free(st);
 }
 
 static void axi_dac_child_remove(void *data)

-- 
2.49.0

Re: [PATCH v2 2/2] iio: dac: adi-axi-dac: use unique bus free check

[Andy Shevchenko]

On Wed, Apr 09, 2025 at 11:16:55AM +0200, Angelo Dureghello wrote:
> From: Angelo Dureghello <adureghello@baylibre.com>
> 
> Use a unique function for the bus free check by polling, to reduce
> duplicated code. An error is always thrown in case of timeout.

...

> +static int axi_dac_wait_bus_free(struct axi_dac_state *st)
> +{
> +	u32 val;
> +	int ret;
> +
> +	ret = regmap_read_poll_timeout(st->regmap, AXI_DAC_UI_STATUS_REG, val,
> +		FIELD_GET(AXI_DAC_UI_STATUS_IF_BUSY, val) == -1, 10,
> +		100 * KILO);

Same comment as in the previous patch. Okay, it seems more than in the single
case. Perhaps to change that as well here?

> +	if (ret == -ETIMEDOUT)
> +		dev_err(st->dev, "AXI bus timeout\n");

Why do you need this? The error code will go to the user space at the end? If
yes, it will be enough to have it printed there, no?

> +	return ret;
> +}


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 2/2] iio: dac: adi-axi-dac: use unique bus free check

[Angelo Dureghello]

Hi Andy,

sorry, seen this message now only, for some reason sometimes your emails
goes to the spam. Now i marked all as "not spam", let's see.

On 09.04.2025 19:51, Andy Shevchenko wrote:
> On Wed, Apr 09, 2025 at 11:16:55AM +0200, Angelo Dureghello wrote:
> > From: Angelo Dureghello <adureghello@baylibre.com>
> > 
> > Use a unique function for the bus free check by polling, to reduce
> > duplicated code. An error is always thrown in case of timeout.
> 
> ...
> 
> > +static int axi_dac_wait_bus_free(struct axi_dac_state *st)
> > +{
> > +	u32 val;
> > +	int ret;
> > +
> > +	ret = regmap_read_poll_timeout(st->regmap, AXI_DAC_UI_STATUS_REG, val,
> > +		FIELD_GET(AXI_DAC_UI_STATUS_IF_BUSY, val) == -1, 10,
> > +		100 * KILO);
> 
> Same comment as in the previous patch. Okay, it seems more than in the single
> case. Perhaps to change that as well here?
> 

for my personal taste would not use more specific named defines here,
would not change this, in case we can send a separate patch to fix
them all. 

> > +	if (ret == -ETIMEDOUT)
> > +		dev_err(st->dev, "AXI bus timeout\n");
> 
> Why do you need this? The error code will go to the user space at the end? If
> yes, it will be enough to have it printed there, no?
> 

This warning means something very bad happen at AXI level. I never seen
this warning issued, but it may help to debug AXI/HDL issues, would not 
remove it. 

> > +	return ret;
> > +}
> 
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 
Regards,
angelo

Re: [PATCH v2 2/2] iio: dac: adi-axi-dac: use unique bus free check

[Andy Shevchenko]

On Tue, Apr 29, 2025 at 5:34 PM Angelo Dureghello
<adureghello@baylibre.com> wrote:
> On 09.04.2025 19:51, Andy Shevchenko wrote:
> > On Wed, Apr 09, 2025 at 11:16:55AM +0200, Angelo Dureghello wrote:

...

> > > +   if (ret == -ETIMEDOUT)
> > > +           dev_err(st->dev, "AXI bus timeout\n");
> >
> > Why do you need this? The error code will go to the user space at the end? If
> > yes, it will be enough to have it printed there, no?
> >
>
> This warning means something very bad happen at AXI level. I never seen
> this warning issued, but it may help to debug AXI/HDL issues, would not
> remove it.

But wouldn't user space get the error code and translate to a message if needed?

> > > +   return ret;

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 2/2] iio: dac: adi-axi-dac: use unique bus free check

[Angelo Dureghello]

Hi Andy,

On 30.04.2025 01:05, Andy Shevchenko wrote:
> On Tue, Apr 29, 2025 at 5:34 PM Angelo Dureghello
> <adureghello@baylibre.com> wrote:
> > On 09.04.2025 19:51, Andy Shevchenko wrote:
> > > On Wed, Apr 09, 2025 at 11:16:55AM +0200, Angelo Dureghello wrote:
> 
> ...
> 
> > > > +   if (ret == -ETIMEDOUT)
> > > > +           dev_err(st->dev, "AXI bus timeout\n");
> > >
> > > Why do you need this? The error code will go to the user space at the end? If
> > > yes, it will be enough to have it printed there, no?
> > >
> >
> > This warning means something very bad happen at AXI level. I never seen
> > this warning issued, but it may help to debug AXI/HDL issues, would not
> > remove it.
> 
> But wouldn't user space get the error code and translate to a message if needed?
> 

depends, bus access is done also at probe level, you would get a generic
-ETIMEOUT, then you need to put traces to understand who is causing it.
I think the message may be useful to check issues, like a buggy HDL.
Anyway, would not re-issue another patch just for this.


Regards,
angelo

> > > > +   return ret;
> 
> -- 
> With Best Regards,
> Andy Shevchenko

Re: [PATCH v2 0/2] iio: dac: adi-axi-dac: fix for wrong bus read

[Nuno Sá]

On Wed, 2025-04-09 at 11:16 +0200, Angelo Dureghello wrote:
> This patchset is intended to fix a random wrong chip ID read, or a
> scratchpad test mismatch, tests done in the ad3552r-hs driver probe. The 
> bus "read" operation must always check for busy flag before reading.
> 
> First patch reorganizes a bit the busy-wait polling code, second patch
> fixes the wrong bus read occurence. 
> 
> NOTE: due to ongoing changes in adi-axi-dac.c, this patch is intended to be
> applied after the linked "ramp generator" patch.
> 
> Link:
> https://lore.kernel.org/linux-iio/20250408-wip-bl-ad3552r-fixes-v4-0-b33c0264bd78@baylibre.com
> Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
> ---

LGTM,

Reviewed-by: Nuno Sá <nuno.sa@analog.com>

> Changes in v2:
> - invert patch order, fix first.
> - Link to v1:
> https://lore.kernel.org/r/20250408-ad3552r-fix-bus-read-v1-0-37add66aeb08@baylibre.com
> 
> ---
> Angelo Dureghello (2):
>       iio: dac: adi-axi-dac: fix bus read
>       iio: dac: adi-axi-dac: use unique bus free check
> 
>  drivers/iio/dac/adi-axi-dac.c | 40 +++++++++++++++++++++++++---------------
>  1 file changed, 25 insertions(+), 15 deletions(-)
> ---
> base-commit: 6fb85f14853ddde06d57030c753168402bf69cd9
> change-id: 20250408-ad3552r-fix-bus-read-1522622fbd2b
> 
> Best regards,

Re: [PATCH v2 0/2] iio: dac: adi-axi-dac: fix for wrong bus read

[Angelo Dureghello]

Hi all,

please let me know if you want me to use defines for this patchset,
i don't see the need honestly, but eventually it can be done in another
separate patch, for the whole adi-axi-dac file.

On 09.04.2025 14:57, Nuno Sá wrote:
> On Wed, 2025-04-09 at 11:16 +0200, Angelo Dureghello wrote:
> > This patchset is intended to fix a random wrong chip ID read, or a
> > scratchpad test mismatch, tests done in the ad3552r-hs driver probe. The 
> > bus "read" operation must always check for busy flag before reading.
> > 
> > First patch reorganizes a bit the busy-wait polling code, second patch
> > fixes the wrong bus read occurence. 
> > 
> > NOTE: due to ongoing changes in adi-axi-dac.c, this patch is intended to be
> > applied after the linked "ramp generator" patch.
> > 
> > Link:
> > https://lore.kernel.org/linux-iio/20250408-wip-bl-ad3552r-fixes-v4-0-b33c0264bd78@baylibre.com
> > Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
> > ---
> 
> LGTM,
> 
> Reviewed-by: Nuno Sá <nuno.sa@analog.com>
> 
> > Changes in v2:
> > - invert patch order, fix first.
> > - Link to v1:
> > https://lore.kernel.org/r/20250408-ad3552r-fix-bus-read-v1-0-37add66aeb08@baylibre.com
> > 
> > ---
> > Angelo Dureghello (2):
> >       iio: dac: adi-axi-dac: fix bus read
> >       iio: dac: adi-axi-dac: use unique bus free check
> > 
> >  drivers/iio/dac/adi-axi-dac.c | 40 +++++++++++++++++++++++++---------------
> >  1 file changed, 25 insertions(+), 15 deletions(-)
> > ---
> > base-commit: 6fb85f14853ddde06d57030c753168402bf69cd9
> > change-id: 20250408-ad3552r-fix-bus-read-1522622fbd2b
> > 
> > Best regards,

Regards,
angelo

Re: [PATCH v2 0/2] iio: dac: adi-axi-dac: fix for wrong bus read

[Angelo Dureghello]

Hi Jonathan,

asking info about this patch.

As explained, would not do anything else here, please let me know if it can
be accepted or how to proceed.

Thanks a lot,
angelo

On 09.04.2025 14:57, Nuno Sá wrote:
> On Wed, 2025-04-09 at 11:16 +0200, Angelo Dureghello wrote:
> > This patchset is intended to fix a random wrong chip ID read, or a
> > scratchpad test mismatch, tests done in the ad3552r-hs driver probe. The 
> > bus "read" operation must always check for busy flag before reading.
> > 
> > First patch reorganizes a bit the busy-wait polling code, second patch
> > fixes the wrong bus read occurence. 
> > 
> > NOTE: due to ongoing changes in adi-axi-dac.c, this patch is intended to be
> > applied after the linked "ramp generator" patch.
> > 
> > Link:
> > https://lore.kernel.org/linux-iio/20250408-wip-bl-ad3552r-fixes-v4-0-b33c0264bd78@baylibre.com
> > Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
> > ---
> 
> LGTM,
> 
> Reviewed-by: Nuno Sá <nuno.sa@analog.com>
> 
> > Changes in v2:
> > - invert patch order, fix first.
> > - Link to v1:
> > https://lore.kernel.org/r/20250408-ad3552r-fix-bus-read-v1-0-37add66aeb08@baylibre.com
> > 
> > ---
> > Angelo Dureghello (2):
> >       iio: dac: adi-axi-dac: fix bus read
> >       iio: dac: adi-axi-dac: use unique bus free check
> > 
> >  drivers/iio/dac/adi-axi-dac.c | 40 +++++++++++++++++++++++++---------------
> >  1 file changed, 25 insertions(+), 15 deletions(-)
> > ---
> > base-commit: 6fb85f14853ddde06d57030c753168402bf69cd9
> > change-id: 20250408-ad3552r-fix-bus-read-1522622fbd2b
> > 
> > Best regards,

Re: [PATCH v2 0/2] iio: dac: adi-axi-dac: fix for wrong bus read

[Jonathan Cameron]

On Mon, 5 May 2025 13:33:16 +0200
Angelo Dureghello <adureghello@baylibre.com> wrote:

> Hi Jonathan,
> 
> asking info about this patch.
> 
> As explained, would not do anything else here, please let me know if it can
> be accepted or how to proceed.
I've queued up patch 1 as a fix.  Patch 2 will need to wait on that.

> 
> Thanks a lot,
> angelo
> 
> On 09.04.2025 14:57, Nuno Sá wrote:
> > On Wed, 2025-04-09 at 11:16 +0200, Angelo Dureghello wrote:  
> > > This patchset is intended to fix a random wrong chip ID read, or a
> > > scratchpad test mismatch, tests done in the ad3552r-hs driver probe. The 
> > > bus "read" operation must always check for busy flag before reading.
> > > 
> > > First patch reorganizes a bit the busy-wait polling code, second patch
> > > fixes the wrong bus read occurence. 
> > > 
> > > NOTE: due to ongoing changes in adi-axi-dac.c, this patch is intended to be
> > > applied after the linked "ramp generator" patch.
> > > 
> > > Link:
> > > https://lore.kernel.org/linux-iio/20250408-wip-bl-ad3552r-fixes-v4-0-b33c0264bd78@baylibre.com
> > > Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
> > > ---  
> > 
> > LGTM,
> > 
> > Reviewed-by: Nuno Sá <nuno.sa@analog.com>
> >   
> > > Changes in v2:
> > > - invert patch order, fix first.
> > > - Link to v1:
> > > https://lore.kernel.org/r/20250408-ad3552r-fix-bus-read-v1-0-37add66aeb08@baylibre.com
> > > 
> > > ---
> > > Angelo Dureghello (2):
> > >       iio: dac: adi-axi-dac: fix bus read
> > >       iio: dac: adi-axi-dac: use unique bus free check
> > > 
> > >  drivers/iio/dac/adi-axi-dac.c | 40 +++++++++++++++++++++++++---------------
> > >  1 file changed, 25 insertions(+), 15 deletions(-)
> > > ---
> > > base-commit: 6fb85f14853ddde06d57030c753168402bf69cd9
> > > change-id: 20250408-ad3552r-fix-bus-read-1522622fbd2b
> > > 
> > > Best regards,  
> 

Re: [PATCH v2 0/2] iio: dac: adi-axi-dac: fix for wrong bus read

[Angelo Dureghello]

Hi Jonathan,

i did a mistake on the second patch 2/2.

in axi_dac_wait_bus_free()

	ret = regmap_read_poll_timeout(st->regmap, AXI_DAC_UI_STATUS_REG, val,
		FIELD_GET(AXI_DAC_UI_STATUS_IF_BUSY, val) == -1, 10,

must be

	ret = regmap_read_poll_timeout(st->regmap, AXI_DAC_UI_STATUS_REG, val,
		FIELD_GET(AXI_DAC_UI_STATUS_IF_BUSY, val) == 0, 10,

Not clear what's happen, i missed something while testing.

I apologize, not sure if it is already queued, please let me know
how to fix it.

Regards,
angelo


On 05.05.2025 13:38, Jonathan Cameron wrote:
> On Mon, 5 May 2025 13:33:16 +0200
> Angelo Dureghello <adureghello@baylibre.com> wrote:
> 
> > Hi Jonathan,
> > 
> > asking info about this patch.
> > 
> > As explained, would not do anything else here, please let me know if it can
> > be accepted or how to proceed.
> I've queued up patch 1 as a fix.  Patch 2 will need to wait on that.
> 
> > 
> > Thanks a lot,
> > angelo
> > 
> > On 09.04.2025 14:57, Nuno Sá wrote:
> > > On Wed, 2025-04-09 at 11:16 +0200, Angelo Dureghello wrote:  
> > > > This patchset is intended to fix a random wrong chip ID read, or a
> > > > scratchpad test mismatch, tests done in the ad3552r-hs driver probe. The 
> > > > bus "read" operation must always check for busy flag before reading.
> > > > 
> > > > First patch reorganizes a bit the busy-wait polling code, second patch
> > > > fixes the wrong bus read occurence. 
> > > > 
> > > > NOTE: due to ongoing changes in adi-axi-dac.c, this patch is intended to be
> > > > applied after the linked "ramp generator" patch.
> > > > 
> > > > Link:
> > > > https://lore.kernel.org/linux-iio/20250408-wip-bl-ad3552r-fixes-v4-0-b33c0264bd78@baylibre.com
> > > > Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
> > > > ---  
> > > 
> > > LGTM,
> > > 
> > > Reviewed-by: Nuno Sá <nuno.sa@analog.com>
> > >   
> > > > Changes in v2:
> > > > - invert patch order, fix first.
> > > > - Link to v1:
> > > > https://lore.kernel.org/r/20250408-ad3552r-fix-bus-read-v1-0-37add66aeb08@baylibre.com
> > > > 
> > > > ---
> > > > Angelo Dureghello (2):
> > > >       iio: dac: adi-axi-dac: fix bus read
> > > >       iio: dac: adi-axi-dac: use unique bus free check
> > > > 
> > > >  drivers/iio/dac/adi-axi-dac.c | 40 +++++++++++++++++++++++++---------------
> > > >  1 file changed, 25 insertions(+), 15 deletions(-)
> > > > ---
> > > > base-commit: 6fb85f14853ddde06d57030c753168402bf69cd9
> > > > change-id: 20250408-ad3552r-fix-bus-read-1522622fbd2b
> > > > 
> > > > Best regards,  
> > 
>