diff mbox series

[v4,1/5] dt-bindings: arm: renesas: Document Renesas RZ/G2UL SMARC EVK

Message ID 20220402073234.24625-1-biju.das.jz@bp.renesas.com (mailing list archive)
State Superseded
Delegated to: Geert Uytterhoeven
Headers show
Series Add Renesas RZ/G2UL Type-1 {SoC,SMARC EVK} support | expand

Commit Message

Biju Das April 2, 2022, 7:32 a.m. UTC
Document the Renesas SMARC EVK board which is based on the Renesas
RZ/G2UL Type-1 (R9A07G043U11) SoC.  The SMARC EVK consists of an
RZ/G2UL Type-1 SoM module and a SMARC carrier board.  The SoM module
sits on top of the carrier board.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
V4:
* new patch
---
 Documentation/devicetree/bindings/arm/renesas.yaml | 2 ++
 1 file changed, 2 insertions(+)

Comments

Krzysztof Kozlowski April 2, 2022, 4:36 p.m. UTC | #1
On 02/04/2022 09:32, Biju Das wrote:
> Document the Renesas SMARC EVK board which is based on the Renesas
> RZ/G2UL Type-1 (R9A07G043U11) SoC.  The SMARC EVK consists of an
> RZ/G2UL Type-1 SoM module and a SMARC carrier board.  The SoM module
> sits on top of the carrier board.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
> V4:
> * new patch
> ---
>  Documentation/devicetree/bindings/arm/renesas.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/renesas.yaml b/Documentation/devicetree/bindings/arm/renesas.yaml
> index fa435d6fda77..f61807103867 100644
> --- a/Documentation/devicetree/bindings/arm/renesas.yaml
> +++ b/Documentation/devicetree/bindings/arm/renesas.yaml
> @@ -405,6 +405,8 @@ properties:
>  
>        - description: RZ/G2UL (R9A07G043)
>          items:
> +          - enum:
> +              - renesas,smarc-evk # SMARC EVK

I see you are using same compatible for different configurations. I
think it should be rather a specific compatible (e.g.
renesas,smarc-evk-r9a07g043). It's the most detailed compatible, so the
user is expected to check it and have the answer about specific board.
Here it won't work - you have three different configurations with the
same, most specific compatible.

Best regards,
Krzysztof
Biju Das April 2, 2022, 7:05 p.m. UTC | #2
Hi Krzysztof Kozlowski,

Thanks for the feedback.

> Subject: Re: [PATCH v4 1/5] dt-bindings: arm: renesas: Document Renesas
> RZ/G2UL SMARC EVK
> 
> On 02/04/2022 09:32, Biju Das wrote:
> > Document the Renesas SMARC EVK board which is based on the Renesas
> > RZ/G2UL Type-1 (R9A07G043U11) SoC.  The SMARC EVK consists of an
> > RZ/G2UL Type-1 SoM module and a SMARC carrier board.  The SoM module
> > sits on top of the carrier board.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > ---
> > V4:
> > * new patch
> > ---
> >  Documentation/devicetree/bindings/arm/renesas.yaml | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/arm/renesas.yaml
> > b/Documentation/devicetree/bindings/arm/renesas.yaml
> > index fa435d6fda77..f61807103867 100644
> > --- a/Documentation/devicetree/bindings/arm/renesas.yaml
> > +++ b/Documentation/devicetree/bindings/arm/renesas.yaml
> > @@ -405,6 +405,8 @@ properties:
> >
> >        - description: RZ/G2UL (R9A07G043)
> >          items:
> > +          - enum:
> > +              - renesas,smarc-evk # SMARC EVK
> 
> I see you are using same compatible for different configurations. I think
> it should be rather a specific compatible (e.g.
> renesas,smarc-evk-r9a07g043). It's the most detailed compatible, so the
> user is expected to check it and have the answer about specific board.
> Here it won't work - you have three different configurations with the
> same, most specific compatible.

SMARC-EVK is common to RZ/G2L(R9A07G044L), RZ/G2LC(R9A07G044C) , RZ/V2L(R9A07G054L),
RZ/G2UL Type-1(r9a07g043u11) and RZ/Five(r9a07g043f) SoC's.

For consistency I have made similar change. So you recommend to change
Other SoC's as well?

SMARC-EVK is common carrier board and We have a SoM module which contains SoC.

R9A07G043 is generic compatible for RZ/G2UL arm based SoC and RZ/Five RISC
Based SoC.

Do I miss any thing compared to other existing  renesas SoC's, please let me know.

Cheers,
Biju
Krzysztof Kozlowski April 2, 2022, 7:20 p.m. UTC | #3
On 02/04/2022 21:05, Biju Das wrote:
> Hi Krzysztof Kozlowski,
> 
> Thanks for the feedback.
> 
>> Subject: Re: [PATCH v4 1/5] dt-bindings: arm: renesas: Document Renesas
>> RZ/G2UL SMARC EVK
>>
>> On 02/04/2022 09:32, Biju Das wrote:
>>> Document the Renesas SMARC EVK board which is based on the Renesas
>>> RZ/G2UL Type-1 (R9A07G043U11) SoC.  The SMARC EVK consists of an
>>> RZ/G2UL Type-1 SoM module and a SMARC carrier board.  The SoM module
>>> sits on top of the carrier board.
>>>
>>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
>>> ---
>>> V4:
>>> * new patch
>>> ---
>>>  Documentation/devicetree/bindings/arm/renesas.yaml | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/arm/renesas.yaml
>>> b/Documentation/devicetree/bindings/arm/renesas.yaml
>>> index fa435d6fda77..f61807103867 100644
>>> --- a/Documentation/devicetree/bindings/arm/renesas.yaml
>>> +++ b/Documentation/devicetree/bindings/arm/renesas.yaml
>>> @@ -405,6 +405,8 @@ properties:
>>>
>>>        - description: RZ/G2UL (R9A07G043)
>>>          items:
>>> +          - enum:
>>> +              - renesas,smarc-evk # SMARC EVK
>>
>> I see you are using same compatible for different configurations. I think
>> it should be rather a specific compatible (e.g.
>> renesas,smarc-evk-r9a07g043). It's the most detailed compatible, so the
>> user is expected to check it and have the answer about specific board.
>> Here it won't work - you have three different configurations with the
>> same, most specific compatible.
> 
> SMARC-EVK is common to RZ/G2L(R9A07G044L), RZ/G2LC(R9A07G044C) , RZ/V2L(R9A07G054L),
> RZ/G2UL Type-1(r9a07g043u11) and RZ/Five(r9a07g043f) SoC's.
> 
> For consistency I have made similar change. So you recommend to change
> Other SoC's as well?
> 
> SMARC-EVK is common carrier board and We have a SoM module which contains SoC.
> 
> R9A07G043 is generic compatible for RZ/G2UL arm based SoC and RZ/Five RISC
> Based SoC.
> 
> Do I miss any thing compared to other existing  renesas SoC's, please let me know.

I understand that carrier board is the same, so the SoM differs. In your
model to figure out what type of hardware is it, your choice is to
compare two compatibles:
renesas,smarc-evk + renesas,r9a07g043u11

If user-space compares only last compatible, it get's only SMARC, so it
does not know on what hardware it runs.

Maybe I am wrong, but the combination of compatibles should not be used
as a specific description. IOW, only one, final compatible should
determine the type of hardware.

Therefore in your case it should be:
renesas,smarc-evk-r9a07g043u11 + renesas,r9a07g043u11 + renesas,r9a07g043

That's my understanding of compatibles from Devicetree spec.

Such approach is used in other boards, you can check for example Toradex
and Variscite boards in arm/fsl.yaml.

For example VAR-SOM-MX8MM is a SoM and it's boards are:
 - const: variscite,var-som-mx8mm-symphony

 - const: variscite,var-som-mx8mm
 - const: fsl,imx8mm


The first const could be an enum, but there are no other boards, except
Symphony kit. Symphony can be used with other modules as well, so there is:
variscite,var-som-mx8mn-symphony
(notice 8mm -> 8mn)

Best regards,
Krzysztof
Biju Das April 2, 2022, 7:54 p.m. UTC | #4
Hi Krzysztof Kozlowski,

Thanks for the feedback.

> Subject: Re: [PATCH v4 1/5] dt-bindings: arm: renesas: Document Renesas
> RZ/G2UL SMARC EVK
> 
> On 02/04/2022 21:05, Biju Das wrote:
> > Hi Krzysztof Kozlowski,
> >
> > Thanks for the feedback.
> >
> >> Subject: Re: [PATCH v4 1/5] dt-bindings: arm: renesas: Document
> >> Renesas RZ/G2UL SMARC EVK
> >>
> >> On 02/04/2022 09:32, Biju Das wrote:
> >>> Document the Renesas SMARC EVK board which is based on the Renesas
> >>> RZ/G2UL Type-1 (R9A07G043U11) SoC.  The SMARC EVK consists of an
> >>> RZ/G2UL Type-1 SoM module and a SMARC carrier board.  The SoM module
> >>> sits on top of the carrier board.
> >>>
> >>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> >>> ---
> >>> V4:
> >>> * new patch
> >>> ---
> >>>  Documentation/devicetree/bindings/arm/renesas.yaml | 2 ++
> >>>  1 file changed, 2 insertions(+)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/arm/renesas.yaml
> >>> b/Documentation/devicetree/bindings/arm/renesas.yaml
> >>> index fa435d6fda77..f61807103867 100644
> >>> --- a/Documentation/devicetree/bindings/arm/renesas.yaml
> >>> +++ b/Documentation/devicetree/bindings/arm/renesas.yaml
> >>> @@ -405,6 +405,8 @@ properties:
> >>>
> >>>        - description: RZ/G2UL (R9A07G043)
> >>>          items:
> >>> +          - enum:
> >>> +              - renesas,smarc-evk # SMARC EVK
> >>
> >> I see you are using same compatible for different configurations. I
> >> think it should be rather a specific compatible (e.g.
> >> renesas,smarc-evk-r9a07g043). It's the most detailed compatible, so
> >> the user is expected to check it and have the answer about specific
> board.
> >> Here it won't work - you have three different configurations with the
> >> same, most specific compatible.
> >
> > SMARC-EVK is common to RZ/G2L(R9A07G044L), RZ/G2LC(R9A07G044C) ,
> > RZ/V2L(R9A07G054L), RZ/G2UL Type-1(r9a07g043u11) and RZ/Five(r9a07g043f)
> SoC's.
> >
> > For consistency I have made similar change. So you recommend to change
> > Other SoC's as well?
> >
> > SMARC-EVK is common carrier board and We have a SoM module which
> contains SoC.
> >
> > R9A07G043 is generic compatible for RZ/G2UL arm based SoC and RZ/Five
> > RISC Based SoC.
> >
> > Do I miss any thing compared to other existing  renesas SoC's, please
> let me know.
> 
> I understand that carrier board is the same, so the SoM differs.

For R9A07G043 case, even SoM is same, only SOC differs.

>In your
> model to figure out what type of hardware is it, your choice is to compare
> two compatibles:
> renesas,smarc-evk + renesas,r9a07g043u11
> 
> If user-space compares only last compatible, it get's only SMARC, so it
> does not know on what hardware it runs.

But Here user-space can easily identify the H/W with existing scheme. See the logs from user-space.

/ # for i in machine family soc_id revision; do echo -n "$i: "; cat /sys/devices/soc0/$i;done
machine: Renesas SMARC EVK based on r9a07g043u11
family: RZ/G2UL
soc_id: r9a07g043
revision: 0

Cheers,
Biju
Krzysztof Kozlowski April 2, 2022, 8:03 p.m. UTC | #5
On 02/04/2022 21:54, Biju Das wrote:
>>
>> I understand that carrier board is the same, so the SoM differs.
> 
> For R9A07G043 case, even SoM is same, only SOC differs.

I assumed that you cannot have same SoMs with different SoCs...

> 
>> In your
>> model to figure out what type of hardware is it, your choice is to compare
>> two compatibles:
>> renesas,smarc-evk + renesas,r9a07g043u11
>>
>> If user-space compares only last compatible, it get's only SMARC, so it
>> does not know on what hardware it runs.
> 
> But Here user-space can easily identify the H/W with existing scheme. See the logs from user-space.
> 
> / # for i in machine family soc_id revision; do echo -n "$i: "; cat /sys/devices/soc0/$i;done
> machine: Renesas SMARC EVK based on r9a07g043u11
> family: RZ/G2UL
> soc_id: r9a07g043
> revision: 0

User-space is one example. We don't limit to this. Anyway, the
compatible is the main way to check it. Machine is just test, not
compatible, not part of ABI. soc_id and revision could help, but these
are separate ABIs. They can be not compiled-in and then you have only
compatible.

Regardless whether there is another way for user-space to figure out
hardware, it does not change the fact that such usage of compatibles
does not look correct with Devicetree spec.
"...They
 allow a device to express its compatibility with a family of similar
devices, potentially allowing a single
 device driver to match against several devices."

The "renesas,smarc-evk" compatible is not the most specific one, because
different configurations have it.

Again - you intend to use a pair or even a triple of compatibles to
uniquely identify type of hardware. I don't think it is correct - the
final, most specific compatible, uniquely identifies the hardware. All
other compatibles are just for fallback.

Best regards,
Krzysztof
Biju Das April 2, 2022, 8:48 p.m. UTC | #6
Hi Krzysztof Kozlowski,

Thanks for the feedback.

> Subject: Re: [PATCH v4 1/5] dt-bindings: arm: renesas: Document Renesas
> RZ/G2UL SMARC EVK
> 
> On 02/04/2022 21:54, Biju Das wrote:
> >>
> >> I understand that carrier board is the same, so the SoM differs.
> >
> > For R9A07G043 case, even SoM is same, only SOC differs.
> 
> I assumed that you cannot have same SoMs with different SoCs...

OK, What I meant here is pin-mapping of 2 SoC's(RZ/G2UL and RZ/Five) are same on the SoM.

> 
> >
> >> In your
> >> model to figure out what type of hardware is it, your choice is to
> >> compare two compatibles:
> >> renesas,smarc-evk + renesas,r9a07g043u11
> >>
> >> If user-space compares only last compatible, it get's only SMARC, so
> >> it does not know on what hardware it runs.
> >
> > But Here user-space can easily identify the H/W with existing scheme.
> See the logs from user-space.
> >
> > / # for i in machine family soc_id revision; do echo -n "$i: "; cat
> > /sys/devices/soc0/$i;done
> > machine: Renesas SMARC EVK based on r9a07g043u11
> > family: RZ/G2UL
> > soc_id: r9a07g043
> > revision: 0
> 
> User-space is one example. We don't limit to this. Anyway, the compatible
> is the main way to check it. Machine is just test, not compatible, not
> part of ABI. soc_id and revision could help, but these are separate ABIs.
> They can be not compiled-in and then you have only compatible.
> 
> Regardless whether there is another way for user-space to figure out
> hardware, it does not change the fact that such usage of compatibles does
> not look correct with Devicetree spec.
> "...They
>  allow a device to express its compatibility with a family of similar
> devices, potentially allowing a single  device driver to match against
> several devices."

Soc specific driver compatible will take care of this. If any quirks to be added
Soc-id and revision will take care that.

> 
> The "renesas,smarc-evk" compatible is not the most specific one, because
> different configurations have it.

It is just a compatible for the carrier board.

> 
> Again - you intend to use a pair or even a triple of compatibles to
> uniquely identify type of hardware. I don't think it is correct - the
> final, most specific compatible, uniquely identifies the hardware. All
> other compatibles are just for fallback.

See the examples we have discussed. Geert Do you agree with Krzysztof Kozlowski
Suggestion?


If we have 2 boards Board-a and Board-B based on SoCX
----------------------------------------------------

Case 1: As per your example with freescale,

Enum
 - Board-a-SoCX
 - Board-b-SoCX
Const
 - SoCX

Case 2: our case
Enum
 - Board-a
 - Board-b

Const
 - SoCX


If Same board used by different SoC's of same SoC faily
====================================

Case 1: As per your example with freescale,

Enum
 - Board-SoCA
 - Board-SoCB
Enum
 - SoCA
 - SoCB
Const
 - SoC

Case 2: Our case
Enum
 - Board
Enum
 - SoCA
 - SoCB
Const
 - SoC

Cheers,
Biju
Krzysztof Kozlowski April 3, 2022, 7:52 a.m. UTC | #7
On 02/04/2022 22:48, Biju Das wrote:
> Hi Krzysztof Kozlowski,
> 
> Thanks for the feedback.
> 
>> Subject: Re: [PATCH v4 1/5] dt-bindings: arm: renesas: Document Renesas
>> RZ/G2UL SMARC EVK
>>
>> On 02/04/2022 21:54, Biju Das wrote:
>>>>
>>>> I understand that carrier board is the same, so the SoM differs.
>>>
>>> For R9A07G043 case, even SoM is same, only SOC differs.
>>
>> I assumed that you cannot have same SoMs with different SoCs...
> 
> OK, What I meant here is pin-mapping of 2 SoC's(RZ/G2UL and RZ/Five) are same on the SoM.
> 
>>
>>>
>>>> In your
>>>> model to figure out what type of hardware is it, your choice is to
>>>> compare two compatibles:
>>>> renesas,smarc-evk + renesas,r9a07g043u11
>>>>
>>>> If user-space compares only last compatible, it get's only SMARC, so
>>>> it does not know on what hardware it runs.
>>>
>>> But Here user-space can easily identify the H/W with existing scheme.
>> See the logs from user-space.
>>>
>>> / # for i in machine family soc_id revision; do echo -n "$i: "; cat
>>> /sys/devices/soc0/$i;done
>>> machine: Renesas SMARC EVK based on r9a07g043u11
>>> family: RZ/G2UL
>>> soc_id: r9a07g043
>>> revision: 0
>>
>> User-space is one example. We don't limit to this. Anyway, the compatible
>> is the main way to check it. Machine is just test, not compatible, not
>> part of ABI. soc_id and revision could help, but these are separate ABIs.
>> They can be not compiled-in and then you have only compatible.
>>
>> Regardless whether there is another way for user-space to figure out
>> hardware, it does not change the fact that such usage of compatibles does
>> not look correct with Devicetree spec.
>> "...They
>>  allow a device to express its compatibility with a family of similar
>> devices, potentially allowing a single  device driver to match against
>> several devices."
> 
> Soc specific driver compatible will take care of this. If any quirks to be added
> Soc-id and revision will take care that.

soc_id is independent mechanism and is not related to the discussion
whether Devicetree binding is correct or not.

Soc specific compatible in the driver does not solve this case, because
the top level compatible describes the machine. In this case the machine
is SMARC and it is indistinguishable from other SMARC machines, even
though they are quite different.

> 
>>
>> The "renesas,smarc-evk" compatible is not the most specific one, because
>> different configurations have it.
> 
> It is just a compatible for the carrier board.

Yes, I understand that and you we agreed on that before.

The last compatible (counting from the right in the list), according to
devicetree, is the most specific compatible describing the device. Using
here compatible for a generic carrier board is therefore not correct
from the Devicetree point of view.

> 
>>
>> Again - you intend to use a pair or even a triple of compatibles to
>> uniquely identify type of hardware. I don't think it is correct - the
>> final, most specific compatible, uniquely identifies the hardware. All
>> other compatibles are just for fallback.
> 
> See the examples we have discussed. Geert Do you agree with Krzysztof Kozlowski
> Suggestion?
> 
> 
> If we have 2 boards Board-a and Board-B based on SoCX
> ----------------------------------------------------
> 
> Case 1: As per your example with freescale,
> 
> Enum
>  - Board-a-SoCX
>  - Board-b-SoCX
> Const
>  - SoCX
> 
> Case 2: our case
> Enum
>  - Board-a
>  - Board-b
> 
> Const
>  - SoCX
> 
> 
> If Same board used by different SoC's of same SoC faily
> ====================================
> 
> Case 1: As per your example with freescale,
> 
> Enum
>  - Board-SoCA
>  - Board-SoCB
> Enum
>  - SoCA
>  - SoCB
> Const
>  - SoC

No, that case will look different:

Const
 - Board-SoCA
 - SoCA
 - SoC

Const
 - Board-SoCB
 - SoCB
 - SoC

> 
> Case 2: Our case
> Enum
>  - Board
> Enum
>  - SoCA
>  - SoCB
> Const
>  - SoC
> 
> Cheers,
> Biju


Best regards,
Krzysztof
Geert Uytterhoeven April 4, 2022, 12:29 p.m. UTC | #8
Hi Krzysztof,

On Sat, Apr 2, 2022 at 10:03 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
> On 02/04/2022 21:54, Biju Das wrote:
> >> I understand that carrier board is the same, so the SoM differs.
> >
> > For R9A07G043 case, even SoM is same, only SOC differs.
>
> I assumed that you cannot have same SoMs with different SoCs...
>
> >
> >> In your
> >> model to figure out what type of hardware is it, your choice is to compare
> >> two compatibles:
> >> renesas,smarc-evk + renesas,r9a07g043u11
> >>
> >> If user-space compares only last compatible, it get's only SMARC, so it
> >> does not know on what hardware it runs.
> >
> > But Here user-space can easily identify the H/W with existing scheme. See the logs from user-space.
> >
> > / # for i in machine family soc_id revision; do echo -n "$i: "; cat /sys/devices/soc0/$i;done
> > machine: Renesas SMARC EVK based on r9a07g043u11
> > family: RZ/G2UL
> > soc_id: r9a07g043
> > revision: 0
>
> User-space is one example. We don't limit to this. Anyway, the
> compatible is the main way to check it. Machine is just test, not
> compatible, not part of ABI. soc_id and revision could help, but these
> are separate ABIs. They can be not compiled-in and then you have only
> compatible.
>
> Regardless whether there is another way for user-space to figure out
> hardware, it does not change the fact that such usage of compatibles
> does not look correct with Devicetree spec.
> "...They
>  allow a device to express its compatibility with a family of similar
> devices, potentially allowing a single
>  device driver to match against several devices."
>
> The "renesas,smarc-evk" compatible is not the most specific one, because
> different configurations have it.

From the letter of the spec, this is indeed not valid.
However, we started doing this several years ago, as the various
variants of the Salvator-X(S) and ULCB boards are identical, and just
differ in SoC (actually SiP) mounted.

E.g. arch/arm64/boot/dts/renesas/r8a77951-salvator-xs.dts has
compatible = "renesas,salvator-xs", "renesas,r8a7795".

While we could add e.g. "renesas,salvator-xs-r8a7791", doing so
would inflate the bindings a lot.

> Again - you intend to use a pair or even a triple of compatibles to
> uniquely identify type of hardware. I don't think it is correct - the
> final, most specific compatible, uniquely identifies the hardware. All
> other compatibles are just for fallback.

And what to do when adding more DT overlays for expansion boards?
This would become unmanageable soon.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Krzysztof Kozlowski April 4, 2022, 1 p.m. UTC | #9
On 04/04/2022 14:29, Geert Uytterhoeven wrote:
>>
>> User-space is one example. We don't limit to this. Anyway, the
>> compatible is the main way to check it. Machine is just test, not
>> compatible, not part of ABI. soc_id and revision could help, but these
>> are separate ABIs. They can be not compiled-in and then you have only
>> compatible.
>>
>> Regardless whether there is another way for user-space to figure out
>> hardware, it does not change the fact that such usage of compatibles
>> does not look correct with Devicetree spec.
>> "...They
>>  allow a device to express its compatibility with a family of similar
>> devices, potentially allowing a single
>>  device driver to match against several devices."
>>
>> The "renesas,smarc-evk" compatible is not the most specific one, because
>> different configurations have it.
> 
> From the letter of the spec, this is indeed not valid.

It is also invalid from logical meaning of compatibles... The generic
compatible (SMARC board) is not compatible with a specific SoC. It's the
other way around.

> However, we started doing this several years ago, as the various
> variants of the Salvator-X(S) and ULCB boards are identical, and just
> differ in SoC (actually SiP) mounted.
> 
> E.g. arch/arm64/boot/dts/renesas/r8a77951-salvator-xs.dts has
> compatible = "renesas,salvator-xs", "renesas,r8a7795".
> 
> While we could add e.g. "renesas,salvator-xs-r8a7791", doing so
> would inflate the bindings a lot.

That what is actually happening for example in SoM-like cases on NXP. I
understand that it grows, but that's why we have schema to find mistakes. :)

>> Again - you intend to use a pair or even a triple of compatibles to
>> uniquely identify type of hardware. I don't think it is correct - the
>> final, most specific compatible, uniquely identifies the hardware. All
>> other compatibles are just for fallback.
> 
> And what to do when adding more DT overlays for expansion boards?
> This would become unmanageable soon.

There are two topics here:
1. Whether we should follow DT spec. If no, why Renesas is special and
for other cases we follow DT spec? "Unmanageable" is not the answer
here, because other platforms will have the same problem.

2. If the answer for above is yes, we follow DT spec, then the question
is how to deal with overlays. In current code - I don't know. Long term,
maybe we need a way to append to existing compatible (to extend it)?
Some expansion boards do not need to change top level compatible,
because they only add constrained features (like Arduino shields with
some regulator). You just add it to DT and presence of new compatible,
e.g. of regulator, is enough. You do not need to change the top level
compatible.


Best regards,
Krzysztof
Biju Das April 5, 2022, 11:47 a.m. UTC | #10
> Subject: Re: [PATCH v4 1/5] dt-bindings: arm: renesas: Document Renesas
> RZ/G2UL SMARC EVK
> 
> On 04/04/2022 14:29, Geert Uytterhoeven wrote:
> >>
> >> User-space is one example. We don't limit to this. Anyway, the
> >> compatible is the main way to check it. Machine is just test, not
> >> compatible, not part of ABI. soc_id and revision could help, but
> >> these are separate ABIs. They can be not compiled-in and then you
> >> have only compatible.
> >>
> >> Regardless whether there is another way for user-space to figure out
> >> hardware, it does not change the fact that such usage of compatibles
> >> does not look correct with Devicetree spec.
> >> "...They
> >>  allow a device to express its compatibility with a family of similar
> >> devices, potentially allowing a single  device driver to match
> >> against several devices."
> >>
> >> The "renesas,smarc-evk" compatible is not the most specific one,
> >> because different configurations have it.
> >
> > From the letter of the spec, this is indeed not valid.
> 
> It is also invalid from logical meaning of compatibles... The generic
> compatible (SMARC board) is not compatible with a specific SoC. It's the
> other way around.
> 
> > However, we started doing this several years ago, as the various
> > variants of the Salvator-X(S) and ULCB boards are identical, and just
> > differ in SoC (actually SiP) mounted.
> >
> > E.g. arch/arm64/boot/dts/renesas/r8a77951-salvator-xs.dts has
> > compatible = "renesas,salvator-xs", "renesas,r8a7795".
> >
> > While we could add e.g. "renesas,salvator-xs-r8a7791", doing so would
> > inflate the bindings a lot.

Rob,

Any thoughts on this topic?

> 
> That what is actually happening for example in SoM-like cases on NXP. I
> understand that it grows, but that's why we have schema to find mistakes.
> :)
> 
> >> Again - you intend to use a pair or even a triple of compatibles to
> >> uniquely identify type of hardware. I don't think it is correct - the
> >> final, most specific compatible, uniquely identifies the hardware.
> >> All other compatibles are just for fallback.
> >
> > And what to do when adding more DT overlays for expansion boards?
> > This would become unmanageable soon.
> 
> There are two topics here:
> 1. Whether we should follow DT spec. If no, why Renesas is special and for
> other cases we follow DT spec? "Unmanageable" is not the answer here,
> because other platforms will have the same problem.
> 
> 2. If the answer for above is yes, we follow DT spec, then the question is
> how to deal with overlays. In current code - I don't know. Long term,
> maybe we need a way to append to existing compatible (to extend it)?
> Some expansion boards do not need to change top level compatible, because
> they only add constrained features (like Arduino shields with some
> regulator). You just add it to DT and presence of new compatible, e.g. of
> regulator, is enough. You do not need to change the top level compatible.
> 

Does the rules for compatible values (most to least descriptive) 
also apply to the root node?

Cheers,
Biju
Krzysztof Kozlowski April 5, 2022, 12:05 p.m. UTC | #11
On 05/04/2022 13:47, Biju Das wrote:
>> Subject: Re: [PATCH v4 1/5] dt-bindings: arm: renesas: Document Renesas
>> RZ/G2UL SMARC EVK
>>

(...)

>>>
>>> And what to do when adding more DT overlays for expansion boards?
>>> This would become unmanageable soon.
>>
>> There are two topics here:
>> 1. Whether we should follow DT spec. If no, why Renesas is special and for
>> other cases we follow DT spec? "Unmanageable" is not the answer here,
>> because other platforms will have the same problem.
>>
>> 2. If the answer for above is yes, we follow DT spec, then the question is
>> how to deal with overlays. In current code - I don't know. Long term,
>> maybe we need a way to append to existing compatible (to extend it)?
>> Some expansion boards do not need to change top level compatible, because
>> they only add constrained features (like Arduino shields with some
>> regulator). You just add it to DT and presence of new compatible, e.g. of
>> regulator, is enough. You do not need to change the top level compatible.
>>
> 
> Does the rules for compatible values (most to least descriptive) 
> also apply to the root node?

I don't see any exception in DT spec (page 26), so my answer is yes, the
root node has same meaning of "compatible" as other nodes.

Best regards,
Krzysztof
Rob Herring (Arm) April 6, 2022, 4:29 p.m. UTC | #12
On Tue, Apr 05, 2022 at 02:05:31PM +0200, Krzysztof Kozlowski wrote:
> On 05/04/2022 13:47, Biju Das wrote:
> >> Subject: Re: [PATCH v4 1/5] dt-bindings: arm: renesas: Document Renesas
> >> RZ/G2UL SMARC EVK
> >>
> 
> (...)
> 
> >>>
> >>> And what to do when adding more DT overlays for expansion boards?
> >>> This would become unmanageable soon.
> >>
> >> There are two topics here:
> >> 1. Whether we should follow DT spec. If no, why Renesas is special and for
> >> other cases we follow DT spec? "Unmanageable" is not the answer here,
> >> because other platforms will have the same problem.
> >>
> >> 2. If the answer for above is yes, we follow DT spec, then the question is
> >> how to deal with overlays. In current code - I don't know. Long term,
> >> maybe we need a way to append to existing compatible (to extend it)?
> >> Some expansion boards do not need to change top level compatible, because
> >> they only add constrained features (like Arduino shields with some
> >> regulator). You just add it to DT and presence of new compatible, e.g. of
> >> regulator, is enough. You do not need to change the top level compatible.
> >>
> > 
> > Does the rules for compatible values (most to least descriptive) 
> > also apply to the root node?
> 
> I don't see any exception in DT spec (page 26), so my answer is yes, the
> root node has same meaning of "compatible" as other nodes.

At the end of the day, what matters is can we identify what h/w we are 
running on from the value of 'compatible'. Either way I think the answer 
is yes.

The modern mixture of SoC, baseboard, SoM, expansion cards, etc. doesn't 
map perfectly to compatible's definition. If someone wants to define 
something, then that would be good. However, there's already existing 
practice to take into account.

Rob
Geert Uytterhoeven April 12, 2022, 10:28 a.m. UTC | #13
On Sat, Apr 2, 2022 at 9:32 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> Document the Renesas SMARC EVK board which is based on the Renesas
> RZ/G2UL Type-1 (R9A07G043U11) SoC.  The SMARC EVK consists of an
> RZ/G2UL Type-1 SoM module and a SMARC carrier board.  The SoM module
> sits on top of the carrier board.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
i.e. will queue in renesas-devel for v5.19.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/arm/renesas.yaml b/Documentation/devicetree/bindings/arm/renesas.yaml
index fa435d6fda77..f61807103867 100644
--- a/Documentation/devicetree/bindings/arm/renesas.yaml
+++ b/Documentation/devicetree/bindings/arm/renesas.yaml
@@ -405,6 +405,8 @@  properties:
 
       - description: RZ/G2UL (R9A07G043)
         items:
+          - enum:
+              - renesas,smarc-evk # SMARC EVK
           - enum:
               - renesas,r9a07g043u11 # RZ/G2UL Type-1
               - renesas,r9a07g043u12 # RZ/G2UL Type-2