Skip to contentSkip to conversation

SDM670 Pin Control Driver

PATCH Superseded Richard Acayan 3 participants 7 messages Oct 6, 2022 – Oct 7, 2022

Subsystems (6)

v8 of 10 Superseded Latest is v10 →

lore
Share & tools
lore
#0 lore raw

v8 cover SDM670 Pin Control Driver

Changes since v7:
- remove complement_fw_reserved_gpios from patch 3

Changes since v6:
- remove bitmap_fill() call completely (original commit was 4c0efbfb2669 ("pinctrl: msm: Add ability for drivers to supply a reserved GPIO list"), original author CC'd)
- group some entries in pingroup arrays for less lines
- accumulate review tags

Changes since v5:
- remove function requirement in schema
- change "-state$" regex to double quotes in schema
- drop quotes from qcom-tlmm-state ref in dt schema

Changes since v4:
- accumulate review tag
- use --cover-letter
- this is effectively a resend

Changes since v3:
- constrain gpio-reserved-ranges to 1-75 items
- proper indentation in dt-bindings example (actually tagged this time instead of an outdated commit)
- remove unnecessary commit reference
- rename complement_fw_reserved_gpios -> complement_fw_gpio_ranges

Changes since v2:
- remove quotes from pinctrl.yaml# in dt-bindings
- constrain gpio-reserved-ranges to 1-76 items (includes ufs_reset)
- indentation in dt-bindings example
- cite downstream kernel source
- remove MODULE_AUTHOR (most imported pinctrl drivers don't have this)

Changes since v1:
- add a field in msm_pinctrl_soc_data to accomodate the needs of the driver and device dts
- apply changes made to existing tlmm dt documentation
- add reserved gpios array
- rename device tree compat string to qcom,sdm670-tlmm
- remove dependency on ACPI
- move MODULE_DEVICE_TABLE macro call up
- add missing pins (fixes most of the debugfs problems)
- move qup0_pins down
- add whitespace between UFS_RESET macro and pins array

This patch series adds the driver for the Qualcomm Snapdragon 670 TLMM (Top-Level Mode Multiplexer) and introduces a new field so that SDM670- related device trees can reserve their own gpios.

Richard Acayan (3):
dt-bindings: pinctrl: qcom: add sdm670 pinctrl
pinctrl: qcom: do not reinitialize gpio valid mask
pinctrl: qcom: add sdm670 pinctrl

 .../bindings/pinctrl/qcom,sdm670-tlmm.yaml    |  129 ++
 drivers/pinctrl/qcom/Kconfig                  |    9 +
 drivers/pinctrl/qcom/Makefile                 |    1 +
 drivers/pinctrl/qcom/pinctrl-msm.c            |    3 +-
 drivers/pinctrl/qcom/pinctrl-sdm670.c         | 1345 +++++++++++++++++
 5 files changed, 1485 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,sdm670-tlmm.yaml
 create mode 100644 drivers/pinctrl/qcom/pinctrl-sdm670.c
Signature · 1 line
2.38.0
#1 lore raw

PATCH v8 1/3 dt-bindings: pinctrl: qcom: add sdm670 pinctrl

There is a new driver for the Snapdragon 670 TLMM (Top-Level Mode Multiplexer). Document it.

Adapted from qcom,sm6350-pinctrl.yaml.

---
 .../bindings/pinctrl/qcom,sdm670-tlmm.yaml    | 129 ++++++++++++++++++
 1 file changed, 129 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,sdm670-tlmm.yaml
Documentation/devicetree/bindings/pinctrl/qcom,sdm670-tlmm.yaml new file +129 -0
new file mode 100644
index 000000000000..e2f563ae6bbf
@@ -0,0 +1,129 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pinctrl/qcom,sdm670-tlmm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Technologies, Inc. SDM670 TLMM block
+
+maintainers:
+  - Richard Acayan <[email protected]>
+
+description: |
+  This binding describes the Top Level Mode Multiplexer (TLMM) block found
+  in the SDM670 platform.
+
+allOf:
+  - $ref: pinctrl.yaml#
+  - $ref: /schemas/pinctrl/qcom,tlmm-common.yaml#
+
+properties:
+  compatible:
+    const: qcom,sdm670-tlmm
+
+  reg:
+    maxItems: 1
+
+  interrupts: true
+  interrupt-controller: true
+  '#interrupt-cells': true
+  gpio-controller: true
+  gpio-reserved-ranges:
+    minItems: 1
+    maxItems: 75
+
+  '#gpio-cells': true
+  gpio-ranges: true
+  wakeup-parent: true
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+patternProperties:
+  "-state$":
+    oneOf:
+      - $ref: "#/$defs/qcom-sdm670-tlmm-state"
+      - patternProperties:
+          "-pins$":
+            $ref: "#/$defs/qcom-sdm670-tlmm-state"
+        additionalProperties: false
+
+$defs:
+  qcom-sdm670-tlmm-state:
+    type: object
+    description:
+      Pinctrl node's client devices use subnodes for desired pin configuration.
+      Client device subnodes use below standard properties.
+    $ref: qcom,tlmm-common.yaml#/$defs/qcom-tlmm-state
+
+    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]|1[0-4][0-9])$"
+            - enum: [ ufs_reset, sdc1_rclk, sdc1_clk, sdc1_cmd, sdc1_data,
+                      sdc2_clk, sdc2_cmd, sdc2_data ]
+        minItems: 1
+        maxItems: 36
+
+      function:
+        description:
+          Specify the alternative function to be configured for the specified
+          pins.
+
+        enum: [ adsp_ext, agera_pll, atest_char, atest_tsens, atest_tsens2, atest_usb1, atest_usb10,
+                atest_usb11, atest_usb12, atest_usb13, atest_usb2, atest_usb20, atest_usb21,
+                atest_usb22, atest_usb23, cam_mclk, cci_async, cci_i2c, cci_timer0, cci_timer1,
+                cci_timer2, cci_timer3, cci_timer4, copy_gp, copy_phase, dbg_out, ddr_bist,
+                ddr_pxi0, ddr_pxi1, ddr_pxi2, ddr_pxi3, edp_hot, edp_lcd, gcc_gp1, gcc_gp2, gcc_gp3,
+                gp_pdm0, gp_pdm1, gp_pdm2, gpio, gps_tx, jitter_bist, ldo_en, ldo_update,
+                lpass_slimbus, m_voc, mdp_vsync, mdp_vsync0, mdp_vsync1, mdp_vsync2, mdp_vsync3,
+                mss_lte, nav_pps, pa_indicator, pci_e0, pci_e1, phase_flag, pll_bist, pll_bypassnl,
+                pll_reset, pri_mi2s, pri_mi2s_ws, prng_rosc, qdss_cti, qdss, qlink_enable,
+                qlink_request, qua_mi2s, qup0, qup1, qup10, qup11, qup12, qup13, qup14, qup15, qup2,
+                qup3, qup4, qup5, qup6, qup7, qup8, qup9, qup_l4, qup_l5, qup_l6, sdc4_clk,
+                sdc4_cmd, sdc4_data, sd_write, sec_mi2s, ter_mi2s, tgu_ch0, tgu_ch1, tgu_ch2,
+                tgu_ch3, tsif1_clk, tsif1_data, tsif1_en, tsif1_error, tsif1_sync, tsif2_clk,
+                tsif2_data, tsif2_en, tsif2_error, tsif2_sync, uim1_clk, uim1_data, uim1_present,
+                uim1_reset, uim2_clk, uim2_data, uim2_present, uim2_reset, uim_batt, usb_phy, vfr_1,
+                vsense_trigger, wlan1_adc0, wlan1_adc1, wlan2_adc0, wlan2_adc1, wsa_clk, wsa_data, ]
+
+
+      bias-disable: true
+      bias-pull-down: true
+      bias-pull-up: true
+      drive-strength: true
+      input-enable: true
+      output-high: true
+      output-low: true
+
+    required:
+      - pins
+
+    additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    pinctrl@3400000 {
+        compatible = "qcom,sdm670-tlmm";
+        reg = <0x03400000 0x300000>;
+        interrupts = <GIC_SPI 208 IRQ_TYPE_LEVEL_HIGH>;
+        gpio-controller;
+        #gpio-cells = <2>;
+        interrupt-controller;
+        #interrupt-cells = <2>;
+        gpio-ranges = <&tlmm 0 0 151>;
+
+        qup-i2c9-state {
+            pins = "gpio6", "gpio7";
+            function = "qup9";
+        };
+    };
+...
Signature · 1 line
2.38.0
#2 lore raw

Re: [PATCH v8 1/3] dt-bindings: pinctrl: qcom: add sdm670 pinctrl

Quoted 23 lines · Richard Acayan

There is a new driver for the Snapdragon 670 TLMM (Top-Level Mode Multiplexer). Document it.

Adapted from qcom,sm6350-pinctrl.yaml.

---
 .../bindings/pinctrl/qcom,sdm670-tlmm.yaml    | 129 ++++++++++++++++++
 1 file changed, 129 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,sdm670-tlmm.yaml
Documentation/devicetree/bindings/pinctrl/qcom,sdm670-tlmm.yaml new file +4 -0
new file mode 100644
index 000000000000..e2f563ae6bbf
@@ -0,0 +1,129 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pinctrl/qcom,sdm670-tlmm.yaml#

Documentation/devicetree/bindings/pinctrl/qcom,sdm670-tlmm.yaml line 14

Quoted 10 lines
Documentation/devicetree/bindings/pinctrl/qcom,sdm670-tlmm.yaml new file +10 -0
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Technologies, Inc. SDM670 TLMM block
+
+maintainers:
+  - Richard Acayan <[email protected]>
+
+description: |
+  This binding describes the Top Level Mode Multiplexer (TLMM) block found
+  in the SDM670 platform.

Drop "This bindings describes the"

Documentation/devicetree/bindings/pinctrl/qcom,sdm670-tlmm.yaml line 17

Quoted 3 lines
Documentation/devicetree/bindings/pinctrl/qcom,sdm670-tlmm.yaml new file +3 -0
+
+allOf:
+  - $ref: pinctrl.yaml#

Drop.

Documentation/devicetree/bindings/pinctrl/qcom,sdm670-tlmm.yaml line 29

Quoted 12 lines
Documentation/devicetree/bindings/pinctrl/qcom,sdm670-tlmm.yaml new file +12 -0
+  - $ref: /schemas/pinctrl/qcom,tlmm-common.yaml#
+
+properties:
+  compatible:
+    const: qcom,sdm670-tlmm
+
+  reg:
+    maxItems: 1
+
+  interrupts: true
+  interrupt-controller: true
+  '#interrupt-cells': true

Use consistent " everywhere.

Best regards,
Krzysztof

#3 lore raw

PATCH v8 2/3 pinctrl: qcom: do not reinitialize gpio valid mask

In reply to Richard Acayan

It may be necessary for some devices to specify reserved gpios in the device-specific DTS, in addition to the reserved gpios common to all devices with a given SoC. Remove this bitmap_fill() call so that the settings applied to the gpio valid mask by DTS are not overridden by the driver's reserved gpios.

---
 drivers/pinctrl/qcom/pinctrl-msm.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/pinctrl/qcom/pinctrl-msm.c +1 -2
index a2abfe987ab1..f697e9f64360 100644
@@ -687,9 +687,8 @@ static int msm_gpio_init_valid_mask(struct gpio_chip *gc,
 	const int *reserved = pctrl->soc->reserved_gpios;
 	u16 *tmp;
 
-	/* Driver provided reserved list overrides DT and ACPI */
+	/* Driver provided reserved list overrides other settings here */
 	if (reserved) {
-		bitmap_fill(valid_mask, ngpios);
 		for (i = 0; reserved[i] >= 0; i++) {
 			if (i >= ngpios || reserved[i] >= ngpios) {
 				dev_err(pctrl->dev, "invalid list of reserved GPIOs\n");
Signature · 1 line
2.38.0
maintainer Reviewed-by on v8 2/3
#4 lore raw

Re: [PATCH v8 2/3] pinctrl: qcom: do not reinitialize gpio valid mask

Quoted 16 lines · Richard Acayan

It may be necessary for some devices to specify reserved gpios in the device-specific DTS, in addition to the reserved gpios common to all devices with a given SoC. Remove this bitmap_fill() call so that the settings applied to the gpio valid mask by DTS are not overridden by the driver's reserved gpios.

---
 drivers/pinctrl/qcom/pinctrl-msm.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/pinctrl/qcom/pinctrl-msm.c +0 -0
index a2abfe987ab1..f697e9f64360 100644

drivers/pinctrl/qcom/pinctrl-msm.c static int msm_gpio_init_valid_mask(struct gpio_chip *gc, · line 690

Quoted 5 lines
drivers/pinctrl/qcom/pinctrl-msm.c +1 -1
@@ -687,9 +687,8 @@ static int msm_gpio_init_valid_mask(struct gpio_chip *gc,
 	const int *reserved = pctrl->soc->reserved_gpios;
 	u16 *tmp;
 
-	/* Driver provided reserved list overrides DT and ACPI */
+	/* Driver provided reserved list overrides other settings here */

IMHO this no longer overrides things, how about changing it to:

	/* Remove driver-provided reserved GPIOs from valid_mask */

The rest of the change looks good and the commit message looks good. So please consider updating the comment and feel free to add my

Regards,
Bjorn

Quoted 8 lines
drivers/pinctrl/qcom/pinctrl-msm.c +0 -1
 	if (reserved) {
-		bitmap_fill(valid_mask, ngpios);
 		for (i = 0; reserved[i] >= 0; i++) {
 			if (i >= ngpios || reserved[i] >= ngpios) {
 				dev_err(pctrl->dev, "invalid list of reserved GPIOs\n");
Signature · 1 line
2.38.0
#5 lore raw

Re: [PATCH v8 2/3] pinctrl: qcom: do not reinitialize gpio valid mask

Quoted 20 lines · Richard Acayan
Quoted 19 lines

It may be necessary for some devices to specify reserved gpios in the device-specific DTS, in addition to the reserved gpios common to all devices with a given SoC. Remove this bitmap_fill() call so that the settings applied to the gpio valid mask by DTS are not overridden by the driver's reserved gpios.

---
 drivers/pinctrl/qcom/pinctrl-msm.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/pinctrl/qcom/pinctrl-msm.c +0 -0
index a2abfe987ab1..f697e9f64360 100644
@@ -687,9 +687,8 @@ static int msm_gpio_init_valid_mask(struct gpio_chip *gc,
 	const int *reserved = pctrl->soc->reserved_gpios;
 	u16 *tmp;
 
Quoted 4 lines

drivers/pinctrl/qcom/pinctrl-msm.c static int msm_gpio_init_valid_mask(struct gpio_chip *gc, · line 690

Quoted 2 lines
drivers/pinctrl/qcom/pinctrl-msm.c +1 -1
-	/* Driver provided reserved list overrides DT and ACPI */
+	/* Driver provided reserved list overrides other settings here */

IMHO this no longer overrides things, how about changing it to:

Technically, it doesn't. It skips the ACPI settings entirely:

	if (reserved) {
		for (i = 0; reserved[i] >= 0; i++) {
			if (i >= ngpios || reserved[i] >= ngpios) {
				dev_err(pctrl->dev, "invalid list of reserved GPIOs\n");
				return -EINVAL;
			}
			clear_bit(reserved[i], valid_mask);
		}

		return 0;
	}

	/* The number of GPIOs in the ACPI tables */
	len = ret = device_property_count_u16(pctrl->dev, "gpios");
	if (ret < 0)
		return 0;
Quoted 2 lines
	/* Remove driver-provided reserved GPIOs from valid_mask */

Will do. Let me know if you think the above information changes what the comment should be.

Quoted 18 lines

The rest of the change looks good and the commit message looks good. So please consider updating the comment and feel free to add my

Regards,
Bjorn

Quoted 8 lines
drivers/pinctrl/qcom/pinctrl-msm.c +0 -1
 	if (reserved) {
-		bitmap_fill(valid_mask, ngpios);
 		for (i = 0; reserved[i] >= 0; i++) {
 			if (i >= ngpios || reserved[i] >= ngpios) {
 				dev_err(pctrl->dev, "invalid list of reserved GPIOs\n");
Signature · 1 line
2.38.0
#6 lore raw

PATCH v8 3/3 pinctrl: qcom: add sdm670 pinctrl

In reply to Richard Acayan

The Snapdragon 670 has a Top-Level Mode Multiplexer (TLMM) for various features. Add a driver to support it.

---
 drivers/pinctrl/qcom/Kconfig          |    9 +
 drivers/pinctrl/qcom/Makefile         |    1 +
 drivers/pinctrl/qcom/pinctrl-sdm670.c | 1345 +++++++++++++++++++++++++
 3 files changed, 1355 insertions(+)
 create mode 100644 drivers/pinctrl/qcom/pinctrl-sdm670.c
drivers/pinctrl/qcom/Kconfig +9 -0
index 2961b5eb8e10..62187c5597d4 100644
@@ -283,6 +283,15 @@ config PINCTRL_SDM660
 	 Qualcomm Technologies Inc TLMM block found on the Qualcomm
 	 Technologies Inc SDM660 platform.
 
+config PINCTRL_SDM670
+	tristate "Qualcomm Technologies Inc SDM670 pin controller driver"
+	depends on OF
+	depends on PINCTRL_MSM
+	help
+	 This is the pinctrl, pinmux, pinconf and gpiolib driver for the
+	 Qualcomm Technologies Inc TLMM block found on the Qualcomm
+	 Technologies Inc SDM670 platform.
+
 config PINCTRL_SDM845
 	tristate "Qualcomm Technologies Inc SDM845 pin controller driver"
 	depends on (OF || ACPI)
drivers/pinctrl/qcom/Makefile +1 -0
index 8269a1db8794..a5c40f552e5c 100644
@@ -33,6 +33,7 @@ obj-$(CONFIG_PINCTRL_SC7280_LPASS_LPI) += pinctrl-sc7280-lpass-lpi.o
 obj-$(CONFIG_PINCTRL_SC8180X)	+= pinctrl-sc8180x.o
 obj-$(CONFIG_PINCTRL_SC8280XP)	+= pinctrl-sc8280xp.o
 obj-$(CONFIG_PINCTRL_SDM660)   += pinctrl-sdm660.o
+obj-$(CONFIG_PINCTRL_SDM670) += pinctrl-sdm670.o
 obj-$(CONFIG_PINCTRL_SDM845) += pinctrl-sdm845.o
 obj-$(CONFIG_PINCTRL_SDX55) += pinctrl-sdx55.o
 obj-$(CONFIG_PINCTRL_SM6115) += pinctrl-sm6115.o
drivers/pinctrl/qcom/pinctrl-sdm670.c new file +1345 -0

1,345 lines not on this page · raw message

Signature · 1 line
2.38.0