HI Andy, On Tue, 12 Feb 2019 at 04:05, Andy Yan <andy....@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. 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.toms...@theobroma-systems.com > >> <mailto:philipp.toms...@theobroma-systems.com>> wrote: > >> > >> > >> > >>> On 01.02.2019, at 10:32, Kever Yang <kever.y...@rock-chips.com > >>> <mailto:kever.y...@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 <s...@chromium.org > >>>>> <mailto:s...@chromium.org>> > >>>>> Reviewed-by: Philipp Tomsich > >>>>> <philipp.toms...@theobroma-systems.com > >>>>> <mailto:philipp.toms...@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@lists.denx.de <mailto:U-Boot@lists.denx.de> > >>>> https://lists.denx.de/listinfo/u-boot > >>> > >>> > >>> > >> > >> _______________________________________________ > >> U-Boot mailing list > >> U-Boot@lists.denx.de <mailto:U-Boot@lists.denx.de> > >> https://lists.denx.de/listinfo/u-boot > > > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > https://lists.denx.de/listinfo/u-boot _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot