diff mbox

[v4,1/4] mmc: dw_mmc: Don't try to enable the CD until we're sure we're not deferring

Message ID 1426112117-18220-1-git-send-email-dianders@chromium.org (mailing list archive)
State New, archived
Headers show

Commit Message

Doug Anderson March 11, 2015, 10:15 p.m. UTC
If dw_mci_init_slot() returns that we got a probe deferral then it may
leave slot->mmc as NULL.  That will cause dw_mci_enable_cd() to crash
when it calls mmc_gpio_get_cd().

Fix this by moving the call of dw_mci_enable_cd() until we're sure
that we're good.  Note that if we have more than one slot and one
defers (but the others don't) things won't work so well.  ...but
that's not a new thing and everyone has already agreed that multislot
support ought to be removed from dw_mmc eventually anyway since it is
unused, untested, and you can see several bugs like this by inspecting
the code.

Fixes: bcafaf5470f0 ("mmc: dw_mmc: Only enable CD after setup and only if needed")
Signed-off-by: Doug Anderson <dianders@chromium.org>
---
Changes in v4:
- Defer vs. card detect fix patch new for v4.

Changes in v3: None
Changes in v2: None

 drivers/mmc/host/dw_mmc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Jaehoon Chung March 13, 2015, 11:32 a.m. UTC | #1
Hi Doug,

Will apply. Thanks!

Best Regards,
Jaehoon Chung

On 03/12/2015 07:15 AM, Doug Anderson wrote:
> If dw_mci_init_slot() returns that we got a probe deferral then it may
> leave slot->mmc as NULL.  That will cause dw_mci_enable_cd() to crash
> when it calls mmc_gpio_get_cd().
> 
> Fix this by moving the call of dw_mci_enable_cd() until we're sure
> that we're good.  Note that if we have more than one slot and one
> defers (but the others don't) things won't work so well.  ...but
> that's not a new thing and everyone has already agreed that multislot
> support ought to be removed from dw_mmc eventually anyway since it is
> unused, untested, and you can see several bugs like this by inspecting
> the code.
> 
> Fixes: bcafaf5470f0 ("mmc: dw_mmc: Only enable CD after setup and only if needed")
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> ---
> Changes in v4:
> - Defer vs. card detect fix patch new for v4.
> 
> Changes in v3: None
> Changes in v2: None
> 
>  drivers/mmc/host/dw_mmc.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 47dfd0e..e2811cf 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -2863,9 +2863,6 @@ int dw_mci_probe(struct dw_mci *host)
>  			init_slots++;
>  	}
>  
> -	/* Now that slots are all setup, we can enable card detect */
> -	dw_mci_enable_cd(host);
> -
>  	if (init_slots) {
>  		dev_info(host->dev, "%d slots initialized\n", init_slots);
>  	} else {
> @@ -2874,6 +2871,9 @@ int dw_mci_probe(struct dw_mci *host)
>  		goto err_dmaunmap;
>  	}
>  
> +	/* Now that slots are all setup, we can enable card detect */
> +	dw_mci_enable_cd(host);
> +
>  	if (host->quirks & DW_MCI_QUIRK_IDMAC_DTO)
>  		dev_info(host->dev, "Internal DMAC interrupt fix enabled.\n");
>  
> 

--
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
Heiko Stuebner March 13, 2015, 12:10 p.m. UTC | #2
Hi,

Am Freitag, 13. März 2015, 20:32:43 schrieb Jaehoon Chung:
> Hi Doug,
> 
> Will apply. Thanks!

just to make sure, you'll take patches 1-3 and I'll take the dts change from 
patch 4, right?


Thanks
Heiko
--
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
Jaehoon Chung March 16, 2015, 2:09 a.m. UTC | #3
Hi, Heiko.


On 03/13/2015 09:10 PM, Heiko Stuebner wrote:
> Hi,
> 
> Am Freitag, 13. März 2015, 20:32:43 schrieb Jaehoon Chung:
>> Hi Doug,
>>
>> Will apply. Thanks!
> 
> just to make sure, you'll take patches 1-3 and I'll take the dts change from 
> patch 4, right?

Right. I will check on today and request pull to ulf on tomorrow.
Will CC'd your email when PR.

Best Regards,
Jaehoon Chung

> 
> 
> Thanks
> Heiko
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

--
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
Jaehoon Chung March 27, 2015, 5:55 a.m. UTC | #4
Hi, Doug.

This patch is not related with [patch 2/4~4/4].
"[PATCH v4 2/4] mmc: core: Add mmc_regulator_set_vqmmc()" is discussing..
So i think if you're ok, i will pick this one [PATCH v4 1/4]. how about?

Best Regards,
Jaehoon Chung

On 03/12/2015 07:15 AM, Doug Anderson wrote:
> If dw_mci_init_slot() returns that we got a probe deferral then it may
> leave slot->mmc as NULL.  That will cause dw_mci_enable_cd() to crash
> when it calls mmc_gpio_get_cd().
> 
> Fix this by moving the call of dw_mci_enable_cd() until we're sure
> that we're good.  Note that if we have more than one slot and one
> defers (but the others don't) things won't work so well.  ...but
> that's not a new thing and everyone has already agreed that multislot
> support ought to be removed from dw_mmc eventually anyway since it is
> unused, untested, and you can see several bugs like this by inspecting
> the code.
> 
> Fixes: bcafaf5470f0 ("mmc: dw_mmc: Only enable CD after setup and only if needed")
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> ---
> Changes in v4:
> - Defer vs. card detect fix patch new for v4.
> 
> Changes in v3: None
> Changes in v2: None
> 
>  drivers/mmc/host/dw_mmc.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 47dfd0e..e2811cf 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -2863,9 +2863,6 @@ int dw_mci_probe(struct dw_mci *host)
>  			init_slots++;
>  	}
>  
> -	/* Now that slots are all setup, we can enable card detect */
> -	dw_mci_enable_cd(host);
> -
>  	if (init_slots) {
>  		dev_info(host->dev, "%d slots initialized\n", init_slots);
>  	} else {
> @@ -2874,6 +2871,9 @@ int dw_mci_probe(struct dw_mci *host)
>  		goto err_dmaunmap;
>  	}
>  
> +	/* Now that slots are all setup, we can enable card detect */
> +	dw_mci_enable_cd(host);
> +
>  	if (host->quirks & DW_MCI_QUIRK_IDMAC_DTO)
>  		dev_info(host->dev, "Internal DMAC interrupt fix enabled.\n");
>  
> 

--
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
Doug Anderson March 27, 2015, 3:46 p.m. UTC | #5
Jaehoon,

On Thu, Mar 26, 2015 at 10:55 PM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> Hi, Doug.
>
> This patch is not related with [patch 2/4~4/4].
> "[PATCH v4 2/4] mmc: core: Add mmc_regulator_set_vqmmc()" is discussing..
> So i think if you're ok, i will pick this one [PATCH v4 1/4]. how about?

Please take it and send it to Ulf as soon as you can.  It is only
related in that the later patches in this series suddenly make the
rk3288-evb start deferring and that causes a crash without this patch.
--
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
diff mbox

Patch

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 47dfd0e..e2811cf 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -2863,9 +2863,6 @@  int dw_mci_probe(struct dw_mci *host)
 			init_slots++;
 	}
 
-	/* Now that slots are all setup, we can enable card detect */
-	dw_mci_enable_cd(host);
-
 	if (init_slots) {
 		dev_info(host->dev, "%d slots initialized\n", init_slots);
 	} else {
@@ -2874,6 +2871,9 @@  int dw_mci_probe(struct dw_mci *host)
 		goto err_dmaunmap;
 	}
 
+	/* Now that slots are all setup, we can enable card detect */
+	dw_mci_enable_cd(host);
+
 	if (host->quirks & DW_MCI_QUIRK_IDMAC_DTO)
 		dev_info(host->dev, "Internal DMAC interrupt fix enabled.\n");