[PATCH] interconnect: qcom: sm8450: Revert "interconnect: qcom: sm8450: Enable sync_state"

[PATCH] interconnect: qcom: sm8450: Revert "interconnect: qcom: sm8450: Enable sync_state"

[Krzysztof Kozlowski]

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: <stable@vger.kernel.org>
Fixes: 16862f1b2110 ("interconnect: qcom: sm8450: Enable sync_state")
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 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

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

[Konrad Dybcio]

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: <stable@vger.kernel.org>
> Fixes: 16862f1b2110 ("interconnect: qcom: sm8450: Enable sync_state")
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---

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

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

[Krzysztof Kozlowski]

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: <stable@vger.kernel.org>
>> Fixes: 16862f1b2110 ("interconnect: qcom: sm8450: Enable sync_state")
>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> ---
> 
> 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

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

[Konrad Dybcio]

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: <stable@vger.kernel.org>
>>> Fixes: 16862f1b2110 ("interconnect: qcom: sm8450: Enable sync_state")
>>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>> ---
>>
>> 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/

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

[Krzysztof Kozlowski]

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: <stable@vger.kernel.org>
>>>> Fixes: 16862f1b2110 ("interconnect: qcom: sm8450: Enable sync_state")
>>>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>> ---
>>>
>>> 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

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

[Georgi Djakov]

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: <stable@vger.kernel.org>
> Fixes: 16862f1b2110 ("interconnect: qcom: sm8450: Enable sync_state")
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>   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,
>   	},
>   };
>   

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

[Krzysztof Kozlowski]

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

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

[Konrad Dybcio]

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

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

[Krzysztof Kozlowski]

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