Re: [PATCH v2 1/4] dt-bindings: media: csi2dc: add bindings for microchip csi2dc

[<Eugen.Hristev@microchip.com>]

On 03.07.2020 10:44, Eugen Hristev wrote:
> Add bindings documentation for microchip CSI2 Demultiplexer controller.
> 
> CSI2DC is a demultiplexer from Synopsys IDI interface specification to
> parallel interface connection or direct memory access.
> 
> Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
> ---
> Changes in v2:
> - fixed warnings reported by dt_binding_check
> 
> 
>   .../bindings/media/microchip,csi2dc.yaml      | 185 ++++++++++++++++++
>   1 file changed, 185 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/media/microchip,csi2dc.yaml
> 
> diff --git a/Documentation/devicetree/bindings/media/microchip,csi2dc.yaml b/Documentation/devicetree/bindings/media/microchip,csi2dc.yaml
> new file mode 100644
> index 000000000000..b7c46f7ad2a4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/microchip,csi2dc.yaml
> @@ -0,0 +1,185 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/microchip,csi2dc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Microchip CSI2 Demux Controller (CSI2DC)
> +
> +maintainers:
> +  - Eugen Hristev <eugen.hristev@microchip.com>
> +
> +description:
> +  CSI2DC - Camera Serial Interface 2 Demux Controller
> +
> +  CSI2DC is a hardware block that receives incoming data from an IDI interface
> +  and filters packets based on their data type and virtual channel identifier,
> +  then converts the byte stream into a cross clock domain to a pixel stream
> +  to a parallel interface that can be read by a sensor controller.
> +
> +  CSI2DC provides two pipes, one video pipe and one data pipe. Video pipe
> +  is connected to a sensor controller and the data pipe is accessible
> +  as a DMA slave port to a DMA controller.
> +
> +  CSI2DC supports a single 'port' node as a source pad with Synopsys 32-bit
> +  IDI interface. The connected endpoint must be a IDI interface compatible
> +  device (like Synopsys CSI2HOST) , that can provide 32-bit IDI interface
> +  connection as sink pad.

Hello Sakari,

Could you have a look at my binding, especially this paragraph above. 
This controller that we have requires the Synopsys IDI interface as 
source pad. As this interface is not yet in kernel, do you think it's 
fine to mention it here ? Or how should I proceed ?

In my tests and the example at the end of this binding, I just use the 
port and endpoints without any specific properties.
The CSI2DC will actually wait an async completion (with a notifier) for 
that device to be registered, and then use it as a subdevice. In turn, 
CSI2DC registers a subdev for the next port in pipeline, towards the 
upper level ( the image sensor controller)

However, the connection with the subdevice in this case will likely 
default to parallel, and not be at all in any of the cases in the 
video-interfaces.txt :
- bus-type: data bus type. Possible values are: 

   1 - MIPI CSI-2 C-PHY 

   2 - MIPI CSI1 

   3 - CCP2 

   4 - MIPI CSI-2 D-PHY 

   5 - Parallel 

   6 - Bt.656

as this interface type is actually Synopsys IDI.
Do you think this is fine ?

Thanks,
Eugen

> +  It should contain one 'port' child node with one child 'endpoint' node.
> +  This node should always have the 'reg' property as 0, being the first child
> +  node.
> +  For media entity and endpoints please refer to the bindings defined in
> +  Documentation/devicetree/bindings/media/video-interfaces.txt.
> +  For Synopsys IDI interface please refer to
> +  Documentation/devicetree/bindings/media/snps,dw-csi-plat.txt
> +
> +  CSI2DC supports one 'port' node as sink pad with parallel interface. This is
> +  called video pipe.
> +  The reg property inside this 'port' node must have the 'reg' property as 1,
> +  being the second child node.
> +  This node must have one 'endpoint', and this 'endpoint' is related to the
> +  virtual channel identifier.
> +  The 'reg' property inside this 'endpoint' represents the CSI2 virtual channel
> +  identifier.
> +  This 'endpoint' can then be used as a source pad for another controller
> +  (next in pipeline).
> +  Please refer to the bindings defined in
> +  Documentation/devicetree/bindings/media/video-interfaces.txt.
> +
> +  CSI2DC must have two clocks to function correctly. One clock is the
> +  peripheral clock for the inside functionality of the hardware block.
> +  This is named 'pclk'. The second clock must be the cross domain clock,
> +  in which CSI2DC will perform clock crossing. This clock must be fed
> +  by the next controller in pipeline, which usually is a sensor controller.
> +  Normally this clock should be given by this sensor controller who
> +  is also a clock source. This clock is named 'scck', sensor controller clock.
> +
> +  CSI2DC also supports direct access to the data through AHB, via DMA channel,
> +  called data pipe.
> +  Because of this, the sink 'port' child node (second) is not mandatory.
> +  If the sink 'port' child node is missing, only data pipe is available.
> +
> +properties:
> +  compatible:
> +    const: microchip,sama7g5-csi2dc
> +
> +  reg:
> +    description:
> +      Physical base address and length of the registers set for the device.
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 2
> +
> +  clock-names:
> +    items:
> +      - const: pclk
> +      - const: scck
> +
> +  microchip,clk-gated:
> +    type: boolean
> +    description:
> +      If present, indicates that the clock is gated.
> +      Otherwise, the clock is free-running.
> +
> +  microchip,inter-line-delay:
> +    allOf:
> +    - $ref: /schemas/types.yaml#/definitions/uint32
> +    - minimum: 1
> +    - maximum: 16
> +    default: 16
> +    description:
> +      Indicates how many clock cycles should be introduced between each line.
> +
> +  port@0:
> +    type: object
> +    description:
> +      Input port node, single endpoint describing the input pad.
> +
> +    properties:
> +      reg:
> +        const: 0
> +
> +      endpoint:
> +        type: object
> +
> +        properties:
> +          remote-endpoint: true
> +
> +        required:
> +          - remote-endpoint
> +
> +        additionalProperties: false
> +
> +    additionalProperties: false
> +
> +  port@1:
> +    type: object
> +    description:
> +      Output port node, single endpoint, describing the output pad.
> +
> +    properties:
> +      '#address-cells':
> +        const: 1
> +
> +      '#size-cells':
> +        const: 0
> +
> +      reg:
> +        const: 1
> +
> +    patternProperties:
> +      "^endpoint@[0-9a-f]$":
> +        type: object
> +
> +        properties:
> +          reg:
> +            enum: [0, 1, 2, 3]
> +            description: virtual channel for the endpoint
> +
> +          remote-endpoint: true
> +
> +        required:
> +          - remote-endpoint
> +          - reg
> +
> +        additionalProperties: false
> +
> +    additionalProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - port@0
> +
> +examples:
> +  - |
> +    csi2dc@e1404000 {
> +        compatible = "microchip,sama7g5-csi2dc";
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +        reg = <0xe1404000 0x500>;
> +        clocks = <&pclk>, <&scck>;
> +        clock-names = "pclk", "scck";
> +
> +        port@0 {
> +               reg = <0>; /* must be 0, first child port */
> +               csi2dc_in: endpoint { /* input from IDI interface */
> +                     remote-endpoint = <&csi2host_out>;
> +               };
> +        };
> +
> +        port@1 {
> +                #address-cells = <1>;
> +                #size-cells = <0>;
> +                reg = <1>; /* must be 1, second child port */
> +                csi2dc_out: endpoint@2 {
> +                        reg = <2>;  /* virtual channel identifier */
> +                        remote-endpoint = <&xisc_in>; /* output to sensor controller */
> +                };
> +        };
> +    };
> +
> +...
>