| Message ID | 20170806023954.wwaredjwcm2p5pw7@wens.csie.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
于 2017年8月6日 GMT+08:00 上午10:39:54, Chen-Yu Tsai <wens@csie.org> 写到: >On Sat, Aug 05, 2017 at 05:35:55AM +0800, Icenowy Zheng wrote: >> The configuration struct of A64 EMMC(MMC2) compatible used to >> have the needs_new_timings variable missing, which lead to NULL >> pointer dereference now when trying to set up the old timings mode, >as >> the old timings mode doesn't exist at all on A64. > >I'm not familiar with the A64's eMMC controller. The datasheet says >it does not have the timing switch register. It does not say whether >it is always in the old or new timing mode. "needs_new_timings" >probably meant that the switch has to be set. As I know, it supports DDR mode, but the CCU has no clk delays. So it should be the new timings mode. > >This fix doesn't really fix the underlying issue, which is the check >for clk_delays was incorrectly replaced. sun4i/sun5i is also broken. > >Could you try this patch instead: > ><--- > >From 23b841a11294cb6a0cf1e146616b068f60c2ec7d Mon Sep 17 00:00:00 2001 >From: Chen-Yu Tsai <wens@csie.org> >Date: Sun, 6 Aug 2017 10:24:47 +0800 >Subject: [PATCH] mmc: sunxi: Fix NULL pointer reference on clk_delays > >Some SoCs do not support clk delays for MMC in the clock control unit. >These include the old controllers in A10/A10s/A13/R8, and the new eMMC >controller in A64. The config structure for these controllers do not >specify clk_delays, but the check for this was replaced in commit >b0600daebf31 ("mmc: sunxi: Support controllers that can use both old >and new timings"). > >This patch adds back the check for clk_delays, and also adds comments >for both checks in sunxi_mmc_clk_set_phase(). > >Fixes: b0600daebf31 ("mmc: sunxi: Support controllers that can use > both old and new timings") >Signed-off-by: Chen-Yu Tsai <wens@csie.org> >--- > drivers/mmc/host/sunxi-mmc.c | 5 +++++ > 1 file changed, 5 insertions(+) > >diff --git a/drivers/mmc/host/sunxi-mmc.c >b/drivers/mmc/host/sunxi-mmc.c >index 3777517982dd..020547e5fa45 100644 >--- a/drivers/mmc/host/sunxi-mmc.c >+++ b/drivers/mmc/host/sunxi-mmc.c >@@ -722,9 +722,14 @@ static int sunxi_mmc_clk_set_phase(struct >sunxi_mmc_host *host, > { > int index; > >+ /* No need to set clk controller delays under new timings */ > if (host->use_new_timings) > return 0; > >+ /* Some old controllers don't support delays */ >+ if (!host->cfg->clk_delays) >+ return 0; >+ > /* determine delays */ > if (rate <= 400000) { > index = SDXC_CLK_400K; -- 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
在 2017-08-06 10:39，Chen-Yu Tsai 写道： > On Sat, Aug 05, 2017 at 05:35:55AM +0800, Icenowy Zheng wrote: >> The configuration struct of A64 EMMC(MMC2) compatible used to >> have the needs_new_timings variable missing, which lead to NULL >> pointer dereference now when trying to set up the old timings mode, as >> the old timings mode doesn't exist at all on A64. > > I'm not familiar with the A64's eMMC controller. The datasheet says > it does not have the timing switch register. It does not say whether > it is always in the old or new timing mode. "needs_new_timings" > probably meant that the switch has to be set. > > This fix doesn't really fix the underlying issue, which is the check > for clk_delays was incorrectly replaced. sun4i/sun5i is also broken. > > Could you try this patch instead: > > <--- > > From 23b841a11294cb6a0cf1e146616b068f60c2ec7d Mon Sep 17 00:00:00 2001 > From: Chen-Yu Tsai <wens@csie.org> > Date: Sun, 6 Aug 2017 10:24:47 +0800 > Subject: [PATCH] mmc: sunxi: Fix NULL pointer reference on clk_delays > > Some SoCs do not support clk delays for MMC in the clock control unit. > These include the old controllers in A10/A10s/A13/R8, and the new eMMC > controller in A64. The config structure for these controllers do not > specify clk_delays, but the check for this was replaced in commit > b0600daebf31 ("mmc: sunxi: Support controllers that can use both old > and new timings"). > > This patch adds back the check for clk_delays, and also adds comments > for both checks in sunxi_mmc_clk_set_phase(). > > Fixes: b0600daebf31 ("mmc: sunxi: Support controllers that can use > both old and new timings") > Signed-off-by: Chen-Yu Tsai <wens@csie.org> Successfully tested on A64 (SoPine with baseboard and eMMC module). > --- > drivers/mmc/host/sunxi-mmc.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/mmc/host/sunxi-mmc.c > b/drivers/mmc/host/sunxi-mmc.c > index 3777517982dd..020547e5fa45 100644 > --- a/drivers/mmc/host/sunxi-mmc.c > +++ b/drivers/mmc/host/sunxi-mmc.c > @@ -722,9 +722,14 @@ static int sunxi_mmc_clk_set_phase(struct > sunxi_mmc_host *host, > { > int index; > > + /* No need to set clk controller delays under new timings */ > if (host->use_new_timings) > return 0; > > + /* Some old controllers don't support delays */ > + if (!host->cfg->clk_delays) > + return 0; > + > /* determine delays */ > if (rate <= 400000) { > index = SDXC_CLK_400K; > -- > 2.13.3 > > <--- > > Separately we should ask what is the proper set of config flags that > describes the A64 EMMC controller. > > Regards > ChenYu > >> >> Fix this issue by adding this variable and setting it to true in >> the configuration struct. >> >> Fixes: 4fb3ce07eafa ("mmc: sunxi: Add EMMC (MMC2) controller >> compatible") >> >> Signed-off-by: Icenowy Zheng <icenowy@aosc.io> >> --- >> drivers/mmc/host/sunxi-mmc.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/drivers/mmc/host/sunxi-mmc.c >> b/drivers/mmc/host/sunxi-mmc.c >> index 59aba93beffb..4ad643e37014 100644 >> --- a/drivers/mmc/host/sunxi-mmc.c >> +++ b/drivers/mmc/host/sunxi-mmc.c >> @@ -1142,6 +1142,7 @@ static const struct sunxi_mmc_cfg >> sun50i_a64_emmc_cfg = { >> .idma_des_size_bits = 13, >> .clk_delays = NULL, >> .can_calibrate = true, >> + .needs_new_timings = true, >> }; >> >> static const struct of_device_id sunxi_mmc_of_match[] = { >> -- >> 2.13.0 >> >> -- >> You received this message because you are subscribed to the Google >> Groups "linux-sunxi" group. >> To unsubscribe from this group and stop receiving emails from it, send >> an email to linux-sunxi+unsubscribe@googlegroups.com. >> For more options, visit https://groups.google.com/d/optout. > > _______________________________________________ > 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
diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c index 3777517982dd..020547e5fa45 100644 --- a/drivers/mmc/host/sunxi-mmc.c +++ b/drivers/mmc/host/sunxi-mmc.c @@ -722,9 +722,14 @@ static int sunxi_mmc_clk_set_phase(struct sunxi_mmc_host *host, { int index; + /* No need to set clk controller delays under new timings */ if (host->use_new_timings) return 0; + /* Some old controllers don't support delays */ + if (!host->cfg->clk_delays) + return 0; + /* determine delays */ if (rate <= 400000) { index = SDXC_CLK_400K;