Hello,
I found a couple of issues with todays' next (tag next-20150211) when
booting an Exynos5420 Peach Pit Chromebook. These were regressions
introduced when converting the clk API to return a per-user clk.
This series is composed of the following trivial fixes:
Javier Martinez Canillas (2):
clk: Don't dereference parent clock if is NULL
clk: composite: Set clk_core to composite rate and mux components
drivers/clk/clk-composite.c | 2 ++
drivers/clk/clk.c | 2 +-
2 files changed, 3 insertions(+), 1 deletion(-)
Best regards,
Javier
The clock passed as an argument to clk_mux_determine_rate_flags() can
not have a parent clock if is either a root clock or an orphan.
In those cases parent is NULL so parent->hw shouldn't be dereferenced.
Fixes: 035a61c314eb3 ("clk: Make clk API return per-user struct clk instances")
Signed-off-by: Javier Martinez Canillas <[email protected]>
---
drivers/clk/clk.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 7f53166af5e6..7bd8893c94d6 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -799,7 +799,7 @@ clk_mux_determine_rate_flags(struct clk_hw *hw, unsigned long rate,
/* if NO_REPARENT flag set, pass through to current parent */
if (core->flags & CLK_SET_RATE_NO_REPARENT) {
parent = core->parent;
- if (core->flags & CLK_SET_RATE_PARENT)
+ if (core->flags & CLK_SET_RATE_PARENT && parent)
best = __clk_determine_rate(parent->hw, rate,
min_rate, max_rate);
else if (parent)
--
2.1.3
Commit 035a61c314eb3 ("clk: Make clk API return per-user struct clk instances")
moved the clock state to struct clk_core to allow the clk API returning per
user clk instances so now the struct clk_hw .core field is used instead of .clk
for most operations.
But in case of composite clocks, the rate and mux components didn't have a .core
set which leads to a NULL pointer dereference in clk_mux_determine_rate_flags():
Unable to handle kernel NULL pointer dereference at virtual address 00000034
pgd = c0004000
[00000034] *pgd=00000000
Internal error: Oops: 5 [#1] PREEMPT SMP ARM
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.19.0-next-20150211-00002-g1fb7f0e1150d #423
Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
task: ee480000 ti: ee488000 task.ti: ee488000
PC is at clk_mux_determine_rate_flags+0x14/0x19c
LR is at __clk_mux_determine_rate+0x24/0x2c
pc : [<c03a355c>] lr : [<c03a3734>] psr: a0000113
sp : ee489ce8 ip : ee489d84 fp : ee489d84
r10: 0000005c r9 : 00000001 r8 : 016e3600
r7 : 00000000 r6 : 00000000 r5 : ee442200 r4 : ee440c98
r3 : ffffffff r2 : 00000000 r1 : 016e3600 r0 : ee440c98
Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment kernel
Control: 10c5387d Table: 4000406a DAC: 00000015
Process swapper/0 (pid: 1, stack limit = 0xee488210)
Stack: (0xee489ce8 to 0xee48a000)
9ce0: 00000000 ffffffff 60000113 ee440c98 ee442200 00000000
9d00: 016e3600 ffffffff 00000001 0000005c ee489d84 c03a3734 ee489d80 ee489d84
9d20: 00000000 c048b130 00000400 c03a5798 ee489d80 ee489d84 c0607f60 ffffffea
9d40: 00000001 00000001 ee489d5c c003f844 c06e3340 ee402680 ee440d0c ed935000
9d60: 016e3600 00000003 00000001 0000005c eded3700 c03a11a0 ee489d80 ee489d84
9d80: 016e3600 ee402680 c05b413a eddc9900 016e3600 c03a1228 00000000 ffffffff
9da0: ffffffff eddc9900 016e3600 c03a1c1c ffffffff 016e3600 ed8c6710 c03d6ce4
9dc0: eded3400 00000000 00000000 c03c797c 00000001 0000005c eded3700 eded3700
9de0: 000005e0 00000001 0000005c c03db8ac c06e7e54 c03c8f08 00000000 c06e7e64
9e00: c06b6e74 c06e7f64 000005e0 c06e7df8 c06e5100 00000000 c06e7e6c c06e7f54
9e20: 00000000 00000000 eebd9550 00000000 c06e7da0 c06e7e54 ee7b5010 c06e7da0
9e40: eddc9690 c06e7db4 c06b6e74 00000097 00000000 c03d4398 00000000 ee7b5010
9e60: eebd9550 c06e7da0 00000000 c03db824 ee7b5010 fffffffe c06e7db4 c0299c7c
9e80: ee7b5010 c072a05c 00000000 c0298858 ee7b5010 c06e7db4 ee7b5044 00000000
9ea0: eddc9580 c0298a04 c06e7db4 00000000 c0298978 c02971d4 ee405c78 ee732b40
9ec0: c06e7db4 eded3800 c06d6738 c0298044 c0608300 c06e7db4 00000000 c06e7db4
9ee0: 00000000 c06beb58 c06beb58 c0299024 00000000 c068dd00 00000000 c0008944
9f00: 00000038 c049013c ee462200 c0711920 ee480000 60000113 c06c2cb0 00000000
9f20: 00000000 c06c2cb0 60000113 00000000 ef7fcafc 00000000 c0640194 c00389ec
9f40: c05ec3a8 c063f824 00000006 00000006 c06c2c50 c0696444 00000006 c0696424
9f60: c06ee1c0 c066b588 c06b6e74 00000097 00000000 c066bd44 00000006 00000006
9f80: c066b588 c003d684 00000000 c0481938 00000000 00000000 00000000 00000000
9fa0: 00000000 c0481940 00000000 c000e680 00000000 00000000 00000000 00000000
9fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
9fe0: 00000000 00000000 00000000 00000000 00000013 00000000 00000000 00000000
[<c03a355c>] (clk_mux_determine_rate_flags) from [<c03a3734>] (__clk_mux_determine_rate+0x24/0x2c)
[<c03a3734>] (__clk_mux_determine_rate) from [<c03a5798>] (clk_composite_determine_rate+0xbc/0x238)
[<c03a5798>] (clk_composite_determine_rate) from [<c03a11a0>] (clk_core_round_rate_nolock+0x5c/0x9c)
[<c03a11a0>] (clk_core_round_rate_nolock) from [<c03a1228>] (__clk_round_rate+0x38/0x40)
[<c03a1228>] (__clk_round_rate) from [<c03a1c1c>] (clk_round_rate+0x20/0x38)
[<c03a1c1c>] (clk_round_rate) from [<c03d6ce4>] (max98090_dai_set_sysclk+0x34/0x118)
[<c03d6ce4>] (max98090_dai_set_sysclk) from [<c03c797c>] (snd_soc_dai_set_sysclk+0x38/0x80)
[<c03c797c>] (snd_soc_dai_set_sysclk) from [<c03db8ac>] (snow_late_probe+0x24/0x48)
[<c03db8ac>] (snow_late_probe) from [<c03c8f08>] (snd_soc_register_card+0xf04/0x1070)
[<c03c8f08>] (snd_soc_register_card) from [<c03d4398>] (devm_snd_soc_register_card+0x30/0x64)
[<c03d4398>] (devm_snd_soc_register_card) from [<c03db824>] (snow_probe+0x68/0xcc)
[<c03db824>] (snow_probe) from [<c0299c7c>] (platform_drv_probe+0x48/0x98)
[<c0299c7c>] (platform_drv_probe) from [<c0298858>] (driver_probe_device+0x114/0x234)
[<c0298858>] (driver_probe_device) from [<c0298a04>] (__driver_attach+0x8c/0x90)
[<c0298a04>] (__driver_attach) from [<c02971d4>] (bus_for_each_dev+0x54/0x88)
[<c02971d4>] (bus_for_each_dev) from [<c0298044>] (bus_add_driver+0xd8/0x1cc)
[<c0298044>] (bus_add_driver) from [<c0299024>] (driver_register+0x78/0xf4)
[<c0299024>] (driver_register) from [<c0008944>] (do_one_initcall+0x80/0x1d0)
[<c0008944>] (do_one_initcall) from [<c066bd44>] (kernel_init_freeable+0x10c/0x1d8)
[<c066bd44>] (kernel_init_freeable) from [<c0481940>] (kernel_init+0x8/0xe4)
[<c0481940>] (kernel_init) from [<c000e680>] (ret_from_fork+0x14/0x34)
Code: e24dd00c e5907000 e1a08001 e88d000c (e5970034)
---[ end trace 825e9bbb0898d421 ]---
Fixes: 035a61c314eb3 ("clk: Make clk API return per-user struct clk instances")
Signed-off-by: Javier Martinez Canillas <[email protected]>
---
Hello,
I set the rate and mux components' .core in clk_composite_determine_rate()
because that is the least intrusive change and where the .clk field is set
too but I wonder if there is a reason to change the state of those clocks
in that function (which shouldn't have this side effect afaict) instead of
doing it in clk_register_composite().
Best regards,
Javier
---
drivers/clk/clk-composite.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/clk/clk-composite.c b/drivers/clk/clk-composite.c
index dee81b83c4b3..2a53b9580ff7 100644
--- a/drivers/clk/clk-composite.c
+++ b/drivers/clk/clk-composite.c
@@ -75,6 +75,7 @@ static long clk_composite_determine_rate(struct clk_hw *hw, unsigned long rate,
if (rate_hw && rate_ops && rate_ops->determine_rate) {
rate_hw->clk = hw->clk;
+ rate_hw->core = hw->core;
return rate_ops->determine_rate(rate_hw, rate, min_rate,
max_rate,
best_parent_rate,
@@ -121,6 +122,7 @@ static long clk_composite_determine_rate(struct clk_hw *hw, unsigned long rate,
return best_rate;
} else if (mux_hw && mux_ops && mux_ops->determine_rate) {
mux_hw->clk = hw->clk;
+ mux_hw->core = hw->core;
return mux_ops->determine_rate(mux_hw, rate, min_rate,
max_rate, best_parent_rate,
best_parent_p);
--
2.1.3
On 02/11, Javier Martinez Canillas wrote:
> Commit 035a61c314eb3 ("clk: Make clk API return per-user struct clk instances")
> moved the clock state to struct clk_core to allow the clk API returning per
> user clk instances so now the struct clk_hw .core field is used instead of .clk
> for most operations.
>
> But in case of composite clocks, the rate and mux components didn't have a .core
> set which leads to a NULL pointer dereference in clk_mux_determine_rate_flags():
>
> Unable to handle kernel NULL pointer dereference at virtual address 00000034
> pgd = c0004000
> [00000034] *pgd=00000000
> Internal error: Oops: 5 [#1] PREEMPT SMP ARM
> Modules linked in:
> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.19.0-next-20150211-00002-g1fb7f0e1150d #423
> Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
> task: ee480000 ti: ee488000 task.ti: ee488000
> PC is at clk_mux_determine_rate_flags+0x14/0x19c
> LR is at __clk_mux_determine_rate+0x24/0x2c
> pc : [<c03a355c>] lr : [<c03a3734>] psr: a0000113
> sp : ee489ce8 ip : ee489d84 fp : ee489d84
> r10: 0000005c r9 : 00000001 r8 : 016e3600
> r7 : 00000000 r6 : 00000000 r5 : ee442200 r4 : ee440c98
> r3 : ffffffff r2 : 00000000 r1 : 016e3600 r0 : ee440c98
> Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment kernel
> Control: 10c5387d Table: 4000406a DAC: 00000015
> Process swapper/0 (pid: 1, stack limit = 0xee488210)
> Stack: (0xee489ce8 to 0xee48a000)
> 9ce0: 00000000 ffffffff 60000113 ee440c98 ee442200 00000000
> 9d00: 016e3600 ffffffff 00000001 0000005c ee489d84 c03a3734 ee489d80 ee489d84
> 9d20: 00000000 c048b130 00000400 c03a5798 ee489d80 ee489d84 c0607f60 ffffffea
> 9d40: 00000001 00000001 ee489d5c c003f844 c06e3340 ee402680 ee440d0c ed935000
> 9d60: 016e3600 00000003 00000001 0000005c eded3700 c03a11a0 ee489d80 ee489d84
> 9d80: 016e3600 ee402680 c05b413a eddc9900 016e3600 c03a1228 00000000 ffffffff
> 9da0: ffffffff eddc9900 016e3600 c03a1c1c ffffffff 016e3600 ed8c6710 c03d6ce4
> 9dc0: eded3400 00000000 00000000 c03c797c 00000001 0000005c eded3700 eded3700
> 9de0: 000005e0 00000001 0000005c c03db8ac c06e7e54 c03c8f08 00000000 c06e7e64
> 9e00: c06b6e74 c06e7f64 000005e0 c06e7df8 c06e5100 00000000 c06e7e6c c06e7f54
> 9e20: 00000000 00000000 eebd9550 00000000 c06e7da0 c06e7e54 ee7b5010 c06e7da0
> 9e40: eddc9690 c06e7db4 c06b6e74 00000097 00000000 c03d4398 00000000 ee7b5010
> 9e60: eebd9550 c06e7da0 00000000 c03db824 ee7b5010 fffffffe c06e7db4 c0299c7c
> 9e80: ee7b5010 c072a05c 00000000 c0298858 ee7b5010 c06e7db4 ee7b5044 00000000
> 9ea0: eddc9580 c0298a04 c06e7db4 00000000 c0298978 c02971d4 ee405c78 ee732b40
> 9ec0: c06e7db4 eded3800 c06d6738 c0298044 c0608300 c06e7db4 00000000 c06e7db4
> 9ee0: 00000000 c06beb58 c06beb58 c0299024 00000000 c068dd00 00000000 c0008944
> 9f00: 00000038 c049013c ee462200 c0711920 ee480000 60000113 c06c2cb0 00000000
> 9f20: 00000000 c06c2cb0 60000113 00000000 ef7fcafc 00000000 c0640194 c00389ec
> 9f40: c05ec3a8 c063f824 00000006 00000006 c06c2c50 c0696444 00000006 c0696424
> 9f60: c06ee1c0 c066b588 c06b6e74 00000097 00000000 c066bd44 00000006 00000006
> 9f80: c066b588 c003d684 00000000 c0481938 00000000 00000000 00000000 00000000
> 9fa0: 00000000 c0481940 00000000 c000e680 00000000 00000000 00000000 00000000
> 9fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> 9fe0: 00000000 00000000 00000000 00000000 00000013 00000000 00000000 00000000
> [<c03a355c>] (clk_mux_determine_rate_flags) from [<c03a3734>] (__clk_mux_determine_rate+0x24/0x2c)
> [<c03a3734>] (__clk_mux_determine_rate) from [<c03a5798>] (clk_composite_determine_rate+0xbc/0x238)
> [<c03a5798>] (clk_composite_determine_rate) from [<c03a11a0>] (clk_core_round_rate_nolock+0x5c/0x9c)
> [<c03a11a0>] (clk_core_round_rate_nolock) from [<c03a1228>] (__clk_round_rate+0x38/0x40)
> [<c03a1228>] (__clk_round_rate) from [<c03a1c1c>] (clk_round_rate+0x20/0x38)
> [<c03a1c1c>] (clk_round_rate) from [<c03d6ce4>] (max98090_dai_set_sysclk+0x34/0x118)
> [<c03d6ce4>] (max98090_dai_set_sysclk) from [<c03c797c>] (snd_soc_dai_set_sysclk+0x38/0x80)
> [<c03c797c>] (snd_soc_dai_set_sysclk) from [<c03db8ac>] (snow_late_probe+0x24/0x48)
> [<c03db8ac>] (snow_late_probe) from [<c03c8f08>] (snd_soc_register_card+0xf04/0x1070)
> [<c03c8f08>] (snd_soc_register_card) from [<c03d4398>] (devm_snd_soc_register_card+0x30/0x64)
> [<c03d4398>] (devm_snd_soc_register_card) from [<c03db824>] (snow_probe+0x68/0xcc)
> [<c03db824>] (snow_probe) from [<c0299c7c>] (platform_drv_probe+0x48/0x98)
> [<c0299c7c>] (platform_drv_probe) from [<c0298858>] (driver_probe_device+0x114/0x234)
> [<c0298858>] (driver_probe_device) from [<c0298a04>] (__driver_attach+0x8c/0x90)
> [<c0298a04>] (__driver_attach) from [<c02971d4>] (bus_for_each_dev+0x54/0x88)
> [<c02971d4>] (bus_for_each_dev) from [<c0298044>] (bus_add_driver+0xd8/0x1cc)
> [<c0298044>] (bus_add_driver) from [<c0299024>] (driver_register+0x78/0xf4)
> [<c0299024>] (driver_register) from [<c0008944>] (do_one_initcall+0x80/0x1d0)
> [<c0008944>] (do_one_initcall) from [<c066bd44>] (kernel_init_freeable+0x10c/0x1d8)
> [<c066bd44>] (kernel_init_freeable) from [<c0481940>] (kernel_init+0x8/0xe4)
> [<c0481940>] (kernel_init) from [<c000e680>] (ret_from_fork+0x14/0x34)
> Code: e24dd00c e5907000 e1a08001 e88d000c (e5970034)
> ---[ end trace 825e9bbb0898d421 ]---
>
> Fixes: 035a61c314eb3 ("clk: Make clk API return per-user struct clk instances")
> Signed-off-by: Javier Martinez Canillas <[email protected]>
> ---
Thanks for the patch.
>
> Hello,
>
> I set the rate and mux components' .core in clk_composite_determine_rate()
> because that is the least intrusive change and where the .clk field is set
> too but I wonder if there is a reason to change the state of those clocks
> in that function (which shouldn't have this side effect afaict) instead of
> doing it in clk_register_composite().
The reason we have to do it in the ops instead of during the
registration phase is because some of these ops are called inside
the clk_register() function.
>
> drivers/clk/clk-composite.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/clk/clk-composite.c b/drivers/clk/clk-composite.c
> index dee81b83c4b3..2a53b9580ff7 100644
> --- a/drivers/clk/clk-composite.c
> +++ b/drivers/clk/clk-composite.c
> @@ -75,6 +75,7 @@ static long clk_composite_determine_rate(struct clk_hw *hw, unsigned long rate,
>
> if (rate_hw && rate_ops && rate_ops->determine_rate) {
> rate_hw->clk = hw->clk;
> + rate_hw->core = hw->core;
> return rate_ops->determine_rate(rate_hw, rate, min_rate,
> max_rate,
> best_parent_rate,
> @@ -121,6 +122,7 @@ static long clk_composite_determine_rate(struct clk_hw *hw, unsigned long rate,
> return best_rate;
> } else if (mux_hw && mux_ops && mux_ops->determine_rate) {
> mux_hw->clk = hw->clk;
> + mux_hw->core = hw->core;
> return mux_ops->determine_rate(mux_hw, rate, min_rate,
> max_rate, best_parent_rate,
> best_parent_p);
We need to assign the core pointer wherever we assign the clk
pointer. That seems to be quite a few more places than two.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
On 02/11, Javier Martinez Canillas wrote:
> The clock passed as an argument to clk_mux_determine_rate_flags() can
> not have a parent clock if is either a root clock or an orphan.
>
> In those cases parent is NULL so parent->hw shouldn't be dereferenced.
>
> Fixes: 035a61c314eb3 ("clk: Make clk API return per-user struct clk instances")
> Signed-off-by: Javier Martinez Canillas <[email protected]>
> ---
> drivers/clk/clk.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 7f53166af5e6..7bd8893c94d6 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -799,7 +799,7 @@ clk_mux_determine_rate_flags(struct clk_hw *hw, unsigned long rate,
> /* if NO_REPARENT flag set, pass through to current parent */
> if (core->flags & CLK_SET_RATE_NO_REPARENT) {
> parent = core->parent;
> - if (core->flags & CLK_SET_RATE_PARENT)
> + if (core->flags & CLK_SET_RATE_PARENT && parent)
> best = __clk_determine_rate(parent->hw, rate,
> min_rate, max_rate);
> else if (parent)
Sorry this doesn't look right. Before all the recent changes to
this file we would call __clk_round_rate() which would return 0
if the first argument was NULL. Now we're going to take the else
if path and do something different. So we need a parent ?
parent->hw : NULL here.
Of course, I wonder why a clock has the CLK_SET_RATE_PARENT flag
set if it doesn't actually have a parent. That also seems wrong.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
Hello Stephen,
On 02/11/2015 07:50 PM, Stephen Boyd wrote:
>> ---
>
> Thanks for the patch.
>
Thanks a lot for your feedback.
>>
>> Hello,
>>
>> I set the rate and mux components' .core in clk_composite_determine_rate()
>> because that is the least intrusive change and where the .clk field is set
>> too but I wonder if there is a reason to change the state of those clocks
>> in that function (which shouldn't have this side effect afaict) instead of
>> doing it in clk_register_composite().
>
> The reason we have to do it in the ops instead of during the
> registration phase is because some of these ops are called inside
> the clk_register() function.
>
Got it.
>>
>> drivers/clk/clk-composite.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/clk/clk-composite.c b/drivers/clk/clk-composite.c
>> index dee81b83c4b3..2a53b9580ff7 100644
>> --- a/drivers/clk/clk-composite.c
>> +++ b/drivers/clk/clk-composite.c
>> @@ -75,6 +75,7 @@ static long clk_composite_determine_rate(struct clk_hw *hw, unsigned long rate,
>>
>> if (rate_hw && rate_ops && rate_ops->determine_rate) {
>> rate_hw->clk = hw->clk;
>> + rate_hw->core = hw->core;
>> return rate_ops->determine_rate(rate_hw, rate, min_rate,
>> max_rate,
>> best_parent_rate,
>> @@ -121,6 +122,7 @@ static long clk_composite_determine_rate(struct clk_hw *hw, unsigned long rate,
>> return best_rate;
>> } else if (mux_hw && mux_ops && mux_ops->determine_rate) {
>> mux_hw->clk = hw->clk;
>> + mux_hw->core = hw->core;
>> return mux_ops->determine_rate(mux_hw, rate, min_rate,
>> max_rate, best_parent_rate,
>> best_parent_p);
>
> We need to assign the core pointer wherever we assign the clk
> pointer. That seems to be quite a few more places than two.
>
Yes, I found more places in other drivers so I wrote a small coccinelle
script to replace those using a semantic patch. Also I created a inline
function to wrap the assignments since we will have to change it again
once the clk_core is dropped.
Best regards,
Javier
Hello Stephen,
Thanks a lot for your feedback.
On 02/11/2015 07:54 PM, Stephen Boyd wrote:
> On 02/11, Javier Martinez Canillas wrote:
>> --- a/drivers/clk/clk.c
>> +++ b/drivers/clk/clk.c
>> @@ -799,7 +799,7 @@ clk_mux_determine_rate_flags(struct clk_hw *hw, unsigned long rate,
>> /* if NO_REPARENT flag set, pass through to current parent */
>> if (core->flags & CLK_SET_RATE_NO_REPARENT) {
>> parent = core->parent;
>> - if (core->flags & CLK_SET_RATE_PARENT)
>> + if (core->flags & CLK_SET_RATE_PARENT && parent)
>> best = __clk_determine_rate(parent->hw, rate,
>> min_rate, max_rate);
>> else if (parent)
>
> Sorry this doesn't look right. Before all the recent changes to
> this file we would call __clk_round_rate() which would return 0
> if the first argument was NULL. Now we're going to take the else
> if path and do something different. So we need a parent ?
> parent->hw : NULL here.
>
Right, I'm not that familiar with the common clock framework so I
didn't realize I was changing the behavior, sorry about that...
> Of course, I wonder why a clock has the CLK_SET_RATE_PARENT flag
> set if it doesn't actually have a parent. That also seems wrong.
>
Yes, I did not face this issue and only patch #2 was enough to
fix my problem but the theoretical NULL pointer dereference
was found when reading the code.
I agree that a clock with that flag set should have at least one
parent but afaict there is no sanity check on clock registration.
And even if that was the case, I believe that the core should be
robust enough to check for NULL before trying to dereference it.
I'll post a v2 passing NULL as an argument and parent->hw if
parent is not NULL as you suggested.
Best regards,
Javier