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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux