* Re: [PATCH v2] mmc: sdhci-of-arasan: Add quirk for unstable clocks
2018-06-15 8:18 ` [PATCH v2] mmc: sdhci-of-arasan: Add quirk for unstable clocks Helmut Grohne
@ 2018-06-18 8:59 ` Ulf Hansson
2018-06-19 9:09 ` Adrian Hunter
2018-06-20 11:32 ` [PATCH v3 0/2] mmc: sdhci-of-arasan: workaround for broken clocks Helmut Grohne
2 siblings, 0 replies; 11+ messages in thread
From: Ulf Hansson @ 2018-06-18 8:59 UTC (permalink / raw)
To: Helmut Grohne
Cc: Atul Garg, linux-mmc@vger.kernel.org, Adrian Hunter, Michal Simek,
Sören Brinkmann, Linux ARM
On 15 June 2018 at 10:18, Helmut Grohne <h.grohne@intenta.de> wrote:
> 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.
>
> Signed-off-by: Helmut Grohne <h.grohne@intenta.de>
> ---
> 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
> 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.
>
> diff --git a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> index 60481bfc3d31..c0e0f04a8504 100644
> --- a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> +++ b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> @@ -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@e0100000 {
> diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c
> index c33a5f7393bd..f7fe26c75150 100644
> --- a/drivers/mmc/host/sdhci-of-arasan.c
> +++ b/drivers/mmc/host/sdhci-of-arasan.c
> @@ -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
>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v2] mmc: sdhci-of-arasan: Add quirk for unstable clocks
2018-06-15 8:18 ` [PATCH v2] mmc: sdhci-of-arasan: Add quirk for unstable clocks Helmut Grohne
2018-06-18 8:59 ` Ulf Hansson
@ 2018-06-19 9:09 ` Adrian Hunter
2018-06-20 11:32 ` [PATCH v3 0/2] mmc: sdhci-of-arasan: workaround for broken clocks Helmut Grohne
2 siblings, 0 replies; 11+ messages in thread
From: Adrian Hunter @ 2018-06-19 9:09 UTC (permalink / raw)
To: Helmut Grohne, Michal Simek, Sören Brinkmann, Ulf Hansson,
linux-mmc
Cc: Atul Garg, linux-arm-kernel
On 15/06/18 11:18, Helmut Grohne wrote:
> 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.
>
> Signed-off-by: Helmut Grohne <h.grohne@intenta.de>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
> 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.
>
> diff --git a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> index 60481bfc3d31..c0e0f04a8504 100644
> --- a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> +++ b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> @@ -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@e0100000 {
> diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c
> index c33a5f7393bd..f7fe26c75150 100644
> --- a/drivers/mmc/host/sdhci-of-arasan.c
> +++ b/drivers/mmc/host/sdhci-of-arasan.c
> @@ -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,
>
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH v3 0/2] mmc: sdhci-of-arasan: workaround for broken clocks
2018-06-15 8:18 ` [PATCH v2] mmc: sdhci-of-arasan: Add quirk for unstable clocks Helmut Grohne
2018-06-18 8:59 ` Ulf Hansson
2018-06-19 9:09 ` Adrian Hunter
@ 2018-06-20 11:32 ` Helmut Grohne
2018-06-20 11:32 ` [PATCH v3 1/2] dt-bindings: mmc: broken clock stable indicator on arasan controllers Helmut Grohne
` (2 more replies)
2 siblings, 3 replies; 11+ messages in thread
From: Helmut Grohne @ 2018-06-20 11:32 UTC (permalink / raw)
To: Adrian Hunter, Michal Simek, Sören Brinkmann, Ulf Hansson,
Rob Herring, Mark Rutland, linux-mmc, devicetree
Cc: Atul Garg, linux-arm-kernel
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 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(+)
--
2.11.0
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH v3 1/2] dt-bindings: mmc: broken clock stable indicator on arasan controllers
2018-06-20 11:32 ` [PATCH v3 0/2] mmc: sdhci-of-arasan: workaround for broken clocks Helmut Grohne
@ 2018-06-20 11:32 ` Helmut Grohne
2018-06-20 19:08 ` Rob Herring
2018-06-20 11:33 ` [PATCH v3 2/2] mmc: sdhci-of-arasan: Add quirk for unstable clocks Helmut Grohne
2018-07-02 13:17 ` [PATCH v3 0/2] mmc: sdhci-of-arasan: workaround for broken clocks Ulf Hansson
2 siblings, 1 reply; 11+ messages in thread
From: Helmut Grohne @ 2018-06-20 11:32 UTC (permalink / raw)
To: Adrian Hunter, Michal Simek, Sören Brinkmann, Ulf Hansson,
Rob Herring, Mark Rutland, linux-mmc, devicetree
Cc: Atul Garg, linux-arm-kernel
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.
Signed-off-by: Helmut Grohne <h.grohne@intenta.de>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
---
Documentation/devicetree/bindings/mmc/arasan,sdhci.txt | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
index 60481bfc3d31..28332b655d2e 100644
--- a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
+++ b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
@@ -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@e0100000 {
--
2.11.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v3 1/2] dt-bindings: mmc: broken clock stable indicator on arasan controllers
2018-06-20 11:32 ` [PATCH v3 1/2] dt-bindings: mmc: broken clock stable indicator on arasan controllers Helmut Grohne
@ 2018-06-20 19:08 ` Rob Herring
0 siblings, 0 replies; 11+ messages in thread
From: Rob Herring @ 2018-06-20 19:08 UTC (permalink / raw)
To: Helmut Grohne
Cc: Mark Rutland, devicetree, Ulf Hansson, Atul Garg, linux-mmc,
Adrian Hunter, Michal Simek, linux-arm-kernel,
Sören Brinkmann
On Wed, Jun 20, 2018 at 01:32:51PM +0200, Helmut Grohne wrote:
> 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.
>
> Signed-off-by: Helmut Grohne <h.grohne@intenta.de>
> Acked-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
> Documentation/devicetree/bindings/mmc/arasan,sdhci.txt | 2 ++
> 1 file changed, 2 insertions(+)
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3 2/2] mmc: sdhci-of-arasan: Add quirk for unstable clocks
2018-06-20 11:32 ` [PATCH v3 0/2] mmc: sdhci-of-arasan: workaround for broken clocks Helmut Grohne
2018-06-20 11:32 ` [PATCH v3 1/2] dt-bindings: mmc: broken clock stable indicator on arasan controllers Helmut Grohne
@ 2018-06-20 11:33 ` Helmut Grohne
2018-07-02 13:17 ` [PATCH v3 0/2] mmc: sdhci-of-arasan: workaround for broken clocks Ulf Hansson
2 siblings, 0 replies; 11+ messages in thread
From: Helmut Grohne @ 2018-06-20 11:33 UTC (permalink / raw)
To: Adrian Hunter, Michal Simek, Sören Brinkmann, Ulf Hansson,
Rob Herring, Mark Rutland, linux-mmc, devicetree
Cc: Atul Garg, linux-arm-kernel
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 known to be broken, we simply wait for the
maximum stabilization period.
Signed-off-by: Helmut Grohne <h.grohne@intenta.de>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
---
drivers/mmc/host/sdhci-of-arasan.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c
index e3332a522a5d..a40bcc27f187 100644
--- a/drivers/mmc/host/sdhci-of-arasan.c
+++ b/drivers/mmc/host/sdhci-of-arasan.c
@@ -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,
--
2.11.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v3 0/2] mmc: sdhci-of-arasan: workaround for broken clocks
2018-06-20 11:32 ` [PATCH v3 0/2] mmc: sdhci-of-arasan: workaround for broken clocks Helmut Grohne
2018-06-20 11:32 ` [PATCH v3 1/2] dt-bindings: mmc: broken clock stable indicator on arasan controllers Helmut Grohne
2018-06-20 11:33 ` [PATCH v3 2/2] mmc: sdhci-of-arasan: Add quirk for unstable clocks Helmut Grohne
@ 2018-07-02 13:17 ` Ulf Hansson
2 siblings, 0 replies; 11+ messages in thread
From: Ulf Hansson @ 2018-07-02 13:17 UTC (permalink / raw)
To: Helmut Grohne
Cc: Mark Rutland, devicetree, Atul Garg, linux-mmc@vger.kernel.org,
Adrian Hunter, Michal Simek, Rob Herring, Linux ARM,
Sören Brinkmann
On 20 June 2018 at 13:32, Helmut Grohne <h.grohne@intenta.de> wrote:
> 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 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(+)
>
> --
> 2.11.0
>
Thanks, applied for next!
Kind regards
Uffe
^ permalink raw reply [flat|nested] 11+ messages in thread