diff mbox series

[4/7] clk: qcom: gcc-sc7180: Fix up gcc_sdcc2_apps_clk_src

Message ID 20230704163848.169853-6-davidwronek@gmail.com
State Superseded
Headers show
Series Add initial support for SM7125 and Xiaomi SM7125 platform | expand

Commit Message

David Wronek July 4, 2023, 4:31 p.m. UTC 
From: map220v <map220v300@gmail.com>

Add the PARENT_ENABLE flag to prevent the clock from getting stuck at
boot.

Fixes: 17269568f726 ("clk: qcom: Add Global Clock controller (GCC) driver for SC7180")
Signed-off-by: map220v <map220v300@gmail.com>
Signed-off-by: David Wronek <davidwronek@gmail.com>
---
 drivers/clk/qcom/gcc-sc7180.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Taniya Das July 5, 2023, 4:27 a.m. UTC | #1 
On 7/4/2023 10:01 PM, David Wronek wrote:
> From: map220v <map220v300@gmail.com>
> 
> Add the PARENT_ENABLE flag to prevent the clock from getting stuck at
> boot.
> 
> Fixes: 17269568f726 ("clk: qcom: Add Global Clock controller (GCC) driver for SC7180")
> Signed-off-by: map220v <map220v300@gmail.com>
> Signed-off-by: David Wronek <davidwronek@gmail.com>
> ---
>   drivers/clk/qcom/gcc-sc7180.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/clk/qcom/gcc-sc7180.c b/drivers/clk/qcom/gcc-sc7180.c
> index cef3c77564cf..49f36e1df4fa 100644
> --- a/drivers/clk/qcom/gcc-sc7180.c
> +++ b/drivers/clk/qcom/gcc-sc7180.c
> @@ -651,6 +651,7 @@ static struct clk_rcg2 gcc_sdcc2_apps_clk_src = {
>   		.name = "gcc_sdcc2_apps_clk_src",
>   		.parent_data = gcc_parent_data_5,
>   		.num_parents = ARRAY_SIZE(gcc_parent_data_5),
> +		.flags = CLK_OPS_PARENT_ENABLE,

Could you please share what Stuck warnings are you observing?

>   		.ops = &clk_rcg2_floor_ops,
>   	},
>   };
Krzysztof Kozlowski July 5, 2023, 9:48 a.m. UTC | #2 
On 04/07/2023 18:31, David Wronek wrote:
> From: map220v <map220v300@gmail.com>
> 
> Add the PARENT_ENABLE flag to prevent the clock from getting stuck at
> boot.

What does it mean that "clock getting stuck at boot"? How a clock can stuck?

> 
> Fixes: 17269568f726 ("clk: qcom: Add Global Clock controller (GCC) driver for SC7180")
> Signed-off-by: map220v <map220v300@gmail.com>

Same concerns as for other patches with this. Look:
https://github.com/map220v/sm7125-mainline/commit/e754e5725cb596049df2437d7c857e4d232e87fb

No SoB.

> Signed-off-by: David Wronek <davidwronek@gmail.com>

Fixes must be either first in series or better sent separately. There is
no reason for them to be in this patchset in the first place, because
they are not related.

Best regards,
Krzysztof
Konrad Dybcio July 5, 2023, 10:45 a.m. UTC | #3 
On 5.07.2023 06:27, Taniya Das wrote:
> 
> 
> On 7/4/2023 10:01 PM, David Wronek wrote:
>> From: map220v <map220v300@gmail.com>
>>
>> Add the PARENT_ENABLE flag to prevent the clock from getting stuck at
>> boot.
>>
>> Fixes: 17269568f726 ("clk: qcom: Add Global Clock controller (GCC) driver for SC7180")
>> Signed-off-by: map220v <map220v300@gmail.com>
>> Signed-off-by: David Wronek <davidwronek@gmail.com>
>> ---
>>   drivers/clk/qcom/gcc-sc7180.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/clk/qcom/gcc-sc7180.c b/drivers/clk/qcom/gcc-sc7180.c
>> index cef3c77564cf..49f36e1df4fa 100644
>> --- a/drivers/clk/qcom/gcc-sc7180.c
>> +++ b/drivers/clk/qcom/gcc-sc7180.c
>> @@ -651,6 +651,7 @@ static struct clk_rcg2 gcc_sdcc2_apps_clk_src = {
>>           .name = "gcc_sdcc2_apps_clk_src",
>>           .parent_data = gcc_parent_data_5,
>>           .num_parents = ARRAY_SIZE(gcc_parent_data_5),
>> +        .flags = CLK_OPS_PARENT_ENABLE,
> 
> Could you please share what Stuck warnings are you observing?
My educated guess would be that the max frequency uses GPLL7, which
is either not enabled by default, or is shut down by unused clk
cleanup down the pipe.

Konrad
> 
>>           .ops = &clk_rcg2_floor_ops,
>>       },
>>   };
>
David Wronek July 5, 2023, 1:41 p.m. UTC | #4 
On Wed, Jul 5, 2023 at 6:27 AM Taniya Das <quic_tdas@quicinc.com> wrote:
>
>
>
> On 7/4/2023 10:01 PM, David Wronek wrote:
> > From: map220v <map220v300@gmail.com>
> >
> > Add the PARENT_ENABLE flag to prevent the clock from getting stuck at
> > boot.
> >
> > Fixes: 17269568f726 ("clk: qcom: Add Global Clock controller (GCC) driver for SC7180")
> > Signed-off-by: map220v <map220v300@gmail.com>
> > Signed-off-by: David Wronek <davidwronek@gmail.com>
> > ---
> >   drivers/clk/qcom/gcc-sc7180.c | 1 +
> >   1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/clk/qcom/gcc-sc7180.c b/drivers/clk/qcom/gcc-sc7180.c
> > index cef3c77564cf..49f36e1df4fa 100644
> > --- a/drivers/clk/qcom/gcc-sc7180.c
> > +++ b/drivers/clk/qcom/gcc-sc7180.c
> > @@ -651,6 +651,7 @@ static struct clk_rcg2 gcc_sdcc2_apps_clk_src = {
> >               .name = "gcc_sdcc2_apps_clk_src",
> >               .parent_data = gcc_parent_data_5,
> >               .num_parents = ARRAY_SIZE(gcc_parent_data_5),
> > +             .flags = CLK_OPS_PARENT_ENABLE,
>
> Could you please share what Stuck warnings are you observing?
>
> >               .ops = &clk_rcg2_floor_ops,
> >       },
> >   };
>
> --
> Thanks & Regards,
> Taniya Das.

Without this patch I get the following warning:
[    0.316057] gcc_sdcc2_apps_clk_src: rcg didn't update its configuration.
[    0.316081] WARNING: CPU: 4 PID: 75 at
drivers/clk/qcom/clk-rcg2.c:133 update_config+0xcc/0xdc
[    0.316226] Modules linked in:
[    0.316265] CPU: 4 PID: 75 Comm: kworker/u16:2 Not tainted
6.4.0-next-20230704-sm7125 #51
[    0.316337] Hardware name: Xiaomi Redmi Note 9 Pro (Global) (DT)
[    0.316394] Workqueue: events_unbound async_run_entry_fn
[    0.316453] pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[    0.316517] pc : update_config+0xcc/0xdc
[    0.316560] lr : update_config+0xcc/0xdc
[    0.316602] sp : ffff80008088b8e0
[    0.316636] x29: ffff80008088b8e0 x28: 0000000000000000 x27: ffffa9c34c571d00
[    0.316708] x26: ffffa9c34c333db0 x25: ffff60d440b5ec10 x24: ffff80008088bb18
[    0.316778] x23: ffff60d441704900 x22: 0000000005f5e100 x21: ffffa9c34c5dc548
[    0.316849] x20: ffffa9c34cc739b8 x19: 0000000000000000 x18: fffffffffffe5f18
[    0.316918] x17: 0000000000000014 x16: 0000000000000020 x15: 0000000000000048
[    0.316988] x14: fffffffffffe5f60 x13: ffffa9c34cb5e928 x12: 00000000000003f9
[    0.317058] x11: 0000000000000153 x10: ffffa9c34cbb88f0 x9 : ffffa9c34cb5e928
[    0.317128] x8 : 00000000ffffefff x7 : ffffa9c34cbb6928 x6 : 0000000000000153
[    0.317197] x5 : 0000000000000000 x4 : 40000000fffff153 x3 : 0000000000000000
[    0.317266] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff60d440284880
[    0.317336] Call trace:
[    0.317365]  update_config+0xcc/0xdc
[    0.317407]  clk_rcg2_configure+0xb0/0xb8
[    0.317451]  clk_rcg2_set_floor_rate_and_parent+0x2c/0x44
[    0.317506]  clk_change_rate+0x7c/0x294
[    0.317551]  clk_core_set_rate_nolock+0x168/0x274
[    0.317601]  clk_set_rate+0x38/0x14c
[    0.317641]  _opp_config_clk_single+0x30/0xa8
[    0.317687]  _set_opp+0x258/0x468
[    0.317726]  dev_pm_opp_set_rate+0x180/0x268
[    0.317770]  sdhci_msm_probe+0x360/0xb4c
[    0.317815]  platform_probe+0x68/0xc4
[    0.317859]  really_probe+0x148/0x2b0
[    0.317900]  __driver_probe_device+0x78/0x12c
[    0.317947]  driver_probe_device+0x3c/0x15c
[    0.317994]  __device_attach_driver+0xb8/0x134
[    0.318040]  bus_for_each_drv+0x84/0xe0
[    0.318082]  __device_attach_async_helper+0xac/0xd0
[    0.318132]  async_run_entry_fn+0x34/0xe0
[    0.318177]  process_one_work+0x1c0/0x334
[    0.318221]  worker_thread+0x68/0x428
[    0.318260]  kthread+0x110/0x114
[    0.318300]  ret_from_fork+0x10/0x20

Sincerely,
David
diff mbox series

Patch

diff --git a/drivers/clk/qcom/gcc-sc7180.c b/drivers/clk/qcom/gcc-sc7180.c
index cef3c77564cf..49f36e1df4fa 100644
--- a/drivers/clk/qcom/gcc-sc7180.c
+++ b/drivers/clk/qcom/gcc-sc7180.c
@@ -651,6 +651,7 @@  static struct clk_rcg2 gcc_sdcc2_apps_clk_src = {
 		.name = "gcc_sdcc2_apps_clk_src",
 		.parent_data = gcc_parent_data_5,
 		.num_parents = ARRAY_SIZE(gcc_parent_data_5),
+		.flags = CLK_OPS_PARENT_ENABLE,
 		.ops = &clk_rcg2_floor_ops,
 	},
 };