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

Cristian Ciocaltea
2026-04-17T16:34:33Z

Hi Heiko,

On 4/17/26 2:32 PM, Heiko Stuebner wrote:
> Hi Cristian,
>
> 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.

>
>
>> 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.

Thanks,
Cristian