Re: [PATCH 2/2] clk: sunxi-ng: sun50i: a64: Add 2x fixed post-divider to MMC module clocks

[Maxime Ripard]

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
>  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
> >>  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 
> >> >
> >> > 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 

Re: [PATCH 2/2] clk: sunxi-ng: sun50i: a64: Add 2x fixed post-divider to MMC module clocks

[Maxime Ripard]

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
>  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
> >>  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 
> >> >
> >> > 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 

Re: [PATCH 2/2] clk: sunxi-ng: sun50i: a64: Add 2x fixed post-divider to MMC module clocks

[Chen-Yu Tsai]

On Wed, Dec 6, 2017 at 11:56 PM, Maxime Ripard
 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
>>  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 
>> >
>> > 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 

Re: [PATCH 2/2] clk: sunxi-ng: sun50i: a64: Add 2x fixed post-divider to MMC module clocks

[Chen-Yu Tsai]

On Wed, Dec 6, 2017 at 11:56 PM, Maxime Ripard
 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
>>  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 
>> >
>> > 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 

Re: [PATCH 2/2] clk: sunxi-ng: sun50i: a64: Add 2x fixed post-divider to MMC module clocks

[Maxime Ripard]

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
>  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 
> >
> > 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 

Re: [PATCH 2/2] clk: sunxi-ng: sun50i: a64: Add 2x fixed post-divider to MMC module clocks

[Maxime Ripard]

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
>  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 
> >
> > 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 

Re: [PATCH 2/2] clk: sunxi-ng: sun50i: a64: Add 2x fixed post-divider to MMC module clocks

[Chen-Yu Tsai]

On Wed, Dec 6, 2017 at 3:59 AM, Maxime Ripard
 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 
>
> 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

Re: [PATCH 2/2] clk: sunxi-ng: sun50i: a64: Add 2x fixed post-divider to MMC module clocks

[Chen-Yu Tsai]

On Wed, Dec 6, 2017 at 3:59 AM, Maxime Ripard
 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 
>
> 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

Re: [PATCH 2/2] clk: sunxi-ng: sun50i: a64: Add 2x fixed post-divider to MMC module clocks

[Maxime Ripard]

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 

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

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature

Re: [PATCH 2/2] clk: sunxi-ng: sun50i: a64: Add 2x fixed post-divider to MMC module clocks

[Maxime Ripard]

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 

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

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature

Re: [linux-sunxi] [PATCH 2/2] clk: sunxi-ng: sun50i: a64: Add 2x fixed post-divider to MMC module clocks

[André Przywara]

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 

Reviewed-by: Andre Przywara 

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,
> 

Re: [linux-sunxi] [PATCH 2/2] clk: sunxi-ng: sun50i: a64: Add 2x fixed post-divider to MMC module clocks

[André Przywara]

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 

Reviewed-by: Andre Przywara 

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,
> 

[PATCH 2/2] clk: sunxi-ng: sun50i: a64: Add 2x fixed post-divider to MMC module clocks

[Chen-Yu Tsai]

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 
---
 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,
-- 
2.15.0

[PATCH 2/2] clk: sunxi-ng: sun50i: a64: Add 2x fixed post-divider to MMC module clocks

[Chen-Yu Tsai]

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 
---
 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,
-- 
2.15.0