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

[Heiko Stuebner <heiko@sntech.de>]

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.
> >>
> >> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> >> ---
> >>  arch/arm64/boot/dts/rockchip/rk3576-luckfox-core3576.dtsi | 9 ++++-----
> >>  1 file changed, 4 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/arch/arm64/boot/dts/rockchip/rk3576-luckfox-core3576.dtsi b/arch/arm64/boot/dts/rockchip/rk3576-luckfox-core3576.dtsi
> >> index 749f0a54b478..93ae37699366 100644
> >> --- a/arch/arm64/boot/dts/rockchip/rk3576-luckfox-core3576.dtsi
> >> +++ b/arch/arm64/boot/dts/rockchip/rk3576-luckfox-core3576.dtsi
> >> @@ -140,10 +140,7 @@ regulator-state-mem {
> >>  
> >>  	vcc_5v0_hdmi: regulator-vcc-5v0-hdmi {
> >>  		compatible = "regulator-fixed";
> >> -		enable-active-high;
> >> -		gpios = <&gpio4 RK_PC6 GPIO_ACTIVE_HIGH>;
> >> -		pinctrl-names = "default";
> >> -		pinctrl-0 = <&hdmi_con_en>;
> >> +		regulator-always-on;
> >>  		regulator-min-microvolt = <5000000>;
> >>  		regulator-max-microvolt = <5000000>;
> >>  		regulator-name = "vcc_5v0_hdmi";
> > 
> > I think this regulator was sort of a complete hack, to set that
> > gpio to some sort of default state, by declaring it as hdmi-pwr-supply.
> > 
> > Only 2 rk3576 boards seem, to use that hack, so I think as that "regulator"
> > is completely functionless now, the whole thing could be removed?
> 
> Ack, let's just drop it.
> 
> > 
> > 
> >> @@ -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 :-) .


> >>  	status = "okay";
> >>  };
> >>  
> >> @@ -655,7 +654,7 @@ &pcie0 {
> >>  
> >>  &pinctrl {
> >>  	hdmi {
> >> -		hdmi_con_en: hdmi-con-en {
> >> +		hdmi_frl_en: hdmi-frl-en {
> > 
> > pinctrl names should ideally match the naming in schematics, for example the
> > "HDMI0_TX_ON_H" for jaguar and tiger. This makes it way easier to> go from DT
> > to schematics and back.
> 
> I opted for a more descriptive name that could be used consistently across all
> boards, given that not all schematics are publicly available.
> 
> You make a fair point though, we should probably stick with the pretty terrible
> hdmi[N]_tx_on_h naming instead.

yep, we're doing that everywhere else already too, and sticking to the
schematics naming, also prevents any discussions about how something
should be named ;-) .


Heiko