Re: [PATCH v1 2/3] iio: adc: meson: consistently use bool/enum in struct meson_sar_adc_param

[George Stark]

Hello Martin

On 12/24/24 17:37, Martin Blumenstingl wrote:
> Hi George,
>
> On Tue, Mar 26, 2024 at 1:41 AM George Stark <gnstark@xxxxxxxxxxxxxxxxx> wrote:
> [...]
> > >    enum meson_sar_adc_avg_mode {
> > >        NO_AVERAGING = 0x0,
> > >        MEAN_AVERAGING = 0x1,
> > > @@ -321,13 +326,13 @@ struct meson_sar_adc_param {
> > >        u8                                      temperature_trimming_bits;
> > >        unsigned int                            temperature_multiplier;
> > >        unsigned int                            temperature_divider;
> > > -     u8                                      disable_ring_counter;
> > > +     bool                                    disable_ring_counter;
> > >        bool                                    has_reg11;
> > >        bool                                    has_vref_select;
> > > -     u8                                      vref_select;
> > > -     u8                                      cmv_select;
> > > -     u8                                      adc_eoc;
> >
> >
> > The reason to choose u8 type over bool was that those are not actually
> > bool values but direct values of hw register bits. We have little
> > information about real meaning of these bits so it won't help much to
> > add bool layer and keep real values in the init code instead of param
> > section (adc_eoc, cmv_select).
> > bool disable_ring_counter will look deceptive too because it doesn't
> > say whether disable ring_counter or not (we always disable it) but
> > how to disable it (write 0 or 1)
> Do you have any contact at Amlogic that can help explain the purpose
> of these bits?

I remember I was digging thru vendor kernel and uboot code to understand 
the meaning of those bits when prepared my adc patches. I'll check it 
again about these and bellow bits and will let you know if I find 
anything worthwhile.

> I tried reaching out to my contact off-list but did not get any answers.
>
> I also have more questions about the SARADC IP block on the GXLX SoC.
> We (Christian and I) have a patch prepared to fix audio (or rather the
> MPLL clocks) on the GXLX SoC (see [0]). Unfortunately there seems to
> be nobody that's answering me with enough info so we can upstream that
> patch.
>
> So if you have any contact or insights into datasheets - please check
> and see if you can provide information so we can improve the naming of
> these bits and hopefully get GXLX SoC support fixed.
>
> Also +Cc Xingyu Chen and Zelong Dong - I found your names in
> downstream patches to the SARADC driver [1].
>
>
> Thank you and best regards,
> Martin
>
>
> [0] https://lore.kernel.org/linux-amlogic/20240604055431.3313961-2-christianshewitt@xxxxxxxxx/
> [1] https://github.com/khadas/linux/commit/13321d23231a7a4d08f4fefebcef11b379475a37

--
Best regards
George