On 19 May 2015 at 10:59, Lu Y.B. <yangbo.lu@xxxxxxxxxxxxx> wrote: >> From: Ulf Hansson [mailto:ulf.hansson@xxxxxxxxxx] >> Sent: Tuesday, May 19, 2015 4:37 PM >> To: Lu Yangbo-B47093 >> Cc: linux-mmc; Chris Ball; Jaehoon Chung >> Subject: Re: [PATCH v2] mmc: sdhci-of-esdhc: add workaround for clock >> glitch issue >> >> On 19 May 2015 at 07:40, Yangbo Lu <yangbo.lu@xxxxxxxxxxxxx> wrote: >> > A-003980: SDHC: Glitch is generated on the card clock with software >> > reset or clock divider change >> > Description: A glitch may occur on the SDHC card clock when the >> > software sets the RSTA bit (software reset) in the system control >> > register. It can also be generated by setting the clock divider value. >> > The glitch produced can cause the external card to switch to an >> > unknown state. The occurrence is not deterministic. >> > >> > Workaround: >> > A simple workaround is to disable the SD card clock before the >> > software reset, and enable it when the module resumes normal >> > operation. The Host and the SD card are in a master-slave >> > relationship. The Host provides clock and control transfer across the >> > interface. Therefore, any existing operation is discarded when the Host >> controller is reset. >> > The recommended flow is as follows: >> > 1. Software disable bit[3], SDCLKEN, of the System Control Register 2. >> > Trigger software reset and/or set clock divider 3. Check bit[3], >> > SDSTB, of the Present State Register for stable clock 4. Enable >> > bit[3], SDCLKEN, of the System Control Register Using the above >> > method, the eSDHC cannot send command or transfer data when there is a >> > glitch in the clock line, and the glitch does not cause any issue. >> > >> > Signed-off-by: Yangbo Lu <yangbo.lu@xxxxxxxxxxxxx> >> > --- >> > Changes for v2 >> > - Used sdhci_ops reset instead of platform_reset_enter/out's >> work >> > - Changed commit message >> > --- >> > drivers/mmc/host/sdhci-esdhc.h | 4 ++ >> > drivers/mmc/host/sdhci-of-esdhc.c | 90 >> > +++++++++++++++++++++++++++++++++++++-- >> > 2 files changed, 91 insertions(+), 3 deletions(-) >> > >> > diff --git a/drivers/mmc/host/sdhci-esdhc.h >> > b/drivers/mmc/host/sdhci-esdhc.h index 3497cfa..ede49f5 100644 >> > --- a/drivers/mmc/host/sdhci-esdhc.h >> > +++ b/drivers/mmc/host/sdhci-esdhc.h >> > @@ -27,10 +27,14 @@ >> > #define ESDHC_CLOCK_MASK 0x0000fff0 >> > #define ESDHC_PREDIV_SHIFT 8 >> > #define ESDHC_DIVIDER_SHIFT 4 >> > +#define ESDHC_CLOCK_CRDEN 0x00000008 >> > #define ESDHC_CLOCK_PEREN 0x00000004 >> > #define ESDHC_CLOCK_HCKEN 0x00000002 >> > #define ESDHC_CLOCK_IPGEN 0x00000001 >> > >> > +#define ESDHCI_PRESENT_STATE 0x24 >> > +#define ESDHC_CLK_STABLE 0x00000008 >> > + >> > /* pltfm-specific */ >> > #define ESDHC_HOST_CONTROL_LE 0x20 >> > >> > diff --git a/drivers/mmc/host/sdhci-of-esdhc.c >> > b/drivers/mmc/host/sdhci-of-esdhc.c >> > index 22e9111..db78b75 100644 >> > --- a/drivers/mmc/host/sdhci-of-esdhc.c >> > +++ b/drivers/mmc/host/sdhci-of-esdhc.c >> > @@ -21,9 +21,16 @@ >> > #include <linux/mmc/host.h> >> > #include "sdhci-pltfm.h" >> > #include "sdhci-esdhc.h" >> > +#ifdef CONFIG_PPC >> > +#include <asm/mpc85xx.h> >> >> As Jaehoon already pointed out, we don't want to include machine specific >> headers in drivers. Please find another solution to this. >> > Do you mean I should include this .h file in sdhci-esdhc.h, or all the PPC-specific code in this patch is not good? I assume this header is included to identify the version of the SoC? Normally we do such things through DT via compatible strings. Earlier that was done via platform callbacks and prior that as how you suggested it in $subject patch. If you have the possibility to enumerate the device ID (reading device information directly from the HW), you should do that when adding the device to its subsystem level bus, not while probing. That's because you need to be able to match for a compatible driver. Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
