[U-Boot] [RESENT PATCH 1/2] rockchip: rk3128: use ROCKCHIP_BOOT_MODE_REG to update reboot flag

[Simon Glass]

Hi Kever,

On Wed, 28 Nov 2018 at 18:10, Kever Yang <kever.yang at rock-chips.com> wrote:
>
> Hi Philipp,
>
>
> On 11/28/2018 05:07 PM, Philipp Tomsich wrote:
> > Kever,
> >
> >> On 28.11.2018, at 03:04, Kever Yang <kever.yang at rock-chips.com> wrote:
> >>
> >> Use ROCKCHIP_BOOT_MODE_REG instead of grf structure so that
> >> we can re-use the source code later.
> >>
> >> Signed-off-by: Kever Yang <kever.yang at rock-chips.com>
> > NAK, as there are still pending changes.
> Yes, I got that, and I send out my comments on your comments with no
> more response.
> > See below for the reminder, in case this got lost.
> >
> >> ---
> >>
> >> arch/arm/mach-rockchip/Kconfig        | 1 +
> >> arch/arm/mach-rockchip/rk3128-board.c | 5 +----
> >> 2 files changed, 2 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig
> >> index 145d96b1f0..94a03e2a38 100644
> >> --- a/arch/arm/mach-rockchip/Kconfig
> >> +++ b/arch/arm/mach-rockchip/Kconfig
> >> @@ -179,6 +179,7 @@ config TPL_ROCKCHIP_BACK_TO_BROM
> >> config ROCKCHIP_BOOT_MODE_REG
> >>      hex "Rockchip boot mode flag register address"
> >>      default 0x200081c8 if ROCKCHIP_RK3036
> >> +    default 0x100a0038 if ROCKCHIP_RK3128
> >>      default 0x20004040 if ROCKCHIP_RK3188
> >>      default 0x110005c8 if ROCKCHIP_RK322X
> >>      default 0xff730094 if ROCKCHIP_RK3288
> > As previously discussed: these should all go into header files, as they are not user-configurable.
> > This affects multiple patch series (as I requested the same for the STIMER address).
> I believe you mention about this: http://patchwork.ozlabs.org/patch/891462/
> Now the model in u-boot rockchip channel is:
> - People send out patches to mailing list;
> - The patch get an ACK patch and review patch may request for change in
> 1 week~4 months;
> - According to the maintainer's comment, people reply for why the patch
> like this,
>    and maybe the patch do not need to change just like what the
> maintainer want.
> - BUT, there will never be more reply/comments.
> - Then, people have to resend the patches they think it may be
> reasonable, and maintainer
>    then complain people doesn't address his comment.
>
> For this patch, I think:
> - This is not an first patch for this operation, this just make rk3128
> work like other SoCs, it's not a new feature;
> - This kind of default value setting is all over the U-Boot project, I'm
> not say it's correct,
>   but it's a good solution and convenient for us to use the same object
> with different value in different SoCs,
>   It's much better to separate them into more then 10 header files or
> lots of "#ifdef CONFIG_ROCKCHIIP_RK3128"
>   in one header files.
>
>   I hope I can get reply for this mail this time.
>
> Hi Simon,
>     Could you help to comment on this?

What happens if the user changes the value?

Can this go in the device tree?

It seems like this should be in a driver, to me. We have a SYSCON
driver for GRF. Should we add an ioctl-type interface to it?

Regards,
Simon