linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] ARM: dts: Fix imprecise external abort error when accessing Exynos MFC
@ 2016-05-24 17:41 Javier Martinez Canillas
  2016-05-24 17:41 ` [PATCH 1/2] clk: exynos5420: Set ID for aclk333 gate clock Javier Martinez Canillas
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Javier Martinez Canillas @ 2016-05-24 17:41 UTC (permalink / raw)
  To: linux-kernel
  Cc: devicetree, Kukjin Kim, Michael Turquette, Krzysztof Kozlowski,
	Marek Szyprowski, Mauro Carvalho Chehab, Shuah Khan,
	Stephen Boyd, linux-samsung-soc, linux-arm-kernel,
	Sylwester Nawrocki, Tomasz Figa, linux-clk, Nicolas Dufresne,
	Javier Martinez Canillas

Hello,

This series fixes an imprecise external abort error when accessing the
Exynos MFC registers due the power domain configuration requiring the
aclk333 clock to be enabled during a domain switch.

There isn't a dependency between the clock and Linux Samsung SoC trees
because the CLK_ACLK333 clock ID is already defined so the patches can
be picked indepedently by the relevant subsystem maintainers.

Best regards,
Javier


Javier Martinez Canillas (2):
  clk: exynos5420: Set ID for aclk333 gate clock
  ARM: dts: Add async-bridge clock to MFC power domain for Exynos5420

 arch/arm/boot/dts/exynos5420.dtsi    | 5 +++--
 drivers/clk/samsung/clk-exynos5420.c | 2 +-
 2 files changed, 4 insertions(+), 3 deletions(-)

-- 
2.5.5

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

* [PATCH 1/2] clk: exynos5420: Set ID for aclk333 gate clock
  2016-05-24 17:41 [PATCH 0/2] ARM: dts: Fix imprecise external abort error when accessing Exynos MFC Javier Martinez Canillas
@ 2016-05-24 17:41 ` Javier Martinez Canillas
  2016-05-25  7:11   ` Krzysztof Kozlowski
  2016-05-30 12:49   ` Sylwester Nawrocki
  2016-05-24 17:41 ` [PATCH 2/2] ARM: dts: Add async-bridge clock to MFC power domain for Exynos5420 Javier Martinez Canillas
  2016-05-25  6:51 ` [PATCH 0/2] ARM: dts: Fix imprecise external abort error when accessing Exynos MFC Marek Szyprowski
  2 siblings, 2 replies; 11+ messages in thread
From: Javier Martinez Canillas @ 2016-05-24 17:41 UTC (permalink / raw)
  To: linux-kernel
  Cc: devicetree, Kukjin Kim, Michael Turquette, Krzysztof Kozlowski,
	Marek Szyprowski, Mauro Carvalho Chehab, Shuah Khan,
	Stephen Boyd, linux-samsung-soc, linux-arm-kernel,
	Sylwester Nawrocki, Tomasz Figa, linux-clk, Nicolas Dufresne,
	Javier Martinez Canillas

The aclk333 clock needs to be ungated during the MFC power domain switch,
so set the clock ID to allow the Exynos power domain logic to lookup this
clock if is defined in the MFC PD device tree node.

Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
---

 drivers/clk/samsung/clk-exynos5420.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/samsung/clk-exynos5420.c b/drivers/clk/samsung/clk-exynos5420.c
index 92382cef9f90..469bcae5de3a 100644
--- a/drivers/clk/samsung/clk-exynos5420.c
+++ b/drivers/clk/samsung/clk-exynos5420.c
@@ -946,7 +946,7 @@ static struct samsung_gate_clock exynos5x_gate_clks[] __initdata = {
 			GATE_BUS_TOP, 13, 0, 0),
 	GATE(0, "aclk166", "mout_user_aclk166",
 			GATE_BUS_TOP, 14, CLK_IGNORE_UNUSED, 0),
-	GATE(0, "aclk333", "mout_user_aclk333",
+	GATE(CLK_ACLK333, "aclk333", "mout_user_aclk333",
 			GATE_BUS_TOP, 15, CLK_IGNORE_UNUSED, 0),
 	GATE(0, "aclk400_isp", "mout_user_aclk400_isp",
 			GATE_BUS_TOP, 16, 0, 0),
-- 
2.5.5

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

* [PATCH 2/2] ARM: dts: Add async-bridge clock to MFC power domain for Exynos5420
  2016-05-24 17:41 [PATCH 0/2] ARM: dts: Fix imprecise external abort error when accessing Exynos MFC Javier Martinez Canillas
  2016-05-24 17:41 ` [PATCH 1/2] clk: exynos5420: Set ID for aclk333 gate clock Javier Martinez Canillas
@ 2016-05-24 17:41 ` Javier Martinez Canillas
  2016-05-25  7:48   ` Krzysztof Kozlowski
  2016-05-30  7:41   ` Krzysztof Kozlowski
  2016-05-25  6:51 ` [PATCH 0/2] ARM: dts: Fix imprecise external abort error when accessing Exynos MFC Marek Szyprowski
  2 siblings, 2 replies; 11+ messages in thread
From: Javier Martinez Canillas @ 2016-05-24 17:41 UTC (permalink / raw)
  To: linux-kernel
  Cc: devicetree, Kukjin Kim, Michael Turquette, Krzysztof Kozlowski,
	Marek Szyprowski, Mauro Carvalho Chehab, Shuah Khan,
	Stephen Boyd, linux-samsung-soc, linux-arm-kernel,
	Sylwester Nawrocki, Tomasz Figa, linux-clk, Nicolas Dufresne,
	Javier Martinez Canillas

The MFC IP is also inter-connected by an Async-Bridge so the CLK_ACLK333
has to be ungated during a power domain switch. Trying to do it when the
clock is gated will fail and lead to an imprecise external abort error
when the driver tries to access the MFC registers with the PD disabled.

For example, if the s5p-mfc module is removed and the MFC PD turned off:

[  186.835606] Power domain power-domain@10044060 disable failed
[  186.835671] s5p-mfc 11000000.codec: Removing 11000000.codec
[  186.837670] Power domain power-domain@10044060 disable failed

And when the module is inserted again:

[ 2395.176956] s5p_mfc_wait_for_done_dev:34: Interrupt (dev->int_type:0, command:12) timed out
[ 2395.177031] s5p_mfc_init_hw:272: Failed to load firmware
[ 2395.177384] Unhandled fault: imprecise external abort (0x1406) at 0x00000000
[ 2395.177441] pgd = ec3b4000
[ 2395.177467] [00000000] *pgd=00000000
[ 2395.177507] Internal error: : 1406 [#1] PREEMPT SMP ARM
[ 2395.177550] Modules linked in: s5p_mfc mwifiex_sdio mwifiex uvcvideo s5p_jpeg v4l2_mem2mem videobuf2_vmalloc videobuf2_dma_contig videobuf2_memops videobuf2_v4l2 videobuf2_core v4l2_common videodev media [last unloaded: s5p_mfc]
[ 2395.177774] CPU: 1 PID: 2382 Comm: v4l_id Tainted: G        W       4.6.0-rc6-next-20160502-00010-g7730dc64d2c1-dirty #179
[ 2395.177857] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[ 2395.177906] task: ed275500 ti: e6c8c000 task.ti: e6c8c000
[ 2395.177996] PC is at s5p_mfc_reset+0x1c4/0x284 [s5p_mfc]
[ 2395.178057] LR is at s5p_mfc_reset+0x1a4/0x284 [s5p_mfc]

This patch fixes this issue by adding the CLK_ACLK333 as an Async-Bridge
clock for the MFC power domain, so the PD configuration works properly.

Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>

---

 arch/arm/boot/dts/exynos5420.dtsi | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi
index 4c8523471c65..f3e9d873633e 100644
--- a/arch/arm/boot/dts/exynos5420.dtsi
+++ b/arch/arm/boot/dts/exynos5420.dtsi
@@ -313,8 +313,9 @@
 	mfc_pd: power-domain@10044060 {
 		compatible = "samsung,exynos4210-pd";
 		reg = <0x10044060 0x20>;
-		clocks = <&clock CLK_FIN_PLL>, <&clock CLK_MOUT_USER_ACLK333>;
-		clock-names = "oscclk", "clk0";
+		clocks = <&clock CLK_FIN_PLL>, <&clock CLK_MOUT_USER_ACLK333>,
+			 <&clock CLK_ACLK333>;
+		clock-names = "oscclk", "clk0","asb0";
 		#power-domain-cells = <0>;
 	};
 
-- 
2.5.5

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

* Re: [PATCH 0/2] ARM: dts: Fix imprecise external abort error when accessing Exynos MFC
  2016-05-24 17:41 [PATCH 0/2] ARM: dts: Fix imprecise external abort error when accessing Exynos MFC Javier Martinez Canillas
  2016-05-24 17:41 ` [PATCH 1/2] clk: exynos5420: Set ID for aclk333 gate clock Javier Martinez Canillas
  2016-05-24 17:41 ` [PATCH 2/2] ARM: dts: Add async-bridge clock to MFC power domain for Exynos5420 Javier Martinez Canillas
@ 2016-05-25  6:51 ` Marek Szyprowski
  2016-05-25 14:17   ` Javier Martinez Canillas
  2 siblings, 1 reply; 11+ messages in thread
From: Marek Szyprowski @ 2016-05-25  6:51 UTC (permalink / raw)
  To: Javier Martinez Canillas, linux-kernel
  Cc: devicetree, Kukjin Kim, Michael Turquette, Krzysztof Kozlowski,
	Mauro Carvalho Chehab, Shuah Khan, Stephen Boyd,
	linux-samsung-soc, linux-arm-kernel, Sylwester Nawrocki,
	Tomasz Figa, linux-clk, Nicolas Dufresne, Andrzej Hajda

Hello,


On 2016-05-24 19:41, Javier Martinez Canillas wrote:
> This series fixes an imprecise external abort error when accessing the
> Exynos MFC registers due the power domain configuration requiring the
> aclk333 clock to be enabled during a domain switch.
>
> There isn't a dependency between the clock and Linux Samsung SoC trees
> because the CLK_ACLK333 clock ID is already defined so the patches can
> be picked indepedently by the relevant subsystem maintainers.

Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>

I'm really curious what kind of async-bridge is there, but this patch
really fixes the problem with mfc power domain.

> Best regards,
> Javier
>
>
> Javier Martinez Canillas (2):
>    clk: exynos5420: Set ID for aclk333 gate clock
>    ARM: dts: Add async-bridge clock to MFC power domain for Exynos5420
>
>   arch/arm/boot/dts/exynos5420.dtsi    | 5 +++--
>   drivers/clk/samsung/clk-exynos5420.c | 2 +-
>   2 files changed, 4 insertions(+), 3 deletions(-)
>

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

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

* Re: [PATCH 1/2] clk: exynos5420: Set ID for aclk333 gate clock
  2016-05-24 17:41 ` [PATCH 1/2] clk: exynos5420: Set ID for aclk333 gate clock Javier Martinez Canillas
@ 2016-05-25  7:11   ` Krzysztof Kozlowski
  2016-05-30 12:49   ` Sylwester Nawrocki
  1 sibling, 0 replies; 11+ messages in thread
From: Krzysztof Kozlowski @ 2016-05-25  7:11 UTC (permalink / raw)
  To: Javier Martinez Canillas, linux-kernel
  Cc: devicetree, Kukjin Kim, Michael Turquette, Marek Szyprowski,
	Mauro Carvalho Chehab, Shuah Khan, Stephen Boyd,
	linux-samsung-soc, linux-arm-kernel, Sylwester Nawrocki,
	Tomasz Figa, linux-clk, Nicolas Dufresne

On 05/24/2016 07:41 PM, Javier Martinez Canillas wrote:
> The aclk333 clock needs to be ungated during the MFC power domain switch,
> so set the clock ID to allow the Exynos power domain logic to lookup this
> clock if is defined in the MFC PD device tree node.
> 
> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
> ---
> 
>  drivers/clk/samsung/clk-exynos5420.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>

Best regards,
Krzysztof

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

* Re: [PATCH 2/2] ARM: dts: Add async-bridge clock to MFC power domain for Exynos5420
  2016-05-24 17:41 ` [PATCH 2/2] ARM: dts: Add async-bridge clock to MFC power domain for Exynos5420 Javier Martinez Canillas
@ 2016-05-25  7:48   ` Krzysztof Kozlowski
  2016-05-25  8:01     ` Krzysztof Kozlowski
  2016-05-25 14:22     ` Javier Martinez Canillas
  2016-05-30  7:41   ` Krzysztof Kozlowski
  1 sibling, 2 replies; 11+ messages in thread
From: Krzysztof Kozlowski @ 2016-05-25  7:48 UTC (permalink / raw)
  To: Javier Martinez Canillas, linux-kernel
  Cc: devicetree, Kukjin Kim, Michael Turquette, Marek Szyprowski,
	Mauro Carvalho Chehab, Shuah Khan, Stephen Boyd,
	linux-samsung-soc, linux-arm-kernel, Sylwester Nawrocki,
	Tomasz Figa, linux-clk, Nicolas Dufresne

On 05/24/2016 07:41 PM, Javier Martinez Canillas wrote:
> The MFC IP is also inter-connected by an Async-Bridge so the CLK_ACLK333
> has to be ungated during a power domain switch. Trying to do it when the
> clock is gated will fail and lead to an imprecise external abort error
> when the driver tries to access the MFC registers with the PD disabled.
> 
> For example, if the s5p-mfc module is removed and the MFC PD turned off:
> 
> [  186.835606] Power domain power-domain@10044060 disable failed
> [  186.835671] s5p-mfc 11000000.codec: Removing 11000000.codec
> [  186.837670] Power domain power-domain@10044060 disable failed
> 
> And when the module is inserted again:
> 
> [ 2395.176956] s5p_mfc_wait_for_done_dev:34: Interrupt (dev->int_type:0, command:12) timed out
> [ 2395.177031] s5p_mfc_init_hw:272: Failed to load firmware
> [ 2395.177384] Unhandled fault: imprecise external abort (0x1406) at 0x00000000
> [ 2395.177441] pgd = ec3b4000
> [ 2395.177467] [00000000] *pgd=00000000
> [ 2395.177507] Internal error: : 1406 [#1] PREEMPT SMP ARM
> [ 2395.177550] Modules linked in: s5p_mfc mwifiex_sdio mwifiex uvcvideo s5p_jpeg v4l2_mem2mem videobuf2_vmalloc videobuf2_dma_contig videobuf2_memops videobuf2_v4l2 videobuf2_core v4l2_common videodev media [last unloaded: s5p_mfc]
> [ 2395.177774] CPU: 1 PID: 2382 Comm: v4l_id Tainted: G        W       4.6.0-rc6-next-20160502-00010-g7730dc64d2c1-dirty #179
> [ 2395.177857] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
> [ 2395.177906] task: ed275500 ti: e6c8c000 task.ti: e6c8c000
> [ 2395.177996] PC is at s5p_mfc_reset+0x1c4/0x284 [s5p_mfc]
> [ 2395.178057] LR is at s5p_mfc_reset+0x1a4/0x284 [s5p_mfc]
> 
> This patch fixes this issue by adding the CLK_ACLK333 as an Async-Bridge
> clock for the MFC power domain, so the PD configuration works properly.
> 
> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
> 
> ---
> 
>  arch/arm/boot/dts/exynos5420.dtsi | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)

Indeed patch #1 is not a hard dependency here because there are no other
asb clocks. It is entirely obvious but works fine.

Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>

Unless all other patches are meant to current fixes cycle (and/or
cc-stable), I do not plan to apply it now. I'll take it for v4.8, because:
1. Your previous patches are needed. Without them bind/unbind won't work.
2. This is not reproducible in a regular driver operation.
3. It needs clock change to actually be useful.

Is it okay?

Best regards,
Krzysztof

> 
> diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi
> index 4c8523471c65..f3e9d873633e 100644
> --- a/arch/arm/boot/dts/exynos5420.dtsi
> +++ b/arch/arm/boot/dts/exynos5420.dtsi
> @@ -313,8 +313,9 @@
>  	mfc_pd: power-domain@10044060 {
>  		compatible = "samsung,exynos4210-pd";
>  		reg = <0x10044060 0x20>;
> -		clocks = <&clock CLK_FIN_PLL>, <&clock CLK_MOUT_USER_ACLK333>;
> -		clock-names = "oscclk", "clk0";
> +		clocks = <&clock CLK_FIN_PLL>, <&clock CLK_MOUT_USER_ACLK333>,
> +			 <&clock CLK_ACLK333>;
> +		clock-names = "oscclk", "clk0","asb0";
>  		#power-domain-cells = <0>;
>  	};
>  
> 

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

* Re: [PATCH 2/2] ARM: dts: Add async-bridge clock to MFC power domain for Exynos5420
  2016-05-25  7:48   ` Krzysztof Kozlowski
@ 2016-05-25  8:01     ` Krzysztof Kozlowski
  2016-05-25 14:22     ` Javier Martinez Canillas
  1 sibling, 0 replies; 11+ messages in thread
From: Krzysztof Kozlowski @ 2016-05-25  8:01 UTC (permalink / raw)
  To: Javier Martinez Canillas, linux-kernel
  Cc: devicetree, Kukjin Kim, Michael Turquette, Marek Szyprowski,
	Mauro Carvalho Chehab, Shuah Khan, Stephen Boyd,
	linux-samsung-soc, linux-arm-kernel, Sylwester Nawrocki,
	Tomasz Figa, linux-clk, Nicolas Dufresne

On 05/25/2016 09:48 AM, Krzysztof Kozlowski wrote:
> On 05/24/2016 07:41 PM, Javier Martinez Canillas wrote:
>> The MFC IP is also inter-connected by an Async-Bridge so the CLK_ACLK333
>> has to be ungated during a power domain switch. Trying to do it when the
>> clock is gated will fail and lead to an imprecise external abort error
>> when the driver tries to access the MFC registers with the PD disabled.
>>
>> For example, if the s5p-mfc module is removed and the MFC PD turned off:
>>
>> [  186.835606] Power domain power-domain@10044060 disable failed
>> [  186.835671] s5p-mfc 11000000.codec: Removing 11000000.codec
>> [  186.837670] Power domain power-domain@10044060 disable failed
>>
>> And when the module is inserted again:
>>
>> [ 2395.176956] s5p_mfc_wait_for_done_dev:34: Interrupt (dev->int_type:0, command:12) timed out
>> [ 2395.177031] s5p_mfc_init_hw:272: Failed to load firmware
>> [ 2395.177384] Unhandled fault: imprecise external abort (0x1406) at 0x00000000
>> [ 2395.177441] pgd = ec3b4000
>> [ 2395.177467] [00000000] *pgd=00000000
>> [ 2395.177507] Internal error: : 1406 [#1] PREEMPT SMP ARM
>> [ 2395.177550] Modules linked in: s5p_mfc mwifiex_sdio mwifiex uvcvideo s5p_jpeg v4l2_mem2mem videobuf2_vmalloc videobuf2_dma_contig videobuf2_memops videobuf2_v4l2 videobuf2_core v4l2_common videodev media [last unloaded: s5p_mfc]
>> [ 2395.177774] CPU: 1 PID: 2382 Comm: v4l_id Tainted: G        W       4.6.0-rc6-next-20160502-00010-g7730dc64d2c1-dirty #179
>> [ 2395.177857] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
>> [ 2395.177906] task: ed275500 ti: e6c8c000 task.ti: e6c8c000
>> [ 2395.177996] PC is at s5p_mfc_reset+0x1c4/0x284 [s5p_mfc]
>> [ 2395.178057] LR is at s5p_mfc_reset+0x1a4/0x284 [s5p_mfc]
>>
>> This patch fixes this issue by adding the CLK_ACLK333 as an Async-Bridge
>> clock for the MFC power domain, so the PD configuration works properly.
>>
>> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
>>
>> ---
>>
>>  arch/arm/boot/dts/exynos5420.dtsi | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> Indeed patch #1 is not a hard dependency here because there are no other
> asb clocks. It is entirely obvious but works fine.

Damn, I wanted to write:
"It is not entirely obvious but works fine."
(in Exynos pm_domains driver the clk_get() returns -ENOENT and the loop
is escaped early)

BR,
Krzysztof

> 
> Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> 
> Unless all other patches are meant to current fixes cycle (and/or
> cc-stable), I do not plan to apply it now. I'll take it for v4.8, because:
> 1. Your previous patches are needed. Without them bind/unbind won't work.
> 2. This is not reproducible in a regular driver operation.
> 3. It needs clock change to actually be useful.
> 
> Is it okay?
> 
> Best regards,
> Krzysztof
> 
>>
>> diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi
>> index 4c8523471c65..f3e9d873633e 100644
>> --- a/arch/arm/boot/dts/exynos5420.dtsi
>> +++ b/arch/arm/boot/dts/exynos5420.dtsi
>> @@ -313,8 +313,9 @@
>>  	mfc_pd: power-domain@10044060 {
>>  		compatible = "samsung,exynos4210-pd";
>>  		reg = <0x10044060 0x20>;
>> -		clocks = <&clock CLK_FIN_PLL>, <&clock CLK_MOUT_USER_ACLK333>;
>> -		clock-names = "oscclk", "clk0";
>> +		clocks = <&clock CLK_FIN_PLL>, <&clock CLK_MOUT_USER_ACLK333>,
>> +			 <&clock CLK_ACLK333>;
>> +		clock-names = "oscclk", "clk0","asb0";
>>  		#power-domain-cells = <0>;
>>  	};
>>  
>>
> 
> 

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

* Re: [PATCH 0/2] ARM: dts: Fix imprecise external abort error when accessing Exynos MFC
  2016-05-25  6:51 ` [PATCH 0/2] ARM: dts: Fix imprecise external abort error when accessing Exynos MFC Marek Szyprowski
@ 2016-05-25 14:17   ` Javier Martinez Canillas
  0 siblings, 0 replies; 11+ messages in thread
From: Javier Martinez Canillas @ 2016-05-25 14:17 UTC (permalink / raw)
  To: Marek Szyprowski, linux-kernel
  Cc: devicetree, Kukjin Kim, Michael Turquette, Krzysztof Kozlowski,
	Mauro Carvalho Chehab, Shuah Khan, Stephen Boyd,
	linux-samsung-soc, linux-arm-kernel, Sylwester Nawrocki,
	Tomasz Figa, linux-clk, Nicolas Dufresne, Andrzej Hajda

Hello Marek,

On 05/25/2016 02:51 AM, Marek Szyprowski wrote:
> Hello,
> 
> 
> On 2016-05-24 19:41, Javier Martinez Canillas wrote:
>> This series fixes an imprecise external abort error when accessing the
>> Exynos MFC registers due the power domain configuration requiring the
>> aclk333 clock to be enabled during a domain switch.
>>
>> There isn't a dependency between the clock and Linux Samsung SoC trees
>> because the CLK_ACLK333 clock ID is already defined so the patches can
>> be picked indepedently by the relevant subsystem maintainers.
> 
> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
> 
> I'm really curious what kind of async-bridge is there, but this patch
> really fixes the problem with mfc power domain.
>

Yes, the Exynos manual is not that clear about what clocks have to remain
enabled during a power domain switch for an IP block so maybe I got wrong.

I see that the MFC block has an async-bridge in "Figure 15-1" at section 
"15.2.1 NoC Probes in 5420 Bus". And I thought that since the IP block
has an async-bridge, the clock needs to remain ungated when the PMU turn
it on and off on a power domain switch.

I chose that clock because "Figure 7-10 MFC Clock Diagram" at section
"7.4.7 MFC Clock Diagram" shows that ACLK_333 is associated with the
MFC internal buses, or at least that's my understanding.

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

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

* Re: [PATCH 2/2] ARM: dts: Add async-bridge clock to MFC power domain for Exynos5420
  2016-05-25  7:48   ` Krzysztof Kozlowski
  2016-05-25  8:01     ` Krzysztof Kozlowski
@ 2016-05-25 14:22     ` Javier Martinez Canillas
  1 sibling, 0 replies; 11+ messages in thread
From: Javier Martinez Canillas @ 2016-05-25 14:22 UTC (permalink / raw)
  To: Krzysztof Kozlowski, linux-kernel
  Cc: devicetree, Kukjin Kim, Michael Turquette, Marek Szyprowski,
	Mauro Carvalho Chehab, Shuah Khan, Stephen Boyd,
	linux-samsung-soc, linux-arm-kernel, Sylwester Nawrocki,
	Tomasz Figa, linux-clk, Nicolas Dufresne

Hello Krzysztof,

On 05/25/2016 03:48 AM, Krzysztof Kozlowski wrote:
> On 05/24/2016 07:41 PM, Javier Martinez Canillas wrote:
>> The MFC IP is also inter-connected by an Async-Bridge so the CLK_ACLK333
>> has to be ungated during a power domain switch. Trying to do it when the
>> clock is gated will fail and lead to an imprecise external abort error
>> when the driver tries to access the MFC registers with the PD disabled.
>>
>> For example, if the s5p-mfc module is removed and the MFC PD turned off:
>>
>> [  186.835606] Power domain power-domain@10044060 disable failed
>> [  186.835671] s5p-mfc 11000000.codec: Removing 11000000.codec
>> [  186.837670] Power domain power-domain@10044060 disable failed
>>
>> And when the module is inserted again:
>>
>> [ 2395.176956] s5p_mfc_wait_for_done_dev:34: Interrupt (dev->int_type:0, command:12) timed out
>> [ 2395.177031] s5p_mfc_init_hw:272: Failed to load firmware
>> [ 2395.177384] Unhandled fault: imprecise external abort (0x1406) at 0x00000000
>> [ 2395.177441] pgd = ec3b4000
>> [ 2395.177467] [00000000] *pgd=00000000
>> [ 2395.177507] Internal error: : 1406 [#1] PREEMPT SMP ARM
>> [ 2395.177550] Modules linked in: s5p_mfc mwifiex_sdio mwifiex uvcvideo s5p_jpeg v4l2_mem2mem videobuf2_vmalloc videobuf2_dma_contig videobuf2_memops videobuf2_v4l2 videobuf2_core v4l2_common videodev media [last unloaded: s5p_mfc]
>> [ 2395.177774] CPU: 1 PID: 2382 Comm: v4l_id Tainted: G        W       4.6.0-rc6-next-20160502-00010-g7730dc64d2c1-dirty #179
>> [ 2395.177857] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
>> [ 2395.177906] task: ed275500 ti: e6c8c000 task.ti: e6c8c000
>> [ 2395.177996] PC is at s5p_mfc_reset+0x1c4/0x284 [s5p_mfc]
>> [ 2395.178057] LR is at s5p_mfc_reset+0x1a4/0x284 [s5p_mfc]
>>
>> This patch fixes this issue by adding the CLK_ACLK333 as an Async-Bridge
>> clock for the MFC power domain, so the PD configuration works properly.
>>
>> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
>>
>> ---
>>
>>  arch/arm/boot/dts/exynos5420.dtsi | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> Indeed patch #1 is not a hard dependency here because there are no other
> asb clocks. It is entirely obvious but works fine.
>

You are right, the reason why is not a hard dependency is because it fails
anyways so this patch (which will make it fail due the clock being missed)
won't have any effect.
 
> Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> 

Thanks!

> Unless all other patches are meant to current fixes cycle (and/or
> cc-stable), I do not plan to apply it now. I'll take it for v4.8, because:
> 1. Your previous patches are needed. Without them bind/unbind won't work.
> 2. This is not reproducible in a regular driver operation.
> 3. It needs clock change to actually be useful.
> 
> Is it okay?
>

Agreed, that's also why I didn't cc stable on this series.
 
> Best regards,
> Krzysztof
> 

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

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

* Re: [PATCH 2/2] ARM: dts: Add async-bridge clock to MFC power domain for Exynos5420
  2016-05-24 17:41 ` [PATCH 2/2] ARM: dts: Add async-bridge clock to MFC power domain for Exynos5420 Javier Martinez Canillas
  2016-05-25  7:48   ` Krzysztof Kozlowski
@ 2016-05-30  7:41   ` Krzysztof Kozlowski
  1 sibling, 0 replies; 11+ messages in thread
From: Krzysztof Kozlowski @ 2016-05-30  7:41 UTC (permalink / raw)
  To: Javier Martinez Canillas, linux-kernel
  Cc: devicetree, Kukjin Kim, Michael Turquette, Marek Szyprowski,
	Mauro Carvalho Chehab, Shuah Khan, Stephen Boyd,
	linux-samsung-soc, linux-arm-kernel, Sylwester Nawrocki,
	Tomasz Figa, linux-clk, Nicolas Dufresne

On 05/24/2016 07:41 PM, Javier Martinez Canillas wrote:
> The MFC IP is also inter-connected by an Async-Bridge so the CLK_ACLK333
> has to be ungated during a power domain switch. Trying to do it when the
> clock is gated will fail and lead to an imprecise external abort error
> when the driver tries to access the MFC registers with the PD disabled.
> 
> For example, if the s5p-mfc module is removed and the MFC PD turned off:
> 
> [  186.835606] Power domain power-domain@10044060 disable failed
> [  186.835671] s5p-mfc 11000000.codec: Removing 11000000.codec
> [  186.837670] Power domain power-domain@10044060 disable failed
> 
> And when the module is inserted again:
> 
> [ 2395.176956] s5p_mfc_wait_for_done_dev:34: Interrupt (dev->int_type:0, command:12) timed out
> [ 2395.177031] s5p_mfc_init_hw:272: Failed to load firmware
> [ 2395.177384] Unhandled fault: imprecise external abort (0x1406) at 0x00000000
> [ 2395.177441] pgd = ec3b4000
> [ 2395.177467] [00000000] *pgd=00000000
> [ 2395.177507] Internal error: : 1406 [#1] PREEMPT SMP ARM
> [ 2395.177550] Modules linked in: s5p_mfc mwifiex_sdio mwifiex uvcvideo s5p_jpeg v4l2_mem2mem videobuf2_vmalloc videobuf2_dma_contig videobuf2_memops videobuf2_v4l2 videobuf2_core v4l2_common videodev media [last unloaded: s5p_mfc]
> [ 2395.177774] CPU: 1 PID: 2382 Comm: v4l_id Tainted: G        W       4.6.0-rc6-next-20160502-00010-g7730dc64d2c1-dirty #179
> [ 2395.177857] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
> [ 2395.177906] task: ed275500 ti: e6c8c000 task.ti: e6c8c000
> [ 2395.177996] PC is at s5p_mfc_reset+0x1c4/0x284 [s5p_mfc]
> [ 2395.178057] LR is at s5p_mfc_reset+0x1a4/0x284 [s5p_mfc]
> 
> This patch fixes this issue by adding the CLK_ACLK333 as an Async-Bridge
> clock for the MFC power domain, so the PD configuration works properly.
> 
> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
> 
> ---
> 
>  arch/arm/boot/dts/exynos5420.dtsi | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)

Thanks, applied.

Krzysztof

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

* Re: [PATCH 1/2] clk: exynos5420: Set ID for aclk333 gate clock
  2016-05-24 17:41 ` [PATCH 1/2] clk: exynos5420: Set ID for aclk333 gate clock Javier Martinez Canillas
  2016-05-25  7:11   ` Krzysztof Kozlowski
@ 2016-05-30 12:49   ` Sylwester Nawrocki
  1 sibling, 0 replies; 11+ messages in thread
From: Sylwester Nawrocki @ 2016-05-30 12:49 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: linux-kernel, devicetree, Kukjin Kim, Michael Turquette,
	Krzysztof Kozlowski, Marek Szyprowski, Mauro Carvalho Chehab,
	Shuah Khan, Stephen Boyd, linux-samsung-soc, linux-arm-kernel,
	Tomasz Figa, linux-clk, Nicolas Dufresne

On 05/24/2016 07:41 PM, Javier Martinez Canillas wrote:
> The aclk333 clock needs to be ungated during the MFC power domain switch,
> so set the clock ID to allow the Exynos power domain logic to lookup this
> clock if is defined in the MFC PD device tree node.
> 
> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>

Thanks, patch applied.

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

end of thread, other threads:[~2016-05-30 12:49 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-24 17:41 [PATCH 0/2] ARM: dts: Fix imprecise external abort error when accessing Exynos MFC Javier Martinez Canillas
2016-05-24 17:41 ` [PATCH 1/2] clk: exynos5420: Set ID for aclk333 gate clock Javier Martinez Canillas
2016-05-25  7:11   ` Krzysztof Kozlowski
2016-05-30 12:49   ` Sylwester Nawrocki
2016-05-24 17:41 ` [PATCH 2/2] ARM: dts: Add async-bridge clock to MFC power domain for Exynos5420 Javier Martinez Canillas
2016-05-25  7:48   ` Krzysztof Kozlowski
2016-05-25  8:01     ` Krzysztof Kozlowski
2016-05-25 14:22     ` Javier Martinez Canillas
2016-05-30  7:41   ` Krzysztof Kozlowski
2016-05-25  6:51 ` [PATCH 0/2] ARM: dts: Fix imprecise external abort error when accessing Exynos MFC Marek Szyprowski
2016-05-25 14:17   ` Javier Martinez Canillas

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).