* [PATCH 1/7] coresight: Rename coresight_{set,clear}_claim_tags()
2025-02-11 10:39 [PATCH 0/7] coresight: Clear self hosted claim tag on probe James Clark
@ 2025-02-11 10:39 ` James Clark
2025-03-13 11:24 ` Leo Yan
2025-02-11 10:39 ` [PATCH 2/7] coresight: Convert disclaim functions to take a struct cs_access James Clark
` (5 subsequent siblings)
6 siblings, 1 reply; 25+ messages in thread
From: James Clark @ 2025-02-11 10:39 UTC (permalink / raw)
To: lcherian, coresight
Cc: James Clark, Suzuki K Poulose, Mike Leach, Alexander Shishkin,
Maxime Coquelin, Alexandre Torgue, linux-arm-kernel, linux-kernel,
linux-stm32
These look like they set the whole tags register as one value, but they
only set and clear the self hosted bit using a SET/CLR bits mechanism.
Rename the functions to reflect this better.
Signed-off-by: James Clark <james.clark@linaro.org>
---
drivers/hwtracing/coresight/coresight-core.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index 0a9380350fb5..523dbb381f90 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -142,14 +142,14 @@ static inline bool coresight_is_claimed_any(struct coresight_device *csdev)
return coresight_read_claim_tags(csdev) != 0;
}
-static inline void coresight_set_claim_tags(struct coresight_device *csdev)
+static inline void coresight_set_self_claim_tag(struct coresight_device *csdev)
{
csdev_access_relaxed_write32(&csdev->access, CORESIGHT_CLAIM_SELF_HOSTED,
CORESIGHT_CLAIMSET);
isb();
}
-static inline void coresight_clear_claim_tags(struct coresight_device *csdev)
+static inline void coresight_clear_self_claim_tag(struct coresight_device *csdev)
{
csdev_access_relaxed_write32(&csdev->access, CORESIGHT_CLAIM_SELF_HOSTED,
CORESIGHT_CLAIMCLR);
@@ -174,11 +174,11 @@ int coresight_claim_device_unlocked(struct coresight_device *csdev)
if (coresight_is_claimed_any(csdev))
return -EBUSY;
- coresight_set_claim_tags(csdev);
+ coresight_set_self_claim_tag(csdev);
if (coresight_is_claimed_self_hosted(csdev))
return 0;
- /* There was a race setting the tags, clean up and fail */
- coresight_clear_claim_tags(csdev);
+ /* There was a race setting the tag, clean up and fail */
+ coresight_clear_self_claim_tag(csdev);
return -EBUSY;
}
EXPORT_SYMBOL_GPL(coresight_claim_device_unlocked);
@@ -199,7 +199,7 @@ int coresight_claim_device(struct coresight_device *csdev)
EXPORT_SYMBOL_GPL(coresight_claim_device);
/*
- * coresight_disclaim_device_unlocked : Clear the claim tags for the device.
+ * coresight_disclaim_device_unlocked : Clear the claim tag for the device.
* Called with CS_UNLOCKed for the component.
*/
void coresight_disclaim_device_unlocked(struct coresight_device *csdev)
@@ -209,7 +209,7 @@ void coresight_disclaim_device_unlocked(struct coresight_device *csdev)
return;
if (coresight_is_claimed_self_hosted(csdev))
- coresight_clear_claim_tags(csdev);
+ coresight_clear_self_claim_tag(csdev);
else
/*
* The external agent may have not honoured our claim
--
2.34.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 1/7] coresight: Rename coresight_{set,clear}_claim_tags()
2025-02-11 10:39 ` [PATCH 1/7] coresight: Rename coresight_{set,clear}_claim_tags() James Clark
@ 2025-03-13 11:24 ` Leo Yan
0 siblings, 0 replies; 25+ messages in thread
From: Leo Yan @ 2025-03-13 11:24 UTC (permalink / raw)
To: James Clark
Cc: lcherian, coresight, Mike Leach, Alexander Shishkin,
Maxime Coquelin, Alexandre Torgue, linux-arm-kernel, linux-kernel,
linux-stm32
On Tue, Feb 11, 2025 at 10:39:37AM +0000, James Clark wrote:
>
> These look like they set the whole tags register as one value, but they
> only set and clear the self hosted bit using a SET/CLR bits mechanism.
> Rename the functions to reflect this better.
>
> Signed-off-by: James Clark <james.clark@linaro.org>
Reviewed-by: Leo Yan <leo.yan@arm.com>
> ---
> drivers/hwtracing/coresight/coresight-core.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> index 0a9380350fb5..523dbb381f90 100644
> --- a/drivers/hwtracing/coresight/coresight-core.c
> +++ b/drivers/hwtracing/coresight/coresight-core.c
> @@ -142,14 +142,14 @@ static inline bool coresight_is_claimed_any(struct coresight_device *csdev)
> return coresight_read_claim_tags(csdev) != 0;
> }
>
> -static inline void coresight_set_claim_tags(struct coresight_device *csdev)
> +static inline void coresight_set_self_claim_tag(struct coresight_device *csdev)
> {
> csdev_access_relaxed_write32(&csdev->access, CORESIGHT_CLAIM_SELF_HOSTED,
> CORESIGHT_CLAIMSET);
> isb();
> }
>
> -static inline void coresight_clear_claim_tags(struct coresight_device *csdev)
> +static inline void coresight_clear_self_claim_tag(struct coresight_device *csdev)
> {
> csdev_access_relaxed_write32(&csdev->access, CORESIGHT_CLAIM_SELF_HOSTED,
> CORESIGHT_CLAIMCLR);
> @@ -174,11 +174,11 @@ int coresight_claim_device_unlocked(struct coresight_device *csdev)
> if (coresight_is_claimed_any(csdev))
> return -EBUSY;
>
> - coresight_set_claim_tags(csdev);
> + coresight_set_self_claim_tag(csdev);
> if (coresight_is_claimed_self_hosted(csdev))
> return 0;
> - /* There was a race setting the tags, clean up and fail */
> - coresight_clear_claim_tags(csdev);
> + /* There was a race setting the tag, clean up and fail */
> + coresight_clear_self_claim_tag(csdev);
> return -EBUSY;
> }
> EXPORT_SYMBOL_GPL(coresight_claim_device_unlocked);
> @@ -199,7 +199,7 @@ int coresight_claim_device(struct coresight_device *csdev)
> EXPORT_SYMBOL_GPL(coresight_claim_device);
>
> /*
> - * coresight_disclaim_device_unlocked : Clear the claim tags for the device.
> + * coresight_disclaim_device_unlocked : Clear the claim tag for the device.
> * Called with CS_UNLOCKed for the component.
> */
> void coresight_disclaim_device_unlocked(struct coresight_device *csdev)
> @@ -209,7 +209,7 @@ void coresight_disclaim_device_unlocked(struct coresight_device *csdev)
> return;
>
> if (coresight_is_claimed_self_hosted(csdev))
> - coresight_clear_claim_tags(csdev);
> + coresight_clear_self_claim_tag(csdev);
> else
> /*
> * The external agent may have not honoured our claim
> --
> 2.34.1
>
> _______________________________________________
> CoreSight mailing list -- coresight@lists.linaro.org
> To unsubscribe send an email to coresight-leave@lists.linaro.org
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 2/7] coresight: Convert disclaim functions to take a struct cs_access
2025-02-11 10:39 [PATCH 0/7] coresight: Clear self hosted claim tag on probe James Clark
2025-02-11 10:39 ` [PATCH 1/7] coresight: Rename coresight_{set,clear}_claim_tags() James Clark
@ 2025-02-11 10:39 ` James Clark
2025-03-13 14:54 ` Leo Yan
2025-02-11 10:39 ` [PATCH 3/7] coresight: Only check bottom two claim bits James Clark
` (4 subsequent siblings)
6 siblings, 1 reply; 25+ messages in thread
From: James Clark @ 2025-02-11 10:39 UTC (permalink / raw)
To: lcherian, coresight
Cc: James Clark, Suzuki K Poulose, Mike Leach, Alexander Shishkin,
Maxime Coquelin, Alexandre Torgue, linux-arm-kernel, linux-kernel,
linux-stm32
The self hosted claim tag will be reset on device probe in a later
commit. We'll want to do this before coresight_register() is called so
won't have a coresight_device and have to use cs_access instead.
Signed-off-by: James Clark <james.clark@linaro.org>
---
drivers/hwtracing/coresight/coresight-catu.c | 2 +-
drivers/hwtracing/coresight/coresight-core.c | 45 ++++++++-----------
.../hwtracing/coresight/coresight-cti-core.c | 6 +--
drivers/hwtracing/coresight/coresight-etb10.c | 2 +-
.../coresight/coresight-etm3x-core.c | 4 +-
.../coresight/coresight-etm4x-core.c | 2 +-
.../hwtracing/coresight/coresight-funnel.c | 2 +-
.../coresight/coresight-replicator.c | 4 +-
.../hwtracing/coresight/coresight-tmc-etf.c | 8 ++--
.../hwtracing/coresight/coresight-tmc-etr.c | 4 +-
include/linux/coresight.h | 4 +-
11 files changed, 38 insertions(+), 45 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-catu.c b/drivers/hwtracing/coresight/coresight-catu.c
index 275cc0d9f505..d9259c0b6e64 100644
--- a/drivers/hwtracing/coresight/coresight-catu.c
+++ b/drivers/hwtracing/coresight/coresight-catu.c
@@ -474,7 +474,7 @@ static int catu_disable_hw(struct catu_drvdata *drvdata)
struct coresight_device *csdev = drvdata->csdev;
catu_write_control(drvdata, 0);
- coresight_disclaim_device_unlocked(csdev);
+ coresight_disclaim_device_unlocked(&csdev->access);
if (catu_wait_for_ready(drvdata)) {
dev_info(dev, "Timeout while waiting for READY\n");
rc = -EAGAIN;
diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index 523dbb381f90..a669872b4118 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -127,31 +127,31 @@ coresight_find_out_connection(struct coresight_device *csdev,
return ERR_PTR(-ENODEV);
}
-static inline u32 coresight_read_claim_tags(struct coresight_device *csdev)
+static inline u32 coresight_read_claim_tags(struct csdev_access *csa)
{
- return csdev_access_relaxed_read32(&csdev->access, CORESIGHT_CLAIMCLR);
+ return csdev_access_relaxed_read32(csa, CORESIGHT_CLAIMCLR);
}
-static inline bool coresight_is_claimed_self_hosted(struct coresight_device *csdev)
+static inline bool coresight_is_claimed_self_hosted(struct csdev_access *csa)
{
- return coresight_read_claim_tags(csdev) == CORESIGHT_CLAIM_SELF_HOSTED;
+ return coresight_read_claim_tags(csa) == CORESIGHT_CLAIM_SELF_HOSTED;
}
static inline bool coresight_is_claimed_any(struct coresight_device *csdev)
{
- return coresight_read_claim_tags(csdev) != 0;
+ return coresight_read_claim_tags(&csdev->access) != 0;
}
-static inline void coresight_set_self_claim_tag(struct coresight_device *csdev)
+static inline void coresight_set_self_claim_tag(struct csdev_access *csa)
{
- csdev_access_relaxed_write32(&csdev->access, CORESIGHT_CLAIM_SELF_HOSTED,
+ csdev_access_relaxed_write32(csa, CORESIGHT_CLAIM_SELF_HOSTED,
CORESIGHT_CLAIMSET);
isb();
}
-static inline void coresight_clear_self_claim_tag(struct coresight_device *csdev)
+static inline void coresight_clear_self_claim_tag(struct csdev_access *csa)
{
- csdev_access_relaxed_write32(&csdev->access, CORESIGHT_CLAIM_SELF_HOSTED,
+ csdev_access_relaxed_write32(csa, CORESIGHT_CLAIM_SELF_HOSTED,
CORESIGHT_CLAIMCLR);
isb();
}
@@ -174,11 +174,11 @@ int coresight_claim_device_unlocked(struct coresight_device *csdev)
if (coresight_is_claimed_any(csdev))
return -EBUSY;
- coresight_set_self_claim_tag(csdev);
- if (coresight_is_claimed_self_hosted(csdev))
+ coresight_set_self_claim_tag(&csdev->access);
+ if (coresight_is_claimed_self_hosted(&csdev->access))
return 0;
/* There was a race setting the tag, clean up and fail */
- coresight_clear_self_claim_tag(csdev);
+ coresight_clear_self_claim_tag(&csdev->access);
return -EBUSY;
}
EXPORT_SYMBOL_GPL(coresight_claim_device_unlocked);
@@ -202,14 +202,10 @@ EXPORT_SYMBOL_GPL(coresight_claim_device);
* coresight_disclaim_device_unlocked : Clear the claim tag for the device.
* Called with CS_UNLOCKed for the component.
*/
-void coresight_disclaim_device_unlocked(struct coresight_device *csdev)
+void coresight_disclaim_device_unlocked(struct csdev_access *csa)
{
-
- if (WARN_ON(!csdev))
- return;
-
- if (coresight_is_claimed_self_hosted(csdev))
- coresight_clear_self_claim_tag(csdev);
+ if (coresight_is_claimed_self_hosted(csa))
+ coresight_clear_self_claim_tag(csa);
else
/*
* The external agent may have not honoured our claim
@@ -220,14 +216,11 @@ void coresight_disclaim_device_unlocked(struct coresight_device *csdev)
}
EXPORT_SYMBOL_GPL(coresight_disclaim_device_unlocked);
-void coresight_disclaim_device(struct coresight_device *csdev)
+void coresight_disclaim_device(struct csdev_access *csa)
{
- if (WARN_ON(!csdev))
- return;
-
- CS_UNLOCK(csdev->access.base);
- coresight_disclaim_device_unlocked(csdev);
- CS_LOCK(csdev->access.base);
+ CS_UNLOCK(csa->base);
+ coresight_disclaim_device_unlocked(csa);
+ CS_LOCK(csa->base);
}
EXPORT_SYMBOL_GPL(coresight_disclaim_device);
diff --git a/drivers/hwtracing/coresight/coresight-cti-core.c b/drivers/hwtracing/coresight/coresight-cti-core.c
index d2b5a5718c29..073f67a41af9 100644
--- a/drivers/hwtracing/coresight/coresight-cti-core.c
+++ b/drivers/hwtracing/coresight/coresight-cti-core.c
@@ -175,7 +175,7 @@ static int cti_disable_hw(struct cti_drvdata *drvdata)
writel_relaxed(0, drvdata->base + CTICONTROL);
config->hw_enabled = false;
- coresight_disclaim_device_unlocked(csdev);
+ coresight_disclaim_device_unlocked(&csdev->access);
CS_LOCK(drvdata->base);
spin_unlock(&drvdata->spinlock);
return ret;
@@ -683,7 +683,7 @@ static int cti_cpu_pm_notify(struct notifier_block *nb, unsigned long cmd,
/* CTI regs all static - we have a copy & nothing to save */
drvdata->config.hw_powered = false;
if (drvdata->config.hw_enabled)
- coresight_disclaim_device(csdev);
+ coresight_disclaim_device(&csdev->access);
break;
case CPU_PM_ENTER_FAILED:
@@ -746,7 +746,7 @@ static int cti_dying_cpu(unsigned int cpu)
spin_lock(&drvdata->spinlock);
drvdata->config.hw_powered = false;
if (drvdata->config.hw_enabled)
- coresight_disclaim_device(drvdata->csdev);
+ coresight_disclaim_device(&drvdata->csdev->access);
spin_unlock(&drvdata->spinlock);
return 0;
}
diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c
index aea9ac9c4bd0..d8bc3e776c88 100644
--- a/drivers/hwtracing/coresight/coresight-etb10.c
+++ b/drivers/hwtracing/coresight/coresight-etb10.c
@@ -344,7 +344,7 @@ static void etb_disable_hw(struct etb_drvdata *drvdata)
{
__etb_disable_hw(drvdata);
etb_dump_hw(drvdata);
- coresight_disclaim_device(drvdata->csdev);
+ coresight_disclaim_device(&drvdata->csdev->access);
}
static int etb_disable(struct coresight_device *csdev)
diff --git a/drivers/hwtracing/coresight/coresight-etm3x-core.c b/drivers/hwtracing/coresight/coresight-etm3x-core.c
index c103f4c70f5d..509f53b69e42 100644
--- a/drivers/hwtracing/coresight/coresight-etm3x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm3x-core.c
@@ -599,7 +599,7 @@ static void etm_disable_hw(void *info)
config->cntr_val[i] = etm_readl(drvdata, ETMCNTVRn(i));
etm_set_pwrdwn(drvdata);
- coresight_disclaim_device_unlocked(csdev);
+ coresight_disclaim_device_unlocked(&csdev->access);
CS_LOCK(drvdata->base);
@@ -624,7 +624,7 @@ static void etm_disable_perf(struct coresight_device *csdev)
* power down the tracer.
*/
etm_set_pwrdwn(drvdata);
- coresight_disclaim_device_unlocked(csdev);
+ coresight_disclaim_device_unlocked(&csdev->access);
CS_LOCK(drvdata->base);
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index dd8c74f893db..45b30a4b3eba 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -922,7 +922,7 @@ static void etm4_disable_hw(void *info)
etm4x_relaxed_read32(csa, TRCCNTVRn(i));
}
- coresight_disclaim_device_unlocked(csdev);
+ coresight_disclaim_device_unlocked(&csdev->access);
etm4_cs_lock(drvdata, csa);
dev_dbg(&drvdata->csdev->dev,
diff --git a/drivers/hwtracing/coresight/coresight-funnel.c b/drivers/hwtracing/coresight/coresight-funnel.c
index 8faf51469bb8..e986922d555b 100644
--- a/drivers/hwtracing/coresight/coresight-funnel.c
+++ b/drivers/hwtracing/coresight/coresight-funnel.c
@@ -116,7 +116,7 @@ static void dynamic_funnel_disable_hw(struct funnel_drvdata *drvdata,
/* Disclaim the device if none of the slaves are now active */
if (!(functl & FUNNEL_ENSx_MASK))
- coresight_disclaim_device_unlocked(csdev);
+ coresight_disclaim_device_unlocked(&csdev->access);
CS_LOCK(drvdata->base);
}
diff --git a/drivers/hwtracing/coresight/coresight-replicator.c b/drivers/hwtracing/coresight/coresight-replicator.c
index a1181c9048c0..5d42a9a8c460 100644
--- a/drivers/hwtracing/coresight/coresight-replicator.c
+++ b/drivers/hwtracing/coresight/coresight-replicator.c
@@ -54,7 +54,7 @@ static void dynamic_replicator_reset(struct replicator_drvdata *drvdata)
if (!coresight_claim_device_unlocked(csdev)) {
writel_relaxed(0xff, drvdata->base + REPLICATOR_IDFILTER0);
writel_relaxed(0xff, drvdata->base + REPLICATOR_IDFILTER1);
- coresight_disclaim_device_unlocked(csdev);
+ coresight_disclaim_device_unlocked(&csdev->access);
}
CS_LOCK(drvdata->base);
@@ -167,7 +167,7 @@ static void dynamic_replicator_disable(struct replicator_drvdata *drvdata,
if ((readl_relaxed(drvdata->base + REPLICATOR_IDFILTER0) == 0xff) &&
(readl_relaxed(drvdata->base + REPLICATOR_IDFILTER1) == 0xff))
- coresight_disclaim_device_unlocked(csdev);
+ coresight_disclaim_device_unlocked(&csdev->access);
CS_LOCK(drvdata->base);
}
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c
index d4f641cd9de6..1e4d874a8448 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
@@ -53,7 +53,7 @@ static int tmc_etb_enable_hw(struct tmc_drvdata *drvdata)
rc = __tmc_etb_enable_hw(drvdata);
if (rc)
- coresight_disclaim_device(drvdata->csdev);
+ coresight_disclaim_device(&drvdata->csdev->access);
return rc;
}
@@ -99,7 +99,7 @@ static void __tmc_etb_disable_hw(struct tmc_drvdata *drvdata)
static void tmc_etb_disable_hw(struct tmc_drvdata *drvdata)
{
__tmc_etb_disable_hw(drvdata);
- coresight_disclaim_device(drvdata->csdev);
+ coresight_disclaim_device(&drvdata->csdev->access);
}
static int __tmc_etf_enable_hw(struct tmc_drvdata *drvdata)
@@ -136,7 +136,7 @@ static int tmc_etf_enable_hw(struct tmc_drvdata *drvdata)
rc = __tmc_etf_enable_hw(drvdata);
if (rc)
- coresight_disclaim_device(drvdata->csdev);
+ coresight_disclaim_device(&drvdata->csdev->access);
return rc;
}
@@ -148,7 +148,7 @@ static void tmc_etf_disable_hw(struct tmc_drvdata *drvdata)
tmc_flush_and_stop(drvdata);
tmc_disable_hw(drvdata);
- coresight_disclaim_device_unlocked(csdev);
+ coresight_disclaim_device_unlocked(&csdev->access);
CS_LOCK(drvdata->base);
}
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
index a48bb85d0e7f..7ba4f79191de 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
@@ -1066,7 +1066,7 @@ static int tmc_etr_enable_hw(struct tmc_drvdata *drvdata,
rc = __tmc_etr_enable_hw(drvdata);
if (rc) {
drvdata->etr_buf = NULL;
- coresight_disclaim_device(drvdata->csdev);
+ coresight_disclaim_device(&drvdata->csdev->access);
}
}
@@ -1156,7 +1156,7 @@ static void __tmc_etr_disable_hw(struct tmc_drvdata *drvdata)
void tmc_etr_disable_hw(struct tmc_drvdata *drvdata)
{
__tmc_etr_disable_hw(drvdata);
- coresight_disclaim_device(drvdata->csdev);
+ coresight_disclaim_device(&drvdata->csdev->access);
/* Reset the ETR buf used by hardware */
drvdata->etr_buf = NULL;
}
diff --git a/include/linux/coresight.h b/include/linux/coresight.h
index 17276965ff1d..937931d107e0 100644
--- a/include/linux/coresight.h
+++ b/include/linux/coresight.h
@@ -653,8 +653,8 @@ extern int coresight_timeout(struct csdev_access *csa, u32 offset,
extern int coresight_claim_device(struct coresight_device *csdev);
extern int coresight_claim_device_unlocked(struct coresight_device *csdev);
-extern void coresight_disclaim_device(struct coresight_device *csdev);
-extern void coresight_disclaim_device_unlocked(struct coresight_device *csdev);
+extern void coresight_disclaim_device(struct csdev_access *csa);
+extern void coresight_disclaim_device_unlocked(struct csdev_access *csa);
extern char *coresight_alloc_device_name(struct coresight_dev_list *devs,
struct device *dev);
--
2.34.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 2/7] coresight: Convert disclaim functions to take a struct cs_access
2025-02-11 10:39 ` [PATCH 2/7] coresight: Convert disclaim functions to take a struct cs_access James Clark
@ 2025-03-13 14:54 ` Leo Yan
2025-03-17 11:36 ` James Clark
0 siblings, 1 reply; 25+ messages in thread
From: Leo Yan @ 2025-03-13 14:54 UTC (permalink / raw)
To: James Clark
Cc: lcherian, coresight, Mike Leach, Alexander Shishkin,
Maxime Coquelin, Alexandre Torgue, linux-arm-kernel, linux-kernel,
linux-stm32
On Tue, Feb 11, 2025 at 10:39:38AM +0000, James Clark wrote:
[...]
> static inline bool coresight_is_claimed_any(struct coresight_device *csdev)
> {
> - return coresight_read_claim_tags(csdev) != 0;
> + return coresight_read_claim_tags(&csdev->access) != 0;
> }
Likewise other claim functions, can coresight_is_claimed_any() change its
argument type from struct coresight_device to struct csdev_access?
Thanks,
Leo
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 2/7] coresight: Convert disclaim functions to take a struct cs_access
2025-03-13 14:54 ` Leo Yan
@ 2025-03-17 11:36 ` James Clark
2025-03-17 18:29 ` Leo Yan
0 siblings, 1 reply; 25+ messages in thread
From: James Clark @ 2025-03-17 11:36 UTC (permalink / raw)
To: Leo Yan
Cc: lcherian, coresight, Mike Leach, Alexander Shishkin,
Maxime Coquelin, Alexandre Torgue, linux-arm-kernel, linux-kernel,
linux-stm32
On 13/03/2025 2:54 pm, Leo Yan wrote:
> On Tue, Feb 11, 2025 at 10:39:38AM +0000, James Clark wrote:
>
> [...]
>
>> static inline bool coresight_is_claimed_any(struct coresight_device *csdev)
>> {
>> - return coresight_read_claim_tags(csdev) != 0;
>> + return coresight_read_claim_tags(&csdev->access) != 0;
>> }
>
> Likewise other claim functions, can coresight_is_claimed_any() change its
> argument type from struct coresight_device to struct csdev_access?
>
> Thanks,
> Leo
I only wanted to change the ones that I had to. I think we should
prioritize passing csdev as much as possible in the coresight framework
to make everything consistent. Otherwise it's extra churn for no
benefit, and if we need something from csdev here in the future we'll
have to change this one back again.
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 2/7] coresight: Convert disclaim functions to take a struct cs_access
2025-03-17 11:36 ` James Clark
@ 2025-03-17 18:29 ` Leo Yan
2025-03-18 9:27 ` James Clark
0 siblings, 1 reply; 25+ messages in thread
From: Leo Yan @ 2025-03-17 18:29 UTC (permalink / raw)
To: James Clark
Cc: lcherian, coresight, Mike Leach, Alexander Shishkin,
Maxime Coquelin, Alexandre Torgue, linux-arm-kernel, linux-kernel,
linux-stm32
On Mon, Mar 17, 2025 at 11:36:40AM +0000, James Clark wrote:
>
>
> On 13/03/2025 2:54 pm, Leo Yan wrote:
> > On Tue, Feb 11, 2025 at 10:39:38AM +0000, James Clark wrote:
> >
> > [...]
> >
> > > static inline bool coresight_is_claimed_any(struct coresight_device *csdev)
> > > {
> > > - return coresight_read_claim_tags(csdev) != 0;
> > > + return coresight_read_claim_tags(&csdev->access) != 0;
> > > }
> >
> > Likewise other claim functions, can coresight_is_claimed_any() change its
> > argument type from struct coresight_device to struct csdev_access?
>
> I only wanted to change the ones that I had to. I think we should prioritize
> passing csdev as much as possible in the coresight framework to make
> everything consistent. Otherwise it's extra churn for no benefit, and if we
> need something from csdev here in the future we'll have to change this one
> back again.
The function coresight_is_claimed_any() has been deleted in a later
patch. So this is fine for me.
In theory, claim tags are low level operations and don't need a
CoreSight device context, I prefer we can keep them as simple as
possible.
With this series, we can see coresight_claim_device() and
coresight_disclaim_device() are inconsistent for their parameters:
one is using "struct coresight_device *" and another is
"struct csdev_access *". Maybe we just proceed to use csdev_access
for all claim tag functions?
If later we need to use a CoreSight device context when operating
claim tags, it means we might have different scenarios and we can
handle that separately.
Thanks,
Leo
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 2/7] coresight: Convert disclaim functions to take a struct cs_access
2025-03-17 18:29 ` Leo Yan
@ 2025-03-18 9:27 ` James Clark
0 siblings, 0 replies; 25+ messages in thread
From: James Clark @ 2025-03-18 9:27 UTC (permalink / raw)
To: Leo Yan
Cc: lcherian, coresight, Mike Leach, Alexander Shishkin,
Maxime Coquelin, Alexandre Torgue, linux-arm-kernel, linux-kernel,
linux-stm32
On 17/03/2025 6:29 pm, Leo Yan wrote:
> On Mon, Mar 17, 2025 at 11:36:40AM +0000, James Clark wrote:
>>
>>
>> On 13/03/2025 2:54 pm, Leo Yan wrote:
>>> On Tue, Feb 11, 2025 at 10:39:38AM +0000, James Clark wrote:
>>>
>>> [...]
>>>
>>>> static inline bool coresight_is_claimed_any(struct coresight_device *csdev)
>>>> {
>>>> - return coresight_read_claim_tags(csdev) != 0;
>>>> + return coresight_read_claim_tags(&csdev->access) != 0;
>>>> }
>>>
>>> Likewise other claim functions, can coresight_is_claimed_any() change its
>>> argument type from struct coresight_device to struct csdev_access?
>>
>> I only wanted to change the ones that I had to. I think we should prioritize
>> passing csdev as much as possible in the coresight framework to make
>> everything consistent. Otherwise it's extra churn for no benefit, and if we
>> need something from csdev here in the future we'll have to change this one
>> back again.
>
> The function coresight_is_claimed_any() has been deleted in a later
> patch. So this is fine for me.
>
> In theory, claim tags are low level operations and don't need a
> CoreSight device context, I prefer we can keep them as simple as
> possible.
>
> With this series, we can see coresight_claim_device() and
> coresight_disclaim_device() are inconsistent for their parameters:
> one is using "struct coresight_device *" and another is
> "struct csdev_access *". Maybe we just proceed to use csdev_access
> for all claim tag functions?
>
That's because coresight_claim_device() has logging which requires the
device name to make it useful. If anything, coresight_disclaim_device()
should actually take a csdev and then print a better log message. I
don't think there's a way to make it consistent when there are different
requirements for each level of operations. The ones with logging need a
csdev and the lower level ones used on probe need to work with only csa.
> If later we need to use a CoreSight device context when operating
> claim tags, it means we might have different scenarios and we can
> handle that separately.
>
> Thanks,
> Leo
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 3/7] coresight: Only check bottom two claim bits
2025-02-11 10:39 [PATCH 0/7] coresight: Clear self hosted claim tag on probe James Clark
2025-02-11 10:39 ` [PATCH 1/7] coresight: Rename coresight_{set,clear}_claim_tags() James Clark
2025-02-11 10:39 ` [PATCH 2/7] coresight: Convert disclaim functions to take a struct cs_access James Clark
@ 2025-02-11 10:39 ` James Clark
2025-03-13 11:46 ` Leo Yan
2025-02-11 10:39 ` [PATCH 4/7] coresight: Add claim tag warnings and debug messages James Clark
` (3 subsequent siblings)
6 siblings, 1 reply; 25+ messages in thread
From: James Clark @ 2025-02-11 10:39 UTC (permalink / raw)
To: lcherian, coresight
Cc: James Clark, Suzuki K Poulose, Mike Leach, Alexander Shishkin,
Maxime Coquelin, Alexandre Torgue, linux-arm-kernel, linux-kernel,
linux-stm32
The use of the whole register and == could break the claim mechanism if
any of the other bits are used in the future. The referenced doc "PSCI -
ARM DEN 0022D" also says to only read and clear the bottom two bits.
Use FIELD_GET() to extract only the relevant part.
Signed-off-by: James Clark <james.clark@linaro.org>
---
drivers/hwtracing/coresight/coresight-core.c | 3 ++-
drivers/hwtracing/coresight/coresight-priv.h | 1 +
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index a669872b4118..7b53165c93af 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -129,7 +129,8 @@ coresight_find_out_connection(struct coresight_device *csdev,
static inline u32 coresight_read_claim_tags(struct csdev_access *csa)
{
- return csdev_access_relaxed_read32(csa, CORESIGHT_CLAIMCLR);
+ return FIELD_GET(CORESIGHT_CLAIM_MASK,
+ csdev_access_relaxed_read32(csa, CORESIGHT_CLAIMCLR));
}
static inline bool coresight_is_claimed_self_hosted(struct csdev_access *csa)
diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
index 05f891ca6b5c..cc7ff1e36ef4 100644
--- a/drivers/hwtracing/coresight/coresight-priv.h
+++ b/drivers/hwtracing/coresight/coresight-priv.h
@@ -35,6 +35,7 @@ extern const struct device_type coresight_dev_type[];
* Coresight device CLAIM protocol.
* See PSCI - ARM DEN 0022D, Section: 6.8.1 Debug and Trace save and restore.
*/
+#define CORESIGHT_CLAIM_MASK GENMASK(1, 0)
#define CORESIGHT_CLAIM_SELF_HOSTED BIT(1)
#define TIMEOUT_US 100
--
2.34.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 3/7] coresight: Only check bottom two claim bits
2025-02-11 10:39 ` [PATCH 3/7] coresight: Only check bottom two claim bits James Clark
@ 2025-03-13 11:46 ` Leo Yan
0 siblings, 0 replies; 25+ messages in thread
From: Leo Yan @ 2025-03-13 11:46 UTC (permalink / raw)
To: James Clark
Cc: lcherian, coresight, Mike Leach, Alexander Shishkin,
Maxime Coquelin, Alexandre Torgue, linux-arm-kernel, linux-kernel,
linux-stm32
On Tue, Feb 11, 2025 at 10:39:39AM +0000, James Clark wrote:
>
> The use of the whole register and == could break the claim mechanism if
> any of the other bits are used in the future. The referenced doc "PSCI -
> ARM DEN 0022D" also says to only read and clear the bottom two bits.
>
> Use FIELD_GET() to extract only the relevant part.
>
> Signed-off-by: James Clark <james.clark@linaro.org>
I checked the latest PSCI spec DEN0022F.b, which has no change for
for only using lowest two bits in claim register.
Reviewed-by: Leo Yan <leo.yan@arm.com>
> ---
> drivers/hwtracing/coresight/coresight-core.c | 3 ++-
> drivers/hwtracing/coresight/coresight-priv.h | 1 +
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> index a669872b4118..7b53165c93af 100644
> --- a/drivers/hwtracing/coresight/coresight-core.c
> +++ b/drivers/hwtracing/coresight/coresight-core.c
> @@ -129,7 +129,8 @@ coresight_find_out_connection(struct coresight_device *csdev,
>
> static inline u32 coresight_read_claim_tags(struct csdev_access *csa)
> {
> - return csdev_access_relaxed_read32(csa, CORESIGHT_CLAIMCLR);
> + return FIELD_GET(CORESIGHT_CLAIM_MASK,
> + csdev_access_relaxed_read32(csa, CORESIGHT_CLAIMCLR));
> }
>
> static inline bool coresight_is_claimed_self_hosted(struct csdev_access *csa)
> diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
> index 05f891ca6b5c..cc7ff1e36ef4 100644
> --- a/drivers/hwtracing/coresight/coresight-priv.h
> +++ b/drivers/hwtracing/coresight/coresight-priv.h
> @@ -35,6 +35,7 @@ extern const struct device_type coresight_dev_type[];
> * Coresight device CLAIM protocol.
> * See PSCI - ARM DEN 0022D, Section: 6.8.1 Debug and Trace save and restore.
> */
> +#define CORESIGHT_CLAIM_MASK GENMASK(1, 0)
> #define CORESIGHT_CLAIM_SELF_HOSTED BIT(1)
>
> #define TIMEOUT_US 100
> --
> 2.34.1
>
> _______________________________________________
> CoreSight mailing list -- coresight@lists.linaro.org
> To unsubscribe send an email to coresight-leave@lists.linaro.org
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 4/7] coresight: Add claim tag warnings and debug messages
2025-02-11 10:39 [PATCH 0/7] coresight: Clear self hosted claim tag on probe James Clark
` (2 preceding siblings ...)
2025-02-11 10:39 ` [PATCH 3/7] coresight: Only check bottom two claim bits James Clark
@ 2025-02-11 10:39 ` James Clark
2025-03-13 14:40 ` Leo Yan
2025-02-11 10:39 ` [PATCH 5/7] coresight: Clear self hosted claim tag on probe James Clark
` (2 subsequent siblings)
6 siblings, 1 reply; 25+ messages in thread
From: James Clark @ 2025-02-11 10:39 UTC (permalink / raw)
To: lcherian, coresight
Cc: James Clark, Suzuki K Poulose, Mike Leach, Alexander Shishkin,
Maxime Coquelin, Alexandre Torgue, linux-arm-kernel, linux-kernel,
linux-stm32
Add a dev_dbg() message so that external debugger conflicts are more
visible. There are multiple reasons for -EBUSY so a message for this
particular one could be helpful. Add errors for and enumerate all the
other cases that are impossible.
Signed-off-by: James Clark <james.clark@linaro.org>
---
drivers/hwtracing/coresight/coresight-core.c | 48 ++++++++++++--------
drivers/hwtracing/coresight/coresight-priv.h | 5 +-
2 files changed, 34 insertions(+), 19 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index 7b53165c93af..7fe5d5d432c4 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -133,16 +133,6 @@ static inline u32 coresight_read_claim_tags(struct csdev_access *csa)
csdev_access_relaxed_read32(csa, CORESIGHT_CLAIMCLR));
}
-static inline bool coresight_is_claimed_self_hosted(struct csdev_access *csa)
-{
- return coresight_read_claim_tags(csa) == CORESIGHT_CLAIM_SELF_HOSTED;
-}
-
-static inline bool coresight_is_claimed_any(struct coresight_device *csdev)
-{
- return coresight_read_claim_tags(&csdev->access) != 0;
-}
-
static inline void coresight_set_self_claim_tag(struct csdev_access *csa)
{
csdev_access_relaxed_write32(csa, CORESIGHT_CLAIM_SELF_HOSTED,
@@ -169,18 +159,40 @@ static inline void coresight_clear_self_claim_tag(struct csdev_access *csa)
*/
int coresight_claim_device_unlocked(struct coresight_device *csdev)
{
+ int tag;
+ struct csdev_access *csa;
+
if (WARN_ON(!csdev))
return -EINVAL;
- if (coresight_is_claimed_any(csdev))
+ csa = &csdev->access;
+ tag = coresight_read_claim_tags(csa);
+
+ switch (tag) {
+ case CORESIGHT_CLAIM_FREE:
+ coresight_set_self_claim_tag(csa);
+ if (coresight_read_claim_tags(csa) == CORESIGHT_CLAIM_SELF_HOSTED)
+ return 0;
+
+ /* There was a race setting the tag, clean up and fail */
+ coresight_clear_self_claim_tag(csa);
return -EBUSY;
- coresight_set_self_claim_tag(&csdev->access);
- if (coresight_is_claimed_self_hosted(&csdev->access))
- return 0;
- /* There was a race setting the tag, clean up and fail */
- coresight_clear_self_claim_tag(&csdev->access);
- return -EBUSY;
+ case CORESIGHT_CLAIM_EXTERNAL:
+ /* External debug is an expected state, so log and report BUSY */
+ dev_dbg(&csdev->dev, "Busy: Claimed by external debugger");
+ return -EBUSY;
+
+ default:
+ case CORESIGHT_CLAIM_SELF_HOSTED:
+ case CORESIGHT_CLAIM_INVALID:
+ /*
+ * Warn here because we clear a lingering self hosted tag
+ * on probe, so other tag combinations are impossible.
+ */
+ dev_err_once(&csdev->dev, "Invalid claim tag state: %x", tag);
+ return -EBUSY;
+ }
}
EXPORT_SYMBOL_GPL(coresight_claim_device_unlocked);
@@ -205,7 +217,7 @@ EXPORT_SYMBOL_GPL(coresight_claim_device);
*/
void coresight_disclaim_device_unlocked(struct csdev_access *csa)
{
- if (coresight_is_claimed_self_hosted(csa))
+ if (coresight_read_claim_tags(csa) == CORESIGHT_CLAIM_SELF_HOSTED)
coresight_clear_self_claim_tag(csa);
else
/*
diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
index cc7ff1e36ef4..a83113225797 100644
--- a/drivers/hwtracing/coresight/coresight-priv.h
+++ b/drivers/hwtracing/coresight/coresight-priv.h
@@ -36,7 +36,10 @@ extern const struct device_type coresight_dev_type[];
* See PSCI - ARM DEN 0022D, Section: 6.8.1 Debug and Trace save and restore.
*/
#define CORESIGHT_CLAIM_MASK GENMASK(1, 0)
-#define CORESIGHT_CLAIM_SELF_HOSTED BIT(1)
+#define CORESIGHT_CLAIM_FREE 0
+#define CORESIGHT_CLAIM_EXTERNAL 1
+#define CORESIGHT_CLAIM_SELF_HOSTED 2
+#define CORESIGHT_CLAIM_INVALID 3
#define TIMEOUT_US 100
#define BMVAL(val, lsb, msb) ((val & GENMASK(msb, lsb)) >> lsb)
--
2.34.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 4/7] coresight: Add claim tag warnings and debug messages
2025-02-11 10:39 ` [PATCH 4/7] coresight: Add claim tag warnings and debug messages James Clark
@ 2025-03-13 14:40 ` Leo Yan
2025-03-17 11:56 ` James Clark
0 siblings, 1 reply; 25+ messages in thread
From: Leo Yan @ 2025-03-13 14:40 UTC (permalink / raw)
To: James Clark
Cc: lcherian, coresight, Mike Leach, Alexander Shishkin,
Maxime Coquelin, Alexandre Torgue, linux-arm-kernel, linux-kernel,
linux-stm32
On Tue, Feb 11, 2025 at 10:39:40AM +0000, James Clark wrote:
>
> Add a dev_dbg() message so that external debugger conflicts are more
> visible. There are multiple reasons for -EBUSY so a message for this
> particular one could be helpful. Add errors for and enumerate all the
> other cases that are impossible.
>
> Signed-off-by: James Clark <james.clark@linaro.org>
> ---
> drivers/hwtracing/coresight/coresight-core.c | 48 ++++++++++++--------
> drivers/hwtracing/coresight/coresight-priv.h | 5 +-
> 2 files changed, 34 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> index 7b53165c93af..7fe5d5d432c4 100644
> --- a/drivers/hwtracing/coresight/coresight-core.c
> +++ b/drivers/hwtracing/coresight/coresight-core.c
> @@ -133,16 +133,6 @@ static inline u32 coresight_read_claim_tags(struct csdev_access *csa)
> csdev_access_relaxed_read32(csa, CORESIGHT_CLAIMCLR));
> }
>
> -static inline bool coresight_is_claimed_self_hosted(struct csdev_access *csa)
> -{
> - return coresight_read_claim_tags(csa) == CORESIGHT_CLAIM_SELF_HOSTED;
> -}
> -
> -static inline bool coresight_is_claimed_any(struct coresight_device *csdev)
> -{
> - return coresight_read_claim_tags(&csdev->access) != 0;
> -}
> -
> static inline void coresight_set_self_claim_tag(struct csdev_access *csa)
> {
> csdev_access_relaxed_write32(csa, CORESIGHT_CLAIM_SELF_HOSTED,
> @@ -169,18 +159,40 @@ static inline void coresight_clear_self_claim_tag(struct csdev_access *csa)
> */
> int coresight_claim_device_unlocked(struct coresight_device *csdev)
> {
> + int tag;
> + struct csdev_access *csa;
> +
> if (WARN_ON(!csdev))
> return -EINVAL;
>
> - if (coresight_is_claimed_any(csdev))
> + csa = &csdev->access;
> + tag = coresight_read_claim_tags(csa);
> +
> + switch (tag) {
> + case CORESIGHT_CLAIM_FREE:
> + coresight_set_self_claim_tag(csa);
> + if (coresight_read_claim_tags(csa) == CORESIGHT_CLAIM_SELF_HOSTED)
> + return 0;
> +
It would be a rare case if a failure happens here. Seems to me, it
would be valuable to print a log for this edge case.
Otherwise, looks good to me.
> + /* There was a race setting the tag, clean up and fail */
> + coresight_clear_self_claim_tag(csa);
> return -EBUSY;
>
> - coresight_set_self_claim_tag(&csdev->access);
> - if (coresight_is_claimed_self_hosted(&csdev->access))
> - return 0;
> - /* There was a race setting the tag, clean up and fail */
> - coresight_clear_self_claim_tag(&csdev->access);
> - return -EBUSY;
> + case CORESIGHT_CLAIM_EXTERNAL:
> + /* External debug is an expected state, so log and report BUSY */
> + dev_dbg(&csdev->dev, "Busy: Claimed by external debugger");
> + return -EBUSY;
> +
> + default:
> + case CORESIGHT_CLAIM_SELF_HOSTED:
> + case CORESIGHT_CLAIM_INVALID:
> + /*
> + * Warn here because we clear a lingering self hosted tag
> + * on probe, so other tag combinations are impossible.
> + */
> + dev_err_once(&csdev->dev, "Invalid claim tag state: %x", tag);
> + return -EBUSY;
> + }
> }
> EXPORT_SYMBOL_GPL(coresight_claim_device_unlocked);
>
> @@ -205,7 +217,7 @@ EXPORT_SYMBOL_GPL(coresight_claim_device);
> */
> void coresight_disclaim_device_unlocked(struct csdev_access *csa)
> {
> - if (coresight_is_claimed_self_hosted(csa))
> + if (coresight_read_claim_tags(csa) == CORESIGHT_CLAIM_SELF_HOSTED)
> coresight_clear_self_claim_tag(csa);
> else
> /*
> diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
> index cc7ff1e36ef4..a83113225797 100644
> --- a/drivers/hwtracing/coresight/coresight-priv.h
> +++ b/drivers/hwtracing/coresight/coresight-priv.h
> @@ -36,7 +36,10 @@ extern const struct device_type coresight_dev_type[];
> * See PSCI - ARM DEN 0022D, Section: 6.8.1 Debug and Trace save and restore.
> */
> #define CORESIGHT_CLAIM_MASK GENMASK(1, 0)
> -#define CORESIGHT_CLAIM_SELF_HOSTED BIT(1)
> +#define CORESIGHT_CLAIM_FREE 0
> +#define CORESIGHT_CLAIM_EXTERNAL 1
> +#define CORESIGHT_CLAIM_SELF_HOSTED 2
> +#define CORESIGHT_CLAIM_INVALID 3
>
> #define TIMEOUT_US 100
> #define BMVAL(val, lsb, msb) ((val & GENMASK(msb, lsb)) >> lsb)
> --
> 2.34.1
>
> _______________________________________________
> CoreSight mailing list -- coresight@lists.linaro.org
> To unsubscribe send an email to coresight-leave@lists.linaro.org
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 4/7] coresight: Add claim tag warnings and debug messages
2025-03-13 14:40 ` Leo Yan
@ 2025-03-17 11:56 ` James Clark
0 siblings, 0 replies; 25+ messages in thread
From: James Clark @ 2025-03-17 11:56 UTC (permalink / raw)
To: Leo Yan
Cc: lcherian, coresight, Mike Leach, Alexander Shishkin,
Maxime Coquelin, Alexandre Torgue, linux-arm-kernel, linux-kernel,
linux-stm32
On 13/03/2025 2:40 pm, Leo Yan wrote:
> On Tue, Feb 11, 2025 at 10:39:40AM +0000, James Clark wrote:
>>
>> Add a dev_dbg() message so that external debugger conflicts are more
>> visible. There are multiple reasons for -EBUSY so a message for this
>> particular one could be helpful. Add errors for and enumerate all the
>> other cases that are impossible.
>>
>> Signed-off-by: James Clark <james.clark@linaro.org>
>> ---
>> drivers/hwtracing/coresight/coresight-core.c | 48 ++++++++++++--------
>> drivers/hwtracing/coresight/coresight-priv.h | 5 +-
>> 2 files changed, 34 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
>> index 7b53165c93af..7fe5d5d432c4 100644
>> --- a/drivers/hwtracing/coresight/coresight-core.c
>> +++ b/drivers/hwtracing/coresight/coresight-core.c
>> @@ -133,16 +133,6 @@ static inline u32 coresight_read_claim_tags(struct csdev_access *csa)
>> csdev_access_relaxed_read32(csa, CORESIGHT_CLAIMCLR));
>> }
>>
>> -static inline bool coresight_is_claimed_self_hosted(struct csdev_access *csa)
>> -{
>> - return coresight_read_claim_tags(csa) == CORESIGHT_CLAIM_SELF_HOSTED;
>> -}
>> -
>> -static inline bool coresight_is_claimed_any(struct coresight_device *csdev)
>> -{
>> - return coresight_read_claim_tags(&csdev->access) != 0;
>> -}
>> -
>> static inline void coresight_set_self_claim_tag(struct csdev_access *csa)
>> {
>> csdev_access_relaxed_write32(csa, CORESIGHT_CLAIM_SELF_HOSTED,
>> @@ -169,18 +159,40 @@ static inline void coresight_clear_self_claim_tag(struct csdev_access *csa)
>> */
>> int coresight_claim_device_unlocked(struct coresight_device *csdev)
>> {
>> + int tag;
>> + struct csdev_access *csa;
>> +
>> if (WARN_ON(!csdev))
>> return -EINVAL;
>>
>> - if (coresight_is_claimed_any(csdev))
>> + csa = &csdev->access;
>> + tag = coresight_read_claim_tags(csa);
>> +
>> + switch (tag) {
>> + case CORESIGHT_CLAIM_FREE:
>> + coresight_set_self_claim_tag(csa);
>> + if (coresight_read_claim_tags(csa) == CORESIGHT_CLAIM_SELF_HOSTED)
>> + return 0;
>> +
>
> It would be a rare case if a failure happens here. Seems to me, it
> would be valuable to print a log for this edge case.
>
> Otherwise, looks good to me.
>
Yeah I can add dev_dbg("Busy: Couldn't set self claim tag"). Just to
distinguish this race case from the other busy case.
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 5/7] coresight: Clear self hosted claim tag on probe
2025-02-11 10:39 [PATCH 0/7] coresight: Clear self hosted claim tag on probe James Clark
` (3 preceding siblings ...)
2025-02-11 10:39 ` [PATCH 4/7] coresight: Add claim tag warnings and debug messages James Clark
@ 2025-02-11 10:39 ` James Clark
2025-02-12 18:24 ` Mike Leach
2025-03-13 16:04 ` Leo Yan
2025-02-11 10:39 ` [PATCH 6/7] coresight: Remove inlines from static function definitions James Clark
2025-02-11 10:39 ` [PATCH 7/7] coresight: Remove extern from function declarations James Clark
6 siblings, 2 replies; 25+ messages in thread
From: James Clark @ 2025-02-11 10:39 UTC (permalink / raw)
To: lcherian, coresight
Cc: James Clark, Suzuki K Poulose, Mike Leach, Alexander Shishkin,
Maxime Coquelin, Alexandre Torgue, linux-arm-kernel, linux-kernel,
linux-stm32
This can be left behind from a crashed kernel after a kexec so clear it
when probing each device. Similarly to
coresight_disclaim_device_unlocked(), only clear it if it's already set
to avoid races with an external debugger.
We need a csdev_access struct in etm_init_arch_data() so just replace
the iomem pointer with a full csdev_access struct. This means all usages
need to be updated to go through csa->base.
Signed-off-by: James Clark <james.clark@linaro.org>
---
drivers/hwtracing/coresight/coresight-catu.c | 1 +
drivers/hwtracing/coresight/coresight-core.c | 48 +++++++++++++++----
.../hwtracing/coresight/coresight-cti-core.c | 2 +
drivers/hwtracing/coresight/coresight-etb10.c | 2 +
drivers/hwtracing/coresight/coresight-etm.h | 6 +--
.../coresight/coresight-etm3x-core.c | 28 +++++------
.../coresight/coresight-etm3x-sysfs.c | 8 ++--
.../coresight/coresight-etm4x-core.c | 2 +
.../hwtracing/coresight/coresight-funnel.c | 2 +
.../coresight/coresight-replicator.c | 1 +
.../hwtracing/coresight/coresight-tmc-core.c | 1 +
include/linux/coresight.h | 3 ++
12 files changed, 73 insertions(+), 31 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-catu.c b/drivers/hwtracing/coresight/coresight-catu.c
index d9259c0b6e64..575c2d247a90 100644
--- a/drivers/hwtracing/coresight/coresight-catu.c
+++ b/drivers/hwtracing/coresight/coresight-catu.c
@@ -558,6 +558,7 @@ static int __catu_probe(struct device *dev, struct resource *res)
catu_desc.subtype.helper_subtype = CORESIGHT_DEV_SUBTYPE_HELPER_CATU;
catu_desc.ops = &catu_ops;
+ coresight_reset_claim(&catu_desc.access);
drvdata->csdev = coresight_register(&catu_desc);
if (IS_ERR(drvdata->csdev))
ret = PTR_ERR(drvdata->csdev);
diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index 7fe5d5d432c4..97f33ffad05e 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -212,20 +212,48 @@ int coresight_claim_device(struct coresight_device *csdev)
EXPORT_SYMBOL_GPL(coresight_claim_device);
/*
- * coresight_disclaim_device_unlocked : Clear the claim tag for the device.
+ * Clear the claim tag for the device.
+ * Returns an error if the device wasn't already claimed.
+ */
+int coresight_reset_claim(struct csdev_access *csa)
+{
+ int ret;
+
+ CS_UNLOCK(csa->base);
+ ret = coresight_reset_claim_unlocked(csa);
+ CS_LOCK(csa->base);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(coresight_reset_claim);
+
+/*
+ * Clear the claim tag for the device. Called with CS_UNLOCKed for the component.
+ * Returns an error if the device wasn't already claimed.
+ */
+int coresight_reset_claim_unlocked(struct csdev_access *csa)
+{
+ if (coresight_read_claim_tags(csa) == CORESIGHT_CLAIM_SELF_HOSTED) {
+ coresight_clear_self_claim_tag(csa);
+ return 0;
+ }
+
+ return -EINVAL;
+}
+EXPORT_SYMBOL_GPL(coresight_reset_claim_unlocked);
+
+/*
+ * coresight_disclaim_device_unlocked : Clear the claim tag for the device
+ * and warn if the device wasn't already claimed.
* Called with CS_UNLOCKed for the component.
*/
void coresight_disclaim_device_unlocked(struct csdev_access *csa)
{
- if (coresight_read_claim_tags(csa) == CORESIGHT_CLAIM_SELF_HOSTED)
- coresight_clear_self_claim_tag(csa);
- else
- /*
- * The external agent may have not honoured our claim
- * and has manipulated it. Or something else has seriously
- * gone wrong in our driver.
- */
- WARN_ON_ONCE(1);
+ /*
+ * Warn if the external agent hasn't honoured our claim
+ * and has manipulated it. Or something else has seriously
+ * gone wrong in our driver.
+ */
+ WARN_ON_ONCE(coresight_reset_claim_unlocked(csa));
}
EXPORT_SYMBOL_GPL(coresight_disclaim_device_unlocked);
diff --git a/drivers/hwtracing/coresight/coresight-cti-core.c b/drivers/hwtracing/coresight/coresight-cti-core.c
index 073f67a41af9..389a72362f0c 100644
--- a/drivers/hwtracing/coresight/coresight-cti-core.c
+++ b/drivers/hwtracing/coresight/coresight-cti-core.c
@@ -931,6 +931,8 @@ static int cti_probe(struct amba_device *adev, const struct amba_id *id)
cti_desc.ops = &cti_ops;
cti_desc.groups = drvdata->ctidev.con_groups;
cti_desc.dev = dev;
+
+ coresight_reset_claim(&cti_desc.access);
drvdata->csdev = coresight_register(&cti_desc);
if (IS_ERR(drvdata->csdev)) {
ret = PTR_ERR(drvdata->csdev);
diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c
index d8bc3e776c88..b598b2c0c9bb 100644
--- a/drivers/hwtracing/coresight/coresight-etb10.c
+++ b/drivers/hwtracing/coresight/coresight-etb10.c
@@ -772,6 +772,8 @@ static int etb_probe(struct amba_device *adev, const struct amba_id *id)
desc.pdata = pdata;
desc.dev = dev;
desc.groups = coresight_etb_groups;
+
+ coresight_reset_claim(&desc.access);
drvdata->csdev = coresight_register(&desc);
if (IS_ERR(drvdata->csdev))
return PTR_ERR(drvdata->csdev);
diff --git a/drivers/hwtracing/coresight/coresight-etm.h b/drivers/hwtracing/coresight/coresight-etm.h
index e02c3ea972c9..a89736309c27 100644
--- a/drivers/hwtracing/coresight/coresight-etm.h
+++ b/drivers/hwtracing/coresight/coresight-etm.h
@@ -229,7 +229,7 @@ struct etm_config {
* @config: structure holding configuration parameters.
*/
struct etm_drvdata {
- void __iomem *base;
+ struct csdev_access csa;
struct clk *atclk;
struct coresight_device *csdev;
spinlock_t spinlock;
@@ -260,7 +260,7 @@ static inline void etm_writel(struct etm_drvdata *drvdata,
"invalid CP14 access to ETM reg: %#x", off);
}
} else {
- writel_relaxed(val, drvdata->base + off);
+ writel_relaxed(val, drvdata->csa.base + off);
}
}
@@ -274,7 +274,7 @@ static inline unsigned int etm_readl(struct etm_drvdata *drvdata, u32 off)
"invalid CP14 access to ETM reg: %#x", off);
}
} else {
- val = readl_relaxed(drvdata->base + off);
+ val = readl_relaxed(drvdata->csa.base + off);
}
return val;
diff --git a/drivers/hwtracing/coresight/coresight-etm3x-core.c b/drivers/hwtracing/coresight/coresight-etm3x-core.c
index 509f53b69e42..0b010683b883 100644
--- a/drivers/hwtracing/coresight/coresight-etm3x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm3x-core.c
@@ -86,9 +86,9 @@ static void etm_set_pwrup(struct etm_drvdata *drvdata)
{
u32 etmpdcr;
- etmpdcr = readl_relaxed(drvdata->base + ETMPDCR);
+ etmpdcr = readl_relaxed(drvdata->csa.base + ETMPDCR);
etmpdcr |= ETMPDCR_PWD_UP;
- writel_relaxed(etmpdcr, drvdata->base + ETMPDCR);
+ writel_relaxed(etmpdcr, drvdata->csa.base + ETMPDCR);
/* Ensure pwrup completes before subsequent cp14 accesses */
mb();
isb();
@@ -101,9 +101,9 @@ static void etm_clr_pwrup(struct etm_drvdata *drvdata)
/* Ensure pending cp14 accesses complete before clearing pwrup */
mb();
isb();
- etmpdcr = readl_relaxed(drvdata->base + ETMPDCR);
+ etmpdcr = readl_relaxed(drvdata->csa.base + ETMPDCR);
etmpdcr &= ~ETMPDCR_PWD_UP;
- writel_relaxed(etmpdcr, drvdata->base + ETMPDCR);
+ writel_relaxed(etmpdcr, drvdata->csa.base + ETMPDCR);
}
/**
@@ -365,7 +365,7 @@ static int etm_enable_hw(struct etm_drvdata *drvdata)
struct etm_config *config = &drvdata->config;
struct coresight_device *csdev = drvdata->csdev;
- CS_UNLOCK(drvdata->base);
+ CS_UNLOCK(drvdata->csa.base);
rc = coresight_claim_device_unlocked(csdev);
if (rc)
@@ -427,7 +427,7 @@ static int etm_enable_hw(struct etm_drvdata *drvdata)
etm_clr_prog(drvdata);
done:
- CS_LOCK(drvdata->base);
+ CS_LOCK(drvdata->csa.base);
dev_dbg(&drvdata->csdev->dev, "cpu: %d enable smp call done: %d\n",
drvdata->cpu, rc);
@@ -589,7 +589,7 @@ static void etm_disable_hw(void *info)
struct etm_config *config = &drvdata->config;
struct coresight_device *csdev = drvdata->csdev;
- CS_UNLOCK(drvdata->base);
+ CS_UNLOCK(drvdata->csa.base);
etm_set_prog(drvdata);
/* Read back sequencer and counters for post trace analysis */
@@ -601,7 +601,7 @@ static void etm_disable_hw(void *info)
etm_set_pwrdwn(drvdata);
coresight_disclaim_device_unlocked(&csdev->access);
- CS_LOCK(drvdata->base);
+ CS_LOCK(drvdata->csa.base);
dev_dbg(&drvdata->csdev->dev,
"cpu: %d disable smp call done\n", drvdata->cpu);
@@ -614,7 +614,7 @@ static void etm_disable_perf(struct coresight_device *csdev)
if (WARN_ON_ONCE(drvdata->cpu != smp_processor_id()))
return;
- CS_UNLOCK(drvdata->base);
+ CS_UNLOCK(drvdata->csa.base);
/* Setting the prog bit disables tracing immediately */
etm_set_prog(drvdata);
@@ -626,7 +626,7 @@ static void etm_disable_perf(struct coresight_device *csdev)
etm_set_pwrdwn(drvdata);
coresight_disclaim_device_unlocked(&csdev->access);
- CS_LOCK(drvdata->base);
+ CS_LOCK(drvdata->csa.base);
/*
* perf will release trace ids when _free_aux()
@@ -772,7 +772,7 @@ static void etm_init_arch_data(void *info)
/* Make sure all registers are accessible */
etm_os_unlock(drvdata);
- CS_UNLOCK(drvdata->base);
+ CS_UNLOCK(drvdata->csa.base);
/* First dummy read */
(void)etm_readl(drvdata, ETMPDSR);
@@ -803,9 +803,10 @@ static void etm_init_arch_data(void *info)
drvdata->nr_ext_out = BMVAL(etmccr, 20, 22);
drvdata->nr_ctxid_cmp = BMVAL(etmccr, 24, 25);
+ coresight_reset_claim_unlocked(&drvdata->csa);
etm_set_pwrdwn(drvdata);
etm_clr_pwrup(drvdata);
- CS_LOCK(drvdata->base);
+ CS_LOCK(drvdata->csa.base);
}
static int __init etm_hp_setup(void)
@@ -866,8 +867,7 @@ static int etm_probe(struct amba_device *adev, const struct amba_id *id)
if (IS_ERR(base))
return PTR_ERR(base);
- drvdata->base = base;
- desc.access = CSDEV_ACCESS_IOMEM(base);
+ desc.access = drvdata->csa = CSDEV_ACCESS_IOMEM(base);
spin_lock_init(&drvdata->spinlock);
diff --git a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
index 68c644be9813..af566b62785c 100644
--- a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
+++ b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
@@ -50,11 +50,11 @@ static ssize_t etmsr_show(struct device *dev,
pm_runtime_get_sync(dev->parent);
spin_lock_irqsave(&drvdata->spinlock, flags);
- CS_UNLOCK(drvdata->base);
+ CS_UNLOCK(drvdata->csa.base);
val = etm_readl(drvdata, ETMSR);
- CS_LOCK(drvdata->base);
+ CS_LOCK(drvdata->csa.base);
spin_unlock_irqrestore(&drvdata->spinlock, flags);
pm_runtime_put(dev->parent);
@@ -949,9 +949,9 @@ static ssize_t seq_curr_state_show(struct device *dev,
pm_runtime_get_sync(dev->parent);
spin_lock_irqsave(&drvdata->spinlock, flags);
- CS_UNLOCK(drvdata->base);
+ CS_UNLOCK(drvdata->csa.base);
val = (etm_readl(drvdata, ETMSQR) & ETM_SQR_MASK);
- CS_LOCK(drvdata->base);
+ CS_LOCK(drvdata->csa.base);
spin_unlock_irqrestore(&drvdata->spinlock, flags);
pm_runtime_put(dev->parent);
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index 45b30a4b3eba..3609bb74c4ec 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -1337,6 +1337,8 @@ static void etm4_init_arch_data(void *info)
drvdata->nrseqstate = FIELD_GET(TRCIDR5_NUMSEQSTATE_MASK, etmidr5);
/* NUMCNTR, bits[30:28] number of counters available for tracing */
drvdata->nr_cntr = FIELD_GET(TRCIDR5_NUMCNTR_MASK, etmidr5);
+
+ coresight_reset_claim_unlocked(csa);
etm4_cs_lock(drvdata, csa);
cpu_detect_trace_filtering(drvdata);
}
diff --git a/drivers/hwtracing/coresight/coresight-funnel.c b/drivers/hwtracing/coresight/coresight-funnel.c
index e986922d555b..be2a47636ef3 100644
--- a/drivers/hwtracing/coresight/coresight-funnel.c
+++ b/drivers/hwtracing/coresight/coresight-funnel.c
@@ -255,6 +255,7 @@ static int funnel_probe(struct device *dev, struct resource *res)
drvdata->base = base;
desc.groups = coresight_funnel_groups;
desc.access = CSDEV_ACCESS_IOMEM(base);
+ coresight_reset_claim(&desc.access);
}
dev_set_drvdata(dev, drvdata);
@@ -272,6 +273,7 @@ static int funnel_probe(struct device *dev, struct resource *res)
desc.ops = &funnel_cs_ops;
desc.pdata = pdata;
desc.dev = dev;
+
drvdata->csdev = coresight_register(&desc);
if (IS_ERR(drvdata->csdev)) {
ret = PTR_ERR(drvdata->csdev);
diff --git a/drivers/hwtracing/coresight/coresight-replicator.c b/drivers/hwtracing/coresight/coresight-replicator.c
index 5d42a9a8c460..679a36effbe8 100644
--- a/drivers/hwtracing/coresight/coresight-replicator.c
+++ b/drivers/hwtracing/coresight/coresight-replicator.c
@@ -284,6 +284,7 @@ static int replicator_probe(struct device *dev, struct resource *res)
desc.pdata = dev->platform_data;
desc.dev = dev;
+ coresight_reset_claim(&desc.access);
drvdata->csdev = coresight_register(&desc);
if (IS_ERR(drvdata->csdev)) {
ret = PTR_ERR(drvdata->csdev);
diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c
index e9876252a789..5ac4e3c706ac 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-core.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-core.c
@@ -558,6 +558,7 @@ static int __tmc_probe(struct device *dev, struct resource *res)
dev->platform_data = pdata;
desc.pdata = pdata;
+ coresight_reset_claim(&desc.access);
drvdata->csdev = coresight_register(&desc);
if (IS_ERR(drvdata->csdev)) {
ret = PTR_ERR(drvdata->csdev);
diff --git a/include/linux/coresight.h b/include/linux/coresight.h
index 937931d107e0..11808aee9d1d 100644
--- a/include/linux/coresight.h
+++ b/include/linux/coresight.h
@@ -655,6 +655,9 @@ extern int coresight_claim_device_unlocked(struct coresight_device *csdev);
extern void coresight_disclaim_device(struct csdev_access *csa);
extern void coresight_disclaim_device_unlocked(struct csdev_access *csa);
+int coresight_reset_claim_unlocked(struct csdev_access *csa);
+int coresight_reset_claim(struct csdev_access *csa);
+
extern char *coresight_alloc_device_name(struct coresight_dev_list *devs,
struct device *dev);
--
2.34.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 5/7] coresight: Clear self hosted claim tag on probe
2025-02-11 10:39 ` [PATCH 5/7] coresight: Clear self hosted claim tag on probe James Clark
@ 2025-02-12 18:24 ` Mike Leach
2025-02-13 13:20 ` James Clark
2025-03-13 16:04 ` Leo Yan
1 sibling, 1 reply; 25+ messages in thread
From: Mike Leach @ 2025-02-12 18:24 UTC (permalink / raw)
To: James Clark
Cc: lcherian, coresight, Suzuki K Poulose, Alexander Shishkin,
Maxime Coquelin, Alexandre Torgue, linux-arm-kernel, linux-kernel,
linux-stm32
Hi James,
On Tue, 11 Feb 2025 at 10:40, James Clark <james.clark@linaro.org> wrote:
>
> This can be left behind from a crashed kernel after a kexec so clear it
> when probing each device. Similarly to
> coresight_disclaim_device_unlocked(), only clear it if it's already set
> to avoid races with an external debugger.
>
> We need a csdev_access struct in etm_init_arch_data() so just replace
> the iomem pointer with a full csdev_access struct. This means all usages
> need to be updated to go through csa->base.
>
> Signed-off-by: James Clark <james.clark@linaro.org>
> ---
> drivers/hwtracing/coresight/coresight-catu.c | 1 +
> drivers/hwtracing/coresight/coresight-core.c | 48 +++++++++++++++----
> .../hwtracing/coresight/coresight-cti-core.c | 2 +
> drivers/hwtracing/coresight/coresight-etb10.c | 2 +
> drivers/hwtracing/coresight/coresight-etm.h | 6 +--
> .../coresight/coresight-etm3x-core.c | 28 +++++------
> .../coresight/coresight-etm3x-sysfs.c | 8 ++--
> .../coresight/coresight-etm4x-core.c | 2 +
> .../hwtracing/coresight/coresight-funnel.c | 2 +
> .../coresight/coresight-replicator.c | 1 +
> .../hwtracing/coresight/coresight-tmc-core.c | 1 +
> include/linux/coresight.h | 3 ++
> 12 files changed, 73 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-catu.c b/drivers/hwtracing/coresight/coresight-catu.c
> index d9259c0b6e64..575c2d247a90 100644
> --- a/drivers/hwtracing/coresight/coresight-catu.c
> +++ b/drivers/hwtracing/coresight/coresight-catu.c
> @@ -558,6 +558,7 @@ static int __catu_probe(struct device *dev, struct resource *res)
> catu_desc.subtype.helper_subtype = CORESIGHT_DEV_SUBTYPE_HELPER_CATU;
> catu_desc.ops = &catu_ops;
>
> + coresight_reset_claim(&catu_desc.access);
> drvdata->csdev = coresight_register(&catu_desc);
> if (IS_ERR(drvdata->csdev))
> ret = PTR_ERR(drvdata->csdev);
> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> index 7fe5d5d432c4..97f33ffad05e 100644
> --- a/drivers/hwtracing/coresight/coresight-core.c
> +++ b/drivers/hwtracing/coresight/coresight-core.c
> @@ -212,20 +212,48 @@ int coresight_claim_device(struct coresight_device *csdev)
> EXPORT_SYMBOL_GPL(coresight_claim_device);
>
> /*
> - * coresight_disclaim_device_unlocked : Clear the claim tag for the device.
> + * Clear the claim tag for the device.
> + * Returns an error if the device wasn't already claimed.
> + */
> +int coresight_reset_claim(struct csdev_access *csa)
Given the iusue being fixed - i.e. previous sessions having residual
claims - and the fact that this funtion is only ever called from
device initialisation as far as I can tell,
would this be better to be called coresight_init_claim() and return /
log an error only if the device _was_ actually claimed.
As it stands it seems to return an error if the device was not claimed
- which on initialisation is the correct state!
Since the retrun code is ignored by all of the callees, the rerun
could be dropped and a warning issued withing this function if the
claim needed clearing.
> +{
> + int ret;
> +
> + CS_UNLOCK(csa->base);
> + ret = coresight_reset_claim_unlocked(csa);
> + CS_LOCK(csa->base);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(coresight_reset_claim);
> +
> +/*
> + * Clear the claim tag for the device. Called with CS_UNLOCKed for the component.
> + * Returns an error if the device wasn't already claimed.
> + */
> +int coresight_reset_claim_unlocked(struct csdev_access *csa)
Given the comment and the function - this could be named
"coresight_clear_claim_unlocked()"
Regards
Mike
> +{
> + if (coresight_read_claim_tags(csa) == CORESIGHT_CLAIM_SELF_HOSTED) {
> + coresight_clear_self_claim_tag(csa);
> + return 0;
> + }
> +
> + return -EINVAL;
> +}
> +EXPORT_SYMBOL_GPL(coresight_reset_claim_unlocked);
> +
> +/*
> + * coresight_disclaim_device_unlocked : Clear the claim tag for the device
> + * and warn if the device wasn't already claimed.
> * Called with CS_UNLOCKed for the component.
> */
> void coresight_disclaim_device_unlocked(struct csdev_access *csa)
> {
> - if (coresight_read_claim_tags(csa) == CORESIGHT_CLAIM_SELF_HOSTED)
> - coresight_clear_self_claim_tag(csa);
> - else
> - /*
> - * The external agent may have not honoured our claim
> - * and has manipulated it. Or something else has seriously
> - * gone wrong in our driver.
> - */
> - WARN_ON_ONCE(1);
> + /*
> + * Warn if the external agent hasn't honoured our claim
> + * and has manipulated it. Or something else has seriously
> + * gone wrong in our driver.
> + */
> + WARN_ON_ONCE(coresight_reset_claim_unlocked(csa));
> }
> EXPORT_SYMBOL_GPL(coresight_disclaim_device_unlocked);
>
> diff --git a/drivers/hwtracing/coresight/coresight-cti-core.c b/drivers/hwtracing/coresight/coresight-cti-core.c
> index 073f67a41af9..389a72362f0c 100644
> --- a/drivers/hwtracing/coresight/coresight-cti-core.c
> +++ b/drivers/hwtracing/coresight/coresight-cti-core.c
> @@ -931,6 +931,8 @@ static int cti_probe(struct amba_device *adev, const struct amba_id *id)
> cti_desc.ops = &cti_ops;
> cti_desc.groups = drvdata->ctidev.con_groups;
> cti_desc.dev = dev;
> +
> + coresight_reset_claim(&cti_desc.access);
> drvdata->csdev = coresight_register(&cti_desc);
> if (IS_ERR(drvdata->csdev)) {
> ret = PTR_ERR(drvdata->csdev);
> diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c
> index d8bc3e776c88..b598b2c0c9bb 100644
> --- a/drivers/hwtracing/coresight/coresight-etb10.c
> +++ b/drivers/hwtracing/coresight/coresight-etb10.c
> @@ -772,6 +772,8 @@ static int etb_probe(struct amba_device *adev, const struct amba_id *id)
> desc.pdata = pdata;
> desc.dev = dev;
> desc.groups = coresight_etb_groups;
> +
> + coresight_reset_claim(&desc.access);
> drvdata->csdev = coresight_register(&desc);
> if (IS_ERR(drvdata->csdev))
> return PTR_ERR(drvdata->csdev);
> diff --git a/drivers/hwtracing/coresight/coresight-etm.h b/drivers/hwtracing/coresight/coresight-etm.h
> index e02c3ea972c9..a89736309c27 100644
> --- a/drivers/hwtracing/coresight/coresight-etm.h
> +++ b/drivers/hwtracing/coresight/coresight-etm.h
> @@ -229,7 +229,7 @@ struct etm_config {
> * @config: structure holding configuration parameters.
> */
> struct etm_drvdata {
> - void __iomem *base;
> + struct csdev_access csa;
> struct clk *atclk;
> struct coresight_device *csdev;
> spinlock_t spinlock;
> @@ -260,7 +260,7 @@ static inline void etm_writel(struct etm_drvdata *drvdata,
> "invalid CP14 access to ETM reg: %#x", off);
> }
> } else {
> - writel_relaxed(val, drvdata->base + off);
> + writel_relaxed(val, drvdata->csa.base + off);
> }
> }
>
> @@ -274,7 +274,7 @@ static inline unsigned int etm_readl(struct etm_drvdata *drvdata, u32 off)
> "invalid CP14 access to ETM reg: %#x", off);
> }
> } else {
> - val = readl_relaxed(drvdata->base + off);
> + val = readl_relaxed(drvdata->csa.base + off);
> }
>
> return val;
> diff --git a/drivers/hwtracing/coresight/coresight-etm3x-core.c b/drivers/hwtracing/coresight/coresight-etm3x-core.c
> index 509f53b69e42..0b010683b883 100644
> --- a/drivers/hwtracing/coresight/coresight-etm3x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm3x-core.c
> @@ -86,9 +86,9 @@ static void etm_set_pwrup(struct etm_drvdata *drvdata)
> {
> u32 etmpdcr;
>
> - etmpdcr = readl_relaxed(drvdata->base + ETMPDCR);
> + etmpdcr = readl_relaxed(drvdata->csa.base + ETMPDCR);
> etmpdcr |= ETMPDCR_PWD_UP;
> - writel_relaxed(etmpdcr, drvdata->base + ETMPDCR);
> + writel_relaxed(etmpdcr, drvdata->csa.base + ETMPDCR);
> /* Ensure pwrup completes before subsequent cp14 accesses */
> mb();
> isb();
> @@ -101,9 +101,9 @@ static void etm_clr_pwrup(struct etm_drvdata *drvdata)
> /* Ensure pending cp14 accesses complete before clearing pwrup */
> mb();
> isb();
> - etmpdcr = readl_relaxed(drvdata->base + ETMPDCR);
> + etmpdcr = readl_relaxed(drvdata->csa.base + ETMPDCR);
> etmpdcr &= ~ETMPDCR_PWD_UP;
> - writel_relaxed(etmpdcr, drvdata->base + ETMPDCR);
> + writel_relaxed(etmpdcr, drvdata->csa.base + ETMPDCR);
> }
>
> /**
> @@ -365,7 +365,7 @@ static int etm_enable_hw(struct etm_drvdata *drvdata)
> struct etm_config *config = &drvdata->config;
> struct coresight_device *csdev = drvdata->csdev;
>
> - CS_UNLOCK(drvdata->base);
> + CS_UNLOCK(drvdata->csa.base);
>
> rc = coresight_claim_device_unlocked(csdev);
> if (rc)
> @@ -427,7 +427,7 @@ static int etm_enable_hw(struct etm_drvdata *drvdata)
> etm_clr_prog(drvdata);
>
> done:
> - CS_LOCK(drvdata->base);
> + CS_LOCK(drvdata->csa.base);
>
> dev_dbg(&drvdata->csdev->dev, "cpu: %d enable smp call done: %d\n",
> drvdata->cpu, rc);
> @@ -589,7 +589,7 @@ static void etm_disable_hw(void *info)
> struct etm_config *config = &drvdata->config;
> struct coresight_device *csdev = drvdata->csdev;
>
> - CS_UNLOCK(drvdata->base);
> + CS_UNLOCK(drvdata->csa.base);
> etm_set_prog(drvdata);
>
> /* Read back sequencer and counters for post trace analysis */
> @@ -601,7 +601,7 @@ static void etm_disable_hw(void *info)
> etm_set_pwrdwn(drvdata);
> coresight_disclaim_device_unlocked(&csdev->access);
>
> - CS_LOCK(drvdata->base);
> + CS_LOCK(drvdata->csa.base);
>
> dev_dbg(&drvdata->csdev->dev,
> "cpu: %d disable smp call done\n", drvdata->cpu);
> @@ -614,7 +614,7 @@ static void etm_disable_perf(struct coresight_device *csdev)
> if (WARN_ON_ONCE(drvdata->cpu != smp_processor_id()))
> return;
>
> - CS_UNLOCK(drvdata->base);
> + CS_UNLOCK(drvdata->csa.base);
>
> /* Setting the prog bit disables tracing immediately */
> etm_set_prog(drvdata);
> @@ -626,7 +626,7 @@ static void etm_disable_perf(struct coresight_device *csdev)
> etm_set_pwrdwn(drvdata);
> coresight_disclaim_device_unlocked(&csdev->access);
>
> - CS_LOCK(drvdata->base);
> + CS_LOCK(drvdata->csa.base);
>
> /*
> * perf will release trace ids when _free_aux()
> @@ -772,7 +772,7 @@ static void etm_init_arch_data(void *info)
> /* Make sure all registers are accessible */
> etm_os_unlock(drvdata);
>
> - CS_UNLOCK(drvdata->base);
> + CS_UNLOCK(drvdata->csa.base);
>
> /* First dummy read */
> (void)etm_readl(drvdata, ETMPDSR);
> @@ -803,9 +803,10 @@ static void etm_init_arch_data(void *info)
> drvdata->nr_ext_out = BMVAL(etmccr, 20, 22);
> drvdata->nr_ctxid_cmp = BMVAL(etmccr, 24, 25);
>
> + coresight_reset_claim_unlocked(&drvdata->csa);
> etm_set_pwrdwn(drvdata);
> etm_clr_pwrup(drvdata);
> - CS_LOCK(drvdata->base);
> + CS_LOCK(drvdata->csa.base);
> }
>
> static int __init etm_hp_setup(void)
> @@ -866,8 +867,7 @@ static int etm_probe(struct amba_device *adev, const struct amba_id *id)
> if (IS_ERR(base))
> return PTR_ERR(base);
>
> - drvdata->base = base;
> - desc.access = CSDEV_ACCESS_IOMEM(base);
> + desc.access = drvdata->csa = CSDEV_ACCESS_IOMEM(base);
>
> spin_lock_init(&drvdata->spinlock);
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
> index 68c644be9813..af566b62785c 100644
> --- a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
> +++ b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
> @@ -50,11 +50,11 @@ static ssize_t etmsr_show(struct device *dev,
>
> pm_runtime_get_sync(dev->parent);
> spin_lock_irqsave(&drvdata->spinlock, flags);
> - CS_UNLOCK(drvdata->base);
> + CS_UNLOCK(drvdata->csa.base);
>
> val = etm_readl(drvdata, ETMSR);
>
> - CS_LOCK(drvdata->base);
> + CS_LOCK(drvdata->csa.base);
> spin_unlock_irqrestore(&drvdata->spinlock, flags);
> pm_runtime_put(dev->parent);
>
> @@ -949,9 +949,9 @@ static ssize_t seq_curr_state_show(struct device *dev,
> pm_runtime_get_sync(dev->parent);
> spin_lock_irqsave(&drvdata->spinlock, flags);
>
> - CS_UNLOCK(drvdata->base);
> + CS_UNLOCK(drvdata->csa.base);
> val = (etm_readl(drvdata, ETMSQR) & ETM_SQR_MASK);
> - CS_LOCK(drvdata->base);
> + CS_LOCK(drvdata->csa.base);
>
> spin_unlock_irqrestore(&drvdata->spinlock, flags);
> pm_runtime_put(dev->parent);
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index 45b30a4b3eba..3609bb74c4ec 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -1337,6 +1337,8 @@ static void etm4_init_arch_data(void *info)
> drvdata->nrseqstate = FIELD_GET(TRCIDR5_NUMSEQSTATE_MASK, etmidr5);
> /* NUMCNTR, bits[30:28] number of counters available for tracing */
> drvdata->nr_cntr = FIELD_GET(TRCIDR5_NUMCNTR_MASK, etmidr5);
> +
> + coresight_reset_claim_unlocked(csa);
> etm4_cs_lock(drvdata, csa);
> cpu_detect_trace_filtering(drvdata);
> }
> diff --git a/drivers/hwtracing/coresight/coresight-funnel.c b/drivers/hwtracing/coresight/coresight-funnel.c
> index e986922d555b..be2a47636ef3 100644
> --- a/drivers/hwtracing/coresight/coresight-funnel.c
> +++ b/drivers/hwtracing/coresight/coresight-funnel.c
> @@ -255,6 +255,7 @@ static int funnel_probe(struct device *dev, struct resource *res)
> drvdata->base = base;
> desc.groups = coresight_funnel_groups;
> desc.access = CSDEV_ACCESS_IOMEM(base);
> + coresight_reset_claim(&desc.access);
> }
>
> dev_set_drvdata(dev, drvdata);
> @@ -272,6 +273,7 @@ static int funnel_probe(struct device *dev, struct resource *res)
> desc.ops = &funnel_cs_ops;
> desc.pdata = pdata;
> desc.dev = dev;
> +
> drvdata->csdev = coresight_register(&desc);
> if (IS_ERR(drvdata->csdev)) {
> ret = PTR_ERR(drvdata->csdev);
> diff --git a/drivers/hwtracing/coresight/coresight-replicator.c b/drivers/hwtracing/coresight/coresight-replicator.c
> index 5d42a9a8c460..679a36effbe8 100644
> --- a/drivers/hwtracing/coresight/coresight-replicator.c
> +++ b/drivers/hwtracing/coresight/coresight-replicator.c
> @@ -284,6 +284,7 @@ static int replicator_probe(struct device *dev, struct resource *res)
> desc.pdata = dev->platform_data;
> desc.dev = dev;
>
> + coresight_reset_claim(&desc.access);
> drvdata->csdev = coresight_register(&desc);
> if (IS_ERR(drvdata->csdev)) {
> ret = PTR_ERR(drvdata->csdev);
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c
> index e9876252a789..5ac4e3c706ac 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-core.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-core.c
> @@ -558,6 +558,7 @@ static int __tmc_probe(struct device *dev, struct resource *res)
> dev->platform_data = pdata;
> desc.pdata = pdata;
>
> + coresight_reset_claim(&desc.access);
> drvdata->csdev = coresight_register(&desc);
> if (IS_ERR(drvdata->csdev)) {
> ret = PTR_ERR(drvdata->csdev);
> diff --git a/include/linux/coresight.h b/include/linux/coresight.h
> index 937931d107e0..11808aee9d1d 100644
> --- a/include/linux/coresight.h
> +++ b/include/linux/coresight.h
> @@ -655,6 +655,9 @@ extern int coresight_claim_device_unlocked(struct coresight_device *csdev);
>
> extern void coresight_disclaim_device(struct csdev_access *csa);
> extern void coresight_disclaim_device_unlocked(struct csdev_access *csa);
> +int coresight_reset_claim_unlocked(struct csdev_access *csa);
> +int coresight_reset_claim(struct csdev_access *csa);
> +
> extern char *coresight_alloc_device_name(struct coresight_dev_list *devs,
> struct device *dev);
>
> --
> 2.34.1
>
--
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 5/7] coresight: Clear self hosted claim tag on probe
2025-02-12 18:24 ` Mike Leach
@ 2025-02-13 13:20 ` James Clark
0 siblings, 0 replies; 25+ messages in thread
From: James Clark @ 2025-02-13 13:20 UTC (permalink / raw)
To: Mike Leach
Cc: lcherian, coresight, Suzuki K Poulose, Alexander Shishkin,
Maxime Coquelin, Alexandre Torgue, linux-arm-kernel, linux-kernel,
linux-stm32
On 12/02/2025 6:24 pm, Mike Leach wrote:
> Hi James,
>
> On Tue, 11 Feb 2025 at 10:40, James Clark <james.clark@linaro.org> wrote:
>>
>> This can be left behind from a crashed kernel after a kexec so clear it
>> when probing each device. Similarly to
>> coresight_disclaim_device_unlocked(), only clear it if it's already set
>> to avoid races with an external debugger.
>>
>> We need a csdev_access struct in etm_init_arch_data() so just replace
>> the iomem pointer with a full csdev_access struct. This means all usages
>> need to be updated to go through csa->base.
>>
>> Signed-off-by: James Clark <james.clark@linaro.org>
>> ---
>> drivers/hwtracing/coresight/coresight-catu.c | 1 +
>> drivers/hwtracing/coresight/coresight-core.c | 48 +++++++++++++++----
>> .../hwtracing/coresight/coresight-cti-core.c | 2 +
>> drivers/hwtracing/coresight/coresight-etb10.c | 2 +
>> drivers/hwtracing/coresight/coresight-etm.h | 6 +--
>> .../coresight/coresight-etm3x-core.c | 28 +++++------
>> .../coresight/coresight-etm3x-sysfs.c | 8 ++--
>> .../coresight/coresight-etm4x-core.c | 2 +
>> .../hwtracing/coresight/coresight-funnel.c | 2 +
>> .../coresight/coresight-replicator.c | 1 +
>> .../hwtracing/coresight/coresight-tmc-core.c | 1 +
>> include/linux/coresight.h | 3 ++
>> 12 files changed, 73 insertions(+), 31 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-catu.c b/drivers/hwtracing/coresight/coresight-catu.c
>> index d9259c0b6e64..575c2d247a90 100644
>> --- a/drivers/hwtracing/coresight/coresight-catu.c
>> +++ b/drivers/hwtracing/coresight/coresight-catu.c
>> @@ -558,6 +558,7 @@ static int __catu_probe(struct device *dev, struct resource *res)
>> catu_desc.subtype.helper_subtype = CORESIGHT_DEV_SUBTYPE_HELPER_CATU;
>> catu_desc.ops = &catu_ops;
>>
>> + coresight_reset_claim(&catu_desc.access);
>> drvdata->csdev = coresight_register(&catu_desc);
>> if (IS_ERR(drvdata->csdev))
>> ret = PTR_ERR(drvdata->csdev);
>> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
>> index 7fe5d5d432c4..97f33ffad05e 100644
>> --- a/drivers/hwtracing/coresight/coresight-core.c
>> +++ b/drivers/hwtracing/coresight/coresight-core.c
>> @@ -212,20 +212,48 @@ int coresight_claim_device(struct coresight_device *csdev)
>> EXPORT_SYMBOL_GPL(coresight_claim_device);
>>
>> /*
>> - * coresight_disclaim_device_unlocked : Clear the claim tag for the device.
>> + * Clear the claim tag for the device.
>> + * Returns an error if the device wasn't already claimed.
>> + */
>> +int coresight_reset_claim(struct csdev_access *csa)
>
> Given the iusue being fixed - i.e. previous sessions having residual
> claims - and the fact that this funtion is only ever called from
> device initialisation as far as I can tell,
> would this be better to be called coresight_init_claim() and return /
> log an error only if the device _was_ actually claimed.
>
> As it stands it seems to return an error if the device was not claimed
> - which on initialisation is the correct state!
>
> Since the retrun code is ignored by all of the callees, the rerun
> could be dropped and a warning issued withing this function if the
> claim needed clearing.
>
This is only to reuse code with coresight_disclaim_device_unlocked()
which does use the return code. I can drop the return code from here but
then we'd have to have two functions that do the same thing. One used on
probe and the other used on disable.
>> +{
>> + int ret;
>> +
>> + CS_UNLOCK(csa->base);
>> + ret = coresight_reset_claim_unlocked(csa);
>> + CS_LOCK(csa->base);
>> + return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(coresight_reset_claim);
>> +
>> +/*
>> + * Clear the claim tag for the device. Called with CS_UNLOCKed for the component.
>> + * Returns an error if the device wasn't already claimed.
>> + */
>> +int coresight_reset_claim_unlocked(struct csdev_access *csa)
>
> Given the comment and the function - this could be named
> "coresight_clear_claim_unlocked()"
>
I suppose we could rename it. It might cause some confusion with
coresight_clear_self_claim_tag() though. I deliberately picked 'reset'
because of when it was used on probe. Whereas 'clearing' is some
existing function that only clears a bit without any extra logic. That
kind of leaves it structured like this:
clear: lowest level, only mechanical clear of the bit
reset: clear if set, no warning
disclaim: clear if set, warn if not set
>
> Regards
>
> Mike
>
>> +{
>> + if (coresight_read_claim_tags(csa) == CORESIGHT_CLAIM_SELF_HOSTED) {
>> + coresight_clear_self_claim_tag(csa);
>> + return 0;
>> + }
>> +
>> + return -EINVAL;
>> +}
>> +EXPORT_SYMBOL_GPL(coresight_reset_claim_unlocked);
>> +
>> +/*
>> + * coresight_disclaim_device_unlocked : Clear the claim tag for the device
>> + * and warn if the device wasn't already claimed.
>> * Called with CS_UNLOCKed for the component.
>> */
>> void coresight_disclaim_device_unlocked(struct csdev_access *csa)
>> {
>> - if (coresight_read_claim_tags(csa) == CORESIGHT_CLAIM_SELF_HOSTED)
>> - coresight_clear_self_claim_tag(csa);
>> - else
>> - /*
>> - * The external agent may have not honoured our claim
>> - * and has manipulated it. Or something else has seriously
>> - * gone wrong in our driver.
>> - */
>> - WARN_ON_ONCE(1);
>> + /*
>> + * Warn if the external agent hasn't honoured our claim
>> + * and has manipulated it. Or something else has seriously
>> + * gone wrong in our driver.
>> + */
>> + WARN_ON_ONCE(coresight_reset_claim_unlocked(csa));
>> }
>> EXPORT_SYMBOL_GPL(coresight_disclaim_device_unlocked);
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-cti-core.c b/drivers/hwtracing/coresight/coresight-cti-core.c
>> index 073f67a41af9..389a72362f0c 100644
>> --- a/drivers/hwtracing/coresight/coresight-cti-core.c
>> +++ b/drivers/hwtracing/coresight/coresight-cti-core.c
>> @@ -931,6 +931,8 @@ static int cti_probe(struct amba_device *adev, const struct amba_id *id)
>> cti_desc.ops = &cti_ops;
>> cti_desc.groups = drvdata->ctidev.con_groups;
>> cti_desc.dev = dev;
>> +
>> + coresight_reset_claim(&cti_desc.access);
>> drvdata->csdev = coresight_register(&cti_desc);
>> if (IS_ERR(drvdata->csdev)) {
>> ret = PTR_ERR(drvdata->csdev);
>> diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c
>> index d8bc3e776c88..b598b2c0c9bb 100644
>> --- a/drivers/hwtracing/coresight/coresight-etb10.c
>> +++ b/drivers/hwtracing/coresight/coresight-etb10.c
>> @@ -772,6 +772,8 @@ static int etb_probe(struct amba_device *adev, const struct amba_id *id)
>> desc.pdata = pdata;
>> desc.dev = dev;
>> desc.groups = coresight_etb_groups;
>> +
>> + coresight_reset_claim(&desc.access);
>> drvdata->csdev = coresight_register(&desc);
>> if (IS_ERR(drvdata->csdev))
>> return PTR_ERR(drvdata->csdev);
>> diff --git a/drivers/hwtracing/coresight/coresight-etm.h b/drivers/hwtracing/coresight/coresight-etm.h
>> index e02c3ea972c9..a89736309c27 100644
>> --- a/drivers/hwtracing/coresight/coresight-etm.h
>> +++ b/drivers/hwtracing/coresight/coresight-etm.h
>> @@ -229,7 +229,7 @@ struct etm_config {
>> * @config: structure holding configuration parameters.
>> */
>> struct etm_drvdata {
>> - void __iomem *base;
>> + struct csdev_access csa;
>> struct clk *atclk;
>> struct coresight_device *csdev;
>> spinlock_t spinlock;
>> @@ -260,7 +260,7 @@ static inline void etm_writel(struct etm_drvdata *drvdata,
>> "invalid CP14 access to ETM reg: %#x", off);
>> }
>> } else {
>> - writel_relaxed(val, drvdata->base + off);
>> + writel_relaxed(val, drvdata->csa.base + off);
>> }
>> }
>>
>> @@ -274,7 +274,7 @@ static inline unsigned int etm_readl(struct etm_drvdata *drvdata, u32 off)
>> "invalid CP14 access to ETM reg: %#x", off);
>> }
>> } else {
>> - val = readl_relaxed(drvdata->base + off);
>> + val = readl_relaxed(drvdata->csa.base + off);
>> }
>>
>> return val;
>> diff --git a/drivers/hwtracing/coresight/coresight-etm3x-core.c b/drivers/hwtracing/coresight/coresight-etm3x-core.c
>> index 509f53b69e42..0b010683b883 100644
>> --- a/drivers/hwtracing/coresight/coresight-etm3x-core.c
>> +++ b/drivers/hwtracing/coresight/coresight-etm3x-core.c
>> @@ -86,9 +86,9 @@ static void etm_set_pwrup(struct etm_drvdata *drvdata)
>> {
>> u32 etmpdcr;
>>
>> - etmpdcr = readl_relaxed(drvdata->base + ETMPDCR);
>> + etmpdcr = readl_relaxed(drvdata->csa.base + ETMPDCR);
>> etmpdcr |= ETMPDCR_PWD_UP;
>> - writel_relaxed(etmpdcr, drvdata->base + ETMPDCR);
>> + writel_relaxed(etmpdcr, drvdata->csa.base + ETMPDCR);
>> /* Ensure pwrup completes before subsequent cp14 accesses */
>> mb();
>> isb();
>> @@ -101,9 +101,9 @@ static void etm_clr_pwrup(struct etm_drvdata *drvdata)
>> /* Ensure pending cp14 accesses complete before clearing pwrup */
>> mb();
>> isb();
>> - etmpdcr = readl_relaxed(drvdata->base + ETMPDCR);
>> + etmpdcr = readl_relaxed(drvdata->csa.base + ETMPDCR);
>> etmpdcr &= ~ETMPDCR_PWD_UP;
>> - writel_relaxed(etmpdcr, drvdata->base + ETMPDCR);
>> + writel_relaxed(etmpdcr, drvdata->csa.base + ETMPDCR);
>> }
>>
>> /**
>> @@ -365,7 +365,7 @@ static int etm_enable_hw(struct etm_drvdata *drvdata)
>> struct etm_config *config = &drvdata->config;
>> struct coresight_device *csdev = drvdata->csdev;
>>
>> - CS_UNLOCK(drvdata->base);
>> + CS_UNLOCK(drvdata->csa.base);
>>
>> rc = coresight_claim_device_unlocked(csdev);
>> if (rc)
>> @@ -427,7 +427,7 @@ static int etm_enable_hw(struct etm_drvdata *drvdata)
>> etm_clr_prog(drvdata);
>>
>> done:
>> - CS_LOCK(drvdata->base);
>> + CS_LOCK(drvdata->csa.base);
>>
>> dev_dbg(&drvdata->csdev->dev, "cpu: %d enable smp call done: %d\n",
>> drvdata->cpu, rc);
>> @@ -589,7 +589,7 @@ static void etm_disable_hw(void *info)
>> struct etm_config *config = &drvdata->config;
>> struct coresight_device *csdev = drvdata->csdev;
>>
>> - CS_UNLOCK(drvdata->base);
>> + CS_UNLOCK(drvdata->csa.base);
>> etm_set_prog(drvdata);
>>
>> /* Read back sequencer and counters for post trace analysis */
>> @@ -601,7 +601,7 @@ static void etm_disable_hw(void *info)
>> etm_set_pwrdwn(drvdata);
>> coresight_disclaim_device_unlocked(&csdev->access);
>>
>> - CS_LOCK(drvdata->base);
>> + CS_LOCK(drvdata->csa.base);
>>
>> dev_dbg(&drvdata->csdev->dev,
>> "cpu: %d disable smp call done\n", drvdata->cpu);
>> @@ -614,7 +614,7 @@ static void etm_disable_perf(struct coresight_device *csdev)
>> if (WARN_ON_ONCE(drvdata->cpu != smp_processor_id()))
>> return;
>>
>> - CS_UNLOCK(drvdata->base);
>> + CS_UNLOCK(drvdata->csa.base);
>>
>> /* Setting the prog bit disables tracing immediately */
>> etm_set_prog(drvdata);
>> @@ -626,7 +626,7 @@ static void etm_disable_perf(struct coresight_device *csdev)
>> etm_set_pwrdwn(drvdata);
>> coresight_disclaim_device_unlocked(&csdev->access);
>>
>> - CS_LOCK(drvdata->base);
>> + CS_LOCK(drvdata->csa.base);
>>
>> /*
>> * perf will release trace ids when _free_aux()
>> @@ -772,7 +772,7 @@ static void etm_init_arch_data(void *info)
>> /* Make sure all registers are accessible */
>> etm_os_unlock(drvdata);
>>
>> - CS_UNLOCK(drvdata->base);
>> + CS_UNLOCK(drvdata->csa.base);
>>
>> /* First dummy read */
>> (void)etm_readl(drvdata, ETMPDSR);
>> @@ -803,9 +803,10 @@ static void etm_init_arch_data(void *info)
>> drvdata->nr_ext_out = BMVAL(etmccr, 20, 22);
>> drvdata->nr_ctxid_cmp = BMVAL(etmccr, 24, 25);
>>
>> + coresight_reset_claim_unlocked(&drvdata->csa);
>> etm_set_pwrdwn(drvdata);
>> etm_clr_pwrup(drvdata);
>> - CS_LOCK(drvdata->base);
>> + CS_LOCK(drvdata->csa.base);
>> }
>>
>> static int __init etm_hp_setup(void)
>> @@ -866,8 +867,7 @@ static int etm_probe(struct amba_device *adev, const struct amba_id *id)
>> if (IS_ERR(base))
>> return PTR_ERR(base);
>>
>> - drvdata->base = base;
>> - desc.access = CSDEV_ACCESS_IOMEM(base);
>> + desc.access = drvdata->csa = CSDEV_ACCESS_IOMEM(base);
>>
>> spin_lock_init(&drvdata->spinlock);
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
>> index 68c644be9813..af566b62785c 100644
>> --- a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
>> +++ b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
>> @@ -50,11 +50,11 @@ static ssize_t etmsr_show(struct device *dev,
>>
>> pm_runtime_get_sync(dev->parent);
>> spin_lock_irqsave(&drvdata->spinlock, flags);
>> - CS_UNLOCK(drvdata->base);
>> + CS_UNLOCK(drvdata->csa.base);
>>
>> val = etm_readl(drvdata, ETMSR);
>>
>> - CS_LOCK(drvdata->base);
>> + CS_LOCK(drvdata->csa.base);
>> spin_unlock_irqrestore(&drvdata->spinlock, flags);
>> pm_runtime_put(dev->parent);
>>
>> @@ -949,9 +949,9 @@ static ssize_t seq_curr_state_show(struct device *dev,
>> pm_runtime_get_sync(dev->parent);
>> spin_lock_irqsave(&drvdata->spinlock, flags);
>>
>> - CS_UNLOCK(drvdata->base);
>> + CS_UNLOCK(drvdata->csa.base);
>> val = (etm_readl(drvdata, ETMSQR) & ETM_SQR_MASK);
>> - CS_LOCK(drvdata->base);
>> + CS_LOCK(drvdata->csa.base);
>>
>> spin_unlock_irqrestore(&drvdata->spinlock, flags);
>> pm_runtime_put(dev->parent);
>> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> index 45b30a4b3eba..3609bb74c4ec 100644
>> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> @@ -1337,6 +1337,8 @@ static void etm4_init_arch_data(void *info)
>> drvdata->nrseqstate = FIELD_GET(TRCIDR5_NUMSEQSTATE_MASK, etmidr5);
>> /* NUMCNTR, bits[30:28] number of counters available for tracing */
>> drvdata->nr_cntr = FIELD_GET(TRCIDR5_NUMCNTR_MASK, etmidr5);
>> +
>> + coresight_reset_claim_unlocked(csa);
>> etm4_cs_lock(drvdata, csa);
>> cpu_detect_trace_filtering(drvdata);
>> }
>> diff --git a/drivers/hwtracing/coresight/coresight-funnel.c b/drivers/hwtracing/coresight/coresight-funnel.c
>> index e986922d555b..be2a47636ef3 100644
>> --- a/drivers/hwtracing/coresight/coresight-funnel.c
>> +++ b/drivers/hwtracing/coresight/coresight-funnel.c
>> @@ -255,6 +255,7 @@ static int funnel_probe(struct device *dev, struct resource *res)
>> drvdata->base = base;
>> desc.groups = coresight_funnel_groups;
>> desc.access = CSDEV_ACCESS_IOMEM(base);
>> + coresight_reset_claim(&desc.access);
>> }
>>
>> dev_set_drvdata(dev, drvdata);
>> @@ -272,6 +273,7 @@ static int funnel_probe(struct device *dev, struct resource *res)
>> desc.ops = &funnel_cs_ops;
>> desc.pdata = pdata;
>> desc.dev = dev;
>> +
>> drvdata->csdev = coresight_register(&desc);
>> if (IS_ERR(drvdata->csdev)) {
>> ret = PTR_ERR(drvdata->csdev);
>> diff --git a/drivers/hwtracing/coresight/coresight-replicator.c b/drivers/hwtracing/coresight/coresight-replicator.c
>> index 5d42a9a8c460..679a36effbe8 100644
>> --- a/drivers/hwtracing/coresight/coresight-replicator.c
>> +++ b/drivers/hwtracing/coresight/coresight-replicator.c
>> @@ -284,6 +284,7 @@ static int replicator_probe(struct device *dev, struct resource *res)
>> desc.pdata = dev->platform_data;
>> desc.dev = dev;
>>
>> + coresight_reset_claim(&desc.access);
>> drvdata->csdev = coresight_register(&desc);
>> if (IS_ERR(drvdata->csdev)) {
>> ret = PTR_ERR(drvdata->csdev);
>> diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c
>> index e9876252a789..5ac4e3c706ac 100644
>> --- a/drivers/hwtracing/coresight/coresight-tmc-core.c
>> +++ b/drivers/hwtracing/coresight/coresight-tmc-core.c
>> @@ -558,6 +558,7 @@ static int __tmc_probe(struct device *dev, struct resource *res)
>> dev->platform_data = pdata;
>> desc.pdata = pdata;
>>
>> + coresight_reset_claim(&desc.access);
>> drvdata->csdev = coresight_register(&desc);
>> if (IS_ERR(drvdata->csdev)) {
>> ret = PTR_ERR(drvdata->csdev);
>> diff --git a/include/linux/coresight.h b/include/linux/coresight.h
>> index 937931d107e0..11808aee9d1d 100644
>> --- a/include/linux/coresight.h
>> +++ b/include/linux/coresight.h
>> @@ -655,6 +655,9 @@ extern int coresight_claim_device_unlocked(struct coresight_device *csdev);
>>
>> extern void coresight_disclaim_device(struct csdev_access *csa);
>> extern void coresight_disclaim_device_unlocked(struct csdev_access *csa);
>> +int coresight_reset_claim_unlocked(struct csdev_access *csa);
>> +int coresight_reset_claim(struct csdev_access *csa);
>> +
>> extern char *coresight_alloc_device_name(struct coresight_dev_list *devs,
>> struct device *dev);
>>
>> --
>> 2.34.1
>>
>
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 5/7] coresight: Clear self hosted claim tag on probe
2025-02-11 10:39 ` [PATCH 5/7] coresight: Clear self hosted claim tag on probe James Clark
2025-02-12 18:24 ` Mike Leach
@ 2025-03-13 16:04 ` Leo Yan
2025-03-17 15:05 ` James Clark
1 sibling, 1 reply; 25+ messages in thread
From: Leo Yan @ 2025-03-13 16:04 UTC (permalink / raw)
To: James Clark
Cc: lcherian, coresight, Mike Leach, Alexander Shishkin,
Maxime Coquelin, Alexandre Torgue, linux-arm-kernel, linux-kernel,
linux-stm32
On Tue, Feb 11, 2025 at 10:39:41AM +0000, James Clark wrote:
>
> This can be left behind from a crashed kernel after a kexec so clear it
> when probing each device. Similarly to
> coresight_disclaim_device_unlocked(), only clear it if it's already set
> to avoid races with an external debugger.
>
> We need a csdev_access struct in etm_init_arch_data() so just replace
> the iomem pointer with a full csdev_access struct. This means all usages
> need to be updated to go through csa->base.
>
> Signed-off-by: James Clark <james.clark@linaro.org>
> ---
> drivers/hwtracing/coresight/coresight-catu.c | 1 +
> drivers/hwtracing/coresight/coresight-core.c | 48 +++++++++++++++----
> .../hwtracing/coresight/coresight-cti-core.c | 2 +
> drivers/hwtracing/coresight/coresight-etb10.c | 2 +
> drivers/hwtracing/coresight/coresight-etm.h | 6 +--
> .../coresight/coresight-etm3x-core.c | 28 +++++------
> .../coresight/coresight-etm3x-sysfs.c | 8 ++--
> .../coresight/coresight-etm4x-core.c | 2 +
> .../hwtracing/coresight/coresight-funnel.c | 2 +
> .../coresight/coresight-replicator.c | 1 +
> .../hwtracing/coresight/coresight-tmc-core.c | 1 +
> include/linux/coresight.h | 3 ++
> 12 files changed, 73 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-catu.c b/drivers/hwtracing/coresight/coresight-catu.c
> index d9259c0b6e64..575c2d247a90 100644
> --- a/drivers/hwtracing/coresight/coresight-catu.c
> +++ b/drivers/hwtracing/coresight/coresight-catu.c
> @@ -558,6 +558,7 @@ static int __catu_probe(struct device *dev, struct resource *res)
> catu_desc.subtype.helper_subtype = CORESIGHT_DEV_SUBTYPE_HELPER_CATU;
> catu_desc.ops = &catu_ops;
>
> + coresight_reset_claim(&catu_desc.access);
> drvdata->csdev = coresight_register(&catu_desc);
> if (IS_ERR(drvdata->csdev))
> ret = PTR_ERR(drvdata->csdev);
> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> index 7fe5d5d432c4..97f33ffad05e 100644
> --- a/drivers/hwtracing/coresight/coresight-core.c
> +++ b/drivers/hwtracing/coresight/coresight-core.c
> @@ -212,20 +212,48 @@ int coresight_claim_device(struct coresight_device *csdev)
> EXPORT_SYMBOL_GPL(coresight_claim_device);
>
> /*
> - * coresight_disclaim_device_unlocked : Clear the claim tag for the device.
> + * Clear the claim tag for the device.
> + * Returns an error if the device wasn't already claimed.
> + */
> +int coresight_reset_claim(struct csdev_access *csa)
> +{
> + int ret;
> +
> + CS_UNLOCK(csa->base);
> + ret = coresight_reset_claim_unlocked(csa);
> + CS_LOCK(csa->base);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(coresight_reset_claim);
Maybe my question is overlapping with Mike's comment.
Callers never check the return values from coresight_reset_claim(). I am
wandering if coresight_reset_claim() can directly call
coresight_clear_self_claim_tag() for _trying_ to clear self-host tag in
probe phase. Any self claim tag issues will be deferred to detect until
enable the component.
For consistent, we might rename coresight_reset_claim() to
coresight_reset_self_claim_tag(), which acquires CS lock and clear
self claim tag.
> +/*
> + * Clear the claim tag for the device. Called with CS_UNLOCKed for the component.
> + * Returns an error if the device wasn't already claimed.
> + */
> +int coresight_reset_claim_unlocked(struct csdev_access *csa)
> +{
> + if (coresight_read_claim_tags(csa) == CORESIGHT_CLAIM_SELF_HOSTED) {
> + coresight_clear_self_claim_tag(csa);
> + return 0;
> + }
> +
> + return -EINVAL;
> +}
> +EXPORT_SYMBOL_GPL(coresight_reset_claim_unlocked);
> +
> +/*
> + * coresight_disclaim_device_unlocked : Clear the claim tag for the device
> + * and warn if the device wasn't already claimed.
> * Called with CS_UNLOCKed for the component.
> */
> void coresight_disclaim_device_unlocked(struct csdev_access *csa)
> {
> - if (coresight_read_claim_tags(csa) == CORESIGHT_CLAIM_SELF_HOSTED)
> - coresight_clear_self_claim_tag(csa);
> - else
> - /*
> - * The external agent may have not honoured our claim
> - * and has manipulated it. Or something else has seriously
> - * gone wrong in our driver.
> - */
> - WARN_ON_ONCE(1);
> + /*
> + * Warn if the external agent hasn't honoured our claim
> + * and has manipulated it. Or something else has seriously
> + * gone wrong in our driver.
> + */
> + WARN_ON_ONCE(coresight_reset_claim_unlocked(csa));
> }
> EXPORT_SYMBOL_GPL(coresight_disclaim_device_unlocked);
>
> diff --git a/drivers/hwtracing/coresight/coresight-cti-core.c b/drivers/hwtracing/coresight/coresight-cti-core.c
> index 073f67a41af9..389a72362f0c 100644
> --- a/drivers/hwtracing/coresight/coresight-cti-core.c
> +++ b/drivers/hwtracing/coresight/coresight-cti-core.c
> @@ -931,6 +931,8 @@ static int cti_probe(struct amba_device *adev, const struct amba_id *id)
> cti_desc.ops = &cti_ops;
> cti_desc.groups = drvdata->ctidev.con_groups;
> cti_desc.dev = dev;
> +
> + coresight_reset_claim(&cti_desc.access);
> drvdata->csdev = coresight_register(&cti_desc);
> if (IS_ERR(drvdata->csdev)) {
> ret = PTR_ERR(drvdata->csdev);
> diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c
> index d8bc3e776c88..b598b2c0c9bb 100644
> --- a/drivers/hwtracing/coresight/coresight-etb10.c
> +++ b/drivers/hwtracing/coresight/coresight-etb10.c
> @@ -772,6 +772,8 @@ static int etb_probe(struct amba_device *adev, const struct amba_id *id)
> desc.pdata = pdata;
> desc.dev = dev;
> desc.groups = coresight_etb_groups;
> +
> + coresight_reset_claim(&desc.access);
> drvdata->csdev = coresight_register(&desc);
> if (IS_ERR(drvdata->csdev))
> return PTR_ERR(drvdata->csdev);
> diff --git a/drivers/hwtracing/coresight/coresight-etm.h b/drivers/hwtracing/coresight/coresight-etm.h
> index e02c3ea972c9..a89736309c27 100644
> --- a/drivers/hwtracing/coresight/coresight-etm.h
> +++ b/drivers/hwtracing/coresight/coresight-etm.h
> @@ -229,7 +229,7 @@ struct etm_config {
> * @config: structure holding configuration parameters.
> */
> struct etm_drvdata {
> - void __iomem *base;
> + struct csdev_access csa;
I would like to extract the change for using `csdev_access` in the
ETMv3 driver into a new patch, which is irrelevant to reset self claim
tags and would significantly reduce the complexity in this patch.
Thanks,
Leo
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 5/7] coresight: Clear self hosted claim tag on probe
2025-03-13 16:04 ` Leo Yan
@ 2025-03-17 15:05 ` James Clark
2025-03-17 18:09 ` Leo Yan
0 siblings, 1 reply; 25+ messages in thread
From: James Clark @ 2025-03-17 15:05 UTC (permalink / raw)
To: Leo Yan
Cc: lcherian, coresight, Mike Leach, Alexander Shishkin,
Maxime Coquelin, Alexandre Torgue, linux-arm-kernel, linux-kernel,
linux-stm32
On 13/03/2025 4:04 pm, Leo Yan wrote:
> On Tue, Feb 11, 2025 at 10:39:41AM +0000, James Clark wrote:
>>
>> This can be left behind from a crashed kernel after a kexec so clear it
>> when probing each device. Similarly to
>> coresight_disclaim_device_unlocked(), only clear it if it's already set
>> to avoid races with an external debugger.
>>
>> We need a csdev_access struct in etm_init_arch_data() so just replace
>> the iomem pointer with a full csdev_access struct. This means all usages
>> need to be updated to go through csa->base.
>>
>> Signed-off-by: James Clark <james.clark@linaro.org>
>> ---
>> drivers/hwtracing/coresight/coresight-catu.c | 1 +
>> drivers/hwtracing/coresight/coresight-core.c | 48 +++++++++++++++----
>> .../hwtracing/coresight/coresight-cti-core.c | 2 +
>> drivers/hwtracing/coresight/coresight-etb10.c | 2 +
>> drivers/hwtracing/coresight/coresight-etm.h | 6 +--
>> .../coresight/coresight-etm3x-core.c | 28 +++++------
>> .../coresight/coresight-etm3x-sysfs.c | 8 ++--
>> .../coresight/coresight-etm4x-core.c | 2 +
>> .../hwtracing/coresight/coresight-funnel.c | 2 +
>> .../coresight/coresight-replicator.c | 1 +
>> .../hwtracing/coresight/coresight-tmc-core.c | 1 +
>> include/linux/coresight.h | 3 ++
>> 12 files changed, 73 insertions(+), 31 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-catu.c b/drivers/hwtracing/coresight/coresight-catu.c
>> index d9259c0b6e64..575c2d247a90 100644
>> --- a/drivers/hwtracing/coresight/coresight-catu.c
>> +++ b/drivers/hwtracing/coresight/coresight-catu.c
>> @@ -558,6 +558,7 @@ static int __catu_probe(struct device *dev, struct resource *res)
>> catu_desc.subtype.helper_subtype = CORESIGHT_DEV_SUBTYPE_HELPER_CATU;
>> catu_desc.ops = &catu_ops;
>>
>> + coresight_reset_claim(&catu_desc.access);
>> drvdata->csdev = coresight_register(&catu_desc);
>> if (IS_ERR(drvdata->csdev))
>> ret = PTR_ERR(drvdata->csdev);
>> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
>> index 7fe5d5d432c4..97f33ffad05e 100644
>> --- a/drivers/hwtracing/coresight/coresight-core.c
>> +++ b/drivers/hwtracing/coresight/coresight-core.c
>> @@ -212,20 +212,48 @@ int coresight_claim_device(struct coresight_device *csdev)
>> EXPORT_SYMBOL_GPL(coresight_claim_device);
>>
>> /*
>> - * coresight_disclaim_device_unlocked : Clear the claim tag for the device.
>> + * Clear the claim tag for the device.
>> + * Returns an error if the device wasn't already claimed.
>> + */
>> +int coresight_reset_claim(struct csdev_access *csa)
>> +{
>> + int ret;
>> +
>> + CS_UNLOCK(csa->base);
>> + ret = coresight_reset_claim_unlocked(csa);
>> + CS_LOCK(csa->base);
>> + return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(coresight_reset_claim);
>
> Maybe my question is overlapping with Mike's comment.
>
> Callers never check the return values from coresight_reset_claim(). I am
I can remove the return value if it's confusing. The thought process was
probably that it could be useful somewhere in the future, and
coresight_reset_claim_unlocked() returns something anyway so might as
well pass it up.
> wandering if coresight_reset_claim() can directly call
> coresight_clear_self_claim_tag() for _trying_ to clear self-host tag in
> probe phase. Any self claim tag issues will be deferred to detect until
> enable the component.
>
Maybe, the spec does a read before setting which I assumed should be
done for clearing as well. As in to not touch anything if it's in use
externally. It doesn't specifically describe any clearing sequence, but
if we assume it's ok to blindly clear self hosted flag even when it's in
use then yes we can directly call coresight_clear_self_claim_tag().
This doesn't change anything about deferring errors though,
coresight_reset_claim() doesn't warn early either.
> For consistent, we might rename coresight_reset_claim() to
> coresight_reset_self_claim_tag(), which acquires CS lock and clear
> self claim tag.
>
>> +/*
>> + * Clear the claim tag for the device. Called with CS_UNLOCKed for the component.
>> + * Returns an error if the device wasn't already claimed.
>> + */
>> +int coresight_reset_claim_unlocked(struct csdev_access *csa)
>> +{
>> + if (coresight_read_claim_tags(csa) == CORESIGHT_CLAIM_SELF_HOSTED) {
>> + coresight_clear_self_claim_tag(csa);
>> + return 0;
>> + }
>> +
>> + return -EINVAL;
>> +}
>> +EXPORT_SYMBOL_GPL(coresight_reset_claim_unlocked);
>> +
>> +/*
>> + * coresight_disclaim_device_unlocked : Clear the claim tag for the device
>> + * and warn if the device wasn't already claimed.
>> * Called with CS_UNLOCKed for the component.
>> */
>> void coresight_disclaim_device_unlocked(struct csdev_access *csa)
>> {
>> - if (coresight_read_claim_tags(csa) == CORESIGHT_CLAIM_SELF_HOSTED)
>> - coresight_clear_self_claim_tag(csa);
>> - else
>> - /*
>> - * The external agent may have not honoured our claim
>> - * and has manipulated it. Or something else has seriously
>> - * gone wrong in our driver.
>> - */
>> - WARN_ON_ONCE(1);
>> + /*
>> + * Warn if the external agent hasn't honoured our claim
>> + * and has manipulated it. Or something else has seriously
>> + * gone wrong in our driver.
>> + */
>> + WARN_ON_ONCE(coresight_reset_claim_unlocked(csa));
>> }
>> EXPORT_SYMBOL_GPL(coresight_disclaim_device_unlocked);
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-cti-core.c b/drivers/hwtracing/coresight/coresight-cti-core.c
>> index 073f67a41af9..389a72362f0c 100644
>> --- a/drivers/hwtracing/coresight/coresight-cti-core.c
>> +++ b/drivers/hwtracing/coresight/coresight-cti-core.c
>> @@ -931,6 +931,8 @@ static int cti_probe(struct amba_device *adev, const struct amba_id *id)
>> cti_desc.ops = &cti_ops;
>> cti_desc.groups = drvdata->ctidev.con_groups;
>> cti_desc.dev = dev;
>> +
>> + coresight_reset_claim(&cti_desc.access);
>> drvdata->csdev = coresight_register(&cti_desc);
>> if (IS_ERR(drvdata->csdev)) {
>> ret = PTR_ERR(drvdata->csdev);
>> diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c
>> index d8bc3e776c88..b598b2c0c9bb 100644
>> --- a/drivers/hwtracing/coresight/coresight-etb10.c
>> +++ b/drivers/hwtracing/coresight/coresight-etb10.c
>> @@ -772,6 +772,8 @@ static int etb_probe(struct amba_device *adev, const struct amba_id *id)
>> desc.pdata = pdata;
>> desc.dev = dev;
>> desc.groups = coresight_etb_groups;
>> +
>> + coresight_reset_claim(&desc.access);
>> drvdata->csdev = coresight_register(&desc);
>> if (IS_ERR(drvdata->csdev))
>> return PTR_ERR(drvdata->csdev);
>> diff --git a/drivers/hwtracing/coresight/coresight-etm.h b/drivers/hwtracing/coresight/coresight-etm.h
>> index e02c3ea972c9..a89736309c27 100644
>> --- a/drivers/hwtracing/coresight/coresight-etm.h
>> +++ b/drivers/hwtracing/coresight/coresight-etm.h
>> @@ -229,7 +229,7 @@ struct etm_config {
>> * @config: structure holding configuration parameters.
>> */
>> struct etm_drvdata {
>> - void __iomem *base;
>> + struct csdev_access csa;
>
> I would like to extract the change for using `csdev_access` in the
> ETMv3 driver into a new patch, which is irrelevant to reset self claim
> tags and would significantly reduce the complexity in this patch.
>
Will do, good point.
> Thanks,
> Leo
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 5/7] coresight: Clear self hosted claim tag on probe
2025-03-17 15:05 ` James Clark
@ 2025-03-17 18:09 ` Leo Yan
0 siblings, 0 replies; 25+ messages in thread
From: Leo Yan @ 2025-03-17 18:09 UTC (permalink / raw)
To: James Clark
Cc: lcherian, coresight, Mike Leach, Alexander Shishkin,
Maxime Coquelin, Alexandre Torgue, linux-arm-kernel, linux-kernel,
linux-stm32
On Mon, Mar 17, 2025 at 03:05:41PM +0000, James Clark wrote:
[...]
> > > /*
> > > - * coresight_disclaim_device_unlocked : Clear the claim tag for the device.
> > > + * Clear the claim tag for the device.
> > > + * Returns an error if the device wasn't already claimed.
> > > + */
> > > +int coresight_reset_claim(struct csdev_access *csa)
> > > +{
> > > + int ret;
> > > +
> > > + CS_UNLOCK(csa->base);
> > > + ret = coresight_reset_claim_unlocked(csa);
> > > + CS_LOCK(csa->base);
> > > + return ret;
> > > +}
> > > +EXPORT_SYMBOL_GPL(coresight_reset_claim);
> >
> > Maybe my question is overlapping with Mike's comment.
> >
> > Callers never check the return values from coresight_reset_claim(). I am
>
> I can remove the return value if it's confusing. The thought process was
> probably that it could be useful somewhere in the future, and
> coresight_reset_claim_unlocked() returns something anyway so might as well
> pass it up.
>
> > wandering if coresight_reset_claim() can directly call
> > coresight_clear_self_claim_tag() for _trying_ to clear self-host tag in
> > probe phase. Any self claim tag issues will be deferred to detect until
> > enable the component.
> >
>
> Maybe, the spec does a read before setting which I assumed should be done
> for clearing as well.
If you mean PSCI doc, the flow for claiming tag is a handshake between
self-host software and external debugger, this is why a read is
required.
For a cleaning up, it is not about syncing with external debugger. And
writing bit 0 to TRCCLAIMCLR will not impact external debugger. So I
don't think a read prior to cleaning self claim tag is needed.
> As in to not touch anything if it's in use externally.
> It doesn't specifically describe any clearing sequence, but if we assume
> it's ok to blindly clear self hosted flag even when it's in use then yes we
> can directly call coresight_clear_self_claim_tag().
As a result, directly calling coresight_clear_self_claim_tag() works
for me.
Thanks,
Leo
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 6/7] coresight: Remove inlines from static function definitions
2025-02-11 10:39 [PATCH 0/7] coresight: Clear self hosted claim tag on probe James Clark
` (4 preceding siblings ...)
2025-02-11 10:39 ` [PATCH 5/7] coresight: Clear self hosted claim tag on probe James Clark
@ 2025-02-11 10:39 ` James Clark
2025-03-14 9:50 ` Leo Yan
2025-02-11 10:39 ` [PATCH 7/7] coresight: Remove extern from function declarations James Clark
6 siblings, 1 reply; 25+ messages in thread
From: James Clark @ 2025-02-11 10:39 UTC (permalink / raw)
To: lcherian, coresight
Cc: James Clark, Suzuki K Poulose, Mike Leach, Alexander Shishkin,
Maxime Coquelin, Alexandre Torgue, linux-arm-kernel, linux-kernel,
linux-stm32
These are all static and in one compilation unit so the inline has no
effect on the binary. Except if FTRACE is enabled, then some functions
which were already not inlined now get the nops added which allows them
to be traced.
Signed-off-by: James Clark <james.clark@linaro.org>
---
drivers/hwtracing/coresight/coresight-catu.c | 11 ++++----
drivers/hwtracing/coresight/coresight-core.c | 16 ++++++------
drivers/hwtracing/coresight/coresight-etb10.c | 2 +-
.../coresight/coresight-etm4x-core.c | 8 +++---
.../coresight/coresight-etm4x-sysfs.c | 4 +--
.../hwtracing/coresight/coresight-platform.c | 26 +++++++++----------
.../coresight/coresight-replicator.c | 2 +-
drivers/hwtracing/coresight/coresight-stm.c | 6 ++---
.../coresight/coresight-syscfg-configfs.c | 2 +-
.../hwtracing/coresight/coresight-tmc-core.c | 8 +++---
.../hwtracing/coresight/coresight-tmc-etr.c | 16 +++++-------
drivers/hwtracing/coresight/coresight-trbe.c | 18 ++++++-------
12 files changed, 58 insertions(+), 61 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-catu.c b/drivers/hwtracing/coresight/coresight-catu.c
index 575c2d247a90..ffa08ea3c296 100644
--- a/drivers/hwtracing/coresight/coresight-catu.c
+++ b/drivers/hwtracing/coresight/coresight-catu.c
@@ -113,9 +113,8 @@ typedef u64 cate_t;
* containing the data page pointer for @offset. If @daddrp is not NULL,
* @daddrp points the DMA address of the beginning of the table.
*/
-static inline cate_t *catu_get_table(struct tmc_sg_table *catu_table,
- unsigned long offset,
- dma_addr_t *daddrp)
+static cate_t *catu_get_table(struct tmc_sg_table *catu_table, unsigned long offset,
+ dma_addr_t *daddrp)
{
unsigned long buf_size = tmc_sg_table_buf_size(catu_table);
unsigned int table_nr, pg_idx, pg_offset;
@@ -165,12 +164,12 @@ static void catu_dump_table(struct tmc_sg_table *catu_table)
}
#else
-static inline void catu_dump_table(struct tmc_sg_table *catu_table)
+static void catu_dump_table(struct tmc_sg_table *catu_table)
{
}
#endif
-static inline cate_t catu_make_entry(dma_addr_t addr)
+static cate_t catu_make_entry(dma_addr_t addr)
{
return addr ? CATU_VALID_ENTRY(addr) : 0;
}
@@ -390,7 +389,7 @@ static const struct attribute_group *catu_groups[] = {
};
-static inline int catu_wait_for_ready(struct catu_drvdata *drvdata)
+static int catu_wait_for_ready(struct catu_drvdata *drvdata)
{
struct csdev_access *csa = &drvdata->csdev->access;
diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index 97f33ffad05e..396218819f26 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -127,20 +127,20 @@ coresight_find_out_connection(struct coresight_device *csdev,
return ERR_PTR(-ENODEV);
}
-static inline u32 coresight_read_claim_tags(struct csdev_access *csa)
+static u32 coresight_read_claim_tags(struct csdev_access *csa)
{
return FIELD_GET(CORESIGHT_CLAIM_MASK,
csdev_access_relaxed_read32(csa, CORESIGHT_CLAIMCLR));
}
-static inline void coresight_set_self_claim_tag(struct csdev_access *csa)
+static void coresight_set_self_claim_tag(struct csdev_access *csa)
{
csdev_access_relaxed_write32(csa, CORESIGHT_CLAIM_SELF_HOSTED,
CORESIGHT_CLAIMSET);
isb();
}
-static inline void coresight_clear_self_claim_tag(struct csdev_access *csa)
+static void coresight_clear_self_claim_tag(struct csdev_access *csa)
{
csdev_access_relaxed_write32(csa, CORESIGHT_CLAIM_SELF_HOSTED,
CORESIGHT_CLAIMCLR);
@@ -610,7 +610,7 @@ struct coresight_device *coresight_get_sink_by_id(u32 id)
* Return true in successful case and power up the device.
* Return false when failed to get reference of module.
*/
-static inline bool coresight_get_ref(struct coresight_device *csdev)
+static bool coresight_get_ref(struct coresight_device *csdev)
{
struct device *dev = csdev->dev.parent;
@@ -629,7 +629,7 @@ static inline bool coresight_get_ref(struct coresight_device *csdev)
*
* @csdev: The coresight device to decrement a reference from.
*/
-static inline void coresight_put_ref(struct coresight_device *csdev)
+static void coresight_put_ref(struct coresight_device *csdev)
{
struct device *dev = csdev->dev.parent;
@@ -808,7 +808,7 @@ void coresight_release_path(struct list_head *path)
}
/* return true if the device is a suitable type for a default sink */
-static inline bool coresight_is_def_sink_type(struct coresight_device *csdev)
+static bool coresight_is_def_sink_type(struct coresight_device *csdev)
{
/* sink & correct subtype */
if (((csdev->type == CORESIGHT_DEV_TYPE_SINK) ||
@@ -1362,8 +1362,8 @@ EXPORT_SYMBOL_GPL(coresight_unregister);
*
* Returns the index of the entry, when found. Otherwise, -ENOENT.
*/
-static inline int coresight_search_device_idx(struct coresight_dev_list *dict,
- struct fwnode_handle *fwnode)
+static int coresight_search_device_idx(struct coresight_dev_list *dict,
+ struct fwnode_handle *fwnode)
{
int i;
diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c
index b598b2c0c9bb..de6d3a6fb828 100644
--- a/drivers/hwtracing/coresight/coresight-etb10.c
+++ b/drivers/hwtracing/coresight/coresight-etb10.c
@@ -95,7 +95,7 @@ struct etb_drvdata {
static int etb_set_buffer(struct coresight_device *csdev,
struct perf_output_handle *handle);
-static inline unsigned int etb_get_buffer_depth(struct etb_drvdata *drvdata)
+static unsigned int etb_get_buffer_depth(struct etb_drvdata *drvdata)
{
return readl_relaxed(drvdata->base + ETB_RAM_DEPTH_REG);
}
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index 3609bb74c4ec..9e7d423c1ae9 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -84,7 +84,7 @@ static int etm4_probe_cpu(unsigned int cpu);
* TRCIDR4.NUMPC > 0b0000 .
* TRCSSCSR<n>.PC == 0b1
*/
-static inline bool etm4x_sspcicrn_present(struct etmv4_drvdata *drvdata, int n)
+static bool etm4x_sspcicrn_present(struct etmv4_drvdata *drvdata, int n)
{
return (n < drvdata->nr_ss_cmp) &&
drvdata->nr_pe &&
@@ -185,7 +185,7 @@ static void etm_write_os_lock(struct etmv4_drvdata *drvdata,
isb();
}
-static inline void etm4_os_unlock_csa(struct etmv4_drvdata *drvdata,
+static void etm4_os_unlock_csa(struct etmv4_drvdata *drvdata,
struct csdev_access *csa)
{
WARN_ON(drvdata->cpu != smp_processor_id());
@@ -1035,7 +1035,7 @@ static const struct coresight_ops etm4_cs_ops = {
.source_ops = &etm4_source_ops,
};
-static inline bool cpu_supports_sysreg_trace(void)
+static bool cpu_supports_sysreg_trace(void)
{
u64 dfr0 = read_sysreg_s(SYS_ID_AA64DFR0_EL1);
@@ -1343,7 +1343,7 @@ static void etm4_init_arch_data(void *info)
cpu_detect_trace_filtering(drvdata);
}
-static inline u32 etm4_get_victlr_access_type(struct etmv4_config *config)
+static u32 etm4_get_victlr_access_type(struct etmv4_config *config)
{
return etm4_get_access_type(config) << __bf_shf(TRCVICTLR_EXLEVEL_MASK);
}
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
index a9f19629f3f8..0465c32f7ed4 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
@@ -2440,7 +2440,7 @@ static u32 etmv4_cross_read(const struct etmv4_drvdata *drvdata, u32 offset)
return reg.data;
}
-static inline u32 coresight_etm4x_attr_to_offset(struct device_attribute *attr)
+static u32 coresight_etm4x_attr_to_offset(struct device_attribute *attr)
{
struct dev_ext_attribute *eattr;
@@ -2464,7 +2464,7 @@ static ssize_t coresight_etm4x_reg_show(struct device *dev,
return scnprintf(buf, PAGE_SIZE, "0x%x\n", val);
}
-static inline bool
+static bool
etm4x_register_implemented(struct etmv4_drvdata *drvdata, u32 offset)
{
switch (offset) {
diff --git a/drivers/hwtracing/coresight/coresight-platform.c b/drivers/hwtracing/coresight/coresight-platform.c
index 8192ba3279f0..0db64c5f4995 100644
--- a/drivers/hwtracing/coresight/coresight-platform.c
+++ b/drivers/hwtracing/coresight/coresight-platform.c
@@ -139,7 +139,7 @@ coresight_find_csdev_by_fwnode(struct fwnode_handle *r_fwnode)
EXPORT_SYMBOL_GPL(coresight_find_csdev_by_fwnode);
#ifdef CONFIG_OF
-static inline bool of_coresight_legacy_ep_is_input(struct device_node *ep)
+static bool of_coresight_legacy_ep_is_input(struct device_node *ep)
{
return of_property_read_bool(ep, "slave-mode");
}
@@ -159,7 +159,7 @@ static struct device_node *of_coresight_get_port_parent(struct device_node *ep)
return parent;
}
-static inline struct device_node *
+static struct device_node *
of_coresight_get_output_ports_node(const struct device_node *node)
{
return of_get_child_by_name(node, "out-ports");
@@ -327,14 +327,14 @@ static int of_get_coresight_platform_data(struct device *dev,
return 0;
}
#else
-static inline int
+static int
of_get_coresight_platform_data(struct device *dev,
struct coresight_platform_data *pdata)
{
return -ENOENT;
}
-static inline int of_coresight_get_cpu(struct device *dev)
+static int of_coresight_get_cpu(struct device *dev)
{
return -ENODEV;
}
@@ -356,7 +356,7 @@ static const guid_t coresight_graph_uuid = GUID_INIT(0x3ecbc8b6, 0x1d0e, 0x4fb3,
#define ACPI_CORESIGHT_LINK_SLAVE 0
#define ACPI_CORESIGHT_LINK_MASTER 1
-static inline bool is_acpi_guid(const union acpi_object *obj)
+static bool is_acpi_guid(const union acpi_object *obj)
{
return (obj->type == ACPI_TYPE_BUFFER) && (obj->buffer.length == 16);
}
@@ -365,24 +365,24 @@ static inline bool is_acpi_guid(const union acpi_object *obj)
* acpi_guid_matches - Checks if the given object is a GUID object and
* that it matches the supplied the GUID.
*/
-static inline bool acpi_guid_matches(const union acpi_object *obj,
+static bool acpi_guid_matches(const union acpi_object *obj,
const guid_t *guid)
{
return is_acpi_guid(obj) &&
guid_equal((guid_t *)obj->buffer.pointer, guid);
}
-static inline bool is_acpi_dsd_graph_guid(const union acpi_object *obj)
+static bool is_acpi_dsd_graph_guid(const union acpi_object *obj)
{
return acpi_guid_matches(obj, &acpi_graph_uuid);
}
-static inline bool is_acpi_coresight_graph_guid(const union acpi_object *obj)
+static bool is_acpi_coresight_graph_guid(const union acpi_object *obj)
{
return acpi_guid_matches(obj, &coresight_graph_uuid);
}
-static inline bool is_acpi_coresight_graph(const union acpi_object *obj)
+static bool is_acpi_coresight_graph(const union acpi_object *obj)
{
const union acpi_object *graphid, *guid, *links;
@@ -469,7 +469,7 @@ static inline bool is_acpi_coresight_graph(const union acpi_object *obj)
* }, // End of ACPI Graph Property
* })
*/
-static inline bool acpi_validate_dsd_graph(const union acpi_object *graph)
+static bool acpi_validate_dsd_graph(const union acpi_object *graph)
{
int i, n;
const union acpi_object *rev, *nr_graphs;
@@ -553,7 +553,7 @@ acpi_get_dsd_graph(struct acpi_device *adev, struct acpi_buffer *buf)
return NULL;
}
-static inline bool
+static bool
acpi_validate_coresight_graph(const union acpi_object *cs_graph)
{
int nlinks;
@@ -794,14 +794,14 @@ acpi_get_coresight_platform_data(struct device *dev,
#else
-static inline int
+static int
acpi_get_coresight_platform_data(struct device *dev,
struct coresight_platform_data *pdata)
{
return -ENOENT;
}
-static inline int acpi_coresight_get_cpu(struct device *dev)
+static int acpi_coresight_get_cpu(struct device *dev)
{
return -ENODEV;
}
diff --git a/drivers/hwtracing/coresight/coresight-replicator.c b/drivers/hwtracing/coresight/coresight-replicator.c
index 679a36effbe8..9f27374f1310 100644
--- a/drivers/hwtracing/coresight/coresight-replicator.c
+++ b/drivers/hwtracing/coresight/coresight-replicator.c
@@ -63,7 +63,7 @@ static void dynamic_replicator_reset(struct replicator_drvdata *drvdata)
/*
* replicator_reset : Reset the replicator configuration to sane values.
*/
-static inline void replicator_reset(struct replicator_drvdata *drvdata)
+static void replicator_reset(struct replicator_drvdata *drvdata)
{
if (drvdata->base)
dynamic_replicator_reset(drvdata);
diff --git a/drivers/hwtracing/coresight/coresight-stm.c b/drivers/hwtracing/coresight/coresight-stm.c
index b581a30a1cd9..5decc87d1fba 100644
--- a/drivers/hwtracing/coresight/coresight-stm.c
+++ b/drivers/hwtracing/coresight/coresight-stm.c
@@ -290,7 +290,7 @@ static const struct coresight_ops stm_cs_ops = {
.source_ops = &stm_source_ops,
};
-static inline bool stm_addr_unaligned(const void *addr, u8 write_bytes)
+static bool stm_addr_unaligned(const void *addr, u8 write_bytes)
{
return ((unsigned long)addr & (write_bytes - 1));
}
@@ -674,7 +674,7 @@ static int of_stm_get_stimulus_area(struct device *dev, struct resource *res)
return of_address_to_resource(np, index, res);
}
#else
-static inline int of_stm_get_stimulus_area(struct device *dev,
+static int of_stm_get_stimulus_area(struct device *dev,
struct resource *res)
{
return -ENOENT;
@@ -718,7 +718,7 @@ static int acpi_stm_get_stimulus_area(struct device *dev, struct resource *res)
return rc;
}
#else
-static inline int acpi_stm_get_stimulus_area(struct device *dev,
+static int acpi_stm_get_stimulus_area(struct device *dev,
struct resource *res)
{
return -ENOENT;
diff --git a/drivers/hwtracing/coresight/coresight-syscfg-configfs.c b/drivers/hwtracing/coresight/coresight-syscfg-configfs.c
index 213b4159b062..2b40e556be87 100644
--- a/drivers/hwtracing/coresight/coresight-syscfg-configfs.c
+++ b/drivers/hwtracing/coresight/coresight-syscfg-configfs.c
@@ -10,7 +10,7 @@
#include "coresight-syscfg-configfs.h"
/* create a default ci_type. */
-static inline struct config_item_type *cscfg_create_ci_type(void)
+static struct config_item_type *cscfg_create_ci_type(void)
{
struct config_item_type *ci_type;
diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c
index 5ac4e3c706ac..2e3354b5c5c7 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-core.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-core.c
@@ -164,8 +164,8 @@ static int tmc_open(struct inode *inode, struct file *file)
return 0;
}
-static inline ssize_t tmc_get_sysfs_trace(struct tmc_drvdata *drvdata,
- loff_t pos, size_t len, char **bufpp)
+static ssize_t tmc_get_sysfs_trace(struct tmc_drvdata *drvdata, loff_t pos, size_t len,
+ char **bufpp)
{
switch (drvdata->config_type) {
case TMC_CONFIG_TYPE_ETB:
@@ -359,7 +359,7 @@ static const struct attribute_group *coresight_etr_groups[] = {
NULL,
};
-static inline bool tmc_etr_can_use_sg(struct device *dev)
+static bool tmc_etr_can_use_sg(struct device *dev)
{
int ret;
u8 val_u8;
@@ -389,7 +389,7 @@ static inline bool tmc_etr_can_use_sg(struct device *dev)
return false;
}
-static inline bool tmc_etr_has_non_secure_access(struct tmc_drvdata *drvdata)
+static bool tmc_etr_has_non_secure_access(struct tmc_drvdata *drvdata)
{
u32 auth = readl_relaxed(drvdata->base + TMC_AUTHSTATUS);
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
index 7ba4f79191de..0d4e3af0d4b5 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
@@ -124,7 +124,7 @@ struct etr_sg_table {
* If we spill over to a new page for mapping 1 entry, we could as
* well replace the link entry of the previous page with the last entry.
*/
-static inline unsigned long __attribute_const__
+static unsigned long __attribute_const__
tmc_etr_sg_table_entries(int nr_pages)
{
unsigned long nr_sgpages = nr_pages * ETR_SG_PAGES_PER_SYSPAGE;
@@ -238,13 +238,13 @@ static int tmc_pages_alloc(struct tmc_pages *tmc_pages,
return -ENOMEM;
}
-static inline long
+static long
tmc_sg_get_data_page_offset(struct tmc_sg_table *sg_table, dma_addr_t addr)
{
return tmc_pages_get_offset(&sg_table->data_pages, addr);
}
-static inline void tmc_free_table_pages(struct tmc_sg_table *sg_table)
+static void tmc_free_table_pages(struct tmc_sg_table *sg_table)
{
if (sg_table->table_vaddr)
vunmap(sg_table->table_vaddr);
@@ -480,7 +480,7 @@ static void tmc_etr_sg_table_dump(struct etr_sg_table *etr_table)
dev_dbg(sg_table->dev, "******* End of Table *****\n");
}
#else
-static inline void tmc_etr_sg_table_dump(struct etr_sg_table *etr_table) {}
+static void tmc_etr_sg_table_dump(struct etr_sg_table *etr_table) {}
#endif
/*
@@ -815,10 +815,8 @@ void tmc_etr_remove_catu_ops(void)
}
EXPORT_SYMBOL_GPL(tmc_etr_remove_catu_ops);
-static inline int tmc_etr_mode_alloc_buf(int mode,
- struct tmc_drvdata *drvdata,
- struct etr_buf *etr_buf, int node,
- void **pages)
+static int tmc_etr_mode_alloc_buf(int mode, struct tmc_drvdata *drvdata, struct etr_buf *etr_buf,
+ int node, void **pages)
{
int rc = -EINVAL;
@@ -936,7 +934,7 @@ static ssize_t tmc_etr_buf_get_data(struct etr_buf *etr_buf,
return etr_buf->ops->get_data(etr_buf, (u64)offset, len, bufpp);
}
-static inline s64
+static s64
tmc_etr_buf_insert_barrier_packet(struct etr_buf *etr_buf, u64 offset)
{
ssize_t len;
diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
index 919804b12a67..e9a19ef52359 100644
--- a/drivers/hwtracing/coresight/coresight-trbe.c
+++ b/drivers/hwtracing/coresight/coresight-trbe.c
@@ -159,22 +159,22 @@ static void trbe_check_errata(struct trbe_cpudata *cpudata)
}
}
-static inline bool trbe_has_erratum(struct trbe_cpudata *cpudata, int i)
+static bool trbe_has_erratum(struct trbe_cpudata *cpudata, int i)
{
return (i < TRBE_ERRATA_MAX) && test_bit(i, cpudata->errata);
}
-static inline bool trbe_may_overwrite_in_fill_mode(struct trbe_cpudata *cpudata)
+static bool trbe_may_overwrite_in_fill_mode(struct trbe_cpudata *cpudata)
{
return trbe_has_erratum(cpudata, TRBE_WORKAROUND_OVERWRITE_FILL_MODE);
}
-static inline bool trbe_may_write_out_of_range(struct trbe_cpudata *cpudata)
+static bool trbe_may_write_out_of_range(struct trbe_cpudata *cpudata)
{
return trbe_has_erratum(cpudata, TRBE_WORKAROUND_WRITE_OUT_OF_RANGE);
}
-static inline bool trbe_needs_drain_after_disable(struct trbe_cpudata *cpudata)
+static bool trbe_needs_drain_after_disable(struct trbe_cpudata *cpudata)
{
/*
* Errata affected TRBE implementation will need TSB CSYNC and
@@ -184,7 +184,7 @@ static inline bool trbe_needs_drain_after_disable(struct trbe_cpudata *cpudata)
return trbe_has_erratum(cpudata, TRBE_NEEDS_DRAIN_AFTER_DISABLE);
}
-static inline bool trbe_needs_ctxt_sync_after_enable(struct trbe_cpudata *cpudata)
+static bool trbe_needs_ctxt_sync_after_enable(struct trbe_cpudata *cpudata)
{
/*
* Errata affected TRBE implementation will need an additional
@@ -195,7 +195,7 @@ static inline bool trbe_needs_ctxt_sync_after_enable(struct trbe_cpudata *cpudat
return trbe_has_erratum(cpudata, TRBE_NEEDS_CTXT_SYNC_AFTER_ENABLE);
}
-static inline bool trbe_is_broken(struct trbe_cpudata *cpudata)
+static bool trbe_is_broken(struct trbe_cpudata *cpudata)
{
return trbe_has_erratum(cpudata, TRBE_IS_BROKEN);
}
@@ -207,13 +207,13 @@ static int trbe_alloc_node(struct perf_event *event)
return cpu_to_node(event->cpu);
}
-static inline void trbe_drain_buffer(void)
+static void trbe_drain_buffer(void)
{
tsb_csync();
dsb(nsh);
}
-static inline void set_trbe_enabled(struct trbe_cpudata *cpudata, u64 trblimitr)
+static void set_trbe_enabled(struct trbe_cpudata *cpudata, u64 trblimitr)
{
/*
* Enable the TRBE without clearing LIMITPTR which
@@ -229,7 +229,7 @@ static inline void set_trbe_enabled(struct trbe_cpudata *cpudata, u64 trblimitr)
isb();
}
-static inline void set_trbe_disabled(struct trbe_cpudata *cpudata)
+static void set_trbe_disabled(struct trbe_cpudata *cpudata)
{
u64 trblimitr = read_sysreg_s(SYS_TRBLIMITR_EL1);
--
2.34.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 6/7] coresight: Remove inlines from static function definitions
2025-02-11 10:39 ` [PATCH 6/7] coresight: Remove inlines from static function definitions James Clark
@ 2025-03-14 9:50 ` Leo Yan
2025-03-17 15:26 ` James Clark
0 siblings, 1 reply; 25+ messages in thread
From: Leo Yan @ 2025-03-14 9:50 UTC (permalink / raw)
To: James Clark
Cc: lcherian, coresight, Mike Leach, Alexander Shishkin,
Maxime Coquelin, Alexandre Torgue, linux-arm-kernel, linux-kernel,
linux-stm32
On Tue, Feb 11, 2025 at 10:39:42AM +0000, James Clark wrote:
>
> These are all static and in one compilation unit so the inline has no
> effect on the binary.
I read the doc coding-style.rst, the section "15) The inline disease",
my understanding is though the inline is not encouraged, I prefer we can
follow up the rule:
A reasonable rule of thumb is to not put inline at functions that have more
than 3 lines of code in them. An exception to this rule are the cases where
a parameter is known to be a compile time constant, and as a result of this
constantness you *know* the compiler will be able to optimize most of your
function away at compile time. For a good example of this later case, see
the kmalloc() inline function.
> Except if FTRACE is enabled, then some functions
> which were already not inlined now get the nops added which allows them
> to be traced.
I understand it is a side effect that we cannot use function tracer for
inline functions. This is not a big issue for me, as we still can use
"perf probe" and trace_printk() for debugging.
Thanks,
Leo
> Signed-off-by: James Clark <james.clark@linaro.org>
> ---
> drivers/hwtracing/coresight/coresight-catu.c | 11 ++++----
> drivers/hwtracing/coresight/coresight-core.c | 16 ++++++------
> drivers/hwtracing/coresight/coresight-etb10.c | 2 +-
> .../coresight/coresight-etm4x-core.c | 8 +++---
> .../coresight/coresight-etm4x-sysfs.c | 4 +--
> .../hwtracing/coresight/coresight-platform.c | 26 +++++++++----------
> .../coresight/coresight-replicator.c | 2 +-
> drivers/hwtracing/coresight/coresight-stm.c | 6 ++---
> .../coresight/coresight-syscfg-configfs.c | 2 +-
> .../hwtracing/coresight/coresight-tmc-core.c | 8 +++---
> .../hwtracing/coresight/coresight-tmc-etr.c | 16 +++++-------
> drivers/hwtracing/coresight/coresight-trbe.c | 18 ++++++-------
> 12 files changed, 58 insertions(+), 61 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-catu.c b/drivers/hwtracing/coresight/coresight-catu.c
> index 575c2d247a90..ffa08ea3c296 100644
> --- a/drivers/hwtracing/coresight/coresight-catu.c
> +++ b/drivers/hwtracing/coresight/coresight-catu.c
> @@ -113,9 +113,8 @@ typedef u64 cate_t;
> * containing the data page pointer for @offset. If @daddrp is not NULL,
> * @daddrp points the DMA address of the beginning of the table.
> */
> -static inline cate_t *catu_get_table(struct tmc_sg_table *catu_table,
> - unsigned long offset,
> - dma_addr_t *daddrp)
> +static cate_t *catu_get_table(struct tmc_sg_table *catu_table, unsigned long offset,
> + dma_addr_t *daddrp)
> {
> unsigned long buf_size = tmc_sg_table_buf_size(catu_table);
> unsigned int table_nr, pg_idx, pg_offset;
> @@ -165,12 +164,12 @@ static void catu_dump_table(struct tmc_sg_table *catu_table)
> }
>
> #else
> -static inline void catu_dump_table(struct tmc_sg_table *catu_table)
> +static void catu_dump_table(struct tmc_sg_table *catu_table)
> {
> }
> #endif
>
> -static inline cate_t catu_make_entry(dma_addr_t addr)
> +static cate_t catu_make_entry(dma_addr_t addr)
> {
> return addr ? CATU_VALID_ENTRY(addr) : 0;
> }
> @@ -390,7 +389,7 @@ static const struct attribute_group *catu_groups[] = {
> };
>
>
> -static inline int catu_wait_for_ready(struct catu_drvdata *drvdata)
> +static int catu_wait_for_ready(struct catu_drvdata *drvdata)
> {
> struct csdev_access *csa = &drvdata->csdev->access;
>
> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> index 97f33ffad05e..396218819f26 100644
> --- a/drivers/hwtracing/coresight/coresight-core.c
> +++ b/drivers/hwtracing/coresight/coresight-core.c
> @@ -127,20 +127,20 @@ coresight_find_out_connection(struct coresight_device *csdev,
> return ERR_PTR(-ENODEV);
> }
>
> -static inline u32 coresight_read_claim_tags(struct csdev_access *csa)
> +static u32 coresight_read_claim_tags(struct csdev_access *csa)
> {
> return FIELD_GET(CORESIGHT_CLAIM_MASK,
> csdev_access_relaxed_read32(csa, CORESIGHT_CLAIMCLR));
> }
>
> -static inline void coresight_set_self_claim_tag(struct csdev_access *csa)
> +static void coresight_set_self_claim_tag(struct csdev_access *csa)
> {
> csdev_access_relaxed_write32(csa, CORESIGHT_CLAIM_SELF_HOSTED,
> CORESIGHT_CLAIMSET);
> isb();
> }
>
> -static inline void coresight_clear_self_claim_tag(struct csdev_access *csa)
> +static void coresight_clear_self_claim_tag(struct csdev_access *csa)
> {
> csdev_access_relaxed_write32(csa, CORESIGHT_CLAIM_SELF_HOSTED,
> CORESIGHT_CLAIMCLR);
> @@ -610,7 +610,7 @@ struct coresight_device *coresight_get_sink_by_id(u32 id)
> * Return true in successful case and power up the device.
> * Return false when failed to get reference of module.
> */
> -static inline bool coresight_get_ref(struct coresight_device *csdev)
> +static bool coresight_get_ref(struct coresight_device *csdev)
> {
> struct device *dev = csdev->dev.parent;
>
> @@ -629,7 +629,7 @@ static inline bool coresight_get_ref(struct coresight_device *csdev)
> *
> * @csdev: The coresight device to decrement a reference from.
> */
> -static inline void coresight_put_ref(struct coresight_device *csdev)
> +static void coresight_put_ref(struct coresight_device *csdev)
> {
> struct device *dev = csdev->dev.parent;
>
> @@ -808,7 +808,7 @@ void coresight_release_path(struct list_head *path)
> }
>
> /* return true if the device is a suitable type for a default sink */
> -static inline bool coresight_is_def_sink_type(struct coresight_device *csdev)
> +static bool coresight_is_def_sink_type(struct coresight_device *csdev)
> {
> /* sink & correct subtype */
> if (((csdev->type == CORESIGHT_DEV_TYPE_SINK) ||
> @@ -1362,8 +1362,8 @@ EXPORT_SYMBOL_GPL(coresight_unregister);
> *
> * Returns the index of the entry, when found. Otherwise, -ENOENT.
> */
> -static inline int coresight_search_device_idx(struct coresight_dev_list *dict,
> - struct fwnode_handle *fwnode)
> +static int coresight_search_device_idx(struct coresight_dev_list *dict,
> + struct fwnode_handle *fwnode)
> {
> int i;
>
> diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c
> index b598b2c0c9bb..de6d3a6fb828 100644
> --- a/drivers/hwtracing/coresight/coresight-etb10.c
> +++ b/drivers/hwtracing/coresight/coresight-etb10.c
> @@ -95,7 +95,7 @@ struct etb_drvdata {
> static int etb_set_buffer(struct coresight_device *csdev,
> struct perf_output_handle *handle);
>
> -static inline unsigned int etb_get_buffer_depth(struct etb_drvdata *drvdata)
> +static unsigned int etb_get_buffer_depth(struct etb_drvdata *drvdata)
> {
> return readl_relaxed(drvdata->base + ETB_RAM_DEPTH_REG);
> }
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index 3609bb74c4ec..9e7d423c1ae9 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -84,7 +84,7 @@ static int etm4_probe_cpu(unsigned int cpu);
> * TRCIDR4.NUMPC > 0b0000 .
> * TRCSSCSR<n>.PC == 0b1
> */
> -static inline bool etm4x_sspcicrn_present(struct etmv4_drvdata *drvdata, int n)
> +static bool etm4x_sspcicrn_present(struct etmv4_drvdata *drvdata, int n)
> {
> return (n < drvdata->nr_ss_cmp) &&
> drvdata->nr_pe &&
> @@ -185,7 +185,7 @@ static void etm_write_os_lock(struct etmv4_drvdata *drvdata,
> isb();
> }
>
> -static inline void etm4_os_unlock_csa(struct etmv4_drvdata *drvdata,
> +static void etm4_os_unlock_csa(struct etmv4_drvdata *drvdata,
> struct csdev_access *csa)
> {
> WARN_ON(drvdata->cpu != smp_processor_id());
> @@ -1035,7 +1035,7 @@ static const struct coresight_ops etm4_cs_ops = {
> .source_ops = &etm4_source_ops,
> };
>
> -static inline bool cpu_supports_sysreg_trace(void)
> +static bool cpu_supports_sysreg_trace(void)
> {
> u64 dfr0 = read_sysreg_s(SYS_ID_AA64DFR0_EL1);
>
> @@ -1343,7 +1343,7 @@ static void etm4_init_arch_data(void *info)
> cpu_detect_trace_filtering(drvdata);
> }
>
> -static inline u32 etm4_get_victlr_access_type(struct etmv4_config *config)
> +static u32 etm4_get_victlr_access_type(struct etmv4_config *config)
> {
> return etm4_get_access_type(config) << __bf_shf(TRCVICTLR_EXLEVEL_MASK);
> }
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> index a9f19629f3f8..0465c32f7ed4 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> @@ -2440,7 +2440,7 @@ static u32 etmv4_cross_read(const struct etmv4_drvdata *drvdata, u32 offset)
> return reg.data;
> }
>
> -static inline u32 coresight_etm4x_attr_to_offset(struct device_attribute *attr)
> +static u32 coresight_etm4x_attr_to_offset(struct device_attribute *attr)
> {
> struct dev_ext_attribute *eattr;
>
> @@ -2464,7 +2464,7 @@ static ssize_t coresight_etm4x_reg_show(struct device *dev,
> return scnprintf(buf, PAGE_SIZE, "0x%x\n", val);
> }
>
> -static inline bool
> +static bool
> etm4x_register_implemented(struct etmv4_drvdata *drvdata, u32 offset)
> {
> switch (offset) {
> diff --git a/drivers/hwtracing/coresight/coresight-platform.c b/drivers/hwtracing/coresight/coresight-platform.c
> index 8192ba3279f0..0db64c5f4995 100644
> --- a/drivers/hwtracing/coresight/coresight-platform.c
> +++ b/drivers/hwtracing/coresight/coresight-platform.c
> @@ -139,7 +139,7 @@ coresight_find_csdev_by_fwnode(struct fwnode_handle *r_fwnode)
> EXPORT_SYMBOL_GPL(coresight_find_csdev_by_fwnode);
>
> #ifdef CONFIG_OF
> -static inline bool of_coresight_legacy_ep_is_input(struct device_node *ep)
> +static bool of_coresight_legacy_ep_is_input(struct device_node *ep)
> {
> return of_property_read_bool(ep, "slave-mode");
> }
> @@ -159,7 +159,7 @@ static struct device_node *of_coresight_get_port_parent(struct device_node *ep)
> return parent;
> }
>
> -static inline struct device_node *
> +static struct device_node *
> of_coresight_get_output_ports_node(const struct device_node *node)
> {
> return of_get_child_by_name(node, "out-ports");
> @@ -327,14 +327,14 @@ static int of_get_coresight_platform_data(struct device *dev,
> return 0;
> }
> #else
> -static inline int
> +static int
> of_get_coresight_platform_data(struct device *dev,
> struct coresight_platform_data *pdata)
> {
> return -ENOENT;
> }
>
> -static inline int of_coresight_get_cpu(struct device *dev)
> +static int of_coresight_get_cpu(struct device *dev)
> {
> return -ENODEV;
> }
> @@ -356,7 +356,7 @@ static const guid_t coresight_graph_uuid = GUID_INIT(0x3ecbc8b6, 0x1d0e, 0x4fb3,
> #define ACPI_CORESIGHT_LINK_SLAVE 0
> #define ACPI_CORESIGHT_LINK_MASTER 1
>
> -static inline bool is_acpi_guid(const union acpi_object *obj)
> +static bool is_acpi_guid(const union acpi_object *obj)
> {
> return (obj->type == ACPI_TYPE_BUFFER) && (obj->buffer.length == 16);
> }
> @@ -365,24 +365,24 @@ static inline bool is_acpi_guid(const union acpi_object *obj)
> * acpi_guid_matches - Checks if the given object is a GUID object and
> * that it matches the supplied the GUID.
> */
> -static inline bool acpi_guid_matches(const union acpi_object *obj,
> +static bool acpi_guid_matches(const union acpi_object *obj,
> const guid_t *guid)
> {
> return is_acpi_guid(obj) &&
> guid_equal((guid_t *)obj->buffer.pointer, guid);
> }
>
> -static inline bool is_acpi_dsd_graph_guid(const union acpi_object *obj)
> +static bool is_acpi_dsd_graph_guid(const union acpi_object *obj)
> {
> return acpi_guid_matches(obj, &acpi_graph_uuid);
> }
>
> -static inline bool is_acpi_coresight_graph_guid(const union acpi_object *obj)
> +static bool is_acpi_coresight_graph_guid(const union acpi_object *obj)
> {
> return acpi_guid_matches(obj, &coresight_graph_uuid);
> }
>
> -static inline bool is_acpi_coresight_graph(const union acpi_object *obj)
> +static bool is_acpi_coresight_graph(const union acpi_object *obj)
> {
> const union acpi_object *graphid, *guid, *links;
>
> @@ -469,7 +469,7 @@ static inline bool is_acpi_coresight_graph(const union acpi_object *obj)
> * }, // End of ACPI Graph Property
> * })
> */
> -static inline bool acpi_validate_dsd_graph(const union acpi_object *graph)
> +static bool acpi_validate_dsd_graph(const union acpi_object *graph)
> {
> int i, n;
> const union acpi_object *rev, *nr_graphs;
> @@ -553,7 +553,7 @@ acpi_get_dsd_graph(struct acpi_device *adev, struct acpi_buffer *buf)
> return NULL;
> }
>
> -static inline bool
> +static bool
> acpi_validate_coresight_graph(const union acpi_object *cs_graph)
> {
> int nlinks;
> @@ -794,14 +794,14 @@ acpi_get_coresight_platform_data(struct device *dev,
>
> #else
>
> -static inline int
> +static int
> acpi_get_coresight_platform_data(struct device *dev,
> struct coresight_platform_data *pdata)
> {
> return -ENOENT;
> }
>
> -static inline int acpi_coresight_get_cpu(struct device *dev)
> +static int acpi_coresight_get_cpu(struct device *dev)
> {
> return -ENODEV;
> }
> diff --git a/drivers/hwtracing/coresight/coresight-replicator.c b/drivers/hwtracing/coresight/coresight-replicator.c
> index 679a36effbe8..9f27374f1310 100644
> --- a/drivers/hwtracing/coresight/coresight-replicator.c
> +++ b/drivers/hwtracing/coresight/coresight-replicator.c
> @@ -63,7 +63,7 @@ static void dynamic_replicator_reset(struct replicator_drvdata *drvdata)
> /*
> * replicator_reset : Reset the replicator configuration to sane values.
> */
> -static inline void replicator_reset(struct replicator_drvdata *drvdata)
> +static void replicator_reset(struct replicator_drvdata *drvdata)
> {
> if (drvdata->base)
> dynamic_replicator_reset(drvdata);
> diff --git a/drivers/hwtracing/coresight/coresight-stm.c b/drivers/hwtracing/coresight/coresight-stm.c
> index b581a30a1cd9..5decc87d1fba 100644
> --- a/drivers/hwtracing/coresight/coresight-stm.c
> +++ b/drivers/hwtracing/coresight/coresight-stm.c
> @@ -290,7 +290,7 @@ static const struct coresight_ops stm_cs_ops = {
> .source_ops = &stm_source_ops,
> };
>
> -static inline bool stm_addr_unaligned(const void *addr, u8 write_bytes)
> +static bool stm_addr_unaligned(const void *addr, u8 write_bytes)
> {
> return ((unsigned long)addr & (write_bytes - 1));
> }
> @@ -674,7 +674,7 @@ static int of_stm_get_stimulus_area(struct device *dev, struct resource *res)
> return of_address_to_resource(np, index, res);
> }
> #else
> -static inline int of_stm_get_stimulus_area(struct device *dev,
> +static int of_stm_get_stimulus_area(struct device *dev,
> struct resource *res)
> {
> return -ENOENT;
> @@ -718,7 +718,7 @@ static int acpi_stm_get_stimulus_area(struct device *dev, struct resource *res)
> return rc;
> }
> #else
> -static inline int acpi_stm_get_stimulus_area(struct device *dev,
> +static int acpi_stm_get_stimulus_area(struct device *dev,
> struct resource *res)
> {
> return -ENOENT;
> diff --git a/drivers/hwtracing/coresight/coresight-syscfg-configfs.c b/drivers/hwtracing/coresight/coresight-syscfg-configfs.c
> index 213b4159b062..2b40e556be87 100644
> --- a/drivers/hwtracing/coresight/coresight-syscfg-configfs.c
> +++ b/drivers/hwtracing/coresight/coresight-syscfg-configfs.c
> @@ -10,7 +10,7 @@
> #include "coresight-syscfg-configfs.h"
>
> /* create a default ci_type. */
> -static inline struct config_item_type *cscfg_create_ci_type(void)
> +static struct config_item_type *cscfg_create_ci_type(void)
> {
> struct config_item_type *ci_type;
>
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c
> index 5ac4e3c706ac..2e3354b5c5c7 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-core.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-core.c
> @@ -164,8 +164,8 @@ static int tmc_open(struct inode *inode, struct file *file)
> return 0;
> }
>
> -static inline ssize_t tmc_get_sysfs_trace(struct tmc_drvdata *drvdata,
> - loff_t pos, size_t len, char **bufpp)
> +static ssize_t tmc_get_sysfs_trace(struct tmc_drvdata *drvdata, loff_t pos, size_t len,
> + char **bufpp)
> {
> switch (drvdata->config_type) {
> case TMC_CONFIG_TYPE_ETB:
> @@ -359,7 +359,7 @@ static const struct attribute_group *coresight_etr_groups[] = {
> NULL,
> };
>
> -static inline bool tmc_etr_can_use_sg(struct device *dev)
> +static bool tmc_etr_can_use_sg(struct device *dev)
> {
> int ret;
> u8 val_u8;
> @@ -389,7 +389,7 @@ static inline bool tmc_etr_can_use_sg(struct device *dev)
> return false;
> }
>
> -static inline bool tmc_etr_has_non_secure_access(struct tmc_drvdata *drvdata)
> +static bool tmc_etr_has_non_secure_access(struct tmc_drvdata *drvdata)
> {
> u32 auth = readl_relaxed(drvdata->base + TMC_AUTHSTATUS);
>
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> index 7ba4f79191de..0d4e3af0d4b5 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> @@ -124,7 +124,7 @@ struct etr_sg_table {
> * If we spill over to a new page for mapping 1 entry, we could as
> * well replace the link entry of the previous page with the last entry.
> */
> -static inline unsigned long __attribute_const__
> +static unsigned long __attribute_const__
> tmc_etr_sg_table_entries(int nr_pages)
> {
> unsigned long nr_sgpages = nr_pages * ETR_SG_PAGES_PER_SYSPAGE;
> @@ -238,13 +238,13 @@ static int tmc_pages_alloc(struct tmc_pages *tmc_pages,
> return -ENOMEM;
> }
>
> -static inline long
> +static long
> tmc_sg_get_data_page_offset(struct tmc_sg_table *sg_table, dma_addr_t addr)
> {
> return tmc_pages_get_offset(&sg_table->data_pages, addr);
> }
>
> -static inline void tmc_free_table_pages(struct tmc_sg_table *sg_table)
> +static void tmc_free_table_pages(struct tmc_sg_table *sg_table)
> {
> if (sg_table->table_vaddr)
> vunmap(sg_table->table_vaddr);
> @@ -480,7 +480,7 @@ static void tmc_etr_sg_table_dump(struct etr_sg_table *etr_table)
> dev_dbg(sg_table->dev, "******* End of Table *****\n");
> }
> #else
> -static inline void tmc_etr_sg_table_dump(struct etr_sg_table *etr_table) {}
> +static void tmc_etr_sg_table_dump(struct etr_sg_table *etr_table) {}
> #endif
>
> /*
> @@ -815,10 +815,8 @@ void tmc_etr_remove_catu_ops(void)
> }
> EXPORT_SYMBOL_GPL(tmc_etr_remove_catu_ops);
>
> -static inline int tmc_etr_mode_alloc_buf(int mode,
> - struct tmc_drvdata *drvdata,
> - struct etr_buf *etr_buf, int node,
> - void **pages)
> +static int tmc_etr_mode_alloc_buf(int mode, struct tmc_drvdata *drvdata, struct etr_buf *etr_buf,
> + int node, void **pages)
> {
> int rc = -EINVAL;
>
> @@ -936,7 +934,7 @@ static ssize_t tmc_etr_buf_get_data(struct etr_buf *etr_buf,
> return etr_buf->ops->get_data(etr_buf, (u64)offset, len, bufpp);
> }
>
> -static inline s64
> +static s64
> tmc_etr_buf_insert_barrier_packet(struct etr_buf *etr_buf, u64 offset)
> {
> ssize_t len;
> diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
> index 919804b12a67..e9a19ef52359 100644
> --- a/drivers/hwtracing/coresight/coresight-trbe.c
> +++ b/drivers/hwtracing/coresight/coresight-trbe.c
> @@ -159,22 +159,22 @@ static void trbe_check_errata(struct trbe_cpudata *cpudata)
> }
> }
>
> -static inline bool trbe_has_erratum(struct trbe_cpudata *cpudata, int i)
> +static bool trbe_has_erratum(struct trbe_cpudata *cpudata, int i)
> {
> return (i < TRBE_ERRATA_MAX) && test_bit(i, cpudata->errata);
> }
>
> -static inline bool trbe_may_overwrite_in_fill_mode(struct trbe_cpudata *cpudata)
> +static bool trbe_may_overwrite_in_fill_mode(struct trbe_cpudata *cpudata)
> {
> return trbe_has_erratum(cpudata, TRBE_WORKAROUND_OVERWRITE_FILL_MODE);
> }
>
> -static inline bool trbe_may_write_out_of_range(struct trbe_cpudata *cpudata)
> +static bool trbe_may_write_out_of_range(struct trbe_cpudata *cpudata)
> {
> return trbe_has_erratum(cpudata, TRBE_WORKAROUND_WRITE_OUT_OF_RANGE);
> }
>
> -static inline bool trbe_needs_drain_after_disable(struct trbe_cpudata *cpudata)
> +static bool trbe_needs_drain_after_disable(struct trbe_cpudata *cpudata)
> {
> /*
> * Errata affected TRBE implementation will need TSB CSYNC and
> @@ -184,7 +184,7 @@ static inline bool trbe_needs_drain_after_disable(struct trbe_cpudata *cpudata)
> return trbe_has_erratum(cpudata, TRBE_NEEDS_DRAIN_AFTER_DISABLE);
> }
>
> -static inline bool trbe_needs_ctxt_sync_after_enable(struct trbe_cpudata *cpudata)
> +static bool trbe_needs_ctxt_sync_after_enable(struct trbe_cpudata *cpudata)
> {
> /*
> * Errata affected TRBE implementation will need an additional
> @@ -195,7 +195,7 @@ static inline bool trbe_needs_ctxt_sync_after_enable(struct trbe_cpudata *cpudat
> return trbe_has_erratum(cpudata, TRBE_NEEDS_CTXT_SYNC_AFTER_ENABLE);
> }
>
> -static inline bool trbe_is_broken(struct trbe_cpudata *cpudata)
> +static bool trbe_is_broken(struct trbe_cpudata *cpudata)
> {
> return trbe_has_erratum(cpudata, TRBE_IS_BROKEN);
> }
> @@ -207,13 +207,13 @@ static int trbe_alloc_node(struct perf_event *event)
> return cpu_to_node(event->cpu);
> }
>
> -static inline void trbe_drain_buffer(void)
> +static void trbe_drain_buffer(void)
> {
> tsb_csync();
> dsb(nsh);
> }
>
> -static inline void set_trbe_enabled(struct trbe_cpudata *cpudata, u64 trblimitr)
> +static void set_trbe_enabled(struct trbe_cpudata *cpudata, u64 trblimitr)
> {
> /*
> * Enable the TRBE without clearing LIMITPTR which
> @@ -229,7 +229,7 @@ static inline void set_trbe_enabled(struct trbe_cpudata *cpudata, u64 trblimitr)
> isb();
> }
>
> -static inline void set_trbe_disabled(struct trbe_cpudata *cpudata)
> +static void set_trbe_disabled(struct trbe_cpudata *cpudata)
> {
> u64 trblimitr = read_sysreg_s(SYS_TRBLIMITR_EL1);
>
> --
> 2.34.1
>
> _______________________________________________
> CoreSight mailing list -- coresight@lists.linaro.org
> To unsubscribe send an email to coresight-leave@lists.linaro.org
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 6/7] coresight: Remove inlines from static function definitions
2025-03-14 9:50 ` Leo Yan
@ 2025-03-17 15:26 ` James Clark
2025-03-17 17:45 ` Leo Yan
0 siblings, 1 reply; 25+ messages in thread
From: James Clark @ 2025-03-17 15:26 UTC (permalink / raw)
To: Leo Yan
Cc: lcherian, coresight, Mike Leach, Alexander Shishkin,
Maxime Coquelin, Alexandre Torgue, linux-arm-kernel, linux-kernel,
linux-stm32
On 14/03/2025 9:50 am, Leo Yan wrote:
> On Tue, Feb 11, 2025 at 10:39:42AM +0000, James Clark wrote:
>>
>> These are all static and in one compilation unit so the inline has no
>> effect on the binary.
>
> I read the doc coding-style.rst, the section "15) The inline disease",
> my understanding is though the inline is not encouraged, I prefer we can
> follow up the rule:
>
> A reasonable rule of thumb is to not put inline at functions that have more
> than 3 lines of code in them. An exception to this rule are the cases where
> a parameter is known to be a compile time constant, and as a result of this
> constantness you *know* the compiler will be able to optimize most of your
> function away at compile time. For a good example of this later case, see
> the kmalloc() inline function.
>
Isn't this advice about whether to make an inline function in a header
file or do definition + declaration? I only removed static ones local to
.c files where 'inline' has no effect at all. We build with optimisation
enabled so the compiler already auto inlines small functions and ignores
the inline keyword for large re-used ones anyway.
>> Except if FTRACE is enabled, then some functions
>> which were already not inlined now get the nops added which allows them
>> to be traced.
>
> I understand it is a side effect that we cannot use function tracer for
> inline functions. This is not a big issue for me, as we still can use
> "perf probe" and trace_printk() for debugging.
>
> Thanks,
> Leo
>
I think it can be useful, I've used it a little bit. Might as well make
it easier to use for someone if it exists.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 6/7] coresight: Remove inlines from static function definitions
2025-03-17 15:26 ` James Clark
@ 2025-03-17 17:45 ` Leo Yan
0 siblings, 0 replies; 25+ messages in thread
From: Leo Yan @ 2025-03-17 17:45 UTC (permalink / raw)
To: James Clark
Cc: lcherian, coresight, Mike Leach, Alexander Shishkin,
Maxime Coquelin, Alexandre Torgue, linux-arm-kernel, linux-kernel,
linux-stm32
On Mon, Mar 17, 2025 at 03:26:42PM +0000, James Clark wrote:
> On 14/03/2025 9:50 am, Leo Yan wrote:
> > On Tue, Feb 11, 2025 at 10:39:42AM +0000, James Clark wrote:
> > >
> > > These are all static and in one compilation unit so the inline has no
> > > effect on the binary.
> >
> > I read the doc coding-style.rst, the section "15) The inline disease",
> > my understanding is though the inline is not encouraged, I prefer we can
> > follow up the rule:
> >
> > A reasonable rule of thumb is to not put inline at functions that have more
> > than 3 lines of code in them. An exception to this rule are the cases where
> > a parameter is known to be a compile time constant, and as a result of this
> > constantness you *know* the compiler will be able to optimize most of your
> > function away at compile time. For a good example of this later case, see
> > the kmalloc() inline function.
> >
>
> Isn't this advice about whether to make an inline function in a header file
> or do definition + declaration?
Maybe it is not about where defines inline functions. It is about
how these inline functions are invoked. E.g., in a single C file, a
inline function can be invoked in multiple places, and it is possible
the incline function can optimized by compiler case by case.
To be honest, I don't really understand how compiler works :)
[...]
> I think it can be useful, I've used it a little bit. Might as well make it
> easier to use for someone if it exists.
Sure. This patch is fine for me. I verified this patch with default
kernel configuration (-O2, -g), it does works as expected (no code and
data section's size change between with and without this patch).
Thanks,
Leo
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 7/7] coresight: Remove extern from function declarations
2025-02-11 10:39 [PATCH 0/7] coresight: Clear self hosted claim tag on probe James Clark
` (5 preceding siblings ...)
2025-02-11 10:39 ` [PATCH 6/7] coresight: Remove inlines from static function definitions James Clark
@ 2025-02-11 10:39 ` James Clark
2025-03-13 16:17 ` Leo Yan
6 siblings, 1 reply; 25+ messages in thread
From: James Clark @ 2025-02-11 10:39 UTC (permalink / raw)
To: lcherian, coresight
Cc: James Clark, Suzuki K Poulose, Mike Leach, Alexander Shishkin,
Maxime Coquelin, Alexandre Torgue, linux-arm-kernel, linux-kernel,
linux-stm32
Function declarations are extern by default so remove the extra noise
and inconsistency.
Signed-off-by: James Clark <james.clark@linaro.org>
---
drivers/hwtracing/coresight/coresight-priv.h | 14 ++++-----
include/linux/coresight.h | 33 +++++++++-----------
2 files changed, 21 insertions(+), 26 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
index a83113225797..e09071a720f0 100644
--- a/drivers/hwtracing/coresight/coresight-priv.h
+++ b/drivers/hwtracing/coresight/coresight-priv.h
@@ -57,10 +57,8 @@ struct cs_off_attribute {
u32 off;
};
-extern ssize_t coresight_simple_show32(struct device *_dev,
- struct device_attribute *attr, char *buf);
-extern ssize_t coresight_simple_show_pair(struct device *_dev,
- struct device_attribute *attr, char *buf);
+ssize_t coresight_simple_show32(struct device *_dev, struct device_attribute *attr, char *buf);
+ssize_t coresight_simple_show_pair(struct device *_dev, struct device_attribute *attr, char *buf);
#define coresight_simple_reg32(name, offset) \
(&((struct cs_off_attribute[]) { \
@@ -155,8 +153,8 @@ void coresight_remove_links(struct coresight_device *orig,
u32 coresight_get_sink_id(struct coresight_device *csdev);
#if IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM3X)
-extern int etm_readl_cp14(u32 off, unsigned int *val);
-extern int etm_writel_cp14(u32 off, u32 val);
+int etm_readl_cp14(u32 off, unsigned int *val);
+int etm_writel_cp14(u32 off, u32 val);
#else
static inline int etm_readl_cp14(u32 off, unsigned int *val) { return 0; }
static inline int etm_writel_cp14(u32 off, u32 val) { return 0; }
@@ -167,8 +165,8 @@ struct cti_assoc_op {
void (*remove)(struct coresight_device *csdev);
};
-extern void coresight_set_cti_ops(const struct cti_assoc_op *cti_op);
-extern void coresight_remove_cti_ops(void);
+void coresight_set_cti_ops(const struct cti_assoc_op *cti_op);
+void coresight_remove_cti_ops(void);
/*
* Macros and inline functions to handle CoreSight UCI data and driver
diff --git a/include/linux/coresight.h b/include/linux/coresight.h
index 11808aee9d1d..2b43698c0b25 100644
--- a/include/linux/coresight.h
+++ b/include/linux/coresight.h
@@ -642,26 +642,23 @@ static inline void coresight_set_mode(struct coresight_device *csdev,
local_set(&csdev->mode, new_mode);
}
-extern struct coresight_device *
-coresight_register(struct coresight_desc *desc);
-extern void coresight_unregister(struct coresight_device *csdev);
-extern int coresight_enable_sysfs(struct coresight_device *csdev);
-extern void coresight_disable_sysfs(struct coresight_device *csdev);
-extern int coresight_timeout(struct csdev_access *csa, u32 offset,
- int position, int value);
-
-extern int coresight_claim_device(struct coresight_device *csdev);
-extern int coresight_claim_device_unlocked(struct coresight_device *csdev);
-
-extern void coresight_disclaim_device(struct csdev_access *csa);
-extern void coresight_disclaim_device_unlocked(struct csdev_access *csa);
+struct coresight_device *coresight_register(struct coresight_desc *desc);
+void coresight_unregister(struct coresight_device *csdev);
+int coresight_enable_sysfs(struct coresight_device *csdev);
+void coresight_disable_sysfs(struct coresight_device *csdev);
+int coresight_timeout(struct csdev_access *csa, u32 offset, int position, int value);
+
+int coresight_claim_device(struct coresight_device *csdev);
+int coresight_claim_device_unlocked(struct coresight_device *csdev);
+
+void coresight_disclaim_device(struct csdev_access *csa);
+void coresight_disclaim_device_unlocked(struct csdev_access *csa);
int coresight_reset_claim_unlocked(struct csdev_access *csa);
int coresight_reset_claim(struct csdev_access *csa);
-extern char *coresight_alloc_device_name(struct coresight_dev_list *devs,
- struct device *dev);
+char *coresight_alloc_device_name(struct coresight_dev_list *devs, struct device *dev);
-extern bool coresight_loses_context_with_cpu(struct device *dev);
+bool coresight_loses_context_with_cpu(struct device *dev);
u32 coresight_relaxed_read32(struct coresight_device *csdev, u32 offset);
u32 coresight_read32(struct coresight_device *csdev, u32 offset);
@@ -674,8 +671,8 @@ void coresight_relaxed_write64(struct coresight_device *csdev,
u64 val, u32 offset);
void coresight_write64(struct coresight_device *csdev, u64 val, u32 offset);
-extern int coresight_get_cpu(struct device *dev);
-extern int coresight_get_static_trace_id(struct device *dev, u32 *id);
+int coresight_get_cpu(struct device *dev);
+int coresight_get_static_trace_id(struct device *dev, u32 *id);
struct coresight_platform_data *coresight_get_platform_data(struct device *dev);
struct coresight_connection *
--
2.34.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 7/7] coresight: Remove extern from function declarations
2025-02-11 10:39 ` [PATCH 7/7] coresight: Remove extern from function declarations James Clark
@ 2025-03-13 16:17 ` Leo Yan
0 siblings, 0 replies; 25+ messages in thread
From: Leo Yan @ 2025-03-13 16:17 UTC (permalink / raw)
To: James Clark
Cc: lcherian, coresight, Mike Leach, Alexander Shishkin,
Maxime Coquelin, Alexandre Torgue, linux-arm-kernel, linux-kernel,
linux-stm32
On Tue, Feb 11, 2025 at 10:39:43AM +0000, James Clark wrote:
>
> Function declarations are extern by default so remove the extra noise
> and inconsistency.
This change is consistent with the coding-style.rst:
"Do not use the ``extern`` keyword with function declarations as
this makes lines longer and isn't strictly necessary."
Reviewed-by: Leo Yan <leo.yan@arm.com>
> Signed-off-by: James Clark <james.clark@linaro.org>
> ---
> drivers/hwtracing/coresight/coresight-priv.h | 14 ++++-----
> include/linux/coresight.h | 33 +++++++++-----------
> 2 files changed, 21 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
> index a83113225797..e09071a720f0 100644
> --- a/drivers/hwtracing/coresight/coresight-priv.h
> +++ b/drivers/hwtracing/coresight/coresight-priv.h
> @@ -57,10 +57,8 @@ struct cs_off_attribute {
> u32 off;
> };
>
> -extern ssize_t coresight_simple_show32(struct device *_dev,
> - struct device_attribute *attr, char *buf);
> -extern ssize_t coresight_simple_show_pair(struct device *_dev,
> - struct device_attribute *attr, char *buf);
> +ssize_t coresight_simple_show32(struct device *_dev, struct device_attribute *attr, char *buf);
> +ssize_t coresight_simple_show_pair(struct device *_dev, struct device_attribute *attr, char *buf);
>
> #define coresight_simple_reg32(name, offset) \
> (&((struct cs_off_attribute[]) { \
> @@ -155,8 +153,8 @@ void coresight_remove_links(struct coresight_device *orig,
> u32 coresight_get_sink_id(struct coresight_device *csdev);
>
> #if IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM3X)
> -extern int etm_readl_cp14(u32 off, unsigned int *val);
> -extern int etm_writel_cp14(u32 off, u32 val);
> +int etm_readl_cp14(u32 off, unsigned int *val);
> +int etm_writel_cp14(u32 off, u32 val);
> #else
> static inline int etm_readl_cp14(u32 off, unsigned int *val) { return 0; }
> static inline int etm_writel_cp14(u32 off, u32 val) { return 0; }
> @@ -167,8 +165,8 @@ struct cti_assoc_op {
> void (*remove)(struct coresight_device *csdev);
> };
>
> -extern void coresight_set_cti_ops(const struct cti_assoc_op *cti_op);
> -extern void coresight_remove_cti_ops(void);
> +void coresight_set_cti_ops(const struct cti_assoc_op *cti_op);
> +void coresight_remove_cti_ops(void);
>
> /*
> * Macros and inline functions to handle CoreSight UCI data and driver
> diff --git a/include/linux/coresight.h b/include/linux/coresight.h
> index 11808aee9d1d..2b43698c0b25 100644
> --- a/include/linux/coresight.h
> +++ b/include/linux/coresight.h
> @@ -642,26 +642,23 @@ static inline void coresight_set_mode(struct coresight_device *csdev,
> local_set(&csdev->mode, new_mode);
> }
>
> -extern struct coresight_device *
> -coresight_register(struct coresight_desc *desc);
> -extern void coresight_unregister(struct coresight_device *csdev);
> -extern int coresight_enable_sysfs(struct coresight_device *csdev);
> -extern void coresight_disable_sysfs(struct coresight_device *csdev);
> -extern int coresight_timeout(struct csdev_access *csa, u32 offset,
> - int position, int value);
> -
> -extern int coresight_claim_device(struct coresight_device *csdev);
> -extern int coresight_claim_device_unlocked(struct coresight_device *csdev);
> -
> -extern void coresight_disclaim_device(struct csdev_access *csa);
> -extern void coresight_disclaim_device_unlocked(struct csdev_access *csa);
> +struct coresight_device *coresight_register(struct coresight_desc *desc);
> +void coresight_unregister(struct coresight_device *csdev);
> +int coresight_enable_sysfs(struct coresight_device *csdev);
> +void coresight_disable_sysfs(struct coresight_device *csdev);
> +int coresight_timeout(struct csdev_access *csa, u32 offset, int position, int value);
> +
> +int coresight_claim_device(struct coresight_device *csdev);
> +int coresight_claim_device_unlocked(struct coresight_device *csdev);
> +
> +void coresight_disclaim_device(struct csdev_access *csa);
> +void coresight_disclaim_device_unlocked(struct csdev_access *csa);
> int coresight_reset_claim_unlocked(struct csdev_access *csa);
> int coresight_reset_claim(struct csdev_access *csa);
>
> -extern char *coresight_alloc_device_name(struct coresight_dev_list *devs,
> - struct device *dev);
> +char *coresight_alloc_device_name(struct coresight_dev_list *devs, struct device *dev);
>
> -extern bool coresight_loses_context_with_cpu(struct device *dev);
> +bool coresight_loses_context_with_cpu(struct device *dev);
>
> u32 coresight_relaxed_read32(struct coresight_device *csdev, u32 offset);
> u32 coresight_read32(struct coresight_device *csdev, u32 offset);
> @@ -674,8 +671,8 @@ void coresight_relaxed_write64(struct coresight_device *csdev,
> u64 val, u32 offset);
> void coresight_write64(struct coresight_device *csdev, u64 val, u32 offset);
>
> -extern int coresight_get_cpu(struct device *dev);
> -extern int coresight_get_static_trace_id(struct device *dev, u32 *id);
> +int coresight_get_cpu(struct device *dev);
> +int coresight_get_static_trace_id(struct device *dev, u32 *id);
>
> struct coresight_platform_data *coresight_get_platform_data(struct device *dev);
> struct coresight_connection *
> --
> 2.34.1
>
> _______________________________________________
> CoreSight mailing list -- coresight@lists.linaro.org
> To unsubscribe send an email to coresight-leave@lists.linaro.org
^ permalink raw reply [flat|nested] 25+ messages in thread