diff mbox series

[1/8] dt-bindings: arm: fsl: add NXP S32G2 boards

Message ID 20210805065429.27485-2-clin@suse.com (mailing list archive)
State New, archived
Headers show
Series arm64: dts: initial NXP S32G2 support | expand

Commit Message

Chester Lin Aug. 5, 2021, 6:54 a.m. UTC
Add bindings for S32G2's evaluation board (S32G-VNP-EVB) and reference
design 2 board ( S32G-VNP-RDB2).

Signed-off-by: Chester Lin <clin@suse.com>
---
 Documentation/devicetree/bindings/arm/fsl.yaml | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Andreas Färber Aug. 12, 2021, 3:46 p.m. UTC | #1
Hello Rob and NXP,

On 05.08.21 08:54, Chester Lin wrote:
> Add bindings for S32G2's evaluation board (S32G-VNP-EVB) and reference
> design 2 board ( S32G-VNP-RDB2).
> 
> Signed-off-by: Chester Lin <clin@suse.com>
> ---
>  Documentation/devicetree/bindings/arm/fsl.yaml | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
> index e2097011c4b0..3914aa09e503 100644
> --- a/Documentation/devicetree/bindings/arm/fsl.yaml
> +++ b/Documentation/devicetree/bindings/arm/fsl.yaml
> @@ -983,6 +983,13 @@ properties:
>            - const: solidrun,lx2160a-cex7
>            - const: fsl,lx2160a
>  
> +      - description: S32G2 based Boards
> +        items:
> +          - enum:
> +              - fsl,s32g274a-evb
> +              - fsl,s32g274a-rdb2

@Rob: Should for new boards the description: syntax be used also for
enums? Or just at SoC level, and for board enums still traditional #
comments?

> +          - const: fsl,s32g2

@NXP: Is it sufficient here to have s32g2, or should we call this
s32g274a and adjust the description above to S32G274A?

Related, is the trailing A for Arm, like for the Layerscape chips? I.e.,
not for Alpha or rev.A or something that will change for non-eval chips?

> +
>        - description: S32V234 based Boards
>          items:
>            - enum:

Otherwise,

Reviewed-by: Andreas Färber <afaerber@suse.de>

Thanks,
Andreas
Rob Herring Aug. 13, 2021, 5:49 p.m. UTC | #2
On Thu, Aug 12, 2021 at 05:46:16PM +0200, Andreas Färber wrote:
> Hello Rob and NXP,
> 
> On 05.08.21 08:54, Chester Lin wrote:
> > Add bindings for S32G2's evaluation board (S32G-VNP-EVB) and reference
> > design 2 board ( S32G-VNP-RDB2).
> > 
> > Signed-off-by: Chester Lin <clin@suse.com>
> > ---
> >  Documentation/devicetree/bindings/arm/fsl.yaml | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
> > index e2097011c4b0..3914aa09e503 100644
> > --- a/Documentation/devicetree/bindings/arm/fsl.yaml
> > +++ b/Documentation/devicetree/bindings/arm/fsl.yaml
> > @@ -983,6 +983,13 @@ properties:
> >            - const: solidrun,lx2160a-cex7
> >            - const: fsl,lx2160a
> >  
> > +      - description: S32G2 based Boards
> > +        items:
> > +          - enum:
> > +              - fsl,s32g274a-evb
> > +              - fsl,s32g274a-rdb2
> 
> @Rob: Should for new boards the description: syntax be used also for
> enums? Or just at SoC level, and for board enums still traditional #
> comments?

It's up to how the platform wants to do it.

Rob
Rob Herring Aug. 13, 2021, 5:53 p.m. UTC | #3
On Thu, Aug 05, 2021 at 02:54:22PM +0800, Chester Lin wrote:
> Add bindings for S32G2's evaluation board (S32G-VNP-EVB) and reference
> design 2 board ( S32G-VNP-RDB2).
> 
> Signed-off-by: Chester Lin <clin@suse.com>
> ---
>  Documentation/devicetree/bindings/arm/fsl.yaml | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
> index e2097011c4b0..3914aa09e503 100644
> --- a/Documentation/devicetree/bindings/arm/fsl.yaml
> +++ b/Documentation/devicetree/bindings/arm/fsl.yaml
> @@ -983,6 +983,13 @@ properties:
>            - const: solidrun,lx2160a-cex7
>            - const: fsl,lx2160a
>  
> +      - description: S32G2 based Boards
> +        items:
> +          - enum:
> +              - fsl,s32g274a-evb
> +              - fsl,s32g274a-rdb2
> +          - const: fsl,s32g2

Given this is an entirely different family from i.MX and new?, shouldn't 
it use 'nxp' instead of 'fsl'? Either way,

Acked-by: Rob Herring <robh@kernel.org>

Rob
Chester Lin Aug. 18, 2021, 2:34 p.m. UTC | #4
On Fri, Aug 13, 2021 at 12:53:59PM -0500, Rob Herring wrote:
> On Thu, Aug 05, 2021 at 02:54:22PM +0800, Chester Lin wrote:
> > Add bindings for S32G2's evaluation board (S32G-VNP-EVB) and reference
> > design 2 board ( S32G-VNP-RDB2).
> > 
> > Signed-off-by: Chester Lin <clin@suse.com>
> > ---
> >  Documentation/devicetree/bindings/arm/fsl.yaml | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
> > index e2097011c4b0..3914aa09e503 100644
> > --- a/Documentation/devicetree/bindings/arm/fsl.yaml
> > +++ b/Documentation/devicetree/bindings/arm/fsl.yaml
> > @@ -983,6 +983,13 @@ properties:
> >            - const: solidrun,lx2160a-cex7
> >            - const: fsl,lx2160a
> >  
> > +      - description: S32G2 based Boards
> > +        items:
> > +          - enum:
> > +              - fsl,s32g274a-evb
> > +              - fsl,s32g274a-rdb2
> > +          - const: fsl,s32g2
> 
> Given this is an entirely different family from i.MX and new?, shouldn't 
> it use 'nxp' instead of 'fsl'? Either way,

It sounds good and Radu from NXP has mentioned a similar idea for the
compatible string of linflexuart. To keep the naming consistency, should we
change all 'fsl' to 'nxp' as well? For example, we could rename the fsl.yaml
to nxp.yaml. However, changing all of them would cause some impacts, which will
need more verifications on new strings. Otherwise we would have to tolerate the
naming differences only used by s32g2.

Thanks,
Chester

> 
> Acked-by: Rob Herring <robh@kernel.org>
> 
> Rob
>
Andreas Färber Sept. 6, 2021, 7:35 p.m. UTC | #5
On 13.08.21 19:53, Rob Herring wrote:
> On Thu, Aug 05, 2021 at 02:54:22PM +0800, Chester Lin wrote:
>> Add bindings for S32G2's evaluation board (S32G-VNP-EVB) and reference
>> design 2 board ( S32G-VNP-RDB2).
>>
>> Signed-off-by: Chester Lin <clin@suse.com>
>> ---
>>  Documentation/devicetree/bindings/arm/fsl.yaml | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
>> index e2097011c4b0..3914aa09e503 100644
>> --- a/Documentation/devicetree/bindings/arm/fsl.yaml
>> +++ b/Documentation/devicetree/bindings/arm/fsl.yaml
>> @@ -983,6 +983,13 @@ properties:
>>            - const: solidrun,lx2160a-cex7
>>            - const: fsl,lx2160a
>>  
>> +      - description: S32G2 based Boards
>> +        items:
>> +          - enum:
>> +              - fsl,s32g274a-evb
>> +              - fsl,s32g274a-rdb2
>> +          - const: fsl,s32g2
> 
> Given this is an entirely different family from i.MX and new?, shouldn't 
> it use 'nxp' instead of 'fsl'?

S32V also still used fsl prefix, despite the company name long being NXP
(same for several Layerscape and i.MX models).

If, as Radu indicated on 3/8, NXP wants to make that switch now for S32G
then I see no reason against nxp. I verified that it's already defined:

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/vendor-prefixes.yaml

However, should the matching .dts[i] files using nxp prefix (4-6/8) then
still go under dts/freescale/, or should they go to a new dts/nxp/ then?
That would separate it from S32V. Intel did do a switch from dts/altera/
to dts/intel/ at some point, so there's precedence for either, I guess.
No idea whether anything might break if we moved S32V alongside S32G.

Similarly, the easiest and most merge-friendly would be to leave
arm/fsl.yaml and add the nxp-prefixed S32G2 there, as done here. If NXP
want to rename fsl.yaml to nxp.yaml in a general housekeeping effort,
that could be done independently, outside Chester's patchset.

> Either way,
> 
> Acked-by: Rob Herring <robh@kernel.org>

Thanks,
Andreas
Andreas Färber Sept. 6, 2021, 8:38 p.m. UTC | #6
Hi Chester,

On 18.08.21 16:34, Chester Lin wrote:
> On Fri, Aug 13, 2021 at 12:53:59PM -0500, Rob Herring wrote:
>> On Thu, Aug 05, 2021 at 02:54:22PM +0800, Chester Lin wrote:
>>> Add bindings for S32G2's evaluation board (S32G-VNP-EVB) and reference
>>> design 2 board ( S32G-VNP-RDB2).
>>>
>>> Signed-off-by: Chester Lin <clin@suse.com>
>>> ---
>>>  Documentation/devicetree/bindings/arm/fsl.yaml | 7 +++++++
>>>  1 file changed, 7 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
>>> index e2097011c4b0..3914aa09e503 100644
>>> --- a/Documentation/devicetree/bindings/arm/fsl.yaml
>>> +++ b/Documentation/devicetree/bindings/arm/fsl.yaml
>>> @@ -983,6 +983,13 @@ properties:
>>>            - const: solidrun,lx2160a-cex7
>>>            - const: fsl,lx2160a
>>>  
>>> +      - description: S32G2 based Boards
>>> +        items:
>>> +          - enum:
>>> +              - fsl,s32g274a-evb
>>> +              - fsl,s32g274a-rdb2
>>> +          - const: fsl,s32g2
>>
>> Given this is an entirely different family from i.MX and new?, shouldn't 
>> it use 'nxp' instead of 'fsl'? Either way,
> 
> It sounds good and Radu from NXP has mentioned a similar idea for the
> compatible string of linflexuart. To keep the naming consistency, should we
> change all 'fsl' to 'nxp' as well?

I assume that question was just unclearly phrased, so for the record:

ABI stability rules forbid us from changing "all 'fsl'" in compatible
strings or property names.

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/ABI.rst

Deployed firmware providing mainline-merged platforms with DTBs using
fsl prefix (e.g., the quoted LX2160A) needs to continue working with
newer drivers, and deployed mainline Linux should continue working after
firmware updates that modify the DTB provided to Linux.

So, if NXP wants to use nxp prefix for new S32G bindings, you can do
that for your additions only, but for LINFlexD UART (3/8) you will still
need to use fsl for the "historical" S32V binding used as fallback.

Please keep S32G consistent with itself - so if we decide on nxp here,
we should apply it to SoC, boards, LINFlexD and any future peripherals.

> For example, we could rename the fsl.yaml
> to nxp.yaml.

Since other people might be contributing i.MX boards etc. to that file,
better not make your patch series conflict with other people's patches,
so that it can get merged and we can move on to the next patchsets.

The schema filename is not ABI, so it can be renamed later.

The .dtb path may become ABI (e.g., U-Boot $fdtfile), thus my comment
about consciously deciding between freescale/ vs. nxp/ subdirectory.

> However, changing all of them would cause some impacts, which will
> need more verifications on new strings. Otherwise we would have to tolerate the
> naming differences only used by s32g2.

I fear tolerating the mess one way or another is the only viable way.
Otherwise both bindings and drivers would need duplication for backwards
compatibility, for no good reason - Freescale was acquired back in 2015.

Cheers,
Andreas
Krzysztof Kozlowski Sept. 7, 2021, 6:59 a.m. UTC | #7
On Mon, 6 Sept 2021 at 22:38, Andreas Färber <afaerber@suse.de> wrote:
>
> Hi Chester,
>
> On 18.08.21 16:34, Chester Lin wrote:
> > On Fri, Aug 13, 2021 at 12:53:59PM -0500, Rob Herring wrote:
> >> On Thu, Aug 05, 2021 at 02:54:22PM +0800, Chester Lin wrote:
> >>> Add bindings for S32G2's evaluation board (S32G-VNP-EVB) and reference
> >>> design 2 board ( S32G-VNP-RDB2).
> >>>
> >>> Signed-off-by: Chester Lin <clin@suse.com>
> >>> ---
> >>>  Documentation/devicetree/bindings/arm/fsl.yaml | 7 +++++++
> >>>  1 file changed, 7 insertions(+)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
> >>> index e2097011c4b0..3914aa09e503 100644
> >>> --- a/Documentation/devicetree/bindings/arm/fsl.yaml
> >>> +++ b/Documentation/devicetree/bindings/arm/fsl.yaml
> >>> @@ -983,6 +983,13 @@ properties:
> >>>            - const: solidrun,lx2160a-cex7
> >>>            - const: fsl,lx2160a
> >>>
> >>> +      - description: S32G2 based Boards
> >>> +        items:
> >>> +          - enum:
> >>> +              - fsl,s32g274a-evb
> >>> +              - fsl,s32g274a-rdb2
> >>> +          - const: fsl,s32g2
> >>
> >> Given this is an entirely different family from i.MX and new?, shouldn't
> >> it use 'nxp' instead of 'fsl'? Either way,
> >
> > It sounds good and Radu from NXP has mentioned a similar idea for the
> > compatible string of linflexuart. To keep the naming consistency, should we
> > change all 'fsl' to 'nxp' as well?
>
> I assume that question was just unclearly phrased, so for the record:
>
> ABI stability rules forbid us from changing "all 'fsl'" in compatible
> strings or property names.
>
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/ABI.rst
>
> Deployed firmware providing mainline-merged platforms with DTBs using
> fsl prefix (e.g., the quoted LX2160A) needs to continue working with
> newer drivers, and deployed mainline Linux should continue working after
> firmware updates that modify the DTB provided to Linux.

This is a new platform/SoC therefore there is no ABI. There is no
requirement in the kernel that a new ABI (which you define in this
patchset in the bindings) should be compatible with something
somewhere. It's some misunderstanding of stable ABI. Therefore all new
compatibles are allowed to be nxp, not fsl.

No one here proposed renaming existing compatibles from fsl tro nxp.
We talk about new ones.

Different question of course whether you want to be nice to some
existing out-of-tree users... but then have in mind that we don't care
about out of tree. :) Anyway being nice to out-of-tree is not part of
ABI. It's just being nice and useful.

Best regards,
Krzysztof
Andreas Färber Sept. 7, 2021, 8:59 a.m. UTC | #8
Hi Krzysztof,

On 07.09.21 08:59, Krzysztof Kozlowski wrote:
> On Mon, 6 Sept 2021 at 22:38, Andreas Färber <afaerber@suse.de> wrote:
>> On 18.08.21 16:34, Chester Lin wrote:
>>> On Fri, Aug 13, 2021 at 12:53:59PM -0500, Rob Herring wrote:
>>>> On Thu, Aug 05, 2021 at 02:54:22PM +0800, Chester Lin wrote:
>>>>> Add bindings for S32G2's evaluation board (S32G-VNP-EVB) and reference
>>>>> design 2 board ( S32G-VNP-RDB2).
>>>>>
>>>>> Signed-off-by: Chester Lin <clin@suse.com>
>>>>> ---
>>>>>  Documentation/devicetree/bindings/arm/fsl.yaml | 7 +++++++
>>>>>  1 file changed, 7 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
>>>>> index e2097011c4b0..3914aa09e503 100644
>>>>> --- a/Documentation/devicetree/bindings/arm/fsl.yaml
>>>>> +++ b/Documentation/devicetree/bindings/arm/fsl.yaml
>>>>> @@ -983,6 +983,13 @@ properties:
>>>>>            - const: solidrun,lx2160a-cex7
>>>>>            - const: fsl,lx2160a
>>>>>
>>>>> +      - description: S32G2 based Boards
>>>>> +        items:
>>>>> +          - enum:
>>>>> +              - fsl,s32g274a-evb
>>>>> +              - fsl,s32g274a-rdb2
>>>>> +          - const: fsl,s32g2
>>>>
>>>> Given this is an entirely different family from i.MX and new?, shouldn't
>>>> it use 'nxp' instead of 'fsl'? Either way,
>>>
>>> It sounds good and Radu from NXP has mentioned a similar idea for the
>>> compatible string of linflexuart. To keep the naming consistency, should we
>>> change all 'fsl' to 'nxp' as well?
>>
>> I assume that question was just unclearly phrased, so for the record:
>>
>> ABI stability rules forbid us from changing "all 'fsl'" in compatible
>> strings or property names.
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/ABI.rst
>>
>> Deployed firmware providing mainline-merged platforms with DTBs using
>> fsl prefix (e.g., the quoted LX2160A) needs to continue working with
>> newer drivers, and deployed mainline Linux should continue working after
>> firmware updates that modify the DTB provided to Linux.
> 
> This is a new platform/SoC therefore there is no ABI. There is no
> requirement in the kernel that a new ABI (which you define in this
> patchset in the bindings) should be compatible with something
> somewhere. It's some misunderstanding of stable ABI. Therefore all new
> compatibles are allowed to be nxp, not fsl.
> 
> No one here proposed renaming existing compatibles from fsl tro nxp.
> We talk about new ones.

Chester seemingly did: "all 'fsl' ... as well", not "all new 'fsl'"
ones, in the patch context of existing fsl.yaml. Like I said, it may
just have been unluckily worded.

Therefore my saying that it does contain tons of non-new SoC/platform
bindings that he's not allowed to break by changing them.

> Different question of course whether you want to be nice to some
> existing out-of-tree users... but then have in mind that we don't care
> about out of tree. :) Anyway being nice to out-of-tree is not part of
> ABI. It's just being nice and useful.

Nobody is suggesting new S32G ABI be compatible with downstream BSPs.
These patches and changes we're discussing already differ from the BSP.

My point was that as soon as we merge S32G into mainline, it will become
ABI and shouldn't be changed incompatibly anymore once in a release.

These automotive platforms don't run off-the-shelf distros yet and will
need to get their bootloaders upstreamed, too. In particular we'll need
mainline TF-A to merge the SCMI implementation before we can rely on it
here in the kernel for a clk driver; that's holding up MMC and Ethernet.

Best regards,
Andreas
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
index e2097011c4b0..3914aa09e503 100644
--- a/Documentation/devicetree/bindings/arm/fsl.yaml
+++ b/Documentation/devicetree/bindings/arm/fsl.yaml
@@ -983,6 +983,13 @@  properties:
           - const: solidrun,lx2160a-cex7
           - const: fsl,lx2160a
 
+      - description: S32G2 based Boards
+        items:
+          - enum:
+              - fsl,s32g274a-evb
+              - fsl,s32g274a-rdb2
+          - const: fsl,s32g2
+
       - description: S32V234 based Boards
         items:
           - enum: