diff mbox series

interconnect: qcom: sm8450: Revert "interconnect: qcom: sm8450: Enable sync_state"

Message ID 20240115153420.1525037-1-krzysztof.kozlowski@linaro.org
State New
Headers
Series interconnect: qcom: sm8450: Revert "interconnect: qcom: sm8450: Enable sync_state" |

Commit Message

Krzysztof Kozlowski Jan. 15, 2024, 3:34 p.m. UTC 
  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(-)

Comments

Krzysztof Kozlowski Jan. 15, 2024, 3:55 p.m. UTC | #1 
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
Konrad Dybcio Jan. 15, 2024, 4:39 p.m. UTC | #2 
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/
Georgi Djakov Jan. 15, 2024, 4:44 p.m. UTC | #3 
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,
>   	},
>   };
>
Krzysztof Kozlowski Jan. 15, 2024, 5:52 p.m. UTC | #4 
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
Krzysztof Kozlowski Jan. 15, 2024, 5:59 p.m. UTC | #5 
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
Konrad Dybcio Jan. 15, 2024, 6:17 p.m. UTC | #6 
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
Krzysztof Kozlowski Jan. 15, 2024, 6:32 p.m. UTC | #7 
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
diff mbox series

Patch

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,
 	},
 };