Hi Levi,
On Fri, Jun 27, 2025 at 02:49:19PM +0100, Yeoreum Yun wrote:
> >
> > > @@ -789,6 +789,10 @@ static int __tmc_probe(struct device *dev, struct resource *res)
> > > struct coresight_desc desc = { 0 };
> > > struct coresight_dev_list *dev_list = NULL;
> > >
> > > + drvdata->atclk = devm_clk_get_optional_enabled(dev, "atclk");
> > > + if (IS_ERR(drvdata->atclk))
> > > + return PTR_ERR(drvdata->atclk);
> > > +
> > > ret = -ENOMEM;
> > >
> >
> > Just another quetion.
> >
> > If this function is called from tmc_platform_probe() and failed,
> > should it call the clk_put() for drvdata->pclk when it failed?
>
> Sorry, I missed the Patch #7.
No worries.
devm_clk_release() is a registered callback used by the device model
layer to release resources. The clock will be released in the flow:
devm_clk_release()
`> clk_put()
`> free_clk()
Thanks,
Leo
This series fixes and improves clock usage in the Arm CoreSight drivers.
Based on the DT binding documents, the trace clock (atclk) is defined in
some CoreSight modules, but support is absent. In most cases, the issue
is hidden because the atclk clock is shared by multiple CoreSight
modules and the clock is enabled anyway by other drivers. The first
three patches address this issue.
The programming clock (pclk) management in CoreSight drivers does not
use the devm_XXX() variant APIs, the drivers needs to manually disable
and release clocks for errors and for normal module exit. However, the
drivers miss to disable clocks during module exit. The atclk may also
not be disabled in CoreSight drivers during module exit. By using devm
APIs, patches 04 and 05 fix clock disabling issues.
Another issue is pclk might be enabled twice in init phase - once by
AMBA bus driver, and again by CoreSight drivers. This is fixed in
patch 06.
Patches 07 to 10 refactor the clock related code. Patch 07 consolidates
the clock initialization into a central place. Patch 08 polishes driver
data allocation. Patch 09 makes the clock enabling sequence consistent.
Patch 09 removes redundant condition checks and adds error handling in
runtime PM.
This series has been verified on Arm64 Hikey960 and Juno platforms.
Changes from v3:
- Separated patch 07 into two patches, one is for clock consolidation
and another is for polishing driver data allocation (Anshuman).
Changes from v2:
- Updated subjects for patches 04 and 05 (Anshuman).
- Refined condition checking "if (dev_is_amba(dev))" in patch 07
(Anshuman).
Changes from v1:
- Moved the coresight_get_enable_clocks() function into CoreSight core
layer (James).
- Added comments for clock naming "apb_pclk" and "apb" (James).
- Re-ordered patches for easier understanding (Anshuman).
- Minor improvement for commit log in patch 01 (Anshuman).
Signed-off-by: Leo Yan <leo.yan(a)arm.com>
---
Leo Yan (10):
coresight: tmc: Support atclk
coresight: catu: Support atclk
coresight: etm4x: Support atclk
coresight: Appropriately disable programming clocks
coresight: Appropriately disable trace bus clocks
coresight: Avoid enable programming clock duplicately
coresight: Consolidate clock enabling
coresight: Refactor driver data allocation
coresight: Make clock sequence consistent
coresight: Refactor runtime PM
drivers/hwtracing/coresight/coresight-catu.c | 53 ++++++++---------
drivers/hwtracing/coresight/coresight-catu.h | 1 +
drivers/hwtracing/coresight/coresight-core.c | 46 +++++++++++++++
drivers/hwtracing/coresight/coresight-cpu-debug.c | 41 +++++---------
drivers/hwtracing/coresight/coresight-ctcu-core.c | 24 +++-----
drivers/hwtracing/coresight/coresight-etb10.c | 18 ++----
drivers/hwtracing/coresight/coresight-etm3x-core.c | 17 ++----
drivers/hwtracing/coresight/coresight-etm4x-core.c | 32 ++++++-----
drivers/hwtracing/coresight/coresight-etm4x.h | 4 +-
drivers/hwtracing/coresight/coresight-funnel.c | 66 ++++++++--------------
drivers/hwtracing/coresight/coresight-replicator.c | 63 ++++++++-------------
drivers/hwtracing/coresight/coresight-stm.c | 34 +++++------
drivers/hwtracing/coresight/coresight-tmc-core.c | 48 ++++++++--------
drivers/hwtracing/coresight/coresight-tmc.h | 2 +
drivers/hwtracing/coresight/coresight-tpiu.c | 36 +++++-------
include/linux/coresight.h | 30 +---------
16 files changed, 226 insertions(+), 289 deletions(-)
---
base-commit: 67a993863163cb88b1b68974c31b0d84ece4293e
change-id: 20250627-arm_cs_fix_clock_v4-e24b1e1f8920
Best regards,
--
Leo Yan <leo.yan(a)arm.com>
On 2025/6/18 11:16, Bjorn Andersson wrote:
> On Thu, Apr 24, 2025 at 04:58:52AM -0700, Mao Jinlong wrote:
>> qcom,qmi-id is required for remote etm driver to find the remote
>> subsystem connection. It is the instance id used by qmi API to
>> communicate with remote processor.
>>
>> Signed-off-by: Mao Jinlong <quic_jinlmao(a)quicinc.com>
>> ---
>> .../bindings/arm/qcom,coresight-remote-etm.yaml | 10 ++++++++++
>> 1 file changed, 10 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/qcom,coresight-remote-etm.yaml b/Documentation/devicetree/bindings/arm/qcom,coresight-remote-etm.yaml
>> index 4fd5752978cd..947fe33738a3 100644
>> --- a/Documentation/devicetree/bindings/arm/qcom,coresight-remote-etm.yaml
>> +++ b/Documentation/devicetree/bindings/arm/qcom,coresight-remote-etm.yaml
>> @@ -20,6 +20,13 @@ properties:
>> compatible:
>> const: qcom,coresight-remote-etm
>>
>> + qcom,qmi-id:
>
> Why isn't this "qcom,qmi-instance-id" if that's what it represents?
ok.
>
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + description:
>> + This id is used by qmi API to communicate with remote processor for
>> + enabling and disabling remote etm. Each processor has its unique instance
>> + id.
>
> DeviceTree describes the hardware and firmware interface, so don't
> describe properties in terms of what Linux will do with this value, but
> what it represents.
Sure. I will update it.
>
>> +
>> out-ports:
>> $ref: /schemas/graph.yaml#/properties/ports
>> additionalProperties: false
>> @@ -32,6 +39,7 @@ properties:
>> required:
>> - compatible
>> - out-ports
>> + - qcom,qmi-id
>
> How can this suddenly be required, did devices described by this binding
> up until this point not work?
Without this instance id, remote etm won't work.
>
> If this is the case, make sure to clearly describe this in the commit
> message.
>
> Regards,
> Bjorn
>
>>
>> additionalProperties: false
>>
>> @@ -40,6 +48,8 @@ examples:
>> etm {
>> compatible = "qcom,coresight-remote-etm";
>>
>> + qcom,qmi-id = <2>;
>> +
>> out-ports {
>> port {
>> modem_etm0_out_funnel_modem: endpoint {
>> --
>> 2.25.1
>>
On 23/06/2025 9:59 am, Keita Morisaki wrote:
>> We have the ETM driver performing the save/restore of ETM context during
>> a CPUidle. This is only done when the ETM/ETE is described to be loosing
>> context over PM operation. If this is not done (via DT), the driver
>> doesn't do anything. This could be problematic. Could you try adding:
>>
>> "arm,coresight-loses-context-with-cpu"
>>
>>
>> property to the ETE nodes and see if it makes a difference ?
>
> I tried this in our environment, and this worked well. The "arm,coresight-loses-context-with-cpu" property was what we needed.
> Thank you so much again for the swift response with the useful information!
>
> Best,
> Keita
Hi Keita,
Thanks for the report. We discussed internally and decided that it would
be better for the driver to always save the context by default, because
this mistake is easy to make. Saving when it doesn't need to be saved
doesn't do any harm, but not saving when it should be causes quite bad bugs.
So "arm,coresight-loses-context-with-cpu" will be ignored in the future
and we'll add a new flag like "arm,coresight-save-context" if anyone
wants the optimization of not saving.
Thanks
James
Besides managing tracers (ETM) in CPU PM and hotplug flows, the
CoreSight framework is found the issues below:
Firstly, on some hardware platforms, CoreSight links (e.g., funnels and
replicators, etc) reside in a cluster power domain. If the cluster is
powered off, the link components also will lose their context. In this
case, Arm CoreSight drivers report errors when detect unpaired self-host
claim tags.
Secondly, if a path has been activated from per CPU's tracer (ETM) to
links and a sink in Sysfs mode, then when the CPU is hot-plugged off,
only the associated ETM will be disabled. Afterwards, the links and the
sink always keep on and no chance to be disabled.
The last issue was reported by Yabin Cui (Google) that the TRBE driver
misses to save and restore context during CPU low power states. As a
result, it may cause hardware lockup issue on some devices.
To resolve the power management issues, this series extends CPU power
management to cover the entire activated path, including links and
sinks. It moves CPU PM and hotplug notifiers from the ETMv4 driver to
the CoreSight core layer. The core layer has sufficient info to
maintain activated paths and can traverse the entire path to manipulate
CoreSight modules accordingly.
Patch 01 is to fix a bug in ETMv4 save and restore callbacks.
Patches 02 ~ 06 move CPU PM code from ETMv4 driver to the core layer, and
extends to maintain activated paths and control links.
Patches 07 and 08 support save and restore context for per-CPU sink
(TRBE). Note, for avoid long latency, system level's sinks in an
activated path are not touched during CPU suspend and resume.
Patches 09 ~ 11 move CPU hotplug notifier from ETMv4 driver to the core
layer. The entire path will be controlled if the corresponding CPU is
hot-plugged on or off.
This series has been verified on Hikey960 for CPUIdle and hotplug. And
it is tested on FVP for verifying TRBE with idle states.
Leo Yan (10):
coresight: etm4x: Control the trace unit in CPU suspend
coresight: Set per CPU source pointer
coresight: Register CPU PM notifier in core layer
coresight: etm4x: Hook CPU PM callbacks
coresight: Save activated path into source device
coresight: Control path during CPU PM
coresight: Add PM callbacks in sink operation
coresight: Take hotplug lock in enable_source_store() for Sysfs mode
coresight: Move CPU hotplug callbacks to core layer
coresight: Manage activated paths during CPU hotplug
Yabin Cui (1):
coresight: trbe: Save and restore state across CPU low power state
drivers/hwtracing/coresight/coresight-core.c | 252 +++++++++++++++++++++++++++++++++++++++++
drivers/hwtracing/coresight/coresight-etm3x-core.c | 2 +
drivers/hwtracing/coresight/coresight-etm4x-core.c | 133 ++++++----------------
drivers/hwtracing/coresight/coresight-priv.h | 1 +
drivers/hwtracing/coresight/coresight-sysfs.c | 12 +-
drivers/hwtracing/coresight/coresight-trbe.c | 65 +++++++++++
include/linux/coresight.h | 11 ++
7 files changed, 375 insertions(+), 101 deletions(-)
--
2.34.1
This series fixes and improves clock usage in the Arm CoreSight drivers.
Based on the DT binding documents, the trace clock (atclk) is defined in
some CoreSight modules, but support is absent. In most cases, the issue
is hidden because the atclk clock is shared by multiple CoreSight
modules and the clock is enabled anyway by other drivers. The first
three patches address this issue.
The programming clock (pclk) management in CoreSight drivers does not
use the devm_XXX() variant APIs, the drivers needs to manually disable
and release clocks for errors and for normal module exit. However, the
drivers miss to disable clocks during module exit. The atclk may also
not be disabled in CoreSight drivers during module exit. By using devm
APIs, patches 04 and 05 fix clock disabling issues.
Another issue is pclk might be enabled twice in init phase - once by
AMBA bus driver, and again by CoreSight drivers. This is fixed in
patch 06.
Patches 07 to 09 refactor the clock related code. Patch 07 consolidats
the clock initialization into a central place. Patch 08 makes the
clock enabling sequence consistent. Patch 09 removes redundant
condition checks and adds error handling in runtime PM.
This series has been verified on Arm64 Hikey960 and Juno platforms.
Changes from v2:
- Updated subjects for patches 04 and 05 (Anshuman).
- Refined condition checking "if (dev_is_amba(dev))" in patch 07
(Anshuman).
Changes from v1:
- Moved the coresight_get_enable_clocks() function into CoreSight core
layer (James).
- Added comments for clock naming "apb_pclk" and "apb" (James).
- Re-ordered patches for easier understanding (Anshuman).
- Minor improvement for commit log in patch 01 (Anshuman).
Signed-off-by: Leo Yan <leo.yan(a)arm.com>
---
Leo Yan (9):
coresight: tmc: Support atclk
coresight: catu: Support atclk
coresight: etm4x: Support atclk
coresight: Appropriately disable programming clocks
coresight: Appropriately disable trace bus clocks
coresight: Avoid enable programming clock duplicately
coresight: Consolidate clock enabling
coresight: Make clock sequence consistent
coresight: Refactor runtime PM
drivers/hwtracing/coresight/coresight-catu.c | 53 ++++++++---------
drivers/hwtracing/coresight/coresight-catu.h | 1 +
drivers/hwtracing/coresight/coresight-core.c | 45 +++++++++++++++
drivers/hwtracing/coresight/coresight-cpu-debug.c | 41 +++++---------
drivers/hwtracing/coresight/coresight-ctcu-core.c | 24 +++-----
drivers/hwtracing/coresight/coresight-etb10.c | 18 ++----
drivers/hwtracing/coresight/coresight-etm3x-core.c | 17 ++----
drivers/hwtracing/coresight/coresight-etm4x-core.c | 32 ++++++-----
drivers/hwtracing/coresight/coresight-etm4x.h | 4 +-
drivers/hwtracing/coresight/coresight-funnel.c | 66 ++++++++--------------
drivers/hwtracing/coresight/coresight-replicator.c | 63 ++++++++-------------
drivers/hwtracing/coresight/coresight-stm.c | 34 +++++------
drivers/hwtracing/coresight/coresight-tmc-core.c | 48 ++++++++--------
drivers/hwtracing/coresight/coresight-tmc.h | 2 +
drivers/hwtracing/coresight/coresight-tpiu.c | 36 +++++-------
include/linux/coresight.h | 30 +---------
16 files changed, 225 insertions(+), 289 deletions(-)
---
base-commit: 408c97c4a5e0b634dcd15bf8b8808b382e888164
change-id: 20250609-arm_cs_fix_clock_v3_public-d546e8bfc852
Best regards,
--
Leo Yan <leo.yan(a)arm.com>
Cc: coresight lists, Leo, James, Mike L
Hello !
Thanks for the report ! In the future, please use
scripts/get_maintainer.pl for the clear list of people/list
for reporting issues.
Response inline, below.
On 20/06/2025 08:21, Keita Morisaki wrote:
> Hello folks,
>
> I am writing to report a WARN_ON message I'm encountering in the
> CoreSight driver on a multi-core ARM system running a 6.12-based kernel.
> The warning appears consistently when disabling an Embedded Trace
> Extension (ETE) source after it has been active. The issue is not
> reproducible when CPUidle is disabled.
>
> The problem occurs because the driver assumes the CoreSight claim
> register is persistent, but it could be reset by the CPUidle power
> management flow. The section B2.3.2 of Arm CoreSight Architecture
> Specification v3.0[1] indicates that the claim register must reset at
> “reset”. A CPU power-up from an idle state can trigger a Cold reset,
> which might explain this behavior.
>
> My ftrace analysis confirms this. I traced the only two functions that
> modify the claim state: coresight_set_claim_tags (which sets the claim)
> and coresight_clear_claim_tags (which is the only part of the kernel
> that writes to CLAIMCLR). The trace shows the claim being set, followed
> by a CPUidle transition, but no subsequent call to
> coresight_clear_claim_tags.
>
> Here are the steps to reproduce the issue:
>
> modprobecoresight_etm4x
>
> # Enable any relevant sink
>
> echo1>/sys/bus/coresight/devices/ete0/enable_source
>
> echo0>/sys/bus/coresight/devices/ete0/enable_source
>
>
> Here is a relevant snippet from the ftrace log that illustrates the
> sequence:
>
> #tracer:function_graph
>
> #
>
> #CPUDURATIONFUNCTIONCALLS
>
> #|||||||
>
> 0)|coresight_claim_device_unlocked[coresight](){
>
> 0)3.750us|coresight_set_claim_tags[coresight]();//Claimissethere
>
> 0)+20.260us|}
>
> 0)|/*psci_domain_idle_enter:cpu_id=0state={Our PSCI parameter value}*///
> CPUgoesidle
>
> 0)|/*psci_domain_idle_exit:cpu_id=0state={Our PSCI parameter value}*///
> CPUwakesup,causingColdreset
>
> ...
>
> 0)(a)309346.3us|coresight_disclaim_device_unlocked[coresight]();//
> TriggersWARN_ON
>
>
> The following WARN_ON [2] is printed because the CLAIMCLR register has
> already been reset at the time coresight_disclaim_device_unlocked is
> called, contrary to the driver's expectation.
>
We have the ETM driver performing the save/restore of ETM context during
a CPUidle. This is only done when the ETM/ETE is described to be loosing
context over PM operation. If this is not done (via DT), the driver
doesn't do anything. This could be problematic. Could you try adding:
"arm,coresight-loses-context-with-cpu"
property to the ETE nodes and see if it makes a difference ?
Kind regards
Suzuki
[0]
https://elixir.bootlin.com/linux/v6.12/source/Documentation/devicetree/bind…
> [416.354181][C0]WARNING:CPU:0PID:0atdrivers/hwtracing/coresight/
> coresight-core.c:187coresight_disclaim_device_unlocked+0x84/0x9c[coresight]
>
> [416.535454][C0]Calltrace:
>
> [416.538606][C0]coresight_disclaim_device_unlocked+0x84/0x9c[coresight]
>
> [416.549359][C0]etm4_disable_hw+0x2d8/0x374[coresight_etm4x]
>
> [416.623310][C0]do_idle+0x1d4/0x264
>
> (Note on tracing: To get this detailed trace, I made two modifications
> to the kernel. First, since the trace_psci_domain_idle_enter/exit events
> are not available in kernel 6.12, I cherry-picked the upstream patch
> 7b7644831e72 [3] to add them. Second, to specifically trace the claim
> functions, I temporarily replaced their inline compiler hints with
> noinline.)
>
> Given the evidence, it appears the driver's assumption that the claim
> register is persistent across CPU power states is incorrect and may need
> to be addressed.
>
> Could you please provide your guidance on this?
>
> Thank you for your time and assistance.
>
> [1] https://developer.arm.com/documentation/ihi0029/latest/ <https://
> developer.arm.com/documentation/ihi0029/latest/>_
> _[2] https://elixir.bootlin.com/linux/v6.12/source/drivers/hwtracing/
> coresight/coresight-core.c#L187 <https://elixir.bootlin.com/linux/v6.12/
> source/drivers/hwtracing/coresight/coresight-core.c#L187>_
> _[3] https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/
> linux.git/commit/?id=7b7644831e7276f52a233ec685d13c965fff09d9 <https://
> web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?
> id=7b7644831e7276f52a233ec685d13c965fff09d9>
>
> Best regards,
> Keita