Re: [PATCH 2/5] dt-bindings: arm: stm32: Add clock-controller sub-node to STM32MP2 SYSCFG

[Gabriel FERNANDEZ <gabriel.fernandez@foss.st.com>]

Hi Conor,

Thanks for review

On 11/10/26 19:54, Conor Dooley wrote:
> On Tue, Nov 10, 2026 at 11:42:17AM +0100, Gabriel Fernandez wrote:
>> The LTDC pixel clock multiplexer of the STM32MP23x and STM32MP25x
>> SYSCFG is described by a clock-controller sub-node of the SYSCFG node.
>>
>> Add this sub-node to the SYSCFG binding. It is required on STM32MP23x
>> and STM32MP25x and not allowed on the other SoCs.
>>
>> Signed-off-by: Gabriel Fernandez <gabriel.fernandez@foss.st.com>
> Why does this need a child node at all, can't the clock controller
> properties go straight into this syscon node?
>
> Cheers,
> Conor.

I sent a v2 [1] shortly before your reply, which explains this in the
commit message, but the question still applies to it, so let me answer
here.

 From a hardware point of view, the mux inputs could indeed be described
in the SYSCFG node. The problem is that this node is a "syscon": when
the syscon core creates the regmap of the node (syscon_node_to_regmap()
or syscon_regmap_lookup_by_*()), it takes the first clock of the node as
the clock of the registers, keeps it prepared and enables/disables it
around each register access.

The SYSCFG has no clock gate in the RCC, so this first clock would be
the LTDC kernel clock, i.e. the first input of the mux. It would then be
enabled on every SYSCFG access by all the users of this syscon (PCIe,
ComboPHY, Ethernet, OMM...).

Registering our own regmap with of_syscon_register_regmap() from the
SYSCFG driver doesn't solve it either: these users reach the SYSCFG
through vendor phandles (st,syscon, st,syscfg-amcr) or by compatible,
which are not tracked by fw_devlink, so one of them can create the
syscon regmap, with the clock, before the SYSCFG driver probes.

Apart from adding a dummy clock in first position, which would describe
something which doesn't exist in the hardware, I didn't find a way to
keep these properties in the SYSCFG node. So the mux is described by a
sub-node with its own clock inputs, like the USB2 PHYs of the SYSCFG
which are already described as sub-nodes.

In v2, the '#clock-cells' property of the SYSCFG node is deprecated, so
that the sub-node is the only provider of this clock. No driver ever
registered a clock on the SYSCFG node, so this doesn't change anything
for existing devicetrees.

[1] 
https://lore.kernel.org/r/20261110-ups_mp25_clk_syscfg-v2-0-624559bb8ec7@foss.st.com

Best regards,

Gabriel


>
>> ---
>>   Documentation/devicetree/bindings/arm/stm32/st,stm32-syscon.yaml | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/stm32/st,stm32-syscon.yaml b/Documentation/devicetree/bindings/arm/stm32/st,stm32-syscon.yaml
>> index a559f8b03b84..38c844e8f19d 100644
>> --- a/Documentation/devicetree/bindings/arm/stm32/st,stm32-syscon.yaml
>> +++ b/Documentation/devicetree/bindings/arm/stm32/st,stm32-syscon.yaml
>> @@ -45,6 +45,10 @@ properties:
>>     "#size-cells":
>>       const: 1
>>   
>> +  clock-controller:
>> +    type: object
>> +    $ref: /schemas/clock/st,stm32mp25-syscfg-clk.yaml#
>> +
>>   patternProperties:
>>     "^phy@2[48]00$":
>>       type: object
>> @@ -80,6 +84,7 @@ allOf:
>>           - "#address-cells"
>>           - "#clock-cells"
>>           - "#size-cells"
>> +        - clock-controller
>>           - phy@2400
>>           - phy@2800
>>       else:
>> @@ -87,6 +92,7 @@ allOf:
>>           '#address-cells': false
>>           '#clock-cells': false
>>           '#size-cells': false
>> +        clock-controller: false
>>           phy@2400: false
>>           phy@2800: false
>>   
>>
>> -- 
>> 2.43.0
>>