Skip to contentSkip to conversation

clk: exynos4: Add CLK_GET_RATE_NOCACHE flag for the Exynos4x12 ISP clocks

PATCH New Sylwester Nawrocki 2 participants 4 messages Jul 25, 2013 – Jul 26, 2013

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 clk: exynos4: Add CLK_GET_RATE_NOCACHE flag for the Exynos4x12 ISP clocks

From: Sylwester Nawrocki <[email protected]>

The ISP clock registers belong to the ISP power domain and may change their values if this power domain is switched off/on. Add CLK_GET_RATE_NOCACHE flags to ensure we do not rely on invalid cached data when setting or getting frequency of those clocks.

---

Rebased onto v3.11-rc2.

 drivers/clk/samsung/clk-exynos4.c |   64 +++++++++++++++++++-----------------
 1 files changed, 34 insertions(+), 30 deletions(-)
drivers/clk/samsung/clk-exynos4.c +34 -30
index 1bdb882..4e57397 100644
@@ -581,11 +581,15 @@ struct samsung_div_clock exynos4x12_div_clks[] __initdata = {
 	DIV(none, "div_spi1_isp", "mout_spi1_isp", E4X12_DIV_ISP, 16, 4),
 	DIV(none, "div_spi1_isp_pre", "div_spi1_isp", E4X12_DIV_ISP, 20, 8),
 	DIV(none, "div_uart_isp", "mout_uart_isp", E4X12_DIV_ISP, 28, 4),
-	DIV(div_isp0, "div_isp0", "aclk200", E4X12_DIV_ISP0, 0, 3),
-	DIV(div_isp1, "div_isp1", "aclk200", E4X12_DIV_ISP0, 4, 3),
+	DIV_F(div_isp0, "div_isp0", "aclk200", E4X12_DIV_ISP0, 0, 3,
+						CLK_GET_RATE_NOCACHE, 0),
+	DIV_F(div_isp1, "div_isp1", "aclk200", E4X12_DIV_ISP0, 4, 3,
+						CLK_GET_RATE_NOCACHE, 0),
 	DIV(none, "div_mpwm", "div_isp1", E4X12_DIV_ISP1, 0, 3),
-	DIV(div_mcuisp0, "div_mcuisp0", "aclk400_mcuisp", E4X12_DIV_ISP1, 4, 3),
-	DIV(div_mcuisp1, "div_mcuisp1", "div_mcuisp0", E4X12_DIV_ISP1, 8, 3),
+	DIV_F(div_mcuisp0, "div_mcuisp0", "aclk400_mcuisp", E4X12_DIV_ISP1,
+						4, 3, CLK_GET_RATE_NOCACHE, 0),
+	DIV_F(div_mcuisp1, "div_mcuisp1", "div_mcuisp0", E4X12_DIV_ISP1,
+						8, 3, CLK_GET_RATE_NOCACHE, 0),
 	DIV(sclk_fimg2d, "sclk_fimg2d", "mout_g2d", DIV_DMC1, 0, 4),
 };
 
@@ -863,57 +867,57 @@ struct samsung_gate_clock exynos4x12_gate_clks[] __initdata = {
 	GATE_DA(i2s0, "samsung-i2s.0", "i2s0", "aclk100",
 			E4X12_GATE_IP_MAUDIO, 3, 0, 0, "iis"),
 	GATE(fimc_isp, "isp", "aclk200", E4X12_GATE_ISP0, 0,
-			CLK_IGNORE_UNUSED, 0),
+			CLK_IGNORE_UNUSED | CLK_GET_RATE_NOCACHE, 0),
 	GATE(fimc_drc, "drc", "aclk200", E4X12_GATE_ISP0, 1,
-			CLK_IGNORE_UNUSED, 0),
+			CLK_IGNORE_UNUSED | CLK_GET_RATE_NOCACHE, 0),
 	GATE(fimc_fd, "fd", "aclk200", E4X12_GATE_ISP0, 2,
-			CLK_IGNORE_UNUSED, 0),
+			CLK_IGNORE_UNUSED | CLK_GET_RATE_NOCACHE, 0),
 	GATE(fimc_lite0, "lite0", "aclk200", E4X12_GATE_ISP0, 3,
-			CLK_IGNORE_UNUSED, 0),
+			CLK_IGNORE_UNUSED | CLK_GET_RATE_NOCACHE, 0),
 	GATE(fimc_lite1, "lite1", "aclk200", E4X12_GATE_ISP0, 4,
-			CLK_IGNORE_UNUSED, 0),
+			CLK_IGNORE_UNUSED | CLK_GET_RATE_NOCACHE, 0),
 	GATE(mcuisp, "mcuisp", "aclk200", E4X12_GATE_ISP0, 5,
-			CLK_IGNORE_UNUSED, 0),
+			CLK_IGNORE_UNUSED | CLK_GET_RATE_NOCACHE, 0),
 	GATE(gicisp, "gicisp", "aclk200", E4X12_GATE_ISP0, 7,
-			CLK_IGNORE_UNUSED, 0),
+			CLK_IGNORE_UNUSED | CLK_GET_RATE_NOCACHE, 0),
 	GATE(smmu_isp, "smmu_isp", "aclk200", E4X12_GATE_ISP0, 8,
-			CLK_IGNORE_UNUSED, 0),
+			CLK_IGNORE_UNUSED | CLK_GET_RATE_NOCACHE, 0),
 	GATE(smmu_drc, "smmu_drc", "aclk200", E4X12_GATE_ISP0, 9,
-			CLK_IGNORE_UNUSED, 0),
+			CLK_IGNORE_UNUSED | CLK_GET_RATE_NOCACHE, 0),
 	GATE(smmu_fd, "smmu_fd", "aclk200", E4X12_GATE_ISP0, 10,
-			CLK_IGNORE_UNUSED, 0),
+			CLK_IGNORE_UNUSED | CLK_GET_RATE_NOCACHE, 0),
 	GATE(smmu_lite0, "smmu_lite0", "aclk200", E4X12_GATE_ISP0, 11,
-			CLK_IGNORE_UNUSED, 0),
+			CLK_IGNORE_UNUSED | CLK_GET_RATE_NOCACHE, 0),
 	GATE(smmu_lite1, "smmu_lite1", "aclk200", E4X12_GATE_ISP0, 12,
-			CLK_IGNORE_UNUSED, 0),
+			CLK_IGNORE_UNUSED | CLK_GET_RATE_NOCACHE, 0),
 	GATE(ppmuispmx, "ppmuispmx", "aclk200", E4X12_GATE_ISP0, 20,
-			CLK_IGNORE_UNUSED, 0),
+			CLK_IGNORE_UNUSED | CLK_GET_RATE_NOCACHE, 0),
 	GATE(ppmuispx, "ppmuispx", "aclk200", E4X12_GATE_ISP0, 21,
-			CLK_IGNORE_UNUSED, 0),
+			CLK_IGNORE_UNUSED | CLK_GET_RATE_NOCACHE, 0),
 	GATE(mcuctl_isp, "mcuctl_isp", "aclk200", E4X12_GATE_ISP0, 23,
-			CLK_IGNORE_UNUSED, 0),
+			CLK_IGNORE_UNUSED | CLK_GET_RATE_NOCACHE, 0),
 	GATE(mpwm_isp, "mpwm_isp", "aclk200", E4X12_GATE_ISP0, 24,
-			CLK_IGNORE_UNUSED, 0),
+			CLK_IGNORE_UNUSED | CLK_GET_RATE_NOCACHE, 0),
 	GATE(i2c0_isp, "i2c0_isp", "aclk200", E4X12_GATE_ISP0, 25,
-			CLK_IGNORE_UNUSED, 0),
+			CLK_IGNORE_UNUSED | CLK_GET_RATE_NOCACHE, 0),
 	GATE(i2c1_isp, "i2c1_isp", "aclk200", E4X12_GATE_ISP0, 26,
-			CLK_IGNORE_UNUSED, 0),
+			CLK_IGNORE_UNUSED | CLK_GET_RATE_NOCACHE, 0),
 	GATE(mtcadc_isp, "mtcadc_isp", "aclk200", E4X12_GATE_ISP0, 27,
-			CLK_IGNORE_UNUSED, 0),
+			CLK_IGNORE_UNUSED | CLK_GET_RATE_NOCACHE, 0),
 	GATE(pwm_isp, "pwm_isp", "aclk200", E4X12_GATE_ISP0, 28,
-			CLK_IGNORE_UNUSED, 0),
+			CLK_IGNORE_UNUSED | CLK_GET_RATE_NOCACHE, 0),
 	GATE(wdt_isp, "wdt_isp", "aclk200", E4X12_GATE_ISP0, 30,
-			CLK_IGNORE_UNUSED, 0),
+			CLK_IGNORE_UNUSED | CLK_GET_RATE_NOCACHE, 0),
 	GATE(uart_isp, "uart_isp", "aclk200", E4X12_GATE_ISP0, 31,
-			CLK_IGNORE_UNUSED, 0),
+			CLK_IGNORE_UNUSED | CLK_GET_RATE_NOCACHE, 0),
 	GATE(asyncaxim, "asyncaxim", "aclk200", E4X12_GATE_ISP1, 0,
-			CLK_IGNORE_UNUSED, 0),
+			CLK_IGNORE_UNUSED | CLK_GET_RATE_NOCACHE, 0),
 	GATE(smmu_ispcx, "smmu_ispcx", "aclk200", E4X12_GATE_ISP1, 4,
-			CLK_IGNORE_UNUSED, 0),
+			CLK_IGNORE_UNUSED | CLK_GET_RATE_NOCACHE, 0),
 	GATE(spi0_isp, "spi0_isp", "aclk200", E4X12_GATE_ISP1, 12,
-			CLK_IGNORE_UNUSED, 0),
+			CLK_IGNORE_UNUSED | CLK_GET_RATE_NOCACHE, 0),
 	GATE(spi1_isp, "spi1_isp", "aclk200", E4X12_GATE_ISP1, 13,
-			CLK_IGNORE_UNUSED, 0),
+			CLK_IGNORE_UNUSED | CLK_GET_RATE_NOCACHE, 0),
 	GATE(g2d, "g2d", "aclk200", GATE_IP_DMC, 23, 0, 0),
 };
 

--
1.7.4.1

#1 lore raw

PATCH v2 1/1 clk: exynos4: Add CLK_GET_RATE_NOCACHE flag for the Exynos4x12 ISP clocks

Quoting Sylwester Nawrocki (2013-07-25 14:07:05)

Quoted 9 lines

From: Sylwester Nawrocki <[email protected]>

The ISP clock registers belong to the ISP power domain and may change their values if this power domain is switched off/on. Add CLK_GET_RATE_NOCACHE flags to ensure we do not rely on invalid cached data when setting or getting frequency of those clocks.

Thanks for the fix. I've taken into clk-fixes. Is there a specific regression this fixes (besides just having wrong clock rates). I can amend the changelog if you have an example (e.g. device X explodes).

Regards,
Mike

Quoted 117 lines

---

Rebased onto v3.11-rc2.

 drivers/clk/samsung/clk-exynos4.c |   64 +++++++++++++++++++-----------------
 1 files changed, 34 insertions(+), 30 deletions(-)
drivers/clk/samsung/clk-exynos4.c +34 -31
index 1bdb882..4e57397 100644
@@ -581,11 +581,15 @@ struct samsung_div_clock exynos4x12_div_clks[] __initdata = {
        DIV(none, "div_spi1_isp", "mout_spi1_isp", E4X12_DIV_ISP, 16, 4),
        DIV(none, "div_spi1_isp_pre", "div_spi1_isp", E4X12_DIV_ISP, 20, 8),
        DIV(none, "div_uart_isp", "mout_uart_isp", E4X12_DIV_ISP, 28, 4),
-       DIV(div_isp0, "div_isp0", "aclk200", E4X12_DIV_ISP0, 0, 3),
-       DIV(div_isp1, "div_isp1", "aclk200", E4X12_DIV_ISP0, 4, 3),
+       DIV_F(div_isp0, "div_isp0", "aclk200", E4X12_DIV_ISP0, 0, 3,
+                                               CLK_GET_RATE_NOCACHE, 0),
+       DIV_F(div_isp1, "div_isp1", "aclk200", E4X12_DIV_ISP0, 4, 3,
+                                               CLK_GET_RATE_NOCACHE, 0),
        DIV(none, "div_mpwm", "div_isp1", E4X12_DIV_ISP1, 0, 3),
-       DIV(div_mcuisp0, "div_mcuisp0", "aclk400_mcuisp", E4X12_DIV_ISP1, 4, 3),
-       DIV(div_mcuisp1, "div_mcuisp1", "div_mcuisp0", E4X12_DIV_ISP1, 8, 3),
+       DIV_F(div_mcuisp0, "div_mcuisp0", "aclk400_mcuisp", E4X12_DIV_ISP1,
+                                               4, 3, CLK_GET_RATE_NOCACHE, 0),
+       DIV_F(div_mcuisp1, "div_mcuisp1", "div_mcuisp0", E4X12_DIV_ISP1,
+                                               8, 3, CLK_GET_RATE_NOCACHE, 0),
        DIV(sclk_fimg2d, "sclk_fimg2d", "mout_g2d", DIV_DMC1, 0, 4),
 };
 
@@ -863,57 +867,57 @@ struct samsung_gate_clock exynos4x12_gate_clks[] __initdata = {
        GATE_DA(i2s0, "samsung-i2s.0", "i2s0", "aclk100",
                        E4X12_GATE_IP_MAUDIO, 3, 0, 0, "iis"),
        GATE(fimc_isp, "isp", "aclk200", E4X12_GATE_ISP0, 0,
-                       CLK_IGNORE_UNUSED, 0),
+                       CLK_IGNORE_UNUSED | CLK_GET_RATE_NOCACHE, 0),
        GATE(fimc_drc, "drc", "aclk200", E4X12_GATE_ISP0, 1,
-                       CLK_IGNORE_UNUSED, 0),
+                       CLK_IGNORE_UNUSED | CLK_GET_RATE_NOCACHE, 0),
        GATE(fimc_fd, "fd", "aclk200", E4X12_GATE_ISP0, 2,
-                       CLK_IGNORE_UNUSED, 0),
+                       CLK_IGNORE_UNUSED | CLK_GET_RATE_NOCACHE, 0),
        GATE(fimc_lite0, "lite0", "aclk200", E4X12_GATE_ISP0, 3,
-                       CLK_IGNORE_UNUSED, 0),
+                       CLK_IGNORE_UNUSED | CLK_GET_RATE_NOCACHE, 0),
        GATE(fimc_lite1, "lite1", "aclk200", E4X12_GATE_ISP0, 4,
-                       CLK_IGNORE_UNUSED, 0),
+                       CLK_IGNORE_UNUSED | CLK_GET_RATE_NOCACHE, 0),
        GATE(mcuisp, "mcuisp", "aclk200", E4X12_GATE_ISP0, 5,
-                       CLK_IGNORE_UNUSED, 0),
+                       CLK_IGNORE_UNUSED | CLK_GET_RATE_NOCACHE, 0),
        GATE(gicisp, "gicisp", "aclk200", E4X12_GATE_ISP0, 7,
-                       CLK_IGNORE_UNUSED, 0),
+                       CLK_IGNORE_UNUSED | CLK_GET_RATE_NOCACHE, 0),
        GATE(smmu_isp, "smmu_isp", "aclk200", E4X12_GATE_ISP0, 8,
-                       CLK_IGNORE_UNUSED, 0),
+                       CLK_IGNORE_UNUSED | CLK_GET_RATE_NOCACHE, 0),
        GATE(smmu_drc, "smmu_drc", "aclk200", E4X12_GATE_ISP0, 9,
-                       CLK_IGNORE_UNUSED, 0),
+                       CLK_IGNORE_UNUSED | CLK_GET_RATE_NOCACHE, 0),
        GATE(smmu_fd, "smmu_fd", "aclk200", E4X12_GATE_ISP0, 10,
-                       CLK_IGNORE_UNUSED, 0),
+                       CLK_IGNORE_UNUSED | CLK_GET_RATE_NOCACHE, 0),
        GATE(smmu_lite0, "smmu_lite0", "aclk200", E4X12_GATE_ISP0, 11,
-                       CLK_IGNORE_UNUSED, 0),
+                       CLK_IGNORE_UNUSED | CLK_GET_RATE_NOCACHE, 0),
        GATE(smmu_lite1, "smmu_lite1", "aclk200", E4X12_GATE_ISP0, 12,
-                       CLK_IGNORE_UNUSED, 0),
+                       CLK_IGNORE_UNUSED | CLK_GET_RATE_NOCACHE, 0),
        GATE(ppmuispmx, "ppmuispmx", "aclk200", E4X12_GATE_ISP0, 20,
-                       CLK_IGNORE_UNUSED, 0),
+                       CLK_IGNORE_UNUSED | CLK_GET_RATE_NOCACHE, 0),
        GATE(ppmuispx, "ppmuispx", "aclk200", E4X12_GATE_ISP0, 21,
-                       CLK_IGNORE_UNUSED, 0),
+                       CLK_IGNORE_UNUSED | CLK_GET_RATE_NOCACHE, 0),
        GATE(mcuctl_isp, "mcuctl_isp", "aclk200", E4X12_GATE_ISP0, 23,
-                       CLK_IGNORE_UNUSED, 0),
+                       CLK_IGNORE_UNUSED | CLK_GET_RATE_NOCACHE, 0),
        GATE(mpwm_isp, "mpwm_isp", "aclk200", E4X12_GATE_ISP0, 24,
-                       CLK_IGNORE_UNUSED, 0),
+                       CLK_IGNORE_UNUSED | CLK_GET_RATE_NOCACHE, 0),
        GATE(i2c0_isp, "i2c0_isp", "aclk200", E4X12_GATE_ISP0, 25,
-                       CLK_IGNORE_UNUSED, 0),
+                       CLK_IGNORE_UNUSED | CLK_GET_RATE_NOCACHE, 0),
        GATE(i2c1_isp, "i2c1_isp", "aclk200", E4X12_GATE_ISP0, 26,
-                       CLK_IGNORE_UNUSED, 0),
+                       CLK_IGNORE_UNUSED | CLK_GET_RATE_NOCACHE, 0),
        GATE(mtcadc_isp, "mtcadc_isp", "aclk200", E4X12_GATE_ISP0, 27,
-                       CLK_IGNORE_UNUSED, 0),
+                       CLK_IGNORE_UNUSED | CLK_GET_RATE_NOCACHE, 0),
        GATE(pwm_isp, "pwm_isp", "aclk200", E4X12_GATE_ISP0, 28,
-                       CLK_IGNORE_UNUSED, 0),
+                       CLK_IGNORE_UNUSED | CLK_GET_RATE_NOCACHE, 0),
        GATE(wdt_isp, "wdt_isp", "aclk200", E4X12_GATE_ISP0, 30,
-                       CLK_IGNORE_UNUSED, 0),
+                       CLK_IGNORE_UNUSED | CLK_GET_RATE_NOCACHE, 0),
        GATE(uart_isp, "uart_isp", "aclk200", E4X12_GATE_ISP0, 31,
-                       CLK_IGNORE_UNUSED, 0),
+                       CLK_IGNORE_UNUSED | CLK_GET_RATE_NOCACHE, 0),
        GATE(asyncaxim, "asyncaxim", "aclk200", E4X12_GATE_ISP1, 0,
-                       CLK_IGNORE_UNUSED, 0),
+                       CLK_IGNORE_UNUSED | CLK_GET_RATE_NOCACHE, 0),
        GATE(smmu_ispcx, "smmu_ispcx", "aclk200", E4X12_GATE_ISP1, 4,
-                       CLK_IGNORE_UNUSED, 0),
+                       CLK_IGNORE_UNUSED | CLK_GET_RATE_NOCACHE, 0),
        GATE(spi0_isp, "spi0_isp", "aclk200", E4X12_GATE_ISP1, 12,
-                       CLK_IGNORE_UNUSED, 0),
+                       CLK_IGNORE_UNUSED | CLK_GET_RATE_NOCACHE, 0),
        GATE(spi1_isp, "spi1_isp", "aclk200", E4X12_GATE_ISP1, 13,
-                       CLK_IGNORE_UNUSED, 0),
+                       CLK_IGNORE_UNUSED | CLK_GET_RATE_NOCACHE, 0),
        GATE(g2d, "g2d", "aclk200", GATE_IP_DMC, 23, 0, 0),
 };
 
--

1.7.4.1

#2 lore raw
Quoted 8 lines · Mike Turquette

Quoting Sylwester Nawrocki (2013-07-25 14:07:05)

Quoted 7 lines

From: Sylwester Nawrocki<[email protected]>

The ISP clock registers belong to the ISP power domain and may change their values if this power domain is switched off/on. Add CLK_GET_RATE_NOCACHE flags to ensure we do not rely on invalid cached data when setting or getting frequency of those clocks.

Quoted 6 lines
Quoted 2 lines

Thanks for the fix. I've taken into clk-fixes. Is there a specific regression this fixes (besides just having wrong clock rates). I can amend the changelog if you have an example (e.g. device X explodes).

Hmm, yes, this fixes a pretty serious problem. There is a companion commit [1] already in Linus' tree, more details can be found there.

Perhaps something like this could be added:

"Otherwise the FIMC-IS Cortex-A5 core and AXI bus clocks have incorrect frequencies, which breaks the ISP operation and starting the video pipeline fails with timeouts reported by the FIMC-IS firmware.

See related commit 722a860ecb29aa34ec6f7d7f32b949209e8 "[media] exynos4-is: Fix FIMC-IS clocks initialization" for more details."

[1]
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit?id=722a860ecb29aa34ec6f7d7f32b949209e86a2f3

Thanks,
Sylwester

#3 lore raw

PATCH v2 1/1 clk: exynos4: Add CLK_GET_RATE_NOCACHE flag for the Exynos4x12 ISP clocks

Quoting Sylwester Nawrocki (2013-07-26 15:04:30)

Quoted 21 lines · Mike Turquette
Quoted 8 lines

Quoting Sylwester Nawrocki (2013-07-25 14:07:05)

Quoted 7 lines

From: Sylwester Nawrocki<[email protected]>

The ISP clock registers belong to the ISP power domain and may change their values if this power domain is switched off/on. Add CLK_GET_RATE_NOCACHE flags to ensure we do not rely on invalid cached data when setting or getting frequency of those clocks.

Quoted 6 lines
Quoted 2 lines

Thanks for the fix. I've taken into clk-fixes. Is there a specific regression this fixes (besides just having wrong clock rates). I can amend the changelog if you have an example (e.g. device X explodes).

Hmm, yes, this fixes a pretty serious problem. There is a companion commit [1] already in Linus' tree, more details can be found there.

Perhaps something like this could be added:

Quoted 8 lines

"Otherwise the FIMC-IS Cortex-A5 core and AXI bus clocks have incorrect frequencies, which breaks the ISP operation and starting the video pipeline fails with timeouts reported by the FIMC-IS firmware.

See related commit 722a860ecb29aa34ec6f7d7f32b949209e8 "[media] exynos4-is: Fix FIMC-IS clocks initialization" for more details."

That's great.

Thanks,
Mike

Quoted 6 lines

[1]
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit?id=722a860ecb29aa34ec6f7d7f32b949209e86a2f3

Thanks,
Sylwester