| Message ID | 6f6b511bce9df86ee43486653ac55df0d135f198.1484585798.git-series.maxime.ripard@free-electrons.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 16 January 2017 at 17:56, Maxime Ripard <maxime.ripard@free-electrons.com> wrote: > The MMC and SD specifications documents that the clock frequency should > only be changed once gated. Where? > > The current code first modifies the parent clock, gates it and then > modifies the internal divider. This means that since the parent clock rate > might be changed, the bus clock might be changed as well before it is > gated, which breaks the specification. > > Move the gating before the parent rate modification. This all makes perfect sense to me, however I am not sure you need to refer to the spec to justify these changes. Kind regards Uffe > > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> > --- > drivers/mmc/host/sunxi-mmc.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c > index b1d1303389a7..ab4324e6eb74 100644 > --- a/drivers/mmc/host/sunxi-mmc.c > +++ b/drivers/mmc/host/sunxi-mmc.c > @@ -761,6 +761,10 @@ static int sunxi_mmc_clk_set_rate(struct sunxi_mmc_host *host, > u32 rval, clock = ios->clock; > int ret; > > + ret = sunxi_mmc_oclk_onoff(host, 0); > + if (ret) > + return ret; > + > /* 8 bit DDR requires a higher module clock */ > if (ios->timing == MMC_TIMING_MMC_DDR52 && > ios->bus_width == MMC_BUS_WIDTH_8) > @@ -783,10 +787,6 @@ static int sunxi_mmc_clk_set_rate(struct sunxi_mmc_host *host, > return ret; > } > > - ret = sunxi_mmc_oclk_onoff(host, 0); > - if (ret) > - return ret; > - > /* clear internal divider */ > rval = mmc_readl(host, REG_CLKCR); > rval &= ~0xff; > -- > git-series 0.8.11 -- 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
Hi Ulf, On Tue, Jan 24, 2017 at 09:12:07AM +0100, Ulf Hansson wrote: > On 16 January 2017 at 17:56, Maxime Ripard > <maxime.ripard@free-electrons.com> wrote: > > The MMC and SD specifications documents that the clock frequency should > > only be changed once gated. > > Where? > > > > > The current code first modifies the parent clock, gates it and then > > modifies the internal divider. This means that since the parent clock rate > > might be changed, the bus clock might be changed as well before it is > > gated, which breaks the specification. > > > > Move the gating before the parent rate modification. > > This all makes perfect sense to me, however I am not sure you need to > refer to the spec to justify these changes. I can't find it anymore :/ I'll resend that patch reworking the commit log. Thanks! Maxime
diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c index b1d1303389a7..ab4324e6eb74 100644 --- a/drivers/mmc/host/sunxi-mmc.c +++ b/drivers/mmc/host/sunxi-mmc.c @@ -761,6 +761,10 @@ static int sunxi_mmc_clk_set_rate(struct sunxi_mmc_host *host, u32 rval, clock = ios->clock; int ret; + ret = sunxi_mmc_oclk_onoff(host, 0); + if (ret) + return ret; + /* 8 bit DDR requires a higher module clock */ if (ios->timing == MMC_TIMING_MMC_DDR52 && ios->bus_width == MMC_BUS_WIDTH_8) @@ -783,10 +787,6 @@ static int sunxi_mmc_clk_set_rate(struct sunxi_mmc_host *host, return ret; } - ret = sunxi_mmc_oclk_onoff(host, 0); - if (ret) - return ret; - /* clear internal divider */ rval = mmc_readl(host, REG_CLKCR); rval &= ~0xff;
The MMC and SD specifications documents that the clock frequency should only be changed once gated. The current code first modifies the parent clock, gates it and then modifies the internal divider. This means that since the parent clock rate might be changed, the bus clock might be changed as well before it is gated, which breaks the specification. Move the gating before the parent rate modification. Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> --- drivers/mmc/host/sunxi-mmc.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)