On 11.03.2019, at 03:30, Simon Glass sjg@chromium.org wrote:
Hi Andy,
On Wed, 6 Mar 2019 at 03:52, Andy Yan andy.yan@rock-chips.com wrote:
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@rock-chips.com wrote:
Hi Philipp:
Sorry for the late reply, we just come back from the Chinese SpringFestival.
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/
Yes I read that, and took a look myself, and certainly it does not look very easy and I take your point about potential bugs being introduced.
It seems like you have tried quite hard, so I'm not going to object if you can't find a way. My main objection was that it broke other boards.
Is there any way the check could be delayed enough to actually be able to read the device tree? It could call spl_early_init() quite early and then do that.
Note that the minimum improvement that I’d expect to get this fully enabled again would be a clean-up of the Kconfig options, so it is easy and (from the documentation) predictable for boards to turn this off.
I still want this to be turned back on in the current cycle.
Thanks, Philipp.