Hi Kever, On Wed, 28 Nov 2018 at 18:10, Kever Yang <kever.y...@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.y...@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.y...@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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
