Re: [PATCH v3 2/2] iio: adc: ad7192: Fix clock config

["Nuno Sá" <noname.nuno@gmail.com>]

On Wed, 2024-06-12 at 17:16 +0300, Alisa-Dariana Roman wrote:
> There are actually 4 configuration modes of clock source for AD719X
> devices. Either a crystal can be attached externally between MCLK1 and
> MCLK2 pins, or an external CMOS-compatible clock can drive the MCLK2
> pin. The other 2 modes make use of the 4.92MHz internal clock.
> 
> Note that the fix tag is for the commit that moved the driver out of
> staging.
> 
> Fixes: b581f748cce0 ("staging: iio: adc: ad7192: move out of staging")
> Signed-off-by: Alisa-Dariana Roman <alisa.roman@analog.com>
> ---

Hmmm, I did not looked at the datasheet but looked at the older implementation
and I'm not sure this is an actual fix. Can you elaborate on that?

So on the current implementation I can see that we have some properties that are
not documented:

adi,int-clock-output-enable
adi,clock-xtal

So, I see in your series that you're documenting adi,clock-xtal using clk-names.
I do think your code is cleaner but I don't think the older implementation to be
buggy. Am I missing something?

I can also see that you're ignoring AD7192_CLK_INT_CO... That's also removing
functionality from the driver even though the implementation is not as it should
be I think. If I understand that mode correctly, it's just about having the
internal clock in the MCLK pin. Effectively this would then be a clock provider
with a fixed rate of 4.92MHz. So I believe that exposing it as a clock provider
would likely be the way to go.

- Nuno Sá