[PATCH v5 0/4] mmc: sdhci-of-aspeed: Support toggling SD bus signal

[PATCH v5 0/4] mmc: sdhci-of-aspeed: Support toggling SD bus signal

[Steven Lee]

AST2600-A2 EVB has the reference design for enabling SD bus
power and toggling SD bus signal voltage between 3.3v and 1.8v by
GPIO regulators.
This patch series adds sdhci node and gpio regulators in a new dts file
for AST2600-A2 EVB.
The description of the reference design of AST2600-A2 EVB is added
in the new dts file.

This patch also include a helper for updating AST2600 sdhci capability
registers.

Changes from v4:
* Move sdhci node and gpio regulator from aspeed-ast2600-evb-a2.dts
  to aspeed-ast2600-evb.dts. Now aspeed-ast2600-evb.dts only supports
  A2(or newer) evbs.
* Remove aspeed-ast2600-evb-a2.dts since sdhci nodes were moved to
  aspeed-ast2600-evb.dts.
* Add aspeed-ast2600-evb-a1.dts for A1 and A0 evbs.

Changes from v3:
* Remove the example of gpio regulator from dt-bindings.
* Add sdhci node and gpio regulators to a new dts file.
* Move the comment of the reference design to the new
  dts file.
* Modify commit message of sdhci-of-aspeed.c.
* Fix coding style issues of sdhci-of-aspeed.c.
* Remove the implementation of eMMC resetc since it has no relevance to
  the goal that this patch series want to achieve and it may needs further
  discussion about the design of reset behavior.

Changes from v2:
* Move the comment of the reference design from dt-bindings to device tree.
* Add clk-phase binding for eMMC controller.
* Reimplement aspeed_sdc_set_slot_capability().
* Separate the implementation of eMMC reset to another patch file.
* Fix yaml document error per the report of dt_binding_check and
  dtbs_check.

Changes from v1:
* Add the device tree example for AST2600 A2 EVB in dt-bindings
  document
* Add timing-phase for eMMC controller.
* Remove power-gpio and power-switch-gpio from sdhci driver, they should
  be handled by regulator.
* Add a helper to update capability registers in the driver.
* Sync sdhci settings from device tree to SoC capability registers.
* Sync timing-phase from device tree to SoC Clock Phase Control
  register

Please help to review.

Regards,
Steven

Steven Lee (4):
  ARM: dts: aspeed: ast2600evb: Add sdhci node and gpio regulator for A2
    evb.
  ARM: dts: aspeed: ast2600evb: Add phase correction for emmc
    controller.
  ARM: dts: aspeed: ast2600evb: Add dts file for A1 and A0.
  mmc: sdhci-of-aspeed: Configure the SDHCIs as specified by the
    devicetree.

 arch/arm/boot/dts/aspeed-ast2600-evb-a1.dts | 15 ++++
 arch/arm/boot/dts/aspeed-ast2600-evb.dts    | 87 ++++++++++++++++++++-
 drivers/mmc/host/sdhci-of-aspeed.c          | 48 ++++++++++++
 3 files changed, 149 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/boot/dts/aspeed-ast2600-evb-a1.dts

-- 
2.17.1

[PATCH v5 0/4] mmc: sdhci-of-aspeed: Support toggling SD bus signal

[Steven Lee]

AST2600-A2 EVB has the reference design for enabling SD bus
power and toggling SD bus signal voltage between 3.3v and 1.8v by
GPIO regulators.
This patch series adds sdhci node and gpio regulators in a new dts file
for AST2600-A2 EVB.
The description of the reference design of AST2600-A2 EVB is added
in the new dts file.

This patch also include a helper for updating AST2600 sdhci capability
registers.

Changes from v4:
* Move sdhci node and gpio regulator from aspeed-ast2600-evb-a2.dts
  to aspeed-ast2600-evb.dts. Now aspeed-ast2600-evb.dts only supports
  A2(or newer) evbs.
* Remove aspeed-ast2600-evb-a2.dts since sdhci nodes were moved to
  aspeed-ast2600-evb.dts.
* Add aspeed-ast2600-evb-a1.dts for A1 and A0 evbs.

Changes from v3:
* Remove the example of gpio regulator from dt-bindings.
* Add sdhci node and gpio regulators to a new dts file.
* Move the comment of the reference design to the new
  dts file.
* Modify commit message of sdhci-of-aspeed.c.
* Fix coding style issues of sdhci-of-aspeed.c.
* Remove the implementation of eMMC resetc since it has no relevance to
  the goal that this patch series want to achieve and it may needs further
  discussion about the design of reset behavior.

Changes from v2:
* Move the comment of the reference design from dt-bindings to device tree.
* Add clk-phase binding for eMMC controller.
* Reimplement aspeed_sdc_set_slot_capability().
* Separate the implementation of eMMC reset to another patch file.
* Fix yaml document error per the report of dt_binding_check and
  dtbs_check.

Changes from v1:
* Add the device tree example for AST2600 A2 EVB in dt-bindings
  document
* Add timing-phase for eMMC controller.
* Remove power-gpio and power-switch-gpio from sdhci driver, they should
  be handled by regulator.
* Add a helper to update capability registers in the driver.
* Sync sdhci settings from device tree to SoC capability registers.
* Sync timing-phase from device tree to SoC Clock Phase Control
  register

Please help to review.

Regards,
Steven

Steven Lee (4):
  ARM: dts: aspeed: ast2600evb: Add sdhci node and gpio regulator for A2
    evb.
  ARM: dts: aspeed: ast2600evb: Add phase correction for emmc
    controller.
  ARM: dts: aspeed: ast2600evb: Add dts file for A1 and A0.
  mmc: sdhci-of-aspeed: Configure the SDHCIs as specified by the
    devicetree.

 arch/arm/boot/dts/aspeed-ast2600-evb-a1.dts | 15 ++++
 arch/arm/boot/dts/aspeed-ast2600-evb.dts    | 87 ++++++++++++++++++++-
 drivers/mmc/host/sdhci-of-aspeed.c          | 48 ++++++++++++
 3 files changed, 149 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/boot/dts/aspeed-ast2600-evb-a1.dts

-- 
2.17.1


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

[PATCH v5 0/4] mmc: sdhci-of-aspeed: Support toggling SD bus signal

[Steven Lee]

AST2600-A2 EVB has the reference design for enabling SD bus
power and toggling SD bus signal voltage between 3.3v and 1.8v by
GPIO regulators.
This patch series adds sdhci node and gpio regulators in a new dts file
for AST2600-A2 EVB.
The description of the reference design of AST2600-A2 EVB is added
in the new dts file.

This patch also include a helper for updating AST2600 sdhci capability
registers.

Changes from v4:
* Move sdhci node and gpio regulator from aspeed-ast2600-evb-a2.dts
  to aspeed-ast2600-evb.dts. Now aspeed-ast2600-evb.dts only supports
  A2(or newer) evbs.
* Remove aspeed-ast2600-evb-a2.dts since sdhci nodes were moved to
  aspeed-ast2600-evb.dts.
* Add aspeed-ast2600-evb-a1.dts for A1 and A0 evbs.

Changes from v3:
* Remove the example of gpio regulator from dt-bindings.
* Add sdhci node and gpio regulators to a new dts file.
* Move the comment of the reference design to the new
  dts file.
* Modify commit message of sdhci-of-aspeed.c.
* Fix coding style issues of sdhci-of-aspeed.c.
* Remove the implementation of eMMC resetc since it has no relevance to
  the goal that this patch series want to achieve and it may needs further
  discussion about the design of reset behavior.

Changes from v2:
* Move the comment of the reference design from dt-bindings to device tree.
* Add clk-phase binding for eMMC controller.
* Reimplement aspeed_sdc_set_slot_capability().
* Separate the implementation of eMMC reset to another patch file.
* Fix yaml document error per the report of dt_binding_check and
  dtbs_check.

Changes from v1:
* Add the device tree example for AST2600 A2 EVB in dt-bindings
  document
* Add timing-phase for eMMC controller.
* Remove power-gpio and power-switch-gpio from sdhci driver, they should
  be handled by regulator.
* Add a helper to update capability registers in the driver.
* Sync sdhci settings from device tree to SoC capability registers.
* Sync timing-phase from device tree to SoC Clock Phase Control
  register

Please help to review.

Regards,
Steven

Steven Lee (4):
  ARM: dts: aspeed: ast2600evb: Add sdhci node and gpio regulator for A2
    evb.
  ARM: dts: aspeed: ast2600evb: Add phase correction for emmc
    controller.
  ARM: dts: aspeed: ast2600evb: Add dts file for A1 and A0.
  mmc: sdhci-of-aspeed: Configure the SDHCIs as specified by the
    devicetree.

 arch/arm/boot/dts/aspeed-ast2600-evb-a1.dts | 15 ++++
 arch/arm/boot/dts/aspeed-ast2600-evb.dts    | 87 ++++++++++++++++++++-
 drivers/mmc/host/sdhci-of-aspeed.c          | 48 ++++++++++++
 3 files changed, 149 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/boot/dts/aspeed-ast2600-evb-a1.dts

-- 
2.17.1

[PATCH v5 1/4] ARM: dts: aspeed: ast2600evb: Add sdhci node and gpio regulator for A2 evb.

[Steven Lee]

AST2600 A2(or newer) EVB has gpio regulators for toggling signal voltage
between 3.3v and 1.8v, the patch adds sdhci node and gpio regulator in the
dts file and adds comment for describing the reference design.

Signed-off-by: Steven Lee <steven_lee@aspeedtech.com>
---
 arch/arm/boot/dts/aspeed-ast2600-evb.dts | 84 ++++++++++++++++++++++++
 1 file changed, 84 insertions(+)

diff --git a/arch/arm/boot/dts/aspeed-ast2600-evb.dts b/arch/arm/boot/dts/aspeed-ast2600-evb.dts
index 2772796e215e..401034da6dcc 100644
--- a/arch/arm/boot/dts/aspeed-ast2600-evb.dts
+++ b/arch/arm/boot/dts/aspeed-ast2600-evb.dts
@@ -4,6 +4,7 @@
 /dts-v1/;
 
 #include "aspeed-g6.dtsi"
+#include <dt-bindings/gpio/aspeed-gpio.h>
 
 / {
 	model = "AST2600 EVB";
@@ -21,6 +22,46 @@
 		device_type = "memory";
 		reg = <0x80000000 0x80000000>;
 	};
+
+	vcc_sdhci0: regulator-vcc-sdhci0 {
+		compatible = "regulator-fixed";
+		regulator-name = "SDHCI0 Vcc";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		gpios = <&gpio0 ASPEED_GPIO(V, 0) GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+	};
+
+	vccq_sdhci0: regulator-vccq-sdhci0 {
+		compatible = "regulator-gpio";
+		regulator-name = "SDHCI0 VccQ";
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <3300000>;
+		gpios = <&gpio0 ASPEED_GPIO(V, 1) GPIO_ACTIVE_HIGH>;
+		gpios-states = <1>;
+		states = <3300000 1>,
+			 <1800000 0>;
+	};
+
+	vcc_sdhci1: regulator-vcc-sdhci1 {
+		compatible = "regulator-fixed";
+		regulator-name = "SDHCI1 Vcc";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		gpios = <&gpio0 ASPEED_GPIO(V, 2) GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+	};
+
+	vccq_sdhci1: regulator-vccq-sdhci1 {
+		compatible = "regulator-gpio";
+		regulator-name = "SDHCI1 VccQ";
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <3300000>;
+		gpios = <&gpio0 ASPEED_GPIO(V, 3) GPIO_ACTIVE_HIGH>;
+		gpios-states = <1>;
+		states = <3300000 1>,
+			 <1800000 0>;
+	};
 };
 
 &mdio0 {
@@ -245,3 +286,46 @@
 &uhci {
 	status = "okay";
 };
+
+&sdc {
+	status = "okay";
+};
+
+/*
+ * The signal voltage of sdhci0 and sdhci1 on AST2600-A2 EVB is able to be
+ * toggled by GPIO pins.
+ * In the reference design, GPIOV0 of AST2600-A2 EVB is connected to the
+ * power load switch that provides 3.3v to sdhci0 vdd, GPIOV1 is connected to
+ * a 1.8v and a 3.3v power load switch that provides signal voltage to
+ * sdhci0 bus.
+ * If GPIOV0 is active high, sdhci0 is enabled, otherwise, sdhci0 is disabled.
+ * If GPIOV1 is active high, 3.3v power load switch is enabled, sdhci0 signal
+ * voltage is 3.3v, otherwise, 1.8v power load switch will be enabled,
+ * sdhci0 signal voltage becomes 1.8v.
+ * AST2600-A2 EVB also supports toggling signal voltage for sdhci1.
+ * The design is the same as sdhci0, it uses GPIOV2 as power-gpio and GPIOV3
+ * as power-switch-gpio.
+ */
+&sdhci0 {
+	status = "okay";
+	bus-width = <4>;
+	max-frequency = <100000000>;
+	sdhci-drive-type = /bits/ 8 <3>;
+	sdhci-caps-mask = <0x7 0x0>;
+	sdhci,wp-inverted;
+	vmmc-supply = <&vcc_sdhci0>;
+	vqmmc-supply = <&vccq_sdhci0>;
+	clk-phase-sd-hs = <7>, <200>;
+};
+
+&sdhci1 {
+	status = "okay";
+	bus-width = <4>;
+	max-frequency = <100000000>;
+	sdhci-drive-type = /bits/ 8 <3>;
+	sdhci-caps-mask = <0x7 0x0>;
+	sdhci,wp-inverted;
+	vmmc-supply = <&vcc_sdhci1>;
+	vqmmc-supply = <&vccq_sdhci1>;
+	clk-phase-sd-hs = <7>, <200>;
+};
-- 
2.17.1

[PATCH v5 1/4] ARM: dts: aspeed: ast2600evb: Add sdhci node and gpio regulator for A2 evb.

[Steven Lee]

AST2600 A2(or newer) EVB has gpio regulators for toggling signal voltage
between 3.3v and 1.8v, the patch adds sdhci node and gpio regulator in the
dts file and adds comment for describing the reference design.

Signed-off-by: Steven Lee <steven_lee@aspeedtech.com>
---
 arch/arm/boot/dts/aspeed-ast2600-evb.dts | 84 ++++++++++++++++++++++++
 1 file changed, 84 insertions(+)

diff --git a/arch/arm/boot/dts/aspeed-ast2600-evb.dts b/arch/arm/boot/dts/aspeed-ast2600-evb.dts
index 2772796e215e..401034da6dcc 100644
--- a/arch/arm/boot/dts/aspeed-ast2600-evb.dts
+++ b/arch/arm/boot/dts/aspeed-ast2600-evb.dts
@@ -4,6 +4,7 @@
 /dts-v1/;
 
 #include "aspeed-g6.dtsi"
+#include <dt-bindings/gpio/aspeed-gpio.h>
 
 / {
 	model = "AST2600 EVB";
@@ -21,6 +22,46 @@
 		device_type = "memory";
 		reg = <0x80000000 0x80000000>;
 	};
+
+	vcc_sdhci0: regulator-vcc-sdhci0 {
+		compatible = "regulator-fixed";
+		regulator-name = "SDHCI0 Vcc";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		gpios = <&gpio0 ASPEED_GPIO(V, 0) GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+	};
+
+	vccq_sdhci0: regulator-vccq-sdhci0 {
+		compatible = "regulator-gpio";
+		regulator-name = "SDHCI0 VccQ";
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <3300000>;
+		gpios = <&gpio0 ASPEED_GPIO(V, 1) GPIO_ACTIVE_HIGH>;
+		gpios-states = <1>;
+		states = <3300000 1>,
+			 <1800000 0>;
+	};
+
+	vcc_sdhci1: regulator-vcc-sdhci1 {
+		compatible = "regulator-fixed";
+		regulator-name = "SDHCI1 Vcc";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		gpios = <&gpio0 ASPEED_GPIO(V, 2) GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+	};
+
+	vccq_sdhci1: regulator-vccq-sdhci1 {
+		compatible = "regulator-gpio";
+		regulator-name = "SDHCI1 VccQ";
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <3300000>;
+		gpios = <&gpio0 ASPEED_GPIO(V, 3) GPIO_ACTIVE_HIGH>;
+		gpios-states = <1>;
+		states = <3300000 1>,
+			 <1800000 0>;
+	};
 };
 
 &mdio0 {
@@ -245,3 +286,46 @@
 &uhci {
 	status = "okay";
 };
+
+&sdc {
+	status = "okay";
+};
+
+/*
+ * The signal voltage of sdhci0 and sdhci1 on AST2600-A2 EVB is able to be
+ * toggled by GPIO pins.
+ * In the reference design, GPIOV0 of AST2600-A2 EVB is connected to the
+ * power load switch that provides 3.3v to sdhci0 vdd, GPIOV1 is connected to
+ * a 1.8v and a 3.3v power load switch that provides signal voltage to
+ * sdhci0 bus.
+ * If GPIOV0 is active high, sdhci0 is enabled, otherwise, sdhci0 is disabled.
+ * If GPIOV1 is active high, 3.3v power load switch is enabled, sdhci0 signal
+ * voltage is 3.3v, otherwise, 1.8v power load switch will be enabled,
+ * sdhci0 signal voltage becomes 1.8v.
+ * AST2600-A2 EVB also supports toggling signal voltage for sdhci1.
+ * The design is the same as sdhci0, it uses GPIOV2 as power-gpio and GPIOV3
+ * as power-switch-gpio.
+ */
+&sdhci0 {
+	status = "okay";
+	bus-width = <4>;
+	max-frequency = <100000000>;
+	sdhci-drive-type = /bits/ 8 <3>;
+	sdhci-caps-mask = <0x7 0x0>;
+	sdhci,wp-inverted;
+	vmmc-supply = <&vcc_sdhci0>;
+	vqmmc-supply = <&vccq_sdhci0>;
+	clk-phase-sd-hs = <7>, <200>;
+};
+
+&sdhci1 {
+	status = "okay";
+	bus-width = <4>;
+	max-frequency = <100000000>;
+	sdhci-drive-type = /bits/ 8 <3>;
+	sdhci-caps-mask = <0x7 0x0>;
+	sdhci,wp-inverted;
+	vmmc-supply = <&vcc_sdhci1>;
+	vqmmc-supply = <&vccq_sdhci1>;
+	clk-phase-sd-hs = <7>, <200>;
+};
-- 
2.17.1


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

[PATCH v5 1/4] ARM: dts: aspeed: ast2600evb: Add sdhci node and gpio regulator for A2 evb.

[Steven Lee]

AST2600 A2(or newer) EVB has gpio regulators for toggling signal voltage
between 3.3v and 1.8v, the patch adds sdhci node and gpio regulator in the
dts file and adds comment for describing the reference design.

Signed-off-by: Steven Lee <steven_lee@aspeedtech.com>
---
 arch/arm/boot/dts/aspeed-ast2600-evb.dts | 84 ++++++++++++++++++++++++
 1 file changed, 84 insertions(+)

diff --git a/arch/arm/boot/dts/aspeed-ast2600-evb.dts b/arch/arm/boot/dts/aspeed-ast2600-evb.dts
index 2772796e215e..401034da6dcc 100644
--- a/arch/arm/boot/dts/aspeed-ast2600-evb.dts
+++ b/arch/arm/boot/dts/aspeed-ast2600-evb.dts
@@ -4,6 +4,7 @@
 /dts-v1/;
 
 #include "aspeed-g6.dtsi"
+#include <dt-bindings/gpio/aspeed-gpio.h>
 
 / {
 	model = "AST2600 EVB";
@@ -21,6 +22,46 @@
 		device_type = "memory";
 		reg = <0x80000000 0x80000000>;
 	};
+
+	vcc_sdhci0: regulator-vcc-sdhci0 {
+		compatible = "regulator-fixed";
+		regulator-name = "SDHCI0 Vcc";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		gpios = <&gpio0 ASPEED_GPIO(V, 0) GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+	};
+
+	vccq_sdhci0: regulator-vccq-sdhci0 {
+		compatible = "regulator-gpio";
+		regulator-name = "SDHCI0 VccQ";
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <3300000>;
+		gpios = <&gpio0 ASPEED_GPIO(V, 1) GPIO_ACTIVE_HIGH>;
+		gpios-states = <1>;
+		states = <3300000 1>,
+			 <1800000 0>;
+	};
+
+	vcc_sdhci1: regulator-vcc-sdhci1 {
+		compatible = "regulator-fixed";
+		regulator-name = "SDHCI1 Vcc";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		gpios = <&gpio0 ASPEED_GPIO(V, 2) GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+	};
+
+	vccq_sdhci1: regulator-vccq-sdhci1 {
+		compatible = "regulator-gpio";
+		regulator-name = "SDHCI1 VccQ";
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <3300000>;
+		gpios = <&gpio0 ASPEED_GPIO(V, 3) GPIO_ACTIVE_HIGH>;
+		gpios-states = <1>;
+		states = <3300000 1>,
+			 <1800000 0>;
+	};
 };
 
 &mdio0 {
@@ -245,3 +286,46 @@
 &uhci {
 	status = "okay";
 };
+
+&sdc {
+	status = "okay";
+};
+
+/*
+ * The signal voltage of sdhci0 and sdhci1 on AST2600-A2 EVB is able to be
+ * toggled by GPIO pins.
+ * In the reference design, GPIOV0 of AST2600-A2 EVB is connected to the
+ * power load switch that provides 3.3v to sdhci0 vdd, GPIOV1 is connected to
+ * a 1.8v and a 3.3v power load switch that provides signal voltage to
+ * sdhci0 bus.
+ * If GPIOV0 is active high, sdhci0 is enabled, otherwise, sdhci0 is disabled.
+ * If GPIOV1 is active high, 3.3v power load switch is enabled, sdhci0 signal
+ * voltage is 3.3v, otherwise, 1.8v power load switch will be enabled,
+ * sdhci0 signal voltage becomes 1.8v.
+ * AST2600-A2 EVB also supports toggling signal voltage for sdhci1.
+ * The design is the same as sdhci0, it uses GPIOV2 as power-gpio and GPIOV3
+ * as power-switch-gpio.
+ */
+&sdhci0 {
+	status = "okay";
+	bus-width = <4>;
+	max-frequency = <100000000>;
+	sdhci-drive-type = /bits/ 8 <3>;
+	sdhci-caps-mask = <0x7 0x0>;
+	sdhci,wp-inverted;
+	vmmc-supply = <&vcc_sdhci0>;
+	vqmmc-supply = <&vccq_sdhci0>;
+	clk-phase-sd-hs = <7>, <200>;
+};
+
+&sdhci1 {
+	status = "okay";
+	bus-width = <4>;
+	max-frequency = <100000000>;
+	sdhci-drive-type = /bits/ 8 <3>;
+	sdhci-caps-mask = <0x7 0x0>;
+	sdhci,wp-inverted;
+	vmmc-supply = <&vcc_sdhci1>;
+	vqmmc-supply = <&vccq_sdhci1>;
+	clk-phase-sd-hs = <7>, <200>;
+};
-- 
2.17.1

Re: [PATCH v5 1/4] ARM: dts: aspeed: ast2600evb: Add sdhci node and gpio regulator for A2 evb.

[Andrew Jeffery]

On Mon, 24 May 2021, at 17:02, Steven Lee wrote:
> AST2600 A2(or newer) EVB has gpio regulators for toggling signal voltage
> between 3.3v and 1.8v, the patch adds sdhci node and gpio regulator in the
> dts file and adds comment for describing the reference design.
> 
> Signed-off-by: Steven Lee <steven_lee@aspeedtech.com>

Acked-by: Andrew Jeffery <andrew@aj.id.au>

Re: [PATCH v5 1/4] ARM: dts: aspeed: ast2600evb: Add sdhci node and gpio regulator for A2 evb.

[Andrew Jeffery]

On Mon, 24 May 2021, at 17:02, Steven Lee wrote:
> AST2600 A2(or newer) EVB has gpio regulators for toggling signal voltage
> between 3.3v and 1.8v, the patch adds sdhci node and gpio regulator in the
> dts file and adds comment for describing the reference design.
> 
> Signed-off-by: Steven Lee <steven_lee@aspeedtech.com>

Acked-by: Andrew Jeffery <andrew@aj.id.au>

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

Re: [PATCH v5 1/4] ARM: dts: aspeed: ast2600evb: Add sdhci node and gpio regulator for A2 evb.

[Andrew Jeffery]

On Mon, 24 May 2021, at 17:02, Steven Lee wrote:
> AST2600 A2(or newer) EVB has gpio regulators for toggling signal voltage
> between 3.3v and 1.8v, the patch adds sdhci node and gpio regulator in the
> dts file and adds comment for describing the reference design.
> 
> Signed-off-by: Steven Lee <steven_lee@aspeedtech.com>

Acked-by: Andrew Jeffery <andrew@aj.id.au>

[PATCH v5 2/4] ARM: dts: aspeed: ast2600evb: Add phase correction for emmc controller.

[Steven Lee]

Set MMC timing-phase register by adding the phase correction binding in the
device tree.

Signed-off-by: Steven Lee <steven_lee@aspeedtech.com>
Acked-by: Andrew Jeffery <andrew@aj.id.au>
Reviewed-by: Joel Stanley <joel@jms.id.au>
---
 arch/arm/boot/dts/aspeed-ast2600-evb.dts | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/aspeed-ast2600-evb.dts b/arch/arm/boot/dts/aspeed-ast2600-evb.dts
index 401034da6dcc..c670f3a45fbb 100644
--- a/arch/arm/boot/dts/aspeed-ast2600-evb.dts
+++ b/arch/arm/boot/dts/aspeed-ast2600-evb.dts
@@ -148,7 +148,8 @@
 &emmc {
 	non-removable;
 	bus-width = <4>;
-	max-frequency = <52000000>;
+	max-frequency = <100000000>;
+	clk-phase-mmc-hs200 = <9>, <225>;
 };
 
 &rtc {
-- 
2.17.1

[PATCH v5 2/4] ARM: dts: aspeed: ast2600evb: Add phase correction for emmc controller.

[Steven Lee]

Set MMC timing-phase register by adding the phase correction binding in the
device tree.

Signed-off-by: Steven Lee <steven_lee@aspeedtech.com>
Acked-by: Andrew Jeffery <andrew@aj.id.au>
Reviewed-by: Joel Stanley <joel@jms.id.au>
---
 arch/arm/boot/dts/aspeed-ast2600-evb.dts | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/aspeed-ast2600-evb.dts b/arch/arm/boot/dts/aspeed-ast2600-evb.dts
index 401034da6dcc..c670f3a45fbb 100644
--- a/arch/arm/boot/dts/aspeed-ast2600-evb.dts
+++ b/arch/arm/boot/dts/aspeed-ast2600-evb.dts
@@ -148,7 +148,8 @@
 &emmc {
 	non-removable;
 	bus-width = <4>;
-	max-frequency = <52000000>;
+	max-frequency = <100000000>;
+	clk-phase-mmc-hs200 = <9>, <225>;
 };
 
 &rtc {
-- 
2.17.1


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

[PATCH v5 2/4] ARM: dts: aspeed: ast2600evb: Add phase correction for emmc controller.

[Steven Lee]

Set MMC timing-phase register by adding the phase correction binding in the
device tree.

Signed-off-by: Steven Lee <steven_lee@aspeedtech.com>
Acked-by: Andrew Jeffery <andrew@aj.id.au>
Reviewed-by: Joel Stanley <joel@jms.id.au>
---
 arch/arm/boot/dts/aspeed-ast2600-evb.dts | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/aspeed-ast2600-evb.dts b/arch/arm/boot/dts/aspeed-ast2600-evb.dts
index 401034da6dcc..c670f3a45fbb 100644
--- a/arch/arm/boot/dts/aspeed-ast2600-evb.dts
+++ b/arch/arm/boot/dts/aspeed-ast2600-evb.dts
@@ -148,7 +148,8 @@
 &emmc {
 	non-removable;
 	bus-width = <4>;
-	max-frequency = <52000000>;
+	max-frequency = <100000000>;
+	clk-phase-mmc-hs200 = <9>, <225>;
 };
 
 &rtc {
-- 
2.17.1

[PATCH v5 3/4] ARM: dts: aspeed: ast2600evb: Add dts file for A1 and A0.

[Steven Lee]

aspeed-ast2600-evb.dts was modified for supporting A2 evb.
Since A1/A0 evbs don't have GPIO regulators and SD clock frequency(SCU210)
is different to A2 as well. Adding a new dts that removes new nodes
created in aspeed-ast2600-evb.dts is necessary.

Signed-off-by: Steven Lee <steven_lee@aspeedtech.com>
---
 arch/arm/boot/dts/aspeed-ast2600-evb-a1.dts | 15 +++++++++++++++
 1 file changed, 15 insertions(+)
 create mode 100644 arch/arm/boot/dts/aspeed-ast2600-evb-a1.dts

diff --git a/arch/arm/boot/dts/aspeed-ast2600-evb-a1.dts b/arch/arm/boot/dts/aspeed-ast2600-evb-a1.dts
new file mode 100644
index 000000000000..dd7148060c4a
--- /dev/null
+++ b/arch/arm/boot/dts/aspeed-ast2600-evb-a1.dts
@@ -0,0 +1,15 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+// Copyright 2021 IBM Corp.
+
+#include "aspeed-ast2600-evb.dts"
+
+/ {
+	model = "AST2600 A1 EVB";
+
+	/delete-node/regulator-vcc-sdhci0;
+	/delete-node/regulator-vcc-sdhci1;
+	/delete-node/regulator-vccq-sdhci0;
+	/delete-node/regulator-vccq-sdhci1;
+};
+
+/delete-node/ &sdc;
-- 
2.17.1

[PATCH v5 3/4] ARM: dts: aspeed: ast2600evb: Add dts file for A1 and A0.

[Steven Lee]

aspeed-ast2600-evb.dts was modified for supporting A2 evb.
Since A1/A0 evbs don't have GPIO regulators and SD clock frequency(SCU210)
is different to A2 as well. Adding a new dts that removes new nodes
created in aspeed-ast2600-evb.dts is necessary.

Signed-off-by: Steven Lee <steven_lee@aspeedtech.com>
---
 arch/arm/boot/dts/aspeed-ast2600-evb-a1.dts | 15 +++++++++++++++
 1 file changed, 15 insertions(+)
 create mode 100644 arch/arm/boot/dts/aspeed-ast2600-evb-a1.dts

diff --git a/arch/arm/boot/dts/aspeed-ast2600-evb-a1.dts b/arch/arm/boot/dts/aspeed-ast2600-evb-a1.dts
new file mode 100644
index 000000000000..dd7148060c4a
--- /dev/null
+++ b/arch/arm/boot/dts/aspeed-ast2600-evb-a1.dts
@@ -0,0 +1,15 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+// Copyright 2021 IBM Corp.
+
+#include "aspeed-ast2600-evb.dts"
+
+/ {
+	model = "AST2600 A1 EVB";
+
+	/delete-node/regulator-vcc-sdhci0;
+	/delete-node/regulator-vcc-sdhci1;
+	/delete-node/regulator-vccq-sdhci0;
+	/delete-node/regulator-vccq-sdhci1;
+};
+
+/delete-node/ &sdc;
-- 
2.17.1


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

[PATCH v5 3/4] ARM: dts: aspeed: ast2600evb: Add dts file for A1 and A0.

[Steven Lee]

aspeed-ast2600-evb.dts was modified for supporting A2 evb.
Since A1/A0 evbs don't have GPIO regulators and SD clock frequency(SCU210)
is different to A2 as well. Adding a new dts that removes new nodes
created in aspeed-ast2600-evb.dts is necessary.

Signed-off-by: Steven Lee <steven_lee@aspeedtech.com>
---
 arch/arm/boot/dts/aspeed-ast2600-evb-a1.dts | 15 +++++++++++++++
 1 file changed, 15 insertions(+)
 create mode 100644 arch/arm/boot/dts/aspeed-ast2600-evb-a1.dts

diff --git a/arch/arm/boot/dts/aspeed-ast2600-evb-a1.dts b/arch/arm/boot/dts/aspeed-ast2600-evb-a1.dts
new file mode 100644
index 000000000000..dd7148060c4a
--- /dev/null
+++ b/arch/arm/boot/dts/aspeed-ast2600-evb-a1.dts
@@ -0,0 +1,15 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+// Copyright 2021 IBM Corp.
+
+#include "aspeed-ast2600-evb.dts"
+
+/ {
+	model = "AST2600 A1 EVB";
+
+	/delete-node/regulator-vcc-sdhci0;
+	/delete-node/regulator-vcc-sdhci1;
+	/delete-node/regulator-vccq-sdhci0;
+	/delete-node/regulator-vccq-sdhci1;
+};
+
+/delete-node/ &sdc;
-- 
2.17.1

Re: [PATCH v5 3/4] ARM: dts: aspeed: ast2600evb: Add dts file for A1 and A0.

[Andrew Jeffery]

On Mon, 24 May 2021, at 17:02, Steven Lee wrote:
> aspeed-ast2600-evb.dts was modified for supporting A2 evb.
> Since A1/A0 evbs don't have GPIO regulators and SD clock frequency(SCU210)
> is different to A2 as well. Adding a new dts that removes new nodes
> created in aspeed-ast2600-evb.dts is necessary.
> 
> Signed-off-by: Steven Lee <steven_lee@aspeedtech.com>

Acked-by: Andrew Jeffery <andrew@aj.id.au>

Re: [PATCH v5 3/4] ARM: dts: aspeed: ast2600evb: Add dts file for A1 and A0.

[Andrew Jeffery]

On Mon, 24 May 2021, at 17:02, Steven Lee wrote:
> aspeed-ast2600-evb.dts was modified for supporting A2 evb.
> Since A1/A0 evbs don't have GPIO regulators and SD clock frequency(SCU210)
> is different to A2 as well. Adding a new dts that removes new nodes
> created in aspeed-ast2600-evb.dts is necessary.
> 
> Signed-off-by: Steven Lee <steven_lee@aspeedtech.com>

Acked-by: Andrew Jeffery <andrew@aj.id.au>

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

Re: [PATCH v5 3/4] ARM: dts: aspeed: ast2600evb: Add dts file for A1 and A0.

[Andrew Jeffery]

On Mon, 24 May 2021, at 17:02, Steven Lee wrote:
> aspeed-ast2600-evb.dts was modified for supporting A2 evb.
> Since A1/A0 evbs don't have GPIO regulators and SD clock frequency(SCU210)
> is different to A2 as well. Adding a new dts that removes new nodes
> created in aspeed-ast2600-evb.dts is necessary.
> 
> Signed-off-by: Steven Lee <steven_lee@aspeedtech.com>

Acked-by: Andrew Jeffery <andrew@aj.id.au>

[PATCH v5 4/4] mmc: sdhci-of-aspeed: Configure the SDHCIs as specified by the devicetree.

[Steven Lee]

The hardware provides capability configuration registers for each SDHCI
in the global configuration space for the SD controller. Writes to the
global capability registers are mirrored to the capability registers in
the associated SDHCI. Configuration of the capabilities must be written
through the mirror registers prior to initialisation of the SDHCI.

Signed-off-by: Steven Lee <steven_lee@aspeedtech.com>
Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
Reviewed-by: Joel Stanley <joel@jms.id.au>
---
 drivers/mmc/host/sdhci-of-aspeed.c | 48 ++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/drivers/mmc/host/sdhci-of-aspeed.c b/drivers/mmc/host/sdhci-of-aspeed.c
index d001c51074a0..65b5685f6c15 100644
--- a/drivers/mmc/host/sdhci-of-aspeed.c
+++ b/drivers/mmc/host/sdhci-of-aspeed.c
@@ -31,6 +31,11 @@
 #define   ASPEED_SDC_S0_PHASE_OUT_EN	GENMASK(1, 0)
 #define   ASPEED_SDC_PHASE_MAX		31
 
+/* SDIO{10,20} */
+#define ASPEED_SDC_CAP1_1_8V           (0 * 32 + 26)
+/* SDIO{14,24} */
+#define ASPEED_SDC_CAP2_SDR104         (1 * 32 + 1)
+
 struct aspeed_sdc {
 	struct clk *clk;
 	struct resource *res;
@@ -72,6 +77,37 @@ struct aspeed_sdhci {
 	const struct aspeed_sdhci_phase_desc *phase_desc;
 };
 
+/*
+ * The function sets the mirror register for updating
+ * capbilities of the current slot.
+ *
+ *   slot | capability  | caps_reg | mirror_reg
+ *   -----|-------------|----------|------------
+ *     0  | CAP1_1_8V   | SDIO140  |   SDIO10
+ *     0  | CAP2_SDR104 | SDIO144  |   SDIO14
+ *     1  | CAP1_1_8V   | SDIO240  |   SDIO20
+ *     1  | CAP2_SDR104 | SDIO244  |   SDIO24
+ */
+static void aspeed_sdc_set_slot_capability(struct sdhci_host *host, struct aspeed_sdc *sdc,
+					   int capability, bool enable, u8 slot)
+{
+	u32 mirror_reg_offset;
+	u32 cap_val;
+	u8 cap_reg;
+
+	if (slot > 1)
+		return;
+
+	cap_reg = capability / 32;
+	cap_val = sdhci_readl(host, 0x40 + (cap_reg * 4));
+	if (enable)
+		cap_val |= BIT(capability % 32);
+	else
+		cap_val &= ~BIT(capability % 32);
+	mirror_reg_offset = ((slot + 1) * 0x10) + (cap_reg * 4);
+	writel(cap_val, sdc->regs + mirror_reg_offset);
+}
+
 static void aspeed_sdc_configure_8bit_mode(struct aspeed_sdc *sdc,
 					   struct aspeed_sdhci *sdhci,
 					   bool bus8)
@@ -328,6 +364,7 @@ static inline int aspeed_sdhci_calculate_slot(struct aspeed_sdhci *dev,
 static int aspeed_sdhci_probe(struct platform_device *pdev)
 {
 	const struct aspeed_sdhci_pdata *aspeed_pdata;
+	struct device_node *np = pdev->dev.of_node;
 	struct sdhci_pltfm_host *pltfm_host;
 	struct aspeed_sdhci *dev;
 	struct sdhci_host *host;
@@ -372,6 +409,17 @@ static int aspeed_sdhci_probe(struct platform_device *pdev)
 
 	sdhci_get_of_property(pdev);
 
+	if (of_property_read_bool(np, "mmc-hs200-1_8v") ||
+	    of_property_read_bool(np, "sd-uhs-sdr104")) {
+		aspeed_sdc_set_slot_capability(host, dev->parent, ASPEED_SDC_CAP1_1_8V,
+					       true, slot);
+	}
+
+	if (of_property_read_bool(np, "sd-uhs-sdr104")) {
+		aspeed_sdc_set_slot_capability(host, dev->parent, ASPEED_SDC_CAP2_SDR104,
+					       true, slot);
+	}
+
 	pltfm_host->clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(pltfm_host->clk))
 		return PTR_ERR(pltfm_host->clk);
-- 
2.17.1

[PATCH v5 4/4] mmc: sdhci-of-aspeed: Configure the SDHCIs as specified by the devicetree.

[Steven Lee]

The hardware provides capability configuration registers for each SDHCI
in the global configuration space for the SD controller. Writes to the
global capability registers are mirrored to the capability registers in
the associated SDHCI. Configuration of the capabilities must be written
through the mirror registers prior to initialisation of the SDHCI.

Signed-off-by: Steven Lee <steven_lee@aspeedtech.com>
Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
Reviewed-by: Joel Stanley <joel@jms.id.au>
---
 drivers/mmc/host/sdhci-of-aspeed.c | 48 ++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/drivers/mmc/host/sdhci-of-aspeed.c b/drivers/mmc/host/sdhci-of-aspeed.c
index d001c51074a0..65b5685f6c15 100644
--- a/drivers/mmc/host/sdhci-of-aspeed.c
+++ b/drivers/mmc/host/sdhci-of-aspeed.c
@@ -31,6 +31,11 @@
 #define   ASPEED_SDC_S0_PHASE_OUT_EN	GENMASK(1, 0)
 #define   ASPEED_SDC_PHASE_MAX		31
 
+/* SDIO{10,20} */
+#define ASPEED_SDC_CAP1_1_8V           (0 * 32 + 26)
+/* SDIO{14,24} */
+#define ASPEED_SDC_CAP2_SDR104         (1 * 32 + 1)
+
 struct aspeed_sdc {
 	struct clk *clk;
 	struct resource *res;
@@ -72,6 +77,37 @@ struct aspeed_sdhci {
 	const struct aspeed_sdhci_phase_desc *phase_desc;
 };
 
+/*
+ * The function sets the mirror register for updating
+ * capbilities of the current slot.
+ *
+ *   slot | capability  | caps_reg | mirror_reg
+ *   -----|-------------|----------|------------
+ *     0  | CAP1_1_8V   | SDIO140  |   SDIO10
+ *     0  | CAP2_SDR104 | SDIO144  |   SDIO14
+ *     1  | CAP1_1_8V   | SDIO240  |   SDIO20
+ *     1  | CAP2_SDR104 | SDIO244  |   SDIO24
+ */
+static void aspeed_sdc_set_slot_capability(struct sdhci_host *host, struct aspeed_sdc *sdc,
+					   int capability, bool enable, u8 slot)
+{
+	u32 mirror_reg_offset;
+	u32 cap_val;
+	u8 cap_reg;
+
+	if (slot > 1)
+		return;
+
+	cap_reg = capability / 32;
+	cap_val = sdhci_readl(host, 0x40 + (cap_reg * 4));
+	if (enable)
+		cap_val |= BIT(capability % 32);
+	else
+		cap_val &= ~BIT(capability % 32);
+	mirror_reg_offset = ((slot + 1) * 0x10) + (cap_reg * 4);
+	writel(cap_val, sdc->regs + mirror_reg_offset);
+}
+
 static void aspeed_sdc_configure_8bit_mode(struct aspeed_sdc *sdc,
 					   struct aspeed_sdhci *sdhci,
 					   bool bus8)
@@ -328,6 +364,7 @@ static inline int aspeed_sdhci_calculate_slot(struct aspeed_sdhci *dev,
 static int aspeed_sdhci_probe(struct platform_device *pdev)
 {
 	const struct aspeed_sdhci_pdata *aspeed_pdata;
+	struct device_node *np = pdev->dev.of_node;
 	struct sdhci_pltfm_host *pltfm_host;
 	struct aspeed_sdhci *dev;
 	struct sdhci_host *host;
@@ -372,6 +409,17 @@ static int aspeed_sdhci_probe(struct platform_device *pdev)
 
 	sdhci_get_of_property(pdev);
 
+	if (of_property_read_bool(np, "mmc-hs200-1_8v") ||
+	    of_property_read_bool(np, "sd-uhs-sdr104")) {
+		aspeed_sdc_set_slot_capability(host, dev->parent, ASPEED_SDC_CAP1_1_8V,
+					       true, slot);
+	}
+
+	if (of_property_read_bool(np, "sd-uhs-sdr104")) {
+		aspeed_sdc_set_slot_capability(host, dev->parent, ASPEED_SDC_CAP2_SDR104,
+					       true, slot);
+	}
+
 	pltfm_host->clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(pltfm_host->clk))
 		return PTR_ERR(pltfm_host->clk);
-- 
2.17.1


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

[PATCH v5 4/4] mmc: sdhci-of-aspeed: Configure the SDHCIs as specified by the devicetree.

[Steven Lee]

The hardware provides capability configuration registers for each SDHCI
in the global configuration space for the SD controller. Writes to the
global capability registers are mirrored to the capability registers in
the associated SDHCI. Configuration of the capabilities must be written
through the mirror registers prior to initialisation of the SDHCI.

Signed-off-by: Steven Lee <steven_lee@aspeedtech.com>
Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
Reviewed-by: Joel Stanley <joel@jms.id.au>
---
 drivers/mmc/host/sdhci-of-aspeed.c | 48 ++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/drivers/mmc/host/sdhci-of-aspeed.c b/drivers/mmc/host/sdhci-of-aspeed.c
index d001c51074a0..65b5685f6c15 100644
--- a/drivers/mmc/host/sdhci-of-aspeed.c
+++ b/drivers/mmc/host/sdhci-of-aspeed.c
@@ -31,6 +31,11 @@
 #define   ASPEED_SDC_S0_PHASE_OUT_EN	GENMASK(1, 0)
 #define   ASPEED_SDC_PHASE_MAX		31
 
+/* SDIO{10,20} */
+#define ASPEED_SDC_CAP1_1_8V           (0 * 32 + 26)
+/* SDIO{14,24} */
+#define ASPEED_SDC_CAP2_SDR104         (1 * 32 + 1)
+
 struct aspeed_sdc {
 	struct clk *clk;
 	struct resource *res;
@@ -72,6 +77,37 @@ struct aspeed_sdhci {
 	const struct aspeed_sdhci_phase_desc *phase_desc;
 };
 
+/*
+ * The function sets the mirror register for updating
+ * capbilities of the current slot.
+ *
+ *   slot | capability  | caps_reg | mirror_reg
+ *   -----|-------------|----------|------------
+ *     0  | CAP1_1_8V   | SDIO140  |   SDIO10
+ *     0  | CAP2_SDR104 | SDIO144  |   SDIO14
+ *     1  | CAP1_1_8V   | SDIO240  |   SDIO20
+ *     1  | CAP2_SDR104 | SDIO244  |   SDIO24
+ */
+static void aspeed_sdc_set_slot_capability(struct sdhci_host *host, struct aspeed_sdc *sdc,
+					   int capability, bool enable, u8 slot)
+{
+	u32 mirror_reg_offset;
+	u32 cap_val;
+	u8 cap_reg;
+
+	if (slot > 1)
+		return;
+
+	cap_reg = capability / 32;
+	cap_val = sdhci_readl(host, 0x40 + (cap_reg * 4));
+	if (enable)
+		cap_val |= BIT(capability % 32);
+	else
+		cap_val &= ~BIT(capability % 32);
+	mirror_reg_offset = ((slot + 1) * 0x10) + (cap_reg * 4);
+	writel(cap_val, sdc->regs + mirror_reg_offset);
+}
+
 static void aspeed_sdc_configure_8bit_mode(struct aspeed_sdc *sdc,
 					   struct aspeed_sdhci *sdhci,
 					   bool bus8)
@@ -328,6 +364,7 @@ static inline int aspeed_sdhci_calculate_slot(struct aspeed_sdhci *dev,
 static int aspeed_sdhci_probe(struct platform_device *pdev)
 {
 	const struct aspeed_sdhci_pdata *aspeed_pdata;
+	struct device_node *np = pdev->dev.of_node;
 	struct sdhci_pltfm_host *pltfm_host;
 	struct aspeed_sdhci *dev;
 	struct sdhci_host *host;
@@ -372,6 +409,17 @@ static int aspeed_sdhci_probe(struct platform_device *pdev)
 
 	sdhci_get_of_property(pdev);
 
+	if (of_property_read_bool(np, "mmc-hs200-1_8v") ||
+	    of_property_read_bool(np, "sd-uhs-sdr104")) {
+		aspeed_sdc_set_slot_capability(host, dev->parent, ASPEED_SDC_CAP1_1_8V,
+					       true, slot);
+	}
+
+	if (of_property_read_bool(np, "sd-uhs-sdr104")) {
+		aspeed_sdc_set_slot_capability(host, dev->parent, ASPEED_SDC_CAP2_SDR104,
+					       true, slot);
+	}
+
 	pltfm_host->clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(pltfm_host->clk))
 		return PTR_ERR(pltfm_host->clk);
-- 
2.17.1

Re: [PATCH v5 4/4] mmc: sdhci-of-aspeed: Configure the SDHCIs as specified by the devicetree.

[Ulf Hansson]

On Mon, 24 May 2021 at 09:33, Steven Lee <steven_lee@aspeedtech.com> wrote:
>
> The hardware provides capability configuration registers for each SDHCI
> in the global configuration space for the SD controller. Writes to the
> global capability registers are mirrored to the capability registers in
> the associated SDHCI. Configuration of the capabilities must be written
> through the mirror registers prior to initialisation of the SDHCI.
>
> Signed-off-by: Steven Lee <steven_lee@aspeedtech.com>
> Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
> Reviewed-by: Joel Stanley <joel@jms.id.au>

Applied for next, thanks!

Kind regards
Uffe


> ---
>  drivers/mmc/host/sdhci-of-aspeed.c | 48 ++++++++++++++++++++++++++++++
>  1 file changed, 48 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-of-aspeed.c b/drivers/mmc/host/sdhci-of-aspeed.c
> index d001c51074a0..65b5685f6c15 100644
> --- a/drivers/mmc/host/sdhci-of-aspeed.c
> +++ b/drivers/mmc/host/sdhci-of-aspeed.c
> @@ -31,6 +31,11 @@
>  #define   ASPEED_SDC_S0_PHASE_OUT_EN   GENMASK(1, 0)
>  #define   ASPEED_SDC_PHASE_MAX         31
>
> +/* SDIO{10,20} */
> +#define ASPEED_SDC_CAP1_1_8V           (0 * 32 + 26)
> +/* SDIO{14,24} */
> +#define ASPEED_SDC_CAP2_SDR104         (1 * 32 + 1)
> +
>  struct aspeed_sdc {
>         struct clk *clk;
>         struct resource *res;
> @@ -72,6 +77,37 @@ struct aspeed_sdhci {
>         const struct aspeed_sdhci_phase_desc *phase_desc;
>  };
>
> +/*
> + * The function sets the mirror register for updating
> + * capbilities of the current slot.
> + *
> + *   slot | capability  | caps_reg | mirror_reg
> + *   -----|-------------|----------|------------
> + *     0  | CAP1_1_8V   | SDIO140  |   SDIO10
> + *     0  | CAP2_SDR104 | SDIO144  |   SDIO14
> + *     1  | CAP1_1_8V   | SDIO240  |   SDIO20
> + *     1  | CAP2_SDR104 | SDIO244  |   SDIO24
> + */
> +static void aspeed_sdc_set_slot_capability(struct sdhci_host *host, struct aspeed_sdc *sdc,
> +                                          int capability, bool enable, u8 slot)
> +{
> +       u32 mirror_reg_offset;
> +       u32 cap_val;
> +       u8 cap_reg;
> +
> +       if (slot > 1)
> +               return;
> +
> +       cap_reg = capability / 32;
> +       cap_val = sdhci_readl(host, 0x40 + (cap_reg * 4));
> +       if (enable)
> +               cap_val |= BIT(capability % 32);
> +       else
> +               cap_val &= ~BIT(capability % 32);
> +       mirror_reg_offset = ((slot + 1) * 0x10) + (cap_reg * 4);
> +       writel(cap_val, sdc->regs + mirror_reg_offset);
> +}
> +
>  static void aspeed_sdc_configure_8bit_mode(struct aspeed_sdc *sdc,
>                                            struct aspeed_sdhci *sdhci,
>                                            bool bus8)
> @@ -328,6 +364,7 @@ static inline int aspeed_sdhci_calculate_slot(struct aspeed_sdhci *dev,
>  static int aspeed_sdhci_probe(struct platform_device *pdev)
>  {
>         const struct aspeed_sdhci_pdata *aspeed_pdata;
> +       struct device_node *np = pdev->dev.of_node;
>         struct sdhci_pltfm_host *pltfm_host;
>         struct aspeed_sdhci *dev;
>         struct sdhci_host *host;
> @@ -372,6 +409,17 @@ static int aspeed_sdhci_probe(struct platform_device *pdev)
>
>         sdhci_get_of_property(pdev);
>
> +       if (of_property_read_bool(np, "mmc-hs200-1_8v") ||
> +           of_property_read_bool(np, "sd-uhs-sdr104")) {
> +               aspeed_sdc_set_slot_capability(host, dev->parent, ASPEED_SDC_CAP1_1_8V,
> +                                              true, slot);
> +       }
> +
> +       if (of_property_read_bool(np, "sd-uhs-sdr104")) {
> +               aspeed_sdc_set_slot_capability(host, dev->parent, ASPEED_SDC_CAP2_SDR104,
> +                                              true, slot);
> +       }
> +
>         pltfm_host->clk = devm_clk_get(&pdev->dev, NULL);
>         if (IS_ERR(pltfm_host->clk))
>                 return PTR_ERR(pltfm_host->clk);
> --
> 2.17.1
>

Re: [PATCH v5 4/4] mmc: sdhci-of-aspeed: Configure the SDHCIs as specified by the devicetree.

[Ulf Hansson]

On Mon, 24 May 2021 at 09:33, Steven Lee <steven_lee@aspeedtech.com> wrote:
>
> The hardware provides capability configuration registers for each SDHCI
> in the global configuration space for the SD controller. Writes to the
> global capability registers are mirrored to the capability registers in
> the associated SDHCI. Configuration of the capabilities must be written
> through the mirror registers prior to initialisation of the SDHCI.
>
> Signed-off-by: Steven Lee <steven_lee@aspeedtech.com>
> Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
> Reviewed-by: Joel Stanley <joel@jms.id.au>

Applied for next, thanks!

Kind regards
Uffe


> ---
>  drivers/mmc/host/sdhci-of-aspeed.c | 48 ++++++++++++++++++++++++++++++
>  1 file changed, 48 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-of-aspeed.c b/drivers/mmc/host/sdhci-of-aspeed.c
> index d001c51074a0..65b5685f6c15 100644
> --- a/drivers/mmc/host/sdhci-of-aspeed.c
> +++ b/drivers/mmc/host/sdhci-of-aspeed.c
> @@ -31,6 +31,11 @@
>  #define   ASPEED_SDC_S0_PHASE_OUT_EN   GENMASK(1, 0)
>  #define   ASPEED_SDC_PHASE_MAX         31
>
> +/* SDIO{10,20} */
> +#define ASPEED_SDC_CAP1_1_8V           (0 * 32 + 26)
> +/* SDIO{14,24} */
> +#define ASPEED_SDC_CAP2_SDR104         (1 * 32 + 1)
> +
>  struct aspeed_sdc {
>         struct clk *clk;
>         struct resource *res;
> @@ -72,6 +77,37 @@ struct aspeed_sdhci {
>         const struct aspeed_sdhci_phase_desc *phase_desc;
>  };
>
> +/*
> + * The function sets the mirror register for updating
> + * capbilities of the current slot.
> + *
> + *   slot | capability  | caps_reg | mirror_reg
> + *   -----|-------------|----------|------------
> + *     0  | CAP1_1_8V   | SDIO140  |   SDIO10
> + *     0  | CAP2_SDR104 | SDIO144  |   SDIO14
> + *     1  | CAP1_1_8V   | SDIO240  |   SDIO20
> + *     1  | CAP2_SDR104 | SDIO244  |   SDIO24
> + */
> +static void aspeed_sdc_set_slot_capability(struct sdhci_host *host, struct aspeed_sdc *sdc,
> +                                          int capability, bool enable, u8 slot)
> +{
> +       u32 mirror_reg_offset;
> +       u32 cap_val;
> +       u8 cap_reg;
> +
> +       if (slot > 1)
> +               return;
> +
> +       cap_reg = capability / 32;
> +       cap_val = sdhci_readl(host, 0x40 + (cap_reg * 4));
> +       if (enable)
> +               cap_val |= BIT(capability % 32);
> +       else
> +               cap_val &= ~BIT(capability % 32);
> +       mirror_reg_offset = ((slot + 1) * 0x10) + (cap_reg * 4);
> +       writel(cap_val, sdc->regs + mirror_reg_offset);
> +}
> +
>  static void aspeed_sdc_configure_8bit_mode(struct aspeed_sdc *sdc,
>                                            struct aspeed_sdhci *sdhci,
>                                            bool bus8)
> @@ -328,6 +364,7 @@ static inline int aspeed_sdhci_calculate_slot(struct aspeed_sdhci *dev,
>  static int aspeed_sdhci_probe(struct platform_device *pdev)
>  {
>         const struct aspeed_sdhci_pdata *aspeed_pdata;
> +       struct device_node *np = pdev->dev.of_node;
>         struct sdhci_pltfm_host *pltfm_host;
>         struct aspeed_sdhci *dev;
>         struct sdhci_host *host;
> @@ -372,6 +409,17 @@ static int aspeed_sdhci_probe(struct platform_device *pdev)
>
>         sdhci_get_of_property(pdev);
>
> +       if (of_property_read_bool(np, "mmc-hs200-1_8v") ||
> +           of_property_read_bool(np, "sd-uhs-sdr104")) {
> +               aspeed_sdc_set_slot_capability(host, dev->parent, ASPEED_SDC_CAP1_1_8V,
> +                                              true, slot);
> +       }
> +
> +       if (of_property_read_bool(np, "sd-uhs-sdr104")) {
> +               aspeed_sdc_set_slot_capability(host, dev->parent, ASPEED_SDC_CAP2_SDR104,
> +                                              true, slot);
> +       }
> +
>         pltfm_host->clk = devm_clk_get(&pdev->dev, NULL);
>         if (IS_ERR(pltfm_host->clk))
>                 return PTR_ERR(pltfm_host->clk);
> --
> 2.17.1
>

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

Re: [PATCH v5 4/4] mmc: sdhci-of-aspeed: Configure the SDHCIs as specified by the devicetree.

[Ulf Hansson]

On Mon, 24 May 2021 at 09:33, Steven Lee <steven_lee@aspeedtech.com> wrote:
>
> The hardware provides capability configuration registers for each SDHCI
> in the global configuration space for the SD controller. Writes to the
> global capability registers are mirrored to the capability registers in
> the associated SDHCI. Configuration of the capabilities must be written
> through the mirror registers prior to initialisation of the SDHCI.
>
> Signed-off-by: Steven Lee <steven_lee@aspeedtech.com>
> Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
> Reviewed-by: Joel Stanley <joel@jms.id.au>

Applied for next, thanks!

Kind regards
Uffe


> ---
>  drivers/mmc/host/sdhci-of-aspeed.c | 48 ++++++++++++++++++++++++++++++
>  1 file changed, 48 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-of-aspeed.c b/drivers/mmc/host/sdhci-of-aspeed.c
> index d001c51074a0..65b5685f6c15 100644
> --- a/drivers/mmc/host/sdhci-of-aspeed.c
> +++ b/drivers/mmc/host/sdhci-of-aspeed.c
> @@ -31,6 +31,11 @@
>  #define   ASPEED_SDC_S0_PHASE_OUT_EN   GENMASK(1, 0)
>  #define   ASPEED_SDC_PHASE_MAX         31
>
> +/* SDIO{10,20} */
> +#define ASPEED_SDC_CAP1_1_8V           (0 * 32 + 26)
> +/* SDIO{14,24} */
> +#define ASPEED_SDC_CAP2_SDR104         (1 * 32 + 1)
> +
>  struct aspeed_sdc {
>         struct clk *clk;
>         struct resource *res;
> @@ -72,6 +77,37 @@ struct aspeed_sdhci {
>         const struct aspeed_sdhci_phase_desc *phase_desc;
>  };
>
> +/*
> + * The function sets the mirror register for updating
> + * capbilities of the current slot.
> + *
> + *   slot | capability  | caps_reg | mirror_reg
> + *   -----|-------------|----------|------------
> + *     0  | CAP1_1_8V   | SDIO140  |   SDIO10
> + *     0  | CAP2_SDR104 | SDIO144  |   SDIO14
> + *     1  | CAP1_1_8V   | SDIO240  |   SDIO20
> + *     1  | CAP2_SDR104 | SDIO244  |   SDIO24
> + */
> +static void aspeed_sdc_set_slot_capability(struct sdhci_host *host, struct aspeed_sdc *sdc,
> +                                          int capability, bool enable, u8 slot)
> +{
> +       u32 mirror_reg_offset;
> +       u32 cap_val;
> +       u8 cap_reg;
> +
> +       if (slot > 1)
> +               return;
> +
> +       cap_reg = capability / 32;
> +       cap_val = sdhci_readl(host, 0x40 + (cap_reg * 4));
> +       if (enable)
> +               cap_val |= BIT(capability % 32);
> +       else
> +               cap_val &= ~BIT(capability % 32);
> +       mirror_reg_offset = ((slot + 1) * 0x10) + (cap_reg * 4);
> +       writel(cap_val, sdc->regs + mirror_reg_offset);
> +}
> +
>  static void aspeed_sdc_configure_8bit_mode(struct aspeed_sdc *sdc,
>                                            struct aspeed_sdhci *sdhci,
>                                            bool bus8)
> @@ -328,6 +364,7 @@ static inline int aspeed_sdhci_calculate_slot(struct aspeed_sdhci *dev,
>  static int aspeed_sdhci_probe(struct platform_device *pdev)
>  {
>         const struct aspeed_sdhci_pdata *aspeed_pdata;
> +       struct device_node *np = pdev->dev.of_node;
>         struct sdhci_pltfm_host *pltfm_host;
>         struct aspeed_sdhci *dev;
>         struct sdhci_host *host;
> @@ -372,6 +409,17 @@ static int aspeed_sdhci_probe(struct platform_device *pdev)
>
>         sdhci_get_of_property(pdev);
>
> +       if (of_property_read_bool(np, "mmc-hs200-1_8v") ||
> +           of_property_read_bool(np, "sd-uhs-sdr104")) {
> +               aspeed_sdc_set_slot_capability(host, dev->parent, ASPEED_SDC_CAP1_1_8V,
> +                                              true, slot);
> +       }
> +
> +       if (of_property_read_bool(np, "sd-uhs-sdr104")) {
> +               aspeed_sdc_set_slot_capability(host, dev->parent, ASPEED_SDC_CAP2_SDR104,
> +                                              true, slot);
> +       }
> +
>         pltfm_host->clk = devm_clk_get(&pdev->dev, NULL);
>         if (IS_ERR(pltfm_host->clk))
>                 return PTR_ERR(pltfm_host->clk);
> --
> 2.17.1
>

Re: [PATCH v5 0/4] mmc: sdhci-of-aspeed: Support toggling SD bus signal

[Joel Stanley]

On Mon, 24 May 2021 at 07:33, Steven Lee <steven_lee@aspeedtech.com> wrote:
>
> AST2600-A2 EVB has the reference design for enabling SD bus
> power and toggling SD bus signal voltage between 3.3v and 1.8v by
> GPIO regulators.
> This patch series adds sdhci node and gpio regulators in a new dts file
> for AST2600-A2 EVB.
> The description of the reference design of AST2600-A2 EVB is added
> in the new dts file.
>
> This patch also include a helper for updating AST2600 sdhci capability
> registers.

The device trees look good:

Reviewed-by: Joel Stanley <joel@jms.id.au>

I've applied patches 1-3 to the aspeed tree for v5.14. I made a little
fix to patch 3 as it needed to add the new device tree to the
makefile.

When I was testing on my A2 EVB I saw this:

[    1.436219] sdhci-aspeed 1e750100.sdhci: Requested out of range
phase tap 192 for 9 degrees of phase compensation at 1562500Hz,
clamping to tap 15
[    1.450913] sdhci-aspeed 1e750100.sdhci: Requested out of range
phase tap 963 for 45 degrees of phase compensation at 1562500Hz,
clamping to tap 15

Do you know what is happening there?

Cheers,

Joel

Re: [PATCH v5 0/4] mmc: sdhci-of-aspeed: Support toggling SD bus signal

[Joel Stanley]

On Mon, 24 May 2021 at 07:33, Steven Lee <steven_lee@aspeedtech.com> wrote:
>
> AST2600-A2 EVB has the reference design for enabling SD bus
> power and toggling SD bus signal voltage between 3.3v and 1.8v by
> GPIO regulators.
> This patch series adds sdhci node and gpio regulators in a new dts file
> for AST2600-A2 EVB.
> The description of the reference design of AST2600-A2 EVB is added
> in the new dts file.
>
> This patch also include a helper for updating AST2600 sdhci capability
> registers.

The device trees look good:

Reviewed-by: Joel Stanley <joel@jms.id.au>

I've applied patches 1-3 to the aspeed tree for v5.14. I made a little
fix to patch 3 as it needed to add the new device tree to the
makefile.

When I was testing on my A2 EVB I saw this:

[    1.436219] sdhci-aspeed 1e750100.sdhci: Requested out of range
phase tap 192 for 9 degrees of phase compensation at 1562500Hz,
clamping to tap 15
[    1.450913] sdhci-aspeed 1e750100.sdhci: Requested out of range
phase tap 963 for 45 degrees of phase compensation at 1562500Hz,
clamping to tap 15

Do you know what is happening there?

Cheers,

Joel

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

Re: [PATCH v5 0/4] mmc: sdhci-of-aspeed: Support toggling SD bus signal

[Joel Stanley]

On Mon, 24 May 2021 at 07:33, Steven Lee <steven_lee@aspeedtech.com> wrote:
>
> AST2600-A2 EVB has the reference design for enabling SD bus
> power and toggling SD bus signal voltage between 3.3v and 1.8v by
> GPIO regulators.
> This patch series adds sdhci node and gpio regulators in a new dts file
> for AST2600-A2 EVB.
> The description of the reference design of AST2600-A2 EVB is added
> in the new dts file.
>
> This patch also include a helper for updating AST2600 sdhci capability
> registers.

The device trees look good:

Reviewed-by: Joel Stanley <joel@jms.id.au>

I've applied patches 1-3 to the aspeed tree for v5.14. I made a little
fix to patch 3 as it needed to add the new device tree to the
makefile.

When I was testing on my A2 EVB I saw this:

[    1.436219] sdhci-aspeed 1e750100.sdhci: Requested out of range
phase tap 192 for 9 degrees of phase compensation at 1562500Hz,
clamping to tap 15
[    1.450913] sdhci-aspeed 1e750100.sdhci: Requested out of range
phase tap 963 for 45 degrees of phase compensation at 1562500Hz,
clamping to tap 15

Do you know what is happening there?

Cheers,

Joel

Re: [PATCH v5 0/4] mmc: sdhci-of-aspeed: Support toggling SD bus signal

[Steven Lee]

The 05/25/2021 15:55, Joel Stanley wrote:
> On Mon, 24 May 2021 at 07:33, Steven Lee <steven_lee@aspeedtech.com> wrote:
> >
> > AST2600-A2 EVB has the reference design for enabling SD bus
> > power and toggling SD bus signal voltage between 3.3v and 1.8v by
> > GPIO regulators.
> > This patch series adds sdhci node and gpio regulators in a new dts file
> > for AST2600-A2 EVB.
> > The description of the reference design of AST2600-A2 EVB is added
> > in the new dts file.
> >
> > This patch also include a helper for updating AST2600 sdhci capability
> > registers.
> 
> The device trees look good:
> 
> Reviewed-by: Joel Stanley <joel@jms.id.au>
> 
> I've applied patches 1-3 to the aspeed tree for v5.14. I made a little
> fix to patch 3 as it needed to add the new device tree to the
> makefile.
> 

Thanks!

> When I was testing on my A2 EVB I saw this:
> 
> [    1.436219] sdhci-aspeed 1e750100.sdhci: Requested out of range
> phase tap 192 for 9 degrees of phase compensation at 1562500Hz,
> clamping to tap 15
> [    1.450913] sdhci-aspeed 1e750100.sdhci: Requested out of range
> phase tap 963 for 45 degrees of phase compensation at 1562500Hz,
> clamping to tap 15
> 
> Do you know what is happening there?
> 

Per MMC spec, eMMC bus speed is set as legacy mode(0~26MHz) at startup of
eMMC initializtion flow. Clock phase calculation is triggered in set_clock()
and it calculates taps based on phase_deg(<9>, <225>) in the dts file and the
current speed(1562500Hz), which causes the warning message you mentioned.
As the phase_deg in the dts file should be calculated with 100MHz.

https://lkml.org/lkml/2021/5/24/95

But after some initialization flow, eMMC bus speed will be set to
correct speed(100MHz).
Clock phase calculation will be triggered again to get correct taps.

> Cheers,
> 
> Joel

Re: [PATCH v5 0/4] mmc: sdhci-of-aspeed: Support toggling SD bus signal

[Steven Lee]

The 05/25/2021 15:55, Joel Stanley wrote:
> On Mon, 24 May 2021 at 07:33, Steven Lee <steven_lee@aspeedtech.com> wrote:
> >
> > AST2600-A2 EVB has the reference design for enabling SD bus
> > power and toggling SD bus signal voltage between 3.3v and 1.8v by
> > GPIO regulators.
> > This patch series adds sdhci node and gpio regulators in a new dts file
> > for AST2600-A2 EVB.
> > The description of the reference design of AST2600-A2 EVB is added
> > in the new dts file.
> >
> > This patch also include a helper for updating AST2600 sdhci capability
> > registers.
> 
> The device trees look good:
> 
> Reviewed-by: Joel Stanley <joel@jms.id.au>
> 
> I've applied patches 1-3 to the aspeed tree for v5.14. I made a little
> fix to patch 3 as it needed to add the new device tree to the
> makefile.
> 

Thanks!

> When I was testing on my A2 EVB I saw this:
> 
> [    1.436219] sdhci-aspeed 1e750100.sdhci: Requested out of range
> phase tap 192 for 9 degrees of phase compensation at 1562500Hz,
> clamping to tap 15
> [    1.450913] sdhci-aspeed 1e750100.sdhci: Requested out of range
> phase tap 963 for 45 degrees of phase compensation at 1562500Hz,
> clamping to tap 15
> 
> Do you know what is happening there?
> 

Per MMC spec, eMMC bus speed is set as legacy mode(0~26MHz) at startup of
eMMC initializtion flow. Clock phase calculation is triggered in set_clock()
and it calculates taps based on phase_deg(<9>, <225>) in the dts file and the
current speed(1562500Hz), which causes the warning message you mentioned.
As the phase_deg in the dts file should be calculated with 100MHz.

https://lkml.org/lkml/2021/5/24/95

But after some initialization flow, eMMC bus speed will be set to
correct speed(100MHz).
Clock phase calculation will be triggered again to get correct taps.

> Cheers,
> 
> Joel

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

Re: [PATCH v5 0/4] mmc: sdhci-of-aspeed: Support toggling SD bus signal

[Steven Lee]

The 05/25/2021 15:55, Joel Stanley wrote:
> On Mon, 24 May 2021 at 07:33, Steven Lee <steven_lee@aspeedtech.com> wrote:
> >
> > AST2600-A2 EVB has the reference design for enabling SD bus
> > power and toggling SD bus signal voltage between 3.3v and 1.8v by
> > GPIO regulators.
> > This patch series adds sdhci node and gpio regulators in a new dts file
> > for AST2600-A2 EVB.
> > The description of the reference design of AST2600-A2 EVB is added
> > in the new dts file.
> >
> > This patch also include a helper for updating AST2600 sdhci capability
> > registers.
> 
> The device trees look good:
> 
> Reviewed-by: Joel Stanley <joel@jms.id.au>
> 
> I've applied patches 1-3 to the aspeed tree for v5.14. I made a little
> fix to patch 3 as it needed to add the new device tree to the
> makefile.
> 

Thanks!

> When I was testing on my A2 EVB I saw this:
> 
> [    1.436219] sdhci-aspeed 1e750100.sdhci: Requested out of range
> phase tap 192 for 9 degrees of phase compensation at 1562500Hz,
> clamping to tap 15
> [    1.450913] sdhci-aspeed 1e750100.sdhci: Requested out of range
> phase tap 963 for 45 degrees of phase compensation at 1562500Hz,
> clamping to tap 15
> 
> Do you know what is happening there?
> 

Per MMC spec, eMMC bus speed is set as legacy mode(0~26MHz) at startup of
eMMC initializtion flow. Clock phase calculation is triggered in set_clock()
and it calculates taps based on phase_deg(<9>, <225>) in the dts file and the
current speed(1562500Hz), which causes the warning message you mentioned.
As the phase_deg in the dts file should be calculated with 100MHz.

https://lkml.org/lkml/2021/5/24/95

But after some initialization flow, eMMC bus speed will be set to
correct speed(100MHz).
Clock phase calculation will be triggered again to get correct taps.

> Cheers,
> 
> Joel

Re: [PATCH v5 0/4] mmc: sdhci-of-aspeed: Support toggling SD bus signal

[Joel Stanley]

On Tue, 25 May 2021 at 09:48, Steven Lee <steven_lee@aspeedtech.com> wrote:
>
> The 05/25/2021 15:55, Joel Stanley wrote:
> > When I was testing on my A2 EVB I saw this:
> >
> > [    1.436219] sdhci-aspeed 1e750100.sdhci: Requested out of range
> > phase tap 192 for 9 degrees of phase compensation at 1562500Hz,
> > clamping to tap 15
> > [    1.450913] sdhci-aspeed 1e750100.sdhci: Requested out of range
> > phase tap 963 for 45 degrees of phase compensation at 1562500Hz,
> > clamping to tap 15
> >
> > Do you know what is happening there?
> >
>
> Per MMC spec, eMMC bus speed is set as legacy mode(0~26MHz) at startup of
> eMMC initializtion flow. Clock phase calculation is triggered in set_clock()
> and it calculates taps based on phase_deg(<9>, <225>) in the dts file and the
> current speed(1562500Hz), which causes the warning message you mentioned.
> As the phase_deg in the dts file should be calculated with 100MHz.
>
> https://lkml.org/lkml/2021/5/24/95
>
> But after some initialization flow, eMMC bus speed will be set to
> correct speed(100MHz).
> Clock phase calculation will be triggered again to get correct taps.

Thanks for the explanation. I added another debug print and I can see
it doing what you describe:

[    1.465904] sdhci-aspeed 1e750100.sdhci: Requested out of range
phase tap 192 for 9 degrees of phase compensation at 1562500Hz,
clamping to tap 15
[    1.480598] sdhci-aspeed 1e750100.sdhci: rate 1562500 phase 9 tap 15
[    1.490316] sdhci-aspeed 1e750100.sdhci: Requested out of range
phase tap 963 for 45 degrees of phase compensation at 1562500Hz,
clamping to tap 15
[    1.505077] sdhci-aspeed 1e750100.sdhci: rate 1562500 phase 45 tap 15
[    1.515059] sdhci-aspeed 1e750100.sdhci: rate 100000000 phase 9 tap 3
[    1.524886] sdhci-aspeed 1e750100.sdhci: rate 100000000 phase 45 tap 15
[    1.534904] sdhci-aspeed 1e750100.sdhci: rate 100000000 phase 9 tap 3
[    1.544713] sdhci-aspeed 1e750100.sdhci: rate 100000000 phase 45 tap 15

We should change the "out of range" message to be dev_dbg, as it is
expected on a normal boot.

Cheers,

Joel

Re: [PATCH v5 0/4] mmc: sdhci-of-aspeed: Support toggling SD bus signal

[Joel Stanley]

On Tue, 25 May 2021 at 09:48, Steven Lee <steven_lee@aspeedtech.com> wrote:
>
> The 05/25/2021 15:55, Joel Stanley wrote:
> > When I was testing on my A2 EVB I saw this:
> >
> > [    1.436219] sdhci-aspeed 1e750100.sdhci: Requested out of range
> > phase tap 192 for 9 degrees of phase compensation at 1562500Hz,
> > clamping to tap 15
> > [    1.450913] sdhci-aspeed 1e750100.sdhci: Requested out of range
> > phase tap 963 for 45 degrees of phase compensation at 1562500Hz,
> > clamping to tap 15
> >
> > Do you know what is happening there?
> >
>
> Per MMC spec, eMMC bus speed is set as legacy mode(0~26MHz) at startup of
> eMMC initializtion flow. Clock phase calculation is triggered in set_clock()
> and it calculates taps based on phase_deg(<9>, <225>) in the dts file and the
> current speed(1562500Hz), which causes the warning message you mentioned.
> As the phase_deg in the dts file should be calculated with 100MHz.
>
> https://lkml.org/lkml/2021/5/24/95
>
> But after some initialization flow, eMMC bus speed will be set to
> correct speed(100MHz).
> Clock phase calculation will be triggered again to get correct taps.

Thanks for the explanation. I added another debug print and I can see
it doing what you describe:

[    1.465904] sdhci-aspeed 1e750100.sdhci: Requested out of range
phase tap 192 for 9 degrees of phase compensation at 1562500Hz,
clamping to tap 15
[    1.480598] sdhci-aspeed 1e750100.sdhci: rate 1562500 phase 9 tap 15
[    1.490316] sdhci-aspeed 1e750100.sdhci: Requested out of range
phase tap 963 for 45 degrees of phase compensation at 1562500Hz,
clamping to tap 15
[    1.505077] sdhci-aspeed 1e750100.sdhci: rate 1562500 phase 45 tap 15
[    1.515059] sdhci-aspeed 1e750100.sdhci: rate 100000000 phase 9 tap 3
[    1.524886] sdhci-aspeed 1e750100.sdhci: rate 100000000 phase 45 tap 15
[    1.534904] sdhci-aspeed 1e750100.sdhci: rate 100000000 phase 9 tap 3
[    1.544713] sdhci-aspeed 1e750100.sdhci: rate 100000000 phase 45 tap 15

We should change the "out of range" message to be dev_dbg, as it is
expected on a normal boot.

Cheers,

Joel

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

Re: [PATCH v5 0/4] mmc: sdhci-of-aspeed: Support toggling SD bus signal

[Joel Stanley]

On Tue, 25 May 2021 at 09:48, Steven Lee <steven_lee@aspeedtech.com> wrote:
>
> The 05/25/2021 15:55, Joel Stanley wrote:
> > When I was testing on my A2 EVB I saw this:
> >
> > [    1.436219] sdhci-aspeed 1e750100.sdhci: Requested out of range
> > phase tap 192 for 9 degrees of phase compensation at 1562500Hz,
> > clamping to tap 15
> > [    1.450913] sdhci-aspeed 1e750100.sdhci: Requested out of range
> > phase tap 963 for 45 degrees of phase compensation at 1562500Hz,
> > clamping to tap 15
> >
> > Do you know what is happening there?
> >
>
> Per MMC spec, eMMC bus speed is set as legacy mode(0~26MHz) at startup of
> eMMC initializtion flow. Clock phase calculation is triggered in set_clock()
> and it calculates taps based on phase_deg(<9>, <225>) in the dts file and the
> current speed(1562500Hz), which causes the warning message you mentioned.
> As the phase_deg in the dts file should be calculated with 100MHz.
>
> https://lkml.org/lkml/2021/5/24/95
>
> But after some initialization flow, eMMC bus speed will be set to
> correct speed(100MHz).
> Clock phase calculation will be triggered again to get correct taps.

Thanks for the explanation. I added another debug print and I can see
it doing what you describe:

[    1.465904] sdhci-aspeed 1e750100.sdhci: Requested out of range
phase tap 192 for 9 degrees of phase compensation at 1562500Hz,
clamping to tap 15
[    1.480598] sdhci-aspeed 1e750100.sdhci: rate 1562500 phase 9 tap 15
[    1.490316] sdhci-aspeed 1e750100.sdhci: Requested out of range
phase tap 963 for 45 degrees of phase compensation at 1562500Hz,
clamping to tap 15
[    1.505077] sdhci-aspeed 1e750100.sdhci: rate 1562500 phase 45 tap 15
[    1.515059] sdhci-aspeed 1e750100.sdhci: rate 100000000 phase 9 tap 3
[    1.524886] sdhci-aspeed 1e750100.sdhci: rate 100000000 phase 45 tap 15
[    1.534904] sdhci-aspeed 1e750100.sdhci: rate 100000000 phase 9 tap 3
[    1.544713] sdhci-aspeed 1e750100.sdhci: rate 100000000 phase 45 tap 15

We should change the "out of range" message to be dev_dbg, as it is
expected on a normal boot.

Cheers,

Joel

Re: [PATCH v5 0/4] mmc: sdhci-of-aspeed: Support toggling SD bus signal

[Andrew Jeffery]

On Tue, 25 May 2021, at 22:26, Joel Stanley wrote:
> On Tue, 25 May 2021 at 09:48, Steven Lee <steven_lee@aspeedtech.com> wrote:
> >
> > The 05/25/2021 15:55, Joel Stanley wrote:
> > > When I was testing on my A2 EVB I saw this:
> > >
> > > [    1.436219] sdhci-aspeed 1e750100.sdhci: Requested out of range
> > > phase tap 192 for 9 degrees of phase compensation at 1562500Hz,
> > > clamping to tap 15
> > > [    1.450913] sdhci-aspeed 1e750100.sdhci: Requested out of range
> > > phase tap 963 for 45 degrees of phase compensation at 1562500Hz,
> > > clamping to tap 15
> > >
> > > Do you know what is happening there?
> > >
> >
> > Per MMC spec, eMMC bus speed is set as legacy mode(0~26MHz) at startup of
> > eMMC initializtion flow. Clock phase calculation is triggered in set_clock()
> > and it calculates taps based on phase_deg(<9>, <225>) in the dts file and the
> > current speed(1562500Hz), which causes the warning message you mentioned.
> > As the phase_deg in the dts file should be calculated with 100MHz.
> >
> > https://lkml.org/lkml/2021/5/24/95
> >
> > But after some initialization flow, eMMC bus speed will be set to
> > correct speed(100MHz).
> > Clock phase calculation will be triggered again to get correct taps.
> 
> Thanks for the explanation. I added another debug print and I can see
> it doing what you describe:
> 
> [    1.465904] sdhci-aspeed 1e750100.sdhci: Requested out of range
> phase tap 192 for 9 degrees of phase compensation at 1562500Hz,
> clamping to tap 15
> [    1.480598] sdhci-aspeed 1e750100.sdhci: rate 1562500 phase 9 tap 15
> [    1.490316] sdhci-aspeed 1e750100.sdhci: Requested out of range
> phase tap 963 for 45 degrees of phase compensation at 1562500Hz,
> clamping to tap 15
> [    1.505077] sdhci-aspeed 1e750100.sdhci: rate 1562500 phase 45 tap 15
> [    1.515059] sdhci-aspeed 1e750100.sdhci: rate 100000000 phase 9 tap 3
> [    1.524886] sdhci-aspeed 1e750100.sdhci: rate 100000000 phase 45 tap 15
> [    1.534904] sdhci-aspeed 1e750100.sdhci: rate 100000000 phase 9 tap 3
> [    1.544713] sdhci-aspeed 1e750100.sdhci: rate 100000000 phase 45 tap 15
> 
> We should change the "out of range" message to be dev_dbg, as it is
> expected on a normal boot.

I would think the issue is rather that we shouldn't be applying a phase 
correction for a bus speed that isn't what the correction was specified 
for.

Let me look at this a bit further.

Andrew

Re: [PATCH v5 0/4] mmc: sdhci-of-aspeed: Support toggling SD bus signal

[Andrew Jeffery]

On Tue, 25 May 2021, at 22:26, Joel Stanley wrote:
> On Tue, 25 May 2021 at 09:48, Steven Lee <steven_lee@aspeedtech.com> wrote:
> >
> > The 05/25/2021 15:55, Joel Stanley wrote:
> > > When I was testing on my A2 EVB I saw this:
> > >
> > > [    1.436219] sdhci-aspeed 1e750100.sdhci: Requested out of range
> > > phase tap 192 for 9 degrees of phase compensation at 1562500Hz,
> > > clamping to tap 15
> > > [    1.450913] sdhci-aspeed 1e750100.sdhci: Requested out of range
> > > phase tap 963 for 45 degrees of phase compensation at 1562500Hz,
> > > clamping to tap 15
> > >
> > > Do you know what is happening there?
> > >
> >
> > Per MMC spec, eMMC bus speed is set as legacy mode(0~26MHz) at startup of
> > eMMC initializtion flow. Clock phase calculation is triggered in set_clock()
> > and it calculates taps based on phase_deg(<9>, <225>) in the dts file and the
> > current speed(1562500Hz), which causes the warning message you mentioned.
> > As the phase_deg in the dts file should be calculated with 100MHz.
> >
> > https://lkml.org/lkml/2021/5/24/95
> >
> > But after some initialization flow, eMMC bus speed will be set to
> > correct speed(100MHz).
> > Clock phase calculation will be triggered again to get correct taps.
> 
> Thanks for the explanation. I added another debug print and I can see
> it doing what you describe:
> 
> [    1.465904] sdhci-aspeed 1e750100.sdhci: Requested out of range
> phase tap 192 for 9 degrees of phase compensation at 1562500Hz,
> clamping to tap 15
> [    1.480598] sdhci-aspeed 1e750100.sdhci: rate 1562500 phase 9 tap 15
> [    1.490316] sdhci-aspeed 1e750100.sdhci: Requested out of range
> phase tap 963 for 45 degrees of phase compensation at 1562500Hz,
> clamping to tap 15
> [    1.505077] sdhci-aspeed 1e750100.sdhci: rate 1562500 phase 45 tap 15
> [    1.515059] sdhci-aspeed 1e750100.sdhci: rate 100000000 phase 9 tap 3
> [    1.524886] sdhci-aspeed 1e750100.sdhci: rate 100000000 phase 45 tap 15
> [    1.534904] sdhci-aspeed 1e750100.sdhci: rate 100000000 phase 9 tap 3
> [    1.544713] sdhci-aspeed 1e750100.sdhci: rate 100000000 phase 45 tap 15
> 
> We should change the "out of range" message to be dev_dbg, as it is
> expected on a normal boot.

I would think the issue is rather that we shouldn't be applying a phase 
correction for a bus speed that isn't what the correction was specified 
for.

Let me look at this a bit further.

Andrew

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

Re: [PATCH v5 0/4] mmc: sdhci-of-aspeed: Support toggling SD bus signal

[Andrew Jeffery]

On Tue, 25 May 2021, at 22:26, Joel Stanley wrote:
> On Tue, 25 May 2021 at 09:48, Steven Lee <steven_lee@aspeedtech.com> wrote:
> >
> > The 05/25/2021 15:55, Joel Stanley wrote:
> > > When I was testing on my A2 EVB I saw this:
> > >
> > > [    1.436219] sdhci-aspeed 1e750100.sdhci: Requested out of range
> > > phase tap 192 for 9 degrees of phase compensation at 1562500Hz,
> > > clamping to tap 15
> > > [    1.450913] sdhci-aspeed 1e750100.sdhci: Requested out of range
> > > phase tap 963 for 45 degrees of phase compensation at 1562500Hz,
> > > clamping to tap 15
> > >
> > > Do you know what is happening there?
> > >
> >
> > Per MMC spec, eMMC bus speed is set as legacy mode(0~26MHz) at startup of
> > eMMC initializtion flow. Clock phase calculation is triggered in set_clock()
> > and it calculates taps based on phase_deg(<9>, <225>) in the dts file and the
> > current speed(1562500Hz), which causes the warning message you mentioned.
> > As the phase_deg in the dts file should be calculated with 100MHz.
> >
> > https://lkml.org/lkml/2021/5/24/95
> >
> > But after some initialization flow, eMMC bus speed will be set to
> > correct speed(100MHz).
> > Clock phase calculation will be triggered again to get correct taps.
> 
> Thanks for the explanation. I added another debug print and I can see
> it doing what you describe:
> 
> [    1.465904] sdhci-aspeed 1e750100.sdhci: Requested out of range
> phase tap 192 for 9 degrees of phase compensation at 1562500Hz,
> clamping to tap 15
> [    1.480598] sdhci-aspeed 1e750100.sdhci: rate 1562500 phase 9 tap 15
> [    1.490316] sdhci-aspeed 1e750100.sdhci: Requested out of range
> phase tap 963 for 45 degrees of phase compensation at 1562500Hz,
> clamping to tap 15
> [    1.505077] sdhci-aspeed 1e750100.sdhci: rate 1562500 phase 45 tap 15
> [    1.515059] sdhci-aspeed 1e750100.sdhci: rate 100000000 phase 9 tap 3
> [    1.524886] sdhci-aspeed 1e750100.sdhci: rate 100000000 phase 45 tap 15
> [    1.534904] sdhci-aspeed 1e750100.sdhci: rate 100000000 phase 9 tap 3
> [    1.544713] sdhci-aspeed 1e750100.sdhci: rate 100000000 phase 45 tap 15
> 
> We should change the "out of range" message to be dev_dbg, as it is
> expected on a normal boot.

I would think the issue is rather that we shouldn't be applying a phase 
correction for a bus speed that isn't what the correction was specified 
for.

Let me look at this a bit further.

Andrew

Re: [PATCH v5 0/4] mmc: sdhci-of-aspeed: Support toggling SD bus signal

[Andrew Jeffery]

On Wed, 26 May 2021, at 08:29, Andrew Jeffery wrote:
> 
> 
> On Tue, 25 May 2021, at 22:26, Joel Stanley wrote:
> > On Tue, 25 May 2021 at 09:48, Steven Lee <steven_lee@aspeedtech.com> wrote:
> > >
> > > The 05/25/2021 15:55, Joel Stanley wrote:
> > > > When I was testing on my A2 EVB I saw this:
> > > >
> > > > [    1.436219] sdhci-aspeed 1e750100.sdhci: Requested out of range
> > > > phase tap 192 for 9 degrees of phase compensation at 1562500Hz,
> > > > clamping to tap 15
> > > > [    1.450913] sdhci-aspeed 1e750100.sdhci: Requested out of range
> > > > phase tap 963 for 45 degrees of phase compensation at 1562500Hz,
> > > > clamping to tap 15
> > > >
> > > > Do you know what is happening there?
> > > >
> > >
> > > Per MMC spec, eMMC bus speed is set as legacy mode(0~26MHz) at startup of
> > > eMMC initializtion flow. Clock phase calculation is triggered in set_clock()
> > > and it calculates taps based on phase_deg(<9>, <225>) in the dts file and the
> > > current speed(1562500Hz), which causes the warning message you mentioned.
> > > As the phase_deg in the dts file should be calculated with 100MHz.
> > >
> > > https://lkml.org/lkml/2021/5/24/95
> > >
> > > But after some initialization flow, eMMC bus speed will be set to
> > > correct speed(100MHz).
> > > Clock phase calculation will be triggered again to get correct taps.
> > 
> > Thanks for the explanation. I added another debug print and I can see
> > it doing what you describe:
> > 
> > [    1.465904] sdhci-aspeed 1e750100.sdhci: Requested out of range
> > phase tap 192 for 9 degrees of phase compensation at 1562500Hz,
> > clamping to tap 15
> > [    1.480598] sdhci-aspeed 1e750100.sdhci: rate 1562500 phase 9 tap 15
> > [    1.490316] sdhci-aspeed 1e750100.sdhci: Requested out of range
> > phase tap 963 for 45 degrees of phase compensation at 1562500Hz,
> > clamping to tap 15
> > [    1.505077] sdhci-aspeed 1e750100.sdhci: rate 1562500 phase 45 tap 15
> > [    1.515059] sdhci-aspeed 1e750100.sdhci: rate 100000000 phase 9 tap 3
> > [    1.524886] sdhci-aspeed 1e750100.sdhci: rate 100000000 phase 45 tap 15
> > [    1.534904] sdhci-aspeed 1e750100.sdhci: rate 100000000 phase 9 tap 3
> > [    1.544713] sdhci-aspeed 1e750100.sdhci: rate 100000000 phase 45 tap 15
> > 
> > We should change the "out of range" message to be dev_dbg, as it is
> > expected on a normal boot.
> 
> I would think the issue is rather that we shouldn't be applying a phase 
> correction for a bus speed that isn't what the correction was specified 
> for.
> 
> Let me look at this a bit further.

Okay, looks like the issue is that setting the card timing and the bus frequency are not atomic in the sense that the bus clock enable is toggled in between the two, and the MMC core calls through the driver's set_clock() callback when toggling the bus clock state. This means the driver observes a transient state where the card timing is not aligned with the bus frequency, and so we have garbage inputs to the phase correction calculation.

Ideally there'd be a better solution than just switching to dev_dbg(), but I'm not sure what it would look like.

Andrew

Re: [PATCH v5 0/4] mmc: sdhci-of-aspeed: Support toggling SD bus signal

[Andrew Jeffery]

On Wed, 26 May 2021, at 08:29, Andrew Jeffery wrote:
> 
> 
> On Tue, 25 May 2021, at 22:26, Joel Stanley wrote:
> > On Tue, 25 May 2021 at 09:48, Steven Lee <steven_lee@aspeedtech.com> wrote:
> > >
> > > The 05/25/2021 15:55, Joel Stanley wrote:
> > > > When I was testing on my A2 EVB I saw this:
> > > >
> > > > [    1.436219] sdhci-aspeed 1e750100.sdhci: Requested out of range
> > > > phase tap 192 for 9 degrees of phase compensation at 1562500Hz,
> > > > clamping to tap 15
> > > > [    1.450913] sdhci-aspeed 1e750100.sdhci: Requested out of range
> > > > phase tap 963 for 45 degrees of phase compensation at 1562500Hz,
> > > > clamping to tap 15
> > > >
> > > > Do you know what is happening there?
> > > >
> > >
> > > Per MMC spec, eMMC bus speed is set as legacy mode(0~26MHz) at startup of
> > > eMMC initializtion flow. Clock phase calculation is triggered in set_clock()
> > > and it calculates taps based on phase_deg(<9>, <225>) in the dts file and the
> > > current speed(1562500Hz), which causes the warning message you mentioned.
> > > As the phase_deg in the dts file should be calculated with 100MHz.
> > >
> > > https://lkml.org/lkml/2021/5/24/95
> > >
> > > But after some initialization flow, eMMC bus speed will be set to
> > > correct speed(100MHz).
> > > Clock phase calculation will be triggered again to get correct taps.
> > 
> > Thanks for the explanation. I added another debug print and I can see
> > it doing what you describe:
> > 
> > [    1.465904] sdhci-aspeed 1e750100.sdhci: Requested out of range
> > phase tap 192 for 9 degrees of phase compensation at 1562500Hz,
> > clamping to tap 15
> > [    1.480598] sdhci-aspeed 1e750100.sdhci: rate 1562500 phase 9 tap 15
> > [    1.490316] sdhci-aspeed 1e750100.sdhci: Requested out of range
> > phase tap 963 for 45 degrees of phase compensation at 1562500Hz,
> > clamping to tap 15
> > [    1.505077] sdhci-aspeed 1e750100.sdhci: rate 1562500 phase 45 tap 15
> > [    1.515059] sdhci-aspeed 1e750100.sdhci: rate 100000000 phase 9 tap 3
> > [    1.524886] sdhci-aspeed 1e750100.sdhci: rate 100000000 phase 45 tap 15
> > [    1.534904] sdhci-aspeed 1e750100.sdhci: rate 100000000 phase 9 tap 3
> > [    1.544713] sdhci-aspeed 1e750100.sdhci: rate 100000000 phase 45 tap 15
> > 
> > We should change the "out of range" message to be dev_dbg, as it is
> > expected on a normal boot.
> 
> I would think the issue is rather that we shouldn't be applying a phase 
> correction for a bus speed that isn't what the correction was specified 
> for.
> 
> Let me look at this a bit further.

Okay, looks like the issue is that setting the card timing and the bus frequency are not atomic in the sense that the bus clock enable is toggled in between the two, and the MMC core calls through the driver's set_clock() callback when toggling the bus clock state. This means the driver observes a transient state where the card timing is not aligned with the bus frequency, and so we have garbage inputs to the phase correction calculation.

Ideally there'd be a better solution than just switching to dev_dbg(), but I'm not sure what it would look like.

Andrew

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

Re: [PATCH v5 0/4] mmc: sdhci-of-aspeed: Support toggling SD bus signal

[Andrew Jeffery]

On Wed, 26 May 2021, at 08:29, Andrew Jeffery wrote:
> 
> 
> On Tue, 25 May 2021, at 22:26, Joel Stanley wrote:
> > On Tue, 25 May 2021 at 09:48, Steven Lee <steven_lee@aspeedtech.com> wrote:
> > >
> > > The 05/25/2021 15:55, Joel Stanley wrote:
> > > > When I was testing on my A2 EVB I saw this:
> > > >
> > > > [    1.436219] sdhci-aspeed 1e750100.sdhci: Requested out of range
> > > > phase tap 192 for 9 degrees of phase compensation at 1562500Hz,
> > > > clamping to tap 15
> > > > [    1.450913] sdhci-aspeed 1e750100.sdhci: Requested out of range
> > > > phase tap 963 for 45 degrees of phase compensation at 1562500Hz,
> > > > clamping to tap 15
> > > >
> > > > Do you know what is happening there?
> > > >
> > >
> > > Per MMC spec, eMMC bus speed is set as legacy mode(0~26MHz) at startup of
> > > eMMC initializtion flow. Clock phase calculation is triggered in set_clock()
> > > and it calculates taps based on phase_deg(<9>, <225>) in the dts file and the
> > > current speed(1562500Hz), which causes the warning message you mentioned.
> > > As the phase_deg in the dts file should be calculated with 100MHz.
> > >
> > > https://lkml.org/lkml/2021/5/24/95
> > >
> > > But after some initialization flow, eMMC bus speed will be set to
> > > correct speed(100MHz).
> > > Clock phase calculation will be triggered again to get correct taps.
> > 
> > Thanks for the explanation. I added another debug print and I can see
> > it doing what you describe:
> > 
> > [    1.465904] sdhci-aspeed 1e750100.sdhci: Requested out of range
> > phase tap 192 for 9 degrees of phase compensation at 1562500Hz,
> > clamping to tap 15
> > [    1.480598] sdhci-aspeed 1e750100.sdhci: rate 1562500 phase 9 tap 15
> > [    1.490316] sdhci-aspeed 1e750100.sdhci: Requested out of range
> > phase tap 963 for 45 degrees of phase compensation at 1562500Hz,
> > clamping to tap 15
> > [    1.505077] sdhci-aspeed 1e750100.sdhci: rate 1562500 phase 45 tap 15
> > [    1.515059] sdhci-aspeed 1e750100.sdhci: rate 100000000 phase 9 tap 3
> > [    1.524886] sdhci-aspeed 1e750100.sdhci: rate 100000000 phase 45 tap 15
> > [    1.534904] sdhci-aspeed 1e750100.sdhci: rate 100000000 phase 9 tap 3
> > [    1.544713] sdhci-aspeed 1e750100.sdhci: rate 100000000 phase 45 tap 15
> > 
> > We should change the "out of range" message to be dev_dbg, as it is
> > expected on a normal boot.
> 
> I would think the issue is rather that we shouldn't be applying a phase 
> correction for a bus speed that isn't what the correction was specified 
> for.
> 
> Let me look at this a bit further.

Okay, looks like the issue is that setting the card timing and the bus frequency are not atomic in the sense that the bus clock enable is toggled in between the two, and the MMC core calls through the driver's set_clock() callback when toggling the bus clock state. This means the driver observes a transient state where the card timing is not aligned with the bus frequency, and so we have garbage inputs to the phase correction calculation.

Ideally there'd be a better solution than just switching to dev_dbg(), but I'm not sure what it would look like.

Andrew