All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/6] pinctrl: scmi: support i.MX95 OEM extensions
@ 2024-04-28  5:07 ` Peng Fan (OSS)
  0 siblings, 0 replies; 22+ messages in thread
From: Peng Fan (OSS) @ 2024-04-28  5:07 UTC (permalink / raw)
  To: Sudeep Holla, Cristian Marussi, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Linus Walleij, Dong Aisheng, Jacky Bai
  Cc: linux-arm-kernel, devicetree, linux-kernel, imx, linux-gpio, Peng Fan

ARM SCMI v3.2 Table 24 Pin Configuration Type and Enumerations:
'192 -255 OEM specific units'.

i.MX95 System Manager FW supports SCMI PINCTRL protocol, but it has zero
functions, groups. So pinctrl-scmi.c could not be reused for i.MX95.
Because nxp,pin-func, nxp,pin-conf properties are rejected by dt
maintainers, so use generic property 'pinmux' which requires a new driver
pinctrl-imx-scmi.c

The node will be as below:
 pinctrl_usdhc1: usdhc1-pins {
         sd1-grp0 {
                 pinmux = <IMX95_PAD_SD1_CLK__USDHC1_CLK
                           IMX95_PAD_SD1_STROBE__USDHC1_STROBE>;
                 drive-strength = <0xe>;
                 input-schmitt-enable;
                 bias-pull-down;
                 slew-rate = <0x3>;
         };
         sd1-grp1 {
                 pinmux = <IMX95_PAD_SD1_CMD__USDHC1_CMD
                           IMX95_PAD_SD1_DATA0__USDHC1_DATA0
                           IMX95_PAD_SD1_DATA1__USDHC1_DATA1
                           IMX95_PAD_SD1_DATA2__USDHC1_DATA2
                           IMX95_PAD_SD1_DATA3__USDHC1_DATA3
                           IMX95_PAD_SD1_DATA4__USDHC1_DATA4
                           IMX95_PAD_SD1_DATA5__USDHC1_DATA5
                           IMX95_PAD_SD1_DATA6__USDHC1_DATA6
                           IMX95_PAD_SD1_DATA7__USDHC1_DATA7>;
                 drive-strength = <0xe>;
                 input-schmitt-enable;
                 bias-pull-up;
                 slew-rate = <0x3>;
         };
 };

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
Changes in v3:
- patch 2,3,4,5 are new.
- Rewrite the binding, drop nxp,pin-x properties, use generic properties
  as Rob commented.
- Switch to using pinmux means pinctrl-scmi.c could not be reused, so
  add a new driver in patch 6 for i.MX95. But pinctrl_scmi_get_pins and
  scmi_pinctrl are exported for i.MX95 usage.
- Link to v2: https://lore.kernel.org/r/20240418-pinctrl-scmi-oem-v1-v2-0-3a555a3c58c3@nxp.com

Changes in v2:
- Rename nxp,imx95-pinctrl.yaml  to nxp,imx95-scmi-pinctrl.yaml and move
  to firmware
- Merged patch [1,2]/3 v1 into patch 1/2 v2.
- nxp,imx95-scmi-pinctrl.yaml only has patterProperties for subnode
  The pinctrl will be as below for i.MX95.
        pinctrl_usdhc1: usdhc1-pins {
                sd1cmd {
                        pins = "sd1cmd";
                        nxp,func-id = <0>;
                        nxp,pin-conf = <0x138e>;
                };
                sd1data {
                        pins = "sd1data";
                        nxp,func-id = <0>;
                        nxp,pin-conf = <0x138e>;
                };
        };
- Add pins enum, correct description.
- Link to v1: https://lore.kernel.org/r/20240412-pinctrl-scmi-oem-v1-v1-0-704f242544c1@nxp.com

---
Peng Fan (6):
      dt-bindings: firmware: arm,scmi: Add properties for i.MX95 Pinctrl OEM extensions
      pinctrl: scmi: move pinctrl_ops to scmi_pinctrl
      pinctrl: core: guard with __PINCTRL_CORE_H
      pinctrl: scmi: export pinctrl_scmi_get_pins
      pinctrl: scmi: add blocklist
      pinctrl: imx: support SCMI pinctrl protocol for i.MX95

 .../devicetree/bindings/firmware/arm,scmi.yaml     |   9 +-
 .../bindings/firmware/nxp,imx95-scmi-pinctrl.yaml  |  37 ++
 drivers/pinctrl/core.h                             |   4 +
 drivers/pinctrl/freescale/Kconfig                  |   7 +
 drivers/pinctrl/freescale/Makefile                 |   1 +
 drivers/pinctrl/freescale/pinctrl-imx-scmi.c       | 574 +++++++++++++++++++++
 drivers/pinctrl/pinctrl-scmi.c                     |  60 +--
 drivers/pinctrl/pinctrl-scmi.h                     |  30 ++
 8 files changed, 689 insertions(+), 33 deletions(-)
---
base-commit: bb7a2467e6beef44a80a17d45ebf2931e7631083
change-id: 20240428-pinctrl-scmi-oem-v3-12130031a74d

Best regards,
-- 
Peng Fan <peng.fan@nxp.com>


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

* [PATCH v3 0/6] pinctrl: scmi: support i.MX95 OEM extensions
@ 2024-04-28  5:07 ` Peng Fan (OSS)
  0 siblings, 0 replies; 22+ messages in thread
From: Peng Fan (OSS) @ 2024-04-28  5:07 UTC (permalink / raw)
  To: Sudeep Holla, Cristian Marussi, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Linus Walleij, Dong Aisheng, Jacky Bai
  Cc: linux-arm-kernel, devicetree, linux-kernel, imx, linux-gpio, Peng Fan

ARM SCMI v3.2 Table 24 Pin Configuration Type and Enumerations:
'192 -255 OEM specific units'.

i.MX95 System Manager FW supports SCMI PINCTRL protocol, but it has zero
functions, groups. So pinctrl-scmi.c could not be reused for i.MX95.
Because nxp,pin-func, nxp,pin-conf properties are rejected by dt
maintainers, so use generic property 'pinmux' which requires a new driver
pinctrl-imx-scmi.c

The node will be as below:
 pinctrl_usdhc1: usdhc1-pins {
         sd1-grp0 {
                 pinmux = <IMX95_PAD_SD1_CLK__USDHC1_CLK
                           IMX95_PAD_SD1_STROBE__USDHC1_STROBE>;
                 drive-strength = <0xe>;
                 input-schmitt-enable;
                 bias-pull-down;
                 slew-rate = <0x3>;
         };
         sd1-grp1 {
                 pinmux = <IMX95_PAD_SD1_CMD__USDHC1_CMD
                           IMX95_PAD_SD1_DATA0__USDHC1_DATA0
                           IMX95_PAD_SD1_DATA1__USDHC1_DATA1
                           IMX95_PAD_SD1_DATA2__USDHC1_DATA2
                           IMX95_PAD_SD1_DATA3__USDHC1_DATA3
                           IMX95_PAD_SD1_DATA4__USDHC1_DATA4
                           IMX95_PAD_SD1_DATA5__USDHC1_DATA5
                           IMX95_PAD_SD1_DATA6__USDHC1_DATA6
                           IMX95_PAD_SD1_DATA7__USDHC1_DATA7>;
                 drive-strength = <0xe>;
                 input-schmitt-enable;
                 bias-pull-up;
                 slew-rate = <0x3>;
         };
 };

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
Changes in v3:
- patch 2,3,4,5 are new.
- Rewrite the binding, drop nxp,pin-x properties, use generic properties
  as Rob commented.
- Switch to using pinmux means pinctrl-scmi.c could not be reused, so
  add a new driver in patch 6 for i.MX95. But pinctrl_scmi_get_pins and
  scmi_pinctrl are exported for i.MX95 usage.
- Link to v2: https://lore.kernel.org/r/20240418-pinctrl-scmi-oem-v1-v2-0-3a555a3c58c3@nxp.com

Changes in v2:
- Rename nxp,imx95-pinctrl.yaml  to nxp,imx95-scmi-pinctrl.yaml and move
  to firmware
- Merged patch [1,2]/3 v1 into patch 1/2 v2.
- nxp,imx95-scmi-pinctrl.yaml only has patterProperties for subnode
  The pinctrl will be as below for i.MX95.
        pinctrl_usdhc1: usdhc1-pins {
                sd1cmd {
                        pins = "sd1cmd";
                        nxp,func-id = <0>;
                        nxp,pin-conf = <0x138e>;
                };
                sd1data {
                        pins = "sd1data";
                        nxp,func-id = <0>;
                        nxp,pin-conf = <0x138e>;
                };
        };
- Add pins enum, correct description.
- Link to v1: https://lore.kernel.org/r/20240412-pinctrl-scmi-oem-v1-v1-0-704f242544c1@nxp.com

---
Peng Fan (6):
      dt-bindings: firmware: arm,scmi: Add properties for i.MX95 Pinctrl OEM extensions
      pinctrl: scmi: move pinctrl_ops to scmi_pinctrl
      pinctrl: core: guard with __PINCTRL_CORE_H
      pinctrl: scmi: export pinctrl_scmi_get_pins
      pinctrl: scmi: add blocklist
      pinctrl: imx: support SCMI pinctrl protocol for i.MX95

 .../devicetree/bindings/firmware/arm,scmi.yaml     |   9 +-
 .../bindings/firmware/nxp,imx95-scmi-pinctrl.yaml  |  37 ++
 drivers/pinctrl/core.h                             |   4 +
 drivers/pinctrl/freescale/Kconfig                  |   7 +
 drivers/pinctrl/freescale/Makefile                 |   1 +
 drivers/pinctrl/freescale/pinctrl-imx-scmi.c       | 574 +++++++++++++++++++++
 drivers/pinctrl/pinctrl-scmi.c                     |  60 +--
 drivers/pinctrl/pinctrl-scmi.h                     |  30 ++
 8 files changed, 689 insertions(+), 33 deletions(-)
---
base-commit: bb7a2467e6beef44a80a17d45ebf2931e7631083
change-id: 20240428-pinctrl-scmi-oem-v3-12130031a74d

Best regards,
-- 
Peng Fan <peng.fan@nxp.com>


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH v3 1/6] dt-bindings: firmware: arm,scmi: Add properties for i.MX95 Pinctrl OEM extensions
  2024-04-28  5:07 ` Peng Fan (OSS)
@ 2024-04-28  5:07   ` Peng Fan (OSS)
  -1 siblings, 0 replies; 22+ messages in thread
From: Peng Fan (OSS) @ 2024-04-28  5:07 UTC (permalink / raw)
  To: Sudeep Holla, Cristian Marussi, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Linus Walleij, Dong Aisheng, Jacky Bai
  Cc: linux-arm-kernel, devicetree, linux-kernel, imx, linux-gpio, Peng Fan

From: Peng Fan <peng.fan@nxp.com>

i.MX95 Pinctrl is managed by System Control Management Interface(SCMI)
firmware using OEM extensions. No functions, no groups are provided by
the firmware. To reuse generic properties, add the binding to enable
pinmux, slew-rate, bias-pull-up and etc, under a subnode of '-pins'.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 .../devicetree/bindings/firmware/arm,scmi.yaml     |  9 ++++--
 .../bindings/firmware/nxp,imx95-scmi-pinctrl.yaml  | 37 ++++++++++++++++++++++
 2 files changed, 43 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
index 7de2c29606e5..bd4dfd7a85cd 100644
--- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
+++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
@@ -262,9 +262,12 @@ properties:
     patternProperties:
       '-pins$':
         type: object
-        allOf:
-          - $ref: /schemas/pinctrl/pincfg-node.yaml#
-          - $ref: /schemas/pinctrl/pinmux-node.yaml#
+        anyOf:
+          - $ref: /schemas/firmware/nxp,imx95-scmi-pinctrl.yaml
+          - allOf:
+              - $ref: /schemas/pinctrl/pincfg-node.yaml#
+              - $ref: /schemas/pinctrl/pinmux-node.yaml#
+
         unevaluatedProperties: false
 
         description:
diff --git a/Documentation/devicetree/bindings/firmware/nxp,imx95-scmi-pinctrl.yaml b/Documentation/devicetree/bindings/firmware/nxp,imx95-scmi-pinctrl.yaml
new file mode 100644
index 000000000000..1a694881f193
--- /dev/null
+++ b/Documentation/devicetree/bindings/firmware/nxp,imx95-scmi-pinctrl.yaml
@@ -0,0 +1,37 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2024 NXP
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/firmware/nxp,imx95-scmi-pinctrl.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: i.MX System Control and Management Interface (SCMI) Pinctrl Protocol
+
+maintainers:
+  - Peng Fan <peng.fan@nxp.com>
+
+patternProperties:
+  'grp[0-9a-f]$':
+    type: object
+    unevaluatedProperties: false
+
+    properties:
+      pinmux:
+        description: |
+          An integer array for representing pinmux configurations of
+          a device. Each integer has the format, pinid[31:21], mux[20:16],
+          daisy_value[15:12], daisy_valid[11:11], daisy_id[10:0].
+        $ref: /schemas/types.yaml#/definitions/uint32-array
+
+      drive-strength:
+        enum: [ 0, 1, 3, 7, 15, 31, 63 ]
+
+      slew-rate:
+        enum: [2, 3]
+
+      input-schmitt-enable: true
+      drive-open-drain: true
+      bias-pull-up: true
+      bias-pull-down: true
+
+additionalProperties: true

-- 
2.37.1


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

* [PATCH v3 1/6] dt-bindings: firmware: arm,scmi: Add properties for i.MX95 Pinctrl OEM extensions
@ 2024-04-28  5:07   ` Peng Fan (OSS)
  0 siblings, 0 replies; 22+ messages in thread
From: Peng Fan (OSS) @ 2024-04-28  5:07 UTC (permalink / raw)
  To: Sudeep Holla, Cristian Marussi, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Linus Walleij, Dong Aisheng, Jacky Bai
  Cc: linux-arm-kernel, devicetree, linux-kernel, imx, linux-gpio, Peng Fan

From: Peng Fan <peng.fan@nxp.com>

i.MX95 Pinctrl is managed by System Control Management Interface(SCMI)
firmware using OEM extensions. No functions, no groups are provided by
the firmware. To reuse generic properties, add the binding to enable
pinmux, slew-rate, bias-pull-up and etc, under a subnode of '-pins'.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 .../devicetree/bindings/firmware/arm,scmi.yaml     |  9 ++++--
 .../bindings/firmware/nxp,imx95-scmi-pinctrl.yaml  | 37 ++++++++++++++++++++++
 2 files changed, 43 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
index 7de2c29606e5..bd4dfd7a85cd 100644
--- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
+++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
@@ -262,9 +262,12 @@ properties:
     patternProperties:
       '-pins$':
         type: object
-        allOf:
-          - $ref: /schemas/pinctrl/pincfg-node.yaml#
-          - $ref: /schemas/pinctrl/pinmux-node.yaml#
+        anyOf:
+          - $ref: /schemas/firmware/nxp,imx95-scmi-pinctrl.yaml
+          - allOf:
+              - $ref: /schemas/pinctrl/pincfg-node.yaml#
+              - $ref: /schemas/pinctrl/pinmux-node.yaml#
+
         unevaluatedProperties: false
 
         description:
diff --git a/Documentation/devicetree/bindings/firmware/nxp,imx95-scmi-pinctrl.yaml b/Documentation/devicetree/bindings/firmware/nxp,imx95-scmi-pinctrl.yaml
new file mode 100644
index 000000000000..1a694881f193
--- /dev/null
+++ b/Documentation/devicetree/bindings/firmware/nxp,imx95-scmi-pinctrl.yaml
@@ -0,0 +1,37 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2024 NXP
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/firmware/nxp,imx95-scmi-pinctrl.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: i.MX System Control and Management Interface (SCMI) Pinctrl Protocol
+
+maintainers:
+  - Peng Fan <peng.fan@nxp.com>
+
+patternProperties:
+  'grp[0-9a-f]$':
+    type: object
+    unevaluatedProperties: false
+
+    properties:
+      pinmux:
+        description: |
+          An integer array for representing pinmux configurations of
+          a device. Each integer has the format, pinid[31:21], mux[20:16],
+          daisy_value[15:12], daisy_valid[11:11], daisy_id[10:0].
+        $ref: /schemas/types.yaml#/definitions/uint32-array
+
+      drive-strength:
+        enum: [ 0, 1, 3, 7, 15, 31, 63 ]
+
+      slew-rate:
+        enum: [2, 3]
+
+      input-schmitt-enable: true
+      drive-open-drain: true
+      bias-pull-up: true
+      bias-pull-down: true
+
+additionalProperties: true

-- 
2.37.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH v3 2/6] pinctrl: scmi: move pinctrl_ops to scmi_pinctrl
  2024-04-28  5:07 ` Peng Fan (OSS)
@ 2024-04-28  5:07   ` Peng Fan (OSS)
  -1 siblings, 0 replies; 22+ messages in thread
From: Peng Fan (OSS) @ 2024-04-28  5:07 UTC (permalink / raw)
  To: Sudeep Holla, Cristian Marussi, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Linus Walleij, Dong Aisheng, Jacky Bai
  Cc: linux-arm-kernel, devicetree, linux-kernel, imx, linux-gpio, Peng Fan

From: Peng Fan <peng.fan@nxp.com>

Make pinctrl_ops a global variable not able to support multiple
protocol@19 nodes, so make it per scmi_pinctrl.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/pinctrl/pinctrl-scmi.c | 35 ++++++++++++++++++-----------------
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-scmi.c b/drivers/pinctrl/pinctrl-scmi.c
index 036bc1e3fc6c..682ff595c3c7 100644
--- a/drivers/pinctrl/pinctrl-scmi.c
+++ b/drivers/pinctrl/pinctrl-scmi.c
@@ -30,8 +30,6 @@
 /* Define num configs, if not large than 4 use stack, else use kcalloc() */
 #define SCMI_NUM_CONFIGS	4
 
-static const struct scmi_pinctrl_proto_ops *pinctrl_ops;
-
 struct scmi_pinctrl {
 	struct device *dev;
 	struct scmi_protocol_handle *ph;
@@ -41,13 +39,14 @@ struct scmi_pinctrl {
 	unsigned int nr_functions;
 	struct pinctrl_pin_desc *pins;
 	unsigned int nr_pins;
+	const struct scmi_pinctrl_proto_ops *ops;
 };
 
 static int pinctrl_scmi_get_groups_count(struct pinctrl_dev *pctldev)
 {
 	struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
 
-	return pinctrl_ops->count_get(pmx->ph, GROUP_TYPE);
+	return pmx->ops->count_get(pmx->ph, GROUP_TYPE);
 }
 
 static const char *pinctrl_scmi_get_group_name(struct pinctrl_dev *pctldev,
@@ -57,7 +56,7 @@ static const char *pinctrl_scmi_get_group_name(struct pinctrl_dev *pctldev,
 	const char *name;
 	struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
 
-	ret = pinctrl_ops->name_get(pmx->ph, selector, GROUP_TYPE, &name);
+	ret = pmx->ops->name_get(pmx->ph, selector, GROUP_TYPE, &name);
 	if (ret) {
 		dev_err(pmx->dev, "get name failed with err %d", ret);
 		return NULL;
@@ -73,7 +72,7 @@ static int pinctrl_scmi_get_group_pins(struct pinctrl_dev *pctldev,
 {
 	struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
 
-	return pinctrl_ops->group_pins_get(pmx->ph, selector, pins, num_pins);
+	return pmx->ops->group_pins_get(pmx->ph, selector, pins, num_pins);
 }
 
 static const struct pinctrl_ops pinctrl_scmi_pinctrl_ops = {
@@ -90,7 +89,7 @@ static int pinctrl_scmi_get_functions_count(struct pinctrl_dev *pctldev)
 {
 	struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
 
-	return pinctrl_ops->count_get(pmx->ph, FUNCTION_TYPE);
+	return pmx->ops->count_get(pmx->ph, FUNCTION_TYPE);
 }
 
 static const char *pinctrl_scmi_get_function_name(struct pinctrl_dev *pctldev,
@@ -100,7 +99,7 @@ static const char *pinctrl_scmi_get_function_name(struct pinctrl_dev *pctldev,
 	const char *name;
 	struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
 
-	ret = pinctrl_ops->name_get(pmx->ph, selector, FUNCTION_TYPE, &name);
+	ret = pmx->ops->name_get(pmx->ph, selector, FUNCTION_TYPE, &name);
 	if (ret) {
 		dev_err(pmx->dev, "get name failed with err %d", ret);
 		return NULL;
@@ -131,7 +130,7 @@ static int pinctrl_scmi_get_function_groups(struct pinctrl_dev *pctldev,
 	if (func->ngroups)
 		goto done;
 
-	ret = pinctrl_ops->function_groups_get(pmx->ph, selector, &num_groups,
+	ret = pmx->ops->function_groups_get(pmx->ph, selector, &num_groups,
 					       &group_ids);
 	if (ret) {
 		dev_err(pmx->dev, "Unable to get function groups, err %d", ret);
@@ -171,7 +170,7 @@ static int pinctrl_scmi_func_set_mux(struct pinctrl_dev *pctldev,
 {
 	struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
 
-	return pinctrl_ops->mux_set(pmx->ph, selector, group);
+	return pmx->ops->mux_set(pmx->ph, selector, group);
 }
 
 static int pinctrl_scmi_request(struct pinctrl_dev *pctldev,
@@ -179,14 +178,14 @@ static int pinctrl_scmi_request(struct pinctrl_dev *pctldev,
 {
 	struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
 
-	return pinctrl_ops->pin_request(pmx->ph, offset);
+	return pmx->ops->pin_request(pmx->ph, offset);
 }
 
 static int pinctrl_scmi_free(struct pinctrl_dev *pctldev, unsigned int offset)
 {
 	struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
 
-	return pinctrl_ops->pin_free(pmx->ph, offset);
+	return pmx->ops->pin_free(pmx->ph, offset);
 }
 
 static const struct pinmux_ops pinctrl_scmi_pinmux_ops = {
@@ -295,7 +294,7 @@ static int pinctrl_scmi_pinconf_get(struct pinctrl_dev *pctldev,
 	if (ret)
 		return ret;
 
-	ret = pinctrl_ops->settings_get_one(pmx->ph, pin, PIN_TYPE, type,
+	ret = pmx->ops->settings_get_one(pmx->ph, pin, PIN_TYPE, type,
 					    &config_value);
 	/* Convert SCMI error code to PINCTRL expected error code */
 	if (ret == -EOPNOTSUPP)
@@ -372,7 +371,7 @@ static int pinctrl_scmi_pinconf_set(struct pinctrl_dev *pctldev,
 		p_config_value[i] = pinconf_to_config_argument(configs[i]);
 	}
 
-	ret = pinctrl_ops->settings_conf(pmx->ph, pin, PIN_TYPE, num_configs,
+	ret = pmx->ops->settings_conf(pmx->ph, pin, PIN_TYPE, num_configs,
 					 p_config_type,  p_config_value);
 	if (ret)
 		dev_err(pmx->dev, "Error parsing config %d\n", ret);
@@ -415,7 +414,7 @@ static int pinctrl_scmi_pinconf_group_set(struct pinctrl_dev *pctldev,
 		p_config_value[i] = pinconf_to_config_argument(configs[i]);
 	}
 
-	ret = pinctrl_ops->settings_conf(pmx->ph, group, GROUP_TYPE,
+	ret = pmx->ops->settings_conf(pmx->ph, group, GROUP_TYPE,
 					 num_configs, p_config_type,
 					 p_config_value);
 	if (ret)
@@ -447,7 +446,7 @@ static int pinctrl_scmi_pinconf_group_get(struct pinctrl_dev *pctldev,
 		return ret;
 	}
 
-	ret = pinctrl_ops->settings_get_one(pmx->ph, group, GROUP_TYPE, type,
+	ret = pmx->ops->settings_get_one(pmx->ph, group, GROUP_TYPE, type,
 					    &config_value);
 	/* Convert SCMI error code to PINCTRL expected error code */
 	if (ret == -EOPNOTSUPP)
@@ -476,7 +475,7 @@ static int pinctrl_scmi_get_pins(struct scmi_pinctrl *pmx,
 	unsigned int npins;
 	int ret, i;
 
-	npins = pinctrl_ops->count_get(pmx->ph, PIN_TYPE);
+	npins = pmx->ops->count_get(pmx->ph, PIN_TYPE);
 	/*
 	 * npins will never be zero, the scmi pinctrl driver has bailed out
 	 * if npins is zero.
@@ -491,7 +490,7 @@ static int pinctrl_scmi_get_pins(struct scmi_pinctrl *pmx,
 		 * The memory for name is handled by the scmi firmware driver,
 		 * no need free here
 		 */
-		ret = pinctrl_ops->name_get(pmx->ph, i, PIN_TYPE, &pins[i].name);
+		ret = pmx->ops->name_get(pmx->ph, i, PIN_TYPE, &pins[i].name);
 		if (ret)
 			return dev_err_probe(pmx->dev, ret,
 					     "Can't get name for pin %d", i);
@@ -511,6 +510,7 @@ static int scmi_pinctrl_probe(struct scmi_device *sdev)
 	struct scmi_pinctrl *pmx;
 	const struct scmi_handle *handle;
 	struct scmi_protocol_handle *ph;
+	const struct scmi_pinctrl_proto_ops *pinctrl_ops;
 
 	if (!sdev->handle)
 		return -EINVAL;
@@ -526,6 +526,7 @@ static int scmi_pinctrl_probe(struct scmi_device *sdev)
 		return -ENOMEM;
 
 	pmx->ph = ph;
+	pmx->ops = pinctrl_ops;
 
 	pmx->dev = dev;
 	pmx->pctl_desc.name = DRV_NAME;

-- 
2.37.1


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

* [PATCH v3 2/6] pinctrl: scmi: move pinctrl_ops to scmi_pinctrl
@ 2024-04-28  5:07   ` Peng Fan (OSS)
  0 siblings, 0 replies; 22+ messages in thread
From: Peng Fan (OSS) @ 2024-04-28  5:07 UTC (permalink / raw)
  To: Sudeep Holla, Cristian Marussi, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Linus Walleij, Dong Aisheng, Jacky Bai
  Cc: linux-arm-kernel, devicetree, linux-kernel, imx, linux-gpio, Peng Fan

From: Peng Fan <peng.fan@nxp.com>

Make pinctrl_ops a global variable not able to support multiple
protocol@19 nodes, so make it per scmi_pinctrl.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/pinctrl/pinctrl-scmi.c | 35 ++++++++++++++++++-----------------
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-scmi.c b/drivers/pinctrl/pinctrl-scmi.c
index 036bc1e3fc6c..682ff595c3c7 100644
--- a/drivers/pinctrl/pinctrl-scmi.c
+++ b/drivers/pinctrl/pinctrl-scmi.c
@@ -30,8 +30,6 @@
 /* Define num configs, if not large than 4 use stack, else use kcalloc() */
 #define SCMI_NUM_CONFIGS	4
 
-static const struct scmi_pinctrl_proto_ops *pinctrl_ops;
-
 struct scmi_pinctrl {
 	struct device *dev;
 	struct scmi_protocol_handle *ph;
@@ -41,13 +39,14 @@ struct scmi_pinctrl {
 	unsigned int nr_functions;
 	struct pinctrl_pin_desc *pins;
 	unsigned int nr_pins;
+	const struct scmi_pinctrl_proto_ops *ops;
 };
 
 static int pinctrl_scmi_get_groups_count(struct pinctrl_dev *pctldev)
 {
 	struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
 
-	return pinctrl_ops->count_get(pmx->ph, GROUP_TYPE);
+	return pmx->ops->count_get(pmx->ph, GROUP_TYPE);
 }
 
 static const char *pinctrl_scmi_get_group_name(struct pinctrl_dev *pctldev,
@@ -57,7 +56,7 @@ static const char *pinctrl_scmi_get_group_name(struct pinctrl_dev *pctldev,
 	const char *name;
 	struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
 
-	ret = pinctrl_ops->name_get(pmx->ph, selector, GROUP_TYPE, &name);
+	ret = pmx->ops->name_get(pmx->ph, selector, GROUP_TYPE, &name);
 	if (ret) {
 		dev_err(pmx->dev, "get name failed with err %d", ret);
 		return NULL;
@@ -73,7 +72,7 @@ static int pinctrl_scmi_get_group_pins(struct pinctrl_dev *pctldev,
 {
 	struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
 
-	return pinctrl_ops->group_pins_get(pmx->ph, selector, pins, num_pins);
+	return pmx->ops->group_pins_get(pmx->ph, selector, pins, num_pins);
 }
 
 static const struct pinctrl_ops pinctrl_scmi_pinctrl_ops = {
@@ -90,7 +89,7 @@ static int pinctrl_scmi_get_functions_count(struct pinctrl_dev *pctldev)
 {
 	struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
 
-	return pinctrl_ops->count_get(pmx->ph, FUNCTION_TYPE);
+	return pmx->ops->count_get(pmx->ph, FUNCTION_TYPE);
 }
 
 static const char *pinctrl_scmi_get_function_name(struct pinctrl_dev *pctldev,
@@ -100,7 +99,7 @@ static const char *pinctrl_scmi_get_function_name(struct pinctrl_dev *pctldev,
 	const char *name;
 	struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
 
-	ret = pinctrl_ops->name_get(pmx->ph, selector, FUNCTION_TYPE, &name);
+	ret = pmx->ops->name_get(pmx->ph, selector, FUNCTION_TYPE, &name);
 	if (ret) {
 		dev_err(pmx->dev, "get name failed with err %d", ret);
 		return NULL;
@@ -131,7 +130,7 @@ static int pinctrl_scmi_get_function_groups(struct pinctrl_dev *pctldev,
 	if (func->ngroups)
 		goto done;
 
-	ret = pinctrl_ops->function_groups_get(pmx->ph, selector, &num_groups,
+	ret = pmx->ops->function_groups_get(pmx->ph, selector, &num_groups,
 					       &group_ids);
 	if (ret) {
 		dev_err(pmx->dev, "Unable to get function groups, err %d", ret);
@@ -171,7 +170,7 @@ static int pinctrl_scmi_func_set_mux(struct pinctrl_dev *pctldev,
 {
 	struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
 
-	return pinctrl_ops->mux_set(pmx->ph, selector, group);
+	return pmx->ops->mux_set(pmx->ph, selector, group);
 }
 
 static int pinctrl_scmi_request(struct pinctrl_dev *pctldev,
@@ -179,14 +178,14 @@ static int pinctrl_scmi_request(struct pinctrl_dev *pctldev,
 {
 	struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
 
-	return pinctrl_ops->pin_request(pmx->ph, offset);
+	return pmx->ops->pin_request(pmx->ph, offset);
 }
 
 static int pinctrl_scmi_free(struct pinctrl_dev *pctldev, unsigned int offset)
 {
 	struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
 
-	return pinctrl_ops->pin_free(pmx->ph, offset);
+	return pmx->ops->pin_free(pmx->ph, offset);
 }
 
 static const struct pinmux_ops pinctrl_scmi_pinmux_ops = {
@@ -295,7 +294,7 @@ static int pinctrl_scmi_pinconf_get(struct pinctrl_dev *pctldev,
 	if (ret)
 		return ret;
 
-	ret = pinctrl_ops->settings_get_one(pmx->ph, pin, PIN_TYPE, type,
+	ret = pmx->ops->settings_get_one(pmx->ph, pin, PIN_TYPE, type,
 					    &config_value);
 	/* Convert SCMI error code to PINCTRL expected error code */
 	if (ret == -EOPNOTSUPP)
@@ -372,7 +371,7 @@ static int pinctrl_scmi_pinconf_set(struct pinctrl_dev *pctldev,
 		p_config_value[i] = pinconf_to_config_argument(configs[i]);
 	}
 
-	ret = pinctrl_ops->settings_conf(pmx->ph, pin, PIN_TYPE, num_configs,
+	ret = pmx->ops->settings_conf(pmx->ph, pin, PIN_TYPE, num_configs,
 					 p_config_type,  p_config_value);
 	if (ret)
 		dev_err(pmx->dev, "Error parsing config %d\n", ret);
@@ -415,7 +414,7 @@ static int pinctrl_scmi_pinconf_group_set(struct pinctrl_dev *pctldev,
 		p_config_value[i] = pinconf_to_config_argument(configs[i]);
 	}
 
-	ret = pinctrl_ops->settings_conf(pmx->ph, group, GROUP_TYPE,
+	ret = pmx->ops->settings_conf(pmx->ph, group, GROUP_TYPE,
 					 num_configs, p_config_type,
 					 p_config_value);
 	if (ret)
@@ -447,7 +446,7 @@ static int pinctrl_scmi_pinconf_group_get(struct pinctrl_dev *pctldev,
 		return ret;
 	}
 
-	ret = pinctrl_ops->settings_get_one(pmx->ph, group, GROUP_TYPE, type,
+	ret = pmx->ops->settings_get_one(pmx->ph, group, GROUP_TYPE, type,
 					    &config_value);
 	/* Convert SCMI error code to PINCTRL expected error code */
 	if (ret == -EOPNOTSUPP)
@@ -476,7 +475,7 @@ static int pinctrl_scmi_get_pins(struct scmi_pinctrl *pmx,
 	unsigned int npins;
 	int ret, i;
 
-	npins = pinctrl_ops->count_get(pmx->ph, PIN_TYPE);
+	npins = pmx->ops->count_get(pmx->ph, PIN_TYPE);
 	/*
 	 * npins will never be zero, the scmi pinctrl driver has bailed out
 	 * if npins is zero.
@@ -491,7 +490,7 @@ static int pinctrl_scmi_get_pins(struct scmi_pinctrl *pmx,
 		 * The memory for name is handled by the scmi firmware driver,
 		 * no need free here
 		 */
-		ret = pinctrl_ops->name_get(pmx->ph, i, PIN_TYPE, &pins[i].name);
+		ret = pmx->ops->name_get(pmx->ph, i, PIN_TYPE, &pins[i].name);
 		if (ret)
 			return dev_err_probe(pmx->dev, ret,
 					     "Can't get name for pin %d", i);
@@ -511,6 +510,7 @@ static int scmi_pinctrl_probe(struct scmi_device *sdev)
 	struct scmi_pinctrl *pmx;
 	const struct scmi_handle *handle;
 	struct scmi_protocol_handle *ph;
+	const struct scmi_pinctrl_proto_ops *pinctrl_ops;
 
 	if (!sdev->handle)
 		return -EINVAL;
@@ -526,6 +526,7 @@ static int scmi_pinctrl_probe(struct scmi_device *sdev)
 		return -ENOMEM;
 
 	pmx->ph = ph;
+	pmx->ops = pinctrl_ops;
 
 	pmx->dev = dev;
 	pmx->pctl_desc.name = DRV_NAME;

-- 
2.37.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH v3 3/6] pinctrl: core: guard with __PINCTRL_CORE_H
  2024-04-28  5:07 ` Peng Fan (OSS)
@ 2024-04-28  5:07   ` Peng Fan (OSS)
  -1 siblings, 0 replies; 22+ messages in thread
From: Peng Fan (OSS) @ 2024-04-28  5:07 UTC (permalink / raw)
  To: Sudeep Holla, Cristian Marussi, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Linus Walleij, Dong Aisheng, Jacky Bai
  Cc: linux-arm-kernel, devicetree, linux-kernel, imx, linux-gpio, Peng Fan

From: Peng Fan <peng.fan@nxp.com>

Guard the file with __PINCTRL_CORE_H to avoid build failure in case
core.h is included multiple times.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/pinctrl/core.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/pinctrl/core.h b/drivers/pinctrl/core.h
index 837fd5bd903d..1da7b0d329bc 100644
--- a/drivers/pinctrl/core.h
+++ b/drivers/pinctrl/core.h
@@ -8,6 +8,9 @@
  * Author: Linus Walleij <linus.walleij@linaro.org>
  */
 
+#ifndef __PINCTRL_CORE_H
+#define __PINCTRL_CORE_H
+
 #include <linux/kref.h>
 #include <linux/list.h>
 #include <linux/mutex.h>
@@ -262,3 +265,4 @@ extern struct list_head pinctrl_maps;
 		for (unsigned int __i = 0;						\
 		     __i < _maps_node_->num_maps && (_map_ = &_maps_node_->maps[__i]);	\
 		     __i++)
+#endif

-- 
2.37.1


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

* [PATCH v3 3/6] pinctrl: core: guard with __PINCTRL_CORE_H
@ 2024-04-28  5:07   ` Peng Fan (OSS)
  0 siblings, 0 replies; 22+ messages in thread
From: Peng Fan (OSS) @ 2024-04-28  5:07 UTC (permalink / raw)
  To: Sudeep Holla, Cristian Marussi, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Linus Walleij, Dong Aisheng, Jacky Bai
  Cc: linux-arm-kernel, devicetree, linux-kernel, imx, linux-gpio, Peng Fan

From: Peng Fan <peng.fan@nxp.com>

Guard the file with __PINCTRL_CORE_H to avoid build failure in case
core.h is included multiple times.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/pinctrl/core.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/pinctrl/core.h b/drivers/pinctrl/core.h
index 837fd5bd903d..1da7b0d329bc 100644
--- a/drivers/pinctrl/core.h
+++ b/drivers/pinctrl/core.h
@@ -8,6 +8,9 @@
  * Author: Linus Walleij <linus.walleij@linaro.org>
  */
 
+#ifndef __PINCTRL_CORE_H
+#define __PINCTRL_CORE_H
+
 #include <linux/kref.h>
 #include <linux/list.h>
 #include <linux/mutex.h>
@@ -262,3 +265,4 @@ extern struct list_head pinctrl_maps;
 		for (unsigned int __i = 0;						\
 		     __i < _maps_node_->num_maps && (_map_ = &_maps_node_->maps[__i]);	\
 		     __i++)
+#endif

-- 
2.37.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH v3 4/6] pinctrl: scmi: export pinctrl_scmi_get_pins
  2024-04-28  5:07 ` Peng Fan (OSS)
@ 2024-04-28  5:07   ` Peng Fan (OSS)
  -1 siblings, 0 replies; 22+ messages in thread
From: Peng Fan (OSS) @ 2024-04-28  5:07 UTC (permalink / raw)
  To: Sudeep Holla, Cristian Marussi, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Linus Walleij, Dong Aisheng, Jacky Bai
  Cc: linux-arm-kernel, devicetree, linux-kernel, imx, linux-gpio, Peng Fan

From: Peng Fan <peng.fan@nxp.com>

Add pinctrl-scmi.h to include the function prototype and 'struct
scmi_pinctrl' to export pinctrl_scmi_get_pins, so other drivers
could use it.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/pinctrl/pinctrl-scmi.c | 17 +++--------------
 drivers/pinctrl/pinctrl-scmi.h | 29 +++++++++++++++++++++++++++++
 2 files changed, 32 insertions(+), 14 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-scmi.c b/drivers/pinctrl/pinctrl-scmi.c
index 682ff595c3c7..360b813072df 100644
--- a/drivers/pinctrl/pinctrl-scmi.c
+++ b/drivers/pinctrl/pinctrl-scmi.c
@@ -21,6 +21,7 @@
 #include <linux/pinctrl/pinctrl.h>
 #include <linux/pinctrl/pinmux.h>
 
+#include "pinctrl-scmi.h"
 #include "pinctrl-utils.h"
 #include "core.h"
 #include "pinconf.h"
@@ -30,18 +31,6 @@
 /* Define num configs, if not large than 4 use stack, else use kcalloc() */
 #define SCMI_NUM_CONFIGS	4
 
-struct scmi_pinctrl {
-	struct device *dev;
-	struct scmi_protocol_handle *ph;
-	struct pinctrl_dev *pctldev;
-	struct pinctrl_desc pctl_desc;
-	struct pinfunction *functions;
-	unsigned int nr_functions;
-	struct pinctrl_pin_desc *pins;
-	unsigned int nr_pins;
-	const struct scmi_pinctrl_proto_ops *ops;
-};
-
 static int pinctrl_scmi_get_groups_count(struct pinctrl_dev *pctldev)
 {
 	struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
@@ -468,8 +457,7 @@ static const struct pinconf_ops pinctrl_scmi_pinconf_ops = {
 	.pin_config_config_dbg_show = pinconf_generic_dump_config,
 };
 
-static int pinctrl_scmi_get_pins(struct scmi_pinctrl *pmx,
-				 struct pinctrl_desc *desc)
+int pinctrl_scmi_get_pins(struct scmi_pinctrl *pmx, struct pinctrl_desc *desc)
 {
 	struct pinctrl_pin_desc *pins;
 	unsigned int npins;
@@ -502,6 +490,7 @@ static int pinctrl_scmi_get_pins(struct scmi_pinctrl *pmx,
 
 	return 0;
 }
+EXPORT_SYMBOL(pinctrl_scmi_get_pins);
 
 static int scmi_pinctrl_probe(struct scmi_device *sdev)
 {
diff --git a/drivers/pinctrl/pinctrl-scmi.h b/drivers/pinctrl/pinctrl-scmi.h
new file mode 100644
index 000000000000..ae9e0be7c89e
--- /dev/null
+++ b/drivers/pinctrl/pinctrl-scmi.h
@@ -0,0 +1,29 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright 2024 NXP
+ */
+
+#ifndef __DRIVERS_PINCTRL_SCMI_H
+#define __DRIVERS_PINCTRL_SCMI_H
+
+#include <linux/device.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/scmi_protocol.h>
+
+#include "core.h"
+
+struct scmi_pinctrl {
+	struct device *dev;
+	struct scmi_protocol_handle *ph;
+	struct pinctrl_dev *pctldev;
+	struct pinctrl_desc pctl_desc;
+	struct pinfunction *functions;
+	unsigned int nr_functions;
+	struct pinctrl_pin_desc *pins;
+	unsigned int nr_pins;
+	const struct scmi_pinctrl_proto_ops *ops;
+};
+
+int pinctrl_scmi_get_pins(struct scmi_pinctrl *pmx, struct pinctrl_desc *desc);
+
+#endif /* __DRIVERS_PINCTRL_SCMI_H */

-- 
2.37.1


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

* [PATCH v3 4/6] pinctrl: scmi: export pinctrl_scmi_get_pins
@ 2024-04-28  5:07   ` Peng Fan (OSS)
  0 siblings, 0 replies; 22+ messages in thread
From: Peng Fan (OSS) @ 2024-04-28  5:07 UTC (permalink / raw)
  To: Sudeep Holla, Cristian Marussi, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Linus Walleij, Dong Aisheng, Jacky Bai
  Cc: linux-arm-kernel, devicetree, linux-kernel, imx, linux-gpio, Peng Fan

From: Peng Fan <peng.fan@nxp.com>

Add pinctrl-scmi.h to include the function prototype and 'struct
scmi_pinctrl' to export pinctrl_scmi_get_pins, so other drivers
could use it.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/pinctrl/pinctrl-scmi.c | 17 +++--------------
 drivers/pinctrl/pinctrl-scmi.h | 29 +++++++++++++++++++++++++++++
 2 files changed, 32 insertions(+), 14 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-scmi.c b/drivers/pinctrl/pinctrl-scmi.c
index 682ff595c3c7..360b813072df 100644
--- a/drivers/pinctrl/pinctrl-scmi.c
+++ b/drivers/pinctrl/pinctrl-scmi.c
@@ -21,6 +21,7 @@
 #include <linux/pinctrl/pinctrl.h>
 #include <linux/pinctrl/pinmux.h>
 
+#include "pinctrl-scmi.h"
 #include "pinctrl-utils.h"
 #include "core.h"
 #include "pinconf.h"
@@ -30,18 +31,6 @@
 /* Define num configs, if not large than 4 use stack, else use kcalloc() */
 #define SCMI_NUM_CONFIGS	4
 
-struct scmi_pinctrl {
-	struct device *dev;
-	struct scmi_protocol_handle *ph;
-	struct pinctrl_dev *pctldev;
-	struct pinctrl_desc pctl_desc;
-	struct pinfunction *functions;
-	unsigned int nr_functions;
-	struct pinctrl_pin_desc *pins;
-	unsigned int nr_pins;
-	const struct scmi_pinctrl_proto_ops *ops;
-};
-
 static int pinctrl_scmi_get_groups_count(struct pinctrl_dev *pctldev)
 {
 	struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
@@ -468,8 +457,7 @@ static const struct pinconf_ops pinctrl_scmi_pinconf_ops = {
 	.pin_config_config_dbg_show = pinconf_generic_dump_config,
 };
 
-static int pinctrl_scmi_get_pins(struct scmi_pinctrl *pmx,
-				 struct pinctrl_desc *desc)
+int pinctrl_scmi_get_pins(struct scmi_pinctrl *pmx, struct pinctrl_desc *desc)
 {
 	struct pinctrl_pin_desc *pins;
 	unsigned int npins;
@@ -502,6 +490,7 @@ static int pinctrl_scmi_get_pins(struct scmi_pinctrl *pmx,
 
 	return 0;
 }
+EXPORT_SYMBOL(pinctrl_scmi_get_pins);
 
 static int scmi_pinctrl_probe(struct scmi_device *sdev)
 {
diff --git a/drivers/pinctrl/pinctrl-scmi.h b/drivers/pinctrl/pinctrl-scmi.h
new file mode 100644
index 000000000000..ae9e0be7c89e
--- /dev/null
+++ b/drivers/pinctrl/pinctrl-scmi.h
@@ -0,0 +1,29 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright 2024 NXP
+ */
+
+#ifndef __DRIVERS_PINCTRL_SCMI_H
+#define __DRIVERS_PINCTRL_SCMI_H
+
+#include <linux/device.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/scmi_protocol.h>
+
+#include "core.h"
+
+struct scmi_pinctrl {
+	struct device *dev;
+	struct scmi_protocol_handle *ph;
+	struct pinctrl_dev *pctldev;
+	struct pinctrl_desc pctl_desc;
+	struct pinfunction *functions;
+	unsigned int nr_functions;
+	struct pinctrl_pin_desc *pins;
+	unsigned int nr_pins;
+	const struct scmi_pinctrl_proto_ops *ops;
+};
+
+int pinctrl_scmi_get_pins(struct scmi_pinctrl *pmx, struct pinctrl_desc *desc);
+
+#endif /* __DRIVERS_PINCTRL_SCMI_H */

-- 
2.37.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH v3 5/6] pinctrl: scmi: add blocklist
  2024-04-28  5:07 ` Peng Fan (OSS)
@ 2024-04-28  5:07   ` Peng Fan (OSS)
  -1 siblings, 0 replies; 22+ messages in thread
From: Peng Fan (OSS) @ 2024-04-28  5:07 UTC (permalink / raw)
  To: Sudeep Holla, Cristian Marussi, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Linus Walleij, Dong Aisheng, Jacky Bai
  Cc: linux-arm-kernel, devicetree, linux-kernel, imx, linux-gpio, Peng Fan

From: Peng Fan <peng.fan@nxp.com>

i.MX95 will have its own pinctrl scmi driver, so need block
pinctrl-scmi driver for i.MX95, otherwise there will be two pinctrl
devices for a single scmi protocol@19.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/pinctrl/pinctrl-scmi.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/pinctrl/pinctrl-scmi.c b/drivers/pinctrl/pinctrl-scmi.c
index 360b813072df..8ff35869efea 100644
--- a/drivers/pinctrl/pinctrl-scmi.c
+++ b/drivers/pinctrl/pinctrl-scmi.c
@@ -11,6 +11,7 @@
 #include <linux/errno.h>
 #include <linux/module.h>
 #include <linux/mod_devicetable.h>
+#include <linux/of.h>
 #include <linux/scmi_protocol.h>
 #include <linux/slab.h>
 #include <linux/types.h>
@@ -492,6 +493,11 @@ int pinctrl_scmi_get_pins(struct scmi_pinctrl *pmx, struct pinctrl_desc *desc)
 }
 EXPORT_SYMBOL(pinctrl_scmi_get_pins);
 
+static const struct of_device_id scmi_pinctrl_blocklist[] = {
+	{ .compatible = "fsl,imx95", },
+	{ }
+};
+
 static int scmi_pinctrl_probe(struct scmi_device *sdev)
 {
 	int ret;
@@ -500,10 +506,14 @@ static int scmi_pinctrl_probe(struct scmi_device *sdev)
 	const struct scmi_handle *handle;
 	struct scmi_protocol_handle *ph;
 	const struct scmi_pinctrl_proto_ops *pinctrl_ops;
+	struct device_node *np __free(device_node) = of_find_node_by_path("/");
 
 	if (!sdev->handle)
 		return -EINVAL;
 
+	if (of_match_node(scmi_pinctrl_blocklist, np))
+		return -ENODEV;
+
 	handle = sdev->handle;
 
 	pinctrl_ops = handle->devm_protocol_get(sdev, SCMI_PROTOCOL_PINCTRL, &ph);

-- 
2.37.1


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

* [PATCH v3 5/6] pinctrl: scmi: add blocklist
@ 2024-04-28  5:07   ` Peng Fan (OSS)
  0 siblings, 0 replies; 22+ messages in thread
From: Peng Fan (OSS) @ 2024-04-28  5:07 UTC (permalink / raw)
  To: Sudeep Holla, Cristian Marussi, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Linus Walleij, Dong Aisheng, Jacky Bai
  Cc: linux-arm-kernel, devicetree, linux-kernel, imx, linux-gpio, Peng Fan

From: Peng Fan <peng.fan@nxp.com>

i.MX95 will have its own pinctrl scmi driver, so need block
pinctrl-scmi driver for i.MX95, otherwise there will be two pinctrl
devices for a single scmi protocol@19.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/pinctrl/pinctrl-scmi.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/pinctrl/pinctrl-scmi.c b/drivers/pinctrl/pinctrl-scmi.c
index 360b813072df..8ff35869efea 100644
--- a/drivers/pinctrl/pinctrl-scmi.c
+++ b/drivers/pinctrl/pinctrl-scmi.c
@@ -11,6 +11,7 @@
 #include <linux/errno.h>
 #include <linux/module.h>
 #include <linux/mod_devicetable.h>
+#include <linux/of.h>
 #include <linux/scmi_protocol.h>
 #include <linux/slab.h>
 #include <linux/types.h>
@@ -492,6 +493,11 @@ int pinctrl_scmi_get_pins(struct scmi_pinctrl *pmx, struct pinctrl_desc *desc)
 }
 EXPORT_SYMBOL(pinctrl_scmi_get_pins);
 
+static const struct of_device_id scmi_pinctrl_blocklist[] = {
+	{ .compatible = "fsl,imx95", },
+	{ }
+};
+
 static int scmi_pinctrl_probe(struct scmi_device *sdev)
 {
 	int ret;
@@ -500,10 +506,14 @@ static int scmi_pinctrl_probe(struct scmi_device *sdev)
 	const struct scmi_handle *handle;
 	struct scmi_protocol_handle *ph;
 	const struct scmi_pinctrl_proto_ops *pinctrl_ops;
+	struct device_node *np __free(device_node) = of_find_node_by_path("/");
 
 	if (!sdev->handle)
 		return -EINVAL;
 
+	if (of_match_node(scmi_pinctrl_blocklist, np))
+		return -ENODEV;
+
 	handle = sdev->handle;
 
 	pinctrl_ops = handle->devm_protocol_get(sdev, SCMI_PROTOCOL_PINCTRL, &ph);

-- 
2.37.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH v3 6/6] pinctrl: imx: support SCMI pinctrl protocol for i.MX95
  2024-04-28  5:07 ` Peng Fan (OSS)
@ 2024-04-28  5:07   ` Peng Fan (OSS)
  -1 siblings, 0 replies; 22+ messages in thread
From: Peng Fan (OSS) @ 2024-04-28  5:07 UTC (permalink / raw)
  To: Sudeep Holla, Cristian Marussi, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Linus Walleij, Dong Aisheng, Jacky Bai
  Cc: linux-arm-kernel, devicetree, linux-kernel, imx, linux-gpio, Peng Fan

From: Peng Fan <peng.fan@nxp.com>

The generic pinctrl-scmi.c driver could not be used for i.MX95 because
i.MX95 SCMI firmware not supports functions, groups or generic
'Pin Configuration Type and Enumerations' listed in SCMI Specification.

i.MX95 System Control Management Interface(SCMI) firmware only supports
below pin configuration types which are OEM specific types:
    192: PIN MUX
    193: PIN CONF
    194: DAISY ID
    195: DAISY VAL

To support Linux generic pinctrl properties(pinmux, bias-pull-[up,
down], and etc), need extract the value from the property and map
them to the format that i.MX95 SCMI pinctrl protocol understands,
so add this driver.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/pinctrl/freescale/Kconfig            |   7 +
 drivers/pinctrl/freescale/Makefile           |   1 +
 drivers/pinctrl/freescale/pinctrl-imx-scmi.c | 574 +++++++++++++++++++++++++++
 drivers/pinctrl/pinctrl-scmi.h               |   1 +
 4 files changed, 583 insertions(+)

diff --git a/drivers/pinctrl/freescale/Kconfig b/drivers/pinctrl/freescale/Kconfig
index 27bdc548f3a7..bc23d9f7b5bb 100644
--- a/drivers/pinctrl/freescale/Kconfig
+++ b/drivers/pinctrl/freescale/Kconfig
@@ -7,6 +7,13 @@ config PINCTRL_IMX
 	select PINCONF
 	select REGMAP
 
+config PINCTRL_IMX_SCMI
+	tristate "i.MX95 pinctrl driver using SCMI protocol interface"
+	depends on PINCTRL_SCMI
+	help
+	  i.MX95 SCMI firmware provides pinctrl protocol. This driver
+	  utilizes the SCMI interface to do pinctrl configuration.
+
 config PINCTRL_IMX_SCU
 	tristate
 	depends on IMX_SCU
diff --git a/drivers/pinctrl/freescale/Makefile b/drivers/pinctrl/freescale/Makefile
index 647dff060477..e79b4b06e71b 100644
--- a/drivers/pinctrl/freescale/Makefile
+++ b/drivers/pinctrl/freescale/Makefile
@@ -2,6 +2,7 @@
 # Freescale pin control drivers
 obj-$(CONFIG_PINCTRL_IMX)	+= pinctrl-imx.o
 obj-$(CONFIG_PINCTRL_IMX_SCU)	+= pinctrl-scu.o
+obj-$(CONFIG_PINCTRL_IMX_SCMI)	+= pinctrl-imx-scmi.o
 obj-$(CONFIG_PINCTRL_IMX1_CORE)	+= pinctrl-imx1-core.o
 obj-$(CONFIG_PINCTRL_IMX1)	+= pinctrl-imx1.o
 obj-$(CONFIG_PINCTRL_IMX27)	+= pinctrl-imx27.o
diff --git a/drivers/pinctrl/freescale/pinctrl-imx-scmi.c b/drivers/pinctrl/freescale/pinctrl-imx-scmi.c
new file mode 100644
index 000000000000..13c65ba51269
--- /dev/null
+++ b/drivers/pinctrl/freescale/pinctrl-imx-scmi.c
@@ -0,0 +1,574 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * System Control and Power Interface (SCMI) Protocol based i.MX pinctrl driver
+ *
+ * Copyright 2024 NXP
+ */
+
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/of.h>
+#include <linux/scmi_protocol.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+
+#include <linux/pinctrl/machine.h>
+#include <linux/pinctrl/pinconf.h>
+#include <linux/pinctrl/pinconf-generic.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinmux.h>
+
+#include "../pinctrl-scmi.h"
+#include "../pinctrl-utils.h"
+#include "../core.h"
+#include "../pinconf.h"
+#include "../pinmux.h"
+
+#define DRV_NAME "scmi-pinctrl-imx"
+
+#define SCMI_NUM_CONFIGS	4
+
+struct imx_pin_group {
+	struct pingroup data;
+};
+
+struct scmi_pinctrl_imx_info {
+	struct device *dev;
+	struct imx_pin_group *groups;
+	unsigned int ngroups;
+	struct pinfunction *functions;
+	unsigned int nfunctions;
+	unsigned int grp_index;
+};
+
+/* SCMI pin control types, aligned with SCMI firmware */
+#define IMX_SCMI_NUM_CFG	4
+#define IMX_SCMI_PIN_MUX	192
+#define IMX_SCMI_PIN_CONFIG	193
+#define IMX_SCMI_PIN_DAISY_ID	194
+#define IMX_SCMI_PIN_DAISY_CFG	195
+
+/*
+ * pinmux format:
+ * pin[31:21]|mux[20:16]|daisy_value[15:12]|daisy_valid[11:11]|daisy_id[10:0]
+ */
+#define IMX_PIN_ID_MASK		GENMASK(31, 21)
+#define IMX_PIN_MUX_MASK	GENMASK(20, 16)
+#define IMX_PIN_DAISY_VAL_MASK	GENMASK(15, 12)
+#define IMX_PIN_DAISY_VALID	BIT(11)
+#define IMX_PIN_DAISY_ID_MASK	GENMASK(10, 0)
+
+static inline u32 get_pin_no(u32 pinmux)
+{
+	return FIELD_GET(IMX_PIN_ID_MASK, pinmux);
+}
+
+static inline u32 get_pin_func(u32 pinmux)
+{
+	return FIELD_GET(IMX_PIN_MUX_MASK, pinmux);
+}
+
+static inline u32 get_pin_daisy_valid(u32 pinmux)
+{
+	return FIELD_GET(IMX_PIN_DAISY_VALID, pinmux);
+}
+
+static inline u32 get_pin_daisy_val(u32 pinmux)
+{
+	return FIELD_GET(IMX_PIN_DAISY_VAL_MASK, pinmux);
+}
+
+static inline u32 get_pin_daisy_no(u32 pinmux)
+{
+	return FIELD_GET(IMX_PIN_DAISY_ID_MASK, pinmux);
+}
+
+static int pinctrl_scmi_imx_map_pinconf_type(enum pin_config_param param,
+					     u32 *mask, u32 *shift)
+{
+	u32 arg = param;
+
+	switch (arg) {
+	case PIN_CONFIG_INPUT_SCHMITT_ENABLE:
+		*mask = BIT(12);
+		*shift = 12;
+		break;
+	case PIN_CONFIG_DRIVE_OPEN_DRAIN:
+		*mask = BIT(11);
+		*shift = 11;
+		break;
+	case PIN_CONFIG_BIAS_PULL_DOWN:
+		*mask = BIT(10);
+		*shift = 10;
+		break;
+	case PIN_CONFIG_BIAS_PULL_UP:
+		*mask = BIT(9);
+		*shift = 9;
+		break;
+	case PIN_CONFIG_SLEW_RATE:
+		*mask = GENMASK(8, 7);
+		*shift = 7;
+		break;
+	case PIN_CONFIG_DRIVE_STRENGTH:
+		*mask = GENMASK(6, 1);
+		*shift = 1;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int pinctrl_scmi_imx_dt_group_node_to_map(struct pinctrl_dev *pctldev,
+						 struct device_node *np,
+						 struct pinctrl_map **map,
+						 unsigned int *reserved_maps,
+						 unsigned int *num_maps,
+						 const char *func_name)
+{
+	struct device *dev = pctldev->dev;
+	unsigned long *cfgs = NULL;
+	unsigned int n_cfgs, reserve = 1;
+	int i, n_pins, ret;
+	u32 ncfg, val, mask, shift, pin_conf, pinmux_group;
+	unsigned long cfg[IMX_SCMI_NUM_CFG];
+	enum pin_config_param param;
+	struct property *prop;
+	const __be32 *p;
+
+	n_pins = of_property_count_u32_elems(np, "pinmux");
+	if (n_pins < 0) {
+		dev_warn(dev, "Can't find 'pinmux' property in node %pOFn\n", np);
+		return -EINVAL;
+	} else if (!n_pins) {
+		return -EINVAL;
+	}
+
+	ret = pinconf_generic_parse_dt_config(np, pctldev, &cfgs, &n_cfgs);
+	if (ret) {
+		dev_err(dev, "%pOF: could not parse node property\n", np);
+		return ret;
+	}
+
+	pin_conf = 0;
+	for (i = 0; i < n_cfgs; i++) {
+		param = pinconf_to_config_param(cfgs[i]);
+		ret = pinctrl_scmi_imx_map_pinconf_type(param, &mask, &shift);
+		if (ret) {
+			dev_err(dev, "Error map pinconf_type %d\n", ret);
+			return ret;
+		}
+
+		val = pinconf_to_config_argument(cfgs[i]);
+
+		pin_conf |= (val << shift) & mask;
+
+	}
+
+	reserve = n_pins * (1 + n_cfgs);
+
+	ret = pinctrl_utils_reserve_map(pctldev, map, reserved_maps, num_maps,
+					reserve);
+	if (ret < 0)
+		goto free_cfgs;
+
+	of_property_for_each_u32(np, "pinmux", prop, p, pinmux_group) {
+		u32 pin_id, pin_func, daisy_id, daisy_val, daisy_valid;
+		const char *pin_name;
+
+		i = 0;
+		ncfg = IMX_SCMI_NUM_CFG;
+		pin_id = get_pin_no(pinmux_group);
+		pin_func = get_pin_func(pinmux_group);
+		daisy_id = get_pin_daisy_no(pinmux_group);
+		daisy_val = get_pin_daisy_val(pinmux_group);
+		cfg[i++] = pinconf_to_config_packed(IMX_SCMI_PIN_MUX, pin_func);
+		cfg[i++] = pinconf_to_config_packed(IMX_SCMI_PIN_CONFIG, pin_conf);
+
+		daisy_valid = get_pin_daisy_valid(pinmux_group);
+		if (daisy_valid) {
+			cfg[i++] = pinconf_to_config_packed(IMX_SCMI_PIN_DAISY_ID,
+							    daisy_id);
+			cfg[i++] = pinconf_to_config_packed(IMX_SCMI_PIN_DAISY_CFG,
+							    daisy_val);
+		} else {
+			ncfg -= 2;
+		}
+
+		pin_name = pin_get_name(pctldev, pin_id);
+
+		dev_dbg(dev, "pin: %s, pin_conf: 0x%x, daisy_id: %u, daisy_val: 0x%x\n",
+			pin_name, pin_conf, daisy_id, daisy_val);
+
+		ret = pinctrl_utils_add_map_configs(pctldev, map, reserved_maps,
+						    num_maps, pin_name,
+						    cfg, ncfg,
+						    PIN_MAP_TYPE_CONFIGS_PIN);
+		if (ret < 0)
+			goto free_cfgs;
+	};
+
+
+free_cfgs:
+	kfree(cfgs);
+	return ret;
+}
+
+static int pinctrl_scmi_imx_dt_node_to_map(struct pinctrl_dev *pctldev,
+					   struct device_node *np_config,
+					   struct pinctrl_map **map,
+					   unsigned int *num_maps)
+
+{
+	unsigned int reserved_maps;
+	struct device_node *np;
+	int ret = 0;
+
+	reserved_maps = 0;
+	*map = NULL;
+	*num_maps = 0;
+
+	for_each_available_child_of_node(np_config, np) {
+		ret = pinctrl_scmi_imx_dt_group_node_to_map(pctldev, np, map,
+							    &reserved_maps,
+							    num_maps,
+							    np_config->name);
+		if (ret < 0) {
+			of_node_put(np);
+			break;
+		}
+	}
+
+	if (ret)
+		pinctrl_utils_free_map(pctldev, *map, *num_maps);
+
+	return ret;
+}
+
+static const struct pinctrl_ops pinctrl_scmi_imx_pinctrl_ops = {
+	.get_groups_count = pinctrl_generic_get_group_count,
+	.get_group_name = pinctrl_generic_get_group_name,
+	.get_group_pins = pinctrl_generic_get_group_pins,
+#ifdef CONFIG_OF
+	.dt_node_to_map = pinctrl_scmi_imx_dt_node_to_map,
+	.dt_free_map = pinconf_generic_dt_free_map,
+#endif
+};
+
+static int pinctrl_scmi_imx_func_set_mux(struct pinctrl_dev *pctldev,
+					 unsigned int selector, unsigned int group)
+{
+	/*
+	 * For i.MX SCMI PINCTRL , postpone the mux setting
+	 * until config is set as they can be set together
+	 * in one IPC call
+	 */
+	return 0;
+}
+
+static const struct pinmux_ops pinctrl_scmi_imx_pinmux_ops = {
+	.get_functions_count = pinmux_generic_get_function_count,
+	.get_function_name = pinmux_generic_get_function_name,
+	.get_function_groups = pinmux_generic_get_function_groups,
+	.set_mux = pinctrl_scmi_imx_func_set_mux,
+};
+
+static int pinctrl_scmi_imx_pinconf_get(struct pinctrl_dev *pctldev,
+					unsigned int pin, unsigned long *config)
+{
+	int ret;
+	struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
+	enum pin_config_param config_type;
+	u32 mask, val, shift;
+	u32 config_value;
+
+	if (!config)
+		return -EINVAL;
+
+	config_type = pinconf_to_config_param(*config);
+
+	ret = pinctrl_scmi_imx_map_pinconf_type(config_type, &mask, &shift);
+	if (ret)
+		return ret;
+
+	ret = pmx->ops->settings_get_one(pmx->ph, pin, PIN_TYPE,
+					 IMX_SCMI_PIN_CONFIG, &val);
+	/* Convert SCMI error code to PINCTRL expected error code */
+	if (ret == -EOPNOTSUPP)
+		return -ENOTSUPP;
+	if (ret)
+		return ret;
+
+	config_value = (val & mask) >> shift;
+	*config = pinconf_to_config_packed(config_type, config_value);
+
+	dev_dbg(pmx->dev, "pin:%s, conf:0x%x, type: %d, val: %u",
+		pin_get_name(pctldev, pin), val, config_type, config_value);
+
+	return 0;
+}
+
+static int pinctrl_scmi_imx_pinconf_set(struct pinctrl_dev *pctldev,
+					unsigned int pin,
+					unsigned long *configs,
+					unsigned int num_configs)
+{
+	struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
+	enum scmi_pinctrl_conf_type config_type[SCMI_NUM_CONFIGS];
+	u32 config_value[SCMI_NUM_CONFIGS];
+	enum scmi_pinctrl_conf_type *p_config_type = config_type;
+	u32 *p_config_value = config_value;
+	int ret;
+	int i;
+
+	if (!configs || !num_configs)
+		return -EINVAL;
+
+	if (num_configs > SCMI_NUM_CONFIGS) {
+		dev_err(pmx->dev, "num_configs(%d) too large\n", num_configs);
+		return -EINVAL;
+	}
+
+	for (i = 0; i < num_configs; i++) {
+		/* cast to avoid build warning */
+		p_config_type[i] =
+			(enum scmi_pinctrl_conf_type)pinconf_to_config_param(configs[i]);
+		p_config_value[i] = pinconf_to_config_argument(configs[i]);
+
+		dev_err(pmx->dev, "pin: %u, type: %u, val: 0x%x\n",
+			pin, p_config_type[i], p_config_value[i]);
+	}
+
+	ret = pmx->ops->settings_conf(pmx->ph, pin, PIN_TYPE, num_configs,
+				      p_config_type,  p_config_value);
+	if (ret)
+		dev_err(pmx->dev, "Error set config %d\n", ret);
+
+	return ret;
+}
+
+static const struct pinconf_ops pinctrl_scmi_imx_pinconf_ops = {
+	.is_generic = true,
+	.pin_config_get = pinctrl_scmi_imx_pinconf_get,
+	.pin_config_set = pinctrl_scmi_imx_pinconf_set,
+	.pin_config_config_dbg_show = pinconf_generic_dump_config,
+};
+
+static int scmi_pinctrl_imx_parse_groups(struct device_node *np,
+					 struct imx_pin_group *grp,
+					 struct scmi_pinctrl_imx_info *info)
+{
+	const __be32 *p;
+	struct device *dev;
+	struct property *prop;
+	unsigned int *pins;
+	int i, npins;
+	u32 pinmux;
+
+	dev = info->dev;
+
+	dev_dbg(dev, "group: %pOFn\n", np);
+
+	/* Initialise group */
+	grp->data.name = np->name;
+
+	npins = of_property_count_elems_of_size(np, "pinmux", sizeof(u32));
+	if (npins < 0) {
+		dev_err(dev, "Failed to read 'pinmux' property in node %s.\n",
+			grp->data.name);
+		return -EINVAL;
+	}
+	if (!npins) {
+		dev_err(dev, "The group %s has no pins.\n", grp->data.name);
+		return -EINVAL;
+	}
+
+	grp->data.npins = npins;
+
+	pins = devm_kcalloc(info->dev, npins, sizeof(*pins), GFP_KERNEL);
+	if (!pins)
+		return -ENOMEM;
+
+	i = 0;
+
+	of_property_for_each_u32(np, "pinmux", prop, p, pinmux) {
+		pins[i] = get_pin_no(pinmux);
+		dev_dbg(info->dev, "pin reg: 0x%x", pins[i] * 4);
+		i++;
+	}
+
+	grp->data.pins = pins;
+
+	return 0;
+}
+
+static int scmi_pinctrl_imx_parse_functions(struct device_node *np,
+					    struct scmi_pinctrl *pmx,
+					    u32 index)
+{
+	struct device_node *child;
+	struct pinfunction *func;
+	struct imx_pin_group *grp;
+	const char **groups;
+	struct scmi_pinctrl_imx_info *info = pmx->priv;
+	u32 i = 0;
+	int ret = 0;
+
+	dev_dbg(info->dev, "parse function(%u): %pOFn\n", index, np);
+
+	func = &info->functions[index];
+
+	/* Initialise function */
+	func->name = np->name;
+	func->ngroups = of_get_child_count(np);
+	if (func->ngroups == 0) {
+		dev_err(info->dev, "no groups defined in %pOF\n", np);
+		return -EINVAL;
+	}
+
+	groups = devm_kcalloc(info->dev, func->ngroups, sizeof(*func->groups),
+			      GFP_KERNEL);
+	if (!groups)
+		return -ENOMEM;
+
+	for_each_child_of_node(np, child) {
+		groups[i] = child->name;
+		grp = &info->groups[info->grp_index++];
+		ret = scmi_pinctrl_imx_parse_groups(child, grp, info);
+		if (ret) {
+			of_node_put(child);
+			return ret;
+		}
+		i++;
+	}
+
+	func->groups = groups;
+
+	return 0;
+}
+
+static int scmi_pinctrl_imx_probe_dt(struct scmi_device *sdev,
+				     struct scmi_pinctrl *pmx)
+{
+	int i, ret, nfuncs;
+	struct device_node *child;
+	struct scmi_pinctrl_imx_info *info = pmx->priv;
+	struct device_node *np = sdev->dev.of_node;
+
+	info->dev = &sdev->dev;
+
+	nfuncs = of_get_child_count(np);
+	if (nfuncs <= 0) {
+		dev_err(&sdev->dev, "no functions defined\n");
+		return -EINVAL;
+	}
+
+	info->nfunctions = nfuncs;
+	info->functions = devm_kcalloc(&sdev->dev, nfuncs,
+				       sizeof(*info->functions), GFP_KERNEL);
+	if (!info->functions)
+		return -ENOMEM;
+
+	info->ngroups = 0;
+	for_each_child_of_node(np, child)
+		info->ngroups += of_get_child_count(child);
+
+	info->groups = devm_kcalloc(&sdev->dev, info->ngroups,
+				    sizeof(*info->groups), GFP_KERNEL);
+	if (!info->groups)
+		return -ENOMEM;
+
+	i = 0;
+	for_each_child_of_node(np, child) {
+		ret = scmi_pinctrl_imx_parse_functions(child, pmx, i++);
+		if (ret) {
+			of_node_put(child);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+static const struct of_device_id scmi_pinctrl_imx_allowlist[] = {
+	{ .compatible = "fsl,imx95", },
+	{ }
+};
+
+static int scmi_pinctrl_imx_probe(struct scmi_device *sdev)
+{
+	int ret;
+	struct device *dev = &sdev->dev;
+	struct scmi_pinctrl *pmx;
+	const struct scmi_handle *handle;
+	struct scmi_protocol_handle *ph;
+	struct device_node *np __free(device_node) = of_find_node_by_path("/");
+	const struct scmi_pinctrl_proto_ops *pinctrl_ops;
+
+	if (!sdev->handle)
+		return -EINVAL;
+
+	if (!of_match_node(scmi_pinctrl_imx_allowlist, np))
+		return -ENODEV;
+
+	handle = sdev->handle;
+
+	pinctrl_ops = handle->devm_protocol_get(sdev, SCMI_PROTOCOL_PINCTRL, &ph);
+	if (IS_ERR(pinctrl_ops))
+		return PTR_ERR(pinctrl_ops);
+
+	pmx = devm_kzalloc(dev, sizeof(*pmx), GFP_KERNEL);
+	if (!pmx)
+		return -ENOMEM;
+
+	pmx->priv = devm_kzalloc(dev, sizeof(struct scmi_pinctrl_imx_info),
+				 GFP_KERNEL);
+	if (!pmx->priv)
+		return -ENOMEM;
+
+	pmx->ph = ph;
+	pmx->ops = pinctrl_ops;
+
+	pmx->dev = dev;
+	pmx->pctl_desc.name = DRV_NAME;
+	pmx->pctl_desc.owner = THIS_MODULE;
+	pmx->pctl_desc.pctlops = &pinctrl_scmi_imx_pinctrl_ops;
+	pmx->pctl_desc.pmxops = &pinctrl_scmi_imx_pinmux_ops;
+	pmx->pctl_desc.confops = &pinctrl_scmi_imx_pinconf_ops;
+
+	ret = pinctrl_scmi_get_pins(pmx, &pmx->pctl_desc);
+	if (ret)
+		return ret;
+
+	ret = scmi_pinctrl_imx_probe_dt(sdev, pmx);
+	if (ret)
+		return ret;
+
+	ret = devm_pinctrl_register_and_init(dev, &pmx->pctl_desc, pmx,
+					     &pmx->pctldev);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to register pinctrl\n");
+
+	return pinctrl_enable(pmx->pctldev);
+}
+
+static const struct scmi_device_id scmi_id_table[] = {
+	{ SCMI_PROTOCOL_PINCTRL, "pinctrl-imx" },
+	{ }
+};
+MODULE_DEVICE_TABLE(scmi, scmi_id_table);
+
+static struct scmi_driver scmi_pinctrl_imx_driver = {
+	.name = DRV_NAME,
+	.probe = scmi_pinctrl_imx_probe,
+	.id_table = scmi_id_table,
+};
+module_scmi_driver(scmi_pinctrl_imx_driver);
+
+MODULE_AUTHOR("Peng Fan <peng.fan@nxp.com>");
+MODULE_DESCRIPTION("i.MX SCMI pin controller driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/pinctrl/pinctrl-scmi.h b/drivers/pinctrl/pinctrl-scmi.h
index ae9e0be7c89e..c9a299241d7f 100644
--- a/drivers/pinctrl/pinctrl-scmi.h
+++ b/drivers/pinctrl/pinctrl-scmi.h
@@ -22,6 +22,7 @@ struct scmi_pinctrl {
 	struct pinctrl_pin_desc *pins;
 	unsigned int nr_pins;
 	const struct scmi_pinctrl_proto_ops *ops;
+	void *priv;
 };
 
 int pinctrl_scmi_get_pins(struct scmi_pinctrl *pmx, struct pinctrl_desc *desc);

-- 
2.37.1


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

* [PATCH v3 6/6] pinctrl: imx: support SCMI pinctrl protocol for i.MX95
@ 2024-04-28  5:07   ` Peng Fan (OSS)
  0 siblings, 0 replies; 22+ messages in thread
From: Peng Fan (OSS) @ 2024-04-28  5:07 UTC (permalink / raw)
  To: Sudeep Holla, Cristian Marussi, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Linus Walleij, Dong Aisheng, Jacky Bai
  Cc: linux-arm-kernel, devicetree, linux-kernel, imx, linux-gpio, Peng Fan

From: Peng Fan <peng.fan@nxp.com>

The generic pinctrl-scmi.c driver could not be used for i.MX95 because
i.MX95 SCMI firmware not supports functions, groups or generic
'Pin Configuration Type and Enumerations' listed in SCMI Specification.

i.MX95 System Control Management Interface(SCMI) firmware only supports
below pin configuration types which are OEM specific types:
    192: PIN MUX
    193: PIN CONF
    194: DAISY ID
    195: DAISY VAL

To support Linux generic pinctrl properties(pinmux, bias-pull-[up,
down], and etc), need extract the value from the property and map
them to the format that i.MX95 SCMI pinctrl protocol understands,
so add this driver.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/pinctrl/freescale/Kconfig            |   7 +
 drivers/pinctrl/freescale/Makefile           |   1 +
 drivers/pinctrl/freescale/pinctrl-imx-scmi.c | 574 +++++++++++++++++++++++++++
 drivers/pinctrl/pinctrl-scmi.h               |   1 +
 4 files changed, 583 insertions(+)

diff --git a/drivers/pinctrl/freescale/Kconfig b/drivers/pinctrl/freescale/Kconfig
index 27bdc548f3a7..bc23d9f7b5bb 100644
--- a/drivers/pinctrl/freescale/Kconfig
+++ b/drivers/pinctrl/freescale/Kconfig
@@ -7,6 +7,13 @@ config PINCTRL_IMX
 	select PINCONF
 	select REGMAP
 
+config PINCTRL_IMX_SCMI
+	tristate "i.MX95 pinctrl driver using SCMI protocol interface"
+	depends on PINCTRL_SCMI
+	help
+	  i.MX95 SCMI firmware provides pinctrl protocol. This driver
+	  utilizes the SCMI interface to do pinctrl configuration.
+
 config PINCTRL_IMX_SCU
 	tristate
 	depends on IMX_SCU
diff --git a/drivers/pinctrl/freescale/Makefile b/drivers/pinctrl/freescale/Makefile
index 647dff060477..e79b4b06e71b 100644
--- a/drivers/pinctrl/freescale/Makefile
+++ b/drivers/pinctrl/freescale/Makefile
@@ -2,6 +2,7 @@
 # Freescale pin control drivers
 obj-$(CONFIG_PINCTRL_IMX)	+= pinctrl-imx.o
 obj-$(CONFIG_PINCTRL_IMX_SCU)	+= pinctrl-scu.o
+obj-$(CONFIG_PINCTRL_IMX_SCMI)	+= pinctrl-imx-scmi.o
 obj-$(CONFIG_PINCTRL_IMX1_CORE)	+= pinctrl-imx1-core.o
 obj-$(CONFIG_PINCTRL_IMX1)	+= pinctrl-imx1.o
 obj-$(CONFIG_PINCTRL_IMX27)	+= pinctrl-imx27.o
diff --git a/drivers/pinctrl/freescale/pinctrl-imx-scmi.c b/drivers/pinctrl/freescale/pinctrl-imx-scmi.c
new file mode 100644
index 000000000000..13c65ba51269
--- /dev/null
+++ b/drivers/pinctrl/freescale/pinctrl-imx-scmi.c
@@ -0,0 +1,574 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * System Control and Power Interface (SCMI) Protocol based i.MX pinctrl driver
+ *
+ * Copyright 2024 NXP
+ */
+
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/of.h>
+#include <linux/scmi_protocol.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+
+#include <linux/pinctrl/machine.h>
+#include <linux/pinctrl/pinconf.h>
+#include <linux/pinctrl/pinconf-generic.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinmux.h>
+
+#include "../pinctrl-scmi.h"
+#include "../pinctrl-utils.h"
+#include "../core.h"
+#include "../pinconf.h"
+#include "../pinmux.h"
+
+#define DRV_NAME "scmi-pinctrl-imx"
+
+#define SCMI_NUM_CONFIGS	4
+
+struct imx_pin_group {
+	struct pingroup data;
+};
+
+struct scmi_pinctrl_imx_info {
+	struct device *dev;
+	struct imx_pin_group *groups;
+	unsigned int ngroups;
+	struct pinfunction *functions;
+	unsigned int nfunctions;
+	unsigned int grp_index;
+};
+
+/* SCMI pin control types, aligned with SCMI firmware */
+#define IMX_SCMI_NUM_CFG	4
+#define IMX_SCMI_PIN_MUX	192
+#define IMX_SCMI_PIN_CONFIG	193
+#define IMX_SCMI_PIN_DAISY_ID	194
+#define IMX_SCMI_PIN_DAISY_CFG	195
+
+/*
+ * pinmux format:
+ * pin[31:21]|mux[20:16]|daisy_value[15:12]|daisy_valid[11:11]|daisy_id[10:0]
+ */
+#define IMX_PIN_ID_MASK		GENMASK(31, 21)
+#define IMX_PIN_MUX_MASK	GENMASK(20, 16)
+#define IMX_PIN_DAISY_VAL_MASK	GENMASK(15, 12)
+#define IMX_PIN_DAISY_VALID	BIT(11)
+#define IMX_PIN_DAISY_ID_MASK	GENMASK(10, 0)
+
+static inline u32 get_pin_no(u32 pinmux)
+{
+	return FIELD_GET(IMX_PIN_ID_MASK, pinmux);
+}
+
+static inline u32 get_pin_func(u32 pinmux)
+{
+	return FIELD_GET(IMX_PIN_MUX_MASK, pinmux);
+}
+
+static inline u32 get_pin_daisy_valid(u32 pinmux)
+{
+	return FIELD_GET(IMX_PIN_DAISY_VALID, pinmux);
+}
+
+static inline u32 get_pin_daisy_val(u32 pinmux)
+{
+	return FIELD_GET(IMX_PIN_DAISY_VAL_MASK, pinmux);
+}
+
+static inline u32 get_pin_daisy_no(u32 pinmux)
+{
+	return FIELD_GET(IMX_PIN_DAISY_ID_MASK, pinmux);
+}
+
+static int pinctrl_scmi_imx_map_pinconf_type(enum pin_config_param param,
+					     u32 *mask, u32 *shift)
+{
+	u32 arg = param;
+
+	switch (arg) {
+	case PIN_CONFIG_INPUT_SCHMITT_ENABLE:
+		*mask = BIT(12);
+		*shift = 12;
+		break;
+	case PIN_CONFIG_DRIVE_OPEN_DRAIN:
+		*mask = BIT(11);
+		*shift = 11;
+		break;
+	case PIN_CONFIG_BIAS_PULL_DOWN:
+		*mask = BIT(10);
+		*shift = 10;
+		break;
+	case PIN_CONFIG_BIAS_PULL_UP:
+		*mask = BIT(9);
+		*shift = 9;
+		break;
+	case PIN_CONFIG_SLEW_RATE:
+		*mask = GENMASK(8, 7);
+		*shift = 7;
+		break;
+	case PIN_CONFIG_DRIVE_STRENGTH:
+		*mask = GENMASK(6, 1);
+		*shift = 1;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int pinctrl_scmi_imx_dt_group_node_to_map(struct pinctrl_dev *pctldev,
+						 struct device_node *np,
+						 struct pinctrl_map **map,
+						 unsigned int *reserved_maps,
+						 unsigned int *num_maps,
+						 const char *func_name)
+{
+	struct device *dev = pctldev->dev;
+	unsigned long *cfgs = NULL;
+	unsigned int n_cfgs, reserve = 1;
+	int i, n_pins, ret;
+	u32 ncfg, val, mask, shift, pin_conf, pinmux_group;
+	unsigned long cfg[IMX_SCMI_NUM_CFG];
+	enum pin_config_param param;
+	struct property *prop;
+	const __be32 *p;
+
+	n_pins = of_property_count_u32_elems(np, "pinmux");
+	if (n_pins < 0) {
+		dev_warn(dev, "Can't find 'pinmux' property in node %pOFn\n", np);
+		return -EINVAL;
+	} else if (!n_pins) {
+		return -EINVAL;
+	}
+
+	ret = pinconf_generic_parse_dt_config(np, pctldev, &cfgs, &n_cfgs);
+	if (ret) {
+		dev_err(dev, "%pOF: could not parse node property\n", np);
+		return ret;
+	}
+
+	pin_conf = 0;
+	for (i = 0; i < n_cfgs; i++) {
+		param = pinconf_to_config_param(cfgs[i]);
+		ret = pinctrl_scmi_imx_map_pinconf_type(param, &mask, &shift);
+		if (ret) {
+			dev_err(dev, "Error map pinconf_type %d\n", ret);
+			return ret;
+		}
+
+		val = pinconf_to_config_argument(cfgs[i]);
+
+		pin_conf |= (val << shift) & mask;
+
+	}
+
+	reserve = n_pins * (1 + n_cfgs);
+
+	ret = pinctrl_utils_reserve_map(pctldev, map, reserved_maps, num_maps,
+					reserve);
+	if (ret < 0)
+		goto free_cfgs;
+
+	of_property_for_each_u32(np, "pinmux", prop, p, pinmux_group) {
+		u32 pin_id, pin_func, daisy_id, daisy_val, daisy_valid;
+		const char *pin_name;
+
+		i = 0;
+		ncfg = IMX_SCMI_NUM_CFG;
+		pin_id = get_pin_no(pinmux_group);
+		pin_func = get_pin_func(pinmux_group);
+		daisy_id = get_pin_daisy_no(pinmux_group);
+		daisy_val = get_pin_daisy_val(pinmux_group);
+		cfg[i++] = pinconf_to_config_packed(IMX_SCMI_PIN_MUX, pin_func);
+		cfg[i++] = pinconf_to_config_packed(IMX_SCMI_PIN_CONFIG, pin_conf);
+
+		daisy_valid = get_pin_daisy_valid(pinmux_group);
+		if (daisy_valid) {
+			cfg[i++] = pinconf_to_config_packed(IMX_SCMI_PIN_DAISY_ID,
+							    daisy_id);
+			cfg[i++] = pinconf_to_config_packed(IMX_SCMI_PIN_DAISY_CFG,
+							    daisy_val);
+		} else {
+			ncfg -= 2;
+		}
+
+		pin_name = pin_get_name(pctldev, pin_id);
+
+		dev_dbg(dev, "pin: %s, pin_conf: 0x%x, daisy_id: %u, daisy_val: 0x%x\n",
+			pin_name, pin_conf, daisy_id, daisy_val);
+
+		ret = pinctrl_utils_add_map_configs(pctldev, map, reserved_maps,
+						    num_maps, pin_name,
+						    cfg, ncfg,
+						    PIN_MAP_TYPE_CONFIGS_PIN);
+		if (ret < 0)
+			goto free_cfgs;
+	};
+
+
+free_cfgs:
+	kfree(cfgs);
+	return ret;
+}
+
+static int pinctrl_scmi_imx_dt_node_to_map(struct pinctrl_dev *pctldev,
+					   struct device_node *np_config,
+					   struct pinctrl_map **map,
+					   unsigned int *num_maps)
+
+{
+	unsigned int reserved_maps;
+	struct device_node *np;
+	int ret = 0;
+
+	reserved_maps = 0;
+	*map = NULL;
+	*num_maps = 0;
+
+	for_each_available_child_of_node(np_config, np) {
+		ret = pinctrl_scmi_imx_dt_group_node_to_map(pctldev, np, map,
+							    &reserved_maps,
+							    num_maps,
+							    np_config->name);
+		if (ret < 0) {
+			of_node_put(np);
+			break;
+		}
+	}
+
+	if (ret)
+		pinctrl_utils_free_map(pctldev, *map, *num_maps);
+
+	return ret;
+}
+
+static const struct pinctrl_ops pinctrl_scmi_imx_pinctrl_ops = {
+	.get_groups_count = pinctrl_generic_get_group_count,
+	.get_group_name = pinctrl_generic_get_group_name,
+	.get_group_pins = pinctrl_generic_get_group_pins,
+#ifdef CONFIG_OF
+	.dt_node_to_map = pinctrl_scmi_imx_dt_node_to_map,
+	.dt_free_map = pinconf_generic_dt_free_map,
+#endif
+};
+
+static int pinctrl_scmi_imx_func_set_mux(struct pinctrl_dev *pctldev,
+					 unsigned int selector, unsigned int group)
+{
+	/*
+	 * For i.MX SCMI PINCTRL , postpone the mux setting
+	 * until config is set as they can be set together
+	 * in one IPC call
+	 */
+	return 0;
+}
+
+static const struct pinmux_ops pinctrl_scmi_imx_pinmux_ops = {
+	.get_functions_count = pinmux_generic_get_function_count,
+	.get_function_name = pinmux_generic_get_function_name,
+	.get_function_groups = pinmux_generic_get_function_groups,
+	.set_mux = pinctrl_scmi_imx_func_set_mux,
+};
+
+static int pinctrl_scmi_imx_pinconf_get(struct pinctrl_dev *pctldev,
+					unsigned int pin, unsigned long *config)
+{
+	int ret;
+	struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
+	enum pin_config_param config_type;
+	u32 mask, val, shift;
+	u32 config_value;
+
+	if (!config)
+		return -EINVAL;
+
+	config_type = pinconf_to_config_param(*config);
+
+	ret = pinctrl_scmi_imx_map_pinconf_type(config_type, &mask, &shift);
+	if (ret)
+		return ret;
+
+	ret = pmx->ops->settings_get_one(pmx->ph, pin, PIN_TYPE,
+					 IMX_SCMI_PIN_CONFIG, &val);
+	/* Convert SCMI error code to PINCTRL expected error code */
+	if (ret == -EOPNOTSUPP)
+		return -ENOTSUPP;
+	if (ret)
+		return ret;
+
+	config_value = (val & mask) >> shift;
+	*config = pinconf_to_config_packed(config_type, config_value);
+
+	dev_dbg(pmx->dev, "pin:%s, conf:0x%x, type: %d, val: %u",
+		pin_get_name(pctldev, pin), val, config_type, config_value);
+
+	return 0;
+}
+
+static int pinctrl_scmi_imx_pinconf_set(struct pinctrl_dev *pctldev,
+					unsigned int pin,
+					unsigned long *configs,
+					unsigned int num_configs)
+{
+	struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
+	enum scmi_pinctrl_conf_type config_type[SCMI_NUM_CONFIGS];
+	u32 config_value[SCMI_NUM_CONFIGS];
+	enum scmi_pinctrl_conf_type *p_config_type = config_type;
+	u32 *p_config_value = config_value;
+	int ret;
+	int i;
+
+	if (!configs || !num_configs)
+		return -EINVAL;
+
+	if (num_configs > SCMI_NUM_CONFIGS) {
+		dev_err(pmx->dev, "num_configs(%d) too large\n", num_configs);
+		return -EINVAL;
+	}
+
+	for (i = 0; i < num_configs; i++) {
+		/* cast to avoid build warning */
+		p_config_type[i] =
+			(enum scmi_pinctrl_conf_type)pinconf_to_config_param(configs[i]);
+		p_config_value[i] = pinconf_to_config_argument(configs[i]);
+
+		dev_err(pmx->dev, "pin: %u, type: %u, val: 0x%x\n",
+			pin, p_config_type[i], p_config_value[i]);
+	}
+
+	ret = pmx->ops->settings_conf(pmx->ph, pin, PIN_TYPE, num_configs,
+				      p_config_type,  p_config_value);
+	if (ret)
+		dev_err(pmx->dev, "Error set config %d\n", ret);
+
+	return ret;
+}
+
+static const struct pinconf_ops pinctrl_scmi_imx_pinconf_ops = {
+	.is_generic = true,
+	.pin_config_get = pinctrl_scmi_imx_pinconf_get,
+	.pin_config_set = pinctrl_scmi_imx_pinconf_set,
+	.pin_config_config_dbg_show = pinconf_generic_dump_config,
+};
+
+static int scmi_pinctrl_imx_parse_groups(struct device_node *np,
+					 struct imx_pin_group *grp,
+					 struct scmi_pinctrl_imx_info *info)
+{
+	const __be32 *p;
+	struct device *dev;
+	struct property *prop;
+	unsigned int *pins;
+	int i, npins;
+	u32 pinmux;
+
+	dev = info->dev;
+
+	dev_dbg(dev, "group: %pOFn\n", np);
+
+	/* Initialise group */
+	grp->data.name = np->name;
+
+	npins = of_property_count_elems_of_size(np, "pinmux", sizeof(u32));
+	if (npins < 0) {
+		dev_err(dev, "Failed to read 'pinmux' property in node %s.\n",
+			grp->data.name);
+		return -EINVAL;
+	}
+	if (!npins) {
+		dev_err(dev, "The group %s has no pins.\n", grp->data.name);
+		return -EINVAL;
+	}
+
+	grp->data.npins = npins;
+
+	pins = devm_kcalloc(info->dev, npins, sizeof(*pins), GFP_KERNEL);
+	if (!pins)
+		return -ENOMEM;
+
+	i = 0;
+
+	of_property_for_each_u32(np, "pinmux", prop, p, pinmux) {
+		pins[i] = get_pin_no(pinmux);
+		dev_dbg(info->dev, "pin reg: 0x%x", pins[i] * 4);
+		i++;
+	}
+
+	grp->data.pins = pins;
+
+	return 0;
+}
+
+static int scmi_pinctrl_imx_parse_functions(struct device_node *np,
+					    struct scmi_pinctrl *pmx,
+					    u32 index)
+{
+	struct device_node *child;
+	struct pinfunction *func;
+	struct imx_pin_group *grp;
+	const char **groups;
+	struct scmi_pinctrl_imx_info *info = pmx->priv;
+	u32 i = 0;
+	int ret = 0;
+
+	dev_dbg(info->dev, "parse function(%u): %pOFn\n", index, np);
+
+	func = &info->functions[index];
+
+	/* Initialise function */
+	func->name = np->name;
+	func->ngroups = of_get_child_count(np);
+	if (func->ngroups == 0) {
+		dev_err(info->dev, "no groups defined in %pOF\n", np);
+		return -EINVAL;
+	}
+
+	groups = devm_kcalloc(info->dev, func->ngroups, sizeof(*func->groups),
+			      GFP_KERNEL);
+	if (!groups)
+		return -ENOMEM;
+
+	for_each_child_of_node(np, child) {
+		groups[i] = child->name;
+		grp = &info->groups[info->grp_index++];
+		ret = scmi_pinctrl_imx_parse_groups(child, grp, info);
+		if (ret) {
+			of_node_put(child);
+			return ret;
+		}
+		i++;
+	}
+
+	func->groups = groups;
+
+	return 0;
+}
+
+static int scmi_pinctrl_imx_probe_dt(struct scmi_device *sdev,
+				     struct scmi_pinctrl *pmx)
+{
+	int i, ret, nfuncs;
+	struct device_node *child;
+	struct scmi_pinctrl_imx_info *info = pmx->priv;
+	struct device_node *np = sdev->dev.of_node;
+
+	info->dev = &sdev->dev;
+
+	nfuncs = of_get_child_count(np);
+	if (nfuncs <= 0) {
+		dev_err(&sdev->dev, "no functions defined\n");
+		return -EINVAL;
+	}
+
+	info->nfunctions = nfuncs;
+	info->functions = devm_kcalloc(&sdev->dev, nfuncs,
+				       sizeof(*info->functions), GFP_KERNEL);
+	if (!info->functions)
+		return -ENOMEM;
+
+	info->ngroups = 0;
+	for_each_child_of_node(np, child)
+		info->ngroups += of_get_child_count(child);
+
+	info->groups = devm_kcalloc(&sdev->dev, info->ngroups,
+				    sizeof(*info->groups), GFP_KERNEL);
+	if (!info->groups)
+		return -ENOMEM;
+
+	i = 0;
+	for_each_child_of_node(np, child) {
+		ret = scmi_pinctrl_imx_parse_functions(child, pmx, i++);
+		if (ret) {
+			of_node_put(child);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+static const struct of_device_id scmi_pinctrl_imx_allowlist[] = {
+	{ .compatible = "fsl,imx95", },
+	{ }
+};
+
+static int scmi_pinctrl_imx_probe(struct scmi_device *sdev)
+{
+	int ret;
+	struct device *dev = &sdev->dev;
+	struct scmi_pinctrl *pmx;
+	const struct scmi_handle *handle;
+	struct scmi_protocol_handle *ph;
+	struct device_node *np __free(device_node) = of_find_node_by_path("/");
+	const struct scmi_pinctrl_proto_ops *pinctrl_ops;
+
+	if (!sdev->handle)
+		return -EINVAL;
+
+	if (!of_match_node(scmi_pinctrl_imx_allowlist, np))
+		return -ENODEV;
+
+	handle = sdev->handle;
+
+	pinctrl_ops = handle->devm_protocol_get(sdev, SCMI_PROTOCOL_PINCTRL, &ph);
+	if (IS_ERR(pinctrl_ops))
+		return PTR_ERR(pinctrl_ops);
+
+	pmx = devm_kzalloc(dev, sizeof(*pmx), GFP_KERNEL);
+	if (!pmx)
+		return -ENOMEM;
+
+	pmx->priv = devm_kzalloc(dev, sizeof(struct scmi_pinctrl_imx_info),
+				 GFP_KERNEL);
+	if (!pmx->priv)
+		return -ENOMEM;
+
+	pmx->ph = ph;
+	pmx->ops = pinctrl_ops;
+
+	pmx->dev = dev;
+	pmx->pctl_desc.name = DRV_NAME;
+	pmx->pctl_desc.owner = THIS_MODULE;
+	pmx->pctl_desc.pctlops = &pinctrl_scmi_imx_pinctrl_ops;
+	pmx->pctl_desc.pmxops = &pinctrl_scmi_imx_pinmux_ops;
+	pmx->pctl_desc.confops = &pinctrl_scmi_imx_pinconf_ops;
+
+	ret = pinctrl_scmi_get_pins(pmx, &pmx->pctl_desc);
+	if (ret)
+		return ret;
+
+	ret = scmi_pinctrl_imx_probe_dt(sdev, pmx);
+	if (ret)
+		return ret;
+
+	ret = devm_pinctrl_register_and_init(dev, &pmx->pctl_desc, pmx,
+					     &pmx->pctldev);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to register pinctrl\n");
+
+	return pinctrl_enable(pmx->pctldev);
+}
+
+static const struct scmi_device_id scmi_id_table[] = {
+	{ SCMI_PROTOCOL_PINCTRL, "pinctrl-imx" },
+	{ }
+};
+MODULE_DEVICE_TABLE(scmi, scmi_id_table);
+
+static struct scmi_driver scmi_pinctrl_imx_driver = {
+	.name = DRV_NAME,
+	.probe = scmi_pinctrl_imx_probe,
+	.id_table = scmi_id_table,
+};
+module_scmi_driver(scmi_pinctrl_imx_driver);
+
+MODULE_AUTHOR("Peng Fan <peng.fan@nxp.com>");
+MODULE_DESCRIPTION("i.MX SCMI pin controller driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/pinctrl/pinctrl-scmi.h b/drivers/pinctrl/pinctrl-scmi.h
index ae9e0be7c89e..c9a299241d7f 100644
--- a/drivers/pinctrl/pinctrl-scmi.h
+++ b/drivers/pinctrl/pinctrl-scmi.h
@@ -22,6 +22,7 @@ struct scmi_pinctrl {
 	struct pinctrl_pin_desc *pins;
 	unsigned int nr_pins;
 	const struct scmi_pinctrl_proto_ops *ops;
+	void *priv;
 };
 
 int pinctrl_scmi_get_pins(struct scmi_pinctrl *pmx, struct pinctrl_desc *desc);

-- 
2.37.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v3 6/6] pinctrl: imx: support SCMI pinctrl protocol for i.MX95
  2024-04-28  5:07   ` Peng Fan (OSS)
@ 2024-04-28 12:11     ` Dan Carpenter
  -1 siblings, 0 replies; 22+ messages in thread
From: Dan Carpenter @ 2024-04-28 12:11 UTC (permalink / raw)
  To: Peng Fan (OSS)
  Cc: Sudeep Holla, Cristian Marussi, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Linus Walleij, Dong Aisheng, Jacky Bai,
	linux-arm-kernel, devicetree, linux-kernel, imx, linux-gpio,
	Peng Fan

On Sun, Apr 28, 2024 at 01:07:52PM +0800, Peng Fan (OSS) wrote:
> +static int pinctrl_scmi_imx_dt_group_node_to_map(struct pinctrl_dev *pctldev,
> +						 struct device_node *np,
> +						 struct pinctrl_map **map,
> +						 unsigned int *reserved_maps,
> +						 unsigned int *num_maps,
> +						 const char *func_name)
> +{
> +	struct device *dev = pctldev->dev;
> +	unsigned long *cfgs = NULL;
> +	unsigned int n_cfgs, reserve = 1;
> +	int i, n_pins, ret;
> +	u32 ncfg, val, mask, shift, pin_conf, pinmux_group;
> +	unsigned long cfg[IMX_SCMI_NUM_CFG];
> +	enum pin_config_param param;
> +	struct property *prop;
> +	const __be32 *p;
> +
> +	n_pins = of_property_count_u32_elems(np, "pinmux");
> +	if (n_pins < 0) {
> +		dev_warn(dev, "Can't find 'pinmux' property in node %pOFn\n", np);
> +		return -EINVAL;

	return n_pins;

> +	} else if (!n_pins) {
> +		return -EINVAL;
> +	}
> +
> +	ret = pinconf_generic_parse_dt_config(np, pctldev, &cfgs, &n_cfgs);
> +	if (ret) {
> +		dev_err(dev, "%pOF: could not parse node property\n", np);
> +		return ret;
> +	}
> +
> +	pin_conf = 0;
> +	for (i = 0; i < n_cfgs; i++) {
> +		param = pinconf_to_config_param(cfgs[i]);
> +		ret = pinctrl_scmi_imx_map_pinconf_type(param, &mask, &shift);
> +		if (ret) {
> +			dev_err(dev, "Error map pinconf_type %d\n", ret);
> +			return ret;

This should be goto free_cfgs.

> +		}
> +
> +		val = pinconf_to_config_argument(cfgs[i]);
> +
> +		pin_conf |= (val << shift) & mask;
> +
> +	}
> +
> +	reserve = n_pins * (1 + n_cfgs);
> +
> +	ret = pinctrl_utils_reserve_map(pctldev, map, reserved_maps, num_maps,
> +					reserve);
> +	if (ret < 0)
> +		goto free_cfgs;
> +
> +	of_property_for_each_u32(np, "pinmux", prop, p, pinmux_group) {
> +		u32 pin_id, pin_func, daisy_id, daisy_val, daisy_valid;
> +		const char *pin_name;
> +
> +		i = 0;
> +		ncfg = IMX_SCMI_NUM_CFG;
> +		pin_id = get_pin_no(pinmux_group);
> +		pin_func = get_pin_func(pinmux_group);
> +		daisy_id = get_pin_daisy_no(pinmux_group);
> +		daisy_val = get_pin_daisy_val(pinmux_group);
> +		cfg[i++] = pinconf_to_config_packed(IMX_SCMI_PIN_MUX, pin_func);
> +		cfg[i++] = pinconf_to_config_packed(IMX_SCMI_PIN_CONFIG, pin_conf);
> +
> +		daisy_valid = get_pin_daisy_valid(pinmux_group);
> +		if (daisy_valid) {
> +			cfg[i++] = pinconf_to_config_packed(IMX_SCMI_PIN_DAISY_ID,
> +							    daisy_id);
> +			cfg[i++] = pinconf_to_config_packed(IMX_SCMI_PIN_DAISY_CFG,
> +							    daisy_val);
> +		} else {
> +			ncfg -= 2;
> +		}
> +
> +		pin_name = pin_get_name(pctldev, pin_id);
> +
> +		dev_dbg(dev, "pin: %s, pin_conf: 0x%x, daisy_id: %u, daisy_val: 0x%x\n",
> +			pin_name, pin_conf, daisy_id, daisy_val);
> +
> +		ret = pinctrl_utils_add_map_configs(pctldev, map, reserved_maps,
> +						    num_maps, pin_name,
> +						    cfg, ncfg,
> +						    PIN_MAP_TYPE_CONFIGS_PIN);
> +		if (ret < 0)
> +			goto free_cfgs;
> +	};
> +
> +
> +free_cfgs:
> +	kfree(cfgs);
> +	return ret;
> +}
> +
> +static int pinctrl_scmi_imx_dt_node_to_map(struct pinctrl_dev *pctldev,
> +					   struct device_node *np_config,
> +					   struct pinctrl_map **map,
> +					   unsigned int *num_maps)
> +
> +{
> +	unsigned int reserved_maps;
> +	struct device_node *np;
> +	int ret = 0;
> +
> +	reserved_maps = 0;
> +	*map = NULL;
> +	*num_maps = 0;
> +
> +	for_each_available_child_of_node(np_config, np) {

Btw, if you used the scoped version of these loops such as
for_each_available_child_of_node_scoped(), then

> +		ret = pinctrl_scmi_imx_dt_group_node_to_map(pctldev, np, map,
> +							    &reserved_maps,
> +							    num_maps,
> +							    np_config->name);
> +		if (ret < 0) {
> +			of_node_put(np);

You could get rid of the calls to of_node_put().  I would move the
call to pinctrl_utils_free_map() here.

		if (ret) {
			pinctrl_utils_free_map(pctldev, *map, *num_maps);
			return ret;
		}

> +			break;
> +		}
> +	}
> +
> +	if (ret)
> +		pinctrl_utils_free_map(pctldev, *map, *num_maps);
> +
> +	return ret;

	return 0;

> +}

regards,
dan carpenter


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

* Re: [PATCH v3 6/6] pinctrl: imx: support SCMI pinctrl protocol for i.MX95
@ 2024-04-28 12:11     ` Dan Carpenter
  0 siblings, 0 replies; 22+ messages in thread
From: Dan Carpenter @ 2024-04-28 12:11 UTC (permalink / raw)
  To: Peng Fan (OSS)
  Cc: Sudeep Holla, Cristian Marussi, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Linus Walleij, Dong Aisheng, Jacky Bai,
	linux-arm-kernel, devicetree, linux-kernel, imx, linux-gpio,
	Peng Fan

On Sun, Apr 28, 2024 at 01:07:52PM +0800, Peng Fan (OSS) wrote:
> +static int pinctrl_scmi_imx_dt_group_node_to_map(struct pinctrl_dev *pctldev,
> +						 struct device_node *np,
> +						 struct pinctrl_map **map,
> +						 unsigned int *reserved_maps,
> +						 unsigned int *num_maps,
> +						 const char *func_name)
> +{
> +	struct device *dev = pctldev->dev;
> +	unsigned long *cfgs = NULL;
> +	unsigned int n_cfgs, reserve = 1;
> +	int i, n_pins, ret;
> +	u32 ncfg, val, mask, shift, pin_conf, pinmux_group;
> +	unsigned long cfg[IMX_SCMI_NUM_CFG];
> +	enum pin_config_param param;
> +	struct property *prop;
> +	const __be32 *p;
> +
> +	n_pins = of_property_count_u32_elems(np, "pinmux");
> +	if (n_pins < 0) {
> +		dev_warn(dev, "Can't find 'pinmux' property in node %pOFn\n", np);
> +		return -EINVAL;

	return n_pins;

> +	} else if (!n_pins) {
> +		return -EINVAL;
> +	}
> +
> +	ret = pinconf_generic_parse_dt_config(np, pctldev, &cfgs, &n_cfgs);
> +	if (ret) {
> +		dev_err(dev, "%pOF: could not parse node property\n", np);
> +		return ret;
> +	}
> +
> +	pin_conf = 0;
> +	for (i = 0; i < n_cfgs; i++) {
> +		param = pinconf_to_config_param(cfgs[i]);
> +		ret = pinctrl_scmi_imx_map_pinconf_type(param, &mask, &shift);
> +		if (ret) {
> +			dev_err(dev, "Error map pinconf_type %d\n", ret);
> +			return ret;

This should be goto free_cfgs.

> +		}
> +
> +		val = pinconf_to_config_argument(cfgs[i]);
> +
> +		pin_conf |= (val << shift) & mask;
> +
> +	}
> +
> +	reserve = n_pins * (1 + n_cfgs);
> +
> +	ret = pinctrl_utils_reserve_map(pctldev, map, reserved_maps, num_maps,
> +					reserve);
> +	if (ret < 0)
> +		goto free_cfgs;
> +
> +	of_property_for_each_u32(np, "pinmux", prop, p, pinmux_group) {
> +		u32 pin_id, pin_func, daisy_id, daisy_val, daisy_valid;
> +		const char *pin_name;
> +
> +		i = 0;
> +		ncfg = IMX_SCMI_NUM_CFG;
> +		pin_id = get_pin_no(pinmux_group);
> +		pin_func = get_pin_func(pinmux_group);
> +		daisy_id = get_pin_daisy_no(pinmux_group);
> +		daisy_val = get_pin_daisy_val(pinmux_group);
> +		cfg[i++] = pinconf_to_config_packed(IMX_SCMI_PIN_MUX, pin_func);
> +		cfg[i++] = pinconf_to_config_packed(IMX_SCMI_PIN_CONFIG, pin_conf);
> +
> +		daisy_valid = get_pin_daisy_valid(pinmux_group);
> +		if (daisy_valid) {
> +			cfg[i++] = pinconf_to_config_packed(IMX_SCMI_PIN_DAISY_ID,
> +							    daisy_id);
> +			cfg[i++] = pinconf_to_config_packed(IMX_SCMI_PIN_DAISY_CFG,
> +							    daisy_val);
> +		} else {
> +			ncfg -= 2;
> +		}
> +
> +		pin_name = pin_get_name(pctldev, pin_id);
> +
> +		dev_dbg(dev, "pin: %s, pin_conf: 0x%x, daisy_id: %u, daisy_val: 0x%x\n",
> +			pin_name, pin_conf, daisy_id, daisy_val);
> +
> +		ret = pinctrl_utils_add_map_configs(pctldev, map, reserved_maps,
> +						    num_maps, pin_name,
> +						    cfg, ncfg,
> +						    PIN_MAP_TYPE_CONFIGS_PIN);
> +		if (ret < 0)
> +			goto free_cfgs;
> +	};
> +
> +
> +free_cfgs:
> +	kfree(cfgs);
> +	return ret;
> +}
> +
> +static int pinctrl_scmi_imx_dt_node_to_map(struct pinctrl_dev *pctldev,
> +					   struct device_node *np_config,
> +					   struct pinctrl_map **map,
> +					   unsigned int *num_maps)
> +
> +{
> +	unsigned int reserved_maps;
> +	struct device_node *np;
> +	int ret = 0;
> +
> +	reserved_maps = 0;
> +	*map = NULL;
> +	*num_maps = 0;
> +
> +	for_each_available_child_of_node(np_config, np) {

Btw, if you used the scoped version of these loops such as
for_each_available_child_of_node_scoped(), then

> +		ret = pinctrl_scmi_imx_dt_group_node_to_map(pctldev, np, map,
> +							    &reserved_maps,
> +							    num_maps,
> +							    np_config->name);
> +		if (ret < 0) {
> +			of_node_put(np);

You could get rid of the calls to of_node_put().  I would move the
call to pinctrl_utils_free_map() here.

		if (ret) {
			pinctrl_utils_free_map(pctldev, *map, *num_maps);
			return ret;
		}

> +			break;
> +		}
> +	}
> +
> +	if (ret)
> +		pinctrl_utils_free_map(pctldev, *map, *num_maps);
> +
> +	return ret;

	return 0;

> +}

regards,
dan carpenter


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* RE: [PATCH v3 6/6] pinctrl: imx: support SCMI pinctrl protocol for i.MX95
  2024-04-28 12:11     ` Dan Carpenter
@ 2024-04-28 12:21       ` Peng Fan
  -1 siblings, 0 replies; 22+ messages in thread
From: Peng Fan @ 2024-04-28 12:21 UTC (permalink / raw)
  To: Dan Carpenter, Peng Fan (OSS)
  Cc: Sudeep Holla, Cristian Marussi, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Linus Walleij, Aisheng Dong, Jacky Bai,
	linux-arm-kernel, devicetree, linux-kernel, imx, linux-gpio

Hi Dan,

> Subject: Re: [PATCH v3 6/6] pinctrl: imx: support SCMI pinctrl protocol for
> i.MX95
> 
> On Sun, Apr 28, 2024 at 01:07:52PM +0800, Peng Fan (OSS) wrote:
> > +static int pinctrl_scmi_imx_dt_group_node_to_map(struct pinctrl_dev
> *pctldev,
> > +						 struct device_node *np,
> > +						 struct pinctrl_map **map,
> > +						 unsigned int
> *reserved_maps,
> > +						 unsigned int *num_maps,
> > +						 const char *func_name)
> > +{
> > +	struct device *dev = pctldev->dev;
> > +	unsigned long *cfgs = NULL;
> > +	unsigned int n_cfgs, reserve = 1;
> > +	int i, n_pins, ret;
> > +	u32 ncfg, val, mask, shift, pin_conf, pinmux_group;
> > +	unsigned long cfg[IMX_SCMI_NUM_CFG];
> > +	enum pin_config_param param;
> > +	struct property *prop;
> > +	const __be32 *p;
> > +
> > +	n_pins = of_property_count_u32_elems(np, "pinmux");
> > +	if (n_pins < 0) {
> > +		dev_warn(dev, "Can't find 'pinmux' property in
> node %pOFn\n", np);
> > +		return -EINVAL;
> 
> 	return n_pins;

Yeah.  Fix in v4.

> 
> > +	} else if (!n_pins) {
> > +		return -EINVAL;
> > +	}
> > +
> > +	ret = pinconf_generic_parse_dt_config(np, pctldev, &cfgs, &n_cfgs);
> > +	if (ret) {
> > +		dev_err(dev, "%pOF: could not parse node property\n", np);
> > +		return ret;
> > +	}
> > +
> > +	pin_conf = 0;
> > +	for (i = 0; i < n_cfgs; i++) {
> > +		param = pinconf_to_config_param(cfgs[i]);
> > +		ret = pinctrl_scmi_imx_map_pinconf_type(param, &mask,
> &shift);
> > +		if (ret) {
> > +			dev_err(dev, "Error map pinconf_type %d\n", ret);
> > +			return ret;
> 
> This should be goto free_cfgs.

Good catch. Fix in v4.

> 
> > +		}
> > +
> > +		val = pinconf_to_config_argument(cfgs[i]);
> > +

...
> > +
> > +{
> > +	unsigned int reserved_maps;
> > +	struct device_node *np;
> > +	int ret = 0;
> > +
> > +	reserved_maps = 0;
> > +	*map = NULL;
> > +	*num_maps = 0;
> > +
> > +	for_each_available_child_of_node(np_config, np) {
> 
> Btw, if you used the scoped version of these loops such as
> for_each_available_child_of_node_scoped(), then

Yeah, it will simply code.

> 
> > +		ret = pinctrl_scmi_imx_dt_group_node_to_map(pctldev, np,
> map,
> > +
> &reserved_maps,
> > +							    num_maps,
> > +							    np_config-
> >name);
> > +		if (ret < 0) {
> > +			of_node_put(np);
> 
> You could get rid of the calls to of_node_put().  I would move the call to
> pinctrl_utils_free_map() here.

ok. Update in v4.

> 
> 		if (ret) {
> 			pinctrl_utils_free_map(pctldev, *map, *num_maps);
> 			return ret;
> 		}
> 
> > +			break;
> > +		}
> > +	}
> > +
> > +	if (ret)
> > +		pinctrl_utils_free_map(pctldev, *map, *num_maps);
> > +
> > +	return ret;
> 
> 	return 0;

Thanks for your quick response, I will wait for more reviews, then post v4.

Thanks,
Peng.

> 
> > +}
> 
> regards,
> dan carpenter


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

* RE: [PATCH v3 6/6] pinctrl: imx: support SCMI pinctrl protocol for i.MX95
@ 2024-04-28 12:21       ` Peng Fan
  0 siblings, 0 replies; 22+ messages in thread
From: Peng Fan @ 2024-04-28 12:21 UTC (permalink / raw)
  To: Dan Carpenter, Peng Fan (OSS)
  Cc: Sudeep Holla, Cristian Marussi, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Linus Walleij, Aisheng Dong, Jacky Bai,
	linux-arm-kernel, devicetree, linux-kernel, imx, linux-gpio

Hi Dan,

> Subject: Re: [PATCH v3 6/6] pinctrl: imx: support SCMI pinctrl protocol for
> i.MX95
> 
> On Sun, Apr 28, 2024 at 01:07:52PM +0800, Peng Fan (OSS) wrote:
> > +static int pinctrl_scmi_imx_dt_group_node_to_map(struct pinctrl_dev
> *pctldev,
> > +						 struct device_node *np,
> > +						 struct pinctrl_map **map,
> > +						 unsigned int
> *reserved_maps,
> > +						 unsigned int *num_maps,
> > +						 const char *func_name)
> > +{
> > +	struct device *dev = pctldev->dev;
> > +	unsigned long *cfgs = NULL;
> > +	unsigned int n_cfgs, reserve = 1;
> > +	int i, n_pins, ret;
> > +	u32 ncfg, val, mask, shift, pin_conf, pinmux_group;
> > +	unsigned long cfg[IMX_SCMI_NUM_CFG];
> > +	enum pin_config_param param;
> > +	struct property *prop;
> > +	const __be32 *p;
> > +
> > +	n_pins = of_property_count_u32_elems(np, "pinmux");
> > +	if (n_pins < 0) {
> > +		dev_warn(dev, "Can't find 'pinmux' property in
> node %pOFn\n", np);
> > +		return -EINVAL;
> 
> 	return n_pins;

Yeah.  Fix in v4.

> 
> > +	} else if (!n_pins) {
> > +		return -EINVAL;
> > +	}
> > +
> > +	ret = pinconf_generic_parse_dt_config(np, pctldev, &cfgs, &n_cfgs);
> > +	if (ret) {
> > +		dev_err(dev, "%pOF: could not parse node property\n", np);
> > +		return ret;
> > +	}
> > +
> > +	pin_conf = 0;
> > +	for (i = 0; i < n_cfgs; i++) {
> > +		param = pinconf_to_config_param(cfgs[i]);
> > +		ret = pinctrl_scmi_imx_map_pinconf_type(param, &mask,
> &shift);
> > +		if (ret) {
> > +			dev_err(dev, "Error map pinconf_type %d\n", ret);
> > +			return ret;
> 
> This should be goto free_cfgs.

Good catch. Fix in v4.

> 
> > +		}
> > +
> > +		val = pinconf_to_config_argument(cfgs[i]);
> > +

...
> > +
> > +{
> > +	unsigned int reserved_maps;
> > +	struct device_node *np;
> > +	int ret = 0;
> > +
> > +	reserved_maps = 0;
> > +	*map = NULL;
> > +	*num_maps = 0;
> > +
> > +	for_each_available_child_of_node(np_config, np) {
> 
> Btw, if you used the scoped version of these loops such as
> for_each_available_child_of_node_scoped(), then

Yeah, it will simply code.

> 
> > +		ret = pinctrl_scmi_imx_dt_group_node_to_map(pctldev, np,
> map,
> > +
> &reserved_maps,
> > +							    num_maps,
> > +							    np_config-
> >name);
> > +		if (ret < 0) {
> > +			of_node_put(np);
> 
> You could get rid of the calls to of_node_put().  I would move the call to
> pinctrl_utils_free_map() here.

ok. Update in v4.

> 
> 		if (ret) {
> 			pinctrl_utils_free_map(pctldev, *map, *num_maps);
> 			return ret;
> 		}
> 
> > +			break;
> > +		}
> > +	}
> > +
> > +	if (ret)
> > +		pinctrl_utils_free_map(pctldev, *map, *num_maps);
> > +
> > +	return ret;
> 
> 	return 0;

Thanks for your quick response, I will wait for more reviews, then post v4.

Thanks,
Peng.

> 
> > +}
> 
> regards,
> dan carpenter


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v3 4/6] pinctrl: scmi: export pinctrl_scmi_get_pins
  2024-04-28  5:07   ` Peng Fan (OSS)
@ 2024-05-01 12:36     ` Cristian Marussi
  -1 siblings, 0 replies; 22+ messages in thread
From: Cristian Marussi @ 2024-05-01 12:36 UTC (permalink / raw)
  To: Peng Fan (OSS)
  Cc: Sudeep Holla, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	Linus Walleij, Dong Aisheng, Jacky Bai, linux-arm-kernel,
	devicetree, linux-kernel, imx, linux-gpio, Peng Fan

On Sun, Apr 28, 2024 at 01:07:50PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> Add pinctrl-scmi.h to include the function prototype and 'struct
> scmi_pinctrl' to export pinctrl_scmi_get_pins, so other drivers
> could use it.
> 

Hi Peng,

so you wrote a new alternative SCMI driver using Pinctrl protocol@0x19
so that you can just parse you custom DT bindings and then use the SCMI
pinctrl_ops to set the OEM extensions to configure your platform...
...since your firmware cannot cope with the all SCMI stack footprint....

... you seemed to have solved the issue of having 2 Pinctrl drivers
coexisting under the Linux Pinctrl subsystem while attached to the same
protocol@19 node with patch 5/6 blocklist (if I get that right..)

I think this approach of a standalone SCMI alternative Pinctrl driver
that handles distinctly NXP OEM extensions and DT-parsing is certainly
more preferable than the original series you posted months ago where
custom NXP stuff were simply stuck on top of the Generic SCMI Pinctrl driver...

...what I still dont understand is why you exported data and structure
from pincttl-scmi.c to use it here; when NXP pinctrl is active the
standard Linux generic Pinctrl driver wont be alive, so not probed, so
no data can be shared, the only thing I can imagine is that you are
just trying to avoid duplicating a dozen lines from the logic of
scmi_pinctrl_get_pins() into your new NXP driver.

In this way, though, you are creating a dependency between 2 drivers,
that are not even allowed to cohexist at runtime really (due to the
blocklist trick).

Am I missing something ?

If not, I think it will be much better to just rewrite that few lines of
scmi_pincrtrl_pins_get trivial logic into your NXP driver and keep the 2
drivers fully distinct at all times.

Thanks,
Cristian


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

* Re: [PATCH v3 4/6] pinctrl: scmi: export pinctrl_scmi_get_pins
@ 2024-05-01 12:36     ` Cristian Marussi
  0 siblings, 0 replies; 22+ messages in thread
From: Cristian Marussi @ 2024-05-01 12:36 UTC (permalink / raw)
  To: Peng Fan (OSS)
  Cc: Sudeep Holla, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	Linus Walleij, Dong Aisheng, Jacky Bai, linux-arm-kernel,
	devicetree, linux-kernel, imx, linux-gpio, Peng Fan

On Sun, Apr 28, 2024 at 01:07:50PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> Add pinctrl-scmi.h to include the function prototype and 'struct
> scmi_pinctrl' to export pinctrl_scmi_get_pins, so other drivers
> could use it.
> 

Hi Peng,

so you wrote a new alternative SCMI driver using Pinctrl protocol@0x19
so that you can just parse you custom DT bindings and then use the SCMI
pinctrl_ops to set the OEM extensions to configure your platform...
...since your firmware cannot cope with the all SCMI stack footprint....

... you seemed to have solved the issue of having 2 Pinctrl drivers
coexisting under the Linux Pinctrl subsystem while attached to the same
protocol@19 node with patch 5/6 blocklist (if I get that right..)

I think this approach of a standalone SCMI alternative Pinctrl driver
that handles distinctly NXP OEM extensions and DT-parsing is certainly
more preferable than the original series you posted months ago where
custom NXP stuff were simply stuck on top of the Generic SCMI Pinctrl driver...

...what I still dont understand is why you exported data and structure
from pincttl-scmi.c to use it here; when NXP pinctrl is active the
standard Linux generic Pinctrl driver wont be alive, so not probed, so
no data can be shared, the only thing I can imagine is that you are
just trying to avoid duplicating a dozen lines from the logic of
scmi_pinctrl_get_pins() into your new NXP driver.

In this way, though, you are creating a dependency between 2 drivers,
that are not even allowed to cohexist at runtime really (due to the
blocklist trick).

Am I missing something ?

If not, I think it will be much better to just rewrite that few lines of
scmi_pincrtrl_pins_get trivial logic into your NXP driver and keep the 2
drivers fully distinct at all times.

Thanks,
Cristian


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* RE: [PATCH v3 4/6] pinctrl: scmi: export pinctrl_scmi_get_pins
  2024-05-01 12:36     ` Cristian Marussi
@ 2024-05-01 12:42       ` Peng Fan
  -1 siblings, 0 replies; 22+ messages in thread
From: Peng Fan @ 2024-05-01 12:42 UTC (permalink / raw)
  To: Cristian Marussi, Peng Fan (OSS)
  Cc: Sudeep Holla, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	Linus Walleij, Aisheng Dong, Jacky Bai, linux-arm-kernel,
	devicetree, linux-kernel, imx, linux-gpio

> Subject: Re: [PATCH v3 4/6] pinctrl: scmi: export pinctrl_scmi_get_pins
> 
> On Sun, Apr 28, 2024 at 01:07:50PM +0800, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > Add pinctrl-scmi.h to include the function prototype and 'struct
> > scmi_pinctrl' to export pinctrl_scmi_get_pins, so other drivers could
> > use it.
> >
> 
> Hi Peng,
> 
> so you wrote a new alternative SCMI driver using Pinctrl protocol@0x19 so
> that you can just parse you custom DT bindings and then use the SCMI
> pinctrl_ops to set the OEM extensions to configure your platform...
> ...since your firmware cannot cope with the all SCMI stack footprint....
> 
> ... you seemed to have solved the issue of having 2 Pinctrl drivers coexisting
> under the Linux Pinctrl subsystem while attached to the same
> protocol@19 node with patch 5/6 blocklist (if I get that right..)

Yes, right. With blocklist and allowlist, two drivers could coexist.

> 
> I think this approach of a standalone SCMI alternative Pinctrl driver that
> handles distinctly NXP OEM extensions and DT-parsing is certainly more
> preferable than the original series you posted months ago where custom NXP
> stuff were simply stuck on top of the Generic SCMI Pinctrl driver...
> 
> ...what I still dont understand is why you exported data and structure from
> pincttl-scmi.c to use it here; when NXP pinctrl is active the standard Linux
> generic Pinctrl driver wont be alive, so not probed, so no data can be shared,
> the only thing I can imagine is that you are just trying to avoid duplicating a
> dozen lines from the logic of
> scmi_pinctrl_get_pins() into your new NXP driver.

Yes, you are right, I just wanna avoid duplicating scmi_pinctrl_get_pins.

> 
> In this way, though, you are creating a dependency between 2 drivers, that
> are not even allowed to cohexist at runtime really (due to the blocklist trick).
> 
> Am I missing something ?

No, your understanding is correct.

> 
> If not, I think it will be much better to just rewrite that few lines of
> scmi_pincrtrl_pins_get trivial logic into your NXP driver and keep the 2
> drivers fully distinct at all times.

ok. I could write the pinctrl-scmi-imx.c local get pins logic, not using
pinctrl-scmi.c to decouple the two drivers.

Thanks,
Peng
> 
> Thanks,
> Cristian


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

* RE: [PATCH v3 4/6] pinctrl: scmi: export pinctrl_scmi_get_pins
@ 2024-05-01 12:42       ` Peng Fan
  0 siblings, 0 replies; 22+ messages in thread
From: Peng Fan @ 2024-05-01 12:42 UTC (permalink / raw)
  To: Cristian Marussi, Peng Fan (OSS)
  Cc: Sudeep Holla, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	Linus Walleij, Aisheng Dong, Jacky Bai, linux-arm-kernel,
	devicetree, linux-kernel, imx, linux-gpio

> Subject: Re: [PATCH v3 4/6] pinctrl: scmi: export pinctrl_scmi_get_pins
> 
> On Sun, Apr 28, 2024 at 01:07:50PM +0800, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > Add pinctrl-scmi.h to include the function prototype and 'struct
> > scmi_pinctrl' to export pinctrl_scmi_get_pins, so other drivers could
> > use it.
> >
> 
> Hi Peng,
> 
> so you wrote a new alternative SCMI driver using Pinctrl protocol@0x19 so
> that you can just parse you custom DT bindings and then use the SCMI
> pinctrl_ops to set the OEM extensions to configure your platform...
> ...since your firmware cannot cope with the all SCMI stack footprint....
> 
> ... you seemed to have solved the issue of having 2 Pinctrl drivers coexisting
> under the Linux Pinctrl subsystem while attached to the same
> protocol@19 node with patch 5/6 blocklist (if I get that right..)

Yes, right. With blocklist and allowlist, two drivers could coexist.

> 
> I think this approach of a standalone SCMI alternative Pinctrl driver that
> handles distinctly NXP OEM extensions and DT-parsing is certainly more
> preferable than the original series you posted months ago where custom NXP
> stuff were simply stuck on top of the Generic SCMI Pinctrl driver...
> 
> ...what I still dont understand is why you exported data and structure from
> pincttl-scmi.c to use it here; when NXP pinctrl is active the standard Linux
> generic Pinctrl driver wont be alive, so not probed, so no data can be shared,
> the only thing I can imagine is that you are just trying to avoid duplicating a
> dozen lines from the logic of
> scmi_pinctrl_get_pins() into your new NXP driver.

Yes, you are right, I just wanna avoid duplicating scmi_pinctrl_get_pins.

> 
> In this way, though, you are creating a dependency between 2 drivers, that
> are not even allowed to cohexist at runtime really (due to the blocklist trick).
> 
> Am I missing something ?

No, your understanding is correct.

> 
> If not, I think it will be much better to just rewrite that few lines of
> scmi_pincrtrl_pins_get trivial logic into your NXP driver and keep the 2
> drivers fully distinct at all times.

ok. I could write the pinctrl-scmi-imx.c local get pins logic, not using
pinctrl-scmi.c to decouple the two drivers.

Thanks,
Peng
> 
> Thanks,
> Cristian


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

end of thread, other threads:[~2024-05-01 12:42 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-28  5:07 [PATCH v3 0/6] pinctrl: scmi: support i.MX95 OEM extensions Peng Fan (OSS)
2024-04-28  5:07 ` Peng Fan (OSS)
2024-04-28  5:07 ` [PATCH v3 1/6] dt-bindings: firmware: arm,scmi: Add properties for i.MX95 Pinctrl " Peng Fan (OSS)
2024-04-28  5:07   ` Peng Fan (OSS)
2024-04-28  5:07 ` [PATCH v3 2/6] pinctrl: scmi: move pinctrl_ops to scmi_pinctrl Peng Fan (OSS)
2024-04-28  5:07   ` Peng Fan (OSS)
2024-04-28  5:07 ` [PATCH v3 3/6] pinctrl: core: guard with __PINCTRL_CORE_H Peng Fan (OSS)
2024-04-28  5:07   ` Peng Fan (OSS)
2024-04-28  5:07 ` [PATCH v3 4/6] pinctrl: scmi: export pinctrl_scmi_get_pins Peng Fan (OSS)
2024-04-28  5:07   ` Peng Fan (OSS)
2024-05-01 12:36   ` Cristian Marussi
2024-05-01 12:36     ` Cristian Marussi
2024-05-01 12:42     ` Peng Fan
2024-05-01 12:42       ` Peng Fan
2024-04-28  5:07 ` [PATCH v3 5/6] pinctrl: scmi: add blocklist Peng Fan (OSS)
2024-04-28  5:07   ` Peng Fan (OSS)
2024-04-28  5:07 ` [PATCH v3 6/6] pinctrl: imx: support SCMI pinctrl protocol for i.MX95 Peng Fan (OSS)
2024-04-28  5:07   ` Peng Fan (OSS)
2024-04-28 12:11   ` Dan Carpenter
2024-04-28 12:11     ` Dan Carpenter
2024-04-28 12:21     ` Peng Fan
2024-04-28 12:21       ` Peng Fan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.