Re: [PATCH v3 0/6] Add Synopsys DesignWare HDMI RX Controller

From: Heiko Stübner
Date: Wed Apr 03 2024 - 18:49:16 EST


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 <krzysztof.kozlowski@xxxxxxxxxx> wrote:
> >
> >> On 03/04/2024 11:24, Shreeya Patel wrote:
> >>> On Thursday, March 28, 2024 04:20 IST, Shreeya Patel <shreeya.patel@xxxxxxxxxxxxx> 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 depends on it.
> > I thought it is wrong to send the driver patches which don't even compile?
>
> Hm, why HDMIRX driver depends on clock? How? This sounds really wrong.
> Please get it reviewed internally first.

For the change in question, the clock controller on the soc also handles
the reset controls (hence its name CRU, clock-and-reset-unit) .

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.

Of course, here only the "arm64: dts:" patch depends on the clock
change, is it references the new reset-id.


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!

picking up your reply from patch 4/6, there seem to be different "schools
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 series'
if nobody protests in time (including dts patches)

On the other hand I also remember seeing requests for "the full picture"
and individual maintainers then just picking and applying the patches
meant for their subsystem.

The series as it stands right now is nice in that it allows (random)
developers to just pick it up, apply it to a tree and test the actual driver
without needing to hunt for multiple dependant series.


Of course you're right, the "arm64: dts:" patch should be the last in the
series and not be in the middle of it.


Regards
Heiko