Re: [PATCH 3/6] adc: rockchip-saradc: Add driver data for RK3506

Hi Jonas,

On Sat, 8 Aug 2026 at 14:20, Jonas Karlman <[email protected]> wrote:
>
> Hi Simon,
>
> On 8/8/2026 7:39 PM, Simon Glass wrote:
> > Hi Jonas,
> >
> > On 2026-08-03T19:09:38, Jonas Karlman <[email protected]> wrote:
> >> adc: rockchip-saradc: Add driver data for RK3506
> >>
> >> Current rockchip,rk3528-saradc fallback instruct the driver to use a
> >> 1 GHz clock rate. However, the CLK_SARADC used on RK3506 can only be
> >> configured to use a 32 KHz, 400 KHz or 1.5-24 GHz rate.
> >
> >>
> >> Add driver data for rockchip,rk3506-saradc that request use of a 1.5 GHz
> >> clock rate, to avoid clock divider overflow in the RK3506 clock driver.
> >
> > Units look wrong throughout - clk_rate is in Hz, so 1000000 is 1 MHz
> > and 1500000 is 1.5 MHz, not GHz. Please s/GHz/MHz/ in all four places.
> > Also 'instruct' -> 'instructs' and 'request' -> 'requests'.
>
> Hehe, you are correct, it should be MHz and not GHz :-)
>
> >
> >>
> >> Add driver data for rockchip,rk3506-saradc that request use of a 1.5 GHz
> >> clock rate, to avoid clock divider overflow in the RK3506 clock driver.
> >>
> >> Signed-off-by: Jonas Karlman <[email protected]>
> >>
> >> drivers/adc/rockchip-saradc.c | 10 ++++++++++
> >>  1 file changed, 10 insertions(+)
> >
> >> diff --git a/drivers/adc/rockchip-saradc.c b/drivers/adc/rockchip-saradc.c
> >> @@ -339,6 +339,14 @@ static const struct rockchip_saradc_data rk3399_saradc_data = {
> >> +static const struct rockchip_saradc_data rk3506_saradc_data = {
> >> +     .num_bits = 10,
> >> +     .num_channels = 4,
> >> +     .clk_rate = 1500000,
> >> +     .channel_data = rockchip_saradc_channel_data_v2,
> >> +     .start_channel = rockchip_saradc_start_channel_v2,
> >> +};
> >
> > Just to check - the other v2 entries (rk3528, rk3588) also omit .stop,
> > so this matches the existing pattern. Is that deliberate because v2
> > hardware auto-stops, or is a stop_v2 callback missing? A brief note in
> > the commit message would help.
>
> To my knowledge the v1 needs to be power up/down, hence start/stop. And
> the v2 only need to do initial configuration for single read, and the
> interrupt is acknowledged as part channel_data, after first waiting for
> said interrupt and reading out the data. As you mention this is just a
> copy of the rk3528 (fallback) driver data, so not sure what you want me
> to mention related to this.

Well it makes sense now. Since you are redoing this you could just
mention that the start/stop is not needed.

Regards,
Simon