Re: [PATCH v3 1/2] mmc: sdhci: add error checking for mmc_add_host

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Tue, Apr 05, 2011 at 05:02:19PM +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.
> 
> Signed-off-by: Jaehoon Chung <jh80.chung@xxxxxxxxxxx>
> Signed-off-by: Kyungin Park <kyungmin.park@xxxxxxxxxxx>
> ---
>  drivers/mmc/host/sdhci.c |   20 ++++++++++++++++++--
>  1 files changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 9e15f41..1768ffb 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,29 @@ int sdhci_add_host(struct sdhci_host *host)
>  
>  	return 0;
>  
> +err_free_mmc:
> +	mmc_remove_host(host);

As I asked in my last review: Why do you remove the host when add_host failed?
Doesn't that crash?

> +
>  #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);
> +
>  untasklet:
>  	tasklet_kill(&host->card_tasklet);
>  	tasklet_kill(&host->finish_tasklet);
>  
> +	if (host->flags & SDHCI_USE_ADMA) {
> +		kfree(host->adma_desc);
> +		kfree(host->align_buffer);
> +
> +		host->adma_desc = NULL;
> +		host->align_buffer = NULL;
> +	}

The if-condition is not needed, kfree is NULL-safe.

> +
>  	return ret;
>  }
>  

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Attachment: signature.asc
Description: Digital signature


[Index of Archives]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux