Hi Jie,
On Tue, 5 Aug 2025 at 05:11, Jie Gan <jie.gan(a)oss.qualcomm.com> wrote:
>
>
>
> On 7/28/2025 9:08 AM, Jie Gan wrote:
> >
> >
> > On 7/15/2025 8:41 AM, Jie Gan wrote:
> >>
> >>
> >> On 6/24/2025 5:59 PM, Jie Gan wrote:
> >>> Enable CTCU device for QCS8300 platform. Add a fallback mechnasim in
> >>> binding to utilize
> >>> the compitable of the SA8775p platform becuase the CTCU for QCS8300
> >>> shares same
> >>> configurations as SA8775p platform.
> >>
> >> Gentle ping.
> >
> > Gentle ping.
>
> Gentle ping.
> Hi Coresight maintainers,
>
> Can you please help to review the patch?
>
> Thanks,
> Jie
>
> >
> > Thanks,
> > Jie
> >
> >>
> >> Hi Suzuki, Mike, James, Rob
> >>
> >> Can you plz help to review the patch from Coresight view?
> >>
> >> Thanks,
> >> Jie
> >>
> >>>
> >>> Changes in V2:
> >>> 1. Add Krzysztof's R-B tag for dt-binding patch.
> >>> 2. Add Konrad's Acked-by tag for dt patch.
> >>> 3. Rebased on tag next-20250623.
> >>> 4. Missed email addresses for coresight's maintainers in V1, loop them.
> >>> Link to V1 - https://lore.kernel.org/all/20250327024943.3502313-1-
> >>> jie.gan(a)oss.qualcomm.com/
> >>>
> >>> Jie Gan (2):
> >>> dt-bindings: arm: add CTCU device for QCS8300
> >>> arm64: dts: qcom: qcs8300: Add CTCU and ETR nodes
> >>>
> >>> .../bindings/arm/qcom,coresight-ctcu.yaml | 9 +-
> >>> arch/arm64/boot/dts/qcom/qcs8300.dtsi | 153 ++++++++++++++++++
> >>> 2 files changed, 160 insertions(+), 2 deletions(-)
> >>>
> >>
> >>
> >
>
You need to send a new patch addressing the comments made by Krzysztof..
Regards
Mike
--
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK
On Wed, Jul 30, 2025 at 12:09:42PM +0100, Mark Brown wrote:
> On Wed, Jul 30, 2025 at 12:01:25PM +0100, Suzuki K Poulose wrote:
>
> > I would recommend using that and don't force the use of apb_clk/apb
> > for AMBA devices. If the firmware doesn't specify a clock, but does
> > specify the CoreSight components, it knows it better.
>
> And perhaps more to the point if a currently working system suddenly
> starts requiring additional clocks in it's binding that's an ABI break.
Yes, the change should not break any platforms if the DT binding is
passed correctly. I will update with devm_clk_get_optional_enabled().
Just for the record, I was a bit concerned that the driver might not
report a missing clock after switching to the optional clock API.
After discussed with Rob and Suzuki, I understand this should not be a
problem. Any missing clock issue can be caught by the DT schema static
checker, or a system hang during the development phase would remind
developers to bind clocks properly.
Thanks,
Leo
On 29/07/2025 12:31, Mark Brown wrote:
> On Mon, Jul 28, 2025 at 05:45:04PM +0100, Mark Brown wrote:
>> On Thu, Jul 24, 2025 at 04:22:34PM +0100, Leo Yan wrote:
>>
>> Previously we would return NULL for any error (which isn't super great
>> for deferred probe but never mind).
>>
>>> + pclk = devm_clk_get_enabled(dev, "apb_pclk");
>>> + if (IS_ERR(pclk))
>>> + pclk = devm_clk_get_enabled(dev, "apb");
>>
>> ...
>>
>>> return pclk;
>>> }
>>
>> Now we pass errors back to the caller, making missing clocks fatal.
>
> Thinking about this some more I think for compatiblity these clocks need
> to be treated as optional - that's what the original code was
> effectively doing, and I can imagine this isn't the only SoC which has
> (hopefully) always on clocks and didn't wire things up in DT.
You're right. The static components (funnels, replicators) don't have
APB programming interface and hence no clocks. That said, may be the
"is amba device" check could be used to enforce the presence of a clock.
I will let Leo sort this out
Suzuki
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 Juno platform, for both DT and
ACPI modes.
---
Changes in v5:
- Skip clock management for ACPI devices (Suzuki).
- Link to v4: https://lore.kernel.org/r/20250627-arm_cs_fix_clock_v4-v4-0-0ce0009c38f8@ar…
Changes in v4:
- Separated patch 07 into two patches, one is for clock consolidation
and another is for polishing driver data allocation (Anshuman).
Changes in v3:
- Updated subjects for patches 04 and 05 (Anshuman).
- Refined condition checking "if (dev_is_amba(dev))" in patch 07
(Anshuman).
---
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 | 48 ++++++++++++++++
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 | 31 +---------
16 files changed, 228 insertions(+), 290 deletions(-)
---
base-commit: a80198ba650f50d266d7fc4a6c5262df9970f9f2
change-id: 20250627-arm_cs_fix_clock_v4-e24b1e1f8920
Best regards,
--
Leo Yan <leo.yan(a)arm.com>