Message-ID: <f3089d5b-340f-4d26-9b5a-15e9dbe1d389@foss.st.com>
Date: Mon, 12 Jan 2026 10:06:07 +0100
From: Gatien CHEVALLIER <gatien.chevallier@...s.st.com>
To: Krzysztof Kozlowski <krzk@...nel.org>,
        Suzuki K Poulose
	<suzuki.poulose@....com>,
        Mike Leach <mike.leach@...aro.org>,
        James Clark
	<james.clark@...aro.org>, Rob Herring <robh@...nel.org>,
        Krzysztof Kozlowski
	<krzk+dt@...nel.org>,
        Conor Dooley <conor+dt@...nel.org>,
        Mathieu Poirier
	<mathieu.poirier@...aro.org>,
        Leo Yan <leo.yan@...ux.dev>,
        Clément Le Goffic <legoffic.clement@...il.com>,
        "Linus
 Walleij" <linusw@...nel.org>,
        Maxime Coquelin <mcoquelin.stm32@...il.com>,
        Alexandre Torgue <alexandre.torgue@...s.st.com>
CC: <coresight@...ts.linaro.org>, <linux-arm-kernel@...ts.infradead.org>,
        <devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <linux-gpio@...r.kernel.org>,
        <linux-stm32@...md-mailman.stormreply.com>
Subject: Re: [PATCH 03/11] dt-bindings: bus: document the stm32 debug bus



On 1/11/26 12:27, Krzysztof Kozlowski wrote:
> On 09/01/2026 11:55, Gatien Chevallier wrote:
>> Document the stm32 debug bus. The debug bus is responsible for
>> checking the debug sub-system accessibility before probing any related
>> drivers.
>>
>> Signed-off-by: Gatien Chevallier <gatien.chevallier@...s.st.com>
>> ---
>>   .../bindings/bus/st,stm32mp131-dbg-bus.yaml        | 86 ++++++++++++++++++++++
>>   1 file changed, 86 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/bus/st,stm32mp131-dbg-bus.yaml b/Documentation/devicetree/bindings/bus/st,stm32mp131-dbg-bus.yaml
>> new file mode 100644
>> index 000000000000..68bdfba08909
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/bus/st,stm32mp131-dbg-bus.yaml
>> @@ -0,0 +1,86 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/bus/st,stm32mp131-dbg-bus.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: STM32 Coresight bus
>> +
>> +maintainers:
>> +  - Gatien Chevallier <gatien.chevallier@...s.st.com>
>> +
>> +description: |
> 
> 
> Do not need '|' unless you need to preserve formatting.
> >> +  The STM32 debug bus is in charge of checking the debug configuration
> 
> What sort of bus it is? What protocol? Looks like rather fake wrapping
> node. You cannot just organize children in MMIO and call it a "bus".
> just to justify having child nodes.
> 

It is a standard APB bus, gated through the debug configuration,
dedicated for debug peripherals.

>> +  of the platform before probing the peripheral drivers that rely on the debug
>> +  domain.
>> +
>> +properties:
>> +  compatible:
>> +    oneOf:
> 
> Drop
> 
>> +      - items:
> 
> Drop
> 
>> +          - enum:
>> +              - st,stm32mp131-dbg-bus
>> +              - st,stm32mp151-dbg-bus
>> +
>> +  "#address-cells":
>> +    const: 1
>> +
>> +  "#size-cells":
>> +    const: 1
>> +
>> +  clocks:
>> +    maxItems: 1
>> +
>> +  ranges: true
>> +
>> +  reg:
>> +    maxItems: 1
> 
> What stuff is here? Your driver does not use it at all.
> 

I listed the reg property so that it is clear what memory range
the debug bus covers. Peripherals under it should be in that range.
Do you prefer no reg and explicit ranges property with the memory
range in it?

>> +
>> +  "#access-controller-cells":
>> +    const: 1
>> +    description:
>> +      Contains the debug profile necessary to access the peripheral.
>> +
>> +patternProperties:
>> +  "^.*@[0-9a-f]+$":
>> +    description: Debug related peripherals
>> +    type: object
>> +
>> +    additionalProperties: true
>> +
>> +    required:
>> +      - access-controllers
> 
> I don't get why children need to reference the parent. If you have such
> relationship, then the access-controllers are redundant, no?
> 

They can reference the parent, but nothing prevents them to reference
a different access controller, if needed. It's not only the parent
that is referenced, it is also the argument representing the
specific configuration to check.

For now, the peripherals in this patchset all check the
same configuration bits, but there are others that differ,
so I need per-node property.

No peripherals on this bus can function properly without some
bits of debug configuration being set, so I made the property
mandatory.

>> +
>> +required:
>> +  - "#access-controller-cells"
>> +  - "#address-cells"
>> +  - "#size-cells"
>> +  - clocks
>> +  - compatible
>> +  - ranges
>> +  - reg
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/clock/stm32mp1-clks.h>
>> +
>> +    dbg_bus: bus@...80000 {
>> +      compatible = "st,stm32mp131-dbg-bus";
>> +      reg = <0x50080000 0x3f80000>;
>> +      #address-cells = <1>;
>> +      #size-cells = <1>;
>> +      clocks = <&rcc CK_DBG>;
> 
> So the same clock as child? Another argument that this node is not a
> true bus node.
> 

In fact, I do not need to manipulate the clock at bus level. I'll
simply remove it for V2 and let the peripherals enable the bus clock
when needed. Thank you.

>> +      #access-controller-cells = <1>;
>> +      ranges;
>> +
>> +      cs_cti_trace: cti@...94000 {
> 
> Drop unused label
> 
>> +        compatible = "arm,coresight-cti", "arm,primecell";
>> +        reg = <0x50094000 0x1000>;
>> +        clocks = <&rcc CK_DBG>;
>> +        clock-names = "apb_pclk";
>> +        access-controllers = <&dbg_bus 0>;
>> +        status = "disabled";
> 
> Drop
> 
>> +      };
>> +    };
>>
> 
> 
> Best regards,
> Krzysztof

I'll address the comments for v2

Best regards,
Gatien

Powered by blists - more mailing lists