Re: [PATCH 05/40] arm64: dts: rockchip: Add frl-enable-gpios to rk3576-luckfox-core3576

[Cristian Ciocaltea <cristian.ciocaltea@collabora.com>]

On 4/20/26 6:18 PM, Heiko Stuebner wrote:
> Am Montag, 20. April 2026, 13:00:25 Mitteleuropäische Sommerzeit schrieb Cristian Ciocaltea:
>> Hi Heiko,
>>
>> On 4/18/26 2:12 AM, Heiko Stuebner wrote:
>>> Hi Cristian,
>>>
>>> Am Freitag, 17. April 2026, 18:34:17 Mitteleuropäische Sommerzeit schrieb Cristian Ciocaltea:
>>>> On 4/17/26 2:32 PM, Heiko Stuebner wrote:
>>>>> the comments below apply sort of to all patches in that series.
>>>>>
>>>>> Am Freitag, 17. April 2026, 11:24:39 Mitteleuropäische Sommerzeit schrieb Cristian Ciocaltea:
>>>>>> The board exposes the GPIO4_C6 line to control the voltage bias on the
>>>>>> HDMI data lines.  It must be asserted when operating in HDMI 2.1 FRL
>>>>>> mode and deasserted for HDMI 1.4/2.0 TMDS mode.
>>>>>>
>>>>>> Wire up the HDMI node to the GPIO line using the frl-enable-gpios
>>>>>> property and drop the line from the vcc_5v0_hdmi regulator to allow
>>>>>> adjusting the bias when transitioning between TMDS and FRL operating
>>>>>> modes.
>>
>> [...]
>>
>>>>>
>>>>>
>>>>>> @@ -231,6 +228,8 @@ &gpu {
>>>>>>  };
>>>>>>  
>>>>>>  &hdmi {
>>>>>> +	pinctrl-0 = <&hdmi_txm0_pins &hdmi_tx_scl &hdmi_tx_sda &hdmi_frl_en>;
>>>>>> +	frl-enable-gpios = <&gpio4 RK_PC6 GPIO_ACTIVE_LOW>;
>>>>>
>>>>> this should be sorted the other way around I think.
>>>>>
>>>>> Also please provide a pinctrl-names property too. If for whatever reason
>>>>> the dw-hdmi aquires a 2nd pinctrl state in the future, this makes sure
>>>>> board DTs are staying in the "old" compatible mode until they are adapted.
>>>>
>>>> Just to make sure I fully understand, the convention is that 
>>>>
>>>>   pinctrl-names = "default";
>>>>
>>>> should be always provided, even when the node overrides an existing pinctrl-0
>>>> property?
>>>>
>>>> E.g. in rk3576.dtsi we have:
>>>>
>>>>   hdmi: hdmi@27da0000 {
>>>>     ...
>>>>     pinctrl-names = "default";
>>>>     pinctrl-0 = <&hdmi_txm0_pins &hdmi_tx_scl &hdmi_tx_sda>;
>>>>     ...
>>>>   }
>>>>
>>>> Hence I omitted pinctrl-names which doesn't change and just appended
>>>> &hdmi_frl_en to pinctrl-0's original value.
>>>
>>> correct, please always provide a pinctrl-names entry when setting a new
>>> pinctrl-0 .
>>>
>>> The background is, imagine you have a base:
>>>
>>> pinctrl-names = "default";
>>> pinstrl-0 = <....>;
>>>
>>> and override pinctrl-0 in a board.
>>>
>>> Now a newer binding introduces a 2nd pinctrl state "foo". Of course
>>> we're backwards compatible, and both are valid and the driver checks
>>> what states are defined.
>>>
>>> So the base sets:
>>> pinctrl-names = "default", "foo";
>>> pinctrl-0 = <...>;
>>> pinctrl-1 = <...>;
>>>
>>> in your (old) board you override pinctrl-0, but the driver still sees
>>> the new variant with 2 pinctrl states, where it should've stayed with
>>> the legacy 1-state, until the board-dts might get adapted in the future.
>>>
>>>
>>> And I know, we're likely not doing that everywhere, and also in most
>>> cases it won't really matter, but still it is safer and sets the better
>>> precedent :-) .
>>
>> Thanks for the detailed explanation, that clears things up!
>>
>> There are several other nodes (e.g. i2c, pwm, uart) that also lack
>> pinctrl-names despite providing pinctrl-0 - I can address those in a
>> separate patch.
> 
> As said above it is an ideal to aspire to (having -names together with
> defining states), but if you want to add the "missing" -names,
> go ahead :-) .
> 
> 
>> I also noticed an inconsistency in property ordering: some nodes place
>> pinctrl-names before pinctrl-<n> and others after.  I have always used
>> the former, but we should probably prefer the latter to stay consistent
>> with how clocks, resets, phys, etc. are ordered.
>>
>> Thoughts?
> 
> There is sort of a "conflict" between regular ordering and possibly
> better readability. I.e. the dt-writing guidelines propose alphabetical
> ordering which I guess puts numbers before letters.
> 
> On the other hand the semantic definition of list the states and then
> define them (names first, -0, -1, etc second) looks more sensible from
> a understanding standpoint.
> 
> 
> But there we'd end up with special rules, so just sticking to the
> base sorting will cause less friction in the long run I think.
> Aka, -0, -1 first; -names after, follows the main sorting suggestions
> so it's easy to explain to newcomers.

Yes, that seems like a good enough reason to go with this approach.

> But please don't re-sort existing entries :-)

Oh, no, it's just for the current changes. :-)

Thanks,
Cristian


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip