* [PATCH v5 01/10] iio: dac: adi-axi-dac: fix wrong register bitfield
2024-10-08 15:43 [PATCH v5 00/10] iio: add support for the ad3552r AXI DAC IP Angelo Dureghello
@ 2024-10-08 15:43 ` Angelo Dureghello
2024-10-12 13:45 ` Jonathan Cameron
2024-10-08 15:43 ` [PATCH v5 02/10] iio: dac: adi-axi-dac: update register names Angelo Dureghello
` (8 subsequent siblings)
9 siblings, 1 reply; 26+ messages in thread
From: Angelo Dureghello @ 2024-10-08 15:43 UTC (permalink / raw)
To: Lars-Peter Clausen, Michael Hennerich, Nuno Sa, Jonathan Cameron,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Olivier Moysan
Cc: Jonathan Cameron, linux-iio, linux-kernel, devicetree, dletchner,
Mark Brown, Angelo Dureghello, stable
From: Angelo Dureghello <adureghello@baylibre.com>
Fix ADI_DAC_R1_MODE of AXI_DAC_REG_CNTRL_2.
Both generic DAC and ad3552r DAC IPs docs are reporting
bit 5 for it.
Link: https://wiki.analog.com/resources/fpga/docs/axi_dac_ip
Fixes: 4e3949a192e4 ("iio: dac: add support for AXI DAC IP core")
Cc: stable@vger.kernel.org
Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
Reviewed-by: Nuno Sa <nuno.sa@analog.com>
---
drivers/iio/dac/adi-axi-dac.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iio/dac/adi-axi-dac.c b/drivers/iio/dac/adi-axi-dac.c
index 0cb00f3bec04..b8b4171b8043 100644
--- a/drivers/iio/dac/adi-axi-dac.c
+++ b/drivers/iio/dac/adi-axi-dac.c
@@ -46,7 +46,7 @@
#define AXI_DAC_REG_CNTRL_1 0x0044
#define AXI_DAC_SYNC BIT(0)
#define AXI_DAC_REG_CNTRL_2 0x0048
-#define ADI_DAC_R1_MODE BIT(4)
+#define ADI_DAC_R1_MODE BIT(5)
#define AXI_DAC_DRP_STATUS 0x0074
#define AXI_DAC_DRP_LOCKED BIT(17)
/* DAC Channel controls */
--
2.45.0.rc1
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v5 01/10] iio: dac: adi-axi-dac: fix wrong register bitfield
2024-10-08 15:43 ` [PATCH v5 01/10] iio: dac: adi-axi-dac: fix wrong register bitfield Angelo Dureghello
@ 2024-10-12 13:45 ` Jonathan Cameron
0 siblings, 0 replies; 26+ messages in thread
From: Jonathan Cameron @ 2024-10-12 13:45 UTC (permalink / raw)
To: Angelo Dureghello
Cc: Lars-Peter Clausen, Michael Hennerich, Nuno Sa, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Olivier Moysan,
Jonathan Cameron, linux-iio, linux-kernel, devicetree, dletchner,
Mark Brown, stable
On Tue, 08 Oct 2024 17:43:33 +0200
Angelo Dureghello <adureghello@baylibre.com> wrote:
> From: Angelo Dureghello <adureghello@baylibre.com>
>
> Fix ADI_DAC_R1_MODE of AXI_DAC_REG_CNTRL_2.
>
> Both generic DAC and ad3552r DAC IPs docs are reporting
> bit 5 for it.
>
> Link: https://wiki.analog.com/resources/fpga/docs/axi_dac_ip
> Fixes: 4e3949a192e4 ("iio: dac: add support for AXI DAC IP core")
> Cc: stable@vger.kernel.org
> Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
> Reviewed-by: Nuno Sa <nuno.sa@analog.com>
Because we have a series built on top of this, I've picked it up in the
togreg branch of iio.git - not as the fixes-togreg branch.
That means it will go upstream and get pulled into stable etc next
merge window.
If anyone needs it quicker shout and I can rethink that.
> ---
> drivers/iio/dac/adi-axi-dac.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/iio/dac/adi-axi-dac.c b/drivers/iio/dac/adi-axi-dac.c
> index 0cb00f3bec04..b8b4171b8043 100644
> --- a/drivers/iio/dac/adi-axi-dac.c
> +++ b/drivers/iio/dac/adi-axi-dac.c
> @@ -46,7 +46,7 @@
> #define AXI_DAC_REG_CNTRL_1 0x0044
> #define AXI_DAC_SYNC BIT(0)
> #define AXI_DAC_REG_CNTRL_2 0x0048
> -#define ADI_DAC_R1_MODE BIT(4)
> +#define ADI_DAC_R1_MODE BIT(5)
> #define AXI_DAC_DRP_STATUS 0x0074
> #define AXI_DAC_DRP_LOCKED BIT(17)
> /* DAC Channel controls */
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v5 02/10] iio: dac: adi-axi-dac: update register names
2024-10-08 15:43 [PATCH v5 00/10] iio: add support for the ad3552r AXI DAC IP Angelo Dureghello
2024-10-08 15:43 ` [PATCH v5 01/10] iio: dac: adi-axi-dac: fix wrong register bitfield Angelo Dureghello
@ 2024-10-08 15:43 ` Angelo Dureghello
2024-10-10 12:59 ` Nuno Sá
2024-10-12 13:46 ` Jonathan Cameron
2024-10-08 15:43 ` [PATCH v5 03/10] dt-bindings: iio: dac: ad3552r: add iio backend support Angelo Dureghello
` (7 subsequent siblings)
9 siblings, 2 replies; 26+ messages in thread
From: Angelo Dureghello @ 2024-10-08 15:43 UTC (permalink / raw)
To: Lars-Peter Clausen, Michael Hennerich, Nuno Sa, Jonathan Cameron,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Olivier Moysan
Cc: Jonathan Cameron, linux-iio, linux-kernel, devicetree, dletchner,
Mark Brown, Angelo Dureghello
From: Angelo Dureghello <adureghello@baylibre.com>
Non functional, readability change.
Update register names so that register bitfields can be more easily
linked to the register name.
Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
---
drivers/iio/dac/adi-axi-dac.c | 137 +++++++++++++++++++++++-------------------
1 file changed, 74 insertions(+), 63 deletions(-)
diff --git a/drivers/iio/dac/adi-axi-dac.c b/drivers/iio/dac/adi-axi-dac.c
index b8b4171b8043..04193a98616e 100644
--- a/drivers/iio/dac/adi-axi-dac.c
+++ b/drivers/iio/dac/adi-axi-dac.c
@@ -35,35 +35,37 @@
*/
/* Base controls */
-#define AXI_DAC_REG_CONFIG 0x0c
-#define AXI_DDS_DISABLE BIT(6)
+#define AXI_DAC_CONFIG_REG 0x0c
+#define AXI_DAC_CONFIG_DDS_DISABLE BIT(6)
/* DAC controls */
-#define AXI_DAC_REG_RSTN 0x0040
-#define AXI_DAC_RSTN_CE_N BIT(2)
-#define AXI_DAC_RSTN_MMCM_RSTN BIT(1)
-#define AXI_DAC_RSTN_RSTN BIT(0)
-#define AXI_DAC_REG_CNTRL_1 0x0044
-#define AXI_DAC_SYNC BIT(0)
-#define AXI_DAC_REG_CNTRL_2 0x0048
-#define ADI_DAC_R1_MODE BIT(5)
-#define AXI_DAC_DRP_STATUS 0x0074
-#define AXI_DAC_DRP_LOCKED BIT(17)
+#define AXI_DAC_RSTN_REG 0x0040
+#define AXI_DAC_RSTN_CE_N BIT(2)
+#define AXI_DAC_RSTN_MMCM_RSTN BIT(1)
+#define AXI_DAC_RSTN_RSTN BIT(0)
+#define AXI_DAC_CNTRL_1_REG 0x0044
+#define AXI_DAC_CNTRL_1_SYNC BIT(0)
+#define AXI_DAC_CNTRL_2_REG 0x0048
+#define ADI_DAC_CNTRL_2_R1_MODE BIT(5)
+#define AXI_DAC_DRP_STATUS_REG 0x0074
+#define AXI_DAC_DRP_STATUS_DRP_LOCKED BIT(17)
+
/* DAC Channel controls */
-#define AXI_DAC_REG_CHAN_CNTRL_1(c) (0x0400 + (c) * 0x40)
-#define AXI_DAC_REG_CHAN_CNTRL_3(c) (0x0408 + (c) * 0x40)
-#define AXI_DAC_SCALE_SIGN BIT(15)
-#define AXI_DAC_SCALE_INT BIT(14)
-#define AXI_DAC_SCALE GENMASK(14, 0)
-#define AXI_DAC_REG_CHAN_CNTRL_2(c) (0x0404 + (c) * 0x40)
-#define AXI_DAC_REG_CHAN_CNTRL_4(c) (0x040c + (c) * 0x40)
-#define AXI_DAC_PHASE GENMASK(31, 16)
-#define AXI_DAC_FREQUENCY GENMASK(15, 0)
-#define AXI_DAC_REG_CHAN_CNTRL_7(c) (0x0418 + (c) * 0x40)
-#define AXI_DAC_DATA_SEL GENMASK(3, 0)
+#define AXI_DAC_CHAN_CNTRL_1_REG(c) (0x0400 + (c) * 0x40)
+#define AXI_DAC_CHAN_CNTRL_3_REG(c) (0x0408 + (c) * 0x40)
+#define AXI_DAC_CHAN_CNTRL_3_SCALE_SIGN BIT(15)
+#define AXI_DAC_CHAN_CNTRL_3_SCALE_INT BIT(14)
+#define AXI_DAC_CHAN_CNTRL_3_SCALE GENMASK(14, 0)
+#define AXI_DAC_CHAN_CNTRL_2_REG(c) (0x0404 + (c) * 0x40)
+#define AXI_DAC_CHAN_CNTRL_2_PHASE GENMASK(31, 16)
+#define AXI_DAC_CHAN_CNTRL_2_FREQUENCY GENMASK(15, 0)
+#define AXI_DAC_CHAN_CNTRL_4_REG(c) (0x040c + (c) * 0x40)
+#define AXI_DAC_CHAN_CNTRL_7_REG(c) (0x0418 + (c) * 0x40)
+#define AXI_DAC_CHAN_CNTRL_7_DATA_SEL GENMASK(3, 0)
/* 360 degrees in rad */
-#define AXI_DAC_2_PI_MEGA 6283190
+#define AXI_DAC_2_PI_MEGA 6283190
+
enum {
AXI_DAC_DATA_INTERNAL_TONE,
AXI_DAC_DATA_DMA = 2,
@@ -89,7 +91,7 @@ static int axi_dac_enable(struct iio_backend *back)
int ret;
guard(mutex)(&st->lock);
- ret = regmap_set_bits(st->regmap, AXI_DAC_REG_RSTN,
+ ret = regmap_set_bits(st->regmap, AXI_DAC_RSTN_REG,
AXI_DAC_RSTN_MMCM_RSTN);
if (ret)
return ret;
@@ -98,12 +100,14 @@ static int axi_dac_enable(struct iio_backend *back)
* designs really use it but if they don't we still get the lock bit
* set. So let's do it all the time so the code is generic.
*/
- ret = regmap_read_poll_timeout(st->regmap, AXI_DAC_DRP_STATUS, __val,
- __val & AXI_DAC_DRP_LOCKED, 100, 1000);
+ ret = regmap_read_poll_timeout(st->regmap, AXI_DAC_DRP_STATUS_REG,
+ __val,
+ __val & AXI_DAC_DRP_STATUS_DRP_LOCKED,
+ 100, 1000);
if (ret)
return ret;
- return regmap_set_bits(st->regmap, AXI_DAC_REG_RSTN,
+ return regmap_set_bits(st->regmap, AXI_DAC_RSTN_REG,
AXI_DAC_RSTN_RSTN | AXI_DAC_RSTN_MMCM_RSTN);
}
@@ -112,7 +116,7 @@ static void axi_dac_disable(struct iio_backend *back)
struct axi_dac_state *st = iio_backend_get_priv(back);
guard(mutex)(&st->lock);
- regmap_write(st->regmap, AXI_DAC_REG_RSTN, 0);
+ regmap_write(st->regmap, AXI_DAC_RSTN_REG, 0);
}
static struct iio_buffer *axi_dac_request_buffer(struct iio_backend *back,
@@ -155,15 +159,15 @@ static int __axi_dac_frequency_get(struct axi_dac_state *st, unsigned int chan,
}
if (tone_2)
- reg = AXI_DAC_REG_CHAN_CNTRL_4(chan);
+ reg = AXI_DAC_CHAN_CNTRL_4_REG(chan);
else
- reg = AXI_DAC_REG_CHAN_CNTRL_2(chan);
+ reg = AXI_DAC_CHAN_CNTRL_2_REG(chan);
ret = regmap_read(st->regmap, reg, &raw);
if (ret)
return ret;
- raw = FIELD_GET(AXI_DAC_FREQUENCY, raw);
+ raw = FIELD_GET(AXI_DAC_CHAN_CNTRL_2_FREQUENCY, raw);
*freq = DIV_ROUND_CLOSEST_ULL(raw * st->dac_clk, BIT(16));
return 0;
@@ -194,17 +198,18 @@ static int axi_dac_scale_get(struct axi_dac_state *st,
u32 reg, raw;
if (tone_2)
- reg = AXI_DAC_REG_CHAN_CNTRL_3(chan->channel);
+ reg = AXI_DAC_CHAN_CNTRL_3_REG(chan->channel);
else
- reg = AXI_DAC_REG_CHAN_CNTRL_1(chan->channel);
+ reg = AXI_DAC_CHAN_CNTRL_1_REG(chan->channel);
ret = regmap_read(st->regmap, reg, &raw);
if (ret)
return ret;
- sign = FIELD_GET(AXI_DAC_SCALE_SIGN, raw);
- raw = FIELD_GET(AXI_DAC_SCALE, raw);
- scale = DIV_ROUND_CLOSEST_ULL((u64)raw * MEGA, AXI_DAC_SCALE_INT);
+ sign = FIELD_GET(AXI_DAC_CHAN_CNTRL_3_SCALE_SIGN, raw);
+ raw = FIELD_GET(AXI_DAC_CHAN_CNTRL_3_SCALE, raw);
+ scale = DIV_ROUND_CLOSEST_ULL((u64)raw * MEGA,
+ AXI_DAC_CHAN_CNTRL_3_SCALE_INT);
vals[0] = scale / MEGA;
vals[1] = scale % MEGA;
@@ -227,15 +232,15 @@ static int axi_dac_phase_get(struct axi_dac_state *st,
int ret, vals[2];
if (tone_2)
- reg = AXI_DAC_REG_CHAN_CNTRL_4(chan->channel);
+ reg = AXI_DAC_CHAN_CNTRL_4_REG(chan->channel);
else
- reg = AXI_DAC_REG_CHAN_CNTRL_2(chan->channel);
+ reg = AXI_DAC_CHAN_CNTRL_2_REG(chan->channel);
ret = regmap_read(st->regmap, reg, &raw);
if (ret)
return ret;
- raw = FIELD_GET(AXI_DAC_PHASE, raw);
+ raw = FIELD_GET(AXI_DAC_CHAN_CNTRL_2_PHASE, raw);
phase = DIV_ROUND_CLOSEST_ULL((u64)raw * AXI_DAC_2_PI_MEGA, U16_MAX);
vals[0] = phase / MEGA;
@@ -260,18 +265,20 @@ static int __axi_dac_frequency_set(struct axi_dac_state *st, unsigned int chan,
}
if (tone_2)
- reg = AXI_DAC_REG_CHAN_CNTRL_4(chan);
+ reg = AXI_DAC_CHAN_CNTRL_4_REG(chan);
else
- reg = AXI_DAC_REG_CHAN_CNTRL_2(chan);
+ reg = AXI_DAC_CHAN_CNTRL_2_REG(chan);
raw = DIV64_U64_ROUND_CLOSEST((u64)freq * BIT(16), sample_rate);
- ret = regmap_update_bits(st->regmap, reg, AXI_DAC_FREQUENCY, raw);
+ ret = regmap_update_bits(st->regmap, reg,
+ AXI_DAC_CHAN_CNTRL_2_FREQUENCY, raw);
if (ret)
return ret;
/* synchronize channels */
- return regmap_set_bits(st->regmap, AXI_DAC_REG_CNTRL_1, AXI_DAC_SYNC);
+ return regmap_set_bits(st->regmap, AXI_DAC_CNTRL_1_REG,
+ AXI_DAC_CNTRL_1_SYNC);
}
static int axi_dac_frequency_set(struct axi_dac_state *st,
@@ -312,16 +319,16 @@ static int axi_dac_scale_set(struct axi_dac_state *st,
/* format is 1.1.14 (sign, integer and fractional bits) */
if (scale < 0) {
- raw = FIELD_PREP(AXI_DAC_SCALE_SIGN, 1);
+ raw = FIELD_PREP(AXI_DAC_CHAN_CNTRL_3_SCALE_SIGN, 1);
scale *= -1;
}
- raw |= div_u64((u64)scale * AXI_DAC_SCALE_INT, MEGA);
+ raw |= div_u64((u64)scale * AXI_DAC_CHAN_CNTRL_3_SCALE_INT, MEGA);
if (tone_2)
- reg = AXI_DAC_REG_CHAN_CNTRL_3(chan->channel);
+ reg = AXI_DAC_CHAN_CNTRL_3_REG(chan->channel);
else
- reg = AXI_DAC_REG_CHAN_CNTRL_1(chan->channel);
+ reg = AXI_DAC_CHAN_CNTRL_1_REG(chan->channel);
guard(mutex)(&st->lock);
ret = regmap_write(st->regmap, reg, raw);
@@ -329,7 +336,8 @@ static int axi_dac_scale_set(struct axi_dac_state *st,
return ret;
/* synchronize channels */
- ret = regmap_set_bits(st->regmap, AXI_DAC_REG_CNTRL_1, AXI_DAC_SYNC);
+ ret = regmap_set_bits(st->regmap, AXI_DAC_CNTRL_1_REG,
+ AXI_DAC_CNTRL_1_SYNC);
if (ret)
return ret;
@@ -355,18 +363,19 @@ static int axi_dac_phase_set(struct axi_dac_state *st,
raw = DIV_ROUND_CLOSEST_ULL((u64)phase * U16_MAX, AXI_DAC_2_PI_MEGA);
if (tone_2)
- reg = AXI_DAC_REG_CHAN_CNTRL_4(chan->channel);
+ reg = AXI_DAC_CHAN_CNTRL_4_REG(chan->channel);
else
- reg = AXI_DAC_REG_CHAN_CNTRL_2(chan->channel);
+ reg = AXI_DAC_CHAN_CNTRL_2_REG(chan->channel);
guard(mutex)(&st->lock);
- ret = regmap_update_bits(st->regmap, reg, AXI_DAC_PHASE,
- FIELD_PREP(AXI_DAC_PHASE, raw));
+ ret = regmap_update_bits(st->regmap, reg, AXI_DAC_CHAN_CNTRL_2_PHASE,
+ FIELD_PREP(AXI_DAC_CHAN_CNTRL_2_PHASE, raw));
if (ret)
return ret;
/* synchronize channels */
- ret = regmap_set_bits(st->regmap, AXI_DAC_REG_CNTRL_1, AXI_DAC_SYNC);
+ ret = regmap_set_bits(st->regmap, AXI_DAC_CNTRL_1_REG,
+ AXI_DAC_CNTRL_1_SYNC);
if (ret)
return ret;
@@ -437,7 +446,7 @@ static int axi_dac_extend_chan(struct iio_backend *back,
if (chan->type != IIO_ALTVOLTAGE)
return -EINVAL;
- if (st->reg_config & AXI_DDS_DISABLE)
+ if (st->reg_config & AXI_DAC_CONFIG_DDS_DISABLE)
/* nothing to extend */
return 0;
@@ -454,13 +463,14 @@ static int axi_dac_data_source_set(struct iio_backend *back, unsigned int chan,
switch (data) {
case IIO_BACKEND_INTERNAL_CONTINUOUS_WAVE:
return regmap_update_bits(st->regmap,
- AXI_DAC_REG_CHAN_CNTRL_7(chan),
- AXI_DAC_DATA_SEL,
+ AXI_DAC_CHAN_CNTRL_7_REG(chan),
+ AXI_DAC_CHAN_CNTRL_7_DATA_SEL,
AXI_DAC_DATA_INTERNAL_TONE);
case IIO_BACKEND_EXTERNAL:
return regmap_update_bits(st->regmap,
- AXI_DAC_REG_CHAN_CNTRL_7(chan),
- AXI_DAC_DATA_SEL, AXI_DAC_DATA_DMA);
+ AXI_DAC_CHAN_CNTRL_7_REG(chan),
+ AXI_DAC_CHAN_CNTRL_7_DATA_SEL,
+ AXI_DAC_DATA_DMA);
default:
return -EINVAL;
}
@@ -475,7 +485,7 @@ static int axi_dac_set_sample_rate(struct iio_backend *back, unsigned int chan,
if (!sample_rate)
return -EINVAL;
- if (st->reg_config & AXI_DDS_DISABLE)
+ if (st->reg_config & AXI_DAC_CONFIG_DDS_DISABLE)
/* sample_rate has no meaning if DDS is disabled */
return 0;
@@ -580,7 +590,7 @@ static int axi_dac_probe(struct platform_device *pdev)
* Force disable the core. Up to the frontend to enable us. And we can
* still read/write registers...
*/
- ret = regmap_write(st->regmap, AXI_DAC_REG_RSTN, 0);
+ ret = regmap_write(st->regmap, AXI_DAC_RSTN_REG, 0);
if (ret)
return ret;
@@ -601,7 +611,7 @@ static int axi_dac_probe(struct platform_device *pdev)
}
/* Let's get the core read only configuration */
- ret = regmap_read(st->regmap, AXI_DAC_REG_CONFIG, &st->reg_config);
+ ret = regmap_read(st->regmap, AXI_DAC_CONFIG_REG, &st->reg_config);
if (ret)
return ret;
@@ -613,7 +623,8 @@ static int axi_dac_probe(struct platform_device *pdev)
* want independent channels let's override the core's default value and
* set the R1_MODE bit.
*/
- ret = regmap_set_bits(st->regmap, AXI_DAC_REG_CNTRL_2, ADI_DAC_R1_MODE);
+ ret = regmap_set_bits(st->regmap, AXI_DAC_CNTRL_2_REG,
+ ADI_DAC_CNTRL_2_R1_MODE);
if (ret)
return ret;
--
2.45.0.rc1
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v5 02/10] iio: dac: adi-axi-dac: update register names
2024-10-08 15:43 ` [PATCH v5 02/10] iio: dac: adi-axi-dac: update register names Angelo Dureghello
@ 2024-10-10 12:59 ` Nuno Sá
2024-10-10 17:52 ` Angelo Dureghello
2024-10-12 13:46 ` Jonathan Cameron
1 sibling, 1 reply; 26+ messages in thread
From: Nuno Sá @ 2024-10-10 12:59 UTC (permalink / raw)
To: Angelo Dureghello, Lars-Peter Clausen, Michael Hennerich, Nuno Sa,
Jonathan Cameron, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Olivier Moysan
Cc: Jonathan Cameron, linux-iio, linux-kernel, devicetree, dletchner,
Mark Brown
On Tue, 2024-10-08 at 17:43 +0200, Angelo Dureghello wrote:
> From: Angelo Dureghello <adureghello@baylibre.com>
>
> Non functional, readability change.
>
> Update register names so that register bitfields can be more easily
> linked to the register name.
>
> Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
> ---
I don't fully agree that this is so much better that's worth the churn...
From a quick a look I saw (I think) some defines where _REG seems to be missing.
Those is fine to change for consistency but I don't really seeing the big
benefit in changing them all.
(Sorry for only complaining in v5 about this...)
- Nuno Sá
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v5 02/10] iio: dac: adi-axi-dac: update register names
2024-10-10 12:59 ` Nuno Sá
@ 2024-10-10 17:52 ` Angelo Dureghello
2024-10-11 6:47 ` Nuno Sá
0 siblings, 1 reply; 26+ messages in thread
From: Angelo Dureghello @ 2024-10-10 17:52 UTC (permalink / raw)
To: Nuno Sá
Cc: Lars-Peter Clausen, Michael Hennerich, Nuno Sa, Jonathan Cameron,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Olivier Moysan,
Jonathan Cameron, linux-iio, linux-kernel, devicetree, dletchner,
Mark Brown
Hi Nuno,
On 10.10.2024 14:59, Nuno Sá wrote:
> On Tue, 2024-10-08 at 17:43 +0200, Angelo Dureghello wrote:
> > From: Angelo Dureghello <adureghello@baylibre.com>
> >
> > Non functional, readability change.
> >
> > Update register names so that register bitfields can be more easily
> > linked to the register name.
> >
> > Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
> > ---
>
> I don't fully agree that this is so much better that's worth the churn...
>
> From a quick a look I saw (I think) some defines where _REG seems to be missing.
> Those is fine to change for consistency but I don't really seeing the big
> benefit in changing them all.
>
> (Sorry for only complaining in v5 about this...)
>
no problem,
the change was suggested from Jonathan, was not something i need,
let's see if he has further feedbacks, in case i can roll back
easily.
> - Nuno Sá
>
>
Regards,
angelo
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v5 02/10] iio: dac: adi-axi-dac: update register names
2024-10-10 17:52 ` Angelo Dureghello
@ 2024-10-11 6:47 ` Nuno Sá
2024-10-12 13:50 ` Jonathan Cameron
0 siblings, 1 reply; 26+ messages in thread
From: Nuno Sá @ 2024-10-11 6:47 UTC (permalink / raw)
To: Angelo Dureghello
Cc: Lars-Peter Clausen, Michael Hennerich, Nuno Sa, Jonathan Cameron,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Olivier Moysan,
Jonathan Cameron, linux-iio, linux-kernel, devicetree, dletchner,
Mark Brown
On Thu, 2024-10-10 at 19:52 +0200, Angelo Dureghello wrote:
> Hi Nuno,
>
> On 10.10.2024 14:59, Nuno Sá wrote:
> > On Tue, 2024-10-08 at 17:43 +0200, Angelo Dureghello wrote:
> > > From: Angelo Dureghello <adureghello@baylibre.com>
> > >
> > > Non functional, readability change.
> > >
> > > Update register names so that register bitfields can be more easily
> > > linked to the register name.
> > >
> > > Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
> > > ---
> >
> > I don't fully agree that this is so much better that's worth the churn...
> >
> > From a quick a look I saw (I think) some defines where _REG seems to be
> > missing.
> > Those is fine to change for consistency but I don't really seeing the big
> > benefit in changing them all.
> >
> > (Sorry for only complaining in v5 about this...)
> >
>
> no problem,
>
> the change was suggested from Jonathan, was not something i need,
> let's see if he has further feedbacks, in case i can roll back
> easily.
>
Oh, I see... Well, still don't think it's worth the churn but he has the last
word on this :)
- Nuno Sá
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v5 02/10] iio: dac: adi-axi-dac: update register names
2024-10-11 6:47 ` Nuno Sá
@ 2024-10-12 13:50 ` Jonathan Cameron
0 siblings, 0 replies; 26+ messages in thread
From: Jonathan Cameron @ 2024-10-12 13:50 UTC (permalink / raw)
To: Nuno Sá
Cc: Angelo Dureghello, Lars-Peter Clausen, Michael Hennerich, Nuno Sa,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Olivier Moysan,
Jonathan Cameron, linux-iio, linux-kernel, devicetree, dletchner,
Mark Brown
On Fri, 11 Oct 2024 08:47:00 +0200
Nuno Sá <noname.nuno@gmail.com> wrote:
> On Thu, 2024-10-10 at 19:52 +0200, Angelo Dureghello wrote:
> > Hi Nuno,
> >
> > On 10.10.2024 14:59, Nuno Sá wrote:
> > > On Tue, 2024-10-08 at 17:43 +0200, Angelo Dureghello wrote:
> > > > From: Angelo Dureghello <adureghello@baylibre.com>
> > > >
> > > > Non functional, readability change.
> > > >
> > > > Update register names so that register bitfields can be more easily
> > > > linked to the register name.
> > > >
> > > > Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
> > > > ---
> > >
> > > I don't fully agree that this is so much better that's worth the churn...
> > >
> > > From a quick a look I saw (I think) some defines where _REG seems to be
> > > missing.
> > > Those is fine to change for consistency but I don't really seeing the big
> > > benefit in changing them all.
> > >
> > > (Sorry for only complaining in v5 about this...)
> > >
> >
> > no problem,
> >
> > the change was suggested from Jonathan, was not something i need,
> > let's see if he has further feedbacks, in case i can roll back
> > easily.
> >
>
> Oh, I see... Well, still don't think it's worth the churn but he has the last
> word on this :)
For some of the fields there was no connect between the field naming and
the register whereas there was for others.
That makes it easy for bugs to hide. So on balance I do like this patch.
The disadvantage is that the fix in patch 1 will either cause us dependency
issues or have to wait for the merge window.
Given no one shouted about the bug before I guess merge window is probably
soon enough.
Jonathan
>
> - Nuno Sá
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v5 02/10] iio: dac: adi-axi-dac: update register names
2024-10-08 15:43 ` [PATCH v5 02/10] iio: dac: adi-axi-dac: update register names Angelo Dureghello
2024-10-10 12:59 ` Nuno Sá
@ 2024-10-12 13:46 ` Jonathan Cameron
1 sibling, 0 replies; 26+ messages in thread
From: Jonathan Cameron @ 2024-10-12 13:46 UTC (permalink / raw)
To: Angelo Dureghello
Cc: Lars-Peter Clausen, Michael Hennerich, Nuno Sa, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Olivier Moysan,
Jonathan Cameron, linux-iio, linux-kernel, devicetree, dletchner,
Mark Brown
On Tue, 08 Oct 2024 17:43:34 +0200
Angelo Dureghello <adureghello@baylibre.com> wrote:
> From: Angelo Dureghello <adureghello@baylibre.com>
>
> Non functional, readability change.
>
> Update register names so that register bitfields can be more easily
> linked to the register name.
>
> Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
Applied to the togreg branch of iio.git and pushed out as testing.
I'm having one of those weeks where I look at what we have in flight
and decide to pick up anything that is ready to reduce what will go
to another version.
Thanks,
Jonathan
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v5 03/10] dt-bindings: iio: dac: ad3552r: add iio backend support
2024-10-08 15:43 [PATCH v5 00/10] iio: add support for the ad3552r AXI DAC IP Angelo Dureghello
2024-10-08 15:43 ` [PATCH v5 01/10] iio: dac: adi-axi-dac: fix wrong register bitfield Angelo Dureghello
2024-10-08 15:43 ` [PATCH v5 02/10] iio: dac: adi-axi-dac: update register names Angelo Dureghello
@ 2024-10-08 15:43 ` Angelo Dureghello
2024-10-08 15:43 ` [PATCH v5 04/10] dt-bindings: iio: dac: adi-axi-dac: add ad3552r axi variant Angelo Dureghello
` (6 subsequent siblings)
9 siblings, 0 replies; 26+ messages in thread
From: Angelo Dureghello @ 2024-10-08 15:43 UTC (permalink / raw)
To: Lars-Peter Clausen, Michael Hennerich, Nuno Sa, Jonathan Cameron,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Olivier Moysan
Cc: Jonathan Cameron, linux-iio, linux-kernel, devicetree, dletchner,
Mark Brown, Angelo Dureghello, Conor Dooley
From: Angelo Dureghello <adureghello@baylibre.com>
There is a version of AXI DAC IP block (for FPGAs) that provides
a physical QSPI bus for AD3552R and similar chips, so supporting
spi-controller functionalities.
For this case, the binding is modified to include some additional
properties.
Acked-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
---
Documentation/devicetree/bindings/iio/dac/adi,ad3552r.yaml | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad3552r.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad3552r.yaml
index 41fe00034742..2d2561a52683 100644
--- a/Documentation/devicetree/bindings/iio/dac/adi,ad3552r.yaml
+++ b/Documentation/devicetree/bindings/iio/dac/adi,ad3552r.yaml
@@ -60,6 +60,12 @@ properties:
$ref: /schemas/types.yaml#/definitions/uint32
enum: [0, 1, 2, 3]
+ io-backends:
+ description: The iio backend reference.
+ Device can be optionally connected to the "axi-ad3552r IP" fpga-based
+ QSPI + DDR (Double Data Rate) controller to reach high speed transfers.
+ maxItems: 1
+
'#address-cells':
const: 1
@@ -128,6 +134,7 @@ patternProperties:
- custom-output-range-config
allOf:
+ - $ref: /schemas/spi/spi-peripheral-props.yaml#
- if:
properties:
compatible:
--
2.45.0.rc1
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v5 04/10] dt-bindings: iio: dac: adi-axi-dac: add ad3552r axi variant
2024-10-08 15:43 [PATCH v5 00/10] iio: add support for the ad3552r AXI DAC IP Angelo Dureghello
` (2 preceding siblings ...)
2024-10-08 15:43 ` [PATCH v5 03/10] dt-bindings: iio: dac: ad3552r: add iio backend support Angelo Dureghello
@ 2024-10-08 15:43 ` Angelo Dureghello
2024-10-08 17:22 ` Rob Herring (Arm)
2024-10-12 13:49 ` Jonathan Cameron
2024-10-08 15:43 ` [PATCH v5 05/10] iio: backend: extend features Angelo Dureghello
` (5 subsequent siblings)
9 siblings, 2 replies; 26+ messages in thread
From: Angelo Dureghello @ 2024-10-08 15:43 UTC (permalink / raw)
To: Lars-Peter Clausen, Michael Hennerich, Nuno Sa, Jonathan Cameron,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Olivier Moysan
Cc: Jonathan Cameron, linux-iio, linux-kernel, devicetree, dletchner,
Mark Brown, Angelo Dureghello
From: Angelo Dureghello <adureghello@baylibre.com>
Add a new compatible and related bindigns for the fpga-based
"ad3552r" AXI IP core, a variant of the generic AXI DAC IP.
The AXI "ad3552r" IP is a very similar HDL (fpga) variant of the
generic AXI "DAC" IP, intended to control ad3552r and similar chips,
mainly to reach high speed transfer rates using a QSPI DDR
(dobule-data-rate) interface.
The ad3552r device is defined as a child of the AXI DAC, that in
this case is acting as an SPI controller.
Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
---
.../devicetree/bindings/iio/dac/adi,axi-dac.yaml | 56 ++++++++++++++++++++--
1 file changed, 53 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/iio/dac/adi,axi-dac.yaml b/Documentation/devicetree/bindings/iio/dac/adi,axi-dac.yaml
index a55e9bfc66d7..2b7e16717219 100644
--- a/Documentation/devicetree/bindings/iio/dac/adi,axi-dac.yaml
+++ b/Documentation/devicetree/bindings/iio/dac/adi,axi-dac.yaml
@@ -19,11 +19,13 @@ description: |
memory via DMA into the DAC.
https://wiki.analog.com/resources/fpga/docs/axi_dac_ip
+ https://analogdevicesinc.github.io/hdl/library/axi_ad3552r/index.html
properties:
compatible:
enum:
- adi,axi-dac-9.1.b
+ - adi,axi-ad3552r
reg:
maxItems: 1
@@ -36,7 +38,14 @@ properties:
- const: tx
clocks:
- maxItems: 1
+ minItems: 1
+ maxItems: 2
+
+ clock-names:
+ minItems: 1
+ items:
+ - const: s_axi_aclk
+ - const: dac_clk
'#io-backend-cells':
const: 0
@@ -47,7 +56,16 @@ required:
- reg
- clocks
-additionalProperties: false
+allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: adi,axi-ad3552r
+ then:
+ $ref: /schemas/spi/spi-controller.yaml#
+
+unevaluatedProperties: false
examples:
- |
@@ -57,6 +75,38 @@ examples:
dmas = <&tx_dma 0>;
dma-names = "tx";
#io-backend-cells = <0>;
- clocks = <&axi_clk>;
+ clocks = <&clkc 15>;
+ clock-names = "s_axi_aclk";
+ };
+
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+ axi_dac: spi@44a70000 {
+ compatible = "adi,axi-ad3552r";
+ reg = <0x44a70000 0x1000>;
+ dmas = <&dac_tx_dma 0>;
+ dma-names = "tx";
+ #io-backend-cells = <0>;
+ clocks = <&clkc 15>, <&ref_clk>;
+ clock-names = "s_axi_aclk", "dac_clk";
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ dac@0 {
+ compatible = "adi,ad3552r";
+ reg = <0>;
+ reset-gpios = <&gpio0 92 GPIO_ACTIVE_HIGH>;
+ io-backends = <&axi_dac>;
+ spi-max-frequency = <66000000>;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ channel@0 {
+ reg = <0>;
+ adi,output-range-microvolt = <(-10000000) (10000000)>;
+ };
+ };
};
...
--
2.45.0.rc1
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v5 04/10] dt-bindings: iio: dac: adi-axi-dac: add ad3552r axi variant
2024-10-08 15:43 ` [PATCH v5 04/10] dt-bindings: iio: dac: adi-axi-dac: add ad3552r axi variant Angelo Dureghello
@ 2024-10-08 17:22 ` Rob Herring (Arm)
2024-10-12 13:43 ` Jonathan Cameron
2024-10-12 13:49 ` Jonathan Cameron
1 sibling, 1 reply; 26+ messages in thread
From: Rob Herring (Arm) @ 2024-10-08 17:22 UTC (permalink / raw)
To: Angelo Dureghello
Cc: devicetree, Krzysztof Kozlowski, Olivier Moysan, Jonathan Cameron,
Lars-Peter Clausen, Nuno Sa, Michael Hennerich, linux-kernel,
Mark Brown, dletchner, Jonathan Cameron, Conor Dooley, linux-iio
On Tue, 08 Oct 2024 17:43:36 +0200, Angelo Dureghello wrote:
> From: Angelo Dureghello <adureghello@baylibre.com>
>
> Add a new compatible and related bindigns for the fpga-based
> "ad3552r" AXI IP core, a variant of the generic AXI DAC IP.
>
> The AXI "ad3552r" IP is a very similar HDL (fpga) variant of the
> generic AXI "DAC" IP, intended to control ad3552r and similar chips,
> mainly to reach high speed transfer rates using a QSPI DDR
> (dobule-data-rate) interface.
>
> The ad3552r device is defined as a child of the AXI DAC, that in
> this case is acting as an SPI controller.
>
> Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
> ---
> .../devicetree/bindings/iio/dac/adi,axi-dac.yaml | 56 ++++++++++++++++++++--
> 1 file changed, 53 insertions(+), 3 deletions(-)
>
My bot found errors running 'make dt_binding_check' on your patch:
yamllint warnings/errors:
dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/dac/adi,axi-dac.example.dtb: dac@0: spi-max-frequency: 66000000 is greater than the maximum of 30000000
from schema $id: http://devicetree.org/schemas/iio/dac/adi,ad3552r.yaml#
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20241008-wip-bl-ad3552r-axi-v0-iio-testing-v5-4-3d410944a63d@baylibre.com
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v5 04/10] dt-bindings: iio: dac: adi-axi-dac: add ad3552r axi variant
2024-10-08 17:22 ` Rob Herring (Arm)
@ 2024-10-12 13:43 ` Jonathan Cameron
0 siblings, 0 replies; 26+ messages in thread
From: Jonathan Cameron @ 2024-10-12 13:43 UTC (permalink / raw)
To: Rob Herring (Arm)
Cc: Angelo Dureghello, devicetree, Krzysztof Kozlowski,
Olivier Moysan, Lars-Peter Clausen, Nuno Sa, Michael Hennerich,
linux-kernel, Mark Brown, dletchner, Jonathan Cameron,
Conor Dooley, linux-iio
On Tue, 08 Oct 2024 12:22:06 -0500
"Rob Herring (Arm)" <robh@kernel.org> wrote:
> On Tue, 08 Oct 2024 17:43:36 +0200, Angelo Dureghello wrote:
> > From: Angelo Dureghello <adureghello@baylibre.com>
> >
> > Add a new compatible and related bindigns for the fpga-based
> > "ad3552r" AXI IP core, a variant of the generic AXI DAC IP.
> >
> > The AXI "ad3552r" IP is a very similar HDL (fpga) variant of the
> > generic AXI "DAC" IP, intended to control ad3552r and similar chips,
> > mainly to reach high speed transfer rates using a QSPI DDR
> > (dobule-data-rate) interface.
> >
> > The ad3552r device is defined as a child of the AXI DAC, that in
> > this case is acting as an SPI controller.
> >
> > Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
> > ---
> > .../devicetree/bindings/iio/dac/adi,axi-dac.yaml | 56 ++++++++++++++++++++--
> > 1 file changed, 53 insertions(+), 3 deletions(-)
> >
>
> My bot found errors running 'make dt_binding_check' on your patch:
For the record, this is due to me having already picked up a fix on this
frequency range into the togreg branch of iio.git
So wrong base tree for the test run.
Jonathan
>
> yamllint warnings/errors:
>
> dtschema/dtc warnings/errors:
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/dac/adi,axi-dac.example.dtb: dac@0: spi-max-frequency: 66000000 is greater than the maximum of 30000000
> from schema $id: http://devicetree.org/schemas/iio/dac/adi,ad3552r.yaml#
>
> doc reference errors (make refcheckdocs):
>
> See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20241008-wip-bl-ad3552r-axi-v0-iio-testing-v5-4-3d410944a63d@baylibre.com
>
> The base for the series is generally the latest rc1. A different dependency
> should be noted in *this* patch.
>
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
>
> pip3 install dtschema --upgrade
>
> Please check and re-submit after running the above command yourself. Note
> that DT_SCHEMA_FILES can be set to your schema file to speed up checking
> your schema. However, it must be unset to test all examples with your schema.
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v5 04/10] dt-bindings: iio: dac: adi-axi-dac: add ad3552r axi variant
2024-10-08 15:43 ` [PATCH v5 04/10] dt-bindings: iio: dac: adi-axi-dac: add ad3552r axi variant Angelo Dureghello
2024-10-08 17:22 ` Rob Herring (Arm)
@ 2024-10-12 13:49 ` Jonathan Cameron
1 sibling, 0 replies; 26+ messages in thread
From: Jonathan Cameron @ 2024-10-12 13:49 UTC (permalink / raw)
To: Angelo Dureghello
Cc: Lars-Peter Clausen, Michael Hennerich, Nuno Sa, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Olivier Moysan,
Jonathan Cameron, linux-iio, linux-kernel, devicetree, dletchner,
Mark Brown
On Tue, 08 Oct 2024 17:43:36 +0200
Angelo Dureghello <adureghello@baylibre.com> wrote:
> From: Angelo Dureghello <adureghello@baylibre.com>
>
> Add a new compatible and related bindigns for the fpga-based
> "ad3552r" AXI IP core, a variant of the generic AXI DAC IP.
>
> The AXI "ad3552r" IP is a very similar HDL (fpga) variant of the
> generic AXI "DAC" IP, intended to control ad3552r and similar chips,
> mainly to reach high speed transfer rates using a QSPI DDR
> (dobule-data-rate) interface.
>
> The ad3552r device is defined as a child of the AXI DAC, that in
> this case is acting as an SPI controller.
>
Hi Angelo,
If this goes to a v6, I'd suggest adding a very brief statement that
#io-backend is present because it is possible (in theory anyway) to
use a separate controller for the control path than that used for the
datapath.
So a one line summary of that long discussion from an earlier version!
Otherwise this LGTM.
Thanks,
Jonathan
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v5 05/10] iio: backend: extend features
2024-10-08 15:43 [PATCH v5 00/10] iio: add support for the ad3552r AXI DAC IP Angelo Dureghello
` (3 preceding siblings ...)
2024-10-08 15:43 ` [PATCH v5 04/10] dt-bindings: iio: dac: adi-axi-dac: add ad3552r axi variant Angelo Dureghello
@ 2024-10-08 15:43 ` Angelo Dureghello
2024-10-10 13:06 ` Nuno Sá
2024-10-08 15:43 ` [PATCH v5 06/10] iio: dac: adi-axi-dac: " Angelo Dureghello
` (4 subsequent siblings)
9 siblings, 1 reply; 26+ messages in thread
From: Angelo Dureghello @ 2024-10-08 15:43 UTC (permalink / raw)
To: Lars-Peter Clausen, Michael Hennerich, Nuno Sa, Jonathan Cameron,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Olivier Moysan
Cc: Jonathan Cameron, linux-iio, linux-kernel, devicetree, dletchner,
Mark Brown, Angelo Dureghello
From: Angelo Dureghello <adureghello@baylibre.com>
Extend backend features with new calls needed later on this
patchset from axi version of ad3552r.
The follwoing calls are added:
iio_backend_ddr_enable
enable ddr bus transfer
iio_backend_ddr_disable
disable ddr bus transfer
iio_backend_data_stream_enable
enable data stream over bus interface
iio_backend_data_stream_disable
disable data stream over bus interface
iio_backend_data_transfer_addr
define the target register address where the DAC sample
will be written.
Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
---
drivers/iio/industrialio-backend.c | 78 ++++++++++++++++++++++++++++++++++++++
include/linux/iio/backend.h | 17 +++++++++
2 files changed, 95 insertions(+)
diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-backend.c
index 20b3b5212da7..81f3d24f0c50 100644
--- a/drivers/iio/industrialio-backend.c
+++ b/drivers/iio/industrialio-backend.c
@@ -718,6 +718,84 @@ static int __devm_iio_backend_get(struct device *dev, struct iio_backend *back)
return 0;
}
+/**
+ * iio_backend_ddr_enable - Enable interface DDR (Double Data Rate) mode
+ * @back: Backend device
+ *
+ * Enable DDR, data is generated by the IP at each front (raising and falling)
+ * of the bus clock signal.
+ *
+ * RETURNS:
+ * 0 on success, negative error number on failure.
+ */
+int iio_backend_ddr_enable(struct iio_backend *back)
+{
+ return iio_backend_op_call(back, ddr_enable);
+}
+EXPORT_SYMBOL_NS_GPL(iio_backend_ddr_enable, IIO_BACKEND);
+
+/**
+ * iio_backend_ddr_disable - Disable interface DDR (Double Data Rate) mode
+ * @back: Backend device
+ *
+ * Disable DDR, setting into SDR mode (Single Data Rate).
+ *
+ * RETURNS:
+ * 0 on success, negative error number on failure.
+ */
+int iio_backend_ddr_disable(struct iio_backend *back)
+{
+ return iio_backend_op_call(back, ddr_disable);
+}
+EXPORT_SYMBOL_NS_GPL(iio_backend_ddr_disable, IIO_BACKEND);
+
+/**
+ * iio_backend_data_stream_enable - Enable data stream
+ * @back: Backend device
+ *
+ * Enable data stream over the bus interface.
+ *
+ * RETURNS:
+ * 0 on success, negative error number on failure.
+ */
+int iio_backend_data_stream_enable(struct iio_backend *back)
+{
+ return iio_backend_op_call(back, data_stream_enable);
+}
+EXPORT_SYMBOL_NS_GPL(iio_backend_data_stream_enable, IIO_BACKEND);
+
+/**
+ * iio_backend_data_stream_disable - Disable data stream
+ * @back: Backend device
+ *
+ * Disable data stream over the bus interface.
+ *
+ * RETURNS:
+ * 0 on success, negative error number on failure.
+ */
+int iio_backend_data_stream_disable(struct iio_backend *back)
+{
+ return iio_backend_op_call(back, data_stream_disable);
+}
+EXPORT_SYMBOL_NS_GPL(iio_backend_data_stream_disable, IIO_BACKEND);
+
+/**
+ * iio_backend_data_transfer_addr - Set data address.
+ * @back: Backend device
+ * @address: Data register address
+ *
+ * Some devices may need to inform the backend about an address
+ * where to read or write the data.
+ *
+ * RETURNS:
+ * 0 on success, negative error number on failure.
+ */
+int iio_backend_data_transfer_addr(struct iio_backend *back, u32 address)
+{
+ return iio_backend_op_call(back, data_transfer_addr, address);
+}
+EXPORT_SYMBOL_NS_GPL(iio_backend_data_transfer_addr, IIO_BACKEND);
+
static struct iio_backend *__devm_iio_backend_fwnode_get(struct device *dev, const char *name,
struct fwnode_handle *fwnode)
{
diff --git a/include/linux/iio/backend.h b/include/linux/iio/backend.h
index 37d56914d485..10be00f3b120 100644
--- a/include/linux/iio/backend.h
+++ b/include/linux/iio/backend.h
@@ -14,12 +14,14 @@ struct iio_dev;
enum iio_backend_data_type {
IIO_BACKEND_TWOS_COMPLEMENT,
IIO_BACKEND_OFFSET_BINARY,
+ IIO_BACKEND_DATA_UNSIGNED,
IIO_BACKEND_DATA_TYPE_MAX
};
enum iio_backend_data_source {
IIO_BACKEND_INTERNAL_CONTINUOUS_WAVE,
IIO_BACKEND_EXTERNAL,
+ IIO_BACKEND_INTERNAL_RAMP_16BIT,
IIO_BACKEND_DATA_SOURCE_MAX
};
@@ -89,6 +91,11 @@ enum iio_backend_sample_trigger {
* @read_raw: Read a channel attribute from a backend device
* @debugfs_print_chan_status: Print channel status into a buffer.
* @debugfs_reg_access: Read or write register value of backend.
+ * @ddr_enable: Enable interface DDR (Double Data Rate) mode.
+ * @ddr_disable: Disable interface DDR (Double Data Rate) mode.
+ * @data_stream_enable: Enable data stream.
+ * @data_stream_disable: Disable data stream.
+ * @data_transfer_addr: Set data address.
**/
struct iio_backend_ops {
int (*enable)(struct iio_backend *back);
@@ -129,6 +136,11 @@ struct iio_backend_ops {
size_t len);
int (*debugfs_reg_access)(struct iio_backend *back, unsigned int reg,
unsigned int writeval, unsigned int *readval);
+ int (*ddr_enable)(struct iio_backend *back);
+ int (*ddr_disable)(struct iio_backend *back);
+ int (*data_stream_enable)(struct iio_backend *back);
+ int (*data_stream_disable)(struct iio_backend *back);
+ int (*data_transfer_addr)(struct iio_backend *back, u32 address);
};
/**
@@ -164,6 +176,11 @@ int iio_backend_data_sample_trigger(struct iio_backend *back,
int devm_iio_backend_request_buffer(struct device *dev,
struct iio_backend *back,
struct iio_dev *indio_dev);
+int iio_backend_ddr_enable(struct iio_backend *back);
+int iio_backend_ddr_disable(struct iio_backend *back);
+int iio_backend_data_stream_enable(struct iio_backend *back);
+int iio_backend_data_stream_disable(struct iio_backend *back);
+int iio_backend_data_transfer_addr(struct iio_backend *back, u32 address);
ssize_t iio_backend_ext_info_set(struct iio_dev *indio_dev, uintptr_t private,
const struct iio_chan_spec *chan,
const char *buf, size_t len);
--
2.45.0.rc1
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v5 05/10] iio: backend: extend features
2024-10-08 15:43 ` [PATCH v5 05/10] iio: backend: extend features Angelo Dureghello
@ 2024-10-10 13:06 ` Nuno Sá
0 siblings, 0 replies; 26+ messages in thread
From: Nuno Sá @ 2024-10-10 13:06 UTC (permalink / raw)
To: Angelo Dureghello, Lars-Peter Clausen, Michael Hennerich, Nuno Sa,
Jonathan Cameron, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Olivier Moysan
Cc: Jonathan Cameron, linux-iio, linux-kernel, devicetree, dletchner,
Mark Brown
On Tue, 2024-10-08 at 17:43 +0200, Angelo Dureghello wrote:
> From: Angelo Dureghello <adureghello@baylibre.com>
>
> Extend backend features with new calls needed later on this
> patchset from axi version of ad3552r.
>
> The follwoing calls are added:
>
> iio_backend_ddr_enable
> enable ddr bus transfer
> iio_backend_ddr_disable
> disable ddr bus transfer
> iio_backend_data_stream_enable
> enable data stream over bus interface
> iio_backend_data_stream_disable
> disable data stream over bus interface
> iio_backend_data_transfer_addr
> define the target register address where the DAC sample
> will be written.
>
> Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
> ---
nit: On the commit message you could properly name functions like
iio_backend_ddr_enable() etc... If you need a v6, please have that in mind.
Anyways:
Reviewed-by: Nuno Sa <nuno.sa@analog.com>
> drivers/iio/industrialio-backend.c | 78
> ++++++++++++++++++++++++++++++++++++++
> include/linux/iio/backend.h | 17 +++++++++
> 2 files changed, 95 insertions(+)
>
> diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-
> backend.c
> index 20b3b5212da7..81f3d24f0c50 100644
> --- a/drivers/iio/industrialio-backend.c
> +++ b/drivers/iio/industrialio-backend.c
> @@ -718,6 +718,84 @@ static int __devm_iio_backend_get(struct device *dev,
> struct iio_backend *back)
> return 0;
> }
>
> +/**
> + * iio_backend_ddr_enable - Enable interface DDR (Double Data Rate) mode
> + * @back: Backend device
> + *
> + * Enable DDR, data is generated by the IP at each front (raising and
> falling)
> + * of the bus clock signal.
> + *
> + * RETURNS:
> + * 0 on success, negative error number on failure.
> + */
> +int iio_backend_ddr_enable(struct iio_backend *back)
> +{
> + return iio_backend_op_call(back, ddr_enable);
> +}
> +EXPORT_SYMBOL_NS_GPL(iio_backend_ddr_enable, IIO_BACKEND);
> +
> +/**
> + * iio_backend_ddr_disable - Disable interface DDR (Double Data Rate) mode
> + * @back: Backend device
> + *
> + * Disable DDR, setting into SDR mode (Single Data Rate).
> + *
> + * RETURNS:
> + * 0 on success, negative error number on failure.
> + */
> +int iio_backend_ddr_disable(struct iio_backend *back)
> +{
> + return iio_backend_op_call(back, ddr_disable);
> +}
> +EXPORT_SYMBOL_NS_GPL(iio_backend_ddr_disable, IIO_BACKEND);
> +
> +/**
> + * iio_backend_data_stream_enable - Enable data stream
> + * @back: Backend device
> + *
> + * Enable data stream over the bus interface.
> + *
> + * RETURNS:
> + * 0 on success, negative error number on failure.
> + */
> +int iio_backend_data_stream_enable(struct iio_backend *back)
> +{
> + return iio_backend_op_call(back, data_stream_enable);
> +}
> +EXPORT_SYMBOL_NS_GPL(iio_backend_data_stream_enable, IIO_BACKEND);
> +
> +/**
> + * iio_backend_data_stream_disable - Disable data stream
> + * @back: Backend device
> + *
> + * Disable data stream over the bus interface.
> + *
> + * RETURNS:
> + * 0 on success, negative error number on failure.
> + */
> +int iio_backend_data_stream_disable(struct iio_backend *back)
> +{
> + return iio_backend_op_call(back, data_stream_disable);
> +}
> +EXPORT_SYMBOL_NS_GPL(iio_backend_data_stream_disable, IIO_BACKEND);
> +
> +/**
> + * iio_backend_data_transfer_addr - Set data address.
> + * @back: Backend device
> + * @address: Data register address
> + *
> + * Some devices may need to inform the backend about an address
> + * where to read or write the data.
> + *
> + * RETURNS:
> + * 0 on success, negative error number on failure.
> + */
> +int iio_backend_data_transfer_addr(struct iio_backend *back, u32 address)
> +{
> + return iio_backend_op_call(back, data_transfer_addr, address);
> +}
> +EXPORT_SYMBOL_NS_GPL(iio_backend_data_transfer_addr, IIO_BACKEND);
> +
> static struct iio_backend *__devm_iio_backend_fwnode_get(struct device *dev,
> const char *name,
> struct fwnode_handle
> *fwnode)
> {
> diff --git a/include/linux/iio/backend.h b/include/linux/iio/backend.h
> index 37d56914d485..10be00f3b120 100644
> --- a/include/linux/iio/backend.h
> +++ b/include/linux/iio/backend.h
> @@ -14,12 +14,14 @@ struct iio_dev;
> enum iio_backend_data_type {
> IIO_BACKEND_TWOS_COMPLEMENT,
> IIO_BACKEND_OFFSET_BINARY,
> + IIO_BACKEND_DATA_UNSIGNED,
> IIO_BACKEND_DATA_TYPE_MAX
> };
>
> enum iio_backend_data_source {
> IIO_BACKEND_INTERNAL_CONTINUOUS_WAVE,
> IIO_BACKEND_EXTERNAL,
> + IIO_BACKEND_INTERNAL_RAMP_16BIT,
> IIO_BACKEND_DATA_SOURCE_MAX
> };
>
> @@ -89,6 +91,11 @@ enum iio_backend_sample_trigger {
> * @read_raw: Read a channel attribute from a backend device
> * @debugfs_print_chan_status: Print channel status into a buffer.
> * @debugfs_reg_access: Read or write register value of backend.
> + * @ddr_enable: Enable interface DDR (Double Data Rate) mode.
> + * @ddr_disable: Disable interface DDR (Double Data Rate) mode.
> + * @data_stream_enable: Enable data stream.
> + * @data_stream_disable: Disable data stream.
> + * @data_transfer_addr: Set data address.
> **/
> struct iio_backend_ops {
> int (*enable)(struct iio_backend *back);
> @@ -129,6 +136,11 @@ struct iio_backend_ops {
> size_t len);
> int (*debugfs_reg_access)(struct iio_backend *back, unsigned int reg,
> unsigned int writeval, unsigned int
> *readval);
> + int (*ddr_enable)(struct iio_backend *back);
> + int (*ddr_disable)(struct iio_backend *back);
> + int (*data_stream_enable)(struct iio_backend *back);
> + int (*data_stream_disable)(struct iio_backend *back);
> + int (*data_transfer_addr)(struct iio_backend *back, u32 address);
> };
>
> /**
> @@ -164,6 +176,11 @@ int iio_backend_data_sample_trigger(struct iio_backend
> *back,
> int devm_iio_backend_request_buffer(struct device *dev,
> struct iio_backend *back,
> struct iio_dev *indio_dev);
> +int iio_backend_ddr_enable(struct iio_backend *back);
> +int iio_backend_ddr_disable(struct iio_backend *back);
> +int iio_backend_data_stream_enable(struct iio_backend *back);
> +int iio_backend_data_stream_disable(struct iio_backend *back);
> +int iio_backend_data_transfer_addr(struct iio_backend *back, u32 address);
> ssize_t iio_backend_ext_info_set(struct iio_dev *indio_dev, uintptr_t
> private,
> const struct iio_chan_spec *chan,
> const char *buf, size_t len);
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v5 06/10] iio: dac: adi-axi-dac: extend features
2024-10-08 15:43 [PATCH v5 00/10] iio: add support for the ad3552r AXI DAC IP Angelo Dureghello
` (4 preceding siblings ...)
2024-10-08 15:43 ` [PATCH v5 05/10] iio: backend: extend features Angelo Dureghello
@ 2024-10-08 15:43 ` Angelo Dureghello
2024-10-10 13:20 ` Nuno Sá
2024-10-08 15:43 ` [PATCH v5 07/10] iio: dac: ad3552r: changes to use FIELD_PREP Angelo Dureghello
` (3 subsequent siblings)
9 siblings, 1 reply; 26+ messages in thread
From: Angelo Dureghello @ 2024-10-08 15:43 UTC (permalink / raw)
To: Lars-Peter Clausen, Michael Hennerich, Nuno Sa, Jonathan Cameron,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Olivier Moysan
Cc: Jonathan Cameron, linux-iio, linux-kernel, devicetree, dletchner,
Mark Brown, Angelo Dureghello
From: Angelo Dureghello <adureghello@baylibre.com>
Extend AXI-DAC backend with new features required to interface
to the ad3552r DAC. Mainly, a new compatible string is added to
support the ad3552r-axi DAC IP, very similar to the generic DAC
IP but with some customizations to work with the ad3552r.
Then, a serie of generic functions has been added to match with
ad3552r needs. Function names has been kept generic as much as
possible, to allow re-utilization from other frontend drivers.
Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
---
drivers/iio/dac/adi-axi-dac.c | 285 ++++++++++++++++++++++++++++++++++++++++--
1 file changed, 274 insertions(+), 11 deletions(-)
diff --git a/drivers/iio/dac/adi-axi-dac.c b/drivers/iio/dac/adi-axi-dac.c
index 04193a98616e..e43d0ecccb50 100644
--- a/drivers/iio/dac/adi-axi-dac.c
+++ b/drivers/iio/dac/adi-axi-dac.c
@@ -46,9 +46,28 @@
#define AXI_DAC_CNTRL_1_REG 0x0044
#define AXI_DAC_CNTRL_1_SYNC BIT(0)
#define AXI_DAC_CNTRL_2_REG 0x0048
+#define AXI_DAC_CNTRL_2_SDR_DDR_N BIT(16)
+#define AXI_DAC_CNTRL_2_SYMB_8B BIT(14)
#define ADI_DAC_CNTRL_2_R1_MODE BIT(5)
+#define AXI_DAC_CNTRL_2_UNSIGNED_DATA BIT(4)
+#define AXI_DAC_STATUS_1_REG 0x0054
+#define AXI_DAC_STATUS_2_REG 0x0058
#define AXI_DAC_DRP_STATUS_REG 0x0074
#define AXI_DAC_DRP_STATUS_DRP_LOCKED BIT(17)
+#define AXI_DAC_CUSTOM_RD_REG 0x0080
+#define AXI_DAC_CUSTOM_WR_REG 0x0084
+#define AXI_DAC_CUSTOM_WR_DATA_8 GENMASK(23, 16)
+#define AXI_DAC_CUSTOM_WR_DATA_16 GENMASK(23, 8)
+#define AXI_DAC_UI_STATUS_REG 0x0088
+#define AXI_DAC_UI_STATUS_IF_BUSY BIT(4)
+#define AXI_DAC_CUSTOM_CTRL_REG 0x008C
+#define AXI_DAC_CUSTOM_CTRL_ADDRESS GENMASK(31, 24)
+#define AXI_DAC_CUSTOM_CTRL_SYNCED_TRANSFER BIT(2)
+#define AXI_DAC_CUSTOM_CTRL_STREAM BIT(1)
+#define AXI_DAC_CUSTOM_CTRL_TRANSFER_DATA BIT(0)
+
+#define AXI_DAC_STREAM_ENABLE (AXI_DAC_CUSTOM_CTRL_TRANSFER_DATA | \
+ AXI_DAC_CUSTOM_CTRL_STREAM)
/* DAC Channel controls */
#define AXI_DAC_CHAN_CNTRL_1_REG(c) (0x0400 + (c) * 0x40)
@@ -63,12 +82,27 @@
#define AXI_DAC_CHAN_CNTRL_7_REG(c) (0x0418 + (c) * 0x40)
#define AXI_DAC_CHAN_CNTRL_7_DATA_SEL GENMASK(3, 0)
+#define AXI_DAC_RD_ADDR(x) (BIT(7) | (x))
+
/* 360 degrees in rad */
#define AXI_DAC_2_PI_MEGA 6283190
enum {
AXI_DAC_DATA_INTERNAL_TONE,
AXI_DAC_DATA_DMA = 2,
+ AXI_DAC_DATA_INTERNAL_RAMP_16BIT = 11,
+};
+
+enum {
+ AXI_DAC_BUS_TYPE_NONE,
+ AXI_DAC_BUS_TYPE_DDR_QSPI,
+};
+
+struct axi_dac_info {
+ unsigned int version;
+ const struct iio_backend_info *backend_info;
+ bool bus_controller;
+ bool has_dac_clk;
};
struct axi_dac_state {
@@ -79,9 +113,12 @@ struct axi_dac_state {
* data/variables.
*/
struct mutex lock;
+ const struct axi_dac_info *info;
+ struct clk *clk;
u64 dac_clk;
u32 reg_config;
bool int_tone;
+ int dac_clk_rate;
};
static int axi_dac_enable(struct iio_backend *back)
@@ -471,6 +508,11 @@ static int axi_dac_data_source_set(struct iio_backend *back, unsigned int chan,
AXI_DAC_CHAN_CNTRL_7_REG(chan),
AXI_DAC_CHAN_CNTRL_7_DATA_SEL,
AXI_DAC_DATA_DMA);
+ case IIO_BACKEND_INTERNAL_RAMP_16BIT:
+ return regmap_update_bits(st->regmap,
+ AXI_DAC_CHAN_CNTRL_7_REG(chan),
+ AXI_DAC_CHAN_CNTRL_7_DATA_SEL,
+ AXI_DAC_DATA_INTERNAL_RAMP_16BIT);
default:
return -EINVAL;
}
@@ -528,6 +570,186 @@ static int axi_dac_reg_access(struct iio_backend *back, unsigned int reg,
return regmap_write(st->regmap, reg, writeval);
}
+static int axi_dac_ddr_enable(struct iio_backend *back)
+{
+ struct axi_dac_state *st = iio_backend_get_priv(back);
+
+ return regmap_clear_bits(st->regmap, AXI_DAC_CNTRL_2_REG,
+ AXI_DAC_CNTRL_2_SDR_DDR_N);
+}
+
+static int axi_dac_ddr_disable(struct iio_backend *back)
+{
+ struct axi_dac_state *st = iio_backend_get_priv(back);
+
+ return regmap_set_bits(st->regmap, AXI_DAC_CNTRL_2_REG,
+ AXI_DAC_CNTRL_2_SDR_DDR_N);
+}
+
+static int axi_dac_data_stream_enable(struct iio_backend *back)
+{
+ struct axi_dac_state *st = iio_backend_get_priv(back);
+
+ return regmap_set_bits(st->regmap, AXI_DAC_CUSTOM_CTRL_REG,
+ AXI_DAC_STREAM_ENABLE);
+}
+
+static int axi_dac_data_stream_disable(struct iio_backend *back)
+{
+ struct axi_dac_state *st = iio_backend_get_priv(back);
+
+ return regmap_clear_bits(st->regmap, AXI_DAC_CUSTOM_CTRL_REG,
+ AXI_DAC_STREAM_ENABLE);
+}
+
+static int axi_dac_data_transfer_addr(struct iio_backend *back, u32 address)
+{
+ struct axi_dac_state *st = iio_backend_get_priv(back);
+
+ /*
+ * Sample register address, when the DAC is configured, or stream
+ * start address when the FSM is in stream state.
+ */
+ return regmap_update_bits(st->regmap, AXI_DAC_CUSTOM_CTRL_REG,
+ AXI_DAC_CUSTOM_CTRL_ADDRESS,
+ FIELD_PREP(AXI_DAC_CUSTOM_CTRL_ADDRESS,
+ address));
+}
+
+static int axi_dac_data_format_set(struct iio_backend *back, unsigned int ch,
+ const struct iio_backend_data_fmt *data)
+{
+ struct axi_dac_state *st = iio_backend_get_priv(back);
+ int err;
+
+ switch (data->type) {
+ case IIO_BACKEND_DATA_UNSIGNED:
+ err = regmap_clear_bits(st->regmap, AXI_DAC_CNTRL_2_REG,
+ AXI_DAC_CNTRL_2_UNSIGNED_DATA);
+ if (err)
+ return err;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int axi_dac_read_raw(struct iio_backend *back,
+ struct iio_chan_spec const *chan,
+ int *val, int *val2, long mask)
+{
+ struct axi_dac_state *st = iio_backend_get_priv(back);
+ int err;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_FREQUENCY: {
+ int reg;
+
+ if (!st->info->has_dac_clk)
+ return -EOPNOTSUPP;
+
+ /*
+ * As from ad3552r AXI IP documentation,
+ * returning the SCLK depending on the stream mode.
+ */
+ err = regmap_read(st->regmap, AXI_DAC_CUSTOM_CTRL_REG, ®);
+ if (err)
+ return err;
+
+ if (reg & AXI_DAC_CUSTOM_CTRL_STREAM)
+ *val = st->dac_clk_rate / 2;
+ else
+ *val = st->dac_clk_rate / 8;
+
+ return IIO_VAL_INT;
+ }
+ default:
+ return -EINVAL;
+ }
+}
+
+static int axi_dac_bus_reg_write(struct iio_backend *back, u32 reg, u32 val,
+ size_t data_size)
+{
+ struct axi_dac_state *st = iio_backend_get_priv(back);
+ int ret;
+ u32 ival;
+
+ if (data_size == 2)
+ ival = FIELD_PREP(AXI_DAC_CUSTOM_WR_DATA_16, val);
+ else
+ ival = FIELD_PREP(AXI_DAC_CUSTOM_WR_DATA_8, val);
+
+ ret = regmap_write(st->regmap, AXI_DAC_CUSTOM_WR_REG, ival);
+ if (ret)
+ return ret;
+
+ /*
+ * Both REG_CNTRL_2 and AXI_DAC_CNTRL_DATA_WR need to know
+ * the data size. So keeping data size control here only,
+ * since data size is mandatory for the current transfer.
+ * DDR state handled separately by specific backend calls,
+ * generally all raw register writes are SDR.
+ */
+ if (data_size == 1)
+ ret = regmap_set_bits(st->regmap, AXI_DAC_CNTRL_2_REG,
+ AXI_DAC_CNTRL_2_SYMB_8B);
+ else
+ ret = regmap_clear_bits(st->regmap, AXI_DAC_CNTRL_2_REG,
+ AXI_DAC_CNTRL_2_SYMB_8B);
+ if (ret)
+ return ret;
+
+ ret = regmap_update_bits(st->regmap, AXI_DAC_CUSTOM_CTRL_REG,
+ AXI_DAC_CUSTOM_CTRL_ADDRESS,
+ FIELD_PREP(AXI_DAC_CUSTOM_CTRL_ADDRESS, reg));
+ if (ret)
+ return ret;
+
+ ret = regmap_update_bits(st->regmap, AXI_DAC_CUSTOM_CTRL_REG,
+ AXI_DAC_CUSTOM_CTRL_TRANSFER_DATA,
+ AXI_DAC_CUSTOM_CTRL_TRANSFER_DATA);
+ if (ret)
+ return ret;
+
+ ret = regmap_read_poll_timeout(st->regmap,
+ AXI_DAC_CUSTOM_CTRL_REG, ival,
+ ival & AXI_DAC_CUSTOM_CTRL_TRANSFER_DATA,
+ 10, 100 * KILO);
+ if (ret)
+ return ret;
+
+ return regmap_clear_bits(st->regmap, AXI_DAC_CUSTOM_CTRL_REG,
+ AXI_DAC_CUSTOM_CTRL_TRANSFER_DATA);
+}
+
+static int axi_dac_bus_reg_read(struct iio_backend *back, u32 reg, u32 *val,
+ size_t data_size)
+{
+ struct axi_dac_state *st = iio_backend_get_priv(back);
+ int ret;
+ u32 ival;
+
+ /*
+ * SPI, we write with read flag, then we read just at the AXI
+ * io address space to get data read.
+ */
+ ret = axi_dac_bus_reg_write(back, AXI_DAC_RD_ADDR(reg), 0, data_size);
+ 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) !=
+ AXI_DAC_UI_STATUS_IF_BUSY,
+ 10, 100);
+ if (ret)
+ return ret;
+
+ return regmap_read(st->regmap, AXI_DAC_CUSTOM_RD_REG, val);
+}
+
static const struct iio_backend_ops axi_dac_generic_ops = {
.enable = axi_dac_enable,
.disable = axi_dac_disable,
@@ -541,11 +763,29 @@ static const struct iio_backend_ops axi_dac_generic_ops = {
.debugfs_reg_access = iio_backend_debugfs_ptr(axi_dac_reg_access),
};
+static const struct iio_backend_ops axi_ad3552r_ops = {
+ .enable = axi_dac_enable,
+ .read_raw = axi_dac_read_raw,
+ .request_buffer = axi_dac_request_buffer,
+ .data_source_set = axi_dac_data_source_set,
+ .ddr_enable = axi_dac_ddr_enable,
+ .ddr_disable = axi_dac_ddr_disable,
+ .data_stream_enable = axi_dac_data_stream_enable,
+ .data_stream_disable = axi_dac_data_stream_disable,
+ .data_format_set = axi_dac_data_format_set,
+ .data_transfer_addr = axi_dac_data_transfer_addr,
+};
+
static const struct iio_backend_info axi_dac_generic = {
.name = "axi-dac",
.ops = &axi_dac_generic_ops,
};
+static const struct iio_backend_info axi_ad3552r = {
+ .name = "axi-ad3552r",
+ .ops = &axi_ad3552r_ops,
+};
+
static const struct regmap_config axi_dac_regmap_config = {
.val_bits = 32,
.reg_bits = 32,
@@ -555,7 +795,6 @@ static const struct regmap_config axi_dac_regmap_config = {
static int axi_dac_probe(struct platform_device *pdev)
{
- const unsigned int *expected_ver;
struct axi_dac_state *st;
void __iomem *base;
unsigned int ver;
@@ -566,15 +805,26 @@ static int axi_dac_probe(struct platform_device *pdev)
if (!st)
return -ENOMEM;
- expected_ver = device_get_match_data(&pdev->dev);
- if (!expected_ver)
+ st->info = device_get_match_data(&pdev->dev);
+ if (!st->info)
return -ENODEV;
- clk = devm_clk_get_enabled(&pdev->dev, NULL);
+ clk = devm_clk_get_enabled(&pdev->dev, "s_axi_aclk");
if (IS_ERR(clk))
return dev_err_probe(&pdev->dev, PTR_ERR(clk),
"failed to get clock\n");
+ if (st->info->has_dac_clk) {
+ struct clk *dac_clk;
+
+ dac_clk = devm_clk_get_enabled(&pdev->dev, "dac_clk");
+ if (IS_ERR(dac_clk))
+ return dev_err_probe(&pdev->dev, PTR_ERR(dac_clk),
+ "failed to get dac_clk clock\n");
+
+ st->dac_clk_rate = clk_get_rate(dac_clk);
+ }
+
base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(base))
return PTR_ERR(base);
@@ -598,12 +848,13 @@ static int axi_dac_probe(struct platform_device *pdev)
if (ret)
return ret;
- if (ADI_AXI_PCORE_VER_MAJOR(ver) != ADI_AXI_PCORE_VER_MAJOR(*expected_ver)) {
+ if (ADI_AXI_PCORE_VER_MAJOR(ver) !=
+ ADI_AXI_PCORE_VER_MAJOR(st->info->version)) {
dev_err(&pdev->dev,
"Major version mismatch. Expected %d.%.2d.%c, Reported %d.%.2d.%c\n",
- ADI_AXI_PCORE_VER_MAJOR(*expected_ver),
- ADI_AXI_PCORE_VER_MINOR(*expected_ver),
- ADI_AXI_PCORE_VER_PATCH(*expected_ver),
+ ADI_AXI_PCORE_VER_MAJOR(st->info->version),
+ ADI_AXI_PCORE_VER_MINOR(st->info->version),
+ ADI_AXI_PCORE_VER_PATCH(st->info->version),
ADI_AXI_PCORE_VER_MAJOR(ver),
ADI_AXI_PCORE_VER_MINOR(ver),
ADI_AXI_PCORE_VER_PATCH(ver));
@@ -629,7 +880,8 @@ static int axi_dac_probe(struct platform_device *pdev)
return ret;
mutex_init(&st->lock);
- ret = devm_iio_backend_register(&pdev->dev, &axi_dac_generic, st);
+
+ ret = devm_iio_backend_register(&pdev->dev, st->info->backend_info, st);
if (ret)
return dev_err_probe(&pdev->dev, ret,
"failed to register iio backend\n");
@@ -642,10 +894,21 @@ static int axi_dac_probe(struct platform_device *pdev)
return 0;
}
-static unsigned int axi_dac_9_1_b_info = ADI_AXI_PCORE_VER(9, 1, 'b');
+static const struct axi_dac_info dac_generic = {
+ .version = ADI_AXI_PCORE_VER(9, 1, 'b'),
+ .backend_info = &axi_dac_generic,
+};
+
+static const struct axi_dac_info dac_ad3552r = {
+ .version = ADI_AXI_PCORE_VER(9, 1, 'b'),
+ .backend_info = &axi_ad3552r,
+ .bus_controller = true,
+ .has_dac_clk = true,
+};
static const struct of_device_id axi_dac_of_match[] = {
- { .compatible = "adi,axi-dac-9.1.b", .data = &axi_dac_9_1_b_info },
+ { .compatible = "adi,axi-dac-9.1.b", .data = &dac_generic },
+ { .compatible = "adi,axi-ad3552r", .data = &dac_ad3552r },
{}
};
MODULE_DEVICE_TABLE(of, axi_dac_of_match);
--
2.45.0.rc1
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v5 06/10] iio: dac: adi-axi-dac: extend features
2024-10-08 15:43 ` [PATCH v5 06/10] iio: dac: adi-axi-dac: " Angelo Dureghello
@ 2024-10-10 13:20 ` Nuno Sá
2024-10-11 8:06 ` Angelo Dureghello
0 siblings, 1 reply; 26+ messages in thread
From: Nuno Sá @ 2024-10-10 13:20 UTC (permalink / raw)
To: Angelo Dureghello, Lars-Peter Clausen, Michael Hennerich, Nuno Sa,
Jonathan Cameron, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Olivier Moysan
Cc: Jonathan Cameron, linux-iio, linux-kernel, devicetree, dletchner,
Mark Brown
On Tue, 2024-10-08 at 17:43 +0200, Angelo Dureghello wrote:
> From: Angelo Dureghello <adureghello@baylibre.com>
>
> Extend AXI-DAC backend with new features required to interface
> to the ad3552r DAC. Mainly, a new compatible string is added to
> support the ad3552r-axi DAC IP, very similar to the generic DAC
> IP but with some customizations to work with the ad3552r.
>
> Then, a serie of generic functions has been added to match with
> ad3552r needs. Function names has been kept generic as much as
> possible, to allow re-utilization from other frontend drivers.
>
> Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
> ---
Just some minor stuff. Not seeing any fundamental issue with this patch.
> drivers/iio/dac/adi-axi-dac.c | 285 ++++++++++++++++++++++++++++++++++++++++-
> -
> 1 file changed, 274 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/iio/dac/adi-axi-dac.c b/drivers/iio/dac/adi-axi-dac.c
> index 04193a98616e..e43d0ecccb50 100644
> --- a/drivers/iio/dac/adi-axi-dac.c
> +++ b/drivers/iio/dac/adi-axi-dac.c
> @@ -46,9 +46,28 @@
> #define AXI_DAC_CNTRL_1_REG 0x0044
> #define AXI_DAC_CNTRL_1_SYNC BIT(0)
> #define AXI_DAC_CNTRL_2_REG 0x0048
> +#define AXI_DAC_CNTRL_2_SDR_DDR_N BIT(16)
> +#define AXI_DAC_CNTRL_2_SYMB_8B BIT(14)
> #define ADI_DAC_CNTRL_2_R1_MODE BIT(5)
> +#define AXI_DAC_CNTRL_2_UNSIGNED_DATA BIT(4)
> +#define AXI_DAC_STATUS_1_REG 0x0054
> +#define AXI_DAC_STATUS_2_REG 0x0058
> #define AXI_DAC_DRP_STATUS_REG 0x0074
> #define AXI_DAC_DRP_STATUS_DRP_LOCKED BIT(17)
> +#define AXI_DAC_CUSTOM_RD_REG 0x0080
> +#define AXI_DAC_CUSTOM_WR_REG 0x0084
> +#define AXI_DAC_CUSTOM_WR_DATA_8 GENMASK(23, 16)
> +#define AXI_DAC_CUSTOM_WR_DATA_16 GENMASK(23, 8)
> +#define AXI_DAC_UI_STATUS_REG 0x0088
> +#define AXI_DAC_UI_STATUS_IF_BUSY BIT(4)
> +#define AXI_DAC_CUSTOM_CTRL_REG 0x008C
> +#define AXI_DAC_CUSTOM_CTRL_ADDRESS GENMASK(31, 24)
> +#define AXI_DAC_CUSTOM_CTRL_SYNCED_TRANSFER BIT(2)
> +#define AXI_DAC_CUSTOM_CTRL_STREAM BIT(1)
> +#define AXI_DAC_CUSTOM_CTRL_TRANSFER_DATA BIT(0)
> +
> +#define
> AXI_DAC_STREAM_ENABLE (AXI_DAC_CUSTOM_CTRL_TRANSFER_DATA | \
> + AXI_DAC_CUSTOM_CTRL_STREAM)
>
> /* DAC Channel controls */
> #define AXI_DAC_CHAN_CNTRL_1_REG(c) (0x0400 + (c) * 0x40)
> @@ -63,12 +82,27 @@
> #define AXI_DAC_CHAN_CNTRL_7_REG(c) (0x0418 + (c) * 0x40)
> #define AXI_DAC_CHAN_CNTRL_7_DATA_SEL GENMASK(3, 0)
>
> +#define AXI_DAC_RD_ADDR(x) (BIT(7) | (x))
> +
> /* 360 degrees in rad */
> #define AXI_DAC_2_PI_MEGA 6283190
>
> enum {
> AXI_DAC_DATA_INTERNAL_TONE,
> AXI_DAC_DATA_DMA = 2,
> + AXI_DAC_DATA_INTERNAL_RAMP_16BIT = 11,
> +};
> +
> +enum {
> + AXI_DAC_BUS_TYPE_NONE,
> + AXI_DAC_BUS_TYPE_DDR_QSPI,
> +};
Do we still need the above?
> +
> +struct axi_dac_info {
> + unsigned int version;
> + const struct iio_backend_info *backend_info;
> + bool bus_controller;
> + bool has_dac_clk;
> };
>
> struct axi_dac_state {
> @@ -79,9 +113,12 @@ struct axi_dac_state {
> * data/variables.
> */
> struct mutex lock;
> + const struct axi_dac_info *info;
> + struct clk *clk;
Is it used?
> u64 dac_clk;
> u32 reg_config;
> bool int_tone;
> + int dac_clk_rate;
> };
>
> static int axi_dac_enable(struct iio_backend *back)
> @@ -471,6 +508,11 @@ static int axi_dac_data_source_set(struct iio_backend
> *back, unsigned int chan,
> AXI_DAC_CHAN_CNTRL_7_REG(chan),
> AXI_DAC_CHAN_CNTRL_7_DATA_SEL,
> AXI_DAC_DATA_DMA);
> + case IIO_BACKEND_INTERNAL_RAMP_16BIT:
> + return regmap_update_bits(st->regmap,
> + AXI_DAC_CHAN_CNTRL_7_REG(chan),
> + AXI_DAC_CHAN_CNTRL_7_DATA_SEL,
> + AXI_DAC_DATA_INTERNAL_RAMP_16BIT);
> default:
> return -EINVAL;
> }
> @@ -528,6 +570,186 @@ static int axi_dac_reg_access(struct iio_backend *back,
> unsigned int reg,
> return regmap_write(st->regmap, reg, writeval);
> }
>
> +static int axi_dac_ddr_enable(struct iio_backend *back)
> +{
> + struct axi_dac_state *st = iio_backend_get_priv(back);
> +
> + return regmap_clear_bits(st->regmap, AXI_DAC_CNTRL_2_REG,
> + AXI_DAC_CNTRL_2_SDR_DDR_N);
> +}
> +
> +static int axi_dac_ddr_disable(struct iio_backend *back)
> +{
> + struct axi_dac_state *st = iio_backend_get_priv(back);
> +
> + return regmap_set_bits(st->regmap, AXI_DAC_CNTRL_2_REG,
> + AXI_DAC_CNTRL_2_SDR_DDR_N);
> +}
> +
> +static int axi_dac_data_stream_enable(struct iio_backend *back)
> +{
> + struct axi_dac_state *st = iio_backend_get_priv(back);
> +
> + return regmap_set_bits(st->regmap, AXI_DAC_CUSTOM_CTRL_REG,
> + AXI_DAC_STREAM_ENABLE);
> +}
> +
> +static int axi_dac_data_stream_disable(struct iio_backend *back)
> +{
> + struct axi_dac_state *st = iio_backend_get_priv(back);
> +
> + return regmap_clear_bits(st->regmap, AXI_DAC_CUSTOM_CTRL_REG,
> + AXI_DAC_STREAM_ENABLE);
> +}
> +
> +static int axi_dac_data_transfer_addr(struct iio_backend *back, u32 address)
> +{
> + struct axi_dac_state *st = iio_backend_get_priv(back);
> +
We could add some validation for address with FIELD_MAX()
> + /*
> + * Sample register address, when the DAC is configured, or stream
> + * start address when the FSM is in stream state.
> + */
> + return regmap_update_bits(st->regmap, AXI_DAC_CUSTOM_CTRL_REG,
> + AXI_DAC_CUSTOM_CTRL_ADDRESS,
> + FIELD_PREP(AXI_DAC_CUSTOM_CTRL_ADDRESS,
> + address));
> +}
> +
> +static int axi_dac_data_format_set(struct iio_backend *back, unsigned int ch,
> + const struct iio_backend_data_fmt *data)
> +{
> + struct axi_dac_state *st = iio_backend_get_priv(back);
> + int err;
>
Not sure if I mentioned it already but please use 'ret' so it's consistent with
what we have. But for this function, see below
> + switch (data->type) {
> + case IIO_BACKEND_DATA_UNSIGNED:
> + err = regmap_clear_bits(st->regmap, AXI_DAC_CNTRL_2_REG,
> + AXI_DAC_CNTRL_2_UNSIGNED_DATA);
> + if (err)
> + return err;
> + break;
return regmap_clear_bits();
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;
remove the above...
> +}
> +
> +static int axi_dac_read_raw(struct iio_backend *back,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2, long mask)
> +{
> + struct axi_dac_state *st = iio_backend_get_priv(back);
> + int err;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_FREQUENCY: {
> + int reg;
I would just declare it at top scope and remove {}. Otherwise...
> +
> + if (!st->info->has_dac_clk)
> + return -EOPNOTSUPP;
> +
> + /*
> + * As from ad3552r AXI IP documentation,
> + * returning the SCLK depending on the stream mode.
> + */
> + err = regmap_read(st->regmap, AXI_DAC_CUSTOM_CTRL_REG, ®);
> + if (err)
> + return err;
> +
> + if (reg & AXI_DAC_CUSTOM_CTRL_STREAM)
> + *val = st->dac_clk_rate / 2;
> + else
> + *val = st->dac_clk_rate / 8;
> +
> + return IIO_VAL_INT;
> + }
... does not look good
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int axi_dac_bus_reg_write(struct iio_backend *back, u32 reg, u32 val,
> + size_t data_size)
> +{
> + struct axi_dac_state *st = iio_backend_get_priv(back);
> + int ret;
> + u32 ival;
> +
> + if (data_size == 2)
nit: sizeof(u16)
> + ival = FIELD_PREP(AXI_DAC_CUSTOM_WR_DATA_16, val);
> + else
> + ival = FIELD_PREP(AXI_DAC_CUSTOM_WR_DATA_8, val);
> +
> + ret = regmap_write(st->regmap, AXI_DAC_CUSTOM_WR_REG, ival);
> + if (ret)
> + return ret;
> +
> + /*
> + * Both REG_CNTRL_2 and AXI_DAC_CNTRL_DATA_WR need to know
> + * the data size. So keeping data size control here only,
> + * since data size is mandatory for the current transfer.
> + * DDR state handled separately by specific backend calls,
> + * generally all raw register writes are SDR.
> + */
> + if (data_size == 1)
> + ret = regmap_set_bits(st->regmap, AXI_DAC_CNTRL_2_REG,
> + AXI_DAC_CNTRL_2_SYMB_8B);
> + else
> + ret = regmap_clear_bits(st->regmap, AXI_DAC_CNTRL_2_REG,
> + AXI_DAC_CNTRL_2_SYMB_8B);
> + if (ret)
> + return ret;
> +
> + ret = regmap_update_bits(st->regmap, AXI_DAC_CUSTOM_CTRL_REG,
> + AXI_DAC_CUSTOM_CTRL_ADDRESS,
> + FIELD_PREP(AXI_DAC_CUSTOM_CTRL_ADDRESS,
> reg));
> + if (ret)
> + return ret;
> +
> + ret = regmap_update_bits(st->regmap, AXI_DAC_CUSTOM_CTRL_REG,
> + AXI_DAC_CUSTOM_CTRL_TRANSFER_DATA,
> + AXI_DAC_CUSTOM_CTRL_TRANSFER_DATA);
> + if (ret)
> + return ret;
> +
> + ret = regmap_read_poll_timeout(st->regmap,
> + AXI_DAC_CUSTOM_CTRL_REG, ival,
> + ival &
> AXI_DAC_CUSTOM_CTRL_TRANSFER_DATA,
> + 10, 100 * KILO);
> + if (ret)
> + return ret;
> +
> + return regmap_clear_bits(st->regmap, AXI_DAC_CUSTOM_CTRL_REG,
> + AXI_DAC_CUSTOM_CTRL_TRANSFER_DATA);
> +}
> +
> +static int axi_dac_bus_reg_read(struct iio_backend *back, u32 reg, u32 *val,
> + size_t data_size)
> +{
> + struct axi_dac_state *st = iio_backend_get_priv(back);
> + int ret;
> + u32 ival;
> +
> + /*
> + * SPI, we write with read flag, then we read just at the AXI
> + * io address space to get data read.
> + */
> + ret = axi_dac_bus_reg_write(back, AXI_DAC_RD_ADDR(reg), 0,
> data_size);
> + 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) !=
> + AXI_DAC_UI_STATUS_IF_BUSY,
> + 10, 100);
> + if (ret)
> + return ret;
> +
> + return regmap_read(st->regmap, AXI_DAC_CUSTOM_RD_REG, val);
> +}
> +
> static const struct iio_backend_ops axi_dac_generic_ops = {
> .enable = axi_dac_enable,
> .disable = axi_dac_disable,
> @@ -541,11 +763,29 @@ static const struct iio_backend_ops axi_dac_generic_ops
> = {
> .debugfs_reg_access = iio_backend_debugfs_ptr(axi_dac_reg_access),
> };
>
> +static const struct iio_backend_ops axi_ad3552r_ops = {
> + .enable = axi_dac_enable,
> + .read_raw = axi_dac_read_raw,
> + .request_buffer = axi_dac_request_buffer,
> + .data_source_set = axi_dac_data_source_set,
> + .ddr_enable = axi_dac_ddr_enable,
> + .ddr_disable = axi_dac_ddr_disable,
> + .data_stream_enable = axi_dac_data_stream_enable,
> + .data_stream_disable = axi_dac_data_stream_disable,
> + .data_format_set = axi_dac_data_format_set,
> + .data_transfer_addr = axi_dac_data_transfer_addr,
> +};
> +
> static const struct iio_backend_info axi_dac_generic = {
> .name = "axi-dac",
> .ops = &axi_dac_generic_ops,
> };
>
> +static const struct iio_backend_info axi_ad3552r = {
> + .name = "axi-ad3552r",
> + .ops = &axi_ad3552r_ops,
> +};
> +
> static const struct regmap_config axi_dac_regmap_config = {
> .val_bits = 32,
> .reg_bits = 32,
> @@ -555,7 +795,6 @@ static const struct regmap_config axi_dac_regmap_config =
> {
>
> static int axi_dac_probe(struct platform_device *pdev)
> {
> - const unsigned int *expected_ver;
> struct axi_dac_state *st;
> void __iomem *base;
> unsigned int ver;
> @@ -566,15 +805,26 @@ static int axi_dac_probe(struct platform_device *pdev)
> if (!st)
> return -ENOMEM;
>
> - expected_ver = device_get_match_data(&pdev->dev);
> - if (!expected_ver)
> + st->info = device_get_match_data(&pdev->dev);
> + if (!st->info)
> return -ENODEV;
>
> - clk = devm_clk_get_enabled(&pdev->dev, NULL);
> + clk = devm_clk_get_enabled(&pdev->dev, "s_axi_aclk");
> if (IS_ERR(clk))
> return dev_err_probe(&pdev->dev, PTR_ERR(clk),
> "failed to get clock\n");
>
> + if (st->info->has_dac_clk) {
> + struct clk *dac_clk;
> +
> + dac_clk = devm_clk_get_enabled(&pdev->dev, "dac_clk");
> + if (IS_ERR(dac_clk))
> + return dev_err_probe(&pdev->dev, PTR_ERR(dac_clk),
> + "failed to get dac_clk
> clock\n");
> +
> + st->dac_clk_rate = clk_get_rate(dac_clk);
> + }
> +
> base = devm_platform_ioremap_resource(pdev, 0);
> if (IS_ERR(base))
> return PTR_ERR(base);
> @@ -598,12 +848,13 @@ static int axi_dac_probe(struct platform_device *pdev)
> if (ret)
> return ret;
>
> - if (ADI_AXI_PCORE_VER_MAJOR(ver) !=
> ADI_AXI_PCORE_VER_MAJOR(*expected_ver)) {
> + if (ADI_AXI_PCORE_VER_MAJOR(ver) !=
> + ADI_AXI_PCORE_VER_MAJOR(st->info->version)) {
> dev_err(&pdev->dev,
> "Major version mismatch. Expected %d.%.2d.%c,
> Reported %d.%.2d.%c\n",
> - ADI_AXI_PCORE_VER_MAJOR(*expected_ver),
> - ADI_AXI_PCORE_VER_MINOR(*expected_ver),
> - ADI_AXI_PCORE_VER_PATCH(*expected_ver),
> + ADI_AXI_PCORE_VER_MAJOR(st->info->version),
> + ADI_AXI_PCORE_VER_MINOR(st->info->version),
> + ADI_AXI_PCORE_VER_PATCH(st->info->version),
> ADI_AXI_PCORE_VER_MAJOR(ver),
> ADI_AXI_PCORE_VER_MINOR(ver),
> ADI_AXI_PCORE_VER_PATCH(ver));
> @@ -629,7 +880,8 @@ static int axi_dac_probe(struct platform_device *pdev)
> return ret;
>
> mutex_init(&st->lock);
> - ret = devm_iio_backend_register(&pdev->dev, &axi_dac_generic, st);
> +
> + ret = devm_iio_backend_register(&pdev->dev, st->info->backend_info,
> st);
> if (ret)
> return dev_err_probe(&pdev->dev, ret,
> "failed to register iio backend\n");
> @@ -642,10 +894,21 @@ static int axi_dac_probe(struct platform_device *pdev)
> return 0;
> }
>
> -static unsigned int axi_dac_9_1_b_info = ADI_AXI_PCORE_VER(9, 1, 'b');
> +static const struct axi_dac_info dac_generic = {
> + .version = ADI_AXI_PCORE_VER(9, 1, 'b'),
> + .backend_info = &axi_dac_generic,
> +};
> +
> +static const struct axi_dac_info dac_ad3552r = {
> + .version = ADI_AXI_PCORE_VER(9, 1, 'b'),
> + .backend_info = &axi_ad3552r,
> + .bus_controller = true,
Do we still need bus_controller?
- Nuno Sá
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v5 06/10] iio: dac: adi-axi-dac: extend features
2024-10-10 13:20 ` Nuno Sá
@ 2024-10-11 8:06 ` Angelo Dureghello
0 siblings, 0 replies; 26+ messages in thread
From: Angelo Dureghello @ 2024-10-11 8:06 UTC (permalink / raw)
To: Nuno Sá
Cc: Lars-Peter Clausen, Michael Hennerich, Nuno Sa, Jonathan Cameron,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Olivier Moysan,
Jonathan Cameron, linux-iio, linux-kernel, devicetree, dletchner,
Mark Brown
On 10.10.2024 15:20, Nuno Sá wrote:
> On Tue, 2024-10-08 at 17:43 +0200, Angelo Dureghello wrote:
> > From: Angelo Dureghello <adureghello@baylibre.com>
> >
> > Extend AXI-DAC backend with new features required to interface
> > to the ad3552r DAC. Mainly, a new compatible string is added to
> > support the ad3552r-axi DAC IP, very similar to the generic DAC
> > IP but with some customizations to work with the ad3552r.
> >
> > Then, a serie of generic functions has been added to match with
> > ad3552r needs. Function names has been kept generic as much as
> > possible, to allow re-utilization from other frontend drivers.
> >
> > Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
> > ---
>
> Just some minor stuff. Not seeing any fundamental issue with this patch.
>
> > drivers/iio/dac/adi-axi-dac.c | 285 ++++++++++++++++++++++++++++++++++++++++-
> > -
> > 1 file changed, 274 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/iio/dac/adi-axi-dac.c b/drivers/iio/dac/adi-axi-dac.c
> > index 04193a98616e..e43d0ecccb50 100644
> > --- a/drivers/iio/dac/adi-axi-dac.c
> > +++ b/drivers/iio/dac/adi-axi-dac.c
> > @@ -46,9 +46,28 @@
> > #define AXI_DAC_CNTRL_1_REG 0x0044
> > #define AXI_DAC_CNTRL_1_SYNC BIT(0)
> > #define AXI_DAC_CNTRL_2_REG 0x0048
> > +#define AXI_DAC_CNTRL_2_SDR_DDR_N BIT(16)
> > +#define AXI_DAC_CNTRL_2_SYMB_8B BIT(14)
> > #define ADI_DAC_CNTRL_2_R1_MODE BIT(5)
> > +#define AXI_DAC_CNTRL_2_UNSIGNED_DATA BIT(4)
> > +#define AXI_DAC_STATUS_1_REG 0x0054
> > +#define AXI_DAC_STATUS_2_REG 0x0058
> > #define AXI_DAC_DRP_STATUS_REG 0x0074
> > #define AXI_DAC_DRP_STATUS_DRP_LOCKED BIT(17)
> > +#define AXI_DAC_CUSTOM_RD_REG 0x0080
> > +#define AXI_DAC_CUSTOM_WR_REG 0x0084
> > +#define AXI_DAC_CUSTOM_WR_DATA_8 GENMASK(23, 16)
> > +#define AXI_DAC_CUSTOM_WR_DATA_16 GENMASK(23, 8)
> > +#define AXI_DAC_UI_STATUS_REG 0x0088
> > +#define AXI_DAC_UI_STATUS_IF_BUSY BIT(4)
> > +#define AXI_DAC_CUSTOM_CTRL_REG 0x008C
> > +#define AXI_DAC_CUSTOM_CTRL_ADDRESS GENMASK(31, 24)
> > +#define AXI_DAC_CUSTOM_CTRL_SYNCED_TRANSFER BIT(2)
> > +#define AXI_DAC_CUSTOM_CTRL_STREAM BIT(1)
> > +#define AXI_DAC_CUSTOM_CTRL_TRANSFER_DATA BIT(0)
> > +
> > +#define
> > AXI_DAC_STREAM_ENABLE (AXI_DAC_CUSTOM_CTRL_TRANSFER_DATA | \
> > + AXI_DAC_CUSTOM_CTRL_STREAM)
> >
> > /* DAC Channel controls */
> > #define AXI_DAC_CHAN_CNTRL_1_REG(c) (0x0400 + (c) * 0x40)
> > @@ -63,12 +82,27 @@
> > #define AXI_DAC_CHAN_CNTRL_7_REG(c) (0x0418 + (c) * 0x40)
> > #define AXI_DAC_CHAN_CNTRL_7_DATA_SEL GENMASK(3, 0)
> >
> > +#define AXI_DAC_RD_ADDR(x) (BIT(7) | (x))
> > +
> > /* 360 degrees in rad */
> > #define AXI_DAC_2_PI_MEGA 6283190
> >
> > enum {
> > AXI_DAC_DATA_INTERNAL_TONE,
> > AXI_DAC_DATA_DMA = 2,
> > + AXI_DAC_DATA_INTERNAL_RAMP_16BIT = 11,
> > +};
> > +
> > +enum {
> > + AXI_DAC_BUS_TYPE_NONE,
> > + AXI_DAC_BUS_TYPE_DDR_QSPI,
> > +};
>
> Do we still need the above?
>
> > +
> > +struct axi_dac_info {
> > + unsigned int version;
> > + const struct iio_backend_info *backend_info;
> > + bool bus_controller;
> > + bool has_dac_clk;
> > };
> >
> > struct axi_dac_state {
> > @@ -79,9 +113,12 @@ struct axi_dac_state {
> > * data/variables.
> > */
> > struct mutex lock;
> > + const struct axi_dac_info *info;
> > + struct clk *clk;
>
> Is it used?
>
> > u64 dac_clk;
> > u32 reg_config;
> > bool int_tone;
> > + int dac_clk_rate;
> > };
> >
> > static int axi_dac_enable(struct iio_backend *back)
> > @@ -471,6 +508,11 @@ static int axi_dac_data_source_set(struct iio_backend
> > *back, unsigned int chan,
> > AXI_DAC_CHAN_CNTRL_7_REG(chan),
> > AXI_DAC_CHAN_CNTRL_7_DATA_SEL,
> > AXI_DAC_DATA_DMA);
> > + case IIO_BACKEND_INTERNAL_RAMP_16BIT:
> > + return regmap_update_bits(st->regmap,
> > + AXI_DAC_CHAN_CNTRL_7_REG(chan),
> > + AXI_DAC_CHAN_CNTRL_7_DATA_SEL,
> > + AXI_DAC_DATA_INTERNAL_RAMP_16BIT);
> > default:
> > return -EINVAL;
> > }
> > @@ -528,6 +570,186 @@ static int axi_dac_reg_access(struct iio_backend *back,
> > unsigned int reg,
> > return regmap_write(st->regmap, reg, writeval);
> > }
> >
> > +static int axi_dac_ddr_enable(struct iio_backend *back)
> > +{
> > + struct axi_dac_state *st = iio_backend_get_priv(back);
> > +
> > + return regmap_clear_bits(st->regmap, AXI_DAC_CNTRL_2_REG,
> > + AXI_DAC_CNTRL_2_SDR_DDR_N);
> > +}
> > +
> > +static int axi_dac_ddr_disable(struct iio_backend *back)
> > +{
> > + struct axi_dac_state *st = iio_backend_get_priv(back);
> > +
> > + return regmap_set_bits(st->regmap, AXI_DAC_CNTRL_2_REG,
> > + AXI_DAC_CNTRL_2_SDR_DDR_N);
> > +}
> > +
> > +static int axi_dac_data_stream_enable(struct iio_backend *back)
> > +{
> > + struct axi_dac_state *st = iio_backend_get_priv(back);
> > +
> > + return regmap_set_bits(st->regmap, AXI_DAC_CUSTOM_CTRL_REG,
> > + AXI_DAC_STREAM_ENABLE);
> > +}
> > +
> > +static int axi_dac_data_stream_disable(struct iio_backend *back)
> > +{
> > + struct axi_dac_state *st = iio_backend_get_priv(back);
> > +
> > + return regmap_clear_bits(st->regmap, AXI_DAC_CUSTOM_CTRL_REG,
> > + AXI_DAC_STREAM_ENABLE);
> > +}
> > +
> > +static int axi_dac_data_transfer_addr(struct iio_backend *back, u32 address)
> > +{
> > + struct axi_dac_state *st = iio_backend_get_priv(back);
> > +
>
> We could add some validation for address with FIELD_MAX()
>
> > + /*
> > + * Sample register address, when the DAC is configured, or stream
> > + * start address when the FSM is in stream state.
> > + */
> > + return regmap_update_bits(st->regmap, AXI_DAC_CUSTOM_CTRL_REG,
> > + AXI_DAC_CUSTOM_CTRL_ADDRESS,
> > + FIELD_PREP(AXI_DAC_CUSTOM_CTRL_ADDRESS,
> > + address));
> > +}
> > +
> > +static int axi_dac_data_format_set(struct iio_backend *back, unsigned int ch,
> > + const struct iio_backend_data_fmt *data)
> > +{
> > + struct axi_dac_state *st = iio_backend_get_priv(back);
> > + int err;
> >
>
> Not sure if I mentioned it already but please use 'ret' so it's consistent with
> what we have. But for this function, see below
>
> > + switch (data->type) {
> > + case IIO_BACKEND_DATA_UNSIGNED:
> > + err = regmap_clear_bits(st->regmap, AXI_DAC_CNTRL_2_REG,
> > + AXI_DAC_CNTRL_2_UNSIGNED_DATA);
> > + if (err)
> > + return err;
> > + break;
> return regmap_clear_bits();
> > + default:
> > + return -EINVAL;
> > + }
> > +
> > + return 0;
>
> remove the above...
>
> > +}
> > +
> > +static int axi_dac_read_raw(struct iio_backend *back,
> > + struct iio_chan_spec const *chan,
> > + int *val, int *val2, long mask)
> > +{
> > + struct axi_dac_state *st = iio_backend_get_priv(back);
> > + int err;
> > +
> > + switch (mask) {
> > + case IIO_CHAN_INFO_FREQUENCY: {
> > + int reg;
>
> I would just declare it at top scope and remove {}. Otherwise...
> > +
> > + if (!st->info->has_dac_clk)
> > + return -EOPNOTSUPP;
> > +
> > + /*
> > + * As from ad3552r AXI IP documentation,
> > + * returning the SCLK depending on the stream mode.
> > + */
> > + err = regmap_read(st->regmap, AXI_DAC_CUSTOM_CTRL_REG, ®);
> > + if (err)
> > + return err;
> > +
> > + if (reg & AXI_DAC_CUSTOM_CTRL_STREAM)
> > + *val = st->dac_clk_rate / 2;
> > + else
> > + *val = st->dac_clk_rate / 8;
> > +
> > + return IIO_VAL_INT;
> > + }
>
> ... does not look good
>
> > + default:
> > + return -EINVAL;
> > + }
> > +}
> > +
> > +static int axi_dac_bus_reg_write(struct iio_backend *back, u32 reg, u32 val,
> > + size_t data_size)
> > +{
> > + struct axi_dac_state *st = iio_backend_get_priv(back);
> > + int ret;
> > + u32 ival;
> > +
> > + if (data_size == 2)
>
> nit: sizeof(u16)
>
> > + ival = FIELD_PREP(AXI_DAC_CUSTOM_WR_DATA_16, val);
> > + else
> > + ival = FIELD_PREP(AXI_DAC_CUSTOM_WR_DATA_8, val);
> > +
> > + ret = regmap_write(st->regmap, AXI_DAC_CUSTOM_WR_REG, ival);
> > + if (ret)
> > + return ret;
> > +
> > + /*
> > + * Both REG_CNTRL_2 and AXI_DAC_CNTRL_DATA_WR need to know
> > + * the data size. So keeping data size control here only,
> > + * since data size is mandatory for the current transfer.
> > + * DDR state handled separately by specific backend calls,
> > + * generally all raw register writes are SDR.
> > + */
> > + if (data_size == 1)
> > + ret = regmap_set_bits(st->regmap, AXI_DAC_CNTRL_2_REG,
> > + AXI_DAC_CNTRL_2_SYMB_8B);
> > + else
> > + ret = regmap_clear_bits(st->regmap, AXI_DAC_CNTRL_2_REG,
> > + AXI_DAC_CNTRL_2_SYMB_8B);
> > + if (ret)
> > + return ret;
> > +
> > + ret = regmap_update_bits(st->regmap, AXI_DAC_CUSTOM_CTRL_REG,
> > + AXI_DAC_CUSTOM_CTRL_ADDRESS,
> > + FIELD_PREP(AXI_DAC_CUSTOM_CTRL_ADDRESS,
> > reg));
> > + if (ret)
> > + return ret;
> > +
> > + ret = regmap_update_bits(st->regmap, AXI_DAC_CUSTOM_CTRL_REG,
> > + AXI_DAC_CUSTOM_CTRL_TRANSFER_DATA,
> > + AXI_DAC_CUSTOM_CTRL_TRANSFER_DATA);
> > + if (ret)
> > + return ret;
> > +
> > + ret = regmap_read_poll_timeout(st->regmap,
> > + AXI_DAC_CUSTOM_CTRL_REG, ival,
> > + ival &
> > AXI_DAC_CUSTOM_CTRL_TRANSFER_DATA,
> > + 10, 100 * KILO);
> > + if (ret)
> > + return ret;
> > +
> > + return regmap_clear_bits(st->regmap, AXI_DAC_CUSTOM_CTRL_REG,
> > + AXI_DAC_CUSTOM_CTRL_TRANSFER_DATA);
> > +}
> > +
> > +static int axi_dac_bus_reg_read(struct iio_backend *back, u32 reg, u32 *val,
> > + size_t data_size)
> > +{
> > + struct axi_dac_state *st = iio_backend_get_priv(back);
> > + int ret;
> > + u32 ival;
> > +
> > + /*
> > + * SPI, we write with read flag, then we read just at the AXI
> > + * io address space to get data read.
> > + */
> > + ret = axi_dac_bus_reg_write(back, AXI_DAC_RD_ADDR(reg), 0,
> > data_size);
> > + 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) !=
> > + AXI_DAC_UI_STATUS_IF_BUSY,
> > + 10, 100);
> > + if (ret)
> > + return ret;
> > +
> > + return regmap_read(st->regmap, AXI_DAC_CUSTOM_RD_REG, val);
> > +}
> > +
> > static const struct iio_backend_ops axi_dac_generic_ops = {
> > .enable = axi_dac_enable,
> > .disable = axi_dac_disable,
> > @@ -541,11 +763,29 @@ static const struct iio_backend_ops axi_dac_generic_ops
> > = {
> > .debugfs_reg_access = iio_backend_debugfs_ptr(axi_dac_reg_access),
> > };
> >
> > +static const struct iio_backend_ops axi_ad3552r_ops = {
> > + .enable = axi_dac_enable,
> > + .read_raw = axi_dac_read_raw,
> > + .request_buffer = axi_dac_request_buffer,
> > + .data_source_set = axi_dac_data_source_set,
> > + .ddr_enable = axi_dac_ddr_enable,
> > + .ddr_disable = axi_dac_ddr_disable,
> > + .data_stream_enable = axi_dac_data_stream_enable,
> > + .data_stream_disable = axi_dac_data_stream_disable,
> > + .data_format_set = axi_dac_data_format_set,
> > + .data_transfer_addr = axi_dac_data_transfer_addr,
> > +};
> > +
> > static const struct iio_backend_info axi_dac_generic = {
> > .name = "axi-dac",
> > .ops = &axi_dac_generic_ops,
> > };
> >
> > +static const struct iio_backend_info axi_ad3552r = {
> > + .name = "axi-ad3552r",
> > + .ops = &axi_ad3552r_ops,
> > +};
> > +
> > static const struct regmap_config axi_dac_regmap_config = {
> > .val_bits = 32,
> > .reg_bits = 32,
> > @@ -555,7 +795,6 @@ static const struct regmap_config axi_dac_regmap_config =
> > {
> >
> > static int axi_dac_probe(struct platform_device *pdev)
> > {
> > - const unsigned int *expected_ver;
> > struct axi_dac_state *st;
> > void __iomem *base;
> > unsigned int ver;
> > @@ -566,15 +805,26 @@ static int axi_dac_probe(struct platform_device *pdev)
> > if (!st)
> > return -ENOMEM;
> >
> > - expected_ver = device_get_match_data(&pdev->dev);
> > - if (!expected_ver)
> > + st->info = device_get_match_data(&pdev->dev);
> > + if (!st->info)
> > return -ENODEV;
> >
> > - clk = devm_clk_get_enabled(&pdev->dev, NULL);
> > + clk = devm_clk_get_enabled(&pdev->dev, "s_axi_aclk");
> > if (IS_ERR(clk))
> > return dev_err_probe(&pdev->dev, PTR_ERR(clk),
> > "failed to get clock\n");
> >
> > + if (st->info->has_dac_clk) {
> > + struct clk *dac_clk;
> > +
> > + dac_clk = devm_clk_get_enabled(&pdev->dev, "dac_clk");
> > + if (IS_ERR(dac_clk))
> > + return dev_err_probe(&pdev->dev, PTR_ERR(dac_clk),
> > + "failed to get dac_clk
> > clock\n");
> > +
> > + st->dac_clk_rate = clk_get_rate(dac_clk);
> > + }
> > +
> > base = devm_platform_ioremap_resource(pdev, 0);
> > if (IS_ERR(base))
> > return PTR_ERR(base);
> > @@ -598,12 +848,13 @@ static int axi_dac_probe(struct platform_device *pdev)
> > if (ret)
> > return ret;
> >
> > - if (ADI_AXI_PCORE_VER_MAJOR(ver) !=
> > ADI_AXI_PCORE_VER_MAJOR(*expected_ver)) {
> > + if (ADI_AXI_PCORE_VER_MAJOR(ver) !=
> > + ADI_AXI_PCORE_VER_MAJOR(st->info->version)) {
> > dev_err(&pdev->dev,
> > "Major version mismatch. Expected %d.%.2d.%c,
> > Reported %d.%.2d.%c\n",
> > - ADI_AXI_PCORE_VER_MAJOR(*expected_ver),
> > - ADI_AXI_PCORE_VER_MINOR(*expected_ver),
> > - ADI_AXI_PCORE_VER_PATCH(*expected_ver),
> > + ADI_AXI_PCORE_VER_MAJOR(st->info->version),
> > + ADI_AXI_PCORE_VER_MINOR(st->info->version),
> > + ADI_AXI_PCORE_VER_PATCH(st->info->version),
> > ADI_AXI_PCORE_VER_MAJOR(ver),
> > ADI_AXI_PCORE_VER_MINOR(ver),
> > ADI_AXI_PCORE_VER_PATCH(ver));
> > @@ -629,7 +880,8 @@ static int axi_dac_probe(struct platform_device *pdev)
> > return ret;
> >
> > mutex_init(&st->lock);
> > - ret = devm_iio_backend_register(&pdev->dev, &axi_dac_generic, st);
> > +
> > + ret = devm_iio_backend_register(&pdev->dev, st->info->backend_info,
> > st);
> > if (ret)
> > return dev_err_probe(&pdev->dev, ret,
> > "failed to register iio backend\n");
> > @@ -642,10 +894,21 @@ static int axi_dac_probe(struct platform_device *pdev)
> > return 0;
> > }
> >
> > -static unsigned int axi_dac_9_1_b_info = ADI_AXI_PCORE_VER(9, 1, 'b');
> > +static const struct axi_dac_info dac_generic = {
> > + .version = ADI_AXI_PCORE_VER(9, 1, 'b'),
> > + .backend_info = &axi_dac_generic,
> > +};
> > +
> > +static const struct axi_dac_info dac_ad3552r = {
> > + .version = ADI_AXI_PCORE_VER(9, 1, 'b'),
> > + .backend_info = &axi_ad3552r,
> > + .bus_controller = true,
>
> Do we still need bus_controller?
actually still using it to create devices subnodes.
Will move it to the last patch.
>
> - Nuno Sá
>
>
Regards,
angelo
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v5 07/10] iio: dac: ad3552r: changes to use FIELD_PREP
2024-10-08 15:43 [PATCH v5 00/10] iio: add support for the ad3552r AXI DAC IP Angelo Dureghello
` (5 preceding siblings ...)
2024-10-08 15:43 ` [PATCH v5 06/10] iio: dac: adi-axi-dac: " Angelo Dureghello
@ 2024-10-08 15:43 ` Angelo Dureghello
2024-10-08 15:43 ` [PATCH v5 08/10] iio: dac: ad3552r: extract common code (no changes in behavior intended) Angelo Dureghello
` (2 subsequent siblings)
9 siblings, 0 replies; 26+ messages in thread
From: Angelo Dureghello @ 2024-10-08 15:43 UTC (permalink / raw)
To: Lars-Peter Clausen, Michael Hennerich, Nuno Sa, Jonathan Cameron,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Olivier Moysan
Cc: Jonathan Cameron, linux-iio, linux-kernel, devicetree, dletchner,
Mark Brown, Angelo Dureghello
From: Angelo Dureghello <adureghello@baylibre.com>
Changes to use FIELD_PREP, so that driver-specific ad3552r_field_prep
is removed. Variables (arrays) that was used to call ad3552r_field_prep
are removed too.
Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
---
drivers/iio/dac/ad3552r.c | 166 ++++++++++++++--------------------------------
1 file changed, 49 insertions(+), 117 deletions(-)
diff --git a/drivers/iio/dac/ad3552r.c b/drivers/iio/dac/ad3552r.c
index bd37d304ca70..c27706c5ba10 100644
--- a/drivers/iio/dac/ad3552r.c
+++ b/drivers/iio/dac/ad3552r.c
@@ -210,46 +210,6 @@ static const s32 gains_scaling_table[] = {
[AD3552R_CH_GAIN_SCALING_0_125] = 125
};
-enum ad3552r_dev_attributes {
- /* - Direct register values */
- /* From 0-3 */
- AD3552R_SDO_DRIVE_STRENGTH,
- /*
- * 0 -> Internal Vref, vref_io pin floating (default)
- * 1 -> Internal Vref, vref_io driven by internal vref
- * 2 or 3 -> External Vref
- */
- AD3552R_VREF_SELECT,
- /* Read registers in ascending order if set. Else descending */
- AD3552R_ADDR_ASCENSION,
-};
-
-enum ad3552r_ch_attributes {
- /* DAC powerdown */
- AD3552R_CH_DAC_POWERDOWN,
- /* DAC amplifier powerdown */
- AD3552R_CH_AMPLIFIER_POWERDOWN,
- /* Select the output range. Select from enum ad3552r_ch_output_range */
- AD3552R_CH_OUTPUT_RANGE_SEL,
- /*
- * Over-rider the range selector in order to manually set the output
- * voltage range
- */
- AD3552R_CH_RANGE_OVERRIDE,
- /* Manually set the offset voltage */
- AD3552R_CH_GAIN_OFFSET,
- /* Sets the polarity of the offset. */
- AD3552R_CH_GAIN_OFFSET_POLARITY,
- /* PDAC gain scaling */
- AD3552R_CH_GAIN_SCALING_P,
- /* NDAC gain scaling */
- AD3552R_CH_GAIN_SCALING_N,
- /* Rfb value */
- AD3552R_CH_RFB,
- /* Channel select. When set allow Input -> DAC and Mask -> DAC */
- AD3552R_CH_SELECT,
-};
-
struct ad3552r_ch_data {
s32 scale_int;
s32 scale_dec;
@@ -285,45 +245,6 @@ struct ad3552r_desc {
unsigned int num_ch;
};
-static const u16 addr_mask_map[][2] = {
- [AD3552R_ADDR_ASCENSION] = {
- AD3552R_REG_ADDR_INTERFACE_CONFIG_A,
- AD3552R_MASK_ADDR_ASCENSION
- },
- [AD3552R_SDO_DRIVE_STRENGTH] = {
- AD3552R_REG_ADDR_INTERFACE_CONFIG_D,
- AD3552R_MASK_SDO_DRIVE_STRENGTH
- },
- [AD3552R_VREF_SELECT] = {
- AD3552R_REG_ADDR_SH_REFERENCE_CONFIG,
- AD3552R_MASK_REFERENCE_VOLTAGE_SEL
- },
-};
-
-/* 0 -> reg addr, 1->ch0 mask, 2->ch1 mask */
-static const u16 addr_mask_map_ch[][3] = {
- [AD3552R_CH_DAC_POWERDOWN] = {
- AD3552R_REG_ADDR_POWERDOWN_CONFIG,
- AD3552R_MASK_CH_DAC_POWERDOWN(0),
- AD3552R_MASK_CH_DAC_POWERDOWN(1)
- },
- [AD3552R_CH_AMPLIFIER_POWERDOWN] = {
- AD3552R_REG_ADDR_POWERDOWN_CONFIG,
- AD3552R_MASK_CH_AMPLIFIER_POWERDOWN(0),
- AD3552R_MASK_CH_AMPLIFIER_POWERDOWN(1)
- },
- [AD3552R_CH_OUTPUT_RANGE_SEL] = {
- AD3552R_REG_ADDR_CH0_CH1_OUTPUT_RANGE,
- AD3552R_MASK_CH_OUTPUT_RANGE_SEL(0),
- AD3552R_MASK_CH_OUTPUT_RANGE_SEL(1)
- },
- [AD3552R_CH_SELECT] = {
- AD3552R_REG_ADDR_CH_SELECT_16B,
- AD3552R_MASK_CH(0),
- AD3552R_MASK_CH(1)
- }
-};
-
static u8 _ad3552r_reg_len(u8 addr)
{
switch (addr) {
@@ -399,11 +320,6 @@ static int ad3552r_read_reg(struct ad3552r_desc *dac, u8 addr, u16 *val)
return 0;
}
-static u16 ad3552r_field_prep(u16 val, u16 mask)
-{
- return (val << __ffs(mask)) & mask;
-}
-
/* Update field of a register, shift val if needed */
static int ad3552r_update_reg_field(struct ad3552r_desc *dac, u8 addr, u16 mask,
u16 val)
@@ -416,21 +332,11 @@ static int ad3552r_update_reg_field(struct ad3552r_desc *dac, u8 addr, u16 mask,
return ret;
reg &= ~mask;
- reg |= ad3552r_field_prep(val, mask);
+ reg |= val;
return ad3552r_write_reg(dac, addr, reg);
}
-static int ad3552r_set_ch_value(struct ad3552r_desc *dac,
- enum ad3552r_ch_attributes attr,
- u8 ch,
- u16 val)
-{
- /* Update register related to attributes in chip */
- return ad3552r_update_reg_field(dac, addr_mask_map_ch[attr][0],
- addr_mask_map_ch[attr][ch + 1], val);
-}
-
#define AD3552R_CH_DAC(_idx) ((struct iio_chan_spec) { \
.type = IIO_VOLTAGE, \
.output = true, \
@@ -510,8 +416,14 @@ static int ad3552r_write_raw(struct iio_dev *indio_dev,
val);
break;
case IIO_CHAN_INFO_ENABLE:
- err = ad3552r_set_ch_value(dac, AD3552R_CH_DAC_POWERDOWN,
- chan->channel, !val);
+ if (chan->channel == 0)
+ val = FIELD_PREP(AD3552R_MASK_CH_DAC_POWERDOWN(0), !val);
+ else
+ val = FIELD_PREP(AD3552R_MASK_CH_DAC_POWERDOWN(1), !val);
+
+ err = ad3552r_update_reg_field(dac, AD3552R_REG_ADDR_POWERDOWN_CONFIG,
+ AD3552R_MASK_CH_DAC_POWERDOWN(chan->channel),
+ val);
break;
default:
err = -EINVAL;
@@ -715,9 +627,9 @@ static int ad3552r_reset(struct ad3552r_desc *dac)
}
return ad3552r_update_reg_field(dac,
- addr_mask_map[AD3552R_ADDR_ASCENSION][0],
- addr_mask_map[AD3552R_ADDR_ASCENSION][1],
- val);
+ AD3552R_REG_ADDR_INTERFACE_CONFIG_A,
+ AD3552R_MASK_ADDR_ASCENSION,
+ FIELD_PREP(AD3552R_MASK_ADDR_ASCENSION, val));
}
static void ad3552r_get_custom_range(struct ad3552r_desc *dac, s32 i, s32 *v_min,
@@ -812,20 +724,20 @@ static int ad3552r_configure_custom_gain(struct ad3552r_desc *dac,
"mandatory custom-output-range-config property missing\n");
dac->ch_data[ch].range_override = 1;
- reg |= ad3552r_field_prep(1, AD3552R_MASK_CH_RANGE_OVERRIDE);
+ reg |= FIELD_PREP(AD3552R_MASK_CH_RANGE_OVERRIDE, 1);
err = fwnode_property_read_u32(gain_child, "adi,gain-scaling-p", &val);
if (err)
return dev_err_probe(dev, err,
"mandatory adi,gain-scaling-p property missing\n");
- reg |= ad3552r_field_prep(val, AD3552R_MASK_CH_GAIN_SCALING_P);
+ reg |= FIELD_PREP(AD3552R_MASK_CH_GAIN_SCALING_P, val);
dac->ch_data[ch].p = val;
err = fwnode_property_read_u32(gain_child, "adi,gain-scaling-n", &val);
if (err)
return dev_err_probe(dev, err,
"mandatory adi,gain-scaling-n property missing\n");
- reg |= ad3552r_field_prep(val, AD3552R_MASK_CH_GAIN_SCALING_N);
+ reg |= FIELD_PREP(AD3552R_MASK_CH_GAIN_SCALING_N, val);
dac->ch_data[ch].n = val;
err = fwnode_property_read_u32(gain_child, "adi,rfb-ohms", &val);
@@ -841,9 +753,9 @@ static int ad3552r_configure_custom_gain(struct ad3552r_desc *dac,
dac->ch_data[ch].gain_offset = val;
offset = abs((s32)val);
- reg |= ad3552r_field_prep((offset >> 8), AD3552R_MASK_CH_OFFSET_BIT_8);
+ reg |= FIELD_PREP(AD3552R_MASK_CH_OFFSET_BIT_8, (offset >> 8));
- reg |= ad3552r_field_prep((s32)val < 0, AD3552R_MASK_CH_OFFSET_POLARITY);
+ reg |= FIELD_PREP(AD3552R_MASK_CH_OFFSET_POLARITY, (s32)val < 0);
addr = AD3552R_REG_ADDR_CH_GAIN(ch);
err = ad3552r_write_reg(dac, addr,
offset & AD3552R_MASK_CH_OFFSET_BITS_0_7);
@@ -886,9 +798,9 @@ static int ad3552r_configure_device(struct ad3552r_desc *dac)
}
err = ad3552r_update_reg_field(dac,
- addr_mask_map[AD3552R_VREF_SELECT][0],
- addr_mask_map[AD3552R_VREF_SELECT][1],
- val);
+ AD3552R_REG_ADDR_SH_REFERENCE_CONFIG,
+ AD3552R_MASK_REFERENCE_VOLTAGE_SEL,
+ FIELD_PREP(AD3552R_MASK_REFERENCE_VOLTAGE_SEL, val));
if (err)
return err;
@@ -900,9 +812,9 @@ static int ad3552r_configure_device(struct ad3552r_desc *dac)
}
err = ad3552r_update_reg_field(dac,
- addr_mask_map[AD3552R_SDO_DRIVE_STRENGTH][0],
- addr_mask_map[AD3552R_SDO_DRIVE_STRENGTH][1],
- val);
+ AD3552R_REG_ADDR_INTERFACE_CONFIG_D,
+ AD3552R_MASK_SDO_DRIVE_STRENGTH,
+ FIELD_PREP(AD3552R_MASK_SDO_DRIVE_STRENGTH, val));
if (err)
return err;
}
@@ -938,9 +850,15 @@ static int ad3552r_configure_device(struct ad3552r_desc *dac)
"Invalid adi,output-range-microvolt value\n");
val = err;
- err = ad3552r_set_ch_value(dac,
- AD3552R_CH_OUTPUT_RANGE_SEL,
- ch, val);
+ if (ch == 0)
+ val = FIELD_PREP(AD3552R_MASK_CH_OUTPUT_RANGE_SEL(0), val);
+ else
+ val = FIELD_PREP(AD3552R_MASK_CH_OUTPUT_RANGE_SEL(1), val);
+
+ err = ad3552r_update_reg_field(dac,
+ AD3552R_REG_ADDR_CH0_CH1_OUTPUT_RANGE,
+ AD3552R_MASK_CH_OUTPUT_RANGE_SEL(ch),
+ val);
if (err)
return err;
@@ -958,7 +876,14 @@ static int ad3552r_configure_device(struct ad3552r_desc *dac)
ad3552r_calc_gain_and_offset(dac, ch);
dac->enabled_ch |= BIT(ch);
- err = ad3552r_set_ch_value(dac, AD3552R_CH_SELECT, ch, 1);
+ if (ch == 0)
+ val = FIELD_PREP(AD3552R_MASK_CH(0), 1);
+ else
+ val = FIELD_PREP(AD3552R_MASK_CH(1), 1);
+
+ err = ad3552r_update_reg_field(dac,
+ AD3552R_REG_ADDR_CH_SELECT_16B,
+ AD3552R_MASK_CH(ch), val);
if (err < 0)
return err;
@@ -970,8 +895,15 @@ static int ad3552r_configure_device(struct ad3552r_desc *dac)
/* Disable unused channels */
for_each_clear_bit(ch, &dac->enabled_ch,
dac->model_data->num_hw_channels) {
- err = ad3552r_set_ch_value(dac, AD3552R_CH_AMPLIFIER_POWERDOWN,
- ch, 1);
+ if (ch == 0)
+ val = FIELD_PREP(AD3552R_MASK_CH_OUTPUT_RANGE_SEL(0), 1);
+ else
+ val = FIELD_PREP(AD3552R_MASK_CH_OUTPUT_RANGE_SEL(1), 1);
+
+ err = ad3552r_update_reg_field(dac,
+ AD3552R_REG_ADDR_POWERDOWN_CONFIG,
+ AD3552R_MASK_CH_OUTPUT_RANGE_SEL(ch),
+ val);
if (err)
return err;
}
--
2.45.0.rc1
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v5 08/10] iio: dac: ad3552r: extract common code (no changes in behavior intended)
2024-10-08 15:43 [PATCH v5 00/10] iio: add support for the ad3552r AXI DAC IP Angelo Dureghello
` (6 preceding siblings ...)
2024-10-08 15:43 ` [PATCH v5 07/10] iio: dac: ad3552r: changes to use FIELD_PREP Angelo Dureghello
@ 2024-10-08 15:43 ` Angelo Dureghello
2024-10-08 15:43 ` [PATCH v5 09/10] iio: dac: ad3552r: add high-speed platform driver Angelo Dureghello
2024-10-08 15:43 ` [PATCH v5 10/10] iio: dac: adi-axi-dac: add registering of child fdt node Angelo Dureghello
9 siblings, 0 replies; 26+ messages in thread
From: Angelo Dureghello @ 2024-10-08 15:43 UTC (permalink / raw)
To: Lars-Peter Clausen, Michael Hennerich, Nuno Sa, Jonathan Cameron,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Olivier Moysan
Cc: Jonathan Cameron, linux-iio, linux-kernel, devicetree, dletchner,
Mark Brown, Angelo Dureghello
From: Angelo Dureghello <adureghello@baylibre.com>
Extracting common code, to share common code to be used later
by the AXI driver version (ad3552r-axi.c).
Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
---
drivers/iio/dac/Makefile | 2 +-
drivers/iio/dac/ad3552r-common.c | 170 ++++++++++++++++++++++
drivers/iio/dac/ad3552r.c | 303 ++++-----------------------------------
drivers/iio/dac/ad3552r.h | 200 ++++++++++++++++++++++++++
4 files changed, 398 insertions(+), 277 deletions(-)
diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
index 621d553bd6e3..c92de0366238 100644
--- a/drivers/iio/dac/Makefile
+++ b/drivers/iio/dac/Makefile
@@ -4,7 +4,7 @@
#
# When adding new entries keep the list in alphabetical order
-obj-$(CONFIG_AD3552R) += ad3552r.o
+obj-$(CONFIG_AD3552R) += ad3552r.o ad3552r-common.o
obj-$(CONFIG_AD5360) += ad5360.o
obj-$(CONFIG_AD5380) += ad5380.o
obj-$(CONFIG_AD5421) += ad5421.o
diff --git a/drivers/iio/dac/ad3552r-common.c b/drivers/iio/dac/ad3552r-common.c
new file mode 100644
index 000000000000..9a892abf99ac
--- /dev/null
+++ b/drivers/iio/dac/ad3552r-common.c
@@ -0,0 +1,170 @@
+// SPDX-License-Identifier: GPL-2.0+
+//
+// Copyright (c) 2010-2024 Analog Devices Inc.
+// Copyright (c) 2024 Baylibre, SAS
+
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/property.h>
+#include <linux/regulator/consumer.h>
+
+#include "ad3552r.h"
+
+const s32 ad3552r_ch_ranges[AD3552R_MAX_RANGES][2] = {
+ [AD3552R_CH_OUTPUT_RANGE_0__2P5V] = { 0, 2500 },
+ [AD3552R_CH_OUTPUT_RANGE_0__5V] = { 0, 5000 },
+ [AD3552R_CH_OUTPUT_RANGE_0__10V] = { 0, 10000 },
+ [AD3552R_CH_OUTPUT_RANGE_NEG_5__5V] = { -5000, 5000 },
+ [AD3552R_CH_OUTPUT_RANGE_NEG_10__10V] = { -10000, 10000 }
+};
+EXPORT_SYMBOL_NS_GPL(ad3552r_ch_ranges, IIO_AD355R);
+
+const s32 ad3542r_ch_ranges[AD3542R_MAX_RANGES][2] = {
+ [AD3542R_CH_OUTPUT_RANGE_0__2P5V] = { 0, 2500 },
+ [AD3542R_CH_OUTPUT_RANGE_0__3V] = { 0, 3000 },
+ [AD3542R_CH_OUTPUT_RANGE_0__5V] = { 0, 5000 },
+ [AD3542R_CH_OUTPUT_RANGE_0__10V] = { 0, 10000 },
+ [AD3542R_CH_OUTPUT_RANGE_NEG_2P5__7P5V] = { -2500, 7500 },
+ [AD3542R_CH_OUTPUT_RANGE_NEG_5__5V] = { -5000, 5000 }
+};
+EXPORT_SYMBOL_NS_GPL(ad3542r_ch_ranges, IIO_AD355R);
+
+u16 ad3552r_calc_custom_gain(u8 p, u8 n, s16 goffs)
+{
+ u16 reg;
+
+ reg = FIELD_PREP(AD3552R_MASK_CH_RANGE_OVERRIDE, 1);
+ reg |= FIELD_PREP(AD3552R_MASK_CH_GAIN_SCALING_P, p);
+ reg |= FIELD_PREP(AD3552R_MASK_CH_GAIN_SCALING_N, n);
+ reg |= FIELD_PREP(AD3552R_MASK_CH_OFFSET_BIT_8, abs(goffs));
+ reg |= FIELD_PREP(AD3552R_MASK_CH_OFFSET_POLARITY, goffs < 0);
+
+ return reg;
+}
+
+int ad3552r_get_ref_voltage(struct device *dev)
+{
+ int voltage;
+ int delta = 100000;
+
+ voltage = devm_regulator_get_enable_read_voltage(dev, "vref");
+ if (voltage < 0 && voltage != -ENODEV)
+ return dev_err_probe(dev, voltage,
+ "Error getting vref voltage\n");
+
+ if (voltage == -ENODEV) {
+ if (device_property_read_bool(dev, "adi,vref-out-en"))
+ return AD3552R_INTERNAL_VREF_PIN_2P5V;
+ else
+ return AD3552R_INTERNAL_VREF_PIN_FLOATING;
+ }
+
+ if (voltage > 2500000 + delta || voltage < 2500000 - delta) {
+ dev_warn(dev, "vref-supply must be 2.5V");
+ return -EINVAL;
+ }
+
+ return AD3552R_EXTERNAL_VREF_PIN_INPUT;
+}
+
+int ad3552r_get_drive_strength(struct device *dev, u32 *val)
+{
+ int err;
+ u32 drive_strength;
+
+ err = device_property_read_u32(dev, "adi,sdo-drive-strength",
+ &drive_strength);
+ if (err)
+ return err;
+
+ if (drive_strength > 3) {
+ dev_err_probe(dev, -EINVAL,
+ "adi,sdo-drive-strength must be less than 4\n");
+ return -EINVAL;
+ }
+
+ *val = drive_strength;
+
+ return 0;
+}
+
+int ad3552r_get_custom_gain(struct device *dev, struct fwnode_handle *child,
+ u8 *gs_p, u8 *gs_n, u16 *rfb, s16 *goffs)
+{
+ int err;
+ u32 val;
+ struct fwnode_handle *gain_child __free(fwnode_handle) =
+ fwnode_get_named_child_node(child,
+ "custom-output-range-config");
+
+ if (!gain_child)
+ return dev_err_probe(dev, -EINVAL,
+ "custom-output-range-config mandatory\n");
+
+ err = fwnode_property_read_u32(gain_child, "adi,gain-scaling-p", &val);
+ if (err)
+ return dev_err_probe(dev, err,
+ "adi,gain-scaling-p mandatory\n");
+ *gs_p = val;
+
+ err = fwnode_property_read_u32(gain_child, "adi,gain-scaling-n", &val);
+ if (err)
+ return dev_err_probe(dev, err,
+ "adi,gain-scaling-n property mandatory\n");
+ *gs_n = val;
+
+ err = fwnode_property_read_u32(gain_child, "adi,rfb-ohms", &val);
+ if (err)
+ return dev_err_probe(dev, err,
+ "adi,rfb-ohms mandatory\n");
+ *rfb = val;
+
+ err = fwnode_property_read_u32(gain_child, "adi,gain-offset", &val);
+ if (err)
+ return dev_err_probe(dev, err,
+ "adi,gain-offset mandatory\n");
+ *goffs = val;
+
+ return 0;
+}
+
+static int ad3552r_find_range(const struct ad3552r_model_data *model_info,
+ s32 *vals)
+{
+ int i;
+
+ for (i = 0; i < model_info->num_ranges; i++)
+ if (vals[0] == model_info->ranges_table[i][0] * 1000 &&
+ vals[1] == model_info->ranges_table[i][1] * 1000)
+ return i;
+
+ return -EINVAL;
+}
+
+int ad3552r_get_output_range(struct device *dev,
+ const struct ad3552r_model_data *model_info,
+ struct fwnode_handle *child, u32 *val)
+{
+ int ret;
+ s32 vals[2];
+
+ /* This property is optional, so returning -ENOENT if missing */
+ if (!fwnode_property_present(child, "adi,output-range-microvolt"))
+ return -ENOENT;
+
+ ret = fwnode_property_read_u32_array(child,
+ "adi,output-range-microvolt",
+ vals, 2);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "invalid adi,output-range-microvolt\n");
+
+ ret = ad3552r_find_range(model_info, vals);
+ if (ret < 0)
+ return dev_err_probe(dev, ret,
+ "invalid adi,output-range-microvolt value\n");
+
+ *val = ret;
+
+ return 0;
+}
diff --git a/drivers/iio/dac/ad3552r.c b/drivers/iio/dac/ad3552r.c
index c27706c5ba10..21a0b4d87bc7 100644
--- a/drivers/iio/dac/ad3552r.c
+++ b/drivers/iio/dac/ad3552r.c
@@ -11,185 +11,9 @@
#include <linux/iio/trigger_consumer.h>
#include <linux/iopoll.h>
#include <linux/kernel.h>
-#include <linux/regulator/consumer.h>
#include <linux/spi/spi.h>
-/* Register addresses */
-/* Primary address space */
-#define AD3552R_REG_ADDR_INTERFACE_CONFIG_A 0x00
-#define AD3552R_MASK_SOFTWARE_RESET (BIT(7) | BIT(0))
-#define AD3552R_MASK_ADDR_ASCENSION BIT(5)
-#define AD3552R_MASK_SDO_ACTIVE BIT(4)
-#define AD3552R_REG_ADDR_INTERFACE_CONFIG_B 0x01
-#define AD3552R_MASK_SINGLE_INST BIT(7)
-#define AD3552R_MASK_SHORT_INSTRUCTION BIT(3)
-#define AD3552R_REG_ADDR_DEVICE_CONFIG 0x02
-#define AD3552R_MASK_DEVICE_STATUS(n) BIT(4 + (n))
-#define AD3552R_MASK_CUSTOM_MODES GENMASK(3, 2)
-#define AD3552R_MASK_OPERATING_MODES GENMASK(1, 0)
-#define AD3552R_REG_ADDR_CHIP_TYPE 0x03
-#define AD3552R_MASK_CLASS GENMASK(7, 0)
-#define AD3552R_REG_ADDR_PRODUCT_ID_L 0x04
-#define AD3552R_REG_ADDR_PRODUCT_ID_H 0x05
-#define AD3552R_REG_ADDR_CHIP_GRADE 0x06
-#define AD3552R_MASK_GRADE GENMASK(7, 4)
-#define AD3552R_MASK_DEVICE_REVISION GENMASK(3, 0)
-#define AD3552R_REG_ADDR_SCRATCH_PAD 0x0A
-#define AD3552R_REG_ADDR_SPI_REVISION 0x0B
-#define AD3552R_REG_ADDR_VENDOR_L 0x0C
-#define AD3552R_REG_ADDR_VENDOR_H 0x0D
-#define AD3552R_REG_ADDR_STREAM_MODE 0x0E
-#define AD3552R_MASK_LENGTH GENMASK(7, 0)
-#define AD3552R_REG_ADDR_TRANSFER_REGISTER 0x0F
-#define AD3552R_MASK_MULTI_IO_MODE GENMASK(7, 6)
-#define AD3552R_MASK_STREAM_LENGTH_KEEP_VALUE BIT(2)
-#define AD3552R_REG_ADDR_INTERFACE_CONFIG_C 0x10
-#define AD3552R_MASK_CRC_ENABLE (GENMASK(7, 6) |\
- GENMASK(1, 0))
-#define AD3552R_MASK_STRICT_REGISTER_ACCESS BIT(5)
-#define AD3552R_REG_ADDR_INTERFACE_STATUS_A 0x11
-#define AD3552R_MASK_INTERFACE_NOT_READY BIT(7)
-#define AD3552R_MASK_CLOCK_COUNTING_ERROR BIT(5)
-#define AD3552R_MASK_INVALID_OR_NO_CRC BIT(3)
-#define AD3552R_MASK_WRITE_TO_READ_ONLY_REGISTER BIT(2)
-#define AD3552R_MASK_PARTIAL_REGISTER_ACCESS BIT(1)
-#define AD3552R_MASK_REGISTER_ADDRESS_INVALID BIT(0)
-#define AD3552R_REG_ADDR_INTERFACE_CONFIG_D 0x14
-#define AD3552R_MASK_ALERT_ENABLE_PULLUP BIT(6)
-#define AD3552R_MASK_MEM_CRC_EN BIT(4)
-#define AD3552R_MASK_SDO_DRIVE_STRENGTH GENMASK(3, 2)
-#define AD3552R_MASK_DUAL_SPI_SYNCHROUNOUS_EN BIT(1)
-#define AD3552R_MASK_SPI_CONFIG_DDR BIT(0)
-#define AD3552R_REG_ADDR_SH_REFERENCE_CONFIG 0x15
-#define AD3552R_MASK_IDUMP_FAST_MODE BIT(6)
-#define AD3552R_MASK_SAMPLE_HOLD_DIFFERENTIAL_USER_EN BIT(5)
-#define AD3552R_MASK_SAMPLE_HOLD_USER_TRIM GENMASK(4, 3)
-#define AD3552R_MASK_SAMPLE_HOLD_USER_ENABLE BIT(2)
-#define AD3552R_MASK_REFERENCE_VOLTAGE_SEL GENMASK(1, 0)
-#define AD3552R_REG_ADDR_ERR_ALARM_MASK 0x16
-#define AD3552R_MASK_REF_RANGE_ALARM BIT(6)
-#define AD3552R_MASK_CLOCK_COUNT_ERR_ALARM BIT(5)
-#define AD3552R_MASK_MEM_CRC_ERR_ALARM BIT(4)
-#define AD3552R_MASK_SPI_CRC_ERR_ALARM BIT(3)
-#define AD3552R_MASK_WRITE_TO_READ_ONLY_ALARM BIT(2)
-#define AD3552R_MASK_PARTIAL_REGISTER_ACCESS_ALARM BIT(1)
-#define AD3552R_MASK_REGISTER_ADDRESS_INVALID_ALARM BIT(0)
-#define AD3552R_REG_ADDR_ERR_STATUS 0x17
-#define AD3552R_MASK_REF_RANGE_ERR_STATUS BIT(6)
-#define AD3552R_MASK_DUAL_SPI_STREAM_EXCEEDS_DAC_ERR_STATUS BIT(5)
-#define AD3552R_MASK_MEM_CRC_ERR_STATUS BIT(4)
-#define AD3552R_MASK_RESET_STATUS BIT(0)
-#define AD3552R_REG_ADDR_POWERDOWN_CONFIG 0x18
-#define AD3552R_MASK_CH_DAC_POWERDOWN(ch) BIT(4 + (ch))
-#define AD3552R_MASK_CH_AMPLIFIER_POWERDOWN(ch) BIT(ch)
-#define AD3552R_REG_ADDR_CH0_CH1_OUTPUT_RANGE 0x19
-#define AD3552R_MASK_CH_OUTPUT_RANGE_SEL(ch) ((ch) ? GENMASK(7, 4) :\
- GENMASK(3, 0))
-#define AD3552R_REG_ADDR_CH_OFFSET(ch) (0x1B + (ch) * 2)
-#define AD3552R_MASK_CH_OFFSET_BITS_0_7 GENMASK(7, 0)
-#define AD3552R_REG_ADDR_CH_GAIN(ch) (0x1C + (ch) * 2)
-#define AD3552R_MASK_CH_RANGE_OVERRIDE BIT(7)
-#define AD3552R_MASK_CH_GAIN_SCALING_N GENMASK(6, 5)
-#define AD3552R_MASK_CH_GAIN_SCALING_P GENMASK(4, 3)
-#define AD3552R_MASK_CH_OFFSET_POLARITY BIT(2)
-#define AD3552R_MASK_CH_OFFSET_BIT_8 BIT(0)
-/*
- * Secondary region
- * For multibyte registers specify the highest address because the access is
- * done in descending order
- */
-#define AD3552R_SECONDARY_REGION_START 0x28
-#define AD3552R_REG_ADDR_HW_LDAC_16B 0x28
-#define AD3552R_REG_ADDR_CH_DAC_16B(ch) (0x2C - (1 - ch) * 2)
-#define AD3552R_REG_ADDR_DAC_PAGE_MASK_16B 0x2E
-#define AD3552R_REG_ADDR_CH_SELECT_16B 0x2F
-#define AD3552R_REG_ADDR_INPUT_PAGE_MASK_16B 0x31
-#define AD3552R_REG_ADDR_SW_LDAC_16B 0x32
-#define AD3552R_REG_ADDR_CH_INPUT_16B(ch) (0x36 - (1 - ch) * 2)
-/* 3 bytes registers */
-#define AD3552R_REG_START_24B 0x37
-#define AD3552R_REG_ADDR_HW_LDAC_24B 0x37
-#define AD3552R_REG_ADDR_CH_DAC_24B(ch) (0x3D - (1 - ch) * 3)
-#define AD3552R_REG_ADDR_DAC_PAGE_MASK_24B 0x40
-#define AD3552R_REG_ADDR_CH_SELECT_24B 0x41
-#define AD3552R_REG_ADDR_INPUT_PAGE_MASK_24B 0x44
-#define AD3552R_REG_ADDR_SW_LDAC_24B 0x45
-#define AD3552R_REG_ADDR_CH_INPUT_24B(ch) (0x4B - (1 - ch) * 3)
-
-/* Useful defines */
-#define AD3552R_MAX_CH 2
-#define AD3552R_MASK_CH(ch) BIT(ch)
-#define AD3552R_MASK_ALL_CH GENMASK(1, 0)
-#define AD3552R_MAX_REG_SIZE 3
-#define AD3552R_READ_BIT BIT(7)
-#define AD3552R_ADDR_MASK GENMASK(6, 0)
-#define AD3552R_MASK_DAC_12B 0xFFF0
-#define AD3552R_DEFAULT_CONFIG_B_VALUE 0x8
-#define AD3552R_SCRATCH_PAD_TEST_VAL1 0x34
-#define AD3552R_SCRATCH_PAD_TEST_VAL2 0xB2
-#define AD3552R_GAIN_SCALE 1000
-#define AD3552R_LDAC_PULSE_US 100
-
-enum ad3552r_ch_vref_select {
- /* Internal source with Vref I/O floating */
- AD3552R_INTERNAL_VREF_PIN_FLOATING,
- /* Internal source with Vref I/O at 2.5V */
- AD3552R_INTERNAL_VREF_PIN_2P5V,
- /* External source with Vref I/O as input */
- AD3552R_EXTERNAL_VREF_PIN_INPUT
-};
-
-enum ad3552r_id {
- AD3541R_ID = 0x400b,
- AD3542R_ID = 0x4009,
- AD3551R_ID = 0x400a,
- AD3552R_ID = 0x4008,
-};
-
-enum ad3552r_ch_output_range {
- /* Range from 0 V to 2.5 V. Requires Rfb1x connection */
- AD3552R_CH_OUTPUT_RANGE_0__2P5V,
- /* Range from 0 V to 5 V. Requires Rfb1x connection */
- AD3552R_CH_OUTPUT_RANGE_0__5V,
- /* Range from 0 V to 10 V. Requires Rfb2x connection */
- AD3552R_CH_OUTPUT_RANGE_0__10V,
- /* Range from -5 V to 5 V. Requires Rfb2x connection */
- AD3552R_CH_OUTPUT_RANGE_NEG_5__5V,
- /* Range from -10 V to 10 V. Requires Rfb4x connection */
- AD3552R_CH_OUTPUT_RANGE_NEG_10__10V,
-};
-
-static const s32 ad3552r_ch_ranges[][2] = {
- [AD3552R_CH_OUTPUT_RANGE_0__2P5V] = {0, 2500},
- [AD3552R_CH_OUTPUT_RANGE_0__5V] = {0, 5000},
- [AD3552R_CH_OUTPUT_RANGE_0__10V] = {0, 10000},
- [AD3552R_CH_OUTPUT_RANGE_NEG_5__5V] = {-5000, 5000},
- [AD3552R_CH_OUTPUT_RANGE_NEG_10__10V] = {-10000, 10000}
-};
-
-enum ad3542r_ch_output_range {
- /* Range from 0 V to 2.5 V. Requires Rfb1x connection */
- AD3542R_CH_OUTPUT_RANGE_0__2P5V,
- /* Range from 0 V to 3 V. Requires Rfb1x connection */
- AD3542R_CH_OUTPUT_RANGE_0__3V,
- /* Range from 0 V to 5 V. Requires Rfb1x connection */
- AD3542R_CH_OUTPUT_RANGE_0__5V,
- /* Range from 0 V to 10 V. Requires Rfb2x connection */
- AD3542R_CH_OUTPUT_RANGE_0__10V,
- /* Range from -2.5 V to 7.5 V. Requires Rfb2x connection */
- AD3542R_CH_OUTPUT_RANGE_NEG_2P5__7P5V,
- /* Range from -5 V to 5 V. Requires Rfb2x connection */
- AD3542R_CH_OUTPUT_RANGE_NEG_5__5V,
-};
-
-static const s32 ad3542r_ch_ranges[][2] = {
- [AD3542R_CH_OUTPUT_RANGE_0__2P5V] = {0, 2500},
- [AD3542R_CH_OUTPUT_RANGE_0__3V] = {0, 3000},
- [AD3542R_CH_OUTPUT_RANGE_0__5V] = {0, 5000},
- [AD3542R_CH_OUTPUT_RANGE_0__10V] = {0, 10000},
- [AD3542R_CH_OUTPUT_RANGE_NEG_2P5__7P5V] = {-2500, 7500},
- [AD3542R_CH_OUTPUT_RANGE_NEG_5__5V] = {-5000, 5000}
-};
+#include "ad3552r.h"
enum ad3552r_ch_gain_scaling {
/* Gain scaling of 1 */
@@ -223,15 +47,6 @@ struct ad3552r_ch_data {
bool range_override;
};
-struct ad3552r_model_data {
- const char *model_name;
- enum ad3552r_id chip_id;
- unsigned int num_hw_channels;
- const s32 (*ranges_table)[2];
- int num_ranges;
- bool requires_output_range;
-};
-
struct ad3552r_desc {
const struct ad3552r_model_data *model_data;
/* Used to look the spi bus for atomic operations where needed */
@@ -693,75 +508,35 @@ static void ad3552r_calc_gain_and_offset(struct ad3552r_desc *dac, s32 ch)
dac->ch_data[ch].offset_dec = div_s64(tmp, span);
}
-static int ad3552r_find_range(const struct ad3552r_model_data *model_data,
- s32 *vals)
-{
- int i;
-
- for (i = 0; i < model_data->num_ranges; i++)
- if (vals[0] == model_data->ranges_table[i][0] * 1000 &&
- vals[1] == model_data->ranges_table[i][1] * 1000)
- return i;
-
- return -EINVAL;
-}
-
static int ad3552r_configure_custom_gain(struct ad3552r_desc *dac,
struct fwnode_handle *child,
u32 ch)
{
struct device *dev = &dac->spi->dev;
- u32 val;
int err;
u8 addr;
- u16 reg = 0, offset;
-
- struct fwnode_handle *gain_child __free(fwnode_handle)
- = fwnode_get_named_child_node(child,
- "custom-output-range-config");
- if (!gain_child)
- return dev_err_probe(dev, -EINVAL,
- "mandatory custom-output-range-config property missing\n");
-
- dac->ch_data[ch].range_override = 1;
- reg |= FIELD_PREP(AD3552R_MASK_CH_RANGE_OVERRIDE, 1);
-
- err = fwnode_property_read_u32(gain_child, "adi,gain-scaling-p", &val);
- if (err)
- return dev_err_probe(dev, err,
- "mandatory adi,gain-scaling-p property missing\n");
- reg |= FIELD_PREP(AD3552R_MASK_CH_GAIN_SCALING_P, val);
- dac->ch_data[ch].p = val;
-
- err = fwnode_property_read_u32(gain_child, "adi,gain-scaling-n", &val);
- if (err)
- return dev_err_probe(dev, err,
- "mandatory adi,gain-scaling-n property missing\n");
- reg |= FIELD_PREP(AD3552R_MASK_CH_GAIN_SCALING_N, val);
- dac->ch_data[ch].n = val;
-
- err = fwnode_property_read_u32(gain_child, "adi,rfb-ohms", &val);
- if (err)
- return dev_err_probe(dev, err,
- "mandatory adi,rfb-ohms property missing\n");
- dac->ch_data[ch].rfb = val;
+ u16 reg;
- err = fwnode_property_read_u32(gain_child, "adi,gain-offset", &val);
+ err = ad3552r_get_custom_gain(dev, child,
+ &dac->ch_data[ch].p,
+ &dac->ch_data[ch].n,
+ &dac->ch_data[ch].rfb,
+ &dac->ch_data[ch].gain_offset);
if (err)
- return dev_err_probe(dev, err,
- "mandatory adi,gain-offset property missing\n");
- dac->ch_data[ch].gain_offset = val;
+ return err;
- offset = abs((s32)val);
- reg |= FIELD_PREP(AD3552R_MASK_CH_OFFSET_BIT_8, (offset >> 8));
+ dac->ch_data[ch].range_override = 1;
- reg |= FIELD_PREP(AD3552R_MASK_CH_OFFSET_POLARITY, (s32)val < 0);
addr = AD3552R_REG_ADDR_CH_GAIN(ch);
err = ad3552r_write_reg(dac, addr,
- offset & AD3552R_MASK_CH_OFFSET_BITS_0_7);
+ abs((s32)dac->ch_data[ch].gain_offset) &
+ AD3552R_MASK_CH_OFFSET_BITS_0_7);
if (err)
return dev_err_probe(dev, err, "Error writing register\n");
+ reg = ad3552r_calc_custom_gain(dac->ch_data[ch].p, dac->ch_data[ch].n,
+ dac->ch_data[ch].gain_offset);
+
err = ad3552r_write_reg(dac, addr, reg);
if (err)
return dev_err_probe(dev, err, "Error writing register\n");
@@ -772,30 +547,19 @@ static int ad3552r_configure_custom_gain(struct ad3552r_desc *dac,
static int ad3552r_configure_device(struct ad3552r_desc *dac)
{
struct device *dev = &dac->spi->dev;
- int err, cnt = 0, voltage, delta = 100000;
- u32 vals[2], val, ch;
+ int err, cnt = 0;
+ u32 val, ch;
dac->gpio_ldac = devm_gpiod_get_optional(dev, "ldac", GPIOD_OUT_HIGH);
if (IS_ERR(dac->gpio_ldac))
return dev_err_probe(dev, PTR_ERR(dac->gpio_ldac),
"Error getting gpio ldac");
- voltage = devm_regulator_get_enable_read_voltage(dev, "vref");
- if (voltage < 0 && voltage != -ENODEV)
- return dev_err_probe(dev, voltage, "Error getting vref voltage\n");
+ err = ad3552r_get_ref_voltage(dev);
+ if (err < 0)
+ return err;
- if (voltage == -ENODEV) {
- if (device_property_read_bool(dev, "adi,vref-out-en"))
- val = AD3552R_INTERNAL_VREF_PIN_2P5V;
- else
- val = AD3552R_INTERNAL_VREF_PIN_FLOATING;
- } else {
- if (voltage > 2500000 + delta || voltage < 2500000 - delta) {
- dev_warn(dev, "vref-supply must be 2.5V");
- return -EINVAL;
- }
- val = AD3552R_EXTERNAL_VREF_PIN_INPUT;
- }
+ val = err;
err = ad3552r_update_reg_field(dac,
AD3552R_REG_ADDR_SH_REFERENCE_CONFIG,
@@ -804,13 +568,8 @@ static int ad3552r_configure_device(struct ad3552r_desc *dac)
if (err)
return err;
- err = device_property_read_u32(dev, "adi,sdo-drive-strength", &val);
+ err = ad3552r_get_drive_strength(dev, &val);
if (!err) {
- if (val > 3) {
- dev_err(dev, "adi,sdo-drive-strength must be less than 4\n");
- return -EINVAL;
- }
-
err = ad3552r_update_reg_field(dac,
AD3552R_REG_ADDR_INTERFACE_CONFIG_D,
AD3552R_MASK_SDO_DRIVE_STRENGTH,
@@ -835,21 +594,12 @@ static int ad3552r_configure_device(struct ad3552r_desc *dac)
"reg must be less than %d\n",
dac->model_data->num_hw_channels);
- if (fwnode_property_present(child, "adi,output-range-microvolt")) {
- err = fwnode_property_read_u32_array(child,
- "adi,output-range-microvolt",
- vals,
- 2);
- if (err)
- return dev_err_probe(dev, err,
- "adi,output-range-microvolt property could not be parsed\n");
-
- err = ad3552r_find_range(dac->model_data, vals);
- if (err < 0)
- return dev_err_probe(dev, err,
- "Invalid adi,output-range-microvolt value\n");
+ err = ad3552r_get_output_range(dev, dac->model_data,
+ child, &val);
+ if (err && err != -ENOENT)
+ return err;
- val = err;
+ if (!err) {
if (ch == 0)
val = FIELD_PREP(AD3552R_MASK_CH_OUTPUT_RANGE_SEL(0), val);
else
@@ -1072,3 +822,4 @@ module_spi_driver(ad3552r_driver);
MODULE_AUTHOR("Mihail Chindris <mihail.chindris@analog.com>");
MODULE_DESCRIPTION("Analog Device AD3552R DAC");
MODULE_LICENSE("GPL v2");
+MODULE_IMPORT_NS(IIO_AD3552R);
diff --git a/drivers/iio/dac/ad3552r.h b/drivers/iio/dac/ad3552r.h
new file mode 100644
index 000000000000..088eb8ecfac6
--- /dev/null
+++ b/drivers/iio/dac/ad3552r.h
@@ -0,0 +1,200 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * AD3552R Digital <-> Analog converters common header
+ *
+ * Copyright 2021-2024 Analog Devices Inc.
+ * Author: Angelo Dureghello <adureghello@baylibre.com>
+ */
+
+#ifndef __DRIVERS_IIO_DAC_AD3552R_H__
+#define __DRIVERS_IIO_DAC_AD3552R_H__
+
+/* Register addresses */
+/* Primary address space */
+#define AD3552R_REG_ADDR_INTERFACE_CONFIG_A 0x00
+#define AD3552R_MASK_SOFTWARE_RESET (BIT(7) | BIT(0))
+#define AD3552R_MASK_ADDR_ASCENSION BIT(5)
+#define AD3552R_MASK_SDO_ACTIVE BIT(4)
+#define AD3552R_REG_ADDR_INTERFACE_CONFIG_B 0x01
+#define AD3552R_MASK_SINGLE_INST BIT(7)
+#define AD3552R_MASK_SHORT_INSTRUCTION BIT(3)
+#define AD3552R_REG_ADDR_DEVICE_CONFIG 0x02
+#define AD3552R_MASK_DEVICE_STATUS(n) BIT(4 + (n))
+#define AD3552R_MASK_CUSTOM_MODES GENMASK(3, 2)
+#define AD3552R_MASK_OPERATING_MODES GENMASK(1, 0)
+#define AD3552R_REG_ADDR_CHIP_TYPE 0x03
+#define AD3552R_MASK_CLASS GENMASK(7, 0)
+#define AD3552R_REG_ADDR_PRODUCT_ID_L 0x04
+#define AD3552R_REG_ADDR_PRODUCT_ID_H 0x05
+#define AD3552R_REG_ADDR_CHIP_GRADE 0x06
+#define AD3552R_MASK_GRADE GENMASK(7, 4)
+#define AD3552R_MASK_DEVICE_REVISION GENMASK(3, 0)
+#define AD3552R_REG_ADDR_SCRATCH_PAD 0x0A
+#define AD3552R_REG_ADDR_SPI_REVISION 0x0B
+#define AD3552R_REG_ADDR_VENDOR_L 0x0C
+#define AD3552R_REG_ADDR_VENDOR_H 0x0D
+#define AD3552R_REG_ADDR_STREAM_MODE 0x0E
+#define AD3552R_MASK_LENGTH GENMASK(7, 0)
+#define AD3552R_REG_ADDR_TRANSFER_REGISTER 0x0F
+#define AD3552R_MASK_MULTI_IO_MODE GENMASK(7, 6)
+#define AD3552R_MASK_STREAM_LENGTH_KEEP_VALUE BIT(2)
+#define AD3552R_REG_ADDR_INTERFACE_CONFIG_C 0x10
+#define AD3552R_MASK_CRC_ENABLE (GENMASK(7, 6) |\
+ GENMASK(1, 0))
+#define AD3552R_MASK_STRICT_REGISTER_ACCESS BIT(5)
+#define AD3552R_REG_ADDR_INTERFACE_STATUS_A 0x11
+#define AD3552R_MASK_INTERFACE_NOT_READY BIT(7)
+#define AD3552R_MASK_CLOCK_COUNTING_ERROR BIT(5)
+#define AD3552R_MASK_INVALID_OR_NO_CRC BIT(3)
+#define AD3552R_MASK_WRITE_TO_READ_ONLY_REGISTER BIT(2)
+#define AD3552R_MASK_PARTIAL_REGISTER_ACCESS BIT(1)
+#define AD3552R_MASK_REGISTER_ADDRESS_INVALID BIT(0)
+#define AD3552R_REG_ADDR_INTERFACE_CONFIG_D 0x14
+#define AD3552R_MASK_ALERT_ENABLE_PULLUP BIT(6)
+#define AD3552R_MASK_MEM_CRC_EN BIT(4)
+#define AD3552R_MASK_SDO_DRIVE_STRENGTH GENMASK(3, 2)
+#define AD3552R_MASK_DUAL_SPI_SYNCHROUNOUS_EN BIT(1)
+#define AD3552R_MASK_SPI_CONFIG_DDR BIT(0)
+#define AD3552R_REG_ADDR_SH_REFERENCE_CONFIG 0x15
+#define AD3552R_MASK_IDUMP_FAST_MODE BIT(6)
+#define AD3552R_MASK_SAMPLE_HOLD_DIFF_USER_EN BIT(5)
+#define AD3552R_MASK_SAMPLE_HOLD_USER_TRIM GENMASK(4, 3)
+#define AD3552R_MASK_SAMPLE_HOLD_USER_ENABLE BIT(2)
+#define AD3552R_MASK_REFERENCE_VOLTAGE_SEL GENMASK(1, 0)
+#define AD3552R_REG_ADDR_ERR_ALARM_MASK 0x16
+#define AD3552R_MASK_REF_RANGE_ALARM BIT(6)
+#define AD3552R_MASK_CLOCK_COUNT_ERR_ALARM BIT(5)
+#define AD3552R_MASK_MEM_CRC_ERR_ALARM BIT(4)
+#define AD3552R_MASK_SPI_CRC_ERR_ALARM BIT(3)
+#define AD3552R_MASK_WRITE_TO_READ_ONLY_ALARM BIT(2)
+#define AD3552R_MASK_PARTIAL_REGISTER_ACCESS_ALARM BIT(1)
+#define AD3552R_MASK_REGISTER_ADDRESS_INVALID_ALARM BIT(0)
+#define AD3552R_REG_ADDR_ERR_STATUS 0x17
+#define AD3552R_MASK_REF_RANGE_ERR_STATUS BIT(6)
+#define AD3552R_MASK_STREAM_EXCEEDS_DAC_ERR_STATUS BIT(5)
+#define AD3552R_MASK_MEM_CRC_ERR_STATUS BIT(4)
+#define AD3552R_MASK_RESET_STATUS BIT(0)
+#define AD3552R_REG_ADDR_POWERDOWN_CONFIG 0x18
+#define AD3552R_MASK_CH_DAC_POWERDOWN(ch) BIT(4 + (ch))
+#define AD3552R_MASK_CH_AMPLIFIER_POWERDOWN(ch) BIT(ch)
+#define AD3552R_REG_ADDR_CH0_CH1_OUTPUT_RANGE 0x19
+#define AD3552R_MASK_CH0_RANGE GENMASK(2, 0)
+#define AD3552R_MASK_CH1_RANGE GENMASK(6, 4)
+#define AD3552R_MASK_CH_OUTPUT_RANGE GENMASK(7, 0)
+#define AD3552R_MASK_CH_OUTPUT_RANGE_SEL(ch) ((ch) ? \
+ GENMASK(7, 4) : \
+ GENMASK(3, 0))
+#define AD3552R_REG_ADDR_CH_OFFSET(ch) (0x1B + (ch) * 2)
+#define AD3552R_MASK_CH_OFFSET_BITS_0_7 GENMASK(7, 0)
+#define AD3552R_REG_ADDR_CH_GAIN(ch) (0x1C + (ch) * 2)
+#define AD3552R_MASK_CH_RANGE_OVERRIDE BIT(7)
+#define AD3552R_MASK_CH_GAIN_SCALING_N GENMASK(6, 5)
+#define AD3552R_MASK_CH_GAIN_SCALING_P GENMASK(4, 3)
+#define AD3552R_MASK_CH_OFFSET_POLARITY BIT(2)
+#define AD3552R_MASK_CH_OFFSET_BIT_8 BIT(8)
+/*
+ * Secondary region
+ * For multibyte registers specify the highest address because the access is
+ * done in descending order
+ */
+#define AD3552R_SECONDARY_REGION_START 0x28
+#define AD3552R_REG_ADDR_HW_LDAC_16B 0x28
+#define AD3552R_REG_ADDR_CH_DAC_16B(ch) (0x2C - (1 - (ch)) * 2)
+#define AD3552R_REG_ADDR_DAC_PAGE_MASK_16B 0x2E
+#define AD3552R_REG_ADDR_CH_SELECT_16B 0x2F
+#define AD3552R_REG_ADDR_INPUT_PAGE_MASK_16B 0x31
+#define AD3552R_REG_ADDR_SW_LDAC_16B 0x32
+#define AD3552R_REG_ADDR_CH_INPUT_16B(ch) (0x36 - (1 - (ch)) * 2)
+/* 3 bytes registers */
+#define AD3552R_REG_START_24B 0x37
+#define AD3552R_REG_ADDR_HW_LDAC_24B 0x37
+#define AD3552R_REG_ADDR_CH_DAC_24B(ch) (0x3D - (1 - (ch)) * 3)
+#define AD3552R_REG_ADDR_DAC_PAGE_MASK_24B 0x40
+#define AD3552R_REG_ADDR_CH_SELECT_24B 0x41
+#define AD3552R_REG_ADDR_INPUT_PAGE_MASK_24B 0x44
+#define AD3552R_REG_ADDR_SW_LDAC_24B 0x45
+#define AD3552R_REG_ADDR_CH_INPUT_24B(ch) (0x4B - (1 - (ch)) * 3)
+
+/* Useful defines */
+#define AD3552R_MAX_CH 2
+#define AD3552R_MASK_CH(ch) BIT(ch)
+#define AD3552R_MASK_ALL_CH GENMASK(1, 0)
+#define AD3552R_MAX_REG_SIZE 3
+#define AD3552R_READ_BIT BIT(7)
+#define AD3552R_ADDR_MASK GENMASK(6, 0)
+#define AD3552R_MASK_DAC_12B GENMASK(15, 4)
+#define AD3552R_DEFAULT_CONFIG_B_VALUE 0x8
+#define AD3552R_SCRATCH_PAD_TEST_VAL1 0x34
+#define AD3552R_SCRATCH_PAD_TEST_VAL2 0xB2
+#define AD3552R_GAIN_SCALE 1000
+#define AD3552R_LDAC_PULSE_US 100
+
+#define AD3552R_MAX_RANGES 5
+#define AD3542R_MAX_RANGES 6
+
+extern const s32 ad3552r_ch_ranges[AD3552R_MAX_RANGES][2];
+extern const s32 ad3542r_ch_ranges[AD3542R_MAX_RANGES][2];
+
+enum ad3552r_id {
+ AD3541R_ID = 0x400b,
+ AD3542R_ID = 0x4009,
+ AD3551R_ID = 0x400a,
+ AD3552R_ID = 0x4008,
+};
+
+struct ad3552r_model_data {
+ const char *model_name;
+ enum ad3552r_id chip_id;
+ unsigned int num_hw_channels;
+ const s32 (*ranges_table)[2];
+ int num_ranges;
+ bool requires_output_range;
+};
+
+enum ad3552r_ch_vref_select {
+ /* Internal source with Vref I/O floating */
+ AD3552R_INTERNAL_VREF_PIN_FLOATING,
+ /* Internal source with Vref I/O at 2.5V */
+ AD3552R_INTERNAL_VREF_PIN_2P5V,
+ /* External source with Vref I/O as input */
+ AD3552R_EXTERNAL_VREF_PIN_INPUT
+};
+
+enum ad3542r_ch_output_range {
+ /* Range from 0 V to 2.5 V. Requires Rfb1x connection */
+ AD3542R_CH_OUTPUT_RANGE_0__2P5V,
+ /* Range from 0 V to 3 V. Requires Rfb1x connection */
+ AD3542R_CH_OUTPUT_RANGE_0__3V,
+ /* Range from 0 V to 5 V. Requires Rfb1x connection */
+ AD3542R_CH_OUTPUT_RANGE_0__5V,
+ /* Range from 0 V to 10 V. Requires Rfb2x connection */
+ AD3542R_CH_OUTPUT_RANGE_0__10V,
+ /* Range from -2.5 V to 7.5 V. Requires Rfb2x connection */
+ AD3542R_CH_OUTPUT_RANGE_NEG_2P5__7P5V,
+ /* Range from -5 V to 5 V. Requires Rfb2x connection */
+ AD3542R_CH_OUTPUT_RANGE_NEG_5__5V,
+};
+
+enum ad3552r_ch_output_range {
+ /* Range from 0 V to 2.5 V. Requires Rfb1x connection */
+ AD3552R_CH_OUTPUT_RANGE_0__2P5V,
+ /* Range from 0 V to 5 V. Requires Rfb1x connection */
+ AD3552R_CH_OUTPUT_RANGE_0__5V,
+ /* Range from 0 V to 10 V. Requires Rfb2x connection */
+ AD3552R_CH_OUTPUT_RANGE_0__10V,
+ /* Range from -5 V to 5 V. Requires Rfb2x connection */
+ AD3552R_CH_OUTPUT_RANGE_NEG_5__5V,
+ /* Range from -10 V to 10 V. Requires Rfb4x connection */
+ AD3552R_CH_OUTPUT_RANGE_NEG_10__10V,
+};
+
+int ad3552r_get_output_range(struct device *dev,
+ const struct ad3552r_model_data *model_info,
+ struct fwnode_handle *child, u32 *val);
+int ad3552r_get_custom_gain(struct device *dev, struct fwnode_handle *child,
+ u8 *gs_p, u8 *gs_n, u16 *rfb, s16 *goffs);
+u16 ad3552r_calc_custom_gain(u8 p, u8 n, s16 goffs);
+int ad3552r_get_ref_voltage(struct device *dev);
+int ad3552r_get_drive_strength(struct device *dev, u32 *val);
+
+#endif /* __DRIVERS_IIO_DAC_AD3552R_H__ */
--
2.45.0.rc1
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v5 09/10] iio: dac: ad3552r: add high-speed platform driver
2024-10-08 15:43 [PATCH v5 00/10] iio: add support for the ad3552r AXI DAC IP Angelo Dureghello
` (7 preceding siblings ...)
2024-10-08 15:43 ` [PATCH v5 08/10] iio: dac: ad3552r: extract common code (no changes in behavior intended) Angelo Dureghello
@ 2024-10-08 15:43 ` Angelo Dureghello
2024-10-12 14:07 ` Jonathan Cameron
2024-10-08 15:43 ` [PATCH v5 10/10] iio: dac: adi-axi-dac: add registering of child fdt node Angelo Dureghello
9 siblings, 1 reply; 26+ messages in thread
From: Angelo Dureghello @ 2024-10-08 15:43 UTC (permalink / raw)
To: Lars-Peter Clausen, Michael Hennerich, Nuno Sa, Jonathan Cameron,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Olivier Moysan
Cc: Jonathan Cameron, linux-iio, linux-kernel, devicetree, dletchner,
Mark Brown, Angelo Dureghello
From: Angelo Dureghello <adureghello@baylibre.com>
Add High Speed ad3552r platform driver.
The ad3552r DAC is controlled by a custom (fpga-based) DAC IP
through the current AXI backend, or similar alternative IIO backend.
Compared to the existing driver (ad3552r.c), that is a simple SPI
driver, this driver is coupled with a DAC IIO backend that finally
controls the ad3552r by a fpga-based "QSPI+DDR" interface, to reach
maximum transfer rate of 33MUPS using dma stream capabilities.
All commands involving QSPI bus read/write are delegated to the backend
through the provided APIs for bus read/write.
Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
---
drivers/iio/dac/Kconfig | 14 +
drivers/iio/dac/Makefile | 1 +
drivers/iio/dac/ad3552r-hs.c | 526 +++++++++++++++++++++++++++++++
drivers/iio/dac/ad3552r.h | 7 +
include/linux/platform_data/ad3552r-hs.h | 18 ++
5 files changed, 566 insertions(+)
diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
index fa091995d002..fc11698e88f2 100644
--- a/drivers/iio/dac/Kconfig
+++ b/drivers/iio/dac/Kconfig
@@ -6,6 +6,20 @@
menu "Digital to analog converters"
+config AD3552R_HS
+ tristate "Analog Devices AD3552R DAC High Speed driver"
+ select ADI_AXI_DAC
+ help
+ Say yes here to build support for Analog Devices AD3552R
+ Digital to Analog Converter High Speed driver.
+
+ The driver requires the assistance of an IP core to operate,
+ since data is streamed into target device via DMA, sent over a
+ QSPI + DDR (Double Data Rate) bus.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ad3552r-hs.
+
config AD3552R
tristate "Analog Devices AD3552R DAC driver"
depends on SPI_MASTER
diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
index c92de0366238..d92e08ca93ca 100644
--- a/drivers/iio/dac/Makefile
+++ b/drivers/iio/dac/Makefile
@@ -4,6 +4,7 @@
#
# When adding new entries keep the list in alphabetical order
+obj-$(CONFIG_AD3552R_HS) += ad3552r-hs.o ad3552r-common.o
obj-$(CONFIG_AD3552R) += ad3552r.o ad3552r-common.o
obj-$(CONFIG_AD5360) += ad5360.o
obj-$(CONFIG_AD5380) += ad5380.o
diff --git a/drivers/iio/dac/ad3552r-hs.c b/drivers/iio/dac/ad3552r-hs.c
new file mode 100644
index 000000000000..4de33988721f
--- /dev/null
+++ b/drivers/iio/dac/ad3552r-hs.c
@@ -0,0 +1,526 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Analog Devices AD3552R
+ * Digital to Analog converter driver, High Speed version
+ *
+ * Copyright 2024 Analog Devices Inc.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/iio/backend.h>
+#include <linux/iio/buffer.h>
+#include <linux/mod_devicetable.h>
+#include <linux/platform_data/ad3552r-hs.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/units.h>
+
+#include "ad3552r.h"
+
+struct ad3552r_hs_state {
+ const struct ad3552r_model_data *model_data;
+ struct gpio_desc *reset_gpio;
+ struct device *dev;
+ struct iio_backend *back;
+ bool single_channel;
+ struct ad3552r_hs_platform_data *data;
+ bool ddr_mode;
+};
+
+static int ad3552r_qspi_update_reg_bits(struct ad3552r_hs_state *st,
+ u32 reg, u32 mask, u32 val,
+ size_t xfer_size)
+{
+ u32 rval;
+ int err;
+
+ err = st->data->bus_reg_read(st->back, reg, &rval, xfer_size);
+ if (err)
+ return err;
+
+ rval &= ~mask;
+ rval |= val;
+
+ return st->data->bus_reg_write(st->back, reg, rval, xfer_size);
+}
+
+static int ad3552r_hs_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int *val, int *val2, long mask)
+{
+ struct ad3552r_hs_state *st = iio_priv(indio_dev);
+ int ret;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_SAMP_FREQ: {
+ int sclk;
+
+ ret = iio_backend_read_raw(st->back, chan, &sclk, 0,
+ IIO_CHAN_INFO_FREQUENCY);
+ if (ret != IIO_VAL_INT)
+ return -EINVAL;
+
+ /* Using 4 lanes (QSPI) */
+ *val = DIV_ROUND_CLOSEST(sclk * 4 * (1 + st->ddr_mode),
+ chan->scan_type.storagebits);
+
+ return IIO_VAL_INT;
+ }
+ case IIO_CHAN_INFO_RAW:
+ ret = st->data->bus_reg_read(st->back,
+ AD3552R_REG_ADDR_CH_DAC_16B(chan->channel),
+ val, 2);
+ if (ret)
+ return ret;
+
+ return IIO_VAL_INT;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int ad3552r_hs_write_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int val, int val2, long mask)
+{
+ struct ad3552r_hs_state *st = iio_priv(indio_dev);
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
+ return st->data->bus_reg_write(st->back,
+ AD3552R_REG_ADDR_CH_DAC_16B(chan->channel),
+ val, 2);
+ }
+ unreachable();
+ default:
+ return -EINVAL;
+ }
+}
+
+static int ad3552r_hs_buffer_postenable(struct iio_dev *indio_dev)
+{
+ struct ad3552r_hs_state *st = iio_priv(indio_dev);
+ struct iio_backend_data_fmt fmt = {
+ .type = IIO_BACKEND_DATA_UNSIGNED
+ };
+ int loop_len, val, err;
+
+ /* Inform DAC chip to switch into DDR mode */
+ err = ad3552r_qspi_update_reg_bits(st,
+ AD3552R_REG_ADDR_INTERFACE_CONFIG_D,
+ AD3552R_MASK_SPI_CONFIG_DDR,
+ AD3552R_MASK_SPI_CONFIG_DDR, 1);
+ if (err)
+ return err;
+
+ /* Inform DAC IP to go for DDR mode from now on */
+ err = iio_backend_ddr_enable(st->back);
+ if (err) {
+ dev_warn(st->dev, "could not set DDR mode, not streaming");
+ goto exit_err;
+ }
+
+ st->ddr_mode = true;
+
+ switch (*indio_dev->active_scan_mask) {
+ case AD3552R_CH0_ACTIVE:
+ st->single_channel = true;
+ loop_len = 2;
+ val = AD3552R_REG_ADDR_CH_DAC_16B(0);
+ break;
+ case AD3552R_CH1_ACTIVE:
+ st->single_channel = true;
+ loop_len = 2;
+ val = AD3552R_REG_ADDR_CH_DAC_16B(1);
+ break;
+ case AD3552R_CH0_CH1_ACTIVE:
+ st->single_channel = false;
+ loop_len = 4;
+ val = AD3552R_REG_ADDR_CH_DAC_16B(1);
+ break;
+ default:
+ err = -EINVAL;
+ goto exit_err_ddr;
+ }
+
+ err = st->data->bus_reg_write(st->back, AD3552R_REG_ADDR_STREAM_MODE,
+ loop_len, 1);
+ if (err)
+ goto exit_err_ddr;
+
+ err = iio_backend_data_transfer_addr(st->back, val);
+ if (err)
+ goto exit_err_ddr;
+
+ err = iio_backend_data_format_set(st->back, 0, &fmt);
+ if (err)
+ goto exit_err_ddr;
+
+ err = iio_backend_data_stream_enable(st->back);
+ if (err)
+ goto exit_err_ddr;
+
+ return 0;
+
+exit_err_ddr:
+ iio_backend_ddr_disable(st->back);
+
+exit_err:
+ ad3552r_qspi_update_reg_bits(st,
+ AD3552R_REG_ADDR_INTERFACE_CONFIG_D,
+ AD3552R_MASK_SPI_CONFIG_DDR,
+ 0, 1);
+
+ iio_backend_ddr_disable(st->back);
+
+ st->ddr_mode = false;
+
+ return err;
+}
+
+static int ad3552r_hs_buffer_predisable(struct iio_dev *indio_dev)
+{
+ struct ad3552r_hs_state *st = iio_priv(indio_dev);
+ int err;
+
+ err = iio_backend_data_stream_disable(st->back);
+ if (err)
+ return err;
+
+ /* Inform DAC to set in SDR mode */
+ err = ad3552r_qspi_update_reg_bits(st,
+ AD3552R_REG_ADDR_INTERFACE_CONFIG_D,
+ AD3552R_MASK_SPI_CONFIG_DDR,
+ 0, 1);
+ if (err)
+ return err;
+
+ err = iio_backend_ddr_disable(st->back);
+ if (err)
+ return err;
+
+ st->ddr_mode = false;
+
+ return 0;
+}
+
+static int ad3552r_hs_set_output_range(struct ad3552r_hs_state *st,
+ unsigned int mode)
+{
+ return ad3552r_qspi_update_reg_bits(st,
+ AD3552R_REG_ADDR_CH0_CH1_OUTPUT_RANGE,
+ AD3552R_MASK_CH_OUTPUT_RANGE,
+ FIELD_PREP(AD3552R_MASK_CH0_RANGE, mode) |
+ FIELD_PREP(AD3552R_MASK_CH1_RANGE, mode),
+ 1);
+}
+
+static int ad3552r_hs_reset(struct ad3552r_hs_state *st)
+{
+ int err;
+
+ st->reset_gpio = devm_gpiod_get_optional(st->dev,
+ "reset", GPIOD_OUT_LOW);
+ if (IS_ERR(st->reset_gpio))
+ return PTR_ERR(st->reset_gpio);
+
+ if (st->reset_gpio) {
+ fsleep(10);
+ gpiod_set_value_cansleep(st->reset_gpio, 1);
+ } else {
+ err = ad3552r_qspi_update_reg_bits(st,
+ AD3552R_REG_ADDR_INTERFACE_CONFIG_A,
+ AD3552R_MASK_SOFTWARE_RESET,
+ AD3552R_MASK_SOFTWARE_RESET, 1);
+ if (err)
+ return err;
+ }
+ msleep(100);
+
+ return 0;
+}
+
+static int ad3552r_hs_scratch_pad_test(struct ad3552r_hs_state *st)
+{
+ int err, val;
+
+ err = st->data->bus_reg_write(st->back, AD3552R_REG_ADDR_SCRATCH_PAD,
+ AD3552R_SCRATCH_PAD_TEST_VAL1, 1);
+ if (err)
+ return err;
+
+ err = st->data->bus_reg_read(st->back, AD3552R_REG_ADDR_SCRATCH_PAD,
+ &val, 1);
+ if (err)
+ return err;
+
+ if (val != AD3552R_SCRATCH_PAD_TEST_VAL1) {
+ dev_err(st->dev,
+ "SCRATCH_PAD_TEST mismatch. Expected 0x%x, Read 0x%x\n",
+ AD3552R_SCRATCH_PAD_TEST_VAL1, val);
+ return -EIO;
+ }
+
+ err = st->data->bus_reg_write(st->back, AD3552R_REG_ADDR_SCRATCH_PAD,
+ AD3552R_SCRATCH_PAD_TEST_VAL2, 1);
+ if (err)
+ return err;
+
+ err = st->data->bus_reg_read(st->back, AD3552R_REG_ADDR_SCRATCH_PAD,
+ &val, 1);
+ if (err)
+ return err;
+
+ if (val != AD3552R_SCRATCH_PAD_TEST_VAL2) {
+ dev_err(st->dev,
+ "SCRATCH_PAD_TEST mismatch. Expected 0x%x, Read 0x%x\n",
+ AD3552R_SCRATCH_PAD_TEST_VAL2, val);
+ return -EIO;
+ }
+
+ return 0;
+}
+
+static int ad3552r_hs_setup_custom_gain(struct ad3552r_hs_state *st,
+ u16 gain, u16 offset)
+{
+ int err;
+
+ err = st->data->bus_reg_write(st->back, AD3552R_REG_ADDR_CH_OFFSET(0),
+ offset, 1);
+ if (err)
+ return dev_err_probe(st->dev, err, "Error writing register\n");
+
+ err = st->data->bus_reg_write(st->back, AD3552R_REG_ADDR_CH_OFFSET(1),
+ offset, 1);
+ if (err)
+ return dev_err_probe(st->dev, err, "Error writing register\n");
+
+ err = st->data->bus_reg_write(st->back, AD3552R_REG_ADDR_CH_GAIN(0),
+ gain, 1);
+ if (err)
+ return dev_err_probe(st->dev, err, "Error writing register\n");
+
+ err = st->data->bus_reg_write(st->back, AD3552R_REG_ADDR_CH_GAIN(1),
+ gain, 1);
+ if (err)
+ return dev_err_probe(st->dev, err, "Error writing register\n");
+
+ return 0;
+}
+
+static int ad3552r_hs_setup(struct ad3552r_hs_state *st)
+{
+ u8 gs_p, gs_n;
+ s16 goffs;
+ u16 id, rfb;
+ u16 gain = 0, offset = 0;
+ u32 val, range;
+ int err;
+
+ err = ad3552r_hs_reset(st);
+ if (err)
+ return err;
+
+ err = iio_backend_ddr_disable(st->back);
+ if (err)
+ return err;
+
+ err = ad3552r_hs_scratch_pad_test(st);
+ if (err)
+ return err;
+
+ err = st->data->bus_reg_read(st->back, AD3552R_REG_ADDR_PRODUCT_ID_L,
+ &val, 1);
+ if (err)
+ return err;
+
+ id = val;
+
+ err = st->data->bus_reg_read(st->back, AD3552R_REG_ADDR_PRODUCT_ID_H,
+ &val, 1);
+ if (err)
+ return err;
+
+ id |= val << 8;
+ if (id != st->model_data->chip_id)
+ dev_info(st->dev, "Chip ID error. Expected 0x%x, Read 0x%x\n",
+ AD3552R_ID, id);
+
+ err = st->data->bus_reg_write(st->back,
+ AD3552R_REG_ADDR_SH_REFERENCE_CONFIG,
+ 0, 1);
+ if (err)
+ return err;
+
+ err = st->data->bus_reg_write(st->back,
+ AD3552R_REG_ADDR_TRANSFER_REGISTER,
+ AD3552R_MASK_QUAD_SPI |
+ AD3552R_MASK_STREAM_LENGTH_KEEP_VALUE, 1);
+ if (err)
+ return err;
+
+ err = iio_backend_data_source_set(st->back, 0, IIO_BACKEND_EXTERNAL);
+ if (err)
+ return err;
+
+ err = iio_backend_data_source_set(st->back, 1, IIO_BACKEND_EXTERNAL);
+ if (err)
+ return err;
+
+ err = ad3552r_get_ref_voltage(st->dev);
+ if (err < 0)
+ return err;
+
+ val = err;
+
+ err = ad3552r_qspi_update_reg_bits(st,
+ AD3552R_REG_ADDR_SH_REFERENCE_CONFIG,
+ AD3552R_MASK_REFERENCE_VOLTAGE_SEL,
+ val, 1);
+ if (err)
+ return err;
+
+ err = ad3552r_get_drive_strength(st->dev, &val);
+ if (!err) {
+ err = ad3552r_qspi_update_reg_bits(st,
+ AD3552R_REG_ADDR_INTERFACE_CONFIG_D,
+ AD3552R_MASK_SDO_DRIVE_STRENGTH,
+ val, 1);
+ if (err)
+ return err;
+ }
+
+ struct fwnode_handle *child __free(fwnode_handle) =
+ device_get_named_child_node(st->dev, "channel");
+ if (!child)
+ return -EINVAL;
+
+ /*
+ * One of "adi,output-range-microvolt" or "custom-output-range-config"
+ * must be available in fdt.
+ */
+ err = ad3552r_get_output_range(st->dev, st->model_data, child, &range);
+ if (!err)
+ return ad3552r_hs_set_output_range(st, range);
+ if (err != -ENOENT)
+ return err;
+
+ err = ad3552r_get_custom_gain(st->dev, child, &gs_p, &gs_n, &rfb,
+ &goffs);
+ if (err)
+ return err;
+
+ gain = ad3552r_calc_custom_gain(gs_p, gs_n, goffs);
+ offset = abs(goffs);
+
+ return ad3552r_hs_setup_custom_gain(st, gain, offset);
+}
+
+static const struct iio_buffer_setup_ops ad3552r_hs_buffer_setup_ops = {
+ .postenable = ad3552r_hs_buffer_postenable,
+ .predisable = ad3552r_hs_buffer_predisable,
+};
+
+#define AD3552R_CHANNEL(ch) { \
+ .type = IIO_VOLTAGE, \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
+ .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ), \
+ .output = 1, \
+ .indexed = 1, \
+ .channel = (ch), \
+ .scan_index = (ch), \
+ .scan_type = { \
+ .sign = 'u', \
+ .realbits = 16, \
+ .storagebits = 16, \
+ .endianness = IIO_BE, \
+ } \
+}
+
+static const struct iio_chan_spec ad3552r_hs_channels[] = {
+ AD3552R_CHANNEL(0),
+ AD3552R_CHANNEL(1),
+};
+
+static const struct iio_info ad3552r_hs_info = {
+ .read_raw = &ad3552r_hs_read_raw,
+ .write_raw = &ad3552r_hs_write_raw,
+};
+
+static int ad3552r_hs_probe(struct platform_device *pdev)
+{
+ struct ad3552r_hs_state *st;
+ struct iio_dev *indio_dev;
+ int ret;
+
+ indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*st));
+ if (!indio_dev)
+ return -ENOMEM;
+
+ st = iio_priv(indio_dev);
+ st->dev = &pdev->dev;
+
+ st->data = pdev->dev.platform_data;
+ if (!st->data)
+ dev_err_probe(st->dev, -ENODEV, "No platform data !");
+
+ st->back = devm_iio_backend_get(&pdev->dev, NULL);
+ if (IS_ERR(st->back))
+ return PTR_ERR(st->back);
+
+ ret = devm_iio_backend_enable(&pdev->dev, st->back);
+ if (ret)
+ return ret;
+
+ st->model_data = device_get_match_data(&pdev->dev);
+
+ indio_dev->name = "ad3552r";
+ indio_dev->modes = INDIO_DIRECT_MODE;
+ indio_dev->setup_ops = &ad3552r_hs_buffer_setup_ops;
+ indio_dev->channels = ad3552r_hs_channels;
+ indio_dev->num_channels = ARRAY_SIZE(ad3552r_hs_channels);
+ indio_dev->info = &ad3552r_hs_info;
+
+ ret = devm_iio_backend_request_buffer(&pdev->dev, st->back, indio_dev);
+ if (ret)
+ return ret;
+
+ ret = ad3552r_hs_setup(st);
+ if (ret)
+ return ret;
+
+ return devm_iio_device_register(&pdev->dev, indio_dev);
+}
+
+static const struct ad3552r_model_data ad3552r_model_data = {
+ .model_name = "ad3552r",
+ .chip_id = AD3552R_ID,
+ .num_hw_channels = 2,
+ .ranges_table = ad3552r_ch_ranges,
+ .num_ranges = ARRAY_SIZE(ad3552r_ch_ranges),
+};
+
+static const struct of_device_id ad3552r_hs_of_id[] = {
+ { .compatible = "adi,ad3552r", .data = &ad3552r_model_data },
+ { }
+};
+MODULE_DEVICE_TABLE(of, ad3552r_hs_of_id);
+
+static struct platform_driver axi_ad3552r_driver = {
+ .driver = {
+ .name = "ad3552r-axi",
+ .of_match_table = ad3552r_hs_of_id,
+ },
+ .probe = ad3552r_hs_probe,
+};
+module_platform_driver(axi_ad3552r_driver);
+
+MODULE_AUTHOR("Dragos Bogdan <dragos.bogdan@analog.com>");
+MODULE_AUTHOR("Angelo Dureghello <adueghello@baylibre.com>");
+MODULE_DESCRIPTION("AD3552R Driver - AXI IP version");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(IIO_AD3552R);
diff --git a/drivers/iio/dac/ad3552r.h b/drivers/iio/dac/ad3552r.h
index 088eb8ecfac6..fc00ed4c2565 100644
--- a/drivers/iio/dac/ad3552r.h
+++ b/drivers/iio/dac/ad3552r.h
@@ -38,6 +38,8 @@
#define AD3552R_REG_ADDR_TRANSFER_REGISTER 0x0F
#define AD3552R_MASK_MULTI_IO_MODE GENMASK(7, 6)
#define AD3552R_MASK_STREAM_LENGTH_KEEP_VALUE BIT(2)
+#define AD3552R_MASK_DUAL_SPI BIT(6)
+#define AD3552R_MASK_QUAD_SPI BIT(7)
#define AD3552R_REG_ADDR_INTERFACE_CONFIG_C 0x10
#define AD3552R_MASK_CRC_ENABLE (GENMASK(7, 6) |\
GENMASK(1, 0))
@@ -129,6 +131,11 @@
#define AD3552R_GAIN_SCALE 1000
#define AD3552R_LDAC_PULSE_US 100
+#define AD3552R_CH0_ACTIVE BIT(0)
+#define AD3552R_CH1_ACTIVE BIT(1)
+#define AD3552R_CH0_CH1_ACTIVE (AD3552R_CH0_ACTIVE | \
+ AD3552R_CH1_ACTIVE)
+
#define AD3552R_MAX_RANGES 5
#define AD3542R_MAX_RANGES 6
diff --git a/include/linux/platform_data/ad3552r-hs.h b/include/linux/platform_data/ad3552r-hs.h
new file mode 100644
index 000000000000..4e3213a0c73b
--- /dev/null
+++ b/include/linux/platform_data/ad3552r-hs.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (c) 2010-2024 Analog Devices Inc.
+ * Copyright (c) 2024 Baylibre, SAS
+ */
+#ifndef __LINUX_PLATFORM_DATA_AD3552R_HS_H__
+#define __LINUX_PLATFORM_DATA_AD3552R_HS_H__
+
+#include <linux/iio/backend.h>
+
+struct ad3552r_hs_platform_data {
+ int (*bus_reg_read)(struct iio_backend *back, u32 reg, u32 *val,
+ size_t data_size);
+ int (*bus_reg_write)(struct iio_backend *back, u32 reg, u32 val,
+ size_t data_size);
+};
+
+#endif /* __LINUX_PLATFORM_DATA_AD3552R_HS_H__ */
--
2.45.0.rc1
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v5 09/10] iio: dac: ad3552r: add high-speed platform driver
2024-10-08 15:43 ` [PATCH v5 09/10] iio: dac: ad3552r: add high-speed platform driver Angelo Dureghello
@ 2024-10-12 14:07 ` Jonathan Cameron
0 siblings, 0 replies; 26+ messages in thread
From: Jonathan Cameron @ 2024-10-12 14:07 UTC (permalink / raw)
To: Angelo Dureghello
Cc: Lars-Peter Clausen, Michael Hennerich, Nuno Sa, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Olivier Moysan,
Jonathan Cameron, linux-iio, linux-kernel, devicetree, dletchner,
Mark Brown
On Tue, 08 Oct 2024 17:43:41 +0200
Angelo Dureghello <adureghello@baylibre.com> wrote:
> From: Angelo Dureghello <adureghello@baylibre.com>
>
> Add High Speed ad3552r platform driver.
>
> The ad3552r DAC is controlled by a custom (fpga-based) DAC IP
> through the current AXI backend, or similar alternative IIO backend.
>
> Compared to the existing driver (ad3552r.c), that is a simple SPI
> driver, this driver is coupled with a DAC IIO backend that finally
> controls the ad3552r by a fpga-based "QSPI+DDR" interface, to reach
> maximum transfer rate of 33MUPS using dma stream capabilities.
>
> All commands involving QSPI bus read/write are delegated to the backend
> through the provided APIs for bus read/write.
>
> Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
> ---
> drivers/iio/dac/Kconfig | 14 +
> drivers/iio/dac/Makefile | 1 +
> drivers/iio/dac/ad3552r-hs.c | 526 +++++++++++++++++++++++++++++++
> drivers/iio/dac/ad3552r.h | 7 +
> include/linux/platform_data/ad3552r-hs.h | 18 ++
I'd missed this before. No obvious reason to put the include in a 'global' location
when for now at least only includers re in drivers/iio/dac/
So please move it there.
Some stuff include/linux/platform_data/ is ancient things dating back to board file
days.
Other than that and things bit Nuno raised, this series looks good to me.
Please drop the first 2 patches from v6 as I've applied those now.
Jonathan
> diff --git a/include/linux/platform_data/ad3552r-hs.h b/include/linux/platform_data/ad3552r-hs.h
> new file mode 100644
> index 000000000000..4e3213a0c73b
> --- /dev/null
> +++ b/include/linux/platform_data/ad3552r-hs.h
This isn't what we'd think of as 'platform_data' normally and it is only used
in drivers/iio/dac/ so I would simply move the header to drivers/iio/dac/ad3552r-hs.h
> @@ -0,0 +1,18 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (c) 2010-2024 Analog Devices Inc.
Generous on the years. This one should probably just be new copyright given
the code is effectively new.
> + * Copyright (c) 2024 Baylibre, SAS
> + */
> +#ifndef __LINUX_PLATFORM_DATA_AD3552R_HS_H__
> +#define __LINUX_PLATFORM_DATA_AD3552R_HS_H__
> +
> +#include <linux/iio/backend.h>
For this use a forwards reference
struct iio_backend;
Nice to avoid nests of includes where we can easily do so.
> +
> +struct ad3552r_hs_platform_data {
> + int (*bus_reg_read)(struct iio_backend *back, u32 reg, u32 *val,
> + size_t data_size);
> + int (*bus_reg_write)(struct iio_backend *back, u32 reg, u32 val,
> + size_t data_size);
> +};
> +
> +#endif /* __LINUX_PLATFORM_DATA_AD3552R_HS_H__ */
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v5 10/10] iio: dac: adi-axi-dac: add registering of child fdt node
2024-10-08 15:43 [PATCH v5 00/10] iio: add support for the ad3552r AXI DAC IP Angelo Dureghello
` (8 preceding siblings ...)
2024-10-08 15:43 ` [PATCH v5 09/10] iio: dac: ad3552r: add high-speed platform driver Angelo Dureghello
@ 2024-10-08 15:43 ` Angelo Dureghello
2024-10-10 13:51 ` Nuno Sá
2024-10-11 12:31 ` Nuno Sá
9 siblings, 2 replies; 26+ messages in thread
From: Angelo Dureghello @ 2024-10-08 15:43 UTC (permalink / raw)
To: Lars-Peter Clausen, Michael Hennerich, Nuno Sa, Jonathan Cameron,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Olivier Moysan
Cc: Jonathan Cameron, linux-iio, linux-kernel, devicetree, dletchner,
Mark Brown, Angelo Dureghello
From: Angelo Dureghello <adureghello@baylibre.com>
Change to obtain the fdt use case as reported in the
adi,ad3552r.yaml file in this patchset.
The DAC device is defined as a child node of the backend.
Registering the child fdt node as a platform devices.
Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
---
drivers/iio/dac/adi-axi-dac.c | 61 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 61 insertions(+)
diff --git a/drivers/iio/dac/adi-axi-dac.c b/drivers/iio/dac/adi-axi-dac.c
index e43d0ecccb50..754c4061d0e3 100644
--- a/drivers/iio/dac/adi-axi-dac.c
+++ b/drivers/iio/dac/adi-axi-dac.c
@@ -18,6 +18,7 @@
#include <linux/module.h>
#include <linux/mod_devicetable.h>
#include <linux/mutex.h>
+#include <linux/platform_data/ad3552r-hs.h>
#include <linux/platform_device.h>
#include <linux/property.h>
#include <linux/regmap.h>
@@ -108,6 +109,8 @@ struct axi_dac_info {
struct axi_dac_state {
struct regmap *regmap;
struct device *dev;
+ /* Target DAC platform device */
+ struct platform_device *dac_pdev;
/*
* lock to protect multiple accesses to the device registers and global
* data/variables.
@@ -750,6 +753,44 @@ static int axi_dac_bus_reg_read(struct iio_backend *back, u32 reg, u32 *val,
return regmap_read(st->regmap, AXI_DAC_CUSTOM_RD_REG, val);
}
+static void axi_dac_child_remove(void *data)
+{
+ struct axi_dac_state *st = data;
+
+ platform_device_unregister(st->dac_pdev);
+}
+
+static int axi_dac_create_platform_device(struct axi_dac_state *st,
+ struct fwnode_handle *child)
+{
+ struct ad3552r_hs_platform_data pdata = {
+ .bus_reg_read = axi_dac_bus_reg_read,
+ .bus_reg_write = axi_dac_bus_reg_write,
+ };
+ struct platform_device_info pi = {
+ .parent = st->dev,
+ .name = fwnode_get_name(child),
+ .id = PLATFORM_DEVID_AUTO,
+ .fwnode = child,
+ .data = &pdata,
+ .size_data = sizeof(pdata),
+ };
+ struct platform_device *pdev;
+ int ret;
+
+ pdev = platform_device_register_full(&pi);
+ if (IS_ERR(pdev))
+ return PTR_ERR(pdev);
+
+ st->dac_pdev = pdev;
+
+ ret = devm_add_action_or_reset(st->dev, axi_dac_child_remove, st);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
static const struct iio_backend_ops axi_dac_generic_ops = {
.enable = axi_dac_enable,
.disable = axi_dac_disable,
@@ -886,6 +927,26 @@ static int axi_dac_probe(struct platform_device *pdev)
return dev_err_probe(&pdev->dev, ret,
"failed to register iio backend\n");
+ if (st->info->bus_controller) {
+ device_for_each_child_node_scoped(&pdev->dev, child) {
+ int val;
+
+ /* Processing only reg 0 node */
+ ret = fwnode_property_read_u32(child, "reg", &val);
+ if (ret || val != 0)
+ continue;
+
+ ret = fwnode_property_read_u32(child, "io-backends",
+ &val);
+ if (ret)
+ continue;
+
+ ret = axi_dac_create_platform_device(st, child);
+ if (ret)
+ continue;
+ }
+ }
+
dev_info(&pdev->dev, "AXI DAC IP core (%d.%.2d.%c) probed\n",
ADI_AXI_PCORE_VER_MAJOR(ver),
ADI_AXI_PCORE_VER_MINOR(ver),
--
2.45.0.rc1
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v5 10/10] iio: dac: adi-axi-dac: add registering of child fdt node
2024-10-08 15:43 ` [PATCH v5 10/10] iio: dac: adi-axi-dac: add registering of child fdt node Angelo Dureghello
@ 2024-10-10 13:51 ` Nuno Sá
2024-10-11 12:31 ` Nuno Sá
1 sibling, 0 replies; 26+ messages in thread
From: Nuno Sá @ 2024-10-10 13:51 UTC (permalink / raw)
To: Angelo Dureghello, Lars-Peter Clausen, Michael Hennerich, Nuno Sa,
Jonathan Cameron, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Olivier Moysan
Cc: Jonathan Cameron, linux-iio, linux-kernel, devicetree, dletchner,
Mark Brown
On Tue, 2024-10-08 at 17:43 +0200, Angelo Dureghello wrote:
> From: Angelo Dureghello <adureghello@baylibre.com>
>
> Change to obtain the fdt use case as reported in the
> adi,ad3552r.yaml file in this patchset.
>
> The DAC device is defined as a child node of the backend.
> Registering the child fdt node as a platform devices.
>
> Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
> ---
> drivers/iio/dac/adi-axi-dac.c | 61
> +++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 61 insertions(+)
>
> diff --git a/drivers/iio/dac/adi-axi-dac.c b/drivers/iio/dac/adi-axi-dac.c
> index e43d0ecccb50..754c4061d0e3 100644
> --- a/drivers/iio/dac/adi-axi-dac.c
> +++ b/drivers/iio/dac/adi-axi-dac.c
> @@ -18,6 +18,7 @@
> #include <linux/module.h>
> #include <linux/mod_devicetable.h>
> #include <linux/mutex.h>
> +#include <linux/platform_data/ad3552r-hs.h>
> #include <linux/platform_device.h>
> #include <linux/property.h>
> #include <linux/regmap.h>
> @@ -108,6 +109,8 @@ struct axi_dac_info {
> struct axi_dac_state {
> struct regmap *regmap;
> struct device *dev;
> + /* Target DAC platform device */
> + struct platform_device *dac_pdev;
> /*
> * lock to protect multiple accesses to the device registers and
> global
> * data/variables.
> @@ -750,6 +753,44 @@ static int axi_dac_bus_reg_read(struct iio_backend *back,
> u32 reg, u32 *val,
> return regmap_read(st->regmap, AXI_DAC_CUSTOM_RD_REG, val);
> }
>
> +static void axi_dac_child_remove(void *data)
> +{
> + struct axi_dac_state *st = data;
> +
> + platform_device_unregister(st->dac_pdev);
> +}
> +
> +static int axi_dac_create_platform_device(struct axi_dac_state *st,
> + struct fwnode_handle *child)
> +{
> + struct ad3552r_hs_platform_data pdata = {
> + .bus_reg_read = axi_dac_bus_reg_read,
> + .bus_reg_write = axi_dac_bus_reg_write,
> + };
> + struct platform_device_info pi = {
> + .parent = st->dev,
> + .name = fwnode_get_name(child),
> + .id = PLATFORM_DEVID_AUTO,
> + .fwnode = child,
> + .data = &pdata,
> + .size_data = sizeof(pdata),
> + };
> + struct platform_device *pdev;
> + int ret;
> +
> + pdev = platform_device_register_full(&pi);
> + if (IS_ERR(pdev))
> + return PTR_ERR(pdev);
> +
> + st->dac_pdev = pdev;
Don't need to save it in the state struct. Pass it directly to
devm_add_action_or_reset()
> +
> + ret = devm_add_action_or_reset(st->dev, axi_dac_child_remove, st);
> + if (ret)
> + return ret;
> +
> + return 0;
return devm_add_action_or_reset();
> +}
> +
> static const struct iio_backend_ops axi_dac_generic_ops = {
> .enable = axi_dac_enable,
> .disable = axi_dac_disable,
> @@ -886,6 +927,26 @@ static int axi_dac_probe(struct platform_device *pdev)
> return dev_err_probe(&pdev->dev, ret,
> "failed to register iio backend\n");
>
> + if (st->info->bus_controller) {
I guess for now all child nodes that the IP has are for this usecase so I would
just assume it and drop the bus_controller. Let's deal with something different
when the usecase for it pops up. In any case, the flag is only needed in this
patch so it should only be introduced now.
> + device_for_each_child_node_scoped(&pdev->dev, child) {
> + int val;
> +
> + /* Processing only reg 0 node */
> + ret = fwnode_property_read_u32(child, "reg", &val);
> + if (ret || val != 0)
> + continue;
The conditions are not really related so I would not mix them:
if (ret)
return dev_err_probe(); // some logs might be helpful...
if (val > 0)
return dev_err_probe();
> +
> + ret = fwnode_property_read_u32(child, "io-backends",
> + &val);
> + if (ret)
> + continue;
The above looks redundant...
> +
> + ret = axi_dac_create_platform_device(st, child);
> + if (ret)
> + continue;
Should we really ignore all errors?
> + }
> + }
> +
> dev_info(&pdev->dev, "AXI DAC IP core (%d.%.2d.%c) probed\n",
> ADI_AXI_PCORE_VER_MAJOR(ver),
> ADI_AXI_PCORE_VER_MINOR(ver),
>
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v5 10/10] iio: dac: adi-axi-dac: add registering of child fdt node
2024-10-08 15:43 ` [PATCH v5 10/10] iio: dac: adi-axi-dac: add registering of child fdt node Angelo Dureghello
2024-10-10 13:51 ` Nuno Sá
@ 2024-10-11 12:31 ` Nuno Sá
1 sibling, 0 replies; 26+ messages in thread
From: Nuno Sá @ 2024-10-11 12:31 UTC (permalink / raw)
To: Angelo Dureghello, Lars-Peter Clausen, Michael Hennerich, Nuno Sa,
Jonathan Cameron, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Olivier Moysan
Cc: Jonathan Cameron, linux-iio, linux-kernel, devicetree, dletchner,
Mark Brown
On Tue, 2024-10-08 at 17:43 +0200, Angelo Dureghello wrote:
> From: Angelo Dureghello <adureghello@baylibre.com>
>
> Change to obtain the fdt use case as reported in the
> adi,ad3552r.yaml file in this patchset.
>
> The DAC device is defined as a child node of the backend.
> Registering the child fdt node as a platform devices.
>
> Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
> ---
> drivers/iio/dac/adi-axi-dac.c | 61
> +++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 61 insertions(+)
>
> diff --git a/drivers/iio/dac/adi-axi-dac.c b/drivers/iio/dac/adi-axi-dac.c
> index e43d0ecccb50..754c4061d0e3 100644
> --- a/drivers/iio/dac/adi-axi-dac.c
> +++ b/drivers/iio/dac/adi-axi-dac.c
> @@ -18,6 +18,7 @@
> #include <linux/module.h>
> #include <linux/mod_devicetable.h>
> #include <linux/mutex.h>
> +#include <linux/platform_data/ad3552r-hs.h>
> #include <linux/platform_device.h>
> #include <linux/property.h>
> #include <linux/regmap.h>
> @@ -108,6 +109,8 @@ struct axi_dac_info {
> struct axi_dac_state {
> struct regmap *regmap;
> struct device *dev;
> + /* Target DAC platform device */
> + struct platform_device *dac_pdev;
> /*
> * lock to protect multiple accesses to the device registers and
> global
> * data/variables.
> @@ -750,6 +753,44 @@ static int axi_dac_bus_reg_read(struct iio_backend *back,
> u32 reg, u32 *val,
> return regmap_read(st->regmap, AXI_DAC_CUSTOM_RD_REG, val);
> }
>
> +static void axi_dac_child_remove(void *data)
> +{
> + struct axi_dac_state *st = data;
> +
> + platform_device_unregister(st->dac_pdev);
> +}
> +
> +static int axi_dac_create_platform_device(struct axi_dac_state *st,
> + struct fwnode_handle *child)
> +{
> + struct ad3552r_hs_platform_data pdata = {
> + .bus_reg_read = axi_dac_bus_reg_read,
> + .bus_reg_write = axi_dac_bus_reg_write,
> + };
> + struct platform_device_info pi = {
> + .parent = st->dev,
> + .name = fwnode_get_name(child),
> + .id = PLATFORM_DEVID_AUTO,
> + .fwnode = child,
also device_set_node() for this
- Nuno Sá
^ permalink raw reply [flat|nested] 26+ messages in thread