Re: [PATCH v2 2/5] dt-bindings: media: qcom,sm8150-camss: Add SM8150 CAMSS

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] dt-bindings: media: qcom,sm8150-camss: DTS example violates strict coding style
- [Low] dt-bindings: media: qcom,sm8150-camss: duplicate port schemas and property order
--

commit f1e0cef5355c3cc841108a7c44175130434fd78c
Author: Andrew Gigena <mail@andrewgigena.dev>

dt-bindings: media: qcom,sm8150-camss: Add SM8150 CAMSS

This commit adds a dedicated Devicetree binding schema for the Qualcomm
SM8150 Camera Subsystem (CAMSS), describing its CSIPHY, CSID, VFE, and
VFE-Lite instances along with its clocks, interrupts, IOMMU streams,
interconnects, and power domains.

> diff --git a/Documentation/devicetree/bindings/media/qcom,sm8150-camss.yaml b/Documentation/devicetree/bindings/media/qcom,sm8150-camss.yaml
> new file mode 100644
> index 0000000000000..26f8939001df2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/qcom,sm8150-camss.yaml
> @@ -0,0 +1,412 @@
[ ... ]
> +properties:
> +  compatible:
> +    const: qcom,sm8150-camss
> +
> +  clocks:
> +    minItems: 27
> +    maxItems: 27
[ ... ]
> +  interrupts:
> +    minItems: 12
> +    maxItems: 12
[ ... ]
> +  ports:
> +    $ref: /schemas/graph.yaml#/properties/ports
> +
> +    description:
> +      CSI input ports.
> +
> +    properties:
> +      port@0:
> +        $ref: /schemas/graph.yaml#/$defs/port-base
> +        unevaluatedProperties: false
> +        description:
> +          Input port for receiving CSI data from CSIPHY0.
> +
> +        properties:
> +          endpoint:
> +            $ref: video-interfaces.yaml#
> +            unevaluatedProperties: false
> +
> +            properties:
> +              data-lanes:
> +                minItems: 1
> +                maxItems: 4
> +
> +              bus-type:
> +                enum:
> +                  - 1 # MEDIA_BUS_TYPE_CSI2_CPHY
> +                  - 4 # MEDIA_BUS_TYPE_CSI2_DPHY
> +
> +            required:
> +              - data-lanes
> +
> +      port@1:
[ ... ]
> +      port@2:
[ ... ]
> +      port@3:
[ ... ]
> +  reg:
> +    minItems: 12
> +    maxItems: 12
> +
> +  reg-names:
> +    items:
> +      - const: csiphy0
> +      - const: csiphy1
> +      - const: csiphy2
> +      - const: csiphy3
> +      - const: vfe0
> +      - const: csid0
> +      - const: vfe1
> +      - const: csid1
> +      - const: vfe_lite0
> +      - const: csid_lite0
> +      - const: vfe_lite1
> +      - const: csid_lite1
> +
> +  vdda-phy-supply:
> +    description:
> +      0.88V supply to CSIPHY IP blocks.
> +
> +  vdda-pll-supply:
> +    description:
> +      1.2V supply to CSIPHY IP blocks.
> +
> +required:
> +  - clock-names
> +  - clocks
> +  - compatible

[Severity: Low]
This isn't a bug, but in Documentation/devicetree/bindings/media/qcom,sm8150-camss.yaml,
port@0 through port@3 under properties.ports duplicate the exact same
schema definition four times. Could this use patternProperties with
^port@[0-3]$ instead, matching recent Qualcomm CAMSS schemas such as
qcom,sm8650-camss.yaml, qcom,x1e80100-camss.yaml, and qcom,sm6350-camss.yaml?

Also, could reg and reg-names be placed immediately after compatible in
both properties and required (and clocks placed before clock-names in
required) to follow Documentation/devicetree/bindings/writing-schema.rst
and Documentation/devicetree/bindings/dts-coding-style.rst, and could the
redundant minItems entries on clocks, interrupts, and reg be dropped since
maxItems is already set to the same value?

[ ... ]
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/clock/qcom,gcc-sm8150.h>
> +    #include <dt-bindings/clock/qcom,sm8150-camcc.h>
> +    #include <dt-bindings/interconnect/qcom,sm8150.h>
> +    #include <dt-bindings/power/qcom-rpmpd.h>
> +
> +    soc {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +
> +        camss: camss@ac65000 {
> +            compatible = "qcom,sm8150-camss";
> +
> +            reg = <0 0xac65000 0 0x1000>,
> +              <0 0xac66000 0 0x1000>,
[ ... ]
> +            iommus = <&apps_smmu 0x840 0x620>,
> +              <&apps_smmu 0x860 0x620>,
> +              <&apps_smmu 0xA40 0x620>,
> +              <&apps_smmu 0xA60 0x620>,
> +              <&apps_smmu 0xC40 0x620>,
> +              <&apps_smmu 0xC60 0x620>,
> +              <&apps_smmu 0xE40 0x620>,
> +              <&apps_smmu 0xE60 0x620>;
> +
> +            interconnects = <&gem_noc MASTER_AMPSS_M0 0 &config_noc SLAVE_CAMERA_CFG 0>,
> +                <&mmss_noc MASTER_CAMNOC_HF0 0 &mc_virt SLAVE_EBI_CH0 0>,
> +                <&mmss_noc MASTER_CAMNOC_HF1 0 &mc_virt SLAVE_EBI_CH0 0>,
> +                <&mmss_noc MASTER_CAMNOC_SF 0 &mc_virt SLAVE_EBI_CH0 0>;
> +
> +            interconnect-names = "cam_ahb",
> +              "cam_hf_0_mnoc",
> +              "cam_hf_1_mnoc",
> +              "cam_sf_0_mnoc";
> +
> +            vdda-phy-supply = <&vreg_l5a_0p88>;
> +            vdda-pll-supply = <&vreg_l3c_1p2>;
> +
> +            ports {
> +                #address-cells = <1>;
> +                #size-cells = <0>;
> +
> +                port@0 {
> +                    reg = <0>;
> +
> +                    csiphy_ep0: endpoint {
> +                        data-lanes = <0 1 2 3>;
> +                        remote-endpoint = <&sensor_ep>;
> +                    };

[Severity: Low]
When running scripts/dtc/dt-check-style --mode=strict on
Documentation/devicetree/bindings/media/qcom,sm8150-camss.yaml, the
examples block triggers several style warnings:

1. Continuation lines in reg, reg-names, interrupts, interrupt-names,
power-domains, power-domain-names, clocks, clock-names, iommus,
interconnects, and interconnect-names are indented by 2 or 4 spaces instead
of aligning with the opening < or quote character on the first line.

2. The iommus entries 0xA40, 0xA60, 0xC40, 0xC60, 0xE40, and 0xE60 use
uppercase hex digits instead of lowercase.

3. The first entry of interconnects exceeds 80 columns in the extracted
example.

4. The labels camss and csiphy_ep0 are defined but never referenced in the
example.

In addition, dt-bindings/power/qcom-rpmpd.h is not used in the example, and
the node is named camss@ac65000 instead of the generic isp@ac65000 node
name. Could these be cleaned up in the example?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106-sm8150-v7.4-rc1-camss-v2-0-39ee37787b00@andrewgigena.dev?part=2