mbox series

[v9,0/8] iio: add support for the ad3552r AXI DAC IP

Message ID 20241028-wip-bl-ad3552r-axi-v0-iio-testing-v9-0-f6960b4f9719@kernel-space.org (mailing list archive)
Headers show
Series iio: add support for the ad3552r AXI DAC IP | expand

Message

Angelo Dureghello Oct. 28, 2024, 9:45 p.m. UTC
Purpose is to add ad3552r AXI DAC (fpga-based) support.

The "ad3552r" AXI IP, a variant of the generic "DAC" AXI IP,
has been created to reach the maximum speed (33MUPS) supported
from the ad3552r. To obtain the maximum transfer rate, a custom
IP core module has been implemented with a QSPI interface with
DDR (Double Data Rate) mode.

The design is actually using the DAC backend since the register
map is the same of the generic DAC IP, except for some customized
bitfields. For this reason, a new "compatible" has been added
in adi-axi-dac.c.

Also, backend has been extended with all the needed functions
for this use case, keeping the names gneric.

The following patch is actually applying to linux-iio/testing.

---
Changes in v2:
- use unsigned int on bus_reg_read/write
- add a compatible in axi-dac backend for the ad3552r DAC IP
- minor code alignment fixes
- fix a return value not checked
- change devicetree structure setting ad3552r-axi as a backend
  subnode
- add synchronous_mode_available in the ABI doc

Changes in v3:
- changing AXI backend approach using a dac ip compatible
- fdt bindings updates accordingly
- fdt, ad3552r device must be a subnode of the backend
- allow probe of child devices
- passing QSPI bus access function by platform data
- move synchronous mode as a fdt parameter
- reorganizing defines in proper patches
- fix make dt_binding_check errors
- fix ad3552r maximum SPI speed
- fix samplerate calulcation
- minor code style fixes

Changes in v4:
- fix Kconfig
- fix backend documentation
- driver renamed to a more gneric "high speed" (ad3552r-hs)
- restyled axi-dac register names
- removed synchronous support, dead code
  (could be added in the future with David sugestions if needed)
- renaming backend buffer enable/disable calls
- using model_data in common code
- using devm_add_action_or_reset
- minor code style fixes

Changes in v5:
- patch 2/11 set before fix of ADI_DAC_R1_MODE patch
- fix dt binding check error
- patch 4/11 removed
- fix stream enable/disable call names
- fix axi-dac clock names
- fix axi-dac platform device unregistering
- minor code style fixes

Changes in v6:
- remove patches (fixes) already accepted
- move platform data include in drivers/iio/dac dir
- minor notes added to commit description
- fix axi-dac platform child-device creation
- minor code style fixes

Changes in v7:
- add per channel offset and scale
- change dac clock rate considering always buffering and DDR 
- fix axi-dac fdt property conditionals 
- fix getting clocks from high-speed driver, with a NULL as fallback
- fix missing dma buffer free callback
- minor code style fixes

Changes in v8:
- getting sclk from platform data
- fix axi-dac yaml bindings
- fix reset logic to active-low
- remove dev_err_probe messages from gain/offset calc
- minor code style fixes

Changes in v9:
- add locking to axi dac reg_read/write
- fix dac Kconfig to have a separate common library
- minor code style fixes

Signed-off-by: Angelo Dureghello <angelo@kernel-space.org>

---
Angelo Dureghello (8):
      dt-bindings: iio: dac: ad3552r: add iio backend support
      dt-bindings: iio: dac: adi-axi-dac: add ad3552r axi variant
      iio: backend: extend features
      iio: dac: adi-axi-dac: extend features
      iio: dac: ad3552r: changes to use FIELD_PREP
      iio: dac: ad3552r: extract common code (no changes in behavior intended)
      iio: dac: ad3552r: add high-speed platform driver
      iio: dac: adi-axi-dac: add registering of child fdt node

 .../devicetree/bindings/iio/dac/adi,ad3552r.yaml   |   7 +
 .../devicetree/bindings/iio/dac/adi,axi-dac.yaml   |  69 ++-
 drivers/iio/dac/Kconfig                            |  19 +
 drivers/iio/dac/Makefile                           |   2 +
 drivers/iio/dac/ad3552r-common.c                   | 249 +++++++++
 drivers/iio/dac/ad3552r-hs.c                       | 529 +++++++++++++++++++
 drivers/iio/dac/ad3552r-hs.h                       |  19 +
 drivers/iio/dac/ad3552r.c                          | 557 +++------------------
 drivers/iio/dac/ad3552r.h                          | 228 +++++++++
 drivers/iio/dac/adi-axi-dac.c                      | 312 +++++++++++-
 drivers/iio/industrialio-backend.c                 |  78 +++
 include/linux/iio/backend.h                        |  17 +
 12 files changed, 1585 insertions(+), 501 deletions(-)
---
base-commit: e469c6c036907dec25c3979f527a102e44d6f9b8
change-id: 20241028-wip-bl-ad3552r-axi-v0-iio-testing-7dc193405779

Best regards,

Comments

David Lechner Oct. 29, 2024, 7:08 p.m. UTC | #1
On 10/28/24 4:45 PM, Angelo Dureghello wrote:
> Purpose is to add ad3552r AXI DAC (fpga-based) support.
> 
> The "ad3552r" AXI IP, a variant of the generic "DAC" AXI IP,
> has been created to reach the maximum speed (33MUPS) supported
> from the ad3552r. To obtain the maximum transfer rate, a custom
> IP core module has been implemented with a QSPI interface with
> DDR (Double Data Rate) mode.
> 
> The design is actually using the DAC backend since the register
> map is the same of the generic DAC IP, except for some customized
> bitfields. For this reason, a new "compatible" has been added
> in adi-axi-dac.c.
> 
> Also, backend has been extended with all the needed functions
> for this use case, keeping the names gneric.
> 
> The following patch is actually applying to linux-iio/testing.
> 
> ---
Reviewed-by: David Lechner <dlechner@baylibre.com>
Jonathan Cameron Oct. 31, 2024, 9:26 p.m. UTC | #2
On Tue, 29 Oct 2024 14:08:15 -0500
David Lechner <dlechner@baylibre.com> wrote:

> On 10/28/24 4:45 PM, Angelo Dureghello wrote:
> > Purpose is to add ad3552r AXI DAC (fpga-based) support.
> > 
> > The "ad3552r" AXI IP, a variant of the generic "DAC" AXI IP,
> > has been created to reach the maximum speed (33MUPS) supported
> > from the ad3552r. To obtain the maximum transfer rate, a custom
> > IP core module has been implemented with a QSPI interface with
> > DDR (Double Data Rate) mode.
> > 
> > The design is actually using the DAC backend since the register
> > map is the same of the generic DAC IP, except for some customized
> > bitfields. For this reason, a new "compatible" has been added
> > in adi-axi-dac.c.
> > 
> > Also, backend has been extended with all the needed functions
> > for this use case, keeping the names gneric.
> > 
> > The following patch is actually applying to linux-iio/testing.
> > 
> > ---  
> Reviewed-by: David Lechner <dlechner@baylibre.com>
> 

Series applied with 2 tweaks as called out in replies to individual
patches. Pushed out initially as testing for 0-day to see if there
are any issues my (admittedly now very lazy) build tests didn't find.

Thanks,

Jonathan
Jonathan Cameron Nov. 1, 2024, 2:46 p.m. UTC | #3
On Thu, 31 Oct 2024 21:26:26 +0000
Jonathan Cameron <jic23@kernel.org> wrote:

> On Tue, 29 Oct 2024 14:08:15 -0500
> David Lechner <dlechner@baylibre.com> wrote:
> 
> > On 10/28/24 4:45 PM, Angelo Dureghello wrote:  
> > > Purpose is to add ad3552r AXI DAC (fpga-based) support.
> > > 
> > > The "ad3552r" AXI IP, a variant of the generic "DAC" AXI IP,
> > > has been created to reach the maximum speed (33MUPS) supported
> > > from the ad3552r. To obtain the maximum transfer rate, a custom
> > > IP core module has been implemented with a QSPI interface with
> > > DDR (Double Data Rate) mode.
> > > 
> > > The design is actually using the DAC backend since the register
> > > map is the same of the generic DAC IP, except for some customized
> > > bitfields. For this reason, a new "compatible" has been added
> > > in adi-axi-dac.c.
> > > 
> > > Also, backend has been extended with all the needed functions
> > > for this use case, keeping the names gneric.
> > > 
> > > The following patch is actually applying to linux-iio/testing.
> > > 
> > > ---    
> > Reviewed-by: David Lechner <dlechner@baylibre.com>
> >   
> 
> Series applied with 2 tweaks as called out in replies to individual
> patches. Pushed out initially as testing for 0-day to see if there
> are any issues my (admittedly now very lazy) build tests didn't find.
 [jic23-iio:testing 104/113]  drivers/iio/dac/ad3552r-hs.c:400:17: warning: variable 'goffs' is  uninitialized when used here

was a report from 0-day. 
I 'think' the fix (based on v6 which is last time anything set this)
is the following and that's what I'll push out for now.

If it should be something else send me a patch on top of my testing branch
so I can quickly apply it.

I've also dropped the explicit select of ADI_AXI_DAC as you can only do that
if you also carry all it's dependencies. (another 0-day report)

Thanks as ever to the lkp team for their incredibly useful service!



diff --git a/drivers/iio/dac/ad3552r-hs.c b/drivers/iio/dac/ad3552r-hs.c
index 97dfc598aec6..7a5b277ee370 100644
--- a/drivers/iio/dac/ad3552r-hs.c
+++ b/drivers/iio/dac/ad3552r-hs.c
@@ -295,7 +295,6 @@ static int ad3552r_hs_setup_custom_gain(struct ad3552r_hs_state *st,
 
 static int ad3552r_hs_setup(struct ad3552r_hs_state *st)
 {
-       s16 goffs;
        u16 id;
        u16 gain = 0, offset = 0;
        u32 ch, val, range;
@@ -397,7 +396,7 @@ static int ad3552r_hs_setup(struct ad3552r_hs_state *st)
                        gain = ad3552r_calc_custom_gain(st->ch_data[ch].p,
                                                st->ch_data[ch].n,
                                                st->ch_data[ch].gain_offset);
-                       offset = abs(goffs);
+                       offset = abs(st->ch_data[ch].gain_offset);
 
                        st->ch_data[ch].range_override = 1;


> 
> Thanks,
> 
> Jonathan
>