2024-01-15 15:34:42

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH] interconnect: qcom: sm8450: Revert "interconnect: qcom: sm8450: Enable sync_state"

Revert commit 16862f1b2110 ("interconnect: qcom: sm8450: Enable
sync_state"), because it causes serial console to corrupt, later freeze
and become either entirely corrupted or only print without accepting any
input.

Cc: <[email protected]>
Fixes: 16862f1b2110 ("interconnect: qcom: sm8450: Enable sync_state")
Signed-off-by: Krzysztof Kozlowski <[email protected]>
---
drivers/interconnect/qcom/sm8450.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/drivers/interconnect/qcom/sm8450.c b/drivers/interconnect/qcom/sm8450.c
index b3cd0087377c..952017940b02 100644
--- a/drivers/interconnect/qcom/sm8450.c
+++ b/drivers/interconnect/qcom/sm8450.c
@@ -1888,7 +1888,6 @@ static struct platform_driver qnoc_driver = {
.driver = {
.name = "qnoc-sm8450",
.of_match_table = qnoc_of_match,
- .sync_state = icc_sync_state,
},
};

--
2.34.1



2024-01-15 15:38:22

by Konrad Dybcio

[permalink] [raw]
Subject: Re: [PATCH] interconnect: qcom: sm8450: Revert "interconnect: qcom: sm8450: Enable sync_state"

On 15.01.2024 16:34, Krzysztof Kozlowski wrote:
> Revert commit 16862f1b2110 ("interconnect: qcom: sm8450: Enable
> sync_state"), because it causes serial console to corrupt, later freeze
> and become either entirely corrupted or only print without accepting any
> input.
>
> Cc: <[email protected]>
> Fixes: 16862f1b2110 ("interconnect: qcom: sm8450: Enable sync_state")
> Signed-off-by: Krzysztof Kozlowski <[email protected]>
> ---

What's the board you're testing this on? And kernel base revision?

The symptoms you mentioned happened for me with this on some recent
-next:

https://lore.kernel.org/lkml/f24f32f1213b4b9e9ff2b4a36922f8d6e3abac51.1704278832.git.viresh.kumar@linaro.org/

Konrad

2024-01-15 15:57:13

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH] interconnect: qcom: sm8450: Revert "interconnect: qcom: sm8450: Enable sync_state"

On 15/01/2024 16:38, Konrad Dybcio wrote:
> On 15.01.2024 16:34, Krzysztof Kozlowski wrote:
>> Revert commit 16862f1b2110 ("interconnect: qcom: sm8450: Enable
>> sync_state"), because it causes serial console to corrupt, later freeze
>> and become either entirely corrupted or only print without accepting any
>> input.
>>
>> Cc: <[email protected]>
>> Fixes: 16862f1b2110 ("interconnect: qcom: sm8450: Enable sync_state")
>> Signed-off-by: Krzysztof Kozlowski <[email protected]>
>> ---
>
> What's the board you're testing this on? And kernel base revision?

HDK8450

>
> The symptoms you mentioned happened for me with this on some recent
> -next:

This was bisected, so all mainline kernels with this patch. Reverting
this patch helps (on top of that commit or on next).

>
> https://lore.kernel.org/lkml/f24f32f1213b4b9e9ff2b4a36922f8d6e3abac51.1704278832.git.viresh.kumar@linaro.org/

Best regards,
Krzysztof


2024-01-15 16:39:55

by Konrad Dybcio

[permalink] [raw]
Subject: Re: [PATCH] interconnect: qcom: sm8450: Revert "interconnect: qcom: sm8450: Enable sync_state"

On 15.01.2024 16:55, Krzysztof Kozlowski wrote:
> On 15/01/2024 16:38, Konrad Dybcio wrote:
>> On 15.01.2024 16:34, Krzysztof Kozlowski wrote:
>>> Revert commit 16862f1b2110 ("interconnect: qcom: sm8450: Enable
>>> sync_state"), because it causes serial console to corrupt, later freeze
>>> and become either entirely corrupted or only print without accepting any
>>> input.
>>>
>>> Cc: <[email protected]>
>>> Fixes: 16862f1b2110 ("interconnect: qcom: sm8450: Enable sync_state")
>>> Signed-off-by: Krzysztof Kozlowski <[email protected]>
>>> ---
>>
>> What's the board you're testing this on? And kernel base revision?
>
> HDK8450
>
>>
>> The symptoms you mentioned happened for me with this on some recent
>> -next:
>
> This was bisected, so all mainline kernels with this patch. Reverting
> this patch helps (on top of that commit or on next).

I don't quite get your answer. Was reverting \/ the solution for you?

Konrad
>
>>
>> https://lore.kernel.org/lkml/f24f32f1213b4b9e9ff2b4a36922f8d6e3abac51.1704278832.git.viresh.kumar@linaro.org/


2024-01-15 16:45:05

by Georgi Djakov

[permalink] [raw]
Subject: Re: [PATCH] interconnect: qcom: sm8450: Revert "interconnect: qcom: sm8450: Enable sync_state"

On 15.01.24 17:34, Krzysztof Kozlowski wrote:
> Revert commit 16862f1b2110 ("interconnect: qcom: sm8450: Enable
> sync_state"), because it causes serial console to corrupt, later freeze
> and become either entirely corrupted or only print without accepting any
> input.

Sounds like some driver is not requesting bandwidth and is relying on
bandwidth requests made by other drivers. Maybe we are missing some
"interconnects" property in DT?

Thanks,
Georgi

> Cc: <[email protected]>
> Fixes: 16862f1b2110 ("interconnect: qcom: sm8450: Enable sync_state")
> Signed-off-by: Krzysztof Kozlowski <[email protected]>
> ---
> drivers/interconnect/qcom/sm8450.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/interconnect/qcom/sm8450.c b/drivers/interconnect/qcom/sm8450.c
> index b3cd0087377c..952017940b02 100644
> --- a/drivers/interconnect/qcom/sm8450.c
> +++ b/drivers/interconnect/qcom/sm8450.c
> @@ -1888,7 +1888,6 @@ static struct platform_driver qnoc_driver = {
> .driver = {
> .name = "qnoc-sm8450",
> .of_match_table = qnoc_of_match,
> - .sync_state = icc_sync_state,
> },
> };
>


2024-01-15 17:52:45

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH] interconnect: qcom: sm8450: Revert "interconnect: qcom: sm8450: Enable sync_state"

On 15/01/2024 17:39, Konrad Dybcio wrote:
> On 15.01.2024 16:55, Krzysztof Kozlowski wrote:
>> On 15/01/2024 16:38, Konrad Dybcio wrote:
>>> On 15.01.2024 16:34, Krzysztof Kozlowski wrote:
>>>> Revert commit 16862f1b2110 ("interconnect: qcom: sm8450: Enable
>>>> sync_state"), because it causes serial console to corrupt, later freeze
>>>> and become either entirely corrupted or only print without accepting any
>>>> input.
>>>>
>>>> Cc: <[email protected]>
>>>> Fixes: 16862f1b2110 ("interconnect: qcom: sm8450: Enable sync_state")
>>>> Signed-off-by: Krzysztof Kozlowski <[email protected]>
>>>> ---
>>>
>>> What's the board you're testing this on? And kernel base revision?
>>
>> HDK8450
>>
>>>
>>> The symptoms you mentioned happened for me with this on some recent
>>> -next:
>>
>> This was bisected, so all mainline kernels with this patch. Reverting
>> this patch helps (on top of that commit or on next).
>
> I don't quite get your answer. Was reverting \/ the solution for you?

Yes...

Best regards,
Krzysztof


2024-01-15 17:59:28

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH] interconnect: qcom: sm8450: Revert "interconnect: qcom: sm8450: Enable sync_state"

On 15/01/2024 17:44, Georgi Djakov wrote:
> On 15.01.24 17:34, Krzysztof Kozlowski wrote:
>> Revert commit 16862f1b2110 ("interconnect: qcom: sm8450: Enable
>> sync_state"), because it causes serial console to corrupt, later freeze
>> and become either entirely corrupted or only print without accepting any
>> input.
>
> Sounds like some driver is not requesting bandwidth and is relying on
> bandwidth requests made by other drivers. Maybe we are missing some
> "interconnects" property in DT?

Yes, the debug UART (console) misses the interconnects. They could be
added but it does not change the fact that console is broken since v6.6
and this was probably never tested on actual hardware :/

Best regards,
Krzysztof


2024-01-15 18:17:30

by Konrad Dybcio

[permalink] [raw]
Subject: Re: [PATCH] interconnect: qcom: sm8450: Revert "interconnect: qcom: sm8450: Enable sync_state"

On 15.01.2024 18:59, Krzysztof Kozlowski wrote:
> On 15/01/2024 17:44, Georgi Djakov wrote:
>> On 15.01.24 17:34, Krzysztof Kozlowski wrote:
>>> Revert commit 16862f1b2110 ("interconnect: qcom: sm8450: Enable
>>> sync_state"), because it causes serial console to corrupt, later freeze
>>> and become either entirely corrupted or only print without accepting any
>>> input.
>>
>> Sounds like some driver is not requesting bandwidth and is relying on
>> bandwidth requests made by other drivers. Maybe we are missing some
>> "interconnects" property in DT?
>
> Yes, the debug UART (console) misses the interconnects. They could be
> added but it does not change the fact that console is broken since v6.6
> and this was probably never tested on actual hardware :/

This patch? I definitely tested it out on a headless remote board..

Konrad

2024-01-15 18:32:56

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH] interconnect: qcom: sm8450: Revert "interconnect: qcom: sm8450: Enable sync_state"

On 15/01/2024 19:17, Konrad Dybcio wrote:
> On 15.01.2024 18:59, Krzysztof Kozlowski wrote:
>> On 15/01/2024 17:44, Georgi Djakov wrote:
>>> On 15.01.24 17:34, Krzysztof Kozlowski wrote:
>>>> Revert commit 16862f1b2110 ("interconnect: qcom: sm8450: Enable
>>>> sync_state"), because it causes serial console to corrupt, later freeze
>>>> and become either entirely corrupted or only print without accepting any
>>>> input.
>>>
>>> Sounds like some driver is not requesting bandwidth and is relying on
>>> bandwidth requests made by other drivers. Maybe we are missing some
>>> "interconnects" property in DT?
>>
>> Yes, the debug UART (console) misses the interconnects. They could be
>> added but it does not change the fact that console is broken since v6.6
>> and this was probably never tested on actual hardware :/
>
> This patch? I definitely tested it out on a headless remote board..

OK, then maybe something changed between your tests and when it was
applied? Anyway issue is reproducible 100%. Even older kernel (v6.5)
with your commit cherry-picked fails.

Best regards,
Krzysztof