| Message ID | 20241022111025.25157-1-quic_rampraka@quicinc.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/1] mmc: sdhci-msm: Toggle the FIFO write clock after ungate | expand |
On Tue, 22 Oct 2024 at 13:10, Ram Prakash Gupta <quic_rampraka@quicinc.com> wrote: > > For Qualcomm SoCs with sdcc minor version 6B and more, command path > state machine is getting corrupted post clock ungate which is leading > to software timeout. > > Toggle the write fifo clock to reset the async fifo to fix this issue. > > Signed-off-by: Ram Prakash Gupta <quic_rampraka@quicinc.com> Seems like we should add a stable tag, but do we have a fixes tag that we can use too? Kind regards Uffe > --- > drivers/mmc/host/sdhci-msm.c | 41 ++++++++++++++++++++++++++++++++++++ > 1 file changed, 41 insertions(+) > > diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c > index e113b99a3eab..c2ccdac21232 100644 > --- a/drivers/mmc/host/sdhci-msm.c > +++ b/drivers/mmc/host/sdhci-msm.c > @@ -146,6 +146,7 @@ > /* CQHCI vendor specific registers */ > #define CQHCI_VENDOR_CFG1 0xA00 > #define CQHCI_VENDOR_DIS_RST_ON_CQ_EN (0x3 << 13) > +#define RCLK_TOGGLE BIT(1) > > struct sdhci_msm_offset { > u32 core_hc_mode; > @@ -290,6 +291,7 @@ struct sdhci_msm_host { > u32 dll_config; > u32 ddr_config; > bool vqmmc_enabled; > + bool toggle_fifo_clk; > }; > > static const struct sdhci_msm_offset *sdhci_priv_msm_offset(struct sdhci_host *host) > @@ -1162,6 +1164,39 @@ static int sdhci_msm_restore_sdr_dll_config(struct sdhci_host *host) > return ret; > } > > +/* > + * After MCLK ugating, toggle the FIFO write clock to get > + * the FIFO pointers and flags to valid state. > + */ > +static void sdhci_msm_toggle_fifo_write_clk(struct sdhci_host *host) > +{ > + u32 config; > + struct mmc_ios ios = host->mmc->ios; > + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); > + struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host); > + const struct sdhci_msm_offset *msm_offset = msm_host->offset; > + > + if ((msm_host->tuning_done || ios.enhanced_strobe) && > + host->mmc->ios.timing == MMC_TIMING_MMC_HS400) { > + /* > + * Select MCLK as DLL input clock. > + */ > + config = readl_relaxed(host->ioaddr + msm_offset->core_dll_config_3); > + config |= RCLK_TOGGLE; > + writel_relaxed(config, host->ioaddr + msm_offset->core_dll_config_3); > + > + /* ensure above write as toggling same bit quickly */ > + wmb(); > + udelay(2); > + > + /* > + * Select RCLK as DLL input clock > + */ > + config &= ~RCLK_TOGGLE; > + writel_relaxed(config, host->ioaddr + msm_offset->core_dll_config_3); > + } > +} > + > static void sdhci_msm_set_cdr(struct sdhci_host *host, bool enable) > { > const struct sdhci_msm_offset *msm_offset = sdhci_priv_msm_offset(host); > @@ -2587,6 +2622,9 @@ static int sdhci_msm_probe(struct platform_device *pdev) > if (core_major == 1 && core_minor >= 0x71) > msm_host->uses_tassadar_dll = true; > > + if (core_major == 1 && core_minor >= 0x6B) > + msm_host->toggle_fifo_clk = true; > + > ret = sdhci_msm_register_vreg(msm_host); > if (ret) > goto clk_disable; > @@ -2720,6 +2758,9 @@ static __maybe_unused int sdhci_msm_runtime_resume(struct device *dev) > msm_host->bulk_clks); > if (ret) > return ret; > + > + if (msm_host->toggle_fifo_clk) > + sdhci_msm_toggle_fifo_write_clk(host); > /* > * Whenever core-clock is gated dynamically, it's needed to > * restore the SDR DLL settings when the clock is ungated. > -- > 2.17.1 >
On Tue, Oct 22, 2024 at 04:40:25PM GMT, Ram Prakash Gupta wrote: > For Qualcomm SoCs with sdcc minor version 6B and more, command path > state machine is getting corrupted post clock ungate which is leading > to software timeout. > > Toggle the write fifo clock to reset the async fifo to fix this issue. > > Signed-off-by: Ram Prakash Gupta <quic_rampraka@quicinc.com> > --- > drivers/mmc/host/sdhci-msm.c | 41 ++++++++++++++++++++++++++++++++++++ > 1 file changed, 41 insertions(+) > > diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c > index e113b99a3eab..c2ccdac21232 100644 > --- a/drivers/mmc/host/sdhci-msm.c > +++ b/drivers/mmc/host/sdhci-msm.c > @@ -146,6 +146,7 @@ > /* CQHCI vendor specific registers */ > #define CQHCI_VENDOR_CFG1 0xA00 > #define CQHCI_VENDOR_DIS_RST_ON_CQ_EN (0x3 << 13) > +#define RCLK_TOGGLE BIT(1) > > struct sdhci_msm_offset { > u32 core_hc_mode; > @@ -290,6 +291,7 @@ struct sdhci_msm_host { > u32 dll_config; > u32 ddr_config; > bool vqmmc_enabled; > + bool toggle_fifo_clk; > }; > > static const struct sdhci_msm_offset *sdhci_priv_msm_offset(struct sdhci_host *host) > @@ -1162,6 +1164,39 @@ static int sdhci_msm_restore_sdr_dll_config(struct sdhci_host *host) > return ret; > } > > +/* > + * After MCLK ugating, toggle the FIFO write clock to get > + * the FIFO pointers and flags to valid state. > + */ > +static void sdhci_msm_toggle_fifo_write_clk(struct sdhci_host *host) > +{ > + u32 config; > + struct mmc_ios ios = host->mmc->ios; > + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); > + struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host); > + const struct sdhci_msm_offset *msm_offset = msm_host->offset; > + > + if ((msm_host->tuning_done || ios.enhanced_strobe) && > + host->mmc->ios.timing == MMC_TIMING_MMC_HS400) { > + /* > + * Select MCLK as DLL input clock. > + */ Seems you could fit this in a single-line comment, perhaps with an empty line above if you want to create some separation(?) > + config = readl_relaxed(host->ioaddr + msm_offset->core_dll_config_3); > + config |= RCLK_TOGGLE; > + writel_relaxed(config, host->ioaddr + msm_offset->core_dll_config_3); > + > + /* ensure above write as toggling same bit quickly */ "same bit quickly" is completely arbitrary. Please state exactly what you need to wait for? E.g. is it some number of some clock ticks? If so, which clock and is it 1, 5, or 42 ticks? > + wmb(); This does not guarantee that the write completes before the delay. If you want to know that the write hit the hardware, before the sleep, issue a readl. Regards, Bjorn > + udelay(2); > + > + /* > + * Select RCLK as DLL input clock > + */ > + config &= ~RCLK_TOGGLE; > + writel_relaxed(config, host->ioaddr + msm_offset->core_dll_config_3); > + } > +} > + > static void sdhci_msm_set_cdr(struct sdhci_host *host, bool enable) > { > const struct sdhci_msm_offset *msm_offset = sdhci_priv_msm_offset(host); > @@ -2587,6 +2622,9 @@ static int sdhci_msm_probe(struct platform_device *pdev) > if (core_major == 1 && core_minor >= 0x71) > msm_host->uses_tassadar_dll = true; > > + if (core_major == 1 && core_minor >= 0x6B) > + msm_host->toggle_fifo_clk = true; > + > ret = sdhci_msm_register_vreg(msm_host); > if (ret) > goto clk_disable; > @@ -2720,6 +2758,9 @@ static __maybe_unused int sdhci_msm_runtime_resume(struct device *dev) > msm_host->bulk_clks); > if (ret) > return ret; > + > + if (msm_host->toggle_fifo_clk) > + sdhci_msm_toggle_fifo_write_clk(host); > /* > * Whenever core-clock is gated dynamically, it's needed to > * restore the SDR DLL settings when the clock is ungated. > -- > 2.17.1 > >
On 10/22/2024 6:15 PM, Ulf Hansson wrote: > On Tue, 22 Oct 2024 at 13:10, Ram Prakash Gupta > <quic_rampraka@quicinc.com> wrote: >> >> For Qualcomm SoCs with sdcc minor version 6B and more, command path >> state machine is getting corrupted post clock ungate which is leading >> to software timeout. >> >> Toggle the write fifo clock to reset the async fifo to fix this issue. >> >> Signed-off-by: Ram Prakash Gupta <quic_rampraka@quicinc.com> > > Seems like we should add a stable tag, but do we have a fixes tag that > we can use too? > > Kind regards > Uffe > No, we don't have fixes tag for this as this is not dependent on any software version. And there is no way to map minor version with stable tag also here, I guess. But I agree this needs stable tag, if you have any suggestion to find right stable tag, we can add. Thanks Ram >> --- >> drivers/mmc/host/sdhci-msm.c | 41 ++++++++++++++++++++++++++++++++++++ >> 1 file changed, 41 insertions(+) >> >> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c >> index e113b99a3eab..c2ccdac21232 100644 >> --- a/drivers/mmc/host/sdhci-msm.c >> +++ b/drivers/mmc/host/sdhci-msm.c >> @@ -146,6 +146,7 @@ >> /* CQHCI vendor specific registers */ >> #define CQHCI_VENDOR_CFG1 0xA00 >> #define CQHCI_VENDOR_DIS_RST_ON_CQ_EN (0x3 << 13) >> +#define RCLK_TOGGLE BIT(1) >> >> struct sdhci_msm_offset { >> u32 core_hc_mode; >> @@ -290,6 +291,7 @@ struct sdhci_msm_host { >> u32 dll_config; >> u32 ddr_config; >> bool vqmmc_enabled; >> + bool toggle_fifo_clk; >> }; >> >> static const struct sdhci_msm_offset *sdhci_priv_msm_offset(struct sdhci_host *host) >> @@ -1162,6 +1164,39 @@ static int sdhci_msm_restore_sdr_dll_config(struct sdhci_host *host) >> return ret; >> } >> >> +/* >> + * After MCLK ugating, toggle the FIFO write clock to get >> + * the FIFO pointers and flags to valid state. >> + */ >> +static void sdhci_msm_toggle_fifo_write_clk(struct sdhci_host *host) >> +{ >> + u32 config; >> + struct mmc_ios ios = host->mmc->ios; >> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); >> + struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host); >> + const struct sdhci_msm_offset *msm_offset = msm_host->offset; >> + >> + if ((msm_host->tuning_done || ios.enhanced_strobe) && >> + host->mmc->ios.timing == MMC_TIMING_MMC_HS400) { >> + /* >> + * Select MCLK as DLL input clock. >> + */ >> + config = readl_relaxed(host->ioaddr + msm_offset->core_dll_config_3); >> + config |= RCLK_TOGGLE; >> + writel_relaxed(config, host->ioaddr + msm_offset->core_dll_config_3); >> + >> + /* ensure above write as toggling same bit quickly */ >> + wmb(); >> + udelay(2); >> + >> + /* >> + * Select RCLK as DLL input clock >> + */ >> + config &= ~RCLK_TOGGLE; >> + writel_relaxed(config, host->ioaddr + msm_offset->core_dll_config_3); >> + } >> +} >> + >> static void sdhci_msm_set_cdr(struct sdhci_host *host, bool enable) >> { >> const struct sdhci_msm_offset *msm_offset = sdhci_priv_msm_offset(host); >> @@ -2587,6 +2622,9 @@ static int sdhci_msm_probe(struct platform_device *pdev) >> if (core_major == 1 && core_minor >= 0x71) >> msm_host->uses_tassadar_dll = true; >> >> + if (core_major == 1 && core_minor >= 0x6B) >> + msm_host->toggle_fifo_clk = true; >> + >> ret = sdhci_msm_register_vreg(msm_host); >> if (ret) >> goto clk_disable; >> @@ -2720,6 +2758,9 @@ static __maybe_unused int sdhci_msm_runtime_resume(struct device *dev) >> msm_host->bulk_clks); >> if (ret) >> return ret; >> + >> + if (msm_host->toggle_fifo_clk) >> + sdhci_msm_toggle_fifo_write_clk(host); >> /* >> * Whenever core-clock is gated dynamically, it's needed to >> * restore the SDR DLL settings when the clock is ungated. >> -- >> 2.17.1 >>
On Fri, Oct 25, 2024 at 07:04:37AM +0530, Ram Prakash Gupta wrote: > > > On 10/22/2024 6:15 PM, Ulf Hansson wrote: > > On Tue, 22 Oct 2024 at 13:10, Ram Prakash Gupta > > <quic_rampraka@quicinc.com> wrote: > > > > > > For Qualcomm SoCs with sdcc minor version 6B and more, command path > > > state machine is getting corrupted post clock ungate which is leading > > > to software timeout. Affected platforms are ... > > > > > > Toggle the write fifo clock to reset the async fifo to fix this issue. > > > > > > Signed-off-by: Ram Prakash Gupta <quic_rampraka@quicinc.com> > > > > Seems like we should add a stable tag, but do we have a fixes tag that > > we can use too? > > > > Kind regards > > Uffe > > > > No, we don't have fixes tag for this as this is not dependent on any > software version. And there is no way to map minor version with stable > tag also here, I guess. But I agree this needs stable tag, if you have > any suggestion to find right stable tag, we can add. In the least case you can add Fixes tag pointing to the first affected hardware being added to the DT schema. As for the stable tag, I think, Uffe has been asking for Cc:stable in adddition to Fixes. > > Thanks > Ram > > > > --- > > > drivers/mmc/host/sdhci-msm.c | 41 ++++++++++++++++++++++++++++++++++++ > > > 1 file changed, 41 insertions(+) > > > > > > diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c > > > index e113b99a3eab..c2ccdac21232 100644 > > > --- a/drivers/mmc/host/sdhci-msm.c > > > +++ b/drivers/mmc/host/sdhci-msm.c > > > @@ -146,6 +146,7 @@ > > > /* CQHCI vendor specific registers */ > > > #define CQHCI_VENDOR_CFG1 0xA00 > > > #define CQHCI_VENDOR_DIS_RST_ON_CQ_EN (0x3 << 13) > > > +#define RCLK_TOGGLE BIT(1) > > > > > > struct sdhci_msm_offset { > > > u32 core_hc_mode; > > > @@ -290,6 +291,7 @@ struct sdhci_msm_host { > > > u32 dll_config; > > > u32 ddr_config; > > > bool vqmmc_enabled; > > > + bool toggle_fifo_clk; > > > }; > > > > > > static const struct sdhci_msm_offset *sdhci_priv_msm_offset(struct sdhci_host *host) > > > @@ -1162,6 +1164,39 @@ static int sdhci_msm_restore_sdr_dll_config(struct sdhci_host *host) > > > return ret; > > > } > > > > > > +/* > > > + * After MCLK ugating, toggle the FIFO write clock to get > > > + * the FIFO pointers and flags to valid state. > > > + */ > > > +static void sdhci_msm_toggle_fifo_write_clk(struct sdhci_host *host) > > > +{ > > > + u32 config; > > > + struct mmc_ios ios = host->mmc->ios; > > > + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); > > > + struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host); > > > + const struct sdhci_msm_offset *msm_offset = msm_host->offset; > > > + > > > + if ((msm_host->tuning_done || ios.enhanced_strobe) && > > > + host->mmc->ios.timing == MMC_TIMING_MMC_HS400) { > > > + /* > > > + * Select MCLK as DLL input clock. > > > + */ > > > + config = readl_relaxed(host->ioaddr + msm_offset->core_dll_config_3); > > > + config |= RCLK_TOGGLE; > > > + writel_relaxed(config, host->ioaddr + msm_offset->core_dll_config_3); > > > + > > > + /* ensure above write as toggling same bit quickly */ > > > + wmb(); > > > + udelay(2); > > > + > > > + /* > > > + * Select RCLK as DLL input clock > > > + */ > > > + config &= ~RCLK_TOGGLE; > > > + writel_relaxed(config, host->ioaddr + msm_offset->core_dll_config_3); > > > + } > > > +} > > > + > > > static void sdhci_msm_set_cdr(struct sdhci_host *host, bool enable) > > > { > > > const struct sdhci_msm_offset *msm_offset = sdhci_priv_msm_offset(host); > > > @@ -2587,6 +2622,9 @@ static int sdhci_msm_probe(struct platform_device *pdev) > > > if (core_major == 1 && core_minor >= 0x71) > > > msm_host->uses_tassadar_dll = true; > > > > > > + if (core_major == 1 && core_minor >= 0x6B) lowercase hex, please. > > > + msm_host->toggle_fifo_clk = true; > > > + > > > ret = sdhci_msm_register_vreg(msm_host); > > > if (ret) > > > goto clk_disable; > > > @@ -2720,6 +2758,9 @@ static __maybe_unused int sdhci_msm_runtime_resume(struct device *dev) > > > msm_host->bulk_clks); > > > if (ret) > > > return ret; > > > + > > > + if (msm_host->toggle_fifo_clk) > > > + sdhci_msm_toggle_fifo_write_clk(host); > > > /* > > > * Whenever core-clock is gated dynamically, it's needed to > > > * restore the SDR DLL settings when the clock is ungated. > > > -- > > > 2.17.1 > > >
diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c index e113b99a3eab..c2ccdac21232 100644 --- a/drivers/mmc/host/sdhci-msm.c +++ b/drivers/mmc/host/sdhci-msm.c @@ -146,6 +146,7 @@ /* CQHCI vendor specific registers */ #define CQHCI_VENDOR_CFG1 0xA00 #define CQHCI_VENDOR_DIS_RST_ON_CQ_EN (0x3 << 13) +#define RCLK_TOGGLE BIT(1) struct sdhci_msm_offset { u32 core_hc_mode; @@ -290,6 +291,7 @@ struct sdhci_msm_host { u32 dll_config; u32 ddr_config; bool vqmmc_enabled; + bool toggle_fifo_clk; }; static const struct sdhci_msm_offset *sdhci_priv_msm_offset(struct sdhci_host *host) @@ -1162,6 +1164,39 @@ static int sdhci_msm_restore_sdr_dll_config(struct sdhci_host *host) return ret; } +/* + * After MCLK ugating, toggle the FIFO write clock to get + * the FIFO pointers and flags to valid state. + */ +static void sdhci_msm_toggle_fifo_write_clk(struct sdhci_host *host) +{ + u32 config; + struct mmc_ios ios = host->mmc->ios; + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); + struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host); + const struct sdhci_msm_offset *msm_offset = msm_host->offset; + + if ((msm_host->tuning_done || ios.enhanced_strobe) && + host->mmc->ios.timing == MMC_TIMING_MMC_HS400) { + /* + * Select MCLK as DLL input clock. + */ + config = readl_relaxed(host->ioaddr + msm_offset->core_dll_config_3); + config |= RCLK_TOGGLE; + writel_relaxed(config, host->ioaddr + msm_offset->core_dll_config_3); + + /* ensure above write as toggling same bit quickly */ + wmb(); + udelay(2); + + /* + * Select RCLK as DLL input clock + */ + config &= ~RCLK_TOGGLE; + writel_relaxed(config, host->ioaddr + msm_offset->core_dll_config_3); + } +} + static void sdhci_msm_set_cdr(struct sdhci_host *host, bool enable) { const struct sdhci_msm_offset *msm_offset = sdhci_priv_msm_offset(host); @@ -2587,6 +2622,9 @@ static int sdhci_msm_probe(struct platform_device *pdev) if (core_major == 1 && core_minor >= 0x71) msm_host->uses_tassadar_dll = true; + if (core_major == 1 && core_minor >= 0x6B) + msm_host->toggle_fifo_clk = true; + ret = sdhci_msm_register_vreg(msm_host); if (ret) goto clk_disable; @@ -2720,6 +2758,9 @@ static __maybe_unused int sdhci_msm_runtime_resume(struct device *dev) msm_host->bulk_clks); if (ret) return ret; + + if (msm_host->toggle_fifo_clk) + sdhci_msm_toggle_fifo_write_clk(host); /* * Whenever core-clock is gated dynamically, it's needed to * restore the SDR DLL settings when the clock is ungated.
For Qualcomm SoCs with sdcc minor version 6B and more, command path state machine is getting corrupted post clock ungate which is leading to software timeout. Toggle the write fifo clock to reset the async fifo to fix this issue. Signed-off-by: Ram Prakash Gupta <quic_rampraka@quicinc.com> --- drivers/mmc/host/sdhci-msm.c | 41 ++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+)