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

[Andy Yan]

Hi Simon:

On 2019/2/12 下午11:31, Simon Glass wrote:
> HI Andy,
>
> On Tue, 12 Feb 2019 at 04:05, Andy Yan <andy.yan at rock-chips.com> wrote:
>> 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.
> Yes this is happening very early, but I wonder why this is necessary?
> Can it be moved to later?
>
> The call to check_back_to_brom_dnl_flag() happens much later so there
> should be no problem to convert that.
>
>> 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.
>>
> OK, but given that it is called later, it seems to me that it could
> use a driver.



We can't let it called later. See the discuss here : 
http://patchwork.ozlabs.org/patch/812228/


> Regards,
> Simon
>
>
>>> 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
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot at lists.denx.de
>> https://lists.denx.de/listinfo/u-boot
>
>