Reset GCC_SDCC_BCR register before every fresh initilazation. This will
reset whole SDHC-msm controller, clears the previous power control
states and avoids, software reset timeout issues as below.
[ 5.458061][ T262] mmc1: Reset 0x1 never completed.
[ 5.462454][ T262] mmc1: sdhci: ============ SDHCI REGISTER DUMP
===========
[ 5.469065][ T262] mmc1: sdhci: Sys addr: 0x00000000 | Version:
0x00007202
[ 5.475688][ T262] mmc1: sdhci: Blk size: 0x00000000 | Blk cnt:
0x00000000
[ 5.482315][ T262] mmc1: sdhci: Argument: 0x00000000 | Trn mode:
0x00000000
[ 5.488927][ T262] mmc1: sdhci: Present: 0x01f800f0 | Host ctl:
0x00000000
[ 5.495539][ T262] mmc1: sdhci: Power: 0x00000000 | Blk gap: 0x00000000
[ 5.502162][ T262] mmc1: sdhci: Wake-up: 0x00000000 | Clock: 0x00000003
[ 5.508768][ T262] mmc1: sdhci: Timeout: 0x00000000 | Int stat:
0x00000000
[ 5.515381][ T262] mmc1: sdhci: Int enab: 0x00000000 | Sig enab:
0x00000000
[ 5.521996][ T262] mmc1: sdhci: ACmd stat: 0x00000000 | Slot int:
0x00000000
[ 5.528607][ T262] mmc1: sdhci: Caps: 0x362dc8b2 | Caps_1: 0x0000808f
[ 5.535227][ T262] mmc1: sdhci: Cmd: 0x00000000 | Max curr: 0x00000000
[ 5.541841][ T262] mmc1: sdhci: Resp[0]: 0x00000000 | Resp[1]:
0x00000000
[ 5.548454][ T262] mmc1: sdhci: Resp[2]: 0x00000000 | Resp[3]:
0x00000000
[ 5.555079][ T262] mmc1: sdhci: Host ctl2: 0x00000000
[ 5.559651][ T262] mmc1: sdhci_msm: ----------- VENDOR REGISTER
DUMP-----------
[ 5.566621][ T262] mmc1: sdhci_msm: DLL sts: 0x00000000 | DLL cfg:
0x6000642c |
DLL cfg2: 0x0020a000
[ 5.575465][ T262] mmc1: sdhci_msm: DLL cfg3: 0x00000000 | DLL usr ctl:
0x00010800 | DDR cfg: 0x80040873
[ 5.584658][ T262] mmc1: sdhci_msm: Vndr func: 0x00018a9c | Vndr func2 :
0xf88218a8 Vndr func3: 0x02626040
Signed-off-by: Shaik Sajida Bhanu <[email protected]>
---
drivers/mmc/host/sdhci-msm.c | 48 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 48 insertions(+)
diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 50c71e0..f10b3c7 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -17,6 +17,7 @@
#include <linux/regulator/consumer.h>
#include <linux/interconnect.h>
#include <linux/pinctrl/consumer.h>
+#include <linux/reset.h>
#include "sdhci-pltfm.h"
#include "cqhci.h"
@@ -284,6 +285,7 @@ struct sdhci_msm_host {
bool uses_tassadar_dll;
u32 dll_config;
u32 ddr_config;
+ struct reset_control *core_reset;
bool vqmmc_enabled;
};
@@ -2482,6 +2484,45 @@ static inline void sdhci_msm_get_of_property(struct platform_device *pdev,
of_property_read_u32(node, "qcom,dll-config", &msm_host->dll_config);
}
+static int sdhci_msm_gcc_reset(struct platform_device *pdev,
+ struct sdhci_host *host)
+{
+ struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+ struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
+ int ret = 0;
+
+ msm_host->core_reset = devm_reset_control_get(&pdev->dev, "core_reset");
+ if (IS_ERR(msm_host->core_reset)) {
+ ret = PTR_ERR(msm_host->core_reset);
+ dev_err(&pdev->dev, "core_reset unavailable (%d)\n", ret);
+ msm_host->core_reset = NULL;
+ }
+ if (msm_host->core_reset) {
+ ret = reset_control_assert(msm_host->core_reset);
+ if (ret) {
+ dev_err(&pdev->dev, "core_reset assert failed (%d)\n",
+ ret);
+ goto out;
+ }
+ /*
+ * The hardware requirement for delay between assert/deassert
+ * is at least 3-4 sleep clock (32.7KHz) cycles, which comes to
+ * ~125us (4/32768). To be on the safe side add 200us delay.
+ */
+ usleep_range(200, 210);
+
+ ret = reset_control_deassert(msm_host->core_reset);
+ if (ret) {
+ dev_err(&pdev->dev, "core_reset deassert failed (%d)\n",
+ ret);
+ goto out;
+ }
+ usleep_range(200, 210);
+ }
+
+out:
+ return ret;
+}
static int sdhci_msm_probe(struct platform_device *pdev)
{
@@ -2529,6 +2570,13 @@ static int sdhci_msm_probe(struct platform_device *pdev)
msm_host->saved_tuning_phase = INVALID_TUNING_PHASE;
+ ret = sdhci_msm_gcc_reset(pdev, host);
+ if (ret) {
+ dev_err(&pdev->dev, "core_reset assert/deassert failed (%d)\n",
+ ret);
+ goto pltfm_free;
+ }
+
/* Setup SDCC bus voter clock. */
msm_host->bus_clk = devm_clk_get(&pdev->dev, "bus");
if (!IS_ERR(msm_host->bus_clk)) {
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
On Tue, 1 Mar 2022 at 07:55, Shaik Sajida Bhanu
<[email protected]> wrote:
>
> Reset GCC_SDCC_BCR register before every fresh initilazation. This will
> reset whole SDHC-msm controller, clears the previous power control
> states and avoids, software reset timeout issues as below.
>
> [ 5.458061][ T262] mmc1: Reset 0x1 never completed.
> [ 5.462454][ T262] mmc1: sdhci: ============ SDHCI REGISTER DUMP
> ===========
> [ 5.469065][ T262] mmc1: sdhci: Sys addr: 0x00000000 | Version:
> 0x00007202
> [ 5.475688][ T262] mmc1: sdhci: Blk size: 0x00000000 | Blk cnt:
> 0x00000000
> [ 5.482315][ T262] mmc1: sdhci: Argument: 0x00000000 | Trn mode:
> 0x00000000
> [ 5.488927][ T262] mmc1: sdhci: Present: 0x01f800f0 | Host ctl:
> 0x00000000
> [ 5.495539][ T262] mmc1: sdhci: Power: 0x00000000 | Blk gap: 0x00000000
> [ 5.502162][ T262] mmc1: sdhci: Wake-up: 0x00000000 | Clock: 0x00000003
> [ 5.508768][ T262] mmc1: sdhci: Timeout: 0x00000000 | Int stat:
> 0x00000000
> [ 5.515381][ T262] mmc1: sdhci: Int enab: 0x00000000 | Sig enab:
> 0x00000000
> [ 5.521996][ T262] mmc1: sdhci: ACmd stat: 0x00000000 | Slot int:
> 0x00000000
> [ 5.528607][ T262] mmc1: sdhci: Caps: 0x362dc8b2 | Caps_1: 0x0000808f
> [ 5.535227][ T262] mmc1: sdhci: Cmd: 0x00000000 | Max curr: 0x00000000
> [ 5.541841][ T262] mmc1: sdhci: Resp[0]: 0x00000000 | Resp[1]:
> 0x00000000
> [ 5.548454][ T262] mmc1: sdhci: Resp[2]: 0x00000000 | Resp[3]:
> 0x00000000
> [ 5.555079][ T262] mmc1: sdhci: Host ctl2: 0x00000000
> [ 5.559651][ T262] mmc1: sdhci_msm: ----------- VENDOR REGISTER
> DUMP-----------
> [ 5.566621][ T262] mmc1: sdhci_msm: DLL sts: 0x00000000 | DLL cfg:
> 0x6000642c |
> DLL cfg2: 0x0020a000
> [ 5.575465][ T262] mmc1: sdhci_msm: DLL cfg3: 0x00000000 | DLL usr ctl:
> 0x00010800 | DDR cfg: 0x80040873
> [ 5.584658][ T262] mmc1: sdhci_msm: Vndr func: 0x00018a9c | Vndr func2 :
> 0xf88218a8 Vndr func3: 0x02626040
>
> Signed-off-by: Shaik Sajida Bhanu <[email protected]>
If this is this a regression, then please try to add a fixes tag too.
I assume we should tag this for stable kernels?
> ---
> drivers/mmc/host/sdhci-msm.c | 48 ++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 48 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index 50c71e0..f10b3c7 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -17,6 +17,7 @@
> #include <linux/regulator/consumer.h>
> #include <linux/interconnect.h>
> #include <linux/pinctrl/consumer.h>
> +#include <linux/reset.h>
>
> #include "sdhci-pltfm.h"
> #include "cqhci.h"
> @@ -284,6 +285,7 @@ struct sdhci_msm_host {
> bool uses_tassadar_dll;
> u32 dll_config;
> u32 ddr_config;
> + struct reset_control *core_reset;
> bool vqmmc_enabled;
> };
>
> @@ -2482,6 +2484,45 @@ static inline void sdhci_msm_get_of_property(struct platform_device *pdev,
> of_property_read_u32(node, "qcom,dll-config", &msm_host->dll_config);
> }
>
> +static int sdhci_msm_gcc_reset(struct platform_device *pdev,
> + struct sdhci_host *host)
> +{
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> + int ret = 0;
> +
> + msm_host->core_reset = devm_reset_control_get(&pdev->dev, "core_reset");
> + if (IS_ERR(msm_host->core_reset)) {
> + ret = PTR_ERR(msm_host->core_reset);
> + dev_err(&pdev->dev, "core_reset unavailable (%d)\n", ret);
> + msm_host->core_reset = NULL;
Looks like we should use devm_reset_control_get_optional_exclusive() instead.
> + }
> + if (msm_host->core_reset) {
> + ret = reset_control_assert(msm_host->core_reset);
> + if (ret) {
> + dev_err(&pdev->dev, "core_reset assert failed (%d)\n",
> + ret);
> + goto out;
> + }
> + /*
> + * The hardware requirement for delay between assert/deassert
> + * is at least 3-4 sleep clock (32.7KHz) cycles, which comes to
> + * ~125us (4/32768). To be on the safe side add 200us delay.
> + */
> + usleep_range(200, 210);
Isn't this supposed to be taken care of by the reset driver?
Or is this more an mmc controller specific thing? In that case, could
this delay vary, depending on the variant of the controller?
> +
> + ret = reset_control_deassert(msm_host->core_reset);
> + if (ret) {
> + dev_err(&pdev->dev, "core_reset deassert failed (%d)\n",
> + ret);
> + goto out;
> + }
> + usleep_range(200, 210);
Ditto?
> + }
> +
> +out:
> + return ret;
> +}
>
> static int sdhci_msm_probe(struct platform_device *pdev)
> {
> @@ -2529,6 +2570,13 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>
> msm_host->saved_tuning_phase = INVALID_TUNING_PHASE;
>
> + ret = sdhci_msm_gcc_reset(pdev, host);
> + if (ret) {
> + dev_err(&pdev->dev, "core_reset assert/deassert failed (%d)\n",
> + ret);
> + goto pltfm_free;
> + }
> +
> /* Setup SDCC bus voter clock. */
> msm_host->bus_clk = devm_clk_get(&pdev->dev, "bus");
> if (!IS_ERR(msm_host->bus_clk)) {
Kind regards
Uffe
Hi,
Thanks for the review.
Please find the inline comments.
Thanks,
Sajida
> -----Original Message-----
> From: Ulf Hansson <[email protected]>
> Sent: Wednesday, March 2, 2022 5:16 PM
> To: Sajida Bhanu (Temp) (QUIC) <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; linux-
> [email protected]; [email protected]
> Subject: Re: [PATCH V1] mmc: sdhci-msm: Reset GCC_SDCC_BCR register for
> SDHC
>
> On Tue, 1 Mar 2022 at 07:55, Shaik Sajida Bhanu
> <[email protected]> wrote:
> >
> > Reset GCC_SDCC_BCR register before every fresh initilazation. This
> > will reset whole SDHC-msm controller, clears the previous power
> > control states and avoids, software reset timeout issues as below.
> >
> > [ 5.458061][ T262] mmc1: Reset 0x1 never completed.
> > [ 5.462454][ T262] mmc1: sdhci: ============ SDHCI REGISTER DUMP
> > =========== [ 5.469065][ T262] mmc1: sdhci: Sys addr: 0x00000000 |
> > Version:
> > 0x00007202
> > [ 5.475688][ T262] mmc1: sdhci: Blk size: 0x00000000 | Blk cnt:
> > 0x00000000
> > [ 5.482315][ T262] mmc1: sdhci: Argument: 0x00000000 | Trn mode:
> > 0x00000000
> > [ 5.488927][ T262] mmc1: sdhci: Present: 0x01f800f0 | Host ctl:
> > 0x00000000
> > [ 5.495539][ T262] mmc1: sdhci: Power: 0x00000000 | Blk gap:
> > 0x00000000 [ 5.502162][ T262] mmc1: sdhci: Wake-up: 0x00000000 |
> > Clock: 0x00000003 [ 5.508768][ T262] mmc1: sdhci: Timeout: 0x00000000 |
> Int stat:
> > 0x00000000
> > [ 5.515381][ T262] mmc1: sdhci: Int enab: 0x00000000 | Sig enab:
> > 0x00000000
> > [ 5.521996][ T262] mmc1: sdhci: ACmd stat: 0x00000000 | Slot int:
> > 0x00000000
> > [ 5.528607][ T262] mmc1: sdhci: Caps: 0x362dc8b2 | Caps_1: 0x0000808f
> > [ 5.535227][ T262] mmc1: sdhci: Cmd: 0x00000000 | Max curr: 0x00000000
> > [ 5.541841][ T262] mmc1: sdhci: Resp[0]: 0x00000000 | Resp[1]:
> > 0x00000000
> > [ 5.548454][ T262] mmc1: sdhci: Resp[2]: 0x00000000 | Resp[3]:
> > 0x00000000
> > [ 5.555079][ T262] mmc1: sdhci: Host ctl2: 0x00000000 [ 5.559651][
> > T262] mmc1: sdhci_msm: ----------- VENDOR REGISTER
> > DUMP-----------
> > [ 5.566621][ T262] mmc1: sdhci_msm: DLL sts: 0x00000000 | DLL cfg:
> > 0x6000642c |
> > DLL cfg2: 0x0020a000
> > [ 5.575465][ T262] mmc1: sdhci_msm: DLL cfg3: 0x00000000 | DLL usr ctl:
> > 0x00010800 | DDR cfg: 0x80040873
> > [ 5.584658][ T262] mmc1: sdhci_msm: Vndr func: 0x00018a9c | Vndr func2 :
> > 0xf88218a8 Vndr func3: 0x02626040
> >
> > Signed-off-by: Shaik Sajida Bhanu <[email protected]>
>
> If this is this a regression, then please try to add a fixes tag too.
>
Yes, we have buganizer for this, will add in next patchset.
> I assume we should tag this for stable kernels?
>
> > ---
> > drivers/mmc/host/sdhci-msm.c | 48
> > ++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 48 insertions(+)
> >
> > diff --git a/drivers/mmc/host/sdhci-msm.c
> > b/drivers/mmc/host/sdhci-msm.c index 50c71e0..f10b3c7 100644
> > --- a/drivers/mmc/host/sdhci-msm.c
> > +++ b/drivers/mmc/host/sdhci-msm.c
> > @@ -17,6 +17,7 @@
> > #include <linux/regulator/consumer.h> #include
> > <linux/interconnect.h> #include <linux/pinctrl/consumer.h>
> > +#include <linux/reset.h>
> >
> > #include "sdhci-pltfm.h"
> > #include "cqhci.h"
> > @@ -284,6 +285,7 @@ struct sdhci_msm_host {
> > bool uses_tassadar_dll;
> > u32 dll_config;
> > u32 ddr_config;
> > + struct reset_control *core_reset;
> > bool vqmmc_enabled;
> > };
> >
> > @@ -2482,6 +2484,45 @@ static inline void
> sdhci_msm_get_of_property(struct platform_device *pdev,
> > of_property_read_u32(node, "qcom,dll-config",
> > &msm_host->dll_config); }
> >
> > +static int sdhci_msm_gcc_reset(struct platform_device *pdev,
> > + struct sdhci_host *host) {
> > + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> > + struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> > + int ret = 0;
> > +
> > + msm_host->core_reset = devm_reset_control_get(&pdev->dev,
> "core_reset");
> > + if (IS_ERR(msm_host->core_reset)) {
> > + ret = PTR_ERR(msm_host->core_reset);
> > + dev_err(&pdev->dev, "core_reset unavailable (%d)\n", ret);
> > + msm_host->core_reset = NULL;
>
> Looks like we should use devm_reset_control_get_optional_exclusive()
> instead.
Sure I will incorporate the changes in next patch set.
>
> > + }
> > + if (msm_host->core_reset) {
> > + ret = reset_control_assert(msm_host->core_reset);
> > + if (ret) {
> > + dev_err(&pdev->dev, "core_reset assert failed (%d)\n",
> > + ret);
> > + goto out;
> > + }
> > + /*
> > + * The hardware requirement for delay between assert/deassert
> > + * is at least 3-4 sleep clock (32.7KHz) cycles, which comes to
> > + * ~125us (4/32768). To be on the safe side add 200us delay.
> > + */
> > + usleep_range(200, 210);
>
> Isn't this supposed to be taken care of by the reset driver?
>
> Or is this more an mmc controller specific thing? In that case, could this delay
> vary, depending on the variant of the controller?
Yes, it is specific to QCOM SDHC controller.
>
> > +
> > + ret = reset_control_deassert(msm_host->core_reset);
> > + if (ret) {
> > + dev_err(&pdev->dev, "core_reset deassert failed (%d)\n",
> > + ret);
> > + goto out;
> > + }
> > + usleep_range(200, 210);
>
> Ditto?
Same as above
>
> > + }
> > +
> > +out:
> > + return ret;
> > +}
> >
> > static int sdhci_msm_probe(struct platform_device *pdev) { @@
> > -2529,6 +2570,13 @@ static int sdhci_msm_probe(struct platform_device
> > *pdev)
> >
> > msm_host->saved_tuning_phase = INVALID_TUNING_PHASE;
> >
> > + ret = sdhci_msm_gcc_reset(pdev, host);
> > + if (ret) {
> > + dev_err(&pdev->dev, "core_reset assert/deassert failed (%d)\n",
> > + ret);
> > + goto pltfm_free;
> > + }
> > +
> > /* Setup SDCC bus voter clock. */
> > msm_host->bus_clk = devm_clk_get(&pdev->dev, "bus");
> > if (!IS_ERR(msm_host->bus_clk)) {
>
> Kind regards
> Uffe