| Message ID | 20171204051912.7485-3-wens@csie.org (mailing list archive) |
|---|---|
| State | Mainlined, archived |
| Headers | show |
On 04/12/17 05:19, Chen-Yu Tsai wrote: > On the A64, the MMC module clocks are fixed in the new timing mode, > i.e. they do not have a bit to select the mode. These clocks have > a 2x divider somewhere between the clock and the MMC module. > > To be consistent with other SoCs supporting the new timing mode, > we model the 2x divider as a fixed post-divider on the MMC module > clocks. > > This patch adds the post-dividers to the MMC clocks. > > Signed-off-by: Chen-Yu Tsai <wens@csie.org> Reviewed-by: Andre Przywara <andre.przywara@arm.com> Cheers, Andre > --- > drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 57 +++++++++++++++++++++++------------ > 1 file changed, 37 insertions(+), 20 deletions(-) > > diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c > index 2bb4cabf802f..ee9c12cf3f08 100644 > --- a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c > +++ b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c > @@ -400,28 +400,45 @@ static SUNXI_CCU_MP_WITH_MUX_GATE(nand_clk, "nand", mod0_default_parents, 0x080, > BIT(31), /* gate */ > 0); > > +/* > + * MMC clocks are the new timing mode (see A83T & H3) variety, but without > + * the mode switch. This means they have a 2x post divider between the clock > + * and the MMC module. This is not documented in the manual, but is taken > + * into consideration when setting the mmc module clocks in the BSP kernel. > + * Without it, MMC performance is degraded. > + * > + * We model it here to be consistent with other SoCs supporting this mode. > + * The alternative would be to add the 2x multiplier when setting the MMC > + * module clock in the MMC driver, just for the A64. > + */ > static const char * const mmc_default_parents[] = { "osc24M", "pll-periph0-2x", > "pll-periph1-2x" }; > -static SUNXI_CCU_MP_WITH_MUX_GATE(mmc0_clk, "mmc0", mmc_default_parents, 0x088, > - 0, 4, /* M */ > - 16, 2, /* P */ > - 24, 2, /* mux */ > - BIT(31), /* gate */ > - 0); > - > -static SUNXI_CCU_MP_WITH_MUX_GATE(mmc1_clk, "mmc1", mmc_default_parents, 0x08c, > - 0, 4, /* M */ > - 16, 2, /* P */ > - 24, 2, /* mux */ > - BIT(31), /* gate */ > - 0); > - > -static SUNXI_CCU_MP_WITH_MUX_GATE(mmc2_clk, "mmc2", mmc_default_parents, 0x090, > - 0, 4, /* M */ > - 16, 2, /* P */ > - 24, 2, /* mux */ > - BIT(31), /* gate */ > - 0); > +static SUNXI_CCU_MP_WITH_MUX_GATE_POSTDIV(mmc0_clk, "mmc0", > + mmc_default_parents, 0x088, > + 0, 4, /* M */ > + 16, 2, /* P */ > + 24, 2, /* mux */ > + BIT(31), /* gate */ > + 2, /* post-div */ > + 0); > + > +static SUNXI_CCU_MP_WITH_MUX_GATE_POSTDIV(mmc1_clk, "mmc1", > + mmc_default_parents, 0x08c, > + 0, 4, /* M */ > + 16, 2, /* P */ > + 24, 2, /* mux */ > + BIT(31), /* gate */ > + 2, /* post-div */ > + 0); > + > +static SUNXI_CCU_MP_WITH_MUX_GATE_POSTDIV(mmc2_clk, "mmc2", > + mmc_default_parents, 0x090, > + 0, 4, /* M */ > + 16, 2, /* P */ > + 24, 2, /* mux */ > + BIT(31), /* gate */ > + 2, /* post-div */ > + 0); > > static const char * const ts_parents[] = { "osc24M", "pll-periph0", }; > static SUNXI_CCU_MP_WITH_MUX_GATE(ts_clk, "ts", ts_parents, 0x098, >
Hi, On Mon, Dec 04, 2017 at 01:19:12PM +0800, Chen-Yu Tsai wrote: > On the A64, the MMC module clocks are fixed in the new timing mode, > i.e. they do not have a bit to select the mode. These clocks have > a 2x divider somewhere between the clock and the MMC module. > > To be consistent with other SoCs supporting the new timing mode, > we model the 2x divider as a fixed post-divider on the MMC module > clocks. > > This patch adds the post-dividers to the MMC clocks. > > Signed-off-by: Chen-Yu Tsai <wens@csie.org> I had a doubt applying that one... sorry. > --- > drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 57 +++++++++++++++++++++++------------ > 1 file changed, 37 insertions(+), 20 deletions(-) > > diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c > index 2bb4cabf802f..ee9c12cf3f08 100644 > --- a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c > +++ b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c > @@ -400,28 +400,45 @@ static SUNXI_CCU_MP_WITH_MUX_GATE(nand_clk, "nand", mod0_default_parents, 0x080, > BIT(31), /* gate */ > 0); > > +/* > + * MMC clocks are the new timing mode (see A83T & H3) variety, but without > + * the mode switch. This means they have a 2x post divider between the clock > + * and the MMC module. This is not documented in the manual, but is taken > + * into consideration when setting the mmc module clocks in the BSP kernel. > + * Without it, MMC performance is degraded. > + * > + * We model it here to be consistent with other SoCs supporting this mode. > + * The alternative would be to add the 2x multiplier when setting the MMC > + * module clock in the MMC driver, just for the A64. > + */ > static const char * const mmc_default_parents[] = { "osc24M", "pll-periph0-2x", > "pll-periph1-2x" }; > -static SUNXI_CCU_MP_WITH_MUX_GATE(mmc0_clk, "mmc0", mmc_default_parents, 0x088, > - 0, 4, /* M */ > - 16, 2, /* P */ > - 24, 2, /* mux */ > - BIT(31), /* gate */ > - 0); > - > -static SUNXI_CCU_MP_WITH_MUX_GATE(mmc1_clk, "mmc1", mmc_default_parents, 0x08c, > - 0, 4, /* M */ > - 16, 2, /* P */ > - 24, 2, /* mux */ > - BIT(31), /* gate */ > - 0); > - > -static SUNXI_CCU_MP_WITH_MUX_GATE(mmc2_clk, "mmc2", mmc_default_parents, 0x090, > - 0, 4, /* M */ > - 16, 2, /* P */ > - 24, 2, /* mux */ > - BIT(31), /* gate */ > - 0); > +static SUNXI_CCU_MP_WITH_MUX_GATE_POSTDIV(mmc0_clk, "mmc0", > + mmc_default_parents, 0x088, > + 0, 4, /* M */ > + 16, 2, /* P */ > + 24, 2, /* mux */ > + BIT(31), /* gate */ > + 2, /* post-div */ > + 0); > + > +static SUNXI_CCU_MP_WITH_MUX_GATE_POSTDIV(mmc1_clk, "mmc1", > + mmc_default_parents, 0x08c, > + 0, 4, /* M */ > + 16, 2, /* P */ > + 24, 2, /* mux */ > + BIT(31), /* gate */ > + 2, /* post-div */ > + 0); > + Are you sure that the divider there for the non-eMMC clocks? Usually, the new mode is only here for the eMMC, so we would divide the rate by two in the non-eMMC case. Maxime
On Wed, Dec 6, 2017 at 3:59 AM, Maxime Ripard <maxime.ripard@free-electrons.com> wrote: > Hi, > > On Mon, Dec 04, 2017 at 01:19:12PM +0800, Chen-Yu Tsai wrote: >> On the A64, the MMC module clocks are fixed in the new timing mode, >> i.e. they do not have a bit to select the mode. These clocks have >> a 2x divider somewhere between the clock and the MMC module. >> >> To be consistent with other SoCs supporting the new timing mode, >> we model the 2x divider as a fixed post-divider on the MMC module >> clocks. >> >> This patch adds the post-dividers to the MMC clocks. >> >> Signed-off-by: Chen-Yu Tsai <wens@csie.org> > > I had a doubt applying that one... sorry. > >> --- >> drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 57 +++++++++++++++++++++++------------ >> 1 file changed, 37 insertions(+), 20 deletions(-) >> >> diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c >> index 2bb4cabf802f..ee9c12cf3f08 100644 >> --- a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c >> +++ b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c >> @@ -400,28 +400,45 @@ static SUNXI_CCU_MP_WITH_MUX_GATE(nand_clk, "nand", mod0_default_parents, 0x080, >> BIT(31), /* gate */ >> 0); >> >> +/* >> + * MMC clocks are the new timing mode (see A83T & H3) variety, but without >> + * the mode switch. This means they have a 2x post divider between the clock >> + * and the MMC module. This is not documented in the manual, but is taken >> + * into consideration when setting the mmc module clocks in the BSP kernel. >> + * Without it, MMC performance is degraded. >> + * >> + * We model it here to be consistent with other SoCs supporting this mode. >> + * The alternative would be to add the 2x multiplier when setting the MMC >> + * module clock in the MMC driver, just for the A64. >> + */ >> static const char * const mmc_default_parents[] = { "osc24M", "pll-periph0-2x", >> "pll-periph1-2x" }; >> -static SUNXI_CCU_MP_WITH_MUX_GATE(mmc0_clk, "mmc0", mmc_default_parents, 0x088, >> - 0, 4, /* M */ >> - 16, 2, /* P */ >> - 24, 2, /* mux */ >> - BIT(31), /* gate */ >> - 0); >> - >> -static SUNXI_CCU_MP_WITH_MUX_GATE(mmc1_clk, "mmc1", mmc_default_parents, 0x08c, >> - 0, 4, /* M */ >> - 16, 2, /* P */ >> - 24, 2, /* mux */ >> - BIT(31), /* gate */ >> - 0); >> - >> -static SUNXI_CCU_MP_WITH_MUX_GATE(mmc2_clk, "mmc2", mmc_default_parents, 0x090, >> - 0, 4, /* M */ >> - 16, 2, /* P */ >> - 24, 2, /* mux */ >> - BIT(31), /* gate */ >> - 0); >> +static SUNXI_CCU_MP_WITH_MUX_GATE_POSTDIV(mmc0_clk, "mmc0", >> + mmc_default_parents, 0x088, >> + 0, 4, /* M */ >> + 16, 2, /* P */ >> + 24, 2, /* mux */ >> + BIT(31), /* gate */ >> + 2, /* post-div */ >> + 0); >> + >> +static SUNXI_CCU_MP_WITH_MUX_GATE_POSTDIV(mmc1_clk, "mmc1", >> + mmc_default_parents, 0x08c, >> + 0, 4, /* M */ >> + 16, 2, /* P */ >> + 24, 2, /* mux */ >> + BIT(31), /* gate */ >> + 2, /* post-div */ >> + 0); >> + > > Are you sure that the divider there for the non-eMMC clocks? Usually, > the new mode is only here for the eMMC, so we would divide the rate by > two in the non-eMMC case. The new mode is there for all MMC controllers. The other two MMC controllers even have the old/new timing mode switch. In case you forgot we have ".need_new_timings" set for the A64 compatible. But to eliminate any doubts or concerns, I've rerun tests for the micro SD card, instead of the eMMC. And yes the results are the same, 2x improvement (12 MB/s vs 23.7 MB/s). ChenYu
Hi, On Wed, Dec 06, 2017 at 10:30:26AM +0800, Chen-Yu Tsai wrote: > On Wed, Dec 6, 2017 at 3:59 AM, Maxime Ripard > <maxime.ripard@free-electrons.com> wrote: > > Hi, > > > > On Mon, Dec 04, 2017 at 01:19:12PM +0800, Chen-Yu Tsai wrote: > >> On the A64, the MMC module clocks are fixed in the new timing mode, > >> i.e. they do not have a bit to select the mode. These clocks have > >> a 2x divider somewhere between the clock and the MMC module. > >> > >> To be consistent with other SoCs supporting the new timing mode, > >> we model the 2x divider as a fixed post-divider on the MMC module > >> clocks. > >> > >> This patch adds the post-dividers to the MMC clocks. > >> > >> Signed-off-by: Chen-Yu Tsai <wens@csie.org> > > > > I had a doubt applying that one... sorry. > > > >> --- > >> drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 57 +++++++++++++++++++++++------------ > >> 1 file changed, 37 insertions(+), 20 deletions(-) > >> > >> diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c > >> index 2bb4cabf802f..ee9c12cf3f08 100644 > >> --- a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c > >> +++ b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c > >> @@ -400,28 +400,45 @@ static SUNXI_CCU_MP_WITH_MUX_GATE(nand_clk, "nand", mod0_default_parents, 0x080, > >> BIT(31), /* gate */ > >> 0); > >> > >> +/* > >> + * MMC clocks are the new timing mode (see A83T & H3) variety, but without > >> + * the mode switch. This means they have a 2x post divider between the clock > >> + * and the MMC module. This is not documented in the manual, but is taken > >> + * into consideration when setting the mmc module clocks in the BSP kernel. > >> + * Without it, MMC performance is degraded. > >> + * > >> + * We model it here to be consistent with other SoCs supporting this mode. > >> + * The alternative would be to add the 2x multiplier when setting the MMC > >> + * module clock in the MMC driver, just for the A64. > >> + */ > >> static const char * const mmc_default_parents[] = { "osc24M", "pll-periph0-2x", > >> "pll-periph1-2x" }; > >> -static SUNXI_CCU_MP_WITH_MUX_GATE(mmc0_clk, "mmc0", mmc_default_parents, 0x088, > >> - 0, 4, /* M */ > >> - 16, 2, /* P */ > >> - 24, 2, /* mux */ > >> - BIT(31), /* gate */ > >> - 0); > >> - > >> -static SUNXI_CCU_MP_WITH_MUX_GATE(mmc1_clk, "mmc1", mmc_default_parents, 0x08c, > >> - 0, 4, /* M */ > >> - 16, 2, /* P */ > >> - 24, 2, /* mux */ > >> - BIT(31), /* gate */ > >> - 0); > >> - > >> -static SUNXI_CCU_MP_WITH_MUX_GATE(mmc2_clk, "mmc2", mmc_default_parents, 0x090, > >> - 0, 4, /* M */ > >> - 16, 2, /* P */ > >> - 24, 2, /* mux */ > >> - BIT(31), /* gate */ > >> - 0); > >> +static SUNXI_CCU_MP_WITH_MUX_GATE_POSTDIV(mmc0_clk, "mmc0", > >> + mmc_default_parents, 0x088, > >> + 0, 4, /* M */ > >> + 16, 2, /* P */ > >> + 24, 2, /* mux */ > >> + BIT(31), /* gate */ > >> + 2, /* post-div */ > >> + 0); > >> + > >> +static SUNXI_CCU_MP_WITH_MUX_GATE_POSTDIV(mmc1_clk, "mmc1", > >> + mmc_default_parents, 0x08c, > >> + 0, 4, /* M */ > >> + 16, 2, /* P */ > >> + 24, 2, /* mux */ > >> + BIT(31), /* gate */ > >> + 2, /* post-div */ > >> + 0); > >> + > > > > Are you sure that the divider there for the non-eMMC clocks? Usually, > > the new mode is only here for the eMMC, so we would divide the rate by > > two in the non-eMMC case. > > The new mode is there for all MMC controllers. The other two MMC > controllers even have the old/new timing mode switch. In case you > forgot we have ".need_new_timings" set for the A64 compatible. But then, shouldn't we model them as such, using the work you did on the A83t clocks? > But to eliminate any doubts or concerns, I've rerun tests for the > micro SD card, instead of the eMMC. And yes the results are the same, > 2x improvement (12 MB/s vs 23.7 MB/s). Ok, good. Thanks! Maxime
On Wed, Dec 6, 2017 at 11:56 PM, Maxime Ripard <maxime.ripard@free-electrons.com> wrote: > Hi, > > On Wed, Dec 06, 2017 at 10:30:26AM +0800, Chen-Yu Tsai wrote: >> On Wed, Dec 6, 2017 at 3:59 AM, Maxime Ripard >> <maxime.ripard@free-electrons.com> wrote: >> > Hi, >> > >> > On Mon, Dec 04, 2017 at 01:19:12PM +0800, Chen-Yu Tsai wrote: >> >> On the A64, the MMC module clocks are fixed in the new timing mode, >> >> i.e. they do not have a bit to select the mode. These clocks have >> >> a 2x divider somewhere between the clock and the MMC module. >> >> >> >> To be consistent with other SoCs supporting the new timing mode, >> >> we model the 2x divider as a fixed post-divider on the MMC module >> >> clocks. >> >> >> >> This patch adds the post-dividers to the MMC clocks. >> >> >> >> Signed-off-by: Chen-Yu Tsai <wens@csie.org> >> > >> > I had a doubt applying that one... sorry. >> > >> >> --- >> >> drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 57 +++++++++++++++++++++++------------ >> >> 1 file changed, 37 insertions(+), 20 deletions(-) >> >> >> >> diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c >> >> index 2bb4cabf802f..ee9c12cf3f08 100644 >> >> --- a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c >> >> +++ b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c >> >> @@ -400,28 +400,45 @@ static SUNXI_CCU_MP_WITH_MUX_GATE(nand_clk, "nand", mod0_default_parents, 0x080, >> >> BIT(31), /* gate */ >> >> 0); >> >> >> >> +/* >> >> + * MMC clocks are the new timing mode (see A83T & H3) variety, but without >> >> + * the mode switch. This means they have a 2x post divider between the clock >> >> + * and the MMC module. This is not documented in the manual, but is taken >> >> + * into consideration when setting the mmc module clocks in the BSP kernel. >> >> + * Without it, MMC performance is degraded. >> >> + * >> >> + * We model it here to be consistent with other SoCs supporting this mode. >> >> + * The alternative would be to add the 2x multiplier when setting the MMC >> >> + * module clock in the MMC driver, just for the A64. >> >> + */ >> >> static const char * const mmc_default_parents[] = { "osc24M", "pll-periph0-2x", >> >> "pll-periph1-2x" }; >> >> -static SUNXI_CCU_MP_WITH_MUX_GATE(mmc0_clk, "mmc0", mmc_default_parents, 0x088, >> >> - 0, 4, /* M */ >> >> - 16, 2, /* P */ >> >> - 24, 2, /* mux */ >> >> - BIT(31), /* gate */ >> >> - 0); >> >> - >> >> -static SUNXI_CCU_MP_WITH_MUX_GATE(mmc1_clk, "mmc1", mmc_default_parents, 0x08c, >> >> - 0, 4, /* M */ >> >> - 16, 2, /* P */ >> >> - 24, 2, /* mux */ >> >> - BIT(31), /* gate */ >> >> - 0); >> >> - >> >> -static SUNXI_CCU_MP_WITH_MUX_GATE(mmc2_clk, "mmc2", mmc_default_parents, 0x090, >> >> - 0, 4, /* M */ >> >> - 16, 2, /* P */ >> >> - 24, 2, /* mux */ >> >> - BIT(31), /* gate */ >> >> - 0); >> >> +static SUNXI_CCU_MP_WITH_MUX_GATE_POSTDIV(mmc0_clk, "mmc0", >> >> + mmc_default_parents, 0x088, >> >> + 0, 4, /* M */ >> >> + 16, 2, /* P */ >> >> + 24, 2, /* mux */ >> >> + BIT(31), /* gate */ >> >> + 2, /* post-div */ >> >> + 0); >> >> + >> >> +static SUNXI_CCU_MP_WITH_MUX_GATE_POSTDIV(mmc1_clk, "mmc1", >> >> + mmc_default_parents, 0x08c, >> >> + 0, 4, /* M */ >> >> + 16, 2, /* P */ >> >> + 24, 2, /* mux */ >> >> + BIT(31), /* gate */ >> >> + 2, /* post-div */ >> >> + 0); >> >> + >> > >> > Are you sure that the divider there for the non-eMMC clocks? Usually, >> > the new mode is only here for the eMMC, so we would divide the rate by >> > two in the non-eMMC case. >> >> The new mode is there for all MMC controllers. The other two MMC >> controllers even have the old/new timing mode switch. In case you >> forgot we have ".need_new_timings" set for the A64 compatible. > > But then, shouldn't we model them as such, using the work you did on > the A83t clocks? On the A64, the clocks don't have the switch. Only the MMC controller does. On the A83T, both do. ChenYu >> But to eliminate any doubts or concerns, I've rerun tests for the >> micro SD card, instead of the eMMC. And yes the results are the same, >> 2x improvement (12 MB/s vs 23.7 MB/s). > > Ok, good. > > Thanks! > Maxime > > -- > Maxime Ripard, Free Electrons > Embedded Linux and Kernel engineering > http://free-electrons.com
On Thu, Dec 07, 2017 at 12:10:39AM +0800, Chen-Yu Tsai wrote: > On Wed, Dec 6, 2017 at 11:56 PM, Maxime Ripard > <maxime.ripard@free-electrons.com> wrote: > > Hi, > > > > On Wed, Dec 06, 2017 at 10:30:26AM +0800, Chen-Yu Tsai wrote: > >> On Wed, Dec 6, 2017 at 3:59 AM, Maxime Ripard > >> <maxime.ripard@free-electrons.com> wrote: > >> > Hi, > >> > > >> > On Mon, Dec 04, 2017 at 01:19:12PM +0800, Chen-Yu Tsai wrote: > >> >> On the A64, the MMC module clocks are fixed in the new timing mode, > >> >> i.e. they do not have a bit to select the mode. These clocks have > >> >> a 2x divider somewhere between the clock and the MMC module. > >> >> > >> >> To be consistent with other SoCs supporting the new timing mode, > >> >> we model the 2x divider as a fixed post-divider on the MMC module > >> >> clocks. > >> >> > >> >> This patch adds the post-dividers to the MMC clocks. > >> >> > >> >> Signed-off-by: Chen-Yu Tsai <wens@csie.org> > >> > > >> > I had a doubt applying that one... sorry. > >> > > >> >> --- > >> >> drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 57 +++++++++++++++++++++++------------ > >> >> 1 file changed, 37 insertions(+), 20 deletions(-) > >> >> > >> >> diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c > >> >> index 2bb4cabf802f..ee9c12cf3f08 100644 > >> >> --- a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c > >> >> +++ b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c > >> >> @@ -400,28 +400,45 @@ static SUNXI_CCU_MP_WITH_MUX_GATE(nand_clk, "nand", mod0_default_parents, 0x080, > >> >> BIT(31), /* gate */ > >> >> 0); > >> >> > >> >> +/* > >> >> + * MMC clocks are the new timing mode (see A83T & H3) variety, but without > >> >> + * the mode switch. This means they have a 2x post divider between the clock > >> >> + * and the MMC module. This is not documented in the manual, but is taken > >> >> + * into consideration when setting the mmc module clocks in the BSP kernel. > >> >> + * Without it, MMC performance is degraded. > >> >> + * > >> >> + * We model it here to be consistent with other SoCs supporting this mode. > >> >> + * The alternative would be to add the 2x multiplier when setting the MMC > >> >> + * module clock in the MMC driver, just for the A64. > >> >> + */ > >> >> static const char * const mmc_default_parents[] = { "osc24M", "pll-periph0-2x", > >> >> "pll-periph1-2x" }; > >> >> -static SUNXI_CCU_MP_WITH_MUX_GATE(mmc0_clk, "mmc0", mmc_default_parents, 0x088, > >> >> - 0, 4, /* M */ > >> >> - 16, 2, /* P */ > >> >> - 24, 2, /* mux */ > >> >> - BIT(31), /* gate */ > >> >> - 0); > >> >> - > >> >> -static SUNXI_CCU_MP_WITH_MUX_GATE(mmc1_clk, "mmc1", mmc_default_parents, 0x08c, > >> >> - 0, 4, /* M */ > >> >> - 16, 2, /* P */ > >> >> - 24, 2, /* mux */ > >> >> - BIT(31), /* gate */ > >> >> - 0); > >> >> - > >> >> -static SUNXI_CCU_MP_WITH_MUX_GATE(mmc2_clk, "mmc2", mmc_default_parents, 0x090, > >> >> - 0, 4, /* M */ > >> >> - 16, 2, /* P */ > >> >> - 24, 2, /* mux */ > >> >> - BIT(31), /* gate */ > >> >> - 0); > >> >> +static SUNXI_CCU_MP_WITH_MUX_GATE_POSTDIV(mmc0_clk, "mmc0", > >> >> + mmc_default_parents, 0x088, > >> >> + 0, 4, /* M */ > >> >> + 16, 2, /* P */ > >> >> + 24, 2, /* mux */ > >> >> + BIT(31), /* gate */ > >> >> + 2, /* post-div */ > >> >> + 0); > >> >> + > >> >> +static SUNXI_CCU_MP_WITH_MUX_GATE_POSTDIV(mmc1_clk, "mmc1", > >> >> + mmc_default_parents, 0x08c, > >> >> + 0, 4, /* M */ > >> >> + 16, 2, /* P */ > >> >> + 24, 2, /* mux */ > >> >> + BIT(31), /* gate */ > >> >> + 2, /* post-div */ > >> >> + 0); > >> >> + > >> > > >> > Are you sure that the divider there for the non-eMMC clocks? Usually, > >> > the new mode is only here for the eMMC, so we would divide the rate by > >> > two in the non-eMMC case. > >> > >> The new mode is there for all MMC controllers. The other two MMC > >> controllers even have the old/new timing mode switch. In case you > >> forgot we have ".need_new_timings" set for the A64 compatible. > > > > But then, shouldn't we model them as such, using the work you did on > > the A83t clocks? > > On the A64, the clocks don't have the switch. Only the MMC controller > does. On the A83T, both do. Ah, right. I've applied both patches. Maxime
diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c index 2bb4cabf802f..ee9c12cf3f08 100644 --- a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c +++ b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c @@ -400,28 +400,45 @@ static SUNXI_CCU_MP_WITH_MUX_GATE(nand_clk, "nand", mod0_default_parents, 0x080, BIT(31), /* gate */ 0); +/* + * MMC clocks are the new timing mode (see A83T & H3) variety, but without + * the mode switch. This means they have a 2x post divider between the clock + * and the MMC module. This is not documented in the manual, but is taken + * into consideration when setting the mmc module clocks in the BSP kernel. + * Without it, MMC performance is degraded. + * + * We model it here to be consistent with other SoCs supporting this mode. + * The alternative would be to add the 2x multiplier when setting the MMC + * module clock in the MMC driver, just for the A64. + */ static const char * const mmc_default_parents[] = { "osc24M", "pll-periph0-2x", "pll-periph1-2x" }; -static SUNXI_CCU_MP_WITH_MUX_GATE(mmc0_clk, "mmc0", mmc_default_parents, 0x088, - 0, 4, /* M */ - 16, 2, /* P */ - 24, 2, /* mux */ - BIT(31), /* gate */ - 0); - -static SUNXI_CCU_MP_WITH_MUX_GATE(mmc1_clk, "mmc1", mmc_default_parents, 0x08c, - 0, 4, /* M */ - 16, 2, /* P */ - 24, 2, /* mux */ - BIT(31), /* gate */ - 0); - -static SUNXI_CCU_MP_WITH_MUX_GATE(mmc2_clk, "mmc2", mmc_default_parents, 0x090, - 0, 4, /* M */ - 16, 2, /* P */ - 24, 2, /* mux */ - BIT(31), /* gate */ - 0); +static SUNXI_CCU_MP_WITH_MUX_GATE_POSTDIV(mmc0_clk, "mmc0", + mmc_default_parents, 0x088, + 0, 4, /* M */ + 16, 2, /* P */ + 24, 2, /* mux */ + BIT(31), /* gate */ + 2, /* post-div */ + 0); + +static SUNXI_CCU_MP_WITH_MUX_GATE_POSTDIV(mmc1_clk, "mmc1", + mmc_default_parents, 0x08c, + 0, 4, /* M */ + 16, 2, /* P */ + 24, 2, /* mux */ + BIT(31), /* gate */ + 2, /* post-div */ + 0); + +static SUNXI_CCU_MP_WITH_MUX_GATE_POSTDIV(mmc2_clk, "mmc2", + mmc_default_parents, 0x090, + 0, 4, /* M */ + 16, 2, /* P */ + 24, 2, /* mux */ + BIT(31), /* gate */ + 2, /* post-div */ + 0); static const char * const ts_parents[] = { "osc24M", "pll-periph0", }; static SUNXI_CCU_MP_WITH_MUX_GATE(ts_clk, "ts", ts_parents, 0x098,
On the A64, the MMC module clocks are fixed in the new timing mode, i.e. they do not have a bit to select the mode. These clocks have a 2x divider somewhere between the clock and the MMC module. To be consistent with other SoCs supporting the new timing mode, we model the 2x divider as a fixed post-divider on the MMC module clocks. This patch adds the post-dividers to the MMC clocks. Signed-off-by: Chen-Yu Tsai <wens@csie.org> --- drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 57 +++++++++++++++++++++++------------ 1 file changed, 37 insertions(+), 20 deletions(-)