| Message ID | 4DA2633C.5000103@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Apr 11, 2011 at 11:11:08AM +0900, Jaehoon Chung wrote: > Sometimes we can't add the device,but we didn't check any error status. > Need to check error status for mmc_add_host. > And Missing regulator disable/put. Fixed them. Is there any special reason you don't want to answer the open question from the last two reviews? > [PATCH v4] : merged for v3 [PATCH 1/2] and [PATCH 2/2] reviewed on Wolfram Sang This should go below the dashed line (---) Wolfram
Wolfram Sang wrote: > On Mon, Apr 11, 2011 at 11:11:08AM +0900, Jaehoon Chung wrote: >> Sometimes we can't add the device,but we didn't check any error status. >> Need to check error status for mmc_add_host. >> And Missing regulator disable/put. Fixed them. > > Is there any special reason you don't want to answer the open question from the > last two reviews? Sorry..i didn't find your questions. So i didn't answer anything. You asked "why removed host?" right? I just think good that remove the host...(any crash didn't occur). there is not special reason. > >> [PATCH v4] : merged for v3 [PATCH 1/2] and [PATCH 2/2] reviewed on Wolfram Sang > > This should go below the dashed line (---) > > Wolfram > -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> I just think good that remove the host...(any crash didn't occur). > there is not special reason. Look at the mmc-core code, please (host.c). Compare what remove_host does and what add_host does before it fails. What do you think? Regards, Wolfram
Wolfram Sang wrote: >> I just think good that remove the host...(any crash didn't occur). >> there is not special reason. > > Look at the mmc-core code, please (host.c). Compare what remove_host does and > what add_host does before it fails. What do you think? Ok..i'm looking at the mmc-core code(host.c) mmc_add_host does devices_add(), then run start_host(). mmc_remove_host does stop_host(). You means that didn't run start_host() before add_host failed. I think that need not to run stop_host. right? To use mmc_free_host, better than mmc_remove_host. Regards, Jaehoon Chung > > Regards, > > Wolfram > -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> You means that didn't run start_host() before add_host failed. > I think that need not to run stop_host. right? Right, and other stuff like registering the pm_notifier. > To use mmc_free_host, better than mmc_remove_host. That would be true for most drivers. However, sdhci has encapsulated this in its own sdhci_free_host. So, you don't need to call this as well. Regards, Wolfram
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index 9e15f41..e9cb09c 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -2025,7 +2025,9 @@ int sdhci_add_host(struct sdhci_host *host) mmiowb(); - mmc_add_host(mmc); + ret = mmc_add_host(mmc); + if (unlikely(ret)) + goto err_free_mmc; printk(KERN_INFO "%s: SDHCI controller on %s [%s] using %s\n", mmc_hostname(mmc), host->hw_name, dev_name(mmc_dev(mmc)), @@ -2036,15 +2038,31 @@ int sdhci_add_host(struct sdhci_host *host) return 0; +err_free_mmc: + mmc_remove_host(host); + #ifdef SDHCI_USE_LEDS_CLASS + led_classdev_ungregister(&host->led); reset: +#endif sdhci_reset(host, SDHCI_RESET_ALL); free_irq(host->irq, host); -#endif + del_timer_sync(&host->timer); + if (host->vmmc) { + regulator_disable(host->vmmc); + regulator_put(host->vmmc); + } + untasklet: tasklet_kill(&host->card_tasklet); tasklet_kill(&host->finish_tasklet); + kfree(host->adma_desc); + kfree(host->align_buffer); + + host->adma_desc = NULL; + host->align_buffer = NULL; + return ret; }