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

Heiko Stuebner
2026-04-17T23:12:13Z

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