| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <ed012367-1bfd-4eef-931b-37e1ac839176@quicinc.com>
Date: Wed, 25 Sep 2024 23:13:17 +0800
From: Depeng Shao <quic_depengs@...cinc.com>
To: Vladimir Zapolskiy <vladimir.zapolskiy@...aro.org>, <rfoss@...nel.org>,
<todor.too@...il.com>, <bryan.odonoghue@...aro.org>,
<mchehab@...nel.org>, <robh@...nel.org>, <krzk+dt@...nel.org>,
<conor+dt@...nel.org>
CC: <linux-arm-msm@...r.kernel.org>, <linux-media@...r.kernel.org>,
<devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<kernel@...cinc.com>, Yongsheng Li <quic_yon@...cinc.com>
Subject: Re: [PATCH 07/13] dt-bindings: media: camss: Add qcom,sm8550-camss
binding
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@kernel.org/
>> + 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@...4000/ports/port@...ndpoint@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@...4000/ports/port@...ndpoint@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@...4000/ports/port@...ndpoint@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@...4000/ports/port@...ndpoint@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@...4000/ports/port@...ndpoint@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
Powered by blists - more mailing lists