[PATCH v8 00/10] gpio: siul2-s32g2: add initial GPIO driver

[PATCH v8 00/10] gpio: siul2-s32g2: add initial GPIO driver

[Khristine Andreea Barbulescu]

This patch series adds support for basic GPIO
operations(set, get, direction_output/input, set_config).

There are two SIUL2 hardware modules: SIUL2_0 and SIUL2_1.
However, this driver exports both as a single GPIO driver.
This is because the interrupt registers are located only
in SIUL2_1, even for GPIOs that are part of SIUL2_0.

There are two gaps in the GPIO ranges:
- 102-111(inclusive) are invalid
- 123-143(inclusive) are invalid

Writing and reading GPIO values is done via the PGPDO/PGPDI
registers(Parallel GPIO Pad Data Output/Input) which are
16 bit registers, each bit corresponding to a GPIO.

Note that the PGPDO order is similar to a big-endian grouping
of two registers:
PGPDO1, PGPDO0, PGPDO3, PGPDO2, PGPDO5, PGPDO4, gap, PGPDO6.

v8 -> v7
- remove all ': true' lines from properties in dt bindings
- remove NVMEM MFD cell from SIUL2 in dtsi
- remove NVMEM driver and configs
- expose SoC information via syscon cells SIUL2_0
and SIUL2_1 in MFD driver
- add SIUL2_0 and SIUL2_1 syscon nodes in dtsi
- add patternProperties for "^siul2_[0-1]$" for syscon nodes
- update example to include syscon cells with proper format
- remove `reg` property from pinctrl node in dt binding
- update Kconfig help text to reflect new syscon structure
instead of NVMEM for SoC information
- squash deprecated SIUL2 pinctrl binding with new MFD binding
- dropped "nxp,s32g3-siul2" from MFD driver match table
- fixed commit messages
- fixed dtb warnings

v7 -> v6
- fixed MAINTAINERS wrong file path
- add unevaluatedProperties, change siul2 node name, remove
  jtag_pins label in the device tree schema
- change compatible definition in schema
- change node name in dtsi
- mentioned binding deprecation in commit messages
- split mfd cell conversion commit in two: one for the
  previous refactoring, one for the mfd cell conversion
- removed Acked-by: Linus Walleij from commit:
  "pinctrl: s32: convert the driver into an mfd cell"
  because of changes to that commit
- deprecate the nxp,s32g2-siul2-pinctrl binding
- add NVMEM MFD cell for SIUL2
- made the GPIO driver not export invalid pins
  (there are some gaps 102-111, 123-143)
- removed the need for gpio-reserved-ranges
- force initialized pinctrl_desc->num_custom_params to 0

v6 -> v5
- removed description for reg in the dt-bindings and added
  maxItems
- dropped label for example in the dt-bindings
- simplified the example in the dt-bindings
- changed dt-bindings filename to nxp,s32g2-siul2.yaml
- changed title in the dt-bindings
- dropped minItmes from gpio-ranges/gpio-reserved-ranges
  and added maxItems to gpio-reserved-ranges
- added required block for -grp[0-9]$ nodes
- switch to using "" as quotes
- kernel test robot: fixed frame sizes, added description
  for reg_name, fixed typo in gpio_configs_lock, removed
  uninitialized ret variable usage
- ordered includes in nxp-siul2.c, switched to dev-err-probe
  added a mention that other commits will add nvmem functionality
  to the mfd driver
- switched spin_lock_irqsave to scoped_guard statement
- switched dev_err to dev_err_probe in pinctrl-s32cc in places
  reached during the probing part

v5 -> v4
- fixed di_div error
- fixed dt-bindings error
- added Co-developed-by tags
- added new MFD driver nxp-siul2.c
- made the old pinctrl driver an MFD cell
- added the GPIO driver in the existing SIUL2 pinctrl one
- Switch from "devm_pinctrl_register" to
  "devm_pinctrl_register_and_init"

v4 -> v3
- removed useless parentheses
- added S32G3 fallback compatible
- fixed comment alignment
- fixed dt-bindings license
- fixed modpost: "__udivdi3"
- moved MAINTAINERS entry to have the new GPIO driver
  together with other files related to S32G

v3 -> v2
- fix dt-bindings schema id
- add maxItems to gpio-ranges
- removed gpio label from dt-bindings example
- added changelog for the MAINTAINERS commit and
  added separate entry for the SIUL2 GPIO driver
- added guard(raw_spinlock_irqsave) in
  'siul2_gpio_set_direction'
- updated the description for
  'devm_platform_get_and_ioremap_resource_byname'

v2 -> v1
dt-bindings:
- changed filename to match compatible
- fixed commit messages
- removed dt-bindings unnecessary properties descriptions
- added minItems for the interrupts property
driver:
- added depends on ARCH_S32 || COMPILE_TEST to Kconfig
- added select REGMAP_MMIO to Kconfig
- remove unnecessary include
- add of_node_put after `siul2_get_gpio_pinspec`
- removed inline from function definitions
- removed match data and moved the previous platdata
  definition to the top of the file to be visible
- replace bitmap_set/clear with __clear_bit/set_bit
  and devm_bitmap_zalloc with devm_kzalloc
- switched to gpiochip_generic_request/free/config
- fixed dev_err format for size_t reported by
  kernel test robot
- add platform_get_and_ioremap_resource_byname wrapper

Andrei Stefanescu (9):
  dt-bindings: mfd: add support for the NXP SIUL2 module
  mfd: nxp-siul2: add support for NXP SIUL2
  arm64: dts: s32g: change pinctrl node into the new mfd node
  pinctrl: s32cc: use dev_err_probe() and improve error messages
  pinctrl: s32cc: change to "devm_pinctrl_register_and_init"
  pinctrl: s32g2: change the driver to also be probed as an MFD cell
  pinctrl: s32cc: implement GPIO functionality
  MAINTAINERS: add MAINTAINER for NXP SIUL2 MFD driver
  pinctrl: s32cc: set num_custom_params to 0

Khristine Andreea Barbulescu (1):
  pinctrl: s32cc: skip syscon child nodes when parsing funcs and groups

 .../bindings/mfd/nxp,s32g2-siul2.yaml         | 165 +++++
 .../pinctrl/nxp,s32g2-siul2-pinctrl.yaml      |   2 +
 MAINTAINERS                                   |   2 +
 arch/arm64/boot/dts/freescale/s32g2.dtsi      |  35 +-
 arch/arm64/boot/dts/freescale/s32g3.dtsi      |  35 +-
 drivers/mfd/Kconfig                           |  13 +
 drivers/mfd/Makefile                          |   1 +
 drivers/mfd/nxp-siul2.c                       | 440 ++++++++++++
 drivers/pinctrl/nxp/pinctrl-s32.h             |   4 +-
 drivers/pinctrl/nxp/pinctrl-s32cc.c           | 652 ++++++++++++++----
 drivers/pinctrl/nxp/pinctrl-s32g2.c           |  32 +-
 include/linux/mfd/nxp-siul2.h                 |  55 ++
 12 files changed, 1259 insertions(+), 177 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mfd/nxp,s32g2-siul2.yaml
 create mode 100644 drivers/mfd/nxp-siul2.c
 create mode 100644 include/linux/mfd/nxp-siul2.h

-- 
2.50.1

[PATCH v8 01/10] dt-bindings: mfd: add support for the NXP SIUL2 module

[Khristine Andreea Barbulescu]

From: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>

Add the new dt-bindings for the NXP SIUL2 module which is a multi
function device. It can export information about the SoC, configure
the pinmux&pinconf for pins and it is also a GPIO controller with
interrupt capability.

The existing SIUL2 pinctrl bindings becomes deprecated because it
do not correctly describe the hardware. The SIUL2 module also
offers GPIO control and exposes some registers which contain
information about the SoC. Adding drivers for these functionalities
would result in incorrect bindings with a lot of carved out regions
for registers.

SIUL2 is a complex module that spans multiple register regions
and provides several functions: pinmux and pin configuration
through MSCR and IMCR registers, GPIO control through PGPDO
and PGPDI registers, interrupt configuration registers,
and SoC identification registers (MIDR).
These registers are grouped under two instances, SIUL2_0 and
SIUL2_1, and share the same functional context. The legacy
binding models SIUL2 as a standalone pinctrl node, which only
covers MSCR and IMCR.

Signed-off-by: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
Signed-off-by: Khristine Andreea Barbulescu <khristineandreea.barbulescu@oss.nxp.com>
---
 .../bindings/mfd/nxp,s32g2-siul2.yaml         | 165 ++++++++++++++++++
 .../pinctrl/nxp,s32g2-siul2-pinctrl.yaml      |   2 +
 2 files changed, 167 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/nxp,s32g2-siul2.yaml

diff --git a/Documentation/devicetree/bindings/mfd/nxp,s32g2-siul2.yaml b/Documentation/devicetree/bindings/mfd/nxp,s32g2-siul2.yaml
new file mode 100644
index 000000000000..ec743cf5f73e
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/nxp,s32g2-siul2.yaml
@@ -0,0 +1,165 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright 2024 NXP
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/nxp,s32g2-siul2.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NXP S32 System Integration Unit Lite2 (SIUL2)
+
+maintainers:
+  - Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
+
+description: |
+  SIUL2 is a hardware block which implements pinmuxing,
+  pinconf, GPIOs (some with interrupt capability) and
+  registers which contain information about the SoC.
+  There are generally two SIUL2 modules whose functionality
+  is grouped together. For example interrupt configuration
+  registers are part of SIUL2_1 even though interrupts are
+  also available for SIUL2_0 pins.
+
+  The following register types are exported by SIUL2:
+    - MIDR (MCU ID Register) - information related to the SoC
+    - interrupt configuration registers
+    - MSCR (Multiplexed Signal Configuration Register) - pinmuxing and pinconf
+    - IMCR (Input Multiplexed Signal Configuration Register)- pinmuxing
+    - PGPDO (Parallel GPIO Pad Data Out Register) - GPIO output value
+    - PGPDI (Parallel GPIO Pad Data In Register) - GPIO input value
+
+  Most registers are 32bit wide with the exception of PGPDO/PGPDI which are
+  16bit wide.
+
+properties:
+  compatible:
+    oneOf:
+      - const: nxp,s32g2-siul2
+      - items:
+          - enum:
+              - nxp,s32g3-siul2
+          - const: nxp,s32g2-siul2
+
+  gpio-controller: true
+
+  "#gpio-cells":
+    const: 2
+
+  gpio-ranges:
+    maxItems: 2
+
+  interrupts:
+    maxItems: 1
+
+  interrupt-controller: true
+
+  "#interrupt-cells":
+    const: 2
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 1
+
+  ranges:
+    description: Address translation ranges for child nodes.
+
+
+patternProperties:
+  "^siul2_[0-1]$":
+    type: object
+    description: SIUL2 hardware instances represented as syscon.
+    properties:
+      compatible:
+        const: syscon
+      reg:
+        maxItems: 1
+    required:
+      - compatible
+      - reg
+
+  "-hog(-[0-9]+)?$":
+    required:
+      - gpio-hog
+
+  "-pins$":
+    type: object
+    additionalProperties: false
+
+    patternProperties:
+      "-grp[0-9]$":
+        type: object
+        allOf:
+          - $ref: /schemas/pinctrl/pinmux-node.yaml#
+          - $ref: /schemas/pinctrl/pincfg-node.yaml#
+        description:
+          Pinctrl node's client devices specify pin muxes using subnodes,
+          which in turn use the standard properties below.
+
+        properties:
+          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: Supported slew rate based on Fmax values (MHz)
+            enum: [83, 133, 150, 166, 208]
+        required:
+          - pinmux
+
+        unevaluatedProperties: false
+
+required:
+  - compatible
+  - gpio-controller
+  - "#gpio-cells"
+  - gpio-ranges
+  - interrupts
+  - interrupt-controller
+  - "#interrupt-cells"
+  - "#address-cells"
+  - "#size-cells"
+  - ranges
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    pinctrl@4009c000 {
+      compatible = "nxp,s32g2-siul2";
+      gpio-controller;
+      #gpio-cells = <2>;
+      gpio-ranges = <&siul2 0 0 102>, <&siul2 112 112 79>;
+      interrupt-controller;
+      #interrupt-cells = <2>;
+      interrupts = <GIC_SPI 210 IRQ_TYPE_LEVEL_HIGH>;
+      #address-cells = <1>;
+      #size-cells = <1>;
+      ranges;
+
+      siul2_0: siul2_0@4009c000 {
+        compatible = "syscon";
+        reg = <0x0 0x4009c000 0x0 0x179c>;
+      };
+
+      siul2_1: siul2_1@44010000 {
+        compatible = "syscon";
+        reg = <0x0 0x44010000 0x0 0x17b0>;
+      };
+
+      jtag-pins {
+        jtag-grp0 {
+          pinmux = <0x0>;
+          input-enable;
+          bias-pull-up;
+          slew-rate = <166>;
+        };
+      };
+    };
+...
diff --git a/Documentation/devicetree/bindings/pinctrl/nxp,s32g2-siul2-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/nxp,s32g2-siul2-pinctrl.yaml
index a24286e4def6..332397a21394 100644
--- a/Documentation/devicetree/bindings/pinctrl/nxp,s32g2-siul2-pinctrl.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/nxp,s32g2-siul2-pinctrl.yaml
@@ -11,6 +11,8 @@ maintainers:
   - Ghennadi Procopciuc <Ghennadi.Procopciuc@oss.nxp.com>
   - Chester Lin <chester62515@gmail.com>
 
+deprecated: true
+
 description: |
   S32G2 pinmux is implemented in SIUL2 (System Integration Unit Lite2),
   whose memory map is split into two regions:
-- 
2.50.1

Re: [PATCH v8 01/10] dt-bindings: mfd: add support for the NXP SIUL2 module

[Rob Herring]

On Tue, Jan 20, 2026 at 01:59:13PM +0200, Khristine Andreea Barbulescu wrote:
> From: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
> 
> Add the new dt-bindings for the NXP SIUL2 module which is a multi
> function device. It can export information about the SoC, configure
> the pinmux&pinconf for pins and it is also a GPIO controller with
> interrupt capability.
> 
> The existing SIUL2 pinctrl bindings becomes deprecated because it
> do not correctly describe the hardware. The SIUL2 module also
> offers GPIO control and exposes some registers which contain
> information about the SoC. Adding drivers for these functionalities
> would result in incorrect bindings with a lot of carved out regions
> for registers.
> 
> SIUL2 is a complex module that spans multiple register regions
> and provides several functions: pinmux and pin configuration
> through MSCR and IMCR registers, GPIO control through PGPDO
> and PGPDI registers, interrupt configuration registers,
> and SoC identification registers (MIDR).
> These registers are grouped under two instances, SIUL2_0 and
> SIUL2_1, and share the same functional context. The legacy
> binding models SIUL2 as a standalone pinctrl node, which only
> covers MSCR and IMCR.
> 
> Signed-off-by: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
> Signed-off-by: Khristine Andreea Barbulescu <khristineandreea.barbulescu@oss.nxp.com>
> ---
>  .../bindings/mfd/nxp,s32g2-siul2.yaml         | 165 ++++++++++++++++++
>  .../pinctrl/nxp,s32g2-siul2-pinctrl.yaml      |   2 +
>  2 files changed, 167 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/nxp,s32g2-siul2.yaml

Doesn't look like this was tested...

> 
> diff --git a/Documentation/devicetree/bindings/mfd/nxp,s32g2-siul2.yaml b/Documentation/devicetree/bindings/mfd/nxp,s32g2-siul2.yaml
> new file mode 100644
> index 000000000000..ec743cf5f73e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/nxp,s32g2-siul2.yaml
> @@ -0,0 +1,165 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright 2024 NXP
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/nxp,s32g2-siul2.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NXP S32 System Integration Unit Lite2 (SIUL2)
> +
> +maintainers:
> +  - Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
> +
> +description: |
> +  SIUL2 is a hardware block which implements pinmuxing,
> +  pinconf, GPIOs (some with interrupt capability) and
> +  registers which contain information about the SoC.
> +  There are generally two SIUL2 modules whose functionality
> +  is grouped together. For example interrupt configuration
> +  registers are part of SIUL2_1 even though interrupts are
> +  also available for SIUL2_0 pins.
> +
> +  The following register types are exported by SIUL2:
> +    - MIDR (MCU ID Register) - information related to the SoC
> +    - interrupt configuration registers
> +    - MSCR (Multiplexed Signal Configuration Register) - pinmuxing and pinconf
> +    - IMCR (Input Multiplexed Signal Configuration Register)- pinmuxing
> +    - PGPDO (Parallel GPIO Pad Data Out Register) - GPIO output value
> +    - PGPDI (Parallel GPIO Pad Data In Register) - GPIO input value
> +
> +  Most registers are 32bit wide with the exception of PGPDO/PGPDI which are
> +  16bit wide.
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - const: nxp,s32g2-siul2
> +      - items:
> +          - enum:
> +              - nxp,s32g3-siul2
> +          - const: nxp,s32g2-siul2
> +
> +  gpio-controller: true
> +
> +  "#gpio-cells":
> +    const: 2
> +
> +  gpio-ranges:
> +    maxItems: 2
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  interrupt-controller: true
> +
> +  "#interrupt-cells":
> +    const: 2
> +
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 1
> +
> +  ranges:
> +    description: Address translation ranges for child nodes.
> +
> +
> +patternProperties:
> +  "^siul2_[0-1]$":

Don't use '_'.

> +    type: object
> +    description: SIUL2 hardware instances represented as syscon.
> +    properties:
> +      compatible:
> +        const: syscon

'syscon' alone is not allowed.

> +      reg:
> +        maxItems: 1

You have 'reg' so the node name should have unit-address.

However, there's not any real DT resources in this child node, so you 
should just drop it.

> +    required:
> +      - compatible
> +      - reg
> +
> +  "-hog(-[0-9]+)?$":
> +    required:
> +      - gpio-hog
> +
> +  "-pins$":
> +    type: object
> +    additionalProperties: false
> +
> +    patternProperties:
> +      "-grp[0-9]$":
> +        type: object
> +        allOf:
> +          - $ref: /schemas/pinctrl/pinmux-node.yaml#
> +          - $ref: /schemas/pinctrl/pincfg-node.yaml#
> +        description:
> +          Pinctrl node's client devices specify pin muxes using subnodes,
> +          which in turn use the standard properties below.
> +
> +        properties:
> +          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: Supported slew rate based on Fmax values (MHz)
> +            enum: [83, 133, 150, 166, 208]
> +        required:
> +          - pinmux
> +
> +        unevaluatedProperties: false
> +
> +required:
> +  - compatible
> +  - gpio-controller
> +  - "#gpio-cells"
> +  - gpio-ranges
> +  - interrupts
> +  - interrupt-controller
> +  - "#interrupt-cells"
> +  - "#address-cells"
> +  - "#size-cells"
> +  - ranges
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    pinctrl@4009c000 {
> +      compatible = "nxp,s32g2-siul2";
> +      gpio-controller;
> +      #gpio-cells = <2>;
> +      gpio-ranges = <&siul2 0 0 102>, <&siul2 112 112 79>;
> +      interrupt-controller;
> +      #interrupt-cells = <2>;
> +      interrupts = <GIC_SPI 210 IRQ_TYPE_LEVEL_HIGH>;
> +      #address-cells = <1>;
> +      #size-cells = <1>;
> +      ranges;
> +
> +      siul2_0: siul2_0@4009c000 {
> +        compatible = "syscon";
> +        reg = <0x0 0x4009c000 0x0 0x179c>;
> +      };
> +
> +      siul2_1: siul2_1@44010000 {
> +        compatible = "syscon";
> +        reg = <0x0 0x44010000 0x0 0x17b0>;
> +      };
> +
> +      jtag-pins {
> +        jtag-grp0 {
> +          pinmux = <0x0>;
> +          input-enable;
> +          bias-pull-up;
> +          slew-rate = <166>;
> +        };
> +      };
> +    };
> +...
> diff --git a/Documentation/devicetree/bindings/pinctrl/nxp,s32g2-siul2-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/nxp,s32g2-siul2-pinctrl.yaml
> index a24286e4def6..332397a21394 100644
> --- a/Documentation/devicetree/bindings/pinctrl/nxp,s32g2-siul2-pinctrl.yaml
> +++ b/Documentation/devicetree/bindings/pinctrl/nxp,s32g2-siul2-pinctrl.yaml
> @@ -11,6 +11,8 @@ maintainers:
>    - Ghennadi Procopciuc <Ghennadi.Procopciuc@oss.nxp.com>
>    - Chester Lin <chester62515@gmail.com>
>  
> +deprecated: true
> +

I don't really see why you can't just extend this binding with GPIO and 
interrupt provider properties.

Rob

Re: [PATCH v8 01/10] dt-bindings: mfd: add support for the NXP SIUL2 module

[Khristine Andreea Barbulescu]

Hello Rob,

On 1/21/2026 4:19 AM, Rob Herring wrote:
> On Tue, Jan 20, 2026 at 01:59:13PM +0200, Khristine Andreea Barbulescu wrote:
>> From: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
>>
>> Add the new dt-bindings for the NXP SIUL2 module which is a multi
>> function device. It can export information about the SoC, configure
>> the pinmux&pinconf for pins and it is also a GPIO controller with
>> interrupt capability.
>>
>> The existing SIUL2 pinctrl bindings becomes deprecated because it
>> do not correctly describe the hardware. The SIUL2 module also
>> offers GPIO control and exposes some registers which contain
>> information about the SoC. Adding drivers for these functionalities
>> would result in incorrect bindings with a lot of carved out regions
>> for registers.
>>
>> SIUL2 is a complex module that spans multiple register regions
>> and provides several functions: pinmux and pin configuration
>> through MSCR and IMCR registers, GPIO control through PGPDO
>> and PGPDI registers, interrupt configuration registers,
>> and SoC identification registers (MIDR).
>> These registers are grouped under two instances, SIUL2_0 and
>> SIUL2_1, and share the same functional context. The legacy
>> binding models SIUL2 as a standalone pinctrl node, which only
>> covers MSCR and IMCR.
>>
>> Signed-off-by: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
>> Signed-off-by: Khristine Andreea Barbulescu <khristineandreea.barbulescu@oss.nxp.com>
>> ---
>>  .../bindings/mfd/nxp,s32g2-siul2.yaml         | 165 ++++++++++++++++++
>>  .../pinctrl/nxp,s32g2-siul2-pinctrl.yaml      |   2 +
>>  2 files changed, 167 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/mfd/nxp,s32g2-siul2.yaml
> 
> Doesn't look like this was tested...
> 
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/nxp,s32g2-siul2.yaml b/Documentation/devicetree/bindings/mfd/nxp,s32g2-siul2.yaml
>> new file mode 100644
>> index 000000000000..ec743cf5f73e
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mfd/nxp,s32g2-siul2.yaml
>> @@ -0,0 +1,165 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +# Copyright 2024 NXP
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/mfd/nxp,s32g2-siul2.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: NXP S32 System Integration Unit Lite2 (SIUL2)
>> +
>> +maintainers:
>> +  - Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
>> +
>> +description: |
>> +  SIUL2 is a hardware block which implements pinmuxing,
>> +  pinconf, GPIOs (some with interrupt capability) and
>> +  registers which contain information about the SoC.
>> +  There are generally two SIUL2 modules whose functionality
>> +  is grouped together. For example interrupt configuration
>> +  registers are part of SIUL2_1 even though interrupts are
>> +  also available for SIUL2_0 pins.
>> +
>> +  The following register types are exported by SIUL2:
>> +    - MIDR (MCU ID Register) - information related to the SoC
>> +    - interrupt configuration registers
>> +    - MSCR (Multiplexed Signal Configuration Register) - pinmuxing and pinconf
>> +    - IMCR (Input Multiplexed Signal Configuration Register)- pinmuxing
>> +    - PGPDO (Parallel GPIO Pad Data Out Register) - GPIO output value
>> +    - PGPDI (Parallel GPIO Pad Data In Register) - GPIO input value
>> +
>> +  Most registers are 32bit wide with the exception of PGPDO/PGPDI which are
>> +  16bit wide.
>> +
>> +properties:
>> +  compatible:
>> +    oneOf:
>> +      - const: nxp,s32g2-siul2
>> +      - items:
>> +          - enum:
>> +              - nxp,s32g3-siul2
>> +          - const: nxp,s32g2-siul2
>> +
>> +  gpio-controller: true
>> +
>> +  "#gpio-cells":
>> +    const: 2
>> +
>> +  gpio-ranges:
>> +    maxItems: 2
>> +
>> +  interrupts:
>> +    maxItems: 1
>> +
>> +  interrupt-controller: true
>> +
>> +  "#interrupt-cells":
>> +    const: 2
>> +
>> +  "#address-cells":
>> +    const: 1
>> +
>> +  "#size-cells":
>> +    const: 1
>> +
>> +  ranges:
>> +    description: Address translation ranges for child nodes.
>> +
>> +
>> +patternProperties:
>> +  "^siul2_[0-1]$":
> 
> Don't use '_'.
Will be fixed.
> 
>> +    type: object
>> +    description: SIUL2 hardware instances represented as syscon.
>> +    properties:
>> +      compatible:
>> +        const: syscon
> 
> 'syscon' alone is not allowed.

Will add a SoC specific compatible too.

> 
>> +      reg:
>> +        maxItems: 1
> 
> You have 'reg' so the node name should have unit-address.
> 
> However, there's not any real DT resources in this child node, so you 
> should just drop it.
> 

For context, SIUL2 exposes a set of platform‑capability and SoC identification registers that are split across the two discontiguous ranges: SIUL2-0 and SIUL2-1. These registers are the source of SoC information (e.g. identification and capability flags) that other subsystems are expected to consume (e.g. PCI Express). Because those fields are physically divided between the two SIUL2 ranges, consumers need reliable access to both ranges to correctly discover and configure the platform.

Hence, my proposal is to keep the two 'syscon' child nodes.

>> +    required:
>> +      - compatible
>> +      - reg
>> +
>> +  "-hog(-[0-9]+)?$":
>> +    required:
>> +      - gpio-hog
>> +
>> +  "-pins$":
>> +    type: object
>> +    additionalProperties: false
>> +
>> +    patternProperties:
>> +      "-grp[0-9]$":
>> +        type: object
>> +        allOf:
>> +          - $ref: /schemas/pinctrl/pinmux-node.yaml#
>> +          - $ref: /schemas/pinctrl/pincfg-node.yaml#
>> +        description:
>> +          Pinctrl node's client devices specify pin muxes using subnodes,
>> +          which in turn use the standard properties below.
>> +
>> +        properties:
>> +          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: Supported slew rate based on Fmax values (MHz)
>> +            enum: [83, 133, 150, 166, 208]
>> +        required:
>> +          - pinmux
>> +
>> +        unevaluatedProperties: false
>> +
>> +required:
>> +  - compatible
>> +  - gpio-controller
>> +  - "#gpio-cells"
>> +  - gpio-ranges
>> +  - interrupts
>> +  - interrupt-controller
>> +  - "#interrupt-cells"
>> +  - "#address-cells"
>> +  - "#size-cells"
>> +  - ranges
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>> +    #include <dt-bindings/interrupt-controller/irq.h>
>> +
>> +    pinctrl@4009c000 {
>> +      compatible = "nxp,s32g2-siul2";
>> +      gpio-controller;
>> +      #gpio-cells = <2>;
>> +      gpio-ranges = <&siul2 0 0 102>, <&siul2 112 112 79>;
>> +      interrupt-controller;
>> +      #interrupt-cells = <2>;
>> +      interrupts = <GIC_SPI 210 IRQ_TYPE_LEVEL_HIGH>;
>> +      #address-cells = <1>;
>> +      #size-cells = <1>;
>> +      ranges;
>> +
>> +      siul2_0: siul2_0@4009c000 {
>> +        compatible = "syscon";
>> +        reg = <0x0 0x4009c000 0x0 0x179c>;
>> +      };
>> +
>> +      siul2_1: siul2_1@44010000 {
>> +        compatible = "syscon";
>> +        reg = <0x0 0x44010000 0x0 0x17b0>;
>> +      };
>> +
>> +      jtag-pins {
>> +        jtag-grp0 {
>> +          pinmux = <0x0>;
>> +          input-enable;
>> +          bias-pull-up;
>> +          slew-rate = <166>;
>> +        };
>> +      };
>> +    };
>> +...
>> diff --git a/Documentation/devicetree/bindings/pinctrl/nxp,s32g2-siul2-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/nxp,s32g2-siul2-pinctrl.yaml
>> index a24286e4def6..332397a21394 100644
>> --- a/Documentation/devicetree/bindings/pinctrl/nxp,s32g2-siul2-pinctrl.yaml
>> +++ b/Documentation/devicetree/bindings/pinctrl/nxp,s32g2-siul2-pinctrl.yaml
>> @@ -11,6 +11,8 @@ maintainers:
>>    - Ghennadi Procopciuc <Ghennadi.Procopciuc@oss.nxp.com>
>>    - Chester Lin <chester62515@gmail.com>
>>  
>> +deprecated: true
>> +
> 
> I don't really see why you can't just extend this binding with GPIO and 
> interrupt provider properties.

The existing SIUL2 pinctrl binding only describes the MSCR/IMCR registers and treats SIUL2 as a standalone pinctrl block. This is incomplete and does not correctly represent the SIUL2 hardware, which also provides GPIO control, interrupt configuration, and MIDR identification registers across two register windows. Extending the old binding would require incompatible ABI changes and would result in carved-out subregions, which is discouraged.

> 
> Rob

Re: [PATCH v8 01/10] dt-bindings: mfd: add support for the NXP SIUL2 module

[Krzysztof Kozlowski]

On 19/02/2026 12:36, Khristine Andreea Barbulescu wrote:
>>
>>> +      reg:
>>> +        maxItems: 1
>>
>> You have 'reg' so the node name should have unit-address.
>>
>> However, there's not any real DT resources in this child node, so you 
>> should just drop it.
>>
> 
> For context, SIUL2 exposes a set of platform‑capability and SoC identification registers that are split across the two discontiguous ranges: SIUL2-0 and SIUL2-1. These registers are the source of SoC information (e.g. identification and capability flags) that other subsystems are expected to consume (e.g. PCI Express). Because those fields are physically divided between the two SIUL2 ranges, consumers need reliable access to both ranges to correctly discover and configure the platform.
> 
> Hence, my proposal is to keep the two 'syscon' child nodes.

Please wrap your replies correctly, so this will be easily parseable.

I do not understand the reasoning. If you have two register ranges, you
have two <reg> entries and having a child node has nothing to do with it.

> 
>>> +    required:
>>> +      - compatible
>>> +      - reg
>>> +
>>> +  "-hog(-[0-9]+)?$":
>>> +    required:
>>> +      - gpio-hog
>>> +
>>> +  "-pins$":
>>> +    type: object
>>> +    additionalProperties: false
>>> +
>>> +    patternProperties:
>>> +      "-grp[0-9]$":
>>> +        type: object
>>> +        allOf:
>>> +          - $ref: /schemas/pinctrl/pinmux-node.yaml#
>>> +          - $ref: /schemas/pinctrl/pincfg-node.yaml#
>>> +        description:
>>> +          Pinctrl node's client devices specify pin muxes using subnodes,
>>> +          which in turn use the standard properties below.
>>> +
>>> +        properties:
>>> +          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: Supported slew rate based on Fmax values (MHz)
>>> +            enum: [83, 133, 150, 166, 208]
>>> +        required:
>>> +          - pinmux
>>> +
>>> +        unevaluatedProperties: false
>>> +
>>> +required:
>>> +  - compatible
>>> +  - gpio-controller
>>> +  - "#gpio-cells"
>>> +  - gpio-ranges
>>> +  - interrupts
>>> +  - interrupt-controller
>>> +  - "#interrupt-cells"
>>> +  - "#address-cells"
>>> +  - "#size-cells"
>>> +  - ranges
>>> +
>>> +additionalProperties: false
>>> +
>>> +examples:
>>> +  - |
>>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>>> +    #include <dt-bindings/interrupt-controller/irq.h>
>>> +
>>> +    pinctrl@4009c000 {
>>> +      compatible = "nxp,s32g2-siul2";
>>> +      gpio-controller;
>>> +      #gpio-cells = <2>;
>>> +      gpio-ranges = <&siul2 0 0 102>, <&siul2 112 112 79>;
>>> +      interrupt-controller;
>>> +      #interrupt-cells = <2>;
>>> +      interrupts = <GIC_SPI 210 IRQ_TYPE_LEVEL_HIGH>;
>>> +      #address-cells = <1>;
>>> +      #size-cells = <1>;
>>> +      ranges;
>>> +
>>> +      siul2_0: siul2_0@4009c000 {
>>> +        compatible = "syscon";
>>> +        reg = <0x0 0x4009c000 0x0 0x179c>;
>>> +      };
>>> +
>>> +      siul2_1: siul2_1@44010000 {
>>> +        compatible = "syscon";
>>> +        reg = <0x0 0x44010000 0x0 0x17b0>;
>>> +      };
>>> +
>>> +      jtag-pins {
>>> +        jtag-grp0 {
>>> +          pinmux = <0x0>;
>>> +          input-enable;
>>> +          bias-pull-up;
>>> +          slew-rate = <166>;
>>> +        };
>>> +      };
>>> +    };
>>> +...
>>> diff --git a/Documentation/devicetree/bindings/pinctrl/nxp,s32g2-siul2-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/nxp,s32g2-siul2-pinctrl.yaml
>>> index a24286e4def6..332397a21394 100644
>>> --- a/Documentation/devicetree/bindings/pinctrl/nxp,s32g2-siul2-pinctrl.yaml
>>> +++ b/Documentation/devicetree/bindings/pinctrl/nxp,s32g2-siul2-pinctrl.yaml
>>> @@ -11,6 +11,8 @@ maintainers:
>>>    - Ghennadi Procopciuc <Ghennadi.Procopciuc@oss.nxp.com>
>>>    - Chester Lin <chester62515@gmail.com>
>>>  
>>> +deprecated: true
>>> +
>>
>> I don't really see why you can't just extend this binding with GPIO and 
>> interrupt provider properties.
> 
> The existing SIUL2 pinctrl binding only describes the MSCR/IMCR registers and treats SIUL2 as a standalone pinctrl block. This is incomplete and does not correctly represent the SIUL2 hardware, which also provides GPIO control, interrupt configuration, and MIDR identification registers across two register windows. Extending the old binding would require incompatible ABI changes and would result in carved-out subregions, which is discouraged.

Can you just add missing register ranges to existing device? I really do
not see how this is incompatible ABI or how does it result in carved-out
regions.


Best regards,
Krzysztof

Re: [PATCH v8 01/10] dt-bindings: mfd: add support for the NXP SIUL2 module

[Khristine Andreea Barbulescu]

Hello Krzysztof,

On 2/20/2026 12:16 PM, Krzysztof Kozlowski wrote:
> [You don't often get email from krzk@kernel.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> On 19/02/2026 12:36, Khristine Andreea Barbulescu wrote:
>>>
>>>> +      reg:
>>>> +        maxItems: 1
>>>
>>> You have 'reg' so the node name should have unit-address.
>>>
>>> However, there's not any real DT resources in this child node, so you
>>> should just drop it.
>>>
>>
>> For context, SIUL2 exposes a set of platform‑capability and SoC identification registers that are split across the two discontiguous ranges: SIUL2-0 and SIUL2-1. These registers are the source of SoC information (e.g. identification and capability flags) that other subsystems are expected to consume (e.g. PCI Express). Because those fields are physically divided between the two SIUL2 ranges, consumers need reliable access to both ranges to correctly discover and configure the platform.
>>
>> Hence, my proposal is to keep the two 'syscon' child nodes.
> 
> Please wrap your replies correctly, so this will be easily parseable.
> 
> I do not understand the reasoning. If you have two register ranges, you
> have two <reg> entries and having a child node has nothing to do with it.
> 

I’ve reorganized the SIUL2 node with two syscon subnodes for the two
register regions used to read system info, and a separate
pinctrl/GPIO child (as discussed in the v8 06/10 thread [0]). The parent
SIUL2 node now carries the bus addressing and ranges:
 
    siul2: siul2@4009c000 {
        compatible = "nxp,s32g3-siul2", "nxp,s32g2-siul2";
        #address-cells = <1>;
        #size-cells = <1>;
        ranges = <0x4009c000 0x4009c000 0x179c>,
		 <0x44010000 0x44010000 0x17b0>;
 
        siul20: siul20@4009c000 {
            compatible = "nxp,s32g-siul2-syscfg", "syscon";
            reg = <0x4009c000 0x179c>;
        };
 
        siul21: siul21@44010000 {
            compatible = "nxp,s32g-siul2-syscfg", "syscon";
            reg = <0x44010000 0x17b0>;
        };
 
        pinctrl: pinctrl {
            compatible = "nxp,s32g-siul2-pinctrl";
            #gpio-cells = <2>;
            gpio-controller;
            gpio-ranges = <&pinctrl 0 0 102>, <&pinctrl 112 112 79>;
            interrupt-controller;
            #interrupt-cells = <2>;
            interrupts = <GIC_SPI 210 IRQ_TYPE_LEVEL_HIGH>;
 
            jtag_pins: jtag-pins {
                jtag-grp0 {
                    pinmux = <0x0>;
                    input-enable;
                    bias-pull-up;
                    slew-rate = <166>;
                };
            };
        };
    };
 
Could you please confirm this matches the intent of your feedback, or let me
know if I misunderstood any part? If this looks correct, I’ll move ahead
with the new patch set.

>>
>>>> +    required:
>>>> +      - compatible
>>>> +      - reg
>>>> +
>>>> +  "-hog(-[0-9]+)?$":
>>>> +    required:
>>>> +      - gpio-hog
>>>> +
>>>> +  "-pins$":
>>>> +    type: object
>>>> +    additionalProperties: false
>>>> +
>>>> +    patternProperties:
>>>> +      "-grp[0-9]$":
>>>> +        type: object
>>>> +        allOf:
>>>> +          - $ref: /schemas/pinctrl/pinmux-node.yaml#
>>>> +          - $ref: /schemas/pinctrl/pincfg-node.yaml#
>>>> +        description:
>>>> +          Pinctrl node's client devices specify pin muxes using subnodes,
>>>> +          which in turn use the standard properties below.
>>>> +
>>>> +        properties:
>>>> +          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: Supported slew rate based on Fmax values (MHz)
>>>> +            enum: [83, 133, 150, 166, 208]
>>>> +        required:
>>>> +          - pinmux
>>>> +
>>>> +        unevaluatedProperties: false
>>>> +
>>>> +required:
>>>> +  - compatible
>>>> +  - gpio-controller
>>>> +  - "#gpio-cells"
>>>> +  - gpio-ranges
>>>> +  - interrupts
>>>> +  - interrupt-controller
>>>> +  - "#interrupt-cells"
>>>> +  - "#address-cells"
>>>> +  - "#size-cells"
>>>> +  - ranges
>>>> +
>>>> +additionalProperties: false
>>>> +
>>>> +examples:
>>>> +  - |
>>>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>>>> +    #include <dt-bindings/interrupt-controller/irq.h>
>>>> +
>>>> +    pinctrl@4009c000 {
>>>> +      compatible = "nxp,s32g2-siul2";
>>>> +      gpio-controller;
>>>> +      #gpio-cells = <2>;
>>>> +      gpio-ranges = <&siul2 0 0 102>, <&siul2 112 112 79>;
>>>> +      interrupt-controller;
>>>> +      #interrupt-cells = <2>;
>>>> +      interrupts = <GIC_SPI 210 IRQ_TYPE_LEVEL_HIGH>;
>>>> +      #address-cells = <1>;
>>>> +      #size-cells = <1>;
>>>> +      ranges;
>>>> +
>>>> +      siul2_0: siul2_0@4009c000 {
>>>> +        compatible = "syscon";
>>>> +        reg = <0x0 0x4009c000 0x0 0x179c>;
>>>> +      };
>>>> +
>>>> +      siul2_1: siul2_1@44010000 {
>>>> +        compatible = "syscon";
>>>> +        reg = <0x0 0x44010000 0x0 0x17b0>;
>>>> +      };
>>>> +
>>>> +      jtag-pins {
>>>> +        jtag-grp0 {
>>>> +          pinmux = <0x0>;
>>>> +          input-enable;
>>>> +          bias-pull-up;
>>>> +          slew-rate = <166>;
>>>> +        };
>>>> +      };
>>>> +    };
>>>> +...
>>>> diff --git a/Documentation/devicetree/bindings/pinctrl/nxp,s32g2-siul2-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/nxp,s32g2-siul2-pinctrl.yaml
>>>> index a24286e4def6..332397a21394 100644
>>>> --- a/Documentation/devicetree/bindings/pinctrl/nxp,s32g2-siul2-pinctrl.yaml
>>>> +++ b/Documentation/devicetree/bindings/pinctrl/nxp,s32g2-siul2-pinctrl.yaml
>>>> @@ -11,6 +11,8 @@ maintainers:
>>>>    - Ghennadi Procopciuc <Ghennadi.Procopciuc@oss.nxp.com>
>>>>    - Chester Lin <chester62515@gmail.com>
>>>>
>>>> +deprecated: true
>>>> +
>>>
>>> I don't really see why you can't just extend this binding with GPIO and
>>> interrupt provider properties.
>>
>> The existing SIUL2 pinctrl binding only describes the MSCR/IMCR registers and treats SIUL2 as a standalone pinctrl block. This is incomplete and does not correctly represent the SIUL2 hardware, which also provides GPIO control, interrupt configuration, and MIDR identification registers across two register windows. Extending the old binding would require incompatible ABI changes and would result in carved-out subregions, which is discouraged.
> 
> Can you just add missing register ranges to existing device? I really do
> not see how this is incompatible ABI or how does it result in carved-out
> regions.
> 

I’m also fine with your proposal to add the missing register ranges to
the existing device. But that would also mean extending it with the
GPIO registers and two syscon subnodes for system info, as they all need
to be provided by this SIUL2 device. This would effectively make the
existing pinctrl binding an MFD.

Is evolving the existing binding acceptable in this manner, or should
we continue with this new MFD binding and leave the old pinctrl
binding deprecated?

> 
> Best regards,
> Krzysztof

Best regards,
Khristine

[0] https://lore.kernel.org/linux-gpio/CAKfTPtD6LOMFGhzG3dhiSQCNbYrGLjBiT83eqz9mmwaDVpNV=w@mail.gmail.com/

Re: [PATCH v8 01/10] dt-bindings: mfd: add support for the NXP SIUL2 module

[Krzysztof Kozlowski]

On 20/02/2026 15:36, Khristine Andreea Barbulescu wrote:
> Hello Krzysztof,
> 
> On 2/20/2026 12:16 PM, Krzysztof Kozlowski wrote:
>> [You don't often get email from krzk@kernel.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>>
>> On 19/02/2026 12:36, Khristine Andreea Barbulescu wrote:
>>>>
>>>>> +      reg:
>>>>> +        maxItems: 1
>>>>
>>>> You have 'reg' so the node name should have unit-address.
>>>>
>>>> However, there's not any real DT resources in this child node, so you
>>>> should just drop it.
>>>>
>>>
>>> For context, SIUL2 exposes a set of platform‑capability and SoC identification registers that are split across the two discontiguous ranges: SIUL2-0 and SIUL2-1. These registers are the source of SoC information (e.g. identification and capability flags) that other subsystems are expected to consume (e.g. PCI Express). Because those fields are physically divided between the two SIUL2 ranges, consumers need reliable access to both ranges to correctly discover and configure the platform.
>>>
>>> Hence, my proposal is to keep the two 'syscon' child nodes.
>>
>> Please wrap your replies correctly, so this will be easily parseable.
>>
>> I do not understand the reasoning. If you have two register ranges, you
>> have two <reg> entries and having a child node has nothing to do with it.
>>
> 
> I’ve reorganized the SIUL2 node with two syscon subnodes for the two
> register regions used to read system info, and a separate
> pinctrl/GPIO child (as discussed in the v8 06/10 thread [0]). The parent
> SIUL2 node now carries the bus addressing and ranges:

That's not the answer to the comment. Read again:

1. Reviewer: No resources, so no separate node.
2. Your answer: some soc capability and two address spaces
3. Reviewer: Does not matter, address spaces can go again to original place
4. Irrelevant reply.

So again, read the first comment - do you have dedicated resources in
children?

>  
>     siul2: siul2@4009c000 {
>         compatible = "nxp,s32g3-siul2", "nxp,s32g2-siul2";
>         #address-cells = <1>;
>         #size-cells = <1>;
>         ranges = <0x4009c000 0x4009c000 0x179c>,
> 		 <0x44010000 0x44010000 0x17b0>;
>  
>         siul20: siul20@4009c000 {
>             compatible = "nxp,s32g-siul2-syscfg", "syscon";
>             reg = <0x4009c000 0x179c>;

0x179c is odd size. Looks fake.


>         };
>  
>         siul21: siul21@44010000 {
>             compatible = "nxp,s32g-siul2-syscfg", "syscon";

And two same devices with same compatible proof it.

Best regards,
Krzysztof

Re: [PATCH v8 01/10] dt-bindings: mfd: add support for the NXP SIUL2 module

[Khristine Andreea Barbulescu]

On 2/20/2026 4:41 PM, Krzysztof Kozlowski wrote:
> On 20/02/2026 15:36, Khristine Andreea Barbulescu wrote:
>> Hello Krzysztof,
>>
>> On 2/20/2026 12:16 PM, Krzysztof Kozlowski wrote:
>>> [You don't often get email from krzk@kernel.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>>>
>>> On 19/02/2026 12:36, Khristine Andreea Barbulescu wrote:
>>>>>
>>>>>> +      reg:
>>>>>> +        maxItems: 1
>>>>>
>>>>> You have 'reg' so the node name should have unit-address.
>>>>>
>>>>> However, there's not any real DT resources in this child node, so you
>>>>> should just drop it.
>>>>>
>>>>
>>>> For context, SIUL2 exposes a set of platform‑capability and SoC identification registers that are split across the two discontiguous ranges: SIUL2-0 and SIUL2-1. These registers are the source of SoC information (e.g. identification and capability flags) that other subsystems are expected to consume (e.g. PCI Express). Because those fields are physically divided between the two SIUL2 ranges, consumers need reliable access to both ranges to correctly discover and configure the platform.
>>>>
>>>> Hence, my proposal is to keep the two 'syscon' child nodes.
>>>
>>> Please wrap your replies correctly, so this will be easily parseable.
>>>
>>> I do not understand the reasoning. If you have two register ranges, you
>>> have two <reg> entries and having a child node has nothing to do with it.
>>>
>>
>> I’ve reorganized the SIUL2 node with two syscon subnodes for the two
>> register regions used to read system info, and a separate
>> pinctrl/GPIO child (as discussed in the v8 06/10 thread [0]). The parent
>> SIUL2 node now carries the bus addressing and ranges:
> 
> That's not the answer to the comment. Read again:
> 
> 1. Reviewer: No resources, so no separate node.
> 2. Your answer: some soc capability and two address spaces
> 3. Reviewer: Does not matter, address spaces can go again to original place
> 4. Irrelevant reply.
> 
> So again, read the first comment - do you have dedicated resources in
> children?
> 
>>  
>>     siul2: siul2@4009c000 {
>>         compatible = "nxp,s32g3-siul2", "nxp,s32g2-siul2";
>>         #address-cells = <1>;
>>         #size-cells = <1>;
>>         ranges = <0x4009c000 0x4009c000 0x179c>,
>> 		 <0x44010000 0x44010000 0x17b0>;
>>  
>>         siul20: siul20@4009c000 {
>>             compatible = "nxp,s32g-siul2-syscfg", "syscon";
>>             reg = <0x4009c000 0x179c>;
> 
> 0x179c is odd size. Looks fake.
> 
> 
>>         };
>>  
>>         siul21: siul21@44010000 {
>>             compatible = "nxp,s32g-siul2-syscfg", "syscon";
> 
> And two same devices with same compatible proof it.
> 
> Best regards,
> Krzysztof

We don’t have dedicated resources for children. In particular,
there are no resources allocated specifically for nodes like
"nxp,s32g-siul2-syscfg". Their consumers are the pinctrl/gpio
driver and other drivers that read SoC‑specific information from
those shared registers.
 
My alternative is to keep two separate syscon providers for the
SIUL2-0 and SIUL2-1 and have consumers (pinctrl/gpio , PCIe, etc.)
reference them via phandles:

    siul20: syscon@4009c000 {
        compatible = "nxp,s32g-siul20-syscfg", "syscon";
        reg = <0x4009c000 0x2000>;
    };
 
    siul21: syscon@44010000 {
        compatible = "nxp,s32g-siul21-syscfg", "syscon";
        reg = <0x44010000 0x2000>;
    };
 
    pinctrl: pinctrl {
        compatible = "nxp,s32g-siul2-pinctrl";
        gpio-controller;
        #gpio-cells = <2>;
        gpio-ranges = <&pinctrl 0 0 102>, <&pinctrl 112 112 79>;
        interrupt-controller;
        #interrupt-cells = <2>;
        interrupts = <GIC_SPI 210 IRQ_TYPE_LEVEL_HIGH>;
        syscon = <&siul20>, <&siul21>;
 
        jtag_pins: jtag-pins {
            jtag-grp0 {
                pinmux = <0x0>;
                input-enable;
                bias-pull-up;
                slew-rate = <166>;
            };
        };
    };
 
    pcie@40400000 {
        reg = <...>;
        /* PCIe Dev ID */
        syscon = <&siul20>;
    };
 
I’ve seen similar approaches upstream [0].
Would this be acceptable (even if not ideal),
or do you prefer a different layout?

Best regards,
Khristine
 
[0] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/pinctrl/xlnx,pinctrl-zynq.yaml#n182

Re: [PATCH v8 01/10] dt-bindings: mfd: add support for the NXP SIUL2 module

[Krzysztof Kozlowski]

On 23/02/2026 12:51, Khristine Andreea Barbulescu wrote:
> On 2/20/2026 4:41 PM, Krzysztof Kozlowski wrote:
>> On 20/02/2026 15:36, Khristine Andreea Barbulescu wrote:
>>> Hello Krzysztof,
>>>
>>> On 2/20/2026 12:16 PM, Krzysztof Kozlowski wrote:
>>>> [You don't often get email from krzk@kernel.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>>>>
>>>> On 19/02/2026 12:36, Khristine Andreea Barbulescu wrote:
>>>>>>
>>>>>>> +      reg:
>>>>>>> +        maxItems: 1
>>>>>>
>>>>>> You have 'reg' so the node name should have unit-address.
>>>>>>
>>>>>> However, there's not any real DT resources in this child node, so you
>>>>>> should just drop it.
>>>>>>
>>>>>
>>>>> For context, SIUL2 exposes a set of platform‑capability and SoC identification registers that are split across the two discontiguous ranges: SIUL2-0 and SIUL2-1. These registers are the source of SoC information (e.g. identification and capability flags) that other subsystems are expected to consume (e.g. PCI Express). Because those fields are physically divided between the two SIUL2 ranges, consumers need reliable access to both ranges to correctly discover and configure the platform.
>>>>>
>>>>> Hence, my proposal is to keep the two 'syscon' child nodes.
>>>>
>>>> Please wrap your replies correctly, so this will be easily parseable.
>>>>
>>>> I do not understand the reasoning. If you have two register ranges, you
>>>> have two <reg> entries and having a child node has nothing to do with it.
>>>>
>>>
>>> I’ve reorganized the SIUL2 node with two syscon subnodes for the two
>>> register regions used to read system info, and a separate
>>> pinctrl/GPIO child (as discussed in the v8 06/10 thread [0]). The parent
>>> SIUL2 node now carries the bus addressing and ranges:
>>
>> That's not the answer to the comment. Read again:
>>
>> 1. Reviewer: No resources, so no separate node.
>> 2. Your answer: some soc capability and two address spaces
>> 3. Reviewer: Does not matter, address spaces can go again to original place
>> 4. Irrelevant reply.
>>
>> So again, read the first comment - do you have dedicated resources in
>> children?
>>
>>>  
>>>     siul2: siul2@4009c000 {
>>>         compatible = "nxp,s32g3-siul2", "nxp,s32g2-siul2";
>>>         #address-cells = <1>;
>>>         #size-cells = <1>;
>>>         ranges = <0x4009c000 0x4009c000 0x179c>,
>>> 		 <0x44010000 0x44010000 0x17b0>;
>>>  
>>>         siul20: siul20@4009c000 {
>>>             compatible = "nxp,s32g-siul2-syscfg", "syscon";
>>>             reg = <0x4009c000 0x179c>;
>>
>> 0x179c is odd size. Looks fake.
>>
>>
>>>         };
>>>  
>>>         siul21: siul21@44010000 {
>>>             compatible = "nxp,s32g-siul2-syscfg", "syscon";
>>
>> And two same devices with same compatible proof it.
>>
>> Best regards,
>> Krzysztof
> 
> We don’t have dedicated resources for children. In particular,

Then previous comments/review stay.

> there are no resources allocated specifically for nodes like
> "nxp,s32g-siul2-syscfg". Their consumers are the pinctrl/gpio
> driver and other drivers that read SoC‑specific information from
> those shared registers.
>  
> My alternative is to keep two separate syscon providers for the

You got review already.

Best regards,
Krzysztof

Re: [PATCH v8 01/10] dt-bindings: mfd: add support for the NXP SIUL2 module

[Ghennadi Procopciuc]

On 2/23/2026 3:14 PM, Krzysztof Kozlowski wrote:
> On 23/02/2026 12:51, Khristine Andreea Barbulescu wrote:
>> On 2/20/2026 4:41 PM, Krzysztof Kozlowski wrote:
>>> On 20/02/2026 15:36, Khristine Andreea Barbulescu wrote:
>>>> Hello Krzysztof,
>>>>
>>>> On 2/20/2026 12:16 PM, Krzysztof Kozlowski wrote:
>>>>> [You don't often get email from krzk@kernel.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>>>>>
>>>>> On 19/02/2026 12:36, Khristine Andreea Barbulescu wrote:
>>>>>>>
>>>>>>>> +      reg:
>>>>>>>> +        maxItems: 1
>>>>>>>
>>>>>>> You have 'reg' so the node name should have unit-address.
>>>>>>>
>>>>>>> However, there's not any real DT resources in this child node, so you
>>>>>>> should just drop it.
>>>>>>>
>>>>>>
>>>>>> For context, SIUL2 exposes a set of platform‑capability and SoC identification registers that are split across the two discontiguous ranges: SIUL2-0 and SIUL2-1. These registers are the source of SoC information (e.g. identification and capability flags) that other subsystems are expected to consume (e.g. PCI Express). Because those fields are physically divided between the two SIUL2 ranges, consumers need reliable access to both ranges to correctly discover and configure the platform.
>>>>>>
>>>>>> Hence, my proposal is to keep the two 'syscon' child nodes.
>>>>>
>>>>> Please wrap your replies correctly, so this will be easily parseable.
>>>>>
>>>>> I do not understand the reasoning. If you have two register ranges, you
>>>>> have two <reg> entries and having a child node has nothing to do with it.
>>>>>
>>>>
>>>> I’ve reorganized the SIUL2 node with two syscon subnodes for the two
>>>> register regions used to read system info, and a separate
>>>> pinctrl/GPIO child (as discussed in the v8 06/10 thread [0]). The parent
>>>> SIUL2 node now carries the bus addressing and ranges:
>>>
>>> That's not the answer to the comment. Read again:
>>>
>>> 1. Reviewer: No resources, so no separate node.
>>> 2. Your answer: some soc capability and two address spaces
>>> 3. Reviewer: Does not matter, address spaces can go again to original place
>>> 4. Irrelevant reply.
>>>
>>> So again, read the first comment - do you have dedicated resources in
>>> children?
>>>
>>>>  
>>>>     siul2: siul2@4009c000 {
>>>>         compatible = "nxp,s32g3-siul2", "nxp,s32g2-siul2";
>>>>         #address-cells = <1>;
>>>>         #size-cells = <1>;
>>>>         ranges = <0x4009c000 0x4009c000 0x179c>,
>>>> 		 <0x44010000 0x44010000 0x17b0>;
>>>>  
>>>>         siul20: siul20@4009c000 {
>>>>             compatible = "nxp,s32g-siul2-syscfg", "syscon";
>>>>             reg = <0x4009c000 0x179c>;
>>>
>>> 0x179c is odd size. Looks fake.
>>>
>>>
>>>>         };
>>>>  
>>>>         siul21: siul21@44010000 {
>>>>             compatible = "nxp,s32g-siul2-syscfg", "syscon";
>>>
>>> And two same devices with same compatible proof it.
>>>
>>> Best regards,
>>> Krzysztof
>>
>> We don’t have dedicated resources for children. In particular,
> 
> Then previous comments/review stay.
> 
>> there are no resources allocated specifically for nodes like
>> "nxp,s32g-siul2-syscfg". Their consumers are the pinctrl/gpio
>> driver and other drivers that read SoC‑specific information from
>> those shared registers.
>>  
>> My alternative is to keep two separate syscon providers for the
> 
> You got review already.
> 
> Best regards,
> Krzysztof

Hi Krzysztof & Arnd,

I still believe that nvmem is a suitable and accurate mechanism for
describing SoC‑specific identification information, as originally
proposed in [0], assuming the necessary adjustments are made.

More specifically, instead of modeling software-defined cells, the nvmem
layout would describe the actual hardware registers backing this
information. One advantage of this approach is that consumer nodes (for
example PCIe, Ethernet, or other IPs that need SoC identification data)
can reference these registers using the standard nvmem-cells /
nvmem-cell-names mechanism, without introducing custom, per-subsystem
bindings.

Looking at the nvmem binding documentation [1], it appears that
individual fields of identification registers (similar to MIDR) could,
in principle, be exposed using a bits property, for example:

family_letter@4009c004 {
  reg = <0x4009c004 0x4>;
  bits = <31 26>;
};

That said, an alternative (and arguably more common) approach is to
expose entire registers as fixed-layout cells, and let consumers decode
bitfields in software. Below is an example of how this would look when
embedded in the existing SIUL2 node, without relying on per-field bits
properties:

siul2: pinctrl@4009c000 {
  compatible = "nxp,s32g2-siul2";
  reg = <0x4009c000 0x2000>,
	<0x44010000 0x2000>;
  #address-cells = <1>;
  #size-cells = <1>;
  ranges = <0x4009c000 0x4009c000 0xc>,
	   <0x44010000 0x44010000 0xc>;
  reg-names = "siul20", "siul21";
  gpio-controller;
  #gpio-cells = <2>;
  gpio-ranges = <&siul2 0 0 102>, <&siul2 112 112 79>;
  interrupt-controller;
  #interrupt-cells = <2>;
  interrupts = <GIC_SPI 210 IRQ_TYPE_LEVEL_HIGH>;

  jtag-pins {
	jtag-grp0 {
	  pinmux = <0x0>;
	  input-enable;
	  bias-pull-up;
	  slew-rate = <166>;
	};
  };

  nvmem-layout {
	compatible = "fixed-layout";
	#address-cells = <1>;
	#size-cells = <1>;

	siul20_midr0@4009c004 {
	  reg = <0x4009c004 0x4>;
	};

	siul20_midr1@4009c008 {
	  reg = <0x4009c008 0x4>;
	};

	siul21_midr0@44010004 {
	  reg = <0x44010004 0x4>;
	};

	siul21_midr1@44010008 {
	  reg = <0x44010008 0x4>;
	};
  };
};


[0]
https://lore.kernel.org/all/20250710142038.1986052-2-andrei.stefanescu@oss.nxp.com/
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/nvmem/nvmem.yaml#n82

-- 
Regards,
Ghennadi

Re: [PATCH v8 01/10] dt-bindings: mfd: add support for the NXP SIUL2 module

[Ghennadi Procopciuc]

Hi Krzysztof & Arnd,

On 2/25/2026 11:40 AM, Ghennadi Procopciuc wrote:

[ ...]

> Hi Krzysztof & Arnd,
> 
> I still believe that nvmem is a suitable and accurate mechanism for
> describing SoC‑specific identification information, as originally
> proposed in [0], assuming the necessary adjustments are made.
> 
> More specifically, instead of modeling software-defined cells, the nvmem
> layout would describe the actual hardware registers backing this
> information. One advantage of this approach is that consumer nodes (for
> example PCIe, Ethernet, or other IPs that need SoC identification data)
> can reference these registers using the standard nvmem-cells /
> nvmem-cell-names mechanism, without introducing custom, per-subsystem
> bindings.
> 
> Looking at the nvmem binding documentation [1], it appears that
> individual fields of identification registers (similar to MIDR) could,
> in principle, be exposed using a bits property, for example:
> 
> family_letter@4009c004 {
>   reg = <0x4009c004 0x4>;
>   bits = <31 26>;
> };
> 
> That said, an alternative (and arguably more common) approach is to
> expose entire registers as fixed-layout cells, and let consumers decode
> bitfields in software. Below is an example of how this would look when
> embedded in the existing SIUL2 node, without relying on per-field bits
> properties:
> 

[...]

> [0]
> https://lore.kernel.org/all/20250710142038.1986052-2-andrei.stefanescu@oss.nxp.com/
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/nvmem/nvmem.yaml#n82
> 

Are you comfortable with this proposal?

I'm asking because it was initially considered a viable option, but the
discussion later shifted to the fact that the nvmem cells were
fabricated and did not accurately describe the underlying hardware.

-- 
Regards,
Ghennadi

Re: [PATCH v8 01/10] dt-bindings: mfd: add support for the NXP SIUL2 module

[Krzysztof Kozlowski]

On 25/02/2026 10:40, Ghennadi Procopciuc wrote:
> On 2/23/2026 3:14 PM, Krzysztof Kozlowski wrote:
>> On 23/02/2026 12:51, Khristine Andreea Barbulescu wrote:
>>> On 2/20/2026 4:41 PM, Krzysztof Kozlowski wrote:
>>>> On 20/02/2026 15:36, Khristine Andreea Barbulescu wrote:
>>>>> Hello Krzysztof,
>>>>>
>>>>> On 2/20/2026 12:16 PM, Krzysztof Kozlowski wrote:
>>>>>> [You don't often get email from krzk@kernel.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>>>>>>
>>>>>> On 19/02/2026 12:36, Khristine Andreea Barbulescu wrote:
>>>>>>>>
>>>>>>>>> +      reg:
>>>>>>>>> +        maxItems: 1
>>>>>>>>
>>>>>>>> You have 'reg' so the node name should have unit-address.
>>>>>>>>
>>>>>>>> However, there's not any real DT resources in this child node, so you
>>>>>>>> should just drop it.
>>>>>>>>
>>>>>>>
>>>>>>> For context, SIUL2 exposes a set of platform‑capability and SoC identification registers that are split across the two discontiguous ranges: SIUL2-0 and SIUL2-1. These registers are the source of SoC information (e.g. identification and capability flags) that other subsystems are expected to consume (e.g. PCI Express). Because those fields are physically divided between the two SIUL2 ranges, consumers need reliable access to both ranges to correctly discover and configure the platform.
>>>>>>>
>>>>>>> Hence, my proposal is to keep the two 'syscon' child nodes.
>>>>>>
>>>>>> Please wrap your replies correctly, so this will be easily parseable.
>>>>>>
>>>>>> I do not understand the reasoning. If you have two register ranges, you
>>>>>> have two <reg> entries and having a child node has nothing to do with it.
>>>>>>
>>>>>
>>>>> I’ve reorganized the SIUL2 node with two syscon subnodes for the two
>>>>> register regions used to read system info, and a separate
>>>>> pinctrl/GPIO child (as discussed in the v8 06/10 thread [0]). The parent
>>>>> SIUL2 node now carries the bus addressing and ranges:
>>>>
>>>> That's not the answer to the comment. Read again:
>>>>
>>>> 1. Reviewer: No resources, so no separate node.
>>>> 2. Your answer: some soc capability and two address spaces
>>>> 3. Reviewer: Does not matter, address spaces can go again to original place
>>>> 4. Irrelevant reply.
>>>>
>>>> So again, read the first comment - do you have dedicated resources in
>>>> children?
>>>>
>>>>>  
>>>>>     siul2: siul2@4009c000 {
>>>>>         compatible = "nxp,s32g3-siul2", "nxp,s32g2-siul2";
>>>>>         #address-cells = <1>;
>>>>>         #size-cells = <1>;
>>>>>         ranges = <0x4009c000 0x4009c000 0x179c>,
>>>>> 		 <0x44010000 0x44010000 0x17b0>;
>>>>>  
>>>>>         siul20: siul20@4009c000 {
>>>>>             compatible = "nxp,s32g-siul2-syscfg", "syscon";
>>>>>             reg = <0x4009c000 0x179c>;
>>>>
>>>> 0x179c is odd size. Looks fake.
>>>>
>>>>
>>>>>         };
>>>>>  
>>>>>         siul21: siul21@44010000 {
>>>>>             compatible = "nxp,s32g-siul2-syscfg", "syscon";
>>>>
>>>> And two same devices with same compatible proof it.
>>>>
>>>> Best regards,
>>>> Krzysztof
>>>
>>> We don’t have dedicated resources for children. In particular,
>>
>> Then previous comments/review stay.
>>
>>> there are no resources allocated specifically for nodes like
>>> "nxp,s32g-siul2-syscfg". Their consumers are the pinctrl/gpio
>>> driver and other drivers that read SoC‑specific information from
>>> those shared registers.
>>>  
>>> My alternative is to keep two separate syscon providers for the
>>
>> You got review already.
>>
>> Best regards,
>> Krzysztof
> 
> Hi Krzysztof & Arnd,
> 
> I still believe that nvmem is a suitable and accurate mechanism for
> describing SoC‑specific identification information, as originally
> proposed in [0], assuming the necessary adjustments are made.
> 
> More specifically, instead of modeling software-defined cells, the nvmem
> layout would describe the actual hardware registers backing this
> information. One advantage of this approach is that consumer nodes (for
> example PCIe, Ethernet, or other IPs that need SoC identification data)
> can reference these registers using the standard nvmem-cells /
> nvmem-cell-names mechanism, without introducing custom, per-subsystem
> bindings.

nvmem is applicable only if this is NVMEM. Information about the soc is
not NVMEM, unless this are blow out fuses / efuse. Does not look like,
because SoC information is set probably during design phase, not board
assembly.


Best regards,
Krzysztof

Re: [PATCH v8 01/10] dt-bindings: mfd: add support for the NXP SIUL2 module

[Arnd Bergmann]

On Fri, Mar 13, 2026, at 18:10, Krzysztof Kozlowski wrote:
> On 25/02/2026 10:40, Ghennadi Procopciuc wrote:
>> On 2/23/2026 3:14 PM, Krzysztof Kozlowski wrote:
>>>> there are no resources allocated specifically for nodes like
>>>> "nxp,s32g-siul2-syscfg". Their consumers are the pinctrl/gpio
>>>> driver and other drivers that read SoC‑specific information from
>>>> those shared registers.
>>>>  
>>>> My alternative is to keep two separate syscon providers for the
>>>
>>> You got review already.
>>>
>> I still believe that nvmem is a suitable and accurate mechanism for
>> describing SoC‑specific identification information, as originally
>> proposed in [0], assuming the necessary adjustments are made.
>> 
>> More specifically, instead of modeling software-defined cells, the nvmem
>> layout would describe the actual hardware registers backing this
>> information. One advantage of this approach is that consumer nodes (for
>> example PCIe, Ethernet, or other IPs that need SoC identification data)
>> can reference these registers using the standard nvmem-cells /
>> nvmem-cell-names mechanism, without introducing custom, per-subsystem
>> bindings.
>
> nvmem is applicable only if this is NVMEM. Information about the soc is
> not NVMEM, unless this are blow out fuses / efuse. Does not look like,
> because SoC information is set probably during design phase, not board
> assembly.

Agreed, nvmem clearly makes no sense here, the patch description
appears to accurately describe the MMIO area as hardware registers
with a fixed meaning rather than a convention for how the
memory is being used.

That said, there is probably room for improvement, since some of
the register contents are read-only and could just be accessed
by the boot firmware in order to move the information into more
regular DT properties instead of defining bindings for drivers
to access the information in raw form.

    Arnd

Re: [PATCH v8 01/10] dt-bindings: mfd: add support for the NXP SIUL2 module

[Khristine Andreea Barbulescu]

On 3/14/2026 9:31 AM, Arnd Bergmann wrote:
> On Fri, Mar 13, 2026, at 18:10, Krzysztof Kozlowski wrote:
>> On 25/02/2026 10:40, Ghennadi Procopciuc wrote:
>>> On 2/23/2026 3:14 PM, Krzysztof Kozlowski wrote:
>>>>> there are no resources allocated specifically for nodes like
>>>>> "nxp,s32g-siul2-syscfg". Their consumers are the pinctrl/gpio
>>>>> driver and other drivers that read SoC‑specific information from
>>>>> those shared registers.
>>>>>  
>>>>> My alternative is to keep two separate syscon providers for the
>>>>
>>>> You got review already.
>>>>
>>> I still believe that nvmem is a suitable and accurate mechanism for
>>> describing SoC‑specific identification information, as originally
>>> proposed in [0], assuming the necessary adjustments are made.
>>>
>>> More specifically, instead of modeling software-defined cells, the nvmem
>>> layout would describe the actual hardware registers backing this
>>> information. One advantage of this approach is that consumer nodes (for
>>> example PCIe, Ethernet, or other IPs that need SoC identification data)
>>> can reference these registers using the standard nvmem-cells /
>>> nvmem-cell-names mechanism, without introducing custom, per-subsystem
>>> bindings.
>>
>> nvmem is applicable only if this is NVMEM. Information about the soc is
>> not NVMEM, unless this are blow out fuses / efuse. Does not look like,
>> because SoC information is set probably during design phase, not board
>> assembly.
> 
> Agreed, nvmem clearly makes no sense here, the patch description
> appears to accurately describe the MMIO area as hardware registers
> with a fixed meaning rather than a convention for how the
> memory is being used.
> 
> That said, there is probably room for improvement, since some of
> the register contents are read-only and could just be accessed
> by the boot firmware in order to move the information into more
> regular DT properties instead of defining bindings for drivers
> to access the information in raw form.
> 
>     Arnd

Hi Krzysztof & Arnd,

Assuming we drop the syscon approach entirely, for the SerDes
presence information we could follow Arnd’s suggestion and have
it provided by the boot firmware instead of exposing it through SIUL2.

However, SerDes presence is not the only information involved.
As mentioned in the earlier replies, we also have the PCIe device ID,
which will be needed once PCIe endpoint support is added.

Would it be acceptable to describe this information in DT, as in
other existing approaches [1], [2], [3], by adding a device-id
property to the PCIe node?
 
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/pci/renesas,r9a08g045-pcie.yaml#n130
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/renesas/r9a08g045.dtsi#n907
[3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi#n66

Best regards,
Khristine
 

Re: [PATCH v8 01/10] dt-bindings: mfd: add support for the NXP SIUL2 module

[Krzysztof Kozlowski]

On 23/03/2026 08:57, Khristine Andreea Barbulescu wrote:
> On 3/14/2026 9:31 AM, Arnd Bergmann wrote:
>> On Fri, Mar 13, 2026, at 18:10, Krzysztof Kozlowski wrote:
>>> On 25/02/2026 10:40, Ghennadi Procopciuc wrote:
>>>> On 2/23/2026 3:14 PM, Krzysztof Kozlowski wrote:
>>>>>> there are no resources allocated specifically for nodes like
>>>>>> "nxp,s32g-siul2-syscfg". Their consumers are the pinctrl/gpio
>>>>>> driver and other drivers that read SoC‑specific information from
>>>>>> those shared registers.
>>>>>>  
>>>>>> My alternative is to keep two separate syscon providers for the
>>>>>
>>>>> You got review already.
>>>>>
>>>> I still believe that nvmem is a suitable and accurate mechanism for
>>>> describing SoC‑specific identification information, as originally
>>>> proposed in [0], assuming the necessary adjustments are made.
>>>>
>>>> More specifically, instead of modeling software-defined cells, the nvmem
>>>> layout would describe the actual hardware registers backing this
>>>> information. One advantage of this approach is that consumer nodes (for
>>>> example PCIe, Ethernet, or other IPs that need SoC identification data)
>>>> can reference these registers using the standard nvmem-cells /
>>>> nvmem-cell-names mechanism, without introducing custom, per-subsystem
>>>> bindings.
>>>
>>> nvmem is applicable only if this is NVMEM. Information about the soc is
>>> not NVMEM, unless this are blow out fuses / efuse. Does not look like,
>>> because SoC information is set probably during design phase, not board
>>> assembly.
>>
>> Agreed, nvmem clearly makes no sense here, the patch description
>> appears to accurately describe the MMIO area as hardware registers
>> with a fixed meaning rather than a convention for how the
>> memory is being used.
>>
>> That said, there is probably room for improvement, since some of
>> the register contents are read-only and could just be accessed
>> by the boot firmware in order to move the information into more
>> regular DT properties instead of defining bindings for drivers
>> to access the information in raw form.
>>
>>     Arnd
> 
> Hi Krzysztof & Arnd,
> 
> Assuming we drop the syscon approach entirely, for the SerDes
> presence information we could follow Arnd’s suggestion and have
> it provided by the boot firmware instead of exposing it through SIUL2.

I think there is misunderstanding. By dropping syscon nodes, I meant to
drop the nodes. Remove them. It implies that whatever their contain must
go somewhere, right? Because your hardware is fixed and you cannot drop
it from the hardware, right?

So their parent node is the syscon.

Best regards,
Krzysztof

Re: [PATCH v8 01/10] dt-bindings: mfd: add support for the NXP SIUL2 module

[Arnd Bergmann]

On Mon, Mar 23, 2026, at 08:57, Khristine Andreea Barbulescu wrote:
> On 3/14/2026 9:31 AM, Arnd Bergmann wrote:
>> On Fri, Mar 13, 2026, at 18:10, Krzysztof Kozlowski wrote:
>> 
>> That said, there is probably room for improvement, since some of
>> the register contents are read-only and could just be accessed
>> by the boot firmware in order to move the information into more
>> regular DT properties instead of defining bindings for drivers
>> to access the information in raw form.
>
> Assuming we drop the syscon approach entirely, for the SerDes
> presence information we could follow Arnd’s suggestion and have
> it provided by the boot firmware instead of exposing it through SIUL2.

I didn't say you would necessarily drop the syscon interface
entirely, but for each user of it, you can see if the data
in it is static at runtime, and if so, you turn it into
devicetree properties.

> However, SerDes presence is not the only information involved.
> As mentioned in the earlier replies, we also have the PCIe device ID,
> which will be needed once PCIe endpoint support is added.
>
> Would it be acceptable to describe this information in DT, as in
> other existing approaches [1], [2], [3], by adding a device-id
> property to the PCIe node?

I don't know how endpoint devices normally get the vendor/device
ID pair, but as far as I can tell, these are not normally assigned
by the boot firmware but are picked by whatever function driver is
running on the endpoint device: the idea for the device ID is
to identify the protocol that a driver on the host side needs to
use, rather than identify what hardware it is running on.

In that case, neither DT nor syscon are an appropriate way to
pass the PCI device ID.

From looking at the links you provided, all of those appear
to refer to host mode vendor/device ID pairs that get written
into the device at probe time, either because the boot firmware
fails to initialize the devices properly, or because the ID data
is not persistent across a device reset. In endpoint mode,
this would not apply.

      Arnd

Re: [PATCH v8 01/10] dt-bindings: mfd: add support for the NXP SIUL2 module

[Krzysztof Kozlowski]

On 19/02/2026 12:36, Khristine Andreea Barbulescu wrote:
> Hello Rob,
> 
> On 1/21/2026 4:19 AM, Rob Herring wrote:
>> On Tue, Jan 20, 2026 at 01:59:13PM +0200, Khristine Andreea Barbulescu wrote:
>>> From: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
>>>


BTW, one more thing, replying one month after the review, which was
given to you within one day, means entire context is gone, so actually
we assume that you will agree with the review, not discuss it.

Otherwise, please wait one more month for our response to your response,
before you post next version. :/

Best regards,
Krzysztof

Re: [PATCH v8 01/10] dt-bindings: mfd: add support for the NXP SIUL2 module

[Khristine Andreea Barbulescu]

Hello Krzysztof,

On 2/20/2026 12:18 PM, Krzysztof Kozlowski wrote:
> [You don't often get email from krzk@kernel.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> On 19/02/2026 12:36, Khristine Andreea Barbulescu wrote:
>> Hello Rob,
>>
>> On 1/21/2026 4:19 AM, Rob Herring wrote:
>>> On Tue, Jan 20, 2026 at 01:59:13PM +0200, Khristine Andreea Barbulescu wrote:
>>>> From: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
>>>>
> 
> 
> BTW, one more thing, replying one month after the review, which was
> given to you within one day, means entire context is gone, so actually
> we assume that you will agree with the review, not discuss it.
> 
> Otherwise, please wait one more month for our response to your response,
> before you post next version. :/
> 
> Best regards,
> KrzysztofThanks for the review. I’ve recently taken over this patch set
which has a long history and I delayed replies to avoid sending
incomplete or misleading information while I was catching up.
I understand your point about timing and will prioritize replying
if there is anything unclear going forward. Apologies for my delay.

Best regards,
Khristine

[PATCH v8 02/10] mfd: nxp-siul2: add support for NXP SIUL2

[Khristine Andreea Barbulescu]

From: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>

Add support for the NXP SIUL2 module as an MFD device.

SIUL2 (System Integration Unit Lite) is a hardware module which
implements various functionalities:
- reading SoC information
- pinctrl
- GPIO (including interrupts)

Add support for pinctrl&GPIO(one cell).

There are multiple register types in the SIUL2 module:
- MIDR (MCU ID Register)
	* contains information about the SoC.
- Interrupt related registers
	* There are 32 interrupts named EIRQ. An EIRQ
	  may be routed to one or more GPIOs. Not all
	  GPIOs have EIRQs associated with them
- MSCR (Multiplexed Signal Configuration Register)
	* handle pinmuxing and pinconf
- IMCR (Input Multiplexed Signal Configuration Register)
	* are part of pinmuxing
- PGPDO/PGPDI (Parallel GPIO Pad Data Out/In Register)
	* Write/Read the GPIO value

There are two SIUL2 modules in the S32G SoC. This driver handles
both because functionality is shared between them. For example:
some GPIOs in SIUL2_0 have interrupt capability but the registers
configuring this are in SIUL2_1.

Signed-off-by: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
Signed-off-by: Khristine Andreea Barbulescu <khristineandreea.barbulescu@oss.nxp.com>
---
 drivers/mfd/Kconfig           |  13 +
 drivers/mfd/Makefile          |   1 +
 drivers/mfd/nxp-siul2.c       | 440 ++++++++++++++++++++++++++++++++++
 include/linux/mfd/nxp-siul2.h |  55 +++++
 4 files changed, 509 insertions(+)
 create mode 100644 drivers/mfd/nxp-siul2.c
 create mode 100644 include/linux/mfd/nxp-siul2.h

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index aace5766b38a..3a8104a703ff 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1232,6 +1232,19 @@ config MFD_NTXEC
 	  certain e-book readers designed by the original design manufacturer
 	  Netronix.
 
+config MFD_NXP_SIUL2
+	tristate "NXP SIUL2 MFD driver"
+	select MFD_CORE
+	select REGMAP_MMIO
+	depends on ARCH_S32 || COMPILE_TEST
+	help
+	  Select this to get support for the NXP SIUL2 (System Integration
+	  Unit Lite) module. This hardware block contains registers for
+	  SoC information, pinctrl and GPIO functionality. This will
+	  probe a MFD driver which will contain cells for a combined
+	  pinctrl&GPIO driver and syscon cells for SIUL2_0 and SIUL2_1
+	  to expose SoC information.
+
 config MFD_RETU
 	tristate "Nokia Retu and Tahvo multi-function device"
 	select MFD_CORE
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index e75e8045c28a..d29adf8f04f0 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -230,6 +230,7 @@ obj-$(CONFIG_MFD_INTEL_PMC_BXT)	+= intel_pmc_bxt.o
 obj-$(CONFIG_MFD_PALMAS)	+= palmas.o
 obj-$(CONFIG_MFD_VIPERBOARD)    += viperboard.o
 obj-$(CONFIG_MFD_NTXEC)		+= ntxec.o
+obj-$(CONFIG_MFD_NXP_SIUL2) 	+= nxp-siul2.o
 obj-$(CONFIG_MFD_RC5T583)	+= rc5t583.o rc5t583-irq.o
 obj-$(CONFIG_MFD_RK8XX)		+= rk8xx-core.o
 obj-$(CONFIG_MFD_RK8XX_I2C)	+= rk8xx-i2c.o
diff --git a/drivers/mfd/nxp-siul2.c b/drivers/mfd/nxp-siul2.c
new file mode 100644
index 000000000000..543de4aac1c0
--- /dev/null
+++ b/drivers/mfd/nxp-siul2.c
@@ -0,0 +1,440 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * SIUL2(System Integration Unit Lite) MFD driver
+ *
+ * Copyright 2025-2026 NXP
+ */
+#include <linux/init.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/nxp-siul2.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+
+#define S32G_NUM_SIUL2 2
+
+#define S32_REG_RANGE(start, end, name, access)		\
+	{						\
+		.reg_name = (name),			\
+		.reg_start_offset = (start),		\
+		.reg_end_offset = (end),		\
+		.reg_access = (access),			\
+		.valid = true,				\
+	}
+
+#define S32_INVALID_REG_RANGE		\
+	{				\
+		.reg_name = NULL,	\
+		.reg_access = NULL,	\
+		.valid = false,		\
+	}
+
+static const struct mfd_cell nxp_siul2_devs[] = {
+	{
+		.name = "s32g-siul2-pinctrl",
+	}
+};
+
+/**
+ * struct nxp_siul2_reg_range_info: a register range in SIUL2
+ * @reg_name: the name for the register range
+ * @reg_access: the read/write access tables if not NULL
+ * @reg_start_offset: the first valid register offset
+ * @reg_end_offset: the last valid register offset
+ * @valid: whether the register range is valid or not
+ */
+struct nxp_siul2_reg_range_info {
+	const char *reg_name;
+	const struct regmap_access_table *reg_access;
+	unsigned int reg_start_offset;
+	unsigned int reg_end_offset;
+	bool valid;
+};
+
+static const struct regmap_range s32g2_siul2_0_imcr_reg_ranges[] = {
+	/* IMCR0 - IMCR1 */
+	regmap_reg_range(0, 4),
+	/* IMCR3 - IMCR61 */
+	regmap_reg_range(0xC, 0xF4),
+	/* IMCR68 - IMCR83 */
+	regmap_reg_range(0x110, 0x14C)
+};
+
+static const struct regmap_access_table s32g2_siul2_0_imcr = {
+	.yes_ranges = s32g2_siul2_0_imcr_reg_ranges,
+	.n_yes_ranges = ARRAY_SIZE(s32g2_siul2_0_imcr_reg_ranges)
+};
+
+static const struct regmap_range s32g2_siul2_0_pgpd_reg_ranges[] = {
+	/* PGPD*0 - PGPD*5 */
+	regmap_reg_range(0, 0xA),
+	/* PGPD*6 - PGPD*6 */
+	regmap_reg_range(0xE, 0xE),
+};
+
+static const struct regmap_access_table s32g2_siul2_0_pgpd = {
+	.yes_ranges = s32g2_siul2_0_pgpd_reg_ranges,
+	.n_yes_ranges = ARRAY_SIZE(s32g2_siul2_0_pgpd_reg_ranges)
+};
+
+static const struct regmap_range s32g2_siul2_1_irq_reg_ranges[] = {
+	/* DISR0 */
+	regmap_reg_range(0x10, 0x10),
+	/* DIRER0 */
+	regmap_reg_range(0x18, 0x18),
+	/* DIRSR0 */
+	regmap_reg_range(0x20, 0x20),
+	/* IREER0 */
+	regmap_reg_range(0x28, 0x28),
+	/* IFEER0 */
+	regmap_reg_range(0x30, 0x30),
+};
+
+static const struct regmap_access_table s32g2_siul2_1_irq = {
+	.yes_ranges = s32g2_siul2_1_irq_reg_ranges,
+	.n_yes_ranges = ARRAY_SIZE(s32g2_siul2_1_irq_reg_ranges),
+};
+
+static const struct regmap_range s32g2_siul2_1_irq_volatile_reg_range[] = {
+	/* DISR0 */
+	regmap_reg_range(0x10, 0x10)
+};
+
+static const struct regmap_access_table s32g2_siul2_1_irq_volatile = {
+	.yes_ranges = s32g2_siul2_1_irq_volatile_reg_range,
+	.n_yes_ranges = ARRAY_SIZE(s32g2_siul2_1_irq_volatile_reg_range),
+};
+
+static const struct regmap_range s32g2_siul2_1_mscr_reg_ranges[] = {
+	/* MSCR112 - MSCR122 */
+	regmap_reg_range(0, 0x28),
+	/* MSCR144 - MSCR190 */
+	regmap_reg_range(0x80, 0x138)
+};
+
+static const struct regmap_access_table s32g2_siul2_1_mscr = {
+	.yes_ranges = s32g2_siul2_1_mscr_reg_ranges,
+	.n_yes_ranges = ARRAY_SIZE(s32g2_siul2_1_mscr_reg_ranges),
+};
+
+static const struct regmap_range s32g2_siul2_1_imcr_reg_ranges[] = {
+	/* IMCR119 - IMCR121 */
+	regmap_reg_range(0, 8),
+	/* IMCR128 - IMCR129 */
+	regmap_reg_range(0x24, 0x28),
+	/* IMCR143 - IMCR151 */
+	regmap_reg_range(0x60, 0x80),
+	/* IMCR153 - IMCR161 */
+	regmap_reg_range(0x88, 0xA8),
+	/* IMCR205 - IMCR212 */
+	regmap_reg_range(0x158, 0x174),
+	/* IMCR224 - IMCR225 */
+	regmap_reg_range(0x1A4, 0x1A8),
+	/* IMCR233 - IMCR248 */
+	regmap_reg_range(0x1C8, 0x204),
+	/* IMCR273 - IMCR274 */
+	regmap_reg_range(0x268, 0x26C),
+	/* IMCR278 - IMCR281 */
+	regmap_reg_range(0x27C, 0x288),
+	/* IMCR283 - IMCR286 */
+	regmap_reg_range(0x290, 0x29C),
+	/* IMCR288 - IMCR294 */
+	regmap_reg_range(0x2A4, 0x2BC),
+	/* IMCR296 - IMCR302 */
+	regmap_reg_range(0x2C4, 0x2DC),
+	/* IMCR304 - IMCR310 */
+	regmap_reg_range(0x2E4, 0x2FC),
+	/* IMCR312 - IMCR314 */
+	regmap_reg_range(0x304, 0x30C),
+	/* IMCR316 */
+	regmap_reg_range(0x314, 0x314),
+	/* IMCR 318 */
+	regmap_reg_range(0x31C, 0x31C),
+	/* IMCR322 - IMCR340 */
+	regmap_reg_range(0x32C, 0x374),
+	/* IMCR343 - IMCR360 */
+	regmap_reg_range(0x380, 0x3C4),
+	/* IMCR363 - IMCR380 */
+	regmap_reg_range(0x3D0, 0x414),
+	/* IMCR383 - IMCR393 */
+	regmap_reg_range(0x420, 0x448),
+	/* IMCR398 - IMCR433 */
+	regmap_reg_range(0x45C, 0x4E8),
+	/* IMCR467 - IMCR470 */
+	regmap_reg_range(0x570, 0x57C),
+	/* IMCR473 - IMCR475 */
+	regmap_reg_range(0x588, 0x590),
+	/* IMCR478 - IMCR480*/
+	regmap_reg_range(0x59C, 0x5A4),
+	/* IMCR483 - IMCR485 */
+	regmap_reg_range(0x5B0, 0x5B8),
+	/* IMCR488 - IMCR490 */
+	regmap_reg_range(0x5C4, 0x5CC),
+	/* IMCR493 - IMCR495 */
+	regmap_reg_range(0x5D8, 0x5E0),
+};
+
+static const struct regmap_access_table s32g2_siul2_1_imcr = {
+	.yes_ranges = s32g2_siul2_1_imcr_reg_ranges,
+	.n_yes_ranges = ARRAY_SIZE(s32g2_siul2_1_imcr_reg_ranges)
+};
+
+static const struct regmap_range s32g2_siul2_1_pgpd_reg_ranges[] = {
+	/* PGPD*7 */
+	regmap_reg_range(0xC, 0xC),
+	/* PGPD*9 */
+	regmap_reg_range(0x10, 0x10),
+	/* PDPG*10 - PGPD*11 */
+	regmap_reg_range(0x14, 0x16),
+};
+
+static const struct regmap_access_table s32g2_siul2_1_pgpd = {
+	.yes_ranges = s32g2_siul2_1_pgpd_reg_ranges,
+	.n_yes_ranges = ARRAY_SIZE(s32g2_siul2_1_pgpd_reg_ranges)
+};
+
+static const struct nxp_siul2_reg_range_info
+s32g2_reg_ranges[S32G_NUM_SIUL2][SIUL2_NUM_REG_TYPES] = {
+	/* SIUL2_0 */
+	{
+		[SIUL2_MIDR] = S32_REG_RANGE(4, 8, "SIUL2_0_MIDR", NULL),
+		/* Interrupts are to be controlled from SIUL2_1 */
+		[SIUL2_IRQ] = S32_INVALID_REG_RANGE,
+		[SIUL2_MSCR] = S32_REG_RANGE(0x240, 0x3D4, "SIUL2_0_MSCR",
+					     NULL),
+		[SIUL2_IMCR] = S32_REG_RANGE(0xA40, 0xB8C, "SIUL2_0_IMCR",
+					     &s32g2_siul2_0_imcr),
+		[SIUL2_PGPDO] = S32_REG_RANGE(0x1700, 0x170E,
+					      "SIUL2_0_PGPDO",
+					      &s32g2_siul2_0_pgpd),
+		[SIUL2_PGPDI] = S32_REG_RANGE(0x1740, 0x174E,
+					      "SIUL2_0_PGPDI",
+					      &s32g2_siul2_0_pgpd),
+	},
+	/* SIUL2_1 */
+	{
+		[SIUL2_MIDR] = S32_REG_RANGE(4, 8, "SIUL2_1_MIDR", NULL),
+		[SIUL2_IRQ] = S32_REG_RANGE(0x10, 0xC0, "SIUL2_1_IRQ",
+					    &s32g2_siul2_1_irq),
+		[SIUL2_MSCR] = S32_REG_RANGE(0x400, 0x538, "SIUL2_1_MSCR",
+					     &s32g2_siul2_1_mscr),
+		[SIUL2_IMCR] = S32_REG_RANGE(0xC1C, 0x11FC, "SIUL2_1_IMCR",
+					     &s32g2_siul2_1_imcr),
+		[SIUL2_PGPDO] = S32_REG_RANGE(0x1700, 0x1716,
+					      "SIUL2_1_PGPDO",
+					      &s32g2_siul2_1_pgpd),
+		[SIUL2_PGPDI] = S32_REG_RANGE(0x1740, 0x1756,
+					      "SIUL2_1_PGPDI",
+					      &s32g2_siul2_1_pgpd),
+	},
+};
+
+static const struct regmap_config nxp_siul2_regmap_irq_conf = {
+	.val_bits = 32,
+	.val_format_endian = REGMAP_ENDIAN_LITTLE,
+	.reg_bits = 32,
+	.reg_stride = 4,
+	.cache_type = REGCACHE_FLAT,
+	.use_raw_spinlock = true,
+	.volatile_table = &s32g2_siul2_1_irq_volatile,
+};
+
+static const struct regmap_config nxp_siul2_regmap_generic_conf = {
+	.val_bits = 32,
+	.val_format_endian = REGMAP_ENDIAN_LITTLE,
+	.reg_bits = 32,
+	.reg_stride = 4,
+	.cache_type = REGCACHE_FLAT,
+	.use_raw_spinlock = true,
+};
+
+static const struct regmap_config nxp_siul2_regmap_pgpdo_conf = {
+	.val_bits = 16,
+	.val_format_endian = REGMAP_ENDIAN_LITTLE,
+	.reg_bits = 32,
+	.reg_stride = 2,
+	.cache_type = REGCACHE_FLAT,
+	.use_raw_spinlock = true,
+};
+
+static const struct regmap_config nxp_siul2_regmap_pgpdi_conf = {
+	.val_bits = 16,
+	.val_format_endian = REGMAP_ENDIAN_LITTLE,
+	.reg_bits = 32,
+	.reg_stride = 2,
+	.cache_type = REGCACHE_NONE,
+	.use_raw_spinlock = true,
+};
+
+static int nxp_siul2_init_regmap(struct platform_device *pdev,
+				 void __iomem *base, unsigned int siul)
+{
+	const struct regmap_config *regmap_configs[SIUL2_NUM_REG_TYPES] = {
+		[SIUL2_MIDR]	= &nxp_siul2_regmap_generic_conf,
+		[SIUL2_IRQ]	= &nxp_siul2_regmap_irq_conf,
+		[SIUL2_MSCR]	= &nxp_siul2_regmap_generic_conf,
+		[SIUL2_IMCR]	= &nxp_siul2_regmap_generic_conf,
+		[SIUL2_PGPDO]	= &nxp_siul2_regmap_pgpdo_conf,
+		[SIUL2_PGPDI]	= &nxp_siul2_regmap_pgpdi_conf,
+	};
+	const struct nxp_siul2_reg_range_info *tmp_range;
+	struct regmap_config tmp_conf;
+	struct nxp_siul2_info *info;
+	struct nxp_siul2_mfd *priv;
+	void __iomem *reg_start;
+	int i;
+
+	priv = platform_get_drvdata(pdev);
+	info = &priv->siul2[siul];
+
+	for (i = 0; i < SIUL2_NUM_REG_TYPES; i++) {
+		if (!s32g2_reg_ranges[siul][i].valid)
+			continue;
+
+		tmp_range = &s32g2_reg_ranges[siul][i];
+		tmp_conf = *regmap_configs[i];
+		tmp_conf.name = tmp_range->reg_name;
+		tmp_conf.max_register =
+			tmp_range->reg_end_offset - tmp_range->reg_start_offset;
+
+		if (tmp_conf.cache_type != REGCACHE_NONE)
+			tmp_conf.num_reg_defaults_raw =
+				1 + tmp_conf.max_register / tmp_conf.reg_stride;
+
+		if (tmp_range->reg_access) {
+			tmp_conf.wr_table = tmp_range->reg_access;
+			tmp_conf.rd_table = tmp_range->reg_access;
+		}
+
+		reg_start = base + tmp_range->reg_start_offset;
+		info->regmaps[i] = devm_regmap_init_mmio(&pdev->dev, reg_start,
+							 &tmp_conf);
+		if (IS_ERR(info->regmaps[i]))
+			return dev_err_probe(&pdev->dev,
+					     PTR_ERR(info->regmaps[i]),
+					     "regmap %d init failed: %ld\n", i,
+					     PTR_ERR(info->regmaps[i]));
+	}
+
+	return 0;
+}
+
+static int nxp_siul2_parse_dtb(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct of_phandle_args pinspec;
+	struct nxp_siul2_mfd *priv;
+	struct device_node *child;
+	struct resource res;
+	void __iomem *base;
+	int i = 0, ret;
+
+	priv = platform_get_drvdata(pdev);
+
+	for_each_child_of_node(pdev->dev.of_node, child) {
+		if (!of_device_is_compatible(child, "syscon"))
+			continue;
+
+		if (i >= priv->num_siul2)
+			break;
+
+		ret = of_address_to_resource(child, 0, &res);
+		if (ret)
+			return dev_err_probe(&pdev->dev, ret,
+					     "Failed to get resource for %pOF\n", child);
+
+		base = devm_ioremap_resource(&pdev->dev, &res);
+		if (IS_ERR(base))
+			return dev_err_probe(&pdev->dev, PTR_ERR(base),
+					     "Failed to ioremap %pOF\n", child);
+
+		ret = nxp_siul2_init_regmap(pdev, base, i);
+		if (ret)
+			return ret;
+
+		/* Parse gpio-ranges for current SIUL2 instance */
+		{
+			ret = of_parse_phandle_with_fixed_args(np, "gpio-ranges", 3,
+							       i, &pinspec);
+			if (ret)
+				return ret;
+
+			of_node_put(pinspec.np);
+
+			if (pinspec.args_count != 3)
+				return dev_err_probe(&pdev->dev, -EINVAL,
+						     "Invalid pinspec count: %d\n",
+						     pinspec.args_count);
+
+			priv->siul2[i].gpio_base = pinspec.args[1];
+			priv->siul2[i].gpio_num = pinspec.args[2];
+		}
+
+		i++;
+	}
+
+	return 0;
+}
+
+static int nxp_siul2_probe(struct platform_device *pdev)
+{
+	struct nxp_siul2_mfd *priv;
+	struct device_node *child;
+	int ret, i = 0;
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->num_siul2 = S32G_NUM_SIUL2;
+	priv->siul2 = devm_kcalloc(&pdev->dev, priv->num_siul2,
+				   sizeof(*priv->siul2), GFP_KERNEL);
+	if (!priv->siul2)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, priv);
+	ret = nxp_siul2_parse_dtb(pdev);
+	if (ret)
+		return ret;
+
+    /* Register MIDR regmaps as syscon for each SIUL2 node */
+	for_each_child_of_node(pdev->dev.of_node, child) {
+		if (!of_device_is_compatible(child, "syscon"))
+			continue;
+
+		if (i >= priv->num_siul2)
+			break;
+
+		if (priv->siul2[i].regmaps[SIUL2_MIDR]) {
+			of_syscon_register_regmap(child,
+						  priv->siul2[i].regmaps[SIUL2_MIDR]);
+		}
+
+		i++;
+	}
+
+	return devm_mfd_add_devices(&pdev->dev, PLATFORM_DEVID_AUTO,
+				    nxp_siul2_devs, ARRAY_SIZE(nxp_siul2_devs),
+				    NULL, 0, NULL);
+}
+
+static const struct of_device_id nxp_siul2_dt_ids[] = {
+	{ .compatible = "nxp,s32g2-siul2" },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, nxp_siul2_dt_ids);
+
+static struct platform_driver nxp_siul2_mfd_driver = {
+	.driver = {
+		.name		= "nxp-siul2-mfd",
+		.of_match_table	= nxp_siul2_dt_ids,
+	},
+	.probe = nxp_siul2_probe,
+};
+
+module_platform_driver(nxp_siul2_mfd_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("NXP SIUL2 MFD driver");
+MODULE_AUTHOR("Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>");
diff --git a/include/linux/mfd/nxp-siul2.h b/include/linux/mfd/nxp-siul2.h
new file mode 100644
index 000000000000..407be79bbf11
--- /dev/null
+++ b/include/linux/mfd/nxp-siul2.h
@@ -0,0 +1,55 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * S32 SIUL2 core definitions
+ *
+ * Copyright 2025-2026 NXP
+ */
+
+#ifndef __DRIVERS_MFD_NXP_SIUL2_H
+#define __DRIVERS_MFD_NXP_SIUL2_H
+
+#include <linux/regmap.h>
+
+/**
+ * enum nxp_siul2_reg_type - an enum for SIUL2 reg types
+ * @SIUL2_MIDR - SoC info
+ * @SIUL2_IRQ - IRQ related registers, only valid in SIUL2_1
+ * @SIUL2_MSCR - used for pinmuxing and pinconf
+ * @SIUL2_IMCR - used for pinmuxing
+ * @SIUL2_PGPDO - writing the GPIO value
+ * @SIUL2_PGPDI - reading the GPIO value
+ */
+enum nxp_siul2_reg_type {
+	SIUL2_MIDR,
+	SIUL2_IRQ,
+	SIUL2_MSCR,
+	SIUL2_IMCR,
+	SIUL2_PGPDO,
+	SIUL2_PGPDI,
+
+	SIUL2_NUM_REG_TYPES
+};
+
+/**
+ * struct nxp_siul2_info - details about one SIUL2 hardware instance
+ * @regmaps: the regmaps for each register type for a SIUL2 hardware instance
+ * @gpio_base: the first GPIO in this SIUL2 module
+ * @gpio_num: the number of GPIOs in this SIUL2 module
+ */
+struct nxp_siul2_info {
+	struct regmap *regmaps[SIUL2_NUM_REG_TYPES];
+	u32 gpio_base;
+	u32 gpio_num;
+};
+
+/**
+ * struct nxp_siul2_mfd - driver data
+ * @siul2: info about the SIUL2 modules present
+ * @num_siul2: number of siul2 modules
+ */
+struct nxp_siul2_mfd {
+	struct nxp_siul2_info *siul2;
+	u8 num_siul2;
+};
+
+#endif /* __DRIVERS_MFD_NXP_SIUL2_H */
-- 
2.50.1

Re: [PATCH v8 02/10] mfd: nxp-siul2: add support for NXP SIUL2

[Sander Vanheule]

Hi Khristine,

On Tue, 2026-01-20 at 13:59 +0200, Khristine Andreea Barbulescu wrote:
> +static const struct regmap_config nxp_siul2_regmap_pgpdo_conf = {
> +	.val_bits = 16,
> +	.val_format_endian = REGMAP_ENDIAN_LITTLE,
> +	.reg_bits = 32,
> +	.reg_stride = 2,
> +	.cache_type = REGCACHE_FLAT,
> +	.use_raw_spinlock = true,
> +};

I see you are using REGCACHE_FLAT ...

> +		if (tmp_conf.cache_type != REGCACHE_NONE)
> +			tmp_conf.num_reg_defaults_raw =
> +				1 + tmp_conf.max_register /
> tmp_conf.reg_stride;

... and initialize the cache defaults from hardware.

This series predates the addition of REGCACHE_FLAT_S in v6.19 with commit
9c7f7262bc1a ("regmap: add flat cache with sparse validity"), but I think
switching to the sparse flat cache would allow you to drop this. Then the cache
will just be initialized on the first hardware access instead of at regmap init.


Best,
Sander

[PATCH v8 03/10] arm64: dts: s32g: change pinctrl node into the new mfd node

[Khristine Andreea Barbulescu]

From: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>

This commit will switch to the new mfd node for representing the SIUL2
hardware. The old pinctrl binding for SIUL2 will be deprecated in a
later commit since it doesn't correctly represent the hardware.

SIUL2 is now represented as an mfd device. Move the pinctrl related
properties inside the new "nxp-siul2" node. The latter one is now used
to represent the mfd device.

This change came as a result of upstream review in the following series:
https://lore.kernel.org/linux-gpio/a924bbb6-96ec-40be-9d82-a76b2ab73afd@oss.nxp.com/
https://lore.kernel.org/all/20240926143122.1385658-3-andrei.stefanescu@oss.nxp.com/

The SIUL2 module has multiple capabilities. It has support for reading
SoC information, pinctrl and GPIO. All of this functionality is part of
the same register space. The initial pinctrl driver treated the pinctrl
functionality as separate from the GPIO one. However, they do rely on
common registers and a long, detailed and specific register range list
would be required for pinctrl&GPIO (carving out the necessary memory
for each function). Moreover, in some cases this wouldn't be enough. For
example reading a GPIO's direction would require a read of the MSCR
register corresponding to that pin. This would not be possible in the
GPIO driver because all of the MSCR registers are referenced by the
pinctrl driver.

Signed-off-by: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
Signed-off-by: Khristine Andreea Barbulescu <khristineandreea.barbulescu@oss.nxp.com>
---
 arch/arm64/boot/dts/freescale/s32g2.dtsi | 35 ++++++++++++++----------
 arch/arm64/boot/dts/freescale/s32g3.dtsi | 35 ++++++++++++++----------
 2 files changed, 42 insertions(+), 28 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/s32g2.dtsi b/arch/arm64/boot/dts/freescale/s32g2.dtsi
index 51d00dac12de..fe36851ded15 100644
--- a/arch/arm64/boot/dts/freescale/s32g2.dtsi
+++ b/arch/arm64/boot/dts/freescale/s32g2.dtsi
@@ -122,20 +122,27 @@ rtc0: rtc@40060000 {
 			clock-names = "ipg", "source0";
 		};
 
-		pinctrl: 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>;
+		pinctrl: pinctrl@4009c000 {
+			compatible = "nxp,s32g2-siul2";
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-ranges = <&pinctrl 0 0 102>, <&pinctrl 112 112 79>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			interrupts = <GIC_SPI 210 IRQ_TYPE_LEVEL_HIGH>;
+			#address-cells = <1>;
+			#size-cells = <1>;
+			ranges;
+
+			siul2_0: siul2_0@4009c000 {
+				compatible = "syscon";
+				reg = <0x4009c000 0x179c>;
+			};
+
+			siul2_1: siul2_1@44010000 {
+				compatible = "syscon";
+				reg = <0x44010000 0x17b0>;
+			};
 
 			jtag_pins: jtag-pins {
 				jtag-grp0 {
diff --git a/arch/arm64/boot/dts/freescale/s32g3.dtsi b/arch/arm64/boot/dts/freescale/s32g3.dtsi
index eff7673e7f34..5d5d21b74019 100644
--- a/arch/arm64/boot/dts/freescale/s32g3.dtsi
+++ b/arch/arm64/boot/dts/freescale/s32g3.dtsi
@@ -180,20 +180,27 @@ rtc0: rtc@40060000 {
 			clock-names = "ipg", "source0";
 		};
 
-		pinctrl: 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>;
+		pinctrl: pinctrl@4009c000 {
+			compatible = "nxp,s32g3-siul2", "nxp,s32g2-siul2";
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-ranges = <&pinctrl 0 0 102>, <&pinctrl 112 112 79>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			interrupts = <GIC_SPI 210 IRQ_TYPE_LEVEL_HIGH>;
+			#address-cells = <1>;
+			#size-cells = <1>;
+			ranges;
+
+			siul2_0: siul2_0@4009c000 {
+				compatible = "syscon";
+				reg = <0x4009c000 0x179c>;
+			};
+
+			siul2_1: siul2_1@44010000 {
+				compatible = "syscon";
+				reg = <0x44010000 0x17b0>;
+			};
 
 			jtag_pins: jtag-pins {
 				jtag-grp0 {
-- 
2.50.1

Re: [PATCH v8 03/10] arm64: dts: s32g: change pinctrl node into the new mfd node

[Linus Walleij]

Hi Khristine,

On Tue, Jan 20, 2026 at 12:59 PM Khristine Andreea Barbulescu
<khristineandreea.barbulescu@oss.nxp.com> wrote:

> From: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
>
> This commit will switch to the new mfd node for representing the SIUL2
> hardware. The old pinctrl binding for SIUL2 will be deprecated in a
> later commit since it doesn't correctly represent the hardware.
>
> SIUL2 is now represented as an mfd device. Move the pinctrl related
> properties inside the new "nxp-siul2" node. The latter one is now used
> to represent the mfd device.
>
> This change came as a result of upstream review in the following series:
> https://lore.kernel.org/linux-gpio/a924bbb6-96ec-40be-9d82-a76b2ab73afd@oss.nxp.com/
> https://lore.kernel.org/all/20240926143122.1385658-3-andrei.stefanescu@oss.nxp.com/
>
> The SIUL2 module has multiple capabilities. It has support for reading
> SoC information, pinctrl and GPIO. All of this functionality is part of
> the same register space. The initial pinctrl driver treated the pinctrl
> functionality as separate from the GPIO one. However, they do rely on
> common registers and a long, detailed and specific register range list
> would be required for pinctrl&GPIO (carving out the necessary memory
> for each function). Moreover, in some cases this wouldn't be enough. For
> example reading a GPIO's direction would require a read of the MSCR
> register corresponding to that pin. This would not be possible in the
> GPIO driver because all of the MSCR registers are referenced by the
> pinctrl driver.
>
> Signed-off-by: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
> Signed-off-by: Khristine Andreea Barbulescu <khristineandreea.barbulescu@oss.nxp.com>

OK this makes sense.
Acked-by: Linus Walleij <linusw@kernel.org>

Yours,
Linus Walleij

[PATCH v8 04/10] pinctrl: s32cc: use dev_err_probe() and improve error messages

[Khristine Andreea Barbulescu]

From: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>

Change dev_err&return statements into dev_err_probe throughout the driver
on the probing path. Moreover, add/fix some comments and print
statements.

Signed-off-by: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
---
 drivers/pinctrl/nxp/pinctrl-s32cc.c | 106 +++++++++++++++-------------
 1 file changed, 55 insertions(+), 51 deletions(-)

diff --git a/drivers/pinctrl/nxp/pinctrl-s32cc.c b/drivers/pinctrl/nxp/pinctrl-s32cc.c
index 35511f83d056..a98f8e7c2768 100644
--- a/drivers/pinctrl/nxp/pinctrl-s32cc.c
+++ b/drivers/pinctrl/nxp/pinctrl-s32cc.c
@@ -2,7 +2,7 @@
 /*
  * Core driver for the S32 CC (Common Chassis) pin controller
  *
- * Copyright 2017-2022,2024 NXP
+ * Copyright 2017-2022,2024-2025 NXP
  * Copyright (C) 2022 SUSE LLC
  * Copyright 2015-2016 Freescale Semiconductor, Inc.
  */
@@ -60,14 +60,20 @@ static u32 get_pin_func(u32 pinmux)
 	return pinmux & GENMASK(3, 0);
 }
 
+/**
+ * struct s32_pinctrl_mem_region - memory region for a set of SIUL2 registers
+ * @map: regmap used for this range
+ * @pin_range: the pins controlled by these registers
+ * @name: name of the current range
+ */
 struct s32_pinctrl_mem_region {
 	struct regmap *map;
 	const struct s32_pin_range *pin_range;
 	char name[8];
 };
 
-/*
- * Holds pin configuration for GPIO's.
+/**
+ * struct gpio_pin_config - holds pin configuration for GPIO's
  * @pin_id: Pin ID for this GPIO
  * @config: Pin settings
  * @list: Linked list entry for each gpio pin
@@ -78,21 +84,23 @@ struct gpio_pin_config {
 	struct list_head list;
 };
 
-/*
- * Pad config save/restore for power suspend/resume.
+/**
+ * struct s32_pinctrl_context - pad config save/restore for suspend/resume
+ * @pads: saved values for the pards
  */
 struct s32_pinctrl_context {
 	unsigned int *pads;
 };
 
-/*
+/**
+ * struct s32_pinctrl - private driver data
  * @dev: a pointer back to containing device
  * @pctl: a pointer to the pinctrl device structure
  * @regions: reserved memory regions with start/end pin
  * @info: structure containing information about the pin
- * @gpio_configs: Saved configurations for GPIO pins
- * @gpiop_configs_lock: lock for the `gpio_configs` list
- * @s32_pinctrl_context: Configuration saved over system sleep
+ * @gpio_configs: saved configurations for GPIO pins
+ * @gpio_configs_lock: lock for the `gpio_configs` list
+ * @saved_context: configuration saved over system sleep
  */
 struct s32_pinctrl {
 	struct device *dev;
@@ -123,13 +131,13 @@ s32_get_region(struct pinctrl_dev *pctldev, unsigned int pin)
 	return NULL;
 }
 
-static inline int s32_check_pin(struct pinctrl_dev *pctldev,
-				unsigned int pin)
+static int s32_check_pin(struct pinctrl_dev *pctldev,
+			 unsigned int pin)
 {
 	return s32_get_region(pctldev, pin) ? 0 : -EINVAL;
 }
 
-static inline int s32_regmap_read(struct pinctrl_dev *pctldev,
+static int s32_regmap_read(struct pinctrl_dev *pctldev,
 			   unsigned int pin, unsigned int *val)
 {
 	struct s32_pinctrl_mem_region *region;
@@ -145,7 +153,7 @@ static inline int s32_regmap_read(struct pinctrl_dev *pctldev,
 	return regmap_read(region->map, offset, val);
 }
 
-static inline int s32_regmap_write(struct pinctrl_dev *pctldev,
+static int s32_regmap_write(struct pinctrl_dev *pctldev,
 			    unsigned int pin,
 			    unsigned int val)
 {
@@ -163,7 +171,7 @@ static inline int s32_regmap_write(struct pinctrl_dev *pctldev,
 
 }
 
-static inline int s32_regmap_update(struct pinctrl_dev *pctldev, unsigned int pin,
+static int s32_regmap_update(struct pinctrl_dev *pctldev, unsigned int pin,
 			     unsigned int mask, unsigned int val)
 {
 	struct s32_pinctrl_mem_region *region;
@@ -236,10 +244,10 @@ static int s32_dt_group_node_to_map(struct pinctrl_dev *pctldev,
 	}
 
 	ret = pinconf_generic_parse_dt_config(np, pctldev, &cfgs, &n_cfgs);
-	if (ret) {
-		dev_err(dev, "%pOF: could not parse node property\n", np);
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "%pOF: could not parse node property\n",
+				     np);
 
 	if (n_cfgs)
 		reserve++;
@@ -321,7 +329,7 @@ static int s32_pmx_set(struct pinctrl_dev *pctldev, unsigned int selector,
 	/* Check beforehand so we don't have a partial config. */
 	for (i = 0; i < grp->data.npins; i++) {
 		if (s32_check_pin(pctldev, grp->data.pins[i]) != 0) {
-			dev_err(info->dev, "invalid pin: %u in group: %u\n",
+			dev_err(info->dev, "Invalid pin: %u in group: %u\n",
 				grp->data.pins[i], group);
 			return -EINVAL;
 		}
@@ -476,8 +484,8 @@ static int s32_get_slew_regval(int arg)
 	return -EINVAL;
 }
 
-static inline void s32_pin_set_pull(enum pin_config_param param,
-				   unsigned int *mask, unsigned int *config)
+static void s32_pin_set_pull(enum pin_config_param param,
+			     unsigned int *mask, unsigned int *config)
 {
 	switch (param) {
 	case PIN_CONFIG_BIAS_DISABLE:
@@ -763,15 +771,15 @@ static int s32_pinctrl_parse_groups(struct device_node *np,
 	grp->data.name = np->name;
 
 	npins = of_property_count_elems_of_size(np, "pinmux", sizeof(u32));
-	if (npins < 0) {
-		dev_err(dev, "Failed to read 'pinmux' property in node %s.\n",
-			grp->data.name);
-		return -EINVAL;
-	}
-	if (!npins) {
-		dev_err(dev, "The group %s has no pins.\n", grp->data.name);
-		return -EINVAL;
-	}
+	if (npins < 0)
+		return dev_err_probe(dev, -EINVAL,
+				     "Failed to read 'pinmux' in node %s\n",
+				     grp->data.name);
+
+	if (!npins)
+		return dev_err_probe(dev, -EINVAL,
+				     "The group %s has no pins\n",
+				     grp->data.name);
 
 	grp->data.npins = npins;
 
@@ -812,10 +820,9 @@ static int s32_pinctrl_parse_functions(struct device_node *np,
 	/* Initialise function */
 	func->name = np->name;
 	func->ngroups = of_get_child_count(np);
-	if (func->ngroups == 0) {
-		dev_err(info->dev, "no groups defined in %pOF\n", np);
-		return -EINVAL;
-	}
+	if (func->ngroups == 0)
+		return dev_err_probe(info->dev, -EINVAL,
+				     "No groups defined in %pOF\n", np);
 
 	groups = devm_kcalloc(info->dev, func->ngroups,
 				    sizeof(*func->groups), GFP_KERNEL);
@@ -886,10 +893,9 @@ static int s32_pinctrl_probe_dt(struct platform_device *pdev,
 	}
 
 	nfuncs = of_get_child_count(np);
-	if (nfuncs <= 0) {
-		dev_err(&pdev->dev, "no functions defined\n");
-		return -EINVAL;
-	}
+	if (nfuncs <= 0)
+		return dev_err_probe(&pdev->dev, -EINVAL,
+				     "No functions defined\n");
 
 	info->nfunctions = nfuncs;
 	info->functions = devm_kcalloc(&pdev->dev, nfuncs,
@@ -919,18 +925,17 @@ static int s32_pinctrl_probe_dt(struct platform_device *pdev,
 int s32_pinctrl_probe(struct platform_device *pdev,
 		      const struct s32_pinctrl_soc_data *soc_data)
 {
-	struct s32_pinctrl *ipctl;
-	int ret;
-	struct pinctrl_desc *s32_pinctrl_desc;
-	struct s32_pinctrl_soc_info *info;
 #ifdef CONFIG_PM_SLEEP
 	struct s32_pinctrl_context *saved_context;
 #endif
+	struct pinctrl_desc *s32_pinctrl_desc;
+	struct s32_pinctrl_soc_info *info;
+	struct s32_pinctrl *ipctl;
+	int ret;
 
-	if (!soc_data || !soc_data->pins || !soc_data->npins) {
-		dev_err(&pdev->dev, "wrong pinctrl info\n");
-		return -EINVAL;
-	}
+	if (!soc_data || !soc_data->pins || !soc_data->npins)
+		return dev_err_probe(&pdev->dev, -EINVAL,
+				     "Wrong pinctrl info\n");
 
 	info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
 	if (!info)
@@ -965,16 +970,15 @@ int s32_pinctrl_probe(struct platform_device *pdev,
 	s32_pinctrl_desc->owner = THIS_MODULE;
 
 	ret = s32_pinctrl_probe_dt(pdev, ipctl);
-	if (ret) {
-		dev_err(&pdev->dev, "fail to probe dt properties\n");
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret,
+				     "Fail to probe dt properties\n");
 
 	ipctl->pctl = devm_pinctrl_register(&pdev->dev, s32_pinctrl_desc,
 					    ipctl);
 	if (IS_ERR(ipctl->pctl))
 		return dev_err_probe(&pdev->dev, PTR_ERR(ipctl->pctl),
-				     "could not register s32 pinctrl driver\n");
+				     "Could not register s32 pinctrl driver\n");
 
 #ifdef CONFIG_PM_SLEEP
 	saved_context = &ipctl->saved_context;
-- 
2.50.1

Re: [PATCH v8 04/10] pinctrl: s32cc: use dev_err_probe() and improve error messages

[Bartosz Golaszewski]

On Tue, 20 Jan 2026 12:59:16 +0100, Khristine Andreea Barbulescu
<khristineandreea.barbulescu@oss.nxp.com> said:
> From: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
>
> Change dev_err&return statements into dev_err_probe throughout the driver
> on the probing path. Moreover, add/fix some comments and print
> statements.
>
> Signed-off-by: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
> ---
>  drivers/pinctrl/nxp/pinctrl-s32cc.c | 106 +++++++++++++++-------------
>  1 file changed, 55 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/pinctrl/nxp/pinctrl-s32cc.c b/drivers/pinctrl/nxp/pinctrl-s32cc.c
> index 35511f83d056..a98f8e7c2768 100644
> --- a/drivers/pinctrl/nxp/pinctrl-s32cc.c
> +++ b/drivers/pinctrl/nxp/pinctrl-s32cc.c
> @@ -2,7 +2,7 @@
>  /*
>   * Core driver for the S32 CC (Common Chassis) pin controller
>   *
> - * Copyright 2017-2022,2024 NXP
> + * Copyright 2017-2022,2024-2025 NXP
>   * Copyright (C) 2022 SUSE LLC
>   * Copyright 2015-2016 Freescale Semiconductor, Inc.
>   */
> @@ -60,14 +60,20 @@ static u32 get_pin_func(u32 pinmux)
>  	return pinmux & GENMASK(3, 0);
>  }
>
> +/**
> + * struct s32_pinctrl_mem_region - memory region for a set of SIUL2 registers
> + * @map: regmap used for this range
> + * @pin_range: the pins controlled by these registers
> + * @name: name of the current range
> + */

This should be a separate commit, it has nothing to do with dev_err_probe().

Same elsewhere, just add a commit fixing kerneldocs in this driver.

>  struct s32_pinctrl_mem_region {
>  	struct regmap *map;
>  	const struct s32_pin_range *pin_range;
>  	char name[8];
>  };
>
> -/*
> - * Holds pin configuration for GPIO's.
> +/**
> + * struct gpio_pin_config - holds pin configuration for GPIO's
>   * @pin_id: Pin ID for this GPIO
>   * @config: Pin settings
>   * @list: Linked list entry for each gpio pin
> @@ -78,21 +84,23 @@ struct gpio_pin_config {
>  	struct list_head list;
>  };
>
> -/*
> - * Pad config save/restore for power suspend/resume.
> +/**
> + * struct s32_pinctrl_context - pad config save/restore for suspend/resume
> + * @pads: saved values for the pards
>   */
>  struct s32_pinctrl_context {
>  	unsigned int *pads;
>  };
>
> -/*
> +/**
> + * struct s32_pinctrl - private driver data
>   * @dev: a pointer back to containing device
>   * @pctl: a pointer to the pinctrl device structure
>   * @regions: reserved memory regions with start/end pin
>   * @info: structure containing information about the pin
> - * @gpio_configs: Saved configurations for GPIO pins
> - * @gpiop_configs_lock: lock for the `gpio_configs` list
> - * @s32_pinctrl_context: Configuration saved over system sleep
> + * @gpio_configs: saved configurations for GPIO pins
> + * @gpio_configs_lock: lock for the `gpio_configs` list
> + * @saved_context: configuration saved over system sleep
>   */
>  struct s32_pinctrl {
>  	struct device *dev;
> @@ -123,13 +131,13 @@ s32_get_region(struct pinctrl_dev *pctldev, unsigned int pin)
>  	return NULL;
>  }
>
> -static inline int s32_check_pin(struct pinctrl_dev *pctldev,
> -				unsigned int pin)
> +static int s32_check_pin(struct pinctrl_dev *pctldev,
> +			 unsigned int pin)

Likewise, this merits a separate patch.

>  {
>  	return s32_get_region(pctldev, pin) ? 0 : -EINVAL;
>  }
>
> -static inline int s32_regmap_read(struct pinctrl_dev *pctldev,
> +static int s32_regmap_read(struct pinctrl_dev *pctldev,
>  			   unsigned int pin, unsigned int *val)
>  {
>  	struct s32_pinctrl_mem_region *region;
> @@ -145,7 +153,7 @@ static inline int s32_regmap_read(struct pinctrl_dev *pctldev,
>  	return regmap_read(region->map, offset, val);
>  }
>
> -static inline int s32_regmap_write(struct pinctrl_dev *pctldev,
> +static int s32_regmap_write(struct pinctrl_dev *pctldev,
>  			    unsigned int pin,
>  			    unsigned int val)
>  {
> @@ -163,7 +171,7 @@ static inline int s32_regmap_write(struct pinctrl_dev *pctldev,
>
>  }
>
> -static inline int s32_regmap_update(struct pinctrl_dev *pctldev, unsigned int pin,
> +static int s32_regmap_update(struct pinctrl_dev *pctldev, unsigned int pin,
>  			     unsigned int mask, unsigned int val)
>  {
>  	struct s32_pinctrl_mem_region *region;
> @@ -236,10 +244,10 @@ static int s32_dt_group_node_to_map(struct pinctrl_dev *pctldev,
>  	}
>
>  	ret = pinconf_generic_parse_dt_config(np, pctldev, &cfgs, &n_cfgs);
> -	if (ret) {
> -		dev_err(dev, "%pOF: could not parse node property\n", np);
> -		return ret;
> -	}
> +	if (ret)
> +		return dev_err_probe(dev, ret,
> +				     "%pOF: could not parse node property\n",
> +				     np);
>
>  	if (n_cfgs)
>  		reserve++;
> @@ -321,7 +329,7 @@ static int s32_pmx_set(struct pinctrl_dev *pctldev, unsigned int selector,
>  	/* Check beforehand so we don't have a partial config. */
>  	for (i = 0; i < grp->data.npins; i++) {
>  		if (s32_check_pin(pctldev, grp->data.pins[i]) != 0) {
> -			dev_err(info->dev, "invalid pin: %u in group: %u\n",
> +			dev_err(info->dev, "Invalid pin: %u in group: %u\n",
>  				grp->data.pins[i], group);
>  			return -EINVAL;
>  		}
> @@ -476,8 +484,8 @@ static int s32_get_slew_regval(int arg)
>  	return -EINVAL;
>  }
>
> -static inline void s32_pin_set_pull(enum pin_config_param param,
> -				   unsigned int *mask, unsigned int *config)
> +static void s32_pin_set_pull(enum pin_config_param param,
> +			     unsigned int *mask, unsigned int *config)
>  {
>  	switch (param) {
>  	case PIN_CONFIG_BIAS_DISABLE:
> @@ -763,15 +771,15 @@ static int s32_pinctrl_parse_groups(struct device_node *np,
>  	grp->data.name = np->name;
>
>  	npins = of_property_count_elems_of_size(np, "pinmux", sizeof(u32));
> -	if (npins < 0) {
> -		dev_err(dev, "Failed to read 'pinmux' property in node %s.\n",
> -			grp->data.name);
> -		return -EINVAL;
> -	}
> -	if (!npins) {
> -		dev_err(dev, "The group %s has no pins.\n", grp->data.name);
> -		return -EINVAL;
> -	}
> +	if (npins < 0)
> +		return dev_err_probe(dev, -EINVAL,
> +				     "Failed to read 'pinmux' in node %s\n",
> +				     grp->data.name);
> +
> +	if (!npins)
> +		return dev_err_probe(dev, -EINVAL,
> +				     "The group %s has no pins\n",
> +				     grp->data.name);
>
>  	grp->data.npins = npins;
>
> @@ -812,10 +820,9 @@ static int s32_pinctrl_parse_functions(struct device_node *np,
>  	/* Initialise function */
>  	func->name = np->name;
>  	func->ngroups = of_get_child_count(np);
> -	if (func->ngroups == 0) {
> -		dev_err(info->dev, "no groups defined in %pOF\n", np);
> -		return -EINVAL;
> -	}
> +	if (func->ngroups == 0)
> +		return dev_err_probe(info->dev, -EINVAL,
> +				     "No groups defined in %pOF\n", np);
>
>  	groups = devm_kcalloc(info->dev, func->ngroups,
>  				    sizeof(*func->groups), GFP_KERNEL);
> @@ -886,10 +893,9 @@ static int s32_pinctrl_probe_dt(struct platform_device *pdev,
>  	}
>
>  	nfuncs = of_get_child_count(np);
> -	if (nfuncs <= 0) {
> -		dev_err(&pdev->dev, "no functions defined\n");
> -		return -EINVAL;
> -	}
> +	if (nfuncs <= 0)
> +		return dev_err_probe(&pdev->dev, -EINVAL,
> +				     "No functions defined\n");
>
>  	info->nfunctions = nfuncs;
>  	info->functions = devm_kcalloc(&pdev->dev, nfuncs,
> @@ -919,18 +925,17 @@ static int s32_pinctrl_probe_dt(struct platform_device *pdev,
>  int s32_pinctrl_probe(struct platform_device *pdev,
>  		      const struct s32_pinctrl_soc_data *soc_data)
>  {
> -	struct s32_pinctrl *ipctl;
> -	int ret;
> -	struct pinctrl_desc *s32_pinctrl_desc;
> -	struct s32_pinctrl_soc_info *info;
>  #ifdef CONFIG_PM_SLEEP
>  	struct s32_pinctrl_context *saved_context;
>  #endif
> +	struct pinctrl_desc *s32_pinctrl_desc;
> +	struct s32_pinctrl_soc_info *info;
> +	struct s32_pinctrl *ipctl;
> +	int ret;
>
> -	if (!soc_data || !soc_data->pins || !soc_data->npins) {
> -		dev_err(&pdev->dev, "wrong pinctrl info\n");
> -		return -EINVAL;
> -	}
> +	if (!soc_data || !soc_data->pins || !soc_data->npins)
> +		return dev_err_probe(&pdev->dev, -EINVAL,
> +				     "Wrong pinctrl info\n");
>
>  	info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
>  	if (!info)
> @@ -965,16 +970,15 @@ int s32_pinctrl_probe(struct platform_device *pdev,
>  	s32_pinctrl_desc->owner = THIS_MODULE;
>
>  	ret = s32_pinctrl_probe_dt(pdev, ipctl);
> -	if (ret) {
> -		dev_err(&pdev->dev, "fail to probe dt properties\n");
> -		return ret;
> -	}
> +	if (ret)
> +		return dev_err_probe(&pdev->dev, ret,
> +				     "Fail to probe dt properties\n");
>
>  	ipctl->pctl = devm_pinctrl_register(&pdev->dev, s32_pinctrl_desc,
>  					    ipctl);
>  	if (IS_ERR(ipctl->pctl))
>  		return dev_err_probe(&pdev->dev, PTR_ERR(ipctl->pctl),
> -				     "could not register s32 pinctrl driver\n");
> +				     "Could not register s32 pinctrl driver\n");
>
>  #ifdef CONFIG_PM_SLEEP
>  	saved_context = &ipctl->saved_context;
> --
> 2.50.1
>
>

Bart

[PATCH v8 05/10] pinctrl: s32cc: change to "devm_pinctrl_register_and_init"

[Khristine Andreea Barbulescu]

From: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>

Switch from "devm_pinctrl_register" to "devm_pinctrl_register_and_init"
and "pinctrl_enable" since this is the recommended way.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
---
 drivers/pinctrl/nxp/pinctrl-s32cc.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/pinctrl/nxp/pinctrl-s32cc.c b/drivers/pinctrl/nxp/pinctrl-s32cc.c
index a98f8e7c2768..4767916dbcab 100644
--- a/drivers/pinctrl/nxp/pinctrl-s32cc.c
+++ b/drivers/pinctrl/nxp/pinctrl-s32cc.c
@@ -974,10 +974,10 @@ int s32_pinctrl_probe(struct platform_device *pdev,
 		return dev_err_probe(&pdev->dev, ret,
 				     "Fail to probe dt properties\n");
 
-	ipctl->pctl = devm_pinctrl_register(&pdev->dev, s32_pinctrl_desc,
-					    ipctl);
-	if (IS_ERR(ipctl->pctl))
-		return dev_err_probe(&pdev->dev, PTR_ERR(ipctl->pctl),
+	ret = devm_pinctrl_register_and_init(&pdev->dev, s32_pinctrl_desc,
+					     ipctl, &ipctl->pctl);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret,
 				     "Could not register s32 pinctrl driver\n");
 
 #ifdef CONFIG_PM_SLEEP
@@ -990,7 +990,12 @@ int s32_pinctrl_probe(struct platform_device *pdev,
 		return -ENOMEM;
 #endif
 
-	dev_info(&pdev->dev, "initialized s32 pinctrl driver\n");
+	ret = pinctrl_enable(ipctl->pctl);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret,
+				     "Failed to enable pinctrl\n");
+
+	dev_info(&pdev->dev, "Initialized S32 pinctrl driver\n");
 
 	return 0;
 }
-- 
2.50.1

Re: [PATCH v8 05/10] pinctrl: s32cc: change to "devm_pinctrl_register_and_init"

[Bartosz Golaszewski]

On Tue, 20 Jan 2026 12:59:17 +0100, Khristine Andreea Barbulescu
<khristineandreea.barbulescu@oss.nxp.com> said:
> From: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
>
> Switch from "devm_pinctrl_register" to "devm_pinctrl_register_and_init"
> and "pinctrl_enable" since this is the recommended way.
>
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
> ---
>  drivers/pinctrl/nxp/pinctrl-s32cc.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/pinctrl/nxp/pinctrl-s32cc.c b/drivers/pinctrl/nxp/pinctrl-s32cc.c
> index a98f8e7c2768..4767916dbcab 100644
> --- a/drivers/pinctrl/nxp/pinctrl-s32cc.c
> +++ b/drivers/pinctrl/nxp/pinctrl-s32cc.c
> @@ -974,10 +974,10 @@ int s32_pinctrl_probe(struct platform_device *pdev,
>  		return dev_err_probe(&pdev->dev, ret,
>  				     "Fail to probe dt properties\n");
>
> -	ipctl->pctl = devm_pinctrl_register(&pdev->dev, s32_pinctrl_desc,
> -					    ipctl);
> -	if (IS_ERR(ipctl->pctl))
> -		return dev_err_probe(&pdev->dev, PTR_ERR(ipctl->pctl),
> +	ret = devm_pinctrl_register_and_init(&pdev->dev, s32_pinctrl_desc,
> +					     ipctl, &ipctl->pctl);
> +	if (ret)
> +		return dev_err_probe(&pdev->dev, ret,
>  				     "Could not register s32 pinctrl driver\n");
>
>  #ifdef CONFIG_PM_SLEEP
> @@ -990,7 +990,12 @@ int s32_pinctrl_probe(struct platform_device *pdev,
>  		return -ENOMEM;
>  #endif
>
> -	dev_info(&pdev->dev, "initialized s32 pinctrl driver\n");
> +	ret = pinctrl_enable(ipctl->pctl);
> +	if (ret)
> +		return dev_err_probe(&pdev->dev, ret,
> +				     "Failed to enable pinctrl\n");
> +
> +	dev_info(&pdev->dev, "Initialized S32 pinctrl driver\n");
>
>  	return 0;
>  }
> --
> 2.50.1
>
>

Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

[PATCH v8 06/10] pinctrl: s32g2: change the driver to also be probed as an MFD cell

[Khristine Andreea Barbulescu]

From: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>

The old pinctrl bindings for SIUL2 are deprecated by a previous commit.
The new bindings for the SIUL2 represent it as an MFD device:
- one cell for combined pinctrl&GPIO
- two cella acting as syscon providers for SoC registers access

This commit allows the existing driver to also be probed as an MFD cell.
The changes only impact the way the driver initializes the regmaps for
accessing MSCR and IMCR registers.

Signed-off-by: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
Signed-off-by: Khristine Andreea Barbulescu <khristineandreea.barbulescu@oss.nxp.com>
---
 drivers/pinctrl/nxp/pinctrl-s32.h   |   4 +-
 drivers/pinctrl/nxp/pinctrl-s32cc.c | 114 ++++++++++++++++++++++------
 drivers/pinctrl/nxp/pinctrl-s32g2.c |  32 ++++++--
 3 files changed, 118 insertions(+), 32 deletions(-)

diff --git a/drivers/pinctrl/nxp/pinctrl-s32.h b/drivers/pinctrl/nxp/pinctrl-s32.h
index add3c77ddfed..6ce7981208c7 100644
--- a/drivers/pinctrl/nxp/pinctrl-s32.h
+++ b/drivers/pinctrl/nxp/pinctrl-s32.h
@@ -2,7 +2,7 @@
  *
  * S32 pinmux core definitions
  *
- * Copyright 2016-2020, 2022 NXP
+ * Copyright 2016-2020, 2022, 2025 NXP
  * Copyright (C) 2022 SUSE LLC
  * Copyright 2015-2016 Freescale Semiconductor, Inc.
  * Copyright (C) 2012 Linaro Ltd.
@@ -28,6 +28,7 @@ struct s32_pin_group {
  * struct s32_pin_range - pin ID range for each memory region.
  * @start: start pin ID
  * @end: end pin ID
+ * @legacy: legacy standalone pinctrl driver or MFD cell
  */
 struct s32_pin_range {
 	unsigned int start;
@@ -39,6 +40,7 @@ struct s32_pinctrl_soc_data {
 	unsigned int npins;
 	const struct s32_pin_range *mem_pin_ranges;
 	unsigned int mem_regions;
+	bool legacy;
 };
 
 struct s32_pinctrl_soc_info {
diff --git a/drivers/pinctrl/nxp/pinctrl-s32cc.c b/drivers/pinctrl/nxp/pinctrl-s32cc.c
index 4767916dbcab..cdd3a1cd4fe5 100644
--- a/drivers/pinctrl/nxp/pinctrl-s32cc.c
+++ b/drivers/pinctrl/nxp/pinctrl-s32cc.c
@@ -12,6 +12,7 @@
 #include <linux/gpio/driver.h>
 #include <linux/init.h>
 #include <linux/io.h>
+#include <linux/mfd/nxp-siul2.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
@@ -101,6 +102,8 @@ struct s32_pinctrl_context {
  * @gpio_configs: saved configurations for GPIO pins
  * @gpio_configs_lock: lock for the `gpio_configs` list
  * @saved_context: configuration saved over system sleep
+ * @legacy: true if the old pinctrl bindings are in use
+ *	    instead of the newer MFD based ones
  */
 struct s32_pinctrl {
 	struct device *dev;
@@ -112,6 +115,7 @@ struct s32_pinctrl {
 #ifdef CONFIG_PM_SLEEP
 	struct s32_pinctrl_context saved_context;
 #endif
+	bool legacy;
 };
 
 static struct s32_pinctrl_mem_region *
@@ -131,6 +135,19 @@ s32_get_region(struct pinctrl_dev *pctldev, unsigned int pin)
 	return NULL;
 }
 
+static struct device *s32_get_dev(struct s32_pinctrl *ipctl)
+{
+	if (ipctl->legacy)
+		return ipctl->dev;
+
+	return ipctl->dev->parent;
+}
+
+static struct device_node *s32_get_np(struct s32_pinctrl *ipctl)
+{
+	return s32_get_dev(ipctl)->of_node;
+}
+
 static int s32_check_pin(struct pinctrl_dev *pctldev,
 			 unsigned int pin)
 {
@@ -231,7 +248,7 @@ static int s32_dt_group_node_to_map(struct pinctrl_dev *pctldev,
 				    const char *func_name)
 {
 	struct s32_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
-	struct device *dev = ipctl->dev;
+	struct device *dev = s32_get_dev(ipctl);
 	unsigned long *cfgs = NULL;
 	unsigned int n_cfgs, reserve = 1;
 	int n_pins, ret;
@@ -804,8 +821,8 @@ static int s32_pinctrl_parse_groups(struct device_node *np,
 }
 
 static int s32_pinctrl_parse_functions(struct device_node *np,
-					struct s32_pinctrl_soc_info *info,
-					u32 index)
+				       struct s32_pinctrl_soc_info *info,
+				       u32 index)
 {
 	struct pinfunction *func;
 	struct s32_pin_group *grp;
@@ -843,31 +860,21 @@ static int s32_pinctrl_parse_functions(struct device_node *np,
 	return 0;
 }
 
-static int s32_pinctrl_probe_dt(struct platform_device *pdev,
-				struct s32_pinctrl *ipctl)
+static int legacy_s32_pinctrl_regmap_init(struct platform_device *pdev,
+					  struct s32_pinctrl *ipctl)
 {
 	struct s32_pinctrl_soc_info *info = ipctl->info;
-	struct device_node *np = pdev->dev.of_node;
+	unsigned int mem_regions;
 	struct resource *res;
 	struct regmap *map;
 	void __iomem *base;
-	unsigned int mem_regions = info->soc_data->mem_regions;
-	int ret;
-	u32 nfuncs = 0;
 	u32 i = 0;
 
-	if (!np)
-		return -ENODEV;
-
-	if (mem_regions == 0 || mem_regions >= 10000) {
-		dev_err(&pdev->dev, "mem_regions is invalid: %u\n", mem_regions);
-		return -EINVAL;
-	}
-
-	ipctl->regions = devm_kcalloc(&pdev->dev, mem_regions,
-				      sizeof(*ipctl->regions), GFP_KERNEL);
-	if (!ipctl->regions)
-		return -ENOMEM;
+	mem_regions = info->soc_data->mem_regions;
+	if (mem_regions == 0 || mem_regions >= 10000)
+		return dev_err_probe(&pdev->dev, -EINVAL,
+				     "mem_regions is invalid: %u\n",
+				     mem_regions);
 
 	for (i = 0; i < mem_regions; i++) {
 		base = devm_platform_get_and_ioremap_resource(pdev, i, &res);
@@ -882,7 +889,7 @@ static int s32_pinctrl_probe_dt(struct platform_device *pdev,
 						 s32_regmap_config.reg_stride;
 
 		map = devm_regmap_init_mmio(&pdev->dev, base,
-						&s32_regmap_config);
+					    &s32_regmap_config);
 		if (IS_ERR(map)) {
 			dev_err(&pdev->dev, "Failed to init regmap[%u]\n", i);
 			return PTR_ERR(map);
@@ -892,7 +899,49 @@ static int s32_pinctrl_probe_dt(struct platform_device *pdev,
 		ipctl->regions[i].pin_range = &info->soc_data->mem_pin_ranges[i];
 	}
 
-	nfuncs = of_get_child_count(np);
+	return 0;
+}
+
+static int s32_pinctrl_mfd_regmap_init(struct platform_device *pdev,
+				       struct s32_pinctrl *ipctl)
+
+{
+	struct nxp_siul2_mfd *mfd = dev_get_drvdata(pdev->dev.parent);
+	struct s32_pinctrl_soc_info *info = ipctl->info;
+	unsigned int mem_regions;
+	u8 regmap_type;
+	u32 i = 0, j;
+
+	/* One MSCR and one IMCR region per SIUL2 module. */
+	mem_regions = info->soc_data->mem_regions;
+	if (mem_regions != mfd->num_siul2 * 2)
+		return dev_err_probe(&pdev->dev, -EINVAL,
+				     "mem_regions is invalid: %u\n",
+				     mem_regions);
+
+	for (i = 0; i < mem_regions; i++) {
+		regmap_type = i < mem_regions / 2 ? SIUL2_MSCR : SIUL2_IMCR;
+		j = i % mfd->num_siul2;
+		ipctl->regions[i].map = mfd->siul2[j].regmaps[regmap_type];
+		ipctl->regions[i].pin_range = &info->soc_data->mem_pin_ranges[i];
+	}
+
+	return 0;
+}
+
+static int s32_pinctrl_probe_dt(struct platform_device *pdev,
+				struct s32_pinctrl *ipctl)
+{
+	struct s32_pinctrl_soc_info *info = ipctl->info;
+	struct device_node *np = s32_get_np(ipctl);
+	u32 nfuncs = 0, i = 0;
+	int ret;
+
+	if (!np)
+		return -ENODEV;
+
+	for_each_child_of_node_scoped(np, child)
+		++nfuncs;
 	if (nfuncs <= 0)
 		return dev_err_probe(&pdev->dev, -EINVAL,
 				     "No functions defined\n");
@@ -912,7 +961,6 @@ static int s32_pinctrl_probe_dt(struct platform_device *pdev,
 	if (!info->groups)
 		return -ENOMEM;
 
-	i = 0;
 	for_each_child_of_node_scoped(np, child) {
 		ret = s32_pinctrl_parse_functions(child, info, i++);
 		if (ret)
@@ -969,12 +1017,28 @@ int s32_pinctrl_probe(struct platform_device *pdev,
 	s32_pinctrl_desc->confops = &s32_pinconf_ops;
 	s32_pinctrl_desc->owner = THIS_MODULE;
 
+	ipctl->regions = devm_kcalloc(&pdev->dev, soc_data->mem_regions,
+				      sizeof(*ipctl->regions), GFP_KERNEL);
+	if (!ipctl->regions)
+		return -ENOMEM;
+
+	ipctl->legacy = soc_data->legacy;
+	if (soc_data->legacy)
+		ret = legacy_s32_pinctrl_regmap_init(pdev, ipctl);
+	else
+		ret = s32_pinctrl_mfd_regmap_init(pdev, ipctl);
+
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret,
+				     "Failed to init driver regmap!\n");
+
 	ret = s32_pinctrl_probe_dt(pdev, ipctl);
 	if (ret)
 		return dev_err_probe(&pdev->dev, ret,
 				     "Fail to probe dt properties\n");
 
-	ret = devm_pinctrl_register_and_init(&pdev->dev, s32_pinctrl_desc,
+	ret = devm_pinctrl_register_and_init(s32_get_dev(ipctl),
+					     s32_pinctrl_desc,
 					     ipctl, &ipctl->pctl);
 	if (ret)
 		return dev_err_probe(&pdev->dev, ret,
diff --git a/drivers/pinctrl/nxp/pinctrl-s32g2.c b/drivers/pinctrl/nxp/pinctrl-s32g2.c
index c49d28793b69..2d56ffb1a109 100644
--- a/drivers/pinctrl/nxp/pinctrl-s32g2.c
+++ b/drivers/pinctrl/nxp/pinctrl-s32g2.c
@@ -3,7 +3,7 @@
  * NXP S32G pinctrl driver
  *
  * Copyright 2015-2016 Freescale Semiconductor, Inc.
- * Copyright 2017-2018, 2020-2022 NXP
+ * Copyright 2017-2018, 2020-2022, 2024-2025 NXP
  * Copyright (C) 2022 SUSE LLC
  */
 
@@ -762,7 +762,7 @@ static const struct pinctrl_pin_desc s32_pinctrl_pads_siul2[] = {
 	S32_PINCTRL_PIN(S32G_IMCR_SIUL_EIRQ31),
 };
 
-static const struct s32_pin_range s32_pin_ranges_siul2[] = {
+static const struct s32_pin_range legacy_s32_pin_ranges_siul2[] = {
 	/* MSCR pin ID ranges */
 	S32_PIN_RANGE(0, 101),
 	S32_PIN_RANGE(112, 122),
@@ -773,27 +773,47 @@ static const struct s32_pin_range s32_pin_ranges_siul2[] = {
 	S32_PIN_RANGE(942, 1007),
 };
 
-static const struct s32_pinctrl_soc_data s32_pinctrl_data = {
+static const struct s32_pinctrl_soc_data legacy_s32_pinctrl_data = {
 	.pins = s32_pinctrl_pads_siul2,
 	.npins = ARRAY_SIZE(s32_pinctrl_pads_siul2),
-	.mem_pin_ranges = s32_pin_ranges_siul2,
-	.mem_regions = ARRAY_SIZE(s32_pin_ranges_siul2),
+	.mem_pin_ranges = legacy_s32_pin_ranges_siul2,
+	.mem_regions = ARRAY_SIZE(legacy_s32_pin_ranges_siul2),
+	.legacy = true,
 };
 
 static const struct of_device_id s32_pinctrl_of_match[] = {
 	{
 		.compatible = "nxp,s32g2-siul2-pinctrl",
-		.data = &s32_pinctrl_data,
+		.data = &legacy_s32_pinctrl_data,
 	},
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, s32_pinctrl_of_match);
 
+static const struct s32_pin_range s32_pin_ranges_siul2[] = {
+	/* MSCR pin ID ranges */
+	S32_PIN_RANGE(0, 101),
+	S32_PIN_RANGE(112, 190),
+	/* IMCR pin ID ranges */
+	S32_PIN_RANGE(512, 595),
+	S32_PIN_RANGE(631, 1007),
+};
+
+static const struct s32_pinctrl_soc_data s32_pinctrl_data = {
+	.pins = s32_pinctrl_pads_siul2,
+	.npins = ARRAY_SIZE(s32_pinctrl_pads_siul2),
+	.mem_pin_ranges = s32_pin_ranges_siul2,
+	.mem_regions = ARRAY_SIZE(s32_pin_ranges_siul2),
+	.legacy = false,
+};
+
 static int s32g_pinctrl_probe(struct platform_device *pdev)
 {
 	const struct s32_pinctrl_soc_data *soc_data;
 
 	soc_data = of_device_get_match_data(&pdev->dev);
+	if (!soc_data)
+		soc_data = &s32_pinctrl_data;
 
 	return s32_pinctrl_probe(pdev, soc_data);
 }
-- 
2.50.1

Re: [PATCH v8 06/10] pinctrl: s32g2: change the driver to also be probed as an MFD cell

[Bartosz Golaszewski]

On Tue, 20 Jan 2026 12:59:18 +0100, Khristine Andreea Barbulescu
<khristineandreea.barbulescu@oss.nxp.com> said:
> From: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
>
> The old pinctrl bindings for SIUL2 are deprecated by a previous commit.
> The new bindings for the SIUL2 represent it as an MFD device:
> - one cell for combined pinctrl&GPIO
> - two cella acting as syscon providers for SoC registers access
>
> This commit allows the existing driver to also be probed as an MFD cell.
> The changes only impact the way the driver initializes the regmaps for
> accessing MSCR and IMCR registers.
>
> Signed-off-by: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
> Signed-off-by: Khristine Andreea Barbulescu <khristineandreea.barbulescu@oss.nxp.com>
> ---

Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

Re: [PATCH v8 06/10] pinctrl: s32g2: change the driver to also be probed as an MFD cell

[Vincent Guittot]

On Tue, 20 Jan 2026 at 12:59, Khristine Andreea Barbulescu
<khristineandreea.barbulescu@oss.nxp.com> wrote:
>
> From: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
>
> The old pinctrl bindings for SIUL2 are deprecated by a previous commit.
> The new bindings for the SIUL2 represent it as an MFD device:
> - one cell for combined pinctrl&GPIO
> - two cella acting as syscon providers for SoC registers access
>
> This commit allows the existing driver to also be probed as an MFD cell.
> The changes only impact the way the driver initializes the regmaps for
> accessing MSCR and IMCR registers.
>
> Signed-off-by: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
> Signed-off-by: Khristine Andreea Barbulescu <khristineandreea.barbulescu@oss.nxp.com>

[..]

> @@ -969,12 +1017,28 @@ int s32_pinctrl_probe(struct platform_device *pdev,
>         s32_pinctrl_desc->confops = &s32_pinconf_ops;
>         s32_pinctrl_desc->owner = THIS_MODULE;
>
> +       ipctl->regions = devm_kcalloc(&pdev->dev, soc_data->mem_regions,
> +                                     sizeof(*ipctl->regions), GFP_KERNEL);
> +       if (!ipctl->regions)
> +               return -ENOMEM;
> +
> +       ipctl->legacy = soc_data->legacy;
> +       if (soc_data->legacy)
> +               ret = legacy_s32_pinctrl_regmap_init(pdev, ipctl);
> +       else
> +               ret = s32_pinctrl_mfd_regmap_init(pdev, ipctl);
> +
> +       if (ret)
> +               return dev_err_probe(&pdev->dev, ret,
> +                                    "Failed to init driver regmap!\n");
> +
>         ret = s32_pinctrl_probe_dt(pdev, ipctl);
>         if (ret)
>                 return dev_err_probe(&pdev->dev, ret,
>                                      "Fail to probe dt properties\n");
>
> -       ret = devm_pinctrl_register_and_init(&pdev->dev, s32_pinctrl_desc,
> +       ret = devm_pinctrl_register_and_init(s32_get_dev(ipctl),

you should better add a child node for your pinctrl mfd device in its
"nxp,s32g2-siul2" parent instead of registering the pinctrl device on
the parent device. This would fix phandle to gpio too



> +                                            s32_pinctrl_desc,
>                                              ipctl, &ipctl->pctl);
>         if (ret)
>                 return dev_err_probe(&pdev->dev, ret,
> diff --git a/drivers/pinctrl/nxp/pinctrl-s32g2.c b/drivers/pinctrl/nxp/pinctrl-s32g2.c
> index c49d28793b69..2d56ffb1a109 100644
> --- a/drivers/pinctrl/nxp/pinctrl-s32g2.c
> +++ b/drivers/pinctrl/nxp/pinctrl-s32g2.c
> @@ -3,7 +3,7 @@
>   * NXP S32G pinctrl driver
>   *
>   * Copyright 2015-2016 Freescale Semiconductor, Inc.
> - * Copyright 2017-2018, 2020-2022 NXP
> + * Copyright 2017-2018, 2020-2022, 2024-2025 NXP
>   * Copyright (C) 2022 SUSE LLC
>   */
>
> @@ -762,7 +762,7 @@ static const struct pinctrl_pin_desc s32_pinctrl_pads_siul2[] = {
>         S32_PINCTRL_PIN(S32G_IMCR_SIUL_EIRQ31),
>  };
>
> -static const struct s32_pin_range s32_pin_ranges_siul2[] = {
> +static const struct s32_pin_range legacy_s32_pin_ranges_siul2[] = {
>         /* MSCR pin ID ranges */
>         S32_PIN_RANGE(0, 101),
>         S32_PIN_RANGE(112, 122),
> @@ -773,27 +773,47 @@ static const struct s32_pin_range s32_pin_ranges_siul2[] = {
>         S32_PIN_RANGE(942, 1007),
>  };
>
> -static const struct s32_pinctrl_soc_data s32_pinctrl_data = {
> +static const struct s32_pinctrl_soc_data legacy_s32_pinctrl_data = {
>         .pins = s32_pinctrl_pads_siul2,
>         .npins = ARRAY_SIZE(s32_pinctrl_pads_siul2),
> -       .mem_pin_ranges = s32_pin_ranges_siul2,
> -       .mem_regions = ARRAY_SIZE(s32_pin_ranges_siul2),
> +       .mem_pin_ranges = legacy_s32_pin_ranges_siul2,
> +       .mem_regions = ARRAY_SIZE(legacy_s32_pin_ranges_siul2),
> +       .legacy = true,
>  };
>
>  static const struct of_device_id s32_pinctrl_of_match[] = {
>         {
>                 .compatible = "nxp,s32g2-siul2-pinctrl",
> -               .data = &s32_pinctrl_data,
> +               .data = &legacy_s32_pinctrl_data,
>         },
>         { /* sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(of, s32_pinctrl_of_match);
>
> +static const struct s32_pin_range s32_pin_ranges_siul2[] = {
> +       /* MSCR pin ID ranges */
> +       S32_PIN_RANGE(0, 101),
> +       S32_PIN_RANGE(112, 190),
> +       /* IMCR pin ID ranges */
> +       S32_PIN_RANGE(512, 595),
> +       S32_PIN_RANGE(631, 1007),
> +};
> +
> +static const struct s32_pinctrl_soc_data s32_pinctrl_data = {
> +       .pins = s32_pinctrl_pads_siul2,
> +       .npins = ARRAY_SIZE(s32_pinctrl_pads_siul2),
> +       .mem_pin_ranges = s32_pin_ranges_siul2,
> +       .mem_regions = ARRAY_SIZE(s32_pin_ranges_siul2),
> +       .legacy = false,
> +};
> +
>  static int s32g_pinctrl_probe(struct platform_device *pdev)
>  {
>         const struct s32_pinctrl_soc_data *soc_data;
>
>         soc_data = of_device_get_match_data(&pdev->dev);
> +       if (!soc_data)
> +               soc_data = &s32_pinctrl_data;
>
>         return s32_pinctrl_probe(pdev, soc_data);
>  }
> --
> 2.50.1
>

[PATCH v8 07/10] pinctrl: s32cc: skip syscon child nodes when parsing funcs and groups

[Khristine Andreea Barbulescu]

The SIUL2 node contains child nodes for syscon
instances (SIUL2_0 and SIUL2_1) to expose register
ranges for SoC information. These nodes are not
part of the pinctrl configuration and should not
be treated as pinctrl functions or groups.

Signed-off-by: Khristine Andreea Barbulescu <khristineandreea.barbulescu@oss.nxp.com>
---
 drivers/pinctrl/nxp/pinctrl-s32cc.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/nxp/pinctrl-s32cc.c b/drivers/pinctrl/nxp/pinctrl-s32cc.c
index cdd3a1cd4fe5..50d5a01107eb 100644
--- a/drivers/pinctrl/nxp/pinctrl-s32cc.c
+++ b/drivers/pinctrl/nxp/pinctrl-s32cc.c
@@ -2,7 +2,7 @@
 /*
  * Core driver for the S32 CC (Common Chassis) pin controller
  *
- * Copyright 2017-2022,2024-2025 NXP
+ * Copyright 2017-2022,2024-2025-2026 NXP
  * Copyright (C) 2022 SUSE LLC
  * Copyright 2015-2016 Freescale Semiconductor, Inc.
  */
@@ -832,6 +832,9 @@ static int s32_pinctrl_parse_functions(struct device_node *np,
 
 	dev_dbg(info->dev, "parse function(%u): %pOFn\n", index, np);
 
+	if (of_device_is_compatible(np, "syscon"))
+		return 0;
+
 	func = &info->functions[index];
 
 	/* Initialise function */
@@ -941,7 +944,8 @@ static int s32_pinctrl_probe_dt(struct platform_device *pdev,
 		return -ENODEV;
 
 	for_each_child_of_node_scoped(np, child)
-		++nfuncs;
+		if (!of_device_is_compatible(child, "syscon"))
+			++nfuncs;
 	if (nfuncs <= 0)
 		return dev_err_probe(&pdev->dev, -EINVAL,
 				     "No functions defined\n");
@@ -962,6 +966,9 @@ static int s32_pinctrl_probe_dt(struct platform_device *pdev,
 		return -ENOMEM;
 
 	for_each_child_of_node_scoped(np, child) {
+		if (of_device_is_compatible(child, "syscon"))
+			continue;
+
 		ret = s32_pinctrl_parse_functions(child, info, i++);
 		if (ret)
 			return ret;
-- 
2.50.1

Re: [PATCH v8 07/10] pinctrl: s32cc: skip syscon child nodes when parsing funcs and groups

[Bartosz Golaszewski]

On Tue, 20 Jan 2026 12:59:19 +0100, Khristine Andreea Barbulescu
<khristineandreea.barbulescu@oss.nxp.com> said:
> The SIUL2 node contains child nodes for syscon
> instances (SIUL2_0 and SIUL2_1) to expose register
> ranges for SoC information. These nodes are not
> part of the pinctrl configuration and should not
> be treated as pinctrl functions or groups.
>
> Signed-off-by: Khristine Andreea Barbulescu <khristineandreea.barbulescu@oss.nxp.com>
> ---
>  drivers/pinctrl/nxp/pinctrl-s32cc.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pinctrl/nxp/pinctrl-s32cc.c b/drivers/pinctrl/nxp/pinctrl-s32cc.c
> index cdd3a1cd4fe5..50d5a01107eb 100644
> --- a/drivers/pinctrl/nxp/pinctrl-s32cc.c
> +++ b/drivers/pinctrl/nxp/pinctrl-s32cc.c
> @@ -2,7 +2,7 @@
>  /*
>   * Core driver for the S32 CC (Common Chassis) pin controller
>   *
> - * Copyright 2017-2022,2024-2025 NXP
> + * Copyright 2017-2022,2024-2025-2026 NXP
>   * Copyright (C) 2022 SUSE LLC
>   * Copyright 2015-2016 Freescale Semiconductor, Inc.
>   */
> @@ -832,6 +832,9 @@ static int s32_pinctrl_parse_functions(struct device_node *np,
>
>  	dev_dbg(info->dev, "parse function(%u): %pOFn\n", index, np);
>
> +	if (of_device_is_compatible(np, "syscon"))
> +		return 0;
> +
>  	func = &info->functions[index];
>
>  	/* Initialise function */
> @@ -941,7 +944,8 @@ static int s32_pinctrl_probe_dt(struct platform_device *pdev,
>  		return -ENODEV;
>
>  	for_each_child_of_node_scoped(np, child)
> -		++nfuncs;
> +		if (!of_device_is_compatible(child, "syscon"))
> +			++nfuncs;
>  	if (nfuncs <= 0)
>  		return dev_err_probe(&pdev->dev, -EINVAL,
>  				     "No functions defined\n");
> @@ -962,6 +966,9 @@ static int s32_pinctrl_probe_dt(struct platform_device *pdev,
>  		return -ENOMEM;
>
>  	for_each_child_of_node_scoped(np, child) {
> +		if (of_device_is_compatible(child, "syscon"))
> +			continue;
> +
>  		ret = s32_pinctrl_parse_functions(child, info, i++);
>  		if (ret)
>  			return ret;
> --
> 2.50.1
>
>

Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

Re: [PATCH v8 07/10] pinctrl: s32cc: skip syscon child nodes when parsing funcs and groups

[Linus Walleij]

On Tue, Jan 20, 2026 at 1:01 PM Khristine Andreea Barbulescu
<khristineandreea.barbulescu@oss.nxp.com> wrote:

> The SIUL2 node contains child nodes for syscon
> instances (SIUL2_0 and SIUL2_1) to expose register
> ranges for SoC information. These nodes are not
> part of the pinctrl configuration and should not
> be treated as pinctrl functions or groups.
>
> Signed-off-by: Khristine Andreea Barbulescu <khristineandreea.barbulescu@oss.nxp.com>

Acked-by: Linus Walleij <linusw@kernel.org>

Yours,
Linus Walleij

[PATCH v8 08/10] pinctrl: s32cc: implement GPIO functionality

[Khristine Andreea Barbulescu]

From: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>

Add basic GPIO functionality (request, free, get, set) for the existing
pinctrl SIUL2 driver since the hardware for pinctrl&GPIO is tightly
coupled.

Also, remove pinmux_ops which are no longer needed.

Signed-off-by: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
---
 drivers/pinctrl/nxp/pinctrl-s32cc.c | 413 +++++++++++++++++++++++-----
 1 file changed, 350 insertions(+), 63 deletions(-)

diff --git a/drivers/pinctrl/nxp/pinctrl-s32cc.c b/drivers/pinctrl/nxp/pinctrl-s32cc.c
index 50d5a01107eb..3e0c48068f08 100644
--- a/drivers/pinctrl/nxp/pinctrl-s32cc.c
+++ b/drivers/pinctrl/nxp/pinctrl-s32cc.c
@@ -40,6 +40,14 @@
 #define S32_MSCR_ODE		BIT(20)
 #define S32_MSCR_OBE		BIT(21)
 
+/* PGPDOs are 16bit registers that come in big endian
+ * order if they are grouped in pairs of two.
+ *
+ * For example, the order is PGPDO1, PGPDO0, PGPDO3, PGPDO2...
+ */
+#define S32_PGPD(N)		(((N) ^ 1) * 2)
+#define S32_PGPD_SIZE		16
+
 enum s32_write_type {
 	S32_PINCONF_UPDATE_ONLY,
 	S32_PINCONF_OVERWRITE,
@@ -97,6 +105,7 @@ struct s32_pinctrl_context {
  * struct s32_pinctrl - private driver data
  * @dev: a pointer back to containing device
  * @pctl: a pointer to the pinctrl device structure
+ * @gc: a pointer to the gpio_chip
  * @regions: reserved memory regions with start/end pin
  * @info: structure containing information about the pin
  * @gpio_configs: saved configurations for GPIO pins
@@ -108,6 +117,7 @@ struct s32_pinctrl_context {
 struct s32_pinctrl {
 	struct device *dev;
 	struct pinctrl_dev *pctl;
+	struct gpio_chip gc;
 	struct s32_pinctrl_mem_region *regions;
 	struct s32_pinctrl_soc_info *info;
 	struct list_head gpio_configs;
@@ -396,67 +406,6 @@ static int s32_pmx_get_groups(struct pinctrl_dev *pctldev,
 	return 0;
 }
 
-static int s32_pmx_gpio_request_enable(struct pinctrl_dev *pctldev,
-				       struct pinctrl_gpio_range *range,
-				       unsigned int offset)
-{
-	struct s32_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
-	struct gpio_pin_config *gpio_pin;
-	unsigned int config;
-	unsigned long flags;
-	int ret;
-
-	ret = s32_regmap_read(pctldev, offset, &config);
-	if (ret)
-		return ret;
-
-	/* Save current configuration */
-	gpio_pin = kmalloc(sizeof(*gpio_pin), GFP_KERNEL);
-	if (!gpio_pin)
-		return -ENOMEM;
-
-	gpio_pin->pin_id = offset;
-	gpio_pin->config = config;
-	INIT_LIST_HEAD(&gpio_pin->list);
-
-	spin_lock_irqsave(&ipctl->gpio_configs_lock, flags);
-	list_add(&gpio_pin->list, &ipctl->gpio_configs);
-	spin_unlock_irqrestore(&ipctl->gpio_configs_lock, flags);
-
-	/* GPIO pin means SSS = 0 */
-	config &= ~S32_MSCR_SSS_MASK;
-
-	return s32_regmap_write(pctldev, offset, config);
-}
-
-static void s32_pmx_gpio_disable_free(struct pinctrl_dev *pctldev,
-				      struct pinctrl_gpio_range *range,
-				      unsigned int offset)
-{
-	struct s32_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
-	struct gpio_pin_config *gpio_pin, *tmp;
-	unsigned long flags;
-	int ret;
-
-	spin_lock_irqsave(&ipctl->gpio_configs_lock, flags);
-
-	list_for_each_entry_safe(gpio_pin, tmp, &ipctl->gpio_configs, list) {
-		if (gpio_pin->pin_id == offset) {
-			ret = s32_regmap_write(pctldev, gpio_pin->pin_id,
-						 gpio_pin->config);
-			if (ret != 0)
-				goto unlock;
-
-			list_del(&gpio_pin->list);
-			kfree(gpio_pin);
-			break;
-		}
-	}
-
-unlock:
-	spin_unlock_irqrestore(&ipctl->gpio_configs_lock, flags);
-}
-
 static int s32_pmx_gpio_set_direction(struct pinctrl_dev *pctldev,
 				      struct pinctrl_gpio_range *range,
 				      unsigned int offset,
@@ -480,8 +429,6 @@ static const struct pinmux_ops s32_pmx_ops = {
 	.get_function_name = s32_pmx_get_func_name,
 	.get_function_groups = s32_pmx_get_groups,
 	.set_mux = s32_pmx_set,
-	.gpio_request_enable = s32_pmx_gpio_request_enable,
-	.gpio_disable_free = s32_pmx_gpio_disable_free,
 	.gpio_set_direction = s32_pmx_gpio_set_direction,
 };
 
@@ -700,6 +647,307 @@ static const struct pinconf_ops s32_pinconf_ops = {
 	.pin_config_group_dbg_show = s32_pinconf_group_dbg_show,
 };
 
+static struct s32_pinctrl *to_s32_pinctrl(struct gpio_chip *chip)
+{
+	return container_of(chip, struct s32_pinctrl, gc);
+}
+
+static struct regmap *s32_gpio_get_pgpd_regmap(struct gpio_chip *chip,
+					       unsigned int pin,
+					       bool output)
+{
+	struct s32_pinctrl *ipctl = to_s32_pinctrl(chip);
+	struct nxp_siul2_mfd *mfd;
+	u32 base, num;
+	int i;
+
+	mfd = dev_get_drvdata(ipctl->dev->parent);
+
+	for (i = 0; i < mfd->num_siul2; i++) {
+		base = mfd->siul2[i].gpio_base;
+		num = mfd->siul2[i].gpio_num;
+
+		if (pin >= base && pin < base + num)
+			return output ? mfd->siul2[i].regmaps[SIUL2_PGPDO] :
+					mfd->siul2[i].regmaps[SIUL2_PGPDI];
+	}
+
+	return NULL;
+}
+
+static int s32_gpio_request(struct gpio_chip *gc, unsigned int gpio)
+{
+	struct s32_pinctrl *ipctl = to_s32_pinctrl(gc);
+	struct pinctrl_dev *pctldev = ipctl->pctl;
+	struct gpio_pin_config *gpio_pin;
+	unsigned int config;
+	int ret;
+
+	ret = s32_regmap_read(pctldev, gpio, &config);
+	if (ret)
+		return ret;
+
+	/* Save current configuration */
+	gpio_pin = kmalloc(sizeof(*gpio_pin), GFP_KERNEL);
+	if (!gpio_pin)
+		return -ENOMEM;
+
+	gpio_pin->pin_id = gpio;
+	gpio_pin->config = config;
+
+	scoped_guard(spinlock_irqsave, &ipctl->gpio_configs_lock)
+		list_add(&gpio_pin->list, &ipctl->gpio_configs);
+
+	/* GPIO pin means SSS = 0 */
+	config &= ~S32_MSCR_SSS_MASK;
+
+	return s32_regmap_write(pctldev, gpio, config);
+}
+
+static void s32_gpio_free(struct gpio_chip *gc, unsigned int gpio)
+{
+	struct s32_pinctrl *ipctl = to_s32_pinctrl(gc);
+	struct pinctrl_dev *pctldev = ipctl->pctl;
+	struct gpio_pin_config *gpio_pin, *tmp;
+	int ret;
+
+	guard(spinlock_irqsave)(&ipctl->gpio_configs_lock);
+
+	list_for_each_entry_safe(gpio_pin, tmp, &ipctl->gpio_configs, list) {
+		if (gpio_pin->pin_id == gpio) {
+			ret = s32_regmap_write(pctldev, gpio_pin->pin_id,
+					       gpio_pin->config);
+			if (ret != 0)
+				return;
+
+			list_del(&gpio_pin->list);
+			kfree(gpio_pin);
+			return;
+		}
+	}
+}
+
+static int s32_gpio_get_dir(struct gpio_chip *chip, unsigned int gpio)
+{
+	struct s32_pinctrl *ipctl = to_s32_pinctrl(chip);
+	unsigned int reg_value;
+	int ret;
+
+	ret = s32_regmap_read(ipctl->pctl, gpio, &reg_value);
+	if (ret)
+		return ret;
+
+	if (!(reg_value & S32_MSCR_IBE))
+		return -EINVAL;
+
+	return reg_value & S32_MSCR_OBE ? GPIO_LINE_DIRECTION_OUT :
+					  GPIO_LINE_DIRECTION_IN;
+}
+
+static unsigned int s32_pin2pad(unsigned int pin)
+{
+	return pin / S32_PGPD_SIZE;
+}
+
+static u16 s32_pin2mask(unsigned int pin)
+{
+	/**
+	 * From Reference manual :
+	 * PGPDOx[PPDOy] = GPDO(x × 16) + (15 - y)[PDO_(x × 16) + (15 - y)]
+	 */
+	return BIT(S32_PGPD_SIZE - 1 - pin % S32_PGPD_SIZE);
+}
+
+static struct regmap *s32_gpio_get_regmap_offset_mask(struct gpio_chip *chip,
+						      unsigned int gpio,
+						      unsigned int *reg_offset,
+						      u16 *mask,
+						      bool output)
+{
+	struct regmap *regmap;
+	unsigned int pad;
+
+	regmap = s32_gpio_get_pgpd_regmap(chip, gpio, output);
+	if (!regmap)
+		return NULL;
+
+	*mask = s32_pin2mask(gpio);
+	pad = s32_pin2pad(gpio);
+
+	*reg_offset = S32_PGPD(pad);
+
+	return regmap;
+}
+
+static int s32_gpio_set_val(struct gpio_chip *chip, unsigned int gpio,
+			     int value)
+{
+	unsigned int reg_offset;
+	struct regmap *regmap;
+	u16 mask;
+	int ret;
+
+	regmap = s32_gpio_get_regmap_offset_mask(chip, gpio, &reg_offset,
+						 &mask, true);
+	if (!regmap)
+		return -ENODEV;
+
+	value = value ? mask : 0;
+
+	ret = regmap_update_bits(regmap, reg_offset, mask, value);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int s32_gpio_set(struct gpio_chip *chip, unsigned int gpio,
+			 int value)
+{
+	if (s32_gpio_get_dir(chip, gpio) != GPIO_LINE_DIRECTION_OUT)
+		return -EPERM;
+
+	return s32_gpio_set_val(chip, gpio, value);
+}
+
+static int s32_gpio_get(struct gpio_chip *chip, unsigned int gpio)
+{
+	unsigned int reg_offset, value;
+	struct regmap *regmap;
+	u16 mask;
+	int ret;
+
+	if (s32_gpio_get_dir(chip, gpio) != GPIO_LINE_DIRECTION_IN)
+		return -EINVAL;
+
+	regmap = s32_gpio_get_regmap_offset_mask(chip, gpio, &reg_offset,
+						 &mask, false);
+	if (!regmap)
+		return -EINVAL;
+
+	ret = regmap_read(regmap, reg_offset, &value);
+	if (ret)
+		return ret;
+
+	return !!(value & mask);
+}
+
+static int s32_gpio_dir_out(struct gpio_chip *chip, unsigned int gpio,
+			    int val)
+{
+	struct s32_pinctrl *ipctl = to_s32_pinctrl(chip);
+
+	s32_gpio_set_val(chip, gpio, val);
+
+	return s32_pmx_gpio_set_direction(ipctl->pctl, NULL, gpio, false);
+}
+
+static int s32_gpio_dir_in(struct gpio_chip *chip, unsigned int gpio)
+{
+	struct s32_pinctrl *ipctl = to_s32_pinctrl(chip);
+
+	return s32_pmx_gpio_set_direction(ipctl->pctl, NULL, gpio, true);
+}
+
+static bool s32_gpio_is_valid(struct gpio_chip *chip, unsigned int gpio)
+{
+	struct s32_pinctrl *ipctl = to_s32_pinctrl(chip);
+	const struct s32_pinctrl_soc_data *soc_data;
+	const struct pinctrl_pin_desc *pins;
+	int i;
+
+	soc_data = ipctl->info->soc_data;
+	pins = ipctl->info->soc_data->pins;
+	for (i = 0; i < soc_data->npins && pins[i].number <= gpio; i++)
+		if (pins[i].number == gpio)
+			return true;
+
+	return false;
+}
+
+static int s32_init_valid_mask(struct gpio_chip *chip, unsigned long *mask,
+			       unsigned int ngpios)
+{
+	struct s32_pinctrl *ipctl = to_s32_pinctrl(chip);
+	const struct s32_pinctrl_soc_data *soc_data;
+	const struct pinctrl_pin_desc *pins;
+	int i;
+
+	bitmap_zero(mask, ngpios);
+
+	soc_data = ipctl->info->soc_data;
+	pins = soc_data->pins;
+	for (i = 0; i < soc_data->npins && pins[i].number < ngpios; i++)
+		bitmap_set(mask, pins[i].number, 1);
+
+	return 0;
+}
+
+static int s32_gpio_gen_names(struct device *dev, struct gpio_chip *gc,
+			      unsigned int cnt, char **names, char *ch_index,
+			      unsigned int *num_index)
+{
+	unsigned int i;
+
+	for (i = 0; i < cnt; i++) {
+		if (i != 0 && !(*num_index % 16))
+			(*ch_index)++;
+
+		if (!s32_gpio_is_valid(gc, *num_index))
+			continue;
+
+		names[i] = devm_kasprintf(dev, GFP_KERNEL, "P%c_%02d",
+					  *ch_index, 0xFU & (*num_index)++);
+		if (!names[i])
+			return -ENOMEM;
+	}
+
+	return 0;
+}
+
+static int s32_gpio_populate_names(struct device *dev,
+				   struct s32_pinctrl *ipctl)
+{
+	struct nxp_siul2_mfd *mfd = dev_get_drvdata(ipctl->dev->parent);
+	unsigned int num_index = 0;
+	char ch_index = 'A';
+	char **names;
+	int i, ret;
+
+	names = devm_kcalloc(dev, ipctl->gc.ngpio, sizeof(*names),
+			     GFP_KERNEL);
+	if (!names)
+		return -ENOMEM;
+
+	for (i = 0; i < mfd->num_siul2; i++) {
+		/*
+		 * Pin names' have the following naming: P${letter}_${index},
+		 * where:
+		 * - letters are from: {A, B, C, ...}
+		 * - indexes from {0, 1, 2, ... 15}
+		 *
+		 * If the pin is a multiple of 16, its index needs to be 0.
+		 */
+		if (mfd->siul2[i].gpio_base % 16 == 0)
+			num_index = 0;
+
+		ret = s32_gpio_gen_names(dev, &ipctl->gc,
+					 mfd->siul2[i].gpio_num,
+					 names + mfd->siul2[i].gpio_base,
+					 &ch_index, &num_index);
+		if (ret)
+			return dev_err_probe(dev, ret,
+					     "Error setting SIUL2_%d names\n",
+					     i);
+
+		ch_index++;
+	}
+
+	ipctl->gc.names = (const char *const *)names;
+
+	return 0;
+}
+
 #ifdef CONFIG_PM_SLEEP
 static bool s32_pinctrl_should_save(struct s32_pinctrl *ipctl,
 				    unsigned int pin)
@@ -986,6 +1234,8 @@ int s32_pinctrl_probe(struct platform_device *pdev,
 	struct pinctrl_desc *s32_pinctrl_desc;
 	struct s32_pinctrl_soc_info *info;
 	struct s32_pinctrl *ipctl;
+	struct nxp_siul2_mfd *mfd;
+	struct gpio_chip *gc;
 	int ret;
 
 	if (!soc_data || !soc_data->pins || !soc_data->npins)
@@ -1068,5 +1318,42 @@ int s32_pinctrl_probe(struct platform_device *pdev,
 
 	dev_info(&pdev->dev, "Initialized S32 pinctrl driver\n");
 
+
+	/* Legacy bindings only cover pinctrl functionality. */
+	if (soc_data->legacy)
+		return 0;
+
+	mfd = dev_get_drvdata(pdev->dev.parent);
+	if (!mfd)
+		return dev_err_probe(&pdev->dev, -EINVAL, "Invalid parent!\n");
+
+	gc = &ipctl->gc;
+	gc->parent = &pdev->dev;
+	gc->label = dev_name(&pdev->dev);
+	gc->base = -1;
+	/* In some cases, there is a gap between the SIUL GPIOs. */
+	gc->ngpio = mfd->siul2[mfd->num_siul2 - 1].gpio_base +
+		    mfd->siul2[mfd->num_siul2 - 1].gpio_num;
+	ret = s32_gpio_populate_names(&pdev->dev, ipctl);
+	if (ret)
+		return ret;
+
+	gc->set = s32_gpio_set;
+	gc->get = s32_gpio_get;
+	gc->set_config = gpiochip_generic_config;
+	gc->request = s32_gpio_request;
+	gc->free = s32_gpio_free;
+	gc->direction_output = s32_gpio_dir_out;
+	gc->direction_input = s32_gpio_dir_in;
+	gc->get_direction = s32_gpio_get_dir;
+	gc->init_valid_mask = s32_init_valid_mask;
+
+	ret = devm_gpiochip_add_data(&pdev->dev, gc, ipctl);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret,
+				     "Unable to add gpiochip\n");
+
+	dev_info(&pdev->dev, "Initialized s32 GPIO functionality\n");
+
 	return 0;
 }
-- 
2.50.1

Re: [PATCH v8 08/10] pinctrl: s32cc: implement GPIO functionality

[Vincent Guittot]

On Tue, 20 Jan 2026 at 12:59, Khristine Andreea Barbulescu
<khristineandreea.barbulescu@oss.nxp.com> wrote:
>
> From: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
>
> Add basic GPIO functionality (request, free, get, set) for the existing
> pinctrl SIUL2 driver since the hardware for pinctrl&GPIO is tightly
> coupled.
>
> Also, remove pinmux_ops which are no longer needed.
>
> Signed-off-by: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>

[..]

> @@ -1068,5 +1318,42 @@ int s32_pinctrl_probe(struct platform_device *pdev,
>
>         dev_info(&pdev->dev, "Initialized S32 pinctrl driver\n");
>
> +
> +       /* Legacy bindings only cover pinctrl functionality. */
> +       if (soc_data->legacy)
> +               return 0;
> +
> +       mfd = dev_get_drvdata(pdev->dev.parent);
> +       if (!mfd)
> +               return dev_err_probe(&pdev->dev, -EINVAL, "Invalid parent!\n");
> +
> +       gc = &ipctl->gc;
> +       gc->parent = &pdev->dev;
> +       gc->label = dev_name(&pdev->dev);
> +       gc->base = -1;
> +       /* In some cases, there is a gap between the SIUL GPIOs. */
> +       gc->ngpio = mfd->siul2[mfd->num_siul2 - 1].gpio_base +
> +                   mfd->siul2[mfd->num_siul2 - 1].gpio_num;
> +       ret = s32_gpio_populate_names(&pdev->dev, ipctl);
> +       if (ret)
> +               return ret;
> +
> +       gc->set = s32_gpio_set;
> +       gc->get = s32_gpio_get;
> +       gc->set_config = gpiochip_generic_config;
> +       gc->request = s32_gpio_request;
> +       gc->free = s32_gpio_free;
> +       gc->direction_output = s32_gpio_dir_out;
> +       gc->direction_input = s32_gpio_dir_in;
> +       gc->get_direction = s32_gpio_get_dir;
> +       gc->init_valid_mask = s32_init_valid_mask;
> +
> +       ret = devm_gpiochip_add_data(&pdev->dev, gc, ipctl);

Your mfd child device doesn't have a DT node, only its parent has one.
How do you point to the gpio controller in DT with a phandle ?

You probably need to create a child DT node


> +       if (ret)
> +               return dev_err_probe(&pdev->dev, ret,
> +                                    "Unable to add gpiochip\n");
> +
> +       dev_info(&pdev->dev, "Initialized s32 GPIO functionality\n");
> +
>         return 0;
>  }
> --
> 2.50.1
>

[PATCH v8 09/10] MAINTAINERS: add MAINTAINER for NXP SIUL2 MFD driver

[Khristine Andreea Barbulescu]

From: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>

Add the new MFD driver for the SIUL2 module under the NXP S32G existing
entry. This MFD driver currently has one cell for a combined pinctrl&GPIO
driver and will, in the future, contain another cell for an NVMEM driver.

Signed-off-by: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
---
 MAINTAINERS | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index f1b020588597..37d80ff0ea4f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3183,8 +3183,10 @@ R:	Ghennadi Procopciuc <ghennadi.procopciuc@oss.nxp.com>
 R:	NXP S32 Linux Team <s32@nxp.com>
 L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
 S:	Maintained
+F:	Documentation/devicetree/bindings/mfd/nxp,s32g2-siul2.yaml
 F:	Documentation/devicetree/bindings/rtc/nxp,s32g-rtc.yaml
 F:	arch/arm64/boot/dts/freescale/s32g*.dts*
+F:	drivers/mfd/nxp-siul2.c
 F:	drivers/pinctrl/nxp/
 F:	drivers/rtc/rtc-s32g.c
 
-- 
2.50.1

Re: [PATCH v8 09/10] MAINTAINERS: add MAINTAINER for NXP SIUL2 MFD driver

[Linus Walleij]

Hi Khristine,

On Tue, Jan 20, 2026 at 1:01 PM Khristine Andreea Barbulescu
<khristineandreea.barbulescu@oss.nxp.com> wrote:

> diff --git a/MAINTAINERS b/MAINTAINERS
> index f1b020588597..37d80ff0ea4f 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -3183,8 +3183,10 @@ R:       Ghennadi Procopciuc <ghennadi.procopciuc@oss.nxp.com>
>  R:     NXP S32 Linux Team <s32@nxp.com>
>  L:     linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
>  S:     Maintained
> +F:     Documentation/devicetree/bindings/mfd/nxp,s32g2-siul2.yaml
>  F:     Documentation/devicetree/bindings/rtc/nxp,s32g-rtc.yaml
>  F:     arch/arm64/boot/dts/freescale/s32g*.dts*
> +F:     drivers/mfd/nxp-siul2.c
>  F:     drivers/pinctrl/nxp/
>  F:     drivers/rtc/rtc-s32g.c

Isn't this a good time to also add yourself as maintainer for s32g?

Yours,
Linus Walleij

[PATCH v8 10/10] pinctrl: s32cc: set num_custom_params to 0

[Khristine Andreea Barbulescu]

From: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>

The `num_custom_params` was not set to 0 and the pinctrl_desc structure
was not initialized with 0. This would result in errors when parsing
pinconf properties from the device tree.

Signed-off-by: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
---
 drivers/pinctrl/nxp/pinctrl-s32cc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/pinctrl/nxp/pinctrl-s32cc.c b/drivers/pinctrl/nxp/pinctrl-s32cc.c
index 3e0c48068f08..27ea0b44a7aa 100644
--- a/drivers/pinctrl/nxp/pinctrl-s32cc.c
+++ b/drivers/pinctrl/nxp/pinctrl-s32cc.c
@@ -1272,6 +1272,7 @@ int s32_pinctrl_probe(struct platform_device *pdev,
 	s32_pinctrl_desc->pctlops = &s32_pctrl_ops;
 	s32_pinctrl_desc->pmxops = &s32_pmx_ops;
 	s32_pinctrl_desc->confops = &s32_pinconf_ops;
+	s32_pinctrl_desc->num_custom_params = 0;
 	s32_pinctrl_desc->owner = THIS_MODULE;
 
 	ipctl->regions = devm_kcalloc(&pdev->dev, soc_data->mem_regions,
-- 
2.50.1

Re: [PATCH v8 10/10] pinctrl: s32cc: set num_custom_params to 0

[Bartosz Golaszewski]

On Tue, 20 Jan 2026 12:59:22 +0100, Khristine Andreea Barbulescu
<khristineandreea.barbulescu@oss.nxp.com> said:
> From: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
>
> The `num_custom_params` was not set to 0 and the pinctrl_desc structure
> was not initialized with 0. This would result in errors when parsing
> pinconf properties from the device tree.
>

Shoudn't this come as first in the series and with a Fixes tag?

Bartosz

> Signed-off-by: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
> ---
>  drivers/pinctrl/nxp/pinctrl-s32cc.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/pinctrl/nxp/pinctrl-s32cc.c b/drivers/pinctrl/nxp/pinctrl-s32cc.c
> index 3e0c48068f08..27ea0b44a7aa 100644
> --- a/drivers/pinctrl/nxp/pinctrl-s32cc.c
> +++ b/drivers/pinctrl/nxp/pinctrl-s32cc.c
> @@ -1272,6 +1272,7 @@ int s32_pinctrl_probe(struct platform_device *pdev,
>  	s32_pinctrl_desc->pctlops = &s32_pctrl_ops;
>  	s32_pinctrl_desc->pmxops = &s32_pmx_ops;
>  	s32_pinctrl_desc->confops = &s32_pinconf_ops;
> +	s32_pinctrl_desc->num_custom_params = 0;
>  	s32_pinctrl_desc->owner = THIS_MODULE;
>
>  	ipctl->regions = devm_kcalloc(&pdev->dev, soc_data->mem_regions,
> --
> 2.50.1
>
>

Re: [PATCH v8 10/10] pinctrl: s32cc: set num_custom_params to 0

[Daniel Baluta]

On Tue, Jan 20, 2026 at 2:26 PM Bartosz Golaszewski <brgl@kernel.org> wrote:
>
> On Tue, 20 Jan 2026 12:59:22 +0100, Khristine Andreea Barbulescu
> <khristineandreea.barbulescu@oss.nxp.com> said:
> > From: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
> >
> > The `num_custom_params` was not set to 0 and the pinctrl_desc structure
> > was not initialized with 0. This would result in errors when parsing
> > pinconf properties from the device tree.
> >
>
> Shoudn't this come as first in the series and with a Fixes tag?

Hi Khristine,

The entire series could use a better patch reordering to arrange the
code as follows:

* Bug fixes
* Code refactorization or API changes
* Device tree bindings
* Driver implementation
* DTS changes

Re: [PATCH v8 00/10] gpio: siul2-s32g2: add initial GPIO driver

[Rob Herring]

On Tue, 20 Jan 2026 13:59:12 +0200, Khristine Andreea Barbulescu wrote:
> This patch series adds support for basic GPIO
> operations(set, get, direction_output/input, set_config).
> 
> There are two SIUL2 hardware modules: SIUL2_0 and SIUL2_1.
> However, this driver exports both as a single GPIO driver.
> This is because the interrupt registers are located only
> in SIUL2_1, even for GPIOs that are part of SIUL2_0.
> 
> There are two gaps in the GPIO ranges:
> - 102-111(inclusive) are invalid
> - 123-143(inclusive) are invalid
> 
> Writing and reading GPIO values is done via the PGPDO/PGPDI
> registers(Parallel GPIO Pad Data Output/Input) which are
> 16 bit registers, each bit corresponding to a GPIO.
> 
> Note that the PGPDO order is similar to a big-endian grouping
> of two registers:
> PGPDO1, PGPDO0, PGPDO3, PGPDO2, PGPDO5, PGPDO4, gap, PGPDO6.
> 
> v8 -> v7
> - remove all ': true' lines from properties in dt bindings
> - remove NVMEM MFD cell from SIUL2 in dtsi
> - remove NVMEM driver and configs
> - expose SoC information via syscon cells SIUL2_0
> and SIUL2_1 in MFD driver
> - add SIUL2_0 and SIUL2_1 syscon nodes in dtsi
> - add patternProperties for "^siul2_[0-1]$" for syscon nodes
> - update example to include syscon cells with proper format
> - remove `reg` property from pinctrl node in dt binding
> - update Kconfig help text to reflect new syscon structure
> instead of NVMEM for SoC information
> - squash deprecated SIUL2 pinctrl binding with new MFD binding
> - dropped "nxp,s32g3-siul2" from MFD driver match table
> - fixed commit messages
> - fixed dtb warnings
> 
> v7 -> v6
> - fixed MAINTAINERS wrong file path
> - add unevaluatedProperties, change siul2 node name, remove
>   jtag_pins label in the device tree schema
> - change compatible definition in schema
> - change node name in dtsi
> - mentioned binding deprecation in commit messages
> - split mfd cell conversion commit in two: one for the
>   previous refactoring, one for the mfd cell conversion
> - removed Acked-by: Linus Walleij from commit:
>   "pinctrl: s32: convert the driver into an mfd cell"
>   because of changes to that commit
> - deprecate the nxp,s32g2-siul2-pinctrl binding
> - add NVMEM MFD cell for SIUL2
> - made the GPIO driver not export invalid pins
>   (there are some gaps 102-111, 123-143)
> - removed the need for gpio-reserved-ranges
> - force initialized pinctrl_desc->num_custom_params to 0
> 
> v6 -> v5
> - removed description for reg in the dt-bindings and added
>   maxItems
> - dropped label for example in the dt-bindings
> - simplified the example in the dt-bindings
> - changed dt-bindings filename to nxp,s32g2-siul2.yaml
> - changed title in the dt-bindings
> - dropped minItmes from gpio-ranges/gpio-reserved-ranges
>   and added maxItems to gpio-reserved-ranges
> - added required block for -grp[0-9]$ nodes
> - switch to using "" as quotes
> - kernel test robot: fixed frame sizes, added description
>   for reg_name, fixed typo in gpio_configs_lock, removed
>   uninitialized ret variable usage
> - ordered includes in nxp-siul2.c, switched to dev-err-probe
>   added a mention that other commits will add nvmem functionality
>   to the mfd driver
> - switched spin_lock_irqsave to scoped_guard statement
> - switched dev_err to dev_err_probe in pinctrl-s32cc in places
>   reached during the probing part
> 
> v5 -> v4
> - fixed di_div error
> - fixed dt-bindings error
> - added Co-developed-by tags
> - added new MFD driver nxp-siul2.c
> - made the old pinctrl driver an MFD cell
> - added the GPIO driver in the existing SIUL2 pinctrl one
> - Switch from "devm_pinctrl_register" to
>   "devm_pinctrl_register_and_init"
> 
> v4 -> v3
> - removed useless parentheses
> - added S32G3 fallback compatible
> - fixed comment alignment
> - fixed dt-bindings license
> - fixed modpost: "__udivdi3"
> - moved MAINTAINERS entry to have the new GPIO driver
>   together with other files related to S32G
> 
> v3 -> v2
> - fix dt-bindings schema id
> - add maxItems to gpio-ranges
> - removed gpio label from dt-bindings example
> - added changelog for the MAINTAINERS commit and
>   added separate entry for the SIUL2 GPIO driver
> - added guard(raw_spinlock_irqsave) in
>   'siul2_gpio_set_direction'
> - updated the description for
>   'devm_platform_get_and_ioremap_resource_byname'
> 
> v2 -> v1
> dt-bindings:
> - changed filename to match compatible
> - fixed commit messages
> - removed dt-bindings unnecessary properties descriptions
> - added minItems for the interrupts property
> driver:
> - added depends on ARCH_S32 || COMPILE_TEST to Kconfig
> - added select REGMAP_MMIO to Kconfig
> - remove unnecessary include
> - add of_node_put after `siul2_get_gpio_pinspec`
> - removed inline from function definitions
> - removed match data and moved the previous platdata
>   definition to the top of the file to be visible
> - replace bitmap_set/clear with __clear_bit/set_bit
>   and devm_bitmap_zalloc with devm_kzalloc
> - switched to gpiochip_generic_request/free/config
> - fixed dev_err format for size_t reported by
>   kernel test robot
> - add platform_get_and_ioremap_resource_byname wrapper
> 
> Andrei Stefanescu (9):
>   dt-bindings: mfd: add support for the NXP SIUL2 module
>   mfd: nxp-siul2: add support for NXP SIUL2
>   arm64: dts: s32g: change pinctrl node into the new mfd node
>   pinctrl: s32cc: use dev_err_probe() and improve error messages
>   pinctrl: s32cc: change to "devm_pinctrl_register_and_init"
>   pinctrl: s32g2: change the driver to also be probed as an MFD cell
>   pinctrl: s32cc: implement GPIO functionality
>   MAINTAINERS: add MAINTAINER for NXP SIUL2 MFD driver
>   pinctrl: s32cc: set num_custom_params to 0
> 
> Khristine Andreea Barbulescu (1):
>   pinctrl: s32cc: skip syscon child nodes when parsing funcs and groups
> 
>  .../bindings/mfd/nxp,s32g2-siul2.yaml         | 165 +++++
>  .../pinctrl/nxp,s32g2-siul2-pinctrl.yaml      |   2 +
>  MAINTAINERS                                   |   2 +
>  arch/arm64/boot/dts/freescale/s32g2.dtsi      |  35 +-
>  arch/arm64/boot/dts/freescale/s32g3.dtsi      |  35 +-
>  drivers/mfd/Kconfig                           |  13 +
>  drivers/mfd/Makefile                          |   1 +
>  drivers/mfd/nxp-siul2.c                       | 440 ++++++++++++
>  drivers/pinctrl/nxp/pinctrl-s32.h             |   4 +-
>  drivers/pinctrl/nxp/pinctrl-s32cc.c           | 652 ++++++++++++++----
>  drivers/pinctrl/nxp/pinctrl-s32g2.c           |  32 +-
>  include/linux/mfd/nxp-siul2.h                 |  55 ++
>  12 files changed, 1259 insertions(+), 177 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/mfd/nxp,s32g2-siul2.yaml
>  create mode 100644 drivers/mfd/nxp-siul2.c
>  create mode 100644 include/linux/mfd/nxp-siul2.h
> 
> --
> 2.50.1
> 
> 
> 


My bot found new DTB warnings on the .dts files added or changed in this
series.

Some warnings may be from an existing SoC .dtsi. Or perhaps the warnings
are fixed by another series. Ultimately, it is up to the platform
maintainer whether these warnings are acceptable or not. No need to reply
unless the platform maintainer has comments.

If you already ran DT checks and didn't see these error(s), then
make sure dt-schema is up to date:

  pip3 install dtschema --upgrade


This patch series was applied (using b4) to base:
 Base: attempting to guess base-commit...
 Base: tags/v6.19-rc1-8-gea6806346080 (best guess, 8/9 blobs matched)
 Base: tags/v6.19-rc1-8-gea6806346080 (use --merge-base to override)

If this is not the correct base, please add 'base-commit' tag
(or use b4 which does this automatically)


Warnings in base: 24
Warnings after series: 42

New warnings running 'make CHECK_DTBS=y for arch/arm64/boot/dts/freescale/' for 20260120115923.3463866-1-khristineandreea.barbulescu@oss.nxp.com:

arch/arm64/boot/dts/freescale/s32g399a-rdb3.dtb: pinctrl@4009c000 (nxp,s32g3-siul2): 'siul2_0@4009c000', 'siul2_1@44010000' do not match any of the regexes: '-hog(-[0-9]+)?$', '-pins$', '^pinctrl-[0-9]+$', '^siul2_[0-1]$'
	from schema $id: http://devicetree.org/schemas/mfd/nxp,s32g2-siul2.yaml
arch/arm64/boot/dts/freescale/s32g399a-rdb3.dtb: siul2_0@4009c000 (syscon): compatible: ['syscon'] is too short
	from schema $id: http://devicetree.org/schemas/mfd/syscon-common.yaml
arch/arm64/boot/dts/freescale/s32g399a-rdb3.dtb: siul2_1@44010000 (syscon): compatible: ['syscon'] is too short
	from schema $id: http://devicetree.org/schemas/mfd/syscon-common.yaml
arch/arm64/boot/dts/freescale/s32g274a-evb.dtb: pinctrl@4009c000 (nxp,s32g2-siul2): 'siul2_0@4009c000', 'siul2_1@44010000' do not match any of the regexes: '-hog(-[0-9]+)?$', '-pins$', '^pinctrl-[0-9]+$', '^siul2_[0-1]$'
	from schema $id: http://devicetree.org/schemas/mfd/nxp,s32g2-siul2.yaml
arch/arm64/boot/dts/freescale/s32g274a-evb.dtb: siul2_0@4009c000 (syscon): compatible: ['syscon'] is too short
	from schema $id: http://devicetree.org/schemas/mfd/syscon-common.yaml
arch/arm64/boot/dts/freescale/s32g274a-evb.dtb: siul2_1@44010000 (syscon): compatible: ['syscon'] is too short
	from schema $id: http://devicetree.org/schemas/mfd/syscon-common.yaml
arch/arm64/boot/dts/freescale/s32g274a-rdb2.dtb: pinctrl@4009c000 (nxp,s32g2-siul2): 'siul2_0@4009c000', 'siul2_1@44010000' do not match any of the regexes: '-hog(-[0-9]+)?$', '-pins$', '^pinctrl-[0-9]+$', '^siul2_[0-1]$'
	from schema $id: http://devicetree.org/schemas/mfd/nxp,s32g2-siul2.yaml
arch/arm64/boot/dts/freescale/s32g274a-rdb2.dtb: siul2_0@4009c000 (syscon): compatible: ['syscon'] is too short
	from schema $id: http://devicetree.org/schemas/mfd/syscon-common.yaml
arch/arm64/boot/dts/freescale/s32g274a-rdb2.dtb: siul2_1@44010000 (syscon): compatible: ['syscon'] is too short
	from schema $id: http://devicetree.org/schemas/mfd/syscon-common.yaml