2023-07-04 16:51:16

by David Wronek

[permalink] [raw]
Subject: [PATCH 4/7] clk: qcom: gcc-sc7180: Fix up gcc_sdcc2_apps_clk_src

From: map220v <[email protected]>

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 <[email protected]>
Signed-off-by: David Wronek <[email protected]>
---
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,
.ops = &clk_rcg2_floor_ops,
},
};
--
2.41.0



2023-07-05 05:24:21

by Taniya Das

[permalink] [raw]
Subject: Re: [PATCH 4/7] clk: qcom: gcc-sc7180: Fix up gcc_sdcc2_apps_clk_src



On 7/4/2023 10:01 PM, David Wronek wrote:
> From: map220v <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: David Wronek <[email protected]>
> ---
> 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.

2023-07-05 10:09:43

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 4/7] clk: qcom: gcc-sc7180: Fix up gcc_sdcc2_apps_clk_src

On 04/07/2023 18:31, David Wronek wrote:
> From: map220v <[email protected]>
>
> 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 <[email protected]>

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

No SoB.

> Signed-off-by: David Wronek <[email protected]>

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


2023-07-05 11:37:33

by Konrad Dybcio

[permalink] [raw]
Subject: Re: [PATCH 4/7] clk: qcom: gcc-sc7180: Fix up gcc_sdcc2_apps_clk_src

On 5.07.2023 06:27, Taniya Das wrote:
>
>
> On 7/4/2023 10:01 PM, David Wronek wrote:
>> From: map220v <[email protected]>
>>
>> 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 <[email protected]>
>> Signed-off-by: David Wronek <[email protected]>
>> ---
>>   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,
>>       },
>>   };
>

2023-07-05 13:54:30

by David Wronek

[permalink] [raw]
Subject: Re: [PATCH 4/7] clk: qcom: gcc-sc7180: Fix up gcc_sdcc2_apps_clk_src

On Wed, Jul 5, 2023 at 6:27 AM Taniya Das <[email protected]> wrote:
>
>
>
> On 7/4/2023 10:01 PM, David Wronek wrote:
> > From: map220v <[email protected]>
> >
> > 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 <[email protected]>
> > Signed-off-by: David Wronek <[email protected]>
> > ---
> > 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