sdhci-of-arasan: card initialization failure -84
Subsystems (4)
RFC PATCH 1/1 sdhci-of-arasan: card initialization failure -84
Work around repeated:
mmc0: error -84 whilst initialising SD card
--- drivers/mmc/host/sdhci.c | 1 + 1 file changed, 1 insertion(+)
I'm seeing the above error -84 with a series of Kingston cards of 4GB capacity when used with an arasan host controller. Using the cards with other hosts or using other cards with the host is not problematic. The host controller has the following device tree:
ps7-sdio at e0100000 {
compatible = "arasan,sdhci-8.9a";
reg = <0xe0100000 0x1000>;
clock-names = "clk_xin", "clk_ahb";
clock-frequency = <50000000>;
clocks = <&clkc 21>, <&clkc 32>;
interrupt-parent = <&ps7_scugic_0>;
interrupts = <0 24 4>;
};When enabling sufficient debugging (and thus slowing the kernel down), the problem goes away. Without debug features, the insertion of this delay suffices for my particular instance. Inserting the same delay before the SDHCI_CLOCK_CARD_EN write also makes the error go away.
While instrumenting the code I found that the loop waiting for the SDHCI_CLOCK_INT_STABLE would always terminate immediately (regardless of the card being used). Only when the card is removed, the loop would spin once. This hints that potentially the stable checking is broken for this host controller.
During my testing I checked 7 Kingston cards, 2 Transcend Cards, 1 Apacer card, 3 arasn hosts, 3 non-arasan hosts and had consistent reports for a similar number of devices from others.
If my hypothesis is reasonable, I can turn this patch into a new quirk that replaces the stable polling with msleep(20), which is the poll timeout. Is there anything else I should investigate before doing so?
Helmut
drivers/mmc/host/sdhci.c +1 -0
@@ -1430,6 +1430,7 @@ void sdhci_enable_clk(struct sdhci_host *host, u16 clk) clk |=SDHCI_CLOCK_CARD_EN ;sdhci_writew (host, clk,SDHCI_CLOCK_CONTROL ); +udelay (80 ); }EXPORT_SYMBOL_GPL (sdhci_enable_clk);
Signature · 1 line
2.11.0
Adrian Hunter
maintainerRFC PATCH 1/1 sdhci-of-arasan: card initialization failure -84
Quoted 32 lines · Helmut Grohne
Work around repeated:
mmc0: error -84 whilst initialising SD card
--- drivers/mmc/host/sdhci.c | 1 + 1 file changed, 1 insertion(+)I'm seeing the above error -84 with a series of Kingston cards of 4GB capacity when used with an arasan host controller. Using the cards with other hosts or using other cards with the host is not problematic. The host controller has the following device tree:
ps7-sdio at e0100000 { compatible = "arasan,sdhci-8.9a"; reg = <0xe0100000 0x1000>; clock-names = "clk_xin", "clk_ahb"; clock-frequency = <50000000>; clocks = <&clkc 21>, <&clkc 32>; interrupt-parent = <&ps7_scugic_0>; interrupts = <0 24 4>; };When enabling sufficient debugging (and thus slowing the kernel down), the problem goes away. Without debug features, the insertion of this delay suffices for my particular instance. Inserting the same delay before the
SDHCI_CLOCK_CARD_ENwrite also makes the error go away.While instrumenting the code I found that the loop waiting for the
SDHCI_CLOCK_INT_STABLEwould always terminate immediately (regardless of the card being used). Only when the card is removed, the loop would spin once. This hints that potentially the stable checking is broken for this host controller.
Quoted 7 lines
During my testing I checked 7 Kingston cards, 2 Transcend Cards, 1 Apacer card, 3 arasn hosts, 3 non-arasan hosts and had consistent reports for a similar number of devices from others.
If my hypothesis is reasonable, I can turn this patch into a new quirk that replaces the stable polling with msleep(20), which is the poll timeout. Is there anything else I should investigate before doing so?
Please avoid quirks. Perhaps use the ->set_clock() callback.
Quoted 16 lines
Helmut
drivers/mmc/host/sdhci.c +1
-0@@ -1430,6 +1430,7 @@ void sdhci_enable_clk(struct sdhci_host *host, u16 clk) clk |=SDHCI_CLOCK_CARD_EN ;sdhci_writew (host, clk,SDHCI_CLOCK_CONTROL ); +udelay (80 ); }EXPORT_SYMBOL_GPL (sdhci_enable_clk);
Adrian Hunter
maintainerRFC PATCH 1/1 sdhci-of-arasan: card initialization failure -84
In reply to Helmut GrohneAnd again without mangling Atul's email address.
Quoted 32 lines · Helmut Grohne
Work around repeated:
mmc0: error -84 whilst initialising SD card
--- drivers/mmc/host/sdhci.c | 1 + 1 file changed, 1 insertion(+)I'm seeing the above error -84 with a series of Kingston cards of 4GB capacity when used with an arasan host controller. Using the cards with other hosts or using other cards with the host is not problematic. The host controller has the following device tree:
ps7-sdio at e0100000 { compatible = "arasan,sdhci-8.9a"; reg = <0xe0100000 0x1000>; clock-names = "clk_xin", "clk_ahb"; clock-frequency = <50000000>; clocks = <&clkc 21>, <&clkc 32>; interrupt-parent = <&ps7_scugic_0>; interrupts = <0 24 4>; };When enabling sufficient debugging (and thus slowing the kernel down), the problem goes away. Without debug features, the insertion of this delay suffices for my particular instance. Inserting the same delay before the
SDHCI_CLOCK_CARD_ENwrite also makes the error go away.While instrumenting the code I found that the loop waiting for the
SDHCI_CLOCK_INT_STABLEwould always terminate immediately (regardless of the card being used). Only when the card is removed, the loop would spin once. This hints that potentially the stable checking is broken for this host controller.
Quoted 7 lines
During my testing I checked 7 Kingston cards, 2 Transcend Cards, 1 Apacer card, 3 arasn hosts, 3 non-arasan hosts and had consistent reports for a similar number of devices from others.
If my hypothesis is reasonable, I can turn this patch into a new quirk that replaces the stable polling with msleep(20), which is the poll timeout. Is there anything else I should investigate before doing so?
Please avoid quirks. Perhaps use the ->set_clock() callback.
Quoted 16 lines
Helmut
drivers/mmc/host/sdhci.c +1
-0@@ -1430,6 +1430,7 @@ void sdhci_enable_clk(struct sdhci_host *host, u16 clk) clk |=SDHCI_CLOCK_CARD_EN ;sdhci_writew (host, clk,SDHCI_CLOCK_CONTROL ); +udelay (80 ); }EXPORT_SYMBOL_GPL (sdhci_enable_clk);
PATCH v2 1/1 mmc: sdhci-of-arasan: Add quirk for unstable clocks
Some controllers immediately report SDHCI_CLOCK_INT_STABLE after enabling the clock even when the clock is not stable. When used in conjunction with older/slower cards, this can result in:
mmc0: error -84 whilst initialising SD card
When the stable reporting is broken, we simply wait for the maximum stabilization period.
--- Documentation/devicetree/bindings/mmc/arasan,sdhci.txt | 2 ++ drivers/mmc/host/sdhci-of-arasan.c | 16 ++++++++++++++++ 2 files changed, 18 insertions(+)
Changes since v1 (RFC):
* Use an arasan-specific quirk in the ->set_clock() callback as requested by Adrian Hunter.
Documentation/devicetree/bindings/mmc/arasan,sdhci.txt +2 -0
@@ -39,6 +39,8 @@ Optional Properties: - xlnx,fails-without-test-cd: when present, the controller doesn't work when the CD line is not connected properly, and the line is not connected properly. Test mode can be used to force the controller to function. + - xlnx,int-clock-stable-broken: when present, the controller always reports + that the internal clock is stable even when it is not. Example: sdhci at e0100000 {
drivers/mmc/host/sdhci-of-arasan.c +16 -0
@@ -102,6 +102,9 @@ struct sdhci_arasan_data {/* Controller does not have CD wired and will not function normally without */ #define SDHCI_ARASAN_QUIRK_FORCE_CDTEST BIT(0) +/* Controller immediately reports SDHCI_CLOCK_INT_STABLE after enabling the +* internal clock even when the clock isn't stable */ +#define SDHCI_ARASAN_QUIRK_CLOCK_UNSTABLE BIT(1) };static const struct sdhci_arasan_soc_ctl_map rk3399_soc_ctl_map = {
@@ -207,6 +210,16 @@ static void sdhci_arasan_set_clock(struct sdhci_host *host, unsigned int clock)sdhci_set_clock (host, clock); +if (sdhci_arasan->quirks &SDHCI_ARASAN_QUIRK_CLOCK_UNSTABLE ) +/* +* Some controllers immediately report SDHCI_CLOCK_INT_STABLE +* after enabling the clock even though the clock is not +* stable. Trying to use a clock without waiting here results +* in EILSEQ while detecting some older/slower cards. The +* chosen delay is the maximum delay from sdhci_set_clock. +*/ +msleep (20 ); +if (ctrl_phy) {phy_power_on (sdhci_arasan->phy); sdhci_arasan->is_phy_on = true;
@@ -759,6 +772,9 @@ static int sdhci_arasan_probe(struct platform_device *pdev)if (of_property_read_bool (np,"xlnx,fails-without-test-cd" )) sdhci_arasan->quirks |=SDHCI_ARASAN_QUIRK_FORCE_CDTEST ; +if (of_property_read_bool (np,"xlnx,int-clock-stable-broken" )) + sdhci_arasan->quirks |=SDHCI_ARASAN_QUIRK_CLOCK_UNSTABLE ; + pltfm_host->clk = clk_xin; if (of_device_is_compatible(pdev->dev.of_node,
Signature · 1 line
2.11.0
PATCH v2 1/1 mmc: sdhci-of-arasan: Add quirk for unstable clocks
Quoted 4 lines · Helmut Grohne
Some controllers immediately report
SDHCI_CLOCK_INT_STABLEafter enabling the clock even when the clock is not stable. When used in conjunction with older/slower cards, this can result in:
Quoted 8 lines
mmc0: error -84 whilst initialising SD card
When the stable reporting is broken, we simply wait for the maximum stabilization period.
Signed-off-by Helmut Grohne <[email protected]>--- Documentation/devicetree/bindings/mmc/arasan,sdhci.txt | 2 ++
Please split this. DT doc should be a separate patch.
Otherwise this looks good to me.
Kind regards
Uffe
Quoted 64 lines
drivers/mmc/host/sdhci-of-arasan.c | 16 ++++++++++++++++ 2 files changed, 18 insertions(+)Changes since v1 (RFC):
* Use an arasan-specific quirk in the ->set_clock() callback as requested by Adrian Hunter.Documentation/devicetree/bindings/mmc/arasan,sdhci.txt +2
-0@@ -39,6 +39,8 @@ Optional Properties: - xlnx,fails-without-test-cd: when present, the controller doesn't work when the CD line is not connected properly, and the line is not connected properly. Test mode can be used to force the controller to function. + - xlnx,int-clock-stable-broken: when present, the controller always reports + that the internal clock is stable even when it is not. Example: sdhci at e0100000 {drivers/mmc/host/sdhci-of-arasan.c +16
-1@@ -102,6 +102,9 @@ struct sdhci_arasan_data {/* Controller does not have CD wired and will not function normally without */ #define SDHCI_ARASAN_QUIRK_FORCE_CDTEST BIT(0) +/* Controller immediately reports SDHCI_CLOCK_INT_STABLE after enabling the +* internal clock even when the clock isn't stable */ +#define SDHCI_ARASAN_QUIRK_CLOCK_UNSTABLE BIT(1) };static const struct sdhci_arasan_soc_ctl_map rk3399_soc_ctl_map = {@@ -207,6 +210,16 @@ static void sdhci_arasan_set_clock(struct sdhci_host *host, unsigned int clock)sdhci_set_clock (host, clock); +if (sdhci_arasan->quirks &SDHCI_ARASAN_QUIRK_CLOCK_UNSTABLE ) +/* +* Some controllers immediately report SDHCI_CLOCK_INT_STABLE +* after enabling the clock even though the clock is not +* stable. Trying to use a clock without waiting here results +* in EILSEQ while detecting some older/slower cards. The +* chosen delay is the maximum delay from sdhci_set_clock. +*/ +msleep (20 ); +if (ctrl_phy) {phy_power_on (sdhci_arasan->phy); sdhci_arasan->is_phy_on = true;@@ -759,6 +772,9 @@ static int sdhci_arasan_probe(struct platform_device *pdev)if (of_property_read_bool (np,"xlnx,fails-without-test-cd" )) sdhci_arasan->quirks |=SDHCI_ARASAN_QUIRK_FORCE_CDTEST ; +if (of_property_read_bool (np,"xlnx,int-clock-stable-broken" )) + sdhci_arasan->quirks |=SDHCI_ARASAN_QUIRK_CLOCK_UNSTABLE ; + pltfm_host->clk = clk_xin; if (of_device_is_compatible(pdev->dev.of_node, --2.11.0
Adrian Hunter
maintainerPATCH v2 1/1 mmc: sdhci-of-arasan: Add quirk for unstable clocks
In reply to Helmut GrohneQuoted 10 lines · Helmut Grohne
Some controllers immediately report
SDHCI_CLOCK_INT_STABLEafter enabling the clock even when the clock is not stable. When used in conjunction with older/slower cards, this can result in:mmc0: error -84 whilst initialising SD card
When the stable reporting is broken, we simply wait for the maximum stabilization period.
Signed-off-by Helmut Grohne <[email protected]>
Quoted 64 lines
--- Documentation/devicetree/bindings/mmc/arasan,sdhci.txt | 2 ++ drivers/mmc/host/sdhci-of-arasan.c | 16 ++++++++++++++++ 2 files changed, 18 insertions(+)Changes since v1 (RFC):
* Use an arasan-specific quirk in the ->set_clock() callback as requested by Adrian Hunter.Documentation/devicetree/bindings/mmc/arasan,sdhci.txt +2
-0@@ -39,6 +39,8 @@ Optional Properties: - xlnx,fails-without-test-cd: when present, the controller doesn't work when the CD line is not connected properly, and the line is not connected properly. Test mode can be used to force the controller to function. + - xlnx,int-clock-stable-broken: when present, the controller always reports + that the internal clock is stable even when it is not. Example: sdhci at e0100000 {drivers/mmc/host/sdhci-of-arasan.c +16
-0@@ -102,6 +102,9 @@ struct sdhci_arasan_data {/* Controller does not have CD wired and will not function normally without */ #define SDHCI_ARASAN_QUIRK_FORCE_CDTEST BIT(0) +/* Controller immediately reports SDHCI_CLOCK_INT_STABLE after enabling the +* internal clock even when the clock isn't stable */ +#define SDHCI_ARASAN_QUIRK_CLOCK_UNSTABLE BIT(1) };static const struct sdhci_arasan_soc_ctl_map rk3399_soc_ctl_map = {@@ -207,6 +210,16 @@ static void sdhci_arasan_set_clock(struct sdhci_host *host, unsigned int clock)sdhci_set_clock (host, clock); +if (sdhci_arasan->quirks &SDHCI_ARASAN_QUIRK_CLOCK_UNSTABLE ) +/* +* Some controllers immediately report SDHCI_CLOCK_INT_STABLE +* after enabling the clock even though the clock is not +* stable. Trying to use a clock without waiting here results +* in EILSEQ while detecting some older/slower cards. The +* chosen delay is the maximum delay from sdhci_set_clock. +*/ +msleep (20 ); +if (ctrl_phy) {phy_power_on (sdhci_arasan->phy); sdhci_arasan->is_phy_on = true;@@ -759,6 +772,9 @@ static int sdhci_arasan_probe(struct platform_device *pdev)if (of_property_read_bool (np,"xlnx,fails-without-test-cd" )) sdhci_arasan->quirks |=SDHCI_ARASAN_QUIRK_FORCE_CDTEST ; +if (of_property_read_bool (np,"xlnx,int-clock-stable-broken" )) + sdhci_arasan->quirks |=SDHCI_ARASAN_QUIRK_CLOCK_UNSTABLE ; + pltfm_host->clk = clk_xin; if (of_device_is_compatible(pdev->dev.of_node,
v3 cover mmc: sdhci-of-arasan: workaround for broken clocks
In reply to Helmut GrohneThis patch series works around the internal clock indicating stability too quickly to operate. It manifests itself as:
mmc0: error -84 whilst initialising SD card
Changes since v2:
* Ulf Hansson requested splitting the patch in accordance with Documentation/devicetree/bindings/submitting-patches.txt. Also update recipient list accordingly.
* Added Acked-by from Adrian Hunter.
Helmut Grohne (2):
dt-bindings: mmc: broken clock stable indicator on arasan controllers
mmc: sdhci-of-arasan: Add quirk for unstable clocks
Documentation/devicetree/bindings/mmc/arasan,sdhci.txt | 2 ++ drivers/mmc/host/sdhci-of-arasan.c | 16 ++++++++++++++++ 2 files changed, 18 insertions(+)
Signature · 1 line
2.11.0
PATCH v3 1/2 dt-bindings: mmc: broken clock stable indicator on arasan controllers
Some controllers immediately report that their internal clock is stable after activating it even when the clock is not stable. When used in conjunction with older/slower cards, this can result in:
mmc0: error -84 whilst initialising SD card
This flag allows documenting and thus working around such a hardware defect.
--- Documentation/devicetree/bindings/mmc/arasan,sdhci.txt | 2 ++ 1 file changed, 2 insertions(+)
Documentation/devicetree/bindings/mmc/arasan,sdhci.txt +2 -0
@@ -39,6 +39,8 @@ Optional Properties: - xlnx,fails-without-test-cd: when present, the controller doesn't work when the CD line is not connected properly, and the line is not connected properly. Test mode can be used to force the controller to function. + - xlnx,int-clock-stable-broken: when present, the controller always reports + that the internal clock is stable even when it is not. Example: sdhci at e0100000 {
Signature · 1 line
2.11.0
Rob Herring
maintainerPATCH v3 1/2 dt-bindings: mmc: broken clock stable indicator on arasan controllers
Quoted 6 lines · Helmut Grohne
Some controllers immediately report that their internal clock is stable after activating it even when the clock is not stable. When used in conjunction with older/slower cards, this can result in:
mmc0: error -84 whilst initialising SD card
Quoted 8 lines
This flag allows documenting and thus working around such a hardware defect.
Signed-off-by Helmut Grohne <[email protected]>Acked-by Adrian Hunter <[email protected]>--- Documentation/devicetree/bindings/mmc/arasan,sdhci.txt | 2 ++ 1 file changed, 2 insertions(+)
PATCH v3 2/2 mmc: sdhci-of-arasan: Add quirk for unstable clocks
In reply to Helmut GrohneSome controllers immediately report SDHCI_CLOCK_INT_STABLE after enabling the clock even when the clock is not stable. When used in conjunction with older/slower cards, this can result in:
mmc0: error -84 whilst initialising SD card
When the stable reporting is known to be broken, we simply wait for the maximum stabilization period.
--- drivers/mmc/host/sdhci-of-arasan.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
drivers/mmc/host/sdhci-of-arasan.c +16 -0
@@ -102,6 +102,9 @@ struct sdhci_arasan_data {/* Controller does not have CD wired and will not function normally without */ #define SDHCI_ARASAN_QUIRK_FORCE_CDTEST BIT(0) +/* Controller immediately reports SDHCI_CLOCK_INT_STABLE after enabling the +* internal clock even when the clock isn't stable */ +#define SDHCI_ARASAN_QUIRK_CLOCK_UNSTABLE BIT(1) };static const struct sdhci_arasan_soc_ctl_map rk3399_soc_ctl_map = {
@@ -207,6 +210,16 @@ static void sdhci_arasan_set_clock(struct sdhci_host *host, unsigned int clock)sdhci_set_clock (host, clock); +if (sdhci_arasan->quirks &SDHCI_ARASAN_QUIRK_CLOCK_UNSTABLE ) +/* +* Some controllers immediately report SDHCI_CLOCK_INT_STABLE +* after enabling the clock even though the clock is not +* stable. Trying to use a clock without waiting here results +* in EILSEQ while detecting some older/slower cards. The +* chosen delay is the maximum delay from sdhci_set_clock. +*/ +msleep (20 ); +if (ctrl_phy) {phy_power_on (sdhci_arasan->phy); sdhci_arasan->is_phy_on = true;
@@ -758,6 +771,9 @@ static int sdhci_arasan_probe(struct platform_device *pdev)if (of_property_read_bool (np,"xlnx,fails-without-test-cd" )) sdhci_arasan->quirks |=SDHCI_ARASAN_QUIRK_FORCE_CDTEST ; +if (of_property_read_bool (np,"xlnx,int-clock-stable-broken" )) + sdhci_arasan->quirks |=SDHCI_ARASAN_QUIRK_CLOCK_UNSTABLE ; + pltfm_host->clk = clk_xin; if (of_device_is_compatible(pdev->dev.of_node,
Signature · 1 line
2.11.0
Ulf Hansson
Appliedv3 cover mmc: sdhci-of-arasan: workaround for broken clocks
In reply to Helmut GrohneQuoted 15 lines · Helmut Grohne
This patch series works around the internal clock indicating stability too quickly to operate. It manifests itself as:
mmc0: error -84 whilst initialising SD card
Changes since v2:
* Ulf Hansson requested splitting the patch in accordance withDocumentation/devicetree/bindings/submitting-patches.txt. Also update recipient list accordingly.
* Added Acked-by from Adrian Hunter.Helmut Grohne (2):
dt-bindings: mmc: broken clock stable indicator on arasan controllers
mmc: sdhci-of-arasan: Add quirk for unstable clocks
Quoted 7 lines
Documentation/devicetree/bindings/mmc/arasan,sdhci.txt | 2 ++ drivers/mmc/host/sdhci-of-arasan.c | 16 ++++++++++++++++ 2 files changed, 18 insertions(+)--
2.11.0
Thanks, applied for next!
Kind regards
Uffe