On 12/07/2022 12:25, Laurent Pinchart wrote: > Hi Krzysztof, > > On Tue, Jul 12, 2022 at 09:49:45AM +0200, Krzysztof Kozlowski wrote: >> On 12/07/2022 02:02, Laurent Pinchart wrote: >>> The Image Sensing Interface (ISI) combines image processing pipelines >>> with DMA engines to process and capture frames originating from a >>> variety of sources. The inputs to the ISI go through Pixel Link >>> interfaces, and their number and nature is SoC-dependent. They cover >>> both capture interfaces (MIPI CSI-2 RX, HDMI RX) and memory inputs. >>> >>> Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> >>> --- >>> Changes since v1: >>> >>> - Fix compatible string checks in conditional schema >>> - Fix interrupts property handling >>> --- >>> .../bindings/media/nxp,imx8-isi.yaml | 148 ++++++++++++++++++ >>> 1 file changed, 148 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/media/nxp,imx8-isi.yaml >>> >>> diff --git a/Documentation/devicetree/bindings/media/nxp,imx8-isi.yaml b/Documentation/devicetree/bindings/media/nxp,imx8-isi.yaml >>> new file mode 100644 >>> index 000000000000..390dfa03026b >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/media/nxp,imx8-isi.yaml >>> @@ -0,0 +1,148 @@ >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/media/nxp,imx8-isi.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: i.MX8 Image Sensing Interface >>> + >>> +maintainers: >>> + - Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> >>> + >>> +description: | >>> + The Image Sensing Interface (ISI) combines image processing pipelines with >>> + DMA engines to process and capture frames originating from a variety of >>> + sources. The inputs to the ISI go through Pixel Link interfaces, and their >>> + number and nature is SoC-dependent. They cover both capture interfaces (MIPI >>> + CSI-2 RX, HDMI RX, ...) and display engine outputs for writeback support. >>> + >>> +properties: >>> + compatible: >>> + enum: >>> + - fsl,imx8mn-isi >>> + - fsl,imx8mp-isi >>> + >>> + reg: >>> + maxItems: 1 >>> + >>> + clocks: >>> + items: >>> + - description: The AXI clock >>> + - description: The APB clock >>> + # TODO: Check if the per-channel ipg_proc_clk clocks need to be specified >>> + # as well, in case some SoCs have the ability to control them separately. >>> + # This may be the case of the i.MX8[DQ]X(P) >>> + >>> + clock-names: >>> + items: >>> + - const: axi >>> + - const: apb >>> + >>> + fsl,blk-ctrl: >>> + $ref: /schemas/types.yaml#/definitions/phandle >>> + description: >>> + A phandle referencing the block control that contains the CSIS to ISI >>> + gasket. >>> + >>> + interrupts: true >> >> Need generic constraints - min/maxItems. > > I can't set maxItems here, as the value depends on the compatible > string. It's set below as part of the "allOf". I could set minItems to > 1, but I don't really see a point in doing so. Of course you can, just like all other files could. https://elixir.bootlin.com/linux/v5.19-rc6/source/Documentation/devicetree/bindings/clock/samsung,exynos7-clock.yaml#L57 > >>> + >>> + power-domains: true >> >> Ditto. > > I'll fix this one. > >>> + >>> + ports: >>> + $ref: /schemas/graph.yaml#/properties/ports >>> + description: | >>> + Ports represent the Pixel Link inputs to the ISI. Their number and >>> + assignment are model-dependent. Each port shall have a single endpoint. >>> + >>> + patternProperties: >>> + "^port@[0-9]$": >>> + $ref: /schemas/graph.yaml#/properties/port >>> + unevaluatedProperties: false >>> + >>> + unevaluatedProperties: false >> >> At least one port is always required? > > That's a fair assumption I think. How would you express that ? There's > no patternRequired as far as I know. Note that the device-dependent > ports are described in the "allOf" section below, where "required" is > set per device model. required: - port@0 > >>> + >>> +required: >>> + - compatible >>> + - reg >>> + - interrupts >>> + - clocks >>> + - clock-names >>> + - fsl,blk-ctrl >>> + - ports >>> + >>> +allOf: >>> + - if: >>> + properties: >>> + compatible: >>> + contains: >>> + const: fsl,imx8mn-isi >>> + then: >>> + properties: >>> + interrupts: >>> + maxItems: 1 >>> + ports: >>> + properties: >>> + port@0: >>> + description: MIPI CSI-2 RX >>> + required: >>> + - port@0 >>> + >>> + - if: >>> + properties: >>> + compatible: >>> + contains: >>> + const: fsl,imx8mp-isi >>> + then: >>> + properties: >>> + interrupts: >>> + maxItems: 2 >> >> You need to describe the items. > > It's one interrupt per pipeline. Can I add the description to the > generic interrupts property instead of documenting each item > individually ? Something along the lines of > > interrupts: > description: Processing pipeline interrupts, one per pipeline > This sounds good, thanks! Best regards, Krzysztof
