All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] clk: divider: Add CLK_DIVIDER_EVEN flag support
@ 2018-04-05  5:38 ` Shawn Lin
  0 siblings, 0 replies; 12+ messages in thread
From: Shawn Lin @ 2018-04-05  5:38 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd, Heiko Stuebner
  Cc: linux-rockchip, linux-clk, Jeffy Chen, Finley Xiao, Andy Yan,
	Xing Zheng, Elaine Zhang, Ziyuan Xu, Shawn Lin

CLK_DIVIDER_EVEN is used for clock divders that should only
use even number in the div field.

Two clock divder should consider to use this flag
(1) The divder is physically only support even div number to
generate 50% duty cycle clock rate.
(2) The divder's clock consumer request it to use even div number
to generate the most closest requested rate for whatever reason.

In some platforms, for instance Rockchip, the eMMC/SDIO/SDMMC should
request divder to use even number when working at a high throughput
speed mode reliably. However, that wasn't guaranteed by clock framework.
So the previous tricky is to carefully assign magic clock rate to their
parents as well as consumer's clock rate when requesting. That works
bad in practice if folks change the parents clock rate or the clock
hierarchy randomly. That also work bad if the consumer's clock rate
came from the DT, which is changed so fraquent for different boards.

To make it's less prone to make mistake and to make it really respect
the fact that the divder should use even number to the div field, we
need the clock framework's help. Now we have CLK_DIVIDER_POWER_OF_TWO,
which could guarantee the div field is even number, however, obviously
it skips the even numbers which is the power of 2, but maybe which is
the best div to generate closest clock rate for consumer.

Look at the examples here when doing some random test on Rockchip's
platforms, by changing the requested clock rate from DT, namely
assigning different max-frquency for MMC node,

when the mmc host driver requests 80MHz with CLK_DIVIDER_POWER_OF_TWO
flag for the clock divder, it shows the final clock rate is 61.44Mhz

pll_vpll0		1		1 983039999     0   0
   vpll0		4		4 983039999     0   0
     clk_emmc_div       1		1 122880000     0   0
	clk_emmc	1		1 122880000     0   0
	  emmc_sample	0		0  61440000	0 155
	  emmc_drv	0		0  61440000	0 180

With this patch and add CLK_DIVIDER_EVEN flag for clk_emmc_div, we get
the final clock rate, 67.7376MHz.

pll_vpll1               1		1 812851199	0   0
   vpll1                2		2 812851199     0   0
     clk_emmc_div       1		1 135475200     0   0
	clk_emmc	1		1 135475200     0   0
	  emmc_sample   0		0  67737600     0 113
	  emmc_drv	0		0  67737600	0 180

Apprently 67737600 is better than 61440000 when requesting 80MHz.
Of course, we could have more case that worsen the gap between
the desired rate and the actual rate if using CLK_DIVIDER_POWER_OF_TWO.

So this introduces CLK_DIVIDER_EVEN flag for clock framework to make
the best in this process.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/clk/clk-divider.c    | 9 +++++++++
 include/linux/clk-provider.h | 3 +++
 2 files changed, 12 insertions(+)

diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
index b6234a5..97f7b94 100644
--- a/drivers/clk/clk-divider.c
+++ b/drivers/clk/clk-divider.c
@@ -161,6 +161,8 @@ static bool _is_valid_div(const struct clk_div_table *table, unsigned int div,
 {
 	if (flags & CLK_DIVIDER_POWER_OF_TWO)
 		return is_power_of_2(div);
+	if (flags & CLK_DIVIDER_EVEN)
+		return !(div % 2);
 	if (table)
 		return _is_valid_table_div(table, div);
 	return true;
@@ -210,6 +212,8 @@ static int _div_round_up(const struct clk_div_table *table,
 
 	if (flags & CLK_DIVIDER_POWER_OF_TWO)
 		div = __roundup_pow_of_two(div);
+	if (flags & CLK_DIVIDER_EVEN)
+		div = !(div % 2) ? div : (div + 1);
 	if (table)
 		div = _round_up_table(table, div);
 
@@ -229,6 +233,9 @@ static int _div_round_closest(const struct clk_div_table *table,
 	if (flags & CLK_DIVIDER_POWER_OF_TWO) {
 		up = __roundup_pow_of_two(up);
 		down = __rounddown_pow_of_two(down);
+	} else if (flags & CLK_DIVIDER_EVEN) {
+		up = !(up % 2) ? up : (up + 1);
+		down = !(down % 2) ? down : (down - 1);
 	} else if (table) {
 		up = _round_up_table(table, up);
 		down = _round_down_table(table, down);
@@ -266,6 +273,8 @@ static int _next_div(const struct clk_div_table *table, int div,
 
 	if (flags & CLK_DIVIDER_POWER_OF_TWO)
 		return __roundup_pow_of_two(div);
+	if (flags & CLK_DIVIDER_EVEN)
+		return !(div % 2) ? div : (div + 1);
 	if (table)
 		return _round_up_table(table, div);
 
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 210a890..7c59611 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -388,6 +388,8 @@ struct clk_div_table {
  * CLK_DIVIDER_MAX_AT_ZERO - For dividers which are like CLK_DIVIDER_ONE_BASED
  *	except when the value read from the register is zero, the divisor is
  *	2^width of the field.
+ * CLK_DIVIDER_EVEN - For the dividers which could only use even number in the
+ *	div field.
  */
 struct clk_divider {
 	struct clk_hw	hw;
@@ -409,6 +411,7 @@ struct clk_divider {
 #define CLK_DIVIDER_ROUND_CLOSEST	BIT(4)
 #define CLK_DIVIDER_READ_ONLY		BIT(5)
 #define CLK_DIVIDER_MAX_AT_ZERO		BIT(6)
+#define CLK_DIVIDER_EVEN		BIT(7)
 
 extern const struct clk_ops clk_divider_ops;
 extern const struct clk_ops clk_divider_ro_ops;
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 1/2] clk: divider: Add CLK_DIVIDER_EVEN flag support
@ 2018-04-05  5:38 ` Shawn Lin
  0 siblings, 0 replies; 12+ messages in thread
From: Shawn Lin @ 2018-04-05  5:38 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd, Heiko Stuebner
  Cc: Elaine Zhang, Shawn Lin, Jeffy Chen,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Finley Xiao,
	Xing Zheng, Andy Yan, linux-clk-u79uwXL29TY76Z2rM5mHXA,
	Ziyuan Xu

CLK_DIVIDER_EVEN is used for clock divders that should only
use even number in the div field.

Two clock divder should consider to use this flag
(1) The divder is physically only support even div number to
generate 50% duty cycle clock rate.
(2) The divder's clock consumer request it to use even div number
to generate the most closest requested rate for whatever reason.

In some platforms, for instance Rockchip, the eMMC/SDIO/SDMMC should
request divder to use even number when working at a high throughput
speed mode reliably. However, that wasn't guaranteed by clock framework.
So the previous tricky is to carefully assign magic clock rate to their
parents as well as consumer's clock rate when requesting. That works
bad in practice if folks change the parents clock rate or the clock
hierarchy randomly. That also work bad if the consumer's clock rate
came from the DT, which is changed so fraquent for different boards.

To make it's less prone to make mistake and to make it really respect
the fact that the divder should use even number to the div field, we
need the clock framework's help. Now we have CLK_DIVIDER_POWER_OF_TWO,
which could guarantee the div field is even number, however, obviously
it skips the even numbers which is the power of 2, but maybe which is
the best div to generate closest clock rate for consumer.

Look at the examples here when doing some random test on Rockchip's
platforms, by changing the requested clock rate from DT, namely
assigning different max-frquency for MMC node,

when the mmc host driver requests 80MHz with CLK_DIVIDER_POWER_OF_TWO
flag for the clock divder, it shows the final clock rate is 61.44Mhz

pll_vpll0		1		1 983039999     0   0
   vpll0		4		4 983039999     0   0
     clk_emmc_div       1		1 122880000     0   0
	clk_emmc	1		1 122880000     0   0
	  emmc_sample	0		0  61440000	0 155
	  emmc_drv	0		0  61440000	0 180

With this patch and add CLK_DIVIDER_EVEN flag for clk_emmc_div, we get
the final clock rate, 67.7376MHz.

pll_vpll1               1		1 812851199	0   0
   vpll1                2		2 812851199     0   0
     clk_emmc_div       1		1 135475200     0   0
	clk_emmc	1		1 135475200     0   0
	  emmc_sample   0		0  67737600     0 113
	  emmc_drv	0		0  67737600	0 180

Apprently 67737600 is better than 61440000 when requesting 80MHz.
Of course, we could have more case that worsen the gap between
the desired rate and the actual rate if using CLK_DIVIDER_POWER_OF_TWO.

So this introduces CLK_DIVIDER_EVEN flag for clock framework to make
the best in this process.

Signed-off-by: Shawn Lin <shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
---

 drivers/clk/clk-divider.c    | 9 +++++++++
 include/linux/clk-provider.h | 3 +++
 2 files changed, 12 insertions(+)

diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
index b6234a5..97f7b94 100644
--- a/drivers/clk/clk-divider.c
+++ b/drivers/clk/clk-divider.c
@@ -161,6 +161,8 @@ static bool _is_valid_div(const struct clk_div_table *table, unsigned int div,
 {
 	if (flags & CLK_DIVIDER_POWER_OF_TWO)
 		return is_power_of_2(div);
+	if (flags & CLK_DIVIDER_EVEN)
+		return !(div % 2);
 	if (table)
 		return _is_valid_table_div(table, div);
 	return true;
@@ -210,6 +212,8 @@ static int _div_round_up(const struct clk_div_table *table,
 
 	if (flags & CLK_DIVIDER_POWER_OF_TWO)
 		div = __roundup_pow_of_two(div);
+	if (flags & CLK_DIVIDER_EVEN)
+		div = !(div % 2) ? div : (div + 1);
 	if (table)
 		div = _round_up_table(table, div);
 
@@ -229,6 +233,9 @@ static int _div_round_closest(const struct clk_div_table *table,
 	if (flags & CLK_DIVIDER_POWER_OF_TWO) {
 		up = __roundup_pow_of_two(up);
 		down = __rounddown_pow_of_two(down);
+	} else if (flags & CLK_DIVIDER_EVEN) {
+		up = !(up % 2) ? up : (up + 1);
+		down = !(down % 2) ? down : (down - 1);
 	} else if (table) {
 		up = _round_up_table(table, up);
 		down = _round_down_table(table, down);
@@ -266,6 +273,8 @@ static int _next_div(const struct clk_div_table *table, int div,
 
 	if (flags & CLK_DIVIDER_POWER_OF_TWO)
 		return __roundup_pow_of_two(div);
+	if (flags & CLK_DIVIDER_EVEN)
+		return !(div % 2) ? div : (div + 1);
 	if (table)
 		return _round_up_table(table, div);
 
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 210a890..7c59611 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -388,6 +388,8 @@ struct clk_div_table {
  * CLK_DIVIDER_MAX_AT_ZERO - For dividers which are like CLK_DIVIDER_ONE_BASED
  *	except when the value read from the register is zero, the divisor is
  *	2^width of the field.
+ * CLK_DIVIDER_EVEN - For the dividers which could only use even number in the
+ *	div field.
  */
 struct clk_divider {
 	struct clk_hw	hw;
@@ -409,6 +411,7 @@ struct clk_divider {
 #define CLK_DIVIDER_ROUND_CLOSEST	BIT(4)
 #define CLK_DIVIDER_READ_ONLY		BIT(5)
 #define CLK_DIVIDER_MAX_AT_ZERO		BIT(6)
+#define CLK_DIVIDER_EVEN		BIT(7)
 
 extern const struct clk_ops clk_divider_ops;
 extern const struct clk_ops clk_divider_ro_ops;
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/2] clk: rockchip: Add CLK_DIVIDER_EVEN for all mmc clock consumers
@ 2018-04-05  5:38   ` Shawn Lin
  0 siblings, 0 replies; 12+ messages in thread
From: Shawn Lin @ 2018-04-05  5:38 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd, Heiko Stuebner
  Cc: linux-rockchip, linux-clk, Jeffy Chen, Finley Xiao, Andy Yan,
	Xing Zheng, Elaine Zhang, Ziyuan Xu, Shawn Lin

For conventional Rockchip platforms, mmc hosts, except RK3399's
eMMC controller, should ask their clock parents to provider the
closest clock rate with a even div number. That wasn't done
explicitly by the clock framework. but now we have CLK_DIVIDER_EVEN
flag to help.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/clk/rockchip/clk-rk3036.c |  7 ++++---
 drivers/clk/rockchip/clk-rk3128.c |  9 ++++++---
 drivers/clk/rockchip/clk-rk3188.c |  6 +++---
 drivers/clk/rockchip/clk-rk3228.c |  7 ++++---
 drivers/clk/rockchip/clk-rk3288.c | 12 ++++++++----
 drivers/clk/rockchip/clk-rk3328.c |  9 ++++++---
 drivers/clk/rockchip/clk-rk3368.c |  9 ++++++---
 drivers/clk/rockchip/clk-rk3399.c |  6 ++++--
 drivers/clk/rockchip/clk-rv1108.c |  7 ++++---
 9 files changed, 45 insertions(+), 27 deletions(-)

diff --git a/drivers/clk/rockchip/clk-rk3036.c b/drivers/clk/rockchip/clk-rk3036.c
index c300198..155fd4f 100644
--- a/drivers/clk/rockchip/clk-rk3036.c
+++ b/drivers/clk/rockchip/clk-rk3036.c
@@ -290,16 +290,17 @@ enum rk3036_plls {
 			RK2928_CLKSEL_CON(12), 8, 2, MFLAGS,
 			RK2928_CLKGATE_CON(2), 11, GFLAGS),
 	DIV(SCLK_SDMMC, "sclk_sdmmc", "sclk_sdmmc_src", 0,
-			RK2928_CLKSEL_CON(11), 0, 7, DFLAGS),
+			RK2928_CLKSEL_CON(11), 0, 7, DFLAGS | CLK_DIVIDER_EVEN),
 
 	COMPOSITE_NODIV(0, "sclk_sdio_src", mux_mmc_src_p, 0,
 			RK2928_CLKSEL_CON(12), 10, 2, MFLAGS,
 			RK2928_CLKGATE_CON(2), 13, GFLAGS),
 	DIV(SCLK_SDIO, "sclk_sdio", "sclk_sdio_src", 0,
-			RK2928_CLKSEL_CON(11), 8, 7, DFLAGS),
+			RK2928_CLKSEL_CON(11), 8, 7, DFLAGS | CLK_DIVIDER_EVEN),
 
 	COMPOSITE(SCLK_EMMC, "sclk_emmc", mux_mmc_src_p, 0,
-			RK2928_CLKSEL_CON(12), 12, 2, MFLAGS, 0, 7, DFLAGS,
+			RK2928_CLKSEL_CON(12), 12, 2, MFLAGS, 0, 7,
+			DFLAGS | CLK_DIVIDER_EVEN,
 			RK2928_CLKGATE_CON(2), 14, GFLAGS),
 
 	MMC(SCLK_SDMMC_DRV,    "sdmmc_drv",    "sclk_sdmmc", RK3036_SDMMC_CON0, 1),
diff --git a/drivers/clk/rockchip/clk-rk3128.c b/drivers/clk/rockchip/clk-rk3128.c
index 5970a50..7c6e30a 100644
--- a/drivers/clk/rockchip/clk-rk3128.c
+++ b/drivers/clk/rockchip/clk-rk3128.c
@@ -324,15 +324,18 @@ enum rk3128_plls {
 			RK2928_CLKGATE_CON(2), 15, GFLAGS),
 
 	COMPOSITE(SCLK_SDMMC, "sclk_sdmmc0", mux_mmc_src_p, 0,
-			RK2928_CLKSEL_CON(11), 6, 2, MFLAGS, 0, 6, DFLAGS,
+			RK2928_CLKSEL_CON(11), 6, 2, MFLAGS, 0, 6,
+			DFLAGS | CLK_DIVIDER_EVEN,
 			RK2928_CLKGATE_CON(2), 11, GFLAGS),
 
 	COMPOSITE(SCLK_SDIO, "sclk_sdio", mux_mmc_src_p, 0,
-			RK2928_CLKSEL_CON(12), 6, 2, MFLAGS, 0, 6, DFLAGS,
+			RK2928_CLKSEL_CON(12), 6, 2, MFLAGS, 0, 6,
+			DFLAGS | CLK_DIVIDER_EVEN,
 			RK2928_CLKGATE_CON(2), 13, GFLAGS),
 
 	COMPOSITE(SCLK_EMMC, "sclk_emmc", mux_mmc_src_p, 0,
-			RK2928_CLKSEL_CON(12), 14, 2, MFLAGS, 8, 6, DFLAGS,
+			RK2928_CLKSEL_CON(12), 14, 2, MFLAGS, 8, 6,
+			DFLAGS | CLK_DIVIDER_EVEN,
 			RK2928_CLKGATE_CON(2), 14, GFLAGS),
 
 	DIV(SCLK_PVTM, "clk_pvtm", "clk_pvtm_func", 0,
diff --git a/drivers/clk/rockchip/clk-rk3188.c b/drivers/clk/rockchip/clk-rk3188.c
index 67e73fd..32f199d 100644
--- a/drivers/clk/rockchip/clk-rk3188.c
+++ b/drivers/clk/rockchip/clk-rk3188.c
@@ -402,13 +402,13 @@ enum rk3188_plls {
 			RK2928_CLKGATE_CON(2), 10, GFLAGS),
 
 	COMPOSITE_NOMUX(SCLK_SDMMC, "sclk_sdmmc", "hclk_peri", 0,
-			RK2928_CLKSEL_CON(11), 0, 6, DFLAGS,
+			RK2928_CLKSEL_CON(11), 0, 6, DFLAGS | CLK_DIVIDER_EVEN,
 			RK2928_CLKGATE_CON(2), 11, GFLAGS),
 	COMPOSITE_NOMUX(SCLK_SDIO, "sclk_sdio", "hclk_peri", 0,
-			RK2928_CLKSEL_CON(12), 0, 6, DFLAGS,
+			RK2928_CLKSEL_CON(12), 0, 6, DFLAGS | CLK_DIVIDER_EVEN,
 			RK2928_CLKGATE_CON(2), 13, GFLAGS),
 	COMPOSITE_NOMUX(SCLK_EMMC, "sclk_emmc", "hclk_peri", 0,
-			RK2928_CLKSEL_CON(12), 8, 6, DFLAGS,
+			RK2928_CLKSEL_CON(12), 8, 6, DFLAGS | CLK_DIVIDER_EVEN,
 			RK2928_CLKGATE_CON(2), 14, GFLAGS),
 
 	MUX(0, "uart_src", mux_pll_src_gpll_cpll_p, 0,
diff --git a/drivers/clk/rockchip/clk-rk3228.c b/drivers/clk/rockchip/clk-rk3228.c
index 7af4818..15da16a 100644
--- a/drivers/clk/rockchip/clk-rk3228.c
+++ b/drivers/clk/rockchip/clk-rk3228.c
@@ -388,20 +388,21 @@ enum rk3228_plls {
 			RK2928_CLKGATE_CON(2), 15, GFLAGS),
 
 	COMPOSITE(SCLK_SDMMC, "sclk_sdmmc", mux_mmc_src_p, 0,
-			RK2928_CLKSEL_CON(11), 8, 2, MFLAGS, 0, 8, DFLAGS,
+			RK2928_CLKSEL_CON(11), 8, 2, MFLAGS, 0, 8,
+			DFLAGS | CLK_DIVIDER_EVEN,
 			RK2928_CLKGATE_CON(2), 11, GFLAGS),
 
 	COMPOSITE_NODIV(SCLK_SDIO_SRC, "sclk_sdio_src", mux_mmc_src_p, 0,
 			RK2928_CLKSEL_CON(11), 10, 2, MFLAGS,
 			RK2928_CLKGATE_CON(2), 13, GFLAGS),
 	DIV(SCLK_SDIO, "sclk_sdio", "sclk_sdio_src", 0,
-			RK2928_CLKSEL_CON(12), 0, 8, DFLAGS),
+			RK2928_CLKSEL_CON(12), 0, 8, DFLAGS | CLK_DIVIDER_EVEN),
 
 	COMPOSITE_NODIV(0, "sclk_emmc_src", mux_mmc_src_p, 0,
 			RK2928_CLKSEL_CON(11), 12, 2, MFLAGS,
 			RK2928_CLKGATE_CON(2), 14, GFLAGS),
 	DIV(SCLK_EMMC, "sclk_emmc", "sclk_emmc_src", 0,
-			RK2928_CLKSEL_CON(12), 8, 8, DFLAGS),
+			RK2928_CLKSEL_CON(12), 8, 8, DFLAGS | CLK_DIVIDER_EVEN),
 
 	/*
 	 * Clock-Architecture Diagram 2
diff --git a/drivers/clk/rockchip/clk-rk3288.c b/drivers/clk/rockchip/clk-rk3288.c
index 450de24..2a6f231 100644
--- a/drivers/clk/rockchip/clk-rk3288.c
+++ b/drivers/clk/rockchip/clk-rk3288.c
@@ -508,16 +508,20 @@ enum rk3288_plls {
 			RK3288_CLKGATE_CON(2), 11, GFLAGS),
 
 	COMPOSITE(SCLK_SDMMC, "sclk_sdmmc", mux_mmc_src_p, 0,
-			RK3288_CLKSEL_CON(11), 6, 2, MFLAGS, 0, 6, DFLAGS,
+			RK3288_CLKSEL_CON(11), 6, 2, MFLAGS, 0, 6,
+			DFLAGS | CLK_DIVIDER_EVEN,
 			RK3288_CLKGATE_CON(13), 0, GFLAGS),
 	COMPOSITE(SCLK_SDIO0, "sclk_sdio0", mux_mmc_src_p, 0,
-			RK3288_CLKSEL_CON(12), 6, 2, MFLAGS, 0, 6, DFLAGS,
+			RK3288_CLKSEL_CON(12), 6, 2, MFLAGS, 0, 6,
+			DFLAGS | CLK_DIVIDER_EVEN,
 			RK3288_CLKGATE_CON(13), 1, GFLAGS),
 	COMPOSITE(SCLK_SDIO1, "sclk_sdio1", mux_mmc_src_p, 0,
-			RK3288_CLKSEL_CON(34), 14, 2, MFLAGS, 8, 6, DFLAGS,
+			RK3288_CLKSEL_CON(34), 14, 2, MFLAGS, 8, 6,
+			DFLAGS | CLK_DIVIDER_EVEN,
 			RK3288_CLKGATE_CON(13), 2, GFLAGS),
 	COMPOSITE(SCLK_EMMC, "sclk_emmc", mux_mmc_src_p, 0,
-			RK3288_CLKSEL_CON(12), 14, 2, MFLAGS, 8, 6, DFLAGS,
+			RK3288_CLKSEL_CON(12), 14, 2, MFLAGS, 8, 6,
+			DFLAGS | CLK_DIVIDER_EVEN,
 			RK3288_CLKGATE_CON(13), 3, GFLAGS),
 
 	MMC(SCLK_SDMMC_DRV,    "sdmmc_drv",    "sclk_sdmmc", RK3288_SDMMC_CON0, 1),
diff --git a/drivers/clk/rockchip/clk-rk3328.c b/drivers/clk/rockchip/clk-rk3328.c
index 252366a..f81acf8 100644
--- a/drivers/clk/rockchip/clk-rk3328.c
+++ b/drivers/clk/rockchip/clk-rk3328.c
@@ -632,15 +632,18 @@ enum rk3328_plls {
 			RK3328_CLKGATE_CON(4), 3, GFLAGS),
 
 	COMPOSITE(SCLK_SDIO, "clk_sdio", mux_2plls_24m_u480m_p, 0,
-			RK3328_CLKSEL_CON(31), 8, 2, MFLAGS, 0, 8, DFLAGS,
+			RK3328_CLKSEL_CON(31), 8, 2, MFLAGS, 0, 8,
+			DFLAGS | CLK_DIVIDER_EVEN,
 			RK3328_CLKGATE_CON(4), 4, GFLAGS),
 
 	COMPOSITE(SCLK_EMMC, "clk_emmc", mux_2plls_24m_u480m_p, 0,
-			RK3328_CLKSEL_CON(32), 8, 2, MFLAGS, 0, 8, DFLAGS,
+			RK3328_CLKSEL_CON(32), 8, 2, MFLAGS, 0, 8,
+			DFLAGS | CLK_DIVIDER_EVEN,
 			RK3328_CLKGATE_CON(4), 5, GFLAGS),
 
 	COMPOSITE(SCLK_SDMMC_EXT, "clk_sdmmc_ext", mux_2plls_24m_u480m_p, 0,
-			RK3328_CLKSEL_CON(43), 8, 2, MFLAGS, 0, 8, DFLAGS,
+			RK3328_CLKSEL_CON(43), 8, 2, MFLAGS, 0, 8,
+			DFLAGS | CLK_DIVIDER_EVEN,
 			RK3328_CLKGATE_CON(4), 10, GFLAGS),
 
 	COMPOSITE(SCLK_REF_USB3OTG_SRC, "clk_ref_usb3otg_src", mux_2plls_p, 0,
diff --git a/drivers/clk/rockchip/clk-rk3368.c b/drivers/clk/rockchip/clk-rk3368.c
index 7c4d242..17a6293 100644
--- a/drivers/clk/rockchip/clk-rk3368.c
+++ b/drivers/clk/rockchip/clk-rk3368.c
@@ -550,13 +550,16 @@ enum rk3368_plls {
 
 
 	COMPOSITE(SCLK_SDMMC, "sclk_sdmmc", mux_mmc_src_p, 0,
-			RK3368_CLKSEL_CON(50), 8, 2, MFLAGS, 0, 7, DFLAGS,
+			RK3368_CLKSEL_CON(50), 8, 2, MFLAGS, 0, 7,
+			DFLAGS | CLK_DIVIDER_EVEN,
 			RK3368_CLKGATE_CON(7), 12, GFLAGS),
 	COMPOSITE(SCLK_SDIO0, "sclk_sdio0", mux_mmc_src_p, 0,
-			RK3368_CLKSEL_CON(48), 8, 2, MFLAGS, 0, 7, DFLAGS,
+			RK3368_CLKSEL_CON(48), 8, 2, MFLAGS, 0, 7,
+			DFLAGS | CLK_DIVIDER_EVEN,
 			RK3368_CLKGATE_CON(7), 13, GFLAGS),
 	COMPOSITE(SCLK_EMMC, "sclk_emmc", mux_mmc_src_p, 0,
-			RK3368_CLKSEL_CON(51), 8, 2, MFLAGS, 0, 7, DFLAGS,
+			RK3368_CLKSEL_CON(51), 8, 2, MFLAGS, 0, 7,
+			DFLAGS | CLK_DIVIDER_EVEN,
 			RK3368_CLKGATE_CON(7), 15, GFLAGS),
 
 	MMC(SCLK_SDMMC_DRV,    "sdmmc_drv",    "sclk_sdmmc", RK3368_SDMMC_CON0, 1),
diff --git a/drivers/clk/rockchip/clk-rk3399.c b/drivers/clk/rockchip/clk-rk3399.c
index 118759d..aedb3d0 100644
--- a/drivers/clk/rockchip/clk-rk3399.c
+++ b/drivers/clk/rockchip/clk-rk3399.c
@@ -896,11 +896,13 @@ enum rk3399_pmu_plls {
 			RK3399_CLKGATE_CON(33), 9, GFLAGS),
 
 	COMPOSITE(SCLK_SDIO, "clk_sdio", mux_pll_src_cpll_gpll_npll_ppll_upll_24m_p, 0,
-			RK3399_CLKSEL_CON(15), 8, 3, MFLAGS, 0, 7, DFLAGS,
+			RK3399_CLKSEL_CON(15), 8, 3, MFLAGS, 0, 7,
+			DFLAGS | CLK_DIVIDER_EVEN,
 			RK3399_CLKGATE_CON(6), 0, GFLAGS),
 
 	COMPOSITE(SCLK_SDMMC, "clk_sdmmc", mux_pll_src_cpll_gpll_npll_ppll_upll_24m_p, 0,
-			RK3399_CLKSEL_CON(16), 8, 3, MFLAGS, 0, 7, DFLAGS,
+			RK3399_CLKSEL_CON(16), 8, 3, MFLAGS, 0, 7,
+			DFLAGS | CLK_DIVIDER_EVEN,
 			RK3399_CLKGATE_CON(6), 1, GFLAGS),
 
 	MMC(SCLK_SDMMC_DRV,     "sdmmc_drv",    "clk_sdmmc", RK3399_SDMMC_CON0, 1),
diff --git a/drivers/clk/rockchip/clk-rv1108.c b/drivers/clk/rockchip/clk-rv1108.c
index 089cb17..b63af26 100644
--- a/drivers/clk/rockchip/clk-rv1108.c
+++ b/drivers/clk/rockchip/clk-rv1108.c
@@ -721,20 +721,21 @@ enum rv1108_plls {
 			RV1108_CLKGATE_CON(15), 11, GFLAGS),
 
 	COMPOSITE(SCLK_SDMMC, "sclk_sdmmc", mux_mmc_src_p, 0,
-			RV1108_CLKSEL_CON(25), 8, 2, MFLAGS, 0, 8, DFLAGS,
+			RV1108_CLKSEL_CON(25), 8, 2, MFLAGS, 0, 8,
+			DFLAGS | CLK_DIVIDER_EVEN,
 			RV1108_CLKGATE_CON(5), 0, GFLAGS),
 
 	COMPOSITE_NODIV(0, "sclk_sdio_src", mux_mmc_src_p, 0,
 			RV1108_CLKSEL_CON(25), 10, 2, MFLAGS,
 			RV1108_CLKGATE_CON(5), 2, GFLAGS),
 	DIV(SCLK_SDIO, "sclk_sdio", "sclk_sdio_src", 0,
-			RV1108_CLKSEL_CON(26), 0, 8, DFLAGS),
+			RV1108_CLKSEL_CON(26), 0, 8, DFLAGS | CLK_DIVIDER_EVEN),
 
 	COMPOSITE_NODIV(0, "sclk_emmc_src", mux_mmc_src_p, 0,
 			RV1108_CLKSEL_CON(25), 12, 2, MFLAGS,
 			RV1108_CLKGATE_CON(5), 1, GFLAGS),
 	DIV(SCLK_EMMC, "sclk_emmc", "sclk_emmc_src", 0,
-			RK2928_CLKSEL_CON(26), 8, 8, DFLAGS),
+			RK2928_CLKSEL_CON(26), 8, 8, DFLAGS | CLK_DIVIDER_EVEN),
 	GATE(HCLK_SDMMC, "hclk_sdmmc", "hclk_periph", 0, RV1108_CLKGATE_CON(15), 0, GFLAGS),
 	GATE(HCLK_SDIO, "hclk_sdio", "hclk_periph", 0, RV1108_CLKGATE_CON(15), 1, GFLAGS),
 	GATE(HCLK_EMMC, "hclk_emmc", "hclk_periph", 0, RV1108_CLKGATE_CON(15), 2, GFLAGS),
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/2] clk: rockchip: Add CLK_DIVIDER_EVEN for all mmc clock consumers
@ 2018-04-05  5:38   ` Shawn Lin
  0 siblings, 0 replies; 12+ messages in thread
From: Shawn Lin @ 2018-04-05  5:38 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd, Heiko Stuebner
  Cc: Elaine Zhang, Shawn Lin, Jeffy Chen,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Finley Xiao,
	Xing Zheng, Andy Yan, linux-clk-u79uwXL29TY76Z2rM5mHXA,
	Ziyuan Xu

For conventional Rockchip platforms, mmc hosts, except RK3399's
eMMC controller, should ask their clock parents to provider the
closest clock rate with a even div number. That wasn't done
explicitly by the clock framework. but now we have CLK_DIVIDER_EVEN
flag to help.

Signed-off-by: Shawn Lin <shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
---

 drivers/clk/rockchip/clk-rk3036.c |  7 ++++---
 drivers/clk/rockchip/clk-rk3128.c |  9 ++++++---
 drivers/clk/rockchip/clk-rk3188.c |  6 +++---
 drivers/clk/rockchip/clk-rk3228.c |  7 ++++---
 drivers/clk/rockchip/clk-rk3288.c | 12 ++++++++----
 drivers/clk/rockchip/clk-rk3328.c |  9 ++++++---
 drivers/clk/rockchip/clk-rk3368.c |  9 ++++++---
 drivers/clk/rockchip/clk-rk3399.c |  6 ++++--
 drivers/clk/rockchip/clk-rv1108.c |  7 ++++---
 9 files changed, 45 insertions(+), 27 deletions(-)

diff --git a/drivers/clk/rockchip/clk-rk3036.c b/drivers/clk/rockchip/clk-rk3036.c
index c300198..155fd4f 100644
--- a/drivers/clk/rockchip/clk-rk3036.c
+++ b/drivers/clk/rockchip/clk-rk3036.c
@@ -290,16 +290,17 @@ enum rk3036_plls {
 			RK2928_CLKSEL_CON(12), 8, 2, MFLAGS,
 			RK2928_CLKGATE_CON(2), 11, GFLAGS),
 	DIV(SCLK_SDMMC, "sclk_sdmmc", "sclk_sdmmc_src", 0,
-			RK2928_CLKSEL_CON(11), 0, 7, DFLAGS),
+			RK2928_CLKSEL_CON(11), 0, 7, DFLAGS | CLK_DIVIDER_EVEN),
 
 	COMPOSITE_NODIV(0, "sclk_sdio_src", mux_mmc_src_p, 0,
 			RK2928_CLKSEL_CON(12), 10, 2, MFLAGS,
 			RK2928_CLKGATE_CON(2), 13, GFLAGS),
 	DIV(SCLK_SDIO, "sclk_sdio", "sclk_sdio_src", 0,
-			RK2928_CLKSEL_CON(11), 8, 7, DFLAGS),
+			RK2928_CLKSEL_CON(11), 8, 7, DFLAGS | CLK_DIVIDER_EVEN),
 
 	COMPOSITE(SCLK_EMMC, "sclk_emmc", mux_mmc_src_p, 0,
-			RK2928_CLKSEL_CON(12), 12, 2, MFLAGS, 0, 7, DFLAGS,
+			RK2928_CLKSEL_CON(12), 12, 2, MFLAGS, 0, 7,
+			DFLAGS | CLK_DIVIDER_EVEN,
 			RK2928_CLKGATE_CON(2), 14, GFLAGS),
 
 	MMC(SCLK_SDMMC_DRV,    "sdmmc_drv",    "sclk_sdmmc", RK3036_SDMMC_CON0, 1),
diff --git a/drivers/clk/rockchip/clk-rk3128.c b/drivers/clk/rockchip/clk-rk3128.c
index 5970a50..7c6e30a 100644
--- a/drivers/clk/rockchip/clk-rk3128.c
+++ b/drivers/clk/rockchip/clk-rk3128.c
@@ -324,15 +324,18 @@ enum rk3128_plls {
 			RK2928_CLKGATE_CON(2), 15, GFLAGS),
 
 	COMPOSITE(SCLK_SDMMC, "sclk_sdmmc0", mux_mmc_src_p, 0,
-			RK2928_CLKSEL_CON(11), 6, 2, MFLAGS, 0, 6, DFLAGS,
+			RK2928_CLKSEL_CON(11), 6, 2, MFLAGS, 0, 6,
+			DFLAGS | CLK_DIVIDER_EVEN,
 			RK2928_CLKGATE_CON(2), 11, GFLAGS),
 
 	COMPOSITE(SCLK_SDIO, "sclk_sdio", mux_mmc_src_p, 0,
-			RK2928_CLKSEL_CON(12), 6, 2, MFLAGS, 0, 6, DFLAGS,
+			RK2928_CLKSEL_CON(12), 6, 2, MFLAGS, 0, 6,
+			DFLAGS | CLK_DIVIDER_EVEN,
 			RK2928_CLKGATE_CON(2), 13, GFLAGS),
 
 	COMPOSITE(SCLK_EMMC, "sclk_emmc", mux_mmc_src_p, 0,
-			RK2928_CLKSEL_CON(12), 14, 2, MFLAGS, 8, 6, DFLAGS,
+			RK2928_CLKSEL_CON(12), 14, 2, MFLAGS, 8, 6,
+			DFLAGS | CLK_DIVIDER_EVEN,
 			RK2928_CLKGATE_CON(2), 14, GFLAGS),
 
 	DIV(SCLK_PVTM, "clk_pvtm", "clk_pvtm_func", 0,
diff --git a/drivers/clk/rockchip/clk-rk3188.c b/drivers/clk/rockchip/clk-rk3188.c
index 67e73fd..32f199d 100644
--- a/drivers/clk/rockchip/clk-rk3188.c
+++ b/drivers/clk/rockchip/clk-rk3188.c
@@ -402,13 +402,13 @@ enum rk3188_plls {
 			RK2928_CLKGATE_CON(2), 10, GFLAGS),
 
 	COMPOSITE_NOMUX(SCLK_SDMMC, "sclk_sdmmc", "hclk_peri", 0,
-			RK2928_CLKSEL_CON(11), 0, 6, DFLAGS,
+			RK2928_CLKSEL_CON(11), 0, 6, DFLAGS | CLK_DIVIDER_EVEN,
 			RK2928_CLKGATE_CON(2), 11, GFLAGS),
 	COMPOSITE_NOMUX(SCLK_SDIO, "sclk_sdio", "hclk_peri", 0,
-			RK2928_CLKSEL_CON(12), 0, 6, DFLAGS,
+			RK2928_CLKSEL_CON(12), 0, 6, DFLAGS | CLK_DIVIDER_EVEN,
 			RK2928_CLKGATE_CON(2), 13, GFLAGS),
 	COMPOSITE_NOMUX(SCLK_EMMC, "sclk_emmc", "hclk_peri", 0,
-			RK2928_CLKSEL_CON(12), 8, 6, DFLAGS,
+			RK2928_CLKSEL_CON(12), 8, 6, DFLAGS | CLK_DIVIDER_EVEN,
 			RK2928_CLKGATE_CON(2), 14, GFLAGS),
 
 	MUX(0, "uart_src", mux_pll_src_gpll_cpll_p, 0,
diff --git a/drivers/clk/rockchip/clk-rk3228.c b/drivers/clk/rockchip/clk-rk3228.c
index 7af4818..15da16a 100644
--- a/drivers/clk/rockchip/clk-rk3228.c
+++ b/drivers/clk/rockchip/clk-rk3228.c
@@ -388,20 +388,21 @@ enum rk3228_plls {
 			RK2928_CLKGATE_CON(2), 15, GFLAGS),
 
 	COMPOSITE(SCLK_SDMMC, "sclk_sdmmc", mux_mmc_src_p, 0,
-			RK2928_CLKSEL_CON(11), 8, 2, MFLAGS, 0, 8, DFLAGS,
+			RK2928_CLKSEL_CON(11), 8, 2, MFLAGS, 0, 8,
+			DFLAGS | CLK_DIVIDER_EVEN,
 			RK2928_CLKGATE_CON(2), 11, GFLAGS),
 
 	COMPOSITE_NODIV(SCLK_SDIO_SRC, "sclk_sdio_src", mux_mmc_src_p, 0,
 			RK2928_CLKSEL_CON(11), 10, 2, MFLAGS,
 			RK2928_CLKGATE_CON(2), 13, GFLAGS),
 	DIV(SCLK_SDIO, "sclk_sdio", "sclk_sdio_src", 0,
-			RK2928_CLKSEL_CON(12), 0, 8, DFLAGS),
+			RK2928_CLKSEL_CON(12), 0, 8, DFLAGS | CLK_DIVIDER_EVEN),
 
 	COMPOSITE_NODIV(0, "sclk_emmc_src", mux_mmc_src_p, 0,
 			RK2928_CLKSEL_CON(11), 12, 2, MFLAGS,
 			RK2928_CLKGATE_CON(2), 14, GFLAGS),
 	DIV(SCLK_EMMC, "sclk_emmc", "sclk_emmc_src", 0,
-			RK2928_CLKSEL_CON(12), 8, 8, DFLAGS),
+			RK2928_CLKSEL_CON(12), 8, 8, DFLAGS | CLK_DIVIDER_EVEN),
 
 	/*
 	 * Clock-Architecture Diagram 2
diff --git a/drivers/clk/rockchip/clk-rk3288.c b/drivers/clk/rockchip/clk-rk3288.c
index 450de24..2a6f231 100644
--- a/drivers/clk/rockchip/clk-rk3288.c
+++ b/drivers/clk/rockchip/clk-rk3288.c
@@ -508,16 +508,20 @@ enum rk3288_plls {
 			RK3288_CLKGATE_CON(2), 11, GFLAGS),
 
 	COMPOSITE(SCLK_SDMMC, "sclk_sdmmc", mux_mmc_src_p, 0,
-			RK3288_CLKSEL_CON(11), 6, 2, MFLAGS, 0, 6, DFLAGS,
+			RK3288_CLKSEL_CON(11), 6, 2, MFLAGS, 0, 6,
+			DFLAGS | CLK_DIVIDER_EVEN,
 			RK3288_CLKGATE_CON(13), 0, GFLAGS),
 	COMPOSITE(SCLK_SDIO0, "sclk_sdio0", mux_mmc_src_p, 0,
-			RK3288_CLKSEL_CON(12), 6, 2, MFLAGS, 0, 6, DFLAGS,
+			RK3288_CLKSEL_CON(12), 6, 2, MFLAGS, 0, 6,
+			DFLAGS | CLK_DIVIDER_EVEN,
 			RK3288_CLKGATE_CON(13), 1, GFLAGS),
 	COMPOSITE(SCLK_SDIO1, "sclk_sdio1", mux_mmc_src_p, 0,
-			RK3288_CLKSEL_CON(34), 14, 2, MFLAGS, 8, 6, DFLAGS,
+			RK3288_CLKSEL_CON(34), 14, 2, MFLAGS, 8, 6,
+			DFLAGS | CLK_DIVIDER_EVEN,
 			RK3288_CLKGATE_CON(13), 2, GFLAGS),
 	COMPOSITE(SCLK_EMMC, "sclk_emmc", mux_mmc_src_p, 0,
-			RK3288_CLKSEL_CON(12), 14, 2, MFLAGS, 8, 6, DFLAGS,
+			RK3288_CLKSEL_CON(12), 14, 2, MFLAGS, 8, 6,
+			DFLAGS | CLK_DIVIDER_EVEN,
 			RK3288_CLKGATE_CON(13), 3, GFLAGS),
 
 	MMC(SCLK_SDMMC_DRV,    "sdmmc_drv",    "sclk_sdmmc", RK3288_SDMMC_CON0, 1),
diff --git a/drivers/clk/rockchip/clk-rk3328.c b/drivers/clk/rockchip/clk-rk3328.c
index 252366a..f81acf8 100644
--- a/drivers/clk/rockchip/clk-rk3328.c
+++ b/drivers/clk/rockchip/clk-rk3328.c
@@ -632,15 +632,18 @@ enum rk3328_plls {
 			RK3328_CLKGATE_CON(4), 3, GFLAGS),
 
 	COMPOSITE(SCLK_SDIO, "clk_sdio", mux_2plls_24m_u480m_p, 0,
-			RK3328_CLKSEL_CON(31), 8, 2, MFLAGS, 0, 8, DFLAGS,
+			RK3328_CLKSEL_CON(31), 8, 2, MFLAGS, 0, 8,
+			DFLAGS | CLK_DIVIDER_EVEN,
 			RK3328_CLKGATE_CON(4), 4, GFLAGS),
 
 	COMPOSITE(SCLK_EMMC, "clk_emmc", mux_2plls_24m_u480m_p, 0,
-			RK3328_CLKSEL_CON(32), 8, 2, MFLAGS, 0, 8, DFLAGS,
+			RK3328_CLKSEL_CON(32), 8, 2, MFLAGS, 0, 8,
+			DFLAGS | CLK_DIVIDER_EVEN,
 			RK3328_CLKGATE_CON(4), 5, GFLAGS),
 
 	COMPOSITE(SCLK_SDMMC_EXT, "clk_sdmmc_ext", mux_2plls_24m_u480m_p, 0,
-			RK3328_CLKSEL_CON(43), 8, 2, MFLAGS, 0, 8, DFLAGS,
+			RK3328_CLKSEL_CON(43), 8, 2, MFLAGS, 0, 8,
+			DFLAGS | CLK_DIVIDER_EVEN,
 			RK3328_CLKGATE_CON(4), 10, GFLAGS),
 
 	COMPOSITE(SCLK_REF_USB3OTG_SRC, "clk_ref_usb3otg_src", mux_2plls_p, 0,
diff --git a/drivers/clk/rockchip/clk-rk3368.c b/drivers/clk/rockchip/clk-rk3368.c
index 7c4d242..17a6293 100644
--- a/drivers/clk/rockchip/clk-rk3368.c
+++ b/drivers/clk/rockchip/clk-rk3368.c
@@ -550,13 +550,16 @@ enum rk3368_plls {
 
 
 	COMPOSITE(SCLK_SDMMC, "sclk_sdmmc", mux_mmc_src_p, 0,
-			RK3368_CLKSEL_CON(50), 8, 2, MFLAGS, 0, 7, DFLAGS,
+			RK3368_CLKSEL_CON(50), 8, 2, MFLAGS, 0, 7,
+			DFLAGS | CLK_DIVIDER_EVEN,
 			RK3368_CLKGATE_CON(7), 12, GFLAGS),
 	COMPOSITE(SCLK_SDIO0, "sclk_sdio0", mux_mmc_src_p, 0,
-			RK3368_CLKSEL_CON(48), 8, 2, MFLAGS, 0, 7, DFLAGS,
+			RK3368_CLKSEL_CON(48), 8, 2, MFLAGS, 0, 7,
+			DFLAGS | CLK_DIVIDER_EVEN,
 			RK3368_CLKGATE_CON(7), 13, GFLAGS),
 	COMPOSITE(SCLK_EMMC, "sclk_emmc", mux_mmc_src_p, 0,
-			RK3368_CLKSEL_CON(51), 8, 2, MFLAGS, 0, 7, DFLAGS,
+			RK3368_CLKSEL_CON(51), 8, 2, MFLAGS, 0, 7,
+			DFLAGS | CLK_DIVIDER_EVEN,
 			RK3368_CLKGATE_CON(7), 15, GFLAGS),
 
 	MMC(SCLK_SDMMC_DRV,    "sdmmc_drv",    "sclk_sdmmc", RK3368_SDMMC_CON0, 1),
diff --git a/drivers/clk/rockchip/clk-rk3399.c b/drivers/clk/rockchip/clk-rk3399.c
index 118759d..aedb3d0 100644
--- a/drivers/clk/rockchip/clk-rk3399.c
+++ b/drivers/clk/rockchip/clk-rk3399.c
@@ -896,11 +896,13 @@ enum rk3399_pmu_plls {
 			RK3399_CLKGATE_CON(33), 9, GFLAGS),
 
 	COMPOSITE(SCLK_SDIO, "clk_sdio", mux_pll_src_cpll_gpll_npll_ppll_upll_24m_p, 0,
-			RK3399_CLKSEL_CON(15), 8, 3, MFLAGS, 0, 7, DFLAGS,
+			RK3399_CLKSEL_CON(15), 8, 3, MFLAGS, 0, 7,
+			DFLAGS | CLK_DIVIDER_EVEN,
 			RK3399_CLKGATE_CON(6), 0, GFLAGS),
 
 	COMPOSITE(SCLK_SDMMC, "clk_sdmmc", mux_pll_src_cpll_gpll_npll_ppll_upll_24m_p, 0,
-			RK3399_CLKSEL_CON(16), 8, 3, MFLAGS, 0, 7, DFLAGS,
+			RK3399_CLKSEL_CON(16), 8, 3, MFLAGS, 0, 7,
+			DFLAGS | CLK_DIVIDER_EVEN,
 			RK3399_CLKGATE_CON(6), 1, GFLAGS),
 
 	MMC(SCLK_SDMMC_DRV,     "sdmmc_drv",    "clk_sdmmc", RK3399_SDMMC_CON0, 1),
diff --git a/drivers/clk/rockchip/clk-rv1108.c b/drivers/clk/rockchip/clk-rv1108.c
index 089cb17..b63af26 100644
--- a/drivers/clk/rockchip/clk-rv1108.c
+++ b/drivers/clk/rockchip/clk-rv1108.c
@@ -721,20 +721,21 @@ enum rv1108_plls {
 			RV1108_CLKGATE_CON(15), 11, GFLAGS),
 
 	COMPOSITE(SCLK_SDMMC, "sclk_sdmmc", mux_mmc_src_p, 0,
-			RV1108_CLKSEL_CON(25), 8, 2, MFLAGS, 0, 8, DFLAGS,
+			RV1108_CLKSEL_CON(25), 8, 2, MFLAGS, 0, 8,
+			DFLAGS | CLK_DIVIDER_EVEN,
 			RV1108_CLKGATE_CON(5), 0, GFLAGS),
 
 	COMPOSITE_NODIV(0, "sclk_sdio_src", mux_mmc_src_p, 0,
 			RV1108_CLKSEL_CON(25), 10, 2, MFLAGS,
 			RV1108_CLKGATE_CON(5), 2, GFLAGS),
 	DIV(SCLK_SDIO, "sclk_sdio", "sclk_sdio_src", 0,
-			RV1108_CLKSEL_CON(26), 0, 8, DFLAGS),
+			RV1108_CLKSEL_CON(26), 0, 8, DFLAGS | CLK_DIVIDER_EVEN),
 
 	COMPOSITE_NODIV(0, "sclk_emmc_src", mux_mmc_src_p, 0,
 			RV1108_CLKSEL_CON(25), 12, 2, MFLAGS,
 			RV1108_CLKGATE_CON(5), 1, GFLAGS),
 	DIV(SCLK_EMMC, "sclk_emmc", "sclk_emmc_src", 0,
-			RK2928_CLKSEL_CON(26), 8, 8, DFLAGS),
+			RK2928_CLKSEL_CON(26), 8, 8, DFLAGS | CLK_DIVIDER_EVEN),
 	GATE(HCLK_SDMMC, "hclk_sdmmc", "hclk_periph", 0, RV1108_CLKGATE_CON(15), 0, GFLAGS),
 	GATE(HCLK_SDIO, "hclk_sdio", "hclk_periph", 0, RV1108_CLKGATE_CON(15), 1, GFLAGS),
 	GATE(HCLK_EMMC, "hclk_emmc", "hclk_periph", 0, RV1108_CLKGATE_CON(15), 2, GFLAGS),
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] clk: divider: Add CLK_DIVIDER_EVEN flag support
@ 2018-04-05 13:30   ` Heiko Stuebner
  0 siblings, 0 replies; 12+ messages in thread
From: Heiko Stuebner @ 2018-04-05 13:30 UTC (permalink / raw)
  To: Shawn Lin
  Cc: Michael Turquette, Stephen Boyd, linux-rockchip, linux-clk,
	Jeffy Chen, Finley Xiao, Andy Yan, Xing Zheng, Elaine Zhang,
	Ziyuan Xu

Hi Shawn,

Am Donnerstag, 5. April 2018, 07:38:18 CEST schrieb Shawn Lin:
> CLK_DIVIDER_EVEN is used for clock divders that should only
> use even number in the div field.
> 
> Two clock divder should consider to use this flag
> (1) The divder is physically only support even div number to
> generate 50% duty cycle clock rate.
> (2) The divder's clock consumer request it to use even div number
> to generate the most closest requested rate for whatever reason.
> 
> In some platforms, for instance Rockchip, the eMMC/SDIO/SDMMC should
> request divder to use even number when working at a high throughput
> speed mode reliably. However, that wasn't guaranteed by clock framework.
> So the previous tricky is to carefully assign magic clock rate to their
> parents as well as consumer's clock rate when requesting. That works
> bad in practice if folks change the parents clock rate or the clock
> hierarchy randomly. That also work bad if the consumer's clock rate
> came from the DT, which is changed so fraquent for different boards.
> 
> To make it's less prone to make mistake and to make it really respect
> the fact that the divder should use even number to the div field, we
> need the clock framework's help. Now we have CLK_DIVIDER_POWER_OF_TWO,
> which could guarantee the div field is even number, however, obviously
> it skips the even numbers which is the power of 2, but maybe which is
> the best div to generate closest clock rate for consumer.

I think there is a slight misunderstanding here, CLK_DIVIDER_POWER_OF_TWO
means "2 raised to the value read from the hardware register", so describes
a hardware property, while your CLK_DIVIDER_EVEN describes needed special
handling due to some hardware necessity.

That is not meant to nack your change, but just to point out that you can
also have CLK_DIVIDER_POWER_OF_TWO | CLK_DIVIDER_EVEN, which is
relevant for the uneven 2^0 = 1 case and should be taken in to account.


Heiko

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] clk: divider: Add CLK_DIVIDER_EVEN flag support
@ 2018-04-05 13:30   ` Heiko Stuebner
  0 siblings, 0 replies; 12+ messages in thread
From: Heiko Stuebner @ 2018-04-05 13:30 UTC (permalink / raw)
  To: Shawn Lin
  Cc: Elaine Zhang, Stephen Boyd, Michael Turquette, Jeffy Chen,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Finley Xiao,
	Xing Zheng, Andy Yan, linux-clk-u79uwXL29TY76Z2rM5mHXA,
	Ziyuan Xu

Hi Shawn,

Am Donnerstag, 5. April 2018, 07:38:18 CEST schrieb Shawn Lin:
> CLK_DIVIDER_EVEN is used for clock divders that should only
> use even number in the div field.
> 
> Two clock divder should consider to use this flag
> (1) The divder is physically only support even div number to
> generate 50% duty cycle clock rate.
> (2) The divder's clock consumer request it to use even div number
> to generate the most closest requested rate for whatever reason.
> 
> In some platforms, for instance Rockchip, the eMMC/SDIO/SDMMC should
> request divder to use even number when working at a high throughput
> speed mode reliably. However, that wasn't guaranteed by clock framework.
> So the previous tricky is to carefully assign magic clock rate to their
> parents as well as consumer's clock rate when requesting. That works
> bad in practice if folks change the parents clock rate or the clock
> hierarchy randomly. That also work bad if the consumer's clock rate
> came from the DT, which is changed so fraquent for different boards.
> 
> To make it's less prone to make mistake and to make it really respect
> the fact that the divder should use even number to the div field, we
> need the clock framework's help. Now we have CLK_DIVIDER_POWER_OF_TWO,
> which could guarantee the div field is even number, however, obviously
> it skips the even numbers which is the power of 2, but maybe which is
> the best div to generate closest clock rate for consumer.

I think there is a slight misunderstanding here, CLK_DIVIDER_POWER_OF_TWO
means "2 raised to the value read from the hardware register", so describes
a hardware property, while your CLK_DIVIDER_EVEN describes needed special
handling due to some hardware necessity.

That is not meant to nack your change, but just to point out that you can
also have CLK_DIVIDER_POWER_OF_TWO | CLK_DIVIDER_EVEN, which is
relevant for the uneven 2^0 = 1 case and should be taken in to account.


Heiko

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] clk: divider: Add CLK_DIVIDER_EVEN flag support
  2018-04-05 13:30   ` Heiko Stuebner
@ 2018-04-05 14:47     ` Shawn Lin
  -1 siblings, 0 replies; 12+ messages in thread
From: Shawn Lin @ 2018-04-05 14:47 UTC (permalink / raw)
  To: Heiko Stuebner
  Cc: shawn.lin, Michael Turquette, Stephen Boyd, linux-rockchip,
	linux-clk, Jeffy Chen, Finley Xiao, Andy Yan, Xing Zheng,
	Elaine Zhang, Ziyuan Xu

Hi Heiko,

On 2018/4/5 21:30, Heiko Stuebner wrote:
> Hi Shawn,
> 
> Am Donnerstag, 5. April 2018, 07:38:18 CEST schrieb Shawn Lin:
>> CLK_DIVIDER_EVEN is used for clock divders that should only
>> use even number in the div field.
>>
>> Two clock divder should consider to use this flag
>> (1) The divder is physically only support even div number to
>> generate 50% duty cycle clock rate.
>> (2) The divder's clock consumer request it to use even div number
>> to generate the most closest requested rate for whatever reason.
>>
>> In some platforms, for instance Rockchip, the eMMC/SDIO/SDMMC should
>> request divder to use even number when working at a high throughput
>> speed mode reliably. However, that wasn't guaranteed by clock framework.
>> So the previous tricky is to carefully assign magic clock rate to their
>> parents as well as consumer's clock rate when requesting. That works
>> bad in practice if folks change the parents clock rate or the clock
>> hierarchy randomly. That also work bad if the consumer's clock rate
>> came from the DT, which is changed so fraquent for different boards.
>>
>> To make it's less prone to make mistake and to make it really respect
>> the fact that the divder should use even number to the div field, we
>> need the clock framework's help. Now we have CLK_DIVIDER_POWER_OF_TWO,
>> which could guarantee the div field is even number, however, obviously
>> it skips the even numbers which is the power of 2, but maybe which is
>> the best div to generate closest clock rate for consumer.
> 
> I think there is a slight misunderstanding here, CLK_DIVIDER_POWER_OF_TWO
> means "2 raised to the value read from the hardware register", so describes
> a hardware property, while your CLK_DIVIDER_EVEN describes needed special
> handling due to some hardware necessity >
> That is not meant to nack your change, but just to point out that you can
> also have CLK_DIVIDER_POWER_OF_TWO | CLK_DIVIDER_EVEN, which is
> relevant for the uneven 2^0 = 1 case and should be taken in to account.

Ahh, really good catch, and I always forgot 1 is the power of 2.

More or less, it's a hardware property for Rockchip as these divders
only hardwired to specific controllers which always request even div
field, even if the div field could be odd number physcially. But yes,
the common case wouldn't be that, and another combination would be
CLK_DIVIDER_EVEN | CLK_DIVIDER_ONE_BASED?

> 
> 
> Heiko
> 
> 
> 
> 


-- 
Best Regards
Shawn Lin

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] clk: divider: Add CLK_DIVIDER_EVEN flag support
@ 2018-04-05 14:47     ` Shawn Lin
  0 siblings, 0 replies; 12+ messages in thread
From: Shawn Lin @ 2018-04-05 14:47 UTC (permalink / raw)
  To: Heiko Stuebner
  Cc: Elaine Zhang, Stephen Boyd, shawn.lin-TNX95d0MmH7DzftRWevZcw,
	Jeffy Chen, linux-clk-u79uwXL29TY76Z2rM5mHXA,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Finley Xiao,
	Xing Zheng, Andy Yan, Michael Turquette, Ziyuan Xu

Hi Heiko,

On 2018/4/5 21:30, Heiko Stuebner wrote:
> Hi Shawn,
> 
> Am Donnerstag, 5. April 2018, 07:38:18 CEST schrieb Shawn Lin:
>> CLK_DIVIDER_EVEN is used for clock divders that should only
>> use even number in the div field.
>>
>> Two clock divder should consider to use this flag
>> (1) The divder is physically only support even div number to
>> generate 50% duty cycle clock rate.
>> (2) The divder's clock consumer request it to use even div number
>> to generate the most closest requested rate for whatever reason.
>>
>> In some platforms, for instance Rockchip, the eMMC/SDIO/SDMMC should
>> request divder to use even number when working at a high throughput
>> speed mode reliably. However, that wasn't guaranteed by clock framework.
>> So the previous tricky is to carefully assign magic clock rate to their
>> parents as well as consumer's clock rate when requesting. That works
>> bad in practice if folks change the parents clock rate or the clock
>> hierarchy randomly. That also work bad if the consumer's clock rate
>> came from the DT, which is changed so fraquent for different boards.
>>
>> To make it's less prone to make mistake and to make it really respect
>> the fact that the divder should use even number to the div field, we
>> need the clock framework's help. Now we have CLK_DIVIDER_POWER_OF_TWO,
>> which could guarantee the div field is even number, however, obviously
>> it skips the even numbers which is the power of 2, but maybe which is
>> the best div to generate closest clock rate for consumer.
> 
> I think there is a slight misunderstanding here, CLK_DIVIDER_POWER_OF_TWO
> means "2 raised to the value read from the hardware register", so describes
> a hardware property, while your CLK_DIVIDER_EVEN describes needed special
> handling due to some hardware necessity >
> That is not meant to nack your change, but just to point out that you can
> also have CLK_DIVIDER_POWER_OF_TWO | CLK_DIVIDER_EVEN, which is
> relevant for the uneven 2^0 = 1 case and should be taken in to account.

Ahh, really good catch, and I always forgot 1 is the power of 2.

More or less, it's a hardware property for Rockchip as these divders
only hardwired to specific controllers which always request even div
field, even if the div field could be odd number physcially. But yes,
the common case wouldn't be that, and another combination would be
CLK_DIVIDER_EVEN | CLK_DIVIDER_ONE_BASED?

> 
> 
> Heiko
> 
> 
> 
> 


-- 
Best Regards
Shawn Lin

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] clk: divider: Add CLK_DIVIDER_EVEN flag support
@ 2018-04-06 21:13       ` Stephen Boyd
  0 siblings, 0 replies; 12+ messages in thread
From: Stephen Boyd @ 2018-04-06 21:13 UTC (permalink / raw)
  To: Heiko Stuebner, Shawn Lin
  Cc: shawn.lin, Michael Turquette, linux-rockchip, linux-clk,
	Jeffy Chen, Finley Xiao, Andy Yan, Xing Zheng, Elaine Zhang,
	Ziyuan Xu

Quoting Shawn Lin (2018-04-05 07:47:41)
> Hi Heiko,
> =

> On 2018/4/5 21:30, Heiko Stuebner wrote:
> > Hi Shawn,
> > =

> > Am Donnerstag, 5. April 2018, 07:38:18 CEST schrieb Shawn Lin:
> >> CLK_DIVIDER_EVEN is used for clock divders that should only
> >> use even number in the div field.
> >>
> >> Two clock divder should consider to use this flag
> >> (1) The divder is physically only support even div number to
> >> generate 50% duty cycle clock rate.
> >> (2) The divder's clock consumer request it to use even div number
> >> to generate the most closest requested rate for whatever reason.
> >>
> >> In some platforms, for instance Rockchip, the eMMC/SDIO/SDMMC should
> >> request divder to use even number when working at a high throughput
> >> speed mode reliably. However, that wasn't guaranteed by clock framewor=
k.
> >> So the previous tricky is to carefully assign magic clock rate to their
> >> parents as well as consumer's clock rate when requesting. That works
> >> bad in practice if folks change the parents clock rate or the clock
> >> hierarchy randomly. That also work bad if the consumer's clock rate
> >> came from the DT, which is changed so fraquent for different boards.
> >>
> >> To make it's less prone to make mistake and to make it really respect
> >> the fact that the divder should use even number to the div field, we
> >> need the clock framework's help. Now we have CLK_DIVIDER_POWER_OF_TWO,
> >> which could guarantee the div field is even number, however, obviously
> >> it skips the even numbers which is the power of 2, but maybe which is
> >> the best div to generate closest clock rate for consumer.
> > =

> > I think there is a slight misunderstanding here, CLK_DIVIDER_POWER_OF_T=
WO
> > means "2 raised to the value read from the hardware register", so descr=
ibes
> > a hardware property, while your CLK_DIVIDER_EVEN describes needed speci=
al
> > handling due to some hardware necessity >
> > That is not meant to nack your change, but just to point out that you c=
an
> > also have CLK_DIVIDER_POWER_OF_TWO | CLK_DIVIDER_EVEN, which is
> > relevant for the uneven 2^0 =3D 1 case and should be taken in to accoun=
t.
> =

> Ahh, really good catch, and I always forgot 1 is the power of 2.
> =

> More or less, it's a hardware property for Rockchip as these divders
> only hardwired to specific controllers which always request even div
> field, even if the div field could be odd number physcially. But yes,
> the common case wouldn't be that, and another combination would be
> CLK_DIVIDER_EVEN | CLK_DIVIDER_ONE_BASED?
> =


Maybe you can use a table of divider values? That is a simple escape
hatch for this sort of thing and doesn't require adding another flag to
the divider code to handle this.

Otherwise it would be good to handle these combinations of flags as
well. We're only up to 7 flags so it isn't too bad yet.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] clk: divider: Add CLK_DIVIDER_EVEN flag support
@ 2018-04-06 21:13       ` Stephen Boyd
  0 siblings, 0 replies; 12+ messages in thread
From: Stephen Boyd @ 2018-04-06 21:13 UTC (permalink / raw)
  To: Heiko Stuebner
  Cc: Elaine Zhang, shawn.lin-TNX95d0MmH7DzftRWevZcw, Jeffy Chen,
	linux-clk-u79uwXL29TY76Z2rM5mHXA,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Finley Xiao,
	Xing Zheng, Andy Yan, Michael Turquette, Ziyuan Xu

Quoting Shawn Lin (2018-04-05 07:47:41)
> Hi Heiko,
> 
> On 2018/4/5 21:30, Heiko Stuebner wrote:
> > Hi Shawn,
> > 
> > Am Donnerstag, 5. April 2018, 07:38:18 CEST schrieb Shawn Lin:
> >> CLK_DIVIDER_EVEN is used for clock divders that should only
> >> use even number in the div field.
> >>
> >> Two clock divder should consider to use this flag
> >> (1) The divder is physically only support even div number to
> >> generate 50% duty cycle clock rate.
> >> (2) The divder's clock consumer request it to use even div number
> >> to generate the most closest requested rate for whatever reason.
> >>
> >> In some platforms, for instance Rockchip, the eMMC/SDIO/SDMMC should
> >> request divder to use even number when working at a high throughput
> >> speed mode reliably. However, that wasn't guaranteed by clock framework.
> >> So the previous tricky is to carefully assign magic clock rate to their
> >> parents as well as consumer's clock rate when requesting. That works
> >> bad in practice if folks change the parents clock rate or the clock
> >> hierarchy randomly. That also work bad if the consumer's clock rate
> >> came from the DT, which is changed so fraquent for different boards.
> >>
> >> To make it's less prone to make mistake and to make it really respect
> >> the fact that the divder should use even number to the div field, we
> >> need the clock framework's help. Now we have CLK_DIVIDER_POWER_OF_TWO,
> >> which could guarantee the div field is even number, however, obviously
> >> it skips the even numbers which is the power of 2, but maybe which is
> >> the best div to generate closest clock rate for consumer.
> > 
> > I think there is a slight misunderstanding here, CLK_DIVIDER_POWER_OF_TWO
> > means "2 raised to the value read from the hardware register", so describes
> > a hardware property, while your CLK_DIVIDER_EVEN describes needed special
> > handling due to some hardware necessity >
> > That is not meant to nack your change, but just to point out that you can
> > also have CLK_DIVIDER_POWER_OF_TWO | CLK_DIVIDER_EVEN, which is
> > relevant for the uneven 2^0 = 1 case and should be taken in to account.
> 
> Ahh, really good catch, and I always forgot 1 is the power of 2.
> 
> More or less, it's a hardware property for Rockchip as these divders
> only hardwired to specific controllers which always request even div
> field, even if the div field could be odd number physcially. But yes,
> the common case wouldn't be that, and another combination would be
> CLK_DIVIDER_EVEN | CLK_DIVIDER_ONE_BASED?
> 

Maybe you can use a table of divider values? That is a simple escape
hatch for this sort of thing and doesn't require adding another flag to
the divider code to handle this.

Otherwise it would be good to handle these combinations of flags as
well. We're only up to 7 flags so it isn't too bad yet.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] clk: divider: Add CLK_DIVIDER_EVEN flag support
@ 2018-04-08  1:58         ` Shawn Lin
  0 siblings, 0 replies; 12+ messages in thread
From: Shawn Lin @ 2018-04-08  1:58 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Heiko Stuebner, shawn.lin, Michael Turquette, linux-rockchip,
	linux-clk, Jeffy Chen, Finley Xiao, Andy Yan, Xing Zheng,
	Elaine Zhang, Ziyuan Xu

Hi Stephen,

On 2018/4/7 5:13, Stephen Boyd wrote:
> Quoting Shawn Lin (2018-04-05 07:47:41)
>> Hi Heiko,
>>
>> On 2018/4/5 21:30, Heiko Stuebner wrote:
>>> Hi Shawn,
>>>
>>> Am Donnerstag, 5. April 2018, 07:38:18 CEST schrieb Shawn Lin:
>>>> CLK_DIVIDER_EVEN is used for clock divders that should only
>>>> use even number in the div field.
>>>>
>>>> Two clock divder should consider to use this flag
>>>> (1) The divder is physically only support even div number to
>>>> generate 50% duty cycle clock rate.
>>>> (2) The divder's clock consumer request it to use even div number
>>>> to generate the most closest requested rate for whatever reason.
>>>>
>>>> In some platforms, for instance Rockchip, the eMMC/SDIO/SDMMC should
>>>> request divder to use even number when working at a high throughput
>>>> speed mode reliably. However, that wasn't guaranteed by clock framework.
>>>> So the previous tricky is to carefully assign magic clock rate to their
>>>> parents as well as consumer's clock rate when requesting. That works
>>>> bad in practice if folks change the parents clock rate or the clock
>>>> hierarchy randomly. That also work bad if the consumer's clock rate
>>>> came from the DT, which is changed so fraquent for different boards.
>>>>
>>>> To make it's less prone to make mistake and to make it really respect
>>>> the fact that the divder should use even number to the div field, we
>>>> need the clock framework's help. Now we have CLK_DIVIDER_POWER_OF_TWO,
>>>> which could guarantee the div field is even number, however, obviously
>>>> it skips the even numbers which is the power of 2, but maybe which is
>>>> the best div to generate closest clock rate for consumer.
>>>
>>> I think there is a slight misunderstanding here, CLK_DIVIDER_POWER_OF_TWO
>>> means "2 raised to the value read from the hardware register", so describes
>>> a hardware property, while your CLK_DIVIDER_EVEN describes needed special
>>> handling due to some hardware necessity >
>>> That is not meant to nack your change, but just to point out that you can
>>> also have CLK_DIVIDER_POWER_OF_TWO | CLK_DIVIDER_EVEN, which is
>>> relevant for the uneven 2^0 = 1 case and should be taken in to account.
>>
>> Ahh, really good catch, and I always forgot 1 is the power of 2.
>>
>> More or less, it's a hardware property for Rockchip as these divders
>> only hardwired to specific controllers which always request even div
>> field, even if the div field could be odd number physcially. But yes,
>> the common case wouldn't be that, and another combination would be
>> CLK_DIVIDER_EVEN | CLK_DIVIDER_ONE_BASED?
>>
> 
> Maybe you can use a table of divider values? That is a simple escape
> hatch for this sort of thing and doesn't require adding another flag to
> the divider code to handle this.

The very first thought of adding clk_div_table is to support irregular
div calculation/limitation, but even number is rather common, regular
and symmetrical one. So I'm inclined to add it support.

> 
> Otherwise it would be good to handle these combinations of flags as
> well. We're only up to 7 flags so it isn't too bad yet.

Yep. After thinking more, I think we only need to handle div of 1
to avoid combination of CLK_DIVIDER_POWER_OF_TWO | CLK_DIVIDER_EVEN
suggested by Heiko. The others are just fine and not need to bother
with CLK_DIVIDER_EVEN, even with combination.

> 
> 
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] clk: divider: Add CLK_DIVIDER_EVEN flag support
@ 2018-04-08  1:58         ` Shawn Lin
  0 siblings, 0 replies; 12+ messages in thread
From: Shawn Lin @ 2018-04-08  1:58 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Elaine Zhang, Heiko Stuebner, Michael Turquette, Jeffy Chen,
	linux-clk-u79uwXL29TY76Z2rM5mHXA,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Finley Xiao,
	Xing Zheng, Andy Yan, shawn.lin-TNX95d0MmH7DzftRWevZcw,
	Ziyuan Xu

Hi Stephen,

On 2018/4/7 5:13, Stephen Boyd wrote:
> Quoting Shawn Lin (2018-04-05 07:47:41)
>> Hi Heiko,
>>
>> On 2018/4/5 21:30, Heiko Stuebner wrote:
>>> Hi Shawn,
>>>
>>> Am Donnerstag, 5. April 2018, 07:38:18 CEST schrieb Shawn Lin:
>>>> CLK_DIVIDER_EVEN is used for clock divders that should only
>>>> use even number in the div field.
>>>>
>>>> Two clock divder should consider to use this flag
>>>> (1) The divder is physically only support even div number to
>>>> generate 50% duty cycle clock rate.
>>>> (2) The divder's clock consumer request it to use even div number
>>>> to generate the most closest requested rate for whatever reason.
>>>>
>>>> In some platforms, for instance Rockchip, the eMMC/SDIO/SDMMC should
>>>> request divder to use even number when working at a high throughput
>>>> speed mode reliably. However, that wasn't guaranteed by clock framework.
>>>> So the previous tricky is to carefully assign magic clock rate to their
>>>> parents as well as consumer's clock rate when requesting. That works
>>>> bad in practice if folks change the parents clock rate or the clock
>>>> hierarchy randomly. That also work bad if the consumer's clock rate
>>>> came from the DT, which is changed so fraquent for different boards.
>>>>
>>>> To make it's less prone to make mistake and to make it really respect
>>>> the fact that the divder should use even number to the div field, we
>>>> need the clock framework's help. Now we have CLK_DIVIDER_POWER_OF_TWO,
>>>> which could guarantee the div field is even number, however, obviously
>>>> it skips the even numbers which is the power of 2, but maybe which is
>>>> the best div to generate closest clock rate for consumer.
>>>
>>> I think there is a slight misunderstanding here, CLK_DIVIDER_POWER_OF_TWO
>>> means "2 raised to the value read from the hardware register", so describes
>>> a hardware property, while your CLK_DIVIDER_EVEN describes needed special
>>> handling due to some hardware necessity >
>>> That is not meant to nack your change, but just to point out that you can
>>> also have CLK_DIVIDER_POWER_OF_TWO | CLK_DIVIDER_EVEN, which is
>>> relevant for the uneven 2^0 = 1 case and should be taken in to account.
>>
>> Ahh, really good catch, and I always forgot 1 is the power of 2.
>>
>> More or less, it's a hardware property for Rockchip as these divders
>> only hardwired to specific controllers which always request even div
>> field, even if the div field could be odd number physcially. But yes,
>> the common case wouldn't be that, and another combination would be
>> CLK_DIVIDER_EVEN | CLK_DIVIDER_ONE_BASED?
>>
> 
> Maybe you can use a table of divider values? That is a simple escape
> hatch for this sort of thing and doesn't require adding another flag to
> the divider code to handle this.

The very first thought of adding clk_div_table is to support irregular
div calculation/limitation, but even number is rather common, regular
and symmetrical one. So I'm inclined to add it support.

> 
> Otherwise it would be good to handle these combinations of flags as
> well. We're only up to 7 flags so it isn't too bad yet.

Yep. After thinking more, I think we only need to handle div of 1
to avoid combination of CLK_DIVIDER_POWER_OF_TWO | CLK_DIVIDER_EVEN
suggested by Heiko. The others are just fine and not need to bother
with CLK_DIVIDER_EVEN, even with combination.

> 
> 
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2018-04-08  1:58 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-05  5:38 [PATCH 1/2] clk: divider: Add CLK_DIVIDER_EVEN flag support Shawn Lin
2018-04-05  5:38 ` Shawn Lin
2018-04-05  5:38 ` [PATCH 2/2] clk: rockchip: Add CLK_DIVIDER_EVEN for all mmc clock consumers Shawn Lin
2018-04-05  5:38   ` Shawn Lin
2018-04-05 13:30 ` [PATCH 1/2] clk: divider: Add CLK_DIVIDER_EVEN flag support Heiko Stuebner
2018-04-05 13:30   ` Heiko Stuebner
2018-04-05 14:47   ` Shawn Lin
2018-04-05 14:47     ` Shawn Lin
2018-04-06 21:13     ` Stephen Boyd
2018-04-06 21:13       ` Stephen Boyd
2018-04-08  1:58       ` Shawn Lin
2018-04-08  1:58         ` Shawn Lin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.