All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/5] dt-bindings: mmc: imx-esdhc: Update compatible fallbacks
@ 2022-03-27 12:38 ` Adam Ford
  0 siblings, 0 replies; 44+ messages in thread
From: Adam Ford @ 2022-03-27 12:38 UTC (permalink / raw)
  To: linux-mmc
  Cc: haibo.chen, aford, Adam Ford, Ulf Hansson, Rob Herring,
	Krzysztof Kozlowski, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
	devicetree, linux-arm-kernel, linux-kernel

The SDHC controller in the imx8mn and imx8mp have the same controller
as the imx8mm which is slightly different than that of the imx7d.
Using the fallback of the imx8mm enables the controllers to support
HS400-ES which is not available on the imx7d. After discussion with NXP,
it turns out that the imx8qm should fall back to the imx8qxp, because
those have some additional flags not present in the imx8mm.

Suggested-by: haibo.chen@nxp.com
Signed-off-by: Adam Ford <aford173@gmail.com>
---
V2:  Added suggested-by note and imx8qxp updates.
---
 .../devicetree/bindings/mmc/fsl-imx-esdhc.yaml   | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
index 7dbbcae9485c..1427e9b5a6ec 100644
--- a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
+++ b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
@@ -34,23 +34,25 @@ properties:
           - fsl,imx6ull-usdhc
           - fsl,imx7d-usdhc
           - fsl,imx7ulp-usdhc
+          - fsl,imx8mm-usdhc
+          - fsl,imx8qxp-usdhc
           - fsl,imxrt1050-usdhc
           - nxp,s32g2-usdhc
       - items:
           - enum:
-              - fsl,imx8mm-usdhc
-              - fsl,imx8mn-usdhc
-              - fsl,imx8mp-usdhc
               - fsl,imx8mq-usdhc
-              - fsl,imx8qm-usdhc
-              - fsl,imx8qxp-usdhc
           - const: fsl,imx7d-usdhc
       - items:
           - enum:
-              - fsl,imx93-usdhc
+              - fsl,imx8mn-usdhc
+              - fsl,imx8mp-usdhc
               - fsl,imx8ulp-usdhc
+              - fsl,imx93-usdhc
           - const: fsl,imx8mm-usdhc
-
+      - items:
+          - enum:
+              - fsl,imx8qm-usdhc
+          - const: fsl,imx8qxp-usdhc
   reg:
     maxItems: 1
 
-- 
2.34.1


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

* [PATCH 1/5] dt-bindings: mmc: imx-esdhc: Update compatible fallbacks
@ 2022-03-27 12:38 ` Adam Ford
  0 siblings, 0 replies; 44+ messages in thread
From: Adam Ford @ 2022-03-27 12:38 UTC (permalink / raw)
  To: linux-mmc
  Cc: haibo.chen, aford, Adam Ford, Ulf Hansson, Rob Herring,
	Krzysztof Kozlowski, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
	devicetree, linux-arm-kernel, linux-kernel

The SDHC controller in the imx8mn and imx8mp have the same controller
as the imx8mm which is slightly different than that of the imx7d.
Using the fallback of the imx8mm enables the controllers to support
HS400-ES which is not available on the imx7d. After discussion with NXP,
it turns out that the imx8qm should fall back to the imx8qxp, because
those have some additional flags not present in the imx8mm.

Suggested-by: haibo.chen@nxp.com
Signed-off-by: Adam Ford <aford173@gmail.com>
---
V2:  Added suggested-by note and imx8qxp updates.
---
 .../devicetree/bindings/mmc/fsl-imx-esdhc.yaml   | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
index 7dbbcae9485c..1427e9b5a6ec 100644
--- a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
+++ b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
@@ -34,23 +34,25 @@ properties:
           - fsl,imx6ull-usdhc
           - fsl,imx7d-usdhc
           - fsl,imx7ulp-usdhc
+          - fsl,imx8mm-usdhc
+          - fsl,imx8qxp-usdhc
           - fsl,imxrt1050-usdhc
           - nxp,s32g2-usdhc
       - items:
           - enum:
-              - fsl,imx8mm-usdhc
-              - fsl,imx8mn-usdhc
-              - fsl,imx8mp-usdhc
               - fsl,imx8mq-usdhc
-              - fsl,imx8qm-usdhc
-              - fsl,imx8qxp-usdhc
           - const: fsl,imx7d-usdhc
       - items:
           - enum:
-              - fsl,imx93-usdhc
+              - fsl,imx8mn-usdhc
+              - fsl,imx8mp-usdhc
               - fsl,imx8ulp-usdhc
+              - fsl,imx93-usdhc
           - const: fsl,imx8mm-usdhc
-
+      - items:
+          - enum:
+              - fsl,imx8qm-usdhc
+          - const: fsl,imx8qxp-usdhc
   reg:
     maxItems: 1
 
-- 
2.34.1


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

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

* [PATCH 2/5] arm64: dts: imx8mn: Enable HS400-ES
  2022-03-27 12:38 ` Adam Ford
@ 2022-03-27 12:38   ` Adam Ford
  -1 siblings, 0 replies; 44+ messages in thread
From: Adam Ford @ 2022-03-27 12:38 UTC (permalink / raw)
  To: linux-mmc
  Cc: haibo.chen, aford, Adam Ford, Ulf Hansson, Rob Herring,
	Krzysztof Kozlowski, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
	devicetree, linux-arm-kernel, linux-kernel

The SDHC controller in the imx8mn has the same controller
as the imx8mm which supports HS400-ES. Change the compatible
fallback to imx8mm to enable it.

Signed-off-by: Adam Ford <aford173@gmail.com>
---
 arch/arm64/boot/dts/freescale/imx8mn.dtsi | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mn.dtsi b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
index 99f0f5026674..959285c3fee0 100644
--- a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
@@ -933,7 +933,7 @@ mu: mailbox@30aa0000 {
 			};
 
 			usdhc1: mmc@30b40000 {
-				compatible = "fsl,imx8mn-usdhc", "fsl,imx7d-usdhc";
+				compatible = "fsl,imx8mn-usdhc", "fsl,imx8mm-usdhc";
 				reg = <0x30b40000 0x10000>;
 				interrupts = <GIC_SPI 22 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clk IMX8MN_CLK_IPG_ROOT>,
@@ -947,7 +947,7 @@ usdhc1: mmc@30b40000 {
 			};
 
 			usdhc2: mmc@30b50000 {
-				compatible = "fsl,imx8mn-usdhc", "fsl,imx7d-usdhc";
+				compatible = "fsl,imx8mn-usdhc", "fsl,imx8mm-usdhc";
 				reg = <0x30b50000 0x10000>;
 				interrupts = <GIC_SPI 23 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clk IMX8MN_CLK_IPG_ROOT>,
@@ -961,7 +961,7 @@ usdhc2: mmc@30b50000 {
 			};
 
 			usdhc3: mmc@30b60000 {
-				compatible = "fsl,imx8mn-usdhc", "fsl,imx7d-usdhc";
+				compatible = "fsl,imx8mn-usdhc", "fsl,imx8mm-usdhc";
 				reg = <0x30b60000 0x10000>;
 				interrupts = <GIC_SPI 24 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clk IMX8MN_CLK_IPG_ROOT>,
-- 
2.34.1


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

* [PATCH 2/5] arm64: dts: imx8mn: Enable HS400-ES
@ 2022-03-27 12:38   ` Adam Ford
  0 siblings, 0 replies; 44+ messages in thread
From: Adam Ford @ 2022-03-27 12:38 UTC (permalink / raw)
  To: linux-mmc
  Cc: haibo.chen, aford, Adam Ford, Ulf Hansson, Rob Herring,
	Krzysztof Kozlowski, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
	devicetree, linux-arm-kernel, linux-kernel

The SDHC controller in the imx8mn has the same controller
as the imx8mm which supports HS400-ES. Change the compatible
fallback to imx8mm to enable it.

Signed-off-by: Adam Ford <aford173@gmail.com>
---
 arch/arm64/boot/dts/freescale/imx8mn.dtsi | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mn.dtsi b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
index 99f0f5026674..959285c3fee0 100644
--- a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
@@ -933,7 +933,7 @@ mu: mailbox@30aa0000 {
 			};
 
 			usdhc1: mmc@30b40000 {
-				compatible = "fsl,imx8mn-usdhc", "fsl,imx7d-usdhc";
+				compatible = "fsl,imx8mn-usdhc", "fsl,imx8mm-usdhc";
 				reg = <0x30b40000 0x10000>;
 				interrupts = <GIC_SPI 22 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clk IMX8MN_CLK_IPG_ROOT>,
@@ -947,7 +947,7 @@ usdhc1: mmc@30b40000 {
 			};
 
 			usdhc2: mmc@30b50000 {
-				compatible = "fsl,imx8mn-usdhc", "fsl,imx7d-usdhc";
+				compatible = "fsl,imx8mn-usdhc", "fsl,imx8mm-usdhc";
 				reg = <0x30b50000 0x10000>;
 				interrupts = <GIC_SPI 23 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clk IMX8MN_CLK_IPG_ROOT>,
@@ -961,7 +961,7 @@ usdhc2: mmc@30b50000 {
 			};
 
 			usdhc3: mmc@30b60000 {
-				compatible = "fsl,imx8mn-usdhc", "fsl,imx7d-usdhc";
+				compatible = "fsl,imx8mn-usdhc", "fsl,imx8mm-usdhc";
 				reg = <0x30b60000 0x10000>;
 				interrupts = <GIC_SPI 24 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clk IMX8MN_CLK_IPG_ROOT>,
-- 
2.34.1


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

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

* [PATCH 3/5] arm64: dts: imx8mp: Enable HS400-ES
  2022-03-27 12:38 ` Adam Ford
@ 2022-03-27 12:38   ` Adam Ford
  -1 siblings, 0 replies; 44+ messages in thread
From: Adam Ford @ 2022-03-27 12:38 UTC (permalink / raw)
  To: linux-mmc
  Cc: haibo.chen, aford, Adam Ford, Ulf Hansson, Rob Herring,
	Krzysztof Kozlowski, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
	devicetree, linux-arm-kernel, linux-kernel

The SDHC controller in the imx8mp has the same controller
as the imx8mm which supports HS400-ES. Change the compatible
fallback to imx8mm to enable it.

Signed-off-by: Adam Ford <aford173@gmail.com>
---
 arch/arm64/boot/dts/freescale/imx8mp.dtsi | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
index 794d75173cf5..d5ee1520f1fe 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
@@ -769,7 +769,7 @@ i2c6: i2c@30ae0000 {
 			};
 
 			usdhc1: mmc@30b40000 {
-				compatible = "fsl,imx8mp-usdhc", "fsl,imx7d-usdhc";
+				compatible = "fsl,imx8mp-usdhc", "fsl,imx8mm-usdhc";
 				reg = <0x30b40000 0x10000>;
 				interrupts = <GIC_SPI 22 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clk IMX8MP_CLK_DUMMY>,
@@ -783,7 +783,7 @@ usdhc1: mmc@30b40000 {
 			};
 
 			usdhc2: mmc@30b50000 {
-				compatible = "fsl,imx8mp-usdhc", "fsl,imx7d-usdhc";
+				compatible = "fsl,imx8mp-usdhc", "fsl,imx8mm-usdhc";
 				reg = <0x30b50000 0x10000>;
 				interrupts = <GIC_SPI 23 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clk IMX8MP_CLK_DUMMY>,
@@ -797,7 +797,7 @@ usdhc2: mmc@30b50000 {
 			};
 
 			usdhc3: mmc@30b60000 {
-				compatible = "fsl,imx8mp-usdhc", "fsl,imx7d-usdhc";
+				compatible = "fsl,imx8mp-usdhc", "fsl,imx8mm-usdhc";
 				reg = <0x30b60000 0x10000>;
 				interrupts = <GIC_SPI 24 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clk IMX8MP_CLK_DUMMY>,
-- 
2.34.1


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

* [PATCH 3/5] arm64: dts: imx8mp: Enable HS400-ES
@ 2022-03-27 12:38   ` Adam Ford
  0 siblings, 0 replies; 44+ messages in thread
From: Adam Ford @ 2022-03-27 12:38 UTC (permalink / raw)
  To: linux-mmc
  Cc: haibo.chen, aford, Adam Ford, Ulf Hansson, Rob Herring,
	Krzysztof Kozlowski, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
	devicetree, linux-arm-kernel, linux-kernel

The SDHC controller in the imx8mp has the same controller
as the imx8mm which supports HS400-ES. Change the compatible
fallback to imx8mm to enable it.

Signed-off-by: Adam Ford <aford173@gmail.com>
---
 arch/arm64/boot/dts/freescale/imx8mp.dtsi | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
index 794d75173cf5..d5ee1520f1fe 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
@@ -769,7 +769,7 @@ i2c6: i2c@30ae0000 {
 			};
 
 			usdhc1: mmc@30b40000 {
-				compatible = "fsl,imx8mp-usdhc", "fsl,imx7d-usdhc";
+				compatible = "fsl,imx8mp-usdhc", "fsl,imx8mm-usdhc";
 				reg = <0x30b40000 0x10000>;
 				interrupts = <GIC_SPI 22 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clk IMX8MP_CLK_DUMMY>,
@@ -783,7 +783,7 @@ usdhc1: mmc@30b40000 {
 			};
 
 			usdhc2: mmc@30b50000 {
-				compatible = "fsl,imx8mp-usdhc", "fsl,imx7d-usdhc";
+				compatible = "fsl,imx8mp-usdhc", "fsl,imx8mm-usdhc";
 				reg = <0x30b50000 0x10000>;
 				interrupts = <GIC_SPI 23 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clk IMX8MP_CLK_DUMMY>,
@@ -797,7 +797,7 @@ usdhc2: mmc@30b50000 {
 			};
 
 			usdhc3: mmc@30b60000 {
-				compatible = "fsl,imx8mp-usdhc", "fsl,imx7d-usdhc";
+				compatible = "fsl,imx8mp-usdhc", "fsl,imx8mm-usdhc";
 				reg = <0x30b60000 0x10000>;
 				interrupts = <GIC_SPI 24 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clk IMX8MP_CLK_DUMMY>,
-- 
2.34.1


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

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

* [PATCH 4/5] arm64: dts: imx8qxp: Remove imx7d-usdhc compatible fallback
  2022-03-27 12:38 ` Adam Ford
@ 2022-03-27 12:38   ` Adam Ford
  -1 siblings, 0 replies; 44+ messages in thread
From: Adam Ford @ 2022-03-27 12:38 UTC (permalink / raw)
  To: linux-mmc
  Cc: haibo.chen, aford, Adam Ford, Ulf Hansson, Rob Herring,
	Krzysztof Kozlowski, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
	devicetree, linux-arm-kernel, linux-kernel

Since the compatible flag for fsl,imx8qxp-usdhc directly matches
in the driver, there is no need to fall back on the imx7d-usdhc.

Signed-off-by: Adam Ford <aford173@gmail.com>
---
V2:  New to series
---
 arch/arm64/boot/dts/freescale/imx8qxp-ss-conn.dtsi | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8qxp-ss-conn.dtsi b/arch/arm64/boot/dts/freescale/imx8qxp-ss-conn.dtsi
index 46da21af3702..75fc951bca25 100644
--- a/arch/arm64/boot/dts/freescale/imx8qxp-ss-conn.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8qxp-ss-conn.dtsi
@@ -5,15 +5,15 @@
  */
 
 &usdhc1 {
-	compatible = "fsl,imx8qxp-usdhc", "fsl,imx7d-usdhc";
+	compatible = "fsl,imx8qxp-usdhc";
 };
 
 &usdhc2 {
-	compatible = "fsl,imx8qxp-usdhc", "fsl,imx7d-usdhc";
+	compatible = "fsl,imx8qxp-usdhc";
 };
 
 &usdhc3 {
-	compatible = "fsl,imx8qxp-usdhc", "fsl,imx7d-usdhc";
+	compatible = "fsl,imx8qxp-usdhc";
 };
 
 &fec1 {
-- 
2.34.1


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

* [PATCH 4/5] arm64: dts: imx8qxp: Remove imx7d-usdhc compatible fallback
@ 2022-03-27 12:38   ` Adam Ford
  0 siblings, 0 replies; 44+ messages in thread
From: Adam Ford @ 2022-03-27 12:38 UTC (permalink / raw)
  To: linux-mmc
  Cc: haibo.chen, aford, Adam Ford, Ulf Hansson, Rob Herring,
	Krzysztof Kozlowski, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
	devicetree, linux-arm-kernel, linux-kernel

Since the compatible flag for fsl,imx8qxp-usdhc directly matches
in the driver, there is no need to fall back on the imx7d-usdhc.

Signed-off-by: Adam Ford <aford173@gmail.com>
---
V2:  New to series
---
 arch/arm64/boot/dts/freescale/imx8qxp-ss-conn.dtsi | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8qxp-ss-conn.dtsi b/arch/arm64/boot/dts/freescale/imx8qxp-ss-conn.dtsi
index 46da21af3702..75fc951bca25 100644
--- a/arch/arm64/boot/dts/freescale/imx8qxp-ss-conn.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8qxp-ss-conn.dtsi
@@ -5,15 +5,15 @@
  */
 
 &usdhc1 {
-	compatible = "fsl,imx8qxp-usdhc", "fsl,imx7d-usdhc";
+	compatible = "fsl,imx8qxp-usdhc";
 };
 
 &usdhc2 {
-	compatible = "fsl,imx8qxp-usdhc", "fsl,imx7d-usdhc";
+	compatible = "fsl,imx8qxp-usdhc";
 };
 
 &usdhc3 {
-	compatible = "fsl,imx8qxp-usdhc", "fsl,imx7d-usdhc";
+	compatible = "fsl,imx8qxp-usdhc";
 };
 
 &fec1 {
-- 
2.34.1


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

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

* [PATCH 5/5] arm64: dts: imx8qm: Remove fsl,imx7d-usdhc compatible fallback
  2022-03-27 12:38 ` Adam Ford
@ 2022-03-27 12:38   ` Adam Ford
  -1 siblings, 0 replies; 44+ messages in thread
From: Adam Ford @ 2022-03-27 12:38 UTC (permalink / raw)
  To: linux-mmc
  Cc: haibo.chen, aford, Adam Ford, Ulf Hansson, Rob Herring,
	Krzysztof Kozlowski, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
	devicetree, linux-arm-kernel, linux-kernel

The correct fallback for the imx8qm should only be fsl,imx8qxp-usdhc
because fsl,imx8qxp-usdhc is a superset which contains additional
flags not present in fsl,imx7d-usdhc.

Signed-off-by: Adam Ford <aford173@gmail.com>
---
 arch/arm64/boot/dts/freescale/imx8qm-ss-conn.dtsi | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8qm-ss-conn.dtsi b/arch/arm64/boot/dts/freescale/imx8qm-ss-conn.dtsi
index ec1639174e2e..336f0ea7e7b5 100644
--- a/arch/arm64/boot/dts/freescale/imx8qm-ss-conn.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8qm-ss-conn.dtsi
@@ -13,13 +13,13 @@ &fec2 {
 };
 
 &usdhc1 {
-	compatible = "fsl,imx8qm-usdhc", "fsl,imx8qxp-usdhc", "fsl,imx7d-usdhc";
+	compatible = "fsl,imx8qm-usdhc", "fsl,imx8qxp-usdhc";
 };
 
 &usdhc2 {
-	compatible = "fsl,imx8qm-usdhc", "fsl,imx8qxp-usdhc", "fsl,imx7d-usdhc";
+	compatible = "fsl,imx8qm-usdhc", "fsl,imx8qxp-usdhc";
 };
 
 &usdhc3 {
-	compatible = "fsl,imx8qm-usdhc", "fsl,imx8qxp-usdhc", "fsl,imx7d-usdhc";
+	compatible = "fsl,imx8qm-usdhc", "fsl,imx8qxp-usdhc";
 };
-- 
2.34.1


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

* [PATCH 5/5] arm64: dts: imx8qm: Remove fsl, imx7d-usdhc compatible fallback
@ 2022-03-27 12:38   ` Adam Ford
  0 siblings, 0 replies; 44+ messages in thread
From: Adam Ford @ 2022-03-27 12:38 UTC (permalink / raw)
  To: linux-mmc
  Cc: haibo.chen, aford, Adam Ford, Ulf Hansson, Rob Herring,
	Krzysztof Kozlowski, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
	devicetree, linux-arm-kernel, linux-kernel

The correct fallback for the imx8qm should only be fsl,imx8qxp-usdhc
because fsl,imx8qxp-usdhc is a superset which contains additional
flags not present in fsl,imx7d-usdhc.

Signed-off-by: Adam Ford <aford173@gmail.com>
---
 arch/arm64/boot/dts/freescale/imx8qm-ss-conn.dtsi | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8qm-ss-conn.dtsi b/arch/arm64/boot/dts/freescale/imx8qm-ss-conn.dtsi
index ec1639174e2e..336f0ea7e7b5 100644
--- a/arch/arm64/boot/dts/freescale/imx8qm-ss-conn.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8qm-ss-conn.dtsi
@@ -13,13 +13,13 @@ &fec2 {
 };
 
 &usdhc1 {
-	compatible = "fsl,imx8qm-usdhc", "fsl,imx8qxp-usdhc", "fsl,imx7d-usdhc";
+	compatible = "fsl,imx8qm-usdhc", "fsl,imx8qxp-usdhc";
 };
 
 &usdhc2 {
-	compatible = "fsl,imx8qm-usdhc", "fsl,imx8qxp-usdhc", "fsl,imx7d-usdhc";
+	compatible = "fsl,imx8qm-usdhc", "fsl,imx8qxp-usdhc";
 };
 
 &usdhc3 {
-	compatible = "fsl,imx8qm-usdhc", "fsl,imx8qxp-usdhc", "fsl,imx7d-usdhc";
+	compatible = "fsl,imx8qm-usdhc", "fsl,imx8qxp-usdhc";
 };
-- 
2.34.1


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

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

* Re: [PATCH 1/5] dt-bindings: mmc: imx-esdhc: Update compatible fallbacks
  2022-03-27 12:38 ` Adam Ford
@ 2022-03-27 19:26   ` Krzysztof Kozlowski
  -1 siblings, 0 replies; 44+ messages in thread
From: Krzysztof Kozlowski @ 2022-03-27 19:26 UTC (permalink / raw)
  To: Adam Ford, linux-mmc
  Cc: haibo.chen, aford, Ulf Hansson, Rob Herring, Krzysztof Kozlowski,
	Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	NXP Linux Team, devicetree, linux-arm-kernel, linux-kernel

On 27/03/2022 14:38, Adam Ford wrote:
> The SDHC controller in the imx8mn and imx8mp have the same controller
> as the imx8mm which is slightly different than that of the imx7d.
> Using the fallback of the imx8mm enables the controllers to support
> HS400-ES which is not available on the imx7d. After discussion with NXP,
> it turns out that the imx8qm should fall back to the imx8qxp, because
> those have some additional flags not present in the imx8mm.
> 
> Suggested-by: haibo.chen@nxp.com
> Signed-off-by: Adam Ford <aford173@gmail.com>
> ---
> V2:  Added suggested-by note and imx8qxp updates.
> ---
>  .../devicetree/bindings/mmc/fsl-imx-esdhc.yaml   | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 

YAML and logic looks good, although I did not check the actual hardware
properties.
Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>


Best regards,
Krzysztof

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

* Re: [PATCH 1/5] dt-bindings: mmc: imx-esdhc: Update compatible fallbacks
@ 2022-03-27 19:26   ` Krzysztof Kozlowski
  0 siblings, 0 replies; 44+ messages in thread
From: Krzysztof Kozlowski @ 2022-03-27 19:26 UTC (permalink / raw)
  To: Adam Ford, linux-mmc
  Cc: haibo.chen, aford, Ulf Hansson, Rob Herring, Krzysztof Kozlowski,
	Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	NXP Linux Team, devicetree, linux-arm-kernel, linux-kernel

On 27/03/2022 14:38, Adam Ford wrote:
> The SDHC controller in the imx8mn and imx8mp have the same controller
> as the imx8mm which is slightly different than that of the imx7d.
> Using the fallback of the imx8mm enables the controllers to support
> HS400-ES which is not available on the imx7d. After discussion with NXP,
> it turns out that the imx8qm should fall back to the imx8qxp, because
> those have some additional flags not present in the imx8mm.
> 
> Suggested-by: haibo.chen@nxp.com
> Signed-off-by: Adam Ford <aford173@gmail.com>
> ---
> V2:  Added suggested-by note and imx8qxp updates.
> ---
>  .../devicetree/bindings/mmc/fsl-imx-esdhc.yaml   | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 

YAML and logic looks good, although I did not check the actual hardware
properties.
Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>


Best regards,
Krzysztof

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

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

* Re: [PATCH 3/5] arm64: dts: imx8mp: Enable HS400-ES
  2022-03-27 12:38   ` Adam Ford
@ 2022-03-28  7:20     ` Ahmad Fatoum
  -1 siblings, 0 replies; 44+ messages in thread
From: Ahmad Fatoum @ 2022-03-28  7:20 UTC (permalink / raw)
  To: Adam Ford, linux-mmc
  Cc: devicetree, Ulf Hansson, Fabio Estevam, Shawn Guo, Sascha Hauer,
	aford, haibo.chen, linux-kernel, Rob Herring, NXP Linux Team,
	Pengutronix Kernel Team, Krzysztof Kozlowski, linux-arm-kernel

Hello Adam,

On 27.03.22 14:38, Adam Ford wrote:
> The SDHC controller in the imx8mp has the same controller
> as the imx8mm which supports HS400-ES. Change the compatible
> fallback to imx8mm to enable it.

I believe that's a shortcoming of the Linux driver, which should explicitly list
fsl,imx8mp-usdhc in its compatibles and enable HS400-ES for it.

I find dropping compatibles problematic, because like Linux matching
fsl,imx8mm-usdhc, but not fsl,imx8mp-usdhc, other software may match
fsl,imx7d-usdhc, but not fsl,imx8[mp]-usdhc.

I'd prefer that either the kernel driver gains extra compatibles or that
the DTS lists extra compatibles and we refrain from dropping existing
(correct) ones.

What do you think?

Cheers,
Ahmad

> Signed-off-by: Adam Ford <aford173@gmail.com>
> ---
>  arch/arm64/boot/dts/freescale/imx8mp.dtsi | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> index 794d75173cf5..d5ee1520f1fe 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> @@ -769,7 +769,7 @@ i2c6: i2c@30ae0000 {
>  			};
>  
>  			usdhc1: mmc@30b40000 {
> -				compatible = "fsl,imx8mp-usdhc", "fsl,imx7d-usdhc";
> +				compatible = "fsl,imx8mp-usdhc", "fsl,imx8mm-usdhc";
>  				reg = <0x30b40000 0x10000>;
>  				interrupts = <GIC_SPI 22 IRQ_TYPE_LEVEL_HIGH>;
>  				clocks = <&clk IMX8MP_CLK_DUMMY>,
> @@ -783,7 +783,7 @@ usdhc1: mmc@30b40000 {
>  			};
>  
>  			usdhc2: mmc@30b50000 {
> -				compatible = "fsl,imx8mp-usdhc", "fsl,imx7d-usdhc";
> +				compatible = "fsl,imx8mp-usdhc", "fsl,imx8mm-usdhc";
>  				reg = <0x30b50000 0x10000>;
>  				interrupts = <GIC_SPI 23 IRQ_TYPE_LEVEL_HIGH>;
>  				clocks = <&clk IMX8MP_CLK_DUMMY>,
> @@ -797,7 +797,7 @@ usdhc2: mmc@30b50000 {
>  			};
>  
>  			usdhc3: mmc@30b60000 {
> -				compatible = "fsl,imx8mp-usdhc", "fsl,imx7d-usdhc";
> +				compatible = "fsl,imx8mp-usdhc", "fsl,imx8mm-usdhc";
>  				reg = <0x30b60000 0x10000>;
>  				interrupts = <GIC_SPI 24 IRQ_TYPE_LEVEL_HIGH>;
>  				clocks = <&clk IMX8MP_CLK_DUMMY>,


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

* Re: [PATCH 3/5] arm64: dts: imx8mp: Enable HS400-ES
@ 2022-03-28  7:20     ` Ahmad Fatoum
  0 siblings, 0 replies; 44+ messages in thread
From: Ahmad Fatoum @ 2022-03-28  7:20 UTC (permalink / raw)
  To: Adam Ford, linux-mmc
  Cc: devicetree, Ulf Hansson, Fabio Estevam, Shawn Guo, Sascha Hauer,
	aford, haibo.chen, linux-kernel, Rob Herring, NXP Linux Team,
	Pengutronix Kernel Team, Krzysztof Kozlowski, linux-arm-kernel

Hello Adam,

On 27.03.22 14:38, Adam Ford wrote:
> The SDHC controller in the imx8mp has the same controller
> as the imx8mm which supports HS400-ES. Change the compatible
> fallback to imx8mm to enable it.

I believe that's a shortcoming of the Linux driver, which should explicitly list
fsl,imx8mp-usdhc in its compatibles and enable HS400-ES for it.

I find dropping compatibles problematic, because like Linux matching
fsl,imx8mm-usdhc, but not fsl,imx8mp-usdhc, other software may match
fsl,imx7d-usdhc, but not fsl,imx8[mp]-usdhc.

I'd prefer that either the kernel driver gains extra compatibles or that
the DTS lists extra compatibles and we refrain from dropping existing
(correct) ones.

What do you think?

Cheers,
Ahmad

> Signed-off-by: Adam Ford <aford173@gmail.com>
> ---
>  arch/arm64/boot/dts/freescale/imx8mp.dtsi | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> index 794d75173cf5..d5ee1520f1fe 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> @@ -769,7 +769,7 @@ i2c6: i2c@30ae0000 {
>  			};
>  
>  			usdhc1: mmc@30b40000 {
> -				compatible = "fsl,imx8mp-usdhc", "fsl,imx7d-usdhc";
> +				compatible = "fsl,imx8mp-usdhc", "fsl,imx8mm-usdhc";
>  				reg = <0x30b40000 0x10000>;
>  				interrupts = <GIC_SPI 22 IRQ_TYPE_LEVEL_HIGH>;
>  				clocks = <&clk IMX8MP_CLK_DUMMY>,
> @@ -783,7 +783,7 @@ usdhc1: mmc@30b40000 {
>  			};
>  
>  			usdhc2: mmc@30b50000 {
> -				compatible = "fsl,imx8mp-usdhc", "fsl,imx7d-usdhc";
> +				compatible = "fsl,imx8mp-usdhc", "fsl,imx8mm-usdhc";
>  				reg = <0x30b50000 0x10000>;
>  				interrupts = <GIC_SPI 23 IRQ_TYPE_LEVEL_HIGH>;
>  				clocks = <&clk IMX8MP_CLK_DUMMY>,
> @@ -797,7 +797,7 @@ usdhc2: mmc@30b50000 {
>  			};
>  
>  			usdhc3: mmc@30b60000 {
> -				compatible = "fsl,imx8mp-usdhc", "fsl,imx7d-usdhc";
> +				compatible = "fsl,imx8mp-usdhc", "fsl,imx8mm-usdhc";
>  				reg = <0x30b60000 0x10000>;
>  				interrupts = <GIC_SPI 24 IRQ_TYPE_LEVEL_HIGH>;
>  				clocks = <&clk IMX8MP_CLK_DUMMY>,


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

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

* Re: [PATCH 3/5] arm64: dts: imx8mp: Enable HS400-ES
  2022-03-28  7:20     ` Ahmad Fatoum
@ 2022-03-28 10:47       ` Adam Ford
  -1 siblings, 0 replies; 44+ messages in thread
From: Adam Ford @ 2022-03-28 10:47 UTC (permalink / raw)
  To: Ahmad Fatoum
  Cc: linux-mmc, devicetree, Ulf Hansson, Fabio Estevam, Shawn Guo,
	Sascha Hauer, Adam Ford-BE, Haibo Chen,
	Linux Kernel Mailing List, Rob Herring, NXP Linux Team,
	Pengutronix Kernel Team, Krzysztof Kozlowski, arm-soc

On Mon, Mar 28, 2022 at 2:20 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>
> Hello Adam,
>
> On 27.03.22 14:38, Adam Ford wrote:
> > The SDHC controller in the imx8mp has the same controller
> > as the imx8mm which supports HS400-ES. Change the compatible
> > fallback to imx8mm to enable it.
>
> I believe that's a shortcoming of the Linux driver, which should explicitly list
> fsl,imx8mp-usdhc in its compatibles and enable HS400-ES for it.
>
> I find dropping compatibles problematic, because like Linux matching
> fsl,imx8mm-usdhc, but not fsl,imx8mp-usdhc, other software may match
> fsl,imx7d-usdhc, but not fsl,imx8[mp]-usdhc.
>
> I'd prefer that either the kernel driver gains extra compatibles or that
> the DTS lists extra compatibles and we refrain from dropping existing
> (correct) ones.
>

I would argue that imx7d is not correct since the IP blocks between
imx7d and imx8mm have different flags/quirks.  One of which includes
HS400-ES, but there are other differences as well.

> What do you think?

From my understanding of the fallback compatibility strings is to
avoid having to add more and more compatible strings to the drivers
when they do not serve a functional purpose. Based On a conversation
with Krzysztof [1], he suggested we update the YAML file based on the
fallback, but he wanted NXP to give their feedback as to what the
right fallback strings should be.  Haibo from NXP sent me a hierarchy
[1] which is what I used to update the YAML file.  Based on the YAML
file, the fallback in each DTSI file was updated to ensure the use of
the proper IP block.

adam

[1] - https://lore.kernel.org/linux-arm-kernel/CAHCN7xLWoUGi-jfxR2a0gvEFkPT3USUEb+8U3CCqCb5wWEJ8xw@mail.gmail.com/T/

>
> Cheers,
> Ahmad
>
> > Signed-off-by: Adam Ford <aford173@gmail.com>
> > ---
> >  arch/arm64/boot/dts/freescale/imx8mp.dtsi | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > index 794d75173cf5..d5ee1520f1fe 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > @@ -769,7 +769,7 @@ i2c6: i2c@30ae0000 {
> >                       };
> >
> >                       usdhc1: mmc@30b40000 {
> > -                             compatible = "fsl,imx8mp-usdhc", "fsl,imx7d-usdhc";
> > +                             compatible = "fsl,imx8mp-usdhc", "fsl,imx8mm-usdhc";
> >                               reg = <0x30b40000 0x10000>;
> >                               interrupts = <GIC_SPI 22 IRQ_TYPE_LEVEL_HIGH>;
> >                               clocks = <&clk IMX8MP_CLK_DUMMY>,
> > @@ -783,7 +783,7 @@ usdhc1: mmc@30b40000 {
> >                       };
> >
> >                       usdhc2: mmc@30b50000 {
> > -                             compatible = "fsl,imx8mp-usdhc", "fsl,imx7d-usdhc";
> > +                             compatible = "fsl,imx8mp-usdhc", "fsl,imx8mm-usdhc";
> >                               reg = <0x30b50000 0x10000>;
> >                               interrupts = <GIC_SPI 23 IRQ_TYPE_LEVEL_HIGH>;
> >                               clocks = <&clk IMX8MP_CLK_DUMMY>,
> > @@ -797,7 +797,7 @@ usdhc2: mmc@30b50000 {
> >                       };
> >
> >                       usdhc3: mmc@30b60000 {
> > -                             compatible = "fsl,imx8mp-usdhc", "fsl,imx7d-usdhc";
> > +                             compatible = "fsl,imx8mp-usdhc", "fsl,imx8mm-usdhc";
> >                               reg = <0x30b60000 0x10000>;
> >                               interrupts = <GIC_SPI 24 IRQ_TYPE_LEVEL_HIGH>;
> >                               clocks = <&clk IMX8MP_CLK_DUMMY>,
>
>
> --
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

* Re: [PATCH 3/5] arm64: dts: imx8mp: Enable HS400-ES
@ 2022-03-28 10:47       ` Adam Ford
  0 siblings, 0 replies; 44+ messages in thread
From: Adam Ford @ 2022-03-28 10:47 UTC (permalink / raw)
  To: Ahmad Fatoum
  Cc: linux-mmc, devicetree, Ulf Hansson, Fabio Estevam, Shawn Guo,
	Sascha Hauer, Adam Ford-BE, Haibo Chen,
	Linux Kernel Mailing List, Rob Herring, NXP Linux Team,
	Pengutronix Kernel Team, Krzysztof Kozlowski, arm-soc

On Mon, Mar 28, 2022 at 2:20 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>
> Hello Adam,
>
> On 27.03.22 14:38, Adam Ford wrote:
> > The SDHC controller in the imx8mp has the same controller
> > as the imx8mm which supports HS400-ES. Change the compatible
> > fallback to imx8mm to enable it.
>
> I believe that's a shortcoming of the Linux driver, which should explicitly list
> fsl,imx8mp-usdhc in its compatibles and enable HS400-ES for it.
>
> I find dropping compatibles problematic, because like Linux matching
> fsl,imx8mm-usdhc, but not fsl,imx8mp-usdhc, other software may match
> fsl,imx7d-usdhc, but not fsl,imx8[mp]-usdhc.
>
> I'd prefer that either the kernel driver gains extra compatibles or that
> the DTS lists extra compatibles and we refrain from dropping existing
> (correct) ones.
>

I would argue that imx7d is not correct since the IP blocks between
imx7d and imx8mm have different flags/quirks.  One of which includes
HS400-ES, but there are other differences as well.

> What do you think?

From my understanding of the fallback compatibility strings is to
avoid having to add more and more compatible strings to the drivers
when they do not serve a functional purpose. Based On a conversation
with Krzysztof [1], he suggested we update the YAML file based on the
fallback, but he wanted NXP to give their feedback as to what the
right fallback strings should be.  Haibo from NXP sent me a hierarchy
[1] which is what I used to update the YAML file.  Based on the YAML
file, the fallback in each DTSI file was updated to ensure the use of
the proper IP block.

adam

[1] - https://lore.kernel.org/linux-arm-kernel/CAHCN7xLWoUGi-jfxR2a0gvEFkPT3USUEb+8U3CCqCb5wWEJ8xw@mail.gmail.com/T/

>
> Cheers,
> Ahmad
>
> > Signed-off-by: Adam Ford <aford173@gmail.com>
> > ---
> >  arch/arm64/boot/dts/freescale/imx8mp.dtsi | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > index 794d75173cf5..d5ee1520f1fe 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > @@ -769,7 +769,7 @@ i2c6: i2c@30ae0000 {
> >                       };
> >
> >                       usdhc1: mmc@30b40000 {
> > -                             compatible = "fsl,imx8mp-usdhc", "fsl,imx7d-usdhc";
> > +                             compatible = "fsl,imx8mp-usdhc", "fsl,imx8mm-usdhc";
> >                               reg = <0x30b40000 0x10000>;
> >                               interrupts = <GIC_SPI 22 IRQ_TYPE_LEVEL_HIGH>;
> >                               clocks = <&clk IMX8MP_CLK_DUMMY>,
> > @@ -783,7 +783,7 @@ usdhc1: mmc@30b40000 {
> >                       };
> >
> >                       usdhc2: mmc@30b50000 {
> > -                             compatible = "fsl,imx8mp-usdhc", "fsl,imx7d-usdhc";
> > +                             compatible = "fsl,imx8mp-usdhc", "fsl,imx8mm-usdhc";
> >                               reg = <0x30b50000 0x10000>;
> >                               interrupts = <GIC_SPI 23 IRQ_TYPE_LEVEL_HIGH>;
> >                               clocks = <&clk IMX8MP_CLK_DUMMY>,
> > @@ -797,7 +797,7 @@ usdhc2: mmc@30b50000 {
> >                       };
> >
> >                       usdhc3: mmc@30b60000 {
> > -                             compatible = "fsl,imx8mp-usdhc", "fsl,imx7d-usdhc";
> > +                             compatible = "fsl,imx8mp-usdhc", "fsl,imx8mm-usdhc";
> >                               reg = <0x30b60000 0x10000>;
> >                               interrupts = <GIC_SPI 24 IRQ_TYPE_LEVEL_HIGH>;
> >                               clocks = <&clk IMX8MP_CLK_DUMMY>,
>
>
> --
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

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

* Re: [PATCH 3/5] arm64: dts: imx8mp: Enable HS400-ES
  2022-03-28 10:47       ` Adam Ford
@ 2022-03-28 11:09         ` Ahmad Fatoum
  -1 siblings, 0 replies; 44+ messages in thread
From: Ahmad Fatoum @ 2022-03-28 11:09 UTC (permalink / raw)
  To: Adam Ford, Krzysztof Kozlowski
  Cc: linux-mmc, devicetree, Ulf Hansson, Fabio Estevam, Shawn Guo,
	Sascha Hauer, Adam Ford-BE, Haibo Chen,
	Linux Kernel Mailing List, Rob Herring, NXP Linux Team,
	Pengutronix Kernel Team, arm-soc

Hello Adam,

On 28.03.22 12:47, Adam Ford wrote:
> On Mon, Mar 28, 2022 at 2:20 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>>
>> Hello Adam,
>>
>> On 27.03.22 14:38, Adam Ford wrote:
>>> The SDHC controller in the imx8mp has the same controller
>>> as the imx8mm which supports HS400-ES. Change the compatible
>>> fallback to imx8mm to enable it.
>>
>> I believe that's a shortcoming of the Linux driver, which should explicitly list
>> fsl,imx8mp-usdhc in its compatibles and enable HS400-ES for it.
>>
>> I find dropping compatibles problematic, because like Linux matching
>> fsl,imx8mm-usdhc, but not fsl,imx8mp-usdhc, other software may match
>> fsl,imx7d-usdhc, but not fsl,imx8[mp]-usdhc.
>>
>> I'd prefer that either the kernel driver gains extra compatibles or that
>> the DTS lists extra compatibles and we refrain from dropping existing
>> (correct) ones.
>>
> 
> I would argue that imx7d is not correct since the IP blocks between
> imx7d and imx8mm have different flags/quirks.  One of which includes
> HS400-ES, but there are other differences as well.

The DTS currently says that an fsl,imx7d-usdhc is a subset of an
fsl,imx8mm-usdhc. So a driver could treat both HW the exact same
by focusing on the i.MX7D parts. Linux apparently did exactly
that so far. Is this not accurate?


>> What do you think?
> 
> From my understanding of the fallback compatibility strings is to
> avoid having to add more and more compatible strings to the drivers
> when they do not serve a functional purpose. Based On a conversation
> with Krzysztof [1], he suggested we update the YAML file based on the
> fallback, but he wanted NXP to give their feedback as to what the
> right fallback strings should be.  Haibo from NXP sent me a hierarchy
> [1] which is what I used to update the YAML file.  Based on the YAML
> file, the fallback in each DTSI file was updated to ensure the use of
> the proper IP block.

Myself I am in favor of moving to three compatibles instead of dropping one.
For some theoretical fsl,imx8mf-usdhc that's supposed to be exactly the same
as a fsl,imx8mm-usdhc, I don't mind omitting the fsl,imx7d-usdhc compatible,
but for existing device trees, this may introduce needless potential breakage
for other software that also uses Linux device trees.

Cheers,
Ahmad

> 
> adam
> 
> [1] - https://lore.kernel.org/linux-arm-kernel/CAHCN7xLWoUGi-jfxR2a0gvEFkPT3USUEb+8U3CCqCb5wWEJ8xw@mail.gmail.com/T/
> 
>>
>> Cheers,
>> Ahmad
>>
>>> Signed-off-by: Adam Ford <aford173@gmail.com>
>>> ---
>>>  arch/arm64/boot/dts/freescale/imx8mp.dtsi | 6 +++---
>>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
>>> index 794d75173cf5..d5ee1520f1fe 100644
>>> --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
>>> +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
>>> @@ -769,7 +769,7 @@ i2c6: i2c@30ae0000 {
>>>                       };
>>>
>>>                       usdhc1: mmc@30b40000 {
>>> -                             compatible = "fsl,imx8mp-usdhc", "fsl,imx7d-usdhc";
>>> +                             compatible = "fsl,imx8mp-usdhc", "fsl,imx8mm-usdhc";
>>>                               reg = <0x30b40000 0x10000>;
>>>                               interrupts = <GIC_SPI 22 IRQ_TYPE_LEVEL_HIGH>;
>>>                               clocks = <&clk IMX8MP_CLK_DUMMY>,
>>> @@ -783,7 +783,7 @@ usdhc1: mmc@30b40000 {
>>>                       };
>>>
>>>                       usdhc2: mmc@30b50000 {
>>> -                             compatible = "fsl,imx8mp-usdhc", "fsl,imx7d-usdhc";
>>> +                             compatible = "fsl,imx8mp-usdhc", "fsl,imx8mm-usdhc";
>>>                               reg = <0x30b50000 0x10000>;
>>>                               interrupts = <GIC_SPI 23 IRQ_TYPE_LEVEL_HIGH>;
>>>                               clocks = <&clk IMX8MP_CLK_DUMMY>,
>>> @@ -797,7 +797,7 @@ usdhc2: mmc@30b50000 {
>>>                       };
>>>
>>>                       usdhc3: mmc@30b60000 {
>>> -                             compatible = "fsl,imx8mp-usdhc", "fsl,imx7d-usdhc";
>>> +                             compatible = "fsl,imx8mp-usdhc", "fsl,imx8mm-usdhc";
>>>                               reg = <0x30b60000 0x10000>;
>>>                               interrupts = <GIC_SPI 24 IRQ_TYPE_LEVEL_HIGH>;
>>>                               clocks = <&clk IMX8MP_CLK_DUMMY>,
>>
>>
>> --
>> Pengutronix e.K.                           |                             |
>> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
>> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
>> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

* Re: [PATCH 3/5] arm64: dts: imx8mp: Enable HS400-ES
@ 2022-03-28 11:09         ` Ahmad Fatoum
  0 siblings, 0 replies; 44+ messages in thread
From: Ahmad Fatoum @ 2022-03-28 11:09 UTC (permalink / raw)
  To: Adam Ford, Krzysztof Kozlowski
  Cc: linux-mmc, devicetree, Ulf Hansson, Fabio Estevam, Shawn Guo,
	Sascha Hauer, Adam Ford-BE, Haibo Chen,
	Linux Kernel Mailing List, Rob Herring, NXP Linux Team,
	Pengutronix Kernel Team, arm-soc

Hello Adam,

On 28.03.22 12:47, Adam Ford wrote:
> On Mon, Mar 28, 2022 at 2:20 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>>
>> Hello Adam,
>>
>> On 27.03.22 14:38, Adam Ford wrote:
>>> The SDHC controller in the imx8mp has the same controller
>>> as the imx8mm which supports HS400-ES. Change the compatible
>>> fallback to imx8mm to enable it.
>>
>> I believe that's a shortcoming of the Linux driver, which should explicitly list
>> fsl,imx8mp-usdhc in its compatibles and enable HS400-ES for it.
>>
>> I find dropping compatibles problematic, because like Linux matching
>> fsl,imx8mm-usdhc, but not fsl,imx8mp-usdhc, other software may match
>> fsl,imx7d-usdhc, but not fsl,imx8[mp]-usdhc.
>>
>> I'd prefer that either the kernel driver gains extra compatibles or that
>> the DTS lists extra compatibles and we refrain from dropping existing
>> (correct) ones.
>>
> 
> I would argue that imx7d is not correct since the IP blocks between
> imx7d and imx8mm have different flags/quirks.  One of which includes
> HS400-ES, but there are other differences as well.

The DTS currently says that an fsl,imx7d-usdhc is a subset of an
fsl,imx8mm-usdhc. So a driver could treat both HW the exact same
by focusing on the i.MX7D parts. Linux apparently did exactly
that so far. Is this not accurate?


>> What do you think?
> 
> From my understanding of the fallback compatibility strings is to
> avoid having to add more and more compatible strings to the drivers
> when they do not serve a functional purpose. Based On a conversation
> with Krzysztof [1], he suggested we update the YAML file based on the
> fallback, but he wanted NXP to give their feedback as to what the
> right fallback strings should be.  Haibo from NXP sent me a hierarchy
> [1] which is what I used to update the YAML file.  Based on the YAML
> file, the fallback in each DTSI file was updated to ensure the use of
> the proper IP block.

Myself I am in favor of moving to three compatibles instead of dropping one.
For some theoretical fsl,imx8mf-usdhc that's supposed to be exactly the same
as a fsl,imx8mm-usdhc, I don't mind omitting the fsl,imx7d-usdhc compatible,
but for existing device trees, this may introduce needless potential breakage
for other software that also uses Linux device trees.

Cheers,
Ahmad

> 
> adam
> 
> [1] - https://lore.kernel.org/linux-arm-kernel/CAHCN7xLWoUGi-jfxR2a0gvEFkPT3USUEb+8U3CCqCb5wWEJ8xw@mail.gmail.com/T/
> 
>>
>> Cheers,
>> Ahmad
>>
>>> Signed-off-by: Adam Ford <aford173@gmail.com>
>>> ---
>>>  arch/arm64/boot/dts/freescale/imx8mp.dtsi | 6 +++---
>>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
>>> index 794d75173cf5..d5ee1520f1fe 100644
>>> --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
>>> +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
>>> @@ -769,7 +769,7 @@ i2c6: i2c@30ae0000 {
>>>                       };
>>>
>>>                       usdhc1: mmc@30b40000 {
>>> -                             compatible = "fsl,imx8mp-usdhc", "fsl,imx7d-usdhc";
>>> +                             compatible = "fsl,imx8mp-usdhc", "fsl,imx8mm-usdhc";
>>>                               reg = <0x30b40000 0x10000>;
>>>                               interrupts = <GIC_SPI 22 IRQ_TYPE_LEVEL_HIGH>;
>>>                               clocks = <&clk IMX8MP_CLK_DUMMY>,
>>> @@ -783,7 +783,7 @@ usdhc1: mmc@30b40000 {
>>>                       };
>>>
>>>                       usdhc2: mmc@30b50000 {
>>> -                             compatible = "fsl,imx8mp-usdhc", "fsl,imx7d-usdhc";
>>> +                             compatible = "fsl,imx8mp-usdhc", "fsl,imx8mm-usdhc";
>>>                               reg = <0x30b50000 0x10000>;
>>>                               interrupts = <GIC_SPI 23 IRQ_TYPE_LEVEL_HIGH>;
>>>                               clocks = <&clk IMX8MP_CLK_DUMMY>,
>>> @@ -797,7 +797,7 @@ usdhc2: mmc@30b50000 {
>>>                       };
>>>
>>>                       usdhc3: mmc@30b60000 {
>>> -                             compatible = "fsl,imx8mp-usdhc", "fsl,imx7d-usdhc";
>>> +                             compatible = "fsl,imx8mp-usdhc", "fsl,imx8mm-usdhc";
>>>                               reg = <0x30b60000 0x10000>;
>>>                               interrupts = <GIC_SPI 24 IRQ_TYPE_LEVEL_HIGH>;
>>>                               clocks = <&clk IMX8MP_CLK_DUMMY>,
>>
>>
>> --
>> Pengutronix e.K.                           |                             |
>> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
>> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
>> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

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

* Re: [PATCH 3/5] arm64: dts: imx8mp: Enable HS400-ES
  2022-03-28 11:09         ` Ahmad Fatoum
@ 2022-03-28 11:49           ` Krzysztof Kozlowski
  -1 siblings, 0 replies; 44+ messages in thread
From: Krzysztof Kozlowski @ 2022-03-28 11:49 UTC (permalink / raw)
  To: Ahmad Fatoum, Adam Ford, Krzysztof Kozlowski
  Cc: linux-mmc, devicetree, Ulf Hansson, Fabio Estevam, Shawn Guo,
	Sascha Hauer, Adam Ford-BE, Haibo Chen,
	Linux Kernel Mailing List, Rob Herring, NXP Linux Team,
	Pengutronix Kernel Team, arm-soc

On 28/03/2022 13:09, Ahmad Fatoum wrote:
> Hello Adam,
> 
> On 28.03.22 12:47, Adam Ford wrote:
>> On Mon, Mar 28, 2022 at 2:20 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>>>
>>> Hello Adam,
>>>
>>> On 27.03.22 14:38, Adam Ford wrote:
>>>> The SDHC controller in the imx8mp has the same controller
>>>> as the imx8mm which supports HS400-ES. Change the compatible
>>>> fallback to imx8mm to enable it.
>>>
>>> I believe that's a shortcoming of the Linux driver, which should explicitly list
>>> fsl,imx8mp-usdhc in its compatibles and enable HS400-ES for it.
>>>
>>> I find dropping compatibles problematic, because like Linux matching
>>> fsl,imx8mm-usdhc, but not fsl,imx8mp-usdhc, other software may match
>>> fsl,imx7d-usdhc, but not fsl,imx8[mp]-usdhc.
>>>
>>> I'd prefer that either the kernel driver gains extra compatibles or that
>>> the DTS lists extra compatibles and we refrain from dropping existing
>>> (correct) ones.
>>>
>>
>> I would argue that imx7d is not correct since the IP blocks between
>> imx7d and imx8mm have different flags/quirks.  One of which includes
>> HS400-ES, but there are other differences as well.
> 
> The DTS currently says that an fsl,imx7d-usdhc is a subset of an
> fsl,imx8mm-usdhc. So a driver could treat both HW the exact same
> by focusing on the i.MX7D parts. Linux apparently did exactly
> that so far. Is this not accurate?
> 
> 
>>> What do you think?
>>
>> From my understanding of the fallback compatibility strings is to
>> avoid having to add more and more compatible strings to the drivers
>> when they do not serve a functional purpose. Based On a conversation
>> with Krzysztof [1], he suggested we update the YAML file based on the
>> fallback, but he wanted NXP to give their feedback as to what the
>> right fallback strings should be.  Haibo from NXP sent me a hierarchy
>> [1] which is what I used to update the YAML file.  Based on the YAML
>> file, the fallback in each DTSI file was updated to ensure the use of
>> the proper IP block.
> 
> Myself I am in favor of moving to three compatibles instead of dropping one.
> For some theoretical fsl,imx8mf-usdhc that's supposed to be exactly the same
> as a fsl,imx8mm-usdhc, I don't mind omitting the fsl,imx7d-usdhc compatible,
> but for existing device trees, this may introduce needless potential breakage
> for other software that also uses Linux device trees.
> 

Affecting existing users is indeed a concern with this approach, because
in-kernel DTS might be used in other projects as well.

I still cannot find here the answer whether fsl,imx8mm-usdhc is actually
compatible with fsl,imx7d-usdhc. It's not about driver, but about
hardware and programming model. imx8mm can support additional features
and still be compatible with imx7d. However if any flags of imx7d are
actually not valid for imx8mm, then it's different case.


Best regards,
Krzysztof

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

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

* Re: [PATCH 3/5] arm64: dts: imx8mp: Enable HS400-ES
@ 2022-03-28 11:49           ` Krzysztof Kozlowski
  0 siblings, 0 replies; 44+ messages in thread
From: Krzysztof Kozlowski @ 2022-03-28 11:49 UTC (permalink / raw)
  To: Ahmad Fatoum, Adam Ford, Krzysztof Kozlowski
  Cc: linux-mmc, devicetree, Ulf Hansson, Fabio Estevam, Shawn Guo,
	Sascha Hauer, Adam Ford-BE, Haibo Chen,
	Linux Kernel Mailing List, Rob Herring, NXP Linux Team,
	Pengutronix Kernel Team, arm-soc

On 28/03/2022 13:09, Ahmad Fatoum wrote:
> Hello Adam,
> 
> On 28.03.22 12:47, Adam Ford wrote:
>> On Mon, Mar 28, 2022 at 2:20 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>>>
>>> Hello Adam,
>>>
>>> On 27.03.22 14:38, Adam Ford wrote:
>>>> The SDHC controller in the imx8mp has the same controller
>>>> as the imx8mm which supports HS400-ES. Change the compatible
>>>> fallback to imx8mm to enable it.
>>>
>>> I believe that's a shortcoming of the Linux driver, which should explicitly list
>>> fsl,imx8mp-usdhc in its compatibles and enable HS400-ES for it.
>>>
>>> I find dropping compatibles problematic, because like Linux matching
>>> fsl,imx8mm-usdhc, but not fsl,imx8mp-usdhc, other software may match
>>> fsl,imx7d-usdhc, but not fsl,imx8[mp]-usdhc.
>>>
>>> I'd prefer that either the kernel driver gains extra compatibles or that
>>> the DTS lists extra compatibles and we refrain from dropping existing
>>> (correct) ones.
>>>
>>
>> I would argue that imx7d is not correct since the IP blocks between
>> imx7d and imx8mm have different flags/quirks.  One of which includes
>> HS400-ES, but there are other differences as well.
> 
> The DTS currently says that an fsl,imx7d-usdhc is a subset of an
> fsl,imx8mm-usdhc. So a driver could treat both HW the exact same
> by focusing on the i.MX7D parts. Linux apparently did exactly
> that so far. Is this not accurate?
> 
> 
>>> What do you think?
>>
>> From my understanding of the fallback compatibility strings is to
>> avoid having to add more and more compatible strings to the drivers
>> when they do not serve a functional purpose. Based On a conversation
>> with Krzysztof [1], he suggested we update the YAML file based on the
>> fallback, but he wanted NXP to give their feedback as to what the
>> right fallback strings should be.  Haibo from NXP sent me a hierarchy
>> [1] which is what I used to update the YAML file.  Based on the YAML
>> file, the fallback in each DTSI file was updated to ensure the use of
>> the proper IP block.
> 
> Myself I am in favor of moving to three compatibles instead of dropping one.
> For some theoretical fsl,imx8mf-usdhc that's supposed to be exactly the same
> as a fsl,imx8mm-usdhc, I don't mind omitting the fsl,imx7d-usdhc compatible,
> but for existing device trees, this may introduce needless potential breakage
> for other software that also uses Linux device trees.
> 

Affecting existing users is indeed a concern with this approach, because
in-kernel DTS might be used in other projects as well.

I still cannot find here the answer whether fsl,imx8mm-usdhc is actually
compatible with fsl,imx7d-usdhc. It's not about driver, but about
hardware and programming model. imx8mm can support additional features
and still be compatible with imx7d. However if any flags of imx7d are
actually not valid for imx8mm, then it's different case.


Best regards,
Krzysztof

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

* Re: [PATCH 3/5] arm64: dts: imx8mp: Enable HS400-ES
  2022-03-28 11:49           ` Krzysztof Kozlowski
@ 2022-03-28 12:45             ` Adam Ford
  -1 siblings, 0 replies; 44+ messages in thread
From: Adam Ford @ 2022-03-28 12:45 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Ahmad Fatoum, Krzysztof Kozlowski, linux-mmc, devicetree,
	Ulf Hansson, Fabio Estevam, Shawn Guo, Sascha Hauer,
	Adam Ford-BE, Haibo Chen, Linux Kernel Mailing List, Rob Herring,
	NXP Linux Team, Pengutronix Kernel Team, arm-soc

On Mon, Mar 28, 2022 at 6:49 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 28/03/2022 13:09, Ahmad Fatoum wrote:
> > Hello Adam,
> >
> > On 28.03.22 12:47, Adam Ford wrote:
> >> On Mon, Mar 28, 2022 at 2:20 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
> >>>
> >>> Hello Adam,
> >>>
> >>> On 27.03.22 14:38, Adam Ford wrote:
> >>>> The SDHC controller in the imx8mp has the same controller
> >>>> as the imx8mm which supports HS400-ES. Change the compatible
> >>>> fallback to imx8mm to enable it.
> >>>
> >>> I believe that's a shortcoming of the Linux driver, which should explicitly list
> >>> fsl,imx8mp-usdhc in its compatibles and enable HS400-ES for it.
> >>>
> >>> I find dropping compatibles problematic, because like Linux matching
> >>> fsl,imx8mm-usdhc, but not fsl,imx8mp-usdhc, other software may match
> >>> fsl,imx7d-usdhc, but not fsl,imx8[mp]-usdhc.
> >>>
> >>> I'd prefer that either the kernel driver gains extra compatibles or that
> >>> the DTS lists extra compatibles and we refrain from dropping existing
> >>> (correct) ones.
> >>>
> >>
> >> I would argue that imx7d is not correct since the IP blocks between
> >> imx7d and imx8mm have different flags/quirks.  One of which includes
> >> HS400-ES, but there are other differences as well.
> >
> > The DTS currently says that an fsl,imx7d-usdhc is a subset of an
> > fsl,imx8mm-usdhc. So a driver could treat both HW the exact same
> > by focusing on the i.MX7D parts. Linux apparently did exactly
> > that so far. Is this not accurate?
> >
> >
> >>> What do you think?
> >>
> >> From my understanding of the fallback compatibility strings is to
> >> avoid having to add more and more compatible strings to the drivers
> >> when they do not serve a functional purpose. Based On a conversation
> >> with Krzysztof [1], he suggested we update the YAML file based on the
> >> fallback, but he wanted NXP to give their feedback as to what the
> >> right fallback strings should be.  Haibo from NXP sent me a hierarchy
> >> [1] which is what I used to update the YAML file.  Based on the YAML
> >> file, the fallback in each DTSI file was updated to ensure the use of
> >> the proper IP block.
> >
> > Myself I am in favor of moving to three compatibles instead of dropping one.
> > For some theoretical fsl,imx8mf-usdhc that's supposed to be exactly the same
> > as a fsl,imx8mm-usdhc, I don't mind omitting the fsl,imx7d-usdhc compatible,
> > but for existing device trees, this may introduce needless potential breakage
> > for other software that also uses Linux device trees.
> >
>
> Affecting existing users is indeed a concern with this approach, because
> in-kernel DTS might be used in other projects as well.
>
> I still cannot find here the answer whether fsl,imx8mm-usdhc is actually
> compatible with fsl,imx7d-usdhc. It's not about driver, but about
> hardware and programming model. imx8mm can support additional features
> and still be compatible with imx7d. However if any flags of imx7d are
> actually not valid for imx8mm, then it's different case.

The imx7d flags are:
 ESDHC_FLAG_USDHC
ESDHC_FLAG_STD_TUNING
 ESDHC_FLAG_HAVE_CAP1
ESDHC_FLAG_HS200
 ESDHC_FLAG_HS400
 ESDHC_FLAG_STATE_LOST_IN_LPMODE
 ESDHC_FLAG_BROKEN_AUTO_CMD23,

The imx8mm flags are:
 ESDHC_FLAG_USDHC
 ESDHC_FLAG_STD_TUNING
 ESDHC_FLAG_HAVE_CAP1
ESDHC_FLAG_HS200
 ESDHC_FLAG_HS400
 ESDHC_FLAG_HS400_ES
 ESDHC_FLAG_STATE_LOST_IN_LPMODE

It does not have the ESDHC_FLAG_BROKEN_AUTO_CMD23 that is present in the imx7d.

Maybe Haibo can comment on whether or not that would be an issue for the 8m[mnp]

I will defer to Krzysztof and Haibo as to the proper method that we
should add HS400-ES.  I don't have an issue adding the imx8mn or
imx8mp compatible flags to the esdhc driver if that's the decision.
If that is the decision, my follow-up question would be how the YAML
should look, and if it needs to change at all.

adam

>
>
> Best regards,
> Krzysztof

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

* Re: [PATCH 3/5] arm64: dts: imx8mp: Enable HS400-ES
@ 2022-03-28 12:45             ` Adam Ford
  0 siblings, 0 replies; 44+ messages in thread
From: Adam Ford @ 2022-03-28 12:45 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Ahmad Fatoum, Krzysztof Kozlowski, linux-mmc, devicetree,
	Ulf Hansson, Fabio Estevam, Shawn Guo, Sascha Hauer,
	Adam Ford-BE, Haibo Chen, Linux Kernel Mailing List, Rob Herring,
	NXP Linux Team, Pengutronix Kernel Team, arm-soc

On Mon, Mar 28, 2022 at 6:49 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 28/03/2022 13:09, Ahmad Fatoum wrote:
> > Hello Adam,
> >
> > On 28.03.22 12:47, Adam Ford wrote:
> >> On Mon, Mar 28, 2022 at 2:20 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
> >>>
> >>> Hello Adam,
> >>>
> >>> On 27.03.22 14:38, Adam Ford wrote:
> >>>> The SDHC controller in the imx8mp has the same controller
> >>>> as the imx8mm which supports HS400-ES. Change the compatible
> >>>> fallback to imx8mm to enable it.
> >>>
> >>> I believe that's a shortcoming of the Linux driver, which should explicitly list
> >>> fsl,imx8mp-usdhc in its compatibles and enable HS400-ES for it.
> >>>
> >>> I find dropping compatibles problematic, because like Linux matching
> >>> fsl,imx8mm-usdhc, but not fsl,imx8mp-usdhc, other software may match
> >>> fsl,imx7d-usdhc, but not fsl,imx8[mp]-usdhc.
> >>>
> >>> I'd prefer that either the kernel driver gains extra compatibles or that
> >>> the DTS lists extra compatibles and we refrain from dropping existing
> >>> (correct) ones.
> >>>
> >>
> >> I would argue that imx7d is not correct since the IP blocks between
> >> imx7d and imx8mm have different flags/quirks.  One of which includes
> >> HS400-ES, but there are other differences as well.
> >
> > The DTS currently says that an fsl,imx7d-usdhc is a subset of an
> > fsl,imx8mm-usdhc. So a driver could treat both HW the exact same
> > by focusing on the i.MX7D parts. Linux apparently did exactly
> > that so far. Is this not accurate?
> >
> >
> >>> What do you think?
> >>
> >> From my understanding of the fallback compatibility strings is to
> >> avoid having to add more and more compatible strings to the drivers
> >> when they do not serve a functional purpose. Based On a conversation
> >> with Krzysztof [1], he suggested we update the YAML file based on the
> >> fallback, but he wanted NXP to give their feedback as to what the
> >> right fallback strings should be.  Haibo from NXP sent me a hierarchy
> >> [1] which is what I used to update the YAML file.  Based on the YAML
> >> file, the fallback in each DTSI file was updated to ensure the use of
> >> the proper IP block.
> >
> > Myself I am in favor of moving to three compatibles instead of dropping one.
> > For some theoretical fsl,imx8mf-usdhc that's supposed to be exactly the same
> > as a fsl,imx8mm-usdhc, I don't mind omitting the fsl,imx7d-usdhc compatible,
> > but for existing device trees, this may introduce needless potential breakage
> > for other software that also uses Linux device trees.
> >
>
> Affecting existing users is indeed a concern with this approach, because
> in-kernel DTS might be used in other projects as well.
>
> I still cannot find here the answer whether fsl,imx8mm-usdhc is actually
> compatible with fsl,imx7d-usdhc. It's not about driver, but about
> hardware and programming model. imx8mm can support additional features
> and still be compatible with imx7d. However if any flags of imx7d are
> actually not valid for imx8mm, then it's different case.

The imx7d flags are:
 ESDHC_FLAG_USDHC
ESDHC_FLAG_STD_TUNING
 ESDHC_FLAG_HAVE_CAP1
ESDHC_FLAG_HS200
 ESDHC_FLAG_HS400
 ESDHC_FLAG_STATE_LOST_IN_LPMODE
 ESDHC_FLAG_BROKEN_AUTO_CMD23,

The imx8mm flags are:
 ESDHC_FLAG_USDHC
 ESDHC_FLAG_STD_TUNING
 ESDHC_FLAG_HAVE_CAP1
ESDHC_FLAG_HS200
 ESDHC_FLAG_HS400
 ESDHC_FLAG_HS400_ES
 ESDHC_FLAG_STATE_LOST_IN_LPMODE

It does not have the ESDHC_FLAG_BROKEN_AUTO_CMD23 that is present in the imx7d.

Maybe Haibo can comment on whether or not that would be an issue for the 8m[mnp]

I will defer to Krzysztof and Haibo as to the proper method that we
should add HS400-ES.  I don't have an issue adding the imx8mn or
imx8mp compatible flags to the esdhc driver if that's the decision.
If that is the decision, my follow-up question would be how the YAML
should look, and if it needs to change at all.

adam

>
>
> Best regards,
> Krzysztof

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

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

* Re: [PATCH 3/5] arm64: dts: imx8mp: Enable HS400-ES
  2022-03-28 12:45             ` Adam Ford
@ 2022-03-28 12:56               ` Krzysztof Kozlowski
  -1 siblings, 0 replies; 44+ messages in thread
From: Krzysztof Kozlowski @ 2022-03-28 12:56 UTC (permalink / raw)
  To: Adam Ford
  Cc: Ahmad Fatoum, Krzysztof Kozlowski, linux-mmc, devicetree,
	Ulf Hansson, Fabio Estevam, Shawn Guo, Sascha Hauer,
	Adam Ford-BE, Haibo Chen, Linux Kernel Mailing List, Rob Herring,
	NXP Linux Team, Pengutronix Kernel Team, arm-soc

On 28/03/2022 14:45, Adam Ford wrote:
> On Mon, Mar 28, 2022 at 6:49 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>
>> On 28/03/2022 13:09, Ahmad Fatoum wrote:
>>> Hello Adam,
>>>
>>> On 28.03.22 12:47, Adam Ford wrote:
>>>> On Mon, Mar 28, 2022 at 2:20 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>>>>>
>>>>> Hello Adam,
>>>>>
>>>>> On 27.03.22 14:38, Adam Ford wrote:
>>>>>> The SDHC controller in the imx8mp has the same controller
>>>>>> as the imx8mm which supports HS400-ES. Change the compatible
>>>>>> fallback to imx8mm to enable it.
>>>>>
>>>>> I believe that's a shortcoming of the Linux driver, which should explicitly list
>>>>> fsl,imx8mp-usdhc in its compatibles and enable HS400-ES for it.
>>>>>
>>>>> I find dropping compatibles problematic, because like Linux matching
>>>>> fsl,imx8mm-usdhc, but not fsl,imx8mp-usdhc, other software may match
>>>>> fsl,imx7d-usdhc, but not fsl,imx8[mp]-usdhc.
>>>>>
>>>>> I'd prefer that either the kernel driver gains extra compatibles or that
>>>>> the DTS lists extra compatibles and we refrain from dropping existing
>>>>> (correct) ones.
>>>>>
>>>>
>>>> I would argue that imx7d is not correct since the IP blocks between
>>>> imx7d and imx8mm have different flags/quirks.  One of which includes
>>>> HS400-ES, but there are other differences as well.
>>>
>>> The DTS currently says that an fsl,imx7d-usdhc is a subset of an
>>> fsl,imx8mm-usdhc. So a driver could treat both HW the exact same
>>> by focusing on the i.MX7D parts. Linux apparently did exactly
>>> that so far. Is this not accurate?
>>>
>>>
>>>>> What do you think?
>>>>
>>>> From my understanding of the fallback compatibility strings is to
>>>> avoid having to add more and more compatible strings to the drivers
>>>> when they do not serve a functional purpose. Based On a conversation
>>>> with Krzysztof [1], he suggested we update the YAML file based on the
>>>> fallback, but he wanted NXP to give their feedback as to what the
>>>> right fallback strings should be.  Haibo from NXP sent me a hierarchy
>>>> [1] which is what I used to update the YAML file.  Based on the YAML
>>>> file, the fallback in each DTSI file was updated to ensure the use of
>>>> the proper IP block.
>>>
>>> Myself I am in favor of moving to three compatibles instead of dropping one.
>>> For some theoretical fsl,imx8mf-usdhc that's supposed to be exactly the same
>>> as a fsl,imx8mm-usdhc, I don't mind omitting the fsl,imx7d-usdhc compatible,
>>> but for existing device trees, this may introduce needless potential breakage
>>> for other software that also uses Linux device trees.
>>>
>>
>> Affecting existing users is indeed a concern with this approach, because
>> in-kernel DTS might be used in other projects as well.
>>
>> I still cannot find here the answer whether fsl,imx8mm-usdhc is actually
>> compatible with fsl,imx7d-usdhc. It's not about driver, but about
>> hardware and programming model. imx8mm can support additional features
>> and still be compatible with imx7d. However if any flags of imx7d are
>> actually not valid for imx8mm, then it's different case.
> 
> The imx7d flags are:
>  ESDHC_FLAG_USDHC
> ESDHC_FLAG_STD_TUNING
>  ESDHC_FLAG_HAVE_CAP1
> ESDHC_FLAG_HS200
>  ESDHC_FLAG_HS400
>  ESDHC_FLAG_STATE_LOST_IN_LPMODE
>  ESDHC_FLAG_BROKEN_AUTO_CMD23,
> 
> The imx8mm flags are:
>  ESDHC_FLAG_USDHC
>  ESDHC_FLAG_STD_TUNING
>  ESDHC_FLAG_HAVE_CAP1
> ESDHC_FLAG_HS200
>  ESDHC_FLAG_HS400
>  ESDHC_FLAG_HS400_ES
>  ESDHC_FLAG_STATE_LOST_IN_LPMODE
> 
> It does not have the ESDHC_FLAG_BROKEN_AUTO_CMD23 that is present in the imx7d.

AFAIU, it looks imx8mm is compatible with imx7d, because the broken
acmd23 only limits the features. If imx8mm binds according to imx7d, it
will not support acmd23 and HS400-ES.

Having three compatibles is therefore also OK.

You could also add two cases:
1. three compatibles, deprecated: True,
2. two compatibles, without imx7d.

Existing DTS stays with three compatibles for two years and later gets
converted to two compatibles. New DTS should use two compatibles.

It's quite a lot of churn, but would make in the long term bindings
correct and also not break other users/projects.

> 
> Maybe Haibo can comment on whether or not that would be an issue for the 8m[mnp]
> 
> I will defer to Krzysztof and Haibo as to the proper method that we
> should add HS400-ES.  I don't have an issue adding the imx8mn or
> imx8mp compatible flags to the esdhc driver if that's the decision.

I don't get what's the problem with HS400-ES. In any case (your patch
here, other ideas) your DTS will bind to imx8mm-usdhc which has HS400-ES.

Best regards,
Krzysztof

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

* Re: [PATCH 3/5] arm64: dts: imx8mp: Enable HS400-ES
@ 2022-03-28 12:56               ` Krzysztof Kozlowski
  0 siblings, 0 replies; 44+ messages in thread
From: Krzysztof Kozlowski @ 2022-03-28 12:56 UTC (permalink / raw)
  To: Adam Ford
  Cc: Ahmad Fatoum, Krzysztof Kozlowski, linux-mmc, devicetree,
	Ulf Hansson, Fabio Estevam, Shawn Guo, Sascha Hauer,
	Adam Ford-BE, Haibo Chen, Linux Kernel Mailing List, Rob Herring,
	NXP Linux Team, Pengutronix Kernel Team, arm-soc

On 28/03/2022 14:45, Adam Ford wrote:
> On Mon, Mar 28, 2022 at 6:49 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>
>> On 28/03/2022 13:09, Ahmad Fatoum wrote:
>>> Hello Adam,
>>>
>>> On 28.03.22 12:47, Adam Ford wrote:
>>>> On Mon, Mar 28, 2022 at 2:20 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>>>>>
>>>>> Hello Adam,
>>>>>
>>>>> On 27.03.22 14:38, Adam Ford wrote:
>>>>>> The SDHC controller in the imx8mp has the same controller
>>>>>> as the imx8mm which supports HS400-ES. Change the compatible
>>>>>> fallback to imx8mm to enable it.
>>>>>
>>>>> I believe that's a shortcoming of the Linux driver, which should explicitly list
>>>>> fsl,imx8mp-usdhc in its compatibles and enable HS400-ES for it.
>>>>>
>>>>> I find dropping compatibles problematic, because like Linux matching
>>>>> fsl,imx8mm-usdhc, but not fsl,imx8mp-usdhc, other software may match
>>>>> fsl,imx7d-usdhc, but not fsl,imx8[mp]-usdhc.
>>>>>
>>>>> I'd prefer that either the kernel driver gains extra compatibles or that
>>>>> the DTS lists extra compatibles and we refrain from dropping existing
>>>>> (correct) ones.
>>>>>
>>>>
>>>> I would argue that imx7d is not correct since the IP blocks between
>>>> imx7d and imx8mm have different flags/quirks.  One of which includes
>>>> HS400-ES, but there are other differences as well.
>>>
>>> The DTS currently says that an fsl,imx7d-usdhc is a subset of an
>>> fsl,imx8mm-usdhc. So a driver could treat both HW the exact same
>>> by focusing on the i.MX7D parts. Linux apparently did exactly
>>> that so far. Is this not accurate?
>>>
>>>
>>>>> What do you think?
>>>>
>>>> From my understanding of the fallback compatibility strings is to
>>>> avoid having to add more and more compatible strings to the drivers
>>>> when they do not serve a functional purpose. Based On a conversation
>>>> with Krzysztof [1], he suggested we update the YAML file based on the
>>>> fallback, but he wanted NXP to give their feedback as to what the
>>>> right fallback strings should be.  Haibo from NXP sent me a hierarchy
>>>> [1] which is what I used to update the YAML file.  Based on the YAML
>>>> file, the fallback in each DTSI file was updated to ensure the use of
>>>> the proper IP block.
>>>
>>> Myself I am in favor of moving to three compatibles instead of dropping one.
>>> For some theoretical fsl,imx8mf-usdhc that's supposed to be exactly the same
>>> as a fsl,imx8mm-usdhc, I don't mind omitting the fsl,imx7d-usdhc compatible,
>>> but for existing device trees, this may introduce needless potential breakage
>>> for other software that also uses Linux device trees.
>>>
>>
>> Affecting existing users is indeed a concern with this approach, because
>> in-kernel DTS might be used in other projects as well.
>>
>> I still cannot find here the answer whether fsl,imx8mm-usdhc is actually
>> compatible with fsl,imx7d-usdhc. It's not about driver, but about
>> hardware and programming model. imx8mm can support additional features
>> and still be compatible with imx7d. However if any flags of imx7d are
>> actually not valid for imx8mm, then it's different case.
> 
> The imx7d flags are:
>  ESDHC_FLAG_USDHC
> ESDHC_FLAG_STD_TUNING
>  ESDHC_FLAG_HAVE_CAP1
> ESDHC_FLAG_HS200
>  ESDHC_FLAG_HS400
>  ESDHC_FLAG_STATE_LOST_IN_LPMODE
>  ESDHC_FLAG_BROKEN_AUTO_CMD23,
> 
> The imx8mm flags are:
>  ESDHC_FLAG_USDHC
>  ESDHC_FLAG_STD_TUNING
>  ESDHC_FLAG_HAVE_CAP1
> ESDHC_FLAG_HS200
>  ESDHC_FLAG_HS400
>  ESDHC_FLAG_HS400_ES
>  ESDHC_FLAG_STATE_LOST_IN_LPMODE
> 
> It does not have the ESDHC_FLAG_BROKEN_AUTO_CMD23 that is present in the imx7d.

AFAIU, it looks imx8mm is compatible with imx7d, because the broken
acmd23 only limits the features. If imx8mm binds according to imx7d, it
will not support acmd23 and HS400-ES.

Having three compatibles is therefore also OK.

You could also add two cases:
1. three compatibles, deprecated: True,
2. two compatibles, without imx7d.

Existing DTS stays with three compatibles for two years and later gets
converted to two compatibles. New DTS should use two compatibles.

It's quite a lot of churn, but would make in the long term bindings
correct and also not break other users/projects.

> 
> Maybe Haibo can comment on whether or not that would be an issue for the 8m[mnp]
> 
> I will defer to Krzysztof and Haibo as to the proper method that we
> should add HS400-ES.  I don't have an issue adding the imx8mn or
> imx8mp compatible flags to the esdhc driver if that's the decision.

I don't get what's the problem with HS400-ES. In any case (your patch
here, other ideas) your DTS will bind to imx8mm-usdhc which has HS400-ES.

Best regards,
Krzysztof

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

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

* Re: [PATCH 3/5] arm64: dts: imx8mp: Enable HS400-ES
  2022-03-28 12:56               ` Krzysztof Kozlowski
@ 2022-03-28 13:06                 ` Adam Ford
  -1 siblings, 0 replies; 44+ messages in thread
From: Adam Ford @ 2022-03-28 13:06 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Ahmad Fatoum, Krzysztof Kozlowski, linux-mmc, devicetree,
	Ulf Hansson, Fabio Estevam, Shawn Guo, Sascha Hauer,
	Adam Ford-BE, Haibo Chen, Linux Kernel Mailing List, Rob Herring,
	NXP Linux Team, Pengutronix Kernel Team, arm-soc

On Mon, Mar 28, 2022 at 7:56 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 28/03/2022 14:45, Adam Ford wrote:
> > On Mon, Mar 28, 2022 at 6:49 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >>
> >> On 28/03/2022 13:09, Ahmad Fatoum wrote:
> >>> Hello Adam,
> >>>
> >>> On 28.03.22 12:47, Adam Ford wrote:
> >>>> On Mon, Mar 28, 2022 at 2:20 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
> >>>>>
> >>>>> Hello Adam,
> >>>>>
> >>>>> On 27.03.22 14:38, Adam Ford wrote:
> >>>>>> The SDHC controller in the imx8mp has the same controller
> >>>>>> as the imx8mm which supports HS400-ES. Change the compatible
> >>>>>> fallback to imx8mm to enable it.
> >>>>>
> >>>>> I believe that's a shortcoming of the Linux driver, which should explicitly list
> >>>>> fsl,imx8mp-usdhc in its compatibles and enable HS400-ES for it.
> >>>>>
> >>>>> I find dropping compatibles problematic, because like Linux matching
> >>>>> fsl,imx8mm-usdhc, but not fsl,imx8mp-usdhc, other software may match
> >>>>> fsl,imx7d-usdhc, but not fsl,imx8[mp]-usdhc.
> >>>>>
> >>>>> I'd prefer that either the kernel driver gains extra compatibles or that
> >>>>> the DTS lists extra compatibles and we refrain from dropping existing
> >>>>> (correct) ones.
> >>>>>
> >>>>
> >>>> I would argue that imx7d is not correct since the IP blocks between
> >>>> imx7d and imx8mm have different flags/quirks.  One of which includes
> >>>> HS400-ES, but there are other differences as well.
> >>>
> >>> The DTS currently says that an fsl,imx7d-usdhc is a subset of an
> >>> fsl,imx8mm-usdhc. So a driver could treat both HW the exact same
> >>> by focusing on the i.MX7D parts. Linux apparently did exactly
> >>> that so far. Is this not accurate?
> >>>
> >>>
> >>>>> What do you think?
> >>>>
> >>>> From my understanding of the fallback compatibility strings is to
> >>>> avoid having to add more and more compatible strings to the drivers
> >>>> when they do not serve a functional purpose. Based On a conversation
> >>>> with Krzysztof [1], he suggested we update the YAML file based on the
> >>>> fallback, but he wanted NXP to give their feedback as to what the
> >>>> right fallback strings should be.  Haibo from NXP sent me a hierarchy
> >>>> [1] which is what I used to update the YAML file.  Based on the YAML
> >>>> file, the fallback in each DTSI file was updated to ensure the use of
> >>>> the proper IP block.
> >>>
> >>> Myself I am in favor of moving to three compatibles instead of dropping one.
> >>> For some theoretical fsl,imx8mf-usdhc that's supposed to be exactly the same
> >>> as a fsl,imx8mm-usdhc, I don't mind omitting the fsl,imx7d-usdhc compatible,
> >>> but for existing device trees, this may introduce needless potential breakage
> >>> for other software that also uses Linux device trees.
> >>>
> >>
> >> Affecting existing users is indeed a concern with this approach, because
> >> in-kernel DTS might be used in other projects as well.
> >>
> >> I still cannot find here the answer whether fsl,imx8mm-usdhc is actually
> >> compatible with fsl,imx7d-usdhc. It's not about driver, but about
> >> hardware and programming model. imx8mm can support additional features
> >> and still be compatible with imx7d. However if any flags of imx7d are
> >> actually not valid for imx8mm, then it's different case.
> >
> > The imx7d flags are:
> >  ESDHC_FLAG_USDHC
> > ESDHC_FLAG_STD_TUNING
> >  ESDHC_FLAG_HAVE_CAP1
> > ESDHC_FLAG_HS200
> >  ESDHC_FLAG_HS400
> >  ESDHC_FLAG_STATE_LOST_IN_LPMODE
> >  ESDHC_FLAG_BROKEN_AUTO_CMD23,
> >
> > The imx8mm flags are:
> >  ESDHC_FLAG_USDHC
> >  ESDHC_FLAG_STD_TUNING
> >  ESDHC_FLAG_HAVE_CAP1
> > ESDHC_FLAG_HS200
> >  ESDHC_FLAG_HS400
> >  ESDHC_FLAG_HS400_ES
> >  ESDHC_FLAG_STATE_LOST_IN_LPMODE
> >
> > It does not have the ESDHC_FLAG_BROKEN_AUTO_CMD23 that is present in the imx7d.
>
> AFAIU, it looks imx8mm is compatible with imx7d, because the broken
> acmd23 only limits the features. If imx8mm binds according to imx7d, it
> will not support acmd23 and HS400-ES.
>
> Having three compatibles is therefore also OK.
>
> You could also add two cases:
> 1. three compatibles, deprecated: True,
> 2. two compatibles, without imx7d.
>
> Existing DTS stays with three compatibles for two years and later gets
> converted to two compatibles. New DTS should use two compatibles.
>
> It's quite a lot of churn, but would make in the long term bindings
> correct and also not break other users/projects.
>
> >
> > Maybe Haibo can comment on whether or not that would be an issue for the 8m[mnp]
> >
> > I will defer to Krzysztof and Haibo as to the proper method that we
> > should add HS400-ES.  I don't have an issue adding the imx8mn or
> > imx8mp compatible flags to the esdhc driver if that's the decision.
>
> I don't get what's the problem with HS400-ES. In any case (your patch
> here, other ideas) your DTS will bind to imx8mm-usdhc which has HS400-ES.

I was under the impression Ahmad didn't want me to add the imx8mm
compatible to the DTS, but instead, add the imx8mp compatible into the
driver so it binds directly to an imx8mp.
Based on that, I was assuming this patch series would be rejected and
I sould focus on just the driver file itself

From what I am reading from you, I should make the imx8mn and imx8mp
device trees respectively read:

compatible = "fsl,imx8mn-usdhc", "fsl,imx8mm-usdhc", "fsl,imx7d-usdhc";

and

compatible = "fsl,imx8mp-usdhc", "fsl,imx8mm-usdhc", "fsl,imx7d-usdhc";

If that's true, do I need to change the YAML at all?

adam
>
> Best regards,
> Krzysztof

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

* Re: [PATCH 3/5] arm64: dts: imx8mp: Enable HS400-ES
@ 2022-03-28 13:06                 ` Adam Ford
  0 siblings, 0 replies; 44+ messages in thread
From: Adam Ford @ 2022-03-28 13:06 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Ahmad Fatoum, Krzysztof Kozlowski, linux-mmc, devicetree,
	Ulf Hansson, Fabio Estevam, Shawn Guo, Sascha Hauer,
	Adam Ford-BE, Haibo Chen, Linux Kernel Mailing List, Rob Herring,
	NXP Linux Team, Pengutronix Kernel Team, arm-soc

On Mon, Mar 28, 2022 at 7:56 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 28/03/2022 14:45, Adam Ford wrote:
> > On Mon, Mar 28, 2022 at 6:49 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >>
> >> On 28/03/2022 13:09, Ahmad Fatoum wrote:
> >>> Hello Adam,
> >>>
> >>> On 28.03.22 12:47, Adam Ford wrote:
> >>>> On Mon, Mar 28, 2022 at 2:20 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
> >>>>>
> >>>>> Hello Adam,
> >>>>>
> >>>>> On 27.03.22 14:38, Adam Ford wrote:
> >>>>>> The SDHC controller in the imx8mp has the same controller
> >>>>>> as the imx8mm which supports HS400-ES. Change the compatible
> >>>>>> fallback to imx8mm to enable it.
> >>>>>
> >>>>> I believe that's a shortcoming of the Linux driver, which should explicitly list
> >>>>> fsl,imx8mp-usdhc in its compatibles and enable HS400-ES for it.
> >>>>>
> >>>>> I find dropping compatibles problematic, because like Linux matching
> >>>>> fsl,imx8mm-usdhc, but not fsl,imx8mp-usdhc, other software may match
> >>>>> fsl,imx7d-usdhc, but not fsl,imx8[mp]-usdhc.
> >>>>>
> >>>>> I'd prefer that either the kernel driver gains extra compatibles or that
> >>>>> the DTS lists extra compatibles and we refrain from dropping existing
> >>>>> (correct) ones.
> >>>>>
> >>>>
> >>>> I would argue that imx7d is not correct since the IP blocks between
> >>>> imx7d and imx8mm have different flags/quirks.  One of which includes
> >>>> HS400-ES, but there are other differences as well.
> >>>
> >>> The DTS currently says that an fsl,imx7d-usdhc is a subset of an
> >>> fsl,imx8mm-usdhc. So a driver could treat both HW the exact same
> >>> by focusing on the i.MX7D parts. Linux apparently did exactly
> >>> that so far. Is this not accurate?
> >>>
> >>>
> >>>>> What do you think?
> >>>>
> >>>> From my understanding of the fallback compatibility strings is to
> >>>> avoid having to add more and more compatible strings to the drivers
> >>>> when they do not serve a functional purpose. Based On a conversation
> >>>> with Krzysztof [1], he suggested we update the YAML file based on the
> >>>> fallback, but he wanted NXP to give their feedback as to what the
> >>>> right fallback strings should be.  Haibo from NXP sent me a hierarchy
> >>>> [1] which is what I used to update the YAML file.  Based on the YAML
> >>>> file, the fallback in each DTSI file was updated to ensure the use of
> >>>> the proper IP block.
> >>>
> >>> Myself I am in favor of moving to three compatibles instead of dropping one.
> >>> For some theoretical fsl,imx8mf-usdhc that's supposed to be exactly the same
> >>> as a fsl,imx8mm-usdhc, I don't mind omitting the fsl,imx7d-usdhc compatible,
> >>> but for existing device trees, this may introduce needless potential breakage
> >>> for other software that also uses Linux device trees.
> >>>
> >>
> >> Affecting existing users is indeed a concern with this approach, because
> >> in-kernel DTS might be used in other projects as well.
> >>
> >> I still cannot find here the answer whether fsl,imx8mm-usdhc is actually
> >> compatible with fsl,imx7d-usdhc. It's not about driver, but about
> >> hardware and programming model. imx8mm can support additional features
> >> and still be compatible with imx7d. However if any flags of imx7d are
> >> actually not valid for imx8mm, then it's different case.
> >
> > The imx7d flags are:
> >  ESDHC_FLAG_USDHC
> > ESDHC_FLAG_STD_TUNING
> >  ESDHC_FLAG_HAVE_CAP1
> > ESDHC_FLAG_HS200
> >  ESDHC_FLAG_HS400
> >  ESDHC_FLAG_STATE_LOST_IN_LPMODE
> >  ESDHC_FLAG_BROKEN_AUTO_CMD23,
> >
> > The imx8mm flags are:
> >  ESDHC_FLAG_USDHC
> >  ESDHC_FLAG_STD_TUNING
> >  ESDHC_FLAG_HAVE_CAP1
> > ESDHC_FLAG_HS200
> >  ESDHC_FLAG_HS400
> >  ESDHC_FLAG_HS400_ES
> >  ESDHC_FLAG_STATE_LOST_IN_LPMODE
> >
> > It does not have the ESDHC_FLAG_BROKEN_AUTO_CMD23 that is present in the imx7d.
>
> AFAIU, it looks imx8mm is compatible with imx7d, because the broken
> acmd23 only limits the features. If imx8mm binds according to imx7d, it
> will not support acmd23 and HS400-ES.
>
> Having three compatibles is therefore also OK.
>
> You could also add two cases:
> 1. three compatibles, deprecated: True,
> 2. two compatibles, without imx7d.
>
> Existing DTS stays with three compatibles for two years and later gets
> converted to two compatibles. New DTS should use two compatibles.
>
> It's quite a lot of churn, but would make in the long term bindings
> correct and also not break other users/projects.
>
> >
> > Maybe Haibo can comment on whether or not that would be an issue for the 8m[mnp]
> >
> > I will defer to Krzysztof and Haibo as to the proper method that we
> > should add HS400-ES.  I don't have an issue adding the imx8mn or
> > imx8mp compatible flags to the esdhc driver if that's the decision.
>
> I don't get what's the problem with HS400-ES. In any case (your patch
> here, other ideas) your DTS will bind to imx8mm-usdhc which has HS400-ES.

I was under the impression Ahmad didn't want me to add the imx8mm
compatible to the DTS, but instead, add the imx8mp compatible into the
driver so it binds directly to an imx8mp.
Based on that, I was assuming this patch series would be rejected and
I sould focus on just the driver file itself

From what I am reading from you, I should make the imx8mn and imx8mp
device trees respectively read:

compatible = "fsl,imx8mn-usdhc", "fsl,imx8mm-usdhc", "fsl,imx7d-usdhc";

and

compatible = "fsl,imx8mp-usdhc", "fsl,imx8mm-usdhc", "fsl,imx7d-usdhc";

If that's true, do I need to change the YAML at all?

adam
>
> Best regards,
> Krzysztof

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

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

* Re: [PATCH 3/5] arm64: dts: imx8mp: Enable HS400-ES
  2022-03-28 12:56               ` Krzysztof Kozlowski
@ 2022-03-28 13:07                 ` Ahmad Fatoum
  -1 siblings, 0 replies; 44+ messages in thread
From: Ahmad Fatoum @ 2022-03-28 13:07 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Adam Ford
  Cc: Krzysztof Kozlowski, linux-mmc, devicetree, Ulf Hansson,
	Fabio Estevam, Shawn Guo, Sascha Hauer, Adam Ford-BE, Haibo Chen,
	Linux Kernel Mailing List, Rob Herring, NXP Linux Team,
	Pengutronix Kernel Team, arm-soc

Hello Krzysztof,

On 28.03.22 14:56, Krzysztof Kozlowski wrote:
> On 28/03/2022 14:45, Adam Ford wrote:
>> On Mon, Mar 28, 2022 at 6:49 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>>
>>> On 28/03/2022 13:09, Ahmad Fatoum wrote:
>>>> Hello Adam,
>>>>
>>>> On 28.03.22 12:47, Adam Ford wrote:
>>>>> On Mon, Mar 28, 2022 at 2:20 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>>>>>>
>>>>>> Hello Adam,
>>>>>>
>>>>>> On 27.03.22 14:38, Adam Ford wrote:
>>>>>>> The SDHC controller in the imx8mp has the same controller
>>>>>>> as the imx8mm which supports HS400-ES. Change the compatible
>>>>>>> fallback to imx8mm to enable it.
>>>>>>
>>>>>> I believe that's a shortcoming of the Linux driver, which should explicitly list
>>>>>> fsl,imx8mp-usdhc in its compatibles and enable HS400-ES for it.
>>>>>>
>>>>>> I find dropping compatibles problematic, because like Linux matching
>>>>>> fsl,imx8mm-usdhc, but not fsl,imx8mp-usdhc, other software may match
>>>>>> fsl,imx7d-usdhc, but not fsl,imx8[mp]-usdhc.
>>>>>>
>>>>>> I'd prefer that either the kernel driver gains extra compatibles or that
>>>>>> the DTS lists extra compatibles and we refrain from dropping existing
>>>>>> (correct) ones.
>>>>>>
>>>>>
>>>>> I would argue that imx7d is not correct since the IP blocks between
>>>>> imx7d and imx8mm have different flags/quirks.  One of which includes
>>>>> HS400-ES, but there are other differences as well.
>>>>
>>>> The DTS currently says that an fsl,imx7d-usdhc is a subset of an
>>>> fsl,imx8mm-usdhc. So a driver could treat both HW the exact same
>>>> by focusing on the i.MX7D parts. Linux apparently did exactly
>>>> that so far. Is this not accurate?
>>>>
>>>>
>>>>>> What do you think?
>>>>>
>>>>> From my understanding of the fallback compatibility strings is to
>>>>> avoid having to add more and more compatible strings to the drivers
>>>>> when they do not serve a functional purpose. Based On a conversation
>>>>> with Krzysztof [1], he suggested we update the YAML file based on the
>>>>> fallback, but he wanted NXP to give their feedback as to what the
>>>>> right fallback strings should be.  Haibo from NXP sent me a hierarchy
>>>>> [1] which is what I used to update the YAML file.  Based on the YAML
>>>>> file, the fallback in each DTSI file was updated to ensure the use of
>>>>> the proper IP block.
>>>>
>>>> Myself I am in favor of moving to three compatibles instead of dropping one.
>>>> For some theoretical fsl,imx8mf-usdhc that's supposed to be exactly the same
>>>> as a fsl,imx8mm-usdhc, I don't mind omitting the fsl,imx7d-usdhc compatible,
>>>> but for existing device trees, this may introduce needless potential breakage
>>>> for other software that also uses Linux device trees.
>>>>
>>>
>>> Affecting existing users is indeed a concern with this approach, because
>>> in-kernel DTS might be used in other projects as well.
>>>
>>> I still cannot find here the answer whether fsl,imx8mm-usdhc is actually
>>> compatible with fsl,imx7d-usdhc. It's not about driver, but about
>>> hardware and programming model. imx8mm can support additional features
>>> and still be compatible with imx7d. However if any flags of imx7d are
>>> actually not valid for imx8mm, then it's different case.
>>
>> The imx7d flags are:
>>  ESDHC_FLAG_USDHC
>> ESDHC_FLAG_STD_TUNING
>>  ESDHC_FLAG_HAVE_CAP1
>> ESDHC_FLAG_HS200
>>  ESDHC_FLAG_HS400
>>  ESDHC_FLAG_STATE_LOST_IN_LPMODE
>>  ESDHC_FLAG_BROKEN_AUTO_CMD23,
>>
>> The imx8mm flags are:
>>  ESDHC_FLAG_USDHC
>>  ESDHC_FLAG_STD_TUNING
>>  ESDHC_FLAG_HAVE_CAP1
>> ESDHC_FLAG_HS200
>>  ESDHC_FLAG_HS400
>>  ESDHC_FLAG_HS400_ES
>>  ESDHC_FLAG_STATE_LOST_IN_LPMODE
>>
>> It does not have the ESDHC_FLAG_BROKEN_AUTO_CMD23 that is present in the imx7d.
> 
> AFAIU, it looks imx8mm is compatible with imx7d, because the broken
> acmd23 only limits the features. If imx8mm binds according to imx7d, it
> will not support acmd23 and HS400-ES.
> 
> Having three compatibles is therefore also OK.

My thoughts, exactly.

> You could also add two cases:
> 1. three compatibles, deprecated: True,
> 2. two compatibles, without imx7d.
> 
> Existing DTS stays with three compatibles for two years and later gets
> converted to two compatibles. New DTS should use two compatibles.
> 
> It's quite a lot of churn, but would make in the long term bindings
> correct and also not break other users/projects.

I don't see why we need to deprecate the old binding. New SoCs
can be imx8mm-usdhc compatible from the beginning and need not
care about the old binding. Existing SoCs can just remain imx7d-usdhc
compatible as they are now.

I don't see what the deprecation accomplishes.

>> Maybe Haibo can comment on whether or not that would be an issue for the 8m[mnp]
>>
>> I will defer to Krzysztof and Haibo as to the proper method that we
>> should add HS400-ES.  I don't have an issue adding the imx8mn or
>> imx8mp compatible flags to the esdhc driver if that's the decision.
> 
> I don't get what's the problem with HS400-ES. In any case (your patch
> here, other ideas) your DTS will bind to imx8mm-usdhc which has HS400-ES.

Cheers,
Ahmad

> Best regards,
> Krzysztof


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

* Re: [PATCH 3/5] arm64: dts: imx8mp: Enable HS400-ES
@ 2022-03-28 13:07                 ` Ahmad Fatoum
  0 siblings, 0 replies; 44+ messages in thread
From: Ahmad Fatoum @ 2022-03-28 13:07 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Adam Ford
  Cc: Krzysztof Kozlowski, linux-mmc, devicetree, Ulf Hansson,
	Fabio Estevam, Shawn Guo, Sascha Hauer, Adam Ford-BE, Haibo Chen,
	Linux Kernel Mailing List, Rob Herring, NXP Linux Team,
	Pengutronix Kernel Team, arm-soc

Hello Krzysztof,

On 28.03.22 14:56, Krzysztof Kozlowski wrote:
> On 28/03/2022 14:45, Adam Ford wrote:
>> On Mon, Mar 28, 2022 at 6:49 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>>
>>> On 28/03/2022 13:09, Ahmad Fatoum wrote:
>>>> Hello Adam,
>>>>
>>>> On 28.03.22 12:47, Adam Ford wrote:
>>>>> On Mon, Mar 28, 2022 at 2:20 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>>>>>>
>>>>>> Hello Adam,
>>>>>>
>>>>>> On 27.03.22 14:38, Adam Ford wrote:
>>>>>>> The SDHC controller in the imx8mp has the same controller
>>>>>>> as the imx8mm which supports HS400-ES. Change the compatible
>>>>>>> fallback to imx8mm to enable it.
>>>>>>
>>>>>> I believe that's a shortcoming of the Linux driver, which should explicitly list
>>>>>> fsl,imx8mp-usdhc in its compatibles and enable HS400-ES for it.
>>>>>>
>>>>>> I find dropping compatibles problematic, because like Linux matching
>>>>>> fsl,imx8mm-usdhc, but not fsl,imx8mp-usdhc, other software may match
>>>>>> fsl,imx7d-usdhc, but not fsl,imx8[mp]-usdhc.
>>>>>>
>>>>>> I'd prefer that either the kernel driver gains extra compatibles or that
>>>>>> the DTS lists extra compatibles and we refrain from dropping existing
>>>>>> (correct) ones.
>>>>>>
>>>>>
>>>>> I would argue that imx7d is not correct since the IP blocks between
>>>>> imx7d and imx8mm have different flags/quirks.  One of which includes
>>>>> HS400-ES, but there are other differences as well.
>>>>
>>>> The DTS currently says that an fsl,imx7d-usdhc is a subset of an
>>>> fsl,imx8mm-usdhc. So a driver could treat both HW the exact same
>>>> by focusing on the i.MX7D parts. Linux apparently did exactly
>>>> that so far. Is this not accurate?
>>>>
>>>>
>>>>>> What do you think?
>>>>>
>>>>> From my understanding of the fallback compatibility strings is to
>>>>> avoid having to add more and more compatible strings to the drivers
>>>>> when they do not serve a functional purpose. Based On a conversation
>>>>> with Krzysztof [1], he suggested we update the YAML file based on the
>>>>> fallback, but he wanted NXP to give their feedback as to what the
>>>>> right fallback strings should be.  Haibo from NXP sent me a hierarchy
>>>>> [1] which is what I used to update the YAML file.  Based on the YAML
>>>>> file, the fallback in each DTSI file was updated to ensure the use of
>>>>> the proper IP block.
>>>>
>>>> Myself I am in favor of moving to three compatibles instead of dropping one.
>>>> For some theoretical fsl,imx8mf-usdhc that's supposed to be exactly the same
>>>> as a fsl,imx8mm-usdhc, I don't mind omitting the fsl,imx7d-usdhc compatible,
>>>> but for existing device trees, this may introduce needless potential breakage
>>>> for other software that also uses Linux device trees.
>>>>
>>>
>>> Affecting existing users is indeed a concern with this approach, because
>>> in-kernel DTS might be used in other projects as well.
>>>
>>> I still cannot find here the answer whether fsl,imx8mm-usdhc is actually
>>> compatible with fsl,imx7d-usdhc. It's not about driver, but about
>>> hardware and programming model. imx8mm can support additional features
>>> and still be compatible with imx7d. However if any flags of imx7d are
>>> actually not valid for imx8mm, then it's different case.
>>
>> The imx7d flags are:
>>  ESDHC_FLAG_USDHC
>> ESDHC_FLAG_STD_TUNING
>>  ESDHC_FLAG_HAVE_CAP1
>> ESDHC_FLAG_HS200
>>  ESDHC_FLAG_HS400
>>  ESDHC_FLAG_STATE_LOST_IN_LPMODE
>>  ESDHC_FLAG_BROKEN_AUTO_CMD23,
>>
>> The imx8mm flags are:
>>  ESDHC_FLAG_USDHC
>>  ESDHC_FLAG_STD_TUNING
>>  ESDHC_FLAG_HAVE_CAP1
>> ESDHC_FLAG_HS200
>>  ESDHC_FLAG_HS400
>>  ESDHC_FLAG_HS400_ES
>>  ESDHC_FLAG_STATE_LOST_IN_LPMODE
>>
>> It does not have the ESDHC_FLAG_BROKEN_AUTO_CMD23 that is present in the imx7d.
> 
> AFAIU, it looks imx8mm is compatible with imx7d, because the broken
> acmd23 only limits the features. If imx8mm binds according to imx7d, it
> will not support acmd23 and HS400-ES.
> 
> Having three compatibles is therefore also OK.

My thoughts, exactly.

> You could also add two cases:
> 1. three compatibles, deprecated: True,
> 2. two compatibles, without imx7d.
> 
> Existing DTS stays with three compatibles for two years and later gets
> converted to two compatibles. New DTS should use two compatibles.
> 
> It's quite a lot of churn, but would make in the long term bindings
> correct and also not break other users/projects.

I don't see why we need to deprecate the old binding. New SoCs
can be imx8mm-usdhc compatible from the beginning and need not
care about the old binding. Existing SoCs can just remain imx7d-usdhc
compatible as they are now.

I don't see what the deprecation accomplishes.

>> Maybe Haibo can comment on whether or not that would be an issue for the 8m[mnp]
>>
>> I will defer to Krzysztof and Haibo as to the proper method that we
>> should add HS400-ES.  I don't have an issue adding the imx8mn or
>> imx8mp compatible flags to the esdhc driver if that's the decision.
> 
> I don't get what's the problem with HS400-ES. In any case (your patch
> here, other ideas) your DTS will bind to imx8mm-usdhc which has HS400-ES.

Cheers,
Ahmad

> Best regards,
> Krzysztof


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

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

* Re: [PATCH 3/5] arm64: dts: imx8mp: Enable HS400-ES
  2022-03-28 13:06                 ` Adam Ford
@ 2022-03-28 13:12                   ` Ahmad Fatoum
  -1 siblings, 0 replies; 44+ messages in thread
From: Ahmad Fatoum @ 2022-03-28 13:12 UTC (permalink / raw)
  To: Adam Ford, Krzysztof Kozlowski
  Cc: Krzysztof Kozlowski, linux-mmc, devicetree, Ulf Hansson,
	Fabio Estevam, Shawn Guo, Sascha Hauer, Adam Ford-BE, Haibo Chen,
	Linux Kernel Mailing List, Rob Herring, NXP Linux Team,
	Pengutronix Kernel Team, arm-soc

Hello Adam,

On 28.03.22 15:06, Adam Ford wrote:
> On Mon, Mar 28, 2022 at 7:56 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>
>> On 28/03/2022 14:45, Adam Ford wrote:
>>> On Mon, Mar 28, 2022 at 6:49 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>>>
>>>> On 28/03/2022 13:09, Ahmad Fatoum wrote:
>>>>> Hello Adam,
>>>>>
>>>>> On 28.03.22 12:47, Adam Ford wrote:
>>>>>> On Mon, Mar 28, 2022 at 2:20 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>>>>>>>
>>>>>>> Hello Adam,
>>>>>>>
>>>>>>> On 27.03.22 14:38, Adam Ford wrote:
>>>>>>>> The SDHC controller in the imx8mp has the same controller
>>>>>>>> as the imx8mm which supports HS400-ES. Change the compatible
>>>>>>>> fallback to imx8mm to enable it.
>>>>>>>
>>>>>>> I believe that's a shortcoming of the Linux driver, which should explicitly list
>>>>>>> fsl,imx8mp-usdhc in its compatibles and enable HS400-ES for it.
>>>>>>>
>>>>>>> I find dropping compatibles problematic, because like Linux matching
>>>>>>> fsl,imx8mm-usdhc, but not fsl,imx8mp-usdhc, other software may match
>>>>>>> fsl,imx7d-usdhc, but not fsl,imx8[mp]-usdhc.
>>>>>>>
>>>>>>> I'd prefer that either the kernel driver gains extra compatibles or that
>>>>>>> the DTS lists extra compatibles and we refrain from dropping existing
>>>>>>> (correct) ones.
>>>>>>>
>>>>>>
>>>>>> I would argue that imx7d is not correct since the IP blocks between
>>>>>> imx7d and imx8mm have different flags/quirks.  One of which includes
>>>>>> HS400-ES, but there are other differences as well.
>>>>>
>>>>> The DTS currently says that an fsl,imx7d-usdhc is a subset of an
>>>>> fsl,imx8mm-usdhc. So a driver could treat both HW the exact same
>>>>> by focusing on the i.MX7D parts. Linux apparently did exactly
>>>>> that so far. Is this not accurate?
>>>>>
>>>>>
>>>>>>> What do you think?
>>>>>>
>>>>>> From my understanding of the fallback compatibility strings is to
>>>>>> avoid having to add more and more compatible strings to the drivers
>>>>>> when they do not serve a functional purpose. Based On a conversation
>>>>>> with Krzysztof [1], he suggested we update the YAML file based on the
>>>>>> fallback, but he wanted NXP to give their feedback as to what the
>>>>>> right fallback strings should be.  Haibo from NXP sent me a hierarchy
>>>>>> [1] which is what I used to update the YAML file.  Based on the YAML
>>>>>> file, the fallback in each DTSI file was updated to ensure the use of
>>>>>> the proper IP block.
>>>>>
>>>>> Myself I am in favor of moving to three compatibles instead of dropping one.
>>>>> For some theoretical fsl,imx8mf-usdhc that's supposed to be exactly the same
>>>>> as a fsl,imx8mm-usdhc, I don't mind omitting the fsl,imx7d-usdhc compatible,
>>>>> but for existing device trees, this may introduce needless potential breakage
>>>>> for other software that also uses Linux device trees.
>>>>>
>>>>
>>>> Affecting existing users is indeed a concern with this approach, because
>>>> in-kernel DTS might be used in other projects as well.
>>>>
>>>> I still cannot find here the answer whether fsl,imx8mm-usdhc is actually
>>>> compatible with fsl,imx7d-usdhc. It's not about driver, but about
>>>> hardware and programming model. imx8mm can support additional features
>>>> and still be compatible with imx7d. However if any flags of imx7d are
>>>> actually not valid for imx8mm, then it's different case.
>>>
>>> The imx7d flags are:
>>>  ESDHC_FLAG_USDHC
>>> ESDHC_FLAG_STD_TUNING
>>>  ESDHC_FLAG_HAVE_CAP1
>>> ESDHC_FLAG_HS200
>>>  ESDHC_FLAG_HS400
>>>  ESDHC_FLAG_STATE_LOST_IN_LPMODE
>>>  ESDHC_FLAG_BROKEN_AUTO_CMD23,
>>>
>>> The imx8mm flags are:
>>>  ESDHC_FLAG_USDHC
>>>  ESDHC_FLAG_STD_TUNING
>>>  ESDHC_FLAG_HAVE_CAP1
>>> ESDHC_FLAG_HS200
>>>  ESDHC_FLAG_HS400
>>>  ESDHC_FLAG_HS400_ES
>>>  ESDHC_FLAG_STATE_LOST_IN_LPMODE
>>>
>>> It does not have the ESDHC_FLAG_BROKEN_AUTO_CMD23 that is present in the imx7d.
>>
>> AFAIU, it looks imx8mm is compatible with imx7d, because the broken
>> acmd23 only limits the features. If imx8mm binds according to imx7d, it
>> will not support acmd23 and HS400-ES.
>>
>> Having three compatibles is therefore also OK.
>>
>> You could also add two cases:
>> 1. three compatibles, deprecated: True,
>> 2. two compatibles, without imx7d.
>>
>> Existing DTS stays with three compatibles for two years and later gets
>> converted to two compatibles. New DTS should use two compatibles.
>>
>> It's quite a lot of churn, but would make in the long term bindings
>> correct and also not break other users/projects.
>>
>>>
>>> Maybe Haibo can comment on whether or not that would be an issue for the 8m[mnp]
>>>
>>> I will defer to Krzysztof and Haibo as to the proper method that we
>>> should add HS400-ES.  I don't have an issue adding the imx8mn or
>>> imx8mp compatible flags to the esdhc driver if that's the decision.
>>
>> I don't get what's the problem with HS400-ES. In any case (your patch
>> here, other ideas) your DTS will bind to imx8mm-usdhc which has HS400-ES.
> 
> I was under the impression Ahmad didn't want me to add the imx8mm
> compatible to the DTS, but instead, add the imx8mp compatible into the
> driver so it binds directly to an imx8mp.
> Based on that, I was assuming this patch series would be rejected and
> I sould focus on just the driver file itself

My objection is about dropping the compatible. I am fine with either:

 - adding fsl,imx8mm-usdhc as an additional compatible
 - extending the Linux driver

I am myself unsure, which is the preferred way, but both resolve your issue,
while not breaking other so far valid uses.

Cheers,
Ahmad

> From what I am reading from you, I should make the imx8mn and imx8mp
> device trees respectively read:
> 
> compatible = "fsl,imx8mn-usdhc", "fsl,imx8mm-usdhc", "fsl,imx7d-usdhc";
> 
> and
> 
> compatible = "fsl,imx8mp-usdhc", "fsl,imx8mm-usdhc", "fsl,imx7d-usdhc";
> 
> If that's true, do I need to change the YAML at all?
> 
> adam
>>
>> Best regards,
>> Krzysztof
> 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

* Re: [PATCH 3/5] arm64: dts: imx8mp: Enable HS400-ES
@ 2022-03-28 13:12                   ` Ahmad Fatoum
  0 siblings, 0 replies; 44+ messages in thread
From: Ahmad Fatoum @ 2022-03-28 13:12 UTC (permalink / raw)
  To: Adam Ford, Krzysztof Kozlowski
  Cc: Krzysztof Kozlowski, linux-mmc, devicetree, Ulf Hansson,
	Fabio Estevam, Shawn Guo, Sascha Hauer, Adam Ford-BE, Haibo Chen,
	Linux Kernel Mailing List, Rob Herring, NXP Linux Team,
	Pengutronix Kernel Team, arm-soc

Hello Adam,

On 28.03.22 15:06, Adam Ford wrote:
> On Mon, Mar 28, 2022 at 7:56 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>
>> On 28/03/2022 14:45, Adam Ford wrote:
>>> On Mon, Mar 28, 2022 at 6:49 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>>>
>>>> On 28/03/2022 13:09, Ahmad Fatoum wrote:
>>>>> Hello Adam,
>>>>>
>>>>> On 28.03.22 12:47, Adam Ford wrote:
>>>>>> On Mon, Mar 28, 2022 at 2:20 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>>>>>>>
>>>>>>> Hello Adam,
>>>>>>>
>>>>>>> On 27.03.22 14:38, Adam Ford wrote:
>>>>>>>> The SDHC controller in the imx8mp has the same controller
>>>>>>>> as the imx8mm which supports HS400-ES. Change the compatible
>>>>>>>> fallback to imx8mm to enable it.
>>>>>>>
>>>>>>> I believe that's a shortcoming of the Linux driver, which should explicitly list
>>>>>>> fsl,imx8mp-usdhc in its compatibles and enable HS400-ES for it.
>>>>>>>
>>>>>>> I find dropping compatibles problematic, because like Linux matching
>>>>>>> fsl,imx8mm-usdhc, but not fsl,imx8mp-usdhc, other software may match
>>>>>>> fsl,imx7d-usdhc, but not fsl,imx8[mp]-usdhc.
>>>>>>>
>>>>>>> I'd prefer that either the kernel driver gains extra compatibles or that
>>>>>>> the DTS lists extra compatibles and we refrain from dropping existing
>>>>>>> (correct) ones.
>>>>>>>
>>>>>>
>>>>>> I would argue that imx7d is not correct since the IP blocks between
>>>>>> imx7d and imx8mm have different flags/quirks.  One of which includes
>>>>>> HS400-ES, but there are other differences as well.
>>>>>
>>>>> The DTS currently says that an fsl,imx7d-usdhc is a subset of an
>>>>> fsl,imx8mm-usdhc. So a driver could treat both HW the exact same
>>>>> by focusing on the i.MX7D parts. Linux apparently did exactly
>>>>> that so far. Is this not accurate?
>>>>>
>>>>>
>>>>>>> What do you think?
>>>>>>
>>>>>> From my understanding of the fallback compatibility strings is to
>>>>>> avoid having to add more and more compatible strings to the drivers
>>>>>> when they do not serve a functional purpose. Based On a conversation
>>>>>> with Krzysztof [1], he suggested we update the YAML file based on the
>>>>>> fallback, but he wanted NXP to give their feedback as to what the
>>>>>> right fallback strings should be.  Haibo from NXP sent me a hierarchy
>>>>>> [1] which is what I used to update the YAML file.  Based on the YAML
>>>>>> file, the fallback in each DTSI file was updated to ensure the use of
>>>>>> the proper IP block.
>>>>>
>>>>> Myself I am in favor of moving to three compatibles instead of dropping one.
>>>>> For some theoretical fsl,imx8mf-usdhc that's supposed to be exactly the same
>>>>> as a fsl,imx8mm-usdhc, I don't mind omitting the fsl,imx7d-usdhc compatible,
>>>>> but for existing device trees, this may introduce needless potential breakage
>>>>> for other software that also uses Linux device trees.
>>>>>
>>>>
>>>> Affecting existing users is indeed a concern with this approach, because
>>>> in-kernel DTS might be used in other projects as well.
>>>>
>>>> I still cannot find here the answer whether fsl,imx8mm-usdhc is actually
>>>> compatible with fsl,imx7d-usdhc. It's not about driver, but about
>>>> hardware and programming model. imx8mm can support additional features
>>>> and still be compatible with imx7d. However if any flags of imx7d are
>>>> actually not valid for imx8mm, then it's different case.
>>>
>>> The imx7d flags are:
>>>  ESDHC_FLAG_USDHC
>>> ESDHC_FLAG_STD_TUNING
>>>  ESDHC_FLAG_HAVE_CAP1
>>> ESDHC_FLAG_HS200
>>>  ESDHC_FLAG_HS400
>>>  ESDHC_FLAG_STATE_LOST_IN_LPMODE
>>>  ESDHC_FLAG_BROKEN_AUTO_CMD23,
>>>
>>> The imx8mm flags are:
>>>  ESDHC_FLAG_USDHC
>>>  ESDHC_FLAG_STD_TUNING
>>>  ESDHC_FLAG_HAVE_CAP1
>>> ESDHC_FLAG_HS200
>>>  ESDHC_FLAG_HS400
>>>  ESDHC_FLAG_HS400_ES
>>>  ESDHC_FLAG_STATE_LOST_IN_LPMODE
>>>
>>> It does not have the ESDHC_FLAG_BROKEN_AUTO_CMD23 that is present in the imx7d.
>>
>> AFAIU, it looks imx8mm is compatible with imx7d, because the broken
>> acmd23 only limits the features. If imx8mm binds according to imx7d, it
>> will not support acmd23 and HS400-ES.
>>
>> Having three compatibles is therefore also OK.
>>
>> You could also add two cases:
>> 1. three compatibles, deprecated: True,
>> 2. two compatibles, without imx7d.
>>
>> Existing DTS stays with three compatibles for two years and later gets
>> converted to two compatibles. New DTS should use two compatibles.
>>
>> It's quite a lot of churn, but would make in the long term bindings
>> correct and also not break other users/projects.
>>
>>>
>>> Maybe Haibo can comment on whether or not that would be an issue for the 8m[mnp]
>>>
>>> I will defer to Krzysztof and Haibo as to the proper method that we
>>> should add HS400-ES.  I don't have an issue adding the imx8mn or
>>> imx8mp compatible flags to the esdhc driver if that's the decision.
>>
>> I don't get what's the problem with HS400-ES. In any case (your patch
>> here, other ideas) your DTS will bind to imx8mm-usdhc which has HS400-ES.
> 
> I was under the impression Ahmad didn't want me to add the imx8mm
> compatible to the DTS, but instead, add the imx8mp compatible into the
> driver so it binds directly to an imx8mp.
> Based on that, I was assuming this patch series would be rejected and
> I sould focus on just the driver file itself

My objection is about dropping the compatible. I am fine with either:

 - adding fsl,imx8mm-usdhc as an additional compatible
 - extending the Linux driver

I am myself unsure, which is the preferred way, but both resolve your issue,
while not breaking other so far valid uses.

Cheers,
Ahmad

> From what I am reading from you, I should make the imx8mn and imx8mp
> device trees respectively read:
> 
> compatible = "fsl,imx8mn-usdhc", "fsl,imx8mm-usdhc", "fsl,imx7d-usdhc";
> 
> and
> 
> compatible = "fsl,imx8mp-usdhc", "fsl,imx8mm-usdhc", "fsl,imx7d-usdhc";
> 
> If that's true, do I need to change the YAML at all?
> 
> adam
>>
>> Best regards,
>> Krzysztof
> 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

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

* Re: [PATCH 3/5] arm64: dts: imx8mp: Enable HS400-ES
  2022-03-28 13:07                 ` Ahmad Fatoum
@ 2022-03-28 13:14                   ` Krzysztof Kozlowski
  -1 siblings, 0 replies; 44+ messages in thread
From: Krzysztof Kozlowski @ 2022-03-28 13:14 UTC (permalink / raw)
  To: Ahmad Fatoum, Adam Ford
  Cc: Krzysztof Kozlowski, linux-mmc, devicetree, Ulf Hansson,
	Fabio Estevam, Shawn Guo, Sascha Hauer, Adam Ford-BE, Haibo Chen,
	Linux Kernel Mailing List, Rob Herring, NXP Linux Team,
	Pengutronix Kernel Team, arm-soc

On 28/03/2022 15:07, Ahmad Fatoum wrote:
> Hello Krzysztof,
> 
> On 28.03.22 14:56, Krzysztof Kozlowski wrote:
>> On 28/03/2022 14:45, Adam Ford wrote:
>>> On Mon, Mar 28, 2022 at 6:49 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>>>
>>>> On 28/03/2022 13:09, Ahmad Fatoum wrote:
>>>>> Hello Adam,
>>>>>
>>>>> On 28.03.22 12:47, Adam Ford wrote:
>>>>>> On Mon, Mar 28, 2022 at 2:20 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>>>>>>>
>>>>>>> Hello Adam,
>>>>>>>
>>>>>>> On 27.03.22 14:38, Adam Ford wrote:
>>>>>>>> The SDHC controller in the imx8mp has the same controller
>>>>>>>> as the imx8mm which supports HS400-ES. Change the compatible
>>>>>>>> fallback to imx8mm to enable it.
>>>>>>>
>>>>>>> I believe that's a shortcoming of the Linux driver, which should explicitly list
>>>>>>> fsl,imx8mp-usdhc in its compatibles and enable HS400-ES for it.
>>>>>>>
>>>>>>> I find dropping compatibles problematic, because like Linux matching
>>>>>>> fsl,imx8mm-usdhc, but not fsl,imx8mp-usdhc, other software may match
>>>>>>> fsl,imx7d-usdhc, but not fsl,imx8[mp]-usdhc.
>>>>>>>
>>>>>>> I'd prefer that either the kernel driver gains extra compatibles or that
>>>>>>> the DTS lists extra compatibles and we refrain from dropping existing
>>>>>>> (correct) ones.
>>>>>>>
>>>>>>
>>>>>> I would argue that imx7d is not correct since the IP blocks between
>>>>>> imx7d and imx8mm have different flags/quirks.  One of which includes
>>>>>> HS400-ES, but there are other differences as well.
>>>>>
>>>>> The DTS currently says that an fsl,imx7d-usdhc is a subset of an
>>>>> fsl,imx8mm-usdhc. So a driver could treat both HW the exact same
>>>>> by focusing on the i.MX7D parts. Linux apparently did exactly
>>>>> that so far. Is this not accurate?
>>>>>
>>>>>
>>>>>>> What do you think?
>>>>>>
>>>>>> From my understanding of the fallback compatibility strings is to
>>>>>> avoid having to add more and more compatible strings to the drivers
>>>>>> when they do not serve a functional purpose. Based On a conversation
>>>>>> with Krzysztof [1], he suggested we update the YAML file based on the
>>>>>> fallback, but he wanted NXP to give their feedback as to what the
>>>>>> right fallback strings should be.  Haibo from NXP sent me a hierarchy
>>>>>> [1] which is what I used to update the YAML file.  Based on the YAML
>>>>>> file, the fallback in each DTSI file was updated to ensure the use of
>>>>>> the proper IP block.
>>>>>
>>>>> Myself I am in favor of moving to three compatibles instead of dropping one.
>>>>> For some theoretical fsl,imx8mf-usdhc that's supposed to be exactly the same
>>>>> as a fsl,imx8mm-usdhc, I don't mind omitting the fsl,imx7d-usdhc compatible,
>>>>> but for existing device trees, this may introduce needless potential breakage
>>>>> for other software that also uses Linux device trees.
>>>>>
>>>>
>>>> Affecting existing users is indeed a concern with this approach, because
>>>> in-kernel DTS might be used in other projects as well.
>>>>
>>>> I still cannot find here the answer whether fsl,imx8mm-usdhc is actually
>>>> compatible with fsl,imx7d-usdhc. It's not about driver, but about
>>>> hardware and programming model. imx8mm can support additional features
>>>> and still be compatible with imx7d. However if any flags of imx7d are
>>>> actually not valid for imx8mm, then it's different case.
>>>
>>> The imx7d flags are:
>>>  ESDHC_FLAG_USDHC
>>> ESDHC_FLAG_STD_TUNING
>>>  ESDHC_FLAG_HAVE_CAP1
>>> ESDHC_FLAG_HS200
>>>  ESDHC_FLAG_HS400
>>>  ESDHC_FLAG_STATE_LOST_IN_LPMODE
>>>  ESDHC_FLAG_BROKEN_AUTO_CMD23,
>>>
>>> The imx8mm flags are:
>>>  ESDHC_FLAG_USDHC
>>>  ESDHC_FLAG_STD_TUNING
>>>  ESDHC_FLAG_HAVE_CAP1
>>> ESDHC_FLAG_HS200
>>>  ESDHC_FLAG_HS400
>>>  ESDHC_FLAG_HS400_ES
>>>  ESDHC_FLAG_STATE_LOST_IN_LPMODE
>>>
>>> It does not have the ESDHC_FLAG_BROKEN_AUTO_CMD23 that is present in the imx7d.
>>
>> AFAIU, it looks imx8mm is compatible with imx7d, because the broken
>> acmd23 only limits the features. If imx8mm binds according to imx7d, it
>> will not support acmd23 and HS400-ES.
>>
>> Having three compatibles is therefore also OK.
> 
> My thoughts, exactly.
> 
>> You could also add two cases:
>> 1. three compatibles, deprecated: True,
>> 2. two compatibles, without imx7d.
>>
>> Existing DTS stays with three compatibles for two years and later gets
>> converted to two compatibles. New DTS should use two compatibles.
>>
>> It's quite a lot of churn, but would make in the long term bindings
>> correct and also not break other users/projects.
> 
> I don't see why we need to deprecate the old binding. New SoCs
> can be imx8mm-usdhc compatible from the beginning and need not
> care about the old binding. Existing SoCs can just remain imx7d-usdhc
> compatible as they are now.
> 
> I don't see what the deprecation accomplishes.

It avoids to have too many entries of imx8mm (imx8mm alone,
imx8mm+imx7d, imx8xx+imx8mm+imx7d).

Best regards,
Krzysztof

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

* Re: [PATCH 3/5] arm64: dts: imx8mp: Enable HS400-ES
@ 2022-03-28 13:14                   ` Krzysztof Kozlowski
  0 siblings, 0 replies; 44+ messages in thread
From: Krzysztof Kozlowski @ 2022-03-28 13:14 UTC (permalink / raw)
  To: Ahmad Fatoum, Adam Ford
  Cc: Krzysztof Kozlowski, linux-mmc, devicetree, Ulf Hansson,
	Fabio Estevam, Shawn Guo, Sascha Hauer, Adam Ford-BE, Haibo Chen,
	Linux Kernel Mailing List, Rob Herring, NXP Linux Team,
	Pengutronix Kernel Team, arm-soc

On 28/03/2022 15:07, Ahmad Fatoum wrote:
> Hello Krzysztof,
> 
> On 28.03.22 14:56, Krzysztof Kozlowski wrote:
>> On 28/03/2022 14:45, Adam Ford wrote:
>>> On Mon, Mar 28, 2022 at 6:49 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>>>
>>>> On 28/03/2022 13:09, Ahmad Fatoum wrote:
>>>>> Hello Adam,
>>>>>
>>>>> On 28.03.22 12:47, Adam Ford wrote:
>>>>>> On Mon, Mar 28, 2022 at 2:20 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>>>>>>>
>>>>>>> Hello Adam,
>>>>>>>
>>>>>>> On 27.03.22 14:38, Adam Ford wrote:
>>>>>>>> The SDHC controller in the imx8mp has the same controller
>>>>>>>> as the imx8mm which supports HS400-ES. Change the compatible
>>>>>>>> fallback to imx8mm to enable it.
>>>>>>>
>>>>>>> I believe that's a shortcoming of the Linux driver, which should explicitly list
>>>>>>> fsl,imx8mp-usdhc in its compatibles and enable HS400-ES for it.
>>>>>>>
>>>>>>> I find dropping compatibles problematic, because like Linux matching
>>>>>>> fsl,imx8mm-usdhc, but not fsl,imx8mp-usdhc, other software may match
>>>>>>> fsl,imx7d-usdhc, but not fsl,imx8[mp]-usdhc.
>>>>>>>
>>>>>>> I'd prefer that either the kernel driver gains extra compatibles or that
>>>>>>> the DTS lists extra compatibles and we refrain from dropping existing
>>>>>>> (correct) ones.
>>>>>>>
>>>>>>
>>>>>> I would argue that imx7d is not correct since the IP blocks between
>>>>>> imx7d and imx8mm have different flags/quirks.  One of which includes
>>>>>> HS400-ES, but there are other differences as well.
>>>>>
>>>>> The DTS currently says that an fsl,imx7d-usdhc is a subset of an
>>>>> fsl,imx8mm-usdhc. So a driver could treat both HW the exact same
>>>>> by focusing on the i.MX7D parts. Linux apparently did exactly
>>>>> that so far. Is this not accurate?
>>>>>
>>>>>
>>>>>>> What do you think?
>>>>>>
>>>>>> From my understanding of the fallback compatibility strings is to
>>>>>> avoid having to add more and more compatible strings to the drivers
>>>>>> when they do not serve a functional purpose. Based On a conversation
>>>>>> with Krzysztof [1], he suggested we update the YAML file based on the
>>>>>> fallback, but he wanted NXP to give their feedback as to what the
>>>>>> right fallback strings should be.  Haibo from NXP sent me a hierarchy
>>>>>> [1] which is what I used to update the YAML file.  Based on the YAML
>>>>>> file, the fallback in each DTSI file was updated to ensure the use of
>>>>>> the proper IP block.
>>>>>
>>>>> Myself I am in favor of moving to three compatibles instead of dropping one.
>>>>> For some theoretical fsl,imx8mf-usdhc that's supposed to be exactly the same
>>>>> as a fsl,imx8mm-usdhc, I don't mind omitting the fsl,imx7d-usdhc compatible,
>>>>> but for existing device trees, this may introduce needless potential breakage
>>>>> for other software that also uses Linux device trees.
>>>>>
>>>>
>>>> Affecting existing users is indeed a concern with this approach, because
>>>> in-kernel DTS might be used in other projects as well.
>>>>
>>>> I still cannot find here the answer whether fsl,imx8mm-usdhc is actually
>>>> compatible with fsl,imx7d-usdhc. It's not about driver, but about
>>>> hardware and programming model. imx8mm can support additional features
>>>> and still be compatible with imx7d. However if any flags of imx7d are
>>>> actually not valid for imx8mm, then it's different case.
>>>
>>> The imx7d flags are:
>>>  ESDHC_FLAG_USDHC
>>> ESDHC_FLAG_STD_TUNING
>>>  ESDHC_FLAG_HAVE_CAP1
>>> ESDHC_FLAG_HS200
>>>  ESDHC_FLAG_HS400
>>>  ESDHC_FLAG_STATE_LOST_IN_LPMODE
>>>  ESDHC_FLAG_BROKEN_AUTO_CMD23,
>>>
>>> The imx8mm flags are:
>>>  ESDHC_FLAG_USDHC
>>>  ESDHC_FLAG_STD_TUNING
>>>  ESDHC_FLAG_HAVE_CAP1
>>> ESDHC_FLAG_HS200
>>>  ESDHC_FLAG_HS400
>>>  ESDHC_FLAG_HS400_ES
>>>  ESDHC_FLAG_STATE_LOST_IN_LPMODE
>>>
>>> It does not have the ESDHC_FLAG_BROKEN_AUTO_CMD23 that is present in the imx7d.
>>
>> AFAIU, it looks imx8mm is compatible with imx7d, because the broken
>> acmd23 only limits the features. If imx8mm binds according to imx7d, it
>> will not support acmd23 and HS400-ES.
>>
>> Having three compatibles is therefore also OK.
> 
> My thoughts, exactly.
> 
>> You could also add two cases:
>> 1. three compatibles, deprecated: True,
>> 2. two compatibles, without imx7d.
>>
>> Existing DTS stays with three compatibles for two years and later gets
>> converted to two compatibles. New DTS should use two compatibles.
>>
>> It's quite a lot of churn, but would make in the long term bindings
>> correct and also not break other users/projects.
> 
> I don't see why we need to deprecate the old binding. New SoCs
> can be imx8mm-usdhc compatible from the beginning and need not
> care about the old binding. Existing SoCs can just remain imx7d-usdhc
> compatible as they are now.
> 
> I don't see what the deprecation accomplishes.

It avoids to have too many entries of imx8mm (imx8mm alone,
imx8mm+imx7d, imx8xx+imx8mm+imx7d).

Best regards,
Krzysztof

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

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

* Re: [PATCH 3/5] arm64: dts: imx8mp: Enable HS400-ES
  2022-03-28 13:12                   ` Ahmad Fatoum
@ 2022-03-28 13:15                     ` Krzysztof Kozlowski
  -1 siblings, 0 replies; 44+ messages in thread
From: Krzysztof Kozlowski @ 2022-03-28 13:15 UTC (permalink / raw)
  To: Ahmad Fatoum, Adam Ford
  Cc: Krzysztof Kozlowski, linux-mmc, devicetree, Ulf Hansson,
	Fabio Estevam, Shawn Guo, Sascha Hauer, Adam Ford-BE, Haibo Chen,
	Linux Kernel Mailing List, Rob Herring, NXP Linux Team,
	Pengutronix Kernel Team, arm-soc

On 28/03/2022 15:12, Ahmad Fatoum wrote:
> Hello Adam,
> 
> On 28.03.22 15:06, Adam Ford wrote:
>> On Mon, Mar 28, 2022 at 7:56 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>>
>>> On 28/03/2022 14:45, Adam Ford wrote:
>>>> On Mon, Mar 28, 2022 at 6:49 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>>>>
>>>>> On 28/03/2022 13:09, Ahmad Fatoum wrote:
>>>>>> Hello Adam,
>>>>>>
>>>>>> On 28.03.22 12:47, Adam Ford wrote:
>>>>>>> On Mon, Mar 28, 2022 at 2:20 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>>>>>>>>
>>>>>>>> Hello Adam,
>>>>>>>>
>>>>>>>> On 27.03.22 14:38, Adam Ford wrote:
>>>>>>>>> The SDHC controller in the imx8mp has the same controller
>>>>>>>>> as the imx8mm which supports HS400-ES. Change the compatible
>>>>>>>>> fallback to imx8mm to enable it.
>>>>>>>>
>>>>>>>> I believe that's a shortcoming of the Linux driver, which should explicitly list
>>>>>>>> fsl,imx8mp-usdhc in its compatibles and enable HS400-ES for it.
>>>>>>>>
>>>>>>>> I find dropping compatibles problematic, because like Linux matching
>>>>>>>> fsl,imx8mm-usdhc, but not fsl,imx8mp-usdhc, other software may match
>>>>>>>> fsl,imx7d-usdhc, but not fsl,imx8[mp]-usdhc.
>>>>>>>>
>>>>>>>> I'd prefer that either the kernel driver gains extra compatibles or that
>>>>>>>> the DTS lists extra compatibles and we refrain from dropping existing
>>>>>>>> (correct) ones.
>>>>>>>>
>>>>>>>
>>>>>>> I would argue that imx7d is not correct since the IP blocks between
>>>>>>> imx7d and imx8mm have different flags/quirks.  One of which includes
>>>>>>> HS400-ES, but there are other differences as well.
>>>>>>
>>>>>> The DTS currently says that an fsl,imx7d-usdhc is a subset of an
>>>>>> fsl,imx8mm-usdhc. So a driver could treat both HW the exact same
>>>>>> by focusing on the i.MX7D parts. Linux apparently did exactly
>>>>>> that so far. Is this not accurate?
>>>>>>
>>>>>>
>>>>>>>> What do you think?
>>>>>>>
>>>>>>> From my understanding of the fallback compatibility strings is to
>>>>>>> avoid having to add more and more compatible strings to the drivers
>>>>>>> when they do not serve a functional purpose. Based On a conversation
>>>>>>> with Krzysztof [1], he suggested we update the YAML file based on the
>>>>>>> fallback, but he wanted NXP to give their feedback as to what the
>>>>>>> right fallback strings should be.  Haibo from NXP sent me a hierarchy
>>>>>>> [1] which is what I used to update the YAML file.  Based on the YAML
>>>>>>> file, the fallback in each DTSI file was updated to ensure the use of
>>>>>>> the proper IP block.
>>>>>>
>>>>>> Myself I am in favor of moving to three compatibles instead of dropping one.
>>>>>> For some theoretical fsl,imx8mf-usdhc that's supposed to be exactly the same
>>>>>> as a fsl,imx8mm-usdhc, I don't mind omitting the fsl,imx7d-usdhc compatible,
>>>>>> but for existing device trees, this may introduce needless potential breakage
>>>>>> for other software that also uses Linux device trees.
>>>>>>
>>>>>
>>>>> Affecting existing users is indeed a concern with this approach, because
>>>>> in-kernel DTS might be used in other projects as well.
>>>>>
>>>>> I still cannot find here the answer whether fsl,imx8mm-usdhc is actually
>>>>> compatible with fsl,imx7d-usdhc. It's not about driver, but about
>>>>> hardware and programming model. imx8mm can support additional features
>>>>> and still be compatible with imx7d. However if any flags of imx7d are
>>>>> actually not valid for imx8mm, then it's different case.
>>>>
>>>> The imx7d flags are:
>>>>  ESDHC_FLAG_USDHC
>>>> ESDHC_FLAG_STD_TUNING
>>>>  ESDHC_FLAG_HAVE_CAP1
>>>> ESDHC_FLAG_HS200
>>>>  ESDHC_FLAG_HS400
>>>>  ESDHC_FLAG_STATE_LOST_IN_LPMODE
>>>>  ESDHC_FLAG_BROKEN_AUTO_CMD23,
>>>>
>>>> The imx8mm flags are:
>>>>  ESDHC_FLAG_USDHC
>>>>  ESDHC_FLAG_STD_TUNING
>>>>  ESDHC_FLAG_HAVE_CAP1
>>>> ESDHC_FLAG_HS200
>>>>  ESDHC_FLAG_HS400
>>>>  ESDHC_FLAG_HS400_ES
>>>>  ESDHC_FLAG_STATE_LOST_IN_LPMODE
>>>>
>>>> It does not have the ESDHC_FLAG_BROKEN_AUTO_CMD23 that is present in the imx7d.
>>>
>>> AFAIU, it looks imx8mm is compatible with imx7d, because the broken
>>> acmd23 only limits the features. If imx8mm binds according to imx7d, it
>>> will not support acmd23 and HS400-ES.
>>>
>>> Having three compatibles is therefore also OK.
>>>
>>> You could also add two cases:
>>> 1. three compatibles, deprecated: True,
>>> 2. two compatibles, without imx7d.
>>>
>>> Existing DTS stays with three compatibles for two years and later gets
>>> converted to two compatibles. New DTS should use two compatibles.
>>>
>>> It's quite a lot of churn, but would make in the long term bindings
>>> correct and also not break other users/projects.
>>>
>>>>
>>>> Maybe Haibo can comment on whether or not that would be an issue for the 8m[mnp]
>>>>
>>>> I will defer to Krzysztof and Haibo as to the proper method that we
>>>> should add HS400-ES.  I don't have an issue adding the imx8mn or
>>>> imx8mp compatible flags to the esdhc driver if that's the decision.
>>>
>>> I don't get what's the problem with HS400-ES. In any case (your patch
>>> here, other ideas) your DTS will bind to imx8mm-usdhc which has HS400-ES.
>>
>> I was under the impression Ahmad didn't want me to add the imx8mm
>> compatible to the DTS, but instead, add the imx8mp compatible into the
>> driver so it binds directly to an imx8mp.

It's not really related the bindings, so don't ask me.

Best regards,
Krzysztof

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

* Re: [PATCH 3/5] arm64: dts: imx8mp: Enable HS400-ES
@ 2022-03-28 13:15                     ` Krzysztof Kozlowski
  0 siblings, 0 replies; 44+ messages in thread
From: Krzysztof Kozlowski @ 2022-03-28 13:15 UTC (permalink / raw)
  To: Ahmad Fatoum, Adam Ford
  Cc: Krzysztof Kozlowski, linux-mmc, devicetree, Ulf Hansson,
	Fabio Estevam, Shawn Guo, Sascha Hauer, Adam Ford-BE, Haibo Chen,
	Linux Kernel Mailing List, Rob Herring, NXP Linux Team,
	Pengutronix Kernel Team, arm-soc

On 28/03/2022 15:12, Ahmad Fatoum wrote:
> Hello Adam,
> 
> On 28.03.22 15:06, Adam Ford wrote:
>> On Mon, Mar 28, 2022 at 7:56 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>>
>>> On 28/03/2022 14:45, Adam Ford wrote:
>>>> On Mon, Mar 28, 2022 at 6:49 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>>>>
>>>>> On 28/03/2022 13:09, Ahmad Fatoum wrote:
>>>>>> Hello Adam,
>>>>>>
>>>>>> On 28.03.22 12:47, Adam Ford wrote:
>>>>>>> On Mon, Mar 28, 2022 at 2:20 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>>>>>>>>
>>>>>>>> Hello Adam,
>>>>>>>>
>>>>>>>> On 27.03.22 14:38, Adam Ford wrote:
>>>>>>>>> The SDHC controller in the imx8mp has the same controller
>>>>>>>>> as the imx8mm which supports HS400-ES. Change the compatible
>>>>>>>>> fallback to imx8mm to enable it.
>>>>>>>>
>>>>>>>> I believe that's a shortcoming of the Linux driver, which should explicitly list
>>>>>>>> fsl,imx8mp-usdhc in its compatibles and enable HS400-ES for it.
>>>>>>>>
>>>>>>>> I find dropping compatibles problematic, because like Linux matching
>>>>>>>> fsl,imx8mm-usdhc, but not fsl,imx8mp-usdhc, other software may match
>>>>>>>> fsl,imx7d-usdhc, but not fsl,imx8[mp]-usdhc.
>>>>>>>>
>>>>>>>> I'd prefer that either the kernel driver gains extra compatibles or that
>>>>>>>> the DTS lists extra compatibles and we refrain from dropping existing
>>>>>>>> (correct) ones.
>>>>>>>>
>>>>>>>
>>>>>>> I would argue that imx7d is not correct since the IP blocks between
>>>>>>> imx7d and imx8mm have different flags/quirks.  One of which includes
>>>>>>> HS400-ES, but there are other differences as well.
>>>>>>
>>>>>> The DTS currently says that an fsl,imx7d-usdhc is a subset of an
>>>>>> fsl,imx8mm-usdhc. So a driver could treat both HW the exact same
>>>>>> by focusing on the i.MX7D parts. Linux apparently did exactly
>>>>>> that so far. Is this not accurate?
>>>>>>
>>>>>>
>>>>>>>> What do you think?
>>>>>>>
>>>>>>> From my understanding of the fallback compatibility strings is to
>>>>>>> avoid having to add more and more compatible strings to the drivers
>>>>>>> when they do not serve a functional purpose. Based On a conversation
>>>>>>> with Krzysztof [1], he suggested we update the YAML file based on the
>>>>>>> fallback, but he wanted NXP to give their feedback as to what the
>>>>>>> right fallback strings should be.  Haibo from NXP sent me a hierarchy
>>>>>>> [1] which is what I used to update the YAML file.  Based on the YAML
>>>>>>> file, the fallback in each DTSI file was updated to ensure the use of
>>>>>>> the proper IP block.
>>>>>>
>>>>>> Myself I am in favor of moving to three compatibles instead of dropping one.
>>>>>> For some theoretical fsl,imx8mf-usdhc that's supposed to be exactly the same
>>>>>> as a fsl,imx8mm-usdhc, I don't mind omitting the fsl,imx7d-usdhc compatible,
>>>>>> but for existing device trees, this may introduce needless potential breakage
>>>>>> for other software that also uses Linux device trees.
>>>>>>
>>>>>
>>>>> Affecting existing users is indeed a concern with this approach, because
>>>>> in-kernel DTS might be used in other projects as well.
>>>>>
>>>>> I still cannot find here the answer whether fsl,imx8mm-usdhc is actually
>>>>> compatible with fsl,imx7d-usdhc. It's not about driver, but about
>>>>> hardware and programming model. imx8mm can support additional features
>>>>> and still be compatible with imx7d. However if any flags of imx7d are
>>>>> actually not valid for imx8mm, then it's different case.
>>>>
>>>> The imx7d flags are:
>>>>  ESDHC_FLAG_USDHC
>>>> ESDHC_FLAG_STD_TUNING
>>>>  ESDHC_FLAG_HAVE_CAP1
>>>> ESDHC_FLAG_HS200
>>>>  ESDHC_FLAG_HS400
>>>>  ESDHC_FLAG_STATE_LOST_IN_LPMODE
>>>>  ESDHC_FLAG_BROKEN_AUTO_CMD23,
>>>>
>>>> The imx8mm flags are:
>>>>  ESDHC_FLAG_USDHC
>>>>  ESDHC_FLAG_STD_TUNING
>>>>  ESDHC_FLAG_HAVE_CAP1
>>>> ESDHC_FLAG_HS200
>>>>  ESDHC_FLAG_HS400
>>>>  ESDHC_FLAG_HS400_ES
>>>>  ESDHC_FLAG_STATE_LOST_IN_LPMODE
>>>>
>>>> It does not have the ESDHC_FLAG_BROKEN_AUTO_CMD23 that is present in the imx7d.
>>>
>>> AFAIU, it looks imx8mm is compatible with imx7d, because the broken
>>> acmd23 only limits the features. If imx8mm binds according to imx7d, it
>>> will not support acmd23 and HS400-ES.
>>>
>>> Having three compatibles is therefore also OK.
>>>
>>> You could also add two cases:
>>> 1. three compatibles, deprecated: True,
>>> 2. two compatibles, without imx7d.
>>>
>>> Existing DTS stays with three compatibles for two years and later gets
>>> converted to two compatibles. New DTS should use two compatibles.
>>>
>>> It's quite a lot of churn, but would make in the long term bindings
>>> correct and also not break other users/projects.
>>>
>>>>
>>>> Maybe Haibo can comment on whether or not that would be an issue for the 8m[mnp]
>>>>
>>>> I will defer to Krzysztof and Haibo as to the proper method that we
>>>> should add HS400-ES.  I don't have an issue adding the imx8mn or
>>>> imx8mp compatible flags to the esdhc driver if that's the decision.
>>>
>>> I don't get what's the problem with HS400-ES. In any case (your patch
>>> here, other ideas) your DTS will bind to imx8mm-usdhc which has HS400-ES.
>>
>> I was under the impression Ahmad didn't want me to add the imx8mm
>> compatible to the DTS, but instead, add the imx8mp compatible into the
>> driver so it binds directly to an imx8mp.

It's not really related the bindings, so don't ask me.

Best regards,
Krzysztof

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

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

* Re: [PATCH 3/5] arm64: dts: imx8mp: Enable HS400-ES
  2022-03-28 13:06                 ` Adam Ford
@ 2022-03-28 13:17                   ` Krzysztof Kozlowski
  -1 siblings, 0 replies; 44+ messages in thread
From: Krzysztof Kozlowski @ 2022-03-28 13:17 UTC (permalink / raw)
  To: Adam Ford
  Cc: Ahmad Fatoum, Krzysztof Kozlowski, linux-mmc, devicetree,
	Ulf Hansson, Fabio Estevam, Shawn Guo, Sascha Hauer,
	Adam Ford-BE, Haibo Chen, Linux Kernel Mailing List, Rob Herring,
	NXP Linux Team, Pengutronix Kernel Team, arm-soc

On 28/03/2022 15:06, Adam Ford wrote:
> On Mon, Mar 28, 2022 at 7:56 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>
>> On 28/03/2022 14:45, Adam Ford wrote:
>>> On Mon, Mar 28, 2022 at 6:49 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>>>
>>>> On 28/03/2022 13:09, Ahmad Fatoum wrote:
>>>>> Hello Adam,
>>>>>
>>>>> On 28.03.22 12:47, Adam Ford wrote:
>>>>>> On Mon, Mar 28, 2022 at 2:20 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>>>>>>>
>>>>>>> Hello Adam,
>>>>>>>
>>>>>>> On 27.03.22 14:38, Adam Ford wrote:
>>>>>>>> The SDHC controller in the imx8mp has the same controller
>>>>>>>> as the imx8mm which supports HS400-ES. Change the compatible
>>>>>>>> fallback to imx8mm to enable it.
>>>>>>>
>>>>>>> I believe that's a shortcoming of the Linux driver, which should explicitly list
>>>>>>> fsl,imx8mp-usdhc in its compatibles and enable HS400-ES for it.
>>>>>>>
>>>>>>> I find dropping compatibles problematic, because like Linux matching
>>>>>>> fsl,imx8mm-usdhc, but not fsl,imx8mp-usdhc, other software may match
>>>>>>> fsl,imx7d-usdhc, but not fsl,imx8[mp]-usdhc.
>>>>>>>
>>>>>>> I'd prefer that either the kernel driver gains extra compatibles or that
>>>>>>> the DTS lists extra compatibles and we refrain from dropping existing
>>>>>>> (correct) ones.
>>>>>>>
>>>>>>
>>>>>> I would argue that imx7d is not correct since the IP blocks between
>>>>>> imx7d and imx8mm have different flags/quirks.  One of which includes
>>>>>> HS400-ES, but there are other differences as well.
>>>>>
>>>>> The DTS currently says that an fsl,imx7d-usdhc is a subset of an
>>>>> fsl,imx8mm-usdhc. So a driver could treat both HW the exact same
>>>>> by focusing on the i.MX7D parts. Linux apparently did exactly
>>>>> that so far. Is this not accurate?
>>>>>
>>>>>
>>>>>>> What do you think?
>>>>>>
>>>>>> From my understanding of the fallback compatibility strings is to
>>>>>> avoid having to add more and more compatible strings to the drivers
>>>>>> when they do not serve a functional purpose. Based On a conversation
>>>>>> with Krzysztof [1], he suggested we update the YAML file based on the
>>>>>> fallback, but he wanted NXP to give their feedback as to what the
>>>>>> right fallback strings should be.  Haibo from NXP sent me a hierarchy
>>>>>> [1] which is what I used to update the YAML file.  Based on the YAML
>>>>>> file, the fallback in each DTSI file was updated to ensure the use of
>>>>>> the proper IP block.
>>>>>
>>>>> Myself I am in favor of moving to three compatibles instead of dropping one.
>>>>> For some theoretical fsl,imx8mf-usdhc that's supposed to be exactly the same
>>>>> as a fsl,imx8mm-usdhc, I don't mind omitting the fsl,imx7d-usdhc compatible,
>>>>> but for existing device trees, this may introduce needless potential breakage
>>>>> for other software that also uses Linux device trees.
>>>>>
>>>>
>>>> Affecting existing users is indeed a concern with this approach, because
>>>> in-kernel DTS might be used in other projects as well.
>>>>
>>>> I still cannot find here the answer whether fsl,imx8mm-usdhc is actually
>>>> compatible with fsl,imx7d-usdhc. It's not about driver, but about
>>>> hardware and programming model. imx8mm can support additional features
>>>> and still be compatible with imx7d. However if any flags of imx7d are
>>>> actually not valid for imx8mm, then it's different case.
>>>
>>> The imx7d flags are:
>>>  ESDHC_FLAG_USDHC
>>> ESDHC_FLAG_STD_TUNING
>>>  ESDHC_FLAG_HAVE_CAP1
>>> ESDHC_FLAG_HS200
>>>  ESDHC_FLAG_HS400
>>>  ESDHC_FLAG_STATE_LOST_IN_LPMODE
>>>  ESDHC_FLAG_BROKEN_AUTO_CMD23,
>>>
>>> The imx8mm flags are:
>>>  ESDHC_FLAG_USDHC
>>>  ESDHC_FLAG_STD_TUNING
>>>  ESDHC_FLAG_HAVE_CAP1
>>> ESDHC_FLAG_HS200
>>>  ESDHC_FLAG_HS400
>>>  ESDHC_FLAG_HS400_ES
>>>  ESDHC_FLAG_STATE_LOST_IN_LPMODE
>>>
>>> It does not have the ESDHC_FLAG_BROKEN_AUTO_CMD23 that is present in the imx7d.
>>
>> AFAIU, it looks imx8mm is compatible with imx7d, because the broken
>> acmd23 only limits the features. If imx8mm binds according to imx7d, it
>> will not support acmd23 and HS400-ES.
>>
>> Having three compatibles is therefore also OK.
>>
>> You could also add two cases:
>> 1. three compatibles, deprecated: True,
>> 2. two compatibles, without imx7d.
>>
>> Existing DTS stays with three compatibles for two years and later gets
>> converted to two compatibles. New DTS should use two compatibles.
>>
>> It's quite a lot of churn, but would make in the long term bindings
>> correct and also not break other users/projects.
>>
>>>
>>> Maybe Haibo can comment on whether or not that would be an issue for the 8m[mnp]
>>>
>>> I will defer to Krzysztof and Haibo as to the proper method that we
>>> should add HS400-ES.  I don't have an issue adding the imx8mn or
>>> imx8mp compatible flags to the esdhc driver if that's the decision.
>>
>> I don't get what's the problem with HS400-ES. In any case (your patch
>> here, other ideas) your DTS will bind to imx8mm-usdhc which has HS400-ES.
> 
> I was under the impression Ahmad didn't want me to add the imx8mm
> compatible to the DTS, but instead, add the imx8mp compatible into the
> driver so it binds directly to an imx8mp.
> Based on that, I was assuming this patch series would be rejected and
> I sould focus on just the driver file itself
> 
> From what I am reading from you, I should make the imx8mn and imx8mp
> device trees respectively read:
> 
> compatible = "fsl,imx8mn-usdhc", "fsl,imx8mm-usdhc", "fsl,imx7d-usdhc";
> 
> and
> 
> compatible = "fsl,imx8mp-usdhc", "fsl,imx8mm-usdhc", "fsl,imx7d-usdhc";
> 
> If that's true, do I need to change the YAML at all?

Sorry, I do not remember all the Linux kernel bindings by heart, so you
need to check by yourself whether they cover this case or not.

Best regards,
Krzysztof

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

* Re: [PATCH 3/5] arm64: dts: imx8mp: Enable HS400-ES
@ 2022-03-28 13:17                   ` Krzysztof Kozlowski
  0 siblings, 0 replies; 44+ messages in thread
From: Krzysztof Kozlowski @ 2022-03-28 13:17 UTC (permalink / raw)
  To: Adam Ford
  Cc: Ahmad Fatoum, Krzysztof Kozlowski, linux-mmc, devicetree,
	Ulf Hansson, Fabio Estevam, Shawn Guo, Sascha Hauer,
	Adam Ford-BE, Haibo Chen, Linux Kernel Mailing List, Rob Herring,
	NXP Linux Team, Pengutronix Kernel Team, arm-soc

On 28/03/2022 15:06, Adam Ford wrote:
> On Mon, Mar 28, 2022 at 7:56 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>
>> On 28/03/2022 14:45, Adam Ford wrote:
>>> On Mon, Mar 28, 2022 at 6:49 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>>>
>>>> On 28/03/2022 13:09, Ahmad Fatoum wrote:
>>>>> Hello Adam,
>>>>>
>>>>> On 28.03.22 12:47, Adam Ford wrote:
>>>>>> On Mon, Mar 28, 2022 at 2:20 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>>>>>>>
>>>>>>> Hello Adam,
>>>>>>>
>>>>>>> On 27.03.22 14:38, Adam Ford wrote:
>>>>>>>> The SDHC controller in the imx8mp has the same controller
>>>>>>>> as the imx8mm which supports HS400-ES. Change the compatible
>>>>>>>> fallback to imx8mm to enable it.
>>>>>>>
>>>>>>> I believe that's a shortcoming of the Linux driver, which should explicitly list
>>>>>>> fsl,imx8mp-usdhc in its compatibles and enable HS400-ES for it.
>>>>>>>
>>>>>>> I find dropping compatibles problematic, because like Linux matching
>>>>>>> fsl,imx8mm-usdhc, but not fsl,imx8mp-usdhc, other software may match
>>>>>>> fsl,imx7d-usdhc, but not fsl,imx8[mp]-usdhc.
>>>>>>>
>>>>>>> I'd prefer that either the kernel driver gains extra compatibles or that
>>>>>>> the DTS lists extra compatibles and we refrain from dropping existing
>>>>>>> (correct) ones.
>>>>>>>
>>>>>>
>>>>>> I would argue that imx7d is not correct since the IP blocks between
>>>>>> imx7d and imx8mm have different flags/quirks.  One of which includes
>>>>>> HS400-ES, but there are other differences as well.
>>>>>
>>>>> The DTS currently says that an fsl,imx7d-usdhc is a subset of an
>>>>> fsl,imx8mm-usdhc. So a driver could treat both HW the exact same
>>>>> by focusing on the i.MX7D parts. Linux apparently did exactly
>>>>> that so far. Is this not accurate?
>>>>>
>>>>>
>>>>>>> What do you think?
>>>>>>
>>>>>> From my understanding of the fallback compatibility strings is to
>>>>>> avoid having to add more and more compatible strings to the drivers
>>>>>> when they do not serve a functional purpose. Based On a conversation
>>>>>> with Krzysztof [1], he suggested we update the YAML file based on the
>>>>>> fallback, but he wanted NXP to give their feedback as to what the
>>>>>> right fallback strings should be.  Haibo from NXP sent me a hierarchy
>>>>>> [1] which is what I used to update the YAML file.  Based on the YAML
>>>>>> file, the fallback in each DTSI file was updated to ensure the use of
>>>>>> the proper IP block.
>>>>>
>>>>> Myself I am in favor of moving to three compatibles instead of dropping one.
>>>>> For some theoretical fsl,imx8mf-usdhc that's supposed to be exactly the same
>>>>> as a fsl,imx8mm-usdhc, I don't mind omitting the fsl,imx7d-usdhc compatible,
>>>>> but for existing device trees, this may introduce needless potential breakage
>>>>> for other software that also uses Linux device trees.
>>>>>
>>>>
>>>> Affecting existing users is indeed a concern with this approach, because
>>>> in-kernel DTS might be used in other projects as well.
>>>>
>>>> I still cannot find here the answer whether fsl,imx8mm-usdhc is actually
>>>> compatible with fsl,imx7d-usdhc. It's not about driver, but about
>>>> hardware and programming model. imx8mm can support additional features
>>>> and still be compatible with imx7d. However if any flags of imx7d are
>>>> actually not valid for imx8mm, then it's different case.
>>>
>>> The imx7d flags are:
>>>  ESDHC_FLAG_USDHC
>>> ESDHC_FLAG_STD_TUNING
>>>  ESDHC_FLAG_HAVE_CAP1
>>> ESDHC_FLAG_HS200
>>>  ESDHC_FLAG_HS400
>>>  ESDHC_FLAG_STATE_LOST_IN_LPMODE
>>>  ESDHC_FLAG_BROKEN_AUTO_CMD23,
>>>
>>> The imx8mm flags are:
>>>  ESDHC_FLAG_USDHC
>>>  ESDHC_FLAG_STD_TUNING
>>>  ESDHC_FLAG_HAVE_CAP1
>>> ESDHC_FLAG_HS200
>>>  ESDHC_FLAG_HS400
>>>  ESDHC_FLAG_HS400_ES
>>>  ESDHC_FLAG_STATE_LOST_IN_LPMODE
>>>
>>> It does not have the ESDHC_FLAG_BROKEN_AUTO_CMD23 that is present in the imx7d.
>>
>> AFAIU, it looks imx8mm is compatible with imx7d, because the broken
>> acmd23 only limits the features. If imx8mm binds according to imx7d, it
>> will not support acmd23 and HS400-ES.
>>
>> Having three compatibles is therefore also OK.
>>
>> You could also add two cases:
>> 1. three compatibles, deprecated: True,
>> 2. two compatibles, without imx7d.
>>
>> Existing DTS stays with three compatibles for two years and later gets
>> converted to two compatibles. New DTS should use two compatibles.
>>
>> It's quite a lot of churn, but would make in the long term bindings
>> correct and also not break other users/projects.
>>
>>>
>>> Maybe Haibo can comment on whether or not that would be an issue for the 8m[mnp]
>>>
>>> I will defer to Krzysztof and Haibo as to the proper method that we
>>> should add HS400-ES.  I don't have an issue adding the imx8mn or
>>> imx8mp compatible flags to the esdhc driver if that's the decision.
>>
>> I don't get what's the problem with HS400-ES. In any case (your patch
>> here, other ideas) your DTS will bind to imx8mm-usdhc which has HS400-ES.
> 
> I was under the impression Ahmad didn't want me to add the imx8mm
> compatible to the DTS, but instead, add the imx8mp compatible into the
> driver so it binds directly to an imx8mp.
> Based on that, I was assuming this patch series would be rejected and
> I sould focus on just the driver file itself
> 
> From what I am reading from you, I should make the imx8mn and imx8mp
> device trees respectively read:
> 
> compatible = "fsl,imx8mn-usdhc", "fsl,imx8mm-usdhc", "fsl,imx7d-usdhc";
> 
> and
> 
> compatible = "fsl,imx8mp-usdhc", "fsl,imx8mm-usdhc", "fsl,imx7d-usdhc";
> 
> If that's true, do I need to change the YAML at all?

Sorry, I do not remember all the Linux kernel bindings by heart, so you
need to check by yourself whether they cover this case or not.

Best regards,
Krzysztof

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

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

* Re: [PATCH 3/5] arm64: dts: imx8mp: Enable HS400-ES
  2022-03-28 13:14                   ` Krzysztof Kozlowski
@ 2022-03-28 13:42                     ` Ahmad Fatoum
  -1 siblings, 0 replies; 44+ messages in thread
From: Ahmad Fatoum @ 2022-03-28 13:42 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Adam Ford
  Cc: Krzysztof Kozlowski, linux-mmc, devicetree, Ulf Hansson,
	Fabio Estevam, Shawn Guo, Sascha Hauer, Adam Ford-BE, Haibo Chen,
	Linux Kernel Mailing List, Rob Herring, NXP Linux Team,
	Pengutronix Kernel Team, arm-soc

On 28.03.22 15:14, Krzysztof Kozlowski wrote:
> On 28/03/2022 15:07, Ahmad Fatoum wrote:
>> On 28.03.22 14:56, Krzysztof Kozlowski wrote:
>>> You could also add two cases:
>>> 1. three compatibles, deprecated: True,
>>> 2. two compatibles, without imx7d.
>>>
>>> Existing DTS stays with three compatibles for two years and later gets
>>> converted to two compatibles. New DTS should use two compatibles.
>>>
>>> It's quite a lot of churn, but would make in the long term bindings
>>> correct and also not break other users/projects.
>>
>> I don't see why we need to deprecate the old binding. New SoCs
>> can be imx8mm-usdhc compatible from the beginning and need not
>> care about the old binding. Existing SoCs can just remain imx7d-usdhc
>> compatible as they are now.
>>
>> I don't see what the deprecation accomplishes.
> 
> It avoids to have too many entries of imx8mm (imx8mm alone,
> imx8mm+imx7d, imx8xx+imx8mm+imx7d).

I see. I assume use of deprecated binding will be reported on a dtbs_check?

If so, the expectation is that downstream projects run dtbs_check on their
imported Linux DT repository, see the deprecation warning and extend
their drivers to comply with it.

Some time later upstream will remove the deprecated binding and adjust
the device trees. This works for me.

Cheers,
Ahmad


> 
> Best regards,
> Krzysztof
> 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

* Re: [PATCH 3/5] arm64: dts: imx8mp: Enable HS400-ES
@ 2022-03-28 13:42                     ` Ahmad Fatoum
  0 siblings, 0 replies; 44+ messages in thread
From: Ahmad Fatoum @ 2022-03-28 13:42 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Adam Ford
  Cc: Krzysztof Kozlowski, linux-mmc, devicetree, Ulf Hansson,
	Fabio Estevam, Shawn Guo, Sascha Hauer, Adam Ford-BE, Haibo Chen,
	Linux Kernel Mailing List, Rob Herring, NXP Linux Team,
	Pengutronix Kernel Team, arm-soc

On 28.03.22 15:14, Krzysztof Kozlowski wrote:
> On 28/03/2022 15:07, Ahmad Fatoum wrote:
>> On 28.03.22 14:56, Krzysztof Kozlowski wrote:
>>> You could also add two cases:
>>> 1. three compatibles, deprecated: True,
>>> 2. two compatibles, without imx7d.
>>>
>>> Existing DTS stays with three compatibles for two years and later gets
>>> converted to two compatibles. New DTS should use two compatibles.
>>>
>>> It's quite a lot of churn, but would make in the long term bindings
>>> correct and also not break other users/projects.
>>
>> I don't see why we need to deprecate the old binding. New SoCs
>> can be imx8mm-usdhc compatible from the beginning and need not
>> care about the old binding. Existing SoCs can just remain imx7d-usdhc
>> compatible as they are now.
>>
>> I don't see what the deprecation accomplishes.
> 
> It avoids to have too many entries of imx8mm (imx8mm alone,
> imx8mm+imx7d, imx8xx+imx8mm+imx7d).

I see. I assume use of deprecated binding will be reported on a dtbs_check?

If so, the expectation is that downstream projects run dtbs_check on their
imported Linux DT repository, see the deprecation warning and extend
their drivers to comply with it.

Some time later upstream will remove the deprecated binding and adjust
the device trees. This works for me.

Cheers,
Ahmad


> 
> Best regards,
> Krzysztof
> 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

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

* Re: [PATCH 3/5] arm64: dts: imx8mp: Enable HS400-ES
  2022-03-28 13:42                     ` Ahmad Fatoum
@ 2022-03-28 17:35                       ` Krzysztof Kozlowski
  -1 siblings, 0 replies; 44+ messages in thread
From: Krzysztof Kozlowski @ 2022-03-28 17:35 UTC (permalink / raw)
  To: Ahmad Fatoum, Adam Ford
  Cc: Krzysztof Kozlowski, linux-mmc, devicetree, Ulf Hansson,
	Fabio Estevam, Shawn Guo, Sascha Hauer, Adam Ford-BE, Haibo Chen,
	Linux Kernel Mailing List, Rob Herring, NXP Linux Team,
	Pengutronix Kernel Team, arm-soc

On 28/03/2022 15:42, Ahmad Fatoum wrote:
> On 28.03.22 15:14, Krzysztof Kozlowski wrote:
>> On 28/03/2022 15:07, Ahmad Fatoum wrote:
>>> On 28.03.22 14:56, Krzysztof Kozlowski wrote:
>>>> You could also add two cases:
>>>> 1. three compatibles, deprecated: True,
>>>> 2. two compatibles, without imx7d.
>>>>
>>>> Existing DTS stays with three compatibles for two years and later gets
>>>> converted to two compatibles. New DTS should use two compatibles.
>>>>
>>>> It's quite a lot of churn, but would make in the long term bindings
>>>> correct and also not break other users/projects.
>>>
>>> I don't see why we need to deprecate the old binding. New SoCs
>>> can be imx8mm-usdhc compatible from the beginning and need not
>>> care about the old binding. Existing SoCs can just remain imx7d-usdhc
>>> compatible as they are now.
>>>
>>> I don't see what the deprecation accomplishes.
>>
>> It avoids to have too many entries of imx8mm (imx8mm alone,
>> imx8mm+imx7d, imx8xx+imx8mm+imx7d).
> 
> I see. I assume use of deprecated binding will be reported on a dtbs_check?

Unfortunately no, at least not yet. :-(

> If so, the expectation is that downstream projects run dtbs_check on their
> imported Linux DT repository, see the deprecation warning and extend
> their drivers to comply with it.
> 
> Some time later upstream will remove the deprecated binding and adjust
> the device trees. This works for me.
>

Yes. Plus this gives the notice to downstream projects, to comply with
the change of DTS.

Best regards,
Krzysztof

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

* Re: [PATCH 3/5] arm64: dts: imx8mp: Enable HS400-ES
@ 2022-03-28 17:35                       ` Krzysztof Kozlowski
  0 siblings, 0 replies; 44+ messages in thread
From: Krzysztof Kozlowski @ 2022-03-28 17:35 UTC (permalink / raw)
  To: Ahmad Fatoum, Adam Ford
  Cc: Krzysztof Kozlowski, linux-mmc, devicetree, Ulf Hansson,
	Fabio Estevam, Shawn Guo, Sascha Hauer, Adam Ford-BE, Haibo Chen,
	Linux Kernel Mailing List, Rob Herring, NXP Linux Team,
	Pengutronix Kernel Team, arm-soc

On 28/03/2022 15:42, Ahmad Fatoum wrote:
> On 28.03.22 15:14, Krzysztof Kozlowski wrote:
>> On 28/03/2022 15:07, Ahmad Fatoum wrote:
>>> On 28.03.22 14:56, Krzysztof Kozlowski wrote:
>>>> You could also add two cases:
>>>> 1. three compatibles, deprecated: True,
>>>> 2. two compatibles, without imx7d.
>>>>
>>>> Existing DTS stays with three compatibles for two years and later gets
>>>> converted to two compatibles. New DTS should use two compatibles.
>>>>
>>>> It's quite a lot of churn, but would make in the long term bindings
>>>> correct and also not break other users/projects.
>>>
>>> I don't see why we need to deprecate the old binding. New SoCs
>>> can be imx8mm-usdhc compatible from the beginning and need not
>>> care about the old binding. Existing SoCs can just remain imx7d-usdhc
>>> compatible as they are now.
>>>
>>> I don't see what the deprecation accomplishes.
>>
>> It avoids to have too many entries of imx8mm (imx8mm alone,
>> imx8mm+imx7d, imx8xx+imx8mm+imx7d).
> 
> I see. I assume use of deprecated binding will be reported on a dtbs_check?

Unfortunately no, at least not yet. :-(

> If so, the expectation is that downstream projects run dtbs_check on their
> imported Linux DT repository, see the deprecation warning and extend
> their drivers to comply with it.
> 
> Some time later upstream will remove the deprecated binding and adjust
> the device trees. This works for me.
>

Yes. Plus this gives the notice to downstream projects, to comply with
the change of DTS.

Best regards,
Krzysztof

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

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

* Re: [PATCH 1/5] dt-bindings: mmc: imx-esdhc: Update compatible fallbacks
  2022-03-27 12:38 ` Adam Ford
@ 2022-03-31 13:04   ` Ulf Hansson
  -1 siblings, 0 replies; 44+ messages in thread
From: Ulf Hansson @ 2022-03-31 13:04 UTC (permalink / raw)
  To: Adam Ford
  Cc: linux-mmc, haibo.chen, aford, Rob Herring, Krzysztof Kozlowski,
	Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	NXP Linux Team, devicetree, linux-arm-kernel, linux-kernel

On Sun, 27 Mar 2022 at 14:39, Adam Ford <aford173@gmail.com> wrote:
>
> The SDHC controller in the imx8mn and imx8mp have the same controller
> as the imx8mm which is slightly different than that of the imx7d.
> Using the fallback of the imx8mm enables the controllers to support
> HS400-ES which is not available on the imx7d. After discussion with NXP,
> it turns out that the imx8qm should fall back to the imx8qxp, because
> those have some additional flags not present in the imx8mm.
>
> Suggested-by: haibo.chen@nxp.com
> Signed-off-by: Adam Ford <aford173@gmail.com>

I didn't quite follow all the discussions on patch3 - and whether that
may affect the binding. Anyway, I assume you will send a new version.
If not, please tell and will pick this up.

Kind regards
Uffe


> ---
> V2:  Added suggested-by note and imx8qxp updates.
> ---
>  .../devicetree/bindings/mmc/fsl-imx-esdhc.yaml   | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
> index 7dbbcae9485c..1427e9b5a6ec 100644
> --- a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
> +++ b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
> @@ -34,23 +34,25 @@ properties:
>            - fsl,imx6ull-usdhc
>            - fsl,imx7d-usdhc
>            - fsl,imx7ulp-usdhc
> +          - fsl,imx8mm-usdhc
> +          - fsl,imx8qxp-usdhc
>            - fsl,imxrt1050-usdhc
>            - nxp,s32g2-usdhc
>        - items:
>            - enum:
> -              - fsl,imx8mm-usdhc
> -              - fsl,imx8mn-usdhc
> -              - fsl,imx8mp-usdhc
>                - fsl,imx8mq-usdhc
> -              - fsl,imx8qm-usdhc
> -              - fsl,imx8qxp-usdhc
>            - const: fsl,imx7d-usdhc
>        - items:
>            - enum:
> -              - fsl,imx93-usdhc
> +              - fsl,imx8mn-usdhc
> +              - fsl,imx8mp-usdhc
>                - fsl,imx8ulp-usdhc
> +              - fsl,imx93-usdhc
>            - const: fsl,imx8mm-usdhc
> -
> +      - items:
> +          - enum:
> +              - fsl,imx8qm-usdhc
> +          - const: fsl,imx8qxp-usdhc
>    reg:
>      maxItems: 1
>
> --
> 2.34.1
>

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

* Re: [PATCH 1/5] dt-bindings: mmc: imx-esdhc: Update compatible fallbacks
@ 2022-03-31 13:04   ` Ulf Hansson
  0 siblings, 0 replies; 44+ messages in thread
From: Ulf Hansson @ 2022-03-31 13:04 UTC (permalink / raw)
  To: Adam Ford
  Cc: linux-mmc, haibo.chen, aford, Rob Herring, Krzysztof Kozlowski,
	Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	NXP Linux Team, devicetree, linux-arm-kernel, linux-kernel

On Sun, 27 Mar 2022 at 14:39, Adam Ford <aford173@gmail.com> wrote:
>
> The SDHC controller in the imx8mn and imx8mp have the same controller
> as the imx8mm which is slightly different than that of the imx7d.
> Using the fallback of the imx8mm enables the controllers to support
> HS400-ES which is not available on the imx7d. After discussion with NXP,
> it turns out that the imx8qm should fall back to the imx8qxp, because
> those have some additional flags not present in the imx8mm.
>
> Suggested-by: haibo.chen@nxp.com
> Signed-off-by: Adam Ford <aford173@gmail.com>

I didn't quite follow all the discussions on patch3 - and whether that
may affect the binding. Anyway, I assume you will send a new version.
If not, please tell and will pick this up.

Kind regards
Uffe


> ---
> V2:  Added suggested-by note and imx8qxp updates.
> ---
>  .../devicetree/bindings/mmc/fsl-imx-esdhc.yaml   | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
> index 7dbbcae9485c..1427e9b5a6ec 100644
> --- a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
> +++ b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
> @@ -34,23 +34,25 @@ properties:
>            - fsl,imx6ull-usdhc
>            - fsl,imx7d-usdhc
>            - fsl,imx7ulp-usdhc
> +          - fsl,imx8mm-usdhc
> +          - fsl,imx8qxp-usdhc
>            - fsl,imxrt1050-usdhc
>            - nxp,s32g2-usdhc
>        - items:
>            - enum:
> -              - fsl,imx8mm-usdhc
> -              - fsl,imx8mn-usdhc
> -              - fsl,imx8mp-usdhc
>                - fsl,imx8mq-usdhc
> -              - fsl,imx8qm-usdhc
> -              - fsl,imx8qxp-usdhc
>            - const: fsl,imx7d-usdhc
>        - items:
>            - enum:
> -              - fsl,imx93-usdhc
> +              - fsl,imx8mn-usdhc
> +              - fsl,imx8mp-usdhc
>                - fsl,imx8ulp-usdhc
> +              - fsl,imx93-usdhc
>            - const: fsl,imx8mm-usdhc
> -
> +      - items:
> +          - enum:
> +              - fsl,imx8qm-usdhc
> +          - const: fsl,imx8qxp-usdhc
>    reg:
>      maxItems: 1
>
> --
> 2.34.1
>

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

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

* Re: [PATCH 1/5] dt-bindings: mmc: imx-esdhc: Update compatible fallbacks
  2022-03-31 13:04   ` Ulf Hansson
@ 2022-03-31 16:00     ` Adam Ford
  -1 siblings, 0 replies; 44+ messages in thread
From: Adam Ford @ 2022-03-31 16:00 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: linux-mmc, Haibo Chen, Adam Ford-BE, Rob Herring,
	Krzysztof Kozlowski, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
	devicetree, arm-soc, Linux Kernel Mailing List

On Thu, Mar 31, 2022 at 8:05 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> On Sun, 27 Mar 2022 at 14:39, Adam Ford <aford173@gmail.com> wrote:
> >
> > The SDHC controller in the imx8mn and imx8mp have the same controller
> > as the imx8mm which is slightly different than that of the imx7d.
> > Using the fallback of the imx8mm enables the controllers to support
> > HS400-ES which is not available on the imx7d. After discussion with NXP,
> > it turns out that the imx8qm should fall back to the imx8qxp, because
> > those have some additional flags not present in the imx8mm.
> >
> > Suggested-by: haibo.chen@nxp.com
> > Signed-off-by: Adam Ford <aford173@gmail.com>
>
> I didn't quite follow all the discussions on patch3 - and whether that
> may affect the binding. Anyway, I assume you will send a new version.
> If not, please tell and will pick this up.

 I am not sure exactly how the YAML should look, but I'm going to give
it a try.  I hope to have something this weekend.


adam
>
> Kind regards
> Uffe
>
>
> > ---
> > V2:  Added suggested-by note and imx8qxp updates.
> > ---
> >  .../devicetree/bindings/mmc/fsl-imx-esdhc.yaml   | 16 +++++++++-------
> >  1 file changed, 9 insertions(+), 7 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
> > index 7dbbcae9485c..1427e9b5a6ec 100644
> > --- a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
> > +++ b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
> > @@ -34,23 +34,25 @@ properties:
> >            - fsl,imx6ull-usdhc
> >            - fsl,imx7d-usdhc
> >            - fsl,imx7ulp-usdhc
> > +          - fsl,imx8mm-usdhc
> > +          - fsl,imx8qxp-usdhc
> >            - fsl,imxrt1050-usdhc
> >            - nxp,s32g2-usdhc
> >        - items:
> >            - enum:
> > -              - fsl,imx8mm-usdhc
> > -              - fsl,imx8mn-usdhc
> > -              - fsl,imx8mp-usdhc
> >                - fsl,imx8mq-usdhc
> > -              - fsl,imx8qm-usdhc
> > -              - fsl,imx8qxp-usdhc
> >            - const: fsl,imx7d-usdhc
> >        - items:
> >            - enum:
> > -              - fsl,imx93-usdhc
> > +              - fsl,imx8mn-usdhc
> > +              - fsl,imx8mp-usdhc
> >                - fsl,imx8ulp-usdhc
> > +              - fsl,imx93-usdhc
> >            - const: fsl,imx8mm-usdhc
> > -
> > +      - items:
> > +          - enum:
> > +              - fsl,imx8qm-usdhc
> > +          - const: fsl,imx8qxp-usdhc
> >    reg:
> >      maxItems: 1
> >
> > --
> > 2.34.1
> >

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

* Re: [PATCH 1/5] dt-bindings: mmc: imx-esdhc: Update compatible fallbacks
@ 2022-03-31 16:00     ` Adam Ford
  0 siblings, 0 replies; 44+ messages in thread
From: Adam Ford @ 2022-03-31 16:00 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: linux-mmc, Haibo Chen, Adam Ford-BE, Rob Herring,
	Krzysztof Kozlowski, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
	devicetree, arm-soc, Linux Kernel Mailing List

On Thu, Mar 31, 2022 at 8:05 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> On Sun, 27 Mar 2022 at 14:39, Adam Ford <aford173@gmail.com> wrote:
> >
> > The SDHC controller in the imx8mn and imx8mp have the same controller
> > as the imx8mm which is slightly different than that of the imx7d.
> > Using the fallback of the imx8mm enables the controllers to support
> > HS400-ES which is not available on the imx7d. After discussion with NXP,
> > it turns out that the imx8qm should fall back to the imx8qxp, because
> > those have some additional flags not present in the imx8mm.
> >
> > Suggested-by: haibo.chen@nxp.com
> > Signed-off-by: Adam Ford <aford173@gmail.com>
>
> I didn't quite follow all the discussions on patch3 - and whether that
> may affect the binding. Anyway, I assume you will send a new version.
> If not, please tell and will pick this up.

 I am not sure exactly how the YAML should look, but I'm going to give
it a try.  I hope to have something this weekend.


adam
>
> Kind regards
> Uffe
>
>
> > ---
> > V2:  Added suggested-by note and imx8qxp updates.
> > ---
> >  .../devicetree/bindings/mmc/fsl-imx-esdhc.yaml   | 16 +++++++++-------
> >  1 file changed, 9 insertions(+), 7 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
> > index 7dbbcae9485c..1427e9b5a6ec 100644
> > --- a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
> > +++ b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
> > @@ -34,23 +34,25 @@ properties:
> >            - fsl,imx6ull-usdhc
> >            - fsl,imx7d-usdhc
> >            - fsl,imx7ulp-usdhc
> > +          - fsl,imx8mm-usdhc
> > +          - fsl,imx8qxp-usdhc
> >            - fsl,imxrt1050-usdhc
> >            - nxp,s32g2-usdhc
> >        - items:
> >            - enum:
> > -              - fsl,imx8mm-usdhc
> > -              - fsl,imx8mn-usdhc
> > -              - fsl,imx8mp-usdhc
> >                - fsl,imx8mq-usdhc
> > -              - fsl,imx8qm-usdhc
> > -              - fsl,imx8qxp-usdhc
> >            - const: fsl,imx7d-usdhc
> >        - items:
> >            - enum:
> > -              - fsl,imx93-usdhc
> > +              - fsl,imx8mn-usdhc
> > +              - fsl,imx8mp-usdhc
> >                - fsl,imx8ulp-usdhc
> > +              - fsl,imx93-usdhc
> >            - const: fsl,imx8mm-usdhc
> > -
> > +      - items:
> > +          - enum:
> > +              - fsl,imx8qm-usdhc
> > +          - const: fsl,imx8qxp-usdhc
> >    reg:
> >      maxItems: 1
> >
> > --
> > 2.34.1
> >

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

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

end of thread, other threads:[~2022-03-31 16:01 UTC | newest]

Thread overview: 44+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-27 12:38 [PATCH 1/5] dt-bindings: mmc: imx-esdhc: Update compatible fallbacks Adam Ford
2022-03-27 12:38 ` Adam Ford
2022-03-27 12:38 ` [PATCH 2/5] arm64: dts: imx8mn: Enable HS400-ES Adam Ford
2022-03-27 12:38   ` Adam Ford
2022-03-27 12:38 ` [PATCH 3/5] arm64: dts: imx8mp: " Adam Ford
2022-03-27 12:38   ` Adam Ford
2022-03-28  7:20   ` Ahmad Fatoum
2022-03-28  7:20     ` Ahmad Fatoum
2022-03-28 10:47     ` Adam Ford
2022-03-28 10:47       ` Adam Ford
2022-03-28 11:09       ` Ahmad Fatoum
2022-03-28 11:09         ` Ahmad Fatoum
2022-03-28 11:49         ` Krzysztof Kozlowski
2022-03-28 11:49           ` Krzysztof Kozlowski
2022-03-28 12:45           ` Adam Ford
2022-03-28 12:45             ` Adam Ford
2022-03-28 12:56             ` Krzysztof Kozlowski
2022-03-28 12:56               ` Krzysztof Kozlowski
2022-03-28 13:06               ` Adam Ford
2022-03-28 13:06                 ` Adam Ford
2022-03-28 13:12                 ` Ahmad Fatoum
2022-03-28 13:12                   ` Ahmad Fatoum
2022-03-28 13:15                   ` Krzysztof Kozlowski
2022-03-28 13:15                     ` Krzysztof Kozlowski
2022-03-28 13:17                 ` Krzysztof Kozlowski
2022-03-28 13:17                   ` Krzysztof Kozlowski
2022-03-28 13:07               ` Ahmad Fatoum
2022-03-28 13:07                 ` Ahmad Fatoum
2022-03-28 13:14                 ` Krzysztof Kozlowski
2022-03-28 13:14                   ` Krzysztof Kozlowski
2022-03-28 13:42                   ` Ahmad Fatoum
2022-03-28 13:42                     ` Ahmad Fatoum
2022-03-28 17:35                     ` Krzysztof Kozlowski
2022-03-28 17:35                       ` Krzysztof Kozlowski
2022-03-27 12:38 ` [PATCH 4/5] arm64: dts: imx8qxp: Remove imx7d-usdhc compatible fallback Adam Ford
2022-03-27 12:38   ` Adam Ford
2022-03-27 12:38 ` [PATCH 5/5] arm64: dts: imx8qm: Remove fsl,imx7d-usdhc " Adam Ford
2022-03-27 12:38   ` [PATCH 5/5] arm64: dts: imx8qm: Remove fsl, imx7d-usdhc " Adam Ford
2022-03-27 19:26 ` [PATCH 1/5] dt-bindings: mmc: imx-esdhc: Update compatible fallbacks Krzysztof Kozlowski
2022-03-27 19:26   ` Krzysztof Kozlowski
2022-03-31 13:04 ` Ulf Hansson
2022-03-31 13:04   ` Ulf Hansson
2022-03-31 16:00   ` Adam Ford
2022-03-31 16:00     ` Adam Ford

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