Good morning Sai,

On Thu, May 14, 2020 at 04:29:15PM +0530, Sai Prakash Ranjan wrote:
> From: Tingwei Zhang <ting...@codeaurora.org>
> 
> 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 <ting...@codeaurora.org>
> Co-developed-by: Sai Prakash Ranjan <saiprakash.ran...@codeaurora.org>
> Signed-off-by: Sai Prakash Ranjan <saiprakash.ran...@codeaurora.org>

Co-developed-by: Sai Prakash Ranjan <saiprakash.ran...@codeaurora.org>
Signed-off-by: Tingwei Zhang <ting...@codeaurora.org>

> ---
>  .../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

Reply via email to