Re: [PATCH v3 5/9] ARM: dts: stm32: add Hardware debug port (HDP) on stm32mp13

[Clement LE GOFFIC <clement.legoffic@foss.st.com>]

On 6/12/25 15:09, Krzysztof Kozlowski wrote:
> On 12/06/2025 15:02, Clement LE GOFFIC wrote:
>> On 6/12/25 13:05, Krzysztof Kozlowski wrote:
>>> On 12/06/2025 11:31, Clement LE GOFFIC wrote:
>>>> On 6/11/25 17:48, Krzysztof Kozlowski wrote:
>>>>> On 11/06/2025 16:08, Clement LE GOFFIC wrote:
>>>>>> On 6/11/25 08:35, Krzysztof Kozlowski wrote:
>>>>>>> On 10/06/2025 15:33, Clement LE GOFFIC wrote:
>>>>>>>> On 6/10/25 14:38, Krzysztof Kozlowski wrote:
>>>>>>>>> On 10/06/2025 14:02, Clement LE GOFFIC wrote:
>>>>>>>>>> On 5/29/25 11:01, Krzysztof Kozlowski wrote:
>>>>>>>>>>> On 28/05/2025 14:14, Clement LE GOFFIC wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>>> +		};
>>>>>>>>>>>>>> +
>>>>>>>>>>>>>> +		hdp: pinctrl@5002a000 {
>>>>>>>>>>>>>> +			compatible = "st,stm32mp131-hdp";
>>>>>>>>>>>>>> +			reg = <0x5002a000 0x400>;
>>>>>>>>>>>>>> +			clocks = <&rcc HDP>;
>>>>>>>>>>>>>>         			status = "disabled";
>>>>>>>>>>>>>
>>>>>>>>>>>>> Why are you disabling it? What is missing?
>>>>>>>>>>>>
>>>>>>>>>>>> Nothing is missing just disabled by default.
>>>>>>>>>>>> The node is then enabled when needed in board's dts file.
>>>>>>>>>>> Nodes should not be disabled by default if they are complete. That's why
>>>>>>>>>>> I asked what is missing. Drop.
>>>>>>>>>>
>>>>>>>>>> Hi Krzysztof, OK I better understand now.
>>>>>>>>>> So yes the 'pinctrl-*' properties which are board dependent are lacking.
>>>>>>>>>
>>>>>>>>> These are not properties of this node.
>>>>>>>>
>>>>>>>> Does this mean I should add 'pinctrl-*' properties in bindings yaml file ?
>>>>>>>> I don't get it..
>>>>>>>
>>>>>>> These properties have no meaning here, so the hardware description is
>>>>>>> complete. You claim that you miss them thus device is incomplete is just
>>>>>>> not correct: these properties do not belong here! They belong to the
>>>>>>> board but even there they are totally optional. Why would they be a
>>>>>>> required resource?
>>>>>>>
>>>>>>> To remind: we talk here ONLY about required resources.
>>>>>>
>>>>>> Yes, 'pinctrl-*' properties belongs to the board and are not required.
>>>>>> So nothing is missing.
>>>>>>
>>>>>> This hdp node in the SoC dtsi file can be enabled by default.
>>>>>> But the hdp driver will probe and do nothing because without the
>>>>>> 'pinctrl-*' properties from the board files it would not be able to
>>>>>> access to any pin.
>>>>>
>>>>>
>>>>> Pinctrl has other features in general, including interfaces to userspace
>>>>> (as pretty often combined with gpio, although not sure if relevant here).
>>>>
>>>> You're right. Also HDP pinctrl has a GPO feature accessible from userspace.
>>>> But by default the HDP is not connected to any pad; it needs the board
>>>
>>> OK, then that was the answer to my first question - what is missing.
>>> However aren't these pads connected internally also to other parts of
>>> the SoC (like in most other vendors)?
>>
>> No, HDP "output pads" are only connected to SoC pinctrl to route outside
>> the internal SoC signals for debug purpose.
>>
>>>> 'pinctrl-*' properties to configure the SoC pinctrl and expose HDP on
>>>> the SoC pads.
>>>>
>>>> That's why for me the enabling of the driver should be in the board file
>>>> together with the SoC pinctrl configuration.
>>>
>>> And what are the default pad settings configured by HPD driver in
>>> bootloader (and by bootloader I mean one of few bootloaders this is
>>> going to be used on like U-Boot)
>>
>> The default is to use the GPIO of the SoC pinctrl. The HDP is not routed
>> outside.
>>    >>
>>>> The userland cannot change the pinctrl alternate function mux, this is
>>>> statically defined by the devicetree.
>>>
>>> If you expose GPIO then userland needs this regardless of alternate mux.
>>> IOW, board level could not configure mux because it should be always
>>> configured to safe GPIO input.
>>
>> For userland sight view, SoC GPIO are preferred instead of HDP.
>> HDP is only GPO not GPIO. 'pinctrl-*' properties configure at the same
>> time the SoC pinctrl mux to HDP and the HDP pinctrl mux to one of the
>> HDP functions (e.g. GPO).
> Thanks, that's explains, fine to keep it disabled. Unless it is obvious
> for everyone, it would be nice to put it in commit msg.

You're welcome, so I'll provide the V6 with more information in the 
commit message of patch [5-7] among other needed fixes.

> 
> Best regards,
> Krzysztof