Re: [PATCH 07/14] dt-bindings: media: i2c: Add A31 MIPI CSI-2 bindings documentation

2020-11-04 Thread Maxime Ripard
On Wed, Nov 04, 2020 at 11:48:27AM +0100, Paul Kocialkowski wrote:
> Hi,
> 
> On Tue 27 Oct 20, 19:44, Maxime Ripard wrote:
> > On Tue, Oct 27, 2020 at 10:52:21AM +0100, Paul Kocialkowski wrote:
> > > Hi,
> > > 
> > > On Mon 26 Oct 20, 17:14, Maxime Ripard wrote:
> > > > i2c? :)
> > > 
> > > Oops, good catch!
> > >  
> > > > On Fri, Oct 23, 2020 at 07:45:39PM +0200, Paul Kocialkowski wrote:
> > > > > This introduces YAML bindings documentation for the A31 MIPI CSI-2
> > > > > controller.
> > > > > 
> > > > > Signed-off-by: Paul Kocialkowski 
> > > > > ---
> > > > >  .../media/allwinner,sun6i-a31-mipi-csi2.yaml  | 168 
> > > > > ++
> > > > >  1 file changed, 168 insertions(+)
> > > > >  create mode 100644 
> > > > > Documentation/devicetree/bindings/media/allwinner,sun6i-a31-mipi-csi2.yaml
> > > > > 
> > > > > diff --git 
> > > > > a/Documentation/devicetree/bindings/media/allwinner,sun6i-a31-mipi-csi2.yaml
> > > > >  
> > > > > b/Documentation/devicetree/bindings/media/allwinner,sun6i-a31-mipi-csi2.yaml
> > > > > new file mode 100644
> > > > > index ..9adc0bc27033
> > > > > --- /dev/null
> > > > > +++ 
> > > > > b/Documentation/devicetree/bindings/media/allwinner,sun6i-a31-mipi-csi2.yaml
> > > > > @@ -0,0 +1,168 @@
> > > > > +# SPDX-License-Identifier: GPL-2.0
> > > > > +%YAML 1.2
> > > > > +---
> > > > > +$id: 
> > > > > http://devicetree.org/schemas/media/allwinner,sun6i-a31-mipi-csi2.yaml#
> > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > > +
> > > > > +title: Allwinner A31 MIPI CSI-2 Device Tree Bindings
> > > > > +
> > > > > +maintainers:
> > > > > +  - Paul Kocialkowski 
> > > > > +
> > > > > +properties:
> > > > > +  compatible:
> > > > > +oneOf:
> > > > > +  - const: allwinner,sun6i-a31-mipi-csi2
> > > > > +  - items:
> > > > > +  - const: allwinner,sun8i-v3s-mipi-csi2
> > > > > +  - const: allwinner,sun6i-a31-mipi-csi2
> > > > > +
> > > > > +  reg:
> > > > > +maxItems: 1
> > > > > +
> > > > > +  interrupts:
> > > > > +maxItems: 1
> > > > > +
> > > > > +  clocks:
> > > > > +items:
> > > > > +  - description: Bus Clock
> > > > > +  - description: Module Clock
> > > > > +
> > > > > +  clock-names:
> > > > > +items:
> > > > > +  - const: bus
> > > > > +  - const: mod
> > > > > +
> > > > > +  phys:
> > > > > +items:
> > > > > +  - description: MIPI D-PHY
> > > > > +
> > > > > +  phy-names:
> > > > > +items:
> > > > > +  - const: dphy
> > > > > +
> > > > > +  resets:
> > > > > +maxItems: 1
> > > > > +
> > > > > +  # See ./video-interfaces.txt for details
> > > > > +  ports:
> > > > > +type: object
> > > > > +
> > > > > +properties:
> > > > > +  port@0:
> > > > > +type: object
> > > > > +description: Input port, connect to a MIPI CSI-2 sensor
> > > > > +
> > > > > +properties:
> > > > > +  reg:
> > > > > +const: 0
> > > > > +
> > > > > +  endpoint:
> > > > > +type: object
> > > > > +
> > > > > +properties:
> > > > > +  remote-endpoint: true
> > > > > +
> > > > > +  bus-type:
> > > > > +const: 4
> > > > > +
> > > > > +  clock-lanes:
> > > > > +maxItems: 1
> > > > > +
> > > > > +  data-lanes:
> > > > > +minItems: 1
> > > > > +maxItems: 4
> > > > > +
> > > > > +required:
> > > > > +  - bus-type
> > > > > +  - data-lanes
> > > > > +  - remote-endpoint
> > > > > +
> > > > > +additionalProperties: false
> > > > > +
> > > > > +required:
> > > > > +  - endpoint
> > > > > +
> > > > > +additionalProperties: false
> > > > > +
> > > > > +  port@1:
> > > > > +type: object
> > > > > +description: Output port, connect to a CSI controller
> > > > > +
> > > > > +properties:
> > > > > +  reg:
> > > > > +const: 1
> > > > > +
> > > > > +  endpoint:
> > > > > +type: object
> > > > > +
> > > > > +properties:
> > > > > +  remote-endpoint: true
> > > > > +
> > > > > +  bus-type:
> > > > > +const: 4
> > > > 
> > > > That one seems a bit weird. If the input and output ports are using the
> > > > same format, what is that "bridge" supposed to be doing?
> > > 
> > > Fair enough. What this represents is the internal link (likely a FIFO) 
> > > between
> > > the two controllers. It is definitely not a MIPI CSI-2 bus but there's no
> > > mbus type for an internal link (probably because it's not a bus after 
> > > all).
> > > 
> > > Note that on the CSI controller side, we need the bus-type to be set to 4 
> > > for it
> > > to properly select the MIPI CSI-2 input. So it just felt more logical to 
> > > have
> > > the same on the other side of the endpoint. On the other hand, we can just
> > > remove it on 

Re: [PATCH 07/14] dt-bindings: media: i2c: Add A31 MIPI CSI-2 bindings documentation

2020-11-04 Thread Paul Kocialkowski
Hi,

On Tue 27 Oct 20, 19:44, Maxime Ripard wrote:
> On Tue, Oct 27, 2020 at 10:52:21AM +0100, Paul Kocialkowski wrote:
> > Hi,
> > 
> > On Mon 26 Oct 20, 17:14, Maxime Ripard wrote:
> > > i2c? :)
> > 
> > Oops, good catch!
> >  
> > > On Fri, Oct 23, 2020 at 07:45:39PM +0200, Paul Kocialkowski wrote:
> > > > This introduces YAML bindings documentation for the A31 MIPI CSI-2
> > > > controller.
> > > > 
> > > > Signed-off-by: Paul Kocialkowski 
> > > > ---
> > > >  .../media/allwinner,sun6i-a31-mipi-csi2.yaml  | 168 ++
> > > >  1 file changed, 168 insertions(+)
> > > >  create mode 100644 
> > > > Documentation/devicetree/bindings/media/allwinner,sun6i-a31-mipi-csi2.yaml
> > > > 
> > > > diff --git 
> > > > a/Documentation/devicetree/bindings/media/allwinner,sun6i-a31-mipi-csi2.yaml
> > > >  
> > > > b/Documentation/devicetree/bindings/media/allwinner,sun6i-a31-mipi-csi2.yaml
> > > > new file mode 100644
> > > > index ..9adc0bc27033
> > > > --- /dev/null
> > > > +++ 
> > > > b/Documentation/devicetree/bindings/media/allwinner,sun6i-a31-mipi-csi2.yaml
> > > > @@ -0,0 +1,168 @@
> > > > +# SPDX-License-Identifier: GPL-2.0
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: 
> > > > http://devicetree.org/schemas/media/allwinner,sun6i-a31-mipi-csi2.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Allwinner A31 MIPI CSI-2 Device Tree Bindings
> > > > +
> > > > +maintainers:
> > > > +  - Paul Kocialkowski 
> > > > +
> > > > +properties:
> > > > +  compatible:
> > > > +oneOf:
> > > > +  - const: allwinner,sun6i-a31-mipi-csi2
> > > > +  - items:
> > > > +  - const: allwinner,sun8i-v3s-mipi-csi2
> > > > +  - const: allwinner,sun6i-a31-mipi-csi2
> > > > +
> > > > +  reg:
> > > > +maxItems: 1
> > > > +
> > > > +  interrupts:
> > > > +maxItems: 1
> > > > +
> > > > +  clocks:
> > > > +items:
> > > > +  - description: Bus Clock
> > > > +  - description: Module Clock
> > > > +
> > > > +  clock-names:
> > > > +items:
> > > > +  - const: bus
> > > > +  - const: mod
> > > > +
> > > > +  phys:
> > > > +items:
> > > > +  - description: MIPI D-PHY
> > > > +
> > > > +  phy-names:
> > > > +items:
> > > > +  - const: dphy
> > > > +
> > > > +  resets:
> > > > +maxItems: 1
> > > > +
> > > > +  # See ./video-interfaces.txt for details
> > > > +  ports:
> > > > +type: object
> > > > +
> > > > +properties:
> > > > +  port@0:
> > > > +type: object
> > > > +description: Input port, connect to a MIPI CSI-2 sensor
> > > > +
> > > > +properties:
> > > > +  reg:
> > > > +const: 0
> > > > +
> > > > +  endpoint:
> > > > +type: object
> > > > +
> > > > +properties:
> > > > +  remote-endpoint: true
> > > > +
> > > > +  bus-type:
> > > > +const: 4
> > > > +
> > > > +  clock-lanes:
> > > > +maxItems: 1
> > > > +
> > > > +  data-lanes:
> > > > +minItems: 1
> > > > +maxItems: 4
> > > > +
> > > > +required:
> > > > +  - bus-type
> > > > +  - data-lanes
> > > > +  - remote-endpoint
> > > > +
> > > > +additionalProperties: false
> > > > +
> > > > +required:
> > > > +  - endpoint
> > > > +
> > > > +additionalProperties: false
> > > > +
> > > > +  port@1:
> > > > +type: object
> > > > +description: Output port, connect to a CSI controller
> > > > +
> > > > +properties:
> > > > +  reg:
> > > > +const: 1
> > > > +
> > > > +  endpoint:
> > > > +type: object
> > > > +
> > > > +properties:
> > > > +  remote-endpoint: true
> > > > +
> > > > +  bus-type:
> > > > +const: 4
> > > 
> > > That one seems a bit weird. If the input and output ports are using the
> > > same format, what is that "bridge" supposed to be doing?
> > 
> > Fair enough. What this represents is the internal link (likely a FIFO) 
> > between
> > the two controllers. It is definitely not a MIPI CSI-2 bus but there's no
> > mbus type for an internal link (probably because it's not a bus after all).
> > 
> > Note that on the CSI controller side, we need the bus-type to be set to 4 
> > for it
> > to properly select the MIPI CSI-2 input. So it just felt more logical to 
> > have
> > the same on the other side of the endpoint. On the other hand, we can just
> > remove it on the MIPI CSI-2 controller side since it won't check it and 
> > have it
> > fallback to the unknown mbus type.
> > 
> > But that would make the types inconsistent on the two sides of the link.
> > I don't think V4L2 will complain about it at the moment, but it would also 
> > make
> > sense that it does eventually.
> > 
> > What do you think?
> 
> There's still the same issue though, it 

Re: [PATCH 07/14] dt-bindings: media: i2c: Add A31 MIPI CSI-2 bindings documentation

2020-10-30 Thread Sakari Ailus
Hi Paul,

On Fri, Oct 23, 2020 at 07:45:39PM +0200, Paul Kocialkowski wrote:
> This introduces YAML bindings documentation for the A31 MIPI CSI-2
> controller.
> 
> Signed-off-by: Paul Kocialkowski 
> ---
>  .../media/allwinner,sun6i-a31-mipi-csi2.yaml  | 168 ++
>  1 file changed, 168 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/media/allwinner,sun6i-a31-mipi-csi2.yaml
> 
> diff --git 
> a/Documentation/devicetree/bindings/media/allwinner,sun6i-a31-mipi-csi2.yaml 
> b/Documentation/devicetree/bindings/media/allwinner,sun6i-a31-mipi-csi2.yaml
> new file mode 100644
> index ..9adc0bc27033
> --- /dev/null
> +++ 
> b/Documentation/devicetree/bindings/media/allwinner,sun6i-a31-mipi-csi2.yaml
> @@ -0,0 +1,168 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/allwinner,sun6i-a31-mipi-csi2.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Allwinner A31 MIPI CSI-2 Device Tree Bindings
> +
> +maintainers:
> +  - Paul Kocialkowski 
> +
> +properties:
> +  compatible:
> +oneOf:
> +  - const: allwinner,sun6i-a31-mipi-csi2
> +  - items:
> +  - const: allwinner,sun8i-v3s-mipi-csi2
> +  - const: allwinner,sun6i-a31-mipi-csi2
> +
> +  reg:
> +maxItems: 1
> +
> +  interrupts:
> +maxItems: 1
> +
> +  clocks:
> +items:
> +  - description: Bus Clock
> +  - description: Module Clock
> +
> +  clock-names:
> +items:
> +  - const: bus
> +  - const: mod
> +
> +  phys:
> +items:
> +  - description: MIPI D-PHY
> +
> +  phy-names:
> +items:
> +  - const: dphy
> +
> +  resets:
> +maxItems: 1
> +
> +  # See ./video-interfaces.txt for details
> +  ports:
> +type: object
> +
> +properties:
> +  port@0:
> +type: object
> +description: Input port, connect to a MIPI CSI-2 sensor
> +
> +properties:
> +  reg:
> +const: 0
> +
> +  endpoint:
> +type: object
> +
> +properties:
> +  remote-endpoint: true
> +
> +  bus-type:
> +const: 4
> +
> +  clock-lanes:
> +maxItems: 1

You can drop bus-type and clock-lanes (assuming no lane remapping is
supported by the hardware).

> +
> +  data-lanes:
> +minItems: 1
> +maxItems: 4
> +
> +required:
> +  - bus-type
> +  - data-lanes
> +  - remote-endpoint
> +
> +additionalProperties: false
> +
> +required:
> +  - endpoint
> +
> +additionalProperties: false
> +
> +  port@1:
> +type: object
> +description: Output port, connect to a CSI controller
> +
> +properties:
> +  reg:
> +const: 1
> +
> +  endpoint:
> +type: object
> +
> +properties:
> +  remote-endpoint: true
> +
> +  bus-type:
> +const: 4

Same here.

> +
> +additionalProperties: false
> +
> +required:
> +  - endpoint
> +
> +additionalProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - clock-names
> +  - resets
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +#include 
> +#include 
> +#include 
> +
> +mipi_csi2: mipi-csi2@1cb1000 {
> +compatible = "allwinner,sun8i-v3s-mipi-csi2",
> + "allwinner,sun6i-a31-mipi-csi2";
> +reg = <0x01cb1000 0x1000>;
> +interrupts = ;
> +clocks = < CLK_BUS_CSI>,
> + < CLK_CSI1_SCLK>;
> +clock-names = "bus", "mod";
> +resets = < RST_BUS_CSI>;
> +
> +phys = <>;
> +phy-names = "dphy";
> +
> +ports {
> +#address-cells = <1>;
> +#size-cells = <0>;
> +
> +mipi_csi2_in: port@0 {
> +reg = <0>;
> +
> +mipi_csi2_in_ov5648: endpoint {
> +bus-type = <4>; /* MIPI CSI-2 D-PHY */
> +clock-lanes = <0>;
> +data-lanes = <1 2 3 4>;
> +
> +remote-endpoint = <_out_mipi_csi2>;
> +};
> +};
> +
> +mipi_csi2_out: port@1 {
> +reg = <1>;
> +
> +mipi_csi2_out_csi0: endpoint {
> +bus-type = <4>; /* MIPI CSI-2 D-PHY */
> +remote-endpoint = <_in_mipi_csi2>;
> +};
> +};
> +};
> +};
> +
> +...

-- 
Kind regards,

Sakari Ailus
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 07/14] dt-bindings: media: i2c: Add A31 MIPI CSI-2 bindings documentation

2020-10-30 Thread Rob Herring
On Fri, Oct 23, 2020 at 07:45:39PM +0200, Paul Kocialkowski wrote:
> This introduces YAML bindings documentation for the A31 MIPI CSI-2
> controller.
> 
> Signed-off-by: Paul Kocialkowski 
> ---
>  .../media/allwinner,sun6i-a31-mipi-csi2.yaml  | 168 ++
>  1 file changed, 168 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/media/allwinner,sun6i-a31-mipi-csi2.yaml
> 
> diff --git 
> a/Documentation/devicetree/bindings/media/allwinner,sun6i-a31-mipi-csi2.yaml 
> b/Documentation/devicetree/bindings/media/allwinner,sun6i-a31-mipi-csi2.yaml
> new file mode 100644
> index ..9adc0bc27033
> --- /dev/null
> +++ 
> b/Documentation/devicetree/bindings/media/allwinner,sun6i-a31-mipi-csi2.yaml
> @@ -0,0 +1,168 @@
> +# SPDX-License-Identifier: GPL-2.0

Dual license new bindings.

> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/allwinner,sun6i-a31-mipi-csi2.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Allwinner A31 MIPI CSI-2 Device Tree Bindings
> +
> +maintainers:
> +  - Paul Kocialkowski 
> +
> +properties:
> +  compatible:
> +oneOf:
> +  - const: allwinner,sun6i-a31-mipi-csi2
> +  - items:
> +  - const: allwinner,sun8i-v3s-mipi-csi2
> +  - const: allwinner,sun6i-a31-mipi-csi2
> +
> +  reg:
> +maxItems: 1
> +
> +  interrupts:
> +maxItems: 1
> +
> +  clocks:
> +items:
> +  - description: Bus Clock
> +  - description: Module Clock
> +
> +  clock-names:
> +items:
> +  - const: bus
> +  - const: mod
> +
> +  phys:
> +items:
> +  - description: MIPI D-PHY
> +
> +  phy-names:
> +items:
> +  - const: dphy
> +
> +  resets:
> +maxItems: 1
> +
> +  # See ./video-interfaces.txt for details
> +  ports:
> +type: object
> +
> +properties:
> +  port@0:
> +type: object
> +description: Input port, connect to a MIPI CSI-2 sensor
> +
> +properties:
> +  reg:
> +const: 0
> +
> +  endpoint:
> +type: object
> +
> +properties:
> +  remote-endpoint: true
> +
> +  bus-type:
> +const: 4
> +
> +  clock-lanes:
> +maxItems: 1
> +
> +  data-lanes:
> +minItems: 1
> +maxItems: 4
> +
> +required:
> +  - bus-type
> +  - data-lanes
> +  - remote-endpoint
> +
> +additionalProperties: false
> +
> +required:
> +  - endpoint
> +
> +additionalProperties: false
> +
> +  port@1:
> +type: object
> +description: Output port, connect to a CSI controller
> +
> +properties:
> +  reg:
> +const: 1
> +
> +  endpoint:
> +type: object
> +
> +properties:
> +  remote-endpoint: true
> +
> +  bus-type:
> +const: 4
> +
> +additionalProperties: false
> +
> +required:
> +  - endpoint
> +
> +additionalProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - clock-names
> +  - resets
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +#include 
> +#include 
> +#include 
> +
> +mipi_csi2: mipi-csi2@1cb1000 {

I agree with using 'csi' here as that is at least aligned with 'dsi' 
meaning the host side of the protocol. We've not been consistent here...

> +compatible = "allwinner,sun8i-v3s-mipi-csi2",
> + "allwinner,sun6i-a31-mipi-csi2";
> +reg = <0x01cb1000 0x1000>;
> +interrupts = ;
> +clocks = < CLK_BUS_CSI>,
> + < CLK_CSI1_SCLK>;
> +clock-names = "bus", "mod";
> +resets = < RST_BUS_CSI>;
> +
> +phys = <>;
> +phy-names = "dphy";
> +
> +ports {
> +#address-cells = <1>;
> +#size-cells = <0>;
> +
> +mipi_csi2_in: port@0 {
> +reg = <0>;
> +
> +mipi_csi2_in_ov5648: endpoint {
> +bus-type = <4>; /* MIPI CSI-2 D-PHY */
> +clock-lanes = <0>;
> +data-lanes = <1 2 3 4>;
> +
> +remote-endpoint = <_out_mipi_csi2>;
> +};
> +};
> +
> +mipi_csi2_out: port@1 {
> +reg = <1>;
> +
> +mipi_csi2_out_csi0: endpoint {
> +bus-type = <4>; /* MIPI CSI-2 D-PHY */
> +remote-endpoint = <_in_mipi_csi2>;
> +};
> +};
> +};
> +};
> +
> +...
> -- 
> 2.28.0
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 07/14] dt-bindings: media: i2c: Add A31 MIPI CSI-2 bindings documentation

2020-10-27 Thread Maxime Ripard
On Tue, Oct 27, 2020 at 10:52:21AM +0100, Paul Kocialkowski wrote:
> Hi,
> 
> On Mon 26 Oct 20, 17:14, Maxime Ripard wrote:
> > i2c? :)
> 
> Oops, good catch!
>  
> > On Fri, Oct 23, 2020 at 07:45:39PM +0200, Paul Kocialkowski wrote:
> > > This introduces YAML bindings documentation for the A31 MIPI CSI-2
> > > controller.
> > > 
> > > Signed-off-by: Paul Kocialkowski 
> > > ---
> > >  .../media/allwinner,sun6i-a31-mipi-csi2.yaml  | 168 ++
> > >  1 file changed, 168 insertions(+)
> > >  create mode 100644 
> > > Documentation/devicetree/bindings/media/allwinner,sun6i-a31-mipi-csi2.yaml
> > > 
> > > diff --git 
> > > a/Documentation/devicetree/bindings/media/allwinner,sun6i-a31-mipi-csi2.yaml
> > >  
> > > b/Documentation/devicetree/bindings/media/allwinner,sun6i-a31-mipi-csi2.yaml
> > > new file mode 100644
> > > index ..9adc0bc27033
> > > --- /dev/null
> > > +++ 
> > > b/Documentation/devicetree/bindings/media/allwinner,sun6i-a31-mipi-csi2.yaml
> > > @@ -0,0 +1,168 @@
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +%YAML 1.2
> > > +---
> > > +$id: 
> > > http://devicetree.org/schemas/media/allwinner,sun6i-a31-mipi-csi2.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Allwinner A31 MIPI CSI-2 Device Tree Bindings
> > > +
> > > +maintainers:
> > > +  - Paul Kocialkowski 
> > > +
> > > +properties:
> > > +  compatible:
> > > +oneOf:
> > > +  - const: allwinner,sun6i-a31-mipi-csi2
> > > +  - items:
> > > +  - const: allwinner,sun8i-v3s-mipi-csi2
> > > +  - const: allwinner,sun6i-a31-mipi-csi2
> > > +
> > > +  reg:
> > > +maxItems: 1
> > > +
> > > +  interrupts:
> > > +maxItems: 1
> > > +
> > > +  clocks:
> > > +items:
> > > +  - description: Bus Clock
> > > +  - description: Module Clock
> > > +
> > > +  clock-names:
> > > +items:
> > > +  - const: bus
> > > +  - const: mod
> > > +
> > > +  phys:
> > > +items:
> > > +  - description: MIPI D-PHY
> > > +
> > > +  phy-names:
> > > +items:
> > > +  - const: dphy
> > > +
> > > +  resets:
> > > +maxItems: 1
> > > +
> > > +  # See ./video-interfaces.txt for details
> > > +  ports:
> > > +type: object
> > > +
> > > +properties:
> > > +  port@0:
> > > +type: object
> > > +description: Input port, connect to a MIPI CSI-2 sensor
> > > +
> > > +properties:
> > > +  reg:
> > > +const: 0
> > > +
> > > +  endpoint:
> > > +type: object
> > > +
> > > +properties:
> > > +  remote-endpoint: true
> > > +
> > > +  bus-type:
> > > +const: 4
> > > +
> > > +  clock-lanes:
> > > +maxItems: 1
> > > +
> > > +  data-lanes:
> > > +minItems: 1
> > > +maxItems: 4
> > > +
> > > +required:
> > > +  - bus-type
> > > +  - data-lanes
> > > +  - remote-endpoint
> > > +
> > > +additionalProperties: false
> > > +
> > > +required:
> > > +  - endpoint
> > > +
> > > +additionalProperties: false
> > > +
> > > +  port@1:
> > > +type: object
> > > +description: Output port, connect to a CSI controller
> > > +
> > > +properties:
> > > +  reg:
> > > +const: 1
> > > +
> > > +  endpoint:
> > > +type: object
> > > +
> > > +properties:
> > > +  remote-endpoint: true
> > > +
> > > +  bus-type:
> > > +const: 4
> > 
> > That one seems a bit weird. If the input and output ports are using the
> > same format, what is that "bridge" supposed to be doing?
> 
> Fair enough. What this represents is the internal link (likely a FIFO) between
> the two controllers. It is definitely not a MIPI CSI-2 bus but there's no
> mbus type for an internal link (probably because it's not a bus after all).
> 
> Note that on the CSI controller side, we need the bus-type to be set to 4 for 
> it
> to properly select the MIPI CSI-2 input. So it just felt more logical to have
> the same on the other side of the endpoint. On the other hand, we can just
> remove it on the MIPI CSI-2 controller side since it won't check it and have 
> it
> fallback to the unknown mbus type.
> 
> But that would make the types inconsistent on the two sides of the link.
> I don't think V4L2 will complain about it at the moment, but it would also 
> make
> sense that it does eventually.
> 
> What do you think?

There's still the same issue though, it doesn't make any sense that a
bridge doesn't change the bus type. If it really did, we wouldn't need
that in the first place.

What you want to check in your driver is whether the subdev you're
connected to has a sink pad that uses MIPI-CSI

Maxime

> > > +additionalProperties: false
> > > +
> > > +required:
> > > +  - endpoint
> > > +
> > > +  

Re: [PATCH 07/14] dt-bindings: media: i2c: Add A31 MIPI CSI-2 bindings documentation

2020-10-27 Thread Paul Kocialkowski
Hi,

On Mon 26 Oct 20, 17:14, Maxime Ripard wrote:
> i2c? :)

Oops, good catch!
 
> On Fri, Oct 23, 2020 at 07:45:39PM +0200, Paul Kocialkowski wrote:
> > This introduces YAML bindings documentation for the A31 MIPI CSI-2
> > controller.
> > 
> > Signed-off-by: Paul Kocialkowski 
> > ---
> >  .../media/allwinner,sun6i-a31-mipi-csi2.yaml  | 168 ++
> >  1 file changed, 168 insertions(+)
> >  create mode 100644 
> > Documentation/devicetree/bindings/media/allwinner,sun6i-a31-mipi-csi2.yaml
> > 
> > diff --git 
> > a/Documentation/devicetree/bindings/media/allwinner,sun6i-a31-mipi-csi2.yaml
> >  
> > b/Documentation/devicetree/bindings/media/allwinner,sun6i-a31-mipi-csi2.yaml
> > new file mode 100644
> > index ..9adc0bc27033
> > --- /dev/null
> > +++ 
> > b/Documentation/devicetree/bindings/media/allwinner,sun6i-a31-mipi-csi2.yaml
> > @@ -0,0 +1,168 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +%YAML 1.2
> > +---
> > +$id: 
> > http://devicetree.org/schemas/media/allwinner,sun6i-a31-mipi-csi2.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Allwinner A31 MIPI CSI-2 Device Tree Bindings
> > +
> > +maintainers:
> > +  - Paul Kocialkowski 
> > +
> > +properties:
> > +  compatible:
> > +oneOf:
> > +  - const: allwinner,sun6i-a31-mipi-csi2
> > +  - items:
> > +  - const: allwinner,sun8i-v3s-mipi-csi2
> > +  - const: allwinner,sun6i-a31-mipi-csi2
> > +
> > +  reg:
> > +maxItems: 1
> > +
> > +  interrupts:
> > +maxItems: 1
> > +
> > +  clocks:
> > +items:
> > +  - description: Bus Clock
> > +  - description: Module Clock
> > +
> > +  clock-names:
> > +items:
> > +  - const: bus
> > +  - const: mod
> > +
> > +  phys:
> > +items:
> > +  - description: MIPI D-PHY
> > +
> > +  phy-names:
> > +items:
> > +  - const: dphy
> > +
> > +  resets:
> > +maxItems: 1
> > +
> > +  # See ./video-interfaces.txt for details
> > +  ports:
> > +type: object
> > +
> > +properties:
> > +  port@0:
> > +type: object
> > +description: Input port, connect to a MIPI CSI-2 sensor
> > +
> > +properties:
> > +  reg:
> > +const: 0
> > +
> > +  endpoint:
> > +type: object
> > +
> > +properties:
> > +  remote-endpoint: true
> > +
> > +  bus-type:
> > +const: 4
> > +
> > +  clock-lanes:
> > +maxItems: 1
> > +
> > +  data-lanes:
> > +minItems: 1
> > +maxItems: 4
> > +
> > +required:
> > +  - bus-type
> > +  - data-lanes
> > +  - remote-endpoint
> > +
> > +additionalProperties: false
> > +
> > +required:
> > +  - endpoint
> > +
> > +additionalProperties: false
> > +
> > +  port@1:
> > +type: object
> > +description: Output port, connect to a CSI controller
> > +
> > +properties:
> > +  reg:
> > +const: 1
> > +
> > +  endpoint:
> > +type: object
> > +
> > +properties:
> > +  remote-endpoint: true
> > +
> > +  bus-type:
> > +const: 4
> 
> That one seems a bit weird. If the input and output ports are using the
> same format, what is that "bridge" supposed to be doing?

Fair enough. What this represents is the internal link (likely a FIFO) between
the two controllers. It is definitely not a MIPI CSI-2 bus but there's no
mbus type for an internal link (probably because it's not a bus after all).

Note that on the CSI controller side, we need the bus-type to be set to 4 for it
to properly select the MIPI CSI-2 input. So it just felt more logical to have
the same on the other side of the endpoint. On the other hand, we can just
remove it on the MIPI CSI-2 controller side since it won't check it and have it
fallback to the unknown mbus type.

But that would make the types inconsistent on the two sides of the link.
I don't think V4L2 will complain about it at the moment, but it would also make
sense that it does eventually.

What do you think?

> > +additionalProperties: false
> > +
> > +required:
> > +  - endpoint
> > +
> > +additionalProperties: false
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +  - clocks
> > +  - clock-names
> > +  - resets
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +#include 
> > +#include 
> > +#include 
> > +
> > +mipi_csi2: mipi-csi2@1cb1000 {
> 
> The unit name should be pretty standard, with the list here:
> 
> https://github.com/devicetree-org/devicetree-specification/blob/master/source/chapter2-devicetree-basics.rst#generic-names-recommendation
> 
> there's nothing really standing out for us in that list, but given that
> there's dsi, we should stick with csi

Then what really 

Re: [PATCH 07/14] dt-bindings: media: i2c: Add A31 MIPI CSI-2 bindings documentation

2020-10-26 Thread Maxime Ripard
i2c? :)

On Fri, Oct 23, 2020 at 07:45:39PM +0200, Paul Kocialkowski wrote:
> This introduces YAML bindings documentation for the A31 MIPI CSI-2
> controller.
> 
> Signed-off-by: Paul Kocialkowski 
> ---
>  .../media/allwinner,sun6i-a31-mipi-csi2.yaml  | 168 ++
>  1 file changed, 168 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/media/allwinner,sun6i-a31-mipi-csi2.yaml
> 
> diff --git 
> a/Documentation/devicetree/bindings/media/allwinner,sun6i-a31-mipi-csi2.yaml 
> b/Documentation/devicetree/bindings/media/allwinner,sun6i-a31-mipi-csi2.yaml
> new file mode 100644
> index ..9adc0bc27033
> --- /dev/null
> +++ 
> b/Documentation/devicetree/bindings/media/allwinner,sun6i-a31-mipi-csi2.yaml
> @@ -0,0 +1,168 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/allwinner,sun6i-a31-mipi-csi2.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Allwinner A31 MIPI CSI-2 Device Tree Bindings
> +
> +maintainers:
> +  - Paul Kocialkowski 
> +
> +properties:
> +  compatible:
> +oneOf:
> +  - const: allwinner,sun6i-a31-mipi-csi2
> +  - items:
> +  - const: allwinner,sun8i-v3s-mipi-csi2
> +  - const: allwinner,sun6i-a31-mipi-csi2
> +
> +  reg:
> +maxItems: 1
> +
> +  interrupts:
> +maxItems: 1
> +
> +  clocks:
> +items:
> +  - description: Bus Clock
> +  - description: Module Clock
> +
> +  clock-names:
> +items:
> +  - const: bus
> +  - const: mod
> +
> +  phys:
> +items:
> +  - description: MIPI D-PHY
> +
> +  phy-names:
> +items:
> +  - const: dphy
> +
> +  resets:
> +maxItems: 1
> +
> +  # See ./video-interfaces.txt for details
> +  ports:
> +type: object
> +
> +properties:
> +  port@0:
> +type: object
> +description: Input port, connect to a MIPI CSI-2 sensor
> +
> +properties:
> +  reg:
> +const: 0
> +
> +  endpoint:
> +type: object
> +
> +properties:
> +  remote-endpoint: true
> +
> +  bus-type:
> +const: 4
> +
> +  clock-lanes:
> +maxItems: 1
> +
> +  data-lanes:
> +minItems: 1
> +maxItems: 4
> +
> +required:
> +  - bus-type
> +  - data-lanes
> +  - remote-endpoint
> +
> +additionalProperties: false
> +
> +required:
> +  - endpoint
> +
> +additionalProperties: false
> +
> +  port@1:
> +type: object
> +description: Output port, connect to a CSI controller
> +
> +properties:
> +  reg:
> +const: 1
> +
> +  endpoint:
> +type: object
> +
> +properties:
> +  remote-endpoint: true
> +
> +  bus-type:
> +const: 4

That one seems a bit weird. If the input and output ports are using the
same format, what is that "bridge" supposed to be doing?

> +additionalProperties: false
> +
> +required:
> +  - endpoint
> +
> +additionalProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - clock-names
> +  - resets
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +#include 
> +#include 
> +#include 
> +
> +mipi_csi2: mipi-csi2@1cb1000 {

The unit name should be pretty standard, with the list here:

https://github.com/devicetree-org/devicetree-specification/blob/master/source/chapter2-devicetree-basics.rst#generic-names-recommendation

there's nothing really standing out for us in that list, but given that
there's dsi, we should stick with csi

Maxime


signature.asc
Description: PGP signature
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 07/14] dt-bindings: media: i2c: Add A31 MIPI CSI-2 bindings documentation

2020-10-23 Thread Paul Kocialkowski
This introduces YAML bindings documentation for the A31 MIPI CSI-2
controller.

Signed-off-by: Paul Kocialkowski 
---
 .../media/allwinner,sun6i-a31-mipi-csi2.yaml  | 168 ++
 1 file changed, 168 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/media/allwinner,sun6i-a31-mipi-csi2.yaml

diff --git 
a/Documentation/devicetree/bindings/media/allwinner,sun6i-a31-mipi-csi2.yaml 
b/Documentation/devicetree/bindings/media/allwinner,sun6i-a31-mipi-csi2.yaml
new file mode 100644
index ..9adc0bc27033
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/allwinner,sun6i-a31-mipi-csi2.yaml
@@ -0,0 +1,168 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/allwinner,sun6i-a31-mipi-csi2.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Allwinner A31 MIPI CSI-2 Device Tree Bindings
+
+maintainers:
+  - Paul Kocialkowski 
+
+properties:
+  compatible:
+oneOf:
+  - const: allwinner,sun6i-a31-mipi-csi2
+  - items:
+  - const: allwinner,sun8i-v3s-mipi-csi2
+  - const: allwinner,sun6i-a31-mipi-csi2
+
+  reg:
+maxItems: 1
+
+  interrupts:
+maxItems: 1
+
+  clocks:
+items:
+  - description: Bus Clock
+  - description: Module Clock
+
+  clock-names:
+items:
+  - const: bus
+  - const: mod
+
+  phys:
+items:
+  - description: MIPI D-PHY
+
+  phy-names:
+items:
+  - const: dphy
+
+  resets:
+maxItems: 1
+
+  # See ./video-interfaces.txt for details
+  ports:
+type: object
+
+properties:
+  port@0:
+type: object
+description: Input port, connect to a MIPI CSI-2 sensor
+
+properties:
+  reg:
+const: 0
+
+  endpoint:
+type: object
+
+properties:
+  remote-endpoint: true
+
+  bus-type:
+const: 4
+
+  clock-lanes:
+maxItems: 1
+
+  data-lanes:
+minItems: 1
+maxItems: 4
+
+required:
+  - bus-type
+  - data-lanes
+  - remote-endpoint
+
+additionalProperties: false
+
+required:
+  - endpoint
+
+additionalProperties: false
+
+  port@1:
+type: object
+description: Output port, connect to a CSI controller
+
+properties:
+  reg:
+const: 1
+
+  endpoint:
+type: object
+
+properties:
+  remote-endpoint: true
+
+  bus-type:
+const: 4
+
+additionalProperties: false
+
+required:
+  - endpoint
+
+additionalProperties: false
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+  - resets
+
+additionalProperties: false
+
+examples:
+  - |
+#include 
+#include 
+#include 
+
+mipi_csi2: mipi-csi2@1cb1000 {
+compatible = "allwinner,sun8i-v3s-mipi-csi2",
+ "allwinner,sun6i-a31-mipi-csi2";
+reg = <0x01cb1000 0x1000>;
+interrupts = ;
+clocks = < CLK_BUS_CSI>,
+ < CLK_CSI1_SCLK>;
+clock-names = "bus", "mod";
+resets = < RST_BUS_CSI>;
+
+phys = <>;
+phy-names = "dphy";
+
+ports {
+#address-cells = <1>;
+#size-cells = <0>;
+
+mipi_csi2_in: port@0 {
+reg = <0>;
+
+mipi_csi2_in_ov5648: endpoint {
+bus-type = <4>; /* MIPI CSI-2 D-PHY */
+clock-lanes = <0>;
+data-lanes = <1 2 3 4>;
+
+remote-endpoint = <_out_mipi_csi2>;
+};
+};
+
+mipi_csi2_out: port@1 {
+reg = <1>;
+
+mipi_csi2_out_csi0: endpoint {
+bus-type = <4>; /* MIPI CSI-2 D-PHY */
+remote-endpoint = <_in_mipi_csi2>;
+};
+};
+};
+};
+
+...
-- 
2.28.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel