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 Vladimir,

On 9/6/2024 11:56 PM, Vladimir Zapolskiy wrote:

+            compatible = "qcom,sm8550-camss";
+
+            reg = <0 0x0acb7000 0 0xd00>,
+                  <0 0x0acb9000 0 0xd00>,
+                  <0 0x0acbb000 0 0xd00>,
+                  <0 0x0acca000 0 0xa00>,
+                  <0 0x0acce000 0 0xa00>,
+                  <0 0x0acb6000 0 0x1000>,
+                  <0 0x0ace4000 0 0x2000>,
+                  <0 0x0ace6000 0 0x2000>,
+                  <0 0x0ace8000 0 0x2000>,
+                  <0 0x0acea000 0 0x2000>,
+                  <0 0x0acec000 0 0x2000>,
+                  <0 0x0acee000 0 0x2000>,
+                  <0 0x0acf0000 0 0x2000>,
+                  <0 0x0acf2000 0 0x2000>,
+                  <0 0x0ac62000 0 0xf000>,
+                  <0 0x0ac71000 0 0xf000>,
+                  <0 0x0ac80000 0 0xf000>,
+                  <0 0x0accb000 0 0x2800>,
+                  <0 0x0accf000 0 0x2800>;

Please sort the list above in numerical order, this will change positions
of "vfe_lite0", "vfe_lite1" etc.

Another note, since it's not possible to map less than a page, so I believe
it might make sense to align all sizes to 0x1000.


Sure, I previously sorted by the alphabetical order of reg_name.
I will update it based on your suggestion. And will also make sure the align all sizes to 0x1000.


+                     <&camcc CAM_CC_IFE_LITE_CLK>,
+                     <&camcc CAM_CC_IFE_LITE_AHB_CLK>,
+                     <&camcc CAM_CC_IFE_LITE_CPHY_RX_CLK>,
+                     <&camcc CAM_CC_IFE_LITE_CSID_CLK>,
+                     <&gcc GCC_CAMERA_HF_AXI_CLK>;

Could you please put the &gcc provided clock as the first one in the list?


Sure, will do.

+
+            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_ICP 0 &mc_virt SLAVE_EBI1 0>, +                            <&mmss_noc MASTER_CAMNOC_SF 0 &mc_virt SLAVE_EBI1 0>;
+            interconnect-names = "ahb",
+                                 "hf_0_mnoc",
+                                 "icp_mnoc",
+                                 "sf_0_mnoc";

Just a note for myself, interconnect names lost "cam_" prefix, and it should
be fine.

Krzysztof posted a comment in a SC7280 camss change and asked to remove the "cam_" prefix.


https://lore.kernel.org/all/087e7f29-1fa8-4bc2-bb3d-acb941432381@xxxxxxxxxx/


+            ports {
+                #address-cells = <1>;
+                #size-cells = <0>;

In case of a single child node #address-cells/#size-cells could be omitted,
if I'm not mistaken about it...


I tried it, but dt_binding_check report below warning.

Documentation/devicetree/bindings/media/qcom,sm8550-camss.example.dts:221.29-39: Warning (reg_format): /example-0/soc/camss@ace4000/ports/port@0/endpoint@0:reg: property has invalid length (4 bytes) (#address-cells == 2, #size-cells == 1) Documentation/devicetree/bindings/media/qcom,sm8550-camss.example.dtb: Warning (pci_device_reg): Failed prerequisite 'reg_format' Documentation/devicetree/bindings/media/qcom,sm8550-camss.example.dtb: Warning (pci_device_bus_num): Failed prerequisite 'reg_format' Documentation/devicetree/bindings/media/qcom,sm8550-camss.example.dtb: Warning (simple_bus_reg): Failed prerequisite 'reg_format' Documentation/devicetree/bindings/media/qcom,sm8550-camss.example.dtb: Warning (i2c_bus_reg): Failed prerequisite 'reg_format' Documentation/devicetree/bindings/media/qcom,sm8550-camss.example.dtb: Warning (spi_bus_reg): Failed prerequisite 'reg_format' Documentation/devicetree/bindings/media/qcom,sm8550-camss.example.dts:220.48-225.27: Warning (avoid_default_addr_size): /example-0/soc/camss@ace4000/ports/port@0/endpoint@0: Relying on default #address-cells value Documentation/devicetree/bindings/media/qcom,sm8550-camss.example.dts:220.48-225.27: Warning (avoid_default_addr_size): /example-0/soc/camss@ace4000/ports/port@0/endpoint@0: Relying on default #size-cells value Documentation/devicetree/bindings/media/qcom,sm8550-camss.example.dtb: Warning (unique_unit_address_if_enabled): Failed prerequisite 'avoid_default_addr_size' Documentation/devicetree/bindings/media/qcom,sm8550-camss.example.dts:220.48-225.27: Warning (graph_endpoint): /example-0/soc/camss@ace4000/ports/port@0/endpoint@0: graph node '#address-cells' is -1, must be 1 Documentation/devicetree/bindings/media/qcom,sm8550-camss.example.dts:220.48-225.27: Warning (graph_endpoint): /example-0/soc/camss@ace4000/ports/port@0/endpoint@0: graph node '#size-cells' is -1, must be 0

+                port@0 {
+                    reg = <0>;
+                    #address-cells = <1>;
+                    #size-cells = <0>;

Same as above.


Same warning..

+
+                    csiphy_ep0: endpoint@0 {
+                        reg = <0>;
+                        clock-lanes = <7>;
+                        data-lanes = <0 1>;
+                        remote-endpoint = <&sensor_ep>;
+                    };
+                };
+            };
+        };
+    };


---
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