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

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

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