* [RESEND RFC 0/3] mmc: sdhci: sdhci-msm: Add more debug logs
@ 2017-01-10 6:41 Ritesh Harjani
2017-01-10 6:41 ` [RESEND RFC 1/3] mmc: sdhci: Add platform_dumpregs callback support to sdhci_ops Ritesh Harjani
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Ritesh Harjani @ 2017-01-10 6:41 UTC (permalink / raw)
To: adrian.hunter, ulf.hansson
Cc: linux-mmc, linux-kernel, shawn.lin, linux-arm-msm, georgi.djakov,
asutoshd, stummala, venkatg, pramod.gurav, jeremymc,
Ritesh Harjani
Resending this small set of patches.
Below is a small set of patches which provide useful info for
debugging issues.
Patch 1 :- This provide callback mechanism for platform drivers
to dump their necessary register info at the time of sdhci_dumpregs.
Patch 2 :- This implements this callback (->platform_dumpregs)
for sdhci-msm.
Patch 3 :- Adds more logs for data errors to aid debugging.
Ritesh Harjani (1):
mmc: sdhci: Add more debug info in case of data error
Sahitya Tummala (2):
mmc: sdhci: Add platform_dumpregs callback support to sdhci_ops.
mmc: sdhci-msm: Implement platform_dumpregs callback in sdhci-msm
drivers/mmc/host/sdhci-msm.c | 34 ++++++++++++++++++++++++++++++++++
drivers/mmc/host/sdhci.c | 24 ++++++++++++++++++++++--
drivers/mmc/host/sdhci.h | 1 +
3 files changed, 57 insertions(+), 2 deletions(-)
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project.
^ permalink raw reply [flat|nested] 12+ messages in thread* [RESEND RFC 1/3] mmc: sdhci: Add platform_dumpregs callback support to sdhci_ops. 2017-01-10 6:41 [RESEND RFC 0/3] mmc: sdhci: sdhci-msm: Add more debug logs Ritesh Harjani @ 2017-01-10 6:41 ` Ritesh Harjani 2017-01-10 9:15 ` Shawn Lin 2017-01-10 6:41 ` [RESEND RFC 2/3] mmc: sdhci-msm: Implement platform_dumpregs callback in sdhci-msm Ritesh Harjani 2017-01-10 6:41 ` [RESEND RFC 3/3] mmc: sdhci: Add more debug info in case of data error Ritesh Harjani 2 siblings, 1 reply; 12+ messages in thread From: Ritesh Harjani @ 2017-01-10 6:41 UTC (permalink / raw) To: adrian.hunter, ulf.hansson Cc: linux-mmc, linux-kernel, shawn.lin, linux-arm-msm, georgi.djakov, asutoshd, stummala, venkatg, pramod.gurav, jeremymc, Ritesh Harjani From: Sahitya Tummala <stummala@codeaurora.org> Add new host operation ->platform_dumpregs to provide a mechanism through which host drivers can dump platform specific registers in addition to SDHC registers during error conditions. Signed-off-by: Sahitya Tummala <stummala@codeaurora.org> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org> --- drivers/mmc/host/sdhci.c | 3 +++ drivers/mmc/host/sdhci.h | 1 + 2 files changed, 4 insertions(+) diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index 2390980..73a8918 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -101,6 +101,9 @@ static void sdhci_dumpregs(struct sdhci_host *host) readl(host->ioaddr + SDHCI_ADMA_ADDRESS)); } + if (host->ops->platform_dumpregs) + host->ops->platform_dumpregs(host); + pr_err(DRIVER_NAME ": ===========================================\n"); } diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h index 0b66f21..400f3a1 100644 --- a/drivers/mmc/host/sdhci.h +++ b/drivers/mmc/host/sdhci.h @@ -564,6 +564,7 @@ struct sdhci_ops { struct mmc_card *card, unsigned int max_dtr, int host_drv, int card_drv, int *drv_type); + void (*platform_dumpregs)(struct sdhci_host *host); }; #ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [RESEND RFC 1/3] mmc: sdhci: Add platform_dumpregs callback support to sdhci_ops. 2017-01-10 6:41 ` [RESEND RFC 1/3] mmc: sdhci: Add platform_dumpregs callback support to sdhci_ops Ritesh Harjani @ 2017-01-10 9:15 ` Shawn Lin 2017-01-18 7:57 ` Ritesh Harjani 0 siblings, 1 reply; 12+ messages in thread From: Shawn Lin @ 2017-01-10 9:15 UTC (permalink / raw) To: Ritesh Harjani, adrian.hunter, ulf.hansson Cc: shawn.lin, linux-mmc, linux-kernel, linux-arm-msm, georgi.djakov, asutoshd, stummala, venkatg, pramod.gurav, jeremymc On 2017/1/10 14:41, Ritesh Harjani wrote: > From: Sahitya Tummala <stummala@codeaurora.org> > > Add new host operation ->platform_dumpregs to provide a > mechanism through which host drivers can dump platform > specific registers in addition to SDHC registers > during error conditions. > Although we have been preventing from adding new callback for sdhci core, this one makes sense as there are more and more vendor registers outside the scope of SDHCI spec. > Signed-off-by: Sahitya Tummala <stummala@codeaurora.org> > Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org> > --- > drivers/mmc/host/sdhci.c | 3 +++ > drivers/mmc/host/sdhci.h | 1 + > 2 files changed, 4 insertions(+) > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c > index 2390980..73a8918 100644 > --- a/drivers/mmc/host/sdhci.c > +++ b/drivers/mmc/host/sdhci.c > @@ -101,6 +101,9 @@ static void sdhci_dumpregs(struct sdhci_host *host) > readl(host->ioaddr + SDHCI_ADMA_ADDRESS)); > } > > + if (host->ops->platform_dumpregs) > + host->ops->platform_dumpregs(host); > + > pr_err(DRIVER_NAME ": ===========================================\n"); > } > > diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h > index 0b66f21..400f3a1 100644 > --- a/drivers/mmc/host/sdhci.h > +++ b/drivers/mmc/host/sdhci.h > @@ -564,6 +564,7 @@ struct sdhci_ops { > struct mmc_card *card, > unsigned int max_dtr, int host_drv, > int card_drv, int *drv_type); > + void (*platform_dumpregs)(struct sdhci_host *host); > }; > > #ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS > -- Best Regards Shawn Lin ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RESEND RFC 1/3] mmc: sdhci: Add platform_dumpregs callback support to sdhci_ops. 2017-01-10 9:15 ` Shawn Lin @ 2017-01-18 7:57 ` Ritesh Harjani 2017-01-19 10:43 ` Adrian Hunter 0 siblings, 1 reply; 12+ messages in thread From: Ritesh Harjani @ 2017-01-18 7:57 UTC (permalink / raw) To: Shawn Lin, adrian.hunter, ulf.hansson Cc: linux-mmc, linux-kernel, linux-arm-msm, georgi.djakov, asutoshd, stummala, venkatg, pramod.gurav, jeremymc Hi Shawn, On 1/10/2017 2:45 PM, Shawn Lin wrote: > On 2017/1/10 14:41, Ritesh Harjani wrote: >> From: Sahitya Tummala <stummala@codeaurora.org> >> >> Add new host operation ->platform_dumpregs to provide a >> mechanism through which host drivers can dump platform >> specific registers in addition to SDHC registers >> during error conditions. >> > > Although we have been preventing from adding new callback > for sdhci core, this one makes sense as there are more and more > vendor registers outside the scope of SDHCI spec. Sure thanks, shall I add your Reviewed-by on this patch 1/3. Would you be able to review other as well? -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RESEND RFC 1/3] mmc: sdhci: Add platform_dumpregs callback support to sdhci_ops. 2017-01-18 7:57 ` Ritesh Harjani @ 2017-01-19 10:43 ` Adrian Hunter 2017-01-20 5:59 ` Ritesh Harjani 0 siblings, 1 reply; 12+ messages in thread From: Adrian Hunter @ 2017-01-19 10:43 UTC (permalink / raw) To: Ritesh Harjani, Shawn Lin, ulf.hansson Cc: linux-mmc, linux-kernel, linux-arm-msm, georgi.djakov, asutoshd, stummala, venkatg, pramod.gurav, jeremymc On 18/01/17 09:57, Ritesh Harjani wrote: > Hi Shawn, > > On 1/10/2017 2:45 PM, Shawn Lin wrote: >> On 2017/1/10 14:41, Ritesh Harjani wrote: >>> From: Sahitya Tummala <stummala@codeaurora.org> >>> >>> Add new host operation ->platform_dumpregs to provide a >>> mechanism through which host drivers can dump platform >>> specific registers in addition to SDHC registers >>> during error conditions. >>> >> >> Although we have been preventing from adding new callback >> for sdhci core, this one makes sense as there are more and more >> vendor registers outside the scope of SDHCI spec. We are not prevented from adding new callbacks, but they have to represent logical functions not quirks. This patch seems fine to me except the name "platform_dumpregs" because "platform" doesn't mean "platform" here. Just plain "dumpregs" is better. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RESEND RFC 1/3] mmc: sdhci: Add platform_dumpregs callback support to sdhci_ops. 2017-01-19 10:43 ` Adrian Hunter @ 2017-01-20 5:59 ` Ritesh Harjani 0 siblings, 0 replies; 12+ messages in thread From: Ritesh Harjani @ 2017-01-20 5:59 UTC (permalink / raw) To: Adrian Hunter Cc: Shawn Lin, ulf.hansson, linux-mmc, linux-kernel, linux-arm-msm, georgi.djakov, asutoshd, stummala, venkatg, pramod.gurav, jeremymc On 1/19/2017 4:13 PM, Adrian Hunter wrote: > On 18/01/17 09:57, Ritesh Harjani wrote: >> Hi Shawn, >> >> On 1/10/2017 2:45 PM, Shawn Lin wrote: >>> On 2017/1/10 14:41, Ritesh Harjani wrote: >>>> From: Sahitya Tummala <stummala@codeaurora.org> >>>> >>>> Add new host operation ->platform_dumpregs to provide a >>>> mechanism through which host drivers can dump platform >>>> specific registers in addition to SDHC registers >>>> during error conditions. >>>> >>> >>> Although we have been preventing from adding new callback >>> for sdhci core, this one makes sense as there are more and more >>> vendor registers outside the scope of SDHCI spec. > > We are not prevented from adding new callbacks, but they have to represent > logical functions not quirks. > > This patch seems fine to me except the name "platform_dumpregs" because I took platform prefix similar to platform_execute_tuning. > "platform" doesn't mean "platform" here. Just plain "dumpregs" is better. Sure, I will only keep "dumpregs" in next revision. > > -- > To unsubscribe from this list: send the line "unsubscribe linux-mmc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 12+ messages in thread
* [RESEND RFC 2/3] mmc: sdhci-msm: Implement platform_dumpregs callback in sdhci-msm 2017-01-10 6:41 [RESEND RFC 0/3] mmc: sdhci: sdhci-msm: Add more debug logs Ritesh Harjani 2017-01-10 6:41 ` [RESEND RFC 1/3] mmc: sdhci: Add platform_dumpregs callback support to sdhci_ops Ritesh Harjani @ 2017-01-10 6:41 ` Ritesh Harjani 2017-01-19 10:55 ` Adrian Hunter 2017-01-10 6:41 ` [RESEND RFC 3/3] mmc: sdhci: Add more debug info in case of data error Ritesh Harjani 2 siblings, 1 reply; 12+ messages in thread From: Ritesh Harjani @ 2017-01-10 6:41 UTC (permalink / raw) To: adrian.hunter, ulf.hansson Cc: linux-mmc, linux-kernel, shawn.lin, linux-arm-msm, georgi.djakov, asutoshd, stummala, venkatg, pramod.gurav, jeremymc, Ritesh Harjani From: Sahitya Tummala <stummala@codeaurora.org> Implement ->platform_dumpregs host operation to print the platform specific registers in addition to standard SDHC register during error conditions. Signed-off-by: Sahitya Tummala <stummala@codeaurora.org> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org> --- drivers/mmc/host/sdhci-msm.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c index 32879b8..1241dbd 100644 --- a/drivers/mmc/host/sdhci-msm.c +++ b/drivers/mmc/host/sdhci-msm.c @@ -29,6 +29,11 @@ #define CORE_VERSION_MAJOR_MASK (0xf << CORE_VERSION_MAJOR_SHIFT) #define CORE_VERSION_MINOR_MASK 0xff +#define CORE_MCI_DATA_CNT 0x30 +#define CORE_MCI_STATUS 0x34 +#define CORE_MCI_FIFO_CNT 0x44 +#define CORE_MCI_STATUS2 0x6c + #define CORE_HC_MODE 0x78 #define HC_MODE_EN 0x1 #define CORE_POWER 0x0 @@ -77,6 +82,10 @@ #define CORE_HC_SELECT_IN_HS400 (6 << 19) #define CORE_HC_SELECT_IN_MASK (7 << 19) +#define CORE_VENDOR_SPEC_FUNC2 0x110 +#define CORE_VENDOR_SPEC_ADMA_ERR_ADDR0 0x114 +#define CORE_VENDOR_SPEC_ADMA_ERR_ADDR1 0x118 + #define CORE_CSR_CDC_CTLR_CFG0 0x130 #define CORE_SW_TRIG_FULL_CALIB BIT(16) #define CORE_HW_AUTOCAL_ENA BIT(17) @@ -658,6 +667,30 @@ static int sdhci_msm_hs400_dll_calibration(struct sdhci_host *host) return ret; } +static void sdhci_msm_dumpregs(struct sdhci_host *host) +{ + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); + struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host); + + pr_err("----------- PLATFORM REGISTER DUMP -----------\n"); + + pr_err("Data cnt: 0x%08x | Fifo cnt: 0x%08x | Int sts: 0x%08x | Int sts2: 0x%08x\n", + readl_relaxed(msm_host->core_mem + CORE_MCI_DATA_CNT), + readl_relaxed(msm_host->core_mem + CORE_MCI_FIFO_CNT), + readl_relaxed(msm_host->core_mem + CORE_MCI_STATUS), + readl_relaxed(msm_host->core_mem + CORE_MCI_STATUS2)); + pr_err("DLL cfg: 0x%08x | DLL sts: 0x%08x | SDCC ver: 0x%08x\n", + readl_relaxed(host->ioaddr + CORE_DLL_CONFIG), + readl_relaxed(host->ioaddr + CORE_DLL_STATUS), + readl_relaxed(msm_host->core_mem + CORE_MCI_VERSION)); + pr_err("Vndr func: 0x%08x | Vndr adma err : addr0: 0x%08x addr1: 0x%08x\n", + readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC), + readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC_ADMA_ERR_ADDR0), + readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC_ADMA_ERR_ADDR1)); + pr_err("Vndr func2: 0x%08x\n", + readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC_FUNC2)); +} + static int sdhci_msm_execute_tuning(struct sdhci_host *host, u32 opcode) { int tuning_seq_cnt = 3; @@ -1035,6 +1068,7 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock) .set_bus_width = sdhci_set_bus_width, .set_uhs_signaling = sdhci_msm_set_uhs_signaling, .voltage_switch = sdhci_msm_voltage_switch, + .platform_dumpregs = sdhci_msm_dumpregs, }; static const struct sdhci_pltfm_data sdhci_msm_pdata = { -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [RESEND RFC 2/3] mmc: sdhci-msm: Implement platform_dumpregs callback in sdhci-msm 2017-01-10 6:41 ` [RESEND RFC 2/3] mmc: sdhci-msm: Implement platform_dumpregs callback in sdhci-msm Ritesh Harjani @ 2017-01-19 10:55 ` Adrian Hunter 2017-01-20 5:54 ` Ritesh Harjani 0 siblings, 1 reply; 12+ messages in thread From: Adrian Hunter @ 2017-01-19 10:55 UTC (permalink / raw) To: Ritesh Harjani, ulf.hansson Cc: linux-mmc, linux-kernel, shawn.lin, linux-arm-msm, georgi.djakov, asutoshd, stummala, venkatg, pramod.gurav, jeremymc On 10/01/17 08:41, Ritesh Harjani wrote: > From: Sahitya Tummala <stummala@codeaurora.org> > > Implement ->platform_dumpregs host operation to print the > platform specific registers in addition to standard SDHC > register during error conditions. You could add an example of the prints so we can see it looks nice. > > Signed-off-by: Sahitya Tummala <stummala@codeaurora.org> > Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org> > --- > drivers/mmc/host/sdhci-msm.c | 34 ++++++++++++++++++++++++++++++++++ > 1 file changed, 34 insertions(+) > > diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c > index 32879b8..1241dbd 100644 > --- a/drivers/mmc/host/sdhci-msm.c > +++ b/drivers/mmc/host/sdhci-msm.c > @@ -29,6 +29,11 @@ > #define CORE_VERSION_MAJOR_MASK (0xf << CORE_VERSION_MAJOR_SHIFT) > #define CORE_VERSION_MINOR_MASK 0xff > > +#define CORE_MCI_DATA_CNT 0x30 > +#define CORE_MCI_STATUS 0x34 > +#define CORE_MCI_FIFO_CNT 0x44 > +#define CORE_MCI_STATUS2 0x6c > + > #define CORE_HC_MODE 0x78 > #define HC_MODE_EN 0x1 > #define CORE_POWER 0x0 > @@ -77,6 +82,10 @@ > #define CORE_HC_SELECT_IN_HS400 (6 << 19) > #define CORE_HC_SELECT_IN_MASK (7 << 19) > > +#define CORE_VENDOR_SPEC_FUNC2 0x110 > +#define CORE_VENDOR_SPEC_ADMA_ERR_ADDR0 0x114 > +#define CORE_VENDOR_SPEC_ADMA_ERR_ADDR1 0x118 > + > #define CORE_CSR_CDC_CTLR_CFG0 0x130 > #define CORE_SW_TRIG_FULL_CALIB BIT(16) > #define CORE_HW_AUTOCAL_ENA BIT(17) > @@ -658,6 +667,30 @@ static int sdhci_msm_hs400_dll_calibration(struct sdhci_host *host) > return ret; > } > > +static void sdhci_msm_dumpregs(struct sdhci_host *host) > +{ > + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); > + struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host); > + > + pr_err("----------- PLATFORM REGISTER DUMP -----------\n"); "PLATFORM" isn't right here. It should be something that identifies the IP / driver. e.g. "SDHCI MSM" > + > + pr_err("Data cnt: 0x%08x | Fifo cnt: 0x%08x | Int sts: 0x%08x | Int sts2: 0x%08x\n", What sdhci_dumpregs() should do is prefix the message with the host name i.e. pr_err("%s: sdhci: Sys addr: 0x%08x | Version: 0x%08x\n", sdhci_readl(host, SDHCI_DMA_ADDRESS), sdhci_readw(host, SDHCI_HOST_VERSION), mmc_hostname(host->mmc)); And then we should do that here too. > + readl_relaxed(msm_host->core_mem + CORE_MCI_DATA_CNT), > + readl_relaxed(msm_host->core_mem + CORE_MCI_FIFO_CNT), > + readl_relaxed(msm_host->core_mem + CORE_MCI_STATUS), > + readl_relaxed(msm_host->core_mem + CORE_MCI_STATUS2)); > + pr_err("DLL cfg: 0x%08x | DLL sts: 0x%08x | SDCC ver: 0x%08x\n", > + readl_relaxed(host->ioaddr + CORE_DLL_CONFIG), > + readl_relaxed(host->ioaddr + CORE_DLL_STATUS), > + readl_relaxed(msm_host->core_mem + CORE_MCI_VERSION)); > + pr_err("Vndr func: 0x%08x | Vndr adma err : addr0: 0x%08x addr1: 0x%08x\n", > + readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC), > + readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC_ADMA_ERR_ADDR0), > + readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC_ADMA_ERR_ADDR1)); > + pr_err("Vndr func2: 0x%08x\n", > + readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC_FUNC2)); > +} > + > static int sdhci_msm_execute_tuning(struct sdhci_host *host, u32 opcode) > { > int tuning_seq_cnt = 3; > @@ -1035,6 +1068,7 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock) > .set_bus_width = sdhci_set_bus_width, > .set_uhs_signaling = sdhci_msm_set_uhs_signaling, > .voltage_switch = sdhci_msm_voltage_switch, > + .platform_dumpregs = sdhci_msm_dumpregs, > }; > > static const struct sdhci_pltfm_data sdhci_msm_pdata = { > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RESEND RFC 2/3] mmc: sdhci-msm: Implement platform_dumpregs callback in sdhci-msm 2017-01-19 10:55 ` Adrian Hunter @ 2017-01-20 5:54 ` Ritesh Harjani 0 siblings, 0 replies; 12+ messages in thread From: Ritesh Harjani @ 2017-01-20 5:54 UTC (permalink / raw) To: Adrian Hunter Cc: ulf.hansson, linux-mmc, linux-kernel, shawn.lin, linux-arm-msm, georgi.djakov, asutoshd, stummala, venkatg, pramod.gurav, jeremymc Hi Adrian, On 1/19/2017 4:25 PM, Adrian Hunter wrote: > On 10/01/17 08:41, Ritesh Harjani wrote: >> From: Sahitya Tummala <stummala@codeaurora.org> >> >> Implement ->platform_dumpregs host operation to print the >> platform specific registers in addition to standard SDHC >> register during error conditions. > > You could add an example of the prints so we can see it looks nice. Sure. Below is an example, taken during bootup. That's why you see all registers as 0. http://pastebin.com/7j94punq > >> >> Signed-off-by: Sahitya Tummala <stummala@codeaurora.org> >> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org> >> --- >> drivers/mmc/host/sdhci-msm.c | 34 ++++++++++++++++++++++++++++++++++ >> 1 file changed, 34 insertions(+) >> >> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c >> index 32879b8..1241dbd 100644 >> --- a/drivers/mmc/host/sdhci-msm.c >> +++ b/drivers/mmc/host/sdhci-msm.c >> @@ -29,6 +29,11 @@ >> #define CORE_VERSION_MAJOR_MASK (0xf << CORE_VERSION_MAJOR_SHIFT) >> #define CORE_VERSION_MINOR_MASK 0xff >> >> +#define CORE_MCI_DATA_CNT 0x30 >> +#define CORE_MCI_STATUS 0x34 >> +#define CORE_MCI_FIFO_CNT 0x44 >> +#define CORE_MCI_STATUS2 0x6c >> + >> #define CORE_HC_MODE 0x78 >> #define HC_MODE_EN 0x1 >> #define CORE_POWER 0x0 >> @@ -77,6 +82,10 @@ >> #define CORE_HC_SELECT_IN_HS400 (6 << 19) >> #define CORE_HC_SELECT_IN_MASK (7 << 19) >> >> +#define CORE_VENDOR_SPEC_FUNC2 0x110 >> +#define CORE_VENDOR_SPEC_ADMA_ERR_ADDR0 0x114 >> +#define CORE_VENDOR_SPEC_ADMA_ERR_ADDR1 0x118 >> + >> #define CORE_CSR_CDC_CTLR_CFG0 0x130 >> #define CORE_SW_TRIG_FULL_CALIB BIT(16) >> #define CORE_HW_AUTOCAL_ENA BIT(17) >> @@ -658,6 +667,30 @@ static int sdhci_msm_hs400_dll_calibration(struct sdhci_host *host) >> return ret; >> } >> >> +static void sdhci_msm_dumpregs(struct sdhci_host *host) >> +{ >> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); >> + struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host); >> + >> + pr_err("----------- PLATFORM REGISTER DUMP -----------\n"); > > "PLATFORM" isn't right here. It should be something that identifies the IP > / driver. e.g. "SDHCI MSM" Sure. > >> + >> + pr_err("Data cnt: 0x%08x | Fifo cnt: 0x%08x | Int sts: 0x%08x | Int sts2: 0x%08x\n", > > What sdhci_dumpregs() should do is prefix the message with the host name i.e. I feel it may be redundant info. As all sdhci-msm register dump with come with mmc0/mmc1 prefixed with it. Instead I see sdhci_dumpregs already adds hostname in the starting header of register dump. So this may not be required I guess. I have added an example dumpregs (http://pastebin.com/7j94punq). Please let me know if you still think otherwise. Regards Ritesh > > pr_err("%s: sdhci: Sys addr: 0x%08x | Version: 0x%08x\n", > sdhci_readl(host, SDHCI_DMA_ADDRESS), > sdhci_readw(host, SDHCI_HOST_VERSION), > mmc_hostname(host->mmc)); > > And then we should do that here too. > >> + readl_relaxed(msm_host->core_mem + CORE_MCI_DATA_CNT), >> + readl_relaxed(msm_host->core_mem + CORE_MCI_FIFO_CNT), >> + readl_relaxed(msm_host->core_mem + CORE_MCI_STATUS), >> + readl_relaxed(msm_host->core_mem + CORE_MCI_STATUS2)); >> + pr_err("DLL cfg: 0x%08x | DLL sts: 0x%08x | SDCC ver: 0x%08x\n", >> + readl_relaxed(host->ioaddr + CORE_DLL_CONFIG), >> + readl_relaxed(host->ioaddr + CORE_DLL_STATUS), >> + readl_relaxed(msm_host->core_mem + CORE_MCI_VERSION)); >> + pr_err("Vndr func: 0x%08x | Vndr adma err : addr0: 0x%08x addr1: 0x%08x\n", >> + readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC), >> + readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC_ADMA_ERR_ADDR0), >> + readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC_ADMA_ERR_ADDR1)); >> + pr_err("Vndr func2: 0x%08x\n", >> + readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC_FUNC2)); >> +} >> + >> static int sdhci_msm_execute_tuning(struct sdhci_host *host, u32 opcode) >> { >> int tuning_seq_cnt = 3; >> @@ -1035,6 +1068,7 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock) >> .set_bus_width = sdhci_set_bus_width, >> .set_uhs_signaling = sdhci_msm_set_uhs_signaling, >> .voltage_switch = sdhci_msm_voltage_switch, >> + .platform_dumpregs = sdhci_msm_dumpregs, >> }; >> >> static const struct sdhci_pltfm_data sdhci_msm_pdata = { >> > > -- > To unsubscribe from this list: send the line "unsubscribe linux-mmc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 12+ messages in thread
* [RESEND RFC 3/3] mmc: sdhci: Add more debug info in case of data error 2017-01-10 6:41 [RESEND RFC 0/3] mmc: sdhci: sdhci-msm: Add more debug logs Ritesh Harjani 2017-01-10 6:41 ` [RESEND RFC 1/3] mmc: sdhci: Add platform_dumpregs callback support to sdhci_ops Ritesh Harjani 2017-01-10 6:41 ` [RESEND RFC 2/3] mmc: sdhci-msm: Implement platform_dumpregs callback in sdhci-msm Ritesh Harjani @ 2017-01-10 6:41 ` Ritesh Harjani 2017-01-19 11:54 ` Adrian Hunter 2 siblings, 1 reply; 12+ messages in thread From: Ritesh Harjani @ 2017-01-10 6:41 UTC (permalink / raw) To: adrian.hunter, ulf.hansson Cc: linux-mmc, linux-kernel, shawn.lin, linux-arm-msm, georgi.djakov, asutoshd, stummala, venkatg, pramod.gurav, jeremymc, Ritesh Harjani print error log message and dump sdhc registers for debugging purpose in case of data errors (except when tuning commands generate CRC/timeout/end bit errors). Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org> --- drivers/mmc/host/sdhci.c | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index 73a8918..2e51e49 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -2616,9 +2616,26 @@ static void sdhci_data_irq(struct sdhci_host *host, u32 intmask) host->ops->adma_workaround(host, intmask); } - if (host->data->error) + if (host->data->error) { + bool pr_msg = true; + + if (intmask & (SDHCI_INT_DATA_CRC | SDHCI_INT_DATA_TIMEOUT | + SDHCI_INT_DATA_END_BIT)) { + command = SDHCI_GET_CMD(sdhci_readw(host, + SDHCI_COMMAND)); + if (command == MMC_SEND_TUNING_BLOCK_HS200 || + command == MMC_SEND_TUNING_BLOCK) + pr_msg = false; + } + if (pr_msg) { + pr_err("%s: data txfr (0x%08x) error: %d\n", + mmc_hostname(host->mmc), intmask, + host->data->error); + sdhci_dumpregs(host); + } + sdhci_finish_data(host); - else { + } else { if (intmask & (SDHCI_INT_DATA_AVAIL | SDHCI_INT_SPACE_AVAIL)) sdhci_transfer_pio(host); -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [RESEND RFC 3/3] mmc: sdhci: Add more debug info in case of data error 2017-01-10 6:41 ` [RESEND RFC 3/3] mmc: sdhci: Add more debug info in case of data error Ritesh Harjani @ 2017-01-19 11:54 ` Adrian Hunter 2017-01-20 6:26 ` Ritesh Harjani 0 siblings, 1 reply; 12+ messages in thread From: Adrian Hunter @ 2017-01-19 11:54 UTC (permalink / raw) To: Ritesh Harjani, ulf.hansson Cc: linux-mmc, linux-kernel, shawn.lin, linux-arm-msm, georgi.djakov, asutoshd, stummala, venkatg, pramod.gurav, jeremymc On 10/01/17 08:41, Ritesh Harjani wrote: > print error log message and dump sdhc registers for debugging > purpose in case of data errors (except when tuning commands > generate CRC/timeout/end bit errors). It is a bit ugly and probably only useful during driver development, so I am not at all enthusiastic about this. > > Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org> > Signed-off-by: Asutosh Das <asutoshd@codeaurora.org> > --- > drivers/mmc/host/sdhci.c | 21 +++++++++++++++++++-- > 1 file changed, 19 insertions(+), 2 deletions(-) > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c > index 73a8918..2e51e49 100644 > --- a/drivers/mmc/host/sdhci.c > +++ b/drivers/mmc/host/sdhci.c > @@ -2616,9 +2616,26 @@ static void sdhci_data_irq(struct sdhci_host *host, u32 intmask) > host->ops->adma_workaround(host, intmask); > } > > - if (host->data->error) > + if (host->data->error) { > + bool pr_msg = true; > + > + if (intmask & (SDHCI_INT_DATA_CRC | SDHCI_INT_DATA_TIMEOUT | > + SDHCI_INT_DATA_END_BIT)) { > + command = SDHCI_GET_CMD(sdhci_readw(host, > + SDHCI_COMMAND)); > + if (command == MMC_SEND_TUNING_BLOCK_HS200 || > + command == MMC_SEND_TUNING_BLOCK) > + pr_msg = false; > + } > + if (pr_msg) { > + pr_err("%s: data txfr (0x%08x) error: %d\n", > + mmc_hostname(host->mmc), intmask, > + host->data->error); > + sdhci_dumpregs(host); > + } > + > sdhci_finish_data(host); > - else { > + } else { > if (intmask & (SDHCI_INT_DATA_AVAIL | SDHCI_INT_SPACE_AVAIL)) > sdhci_transfer_pio(host); > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RESEND RFC 3/3] mmc: sdhci: Add more debug info in case of data error 2017-01-19 11:54 ` Adrian Hunter @ 2017-01-20 6:26 ` Ritesh Harjani 0 siblings, 0 replies; 12+ messages in thread From: Ritesh Harjani @ 2017-01-20 6:26 UTC (permalink / raw) To: Adrian Hunter Cc: ulf.hansson, linux-mmc, linux-kernel, shawn.lin, linux-arm-msm, georgi.djakov, asutoshd, stummala, venkatg, pramod.gurav, jeremymc Hi Adrian, Thanks for reviewing. On 1/19/2017 5:24 PM, Adrian Hunter wrote: > On 10/01/17 08:41, Ritesh Harjani wrote: >> print error log message and dump sdhc registers for debugging >> purpose in case of data errors (except when tuning commands >> generate CRC/timeout/end bit errors). > > It is a bit ugly and probably only useful during driver development, so I am > not at all enthusiastic about this. Ok, since this patch was anyway for debugging, I will drop this patch for now. I will revisit in case if the need really arises, since this could be useful in debugging data txfr error and avoid re-test cycle to get sdhci register dumps. Also, Do you think we should have this below change added? Could you please give your thoughts on this. Jeremy and I feel that even below info can be helpful along with sdhci_dumpregs - He requested me to take below diff as another patch in my next revision of this series which he has submitted here - https://patchwork.kernel.org/patch/9442449/ diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index 71654b9..5911f98 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -47,6 +47,27 @@ static void sdhci_finish_data(struct sdhci_host *); static void sdhci_enable_preset_value(struct sdhci_host *host, bool enable); +static void sdhci_dump_rpm_info(struct sdhci_host *host) +{ + struct mmc_host *mmc = host->mmc; + + pr_info("%s: rpmstatus[pltfm](runtime-suspend:usage_count:disable_depth)(%d:%d:%d)\n", + mmc_hostname(mmc), mmc->parent->power.runtime_status, + atomic_read(&mmc->parent->power.usage_count), + mmc->parent->power.disable_depth); +} + + +static void sdhci_dump_state(struct sdhci_host *host) +{ + struct mmc_host *mmc = host->mmc; + + pr_info("%s: clk: %d claimer: %s pwr: %d\n", + mmc_hostname(mmc), host->clock, + mmc->claimer->comm, host->pwr); + sdhci_dump_rpm_info(host); +} + static void sdhci_dumpregs(struct sdhci_host *host) { pr_err(DRIVER_NAME ": =========== REGISTER DUMP (%s)===========\n", @@ -100,6 +121,10 @@ static void sdhci_dumpregs(struct sdhci_host *host) readl(host->ioaddr + SDHCI_ADMA_ADDRESS)); } + if (host->ops->dump_vendor_regs) + host->ops->dump_vendor_regs(host); + + sdhci_dump_state(host); pr_err(DRIVER_NAME ": ===========================================\n"); } Regards Ritesh > >> >> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org> >> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org> >> --- >> drivers/mmc/host/sdhci.c | 21 +++++++++++++++++++-- >> 1 file changed, 19 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c >> index 73a8918..2e51e49 100644 >> --- a/drivers/mmc/host/sdhci.c >> +++ b/drivers/mmc/host/sdhci.c >> @@ -2616,9 +2616,26 @@ static void sdhci_data_irq(struct sdhci_host *host, u32 intmask) >> host->ops->adma_workaround(host, intmask); >> } >> >> - if (host->data->error) >> + if (host->data->error) { >> + bool pr_msg = true; >> + >> + if (intmask & (SDHCI_INT_DATA_CRC | SDHCI_INT_DATA_TIMEOUT | >> + SDHCI_INT_DATA_END_BIT)) { >> + command = SDHCI_GET_CMD(sdhci_readw(host, >> + SDHCI_COMMAND)); >> + if (command == MMC_SEND_TUNING_BLOCK_HS200 || >> + command == MMC_SEND_TUNING_BLOCK) >> + pr_msg = false; >> + } >> + if (pr_msg) { >> + pr_err("%s: data txfr (0x%08x) error: %d\n", >> + mmc_hostname(host->mmc), intmask, >> + host->data->error); >> + sdhci_dumpregs(host); >> + } >> + >> sdhci_finish_data(host); >> - else { >> + } else { >> if (intmask & (SDHCI_INT_DATA_AVAIL | SDHCI_INT_SPACE_AVAIL)) >> sdhci_transfer_pio(host); >> >> > -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply related [flat|nested] 12+ messages in thread
end of thread, other threads:[~2017-01-20 6:26 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-01-10 6:41 [RESEND RFC 0/3] mmc: sdhci: sdhci-msm: Add more debug logs Ritesh Harjani 2017-01-10 6:41 ` [RESEND RFC 1/3] mmc: sdhci: Add platform_dumpregs callback support to sdhci_ops Ritesh Harjani 2017-01-10 9:15 ` Shawn Lin 2017-01-18 7:57 ` Ritesh Harjani 2017-01-19 10:43 ` Adrian Hunter 2017-01-20 5:59 ` Ritesh Harjani 2017-01-10 6:41 ` [RESEND RFC 2/3] mmc: sdhci-msm: Implement platform_dumpregs callback in sdhci-msm Ritesh Harjani 2017-01-19 10:55 ` Adrian Hunter 2017-01-20 5:54 ` Ritesh Harjani 2017-01-10 6:41 ` [RESEND RFC 3/3] mmc: sdhci: Add more debug info in case of data error Ritesh Harjani 2017-01-19 11:54 ` Adrian Hunter 2017-01-20 6:26 ` Ritesh Harjani
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.