[U-Boot] [U-Boot, 2/2] rockchip: Drop call to rockchip_dnl_mode_check() for now【请注意，邮件由u-boot-bounces at lists.denx.de代发】

[Andy Yan]

Hi Philipp:

     Sorry for the late reply, we just come back from the Chinese Spring 
Festival.

On 2019/2/1 下午6:26, Philipp Tomsich wrote:
> Kever,
>
> Independent of whether we revert this for the current cycle (and also 
> independent of
> if I ever find the other patch you had been referring to — I couldn’t 
> find it in my local
> mailing list archive) and then deprecate it for the next release 
> (unless converted to
> DM), we still have a number of architectural issues that need to be 
> addressed:

I still doubt  is this a right  work-flow for patch apply. Before we  
apply  a patch  which will break many other boards , should we  make 
sure there is a solution patch applied for these boars first?


> 1.This really should be a driver under DTS control.
> 2.We need to not get away from configuring SOM-specific addresses via 
> Kconfig
>
> Both these issues are technical debt that we’ve accumulated over the 
> last 18 months
> and need to address for the sake of future maintainability.
> E.g. ‘setting an address to 0x0 via Kconfig to disable a 
> driver/feature’ really isn’t in line
> with the architectural direction of U-Boot.
>
For technical side, I think CONFIG_ROCKCHIP_BOOT_MODE_REG is necessary 
here, we will read this register from save_boot_params when we get out 
from bootrom,  the dtb is not available at this point.

On the other hand, almost rockchip based products use a recovery key to 
enter download(upgrade)mode, this is a muti-funtion key, most of them 
reuse with vol+- key,  we would like the u-boot share

dtb with linux kernel. To keep the linux kernel dts as clean as possible 
,we don't want to add another dts property to describe this key either. 
This is why I implement function rockchip_dnl_key_pressed as __weak.


> I don’t have my own house completely in order (I’ve been talking for a 
> year now about
> finally wrapping the RGMII/GMII selection into an ioctl-call to a 
> driver) yet, but that doesn’t
> mean that we we should delay this clean-up more than absolutely necessary.
>
> Thanks,
> Philipp.
>
>> On 01.02.2019, at 10:34, Philipp Tomsich 
>> <philipp.tomsich at theobroma-systems.com 
>> <mailto:philipp.tomsich at theobroma-systems.com>> wrote:
>>
>>
>>
>>> On 01.02.2019, at 10:32, Kever Yang <kever.yang at rock-chips.com 
>>> <mailto:kever.yang at rock-chips.com>> wrote:
>>>
>>> Hi Philipp,
>>>
>>>    This is not right,  this patch should not merged like this!!!
>>>
>>>    I have give my review comment in previous mail, and this will break
>>> many boards.
>>>
>>>    My another patch do not break anything, but you insist NAK it
>>> without acceptable reason;
>>
>> What other patch?
>> I don’t remember seeing that one...
>>
>>>    This patch definitely break other board and I have comment it, but
>>> you just ignore other people's review and merge it, good job!
>>>
>>> Thanks,
>>> - Kever
>>> On 02/01/2019 05:12 AM, Philipp Tomsich wrote:
>>>>> This function causes a 5-second delay and stops the display working on
>>>>> minnie. 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 <sjg at chromium.org 
>>>>> <mailto:sjg at chromium.org>>
>>>>> Reviewed-by: Philipp Tomsich 
>>>>> <philipp.tomsich at theobroma-systems.com 
>>>>> <mailto:philipp.tomsich at theobroma-systems.com>>
>>>>> ---
>>>>>
>>>>> arch/arm/mach-rockchip/boot_mode.c | 8 +++++++-
>>>>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>>>>
>>>> Applied to u-boot-rockchip, thanks!
>>>> _______________________________________________
>>>> U-Boot mailing list
>>>> U-Boot at lists.denx.de <mailto:U-Boot at lists.denx.de>
>>>> https://lists.denx.de/listinfo/u-boot
>>>
>>>
>>>
>>
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot at lists.denx.de <mailto:U-Boot at lists.denx.de>
>> https://lists.denx.de/listinfo/u-boot
>