devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/5] dt-bindings: mmc: / ARM: qcom: correct reg-names and clock entries
@ 2022-07-11  8:29 Krzysztof Kozlowski
  2022-07-11  8:29 ` [PATCH v2 1/5] dt-bindings: mmc: sdhci-msm: fix reg-names entries Krzysztof Kozlowski
                   ` (4 more replies)
  0 siblings, 5 replies; 15+ messages in thread
From: Krzysztof Kozlowski @ 2022-07-11  8:29 UTC (permalink / raw)
  To: Ulf Hansson, Rob Herring, Krzysztof Kozlowski, Andy Gross,
	Bjorn Andersson, Konrad Dybcio, Bhupesh Sharma, linux-mmc,
	devicetree, linux-kernel, linux-arm-msm
  Cc: Douglas Anderson, Krzysztof Kozlowski

Hi,

No dependencies.  DT bindings patches are independent from DTS, so they can go
via separate tree.

Changes since v1
================
1. Add Rb tags.
2. Rework reg-names constraints according to Doug's feedback.

Best regards,
Krzysztof

Krzysztof Kozlowski (5):
  dt-bindings: mmc: sdhci-msm: fix reg-names entries
  dt-bindings: mmc: sdhci-msm: constrain reg-names perp variants
  arm64: dts: qcom: align SDHCI reg-names with DT schema
  ARM: dts: qcom: align SDHCI reg-names with DT schema
  ARM: dts: qcom: align SDHCI clocks with DT schema

 .../devicetree/bindings/mmc/sdhci-msm.yaml    | 61 ++++++++++++-------
 arch/arm/boot/dts/qcom-apq8084.dtsi           | 16 ++---
 arch/arm/boot/dts/qcom-ipq4019.dtsi           |  5 +-
 arch/arm/boot/dts/qcom-msm8226.dtsi           | 24 ++++----
 arch/arm/boot/dts/qcom-msm8974.dtsi           | 24 ++++----
 arch/arm/boot/dts/qcom-msm8974pro.dtsi        |  6 +-
 arch/arm/boot/dts/qcom-sdx65.dtsi             |  2 +-
 arch/arm64/boot/dts/qcom/ipq8074.dtsi         |  2 +-
 arch/arm64/boot/dts/qcom/msm8916.dtsi         |  4 +-
 arch/arm64/boot/dts/qcom/msm8953.dtsi         |  4 +-
 arch/arm64/boot/dts/qcom/msm8994.dtsi         |  4 +-
 arch/arm64/boot/dts/qcom/msm8996.dtsi         |  4 +-
 arch/arm64/boot/dts/qcom/msm8998.dtsi         |  2 +-
 13 files changed, 87 insertions(+), 71 deletions(-)

-- 
2.34.1


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

* [PATCH v2 1/5] dt-bindings: mmc: sdhci-msm: fix reg-names entries
  2022-07-11  8:29 [PATCH v2 0/5] dt-bindings: mmc: / ARM: qcom: correct reg-names and clock entries Krzysztof Kozlowski
@ 2022-07-11  8:29 ` Krzysztof Kozlowski
  2022-07-11  8:29 ` [PATCH v2 2/5] dt-bindings: mmc: sdhci-msm: constrain reg-names perp variants Krzysztof Kozlowski
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 15+ messages in thread
From: Krzysztof Kozlowski @ 2022-07-11  8:29 UTC (permalink / raw)
  To: Ulf Hansson, Rob Herring, Krzysztof Kozlowski, Andy Gross,
	Bjorn Andersson, Konrad Dybcio, Bhupesh Sharma, linux-mmc,
	devicetree, linux-kernel, linux-arm-msm
  Cc: Douglas Anderson, Krzysztof Kozlowski

Bindings before conversion to DT schema expected reg-names without
"_mem" suffix.  This was used by older DTS files and by the MSM SDHCI
driver.

Reported-by: Douglas Anderson <dianders@chromium.org>
Fixes: edfbf8c307ff ("dt-bindings: mmc: sdhci-msm: Fix issues in yaml bindings")
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
---
 .../devicetree/bindings/mmc/sdhci-msm.yaml    | 24 +++++++++----------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml b/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
index 0853d0c32dc7..fc6e5221985a 100644
--- a/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
+++ b/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
@@ -60,22 +60,22 @@ properties:
     maxItems: 4
     oneOf:
       - items:
-          - const: hc_mem
+          - const: hc
       - items:
-          - const: hc_mem
-          - const: core_mem
+          - const: hc
+          - const: core
       - items:
-          - const: hc_mem
-          - const: cqe_mem
+          - const: hc
+          - const: cqhci
       - items:
-          - const: hc_mem
-          - const: cqe_mem
-          - const: ice_mem
+          - const: hc
+          - const: cqhci
+          - const: ice
       - items:
-          - const: hc_mem
-          - const: core_mem
-          - const: cqe_mem
-          - const: ice_mem
+          - const: hc
+          - const: core
+          - const: cqhci
+          - const: ice
 
   clocks:
     minItems: 3
-- 
2.34.1


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

* [PATCH v2 2/5] dt-bindings: mmc: sdhci-msm: constrain reg-names perp variants
  2022-07-11  8:29 [PATCH v2 0/5] dt-bindings: mmc: / ARM: qcom: correct reg-names and clock entries Krzysztof Kozlowski
  2022-07-11  8:29 ` [PATCH v2 1/5] dt-bindings: mmc: sdhci-msm: fix reg-names entries Krzysztof Kozlowski
@ 2022-07-11  8:29 ` Krzysztof Kozlowski
  2022-07-11 14:52   ` Doug Anderson
  2022-07-11  8:29 ` [PATCH v2 3/5] arm64: dts: qcom: align SDHCI reg-names with DT schema Krzysztof Kozlowski
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 15+ messages in thread
From: Krzysztof Kozlowski @ 2022-07-11  8:29 UTC (permalink / raw)
  To: Ulf Hansson, Rob Herring, Krzysztof Kozlowski, Andy Gross,
	Bjorn Andersson, Konrad Dybcio, Bhupesh Sharma, linux-mmc,
	devicetree, linux-kernel, linux-arm-msm
  Cc: Douglas Anderson, Krzysztof Kozlowski

The entries in arrays must have fixed order, so the bindings and Linux
driver expecting various combinations of 'reg' addresses was never
actually conforming to guidelines.

The 'core' reg entry is valid only for SDCC v4 and lower, so disallow it
in SDCC v5.  SDCC v4 supports CQE and ICE, so allow them, even though
the qcom,sdhci-msm-v4 compatible is used also for earlier SoCs with SDCC
v2 or v3, so it is not entirely accurate.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

---

Changes since v1:
1. Rework the patch based on Doug's feedback.
---
 .../devicetree/bindings/mmc/sdhci-msm.yaml    | 61 ++++++++++++-------
 1 file changed, 38 insertions(+), 23 deletions(-)

diff --git a/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml b/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
index fc6e5221985a..2f0fdd65e908 100644
--- a/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
+++ b/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
@@ -49,33 +49,11 @@ properties:
 
   reg:
     minItems: 1
-    items:
-      - description: Host controller register map
-      - description: SD Core register map
-      - description: CQE register map
-      - description: Inline Crypto Engine register map
+    maxItems: 4
 
   reg-names:
     minItems: 1
     maxItems: 4
-    oneOf:
-      - items:
-          - const: hc
-      - items:
-          - const: hc
-          - const: core
-      - items:
-          - const: hc
-          - const: cqhci
-      - items:
-          - const: hc
-          - const: cqhci
-          - const: ice
-      - items:
-          - const: hc
-          - const: core
-          - const: cqhci
-          - const: ice
 
   clocks:
     minItems: 3
@@ -177,6 +155,43 @@ required:
 allOf:
   - $ref: mmc-controller.yaml#
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,sdhci-msm-v4
+    then:
+      properties:
+        reg:
+          minItems: 2
+          items:
+            - description: Host controller register map
+            - description: SD Core register map
+            - description: CQE register map
+            - description: Inline Crypto Engine register map
+        reg-names:
+          minItems: 2
+          items:
+            - const: hc
+            - const: core
+            - const: cqhci
+            - const: ice
+    else:
+      properties:
+        reg:
+          minItems: 1
+          items:
+            - description: Host controller register map
+            - description: CQE register map
+            - description: Inline Crypto Engine register map
+        reg-names:
+          minItems: 1
+          items:
+            - const: hc
+            - const: cqhci
+            - const: ice
+
 unevaluatedProperties: false
 
 examples:
-- 
2.34.1


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

* [PATCH v2 3/5] arm64: dts: qcom: align SDHCI reg-names with DT schema
  2022-07-11  8:29 [PATCH v2 0/5] dt-bindings: mmc: / ARM: qcom: correct reg-names and clock entries Krzysztof Kozlowski
  2022-07-11  8:29 ` [PATCH v2 1/5] dt-bindings: mmc: sdhci-msm: fix reg-names entries Krzysztof Kozlowski
  2022-07-11  8:29 ` [PATCH v2 2/5] dt-bindings: mmc: sdhci-msm: constrain reg-names perp variants Krzysztof Kozlowski
@ 2022-07-11  8:29 ` Krzysztof Kozlowski
  2022-07-11  8:42   ` Konrad Dybcio
  2022-07-11  8:29 ` [PATCH v2 4/5] ARM: " Krzysztof Kozlowski
  2022-07-11  8:29 ` [PATCH v2 5/5] ARM: dts: qcom: align SDHCI clocks " Krzysztof Kozlowski
  4 siblings, 1 reply; 15+ messages in thread
From: Krzysztof Kozlowski @ 2022-07-11  8:29 UTC (permalink / raw)
  To: Ulf Hansson, Rob Herring, Krzysztof Kozlowski, Andy Gross,
	Bjorn Andersson, Konrad Dybcio, Bhupesh Sharma, linux-mmc,
	devicetree, linux-kernel, linux-arm-msm
  Cc: Douglas Anderson, Krzysztof Kozlowski

DT schema requires SDHCI reg names to be hc/core without "_mem" suffix,
just like TXT bindings were expecting before the conversion.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
---
 arch/arm64/boot/dts/qcom/ipq8074.dtsi | 2 +-
 arch/arm64/boot/dts/qcom/msm8916.dtsi | 4 ++--
 arch/arm64/boot/dts/qcom/msm8953.dtsi | 4 ++--
 arch/arm64/boot/dts/qcom/msm8994.dtsi | 4 ++--
 arch/arm64/boot/dts/qcom/msm8996.dtsi | 4 ++--
 arch/arm64/boot/dts/qcom/msm8998.dtsi | 2 +-
 6 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/ipq8074.dtsi b/arch/arm64/boot/dts/qcom/ipq8074.dtsi
index a6cb0dafcc17..2b9374f61d5b 100644
--- a/arch/arm64/boot/dts/qcom/ipq8074.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq8074.dtsi
@@ -379,7 +379,7 @@ spmi_bus: spmi@200f000 {
 		sdhc_1: mmc@7824900 {
 			compatible = "qcom,sdhci-msm-v4";
 			reg = <0x7824900 0x500>, <0x7824000 0x800>;
-			reg-names = "hc_mem", "core_mem";
+			reg-names = "hc", "core";
 
 			interrupts = <GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>,
 				     <GIC_SPI 138 IRQ_TYPE_LEVEL_HIGH>;
diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi
index 48bc2e09128d..0bdf4d39f778 100644
--- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
@@ -1469,7 +1469,7 @@ lpass_codec: audio-codec@771c000 {
 		sdhc_1: mmc@7824000 {
 			compatible = "qcom,msm8916-sdhci", "qcom,sdhci-msm-v4";
 			reg = <0x07824900 0x11c>, <0x07824000 0x800>;
-			reg-names = "hc_mem", "core_mem";
+			reg-names = "hc", "core";
 
 			interrupts = <GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>,
 				     <GIC_SPI 138 IRQ_TYPE_LEVEL_HIGH>;
@@ -1487,7 +1487,7 @@ sdhc_1: mmc@7824000 {
 		sdhc_2: mmc@7864000 {
 			compatible = "qcom,msm8916-sdhci", "qcom,sdhci-msm-v4";
 			reg = <0x07864900 0x11c>, <0x07864000 0x800>;
-			reg-names = "hc_mem", "core_mem";
+			reg-names = "hc", "core";
 
 			interrupts = <GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>,
 				     <GIC_SPI 221 IRQ_TYPE_LEVEL_HIGH>;
diff --git a/arch/arm64/boot/dts/qcom/msm8953.dtsi b/arch/arm64/boot/dts/qcom/msm8953.dtsi
index 1bc0ef476cdb..97dde1a429d9 100644
--- a/arch/arm64/boot/dts/qcom/msm8953.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8953.dtsi
@@ -799,7 +799,7 @@ sdhc_1: mmc@7824900 {
 			compatible = "qcom,msm8953-sdhci", "qcom,sdhci-msm-v4";
 
 			reg = <0x7824900 0x500>, <0x7824000 0x800>;
-			reg-names = "hc_mem", "core_mem";
+			reg-names = "hc", "core";
 
 			interrupts = <GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>,
 				     <GIC_SPI 138 IRQ_TYPE_LEVEL_HIGH>;
@@ -859,7 +859,7 @@ sdhc_2: mmc@7864900 {
 			compatible = "qcom,msm8953-sdhci", "qcom,sdhci-msm-v4";
 
 			reg = <0x7864900 0x500>, <0x7864000 0x800>;
-			reg-names = "hc_mem", "core_mem";
+			reg-names = "hc", "core";
 
 			interrupts = <GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>,
 				     <GIC_SPI 221 IRQ_TYPE_LEVEL_HIGH>;
diff --git a/arch/arm64/boot/dts/qcom/msm8994.dtsi b/arch/arm64/boot/dts/qcom/msm8994.dtsi
index 8bc6c070e306..35c1ca080684 100644
--- a/arch/arm64/boot/dts/qcom/msm8994.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8994.dtsi
@@ -464,7 +464,7 @@ usb@f9200000 {
 		sdhc1: mmc@f9824900 {
 			compatible = "qcom,msm8994-sdhci", "qcom,sdhci-msm-v4";
 			reg = <0xf9824900 0x1a0>, <0xf9824000 0x800>;
-			reg-names = "hc_mem", "core_mem";
+			reg-names = "hc", "core";
 
 			interrupts = <GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>,
 				     <GIC_SPI 138 IRQ_TYPE_LEVEL_HIGH>;
@@ -487,7 +487,7 @@ sdhc1: mmc@f9824900 {
 		sdhc2: mmc@f98a4900 {
 			compatible = "qcom,msm8994-sdhci", "qcom,sdhci-msm-v4";
 			reg = <0xf98a4900 0x11c>, <0xf98a4000 0x800>;
-			reg-names = "hc_mem", "core_mem";
+			reg-names = "hc", "core";
 
 			interrupts = <GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>,
 				<GIC_SPI 221 IRQ_TYPE_LEVEL_HIGH>;
diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
index 25d6b26fab60..9745df5dc007 100644
--- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
@@ -2896,7 +2896,7 @@ hsusb_phy2: phy@7412000 {
 		sdhc1: mmc@7464900 {
 			compatible = "qcom,msm8996-sdhci", "qcom,sdhci-msm-v4";
 			reg = <0x07464900 0x11c>, <0x07464000 0x800>;
-			reg-names = "hc_mem", "core_mem";
+			reg-names = "hc", "core";
 
 			interrupts = <GIC_SPI 141 IRQ_TYPE_LEVEL_HIGH>,
 					<GIC_SPI 134 IRQ_TYPE_LEVEL_HIGH>;
@@ -2920,7 +2920,7 @@ sdhc1: mmc@7464900 {
 		sdhc2: mmc@74a4900 {
 			compatible = "qcom,msm8996-sdhci", "qcom,sdhci-msm-v4";
 			reg = <0x074a4900 0x314>, <0x074a4000 0x800>;
-			reg-names = "hc_mem", "core_mem";
+			reg-names = "hc", "core";
 
 			interrupts = <GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>,
 				      <GIC_SPI 221 IRQ_TYPE_LEVEL_HIGH>;
diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi b/arch/arm64/boot/dts/qcom/msm8998.dtsi
index e263a59d84b0..c98f36f95f3c 100644
--- a/arch/arm64/boot/dts/qcom/msm8998.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi
@@ -2078,7 +2078,7 @@ qusb2phy: phy@c012000 {
 		sdhc2: mmc@c0a4900 {
 			compatible = "qcom,msm8998-sdhci", "qcom,sdhci-msm-v4";
 			reg = <0x0c0a4900 0x314>, <0x0c0a4000 0x800>;
-			reg-names = "hc_mem", "core_mem";
+			reg-names = "hc", "core";
 
 			interrupts = <GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>,
 				     <GIC_SPI 221 IRQ_TYPE_LEVEL_HIGH>;
-- 
2.34.1


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

* [PATCH v2 4/5] ARM: dts: qcom: align SDHCI reg-names with DT schema
  2022-07-11  8:29 [PATCH v2 0/5] dt-bindings: mmc: / ARM: qcom: correct reg-names and clock entries Krzysztof Kozlowski
                   ` (2 preceding siblings ...)
  2022-07-11  8:29 ` [PATCH v2 3/5] arm64: dts: qcom: align SDHCI reg-names with DT schema Krzysztof Kozlowski
@ 2022-07-11  8:29 ` Krzysztof Kozlowski
  2022-07-11  8:42   ` Konrad Dybcio
  2022-07-11  8:29 ` [PATCH v2 5/5] ARM: dts: qcom: align SDHCI clocks " Krzysztof Kozlowski
  4 siblings, 1 reply; 15+ messages in thread
From: Krzysztof Kozlowski @ 2022-07-11  8:29 UTC (permalink / raw)
  To: Ulf Hansson, Rob Herring, Krzysztof Kozlowski, Andy Gross,
	Bjorn Andersson, Konrad Dybcio, Bhupesh Sharma, linux-mmc,
	devicetree, linux-kernel, linux-arm-msm
  Cc: Douglas Anderson, Krzysztof Kozlowski

DT schema requires SDHCI reg names to be hc/core without "_mem" suffix,
just like TXT bindings were expecting before the conversion.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
---
 arch/arm/boot/dts/qcom-apq8084.dtsi | 4 ++--
 arch/arm/boot/dts/qcom-ipq4019.dtsi | 1 +
 arch/arm/boot/dts/qcom-msm8226.dtsi | 6 +++---
 arch/arm/boot/dts/qcom-msm8974.dtsi | 6 +++---
 arch/arm/boot/dts/qcom-sdx65.dtsi   | 2 +-
 5 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/arch/arm/boot/dts/qcom-apq8084.dtsi b/arch/arm/boot/dts/qcom-apq8084.dtsi
index 3e8bded2b5c8..45f3cbcf6238 100644
--- a/arch/arm/boot/dts/qcom-apq8084.dtsi
+++ b/arch/arm/boot/dts/qcom-apq8084.dtsi
@@ -422,7 +422,7 @@ blsp2_uart2: serial@f995e000 {
 		mmc@f9824900 {
 			compatible = "qcom,apq8084-sdhci", "qcom,sdhci-msm-v4";
 			reg = <0xf9824900 0x11c>, <0xf9824000 0x800>;
-			reg-names = "hc_mem", "core_mem";
+			reg-names = "hc", "core";
 			interrupts = <GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 138 IRQ_TYPE_LEVEL_HIGH>;
 			interrupt-names = "hc_irq", "pwr_irq";
 			clocks = <&gcc GCC_SDCC1_APPS_CLK>,
@@ -435,7 +435,7 @@ mmc@f9824900 {
 		mmc@f98a4900 {
 			compatible = "qcom,apq8084-sdhci", "qcom,sdhci-msm-v4";
 			reg = <0xf98a4900 0x11c>, <0xf98a4000 0x800>;
-			reg-names = "hc_mem", "core_mem";
+			reg-names = "hc", "core";
 			interrupts = <GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 221 IRQ_TYPE_LEVEL_HIGH>;
 			interrupt-names = "hc_irq", "pwr_irq";
 			clocks = <&gcc GCC_SDCC2_APPS_CLK>,
diff --git a/arch/arm/boot/dts/qcom-ipq4019.dtsi b/arch/arm/boot/dts/qcom-ipq4019.dtsi
index a2632349cec4..1b98764bab7a 100644
--- a/arch/arm/boot/dts/qcom-ipq4019.dtsi
+++ b/arch/arm/boot/dts/qcom-ipq4019.dtsi
@@ -224,6 +224,7 @@ vqmmc: regulator@1948000 {
 		sdhci: mmc@7824900 {
 			compatible = "qcom,sdhci-msm-v4";
 			reg = <0x7824900 0x11c>, <0x7824000 0x800>;
+			reg-names = "hc", "core";
 			interrupts = <GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 138 IRQ_TYPE_LEVEL_HIGH>;
 			interrupt-names = "hc_irq", "pwr_irq";
 			bus-width = <8>;
diff --git a/arch/arm/boot/dts/qcom-msm8226.dtsi b/arch/arm/boot/dts/qcom-msm8226.dtsi
index 0b5effdb269a..f711463d22dc 100644
--- a/arch/arm/boot/dts/qcom-msm8226.dtsi
+++ b/arch/arm/boot/dts/qcom-msm8226.dtsi
@@ -137,7 +137,7 @@ apcs: syscon@f9011000 {
 		sdhc_1: mmc@f9824900 {
 			compatible = "qcom,msm8226-sdhci", "qcom,sdhci-msm-v4";
 			reg = <0xf9824900 0x11c>, <0xf9824000 0x800>;
-			reg-names = "hc_mem", "core_mem";
+			reg-names = "hc", "core";
 			interrupts = <GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>,
 				     <GIC_SPI 138 IRQ_TYPE_LEVEL_HIGH>;
 			interrupt-names = "hc_irq", "pwr_irq";
@@ -153,7 +153,7 @@ sdhc_1: mmc@f9824900 {
 		sdhc_2: mmc@f98a4900 {
 			compatible = "qcom,msm8226-sdhci", "qcom,sdhci-msm-v4";
 			reg = <0xf98a4900 0x11c>, <0xf98a4000 0x800>;
-			reg-names = "hc_mem", "core_mem";
+			reg-names = "hc", "core";
 			interrupts = <GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>,
 				     <GIC_SPI 221 IRQ_TYPE_LEVEL_HIGH>;
 			interrupt-names = "hc_irq", "pwr_irq";
@@ -169,7 +169,7 @@ sdhc_2: mmc@f98a4900 {
 		sdhc_3: mmc@f9864900 {
 			compatible = "qcom,msm8226-sdhci", "qcom,sdhci-msm-v4";
 			reg = <0xf9864900 0x11c>, <0xf9864000 0x800>;
-			reg-names = "hc_mem", "core_mem";
+			reg-names = "hc", "core";
 			interrupts = <GIC_SPI 127 IRQ_TYPE_LEVEL_HIGH>,
 				     <GIC_SPI 224 IRQ_TYPE_LEVEL_HIGH>;
 			interrupt-names = "hc_irq", "pwr_irq";
diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi
index 11b4206036e6..971eceaef3d1 100644
--- a/arch/arm/boot/dts/qcom-msm8974.dtsi
+++ b/arch/arm/boot/dts/qcom-msm8974.dtsi
@@ -439,7 +439,7 @@ acc3: clock-controller@f90b8000 {
 		sdhc_1: mmc@f9824900 {
 			compatible = "qcom,msm8974-sdhci", "qcom,sdhci-msm-v4";
 			reg = <0xf9824900 0x11c>, <0xf9824000 0x800>;
-			reg-names = "hc_mem", "core_mem";
+			reg-names = "hc", "core";
 			interrupts = <GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>,
 				     <GIC_SPI 138 IRQ_TYPE_LEVEL_HIGH>;
 			interrupt-names = "hc_irq", "pwr_irq";
@@ -456,7 +456,7 @@ sdhc_1: mmc@f9824900 {
 		sdhc_3: mmc@f9864900 {
 			compatible = "qcom,msm8974-sdhci", "qcom,sdhci-msm-v4";
 			reg = <0xf9864900 0x11c>, <0xf9864000 0x800>;
-			reg-names = "hc_mem", "core_mem";
+			reg-names = "hc", "core";
 			interrupts = <GIC_SPI 127 IRQ_TYPE_LEVEL_HIGH>,
 				     <GIC_SPI 224 IRQ_TYPE_LEVEL_HIGH>;
 			interrupt-names = "hc_irq", "pwr_irq";
@@ -475,7 +475,7 @@ sdhc_3: mmc@f9864900 {
 		sdhc_2: mmc@f98a4900 {
 			compatible = "qcom,msm8974-sdhci", "qcom,sdhci-msm-v4";
 			reg = <0xf98a4900 0x11c>, <0xf98a4000 0x800>;
-			reg-names = "hc_mem", "core_mem";
+			reg-names = "hc", "core";
 			interrupts = <GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>,
 				     <GIC_SPI 221 IRQ_TYPE_LEVEL_HIGH>;
 			interrupt-names = "hc_irq", "pwr_irq";
diff --git a/arch/arm/boot/dts/qcom-sdx65.dtsi b/arch/arm/boot/dts/qcom-sdx65.dtsi
index 7a193678b4f5..4f3389cb6300 100644
--- a/arch/arm/boot/dts/qcom-sdx65.dtsi
+++ b/arch/arm/boot/dts/qcom-sdx65.dtsi
@@ -334,7 +334,7 @@ glink-edge {
 		sdhc_1: mmc@8804000 {
 			compatible = "qcom,sdx65-sdhci", "qcom,sdhci-msm-v5";
 			reg = <0x08804000 0x1000>;
-			reg-names = "hc_mem";
+			reg-names = "hc";
 			interrupts = <GIC_SPI 210 IRQ_TYPE_LEVEL_HIGH>,
 				     <GIC_SPI 227 IRQ_TYPE_LEVEL_HIGH>;
 			interrupt-names = "hc_irq", "pwr_irq";
-- 
2.34.1


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

* [PATCH v2 5/5] ARM: dts: qcom: align SDHCI clocks with DT schema
  2022-07-11  8:29 [PATCH v2 0/5] dt-bindings: mmc: / ARM: qcom: correct reg-names and clock entries Krzysztof Kozlowski
                   ` (3 preceding siblings ...)
  2022-07-11  8:29 ` [PATCH v2 4/5] ARM: " Krzysztof Kozlowski
@ 2022-07-11  8:29 ` Krzysztof Kozlowski
  2022-07-11  8:43   ` Konrad Dybcio
  4 siblings, 1 reply; 15+ messages in thread
From: Krzysztof Kozlowski @ 2022-07-11  8:29 UTC (permalink / raw)
  To: Ulf Hansson, Rob Herring, Krzysztof Kozlowski, Andy Gross,
	Bjorn Andersson, Konrad Dybcio, Bhupesh Sharma, linux-mmc,
	devicetree, linux-kernel, linux-arm-msm
  Cc: Douglas Anderson, Krzysztof Kozlowski

The DT schema expects clocks iface-core order.  No functional change.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
---
 arch/arm/boot/dts/qcom-apq8084.dtsi    | 12 ++++++------
 arch/arm/boot/dts/qcom-ipq4019.dtsi    |  4 ++--
 arch/arm/boot/dts/qcom-msm8226.dtsi    | 18 +++++++++---------
 arch/arm/boot/dts/qcom-msm8974.dtsi    | 18 +++++++++---------
 arch/arm/boot/dts/qcom-msm8974pro.dtsi |  6 +++---
 5 files changed, 29 insertions(+), 29 deletions(-)

diff --git a/arch/arm/boot/dts/qcom-apq8084.dtsi b/arch/arm/boot/dts/qcom-apq8084.dtsi
index 45f3cbcf6238..c887ac5cdd7d 100644
--- a/arch/arm/boot/dts/qcom-apq8084.dtsi
+++ b/arch/arm/boot/dts/qcom-apq8084.dtsi
@@ -425,10 +425,10 @@ mmc@f9824900 {
 			reg-names = "hc", "core";
 			interrupts = <GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 138 IRQ_TYPE_LEVEL_HIGH>;
 			interrupt-names = "hc_irq", "pwr_irq";
-			clocks = <&gcc GCC_SDCC1_APPS_CLK>,
-				 <&gcc GCC_SDCC1_AHB_CLK>,
+			clocks = <&gcc GCC_SDCC1_AHB_CLK>,
+				 <&gcc GCC_SDCC1_APPS_CLK>,
 				 <&xo_board>;
-			clock-names = "core", "iface", "xo";
+			clock-names = "iface", "core", "xo";
 			status = "disabled";
 		};
 
@@ -438,10 +438,10 @@ mmc@f98a4900 {
 			reg-names = "hc", "core";
 			interrupts = <GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 221 IRQ_TYPE_LEVEL_HIGH>;
 			interrupt-names = "hc_irq", "pwr_irq";
-			clocks = <&gcc GCC_SDCC2_APPS_CLK>,
-				 <&gcc GCC_SDCC2_AHB_CLK>,
+			clocks = <&gcc GCC_SDCC2_AHB_CLK>,
+				 <&gcc GCC_SDCC2_APPS_CLK>,
 				 <&xo_board>;
-			clock-names = "core", "iface", "xo";
+			clock-names = "iface", "core", "xo";
 			status = "disabled";
 		};
 
diff --git a/arch/arm/boot/dts/qcom-ipq4019.dtsi b/arch/arm/boot/dts/qcom-ipq4019.dtsi
index 1b98764bab7a..a8a32a5e7e5d 100644
--- a/arch/arm/boot/dts/qcom-ipq4019.dtsi
+++ b/arch/arm/boot/dts/qcom-ipq4019.dtsi
@@ -228,9 +228,9 @@ sdhci: mmc@7824900 {
 			interrupts = <GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 138 IRQ_TYPE_LEVEL_HIGH>;
 			interrupt-names = "hc_irq", "pwr_irq";
 			bus-width = <8>;
-			clocks = <&gcc GCC_SDCC1_APPS_CLK>, <&gcc GCC_SDCC1_AHB_CLK>,
+			clocks = <&gcc GCC_SDCC1_AHB_CLK>, <&gcc GCC_SDCC1_APPS_CLK>,
 				 <&gcc GCC_DCD_XO_CLK>;
-			clock-names = "core", "iface", "xo";
+			clock-names = "iface", "core", "xo";
 			status = "disabled";
 		};
 
diff --git a/arch/arm/boot/dts/qcom-msm8226.dtsi b/arch/arm/boot/dts/qcom-msm8226.dtsi
index f711463d22dc..9d4223bf8fc1 100644
--- a/arch/arm/boot/dts/qcom-msm8226.dtsi
+++ b/arch/arm/boot/dts/qcom-msm8226.dtsi
@@ -141,10 +141,10 @@ sdhc_1: mmc@f9824900 {
 			interrupts = <GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>,
 				     <GIC_SPI 138 IRQ_TYPE_LEVEL_HIGH>;
 			interrupt-names = "hc_irq", "pwr_irq";
-			clocks = <&gcc GCC_SDCC1_APPS_CLK>,
-				 <&gcc GCC_SDCC1_AHB_CLK>,
+			clocks = <&gcc GCC_SDCC1_AHB_CLK>,
+				 <&gcc GCC_SDCC1_APPS_CLK>,
 				 <&xo_board>;
-			clock-names = "core", "iface", "xo";
+			clock-names = "iface", "core", "xo";
 			pinctrl-names = "default";
 			pinctrl-0 = <&sdhc1_default_state>;
 			status = "disabled";
@@ -157,10 +157,10 @@ sdhc_2: mmc@f98a4900 {
 			interrupts = <GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>,
 				     <GIC_SPI 221 IRQ_TYPE_LEVEL_HIGH>;
 			interrupt-names = "hc_irq", "pwr_irq";
-			clocks = <&gcc GCC_SDCC2_APPS_CLK>,
-				 <&gcc GCC_SDCC2_AHB_CLK>,
+			clocks = <&gcc GCC_SDCC2_AHB_CLK>,
+				 <&gcc GCC_SDCC2_APPS_CLK>,
 				 <&xo_board>;
-			clock-names = "core", "iface", "xo";
+			clock-names = "iface", "core", "xo";
 			pinctrl-names = "default";
 			pinctrl-0 = <&sdhc2_default_state>;
 			status = "disabled";
@@ -173,10 +173,10 @@ sdhc_3: mmc@f9864900 {
 			interrupts = <GIC_SPI 127 IRQ_TYPE_LEVEL_HIGH>,
 				     <GIC_SPI 224 IRQ_TYPE_LEVEL_HIGH>;
 			interrupt-names = "hc_irq", "pwr_irq";
-			clocks = <&gcc GCC_SDCC3_APPS_CLK>,
-				 <&gcc GCC_SDCC3_AHB_CLK>,
+			clocks = <&gcc GCC_SDCC3_AHB_CLK>,
+				 <&gcc GCC_SDCC3_APPS_CLK>,
 				 <&xo_board>;
-			clock-names = "core", "iface", "xo";
+			clock-names = "iface", "core", "xo";
 			pinctrl-names = "default";
 			pinctrl-0 = <&sdhc3_default_state>;
 			status = "disabled";
diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi
index 971eceaef3d1..1f4baa6ac64d 100644
--- a/arch/arm/boot/dts/qcom-msm8974.dtsi
+++ b/arch/arm/boot/dts/qcom-msm8974.dtsi
@@ -443,10 +443,10 @@ sdhc_1: mmc@f9824900 {
 			interrupts = <GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>,
 				     <GIC_SPI 138 IRQ_TYPE_LEVEL_HIGH>;
 			interrupt-names = "hc_irq", "pwr_irq";
-			clocks = <&gcc GCC_SDCC1_APPS_CLK>,
-				 <&gcc GCC_SDCC1_AHB_CLK>,
+			clocks = <&gcc GCC_SDCC1_AHB_CLK>,
+				 <&gcc GCC_SDCC1_APPS_CLK>,
 				 <&xo_board>;
-			clock-names = "core", "iface", "xo";
+			clock-names = "iface", "core", "xo";
 			bus-width = <8>;
 			non-removable;
 
@@ -460,10 +460,10 @@ sdhc_3: mmc@f9864900 {
 			interrupts = <GIC_SPI 127 IRQ_TYPE_LEVEL_HIGH>,
 				     <GIC_SPI 224 IRQ_TYPE_LEVEL_HIGH>;
 			interrupt-names = "hc_irq", "pwr_irq";
-			clocks = <&gcc GCC_SDCC3_APPS_CLK>,
-				 <&gcc GCC_SDCC3_AHB_CLK>,
+			clocks = <&gcc GCC_SDCC3_AHB_CLK>,
+				 <&gcc GCC_SDCC3_APPS_CLK>,
 				 <&xo_board>;
-			clock-names = "core", "iface", "xo";
+			clock-names = "iface", "core", "xo";
 			bus-width = <4>;
 
 			#address-cells = <1>;
@@ -479,10 +479,10 @@ sdhc_2: mmc@f98a4900 {
 			interrupts = <GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>,
 				     <GIC_SPI 221 IRQ_TYPE_LEVEL_HIGH>;
 			interrupt-names = "hc_irq", "pwr_irq";
-			clocks = <&gcc GCC_SDCC2_APPS_CLK>,
-				 <&gcc GCC_SDCC2_AHB_CLK>,
+			clocks = <&gcc GCC_SDCC2_AHB_CLK>,
+				 <&gcc GCC_SDCC2_APPS_CLK>,
 				 <&xo_board>;
-			clock-names = "core", "iface", "xo";
+			clock-names = "iface", "core", "xo";
 			bus-width = <4>;
 
 			#address-cells = <1>;
diff --git a/arch/arm/boot/dts/qcom-msm8974pro.dtsi b/arch/arm/boot/dts/qcom-msm8974pro.dtsi
index 1e882e16a221..58df6e75ab6d 100644
--- a/arch/arm/boot/dts/qcom-msm8974pro.dtsi
+++ b/arch/arm/boot/dts/qcom-msm8974pro.dtsi
@@ -10,10 +10,10 @@ &gpu {
 };
 
 &sdhc_1 {
-	clocks = <&gcc GCC_SDCC1_APPS_CLK>,
-		 <&gcc GCC_SDCC1_AHB_CLK>,
+	clocks = <&gcc GCC_SDCC1_AHB_CLK>,
+		 <&gcc GCC_SDCC1_APPS_CLK>,
 		 <&xo_board>,
 		 <&gcc GCC_SDCC1_CDCCAL_FF_CLK>,
 		 <&gcc GCC_SDCC1_CDCCAL_SLEEP_CLK>;
-	clock-names = "core", "iface", "xo", "cal", "sleep";
+	clock-names = "iface", "core", "xo", "cal", "sleep";
 };
-- 
2.34.1


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

* Re: [PATCH v2 3/5] arm64: dts: qcom: align SDHCI reg-names with DT schema
  2022-07-11  8:29 ` [PATCH v2 3/5] arm64: dts: qcom: align SDHCI reg-names with DT schema Krzysztof Kozlowski
@ 2022-07-11  8:42   ` Konrad Dybcio
  0 siblings, 0 replies; 15+ messages in thread
From: Konrad Dybcio @ 2022-07-11  8:42 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Ulf Hansson, Rob Herring,
	Krzysztof Kozlowski, Andy Gross, Bjorn Andersson, Bhupesh Sharma,
	linux-mmc, devicetree, linux-kernel, linux-arm-msm
  Cc: Douglas Anderson



On 11.07.2022 10:29, Krzysztof Kozlowski wrote:
> DT schema requires SDHCI reg names to be hc/core without "_mem" suffix,
> just like TXT bindings were expecting before the conversion.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@somainline.org>

Konrad
>  arch/arm64/boot/dts/qcom/ipq8074.dtsi | 2 +-
>  arch/arm64/boot/dts/qcom/msm8916.dtsi | 4 ++--
>  arch/arm64/boot/dts/qcom/msm8953.dtsi | 4 ++--
>  arch/arm64/boot/dts/qcom/msm8994.dtsi | 4 ++--
>  arch/arm64/boot/dts/qcom/msm8996.dtsi | 4 ++--
>  arch/arm64/boot/dts/qcom/msm8998.dtsi | 2 +-
>  6 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/ipq8074.dtsi b/arch/arm64/boot/dts/qcom/ipq8074.dtsi
> index a6cb0dafcc17..2b9374f61d5b 100644
> --- a/arch/arm64/boot/dts/qcom/ipq8074.dtsi
> +++ b/arch/arm64/boot/dts/qcom/ipq8074.dtsi
> @@ -379,7 +379,7 @@ spmi_bus: spmi@200f000 {
>  		sdhc_1: mmc@7824900 {
>  			compatible = "qcom,sdhci-msm-v4";
>  			reg = <0x7824900 0x500>, <0x7824000 0x800>;
> -			reg-names = "hc_mem", "core_mem";
> +			reg-names = "hc", "core";
>  
>  			interrupts = <GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>,
>  				     <GIC_SPI 138 IRQ_TYPE_LEVEL_HIGH>;
> diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi
> index 48bc2e09128d..0bdf4d39f778 100644
> --- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
> @@ -1469,7 +1469,7 @@ lpass_codec: audio-codec@771c000 {
>  		sdhc_1: mmc@7824000 {
>  			compatible = "qcom,msm8916-sdhci", "qcom,sdhci-msm-v4";
>  			reg = <0x07824900 0x11c>, <0x07824000 0x800>;
> -			reg-names = "hc_mem", "core_mem";
> +			reg-names = "hc", "core";
>  
>  			interrupts = <GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>,
>  				     <GIC_SPI 138 IRQ_TYPE_LEVEL_HIGH>;
> @@ -1487,7 +1487,7 @@ sdhc_1: mmc@7824000 {
>  		sdhc_2: mmc@7864000 {
>  			compatible = "qcom,msm8916-sdhci", "qcom,sdhci-msm-v4";
>  			reg = <0x07864900 0x11c>, <0x07864000 0x800>;
> -			reg-names = "hc_mem", "core_mem";
> +			reg-names = "hc", "core";
>  
>  			interrupts = <GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>,
>  				     <GIC_SPI 221 IRQ_TYPE_LEVEL_HIGH>;
> diff --git a/arch/arm64/boot/dts/qcom/msm8953.dtsi b/arch/arm64/boot/dts/qcom/msm8953.dtsi
> index 1bc0ef476cdb..97dde1a429d9 100644
> --- a/arch/arm64/boot/dts/qcom/msm8953.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8953.dtsi
> @@ -799,7 +799,7 @@ sdhc_1: mmc@7824900 {
>  			compatible = "qcom,msm8953-sdhci", "qcom,sdhci-msm-v4";
>  
>  			reg = <0x7824900 0x500>, <0x7824000 0x800>;
> -			reg-names = "hc_mem", "core_mem";
> +			reg-names = "hc", "core";
>  
>  			interrupts = <GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>,
>  				     <GIC_SPI 138 IRQ_TYPE_LEVEL_HIGH>;
> @@ -859,7 +859,7 @@ sdhc_2: mmc@7864900 {
>  			compatible = "qcom,msm8953-sdhci", "qcom,sdhci-msm-v4";
>  
>  			reg = <0x7864900 0x500>, <0x7864000 0x800>;
> -			reg-names = "hc_mem", "core_mem";
> +			reg-names = "hc", "core";
>  
>  			interrupts = <GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>,
>  				     <GIC_SPI 221 IRQ_TYPE_LEVEL_HIGH>;
> diff --git a/arch/arm64/boot/dts/qcom/msm8994.dtsi b/arch/arm64/boot/dts/qcom/msm8994.dtsi
> index 8bc6c070e306..35c1ca080684 100644
> --- a/arch/arm64/boot/dts/qcom/msm8994.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8994.dtsi
> @@ -464,7 +464,7 @@ usb@f9200000 {
>  		sdhc1: mmc@f9824900 {
>  			compatible = "qcom,msm8994-sdhci", "qcom,sdhci-msm-v4";
>  			reg = <0xf9824900 0x1a0>, <0xf9824000 0x800>;
> -			reg-names = "hc_mem", "core_mem";
> +			reg-names = "hc", "core";
>  
>  			interrupts = <GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>,
>  				     <GIC_SPI 138 IRQ_TYPE_LEVEL_HIGH>;
> @@ -487,7 +487,7 @@ sdhc1: mmc@f9824900 {
>  		sdhc2: mmc@f98a4900 {
>  			compatible = "qcom,msm8994-sdhci", "qcom,sdhci-msm-v4";
>  			reg = <0xf98a4900 0x11c>, <0xf98a4000 0x800>;
> -			reg-names = "hc_mem", "core_mem";
> +			reg-names = "hc", "core";
>  
>  			interrupts = <GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>,
>  				<GIC_SPI 221 IRQ_TYPE_LEVEL_HIGH>;
> diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
> index 25d6b26fab60..9745df5dc007 100644
> --- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
> @@ -2896,7 +2896,7 @@ hsusb_phy2: phy@7412000 {
>  		sdhc1: mmc@7464900 {
>  			compatible = "qcom,msm8996-sdhci", "qcom,sdhci-msm-v4";
>  			reg = <0x07464900 0x11c>, <0x07464000 0x800>;
> -			reg-names = "hc_mem", "core_mem";
> +			reg-names = "hc", "core";
>  
>  			interrupts = <GIC_SPI 141 IRQ_TYPE_LEVEL_HIGH>,
>  					<GIC_SPI 134 IRQ_TYPE_LEVEL_HIGH>;
> @@ -2920,7 +2920,7 @@ sdhc1: mmc@7464900 {
>  		sdhc2: mmc@74a4900 {
>  			compatible = "qcom,msm8996-sdhci", "qcom,sdhci-msm-v4";
>  			reg = <0x074a4900 0x314>, <0x074a4000 0x800>;
> -			reg-names = "hc_mem", "core_mem";
> +			reg-names = "hc", "core";
>  
>  			interrupts = <GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>,
>  				      <GIC_SPI 221 IRQ_TYPE_LEVEL_HIGH>;
> diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi b/arch/arm64/boot/dts/qcom/msm8998.dtsi
> index e263a59d84b0..c98f36f95f3c 100644
> --- a/arch/arm64/boot/dts/qcom/msm8998.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi
> @@ -2078,7 +2078,7 @@ qusb2phy: phy@c012000 {
>  		sdhc2: mmc@c0a4900 {
>  			compatible = "qcom,msm8998-sdhci", "qcom,sdhci-msm-v4";
>  			reg = <0x0c0a4900 0x314>, <0x0c0a4000 0x800>;
> -			reg-names = "hc_mem", "core_mem";
> +			reg-names = "hc", "core";
>  
>  			interrupts = <GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>,
>  				     <GIC_SPI 221 IRQ_TYPE_LEVEL_HIGH>;

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

* Re: [PATCH v2 4/5] ARM: dts: qcom: align SDHCI reg-names with DT schema
  2022-07-11  8:29 ` [PATCH v2 4/5] ARM: " Krzysztof Kozlowski
@ 2022-07-11  8:42   ` Konrad Dybcio
  0 siblings, 0 replies; 15+ messages in thread
From: Konrad Dybcio @ 2022-07-11  8:42 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Ulf Hansson, Rob Herring,
	Krzysztof Kozlowski, Andy Gross, Bjorn Andersson, Bhupesh Sharma,
	linux-mmc, devicetree, linux-kernel, linux-arm-msm
  Cc: Douglas Anderson



On 11.07.2022 10:29, Krzysztof Kozlowski wrote:
> DT schema requires SDHCI reg names to be hc/core without "_mem" suffix,
> just like TXT bindings were expecting before the conversion.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@somainline.org>

Konrad
>  arch/arm/boot/dts/qcom-apq8084.dtsi | 4 ++--
>  arch/arm/boot/dts/qcom-ipq4019.dtsi | 1 +
>  arch/arm/boot/dts/qcom-msm8226.dtsi | 6 +++---
>  arch/arm/boot/dts/qcom-msm8974.dtsi | 6 +++---
>  arch/arm/boot/dts/qcom-sdx65.dtsi   | 2 +-
>  5 files changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/qcom-apq8084.dtsi b/arch/arm/boot/dts/qcom-apq8084.dtsi
> index 3e8bded2b5c8..45f3cbcf6238 100644
> --- a/arch/arm/boot/dts/qcom-apq8084.dtsi
> +++ b/arch/arm/boot/dts/qcom-apq8084.dtsi
> @@ -422,7 +422,7 @@ blsp2_uart2: serial@f995e000 {
>  		mmc@f9824900 {
>  			compatible = "qcom,apq8084-sdhci", "qcom,sdhci-msm-v4";
>  			reg = <0xf9824900 0x11c>, <0xf9824000 0x800>;
> -			reg-names = "hc_mem", "core_mem";
> +			reg-names = "hc", "core";
>  			interrupts = <GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 138 IRQ_TYPE_LEVEL_HIGH>;
>  			interrupt-names = "hc_irq", "pwr_irq";
>  			clocks = <&gcc GCC_SDCC1_APPS_CLK>,
> @@ -435,7 +435,7 @@ mmc@f9824900 {
>  		mmc@f98a4900 {
>  			compatible = "qcom,apq8084-sdhci", "qcom,sdhci-msm-v4";
>  			reg = <0xf98a4900 0x11c>, <0xf98a4000 0x800>;
> -			reg-names = "hc_mem", "core_mem";
> +			reg-names = "hc", "core";
>  			interrupts = <GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 221 IRQ_TYPE_LEVEL_HIGH>;
>  			interrupt-names = "hc_irq", "pwr_irq";
>  			clocks = <&gcc GCC_SDCC2_APPS_CLK>,
> diff --git a/arch/arm/boot/dts/qcom-ipq4019.dtsi b/arch/arm/boot/dts/qcom-ipq4019.dtsi
> index a2632349cec4..1b98764bab7a 100644
> --- a/arch/arm/boot/dts/qcom-ipq4019.dtsi
> +++ b/arch/arm/boot/dts/qcom-ipq4019.dtsi
> @@ -224,6 +224,7 @@ vqmmc: regulator@1948000 {
>  		sdhci: mmc@7824900 {
>  			compatible = "qcom,sdhci-msm-v4";
>  			reg = <0x7824900 0x11c>, <0x7824000 0x800>;
> +			reg-names = "hc", "core";
>  			interrupts = <GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 138 IRQ_TYPE_LEVEL_HIGH>;
>  			interrupt-names = "hc_irq", "pwr_irq";
>  			bus-width = <8>;
> diff --git a/arch/arm/boot/dts/qcom-msm8226.dtsi b/arch/arm/boot/dts/qcom-msm8226.dtsi
> index 0b5effdb269a..f711463d22dc 100644
> --- a/arch/arm/boot/dts/qcom-msm8226.dtsi
> +++ b/arch/arm/boot/dts/qcom-msm8226.dtsi
> @@ -137,7 +137,7 @@ apcs: syscon@f9011000 {
>  		sdhc_1: mmc@f9824900 {
>  			compatible = "qcom,msm8226-sdhci", "qcom,sdhci-msm-v4";
>  			reg = <0xf9824900 0x11c>, <0xf9824000 0x800>;
> -			reg-names = "hc_mem", "core_mem";
> +			reg-names = "hc", "core";
>  			interrupts = <GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>,
>  				     <GIC_SPI 138 IRQ_TYPE_LEVEL_HIGH>;
>  			interrupt-names = "hc_irq", "pwr_irq";
> @@ -153,7 +153,7 @@ sdhc_1: mmc@f9824900 {
>  		sdhc_2: mmc@f98a4900 {
>  			compatible = "qcom,msm8226-sdhci", "qcom,sdhci-msm-v4";
>  			reg = <0xf98a4900 0x11c>, <0xf98a4000 0x800>;
> -			reg-names = "hc_mem", "core_mem";
> +			reg-names = "hc", "core";
>  			interrupts = <GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>,
>  				     <GIC_SPI 221 IRQ_TYPE_LEVEL_HIGH>;
>  			interrupt-names = "hc_irq", "pwr_irq";
> @@ -169,7 +169,7 @@ sdhc_2: mmc@f98a4900 {
>  		sdhc_3: mmc@f9864900 {
>  			compatible = "qcom,msm8226-sdhci", "qcom,sdhci-msm-v4";
>  			reg = <0xf9864900 0x11c>, <0xf9864000 0x800>;
> -			reg-names = "hc_mem", "core_mem";
> +			reg-names = "hc", "core";
>  			interrupts = <GIC_SPI 127 IRQ_TYPE_LEVEL_HIGH>,
>  				     <GIC_SPI 224 IRQ_TYPE_LEVEL_HIGH>;
>  			interrupt-names = "hc_irq", "pwr_irq";
> diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi
> index 11b4206036e6..971eceaef3d1 100644
> --- a/arch/arm/boot/dts/qcom-msm8974.dtsi
> +++ b/arch/arm/boot/dts/qcom-msm8974.dtsi
> @@ -439,7 +439,7 @@ acc3: clock-controller@f90b8000 {
>  		sdhc_1: mmc@f9824900 {
>  			compatible = "qcom,msm8974-sdhci", "qcom,sdhci-msm-v4";
>  			reg = <0xf9824900 0x11c>, <0xf9824000 0x800>;
> -			reg-names = "hc_mem", "core_mem";
> +			reg-names = "hc", "core";
>  			interrupts = <GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>,
>  				     <GIC_SPI 138 IRQ_TYPE_LEVEL_HIGH>;
>  			interrupt-names = "hc_irq", "pwr_irq";
> @@ -456,7 +456,7 @@ sdhc_1: mmc@f9824900 {
>  		sdhc_3: mmc@f9864900 {
>  			compatible = "qcom,msm8974-sdhci", "qcom,sdhci-msm-v4";
>  			reg = <0xf9864900 0x11c>, <0xf9864000 0x800>;
> -			reg-names = "hc_mem", "core_mem";
> +			reg-names = "hc", "core";
>  			interrupts = <GIC_SPI 127 IRQ_TYPE_LEVEL_HIGH>,
>  				     <GIC_SPI 224 IRQ_TYPE_LEVEL_HIGH>;
>  			interrupt-names = "hc_irq", "pwr_irq";
> @@ -475,7 +475,7 @@ sdhc_3: mmc@f9864900 {
>  		sdhc_2: mmc@f98a4900 {
>  			compatible = "qcom,msm8974-sdhci", "qcom,sdhci-msm-v4";
>  			reg = <0xf98a4900 0x11c>, <0xf98a4000 0x800>;
> -			reg-names = "hc_mem", "core_mem";
> +			reg-names = "hc", "core";
>  			interrupts = <GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>,
>  				     <GIC_SPI 221 IRQ_TYPE_LEVEL_HIGH>;
>  			interrupt-names = "hc_irq", "pwr_irq";
> diff --git a/arch/arm/boot/dts/qcom-sdx65.dtsi b/arch/arm/boot/dts/qcom-sdx65.dtsi
> index 7a193678b4f5..4f3389cb6300 100644
> --- a/arch/arm/boot/dts/qcom-sdx65.dtsi
> +++ b/arch/arm/boot/dts/qcom-sdx65.dtsi
> @@ -334,7 +334,7 @@ glink-edge {
>  		sdhc_1: mmc@8804000 {
>  			compatible = "qcom,sdx65-sdhci", "qcom,sdhci-msm-v5";
>  			reg = <0x08804000 0x1000>;
> -			reg-names = "hc_mem";
> +			reg-names = "hc";
>  			interrupts = <GIC_SPI 210 IRQ_TYPE_LEVEL_HIGH>,
>  				     <GIC_SPI 227 IRQ_TYPE_LEVEL_HIGH>;
>  			interrupt-names = "hc_irq", "pwr_irq";

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

* Re: [PATCH v2 5/5] ARM: dts: qcom: align SDHCI clocks with DT schema
  2022-07-11  8:29 ` [PATCH v2 5/5] ARM: dts: qcom: align SDHCI clocks " Krzysztof Kozlowski
@ 2022-07-11  8:43   ` Konrad Dybcio
  0 siblings, 0 replies; 15+ messages in thread
From: Konrad Dybcio @ 2022-07-11  8:43 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Ulf Hansson, Rob Herring,
	Krzysztof Kozlowski, Andy Gross, Bjorn Andersson, Bhupesh Sharma,
	linux-mmc, devicetree, linux-kernel, linux-arm-msm
  Cc: Douglas Anderson



On 11.07.2022 10:29, Krzysztof Kozlowski wrote:
> The DT schema expects clocks iface-core order.  No functional change.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@somainline.org>

Konrad
>  arch/arm/boot/dts/qcom-apq8084.dtsi    | 12 ++++++------
>  arch/arm/boot/dts/qcom-ipq4019.dtsi    |  4 ++--
>  arch/arm/boot/dts/qcom-msm8226.dtsi    | 18 +++++++++---------
>  arch/arm/boot/dts/qcom-msm8974.dtsi    | 18 +++++++++---------
>  arch/arm/boot/dts/qcom-msm8974pro.dtsi |  6 +++---
>  5 files changed, 29 insertions(+), 29 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/qcom-apq8084.dtsi b/arch/arm/boot/dts/qcom-apq8084.dtsi
> index 45f3cbcf6238..c887ac5cdd7d 100644
> --- a/arch/arm/boot/dts/qcom-apq8084.dtsi
> +++ b/arch/arm/boot/dts/qcom-apq8084.dtsi
> @@ -425,10 +425,10 @@ mmc@f9824900 {
>  			reg-names = "hc", "core";
>  			interrupts = <GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 138 IRQ_TYPE_LEVEL_HIGH>;
>  			interrupt-names = "hc_irq", "pwr_irq";
> -			clocks = <&gcc GCC_SDCC1_APPS_CLK>,
> -				 <&gcc GCC_SDCC1_AHB_CLK>,
> +			clocks = <&gcc GCC_SDCC1_AHB_CLK>,
> +				 <&gcc GCC_SDCC1_APPS_CLK>,
>  				 <&xo_board>;
> -			clock-names = "core", "iface", "xo";
> +			clock-names = "iface", "core", "xo";
>  			status = "disabled";
>  		};
>  
> @@ -438,10 +438,10 @@ mmc@f98a4900 {
>  			reg-names = "hc", "core";
>  			interrupts = <GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 221 IRQ_TYPE_LEVEL_HIGH>;
>  			interrupt-names = "hc_irq", "pwr_irq";
> -			clocks = <&gcc GCC_SDCC2_APPS_CLK>,
> -				 <&gcc GCC_SDCC2_AHB_CLK>,
> +			clocks = <&gcc GCC_SDCC2_AHB_CLK>,
> +				 <&gcc GCC_SDCC2_APPS_CLK>,
>  				 <&xo_board>;
> -			clock-names = "core", "iface", "xo";
> +			clock-names = "iface", "core", "xo";
>  			status = "disabled";
>  		};
>  
> diff --git a/arch/arm/boot/dts/qcom-ipq4019.dtsi b/arch/arm/boot/dts/qcom-ipq4019.dtsi
> index 1b98764bab7a..a8a32a5e7e5d 100644
> --- a/arch/arm/boot/dts/qcom-ipq4019.dtsi
> +++ b/arch/arm/boot/dts/qcom-ipq4019.dtsi
> @@ -228,9 +228,9 @@ sdhci: mmc@7824900 {
>  			interrupts = <GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 138 IRQ_TYPE_LEVEL_HIGH>;
>  			interrupt-names = "hc_irq", "pwr_irq";
>  			bus-width = <8>;
> -			clocks = <&gcc GCC_SDCC1_APPS_CLK>, <&gcc GCC_SDCC1_AHB_CLK>,
> +			clocks = <&gcc GCC_SDCC1_AHB_CLK>, <&gcc GCC_SDCC1_APPS_CLK>,
>  				 <&gcc GCC_DCD_XO_CLK>;
> -			clock-names = "core", "iface", "xo";
> +			clock-names = "iface", "core", "xo";
>  			status = "disabled";
>  		};
>  
> diff --git a/arch/arm/boot/dts/qcom-msm8226.dtsi b/arch/arm/boot/dts/qcom-msm8226.dtsi
> index f711463d22dc..9d4223bf8fc1 100644
> --- a/arch/arm/boot/dts/qcom-msm8226.dtsi
> +++ b/arch/arm/boot/dts/qcom-msm8226.dtsi
> @@ -141,10 +141,10 @@ sdhc_1: mmc@f9824900 {
>  			interrupts = <GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>,
>  				     <GIC_SPI 138 IRQ_TYPE_LEVEL_HIGH>;
>  			interrupt-names = "hc_irq", "pwr_irq";
> -			clocks = <&gcc GCC_SDCC1_APPS_CLK>,
> -				 <&gcc GCC_SDCC1_AHB_CLK>,
> +			clocks = <&gcc GCC_SDCC1_AHB_CLK>,
> +				 <&gcc GCC_SDCC1_APPS_CLK>,
>  				 <&xo_board>;
> -			clock-names = "core", "iface", "xo";
> +			clock-names = "iface", "core", "xo";
>  			pinctrl-names = "default";
>  			pinctrl-0 = <&sdhc1_default_state>;
>  			status = "disabled";
> @@ -157,10 +157,10 @@ sdhc_2: mmc@f98a4900 {
>  			interrupts = <GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>,
>  				     <GIC_SPI 221 IRQ_TYPE_LEVEL_HIGH>;
>  			interrupt-names = "hc_irq", "pwr_irq";
> -			clocks = <&gcc GCC_SDCC2_APPS_CLK>,
> -				 <&gcc GCC_SDCC2_AHB_CLK>,
> +			clocks = <&gcc GCC_SDCC2_AHB_CLK>,
> +				 <&gcc GCC_SDCC2_APPS_CLK>,
>  				 <&xo_board>;
> -			clock-names = "core", "iface", "xo";
> +			clock-names = "iface", "core", "xo";
>  			pinctrl-names = "default";
>  			pinctrl-0 = <&sdhc2_default_state>;
>  			status = "disabled";
> @@ -173,10 +173,10 @@ sdhc_3: mmc@f9864900 {
>  			interrupts = <GIC_SPI 127 IRQ_TYPE_LEVEL_HIGH>,
>  				     <GIC_SPI 224 IRQ_TYPE_LEVEL_HIGH>;
>  			interrupt-names = "hc_irq", "pwr_irq";
> -			clocks = <&gcc GCC_SDCC3_APPS_CLK>,
> -				 <&gcc GCC_SDCC3_AHB_CLK>,
> +			clocks = <&gcc GCC_SDCC3_AHB_CLK>,
> +				 <&gcc GCC_SDCC3_APPS_CLK>,
>  				 <&xo_board>;
> -			clock-names = "core", "iface", "xo";
> +			clock-names = "iface", "core", "xo";
>  			pinctrl-names = "default";
>  			pinctrl-0 = <&sdhc3_default_state>;
>  			status = "disabled";
> diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi
> index 971eceaef3d1..1f4baa6ac64d 100644
> --- a/arch/arm/boot/dts/qcom-msm8974.dtsi
> +++ b/arch/arm/boot/dts/qcom-msm8974.dtsi
> @@ -443,10 +443,10 @@ sdhc_1: mmc@f9824900 {
>  			interrupts = <GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>,
>  				     <GIC_SPI 138 IRQ_TYPE_LEVEL_HIGH>;
>  			interrupt-names = "hc_irq", "pwr_irq";
> -			clocks = <&gcc GCC_SDCC1_APPS_CLK>,
> -				 <&gcc GCC_SDCC1_AHB_CLK>,
> +			clocks = <&gcc GCC_SDCC1_AHB_CLK>,
> +				 <&gcc GCC_SDCC1_APPS_CLK>,
>  				 <&xo_board>;
> -			clock-names = "core", "iface", "xo";
> +			clock-names = "iface", "core", "xo";
>  			bus-width = <8>;
>  			non-removable;
>  
> @@ -460,10 +460,10 @@ sdhc_3: mmc@f9864900 {
>  			interrupts = <GIC_SPI 127 IRQ_TYPE_LEVEL_HIGH>,
>  				     <GIC_SPI 224 IRQ_TYPE_LEVEL_HIGH>;
>  			interrupt-names = "hc_irq", "pwr_irq";
> -			clocks = <&gcc GCC_SDCC3_APPS_CLK>,
> -				 <&gcc GCC_SDCC3_AHB_CLK>,
> +			clocks = <&gcc GCC_SDCC3_AHB_CLK>,
> +				 <&gcc GCC_SDCC3_APPS_CLK>,
>  				 <&xo_board>;
> -			clock-names = "core", "iface", "xo";
> +			clock-names = "iface", "core", "xo";
>  			bus-width = <4>;
>  
>  			#address-cells = <1>;
> @@ -479,10 +479,10 @@ sdhc_2: mmc@f98a4900 {
>  			interrupts = <GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>,
>  				     <GIC_SPI 221 IRQ_TYPE_LEVEL_HIGH>;
>  			interrupt-names = "hc_irq", "pwr_irq";
> -			clocks = <&gcc GCC_SDCC2_APPS_CLK>,
> -				 <&gcc GCC_SDCC2_AHB_CLK>,
> +			clocks = <&gcc GCC_SDCC2_AHB_CLK>,
> +				 <&gcc GCC_SDCC2_APPS_CLK>,
>  				 <&xo_board>;
> -			clock-names = "core", "iface", "xo";
> +			clock-names = "iface", "core", "xo";
>  			bus-width = <4>;
>  
>  			#address-cells = <1>;
> diff --git a/arch/arm/boot/dts/qcom-msm8974pro.dtsi b/arch/arm/boot/dts/qcom-msm8974pro.dtsi
> index 1e882e16a221..58df6e75ab6d 100644
> --- a/arch/arm/boot/dts/qcom-msm8974pro.dtsi
> +++ b/arch/arm/boot/dts/qcom-msm8974pro.dtsi
> @@ -10,10 +10,10 @@ &gpu {
>  };
>  
>  &sdhc_1 {
> -	clocks = <&gcc GCC_SDCC1_APPS_CLK>,
> -		 <&gcc GCC_SDCC1_AHB_CLK>,
> +	clocks = <&gcc GCC_SDCC1_AHB_CLK>,
> +		 <&gcc GCC_SDCC1_APPS_CLK>,
>  		 <&xo_board>,
>  		 <&gcc GCC_SDCC1_CDCCAL_FF_CLK>,
>  		 <&gcc GCC_SDCC1_CDCCAL_SLEEP_CLK>;
> -	clock-names = "core", "iface", "xo", "cal", "sleep";
> +	clock-names = "iface", "core", "xo", "cal", "sleep";
>  };

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

* Re: [PATCH v2 2/5] dt-bindings: mmc: sdhci-msm: constrain reg-names perp variants
  2022-07-11  8:29 ` [PATCH v2 2/5] dt-bindings: mmc: sdhci-msm: constrain reg-names perp variants Krzysztof Kozlowski
@ 2022-07-11 14:52   ` Doug Anderson
  2022-07-11 14:53     ` Krzysztof Kozlowski
  0 siblings, 1 reply; 15+ messages in thread
From: Doug Anderson @ 2022-07-11 14:52 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Ulf Hansson, Rob Herring, Krzysztof Kozlowski, Andy Gross,
	Bjorn Andersson, Konrad Dybcio, Bhupesh Sharma, Linux MMC List,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
	linux-arm-msm

Hi

On Mon, Jul 11, 2022 at 1:29 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> The entries in arrays must have fixed order, so the bindings and Linux
> driver expecting various combinations of 'reg' addresses was never
> actually conforming to guidelines.
>
> The 'core' reg entry is valid only for SDCC v4 and lower, so disallow it
> in SDCC v5.  SDCC v4 supports CQE and ICE, so allow them, even though
> the qcom,sdhci-msm-v4 compatible is used also for earlier SoCs with SDCC
> v2 or v3, so it is not entirely accurate.
>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>
> ---
>
> Changes since v1:
> 1. Rework the patch based on Doug's feedback.
> ---
>  .../devicetree/bindings/mmc/sdhci-msm.yaml    | 61 ++++++++++++-------
>  1 file changed, 38 insertions(+), 23 deletions(-)

In the ${SUBJECT} I'm not sure what a "perp variant" is. Is that a
typo or just a phrase I'm not aware of?


> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml b/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
> index fc6e5221985a..2f0fdd65e908 100644
> --- a/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
> +++ b/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
> @@ -49,33 +49,11 @@ properties:
>
>    reg:
>      minItems: 1
> -    items:
> -      - description: Host controller register map
> -      - description: SD Core register map
> -      - description: CQE register map
> -      - description: Inline Crypto Engine register map
> +    maxItems: 4
>
>    reg-names:
>      minItems: 1
>      maxItems: 4
> -    oneOf:
> -      - items:
> -          - const: hc
> -      - items:
> -          - const: hc
> -          - const: core
> -      - items:
> -          - const: hc
> -          - const: cqhci
> -      - items:
> -          - const: hc
> -          - const: cqhci
> -          - const: ice
> -      - items:
> -          - const: hc
> -          - const: core
> -          - const: cqhci
> -          - const: ice
>
>    clocks:
>      minItems: 3
> @@ -177,6 +155,43 @@ required:
>  allOf:
>    - $ref: mmc-controller.yaml#
>
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,sdhci-msm-v4
> +    then:
> +      properties:
> +        reg:
> +          minItems: 2
> +          items:
> +            - description: Host controller register map
> +            - description: SD Core register map
> +            - description: CQE register map
> +            - description: Inline Crypto Engine register map
> +        reg-names:
> +          minItems: 2
> +          items:
> +            - const: hc
> +            - const: core
> +            - const: cqhci
> +            - const: ice
> +    else:
> +      properties:
> +        reg:
> +          minItems: 1
> +          items:
> +            - description: Host controller register map
> +            - description: CQE register map
> +            - description: Inline Crypto Engine register map
> +        reg-names:
> +          minItems: 1
> +          items:
> +            - const: hc
> +            - const: cqhci
> +            - const: ice

Do you need to set "maxItems" here? If you don't then will it inherit
the maxItems of 4 from above?

-Doug

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

* Re: [PATCH v2 2/5] dt-bindings: mmc: sdhci-msm: constrain reg-names perp variants
  2022-07-11 14:52   ` Doug Anderson
@ 2022-07-11 14:53     ` Krzysztof Kozlowski
  2022-07-11 15:11       ` Doug Anderson
  0 siblings, 1 reply; 15+ messages in thread
From: Krzysztof Kozlowski @ 2022-07-11 14:53 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Ulf Hansson, Rob Herring, Krzysztof Kozlowski, Andy Gross,
	Bjorn Andersson, Konrad Dybcio, Bhupesh Sharma, Linux MMC List,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
	linux-arm-msm

On 11/07/2022 16:52, Doug Anderson wrote:
> Hi
> 
> On Mon, Jul 11, 2022 at 1:29 AM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> The entries in arrays must have fixed order, so the bindings and Linux
>> driver expecting various combinations of 'reg' addresses was never
>> actually conforming to guidelines.
>>
>> The 'core' reg entry is valid only for SDCC v4 and lower, so disallow it
>> in SDCC v5.  SDCC v4 supports CQE and ICE, so allow them, even though
>> the qcom,sdhci-msm-v4 compatible is used also for earlier SoCs with SDCC
>> v2 or v3, so it is not entirely accurate.
>>
>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>
>> ---
>>
>> Changes since v1:
>> 1. Rework the patch based on Doug's feedback.
>> ---
>>  .../devicetree/bindings/mmc/sdhci-msm.yaml    | 61 ++++++++++++-------
>>  1 file changed, 38 insertions(+), 23 deletions(-)
> 
> In the ${SUBJECT} I'm not sure what a "perp variant" is. Is that a
> typo or just a phrase I'm not aware of?

Should be:
"per variants"

> 
> 
>> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml b/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
>> index fc6e5221985a..2f0fdd65e908 100644
>> --- a/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
>> +++ b/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
>> @@ -49,33 +49,11 @@ properties:
>>
>>    reg:
>>      minItems: 1
>> -    items:
>> -      - description: Host controller register map
>> -      - description: SD Core register map
>> -      - description: CQE register map
>> -      - description: Inline Crypto Engine register map
>> +    maxItems: 4
>>
>>    reg-names:
>>      minItems: 1
>>      maxItems: 4
>> -    oneOf:
>> -      - items:
>> -          - const: hc
>> -      - items:
>> -          - const: hc
>> -          - const: core
>> -      - items:
>> -          - const: hc
>> -          - const: cqhci
>> -      - items:
>> -          - const: hc
>> -          - const: cqhci
>> -          - const: ice
>> -      - items:
>> -          - const: hc
>> -          - const: core
>> -          - const: cqhci
>> -          - const: ice
>>
>>    clocks:
>>      minItems: 3
>> @@ -177,6 +155,43 @@ required:
>>  allOf:
>>    - $ref: mmc-controller.yaml#
>>
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - qcom,sdhci-msm-v4
>> +    then:
>> +      properties:
>> +        reg:
>> +          minItems: 2
>> +          items:
>> +            - description: Host controller register map
>> +            - description: SD Core register map
>> +            - description: CQE register map
>> +            - description: Inline Crypto Engine register map
>> +        reg-names:
>> +          minItems: 2
>> +          items:
>> +            - const: hc
>> +            - const: core
>> +            - const: cqhci
>> +            - const: ice
>> +    else:
>> +      properties:
>> +        reg:
>> +          minItems: 1
>> +          items:
>> +            - description: Host controller register map
>> +            - description: CQE register map
>> +            - description: Inline Crypto Engine register map
>> +        reg-names:
>> +          minItems: 1
>> +          items:
>> +            - const: hc
>> +            - const: cqhci
>> +            - const: ice
> 
> Do you need to set "maxItems" here? If you don't then will it inherit
> the maxItems of 4 from above?

No, items determine the size instead.


Best regards,
Krzysztof

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

* Re: [PATCH v2 2/5] dt-bindings: mmc: sdhci-msm: constrain reg-names perp variants
  2022-07-11 14:53     ` Krzysztof Kozlowski
@ 2022-07-11 15:11       ` Doug Anderson
  2022-07-12  7:02         ` Krzysztof Kozlowski
  0 siblings, 1 reply; 15+ messages in thread
From: Doug Anderson @ 2022-07-11 15:11 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Ulf Hansson, Rob Herring, Krzysztof Kozlowski, Andy Gross,
	Bjorn Andersson, Konrad Dybcio, Bhupesh Sharma, Linux MMC List,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
	linux-arm-msm

Hi,

On Mon, Jul 11, 2022 at 7:53 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 11/07/2022 16:52, Doug Anderson wrote:
> > Hi
> >
> > On Mon, Jul 11, 2022 at 1:29 AM Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> wrote:
> >>
> >> The entries in arrays must have fixed order, so the bindings and Linux
> >> driver expecting various combinations of 'reg' addresses was never
> >> actually conforming to guidelines.
> >>
> >> The 'core' reg entry is valid only for SDCC v4 and lower, so disallow it
> >> in SDCC v5.  SDCC v4 supports CQE and ICE, so allow them, even though
> >> the qcom,sdhci-msm-v4 compatible is used also for earlier SoCs with SDCC
> >> v2 or v3, so it is not entirely accurate.
> >>
> >> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> >>
> >> ---
> >>
> >> Changes since v1:
> >> 1. Rework the patch based on Doug's feedback.
> >> ---
> >>  .../devicetree/bindings/mmc/sdhci-msm.yaml    | 61 ++++++++++++-------
> >>  1 file changed, 38 insertions(+), 23 deletions(-)
> >
> > In the ${SUBJECT} I'm not sure what a "perp variant" is. Is that a
> > typo or just a phrase I'm not aware of?
>
> Should be:
> "per variants"
>
> >
> >
> >> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml b/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
> >> index fc6e5221985a..2f0fdd65e908 100644
> >> --- a/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
> >> +++ b/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
> >> @@ -49,33 +49,11 @@ properties:
> >>
> >>    reg:
> >>      minItems: 1
> >> -    items:
> >> -      - description: Host controller register map
> >> -      - description: SD Core register map
> >> -      - description: CQE register map
> >> -      - description: Inline Crypto Engine register map
> >> +    maxItems: 4
> >>
> >>    reg-names:
> >>      minItems: 1
> >>      maxItems: 4
> >> -    oneOf:
> >> -      - items:
> >> -          - const: hc
> >> -      - items:
> >> -          - const: hc
> >> -          - const: core
> >> -      - items:
> >> -          - const: hc
> >> -          - const: cqhci
> >> -      - items:
> >> -          - const: hc
> >> -          - const: cqhci
> >> -          - const: ice
> >> -      - items:
> >> -          - const: hc
> >> -          - const: core
> >> -          - const: cqhci
> >> -          - const: ice
> >>
> >>    clocks:
> >>      minItems: 3
> >> @@ -177,6 +155,43 @@ required:
> >>  allOf:
> >>    - $ref: mmc-controller.yaml#
> >>
> >> +  - if:
> >> +      properties:
> >> +        compatible:
> >> +          contains:
> >> +            enum:
> >> +              - qcom,sdhci-msm-v4
> >> +    then:
> >> +      properties:
> >> +        reg:
> >> +          minItems: 2
> >> +          items:
> >> +            - description: Host controller register map
> >> +            - description: SD Core register map
> >> +            - description: CQE register map
> >> +            - description: Inline Crypto Engine register map
> >> +        reg-names:
> >> +          minItems: 2
> >> +          items:
> >> +            - const: hc
> >> +            - const: core
> >> +            - const: cqhci
> >> +            - const: ice
> >> +    else:
> >> +      properties:
> >> +        reg:
> >> +          minItems: 1
> >> +          items:
> >> +            - description: Host controller register map
> >> +            - description: CQE register map
> >> +            - description: Inline Crypto Engine register map
> >> +        reg-names:
> >> +          minItems: 1
> >> +          items:
> >> +            - const: hc
> >> +            - const: cqhci
> >> +            - const: ice
> >
> > Do you need to set "maxItems" here? If you don't then will it inherit
> > the maxItems of 4 from above?
>
> No, items determine the size instead.

Can you just remove the "maxItems" from above then? Does it buy us anything?

In any case, with the ${SUBJECT} fixed:

Reviewed-by: Douglas Anderson <dianders@chromium.org>

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

* Re: [PATCH v2 2/5] dt-bindings: mmc: sdhci-msm: constrain reg-names perp variants
  2022-07-11 15:11       ` Doug Anderson
@ 2022-07-12  7:02         ` Krzysztof Kozlowski
  2022-07-12 14:29           ` Doug Anderson
  0 siblings, 1 reply; 15+ messages in thread
From: Krzysztof Kozlowski @ 2022-07-12  7:02 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Ulf Hansson, Rob Herring, Krzysztof Kozlowski, Andy Gross,
	Bjorn Andersson, Konrad Dybcio, Bhupesh Sharma, Linux MMC List,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
	linux-arm-msm

On 11/07/2022 17:11, Doug Anderson wrote:
> Hi,
> 
> On Mon, Jul 11, 2022 at 7:53 AM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 11/07/2022 16:52, Doug Anderson wrote:
>>> Hi
>>>
>>> On Mon, Jul 11, 2022 at 1:29 AM Krzysztof Kozlowski
>>> <krzysztof.kozlowski@linaro.org> wrote:
>>>>
>>>> The entries in arrays must have fixed order, so the bindings and Linux
>>>> driver expecting various combinations of 'reg' addresses was never
>>>> actually conforming to guidelines.
>>>>
>>>> The 'core' reg entry is valid only for SDCC v4 and lower, so disallow it
>>>> in SDCC v5.  SDCC v4 supports CQE and ICE, so allow them, even though
>>>> the qcom,sdhci-msm-v4 compatible is used also for earlier SoCs with SDCC
>>>> v2 or v3, so it is not entirely accurate.
>>>>
>>>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>>
>>>> ---
>>>>
>>>> Changes since v1:
>>>> 1. Rework the patch based on Doug's feedback.
>>>> ---
>>>>  .../devicetree/bindings/mmc/sdhci-msm.yaml    | 61 ++++++++++++-------
>>>>  1 file changed, 38 insertions(+), 23 deletions(-)
>>>
>>> In the ${SUBJECT} I'm not sure what a "perp variant" is. Is that a
>>> typo or just a phrase I'm not aware of?
>>
>> Should be:
>> "per variants"
>>
>>>
>>>
>>>> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml b/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
>>>> index fc6e5221985a..2f0fdd65e908 100644
>>>> --- a/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
>>>> +++ b/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
>>>> @@ -49,33 +49,11 @@ properties:
>>>>
>>>>    reg:
>>>>      minItems: 1
>>>> -    items:
>>>> -      - description: Host controller register map
>>>> -      - description: SD Core register map
>>>> -      - description: CQE register map
>>>> -      - description: Inline Crypto Engine register map
>>>> +    maxItems: 4
>>>>
>>>>    reg-names:
>>>>      minItems: 1
>>>>      maxItems: 4
>>>> -    oneOf:
>>>> -      - items:
>>>> -          - const: hc
>>>> -      - items:
>>>> -          - const: hc
>>>> -          - const: core
>>>> -      - items:
>>>> -          - const: hc
>>>> -          - const: cqhci
>>>> -      - items:
>>>> -          - const: hc
>>>> -          - const: cqhci
>>>> -          - const: ice
>>>> -      - items:
>>>> -          - const: hc
>>>> -          - const: core
>>>> -          - const: cqhci
>>>> -          - const: ice
>>>>
>>>>    clocks:
>>>>      minItems: 3
>>>> @@ -177,6 +155,43 @@ required:
>>>>  allOf:
>>>>    - $ref: mmc-controller.yaml#
>>>>
>>>> +  - if:
>>>> +      properties:
>>>> +        compatible:
>>>> +          contains:
>>>> +            enum:
>>>> +              - qcom,sdhci-msm-v4
>>>> +    then:
>>>> +      properties:
>>>> +        reg:
>>>> +          minItems: 2
>>>> +          items:
>>>> +            - description: Host controller register map
>>>> +            - description: SD Core register map
>>>> +            - description: CQE register map
>>>> +            - description: Inline Crypto Engine register map
>>>> +        reg-names:
>>>> +          minItems: 2
>>>> +          items:
>>>> +            - const: hc
>>>> +            - const: core
>>>> +            - const: cqhci
>>>> +            - const: ice
>>>> +    else:
>>>> +      properties:
>>>> +        reg:
>>>> +          minItems: 1
>>>> +          items:
>>>> +            - description: Host controller register map
>>>> +            - description: CQE register map
>>>> +            - description: Inline Crypto Engine register map
>>>> +        reg-names:
>>>> +          minItems: 1
>>>> +          items:
>>>> +            - const: hc
>>>> +            - const: cqhci
>>>> +            - const: ice
>>>
>>> Do you need to set "maxItems" here? If you don't then will it inherit
>>> the maxItems of 4 from above?
>>
>> No, items determine the size instead.
> 
> Can you just remove the "maxItems" from above then? Does it buy us anything?

There is no maxItems directly here...

> 
> In any case, with the ${SUBJECT} fixed:
> 
> Reviewed-by: Douglas Anderson <dianders@chromium.org>


Best regards,
Krzysztof

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

* Re: [PATCH v2 2/5] dt-bindings: mmc: sdhci-msm: constrain reg-names perp variants
  2022-07-12  7:02         ` Krzysztof Kozlowski
@ 2022-07-12 14:29           ` Doug Anderson
  2022-07-12 14:38             ` Krzysztof Kozlowski
  0 siblings, 1 reply; 15+ messages in thread
From: Doug Anderson @ 2022-07-12 14:29 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Ulf Hansson, Rob Herring, Krzysztof Kozlowski, Andy Gross,
	Bjorn Andersson, Konrad Dybcio, Bhupesh Sharma, Linux MMC List,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
	linux-arm-msm

Hi,

On Tue, Jul 12, 2022 at 12:02 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 11/07/2022 17:11, Doug Anderson wrote:
> > Hi,
> >
> > On Mon, Jul 11, 2022 at 7:53 AM Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> wrote:
> >>
> >> On 11/07/2022 16:52, Doug Anderson wrote:
> >>> Hi
> >>>
> >>> On Mon, Jul 11, 2022 at 1:29 AM Krzysztof Kozlowski
> >>> <krzysztof.kozlowski@linaro.org> wrote:
> >>>>
> >>>> The entries in arrays must have fixed order, so the bindings and Linux
> >>>> driver expecting various combinations of 'reg' addresses was never
> >>>> actually conforming to guidelines.
> >>>>
> >>>> The 'core' reg entry is valid only for SDCC v4 and lower, so disallow it
> >>>> in SDCC v5.  SDCC v4 supports CQE and ICE, so allow them, even though
> >>>> the qcom,sdhci-msm-v4 compatible is used also for earlier SoCs with SDCC
> >>>> v2 or v3, so it is not entirely accurate.
> >>>>
> >>>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> >>>>
> >>>> ---
> >>>>
> >>>> Changes since v1:
> >>>> 1. Rework the patch based on Doug's feedback.
> >>>> ---
> >>>>  .../devicetree/bindings/mmc/sdhci-msm.yaml    | 61 ++++++++++++-------
> >>>>  1 file changed, 38 insertions(+), 23 deletions(-)
> >>>
> >>> In the ${SUBJECT} I'm not sure what a "perp variant" is. Is that a
> >>> typo or just a phrase I'm not aware of?
> >>
> >> Should be:
> >> "per variants"
> >>
> >>>
> >>>
> >>>> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml b/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
> >>>> index fc6e5221985a..2f0fdd65e908 100644
> >>>> --- a/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
> >>>> +++ b/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
> >>>> @@ -49,33 +49,11 @@ properties:
> >>>>
> >>>>    reg:
> >>>>      minItems: 1
> >>>> -    items:
> >>>> -      - description: Host controller register map
> >>>> -      - description: SD Core register map
> >>>> -      - description: CQE register map
> >>>> -      - description: Inline Crypto Engine register map
> >>>> +    maxItems: 4
> >>>>
> >>>>    reg-names:
> >>>>      minItems: 1
> >>>>      maxItems: 4
> >>>> -    oneOf:
> >>>> -      - items:
> >>>> -          - const: hc
> >>>> -      - items:
> >>>> -          - const: hc
> >>>> -          - const: core
> >>>> -      - items:
> >>>> -          - const: hc
> >>>> -          - const: cqhci
> >>>> -      - items:
> >>>> -          - const: hc
> >>>> -          - const: cqhci
> >>>> -          - const: ice
> >>>> -      - items:
> >>>> -          - const: hc
> >>>> -          - const: core
> >>>> -          - const: cqhci
> >>>> -          - const: ice
> >>>>
> >>>>    clocks:
> >>>>      minItems: 3
> >>>> @@ -177,6 +155,43 @@ required:
> >>>>  allOf:
> >>>>    - $ref: mmc-controller.yaml#
> >>>>
> >>>> +  - if:
> >>>> +      properties:
> >>>> +        compatible:
> >>>> +          contains:
> >>>> +            enum:
> >>>> +              - qcom,sdhci-msm-v4
> >>>> +    then:
> >>>> +      properties:
> >>>> +        reg:
> >>>> +          minItems: 2
> >>>> +          items:
> >>>> +            - description: Host controller register map
> >>>> +            - description: SD Core register map
> >>>> +            - description: CQE register map
> >>>> +            - description: Inline Crypto Engine register map
> >>>> +        reg-names:
> >>>> +          minItems: 2
> >>>> +          items:
> >>>> +            - const: hc
> >>>> +            - const: core
> >>>> +            - const: cqhci
> >>>> +            - const: ice
> >>>> +    else:
> >>>> +      properties:
> >>>> +        reg:
> >>>> +          minItems: 1
> >>>> +          items:
> >>>> +            - description: Host controller register map
> >>>> +            - description: CQE register map
> >>>> +            - description: Inline Crypto Engine register map
> >>>> +        reg-names:
> >>>> +          minItems: 1
> >>>> +          items:
> >>>> +            - const: hc
> >>>> +            - const: cqhci
> >>>> +            - const: ice
> >>>
> >>> Do you need to set "maxItems" here? If you don't then will it inherit
> >>> the maxItems of 4 from above?
> >>
> >> No, items determine the size instead.
> >
> > Can you just remove the "maxItems" from above then? Does it buy us anything?
>
> There is no maxItems directly here...

Sorry, I mean above in the schema. After your patch the schema is effectively:

reg:
  minItems: 1
  maxItems: 4
reg-names:
  minItems: 1
  maxItems: 4

...

allOf:
  - if:
      blah-blah-blah
    then:
      properties:
        reg:
          minItems: 2
          items:
            - description: ...
            - description: ...
            - description: ...
            - description: ...
        reg-names:
          blah-blah-blah
    else:
      blah-blah-blah

I'm asking about the maxItems _above_, AKA in the section:

reg:
  minItems: 1
  maxItems: 4
reg-names:
  minItems: 1
  maxItems: 4

Can we remove the "maxItems: 4" from the above and have it just be:

reg:
  minItems: 1
reg-names:
  minItems: 1

-Doug

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

* Re: [PATCH v2 2/5] dt-bindings: mmc: sdhci-msm: constrain reg-names perp variants
  2022-07-12 14:29           ` Doug Anderson
@ 2022-07-12 14:38             ` Krzysztof Kozlowski
  0 siblings, 0 replies; 15+ messages in thread
From: Krzysztof Kozlowski @ 2022-07-12 14:38 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Ulf Hansson, Rob Herring, Krzysztof Kozlowski, Andy Gross,
	Bjorn Andersson, Konrad Dybcio, Bhupesh Sharma, Linux MMC List,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
	linux-arm-msm

On 12/07/2022 16:29, Doug Anderson wrote:
> Hi,
> 
> On Tue, Jul 12, 2022 at 12:02 AM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 11/07/2022 17:11, Doug Anderson wrote:
>>> Hi,
>>>
>>> On Mon, Jul 11, 2022 at 7:53 AM Krzysztof Kozlowski
>>> <krzysztof.kozlowski@linaro.org> wrote:
>>>>
>>>> On 11/07/2022 16:52, Doug Anderson wrote:
>>>>> Hi
>>>>>
>>>>> On Mon, Jul 11, 2022 at 1:29 AM Krzysztof Kozlowski
>>>>> <krzysztof.kozlowski@linaro.org> wrote:
>>>>>>
>>>>>> The entries in arrays must have fixed order, so the bindings and Linux
>>>>>> driver expecting various combinations of 'reg' addresses was never
>>>>>> actually conforming to guidelines.
>>>>>>
>>>>>> The 'core' reg entry is valid only for SDCC v4 and lower, so disallow it
>>>>>> in SDCC v5.  SDCC v4 supports CQE and ICE, so allow them, even though
>>>>>> the qcom,sdhci-msm-v4 compatible is used also for earlier SoCs with SDCC
>>>>>> v2 or v3, so it is not entirely accurate.
>>>>>>
>>>>>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>>>>
>>>>>> ---
>>>>>>
>>>>>> Changes since v1:
>>>>>> 1. Rework the patch based on Doug's feedback.
>>>>>> ---
>>>>>>  .../devicetree/bindings/mmc/sdhci-msm.yaml    | 61 ++++++++++++-------
>>>>>>  1 file changed, 38 insertions(+), 23 deletions(-)
>>>>>
>>>>> In the ${SUBJECT} I'm not sure what a "perp variant" is. Is that a
>>>>> typo or just a phrase I'm not aware of?
>>>>
>>>> Should be:
>>>> "per variants"
>>>>
>>>>>
>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml b/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
>>>>>> index fc6e5221985a..2f0fdd65e908 100644
>>>>>> --- a/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
>>>>>> +++ b/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
>>>>>> @@ -49,33 +49,11 @@ properties:
>>>>>>
>>>>>>    reg:
>>>>>>      minItems: 1
>>>>>> -    items:
>>>>>> -      - description: Host controller register map
>>>>>> -      - description: SD Core register map
>>>>>> -      - description: CQE register map
>>>>>> -      - description: Inline Crypto Engine register map
>>>>>> +    maxItems: 4
>>>>>>
>>>>>>    reg-names:
>>>>>>      minItems: 1
>>>>>>      maxItems: 4
>>>>>> -    oneOf:
>>>>>> -      - items:
>>>>>> -          - const: hc
>>>>>> -      - items:
>>>>>> -          - const: hc
>>>>>> -          - const: core
>>>>>> -      - items:
>>>>>> -          - const: hc
>>>>>> -          - const: cqhci
>>>>>> -      - items:
>>>>>> -          - const: hc
>>>>>> -          - const: cqhci
>>>>>> -          - const: ice
>>>>>> -      - items:
>>>>>> -          - const: hc
>>>>>> -          - const: core
>>>>>> -          - const: cqhci
>>>>>> -          - const: ice
>>>>>>
>>>>>>    clocks:
>>>>>>      minItems: 3
>>>>>> @@ -177,6 +155,43 @@ required:
>>>>>>  allOf:
>>>>>>    - $ref: mmc-controller.yaml#
>>>>>>
>>>>>> +  - if:
>>>>>> +      properties:
>>>>>> +        compatible:
>>>>>> +          contains:
>>>>>> +            enum:
>>>>>> +              - qcom,sdhci-msm-v4
>>>>>> +    then:
>>>>>> +      properties:
>>>>>> +        reg:
>>>>>> +          minItems: 2
>>>>>> +          items:
>>>>>> +            - description: Host controller register map
>>>>>> +            - description: SD Core register map
>>>>>> +            - description: CQE register map
>>>>>> +            - description: Inline Crypto Engine register map
>>>>>> +        reg-names:
>>>>>> +          minItems: 2
>>>>>> +          items:
>>>>>> +            - const: hc
>>>>>> +            - const: core
>>>>>> +            - const: cqhci
>>>>>> +            - const: ice
>>>>>> +    else:
>>>>>> +      properties:
>>>>>> +        reg:
>>>>>> +          minItems: 1
>>>>>> +          items:
>>>>>> +            - description: Host controller register map
>>>>>> +            - description: CQE register map
>>>>>> +            - description: Inline Crypto Engine register map
>>>>>> +        reg-names:
>>>>>> +          minItems: 1
>>>>>> +          items:
>>>>>> +            - const: hc
>>>>>> +            - const: cqhci
>>>>>> +            - const: ice
>>>>>
>>>>> Do you need to set "maxItems" here? If you don't then will it inherit
>>>>> the maxItems of 4 from above?
>>>>
>>>> No, items determine the size instead.
>>>
>>> Can you just remove the "maxItems" from above then? Does it buy us anything?
>>
>> There is no maxItems directly here...
> 
> Sorry, I mean above in the schema. After your patch the schema is effectively:
> 
> reg:
>   minItems: 1
>   maxItems: 4
> reg-names:
>   minItems: 1
>   maxItems: 4
> 
> ...
> 
> allOf:
>   - if:
>       blah-blah-blah
>     then:
>       properties:
>         reg:
>           minItems: 2
>           items:
>             - description: ...
>             - description: ...
>             - description: ...
>             - description: ...
>         reg-names:
>           blah-blah-blah
>     else:
>       blah-blah-blah
> 
> I'm asking about the maxItems _above_, AKA in the section:
> 
> reg:
>   minItems: 1
>   maxItems: 4
> reg-names:
>   minItems: 1
>   maxItems: 4
> 
> Can we remove the "maxItems: 4" from the above and have it just be:
> 
> reg:
>   minItems: 1
> reg-names:
>   minItems: 1
> 


Yes, we can, but preferred is to have it because it makes the broad
constraints easily visible. You don't need to check each if:else branch
to find upper bounds or check if maxItems are defined at all. This also
matches pattern used in bindings without allOf:if:then - each time you
are expected to see array types constraint in the list of properties.

Best regards,
Krzysztof

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

end of thread, other threads:[~2022-07-12 14:38 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-07-11  8:29 [PATCH v2 0/5] dt-bindings: mmc: / ARM: qcom: correct reg-names and clock entries Krzysztof Kozlowski
2022-07-11  8:29 ` [PATCH v2 1/5] dt-bindings: mmc: sdhci-msm: fix reg-names entries Krzysztof Kozlowski
2022-07-11  8:29 ` [PATCH v2 2/5] dt-bindings: mmc: sdhci-msm: constrain reg-names perp variants Krzysztof Kozlowski
2022-07-11 14:52   ` Doug Anderson
2022-07-11 14:53     ` Krzysztof Kozlowski
2022-07-11 15:11       ` Doug Anderson
2022-07-12  7:02         ` Krzysztof Kozlowski
2022-07-12 14:29           ` Doug Anderson
2022-07-12 14:38             ` Krzysztof Kozlowski
2022-07-11  8:29 ` [PATCH v2 3/5] arm64: dts: qcom: align SDHCI reg-names with DT schema Krzysztof Kozlowski
2022-07-11  8:42   ` Konrad Dybcio
2022-07-11  8:29 ` [PATCH v2 4/5] ARM: " Krzysztof Kozlowski
2022-07-11  8:42   ` Konrad Dybcio
2022-07-11  8:29 ` [PATCH v2 5/5] ARM: dts: qcom: align SDHCI clocks " Krzysztof Kozlowski
2022-07-11  8:43   ` Konrad Dybcio

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).