* [PATCH 0/2] pinctrl/arm64: qcom: convert sdm845 bindings to DT schema
@ 2022-09-30 20:05 Krzysztof Kozlowski
2022-09-30 20:05 ` [PATCH 1/2] arm64: dts: qcom: sdm845: align TLMM pin configuration with " Krzysztof Kozlowski
2022-09-30 20:05 ` [PATCH 2/2] dt-bindings: pinctrl: qcom,sdm845: convert to dtschema Krzysztof Kozlowski
0 siblings, 2 replies; 17+ messages in thread
From: Krzysztof Kozlowski @ 2022-09-30 20:05 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Linus Walleij,
Rob Herring, Krzysztof Kozlowski, linux-arm-msm, linux-gpio,
devicetree, linux-kernel
Cc: Krzysztof Kozlowski
Hi,
Overview
========
This is independent patchset of my recent Qualcomm pinctrl fixes.
If the total amount of patches from me around Qualcomm pinctrl is a bit
overwhelming, well, yeah, expect more to come. One way to solve it is to
merge/apply faster than I can create new patches. :-)
Dependencies
============
1. No dependencies.
2. dt-bindings are independent of DTS patches.
Best regards,
Krzysztof
Krzysztof Kozlowski (2):
arm64: dts: qcom: sdm845: align TLMM pin configuration with DT schema
dt-bindings: pinctrl: qcom,sdm845: convert to dtschema
.../bindings/pinctrl/qcom,sdm845-pinctrl.txt | 176 ---------
.../bindings/pinctrl/qcom,sdm845-pinctrl.yaml | 158 ++++++++
arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi | 344 +++++++-----------
arch/arm64/boot/dts/qcom/sdm845-db845c.dts | 76 ++--
.../arm64/boot/dts/qcom/sdm845-lg-common.dtsi | 60 ++-
arch/arm64/boot/dts/qcom/sdm845-lg-judyln.dts | 2 +-
arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 60 ++-
.../boot/dts/qcom/sdm845-oneplus-common.dtsi | 88 ++---
.../boot/dts/qcom/sdm845-shift-axolotl.dts | 138 +++----
.../dts/qcom/sdm845-sony-xperia-tama.dtsi | 6 +-
.../boot/dts/qcom/sdm845-xiaomi-beryllium.dts | 26 +-
.../boot/dts/qcom/sdm845-xiaomi-polaris.dts | 30 +-
arch/arm64/boot/dts/qcom/sdm845.dtsi | 305 +++++++---------
.../boot/dts/qcom/sdm850-lenovo-yoga-c630.dts | 33 +-
.../boot/dts/qcom/sdm850-samsung-w737.dts | 96 ++---
15 files changed, 671 insertions(+), 927 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,sdm845-pinctrl.txt
create mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,sdm845-pinctrl.yaml
--
2.34.1
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/2] arm64: dts: qcom: sdm845: align TLMM pin configuration with DT schema
2022-09-30 20:05 [PATCH 0/2] pinctrl/arm64: qcom: convert sdm845 bindings to DT schema Krzysztof Kozlowski
@ 2022-09-30 20:05 ` Krzysztof Kozlowski
2022-09-30 20:13 ` Konrad Dybcio
2022-10-03 16:14 ` Doug Anderson
2022-09-30 20:05 ` [PATCH 2/2] dt-bindings: pinctrl: qcom,sdm845: convert to dtschema Krzysztof Kozlowski
1 sibling, 2 replies; 17+ messages in thread
From: Krzysztof Kozlowski @ 2022-09-30 20:05 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Linus Walleij,
Rob Herring, Krzysztof Kozlowski, linux-arm-msm, linux-gpio,
devicetree, linux-kernel
Cc: Krzysztof Kozlowski
DT schema expects TLMM pin configuration nodes to be named with
'-state' suffix and their optional children with '-pins' suffix.
The sdm854.dtsi file defined several pin configuration nodes which are
customized by the boards. Therefore keep a additional "default-pins"
node inside so the boards can add more of configuration nodes. Such
additional configuration nodes always need 'function' property now
(required by DT schema).
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi | 344 +++++++-----------
arch/arm64/boot/dts/qcom/sdm845-db845c.dts | 76 ++--
.../arm64/boot/dts/qcom/sdm845-lg-common.dtsi | 60 ++-
arch/arm64/boot/dts/qcom/sdm845-lg-judyln.dts | 2 +-
arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 60 ++-
.../boot/dts/qcom/sdm845-oneplus-common.dtsi | 88 ++---
.../boot/dts/qcom/sdm845-shift-axolotl.dts | 138 +++----
.../dts/qcom/sdm845-sony-xperia-tama.dtsi | 6 +-
.../boot/dts/qcom/sdm845-xiaomi-beryllium.dts | 26 +-
.../boot/dts/qcom/sdm845-xiaomi-polaris.dts | 30 +-
arch/arm64/boot/dts/qcom/sdm845.dtsi | 305 +++++++---------
.../boot/dts/qcom/sdm850-lenovo-yoga-c630.dts | 33 +-
.../boot/dts/qcom/sdm850-samsung-w737.dts | 96 ++---
13 files changed, 513 insertions(+), 751 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi b/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi
index b5f11fbcc300..3403cdcdd49c 100644
--- a/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi
@@ -993,21 +993,21 @@ &wifi {
/* PINCTRL - additions to nodes defined in sdm845.dtsi */
&qspi_cs0 {
- pinconf {
+ default-pins {
pins = "gpio90";
bias-disable;
};
};
&qspi_clk {
- pinconf {
+ default-pins {
pins = "gpio95";
bias-disable;
};
};
&qspi_data01 {
- pinconf {
+ default-pins {
pins = "gpio91", "gpio92";
/* High-Z when no transfers; nice to park the lines */
@@ -1016,7 +1016,7 @@ pinconf {
};
&qup_i2c3_default {
- pinconf {
+ default-pins {
pins = "gpio41", "gpio42";
drive-strength = <2>;
@@ -1026,7 +1026,7 @@ pinconf {
};
&qup_i2c11_default {
- pinconf {
+ default-pins {
pins = "gpio31", "gpio32";
drive-strength = <2>;
@@ -1036,7 +1036,7 @@ pinconf {
};
&qup_i2c12_default {
- pinconf {
+ default-pins {
pins = "gpio49", "gpio50";
drive-strength = <2>;
@@ -1046,7 +1046,7 @@ pinconf {
};
&qup_i2c14_default {
- pinconf {
+ default-pins {
pins = "gpio33", "gpio34";
drive-strength = <2>;
@@ -1056,7 +1056,7 @@ pinconf {
};
&qup_spi0_default {
- pinconf {
+ default-pins {
pins = "gpio0", "gpio1", "gpio2", "gpio3";
drive-strength = <2>;
bias-disable;
@@ -1064,7 +1064,7 @@ pinconf {
};
&qup_spi5_default {
- pinconf {
+ default-pins {
pins = "gpio85", "gpio86", "gpio87", "gpio88";
drive-strength = <2>;
bias-disable;
@@ -1072,7 +1072,7 @@ pinconf {
};
&qup_spi10_default {
- pinconf {
+ default-pins {
pins = "gpio53", "gpio54", "gpio55", "gpio56";
drive-strength = <2>;
bias-disable;
@@ -1081,28 +1081,25 @@ pinconf {
&qup_uart6_default {
/* Change pinmux to all 4 pins since CTS and RTS are connected */
- pinmux {
- pins = "gpio45", "gpio46",
- "gpio47", "gpio48";
- };
-
- pinconf-cts {
+ cts-pins {
/*
* Configure a pull-down on 45 (CTS) to match the pull of
* the Bluetooth module.
*/
pins = "gpio45";
+ function = "qup6";
bias-pull-down;
};
- pinconf-rts-tx {
+ rts-tx-pins {
/* We'll drive 46 (RTS) and 47 (TX), so no pull */
pins = "gpio46", "gpio47";
+ function = "qup6";
drive-strength = <2>;
bias-disable;
};
- pinconf-rx {
+ rx-pins {
/*
* Configure a pull-up on 48 (RX). This is needed to avoid
* garbage data when the TX pin of the Bluetooth module is
@@ -1110,19 +1107,22 @@ pinconf-rx {
* signal yet).
*/
pins = "gpio48";
+ function = "qup6";
bias-pull-up;
};
};
&qup_uart9_default {
- pinconf-tx {
+ tx-pins {
pins = "gpio4";
+ function = "qup9";
drive-strength = <2>;
bias-disable;
};
- pinconf-rx {
+ rx-pins {
pins = "gpio5";
+ function = "qup9";
drive-strength = <2>;
bias-pull-up;
};
@@ -1213,243 +1213,153 @@ ap-suspend-l-hog {
output-low;
};
- ap_edp_bklten: ap-edp-bklten {
- pinmux {
- pins = "gpio37";
- function = "gpio";
- };
-
- pinconf {
- pins = "gpio37";
- drive-strength = <2>;
- bias-disable;
- };
+ ap_edp_bklten: ap-edp-bklten-state {
+ pins = "gpio37";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-disable;
};
- bios_flash_wp_r_l: bios-flash-wp-r-l {
- pinmux {
- pins = "gpio128";
- function = "gpio";
- input-enable;
- };
-
- pinconf {
- pins = "gpio128";
- bias-disable;
- };
+ bios_flash_wp_r_l: bios-flash-wp-r-l-state {
+ pins = "gpio128";
+ function = "gpio";
+ input-enable;
+ bias-disable;
};
- ec_ap_int_l: ec-ap-int-l {
- pinmux {
- pins = "gpio122";
- function = "gpio";
- input-enable;
- };
-
- pinconf {
- pins = "gpio122";
- bias-pull-up;
- };
+ ec_ap_int_l: ec-ap-int-l-state {
+ pins = "gpio122";
+ function = "gpio";
+ input-enable;
+ bias-pull-up;
};
- edp_brij_en: edp-brij-en {
- pinmux {
- pins = "gpio102";
- function = "gpio";
- };
-
- pinconf {
- pins = "gpio102";
- drive-strength = <2>;
- bias-disable;
- };
+ edp_brij_en: edp-brij-en-state {
+ pins = "gpio102";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-disable;
};
- edp_brij_irq: edp-brij-irq {
- pinmux {
- pins = "gpio10";
- function = "gpio";
- };
-
- pinconf {
- pins = "gpio10";
- drive-strength = <2>;
- bias-pull-down;
- };
+ edp_brij_irq: edp-brij-irq-state {
+ pins = "gpio10";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-pull-down;
};
- en_pp3300_dx_edp: en-pp3300-dx-edp {
- pinmux {
- pins = "gpio43";
- function = "gpio";
- };
-
- pinconf {
- pins = "gpio43";
- drive-strength = <2>;
- bias-disable;
- };
+ en_pp3300_dx_edp: en-pp3300-dx-edp-state {
+ pins = "gpio43";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-disable;
};
- h1_ap_int_odl: h1-ap-int-odl {
- pinmux {
- pins = "gpio129";
- function = "gpio";
- input-enable;
- };
-
- pinconf {
- pins = "gpio129";
- bias-pull-up;
- };
+ h1_ap_int_odl: h1-ap-int-odl-state {
+ pins = "gpio129";
+ function = "gpio";
+ input-enable;
+ bias-pull-up;
};
- pen_eject_odl: pen-eject-odl {
- pinmux {
- pins = "gpio119";
- function = "gpio";
- bias-pull-up;
- };
+ pen_eject_odl: pen-eject-odl-state {
+ pins = "gpio119";
+ function = "gpio";
+ bias-pull-up;
};
- pen_irq_l: pen-irq-l {
- pinmux {
- pins = "gpio24";
- function = "gpio";
- };
-
- pinconf {
- pins = "gpio24";
+ pen_irq_l: pen-irq-l-state {
+ pins = "gpio24";
+ function = "gpio";
- /* Has external pullup */
- bias-disable;
- };
+ /* Has external pullup */
+ bias-disable;
};
- pen_pdct_l: pen-pdct-l {
- pinmux {
- pins = "gpio63";
- function = "gpio";
- };
-
- pinconf {
- pins = "gpio63";
+ pen_pdct_l: pen-pdct-l-state {
+ pins = "gpio63";
+ function = "gpio";
- /* Has external pullup */
- bias-disable;
- };
+ /* Has external pullup */
+ bias-disable;
};
- pen_rst_l: pen-rst-l {
- pinmux {
- pins = "gpio23";
- function = "gpio";
- };
-
- pinconf {
- pins = "gpio23";
- bias-disable;
- drive-strength = <2>;
+ pen_rst_l: pen-rst-l-state {
+ pins = "gpio23";
+ function = "gpio";
+ bias-disable;
+ drive-strength = <2>;
- /*
- * The pen driver doesn't currently support
- * driving this reset line. By specifying
- * output-high here we're relying on the fact
- * that this pin has a default pulldown at boot
- * (which makes sure the pen was in reset if it
- * was powered) and then we set it high here to
- * take it out of reset. Better would be if the
- * pen driver could control this and we could
- * remove "output-high" here.
- */
- output-high;
- };
+ /*
+ * The pen driver doesn't currently support
+ * driving this reset line. By specifying
+ * output-high here we're relying on the fact
+ * that this pin has a default pulldown at boot
+ * (which makes sure the pen was in reset if it
+ * was powered) and then we set it high here to
+ * take it out of reset. Better would be if the
+ * pen driver could control this and we could
+ * remove "output-high" here.
+ */
+ output-high;
};
- sdc2_clk: sdc2-clk {
- pinconf {
- pins = "sdc2_clk";
- bias-disable;
+ sdc2_clk: sdc2-clk-state {
+ pins = "sdc2_clk";
+ bias-disable;
- /*
- * It seems that mmc_test reports errors if drive
- * strength is not 16.
- */
- drive-strength = <16>;
- };
+ /*
+ * It seems that mmc_test reports errors if drive
+ * strength is not 16.
+ */
+ drive-strength = <16>;
};
- sdc2_cmd: sdc2-cmd {
- pinconf {
- pins = "sdc2_cmd";
- bias-pull-up;
- drive-strength = <16>;
- };
+ sdc2_cmd: sdc2-cmd-state {
+ pins = "sdc2_cmd";
+ bias-pull-up;
+ drive-strength = <16>;
};
- sdc2_data: sdc2-data {
- pinconf {
- pins = "sdc2_data";
- bias-pull-up;
- drive-strength = <16>;
- };
+ sdc2_data: sdc2-data-state {
+ pins = "sdc2_data";
+ bias-pull-up;
+ drive-strength = <16>;
};
- sd_cd_odl: sd-cd-odl {
- pinmux {
- pins = "gpio44";
- function = "gpio";
- };
-
- pinconf {
- pins = "gpio44";
- bias-pull-up;
- };
+ sd_cd_odl: sd-cd-odl-state {
+ pins = "gpio44";
+ function = "gpio";
+ bias-pull-up;
};
- ts_int_l: ts-int-l {
- pinmux {
- pins = "gpio125";
- function = "gpio";
- };
-
- pinconf {
- pins = "gpio125";
- bias-pull-up;
- };
+ ts_int_l: ts-int-l-state {
+ pins = "gpio125";
+ function = "gpio";
+ bias-pull-up;
};
- ts_reset_l: ts-reset-l {
- pinmux {
- pins = "gpio118";
- function = "gpio";
- };
-
- pinconf {
- pins = "gpio118";
- bias-disable;
- drive-strength = <2>;
- };
+ ts_reset_l: ts-reset-l-state {
+ pins = "gpio118";
+ function = "gpio";
+ bias-disable;
+ drive-strength = <2>;
};
- ap_suspend_l_assert: ap_suspend_l_assert {
- config {
- pins = "gpio126";
- function = "gpio";
- bias-disable;
- drive-strength = <2>;
- output-low;
- };
+ ap_suspend_l_assert: ap-suspend-l-assert-state {
+ pins = "gpio126";
+ function = "gpio";
+ bias-disable;
+ drive-strength = <2>;
+ output-low;
};
- ap_suspend_l_deassert: ap_suspend_l_deassert {
- config {
- pins = "gpio126";
- function = "gpio";
- bias-disable;
- drive-strength = <2>;
- output-high;
- };
+ ap_suspend_l_deassert: ap-suspend-l-deassert-state {
+ pins = "gpio126";
+ function = "gpio";
+ bias-disable;
+ drive-strength = <2>;
+ output-high;
};
};
diff --git a/arch/arm64/boot/dts/qcom/sdm845-db845c.dts b/arch/arm64/boot/dts/qcom/sdm845-db845c.dts
index a157eab66dee..c5e17a718049 100644
--- a/arch/arm64/boot/dts/qcom/sdm845-db845c.dts
+++ b/arch/arm64/boot/dts/qcom/sdm845-db845c.dts
@@ -820,8 +820,8 @@ &spi2 {
};
&tlmm {
- cam0_default: cam0_default {
- rst {
+ cam0_default: cam0-default-state {
+ rst-pins {
pins = "gpio9";
function = "gpio";
@@ -829,7 +829,7 @@ rst {
bias-disable;
};
- mclk0 {
+ mclk0-pins {
pins = "gpio13";
function = "cam_mclk";
@@ -838,8 +838,8 @@ mclk0 {
};
};
- cam3_default: cam3_default {
- rst {
+ cam3_default: cam3-default-state {
+ rst-pins {
function = "gpio";
pins = "gpio21";
@@ -847,7 +847,7 @@ rst {
bias-disable;
};
- mclk3 {
+ mclk3-pins {
function = "cam_mclk";
pins = "gpio16";
@@ -856,7 +856,7 @@ mclk3 {
};
};
- dsi_sw_sel: dsi-sw-sel {
+ dsi_sw_sel: dsi-sw-sel-state {
pins = "gpio120";
function = "gpio";
@@ -865,20 +865,20 @@ dsi_sw_sel: dsi-sw-sel {
output-high;
};
- lt9611_irq_pin: lt9611-irq {
+ lt9611_irq_pin: lt9611-irq-state {
pins = "gpio84";
function = "gpio";
bias-disable;
};
- pcie0_default_state: pcie0-default {
- clkreq {
+ pcie0_default_state: pcie0-default-state {
+ clkreq-pins {
pins = "gpio36";
function = "pci_e0";
bias-pull-up;
};
- reset-n {
+ reset-n-pins {
pins = "gpio35";
function = "gpio";
@@ -887,7 +887,7 @@ reset-n {
bias-pull-down;
};
- wake-n {
+ wake-n-pins {
pins = "gpio37";
function = "gpio";
@@ -896,7 +896,7 @@ wake-n {
};
};
- pcie0_pwren_state: pcie0-pwren {
+ pcie0_pwren_state: pcie0-pwren-state {
pins = "gpio90";
function = "gpio";
@@ -904,8 +904,8 @@ pcie0_pwren_state: pcie0-pwren {
bias-disable;
};
- pcie1_default_state: pcie1-default {
- perst-n {
+ pcie1_default_state: pcie1-default-state {
+ perst-n-pins {
pins = "gpio102";
function = "gpio";
@@ -913,13 +913,13 @@ perst-n {
bias-disable;
};
- clkreq {
+ clkreq-pins {
pins = "gpio103";
function = "pci_e1";
bias-pull-up;
};
- wake-n {
+ wake-n-pins {
pins = "gpio11";
function = "gpio";
@@ -927,7 +927,7 @@ wake-n {
bias-pull-up;
};
- reset-n {
+ reset-n-pins {
pins = "gpio75";
function = "gpio";
@@ -937,8 +937,8 @@ reset-n {
};
};
- sdc2_default_state: sdc2-default {
- clk {
+ sdc2_default_state: sdc2-default-state {
+ clk-pins {
pins = "sdc2_clk";
bias-disable;
@@ -949,26 +949,26 @@ clk {
drive-strength = <16>;
};
- cmd {
+ cmd-pins {
pins = "sdc2_cmd";
bias-pull-up;
drive-strength = <10>;
};
- data {
+ data-pins {
pins = "sdc2_data";
bias-pull-up;
drive-strength = <10>;
};
};
- sdc2_card_det_n: sd-card-det-n {
+ sdc2_card_det_n: sd-card-det-n-state {
pins = "gpio126";
function = "gpio";
bias-pull-up;
};
- wcd_intr_default: wcd_intr_default {
+ wcd_intr_default: wcd-intr-default-state {
pins = <54>;
function = "gpio";
@@ -1123,20 +1123,20 @@ &wifi {
/* PINCTRL - additions to nodes defined in sdm845.dtsi */
&qup_spi2_default {
- pinmux {
+ default-pins {
drive-strength = <16>;
};
};
&qup_uart3_default{
- pinmux {
+ default-pins {
pins = "gpio41", "gpio42", "gpio43", "gpio44";
function = "qup3";
};
};
&qup_i2c10_default {
- pinconf {
+ default-pins {
pins = "gpio55", "gpio56";
drive-strength = <2>;
bias-disable;
@@ -1144,37 +1144,37 @@ pinconf {
};
&qup_uart6_default {
- pinmux {
- pins = "gpio45", "gpio46", "gpio47", "gpio48";
- function = "qup6";
- };
-
- cts {
+ cts-pins {
pins = "gpio45";
+ function = "qup6";
bias-disable;
};
- rts-tx {
+ rts-tx-pins {
pins = "gpio46", "gpio47";
+ function = "qup6";
drive-strength = <2>;
bias-disable;
};
- rx {
+ rx-pins {
pins = "gpio48";
+ function = "qup6";
bias-pull-up;
};
};
&qup_uart9_default {
- pinconf-tx {
+ tx-pins {
pins = "gpio4";
+ function = "qup9";
drive-strength = <2>;
bias-disable;
};
- pinconf-rx {
+ rx-pins {
pins = "gpio5";
+ function = "qup9";
drive-strength = <2>;
bias-pull-up;
};
@@ -1285,7 +1285,7 @@ ov7251_ep: endpoint {
/* PINCTRL - additions to nodes defined in sdm845.dtsi */
&qup_spi0_default {
- config {
+ default-pins {
drive-strength = <6>;
bias-disable;
};
diff --git a/arch/arm64/boot/dts/qcom/sdm845-lg-common.dtsi b/arch/arm64/boot/dts/qcom/sdm845-lg-common.dtsi
index 1eb423e4be24..3ccfbdb2880e 100644
--- a/arch/arm64/boot/dts/qcom/sdm845-lg-common.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845-lg-common.dtsi
@@ -558,48 +558,36 @@ &usb_1_qmpphy {
&tlmm {
gpio-reserved-ranges = <28 4>, <81 4>;
- sdc2_clk: sdc2-clk {
- pinconf {
- pins = "sdc2_clk";
- bias-disable;
-
- /*
- * It seems that mmc_test reports errors if drive
- * strength is not 16 on clk, cmd, and data pins.
- *
- * TODO: copy-pasted from mtp, try other values
- * on these devices.
- */
- drive-strength = <16>;
- };
+ sdc2_clk: sdc2-clk-state {
+ pins = "sdc2_clk";
+ bias-disable;
+
+ /*
+ * It seems that mmc_test reports errors if drive
+ * strength is not 16 on clk, cmd, and data pins.
+ *
+ * TODO: copy-pasted from mtp, try other values
+ * on these devices.
+ */
+ drive-strength = <16>;
};
- sdc2_cmd: sdc2-cmd {
- pinconf {
- pins = "sdc2_cmd";
- bias-pull-up;
- drive-strength = <16>;
- };
+ sdc2_cmd: sdc2-cmd-state {
+ pins = "sdc2_cmd";
+ bias-pull-up;
+ drive-strength = <16>;
};
- sdc2_data: sdc2-data {
- pinconf {
- pins = "sdc2_data";
- bias-pull-up;
- drive-strength = <16>;
- };
+ sdc2_data: sdc2-data-state {
+ pins = "sdc2_data";
+ bias-pull-up;
+ drive-strength = <16>;
};
- sd_card_det_n: sd-card-det-n {
- pinmux {
- pins = "gpio126";
- function = "gpio";
- };
-
- pinconf {
- pins = "gpio126";
- bias-pull-up;
- };
+ sd_card_det_n: sd-card-det-n-state {
+ pins = "gpio126";
+ function = "gpio";
+ bias-pull-up;
};
};
diff --git a/arch/arm64/boot/dts/qcom/sdm845-lg-judyln.dts b/arch/arm64/boot/dts/qcom/sdm845-lg-judyln.dts
index 7d967a104b3e..a12723310c8b 100644
--- a/arch/arm64/boot/dts/qcom/sdm845-lg-judyln.dts
+++ b/arch/arm64/boot/dts/qcom/sdm845-lg-judyln.dts
@@ -58,7 +58,7 @@ &mss_pil {
};
&tlmm {
- thinq_key_default: thinq-key-default {
+ thinq_key_default: thinq-key-default-state {
pins = "gpio89";
function = "gpio";
diff --git a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
index de2d10e0315a..b36befe60b08 100644
--- a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
+++ b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
@@ -719,7 +719,7 @@ &wifi {
/* PINCTRL - additions to nodes defined in sdm845.dtsi */
&qup_i2c10_default {
- pinconf {
+ default-pins {
pins = "gpio55", "gpio56";
drive-strength = <2>;
bias-disable;
@@ -727,14 +727,16 @@ pinconf {
};
&qup_uart9_default {
- pinconf-tx {
+ tx-pins {
pins = "gpio4";
+ function = "qup9";
drive-strength = <2>;
bias-disable;
};
- pinconf-rx {
+ rx-pins {
pins = "gpio5";
+ function = "qup9";
drive-strength = <2>;
bias-pull-up;
};
@@ -743,44 +745,32 @@ pinconf-rx {
&tlmm {
gpio-reserved-ranges = <0 4>, <81 4>;
- sdc2_clk: sdc2-clk {
- pinconf {
- pins = "sdc2_clk";
- bias-disable;
+ sdc2_clk: sdc2-clk-state {
+ pins = "sdc2_clk";
+ bias-disable;
- /*
- * It seems that mmc_test reports errors if drive
- * strength is not 16 on clk, cmd, and data pins.
- */
- drive-strength = <16>;
- };
+ /*
+ * It seems that mmc_test reports errors if drive
+ * strength is not 16 on clk, cmd, and data pins.
+ */
+ drive-strength = <16>;
};
- sdc2_cmd: sdc2-cmd {
- pinconf {
- pins = "sdc2_cmd";
- bias-pull-up;
- drive-strength = <16>;
- };
+ sdc2_cmd: sdc2-cmd-state {
+ pins = "sdc2_cmd";
+ bias-pull-up;
+ drive-strength = <16>;
};
- sdc2_data: sdc2-data {
- pinconf {
- pins = "sdc2_data";
- bias-pull-up;
- drive-strength = <16>;
- };
+ sdc2_data: sdc2-data-state {
+ pins = "sdc2_data";
+ bias-pull-up;
+ drive-strength = <16>;
};
- sd_card_det_n: sd-card-det-n {
- pinmux {
- pins = "gpio126";
- function = "gpio";
- };
-
- pinconf {
- pins = "gpio126";
- bias-pull-up;
- };
+ sd_card_det_n: sd-card-det-n-state {
+ pins = "gpio126";
+ function = "gpio";
+ bias-pull-up;
};
};
diff --git a/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi b/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
index 392461c29e76..02a1e056f465 100644
--- a/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
@@ -470,7 +470,7 @@ &qupv3_id_0 {
};
&qup_i2c12_default {
- mux {
+ default-pins {
pins = "gpio49", "gpio50";
function = "qup12";
drive-strength = <2>;
@@ -479,7 +479,7 @@ mux {
};
&qup_i2c10_default {
- pinconf {
+ default-pins {
pins = "gpio55", "gpio56";
drive-strength = <2>;
bias-disable;
@@ -487,14 +487,16 @@ pinconf {
};
&qup_uart9_default {
- pinconf-tx {
+ tx-pins {
pins = "gpio4";
+ function = "qup9";
drive-strength = <2>;
bias-disable;
};
- pinconf-rx {
+ rx-pins {
pins = "gpio5";
+ function = "qup9";
drive-strength = <2>;
bias-pull-up;
};
@@ -504,24 +506,22 @@ pinconf-rx {
* Prevent garbage data on bluetooth UART lines
*/
&qup_uart6_default {
- pinmux {
- pins = "gpio45", "gpio46", "gpio47", "gpio48";
- function = "qup6";
- };
-
- cts {
+ cts-pins {
pins = "gpio45";
+ function = "qup6";
bias-pull-down;
};
- rts-tx {
+ rts-tx-pins {
pins = "gpio46", "gpio47";
+ function = "qup6";
drive-strength = <2>;
bias-disable;
};
- rx {
+ rx-pins {
pins = "gpio48";
+ function = "qup6";
bias-pull-up;
};
};
@@ -607,51 +607,41 @@ &usb_1_hsphy {
&tlmm {
gpio-reserved-ranges = <0 4>, <81 4>;
- tri_state_key_default: tri_state_key_default {
- mux {
- pins = "gpio40", "gpio42", "gpio26";
- function = "gpio";
- drive-strength = <2>;
- bias-disable;
- };
+ tri_state_key_default: tri-state-key-default-state {
+ pins = "gpio40", "gpio42", "gpio26";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-disable;
};
- ts_default_pins: ts-int {
- mux {
- pins = "gpio99", "gpio125";
- function = "gpio";
- drive-strength = <16>;
- bias-pull-up;
- };
+ ts_default_pins: ts-int-state {
+ pins = "gpio99", "gpio125";
+ function = "gpio";
+ drive-strength = <16>;
+ bias-pull-up;
};
- panel_reset_pins: panel-reset {
- mux {
- pins = "gpio6", "gpio25", "gpio26";
- function = "gpio";
- drive-strength = <8>;
- bias-disable;
- };
+ panel_reset_pins: panel-reset-state {
+ pins = "gpio6", "gpio25", "gpio26";
+ function = "gpio";
+ drive-strength = <8>;
+ bias-disable;
};
- panel_te_pin: panel-te {
- mux {
- pins = "gpio10";
- function = "mdp_vsync";
- drive-strength = <2>;
- bias-disable;
- input-enable;
- };
+ panel_te_pin: panel-te-state {
+ pins = "gpio10";
+ function = "mdp_vsync";
+ drive-strength = <2>;
+ bias-disable;
+ input-enable;
};
- panel_esd_pin: panel-esd {
- mux {
- pins = "gpio30";
- function = "gpio";
- drive-strength = <2>;
- bias-pull-down;
- input-enable;
- };
+ panel_esd_pin: panel-esd-state {
+ pins = "gpio30";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-pull-down;
+ input-enable;
};
};
diff --git a/arch/arm64/boot/dts/qcom/sdm845-shift-axolotl.dts b/arch/arm64/boot/dts/qcom/sdm845-shift-axolotl.dts
index 83261c9bb4f2..7c752f621a23 100644
--- a/arch/arm64/boot/dts/qcom/sdm845-shift-axolotl.dts
+++ b/arch/arm64/boot/dts/qcom/sdm845-shift-axolotl.dts
@@ -559,14 +559,16 @@ led@5 {
};
&qup_uart9_default {
- pinconf-rx {
+ rx-pins {
pins = "gpio5";
+ function = "qup9";
drive-strength = <2>;
bias-pull-up;
};
- pinconf-tx {
+ tx-pins {
pins = "gpio4";
+ function = "qup9";
drive-strength = <2>;
bias-disable;
};
@@ -583,110 +585,62 @@ &qupv3_id_1 {
&tlmm {
gpio-reserved-ranges = <0 4>, <81 4>;
- sde_dsi_active: sde-dsi-active {
- mux {
- pins = "gpio6", "gpio11";
- function = "gpio";
- };
-
- config {
- pins = "gpio6", "gpio11";
- drive-strength = <8>;
- bias-disable;
- };
+ sde_dsi_active: sde-dsi-active-state {
+ pins = "gpio6", "gpio11";
+ function = "gpio";
+ drive-strength = <8>;
+ bias-disable;
};
- sde_dsi_suspend: sde-dsi-suspend {
- mux {
- pins = "gpio6", "gpio11";
- function = "gpio";
- };
-
- config {
- pins = "gpio6", "gpio11";
- drive-strength = <2>;
- bias-pull-down;
- };
+ sde_dsi_suspend: sde-dsi-suspend-state {
+ pins = "gpio6", "gpio11";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-pull-down;
};
- sde_te_active: sde-te-active {
- mux {
- pins = "gpio10";
- function = "mdp_vsync";
- };
-
- config {
- pins = "gpio10";
- drive-strength = <2>;
- bias-pull-down;
- };
+ sde_te_active: sde-te-active-state {
+ pins = "gpio10";
+ function = "mdp_vsync";
+ drive-strength = <2>;
+ bias-pull-down;
};
- sde_te_suspend: sde-te-suspend {
- mux {
- pins = "gpio10";
- function = "mdp_vsync";
- };
-
- config {
- pins = "gpio10";
- drive-strength = <2>;
- bias-pull-down;
- };
+ sde_te_suspend: sde-te-suspend-state {
+ pins = "gpio10";
+ function = "mdp_vsync";
+ drive-strength = <2>;
+ bias-pull-down;
};
- ts_int_active: ts-int-active {
- mux {
- pins = "gpio125";
- function = "gpio";
- };
-
- config {
- pins = "gpio125";
- drive-strength = <8>;
- bias-pull-up;
- input-enable;
- };
+ ts_int_active: ts-int-active-state {
+ pins = "gpio125";
+ function = "gpio";
+ drive-strength = <8>;
+ bias-pull-up;
+ input-enable;
};
- ts_int_suspend: ts-int-suspend {
- mux {
- pins = "gpio125";
- function = "gpio";
- };
-
- config {
- pins = "gpio125";
- drive-strength = <2>;
- bias-pull-down;
- input-enable;
- };
+ ts_int_suspend: ts-int-suspend-state {
+ pins = "gpio125";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-pull-down;
+ input-enable;
};
- ts_reset_active: ts-reset-active {
- mux {
- pins = "gpio99";
- function = "gpio";
- };
-
- config {
- pins = "gpio99";
- drive-strength = <8>;
- bias-pull-up;
- };
+ ts_reset_active: ts-reset-active-state {
+ pins = "gpio99";
+ function = "gpio";
+ drive-strength = <8>;
+ bias-pull-up;
};
- ts_reset_suspend: ts-reset-suspend {
- mux {
- pins = "gpio99";
- function = "gpio";
- };
-
- config {
- pins = "gpio99";
- drive-strength = <2>;
- bias-pull-down;
- };
+ ts_reset_suspend: ts-reset-suspend-state {
+ pins = "gpio99";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-pull-down;
};
};
diff --git a/arch/arm64/boot/dts/qcom/sdm845-sony-xperia-tama.dtsi b/arch/arm64/boot/dts/qcom/sdm845-sony-xperia-tama.dtsi
index 51ee42e3c995..3a920275dda3 100644
--- a/arch/arm64/boot/dts/qcom/sdm845-sony-xperia-tama.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845-sony-xperia-tama.dtsi
@@ -383,19 +383,19 @@ &tlmm {
gpio-reserved-ranges = <0 4>, <81 4>;
sdc2_default_state: sdc2-default-state {
- clk {
+ clk-pins {
pins = "sdc2_clk";
drive-strength = <16>;
bias-disable;
};
- cmd {
+ cmd-pins {
pins = "sdc2_cmd";
drive-strength = <10>;
bias-pull-up;
};
- data {
+ data-pins {
pins = "sdc2_data";
drive-strength = <10>;
bias-pull-up;
diff --git a/arch/arm64/boot/dts/qcom/sdm845-xiaomi-beryllium.dts b/arch/arm64/boot/dts/qcom/sdm845-xiaomi-beryllium.dts
index 0f470cf1ed1c..d6392f3486ae 100644
--- a/arch/arm64/boot/dts/qcom/sdm845-xiaomi-beryllium.dts
+++ b/arch/arm64/boot/dts/qcom/sdm845-xiaomi-beryllium.dts
@@ -442,33 +442,33 @@ codec {
&tlmm {
gpio-reserved-ranges = <0 4>, <81 4>;
- sdc2_default_state: sdc2-default {
- clk {
+ sdc2_default_state: sdc2-default-state {
+ clk-pins {
pins = "sdc2_clk";
bias-disable;
drive-strength = <16>;
};
- cmd {
+ cmd-pins {
pins = "sdc2_cmd";
bias-pull-up;
drive-strength = <10>;
};
- data {
+ data-pins {
pins = "sdc2_data";
bias-pull-up;
drive-strength = <10>;
};
};
- sdc2_card_det_n: sd-card-det-n {
+ sdc2_card_det_n: sd-card-det-n-state {
pins = "gpio126";
function = "gpio";
bias-pull-up;
};
- wcd_intr_default: wcd_intr_default {
+ wcd_intr_default: wcd-intr-default-state {
pins = <54>;
function = "gpio";
@@ -571,24 +571,22 @@ &wifi {
/* PINCTRL - additions to nodes defined in sdm845.dtsi */
&qup_uart6_default {
- pinmux {
- pins = "gpio45", "gpio46", "gpio47", "gpio48";
- function = "qup6";
- };
-
- cts {
+ cts-pins {
pins = "gpio45";
+ function = "qup6";
bias-disable;
};
- rts-tx {
+ rts-tx-pins {
pins = "gpio46", "gpio47";
+ function = "qup6";
drive-strength = <2>;
bias-disable;
};
- rx {
+ rx-pins {
pins = "gpio48";
+ function = "qup6";
bias-pull-up;
};
};
diff --git a/arch/arm64/boot/dts/qcom/sdm845-xiaomi-polaris.dts b/arch/arm64/boot/dts/qcom/sdm845-xiaomi-polaris.dts
index f98259489679..9a4e1ff1d3ba 100644
--- a/arch/arm64/boot/dts/qcom/sdm845-xiaomi-polaris.dts
+++ b/arch/arm64/boot/dts/qcom/sdm845-xiaomi-polaris.dts
@@ -573,7 +573,7 @@ &qupv3_id_1 {
};
&qup_i2c14_default {
- pinconf {
+ default-pins {
pins = "gpio33", "gpio34";
drive-strength = <2>;
bias-disable;
@@ -583,14 +583,14 @@ pinconf {
&tlmm {
gpio-reserved-ranges = <0 4>, <81 4>;
- ts_reset_default: ts-reset-default {
+ ts_reset_default: ts-reset-default-state {
pins = "gpio99";
function = "gpio";
drive-strength = <16>;
output-high;
};
- ts_int_default: ts-int-default {
+ ts_int_default: ts-int-default-state {
pins = "gpio125";
function = "gpio";
bias-pull-down;
@@ -598,14 +598,14 @@ ts_int_default: ts-int-default {
input-enable;
};
- ts_reset_sleep: ts-reset-sleep {
+ ts_reset_sleep: ts-reset-sleep-state {
pins = "gpio99";
function = "gpio";
bias-disable;
drive-strength = <2>;
};
- ts_int_sleep: ts-int-sleep {
+ ts_int_sleep: ts-int-sleep-state {
pins = "gpio125";
function = "gpio";
bias-pull-down;
@@ -613,21 +613,21 @@ ts_int_sleep: ts-int-sleep {
input-enable;
};
- sde_dsi_active: sde-dsi-active {
+ sde_dsi_active: sde-dsi-active-state {
pins = "gpio6", "gpio10";
function = "gpio";
drive-strength = <8>;
bias-disable;
};
- sde_dsi_suspend: sde-dsi-suspend {
+ sde_dsi_suspend: sde-dsi-suspend-state {
pins = "gpio6", "gpio10";
function = "gpio";
drive-strength = <2>;
bias-pull-down;
};
- wcd_intr_default: wcd-intr-default {
+ wcd_intr_default: wcd-intr-default-state {
pins = "gpio54";
function = "gpio";
input-enable;
@@ -739,24 +739,22 @@ &wifi {
/* PINCTRL - additions to nodes defined in sdm845.dtsi */
&qup_uart6_default {
- pinmux {
- pins = "gpio45", "gpio46", "gpio47", "gpio48";
- function = "qup6";
- };
-
- cts {
+ cts-pins {
pins = "gpio45";
+ function = "qup6";
bias-disable;
};
- rts-tx {
+ rts-tx-pins {
pins = "gpio46", "gpio47";
+ function = "qup6";
drive-strength = <2>;
bias-disable;
};
- rx {
+ rx-pins {
pins = "gpio48";
+ function = "qup6";
bias-pull-up;
};
};
diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index d761da47220d..915ef8dda023 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -2641,7 +2641,7 @@ tlmm: pinctrl@3400000 {
gpio-ranges = <&tlmm 0 0 151>;
wakeup-parent = <&pdc_intc>;
- cci0_default: cci0-default {
+ cci0_default: cci0-default-state {
/* SDA, SCL */
pins = "gpio17", "gpio18";
function = "cci_i2c";
@@ -2650,7 +2650,7 @@ cci0_default: cci0-default {
drive-strength = <2>; /* 2 mA */
};
- cci0_sleep: cci0-sleep {
+ cci0_sleep: cci0-sleep-state {
/* SDA, SCL */
pins = "gpio17", "gpio18";
function = "cci_i2c";
@@ -2659,7 +2659,7 @@ cci0_sleep: cci0-sleep {
bias-pull-down;
};
- cci1_default: cci1-default {
+ cci1_default: cci1-default-state {
/* SDA, SCL */
pins = "gpio19", "gpio20";
function = "cci_i2c";
@@ -2668,7 +2668,7 @@ cci1_default: cci1-default {
drive-strength = <2>; /* 2 mA */
};
- cci1_sleep: cci1-sleep {
+ cci1_sleep: cci1-sleep-state {
/* SDA, SCL */
pins = "gpio19", "gpio20";
function = "cci_i2c";
@@ -2677,531 +2677,486 @@ cci1_sleep: cci1-sleep {
bias-pull-down;
};
- qspi_clk: qspi-clk {
- pinmux {
+ qspi_clk: qspi-clk-state {
+ default-pins {
pins = "gpio95";
function = "qspi_clk";
};
};
- qspi_cs0: qspi-cs0 {
- pinmux {
+ qspi_cs0: qspi-cs0-state {
+ default-pins {
pins = "gpio90";
function = "qspi_cs";
};
};
- qspi_cs1: qspi-cs1 {
- pinmux {
+ qspi_cs1: qspi-cs1-state {
+ default-pins {
pins = "gpio89";
function = "qspi_cs";
};
};
- qspi_data01: qspi-data01 {
- pinmux-data {
+ qspi_data01: qspi-data01-state {
+ default-pins {
pins = "gpio91", "gpio92";
function = "qspi_data";
};
};
- qspi_data12: qspi-data12 {
- pinmux-data {
+ qspi_data12: qspi-data12-state {
+ default-pins {
pins = "gpio93", "gpio94";
function = "qspi_data";
};
};
- qup_i2c0_default: qup-i2c0-default {
- pinmux {
+ qup_i2c0_default: qup-i2c0-default-state {
+ default-pins {
pins = "gpio0", "gpio1";
function = "qup0";
};
};
- qup_i2c1_default: qup-i2c1-default {
- pinmux {
+ qup_i2c1_default: qup-i2c1-default-state {
+ default-pins {
pins = "gpio17", "gpio18";
function = "qup1";
};
};
- qup_i2c2_default: qup-i2c2-default {
- pinmux {
+ qup_i2c2_default: qup-i2c2-default-state {
+ default-pins {
pins = "gpio27", "gpio28";
function = "qup2";
};
};
- qup_i2c3_default: qup-i2c3-default {
- pinmux {
+ qup_i2c3_default: qup-i2c3-default-state {
+ default-pins {
pins = "gpio41", "gpio42";
function = "qup3";
};
};
- qup_i2c4_default: qup-i2c4-default {
- pinmux {
+ qup_i2c4_default: qup-i2c4-default-state {
+ default-pins {
pins = "gpio89", "gpio90";
function = "qup4";
};
};
- qup_i2c5_default: qup-i2c5-default {
- pinmux {
+ qup_i2c5_default: qup-i2c5-default-state {
+ default-pins {
pins = "gpio85", "gpio86";
function = "qup5";
};
};
- qup_i2c6_default: qup-i2c6-default {
- pinmux {
+ qup_i2c6_default: qup-i2c6-default-state {
+ default-pins {
pins = "gpio45", "gpio46";
function = "qup6";
};
};
- qup_i2c7_default: qup-i2c7-default {
- pinmux {
+ qup_i2c7_default: qup-i2c7-default-state {
+ default-pins {
pins = "gpio93", "gpio94";
function = "qup7";
};
};
- qup_i2c8_default: qup-i2c8-default {
- pinmux {
+ qup_i2c8_default: qup-i2c8-default-state {
+ default-pins {
pins = "gpio65", "gpio66";
function = "qup8";
};
};
- qup_i2c9_default: qup-i2c9-default {
- pinmux {
+ qup_i2c9_default: qup-i2c9-default-state {
+ default-pins {
pins = "gpio6", "gpio7";
function = "qup9";
};
};
- qup_i2c10_default: qup-i2c10-default {
- pinmux {
+ qup_i2c10_default: qup-i2c10-default-state {
+ default-pins {
pins = "gpio55", "gpio56";
function = "qup10";
};
};
- qup_i2c11_default: qup-i2c11-default {
- pinmux {
+ qup_i2c11_default: qup-i2c11-default-state {
+ default-pins {
pins = "gpio31", "gpio32";
function = "qup11";
};
};
- qup_i2c12_default: qup-i2c12-default {
- pinmux {
+ qup_i2c12_default: qup-i2c12-default-state {
+ default-pins {
pins = "gpio49", "gpio50";
function = "qup12";
};
};
- qup_i2c13_default: qup-i2c13-default {
- pinmux {
+ qup_i2c13_default: qup-i2c13-default-state {
+ default-pins {
pins = "gpio105", "gpio106";
function = "qup13";
};
};
- qup_i2c14_default: qup-i2c14-default {
- pinmux {
+ qup_i2c14_default: qup-i2c14-default-state {
+ default-pins {
pins = "gpio33", "gpio34";
function = "qup14";
};
};
- qup_i2c15_default: qup-i2c15-default {
- pinmux {
+ qup_i2c15_default: qup-i2c15-default-state {
+ default-pins {
pins = "gpio81", "gpio82";
function = "qup15";
};
};
- qup_spi0_default: qup-spi0-default {
- pinmux {
+ qup_spi0_default: qup-spi0-default-state {
+ default-pins {
pins = "gpio0", "gpio1",
"gpio2", "gpio3";
function = "qup0";
- };
-
- config {
- pins = "gpio0", "gpio1",
- "gpio2", "gpio3";
drive-strength = <6>;
bias-disable;
};
};
- qup_spi1_default: qup-spi1-default {
- pinmux {
+ qup_spi1_default: qup-spi1-default-state {
+ default-pins {
pins = "gpio17", "gpio18",
"gpio19", "gpio20";
function = "qup1";
};
};
- qup_spi2_default: qup-spi2-default {
- pinmux {
+ qup_spi2_default: qup-spi2-default-state {
+ default-pins {
pins = "gpio27", "gpio28",
"gpio29", "gpio30";
function = "qup2";
};
};
- qup_spi3_default: qup-spi3-default {
- pinmux {
+ qup_spi3_default: qup-spi3-default-state {
+ default-pins {
pins = "gpio41", "gpio42",
"gpio43", "gpio44";
function = "qup3";
};
};
- qup_spi4_default: qup-spi4-default {
- pinmux {
+ qup_spi4_default: qup-spi4-default-state {
+ default-pins {
pins = "gpio89", "gpio90",
"gpio91", "gpio92";
function = "qup4";
};
};
- qup_spi5_default: qup-spi5-default {
- pinmux {
+ qup_spi5_default: qup-spi5-default-state {
+ default-pins {
pins = "gpio85", "gpio86",
"gpio87", "gpio88";
function = "qup5";
};
};
- qup_spi6_default: qup-spi6-default {
- pinmux {
+ qup_spi6_default: qup-spi6-default-state {
+ default-pins {
pins = "gpio45", "gpio46",
"gpio47", "gpio48";
function = "qup6";
};
};
- qup_spi7_default: qup-spi7-default {
- pinmux {
+ qup_spi7_default: qup-spi7-default-state {
+ default-pins {
pins = "gpio93", "gpio94",
"gpio95", "gpio96";
function = "qup7";
};
};
- qup_spi8_default: qup-spi8-default {
- pinmux {
+ qup_spi8_default: qup-spi8-default-state {
+ default-pins {
pins = "gpio65", "gpio66",
"gpio67", "gpio68";
function = "qup8";
};
};
- qup_spi9_default: qup-spi9-default {
- pinmux {
+ qup_spi9_default: qup-spi9-default-state {
+ default-pins {
pins = "gpio6", "gpio7",
"gpio4", "gpio5";
function = "qup9";
};
};
- qup_spi10_default: qup-spi10-default {
- pinmux {
+ qup_spi10_default: qup-spi10-default-state {
+ default-pins {
pins = "gpio55", "gpio56",
"gpio53", "gpio54";
function = "qup10";
};
};
- qup_spi11_default: qup-spi11-default {
- pinmux {
+ qup_spi11_default: qup-spi11-default-state {
+ default-pins {
pins = "gpio31", "gpio32",
"gpio33", "gpio34";
function = "qup11";
};
};
- qup_spi12_default: qup-spi12-default {
- pinmux {
+ qup_spi12_default: qup-spi12-default-state {
+ default-pins {
pins = "gpio49", "gpio50",
"gpio51", "gpio52";
function = "qup12";
};
};
- qup_spi13_default: qup-spi13-default {
- pinmux {
+ qup_spi13_default: qup-spi13-default-state {
+ default-pins {
pins = "gpio105", "gpio106",
"gpio107", "gpio108";
function = "qup13";
};
};
- qup_spi14_default: qup-spi14-default {
- pinmux {
+ qup_spi14_default: qup-spi14-default-state {
+ default-pins {
pins = "gpio33", "gpio34",
"gpio31", "gpio32";
function = "qup14";
};
};
- qup_spi15_default: qup-spi15-default {
- pinmux {
+ qup_spi15_default: qup-spi15-default-state {
+ default-pins {
pins = "gpio81", "gpio82",
"gpio83", "gpio84";
function = "qup15";
};
};
- qup_uart0_default: qup-uart0-default {
- pinmux {
+ qup_uart0_default: qup-uart0-default-state {
+ default-pins {
pins = "gpio2", "gpio3";
function = "qup0";
};
};
- qup_uart1_default: qup-uart1-default {
- pinmux {
+ qup_uart1_default: qup-uart1-default-state {
+ default-pins {
pins = "gpio19", "gpio20";
function = "qup1";
};
};
- qup_uart2_default: qup-uart2-default {
- pinmux {
+ qup_uart2_default: qup-uart2-default-state {
+ default-pins {
pins = "gpio29", "gpio30";
function = "qup2";
};
};
- qup_uart3_default: qup-uart3-default {
- pinmux {
+ qup_uart3_default: qup-uart3-default-state {
+ default-pins {
pins = "gpio43", "gpio44";
function = "qup3";
};
};
- qup_uart4_default: qup-uart4-default {
- pinmux {
+ qup_uart4_default: qup-uart4-default-state {
+ default-pins {
pins = "gpio91", "gpio92";
function = "qup4";
};
};
- qup_uart5_default: qup-uart5-default {
- pinmux {
+ qup_uart5_default: qup-uart5-default-state {
+ default-pins {
pins = "gpio87", "gpio88";
function = "qup5";
};
};
- qup_uart6_default: qup-uart6-default {
- pinmux {
+ qup_uart6_default: qup-uart6-default-state {
+ default-pins {
pins = "gpio47", "gpio48";
function = "qup6";
};
};
- qup_uart7_default: qup-uart7-default {
- pinmux {
+ qup_uart7_default: qup-uart7-default-state {
+ default-pins {
pins = "gpio95", "gpio96";
function = "qup7";
};
};
- qup_uart8_default: qup-uart8-default {
- pinmux {
+ qup_uart8_default: qup-uart8-default-state {
+ default-pins {
pins = "gpio67", "gpio68";
function = "qup8";
};
};
- qup_uart9_default: qup-uart9-default {
- pinmux {
+ qup_uart9_default: qup-uart9-default-state {
+ default-pins {
pins = "gpio4", "gpio5";
function = "qup9";
};
};
- qup_uart10_default: qup-uart10-default {
- pinmux {
+ qup_uart10_default: qup-uart10-default-state {
+ default-pins {
pins = "gpio53", "gpio54";
function = "qup10";
};
};
- qup_uart11_default: qup-uart11-default {
- pinmux {
+ qup_uart11_default: qup-uart11-default-state {
+ default-pins {
pins = "gpio33", "gpio34";
function = "qup11";
};
};
- qup_uart12_default: qup-uart12-default {
- pinmux {
+ qup_uart12_default: qup-uart12-default-state {
+ default-pins {
pins = "gpio51", "gpio52";
function = "qup12";
};
};
- qup_uart13_default: qup-uart13-default {
- pinmux {
+ qup_uart13_default: qup-uart13-default-state {
+ default-pins {
pins = "gpio107", "gpio108";
function = "qup13";
};
};
- qup_uart14_default: qup-uart14-default {
- pinmux {
+ qup_uart14_default: qup-uart14-default-state {
+ default-pins {
pins = "gpio31", "gpio32";
function = "qup14";
};
};
- qup_uart15_default: qup-uart15-default {
- pinmux {
+ qup_uart15_default: qup-uart15-default-state {
+ default-pins {
pins = "gpio83", "gpio84";
function = "qup15";
};
};
- quat_mi2s_sleep: quat_mi2s_sleep {
- mux {
+ quat_mi2s_sleep: quat-mi2s-sleep-state {
+ default-pins {
pins = "gpio58", "gpio59";
function = "gpio";
- };
-
- config {
- pins = "gpio58", "gpio59";
drive-strength = <2>;
bias-pull-down;
input-enable;
};
};
- quat_mi2s_active: quat_mi2s_active {
- mux {
+ quat_mi2s_active: quat-mi2s-active-state {
+ default-pins {
pins = "gpio58", "gpio59";
function = "qua_mi2s";
- };
-
- config {
- pins = "gpio58", "gpio59";
drive-strength = <8>;
bias-disable;
output-high;
};
};
- quat_mi2s_sd0_sleep: quat_mi2s_sd0_sleep {
- mux {
+ quat_mi2s_sd0_sleep: quat-mi2s-sd0-sleep-state {
+ default-pins {
pins = "gpio60";
function = "gpio";
- };
-
- config {
- pins = "gpio60";
drive-strength = <2>;
bias-pull-down;
input-enable;
};
};
- quat_mi2s_sd0_active: quat_mi2s_sd0_active {
- mux {
+ quat_mi2s_sd0_active: quat-mi2s-sd0-active-state {
+ default-pins {
pins = "gpio60";
function = "qua_mi2s";
- };
-
- config {
- pins = "gpio60";
drive-strength = <8>;
bias-disable;
};
};
- quat_mi2s_sd1_sleep: quat_mi2s_sd1_sleep {
- mux {
+ quat_mi2s_sd1_sleep: quat-mi2s-sd1-sleep-state {
+ default-pins {
pins = "gpio61";
function = "gpio";
- };
-
- config {
- pins = "gpio61";
drive-strength = <2>;
bias-pull-down;
input-enable;
};
};
- quat_mi2s_sd1_active: quat_mi2s_sd1_active {
- mux {
+ quat_mi2s_sd1_active: quat-mi2s-sd1-active-state {
+ default-pins {
pins = "gpio61";
function = "qua_mi2s";
- };
-
- config {
- pins = "gpio61";
drive-strength = <8>;
bias-disable;
};
};
- quat_mi2s_sd2_sleep: quat_mi2s_sd2_sleep {
- mux {
+ quat_mi2s_sd2_sleep: quat-mi2s-sd2-sleep-state {
+ default-pins {
pins = "gpio62";
function = "gpio";
- };
-
- config {
- pins = "gpio62";
drive-strength = <2>;
bias-pull-down;
input-enable;
};
};
- quat_mi2s_sd2_active: quat_mi2s_sd2_active {
- mux {
+ quat_mi2s_sd2_active: quat-mi2s-sd2-active-state {
+ default-pins {
pins = "gpio62";
function = "qua_mi2s";
- };
-
- config {
- pins = "gpio62";
drive-strength = <8>;
bias-disable;
};
};
- quat_mi2s_sd3_sleep: quat_mi2s_sd3_sleep {
- mux {
+ quat_mi2s_sd3_sleep: quat-mi2s-sd3-sleep-state {
+ default-pins {
pins = "gpio63";
function = "gpio";
- };
-
- config {
- pins = "gpio63";
drive-strength = <2>;
bias-pull-down;
input-enable;
};
};
- quat_mi2s_sd3_active: quat_mi2s_sd3_active {
- mux {
+ quat_mi2s_sd3_active: quat-mi2s-sd3-active-state {
+ default-pins {
pins = "gpio63";
function = "qua_mi2s";
- };
-
- config {
- pins = "gpio63";
drive-strength = <8>;
bias-disable;
};
diff --git a/arch/arm64/boot/dts/qcom/sdm850-lenovo-yoga-c630.dts b/arch/arm64/boot/dts/qcom/sdm850-lenovo-yoga-c630.dts
index 74f43da51fa5..e4473b31cea9 100644
--- a/arch/arm64/boot/dts/qcom/sdm850-lenovo-yoga-c630.dts
+++ b/arch/arm64/boot/dts/qcom/sdm850-lenovo-yoga-c630.dts
@@ -479,7 +479,7 @@ &mss_pil {
};
&qup_i2c10_default {
- pinconf {
+ default-pins {
pins = "gpio55", "gpio56";
drive-strength = <2>;
bias-disable;
@@ -487,31 +487,29 @@ pinconf {
};
&qup_i2c12_default {
- pinmux {
+ default-pins {
drive-strength = <2>;
bias-disable;
};
};
&qup_uart6_default {
- pinmux {
- pins = "gpio45", "gpio46", "gpio47", "gpio48";
- function = "qup6";
- };
-
- cts {
+ cts-pins {
pins = "gpio45";
+ function = "qup6";
bias-pull-down;
};
- rts-tx {
+ rts-tx-pins {
pins = "gpio46", "gpio47";
+ function = "qup6";
drive-strength = <2>;
bias-disable;
};
- rx {
+ rx-pins {
pins = "gpio48";
+ function = "qup6";
bias-pull-up;
};
};
@@ -621,13 +619,14 @@ codec {
&tlmm {
gpio-reserved-ranges = <0 4>, <81 4>;
- sn65dsi86_pin_active: sn65dsi86-enable {
+ sn65dsi86_pin_active: sn65dsi86-enable-state {
pins = "gpio96";
+ function = "gpio";
drive-strength = <2>;
bias-disable;
};
- i2c3_hid_active: i2c2-hid-active {
+ i2c3_hid_active: i2c2-hid-active-state {
pins = "gpio37";
function = "gpio";
@@ -636,7 +635,7 @@ i2c3_hid_active: i2c2-hid-active {
drive-strength = <2>;
};
- i2c5_hid_active: i2c5-hid-active {
+ i2c5_hid_active: i2c5-hid-active-state {
pins = "gpio125";
function = "gpio";
@@ -645,7 +644,7 @@ i2c5_hid_active: i2c5-hid-active {
drive-strength = <2>;
};
- i2c11_hid_active: i2c11-hid-active {
+ i2c11_hid_active: i2c11-hid-active-state {
pins = "gpio92";
function = "gpio";
@@ -654,7 +653,7 @@ i2c11_hid_active: i2c11-hid-active {
drive-strength = <2>;
};
- wcd_intr_default: wcd_intr_default {
+ wcd_intr_default: wcd-intr-default-state {
pins = "gpio54";
function = "gpio";
@@ -663,7 +662,7 @@ wcd_intr_default: wcd_intr_default {
drive-strength = <2>;
};
- lid_pin_active: lid-pin {
+ lid_pin_active: lid-pin-state {
pins = "gpio124";
function = "gpio";
@@ -671,7 +670,7 @@ lid_pin_active: lid-pin {
bias-disable;
};
- mode_pin_active: mode-pin {
+ mode_pin_active: mode-pin-state {
pins = "gpio95";
function = "gpio";
diff --git a/arch/arm64/boot/dts/qcom/sdm850-samsung-w737.dts b/arch/arm64/boot/dts/qcom/sdm850-samsung-w737.dts
index d028a7eb364a..75b612fb4775 100644
--- a/arch/arm64/boot/dts/qcom/sdm850-samsung-w737.dts
+++ b/arch/arm64/boot/dts/qcom/sdm850-samsung-w737.dts
@@ -399,7 +399,7 @@ &mss_pil {
};
&qup_i2c10_default {
- pinconf {
+ default-pins {
pins = "gpio55", "gpio56";
drive-strength = <2>;
bias-disable;
@@ -407,7 +407,7 @@ pinconf {
};
&qup_i2c11_default {
- pinconf {
+ default-pins {
pins = "gpio31", "gpio32";
drive-strength = <2>;
bias-disable;
@@ -415,31 +415,29 @@ pinconf {
};
&qup_i2c12_default {
- pinmux {
+ default-pins {
drive-strength = <2>;
bias-disable;
};
};
&qup_uart6_default {
- pinmux {
- pins = "gpio45", "gpio46", "gpio47", "gpio48";
- function = "qup6";
- };
-
- cts {
+ cts-pins {
pins = "gpio45";
+ function = "qup6";
bias-pull-down;
};
- rts-tx {
+ rts-tx-pins {
pins = "gpio46", "gpio47";
+ function = "qup6";
drive-strength = <2>;
bias-disable;
};
- rx {
+ rx-pins {
pins = "gpio48";
+ function = "qup6";
bias-pull-up;
};
};
@@ -549,59 +547,41 @@ codec {
&tlmm {
gpio-reserved-ranges = <0 6>, <85 4>;
- pen_irq_l: pen-irq-l {
- pinmux {
- pins = "gpio119";
- function = "gpio";
- };
-
- pinconf {
- pins = "gpio119";
- bias-disable;
- };
+ pen_irq_l: pen-irq-l-state {
+ pins = "gpio119";
+ function = "gpio";
+ bias-disable;
};
- pen_pdct_l: pen-pdct-l {
- pinmux {
- pins = "gpio124";
- function = "gpio";
- };
-
- pinconf {
- pins = "gpio124";
- bias-disable;
- drive-strength = <2>;
- output-high;
- };
+ pen_pdct_l: pen-pdct-l-state {
+ pins = "gpio124";
+ function = "gpio";
+ bias-disable;
+ drive-strength = <2>;
+ output-high;
};
- pen_rst_l: pen-rst-l {
- pinmux {
- pins = "gpio21";
- function = "gpio";
- };
-
- pinconf {
- pins = "gpio21";
- bias-disable;
- drive-strength = <2>;
-
- /*
- * The pen driver doesn't currently support
- * driving this reset line. By specifying
- * output-high here we're relying on the fact
- * that this pin has a default pulldown at boot
- * (which makes sure the pen was in reset if it
- * was powered) and then we set it high here to
- * take it out of reset. Better would be if the
- * pen driver could control this and we could
- * remove "output-high" here.
- */
- output-high;
- };
+ pen_rst_l: pen-rst-l-state {
+ pins = "gpio21";
+ function = "gpio";
+ bias-disable;
+ drive-strength = <2>;
+
+ /*
+ * The pen driver doesn't currently support
+ * driving this reset line. By specifying
+ * output-high here we're relying on the fact
+ * that this pin has a default pulldown at boot
+ * (which makes sure the pen was in reset if it
+ * was powered) and then we set it high here to
+ * take it out of reset. Better would be if the
+ * pen driver could control this and we could
+ * remove "output-high" here.
+ */
+ output-high;
};
- wcd_intr_default: wcd_intr_default {
+ wcd_intr_default: wcd-intr-default-state {
pins = "gpio54";
function = "gpio";
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/2] dt-bindings: pinctrl: qcom,sdm845: convert to dtschema
2022-09-30 20:05 [PATCH 0/2] pinctrl/arm64: qcom: convert sdm845 bindings to DT schema Krzysztof Kozlowski
2022-09-30 20:05 ` [PATCH 1/2] arm64: dts: qcom: sdm845: align TLMM pin configuration with " Krzysztof Kozlowski
@ 2022-09-30 20:05 ` Krzysztof Kozlowski
2022-09-30 22:05 ` Rob Herring
` (2 more replies)
1 sibling, 3 replies; 17+ messages in thread
From: Krzysztof Kozlowski @ 2022-09-30 20:05 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Linus Walleij,
Rob Herring, Krzysztof Kozlowski, linux-arm-msm, linux-gpio,
devicetree, linux-kernel
Cc: Krzysztof Kozlowski
Convert Qualcomm SDM845 pin controller bindings to DT schema. Keep
the parsing of pin configuration subnodes consistent with other Qualcomm
schemas (children named with '-state' suffix, their children with
'-pins').
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
.../bindings/pinctrl/qcom,sdm845-pinctrl.txt | 176 ------------------
.../bindings/pinctrl/qcom,sdm845-pinctrl.yaml | 158 ++++++++++++++++
2 files changed, 158 insertions(+), 176 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,sdm845-pinctrl.txt
create mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,sdm845-pinctrl.yaml
diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,sdm845-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/qcom,sdm845-pinctrl.txt
deleted file mode 100644
index 7462e3743c68..000000000000
--- a/Documentation/devicetree/bindings/pinctrl/qcom,sdm845-pinctrl.txt
+++ /dev/null
@@ -1,176 +0,0 @@
-Qualcomm SDM845 TLMM block
-
-This binding describes the Top Level Mode Multiplexer block found in the
-SDM845 platform.
-
-- compatible:
- Usage: required
- Value type: <string>
- Definition: must be "qcom,sdm845-pinctrl"
-
-- reg:
- Usage: required
- Value type: <prop-encoded-array>
- Definition: the base address and size of the TLMM register space.
-
-- interrupts:
- Usage: required
- Value type: <prop-encoded-array>
- Definition: should specify the TLMM summary IRQ.
-
-- interrupt-controller:
- Usage: required
- Value type: <none>
- Definition: identifies this node as an interrupt controller
-
-- #interrupt-cells:
- Usage: required
- Value type: <u32>
- Definition: must be 2. Specifying the pin number and flags, as defined
- in <dt-bindings/interrupt-controller/irq.h>
-
-- gpio-controller:
- Usage: required
- Value type: <none>
- Definition: identifies this node as a gpio controller
-
-- #gpio-cells:
- Usage: required
- Value type: <u32>
- Definition: must be 2. Specifying the pin number and flags, as defined
- in <dt-bindings/gpio/gpio.h>
-
-Please refer to ../gpio/gpio.txt and ../interrupt-controller/interrupts.txt for
-a general description of GPIO and interrupt bindings.
-
-Please refer to pinctrl-bindings.txt in this directory for details of the
-common pinctrl bindings used by client devices, including the meaning of the
-phrase "pin configuration node".
-
-The pin configuration nodes act as a container for an arbitrary number of
-subnodes. Each of these subnodes represents some desired configuration for a
-pin, a group, or a list of pins or groups. This configuration can include the
-mux function to select on those pin(s)/group(s), and various pin configuration
-parameters, such as pull-up, drive strength, etc.
-
-
-PIN CONFIGURATION NODES:
-
-The name of each subnode is not important; all subnodes should be enumerated
-and processed purely based on their content.
-
-Each subnode only affects those parameters that are explicitly listed. In
-other words, a subnode that lists a mux function but no pin configuration
-parameters implies no information about any pin configuration parameters.
-Similarly, a pin subnode that describes a pullup parameter implies no
-information about e.g. the mux function.
-
-
-The following generic properties as defined in pinctrl-bindings.txt are valid
-to specify in a pin configuration subnode:
-
-- pins:
- Usage: required
- Value type: <string-array>
- Definition: List of gpio pins affected by the properties specified in
- this subnode.
-
- Valid pins are:
- gpio0-gpio149
- Supports mux, bias and drive-strength
-
- sdc2_clk, sdc2_cmd, sdc2_data, ufs_reset
- Supports bias and drive-strength
-
-- function:
- Usage: required
- Value type: <string>
- Definition: Specify the alternative function to be configured for the
- specified pins. Functions are only valid for gpio pins.
- Valid values are:
-
- gpio, 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, audio_ref,
- btfm_slimbus, cam_mclk, cci_async, cci_i2c, cci_timer0,
- cci_timer1, cci_timer2, cci_timer3, cci_timer4, cri_trng,
- cri_trng0, cri_trng1, dbg_out, ddr_bist, ddr_pxi0,
- ddr_pxi1, ddr_pxi2, ddr_pxi3, edp_hot, edp_lcd, gcc_gp1,
- gcc_gp2, gcc_gp3, 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,
- qspi_clk, qspi_cs, qspi_data, sd_write, sdc4_clk, sdc4_cmd,
- sdc4_data, sec_mi2s, sp_cmu, spkr_i2s, ter_mi2s, tgu_ch0,
- tgu_ch1, tgu_ch2, tgu_ch3, tsense_pwm1, tsense_pwm2,
- 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,
-
-- bias-disable:
- Usage: optional
- Value type: <none>
- Definition: The specified pins should be configured as no pull.
-
-- bias-pull-down:
- Usage: optional
- Value type: <none>
- Definition: The specified pins should be configured as pull down.
-
-- bias-pull-up:
- Usage: optional
- Value type: <none>
- Definition: The specified pins should be configured as pull up.
-
-- output-high:
- Usage: optional
- Value type: <none>
- Definition: The specified pins are configured in output mode, driven
- high.
- Not valid for sdc pins.
-
-- output-low:
- Usage: optional
- Value type: <none>
- Definition: The specified pins are configured in output mode, driven
- low.
- Not valid for sdc pins.
-
-- drive-strength:
- Usage: optional
- Value type: <u32>
- Definition: Selects the drive strength for the specified pins, in mA.
- Valid values are: 2, 4, 6, 8, 10, 12, 14 and 16
-
-Example:
-
- tlmm: pinctrl@3400000 {
- compatible = "qcom,sdm845-pinctrl";
- reg = <0x03400000 0xc00000>;
- interrupts = <GIC_SPI 208 0>;
- gpio-controller;
- #gpio-cells = <2>;
- interrupt-controller;
- #interrupt-cells = <2>;
-
- qup9_active: qup9-active {
- mux {
- pins = "gpio4", "gpio5";
- function = "qup9";
- };
-
- config {
- pins = "gpio4", "gpio5";
- drive-strength = <2>;
- bias-disable;
- };
- };
- };
diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,sdm845-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/qcom,sdm845-pinctrl.yaml
new file mode 100644
index 000000000000..da0ef5698f3e
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/qcom,sdm845-pinctrl.yaml
@@ -0,0 +1,158 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pinctrl/qcom,sdm845-pinctrl.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm SDM845 TLMM pin controller
+
+maintainers:
+ - Bjorn Andersson <andersson@kernel.org>
+ - Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
+
+description:
+ Top Level Mode Multiplexer pin controller node in Qualcomm SDM845 SoC.
+
+allOf:
+ - $ref: /schemas/pinctrl/qcom,tlmm-common.yaml#
+
+properties:
+ compatible:
+ const: qcom,sdm845-pinctrl
+
+ reg:
+ maxItems: 1
+
+ interrupts: true
+ interrupt-controller: true
+ "#interrupt-cells": true
+ gpio-controller: true
+
+ gpio-reserved-ranges:
+ minItems: 1
+ maxItems: 75
+
+ gpio-line-names:
+ maxItems: 150
+
+ "#gpio-cells": true
+ gpio-ranges: true
+ wakeup-parent: true
+
+patternProperties:
+ "-state$":
+ oneOf:
+ - $ref: "#/$defs/qcom-sdm845-tlmm-state"
+ - patternProperties:
+ "-pins$":
+ $ref: "#/$defs/qcom-sdm845-tlmm-state"
+ additionalProperties: false
+
+$defs:
+ qcom-sdm845-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, 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,
+ audio_ref, btfm_slimbus, cam_mclk, cci_async, cci_i2c,
+ cci_timer0, cci_timer1, cci_timer2, cci_timer3, cci_timer4,
+ cri_trng, cri_trng0, cri_trng1, dbg_out, ddr_bist, ddr_pxi0,
+ ddr_pxi1, ddr_pxi2, ddr_pxi3, edp_hot, edp_lcd, gcc_gp1,
+ gcc_gp2, gcc_gp3, gpio, jitter_bist, ldo_en, ldo_update,
+ lpass_slimbus, mdp_vsync, mdp_vsync0, mdp_vsync1, mdp_vsync2,
+ mdp_vsync3, mss_lte, m_voc, nav_pps, pa_indicator, pci_e0,
+ pci_e1, phase_flag, pll_bist, pll_bypassnl, pll_reset,
+ pri_mi2s, pri_mi2s_ws, prng_rosc, qdss, qdss_cti, qlink_enable,
+ qlink_request, qspi_clk, qspi_cs, qspi_data, 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, sp_cmu,
+ spkr_i2s, ter_mi2s, tgu_ch0, tgu_ch1, tgu_ch2, tgu_ch3,
+ tsense_pwm1, tsense_pwm2, 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]
+
+ 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
+
+required:
+ - compatible
+ - reg
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+ pinctrl@3400000 {
+ compatible = "qcom,sdm845-pinctrl";
+ reg = <0x03400000 0xc00000>;
+ interrupts = <GIC_SPI 208 IRQ_TYPE_LEVEL_HIGH>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ gpio-ranges = <&tlmm 0 0 151>;
+ wakeup-parent = <&pdc_intc>;
+
+ cci0-default-state {
+ pins = "gpio17", "gpio18";
+ function = "cci_i2c";
+
+ bias-pull-up;
+ drive-strength = <2>;
+ };
+
+ cam0-default-state {
+ rst-pins {
+ pins = "gpio9";
+ function = "gpio";
+
+ drive-strength = <16>;
+ bias-disable;
+ };
+
+ mclk0-pins {
+ pins = "gpio13";
+ function = "cam_mclk";
+
+ drive-strength = <16>;
+ bias-disable;
+ };
+ };
+ };
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] arm64: dts: qcom: sdm845: align TLMM pin configuration with DT schema
2022-09-30 20:05 ` [PATCH 1/2] arm64: dts: qcom: sdm845: align TLMM pin configuration with " Krzysztof Kozlowski
@ 2022-09-30 20:13 ` Konrad Dybcio
2022-09-30 20:19 ` Krzysztof Kozlowski
2022-10-03 16:14 ` Doug Anderson
1 sibling, 1 reply; 17+ messages in thread
From: Konrad Dybcio @ 2022-09-30 20:13 UTC (permalink / raw)
To: Krzysztof Kozlowski, Andy Gross, Bjorn Andersson, Linus Walleij,
Rob Herring, Krzysztof Kozlowski, linux-arm-msm, linux-gpio,
devicetree, linux-kernel
On 30.09.2022 22:05, Krzysztof Kozlowski wrote:
> DT schema expects TLMM pin configuration nodes to be named with
> '-state' suffix and their optional children with '-pins' suffix.
>
> The sdm854.dtsi file defined several pin configuration nodes which are
845
> customized by the boards. Therefore keep a additional "default-pins"
> node inside so the boards can add more of configuration nodes. Such
> additional configuration nodes always need 'function' property now
> (required by DT schema).
Would it not make more sense to drop this and keep the properties in the
root node while at it?
Konrad
>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
> arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi | 344 +++++++-----------
> arch/arm64/boot/dts/qcom/sdm845-db845c.dts | 76 ++--
> .../arm64/boot/dts/qcom/sdm845-lg-common.dtsi | 60 ++-
> arch/arm64/boot/dts/qcom/sdm845-lg-judyln.dts | 2 +-
> arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 60 ++-
> .../boot/dts/qcom/sdm845-oneplus-common.dtsi | 88 ++---
> .../boot/dts/qcom/sdm845-shift-axolotl.dts | 138 +++----
> .../dts/qcom/sdm845-sony-xperia-tama.dtsi | 6 +-
> .../boot/dts/qcom/sdm845-xiaomi-beryllium.dts | 26 +-
> .../boot/dts/qcom/sdm845-xiaomi-polaris.dts | 30 +-
> arch/arm64/boot/dts/qcom/sdm845.dtsi | 305 +++++++---------
> .../boot/dts/qcom/sdm850-lenovo-yoga-c630.dts | 33 +-
> .../boot/dts/qcom/sdm850-samsung-w737.dts | 96 ++---
> 13 files changed, 513 insertions(+), 751 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi b/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi
> index b5f11fbcc300..3403cdcdd49c 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi
> @@ -993,21 +993,21 @@ &wifi {
> /* PINCTRL - additions to nodes defined in sdm845.dtsi */
>
> &qspi_cs0 {
> - pinconf {
> + default-pins {
> pins = "gpio90";
> bias-disable;
> };
> };
>
> &qspi_clk {
> - pinconf {
> + default-pins {
> pins = "gpio95";
> bias-disable;
> };
> };
>
> &qspi_data01 {
> - pinconf {
> + default-pins {
> pins = "gpio91", "gpio92";
>
> /* High-Z when no transfers; nice to park the lines */
> @@ -1016,7 +1016,7 @@ pinconf {
> };
>
> &qup_i2c3_default {
> - pinconf {
> + default-pins {
> pins = "gpio41", "gpio42";
> drive-strength = <2>;
>
> @@ -1026,7 +1026,7 @@ pinconf {
> };
>
> &qup_i2c11_default {
> - pinconf {
> + default-pins {
> pins = "gpio31", "gpio32";
> drive-strength = <2>;
>
> @@ -1036,7 +1036,7 @@ pinconf {
> };
>
> &qup_i2c12_default {
> - pinconf {
> + default-pins {
> pins = "gpio49", "gpio50";
> drive-strength = <2>;
>
> @@ -1046,7 +1046,7 @@ pinconf {
> };
>
> &qup_i2c14_default {
> - pinconf {
> + default-pins {
> pins = "gpio33", "gpio34";
> drive-strength = <2>;
>
> @@ -1056,7 +1056,7 @@ pinconf {
> };
>
> &qup_spi0_default {
> - pinconf {
> + default-pins {
> pins = "gpio0", "gpio1", "gpio2", "gpio3";
> drive-strength = <2>;
> bias-disable;
> @@ -1064,7 +1064,7 @@ pinconf {
> };
>
> &qup_spi5_default {
> - pinconf {
> + default-pins {
> pins = "gpio85", "gpio86", "gpio87", "gpio88";
> drive-strength = <2>;
> bias-disable;
> @@ -1072,7 +1072,7 @@ pinconf {
> };
>
> &qup_spi10_default {
> - pinconf {
> + default-pins {
> pins = "gpio53", "gpio54", "gpio55", "gpio56";
> drive-strength = <2>;
> bias-disable;
> @@ -1081,28 +1081,25 @@ pinconf {
>
> &qup_uart6_default {
> /* Change pinmux to all 4 pins since CTS and RTS are connected */
> - pinmux {
> - pins = "gpio45", "gpio46",
> - "gpio47", "gpio48";
> - };
> -
> - pinconf-cts {
> + cts-pins {
> /*
> * Configure a pull-down on 45 (CTS) to match the pull of
> * the Bluetooth module.
> */
> pins = "gpio45";
> + function = "qup6";
> bias-pull-down;
> };
>
> - pinconf-rts-tx {
> + rts-tx-pins {
> /* We'll drive 46 (RTS) and 47 (TX), so no pull */
> pins = "gpio46", "gpio47";
> + function = "qup6";
> drive-strength = <2>;
> bias-disable;
> };
>
> - pinconf-rx {
> + rx-pins {
> /*
> * Configure a pull-up on 48 (RX). This is needed to avoid
> * garbage data when the TX pin of the Bluetooth module is
> @@ -1110,19 +1107,22 @@ pinconf-rx {
> * signal yet).
> */
> pins = "gpio48";
> + function = "qup6";
> bias-pull-up;
> };
> };
>
> &qup_uart9_default {
> - pinconf-tx {
> + tx-pins {
> pins = "gpio4";
> + function = "qup9";
> drive-strength = <2>;
> bias-disable;
> };
>
> - pinconf-rx {
> + rx-pins {
> pins = "gpio5";
> + function = "qup9";
> drive-strength = <2>;
> bias-pull-up;
> };
> @@ -1213,243 +1213,153 @@ ap-suspend-l-hog {
> output-low;
> };
>
> - ap_edp_bklten: ap-edp-bklten {
> - pinmux {
> - pins = "gpio37";
> - function = "gpio";
> - };
> -
> - pinconf {
> - pins = "gpio37";
> - drive-strength = <2>;
> - bias-disable;
> - };
> + ap_edp_bklten: ap-edp-bklten-state {
> + pins = "gpio37";
> + function = "gpio";
> + drive-strength = <2>;
> + bias-disable;
> };
>
> - bios_flash_wp_r_l: bios-flash-wp-r-l {
> - pinmux {
> - pins = "gpio128";
> - function = "gpio";
> - input-enable;
> - };
> -
> - pinconf {
> - pins = "gpio128";
> - bias-disable;
> - };
> + bios_flash_wp_r_l: bios-flash-wp-r-l-state {
> + pins = "gpio128";
> + function = "gpio";
> + input-enable;
> + bias-disable;
> };
>
> - ec_ap_int_l: ec-ap-int-l {
> - pinmux {
> - pins = "gpio122";
> - function = "gpio";
> - input-enable;
> - };
> -
> - pinconf {
> - pins = "gpio122";
> - bias-pull-up;
> - };
> + ec_ap_int_l: ec-ap-int-l-state {
> + pins = "gpio122";
> + function = "gpio";
> + input-enable;
> + bias-pull-up;
> };
>
> - edp_brij_en: edp-brij-en {
> - pinmux {
> - pins = "gpio102";
> - function = "gpio";
> - };
> -
> - pinconf {
> - pins = "gpio102";
> - drive-strength = <2>;
> - bias-disable;
> - };
> + edp_brij_en: edp-brij-en-state {
> + pins = "gpio102";
> + function = "gpio";
> + drive-strength = <2>;
> + bias-disable;
> };
>
> - edp_brij_irq: edp-brij-irq {
> - pinmux {
> - pins = "gpio10";
> - function = "gpio";
> - };
> -
> - pinconf {
> - pins = "gpio10";
> - drive-strength = <2>;
> - bias-pull-down;
> - };
> + edp_brij_irq: edp-brij-irq-state {
> + pins = "gpio10";
> + function = "gpio";
> + drive-strength = <2>;
> + bias-pull-down;
> };
>
> - en_pp3300_dx_edp: en-pp3300-dx-edp {
> - pinmux {
> - pins = "gpio43";
> - function = "gpio";
> - };
> -
> - pinconf {
> - pins = "gpio43";
> - drive-strength = <2>;
> - bias-disable;
> - };
> + en_pp3300_dx_edp: en-pp3300-dx-edp-state {
> + pins = "gpio43";
> + function = "gpio";
> + drive-strength = <2>;
> + bias-disable;
> };
>
> - h1_ap_int_odl: h1-ap-int-odl {
> - pinmux {
> - pins = "gpio129";
> - function = "gpio";
> - input-enable;
> - };
> -
> - pinconf {
> - pins = "gpio129";
> - bias-pull-up;
> - };
> + h1_ap_int_odl: h1-ap-int-odl-state {
> + pins = "gpio129";
> + function = "gpio";
> + input-enable;
> + bias-pull-up;
> };
>
> - pen_eject_odl: pen-eject-odl {
> - pinmux {
> - pins = "gpio119";
> - function = "gpio";
> - bias-pull-up;
> - };
> + pen_eject_odl: pen-eject-odl-state {
> + pins = "gpio119";
> + function = "gpio";
> + bias-pull-up;
> };
>
> - pen_irq_l: pen-irq-l {
> - pinmux {
> - pins = "gpio24";
> - function = "gpio";
> - };
> -
> - pinconf {
> - pins = "gpio24";
> + pen_irq_l: pen-irq-l-state {
> + pins = "gpio24";
> + function = "gpio";
>
> - /* Has external pullup */
> - bias-disable;
> - };
> + /* Has external pullup */
> + bias-disable;
> };
>
> - pen_pdct_l: pen-pdct-l {
> - pinmux {
> - pins = "gpio63";
> - function = "gpio";
> - };
> -
> - pinconf {
> - pins = "gpio63";
> + pen_pdct_l: pen-pdct-l-state {
> + pins = "gpio63";
> + function = "gpio";
>
> - /* Has external pullup */
> - bias-disable;
> - };
> + /* Has external pullup */
> + bias-disable;
> };
>
> - pen_rst_l: pen-rst-l {
> - pinmux {
> - pins = "gpio23";
> - function = "gpio";
> - };
> -
> - pinconf {
> - pins = "gpio23";
> - bias-disable;
> - drive-strength = <2>;
> + pen_rst_l: pen-rst-l-state {
> + pins = "gpio23";
> + function = "gpio";
> + bias-disable;
> + drive-strength = <2>;
>
> - /*
> - * The pen driver doesn't currently support
> - * driving this reset line. By specifying
> - * output-high here we're relying on the fact
> - * that this pin has a default pulldown at boot
> - * (which makes sure the pen was in reset if it
> - * was powered) and then we set it high here to
> - * take it out of reset. Better would be if the
> - * pen driver could control this and we could
> - * remove "output-high" here.
> - */
> - output-high;
> - };
> + /*
> + * The pen driver doesn't currently support
> + * driving this reset line. By specifying
> + * output-high here we're relying on the fact
> + * that this pin has a default pulldown at boot
> + * (which makes sure the pen was in reset if it
> + * was powered) and then we set it high here to
> + * take it out of reset. Better would be if the
> + * pen driver could control this and we could
> + * remove "output-high" here.
> + */
> + output-high;
> };
>
> - sdc2_clk: sdc2-clk {
> - pinconf {
> - pins = "sdc2_clk";
> - bias-disable;
> + sdc2_clk: sdc2-clk-state {
> + pins = "sdc2_clk";
> + bias-disable;
>
> - /*
> - * It seems that mmc_test reports errors if drive
> - * strength is not 16.
> - */
> - drive-strength = <16>;
> - };
> + /*
> + * It seems that mmc_test reports errors if drive
> + * strength is not 16.
> + */
> + drive-strength = <16>;
> };
>
> - sdc2_cmd: sdc2-cmd {
> - pinconf {
> - pins = "sdc2_cmd";
> - bias-pull-up;
> - drive-strength = <16>;
> - };
> + sdc2_cmd: sdc2-cmd-state {
> + pins = "sdc2_cmd";
> + bias-pull-up;
> + drive-strength = <16>;
> };
>
> - sdc2_data: sdc2-data {
> - pinconf {
> - pins = "sdc2_data";
> - bias-pull-up;
> - drive-strength = <16>;
> - };
> + sdc2_data: sdc2-data-state {
> + pins = "sdc2_data";
> + bias-pull-up;
> + drive-strength = <16>;
> };
>
> - sd_cd_odl: sd-cd-odl {
> - pinmux {
> - pins = "gpio44";
> - function = "gpio";
> - };
> -
> - pinconf {
> - pins = "gpio44";
> - bias-pull-up;
> - };
> + sd_cd_odl: sd-cd-odl-state {
> + pins = "gpio44";
> + function = "gpio";
> + bias-pull-up;
> };
>
> - ts_int_l: ts-int-l {
> - pinmux {
> - pins = "gpio125";
> - function = "gpio";
> - };
> -
> - pinconf {
> - pins = "gpio125";
> - bias-pull-up;
> - };
> + ts_int_l: ts-int-l-state {
> + pins = "gpio125";
> + function = "gpio";
> + bias-pull-up;
> };
>
> - ts_reset_l: ts-reset-l {
> - pinmux {
> - pins = "gpio118";
> - function = "gpio";
> - };
> -
> - pinconf {
> - pins = "gpio118";
> - bias-disable;
> - drive-strength = <2>;
> - };
> + ts_reset_l: ts-reset-l-state {
> + pins = "gpio118";
> + function = "gpio";
> + bias-disable;
> + drive-strength = <2>;
> };
>
> - ap_suspend_l_assert: ap_suspend_l_assert {
> - config {
> - pins = "gpio126";
> - function = "gpio";
> - bias-disable;
> - drive-strength = <2>;
> - output-low;
> - };
> + ap_suspend_l_assert: ap-suspend-l-assert-state {
> + pins = "gpio126";
> + function = "gpio";
> + bias-disable;
> + drive-strength = <2>;
> + output-low;
> };
>
> - ap_suspend_l_deassert: ap_suspend_l_deassert {
> - config {
> - pins = "gpio126";
> - function = "gpio";
> - bias-disable;
> - drive-strength = <2>;
> - output-high;
> - };
> + ap_suspend_l_deassert: ap-suspend-l-deassert-state {
> + pins = "gpio126";
> + function = "gpio";
> + bias-disable;
> + drive-strength = <2>;
> + output-high;
> };
> };
>
> diff --git a/arch/arm64/boot/dts/qcom/sdm845-db845c.dts b/arch/arm64/boot/dts/qcom/sdm845-db845c.dts
> index a157eab66dee..c5e17a718049 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845-db845c.dts
> +++ b/arch/arm64/boot/dts/qcom/sdm845-db845c.dts
> @@ -820,8 +820,8 @@ &spi2 {
> };
>
> &tlmm {
> - cam0_default: cam0_default {
> - rst {
> + cam0_default: cam0-default-state {
> + rst-pins {
> pins = "gpio9";
> function = "gpio";
>
> @@ -829,7 +829,7 @@ rst {
> bias-disable;
> };
>
> - mclk0 {
> + mclk0-pins {
> pins = "gpio13";
> function = "cam_mclk";
>
> @@ -838,8 +838,8 @@ mclk0 {
> };
> };
>
> - cam3_default: cam3_default {
> - rst {
> + cam3_default: cam3-default-state {
> + rst-pins {
> function = "gpio";
> pins = "gpio21";
>
> @@ -847,7 +847,7 @@ rst {
> bias-disable;
> };
>
> - mclk3 {
> + mclk3-pins {
> function = "cam_mclk";
> pins = "gpio16";
>
> @@ -856,7 +856,7 @@ mclk3 {
> };
> };
>
> - dsi_sw_sel: dsi-sw-sel {
> + dsi_sw_sel: dsi-sw-sel-state {
> pins = "gpio120";
> function = "gpio";
>
> @@ -865,20 +865,20 @@ dsi_sw_sel: dsi-sw-sel {
> output-high;
> };
>
> - lt9611_irq_pin: lt9611-irq {
> + lt9611_irq_pin: lt9611-irq-state {
> pins = "gpio84";
> function = "gpio";
> bias-disable;
> };
>
> - pcie0_default_state: pcie0-default {
> - clkreq {
> + pcie0_default_state: pcie0-default-state {
> + clkreq-pins {
> pins = "gpio36";
> function = "pci_e0";
> bias-pull-up;
> };
>
> - reset-n {
> + reset-n-pins {
> pins = "gpio35";
> function = "gpio";
>
> @@ -887,7 +887,7 @@ reset-n {
> bias-pull-down;
> };
>
> - wake-n {
> + wake-n-pins {
> pins = "gpio37";
> function = "gpio";
>
> @@ -896,7 +896,7 @@ wake-n {
> };
> };
>
> - pcie0_pwren_state: pcie0-pwren {
> + pcie0_pwren_state: pcie0-pwren-state {
> pins = "gpio90";
> function = "gpio";
>
> @@ -904,8 +904,8 @@ pcie0_pwren_state: pcie0-pwren {
> bias-disable;
> };
>
> - pcie1_default_state: pcie1-default {
> - perst-n {
> + pcie1_default_state: pcie1-default-state {
> + perst-n-pins {
> pins = "gpio102";
> function = "gpio";
>
> @@ -913,13 +913,13 @@ perst-n {
> bias-disable;
> };
>
> - clkreq {
> + clkreq-pins {
> pins = "gpio103";
> function = "pci_e1";
> bias-pull-up;
> };
>
> - wake-n {
> + wake-n-pins {
> pins = "gpio11";
> function = "gpio";
>
> @@ -927,7 +927,7 @@ wake-n {
> bias-pull-up;
> };
>
> - reset-n {
> + reset-n-pins {
> pins = "gpio75";
> function = "gpio";
>
> @@ -937,8 +937,8 @@ reset-n {
> };
> };
>
> - sdc2_default_state: sdc2-default {
> - clk {
> + sdc2_default_state: sdc2-default-state {
> + clk-pins {
> pins = "sdc2_clk";
> bias-disable;
>
> @@ -949,26 +949,26 @@ clk {
> drive-strength = <16>;
> };
>
> - cmd {
> + cmd-pins {
> pins = "sdc2_cmd";
> bias-pull-up;
> drive-strength = <10>;
> };
>
> - data {
> + data-pins {
> pins = "sdc2_data";
> bias-pull-up;
> drive-strength = <10>;
> };
> };
>
> - sdc2_card_det_n: sd-card-det-n {
> + sdc2_card_det_n: sd-card-det-n-state {
> pins = "gpio126";
> function = "gpio";
> bias-pull-up;
> };
>
> - wcd_intr_default: wcd_intr_default {
> + wcd_intr_default: wcd-intr-default-state {
> pins = <54>;
> function = "gpio";
>
> @@ -1123,20 +1123,20 @@ &wifi {
>
> /* PINCTRL - additions to nodes defined in sdm845.dtsi */
> &qup_spi2_default {
> - pinmux {
> + default-pins {
> drive-strength = <16>;
> };
> };
>
> &qup_uart3_default{
> - pinmux {
> + default-pins {
> pins = "gpio41", "gpio42", "gpio43", "gpio44";
> function = "qup3";
> };
> };
>
> &qup_i2c10_default {
> - pinconf {
> + default-pins {
> pins = "gpio55", "gpio56";
> drive-strength = <2>;
> bias-disable;
> @@ -1144,37 +1144,37 @@ pinconf {
> };
>
> &qup_uart6_default {
> - pinmux {
> - pins = "gpio45", "gpio46", "gpio47", "gpio48";
> - function = "qup6";
> - };
> -
> - cts {
> + cts-pins {
> pins = "gpio45";
> + function = "qup6";
> bias-disable;
> };
>
> - rts-tx {
> + rts-tx-pins {
> pins = "gpio46", "gpio47";
> + function = "qup6";
> drive-strength = <2>;
> bias-disable;
> };
>
> - rx {
> + rx-pins {
> pins = "gpio48";
> + function = "qup6";
> bias-pull-up;
> };
> };
>
> &qup_uart9_default {
> - pinconf-tx {
> + tx-pins {
> pins = "gpio4";
> + function = "qup9";
> drive-strength = <2>;
> bias-disable;
> };
>
> - pinconf-rx {
> + rx-pins {
> pins = "gpio5";
> + function = "qup9";
> drive-strength = <2>;
> bias-pull-up;
> };
> @@ -1285,7 +1285,7 @@ ov7251_ep: endpoint {
>
> /* PINCTRL - additions to nodes defined in sdm845.dtsi */
> &qup_spi0_default {
> - config {
> + default-pins {
> drive-strength = <6>;
> bias-disable;
> };
> diff --git a/arch/arm64/boot/dts/qcom/sdm845-lg-common.dtsi b/arch/arm64/boot/dts/qcom/sdm845-lg-common.dtsi
> index 1eb423e4be24..3ccfbdb2880e 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845-lg-common.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845-lg-common.dtsi
> @@ -558,48 +558,36 @@ &usb_1_qmpphy {
> &tlmm {
> gpio-reserved-ranges = <28 4>, <81 4>;
>
> - sdc2_clk: sdc2-clk {
> - pinconf {
> - pins = "sdc2_clk";
> - bias-disable;
> -
> - /*
> - * It seems that mmc_test reports errors if drive
> - * strength is not 16 on clk, cmd, and data pins.
> - *
> - * TODO: copy-pasted from mtp, try other values
> - * on these devices.
> - */
> - drive-strength = <16>;
> - };
> + sdc2_clk: sdc2-clk-state {
> + pins = "sdc2_clk";
> + bias-disable;
> +
> + /*
> + * It seems that mmc_test reports errors if drive
> + * strength is not 16 on clk, cmd, and data pins.
> + *
> + * TODO: copy-pasted from mtp, try other values
> + * on these devices.
> + */
> + drive-strength = <16>;
> };
>
> - sdc2_cmd: sdc2-cmd {
> - pinconf {
> - pins = "sdc2_cmd";
> - bias-pull-up;
> - drive-strength = <16>;
> - };
> + sdc2_cmd: sdc2-cmd-state {
> + pins = "sdc2_cmd";
> + bias-pull-up;
> + drive-strength = <16>;
> };
>
> - sdc2_data: sdc2-data {
> - pinconf {
> - pins = "sdc2_data";
> - bias-pull-up;
> - drive-strength = <16>;
> - };
> + sdc2_data: sdc2-data-state {
> + pins = "sdc2_data";
> + bias-pull-up;
> + drive-strength = <16>;
> };
>
> - sd_card_det_n: sd-card-det-n {
> - pinmux {
> - pins = "gpio126";
> - function = "gpio";
> - };
> -
> - pinconf {
> - pins = "gpio126";
> - bias-pull-up;
> - };
> + sd_card_det_n: sd-card-det-n-state {
> + pins = "gpio126";
> + function = "gpio";
> + bias-pull-up;
> };
> };
>
> diff --git a/arch/arm64/boot/dts/qcom/sdm845-lg-judyln.dts b/arch/arm64/boot/dts/qcom/sdm845-lg-judyln.dts
> index 7d967a104b3e..a12723310c8b 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845-lg-judyln.dts
> +++ b/arch/arm64/boot/dts/qcom/sdm845-lg-judyln.dts
> @@ -58,7 +58,7 @@ &mss_pil {
> };
>
> &tlmm {
> - thinq_key_default: thinq-key-default {
> + thinq_key_default: thinq-key-default-state {
> pins = "gpio89";
> function = "gpio";
>
> diff --git a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
> index de2d10e0315a..b36befe60b08 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
> +++ b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
> @@ -719,7 +719,7 @@ &wifi {
> /* PINCTRL - additions to nodes defined in sdm845.dtsi */
>
> &qup_i2c10_default {
> - pinconf {
> + default-pins {
> pins = "gpio55", "gpio56";
> drive-strength = <2>;
> bias-disable;
> @@ -727,14 +727,16 @@ pinconf {
> };
>
> &qup_uart9_default {
> - pinconf-tx {
> + tx-pins {
> pins = "gpio4";
> + function = "qup9";
> drive-strength = <2>;
> bias-disable;
> };
>
> - pinconf-rx {
> + rx-pins {
> pins = "gpio5";
> + function = "qup9";
> drive-strength = <2>;
> bias-pull-up;
> };
> @@ -743,44 +745,32 @@ pinconf-rx {
> &tlmm {
> gpio-reserved-ranges = <0 4>, <81 4>;
>
> - sdc2_clk: sdc2-clk {
> - pinconf {
> - pins = "sdc2_clk";
> - bias-disable;
> + sdc2_clk: sdc2-clk-state {
> + pins = "sdc2_clk";
> + bias-disable;
>
> - /*
> - * It seems that mmc_test reports errors if drive
> - * strength is not 16 on clk, cmd, and data pins.
> - */
> - drive-strength = <16>;
> - };
> + /*
> + * It seems that mmc_test reports errors if drive
> + * strength is not 16 on clk, cmd, and data pins.
> + */
> + drive-strength = <16>;
> };
>
> - sdc2_cmd: sdc2-cmd {
> - pinconf {
> - pins = "sdc2_cmd";
> - bias-pull-up;
> - drive-strength = <16>;
> - };
> + sdc2_cmd: sdc2-cmd-state {
> + pins = "sdc2_cmd";
> + bias-pull-up;
> + drive-strength = <16>;
> };
>
> - sdc2_data: sdc2-data {
> - pinconf {
> - pins = "sdc2_data";
> - bias-pull-up;
> - drive-strength = <16>;
> - };
> + sdc2_data: sdc2-data-state {
> + pins = "sdc2_data";
> + bias-pull-up;
> + drive-strength = <16>;
> };
>
> - sd_card_det_n: sd-card-det-n {
> - pinmux {
> - pins = "gpio126";
> - function = "gpio";
> - };
> -
> - pinconf {
> - pins = "gpio126";
> - bias-pull-up;
> - };
> + sd_card_det_n: sd-card-det-n-state {
> + pins = "gpio126";
> + function = "gpio";
> + bias-pull-up;
> };
> };
> diff --git a/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi b/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
> index 392461c29e76..02a1e056f465 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
> @@ -470,7 +470,7 @@ &qupv3_id_0 {
> };
>
> &qup_i2c12_default {
> - mux {
> + default-pins {
> pins = "gpio49", "gpio50";
> function = "qup12";
> drive-strength = <2>;
> @@ -479,7 +479,7 @@ mux {
> };
>
> &qup_i2c10_default {
> - pinconf {
> + default-pins {
> pins = "gpio55", "gpio56";
> drive-strength = <2>;
> bias-disable;
> @@ -487,14 +487,16 @@ pinconf {
> };
>
> &qup_uart9_default {
> - pinconf-tx {
> + tx-pins {
> pins = "gpio4";
> + function = "qup9";
> drive-strength = <2>;
> bias-disable;
> };
>
> - pinconf-rx {
> + rx-pins {
> pins = "gpio5";
> + function = "qup9";
> drive-strength = <2>;
> bias-pull-up;
> };
> @@ -504,24 +506,22 @@ pinconf-rx {
> * Prevent garbage data on bluetooth UART lines
> */
> &qup_uart6_default {
> - pinmux {
> - pins = "gpio45", "gpio46", "gpio47", "gpio48";
> - function = "qup6";
> - };
> -
> - cts {
> + cts-pins {
> pins = "gpio45";
> + function = "qup6";
> bias-pull-down;
> };
>
> - rts-tx {
> + rts-tx-pins {
> pins = "gpio46", "gpio47";
> + function = "qup6";
> drive-strength = <2>;
> bias-disable;
> };
>
> - rx {
> + rx-pins {
> pins = "gpio48";
> + function = "qup6";
> bias-pull-up;
> };
> };
> @@ -607,51 +607,41 @@ &usb_1_hsphy {
> &tlmm {
> gpio-reserved-ranges = <0 4>, <81 4>;
>
> - tri_state_key_default: tri_state_key_default {
> - mux {
> - pins = "gpio40", "gpio42", "gpio26";
> - function = "gpio";
> - drive-strength = <2>;
> - bias-disable;
> - };
> + tri_state_key_default: tri-state-key-default-state {
> + pins = "gpio40", "gpio42", "gpio26";
> + function = "gpio";
> + drive-strength = <2>;
> + bias-disable;
> };
>
> - ts_default_pins: ts-int {
> - mux {
> - pins = "gpio99", "gpio125";
> - function = "gpio";
> - drive-strength = <16>;
> - bias-pull-up;
> - };
> + ts_default_pins: ts-int-state {
> + pins = "gpio99", "gpio125";
> + function = "gpio";
> + drive-strength = <16>;
> + bias-pull-up;
> };
>
> - panel_reset_pins: panel-reset {
> - mux {
> - pins = "gpio6", "gpio25", "gpio26";
> - function = "gpio";
> - drive-strength = <8>;
> - bias-disable;
> - };
> + panel_reset_pins: panel-reset-state {
> + pins = "gpio6", "gpio25", "gpio26";
> + function = "gpio";
> + drive-strength = <8>;
> + bias-disable;
> };
>
> - panel_te_pin: panel-te {
> - mux {
> - pins = "gpio10";
> - function = "mdp_vsync";
> - drive-strength = <2>;
> - bias-disable;
> - input-enable;
> - };
> + panel_te_pin: panel-te-state {
> + pins = "gpio10";
> + function = "mdp_vsync";
> + drive-strength = <2>;
> + bias-disable;
> + input-enable;
> };
>
> - panel_esd_pin: panel-esd {
> - mux {
> - pins = "gpio30";
> - function = "gpio";
> - drive-strength = <2>;
> - bias-pull-down;
> - input-enable;
> - };
> + panel_esd_pin: panel-esd-state {
> + pins = "gpio30";
> + function = "gpio";
> + drive-strength = <2>;
> + bias-pull-down;
> + input-enable;
> };
> };
>
> diff --git a/arch/arm64/boot/dts/qcom/sdm845-shift-axolotl.dts b/arch/arm64/boot/dts/qcom/sdm845-shift-axolotl.dts
> index 83261c9bb4f2..7c752f621a23 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845-shift-axolotl.dts
> +++ b/arch/arm64/boot/dts/qcom/sdm845-shift-axolotl.dts
> @@ -559,14 +559,16 @@ led@5 {
> };
>
> &qup_uart9_default {
> - pinconf-rx {
> + rx-pins {
> pins = "gpio5";
> + function = "qup9";
> drive-strength = <2>;
> bias-pull-up;
> };
>
> - pinconf-tx {
> + tx-pins {
> pins = "gpio4";
> + function = "qup9";
> drive-strength = <2>;
> bias-disable;
> };
> @@ -583,110 +585,62 @@ &qupv3_id_1 {
> &tlmm {
> gpio-reserved-ranges = <0 4>, <81 4>;
>
> - sde_dsi_active: sde-dsi-active {
> - mux {
> - pins = "gpio6", "gpio11";
> - function = "gpio";
> - };
> -
> - config {
> - pins = "gpio6", "gpio11";
> - drive-strength = <8>;
> - bias-disable;
> - };
> + sde_dsi_active: sde-dsi-active-state {
> + pins = "gpio6", "gpio11";
> + function = "gpio";
> + drive-strength = <8>;
> + bias-disable;
> };
>
> - sde_dsi_suspend: sde-dsi-suspend {
> - mux {
> - pins = "gpio6", "gpio11";
> - function = "gpio";
> - };
> -
> - config {
> - pins = "gpio6", "gpio11";
> - drive-strength = <2>;
> - bias-pull-down;
> - };
> + sde_dsi_suspend: sde-dsi-suspend-state {
> + pins = "gpio6", "gpio11";
> + function = "gpio";
> + drive-strength = <2>;
> + bias-pull-down;
> };
>
> - sde_te_active: sde-te-active {
> - mux {
> - pins = "gpio10";
> - function = "mdp_vsync";
> - };
> -
> - config {
> - pins = "gpio10";
> - drive-strength = <2>;
> - bias-pull-down;
> - };
> + sde_te_active: sde-te-active-state {
> + pins = "gpio10";
> + function = "mdp_vsync";
> + drive-strength = <2>;
> + bias-pull-down;
> };
>
> - sde_te_suspend: sde-te-suspend {
> - mux {
> - pins = "gpio10";
> - function = "mdp_vsync";
> - };
> -
> - config {
> - pins = "gpio10";
> - drive-strength = <2>;
> - bias-pull-down;
> - };
> + sde_te_suspend: sde-te-suspend-state {
> + pins = "gpio10";
> + function = "mdp_vsync";
> + drive-strength = <2>;
> + bias-pull-down;
> };
>
> - ts_int_active: ts-int-active {
> - mux {
> - pins = "gpio125";
> - function = "gpio";
> - };
> -
> - config {
> - pins = "gpio125";
> - drive-strength = <8>;
> - bias-pull-up;
> - input-enable;
> - };
> + ts_int_active: ts-int-active-state {
> + pins = "gpio125";
> + function = "gpio";
> + drive-strength = <8>;
> + bias-pull-up;
> + input-enable;
> };
>
> - ts_int_suspend: ts-int-suspend {
> - mux {
> - pins = "gpio125";
> - function = "gpio";
> - };
> -
> - config {
> - pins = "gpio125";
> - drive-strength = <2>;
> - bias-pull-down;
> - input-enable;
> - };
> + ts_int_suspend: ts-int-suspend-state {
> + pins = "gpio125";
> + function = "gpio";
> + drive-strength = <2>;
> + bias-pull-down;
> + input-enable;
> };
>
> - ts_reset_active: ts-reset-active {
> - mux {
> - pins = "gpio99";
> - function = "gpio";
> - };
> -
> - config {
> - pins = "gpio99";
> - drive-strength = <8>;
> - bias-pull-up;
> - };
> + ts_reset_active: ts-reset-active-state {
> + pins = "gpio99";
> + function = "gpio";
> + drive-strength = <8>;
> + bias-pull-up;
> };
>
> - ts_reset_suspend: ts-reset-suspend {
> - mux {
> - pins = "gpio99";
> - function = "gpio";
> - };
> -
> - config {
> - pins = "gpio99";
> - drive-strength = <2>;
> - bias-pull-down;
> - };
> + ts_reset_suspend: ts-reset-suspend-state {
> + pins = "gpio99";
> + function = "gpio";
> + drive-strength = <2>;
> + bias-pull-down;
> };
> };
>
> diff --git a/arch/arm64/boot/dts/qcom/sdm845-sony-xperia-tama.dtsi b/arch/arm64/boot/dts/qcom/sdm845-sony-xperia-tama.dtsi
> index 51ee42e3c995..3a920275dda3 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845-sony-xperia-tama.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845-sony-xperia-tama.dtsi
> @@ -383,19 +383,19 @@ &tlmm {
> gpio-reserved-ranges = <0 4>, <81 4>;
>
> sdc2_default_state: sdc2-default-state {
> - clk {
> + clk-pins {
> pins = "sdc2_clk";
> drive-strength = <16>;
> bias-disable;
> };
>
> - cmd {
> + cmd-pins {
> pins = "sdc2_cmd";
> drive-strength = <10>;
> bias-pull-up;
> };
>
> - data {
> + data-pins {
> pins = "sdc2_data";
> drive-strength = <10>;
> bias-pull-up;
> diff --git a/arch/arm64/boot/dts/qcom/sdm845-xiaomi-beryllium.dts b/arch/arm64/boot/dts/qcom/sdm845-xiaomi-beryllium.dts
> index 0f470cf1ed1c..d6392f3486ae 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845-xiaomi-beryllium.dts
> +++ b/arch/arm64/boot/dts/qcom/sdm845-xiaomi-beryllium.dts
> @@ -442,33 +442,33 @@ codec {
> &tlmm {
> gpio-reserved-ranges = <0 4>, <81 4>;
>
> - sdc2_default_state: sdc2-default {
> - clk {
> + sdc2_default_state: sdc2-default-state {
> + clk-pins {
> pins = "sdc2_clk";
> bias-disable;
> drive-strength = <16>;
> };
>
> - cmd {
> + cmd-pins {
> pins = "sdc2_cmd";
> bias-pull-up;
> drive-strength = <10>;
> };
>
> - data {
> + data-pins {
> pins = "sdc2_data";
> bias-pull-up;
> drive-strength = <10>;
> };
> };
>
> - sdc2_card_det_n: sd-card-det-n {
> + sdc2_card_det_n: sd-card-det-n-state {
> pins = "gpio126";
> function = "gpio";
> bias-pull-up;
> };
>
> - wcd_intr_default: wcd_intr_default {
> + wcd_intr_default: wcd-intr-default-state {
> pins = <54>;
> function = "gpio";
>
> @@ -571,24 +571,22 @@ &wifi {
> /* PINCTRL - additions to nodes defined in sdm845.dtsi */
>
> &qup_uart6_default {
> - pinmux {
> - pins = "gpio45", "gpio46", "gpio47", "gpio48";
> - function = "qup6";
> - };
> -
> - cts {
> + cts-pins {
> pins = "gpio45";
> + function = "qup6";
> bias-disable;
> };
>
> - rts-tx {
> + rts-tx-pins {
> pins = "gpio46", "gpio47";
> + function = "qup6";
> drive-strength = <2>;
> bias-disable;
> };
>
> - rx {
> + rx-pins {
> pins = "gpio48";
> + function = "qup6";
> bias-pull-up;
> };
> };
> diff --git a/arch/arm64/boot/dts/qcom/sdm845-xiaomi-polaris.dts b/arch/arm64/boot/dts/qcom/sdm845-xiaomi-polaris.dts
> index f98259489679..9a4e1ff1d3ba 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845-xiaomi-polaris.dts
> +++ b/arch/arm64/boot/dts/qcom/sdm845-xiaomi-polaris.dts
> @@ -573,7 +573,7 @@ &qupv3_id_1 {
> };
>
> &qup_i2c14_default {
> - pinconf {
> + default-pins {
> pins = "gpio33", "gpio34";
> drive-strength = <2>;
> bias-disable;
> @@ -583,14 +583,14 @@ pinconf {
> &tlmm {
> gpio-reserved-ranges = <0 4>, <81 4>;
>
> - ts_reset_default: ts-reset-default {
> + ts_reset_default: ts-reset-default-state {
> pins = "gpio99";
> function = "gpio";
> drive-strength = <16>;
> output-high;
> };
>
> - ts_int_default: ts-int-default {
> + ts_int_default: ts-int-default-state {
> pins = "gpio125";
> function = "gpio";
> bias-pull-down;
> @@ -598,14 +598,14 @@ ts_int_default: ts-int-default {
> input-enable;
> };
>
> - ts_reset_sleep: ts-reset-sleep {
> + ts_reset_sleep: ts-reset-sleep-state {
> pins = "gpio99";
> function = "gpio";
> bias-disable;
> drive-strength = <2>;
> };
>
> - ts_int_sleep: ts-int-sleep {
> + ts_int_sleep: ts-int-sleep-state {
> pins = "gpio125";
> function = "gpio";
> bias-pull-down;
> @@ -613,21 +613,21 @@ ts_int_sleep: ts-int-sleep {
> input-enable;
> };
>
> - sde_dsi_active: sde-dsi-active {
> + sde_dsi_active: sde-dsi-active-state {
> pins = "gpio6", "gpio10";
> function = "gpio";
> drive-strength = <8>;
> bias-disable;
> };
>
> - sde_dsi_suspend: sde-dsi-suspend {
> + sde_dsi_suspend: sde-dsi-suspend-state {
> pins = "gpio6", "gpio10";
> function = "gpio";
> drive-strength = <2>;
> bias-pull-down;
> };
>
> - wcd_intr_default: wcd-intr-default {
> + wcd_intr_default: wcd-intr-default-state {
> pins = "gpio54";
> function = "gpio";
> input-enable;
> @@ -739,24 +739,22 @@ &wifi {
> /* PINCTRL - additions to nodes defined in sdm845.dtsi */
>
> &qup_uart6_default {
> - pinmux {
> - pins = "gpio45", "gpio46", "gpio47", "gpio48";
> - function = "qup6";
> - };
> -
> - cts {
> + cts-pins {
> pins = "gpio45";
> + function = "qup6";
> bias-disable;
> };
>
> - rts-tx {
> + rts-tx-pins {
> pins = "gpio46", "gpio47";
> + function = "qup6";
> drive-strength = <2>;
> bias-disable;
> };
>
> - rx {
> + rx-pins {
> pins = "gpio48";
> + function = "qup6";
> bias-pull-up;
> };
> };
> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> index d761da47220d..915ef8dda023 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> @@ -2641,7 +2641,7 @@ tlmm: pinctrl@3400000 {
> gpio-ranges = <&tlmm 0 0 151>;
> wakeup-parent = <&pdc_intc>;
>
> - cci0_default: cci0-default {
> + cci0_default: cci0-default-state {
> /* SDA, SCL */
> pins = "gpio17", "gpio18";
> function = "cci_i2c";
> @@ -2650,7 +2650,7 @@ cci0_default: cci0-default {
> drive-strength = <2>; /* 2 mA */
> };
>
> - cci0_sleep: cci0-sleep {
> + cci0_sleep: cci0-sleep-state {
> /* SDA, SCL */
> pins = "gpio17", "gpio18";
> function = "cci_i2c";
> @@ -2659,7 +2659,7 @@ cci0_sleep: cci0-sleep {
> bias-pull-down;
> };
>
> - cci1_default: cci1-default {
> + cci1_default: cci1-default-state {
> /* SDA, SCL */
> pins = "gpio19", "gpio20";
> function = "cci_i2c";
> @@ -2668,7 +2668,7 @@ cci1_default: cci1-default {
> drive-strength = <2>; /* 2 mA */
> };
>
> - cci1_sleep: cci1-sleep {
> + cci1_sleep: cci1-sleep-state {
> /* SDA, SCL */
> pins = "gpio19", "gpio20";
> function = "cci_i2c";
> @@ -2677,531 +2677,486 @@ cci1_sleep: cci1-sleep {
> bias-pull-down;
> };
>
> - qspi_clk: qspi-clk {
> - pinmux {
> + qspi_clk: qspi-clk-state {
> + default-pins {
> pins = "gpio95";
> function = "qspi_clk";
> };
> };
>
> - qspi_cs0: qspi-cs0 {
> - pinmux {
> + qspi_cs0: qspi-cs0-state {
> + default-pins {
> pins = "gpio90";
> function = "qspi_cs";
> };
> };
>
> - qspi_cs1: qspi-cs1 {
> - pinmux {
> + qspi_cs1: qspi-cs1-state {
> + default-pins {
> pins = "gpio89";
> function = "qspi_cs";
> };
> };
>
> - qspi_data01: qspi-data01 {
> - pinmux-data {
> + qspi_data01: qspi-data01-state {
> + default-pins {
> pins = "gpio91", "gpio92";
> function = "qspi_data";
> };
> };
>
> - qspi_data12: qspi-data12 {
> - pinmux-data {
> + qspi_data12: qspi-data12-state {
> + default-pins {
> pins = "gpio93", "gpio94";
> function = "qspi_data";
> };
> };
>
> - qup_i2c0_default: qup-i2c0-default {
> - pinmux {
> + qup_i2c0_default: qup-i2c0-default-state {
> + default-pins {
> pins = "gpio0", "gpio1";
> function = "qup0";
> };
> };
>
> - qup_i2c1_default: qup-i2c1-default {
> - pinmux {
> + qup_i2c1_default: qup-i2c1-default-state {
> + default-pins {
> pins = "gpio17", "gpio18";
> function = "qup1";
> };
> };
>
> - qup_i2c2_default: qup-i2c2-default {
> - pinmux {
> + qup_i2c2_default: qup-i2c2-default-state {
> + default-pins {
> pins = "gpio27", "gpio28";
> function = "qup2";
> };
> };
>
> - qup_i2c3_default: qup-i2c3-default {
> - pinmux {
> + qup_i2c3_default: qup-i2c3-default-state {
> + default-pins {
> pins = "gpio41", "gpio42";
> function = "qup3";
> };
> };
>
> - qup_i2c4_default: qup-i2c4-default {
> - pinmux {
> + qup_i2c4_default: qup-i2c4-default-state {
> + default-pins {
> pins = "gpio89", "gpio90";
> function = "qup4";
> };
> };
>
> - qup_i2c5_default: qup-i2c5-default {
> - pinmux {
> + qup_i2c5_default: qup-i2c5-default-state {
> + default-pins {
> pins = "gpio85", "gpio86";
> function = "qup5";
> };
> };
>
> - qup_i2c6_default: qup-i2c6-default {
> - pinmux {
> + qup_i2c6_default: qup-i2c6-default-state {
> + default-pins {
> pins = "gpio45", "gpio46";
> function = "qup6";
> };
> };
>
> - qup_i2c7_default: qup-i2c7-default {
> - pinmux {
> + qup_i2c7_default: qup-i2c7-default-state {
> + default-pins {
> pins = "gpio93", "gpio94";
> function = "qup7";
> };
> };
>
> - qup_i2c8_default: qup-i2c8-default {
> - pinmux {
> + qup_i2c8_default: qup-i2c8-default-state {
> + default-pins {
> pins = "gpio65", "gpio66";
> function = "qup8";
> };
> };
>
> - qup_i2c9_default: qup-i2c9-default {
> - pinmux {
> + qup_i2c9_default: qup-i2c9-default-state {
> + default-pins {
> pins = "gpio6", "gpio7";
> function = "qup9";
> };
> };
>
> - qup_i2c10_default: qup-i2c10-default {
> - pinmux {
> + qup_i2c10_default: qup-i2c10-default-state {
> + default-pins {
> pins = "gpio55", "gpio56";
> function = "qup10";
> };
> };
>
> - qup_i2c11_default: qup-i2c11-default {
> - pinmux {
> + qup_i2c11_default: qup-i2c11-default-state {
> + default-pins {
> pins = "gpio31", "gpio32";
> function = "qup11";
> };
> };
>
> - qup_i2c12_default: qup-i2c12-default {
> - pinmux {
> + qup_i2c12_default: qup-i2c12-default-state {
> + default-pins {
> pins = "gpio49", "gpio50";
> function = "qup12";
> };
> };
>
> - qup_i2c13_default: qup-i2c13-default {
> - pinmux {
> + qup_i2c13_default: qup-i2c13-default-state {
> + default-pins {
> pins = "gpio105", "gpio106";
> function = "qup13";
> };
> };
>
> - qup_i2c14_default: qup-i2c14-default {
> - pinmux {
> + qup_i2c14_default: qup-i2c14-default-state {
> + default-pins {
> pins = "gpio33", "gpio34";
> function = "qup14";
> };
> };
>
> - qup_i2c15_default: qup-i2c15-default {
> - pinmux {
> + qup_i2c15_default: qup-i2c15-default-state {
> + default-pins {
> pins = "gpio81", "gpio82";
> function = "qup15";
> };
> };
>
> - qup_spi0_default: qup-spi0-default {
> - pinmux {
> + qup_spi0_default: qup-spi0-default-state {
> + default-pins {
> pins = "gpio0", "gpio1",
> "gpio2", "gpio3";
> function = "qup0";
> - };
> -
> - config {
> - pins = "gpio0", "gpio1",
> - "gpio2", "gpio3";
> drive-strength = <6>;
> bias-disable;
> };
> };
>
> - qup_spi1_default: qup-spi1-default {
> - pinmux {
> + qup_spi1_default: qup-spi1-default-state {
> + default-pins {
> pins = "gpio17", "gpio18",
> "gpio19", "gpio20";
> function = "qup1";
> };
> };
>
> - qup_spi2_default: qup-spi2-default {
> - pinmux {
> + qup_spi2_default: qup-spi2-default-state {
> + default-pins {
> pins = "gpio27", "gpio28",
> "gpio29", "gpio30";
> function = "qup2";
> };
> };
>
> - qup_spi3_default: qup-spi3-default {
> - pinmux {
> + qup_spi3_default: qup-spi3-default-state {
> + default-pins {
> pins = "gpio41", "gpio42",
> "gpio43", "gpio44";
> function = "qup3";
> };
> };
>
> - qup_spi4_default: qup-spi4-default {
> - pinmux {
> + qup_spi4_default: qup-spi4-default-state {
> + default-pins {
> pins = "gpio89", "gpio90",
> "gpio91", "gpio92";
> function = "qup4";
> };
> };
>
> - qup_spi5_default: qup-spi5-default {
> - pinmux {
> + qup_spi5_default: qup-spi5-default-state {
> + default-pins {
> pins = "gpio85", "gpio86",
> "gpio87", "gpio88";
> function = "qup5";
> };
> };
>
> - qup_spi6_default: qup-spi6-default {
> - pinmux {
> + qup_spi6_default: qup-spi6-default-state {
> + default-pins {
> pins = "gpio45", "gpio46",
> "gpio47", "gpio48";
> function = "qup6";
> };
> };
>
> - qup_spi7_default: qup-spi7-default {
> - pinmux {
> + qup_spi7_default: qup-spi7-default-state {
> + default-pins {
> pins = "gpio93", "gpio94",
> "gpio95", "gpio96";
> function = "qup7";
> };
> };
>
> - qup_spi8_default: qup-spi8-default {
> - pinmux {
> + qup_spi8_default: qup-spi8-default-state {
> + default-pins {
> pins = "gpio65", "gpio66",
> "gpio67", "gpio68";
> function = "qup8";
> };
> };
>
> - qup_spi9_default: qup-spi9-default {
> - pinmux {
> + qup_spi9_default: qup-spi9-default-state {
> + default-pins {
> pins = "gpio6", "gpio7",
> "gpio4", "gpio5";
> function = "qup9";
> };
> };
>
> - qup_spi10_default: qup-spi10-default {
> - pinmux {
> + qup_spi10_default: qup-spi10-default-state {
> + default-pins {
> pins = "gpio55", "gpio56",
> "gpio53", "gpio54";
> function = "qup10";
> };
> };
>
> - qup_spi11_default: qup-spi11-default {
> - pinmux {
> + qup_spi11_default: qup-spi11-default-state {
> + default-pins {
> pins = "gpio31", "gpio32",
> "gpio33", "gpio34";
> function = "qup11";
> };
> };
>
> - qup_spi12_default: qup-spi12-default {
> - pinmux {
> + qup_spi12_default: qup-spi12-default-state {
> + default-pins {
> pins = "gpio49", "gpio50",
> "gpio51", "gpio52";
> function = "qup12";
> };
> };
>
> - qup_spi13_default: qup-spi13-default {
> - pinmux {
> + qup_spi13_default: qup-spi13-default-state {
> + default-pins {
> pins = "gpio105", "gpio106",
> "gpio107", "gpio108";
> function = "qup13";
> };
> };
>
> - qup_spi14_default: qup-spi14-default {
> - pinmux {
> + qup_spi14_default: qup-spi14-default-state {
> + default-pins {
> pins = "gpio33", "gpio34",
> "gpio31", "gpio32";
> function = "qup14";
> };
> };
>
> - qup_spi15_default: qup-spi15-default {
> - pinmux {
> + qup_spi15_default: qup-spi15-default-state {
> + default-pins {
> pins = "gpio81", "gpio82",
> "gpio83", "gpio84";
> function = "qup15";
> };
> };
>
> - qup_uart0_default: qup-uart0-default {
> - pinmux {
> + qup_uart0_default: qup-uart0-default-state {
> + default-pins {
> pins = "gpio2", "gpio3";
> function = "qup0";
> };
> };
>
> - qup_uart1_default: qup-uart1-default {
> - pinmux {
> + qup_uart1_default: qup-uart1-default-state {
> + default-pins {
> pins = "gpio19", "gpio20";
> function = "qup1";
> };
> };
>
> - qup_uart2_default: qup-uart2-default {
> - pinmux {
> + qup_uart2_default: qup-uart2-default-state {
> + default-pins {
> pins = "gpio29", "gpio30";
> function = "qup2";
> };
> };
>
> - qup_uart3_default: qup-uart3-default {
> - pinmux {
> + qup_uart3_default: qup-uart3-default-state {
> + default-pins {
> pins = "gpio43", "gpio44";
> function = "qup3";
> };
> };
>
> - qup_uart4_default: qup-uart4-default {
> - pinmux {
> + qup_uart4_default: qup-uart4-default-state {
> + default-pins {
> pins = "gpio91", "gpio92";
> function = "qup4";
> };
> };
>
> - qup_uart5_default: qup-uart5-default {
> - pinmux {
> + qup_uart5_default: qup-uart5-default-state {
> + default-pins {
> pins = "gpio87", "gpio88";
> function = "qup5";
> };
> };
>
> - qup_uart6_default: qup-uart6-default {
> - pinmux {
> + qup_uart6_default: qup-uart6-default-state {
> + default-pins {
> pins = "gpio47", "gpio48";
> function = "qup6";
> };
> };
>
> - qup_uart7_default: qup-uart7-default {
> - pinmux {
> + qup_uart7_default: qup-uart7-default-state {
> + default-pins {
> pins = "gpio95", "gpio96";
> function = "qup7";
> };
> };
>
> - qup_uart8_default: qup-uart8-default {
> - pinmux {
> + qup_uart8_default: qup-uart8-default-state {
> + default-pins {
> pins = "gpio67", "gpio68";
> function = "qup8";
> };
> };
>
> - qup_uart9_default: qup-uart9-default {
> - pinmux {
> + qup_uart9_default: qup-uart9-default-state {
> + default-pins {
> pins = "gpio4", "gpio5";
> function = "qup9";
> };
> };
>
> - qup_uart10_default: qup-uart10-default {
> - pinmux {
> + qup_uart10_default: qup-uart10-default-state {
> + default-pins {
> pins = "gpio53", "gpio54";
> function = "qup10";
> };
> };
>
> - qup_uart11_default: qup-uart11-default {
> - pinmux {
> + qup_uart11_default: qup-uart11-default-state {
> + default-pins {
> pins = "gpio33", "gpio34";
> function = "qup11";
> };
> };
>
> - qup_uart12_default: qup-uart12-default {
> - pinmux {
> + qup_uart12_default: qup-uart12-default-state {
> + default-pins {
> pins = "gpio51", "gpio52";
> function = "qup12";
> };
> };
>
> - qup_uart13_default: qup-uart13-default {
> - pinmux {
> + qup_uart13_default: qup-uart13-default-state {
> + default-pins {
> pins = "gpio107", "gpio108";
> function = "qup13";
> };
> };
>
> - qup_uart14_default: qup-uart14-default {
> - pinmux {
> + qup_uart14_default: qup-uart14-default-state {
> + default-pins {
> pins = "gpio31", "gpio32";
> function = "qup14";
> };
> };
>
> - qup_uart15_default: qup-uart15-default {
> - pinmux {
> + qup_uart15_default: qup-uart15-default-state {
> + default-pins {
> pins = "gpio83", "gpio84";
> function = "qup15";
> };
> };
>
> - quat_mi2s_sleep: quat_mi2s_sleep {
> - mux {
> + quat_mi2s_sleep: quat-mi2s-sleep-state {
> + default-pins {
> pins = "gpio58", "gpio59";
> function = "gpio";
> - };
> -
> - config {
> - pins = "gpio58", "gpio59";
> drive-strength = <2>;
> bias-pull-down;
> input-enable;
> };
> };
>
> - quat_mi2s_active: quat_mi2s_active {
> - mux {
> + quat_mi2s_active: quat-mi2s-active-state {
> + default-pins {
> pins = "gpio58", "gpio59";
> function = "qua_mi2s";
> - };
> -
> - config {
> - pins = "gpio58", "gpio59";
> drive-strength = <8>;
> bias-disable;
> output-high;
> };
> };
>
> - quat_mi2s_sd0_sleep: quat_mi2s_sd0_sleep {
> - mux {
> + quat_mi2s_sd0_sleep: quat-mi2s-sd0-sleep-state {
> + default-pins {
> pins = "gpio60";
> function = "gpio";
> - };
> -
> - config {
> - pins = "gpio60";
> drive-strength = <2>;
> bias-pull-down;
> input-enable;
> };
> };
>
> - quat_mi2s_sd0_active: quat_mi2s_sd0_active {
> - mux {
> + quat_mi2s_sd0_active: quat-mi2s-sd0-active-state {
> + default-pins {
> pins = "gpio60";
> function = "qua_mi2s";
> - };
> -
> - config {
> - pins = "gpio60";
> drive-strength = <8>;
> bias-disable;
> };
> };
>
> - quat_mi2s_sd1_sleep: quat_mi2s_sd1_sleep {
> - mux {
> + quat_mi2s_sd1_sleep: quat-mi2s-sd1-sleep-state {
> + default-pins {
> pins = "gpio61";
> function = "gpio";
> - };
> -
> - config {
> - pins = "gpio61";
> drive-strength = <2>;
> bias-pull-down;
> input-enable;
> };
> };
>
> - quat_mi2s_sd1_active: quat_mi2s_sd1_active {
> - mux {
> + quat_mi2s_sd1_active: quat-mi2s-sd1-active-state {
> + default-pins {
> pins = "gpio61";
> function = "qua_mi2s";
> - };
> -
> - config {
> - pins = "gpio61";
> drive-strength = <8>;
> bias-disable;
> };
> };
>
> - quat_mi2s_sd2_sleep: quat_mi2s_sd2_sleep {
> - mux {
> + quat_mi2s_sd2_sleep: quat-mi2s-sd2-sleep-state {
> + default-pins {
> pins = "gpio62";
> function = "gpio";
> - };
> -
> - config {
> - pins = "gpio62";
> drive-strength = <2>;
> bias-pull-down;
> input-enable;
> };
> };
>
> - quat_mi2s_sd2_active: quat_mi2s_sd2_active {
> - mux {
> + quat_mi2s_sd2_active: quat-mi2s-sd2-active-state {
> + default-pins {
> pins = "gpio62";
> function = "qua_mi2s";
> - };
> -
> - config {
> - pins = "gpio62";
> drive-strength = <8>;
> bias-disable;
> };
> };
>
> - quat_mi2s_sd3_sleep: quat_mi2s_sd3_sleep {
> - mux {
> + quat_mi2s_sd3_sleep: quat-mi2s-sd3-sleep-state {
> + default-pins {
> pins = "gpio63";
> function = "gpio";
> - };
> -
> - config {
> - pins = "gpio63";
> drive-strength = <2>;
> bias-pull-down;
> input-enable;
> };
> };
>
> - quat_mi2s_sd3_active: quat_mi2s_sd3_active {
> - mux {
> + quat_mi2s_sd3_active: quat-mi2s-sd3-active-state {
> + default-pins {
> pins = "gpio63";
> function = "qua_mi2s";
> - };
> -
> - config {
> - pins = "gpio63";
> drive-strength = <8>;
> bias-disable;
> };
> diff --git a/arch/arm64/boot/dts/qcom/sdm850-lenovo-yoga-c630.dts b/arch/arm64/boot/dts/qcom/sdm850-lenovo-yoga-c630.dts
> index 74f43da51fa5..e4473b31cea9 100644
> --- a/arch/arm64/boot/dts/qcom/sdm850-lenovo-yoga-c630.dts
> +++ b/arch/arm64/boot/dts/qcom/sdm850-lenovo-yoga-c630.dts
> @@ -479,7 +479,7 @@ &mss_pil {
> };
>
> &qup_i2c10_default {
> - pinconf {
> + default-pins {
> pins = "gpio55", "gpio56";
> drive-strength = <2>;
> bias-disable;
> @@ -487,31 +487,29 @@ pinconf {
> };
>
> &qup_i2c12_default {
> - pinmux {
> + default-pins {
> drive-strength = <2>;
> bias-disable;
> };
> };
>
> &qup_uart6_default {
> - pinmux {
> - pins = "gpio45", "gpio46", "gpio47", "gpio48";
> - function = "qup6";
> - };
> -
> - cts {
> + cts-pins {
> pins = "gpio45";
> + function = "qup6";
> bias-pull-down;
> };
>
> - rts-tx {
> + rts-tx-pins {
> pins = "gpio46", "gpio47";
> + function = "qup6";
> drive-strength = <2>;
> bias-disable;
> };
>
> - rx {
> + rx-pins {
> pins = "gpio48";
> + function = "qup6";
> bias-pull-up;
> };
> };
> @@ -621,13 +619,14 @@ codec {
> &tlmm {
> gpio-reserved-ranges = <0 4>, <81 4>;
>
> - sn65dsi86_pin_active: sn65dsi86-enable {
> + sn65dsi86_pin_active: sn65dsi86-enable-state {
> pins = "gpio96";
> + function = "gpio";
> drive-strength = <2>;
> bias-disable;
> };
>
> - i2c3_hid_active: i2c2-hid-active {
> + i2c3_hid_active: i2c2-hid-active-state {
> pins = "gpio37";
> function = "gpio";
>
> @@ -636,7 +635,7 @@ i2c3_hid_active: i2c2-hid-active {
> drive-strength = <2>;
> };
>
> - i2c5_hid_active: i2c5-hid-active {
> + i2c5_hid_active: i2c5-hid-active-state {
> pins = "gpio125";
> function = "gpio";
>
> @@ -645,7 +644,7 @@ i2c5_hid_active: i2c5-hid-active {
> drive-strength = <2>;
> };
>
> - i2c11_hid_active: i2c11-hid-active {
> + i2c11_hid_active: i2c11-hid-active-state {
> pins = "gpio92";
> function = "gpio";
>
> @@ -654,7 +653,7 @@ i2c11_hid_active: i2c11-hid-active {
> drive-strength = <2>;
> };
>
> - wcd_intr_default: wcd_intr_default {
> + wcd_intr_default: wcd-intr-default-state {
> pins = "gpio54";
> function = "gpio";
>
> @@ -663,7 +662,7 @@ wcd_intr_default: wcd_intr_default {
> drive-strength = <2>;
> };
>
> - lid_pin_active: lid-pin {
> + lid_pin_active: lid-pin-state {
> pins = "gpio124";
> function = "gpio";
>
> @@ -671,7 +670,7 @@ lid_pin_active: lid-pin {
> bias-disable;
> };
>
> - mode_pin_active: mode-pin {
> + mode_pin_active: mode-pin-state {
> pins = "gpio95";
> function = "gpio";
>
> diff --git a/arch/arm64/boot/dts/qcom/sdm850-samsung-w737.dts b/arch/arm64/boot/dts/qcom/sdm850-samsung-w737.dts
> index d028a7eb364a..75b612fb4775 100644
> --- a/arch/arm64/boot/dts/qcom/sdm850-samsung-w737.dts
> +++ b/arch/arm64/boot/dts/qcom/sdm850-samsung-w737.dts
> @@ -399,7 +399,7 @@ &mss_pil {
> };
>
> &qup_i2c10_default {
> - pinconf {
> + default-pins {
> pins = "gpio55", "gpio56";
> drive-strength = <2>;
> bias-disable;
> @@ -407,7 +407,7 @@ pinconf {
> };
>
> &qup_i2c11_default {
> - pinconf {
> + default-pins {
> pins = "gpio31", "gpio32";
> drive-strength = <2>;
> bias-disable;
> @@ -415,31 +415,29 @@ pinconf {
> };
>
> &qup_i2c12_default {
> - pinmux {
> + default-pins {
> drive-strength = <2>;
> bias-disable;
> };
> };
>
> &qup_uart6_default {
> - pinmux {
> - pins = "gpio45", "gpio46", "gpio47", "gpio48";
> - function = "qup6";
> - };
> -
> - cts {
> + cts-pins {
> pins = "gpio45";
> + function = "qup6";
> bias-pull-down;
> };
>
> - rts-tx {
> + rts-tx-pins {
> pins = "gpio46", "gpio47";
> + function = "qup6";
> drive-strength = <2>;
> bias-disable;
> };
>
> - rx {
> + rx-pins {
> pins = "gpio48";
> + function = "qup6";
> bias-pull-up;
> };
> };
> @@ -549,59 +547,41 @@ codec {
> &tlmm {
> gpio-reserved-ranges = <0 6>, <85 4>;
>
> - pen_irq_l: pen-irq-l {
> - pinmux {
> - pins = "gpio119";
> - function = "gpio";
> - };
> -
> - pinconf {
> - pins = "gpio119";
> - bias-disable;
> - };
> + pen_irq_l: pen-irq-l-state {
> + pins = "gpio119";
> + function = "gpio";
> + bias-disable;
> };
>
> - pen_pdct_l: pen-pdct-l {
> - pinmux {
> - pins = "gpio124";
> - function = "gpio";
> - };
> -
> - pinconf {
> - pins = "gpio124";
> - bias-disable;
> - drive-strength = <2>;
> - output-high;
> - };
> + pen_pdct_l: pen-pdct-l-state {
> + pins = "gpio124";
> + function = "gpio";
> + bias-disable;
> + drive-strength = <2>;
> + output-high;
> };
>
> - pen_rst_l: pen-rst-l {
> - pinmux {
> - pins = "gpio21";
> - function = "gpio";
> - };
> -
> - pinconf {
> - pins = "gpio21";
> - bias-disable;
> - drive-strength = <2>;
> -
> - /*
> - * The pen driver doesn't currently support
> - * driving this reset line. By specifying
> - * output-high here we're relying on the fact
> - * that this pin has a default pulldown at boot
> - * (which makes sure the pen was in reset if it
> - * was powered) and then we set it high here to
> - * take it out of reset. Better would be if the
> - * pen driver could control this and we could
> - * remove "output-high" here.
> - */
> - output-high;
> - };
> + pen_rst_l: pen-rst-l-state {
> + pins = "gpio21";
> + function = "gpio";
> + bias-disable;
> + drive-strength = <2>;
> +
> + /*
> + * The pen driver doesn't currently support
> + * driving this reset line. By specifying
> + * output-high here we're relying on the fact
> + * that this pin has a default pulldown at boot
> + * (which makes sure the pen was in reset if it
> + * was powered) and then we set it high here to
> + * take it out of reset. Better would be if the
> + * pen driver could control this and we could
> + * remove "output-high" here.
> + */
> + output-high;
> };
>
> - wcd_intr_default: wcd_intr_default {
> + wcd_intr_default: wcd-intr-default-state {
> pins = "gpio54";
> function = "gpio";
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] arm64: dts: qcom: sdm845: align TLMM pin configuration with DT schema
2022-09-30 20:13 ` Konrad Dybcio
@ 2022-09-30 20:19 ` Krzysztof Kozlowski
2022-09-30 20:19 ` Konrad Dybcio
0 siblings, 1 reply; 17+ messages in thread
From: Krzysztof Kozlowski @ 2022-09-30 20:19 UTC (permalink / raw)
To: Konrad Dybcio, Andy Gross, Bjorn Andersson, Linus Walleij,
Rob Herring, Krzysztof Kozlowski, linux-arm-msm, linux-gpio,
devicetree, linux-kernel
On 30/09/2022 22:13, Konrad Dybcio wrote:
>
>
> On 30.09.2022 22:05, Krzysztof Kozlowski wrote:
>> DT schema expects TLMM pin configuration nodes to be named with
>> '-state' suffix and their optional children with '-pins' suffix.
>>
>> The sdm854.dtsi file defined several pin configuration nodes which are
> 845
>
>> customized by the boards. Therefore keep a additional "default-pins"
>> node inside so the boards can add more of configuration nodes. Such
>> additional configuration nodes always need 'function' property now
>> (required by DT schema).
> Would it not make more sense to drop this and keep the properties in the
> root node while at it?
>
> Konrad
Not possible, because the boards set different bias/config for the pins:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/qcom/sdm845.dtsi?h=v6.0-rc7#n2988
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/qcom/sdm845-mtp.dts?h=v6.0-rc7#n729
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] arm64: dts: qcom: sdm845: align TLMM pin configuration with DT schema
2022-09-30 20:19 ` Krzysztof Kozlowski
@ 2022-09-30 20:19 ` Konrad Dybcio
0 siblings, 0 replies; 17+ messages in thread
From: Konrad Dybcio @ 2022-09-30 20:19 UTC (permalink / raw)
To: Krzysztof Kozlowski, Andy Gross, Bjorn Andersson, Linus Walleij,
Rob Herring, Krzysztof Kozlowski, linux-arm-msm, linux-gpio,
devicetree, linux-kernel
On 30.09.2022 22:19, Krzysztof Kozlowski wrote:
> On 30/09/2022 22:13, Konrad Dybcio wrote:
>>
>>
>> On 30.09.2022 22:05, Krzysztof Kozlowski wrote:
>>> DT schema expects TLMM pin configuration nodes to be named with
>>> '-state' suffix and their optional children with '-pins' suffix.
>>>
>>> The sdm854.dtsi file defined several pin configuration nodes which are
>> 845
>>
>>> customized by the boards. Therefore keep a additional "default-pins"
>>> node inside so the boards can add more of configuration nodes. Such
>>> additional configuration nodes always need 'function' property now
>>> (required by DT schema).
>> Would it not make more sense to drop this and keep the properties in the
>> root node while at it?
>>
>> Konrad
>
> Not possible, because the boards set different bias/config for the pins:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/qcom/sdm845.dtsi?h=v6.0-rc7#n2988
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/qcom/sdm845-mtp.dts?h=v6.0-rc7#n729
>
Ok, I see now.
Konrad
> Best regards,
> Krzysztof
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] dt-bindings: pinctrl: qcom,sdm845: convert to dtschema
2022-09-30 20:05 ` [PATCH 2/2] dt-bindings: pinctrl: qcom,sdm845: convert to dtschema Krzysztof Kozlowski
@ 2022-09-30 22:05 ` Rob Herring
2022-10-01 9:08 ` Krzysztof Kozlowski
2022-10-03 17:11 ` Rob Herring
2022-10-17 22:42 ` (subset) " Krzysztof Kozlowski
2 siblings, 1 reply; 17+ messages in thread
From: Rob Herring @ 2022-09-30 22:05 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Konrad Dybcio, devicetree, linux-gpio, Krzysztof Kozlowski,
Rob Herring, linux-kernel, Andy Gross, Bjorn Andersson,
linux-arm-msm, Linus Walleij
On Fri, 30 Sep 2022 22:05:29 +0200, Krzysztof Kozlowski wrote:
> Convert Qualcomm SDM845 pin controller bindings to DT schema. Keep
> the parsing of pin configuration subnodes consistent with other Qualcomm
> schemas (children named with '-state' suffix, their children with
> '-pins').
>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
> .../bindings/pinctrl/qcom,sdm845-pinctrl.txt | 176 ------------------
> .../bindings/pinctrl/qcom,sdm845-pinctrl.yaml | 158 ++++++++++++++++
> 2 files changed, 158 insertions(+), 176 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,sdm845-pinctrl.txt
> create mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,sdm845-pinctrl.yaml
>
Running 'make dtbs_check' with the schema in this patch gives the
following warnings. Consider if they are expected or the schema is
incorrect. These may not be new warnings.
Note that it is not yet a requirement to have 0 warnings for dtbs_check.
This will change in the future.
Full log is available here: https://patchwork.ozlabs.org/patch/
pinctrl@3400000: 'ap-edp-bklten', 'ap-suspend-l-hog', 'ap_suspend_l_assert', 'ap_suspend_l_deassert', 'bios-flash-wp-r-l', 'cci0-default', 'cci0-sleep', 'cci1-default', 'cci1-sleep', 'ec-ap-int-l', 'edp-brij-en', 'edp-brij-irq', 'en-pp3300-dx-edp', 'h1-ap-int-odl', 'pen-eject-odl', 'pen-irq-l', 'pen-pdct-l', 'pen-rst-l', 'qspi-clk', 'qspi-cs0', 'qspi-cs1', 'qspi-data01', 'qspi-data12', 'quat_mi2s_active', 'quat_mi2s_sd0_active', 'quat_mi2s_sd0_sleep', 'quat_mi2s_sd1_active', 'quat_mi2s_sd1_sleep', 'quat_mi2s_sd2_active', 'quat_mi2s_sd2_sleep', 'quat_mi2s_sd3_active', 'quat_mi2s_sd3_sleep', 'quat_mi2s_sleep', 'qup-i2c0-default', 'qup-i2c1-default', 'qup-i2c10-default', 'qup-i2c11-default', 'qup-i2c12-default', 'qup-i2c13-default', 'qup-i2c14-default', 'qup-i2c15-default', 'qup-i2c2-default', 'qup-i2c3-default', 'qup-i2c4-default', 'qup-i2c5-default', 'qup-i2c6-default', 'qup-i2c7-default', 'qup-i2c8-default', 'qup-i2c9-default', 'qup-spi0-default', 'qup-spi1-default', 'qup-spi10-defau
lt', 'qup-spi11-default', 'qup-spi12-default', 'qup-spi13-default', 'qup-spi14-default', 'qup-spi15-default', 'qup-spi2-default', 'qup-spi3-default', 'qup-spi4-default', 'qup-spi5-default', 'qup-spi6-default', 'qup-spi7-default', 'qup-spi8-default', 'qup-spi9-default', 'qup-uart0-default', 'qup-uart1-default', 'qup-uart10-default', 'qup-uart11-default', 'qup-uart12-default', 'qup-uart13-default', 'qup-uart14-default', 'qup-uart15-default', 'qup-uart2-default', 'qup-uart3-default', 'qup-uart4-default', 'qup-uart5-default', 'qup-uart6-default', 'qup-uart7-default', 'qup-uart8-default', 'qup-uart9-default', 'sd-cd-odl', 'sdc2-clk', 'sdc2-cmd', 'sdc2-data', 'ts-int-l', 'ts-reset-l' do not match any of the regexes: '-state$', 'pinctrl-[0-9]+'
arch/arm64/boot/dts/qcom/sdm845-cheza-r1.dtb
arch/arm64/boot/dts/qcom/sdm845-cheza-r2.dtb
arch/arm64/boot/dts/qcom/sdm845-cheza-r3.dtb
pinctrl@3400000: 'cam0_default', 'cam3_default', 'cci0-default', 'cci0-sleep', 'cci1-default', 'cci1-sleep', 'dsi-sw-sel', 'lt9611-irq', 'pcie0-default', 'pcie0-pwren', 'pcie1-default', 'qspi-clk', 'qspi-cs0', 'qspi-cs1', 'qspi-data01', 'qspi-data12', 'quat_mi2s_active', 'quat_mi2s_sd0_active', 'quat_mi2s_sd0_sleep', 'quat_mi2s_sd1_active', 'quat_mi2s_sd1_sleep', 'quat_mi2s_sd2_active', 'quat_mi2s_sd2_sleep', 'quat_mi2s_sd3_active', 'quat_mi2s_sd3_sleep', 'quat_mi2s_sleep', 'qup-i2c0-default', 'qup-i2c1-default', 'qup-i2c10-default', 'qup-i2c11-default', 'qup-i2c12-default', 'qup-i2c13-default', 'qup-i2c14-default', 'qup-i2c15-default', 'qup-i2c2-default', 'qup-i2c3-default', 'qup-i2c4-default', 'qup-i2c5-default', 'qup-i2c6-default', 'qup-i2c7-default', 'qup-i2c8-default', 'qup-i2c9-default', 'qup-spi0-default', 'qup-spi1-default', 'qup-spi10-default', 'qup-spi11-default', 'qup-spi12-default', 'qup-spi13-default', 'qup-spi14-default', 'qup-spi15-default', 'qup-spi2-default', 'qup-sp
i3-default', 'qup-spi4-default', 'qup-spi5-default', 'qup-spi6-default', 'qup-spi7-default', 'qup-spi8-default', 'qup-spi9-default', 'qup-uart0-default', 'qup-uart1-default', 'qup-uart10-default', 'qup-uart11-default', 'qup-uart12-default', 'qup-uart13-default', 'qup-uart14-default', 'qup-uart15-default', 'qup-uart2-default', 'qup-uart3-default', 'qup-uart4-default', 'qup-uart5-default', 'qup-uart6-default', 'qup-uart7-default', 'qup-uart8-default', 'qup-uart9-default', 'sd-card-det-n', 'sdc2-default', 'wcd_intr_default' do not match any of the regexes: '-state$', 'pinctrl-[0-9]+'
arch/arm64/boot/dts/qcom/sdm845-db845c.dtb
pinctrl@3400000: 'cci0-default', 'cci0-sleep', 'cci1-default', 'cci1-sleep', 'i2c11-hid-active', 'i2c2-hid-active', 'i2c5-hid-active', 'lid-pin', 'mode-pin', 'qspi-clk', 'qspi-cs0', 'qspi-cs1', 'qspi-data01', 'qspi-data12', 'quat_mi2s_active', 'quat_mi2s_sd0_active', 'quat_mi2s_sd0_sleep', 'quat_mi2s_sd1_active', 'quat_mi2s_sd1_sleep', 'quat_mi2s_sd2_active', 'quat_mi2s_sd2_sleep', 'quat_mi2s_sd3_active', 'quat_mi2s_sd3_sleep', 'quat_mi2s_sleep', 'qup-i2c0-default', 'qup-i2c1-default', 'qup-i2c10-default', 'qup-i2c11-default', 'qup-i2c12-default', 'qup-i2c13-default', 'qup-i2c14-default', 'qup-i2c15-default', 'qup-i2c2-default', 'qup-i2c3-default', 'qup-i2c4-default', 'qup-i2c5-default', 'qup-i2c6-default', 'qup-i2c7-default', 'qup-i2c8-default', 'qup-i2c9-default', 'qup-spi0-default', 'qup-spi1-default', 'qup-spi10-default', 'qup-spi11-default', 'qup-spi12-default', 'qup-spi13-default', 'qup-spi14-default', 'qup-spi15-default', 'qup-spi2-default', 'qup-spi3-default', 'qup-spi4-defau
lt', 'qup-spi5-default', 'qup-spi6-default', 'qup-spi7-default', 'qup-spi8-default', 'qup-spi9-default', 'qup-uart0-default', 'qup-uart1-default', 'qup-uart10-default', 'qup-uart11-default', 'qup-uart12-default', 'qup-uart13-default', 'qup-uart14-default', 'qup-uart15-default', 'qup-uart2-default', 'qup-uart3-default', 'qup-uart4-default', 'qup-uart5-default', 'qup-uart6-default', 'qup-uart7-default', 'qup-uart8-default', 'qup-uart9-default', 'sn65dsi86-enable', 'wcd_intr_default' do not match any of the regexes: '-state$', 'pinctrl-[0-9]+'
arch/arm64/boot/dts/qcom/sdm850-lenovo-yoga-c630.dtb
pinctrl@3400000: 'cci0-default', 'cci0-sleep', 'cci1-default', 'cci1-sleep', 'panel-esd', 'panel-reset', 'panel-te', 'qspi-clk', 'qspi-cs0', 'qspi-cs1', 'qspi-data01', 'qspi-data12', 'quat_mi2s_active', 'quat_mi2s_sd0_active', 'quat_mi2s_sd0_sleep', 'quat_mi2s_sd1_active', 'quat_mi2s_sd1_sleep', 'quat_mi2s_sd2_active', 'quat_mi2s_sd2_sleep', 'quat_mi2s_sd3_active', 'quat_mi2s_sd3_sleep', 'quat_mi2s_sleep', 'qup-i2c0-default', 'qup-i2c1-default', 'qup-i2c10-default', 'qup-i2c11-default', 'qup-i2c12-default', 'qup-i2c13-default', 'qup-i2c14-default', 'qup-i2c15-default', 'qup-i2c2-default', 'qup-i2c3-default', 'qup-i2c4-default', 'qup-i2c5-default', 'qup-i2c6-default', 'qup-i2c7-default', 'qup-i2c8-default', 'qup-i2c9-default', 'qup-spi0-default', 'qup-spi1-default', 'qup-spi10-default', 'qup-spi11-default', 'qup-spi12-default', 'qup-spi13-default', 'qup-spi14-default', 'qup-spi15-default', 'qup-spi2-default', 'qup-spi3-default', 'qup-spi4-default', 'qup-spi5-default', 'qup-spi6-defaul
t', 'qup-spi7-default', 'qup-spi8-default', 'qup-spi9-default', 'qup-uart0-default', 'qup-uart1-default', 'qup-uart10-default', 'qup-uart11-default', 'qup-uart12-default', 'qup-uart13-default', 'qup-uart14-default', 'qup-uart15-default', 'qup-uart2-default', 'qup-uart3-default', 'qup-uart4-default', 'qup-uart5-default', 'qup-uart6-default', 'qup-uart7-default', 'qup-uart8-default', 'qup-uart9-default', 'tri_state_key_default', 'ts-int' do not match any of the regexes: '-state$', 'pinctrl-[0-9]+'
arch/arm64/boot/dts/qcom/sdm845-oneplus-enchilada.dtb
arch/arm64/boot/dts/qcom/sdm845-oneplus-fajita.dtb
pinctrl@3400000: 'cci0-default', 'cci0-sleep', 'cci1-default', 'cci1-sleep', 'pen-irq-l', 'pen-pdct-l', 'pen-rst-l', 'qspi-clk', 'qspi-cs0', 'qspi-cs1', 'qspi-data01', 'qspi-data12', 'quat_mi2s_active', 'quat_mi2s_sd0_active', 'quat_mi2s_sd0_sleep', 'quat_mi2s_sd1_active', 'quat_mi2s_sd1_sleep', 'quat_mi2s_sd2_active', 'quat_mi2s_sd2_sleep', 'quat_mi2s_sd3_active', 'quat_mi2s_sd3_sleep', 'quat_mi2s_sleep', 'qup-i2c0-default', 'qup-i2c1-default', 'qup-i2c10-default', 'qup-i2c11-default', 'qup-i2c12-default', 'qup-i2c13-default', 'qup-i2c14-default', 'qup-i2c15-default', 'qup-i2c2-default', 'qup-i2c3-default', 'qup-i2c4-default', 'qup-i2c5-default', 'qup-i2c6-default', 'qup-i2c7-default', 'qup-i2c8-default', 'qup-i2c9-default', 'qup-spi0-default', 'qup-spi1-default', 'qup-spi10-default', 'qup-spi11-default', 'qup-spi12-default', 'qup-spi13-default', 'qup-spi14-default', 'qup-spi15-default', 'qup-spi2-default', 'qup-spi3-default', 'qup-spi4-default', 'qup-spi5-default', 'qup-spi6-defaul
t', 'qup-spi7-default', 'qup-spi8-default', 'qup-spi9-default', 'qup-uart0-default', 'qup-uart1-default', 'qup-uart10-default', 'qup-uart11-default', 'qup-uart12-default', 'qup-uart13-default', 'qup-uart14-default', 'qup-uart15-default', 'qup-uart2-default', 'qup-uart3-default', 'qup-uart4-default', 'qup-uart5-default', 'qup-uart6-default', 'qup-uart7-default', 'qup-uart8-default', 'qup-uart9-default', 'wcd_intr_default' do not match any of the regexes: '-state$', 'pinctrl-[0-9]+'
arch/arm64/boot/dts/qcom/sdm850-samsung-w737.dtb
pinctrl@3400000: 'cci0-default', 'cci0-sleep', 'cci1-default', 'cci1-sleep', 'qspi-clk', 'qspi-cs0', 'qspi-cs1', 'qspi-data01', 'qspi-data12', 'quat_mi2s_active', 'quat_mi2s_sd0_active', 'quat_mi2s_sd0_sleep', 'quat_mi2s_sd1_active', 'quat_mi2s_sd1_sleep', 'quat_mi2s_sd2_active', 'quat_mi2s_sd2_sleep', 'quat_mi2s_sd3_active', 'quat_mi2s_sd3_sleep', 'quat_mi2s_sleep', 'qup-i2c0-default', 'qup-i2c1-default', 'qup-i2c10-default', 'qup-i2c11-default', 'qup-i2c12-default', 'qup-i2c13-default', 'qup-i2c14-default', 'qup-i2c15-default', 'qup-i2c2-default', 'qup-i2c3-default', 'qup-i2c4-default', 'qup-i2c5-default', 'qup-i2c6-default', 'qup-i2c7-default', 'qup-i2c8-default', 'qup-i2c9-default', 'qup-spi0-default', 'qup-spi1-default', 'qup-spi10-default', 'qup-spi11-default', 'qup-spi12-default', 'qup-spi13-default', 'qup-spi14-default', 'qup-spi15-default', 'qup-spi2-default', 'qup-spi3-default', 'qup-spi4-default', 'qup-spi5-default', 'qup-spi6-default', 'qup-spi7-default', 'qup-spi8-defaul
t', 'qup-spi9-default', 'qup-uart0-default', 'qup-uart1-default', 'qup-uart10-default', 'qup-uart11-default', 'qup-uart12-default', 'qup-uart13-default', 'qup-uart14-default', 'qup-uart15-default', 'qup-uart2-default', 'qup-uart3-default', 'qup-uart4-default', 'qup-uart5-default', 'qup-uart6-default', 'qup-uart7-default', 'qup-uart8-default', 'qup-uart9-default' do not match any of the regexes: '-state$', 'pinctrl-[0-9]+'
arch/arm64/boot/dts/qcom/sdm845-sony-xperia-tama-akari.dtb
arch/arm64/boot/dts/qcom/sdm845-sony-xperia-tama-akatsuki.dtb
arch/arm64/boot/dts/qcom/sdm845-sony-xperia-tama-apollo.dtb
pinctrl@3400000: 'cci0-default', 'cci0-sleep', 'cci1-default', 'cci1-sleep', 'qspi-clk', 'qspi-cs0', 'qspi-cs1', 'qspi-data01', 'qspi-data12', 'quat_mi2s_active', 'quat_mi2s_sd0_active', 'quat_mi2s_sd0_sleep', 'quat_mi2s_sd1_active', 'quat_mi2s_sd1_sleep', 'quat_mi2s_sd2_active', 'quat_mi2s_sd2_sleep', 'quat_mi2s_sd3_active', 'quat_mi2s_sd3_sleep', 'quat_mi2s_sleep', 'qup-i2c0-default', 'qup-i2c1-default', 'qup-i2c10-default', 'qup-i2c11-default', 'qup-i2c12-default', 'qup-i2c13-default', 'qup-i2c14-default', 'qup-i2c15-default', 'qup-i2c2-default', 'qup-i2c3-default', 'qup-i2c4-default', 'qup-i2c5-default', 'qup-i2c6-default', 'qup-i2c7-default', 'qup-i2c8-default', 'qup-i2c9-default', 'qup-spi0-default', 'qup-spi1-default', 'qup-spi10-default', 'qup-spi11-default', 'qup-spi12-default', 'qup-spi13-default', 'qup-spi14-default', 'qup-spi15-default', 'qup-spi2-default', 'qup-spi3-default', 'qup-spi4-default', 'qup-spi5-default', 'qup-spi6-default', 'qup-spi7-default', 'qup-spi8-defaul
t', 'qup-spi9-default', 'qup-uart0-default', 'qup-uart1-default', 'qup-uart10-default', 'qup-uart11-default', 'qup-uart12-default', 'qup-uart13-default', 'qup-uart14-default', 'qup-uart15-default', 'qup-uart2-default', 'qup-uart3-default', 'qup-uart4-default', 'qup-uart5-default', 'qup-uart6-default', 'qup-uart7-default', 'qup-uart8-default', 'qup-uart9-default', 'sd-card-det-n', 'sdc2-clk', 'sdc2-cmd', 'sdc2-data' do not match any of the regexes: '-state$', 'pinctrl-[0-9]+'
arch/arm64/boot/dts/qcom/sdm845-lg-judyp.dtb
arch/arm64/boot/dts/qcom/sdm845-mtp.dtb
pinctrl@3400000: 'cci0-default', 'cci0-sleep', 'cci1-default', 'cci1-sleep', 'qspi-clk', 'qspi-cs0', 'qspi-cs1', 'qspi-data01', 'qspi-data12', 'quat_mi2s_active', 'quat_mi2s_sd0_active', 'quat_mi2s_sd0_sleep', 'quat_mi2s_sd1_active', 'quat_mi2s_sd1_sleep', 'quat_mi2s_sd2_active', 'quat_mi2s_sd2_sleep', 'quat_mi2s_sd3_active', 'quat_mi2s_sd3_sleep', 'quat_mi2s_sleep', 'qup-i2c0-default', 'qup-i2c1-default', 'qup-i2c10-default', 'qup-i2c11-default', 'qup-i2c12-default', 'qup-i2c13-default', 'qup-i2c14-default', 'qup-i2c15-default', 'qup-i2c2-default', 'qup-i2c3-default', 'qup-i2c4-default', 'qup-i2c5-default', 'qup-i2c6-default', 'qup-i2c7-default', 'qup-i2c8-default', 'qup-i2c9-default', 'qup-spi0-default', 'qup-spi1-default', 'qup-spi10-default', 'qup-spi11-default', 'qup-spi12-default', 'qup-spi13-default', 'qup-spi14-default', 'qup-spi15-default', 'qup-spi2-default', 'qup-spi3-default', 'qup-spi4-default', 'qup-spi5-default', 'qup-spi6-default', 'qup-spi7-default', 'qup-spi8-defaul
t', 'qup-spi9-default', 'qup-uart0-default', 'qup-uart1-default', 'qup-uart10-default', 'qup-uart11-default', 'qup-uart12-default', 'qup-uart13-default', 'qup-uart14-default', 'qup-uart15-default', 'qup-uart2-default', 'qup-uart3-default', 'qup-uart4-default', 'qup-uart5-default', 'qup-uart6-default', 'qup-uart7-default', 'qup-uart8-default', 'qup-uart9-default', 'sd-card-det-n', 'sdc2-clk', 'sdc2-cmd', 'sdc2-data', 'thinq-key-default' do not match any of the regexes: '-state$', 'pinctrl-[0-9]+'
arch/arm64/boot/dts/qcom/sdm845-lg-judyln.dtb
pinctrl@3400000: 'cci0-default', 'cci0-sleep', 'cci1-default', 'cci1-sleep', 'qspi-clk', 'qspi-cs0', 'qspi-cs1', 'qspi-data01', 'qspi-data12', 'quat_mi2s_active', 'quat_mi2s_sd0_active', 'quat_mi2s_sd0_sleep', 'quat_mi2s_sd1_active', 'quat_mi2s_sd1_sleep', 'quat_mi2s_sd2_active', 'quat_mi2s_sd2_sleep', 'quat_mi2s_sd3_active', 'quat_mi2s_sd3_sleep', 'quat_mi2s_sleep', 'qup-i2c0-default', 'qup-i2c1-default', 'qup-i2c10-default', 'qup-i2c11-default', 'qup-i2c12-default', 'qup-i2c13-default', 'qup-i2c14-default', 'qup-i2c15-default', 'qup-i2c2-default', 'qup-i2c3-default', 'qup-i2c4-default', 'qup-i2c5-default', 'qup-i2c6-default', 'qup-i2c7-default', 'qup-i2c8-default', 'qup-i2c9-default', 'qup-spi0-default', 'qup-spi1-default', 'qup-spi10-default', 'qup-spi11-default', 'qup-spi12-default', 'qup-spi13-default', 'qup-spi14-default', 'qup-spi15-default', 'qup-spi2-default', 'qup-spi3-default', 'qup-spi4-default', 'qup-spi5-default', 'qup-spi6-default', 'qup-spi7-default', 'qup-spi8-defaul
t', 'qup-spi9-default', 'qup-uart0-default', 'qup-uart1-default', 'qup-uart10-default', 'qup-uart11-default', 'qup-uart12-default', 'qup-uart13-default', 'qup-uart14-default', 'qup-uart15-default', 'qup-uart2-default', 'qup-uart3-default', 'qup-uart4-default', 'qup-uart5-default', 'qup-uart6-default', 'qup-uart7-default', 'qup-uart8-default', 'qup-uart9-default', 'sd-card-det-n', 'sdc2-default', 'wcd_intr_default' do not match any of the regexes: '-state$', 'pinctrl-[0-9]+'
arch/arm64/boot/dts/qcom/sdm845-xiaomi-beryllium.dtb
pinctrl@3400000: 'cci0-default', 'cci0-sleep', 'cci1-default', 'cci1-sleep', 'qspi-clk', 'qspi-cs0', 'qspi-cs1', 'qspi-data01', 'qspi-data12', 'quat_mi2s_active', 'quat_mi2s_sd0_active', 'quat_mi2s_sd0_sleep', 'quat_mi2s_sd1_active', 'quat_mi2s_sd1_sleep', 'quat_mi2s_sd2_active', 'quat_mi2s_sd2_sleep', 'quat_mi2s_sd3_active', 'quat_mi2s_sd3_sleep', 'quat_mi2s_sleep', 'qup-i2c0-default', 'qup-i2c1-default', 'qup-i2c10-default', 'qup-i2c11-default', 'qup-i2c12-default', 'qup-i2c13-default', 'qup-i2c14-default', 'qup-i2c15-default', 'qup-i2c2-default', 'qup-i2c3-default', 'qup-i2c4-default', 'qup-i2c5-default', 'qup-i2c6-default', 'qup-i2c7-default', 'qup-i2c8-default', 'qup-i2c9-default', 'qup-spi0-default', 'qup-spi1-default', 'qup-spi10-default', 'qup-spi11-default', 'qup-spi12-default', 'qup-spi13-default', 'qup-spi14-default', 'qup-spi15-default', 'qup-spi2-default', 'qup-spi3-default', 'qup-spi4-default', 'qup-spi5-default', 'qup-spi6-default', 'qup-spi7-default', 'qup-spi8-defaul
t', 'qup-spi9-default', 'qup-uart0-default', 'qup-uart1-default', 'qup-uart10-default', 'qup-uart11-default', 'qup-uart12-default', 'qup-uart13-default', 'qup-uart14-default', 'qup-uart15-default', 'qup-uart2-default', 'qup-uart3-default', 'qup-uart4-default', 'qup-uart5-default', 'qup-uart6-default', 'qup-uart7-default', 'qup-uart8-default', 'qup-uart9-default', 'sde-dsi-active', 'sde-dsi-suspend', 'sde-te-active', 'sde-te-suspend', 'ts-int-active', 'ts-int-suspend', 'ts-reset-active', 'ts-reset-suspend' do not match any of the regexes: '-state$', 'pinctrl-[0-9]+'
arch/arm64/boot/dts/qcom/sdm845-shift-axolotl.dtb
pinctrl@3400000: 'cci0-default', 'cci0-sleep', 'cci1-default', 'cci1-sleep', 'qspi-clk', 'qspi-cs0', 'qspi-cs1', 'qspi-data01', 'qspi-data12', 'quat_mi2s_active', 'quat_mi2s_sd0_active', 'quat_mi2s_sd0_sleep', 'quat_mi2s_sd1_active', 'quat_mi2s_sd1_sleep', 'quat_mi2s_sd2_active', 'quat_mi2s_sd2_sleep', 'quat_mi2s_sd3_active', 'quat_mi2s_sd3_sleep', 'quat_mi2s_sleep', 'qup-i2c0-default', 'qup-i2c1-default', 'qup-i2c10-default', 'qup-i2c11-default', 'qup-i2c12-default', 'qup-i2c13-default', 'qup-i2c14-default', 'qup-i2c15-default', 'qup-i2c2-default', 'qup-i2c3-default', 'qup-i2c4-default', 'qup-i2c5-default', 'qup-i2c6-default', 'qup-i2c7-default', 'qup-i2c8-default', 'qup-i2c9-default', 'qup-spi0-default', 'qup-spi1-default', 'qup-spi10-default', 'qup-spi11-default', 'qup-spi12-default', 'qup-spi13-default', 'qup-spi14-default', 'qup-spi15-default', 'qup-spi2-default', 'qup-spi3-default', 'qup-spi4-default', 'qup-spi5-default', 'qup-spi6-default', 'qup-spi7-default', 'qup-spi8-defaul
t', 'qup-spi9-default', 'qup-uart0-default', 'qup-uart1-default', 'qup-uart10-default', 'qup-uart11-default', 'qup-uart12-default', 'qup-uart13-default', 'qup-uart14-default', 'qup-uart15-default', 'qup-uart2-default', 'qup-uart3-default', 'qup-uart4-default', 'qup-uart5-default', 'qup-uart6-default', 'qup-uart7-default', 'qup-uart8-default', 'qup-uart9-default', 'sde-dsi-active', 'sde-dsi-suspend', 'ts-int-default', 'ts-int-sleep', 'ts-reset-default', 'ts-reset-sleep', 'wcd-intr-default' do not match any of the regexes: '-state$', 'pinctrl-[0-9]+'
arch/arm64/boot/dts/qcom/sdm845-xiaomi-polaris.dtb
pinctrl@3400000: sdc2-default-state: 'oneOf' conditional failed, one must be fixed:
arch/arm64/boot/dts/qcom/sdm845-sony-xperia-tama-akari.dtb
arch/arm64/boot/dts/qcom/sdm845-sony-xperia-tama-akatsuki.dtb
arch/arm64/boot/dts/qcom/sdm845-sony-xperia-tama-apollo.dtb
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] dt-bindings: pinctrl: qcom,sdm845: convert to dtschema
2022-09-30 22:05 ` Rob Herring
@ 2022-10-01 9:08 ` Krzysztof Kozlowski
2022-10-03 17:10 ` Rob Herring
0 siblings, 1 reply; 17+ messages in thread
From: Krzysztof Kozlowski @ 2022-10-01 9:08 UTC (permalink / raw)
To: Rob Herring, Bjorn Andersson, Linus Walleij
Cc: Konrad Dybcio, devicetree, linux-gpio, Krzysztof Kozlowski,
Rob Herring, linux-kernel, Andy Gross, linux-arm-msm
On 01/10/2022 00:05, Rob Herring wrote:
> On Fri, 30 Sep 2022 22:05:29 +0200, Krzysztof Kozlowski wrote:
>> Convert Qualcomm SDM845 pin controller bindings to DT schema. Keep
>> the parsing of pin configuration subnodes consistent with other Qualcomm
>> schemas (children named with '-state' suffix, their children with
>> '-pins').
>>
>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> ---
>> .../bindings/pinctrl/qcom,sdm845-pinctrl.txt | 176 ------------------
>> .../bindings/pinctrl/qcom,sdm845-pinctrl.yaml | 158 ++++++++++++++++
>> 2 files changed, 158 insertions(+), 176 deletions(-)
>> delete mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,sdm845-pinctrl.txt
>> create mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,sdm845-pinctrl.yaml
>>
>
> Running 'make dtbs_check' with the schema in this patch gives the
> following warnings. Consider if they are expected or the schema is
> incorrect. These may not be new warnings.
>
> Note that it is not yet a requirement to have 0 warnings for dtbs_check.
> This will change in the future.
>
> Full log is available here: https://patchwork.ozlabs.org/patch/
>
>
> pinctrl@3400000: 'ap-edp-bklten', 'ap-suspend-l-hog', 'ap_suspend_l_assert', 'ap_suspend_l_deassert', 'bios-flash-wp-r-l', 'cci0-default', 'cci0-sleep', 'cci1-default', 'cci1-sleep', 'ec-ap-int-l', 'edp-brij-en', 'edp-brij-irq', 'en-pp3300-dx-edp', 'h1-ap-int-odl', 'pen-eject-odl', 'pen-irq-l', 'pen-pdct-l', 'pen-rst-l', 'qspi-clk', 'qspi-cs0', 'qspi-cs1', 'qspi-data01', 'qspi-data12', 'quat_mi2s_active', 'quat_mi2s_sd0_active', 'quat_mi2s_sd0_sleep', 'quat_mi2s_sd1_active', 'quat_mi2s_sd1_sleep', 'quat_mi2s_sd2_active', 'quat_mi2s_sd2_sleep', 'quat_mi2s_sd3_active', 'quat_mi2s_sd3_sleep', 'quat_mi2s_sleep', 'qup-i2c0-default', 'qup-i2c1-default', 'qup-i2c10-default', 'qup-i2c11-default', 'qup-i2c12-default', 'qup-i2c13-default', 'qup-i2c14-default', 'qup-i2c15-default', 'qup-i2c2-default', 'qup-i2c3-default', 'qup-i2c4-default', 'qup-i2c5-default', 'qup-i2c6-default', 'qup-i2c7-default', 'qup-i2c8-default', 'qup-i2c9-default', 'qup-spi0-default', 'qup-spi1-default', 'qup-spi10-default', 'qup-spi11-default', 'qup-spi12-default', 'qup-spi13-default', 'qup-spi14-default', 'qup-spi15-default', 'qup-spi2-default', 'qup-spi3-default', 'qup-spi4-default', 'qup-spi5-default', 'qup-spi6-default', 'qup-spi7-default', 'qup-spi8-default', 'qup-spi9-default', 'qup-uart0-default', 'qup-uart1-default', 'qup-uart10-default', 'qup-uart11-default', 'qup-uart12-default', 'qup-uart13-default', 'qup-uart14-default', 'qup-uart15-default', 'qup-uart2-default', 'qup-uart3-default', 'qup-uart4-default', 'qup-uart5-default', 'qup-uart6-default', 'qup-uart7-default', 'qup-uart8-default', 'qup-uart9-default', 'sd-cd-odl', 'sdc2-clk', 'sdc2-cmd', 'sdc2-data', 'ts-int-l', 'ts-reset-l' do not match any of the regexes: '-state$', 'pinctrl-[0-9]+'
> arch/arm64/boot/dts/qcom/sdm845-cheza-r1.dtb
> arch/arm64/boot/dts/qcom/sdm845-cheza-r2.dtb
> arch/arm64/boot/dts/qcom/sdm845-cheza-r3.dtb
This is fixed in patch #1 in this set, however for some reason DT
patchwork did not get it (maybe size too big?).
For reference, the patch is available in Qualcomm patchwork:
https://patchwork.kernel.org/project/linux-arm-msm/patch/20220930200529.331223-2-krzysztof.kozlowski@linaro.org/
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] arm64: dts: qcom: sdm845: align TLMM pin configuration with DT schema
2022-09-30 20:05 ` [PATCH 1/2] arm64: dts: qcom: sdm845: align TLMM pin configuration with " Krzysztof Kozlowski
2022-09-30 20:13 ` Konrad Dybcio
@ 2022-10-03 16:14 ` Doug Anderson
2022-10-03 17:45 ` Krzysztof Kozlowski
1 sibling, 1 reply; 17+ messages in thread
From: Doug Anderson @ 2022-10-03 16:14 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Andy Gross, Bjorn Andersson, Konrad Dybcio, Linus Walleij,
Rob Herring, Krzysztof Kozlowski, linux-arm-msm,
open list:GPIO SUBSYSTEM,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML
Hi,
On Fri, Sep 30, 2022 at 1:06 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> DT schema expects TLMM pin configuration nodes to be named with
> '-state' suffix and their optional children with '-pins' suffix.
>
> The sdm854.dtsi file defined several pin configuration nodes which are
> customized by the boards. Therefore keep a additional "default-pins"
> node inside so the boards can add more of configuration nodes. Such
> additional configuration nodes always need 'function' property now
> (required by DT schema).
>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
> arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi | 344 +++++++-----------
> arch/arm64/boot/dts/qcom/sdm845-db845c.dts | 76 ++--
> .../arm64/boot/dts/qcom/sdm845-lg-common.dtsi | 60 ++-
> arch/arm64/boot/dts/qcom/sdm845-lg-judyln.dts | 2 +-
> arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 60 ++-
> .../boot/dts/qcom/sdm845-oneplus-common.dtsi | 88 ++---
> .../boot/dts/qcom/sdm845-shift-axolotl.dts | 138 +++----
> .../dts/qcom/sdm845-sony-xperia-tama.dtsi | 6 +-
> .../boot/dts/qcom/sdm845-xiaomi-beryllium.dts | 26 +-
> .../boot/dts/qcom/sdm845-xiaomi-polaris.dts | 30 +-
> arch/arm64/boot/dts/qcom/sdm845.dtsi | 305 +++++++---------
> .../boot/dts/qcom/sdm850-lenovo-yoga-c630.dts | 33 +-
> .../boot/dts/qcom/sdm850-samsung-w737.dts | 96 ++---
> 13 files changed, 513 insertions(+), 751 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi b/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi
> index b5f11fbcc300..3403cdcdd49c 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi
> @@ -993,21 +993,21 @@ &wifi {
> /* PINCTRL - additions to nodes defined in sdm845.dtsi */
>
> &qspi_cs0 {
> - pinconf {
> + default-pins {
> pins = "gpio90";
> bias-disable;
> };
> };
>
> &qspi_clk {
> - pinconf {
> + default-pins {
> pins = "gpio95";
> bias-disable;
> };
> };
>
> &qspi_data01 {
> - pinconf {
> + default-pins {
> pins = "gpio91", "gpio92";
I haven't been fully involved in all the discussion here, but the
above doesn't look like it matches the way that Bjorn wanted to go
[1]. I would sorta expect it to look like this:
/* QSPI always needs a clock and IO pins */
qspi_basic: {
qspi_clk: {
pins = "gpio95";
function = "qspi_clk";
};
qspi_data01: {
pins = "gpio95";
function = "qspi_clk";
};
}
/* QSPI will need one or both chip selects */
qspi_cs0: qspi-cs0-state {
pins = "gpio90";
function = "qspi_cs";
};
qspi_cs1: qspi-cs1-state {
pins = "gpio89";
function = "qspi_cs";
};
/* If using all 4 data lines we need these */
qspi_data12: qspi-data12-state {
pins = "gpio93", "gpio94";
function = "qspi_data";
};
Basically grouping things together in a two-level node when it makes
sense and then using 1-level nodes for "mixin" pins. Then you'd end up
doing one of these things:
pinctrl-0 = <&qspi_basic &qspi_cs0>;
pinctrl-0 = <&qspi_basic &qspi_cs1>;
pinctrl-0 = <&qspi_basic &qspi_cs0 &qspi_data12>;
Note that the extra tags of "qspi_clk" and "qspi_data01" are important
since it lets the individual boards easily set pulls / drive strengths
without needing to replicate the hierarchy of the SoC. So if a board
wanted to set the pull of the cs0 line, just:
&qspi_cs0 {
bias-disable;
};
[1] https://lore.kernel.org/lkml/CAD=FV=VUL4GmjaibAMhKNdpEso_Hg_R=XeMaqah1LSj_9-Ce4Q@mail.gmail.com/
> @@ -1016,7 +1016,7 @@ pinconf {
> };
>
> &qup_i2c3_default {
> - pinconf {
> + default-pins {
> pins = "gpio41", "gpio42";
> drive-strength = <2>;
I don't see any benefit to two-levels above. Why not just get rid of
the "default-pins" and put the stuff directly under qup_i2c3_default?
> /* PINCTRL - additions to nodes defined in sdm845.dtsi */
> &qup_spi2_default {
> - pinmux {
> + default-pins {
> drive-strength = <16>;
> };
> };
>
> &qup_uart3_default{
> - pinmux {
> + default-pins {
> pins = "gpio41", "gpio42", "gpio43", "gpio44";
> function = "qup3";
> };
> };
>
> &qup_i2c10_default {
> - pinconf {
> + default-pins {
> pins = "gpio55", "gpio56";
> drive-strength = <2>;
> bias-disable;
> @@ -1144,37 +1144,37 @@ pinconf {
> };
>
> &qup_uart6_default {
> - pinmux {
> - pins = "gpio45", "gpio46", "gpio47", "gpio48";
> - function = "qup6";
> - };
> -
> - cts {
> + cts-pins {
> pins = "gpio45";
> + function = "qup6";
> bias-disable;
> };
>
> - rts-tx {
> + rts-tx-pins {
> pins = "gpio46", "gpio47";
> + function = "qup6";
> drive-strength = <2>;
> bias-disable;
> };
>
> - rx {
> + rx-pins {
> pins = "gpio48";
> + function = "qup6";
> bias-pull-up;
> };
> };
I didn't check everything about this patch, but skimming through I
believe that the uart6 handling here is wrong. You'll end up with:
qup_uart6_default: qup-uart6-default-state {
default-pins {
pins = "gpio47", "gpio48";
function = "qup6";
};
cts-pins {
pins = "gpio45";
function = "qup6";
bias-disable;
};
rts-tx-pins {
pins = "gpio46", "gpio47";
function = "qup6";
drive-strength = <2>;
bias-disable;
};
rx-pins {
pins = "gpio48";
function = "qup6";
bias-pull-up;
};
};
So pins 47 and 48 are each referenced in two nodes. That doesn't seem
correct to me. IMO, better would have been:
In Soc.dtsi:
qup_uart6_txrx: qup-uart6-txrx-state {
qup_uart6_tx {
pins = "gpio47";
function = "qup6";
};
qup_uart6_rx {
pins = "gpio48";
function = "qup6";
};
};
qup_uart6_cts: qup-uart6-cts-state {
pins = "gpio45";
function = "qup6";
};
qup_uart6_rts: qup-uart6-rts-state {
pins = "gpio46";
function = "qup6";
};
In board.dts:
&qup_uart6_cts {
bias-disable;
};
&qup_uart6_rts {
drive-strength = <2>;
bias-disable;
};
&qup_uart6_rx {
bias-pull-up;
};
&qup_uart6_tx {
drive-strength = <2>;
bias-disable;
};
Also, as per latest agreement with Bjorn, we should be moving the
default drive strength to the SoC.dtsi file, so going further:
In Soc.dtsi:
qup_uart6_txrx: qup-uart6-txrx-state {
qup_uart6_tx {
pins = "gpio47";
function = "qup6";
drive-strength = <2>;
};
qup_uart6_rx {
pins = "gpio48";
function = "qup6";
};
};
qup_uart6_cts: qup-uart6-cts-state {
pins = "gpio45";
function = "qup6";
};
qup_uart6_rts: qup-uart6-rts-state {
pins = "gpio46";
function = "qup6";
drive-strength = <2>;
};
In board.dts:
&qup_uart6_cts {
bias-disable;
};
&qup_uart6_rts {
bias-disable;
};
&qup_uart6_rx {
bias-pull-up;
};
&qup_uart6_tx {
bias-disable;
};
In the SoC.dtsi file we could default to just a tx/rx config:
pinctrl-0 = <&qup_uart6_txrx>;
...if a board had the flow control lines hooked up, it could do:
pinctrl-0 = <&qup_uart6_txrx &qup_uart6_cts &qup_uart6_rts>;
-Doug
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] dt-bindings: pinctrl: qcom,sdm845: convert to dtschema
2022-10-01 9:08 ` Krzysztof Kozlowski
@ 2022-10-03 17:10 ` Rob Herring
0 siblings, 0 replies; 17+ messages in thread
From: Rob Herring @ 2022-10-03 17:10 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Bjorn Andersson, Linus Walleij, Konrad Dybcio, devicetree,
linux-gpio, Krzysztof Kozlowski, linux-kernel, Andy Gross,
linux-arm-msm
On Sat, Oct 01, 2022 at 11:08:58AM +0200, Krzysztof Kozlowski wrote:
> On 01/10/2022 00:05, Rob Herring wrote:
> > On Fri, 30 Sep 2022 22:05:29 +0200, Krzysztof Kozlowski wrote:
> >> Convert Qualcomm SDM845 pin controller bindings to DT schema. Keep
> >> the parsing of pin configuration subnodes consistent with other Qualcomm
> >> schemas (children named with '-state' suffix, their children with
> >> '-pins').
> >>
> >> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> >> ---
> >> .../bindings/pinctrl/qcom,sdm845-pinctrl.txt | 176 ------------------
> >> .../bindings/pinctrl/qcom,sdm845-pinctrl.yaml | 158 ++++++++++++++++
> >> 2 files changed, 158 insertions(+), 176 deletions(-)
> >> delete mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,sdm845-pinctrl.txt
> >> create mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,sdm845-pinctrl.yaml
> >>
> >
> > Running 'make dtbs_check' with the schema in this patch gives the
> > following warnings. Consider if they are expected or the schema is
> > incorrect. These may not be new warnings.
> >
> > Note that it is not yet a requirement to have 0 warnings for dtbs_check.
> > This will change in the future.
> >
> > Full log is available here: https://patchwork.ozlabs.org/patch/
> >
> >
> > pinctrl@3400000: 'ap-edp-bklten', 'ap-suspend-l-hog', 'ap_suspend_l_assert', 'ap_suspend_l_deassert', 'bios-flash-wp-r-l', 'cci0-default', 'cci0-sleep', 'cci1-default', 'cci1-sleep', 'ec-ap-int-l', 'edp-brij-en', 'edp-brij-irq', 'en-pp3300-dx-edp', 'h1-ap-int-odl', 'pen-eject-odl', 'pen-irq-l', 'pen-pdct-l', 'pen-rst-l', 'qspi-clk', 'qspi-cs0', 'qspi-cs1', 'qspi-data01', 'qspi-data12', 'quat_mi2s_active', 'quat_mi2s_sd0_active', 'quat_mi2s_sd0_sleep', 'quat_mi2s_sd1_active', 'quat_mi2s_sd1_sleep', 'quat_mi2s_sd2_active', 'quat_mi2s_sd2_sleep', 'quat_mi2s_sd3_active', 'quat_mi2s_sd3_sleep', 'quat_mi2s_sleep', 'qup-i2c0-default', 'qup-i2c1-default', 'qup-i2c10-default', 'qup-i2c11-default', 'qup-i2c12-default', 'qup-i2c13-default', 'qup-i2c14-default', 'qup-i2c15-default', 'qup-i2c2-default', 'qup-i2c3-default', 'qup-i2c4-default', 'qup-i2c5-default', 'qup-i2c6-default', 'qup-i2c7-default', 'qup-i2c8-default', 'qup-i2c9-default', 'qup-spi0-default', 'qup-spi1-default', 'qup-spi10-default', 'qup-spi11-default', 'qup-spi12-default', 'qup-spi13-default', 'qup-spi14-default', 'qup-spi15-default', 'qup-spi2-default', 'qup-spi3-default', 'qup-spi4-default', 'qup-spi5-default', 'qup-spi6-default', 'qup-spi7-default', 'qup-spi8-default', 'qup-spi9-default', 'qup-uart0-default', 'qup-uart1-default', 'qup-uart10-default', 'qup-uart11-default', 'qup-uart12-default', 'qup-uart13-default', 'qup-uart14-default', 'qup-uart15-default', 'qup-uart2-default', 'qup-uart3-default', 'qup-uart4-default', 'qup-uart5-default', 'qup-uart6-default', 'qup-uart7-default', 'qup-uart8-default', 'qup-uart9-default', 'sd-cd-odl', 'sdc2-clk', 'sdc2-cmd', 'sdc2-data', 'ts-int-l', 'ts-reset-l' do not match any of the regexes: '-state$', 'pinctrl-[0-9]+'
> > arch/arm64/boot/dts/qcom/sdm845-cheza-r1.dtb
> > arch/arm64/boot/dts/qcom/sdm845-cheza-r2.dtb
> > arch/arm64/boot/dts/qcom/sdm845-cheza-r3.dtb
>
> This is fixed in patch #1 in this set, however for some reason DT
> patchwork did not get it (maybe size too big?).
PW is filtered to only bindings, binding headers, and drivers/of/.
Rob
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] dt-bindings: pinctrl: qcom,sdm845: convert to dtschema
2022-09-30 20:05 ` [PATCH 2/2] dt-bindings: pinctrl: qcom,sdm845: convert to dtschema Krzysztof Kozlowski
2022-09-30 22:05 ` Rob Herring
@ 2022-10-03 17:11 ` Rob Herring
2022-10-17 22:42 ` (subset) " Krzysztof Kozlowski
2 siblings, 0 replies; 17+ messages in thread
From: Rob Herring @ 2022-10-03 17:11 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: linux-gpio, Rob Herring, Andy Gross, Bjorn Andersson,
Krzysztof Kozlowski, Linus Walleij, linux-kernel, devicetree,
linux-arm-msm, Konrad Dybcio
On Fri, 30 Sep 2022 22:05:29 +0200, Krzysztof Kozlowski wrote:
> Convert Qualcomm SDM845 pin controller bindings to DT schema. Keep
> the parsing of pin configuration subnodes consistent with other Qualcomm
> schemas (children named with '-state' suffix, their children with
> '-pins').
>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
> .../bindings/pinctrl/qcom,sdm845-pinctrl.txt | 176 ------------------
> .../bindings/pinctrl/qcom,sdm845-pinctrl.yaml | 158 ++++++++++++++++
> 2 files changed, 158 insertions(+), 176 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,sdm845-pinctrl.txt
> create mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,sdm845-pinctrl.yaml
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] arm64: dts: qcom: sdm845: align TLMM pin configuration with DT schema
2022-10-03 16:14 ` Doug Anderson
@ 2022-10-03 17:45 ` Krzysztof Kozlowski
2022-10-03 22:59 ` Doug Anderson
0 siblings, 1 reply; 17+ messages in thread
From: Krzysztof Kozlowski @ 2022-10-03 17:45 UTC (permalink / raw)
To: Doug Anderson
Cc: Andy Gross, Bjorn Andersson, Konrad Dybcio, Linus Walleij,
Rob Herring, Krzysztof Kozlowski, linux-arm-msm,
open list:GPIO SUBSYSTEM,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML
On 03/10/2022 18:14, Doug Anderson wrote:
> Hi,
>
> On Fri, Sep 30, 2022 at 1:06 PM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> DT schema expects TLMM pin configuration nodes to be named with
>> '-state' suffix and their optional children with '-pins' suffix.
>>
>> The sdm854.dtsi file defined several pin configuration nodes which are
>> customized by the boards. Therefore keep a additional "default-pins"
>> node inside so the boards can add more of configuration nodes. Such
>> additional configuration nodes always need 'function' property now
>> (required by DT schema).
>>
>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> ---
>> arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi | 344 +++++++-----------
>> arch/arm64/boot/dts/qcom/sdm845-db845c.dts | 76 ++--
>> .../arm64/boot/dts/qcom/sdm845-lg-common.dtsi | 60 ++-
>> arch/arm64/boot/dts/qcom/sdm845-lg-judyln.dts | 2 +-
>> arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 60 ++-
>> .../boot/dts/qcom/sdm845-oneplus-common.dtsi | 88 ++---
>> .../boot/dts/qcom/sdm845-shift-axolotl.dts | 138 +++----
>> .../dts/qcom/sdm845-sony-xperia-tama.dtsi | 6 +-
>> .../boot/dts/qcom/sdm845-xiaomi-beryllium.dts | 26 +-
>> .../boot/dts/qcom/sdm845-xiaomi-polaris.dts | 30 +-
>> arch/arm64/boot/dts/qcom/sdm845.dtsi | 305 +++++++---------
>> .../boot/dts/qcom/sdm850-lenovo-yoga-c630.dts | 33 +-
>> .../boot/dts/qcom/sdm850-samsung-w737.dts | 96 ++---
>> 13 files changed, 513 insertions(+), 751 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi b/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi
>> index b5f11fbcc300..3403cdcdd49c 100644
>> --- a/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi
>> @@ -993,21 +993,21 @@ &wifi {
>> /* PINCTRL - additions to nodes defined in sdm845.dtsi */
>>
>> &qspi_cs0 {
>> - pinconf {
>> + default-pins {
>> pins = "gpio90";
>> bias-disable;
>> };
>> };
>>
>> &qspi_clk {
>> - pinconf {
>> + default-pins {
>> pins = "gpio95";
>> bias-disable;
>> };
>> };
>>
>> &qspi_data01 {
>> - pinconf {
>> + default-pins {
>> pins = "gpio91", "gpio92";
>
> I haven't been fully involved in all the discussion here, but the
> above doesn't look like it matches the way that Bjorn wanted to go
> [1]. I would sorta expect it to look like this:
>
> /* QSPI always needs a clock and IO pins */
> qspi_basic: {
> qspi_clk: {
> pins = "gpio95";
> function = "qspi_clk";
> };
> qspi_data01: {
> pins = "gpio95";
> function = "qspi_clk";
> };
> }
>
> /* QSPI will need one or both chip selects */
> qspi_cs0: qspi-cs0-state {
> pins = "gpio90";
> function = "qspi_cs";
> };
>
> qspi_cs1: qspi-cs1-state {
> pins = "gpio89";
> function = "qspi_cs";
> };
>
> /* If using all 4 data lines we need these */
> qspi_data12: qspi-data12-state {
> pins = "gpio93", "gpio94";
> function = "qspi_data";
> };
>
> Basically grouping things together in a two-level node when it makes
> sense and then using 1-level nodes for "mixin" pins. Then you'd end up
> doing one of these things:
>
> pinctrl-0 = <&qspi_basic &qspi_cs0>;
> pinctrl-0 = <&qspi_basic &qspi_cs1>;
> pinctrl-0 = <&qspi_basic &qspi_cs0 &qspi_data12>;
I don't get how my patch changes the existing approach? Such pattern was
already there.
Again - you end up or you ended up? We discuss here what this patch did.
So are you sure that this patch did something like that (and it wasn't
already there)?
>
> Note that the extra tags of "qspi_clk" and "qspi_data01" are important
> since it lets the individual boards easily set pulls / drive strengths
> without needing to replicate the hierarchy of the SoC. So if a board
> wanted to set the pull of the cs0 line, just:
>
> &qspi_cs0 {
> bias-disable;
> };
>
> [1] https://lore.kernel.org/lkml/CAD=FV=VUL4GmjaibAMhKNdpEso_Hg_R=XeMaqah1LSj_9-Ce4Q@mail.gmail.com/
>
>
>> @@ -1016,7 +1016,7 @@ pinconf {
>> };
>>
>> &qup_i2c3_default {
>> - pinconf {
>> + default-pins {
>> pins = "gpio41", "gpio42";
>> drive-strength = <2>;
>
> I don't see any benefit to two-levels above. Why not just get rid of
> the "default-pins" and put the stuff directly under qup_i2c3_default?
For the same reason I told Konrad?
>
>
>> /* PINCTRL - additions to nodes defined in sdm845.dtsi */
>> &qup_spi2_default {
>> - pinmux {
>> + default-pins {
>> drive-strength = <16>;
>> };
>> };
>>
>> &qup_uart3_default{
>> - pinmux {
>> + default-pins {
>> pins = "gpio41", "gpio42", "gpio43", "gpio44";
>> function = "qup3";
>> };
>> };
>>
>> &qup_i2c10_default {
>> - pinconf {
>> + default-pins {
>> pins = "gpio55", "gpio56";
>> drive-strength = <2>;
>> bias-disable;
>> @@ -1144,37 +1144,37 @@ pinconf {
>> };
>>
>> &qup_uart6_default {
>> - pinmux {
>> - pins = "gpio45", "gpio46", "gpio47", "gpio48";
>> - function = "qup6";
>> - };
>> -
>> - cts {
>> + cts-pins {
>> pins = "gpio45";
>> + function = "qup6";
>> bias-disable;
>> };
>>
>> - rts-tx {
>> + rts-tx-pins {
>> pins = "gpio46", "gpio47";
>> + function = "qup6";
>> drive-strength = <2>;
>> bias-disable;
>> };
>>
>> - rx {
>> + rx-pins {
>> pins = "gpio48";
>> + function = "qup6";
>> bias-pull-up;
>> };
>> };
>
> I didn't check everything about this patch, but skimming through I
> believe that the uart6 handling here is wrong. You'll end up with:>
> qup_uart6_default: qup-uart6-default-state {
> default-pins {
> pins = "gpio47", "gpio48";
> function = "qup6";
This piece was removed.
> };
>
> cts-pins {
> pins = "gpio45";
> function = "qup6";
> bias-disable;
> };
>
> rts-tx-pins {
> pins = "gpio46", "gpio47";
> function = "qup6";
> drive-strength = <2>;
> bias-disable;
> };
>
> rx-pins {
> pins = "gpio48";
> function = "qup6";
> bias-pull-up;
> };
> };
>
> So pins 47 and 48 are each referenced in two nodes. That doesn't seem
> correct to me. IMO, better would have been:
Even though that particular piece was removed, so there is no double
reference, it would still be correct. Or rather - what is there
incorrect? Mentioning pin twice? This is ok, although not necessarily
the most readable.
> In Soc.dtsi:
>
> qup_uart6_txrx: qup-uart6-txrx-state {
> qup_uart6_tx {
> pins = "gpio47";
> function = "qup6";
> };
> qup_uart6_rx {
> pins = "gpio48";
> function = "qup6";
> };
> };
> qup_uart6_cts: qup-uart6-cts-state {
> pins = "gpio45";
> function = "qup6";
> };
> qup_uart6_rts: qup-uart6-rts-state {
> pins = "gpio46";
> function = "qup6";
> };
>
> In board.dts:
>
> &qup_uart6_cts {
> bias-disable;
> };
> &qup_uart6_rts {
> drive-strength = <2>;
> bias-disable;
> };
> &qup_uart6_rx {
> bias-pull-up;
> };
> &qup_uart6_tx {
> drive-strength = <2>;
> bias-disable;
It's not related to this patchset, but sounds good, please change the
DTS to match it. I can rebase my patch on top of it.
> };
>
> Also, as per latest agreement with Bjorn, we should be moving the
> default drive strength to the SoC.dtsi file, so going further:
How is it related to this patch? Sure, feel free to move drive strength
anywhere. We can discuss it. But it is not part of this patch.
>
> In Soc.dtsi:
>
> qup_uart6_txrx: qup-uart6-txrx-state {
> qup_uart6_tx {
> pins = "gpio47";
> function = "qup6";
> drive-strength = <2>;
> };
> qup_uart6_rx {
> pins = "gpio48";
> function = "qup6";
> };
> };
> qup_uart6_cts: qup-uart6-cts-state {
> pins = "gpio45";
> function = "qup6";
> };
> qup_uart6_rts: qup-uart6-rts-state {
> pins = "gpio46";
> function = "qup6";
> drive-strength = <2>;
These are not part of DTSI. They exist in DTS, not in DTSI. You now
introduce a change entirely different than this patchset is doing. It
makes sense on its own, but it is not related to this patchset.
> };
>
> In board.dts:
>
> &qup_uart6_cts {
> bias-disable;
> };
> &qup_uart6_rts {
> bias-disable;
> };
> &qup_uart6_rx {
> bias-pull-up;
> };
> &qup_uart6_tx {
> bias-disable;
> };
>
> In the SoC.dtsi file we could default to just a tx/rx config:
>
> pinctrl-0 = <&qup_uart6_txrx>;
>
> ...if a board had the flow control lines hooked up, it could do:
>
> pinctrl-0 = <&qup_uart6_txrx &qup_uart6_cts &qup_uart6_rts>;
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] arm64: dts: qcom: sdm845: align TLMM pin configuration with DT schema
2022-10-03 17:45 ` Krzysztof Kozlowski
@ 2022-10-03 22:59 ` Doug Anderson
2022-10-04 8:40 ` Krzysztof Kozlowski
0 siblings, 1 reply; 17+ messages in thread
From: Doug Anderson @ 2022-10-03 22:59 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Andy Gross, Bjorn Andersson, Konrad Dybcio, Linus Walleij,
Rob Herring, Krzysztof Kozlowski, linux-arm-msm,
open list:GPIO SUBSYSTEM,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML
Hi,
On Mon, Oct 3, 2022 at 10:45 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 03/10/2022 18:14, Doug Anderson wrote:
> > Hi,
> >
> > On Fri, Sep 30, 2022 at 1:06 PM Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> wrote:
> >>
> >> DT schema expects TLMM pin configuration nodes to be named with
> >> '-state' suffix and their optional children with '-pins' suffix.
> >>
> >> The sdm854.dtsi file defined several pin configuration nodes which are
> >> customized by the boards. Therefore keep a additional "default-pins"
> >> node inside so the boards can add more of configuration nodes. Such
> >> additional configuration nodes always need 'function' property now
> >> (required by DT schema).
> >>
> >> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> >> ---
> >> arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi | 344 +++++++-----------
> >> arch/arm64/boot/dts/qcom/sdm845-db845c.dts | 76 ++--
> >> .../arm64/boot/dts/qcom/sdm845-lg-common.dtsi | 60 ++-
> >> arch/arm64/boot/dts/qcom/sdm845-lg-judyln.dts | 2 +-
> >> arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 60 ++-
> >> .../boot/dts/qcom/sdm845-oneplus-common.dtsi | 88 ++---
> >> .../boot/dts/qcom/sdm845-shift-axolotl.dts | 138 +++----
> >> .../dts/qcom/sdm845-sony-xperia-tama.dtsi | 6 +-
> >> .../boot/dts/qcom/sdm845-xiaomi-beryllium.dts | 26 +-
> >> .../boot/dts/qcom/sdm845-xiaomi-polaris.dts | 30 +-
> >> arch/arm64/boot/dts/qcom/sdm845.dtsi | 305 +++++++---------
> >> .../boot/dts/qcom/sdm850-lenovo-yoga-c630.dts | 33 +-
> >> .../boot/dts/qcom/sdm850-samsung-w737.dts | 96 ++---
> >> 13 files changed, 513 insertions(+), 751 deletions(-)
> >>
> >> diff --git a/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi b/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi
> >> index b5f11fbcc300..3403cdcdd49c 100644
> >> --- a/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi
> >> +++ b/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi
> >> @@ -993,21 +993,21 @@ &wifi {
> >> /* PINCTRL - additions to nodes defined in sdm845.dtsi */
> >>
> >> &qspi_cs0 {
> >> - pinconf {
> >> + default-pins {
> >> pins = "gpio90";
> >> bias-disable;
> >> };
> >> };
> >>
> >> &qspi_clk {
> >> - pinconf {
> >> + default-pins {
> >> pins = "gpio95";
> >> bias-disable;
> >> };
> >> };
> >>
> >> &qspi_data01 {
> >> - pinconf {
> >> + default-pins {
> >> pins = "gpio91", "gpio92";
> >
> > I haven't been fully involved in all the discussion here, but the
> > above doesn't look like it matches the way that Bjorn wanted to go
> > [1]. I would sorta expect it to look like this:
> >
> > /* QSPI always needs a clock and IO pins */
> > qspi_basic: {
> > qspi_clk: {
> > pins = "gpio95";
> > function = "qspi_clk";
> > };
> > qspi_data01: {
> > pins = "gpio95";
> > function = "qspi_clk";
> > };
> > }
> >
> > /* QSPI will need one or both chip selects */
> > qspi_cs0: qspi-cs0-state {
> > pins = "gpio90";
> > function = "qspi_cs";
> > };
> >
> > qspi_cs1: qspi-cs1-state {
> > pins = "gpio89";
> > function = "qspi_cs";
> > };
> >
> > /* If using all 4 data lines we need these */
> > qspi_data12: qspi-data12-state {
> > pins = "gpio93", "gpio94";
> > function = "qspi_data";
> > };
> >
> > Basically grouping things together in a two-level node when it makes
> > sense and then using 1-level nodes for "mixin" pins. Then you'd end up
> > doing one of these things:
> >
> > pinctrl-0 = <&qspi_basic &qspi_cs0>;
> > pinctrl-0 = <&qspi_basic &qspi_cs1>;
> > pinctrl-0 = <&qspi_basic &qspi_cs0 &qspi_data12>;
>
>
> I don't get how my patch changes the existing approach? Such pattern was
> already there.
Before your patch things were split in two nodes, the muxing and the
configuration. AKA when you combined the soc.dtsi and the board.dts
you'd have:
qspi_cs0: qspi-cs0-state {
pinmux {
pins = "...";
... muxing properties ...
};
pinconf {
pins = "...";
... config properties ...
};
};
Your patch combines the "pinmux" and "pinconf" nodes into one. So when
you combine the soc.dtsi and the board.dts after your patch you now
have:
qspi_cs0: qspi-cs0-state {
default-pins {
pins = "...";
... muxing properties ...
... config properties ...
};
};
That's fine and is functionally correct. ...but IMO it sets a bad
example for people to follow (though, of course, it's really up to
Bjorn). The "default-pins" subnode serves no purpose. If you're
touching all this stuff anyway you might as well not end up with
something that's a bad example for people to follow.
> Again - you end up or you ended up? We discuss here what this patch did.
> So are you sure that this patch did something like that (and it wasn't
> already there)?
>
> >
> > Note that the extra tags of "qspi_clk" and "qspi_data01" are important
> > since it lets the individual boards easily set pulls / drive strengths
> > without needing to replicate the hierarchy of the SoC. So if a board
> > wanted to set the pull of the cs0 line, just:
> >
> > &qspi_cs0 {
> > bias-disable;
> > };
> >
> > [1] https://lore.kernel.org/lkml/CAD=FV=VUL4GmjaibAMhKNdpEso_Hg_R=XeMaqah1LSj_9-Ce4Q@mail.gmail.com/
> >
> >
> >> @@ -1016,7 +1016,7 @@ pinconf {
> >> };
> >>
> >> &qup_i2c3_default {
> >> - pinconf {
> >> + default-pins {
> >> pins = "gpio41", "gpio42";
> >> drive-strength = <2>;
> >
> > I don't see any benefit to two-levels above. Why not just get rid of
> > the "default-pins" and put the stuff directly under qup_i2c3_default?
>
> For the same reason I told Konrad?
OK. I looked at what you end up with for "qup_uart9" after your
patches and it's definitely not my favorite place to end up at. If
nothing else you are double-specifying "function" in both
"default-pins" and "tx-pins"/"rx-pins". If those disagree then what
happens?
In general also we end up specifying that extra level of
"default-pins" in many cases for no purpose. We also end up
replicating hierarchy in the board dts files (the dts files are
replicating the "default-pins" nodes from the parent).
> >> /* PINCTRL - additions to nodes defined in sdm845.dtsi */
> >> &qup_spi2_default {
> >> - pinmux {
> >> + default-pins {
> >> drive-strength = <16>;
> >> };
> >> };
> >>
> >> &qup_uart3_default{
> >> - pinmux {
> >> + default-pins {
> >> pins = "gpio41", "gpio42", "gpio43", "gpio44";
> >> function = "qup3";
> >> };
> >> };
> >>
> >> &qup_i2c10_default {
> >> - pinconf {
> >> + default-pins {
> >> pins = "gpio55", "gpio56";
> >> drive-strength = <2>;
> >> bias-disable;
> >> @@ -1144,37 +1144,37 @@ pinconf {
> >> };
> >>
> >> &qup_uart6_default {
> >> - pinmux {
> >> - pins = "gpio45", "gpio46", "gpio47", "gpio48";
> >> - function = "qup6";
> >> - };
> >> -
> >> - cts {
> >> + cts-pins {
> >> pins = "gpio45";
> >> + function = "qup6";
> >> bias-disable;
> >> };
> >>
> >> - rts-tx {
> >> + rts-tx-pins {
> >> pins = "gpio46", "gpio47";
> >> + function = "qup6";
> >> drive-strength = <2>;
> >> bias-disable;
> >> };
> >>
> >> - rx {
> >> + rx-pins {
> >> pins = "gpio48";
> >> + function = "qup6";
> >> bias-pull-up;
> >> };
> >> };
> >
> > I didn't check everything about this patch, but skimming through I
> > believe that the uart6 handling here is wrong. You'll end up with:>
> > qup_uart6_default: qup-uart6-default-state {
> > default-pins {
> > pins = "gpio47", "gpio48";
> > function = "qup6";
>
> This piece was removed.
It was? How/where? I tried applying your patch and I still see "qup6"
under the default-pins node in sdm845.dtsi.
> > };
> >
> > cts-pins {
> > pins = "gpio45";
> > function = "qup6";
> > bias-disable;
> > };
> >
> > rts-tx-pins {
> > pins = "gpio46", "gpio47";
> > function = "qup6";
> > drive-strength = <2>;
> > bias-disable;
> > };
> >
> > rx-pins {
> > pins = "gpio48";
> > function = "qup6";
> > bias-pull-up;
> > };
> > };
> >
> > So pins 47 and 48 are each referenced in two nodes. That doesn't seem
> > correct to me. IMO, better would have been:
>
> Even though that particular piece was removed, so there is no double
> reference, it would still be correct. Or rather - what is there
> incorrect? Mentioning pin twice? This is ok, although not necessarily
> the most readable.
I guess this gets into the corners of pinctrl that I haven't poked at
lots. I guess it should be OK unless the SoC.dtsi and the board.dts
disagree about the "function". In such a case I guess it would be a
problem. So I guess what you end up will be OK but I don't like that
"function" is specified for the same pin in two different sub-nodes.
> > In Soc.dtsi:
> >
> > qup_uart6_txrx: qup-uart6-txrx-state {
> > qup_uart6_tx {
> > pins = "gpio47";
> > function = "qup6";
> > };
> > qup_uart6_rx {
> > pins = "gpio48";
> > function = "qup6";
> > };
> > };
> > qup_uart6_cts: qup-uart6-cts-state {
> > pins = "gpio45";
> > function = "qup6";
> > };
> > qup_uart6_rts: qup-uart6-rts-state {
> > pins = "gpio46";
> > function = "qup6";
> > };
> >
> > In board.dts:
> >
> > &qup_uart6_cts {
> > bias-disable;
> > };
> > &qup_uart6_rts {
> > drive-strength = <2>;
> > bias-disable;
> > };
> > &qup_uart6_rx {
> > bias-pull-up;
> > };
> > &qup_uart6_tx {
> > drive-strength = <2>;
> > bias-disable;
>
> It's not related to this patchset, but sounds good, please change the
> DTS to match it. I can rebase my patch on top of it.
I guess it's related in that the patchset is touching everything and
one would assume that something touched so recently would represent
the current best practices. Maybe that's a weak argument, but if I saw
a patch that was about trying to clean up all the pinctrl across all
the older SoCs that I would assume that the pinctrl would be clean
after that patch and would be a good example to follow as best
practice. Thus it's relevant to talk about whether this patch is
ending us up at best practice or not.
> > };
> >
> > Also, as per latest agreement with Bjorn, we should be moving the
> > default drive strength to the SoC.dtsi file, so going further:
>
> How is it related to this patch? Sure, feel free to move drive strength
> anywhere. We can discuss it. But it is not part of this patch.
Moving the drive strength can certainly be discussed / done in a later patch.
> > In Soc.dtsi:
> >
> > qup_uart6_txrx: qup-uart6-txrx-state {
> > qup_uart6_tx {
> > pins = "gpio47";
> > function = "qup6";
> > drive-strength = <2>;
> > };
> > qup_uart6_rx {
> > pins = "gpio48";
> > function = "qup6";
> > };
> > };
> > qup_uart6_cts: qup-uart6-cts-state {
> > pins = "gpio45";
> > function = "qup6";
> > };
> > qup_uart6_rts: qup-uart6-rts-state {
> > pins = "gpio46";
> > function = "qup6";
> > drive-strength = <2>;
>
> These are not part of DTSI. They exist in DTS, not in DTSI. You now
> introduce a change entirely different than this patchset is doing. It
> makes sense on its own, but it is not related to this patchset.
It is relevant to discuss because it would be the correct way to solve
the same issue with "uart9" that you used to justify why you needed an
extra "uart9-default" subnode.
> > };
> >
> > In board.dts:
> >
> > &qup_uart6_cts {
> > bias-disable;
> > };
> > &qup_uart6_rts {
> > bias-disable;
> > };
> > &qup_uart6_rx {
> > bias-pull-up;
> > };
> > &qup_uart6_tx {
> > bias-disable;
> > };
> >
> > In the SoC.dtsi file we could default to just a tx/rx config:
> >
> > pinctrl-0 = <&qup_uart6_txrx>;
> >
> > ...if a board had the flow control lines hooked up, it could do:
> >
> > pinctrl-0 = <&qup_uart6_txrx &qup_uart6_cts &qup_uart6_rts>;
>
>
> Best regards,
> Krzysztof
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] arm64: dts: qcom: sdm845: align TLMM pin configuration with DT schema
2022-10-03 22:59 ` Doug Anderson
@ 2022-10-04 8:40 ` Krzysztof Kozlowski
2022-10-04 14:55 ` Doug Anderson
0 siblings, 1 reply; 17+ messages in thread
From: Krzysztof Kozlowski @ 2022-10-04 8:40 UTC (permalink / raw)
To: Doug Anderson
Cc: Andy Gross, Bjorn Andersson, Konrad Dybcio, Linus Walleij,
Rob Herring, Krzysztof Kozlowski, linux-arm-msm,
open list:GPIO SUBSYSTEM,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML
On 04/10/2022 00:59, Doug Anderson wrote:
> Hi,
>
> On Mon, Oct 3, 2022 at 10:45 AM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 03/10/2022 18:14, Doug Anderson wrote:
>>> Hi,
>>>
>>> On Fri, Sep 30, 2022 at 1:06 PM Krzysztof Kozlowski
>>> <krzysztof.kozlowski@linaro.org> wrote:
>>>>
>>>> DT schema expects TLMM pin configuration nodes to be named with
>>>> '-state' suffix and their optional children with '-pins' suffix.
>>>>
>>>> The sdm854.dtsi file defined several pin configuration nodes which are
>>>> customized by the boards. Therefore keep a additional "default-pins"
>>>> node inside so the boards can add more of configuration nodes. Such
>>>> additional configuration nodes always need 'function' property now
>>>> (required by DT schema).
>>>>
>>>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>> ---
>>>> arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi | 344 +++++++-----------
>>>> arch/arm64/boot/dts/qcom/sdm845-db845c.dts | 76 ++--
>>>> .../arm64/boot/dts/qcom/sdm845-lg-common.dtsi | 60 ++-
>>>> arch/arm64/boot/dts/qcom/sdm845-lg-judyln.dts | 2 +-
>>>> arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 60 ++-
>>>> .../boot/dts/qcom/sdm845-oneplus-common.dtsi | 88 ++---
>>>> .../boot/dts/qcom/sdm845-shift-axolotl.dts | 138 +++----
>>>> .../dts/qcom/sdm845-sony-xperia-tama.dtsi | 6 +-
>>>> .../boot/dts/qcom/sdm845-xiaomi-beryllium.dts | 26 +-
>>>> .../boot/dts/qcom/sdm845-xiaomi-polaris.dts | 30 +-
>>>> arch/arm64/boot/dts/qcom/sdm845.dtsi | 305 +++++++---------
>>>> .../boot/dts/qcom/sdm850-lenovo-yoga-c630.dts | 33 +-
>>>> .../boot/dts/qcom/sdm850-samsung-w737.dts | 96 ++---
>>>> 13 files changed, 513 insertions(+), 751 deletions(-)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi b/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi
>>>> index b5f11fbcc300..3403cdcdd49c 100644
>>>> --- a/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi
>>>> +++ b/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi
>>>> @@ -993,21 +993,21 @@ &wifi {
>>>> /* PINCTRL - additions to nodes defined in sdm845.dtsi */
>>>>
>>>> &qspi_cs0 {
>>>> - pinconf {
>>>> + default-pins {
>>>> pins = "gpio90";
>>>> bias-disable;
>>>> };
>>>> };
>>>>
>>>> &qspi_clk {
>>>> - pinconf {
>>>> + default-pins {
>>>> pins = "gpio95";
>>>> bias-disable;
>>>> };
>>>> };
>>>>
>>>> &qspi_data01 {
>>>> - pinconf {
>>>> + default-pins {
>>>> pins = "gpio91", "gpio92";
>>>
>>> I haven't been fully involved in all the discussion here, but the
>>> above doesn't look like it matches the way that Bjorn wanted to go
>>> [1]. I would sorta expect it to look like this:
>>>
>>> /* QSPI always needs a clock and IO pins */
>>> qspi_basic: {
>>> qspi_clk: {
>>> pins = "gpio95";
>>> function = "qspi_clk";
>>> };
>>> qspi_data01: {
>>> pins = "gpio95";
>>> function = "qspi_clk";
>>> };
>>> }
>>>
>>> /* QSPI will need one or both chip selects */
>>> qspi_cs0: qspi-cs0-state {
>>> pins = "gpio90";
>>> function = "qspi_cs";
>>> };
>>>
>>> qspi_cs1: qspi-cs1-state {
>>> pins = "gpio89";
>>> function = "qspi_cs";
>>> };
>>>
>>> /* If using all 4 data lines we need these */
>>> qspi_data12: qspi-data12-state {
>>> pins = "gpio93", "gpio94";
>>> function = "qspi_data";
>>> };
>>>
>>> Basically grouping things together in a two-level node when it makes
>>> sense and then using 1-level nodes for "mixin" pins. Then you'd end up
>>> doing one of these things:
>>>
>>> pinctrl-0 = <&qspi_basic &qspi_cs0>;
>>> pinctrl-0 = <&qspi_basic &qspi_cs1>;
>>> pinctrl-0 = <&qspi_basic &qspi_cs0 &qspi_data12>;
>>
>>
>> I don't get how my patch changes the existing approach? Such pattern was
>> already there.
>
> Before your patch things were split in two nodes, the muxing and the
> configuration. AKA when you combined the soc.dtsi and the board.dts
> you'd have:
>
> qspi_cs0: qspi-cs0-state {
> pinmux {
> pins = "...";
> ... muxing properties ...
> };
> pinconf {
> pins = "...";
> ... config properties ...
> };
> };
Which was also not good pattern. Muxing and configuring is basically the
same function of pin controller. Splitting them makes no sense at all.
> Your patch combines the "pinmux" and "pinconf" nodes into one. So when
> you combine the soc.dtsi and the board.dts after your patch you now
> have:
>
> qspi_cs0: qspi-cs0-state {
> default-pins {
> pins = "...";
> ... muxing properties ...
> ... config properties ...
> };
> };
>
>
> That's fine and is functionally correct. ...but IMO it sets a bad
> example for people to follow (though, of course, it's really up to
> Bjorn). The "default-pins" subnode serves no purpose. If you're
> touching all this stuff anyway you might as well not end up with
> something that's a bad example for people to follow.
I understand, you're right. I wanted to keep minimal amount of changes
to the DTS layout but since I am touching almost everything, I can
rework it.
>> Again - you end up or you ended up? We discuss here what this patch did.
>> So are you sure that this patch did something like that (and it wasn't
>> already there)?
>>
>>>
>>> Note that the extra tags of "qspi_clk" and "qspi_data01" are important
>>> since it lets the individual boards easily set pulls / drive strengths
>>> without needing to replicate the hierarchy of the SoC. So if a board
>>> wanted to set the pull of the cs0 line, just:
>>>
>>> &qspi_cs0 {
>>> bias-disable;
>>> };
>>>
>>> [1] https://lore.kernel.org/lkml/CAD=FV=VUL4GmjaibAMhKNdpEso_Hg_R=XeMaqah1LSj_9-Ce4Q@mail.gmail.com/
>>>
>>>
>>>> @@ -1016,7 +1016,7 @@ pinconf {
>>>> };
>>>>
>>>> &qup_i2c3_default {
>>>> - pinconf {
>>>> + default-pins {
>>>> pins = "gpio41", "gpio42";
>>>> drive-strength = <2>;
>>>
>>> I don't see any benefit to two-levels above. Why not just get rid of
>>> the "default-pins" and put the stuff directly under qup_i2c3_default?
>>
>> For the same reason I told Konrad?
>
> OK. I looked at what you end up with for "qup_uart9" after your
> patches and it's definitely not my favorite place to end up at. If
> nothing else you are double-specifying "function" in both
> "default-pins" and "tx-pins"/"rx-pins". If those disagree then what
> happens?
The same happens and happened before. My patch does nothing related to
allowing or disallowing wrong muxing/config nodes. In the past you could
have conflicting setups. Now it's exactly the same.
The double-specifying of "function" is not a result of '-state'/'-pins'
reorganization but aligning with common TLMM schema *which requires
function* for every node.
>
> In general also we end up specifying that extra level of
> "default-pins" in many cases for no purpose. We also end up
> replicating hierarchy in the board dts files (the dts files are
> replicating the "default-pins" nodes from the parent).
OK, let's fix this. That will need either:
1. /delete-node with copying of most of properties from default-pins
2. or move the CTS/TX/RX config pins to the DTSI.
>>>> /* PINCTRL - additions to nodes defined in sdm845.dtsi */
>>>> &qup_spi2_default {
>>>> - pinmux {
>>>> + default-pins {
>>>> drive-strength = <16>;
>>>> };
>>>> };
>>>>
>>>> &qup_uart3_default{
>>>> - pinmux {
>>>> + default-pins {
>>>> pins = "gpio41", "gpio42", "gpio43", "gpio44";
>>>> function = "qup3";
>>>> };
>>>> };
>>>>
>>>> &qup_i2c10_default {
>>>> - pinconf {
>>>> + default-pins {
>>>> pins = "gpio55", "gpio56";
>>>> drive-strength = <2>;
>>>> bias-disable;
>>>> @@ -1144,37 +1144,37 @@ pinconf {
>>>> };
>>>>
>>>> &qup_uart6_default {
>>>> - pinmux {
>>>> - pins = "gpio45", "gpio46", "gpio47", "gpio48";
>>>> - function = "qup6";
>>>> - };
>>>> -
>>>> - cts {
>>>> + cts-pins {
>>>> pins = "gpio45";
>>>> + function = "qup6";
>>>> bias-disable;
>>>> };
>>>>
>>>> - rts-tx {
>>>> + rts-tx-pins {
>>>> pins = "gpio46", "gpio47";
>>>> + function = "qup6";
>>>> drive-strength = <2>;
>>>> bias-disable;
>>>> };
>>>>
>>>> - rx {
>>>> + rx-pins {
>>>> pins = "gpio48";
>>>> + function = "qup6";
>>>> bias-pull-up;
>>>> };
>>>> };
>>>
>>> I didn't check everything about this patch, but skimming through I
>>> believe that the uart6 handling here is wrong. You'll end up with:>
>>> qup_uart6_default: qup-uart6-default-state {
>>> default-pins {
>>> pins = "gpio47", "gpio48";
>>> function = "qup6";
>>
>> This piece was removed.
>
> It was? How/where? I tried applying your patch and I still see "qup6"
> under the default-pins node in sdm845.dtsi.
>
Ah, you're right. The default-pins are coming from DTSI.
So delete-node?
>
>>> };
>>>
>>> cts-pins {
>>> pins = "gpio45";
>>> function = "qup6";
>>> bias-disable;
>>> };
>>>
>>> rts-tx-pins {
>>> pins = "gpio46", "gpio47";
>>> function = "qup6";
>>> drive-strength = <2>;
>>> bias-disable;
>>> };
>>>
>>> rx-pins {
>>> pins = "gpio48";
>>> function = "qup6";
>>> bias-pull-up;
>>> };
>>> };
>>>
>>> So pins 47 and 48 are each referenced in two nodes. That doesn't seem
>>> correct to me. IMO, better would have been:
>>
>> Even though that particular piece was removed, so there is no double
>> reference, it would still be correct. Or rather - what is there
>> incorrect? Mentioning pin twice? This is ok, although not necessarily
>> the most readable.
>
> I guess this gets into the corners of pinctrl that I haven't poked at
> lots. I guess it should be OK unless the SoC.dtsi and the board.dts
> disagree about the "function". In such a case I guess it would be a
> problem. So I guess what you end up will be OK but I don't like that
> "function" is specified for the same pin in two different sub-nodes.
OK.
>
>
>>> In Soc.dtsi:
>>>
>>> qup_uart6_txrx: qup-uart6-txrx-state {
>>> qup_uart6_tx {
>>> pins = "gpio47";
>>> function = "qup6";
>>> };
>>> qup_uart6_rx {
>>> pins = "gpio48";
>>> function = "qup6";
>>> };
>>> };
>>> qup_uart6_cts: qup-uart6-cts-state {
>>> pins = "gpio45";
>>> function = "qup6";
>>> };
>>> qup_uart6_rts: qup-uart6-rts-state {
>>> pins = "gpio46";
>>> function = "qup6";
>>> };
>>>
>>> In board.dts:
>>>
>>> &qup_uart6_cts {
>>> bias-disable;
>>> };
>>> &qup_uart6_rts {
>>> drive-strength = <2>;
>>> bias-disable;
>>> };
>>> &qup_uart6_rx {
>>> bias-pull-up;
>>> };
>>> &qup_uart6_tx {
>>> drive-strength = <2>;
>>> bias-disable;
>>
>> It's not related to this patchset, but sounds good, please change the
>> DTS to match it. I can rebase my patch on top of it.
>
> I guess it's related in that the patchset is touching everything and
> one would assume that something touched so recently would represent
> the current best practices. Maybe that's a weak argument, but if I saw
> a patch that was about trying to clean up all the pinctrl across all
> the older SoCs that I would assume that the pinctrl would be clean
> after that patch and would be a good example to follow as best
> practice. Thus it's relevant to talk about whether this patch is
> ending us up at best practice or not.
>
>
>>> };
>>>
>>> Also, as per latest agreement with Bjorn, we should be moving the
>>> default drive strength to the SoC.dtsi file, so going further:
>>
>> How is it related to this patch? Sure, feel free to move drive strength
>> anywhere. We can discuss it. But it is not part of this patch.
>
> Moving the drive strength can certainly be discussed / done in a later patch.
>
>
>>> In Soc.dtsi:
>>>
>>> qup_uart6_txrx: qup-uart6-txrx-state {
>>> qup_uart6_tx {
>>> pins = "gpio47";
>>> function = "qup6";
>>> drive-strength = <2>;
>>> };
>>> qup_uart6_rx {
>>> pins = "gpio48";
>>> function = "qup6";
>>> };
>>> };
>>> qup_uart6_cts: qup-uart6-cts-state {
>>> pins = "gpio45";
>>> function = "qup6";
>>> };
>>> qup_uart6_rts: qup-uart6-rts-state {
>>> pins = "gpio46";
>>> function = "qup6";
>>> drive-strength = <2>;
>>
>> These are not part of DTSI. They exist in DTS, not in DTSI. You now
>> introduce a change entirely different than this patchset is doing. It
>> makes sense on its own, but it is not related to this patchset.
>
> It is relevant to discuss because it would be the correct way to solve
> the same issue with "uart9" that you used to justify why you needed an
> extra "uart9-default" subnode.
The goal of this patchset is to solve dtbs_check. It's goal is not
"generic guidance how Qualcomm pin configuration nodes should be
written", because whatever you and I agree, it does not much matter. The
maintainers decision matter.
If there is a consensus/preferred way to organize it, sure, document it
somewhere, store it in email/lore in concise way, and I can fix up the
patch to match it. It is just not my goal now, so I am not pushing
towards any such direction.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] arm64: dts: qcom: sdm845: align TLMM pin configuration with DT schema
2022-10-04 8:40 ` Krzysztof Kozlowski
@ 2022-10-04 14:55 ` Doug Anderson
2022-10-05 8:18 ` Krzysztof Kozlowski
0 siblings, 1 reply; 17+ messages in thread
From: Doug Anderson @ 2022-10-04 14:55 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Andy Gross, Bjorn Andersson, Konrad Dybcio, Linus Walleij,
Rob Herring, Krzysztof Kozlowski, linux-arm-msm,
open list:GPIO SUBSYSTEM,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML
Hi,
On Tue, Oct 4, 2022 at 1:40 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 04/10/2022 00:59, Doug Anderson wrote:
> > Hi,
> >
> > On Mon, Oct 3, 2022 at 10:45 AM Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> wrote:
> >>
> >> On 03/10/2022 18:14, Doug Anderson wrote:
> >>> Hi,
> >>>
> >>> On Fri, Sep 30, 2022 at 1:06 PM Krzysztof Kozlowski
> >>> <krzysztof.kozlowski@linaro.org> wrote:
> >>>>
> >>>> DT schema expects TLMM pin configuration nodes to be named with
> >>>> '-state' suffix and their optional children with '-pins' suffix.
> >>>>
> >>>> The sdm854.dtsi file defined several pin configuration nodes which are
> >>>> customized by the boards. Therefore keep a additional "default-pins"
> >>>> node inside so the boards can add more of configuration nodes. Such
> >>>> additional configuration nodes always need 'function' property now
> >>>> (required by DT schema).
> >>>>
> >>>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> >>>> ---
> >>>> arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi | 344 +++++++-----------
> >>>> arch/arm64/boot/dts/qcom/sdm845-db845c.dts | 76 ++--
> >>>> .../arm64/boot/dts/qcom/sdm845-lg-common.dtsi | 60 ++-
> >>>> arch/arm64/boot/dts/qcom/sdm845-lg-judyln.dts | 2 +-
> >>>> arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 60 ++-
> >>>> .../boot/dts/qcom/sdm845-oneplus-common.dtsi | 88 ++---
> >>>> .../boot/dts/qcom/sdm845-shift-axolotl.dts | 138 +++----
> >>>> .../dts/qcom/sdm845-sony-xperia-tama.dtsi | 6 +-
> >>>> .../boot/dts/qcom/sdm845-xiaomi-beryllium.dts | 26 +-
> >>>> .../boot/dts/qcom/sdm845-xiaomi-polaris.dts | 30 +-
> >>>> arch/arm64/boot/dts/qcom/sdm845.dtsi | 305 +++++++---------
> >>>> .../boot/dts/qcom/sdm850-lenovo-yoga-c630.dts | 33 +-
> >>>> .../boot/dts/qcom/sdm850-samsung-w737.dts | 96 ++---
> >>>> 13 files changed, 513 insertions(+), 751 deletions(-)
> >>>>
> >>>> diff --git a/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi b/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi
> >>>> index b5f11fbcc300..3403cdcdd49c 100644
> >>>> --- a/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi
> >>>> +++ b/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi
> >>>> @@ -993,21 +993,21 @@ &wifi {
> >>>> /* PINCTRL - additions to nodes defined in sdm845.dtsi */
> >>>>
> >>>> &qspi_cs0 {
> >>>> - pinconf {
> >>>> + default-pins {
> >>>> pins = "gpio90";
> >>>> bias-disable;
> >>>> };
> >>>> };
> >>>>
> >>>> &qspi_clk {
> >>>> - pinconf {
> >>>> + default-pins {
> >>>> pins = "gpio95";
> >>>> bias-disable;
> >>>> };
> >>>> };
> >>>>
> >>>> &qspi_data01 {
> >>>> - pinconf {
> >>>> + default-pins {
> >>>> pins = "gpio91", "gpio92";
> >>>
> >>> I haven't been fully involved in all the discussion here, but the
> >>> above doesn't look like it matches the way that Bjorn wanted to go
> >>> [1]. I would sorta expect it to look like this:
> >>>
> >>> /* QSPI always needs a clock and IO pins */
> >>> qspi_basic: {
> >>> qspi_clk: {
> >>> pins = "gpio95";
> >>> function = "qspi_clk";
> >>> };
> >>> qspi_data01: {
> >>> pins = "gpio95";
> >>> function = "qspi_clk";
> >>> };
> >>> }
> >>>
> >>> /* QSPI will need one or both chip selects */
> >>> qspi_cs0: qspi-cs0-state {
> >>> pins = "gpio90";
> >>> function = "qspi_cs";
> >>> };
> >>>
> >>> qspi_cs1: qspi-cs1-state {
> >>> pins = "gpio89";
> >>> function = "qspi_cs";
> >>> };
> >>>
> >>> /* If using all 4 data lines we need these */
> >>> qspi_data12: qspi-data12-state {
> >>> pins = "gpio93", "gpio94";
> >>> function = "qspi_data";
> >>> };
> >>>
> >>> Basically grouping things together in a two-level node when it makes
> >>> sense and then using 1-level nodes for "mixin" pins. Then you'd end up
> >>> doing one of these things:
> >>>
> >>> pinctrl-0 = <&qspi_basic &qspi_cs0>;
> >>> pinctrl-0 = <&qspi_basic &qspi_cs1>;
> >>> pinctrl-0 = <&qspi_basic &qspi_cs0 &qspi_data12>;
> >>
> >>
> >> I don't get how my patch changes the existing approach? Such pattern was
> >> already there.
> >
> > Before your patch things were split in two nodes, the muxing and the
> > configuration. AKA when you combined the soc.dtsi and the board.dts
> > you'd have:
> >
> > qspi_cs0: qspi-cs0-state {
> > pinmux {
> > pins = "...";
> > ... muxing properties ...
> > };
> > pinconf {
> > pins = "...";
> > ... config properties ...
> > };
> > };
>
> Which was also not good pattern. Muxing and configuring is basically the
> same function of pin controller. Splitting them makes no sense at all.
Agreed, it was not a good pattern.
> > Your patch combines the "pinmux" and "pinconf" nodes into one. So when
> > you combine the soc.dtsi and the board.dts after your patch you now
> > have:
> >
> > qspi_cs0: qspi-cs0-state {
> > default-pins {
> > pins = "...";
> > ... muxing properties ...
> > ... config properties ...
> > };
> > };
> >
> >
> > That's fine and is functionally correct. ...but IMO it sets a bad
> > example for people to follow (though, of course, it's really up to
> > Bjorn). The "default-pins" subnode serves no purpose. If you're
> > touching all this stuff anyway you might as well not end up with
> > something that's a bad example for people to follow.
>
> I understand, you're right. I wanted to keep minimal amount of changes
> to the DTS layout but since I am touching almost everything, I can
> rework it.
>
>
> >> Again - you end up or you ended up? We discuss here what this patch did.
> >> So are you sure that this patch did something like that (and it wasn't
> >> already there)?
> >>
> >>>
> >>> Note that the extra tags of "qspi_clk" and "qspi_data01" are important
> >>> since it lets the individual boards easily set pulls / drive strengths
> >>> without needing to replicate the hierarchy of the SoC. So if a board
> >>> wanted to set the pull of the cs0 line, just:
> >>>
> >>> &qspi_cs0 {
> >>> bias-disable;
> >>> };
> >>>
> >>> [1] https://lore.kernel.org/lkml/CAD=FV=VUL4GmjaibAMhKNdpEso_Hg_R=XeMaqah1LSj_9-Ce4Q@mail.gmail.com/
> >>>
> >>>
> >>>> @@ -1016,7 +1016,7 @@ pinconf {
> >>>> };
> >>>>
> >>>> &qup_i2c3_default {
> >>>> - pinconf {
> >>>> + default-pins {
> >>>> pins = "gpio41", "gpio42";
> >>>> drive-strength = <2>;
> >>>
> >>> I don't see any benefit to two-levels above. Why not just get rid of
> >>> the "default-pins" and put the stuff directly under qup_i2c3_default?
> >>
> >> For the same reason I told Konrad?
> >
> > OK. I looked at what you end up with for "qup_uart9" after your
> > patches and it's definitely not my favorite place to end up at. If
> > nothing else you are double-specifying "function" in both
> > "default-pins" and "tx-pins"/"rx-pins". If those disagree then what
> > happens?
>
> The same happens and happened before. My patch does nothing related to
> allowing or disallowing wrong muxing/config nodes. In the past you could
> have conflicting setups. Now it's exactly the same.
Right that you could have had conflicting setups before, too. However,
in the past we always avoided specifying the function for a given pin
in more than one node.
The old way was super verbose and had a lot of repetition for sure,
but it sorta made sense if you thought of it in words. AKA the old way
said this after combining the soc.dtsi and the board.dts
* For pins 45, 46, 47, 48: set the function to "qup6"
* For pin 45: set a pulldown
* For pins 46 and 47: set the drive strength to 2 and disable pulls
* For pin 48: set a pullup
So it repeated the "for pins" part and that was most definitely non
ideal. However, with your new setup you are sometimes doing:
* For pins 47 and 48: set the function to "qup6"
* For pin 45: set the function to "qup6" and set a pulldown
* For pins 46 and 47: set the function to "qup6" and the drive
strength to 2 and disable pulls
* For pin 48: set the function to "qup6" and set a pullup
That repeats the 'set the function to "qup6"' more than I'd like.
> The double-specifying of "function" is not a result of '-state'/'-pins'
> reorganization but aligning with common TLMM schema *which requires
> function* for every node.
I guess now that we've talked through it, I'd say that if we have to
specify a function for every node then we should strive for each pin
to be in exactly one node in any given active configuration. That
makes the schema happy and also avoids double-specifying the function.
I think in all of the examples I've given that this is true.
> > In general also we end up specifying that extra level of
> > "default-pins" in many cases for no purpose. We also end up
> > replicating hierarchy in the board dts files (the dts files are
> > replicating the "default-pins" nodes from the parent).
>
> OK, let's fix this. That will need either:
> 1. /delete-node with copying of most of properties from default-pins
> 2. or move the CTS/TX/RX config pins to the DTSI.
I think in my proposal the CTS/TX/RX stuff moved to the dtsi which solved it.
> >>>> /* PINCTRL - additions to nodes defined in sdm845.dtsi */
> >>>> &qup_spi2_default {
> >>>> - pinmux {
> >>>> + default-pins {
> >>>> drive-strength = <16>;
> >>>> };
> >>>> };
> >>>>
> >>>> &qup_uart3_default{
> >>>> - pinmux {
> >>>> + default-pins {
> >>>> pins = "gpio41", "gpio42", "gpio43", "gpio44";
> >>>> function = "qup3";
> >>>> };
> >>>> };
> >>>>
> >>>> &qup_i2c10_default {
> >>>> - pinconf {
> >>>> + default-pins {
> >>>> pins = "gpio55", "gpio56";
> >>>> drive-strength = <2>;
> >>>> bias-disable;
> >>>> @@ -1144,37 +1144,37 @@ pinconf {
> >>>> };
> >>>>
> >>>> &qup_uart6_default {
> >>>> - pinmux {
> >>>> - pins = "gpio45", "gpio46", "gpio47", "gpio48";
> >>>> - function = "qup6";
> >>>> - };
> >>>> -
> >>>> - cts {
> >>>> + cts-pins {
> >>>> pins = "gpio45";
> >>>> + function = "qup6";
> >>>> bias-disable;
> >>>> };
> >>>>
> >>>> - rts-tx {
> >>>> + rts-tx-pins {
> >>>> pins = "gpio46", "gpio47";
> >>>> + function = "qup6";
> >>>> drive-strength = <2>;
> >>>> bias-disable;
> >>>> };
> >>>>
> >>>> - rx {
> >>>> + rx-pins {
> >>>> pins = "gpio48";
> >>>> + function = "qup6";
> >>>> bias-pull-up;
> >>>> };
> >>>> };
> >>>
> >>> I didn't check everything about this patch, but skimming through I
> >>> believe that the uart6 handling here is wrong. You'll end up with:>
> >>> qup_uart6_default: qup-uart6-default-state {
> >>> default-pins {
> >>> pins = "gpio47", "gpio48";
> >>> function = "qup6";
> >>
> >> This piece was removed.
> >
> > It was? How/where? I tried applying your patch and I still see "qup6"
> > under the default-pins node in sdm845.dtsi.
> >
>
> Ah, you're right. The default-pins are coming from DTSI.
>
> So delete-node?
I would prefer the solution I proposed. Re-pasting here:
In Soc.dtsi:
qup_uart6_txrx: qup-uart6-txrx-state {
qup_uart6_tx {
pins = "gpio47";
function = "qup6";
};
qup_uart6_rx {
pins = "gpio48";
function = "qup6";
};
};
qup_uart6_cts: qup-uart6-cts-state {
pins = "gpio45";
function = "qup6";
};
qup_uart6_rts: qup-uart6-rts-state {
pins = "gpio46";
function = "qup6";
};
In board.dts:
&qup_uart6_cts {
bias-disable;
};
&qup_uart6_rts {
drive-strength = <2>;
bias-disable;
};
&qup_uart6_rx {
bias-pull-up;
};
&qup_uart6_tx {
drive-strength = <2>;
bias-disable;
};
> >>> };
> >>>
> >>> cts-pins {
> >>> pins = "gpio45";
> >>> function = "qup6";
> >>> bias-disable;
> >>> };
> >>>
> >>> rts-tx-pins {
> >>> pins = "gpio46", "gpio47";
> >>> function = "qup6";
> >>> drive-strength = <2>;
> >>> bias-disable;
> >>> };
> >>>
> >>> rx-pins {
> >>> pins = "gpio48";
> >>> function = "qup6";
> >>> bias-pull-up;
> >>> };
> >>> };
> >>>
> >>> So pins 47 and 48 are each referenced in two nodes. That doesn't seem
> >>> correct to me. IMO, better would have been:
> >>
> >> Even though that particular piece was removed, so there is no double
> >> reference, it would still be correct. Or rather - what is there
> >> incorrect? Mentioning pin twice? This is ok, although not necessarily
> >> the most readable.
> >
> > I guess this gets into the corners of pinctrl that I haven't poked at
> > lots. I guess it should be OK unless the SoC.dtsi and the board.dts
> > disagree about the "function". In such a case I guess it would be a
> > problem. So I guess what you end up will be OK but I don't like that
> > "function" is specified for the same pin in two different sub-nodes.
>
> OK.
>
> >
> >
> >>> In Soc.dtsi:
> >>>
> >>> qup_uart6_txrx: qup-uart6-txrx-state {
> >>> qup_uart6_tx {
> >>> pins = "gpio47";
> >>> function = "qup6";
> >>> };
> >>> qup_uart6_rx {
> >>> pins = "gpio48";
> >>> function = "qup6";
> >>> };
> >>> };
> >>> qup_uart6_cts: qup-uart6-cts-state {
> >>> pins = "gpio45";
> >>> function = "qup6";
> >>> };
> >>> qup_uart6_rts: qup-uart6-rts-state {
> >>> pins = "gpio46";
> >>> function = "qup6";
> >>> };
> >>>
> >>> In board.dts:
> >>>
> >>> &qup_uart6_cts {
> >>> bias-disable;
> >>> };
> >>> &qup_uart6_rts {
> >>> drive-strength = <2>;
> >>> bias-disable;
> >>> };
> >>> &qup_uart6_rx {
> >>> bias-pull-up;
> >>> };
> >>> &qup_uart6_tx {
> >>> drive-strength = <2>;
> >>> bias-disable;
> >>
> >> It's not related to this patchset, but sounds good, please change the
> >> DTS to match it. I can rebase my patch on top of it.
> >
> > I guess it's related in that the patchset is touching everything and
> > one would assume that something touched so recently would represent
> > the current best practices. Maybe that's a weak argument, but if I saw
> > a patch that was about trying to clean up all the pinctrl across all
> > the older SoCs that I would assume that the pinctrl would be clean
> > after that patch and would be a good example to follow as best
> > practice. Thus it's relevant to talk about whether this patch is
> > ending us up at best practice or not.
> >
> >
> >>> };
> >>>
> >>> Also, as per latest agreement with Bjorn, we should be moving the
> >>> default drive strength to the SoC.dtsi file, so going further:
> >>
> >> How is it related to this patch? Sure, feel free to move drive strength
> >> anywhere. We can discuss it. But it is not part of this patch.
> >
> > Moving the drive strength can certainly be discussed / done in a later patch.
> >
> >
> >>> In Soc.dtsi:
> >>>
> >>> qup_uart6_txrx: qup-uart6-txrx-state {
> >>> qup_uart6_tx {
> >>> pins = "gpio47";
> >>> function = "qup6";
> >>> drive-strength = <2>;
> >>> };
> >>> qup_uart6_rx {
> >>> pins = "gpio48";
> >>> function = "qup6";
> >>> };
> >>> };
> >>> qup_uart6_cts: qup-uart6-cts-state {
> >>> pins = "gpio45";
> >>> function = "qup6";
> >>> };
> >>> qup_uart6_rts: qup-uart6-rts-state {
> >>> pins = "gpio46";
> >>> function = "qup6";
> >>> drive-strength = <2>;
> >>
> >> These are not part of DTSI. They exist in DTS, not in DTSI. You now
> >> introduce a change entirely different than this patchset is doing. It
> >> makes sense on its own, but it is not related to this patchset.
> >
> > It is relevant to discuss because it would be the correct way to solve
> > the same issue with "uart9" that you used to justify why you needed an
> > extra "uart9-default" subnode.
>
> The goal of this patchset is to solve dtbs_check. It's goal is not
> "generic guidance how Qualcomm pin configuration nodes should be
> written", because whatever you and I agree, it does not much matter. The
> maintainers decision matter.
>
> If there is a consensus/preferred way to organize it, sure, document it
> somewhere, store it in email/lore in concise way, and I can fix up the
> patch to match it. It is just not my goal now, so I am not pushing
> towards any such direction.
>
> Best regards,
> Krzysztof
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] arm64: dts: qcom: sdm845: align TLMM pin configuration with DT schema
2022-10-04 14:55 ` Doug Anderson
@ 2022-10-05 8:18 ` Krzysztof Kozlowski
0 siblings, 0 replies; 17+ messages in thread
From: Krzysztof Kozlowski @ 2022-10-05 8:18 UTC (permalink / raw)
To: Doug Anderson
Cc: Andy Gross, Bjorn Andersson, Konrad Dybcio, Linus Walleij,
Rob Herring, Krzysztof Kozlowski, linux-arm-msm,
open list:GPIO SUBSYSTEM,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML
On 04/10/2022 16:55, Doug Anderson wrote:
> Hi,
>
> On Tue, Oct 4, 2022 at 1:40 AM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 04/10/2022 00:59, Doug Anderson wrote:
>>> Hi,
>>>
>>> On Mon, Oct 3, 2022 at 10:45 AM Krzysztof Kozlowski
>>> <krzysztof.kozlowski@linaro.org> wrote:
>>>>
>>>> On 03/10/2022 18:14, Doug Anderson wrote:
>>>>> Hi,
>>>>>
>>>>> On Fri, Sep 30, 2022 at 1:06 PM Krzysztof Kozlowski
>>>>> <krzysztof.kozlowski@linaro.org> wrote:
>>>>>>
>>>>>> DT schema expects TLMM pin configuration nodes to be named with
>>>>>> '-state' suffix and their optional children with '-pins' suffix.
>>>>>>
>>>>>> The sdm854.dtsi file defined several pin configuration nodes which are
>>>>>> customized by the boards. Therefore keep a additional "default-pins"
>>>>>> node inside so the boards can add more of configuration nodes. Such
>>>>>> additional configuration nodes always need 'function' property now
>>>>>> (required by DT schema).
>>>>>>
>>>>>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>>>> ---
>>>>>> arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi | 344 +++++++-----------
>>>>>> arch/arm64/boot/dts/qcom/sdm845-db845c.dts | 76 ++--
>>>>>> .../arm64/boot/dts/qcom/sdm845-lg-common.dtsi | 60 ++-
>>>>>> arch/arm64/boot/dts/qcom/sdm845-lg-judyln.dts | 2 +-
>>>>>> arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 60 ++-
>>>>>> .../boot/dts/qcom/sdm845-oneplus-common.dtsi | 88 ++---
>>>>>> .../boot/dts/qcom/sdm845-shift-axolotl.dts | 138 +++----
>>>>>> .../dts/qcom/sdm845-sony-xperia-tama.dtsi | 6 +-
>>>>>> .../boot/dts/qcom/sdm845-xiaomi-beryllium.dts | 26 +-
>>>>>> .../boot/dts/qcom/sdm845-xiaomi-polaris.dts | 30 +-
>>>>>> arch/arm64/boot/dts/qcom/sdm845.dtsi | 305 +++++++---------
>>>>>> .../boot/dts/qcom/sdm850-lenovo-yoga-c630.dts | 33 +-
>>>>>> .../boot/dts/qcom/sdm850-samsung-w737.dts | 96 ++---
>>>>>> 13 files changed, 513 insertions(+), 751 deletions(-)
>>>>>>
>>>>>> diff --git a/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi b/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi
>>>>>> index b5f11fbcc300..3403cdcdd49c 100644
>>>>>> --- a/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi
>>>>>> +++ b/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi
>>>>>> @@ -993,21 +993,21 @@ &wifi {
>>>>>> /* PINCTRL - additions to nodes defined in sdm845.dtsi */
>>>>>>
>>>>>> &qspi_cs0 {
>>>>>> - pinconf {
>>>>>> + default-pins {
>>>>>> pins = "gpio90";
>>>>>> bias-disable;
>>>>>> };
>>>>>> };
>>>>>>
>>>>>> &qspi_clk {
>>>>>> - pinconf {
>>>>>> + default-pins {
>>>>>> pins = "gpio95";
>>>>>> bias-disable;
>>>>>> };
>>>>>> };
>>>>>>
>>>>>> &qspi_data01 {
>>>>>> - pinconf {
>>>>>> + default-pins {
>>>>>> pins = "gpio91", "gpio92";
>>>>>
>>>>> I haven't been fully involved in all the discussion here, but the
>>>>> above doesn't look like it matches the way that Bjorn wanted to go
>>>>> [1]. I would sorta expect it to look like this:
>>>>>
>>>>> /* QSPI always needs a clock and IO pins */
>>>>> qspi_basic: {
>>>>> qspi_clk: {
>>>>> pins = "gpio95";
>>>>> function = "qspi_clk";
>>>>> };
>>>>> qspi_data01: {
>>>>> pins = "gpio95";
>>>>> function = "qspi_clk";
>>>>> };
>>>>> }
>>>>>
>>>>> /* QSPI will need one or both chip selects */
>>>>> qspi_cs0: qspi-cs0-state {
>>>>> pins = "gpio90";
>>>>> function = "qspi_cs";
>>>>> };
>>>>>
>>>>> qspi_cs1: qspi-cs1-state {
>>>>> pins = "gpio89";
>>>>> function = "qspi_cs";
>>>>> };
>>>>>
>>>>> /* If using all 4 data lines we need these */
>>>>> qspi_data12: qspi-data12-state {
>>>>> pins = "gpio93", "gpio94";
>>>>> function = "qspi_data";
>>>>> };
>>>>>
>>>>> Basically grouping things together in a two-level node when it makes
>>>>> sense and then using 1-level nodes for "mixin" pins. Then you'd end up
>>>>> doing one of these things:
>>>>>
>>>>> pinctrl-0 = <&qspi_basic &qspi_cs0>;
>>>>> pinctrl-0 = <&qspi_basic &qspi_cs1>;
>>>>> pinctrl-0 = <&qspi_basic &qspi_cs0 &qspi_data12>;
>>>>
>>>>
>>>> I don't get how my patch changes the existing approach? Such pattern was
>>>> already there.
>>>
>>> Before your patch things were split in two nodes, the muxing and the
>>> configuration. AKA when you combined the soc.dtsi and the board.dts
>>> you'd have:
>>>
>>> qspi_cs0: qspi-cs0-state {
>>> pinmux {
>>> pins = "...";
>>> ... muxing properties ...
>>> };
>>> pinconf {
>>> pins = "...";
>>> ... config properties ...
>>> };
>>> };
>>
>> Which was also not good pattern. Muxing and configuring is basically the
>> same function of pin controller. Splitting them makes no sense at all.
>
> Agreed, it was not a good pattern.
>
>
>>> Your patch combines the "pinmux" and "pinconf" nodes into one. So when
>>> you combine the soc.dtsi and the board.dts after your patch you now
>>> have:
>>>
>>> qspi_cs0: qspi-cs0-state {
>>> default-pins {
>>> pins = "...";
>>> ... muxing properties ...
>>> ... config properties ...
>>> };
>>> };
>>>
>>>
>>> That's fine and is functionally correct. ...but IMO it sets a bad
>>> example for people to follow (though, of course, it's really up to
>>> Bjorn). The "default-pins" subnode serves no purpose. If you're
>>> touching all this stuff anyway you might as well not end up with
>>> something that's a bad example for people to follow.
>>
>> I understand, you're right. I wanted to keep minimal amount of changes
>> to the DTS layout but since I am touching almost everything, I can
>> rework it.
>>
>>
>>>> Again - you end up or you ended up? We discuss here what this patch did.
>>>> So are you sure that this patch did something like that (and it wasn't
>>>> already there)?
>>>>
>>>>>
>>>>> Note that the extra tags of "qspi_clk" and "qspi_data01" are important
>>>>> since it lets the individual boards easily set pulls / drive strengths
>>>>> without needing to replicate the hierarchy of the SoC. So if a board
>>>>> wanted to set the pull of the cs0 line, just:
>>>>>
>>>>> &qspi_cs0 {
>>>>> bias-disable;
>>>>> };
>>>>>
>>>>> [1] https://lore.kernel.org/lkml/CAD=FV=VUL4GmjaibAMhKNdpEso_Hg_R=XeMaqah1LSj_9-Ce4Q@mail.gmail.com/
>>>>>
>>>>>
>>>>>> @@ -1016,7 +1016,7 @@ pinconf {
>>>>>> };
>>>>>>
>>>>>> &qup_i2c3_default {
>>>>>> - pinconf {
>>>>>> + default-pins {
>>>>>> pins = "gpio41", "gpio42";
>>>>>> drive-strength = <2>;
>>>>>
>>>>> I don't see any benefit to two-levels above. Why not just get rid of
>>>>> the "default-pins" and put the stuff directly under qup_i2c3_default?
>>>>
>>>> For the same reason I told Konrad?
>>>
>>> OK. I looked at what you end up with for "qup_uart9" after your
>>> patches and it's definitely not my favorite place to end up at. If
>>> nothing else you are double-specifying "function" in both
>>> "default-pins" and "tx-pins"/"rx-pins". If those disagree then what
>>> happens?
>>
>> The same happens and happened before. My patch does nothing related to
>> allowing or disallowing wrong muxing/config nodes. In the past you could
>> have conflicting setups. Now it's exactly the same.
>
> Right that you could have had conflicting setups before, too. However,
> in the past we always avoided specifying the function for a given pin
> in more than one node.
>
> The old way was super verbose and had a lot of repetition for sure,
> but it sorta made sense if you thought of it in words. AKA the old way
> said this after combining the soc.dtsi and the board.dts
>
> * For pins 45, 46, 47, 48: set the function to "qup6"
> * For pin 45: set a pulldown
> * For pins 46 and 47: set the drive strength to 2 and disable pulls
> * For pin 48: set a pullup
>
> So it repeated the "for pins" part and that was most definitely non
> ideal. However, with your new setup you are sometimes doing:
>
> * For pins 47 and 48: set the function to "qup6"
> * For pin 45: set the function to "qup6" and set a pulldown
> * For pins 46 and 47: set the function to "qup6" and the drive
> strength to 2 and disable pulls
> * For pin 48: set the function to "qup6" and set a pullup
>
> That repeats the 'set the function to "qup6"' more than I'd like.
Indeed, it got more duplicated/error-prone.
>
>
>> The double-specifying of "function" is not a result of '-state'/'-pins'
>> reorganization but aligning with common TLMM schema *which requires
>> function* for every node.
>
> I guess now that we've talked through it, I'd say that if we have to
> specify a function for every node then we should strive for each pin
> to be in exactly one node in any given active configuration. That
> makes the schema happy and also avoids double-specifying the function.
>
> I think in all of the examples I've given that this is true.
OK, we can rework entire DTS to follow this approach.
>
>
>>> In general also we end up specifying that extra level of
>>> "default-pins" in many cases for no purpose. We also end up
>>> replicating hierarchy in the board dts files (the dts files are
>>> replicating the "default-pins" nodes from the parent).
>>
>> OK, let's fix this. That will need either:
>> 1. /delete-node with copying of most of properties from default-pins
>> 2. or move the CTS/TX/RX config pins to the DTSI.
>
> I think in my proposal the CTS/TX/RX stuff moved to the dtsi which solved it.
OK
>
>
>>>>>> /* PINCTRL - additions to nodes defined in sdm845.dtsi */
>>>>>> &qup_spi2_default {
>>>>>> - pinmux {
>>>>>> + default-pins {
>>>>>> drive-strength = <16>;
>>>>>> };
>>>>>> };
>>>>>>
>>>>>> &qup_uart3_default{
>>>>>> - pinmux {
>>>>>> + default-pins {
>>>>>> pins = "gpio41", "gpio42", "gpio43", "gpio44";
>>>>>> function = "qup3";
>>>>>> };
>>>>>> };
>>>>>>
>>>>>> &qup_i2c10_default {
>>>>>> - pinconf {
>>>>>> + default-pins {
>>>>>> pins = "gpio55", "gpio56";
>>>>>> drive-strength = <2>;
>>>>>> bias-disable;
>>>>>> @@ -1144,37 +1144,37 @@ pinconf {
>>>>>> };
>>>>>>
>>>>>> &qup_uart6_default {
>>>>>> - pinmux {
>>>>>> - pins = "gpio45", "gpio46", "gpio47", "gpio48";
>>>>>> - function = "qup6";
>>>>>> - };
>>>>>> -
>>>>>> - cts {
>>>>>> + cts-pins {
>>>>>> pins = "gpio45";
>>>>>> + function = "qup6";
>>>>>> bias-disable;
>>>>>> };
>>>>>>
>>>>>> - rts-tx {
>>>>>> + rts-tx-pins {
>>>>>> pins = "gpio46", "gpio47";
>>>>>> + function = "qup6";
>>>>>> drive-strength = <2>;
>>>>>> bias-disable;
>>>>>> };
>>>>>>
>>>>>> - rx {
>>>>>> + rx-pins {
>>>>>> pins = "gpio48";
>>>>>> + function = "qup6";
>>>>>> bias-pull-up;
>>>>>> };
>>>>>> };
>>>>>
>>>>> I didn't check everything about this patch, but skimming through I
>>>>> believe that the uart6 handling here is wrong. You'll end up with:>
>>>>> qup_uart6_default: qup-uart6-default-state {
>>>>> default-pins {
>>>>> pins = "gpio47", "gpio48";
>>>>> function = "qup6";
>>>>
>>>> This piece was removed.
>>>
>>> It was? How/where? I tried applying your patch and I still see "qup6"
>>> under the default-pins node in sdm845.dtsi.
>>>
>>
>> Ah, you're right. The default-pins are coming from DTSI.
>>
>> So delete-node?
>
> I would prefer the solution I proposed. Re-pasting here:
>
> In Soc.dtsi:
>
> qup_uart6_txrx: qup-uart6-txrx-state {
> qup_uart6_tx {
> pins = "gpio47";
> function = "qup6";
> };
> qup_uart6_rx {
> pins = "gpio48";
> function = "qup6";
> };
> };
> qup_uart6_cts: qup-uart6-cts-state {
> pins = "gpio45";
> function = "qup6";
> };
> qup_uart6_rts: qup-uart6-rts-state {
> pins = "gpio46";
> function = "qup6";
> };
>
> In board.dts:
>
> &qup_uart6_cts {
> bias-disable;
> };
> &qup_uart6_rts {
> drive-strength = <2>;
> bias-disable;
> };
> &qup_uart6_rx {
> bias-pull-up;
> };
> &qup_uart6_tx {
> drive-strength = <2>;
> bias-disable;
> };
>
OK
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: (subset) [PATCH 2/2] dt-bindings: pinctrl: qcom,sdm845: convert to dtschema
2022-09-30 20:05 ` [PATCH 2/2] dt-bindings: pinctrl: qcom,sdm845: convert to dtschema Krzysztof Kozlowski
2022-09-30 22:05 ` Rob Herring
2022-10-03 17:11 ` Rob Herring
@ 2022-10-17 22:42 ` Krzysztof Kozlowski
2 siblings, 0 replies; 17+ messages in thread
From: Krzysztof Kozlowski @ 2022-10-17 22:42 UTC (permalink / raw)
To: Rob Herring, Andy Gross, Krzysztof Kozlowski, linux-kernel,
devicetree, linux-gpio, Linus Walleij, linux-arm-msm,
Bjorn Andersson, Konrad Dybcio, Krzysztof Kozlowski
On Fri, 30 Sep 2022 22:05:29 +0200, Krzysztof Kozlowski wrote:
> Convert Qualcomm SDM845 pin controller bindings to DT schema. Keep
> the parsing of pin configuration subnodes consistent with other Qualcomm
> schemas (children named with '-state' suffix, their children with
> '-pins').
>
>
Applied, thanks!
[2/2] dt-bindings: pinctrl: qcom,sdm845: convert to dtschema
https://git.kernel.org/krzk/linux-dt/c/dba79c34605d60cb02c2951c701ea0bc16937153
Best regards,
--
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2022-10-17 22:42 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-30 20:05 [PATCH 0/2] pinctrl/arm64: qcom: convert sdm845 bindings to DT schema Krzysztof Kozlowski
2022-09-30 20:05 ` [PATCH 1/2] arm64: dts: qcom: sdm845: align TLMM pin configuration with " Krzysztof Kozlowski
2022-09-30 20:13 ` Konrad Dybcio
2022-09-30 20:19 ` Krzysztof Kozlowski
2022-09-30 20:19 ` Konrad Dybcio
2022-10-03 16:14 ` Doug Anderson
2022-10-03 17:45 ` Krzysztof Kozlowski
2022-10-03 22:59 ` Doug Anderson
2022-10-04 8:40 ` Krzysztof Kozlowski
2022-10-04 14:55 ` Doug Anderson
2022-10-05 8:18 ` Krzysztof Kozlowski
2022-09-30 20:05 ` [PATCH 2/2] dt-bindings: pinctrl: qcom,sdm845: convert to dtschema Krzysztof Kozlowski
2022-09-30 22:05 ` Rob Herring
2022-10-01 9:08 ` Krzysztof Kozlowski
2022-10-03 17:10 ` Rob Herring
2022-10-03 17:11 ` Rob Herring
2022-10-17 22:42 ` (subset) " Krzysztof Kozlowski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).