[v5,1/4] dt-bindings: media: Document bindings for DW MIPI CSI-2 Host

Add bindings for Synopsys DesignWare MIPI CSI-2 host.

Signed-off-by: Luis Oliveira <lolivei@synopsys.com>
[eugen.hristev@microchip.com: reworked binding, converted to yaml]
Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
---
 .../bindings/media/snps,dw-csi.yaml           | 149 ++++++++++++++++++
 1 file changed, 149 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/snps,dw-csi.yaml

On Fri, Dec 16, 2022 at 04:37:14PM +0200, Eugen Hristev wrote:
> Add bindings for Synopsys DesignWare MIPI CSI-2 host.
> 
> Signed-off-by: Luis Oliveira <lolivei@synopsys.com>
> [eugen.hristev@microchip.com: reworked binding, converted to yaml]
> Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
> ---
>  .../bindings/media/snps,dw-csi.yaml           | 149 ++++++++++++++++++
>  1 file changed, 149 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/snps,dw-csi.yaml
> 
> diff --git a/Documentation/devicetree/bindings/media/snps,dw-csi.yaml b/Documentation/devicetree/bindings/media/snps,dw-csi.yaml
> new file mode 100644
> index 000000000000..439eadc8e517
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/snps,dw-csi.yaml
> @@ -0,0 +1,149 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/snps,dw-csi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Synopsys DesignWare CSI-2 Host controller (csi2host)
> +
> +maintainers:
> +  - Eugen Hristev <eugen.hristev@microchip.com>
> +
> +description:
> +  CSI2HOST is used to receive image coming from an MIPI CSI-2 compatible
> +  camera. It will convert the incoming CSI-2 stream into a dedicated
> +  interface called the Synopsys IDI (Image Data Interface).
> +  This interface is a 32-bit SoC internal only, and can be assimilated
> +  with a CSI-2 interface.
> +
> +properties:
> +  compatible:
> +    const: snps,dw-csi

This needs an SoC specific compatible.

> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 2
> +
> +  clock-names:
> +    description:
> +      CSI2HOST can have two clocks connected. One clock is the
> +      peripheral clock for the inside functionality of the hardware block.
> +      This is named 'perclk'. The second clock can be the phy clock,
> +      which is used to clock the phy via an internal link.
> +      This clock is named 'phyclk', phy clock.

The schema says most of this, don't repeat it in freeform text. If you 
want a description of each clock, that goes 'clocks' like this:

items:
  - description: ...
  - description: ...

> +    items:
> +      - const: perclk
> +      - const: phyclk
> +
> +  phys:
> +    maxItems: 1
> +    description: MIPI D-PHY
> +
> +  phy-names:
> +    items:
> +      - const: dphy
> +
> +  resets:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  ports:
> +    $ref: /schemas/graph.yaml#/properties/ports
> +
> +    properties:
> +      port@0:
> +        $ref: /schemas/graph.yaml#/$defs/port-base
> +        unevaluatedProperties: false
> +        description:
> +          Input port node, single endpoint describing the input port.
> +
> +        properties:
> +          endpoint:
> +            $ref: video-interfaces.yaml#
> +            unevaluatedProperties: false
> +            description: Endpoint connected to input device

Don't need generic descriptions. Only the port nodes need to define what 
they are.

> +
> +            properties:
> +              bus-type:
> +                const: 4
> +
> +              data-lanes:
> +                minItems: 1
> +                maxItems: 4
> +                items:
> +                  maximum: 4
> +
> +              clock-lanes:
> +                maxItems: 1
> +
> +              remote-endpoint: true

Don't need to list this.

> +
> +      port@1:
> +        $ref: /schemas/graph.yaml#/$defs/port-base
> +        unevaluatedProperties: false
> +        description:
> +          Output port node, single endpoint describing the output port.
> +
> +        properties:
> +          endpoint:
> +            unevaluatedProperties: false
> +            $ref: video-interfaces.yaml#
> +            description: Endpoint connected to output device
> +
> +            properties:
> +              bus-type:
> +                const: 4
> +
> +              remote-endpoint: true

Ditto.

> +
> +    required:
> +      - port@0
> +      - port@1
> +
> +additionalProperties: false
> +
> +required:
> +  - compatible
> +  - ports
> +
> +examples:
> +  - |
> +    csi2: csi2@3000 {
> +        compatible = "snps,dw-csi";
> +        reg = <0x03000 0x7FF>;
> +        phys = <&mipi_dphy_rx>;
> +        phy-names = "dphy";
> +        resets = <&dw_rst 1>;
> +        interrupts = <2>;
> +
> +        ports {
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +            port@0 {
> +                reg = <0>;
> +
> +                csi_ep1: endpoint {
> +                    bus-type = <4>; /* MIPI CSI2 D-PHY */
> +                    remote-endpoint = <&camera_1>;
> +                    data-lanes = <1 2>;
> +                    clock-lanes = <0>;
> +                };
> +            };
> +
> +            port@1 {
> +                reg = <1>;
> +
> +                csi_ep2: endpoint {
> +                    remote-endpoint = <&idi_receiver>;
> +                    bus-type = <4>;
> +                };
> +            };
> +        };
> +    };
> +
> +...
> -- 
> 2.25.1
> 
>

On 16/12/2022 15:37, Eugen Hristev wrote:
> Add bindings for Synopsys DesignWare MIPI CSI-2 host.
> 
> Signed-off-by: Luis Oliveira <lolivei@synopsys.com>
> [eugen.hristev@microchip.com: reworked binding, converted to yaml]
> Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
> ---

1. Please use scripts/get_maintainers.pl to get a list of necessary
people and lists to CC.  It might happen, that command when run on an
older kernel, gives you outdated entries.  Therefore please be sure you
base your patches on recent Linux kernel.

You did not CC anyone, so who is supposed to take this patch?

2. Subject: drop second, redundant "bindings for".

>  .../bindings/media/snps,dw-csi.yaml           | 149 ++++++++++++++++++
>  1 file changed, 149 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/snps,dw-csi.yaml
> 
> diff --git a/Documentation/devicetree/bindings/media/snps,dw-csi.yaml b/Documentation/devicetree/bindings/media/snps,dw-csi.yaml
> new file mode 100644
> index 000000000000..439eadc8e517
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/snps,dw-csi.yaml
> @@ -0,0 +1,149 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/snps,dw-csi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Synopsys DesignWare CSI-2 Host controller (csi2host)
> +
> +maintainers:
> +  - Eugen Hristev <eugen.hristev@microchip.com>
> +
> +description:
> +  CSI2HOST is used to receive image coming from an MIPI CSI-2 compatible
> +  camera. It will convert the incoming CSI-2 stream into a dedicated
> +  interface called the Synopsys IDI (Image Data Interface).
> +  This interface is a 32-bit SoC internal only, and can be assimilated
> +  with a CSI-2 interface.
> +
> +properties:
> +  compatible:
> +    const: snps,dw-csi
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 2
> +
> +  clock-names:
> +    description:
> +      CSI2HOST can have two clocks connected. One clock is the
> +      peripheral clock for the inside functionality of the hardware block.
> +      This is named 'perclk'. The second clock can be the phy clock,
> +      which is used to clock the phy via an internal link.
> +      This clock is named 'phyclk', phy clock.
> +    items:
> +      - const: perclk
> +      - const: phyclk

Drop "clk" from both


> +
> +  phys:
> +    maxItems: 1
> +    description: MIPI D-PHY
> +
> +  phy-names:
> +    items:
> +      - const: dphy
> +
> +  resets:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  ports:
> +    $ref: /schemas/graph.yaml#/properties/ports
> +
> +    properties:
> +      port@0:
> +        $ref: /schemas/graph.yaml#/$defs/port-base
> +        unevaluatedProperties: false
> +        description:
> +          Input port node, single endpoint describing the input port.
> +
> +        properties:
> +          endpoint:
> +            $ref: video-interfaces.yaml#
> +            unevaluatedProperties: false
> +            description: Endpoint connected to input device
> +
> +            properties:
> +              bus-type:
> +                const: 4
> +
> +              data-lanes:
> +                minItems: 1
> +                maxItems: 4
> +                items:
> +                  maximum: 4
> +
> +              clock-lanes:
> +                maxItems: 1
> +
> +              remote-endpoint: true
> +
> +      port@1:
> +        $ref: /schemas/graph.yaml#/$defs/port-base
> +        unevaluatedProperties: false
> +        description:
> +          Output port node, single endpoint describing the output port.
> +
> +        properties:
> +          endpoint:
> +            unevaluatedProperties: false
> +            $ref: video-interfaces.yaml#
> +            description: Endpoint connected to output device
> +
> +            properties:
> +              bus-type:
> +                const: 4
> +
> +              remote-endpoint: true
> +
> +    required:
> +      - port@0
> +      - port@1
> +
> +additionalProperties: false
> +
> +required:
> +  - compatible
> +  - ports

reg? phys? interrupts? All others?

> +
> +examples:
> +  - |
> +    csi2: csi2@3000 {

Generic node names, so "csi"

> +        compatible = "snps,dw-csi";
> +        reg = <0x03000 0x7FF>;

lowercase hex

> +        phys = <&mipi_dphy_rx>;
> +        phy-names = "dphy";
> +        resets = <&dw_rst 1>;
> +        interrupts = <2>;
> +

Best regards,
Krzysztof

On 12/20/22 16:16, Krzysztof Kozlowski wrote:
> On 16/12/2022 15:37, Eugen Hristev wrote:
>> Add bindings for Synopsys DesignWare MIPI CSI-2 host.
>>
>> Signed-off-by: Luis Oliveira <lolivei@synopsys.com>
>> [eugen.hristev@microchip.com: reworked binding, converted to yaml]
>> Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
>> ---
> 
> 1. Please use scripts/get_maintainers.pl to get a list of necessary
> people and lists to CC.  It might happen, that command when run on an
> older kernel, gives you outdated entries.  Therefore please be sure you
> base your patches on recent Linux kernel.
> 
> You did not CC anyone, so who is supposed to take this patch?

Hi Krzysztof,

Thanks for looking at this . To answer you : nobody . Please read the 
cover letter , I am sending this to publish my work on this driver, as 
it may help someone out there.

Eugen

> 
> 2. Subject: drop second, redundant "bindings for".
> 
>>   .../bindings/media/snps,dw-csi.yaml           | 149 ++++++++++++++++++
>>   1 file changed, 149 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/media/snps,dw-csi.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/media/snps,dw-csi.yaml b/Documentation/devicetree/bindings/media/snps,dw-csi.yaml
>> new file mode 100644
>> index 000000000000..439eadc8e517
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/snps,dw-csi.yaml
>> @@ -0,0 +1,149 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/media/snps,dw-csi.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Synopsys DesignWare CSI-2 Host controller (csi2host)
>> +
>> +maintainers:
>> +  - Eugen Hristev <eugen.hristev@microchip.com>
>> +
>> +description:
>> +  CSI2HOST is used to receive image coming from an MIPI CSI-2 compatible
>> +  camera. It will convert the incoming CSI-2 stream into a dedicated
>> +  interface called the Synopsys IDI (Image Data Interface).
>> +  This interface is a 32-bit SoC internal only, and can be assimilated
>> +  with a CSI-2 interface.
>> +
>> +properties:
>> +  compatible:
>> +    const: snps,dw-csi
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  clocks:
>> +    maxItems: 2
>> +
>> +  clock-names:
>> +    description:
>> +      CSI2HOST can have two clocks connected. One clock is the
>> +      peripheral clock for the inside functionality of the hardware block.
>> +      This is named 'perclk'. The second clock can be the phy clock,
>> +      which is used to clock the phy via an internal link.
>> +      This clock is named 'phyclk', phy clock.
>> +    items:
>> +      - const: perclk
>> +      - const: phyclk
> 
> Drop "clk" from both
> 
> 
>> +
>> +  phys:
>> +    maxItems: 1
>> +    description: MIPI D-PHY
>> +
>> +  phy-names:
>> +    items:
>> +      - const: dphy
>> +
>> +  resets:
>> +    maxItems: 1
>> +
>> +  interrupts:
>> +    maxItems: 1
>> +
>> +  ports:
>> +    $ref: /schemas/graph.yaml#/properties/ports
>> +
>> +    properties:
>> +      port@0:
>> +        $ref: /schemas/graph.yaml#/$defs/port-base
>> +        unevaluatedProperties: false
>> +        description:
>> +          Input port node, single endpoint describing the input port.
>> +
>> +        properties:
>> +          endpoint:
>> +            $ref: video-interfaces.yaml#
>> +            unevaluatedProperties: false
>> +            description: Endpoint connected to input device
>> +
>> +            properties:
>> +              bus-type:
>> +                const: 4
>> +
>> +              data-lanes:
>> +                minItems: 1
>> +                maxItems: 4
>> +                items:
>> +                  maximum: 4
>> +
>> +              clock-lanes:
>> +                maxItems: 1
>> +
>> +              remote-endpoint: true
>> +
>> +      port@1:
>> +        $ref: /schemas/graph.yaml#/$defs/port-base
>> +        unevaluatedProperties: false
>> +        description:
>> +          Output port node, single endpoint describing the output port.
>> +
>> +        properties:
>> +          endpoint:
>> +            unevaluatedProperties: false
>> +            $ref: video-interfaces.yaml#
>> +            description: Endpoint connected to output device
>> +
>> +            properties:
>> +              bus-type:
>> +                const: 4
>> +
>> +              remote-endpoint: true
>> +
>> +    required:
>> +      - port@0
>> +      - port@1
>> +
>> +additionalProperties: false
>> +
>> +required:
>> +  - compatible
>> +  - ports
> 
> reg? phys? interrupts? All others?
> 
>> +
>> +examples:
>> +  - |
>> +    csi2: csi2@3000 {
> 
> Generic node names, so "csi"
> 
>> +        compatible = "snps,dw-csi";
>> +        reg = <0x03000 0x7FF>;
> 
> lowercase hex
> 
>> +        phys = <&mipi_dphy_rx>;
>> +        phy-names = "dphy";
>> +        resets = <&dw_rst 1>;
>> +        interrupts = <2>;
>> +
> 
> Best regards,
> Krzysztof
>