Re: [PATCH v4 02/18] dt-bindings: clock: qcom: Update sc8280xp camcc bindings

[Jagadeesh Kona <quic_jkona@quicinc.com>]

On 5/21/2025 6:16 PM, Dmitry Baryshkov wrote:
> On Wed, May 21, 2025 at 03:32:34PM +0530, Jagadeesh Kona wrote:
>>
>>
>> On 5/19/2025 1:48 PM, Krzysztof Kozlowski wrote:
>>> On Thu, May 15, 2025 at 12:38:47AM GMT, Jagadeesh Kona wrote:
>>>> SC8280XP camcc only requires the MMCX power domain, unlike
>>>> SM8450 camcc which will now support both MMCX and MXC power
>>>
>>> I do not see change to sm8450 here. This makes no sense on its own. You
>>> do not move compatibles - what is the point of such change?
>>>
>>
>> I did the SM8450 changes in next patch (3/18). But I agree with you, this needs to
>> be more structured. So I am planning to drop this patch and instead take care of
>> single power domain requirement for SC8280XP within SM8450 camcc bindings using
>> minItems and maxItems properties based on if check for sc8280xp compatible similar
>> to below snippet.
> 
> I think it is a bad idea. I liked the split that you've implemented:
> separate bindings for platforms that require MMCX (and MX), separate
> bindings for platforms which require MMCX and MXC (and MXA).
> 
> It might be better to start by changing SM8450 binding to support MXC
> and then adding SC8280XP to those bindings.
> 

Okay, I will reverse the order of patches 2 and 3 to support MXC for SM8450 camcc
bindings first and then move SC8280XP to have single power domain support. 

Thanks,
Jagadeesh

>>
>>    power-domains:
>> -    maxItems: 1
>> +    minItems: 1
>>      description:
>> -      A phandle and PM domain specifier for the MMCX power domain.
>> +      Power domains required for the clock controller to operate
>> +    items:
>> +      - description: MMCX power domain
>> +      - description: MXC power domain
>>
>> ......
>>
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - qcom,sc8280xp-camcc
>> +    then:
>> +      properties:
>> +        power-domains:
>> +          maxItems: 1
>> +        required-opps:
>> +          maxItems: 1
>> +
>>
>>
>>>> domains. Hence move SC8280XP camcc bindings from SM8450 to
>>>> SA8775P camcc.
>>>
>>> Subject: everything could be an update. Be specific.
>>>
>>> A nit, subject: drop second/last, redundant "bindings". The
>>> "dt-bindings" prefix is already stating that these are bindings.
>>> See also:
>>> https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18
>>>
>>
>> Sure, I will take care of above in next series.
>>
>> Thanks,
>> Jagadeesh
>>
>>>>
>>>> SA8775P camcc doesn't support required-opps property currently
>>>> but SC8280XP camcc need that property,  so add required-opps
>>>> based on SC8280XP camcc conditional check in SA8775P camcc
>>>> bindings.
>>>
>>> Best regards,
>>> Krzysztof
>>>
>