Received-SPF: pass (google.com: domain of linux-kernel+bounces-131095-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
From: Heiko =?ISO-8859-1?Q?St=FCbner?= <heiko@sntech.de>
To: Shreeya Patel <shreeya.patel@collabora.com>,
 Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Cc: mchehab@kernel.org, hverkuil@xs4all.nl, hverkuil-cisco@xs4all.nl,
 robh@kernel.org, krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org,
 mturquette@baylibre.com, sboyd@kernel.org, p.zabel@pengutronix.de,
 shawn.wen@rock-chips.com, kernel@collabora.com, linux-kernel@vger.kernel.org,
 linux-media@vger.kernel.org, devicetree@vger.kernel.org,
 linux-arm-kernel@lists.infradead.org, linux-rockchip@lists.infradead.org,
 linux-clk@vger.kernel.org, linux-arm@lists.infradead.org
Subject: Re: [PATCH v3 0/6] Add Synopsys DesignWare HDMI RX Controller
Date: Thu, 04 Apr 2024 10:19:55 +0200
Message-ID: <28071718.gRfpFWEtPU@diego>
In-Reply-To: <c790c8ba-a9bd-4820-8084-1294e5e523d9@linaro.org>
References:
 <20240327225057.672304-1-shreeya.patel@collabora.com>
 <3049149.687JKscXgg@diego> <c790c8ba-a9bd-4820-8084-1294e5e523d9@linaro.org>
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset="iso-8859-1"

Am Donnerstag, 4. April 2024, 08:15:50 CEST schrieb Krzysztof Kozlowski:
> On 04/04/2024 00:48, Heiko St=FCbner wrote:
> > Am Mittwoch, 3. April 2024, 13:24:05 CEST schrieb Krzysztof Kozlowski:
> >> On 03/04/2024 13:20, Shreeya Patel wrote:
> >>> On Wednesday, April 03, 2024 15:51 IST, Krzysztof Kozlowski <krzyszto=
f.kozlowski@linaro.org> wrote:
> >>>
> >>>> On 03/04/2024 11:24, Shreeya Patel wrote:
> >>>>> On Thursday, March 28, 2024 04:20 IST, Shreeya Patel <shreeya.patel=
@collabora.com> wrote:
> >>>>>
> >>>>>> This series implements support for the Synopsys DesignWare
> >>>>>> HDMI RX Controller, being compliant with standard HDMI 1.4b
> >>>>>> and HDMI 2.0.
> >>>>>>
> >>>>>
> >>>>> Hi Mauro and Hans,
> >>>>>
> >>>>> I haven't received any reviews so far. Hence, this is just a gentle=
 reminder to review this patch series.
> >>>>
> >>>> Why did you put clk changes here? These go via different subsystem. =
That
> >>>> might be one of obstacles for your patchset.
> >>>>
> >>>
> >>> I added clock changes in this patch series because HDMIRX driver depe=
nds on it.
> >>> I thought it is wrong to send the driver patches which don't even com=
pile?
> >>
> >> Hm, why HDMIRX driver depends on clock? How? This sounds really wrong.
> >> Please get it reviewed internally first.
> >=20
> > For the change in question, the clock controller on the soc also handles
> > the reset controls (hence its name CRU, clock-and-reset-unit) .
> >=20
> > There are at least 660 reset lines in the unit and it seems the hdmi-rx=
 one
> > was overlooked on the initial submission, hence patches 1+2 add the
> > reset-line.
> >=20
> > Of course, here only the "arm64: dts:" patch depends on the clock
> > change, is it references the new reset-id.
>=20
> Wait, that's expected, but it is not what was written. Claim was HDMIRX
> driver depends *build time* ("don't even compile").

Trying to do a full build (kernel + dts) will fail, because the the
device-tree patch references the newly added reset-id .

So you end up with a dtc error. Same with the binding.


I think in the past to uncouple this we did reference the id by number
first:

+      resets =3D <&cru SRST_A_HDMIRX>, <&cru SRST_P_HDMIRX>,
+               <&cru SRST_HDMIRX_REF>, <&cru 660>;

Had the id-addition separately and then a later series for kernel x+1
to convert from 660 to SRST_A_HDMIRX_BIU .


> > Am Mittwoch, 3. April 2024, 12:22:57 CEST schrieb Krzysztof Kozlowski:
> >> Please do not engage multiple subsystems in one patchset, if not
> >> necessary. Especially do not mix DTS into media or USB subsystems. And
> >> do not put DTS in the middle!
> >=20
> > picking up your reply from patch 4/6, there seem to be different "schoo=
ls
> > of thought" for this. Some maintainers might want to really only see
> > patches that are explicitly for their subsystem - I guess networking
> > might be a prime example for that, who will essentially apply whole ser=
ies'
> > if nobody protests in time (including dts patches)
>=20
> There is no school saying DTS is allowed to be in the middle.


I think I wrote exactly that in my original reply :-)

Am Donnerstag, 4. April 2024, 00:48:38 CEST schrieb Heiko St=FCbner:
> Of course you're right, the "arm64: dts:" patch should be the last in the
> series and not be in the middle of it.


Heiko