diff mbox series

coresight: etm4x: Skip setting LPOVERRIDE bit for qcom,skip-power-up

Message ID 20201016101025.26505-1-saiprakash.ranjan@codeaurora.org
State Accepted
Commit ac0f82b1b4956e348a6b2de8104308144ffb6ef7
Headers show
Series coresight: etm4x: Skip setting LPOVERRIDE bit for qcom,skip-power-up | expand

Commit Message

Sai Prakash Ranjan Oct. 16, 2020, 10:10 a.m. UTC 
There is a bug on the systems supporting to skip power up
(qcom,skip-power-up) where setting LPOVERRIDE bit(low-power
state override behaviour) will result in CPU hangs/lockups
even on the implementations which supports it. So skip
setting the LPOVERRIDE bit for such platforms.

Fixes: 02510a5aa78d ("coresight: etm4x: Add support to skip trace unit power up")
Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
---
 drivers/hwtracing/coresight/coresight-etm4x-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


base-commit: 3477326277451000bc667dfcc4fd0774c039184c

Comments

Suzuki K Poulose Oct. 16, 2020, 11:21 a.m. UTC | #1 
Hi Sai,

On 10/16/20 11:10 AM, Sai Prakash Ranjan wrote:
> There is a bug on the systems supporting to skip power up

> (qcom,skip-power-up) where setting LPOVERRIDE bit(low-power

> state override behaviour) will result in CPU hangs/lockups

> even on the implementations which supports it. So skip

> setting the LPOVERRIDE bit for such platforms.

> 

> Fixes: 02510a5aa78d ("coresight: etm4x: Add support to skip trace unit power up")

> Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>


The fix is fine by me. Btw, is there a hardware Erratum assigned for
this ? It would be good to have the Erratum documented somewhere,
preferrably ( Documentation/arm64/silicon-errata.rst )

> ---

>   drivers/hwtracing/coresight/coresight-etm4x-core.c | 2 +-

>   1 file changed, 1 insertion(+), 1 deletion(-)

> 

> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c

> index abd706b216ac..6096d7abf80d 100644

> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c

> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c

> @@ -779,7 +779,7 @@ static void etm4_init_arch_data(void *info)

>   	 * LPOVERRIDE, bit[23] implementation supports

>   	 * low-power state override

>   	 */

> -	if (BMVAL(etmidr5, 23, 23))

> +	if (BMVAL(etmidr5, 23, 23) && (!drvdata->skip_power_up))

>   		drvdata->lpoverride = true;

>   	else

>   		drvdata->lpoverride = false;

> 

> base-commit: 3477326277451000bc667dfcc4fd0774c039184c

>
Sai Prakash Ranjan Oct. 16, 2020, 11:47 a.m. UTC | #2 
Hi Suzuki,

On 2020-10-16 16:51, Suzuki Poulose wrote:
> Hi Sai,

> 

> On 10/16/20 11:10 AM, Sai Prakash Ranjan wrote:

>> There is a bug on the systems supporting to skip power up

>> (qcom,skip-power-up) where setting LPOVERRIDE bit(low-power

>> state override behaviour) will result in CPU hangs/lockups

>> even on the implementations which supports it. So skip

>> setting the LPOVERRIDE bit for such platforms.

>> 

>> Fixes: 02510a5aa78d ("coresight: etm4x: Add support to skip trace unit 

>> power up")

>> Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>

> 

> The fix is fine by me. Btw, is there a hardware Erratum assigned for

> this ? It would be good to have the Erratum documented somewhere,

> preferrably ( Documentation/arm64/silicon-errata.rst )

> 


No, afaik we don't have any erratum assigned to this bug.
It was already present in downstream kernel and since we
support these targets with the previous HW bug
(qcom,skip-power-up) now in upstream, we would need this
fix in upstream kernel as well.

Thanks,
Sai

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a 
member
of Code Aurora Forum, hosted by The Linux Foundation
Suzuki K Poulose Oct. 16, 2020, 1:15 p.m. UTC | #3 
On 10/16/20 12:47 PM, Sai Prakash Ranjan wrote:
> Hi Suzuki,

> 

> On 2020-10-16 16:51, Suzuki Poulose wrote:

>> Hi Sai,

>>

>> On 10/16/20 11:10 AM, Sai Prakash Ranjan wrote:

>>> There is a bug on the systems supporting to skip power up

>>> (qcom,skip-power-up) where setting LPOVERRIDE bit(low-power

>>> state override behaviour) will result in CPU hangs/lockups

>>> even on the implementations which supports it. So skip

>>> setting the LPOVERRIDE bit for such platforms.

>>>

>>> Fixes: 02510a5aa78d ("coresight: etm4x: Add support to skip trace 

>>> unit power up")

>>> Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>

>>

>> The fix is fine by me. Btw, is there a hardware Erratum assigned for

>> this ? It would be good to have the Erratum documented somewhere,

>> preferrably ( Documentation/arm64/silicon-errata.rst )

>>

> 

> No, afaik we don't have any erratum assigned to this bug.


Ok. Please double check, if there are any.

> It was already present in downstream kernel and since we

> support these targets with the previous HW bug

> (qcom,skip-power-up) now in upstream, we would need this

> fix in upstream kernel as well.


I understand the need for the fix and we must fix it. I was
looking to document this in the central place for errata's
handled in the kernel. And I missed asking this question
when the original patch was posted. So, thought of asking
the question now anyways. Better late than never ;-)

Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Sai Prakash Ranjan Oct. 16, 2020, 1:40 p.m. UTC | #4 
Hi Suzuki,

On 2020-10-16 18:45, Suzuki Poulose wrote:
> On 10/16/20 12:47 PM, Sai Prakash Ranjan wrote:

>> Hi Suzuki,

>> 

>> On 2020-10-16 16:51, Suzuki Poulose wrote:

>>> Hi Sai,

>>> 

>>> On 10/16/20 11:10 AM, Sai Prakash Ranjan wrote:

>>>> There is a bug on the systems supporting to skip power up

>>>> (qcom,skip-power-up) where setting LPOVERRIDE bit(low-power

>>>> state override behaviour) will result in CPU hangs/lockups

>>>> even on the implementations which supports it. So skip

>>>> setting the LPOVERRIDE bit for such platforms.

>>>> 

>>>> Fixes: 02510a5aa78d ("coresight: etm4x: Add support to skip trace 

>>>> unit power up")

>>>> Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>

>>> 

>>> The fix is fine by me. Btw, is there a hardware Erratum assigned for

>>> this ? It would be good to have the Erratum documented somewhere,

>>> preferrably ( Documentation/arm64/silicon-errata.rst )

>>> 

>> 

>> No, afaik we don't have any erratum assigned to this bug.

> 

> Ok. Please double check, if there are any.

> 


Sure I will check again.

>> It was already present in downstream kernel and since we

>> support these targets with the previous HW bug

>> (qcom,skip-power-up) now in upstream, we would need this

>> fix in upstream kernel as well.

> 

> I understand the need for the fix and we must fix it. I was

> looking to document this in the central place for errata's

> handled in the kernel. And I missed asking this question

> when the original patch was posted. So, thought of asking

> the question now anyways. Better late than never ;-)

> 

> Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>


Thanks. One more thing, does the internal erratum
number (if it exists) is good enough to be documented in
the Documentation/arm64/silicon-errata.rst ? I ask this
because outside qualcomm, it won't mean much right.

Thanks,
Sai

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a 
member
of Code Aurora Forum, hosted by The Linux Foundation
Mathieu Poirier Oct. 16, 2020, 4:31 p.m. UTC | #5 
On Fri, Oct 16, 2020 at 03:40:25PM +0530, Sai Prakash Ranjan wrote:
> There is a bug on the systems supporting to skip power up

> (qcom,skip-power-up) where setting LPOVERRIDE bit(low-power

> state override behaviour) will result in CPU hangs/lockups

> even on the implementations which supports it. So skip

> setting the LPOVERRIDE bit for such platforms.

> 

> Fixes: 02510a5aa78d ("coresight: etm4x: Add support to skip trace unit power up")

> Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>

> ---

>  drivers/hwtracing/coresight/coresight-etm4x-core.c | 2 +-

>  1 file changed, 1 insertion(+), 1 deletion(-)

> 

> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c

> index abd706b216ac..6096d7abf80d 100644

> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c

> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c

> @@ -779,7 +779,7 @@ static void etm4_init_arch_data(void *info)

>  	 * LPOVERRIDE, bit[23] implementation supports

>  	 * low-power state override

>  	 */

> -	if (BMVAL(etmidr5, 23, 23))

> +	if (BMVAL(etmidr5, 23, 23) && (!drvdata->skip_power_up))

>  		drvdata->lpoverride = true;

>  	else

>  		drvdata->lpoverride = false;

>


I have applied your patch.

Thanks,
Mathieu
 
> base-commit: 3477326277451000bc667dfcc4fd0774c039184c

> -- 

> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member

> of Code Aurora Forum, hosted by The Linux Foundation

>
patchwork-bot+linux-arm-msm@kernel.org Dec. 29, 2020, 8:15 p.m. UTC | #6 
Hello:

This patch was applied to qcom/linux.git (refs/heads/for-next):

On Fri, 16 Oct 2020 15:40:25 +0530 you wrote:
> There is a bug on the systems supporting to skip power up

> (qcom,skip-power-up) where setting LPOVERRIDE bit(low-power

> state override behaviour) will result in CPU hangs/lockups

> even on the implementations which supports it. So skip

> setting the LPOVERRIDE bit for such platforms.

> 

> Fixes: 02510a5aa78d ("coresight: etm4x: Add support to skip trace unit power up")

> Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>

> 

> [...]


Here is the summary with links:
  - coresight: etm4x: Skip setting LPOVERRIDE bit for qcom,skip-power-up
    https://git.kernel.org/qcom/c/ac0f82b1b495

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
diff mbox series

Patch

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index abd706b216ac..6096d7abf80d 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -779,7 +779,7 @@  static void etm4_init_arch_data(void *info)
 	 * LPOVERRIDE, bit[23] implementation supports
 	 * low-power state override
 	 */
-	if (BMVAL(etmidr5, 23, 23))
+	if (BMVAL(etmidr5, 23, 23) && (!drvdata->skip_power_up))
 		drvdata->lpoverride = true;
 	else
 		drvdata->lpoverride = false;