Good morning Sai, On Thu, May 14, 2020 at 04:29:15PM +0530, Sai Prakash Ranjan wrote: > From: Tingwei Zhang <tingwei@xxxxxxxxxxxxxx> > > On some Qualcomm Technologies Inc. SoCs like SC7180, there > exists a hardware errata where the APSS (Application Processor > SubSystem)/CPU watchdog counter is stopped when ETM register > TRCPDCR.PU=1. Fun stuff... > Since the ETMs share the same power domain as > that of respective CPU cores, they are powered on when the > CPU core is powered on. So we can disable powering up of the > trace unit after checking for this errata via new property > called "qcom,tupwr-disable". > > Signed-off-by: Tingwei Zhang <tingwei@xxxxxxxxxxxxxx> > Co-developed-by: Sai Prakash Ranjan <saiprakash.ranjan@xxxxxxxxxxxxxx> > Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@xxxxxxxxxxxxxx> Co-developed-by: Sai Prakash Ranjan <saiprakash.ranjan@xxxxxxxxxxxxxx> Signed-off-by: Tingwei Zhang <tingwei@xxxxxxxxxxxxxx> > --- > .../devicetree/bindings/arm/coresight.txt | 6 ++++ > drivers/hwtracing/coresight/coresight-etm4x.c | 29 ++++++++++++------- Please split in two patches. > 2 files changed, 25 insertions(+), 10 deletions(-) > > diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt > index 846f6daae71b..d2030128fe46 100644 > --- a/Documentation/devicetree/bindings/arm/coresight.txt > +++ b/Documentation/devicetree/bindings/arm/coresight.txt > @@ -108,6 +108,12 @@ its hardware characteristcs. > * arm,cp14: must be present if the system accesses ETM/PTM management > registers via co-processor 14. > > + * qcom,tupwr-disable: boolean. Indicates that trace unit power up can > + be disabled on Qualcomm Technologies Inc. systems where ETMs are in > + the same power domain as their CPU cores. This property is required > + to identify such systems with hardware errata where the CPU watchdog > + counter is stopped when TRCPDCR.PU=1. > + I think something like "qcom,skip-power-up" would be clearer. Also, a better choice of words is that TRCPDCR.PU does not have to be set on Qualcomm... > * Optional property for TMC: > > * arm,buffer-size: size of contiguous buffer space for TMC ETR > diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c > index fb0f5f4f3a91..6886b44f6947 100644 > --- a/drivers/hwtracing/coresight/coresight-etm4x.c > +++ b/drivers/hwtracing/coresight/coresight-etm4x.c > @@ -104,6 +104,11 @@ struct etm4_enable_arg { > int rc; > }; > > +static inline bool etm4_can_disable_tupwr(struct device *dev) > +{ > + return fwnode_property_present(dev_fwnode(dev), "qcom,tupwr-disable"); > +} > + Please call fwnode_property_present() at initialisation time to set a new drvdata::skip_power_up variable. From there just switch on that in etm4_enable/disable_hw(). Thanks, Mathieu > static int etm4_enable_hw(struct etmv4_drvdata *drvdata) > { > int i, rc; > @@ -196,12 +201,14 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata) > writel_relaxed(config->vmid_mask0, drvdata->base + TRCVMIDCCTLR0); > writel_relaxed(config->vmid_mask1, drvdata->base + TRCVMIDCCTLR1); > > - /* > - * Request to keep the trace unit powered and also > - * emulation of powerdown > - */ > - writel_relaxed(readl_relaxed(drvdata->base + TRCPDCR) | TRCPDCR_PU, > - drvdata->base + TRCPDCR); > + if (!etm4_can_disable_tupwr(etm_dev)) { > + /* > + * Request to keep the trace unit powered and also > + * emulation of powerdown > + */ > + writel_relaxed(readl_relaxed(drvdata->base + TRCPDCR) | TRCPDCR_PU, > + drvdata->base + TRCPDCR); > + } > > /* Enable the trace unit */ > writel_relaxed(1, drvdata->base + TRCPRGCTLR); > @@ -476,10 +483,12 @@ static void etm4_disable_hw(void *info) > > CS_UNLOCK(drvdata->base); > > - /* power can be removed from the trace unit now */ > - control = readl_relaxed(drvdata->base + TRCPDCR); > - control &= ~TRCPDCR_PU; > - writel_relaxed(control, drvdata->base + TRCPDCR); > + if (!etm4_can_disable_tupwr(etm_dev)) { > + /* power can be removed from the trace unit now */ > + control = readl_relaxed(drvdata->base + TRCPDCR); > + control &= ~TRCPDCR_PU; > + writel_relaxed(control, drvdata->base + TRCPDCR); > + } > > control = readl_relaxed(drvdata->base + TRCPRGCTLR); > > -- > QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member > of Code Aurora Forum, hosted by The Linux Foundation