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]

 



On 09/07/2024 17:06, Depeng Shao wrote:
Add bindings for qcom,sm8550-camss in order to support the camera
subsystem for sm8550

Co-developed-by: Yongsheng Li <quic_yon@xxxxxxxxxxx>
Signed-off-by: Yongsheng Li <quic_yon@xxxxxxxxxxx>
Signed-off-by: Depeng Shao <quic_depengs@xxxxxxxxxxx>
---

Hey Depeng,

Thank you for your submission, looks good. I have a few comments below.

+  clock-names:
+    items:
+      - const: cam_ahb_clk
+      - const: cam_hf_axi
+      - const: cam_sf_axi
+      - const: camnoc_axi
+      - const: camnoc_axi_src

These "_src" clocks are almost certainly not necessary. The CAMCC should have camnoc_axi_src as the parent clock of camnoc_axi, so you don't need ot list the "_src" clock.

Please go through this list in your yaml, dts and .c code and remove.

There may be an exception where a _src clock is required but my expectation is that all of those _src clocks can be removed.


+  power-domains:
+    items:
+      - description: IFE0 GDSC - Image Front End, Global Distributed Switch Controller.
+      - description: IFE1 GDSC - Image Front End, Global Distributed Switch Controller.
+      - description: IFE2 GDSC - Image Front End, Global Distributed Switch Controller.
+      - description: Titan GDSC - Titan ISP Block, Global Distributed Switch Controller.

Please name these power-domains in the next iteration of the series.

The dependency on the ordering of the power-domains is a bug which new SoC submissions won't be replicating.

https://lore.kernel.org/linux-arm-msm/fcdb072d-6099-4423-b4b5-21e9052b82cc@xxxxxxxxxx/

+
+            interconnects =
+                    <&gem_noc         MASTER_APPSS_PROC          0 &config_noc SLAVE_CAMERA_CFG  0>,
+                    <&mmss_noc        MASTER_CAMNOC_HF           0 &mc_virt SLAVE_EBI1           0>,
+                    <&mmss_noc        MASTER_CAMNOC_SF           0 &mc_virt SLAVE_EBI1           0>,
+                    <&mmss_noc        MASTER_CAMNOC_ICP          0 &mc_virt SLAVE_EBI1           0>;

This tabbing looks weird, do none of the checking tools complain about it ?

See:
Documentation/devicetree/bindings/media/qcom,sc8280xp-camss.yaml

---
bod




[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux