| Message ID | 20230106152331.1374973-3-james.clark@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | coresight: cti: Add PM runtime call in enable_store | expand |
On 06/01/2023 15:23, James Clark wrote: > From: Mao Jinlong <quic_jinlmao@quicinc.com> > > In commit 6746eae4bbad ("coresight: cti: Fix hang in cti_disable_hw()") > PM runtime calls are removed from cti_enable_hw/cti_disable_hw. When > enabling CTI by writing enable sysfs node, clock for accessing CTI > register won't be enabled. Device will crash due to register access > issue. Add PM runtime call in enable_store to fix this issue. > > Fixes: 6746eae4bbad ("coresight: cti: Fix hang in cti_disable_hw()") > Signed-off-by: Mao Jinlong <quic_jinlmao@quicinc.com> > [Change to only call pm_runtime_put if a disable happened] > Signed-off-by: James Clark <james.clark@arm.com> > --- > drivers/hwtracing/coresight/coresight-cti-sysfs.c | 13 +++++++++++-- > 1 file changed, 11 insertions(+), 2 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-cti-sysfs.c b/drivers/hwtracing/coresight/coresight-cti-sysfs.c > index 6d59c815ecf5..71e7a8266bb3 100644 > --- a/drivers/hwtracing/coresight/coresight-cti-sysfs.c > +++ b/drivers/hwtracing/coresight/coresight-cti-sysfs.c > @@ -108,10 +108,19 @@ static ssize_t enable_store(struct device *dev, > if (ret) > return ret; > > - if (val) > + if (val) { > + ret = pm_runtime_resume_and_get(dev->parent); > + if (ret) > + return ret; > ret = cti_enable(drvdata->csdev); > - else > + if (ret) > + pm_runtime_put(dev->parent); > + } else { > ret = cti_disable(drvdata->csdev); > + if (!ret) > + pm_runtime_put(dev->parent); > + } > + > if (ret) > return ret; > return size; Looks good to me. @Mao Jinolong, Are you able to test the patches 1 & 2 and confirm they solve your issue ? Suzuki
On 1/10/2023 12:47 AM, Suzuki K Poulose wrote: > On 06/01/2023 15:23, James Clark wrote: >> From: Mao Jinlong <quic_jinlmao@quicinc.com> >> >> In commit 6746eae4bbad ("coresight: cti: Fix hang in cti_disable_hw()") >> PM runtime calls are removed from cti_enable_hw/cti_disable_hw. When >> enabling CTI by writing enable sysfs node, clock for accessing CTI >> register won't be enabled. Device will crash due to register access >> issue. Add PM runtime call in enable_store to fix this issue. >> >> Fixes: 6746eae4bbad ("coresight: cti: Fix hang in cti_disable_hw()") >> Signed-off-by: Mao Jinlong <quic_jinlmao@quicinc.com> >> [Change to only call pm_runtime_put if a disable happened] >> Signed-off-by: James Clark <james.clark@arm.com> >> --- >> drivers/hwtracing/coresight/coresight-cti-sysfs.c | 13 +++++++++++-- >> 1 file changed, 11 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/hwtracing/coresight/coresight-cti-sysfs.c >> b/drivers/hwtracing/coresight/coresight-cti-sysfs.c >> index 6d59c815ecf5..71e7a8266bb3 100644 >> --- a/drivers/hwtracing/coresight/coresight-cti-sysfs.c >> +++ b/drivers/hwtracing/coresight/coresight-cti-sysfs.c >> @@ -108,10 +108,19 @@ static ssize_t enable_store(struct device *dev, >> if (ret) >> return ret; >> - if (val) >> + if (val) { >> + ret = pm_runtime_resume_and_get(dev->parent); >> + if (ret) >> + return ret; >> ret = cti_enable(drvdata->csdev); >> - else >> + if (ret) >> + pm_runtime_put(dev->parent); >> + } else { >> ret = cti_disable(drvdata->csdev); >> + if (!ret) >> + pm_runtime_put(dev->parent); >> + } >> + >> if (ret) >> return ret; >> return size; > > Looks good to me. > > @Mao Jinolong, > > Are you able to test the patches 1 & 2 and confirm they solve your > issue ? Hi Suzuki, Tested from my side. Patch 1 & 2 can solve the issue when enable CTI by writing 1 to enable sysfs node. Thanks Jinlong Mao > > Suzuki
On 10/01/2023 05:56, Jinlong Mao wrote: > > On 1/10/2023 12:47 AM, Suzuki K Poulose wrote: >> On 06/01/2023 15:23, James Clark wrote: >>> From: Mao Jinlong <quic_jinlmao@quicinc.com> >>> >>> In commit 6746eae4bbad ("coresight: cti: Fix hang in cti_disable_hw()") >>> PM runtime calls are removed from cti_enable_hw/cti_disable_hw. When >>> enabling CTI by writing enable sysfs node, clock for accessing CTI >>> register won't be enabled. Device will crash due to register access >>> issue. Add PM runtime call in enable_store to fix this issue. >>> >>> Fixes: 6746eae4bbad ("coresight: cti: Fix hang in cti_disable_hw()") >>> Signed-off-by: Mao Jinlong <quic_jinlmao@quicinc.com> >>> [Change to only call pm_runtime_put if a disable happened] >>> Signed-off-by: James Clark <james.clark@arm.com> >>> --- >>> drivers/hwtracing/coresight/coresight-cti-sysfs.c | 13 +++++++++++-- >>> 1 file changed, 11 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/hwtracing/coresight/coresight-cti-sysfs.c >>> b/drivers/hwtracing/coresight/coresight-cti-sysfs.c >>> index 6d59c815ecf5..71e7a8266bb3 100644 >>> --- a/drivers/hwtracing/coresight/coresight-cti-sysfs.c >>> +++ b/drivers/hwtracing/coresight/coresight-cti-sysfs.c >>> @@ -108,10 +108,19 @@ static ssize_t enable_store(struct device *dev, >>> if (ret) >>> return ret; >>> - if (val) >>> + if (val) { >>> + ret = pm_runtime_resume_and_get(dev->parent); >>> + if (ret) >>> + return ret; >>> ret = cti_enable(drvdata->csdev); >>> - else >>> + if (ret) >>> + pm_runtime_put(dev->parent); >>> + } else { >>> ret = cti_disable(drvdata->csdev); >>> + if (!ret) >>> + pm_runtime_put(dev->parent); >>> + } >>> + >>> if (ret) >>> return ret; >>> return size; >> >> Looks good to me. >> >> @Mao Jinolong, >> >> Are you able to test the patches 1 & 2 and confirm they solve your >> issue ? > > Hi Suzuki, > > Tested from my side. Patch 1 & 2 can solve the issue when enable CTI by > writing 1 to enable sysfs node. Thanks, I added your tested-by tag on patches 1 + 2.
diff --git a/drivers/hwtracing/coresight/coresight-cti-sysfs.c b/drivers/hwtracing/coresight/coresight-cti-sysfs.c index 6d59c815ecf5..71e7a8266bb3 100644 --- a/drivers/hwtracing/coresight/coresight-cti-sysfs.c +++ b/drivers/hwtracing/coresight/coresight-cti-sysfs.c @@ -108,10 +108,19 @@ static ssize_t enable_store(struct device *dev, if (ret) return ret; - if (val) + if (val) { + ret = pm_runtime_resume_and_get(dev->parent); + if (ret) + return ret; ret = cti_enable(drvdata->csdev); - else + if (ret) + pm_runtime_put(dev->parent); + } else { ret = cti_disable(drvdata->csdev); + if (!ret) + pm_runtime_put(dev->parent); + } + if (ret) return ret; return size;