Re: [PATCH 1/8] media: dt-bindings: qcom-kaanapali-iris: Add kaanapali video codec binding

[Vikash Garodia <vikash.garodia@oss.qualcomm.com>]

On 9/27/2025 3:55 AM, Dmitry Baryshkov wrote:
> On Fri, Sep 26, 2025 at 07:25:30PM +0530, Vikash Garodia wrote:
>>
>> On 9/26/2025 5:17 PM, Konrad Dybcio wrote:
>>> On 9/25/25 9:38 PM, Dmitry Baryshkov wrote:
>>>> On Fri, Sep 26, 2025 at 01:01:29AM +0530, Vikash Garodia wrote:
>>>>>
>>>>> On 9/26/2025 12:55 AM, Dmitry Baryshkov wrote:
>>>>>> On Thu, Sep 25, 2025 at 04:44:39AM +0530, Vikash Garodia wrote:
>>>
>>>
>>> [...]
>>>
>>>>>>> +  power-domains:
>>>>>>> +    minItems: 5
>>>>>>> +    maxItems: 7
>>>>>>
>>>>>> You are sending bindings for a single device on a single platform. How
>>>>>> comes that it has min != max?
>>>>>
>>>>> I was planning to reuse this binding for the variant SOCs of kaanapali/vpu4. If
>>>>> we do not have min interface, then for those variants, we have to either have
>>>>> separate bindings or add if/else conditions(?). Introducing min now can make it
>>>>> easily usable for upcoming vpu4 variants.
>>>>
>>>> No, it makes it harder to follow the changes. This platform has
>>>> this-and-that requirements. Then you add another platform and it's clear
>>>> that the changes are for that platform. Now you have mixed two different
>>>> patches into a single one.
>>>
>>> Vikash, preparing for future submissions is a very good thing,
>>> however "a binding" can be thought of as a tuple of
>>>
>>> (compatible, allowed_properties, required_properties)
>>>
>>> which needs(asterisk) to remain immutable
>>>
>>> You can make changes to this file later, when introducing said
>>> platforms and it will be fine, so long as you preserve the same allowed
>>> and required properties that you're trying to associate with Kanaapali
>>> here
>>
>> Let say, we have a kaanapali hardware (calling it as kaanapali_next) with 6
>> power domains, instead of 7, given that one of the pipe is malfunctional or
>> fused out in that hardware distrubution, should the binding be extended for such
>> variant like below ?
> 
> This comes together with the description of kaanapali_next and a proper
> commit message, describing the usage of fuses in the nvram for this
> hardware, etc. My point is that you are adding support for a fixed class
> of hardware: normal Kaanapali device, no extras, no disabled blocks,
> etc. This class of hardware has a fixed connections between IP blocks,
> fixed number of cores, power domains, etc.
> 
> Only when we actually add kaanapali_next, kaanapali_lite, kaanapali+1 or
> kaanapali-minor it would be logical to extend the base declarations, add
> add if-conditions for both kaanapali and the new device (notice
> if-conditions for kaanapali too).
> 
> I can say it other way around: the bindings that you've submitted are
> not complete as you have not bound kaanapali desription according to its
> actual hardware.
> 
>>
>> power-domains:
>>   maxItems: 7
>>
>>   - if:
>>       properties:
>>         compatible:
>>           enum:
>>             - qcom,kaanapali_next-iris
>>     then:
>>       properties:
>>         power-domains:
>>           maxItems: 6
>>
>>     else:
>>       properties:
>>         power-domains:
>>           maxItems: 7
>>
>> Also, what is the downside in existing approach where we say that the hardware
>> can be functional with 5 pds, and 2 are optional based on hardware having them
>> or not ? So all combinations of [5, 6, 7] pds are valid. IIUC, the optional
>> entries are made for such cases where some hardware parts are variable, please
>> correct my understanding.
> 
> Kaanapali hardware is not variable, is it?

By variable i meant the hardware is functional with or without those bindings,
hence was keeping them as an interface but optional. If that fits into optional
category, i can keep it existing way, otherwise will update to fix binding.

Regards,
Vikash
> 
>>
>> Regards,
>> Vikash
>>
>>> (i.e. YAML refactors are OK but the result must come out identical)
>>>
>>> Konrad
>