| Message ID | 1469713741-17927-1-git-send-email-apatterson@sightlogix.com |
|---|---|
| State | Accepted |
| Commit | 2918d96728379b544fd8ba397cdeb47170dc38f8 |
| Delegated to: | Simon Glass |
| Headers | show |
Hi Sandy, On 28 July 2016 at 07:49, Sandy Patterson <apatterson@sightlogix.com> wrote: > Add an extra byte so that this data is not byteswapped. > > Signed-off-by: Sandy Patterson <apatterson@sightlogix.com> > --- > > arch/arm/dts/rk3288-rock2-square.dts | 2 +- > arch/arm/dts/rk3288-veyron.dtsi | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) Acked-by: Simon Glass <sjg@chromium.org> Do these board use OF_PLATDATA? I thought not. > > diff --git a/arch/arm/dts/rk3288-rock2-square.dts b/arch/arm/dts/rk3288-rock2-square.dts > index 34073c9..2c30355 100644 > --- a/arch/arm/dts/rk3288-rock2-square.dts > +++ b/arch/arm/dts/rk3288-rock2-square.dts > @@ -192,7 +192,7 @@ > 0x5 0x0>; > rockchip,phy-timing = <0x48f9aab4 0xea0910 0x1002c200 > 0xa60 0x40 0x10 0x0>; > - rockchip,sdram-channel = /bits/ 8 <0x1 0xa 0x3 0x2 0x1 0x0 0xf 0xf>; > + rockchip,sdram-channel = /bits/ 8 <0x1 0xa 0x3 0x2 0x1 0x0 0xf 0xf 0xff>; > rockchip,sdram-params = <0x30B25564 0x627 3 666000000 3 9 1>; > }; > > diff --git a/arch/arm/dts/rk3288-veyron.dtsi b/arch/arm/dts/rk3288-veyron.dtsi > index 421d212..d9d5187 100644 > --- a/arch/arm/dts/rk3288-veyron.dtsi > +++ b/arch/arm/dts/rk3288-veyron.dtsi > @@ -253,7 +253,7 @@ > 0x5 0x0>; > rockchip,phy-timing = <0x48f9aab4 0xea0910 0x1002c200 > 0xa60 0x40 0x10 0x0>; > - rockchip,sdram-channel = /bits/ 8 <0x1 0xa 0x3 0x2 0x1 0x0 0xf 0xf>; > + rockchip,sdram-channel = /bits/ 8 <0x1 0xa 0x3 0x2 0x1 0x0 0xf 0xf 0xff>; > rockchip,sdram-params = <0x30B25564 0x627 3 666000000 3 9 1>; > }; > > -- > 1.9.1 > Regards, Simon
Hi Simon, On 2016年08月01日 09:51, Simon Glass wrote: > Hi Sandy, > > On 28 July 2016 at 07:49, Sandy Patterson <apatterson@sightlogix.com> wrote: >> Add an extra byte so that this data is not byteswapped. >> >> Signed-off-by: Sandy Patterson <apatterson@sightlogix.com> >> --- >> >> arch/arm/dts/rk3288-rock2-square.dts | 2 +- >> arch/arm/dts/rk3288-veyron.dtsi | 2 +- >> 2 files changed, 2 insertions(+), 2 deletions(-) > Acked-by: Simon Glass <sjg@chromium.org> > > Do these board use OF_PLATDATA? I thought not. Yes, only firefly-rk3288 board use OF_PLATDATA. But driver get rk3288_sdram_channel via fdtdec_get_byte_array with the size which is sizeof(struct rk3288_sdram_channel). In commit 9ca7e67 rockchip: Update the sdram-channel property to support of-platdata, you add dummy element in struct rk3288_sdram_channel and size was changed to 9. Without this fix, driver get rk3288_sdram_channel failed. Maybe add CONFIG_IS_ENABLED(OF_PLATDATA) for distinction is better, how about? struct rk3288_sdram_channel { u8 rank; u8 col; u8 bk; u8 bw; u8 dbw; u8 row_3_4; u8 cs0_row; u8 cs1_row; #if CONFIG_IS_ENABLED(OF_PLATDATA) /* * For of-platdata, which would otherwise convert this into two * byte-swapped integers. With a size of 9 bytes, this struct will * appear in of-platdata as a byte array. */ u8 dummy; #endif }; >> diff --git a/arch/arm/dts/rk3288-rock2-square.dts b/arch/arm/dts/rk3288-rock2-square.dts >> index 34073c9..2c30355 100644 >> --- a/arch/arm/dts/rk3288-rock2-square.dts >> +++ b/arch/arm/dts/rk3288-rock2-square.dts >> @@ -192,7 +192,7 @@ >> 0x5 0x0>; >> rockchip,phy-timing = <0x48f9aab4 0xea0910 0x1002c200 >> 0xa60 0x40 0x10 0x0>; >> - rockchip,sdram-channel = /bits/ 8 <0x1 0xa 0x3 0x2 0x1 0x0 0xf 0xf>; >> + rockchip,sdram-channel = /bits/ 8 <0x1 0xa 0x3 0x2 0x1 0x0 0xf 0xf 0xff>; >> rockchip,sdram-params = <0x30B25564 0x627 3 666000000 3 9 1>; >> }; >> >> diff --git a/arch/arm/dts/rk3288-veyron.dtsi b/arch/arm/dts/rk3288-veyron.dtsi >> index 421d212..d9d5187 100644 >> --- a/arch/arm/dts/rk3288-veyron.dtsi >> +++ b/arch/arm/dts/rk3288-veyron.dtsi >> @@ -253,7 +253,7 @@ >> 0x5 0x0>; >> rockchip,phy-timing = <0x48f9aab4 0xea0910 0x1002c200 >> 0xa60 0x40 0x10 0x0>; >> - rockchip,sdram-channel = /bits/ 8 <0x1 0xa 0x3 0x2 0x1 0x0 0xf 0xf>; >> + rockchip,sdram-channel = /bits/ 8 <0x1 0xa 0x3 0x2 0x1 0x0 0xf 0xf 0xff>; >> rockchip,sdram-params = <0x30B25564 0x627 3 666000000 3 9 1>; >> }; >> >> -- >> 1.9.1 >> > Regards, > Simon > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot
Hi Ziyuan, On 31 July 2016 at 20:13, Ziyuan Xu <xzy.xu@rock-chips.com> wrote: > Hi Simon, > > > On 2016年08月01日 09:51, Simon Glass wrote: >> >> Hi Sandy, >> >> On 28 July 2016 at 07:49, Sandy Patterson <apatterson@sightlogix.com> >> wrote: >>> >>> Add an extra byte so that this data is not byteswapped. >>> >>> Signed-off-by: Sandy Patterson <apatterson@sightlogix.com> >>> --- >>> >>> arch/arm/dts/rk3288-rock2-square.dts | 2 +- >>> arch/arm/dts/rk3288-veyron.dtsi | 2 +- >>> 2 files changed, 2 insertions(+), 2 deletions(-) >> >> Acked-by: Simon Glass <sjg@chromium.org> >> >> Do these board use OF_PLATDATA? I thought not. > > Yes, only firefly-rk3288 board use OF_PLATDATA. But driver get > rk3288_sdram_channel via fdtdec_get_byte_array with the size which is > sizeof(struct rk3288_sdram_channel). > In commit 9ca7e67 rockchip: Update the sdram-channel property to support > of-platdata, you add dummy element in struct rk3288_sdram_channel and size > was changed to 9. > Without this fix, driver get rk3288_sdram_channel failed. > > Maybe add CONFIG_IS_ENABLED(OF_PLATDATA) for distinction is better, how > about? > > struct rk3288_sdram_channel { > u8 rank; > u8 col; > u8 bk; > u8 bw; > u8 dbw; > u8 row_3_4; > u8 cs0_row; > u8 cs1_row; > #if CONFIG_IS_ENABLED(OF_PLATDATA) > /* > * For of-platdata, which would otherwise convert this into two > * byte-swapped integers. With a size of 9 bytes, this struct will > * appear in of-platdata as a byte array. > */ > u8 dummy; > #endif > }; > > Yes, but I'm happy with either solution. Your one may be a little easier to understand, but if someone switches a board over to OF_PLATDATA then it will be confusing... Please let me know which you prefer. Regards, Simon
Hi Simon, On 2016年08月01日 10:21, Simon Glass wrote: > Hi Ziyuan, > > On 31 July 2016 at 20:13, Ziyuan Xu <xzy.xu@rock-chips.com> wrote: >> Hi Simon, >> >> >> On 2016年08月01日 09:51, Simon Glass wrote: >>> Hi Sandy, >>> >>> On 28 July 2016 at 07:49, Sandy Patterson <apatterson@sightlogix.com> >>> wrote: >>>> Add an extra byte so that this data is not byteswapped. >>>> >>>> Signed-off-by: Sandy Patterson <apatterson@sightlogix.com> >>>> --- >>>> >>>> arch/arm/dts/rk3288-rock2-square.dts | 2 +- >>>> arch/arm/dts/rk3288-veyron.dtsi | 2 +- >>>> 2 files changed, 2 insertions(+), 2 deletions(-) >>> Acked-by: Simon Glass <sjg@chromium.org> >>> >>> Do these board use OF_PLATDATA? I thought not. >> Yes, only firefly-rk3288 board use OF_PLATDATA. But driver get >> rk3288_sdram_channel via fdtdec_get_byte_array with the size which is >> sizeof(struct rk3288_sdram_channel). >> In commit 9ca7e67 rockchip: Update the sdram-channel property to support >> of-platdata, you add dummy element in struct rk3288_sdram_channel and size >> was changed to 9. >> Without this fix, driver get rk3288_sdram_channel failed. >> >> Maybe add CONFIG_IS_ENABLED(OF_PLATDATA) for distinction is better, how >> about? >> >> struct rk3288_sdram_channel { >> u8 rank; >> u8 col; >> u8 bk; >> u8 bw; >> u8 dbw; >> u8 row_3_4; >> u8 cs0_row; >> u8 cs1_row; >> #if CONFIG_IS_ENABLED(OF_PLATDATA) >> /* >> * For of-platdata, which would otherwise convert this into two >> * byte-swapped integers. With a size of 9 bytes, this struct will >> * appear in of-platdata as a byte array. >> */ >> u8 dummy; >> #endif >> }; >> >> > Yes, but I'm happy with either solution. Your one may be a little > easier to understand, but if someone switches a board over to > OF_PLATDATA then it will be confusing... Please let me know which you > prefer. OF_PLATDATA is used to reduce the size of the SPL, right? In most cases, some rk3288 boards use OF_LIBFDT. If OF_PLATDATA is really required, I think your comment is very clear. I perfer my above opinion? :-) > Regards, > Simon > > >
Hi Ziyuan, On 31 July 2016 at 20:36, Ziyuan Xu <xzy.xu@rock-chips.com> wrote: > Hi Simon, > > > > On 2016年08月01日 10:21, Simon Glass wrote: >> >> Hi Ziyuan, >> >> On 31 July 2016 at 20:13, Ziyuan Xu <xzy.xu@rock-chips.com> wrote: >>> >>> Hi Simon, >>> >>> >>> On 2016年08月01日 09:51, Simon Glass wrote: >>>> >>>> Hi Sandy, >>>> >>>> On 28 July 2016 at 07:49, Sandy Patterson <apatterson@sightlogix.com> >>>> wrote: >>>>> >>>>> Add an extra byte so that this data is not byteswapped. >>>>> >>>>> Signed-off-by: Sandy Patterson <apatterson@sightlogix.com> >>>>> --- >>>>> >>>>> arch/arm/dts/rk3288-rock2-square.dts | 2 +- >>>>> arch/arm/dts/rk3288-veyron.dtsi | 2 +- >>>>> 2 files changed, 2 insertions(+), 2 deletions(-) >>>> >>>> Acked-by: Simon Glass <sjg@chromium.org> >>>> >>>> Do these board use OF_PLATDATA? I thought not. >>> >>> Yes, only firefly-rk3288 board use OF_PLATDATA. But driver get >>> rk3288_sdram_channel via fdtdec_get_byte_array with the size which is >>> sizeof(struct rk3288_sdram_channel). >>> In commit 9ca7e67 rockchip: Update the sdram-channel property to support >>> of-platdata, you add dummy element in struct rk3288_sdram_channel and >>> size >>> was changed to 9. >>> Without this fix, driver get rk3288_sdram_channel failed. >>> >>> Maybe add CONFIG_IS_ENABLED(OF_PLATDATA) for distinction is better, how >>> about? >>> >>> struct rk3288_sdram_channel { >>> u8 rank; >>> u8 col; >>> u8 bk; >>> u8 bw; >>> u8 dbw; >>> u8 row_3_4; >>> u8 cs0_row; >>> u8 cs1_row; >>> #if CONFIG_IS_ENABLED(OF_PLATDATA) >>> /* >>> * For of-platdata, which would otherwise convert this into two >>> * byte-swapped integers. With a size of 9 bytes, this struct will >>> * appear in of-platdata as a byte array. >>> */ >>> u8 dummy; >>> #endif >>> }; >>> >>> >> Yes, but I'm happy with either solution. Your one may be a little >> easier to understand, but if someone switches a board over to >> OF_PLATDATA then it will be confusing... Please let me know which you >> prefer. > > OF_PLATDATA is used to reduce the size of the SPL, right? In most cases, > some rk3288 boards use OF_LIBFDT. If OF_PLATDATA is really required, I think > your comment is very clear. > I perfer my above opinion? :-) I'm going to apply this patch, but please feel free to send a patch to change this over if you wish. Applied to u-boot-rockchip, thanks!
diff --git a/arch/arm/dts/rk3288-rock2-square.dts b/arch/arm/dts/rk3288-rock2-square.dts index 34073c9..2c30355 100644 --- a/arch/arm/dts/rk3288-rock2-square.dts +++ b/arch/arm/dts/rk3288-rock2-square.dts @@ -192,7 +192,7 @@ 0x5 0x0>; rockchip,phy-timing = <0x48f9aab4 0xea0910 0x1002c200 0xa60 0x40 0x10 0x0>; - rockchip,sdram-channel = /bits/ 8 <0x1 0xa 0x3 0x2 0x1 0x0 0xf 0xf>; + rockchip,sdram-channel = /bits/ 8 <0x1 0xa 0x3 0x2 0x1 0x0 0xf 0xf 0xff>; rockchip,sdram-params = <0x30B25564 0x627 3 666000000 3 9 1>; }; diff --git a/arch/arm/dts/rk3288-veyron.dtsi b/arch/arm/dts/rk3288-veyron.dtsi index 421d212..d9d5187 100644 --- a/arch/arm/dts/rk3288-veyron.dtsi +++ b/arch/arm/dts/rk3288-veyron.dtsi @@ -253,7 +253,7 @@ 0x5 0x0>; rockchip,phy-timing = <0x48f9aab4 0xea0910 0x1002c200 0xa60 0x40 0x10 0x0>; - rockchip,sdram-channel = /bits/ 8 <0x1 0xa 0x3 0x2 0x1 0x0 0xf 0xf>; + rockchip,sdram-channel = /bits/ 8 <0x1 0xa 0x3 0x2 0x1 0x0 0xf 0xf 0xff>; rockchip,sdram-params = <0x30B25564 0x627 3 666000000 3 9 1>; };
Add an extra byte so that this data is not byteswapped. Signed-off-by: Sandy Patterson <apatterson@sightlogix.com> --- arch/arm/dts/rk3288-rock2-square.dts | 2 +- arch/arm/dts/rk3288-veyron.dtsi | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)