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

[Jorge Ramirez <jorge.ramirez@oss.qualcomm.com>]

On 17/06/25 08:56:37, Krzysztof Kozlowski wrote:
> 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.
>

how could you not? two opposing schema views can not be right. If you
knew this, you should have raised. There is only so many hours in a day.

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

huh? You are overthinking this: I have zero interest on evaluating
anyones work; however I need to make sure we do the right thing when
merging this - discussing previous interpretations in search of a
coherent story is not "discrediting" but the obvious thing to do (do not
assume malice let alone throw straw-man my way).

What you are asking me to do is not consistent with what has been done
in the past: since those commits were signed by well known maintainers
just as yourself I needed to understand the delta as well as making sure
everyone is aligned.

I understood your point, but I also understood theirs as being
accepted. hence I need someone to confirm which way to go. if that
someone is yourself, just confirm it and I'll move forward.