Hi, On Wed, Dec 15, 2010 at 01:23:58PM +0200, Mike Rapoport wrote: > Hi Olof, > > Overall looks good, just some nitpicking comments. > [...] > > +struct tegra_sdhci_platform_data { > > + int cd_gpio; > > + int wp_gpio; > > + int power_gpio; > > The power_gpio seems to be unused... Yep, will remove. > > +static unsigned int tegra_sdhci_get_ro(struct sdhci_host *sdhci) > > +{ > > + struct tegra_sdhci_host *host = sdhci_priv(sdhci); > > + > > + if (host->wp_gpio != -1) > > if (gpio_is_valid(host->wp_gpio)) whould be better IMO. [...] > > + if (plat->cd_gpio != -1) { > > if (gpio_is_valid(host->wp_gpio)) whould be better IMO. > Fair enough (x2). > > + rc = request_irq(gpio_to_irq(plat->cd_gpio), carddetect_irq, > > + IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING, > > + mmc_hostname(sdhci->mmc), sdhci); > > + > > + if (rc) > > + goto err_remove_host; > > + } > > It seems to me that the tegra_sdhci_probe should handle gpio initialization, > rather then keep gpio_request etc calls in the board files. Yeah, I had been planning on moving that over but not at this pass. I can do that though. > > + printk(KERN_INFO "sdhci%d: initialized irq %d ioaddr %p\n", pdev->id, > > + sdhci->irq, sdhci->ioaddr); > > + > > dev_info? Yep, will change. Thanks! -Olof -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html