On Tue, 22 Jun 2010 17:09:02 +0300, Saeed Bishara <saeed@xxxxxxxxxxx> wrote: > This patch implements the driver for the platfrom SDHCI controllers that found on some of Marvell's SoC's. > > Signed-off-by: Saeed Bishara <saeed@xxxxxxxxxxx> That's a nice, small driver ;-) [...] > + > + if (!devm_request_mem_region(&pdev->dev, iomem->start, > + resource_size(iomem), > + mmc_hostname(host->mmc))) { > + dev_err(&pdev->dev, "cannot request region\n"); > + ret = -EBUSY; > + goto err_request; > + } > + > + host->ioaddr = devm_ioremap(&pdev->dev, iomem->start, > + resource_size(iomem)); > + if (!host->ioaddr) { > + dev_err(&pdev->dev, "failed to remap registers\n"); > + ret = -ENOMEM; > + goto err_request; > + } If you fail to remap the registers shouldn't you release the mem_region you've allocated above? > + > +#if defined(CONFIG_HAVE_CLK) > + mv_host->clk = clk_get(&pdev->dev, NULL); > + if (IS_ERR(mv_host->clk)) > + dev_notice(&pdev->dev, "cannot get clkdev\n"); > + else > + clk_enable(mv_host->clk); > +#endif > + > + ret = sdhci_add_host(host); > + if (ret) > + goto err_request; Likewise here, I think you should be iounmap()'ing the registers if this fails. Otherwise this looks good. -- 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