On Mon, Jul 18, 2011 at 11:31 AM, Zang Roy-R61911 <r61911@xxxxxxxxxxxxx> wrote: > > >> -----Original Message----- >> From: S, Venkatraman [mailto:svenkatr@xxxxxx] >> Sent: Tuesday, July 05, 2011 14:17 PM >> To: Zang Roy-R61911 >> Cc: linux-mmc; linuxppc-dev; cbouatmailru; akpm; Xu Lei-B33228; Kumar Gala >> Subject: Re: [PATCH 2/3] eSDHC: Fix errors when booting kernel with fsl esdhc >> >> On Tue, Jul 5, 2011 at 9:49 AM, Roy Zang <tie-fei.zang@xxxxxxxxxxxxx> wrote: >> > From: Xu lei <B33228@xxxxxxxxxxxxx> >> > >> > When esdhc module was enabled in p5020, there were following errors: >> > >> > mmc0: Timeout waiting for hardware interrupt. >> > mmc0: error -110 whilst initialising SD card >> > mmc0: Unexpected interrupt 0x02000000. >> > mmc0: Timeout waiting for hardware interrupt. >> > mmc0: error -110 whilst initialising SD card >> > mmc0: Unexpected interrupt 0x02000000. >> > >> > It is because ESDHC controller has different bit setting for PROCTL >> > register, when kernel sets Power Control Register by method for standard >> > SD Host Specification, it would overwritten FSL ESDHC PROCTL[DMAS]; >> > when it set Host Control Registers[DMAS], it sets PROCTL[EMODE] and >> > PROCTL[D3CD]. These operations will set bad bits for PROCTL Register >> > on FSL ESDHC Controller and cause errors, so this patch will make esdhc >> > driver access FSL PROCTL Register according to block guide instead of >> > standard SD Host Specification. >> > >> > For some FSL chips, such as MPC8536/P2020, PROCTL[VOLT_SEL] and PROCTL[DMAS] >> > bits are reserved and even if they are set to wrong bits there is no error. >> > But considering that all FSL ESDHC Controller register map is not fully >> > compliant to standard SD Host Specification, we put the patch to all of >> > FSL ESDHC Controllers. >> > >> > Signed-off-by: Lei Xu <B33228@xxxxxxxxxxxxx> >> > Signed-off-by: Roy Zang <tie-fei.zang@xxxxxxxxxxxxx> >> > Signed-off-by: Kumar Gala <galak@xxxxxxxxxxxxxxxxxxx> >> > --- >> > drivers/mmc/host/sdhci-of-core.c | 3 ++ >> > drivers/mmc/host/sdhci.c | 62 ++++++++++++++++++++++++++++++----- >> -- >> > include/linux/mmc/sdhci.h | 6 ++- >> > 3 files changed, 57 insertions(+), 14 deletions(-) > [snip] > >> > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c >> > index 58d5436..77174e5 100644 >> > --- a/drivers/mmc/host/sdhci.c >> > +++ b/drivers/mmc/host/sdhci.c >> > @@ -674,7 +674,7 @@ static void sdhci_set_transfer_irqs(struct sdhci_host >> *host) >> > static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_command >> *cmd) >> > { >> > u8 count; >> > - u8 ctrl; >> > + u32 ctrl; >> > struct mmc_data *data = cmd->data; >> > int ret; >> > >> > @@ -807,14 +807,28 @@ static void sdhci_prepare_data(struct sdhci_host *host, >> struct mmc_command *cmd) >> > * is ADMA. >> > */ >> > if (host->version >= SDHCI_SPEC_200) { >> > - ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL); >> > - ctrl &= ~SDHCI_CTRL_DMA_MASK; >> > - if ((host->flags & SDHCI_REQ_USE_DMA) && >> > - (host->flags & SDHCI_USE_ADMA)) >> > - ctrl |= SDHCI_CTRL_ADMA32; >> > - else >> > - ctrl |= SDHCI_CTRL_SDMA; >> > - sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL); >> > + if (host->quirks & SDHCI_QUIRK_QORIQ_PROCTL_WEIRD) { >> > +#define ESDHCI_PROCTL_DMAS_MASK 0x00000300 >> > +#define ESDHCI_PROCTL_ADMA32 0x00000200 >> > +#define ESDHCI_PROCTL_SDMA 0x00000000 >> >> Breaks the code flow / readability. Can be moved to top of the file ? > The defines are only used in the following section. Why it will break > the readability? > I can also see this kind of define in the file > ... > #define SAMPLE_COUNT 5 > > static int sdhci_get_ro(struct mmc_host *mmc) > ... > > Any rule should follow? > > > [snip] >> > @@ -1162,6 +1189,17 @@ static void sdhci_set_power(struct sdhci_host *host, >> unsigned short power) >> > >> > host->pwr = pwr; >> > >> > + /* Now FSL ESDHC Controller has no Bus Power bit, >> > + * and PROCTL[21] bit is for voltage selection */ >> >> Multiline comment style needed.. > Will update. > please help to explain your previous comment. > Thanks. > Roy There aren't very hard rules on this. Simple #defines are good, as a one off usage. These bit mask fields are very verbose, and they tend to grow more than a screenful. The remaining bits will never be defined ? -- 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
