Hi Simon,
On 10/31/2026 10:36 AM, Simon Glass wrote:
> Hi Jonas,
>
> On 2026-10-11T22:28:36, Jonas Karlman <[email protected]> wrote:
>> adc: rockchip-saradc: Use 3 MHz clock rate for RK3528
>>
>> The RK3528 CLK_SARADC divider field stores the divisor minus one.
>> Its three bits encode divisors 1-8, allowing rates of 24, 12, 8, 6,
>> 4.8, 4, ~3.43 or 3 MHz from the 24 MHz oscillator.
>>
>> When the driver requests 1 MHz rate, the required divisor of 24 is
>> encoded as 23. This value is truncated to 7 in the three-bit field,
>> selecting a divisor of 8 and producing a 3 MHz clock.
>
> Just to check, is this really just truncation? rk3528_adc_set_clk()
> calls rk_clrsetreg(&cru->clksel_con[74], mask, (div - 1) << shift) and
> rk_clrsetreg() does not apply the mask to 'set', so the write is ((0x7
> | 0x17) << 16) | 0x17, which also write-enables and sets bit 4. Bit 4
> is in the CLK_TSADC divider field (bits 7:3), so each SARADC probe has
> also been changing the TSADC divider. That is a better reason for this
> patch than matching the rate, so please can you mention it in the
> commit message.
The motivation from the saradc driver perspective is that it should
request a rate that is supported by the clock, not to work around
shortcomings in the clock driver.
I do not think there is a reason to mention the internal clock driver
issues, those will be handled in a separate refactoring series that
changes clock driver to use FIELD_GET/FIELD_PREP macros and make
set_rate fail for rates that does not FIELD_FIT.
>
> I suppose you could rk3528_adc_set_clk() clamp the divider to the
> field width, so that an out-of-range request can't corrupt the
> neighbouring field. Bad for code size though.
I have already converted all RK35xx clock drivers to use FIELD-macros in
my local tree. Such refactoring will be sent as follow-up series as
mentioned in cover letter. This series tries to fix issues that were
discovered when the clock drivers were initially converted/refactored.
Regards,
Jonas
>
>>
>> Request 3 MHz explicitly to match the rate already used after value
>> truncation.
>>
>> Fixes: 6ef723af18b9 ("adc: rockchip-saradc: Add support for RK3528")
>> Signed-off-by: Jonas Karlman <[email protected]>
>>
>> drivers/adc/rockchip-saradc.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> Reviewed-by: Simon Glass <[email protected]>
