| Message ID | 20210805065429.27485-4-clin@suse.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | arm64: dts: initial NXP S32G2 support | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/dt-meta-schema | success | |
| robh/dtbs-check | success |
On 05.08.21 08:54, Chester Lin wrote: > Add a compatible string for the uart binding of NXP S32G2 platforms. Here > we use "s32v234-linflexuart" as fallback since the current linflexuart > driver still works on S32G2. > > Signed-off-by: Chester Lin <clin@suse.com> > --- > .../bindings/serial/fsl,s32-linflexuart.yaml | 26 ++++++++++++++++--- > 1 file changed, 22 insertions(+), 4 deletions(-) > > diff --git a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml > index acfe34706ccb..e731f3f6cea4 100644 > --- a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml > +++ b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml > @@ -21,10 +21,20 @@ allOf: > > properties: > compatible: > - description: The LINFlexD controller on S32V234 SoC, which can be > - configured in UART mode. > - items: > - - const: fsl,s32v234-linflexuart > + minItems: 1 > + maxItems: 2 Are these necessary for oneOf? > + oneOf: > + - description: The LINFlexD controller on S32G2 SoC, which can be > + configured in UART mode. > + items: > + - enum: > + - fsl,s32g2-linflexuart > + - const: fsl,s32v234-linflexuart This reads inconsistent to me: Either this oneOf is for S32G2 only, then please turn the enum into a const. Or change the description to "on SoCs compatible with S32V234" if we expect the enum list to grow. I believe the idea here was to avoid unnecessary driver compatible and earlycon compatible additions, while preparing for eventual quirks specific to S32G2. @NXP: Should this be s32g2- like above or s32g274a- specifically? Do you agree this is a useful thing to prepare here, as opposed to using only s32v234- in the s32g2* DT? I assume the ordering is done alphabetically as S32G < S32V; alternatively we might order S32V234 first and then the compatible ones. > + > + - description: The LINFlexD controller on S32V234 SoC, which can be > + configured in UART mode. > + items: > + - const: fsl,s32v234-linflexuart To minimize this S32G2 patch, would it be valid to do oneOf for the single S32V in the preceding patch already? Then we would avoid the text movement and re-indentation above and more easily see the lines newly getting added for S32G2. > > reg: > maxItems: 1 > @@ -41,8 +51,16 @@ unevaluatedProperties: false > > examples: > - | > + /* S32V234 */ Could this be: - description: S32V234 | ? > serial@40053000 { > compatible = "fsl,s32v234-linflexuart"; > reg = <0x40053000 0x1000>; > interrupts = <0 59 4>; > }; > + > + /* S32G2 */ This should not be part of the S32V example, but a new one: - | (or with description, as discussed above) > + serial@401c8000 { > + compatible = "fsl,s32g2-linflexuart", "fsl,s32v234-linflexuart"; Potentially affected by naming discussions above. > + reg = <0x401c8000 0x3000>; > + interrupts = <0 82 1>; > + }; Regards, Andreas
On Thu, 2021-08-12 at 18:27 +0200, Andreas Färber wrote: > On 05.08.21 08:54, Chester Lin wrote: > > Add a compatible string for the uart binding of NXP S32G2 > > platforms. Here > > we use "s32v234-linflexuart" as fallback since the current > > linflexuart > > driver still works on S32G2. > > > > Signed-off-by: Chester Lin <clin@suse.com> > > --- > > .../bindings/serial/fsl,s32-linflexuart.yaml | 26 > > ++++++++++++++++--- > > 1 file changed, 22 insertions(+), 4 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/serial/fsl,s32- > > linflexuart.yaml > > b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml > > index acfe34706ccb..e731f3f6cea4 100644 > > --- a/Documentation/devicetree/bindings/serial/fsl,s32- > > linflexuart.yaml > > +++ b/Documentation/devicetree/bindings/serial/fsl,s32- > > linflexuart.yaml > > @@ -21,10 +21,20 @@ allOf: > > > > properties: > > compatible: > > - description: The LINFlexD controller on S32V234 SoC, which can > > be > > - configured in UART mode. > > - items: > > - - const: fsl,s32v234-linflexuart > > + minItems: 1 > > + maxItems: 2 > > Are these necessary for oneOf? > > > + oneOf: > > + - description: The LINFlexD controller on S32G2 SoC, which > > can be > > + configured in UART mode. > > + items: > > + - enum: > > + - fsl,s32g2-linflexuart > > + - const: fsl,s32v234-linflexuart > > This reads inconsistent to me: Either this oneOf is for S32G2 only, > then > please turn the enum into a const. Or change the description to "on > SoCs > compatible with S32V234" if we expect the enum list to grow. > > I believe the idea here was to avoid unnecessary driver compatible > and > earlycon compatible additions, while preparing for eventual quirks > specific to S32G2. > > @NXP: Should this be s32g2- like above or s32g274a- specifically? Do > you > agree this is a useful thing to prepare here, as opposed to using > only > s32v234- in the s32g2* DT? s32g2- is fine, but the vendor should be nxp, not fsl. nxp,s32g2-linflexuart > > I assume the ordering is done alphabetically as S32G < S32V; > alternatively we might order S32V234 first and then the compatible > ones. > > > + > > + - description: The LINFlexD controller on S32V234 SoC, which > > can be > > + configured in UART mode. > > + items: > > + - const: fsl,s32v234-linflexuart > > To minimize this S32G2 patch, would it be valid to do oneOf for the > single S32V in the preceding patch already? Then we would avoid the > text > movement and re-indentation above and more easily see the lines newly > getting added for S32G2. > > > > > reg: > > maxItems: 1 > > @@ -41,8 +51,16 @@ unevaluatedProperties: false > > > > examples: > > - | > > + /* S32V234 */ > > Could this be: > - description: S32V234 > | > ? > > > serial@40053000 { > > compatible = "fsl,s32v234-linflexuart"; > > reg = <0x40053000 0x1000>; > > interrupts = <0 59 4>; > > }; > > + > > + /* S32G2 */ > > This should not be part of the S32V example, but a new one: > > - | > > (or with description, as discussed above) > > > + serial@401c8000 { > > + compatible = "fsl,s32g2-linflexuart", "fsl,s32v234- > > linflexuart"; > > Potentially affected by naming discussions above. > > > + reg = <0x401c8000 0x3000>; > > + interrupts = <0 82 1>; > > + }; > > Regards, > Andreas >
On Thu, Aug 05, 2021 at 02:54:24PM +0800, Chester Lin wrote: > Add a compatible string for the uart binding of NXP S32G2 platforms. Here > we use "s32v234-linflexuart" as fallback since the current linflexuart > driver still works on S32G2. > > Signed-off-by: Chester Lin <clin@suse.com> > --- > .../bindings/serial/fsl,s32-linflexuart.yaml | 26 ++++++++++++++++--- > 1 file changed, 22 insertions(+), 4 deletions(-) > > diff --git a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml > index acfe34706ccb..e731f3f6cea4 100644 > --- a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml > +++ b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml > @@ -21,10 +21,20 @@ allOf: > > properties: > compatible: > - description: The LINFlexD controller on S32V234 SoC, which can be > - configured in UART mode. > - items: > - - const: fsl,s32v234-linflexuart > + minItems: 1 > + maxItems: 2 minItems/maxItems not needed here. > + oneOf: > + - description: The LINFlexD controller on S32G2 SoC, which can be > + configured in UART mode. > + items: > + - enum: > + - fsl,s32g2-linflexuart > + - const: fsl,s32v234-linflexuart > + > + - description: The LINFlexD controller on S32V234 SoC, which can be > + configured in UART mode. > + items: > + - const: fsl,s32v234-linflexuart > > reg: > maxItems: 1 > @@ -41,8 +51,16 @@ unevaluatedProperties: false > > examples: > - | > + /* S32V234 */ > serial@40053000 { > compatible = "fsl,s32v234-linflexuart"; > reg = <0x40053000 0x1000>; > interrupts = <0 59 4>; > }; > + > + /* S32G2 */ > + serial@401c8000 { > + compatible = "fsl,s32g2-linflexuart", "fsl,s32v234-linflexuart"; > + reg = <0x401c8000 0x3000>; > + interrupts = <0 82 1>; > + }; This doesn't really warrant another example just for a new compatible string.
On Thu, Aug 12, 2021 at 06:27:57PM +0200, Andreas Färber wrote: > On 05.08.21 08:54, Chester Lin wrote: > > Add a compatible string for the uart binding of NXP S32G2 platforms. Here > > we use "s32v234-linflexuart" as fallback since the current linflexuart > > driver still works on S32G2. > > > > Signed-off-by: Chester Lin <clin@suse.com> > > --- > > .../bindings/serial/fsl,s32-linflexuart.yaml | 26 ++++++++++++++++--- > > 1 file changed, 22 insertions(+), 4 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml > > index acfe34706ccb..e731f3f6cea4 100644 > > --- a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml > > +++ b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml > > @@ -21,10 +21,20 @@ allOf: > > > > properties: > > compatible: > > - description: The LINFlexD controller on S32V234 SoC, which can be > > - configured in UART mode. > > - items: > > - - const: fsl,s32v234-linflexuart > > + minItems: 1 > > + maxItems: 2 > > Are these necessary for oneOf? > > > + oneOf: > > + - description: The LINFlexD controller on S32G2 SoC, which can be > > + configured in UART mode. > > + items: > > + - enum: > > + - fsl,s32g2-linflexuart > > + - const: fsl,s32v234-linflexuart > > This reads inconsistent to me: Either this oneOf is for S32G2 only, then > please turn the enum into a const. Or change the description to "on SoCs > compatible with S32V234" if we expect the enum list to grow. > > I believe the idea here was to avoid unnecessary driver compatible and > earlycon compatible additions, while preparing for eventual quirks > specific to S32G2. > > @NXP: Should this be s32g2- like above or s32g274a- specifically? Do you > agree this is a useful thing to prepare here, as opposed to using only > s32v234- in the s32g2* DT? > > I assume the ordering is done alphabetically as S32G < S32V; > alternatively we might order S32V234 first and then the compatible ones. > > > + > > + - description: The LINFlexD controller on S32V234 SoC, which can be > > + configured in UART mode. > > + items: > > + - const: fsl,s32v234-linflexuart > > To minimize this S32G2 patch, would it be valid to do oneOf for the > single S32V in the preceding patch already? Then we would avoid the text > movement and re-indentation above and more easily see the lines newly > getting added for S32G2. > > > > > reg: > > maxItems: 1 > > @@ -41,8 +51,16 @@ unevaluatedProperties: false > > > > examples: > > - | > > + /* S32V234 */ > > Could this be: > - description: S32V234 > | > ? No, that would not be valid json-schema. Rob
diff --git a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml index acfe34706ccb..e731f3f6cea4 100644 --- a/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml +++ b/Documentation/devicetree/bindings/serial/fsl,s32-linflexuart.yaml @@ -21,10 +21,20 @@ allOf: properties: compatible: - description: The LINFlexD controller on S32V234 SoC, which can be - configured in UART mode. - items: - - const: fsl,s32v234-linflexuart + minItems: 1 + maxItems: 2 + oneOf: + - description: The LINFlexD controller on S32G2 SoC, which can be + configured in UART mode. + items: + - enum: + - fsl,s32g2-linflexuart + - const: fsl,s32v234-linflexuart + + - description: The LINFlexD controller on S32V234 SoC, which can be + configured in UART mode. + items: + - const: fsl,s32v234-linflexuart reg: maxItems: 1 @@ -41,8 +51,16 @@ unevaluatedProperties: false examples: - | + /* S32V234 */ serial@40053000 { compatible = "fsl,s32v234-linflexuart"; reg = <0x40053000 0x1000>; interrupts = <0 59 4>; }; + + /* S32G2 */ + serial@401c8000 { + compatible = "fsl,s32g2-linflexuart", "fsl,s32v234-linflexuart"; + reg = <0x401c8000 0x3000>; + interrupts = <0 82 1>; + };
Add a compatible string for the uart binding of NXP S32G2 platforms. Here we use "s32v234-linflexuart" as fallback since the current linflexuart driver still works on S32G2. Signed-off-by: Chester Lin <clin@suse.com> --- .../bindings/serial/fsl,s32-linflexuart.yaml | 26 ++++++++++++++++--- 1 file changed, 22 insertions(+), 4 deletions(-)