mmc: sdhci: add error checking for mmc_add_host
PATCH v4 1/1 mmc: sdhci: add error checking for mmc_add_host
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.
[PATCH v4] : merged for v3 [PATCH 1/2] and [PATCH 2/2] reviewed on Wolfram Sang
--- drivers/mmc/host/sdhci.c | 22 ++++++++++++++++++++-- 1 files changed, 20 insertions(+), 2 deletions(-)
drivers/mmc/host/sdhci.c +20 -2
@@ -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; }
Quoted 3 lines · Jaehoon Chung
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?
Quoted 1 line
[PATCH v4] : merged for v3 [PATCH 1/2] and [PATCH 2/2] reviewed on Wolfram Sang
This should go below the dashed line (---)
Wolfram
Signature · 2 lines
Pengutronix e.K. | Wolfram Sang | Industrial Linux Solutions | http://www.pengutronix.de/ |
Quoted 7 lines · Wolfram Sang
Quoted 3 lines · Jaehoon Chung
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.
Quoted 7 lines
Quoted 2 lines
[PATCH v4] : merged for v3 [PATCH 1/2] and [PATCH 2/2] reviewed on Wolfram Sang
This should go below the dashed line (---)
Wolfram
Quoted 2 lines
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
Signature · 2 lines
Pengutronix e.K. | Wolfram Sang | Industrial Linux Solutions | http://www.pengutronix.de/ |
Quoted 5 lines · Wolfram Sang
Quoted 2 lines
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 whatremove_hostdoes and whatadd_hostdoes 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
Quoted 5 lines
Regards,
Wolfram
Quoted 3 lines
You means that didn't run
start_host()beforeadd_hostfailed. I think that need not to runstop_host. right?
Right, and other stuff like registering the pm_notifier.
Quoted 1 line
To use
mmc_free_host, better thanmmc_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
Signature · 2 lines
Pengutronix e.K. | Wolfram Sang | Industrial Linux Solutions | http://www.pengutronix.de/ |