Re: [PATCH v5 3/4] dt-bindings: display: rockchip: Add schema for RK3588 HDMI TX Controller

From: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
To: Shimrra Shai <shimrrashai@gmail.com>
Cc: Laurent.pinchart@ideasonboard.com, aarnoud@me.com,
	airlied@gmail.com, andrzej.hajda@intel.com,
	andy.yan@rock-chips.com, conor+dt@kernel.org, daniel@ffwll.ch,
	devicetree@vger.kernel.org, dri-devel@lists.freedesktop.org,
	heiko@sntech.de, hjc@rock-chips.com, jernej.skrabec@gmail.com,
	jonas@kwiboo.se, kernel@collabora.com, krzk+dt@kernel.org,
	krzk@kernel.org, ldearquer@gmail.com,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-rockchip@lists.infradead.org,
	maarten.lankhorst@linux.intel.com, markyao0591@gmail.com,
	mripard@kernel.org, neil.armstrong@linaro.org, rfoss@kernel.org,
	robh@kernel.org, s.hauer@pengutronix.de, tzimmermann@suse.de
Subject: Re: [PATCH v5 3/4] dt-bindings: display: rockchip: Add schema for RK3588 HDMI TX Controller
Date: Tue, 3 Sep 2024 01:14:42 +0300	[thread overview]
Message-ID: <eb9f2d20-9b51-45e1-bf32-504267747f06@collabora.com> (raw)
In-Reply-To: <20240902010948.3654-1-shimrrashai@gmail.com>

On 9/2/24 4:09 AM, Shimrra Shai wrote:
> Cristian Ciocaltea wrote:
>> On 8/31/24 9:13 AM, Krzysztof Kozlowski wrote:
>>>
>>> Please define all clocks.
>>
>> The other clocks are defined in the common binding, should we reiterate
>> them?
> 
> I would suggest yes, they should be reduplicated, if only to maintain
> consistency with all the other docs. A grep through the bridge docs
> shows that there are virtually none which use a "{}" placeholder like
> this. 

Are you sure about that?  This is precisely the approach followed by the
upstream DW HDMI TX controller binding [1].  Moreover, I've already pointed this
out in [2].

> While it seems kind of like one might worry about "don't
> repeat yourself" syndrome, keep in mind this is not code, but human-
> used documentation. Having all the information available at a glance
> would seem to be the most convenient to the end (developer) user, so
> they aren't having to toggle between two separate files. 

I think that's pretty subjective to be stated as a general rule, e.g. I don't
have any problem toggling between multiple files as I regularly keep over 50
files opened in my IDE.  Personally, I'd always go for the slightly less 
readable approach if I can avoid duplicating content.

I'd suggest to follow the whole thread [2], as this topic has been already
discussed.

> Of course
> there may be some questions regarding docs becoming out of sync, but
> *ideally* we don't want to break backward compatibility with device
> trees (esp. given how I am imagining firmware integration to work on
> these platforms, as the RK3588 is at at least low-end desktop-grade
> performance and UEFI packages have already been built for it), though
> of course that doesn't mean adding new options is off the table.
> 
> (FWIW, this is what I did in my now-withdrawn-at-your-request
> re-submission; I reduplicated the bindings as it seemed that's what
> others here were pushing for and thus that felt like the quickest way
> to get this important driver approved.)

This is not really a blocker for the series.  Please remember to be patient
while involved (one way or another) in the upstreaming process, as maintainers
need time to review *all* the patches.  This might be very important for you,
but there are, usually, tons of other way more important things the maintainers
need to handle in parallel.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml#n46
[2] https://lore.kernel.org/lkml/ec84bc0b-c4c2-4735-9f34-52bc3a852aaf@collabora.com/

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip