[U-Boot] [U-Boot, 1/3] rockchip: boot mode: parse adc channel from dts

[Dr. Philipp Tomsich]

Simon,

could you comment on this one from a general U-Boot architecture and DM-maintainer perspective?

Thanks,
Philipp.

> On 29 Nov 2017, at 10:50, Dr. Philipp Tomsich <philipp.tomsich at theobroma-systems.com> wrote:
> 
> 
>> On 29 Nov 2017, at 07:34, Andy Yan <andy.yan at rock-chips.com <mailto:andy.yan at rock-chips.com>> wrote:
>> 
>> Hi Philipp:
>> 
>> 
>> On 2017年11月28日 21:59, Philipp Tomsich wrote:
>>> +sjg
>>> 
>>> On Tue, 28 Nov 2017, Andy Yan wrote:
>>> 
>>>> Most the current rockchip based boards use adc channel
>>>> 1 detect the download key, but there are also some
>>>> boards like rv1108 based plaform use adc channel 0.
>>>> So we parse the adc channel from dts if we can get
>>>> it, otherwise we use the channel 1 as default.
>>>> 
>>>> Signed-off-by: Andy Yan <andy.yan at rock-chips.com <mailto:andy.yan at rock-chips.com>>
>>>> Acked-by: Philipp Tomsich <philipp.tomsich at theobroma-systems.com <mailto:philipp.tomsich at theobroma-systems.com>>
>>>> ---
>>>> 
>>>> arch/arm/mach-rockchip/boot_mode.c | 15 ++++++++++++++-
>>>> 1 file changed, 14 insertions(+), 1 deletion(-)
>>>> 
>>>> diff --git a/arch/arm/mach-rockchip/boot_mode.c b/arch/arm/mach-rockchip/boot_mode.c
>>>> index 942849f..49dfd39 100644
>>>> --- a/arch/arm/mach-rockchip/boot_mode.c
>>>> +++ b/arch/arm/mach-rockchip/boot_mode.c
>>>> @@ -8,6 +8,9 @@
>>>> #include <adc.h>
>>>> #include <asm/io.h>
>>>> #include <asm/arch/boot_mode.h>
>>>> +#include <fdtdec.h>
>>>> +
>>>> +DECLARE_GLOBAL_DATA_PTR;
>>>> 
>>>> void set_back_to_bootrom_dnl_flag(void)
>>>> {
>>>> @@ -26,9 +29,19 @@ void set_back_to_bootrom_dnl_flag(void)
>>>> 
>>>> __weak int rockchip_dnl_key_pressed(void)
>>>> {
>>>> +    const void *blob = gd->fdt_blob;
>>>>     unsigned int val;
>>>> +    int channel = 1;
>>>> +    int node;
>>>> +    u32 chns[2];
>>>> +
>>>> +    node = fdt_node_offset_by_compatible(blob, 0, "adc-keys");
>>>> +    if (node >= 0) {
>>>> +        if (!fdtdec_get_int_array(blob, node, "io-channels", chns, 2))
>>>> +            channel = chns[1];
>>>> +    }
>>> 
>>> The driver for 'adc-keys' should be a driver in drivers/input that can then be retrieved via DM and queried using keyboard_getc().
>> 
>>     Yes, if there is an exiting adc-keys driver, we will use it here with no doubts, but there is not now. I just need  to check the button
>> down event once, no up or other things needed, so I call the adc api directly. I grep all the u-boot project, and found all other boards
>> handle this kind of button status check by call device specific api directly, rather than use the api from input subsystem. So I think
>> this is a acceptable way.
> 
> Which other boards did you find that used the “adc-keys” node this way?
> My grep shows only a few sunxi DTS referring to it:
> ptomsich at android:~/u-boot-rockchip$ git status
> On branch master
> Your branch is up-to-date with 'origin/master'.
> ptomsich at android:~/u-boot-rockchip$ git grep adc-keys
> arch/arm/dts/sun4i-a10.dtsi:                    compatible = "allwinner,sun4i-a10-lradc-keys";
> arch/arm/dts/sun5i-gr8.dtsi:                    compatible = "allwinner,sun4i-a10-lradc-keys";
> arch/arm/dts/sun5i.dtsi:                        compatible = "allwinner,sun4i-a10-lradc-keys";
> arch/arm/dts/sun6i-a31.dtsi:                    compatible = "allwinner,sun4i-a10-lradc-keys";
> arch/arm/dts/sun7i-a20.dtsi:                    compatible = "allwinner,sun4i-a10-lradc-keys";
> arch/arm/dts/sun8i-a23-a33.dtsi:                        compatible = "allwinner,sun4i-a10-lradc-keys";
> 
> 
>>       Would you please consider taking this patch if there is no other problems. And when there is a adc-keys driver in the future, I can move
>> it to input based api, or I will write a adc-keys driver when my workloads is not so heavy as now.
> 
> The problem is that if we go ahead with this as-is, there never be a driver for the "adc-keys” created ...
> Let’s see what for Simon’s input is, before we make a final decision.
> 
>>> 
>>>> 
>>>> -    if (adc_channel_single_shot("saradc", 1, &val)) {
>>>> +    if (adc_channel_single_shot("saradc", channel, &val)) {
>>>>         pr_err("%s: adc_channel_single_shot fail!\n", __func__);
>>>>         return false;
>>>>     }
>