diff mbox series

[09/14] dt-bindings: arm: Document Renesas R-Car M3-N-based ULCB board

Message ID 20180804231114.21420-10-erosca@de.adit-jv.com (mailing list archive)
State Superseded
Delegated to: Simon Horman
Headers show
Series Add minimal DTS support for M3-N Starter Kit | expand

Commit Message

Eugeniu Rosca Aug. 4, 2018, 11:11 p.m. UTC 
In harmony with ATF and U-Boot outputs [1] and [2], the new board is
based on M3-N revision ES1.1 and the amount of memory present on SiP
is 2GiB, contiguously addressed.

[1] BL2: R-Car Gen3 Initial Program Loader(CA57) Rev.1.0.21
    BL2: PRR is R-Car M3N Ver.1.1

[2] U-Boot 2015.04-00295-*
    CPU: Renesas Electronics R8A77965 rev 1.1
    ---8<----
    DRAM:  1.9 GiB
    Bank #0: 0x048000000 - 0x0bfffffff, 1.9 GiB
    ---8<----

Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com>
---
 Documentation/devicetree/bindings/arm/shmobile.txt | 2 ++
 1 file changed, 2 insertions(+)

Comments

Jacopo Mondi Aug. 5, 2018, 8:15 a.m. UTC | #1 
Helle Eugeniu,

On Sun, Aug 05, 2018 at 01:11:09AM +0200, Eugeniu Rosca wrote:
> In harmony with ATF and U-Boot outputs [1] and [2], the new board is
> based on M3-N revision ES1.1 and the amount of memory present on SiP
> is 2GiB, contiguously addressed.

Not sure why the amount of installed system memory is relevant for
this commit..

>
> [1] BL2: R-Car Gen3 Initial Program Loader(CA57) Rev.1.0.21
>     BL2: PRR is R-Car M3N Ver.1.1
>
> [2] U-Boot 2015.04-00295-*
>     CPU: Renesas Electronics R8A77965 rev 1.1
>     ---8<----
>     DRAM:  1.9 GiB
>     Bank #0: 0x048000000 - 0x0bfffffff, 1.9 GiB
>     ---8<----
>
> Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com>
> ---
>  Documentation/devicetree/bindings/arm/shmobile.txt | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/arm/shmobile.txt b/Documentation/devicetree/bindings/arm/shmobile.txt
> index f391dba10574..2f3494a0107c 100644
> --- a/Documentation/devicetree/bindings/arm/shmobile.txt
> +++ b/Documentation/devicetree/bindings/arm/shmobile.txt
> @@ -106,6 +106,8 @@ Boards:
>      compatible = "renesas,lager", "renesas,r8a7790"
>    - M3ULCB (R-Car Starter Kit Pro, RTP0RC7796SKBX0010SA09 (M3 ES1.0))
>      compatible = "renesas,ulcb", "renesas,r8a7796"
> +  - M3-N ULCB (R-Car Starter Kit Pro, RTP0RC77965SKBX010SA00 (M3-N ES1.1))

Other documented ULCB description entries in this file are
H3ULCB and M3ULCB, so for consistency you should add M3NULCB, which
isn't that nice.

Imo, or you either replace "[H|M]3ULCB" with "[H|M]3 ULCB" in other
entries and you keep your "M3-N ULCB" here, which is nicer (you could
do that in patch 2).

Or maybe could you consider doing what has been done for
Salvator-x(s), which do not have the SoC model name in the entry description
at all (but please wait for others to comment before doing something
like that):

        - Salvator-X (RTP0RC7795SIPB0010S)
          compatible = "renesas,salvator-x", "renesas,r8a7795"
        - Salvator-X (RTP0RC7796SIPB0011S)
          compatible = "renesas,salvator-x", "renesas,r8a7796"
        - Salvator-X (RTP0RC7796SIPB0011S (M3-N))
          compatible = "renesas,salvator-x", "renesas,r8a77965"
        - Salvator-XS (Salvator-X 2nd version, RTP0RC7795SIPB0012S)
          compatible = "renesas,salvator-xs", "renesas,r8a7795"
        - Salvator-XS (Salvator-X 2nd version, RTP0RC7796SIPB0012S)
          compatible = "renesas,salvator-xs", "renesas,r8a7796"
        - Salvator-XS (Salvator-X 2nd version, RTP0RC77965SIPB012S)
          compatible = "renesas,salvator-xs", "renesas,r8a77965"

This would then be
        - ULCB (R-Car Starter Kit Premier, RTP0RC7795SKBX0010SA00 (H3 ES1.1))
          ULCB (R-Car Starter Kit Premier, RTP0RC77951SKBX010SA00 (H3 ES2.0))
          compatible = "renesas,ulcb", "renesas,r8a7795
        - ULCB (R-Car Starter Kit Pro, RTP0RC7796SKBX0010SA09 (M3 ES1.0)
          compatible = "renesas,ulcb", "renesas,r8a7796"
        - ULCB (R-Car Starter Kit Pro, RTP0RC77965SKBX010SA00 (M3-N ES1.1))
          compatible = "renesas,ulcb", "renesas,r8a77965"

Thanks
   j

PS: why the r8a7795-es1 version of Salvator-X is not listed? I assume
it has a different part number than the ES2.0 one, as ULCB with es1
does.


> +    compatible = "renesas,ulcb", "renesas,r8a77965"
>    - Marzen (R0P7779A00010S)
>      compatible = "renesas,marzen", "renesas,r8a7779"
>    - Porter (M2-LCDP)
> --
> 2.18.0
>
Eugeniu Rosca Aug. 5, 2018, 10:40 p.m. UTC | #2 
Hi Jacopo,

Thanks for your comments. Please, find my replies below.

On Sun, Aug 05, 2018 at 10:15:43AM +0200, jacopo mondi wrote:
> Helle Eugeniu,
> 
> On Sun, Aug 05, 2018 at 01:11:09AM +0200, Eugeniu Rosca wrote:
> > In harmony with ATF and U-Boot outputs [1] and [2], the new board is
> > based on M3-N revision ES1.1 and the amount of memory present on SiP
> > is 2GiB, contiguously addressed.
> 
> Not sure why the amount of installed system memory is relevant for
> this commit..

To be honest, I don't know precisely what's encoded in the board string
(particularly the one documented in this commit RTP0RC77965SKBX010SA00).

The only thing unmistakenly present there is the SoC model 77965 (i.e.
M3-N), but I am quite clueless about the rest.

What I can say for sure is that the end-user experience of a R-Car Gen3
reference board clearly depends on below parameters:
 - [A] SoC (model, revision) including SRAM and on-chip peripherals
 - [B] DRAM (amount, linear/2ch/4ch split) 
 - [C] Hyperflash (amount)
 - [D] board's PCB (revision)
 - [E] board's off-chip peripherals (model, revision)

I don't know how many of these parameters are embedded in the board
string, but since you are suggesting that RAM amount is not (IOW
Renesas will potentially release several RTP0RC77965SKBX010SA00
boards with different amounts of memory), I will happily update
the commit description.

> 
> >
> > [1] BL2: R-Car Gen3 Initial Program Loader(CA57) Rev.1.0.21
> >     BL2: PRR is R-Car M3N Ver.1.1
> >
> > [2] U-Boot 2015.04-00295-*
> >     CPU: Renesas Electronics R8A77965 rev 1.1
> >     ---8<----
> >     DRAM:  1.9 GiB
> >     Bank #0: 0x048000000 - 0x0bfffffff, 1.9 GiB
> >     ---8<----
> >
> > Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com>
> > ---
> >  Documentation/devicetree/bindings/arm/shmobile.txt | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/arm/shmobile.txt b/Documentation/devicetree/bindings/arm/shmobile.txt
> > index f391dba10574..2f3494a0107c 100644
> > --- a/Documentation/devicetree/bindings/arm/shmobile.txt
> > +++ b/Documentation/devicetree/bindings/arm/shmobile.txt
> > @@ -106,6 +106,8 @@ Boards:
> >      compatible = "renesas,lager", "renesas,r8a7790"
> >    - M3ULCB (R-Car Starter Kit Pro, RTP0RC7796SKBX0010SA09 (M3 ES1.0))
> >      compatible = "renesas,ulcb", "renesas,r8a7796"
> > +  - M3-N ULCB (R-Car Starter Kit Pro, RTP0RC77965SKBX010SA00 (M3-N ES1.1))
> 
> Other documented ULCB description entries in this file are
> H3ULCB and M3ULCB, so for consistency you should add M3NULCB, which
> isn't that nice.

This was my initial intent, but then I noticed commits like
519df0e05d27 ("dt-bindings: arm: consistently name r8a77965 as M3-N"),
which try to use "M3-N" notation consistently and avoid "M3N". So, I
tried to avoid "M3N" too.

> 
> Imo, or you either replace "[H|M]3ULCB" with "[H|M]3 ULCB" in other
> entries and you keep your "M3-N ULCB" here, which is nicer (you could
> do that in patch 2).
> 
> Or maybe could you consider doing what has been done for
> Salvator-x(s), which do not have the SoC model name in the entry description
> at all (but please wait for others to comment before doing something
> like that):

I prefer this second variant, but I will wait for others to comment.

> 
>         - Salvator-X (RTP0RC7795SIPB0010S)
>           compatible = "renesas,salvator-x", "renesas,r8a7795"
>         - Salvator-X (RTP0RC7796SIPB0011S)
>           compatible = "renesas,salvator-x", "renesas,r8a7796"
>         - Salvator-X (RTP0RC7796SIPB0011S (M3-N))
>           compatible = "renesas,salvator-x", "renesas,r8a77965"
>         - Salvator-XS (Salvator-X 2nd version, RTP0RC7795SIPB0012S)
>           compatible = "renesas,salvator-xs", "renesas,r8a7795"
>         - Salvator-XS (Salvator-X 2nd version, RTP0RC7796SIPB0012S)
>           compatible = "renesas,salvator-xs", "renesas,r8a7796"
>         - Salvator-XS (Salvator-X 2nd version, RTP0RC77965SIPB012S)
>           compatible = "renesas,salvator-xs", "renesas,r8a77965"
> 
> This would then be
>         - ULCB (R-Car Starter Kit Premier, RTP0RC7795SKBX0010SA00 (H3 ES1.1))
>           ULCB (R-Car Starter Kit Premier, RTP0RC77951SKBX010SA00 (H3 ES2.0))
>           compatible = "renesas,ulcb", "renesas,r8a7795
>         - ULCB (R-Car Starter Kit Pro, RTP0RC7796SKBX0010SA09 (M3 ES1.0)
>           compatible = "renesas,ulcb", "renesas,r8a7796"
>         - ULCB (R-Car Starter Kit Pro, RTP0RC77965SKBX010SA00 (M3-N ES1.1))
>           compatible = "renesas,ulcb", "renesas,r8a77965"
> 
> Thanks
>    j
> 
> PS: why the r8a7795-es1 version of Salvator-X is not listed? I assume
> it has a different part number than the ES2.0 one, as ULCB with es1
> does.

That's a good question, but I think the concern is not too big since
r8a7795-es1 is EOL. 

> 
> > +    compatible = "renesas,ulcb", "renesas,r8a77965"
> >    - Marzen (R0P7779A00010S)
> >      compatible = "renesas,marzen", "renesas,r8a7779"
> >    - Porter (M2-LCDP)
> > --
> > 2.18.0
> >

Thanks!

Best regards,
Eugeniu.
Eugeniu Rosca Aug. 10, 2018, 1:48 p.m. UTC | #3 
Hi again Jacopo,

On Mon, Aug 06, 2018 at 12:40:02AM +0200, Eugeniu Rosca wrote:
> Hi Jacopo,
> 
> Thanks for your comments. Please, find my replies below.
> 
> On Sun, Aug 05, 2018 at 10:15:43AM +0200, jacopo mondi wrote:
> > Helle Eugeniu,
> > 
> > On Sun, Aug 05, 2018 at 01:11:09AM +0200, Eugeniu Rosca wrote:
> > > In harmony with ATF and U-Boot outputs [1] and [2], the new board is
> > > based on M3-N revision ES1.1 and the amount of memory present on SiP
> > > is 2GiB, contiguously addressed.
> > 
> > Not sure why the amount of installed system memory is relevant for
> > this commit..
> 
> To be honest, I don't know precisely what's encoded in the board string
> (particularly the one documented in this commit RTP0RC77965SKBX010SA00).
> 
> The only thing unmistakenly present there is the SoC model 77965 (i.e.
> M3-N), but I am quite clueless about the rest.
> 
> What I can say for sure is that the end-user experience of a R-Car Gen3
> reference board clearly depends on below parameters:
>  - [A] SoC (model, revision) including SRAM and on-chip peripherals
>  - [B] DRAM (amount, linear/2ch/4ch split) 
>  - [C] Hyperflash (amount)
>  - [D] board's PCB (revision)
>  - [E] board's off-chip peripherals (model, revision)
> 
> I don't know how many of these parameters are embedded in the board
> string, but since you are suggesting that RAM amount is not (IOW
> Renesas will potentially release several RTP0RC77965SKBX010SA00
> boards with different amounts of memory), I will happily update
> the commit description.

Since I found two H3-ES2.0 Starter Kit samples with different amount of
RAM (4 vs 8 GiB) each having a unique board id [1], I take this as
evidence that Renesas encodes the amount of RAM in the board id.
Therefore, I will not change the description of this patch in v2,
unless you comment/NAK. Thank you.

[1] https://patchwork.kernel.org/patch/10555957/#22169325

Best regards,
Eugeniu.
Jacopo Mondi Aug. 10, 2018, 4:17 p.m. UTC | #4 
Hello Engeniu,

On Fri, Aug 10, 2018 at 03:48:44PM +0200, Eugeniu Rosca wrote:
> Hi again Jacopo,
>
> On Mon, Aug 06, 2018 at 12:40:02AM +0200, Eugeniu Rosca wrote:
> > Hi Jacopo,
> >
> > Thanks for your comments. Please, find my replies below.
> >
> > On Sun, Aug 05, 2018 at 10:15:43AM +0200, jacopo mondi wrote:
> > > Helle Eugeniu,
> > >
> > > On Sun, Aug 05, 2018 at 01:11:09AM +0200, Eugeniu Rosca wrote:
> > > > In harmony with ATF and U-Boot outputs [1] and [2], the new board is
> > > > based on M3-N revision ES1.1 and the amount of memory present on SiP
> > > > is 2GiB, contiguously addressed.
> > >
> > > Not sure why the amount of installed system memory is relevant for
> > > this commit..
> >
> > To be honest, I don't know precisely what's encoded in the board string
> > (particularly the one documented in this commit RTP0RC77965SKBX010SA00).
> >
> > The only thing unmistakenly present there is the SoC model 77965 (i.e.
> > M3-N), but I am quite clueless about the rest.
> >
> > What I can say for sure is that the end-user experience of a R-Car Gen3
> > reference board clearly depends on below parameters:
> >  - [A] SoC (model, revision) including SRAM and on-chip peripherals
> >  - [B] DRAM (amount, linear/2ch/4ch split)
> >  - [C] Hyperflash (amount)
> >  - [D] board's PCB (revision)
> >  - [E] board's off-chip peripherals (model, revision)
> >
> > I don't know how many of these parameters are embedded in the board
> > string, but since you are suggesting that RAM amount is not (IOW
> > Renesas will potentially release several RTP0RC77965SKBX010SA00
> > boards with different amounts of memory), I will happily update
> > the commit description.
>
> Since I found two H3-ES2.0 Starter Kit samples with different amount of
> RAM (4 vs 8 GiB) each having a unique board id [1], I take this as
> evidence that Renesas encodes the amount of RAM in the board id.
> Therefore, I will not change the description of this patch in v2,
> unless you comment/NAK. Thank you.
>

Thanks for digging into this, feel free to ignore my comment.


> [1] https://patchwork.kernel.org/patch/10555957/#22169325
>
> Best regards,
> Eugeniu.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/arm/shmobile.txt b/Documentation/devicetree/bindings/arm/shmobile.txt
index f391dba10574..2f3494a0107c 100644
--- a/Documentation/devicetree/bindings/arm/shmobile.txt
+++ b/Documentation/devicetree/bindings/arm/shmobile.txt
@@ -106,6 +106,8 @@  Boards:
     compatible = "renesas,lager", "renesas,r8a7790"
   - M3ULCB (R-Car Starter Kit Pro, RTP0RC7796SKBX0010SA09 (M3 ES1.0))
     compatible = "renesas,ulcb", "renesas,r8a7796"
+  - M3-N ULCB (R-Car Starter Kit Pro, RTP0RC77965SKBX010SA00 (M3-N ES1.1))
+    compatible = "renesas,ulcb", "renesas,r8a77965"
   - Marzen (R0P7779A00010S)
     compatible = "renesas,marzen", "renesas,r8a7779"
   - Porter (M2-LCDP)