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

[Depeng Shao]

Hi Vladimir,

On 8/1/2024 8:05 AM, Vladimir Zapolskiy wrote:
> On 7/9/24 19:06, Depeng Shao wrote:
> > Add bindings for qcom,sm8550-camss in order to support the camera
> > subsystem for sm8550

> > +
> > +  clock-names:
> > +    items:
> > +      - const: cam_ahb_clk
> > +      - const: cam_hf_axi
> > +      - const: cam_sf_axi
> > +      - const: camnoc_axi
> > +      - const: camnoc_axi_src
> > +      - const: core_ahb
> > +      - const: cpas_ahb
> > +      - const: slow_ahb_src
> > +      - const: csiphy0
> > +      - const: csiphy0_timer
> > +      - const: csiphy1
> > +      - const: csiphy1_timer
> > +      - const: csiphy2
> > +      - const: csiphy2_timer
> > +      - const: csiphy3
> > +      - const: csiphy3_timer
> > +      - const: csiphy4
> > +      - const: csiphy4_timer
> > +      - const: csiphy5
> > +      - const: csiphy5_timer
> > +      - const: csiphy6
> > +      - const: csiphy6_timer
> > +      - const: csiphy7
> > +      - const: csiphy7_timer
> > +      - const: csid_src
> > +      - const: csid
> > +      - const: csiphy_rx
> > +      - const: vfe0_fast_ahb
> > +      - const: vfe0_src
> > +      - const: vfe0
> > +      - const: cpas_vfe0
> > +      - const: vfe1_fast_ahb
> > +      - const: vfe1_src
> > +      - const: vfe1
> > +      - const: cpas_vfe2
> > +      - const: vfe2_fast_ahb
> > +      - const: vfe2_src
> > +      - const: vfe2
> > +      - const: cpas_vfe1
> > +      - const: vfe_lite_ahb
> > +      - const: vfe_lite_csid_src
> > +      - const: vfe_lite
> > +      - const: vfe_lite_cphy_rx
> > +      - const: vfe_lite_csid
> > +      - const: cpas_ife_lite
> > +      - const: cpas_fast_ahb_clk
> > +      - const: fast_ahb_src
>
> I believe most of *_src clocks should be removed from the list above as
> parent clocks with no need for own separate management.

Sure, Bryan has same comment, I have removed the *_src clk locally, this 
change will be in new version patch.


> > +
> > +required:
> > +  - clock-names
> > +  - clocks
> > +  - compatible
>
> I would suggest to put 'compatible', 'reg' and 'reg-names' properties as
> the first ones. 'clock-names' should follow 'clocks' property in the list.

Thanks for the suggestion, will add it in next version patch.

> > +  - interconnects
> > +  - interconnect-names
> > +  - interrupts
> > +  - interrupt-names
> > +  - iommus
> > +  - power-domains
> > +  - reg
> > +  - reg-names
> > +  - vdda-phy-supply
> > +  - vdda-pll-supply
> > +  - ports
> > +

Thanks,
Depeng