arm: imx: correct the hardware clock gate setting for shared nodes

PATCH Superseded Anson Huang 4 participants 7 messages Dec 9, 2014 – Dec 10, 2014 1 message not in the archive

Subsystems (3)

Arm (32-bit) Arm sub-architectures Arm i.MX and Layerscape

lore

Share & tools

lore

Anson Huang

author

2014-12-09 07:58 UTC #0 lore raw

PATCH 1/1 arm: imx: correct the hardware clock gate setting for shared nodes

For those clk gates which hold share count, since its is_enabled callback is only checking the share count rather than reading the hardware register setting, in the late phase of kernel bootup, the clk_disable_unused action will NOT handle the scenario of share_count is 0 but the hardware setting is enabled, actually, uboot normally enables all clk gates, then those shared clk gates will be always enabled until they are used by some modules.

So the problem would be: when kernel boot up, the usecount cat from clk tree is 0, but the clk gates actually is enabled in hardware register, it will confuse user and bring unnecessary power consumption, take i.MX6SX for example, the ESAI clk info is as below, the use count is 0, but the hardware register read from CCM_CCGR1_CG8 is ungated.

cat /sys/kernel/debug/clk/clk_summary | grep esai

esai_sel 0 0 393216000 0 0
esai_pred 0 0 196608000 0 0
esai_podf 0 0 24576000 0 0
esai_extal 0 0 24576000 0 0
esai_mem 0 0 132000000 0 0
esai_ipg 0 0 132000000 0 0

Read CCM_CCGR1:

0x020C406C: F33FFF00

This patch disables all those clk gates which hold share count before registering them, then the hardware status will match the clk tree info.

Signed-off-by Anson Huang <[email protected]>

---
 arch/arm/mach-imx/clk-imx6q.c  |    6 ++++++
 arch/arm/mach-imx/clk-imx6sl.c |    2 ++
 arch/arm/mach-imx/clk-imx6sx.c |    6 ++++++
 3 files changed, 14 insertions(+)

arch/arm/mach-imx/clk-imx6q.c +6 -0

index 5951660..fe53473 100644
@@ -369,6 +369,8 @@ static void __init imx6q_clocks_init(struct device_node *ccm_node)
 
 	/*                                            name             parent_name          reg         shift */
 	clk[IMX6QDL_CLK_APBH_DMA]     = imx_clk_gate2("apbh_dma",      "usdhc3",            base + 0x68, 4);
+	/* make sure the initial clock gate setting of share_count is off */
+	writel_relaxed(readl_relaxed(base + 0x68) & (~(0x3 << 6)), base + 0x68);
 	clk[IMX6QDL_CLK_ASRC]         = imx_clk_gate2_shared("asrc",         "asrc_podf",   base + 0x68, 6, &share_count_asrc);
 	clk[IMX6QDL_CLK_ASRC_IPG]     = imx_clk_gate2_shared("asrc_ipg",     "ahb",         base + 0x68, 6, &share_count_asrc);
 	clk[IMX6QDL_CLK_ASRC_MEM]     = imx_clk_gate2_shared("asrc_mem",     "ahb",         base + 0x68, 6, &share_count_asrc);

@@ -385,6 +387,8 @@ static void __init imx6q_clocks_init(struct device_node *ccm_node)
 	else
 		clk[IMX6Q_CLK_ECSPI5] = imx_clk_gate2("ecspi5",        "ecspi_root",        base + 0x6c, 8);
 	clk[IMX6QDL_CLK_ENET]         = imx_clk_gate2("enet",          "ipg",               base + 0x6c, 10);
+	/* make sure the initial clock gate setting of share_count is off */
+	writel_relaxed(readl_relaxed(base + 0x6c) & (~(0x3 << 16)), base + 0x6c);
 	clk[IMX6QDL_CLK_ESAI_EXTAL]   = imx_clk_gate2_shared("esai_extal",   "esai_podf",   base + 0x6c, 16, &share_count_esai);
 	clk[IMX6QDL_CLK_ESAI_IPG]     = imx_clk_gate2_shared("esai_ipg",   "ipg",           base + 0x6c, 16, &share_count_esai);
 	clk[IMX6QDL_CLK_ESAI_MEM]     = imx_clk_gate2_shared("esai_mem", "ahb",             base + 0x6c, 16, &share_count_esai);

@@ -443,6 +447,8 @@ static void __init imx6q_clocks_init(struct device_node *ccm_node)
 	clk[IMX6QDL_CLK_SDMA]         = imx_clk_gate2("sdma",          "ahb",               base + 0x7c, 6);
 	clk[IMX6QDL_CLK_SPBA]         = imx_clk_gate2("spba",          "ipg",               base + 0x7c, 12);
 	clk[IMX6QDL_CLK_SPDIF]        = imx_clk_gate2("spdif",         "spdif_podf",        base + 0x7c, 14);
+	/* make sure the initial clock gate setting of share_count is off */
+	writel_relaxed(readl_relaxed(base + 0x7c) & (~(0x3f << 18)), base + 0x7c);
 	clk[IMX6QDL_CLK_SSI1_IPG]     = imx_clk_gate2_shared("ssi1_ipg",      "ipg",        base + 0x7c, 18, &share_count_ssi1);
 	clk[IMX6QDL_CLK_SSI2_IPG]     = imx_clk_gate2_shared("ssi2_ipg",      "ipg",        base + 0x7c, 20, &share_count_ssi2);
 	clk[IMX6QDL_CLK_SSI3_IPG]     = imx_clk_gate2_shared("ssi3_ipg",      "ipg",        base + 0x7c, 22, &share_count_ssi3);

arch/arm/mach-imx/clk-imx6sl.c +2 -0

index e982ebe..6af14a8 100644
@@ -396,6 +396,8 @@ static void __init imx6sl_clocks_init(struct device_node *ccm_node)
 	clks[IMX6SL_CLK_SDMA]         = imx_clk_gate2("sdma",         "ipg",               base + 0x7c, 6);
 	clks[IMX6SL_CLK_SPBA]         = imx_clk_gate2("spba",         "ipg",               base + 0x7c, 12);
 	clks[IMX6SL_CLK_SPDIF]        = imx_clk_gate2("spdif",        "spdif0_podf",       base + 0x7c, 14);
+	/* make sure the initial clock gate setting of share_count is off */
+	writel_relaxed(readl_relaxed(base + 0x7c) & (~(0x3f << 18)), base + 0x7c);
 	clks[IMX6SL_CLK_SSI1_IPG]     = imx_clk_gate2_shared("ssi1_ipg",     "ipg",        base + 0x7c, 18, &share_count_ssi1);
 	clks[IMX6SL_CLK_SSI2_IPG]     = imx_clk_gate2_shared("ssi2_ipg",     "ipg",        base + 0x7c, 20, &share_count_ssi2);
 	clks[IMX6SL_CLK_SSI3_IPG]     = imx_clk_gate2_shared("ssi3_ipg",     "ipg",        base + 0x7c, 22, &share_count_ssi3);

arch/arm/mach-imx/clk-imx6sx.c +6 -0

index 17354a1..e42052e 100644
@@ -379,6 +379,8 @@ static void __init imx6sx_clocks_init(struct device_node *ccm_node)
 	clks[IMX6SX_CLK_AIPS_TZ1]     = imx_clk_gate2("aips_tz1",      "ahb",               base + 0x68, 0);
 	clks[IMX6SX_CLK_AIPS_TZ2]     = imx_clk_gate2("aips_tz2",      "ahb",               base + 0x68, 2);
 	clks[IMX6SX_CLK_APBH_DMA]     = imx_clk_gate2("apbh_dma",      "usdhc3",            base + 0x68, 4);
+	/* make sure the initial clock gate setting of share_count is off */
+	writel_relaxed(readl_relaxed(base + 0x68) & (~(0x3 << 6)), base + 0x68);
 	clks[IMX6SX_CLK_ASRC_MEM]     = imx_clk_gate2_shared("asrc_mem", "ahb",             base + 0x68, 6, &share_count_asrc);
 	clks[IMX6SX_CLK_ASRC_IPG]     = imx_clk_gate2_shared("asrc_ipg", "ahb",             base + 0x68, 6, &share_count_asrc);
 	clks[IMX6SX_CLK_CAAM_MEM]     = imx_clk_gate2("caam_mem",      "ahb",               base + 0x68, 8);

@@ -400,6 +402,8 @@ static void __init imx6sx_clocks_init(struct device_node *ccm_node)
 	clks[IMX6SX_CLK_ECSPI5]       = imx_clk_gate2("ecspi5",        "ecspi_podf",        base + 0x6c, 8);
 	clks[IMX6SX_CLK_EPIT1]        = imx_clk_gate2("epit1",         "perclk",            base + 0x6c, 12);
 	clks[IMX6SX_CLK_EPIT2]        = imx_clk_gate2("epit2",         "perclk",            base + 0x6c, 14);
+	/* make sure the initial clock gate setting of share_count is off */
+	writel_relaxed(readl_relaxed(base + 0x6c) & (~(0x3 << 16)), base + 0x6c);
 	clks[IMX6SX_CLK_ESAI_EXTAL]   = imx_clk_gate2_shared("esai_extal", "esai_podf",     base + 0x6c, 16, &share_count_esai);
 	clks[IMX6SX_CLK_ESAI_IPG]     = imx_clk_gate2_shared("esai_ipg",   "ahb",           base + 0x6c, 16, &share_count_esai);
 	clks[IMX6SX_CLK_ESAI_MEM]     = imx_clk_gate2_shared("esai_mem",   "ahb",           base + 0x6c, 16, &share_count_esai);

@@ -455,6 +459,8 @@ static void __init imx6sx_clocks_init(struct device_node *ccm_node)
 	clks[IMX6SX_CLK_ROM]          = imx_clk_gate2("rom",           "ahb",               base + 0x7c, 0);
 	clks[IMX6SX_CLK_SDMA]         = imx_clk_gate2("sdma",          "ahb",               base + 0x7c, 6);
 	clks[IMX6SX_CLK_SPBA]         = imx_clk_gate2("spba",          "ipg",               base + 0x7c, 12);
+	/* make sure the initial clock gate setting of share_count is off */
+	writel_relaxed(readl_relaxed(base + 0x7c) & (~(0x3 << 14)) & (~(0x3f << 18)), base + 0x7c);
 	clks[IMX6SX_CLK_AUDIO]        = imx_clk_gate2_shared("audio",  "audio_podf",        base + 0x7c, 14, &share_count_audio);
 	clks[IMX6SX_CLK_SPDIF]        = imx_clk_gate2_shared("spdif",  "spdif_podf",        base + 0x7c, 14, &share_count_audio);
 	clks[IMX6SX_CLK_SSI1_IPG]     = imx_clk_gate2_shared("ssi1_ipg",      "ipg",        base + 0x7c, 18, &share_count_ssi1);

Signature · 1 line

1.7.9.5

Sascha Hauer

maintainer

2014-12-09 08:46 UTC #1 lore raw

PATCH 1/1 arm: imx: correct the hardware clock gate setting for shared nodes

Hi,

Quoted 8 lines · Anson Huang

For those clk gates which hold share count, since its is_enabled callback is only checking the share count rather than reading the hardware register setting, in the late phase of kernel bootup, the clk_disable_unused action will NOT handle the scenario of share_count is 0 but the hardware setting is enabled, actually, uboot normally enables all clk gates, then those shared clk gates will be always enabled until they are used by some modules.

Quoted 6 lines

So the problem would be: when kernel boot up, the usecount cat from clk tree is 0, but the clk gates actually is enabled in hardware register, it will confuse user and bring unnecessary power consumption, take i.MX6SX for example, the ESAI clk info is as below, the use count is 0, but the hardware register read from CCM_CCGR1_CG8 is ungated.

I believe the problem is that the shared count is not increased correctly during registration. When it would be increased during registration for each clock that is enabled in hardware clock_disable_unused could do its job like intended.

Could you test the attached patch?

-------------------------------8<--------------------------

Quoted 1 line

From 8095ee6e407f33887b2afa504767d00fcca4f10c Mon Sep 17 00:00:00 2001

From: Sascha Hauer <[email protected]>
Date: Tue, 9 Dec 2014 09:39:06 +0100
Subject: [PATCH] ARM: i.MX: clk-gate2: Fix shared clock enable counting

When during registration of a shared clock the clock is enabled in hardware the shared_count must be increased. This makes sure that during clk_disable the shared_count is decreased first before actually diabling the clock. When done like this the is_enabled callback can return the hardware state of the clock, like it is intended.

Signed-off-by Sascha Hauer <[email protected]>

---
 arch/arm/mach-imx/clk-gate2.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

arch/arm/mach-imx/clk-gate2.c +4 -4

index 5a75cdc..a673364 100644
@@ -96,10 +96,7 @@ static int clk_gate2_is_enabled(struct clk_hw *hw)
 {
 	struct clk_gate2 *gate = to_clk_gate2(hw);
 
-	if (gate->share_count)
-		return !!__clk_get_enable_count(hw->clk);
-	else
-		return clk_gate2_reg_is_enabled(gate->reg, gate->bit_idx);
+	return clk_gate2_reg_is_enabled(gate->reg, gate->bit_idx);
 }
 
 static struct clk_ops clk_gate2_ops = {

@@ -129,6 +126,9 @@ struct clk *clk_register_gate2(struct device *dev, const char *name,
 	gate->lock = lock;
 	gate->share_count = share_count;
 
+	if (share_count && clk_gate2_reg_is_enabled(reg, bit_idx))
+		(*share_count)++;
+
 	init.name = name;
 	init.ops = &clk_gate2_ops;
 	init.flags = flags;

Signature · 8 lines

2.1.3


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Uwe Kleine-König

2014-12-09 09:20 UTC #2 lore raw

PATCH 1/1 arm: imx: correct the hardware clock gate setting for shared nodes

Hallo Sascha,

Quoted 59 lines · Sascha Hauer

Quoted 8 lines · Anson Huang
For those clk gates which hold share count, since its is_enabled callback is only checking the share count rather than reading the hardware register setting, in the late phase of kernel bootup, the clk_disable_unused action will NOT handle the scenario of share_count is 0 but the hardware setting is enabled, actually, uboot normally enables all clk gates, then those shared clk gates will be always enabled until they are used by some modules.
Quoted 6 lines
So the problem would be: when kernel boot up, the usecount cat from clk tree is 0, but the clk gates actually is enabled in hardware register, it will confuse user and bring unnecessary power consumption, take i.MX6SX for example, the ESAI clk info is as below, the use count is 0, but the hardware register read from CCM_CCGR1_CG8 is ungated.
I believe the problem is that the shared count is not increased correctly during registration. When it would be increased during registration for each clock that is enabled in hardware clock_disable_unused could do its job like intended.
Could you test the attached patch?
-------------------------------8<--------------------------
From 8095ee6e407f33887b2afa504767d00fcca4f10c Mon Sep 17 00:00:00 2001
From: Sascha Hauer <[email protected]>
Date: Tue, 9 Dec 2014 09:39:06 +0100
Subject: [PATCH] ARM: i.MX: clk-gate2: Fix shared clock enable counting
When during registration of a shared clock the clock is enabled in hardware the shared_count must be increased. This makes sure that during clk_disable the shared_count is decreased first before actually diabling the clock. When done like this the is_enabled callback can return the hardware state of the clock, like it is intended.
Signed-off-by Sascha Hauer <[email protected]>
---
 arch/arm/mach-imx/clk-gate2.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
arch/arm/mach-imx/clk-gate2.c +1 -4
index 5a75cdc..a673364 100644
@@ -96,10 +96,7 @@ static int clk_gate2_is_enabled(struct clk_hw *hw)
 {
 	struct clk_gate2 *gate = to_clk_gate2(hw);
 
-	if (gate->share_count)
-		return !!__clk_get_enable_count(hw->clk);
-	else
-		return clk_gate2_reg_is_enabled(gate->reg, gate->bit_idx);
+	return clk_gate2_reg_is_enabled(gate->reg, gate->bit_idx);
 }
 
 static struct clk_ops clk_gate2_ops = {

arch/arm/mach-imx/clk-gate2.c struct clk *clk_register_gate2(struct device *dev, const char *name, · line 131

Quoted 6 lines

arch/arm/mach-imx/clk-gate2.c +3 -0

@@ -129,6 +126,9 @@ struct clk *clk_register_gate2(struct device *dev, const char *name,
 	gate->lock = lock;
 	gate->share_count = share_count;
 
+	if (share_count && clk_gate2_reg_is_enabled(reg, bit_idx))
+		(*share_count)++;
+

This is equivalent to calling clk_enable on this clk. So the result is that clk_disable_unused now succeeds to disable if all related clks are unused. But it introduces a new problem I think.

Consider that the bootloader starts linux with the clk on, so assuming 3 shared clks share_count ends at 3 after the clocks are registered. Then a driver makes use of one of them, calls clk_prepare_enable on it, share_count becomes 4, enable_count = 1. clk_disable_unused disables the two unused clks making share_count = 2. After the driver now calls clk_disable the clk stays on. -> bad

Best regards
Uwe

Signature · 2 lines

Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Uwe Kleine-König

2014-12-09 08:57 UTC #3 lore raw

PATCH 1/1 arm: imx: correct the hardware clock gate setting for shared nodes

In reply to Anson Huang

Hello Anson,

[Cc += Mike Turquette]

Quoted 6 lines · Anson Huang

For those clk gates which hold share count, since its is_enabled callback is only checking the share count rather than reading the hardware register setting, in the late phase of kernel bootup, the clk_disable_unused action will NOT handle the scenario of share_count is 0 but the hardware setting is enabled, actually, uboot normally enables all clk gates, then those shared clk gates

s/uboot/U-Boot/ IIRC

Quoted 1 line

will be always enabled until they are used by some modules.

What is the purpose of the shared clocks? Just to have different names for a single gate?

Wouldn't it be better to fix this problem at the framework level? Then ideally clk_disable_unused would handle these shared clocks just fine.

I guess the problem is that this shared clk implementation doesn't fulfill the expectations of the clk framework.

This is found in arch/arm/mach-imx/clk-imx6q.c

arch/arm/mach-imx/clk-imx6q.c static void __init imx6q_clocks_init(struct device_node *ccm_node) · line 376

Quoted 3 lines

arch/arm/mach-imx/clk-imx6q.c +0 -0

 	clk[IMX6QDL_CLK_ASRC]         = imx_clk_gate2_shared("asrc",         "asrc_podf",   base + 0x68, 6, &share_count_asrc);
 	clk[IMX6QDL_CLK_ASRC_IPG]     = imx_clk_gate2_shared("asrc_ipg",     "ahb",         base + 0x68, 6, &share_count_asrc);
 	clk[IMX6QDL_CLK_ASRC_MEM]     = imx_clk_gate2_shared("asrc_mem",     "ahb",         base + 0x68, 6, &share_count_asrc);

For these clks is_enabled returns just enable_count instead of the status of the corresponding bit in the corresponding register. The reason for that probably is that "asrc" must not be disabled even if unused if "asrc_ipg" is in use. So the problem is there are three struct clk s with a common enable count for a single register bit. Interesting enough these clocks have different parents (the 2nd parameter to imx_clk_gate2_shared) which makes the easy solution to just make asrc_ipg and asrc_mem children of asrc not applicable. Any idea for a sane solution? A multi-parent-dummy-clk would do I think.

Other than that the following might work: Ignore share_count in the is_enabled callback (i.e. report if the register bit is set as in the share_count == NULL case) and implement clk->ops->disable_unused that only unsets the bit if share_count == 0? Ugly but IMHO better and more robust than the idea implemented in this patch. Still I'd prefer to make the usual assumptions of the clk framework just work.

Quoted 8 lines

So the problem would be: when kernel boot up, the usecount cat from clk tree is 0, but the clk gates actually is enabled in hardware register, it will confuse user and bring unnecessary power consumption, take i.MX6SX for example, the ESAI clk info is as below, the use count is 0, but the hardware register read from CCM_CCGR1_CG8 is ungated.
cat /sys/kernel/debug/clk/clk_summary | grep esai

Just a very minor nitpick: Unnecessary usage of cat,

grep esai /sys/kernel/debug/clk/clk_summary

just produces the same result. :-)

Quoted 11 lines

esai_sel 0 0 393216000 0 0
esai_pred 0 0 196608000 0 0
esai_podf 0 0 24576000 0 0
esai_extal 0 0 24576000 0 0
esai_mem 0 0 132000000 0 0
esai_ipg 0 0 132000000 0 0
Read CCM_CCGR1:
0x020C406C: F33FFF00

This is hardly useful if you don't have the reference manual at hand. I'd believe the commit log even without the examples.

Best regards
Uwe

Signature · 2 lines

Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Anson.Huang at freescale.com

2014-12-10 08:49 UTC #4 lore raw

PATCH 1/1 arm: imx: correct the hardware clock gate setting for shared nodes

Hi, Uwe

Best regards!
Anson Huang

-----Original Message-----
From: Uwe Kleine-K?nig [mailto:u.kleine-koenig at pengutronix.de]
Sent: 2014-12-09 4:58 PM
To: Huang Yongcai-B20788
Cc: shawn.guo at linaro.org; kernel at pengutronix.de; linux-arm-kernel at lists.infradead.org; Mike Turquette
Subject: Re: [PATCH] arm: imx: correct the hardware clock gate setting for shared nodes

Hello Anson,

[Cc += Mike Turquette]

Quoted 6 lines · Anson Huang

For those clk gates which hold share count, since its is_enabled callback is only checking the share count rather than reading the hardware register setting, in the late phase of kernel bootup, the clk_disable_unused action will NOT handle the scenario of share_count is 0 but the hardware setting is enabled, actually, uboot normally enables all clk gates, then those shared clk gates

s/uboot/U-Boot/ IIRC

Quoted 1 line

will be always enabled until they are used by some modules.

What is the purpose of the shared clocks? Just to have different names for a single gate?
[Anson] The shared clocks is to meet the case that where one clk gate control multi clks path in module, for example, in esai module, there are three clk paths, esai_core, esai_ipg and esai_mem, but they share same clk gate in CCM_CCGR, and their parents are different...No very good hardware design for us.

Wouldn't it be better to fix this problem at the framework level? Then ideally clk_disable_unused would handle these shared clocks just fine. [Anson] Agree, handle it in framework is much better than doing it in every platform, my bad:(

I guess the problem is that this shared clk implementation doesn't fulfill the expectations of the clk framework.

This is found in arch/arm/mach-imx/clk-imx6q.c

arch/arm/mach-imx/clk-imx6q.c static void __init imx6q_clocks_init(struct device_node *ccm_node) · line 376

Quoted 3 lines

arch/arm/mach-imx/clk-imx6q.c +0 -0

 	clk[IMX6QDL_CLK_ASRC]         = imx_clk_gate2_shared("asrc",         "asrc_podf",   base + 0x68, 6, &share_count_asrc);
 	clk[IMX6QDL_CLK_ASRC_IPG]     = imx_clk_gate2_shared("asrc_ipg",     "ahb",         base + 0x68, 6, &share_count_asrc);
 	clk[IMX6QDL_CLK_ASRC_MEM]     = imx_clk_gate2_shared("asrc_mem",     "ahb",         base + 0x68, 6, &share_count_asrc);

Quoted 8 lines

So the problem would be: when kernel boot up, the usecount cat from clk tree is 0, but the clk gates actually is enabled in hardware register, it will confuse user and bring unnecessary power consumption, take i.MX6SX for example, the ESAI clk info is as below, the use count is 0, but the hardware register read from CCM_CCGR1_CG8 is ungated.
cat /sys/kernel/debug/clk/clk_summary | grep esai

Just a very minor nitpick: Unnecessary usage of cat,

grep esai /sys/kernel/debug/clk/clk_summary

just produces the same result. :-)
[Anson] Thanks for sharing such knowledge:)

Quoted 11 lines

esai_sel 0 0 393216000 0 0
esai_pred 0 0 196608000 0 0
esai_podf 0 0 24576000 0 0
esai_extal 0 0 24576000 0 0
esai_mem 0 0 132000000 0 0
esai_ipg 0 0 132000000 0 0
Read CCM_CCGR1:
0x020C406C: F33FFF00

This is hardly useful if you don't have the reference manual at hand. I'd believe the commit log even without the examples. [Anson] Agreed, I will remove it in V2 patch.

Many thanks for reviewing my patch and very good advice:) Please help review V2 patch.

Best regards
Uwe

Signature · 2 lines

Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Uwe Kleine-König

2014-12-10 09:09 UTC #5 lore raw

PATCH 1/1 arm: imx: correct the hardware clock gate setting for shared nodes

Hello,

Quoted 11 lines · Anson.Huang at freescale.com

Quoted 11 lines
esai_sel 0 0 393216000 0 0
esai_pred 0 0 196608000 0 0
esai_podf 0 0 24576000 0 0
esai_extal 0 0 24576000 0 0
esai_mem 0 0 132000000 0 0
esai_ipg 0 0 132000000 0 0
Read CCM_CCGR1:
0x020C406C: F33FFF00

Quoted 13 lines

This is hardly useful if you don't have the reference manual at hand. I'd believe the commit log even without the examples. [Anson] Agreed, I will remove it in V2 patch.
Many thanks for reviewing my patch and very good advice:) Please help review V2 patch.
Best regards
Uwe
Signature · 2 lines
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

If you want to do me (and most of the other readers of lakml I guess) another favour: please fix your quoting style. The way you replied to my mail is hard to parse in a console mail reader as you might be able to see from the text I quoted above.

Best regards
Uwe

Signature · 2 lines

Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

A message that is not in the archive (sent off-list or not yet received): 20141210091126.GA10165@ubuntu

Uwe Kleine-König

2014-12-10 10:16 UTC #7 lore raw

PATCH 1/1 arm: imx: correct the hardware clock gate setting for shared nodes

Hello,

Quoted 2 lines · Anson Huang

OK, very sorry for using OUTLOOK to reply such review mail, I will use console mail to reply it from now:)

Much better \o/. Note that Outlook can be tuned to behave better, too (still not perfect AFAIK). Back when I was forced to use Outlook I liked Outlook Quote Fix[1]. I hope this is not necessary any more today.

Best regards
Uwe

[1] http://home.in.tum.de/~jain/software/outlook-quotefix/

Signature · 2 lines

Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |