Re: [PATCH V2 1/1]MMC: add support of sdhci-pxa driver

[Matt Fleming <matt@console-pimps.org>]

On Tue, Sep 28, 2010 at 11:23:35PM -0400, zhangfei gao wrote:
> +
> +	pxa->clk = clk_get(dev, "PXA-SDHCLK");
> +	if (IS_ERR(pxa->clk)) {
> +		dev_err(dev, "failed to get io clock\n");
> +		ret = PTR_ERR(pxa->clk);
> +		goto out;
> +	}
> +
> +	pxa->res = request_mem_region(iomem->start, resource_size(iomem),
> +		mmc_hostname(host->mmc));
> +	if (!pxa->res) {
> +		dev_err(&pdev->dev, "cannot request region\n");
> +		ret = -EBUSY;
> +		goto out;
> +	}
> +
> +	host->ioaddr = ioremap(iomem->start, resource_size(iomem));
> +	if (!host->ioaddr) {
> +		dev_err(&pdev->dev, "failed to remap registers\n");
> +		ret = -ENOMEM;
> +		goto out;
> +	}
> +
> +	host->hw_name = "MMC";
> +	host->ops = &sdhci_pxa_ops;
> +	host->irq = irq;
> +	host->quirks = SDHCI_QUIRK_BROKEN_ADMA | SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
> +
> +	if (pdata->pxa_quirk & PXA_QUIRK_BROKEN_CARD_DETECTION)
> +		host->quirks |= SDHCI_QUIRK_BROKEN_CARD_DETECTION;

I don't think there's a good reason to define
PXA_QUIRK_BROKEN_CARD_DETECTION because its only use is to turn on
SDHCI_QUIRK_BROKEN_CARD_DETECTION. As Eric pointed out, you should
just use SDHCI_QUIRK_BROKEN_CARD_DETECTION directly. While adding new
sdhci quirks should be avoided, using the existing ones is fine :-)

> +
> +	ret = sdhci_add_host(host);
> +	if (ret) {
> +		dev_err(&pdev->dev, "failed to add host\n");
> +		goto out;
> +	}
> +
> +	if (pxa->pdata->max_speed)
> +		host->mmc->f_max = pxa->pdata->max_speed;
> +
> +	platform_set_drvdata(pdev, host);
> +
> +	return 0;
> +out:
> +	if (host) {
> +		if (host->ioaddr)
> +			iounmap(host->ioaddr);
> +		if (pxa->res)
> +			release_mem_region(pxa->res->start,
> +					resource_size(pxa->res));
> +		sdhci_free_host(host);
> +	}

Aren't you missing a clk_put() here?