Re: [PATCH v5] powerpc/esdhc: disable CMD23 for some Freescale SoCs

[Anton Vorontsov <cbouatmailru@gmail.com>]

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.