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

Rob Herring
2022-06-02T13:02:00Z

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.