Re: [U-Boot] [PATCH] rockchip: rockchip, sdram-channel 0xff fix remaining dts

[Simon Glass]

Hi Ziyuan,

On 31 July 2016 at 20:36, Ziyuan Xu  wrote:
> Hi Simon,
>
>
>
> On 2016年08月01日 10:21, Simon Glass wrote:
>>
>> Hi Ziyuan,
>>
>> On 31 July 2016 at 20:13, Ziyuan Xu  wrote:
>>>
>>> Hi Simon,
>>>
>>>
>>> On 2016年08月01日 09:51, Simon Glass wrote:

 Hi Sandy,

 On 28 July 2016 at 07:49, Sandy Patterson 
 wrote:
>
> Add an extra byte so that this data is not byteswapped.
>
> Signed-off-by: Sandy Patterson 
> ---
>
>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 

 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!
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] rockchip: rockchip, sdram-channel 0xff fix remaining dts

[Ziyuan Xu]

Hi Simon,


On 2016年08月01日 10:21, Simon Glass wrote:

Hi Ziyuan,

On 31 July 2016 at 20:13, Ziyuan Xu  wrote:

Hi Simon,


On 2016年08月01日 09:51, Simon Glass wrote:

Hi Sandy,

On 28 July 2016 at 07:49, Sandy Patterson 
wrote:

Add an extra byte so that this data is not byteswapped.

Signed-off-by: Sandy Patterson 
---

   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 

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






___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] rockchip: rockchip, sdram-channel 0xff fix remaining dts

[Simon Glass]

Hi Ziyuan,

On 31 July 2016 at 20:13, Ziyuan Xu  wrote:
> Hi Simon,
>
>
> On 2016年08月01日 09:51, Simon Glass wrote:
>>
>> Hi Sandy,
>>
>> On 28 July 2016 at 07:49, Sandy Patterson 
>> wrote:
>>>
>>> Add an extra byte so that this data is not byteswapped.
>>>
>>> Signed-off-by: Sandy Patterson 
>>> ---
>>>
>>>   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 
>>
>> 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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] rockchip: rockchip, sdram-channel 0xff fix remaining dts

[Ziyuan Xu]

Hi Simon,


On 2016年08月01日 09:51, Simon Glass wrote:

Hi Sandy,

On 28 July 2016 at 07:49, Sandy Patterson  wrote:

Add an extra byte so that this data is not byteswapped.

Signed-off-by: Sandy Patterson 
---

  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 

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 66600 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 66600 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



___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] rockchip: rockchip, sdram-channel 0xff fix remaining dts

[Simon Glass]

Hi Sandy,

On 28 July 2016 at 07:49, Sandy Patterson  wrote:
> Add an extra byte so that this data is not byteswapped.
>
> Signed-off-by: Sandy Patterson 
> ---
>
>  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 

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 66600 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 66600 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

[U-Boot] [PATCH] rockchip: rockchip, sdram-channel 0xff fix remaining dts

[Sandy Patterson]

Add an extra byte so that this data is not byteswapped.

Signed-off-by: Sandy Patterson 
---

 arch/arm/dts/rk3288-rock2-square.dts | 2 +-
 arch/arm/dts/rk3288-veyron.dtsi  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

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 66600 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 66600 3 9 1>;
 };
 
-- 
1.9.1

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot