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

[Simon Glass]

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.

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