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-mmc" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
