Re: [U-Boot] [PATCH 2/2] rockchip: Drop call to rockchip_dnl_mode_check() for now[Please note, mail behalf by 3vnmnxammciszqnjoyvtpbt.vynrlcly.fhunyvjr-jopwz....@chromium-sendgmr.bounces.google.com]

[Philipp Tomsich]

> On 03.01.2019, at 22:29, Simon Glass  wrote:
> 
> Hi,
> 
> On Thu, 3 Jan 2019 at 00:32, Kever Yang  > wrote:
>> 
>> Hi Simon,
>> 
>> 
>> On 12/29/2018 09:16 PM, Simon Glass wrote:
>>> This function causes a 5-second delay and stops the display working on
>>> minnie.
>> 
>> I don't understand why need 5-second, the logic is to check the adc key
>> press to decide the mode, maybe because the different usage of the adc
>> in minnie?
>> 
>> Most of boards(which do not have ec like chromebook) need this check to
>> get into download mode.
>> I would prefer chrombook to implement a function to replace the week
>> rockchip_dnl_mode_check();
> 
> But where is it specified in the DT that it uses the ADC for this
> feature? It really should have a driver…

3 things:
a.  It really should have a driver.
b.  One does not have to replace rockchip_dnl_mode_check(), as there
already is logic to remove it if CONFIG_ROCKCHIP_BOOT_MODE_REG
is set to 0 in the config.
c.  We really need to get away from configuring magic addresses via 
Kconfig. 
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH 2/2] rockchip: Drop call to rockchip_dnl_mode_check() for now[Please note, mail behalf by 3vnmnxammciszqnjoyvtpbt.vynrlcly.fhunyvjr-jopwz....@chromium-sendgmr.bounces.google.com]

[Simon Glass]

Hi,

On Thu, 3 Jan 2019 at 00:32, Kever Yang  wrote:
>
> Hi Simon,
>
>
> On 12/29/2018 09:16 PM, Simon Glass wrote:
> > This function causes a 5-second delay and stops the display working on
> > minnie.
>
> I don't understand why need 5-second, the logic is to check the adc key
> press to decide the mode, maybe because the different usage of the adc
> in minnie?
>
> Most of boards(which do not have ec like chromebook) need this check to
> get into download mode.
> I would prefer chrombook to implement a function to replace the week
> rockchip_dnl_mode_check();

But where is it specified in the DT that it uses the ADC for this
feature? It really should have a driver...

Regards,
Simon


>
> Thanks,
> - Kever
> > This code should be in a driver and should only be enabled by
> > a device-tree property, so that it does not affect devices which do not
> > have this feature.
> >
> > Signed-off-by: Simon Glass 
> > ---
> >
> >  arch/arm/mach-rockchip/boot_mode.c | 8 +++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/arm/mach-rockchip/boot_mode.c 
> > b/arch/arm/mach-rockchip/boot_mode.c
> > index f32b3c4ce56..d7997d71e30 100644
> > --- a/arch/arm/mach-rockchip/boot_mode.c
> > +++ b/arch/arm/mach-rockchip/boot_mode.c
> > @@ -61,7 +61,13 @@ int setup_boot_mode(void)
> >   void *reg = (void *)CONFIG_ROCKCHIP_BOOT_MODE_REG;
> >   int boot_mode = readl(reg);
> >
> > - rockchip_dnl_mode_check();
> > + /*
> > +  * This should be handled using a driver-tree property and a suitable
> > +  * driver which can read the appropriate settings. As it is, this
> > +  * breaks chromebook_minnie.\
> > +  *
> > +  * rockchip_dnl_mode_check();
> > +  */
> >
> >   boot_mode = readl(reg);
> >   debug("%s: boot mode 0x%08x\n", __func__, boot_mode);
>
>
>
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH 2/2] rockchip: Drop call to rockchip_dnl_mode_check() for now[Please note, mail behalf by 3vnmnxammciszqnjoyvtpbt.vynrlcly.fhunyvjr-jopwz....@chromium-sendgmr.bounces.google.com]

[Kever Yang]

Hi Simon,


On 12/29/2018 09:16 PM, Simon Glass wrote:
> This function causes a 5-second delay and stops the display working on
> minnie. 

I don't understand why need 5-second, the logic is to check the adc key
press to decide the mode, maybe because the different usage of the adc
in minnie?

Most of boards(which do not have ec like chromebook) need this check to
get into download mode.
I would prefer chrombook to implement a function to replace the week
rockchip_dnl_mode_check();

Thanks,
- Kever
> This code should be in a driver and should only be enabled by
> a device-tree property, so that it does not affect devices which do not
> have this feature.
>
> Signed-off-by: Simon Glass 
> ---
>
>  arch/arm/mach-rockchip/boot_mode.c | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-rockchip/boot_mode.c 
> b/arch/arm/mach-rockchip/boot_mode.c
> index f32b3c4ce56..d7997d71e30 100644
> --- a/arch/arm/mach-rockchip/boot_mode.c
> +++ b/arch/arm/mach-rockchip/boot_mode.c
> @@ -61,7 +61,13 @@ int setup_boot_mode(void)
>   void *reg = (void *)CONFIG_ROCKCHIP_BOOT_MODE_REG;
>   int boot_mode = readl(reg);
>  
> - rockchip_dnl_mode_check();
> + /*
> +  * This should be handled using a driver-tree property and a suitable
> +  * driver which can read the appropriate settings. As it is, this
> +  * breaks chromebook_minnie.\
> +  *
> +  * rockchip_dnl_mode_check();
> +  */
>  
>   boot_mode = readl(reg);
>   debug("%s: boot mode 0x%08x\n", __func__, boot_mode);



___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot