Re: [RFC PATCH 0/3] ad9832: driver cleanup

[Tomas Borquez <tomasborquez13@gmail.com>]

On Sat, Dec 06, 2025 at 04:09:33PM +0000, Jonathan Cameron wrote:
> On Fri,  5 Dec 2025 17:27:40 -0300
> Tomas Borquez <tomasborquez13@gmail.com> wrote:

Hey Jonathan, thanks for all the feedback!

> Opening question for a cleanup of a driver like this is how you plan
> to test it. Do you have the hardware, or are you emulating / stubbing
> functions to test it? It is very brave to take on major refactoring
> without a good way to test.
> 
> I was kind of planning to drop this driver this cycle on basis of no
> interest in sorting it out, but clearly you are interested so great
> as long as we can be sure it works well after your work on it
> (or indeed that it works currently!)

I don't have the hardware, so I've been testing with a stubbed SPI layer.
I created a platform device wrapper that registers the IIO device without
real SPI and replaced spi_sync() with a function that dumps the TX buffer
via print_hex_dump() which allows me verify the register protocol against
the datasheet, I validated:

- Frequency tuning word calculation (32-bit, formula from datasheet)
- Phase register writes (12-bit values across two transfers)
- Control register state machine (SLEEPRESCLR, FPSELECT commands)
- Proper preservation of ctrl_fp bits when updating freq_symbol vs
phase_symbol independently
- Input validation (bounds checking on phase/frequency values)

All the SPI command bytes match what the datasheet specifies. Obviously
this can't catch electrical issues or timing problems that would only show
up with real hardware but it does verify the driver's logic and register
protocol are correct.

I also ran it through sparse, smatch, and checkpatch with no warnings.
I can share the test module + logs if that's useful for review.

...

> > 2) Scale Attributes
> > 
> >    The frequency scale is 1 Hz and phase scale is 2*PI/4096 radians.
> >    I cannot use info_mask_shared_by_type for IIO_CHAN_INFO_SCALE because
> >    all channels share IIO_ALTVOLTAGE.
> > 
> >    So instead I'm using IIO_CONST_ATTR for the scales:
> > 
> >      out_altvoltage_frequency_scale = "1"
> >      out_altvoltage_phase_scale = "0.0015339808"
> > 
> >    Is there a better approach here? Or should I just document the units and
> >    skip scale attributes entirely?
> 
> Good question.  I think right option is to just do the maths in the driver and
> have out_altvoltage0_frequencyN take the scaled value rather than the register
> value.  Then do some fixed point maths to get to the required register value.

Which is already done in write_frequency(), should I do it with phase too?
And should I accept radians or microradians?

...

I've been working on v2 based on your ABI feedback. If the testing approach
sounds acceptable I'll send it out and if someone with hardware wants to
test it, that would be great.

...

Tomas