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

Hi Jonas,

On 8/12/26 7:08 PM, Jonas Karlman wrote:
> Hi Quentin,
> 
> On 8/12/2026 6:48 PM, Quentin Schulz wrote:
>> 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 :)
> 
> I did a quick 'adc scan' test on my ArmSoM Forge1.
> 
> With '.num_bits = 10':
> 
>    => adc scan adc@ff4e8000
>    [00]: 1019, 1792961 uV
>    [01]: 1019, 1792961 uV
>    [02]: 0, 0 uV
>    [03]: 1011, 1778885 uV
> 
> And with '.num_bits = 12':
> 
>    => adc scan adc@ff4e8000
>    [00]: 1019, 447912 uV
>    [01]: 1019, 447912 uV
>    [02]: 0, 0 uV
>    [03]: 1011, 444395 uV
> 
> RK3506G_REF_V11_20241114 lists ADC 0-1023 in the SARADC_IN0_BOOT table,
> with 1023 for FSPI--SDMMC(eMMC/SD Card)--USB/SPI2APB.
> 
> So with this data I think 10-bit should be correct, without having to
> measure or apply 1.8v on an ADC pin.
> 

Yeah, that would be around 1.8V on channel 0.

Looknig at the schematics, you have a pull-up to VCC_1V8 so that checks 
out. Thanks for checking.

With the commit log fixed (misleading divider for only one parent, 
GHz->MHz, + add a small word that the fallback comes from the Device 
Tree binding):

Reviewed-by: Quentin Schulz <[email protected]>

Thanks!
Quentin