| Message ID | 20201016101025.26505-1-saiprakash.ranjan@codeaurora.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | coresight: etm4x: Skip setting LPOVERRIDE bit for qcom, skip-power-up | expand |
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 >
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
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>
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
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 >
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;
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