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

Hi Jonas,

On 8/12/26 6:20 PM, Jonas Karlman wrote:
> Hi Quentin,
> 
> On 8/12/2026 12:33 PM, Quentin Schulz wrote:
>> Hi Jonas,
>>
>> On 8/3/26 9:09 PM, Jonas Karlman wrote:
>>> Current rockchip,rk3528-saradc fallback instruct the driver to use a
>>
>> There is no rk3506 DTSI available either in Linux kernel or U-Boot, so
>> this fallback technically doesn't even exist and we don't have an issue
>> (yet). Just say that none of the the v2 SARADC data match what the
>> RK3506 can do (due to the divider and parent clocks not being able to
>> achieve the requested 1MHz). Please simply rename the commit title to
>> reflect you're adding support for the SARADC found on the RK3506.
> 
> Sure, I will update the commit message in a v2.
> 
> The rk3528 fallback mentioned was in reference to the dt-bindings:
> 
>        - items:
>            - const: rockchip,rk3506-saradc
>            - const: rockchip,rk3528-saradc
> 

Ha! Completely missed that, thanks the clarification.

>>
>>> 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.
>>>
>> This is quite misleading. The parent clocks are 32KHz, 400KHz or 24MHz.
>> The divider is between 1 and 16.
> 
> You are correct, I must have mixed up this clock with a different clock
> that had multiple parent options and the divider was for one of those
> parent options.
> 
>>
>> This means that 1MHz is not achievable with the dividers and we need to
>> aim for something else. Please reword the commit log to reflect this.
>> I'm also not sure where people got 32KHz, 400KHz and 24MHz but it's
>> what's specified in the Linux kernel, so fine with me :)
>>
>>> 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
>>> index 1515951403c9..50861b61eea4 100644
>>> --- 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 = {
>>>    	.stop = rockchip_saradc_stop_v1,
>>>    };
>>>    
>>> +static const struct rockchip_saradc_data rk3506_saradc_data = {
>>> +	.num_bits = 10,
>>
>> It has 12 bits according to the TRM.
> 
> Hum, in the datasheet for RK3506J v1.2, RK3506B v1.4, RK3506G2 v1.5 and
> RK3506 TRM v1.1/v1.2 it lists 10-bit resolution.
> 

Well, I read the register description and it says 11:0. But the first 
sentence in the SARADC section says "10-bit"... sooooo /me shrugs. 
Ideally, we should check whether the ADC returns close to 1023 (10 bits) 
or 4095 (12 bits) when it's fed 1.8V and then we would know for sure :)

Cheers,
Quentin