Re: [PATCH 1/5] dt-bindings: media: venus: Add qcm2290 dt schema

[Krzysztof Kozlowski <krzk@kernel.org>]

On 17/06/2025 08:47, Jorge Ramirez wrote:
> On 17/06/25 08:14:23, Krzysztof Kozlowski wrote:
>> On 16/06/2025 18:59, Jorge Ramirez wrote:
>>> On 16/06/25 18:23:18, Krzysztof Kozlowski wrote:
>>>> On 16/06/2025 18:18, Jorge Ramirez wrote:
>>>>> On 16/06/25 16:41:44, Krzysztof Kozlowski wrote:
>>>>>> On 16/06/2025 14:52, Jorge Ramirez wrote:
>>>>>>>>
>>>>>>>>> +  The Venus AR50_LITE IP is a video encode and decode accelerator present
>>>>>>>>> +  on Qualcomm platforms
>>>>>>>>> +
>>>>>>>>> +allOf:
>>>>>>>>> +  - $ref: qcom,venus-common.yaml#
>>>>>>>>> +
>>>>>>>>> +properties:
>>>>>>>>> +  compatible:
>>>>>>>>> +    const: qcom,qcm2290-venus
>>>>>>>>> +
>>>>>>>>> +  power-domains:
>>>>>>>>> +    minItems: 2
>>>>>>>>> +    maxItems: 3
>>>>>>>>> +
>>>>>>>>> +  power-domain-names:
>>>>>>>>> +    minItems: 2
>>>>>>>>
>>>>>>>> Why is this flexible? Either you have two or three. Not mixed.
>>>>>>>
>>>>>>> please check 5b380f242f360256c96e96adabeb7ce9ec784306
>>>>>>
>>>>>> This does not explain why this is optional HERE. You cannot use for a
>>>>>> new platform an argument that some existing platform was changed in
>>>>>> ABI-preserving way.
>>>>>
>>>>> thanks for quick the follow up.
>>>>>
>>>>> but bear with me please because I dont follow - why can the same logic
>>>>> be used - it being applicable - and therefore result in a definition
>>>>> similar to those other platforms?
>>>>
>>>> Because this platform either has 2 or 3, not both. Unless that's not
>>>> true, but then please share some arguments.
>>>
>>> as with every other venus schema with more than 1 power domain, the
>>> argument is the same one that I have shared with you a couple of
>>> messages back (DVFS).
>>>
>>> verbatim:
>>>     Venus needs to vote for the performance state of a power domain (cx)
>>>     to be able to support DVFS. This 'cx' power domain is controlled by
>>>     rpm and is a common power domain (scalable) not specific to
>>>     venus alone. This is optional in the sense that, leaving this power
>>>     domain out does not really impact the functionality but just makes
>>>     the platform a little less power efficient.
>>
>> That's not definition of optional. The domain is needed for this device,
>> the device is one way or another having its rails routed to that domain.
>> It is not optional.
>>
>>>
>>> Seeing all these venus schemas follow the same pattern, it seems to me
>>> that this is the correct way of implementing the above.
>>
>> No for the reason I mentioned earlier.
> 
> So just to close this story up, were these two commits wrongly
> reviewed and signed off then ? Please do notice they were also - just
> like this one - new additions and not a change in an ABI preserving way
> as you characterize them.
> 
> e48b839b6699c2268e545360e06962bb76ff5b8d
> 8d3a1cb32124eaeb3f2efe4889de214d3b658d8d

I was waiting for this argument: there was something similar some years
ago (but even months ago...) and it got reviewed, so I can do the same.

You can even go further back. Take commits for DT bindings from 2013 and
use that against our new review. So many different things were accepted
in 2013.

You can take any driver code from 2013. Huh, people actually do! People
still send .owner=THIS_MODULE. In 2013 this was reviewed and accepted,
so I can send it, right?

And then people are not happy that they patches receive too much
detailed review or review takes too much time or whatever other
reason... Yeah if any review you ever give will be some day used against
you, you would think 10 times and be 10 times more picky then necessary.

This is like an ultimate, super, triple combo argument against reviewers
and maintainers to discredit their work. I will not play such games.

Best regards,
Krzysztof