linux-gpio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 0/7] gpio: siul2-s32g2: add initial GPIO driver
@ 2024-11-01  8:06 Andrei Stefanescu
  2024-11-01  8:06 ` [PATCH v5 1/7] dt-bindings: mfd: add support for the NXP SIUL2 module Andrei Stefanescu
                   ` (7 more replies)
  0 siblings, 8 replies; 32+ messages in thread
From: Andrei Stefanescu @ 2024-11-01  8:06 UTC (permalink / raw)
  To: Linus Walleij, Bartosz Golaszewski, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Chester Lin, Matthias Brugger,
	Ghennadi Procopciuc, Larisa Grigore, Andrei Stefanescu,
	Greg Kroah-Hartman, Rafael J. Wysocki, Lee Jones, Shawn Guo,
	Sascha Hauer, Fabio Estevam, Dong Aisheng, Jacky Bai
  Cc: linux-gpio, devicetree, linux-kernel, linux-arm-kernel,
	NXP S32 Linux Team, Christophe Lizzi, Alberto Ruiz,
	Enric Balletbo, Pengutronix Kernel Team, imx

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.

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 (7):
  dt-bindings: mfd: add support for the NXP SIUL2 module
  mfd: nxp-siul2: add support for NXP SIUL2
  arm64: dts: s32g: make pinctrl part of mfd node
  pinctrl: s32: convert the driver into an mfd cell
  pinctrl: s32cc: change to "devm_pinctrl_register_and_init"
  pinctrl: s32cc: add driver for GPIO functionality
  MAINTAINERS: add MAINTAINER for NXP SIUL2 MFD driver

 .../devicetree/bindings/mfd/nxp,siul2.yaml    | 191 +++++++
 MAINTAINERS                                   |   2 +
 arch/arm64/boot/dts/freescale/s32g2.dtsi      |  26 +-
 arch/arm64/boot/dts/freescale/s32g3.dtsi      |  26 +-
 drivers/mfd/Kconfig                           |  12 +
 drivers/mfd/Makefile                          |   1 +
 drivers/mfd/nxp-siul2.c                       | 411 +++++++++++++++
 drivers/pinctrl/nxp/pinctrl-s32.h             |   1 +
 drivers/pinctrl/nxp/pinctrl-s32cc.c           | 498 ++++++++++++++----
 drivers/pinctrl/nxp/pinctrl-s32g2.c           |  23 +-
 include/linux/mfd/nxp-siul2.h                 |  55 ++
 11 files changed, 1086 insertions(+), 160 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mfd/nxp,siul2.yaml
 create mode 100644 drivers/mfd/nxp-siul2.c
 create mode 100644 include/linux/mfd/nxp-siul2.h

-- 
2.45.2


^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v5 1/7] dt-bindings: mfd: add support for the NXP SIUL2 module
  2024-11-01  8:06 [PATCH v5 0/7] gpio: siul2-s32g2: add initial GPIO driver Andrei Stefanescu
@ 2024-11-01  8:06 ` Andrei Stefanescu
  2024-11-01 18:11   ` Frank Li
  2024-11-02  8:42   ` Krzysztof Kozlowski
  2024-11-01  8:06 ` [PATCH v5 2/7] mfd: nxp-siul2: add support for NXP SIUL2 Andrei Stefanescu
                   ` (6 subsequent siblings)
  7 siblings, 2 replies; 32+ messages in thread
From: Andrei Stefanescu @ 2024-11-01  8:06 UTC (permalink / raw)
  To: Linus Walleij, Bartosz Golaszewski, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Chester Lin, Matthias Brugger,
	Ghennadi Procopciuc, Larisa Grigore, Andrei Stefanescu,
	Greg Kroah-Hartman, Rafael J. Wysocki, Lee Jones, Shawn Guo,
	Sascha Hauer, Fabio Estevam, Dong Aisheng, Jacky Bai
  Cc: linux-gpio, devicetree, linux-kernel, linux-arm-kernel,
	NXP S32 Linux Team, Christophe Lizzi, Alberto Ruiz,
	Enric Balletbo, Pengutronix Kernel Team, imx

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>
---
 .../devicetree/bindings/mfd/nxp,siul2.yaml    | 191 ++++++++++++++++++
 1 file changed, 191 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/nxp,siul2.yaml

diff --git a/Documentation/devicetree/bindings/mfd/nxp,siul2.yaml b/Documentation/devicetree/bindings/mfd/nxp,siul2.yaml
new file mode 100644
index 000000000000..141ec1219821
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/nxp,siul2.yaml
@@ -0,0 +1,191 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright 2024 NXP
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/nxp,siul2.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: 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:
+    enum:
+      - nxp,s32g2-siul2
+      - nxp,s32g3-siul2
+
+  reg:
+    items:
+      - description: SIUL2_0 module memory
+      - description: SIUL2_1 module memory
+
+  reg-names:
+    items:
+      - const: siul20
+      - const: siul21
+
+  gpio-controller: true
+
+  '#gpio-cells':
+    const: 2
+
+  gpio-ranges:
+    minItems: 2
+    maxItems: 2
+
+  gpio-reserved-ranges:
+    minItems: 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]
+
+        additionalProperties: false
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - gpio-controller
+  - "#gpio-cells"
+  - gpio-ranges
+  - gpio-reserved-ranges
+  - interrupts
+  - interrupt-controller
+  - "#interrupt-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    siul2: siul2@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>;
+      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 {
+          pinmux = <0x0>;
+          input-enable;
+          bias-pull-up;
+          slew-rate = <166>;
+        };
+
+        jtag-grp1 {
+          pinmux = <0x11>;
+          slew-rate = <166>;
+        };
+
+        jtag-grp2 {
+          pinmux = <0x40>;
+          input-enable;
+          bias-pull-down;
+          slew-rate = <166>;
+        };
+
+        jtag-grp3 {
+          pinmux = <0x23c0>,
+                   <0x23d0>,
+                   <0x2320>;
+        };
+
+        jtag-grp4 {
+          pinmux = <0x51>;
+          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] 32+ messages in thread

* [PATCH v5 2/7] mfd: nxp-siul2: add support for NXP SIUL2
  2024-11-01  8:06 [PATCH v5 0/7] gpio: siul2-s32g2: add initial GPIO driver Andrei Stefanescu
  2024-11-01  8:06 ` [PATCH v5 1/7] dt-bindings: mfd: add support for the NXP SIUL2 module Andrei Stefanescu
@ 2024-11-01  8:06 ` Andrei Stefanescu
  2024-11-01 17:18   ` kernel test robot
                     ` (4 more replies)
  2024-11-01  8:06 ` [PATCH v5 3/7] arm64: dts: s32g: make pinctrl part of mfd node Andrei Stefanescu
                   ` (5 subsequent siblings)
  7 siblings, 5 replies; 32+ messages in thread
From: Andrei Stefanescu @ 2024-11-01  8:06 UTC (permalink / raw)
  To: Linus Walleij, Bartosz Golaszewski, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Chester Lin, Matthias Brugger,
	Ghennadi Procopciuc, Larisa Grigore, Andrei Stefanescu,
	Greg Kroah-Hartman, Rafael J. Wysocki, Lee Jones, Shawn Guo,
	Sascha Hauer, Fabio Estevam, Dong Aisheng, Jacky Bai
  Cc: linux-gpio, devicetree, linux-kernel, linux-arm-kernel,
	NXP S32 Linux Team, Christophe Lizzi, Alberto Ruiz,
	Enric Balletbo, Pengutronix Kernel Team, imx

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

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       | 411 ++++++++++++++++++++++++++++++++++
 include/linux/mfd/nxp-siul2.h |  55 +++++
 4 files changed, 479 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 f9325bcce1b9..fc590789e8b3 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1098,6 +1098,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 2a9f91e81af8..7b19ea014221 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -226,6 +226,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..ba13d1beb244
--- /dev/null
+++ b/drivers/mfd/nxp-siul2.c
@@ -0,0 +1,411 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * SIUL2(System Integration Unit Lite) MFD driver
+ *
+ * Copyright 2024 NXP
+ */
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/of.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/nxp-siul2.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_start_offset: the first valid register offset
+ * @reg_end_offset: the last valid register offset
+ * @reg_access: the read/write access tables if not NULL
+ * @valid: whether the register range is valid or not
+ */
+struct nxp_siul2_reg_range_info {
+	const char *reg_name;
+	unsigned int reg_start_offset;
+	unsigned int reg_end_offset;
+	const struct regmap_access_table *reg_access;
+	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_MPIDR] = S32_REG_RANGE(4, 8, "SIUL2_0_MPIDR", 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_MPIDR] = S32_REG_RANGE(4, 8, "SIUL2_1_MPIDR", 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, int siul)
+{
+	struct regmap_config regmap_configs[SIUL2_NUM_REG_TYPES] = {
+		[SIUL2_MPIDR]	= 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, ret;
+
+	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 =
+				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])) {
+			dev_err(&pdev->dev, "regmap %d init failed: %d\n", i,
+				ret);
+			return 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)) {
+			dev_err(&pdev->dev, "Failed to get MEM resource: %s\n",
+				reg_name);
+			return PTR_ERR(base);
+		}
+
+		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) {
+			dev_err(&pdev->dev, "Invalid pinspec count: %d\n",
+				pinspec.args_count);
+			return -EINVAL;
+		}
+
+		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" },
+	{ },
+};
+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..238c812dba29
--- /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 2024 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_MPIDR - 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_MPIDR,
+	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] 32+ messages in thread

* [PATCH v5 3/7] arm64: dts: s32g: make pinctrl part of mfd node
  2024-11-01  8:06 [PATCH v5 0/7] gpio: siul2-s32g2: add initial GPIO driver Andrei Stefanescu
  2024-11-01  8:06 ` [PATCH v5 1/7] dt-bindings: mfd: add support for the NXP SIUL2 module Andrei Stefanescu
  2024-11-01  8:06 ` [PATCH v5 2/7] mfd: nxp-siul2: add support for NXP SIUL2 Andrei Stefanescu
@ 2024-11-01  8:06 ` Andrei Stefanescu
  2024-11-01 18:22   ` Frank Li
  2024-11-02  8:49   ` Krzysztof Kozlowski
  2024-11-01  8:06 ` [PATCH v5 4/7] pinctrl: s32: convert the driver into an mfd cell Andrei Stefanescu
                   ` (4 subsequent siblings)
  7 siblings, 2 replies; 32+ messages in thread
From: Andrei Stefanescu @ 2024-11-01  8:06 UTC (permalink / raw)
  To: Linus Walleij, Bartosz Golaszewski, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Chester Lin, Matthias Brugger,
	Ghennadi Procopciuc, Larisa Grigore, Andrei Stefanescu,
	Greg Kroah-Hartman, Rafael J. Wysocki, Lee Jones, Shawn Guo,
	Sascha Hauer, Fabio Estevam, Dong Aisheng, Jacky Bai
  Cc: linux-gpio, devicetree, linux-kernel, linux-arm-kernel,
	NXP S32 Linux Team, Christophe Lizzi, Alberto Ruiz,
	Enric Balletbo, Pengutronix Kernel Team, imx

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

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

diff --git a/arch/arm64/boot/dts/freescale/s32g2.dtsi b/arch/arm64/boot/dts/freescale/s32g2.dtsi
index fa054bfe7d5c..e14ce5503e1f 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>;
+		siul2: siul2@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>;
+			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 {
diff --git a/arch/arm64/boot/dts/freescale/s32g3.dtsi b/arch/arm64/boot/dts/freescale/s32g3.dtsi
index b4226a9143c8..fa43d036686f 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>;
+		siul2: siul2@4009c000 {
+			compatible = "nxp,s32g3-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>;
+			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 {
-- 
2.45.2


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH v5 4/7] pinctrl: s32: convert the driver into an mfd cell
  2024-11-01  8:06 [PATCH v5 0/7] gpio: siul2-s32g2: add initial GPIO driver Andrei Stefanescu
                   ` (2 preceding siblings ...)
  2024-11-01  8:06 ` [PATCH v5 3/7] arm64: dts: s32g: make pinctrl part of mfd node Andrei Stefanescu
@ 2024-11-01  8:06 ` Andrei Stefanescu
  2024-11-01 11:53   ` Linus Walleij
                     ` (2 more replies)
  2024-11-01  8:06 ` [PATCH v5 5/7] pinctrl: s32cc: change to "devm_pinctrl_register_and_init" Andrei Stefanescu
                   ` (3 subsequent siblings)
  7 siblings, 3 replies; 32+ messages in thread
From: Andrei Stefanescu @ 2024-11-01  8:06 UTC (permalink / raw)
  To: Linus Walleij, Bartosz Golaszewski, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Chester Lin, Matthias Brugger,
	Ghennadi Procopciuc, Larisa Grigore, Andrei Stefanescu,
	Greg Kroah-Hartman, Rafael J. Wysocki, Lee Jones, Shawn Guo,
	Sascha Hauer, Fabio Estevam, Dong Aisheng, Jacky Bai
  Cc: linux-gpio, devicetree, linux-kernel, linux-arm-kernel,
	NXP S32 Linux Team, Christophe Lizzi, Alberto Ruiz,
	Enric Balletbo, Pengutronix Kernel Team, imx

The SIUL2 module is now represented as an mfd device. The pinctrl driver
is now an mfd_cell. Therefore, remove its compatible and adjust its
probing in order to get the necessary information from its mfd parent.

Signed-off-by: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
---
 drivers/pinctrl/nxp/pinctrl-s32.h   |  1 +
 drivers/pinctrl/nxp/pinctrl-s32cc.c | 75 +++++++++++------------------
 drivers/pinctrl/nxp/pinctrl-s32g2.c | 23 ++-------
 3 files changed, 33 insertions(+), 66 deletions(-)

diff --git a/drivers/pinctrl/nxp/pinctrl-s32.h b/drivers/pinctrl/nxp/pinctrl-s32.h
index add3c77ddfed..829211741050 100644
--- a/drivers/pinctrl/nxp/pinctrl-s32.h
+++ b/drivers/pinctrl/nxp/pinctrl-s32.h
@@ -38,6 +38,7 @@ struct s32_pinctrl_soc_data {
 	const struct pinctrl_pin_desc *pins;
 	unsigned int npins;
 	const struct s32_pin_range *mem_pin_ranges;
+	const struct regmap **regmaps;
 	unsigned int mem_regions;
 };
 
diff --git a/drivers/pinctrl/nxp/pinctrl-s32cc.c b/drivers/pinctrl/nxp/pinctrl-s32cc.c
index 501eb296c760..709e823b9c7c 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>
@@ -44,12 +45,6 @@ enum s32_write_type {
 	S32_PINCONF_OVERWRITE,
 };
 
-static struct regmap_config s32_regmap_config = {
-	.reg_bits = 32,
-	.val_bits = 32,
-	.reg_stride = 4,
-};
-
 static u32 get_pin_no(u32 pinmux)
 {
 	return (pinmux & S32_PIN_ID_MASK) >> S32_PIN_ID_SHIFT;
@@ -85,14 +80,15 @@ 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
+ * @saved_context: Configuration saved over system sleep
  */
 struct s32_pinctrl {
 	struct device *dev;
@@ -123,14 +119,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,
-			   unsigned int pin, unsigned int *val)
+static int s32_regmap_read(struct pinctrl_dev *pctldev, unsigned int pin,
+			   unsigned int *val)
 {
 	struct s32_pinctrl_mem_region *region;
 	unsigned int offset;
@@ -145,7 +140,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 +158,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;
@@ -475,8 +470,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:
@@ -838,20 +833,21 @@ static int s32_pinctrl_parse_functions(struct device_node *np,
 static int s32_pinctrl_probe_dt(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;
-	struct device_node *np = pdev->dev.of_node;
-	struct resource *res;
-	struct regmap *map;
-	void __iomem *base;
-	unsigned int mem_regions = info->soc_data->mem_regions;
+	unsigned int mem_regions;
+	struct device_node *np;
+	u32 nfuncs = 0, i = 0, j;
+	u8 regmap_type;
 	int ret;
-	u32 nfuncs = 0;
-	u32 i = 0;
 
+	np = pdev->dev.parent->of_node;
 	if (!np)
 		return -ENODEV;
 
-	if (mem_regions == 0 || mem_regions >= 10000) {
+	/* one MSCR and one IMCR region per SIUL2 module */
+	mem_regions =  info->soc_data->mem_regions;
+	if (mem_regions != mfd->num_siul2 * 2) {
 		dev_err(&pdev->dev, "mem_regions is invalid: %u\n", mem_regions);
 		return -EINVAL;
 	}
@@ -861,26 +857,11 @@ static int s32_pinctrl_probe_dt(struct platform_device *pdev,
 	if (!ipctl->regions)
 		return -ENOMEM;
 
+	/* Order is MSCR regions first, then IMCR ones */
 	for (i = 0; i < mem_regions; i++) {
-		base = devm_platform_get_and_ioremap_resource(pdev, i, &res);
-		if (IS_ERR(base))
-			return PTR_ERR(base);
-
-		snprintf(ipctl->regions[i].name,
-			 sizeof(ipctl->regions[i].name), "map%u", i);
-
-		s32_regmap_config.name = ipctl->regions[i].name;
-		s32_regmap_config.max_register = resource_size(res) -
-						 s32_regmap_config.reg_stride;
-
-		map = devm_regmap_init_mmio(&pdev->dev, base,
-						&s32_regmap_config);
-		if (IS_ERR(map)) {
-			dev_err(&pdev->dev, "Failed to init regmap[%u]\n", i);
-			return PTR_ERR(map);
-		}
-
-		ipctl->regions[i].map = map;
+		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];
 	}
 
@@ -918,13 +899,13 @@ 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");
diff --git a/drivers/pinctrl/nxp/pinctrl-s32g2.c b/drivers/pinctrl/nxp/pinctrl-s32g2.c
index 440ff1879424..9c7fe545cc85 100644
--- a/drivers/pinctrl/nxp/pinctrl-s32g2.c
+++ b/drivers/pinctrl/nxp/pinctrl-s32g2.c
@@ -10,6 +10,7 @@
 #include <linux/err.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>
@@ -713,12 +714,10 @@ static const struct pinctrl_pin_desc s32_pinctrl_pads_siul2[] = {
 static const struct s32_pin_range s32_pin_ranges_siul2[] = {
 	/* MSCR pin ID ranges */
 	S32_PIN_RANGE(0, 101),
-	S32_PIN_RANGE(112, 122),
-	S32_PIN_RANGE(144, 190),
+	S32_PIN_RANGE(112, 190),
 	/* IMCR pin ID ranges */
 	S32_PIN_RANGE(512, 595),
-	S32_PIN_RANGE(631, 909),
-	S32_PIN_RANGE(942, 1007),
+	S32_PIN_RANGE(631, 1007),
 };
 
 static const struct s32_pinctrl_soc_data s32_pinctrl_data = {
@@ -728,22 +727,9 @@ static const struct s32_pinctrl_soc_data s32_pinctrl_data = {
 	.mem_regions = ARRAY_SIZE(s32_pin_ranges_siul2),
 };
 
-static const struct of_device_id s32_pinctrl_of_match[] = {
-	{
-		.compatible = "nxp,s32g2-siul2-pinctrl",
-		.data = &s32_pinctrl_data,
-	},
-	{ /* sentinel */ }
-};
-MODULE_DEVICE_TABLE(of, s32_pinctrl_of_match);
-
 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);
-
-	return s32_pinctrl_probe(pdev, soc_data);
+	return s32_pinctrl_probe(pdev, &s32_pinctrl_data);
 }
 
 static const struct dev_pm_ops s32g_pinctrl_pm_ops = {
@@ -753,7 +739,6 @@ static const struct dev_pm_ops s32g_pinctrl_pm_ops = {
 static struct platform_driver s32g_pinctrl_driver = {
 	.driver = {
 		.name = "s32g-siul2-pinctrl",
-		.of_match_table = s32_pinctrl_of_match,
 		.pm = pm_sleep_ptr(&s32g_pinctrl_pm_ops),
 		.suppress_bind_attrs = true,
 	},
-- 
2.45.2


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH v5 5/7] pinctrl: s32cc: change to "devm_pinctrl_register_and_init"
  2024-11-01  8:06 [PATCH v5 0/7] gpio: siul2-s32g2: add initial GPIO driver Andrei Stefanescu
                   ` (3 preceding siblings ...)
  2024-11-01  8:06 ` [PATCH v5 4/7] pinctrl: s32: convert the driver into an mfd cell Andrei Stefanescu
@ 2024-11-01  8:06 ` Andrei Stefanescu
  2024-11-01 11:54   ` Linus Walleij
  2024-11-01  8:06 ` [PATCH v5 6/7] pinctrl: s32cc: add driver for GPIO functionality Andrei Stefanescu
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 32+ messages in thread
From: Andrei Stefanescu @ 2024-11-01  8:06 UTC (permalink / raw)
  To: Linus Walleij, Bartosz Golaszewski, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Chester Lin, Matthias Brugger,
	Ghennadi Procopciuc, Larisa Grigore, Andrei Stefanescu,
	Greg Kroah-Hartman, Rafael J. Wysocki, Lee Jones, Shawn Guo,
	Sascha Hauer, Fabio Estevam, Dong Aisheng, Jacky Bai
  Cc: linux-gpio, devicetree, linux-kernel, linux-arm-kernel,
	NXP S32 Linux Team, Christophe Lizzi, Alberto Ruiz,
	Enric Balletbo, Pengutronix Kernel Team, imx

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

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

diff --git a/drivers/pinctrl/nxp/pinctrl-s32cc.c b/drivers/pinctrl/nxp/pinctrl-s32cc.c
index 709e823b9c7c..10bff48852b9 100644
--- a/drivers/pinctrl/nxp/pinctrl-s32cc.c
+++ b/drivers/pinctrl/nxp/pinctrl-s32cc.c
@@ -950,10 +950,10 @@ int s32_pinctrl_probe(struct platform_device *pdev,
 		return ret;
 	}
 
-	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
@@ -966,6 +966,11 @@ int s32_pinctrl_probe(struct platform_device *pdev,
 		return -ENOMEM;
 #endif
 
+	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] 32+ messages in thread

* [PATCH v5 6/7] pinctrl: s32cc: add driver for GPIO functionality
  2024-11-01  8:06 [PATCH v5 0/7] gpio: siul2-s32g2: add initial GPIO driver Andrei Stefanescu
                   ` (4 preceding siblings ...)
  2024-11-01  8:06 ` [PATCH v5 5/7] pinctrl: s32cc: change to "devm_pinctrl_register_and_init" Andrei Stefanescu
@ 2024-11-01  8:06 ` Andrei Stefanescu
  2024-11-01 15:45   ` Markus Elfring
  2024-11-01  8:06 ` [PATCH v5 7/7] MAINTAINERS: add MAINTAINER for NXP SIUL2 MFD driver Andrei Stefanescu
  2024-11-01 11:56 ` [PATCH v5 0/7] gpio: siul2-s32g2: add initial GPIO driver Linus Walleij
  7 siblings, 1 reply; 32+ messages in thread
From: Andrei Stefanescu @ 2024-11-01  8:06 UTC (permalink / raw)
  To: Linus Walleij, Bartosz Golaszewski, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Chester Lin, Matthias Brugger,
	Ghennadi Procopciuc, Larisa Grigore, Andrei Stefanescu,
	Greg Kroah-Hartman, Rafael J. Wysocki, Lee Jones, Shawn Guo,
	Sascha Hauer, Fabio Estevam, Dong Aisheng, Jacky Bai
  Cc: linux-gpio, devicetree, linux-kernel, linux-arm-kernel,
	NXP S32 Linux Team, Christophe Lizzi, Alberto Ruiz,
	Enric Balletbo, Pengutronix Kernel Team, imx

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 | 410 +++++++++++++++++++++++-----
 1 file changed, 348 insertions(+), 62 deletions(-)

diff --git a/drivers/pinctrl/nxp/pinctrl-s32cc.c b/drivers/pinctrl/nxp/pinctrl-s32cc.c
index 10bff48852b9..1d4437df29a2 100644
--- a/drivers/pinctrl/nxp/pinctrl-s32cc.c
+++ b/drivers/pinctrl/nxp/pinctrl-s32cc.c
@@ -40,6 +40,14 @@
 #define S32_MSCR_ODE		BIT(20)
 #define S32_MSCR_OBE		BIT(21)
 
+/* PGPDOs are 16bit registers that come in big endian
+ * order if they are grouped in pairs of two.
+ *
+ * For example, the order is PGPDO1, PGPDO0, PGPDO3, PGPDO2...
+ */
+#define S32_PGPD(N)		(((N) ^ 1) * 2)
+#define S32_PGPD_SIZE		16
+
 enum s32_write_type {
 	S32_PINCONF_UPDATE_ONLY,
 	S32_PINCONF_OVERWRITE,
@@ -84,6 +92,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
@@ -93,6 +102,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;
@@ -366,66 +376,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,
@@ -449,8 +399,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,
 };
 
@@ -669,6 +617,315 @@ static const struct pinconf_ops s32_pinconf_ops = {
 	.pin_config_group_dbg_show = s32_pinconf_group_dbg_show,
 };
 
+static struct s32_pinctrl *to_s32_pinctrl(struct gpio_chip *chip)
+{
+	return container_of(chip, struct s32_pinctrl, gc);
+}
+
+static struct regmap *s32_gpio_get_pgpd_regmap(struct gpio_chip *chip,
+					       unsigned int pin,
+					       bool output)
+{
+	struct s32_pinctrl *ipctl = to_s32_pinctrl(chip);
+	struct nxp_siul2_mfd *mfd;
+	u32 base, num;
+	int i;
+
+	mfd = dev_get_drvdata(ipctl->dev->parent);
+
+	for (i = 0; i < mfd->num_siul2; i++) {
+		base = mfd->siul2[i].gpio_base;
+		num = mfd->siul2[i].gpio_num;
+
+		if (pin >= base && pin < base + num)
+			return output ? mfd->siul2[i].regmaps[SIUL2_PGPDO] :
+					mfd->siul2[i].regmaps[SIUL2_PGPDI];
+	}
+
+	return NULL;
+}
+
+static int s32_gpio_request(struct gpio_chip *gc, unsigned int gpio)
+{
+	struct s32_pinctrl *ipctl = to_s32_pinctrl(gc);
+	struct pinctrl_dev *pctldev = ipctl->pctl;
+	struct gpio_pin_config *gpio_pin;
+	unsigned int config;
+	unsigned long flags;
+	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;
+
+	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, 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;
+	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 == gpio) {
+			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_gpio_get_dir(struct gpio_chip *chip, unsigned int gpio)
+{
+	struct s32_pinctrl *ipctl = to_s32_pinctrl(chip);
+	unsigned int reg_value;
+	int ret;
+
+	ret = s32_regmap_read(ipctl->pctl, gpio, &reg_value);
+	if (ret)
+		return ret;
+
+	if (!(reg_value & S32_MSCR_IBE))
+		return -EINVAL;
+
+	return reg_value & S32_MSCR_OBE ? GPIO_LINE_DIRECTION_OUT :
+					  GPIO_LINE_DIRECTION_IN;
+}
+
+static unsigned int s32_pin2pad(unsigned int pin)
+{
+	return pin / S32_PGPD_SIZE;
+}
+
+static u16 s32_pin2mask(unsigned int pin)
+{
+	/**
+	 * From Reference manual :
+	 * PGPDOx[PPDOy] = GPDO(x × 16) + (15 - y)[PDO_(x × 16) + (15 - y)]
+	 */
+	return BIT(S32_PGPD_SIZE - 1 - pin % S32_PGPD_SIZE);
+}
+
+static struct regmap *s32_gpio_get_regmap_offset_mask(struct gpio_chip *chip,
+						      unsigned int gpio,
+						      unsigned int *reg_offset,
+						      u16 *mask,
+						      bool output)
+{
+	struct regmap *regmap;
+	unsigned int pad;
+
+	regmap = s32_gpio_get_pgpd_regmap(chip, gpio, output);
+	if (!regmap)
+		return NULL;
+
+	*mask = s32_pin2mask(gpio);
+	pad = s32_pin2pad(gpio);
+
+	*reg_offset = S32_PGPD(pad);
+
+	return regmap;
+}
+
+static 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, &reg_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, &reg_offset,
+						 &mask, false);
+	if (!regmap)
+		return -EINVAL;
+
+	ret = regmap_read(regmap, reg_offset, &value);
+	if (ret)
+		return ret;
+
+	return !!(value & mask);
+}
+
+static int s32_gpio_dir_out(struct gpio_chip *chip, unsigned int gpio,
+			    int val)
+{
+	struct s32_pinctrl *ipctl = to_s32_pinctrl(chip);
+
+	s32_gpio_set_val(chip, gpio, val);
+
+	return s32_pmx_gpio_set_direction(ipctl->pctl, NULL, gpio, false);
+}
+
+static int s32_gpio_dir_in(struct gpio_chip *chip, unsigned int gpio)
+{
+	struct s32_pinctrl *ipctl = to_s32_pinctrl(chip);
+
+	return s32_pmx_gpio_set_direction(ipctl->pctl, NULL, gpio, true);
+}
+
+static int s32_gpio_gen_names(struct device *dev, 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)++;
+
+		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_remove_reserved_names(struct device *dev,
+					  struct s32_pinctrl *ipctl,
+					  char **names)
+{
+	struct device_node *np = dev->of_node;
+	int num_ranges, i, j, ret;
+	u32 base_gpio, num_gpio;
+
+	/* Parse the gpio-reserved-ranges to know which GPIOs to exclude. */
+
+	num_ranges = of_property_count_u32_elems(dev->of_node,
+						 "gpio-reserved-ranges");
+
+	/* The "gpio-reserved-ranges" is optional. */
+	if (num_ranges < 0)
+		return 0;
+	num_ranges /= 2;
+
+	for (i = 0; i < num_ranges; i++) {
+		ret = of_property_read_u32_index(np, "gpio-reserved-ranges",
+						 i * 2, &base_gpio);
+		if (ret) {
+			dev_err(dev, "Could not parse the start GPIO: %d\n",
+				ret);
+			return ret;
+		}
+
+		ret = of_property_read_u32_index(np, "gpio-reserved-ranges",
+						 i * 2 + 1, &num_gpio);
+		if (ret) {
+			dev_err(dev, "Could not parse num. GPIOs: %d\n", ret);
+			return ret;
+		}
+
+		if (base_gpio + num_gpio > ipctl->gc.ngpio) {
+			dev_err(dev, "Reserved GPIOs outside of GPIO range\n");
+			return -EINVAL;
+		}
+
+		/* Remove names set for reserved GPIOs. */
+		for (j = base_gpio; j < base_gpio + num_gpio; j++) {
+			devm_kfree(dev, names[j]);
+			names[j] = NULL;
+		}
+	}
+
+	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++) {
+		if (mfd->siul2[i].gpio_base % 16 == 0)
+			num_index = 0;
+
+		ret = s32_gpio_gen_names(dev, mfd->siul2[i].gpio_num,
+					 names + mfd->siul2[i].gpio_base,
+					 &ch_index, &num_index);
+		if (ret) {
+			dev_err(dev, "Could not set names for SIUL2_%d GPIOs\n",
+				i);
+			return ret;
+		}
+
+		ch_index++;
+	}
+
+	ret = s32_gpio_remove_reserved_names(dev, ipctl, names);
+	if (ret)
+		return ret;
+
+	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)
@@ -899,12 +1156,14 @@ 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 nxp_siul2_mfd *mfd = dev_get_drvdata(pdev->dev.parent);
 #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;
+	struct gpio_chip *gc;
 	int ret;
 
 	if (!soc_data || !soc_data->pins || !soc_data->npins) {
@@ -973,5 +1232,32 @@ int s32_pinctrl_probe(struct platform_device *pdev,
 
 	dev_info(&pdev->dev, "initialized s32 pinctrl driver\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;
+
+	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 driver\n");
+
 	return 0;
 }
-- 
2.45.2


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH v5 7/7] MAINTAINERS: add MAINTAINER for NXP SIUL2 MFD driver
  2024-11-01  8:06 [PATCH v5 0/7] gpio: siul2-s32g2: add initial GPIO driver Andrei Stefanescu
                   ` (5 preceding siblings ...)
  2024-11-01  8:06 ` [PATCH v5 6/7] pinctrl: s32cc: add driver for GPIO functionality Andrei Stefanescu
@ 2024-11-01  8:06 ` Andrei Stefanescu
  2024-11-01 11:56 ` [PATCH v5 0/7] gpio: siul2-s32g2: add initial GPIO driver Linus Walleij
  7 siblings, 0 replies; 32+ messages in thread
From: Andrei Stefanescu @ 2024-11-01  8:06 UTC (permalink / raw)
  To: Linus Walleij, Bartosz Golaszewski, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Chester Lin, Matthias Brugger,
	Ghennadi Procopciuc, Larisa Grigore, Andrei Stefanescu,
	Greg Kroah-Hartman, Rafael J. Wysocki, Lee Jones, Shawn Guo,
	Sascha Hauer, Fabio Estevam, Dong Aisheng, Jacky Bai
  Cc: linux-gpio, devicetree, linux-kernel, linux-arm-kernel,
	NXP S32 Linux Team, Christophe Lizzi, Alberto Ruiz,
	Enric Balletbo, Pengutronix Kernel Team, imx

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 a27407950242..707cc15e4406 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2787,7 +2787,9 @@ R:	Ghennadi Procopciuc <ghennadi.procopciuc@oss.nxp.com>
 L:	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,siul2.yaml
 F:	arch/arm64/boot/dts/freescale/s32g*.dts*
+F:	drivers/mfd/nxp-siul2.c
 F:	drivers/pinctrl/nxp/
 
 ARM/Orion SoC/Technologic Systems TS-78xx platform support
-- 
2.45.2


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH v5 4/7] pinctrl: s32: convert the driver into an mfd cell
  2024-11-01  8:06 ` [PATCH v5 4/7] pinctrl: s32: convert the driver into an mfd cell Andrei Stefanescu
@ 2024-11-01 11:53   ` Linus Walleij
  2024-11-01 21:47   ` kernel test robot
  2024-11-02  8:51   ` Krzysztof Kozlowski
  2 siblings, 0 replies; 32+ messages in thread
From: Linus Walleij @ 2024-11-01 11:53 UTC (permalink / raw)
  To: Andrei Stefanescu
  Cc: Bartosz Golaszewski, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Chester Lin, Matthias Brugger, Ghennadi Procopciuc,
	Larisa Grigore, Greg Kroah-Hartman, Rafael J. Wysocki, Lee Jones,
	Shawn Guo, Sascha Hauer, Fabio Estevam, Dong Aisheng, Jacky Bai,
	linux-gpio, devicetree, linux-kernel, linux-arm-kernel,
	NXP S32 Linux Team, Christophe Lizzi, Alberto Ruiz,
	Enric Balletbo, Pengutronix Kernel Team, imx

On Fri, Nov 1, 2024 at 9:06 AM Andrei Stefanescu
<andrei.stefanescu@oss.nxp.com> wrote:

> The SIUL2 module is now represented as an mfd device. The pinctrl driver
> is now an mfd_cell. Therefore, remove its compatible and adjust its
> probing in order to get the necessary information from its mfd parent.
>
> Signed-off-by: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>

Acked-by: Linus Walleij <linus.walleij@linaro.org>
I assume this needs to go in with the rest of the patches.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5 5/7] pinctrl: s32cc: change to "devm_pinctrl_register_and_init"
  2024-11-01  8:06 ` [PATCH v5 5/7] pinctrl: s32cc: change to "devm_pinctrl_register_and_init" Andrei Stefanescu
@ 2024-11-01 11:54   ` Linus Walleij
  0 siblings, 0 replies; 32+ messages in thread
From: Linus Walleij @ 2024-11-01 11:54 UTC (permalink / raw)
  To: Andrei Stefanescu
  Cc: Bartosz Golaszewski, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Chester Lin, Matthias Brugger, Ghennadi Procopciuc,
	Larisa Grigore, Greg Kroah-Hartman, Rafael J. Wysocki, Lee Jones,
	Shawn Guo, Sascha Hauer, Fabio Estevam, Dong Aisheng, Jacky Bai,
	linux-gpio, devicetree, linux-kernel, linux-arm-kernel,
	NXP S32 Linux Team, Christophe Lizzi, Alberto Ruiz,
	Enric Balletbo, Pengutronix Kernel Team, imx

On Fri, Nov 1, 2024 at 9:06 AM Andrei Stefanescu
<andrei.stefanescu@oss.nxp.com> wrote:

> Switch from "devm_pinctrl_register" to "devm_pinctrl_register_and_init"
> and "pinctrl_enable" since this is the recommended way.
>
> Signed-off-by: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
I assume this needs to go in with the rest of the patches.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5 0/7] gpio: siul2-s32g2: add initial GPIO driver
  2024-11-01  8:06 [PATCH v5 0/7] gpio: siul2-s32g2: add initial GPIO driver Andrei Stefanescu
                   ` (6 preceding siblings ...)
  2024-11-01  8:06 ` [PATCH v5 7/7] MAINTAINERS: add MAINTAINER for NXP SIUL2 MFD driver Andrei Stefanescu
@ 2024-11-01 11:56 ` Linus Walleij
  2024-11-01 12:10   ` Andrei Stefanescu
  7 siblings, 1 reply; 32+ messages in thread
From: Linus Walleij @ 2024-11-01 11:56 UTC (permalink / raw)
  To: Andrei Stefanescu
  Cc: Bartosz Golaszewski, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Chester Lin, Matthias Brugger, Ghennadi Procopciuc,
	Larisa Grigore, Greg Kroah-Hartman, Rafael J. Wysocki, Lee Jones,
	Shawn Guo, Sascha Hauer, Fabio Estevam, Dong Aisheng, Jacky Bai,
	linux-gpio, devicetree, linux-kernel, linux-arm-kernel,
	NXP S32 Linux Team, Christophe Lizzi, Alberto Ruiz,
	Enric Balletbo, Pengutronix Kernel Team, imx

On Fri, Nov 1, 2024 at 9:06 AM Andrei Stefanescu
<andrei.stefanescu@oss.nxp.com> wrote:

> Andrei Stefanescu (7):
>   dt-bindings: mfd: add support for the NXP SIUL2 module
>   mfd: nxp-siul2: add support for NXP SIUL2
>   arm64: dts: s32g: make pinctrl part of mfd node
>   pinctrl: s32: convert the driver into an mfd cell
>   pinctrl: s32cc: change to "devm_pinctrl_register_and_init"
>   pinctrl: s32cc: add driver for GPIO functionality
>   MAINTAINERS: add MAINTAINER for NXP SIUL2 MFD driver

How do you want to merge this?

Can the MFD and pinctrl parts be merged separately, or shall
it all go into MFD or all into pinctrl?

I can certainly merge it if Lee ACKs the MFD patch.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5 0/7] gpio: siul2-s32g2: add initial GPIO driver
  2024-11-01 11:56 ` [PATCH v5 0/7] gpio: siul2-s32g2: add initial GPIO driver Linus Walleij
@ 2024-11-01 12:10   ` Andrei Stefanescu
  0 siblings, 0 replies; 32+ messages in thread
From: Andrei Stefanescu @ 2024-11-01 12:10 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Bartosz Golaszewski, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Chester Lin, Matthias Brugger, Ghennadi Procopciuc,
	Larisa Grigore, Greg Kroah-Hartman, Rafael J. Wysocki, Lee Jones,
	Shawn Guo, Sascha Hauer, Fabio Estevam, Dong Aisheng, Jacky Bai,
	linux-gpio, devicetree, linux-kernel, linux-arm-kernel,
	NXP S32 Linux Team, Christophe Lizzi, Alberto Ruiz,
	Enric Balletbo, Pengutronix Kernel Team, imx

Hi Linus,

On 01/11/2024 13:56, Linus Walleij wrote:
> On Fri, Nov 1, 2024 at 9:06 AM Andrei Stefanescu
> <andrei.stefanescu@oss.nxp.com> wrote:
> 
>> Andrei Stefanescu (7):
>>   dt-bindings: mfd: add support for the NXP SIUL2 module
>>   mfd: nxp-siul2: add support for NXP SIUL2
>>   arm64: dts: s32g: make pinctrl part of mfd node
>>   pinctrl: s32: convert the driver into an mfd cell
>>   pinctrl: s32cc: change to "devm_pinctrl_register_and_init"
>>   pinctrl: s32cc: add driver for GPIO functionality
>>   MAINTAINERS: add MAINTAINER for NXP SIUL2 MFD driver
> 
> How do you want to merge this?
> 
> Can the MFD and pinctrl parts be merged separately, or shall
> it all go into MFD or all into pinctrl?
> 
> I can certainly merge it if Lee ACKs the MFD patch.

Thank you very much for the quick review! I am not sure which
tree to merge into. If possible, it would be great to have all
of them merged together.

I plan to send further patches to the GPIO driver so it would
be easier to have them all in one place.

Best regards,
Andrei

> 
> Yours,
> Linus Walleij


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5 6/7] pinctrl: s32cc: add driver for GPIO functionality
  2024-11-01  8:06 ` [PATCH v5 6/7] pinctrl: s32cc: add driver for GPIO functionality Andrei Stefanescu
@ 2024-11-01 15:45   ` Markus Elfring
  2024-11-04 11:45     ` Andrei Stefanescu
  0 siblings, 1 reply; 32+ messages in thread
From: Markus Elfring @ 2024-11-01 15:45 UTC (permalink / raw)
  To: Andrei Stefanescu, linux-gpio, devicetree, s32,
	Bartosz Golaszewski, Chester Lin, Conor Dooley, Dong Aisheng,
	Fabio Estevam, Ghennadi Procopciuc, Greg Kroah-Hartman, Jacky Bai,
	Krzysztof Kozlowski, Larisa Grigore, Lee Jones, Linus Walleij,
	Matthias Brugger, Rafael J. Wysocki, Rob Herring, Sascha Hauer,
	Shawn Guo
  Cc: LKML, imx, linux-arm-kernel, kernel, Alberto Ruiz,
	Christophe Lizzi, Enric Balletbo

> Add basic GPIO functionality (request, free, get, set) for the existing
> pinctrl SIUL2 driver since the hardware for pinctrl&GPIO is tightly
> coupled.
> +++ b/drivers/pinctrl/nxp/pinctrl-s32cc.c
> +static int s32_gpio_request(struct gpio_chip *gc, unsigned int gpio)
> +{
> +	spin_lock_irqsave(&ipctl->gpio_configs_lock, flags);
> +	list_add(&gpio_pin->list, &ipctl->gpio_configs);
> +	spin_unlock_irqrestore(&ipctl->gpio_configs_lock, flags);
…

Under which circumstances would you become interested to apply a statement
like “guard(spinlock_irqsave)(&ipctl->gpio_configs_lock);”?
https://elixir.bootlin.com/linux/v6.12-rc5/source/include/linux/spinlock.h#L551

Regards,
Markus

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5 2/7] mfd: nxp-siul2: add support for NXP SIUL2
  2024-11-01  8:06 ` [PATCH v5 2/7] mfd: nxp-siul2: add support for NXP SIUL2 Andrei Stefanescu
@ 2024-11-01 17:18   ` kernel test robot
  2024-11-01 18:20   ` Frank Li
                     ` (3 subsequent siblings)
  4 siblings, 0 replies; 32+ messages in thread
From: kernel test robot @ 2024-11-01 17:18 UTC (permalink / raw)
  To: Andrei Stefanescu, Linus Walleij, Bartosz Golaszewski,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Chester Lin,
	Matthias Brugger, Ghennadi Procopciuc, Larisa Grigore,
	Greg Kroah-Hartman, Rafael J. Wysocki, Lee Jones, Shawn Guo,
	Sascha Hauer, Fabio Estevam, Dong Aisheng, Jacky Bai
  Cc: oe-kbuild-all, linux-gpio, devicetree, linux-kernel,
	linux-arm-kernel, NXP S32 Linux Team, Christophe Lizzi,
	Alberto Ruiz, Enric Balletbo, Pengutronix Kernel Team, imx

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 shawnguo/for-next linus/master lee-mfd/for-mfd-fixes v6.12-rc5 next-20241101]
[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/20241101-160940
base:   https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git devel
patch link:    https://lore.kernel.org/r/20241101080614.1070819-3-andrei.stefanescu%40oss.nxp.com
patch subject: [PATCH v5 2/7] mfd: nxp-siul2: add support for NXP SIUL2
config: m68k-allmodconfig (https://download.01.org/0day-ci/archive/20241102/202411020003.C5suQQDX-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 14.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241102/202411020003.C5suQQDX-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/202411020003.C5suQQDX-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/mfd/nxp-siul2.c: In function 'nxp_siul2_init_regmap':
>> drivers/mfd/nxp-siul2.c:318:1: warning: the frame size of 1116 bytes is larger than 1024 bytes [-Wframe-larger-than=]
     318 | }
         | ^
--
>> drivers/mfd/nxp-siul2.c:50: warning: Function parameter or struct member 'reg_name' not described in 'nxp_siul2_reg_range_info'

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for GET_FREE_REGION
   Depends on [n]: SPARSEMEM [=n]
   Selected by [m]:
   - RESOURCE_KUNIT_TEST [=m] && RUNTIME_TESTING_MENU [=y] && KUNIT [=m]


vim +318 drivers/mfd/nxp-siul2.c

   266	
   267	static int nxp_siul2_init_regmap(struct platform_device *pdev,
   268					 void __iomem *base, int siul)
   269	{
   270		struct regmap_config regmap_configs[SIUL2_NUM_REG_TYPES] = {
   271			[SIUL2_MPIDR]	= nxp_siul2_regmap_generic_conf,
   272			[SIUL2_IRQ]	= nxp_siul2_regmap_irq_conf,
   273			[SIUL2_MSCR]	= nxp_siul2_regmap_generic_conf,
   274			[SIUL2_IMCR]	= nxp_siul2_regmap_generic_conf,
   275			[SIUL2_PGPDO]	= nxp_siul2_regmap_pgpdo_conf,
   276			[SIUL2_PGPDI]	= nxp_siul2_regmap_pgpdi_conf,
   277		};
   278		const struct nxp_siul2_reg_range_info *tmp_range;
   279		struct regmap_config *tmp_conf;
   280		struct nxp_siul2_info *info;
   281		struct nxp_siul2_mfd *priv;
   282		void __iomem *reg_start;
   283		int i, ret;
   284	
   285		priv = platform_get_drvdata(pdev);
   286		info = &priv->siul2[siul];
   287	
   288		for (i = 0; i < SIUL2_NUM_REG_TYPES; i++) {
   289			if (!s32g2_reg_ranges[siul][i].valid)
   290				continue;
   291	
   292			tmp_range = &s32g2_reg_ranges[siul][i];
   293			tmp_conf = &regmap_configs[i];
   294			tmp_conf->name = tmp_range->reg_name;
   295			tmp_conf->max_register =
   296				tmp_range->reg_end_offset - tmp_range->reg_start_offset;
   297	
   298			if (tmp_conf->cache_type != REGCACHE_NONE)
   299				tmp_conf->num_reg_defaults_raw =
   300					tmp_conf->max_register / tmp_conf->reg_stride;
   301	
   302			if (tmp_range->reg_access) {
   303				tmp_conf->wr_table = tmp_range->reg_access;
   304				tmp_conf->rd_table = tmp_range->reg_access;
   305			}
   306	
   307			reg_start = base + tmp_range->reg_start_offset;
   308			info->regmaps[i] = devm_regmap_init_mmio(&pdev->dev, reg_start,
   309								 tmp_conf);
   310			if (IS_ERR(info->regmaps[i])) {
   311				dev_err(&pdev->dev, "regmap %d init failed: %d\n", i,
   312					ret);
   313				return PTR_ERR(info->regmaps[i]);
   314			}
   315		}
   316	
   317		return 0;
 > 318	}
   319	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5 1/7] dt-bindings: mfd: add support for the NXP SIUL2 module
  2024-11-01  8:06 ` [PATCH v5 1/7] dt-bindings: mfd: add support for the NXP SIUL2 module Andrei Stefanescu
@ 2024-11-01 18:11   ` Frank Li
  2024-11-04 11:11     ` Andrei Stefanescu
  2024-11-02  8:42   ` Krzysztof Kozlowski
  1 sibling, 1 reply; 32+ messages in thread
From: Frank Li @ 2024-11-01 18:11 UTC (permalink / raw)
  To: Andrei Stefanescu
  Cc: Linus Walleij, Bartosz Golaszewski, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Chester Lin, Matthias Brugger,
	Ghennadi Procopciuc, Larisa Grigore, Greg Kroah-Hartman,
	Rafael J. Wysocki, Lee Jones, Shawn Guo, Sascha Hauer,
	Fabio Estevam, Dong Aisheng, Jacky Bai, linux-gpio, devicetree,
	linux-kernel, linux-arm-kernel, NXP S32 Linux Team,
	Christophe Lizzi, Alberto Ruiz, Enric Balletbo,
	Pengutronix Kernel Team, imx

On Fri, Nov 01, 2024 at 10:06:07AM +0200, 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>
> ---
>  .../devicetree/bindings/mfd/nxp,siul2.yaml    | 191 ++++++++++++++++++
>  1 file changed, 191 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/nxp,siul2.yaml
>
> diff --git a/Documentation/devicetree/bindings/mfd/nxp,siul2.yaml b/Documentation/devicetree/bindings/mfd/nxp,siul2.yaml
> new file mode 100644
> index 000000000000..141ec1219821
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/nxp,siul2.yaml
> @@ -0,0 +1,191 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright 2024 NXP
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/nxp,siul2.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: 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:
> +    enum:
> +      - nxp,s32g2-siul2
> +      - nxp,s32g3-siul2
> +
> +  reg:
> +    items:
> +      - description: SIUL2_0 module memory
> +      - description: SIUL2_1 module memory

description have not provide more informaiton.
maxItems: 2 should be enough here.

> +
> +  reg-names:
> +    items:
> +      - const: siul20
> +      - const: siul21
> +
> +  gpio-controller: true
> +
> +  '#gpio-cells':
> +    const: 2
> +
> +  gpio-ranges:
> +    minItems: 2
> +    maxItems: 2
> +
> +  gpio-reserved-ranges:
> +    minItems: 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

suppose needn't such common property, if use
	unevaluatedProperties: false

> +
> +          pinmux:
> +            description: |

needn't "|" here

> +              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]
> +
> +        additionalProperties: false

It should be unevaluatedProperties: false because there $ref.

> +
> +required:
> +  - compatible
> +  - reg
> +  - reg-names
> +  - gpio-controller
> +  - "#gpio-cells"
> +  - gpio-ranges
> +  - gpio-reserved-ranges
> +  - interrupts
> +  - interrupt-controller
> +  - "#interrupt-cells"
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    siul2: siul2@4009c000 {

needn't label siul2.

> +      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>;
> +      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 {
> +          pinmux = <0x0>;
> +          input-enable;
> +          bias-pull-up;
> +          slew-rate = <166>;
> +        };
> +
> +        jtag-grp1 {
> +          pinmux = <0x11>;
> +          slew-rate = <166>;
> +        };

one example should be enough.

> +
> +        jtag-grp2 {
> +          pinmux = <0x40>;
> +          input-enable;
> +          bias-pull-down;
> +          slew-rate = <166>;
> +        };
> +
> +        jtag-grp3 {
> +          pinmux = <0x23c0>,
> +                   <0x23d0>,
> +                   <0x2320>;
> +        };
> +
> +        jtag-grp4 {
> +          pinmux = <0x51>;
> +          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] 32+ messages in thread

* Re: [PATCH v5 2/7] mfd: nxp-siul2: add support for NXP SIUL2
  2024-11-01  8:06 ` [PATCH v5 2/7] mfd: nxp-siul2: add support for NXP SIUL2 Andrei Stefanescu
  2024-11-01 17:18   ` kernel test robot
@ 2024-11-01 18:20   ` Frank Li
  2024-11-02  5:13   ` kernel test robot
                     ` (2 subsequent siblings)
  4 siblings, 0 replies; 32+ messages in thread
From: Frank Li @ 2024-11-01 18:20 UTC (permalink / raw)
  To: Andrei Stefanescu
  Cc: Linus Walleij, Bartosz Golaszewski, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Chester Lin, Matthias Brugger,
	Ghennadi Procopciuc, Larisa Grigore, Greg Kroah-Hartman,
	Rafael J. Wysocki, Lee Jones, Shawn Guo, Sascha Hauer,
	Fabio Estevam, Dong Aisheng, Jacky Bai, linux-gpio, devicetree,
	linux-kernel, linux-arm-kernel, NXP S32 Linux Team,
	Christophe Lizzi, Alberto Ruiz, Enric Balletbo,
	Pengutronix Kernel Team, imx

On Fri, Nov 01, 2024 at 10:06:08AM +0200, Andrei Stefanescu wrote:
> SIUL2 (System Integration Unit Lite) is a hardware module which
> implements various functionalities:
> - reading SoC information
> - pinctrl
> - GPIO (including interrupts)
>
> 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       | 411 ++++++++++++++++++++++++++++++++++
>  include/linux/mfd/nxp-siul2.h |  55 +++++
>  4 files changed, 479 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 f9325bcce1b9..fc590789e8b3 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -1098,6 +1098,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 2a9f91e81af8..7b19ea014221 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -226,6 +226,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..ba13d1beb244
> --- /dev/null
> +++ b/drivers/mfd/nxp-siul2.c
> @@ -0,0 +1,411 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * SIUL2(System Integration Unit Lite) MFD driver
> + *
> + * Copyright 2024 NXP
> + */
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/of.h>
> +#include <linux/mfd/core.h>
> +#include <linux/mfd/nxp-siul2.h>

order alphabet

> +
> +#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",
> +	}

Only one device? If add later, commit message should said add more mfd
later.

Frank

> +};
> +
> +/**
> + * struct nxp_siul2_reg_range_info: a register range in SIUL2
> + * @reg_start_offset: the first valid register offset
> + * @reg_end_offset: the last valid register offset
> + * @reg_access: the read/write access tables if not NULL
> + * @valid: whether the register range is valid or not
> + */
> +struct nxp_siul2_reg_range_info {
> +	const char *reg_name;
> +	unsigned int reg_start_offset;
> +	unsigned int reg_end_offset;
> +	const struct regmap_access_table *reg_access;
> +	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_MPIDR] = S32_REG_RANGE(4, 8, "SIUL2_0_MPIDR", 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_MPIDR] = S32_REG_RANGE(4, 8, "SIUL2_1_MPIDR", 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, int siul)
> +{
> +	struct regmap_config regmap_configs[SIUL2_NUM_REG_TYPES] = {
> +		[SIUL2_MPIDR]	= 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, ret;
> +
> +	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 =
> +				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])) {
> +			dev_err(&pdev->dev, "regmap %d init failed: %d\n", i,
> +				ret);
> +			return 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)) {
> +			dev_err(&pdev->dev, "Failed to get MEM resource: %s\n",
> +				reg_name);
> +			return PTR_ERR(base);

return dev_err_probe()

> +		}
> +
> +		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) {
> +			dev_err(&pdev->dev, "Invalid pinspec count: %d\n",
> +				pinspec.args_count);
> +			return -EINVAL;

return dev_err_probe();
> +		}
> +
> +		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" },
> +	{ },
> +};
> +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..238c812dba29
> --- /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 2024 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_MPIDR - 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_MPIDR,
> +	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] 32+ messages in thread

* Re: [PATCH v5 3/7] arm64: dts: s32g: make pinctrl part of mfd node
  2024-11-01  8:06 ` [PATCH v5 3/7] arm64: dts: s32g: make pinctrl part of mfd node Andrei Stefanescu
@ 2024-11-01 18:22   ` Frank Li
  2024-11-04 11:33     ` Andrei Stefanescu
  2024-11-02  8:49   ` Krzysztof Kozlowski
  1 sibling, 1 reply; 32+ messages in thread
From: Frank Li @ 2024-11-01 18:22 UTC (permalink / raw)
  To: Andrei Stefanescu
  Cc: Linus Walleij, Bartosz Golaszewski, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Chester Lin, Matthias Brugger,
	Ghennadi Procopciuc, Larisa Grigore, Greg Kroah-Hartman,
	Rafael J. Wysocki, Lee Jones, Shawn Guo, Sascha Hauer,
	Fabio Estevam, Dong Aisheng, Jacky Bai, linux-gpio, devicetree,
	linux-kernel, linux-arm-kernel, NXP S32 Linux Team,
	Christophe Lizzi, Alberto Ruiz, Enric Balletbo,
	Pengutronix Kernel Team, imx

On Fri, Nov 01, 2024 at 10:06:09AM +0200, Andrei Stefanescu wrote:
> SIUL2 is now represented as an mfd device. Therefore, the old
> pinctrl node is deprecated. Move the pinctrl related properties
> inside the new "nxp-siul2" node. The latter one is now used
> to represent the mfd device.

Generally, dt team want you to keep both for sometime to keep
back compatiblity.

Frank
>
> Signed-off-by: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
> ---
>  arch/arm64/boot/dts/freescale/s32g2.dtsi | 26 +++++++++++-------------
>  arch/arm64/boot/dts/freescale/s32g3.dtsi | 26 +++++++++++-------------
>  2 files changed, 24 insertions(+), 28 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/freescale/s32g2.dtsi b/arch/arm64/boot/dts/freescale/s32g2.dtsi
> index fa054bfe7d5c..e14ce5503e1f 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>;
> +		siul2: siul2@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>;
> +			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 {
> diff --git a/arch/arm64/boot/dts/freescale/s32g3.dtsi b/arch/arm64/boot/dts/freescale/s32g3.dtsi
> index b4226a9143c8..fa43d036686f 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>;
> +		siul2: siul2@4009c000 {
> +			compatible = "nxp,s32g3-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>;
> +			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 {
> --
> 2.45.2
>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5 4/7] pinctrl: s32: convert the driver into an mfd cell
  2024-11-01  8:06 ` [PATCH v5 4/7] pinctrl: s32: convert the driver into an mfd cell Andrei Stefanescu
  2024-11-01 11:53   ` Linus Walleij
@ 2024-11-01 21:47   ` kernel test robot
  2024-11-02  8:51   ` Krzysztof Kozlowski
  2 siblings, 0 replies; 32+ messages in thread
From: kernel test robot @ 2024-11-01 21:47 UTC (permalink / raw)
  To: Andrei Stefanescu, Linus Walleij, Bartosz Golaszewski,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Chester Lin,
	Matthias Brugger, Ghennadi Procopciuc, Larisa Grigore,
	Greg Kroah-Hartman, Rafael J. Wysocki, Lee Jones, Shawn Guo,
	Sascha Hauer, Fabio Estevam, Dong Aisheng, Jacky Bai
  Cc: llvm, oe-kbuild-all, linux-gpio, devicetree, linux-kernel,
	linux-arm-kernel, NXP S32 Linux Team, Christophe Lizzi,
	Alberto Ruiz, Enric Balletbo, Pengutronix Kernel Team, imx

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 shawnguo/for-next linus/master v6.12-rc5 next-20241101]
[cannot apply to lee-mfd/for-mfd-fixes]
[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/20241101-160940
base:   https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git devel
patch link:    https://lore.kernel.org/r/20241101080614.1070819-5-andrei.stefanescu%40oss.nxp.com
patch subject: [PATCH v5 4/7] pinctrl: s32: convert the driver into an mfd cell
config: arm64-allmodconfig (https://download.01.org/0day-ci/archive/20241102/202411020514.qOUrieWa-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project 639a7ac648f1e50ccd2556e17d401c04f9cce625)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241102/202411020514.qOUrieWa-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/202411020514.qOUrieWa-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/pinctrl/nxp/pinctrl-s32cc.c:103: warning: Function parameter or struct member 'gpio_configs_lock' not described in 's32_pinctrl'
>> drivers/pinctrl/nxp/pinctrl-s32cc.c:103: warning: Excess struct member 'gpiop_configs_lock' description in 's32_pinctrl'

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for MODVERSIONS
   Depends on [n]: MODULES [=y] && !COMPILE_TEST [=y]
   Selected by [y]:
   - RANDSTRUCT_FULL [=y] && (CC_HAS_RANDSTRUCT [=y] || GCC_PLUGINS [=n]) && MODULES [=y]


vim +103 drivers/pinctrl/nxp/pinctrl-s32cc.c

fd84aaa8173d3f Chester Lin       2023-02-20   82  
157a51f7e5e81e Andrei Stefanescu 2024-11-01   83  /**
157a51f7e5e81e Andrei Stefanescu 2024-11-01   84   * struct s32_pinctrl - private driver data
fd84aaa8173d3f Chester Lin       2023-02-20   85   * @dev: a pointer back to containing device
fd84aaa8173d3f Chester Lin       2023-02-20   86   * @pctl: a pointer to the pinctrl device structure
fd84aaa8173d3f Chester Lin       2023-02-20   87   * @regions: reserved memory regions with start/end pin
fd84aaa8173d3f Chester Lin       2023-02-20   88   * @info: structure containing information about the pin
fd84aaa8173d3f Chester Lin       2023-02-20   89   * @gpio_configs: Saved configurations for GPIO pins
fd84aaa8173d3f Chester Lin       2023-02-20   90   * @gpiop_configs_lock: lock for the `gpio_configs` list
157a51f7e5e81e Andrei Stefanescu 2024-11-01   91   * @saved_context: Configuration saved over system sleep
fd84aaa8173d3f Chester Lin       2023-02-20   92   */
fd84aaa8173d3f Chester Lin       2023-02-20   93  struct s32_pinctrl {
fd84aaa8173d3f Chester Lin       2023-02-20   94  	struct device *dev;
fd84aaa8173d3f Chester Lin       2023-02-20   95  	struct pinctrl_dev *pctl;
fd84aaa8173d3f Chester Lin       2023-02-20   96  	struct s32_pinctrl_mem_region *regions;
fd84aaa8173d3f Chester Lin       2023-02-20   97  	struct s32_pinctrl_soc_info *info;
fd84aaa8173d3f Chester Lin       2023-02-20   98  	struct list_head gpio_configs;
fd84aaa8173d3f Chester Lin       2023-02-20   99  	spinlock_t gpio_configs_lock;
fd84aaa8173d3f Chester Lin       2023-02-20  100  #ifdef CONFIG_PM_SLEEP
fd84aaa8173d3f Chester Lin       2023-02-20  101  	struct s32_pinctrl_context saved_context;
fd84aaa8173d3f Chester Lin       2023-02-20  102  #endif
fd84aaa8173d3f Chester Lin       2023-02-20 @103  };
fd84aaa8173d3f Chester Lin       2023-02-20  104  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5 2/7] mfd: nxp-siul2: add support for NXP SIUL2
  2024-11-01  8:06 ` [PATCH v5 2/7] mfd: nxp-siul2: add support for NXP SIUL2 Andrei Stefanescu
  2024-11-01 17:18   ` kernel test robot
  2024-11-01 18:20   ` Frank Li
@ 2024-11-02  5:13   ` kernel test robot
  2024-11-02  8:37   ` Dan Carpenter
  2024-11-02  8:52   ` Krzysztof Kozlowski
  4 siblings, 0 replies; 32+ messages in thread
From: kernel test robot @ 2024-11-02  5:13 UTC (permalink / raw)
  To: Andrei Stefanescu, Linus Walleij, Bartosz Golaszewski,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Chester Lin,
	Matthias Brugger, Ghennadi Procopciuc, Larisa Grigore,
	Greg Kroah-Hartman, Rafael J. Wysocki, Lee Jones, Shawn Guo,
	Sascha Hauer, Fabio Estevam, Dong Aisheng, Jacky Bai
  Cc: llvm, oe-kbuild-all, linux-gpio, devicetree, linux-kernel,
	linux-arm-kernel, NXP S32 Linux Team, Christophe Lizzi,
	Alberto Ruiz, Enric Balletbo, Pengutronix Kernel Team, imx

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 shawnguo/for-next linus/master lee-mfd/for-mfd-fixes v6.12-rc5 next-20241101]
[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/20241101-160940
base:   https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git devel
patch link:    https://lore.kernel.org/r/20241101080614.1070819-3-andrei.stefanescu%40oss.nxp.com
patch subject: [PATCH v5 2/7] mfd: nxp-siul2: add support for NXP SIUL2
config: hexagon-allmodconfig (https://download.01.org/0day-ci/archive/20241102/202411021558.f39S3DSV-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project 639a7ac648f1e50ccd2556e17d401c04f9cce625)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241102/202411021558.f39S3DSV-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/202411021558.f39S3DSV-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from drivers/mfd/nxp-siul2.c:11:
   In file included from include/linux/mfd/nxp-siul2.h:11:
   In file included from include/linux/regmap.h:20:
   In file included from include/linux/iopoll.h:14:
   In file included from include/linux/io.h:14:
   In file included from arch/hexagon/include/asm/io.h:328:
   include/asm-generic/io.h:548:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     548 |         val = __raw_readb(PCI_IOBASE + addr);
         |                           ~~~~~~~~~~ ^
   include/asm-generic/io.h:561:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     561 |         val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
      37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
         |                                                   ^
   In file included from drivers/mfd/nxp-siul2.c:11:
   In file included from include/linux/mfd/nxp-siul2.h:11:
   In file included from include/linux/regmap.h:20:
   In file included from include/linux/iopoll.h:14:
   In file included from include/linux/io.h:14:
   In file included from arch/hexagon/include/asm/io.h:328:
   include/asm-generic/io.h:574:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     574 |         val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
      35 | #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
         |                                                   ^
   In file included from drivers/mfd/nxp-siul2.c:11:
   In file included from include/linux/mfd/nxp-siul2.h:11:
   In file included from include/linux/regmap.h:20:
   In file included from include/linux/iopoll.h:14:
   In file included from include/linux/io.h:14:
   In file included from arch/hexagon/include/asm/io.h:328:
   include/asm-generic/io.h:585:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     585 |         __raw_writeb(value, PCI_IOBASE + addr);
         |                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:595:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     595 |         __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
   include/asm-generic/io.h:605:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     605 |         __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
>> drivers/mfd/nxp-siul2.c:312:5: warning: variable 'ret' is uninitialized when used here [-Wuninitialized]
     312 |                                 ret);
         |                                 ^~~
   include/linux/dev_printk.h:154:65: note: expanded from macro 'dev_err'
     154 |         dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), ##__VA_ARGS__)
         |                                                                        ^~~~~~~~~~~
   include/linux/dev_printk.h:110:23: note: expanded from macro 'dev_printk_index_wrap'
     110 |                 _p_func(dev, fmt, ##__VA_ARGS__);                       \
         |                                     ^~~~~~~~~~~
   drivers/mfd/nxp-siul2.c:283:12: note: initialize the variable 'ret' to silence this warning
     283 |         int i, ret;
         |                   ^
         |                    = 0
>> drivers/mfd/nxp-siul2.c:367:12: warning: stack frame size (1296) exceeds limit (1024) in 'nxp_siul2_probe' [-Wframe-larger-than]
     367 | static int nxp_siul2_probe(struct platform_device *pdev)
         |            ^
   8 warnings generated.

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for MODVERSIONS
   Depends on [n]: MODULES [=y] && !COMPILE_TEST [=y]
   Selected by [y]:
   - RANDSTRUCT_FULL [=y] && (CC_HAS_RANDSTRUCT [=y] || GCC_PLUGINS [=n]) && MODULES [=y]
   WARNING: unmet direct dependencies detected for GET_FREE_REGION
   Depends on [n]: SPARSEMEM [=n]
   Selected by [m]:
   - RESOURCE_KUNIT_TEST [=m] && RUNTIME_TESTING_MENU [=y] && KUNIT [=m]


vim +/ret +312 drivers/mfd/nxp-siul2.c

   266	
   267	static int nxp_siul2_init_regmap(struct platform_device *pdev,
   268					 void __iomem *base, int siul)
   269	{
   270		struct regmap_config regmap_configs[SIUL2_NUM_REG_TYPES] = {
   271			[SIUL2_MPIDR]	= nxp_siul2_regmap_generic_conf,
   272			[SIUL2_IRQ]	= nxp_siul2_regmap_irq_conf,
   273			[SIUL2_MSCR]	= nxp_siul2_regmap_generic_conf,
   274			[SIUL2_IMCR]	= nxp_siul2_regmap_generic_conf,
   275			[SIUL2_PGPDO]	= nxp_siul2_regmap_pgpdo_conf,
   276			[SIUL2_PGPDI]	= nxp_siul2_regmap_pgpdi_conf,
   277		};
   278		const struct nxp_siul2_reg_range_info *tmp_range;
   279		struct regmap_config *tmp_conf;
   280		struct nxp_siul2_info *info;
   281		struct nxp_siul2_mfd *priv;
   282		void __iomem *reg_start;
   283		int i, ret;
   284	
   285		priv = platform_get_drvdata(pdev);
   286		info = &priv->siul2[siul];
   287	
   288		for (i = 0; i < SIUL2_NUM_REG_TYPES; i++) {
   289			if (!s32g2_reg_ranges[siul][i].valid)
   290				continue;
   291	
   292			tmp_range = &s32g2_reg_ranges[siul][i];
   293			tmp_conf = &regmap_configs[i];
   294			tmp_conf->name = tmp_range->reg_name;
   295			tmp_conf->max_register =
   296				tmp_range->reg_end_offset - tmp_range->reg_start_offset;
   297	
   298			if (tmp_conf->cache_type != REGCACHE_NONE)
   299				tmp_conf->num_reg_defaults_raw =
   300					tmp_conf->max_register / tmp_conf->reg_stride;
   301	
   302			if (tmp_range->reg_access) {
   303				tmp_conf->wr_table = tmp_range->reg_access;
   304				tmp_conf->rd_table = tmp_range->reg_access;
   305			}
   306	
   307			reg_start = base + tmp_range->reg_start_offset;
   308			info->regmaps[i] = devm_regmap_init_mmio(&pdev->dev, reg_start,
   309								 tmp_conf);
   310			if (IS_ERR(info->regmaps[i])) {
   311				dev_err(&pdev->dev, "regmap %d init failed: %d\n", i,
 > 312					ret);
   313				return PTR_ERR(info->regmaps[i]);
   314			}
   315		}
   316	
   317		return 0;
   318	}
   319	
   320	static int nxp_siul2_parse_dtb(struct platform_device *pdev)
   321	{
   322		struct device_node *np = pdev->dev.of_node;
   323		struct of_phandle_args pinspec;
   324		struct nxp_siul2_mfd *priv;
   325		void __iomem *base;
   326		char reg_name[16];
   327		int i, ret;
   328	
   329		priv = platform_get_drvdata(pdev);
   330	
   331		for (i = 0; i < priv->num_siul2; i++) {
   332			ret = snprintf(reg_name, ARRAY_SIZE(reg_name), "siul2%d", i);
   333			if (ret < 0 || ret >= ARRAY_SIZE(reg_name))
   334				return ret;
   335	
   336			base = devm_platform_ioremap_resource_byname(pdev, reg_name);
   337			if (IS_ERR(base)) {
   338				dev_err(&pdev->dev, "Failed to get MEM resource: %s\n",
   339					reg_name);
   340				return PTR_ERR(base);
   341			}
   342	
   343			ret = nxp_siul2_init_regmap(pdev, base, i);
   344			if (ret)
   345				return ret;
   346	
   347			ret = of_parse_phandle_with_fixed_args(np, "gpio-ranges", 3,
   348							       i, &pinspec);
   349			if (ret)
   350				return ret;
   351	
   352			of_node_put(pinspec.np);
   353	
   354			if (pinspec.args_count != 3) {
   355				dev_err(&pdev->dev, "Invalid pinspec count: %d\n",
   356					pinspec.args_count);
   357				return -EINVAL;
   358			}
   359	
   360			priv->siul2[i].gpio_base = pinspec.args[1];
   361			priv->siul2[i].gpio_num = pinspec.args[2];
   362		}
   363	
   364		return 0;
   365	}
   366	
 > 367	static int nxp_siul2_probe(struct platform_device *pdev)
   368	{
   369		struct nxp_siul2_mfd *priv;
   370		int ret;
   371	
   372		priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
   373		if (!priv)
   374			return -ENOMEM;
   375	
   376		priv->num_siul2 = S32G_NUM_SIUL2;
   377		priv->siul2 = devm_kcalloc(&pdev->dev, priv->num_siul2,
   378					   sizeof(*priv->siul2), GFP_KERNEL);
   379		if (!priv->siul2)
   380			return -ENOMEM;
   381	
   382		platform_set_drvdata(pdev, priv);
   383		ret = nxp_siul2_parse_dtb(pdev);
   384		if (ret)
   385			return ret;
   386	
   387		return devm_mfd_add_devices(&pdev->dev, PLATFORM_DEVID_AUTO,
   388					    nxp_siul2_devs, ARRAY_SIZE(nxp_siul2_devs),
   389					    NULL, 0, NULL);
   390	}
   391	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5 2/7] mfd: nxp-siul2: add support for NXP SIUL2
  2024-11-01  8:06 ` [PATCH v5 2/7] mfd: nxp-siul2: add support for NXP SIUL2 Andrei Stefanescu
                     ` (2 preceding siblings ...)
  2024-11-02  5:13   ` kernel test robot
@ 2024-11-02  8:37   ` Dan Carpenter
  2024-11-04 11:25     ` Andrei Stefanescu
  2024-11-02  8:52   ` Krzysztof Kozlowski
  4 siblings, 1 reply; 32+ messages in thread
From: Dan Carpenter @ 2024-11-02  8:37 UTC (permalink / raw)
  To: oe-kbuild, Andrei Stefanescu, Linus Walleij, Bartosz Golaszewski,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Chester Lin,
	Matthias Brugger, Ghennadi Procopciuc, Larisa Grigore,
	Greg Kroah-Hartman, Rafael J. Wysocki, Lee Jones, Shawn Guo,
	Sascha Hauer, Fabio Estevam, Dong Aisheng, Jacky Bai
  Cc: lkp, oe-kbuild-all, linux-gpio, devicetree, linux-kernel,
	linux-arm-kernel, NXP S32 Linux Team, Christophe Lizzi,
	Alberto Ruiz, Enric Balletbo, Pengutronix Kernel Team, imx

Hi Andrei,

kernel test robot noticed the following build warnings:

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/20241101-160940
base:   https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git devel
patch link:    https://lore.kernel.org/r/20241101080614.1070819-3-andrei.stefanescu%40oss.nxp.com
patch subject: [PATCH v5 2/7] mfd: nxp-siul2: add support for NXP SIUL2
config: csky-randconfig-r072-20241102 (https://download.01.org/0day-ci/archive/20241102/202411021431.282g2yZy-lkp@intel.com/config)
compiler: csky-linux-gcc (GCC) 14.1.0

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>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202411021431.282g2yZy-lkp@intel.com/

smatch warnings:
drivers/mfd/nxp-siul2.c:311 nxp_siul2_init_regmap() error: uninitialized symbol 'ret'.

vim +/ret +311 drivers/mfd/nxp-siul2.c

5c0b3edcf6df17 Andrei Stefanescu 2024-11-01  267  static int nxp_siul2_init_regmap(struct platform_device *pdev,
5c0b3edcf6df17 Andrei Stefanescu 2024-11-01  268  				 void __iomem *base, int siul)
5c0b3edcf6df17 Andrei Stefanescu 2024-11-01  269  {
5c0b3edcf6df17 Andrei Stefanescu 2024-11-01  270  	struct regmap_config regmap_configs[SIUL2_NUM_REG_TYPES] = {
5c0b3edcf6df17 Andrei Stefanescu 2024-11-01  271  		[SIUL2_MPIDR]	= nxp_siul2_regmap_generic_conf,
5c0b3edcf6df17 Andrei Stefanescu 2024-11-01  272  		[SIUL2_IRQ]	= nxp_siul2_regmap_irq_conf,
5c0b3edcf6df17 Andrei Stefanescu 2024-11-01  273  		[SIUL2_MSCR]	= nxp_siul2_regmap_generic_conf,
5c0b3edcf6df17 Andrei Stefanescu 2024-11-01  274  		[SIUL2_IMCR]	= nxp_siul2_regmap_generic_conf,
5c0b3edcf6df17 Andrei Stefanescu 2024-11-01  275  		[SIUL2_PGPDO]	= nxp_siul2_regmap_pgpdo_conf,
5c0b3edcf6df17 Andrei Stefanescu 2024-11-01  276  		[SIUL2_PGPDI]	= nxp_siul2_regmap_pgpdi_conf,
5c0b3edcf6df17 Andrei Stefanescu 2024-11-01  277  	};
5c0b3edcf6df17 Andrei Stefanescu 2024-11-01  278  	const struct nxp_siul2_reg_range_info *tmp_range;
5c0b3edcf6df17 Andrei Stefanescu 2024-11-01  279  	struct regmap_config *tmp_conf;
5c0b3edcf6df17 Andrei Stefanescu 2024-11-01  280  	struct nxp_siul2_info *info;
5c0b3edcf6df17 Andrei Stefanescu 2024-11-01  281  	struct nxp_siul2_mfd *priv;
5c0b3edcf6df17 Andrei Stefanescu 2024-11-01  282  	void __iomem *reg_start;
5c0b3edcf6df17 Andrei Stefanescu 2024-11-01  283  	int i, ret;
5c0b3edcf6df17 Andrei Stefanescu 2024-11-01  284  
5c0b3edcf6df17 Andrei Stefanescu 2024-11-01  285  	priv = platform_get_drvdata(pdev);
5c0b3edcf6df17 Andrei Stefanescu 2024-11-01  286  	info = &priv->siul2[siul];
5c0b3edcf6df17 Andrei Stefanescu 2024-11-01  287  
5c0b3edcf6df17 Andrei Stefanescu 2024-11-01  288  	for (i = 0; i < SIUL2_NUM_REG_TYPES; i++) {
5c0b3edcf6df17 Andrei Stefanescu 2024-11-01  289  		if (!s32g2_reg_ranges[siul][i].valid)
5c0b3edcf6df17 Andrei Stefanescu 2024-11-01  290  			continue;
5c0b3edcf6df17 Andrei Stefanescu 2024-11-01  291  
5c0b3edcf6df17 Andrei Stefanescu 2024-11-01  292  		tmp_range = &s32g2_reg_ranges[siul][i];
5c0b3edcf6df17 Andrei Stefanescu 2024-11-01  293  		tmp_conf = &regmap_configs[i];
5c0b3edcf6df17 Andrei Stefanescu 2024-11-01  294  		tmp_conf->name = tmp_range->reg_name;
5c0b3edcf6df17 Andrei Stefanescu 2024-11-01  295  		tmp_conf->max_register =
5c0b3edcf6df17 Andrei Stefanescu 2024-11-01  296  			tmp_range->reg_end_offset - tmp_range->reg_start_offset;
5c0b3edcf6df17 Andrei Stefanescu 2024-11-01  297  
5c0b3edcf6df17 Andrei Stefanescu 2024-11-01  298  		if (tmp_conf->cache_type != REGCACHE_NONE)
5c0b3edcf6df17 Andrei Stefanescu 2024-11-01  299  			tmp_conf->num_reg_defaults_raw =
5c0b3edcf6df17 Andrei Stefanescu 2024-11-01  300  				tmp_conf->max_register / tmp_conf->reg_stride;
5c0b3edcf6df17 Andrei Stefanescu 2024-11-01  301  
5c0b3edcf6df17 Andrei Stefanescu 2024-11-01  302  		if (tmp_range->reg_access) {
5c0b3edcf6df17 Andrei Stefanescu 2024-11-01  303  			tmp_conf->wr_table = tmp_range->reg_access;
5c0b3edcf6df17 Andrei Stefanescu 2024-11-01  304  			tmp_conf->rd_table = tmp_range->reg_access;
5c0b3edcf6df17 Andrei Stefanescu 2024-11-01  305  		}
5c0b3edcf6df17 Andrei Stefanescu 2024-11-01  306  
5c0b3edcf6df17 Andrei Stefanescu 2024-11-01  307  		reg_start = base + tmp_range->reg_start_offset;
5c0b3edcf6df17 Andrei Stefanescu 2024-11-01  308  		info->regmaps[i] = devm_regmap_init_mmio(&pdev->dev, reg_start,
5c0b3edcf6df17 Andrei Stefanescu 2024-11-01  309  							 tmp_conf);
5c0b3edcf6df17 Andrei Stefanescu 2024-11-01  310  		if (IS_ERR(info->regmaps[i])) {
5c0b3edcf6df17 Andrei Stefanescu 2024-11-01 @311  			dev_err(&pdev->dev, "regmap %d init failed: %d\n", i,
5c0b3edcf6df17 Andrei Stefanescu 2024-11-01  312  				ret);

Do this like: dev_err(&pdev->dev, "regmap %d init failed: %pe\n", i, info->regmaps[i]);

5c0b3edcf6df17 Andrei Stefanescu 2024-11-01  313  			return PTR_ERR(info->regmaps[i]);
5c0b3edcf6df17 Andrei Stefanescu 2024-11-01  314  		}
5c0b3edcf6df17 Andrei Stefanescu 2024-11-01  315  	}
5c0b3edcf6df17 Andrei Stefanescu 2024-11-01  316  
5c0b3edcf6df17 Andrei Stefanescu 2024-11-01  317  	return 0;
5c0b3edcf6df17 Andrei Stefanescu 2024-11-01  318  }

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5 1/7] dt-bindings: mfd: add support for the NXP SIUL2 module
  2024-11-01  8:06 ` [PATCH v5 1/7] dt-bindings: mfd: add support for the NXP SIUL2 module Andrei Stefanescu
  2024-11-01 18:11   ` Frank Li
@ 2024-11-02  8:42   ` Krzysztof Kozlowski
  2024-11-04 11:21     ` Andrei Stefanescu
  1 sibling, 1 reply; 32+ messages in thread
From: Krzysztof Kozlowski @ 2024-11-02  8:42 UTC (permalink / raw)
  To: Andrei Stefanescu
  Cc: Linus Walleij, Bartosz Golaszewski, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Chester Lin, Matthias Brugger,
	Ghennadi Procopciuc, Larisa Grigore, Greg Kroah-Hartman,
	Rafael J. Wysocki, Lee Jones, Shawn Guo, Sascha Hauer,
	Fabio Estevam, Dong Aisheng, Jacky Bai, linux-gpio, devicetree,
	linux-kernel, linux-arm-kernel, NXP S32 Linux Team,
	Christophe Lizzi, Alberto Ruiz, Enric Balletbo,
	Pengutronix Kernel Team, imx

On Fri, Nov 01, 2024 at 10:06:07AM +0200, 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>
> ---
>  .../devicetree/bindings/mfd/nxp,siul2.yaml    | 191 ++++++++++++++++++
>  1 file changed, 191 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/nxp,siul2.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mfd/nxp,siul2.yaml b/Documentation/devicetree/bindings/mfd/nxp,siul2.yaml
> new file mode 100644
> index 000000000000..141ec1219821
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/nxp,siul2.yaml

filename based on compatible, e.g. nxp,s32g2-siul2.yaml

> @@ -0,0 +1,191 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright 2024 NXP
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/nxp,siul2.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: System Integration Unit Lite2 (SIUL2)

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:
> +    enum:
> +      - nxp,s32g2-siul2
> +      - nxp,s32g3-siul2
> +
> +  reg:
> +    items:
> +      - description: SIUL2_0 module memory
> +      - description: SIUL2_1 module memory
> +
> +  reg-names:
> +    items:
> +      - const: siul20
> +      - const: siul21
> +
> +  gpio-controller: true
> +
> +  '#gpio-cells':
> +    const: 2
> +
> +  gpio-ranges:
> +    minItems: 2

You can drop minItems

> +    maxItems: 2
> +
> +  gpio-reserved-ranges:
> +    minItems: 2

Missing maxItems

> +
> +  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]

missing required: block

> +
> +        additionalProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +  - reg-names
> +  - gpio-controller
> +  - "#gpio-cells"

Keep consistent quotes, ' or "

> +  - gpio-ranges
> +  - gpio-reserved-ranges
> +  - interrupts
> +  - interrupt-controller
> +  - "#interrupt-cells"
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    siul2: siul2@4009c000 {

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation

e.g. pinctrl

> +      compatible = "nxp,s32g2-siul2";
> +      reg = <0x4009c000 0x179c>,
> +            <0x44010000 0x17b0>;
> +      reg-names = "siul20", "siul21";

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5 3/7] arm64: dts: s32g: make pinctrl part of mfd node
  2024-11-01  8:06 ` [PATCH v5 3/7] arm64: dts: s32g: make pinctrl part of mfd node Andrei Stefanescu
  2024-11-01 18:22   ` Frank Li
@ 2024-11-02  8:49   ` Krzysztof Kozlowski
  1 sibling, 0 replies; 32+ messages in thread
From: Krzysztof Kozlowski @ 2024-11-02  8:49 UTC (permalink / raw)
  To: Andrei Stefanescu
  Cc: Linus Walleij, Bartosz Golaszewski, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Chester Lin, Matthias Brugger,
	Ghennadi Procopciuc, Larisa Grigore, Greg Kroah-Hartman,
	Rafael J. Wysocki, Lee Jones, Shawn Guo, Sascha Hauer,
	Fabio Estevam, Dong Aisheng, Jacky Bai, linux-gpio, devicetree,
	linux-kernel, linux-arm-kernel, NXP S32 Linux Team,
	Christophe Lizzi, Alberto Ruiz, Enric Balletbo,
	Pengutronix Kernel Team, imx

On Fri, Nov 01, 2024 at 10:06:09AM +0200, Andrei Stefanescu wrote:
> SIUL2 is now represented as an mfd device. Therefore, the old
> pinctrl node is deprecated. Move the pinctrl related properties
> inside the new "nxp-siul2" node. The latter one is now used
> to represent the mfd device.
> 
> Signed-off-by: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
> ---
>  arch/arm64/boot/dts/freescale/s32g2.dtsi | 26 +++++++++++-------------
>  arch/arm64/boot/dts/freescale/s32g3.dtsi | 26 +++++++++++-------------
>  2 files changed, 24 insertions(+), 28 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/freescale/s32g2.dtsi b/arch/arm64/boot/dts/freescale/s32g2.dtsi
> index fa054bfe7d5c..e14ce5503e1f 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>;
> +		siul2: siul2@4009c000 {

This will affect users of this DTS. Your commit lacks rationale for
doing this, considering its impact. Re-describing devices is really
unexpected so should come with strong arguments.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5 4/7] pinctrl: s32: convert the driver into an mfd cell
  2024-11-01  8:06 ` [PATCH v5 4/7] pinctrl: s32: convert the driver into an mfd cell Andrei Stefanescu
  2024-11-01 11:53   ` Linus Walleij
  2024-11-01 21:47   ` kernel test robot
@ 2024-11-02  8:51   ` Krzysztof Kozlowski
  2024-11-04 11:44     ` Andrei Stefanescu
  2 siblings, 1 reply; 32+ messages in thread
From: Krzysztof Kozlowski @ 2024-11-02  8:51 UTC (permalink / raw)
  To: Andrei Stefanescu
  Cc: Linus Walleij, Bartosz Golaszewski, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Chester Lin, Matthias Brugger,
	Ghennadi Procopciuc, Larisa Grigore, Greg Kroah-Hartman,
	Rafael J. Wysocki, Lee Jones, Shawn Guo, Sascha Hauer,
	Fabio Estevam, Dong Aisheng, Jacky Bai, linux-gpio, devicetree,
	linux-kernel, linux-arm-kernel, NXP S32 Linux Team,
	Christophe Lizzi, Alberto Ruiz, Enric Balletbo,
	Pengutronix Kernel Team, imx

On Fri, Nov 01, 2024 at 10:06:10AM +0200, Andrei Stefanescu wrote:
> +	/* Order is MSCR regions first, then IMCR ones */
>  	for (i = 0; i < mem_regions; i++) {
> -		base = devm_platform_get_and_ioremap_resource(pdev, i, &res);
> -		if (IS_ERR(base))
> -			return PTR_ERR(base);
> -
> -		snprintf(ipctl->regions[i].name,
> -			 sizeof(ipctl->regions[i].name), "map%u", i);
> -
> -		s32_regmap_config.name = ipctl->regions[i].name;
> -		s32_regmap_config.max_register = resource_size(res) -
> -						 s32_regmap_config.reg_stride;
> -
> -		map = devm_regmap_init_mmio(&pdev->dev, base,
> -						&s32_regmap_config);
> -		if (IS_ERR(map)) {
> -			dev_err(&pdev->dev, "Failed to init regmap[%u]\n", i);
> -			return PTR_ERR(map);
> -		}
> -
> -		ipctl->regions[i].map = map;
> +		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];

This looks like breaking all the users. Nothing in commit msg about
this: about rationale, impact or backwards compatibility.

Nothing in changelog in cover letter explaining such sudden change in
approach.

Sorry, that's a NAK.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5 2/7] mfd: nxp-siul2: add support for NXP SIUL2
  2024-11-01  8:06 ` [PATCH v5 2/7] mfd: nxp-siul2: add support for NXP SIUL2 Andrei Stefanescu
                     ` (3 preceding siblings ...)
  2024-11-02  8:37   ` Dan Carpenter
@ 2024-11-02  8:52   ` Krzysztof Kozlowski
  2024-11-04 11:29     ` Andrei Stefanescu
  4 siblings, 1 reply; 32+ messages in thread
From: Krzysztof Kozlowski @ 2024-11-02  8:52 UTC (permalink / raw)
  To: Andrei Stefanescu
  Cc: Linus Walleij, Bartosz Golaszewski, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Chester Lin, Matthias Brugger,
	Ghennadi Procopciuc, Larisa Grigore, Greg Kroah-Hartman,
	Rafael J. Wysocki, Lee Jones, Shawn Guo, Sascha Hauer,
	Fabio Estevam, Dong Aisheng, Jacky Bai, linux-gpio, devicetree,
	linux-kernel, linux-arm-kernel, NXP S32 Linux Team,
	Christophe Lizzi, Alberto Ruiz, Enric Balletbo,
	Pengutronix Kernel Team, imx

On Fri, Nov 01, 2024 at 10:06:08AM +0200, Andrei Stefanescu wrote:
> +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" },

So devices are comaptible? Why doesn't your binding express it?

> +	{ },
> +};
> +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,
> +};

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5 1/7] dt-bindings: mfd: add support for the NXP SIUL2 module
  2024-11-01 18:11   ` Frank Li
@ 2024-11-04 11:11     ` Andrei Stefanescu
  0 siblings, 0 replies; 32+ messages in thread
From: Andrei Stefanescu @ 2024-11-04 11:11 UTC (permalink / raw)
  To: Frank Li
  Cc: Linus Walleij, Bartosz Golaszewski, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Chester Lin, Matthias Brugger,
	Ghennadi Procopciuc, Larisa Grigore, Greg Kroah-Hartman,
	Rafael J. Wysocki, Lee Jones, Shawn Guo, Sascha Hauer,
	Fabio Estevam, Dong Aisheng, Jacky Bai, linux-gpio, devicetree,
	linux-kernel, linux-arm-kernel, NXP S32 Linux Team,
	Christophe Lizzi, Alberto Ruiz, Enric Balletbo,
	Pengutronix Kernel Team, imx

Hi Frank,

Thank you for your review!

>> +      - description: SIUL2_1 module memory
> 
> description have not provide more informaiton.
> maxItems: 2 should be enough here.
>

I will fix in v6.

>> +
>> +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
> 
> suppose needn't such common property, if use
> 	unevaluatedProperties: false

This part was taken from pinctrl/nxp,s32g2-siul2-pinctrl.yaml and, if I remember
correctly, feedback from that patch's review was to explicitly specify which
properties are supported by this binding. Would it be ok to keep this section
as-is(with all of the supported properties and the additionalProperties: false)?

>> +
>> +          pinmux:
>> +            description: |
> 
> needn't "|" here
> 
>> +              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)

I need it here because of the "pinmux = (PIN_ID << 4 | SSS)" part. 

>> +
>> +          slew-rate:
>> +            description: Supported slew rate based on Fmax values (MHz)
>> +            enum: [83, 133, 150, 166, 208]
>> +
>> +        additionalProperties: false
> 
> It should be unevaluatedProperties: false because there $ref.

Do you mean to change "additionalProperties:false" to "unevaluatedProperties:false"?
If I understand correctly "additionalProperties:false" only allows the explicitly mentioned
subset of properties from other schemas whereas "unevaluatedProperties:false" allows all
properties from other schemas. I would like to permit only the mentioned properties. Would
that be ok?


>> +  - |
>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>> +    #include <dt-bindings/interrupt-controller/irq.h>
>> +
>> +    siul2: siul2@4009c000 {
> 
> needn't label siul2.

I will fix in v6.

>> +
>> +        jtag-grp1 {
>> +          pinmux = <0x11>;
>> +          slew-rate = <166>;
>> +        };
> 
> one example should be enough.

I will fix in v6.


Best regards,
Andrei

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5 1/7] dt-bindings: mfd: add support for the NXP SIUL2 module
  2024-11-02  8:42   ` Krzysztof Kozlowski
@ 2024-11-04 11:21     ` Andrei Stefanescu
  0 siblings, 0 replies; 32+ messages in thread
From: Andrei Stefanescu @ 2024-11-04 11:21 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Linus Walleij, Bartosz Golaszewski, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Chester Lin, Matthias Brugger,
	Ghennadi Procopciuc, Larisa Grigore, Greg Kroah-Hartman,
	Rafael J. Wysocki, Lee Jones, Shawn Guo, Sascha Hauer,
	Fabio Estevam, Dong Aisheng, Jacky Bai, linux-gpio, devicetree,
	linux-kernel, linux-arm-kernel, NXP S32 Linux Team,
	Christophe Lizzi, Alberto Ruiz, Enric Balletbo,
	Pengutronix Kernel Team, imx

Hi Krzysztof,

Thank you for your review!

>> +
>> +    siul2: siul2@4009c000 {
> 
> Node names should be generic. See also an explanation and list of
> examples (not exhaustive) in DT specification:
> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
> 
> e.g. pinctrl

I wasn't sure which name to pick since it's responsible for pinctrl, GPIO and, in the future,
nvmem cells. I guess I can choose pinctrl if you think it's ok.

I will address all of the other comments in v6.

Best regards,
Andrei

> 
>> +      compatible = "nxp,s32g2-siul2";
>> +      reg = <0x4009c000 0x179c>,
>> +            <0x44010000 0x17b0>;
>> +      reg-names = "siul20", "siul21";
> 
> Best regards,
> Krzysztof
> 


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5 2/7] mfd: nxp-siul2: add support for NXP SIUL2
  2024-11-02  8:37   ` Dan Carpenter
@ 2024-11-04 11:25     ` Andrei Stefanescu
  0 siblings, 0 replies; 32+ messages in thread
From: Andrei Stefanescu @ 2024-11-04 11:25 UTC (permalink / raw)
  To: Dan Carpenter, oe-kbuild, Linus Walleij, Bartosz Golaszewski,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Chester Lin,
	Matthias Brugger, Ghennadi Procopciuc, Larisa Grigore,
	Greg Kroah-Hartman, Rafael J. Wysocki, Lee Jones, Shawn Guo,
	Sascha Hauer, Fabio Estevam, Dong Aisheng, Jacky Bai
  Cc: lkp, oe-kbuild-all, linux-gpio, devicetree, linux-kernel,
	linux-arm-kernel, NXP S32 Linux Team, Christophe Lizzi,
	Alberto Ruiz, Enric Balletbo, Pengutronix Kernel Team, imx

Hi Dan,

On 02/11/2024 10:37, Dan Carpenter wrote:
> Hi Andrei,
> 
> kernel test robot noticed the following build warnings:

Thank you very much! I will fix this and all of the other kernel
test robot errors in v6.

Best regards,
Andrei

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5 2/7] mfd: nxp-siul2: add support for NXP SIUL2
  2024-11-02  8:52   ` Krzysztof Kozlowski
@ 2024-11-04 11:29     ` Andrei Stefanescu
  2024-11-19  9:19       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 32+ messages in thread
From: Andrei Stefanescu @ 2024-11-04 11:29 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Linus Walleij, Bartosz Golaszewski, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Chester Lin, Matthias Brugger,
	Ghennadi Procopciuc, Larisa Grigore, Greg Kroah-Hartman,
	Rafael J. Wysocki, Lee Jones, Shawn Guo, Sascha Hauer,
	Fabio Estevam, Dong Aisheng, Jacky Bai, linux-gpio, devicetree,
	linux-kernel, linux-arm-kernel, NXP S32 Linux Team,
	Christophe Lizzi, Alberto Ruiz, Enric Balletbo,
	Pengutronix Kernel Team, imx

Hi Krzysztof,

On 02/11/2024 10:52, Krzysztof Kozlowski wrote:
> On Fri, Nov 01, 2024 at 10:06:08AM +0200, Andrei Stefanescu wrote:
>> +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" },
> 
> So devices are comaptible? Why doesn't your binding express it?

Yes, as far as I know, there is no difference in the integration
of the SIUL2 module for S32G2 and S32G3 SoCs. I am not sure how
to express this compatibility. Should I mention the "nxp,s32g3-siul2"
compatible as a fallback one?

Best regards,
Andrei

> 
>> +	{ },
>> +};
>> +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,
>> +};
> 
> Best regards,
> Krzysztof
> 


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5 3/7] arm64: dts: s32g: make pinctrl part of mfd node
  2024-11-01 18:22   ` Frank Li
@ 2024-11-04 11:33     ` Andrei Stefanescu
  0 siblings, 0 replies; 32+ messages in thread
From: Andrei Stefanescu @ 2024-11-04 11:33 UTC (permalink / raw)
  To: Frank Li
  Cc: Linus Walleij, Bartosz Golaszewski, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Chester Lin, Matthias Brugger,
	Ghennadi Procopciuc, Larisa Grigore, Greg Kroah-Hartman,
	Rafael J. Wysocki, Lee Jones, Shawn Guo, Sascha Hauer,
	Fabio Estevam, Dong Aisheng, Jacky Bai, linux-gpio, devicetree,
	linux-kernel, linux-arm-kernel, NXP S32 Linux Team,
	Christophe Lizzi, Alberto Ruiz, Enric Balletbo,
	Pengutronix Kernel Team, imx

Hi Frank,

On 01/11/2024 20:22, Frank Li wrote:
> On Fri, Nov 01, 2024 at 10:06:09AM +0200, Andrei Stefanescu wrote:
>> SIUL2 is now represented as an mfd device. Therefore, the old
>> pinctrl node is deprecated. Move the pinctrl related properties
>> inside the new "nxp-siul2" node. The latter one is now used
>> to represent the mfd device.
> 
> Generally, dt team want you to keep both for sometime to keep
> back compatiblity.

Just to make sure I understand correctly, I will keep the previous pinctrl
node and have both the new siul2 node and the old pinctrl one, right?

Best regards,
Andrei

> 
> Frank
>>
>> Signed-off-by: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
>> ---
>>  arch/arm64/boot/dts/freescale/s32g2.dtsi | 26 +++++++++++-------------
>>  arch/arm64/boot/dts/freescale/s32g3.dtsi | 26 +++++++++++-------------
>>  2 files changed, 24 insertions(+), 28 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/freescale/s32g2.dtsi b/arch/arm64/boot/dts/freescale/s32g2.dtsi
>> index fa054bfe7d5c..e14ce5503e1f 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>;
>> +		siul2: siul2@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>;
>> +			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 {
>> diff --git a/arch/arm64/boot/dts/freescale/s32g3.dtsi b/arch/arm64/boot/dts/freescale/s32g3.dtsi
>> index b4226a9143c8..fa43d036686f 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>;
>> +		siul2: siul2@4009c000 {
>> +			compatible = "nxp,s32g3-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>;
>> +			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 {
>> --
>> 2.45.2
>>


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5 4/7] pinctrl: s32: convert the driver into an mfd cell
  2024-11-02  8:51   ` Krzysztof Kozlowski
@ 2024-11-04 11:44     ` Andrei Stefanescu
  0 siblings, 0 replies; 32+ messages in thread
From: Andrei Stefanescu @ 2024-11-04 11:44 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Linus Walleij, Bartosz Golaszewski, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Chester Lin, Matthias Brugger,
	Ghennadi Procopciuc, Larisa Grigore, Greg Kroah-Hartman,
	Rafael J. Wysocki, Lee Jones, Shawn Guo, Sascha Hauer,
	Fabio Estevam, Dong Aisheng, Jacky Bai, linux-gpio, devicetree,
	linux-kernel, linux-arm-kernel, NXP S32 Linux Team,
	Christophe Lizzi, Alberto Ruiz, Enric Balletbo,
	Pengutronix Kernel Team, imx

Hi Krzysztof,

On 02/11/2024 10:51, Krzysztof Kozlowski wrote:
> On Fri, Nov 01, 2024 at 10:06:10AM +0200, Andrei Stefanescu wrote:
>> +	/* Order is MSCR regions first, then IMCR ones */
>>  	for (i = 0; i < mem_regions; i++) {
>> -		base = devm_platform_get_and_ioremap_resource(pdev, i, &res);
>> -		if (IS_ERR(base))
>> -			return PTR_ERR(base);
>> -
>> -		snprintf(ipctl->regions[i].name,
>> -			 sizeof(ipctl->regions[i].name), "map%u", i);
>> -
>> -		s32_regmap_config.name = ipctl->regions[i].name;
>> -		s32_regmap_config.max_register = resource_size(res) -
>> -						 s32_regmap_config.reg_stride;
>> -
>> -		map = devm_regmap_init_mmio(&pdev->dev, base,
>> -						&s32_regmap_config);
>> -		if (IS_ERR(map)) {
>> -			dev_err(&pdev->dev, "Failed to init regmap[%u]\n", i);
>> -			return PTR_ERR(map);
>> -		}
>> -
>> -		ipctl->regions[i].map = map;
>> +		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];
> 
> This looks like breaking all the users. Nothing in commit msg about
> this: about rationale, impact or backwards compatibility.
> 
> Nothing in changelog in cover letter explaining such sudden change in
> approach.
> 
> Sorry, that's a NAK.

I will add a detailed explanation for the change in v6. I changed the existing
implementation because of feedback received in this series and in [0]. The SIUL2
module has the pinctrl&GPIO functionality tightly coupled, which required us to
carve out the registers between GPIO & pinctrl and it resulted in a long and detailed
list.

Also, in the future, we will also have some nvmem cells exported which are part
of SIUL2.

Best regards,
Andrei

[0] - https://lore.kernel.org/linux-gpio/20241002135920.3647322-1-andrei.stefanescu@oss.nxp.com/

> 
> Best regards,
> Krzysztof
> 


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5 6/7] pinctrl: s32cc: add driver for GPIO functionality
  2024-11-01 15:45   ` Markus Elfring
@ 2024-11-04 11:45     ` Andrei Stefanescu
  0 siblings, 0 replies; 32+ messages in thread
From: Andrei Stefanescu @ 2024-11-04 11:45 UTC (permalink / raw)
  To: Markus Elfring, linux-gpio, devicetree, s32, Bartosz Golaszewski,
	Chester Lin, Conor Dooley, Dong Aisheng, Fabio Estevam,
	Ghennadi Procopciuc, Greg Kroah-Hartman, Jacky Bai,
	Krzysztof Kozlowski, Larisa Grigore, Lee Jones, Linus Walleij,
	Matthias Brugger, Rafael J. Wysocki, Rob Herring, Sascha Hauer,
	Shawn Guo
  Cc: LKML, imx, linux-arm-kernel, kernel, Alberto Ruiz,
	Christophe Lizzi, Enric Balletbo

Hi Markus,

On 01/11/2024 17:45, Markus Elfring wrote:
>> Add basic GPIO functionality (request, free, get, set) for the existing
>> pinctrl SIUL2 driver since the hardware for pinctrl&GPIO is tightly
>> coupled.
> …
>> +++ b/drivers/pinctrl/nxp/pinctrl-s32cc.c
> …
>> +static int s32_gpio_request(struct gpio_chip *gc, unsigned int gpio)
>> +{
> …
>> +	spin_lock_irqsave(&ipctl->gpio_configs_lock, flags);
>> +	list_add(&gpio_pin->list, &ipctl->gpio_configs);
>> +	spin_unlock_irqrestore(&ipctl->gpio_configs_lock, flags);
> …
> 
> Under which circumstances would you become interested to apply a statement
> like “guard(spinlock_irqsave)(&ipctl->gpio_configs_lock);”?

Thank you for the suggestion! I will fix it in v6.

Best regards,
Andrei

> https://elixir.bootlin.com/linux/v6.12-rc5/source/include/linux/spinlock.h#L551
> 
> Regards,
> Markus


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v5 2/7] mfd: nxp-siul2: add support for NXP SIUL2
  2024-11-04 11:29     ` Andrei Stefanescu
@ 2024-11-19  9:19       ` Krzysztof Kozlowski
  0 siblings, 0 replies; 32+ messages in thread
From: Krzysztof Kozlowski @ 2024-11-19  9:19 UTC (permalink / raw)
  To: Andrei Stefanescu
  Cc: Linus Walleij, Bartosz Golaszewski, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Chester Lin, Matthias Brugger,
	Ghennadi Procopciuc, Larisa Grigore, Greg Kroah-Hartman,
	Rafael J. Wysocki, Lee Jones, Shawn Guo, Sascha Hauer,
	Fabio Estevam, Dong Aisheng, Jacky Bai, linux-gpio, devicetree,
	linux-kernel, linux-arm-kernel, NXP S32 Linux Team,
	Christophe Lizzi, Alberto Ruiz, Enric Balletbo,
	Pengutronix Kernel Team, imx

On 04/11/2024 12:29, Andrei Stefanescu wrote:
> Hi Krzysztof,
> 
> On 02/11/2024 10:52, Krzysztof Kozlowski wrote:
>> On Fri, Nov 01, 2024 at 10:06:08AM +0200, Andrei Stefanescu wrote:
>>> +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" },
>>
>> So devices are comaptible? Why doesn't your binding express it?
> 
> Yes, as far as I know, there is no difference in the integration
> of the SIUL2 module for S32G2 and S32G3 SoCs. I am not sure how
> to express this compatibility. Should I mention the "nxp,s32g3-siul2"
> compatible as a fallback one?

See example schema. Or any other recent NXP IMX binding.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 32+ messages in thread

end of thread, other threads:[~2024-11-19  9:19 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-11-01  8:06 [PATCH v5 0/7] gpio: siul2-s32g2: add initial GPIO driver Andrei Stefanescu
2024-11-01  8:06 ` [PATCH v5 1/7] dt-bindings: mfd: add support for the NXP SIUL2 module Andrei Stefanescu
2024-11-01 18:11   ` Frank Li
2024-11-04 11:11     ` Andrei Stefanescu
2024-11-02  8:42   ` Krzysztof Kozlowski
2024-11-04 11:21     ` Andrei Stefanescu
2024-11-01  8:06 ` [PATCH v5 2/7] mfd: nxp-siul2: add support for NXP SIUL2 Andrei Stefanescu
2024-11-01 17:18   ` kernel test robot
2024-11-01 18:20   ` Frank Li
2024-11-02  5:13   ` kernel test robot
2024-11-02  8:37   ` Dan Carpenter
2024-11-04 11:25     ` Andrei Stefanescu
2024-11-02  8:52   ` Krzysztof Kozlowski
2024-11-04 11:29     ` Andrei Stefanescu
2024-11-19  9:19       ` Krzysztof Kozlowski
2024-11-01  8:06 ` [PATCH v5 3/7] arm64: dts: s32g: make pinctrl part of mfd node Andrei Stefanescu
2024-11-01 18:22   ` Frank Li
2024-11-04 11:33     ` Andrei Stefanescu
2024-11-02  8:49   ` Krzysztof Kozlowski
2024-11-01  8:06 ` [PATCH v5 4/7] pinctrl: s32: convert the driver into an mfd cell Andrei Stefanescu
2024-11-01 11:53   ` Linus Walleij
2024-11-01 21:47   ` kernel test robot
2024-11-02  8:51   ` Krzysztof Kozlowski
2024-11-04 11:44     ` Andrei Stefanescu
2024-11-01  8:06 ` [PATCH v5 5/7] pinctrl: s32cc: change to "devm_pinctrl_register_and_init" Andrei Stefanescu
2024-11-01 11:54   ` Linus Walleij
2024-11-01  8:06 ` [PATCH v5 6/7] pinctrl: s32cc: add driver for GPIO functionality Andrei Stefanescu
2024-11-01 15:45   ` Markus Elfring
2024-11-04 11:45     ` Andrei Stefanescu
2024-11-01  8:06 ` [PATCH v5 7/7] MAINTAINERS: add MAINTAINER for NXP SIUL2 MFD driver Andrei Stefanescu
2024-11-01 11:56 ` [PATCH v5 0/7] gpio: siul2-s32g2: add initial GPIO driver Linus Walleij
2024-11-01 12:10   ` Andrei Stefanescu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).