Hi Olof, looks good to me. Just some minor comments, but in general
Reviewed-by: Wolfram Sang <[email protected]> On Wed, Dec 22, 2010 at 02:01:11AM -0600, Olof Johansson wrote: > SDHCI driver for Tegra. Pretty straight forward, a few pieces of > functionality left to fill in but nothing that stops it from going > upstream. Board enablement submitted separately. I'd think this can go below the "---" line? ... > This driver plugs in as a new variant of sdhci-pltfm, using the platform > data structure passed in to specify the GPIOs to use for card detect, > write protect and card power enablement. > > Original driver (of which only the header file is left): > Signed-off-by: Yvonne Yip <[email protected]> > > The rest, which has been rewritten by now: > Signed-off-by: Olof Johansson <[email protected]> > Cc: Wolfram Sang <[email protected]> > Cc: Mike Rapoport <[email protected]> > +static unsigned int tegra_sdhci_get_ro(struct sdhci_host *sdhci) > +{ > + struct platform_device *pdev = to_platform_device(mmc_dev(sdhci->mmc)); > + struct tegra_sdhci_platform_data *plat; > + > + plat = pdev->dev.platform_data; > + > + if (gpio_is_valid(plat->wp_gpio)) > + return gpio_get_value(plat->wp_gpio); > + > + return -1; So, read_only is the default case? I would have gone for returning 0. Well, safety vs. convenience... ... > + if (rc) { > + dev_err(mmc_dev(host->mmc), Trailing white space > + "failed to allocate power gpio\n"); > + goto out; > + } > + tegra_gpio_enable(plat->power_gpio); > + gpio_direction_output(plat->power_gpio, 1); > + } > + > + if (gpio_is_valid(plat->cd_gpio)) { > + rc = gpio_request(plat->cd_gpio, "sdhci_cd"); > + if (rc) { > + dev_err(mmc_dev(host->mmc), ditto > + "failed to allocate cd gpio\n"); > + goto out_power; > + } > + tegra_gpio_enable(plat->cd_gpio); > + > + rc = request_irq(gpio_to_irq(plat->cd_gpio), carddetect_irq, > + IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING, > + mmc_hostname(host->mmc), host); > + > + if (rc) { > + dev_err(mmc_dev(host->mmc), "request irq error\n"); > + goto out_cd; > + } > + > + } > + > + if (gpio_is_valid(plat->wp_gpio)) { > + rc = gpio_request(plat->wp_gpio, "sdhci_wp"); > + if (rc) { > + dev_err(mmc_dev(host->mmc), ditto Kind regards, Wolfram -- Pengutronix e.K. | Wolfram Sang | Industrial Linux Solutions | http://www.pengutronix.de/ |
signature.asc
Description: Digital signature
