1. Patchew
  2. linux
  3. [PATCH v1 0/9] coresight: Fix and improve clock usage
  4. View patch

[PATCH v1 1/9] coresight: tmc: Support atclk

Leo Yan posted 9 patches 8 months, 3 weeks ago
There is a newer version of this series
[PATCH v1 1/9] coresight: tmc: Support atclk
Posted by Leo Yan 8 months, 3 weeks ago 
The atclk is an optional clock for the CoreSight TMC, but the driver
misses to initialize it.  In most cases, the TMC shares the same atclk
with other CoreSight components.  Since these components enable the
clock before the TMC device is initialized, the TMC continues properly,
which is why we don’t observe any lockup issues.

This change enables atclk in probe of the TMC driver.  Given the clock
is optional, it is possible to return NULL if the clock does not exist.
IS_ERR() is tolerant for this case.

Dynamically disable and enable atclk during suspend and resume.  The
clock pointers will never be error values if the driver has successfully
probed, and the case of a NULL pointer case will be handled by the clock
core layer.  The driver data is always valid after probe. Therefore,
remove the related checks.  Also in the resume flow adds error handling.

Fixes: bc4bf7fe98da ("coresight-tmc: add CoreSight TMC driver")
Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 drivers/hwtracing/coresight/coresight-tmc-core.c | 22 +++++++++++++++++-----
 drivers/hwtracing/coresight/coresight-tmc.h      |  2 ++
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c
index 5978bcda2556..6aad2acd0378 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-core.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-core.c
@@ -789,6 +789,10 @@ static int __tmc_probe(struct device *dev, struct resource *res)
 	struct coresight_desc desc = { 0 };
 	struct coresight_dev_list *dev_list = NULL;
 
+	drvdata->atclk = devm_clk_get_optional_enabled(dev, "atclk");
+	if (IS_ERR(drvdata->atclk))
+		return PTR_ERR(drvdata->atclk);
+
 	ret = -ENOMEM;
 
 	/* Validity for the resource is already checked by the AMBA core */
@@ -1019,18 +1023,26 @@ static int tmc_runtime_suspend(struct device *dev)
 {
 	struct tmc_drvdata *drvdata = dev_get_drvdata(dev);
 
-	if (drvdata && !IS_ERR_OR_NULL(drvdata->pclk))
-		clk_disable_unprepare(drvdata->pclk);
+	clk_disable_unprepare(drvdata->atclk);
+	clk_disable_unprepare(drvdata->pclk);
+
 	return 0;
 }
 
 static int tmc_runtime_resume(struct device *dev)
 {
 	struct tmc_drvdata *drvdata = dev_get_drvdata(dev);
+	int ret;
 
-	if (drvdata && !IS_ERR_OR_NULL(drvdata->pclk))
-		clk_prepare_enable(drvdata->pclk);
-	return 0;
+	ret = clk_prepare_enable(drvdata->pclk);
+	if (ret)
+		return ret;
+
+	ret = clk_prepare_enable(drvdata->atclk);
+	if (ret)
+		clk_disable_unprepare(drvdata->pclk);
+
+	return ret;
 }
 #endif
 
diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h
index 6541a27a018e..cbb4ba439158 100644
--- a/drivers/hwtracing/coresight/coresight-tmc.h
+++ b/drivers/hwtracing/coresight/coresight-tmc.h
@@ -210,6 +210,7 @@ struct tmc_resrv_buf {
 
 /**
  * struct tmc_drvdata - specifics associated to an TMC component
+ * @atclk:	optional clock for the core parts of the TMC.
  * @pclk:	APB clock if present, otherwise NULL
  * @base:	memory mapped base address for this component.
  * @csdev:	component vitals needed by the framework.
@@ -244,6 +245,7 @@ struct tmc_resrv_buf {
  *		 Used by ETR/ETF.
  */
 struct tmc_drvdata {
+	struct clk		*atclk;
 	struct clk		*pclk;
 	void __iomem		*base;
 	struct coresight_device	*csdev;
-- 
2.34.1
Re: [PATCH v1 1/9] coresight: tmc: Support atclk
Posted by Anshuman Khandual 8 months, 2 weeks ago 
On 3/27/25 17:07, Leo Yan wrote:
> The atclk is an optional clock for the CoreSight TMC, but the driver
> misses to initialize it.  In most cases, the TMC shares the same atclk

TMC shares the atclk or pclk clock with other coresight components ?

> with other CoreSight components.  Since these components enable the
> clock before the TMC device is initialized, the TMC continues properly,
> which is why we don’t observe any lockup issues.
> 
> This change enables atclk in probe of the TMC driver.  Given the clock
> is optional, it is possible to return NULL if the clock does not exist.
> IS_ERR() is tolerant for this case.

Makes sense.

> 
> Dynamically disable and enable atclk during suspend and resume.  The
> clock pointers will never be error values if the driver has successfully
> probed, and the case of a NULL pointer case will be handled by the clock
> core layer.  The driver data is always valid after probe. Therefore,
> remove the related checks.  Also in the resume flow adds error handling.

Probably a good clean up.

> 
> Fixes: bc4bf7fe98da ("coresight-tmc: add CoreSight TMC driver")

Right commit to attribute.

> Signed-off-by: Leo Yan <leo.yan@arm.com>
> ---
>  drivers/hwtracing/coresight/coresight-tmc-core.c | 22 +++++++++++++++++-----
>  drivers/hwtracing/coresight/coresight-tmc.h      |  2 ++
>  2 files changed, 19 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c
> index 5978bcda2556..6aad2acd0378 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-core.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-core.c
> @@ -789,6 +789,10 @@ static int __tmc_probe(struct device *dev, struct resource *res)
>  	struct coresight_desc desc = { 0 };
>  	struct coresight_dev_list *dev_list = NULL;
>  
> +	drvdata->atclk = devm_clk_get_optional_enabled(dev, "atclk");
> +	if (IS_ERR(drvdata->atclk))
> +		return PTR_ERR(drvdata->atclk);
> +

Adding this check here in __tmc_probe() ensures that it gets called
both during AMBA and platform probe methods.

>  	ret = -ENOMEM;
>  
>  	/* Validity for the resource is already checked by the AMBA core */
> @@ -1019,18 +1023,26 @@ static int tmc_runtime_suspend(struct device *dev)
>  {
>  	struct tmc_drvdata *drvdata = dev_get_drvdata(dev);
>  
> -	if (drvdata && !IS_ERR_OR_NULL(drvdata->pclk))
> -		clk_disable_unprepare(drvdata->pclk);
> +	clk_disable_unprepare(drvdata->atclk);
> +	clk_disable_unprepare(drvdata->pclk);
> +
>  	return 0;
>  }
>  
>  static int tmc_runtime_resume(struct device *dev)
>  {
>  	struct tmc_drvdata *drvdata = dev_get_drvdata(dev);
> +	int ret;
>  
> -	if (drvdata && !IS_ERR_OR_NULL(drvdata->pclk))
> -		clk_prepare_enable(drvdata->pclk);
> -	return 0;
> +	ret = clk_prepare_enable(drvdata->pclk);
> +	if (ret)
> +		return ret;
> +
> +	ret = clk_prepare_enable(drvdata->atclk);
> +	if (ret)
> +		clk_disable_unprepare(drvdata->pclk);
> +
> +	return ret;
>  }
>  #endif
>  
> diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h
> index 6541a27a018e..cbb4ba439158 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc.h
> +++ b/drivers/hwtracing/coresight/coresight-tmc.h
> @@ -210,6 +210,7 @@ struct tmc_resrv_buf {
>  
>  /**
>   * struct tmc_drvdata - specifics associated to an TMC component
> + * @atclk:	optional clock for the core parts of the TMC.
>   * @pclk:	APB clock if present, otherwise NULL
>   * @base:	memory mapped base address for this component.
>   * @csdev:	component vitals needed by the framework.
> @@ -244,6 +245,7 @@ struct tmc_resrv_buf {
>   *		 Used by ETR/ETF.
>   */
>  struct tmc_drvdata {
> +	struct clk		*atclk;
>  	struct clk		*pclk;
>  	void __iomem		*base;
>  	struct coresight_device	*csdev;

Otherwise, LGTM.

Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>
Re: [PATCH v1 1/9] coresight: tmc: Support atclk
Posted by Leo Yan 7 months, 4 weeks ago 
On Thu, Apr 03, 2025 at 11:20:38AM +0530, Anshuman Khandual wrote:
> On 3/27/25 17:07, Leo Yan wrote:
> > The atclk is an optional clock for the CoreSight TMC, but the driver
> > misses to initialize it.  In most cases, the TMC shares the same atclk
> 
> TMC shares the atclk or pclk clock with other coresight components ?

I will refine the commit log for this.

[...]

> > --- a/drivers/hwtracing/coresight/coresight-tmc-core.c
> > +++ b/drivers/hwtracing/coresight/coresight-tmc-core.c
> > @@ -789,6 +789,10 @@ static int __tmc_probe(struct device *dev, struct resource *res)
> >  	struct coresight_desc desc = { 0 };
> >  	struct coresight_dev_list *dev_list = NULL;
> >  
> > +	drvdata->atclk = devm_clk_get_optional_enabled(dev, "atclk");
> > +	if (IS_ERR(drvdata->atclk))
> > +		return PTR_ERR(drvdata->atclk);
> > +
> 
> Adding this check here in __tmc_probe() ensures that it gets called
> both during AMBA and platform probe methods.

Yes.

> >  /**
> >   * struct tmc_drvdata - specifics associated to an TMC component
> > + * @atclk:	optional clock for the core parts of the TMC.
> >   * @pclk:	APB clock if present, otherwise NULL
> >   * @base:	memory mapped base address for this component.
> >   * @csdev:	component vitals needed by the framework.
> > @@ -244,6 +245,7 @@ struct tmc_resrv_buf {
> >   *		 Used by ETR/ETF.
> >   */
> >  struct tmc_drvdata {
> > +	struct clk		*atclk;
> >  	struct clk		*pclk;
> >  	void __iomem		*base;
> >  	struct coresight_device	*csdev;
> 
> Otherwise, LGTM.
> 
> Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>

Thanks for reviewing!

Leo

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.