| Message ID | 1426112117-18220-1-git-send-email-dianders@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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"); > >
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
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 >
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"); > >
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.
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");
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(-)