Re: [RFC PATCH 01/14] media: dt-bindings: media: camss: Add qcom,msm8974-camss binding

[Rob Herring <robh@kernel.org>]

On Mon, May 23, 2022 at 12:07:10PM +0200, Krzysztof Kozlowski wrote:
> On 22/05/2022 18:27, Luca Weiss wrote:
> > From: Matti Lehtimäki <matti.lehtimaki@gmail.com>
> 
> Thank you for your patch. There is something to discuss/improve.
> 
> > 
> > Add bindings for qcom,msm8974-camss in order to support the camera
> > subsystem on MSM8974.
> > 
> > Signed-off-by: Matti Lehtimäki <matti.lehtimaki@gmail.com>
> > Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
> > ---
> >  .../bindings/media/qcom,msm8974-camss.yaml    | 321 ++++++++++++++++++
> >  1 file changed, 321 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/media/qcom,msm8974-camss.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/media/qcom,msm8974-camss.yaml b/Documentation/devicetree/bindings/media/qcom,msm8974-camss.yaml
> > new file mode 100644
> > index 000000000000..f8f71e477535
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/qcom,msm8974-camss.yaml
> > @@ -0,0 +1,321 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +
> > +%YAML 1.2
> > +---
> > +$id: "http://devicetree.org/schemas/media/qcom,msm8974-camss.yaml#"
> > +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> > +
> > +title: Qualcomm CAMSS ISP
> > +
> > +maintainers:
> > +  - Robert Foss <robert.foss@linaro.org>
> > +
> > +description: |
> > +  The CAMSS IP is a CSI decoder and ISP present on Qualcomm platforms
> > +
> > +properties:
> > +  compatible:
> > +    const: qcom,msm8974-camss
> > +
> > +  clocks:
> > +    minItems: 31
> 
> No need for minItems, they are equal to max by default.
> 
> > +    maxItems: 31
> > +
> > +  clock-names:
> > +    items:
> > +      - const: top_ahb
> > +      - const: ispif_ahb
> > +      - const: csiphy0_timer
> > +      - const: csiphy1_timer
> > +      - const: csiphy2_timer
> > +      - const: csi0_ahb
> > +      - const: csi0
> > +      - const: csi0_phy
> > +      - const: csi0_pix
> > +      - const: csi0_rdi
> > +      - const: csi1_ahb
> > +      - const: csi1
> > +      - const: csi1_phy
> > +      - const: csi1_pix
> > +      - const: csi1_rdi
> > +      - const: csi2_ahb
> > +      - const: csi2
> > +      - const: csi2_phy
> > +      - const: csi2_pix
> > +      - const: csi2_rdi
> > +      - const: csi3_ahb
> > +      - const: csi3
> > +      - const: csi3_phy
> > +      - const: csi3_pix
> > +      - const: csi3_rdi
> > +      - const: vfe0
> > +      - const: vfe1
> > +      - const: csi_vfe0
> > +      - const: csi_vfe1
> > +      - const: iface
> > +      - const: bus
> > +
> > +  interrupts:
> > +    minItems: 10
> 
> Same.
> 
> > +    maxItems: 10
> > +
> > +  interrupt-names:
> > +    items:
> > +      - const: csiphy0
> > +      - const: csiphy1
> > +      - const: csiphy2
> > +      - const: csid0
> > +      - const: csid1
> > +      - const: csid2
> > +      - const: csid3
> > +      - const: ispif
> > +      - const: vfe0
> > +      - const: vfe1
> > +
> > +  power-domains:
> > +    items:
> > +      - description: VFE GDSC - Video Front End, Global Distributed Switch Controller.
> > +
> > +  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.
> > +
> > +        properties:
> > +          endpoint:
> > +            $ref: video-interfaces.yaml#
> > +            unevaluatedProperties: false
> > +
> > +            properties:
> > +              clock-lanes:
> > +                items:
> > +                  - const: 1
> > +
> > +              data-lanes:
> > +                minItems: 1
> > +                maxItems: 4
> > +
> > +            required:
> > +              - clock-lanes
> > +              - data-lanes
> > +
> > +      port@1:
> 
> These look all the same, so just use patternPropreties (in "ports") with
> proper pattern.

That's fine, but whatever difference there is in ports 0, 1, and 2 needs 
to be described.

> > +        $ref: /schemas/graph.yaml#/$defs/port-base
> > +        unevaluatedProperties: false
> > +        description:
> > +          Input port for receiving CSI data.