From: Heiko Stuebner <heiko@sntech.de>
To: Rob Herring <robh@kernel.org>,
Krzysztof Kozlowski <krzk+dt@kernel.org>,
Conor Dooley <conor+dt@kernel.org>,
Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
Cc: kernel@collabora.com, devicetree@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 05/40] arm64: dts: rockchip: Add frl-enable-gpios to rk3576-luckfox-core3576
Date: Mon, 20 Apr 2026 17:18:20 +0200 [thread overview]
Message-ID: <2480203.yKrmzQ4Hd0@phil> (raw)
In-Reply-To: <401a020f-ca5d-4c7d-941e-f0288e144357@collabora.com>
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.
But please don't re-sort existing entries :-)
Heiko
WARNING: multiple messages have this Message-ID (diff)
From: Heiko Stuebner <heiko@sntech.de>
To: Rob Herring <robh@kernel.org>,
Krzysztof Kozlowski <krzk+dt@kernel.org>,
Conor Dooley <conor+dt@kernel.org>,
Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
Cc: kernel@collabora.com, devicetree@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 05/40] arm64: dts: rockchip: Add frl-enable-gpios to rk3576-luckfox-core3576
Date: Mon, 20 Apr 2026 17:18:20 +0200 [thread overview]
Message-ID: <2480203.yKrmzQ4Hd0@phil> (raw)
In-Reply-To: <401a020f-ca5d-4c7d-941e-f0288e144357@collabora.com>
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.
But please don't re-sort existing entries :-)
Heiko
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
next prev parent reply other threads:[~2026-04-20 15:18 UTC|newest]
Thread overview: 104+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-17 9:24 [PATCH 00/40] arm64: dts: rockchip: Wire up frl-enable-gpios for RK3576/RK3588 boards Cristian Ciocaltea
2026-04-17 9:24 ` Cristian Ciocaltea
2026-04-17 9:24 ` [PATCH 01/40] arm64: dts: rockchip: Add frl-enable-gpios to rk3576-100ask-dshanpi-a1 Cristian Ciocaltea
2026-04-17 9:24 ` Cristian Ciocaltea
2026-04-17 9:24 ` [PATCH 02/40] arm64: dts: rockchip: Add frl-enable-gpios to rk3576-armsom-sige5 Cristian Ciocaltea
2026-04-17 9:24 ` Cristian Ciocaltea
2026-04-17 9:24 ` [PATCH 03/40] arm64: dts: rockchip: Add frl-enable-gpios to rk3576-evb1-v10 Cristian Ciocaltea
2026-04-17 9:24 ` Cristian Ciocaltea
2026-04-17 9:24 ` [PATCH 04/40] arm64: dts: rockchip: Add frl-enable-gpios to rk3576-evb2-v10 Cristian Ciocaltea
2026-04-17 9:24 ` Cristian Ciocaltea
2026-04-17 9:24 ` [PATCH 05/40] arm64: dts: rockchip: Add frl-enable-gpios to rk3576-luckfox-core3576 Cristian Ciocaltea
2026-04-17 9:24 ` Cristian Ciocaltea
2026-04-17 11:32 ` Heiko Stuebner
2026-04-17 11:32 ` Heiko Stuebner
2026-04-17 16:34 ` Cristian Ciocaltea
2026-04-17 16:34 ` Cristian Ciocaltea
2026-04-17 23:12 ` Heiko Stuebner
2026-04-17 23:12 ` Heiko Stuebner
2026-04-20 11:00 ` Cristian Ciocaltea
2026-04-20 11:00 ` Cristian Ciocaltea
2026-04-20 15:18 ` Heiko Stuebner [this message]
2026-04-20 15:18 ` Heiko Stuebner
2026-04-20 15:53 ` Cristian Ciocaltea
2026-04-20 15:53 ` Cristian Ciocaltea
2026-04-17 9:24 ` [PATCH 06/40] arm64: dts: rockchip: Add frl-enable-gpios to rk3576-nanopi-m5 Cristian Ciocaltea
2026-04-17 9:24 ` Cristian Ciocaltea
2026-04-17 9:24 ` [PATCH 07/40] arm64: dts: rockchip: Add frl-enable-gpios to rk3576-nanopi-r76s Cristian Ciocaltea
2026-04-17 9:24 ` Cristian Ciocaltea
2026-04-17 9:24 ` [PATCH 08/40] arm64: dts: rockchip: Add frl-enable-gpios to rk3576-roc-pc Cristian Ciocaltea
2026-04-17 9:24 ` Cristian Ciocaltea
2026-04-17 9:24 ` [PATCH 09/40] arm64: dts: rockchip: Add frl-enable-gpios to rk3576-rock-4d Cristian Ciocaltea
2026-04-17 9:24 ` Cristian Ciocaltea
2026-04-17 9:24 ` [PATCH 10/40] arm64: dts: rockchip: Add frl-enable-gpios to rk3588-armsom-sige7 Cristian Ciocaltea
2026-04-17 9:24 ` Cristian Ciocaltea
2026-04-17 9:24 ` [PATCH 11/40] arm64: dts: rockchip: Add frl-enable-gpios to rk3588-armsom-w3 Cristian Ciocaltea
2026-04-17 9:24 ` Cristian Ciocaltea
2026-04-17 9:24 ` [PATCH 12/40] arm64: dts: rockchip: Add frl-enable-gpios to rk3588-coolpi-cm5-evb Cristian Ciocaltea
2026-04-17 9:24 ` Cristian Ciocaltea
2026-04-17 9:24 ` [PATCH 13/40] arm64: dts: rockchip: Add frl-enable-gpios to rk3588-coolpi-cm5-genbook Cristian Ciocaltea
2026-04-17 9:24 ` Cristian Ciocaltea
2026-04-17 9:24 ` [PATCH 14/40] arm64: dts: rockchip: Add frl-enable-gpios to rk3588-evb1-v10 Cristian Ciocaltea
2026-04-17 9:24 ` Cristian Ciocaltea
2026-04-17 9:24 ` [PATCH 15/40] arm64: dts: rockchip: Add frl-enable-gpios to rk3588-evb2-v10 Cristian Ciocaltea
2026-04-17 9:24 ` Cristian Ciocaltea
2026-04-17 9:24 ` [PATCH 16/40] arm64: dts: rockchip: Add frl-enable-gpios to rk3588-firefly-itx-3588j Cristian Ciocaltea
2026-04-17 9:24 ` Cristian Ciocaltea
2026-04-17 9:24 ` [PATCH 17/40] arm64: dts: rockchip: Add frl-enable-gpios to rk3588-friendlyelec-cm3588-nas Cristian Ciocaltea
2026-04-17 9:24 ` Cristian Ciocaltea
2026-04-17 9:24 ` [PATCH 18/40] arm64: dts: rockchip: Add frl-enable-gpios to rk3588-h96-max-v58 Cristian Ciocaltea
2026-04-17 9:24 ` Cristian Ciocaltea
2026-04-17 9:24 ` [PATCH 19/40] arm64: dts: rockchip: Add frl-enable-gpios to rk3588-jaguar Cristian Ciocaltea
2026-04-17 9:24 ` Cristian Ciocaltea
2026-04-17 9:24 ` [PATCH 20/40] arm64: dts: rockchip: Add frl-enable-gpios to rk3588-mnt-reform2 Cristian Ciocaltea
2026-04-17 9:24 ` Cristian Ciocaltea
2026-04-17 9:24 ` [PATCH 21/40] arm64: dts: rockchip: Add frl-enable-gpios to rk3588-nanopc-t6 Cristian Ciocaltea
2026-04-17 9:24 ` Cristian Ciocaltea
2026-04-17 9:24 ` [PATCH 22/40] arm64: dts: rockchip: Add frl-enable-gpios to rk3588-orangepi-5-max Cristian Ciocaltea
2026-04-17 9:24 ` Cristian Ciocaltea
2026-04-17 9:24 ` [PATCH 23/40] arm64: dts: rockchip: Add frl-enable-gpios to rk3588-orangepi-5-plus Cristian Ciocaltea
2026-04-17 9:24 ` Cristian Ciocaltea
2026-04-17 9:24 ` [PATCH 24/40] arm64: dts: rockchip: Add frl-enable-gpios to rk3588-orangepi-5-ultra Cristian Ciocaltea
2026-04-17 9:24 ` Cristian Ciocaltea
2026-04-17 9:24 ` [PATCH 25/40] arm64: dts: rockchip: Add frl-enable-gpios to rk3588-roc-rt Cristian Ciocaltea
2026-04-17 9:24 ` Cristian Ciocaltea
2026-04-17 9:25 ` [PATCH 26/40] arm64: dts: rockchip: Add frl-enable-gpios to rk3588-rock-5-itx Cristian Ciocaltea
2026-04-17 9:25 ` Cristian Ciocaltea
2026-04-17 9:25 ` [PATCH 27/40] arm64: dts: rockchip: Add frl-enable-gpios to rk3588-rock-5b-5bp-5t Cristian Ciocaltea
2026-04-17 9:25 ` Cristian Ciocaltea
2026-04-17 9:25 ` [PATCH 28/40] arm64: dts: rockchip: Add frl-enable-gpios to rk3588-tiger Cristian Ciocaltea
2026-04-17 9:25 ` Cristian Ciocaltea
2026-04-17 9:25 ` [PATCH 29/40] arm64: dts: rockchip: Add frl-enable-gpios to rk3588s-coolpi-4b Cristian Ciocaltea
2026-04-17 9:25 ` Cristian Ciocaltea
2026-04-17 9:25 ` [PATCH 30/40] arm64: dts: rockchip: Add frl-enable-gpios to rk3588s-gameforce-ace Cristian Ciocaltea
2026-04-17 9:25 ` Cristian Ciocaltea
2026-04-17 9:25 ` [PATCH 31/40] arm64: dts: rockchip: Add frl-enable-gpios to rk3588s-indiedroid-nova Cristian Ciocaltea
2026-04-17 9:25 ` Cristian Ciocaltea
2026-04-17 9:25 ` [PATCH 32/40] arm64: dts: rockchip: Add frl-enable-gpios to rk3588s-khadas-edge2 Cristian Ciocaltea
2026-04-17 9:25 ` Cristian Ciocaltea
2026-04-17 9:25 ` [PATCH 33/40] arm64: dts: rockchip: Add frl-enable-gpios to rk3588s-nanopi-r6 Cristian Ciocaltea
2026-04-17 9:25 ` Cristian Ciocaltea
2026-04-17 9:25 ` [PATCH 34/40] arm64: dts: rockchip: Add frl-enable-gpios to rk3588s-odroid-m2 Cristian Ciocaltea
2026-04-17 9:25 ` Cristian Ciocaltea
2026-04-17 9:25 ` [PATCH 35/40] arm64: dts: rockchip: Add frl-enable-gpios to rk3588s-orangepi-5 Cristian Ciocaltea
2026-04-17 9:25 ` Cristian Ciocaltea
2026-04-17 9:25 ` [PATCH 36/40] arm64: dts: rockchip: Add frl-enable-gpios to rk3588s-orangepi-cm5-base Cristian Ciocaltea
2026-04-17 9:25 ` Cristian Ciocaltea
2026-04-17 9:25 ` [PATCH 37/40] arm64: dts: rockchip: Add frl-enable-gpios to rk3588s-radxa-cm5-io Cristian Ciocaltea
2026-04-17 9:25 ` Cristian Ciocaltea
2026-04-17 9:25 ` [PATCH 38/40] arm64: dts: rockchip: Add frl-enable-gpios to rk3588s-roc-pc Cristian Ciocaltea
2026-04-17 9:25 ` Cristian Ciocaltea
2026-04-17 9:25 ` [PATCH 39/40] arm64: dts: rockchip: Add frl-enable-gpios to rk3588s-rock-5a Cristian Ciocaltea
2026-04-17 9:25 ` Cristian Ciocaltea
2026-04-17 9:25 ` [PATCH 40/40] arm64: dts: rockchip: Add frl-enable-gpios to rk3588s-rock-5c Cristian Ciocaltea
2026-04-17 9:25 ` Cristian Ciocaltea
2026-04-17 11:34 ` [PATCH 00/40] arm64: dts: rockchip: Wire up frl-enable-gpios for RK3576/RK3588 boards Heiko Stuebner
2026-04-17 11:34 ` Heiko Stuebner
2026-04-17 17:55 ` Cristian Ciocaltea
2026-04-17 17:55 ` Cristian Ciocaltea
2026-04-17 23:18 ` Heiko Stuebner
2026-04-17 23:18 ` Heiko Stuebner
2026-04-20 11:10 ` Cristian Ciocaltea
2026-04-20 11:10 ` Cristian Ciocaltea
2026-04-20 15:08 ` Heiko Stuebner
2026-04-20 15:08 ` Heiko Stuebner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2480203.yKrmzQ4Hd0@phil \
--to=heiko@sntech.de \
--cc=conor+dt@kernel.org \
--cc=cristian.ciocaltea@collabora.com \
--cc=devicetree@vger.kernel.org \
--cc=kernel@collabora.com \
--cc=krzk+dt@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=robh@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.