[v3,1/2] dt-bindings: pinctrl: add schema for NXP S32 SoCs

  Add DT schema for the pinctrl driver of NXP S32 SoC family.

Signed-off-by: Larisa Grigore <larisa.grigore@nxp.com>
Signed-off-by: Ghennadi Procopciuc <Ghennadi.Procopciuc@oss.nxp.com>
Signed-off-by: Chester Lin <clin@suse.com>
---

Changes in v3:
- Remove the minItems from reg because there's no optional item for s32g2.
- List supported properties of pinmux-node and pincfg-node and add more
  descriptions.
- Adjust the location of "required:".
- Fix descriptions and wordings.
- Rename the yaml file to nxp,s32g2-siul2-pinctrl.yaml.

Changes in v2:
- Remove the "nxp,pins" property since it has been moved into the driver.
- Add descriptions for reg entries.
- Refine the compatible name from "nxp,s32g-..." to "nxp,s32g2-...".
- Fix schema issues and revise the example.
- Fix the copyright format suggested by NXP.

 .../pinctrl/nxp,s32g2-siul2-pinctrl.yaml      | 129 ++++++++++++++++++
 1 file changed, 129 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pinctrl/nxp,s32g2-siul2-pinctrl.yaml
  

Hi Chester,

> +patternProperties:
> +  '-pins$':

Sorry, I missed this in the previous versions. Could you change it to '_pins' (underscore)? In our .dts files we use underscore in the names for pinctrl configuration nodes e.g. i2c4_pins, usbotg_pins.

> +    patternProperties:
> +      '-grp[0-9]$':

The same comment also applies here.

> +        llce-can0-pins {
> +            llce-can0-grp0 {

This should also change to something like:
llce_can0_pins {
  llce_can0_grp0 {
    ...
  }
}

Otherwise, this and PATCH 2/2 look good to me.

Best regards,
Andrei
  

On 21/12/2022 13:28, Andrei Stefanescu wrote:
> Hi Chester,
> 
>> +patternProperties:
>> +  '-pins$':
> 
> Sorry, I missed this in the previous versions. Could you change it to '_pins' (underscore)? In our .dts files we use underscore in the names for pinctrl configuration nodes e.g. i2c4_pins, usbotg_pins.

You cannot have underscores as node names, so what do you mean here? You
need to fix your DTS not introduce bad practices to mainline kernel.

Best regards,
Krzysztof
  

Hi Krzysztof and Andrei,

Thanks for reviewing this patch!

On Wed, Dec 21, 2022 at 01:30:12PM +0100, Krzysztof Kozlowski wrote:
> On 21/12/2022 13:28, Andrei Stefanescu wrote:
> > Hi Chester,
> > 
> >> +patternProperties:
> >> +  '-pins$':
> > 
> > Sorry, I missed this in the previous versions. Could you change it to '_pins' (underscore)? In our .dts files we use underscore in the names for pinctrl configuration nodes e.g. i2c4_pins, usbotg_pins.
> 
> You cannot have underscores as node names, so what do you mean here? You

Krzysztof is right, and Rob also reminded me in his comments in v1:

https://lore.kernel.org/linux-arm-kernel/20221102154903.GA3726664-robh@kernel.org/

Regards,
Chester

> need to fix your DTS not introduce bad practices to mainline kernel.
> 
> Best regards,
> Krzysztof
>
  

On 21/12/2022 08:32, Chester Lin wrote:
> Add DT schema for the pinctrl driver of NXP S32 SoC family.
> 
> Signed-off-by: Larisa Grigore <larisa.grigore@nxp.com>
> Signed-off-by: Ghennadi Procopciuc <Ghennadi.Procopciuc@oss.nxp.com>
> Signed-off-by: Chester Lin <clin@suse.com>
> ---
> 
> Changes in v3:
> - Remove the minItems from reg because there's no optional item for s32g2.
> - List supported properties of pinmux-node and pincfg-node and add more
>   descriptions.
> - Adjust the location of "required:".
> - Fix descriptions and wordings.
> - Rename the yaml file to nxp,s32g2-siul2-pinctrl.yaml.
> 
> Changes in v2:
> - Remove the "nxp,pins" property since it has been moved into the driver.
> - Add descriptions for reg entries.
> - Refine the compatible name from "nxp,s32g-..." to "nxp,s32g2-...".
> - Fix schema issues and revise the example.
> - Fix the copyright format suggested by NXP.
> 
>  .../pinctrl/nxp,s32g2-siul2-pinctrl.yaml      | 129 ++++++++++++++++++
>  1 file changed, 129 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pinctrl/nxp,s32g2-siul2-pinctrl.yaml
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/nxp,s32g2-siul2-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/nxp,s32g2-siul2-pinctrl.yaml
> new file mode 100644
> index 000000000000..1554ce14214a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/nxp,s32g2-siul2-pinctrl.yaml
> @@ -0,0 +1,129 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright 2022 NXP
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pinctrl/nxp,s32g2-siul2-pinctrl.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NXP S32G2 pin controller
> +
> +maintainers:
> +  - Ghennadi Procopciuc <Ghennadi.Procopciuc@oss.nxp.com>
> +  - Chester Lin <clin@suse.com>
> +
> +description: |
> +  S32G2 pinmux is implemented in SIUL2 (System Integration Unit Lite2),
> +  whose memory map is split into two regions:
> +    SIUL2_0 @ 0x4009c000
> +    SIUL2_1 @ 0x44010000
> +
> +  Every SIUL2 region has multiple register types, and here only MSCR and
> +  IMCR registers need to be revealed for kernel to configure pinmux.
> +
> +  Please note that some register indexes are reserved in S32G2, such as
> +  MSCR102-MSCR111, MSCR123-MSCR143, IMCR84-IMCR118 and IMCR398-IMCR429.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - nxp,s32g2-siul2-pinctrl
> +
> +  reg:
> +    description: |
> +      A list of MSCR/IMCR register regions to be reserved.
> +      - MSCR (Multiplexed Signal Configuration Register)
> +        An MSCR register can configure the associated pin as either a GPIO pin
> +        or a function output pin depends on the selected signal source.
> +      - IMCR (Input Multiplexed Signal Configuration Register)
> +        An IMCR register can configure the associated pin as function input
> +        pin depends on the selected signal source.
> +    items:
> +      - description: MSCR registers group 0 in SIUL2_0
> +      - description: MSCR registers group 1 in SIUL2_1
> +      - description: MSCR registers group 2 in SIUL2_1
> +      - description: IMCR registers group 0 in SIUL2_0
> +      - description: IMCR registers group 1 in SIUL2_1
> +      - description: IMCR registers group 2 in SIUL2_1
> +
> +patternProperties:
> +  '-pins$':
> +    type: object
> +    additionalProperties: false
> +
> +    patternProperties:
> +      '-grp[0-9]$':
> +        type: object
> +        allOf:
> +          - $ref: pinmux-node.yaml#
> +          - $ref: pincfg-node.yaml#
> +        description: |
> +          Pinctrl node's client devices specify pin muxes using subnodes,
> +          which in turn use the standard properties below.
> +
> +        properties:
> +          bias-disable: true
> +          bias-high-impedance: true
> +          bias-pull-up: true
> +          bias-pull-down: true
> +          drive-open-drain: true
> +          input-enable: true
> +          output-enable: true
> +
> +          pinmux:
> +            description: |
> +               An integer array for representing pinmux configurations of
> +               a device. Each integer consists of a PIN_ID and a 4-bit
> +               selected signal source(SSS) as IOMUX setting, which is
> +               calculated as: pinmux = (PIN_ID << 4 | SSS)
> +
> +          slew-rate:
> +            description: |
> +              0: 208MHz
> +              1-3: Reserved
> +              4: 166MHz
> +              5: 150MHz
> +              6: 133MHz
> +              7: 83MHz
> +            enum: [0, 4, 5, 6, 7]

You have known values, then use them. This is much more readable in DTS.


Best regards,
Krzysztof
  

Hi Krzysztof,

On Thu, Dec 22, 2022 at 12:28:31PM +0100, Krzysztof Kozlowski wrote:
> On 21/12/2022 08:32, Chester Lin wrote:
> > Add DT schema for the pinctrl driver of NXP S32 SoC family.
> > 
> > Signed-off-by: Larisa Grigore <larisa.grigore@nxp.com>
> > Signed-off-by: Ghennadi Procopciuc <Ghennadi.Procopciuc@oss.nxp.com>
> > Signed-off-by: Chester Lin <clin@suse.com>
> > ---
> > 
> > Changes in v3:
> > - Remove the minItems from reg because there's no optional item for s32g2.
> > - List supported properties of pinmux-node and pincfg-node and add more
> >   descriptions.
> > - Adjust the location of "required:".
> > - Fix descriptions and wordings.
> > - Rename the yaml file to nxp,s32g2-siul2-pinctrl.yaml.
> > 
> > Changes in v2:
> > - Remove the "nxp,pins" property since it has been moved into the driver.
> > - Add descriptions for reg entries.
> > - Refine the compatible name from "nxp,s32g-..." to "nxp,s32g2-...".
> > - Fix schema issues and revise the example.
> > - Fix the copyright format suggested by NXP.
> > 
> >  .../pinctrl/nxp,s32g2-siul2-pinctrl.yaml      | 129 ++++++++++++++++++
> >  1 file changed, 129 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/pinctrl/nxp,s32g2-siul2-pinctrl.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/pinctrl/nxp,s32g2-siul2-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/nxp,s32g2-siul2-pinctrl.yaml
> > new file mode 100644
> > index 000000000000..1554ce14214a
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pinctrl/nxp,s32g2-siul2-pinctrl.yaml
> > @@ -0,0 +1,129 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +# Copyright 2022 NXP
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/pinctrl/nxp,s32g2-siul2-pinctrl.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: NXP S32G2 pin controller
> > +
> > +maintainers:
> > +  - Ghennadi Procopciuc <Ghennadi.Procopciuc@oss.nxp.com>
> > +  - Chester Lin <clin@suse.com>
> > +
> > +description: |
> > +  S32G2 pinmux is implemented in SIUL2 (System Integration Unit Lite2),
> > +  whose memory map is split into two regions:
> > +    SIUL2_0 @ 0x4009c000
> > +    SIUL2_1 @ 0x44010000
> > +
> > +  Every SIUL2 region has multiple register types, and here only MSCR and
> > +  IMCR registers need to be revealed for kernel to configure pinmux.
> > +
> > +  Please note that some register indexes are reserved in S32G2, such as
> > +  MSCR102-MSCR111, MSCR123-MSCR143, IMCR84-IMCR118 and IMCR398-IMCR429.
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - nxp,s32g2-siul2-pinctrl
> > +
> > +  reg:
> > +    description: |
> > +      A list of MSCR/IMCR register regions to be reserved.
> > +      - MSCR (Multiplexed Signal Configuration Register)
> > +        An MSCR register can configure the associated pin as either a GPIO pin
> > +        or a function output pin depends on the selected signal source.
> > +      - IMCR (Input Multiplexed Signal Configuration Register)
> > +        An IMCR register can configure the associated pin as function input
> > +        pin depends on the selected signal source.
> > +    items:
> > +      - description: MSCR registers group 0 in SIUL2_0
> > +      - description: MSCR registers group 1 in SIUL2_1
> > +      - description: MSCR registers group 2 in SIUL2_1
> > +      - description: IMCR registers group 0 in SIUL2_0
> > +      - description: IMCR registers group 1 in SIUL2_1
> > +      - description: IMCR registers group 2 in SIUL2_1
> > +
> > +patternProperties:
> > +  '-pins$':
> > +    type: object
> > +    additionalProperties: false
> > +
> > +    patternProperties:
> > +      '-grp[0-9]$':
> > +        type: object
> > +        allOf:
> > +          - $ref: pinmux-node.yaml#
> > +          - $ref: pincfg-node.yaml#
> > +        description: |
> > +          Pinctrl node's client devices specify pin muxes using subnodes,
> > +          which in turn use the standard properties below.
> > +
> > +        properties:
> > +          bias-disable: true
> > +          bias-high-impedance: true
> > +          bias-pull-up: true
> > +          bias-pull-down: true
> > +          drive-open-drain: true
> > +          input-enable: true
> > +          output-enable: true
> > +
> > +          pinmux:
> > +            description: |
> > +               An integer array for representing pinmux configurations of
> > +               a device. Each integer consists of a PIN_ID and a 4-bit
> > +               selected signal source(SSS) as IOMUX setting, which is
> > +               calculated as: pinmux = (PIN_ID << 4 | SSS)
> > +
> > +          slew-rate:
> > +            description: |
> > +              0: 208MHz
> > +              1-3: Reserved
> > +              4: 166MHz
> > +              5: 150MHz
> > +              6: 133MHz
> > +              7: 83MHz
> > +            enum: [0, 4, 5, 6, 7]
> 
> You have known values, then use them. This is much more readable in DTS.

The main reason of mapping with register values [0-7] is to simplify the
driver implementation while handling register r/w. To improve readability
as you suggested, I am thinking of having a DT header "s32g2-pinfunc.h" with
a few binding macros/helper as below, the only difference compared to v3 is
using S32G2_PINMUX and S32G2_SLEW_XXXMHZ macros rather than pure integers
to represent pinmux and slew-rate property values.

Regards,
Chester


From 3a29d905ae104e694230ffc02dc9f9de4191c5d1 Mon Sep 17 00:00:00 2001
From: Chester Lin <clin@suse.com>
Date: Fri, 28 Oct 2022 16:44:29 +0800
Subject: [PATCH] dt-bindings: pinctrl: add support for NXP S32 SoCs

Add DT schema and hedaer file for the pinctrl driver of NXP S32 SoC family.

Signed-off-by: Larisa Grigore <larisa.grigore@nxp.com>
Signed-off-by: Ghennadi Procopciuc <Ghennadi.Procopciuc@oss.nxp.com>
Signed-off-by: Chester Lin <clin@suse.com>
---
 .../pinctrl/nxp,s32g2-siul2-pinctrl.yaml      | 136 ++++++++++++++++++
 include/dt-bindings/pinctrl/s32g2-pinfunc.h   |  17 +++
 2 files changed, 153 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pinctrl/nxp,s32g2-siul2-pinctrl.yaml
 create mode 100644 include/dt-bindings/pinctrl/s32g2-pinfunc.h

diff --git a/Documentation/devicetree/bindings/pinctrl/nxp,s32g2-siul2-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/nxp,s32g2-siul2-pinctrl.yaml
new file mode 100644
index 000000000000..9083d299dd85
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/nxp,s32g2-siul2-pinctrl.yaml
@@ -0,0 +1,136 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright 2022 NXP
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pinctrl/nxp,s32g2-siul2-pinctrl.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NXP S32G2 pin controller
+
+maintainers:
+  - Ghennadi Procopciuc <Ghennadi.Procopciuc@oss.nxp.com>
+  - Chester Lin <clin@suse.com>
+
+description: |
+  S32G2 pinmux is implemented in SIUL2 (System Integration Unit Lite2),
+  whose memory map is split into two regions:
+    SIUL2_0 @ 0x4009c000
+    SIUL2_1 @ 0x44010000
+
+  Every SIUL2 region has multiple register types, and here only MSCR and
+  IMCR registers need to be revealed for kernel to configure pinmux.
+
+  Please note that some register indexes are reserved in S32G2, such as
+  MSCR102-MSCR111, MSCR123-MSCR143, IMCR84-IMCR118 and IMCR398-IMCR429.
+
+properties:
+  compatible:
+    enum:
+      - nxp,s32g2-siul2-pinctrl
+
+  reg:
+    description: |
+      A list of MSCR/IMCR register regions to be reserved.
+      - MSCR (Multiplexed Signal Configuration Register)
+        An MSCR register can configure the associated pin as either a GPIO pin
+        or a function output pin depends on the selected signal source.
+      - IMCR (Input Multiplexed Signal Configuration Register)
+        An IMCR register can configure the associated pin as function input
+        pin depends on the selected signal source.
+    items:
+      - description: MSCR registers group 0 in SIUL2_0
+      - description: MSCR registers group 1 in SIUL2_1
+      - description: MSCR registers group 2 in SIUL2_1
+      - description: IMCR registers group 0 in SIUL2_0
+      - description: IMCR registers group 1 in SIUL2_1
+      - description: IMCR registers group 2 in SIUL2_1
+
+patternProperties:
+  '-pins$':
+    type: object
+    additionalProperties: false
+
+    patternProperties:
+      '-grp[0-9]$':
+        type: object
+        allOf:
+          - $ref: pinmux-node.yaml#
+          - $ref: pincfg-node.yaml#
+        description: |
+          Pinctrl node's client devices specify pin muxes using subnodes,
+          which in turn use the standard properties below.
+
+        properties:
+          bias-disable: true
+          bias-high-impedance: true
+          bias-pull-up: true
+          bias-pull-down: true
+          drive-open-drain: true
+          input-enable: true
+          output-enable: true
+
+          pinmux:
+            description: |
+               An integer array for representing pinmux configurations of
+               a device. Each integer consists of a PIN_ID and a 4-bit
+               selected signal source(SSS) as IOMUX setting, which is
+               calculated as: pinmux = (PIN_ID << 4 | SSS)
+               It's recommended that using the helper macro S32G2_PINMUX()
+               defined in dt-bindings/pinctrl/s32g2-pinfunc.h in order to
+               assemble a pinmux value.
+
+          slew-rate:
+            description: |
+              0: 208MHz
+              1-3: Reserved
+              4: 166MHz
+              5: 150MHz
+              6: 133MHz
+              7: 83MHz
+              The available slew rates are defined as macros in
+              dt-bindings/pinctrl/s32g2-pinfunc.h.
+            enum: [0, 4, 5, 6, 7]
+
+        additionalProperties: false
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/pinctrl/s32g2-pinfunc.h>
+
+    pinctrl@4009c240 {
+        compatible = "nxp,s32g2-siul2-pinctrl";
+
+              /* MSCR0-MSCR101 registers on siul2_0 */
+        reg = <0x4009c240 0x198>,
+              /* MSCR112-MSCR122 registers on siul2_1 */
+              <0x44010400 0x2c>,
+              /* MSCR144-MSCR190 registers on siul2_1 */
+              <0x44010480 0xbc>,
+              /* IMCR0-IMCR83 registers on siul2_0 */
+              <0x4009ca40 0x150>,
+              /* IMCR119-IMCR397 registers on siul2_1 */
+              <0x44010c1c 0x45c>,
+              /* IMCR430-IMCR495 registers on siul2_1 */
+              <0x440110f8 0x108>;
+
+        llce-can0-pins {
+            llce-can0-grp0 {
+                pinmux = <S32G2_PINMUX(43, 0)>;
+                input-enable;
+                slew-rate = <S32G2_SLEW_208MHZ>;
+            };
+
+            llce-can0-grp1 {
+                pinmux = <S32G2_PINMUX(44, 2)>;
+                output-enable;
+                slew-rate = <S32G2_SLEW_208MHZ>;
+            };
+        };
+    };
+...
diff --git a/include/dt-bindings/pinctrl/s32g2-pinfunc.h b/include/dt-bindings/pinctrl/s32g2-pinfunc.h
new file mode 100644
index 000000000000..f5abf0388547
--- /dev/null
+++ b/include/dt-bindings/pinctrl/s32g2-pinfunc.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: (BSD-3-Clause OR GPL-2.0-or-later) */
+/*
+ * Copyright 2022 NXP
+ */
+
+#ifndef _DT_BINDINGS_S32G2_PINFUNC_H
+#define _DT_BINDINGS_S32G2_PINFUNC_H
+
+#define S32G2_PINMUX(PIN, FUNC) (((PIN) << 4) | (FUNC))
+
+#define S32G2_SLEW_208MHZ	0
+#define S32G2_SLEW_166MHZ	4
+#define S32G2_SLEW_150MHZ	5
+#define S32G2_SLEW_133MHZ	6
+#define S32G2_SLEW_83MHZ	7
+
+#endif
  

On 09/01/2023 08:04, Chester Lin wrote:
> Hi Krzysztof,
> 
> On Thu, Dec 22, 2022 at 12:28:31PM +0100, Krzysztof Kozlowski wrote:
>> On 21/12/2022 08:32, Chester Lin wrote:
>>> Add DT schema for the pinctrl driver of NXP S32 SoC family.
>>>
>>> Signed-off-by: Larisa Grigore <larisa.grigore@nxp.com>
>>> Signed-off-by: Ghennadi Procopciuc <Ghennadi.Procopciuc@oss.nxp.com>
>>> Signed-off-by: Chester Lin <clin@suse.com>
>>> ---
>>>
>>> Changes in v3:
>>> - Remove the minItems from reg because there's no optional item for s32g2.
>>> - List supported properties of pinmux-node and pincfg-node and add more
>>>   descriptions.
>>> - Adjust the location of "required:".
>>> - Fix descriptions and wordings.
>>> - Rename the yaml file to nxp,s32g2-siul2-pinctrl.yaml.
>>>
>>> Changes in v2:
>>> - Remove the "nxp,pins" property since it has been moved into the driver.
>>> - Add descriptions for reg entries.
>>> - Refine the compatible name from "nxp,s32g-..." to "nxp,s32g2-...".
>>> - Fix schema issues and revise the example.
>>> - Fix the copyright format suggested by NXP.
>>>
>>>  .../pinctrl/nxp,s32g2-siul2-pinctrl.yaml      | 129 ++++++++++++++++++
>>>  1 file changed, 129 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/pinctrl/nxp,s32g2-siul2-pinctrl.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/pinctrl/nxp,s32g2-siul2-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/nxp,s32g2-siul2-pinctrl.yaml
>>> new file mode 100644
>>> index 000000000000..1554ce14214a
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/pinctrl/nxp,s32g2-siul2-pinctrl.yaml
>>> @@ -0,0 +1,129 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +# Copyright 2022 NXP
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/pinctrl/nxp,s32g2-siul2-pinctrl.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: NXP S32G2 pin controller
>>> +
>>> +maintainers:
>>> +  - Ghennadi Procopciuc <Ghennadi.Procopciuc@oss.nxp.com>
>>> +  - Chester Lin <clin@suse.com>
>>> +
>>> +description: |
>>> +  S32G2 pinmux is implemented in SIUL2 (System Integration Unit Lite2),
>>> +  whose memory map is split into two regions:
>>> +    SIUL2_0 @ 0x4009c000
>>> +    SIUL2_1 @ 0x44010000
>>> +
>>> +  Every SIUL2 region has multiple register types, and here only MSCR and
>>> +  IMCR registers need to be revealed for kernel to configure pinmux.
>>> +
>>> +  Please note that some register indexes are reserved in S32G2, such as
>>> +  MSCR102-MSCR111, MSCR123-MSCR143, IMCR84-IMCR118 and IMCR398-IMCR429.
>>> +
>>> +properties:
>>> +  compatible:
>>> +    enum:
>>> +      - nxp,s32g2-siul2-pinctrl
>>> +
>>> +  reg:
>>> +    description: |
>>> +      A list of MSCR/IMCR register regions to be reserved.
>>> +      - MSCR (Multiplexed Signal Configuration Register)
>>> +        An MSCR register can configure the associated pin as either a GPIO pin
>>> +        or a function output pin depends on the selected signal source.
>>> +      - IMCR (Input Multiplexed Signal Configuration Register)
>>> +        An IMCR register can configure the associated pin as function input
>>> +        pin depends on the selected signal source.
>>> +    items:
>>> +      - description: MSCR registers group 0 in SIUL2_0
>>> +      - description: MSCR registers group 1 in SIUL2_1
>>> +      - description: MSCR registers group 2 in SIUL2_1
>>> +      - description: IMCR registers group 0 in SIUL2_0
>>> +      - description: IMCR registers group 1 in SIUL2_1
>>> +      - description: IMCR registers group 2 in SIUL2_1
>>> +
>>> +patternProperties:
>>> +  '-pins$':
>>> +    type: object
>>> +    additionalProperties: false
>>> +
>>> +    patternProperties:
>>> +      '-grp[0-9]$':
>>> +        type: object
>>> +        allOf:
>>> +          - $ref: pinmux-node.yaml#
>>> +          - $ref: pincfg-node.yaml#
>>> +        description: |
>>> +          Pinctrl node's client devices specify pin muxes using subnodes,
>>> +          which in turn use the standard properties below.
>>> +
>>> +        properties:
>>> +          bias-disable: true
>>> +          bias-high-impedance: true
>>> +          bias-pull-up: true
>>> +          bias-pull-down: true
>>> +          drive-open-drain: true
>>> +          input-enable: true
>>> +          output-enable: true
>>> +
>>> +          pinmux:
>>> +            description: |
>>> +               An integer array for representing pinmux configurations of
>>> +               a device. Each integer consists of a PIN_ID and a 4-bit
>>> +               selected signal source(SSS) as IOMUX setting, which is
>>> +               calculated as: pinmux = (PIN_ID << 4 | SSS)
>>> +
>>> +          slew-rate:
>>> +            description: |
>>> +              0: 208MHz
>>> +              1-3: Reserved
>>> +              4: 166MHz
>>> +              5: 150MHz
>>> +              6: 133MHz
>>> +              7: 83MHz
>>> +            enum: [0, 4, 5, 6, 7]
>>
>> You have known values, then use them. This is much more readable in DTS.
> 
> The main reason of mapping with register values [0-7] is to simplify the
> driver implementation while handling register r/w. 

Define bindings for the DTS, not for the drivers.

> To improve readability
> as you suggested, I am thinking of having a DT header "s32g2-pinfunc.h" with
> a few binding macros/helper as below, the only difference compared to v3 is
> using S32G2_PINMUX and S32G2_SLEW_XXXMHZ macros rather than pure integers
> to represent pinmux and slew-rate property values.

Binding headers is not a place for register values. By definition -
these are bindings, not hardware description. Hardware description is
DTS. Feel free to store them in DTS headers, but anyway this does not
solve the issue here.

The issue is: you store register values in DTS, which is limited, not
extendable description. Each of your devices would need entirely
different binding for this because register values can change between
every SoC version. Several other pinctrl bindings use similar approach,
but they have not got a clear mapping to values (e.g. they have fast and
slow). For the case with real values, use the same solution as
drive-strength - real values.

> 
> Regards,
> Chester
> 
> 
> From 3a29d905ae104e694230ffc02dc9f9de4191c5d1 Mon Sep 17 00:00:00 2001
> From: Chester Lin <clin@suse.com>
> Date: Fri, 28 Oct 2022 16:44:29 +0800
> Subject: [PATCH] dt-bindings: pinctrl: add support for NXP S32 SoCs
> 
> Add DT schema and hedaer file for the pinctrl driver of NXP S32 SoC family.
> 
> Signed-off-by: Larisa Grigore <larisa.grigore@nxp.com>
> Signed-off-by: Ghennadi Procopciuc <Ghennadi.Procopciuc@oss.nxp.com>
> Signed-off-by: Chester Lin <clin@suse.com>
> ---
>  .../pinctrl/nxp,s32g2-siul2-pinctrl.yaml      | 136 ++++++++++++++++++
>  include/dt-bindings/pinctrl/s32g2-pinfunc.h   |  17 +++

NAK for bindings.


Best regards,
Krzysztof