[PATCH v6 0/7] Add pin control support for lpass sc7280

[PATCH v6 0/7] Add pin control support for lpass sc7280

[Srinivasa Rao Mandadapu]

This patch series is to split lpass variant common pin control
functions and SoC specific functions and to add lpass sc7280 pincontrol support.
It also Adds dt-bindings for lpass sc7280 lpass lpi pincontrol.

Changes Since V5:
    -- Create new patch by updating macro name to lpi specific.
    -- Create new patch by updating lpi pin group structure with core group_desc structure.
    -- Fix typo errors.
    -- Sort macros in the make file and configuration file.
Changes Since V4:
    -- Update commit message and description of the chip specific extraction patch.
    -- Sort macros in kconfig and makefile.
    -- Update optional clock voting to conditional clock voting.
    -- Fix typo errors.
    -- Move to quicinc domain email id's.
Changes Since V3:
    -- Update separate Kconfig fields for sm8250 and sc7280.
    -- Update module license and description.
    -- Move static variables to corresponding .c files from header file.

Changes Since V2:
    -- Add new dt-bindings for sc7280 lpi driver.
    -- Make clock voting change as separate patch.
    -- Split existing pincontrol driver and make common functions 
       as part of separate file.
    -- Rename lpass pincontrol lpi dt-bindings to sm8250 specific dt-bindings
		
Changes Since V1:
    -- Make lpi pinctrl variant data structure as constant
    -- Add appropriate commit message
    -- Change signedoff by sequence.

Srinivasa Rao Mandadapu (7):
  dt-bindings: pinctrl: qcom: Update lpass lpi file name to SoC specific
  dt-bindings: pinctrl: qcom: Add sc7280 lpass lpi pinctrl bindings
  pinctrl: qcom: Update macro name to LPI specific
  pinctrl: qcom: Update lpi pin group structure
  pinctrl: qcom: Extract chip specific LPASS LPI code
  pinctrl: qcom: Add SC7280 lpass pin configuration
  pinctrl: qcom: Update clock voting as optional

 .../bindings/pinctrl/qcom,lpass-lpi-pinctrl.yaml   | 133 -----------
 .../pinctrl/qcom,sc7280-lpass-lpi-pinctrl.yaml     | 115 +++++++++
 .../pinctrl/qcom,sm8250-lpass-lpi-pinctrl.yaml     | 133 +++++++++++
 drivers/pinctrl/qcom/Kconfig                       |  16 ++
 drivers/pinctrl/qcom/Makefile                      |   2 +
 drivers/pinctrl/qcom/pinctrl-lpass-lpi.c           | 257 ++-------------------
 drivers/pinctrl/qcom/pinctrl-lpass-lpi.h           |  87 +++++++
 drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c    | 170 ++++++++++++++
 drivers/pinctrl/qcom/pinctrl-sm8250-lpass-lpi.c    | 166 +++++++++++++
 9 files changed, 706 insertions(+), 373 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,lpass-lpi-pinctrl.yaml
 create mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,sc7280-lpass-lpi-pinctrl.yaml
 create mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,sm8250-lpass-lpi-pinctrl.yaml
 create mode 100644 drivers/pinctrl/qcom/pinctrl-lpass-lpi.h
 create mode 100644 drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c
 create mode 100644 drivers/pinctrl/qcom/pinctrl-sm8250-lpass-lpi.c

-- 
2.7.4

[PATCH v6 1/7] dt-bindings: pinctrl: qcom: Update lpass lpi file name to SoC specific

[Srinivasa Rao Mandadapu]

Change generic lpass lpi pincotrol bindings file to SoC specific file,
to distinguish and accomadate other SoC specific dt bindings.

Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
Co-developed-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
Signed-off-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
---
 .../bindings/pinctrl/qcom,lpass-lpi-pinctrl.yaml   | 133 ---------------------
 .../pinctrl/qcom,sm8250-lpass-lpi-pinctrl.yaml     | 133 +++++++++++++++++++++
 2 files changed, 133 insertions(+), 133 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,lpass-lpi-pinctrl.yaml
 create mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,sm8250-lpass-lpi-pinctrl.yaml

diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,lpass-lpi-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/qcom,lpass-lpi-pinctrl.yaml
deleted file mode 100644
index 5c5542f..0000000
--- a/Documentation/devicetree/bindings/pinctrl/qcom,lpass-lpi-pinctrl.yaml
+++ /dev/null
@@ -1,133 +0,0 @@
-# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
-%YAML 1.2
----
-$id: http://devicetree.org/schemas/pinctrl/qcom,lpass-lpi-pinctrl.yaml#
-$schema: http://devicetree.org/meta-schemas/core.yaml#
-
-title: Qualcomm Technologies, Inc. Low Power Audio SubSystem (LPASS)
-  Low Power Island (LPI) TLMM block
-
-maintainers:
-  - Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
-
-description: |
-  This binding describes the Top Level Mode Multiplexer block found in the
-  LPASS LPI IP on most Qualcomm SoCs
-
-properties:
-  compatible:
-    const: qcom,sm8250-lpass-lpi-pinctrl
-
-  reg:
-    minItems: 2
-    maxItems: 2
-
-  clocks:
-    items:
-      - description: LPASS Core voting clock
-      - description: LPASS Audio voting clock
-
-  clock-names:
-    items:
-      - const: core
-      - const: audio
-
-  gpio-controller: true
-
-  '#gpio-cells':
-    description: Specifying the pin number and flags, as defined in
-      include/dt-bindings/gpio/gpio.h
-    const: 2
-
-  gpio-ranges:
-    maxItems: 1
-
-#PIN CONFIGURATION NODES
-patternProperties:
-  '-pins$':
-    type: object
-    description:
-      Pinctrl node's client devices use subnodes for desired pin configuration.
-      Client device subnodes use below standard properties.
-    $ref: "/schemas/pinctrl/pincfg-node.yaml"
-
-    properties:
-      pins:
-        description:
-          List of gpio pins affected by the properties specified in this
-          subnode.
-        items:
-          oneOf:
-            - pattern: "^gpio([0-9]|[1-9][0-9])$"
-        minItems: 1
-        maxItems: 14
-
-      function:
-        enum: [ gpio, swr_tx_clk, qua_mi2s_sclk, swr_tx_data, qua_mi2s_ws,
-                qua_mi2s_data, swr_rx_clk, swr_rx_data, dmic1_clk, i2s1_clk,
-                dmic1_data, i2s1_ws, dmic2_clk, dmic2_data, i2s1_data,
-                i2s2_clk, wsa_swr_clk, i2s2_ws, wsa_swr_data, dmic3_clk,
-                dmic3_data, i2s2_data ]
-        description:
-          Specify the alternative function to be configured for the specified
-          pins.
-
-      drive-strength:
-        enum: [2, 4, 6, 8, 10, 12, 14, 16]
-        default: 2
-        description:
-          Selects the drive strength for the specified pins, in mA.
-
-      slew-rate:
-        enum: [0, 1, 2, 3]
-        default: 0
-        description: |
-            0: No adjustments
-            1: Higher Slew rate (faster edges)
-            2: Lower Slew rate (slower edges)
-            3: Reserved (No adjustments)
-
-      bias-pull-down: true
-
-      bias-pull-up: true
-
-      bias-disable: true
-
-      output-high: true
-
-      output-low: true
-
-    required:
-      - pins
-      - function
-
-    additionalProperties: false
-
-allOf:
-  - $ref: "pinctrl.yaml#"
-
-required:
-  - compatible
-  - reg
-  - clocks
-  - clock-names
-  - gpio-controller
-  - '#gpio-cells'
-  - gpio-ranges
-
-additionalProperties: false
-
-examples:
-  - |
-    #include <dt-bindings/sound/qcom,q6afe.h>
-    lpi_tlmm: pinctrl@33c0000 {
-        compatible = "qcom,sm8250-lpass-lpi-pinctrl";
-        reg = <0x33c0000 0x20000>,
-              <0x3550000 0x10000>;
-        clocks = <&q6afecc LPASS_HW_MACRO_VOTE LPASS_CLK_ATTRIBUTE_COUPLE_NO>,
-                 <&q6afecc LPASS_HW_DCODEC_VOTE LPASS_CLK_ATTRIBUTE_COUPLE_NO>;
-        clock-names = "core", "audio";
-        gpio-controller;
-        #gpio-cells = <2>;
-        gpio-ranges = <&lpi_tlmm 0 0 14>;
-    };
diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,sm8250-lpass-lpi-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/qcom,sm8250-lpass-lpi-pinctrl.yaml
new file mode 100644
index 0000000..06efb13
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/qcom,sm8250-lpass-lpi-pinctrl.yaml
@@ -0,0 +1,133 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pinctrl/qcom,sm8250-lpass-lpi-pinctrl.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Technologies, Inc. Low Power Audio SubSystem (LPASS)
+  Low Power Island (LPI) TLMM block
+
+maintainers:
+  - Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
+
+description: |
+  This binding describes the Top Level Mode Multiplexer block found in the
+  LPASS LPI IP on most Qualcomm SoCs
+
+properties:
+  compatible:
+    const: qcom,sm8250-lpass-lpi-pinctrl
+
+  reg:
+    minItems: 2
+    maxItems: 2
+
+  clocks:
+    items:
+      - description: LPASS Core voting clock
+      - description: LPASS Audio voting clock
+
+  clock-names:
+    items:
+      - const: core
+      - const: audio
+
+  gpio-controller: true
+
+  '#gpio-cells':
+    description: Specifying the pin number and flags, as defined in
+      include/dt-bindings/gpio/gpio.h
+    const: 2
+
+  gpio-ranges:
+    maxItems: 1
+
+#PIN CONFIGURATION NODES
+patternProperties:
+  '-pins$':
+    type: object
+    description:
+      Pinctrl node's client devices use subnodes for desired pin configuration.
+      Client device subnodes use below standard properties.
+    $ref: "/schemas/pinctrl/pincfg-node.yaml"
+
+    properties:
+      pins:
+        description:
+          List of gpio pins affected by the properties specified in this
+          subnode.
+        items:
+          oneOf:
+            - pattern: "^gpio([0-9]|[1-9][0-9])$"
+        minItems: 1
+        maxItems: 14
+
+      function:
+        enum: [ gpio, swr_tx_clk, qua_mi2s_sclk, swr_tx_data, qua_mi2s_ws,
+                qua_mi2s_data, swr_rx_clk, swr_rx_data, dmic1_clk, i2s1_clk,
+                dmic1_data, i2s1_ws, dmic2_clk, dmic2_data, i2s1_data,
+                i2s2_clk, wsa_swr_clk, i2s2_ws, wsa_swr_data, dmic3_clk,
+                dmic3_data, i2s2_data ]
+        description:
+          Specify the alternative function to be configured for the specified
+          pins.
+
+      drive-strength:
+        enum: [2, 4, 6, 8, 10, 12, 14, 16]
+        default: 2
+        description:
+          Selects the drive strength for the specified pins, in mA.
+
+      slew-rate:
+        enum: [0, 1, 2, 3]
+        default: 0
+        description: |
+            0: No adjustments
+            1: Higher Slew rate (faster edges)
+            2: Lower Slew rate (slower edges)
+            3: Reserved (No adjustments)
+
+      bias-pull-down: true
+
+      bias-pull-up: true
+
+      bias-disable: true
+
+      output-high: true
+
+      output-low: true
+
+    required:
+      - pins
+      - function
+
+    additionalProperties: false
+
+allOf:
+  - $ref: "pinctrl.yaml#"
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - gpio-controller
+  - '#gpio-cells'
+  - gpio-ranges
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/sound/qcom,q6afe.h>
+    lpi_tlmm: pinctrl@33c0000 {
+        compatible = "qcom,sm8250-lpass-lpi-pinctrl";
+        reg = <0x33c0000 0x20000>,
+              <0x3550000 0x10000>;
+        clocks = <&q6afecc LPASS_HW_MACRO_VOTE LPASS_CLK_ATTRIBUTE_COUPLE_NO>,
+                 <&q6afecc LPASS_HW_DCODEC_VOTE LPASS_CLK_ATTRIBUTE_COUPLE_NO>;
+        clock-names = "core", "audio";
+        gpio-controller;
+        #gpio-cells = <2>;
+        gpio-ranges = <&lpi_tlmm 0 0 14>;
+    };
-- 
2.7.4

Re: [PATCH v6 1/7] dt-bindings: pinctrl: qcom: Update lpass lpi file name to SoC specific

[Rob Herring]

On Mon, 14 Feb 2022 20:49:48 +0530, Srinivasa Rao Mandadapu wrote:
> Change generic lpass lpi pincotrol bindings file to SoC specific file,
> to distinguish and accomadate other SoC specific dt bindings.
> 
> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
> Co-developed-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
> Signed-off-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
> ---
>  .../bindings/pinctrl/qcom,lpass-lpi-pinctrl.yaml   | 133 ---------------------
>  .../pinctrl/qcom,sm8250-lpass-lpi-pinctrl.yaml     | 133 +++++++++++++++++++++
>  2 files changed, 133 insertions(+), 133 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,lpass-lpi-pinctrl.yaml
>  create mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,sm8250-lpass-lpi-pinctrl.yaml
> 


Please add Acked-by/Reviewed-by tags when posting new versions. However,
there's no need to repost patches *only* to add the tags. The upstream
maintainer will do that for acks received on the version they apply.

If a tag was not added on purpose, please state why and what changed.

Re: [PATCH v6 1/7] dt-bindings: pinctrl: qcom: Update lpass lpi file name to SoC specific

[Stephen Boyd]

Quoting Srinivasa Rao Mandadapu (2022-02-14 07:19:48)
> Change generic lpass lpi pincotrol bindings file to SoC specific file,
> to distinguish and accomadate other SoC specific dt bindings.
>
> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
> Co-developed-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
> Signed-off-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
> ---

Reviewed-by: Stephen Boyd <swboyd@chromium.org>

If you generate the patch with git format-patch -M -C does it detect
this is largely a copy? I tried myself and it looks like it is.

diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,lpass-lpi-pinctrl.yaml
b/Documentation/devicetree/bindings/pinctrl/qcom,sm8250-lpass-lpi-pinctrl.yaml
similarity index 97%
rename from Documentation/devicetree/bindings/pinctrl/qcom,lpass-lpi-pinctrl.yaml
rename to Documentation/devicetree/bindings/pinctrl/qcom,sm8250-lpass-lpi-pinctrl.yaml
index 5c5542f1627c..06efb1382876 100644
--- a/Documentation/devicetree/bindings/pinctrl/qcom,lpass-lpi-pinctrl.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/qcom,sm8250-lpass-lpi-pinctrl.yaml
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
 %YAML 1.2
 ---
-$id: http://devicetree.org/schemas/pinctrl/qcom,lpass-lpi-pinctrl.yaml#
+$id: http://devicetree.org/schemas/pinctrl/qcom,sm8250-lpass-lpi-pinctrl.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#

 title: Qualcomm Technologies, Inc. Low Power Audio SubSystem (LPASS)


Please generate with -M and -C in the future.

[PATCH v6 2/7] dt-bindings: pinctrl: qcom: Add sc7280 lpass lpi pinctrl bindings

[Srinivasa Rao Mandadapu]

Add device tree binding Documentation details for Qualcomm SC7280
LPASS LPI pinctrl driver.

Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
Co-developed-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
Signed-off-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../pinctrl/qcom,sc7280-lpass-lpi-pinctrl.yaml     | 115 +++++++++++++++++++++
 1 file changed, 115 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,sc7280-lpass-lpi-pinctrl.yaml

diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,sc7280-lpass-lpi-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/qcom,sc7280-lpass-lpi-pinctrl.yaml
new file mode 100644
index 0000000..d32ee32
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/qcom,sc7280-lpass-lpi-pinctrl.yaml
@@ -0,0 +1,115 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pinctrl/qcom,sc7280-lpass-lpi-pinctrl.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Technologies, Inc. Low Power Audio SubSystem (LPASS)
+  Low Power Island (LPI) TLMM block
+
+maintainers:
+  - Srinivasa Rao Mandadapu <srivasam@codeaurora.org>
+  - Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
+
+description: |
+  This binding describes the Top Level Mode Multiplexer block found in the
+  LPASS LPI IP on most Qualcomm SoCs
+
+properties:
+  compatible:
+    const: qcom,sc7280-lpass-lpi-pinctrl
+
+  reg:
+    minItems: 2
+    maxItems: 2
+
+  gpio-controller: true
+
+  '#gpio-cells':
+    description: Specifying the pin number and flags, as defined in
+      include/dt-bindings/gpio/gpio.h
+    const: 2
+
+  gpio-ranges:
+    maxItems: 1
+
+#PIN CONFIGURATION NODES
+patternProperties:
+  '-pins$':
+    type: object
+    description:
+      Pinctrl node's client devices use subnodes for desired pin configuration.
+      Client device subnodes use below standard properties.
+    $ref: "/schemas/pinctrl/pincfg-node.yaml"
+
+    properties:
+      pins:
+        description:
+          List of gpio pins affected by the properties specified in this
+          subnode.
+        items:
+          oneOf:
+            - pattern: "^gpio([0-9]|[1-9][0-9])$"
+        minItems: 1
+        maxItems: 15
+
+      function:
+        enum: [ gpio, swr_tx_clk, qua_mi2s_sclk, swr_tx_data, qua_mi2s_ws,
+                qua_mi2s_data, swr_rx_clk, swr_rx_data, dmic1_clk, i2s1_clk,
+                dmic1_data, i2s1_ws, dmic2_clk, dmic2_data, i2s1_data,
+                i2s2_clk, wsa_swr_clk, i2s2_ws, wsa_swr_data, dmic3_clk,
+                dmic3_data, i2s2_data ]
+        description:
+          Specify the alternative function to be configured for the specified
+          pins.
+
+      drive-strength:
+        enum: [2, 4, 6, 8, 10, 12, 14, 16]
+        default: 2
+        description:
+          Selects the drive strength for the specified pins, in mA.
+
+      slew-rate:
+        enum: [0, 1, 2, 3]
+        default: 0
+        description: |
+            0: No adjustments
+            1: Higher Slew rate (faster edges)
+            2: Lower Slew rate (slower edges)
+            3: Reserved (No adjustments)
+
+      bias-pull-down: true
+
+      bias-pull-up: true
+
+      bias-disable: true
+
+      output-high: true
+
+      output-low: true
+
+    required:
+      - pins
+      - function
+
+    additionalProperties: false
+
+required:
+  - compatible
+  - reg
+  - gpio-controller
+  - '#gpio-cells'
+  - gpio-ranges
+
+additionalProperties: false
+
+examples:
+  - |
+    lpass_tlmm: pinctrl@33c0000 {
+        compatible = "qcom,sc7280-lpass-lpi-pinctrl";
+        reg = <0x33c0000 0x20000>,
+              <0x3550000 0x10000>;
+        gpio-controller;
+        #gpio-cells = <2>;
+        gpio-ranges = <&lpass_tlmm 0 0 15>;
+    };
-- 
2.7.4

Re: [PATCH v6 2/7] dt-bindings: pinctrl: qcom: Add sc7280 lpass lpi pinctrl bindings

[Stephen Boyd]

Quoting Srinivasa Rao Mandadapu (2022-02-14 07:19:49)
> Add device tree binding Documentation details for Qualcomm SC7280
> LPASS LPI pinctrl driver.
>
> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
> Co-developed-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
> Signed-off-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
> ---

I used --find-copies-harder and got

 copy Documentation/devicetree/bindings/pinctrl/{qcom,sm8250-lpass-lpi-pinctrl.yaml
=> qcom,sc7280-lpass-lpi-pinctrl.yaml} (77%)

and the diff looks good so

Reviewed-by: Stephen Boyd <swboyd@chromium.org>

[PATCH v6 3/7] pinctrl: qcom: Update macro name to LPI specific

[Srinivasa Rao Mandadapu]

Update NO_SLEW macro to LPI_NO_SLEW macro as this driver lpi specific.

Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
Co-developed-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
Signed-off-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
---
 drivers/pinctrl/qcom/pinctrl-lpass-lpi.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
index 2f19ab4..3c15f80 100644
--- a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
+++ b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
@@ -40,7 +40,7 @@
 #define LPI_GPIO_KEEPER			0x2
 #define LPI_GPIO_PULL_UP		0x3
 #define LPI_GPIO_DS_TO_VAL(v)		(v / 2 - 1)
-#define NO_SLEW				-1
+#define LPI_NO_SLEW				-1
 
 #define LPI_FUNCTION(fname)			                \
 	[LPI_MUX_##fname] = {		                \
@@ -193,14 +193,14 @@ static const struct lpi_pingroup sm8250_groups[] = {
 	LPI_PINGROUP(3, 8, swr_rx_clk, qua_mi2s_data, _, _),
 	LPI_PINGROUP(4, 10, swr_rx_data, qua_mi2s_data, _, _),
 	LPI_PINGROUP(5, 12, swr_tx_data, swr_rx_data, _, _),
-	LPI_PINGROUP(6, NO_SLEW, dmic1_clk, i2s1_clk, _,  _),
-	LPI_PINGROUP(7, NO_SLEW, dmic1_data, i2s1_ws, _, _),
-	LPI_PINGROUP(8, NO_SLEW, dmic2_clk, i2s1_data, _, _),
-	LPI_PINGROUP(9, NO_SLEW, dmic2_data, i2s1_data, _, _),
+	LPI_PINGROUP(6, LPI_NO_SLEW, dmic1_clk, i2s1_clk, _,  _),
+	LPI_PINGROUP(7, LPI_NO_SLEW, dmic1_data, i2s1_ws, _, _),
+	LPI_PINGROUP(8, LPI_NO_SLEW, dmic2_clk, i2s1_data, _, _),
+	LPI_PINGROUP(9, LPI_NO_SLEW, dmic2_data, i2s1_data, _, _),
 	LPI_PINGROUP(10, 16, i2s2_clk, wsa_swr_clk, _, _),
 	LPI_PINGROUP(11, 18, i2s2_ws, wsa_swr_data, _, _),
-	LPI_PINGROUP(12, NO_SLEW, dmic3_clk, i2s2_data, _, _),
-	LPI_PINGROUP(13, NO_SLEW, dmic3_data, i2s2_data, _, _),
+	LPI_PINGROUP(12, LPI_NO_SLEW, dmic3_clk, i2s2_data, _, _),
+	LPI_PINGROUP(13, LPI_NO_SLEW, dmic3_data, i2s2_data, _, _),
 };
 
 static const struct lpi_function sm8250_functions[] = {
@@ -435,7 +435,7 @@ static int lpi_config_set(struct pinctrl_dev *pctldev, unsigned int group,
 			}
 
 			slew_offset = g->slew_offset;
-			if (slew_offset == NO_SLEW)
+			if (slew_offset == LPI_NO_SLEW)
 				break;
 
 			mutex_lock(&pctrl->slew_access_lock);
-- 
2.7.4

Re: [PATCH v6 3/7] pinctrl: qcom: Update macro name to LPI specific

[Stephen Boyd]

Quoting Srinivasa Rao Mandadapu (2022-02-14 07:19:50)
> Update NO_SLEW macro to LPI_NO_SLEW macro as this driver lpi specific.

Is this to avoid NO_SLEW somewhere else?

>
> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
> Co-developed-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
> Signed-off-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
> ---

Reviewed-by: Stephen Boyd <swboyd@chromium.org>

Re: [PATCH v6 3/7] pinctrl: qcom: Update macro name to LPI specific

[Srinivasa Rao Mandadapu (Temp)]

On 2/19/2022 8:06 AM, Stephen Boyd wrote:
Thanks for Your time Stephen!!!
> Quoting Srinivasa Rao Mandadapu (2022-02-14 07:19:50)
>> Update NO_SLEW macro to LPI_NO_SLEW macro as this driver lpi specific.
> Is this to avoid NO_SLEW somewhere else?
The above comment addressed resent v7 patches.
>
>> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
>> Co-developed-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
>> Signed-off-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
>> ---
> Reviewed-by: Stephen Boyd <swboyd@chromium.org>

[PATCH v6 4/7] pinctrl: qcom: Update lpi pin group structure

[Srinivasa Rao Mandadapu]

Update lpi group structure with core group_desc structure
to avoid redundant struct params.

Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
Co-developed-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
Signed-off-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
---
 drivers/pinctrl/qcom/pinctrl-lpass-lpi.c | 44 +++++++++++++++-----------------
 1 file changed, 21 insertions(+), 23 deletions(-)

diff --git a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
index 3c15f80..54750ba 100644
--- a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
+++ b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
@@ -51,11 +51,11 @@
 
 #define LPI_PINGROUP(id, soff, f1, f2, f3, f4)		\
 	{						\
-		.name = "gpio" #id,			\
-		.pins = gpio##id##_pins,		\
+		.group.name = "gpio" #id,			\
+		.group.pins = gpio##id##_pins,		\
 		.pin = id,				\
 		.slew_offset = soff,			\
-		.npins = ARRAY_SIZE(gpio##id##_pins),	\
+		.group.num_pins = ARRAY_SIZE(gpio##id##_pins),	\
 		.funcs = (int[]){			\
 			LPI_MUX_gpio,			\
 			LPI_MUX_##f1,			\
@@ -67,9 +67,7 @@
 	}
 
 struct lpi_pingroup {
-	const char *name;
-	const unsigned int *pins;
-	unsigned int npins;
+	struct group_desc group;
 	unsigned int pin;
 	/* Bit offset in slew register for SoundWire pins only */
 	int slew_offset;
@@ -150,20 +148,20 @@ enum sm8250_lpi_functions {
 	LPI_MUX__,
 };
 
-static const unsigned int gpio0_pins[] = { 0 };
-static const unsigned int gpio1_pins[] = { 1 };
-static const unsigned int gpio2_pins[] = { 2 };
-static const unsigned int gpio3_pins[] = { 3 };
-static const unsigned int gpio4_pins[] = { 4 };
-static const unsigned int gpio5_pins[] = { 5 };
-static const unsigned int gpio6_pins[] = { 6 };
-static const unsigned int gpio7_pins[] = { 7 };
-static const unsigned int gpio8_pins[] = { 8 };
-static const unsigned int gpio9_pins[] = { 9 };
-static const unsigned int gpio10_pins[] = { 10 };
-static const unsigned int gpio11_pins[] = { 11 };
-static const unsigned int gpio12_pins[] = { 12 };
-static const unsigned int gpio13_pins[] = { 13 };
+static int gpio0_pins[] = { 0 };
+static int gpio1_pins[] = { 1 };
+static int gpio2_pins[] = { 2 };
+static int gpio3_pins[] = { 3 };
+static int gpio4_pins[] = { 4 };
+static int gpio5_pins[] = { 5 };
+static int gpio6_pins[] = { 6 };
+static int gpio7_pins[] = { 7 };
+static int gpio8_pins[] = { 8 };
+static int gpio9_pins[] = { 9 };
+static int gpio10_pins[] = { 10 };
+static int gpio11_pins[] = { 11 };
+static int gpio12_pins[] = { 12 };
+static int gpio13_pins[] = { 13 };
 static const char * const swr_tx_clk_groups[] = { "gpio0" };
 static const char * const swr_tx_data_groups[] = { "gpio1", "gpio2", "gpio5" };
 static const char * const swr_rx_clk_groups[] = { "gpio3" };
@@ -262,7 +260,7 @@ static const char *lpi_gpio_get_group_name(struct pinctrl_dev *pctldev,
 {
 	struct lpi_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
 
-	return pctrl->data->groups[group].name;
+	return pctrl->data->groups[group].group.name;
 }
 
 static int lpi_gpio_get_group_pins(struct pinctrl_dev *pctldev,
@@ -272,8 +270,8 @@ static int lpi_gpio_get_group_pins(struct pinctrl_dev *pctldev,
 {
 	struct lpi_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
 
-	*pins = pctrl->data->groups[group].pins;
-	*num_pins = pctrl->data->groups[group].npins;
+	*pins = pctrl->data->groups[group].group.pins;
+	*num_pins = pctrl->data->groups[group].group.num_pins;
 
 	return 0;
 }
-- 
2.7.4

Re: [PATCH v6 4/7] pinctrl: qcom: Update lpi pin group structure

[Stephen Boyd]

Quoting Srinivasa Rao Mandadapu (2022-02-14 07:19:51)
> @@ -150,20 +148,20 @@ enum sm8250_lpi_functions {
>         LPI_MUX__,
>  };
>
> -static const unsigned int gpio0_pins[] = { 0 };
> -static const unsigned int gpio1_pins[] = { 1 };
> -static const unsigned int gpio2_pins[] = { 2 };
> -static const unsigned int gpio3_pins[] = { 3 };
> -static const unsigned int gpio4_pins[] = { 4 };
> -static const unsigned int gpio5_pins[] = { 5 };
> -static const unsigned int gpio6_pins[] = { 6 };
> -static const unsigned int gpio7_pins[] = { 7 };
> -static const unsigned int gpio8_pins[] = { 8 };
> -static const unsigned int gpio9_pins[] = { 9 };
> -static const unsigned int gpio10_pins[] = { 10 };
> -static const unsigned int gpio11_pins[] = { 11 };
> -static const unsigned int gpio12_pins[] = { 12 };
> -static const unsigned int gpio13_pins[] = { 13 };
> +static int gpio0_pins[] = { 0 };

Why do we lose const?

> +static int gpio1_pins[] = { 1 };
> +static int gpio2_pins[] = { 2 };
> +static int gpio3_pins[] = { 3 };
> +static int gpio4_pins[] = { 4 };
> +static int gpio5_pins[] = { 5 };
> +static int gpio6_pins[] = { 6 };
> +static int gpio7_pins[] = { 7 };
> +static int gpio8_pins[] = { 8 };
> +static int gpio9_pins[] = { 9 };
> +static int gpio10_pins[] = { 10 };
> +static int gpio11_pins[] = { 11 };
> +static int gpio12_pins[] = { 12 };
> +static int gpio13_pins[] = { 13 };
>  static const char * const swr_tx_clk_groups[] = { "gpio0" };
>  static const char * const swr_tx_data_groups[] = { "gpio1", "gpio2", "gpio5" };
>  static const char * const swr_rx_clk_groups[] = { "gpio3" };

Re: [PATCH v6 4/7] pinctrl: qcom: Update lpi pin group structure

[Srinivasa Rao Mandadapu (Temp)]

On 2/19/2022 8:08 AM, Stephen Boyd wrote:
Thanks for your time Stephen!!!
> Quoting Srinivasa Rao Mandadapu (2022-02-14 07:19:51)
>> @@ -150,20 +148,20 @@ enum sm8250_lpi_functions {
>>          LPI_MUX__,
>>   };
>>
>> -static const unsigned int gpio0_pins[] = { 0 };
>> -static const unsigned int gpio1_pins[] = { 1 };
>> -static const unsigned int gpio2_pins[] = { 2 };
>> -static const unsigned int gpio3_pins[] = { 3 };
>> -static const unsigned int gpio4_pins[] = { 4 };
>> -static const unsigned int gpio5_pins[] = { 5 };
>> -static const unsigned int gpio6_pins[] = { 6 };
>> -static const unsigned int gpio7_pins[] = { 7 };
>> -static const unsigned int gpio8_pins[] = { 8 };
>> -static const unsigned int gpio9_pins[] = { 9 };
>> -static const unsigned int gpio10_pins[] = { 10 };
>> -static const unsigned int gpio11_pins[] = { 11 };
>> -static const unsigned int gpio12_pins[] = { 12 };
>> -static const unsigned int gpio13_pins[] = { 13 };
>> +static int gpio0_pins[] = { 0 };
> Why do we lose const?
Const here is creating conflicts with gpio group_desc structure!!!
>
>> +static int gpio1_pins[] = { 1 };
>> +static int gpio2_pins[] = { 2 };
>> +static int gpio3_pins[] = { 3 };
>> +static int gpio4_pins[] = { 4 };
>> +static int gpio5_pins[] = { 5 };
>> +static int gpio6_pins[] = { 6 };
>> +static int gpio7_pins[] = { 7 };
>> +static int gpio8_pins[] = { 8 };
>> +static int gpio9_pins[] = { 9 };
>> +static int gpio10_pins[] = { 10 };
>> +static int gpio11_pins[] = { 11 };
>> +static int gpio12_pins[] = { 12 };
>> +static int gpio13_pins[] = { 13 };
>>   static const char * const swr_tx_clk_groups[] = { "gpio0" };
>>   static const char * const swr_tx_data_groups[] = { "gpio1", "gpio2", "gpio5" };
>>   static const char * const swr_rx_clk_groups[] = { "gpio3" };

[PATCH v6 5/7] pinctrl: qcom: Extract chip specific LPASS LPI code

[Srinivasa Rao Mandadapu]

Extract the chip specific SM8250 data from the LPASS LPI pinctrl driver
to allow reusing the common code in the addition of subsequent
platforms.

Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
Co-developed-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
Signed-off-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
---
 drivers/pinctrl/qcom/Kconfig                    |   8 +
 drivers/pinctrl/qcom/Makefile                   |   1 +
 drivers/pinctrl/qcom/pinctrl-lpass-lpi.c        | 233 +-----------------------
 drivers/pinctrl/qcom/pinctrl-lpass-lpi.h        |  86 +++++++++
 drivers/pinctrl/qcom/pinctrl-sm8250-lpass-lpi.c | 166 +++++++++++++++++
 5 files changed, 266 insertions(+), 228 deletions(-)
 create mode 100644 drivers/pinctrl/qcom/pinctrl-lpass-lpi.h
 create mode 100644 drivers/pinctrl/qcom/pinctrl-sm8250-lpass-lpi.c

diff --git a/drivers/pinctrl/qcom/Kconfig b/drivers/pinctrl/qcom/Kconfig
index ca6f68a..8871451 100644
--- a/drivers/pinctrl/qcom/Kconfig
+++ b/drivers/pinctrl/qcom/Kconfig
@@ -357,4 +357,12 @@ config PINCTRL_LPASS_LPI
 	  Qualcomm Technologies Inc LPASS (Low Power Audio SubSystem) LPI
 	  (Low Power Island) found on the Qualcomm Technologies Inc SoCs.
 
+config PINCTRL_SM8250_LPASS_LPI
+	tristate "Qualcomm Technologies Inc SM8250 LPASS LPI pin controller driver"
+	depends on PINCTRL_LPASS_LPI
+	help
+	  This is the pinctrl, pinmux, pinconf and gpiolib driver for the
+	  Qualcomm Technologies Inc LPASS (Low Power Audio SubSystem) LPI
+	  (Low Power Island) found on the Qualcomm Technologies Inc SM8250 platform.
+
 endif
diff --git a/drivers/pinctrl/qcom/Makefile b/drivers/pinctrl/qcom/Makefile
index 709882f..c10d14d2 100644
--- a/drivers/pinctrl/qcom/Makefile
+++ b/drivers/pinctrl/qcom/Makefile
@@ -41,3 +41,4 @@ obj-$(CONFIG_PINCTRL_SM8250) += pinctrl-sm8250.o
 obj-$(CONFIG_PINCTRL_SM8350) += pinctrl-sm8350.o
 obj-$(CONFIG_PINCTRL_SM8450) += pinctrl-sm8450.o
 obj-$(CONFIG_PINCTRL_LPASS_LPI) += pinctrl-lpass-lpi.o
+obj-$(CONFIG_PINCTRL_SM8250_LPASS_LPI) += pinctrl-sm8250-lpass-lpi.o
diff --git a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
index 54750ba..8a82fd9 100644
--- a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
+++ b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
@@ -4,91 +4,15 @@
  * Copyright (c) 2020 Linaro Ltd.
  */
 
-#include <linux/bitops.h>
-#include <linux/bitfield.h>
 #include <linux/clk.h>
 #include <linux/gpio/driver.h>
-#include <linux/io.h>
 #include <linux/module.h>
 #include <linux/of_device.h>
-#include <linux/of.h>
 #include <linux/pinctrl/pinconf-generic.h>
 #include <linux/pinctrl/pinconf.h>
 #include <linux/pinctrl/pinmux.h>
-#include <linux/platform_device.h>
-#include <linux/slab.h>
-#include <linux/types.h>
-#include "../core.h"
 #include "../pinctrl-utils.h"
-
-#define LPI_SLEW_RATE_CTL_REG		0xa000
-#define LPI_TLMM_REG_OFFSET		0x1000
-#define LPI_SLEW_RATE_MAX		0x03
-#define LPI_SLEW_BITS_SIZE		0x02
-#define LPI_SLEW_RATE_MASK		GENMASK(1, 0)
-#define LPI_GPIO_CFG_REG		0x00
-#define LPI_GPIO_PULL_MASK		GENMASK(1, 0)
-#define LPI_GPIO_FUNCTION_MASK		GENMASK(5, 2)
-#define LPI_GPIO_OUT_STRENGTH_MASK	GENMASK(8, 6)
-#define LPI_GPIO_OE_MASK		BIT(9)
-#define LPI_GPIO_VALUE_REG		0x04
-#define LPI_GPIO_VALUE_IN_MASK		BIT(0)
-#define LPI_GPIO_VALUE_OUT_MASK		BIT(1)
-
-#define LPI_GPIO_BIAS_DISABLE		0x0
-#define LPI_GPIO_PULL_DOWN		0x1
-#define LPI_GPIO_KEEPER			0x2
-#define LPI_GPIO_PULL_UP		0x3
-#define LPI_GPIO_DS_TO_VAL(v)		(v / 2 - 1)
-#define LPI_NO_SLEW				-1
-
-#define LPI_FUNCTION(fname)			                \
-	[LPI_MUX_##fname] = {		                \
-		.name = #fname,				\
-		.groups = fname##_groups,               \
-		.ngroups = ARRAY_SIZE(fname##_groups),	\
-	}
-
-#define LPI_PINGROUP(id, soff, f1, f2, f3, f4)		\
-	{						\
-		.group.name = "gpio" #id,			\
-		.group.pins = gpio##id##_pins,		\
-		.pin = id,				\
-		.slew_offset = soff,			\
-		.group.num_pins = ARRAY_SIZE(gpio##id##_pins),	\
-		.funcs = (int[]){			\
-			LPI_MUX_gpio,			\
-			LPI_MUX_##f1,			\
-			LPI_MUX_##f2,			\
-			LPI_MUX_##f3,			\
-			LPI_MUX_##f4,			\
-		},					\
-		.nfuncs = 5,				\
-	}
-
-struct lpi_pingroup {
-	struct group_desc group;
-	unsigned int pin;
-	/* Bit offset in slew register for SoundWire pins only */
-	int slew_offset;
-	unsigned int *funcs;
-	unsigned int nfuncs;
-};
-
-struct lpi_function {
-	const char *name;
-	const char * const *groups;
-	unsigned int ngroups;
-};
-
-struct lpi_pinctrl_variant_data {
-	const struct pinctrl_pin_desc *pins;
-	int npins;
-	const struct lpi_pingroup *groups;
-	int ngroups;
-	const struct lpi_function *functions;
-	int nfunctions;
-};
+#include "pinctrl-lpass-lpi.h"
 
 #define MAX_LPI_NUM_CLKS	2
 
@@ -104,136 +28,6 @@ struct lpi_pinctrl {
 	const struct lpi_pinctrl_variant_data *data;
 };
 
-/* sm8250 variant specific data */
-static const struct pinctrl_pin_desc sm8250_lpi_pins[] = {
-	PINCTRL_PIN(0, "gpio0"),
-	PINCTRL_PIN(1, "gpio1"),
-	PINCTRL_PIN(2, "gpio2"),
-	PINCTRL_PIN(3, "gpio3"),
-	PINCTRL_PIN(4, "gpio4"),
-	PINCTRL_PIN(5, "gpio5"),
-	PINCTRL_PIN(6, "gpio6"),
-	PINCTRL_PIN(7, "gpio7"),
-	PINCTRL_PIN(8, "gpio8"),
-	PINCTRL_PIN(9, "gpio9"),
-	PINCTRL_PIN(10, "gpio10"),
-	PINCTRL_PIN(11, "gpio11"),
-	PINCTRL_PIN(12, "gpio12"),
-	PINCTRL_PIN(13, "gpio13"),
-};
-
-enum sm8250_lpi_functions {
-	LPI_MUX_dmic1_clk,
-	LPI_MUX_dmic1_data,
-	LPI_MUX_dmic2_clk,
-	LPI_MUX_dmic2_data,
-	LPI_MUX_dmic3_clk,
-	LPI_MUX_dmic3_data,
-	LPI_MUX_i2s1_clk,
-	LPI_MUX_i2s1_data,
-	LPI_MUX_i2s1_ws,
-	LPI_MUX_i2s2_clk,
-	LPI_MUX_i2s2_data,
-	LPI_MUX_i2s2_ws,
-	LPI_MUX_qua_mi2s_data,
-	LPI_MUX_qua_mi2s_sclk,
-	LPI_MUX_qua_mi2s_ws,
-	LPI_MUX_swr_rx_clk,
-	LPI_MUX_swr_rx_data,
-	LPI_MUX_swr_tx_clk,
-	LPI_MUX_swr_tx_data,
-	LPI_MUX_wsa_swr_clk,
-	LPI_MUX_wsa_swr_data,
-	LPI_MUX_gpio,
-	LPI_MUX__,
-};
-
-static int gpio0_pins[] = { 0 };
-static int gpio1_pins[] = { 1 };
-static int gpio2_pins[] = { 2 };
-static int gpio3_pins[] = { 3 };
-static int gpio4_pins[] = { 4 };
-static int gpio5_pins[] = { 5 };
-static int gpio6_pins[] = { 6 };
-static int gpio7_pins[] = { 7 };
-static int gpio8_pins[] = { 8 };
-static int gpio9_pins[] = { 9 };
-static int gpio10_pins[] = { 10 };
-static int gpio11_pins[] = { 11 };
-static int gpio12_pins[] = { 12 };
-static int gpio13_pins[] = { 13 };
-static const char * const swr_tx_clk_groups[] = { "gpio0" };
-static const char * const swr_tx_data_groups[] = { "gpio1", "gpio2", "gpio5" };
-static const char * const swr_rx_clk_groups[] = { "gpio3" };
-static const char * const swr_rx_data_groups[] = { "gpio4", "gpio5" };
-static const char * const dmic1_clk_groups[] = { "gpio6" };
-static const char * const dmic1_data_groups[] = { "gpio7" };
-static const char * const dmic2_clk_groups[] = { "gpio8" };
-static const char * const dmic2_data_groups[] = { "gpio9" };
-static const char * const i2s2_clk_groups[] = { "gpio10" };
-static const char * const i2s2_ws_groups[] = { "gpio11" };
-static const char * const dmic3_clk_groups[] = { "gpio12" };
-static const char * const dmic3_data_groups[] = { "gpio13" };
-static const char * const qua_mi2s_sclk_groups[] = { "gpio0" };
-static const char * const qua_mi2s_ws_groups[] = { "gpio1" };
-static const char * const qua_mi2s_data_groups[] = { "gpio2", "gpio3", "gpio4" };
-static const char * const i2s1_clk_groups[] = { "gpio6" };
-static const char * const i2s1_ws_groups[] = { "gpio7" };
-static const char * const i2s1_data_groups[] = { "gpio8", "gpio9" };
-static const char * const wsa_swr_clk_groups[] = { "gpio10" };
-static const char * const wsa_swr_data_groups[] = { "gpio11" };
-static const char * const i2s2_data_groups[] = { "gpio12", "gpio12" };
-
-static const struct lpi_pingroup sm8250_groups[] = {
-	LPI_PINGROUP(0, 0, swr_tx_clk, qua_mi2s_sclk, _, _),
-	LPI_PINGROUP(1, 2, swr_tx_data, qua_mi2s_ws, _, _),
-	LPI_PINGROUP(2, 4, swr_tx_data, qua_mi2s_data, _, _),
-	LPI_PINGROUP(3, 8, swr_rx_clk, qua_mi2s_data, _, _),
-	LPI_PINGROUP(4, 10, swr_rx_data, qua_mi2s_data, _, _),
-	LPI_PINGROUP(5, 12, swr_tx_data, swr_rx_data, _, _),
-	LPI_PINGROUP(6, LPI_NO_SLEW, dmic1_clk, i2s1_clk, _,  _),
-	LPI_PINGROUP(7, LPI_NO_SLEW, dmic1_data, i2s1_ws, _, _),
-	LPI_PINGROUP(8, LPI_NO_SLEW, dmic2_clk, i2s1_data, _, _),
-	LPI_PINGROUP(9, LPI_NO_SLEW, dmic2_data, i2s1_data, _, _),
-	LPI_PINGROUP(10, 16, i2s2_clk, wsa_swr_clk, _, _),
-	LPI_PINGROUP(11, 18, i2s2_ws, wsa_swr_data, _, _),
-	LPI_PINGROUP(12, LPI_NO_SLEW, dmic3_clk, i2s2_data, _, _),
-	LPI_PINGROUP(13, LPI_NO_SLEW, dmic3_data, i2s2_data, _, _),
-};
-
-static const struct lpi_function sm8250_functions[] = {
-	LPI_FUNCTION(dmic1_clk),
-	LPI_FUNCTION(dmic1_data),
-	LPI_FUNCTION(dmic2_clk),
-	LPI_FUNCTION(dmic2_data),
-	LPI_FUNCTION(dmic3_clk),
-	LPI_FUNCTION(dmic3_data),
-	LPI_FUNCTION(i2s1_clk),
-	LPI_FUNCTION(i2s1_data),
-	LPI_FUNCTION(i2s1_ws),
-	LPI_FUNCTION(i2s2_clk),
-	LPI_FUNCTION(i2s2_data),
-	LPI_FUNCTION(i2s2_ws),
-	LPI_FUNCTION(qua_mi2s_data),
-	LPI_FUNCTION(qua_mi2s_sclk),
-	LPI_FUNCTION(qua_mi2s_ws),
-	LPI_FUNCTION(swr_rx_clk),
-	LPI_FUNCTION(swr_rx_data),
-	LPI_FUNCTION(swr_tx_clk),
-	LPI_FUNCTION(swr_tx_data),
-	LPI_FUNCTION(wsa_swr_clk),
-	LPI_FUNCTION(wsa_swr_data),
-};
-
-static struct lpi_pinctrl_variant_data sm8250_lpi_data = {
-	.pins = sm8250_lpi_pins,
-	.npins = ARRAY_SIZE(sm8250_lpi_pins),
-	.groups = sm8250_groups,
-	.ngroups = ARRAY_SIZE(sm8250_groups),
-	.functions = sm8250_functions,
-	.nfunctions = ARRAY_SIZE(sm8250_functions),
-};
-
 static int lpi_gpio_read(struct lpi_pinctrl *state, unsigned int pin,
 			 unsigned int addr)
 {
@@ -580,7 +374,7 @@ static const struct gpio_chip lpi_gpio_template = {
 	.dbg_show		= lpi_gpio_dbg_show,
 };
 
-static int lpi_pinctrl_probe(struct platform_device *pdev)
+int lpi_pinctrl_probe(struct platform_device *pdev)
 {
 	const struct lpi_pinctrl_variant_data *data;
 	struct device *dev = &pdev->dev;
@@ -659,8 +453,9 @@ static int lpi_pinctrl_probe(struct platform_device *pdev)
 
 	return ret;
 }
+EXPORT_SYMBOL(lpi_pinctrl_probe);
 
-static int lpi_pinctrl_remove(struct platform_device *pdev)
+int lpi_pinctrl_remove(struct platform_device *pdev)
 {
 	struct lpi_pinctrl *pctrl = platform_get_drvdata(pdev);
 
@@ -669,25 +464,7 @@ static int lpi_pinctrl_remove(struct platform_device *pdev)
 
 	return 0;
 }
+EXPORT_SYMBOL(lpi_pinctrl_remove);
 
-static const struct of_device_id lpi_pinctrl_of_match[] = {
-	{
-	       .compatible = "qcom,sm8250-lpass-lpi-pinctrl",
-	       .data = &sm8250_lpi_data,
-	},
-	{ }
-};
-MODULE_DEVICE_TABLE(of, lpi_pinctrl_of_match);
-
-static struct platform_driver lpi_pinctrl_driver = {
-	.driver = {
-		   .name = "qcom-lpass-lpi-pinctrl",
-		   .of_match_table = lpi_pinctrl_of_match,
-	},
-	.probe = lpi_pinctrl_probe,
-	.remove = lpi_pinctrl_remove,
-};
-
-module_platform_driver(lpi_pinctrl_driver);
 MODULE_DESCRIPTION("QTI LPI GPIO pin control driver");
 MODULE_LICENSE("GPL");
diff --git a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.h b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.h
new file mode 100644
index 0000000..a511d72
--- /dev/null
+++ b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.h
@@ -0,0 +1,86 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2016-2019, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2020 Linaro Ltd.
+ */
+#ifndef __PINCTRL_LPASS_LPI_H__
+#define __PINCTRL_LPASS_LPI_H__
+
+#include <linux/bitops.h>
+#include <linux/bitfield.h>
+#include "../core.h"
+
+#define LPI_SLEW_RATE_CTL_REG	0xa000
+#define LPI_TLMM_REG_OFFSET		0x1000
+#define LPI_SLEW_RATE_MAX		0x03
+#define LPI_SLEW_BITS_SIZE		0x02
+#define LPI_SLEW_RATE_MASK		GENMASK(1, 0)
+#define LPI_GPIO_CFG_REG		0x00
+#define LPI_GPIO_PULL_MASK		GENMASK(1, 0)
+#define LPI_GPIO_FUNCTION_MASK		GENMASK(5, 2)
+#define LPI_GPIO_OUT_STRENGTH_MASK	GENMASK(8, 6)
+#define LPI_GPIO_OE_MASK		BIT(9)
+#define LPI_GPIO_VALUE_REG		0x04
+#define LPI_GPIO_VALUE_IN_MASK		BIT(0)
+#define LPI_GPIO_VALUE_OUT_MASK		BIT(1)
+
+#define LPI_GPIO_BIAS_DISABLE		0x0
+#define LPI_GPIO_PULL_DOWN		0x1
+#define LPI_GPIO_KEEPER			0x2
+#define LPI_GPIO_PULL_UP		0x3
+#define LPI_GPIO_DS_TO_VAL(v)		(v / 2 - 1)
+#define LPI_NO_SLEW				-1
+
+#define LPI_FUNCTION(fname)			                \
+	[LPI_MUX_##fname] = {		                \
+		.name = #fname,				\
+		.groups = fname##_groups,               \
+		.ngroups = ARRAY_SIZE(fname##_groups),	\
+	}
+
+#define LPI_PINGROUP(id, soff, f1, f2, f3, f4)		\
+	{						\
+		.group.name = "gpio" #id,			\
+		.group.pins = gpio##id##_pins,		\
+		.pin = id,				\
+		.slew_offset = soff,			\
+		.group.num_pins = ARRAY_SIZE(gpio##id##_pins),	\
+		.funcs = (int[]){			\
+			LPI_MUX_gpio,			\
+			LPI_MUX_##f1,			\
+			LPI_MUX_##f2,			\
+			LPI_MUX_##f3,			\
+			LPI_MUX_##f4,			\
+		},					\
+		.nfuncs = 5,				\
+	}
+
+struct lpi_pingroup {
+	struct group_desc group;
+	unsigned int pin;
+	/* Bit offset in slew register for SoundWire pins only */
+	int slew_offset;
+	unsigned int *funcs;
+	unsigned int nfuncs;
+};
+
+struct lpi_function {
+	const char *name;
+	const char * const *groups;
+	unsigned int ngroups;
+};
+
+struct lpi_pinctrl_variant_data {
+	const struct pinctrl_pin_desc *pins;
+	int npins;
+	const struct lpi_pingroup *groups;
+	int ngroups;
+	const struct lpi_function *functions;
+	int nfunctions;
+};
+
+int lpi_pinctrl_probe(struct platform_device *pdev);
+int lpi_pinctrl_remove(struct platform_device *pdev);
+
+#endif /*__PINCTRL_LPASS_LPI_H__*/
+
diff --git a/drivers/pinctrl/qcom/pinctrl-sm8250-lpass-lpi.c b/drivers/pinctrl/qcom/pinctrl-sm8250-lpass-lpi.c
new file mode 100644
index 0000000..27e358e
--- /dev/null
+++ b/drivers/pinctrl/qcom/pinctrl-sm8250-lpass-lpi.c
@@ -0,0 +1,166 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2016-2019, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2020 Linaro Ltd.
+ */
+
+#include <linux/clk.h>
+#include <linux/gpio/driver.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+
+#include "pinctrl-lpass-lpi.h"
+
+enum lpass_lpi_functions {
+	LPI_MUX_dmic1_clk,
+	LPI_MUX_dmic1_data,
+	LPI_MUX_dmic2_clk,
+	LPI_MUX_dmic2_data,
+	LPI_MUX_dmic3_clk,
+	LPI_MUX_dmic3_data,
+	LPI_MUX_i2s1_clk,
+	LPI_MUX_i2s1_data,
+	LPI_MUX_i2s1_ws,
+	LPI_MUX_i2s2_clk,
+	LPI_MUX_i2s2_data,
+	LPI_MUX_i2s2_ws,
+	LPI_MUX_qua_mi2s_data,
+	LPI_MUX_qua_mi2s_sclk,
+	LPI_MUX_qua_mi2s_ws,
+	LPI_MUX_swr_rx_clk,
+	LPI_MUX_swr_rx_data,
+	LPI_MUX_swr_tx_clk,
+	LPI_MUX_swr_tx_data,
+	LPI_MUX_wsa_swr_clk,
+	LPI_MUX_wsa_swr_data,
+	LPI_MUX_gpio,
+	LPI_MUX__,
+};
+
+static int gpio0_pins[] = { 0 };
+static int gpio1_pins[] = { 1 };
+static int gpio2_pins[] = { 2 };
+static int gpio3_pins[] = { 3 };
+static int gpio4_pins[] = { 4 };
+static int gpio5_pins[] = { 5 };
+static int gpio6_pins[] = { 6 };
+static int gpio7_pins[] = { 7 };
+static int gpio8_pins[] = { 8 };
+static int gpio9_pins[] = { 9 };
+static int gpio10_pins[] = { 10 };
+static int gpio11_pins[] = { 11 };
+static int gpio12_pins[] = { 12 };
+static int gpio13_pins[] = { 13 };
+
+
+/* sm8250 variant specific data */
+static const struct pinctrl_pin_desc sm8250_lpi_pins[] = {
+	PINCTRL_PIN(0, "gpio0"),
+	PINCTRL_PIN(1, "gpio1"),
+	PINCTRL_PIN(2, "gpio2"),
+	PINCTRL_PIN(3, "gpio3"),
+	PINCTRL_PIN(4, "gpio4"),
+	PINCTRL_PIN(5, "gpio5"),
+	PINCTRL_PIN(6, "gpio6"),
+	PINCTRL_PIN(7, "gpio7"),
+	PINCTRL_PIN(8, "gpio8"),
+	PINCTRL_PIN(9, "gpio9"),
+	PINCTRL_PIN(10, "gpio10"),
+	PINCTRL_PIN(11, "gpio11"),
+	PINCTRL_PIN(12, "gpio12"),
+	PINCTRL_PIN(13, "gpio13"),
+};
+
+static const char * const swr_tx_clk_groups[] = { "gpio0" };
+static const char * const swr_tx_data_groups[] = { "gpio1", "gpio2", "gpio5" };
+static const char * const swr_rx_clk_groups[] = { "gpio3" };
+static const char * const swr_rx_data_groups[] = { "gpio4", "gpio5" };
+static const char * const dmic1_clk_groups[] = { "gpio6" };
+static const char * const dmic1_data_groups[] = { "gpio7" };
+static const char * const dmic2_clk_groups[] = { "gpio8" };
+static const char * const dmic2_data_groups[] = { "gpio9" };
+static const char * const i2s2_clk_groups[] = { "gpio10" };
+static const char * const i2s2_ws_groups[] = { "gpio11" };
+static const char * const dmic3_clk_groups[] = { "gpio12" };
+static const char * const dmic3_data_groups[] = { "gpio13" };
+static const char * const qua_mi2s_sclk_groups[] = { "gpio0" };
+static const char * const qua_mi2s_ws_groups[] = { "gpio1" };
+static const char * const qua_mi2s_data_groups[] = { "gpio2", "gpio3", "gpio4" };
+static const char * const i2s1_clk_groups[] = { "gpio6" };
+static const char * const i2s1_ws_groups[] = { "gpio7" };
+static const char * const i2s1_data_groups[] = { "gpio8", "gpio9" };
+static const char * const wsa_swr_clk_groups[] = { "gpio10" };
+static const char * const wsa_swr_data_groups[] = { "gpio11" };
+static const char * const i2s2_data_groups[] = { "gpio12", "gpio12" };
+
+static const struct lpi_pingroup sm8250_groups[] = {
+	LPI_PINGROUP(0, 0, swr_tx_clk, qua_mi2s_sclk, _, _),
+	LPI_PINGROUP(1, 2, swr_tx_data, qua_mi2s_ws, _, _),
+	LPI_PINGROUP(2, 4, swr_tx_data, qua_mi2s_data, _, _),
+	LPI_PINGROUP(3, 8, swr_rx_clk, qua_mi2s_data, _, _),
+	LPI_PINGROUP(4, 10, swr_rx_data, qua_mi2s_data, _, _),
+	LPI_PINGROUP(5, 12, swr_tx_data, swr_rx_data, _, _),
+	LPI_PINGROUP(6, LPI_NO_SLEW, dmic1_clk, i2s1_clk, _,  _),
+	LPI_PINGROUP(7, LPI_NO_SLEW, dmic1_data, i2s1_ws, _, _),
+	LPI_PINGROUP(8, LPI_NO_SLEW, dmic2_clk, i2s1_data, _, _),
+	LPI_PINGROUP(9, LPI_NO_SLEW, dmic2_data, i2s1_data, _, _),
+	LPI_PINGROUP(10, 16, i2s2_clk, wsa_swr_clk, _, _),
+	LPI_PINGROUP(11, 18, i2s2_ws, wsa_swr_data, _, _),
+	LPI_PINGROUP(12, LPI_NO_SLEW, dmic3_clk, i2s2_data, _, _),
+	LPI_PINGROUP(13, LPI_NO_SLEW, dmic3_data, i2s2_data, _, _),
+};
+
+static const struct lpi_function sm8250_functions[] = {
+	LPI_FUNCTION(dmic1_clk),
+	LPI_FUNCTION(dmic1_data),
+	LPI_FUNCTION(dmic2_clk),
+	LPI_FUNCTION(dmic2_data),
+	LPI_FUNCTION(dmic3_clk),
+	LPI_FUNCTION(dmic3_data),
+	LPI_FUNCTION(i2s1_clk),
+	LPI_FUNCTION(i2s1_data),
+	LPI_FUNCTION(i2s1_ws),
+	LPI_FUNCTION(i2s2_clk),
+	LPI_FUNCTION(i2s2_data),
+	LPI_FUNCTION(i2s2_ws),
+	LPI_FUNCTION(qua_mi2s_data),
+	LPI_FUNCTION(qua_mi2s_sclk),
+	LPI_FUNCTION(qua_mi2s_ws),
+	LPI_FUNCTION(swr_rx_clk),
+	LPI_FUNCTION(swr_rx_data),
+	LPI_FUNCTION(swr_tx_clk),
+	LPI_FUNCTION(swr_tx_data),
+	LPI_FUNCTION(wsa_swr_clk),
+	LPI_FUNCTION(wsa_swr_data),
+};
+
+static struct lpi_pinctrl_variant_data sm8250_lpi_data = {
+	.pins = sm8250_lpi_pins,
+	.npins = ARRAY_SIZE(sm8250_lpi_pins),
+	.groups = sm8250_groups,
+	.ngroups = ARRAY_SIZE(sm8250_groups),
+	.functions = sm8250_functions,
+	.nfunctions = ARRAY_SIZE(sm8250_functions),
+};
+
+static const struct of_device_id lpi_pinctrl_of_match[] = {
+	{
+	       .compatible = "qcom,sm8250-lpass-lpi-pinctrl",
+	       .data = &sm8250_lpi_data,
+	},
+	{ }
+};
+MODULE_DEVICE_TABLE(of, lpi_pinctrl_of_match);
+
+static struct platform_driver lpi_pinctrl_driver = {
+	.driver = {
+		   .name = "qcom-sm8250-lpass-lpi-pinctrl",
+		   .of_match_table = lpi_pinctrl_of_match,
+	},
+	.probe = lpi_pinctrl_probe,
+	.remove = lpi_pinctrl_remove,
+};
+
+module_platform_driver(lpi_pinctrl_driver);
+MODULE_DESCRIPTION("QTI SM8250 LPI GPIO pin control driver");
+MODULE_LICENSE("GPL");
-- 
2.7.4

Re: [PATCH v6 5/7] pinctrl: qcom: Extract chip specific LPASS LPI code

[Srinivas Kandagatla]

On 14/02/2022 15:19, Srinivasa Rao Mandadapu wrote:
> Extract the chip specific SM8250 data from the LPASS LPI pinctrl driver
> to allow reusing the common code in the addition of subsequent
> platforms.
> 
> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
> Co-developed-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
> Signed-off-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
> ---
>   drivers/pinctrl/qcom/Kconfig                    |   8 +
>   drivers/pinctrl/qcom/Makefile                   |   1 +
>   drivers/pinctrl/qcom/pinctrl-lpass-lpi.c        | 233 +-----------------------
>   drivers/pinctrl/qcom/pinctrl-lpass-lpi.h        |  86 +++++++++
>   drivers/pinctrl/qcom/pinctrl-sm8250-lpass-lpi.c | 166 +++++++++++++++++
>   5 files changed, 266 insertions(+), 228 deletions(-)
>   create mode 100644 drivers/pinctrl/qcom/pinctrl-lpass-lpi.h
>   create mode 100644 drivers/pinctrl/qcom/pinctrl-sm8250-lpass-lpi.c
> 
> diff --git a/drivers/pinctrl/qcom/Kconfig b/drivers/pinctrl/qcom/Kconfig
> index ca6f68a..8871451 100644
> --- a/drivers/pinctrl/qcom/Kconfig
> +++ b/drivers/pinctrl/qcom/Kconfig
> @@ -357,4 +357,12 @@ config PINCTRL_LPASS_LPI
>   	  Qualcomm Technologies Inc LPASS (Low Power Audio SubSystem) LPI
>   	  (Low Power Island) found on the Qualcomm Technologies Inc SoCs.
>   
> +config PINCTRL_SM8250_LPASS_LPI
> +	tristate "Qualcomm Technologies Inc SM8250 LPASS LPI pin controller driver"
> +	depends on PINCTRL_LPASS_LPI

shouldn't this be select instead of depends.


> +	help
> +	  This is the pinctrl, pinmux, pinconf and gpiolib driver for the
> +	  Qualcomm Technologies Inc LPASS (Low Power Audio SubSystem) LPI
> +	  (Low Power Island) found on the Qualcomm Technologies Inc SM8250 platform.
> +
>   endif

Re: [PATCH v6 5/7] pinctrl: qcom: Extract chip specific LPASS LPI code

[Stephen Boyd]

Quoting Srinivasa Rao Mandadapu (2022-02-14 07:19:52)
> diff --git a/drivers/pinctrl/qcom/Kconfig b/drivers/pinctrl/qcom/Kconfig
> index ca6f68a..8871451 100644
> --- a/drivers/pinctrl/qcom/Kconfig
> +++ b/drivers/pinctrl/qcom/Kconfig
> @@ -357,4 +357,12 @@ config PINCTRL_LPASS_LPI
>           Qualcomm Technologies Inc LPASS (Low Power Audio SubSystem) LPI
>           (Low Power Island) found on the Qualcomm Technologies Inc SoCs.
>
> +config PINCTRL_SM8250_LPASS_LPI

Can this be sorted somehow? On Kconfig? Adding to the end of the file
causes conflicts more often than adding alphabetically.

> +       tristate "Qualcomm Technologies Inc SM8250 LPASS LPI pin controller driver"
> +       depends on PINCTRL_LPASS_LPI
> +       help
> +         This is the pinctrl, pinmux, pinconf and gpiolib driver for the
> +         Qualcomm Technologies Inc LPASS (Low Power Audio SubSystem) LPI
> +         (Low Power Island) found on the Qualcomm Technologies Inc SM8250 platform.
> +
>  endif
> diff --git a/drivers/pinctrl/qcom/Makefile b/drivers/pinctrl/qcom/Makefile
> index 709882f..c10d14d2 100644
> --- a/drivers/pinctrl/qcom/Makefile
> +++ b/drivers/pinctrl/qcom/Makefile
> @@ -41,3 +41,4 @@ obj-$(CONFIG_PINCTRL_SM8250) += pinctrl-sm8250.o
>  obj-$(CONFIG_PINCTRL_SM8350) += pinctrl-sm8350.o
>  obj-$(CONFIG_PINCTRL_SM8450) += pinctrl-sm8450.o
>  obj-$(CONFIG_PINCTRL_LPASS_LPI) += pinctrl-lpass-lpi.o
> +obj-$(CONFIG_PINCTRL_SM8250_LPASS_LPI) += pinctrl-sm8250-lpass-lpi.o

Can this be sorted somehow? Either on Kconfig or file name?

> diff --git a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
> index 54750ba..8a82fd9 100644
> --- a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
> +++ b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
> @@ -4,91 +4,15 @@
>   * Copyright (c) 2020 Linaro Ltd.
>   */
>
> -#include <linux/bitops.h>
> -#include <linux/bitfield.h>
>  #include <linux/clk.h>
>  #include <linux/gpio/driver.h>
> -#include <linux/io.h>
>  #include <linux/module.h>
>  #include <linux/of_device.h>
> -#include <linux/of.h>
>  #include <linux/pinctrl/pinconf-generic.h>
>  #include <linux/pinctrl/pinconf.h>
>  #include <linux/pinctrl/pinmux.h>
> -#include <linux/platform_device.h>
> -#include <linux/slab.h>
> -#include <linux/types.h>
> -#include "../core.h"
>  #include "../pinctrl-utils.h"
> -
> -#define LPI_SLEW_RATE_CTL_REG          0xa000
> -#define LPI_TLMM_REG_OFFSET            0x1000
> -#define LPI_SLEW_RATE_MAX              0x03
> -#define LPI_SLEW_BITS_SIZE             0x02
> -#define LPI_SLEW_RATE_MASK             GENMASK(1, 0)
> -#define LPI_GPIO_CFG_REG               0x00
> -#define LPI_GPIO_PULL_MASK             GENMASK(1, 0)
> -#define LPI_GPIO_FUNCTION_MASK         GENMASK(5, 2)
> -#define LPI_GPIO_OUT_STRENGTH_MASK     GENMASK(8, 6)
> -#define LPI_GPIO_OE_MASK               BIT(9)
> -#define LPI_GPIO_VALUE_REG             0x04
> -#define LPI_GPIO_VALUE_IN_MASK         BIT(0)
> -#define LPI_GPIO_VALUE_OUT_MASK                BIT(1)
> -
> -#define LPI_GPIO_BIAS_DISABLE          0x0
> -#define LPI_GPIO_PULL_DOWN             0x1
> -#define LPI_GPIO_KEEPER                        0x2
> -#define LPI_GPIO_PULL_UP               0x3
> -#define LPI_GPIO_DS_TO_VAL(v)          (v / 2 - 1)
> -#define LPI_NO_SLEW                            -1
> -
> -#define LPI_FUNCTION(fname)                                    \
> -       [LPI_MUX_##fname] = {                           \
> -               .name = #fname,                         \
> -               .groups = fname##_groups,               \
> -               .ngroups = ARRAY_SIZE(fname##_groups),  \
> -       }
> -
> -#define LPI_PINGROUP(id, soff, f1, f2, f3, f4)         \
> -       {                                               \
> -               .group.name = "gpio" #id,                       \
> -               .group.pins = gpio##id##_pins,          \
> -               .pin = id,                              \
> -               .slew_offset = soff,                    \
> -               .group.num_pins = ARRAY_SIZE(gpio##id##_pins),  \
> -               .funcs = (int[]){                       \
> -                       LPI_MUX_gpio,                   \
> -                       LPI_MUX_##f1,                   \
> -                       LPI_MUX_##f2,                   \
> -                       LPI_MUX_##f3,                   \
> -                       LPI_MUX_##f4,                   \
> -               },                                      \
> -               .nfuncs = 5,                            \
> -       }
> -
> -struct lpi_pingroup {
> -       struct group_desc group;
> -       unsigned int pin;
> -       /* Bit offset in slew register for SoundWire pins only */
> -       int slew_offset;
> -       unsigned int *funcs;
> -       unsigned int nfuncs;
> -};
> -
> -struct lpi_function {
> -       const char *name;
> -       const char * const *groups;
> -       unsigned int ngroups;
> -};
> -
> -struct lpi_pinctrl_variant_data {
> -       const struct pinctrl_pin_desc *pins;
> -       int npins;
> -       const struct lpi_pingroup *groups;
> -       int ngroups;
> -       const struct lpi_function *functions;
> -       int nfunctions;
> -};
> +#include "pinctrl-lpass-lpi.h"
>
>  #define MAX_LPI_NUM_CLKS       2
>
> @@ -104,136 +28,6 @@ struct lpi_pinctrl {
>         const struct lpi_pinctrl_variant_data *data;
>  };
>
> -/* sm8250 variant specific data */
> -static const struct pinctrl_pin_desc sm8250_lpi_pins[] = {
> -       PINCTRL_PIN(0, "gpio0"),
> -       PINCTRL_PIN(1, "gpio1"),
> -       PINCTRL_PIN(2, "gpio2"),
> -       PINCTRL_PIN(3, "gpio3"),
> -       PINCTRL_PIN(4, "gpio4"),
> -       PINCTRL_PIN(5, "gpio5"),
> -       PINCTRL_PIN(6, "gpio6"),
> -       PINCTRL_PIN(7, "gpio7"),
> -       PINCTRL_PIN(8, "gpio8"),
> -       PINCTRL_PIN(9, "gpio9"),
> -       PINCTRL_PIN(10, "gpio10"),
> -       PINCTRL_PIN(11, "gpio11"),
> -       PINCTRL_PIN(12, "gpio12"),
> -       PINCTRL_PIN(13, "gpio13"),
> -};
> -
> -enum sm8250_lpi_functions {
> -       LPI_MUX_dmic1_clk,
> -       LPI_MUX_dmic1_data,
> -       LPI_MUX_dmic2_clk,
> -       LPI_MUX_dmic2_data,
> -       LPI_MUX_dmic3_clk,
> -       LPI_MUX_dmic3_data,
> -       LPI_MUX_i2s1_clk,
> -       LPI_MUX_i2s1_data,
> -       LPI_MUX_i2s1_ws,
> -       LPI_MUX_i2s2_clk,
> -       LPI_MUX_i2s2_data,
> -       LPI_MUX_i2s2_ws,
> -       LPI_MUX_qua_mi2s_data,
> -       LPI_MUX_qua_mi2s_sclk,
> -       LPI_MUX_qua_mi2s_ws,
> -       LPI_MUX_swr_rx_clk,
> -       LPI_MUX_swr_rx_data,
> -       LPI_MUX_swr_tx_clk,
> -       LPI_MUX_swr_tx_data,
> -       LPI_MUX_wsa_swr_clk,
> -       LPI_MUX_wsa_swr_data,
> -       LPI_MUX_gpio,
> -       LPI_MUX__,
> -};
> -
> -static int gpio0_pins[] = { 0 };
> -static int gpio1_pins[] = { 1 };
> -static int gpio2_pins[] = { 2 };
> -static int gpio3_pins[] = { 3 };
> -static int gpio4_pins[] = { 4 };
> -static int gpio5_pins[] = { 5 };
> -static int gpio6_pins[] = { 6 };
> -static int gpio7_pins[] = { 7 };
> -static int gpio8_pins[] = { 8 };
> -static int gpio9_pins[] = { 9 };
> -static int gpio10_pins[] = { 10 };
> -static int gpio11_pins[] = { 11 };
> -static int gpio12_pins[] = { 12 };
> -static int gpio13_pins[] = { 13 };
> -static const char * const swr_tx_clk_groups[] = { "gpio0" };
> -static const char * const swr_tx_data_groups[] = { "gpio1", "gpio2", "gpio5" };
> -static const char * const swr_rx_clk_groups[] = { "gpio3" };
> -static const char * const swr_rx_data_groups[] = { "gpio4", "gpio5" };
> -static const char * const dmic1_clk_groups[] = { "gpio6" };
> -static const char * const dmic1_data_groups[] = { "gpio7" };
> -static const char * const dmic2_clk_groups[] = { "gpio8" };
> -static const char * const dmic2_data_groups[] = { "gpio9" };
> -static const char * const i2s2_clk_groups[] = { "gpio10" };
> -static const char * const i2s2_ws_groups[] = { "gpio11" };
> -static const char * const dmic3_clk_groups[] = { "gpio12" };
> -static const char * const dmic3_data_groups[] = { "gpio13" };
> -static const char * const qua_mi2s_sclk_groups[] = { "gpio0" };
> -static const char * const qua_mi2s_ws_groups[] = { "gpio1" };
> -static const char * const qua_mi2s_data_groups[] = { "gpio2", "gpio3", "gpio4" };
> -static const char * const i2s1_clk_groups[] = { "gpio6" };
> -static const char * const i2s1_ws_groups[] = { "gpio7" };
> -static const char * const i2s1_data_groups[] = { "gpio8", "gpio9" };
> -static const char * const wsa_swr_clk_groups[] = { "gpio10" };
> -static const char * const wsa_swr_data_groups[] = { "gpio11" };
> -static const char * const i2s2_data_groups[] = { "gpio12", "gpio12" };
> -
> -static const struct lpi_pingroup sm8250_groups[] = {
> -       LPI_PINGROUP(0, 0, swr_tx_clk, qua_mi2s_sclk, _, _),
> -       LPI_PINGROUP(1, 2, swr_tx_data, qua_mi2s_ws, _, _),
> -       LPI_PINGROUP(2, 4, swr_tx_data, qua_mi2s_data, _, _),
> -       LPI_PINGROUP(3, 8, swr_rx_clk, qua_mi2s_data, _, _),
> -       LPI_PINGROUP(4, 10, swr_rx_data, qua_mi2s_data, _, _),
> -       LPI_PINGROUP(5, 12, swr_tx_data, swr_rx_data, _, _),
> -       LPI_PINGROUP(6, LPI_NO_SLEW, dmic1_clk, i2s1_clk, _,  _),
> -       LPI_PINGROUP(7, LPI_NO_SLEW, dmic1_data, i2s1_ws, _, _),
> -       LPI_PINGROUP(8, LPI_NO_SLEW, dmic2_clk, i2s1_data, _, _),
> -       LPI_PINGROUP(9, LPI_NO_SLEW, dmic2_data, i2s1_data, _, _),
> -       LPI_PINGROUP(10, 16, i2s2_clk, wsa_swr_clk, _, _),
> -       LPI_PINGROUP(11, 18, i2s2_ws, wsa_swr_data, _, _),
> -       LPI_PINGROUP(12, LPI_NO_SLEW, dmic3_clk, i2s2_data, _, _),
> -       LPI_PINGROUP(13, LPI_NO_SLEW, dmic3_data, i2s2_data, _, _),
> -};
> -
> -static const struct lpi_function sm8250_functions[] = {
> -       LPI_FUNCTION(dmic1_clk),
> -       LPI_FUNCTION(dmic1_data),
> -       LPI_FUNCTION(dmic2_clk),
> -       LPI_FUNCTION(dmic2_data),
> -       LPI_FUNCTION(dmic3_clk),
> -       LPI_FUNCTION(dmic3_data),
> -       LPI_FUNCTION(i2s1_clk),
> -       LPI_FUNCTION(i2s1_data),
> -       LPI_FUNCTION(i2s1_ws),
> -       LPI_FUNCTION(i2s2_clk),
> -       LPI_FUNCTION(i2s2_data),
> -       LPI_FUNCTION(i2s2_ws),
> -       LPI_FUNCTION(qua_mi2s_data),
> -       LPI_FUNCTION(qua_mi2s_sclk),
> -       LPI_FUNCTION(qua_mi2s_ws),
> -       LPI_FUNCTION(swr_rx_clk),
> -       LPI_FUNCTION(swr_rx_data),
> -       LPI_FUNCTION(swr_tx_clk),
> -       LPI_FUNCTION(swr_tx_data),
> -       LPI_FUNCTION(wsa_swr_clk),
> -       LPI_FUNCTION(wsa_swr_data),
> -};
> -
> -static struct lpi_pinctrl_variant_data sm8250_lpi_data = {
> -       .pins = sm8250_lpi_pins,
> -       .npins = ARRAY_SIZE(sm8250_lpi_pins),
> -       .groups = sm8250_groups,
> -       .ngroups = ARRAY_SIZE(sm8250_groups),
> -       .functions = sm8250_functions,
> -       .nfunctions = ARRAY_SIZE(sm8250_functions),
> -};
> -
>  static int lpi_gpio_read(struct lpi_pinctrl *state, unsigned int pin,
>                          unsigned int addr)
>  {
> @@ -580,7 +374,7 @@ static const struct gpio_chip lpi_gpio_template = {
>         .dbg_show               = lpi_gpio_dbg_show,
>  };
>
> -static int lpi_pinctrl_probe(struct platform_device *pdev)
> +int lpi_pinctrl_probe(struct platform_device *pdev)
>  {
>         const struct lpi_pinctrl_variant_data *data;
>         struct device *dev = &pdev->dev;
> @@ -659,8 +453,9 @@ static int lpi_pinctrl_probe(struct platform_device *pdev)
>
>         return ret;
>  }
> +EXPORT_SYMBOL(lpi_pinctrl_probe);

EXPORT_SYMBOL_GPL?

>
> -static int lpi_pinctrl_remove(struct platform_device *pdev)
> +int lpi_pinctrl_remove(struct platform_device *pdev)
>  {
>         struct lpi_pinctrl *pctrl = platform_get_drvdata(pdev);
>
> @@ -669,25 +464,7 @@ static int lpi_pinctrl_remove(struct platform_device *pdev)
>
>         return 0;
>  }
> +EXPORT_SYMBOL(lpi_pinctrl_remove);

EXPORT_SYMBOL_GPL?

>
> -static const struct of_device_id lpi_pinctrl_of_match[] = {
> -       {
> -              .compatible = "qcom,sm8250-lpass-lpi-pinctrl",
> -              .data = &sm8250_lpi_data,
> -       },
> -       { }
> -};
> -MODULE_DEVICE_TABLE(of, lpi_pinctrl_of_match);
> diff --git a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.h b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.h
> new file mode 100644
> index 0000000..a511d72
> --- /dev/null
> +++ b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.h
> @@ -0,0 +1,86 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (c) 2016-2019, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2020 Linaro Ltd.
> + */
> +#ifndef __PINCTRL_LPASS_LPI_H__
> +#define __PINCTRL_LPASS_LPI_H__
> +
> +#include <linux/bitops.h>
> +#include <linux/bitfield.h>
> +#include "../core.h"
> +
> +#define LPI_SLEW_RATE_CTL_REG  0xa000
> +#define LPI_TLMM_REG_OFFSET            0x1000
> +#define LPI_SLEW_RATE_MAX              0x03
> +#define LPI_SLEW_BITS_SIZE             0x02
> +#define LPI_SLEW_RATE_MASK             GENMASK(1, 0)
> +#define LPI_GPIO_CFG_REG               0x00
> +#define LPI_GPIO_PULL_MASK             GENMASK(1, 0)
> +#define LPI_GPIO_FUNCTION_MASK         GENMASK(5, 2)
> +#define LPI_GPIO_OUT_STRENGTH_MASK     GENMASK(8, 6)
> +#define LPI_GPIO_OE_MASK               BIT(9)
> +#define LPI_GPIO_VALUE_REG             0x04
> +#define LPI_GPIO_VALUE_IN_MASK         BIT(0)
> +#define LPI_GPIO_VALUE_OUT_MASK                BIT(1)
> +
> +#define LPI_GPIO_BIAS_DISABLE          0x0
> +#define LPI_GPIO_PULL_DOWN             0x1
> +#define LPI_GPIO_KEEPER                        0x2
> +#define LPI_GPIO_PULL_UP               0x3
> +#define LPI_GPIO_DS_TO_VAL(v)          (v / 2 - 1)
> +#define LPI_NO_SLEW                            -1
> +
> +#define LPI_FUNCTION(fname)                                    \
> +       [LPI_MUX_##fname] = {                           \
> +               .name = #fname,                         \
> +               .groups = fname##_groups,               \
> +               .ngroups = ARRAY_SIZE(fname##_groups),  \
> +       }
> +
> +#define LPI_PINGROUP(id, soff, f1, f2, f3, f4)         \
> +       {                                               \
> +               .group.name = "gpio" #id,                       \
> +               .group.pins = gpio##id##_pins,          \
> +               .pin = id,                              \
> +               .slew_offset = soff,                    \
> +               .group.num_pins = ARRAY_SIZE(gpio##id##_pins),  \
> +               .funcs = (int[]){                       \
> +                       LPI_MUX_gpio,                   \
> +                       LPI_MUX_##f1,                   \
> +                       LPI_MUX_##f2,                   \
> +                       LPI_MUX_##f3,                   \
> +                       LPI_MUX_##f4,                   \
> +               },                                      \
> +               .nfuncs = 5,                            \
> +       }
> +
> +struct lpi_pingroup {
> +       struct group_desc group;
> +       unsigned int pin;
> +       /* Bit offset in slew register for SoundWire pins only */
> +       int slew_offset;
> +       unsigned int *funcs;
> +       unsigned int nfuncs;
> +};
> +
> +struct lpi_function {
> +       const char *name;
> +       const char * const *groups;
> +       unsigned int ngroups;
> +};
> +
> +struct lpi_pinctrl_variant_data {
> +       const struct pinctrl_pin_desc *pins;
> +       int npins;
> +       const struct lpi_pingroup *groups;
> +       int ngroups;
> +       const struct lpi_function *functions;
> +       int nfunctions;
> +};
> +
> +int lpi_pinctrl_probe(struct platform_device *pdev);
> +int lpi_pinctrl_remove(struct platform_device *pdev);
> +
> +#endif /*__PINCTRL_LPASS_LPI_H__*/
> +

Drop the extra newline on this file please.

> diff --git a/drivers/pinctrl/qcom/pinctrl-sm8250-lpass-lpi.c b/drivers/pinctrl/qcom/pinctrl-sm8250-lpass-lpi.c
> new file mode 100644
> index 0000000..27e358e
> --- /dev/null
> +++ b/drivers/pinctrl/qcom/pinctrl-sm8250-lpass-lpi.c
> @@ -0,0 +1,166 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2016-2019, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2020 Linaro Ltd.
> + */
> +
> +#include <linux/clk.h>

Drop unused include please.

> +#include <linux/gpio/driver.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +
> +#include "pinctrl-lpass-lpi.h"
> +
> +enum lpass_lpi_functions {
> +       LPI_MUX_dmic1_clk,
> +       LPI_MUX_dmic1_data,
> +       LPI_MUX_dmic2_clk,
> +       LPI_MUX_dmic2_data,
> +       LPI_MUX_dmic3_clk,
> +       LPI_MUX_dmic3_data,
> +       LPI_MUX_i2s1_clk,
> +       LPI_MUX_i2s1_data,
> +       LPI_MUX_i2s1_ws,
> +       LPI_MUX_i2s2_clk,
> +       LPI_MUX_i2s2_data,
> +       LPI_MUX_i2s2_ws,
> +       LPI_MUX_qua_mi2s_data,
> +       LPI_MUX_qua_mi2s_sclk,
> +       LPI_MUX_qua_mi2s_ws,
> +       LPI_MUX_swr_rx_clk,
> +       LPI_MUX_swr_rx_data,
> +       LPI_MUX_swr_tx_clk,
> +       LPI_MUX_swr_tx_data,
> +       LPI_MUX_wsa_swr_clk,
> +       LPI_MUX_wsa_swr_data,
> +       LPI_MUX_gpio,
> +       LPI_MUX__,
> +};
> +
> +static int gpio0_pins[] = { 0 };

Why not const?

> +static int gpio1_pins[] = { 1 };
> +static int gpio2_pins[] = { 2 };
> +static int gpio3_pins[] = { 3 };
> +static int gpio4_pins[] = { 4 };
> +static int gpio5_pins[] = { 5 };
> +static int gpio6_pins[] = { 6 };
> +static int gpio7_pins[] = { 7 };
> +static int gpio8_pins[] = { 8 };
> +static int gpio9_pins[] = { 9 };
> +static int gpio10_pins[] = { 10 };
> +static int gpio11_pins[] = { 11 };
> +static int gpio12_pins[] = { 12 };
> +static int gpio13_pins[] = { 13 };
> +
> +
> +/* sm8250 variant specific data */
> +static const struct pinctrl_pin_desc sm8250_lpi_pins[] = {
> +       PINCTRL_PIN(0, "gpio0"),
> +       PINCTRL_PIN(1, "gpio1"),
> +       PINCTRL_PIN(2, "gpio2"),
> +       PINCTRL_PIN(3, "gpio3"),
> +       PINCTRL_PIN(4, "gpio4"),
> +       PINCTRL_PIN(5, "gpio5"),
> +       PINCTRL_PIN(6, "gpio6"),
> +       PINCTRL_PIN(7, "gpio7"),
> +       PINCTRL_PIN(8, "gpio8"),
> +       PINCTRL_PIN(9, "gpio9"),
> +       PINCTRL_PIN(10, "gpio10"),
> +       PINCTRL_PIN(11, "gpio11"),
> +       PINCTRL_PIN(12, "gpio12"),
> +       PINCTRL_PIN(13, "gpio13"),
> +};
> +
> +static const char * const swr_tx_clk_groups[] = { "gpio0" };
> +static const char * const swr_tx_data_groups[] = { "gpio1", "gpio2", "gpio5" };
> +static const char * const swr_rx_clk_groups[] = { "gpio3" };
> +static const char * const swr_rx_data_groups[] = { "gpio4", "gpio5" };
> +static const char * const dmic1_clk_groups[] = { "gpio6" };
> +static const char * const dmic1_data_groups[] = { "gpio7" };
> +static const char * const dmic2_clk_groups[] = { "gpio8" };
> +static const char * const dmic2_data_groups[] = { "gpio9" };
> +static const char * const i2s2_clk_groups[] = { "gpio10" };
> +static const char * const i2s2_ws_groups[] = { "gpio11" };
> +static const char * const dmic3_clk_groups[] = { "gpio12" };
> +static const char * const dmic3_data_groups[] = { "gpio13" };
> +static const char * const qua_mi2s_sclk_groups[] = { "gpio0" };
> +static const char * const qua_mi2s_ws_groups[] = { "gpio1" };
> +static const char * const qua_mi2s_data_groups[] = { "gpio2", "gpio3", "gpio4" };
> +static const char * const i2s1_clk_groups[] = { "gpio6" };
> +static const char * const i2s1_ws_groups[] = { "gpio7" };
> +static const char * const i2s1_data_groups[] = { "gpio8", "gpio9" };
> +static const char * const wsa_swr_clk_groups[] = { "gpio10" };
> +static const char * const wsa_swr_data_groups[] = { "gpio11" };
> +static const char * const i2s2_data_groups[] = { "gpio12", "gpio12" };
> +
> +static const struct lpi_pingroup sm8250_groups[] = {
> +       LPI_PINGROUP(0, 0, swr_tx_clk, qua_mi2s_sclk, _, _),
> +       LPI_PINGROUP(1, 2, swr_tx_data, qua_mi2s_ws, _, _),
> +       LPI_PINGROUP(2, 4, swr_tx_data, qua_mi2s_data, _, _),
> +       LPI_PINGROUP(3, 8, swr_rx_clk, qua_mi2s_data, _, _),
> +       LPI_PINGROUP(4, 10, swr_rx_data, qua_mi2s_data, _, _),
> +       LPI_PINGROUP(5, 12, swr_tx_data, swr_rx_data, _, _),
> +       LPI_PINGROUP(6, LPI_NO_SLEW, dmic1_clk, i2s1_clk, _,  _),
> +       LPI_PINGROUP(7, LPI_NO_SLEW, dmic1_data, i2s1_ws, _, _),
> +       LPI_PINGROUP(8, LPI_NO_SLEW, dmic2_clk, i2s1_data, _, _),
> +       LPI_PINGROUP(9, LPI_NO_SLEW, dmic2_data, i2s1_data, _, _),
> +       LPI_PINGROUP(10, 16, i2s2_clk, wsa_swr_clk, _, _),
> +       LPI_PINGROUP(11, 18, i2s2_ws, wsa_swr_data, _, _),
> +       LPI_PINGROUP(12, LPI_NO_SLEW, dmic3_clk, i2s2_data, _, _),
> +       LPI_PINGROUP(13, LPI_NO_SLEW, dmic3_data, i2s2_data, _, _),
> +};
> +
> +static const struct lpi_function sm8250_functions[] = {
> +       LPI_FUNCTION(dmic1_clk),
> +       LPI_FUNCTION(dmic1_data),
> +       LPI_FUNCTION(dmic2_clk),
> +       LPI_FUNCTION(dmic2_data),
> +       LPI_FUNCTION(dmic3_clk),
> +       LPI_FUNCTION(dmic3_data),
> +       LPI_FUNCTION(i2s1_clk),
> +       LPI_FUNCTION(i2s1_data),
> +       LPI_FUNCTION(i2s1_ws),
> +       LPI_FUNCTION(i2s2_clk),
> +       LPI_FUNCTION(i2s2_data),
> +       LPI_FUNCTION(i2s2_ws),
> +       LPI_FUNCTION(qua_mi2s_data),
> +       LPI_FUNCTION(qua_mi2s_sclk),
> +       LPI_FUNCTION(qua_mi2s_ws),
> +       LPI_FUNCTION(swr_rx_clk),
> +       LPI_FUNCTION(swr_rx_data),
> +       LPI_FUNCTION(swr_tx_clk),
> +       LPI_FUNCTION(swr_tx_data),
> +       LPI_FUNCTION(wsa_swr_clk),
> +       LPI_FUNCTION(wsa_swr_data),
> +};
> +
> +static struct lpi_pinctrl_variant_data sm8250_lpi_data = {

It would be great if this could be const.

> +       .pins = sm8250_lpi_pins,
> +       .npins = ARRAY_SIZE(sm8250_lpi_pins),
> +       .groups = sm8250_groups,
> +       .ngroups = ARRAY_SIZE(sm8250_groups),
> +       .functions = sm8250_functions,
> +       .nfunctions = ARRAY_SIZE(sm8250_functions),
> +};
> +

Re: [PATCH v6 5/7] pinctrl: qcom: Extract chip specific LPASS LPI code

[Srinivasa Rao Mandadapu (Temp)]

On 2/19/2022 8:12 AM, Stephen Boyd wrote:
Thanks for Your time Stephen!!!
> Quoting Srinivasa Rao Mandadapu (2022-02-14 07:19:52)
>> diff --git a/drivers/pinctrl/qcom/Kconfig b/drivers/pinctrl/qcom/Kconfig
>> index ca6f68a..8871451 100644
>> --- a/drivers/pinctrl/qcom/Kconfig
>> +++ b/drivers/pinctrl/qcom/Kconfig
>> @@ -357,4 +357,12 @@ config PINCTRL_LPASS_LPI
>>            Qualcomm Technologies Inc LPASS (Low Power Audio SubSystem) LPI
>>            (Low Power Island) found on the Qualcomm Technologies Inc SoCs.
>>
>> +config PINCTRL_SM8250_LPASS_LPI
> Can this be sorted somehow? On Kconfig? Adding to the end of the file
> causes conflicts more often than adding alphabetically.
Okay. Will sort it and re post it.
>
>> +       tristate "Qualcomm Technologies Inc SM8250 LPASS LPI pin controller driver"
>> +       depends on PINCTRL_LPASS_LPI
>> +       help
>> +         This is the pinctrl, pinmux, pinconf and gpiolib driver for the
>> +         Qualcomm Technologies Inc LPASS (Low Power Audio SubSystem) LPI
>> +         (Low Power Island) found on the Qualcomm Technologies Inc SM8250 platform.
>> +
>>   endif
>> diff --git a/drivers/pinctrl/qcom/Makefile b/drivers/pinctrl/qcom/Makefile
>> index 709882f..c10d14d2 100644
>> --- a/drivers/pinctrl/qcom/Makefile
>> +++ b/drivers/pinctrl/qcom/Makefile
>> @@ -41,3 +41,4 @@ obj-$(CONFIG_PINCTRL_SM8250) += pinctrl-sm8250.o
>>   obj-$(CONFIG_PINCTRL_SM8350) += pinctrl-sm8350.o
>>   obj-$(CONFIG_PINCTRL_SM8450) += pinctrl-sm8450.o
>>   obj-$(CONFIG_PINCTRL_LPASS_LPI) += pinctrl-lpass-lpi.o
>> +obj-$(CONFIG_PINCTRL_SM8250_LPASS_LPI) += pinctrl-sm8250-lpass-lpi.o
> Can this be sorted somehow? Either on Kconfig or file name?
Okay.
>
>> diff --git a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
>> index 54750ba..8a82fd9 100644
>> --- a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
>> +++ b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
>> @@ -4,91 +4,15 @@
>>    * Copyright (c) 2020 Linaro Ltd.
>>    */
>>
>> -#include <linux/bitops.h>
>> -#include <linux/bitfield.h>
>>   #include <linux/clk.h>
>>   #include <linux/gpio/driver.h>
>> -#include <linux/io.h>
>>   #include <linux/module.h>
>>   #include <linux/of_device.h>
>> -#include <linux/of.h>
>>   #include <linux/pinctrl/pinconf-generic.h>
>>   #include <linux/pinctrl/pinconf.h>
>>   #include <linux/pinctrl/pinmux.h>
>> -#include <linux/platform_device.h>
>> -#include <linux/slab.h>
>> -#include <linux/types.h>
>> -#include "../core.h"
>>   #include "../pinctrl-utils.h"
>> -
>> -#define LPI_SLEW_RATE_CTL_REG          0xa000
>> -#define LPI_TLMM_REG_OFFSET            0x1000
>> -#define LPI_SLEW_RATE_MAX              0x03
>> -#define LPI_SLEW_BITS_SIZE             0x02
>> -#define LPI_SLEW_RATE_MASK             GENMASK(1, 0)
>> -#define LPI_GPIO_CFG_REG               0x00
>> -#define LPI_GPIO_PULL_MASK             GENMASK(1, 0)
>> -#define LPI_GPIO_FUNCTION_MASK         GENMASK(5, 2)
>> -#define LPI_GPIO_OUT_STRENGTH_MASK     GENMASK(8, 6)
>> -#define LPI_GPIO_OE_MASK               BIT(9)
>> -#define LPI_GPIO_VALUE_REG             0x04
>> -#define LPI_GPIO_VALUE_IN_MASK         BIT(0)
>> -#define LPI_GPIO_VALUE_OUT_MASK                BIT(1)
>> -
>> -#define LPI_GPIO_BIAS_DISABLE          0x0
>> -#define LPI_GPIO_PULL_DOWN             0x1
>> -#define LPI_GPIO_KEEPER                        0x2
>> -#define LPI_GPIO_PULL_UP               0x3
>> -#define LPI_GPIO_DS_TO_VAL(v)          (v / 2 - 1)
>> -#define LPI_NO_SLEW                            -1
>> -
>> -#define LPI_FUNCTION(fname)                                    \
>> -       [LPI_MUX_##fname] = {                           \
>> -               .name = #fname,                         \
>> -               .groups = fname##_groups,               \
>> -               .ngroups = ARRAY_SIZE(fname##_groups),  \
>> -       }
>> -
>> -#define LPI_PINGROUP(id, soff, f1, f2, f3, f4)         \
>> -       {                                               \
>> -               .group.name = "gpio" #id,                       \
>> -               .group.pins = gpio##id##_pins,          \
>> -               .pin = id,                              \
>> -               .slew_offset = soff,                    \
>> -               .group.num_pins = ARRAY_SIZE(gpio##id##_pins),  \
>> -               .funcs = (int[]){                       \
>> -                       LPI_MUX_gpio,                   \
>> -                       LPI_MUX_##f1,                   \
>> -                       LPI_MUX_##f2,                   \
>> -                       LPI_MUX_##f3,                   \
>> -                       LPI_MUX_##f4,                   \
>> -               },                                      \
>> -               .nfuncs = 5,                            \
>> -       }
>> -
>> -struct lpi_pingroup {
>> -       struct group_desc group;
>> -       unsigned int pin;
>> -       /* Bit offset in slew register for SoundWire pins only */
>> -       int slew_offset;
>> -       unsigned int *funcs;
>> -       unsigned int nfuncs;
>> -};
>> -
>> -struct lpi_function {
>> -       const char *name;
>> -       const char * const *groups;
>> -       unsigned int ngroups;
>> -};
>> -
>> -struct lpi_pinctrl_variant_data {
>> -       const struct pinctrl_pin_desc *pins;
>> -       int npins;
>> -       const struct lpi_pingroup *groups;
>> -       int ngroups;
>> -       const struct lpi_function *functions;
>> -       int nfunctions;
>> -};
>> +#include "pinctrl-lpass-lpi.h"
>>
>>   #define MAX_LPI_NUM_CLKS       2
>>
>> @@ -104,136 +28,6 @@ struct lpi_pinctrl {
>>          const struct lpi_pinctrl_variant_data *data;
>>   };
>>
>> -/* sm8250 variant specific data */
>> -static const struct pinctrl_pin_desc sm8250_lpi_pins[] = {
>> -       PINCTRL_PIN(0, "gpio0"),
>> -       PINCTRL_PIN(1, "gpio1"),
>> -       PINCTRL_PIN(2, "gpio2"),
>> -       PINCTRL_PIN(3, "gpio3"),
>> -       PINCTRL_PIN(4, "gpio4"),
>> -       PINCTRL_PIN(5, "gpio5"),
>> -       PINCTRL_PIN(6, "gpio6"),
>> -       PINCTRL_PIN(7, "gpio7"),
>> -       PINCTRL_PIN(8, "gpio8"),
>> -       PINCTRL_PIN(9, "gpio9"),
>> -       PINCTRL_PIN(10, "gpio10"),
>> -       PINCTRL_PIN(11, "gpio11"),
>> -       PINCTRL_PIN(12, "gpio12"),
>> -       PINCTRL_PIN(13, "gpio13"),
>> -};
>> -
>> -enum sm8250_lpi_functions {
>> -       LPI_MUX_dmic1_clk,
>> -       LPI_MUX_dmic1_data,
>> -       LPI_MUX_dmic2_clk,
>> -       LPI_MUX_dmic2_data,
>> -       LPI_MUX_dmic3_clk,
>> -       LPI_MUX_dmic3_data,
>> -       LPI_MUX_i2s1_clk,
>> -       LPI_MUX_i2s1_data,
>> -       LPI_MUX_i2s1_ws,
>> -       LPI_MUX_i2s2_clk,
>> -       LPI_MUX_i2s2_data,
>> -       LPI_MUX_i2s2_ws,
>> -       LPI_MUX_qua_mi2s_data,
>> -       LPI_MUX_qua_mi2s_sclk,
>> -       LPI_MUX_qua_mi2s_ws,
>> -       LPI_MUX_swr_rx_clk,
>> -       LPI_MUX_swr_rx_data,
>> -       LPI_MUX_swr_tx_clk,
>> -       LPI_MUX_swr_tx_data,
>> -       LPI_MUX_wsa_swr_clk,
>> -       LPI_MUX_wsa_swr_data,
>> -       LPI_MUX_gpio,
>> -       LPI_MUX__,
>> -};
>> -
>> -static int gpio0_pins[] = { 0 };
>> -static int gpio1_pins[] = { 1 };
>> -static int gpio2_pins[] = { 2 };
>> -static int gpio3_pins[] = { 3 };
>> -static int gpio4_pins[] = { 4 };
>> -static int gpio5_pins[] = { 5 };
>> -static int gpio6_pins[] = { 6 };
>> -static int gpio7_pins[] = { 7 };
>> -static int gpio8_pins[] = { 8 };
>> -static int gpio9_pins[] = { 9 };
>> -static int gpio10_pins[] = { 10 };
>> -static int gpio11_pins[] = { 11 };
>> -static int gpio12_pins[] = { 12 };
>> -static int gpio13_pins[] = { 13 };
>> -static const char * const swr_tx_clk_groups[] = { "gpio0" };
>> -static const char * const swr_tx_data_groups[] = { "gpio1", "gpio2", "gpio5" };
>> -static const char * const swr_rx_clk_groups[] = { "gpio3" };
>> -static const char * const swr_rx_data_groups[] = { "gpio4", "gpio5" };
>> -static const char * const dmic1_clk_groups[] = { "gpio6" };
>> -static const char * const dmic1_data_groups[] = { "gpio7" };
>> -static const char * const dmic2_clk_groups[] = { "gpio8" };
>> -static const char * const dmic2_data_groups[] = { "gpio9" };
>> -static const char * const i2s2_clk_groups[] = { "gpio10" };
>> -static const char * const i2s2_ws_groups[] = { "gpio11" };
>> -static const char * const dmic3_clk_groups[] = { "gpio12" };
>> -static const char * const dmic3_data_groups[] = { "gpio13" };
>> -static const char * const qua_mi2s_sclk_groups[] = { "gpio0" };
>> -static const char * const qua_mi2s_ws_groups[] = { "gpio1" };
>> -static const char * const qua_mi2s_data_groups[] = { "gpio2", "gpio3", "gpio4" };
>> -static const char * const i2s1_clk_groups[] = { "gpio6" };
>> -static const char * const i2s1_ws_groups[] = { "gpio7" };
>> -static const char * const i2s1_data_groups[] = { "gpio8", "gpio9" };
>> -static const char * const wsa_swr_clk_groups[] = { "gpio10" };
>> -static const char * const wsa_swr_data_groups[] = { "gpio11" };
>> -static const char * const i2s2_data_groups[] = { "gpio12", "gpio12" };
>> -
>> -static const struct lpi_pingroup sm8250_groups[] = {
>> -       LPI_PINGROUP(0, 0, swr_tx_clk, qua_mi2s_sclk, _, _),
>> -       LPI_PINGROUP(1, 2, swr_tx_data, qua_mi2s_ws, _, _),
>> -       LPI_PINGROUP(2, 4, swr_tx_data, qua_mi2s_data, _, _),
>> -       LPI_PINGROUP(3, 8, swr_rx_clk, qua_mi2s_data, _, _),
>> -       LPI_PINGROUP(4, 10, swr_rx_data, qua_mi2s_data, _, _),
>> -       LPI_PINGROUP(5, 12, swr_tx_data, swr_rx_data, _, _),
>> -       LPI_PINGROUP(6, LPI_NO_SLEW, dmic1_clk, i2s1_clk, _,  _),
>> -       LPI_PINGROUP(7, LPI_NO_SLEW, dmic1_data, i2s1_ws, _, _),
>> -       LPI_PINGROUP(8, LPI_NO_SLEW, dmic2_clk, i2s1_data, _, _),
>> -       LPI_PINGROUP(9, LPI_NO_SLEW, dmic2_data, i2s1_data, _, _),
>> -       LPI_PINGROUP(10, 16, i2s2_clk, wsa_swr_clk, _, _),
>> -       LPI_PINGROUP(11, 18, i2s2_ws, wsa_swr_data, _, _),
>> -       LPI_PINGROUP(12, LPI_NO_SLEW, dmic3_clk, i2s2_data, _, _),
>> -       LPI_PINGROUP(13, LPI_NO_SLEW, dmic3_data, i2s2_data, _, _),
>> -};
>> -
>> -static const struct lpi_function sm8250_functions[] = {
>> -       LPI_FUNCTION(dmic1_clk),
>> -       LPI_FUNCTION(dmic1_data),
>> -       LPI_FUNCTION(dmic2_clk),
>> -       LPI_FUNCTION(dmic2_data),
>> -       LPI_FUNCTION(dmic3_clk),
>> -       LPI_FUNCTION(dmic3_data),
>> -       LPI_FUNCTION(i2s1_clk),
>> -       LPI_FUNCTION(i2s1_data),
>> -       LPI_FUNCTION(i2s1_ws),
>> -       LPI_FUNCTION(i2s2_clk),
>> -       LPI_FUNCTION(i2s2_data),
>> -       LPI_FUNCTION(i2s2_ws),
>> -       LPI_FUNCTION(qua_mi2s_data),
>> -       LPI_FUNCTION(qua_mi2s_sclk),
>> -       LPI_FUNCTION(qua_mi2s_ws),
>> -       LPI_FUNCTION(swr_rx_clk),
>> -       LPI_FUNCTION(swr_rx_data),
>> -       LPI_FUNCTION(swr_tx_clk),
>> -       LPI_FUNCTION(swr_tx_data),
>> -       LPI_FUNCTION(wsa_swr_clk),
>> -       LPI_FUNCTION(wsa_swr_data),
>> -};
>> -
>> -static struct lpi_pinctrl_variant_data sm8250_lpi_data = {
>> -       .pins = sm8250_lpi_pins,
>> -       .npins = ARRAY_SIZE(sm8250_lpi_pins),
>> -       .groups = sm8250_groups,
>> -       .ngroups = ARRAY_SIZE(sm8250_groups),
>> -       .functions = sm8250_functions,
>> -       .nfunctions = ARRAY_SIZE(sm8250_functions),
>> -};
>> -
>>   static int lpi_gpio_read(struct lpi_pinctrl *state, unsigned int pin,
>>                           unsigned int addr)
>>   {
>> @@ -580,7 +374,7 @@ static const struct gpio_chip lpi_gpio_template = {
>>          .dbg_show               = lpi_gpio_dbg_show,
>>   };
>>
>> -static int lpi_pinctrl_probe(struct platform_device *pdev)
>> +int lpi_pinctrl_probe(struct platform_device *pdev)
>>   {
>>          const struct lpi_pinctrl_variant_data *data;
>>          struct device *dev = &pdev->dev;
>> @@ -659,8 +453,9 @@ static int lpi_pinctrl_probe(struct platform_device *pdev)
>>
>>          return ret;
>>   }
>> +EXPORT_SYMBOL(lpi_pinctrl_probe);
> EXPORT_SYMBOL_GPL?
Okay. will change it.
>
>> -static int lpi_pinctrl_remove(struct platform_device *pdev)
>> +int lpi_pinctrl_remove(struct platform_device *pdev)
>>   {
>>          struct lpi_pinctrl *pctrl = platform_get_drvdata(pdev);
>>
>> @@ -669,25 +464,7 @@ static int lpi_pinctrl_remove(struct platform_device *pdev)
>>
>>          return 0;
>>   }
>> +EXPORT_SYMBOL(lpi_pinctrl_remove);
> EXPORT_SYMBOL_GPL?
Okay. will change it.
>
>> -static const struct of_device_id lpi_pinctrl_of_match[] = {
>> -       {
>> -              .compatible = "qcom,sm8250-lpass-lpi-pinctrl",
>> -              .data = &sm8250_lpi_data,
>> -       },
>> -       { }
>> -};
>> -MODULE_DEVICE_TABLE(of, lpi_pinctrl_of_match);
>> diff --git a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.h b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.h
>> new file mode 100644
>> index 0000000..a511d72
>> --- /dev/null
>> +++ b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.h
>> @@ -0,0 +1,86 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +/*
>> + * Copyright (c) 2016-2019, The Linux Foundation. All rights reserved.
>> + * Copyright (c) 2020 Linaro Ltd.
>> + */
>> +#ifndef __PINCTRL_LPASS_LPI_H__
>> +#define __PINCTRL_LPASS_LPI_H__
>> +
>> +#include <linux/bitops.h>
>> +#include <linux/bitfield.h>
>> +#include "../core.h"
>> +
>> +#define LPI_SLEW_RATE_CTL_REG  0xa000
>> +#define LPI_TLMM_REG_OFFSET            0x1000
>> +#define LPI_SLEW_RATE_MAX              0x03
>> +#define LPI_SLEW_BITS_SIZE             0x02
>> +#define LPI_SLEW_RATE_MASK             GENMASK(1, 0)
>> +#define LPI_GPIO_CFG_REG               0x00
>> +#define LPI_GPIO_PULL_MASK             GENMASK(1, 0)
>> +#define LPI_GPIO_FUNCTION_MASK         GENMASK(5, 2)
>> +#define LPI_GPIO_OUT_STRENGTH_MASK     GENMASK(8, 6)
>> +#define LPI_GPIO_OE_MASK               BIT(9)
>> +#define LPI_GPIO_VALUE_REG             0x04
>> +#define LPI_GPIO_VALUE_IN_MASK         BIT(0)
>> +#define LPI_GPIO_VALUE_OUT_MASK                BIT(1)
>> +
>> +#define LPI_GPIO_BIAS_DISABLE          0x0
>> +#define LPI_GPIO_PULL_DOWN             0x1
>> +#define LPI_GPIO_KEEPER                        0x2
>> +#define LPI_GPIO_PULL_UP               0x3
>> +#define LPI_GPIO_DS_TO_VAL(v)          (v / 2 - 1)
>> +#define LPI_NO_SLEW                            -1
>> +
>> +#define LPI_FUNCTION(fname)                                    \
>> +       [LPI_MUX_##fname] = {                           \
>> +               .name = #fname,                         \
>> +               .groups = fname##_groups,               \
>> +               .ngroups = ARRAY_SIZE(fname##_groups),  \
>> +       }
>> +
>> +#define LPI_PINGROUP(id, soff, f1, f2, f3, f4)         \
>> +       {                                               \
>> +               .group.name = "gpio" #id,                       \
>> +               .group.pins = gpio##id##_pins,          \
>> +               .pin = id,                              \
>> +               .slew_offset = soff,                    \
>> +               .group.num_pins = ARRAY_SIZE(gpio##id##_pins),  \
>> +               .funcs = (int[]){                       \
>> +                       LPI_MUX_gpio,                   \
>> +                       LPI_MUX_##f1,                   \
>> +                       LPI_MUX_##f2,                   \
>> +                       LPI_MUX_##f3,                   \
>> +                       LPI_MUX_##f4,                   \
>> +               },                                      \
>> +               .nfuncs = 5,                            \
>> +       }
>> +
>> +struct lpi_pingroup {
>> +       struct group_desc group;
>> +       unsigned int pin;
>> +       /* Bit offset in slew register for SoundWire pins only */
>> +       int slew_offset;
>> +       unsigned int *funcs;
>> +       unsigned int nfuncs;
>> +};
>> +
>> +struct lpi_function {
>> +       const char *name;
>> +       const char * const *groups;
>> +       unsigned int ngroups;
>> +};
>> +
>> +struct lpi_pinctrl_variant_data {
>> +       const struct pinctrl_pin_desc *pins;
>> +       int npins;
>> +       const struct lpi_pingroup *groups;
>> +       int ngroups;
>> +       const struct lpi_function *functions;
>> +       int nfunctions;
>> +};
>> +
>> +int lpi_pinctrl_probe(struct platform_device *pdev);
>> +int lpi_pinctrl_remove(struct platform_device *pdev);
>> +
>> +#endif /*__PINCTRL_LPASS_LPI_H__*/
>> +
> Drop the extra newline on this file please.
Okay. will remove it.
>
>> diff --git a/drivers/pinctrl/qcom/pinctrl-sm8250-lpass-lpi.c b/drivers/pinctrl/qcom/pinctrl-sm8250-lpass-lpi.c
>> new file mode 100644
>> index 0000000..27e358e
>> --- /dev/null
>> +++ b/drivers/pinctrl/qcom/pinctrl-sm8250-lpass-lpi.c
>> @@ -0,0 +1,166 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright (c) 2016-2019, The Linux Foundation. All rights reserved.
>> + * Copyright (c) 2020 Linaro Ltd.
>> + */
>> +
>> +#include <linux/clk.h>
> Drop unused include please.
Okay.
>
>> +#include <linux/gpio/driver.h>
>> +#include <linux/module.h>
>> +#include <linux/platform_device.h>
>> +
>> +#include "pinctrl-lpass-lpi.h"
>> +
>> +enum lpass_lpi_functions {
>> +       LPI_MUX_dmic1_clk,
>> +       LPI_MUX_dmic1_data,
>> +       LPI_MUX_dmic2_clk,
>> +       LPI_MUX_dmic2_data,
>> +       LPI_MUX_dmic3_clk,
>> +       LPI_MUX_dmic3_data,
>> +       LPI_MUX_i2s1_clk,
>> +       LPI_MUX_i2s1_data,
>> +       LPI_MUX_i2s1_ws,
>> +       LPI_MUX_i2s2_clk,
>> +       LPI_MUX_i2s2_data,
>> +       LPI_MUX_i2s2_ws,
>> +       LPI_MUX_qua_mi2s_data,
>> +       LPI_MUX_qua_mi2s_sclk,
>> +       LPI_MUX_qua_mi2s_ws,
>> +       LPI_MUX_swr_rx_clk,
>> +       LPI_MUX_swr_rx_data,
>> +       LPI_MUX_swr_tx_clk,
>> +       LPI_MUX_swr_tx_data,
>> +       LPI_MUX_wsa_swr_clk,
>> +       LPI_MUX_wsa_swr_data,
>> +       LPI_MUX_gpio,
>> +       LPI_MUX__,
>> +};
>> +
>> +static int gpio0_pins[] = { 0 };
> Why not const?
It's creating conflict with group_desc structure params.
>
>> +static int gpio1_pins[] = { 1 };
>> +static int gpio2_pins[] = { 2 };
>> +static int gpio3_pins[] = { 3 };
>> +static int gpio4_pins[] = { 4 };
>> +static int gpio5_pins[] = { 5 };
>> +static int gpio6_pins[] = { 6 };
>> +static int gpio7_pins[] = { 7 };
>> +static int gpio8_pins[] = { 8 };
>> +static int gpio9_pins[] = { 9 };
>> +static int gpio10_pins[] = { 10 };
>> +static int gpio11_pins[] = { 11 };
>> +static int gpio12_pins[] = { 12 };
>> +static int gpio13_pins[] = { 13 };
>> +
>> +
>> +/* sm8250 variant specific data */
>> +static const struct pinctrl_pin_desc sm8250_lpi_pins[] = {
>> +       PINCTRL_PIN(0, "gpio0"),
>> +       PINCTRL_PIN(1, "gpio1"),
>> +       PINCTRL_PIN(2, "gpio2"),
>> +       PINCTRL_PIN(3, "gpio3"),
>> +       PINCTRL_PIN(4, "gpio4"),
>> +       PINCTRL_PIN(5, "gpio5"),
>> +       PINCTRL_PIN(6, "gpio6"),
>> +       PINCTRL_PIN(7, "gpio7"),
>> +       PINCTRL_PIN(8, "gpio8"),
>> +       PINCTRL_PIN(9, "gpio9"),
>> +       PINCTRL_PIN(10, "gpio10"),
>> +       PINCTRL_PIN(11, "gpio11"),
>> +       PINCTRL_PIN(12, "gpio12"),
>> +       PINCTRL_PIN(13, "gpio13"),
>> +};
>> +
>> +static const char * const swr_tx_clk_groups[] = { "gpio0" };
>> +static const char * const swr_tx_data_groups[] = { "gpio1", "gpio2", "gpio5" };
>> +static const char * const swr_rx_clk_groups[] = { "gpio3" };
>> +static const char * const swr_rx_data_groups[] = { "gpio4", "gpio5" };
>> +static const char * const dmic1_clk_groups[] = { "gpio6" };
>> +static const char * const dmic1_data_groups[] = { "gpio7" };
>> +static const char * const dmic2_clk_groups[] = { "gpio8" };
>> +static const char * const dmic2_data_groups[] = { "gpio9" };
>> +static const char * const i2s2_clk_groups[] = { "gpio10" };
>> +static const char * const i2s2_ws_groups[] = { "gpio11" };
>> +static const char * const dmic3_clk_groups[] = { "gpio12" };
>> +static const char * const dmic3_data_groups[] = { "gpio13" };
>> +static const char * const qua_mi2s_sclk_groups[] = { "gpio0" };
>> +static const char * const qua_mi2s_ws_groups[] = { "gpio1" };
>> +static const char * const qua_mi2s_data_groups[] = { "gpio2", "gpio3", "gpio4" };
>> +static const char * const i2s1_clk_groups[] = { "gpio6" };
>> +static const char * const i2s1_ws_groups[] = { "gpio7" };
>> +static const char * const i2s1_data_groups[] = { "gpio8", "gpio9" };
>> +static const char * const wsa_swr_clk_groups[] = { "gpio10" };
>> +static const char * const wsa_swr_data_groups[] = { "gpio11" };
>> +static const char * const i2s2_data_groups[] = { "gpio12", "gpio12" };
>> +
>> +static const struct lpi_pingroup sm8250_groups[] = {
>> +       LPI_PINGROUP(0, 0, swr_tx_clk, qua_mi2s_sclk, _, _),
>> +       LPI_PINGROUP(1, 2, swr_tx_data, qua_mi2s_ws, _, _),
>> +       LPI_PINGROUP(2, 4, swr_tx_data, qua_mi2s_data, _, _),
>> +       LPI_PINGROUP(3, 8, swr_rx_clk, qua_mi2s_data, _, _),
>> +       LPI_PINGROUP(4, 10, swr_rx_data, qua_mi2s_data, _, _),
>> +       LPI_PINGROUP(5, 12, swr_tx_data, swr_rx_data, _, _),
>> +       LPI_PINGROUP(6, LPI_NO_SLEW, dmic1_clk, i2s1_clk, _,  _),
>> +       LPI_PINGROUP(7, LPI_NO_SLEW, dmic1_data, i2s1_ws, _, _),
>> +       LPI_PINGROUP(8, LPI_NO_SLEW, dmic2_clk, i2s1_data, _, _),
>> +       LPI_PINGROUP(9, LPI_NO_SLEW, dmic2_data, i2s1_data, _, _),
>> +       LPI_PINGROUP(10, 16, i2s2_clk, wsa_swr_clk, _, _),
>> +       LPI_PINGROUP(11, 18, i2s2_ws, wsa_swr_data, _, _),
>> +       LPI_PINGROUP(12, LPI_NO_SLEW, dmic3_clk, i2s2_data, _, _),
>> +       LPI_PINGROUP(13, LPI_NO_SLEW, dmic3_data, i2s2_data, _, _),
>> +};
>> +
>> +static const struct lpi_function sm8250_functions[] = {
>> +       LPI_FUNCTION(dmic1_clk),
>> +       LPI_FUNCTION(dmic1_data),
>> +       LPI_FUNCTION(dmic2_clk),
>> +       LPI_FUNCTION(dmic2_data),
>> +       LPI_FUNCTION(dmic3_clk),
>> +       LPI_FUNCTION(dmic3_data),
>> +       LPI_FUNCTION(i2s1_clk),
>> +       LPI_FUNCTION(i2s1_data),
>> +       LPI_FUNCTION(i2s1_ws),
>> +       LPI_FUNCTION(i2s2_clk),
>> +       LPI_FUNCTION(i2s2_data),
>> +       LPI_FUNCTION(i2s2_ws),
>> +       LPI_FUNCTION(qua_mi2s_data),
>> +       LPI_FUNCTION(qua_mi2s_sclk),
>> +       LPI_FUNCTION(qua_mi2s_ws),
>> +       LPI_FUNCTION(swr_rx_clk),
>> +       LPI_FUNCTION(swr_rx_data),
>> +       LPI_FUNCTION(swr_tx_clk),
>> +       LPI_FUNCTION(swr_tx_data),
>> +       LPI_FUNCTION(wsa_swr_clk),
>> +       LPI_FUNCTION(wsa_swr_data),
>> +};
>> +
>> +static struct lpi_pinctrl_variant_data sm8250_lpi_data = {
> It would be great if this could be const.
Okay. Will add.
>> +       .pins = sm8250_lpi_pins,
>> +       .npins = ARRAY_SIZE(sm8250_lpi_pins),
>> +       .groups = sm8250_groups,
>> +       .ngroups = ARRAY_SIZE(sm8250_groups),
>> +       .functions = sm8250_functions,
>> +       .nfunctions = ARRAY_SIZE(sm8250_functions),
>> +};
>> +

Re: [PATCH v6 5/7] pinctrl: qcom: Extract chip specific LPASS LPI code

[Stephen Boyd]

Quoting Srinivasa Rao Mandadapu (2022-02-14 07:19:52)
> diff --git a/drivers/pinctrl/qcom/pinctrl-sm8250-lpass-lpi.c b/drivers/pinctrl/qcom/pinctrl-sm8250-lpass-lpi.c
> new file mode 100644
> index 0000000..27e358e
> --- /dev/null
> +++ b/drivers/pinctrl/qcom/pinctrl-sm8250-lpass-lpi.c
> @@ -0,0 +1,166 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2016-2019, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2020 Linaro Ltd.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/gpio/driver.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +
> +#include "pinctrl-lpass-lpi.h"

Please include ../core.h here as well for the pin_group definition.

Re: [PATCH v6 5/7] pinctrl: qcom: Extract chip specific LPASS LPI code

[Srinivasa Rao Mandadapu (Temp)]

On 2/19/2022 8:13 AM, Stephen Boyd wrote:
Thanks for Your time Stephen!!!
> Quoting Srinivasa Rao Mandadapu (2022-02-14 07:19:52)
>> diff --git a/drivers/pinctrl/qcom/pinctrl-sm8250-lpass-lpi.c b/drivers/pinctrl/qcom/pinctrl-sm8250-lpass-lpi.c
>> new file mode 100644
>> index 0000000..27e358e
>> --- /dev/null
>> +++ b/drivers/pinctrl/qcom/pinctrl-sm8250-lpass-lpi.c
>> @@ -0,0 +1,166 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright (c) 2016-2019, The Linux Foundation. All rights reserved.
>> + * Copyright (c) 2020 Linaro Ltd.
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/gpio/driver.h>
>> +#include <linux/module.h>
>> +#include <linux/platform_device.h>
>> +
>> +#include "pinctrl-lpass-lpi.h"
> Please include ../core.h here as well for the pin_group definition.
Okay. will add it.

Re: [PATCH v6 5/7] pinctrl: qcom: Extract chip specific LPASS LPI code

[Srinivasa Rao Mandadapu]

On 2/20/2022 12:02 AM, Srinivasa Rao Mandadapu (Temp) wrote:
>
> On 2/19/2022 8:13 AM, Stephen Boyd wrote:
> Thanks for Your time Stephen!!!
>> Quoting Srinivasa Rao Mandadapu (2022-02-14 07:19:52)
>>> diff --git a/drivers/pinctrl/qcom/pinctrl-sm8250-lpass-lpi.c 
>>> b/drivers/pinctrl/qcom/pinctrl-sm8250-lpass-lpi.c
>>> new file mode 100644
>>> index 0000000..27e358e
>>> --- /dev/null
>>> +++ b/drivers/pinctrl/qcom/pinctrl-sm8250-lpass-lpi.c
>>> @@ -0,0 +1,166 @@
>>> +// SPDX-License-Identifier: GPL-2.0-only
>>> +/*
>>> + * Copyright (c) 2016-2019, The Linux Foundation. All rights reserved.
>>> + * Copyright (c) 2020 Linaro Ltd.
>>> + */
>>> +
>>> +#include <linux/clk.h>
>>> +#include <linux/gpio/driver.h>
>>> +#include <linux/module.h>
>>> +#include <linux/platform_device.h>
>>> +
>>> +#include "pinctrl-lpass-lpi.h"
>> Please include ../core.h here as well for the pin_group definition.
> Okay. will add it.

Actually, core.h was included in pinctrl-lpass-lpi.h. Including again 
here giving redefinition errors.

Already sent V8 with this change. will revert it and post again in v9.

[PATCH v6 6/7] pinctrl: qcom: Add SC7280 lpass pin configuration

[Srinivasa Rao Mandadapu]

Add pin control support for SC7280 LPASS LPI.

Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
Co-developed-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
Signed-off-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
---
 drivers/pinctrl/qcom/Kconfig                    |   8 ++
 drivers/pinctrl/qcom/Makefile                   |   1 +
 drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c | 169 ++++++++++++++++++++++++
 3 files changed, 178 insertions(+)
 create mode 100644 drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c

diff --git a/drivers/pinctrl/qcom/Kconfig b/drivers/pinctrl/qcom/Kconfig
index 8871451..e30c7d2 100644
--- a/drivers/pinctrl/qcom/Kconfig
+++ b/drivers/pinctrl/qcom/Kconfig
@@ -357,6 +357,14 @@ config PINCTRL_LPASS_LPI
 	  Qualcomm Technologies Inc LPASS (Low Power Audio SubSystem) LPI
 	  (Low Power Island) found on the Qualcomm Technologies Inc SoCs.
 
+config PINCTRL_SC7280_LPASS_LPI
+	tristate "Qualcomm Technologies Inc SC7280 LPASS LPI pin controller driver"
+	depends on PINCTRL_LPASS_LPI
+	help
+	  This is the pinctrl, pinmux, pinconf and gpiolib driver for the
+	  Qualcomm Technologies Inc LPASS (Low Power Audio SubSystem) LPI
+	  (Low Power Island) found on the Qualcomm Technologies Inc SC7280 platform.
+
 config PINCTRL_SM8250_LPASS_LPI
 	tristate "Qualcomm Technologies Inc SM8250 LPASS LPI pin controller driver"
 	depends on PINCTRL_LPASS_LPI
diff --git a/drivers/pinctrl/qcom/Makefile b/drivers/pinctrl/qcom/Makefile
index c10d14d2..a008ee0 100644
--- a/drivers/pinctrl/qcom/Makefile
+++ b/drivers/pinctrl/qcom/Makefile
@@ -41,4 +41,5 @@ obj-$(CONFIG_PINCTRL_SM8250) += pinctrl-sm8250.o
 obj-$(CONFIG_PINCTRL_SM8350) += pinctrl-sm8350.o
 obj-$(CONFIG_PINCTRL_SM8450) += pinctrl-sm8450.o
 obj-$(CONFIG_PINCTRL_LPASS_LPI) += pinctrl-lpass-lpi.o
+obj-$(CONFIG_PINCTRL_SC7280_LPASS_LPI) += pinctrl-sc7280-lpass-lpi.o
 obj-$(CONFIG_PINCTRL_SM8250_LPASS_LPI) += pinctrl-sm8250-lpass-lpi.o
diff --git a/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c b/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c
new file mode 100644
index 0000000..5bf30d97
--- /dev/null
+++ b/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c
@@ -0,0 +1,169 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2020-2021, The Linux Foundation. All rights reserved.
+ * ALSA SoC platform-machine driver for QTi LPASS
+ */
+
+#include <linux/clk.h>
+#include <linux/gpio/driver.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+
+#include "pinctrl-lpass-lpi.h"
+
+enum lpass_lpi_functions {
+	LPI_MUX_dmic1_clk,
+	LPI_MUX_dmic1_data,
+	LPI_MUX_dmic2_clk,
+	LPI_MUX_dmic2_data,
+	LPI_MUX_dmic3_clk,
+	LPI_MUX_dmic3_data,
+	LPI_MUX_i2s1_clk,
+	LPI_MUX_i2s1_data,
+	LPI_MUX_i2s1_ws,
+	LPI_MUX_i2s2_clk,
+	LPI_MUX_i2s2_data,
+	LPI_MUX_i2s2_ws,
+	LPI_MUX_qua_mi2s_data,
+	LPI_MUX_qua_mi2s_sclk,
+	LPI_MUX_qua_mi2s_ws,
+	LPI_MUX_swr_rx_clk,
+	LPI_MUX_swr_rx_data,
+	LPI_MUX_swr_tx_clk,
+	LPI_MUX_swr_tx_data,
+	LPI_MUX_wsa_swr_clk,
+	LPI_MUX_wsa_swr_data,
+	LPI_MUX_gpio,
+	LPI_MUX__,
+};
+
+static int gpio0_pins[] = { 0 };
+static int gpio1_pins[] = { 1 };
+static int gpio2_pins[] = { 2 };
+static int gpio3_pins[] = { 3 };
+static int gpio4_pins[] = { 4 };
+static int gpio5_pins[] = { 5 };
+static int gpio6_pins[] = { 6 };
+static int gpio7_pins[] = { 7 };
+static int gpio8_pins[] = { 8 };
+static int gpio9_pins[] = { 9 };
+static int gpio10_pins[] = { 10 };
+static int gpio11_pins[] = { 11 };
+static int gpio12_pins[] = { 12 };
+static int gpio13_pins[] = { 13 };
+static int gpio14_pins[] = { 14 };
+
+/* sc7280 variant specific data */
+static const struct pinctrl_pin_desc sc7280_lpi_pins[] = {
+	PINCTRL_PIN(0, "gpio0"),
+	PINCTRL_PIN(1, "gpio1"),
+	PINCTRL_PIN(2, "gpio2"),
+	PINCTRL_PIN(3, "gpio3"),
+	PINCTRL_PIN(4, "gpio4"),
+	PINCTRL_PIN(5, "gpio5"),
+	PINCTRL_PIN(6, "gpio6"),
+	PINCTRL_PIN(7, "gpio7"),
+	PINCTRL_PIN(8, "gpio8"),
+	PINCTRL_PIN(9, "gpio9"),
+	PINCTRL_PIN(10, "gpio10"),
+	PINCTRL_PIN(11, "gpio11"),
+	PINCTRL_PIN(12, "gpio12"),
+	PINCTRL_PIN(13, "gpio13"),
+	PINCTRL_PIN(14, "gpio14"),
+};
+
+static const char * const swr_tx_clk_groups[] = { "gpio0" };
+static const char * const swr_tx_data_groups[] = { "gpio1", "gpio2", "gpio14" };
+static const char * const swr_rx_clk_groups[] = { "gpio3" };
+static const char * const swr_rx_data_groups[] = { "gpio4", "gpio5" };
+static const char * const dmic1_clk_groups[] = { "gpio6" };
+static const char * const dmic1_data_groups[] = { "gpio7" };
+static const char * const dmic2_clk_groups[] = { "gpio8" };
+static const char * const dmic2_data_groups[] = { "gpio9" };
+static const char * const i2s2_clk_groups[] = { "gpio10" };
+static const char * const i2s2_ws_groups[] = { "gpio11" };
+static const char * const dmic3_clk_groups[] = { "gpio12" };
+static const char * const dmic3_data_groups[] = { "gpio13" };
+static const char * const qua_mi2s_sclk_groups[] = { "gpio0" };
+static const char * const qua_mi2s_ws_groups[] = { "gpio1" };
+static const char * const qua_mi2s_data_groups[] = { "gpio2", "gpio3", "gpio4" };
+static const char * const i2s1_clk_groups[] = { "gpio6" };
+static const char * const i2s1_ws_groups[] = { "gpio7" };
+static const char * const i2s1_data_groups[] = { "gpio8", "gpio9" };
+static const char * const wsa_swr_clk_groups[] = { "gpio10" };
+static const char * const wsa_swr_data_groups[] = { "gpio11" };
+static const char * const i2s2_data_groups[] = { "gpio12", "gpio13" };
+
+static const struct lpi_pingroup sc7280_groups[] = {
+	LPI_PINGROUP(0, 0, swr_tx_clk, qua_mi2s_sclk, _, _),
+	LPI_PINGROUP(1, 2, swr_tx_data, qua_mi2s_ws, _, _),
+	LPI_PINGROUP(2, 4, swr_tx_data, qua_mi2s_data, _, _),
+	LPI_PINGROUP(3, 8, swr_rx_clk, qua_mi2s_data, _, _),
+	LPI_PINGROUP(4, 10, swr_rx_data, qua_mi2s_data, _, _),
+	LPI_PINGROUP(5, 12, swr_rx_data, _, _, _),
+	LPI_PINGROUP(6, LPI_NO_SLEW, dmic1_clk, i2s1_clk, _,  _),
+	LPI_PINGROUP(7, LPI_NO_SLEW, dmic1_data, i2s1_ws, _, _),
+	LPI_PINGROUP(8, LPI_NO_SLEW, dmic2_clk, i2s1_data, _, _),
+	LPI_PINGROUP(9, LPI_NO_SLEW, dmic2_data, i2s1_data, _, _),
+	LPI_PINGROUP(10, 16, i2s2_clk, wsa_swr_clk, _, _),
+	LPI_PINGROUP(11, 18, i2s2_ws, wsa_swr_data, _, _),
+	LPI_PINGROUP(12, LPI_NO_SLEW, dmic3_clk, i2s2_data, _, _),
+	LPI_PINGROUP(13, LPI_NO_SLEW, dmic3_data, i2s2_data, _, _),
+	LPI_PINGROUP(14, 6, swr_tx_data, _, _, _),
+};
+
+static const struct lpi_function sc7280_functions[] = {
+	LPI_FUNCTION(dmic1_clk),
+	LPI_FUNCTION(dmic1_data),
+	LPI_FUNCTION(dmic2_clk),
+	LPI_FUNCTION(dmic2_data),
+	LPI_FUNCTION(dmic3_clk),
+	LPI_FUNCTION(dmic3_data),
+	LPI_FUNCTION(i2s1_clk),
+	LPI_FUNCTION(i2s1_data),
+	LPI_FUNCTION(i2s1_ws),
+	LPI_FUNCTION(i2s2_clk),
+	LPI_FUNCTION(i2s2_data),
+	LPI_FUNCTION(i2s2_ws),
+	LPI_FUNCTION(qua_mi2s_data),
+	LPI_FUNCTION(qua_mi2s_sclk),
+	LPI_FUNCTION(qua_mi2s_ws),
+	LPI_FUNCTION(swr_rx_clk),
+	LPI_FUNCTION(swr_rx_data),
+	LPI_FUNCTION(swr_tx_clk),
+	LPI_FUNCTION(swr_tx_data),
+	LPI_FUNCTION(wsa_swr_clk),
+	LPI_FUNCTION(wsa_swr_data),
+};
+
+static const struct lpi_pinctrl_variant_data sc7280_lpi_data = {
+	.pins = sc7280_lpi_pins,
+	.npins = ARRAY_SIZE(sc7280_lpi_pins),
+	.groups = sc7280_groups,
+	.ngroups = ARRAY_SIZE(sc7280_groups),
+	.functions = sc7280_functions,
+	.nfunctions = ARRAY_SIZE(sc7280_functions),
+};
+
+static const struct of_device_id lpi_pinctrl_of_match[] = {
+	{
+	       .compatible = "qcom,sc7280-lpass-lpi-pinctrl",
+	       .data = &sc7280_lpi_data,
+	},
+	{ }
+};
+MODULE_DEVICE_TABLE(of, lpi_pinctrl_of_match);
+
+static struct platform_driver lpi_pinctrl_driver = {
+	.driver = {
+		   .name = "qcom-sc7280-lpass-lpi-pinctrl",
+		   .of_match_table = lpi_pinctrl_of_match,
+	},
+	.probe = lpi_pinctrl_probe,
+	.remove = lpi_pinctrl_remove,
+};
+
+module_platform_driver(lpi_pinctrl_driver);
+MODULE_DESCRIPTION("QTI SC7280 LPI GPIO pin control driver");
+MODULE_LICENSE("GPL");
+
-- 
2.7.4

Re: [PATCH v6 6/7] pinctrl: qcom: Add SC7280 lpass pin configuration

[Srinivas Kandagatla]

On 14/02/2022 15:19, Srinivasa Rao Mandadapu wrote:
> +module_platform_driver(lpi_pinctrl_driver);
> +MODULE_DESCRIPTION("QTI SC7280 LPI GPIO pin control driver");
> +MODULE_LICENSE("GPL");
> +
New line not required!

--srini
> -- 

Re: [PATCH v6 6/7] pinctrl: qcom: Add SC7280 lpass pin configuration

[Stephen Boyd]

Quoting Srinivasa Rao Mandadapu (2022-02-14 07:19:53)
> diff --git a/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c b/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c
> new file mode 100644
> index 0000000..5bf30d97
> --- /dev/null
> +++ b/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c
> @@ -0,0 +1,169 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2020-2021, The Linux Foundation. All rights reserved.
> + * ALSA SoC platform-machine driver for QTi LPASS
> + */
> +
> +#include <linux/clk.h>

Drop unused include.

> +#include <linux/gpio/driver.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +
> +#include "pinctrl-lpass-lpi.h"

include ../core.h

> +
> +enum lpass_lpi_functions {
> +       LPI_MUX_dmic1_clk,
> +       LPI_MUX_dmic1_data,
> +       LPI_MUX_dmic2_clk,
> +       LPI_MUX_dmic2_data,
> +       LPI_MUX_dmic3_clk,
> +       LPI_MUX_dmic3_data,
> +       LPI_MUX_i2s1_clk,
> +       LPI_MUX_i2s1_data,
> +       LPI_MUX_i2s1_ws,
> +       LPI_MUX_i2s2_clk,
> +       LPI_MUX_i2s2_data,
> +       LPI_MUX_i2s2_ws,
> +       LPI_MUX_qua_mi2s_data,
> +       LPI_MUX_qua_mi2s_sclk,
> +       LPI_MUX_qua_mi2s_ws,
> +       LPI_MUX_swr_rx_clk,
> +       LPI_MUX_swr_rx_data,
> +       LPI_MUX_swr_tx_clk,
> +       LPI_MUX_swr_tx_data,
> +       LPI_MUX_wsa_swr_clk,
> +       LPI_MUX_wsa_swr_data,
> +       LPI_MUX_gpio,
> +       LPI_MUX__,
> +};
> +
> +static int gpio0_pins[] = { 0 };

const?

> +static int gpio1_pins[] = { 1 };
> +static int gpio2_pins[] = { 2 };
> +static int gpio3_pins[] = { 3 };
> +static int gpio4_pins[] = { 4 };
> +static int gpio5_pins[] = { 5 };
> +static int gpio6_pins[] = { 6 };
> +static int gpio7_pins[] = { 7 };

Re: [PATCH v6 6/7] pinctrl: qcom: Add SC7280 lpass pin configuration

[Srinivasa Rao Mandadapu (Temp)]

On 2/19/2022 8:13 AM, Stephen Boyd wrote:
Thanks for Your time Stephen!!!
> Quoting Srinivasa Rao Mandadapu (2022-02-14 07:19:53)
>> diff --git a/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c b/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c
>> new file mode 100644
>> index 0000000..5bf30d97
>> --- /dev/null
>> +++ b/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c
>> @@ -0,0 +1,169 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright (c) 2020-2021, The Linux Foundation. All rights reserved.
>> + * ALSA SoC platform-machine driver for QTi LPASS
>> + */
>> +
>> +#include <linux/clk.h>
> Drop unused include.
Okay. will remove it.
>
>> +#include <linux/gpio/driver.h>
>> +#include <linux/module.h>
>> +#include <linux/platform_device.h>
>> +
>> +#include "pinctrl-lpass-lpi.h"
> include ../core.h
Okay. will add it.
>
>> +
>> +enum lpass_lpi_functions {
>> +       LPI_MUX_dmic1_clk,
>> +       LPI_MUX_dmic1_data,
>> +       LPI_MUX_dmic2_clk,
>> +       LPI_MUX_dmic2_data,
>> +       LPI_MUX_dmic3_clk,
>> +       LPI_MUX_dmic3_data,
>> +       LPI_MUX_i2s1_clk,
>> +       LPI_MUX_i2s1_data,
>> +       LPI_MUX_i2s1_ws,
>> +       LPI_MUX_i2s2_clk,
>> +       LPI_MUX_i2s2_data,
>> +       LPI_MUX_i2s2_ws,
>> +       LPI_MUX_qua_mi2s_data,
>> +       LPI_MUX_qua_mi2s_sclk,
>> +       LPI_MUX_qua_mi2s_ws,
>> +       LPI_MUX_swr_rx_clk,
>> +       LPI_MUX_swr_rx_data,
>> +       LPI_MUX_swr_tx_clk,
>> +       LPI_MUX_swr_tx_data,
>> +       LPI_MUX_wsa_swr_clk,
>> +       LPI_MUX_wsa_swr_data,
>> +       LPI_MUX_gpio,
>> +       LPI_MUX__,
>> +};
>> +
>> +static int gpio0_pins[] = { 0 };
> const?
const giving conflicts with group_desc params.
>
>> +static int gpio1_pins[] = { 1 };
>> +static int gpio2_pins[] = { 2 };
>> +static int gpio3_pins[] = { 3 };
>> +static int gpio4_pins[] = { 4 };
>> +static int gpio5_pins[] = { 5 };
>> +static int gpio6_pins[] = { 6 };
>> +static int gpio7_pins[] = { 7 };

[PATCH v6 7/7] pinctrl: qcom: Update clock voting as optional

[Srinivasa Rao Mandadapu]

Update bulk clock voting to optional voting as ADSP bypass platform doesn't
need macro and decodec clocks, these are maintained as power domains and
operated from lpass audio core cc.

Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
Co-developed-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
Signed-off-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
---
 drivers/pinctrl/qcom/pinctrl-lpass-lpi.c        | 16 +++++++++-------
 drivers/pinctrl/qcom/pinctrl-lpass-lpi.h        |  1 +
 drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c |  1 +
 3 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
index 8a82fd9..103f0a6c 100644
--- a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
+++ b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
@@ -407,13 +407,15 @@ int lpi_pinctrl_probe(struct platform_device *pdev)
 		return dev_err_probe(dev, PTR_ERR(pctrl->slew_base),
 				     "Slew resource not provided\n");
 
-	ret = devm_clk_bulk_get(dev, MAX_LPI_NUM_CLKS, pctrl->clks);
-	if (ret)
-		return dev_err_probe(dev, ret, "Can't get clocks\n");
-
-	ret = clk_bulk_prepare_enable(MAX_LPI_NUM_CLKS, pctrl->clks);
-	if (ret)
-		return dev_err_probe(dev, ret, "Can't enable clocks\n");
+	if (!data->is_clk_optional) {
+		ret = devm_clk_bulk_get(dev, MAX_LPI_NUM_CLKS, pctrl->clks);
+		if (ret)
+			return dev_err_probe(dev, ret, "Can't get clocks\n");
+
+		ret = clk_bulk_prepare_enable(MAX_LPI_NUM_CLKS, pctrl->clks);
+		if (ret)
+			return dev_err_probe(dev, ret, "Can't enable clocks\n");
+	}
 
 	pctrl->desc.pctlops = &lpi_gpio_pinctrl_ops;
 	pctrl->desc.pmxops = &lpi_gpio_pinmux_ops;
diff --git a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.h b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.h
index a511d72..c1079bf 100644
--- a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.h
+++ b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.h
@@ -77,6 +77,7 @@ struct lpi_pinctrl_variant_data {
 	int ngroups;
 	const struct lpi_function *functions;
 	int nfunctions;
+	int is_clk_optional;
 };
 
 int lpi_pinctrl_probe(struct platform_device *pdev);
diff --git a/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c b/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c
index 5bf30d97..4277e31 100644
--- a/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c
+++ b/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c
@@ -143,6 +143,7 @@ static const struct lpi_pinctrl_variant_data sc7280_lpi_data = {
 	.ngroups = ARRAY_SIZE(sc7280_groups),
 	.functions = sc7280_functions,
 	.nfunctions = ARRAY_SIZE(sc7280_functions),
+	.is_clk_optional = 1,
 };
 
 static const struct of_device_id lpi_pinctrl_of_match[] = {
-- 
2.7.4

Re: [PATCH v6 7/7] pinctrl: qcom: Update clock voting as optional

[Srinivas Kandagatla]

On 14/02/2022 15:19, Srinivasa Rao Mandadapu wrote:
> Update bulk clock voting to optional voting as ADSP bypass platform doesn't
> need macro and decodec clocks, these are maintained as power domains and
> operated from lpass audio core cc.
> 
> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
> Co-developed-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
> Signed-off-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
> ---
>   drivers/pinctrl/qcom/pinctrl-lpass-lpi.c        | 16 +++++++++-------
>   drivers/pinctrl/qcom/pinctrl-lpass-lpi.h        |  1 +
>   drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c |  1 +
>   3 files changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
> index 8a82fd9..103f0a6c 100644
> --- a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
> +++ b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
> @@ -407,13 +407,15 @@ int lpi_pinctrl_probe(struct platform_device *pdev)
>   		return dev_err_probe(dev, PTR_ERR(pctrl->slew_base),
>   				     "Slew resource not provided\n");
>   
> -	ret = devm_clk_bulk_get(dev, MAX_LPI_NUM_CLKS, pctrl->clks);
> -	if (ret)
> -		return dev_err_probe(dev, ret, "Can't get clocks\n");
> -
> -	ret = clk_bulk_prepare_enable(MAX_LPI_NUM_CLKS, pctrl->clks);
> -	if (ret)
> -		return dev_err_probe(dev, ret, "Can't enable clocks\n");
> +	if (!data->is_clk_optional) {
> +		ret = devm_clk_bulk_get(dev, MAX_LPI_NUM_CLKS, pctrl->clks);
> +		if (ret)
> +			return dev_err_probe(dev, ret, "Can't get clocks\n");
> +
> +		ret = clk_bulk_prepare_enable(MAX_LPI_NUM_CLKS, pctrl->clks);
> +		if (ret)
> +			return dev_err_probe(dev, ret, "Can't enable clocks\n");
> +	}
>   
>   	pctrl->desc.pctlops = &lpi_gpio_pinctrl_ops;
>   	pctrl->desc.pmxops = &lpi_gpio_pinmux_ops;
> diff --git a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.h b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.h
> index a511d72..c1079bf 100644
> --- a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.h
> +++ b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.h
> @@ -77,6 +77,7 @@ struct lpi_pinctrl_variant_data {
>   	int ngroups;
>   	const struct lpi_function *functions;
>   	int nfunctions;
> +	int is_clk_optional;
>   };
>   
>   int lpi_pinctrl_probe(struct platform_device *pdev);
> diff --git a/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c b/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c
> index 5bf30d97..4277e31 100644
> --- a/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c
> +++ b/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c
> @@ -143,6 +143,7 @@ static const struct lpi_pinctrl_variant_data sc7280_lpi_data = {
>   	.ngroups = ARRAY_SIZE(sc7280_groups),
>   	.functions = sc7280_functions,
>   	.nfunctions = ARRAY_SIZE(sc7280_functions),
> +	.is_clk_optional = 1,

This is forcefully set assuming that sc7280 is always used in ADSP 
bypass mode. Which is not correct.

Can't you use devm_clk_bulk_get_optional instead?

--srini

>   };
>   
>   static const struct of_device_id lpi_pinctrl_of_match[] = {

Re: [PATCH v6 7/7] pinctrl: qcom: Update clock voting as optional

[Srinivasa Rao Mandadapu]

On 2/16/2022 7:50 PM, Srinivas Kandagatla wrote:
Thanks for Your Time Srini!!!
>
> On 14/02/2022 15:19, Srinivasa Rao Mandadapu wrote:
>> Update bulk clock voting to optional voting as ADSP bypass platform 
>> doesn't
>> need macro and decodec clocks, these are maintained as power domains and
>> operated from lpass audio core cc.
>>
>> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
>> Co-developed-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
>> Signed-off-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
>> ---
>>   drivers/pinctrl/qcom/pinctrl-lpass-lpi.c        | 16 +++++++++-------
>>   drivers/pinctrl/qcom/pinctrl-lpass-lpi.h        |  1 +
>>   drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c |  1 +
>>   3 files changed, 11 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c 
>> b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
>> index 8a82fd9..103f0a6c 100644
>> --- a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
>> +++ b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
>> @@ -407,13 +407,15 @@ int lpi_pinctrl_probe(struct platform_device 
>> *pdev)
>>           return dev_err_probe(dev, PTR_ERR(pctrl->slew_base),
>>                        "Slew resource not provided\n");
>>   -    ret = devm_clk_bulk_get(dev, MAX_LPI_NUM_CLKS, pctrl->clks);
>> -    if (ret)
>> -        return dev_err_probe(dev, ret, "Can't get clocks\n");
>> -
>> -    ret = clk_bulk_prepare_enable(MAX_LPI_NUM_CLKS, pctrl->clks);
>> -    if (ret)
>> -        return dev_err_probe(dev, ret, "Can't enable clocks\n");
>> +    if (!data->is_clk_optional) {
>> +        ret = devm_clk_bulk_get(dev, MAX_LPI_NUM_CLKS, pctrl->clks);
>> +        if (ret)
>> +            return dev_err_probe(dev, ret, "Can't get clocks\n");
>> +
>> +        ret = clk_bulk_prepare_enable(MAX_LPI_NUM_CLKS, pctrl->clks);
>> +        if (ret)
>> +            return dev_err_probe(dev, ret, "Can't enable clocks\n");
>> +    }
>>         pctrl->desc.pctlops = &lpi_gpio_pinctrl_ops;
>>       pctrl->desc.pmxops = &lpi_gpio_pinmux_ops;
>> diff --git a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.h 
>> b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.h
>> index a511d72..c1079bf 100644
>> --- a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.h
>> +++ b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.h
>> @@ -77,6 +77,7 @@ struct lpi_pinctrl_variant_data {
>>       int ngroups;
>>       const struct lpi_function *functions;
>>       int nfunctions;
>> +    int is_clk_optional;
>>   };
>>     int lpi_pinctrl_probe(struct platform_device *pdev);
>> diff --git a/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c 
>> b/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c
>> index 5bf30d97..4277e31 100644
>> --- a/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c
>> +++ b/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c
>> @@ -143,6 +143,7 @@ static const struct lpi_pinctrl_variant_data 
>> sc7280_lpi_data = {
>>       .ngroups = ARRAY_SIZE(sc7280_groups),
>>       .functions = sc7280_functions,
>>       .nfunctions = ARRAY_SIZE(sc7280_functions),
>> +    .is_clk_optional = 1,
>
> This is forcefully set assuming that sc7280 is always used in ADSP 
> bypass mode. Which is not correct.
>
> Can't you use devm_clk_bulk_get_optional instead?

Yes. Agreed. Initially used devm_clk_bulk_get_optional, but Bjorn 
suggested for conditional check instead of optional.

Again Shall we go for optional clock voting?

>
> --srini
>
>>   };
>>     static const struct of_device_id lpi_pinctrl_of_match[] = {

Re: [PATCH v6 7/7] pinctrl: qcom: Update clock voting as optional

[Srinivas Kandagatla]

On 16/02/2022 14:41, Srinivasa Rao Mandadapu wrote:
> 
> On 2/16/2022 7:50 PM, Srinivas Kandagatla wrote:
> Thanks for Your Time Srini!!!
>>
>> On 14/02/2022 15:19, Srinivasa Rao Mandadapu wrote:
>>> Update bulk clock voting to optional voting as ADSP bypass platform 
>>> doesn't
>>> need macro and decodec clocks, these are maintained as power domains and
>>> operated from lpass audio core cc.
>>>
>>> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
>>> Co-developed-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
>>> Signed-off-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
>>> ---
>>>   drivers/pinctrl/qcom/pinctrl-lpass-lpi.c        | 16 +++++++++-------
>>>   drivers/pinctrl/qcom/pinctrl-lpass-lpi.h        |  1 +
>>>   drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c |  1 +
>>>   3 files changed, 11 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c 
>>> b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
>>> index 8a82fd9..103f0a6c 100644
>>> --- a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
>>> +++ b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
>>> @@ -407,13 +407,15 @@ int lpi_pinctrl_probe(struct platform_device 
>>> *pdev)
>>>           return dev_err_probe(dev, PTR_ERR(pctrl->slew_base),
>>>                        "Slew resource not provided\n");
>>>   -    ret = devm_clk_bulk_get(dev, MAX_LPI_NUM_CLKS, pctrl->clks);
>>> -    if (ret)
>>> -        return dev_err_probe(dev, ret, "Can't get clocks\n");
>>> -
>>> -    ret = clk_bulk_prepare_enable(MAX_LPI_NUM_CLKS, pctrl->clks);
>>> -    if (ret)
>>> -        return dev_err_probe(dev, ret, "Can't enable clocks\n");
>>> +    if (!data->is_clk_optional) {
>>> +        ret = devm_clk_bulk_get(dev, MAX_LPI_NUM_CLKS, pctrl->clks);
>>> +        if (ret)
>>> +            return dev_err_probe(dev, ret, "Can't get clocks\n");
>>> +
>>> +        ret = clk_bulk_prepare_enable(MAX_LPI_NUM_CLKS, pctrl->clks);
>>> +        if (ret)
>>> +            return dev_err_probe(dev, ret, "Can't enable clocks\n");
>>> +    }
>>>         pctrl->desc.pctlops = &lpi_gpio_pinctrl_ops;
>>>       pctrl->desc.pmxops = &lpi_gpio_pinmux_ops;
>>> diff --git a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.h 
>>> b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.h
>>> index a511d72..c1079bf 100644
>>> --- a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.h
>>> +++ b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.h
>>> @@ -77,6 +77,7 @@ struct lpi_pinctrl_variant_data {
>>>       int ngroups;
>>>       const struct lpi_function *functions;
>>>       int nfunctions;
>>> +    int is_clk_optional;
>>>   };
>>>     int lpi_pinctrl_probe(struct platform_device *pdev);
>>> diff --git a/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c 
>>> b/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c
>>> index 5bf30d97..4277e31 100644
>>> --- a/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c
>>> +++ b/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c
>>> @@ -143,6 +143,7 @@ static const struct lpi_pinctrl_variant_data 
>>> sc7280_lpi_data = {
>>>       .ngroups = ARRAY_SIZE(sc7280_groups),
>>>       .functions = sc7280_functions,
>>>       .nfunctions = ARRAY_SIZE(sc7280_functions),
>>> +    .is_clk_optional = 1,
>>
>> This is forcefully set assuming that sc7280 is always used in ADSP 
>> bypass mode. Which is not correct.
>>
>> Can't you use devm_clk_bulk_get_optional instead?
> 
> Yes. Agreed. Initially used devm_clk_bulk_get_optional, but Bjorn 
> suggested for conditional check instead of optional.
> 
> Again Shall we go for optional clock voting?

That means that the condition has to be dynamic based on the platform 
using DSP or not. Which is impossible to deduce without some help from DT.

I would prefer to stay with optional clock unless Bjorn has some strong 
objection on not using int.

--srini

> 
>>
>> --srini
>>
>>>   };
>>>     static const struct of_device_id lpi_pinctrl_of_match[] = {

Re: [PATCH v6 7/7] pinctrl: qcom: Update clock voting as optional

[Stephen Boyd]

Quoting Srinivas Kandagatla (2022-02-16 07:38:02)
>
>
> On 16/02/2022 14:41, Srinivasa Rao Mandadapu wrote:
> >
> > On 2/16/2022 7:50 PM, Srinivas Kandagatla wrote:
> > Thanks for Your Time Srini!!!
> >>
> >> On 14/02/2022 15:19, Srinivasa Rao Mandadapu wrote:
> >>> diff --git a/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c
> >>> b/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c
> >>> index 5bf30d97..4277e31 100644
> >>> --- a/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c
> >>> +++ b/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c
> >>> @@ -143,6 +143,7 @@ static const struct lpi_pinctrl_variant_data
> >>> sc7280_lpi_data = {
> >>>       .ngroups = ARRAY_SIZE(sc7280_groups),
> >>>       .functions = sc7280_functions,
> >>>       .nfunctions = ARRAY_SIZE(sc7280_functions),
> >>> +    .is_clk_optional = 1,
> >>
> >> This is forcefully set assuming that sc7280 is always used in ADSP
> >> bypass mode. Which is not correct.
> >>
> >> Can't you use devm_clk_bulk_get_optional instead?
> >
> > Yes. Agreed. Initially used devm_clk_bulk_get_optional, but Bjorn
> > suggested for conditional check instead of optional.
> >
> > Again Shall we go for optional clock voting?
>
> That means that the condition has to be dynamic based on the platform
> using DSP or not. Which is impossible to deduce without some help from DT.
>
> I would prefer to stay with optional clock unless Bjorn has some strong
> objection on not using int.

I think we need the combination of optional API and bool flag. My
understanding is it's optional on sc7280, but not on the previous
revision, so we want to be very strict on previous revision and less
strict on sc7280. Hence the flag. Maybe we should change it to
clk_required and then assume optional going forward. Then the callsite
can use one or the other API?

Re: [PATCH v6 7/7] pinctrl: qcom: Update clock voting as optional

[Srinivasa Rao Mandadapu (Temp)]

On 2/19/2022 8:16 AM, Stephen Boyd wrote:
Thanks for Your time Stephen!!!
> Quoting Srinivas Kandagatla (2022-02-16 07:38:02)
>>
>> On 16/02/2022 14:41, Srinivasa Rao Mandadapu wrote:
>>> On 2/16/2022 7:50 PM, Srinivas Kandagatla wrote:
>>> Thanks for Your Time Srini!!!
>>>> On 14/02/2022 15:19, Srinivasa Rao Mandadapu wrote:
>>>>> diff --git a/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c
>>>>> b/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c
>>>>> index 5bf30d97..4277e31 100644
>>>>> --- a/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c
>>>>> +++ b/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c
>>>>> @@ -143,6 +143,7 @@ static const struct lpi_pinctrl_variant_data
>>>>> sc7280_lpi_data = {
>>>>>        .ngroups = ARRAY_SIZE(sc7280_groups),
>>>>>        .functions = sc7280_functions,
>>>>>        .nfunctions = ARRAY_SIZE(sc7280_functions),
>>>>> +    .is_clk_optional = 1,
>>>> This is forcefully set assuming that sc7280 is always used in ADSP
>>>> bypass mode. Which is not correct.
>>>>
>>>> Can't you use devm_clk_bulk_get_optional instead?
>>> Yes. Agreed. Initially used devm_clk_bulk_get_optional, but Bjorn
>>> suggested for conditional check instead of optional.
>>>
>>> Again Shall we go for optional clock voting?
>> That means that the condition has to be dynamic based on the platform
>> using DSP or not. Which is impossible to deduce without some help from DT.
>>
>> I would prefer to stay with optional clock unless Bjorn has some strong
>> objection on not using int.
> I think we need the combination of optional API and bool flag. My
> understanding is it's optional on sc7280, but not on the previous
> revision, so we want to be very strict on previous revision and less
> strict on sc7280. Hence the flag. Maybe we should change it to
> clk_required and then assume optional going forward. Then the callsite
> can use one or the other API?
Okay.Will change accordingly.

Re: [PATCH v6 0/7] Add pin control support for lpass sc7280

[Srinivas Kandagatla]

On 14/02/2022 15:19, Srinivasa Rao Mandadapu wrote:
> This patch series is to split lpass variant common pin control
> functions and SoC specific functions and to add lpass sc7280 pincontrol support.
> It also Adds dt-bindings for lpass sc7280 lpass lpi pincontrol.
> 
> Changes Since V5:
>      -- Create new patch by updating macro name to lpi specific.
>      -- Create new patch by updating lpi pin group structure with core group_desc structure.
>      -- Fix typo errors.
>      -- Sort macros in the make file and configuration file.
> Changes Since V4:
>      -- Update commit message and description of the chip specific extraction patch.
>      -- Sort macros in kconfig and makefile.
>      -- Update optional clock voting to conditional clock voting.
>      -- Fix typo errors.
>      -- Move to quicinc domain email id's.
> Changes Since V3:
>      -- Update separate Kconfig fields for sm8250 and sc7280.
>      -- Update module license and description.
>      -- Move static variables to corresponding .c files from header file.
> 
> Changes Since V2:
>      -- Add new dt-bindings for sc7280 lpi driver.
>      -- Make clock voting change as separate patch.
>      -- Split existing pincontrol driver and make common functions
>         as part of separate file.
>      -- Rename lpass pincontrol lpi dt-bindings to sm8250 specific dt-bindings
> 		
> Changes Since V1:
>      -- Make lpi pinctrl variant data structure as constant
>      -- Add appropriate commit message
>      -- Change signedoff by sequence.
> 
> Srinivasa Rao Mandadapu (7):
>    dt-bindings: pinctrl: qcom: Update lpass lpi file name to SoC specific
>    dt-bindings: pinctrl: qcom: Add sc7280 lpass lpi pinctrl bindings
>    pinctrl: qcom: Update macro name to LPI specific
>    pinctrl: qcom: Update lpi pin group structure
>    pinctrl: qcom: Extract chip specific LPASS LPI code
>    pinctrl: qcom: Add SC7280 lpass pin configuration
>    pinctrl: qcom: Update clock voting as optional


Tested this on SM8250 MTP with WSA and WCD codecs

Tested-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

--srini

> 
>   .../bindings/pinctrl/qcom,lpass-lpi-pinctrl.yaml   | 133 -----------
>   .../pinctrl/qcom,sc7280-lpass-lpi-pinctrl.yaml     | 115 +++++++++
>   .../pinctrl/qcom,sm8250-lpass-lpi-pinctrl.yaml     | 133 +++++++++++
>   drivers/pinctrl/qcom/Kconfig                       |  16 ++
>   drivers/pinctrl/qcom/Makefile                      |   2 +
>   drivers/pinctrl/qcom/pinctrl-lpass-lpi.c           | 257 ++-------------------
>   drivers/pinctrl/qcom/pinctrl-lpass-lpi.h           |  87 +++++++
>   drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c    | 170 ++++++++++++++
>   drivers/pinctrl/qcom/pinctrl-sm8250-lpass-lpi.c    | 166 +++++++++++++
>   9 files changed, 706 insertions(+), 373 deletions(-)
>   delete mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,lpass-lpi-pinctrl.yaml
>   create mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,sc7280-lpass-lpi-pinctrl.yaml
>   create mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,sm8250-lpass-lpi-pinctrl.yaml
>   create mode 100644 drivers/pinctrl/qcom/pinctrl-lpass-lpi.h
>   create mode 100644 drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c
>   create mode 100644 drivers/pinctrl/qcom/pinctrl-sm8250-lpass-lpi.c
>