Hi, anton, I have resent this patch according to your suggestion. Please review. Thanks. Best Regards Jerry Huang > -----Original Message----- > From: Anton Vorontsov [mailto:cbouatmailru@xxxxxxxxx] > Sent: Wednesday, October 24, 2012 4:39 PM > To: Huang Changming-R66093 > Cc: Chris Ball; linux-mmc@xxxxxxxxxxxxxxx; Xie Shaohui-B21989; Girish K S > Subject: Re: [PATCH v5] powerpc/esdhc: disable CMD23 for some Freescale > SoCs > > Sorry for the late reply, Huang. > > On Tue, Oct 09, 2012 at 06:24:13AM +0000, Huang Changming-R66093 wrote: > [...] > > > > +static void esdhc_of_detect_limitation(struct platform_device > *pdev, > > > > + struct sdhci_pltfm_data *pdata) { > > Wrong indentation. Should be one more tab, at least (or align to opening > brace). > > > > > + void __iomem *ioaddr; > > > > + struct resource *iomem; > > > > + u32 vvn; > > > > + > > > > + iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0); > > > > + if (!iomem) { > > > > + dev_warn(&pdev->dev, "failed to get resource\n"); > > > > + goto end; > > > > + } > > > > + if (resource_size(iomem) < 0x100) > > > > + dev_warn(&pdev->dev, "Invalid iomem size!\n"); > > > > + > > > > + ioaddr = ioremap(iomem->start, resource_size(iomem)); > > > > + if (!ioaddr) { > > > > + dev_warn(&pdev->dev, "failed to remap registers\n"); > > > > + goto end; > > > > + } > > > > + > > > > + /* P102x and P4080 has IP version VVN2.2, CMD23 is not > > > supported */ > > > > + vvn = in_be32(ioaddr + SDHCI_SLOT_INT_STATUS); > > > > + vvn = (vvn & SDHCI_VENDOR_VER_MASK) >> > SDHCI_VENDOR_VER_SHIFT; > > > > + if (vvn == VENDOR_V_22) > > > > + pdata->quirks2 |= SDHCI_QUIRK2_HOST_NO_CMD23; > > > > + > > > > + iounmap(ioaddr); > > > > +end: > > > > + return; > > No need for the 'end' label. > > > > > +} > > > > + > > > > static struct sdhci_ops sdhci_esdhc_ops = { > > > > .read_l = esdhc_readl, > > > > .read_w = esdhc_readw, > > > > @@ -199,6 +231,7 @@ static struct sdhci_pltfm_data > > > > sdhci_esdhc_pdata = { > > > > > > > > static int __devinit sdhci_esdhc_probe(struct platform_device > > > > *pdev) { > > > > + esdhc_of_detect_limitation(pdev, &sdhci_esdhc_pdata); > > I would rather prefer it to be in sdhci_ops (i.e. introduce sdhci_ops- > >platform_init), so that way you wouldn't need to ioremap() stuff by > yourself. And make drivers/mmc/host/sdhci-pltfm.c call platform_init > after ioremap(). > > Then your detect_limitation() function would only need to check revision > and set additional quirks. > > Thanks, > Anton. ��.n��������+%������w��{.n�����{��i��)��jg��������ݢj����G�������j:+v���w�m������w�������h�����٥