* [PATCH v4] mmc: sdhci: add error checking for mmc_add_host
@ 2011-04-11 2:11 Jaehoon Chung
2011-04-11 3:04 ` Wolfram Sang
0 siblings, 1 reply; 6+ messages in thread
From: Jaehoon Chung @ 2011-04-11 2:11 UTC (permalink / raw)
To: linux-mmc@vger.kernel.org; +Cc: Chris Ball, Wolfram Sang, Kyungmin Park
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
Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
drivers/mmc/host/sdhci.c | 22 ++++++++++++++++++++--
1 files changed, 20 insertions(+), 2 deletions(-)
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;
}
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v4] mmc: sdhci: add error checking for mmc_add_host
2011-04-11 2:11 [PATCH v4] mmc: sdhci: add error checking for mmc_add_host Jaehoon Chung
@ 2011-04-11 3:04 ` Wolfram Sang
2011-04-11 4:16 ` Jaehoon Chung
0 siblings, 1 reply; 6+ messages in thread
From: Wolfram Sang @ 2011-04-11 3:04 UTC (permalink / raw)
To: Jaehoon Chung; +Cc: linux-mmc@vger.kernel.org, Chris Ball, Kyungmin Park
[-- Attachment #1: Type: text/plain, Size: 643 bytes --]
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
--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4] mmc: sdhci: add error checking for mmc_add_host
2011-04-11 3:04 ` Wolfram Sang
@ 2011-04-11 4:16 ` Jaehoon Chung
2011-04-11 4:40 ` Wolfram Sang
0 siblings, 1 reply; 6+ messages in thread
From: Jaehoon Chung @ 2011-04-11 4:16 UTC (permalink / raw)
To: Wolfram Sang
Cc: Jaehoon Chung, linux-mmc@vger.kernel.org, Chris Ball,
Kyungmin Park
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
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4] mmc: sdhci: add error checking for mmc_add_host
2011-04-11 4:16 ` Jaehoon Chung
@ 2011-04-11 4:40 ` Wolfram Sang
2011-04-11 4:55 ` Jaehoon Chung
0 siblings, 1 reply; 6+ messages in thread
From: Wolfram Sang @ 2011-04-11 4:40 UTC (permalink / raw)
To: Jaehoon Chung; +Cc: linux-mmc@vger.kernel.org, Chris Ball, Kyungmin Park
[-- Attachment #1: Type: text/plain, Size: 424 bytes --]
> 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
--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4] mmc: sdhci: add error checking for mmc_add_host
2011-04-11 4:40 ` Wolfram Sang
@ 2011-04-11 4:55 ` Jaehoon Chung
2011-04-11 5:59 ` Wolfram Sang
0 siblings, 1 reply; 6+ messages in thread
From: Jaehoon Chung @ 2011-04-11 4:55 UTC (permalink / raw)
To: Wolfram Sang
Cc: Jaehoon Chung, linux-mmc@vger.kernel.org, Chris Ball,
Kyungmin Park
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
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4] mmc: sdhci: add error checking for mmc_add_host
2011-04-11 4:55 ` Jaehoon Chung
@ 2011-04-11 5:59 ` Wolfram Sang
0 siblings, 0 replies; 6+ messages in thread
From: Wolfram Sang @ 2011-04-11 5:59 UTC (permalink / raw)
To: Jaehoon Chung; +Cc: linux-mmc@vger.kernel.org, Chris Ball, Kyungmin Park
[-- Attachment #1: Type: text/plain, Size: 566 bytes --]
> 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
--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-04-11 6:00 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-04-11 2:11 [PATCH v4] mmc: sdhci: add error checking for mmc_add_host Jaehoon Chung
2011-04-11 3:04 ` Wolfram Sang
2011-04-11 4:16 ` Jaehoon Chung
2011-04-11 4:40 ` Wolfram Sang
2011-04-11 4:55 ` Jaehoon Chung
2011-04-11 5:59 ` Wolfram Sang
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.