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.0On 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
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
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
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
© 2016 - 2026 Red Hat, Inc.