Re: [PATCH 07/13] dt-bindings: media: camss: Add qcom,sm8550-camss binding

[Depeng Shao <quic_depengs@quicinc.com>]

Hi Bryan,

On 9/25/2024 11:40 PM, Depeng Shao wrote:
> Hi Vladimir, Bryan,
> 
> On 9/18/2024 7:16 AM, Vladimir Zapolskiy wrote:
>> Hi Bryan,
>>
>> On 9/18/24 01:40, Bryan O'Donoghue wrote:
>>> On 13/09/2024 06:06, Vladimir Zapolskiy wrote:
>>>> On 9/13/24 01:41, Bryan O'Donoghue wrote:
>>>>> On 12/09/2024 21:57, Vladimir Zapolskiy wrote:
>>>>>>> 3. Required not optional in the yaml
>>>>>>>
>>>>>>>        => You can't use the PHY without its regulators
>>>>>>
>>>>>> No, the supplies shall be optional, since it's absolutely possible to
>>>>>> have
>>>>>> such a board, where supplies are merely not connected to the SoC.
>>>>>
>>>>> For any _used_ PHY both supplies are certainly required.
>>>>>
>>>>> That's what the yaml/dts check for this should achieve.
>>>>
>>>> I believe it is technically possible by writing an enormously complex
>>>> scheme, when all possible "port" cases and combinations are listed.
>>>>
>>>> Do you see any simpler way? Do you insist that it is utterly needed?
>>>
>>> I asked Krzysztof about this offline.
>>>
>>> He said something like
>>>
>>> Quote:
>>> This is possible, but I think not between child nodes.
>>> https://elixir.bootlin.com/linux/v6.11-rc7/source/Documentation/ 
>>> devicetree/bindings/example-schema.yaml#L194
>>>
>>> You could require something in children, but not in parent node. For
>>> children something around:
>>> https://elixir.bootlin.com/linux/v6.4-rc7/source/Documentation/ 
>>> devicetree/bindings/net/qcom,ipa.yaml#L174
>>>
>>> allOf:
>>>     - if:
>>>         required:
>>>           - something-in-parent
>>>       then:
>>>         properties:
>>>           child-node:
>>>             required:
>>>               - something-in-child
>>>
>>> I will see if I can turn that into a workable proposal/patch.
>>>
>>
>> thank you for pushing my review request forward.
>>
>> Overall I believe making supply properties as optional ones is 
>> sufficient,
>> technically straightforward and merely good enough, thus please let me
>> ask you to ponder on this particular variant one more time.
>>
> 
> So, we are discussing two things.
> 
> 1# Use separate supplies for each CSI block, looks like there is no 
> doubt about it anymore. So, I will update it just like based on suggestion.
> 
> csiphyX-vdda-phy-supply
> csiphyX-vdda-pll-supply
> 
> Then I will need below items in the required list if they are required.
> required:
>    - csiphy0-vdda-phy-supply
>    - csiphy0-vdda-pll-supply
>    - csiphy1-vdda-phy-supply
>    - csiphy1-vdda-pll-supply
> ...
>    - csiphy7-vdda-phy-supply
>    - csiphy7-vdda-pll-supply
> 
> 2# Regarding the CSI supplies, if they need to be making as optional?
> Looks like there is no conclusion now.
> 
> @Bryan, do you agree with this?
> 

I'm preparing the new version patches, and will send out for reviewing 
in few days. I will follow Vladimir's comments if you have no response, 
it means making supply properties as optional one, so they won't be 
added to the required list.

Thanks,
Depeng