All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] clk: sunxi-ng: a31: Fix CLK_OUT_* clock ops
@ 2018-02-17 13:05 ` Chen-Yu Tsai
  0 siblings, 0 replies; 6+ messages in thread
From: Chen-Yu Tsai @ 2018-02-17 13:05 UTC (permalink / raw)
  To: Maxime Ripard, Michael Turquette, Stephen Boyd
  Cc: Chen-Yu Tsai, linux-clk, linux-arm-kernel, linux-kernel,
	Philipp Rossak, stable

When support for the A31/A31s CCU was first added, the clock ops for
the CLK_OUT_* clocks was set to the wrong type. The clocks are MP-type,
but the ops was set for div (M) clocks. This went unnoticed until now.
This was because while they are different clocks, their data structures
aligned in a way that ccu_div_ops would access the second ccu_div_internal
and ccu_mux_internal structures, which were valid, if not incorrect.

Furthermore, the use of these CLK_OUT_* was for feeding a precise 32.768
kHz clock signal to the WiFi chip. This was achievable by using the parent
with the same clock rate and no divider. So the incorrect divider setting
did not affect this usage.

Commit 946797aa3f08 ("clk: sunxi-ng: Support fixed post-dividers on MP
style clocks") added a new field to the ccu_mp structure, which broke
the aforementioned alignment. Now the system crashes as div_ops tries
to look up a nonexistent table.

Reported-by: Philipp Rossak <embed3d@gmail.com>
Fixes: c6e6c96d8fa6 ("clk: sunxi-ng: Add A31/A31s clocks")
Cc: <stable@vger.kernel.org>
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---

Philipp, can you give this a test and report if this fixes thing?
I don't have any A31/A31s boards online to test this.

---
 drivers/clk/sunxi-ng/ccu-sun6i-a31.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/sunxi-ng/ccu-sun6i-a31.c b/drivers/clk/sunxi-ng/ccu-sun6i-a31.c
index 72b16ed1012b..3b97f60540ad 100644
--- a/drivers/clk/sunxi-ng/ccu-sun6i-a31.c
+++ b/drivers/clk/sunxi-ng/ccu-sun6i-a31.c
@@ -762,7 +762,7 @@ static struct ccu_mp out_a_clk = {
 		.features	= CCU_FEATURE_FIXED_PREDIV,
 		.hw.init	= CLK_HW_INIT_PARENTS("out-a",
 						      clk_out_parents,
-						      &ccu_div_ops,
+						      &ccu_mp_ops,
 						      0),
 	},
 };
@@ -783,7 +783,7 @@ static struct ccu_mp out_b_clk = {
 		.features	= CCU_FEATURE_FIXED_PREDIV,
 		.hw.init	= CLK_HW_INIT_PARENTS("out-b",
 						      clk_out_parents,
-						      &ccu_div_ops,
+						      &ccu_mp_ops,
 						      0),
 	},
 };
@@ -804,7 +804,7 @@ static struct ccu_mp out_c_clk = {
 		.features	= CCU_FEATURE_FIXED_PREDIV,
 		.hw.init	= CLK_HW_INIT_PARENTS("out-c",
 						      clk_out_parents,
-						      &ccu_div_ops,
+						      &ccu_mp_ops,
 						      0),
 	},
 };
-- 
2.16.1

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

* [PATCH] clk: sunxi-ng: a31: Fix CLK_OUT_* clock ops
@ 2018-02-17 13:05 ` Chen-Yu Tsai
  0 siblings, 0 replies; 6+ messages in thread
From: Chen-Yu Tsai @ 2018-02-17 13:05 UTC (permalink / raw)
  To: linux-arm-kernel

When support for the A31/A31s CCU was first added, the clock ops for
the CLK_OUT_* clocks was set to the wrong type. The clocks are MP-type,
but the ops was set for div (M) clocks. This went unnoticed until now.
This was because while they are different clocks, their data structures
aligned in a way that ccu_div_ops would access the second ccu_div_internal
and ccu_mux_internal structures, which were valid, if not incorrect.

Furthermore, the use of these CLK_OUT_* was for feeding a precise 32.768
kHz clock signal to the WiFi chip. This was achievable by using the parent
with the same clock rate and no divider. So the incorrect divider setting
did not affect this usage.

Commit 946797aa3f08 ("clk: sunxi-ng: Support fixed post-dividers on MP
style clocks") added a new field to the ccu_mp structure, which broke
the aforementioned alignment. Now the system crashes as div_ops tries
to look up a nonexistent table.

Reported-by: Philipp Rossak <embed3d@gmail.com>
Fixes: c6e6c96d8fa6 ("clk: sunxi-ng: Add A31/A31s clocks")
Cc: <stable@vger.kernel.org>
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---

Philipp, can you give this a test and report if this fixes thing?
I don't have any A31/A31s boards online to test this.

---
 drivers/clk/sunxi-ng/ccu-sun6i-a31.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/sunxi-ng/ccu-sun6i-a31.c b/drivers/clk/sunxi-ng/ccu-sun6i-a31.c
index 72b16ed1012b..3b97f60540ad 100644
--- a/drivers/clk/sunxi-ng/ccu-sun6i-a31.c
+++ b/drivers/clk/sunxi-ng/ccu-sun6i-a31.c
@@ -762,7 +762,7 @@ static struct ccu_mp out_a_clk = {
 		.features	= CCU_FEATURE_FIXED_PREDIV,
 		.hw.init	= CLK_HW_INIT_PARENTS("out-a",
 						      clk_out_parents,
-						      &ccu_div_ops,
+						      &ccu_mp_ops,
 						      0),
 	},
 };
@@ -783,7 +783,7 @@ static struct ccu_mp out_b_clk = {
 		.features	= CCU_FEATURE_FIXED_PREDIV,
 		.hw.init	= CLK_HW_INIT_PARENTS("out-b",
 						      clk_out_parents,
-						      &ccu_div_ops,
+						      &ccu_mp_ops,
 						      0),
 	},
 };
@@ -804,7 +804,7 @@ static struct ccu_mp out_c_clk = {
 		.features	= CCU_FEATURE_FIXED_PREDIV,
 		.hw.init	= CLK_HW_INIT_PARENTS("out-c",
 						      clk_out_parents,
-						      &ccu_div_ops,
+						      &ccu_mp_ops,
 						      0),
 	},
 };
-- 
2.16.1

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

* Re: [PATCH] clk: sunxi-ng: a31: Fix CLK_OUT_* clock ops
  2018-02-17 13:05 ` Chen-Yu Tsai
@ 2018-02-17 13:54   ` Philipp Rossak
  -1 siblings, 0 replies; 6+ messages in thread
From: Philipp Rossak @ 2018-02-17 13:54 UTC (permalink / raw)
  To: Chen-Yu Tsai, Maxime Ripard, Michael Turquette, Stephen Boyd
  Cc: linux-clk, linux-arm-kernel, linux-kernel, stable



On 17.02.2018 14:05, Chen-Yu Tsai wrote:
> When support for the A31/A31s CCU was first added, the clock ops for
> the CLK_OUT_* clocks was set to the wrong type. The clocks are MP-type,
> but the ops was set for div (M) clocks. This went unnoticed until now.
> This was because while they are different clocks, their data structures
> aligned in a way that ccu_div_ops would access the second ccu_div_internal
> and ccu_mux_internal structures, which were valid, if not incorrect.
> 
> Furthermore, the use of these CLK_OUT_* was for feeding a precise 32.768
> kHz clock signal to the WiFi chip. This was achievable by using the parent
> with the same clock rate and no divider. So the incorrect divider setting
> did not affect this usage.
> 
> Commit 946797aa3f08 ("clk: sunxi-ng: Support fixed post-dividers on MP
> style clocks") added a new field to the ccu_mp structure, which broke
> the aforementioned alignment. Now the system crashes as div_ops tries
> to look up a nonexistent table.
> 
> Reported-by: Philipp Rossak <embed3d@gmail.com>
> Fixes: c6e6c96d8fa6 ("clk: sunxi-ng: Add A31/A31s clocks")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
Tested-by: Philipp Rossak <embed3d@gmail.com>
> ---
> 
> Philipp, can you give this a test and report if this fixes thing?
> I don't have any A31/A31s boards online to test this.
> 
Thanks!
That fixes that bug!

Philipp

> ---
>   drivers/clk/sunxi-ng/ccu-sun6i-a31.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/clk/sunxi-ng/ccu-sun6i-a31.c b/drivers/clk/sunxi-ng/ccu-sun6i-a31.c
> index 72b16ed1012b..3b97f60540ad 100644
> --- a/drivers/clk/sunxi-ng/ccu-sun6i-a31.c
> +++ b/drivers/clk/sunxi-ng/ccu-sun6i-a31.c
> @@ -762,7 +762,7 @@ static struct ccu_mp out_a_clk = {
>   		.features	= CCU_FEATURE_FIXED_PREDIV,
>   		.hw.init	= CLK_HW_INIT_PARENTS("out-a",
>   						      clk_out_parents,
> -						      &ccu_div_ops,
> +						      &ccu_mp_ops,
>   						      0),
>   	},
>   };
> @@ -783,7 +783,7 @@ static struct ccu_mp out_b_clk = {
>   		.features	= CCU_FEATURE_FIXED_PREDIV,
>   		.hw.init	= CLK_HW_INIT_PARENTS("out-b",
>   						      clk_out_parents,
> -						      &ccu_div_ops,
> +						      &ccu_mp_ops,
>   						      0),
>   	},
>   };
> @@ -804,7 +804,7 @@ static struct ccu_mp out_c_clk = {
>   		.features	= CCU_FEATURE_FIXED_PREDIV,
>   		.hw.init	= CLK_HW_INIT_PARENTS("out-c",
>   						      clk_out_parents,
> -						      &ccu_div_ops,
> +						      &ccu_mp_ops,
>   						      0),
>   	},
>   };
> 

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

* [PATCH] clk: sunxi-ng: a31: Fix CLK_OUT_* clock ops
@ 2018-02-17 13:54   ` Philipp Rossak
  0 siblings, 0 replies; 6+ messages in thread
From: Philipp Rossak @ 2018-02-17 13:54 UTC (permalink / raw)
  To: linux-arm-kernel



On 17.02.2018 14:05, Chen-Yu Tsai wrote:
> When support for the A31/A31s CCU was first added, the clock ops for
> the CLK_OUT_* clocks was set to the wrong type. The clocks are MP-type,
> but the ops was set for div (M) clocks. This went unnoticed until now.
> This was because while they are different clocks, their data structures
> aligned in a way that ccu_div_ops would access the second ccu_div_internal
> and ccu_mux_internal structures, which were valid, if not incorrect.
> 
> Furthermore, the use of these CLK_OUT_* was for feeding a precise 32.768
> kHz clock signal to the WiFi chip. This was achievable by using the parent
> with the same clock rate and no divider. So the incorrect divider setting
> did not affect this usage.
> 
> Commit 946797aa3f08 ("clk: sunxi-ng: Support fixed post-dividers on MP
> style clocks") added a new field to the ccu_mp structure, which broke
> the aforementioned alignment. Now the system crashes as div_ops tries
> to look up a nonexistent table.
> 
> Reported-by: Philipp Rossak <embed3d@gmail.com>
> Fixes: c6e6c96d8fa6 ("clk: sunxi-ng: Add A31/A31s clocks")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
Tested-by: Philipp Rossak <embed3d@gmail.com>
> ---
> 
> Philipp, can you give this a test and report if this fixes thing?
> I don't have any A31/A31s boards online to test this.
> 
Thanks!
That fixes that bug!

Philipp

> ---
>   drivers/clk/sunxi-ng/ccu-sun6i-a31.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/clk/sunxi-ng/ccu-sun6i-a31.c b/drivers/clk/sunxi-ng/ccu-sun6i-a31.c
> index 72b16ed1012b..3b97f60540ad 100644
> --- a/drivers/clk/sunxi-ng/ccu-sun6i-a31.c
> +++ b/drivers/clk/sunxi-ng/ccu-sun6i-a31.c
> @@ -762,7 +762,7 @@ static struct ccu_mp out_a_clk = {
>   		.features	= CCU_FEATURE_FIXED_PREDIV,
>   		.hw.init	= CLK_HW_INIT_PARENTS("out-a",
>   						      clk_out_parents,
> -						      &ccu_div_ops,
> +						      &ccu_mp_ops,
>   						      0),
>   	},
>   };
> @@ -783,7 +783,7 @@ static struct ccu_mp out_b_clk = {
>   		.features	= CCU_FEATURE_FIXED_PREDIV,
>   		.hw.init	= CLK_HW_INIT_PARENTS("out-b",
>   						      clk_out_parents,
> -						      &ccu_div_ops,
> +						      &ccu_mp_ops,
>   						      0),
>   	},
>   };
> @@ -804,7 +804,7 @@ static struct ccu_mp out_c_clk = {
>   		.features	= CCU_FEATURE_FIXED_PREDIV,
>   		.hw.init	= CLK_HW_INIT_PARENTS("out-c",
>   						      clk_out_parents,
> -						      &ccu_div_ops,
> +						      &ccu_mp_ops,
>   						      0),
>   	},
>   };
> 

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

* Re: [PATCH] clk: sunxi-ng: a31: Fix CLK_OUT_* clock ops
  2018-02-17 13:54   ` Philipp Rossak
@ 2018-02-20  3:17     ` Chen-Yu Tsai
  -1 siblings, 0 replies; 6+ messages in thread
From: Chen-Yu Tsai @ 2018-02-20  3:17 UTC (permalink / raw)
  To: Philipp Rossak
  Cc: Maxime Ripard, Michael Turquette, Stephen Boyd, linux-clk,
	linux-arm-kernel, linux-kernel, stable

On Sat, Feb 17, 2018 at 9:54 PM, Philipp Rossak <embed3d@gmail.com> wrote:
>
>
> On 17.02.2018 14:05, Chen-Yu Tsai wrote:
>>
>> When support for the A31/A31s CCU was first added, the clock ops for
>> the CLK_OUT_* clocks was set to the wrong type. The clocks are MP-type,
>> but the ops was set for div (M) clocks. This went unnoticed until now.
>> This was because while they are different clocks, their data structures
>> aligned in a way that ccu_div_ops would access the second ccu_div_internal
>> and ccu_mux_internal structures, which were valid, if not incorrect.
>>
>> Furthermore, the use of these CLK_OUT_* was for feeding a precise 32.768
>> kHz clock signal to the WiFi chip. This was achievable by using the parent
>> with the same clock rate and no divider. So the incorrect divider setting
>> did not affect this usage.
>>
>> Commit 946797aa3f08 ("clk: sunxi-ng: Support fixed post-dividers on MP
>> style clocks") added a new field to the ccu_mp structure, which broke
>> the aforementioned alignment. Now the system crashes as div_ops tries
>> to look up a nonexistent table.
>>
>> Reported-by: Philipp Rossak <embed3d@gmail.com>
>> Fixes: c6e6c96d8fa6 ("clk: sunxi-ng: Add A31/A31s clocks")
>> Cc: <stable@vger.kernel.org>
>> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>
> Tested-by: Philipp Rossak <embed3d@gmail.com>
>>
>> ---
>>
>> Philipp, can you give this a test and report if this fixes thing?
>> I don't have any A31/A31s boards online to test this.
>>
> Thanks!
> That fixes that bug!

Thanks.

It looks like this has been applied by Maxime.

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

* [PATCH] clk: sunxi-ng: a31: Fix CLK_OUT_* clock ops
@ 2018-02-20  3:17     ` Chen-Yu Tsai
  0 siblings, 0 replies; 6+ messages in thread
From: Chen-Yu Tsai @ 2018-02-20  3:17 UTC (permalink / raw)
  To: linux-arm-kernel

On Sat, Feb 17, 2018 at 9:54 PM, Philipp Rossak <embed3d@gmail.com> wrote:
>
>
> On 17.02.2018 14:05, Chen-Yu Tsai wrote:
>>
>> When support for the A31/A31s CCU was first added, the clock ops for
>> the CLK_OUT_* clocks was set to the wrong type. The clocks are MP-type,
>> but the ops was set for div (M) clocks. This went unnoticed until now.
>> This was because while they are different clocks, their data structures
>> aligned in a way that ccu_div_ops would access the second ccu_div_internal
>> and ccu_mux_internal structures, which were valid, if not incorrect.
>>
>> Furthermore, the use of these CLK_OUT_* was for feeding a precise 32.768
>> kHz clock signal to the WiFi chip. This was achievable by using the parent
>> with the same clock rate and no divider. So the incorrect divider setting
>> did not affect this usage.
>>
>> Commit 946797aa3f08 ("clk: sunxi-ng: Support fixed post-dividers on MP
>> style clocks") added a new field to the ccu_mp structure, which broke
>> the aforementioned alignment. Now the system crashes as div_ops tries
>> to look up a nonexistent table.
>>
>> Reported-by: Philipp Rossak <embed3d@gmail.com>
>> Fixes: c6e6c96d8fa6 ("clk: sunxi-ng: Add A31/A31s clocks")
>> Cc: <stable@vger.kernel.org>
>> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>
> Tested-by: Philipp Rossak <embed3d@gmail.com>
>>
>> ---
>>
>> Philipp, can you give this a test and report if this fixes thing?
>> I don't have any A31/A31s boards online to test this.
>>
> Thanks!
> That fixes that bug!

Thanks.

It looks like this has been applied by Maxime.

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

end of thread, other threads:[~2018-02-20  3:17 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-17 13:05 [PATCH] clk: sunxi-ng: a31: Fix CLK_OUT_* clock ops Chen-Yu Tsai
2018-02-17 13:05 ` Chen-Yu Tsai
2018-02-17 13:54 ` Philipp Rossak
2018-02-17 13:54   ` Philipp Rossak
2018-02-20  3:17   ` Chen-Yu Tsai
2018-02-20  3:17     ` Chen-Yu Tsai

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.