Re: [PATCH v2 6/7] dt-bindings: media: Add i.MX8 ISI DT bindings

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

On 12/07/2022 16:26, Laurent Pinchart wrote:
> Hi Krzysztof,
> 
> On Tue, Jul 12, 2022 at 02:49:06PM +0200, Krzysztof Kozlowski wrote:
>> On 12/07/2022 12:25, Laurent Pinchart wrote:
>>> 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@ideasonboard.com>
>>>>> ---
>>>>> 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@ideasonboard.com>
>>>>> +
>>>>> +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
> 
> I meant I can't set a fixed maximum, other than the max of all max. Is
> there a point in doing do ?

Yes, that's the practice. makes code easier to read and notice all
constraints.


Best regards,
Krzysztof