Hi Olof,
looks good to me. Just some minor comments, but in general
Reviewed-by: Wolfram Sang <w.sang@xxxxxxxxxxxxxx>
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 <y@xxxxxxxx>
>
> The rest, which has been rewritten by now:
> Signed-off-by: Olof Johansson <olof@xxxxxxxxx>
> Cc: Wolfram Sang <w.sang@xxxxxxxxxxxxxx>
> Cc: Mike Rapoport <mike@xxxxxxxxxxxxxx>
> +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/ |
Attachment:
signature.asc
Description: Digital signature
