From: Elad Nachman <[email protected]>
Fix PHY init timeout issues:
1. Clock Stability issue causing PHY timeout
2. Timeout taking longer than needed on AC5X.
Solve by constantly testing the PHY init bit
until it toggles, but up to 100X timeout factor.
v2:
1) convert polling loop to read_poll_timeout()
for both patches.
Elad Nachman (2):
mmc: xenon: fix PHY init clock stability
mmc: xenon: add timeout for PHY init complete
drivers/mmc/host/sdhci-xenon-phy.c | 48 ++++++++++++++++++++++++------
1 file changed, 39 insertions(+), 9 deletions(-)
--
2.25.1
From: Elad Nachman <[email protected]>
Each time SD/mmc phy is initialized, at times, in some of
the attempts, phy fails to completes its initialization
which results into timeout error. Per the HW spec, it is
a pre-requisite to ensure a stable SD clock before a phy
initialization is attempted.
Signed-off-by: Elad Nachman <[email protected]>
---
drivers/mmc/host/sdhci-xenon-phy.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/drivers/mmc/host/sdhci-xenon-phy.c b/drivers/mmc/host/sdhci-xenon-phy.c
index 8cf3a375de65..c3096230a969 100644
--- a/drivers/mmc/host/sdhci-xenon-phy.c
+++ b/drivers/mmc/host/sdhci-xenon-phy.c
@@ -11,6 +11,7 @@
#include <linux/slab.h>
#include <linux/delay.h>
#include <linux/ktime.h>
+#include <linux/iopoll.h>
#include <linux/of_address.h>
#include "sdhci-pltfm.h"
@@ -216,6 +217,19 @@ static int xenon_alloc_emmc_phy(struct sdhci_host *host)
return 0;
}
+static int xenon_check_stability_internal_clk(struct sdhci_host *host)
+{
+ u32 reg;
+ int err;
+
+ err = read_poll_timeout(sdhci_readw, reg, reg & SDHCI_CLOCK_INT_STABLE,
+ 1100, 20000, false, host, SDHCI_CLOCK_CONTROL);
+ if (err)
+ dev_err(mmc_dev(host->mmc), "phy_init: Internal clock never stabilized.\n");
+
+ return err;
+}
+
/*
* eMMC 5.0/5.1 PHY init/re-init.
* eMMC PHY init should be executed after:
@@ -232,6 +246,11 @@ static int xenon_emmc_phy_init(struct sdhci_host *host)
struct xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
struct xenon_emmc_phy_regs *phy_regs = priv->emmc_phy_regs;
+ int ret = xenon_check_stability_internal_clk(host);
+
+ if (ret)
+ return ret;
+
reg = sdhci_readl(host, phy_regs->timing_adj);
reg |= XENON_PHY_INITIALIZAION;
sdhci_writel(host, reg, phy_regs->timing_adj);
--
2.25.1
On Thu, 15 Feb 2024 at 17:16, Elad Nachman <[email protected]> wrote:
>
> From: Elad Nachman <[email protected]>
>
> Fix PHY init timeout issues:
>
> 1. Clock Stability issue causing PHY timeout
>
> 2. Timeout taking longer than needed on AC5X.
> Solve by constantly testing the PHY init bit
> until it toggles, but up to 100X timeout factor.
>
> v2:
> 1) convert polling loop to read_poll_timeout()
> for both patches.
>
> Elad Nachman (2):
> mmc: xenon: fix PHY init clock stability
> mmc: xenon: add timeout for PHY init complete
>
> drivers/mmc/host/sdhci-xenon-phy.c | 48 ++++++++++++++++++++++++------
> 1 file changed, 39 insertions(+), 9 deletions(-)
>
The series looks good to me. Although, I assume we should tag this for
stable kernels too and possibly add a fixes tag?
Moreover, it would be nice to get an ack from Hu Ziji.
Kind regards
Uffe
> -----Original Message-----
> From: Ulf Hansson <[email protected]>
> Sent: Thursday, February 15, 2024 6:51 PM
> To: Elad Nachman <[email protected]>
> Cc: [email protected]; [email protected]; linux-
> [email protected]; [email protected]
> Subject: [EXT] Re: [PATCH v2 0/2] Fix PHY init timeout issues
>
> External Email
>
> ----------------------------------------------------------------------
> On Thu, 15 Feb 2024 at 17:16, Elad Nachman <[email protected]>
> wrote:
> >
> > From: Elad Nachman <[email protected]>
> >
> > Fix PHY init timeout issues:
> >
> > 1. Clock Stability issue causing PHY timeout
> >
> > 2. Timeout taking longer than needed on AC5X.
> > Solve by constantly testing the PHY init bit
> > until it toggles, but up to 100X timeout factor.
> >
> > v2:
> > 1) convert polling loop to read_poll_timeout()
> > for both patches.
> >
> > Elad Nachman (2):
> > mmc: xenon: fix PHY init clock stability
> > mmc: xenon: add timeout for PHY init complete
> >
> > drivers/mmc/host/sdhci-xenon-phy.c | 48
> > ++++++++++++++++++++++++------
> > 1 file changed, 39 insertions(+), 9 deletions(-)
> >
>
> The series looks good to me. Although, I assume we should tag this for stable
> kernels too and possibly add a fixes tag?
No problem.
>
> Moreover, it would be nice to get an ack from Hu Ziji.
He does not longer work with Marvell, so this email is invalid, I tried also to send an e-mail manually as well and got:
[email protected]
Remote Server returned '550 5.1.1 RESOLVER.ADR.RecipNotFound; not found'
Could not locate him via google search either.
>
> Kind regards
> Uffe
From: Elad Nachman <[email protected]>
AC5X spec says PHY init complete bit must be polled until zero.
We see cases in which timeout can take longer than the standard
calculation on AC5X, which is expected following the spec comment above.
According to the spec, we must wait as long as it takes for that bit to
toggle on AC5X.
Cap that with 100 delay loops so we won't get stuck forever.
Signed-off-by: Elad Nachman <[email protected]>
---
drivers/mmc/host/sdhci-xenon-phy.c | 29 ++++++++++++++++++++---------
1 file changed, 20 insertions(+), 9 deletions(-)
diff --git a/drivers/mmc/host/sdhci-xenon-phy.c b/drivers/mmc/host/sdhci-xenon-phy.c
index c3096230a969..cc9d28b75eb9 100644
--- a/drivers/mmc/host/sdhci-xenon-phy.c
+++ b/drivers/mmc/host/sdhci-xenon-phy.c
@@ -110,6 +110,8 @@
#define XENON_EMMC_PHY_LOGIC_TIMING_ADJUST (XENON_EMMC_PHY_REG_BASE + 0x18)
#define XENON_LOGIC_TIMING_VALUE 0x00AA8977
+#define XENON_MAX_PHY_TIMEOUT_LOOPS 100
+
/*
* List offset of PHY registers and some special register values
* in eMMC PHY 5.0 or eMMC PHY 5.1
@@ -278,18 +280,27 @@ static int xenon_emmc_phy_init(struct sdhci_host *host)
/* get the wait time */
wait /= clock;
wait++;
- /* wait for host eMMC PHY init completes */
- udelay(wait);
- reg = sdhci_readl(host, phy_regs->timing_adj);
- reg &= XENON_PHY_INITIALIZAION;
- if (reg) {
+ /*
+ * AC5X spec says bit must be polled until zero.
+ * We see cases in which timeout can take longer
+ * than the standard calculation on AC5X, which is
+ * expected following the spec comment above.
+ * According to the spec, we must wait as long as
+ * it takes for that bit to toggle on AC5X.
+ * Cap that with 100 delay loops so we won't get
+ * stuck here forever:
+ */
+
+ ret = read_poll_timeout(sdhci_readl, reg,
+ !(reg & XENON_PHY_INITIALIZAION),
+ wait, XENON_MAX_PHY_TIMEOUT_LOOPS * wait,
+ false, host, phy_regs->timing_adj);
+ if (ret)
dev_err(mmc_dev(host->mmc), "eMMC PHY init cannot complete after %d us\n",
- wait);
- return -ETIMEDOUT;
- }
+ wait * XENON_MAX_PHY_TIMEOUT_LOOPS);
- return 0;
+ return ret;
}
#define ARMADA_3700_SOC_PAD_1_8V 0x1
--
2.25.1
On 15/02/24 19:01, Elad Nachman wrote:
>
>
>> -----Original Message-----
>> From: Ulf Hansson <[email protected]>
>> Sent: Thursday, February 15, 2024 6:51 PM
>> To: Elad Nachman <[email protected]>
>> Cc: [email protected]; [email protected]; linux-
>> [email protected]; [email protected]
>> Subject: [EXT] Re: [PATCH v2 0/2] Fix PHY init timeout issues
>>
>> External Email
>>
>> ----------------------------------------------------------------------
>> On Thu, 15 Feb 2024 at 17:16, Elad Nachman <[email protected]>
>> wrote:
>>>
>>> From: Elad Nachman <[email protected]>
>>>
>>> Fix PHY init timeout issues:
>>>
>>> 1. Clock Stability issue causing PHY timeout
>>>
>>> 2. Timeout taking longer than needed on AC5X.
>>> Solve by constantly testing the PHY init bit
>>> until it toggles, but up to 100X timeout factor.
>>>
>>> v2:
>>> 1) convert polling loop to read_poll_timeout()
>>> for both patches.
>>>
>>> Elad Nachman (2):
>>> mmc: xenon: fix PHY init clock stability
>>> mmc: xenon: add timeout for PHY init complete
>>>
>>> drivers/mmc/host/sdhci-xenon-phy.c | 48
>>> ++++++++++++++++++++++++------
>>> 1 file changed, 39 insertions(+), 9 deletions(-)
>>>
>>
>> The series looks good to me. Although, I assume we should tag this for stable
>> kernels too and possibly add a fixes tag?
>
> No problem.
Presumably:
Fixes: 06c8b667ff5b ("mmc: sdhci-xenon: Add support to PHYs of Marvell Xenon SDHC")
FWIW:
Acked-by: Adrian Hunter <[email protected]>
>
>>
>> Moreover, it would be nice to get an ack from Hu Ziji.
>
> He does not longer work with Marvell, so this email is invalid, I tried also to send an e-mail manually as well and got:
> [email protected]
> Remote Server returned '550 5.1.1 RESOLVER.ADR.RecipNotFound; not found'
>
> Could not locate him via google search either.
>
>>
>> Kind regards
>> Uffe