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

[Krzysztof Kozlowski]

On 04/04/2024 00:48, Heiko Stübner 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 <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.

Wait, that's expected, but it is not what was written. Claim was HDMIRX
driver depends *build time* ("don't even compile").

> 
> 
> 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)

There is no school saying DTS is allowed to be in the middle.

Other schools are indeed saying that seeing DTS is good and
recommendation is to post it separate and provide a link. That's way you
avoid DTS being pulled by Greg, media or networking.

> 
> 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.


Best regards,
Krzysztof