* [PATCH v7 01/12] dt-bindings: mfd: add support for the NXP SIUL2 module
2025-07-10 14:20 [PATCH v7 00/12] gpio: siul2-s32g2: add initial GPIO driver Andrei Stefanescu
@ 2025-07-10 14:20 ` Andrei Stefanescu
2025-07-10 15:49 ` Frank Li
` (3 more replies)
2025-07-10 14:20 ` [PATCH v7 02/12] mfd: nxp-siul2: add support for NXP SIUL2 Andrei Stefanescu
` (11 subsequent siblings)
12 siblings, 4 replies; 36+ messages in thread
From: Andrei Stefanescu @ 2025-07-10 14:20 UTC (permalink / raw)
To: linus.walleij, brgl, robh, krzk+dt, conor+dt, chester62515,
mbrugger, Ghennadi.Procopciuc, larisa.grigore, lee, shawnguo,
s.hauer, festevam, aisheng.dong, ping.bai, gregkh, rafael, srini
Cc: linux-gpio, devicetree, linux-kernel, linux-arm-kernel, s32,
clizzi, aruizrui, eballetb, echanude, kernel, imx,
vincent.guittot, Andrei Stefanescu
Add the 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.
Signed-off-by: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
---
.../bindings/mfd/nxp,s32g2-siul2.yaml | 163 ++++++++++++++++++
1 file changed, 163 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..8ae185b4bc78
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/nxp,s32g2-siul2.yaml
@@ -0,0 +1,163 @@
+# 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
+
+ reg:
+ minItems: 2
+
+ reg-names:
+ items:
+ - const: siul20
+ - const: siul21
+
+ gpio-controller: true
+
+ "#gpio-cells":
+ const: 2
+
+ gpio-ranges:
+ maxItems: 2
+
+ interrupts:
+ maxItems: 1
+
+ interrupt-controller: true
+
+ "#interrupt-cells":
+ const: 2
+
+ nvmem-layout:
+ $ref: /schemas/nvmem/layouts/nvmem-layout.yaml#
+ description:
+ This container may reference an NVMEM layout parser.
+
+patternProperties:
+ "-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:
+ bias-disable: true
+ bias-high-impedance: true
+ bias-pull-up: true
+ bias-pull-down: true
+ drive-open-drain: true
+ input-enable: true
+ output-enable: true
+
+ pinmux:
+ description: |
+ An integer array for representing pinmux configurations of
+ a device. Each integer consists of a PIN_ID and a 4-bit
+ selected signal source(SSS) as IOMUX setting, which is
+ calculated as: pinmux = (PIN_ID << 4 | SSS)
+
+ slew-rate:
+ description: Supported slew rate based on Fmax values (MHz)
+ enum: [83, 133, 150, 166, 208]
+ required:
+ - pinmux
+
+ unevaluatedProperties: false
+
+required:
+ - compatible
+ - reg
+ - reg-names
+ - gpio-controller
+ - "#gpio-cells"
+ - gpio-ranges
+ - interrupts
+ - interrupt-controller
+ - "#interrupt-cells"
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/interrupt-controller/irq.h>
+
+ pinctrl@4009c000 {
+ compatible = "nxp,s32g2-siul2";
+ reg = <0x4009c000 0x179c>,
+ <0x44010000 0x17b0>;
+ 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>;
+
+ soc-major@0 {
+ reg = <0 0x4>;
+ };
+ };
+ };
+...
--
2.45.2
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH v7 01/12] dt-bindings: mfd: add support for the NXP SIUL2 module
2025-07-10 14:20 ` [PATCH v7 01/12] dt-bindings: mfd: add support for the NXP SIUL2 module Andrei Stefanescu
@ 2025-07-10 15:49 ` Frank Li
2025-07-11 7:36 ` Krzysztof Kozlowski
` (2 subsequent siblings)
3 siblings, 0 replies; 36+ messages in thread
From: Frank Li @ 2025-07-10 15:49 UTC (permalink / raw)
To: Andrei Stefanescu
Cc: linus.walleij, brgl, robh, krzk+dt, conor+dt, chester62515,
mbrugger, Ghennadi.Procopciuc, larisa.grigore, lee, shawnguo,
s.hauer, festevam, aisheng.dong, ping.bai, gregkh, rafael, srini,
linux-gpio, devicetree, linux-kernel, linux-arm-kernel, s32,
clizzi, aruizrui, eballetb, echanude, kernel, imx,
vincent.guittot
On Thu, Jul 10, 2025 at 05:20:24PM +0300, Andrei Stefanescu wrote:
> Add the 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.
wrap at 75 chars.
>
> Signed-off-by: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
> ---
> .../bindings/mfd/nxp,s32g2-siul2.yaml | 163 ++++++++++++++++++
> 1 file changed, 163 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..8ae185b4bc78
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/nxp,s32g2-siul2.yaml
> @@ -0,0 +1,163 @@
> +# 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
> +
> + reg:
> + minItems: 2
> +
> + reg-names:
> + items:
> + - const: siul20
> + - const: siul21
> +
> + gpio-controller: true
> +
> + "#gpio-cells":
> + const: 2
> +
> + gpio-ranges:
> + maxItems: 2
> +
> + interrupts:
> + maxItems: 1
> +
> + interrupt-controller: true
> +
> + "#interrupt-cells":
> + const: 2
> +
> + nvmem-layout:
> + $ref: /schemas/nvmem/layouts/nvmem-layout.yaml#
> + description:
> + This container may reference an NVMEM layout parser.
> +
> +patternProperties:
> + "-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:
> + bias-disable: true
> + bias-high-impedance: true
> + bias-pull-up: true
> + bias-pull-down: true
> + drive-open-drain: true
> + input-enable: true
> + output-enable: true
I remember you needn't mark these as true, default it should be true if use
"unevaluatedProperties: false".
Can you check if pass dt_binding_check if remove these property?
> +
> + 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)
Do you have include file to define it? otherwise hex value is not easy
understand.
Frank
> +
> + slew-rate:
> + description: Supported slew rate based on Fmax values (MHz)
> + enum: [83, 133, 150, 166, 208]
> + required:
> + - pinmux
> +
> + unevaluatedProperties: false
> +
> +required:
> + - compatible
> + - reg
> + - reg-names
> + - gpio-controller
> + - "#gpio-cells"
> + - gpio-ranges
> + - interrupts
> + - interrupt-controller
> + - "#interrupt-cells"
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> + #include <dt-bindings/interrupt-controller/irq.h>
> +
> + pinctrl@4009c000 {
> + compatible = "nxp,s32g2-siul2";
> + reg = <0x4009c000 0x179c>,
> + <0x44010000 0x17b0>;
> + 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>;
> +
> + soc-major@0 {
> + reg = <0 0x4>;
> + };
> + };
> + };
> +...
> --
> 2.45.2
>
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [PATCH v7 01/12] dt-bindings: mfd: add support for the NXP SIUL2 module
2025-07-10 14:20 ` [PATCH v7 01/12] dt-bindings: mfd: add support for the NXP SIUL2 module Andrei Stefanescu
2025-07-10 15:49 ` Frank Li
@ 2025-07-11 7:36 ` Krzysztof Kozlowski
2025-07-11 7:45 ` Krzysztof Kozlowski
2025-07-11 7:37 ` Krzysztof Kozlowski
2025-07-11 7:39 ` Krzysztof Kozlowski
3 siblings, 1 reply; 36+ messages in thread
From: Krzysztof Kozlowski @ 2025-07-11 7:36 UTC (permalink / raw)
To: Andrei Stefanescu
Cc: linus.walleij, brgl, robh, krzk+dt, conor+dt, chester62515,
mbrugger, Ghennadi.Procopciuc, larisa.grigore, lee, shawnguo,
s.hauer, festevam, aisheng.dong, ping.bai, gregkh, rafael, srini,
linux-gpio, devicetree, linux-kernel, linux-arm-kernel, s32,
clizzi, aruizrui, eballetb, echanude, kernel, imx,
vincent.guittot
On Thu, Jul 10, 2025 at 05:20:24PM +0300, Andrei Stefanescu wrote:
> Add the 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.
>
> Signed-off-by: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
> ---
> .../bindings/mfd/nxp,s32g2-siul2.yaml | 163 ++++++++++++++++++
> 1 file changed, 163 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mfd/nxp,s32g2-siul2.yaml
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v7 01/12] dt-bindings: mfd: add support for the NXP SIUL2 module
2025-07-11 7:36 ` Krzysztof Kozlowski
@ 2025-07-11 7:45 ` Krzysztof Kozlowski
0 siblings, 0 replies; 36+ messages in thread
From: Krzysztof Kozlowski @ 2025-07-11 7:45 UTC (permalink / raw)
To: Andrei Stefanescu
Cc: linus.walleij, brgl, robh, krzk+dt, conor+dt, chester62515,
mbrugger, Ghennadi.Procopciuc, larisa.grigore, lee, shawnguo,
s.hauer, festevam, aisheng.dong, ping.bai, gregkh, rafael, srini,
linux-gpio, devicetree, linux-kernel, linux-arm-kernel, s32,
clizzi, aruizrui, eballetb, echanude, kernel, imx,
vincent.guittot
On 11/07/2025 09:36, Krzysztof Kozlowski wrote:
> On Thu, Jul 10, 2025 at 05:20:24PM +0300, Andrei Stefanescu wrote:
>> Add the 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.
>>
>> Signed-off-by: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
>> ---
>> .../bindings/mfd/nxp,s32g2-siul2.yaml | 163 ++++++++++++++++++
>> 1 file changed, 163 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/mfd/nxp,s32g2-siul2.yaml
>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>
Unreviewed. I missed that there were some random changes in the binding
not coming from review, not explained in changelog.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v7 01/12] dt-bindings: mfd: add support for the NXP SIUL2 module
2025-07-10 14:20 ` [PATCH v7 01/12] dt-bindings: mfd: add support for the NXP SIUL2 module Andrei Stefanescu
2025-07-10 15:49 ` Frank Li
2025-07-11 7:36 ` Krzysztof Kozlowski
@ 2025-07-11 7:37 ` Krzysztof Kozlowski
2025-07-11 7:39 ` Krzysztof Kozlowski
3 siblings, 0 replies; 36+ messages in thread
From: Krzysztof Kozlowski @ 2025-07-11 7:37 UTC (permalink / raw)
To: Andrei Stefanescu
Cc: linus.walleij, brgl, robh, krzk+dt, conor+dt, chester62515,
mbrugger, Ghennadi.Procopciuc, larisa.grigore, lee, shawnguo,
s.hauer, festevam, aisheng.dong, ping.bai, gregkh, rafael, srini,
linux-gpio, devicetree, linux-kernel, linux-arm-kernel, s32,
clizzi, aruizrui, eballetb, echanude, kernel, imx,
vincent.guittot
On Thu, Jul 10, 2025 at 05:20:24PM +0300, Andrei Stefanescu wrote:
> + properties:
> + bias-disable: true
> + bias-high-impedance: true
> + bias-pull-up: true
> + bias-pull-down: true
> + drive-open-drain: true
> + input-enable: true
> + output-enable: true
> +
> + pinmux:
> + description: |
> + An integer array for representing pinmux configurations of
> + a device. Each integer consists of a PIN_ID and a 4-bit
> + selected signal source(SSS) as IOMUX setting, which is
> + calculated as: pinmux = (PIN_ID << 4 | SSS)
> +
> + slew-rate:
> + description: Supported slew rate based on Fmax values (MHz)
> + enum: [83, 133, 150, 166, 208]
> + required:
> + - pinmux
> +
> + unevaluatedProperties: false
I noticed Frank's comment and he pointed out that this changed, so all
':true' lines before should be removed as well.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v7 01/12] dt-bindings: mfd: add support for the NXP SIUL2 module
2025-07-10 14:20 ` [PATCH v7 01/12] dt-bindings: mfd: add support for the NXP SIUL2 module Andrei Stefanescu
` (2 preceding siblings ...)
2025-07-11 7:37 ` Krzysztof Kozlowski
@ 2025-07-11 7:39 ` Krzysztof Kozlowski
2025-07-11 12:25 ` Andrei Stefanescu
3 siblings, 1 reply; 36+ messages in thread
From: Krzysztof Kozlowski @ 2025-07-11 7:39 UTC (permalink / raw)
To: Andrei Stefanescu
Cc: linus.walleij, brgl, robh, krzk+dt, conor+dt, chester62515,
mbrugger, Ghennadi.Procopciuc, larisa.grigore, lee, shawnguo,
s.hauer, festevam, aisheng.dong, ping.bai, gregkh, rafael, srini,
linux-gpio, devicetree, linux-kernel, linux-arm-kernel, s32,
clizzi, aruizrui, eballetb, echanude, kernel, imx,
vincent.guittot
On Thu, Jul 10, 2025 at 05:20:24PM +0300, Andrei Stefanescu wrote:
> Add the 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.
>
> Signed-off-by: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
> ---
> .../bindings/mfd/nxp,s32g2-siul2.yaml | 163 ++++++++++++++++++
> 1 file changed, 163 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..8ae185b4bc78
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/nxp,s32g2-siul2.yaml
> @@ -0,0 +1,163 @@
> +# 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
> +
> + reg:
> + minItems: 2
Eh, and after reading your deprecated patch I went back here and see
this changed... Why? Why are you making random changes?
I retract my review, I was too hasty.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [PATCH v7 01/12] dt-bindings: mfd: add support for the NXP SIUL2 module
2025-07-11 7:39 ` Krzysztof Kozlowski
@ 2025-07-11 12:25 ` Andrei Stefanescu
0 siblings, 0 replies; 36+ messages in thread
From: Andrei Stefanescu @ 2025-07-11 12:25 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: linus.walleij, brgl, robh, krzk+dt, conor+dt, chester62515,
mbrugger, Ghennadi.Procopciuc, larisa.grigore, lee, shawnguo,
s.hauer, festevam, aisheng.dong, ping.bai, gregkh, rafael, srini,
linux-gpio, devicetree, linux-kernel, linux-arm-kernel, s32,
clizzi, aruizrui, eballetb, echanude, kernel, imx,
vincent.guittot
Hi Krzysztof,
Thank you for your review!
>> +
>> + reg:
>> + minItems: 2
>
> Eh, and after reading your deprecated patch I went back here and see
> this changed... Why? Why are you making random changes?
I changed it during internal review. Sorry for not mentioning it in the cover
patch. I will revert back to maxItems.
Best regards,
Andrei
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v7 02/12] mfd: nxp-siul2: add support for NXP SIUL2
2025-07-10 14:20 [PATCH v7 00/12] gpio: siul2-s32g2: add initial GPIO driver Andrei Stefanescu
2025-07-10 14:20 ` [PATCH v7 01/12] dt-bindings: mfd: add support for the NXP SIUL2 module Andrei Stefanescu
@ 2025-07-10 14:20 ` Andrei Stefanescu
2025-07-10 16:20 ` Frank Li
2025-07-10 14:20 ` [PATCH v7 03/12] arm64: dts: s32g: change pinctrl node into the new mfd node Andrei Stefanescu
` (10 subsequent siblings)
12 siblings, 1 reply; 36+ messages in thread
From: Andrei Stefanescu @ 2025-07-10 14:20 UTC (permalink / raw)
To: linus.walleij, brgl, robh, krzk+dt, conor+dt, chester62515,
mbrugger, Ghennadi.Procopciuc, larisa.grigore, lee, shawnguo,
s.hauer, festevam, aisheng.dong, ping.bai, gregkh, rafael, srini
Cc: linux-gpio, devicetree, linux-kernel, linux-arm-kernel, s32,
clizzi, aruizrui, eballetb, echanude, kernel, imx,
vincent.guittot, Andrei Stefanescu
This commit will 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)
This commit only adds support for pinctrl&GPIO(one cell). Further
commits will add nvmem functionality(a second 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>
---
drivers/mfd/Kconfig | 12 +
drivers/mfd/Makefile | 1 +
drivers/mfd/nxp-siul2.c | 410 ++++++++++++++++++++++++++++++++++
include/linux/mfd/nxp-siul2.h | 55 +++++
4 files changed, 478 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 6fb3768e3d71..e6634e05091e 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1143,6 +1143,18 @@ 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 nvmem drivers for the 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 79495f9f3457..02e3cc0a2303 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -224,6 +224,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..904f41b3c61b
--- /dev/null
+++ b/drivers/mfd/nxp-siul2.c
@@ -0,0 +1,410 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * SIUL2(System Integration Unit Lite) MFD driver
+ *
+ * Copyright 2025 NXP
+ */
+#include <linux/init.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/nxp-siul2.h>
+#include <linux/module.h>
+#include <linux/of.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;
+ void __iomem *base;
+ char reg_name[16];
+ int i, ret;
+
+ priv = platform_get_drvdata(pdev);
+
+ for (i = 0; i < priv->num_siul2; i++) {
+ ret = snprintf(reg_name, ARRAY_SIZE(reg_name), "siul2%d", i);
+ if (ret < 0 || ret >= ARRAY_SIZE(reg_name))
+ return ret;
+
+ base = devm_platform_ioremap_resource_byname(pdev, reg_name);
+ if (IS_ERR(base))
+ return dev_err_probe(&pdev->dev, PTR_ERR(base),
+ "Failed to get MEM resource: %s\n",
+ reg_name);
+
+ ret = nxp_siul2_init_regmap(pdev, base, i);
+ if (ret)
+ return ret;
+
+ 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];
+ }
+
+ return 0;
+}
+
+static int nxp_siul2_probe(struct platform_device *pdev)
+{
+ struct nxp_siul2_mfd *priv;
+ int ret;
+
+ 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;
+
+ 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" },
+ { .compatible = "nxp,s32g3-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..4603a97e40e4
--- /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 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.45.2
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH v7 02/12] mfd: nxp-siul2: add support for NXP SIUL2
2025-07-10 14:20 ` [PATCH v7 02/12] mfd: nxp-siul2: add support for NXP SIUL2 Andrei Stefanescu
@ 2025-07-10 16:20 ` Frank Li
2025-07-11 7:13 ` Andrei Stefanescu
0 siblings, 1 reply; 36+ messages in thread
From: Frank Li @ 2025-07-10 16:20 UTC (permalink / raw)
To: Andrei Stefanescu
Cc: linus.walleij, brgl, robh, krzk+dt, conor+dt, chester62515,
mbrugger, Ghennadi.Procopciuc, larisa.grigore, lee, shawnguo,
s.hauer, festevam, aisheng.dong, ping.bai, gregkh, rafael, srini,
linux-gpio, devicetree, linux-kernel, linux-arm-kernel, s32,
clizzi, aruizrui, eballetb, echanude, kernel, imx,
vincent.guittot
On Thu, Jul 10, 2025 at 05:20:25PM +0300, Andrei Stefanescu wrote:
> This commit will add support for the NXP SIUL2 module as an MFD device.
Needn't "this commit", just
Add support ....
>
> SIUL2 (System Integration Unit Lite) is a hardware module which
> implements various functionalities:
> - reading SoC information
> - pinctrl
> - GPIO (including interrupts)
>
> This commit only adds support for pinctrl&GPIO(one cell). Further
Add support ...
> commits will add nvmem functionality(a second 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>
> ---
> drivers/mfd/Kconfig | 12 +
> drivers/mfd/Makefile | 1 +
> drivers/mfd/nxp-siul2.c | 410 ++++++++++++++++++++++++++++++++++
> include/linux/mfd/nxp-siul2.h | 55 +++++
> 4 files changed, 478 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 6fb3768e3d71..e6634e05091e 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -1143,6 +1143,18 @@ 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 nvmem drivers for the 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 79495f9f3457..02e3cc0a2303 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -224,6 +224,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..904f41b3c61b
> --- /dev/null
> +++ b/drivers/mfd/nxp-siul2.c
> @@ -0,0 +1,410 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * SIUL2(System Integration Unit Lite) MFD driver
> + *
> + * Copyright 2025 NXP
> + */
> +#include <linux/init.h>
> +#include <linux/mfd/core.h>
> +#include <linux/mfd/nxp-siul2.h>
> +#include <linux/module.h>
> +#include <linux/of.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",
I think you can add full list here.
> + }
> +};
> +
> +/**
> + * 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;
> + void __iomem *base;
> + char reg_name[16];
> + int i, ret;
> +
> + priv = platform_get_drvdata(pdev);
> +
> + for (i = 0; i < priv->num_siul2; i++) {
> + ret = snprintf(reg_name, ARRAY_SIZE(reg_name), "siul2%d", i);
> + if (ret < 0 || ret >= ARRAY_SIZE(reg_name))
> + return ret;
> +
> + base = devm_platform_ioremap_resource_byname(pdev, reg_name);
> + if (IS_ERR(base))
> + return dev_err_probe(&pdev->dev, PTR_ERR(base),
> + "Failed to get MEM resource: %s\n",
> + reg_name);
> +
> + ret = nxp_siul2_init_regmap(pdev, base, i);
> + if (ret)
> + return ret;
> +
> + 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];
> + }
> +
> + return 0;
> +}
> +
> +static int nxp_siul2_probe(struct platform_device *pdev)
> +{
> + struct nxp_siul2_mfd *priv;
> + int ret;
> +
> + 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;
> +
> + 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" },
> + { .compatible = "nxp,s32g3-siul2" },
in binding doc "nxp,s32g3-siul2" fallback to "nxp,s32g2-siul2".
needn't nxp,s32g3-siul2 here.
> + { /* 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..4603a97e40e4
> --- /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 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.45.2
>
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [PATCH v7 02/12] mfd: nxp-siul2: add support for NXP SIUL2
2025-07-10 16:20 ` Frank Li
@ 2025-07-11 7:13 ` Andrei Stefanescu
0 siblings, 0 replies; 36+ messages in thread
From: Andrei Stefanescu @ 2025-07-11 7:13 UTC (permalink / raw)
To: Frank Li
Cc: linus.walleij, brgl, robh, krzk+dt, conor+dt, chester62515,
mbrugger, Ghennadi.Procopciuc, larisa.grigore, lee, shawnguo,
s.hauer, festevam, aisheng.dong, ping.bai, gregkh, rafael, srini,
linux-gpio, devicetree, linux-kernel, linux-arm-kernel, s32,
clizzi, aruizrui, eballetb, echanude, kernel, imx,
vincent.guittot
Hi Frank,
Thank you very much for the reviews!
>> + }
>> +
>> +static const struct mfd_cell nxp_siul2_devs[] = {
>> + {
>> + .name = "s32g-siul2-pinctrl",
>
> I think you can add full list here.
>
The next cell is added by a later commit. Could I add the name here even
if it doesn't match an existing driver? It will match a driver when the
NVMEM driver commit(the one from this patch series) is introduced.
Best regards,
Andrei
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v7 03/12] arm64: dts: s32g: change pinctrl node into the new mfd node
2025-07-10 14:20 [PATCH v7 00/12] gpio: siul2-s32g2: add initial GPIO driver Andrei Stefanescu
2025-07-10 14:20 ` [PATCH v7 01/12] dt-bindings: mfd: add support for the NXP SIUL2 module Andrei Stefanescu
2025-07-10 14:20 ` [PATCH v7 02/12] mfd: nxp-siul2: add support for NXP SIUL2 Andrei Stefanescu
@ 2025-07-10 14:20 ` Andrei Stefanescu
2025-07-10 14:20 ` [PATCH v7 04/12] pinctrl: s32cc: small refactoring Andrei Stefanescu
` (9 subsequent siblings)
12 siblings, 0 replies; 36+ messages in thread
From: Andrei Stefanescu @ 2025-07-10 14:20 UTC (permalink / raw)
To: linus.walleij, brgl, robh, krzk+dt, conor+dt, chester62515,
mbrugger, Ghennadi.Procopciuc, larisa.grigore, lee, shawnguo,
s.hauer, festevam, aisheng.dong, ping.bai, gregkh, rafael, srini
Cc: linux-gpio, devicetree, linux-kernel, linux-arm-kernel, s32,
clizzi, aruizrui, eballetb, echanude, kernel, imx,
vincent.guittot, Andrei Stefanescu
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>
---
arch/arm64/boot/dts/freescale/s32g2.dtsi | 48 +++++++++++++++++-------
arch/arm64/boot/dts/freescale/s32g3.dtsi | 48 +++++++++++++++++-------
2 files changed, 68 insertions(+), 28 deletions(-)
diff --git a/arch/arm64/boot/dts/freescale/s32g2.dtsi b/arch/arm64/boot/dts/freescale/s32g2.dtsi
index ea1456d361a3..78b0d21ff295 100644
--- a/arch/arm64/boot/dts/freescale/s32g2.dtsi
+++ b/arch/arm64/boot/dts/freescale/s32g2.dtsi
@@ -114,20 +114,18 @@ soc@0 {
#size-cells = <1>;
ranges = <0 0 0 0x80000000>;
- 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";
+ reg = <0x4009c000 0x179c>,
+ <0x44010000 0x17b0>;
+ reg-names = "siul20", "siul21";
+ gpio-controller;
+ #gpio-cells = <2>;
+ gpio-ranges = <&pinctrl 0 0 102>, <&pinctrl 112 112 79>;
+ gpio-reserved-ranges = <102 10>, <123 21>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ interrupts = <GIC_SPI 210 IRQ_TYPE_LEVEL_HIGH>;
jtag_pins: jtag-pins {
jtag-grp0 {
@@ -315,6 +313,28 @@ usdhc0-200mhz-grp4 {
<0x20c2>;
};
};
+
+ nvmem-layout {
+ compatible = "fixed-layout";
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ soc_major: soc-major@0 {
+ reg = <0 4>;
+ };
+
+ soc_minor: soc-minor@1 {
+ reg = <1 4>;
+ };
+
+ pcie_dev_id: pcie-dev-id@2 {
+ reg = <2 4>;
+ };
+
+ serdes_presence: serdes-presence@100 {
+ reg = <100 4>;
+ };
+ };
};
edma0: dma-controller@40144000 {
diff --git a/arch/arm64/boot/dts/freescale/s32g3.dtsi b/arch/arm64/boot/dts/freescale/s32g3.dtsi
index 991dbfbfa203..769f8210d73d 100644
--- a/arch/arm64/boot/dts/freescale/s32g3.dtsi
+++ b/arch/arm64/boot/dts/freescale/s32g3.dtsi
@@ -171,20 +171,18 @@ soc@0 {
#size-cells = <1>;
ranges = <0 0 0 0x80000000>;
- 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";
+ reg = <0x4009c000 0x179c>,
+ <0x44010000 0x17b0>;
+ reg-names = "siul20", "siul21";
+ gpio-controller;
+ #gpio-cells = <2>;
+ gpio-ranges = <&pinctrl 0 0 102>, <&pinctrl 112 112 79>;
+ gpio-reserved-ranges = <102 10>, <123 21>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ interrupts = <GIC_SPI 210 IRQ_TYPE_LEVEL_HIGH>;
jtag_pins: jtag-pins {
jtag-grp0 {
@@ -372,6 +370,28 @@ usdhc0-200mhz-grp4 {
<0x20c2>;
};
};
+
+ nvmem-layout {
+ compatible = "fixed-layout";
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ soc_major: soc-major@0 {
+ reg = <0 4>;
+ };
+
+ soc_minor: soc-minor@1 {
+ reg = <1 4>;
+ };
+
+ pcie_dev_id: pcie-dev-id@2 {
+ reg = <2 4>;
+ };
+
+ serdes_presence: serdes-presence@100 {
+ reg = <100 4>;
+ };
+ };
};
edma0: dma-controller@40144000 {
--
2.45.2
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PATCH v7 04/12] pinctrl: s32cc: small refactoring
2025-07-10 14:20 [PATCH v7 00/12] gpio: siul2-s32g2: add initial GPIO driver Andrei Stefanescu
` (2 preceding siblings ...)
2025-07-10 14:20 ` [PATCH v7 03/12] arm64: dts: s32g: change pinctrl node into the new mfd node Andrei Stefanescu
@ 2025-07-10 14:20 ` Andrei Stefanescu
2025-07-10 16:24 ` Frank Li
2025-07-10 14:20 ` [PATCH v7 05/12] pinctrl: s32cc: change to "devm_pinctrl_register_and_init" Andrei Stefanescu
` (8 subsequent siblings)
12 siblings, 1 reply; 36+ messages in thread
From: Andrei Stefanescu @ 2025-07-10 14:20 UTC (permalink / raw)
To: linus.walleij, brgl, robh, krzk+dt, conor+dt, chester62515,
mbrugger, Ghennadi.Procopciuc, larisa.grigore, lee, shawnguo,
s.hauer, festevam, aisheng.dong, ping.bai, gregkh, rafael, srini
Cc: linux-gpio, devicetree, linux-kernel, linux-arm-kernel, s32,
clizzi, aruizrui, eballetb, echanude, kernel, imx,
vincent.guittot, Andrei Stefanescu
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 501eb296c760..c90cd96a9dc4 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;
}
@@ -475,8 +483,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:
@@ -762,15 +770,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;
@@ -811,10 +819,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);
@@ -885,10 +892,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,
@@ -918,18 +924,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)
@@ -964,16 +969,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.45.2
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH v7 04/12] pinctrl: s32cc: small refactoring
2025-07-10 14:20 ` [PATCH v7 04/12] pinctrl: s32cc: small refactoring Andrei Stefanescu
@ 2025-07-10 16:24 ` Frank Li
0 siblings, 0 replies; 36+ messages in thread
From: Frank Li @ 2025-07-10 16:24 UTC (permalink / raw)
To: Andrei Stefanescu
Cc: linus.walleij, brgl, robh, krzk+dt, conor+dt, chester62515,
mbrugger, Ghennadi.Procopciuc, larisa.grigore, lee, shawnguo,
s.hauer, festevam, aisheng.dong, ping.bai, gregkh, rafael, srini,
linux-gpio, devicetree, linux-kernel, linux-arm-kernel, s32,
clizzi, aruizrui, eballetb, echanude, kernel, imx,
vincent.guittot
On Thu, Jul 10, 2025 at 05:20:27PM +0300, Andrei Stefanescu wrote:
Subject need descript what actually you did. "small refactoring" means
nothing.
Use dev_err_probe() simplify code and fix error message/comments.
> 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 501eb296c760..c90cd96a9dc4 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;
> }
> @@ -475,8 +483,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:
> @@ -762,15 +770,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;
>
> @@ -811,10 +819,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);
> @@ -885,10 +892,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,
> @@ -918,18 +924,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)
> @@ -964,16 +969,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.45.2
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v7 05/12] pinctrl: s32cc: change to "devm_pinctrl_register_and_init"
2025-07-10 14:20 [PATCH v7 00/12] gpio: siul2-s32g2: add initial GPIO driver Andrei Stefanescu
` (3 preceding siblings ...)
2025-07-10 14:20 ` [PATCH v7 04/12] pinctrl: s32cc: small refactoring Andrei Stefanescu
@ 2025-07-10 14:20 ` Andrei Stefanescu
2025-07-10 16:24 ` Frank Li
2025-07-10 14:20 ` [PATCH v7 06/12] dt-bindings: pinctrl: deprecate SIUL2 pinctrl bindings Andrei Stefanescu
` (7 subsequent siblings)
12 siblings, 1 reply; 36+ messages in thread
From: Andrei Stefanescu @ 2025-07-10 14:20 UTC (permalink / raw)
To: linus.walleij, brgl, robh, krzk+dt, conor+dt, chester62515,
mbrugger, Ghennadi.Procopciuc, larisa.grigore, lee, shawnguo,
s.hauer, festevam, aisheng.dong, ping.bai, gregkh, rafael, srini
Cc: linux-gpio, devicetree, linux-kernel, linux-arm-kernel, s32,
clizzi, aruizrui, eballetb, echanude, kernel, imx,
vincent.guittot, Andrei Stefanescu
Switch from "devm_pinctrl_register" to "devm_pinctrl_register_and_init"
and "pinctrl_enable" since this is the recommended way.
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 c90cd96a9dc4..c03dac643cb3 100644
--- a/drivers/pinctrl/nxp/pinctrl-s32cc.c
+++ b/drivers/pinctrl/nxp/pinctrl-s32cc.c
@@ -973,10 +973,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
@@ -989,7 +989,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.45.2
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH v7 05/12] pinctrl: s32cc: change to "devm_pinctrl_register_and_init"
2025-07-10 14:20 ` [PATCH v7 05/12] pinctrl: s32cc: change to "devm_pinctrl_register_and_init" Andrei Stefanescu
@ 2025-07-10 16:24 ` Frank Li
0 siblings, 0 replies; 36+ messages in thread
From: Frank Li @ 2025-07-10 16:24 UTC (permalink / raw)
To: Andrei Stefanescu
Cc: linus.walleij, brgl, robh, krzk+dt, conor+dt, chester62515,
mbrugger, Ghennadi.Procopciuc, larisa.grigore, lee, shawnguo,
s.hauer, festevam, aisheng.dong, ping.bai, gregkh, rafael, srini,
linux-gpio, devicetree, linux-kernel, linux-arm-kernel, s32,
clizzi, aruizrui, eballetb, echanude, kernel, imx,
vincent.guittot
On Thu, Jul 10, 2025 at 05:20:28PM +0300, Andrei Stefanescu wrote:
> Switch from "devm_pinctrl_register" to "devm_pinctrl_register_and_init"
> and "pinctrl_enable" since this is the recommended way.
>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
Reviewed-by: Frank Li <Frank.Li@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 c90cd96a9dc4..c03dac643cb3 100644
> --- a/drivers/pinctrl/nxp/pinctrl-s32cc.c
> +++ b/drivers/pinctrl/nxp/pinctrl-s32cc.c
> @@ -973,10 +973,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
> @@ -989,7 +989,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.45.2
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v7 06/12] dt-bindings: pinctrl: deprecate SIUL2 pinctrl bindings
2025-07-10 14:20 [PATCH v7 00/12] gpio: siul2-s32g2: add initial GPIO driver Andrei Stefanescu
` (4 preceding siblings ...)
2025-07-10 14:20 ` [PATCH v7 05/12] pinctrl: s32cc: change to "devm_pinctrl_register_and_init" Andrei Stefanescu
@ 2025-07-10 14:20 ` Andrei Stefanescu
2025-07-10 16:26 ` Frank Li
2025-07-11 7:44 ` Krzysztof Kozlowski
2025-07-10 14:20 ` [PATCH v7 07/12] pinctrl: s32g2: change the driver to also be probed as an MFD cell Andrei Stefanescu
` (6 subsequent siblings)
12 siblings, 2 replies; 36+ messages in thread
From: Andrei Stefanescu @ 2025-07-10 14:20 UTC (permalink / raw)
To: linus.walleij, brgl, robh, krzk+dt, conor+dt, chester62515,
mbrugger, Ghennadi.Procopciuc, larisa.grigore, lee, shawnguo,
s.hauer, festevam, aisheng.dong, ping.bai, gregkh, rafael, srini
Cc: linux-gpio, devicetree, linux-kernel, linux-arm-kernel, s32,
clizzi, aruizrui, eballetb, echanude, kernel, imx,
vincent.guittot, Andrei Stefanescu
The existing SIUL2 pinctrl bindings don't 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. For more details see the discussions
on the community [1] and [2].
The existing SIUL2 pinctrl dt-bindings will be deprecated. The new SIUL2
MFD dt-bindings are to be used from now on.
[1] -
https://lore.kernel.org/linux-gpio/20241003-overall-unblended-7139b17eae23@spud/
[2] -
https://lore.kernel.org/all/a924bbb6-96ec-40be-9d82-a76b2ab73afd@oss.nxp.com/
Signed-off-by: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
---
.../devicetree/bindings/pinctrl/nxp,s32g2-siul2-pinctrl.yaml | 2 ++
1 file changed, 2 insertions(+)
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.45.2
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH v7 06/12] dt-bindings: pinctrl: deprecate SIUL2 pinctrl bindings
2025-07-10 14:20 ` [PATCH v7 06/12] dt-bindings: pinctrl: deprecate SIUL2 pinctrl bindings Andrei Stefanescu
@ 2025-07-10 16:26 ` Frank Li
2025-07-11 7:44 ` Krzysztof Kozlowski
1 sibling, 0 replies; 36+ messages in thread
From: Frank Li @ 2025-07-10 16:26 UTC (permalink / raw)
To: Andrei Stefanescu
Cc: linus.walleij, brgl, robh, krzk+dt, conor+dt, chester62515,
mbrugger, Ghennadi.Procopciuc, larisa.grigore, lee, shawnguo,
s.hauer, festevam, aisheng.dong, ping.bai, gregkh, rafael, srini,
linux-gpio, devicetree, linux-kernel, linux-arm-kernel, s32,
clizzi, aruizrui, eballetb, echanude, kernel, imx,
vincent.guittot
On Thu, Jul 10, 2025 at 05:20:29PM +0300, Andrei Stefanescu wrote:
> The existing SIUL2 pinctrl bindings don't 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. For more details see the discussions
> on the community [1] and [2].
>
> The existing SIUL2 pinctrl dt-bindings will be deprecated. The new SIUL2
> MFD dt-bindings are to be used from now on.
>
> [1] -
> https://lore.kernel.org/linux-gpio/20241003-overall-unblended-7139b17eae23@spud/
> [2] -
> https://lore.kernel.org/all/a924bbb6-96ec-40be-9d82-a76b2ab73afd@oss.nxp.com/
suggest squash to
[PATCH v7 01/12] dt-bindings: mfd: add support for the NXP SIUL2 module
Frank
>
> Signed-off-by: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
> ---
> .../devicetree/bindings/pinctrl/nxp,s32g2-siul2-pinctrl.yaml | 2 ++
> 1 file changed, 2 insertions(+)
>
> 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.45.2
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v7 06/12] dt-bindings: pinctrl: deprecate SIUL2 pinctrl bindings
2025-07-10 14:20 ` [PATCH v7 06/12] dt-bindings: pinctrl: deprecate SIUL2 pinctrl bindings Andrei Stefanescu
2025-07-10 16:26 ` Frank Li
@ 2025-07-11 7:44 ` Krzysztof Kozlowski
1 sibling, 0 replies; 36+ messages in thread
From: Krzysztof Kozlowski @ 2025-07-11 7:44 UTC (permalink / raw)
To: Andrei Stefanescu
Cc: linus.walleij, brgl, robh, krzk+dt, conor+dt, chester62515,
mbrugger, Ghennadi.Procopciuc, larisa.grigore, lee, shawnguo,
s.hauer, festevam, aisheng.dong, ping.bai, gregkh, rafael, srini,
linux-gpio, devicetree, linux-kernel, linux-arm-kernel, s32,
clizzi, aruizrui, eballetb, echanude, kernel, imx,
vincent.guittot
On Thu, Jul 10, 2025 at 05:20:29PM +0300, Andrei Stefanescu wrote:
> The existing SIUL2 pinctrl bindings don't 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. For more details see the discussions
> on the community [1] and [2].
>
> The existing SIUL2 pinctrl dt-bindings will be deprecated. The new SIUL2
> MFD dt-bindings are to be used from now on.
>
> [1] -
> https://lore.kernel.org/linux-gpio/20241003-overall-unblended-7139b17eae23@spud/
> [2] -
> https://lore.kernel.org/all/a924bbb6-96ec-40be-9d82-a76b2ab73afd@oss.nxp.com/
After fast glance I see only nvmem is outside and missing in these
bindings. I don't see a problem nvmem being a separate device node, so I
don't see justification for dropping old bindings.
Anyway, bring the full raltionale here - describe the memory layout
proving that you cannot add nvmem and GPIO in your system without
breaking ABI or without making this binding unreadable.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v7 07/12] pinctrl: s32g2: change the driver to also be probed as an MFD cell
2025-07-10 14:20 [PATCH v7 00/12] gpio: siul2-s32g2: add initial GPIO driver Andrei Stefanescu
` (5 preceding siblings ...)
2025-07-10 14:20 ` [PATCH v7 06/12] dt-bindings: pinctrl: deprecate SIUL2 pinctrl bindings Andrei Stefanescu
@ 2025-07-10 14:20 ` Andrei Stefanescu
2025-07-11 10:52 ` kernel test robot
2025-07-10 14:20 ` [PATCH v7 08/12] pinctrl: s32cc: implement GPIO functionality Andrei Stefanescu
` (5 subsequent siblings)
12 siblings, 1 reply; 36+ messages in thread
From: Andrei Stefanescu @ 2025-07-10 14:20 UTC (permalink / raw)
To: linus.walleij, brgl, robh, krzk+dt, conor+dt, chester62515,
mbrugger, Ghennadi.Procopciuc, larisa.grigore, lee, shawnguo,
s.hauer, festevam, aisheng.dong, ping.bai, gregkh, rafael, srini
Cc: linux-gpio, devicetree, linux-kernel, linux-arm-kernel, s32,
clizzi, aruizrui, eballetb, echanude, kernel, imx,
vincent.guittot, Andrei Stefanescu
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
- one cell for an NVMEM driver
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>
---
drivers/pinctrl/nxp/pinctrl-s32.h | 4 +-
drivers/pinctrl/nxp/pinctrl-s32cc.c | 132 ++++++++++++++++++++++------
drivers/pinctrl/nxp/pinctrl-s32g2.c | 32 +++++--
3 files changed, 136 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 c03dac643cb3..18b81b246bec 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>
@@ -39,6 +40,8 @@
#define S32_MSCR_ODE BIT(20)
#define S32_MSCR_OBE BIT(21)
+#define NVMEM_LAYOUT "nvmem-layout"
+
enum s32_write_type {
S32_PINCONF_UPDATE_ONLY,
S32_PINCONF_OVERWRITE,
@@ -101,6 +104,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 +117,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 +137,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 +250,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;
@@ -682,6 +701,11 @@ 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);
+}
+
#ifdef CONFIG_PM_SLEEP
static bool s32_pinctrl_should_save(struct s32_pinctrl *ipctl,
unsigned int pin)
@@ -803,8 +827,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;
@@ -842,31 +866,24 @@ 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;
+ const struct s32_pinctrl_soc_data *soc_data;
+ 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;
+ soc_data = info->soc_data;
- 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);
@@ -881,7 +898,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);
@@ -891,7 +908,53 @@ 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;
+ const struct s32_pinctrl_soc_data *soc_data;
+ unsigned int mem_regions;
+ u8 regmap_type;
+ u32 i = 0, j;
+
+ soc_data = info->soc_data;
+
+ /* 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)
+ if (strncmp(child->name, NVMEM_LAYOUT, sizeof(NVMEM_LAYOUT)))
+ ++nfuncs;
if (nfuncs <= 0)
return dev_err_probe(&pdev->dev, -EINVAL,
"No functions defined\n");
@@ -904,15 +967,18 @@ static int s32_pinctrl_probe_dt(struct platform_device *pdev,
info->ngroups = 0;
for_each_child_of_node_scoped(np, child)
- info->ngroups += of_get_child_count(child);
+ if (strncmp(child->name, NVMEM_LAYOUT, sizeof(NVMEM_LAYOUT)))
+ info->ngroups += of_get_child_count(child);
info->groups = devm_kcalloc(&pdev->dev, info->ngroups,
sizeof(*info->groups), GFP_KERNEL);
if (!info->groups)
return -ENOMEM;
- i = 0;
for_each_child_of_node_scoped(np, child) {
+ if (!strncmp(child->name, NVMEM_LAYOUT, sizeof(NVMEM_LAYOUT)))
+ continue;
+
ret = s32_pinctrl_parse_functions(child, info, i++);
if (ret)
return ret;
@@ -968,12 +1034,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.45.2
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH v7 07/12] pinctrl: s32g2: change the driver to also be probed as an MFD cell
2025-07-10 14:20 ` [PATCH v7 07/12] pinctrl: s32g2: change the driver to also be probed as an MFD cell Andrei Stefanescu
@ 2025-07-11 10:52 ` kernel test robot
0 siblings, 0 replies; 36+ messages in thread
From: kernel test robot @ 2025-07-11 10:52 UTC (permalink / raw)
To: Andrei Stefanescu, linus.walleij, brgl, robh, krzk+dt, conor+dt,
chester62515, mbrugger, Ghennadi.Procopciuc, larisa.grigore, lee,
shawnguo, s.hauer, festevam, aisheng.dong, ping.bai, gregkh,
rafael, srini
Cc: oe-kbuild-all, linux-gpio, devicetree, linux-kernel,
linux-arm-kernel, s32, clizzi, aruizrui, eballetb, echanude,
kernel, imx, vincent.guittot
Hi Andrei,
kernel test robot noticed the following build warnings:
[auto build test WARNING on linusw-pinctrl/devel]
[also build test WARNING on linusw-pinctrl/for-next lee-mfd/for-mfd-next lee-mfd/for-mfd-fixes linus/master v6.16-rc5]
[cannot apply to shawnguo/for-next next-20250710]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Andrei-Stefanescu/dt-bindings-mfd-add-support-for-the-NXP-SIUL2-module/20250710-222538
base: https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git devel
patch link: https://lore.kernel.org/r/20250710142038.1986052-8-andrei.stefanescu%40oss.nxp.com
patch subject: [PATCH v7 07/12] pinctrl: s32g2: change the driver to also be probed as an MFD cell
config: arm64-randconfig-003-20250711 (https://download.01.org/0day-ci/archive/20250711/202507111841.QdQ2DVpT-lkp@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 12.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250711/202507111841.QdQ2DVpT-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202507111841.QdQ2DVpT-lkp@intel.com/
All warnings (new ones prefixed by >>):
In file included from include/linux/bits.h:32,
from include/linux/bitops.h:6,
from drivers/pinctrl/nxp/pinctrl-s32cc.c:10:
drivers/pinctrl/nxp/pinctrl-s32cc.c: In function 'to_s32_pinctrl':
include/linux/container_of.h:20:54: error: 'struct s32_pinctrl' has no member named 'gc'
20 | static_assert(__same_type(*(ptr), ((type *)0)->member) || \
| ^~
include/linux/build_bug.h:78:56: note: in definition of macro '__static_assert'
78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
| ^~~~
include/linux/container_of.h:20:9: note: in expansion of macro 'static_assert'
20 | static_assert(__same_type(*(ptr), ((type *)0)->member) || \
| ^~~~~~~~~~~~~
include/linux/container_of.h:20:23: note: in expansion of macro '__same_type'
20 | static_assert(__same_type(*(ptr), ((type *)0)->member) || \
| ^~~~~~~~~~~
drivers/pinctrl/nxp/pinctrl-s32cc.c:706:16: note: in expansion of macro 'container_of'
706 | return container_of(chip, struct s32_pinctrl, gc);
| ^~~~~~~~~~~~
include/linux/compiler_types.h:503:27: error: expression in static assertion is not an integer
503 | #define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b))
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/build_bug.h:78:56: note: in definition of macro '__static_assert'
78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
| ^~~~
include/linux/container_of.h:20:9: note: in expansion of macro 'static_assert'
20 | static_assert(__same_type(*(ptr), ((type *)0)->member) || \
| ^~~~~~~~~~~~~
include/linux/container_of.h:20:23: note: in expansion of macro '__same_type'
20 | static_assert(__same_type(*(ptr), ((type *)0)->member) || \
| ^~~~~~~~~~~
drivers/pinctrl/nxp/pinctrl-s32cc.c:706:16: note: in expansion of macro 'container_of'
706 | return container_of(chip, struct s32_pinctrl, gc);
| ^~~~~~~~~~~~
In file included from include/uapi/linux/posix_types.h:5,
from include/uapi/linux/types.h:14,
from include/linux/types.h:6,
from include/linux/kasan-checks.h:5,
from include/asm-generic/rwonce.h:26,
from arch/arm64/include/asm/rwonce.h:67,
from include/linux/compiler.h:390,
from include/linux/build_bug.h:5:
include/linux/stddef.h:16:33: error: 'struct s32_pinctrl' has no member named 'gc'
16 | #define offsetof(TYPE, MEMBER) __builtin_offsetof(TYPE, MEMBER)
| ^~~~~~~~~~~~~~~~~~
include/linux/container_of.h:23:28: note: in expansion of macro 'offsetof'
23 | ((type *)(__mptr - offsetof(type, member))); })
| ^~~~~~~~
drivers/pinctrl/nxp/pinctrl-s32cc.c:706:16: note: in expansion of macro 'container_of'
706 | return container_of(chip, struct s32_pinctrl, gc);
| ^~~~~~~~~~~~
drivers/pinctrl/nxp/pinctrl-s32cc.c: In function 'legacy_s32_pinctrl_regmap_init':
>> drivers/pinctrl/nxp/pinctrl-s32cc.c:873:44: warning: variable 'soc_data' set but not used [-Wunused-but-set-variable]
873 | const struct s32_pinctrl_soc_data *soc_data;
| ^~~~~~~~
drivers/pinctrl/nxp/pinctrl-s32cc.c: In function 's32_pinctrl_mfd_regmap_init':
drivers/pinctrl/nxp/pinctrl-s32cc.c:920:44: warning: variable 'soc_data' set but not used [-Wunused-but-set-variable]
920 | const struct s32_pinctrl_soc_data *soc_data;
| ^~~~~~~~
drivers/pinctrl/nxp/pinctrl-s32cc.c: At top level:
drivers/pinctrl/nxp/pinctrl-s32cc.c:704:28: warning: 'to_s32_pinctrl' defined but not used [-Wunused-function]
704 | static struct s32_pinctrl *to_s32_pinctrl(struct gpio_chip *chip)
| ^~~~~~~~~~~~~~
vim +/soc_data +873 drivers/pinctrl/nxp/pinctrl-s32cc.c
868
869 static int legacy_s32_pinctrl_regmap_init(struct platform_device *pdev,
870 struct s32_pinctrl *ipctl)
871 {
872 struct s32_pinctrl_soc_info *info = ipctl->info;
> 873 const struct s32_pinctrl_soc_data *soc_data;
874 unsigned int mem_regions;
875 struct resource *res;
876 struct regmap *map;
877 void __iomem *base;
878 u32 i = 0;
879
880 soc_data = info->soc_data;
881
882 mem_regions = info->soc_data->mem_regions;
883 if (mem_regions == 0 || mem_regions >= 10000)
884 return dev_err_probe(&pdev->dev, -EINVAL,
885 "mem_regions is invalid: %u\n",
886 mem_regions);
887
888 for (i = 0; i < mem_regions; i++) {
889 base = devm_platform_get_and_ioremap_resource(pdev, i, &res);
890 if (IS_ERR(base))
891 return PTR_ERR(base);
892
893 snprintf(ipctl->regions[i].name,
894 sizeof(ipctl->regions[i].name), "map%u", i);
895
896 s32_regmap_config.name = ipctl->regions[i].name;
897 s32_regmap_config.max_register = resource_size(res) -
898 s32_regmap_config.reg_stride;
899
900 map = devm_regmap_init_mmio(&pdev->dev, base,
901 &s32_regmap_config);
902 if (IS_ERR(map)) {
903 dev_err(&pdev->dev, "Failed to init regmap[%u]\n", i);
904 return PTR_ERR(map);
905 }
906
907 ipctl->regions[i].map = map;
908 ipctl->regions[i].pin_range = &info->soc_data->mem_pin_ranges[i];
909 }
910
911 return 0;
912 }
913
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v7 08/12] pinctrl: s32cc: implement GPIO functionality
2025-07-10 14:20 [PATCH v7 00/12] gpio: siul2-s32g2: add initial GPIO driver Andrei Stefanescu
` (6 preceding siblings ...)
2025-07-10 14:20 ` [PATCH v7 07/12] pinctrl: s32g2: change the driver to also be probed as an MFD cell Andrei Stefanescu
@ 2025-07-10 14:20 ` Andrei Stefanescu
2025-07-10 14:20 ` [PATCH v7 09/12] MAINTAINERS: add MAINTAINER for NXP SIUL2 MFD driver Andrei Stefanescu
` (4 subsequent siblings)
12 siblings, 0 replies; 36+ messages in thread
From: Andrei Stefanescu @ 2025-07-10 14:20 UTC (permalink / raw)
To: linus.walleij, brgl, robh, krzk+dt, conor+dt, chester62515,
mbrugger, Ghennadi.Procopciuc, larisa.grigore, lee, shawnguo,
s.hauer, festevam, aisheng.dong, ping.bai, gregkh, rafael, srini
Cc: linux-gpio, devicetree, linux-kernel, linux-arm-kernel, s32,
clizzi, aruizrui, eballetb, echanude, kernel, imx,
vincent.guittot, Andrei Stefanescu
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 | 402 +++++++++++++++++++++++-----
1 file changed, 340 insertions(+), 62 deletions(-)
diff --git a/drivers/pinctrl/nxp/pinctrl-s32cc.c b/drivers/pinctrl/nxp/pinctrl-s32cc.c
index 18b81b246bec..8e9da792d035 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
+
#define NVMEM_LAYOUT "nvmem-layout"
enum s32_write_type {
@@ -99,6 +107,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
@@ -110,6 +119,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;
@@ -398,66 +408,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;
-
- 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,
@@ -481,8 +431,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,
};
@@ -706,6 +654,297 @@ 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, ®_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 void s32_gpio_set_val(struct gpio_chip *chip, unsigned int gpio,
+ int value)
+{
+ unsigned int reg_offset;
+ struct regmap *regmap;
+ u16 mask;
+
+ regmap = s32_gpio_get_regmap_offset_mask(chip, gpio, ®_offset,
+ &mask, true);
+ if (!regmap)
+ return;
+
+ value = value ? mask : 0;
+
+ regmap_update_bits(regmap, reg_offset, mask, value);
+}
+
+static void s32_gpio_set(struct gpio_chip *chip, unsigned int gpio,
+ int value)
+{
+ if (s32_gpio_get_dir(chip, gpio) != GPIO_LINE_DIRECTION_OUT)
+ 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, ®_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)
@@ -996,6 +1235,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)
@@ -1078,5 +1319,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.45.2
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PATCH v7 09/12] MAINTAINERS: add MAINTAINER for NXP SIUL2 MFD driver
2025-07-10 14:20 [PATCH v7 00/12] gpio: siul2-s32g2: add initial GPIO driver Andrei Stefanescu
` (7 preceding siblings ...)
2025-07-10 14:20 ` [PATCH v7 08/12] pinctrl: s32cc: implement GPIO functionality Andrei Stefanescu
@ 2025-07-10 14:20 ` Andrei Stefanescu
2025-07-10 14:20 ` [PATCH v7 10/12] nvmem: s32g2_siul2: add NVMEM driver for SoC information Andrei Stefanescu
` (3 subsequent siblings)
12 siblings, 0 replies; 36+ messages in thread
From: Andrei Stefanescu @ 2025-07-10 14:20 UTC (permalink / raw)
To: linus.walleij, brgl, robh, krzk+dt, conor+dt, chester62515,
mbrugger, Ghennadi.Procopciuc, larisa.grigore, lee, shawnguo,
s.hauer, festevam, aisheng.dong, ping.bai, gregkh, rafael, srini
Cc: linux-gpio, devicetree, linux-kernel, linux-arm-kernel, s32,
clizzi, aruizrui, eballetb, echanude, kernel, imx,
vincent.guittot, Andrei Stefanescu
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 efb51ee92683..3e9b54bbcdb8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3000,7 +3000,9 @@ 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: arch/arm64/boot/dts/freescale/s32g*.dts*
+F: drivers/mfd/nxp-siul2.c
F: drivers/pinctrl/nxp/
ARM/NXP S32G/S32R DWMAC ETHERNET DRIVER
--
2.45.2
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PATCH v7 10/12] nvmem: s32g2_siul2: add NVMEM driver for SoC information
2025-07-10 14:20 [PATCH v7 00/12] gpio: siul2-s32g2: add initial GPIO driver Andrei Stefanescu
` (8 preceding siblings ...)
2025-07-10 14:20 ` [PATCH v7 09/12] MAINTAINERS: add MAINTAINER for NXP SIUL2 MFD driver Andrei Stefanescu
@ 2025-07-10 14:20 ` Andrei Stefanescu
2025-07-11 5:37 ` Arnd Bergmann
2025-07-18 13:45 ` Lee Jones
2025-07-10 14:20 ` [PATCH v7 11/12] MAINTAINERS: add MAINTAINER for NXP SIUL2 NVMEM cell Andrei Stefanescu
` (2 subsequent siblings)
12 siblings, 2 replies; 36+ messages in thread
From: Andrei Stefanescu @ 2025-07-10 14:20 UTC (permalink / raw)
To: linus.walleij, brgl, robh, krzk+dt, conor+dt, chester62515,
mbrugger, Ghennadi.Procopciuc, larisa.grigore, lee, shawnguo,
s.hauer, festevam, aisheng.dong, ping.bai, gregkh, rafael, srini
Cc: linux-gpio, devicetree, linux-kernel, linux-arm-kernel, s32,
clizzi, aruizrui, eballetb, echanude, kernel, imx,
vincent.guittot, Andrei Stefanescu
The SIUL2 hardware module has registers which expose information about
the given SoC (version, SRAM size, presence of some hw modules).
Signed-off-by: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
---
drivers/mfd/nxp-siul2.c | 6 +-
drivers/nvmem/Kconfig | 10 ++
drivers/nvmem/Makefile | 2 +
drivers/nvmem/s32g2_siul2_nvmem.c | 232 ++++++++++++++++++++++++++++++
4 files changed, 249 insertions(+), 1 deletion(-)
create mode 100644 drivers/nvmem/s32g2_siul2_nvmem.c
diff --git a/drivers/mfd/nxp-siul2.c b/drivers/mfd/nxp-siul2.c
index 904f41b3c61b..edf643e4bcba 100644
--- a/drivers/mfd/nxp-siul2.c
+++ b/drivers/mfd/nxp-siul2.c
@@ -31,7 +31,11 @@
static const struct mfd_cell nxp_siul2_devs[] = {
{
.name = "s32g-siul2-pinctrl",
- }
+ },
+ {
+
+ .name = "s32g2-siul2-nvmem",
+ },
};
/**
diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
index d370b2ad11e7..6efd23a2ee17 100644
--- a/drivers/nvmem/Kconfig
+++ b/drivers/nvmem/Kconfig
@@ -454,4 +454,14 @@ config NVMEM_QORIQ_EFUSE
This driver can also be built as a module. If so, the module
will be called nvmem_qoriq_efuse.
+config NVMEM_S32G2_SIUL2
+ tristate "S32G2 SIUL2 nvmem support: SoC revision"
+ depends on ARCH_S32 || COMPILE_TEST
+ default y
+ help
+ This is a driver to access hardware related data like SoC revision
+ for S32G2/S32G3 SoCs.
+
+ The revision information is retrieved from the SIUL2 module.
+
endif
diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile
index 2021d59688db..84fef48b7ff6 100644
--- a/drivers/nvmem/Makefile
+++ b/drivers/nvmem/Makefile
@@ -89,3 +89,5 @@ obj-$(CONFIG_NVMEM_ZYNQMP) += nvmem_zynqmp_nvmem.o
nvmem_zynqmp_nvmem-y := zynqmp_nvmem.o
obj-$(CONFIG_NVMEM_QORIQ_EFUSE) += nvmem-qoriq-efuse.o
nvmem-qoriq-efuse-y := qoriq-efuse.o
+obj-$(CONFIG_NVMEM_S32G2_SIUL2) += nvmem-s32g2-siul2.o
+nvmem-s32g2-siul2-y := s32g2_siul2_nvmem.o
diff --git a/drivers/nvmem/s32g2_siul2_nvmem.c b/drivers/nvmem/s32g2_siul2_nvmem.c
new file mode 100644
index 000000000000..bf62c8885ff5
--- /dev/null
+++ b/drivers/nvmem/s32g2_siul2_nvmem.c
@@ -0,0 +1,232 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2021-2025 NXP
+ */
+
+#include <linux/mfd/nxp-siul2.h>
+#include <linux/module.h>
+#include <linux/nvmem-provider.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+/* SoC revision */
+#define NVRAM_CELL_SIZE 4
+#define SIUL2_MIDR1_OFF (0x00000000)
+#define SIUL2_MIDR2_OFF (0x00000004)
+
+#define SIUL20_CELL(c) (c)
+#define SIUL21_CELL(c) (100u + (c))
+#define SOC_MAJOR_CELL_OFFSET SIUL20_CELL(0)
+#define SOC_MINOR_CELL_OFFSET SIUL20_CELL(1)
+#define PCIE_DEV_ID_CELL_OFFSET SIUL20_CELL(2)
+#define SERDES_PRESENCE_CELL_OFFSET SIUL21_CELL(0)
+
+/* SIUL20_MIDR1 masks */
+#define SIUL20_MIDR1_MINOR_MASK (0xF << 0)
+#define SIUL20_MIDR1_MAJOR_SHIFT (4)
+#define SIUL20_MIDR1_MAJOR_MASK (0xF << SIUL20_MIDR1_MAJOR_SHIFT)
+#define SIUL20_MIDR1_PART_NO_SHIFT (16)
+#define SIUL20_MIDR1_PART_NO_MASK GENMASK(25, 16)
+
+/* SIUL21_MIDR2 masks */
+#define SIUL21_MIDR2_SERDES_MASK BIT(15)
+
+#define SIUL2_QUIRK_MIDR1_DECREMENT_VAL BIT(1)
+
+struct s32g2_nvmem_drvdata {
+ u32 quirks;
+};
+
+struct s32g2_siul2_nvmem_data {
+ struct device *dev;
+ struct nvmem_device *nvmem;
+ struct regmap **regmaps;
+ struct s32g2_nvmem_drvdata drvdata;
+ u8 num_siul2;
+};
+
+static int needs_minor_decrement(const struct s32g2_nvmem_drvdata *data)
+{
+ return data->quirks & SIUL2_QUIRK_MIDR1_DECREMENT_VAL;
+}
+
+/* 3 digit part number */
+static int get_part_no(struct s32g2_siul2_nvmem_data *priv, u32 *part)
+{
+ int ret;
+
+ ret = regmap_read(priv->regmaps[0], SIUL2_MIDR1_OFF, part);
+ if (ret)
+ dev_err(priv->dev, "Failed to read SIUL2 PART_NO!\n");
+
+ *part &= SIUL20_MIDR1_PART_NO_MASK;
+ *part >>= SIUL20_MIDR1_PART_NO_SHIFT;
+
+ return ret;
+}
+
+static u32 get_variant_bits(u32 value)
+{
+ /*
+ * Mapping between G3 variant ID and the PCIe Device ID,
+ * as described in the "S32G3 Reference Manual",
+ * chapter SerDes Subsystem, section "Device and revision IDs",
+ * where: index = last 2 digits of the variant
+ * value = last hex digit of the PCIe Device ID"
+ */
+ static const u32 s32g3_variants[] = {
+ [78] = 0x6,
+ [79] = 0x4,
+ [98] = 0x2,
+ [99] = 0x0,
+ };
+
+ /* PCIe variant bits with respect to PCIe Device ID update
+ * applies only to S32G3 platforms.
+ */
+ if (value / 100 != 3)
+ return 0;
+
+ value %= 100;
+
+ if (value < ARRAY_SIZE(s32g3_variants))
+ return s32g3_variants[value];
+
+ return 0;
+}
+
+static int s32g2_siul2_nvmem_read(void *context, unsigned int offset,
+ void *val, size_t bytes)
+{
+ u32 major, minor, part_no, serdes, midr1, midr2;
+ struct s32g2_siul2_nvmem_data *priv = context;
+ int ret;
+
+ if (bytes != NVRAM_CELL_SIZE)
+ return -EOPNOTSUPP;
+
+ switch (offset) {
+ /* SIUL20 cells */
+ case SOC_MAJOR_CELL_OFFSET:
+ ret = regmap_read(priv->regmaps[0], SIUL2_MIDR1_OFF, &midr1);
+ if (ret)
+ return ret;
+ major = (midr1 & SIUL20_MIDR1_MAJOR_MASK) >> SIUL20_MIDR1_MAJOR_SHIFT;
+
+ /* Bytes format: 0.0.0.MAJOR */
+ *(u32 *)val = major + 1;
+
+ return 0;
+
+ case SOC_MINOR_CELL_OFFSET:
+ ret = regmap_read(priv->regmaps[0], SIUL2_MIDR1_OFF, &midr1);
+ if (ret)
+ return ret;
+
+ minor = midr1 & SIUL20_MIDR1_MINOR_MASK;
+
+ if (minor > 0 && needs_minor_decrement(&priv->drvdata))
+ minor--;
+
+ /* Bytes format: 0.0.0.MINOR */
+ *(u32 *)val = minor;
+
+ return 0;
+
+ case PCIE_DEV_ID_CELL_OFFSET:
+ ret = get_part_no(priv, &part_no);
+ if (ret)
+ return ret;
+
+ /* Bytes format: 0.0.0.PCIE_VARIANT */
+ *(u32 *)val = get_variant_bits(part_no);
+
+ return 0;
+
+ /* SIUL21 cells */
+ case SERDES_PRESENCE_CELL_OFFSET:
+ ret = regmap_read(priv->regmaps[1], SIUL2_MIDR2_OFF, &midr2);
+ if (ret)
+ return ret;
+
+ serdes = !!(midr2 & SIUL21_MIDR2_SERDES_MASK);
+ *(u32 *)val = serdes;
+ return 0;
+
+ default:
+ return -EOPNOTSUPP;
+ }
+}
+
+static int s32g2_siul2_nvmem_probe(struct platform_device *pdev)
+{
+ struct nxp_siul2_mfd *mfd = dev_get_drvdata(pdev->dev.parent);
+ struct s32g2_siul2_nvmem_data *priv;
+ struct nvmem_config econfig = {
+ .name = "s32g2-siul2_nvmem",
+ .add_legacy_fixed_of_cells = false,
+ .owner = THIS_MODULE,
+ .word_size = 4,
+ .size = 4,
+ .read_only = true,
+ };
+ int i, ret;
+ u32 part;
+
+ if (!mfd)
+ return dev_err_probe(&pdev->dev, -EINVAL,
+ "Invalid SIUL2 NVMEM parent!\n");
+
+ priv = devm_kzalloc(&pdev->dev, sizeof(struct s32g2_siul2_nvmem_data),
+ GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ priv->num_siul2 = mfd->num_siul2;
+ priv->regmaps = devm_kmalloc_array(&pdev->dev, priv->num_siul2,
+ sizeof(*priv->regmaps), GFP_KERNEL);
+ if (!priv->regmaps)
+ return -ENOMEM;
+
+ for (i = 0; i < priv->num_siul2; i++)
+ priv->regmaps[i] = mfd->siul2[i].regmaps[SIUL2_MIDR];
+
+ priv->dev = &pdev->dev;
+ econfig.reg_read = s32g2_siul2_nvmem_read;
+ econfig.dev = pdev->dev.parent;
+ econfig.priv = priv;
+
+ ret = get_part_no(priv, &part);
+ if (ret)
+ return ret;
+
+ /* S32G2 SoCs have a special case. */
+ if (part / 100 == 2)
+ priv->drvdata.quirks |= SIUL2_QUIRK_MIDR1_DECREMENT_VAL;
+
+ priv->nvmem = devm_nvmem_register(pdev->dev.parent, &econfig);
+ if (IS_ERR(priv->nvmem))
+ return dev_err_probe(&pdev->dev, PTR_ERR(priv->nvmem),
+ "Failed to probe SIUL2 NVMEM!\n");
+
+ dev_info(&pdev->dev, "Initialized S32G%u SIUL2 nvmem driver\n",
+ part / 100);
+
+ return 0;
+}
+
+static struct platform_driver s32g2_siul2_nvmem_driver = {
+ .probe = s32g2_siul2_nvmem_probe,
+ .driver = {
+ .name = "s32g2-siul2-nvmem",
+ },
+};
+
+module_platform_driver(s32g2_siul2_nvmem_driver);
+
+MODULE_AUTHOR("Catalin Udma <catalin-dan.udma@nxp.com>");
+MODULE_DESCRIPTION("S32G2 SIUL2 NVMEM driver");
+MODULE_LICENSE("GPL");
--
2.45.2
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH v7 10/12] nvmem: s32g2_siul2: add NVMEM driver for SoC information
2025-07-10 14:20 ` [PATCH v7 10/12] nvmem: s32g2_siul2: add NVMEM driver for SoC information Andrei Stefanescu
@ 2025-07-11 5:37 ` Arnd Bergmann
2025-07-11 12:39 ` Andrei Stefanescu
2025-07-18 13:45 ` Lee Jones
1 sibling, 1 reply; 36+ messages in thread
From: Arnd Bergmann @ 2025-07-11 5:37 UTC (permalink / raw)
To: Andrei Stefanescu, Linus Walleij, Bartosz Golaszewski,
Rob Herring, krzk+dt, Conor Dooley, Chester Lin, Matthias Brugger,
Ghennadi Procopciuc, Larisa Grigore, Lee Jones, Shawn Guo,
Sascha Hauer, Fabio Estevam, aisheng.dong, Jacky Bai,
Greg Kroah-Hartman, Rafael J . Wysocki, Srinivas Kandagatla
Cc: open list:GPIO SUBSYSTEM, devicetree, linux-kernel,
linux-arm-kernel, NXP S32 Linux Team, Christophe Lizzi,
Alberto Ruiz, Enric Balletbo, echanude, Pengutronix Kernel Team,
imx, Vincent Guittot
On Thu, Jul 10, 2025, at 16:20, Andrei Stefanescu wrote:
> The SIUL2 hardware module has registers which expose information about
> the given SoC (version, SRAM size, presence of some hw modules).
>
> Signed-off-by: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
This does not look like an nvmem at all, it appears that you
are creating an alternative to the soc_device infrastructure
based on a binary interface tunneled through the nvmem subsystem.
Why not just make this a soc_device and have drivers use
soc_device_match() if they need to know what chip they are
running on?
Arnd
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [PATCH v7 10/12] nvmem: s32g2_siul2: add NVMEM driver for SoC information
2025-07-11 5:37 ` Arnd Bergmann
@ 2025-07-11 12:39 ` Andrei Stefanescu
2025-08-01 14:36 ` Andrei Stefanescu
0 siblings, 1 reply; 36+ messages in thread
From: Andrei Stefanescu @ 2025-07-11 12:39 UTC (permalink / raw)
To: Arnd Bergmann, Linus Walleij, Bartosz Golaszewski, Rob Herring,
krzk+dt, Conor Dooley, Chester Lin, Matthias Brugger,
Ghennadi Procopciuc, Larisa Grigore, Lee Jones, Shawn Guo,
Sascha Hauer, Fabio Estevam, aisheng.dong, Jacky Bai,
Greg Kroah-Hartman, Rafael J . Wysocki, Srinivas Kandagatla
Cc: open list:GPIO SUBSYSTEM, devicetree, linux-kernel,
linux-arm-kernel, NXP S32 Linux Team, Christophe Lizzi,
Alberto Ruiz, Enric Balletbo, echanude, Pengutronix Kernel Team,
imx, Vincent Guittot
Hi Arnd,
>> The SIUL2 hardware module has registers which expose information about
>> the given SoC (version, SRAM size, presence of some hw modules).
>>
>> Signed-off-by: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
>
> This does not look like an nvmem at all, it appears that you
> are creating an alternative to the soc_device infrastructure
> based on a binary interface tunneled through the nvmem subsystem.
>
> Why not just make this a soc_device and have drivers use
> soc_device_match() if they need to know what chip they are
> running on?
Thank you for the review! I've just taken a look over soc_device
and I agree, this driver should be a soc_device. I will convert
it in the next revision.
Best regards,
Andrei
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v7 10/12] nvmem: s32g2_siul2: add NVMEM driver for SoC information
2025-07-11 12:39 ` Andrei Stefanescu
@ 2025-08-01 14:36 ` Andrei Stefanescu
2025-08-02 8:28 ` Krzysztof Kozlowski
0 siblings, 1 reply; 36+ messages in thread
From: Andrei Stefanescu @ 2025-08-01 14:36 UTC (permalink / raw)
To: Arnd Bergmann, Linus Walleij, Bartosz Golaszewski, Rob Herring,
krzk+dt, Conor Dooley, Chester Lin, Matthias Brugger,
Ghennadi Procopciuc, Larisa Grigore, Lee Jones, Shawn Guo,
Sascha Hauer, Fabio Estevam, aisheng.dong, Jacky Bai,
Greg Kroah-Hartman, Rafael J . Wysocki, Srinivas Kandagatla
Cc: open list:GPIO SUBSYSTEM, devicetree, linux-kernel,
linux-arm-kernel, NXP S32 Linux Team, Christophe Lizzi,
Alberto Ruiz, Enric Balletbo, echanude, Pengutronix Kernel Team,
imx, Vincent Guittot
> Thank you for the review! I've just taken a look over soc_device
> and I agree, this driver should be a soc_device. I will convert
> it in the next revision.
>
Hi Arnd,
I took a more in-depth look over soc_device and how to apply it to SIUL2
and I have encountered an issue. Downstream [1], [2], [3] we use SIUL2
nvmem-cells to set the PCIe vendor id partially based on the part number
and also to ensure that the SerDes subsystem is present. I don't think we can
achieve this with a soc_device driver. I saw that we could export a custom
attribute but I don't think we can read it from the PCIe driver.
Apart from the proposed NVMEM driver, there is also an option of exporting
a syscon regmap for the registers which provide information about the SoC.
I have seen that typically NVMEM drivers export information read from fuses
but I think having a NVMEM driver is nicer way to access the information
instead of using a syscon regmap and manually extracting the needed bits.
To provide a bit of context: the SIUL2 IP has two registers called
SIUL2 MCU ID Register (MIDR1/2) which export information such as:
the part number, major, minor, package, maximum frequency,
flash size, SRAM size, SerDes susbsytem presence and so on.
S32G2/3 SoCs have two SIUL2 blocks named SIUL2_0 and SIUL2_1. We need
to export the MIDR1/2 registers of both SIUL2 hardware blocks.
What do you think? Would it be ok to keep the existing NVMEM implementation?
Do you have any other suggestions?
Best regards,
Andrei
[1] - https://github.com/nxp-auto-linux/linux/blob/release/bsp44.0-6.6.85-rt/arch/arm64/boot/dts/freescale/s32cc.dtsi#L1036
[2] - https://github.com/nxp-auto-linux/linux/blob/release/bsp44.0-6.6.85-rt/drivers/pci/controller/dwc/pci-s32cc.c#L832
[3] - https://github.com/nxp-auto-linux/linux/blob/release/bsp44.0-6.6.85-rt/drivers/pci/controller/dwc/pci-s32cc.c#L163
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v7 10/12] nvmem: s32g2_siul2: add NVMEM driver for SoC information
2025-08-01 14:36 ` Andrei Stefanescu
@ 2025-08-02 8:28 ` Krzysztof Kozlowski
2025-08-02 8:32 ` Krzysztof Kozlowski
0 siblings, 1 reply; 36+ messages in thread
From: Krzysztof Kozlowski @ 2025-08-02 8:28 UTC (permalink / raw)
To: Andrei Stefanescu, Arnd Bergmann, Linus Walleij,
Bartosz Golaszewski, Rob Herring, krzk+dt, Conor Dooley,
Chester Lin, Matthias Brugger, Ghennadi Procopciuc,
Larisa Grigore, Lee Jones, Shawn Guo, Sascha Hauer, Fabio Estevam,
aisheng.dong, Jacky Bai, Greg Kroah-Hartman, Rafael J . Wysocki,
Srinivas Kandagatla
Cc: open list:GPIO SUBSYSTEM, devicetree, linux-kernel,
linux-arm-kernel, NXP S32 Linux Team, Christophe Lizzi,
Alberto Ruiz, Enric Balletbo, echanude, Pengutronix Kernel Team,
imx, Vincent Guittot
On 01/08/2025 16:36, Andrei Stefanescu wrote:
> Apart from the proposed NVMEM driver, there is also an option of exporting
> a syscon regmap for the registers which provide information about the SoC.
>
> I have seen that typically NVMEM drivers export information read from fuses
> but I think having a NVMEM driver is nicer way to access the information
> instead of using a syscon regmap and manually extracting the needed bits.
nvmem is not a syscon. Mixing these two means device is something
completely else.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v7 10/12] nvmem: s32g2_siul2: add NVMEM driver for SoC information
2025-08-02 8:28 ` Krzysztof Kozlowski
@ 2025-08-02 8:32 ` Krzysztof Kozlowski
2025-08-04 7:12 ` Andrei Stefanescu
0 siblings, 1 reply; 36+ messages in thread
From: Krzysztof Kozlowski @ 2025-08-02 8:32 UTC (permalink / raw)
To: Andrei Stefanescu, Arnd Bergmann, Linus Walleij,
Bartosz Golaszewski, Rob Herring, krzk+dt, Conor Dooley,
Chester Lin, Matthias Brugger, Ghennadi Procopciuc,
Larisa Grigore, Lee Jones, Shawn Guo, Sascha Hauer, Fabio Estevam,
aisheng.dong, Jacky Bai, Greg Kroah-Hartman, Rafael J . Wysocki,
Srinivas Kandagatla
Cc: open list:GPIO SUBSYSTEM, devicetree, linux-kernel,
linux-arm-kernel, NXP S32 Linux Team, Christophe Lizzi,
Alberto Ruiz, Enric Balletbo, echanude, Pengutronix Kernel Team,
imx, Vincent Guittot
On 02/08/2025 10:28, Krzysztof Kozlowski wrote:
> On 01/08/2025 16:36, Andrei Stefanescu wrote:
>> Apart from the proposed NVMEM driver, there is also an option of exporting
>> a syscon regmap for the registers which provide information about the SoC.
>>
>> I have seen that typically NVMEM drivers export information read from fuses
>> but I think having a NVMEM driver is nicer way to access the information
>> instead of using a syscon regmap and manually extracting the needed bits.
>
>
> nvmem is not a syscon. Mixing these two means device is something
> completely else.
... and yes, I am aware that three FSL/NXP bindings use nvmem as syscon
already. People are mixing hardware description (nvmem) with some
purpose for drivers (syscon) forgetting that syscon are miscellaneous
SoC internal registers, not non-volatile memory.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v7 10/12] nvmem: s32g2_siul2: add NVMEM driver for SoC information
2025-08-02 8:32 ` Krzysztof Kozlowski
@ 2025-08-04 7:12 ` Andrei Stefanescu
2025-08-04 7:26 ` Krzysztof Kozlowski
0 siblings, 1 reply; 36+ messages in thread
From: Andrei Stefanescu @ 2025-08-04 7:12 UTC (permalink / raw)
To: Krzysztof Kozlowski, Arnd Bergmann, Linus Walleij,
Bartosz Golaszewski, Rob Herring, krzk+dt, Conor Dooley,
Chester Lin, Matthias Brugger, Ghennadi Procopciuc,
Larisa Grigore, Lee Jones, Shawn Guo, Sascha Hauer, Fabio Estevam,
aisheng.dong, Jacky Bai, Greg Kroah-Hartman, Rafael J . Wysocki,
Srinivas Kandagatla
Cc: open list:GPIO SUBSYSTEM, devicetree, linux-kernel,
linux-arm-kernel, NXP S32 Linux Team, Christophe Lizzi,
Alberto Ruiz, Enric Balletbo, echanude, Pengutronix Kernel Team,
imx, Vincent Guittot
Hi Krzysztof,
Thank you for the quick response!
On 02/08/2025 11:32, Krzysztof Kozlowski wrote:
> On 02/08/2025 10:28, Krzysztof Kozlowski wrote:
>> On 01/08/2025 16:36, Andrei Stefanescu wrote:
>>> Apart from the proposed NVMEM driver, there is also an option of exporting
>>> a syscon regmap for the registers which provide information about the SoC.
>>>
>>> I have seen that typically NVMEM drivers export information read from fuses
>>> but I think having a NVMEM driver is nicer way to access the information
>>> instead of using a syscon regmap and manually extracting the needed bits.
>>
>>
>> nvmem is not a syscon. Mixing these two means device is something
>> completely else.
Yes, I don't want to mix them. The driver will either be a NVMEM driver or
a syscon. These registers are read-only. I suggested NVMEM because it's a
an abstraction layer which makes it easier for drivers which want to use
that information without knowing where to actually read it i.e. reg address,
bit mask.
What do think? Which one would be better? Do you have other suggestions?
Best regards,
Andrei
>
>
> ... and yes, I am aware that three FSL/NXP bindings use nvmem as syscon
> already. People are mixing hardware description (nvmem) with some
> purpose for drivers (syscon) forgetting that syscon are miscellaneous
> SoC internal registers, not non-volatile memory.
>
> Best regards,
> Krzysztof
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v7 10/12] nvmem: s32g2_siul2: add NVMEM driver for SoC information
2025-08-04 7:12 ` Andrei Stefanescu
@ 2025-08-04 7:26 ` Krzysztof Kozlowski
2025-08-05 12:53 ` Andrei Stefanescu
0 siblings, 1 reply; 36+ messages in thread
From: Krzysztof Kozlowski @ 2025-08-04 7:26 UTC (permalink / raw)
To: Andrei Stefanescu, Arnd Bergmann, Linus Walleij,
Bartosz Golaszewski, Rob Herring, krzk+dt, Conor Dooley,
Chester Lin, Matthias Brugger, Ghennadi Procopciuc,
Larisa Grigore, Lee Jones, Shawn Guo, Sascha Hauer, Fabio Estevam,
aisheng.dong, Jacky Bai, Greg Kroah-Hartman, Rafael J . Wysocki,
Srinivas Kandagatla
Cc: open list:GPIO SUBSYSTEM, devicetree, linux-kernel,
linux-arm-kernel, NXP S32 Linux Team, Christophe Lizzi,
Alberto Ruiz, Enric Balletbo, echanude, Pengutronix Kernel Team,
imx, Vincent Guittot
On 04/08/2025 09:12, Andrei Stefanescu wrote:
> Hi Krzysztof,
>
> Thank you for the quick response!
> On 02/08/2025 11:32, Krzysztof Kozlowski wrote:
>> On 02/08/2025 10:28, Krzysztof Kozlowski wrote:
>>> On 01/08/2025 16:36, Andrei Stefanescu wrote:
>>>> Apart from the proposed NVMEM driver, there is also an option of exporting
>>>> a syscon regmap for the registers which provide information about the SoC.
>>>>
>>>> I have seen that typically NVMEM drivers export information read from fuses
>>>> but I think having a NVMEM driver is nicer way to access the information
>>>> instead of using a syscon regmap and manually extracting the needed bits.
>>>
>>>
>>> nvmem is not a syscon. Mixing these two means device is something
>>> completely else.
>
> Yes, I don't want to mix them. The driver will either be a NVMEM driver or
> a syscon. These registers are read-only. I suggested NVMEM because it's a
We do not talk about drivers here, but hardware.
> an abstraction layer which makes it easier for drivers which want to use
> that information without knowing where to actually read it i.e. reg address,
> bit mask.
Sorry, but no. You design it for drivers, that's not the way. Describe
properly the hardware.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v7 10/12] nvmem: s32g2_siul2: add NVMEM driver for SoC information
2025-08-04 7:26 ` Krzysztof Kozlowski
@ 2025-08-05 12:53 ` Andrei Stefanescu
0 siblings, 0 replies; 36+ messages in thread
From: Andrei Stefanescu @ 2025-08-05 12:53 UTC (permalink / raw)
To: Krzysztof Kozlowski, Arnd Bergmann, Linus Walleij,
Bartosz Golaszewski, Rob Herring, krzk+dt, Conor Dooley,
Chester Lin, Matthias Brugger, Ghennadi Procopciuc,
Larisa Grigore, Lee Jones, Shawn Guo, Sascha Hauer, Fabio Estevam,
aisheng.dong, Jacky Bai, Greg Kroah-Hartman, Rafael J . Wysocki,
Srinivas Kandagatla
Cc: open list:GPIO SUBSYSTEM, devicetree, linux-kernel,
linux-arm-kernel, NXP S32 Linux Team, Christophe Lizzi,
Alberto Ruiz, Enric Balletbo, echanude, Pengutronix Kernel Team,
imx, Vincent Guittot
Hi Krzysztof,
On 04/08/2025 10:26, Krzysztof Kozlowski wrote:
> On 04/08/2025 09:12, Andrei Stefanescu wrote:
>> Hi Krzysztof,
>>
>> Thank you for the quick response!
>> On 02/08/2025 11:32, Krzysztof Kozlowski wrote:
>>> On 02/08/2025 10:28, Krzysztof Kozlowski wrote:
>>>> On 01/08/2025 16:36, Andrei Stefanescu wrote:
>>>>> Apart from the proposed NVMEM driver, there is also an option of exporting
>>>>> a syscon regmap for the registers which provide information about the SoC.
>>>>>
>>>>> I have seen that typically NVMEM drivers export information read from fuses
>>>>> but I think having a NVMEM driver is nicer way to access the information
>>>>> instead of using a syscon regmap and manually extracting the needed bits.
>>>>
>>>>
>>>> nvmem is not a syscon. Mixing these two means device is something
>>>> completely else.
>>
>> Yes, I don't want to mix them. The driver will either be a NVMEM driver or
>> a syscon. These registers are read-only. I suggested NVMEM because it's a
>
> We do not talk about drivers here, but hardware.
>
>> an abstraction layer which makes it easier for drivers which want to use
>> that information without knowing where to actually read it i.e. reg address,
>> bit mask.
>
> Sorry, but no. You design it for drivers, that's not the way. Describe
> properly the hardware.
I don't think there's a clear way. These are read-only registers, I am not sure
if exporting them as a syscon regmap properly describes the hardware. Moreover,
I saw the following phrase in the NVMEM documentation [1]:
"NVMEM is the abbreviation for Non Volatile Memory layer. It is used to retrieve
configuration of SOC or Device specific data from non volatile memories like eeprom,
efuses and so on."
This suggests that NVMEM might be a fit. I incline more towards the NVVMEM driver
because it will also reduce code duplication (if multiple drivers need to read the
part number for example).
What do you think? Would you consider NVMEM the way to go in this case?
Best regards,
Andrei
[1] - https://www.kernel.org/doc/html/latest/driver-api/nvmem.html
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v7 10/12] nvmem: s32g2_siul2: add NVMEM driver for SoC information
2025-07-10 14:20 ` [PATCH v7 10/12] nvmem: s32g2_siul2: add NVMEM driver for SoC information Andrei Stefanescu
2025-07-11 5:37 ` Arnd Bergmann
@ 2025-07-18 13:45 ` Lee Jones
1 sibling, 0 replies; 36+ messages in thread
From: Lee Jones @ 2025-07-18 13:45 UTC (permalink / raw)
To: Andrei Stefanescu
Cc: linus.walleij, brgl, robh, krzk+dt, conor+dt, chester62515,
mbrugger, Ghennadi.Procopciuc, larisa.grigore, shawnguo, s.hauer,
festevam, aisheng.dong, ping.bai, gregkh, rafael, srini,
linux-gpio, devicetree, linux-kernel, linux-arm-kernel, s32,
clizzi, aruizrui, eballetb, echanude, kernel, imx,
vincent.guittot
On Thu, 10 Jul 2025, Andrei Stefanescu wrote:
> The SIUL2 hardware module has registers which expose information about
> the given SoC (version, SRAM size, presence of some hw modules).
>
> Signed-off-by: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
> ---
> drivers/mfd/nxp-siul2.c | 6 +-
When you re-submit - this needs to be in its own commit.
> drivers/nvmem/Kconfig | 10 ++
> drivers/nvmem/Makefile | 2 +
> drivers/nvmem/s32g2_siul2_nvmem.c | 232 ++++++++++++++++++++++++++++++
> 4 files changed, 249 insertions(+), 1 deletion(-)
> create mode 100644 drivers/nvmem/s32g2_siul2_nvmem.c
--
Lee Jones [李琼斯]
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v7 11/12] MAINTAINERS: add MAINTAINER for NXP SIUL2 NVMEM cell
2025-07-10 14:20 [PATCH v7 00/12] gpio: siul2-s32g2: add initial GPIO driver Andrei Stefanescu
` (9 preceding siblings ...)
2025-07-10 14:20 ` [PATCH v7 10/12] nvmem: s32g2_siul2: add NVMEM driver for SoC information Andrei Stefanescu
@ 2025-07-10 14:20 ` Andrei Stefanescu
2025-07-10 14:20 ` [PATCH v7 12/12] pinctrl: s32cc: set num_custom_params to 0 Andrei Stefanescu
2025-07-10 19:05 ` [PATCH v7 00/12] gpio: siul2-s32g2: add initial GPIO driver Rob Herring (Arm)
12 siblings, 0 replies; 36+ messages in thread
From: Andrei Stefanescu @ 2025-07-10 14:20 UTC (permalink / raw)
To: linus.walleij, brgl, robh, krzk+dt, conor+dt, chester62515,
mbrugger, Ghennadi.Procopciuc, larisa.grigore, lee, shawnguo,
s.hauer, festevam, aisheng.dong, ping.bai, gregkh, rafael, srini
Cc: linux-gpio, devicetree, linux-kernel, linux-arm-kernel, s32,
clizzi, aruizrui, eballetb, echanude, kernel, imx,
vincent.guittot, Andrei Stefanescu
Add the new NVMEM MFD cell for the SIUL2 module under the NXP S32G existing
entry. This driver exposes information about the SoC.
Signed-off-by: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
---
MAINTAINERS | 1 +
1 file changed, 1 insertion(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 3e9b54bbcdb8..2342471d2ac4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3003,6 +3003,7 @@ S: Maintained
F: Documentation/devicetree/bindings/mfd/nxp,s32g2-siul2.yaml
F: arch/arm64/boot/dts/freescale/s32g*.dts*
F: drivers/mfd/nxp-siul2.c
+F: drivers/nvmem/s32g2_siul2_nvmem.c
F: drivers/pinctrl/nxp/
ARM/NXP S32G/S32R DWMAC ETHERNET DRIVER
--
2.45.2
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PATCH v7 12/12] pinctrl: s32cc: set num_custom_params to 0
2025-07-10 14:20 [PATCH v7 00/12] gpio: siul2-s32g2: add initial GPIO driver Andrei Stefanescu
` (10 preceding siblings ...)
2025-07-10 14:20 ` [PATCH v7 11/12] MAINTAINERS: add MAINTAINER for NXP SIUL2 NVMEM cell Andrei Stefanescu
@ 2025-07-10 14:20 ` Andrei Stefanescu
2025-07-10 19:05 ` [PATCH v7 00/12] gpio: siul2-s32g2: add initial GPIO driver Rob Herring (Arm)
12 siblings, 0 replies; 36+ messages in thread
From: Andrei Stefanescu @ 2025-07-10 14:20 UTC (permalink / raw)
To: linus.walleij, brgl, robh, krzk+dt, conor+dt, chester62515,
mbrugger, Ghennadi.Procopciuc, larisa.grigore, lee, shawnguo,
s.hauer, festevam, aisheng.dong, ping.bai, gregkh, rafael, srini
Cc: linux-gpio, devicetree, linux-kernel, linux-arm-kernel, s32,
clizzi, aruizrui, eballetb, echanude, kernel, imx,
vincent.guittot, Andrei Stefanescu
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 8e9da792d035..097f74904c34 100644
--- a/drivers/pinctrl/nxp/pinctrl-s32cc.c
+++ b/drivers/pinctrl/nxp/pinctrl-s32cc.c
@@ -1273,6 +1273,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.45.2
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH v7 00/12] gpio: siul2-s32g2: add initial GPIO driver
2025-07-10 14:20 [PATCH v7 00/12] gpio: siul2-s32g2: add initial GPIO driver Andrei Stefanescu
` (11 preceding siblings ...)
2025-07-10 14:20 ` [PATCH v7 12/12] pinctrl: s32cc: set num_custom_params to 0 Andrei Stefanescu
@ 2025-07-10 19:05 ` Rob Herring (Arm)
12 siblings, 0 replies; 36+ messages in thread
From: Rob Herring (Arm) @ 2025-07-10 19:05 UTC (permalink / raw)
To: Andrei Stefanescu
Cc: imx, s32, eballetb, festevam, linux-arm-kernel, rafael, shawnguo,
clizzi, brgl, aisheng.dong, devicetree, s.hauer,
Ghennadi.Procopciuc, linus.walleij, gregkh, ping.bai, echanude,
aruizrui, mbrugger, chester62515, conor+dt, srini, larisa.grigore,
krzk+dt, linux-kernel, lee, linux-gpio, vincent.guittot, kernel
On Thu, 10 Jul 2025 17:20:23 +0300, Andrei Stefanescu 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
>
> These will be excluded via the `gpio-reserved-ranges`
> property.
>
> 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.
>
> I have other patches for this driver:
> - interrupt support
> - power management callbacks
>
> which I plan to upstream after this series gets merged
> in order to simplify the review process.
>
> 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 (12):
> 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: small refactoring
> pinctrl: s32cc: change to "devm_pinctrl_register_and_init"
> dt-bindings: pinctrl: deprecate SIUL2 pinctrl bindings
> 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
> nvmem: s32g2_siul2: add NVMEM driver for SoC information
> MAINTAINERS: add MAINTAINER for NXP SIUL2 NVMEM cell
> pinctrl: s32cc: set num_custom_params to 0
>
> .../bindings/mfd/nxp,s32g2-siul2.yaml | 163 +++++
> .../pinctrl/nxp,s32g2-siul2-pinctrl.yaml | 2 +
> MAINTAINERS | 3 +
> arch/arm64/boot/dts/freescale/s32g2.dtsi | 48 +-
> arch/arm64/boot/dts/freescale/s32g3.dtsi | 48 +-
> drivers/mfd/Kconfig | 12 +
> drivers/mfd/Makefile | 1 +
> drivers/mfd/nxp-siul2.c | 414 +++++++++++
> drivers/nvmem/Kconfig | 10 +
> drivers/nvmem/Makefile | 2 +
> drivers/nvmem/s32g2_siul2_nvmem.c | 232 +++++++
> 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 ++
> 15 files changed, 1502 insertions(+), 176 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/mfd/nxp,s32g2-siul2.yaml
> create mode 100644 drivers/mfd/nxp-siul2.c
> create mode 100644 drivers/nvmem/s32g2_siul2_nvmem.c
> create mode 100644 include/linux/mfd/nxp-siul2.h
>
> --
> 2.45.2
>
>
>
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.16-rc3-116-ge34a79b96ab9 (exact match)
If this is not the correct base, please add 'base-commit' tag
(or use b4 which does this automatically)
New warnings running 'make CHECK_DTBS=y for arch/arm64/boot/dts/freescale/' for 20250710142038.1986052-1-andrei.stefanescu@oss.nxp.com:
arch/arm64/boot/dts/freescale/s32g399a-rdb3.dtb: pinctrl@4009c000 (nxp,s32g3-siul2): 'gpio-reserved-ranges' does not match any of the regexes: '-hog(-[0-9]+)?$', '-pins$', '^pinctrl-[0-9]+$'
from schema $id: http://devicetree.org/schemas/mfd/nxp,s32g2-siul2.yaml#
arch/arm64/boot/dts/freescale/s32g274a-evb.dtb: pinctrl@4009c000 (nxp,s32g2-siul2): 'gpio-reserved-ranges' does not match any of the regexes: '-hog(-[0-9]+)?$', '-pins$', '^pinctrl-[0-9]+$'
from schema $id: http://devicetree.org/schemas/mfd/nxp,s32g2-siul2.yaml#
arch/arm64/boot/dts/freescale/s32g274a-rdb2.dtb: pinctrl@4009c000 (nxp,s32g2-siul2): 'gpio-reserved-ranges' does not match any of the regexes: '-hog(-[0-9]+)?$', '-pins$', '^pinctrl-[0-9]+$'
from schema $id: http://devicetree.org/schemas/mfd/nxp,s32g2-siul2.yaml#
^ permalink raw reply [flat|nested] 36+ messages in thread