diff mbox series

[12/16] dt-bindings: media: Add DT bindings for TI CSI2RX driver

Message ID 20210330173348.30135-13-p.yadav@ti.com (mailing list archive)
State New, archived
Headers show
Series CSI2RX support on J721E | expand

Commit Message

Pratyush Yadav March 30, 2021, 5:33 p.m. UTC
TI's J721E uses the Cadence CSI2RX and DPHY peripherals to facilitate
capture over a CSI-2 bus. The TI CSI2RX platform driver glues all the
parts together.

Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
---
 .../devicetree/bindings/media/ti,csi2rx.yaml  | 70 +++++++++++++++++++
 1 file changed, 70 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/ti,csi2rx.yaml

Comments

Rob Herring (Arm) March 31, 2021, 1:40 p.m. UTC | #1
On Tue, 30 Mar 2021 23:03:44 +0530, Pratyush Yadav wrote:
> TI's J721E uses the Cadence CSI2RX and DPHY peripherals to facilitate
> capture over a CSI-2 bus. The TI CSI2RX platform driver glues all the
> parts together.
> 
> Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
> ---
>  .../devicetree/bindings/media/ti,csi2rx.yaml  | 70 +++++++++++++++++++
>  1 file changed, 70 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/ti,csi2rx.yaml
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/media/ti,csi2rx.example.dts:21.30-29.11: Warning (unit_address_vs_reg): /example-0/ticsi2rx: node has a reg or ranges property, but no unit name

See https://patchwork.ozlabs.org/patch/1460166

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.
Rob Herring (Arm) April 1, 2021, 3:52 p.m. UTC | #2
On Tue, Mar 30, 2021 at 11:03:44PM +0530, Pratyush Yadav wrote:
> TI's J721E uses the Cadence CSI2RX and DPHY peripherals to facilitate
> capture over a CSI-2 bus. The TI CSI2RX platform driver glues all the
> parts together.
> 
> Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
> ---
>  .../devicetree/bindings/media/ti,csi2rx.yaml  | 70 +++++++++++++++++++
>  1 file changed, 70 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/ti,csi2rx.yaml
> 
> diff --git a/Documentation/devicetree/bindings/media/ti,csi2rx.yaml b/Documentation/devicetree/bindings/media/ti,csi2rx.yaml
> new file mode 100644
> index 000000000000..ebd894364391
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/ti,csi2rx.yaml
> @@ -0,0 +1,70 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/ti,csi2rx.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: TI CSI2RX Wrapper Device Tree Bindings
> +
> +maintainers:
> +  - Pratyush Yadav <p.yadav@ti.com>
> +
> +properties:
> +  compatible:
> +    items:
> +      - const: ti,csi2rx
> +
> +  dmas:
> +    description: RX DMA Channel 0

items:
  - description: RX DMA Channel 0

Or just 'maxItems: 1'

> +
> +  dma-names:
> +    items:
> +      - const: rx0
> +
> +  reg:
> +    maxItems: 1
> +    description: Base address and size of the TI wrapper registers.

That's all 'reg' properties, drop 'description'.

> +
> +  power-domains:
> +    maxItems: 1
> +    description:
> +      PM domain provider node and an args specifier containing
> +      the device id value.

Drop.

> +
> +  ranges: true
> +
> +  "#address-cells":
> +    const: 2
> +
> +  "#size-cells":
> +    const: 2
> +
> +patternProperties:
> +  "csi-bridge@":

"^csi-bridge@"

> +    type: object
> +    description: CSI2 bridge node.

Just an empty node?

> +
> +required:
> +  - compatible
> +  - reg
> +  - dmas
> +  - dma-names
> +  - power-domains
> +  - "#address-cells"
> +  - "#size-cells"
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/soc/ti,sci_pm_domain.h>
> +
> +    ti_csi2rx0: ticsi2rx {
> +        compatible = "ti,csi2rx";
> +        dmas = <&main_udmap 0x4940>;
> +        dma-names = "rx0";
> +        reg = <0x0 0x4500000 0x0 0x1000>;
> +        power-domains = <&k3_pds 26 TI_SCI_PD_EXCLUSIVE>;
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +    };
> -- 
> 2.30.0
>
Rob Herring (Arm) April 1, 2021, 3:52 p.m. UTC | #3
On Tue, Mar 30, 2021 at 11:03:44PM +0530, Pratyush Yadav wrote:
> TI's J721E uses the Cadence CSI2RX and DPHY peripherals to facilitate
> capture over a CSI-2 bus. The TI CSI2RX platform driver glues all the
> parts together.
> 
> Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
> ---
>  .../devicetree/bindings/media/ti,csi2rx.yaml  | 70 +++++++++++++++++++
>  1 file changed, 70 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/ti,csi2rx.yaml
> 
> diff --git a/Documentation/devicetree/bindings/media/ti,csi2rx.yaml b/Documentation/devicetree/bindings/media/ti,csi2rx.yaml
> new file mode 100644
> index 000000000000..ebd894364391
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/ti,csi2rx.yaml
> @@ -0,0 +1,70 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/ti,csi2rx.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: TI CSI2RX Wrapper Device Tree Bindings
> +
> +maintainers:
> +  - Pratyush Yadav <p.yadav@ti.com>
> +
> +properties:
> +  compatible:
> +    items:
> +      - const: ti,csi2rx

Also, needs an SoC specific compatible.

Rob
Laurent Pinchart April 2, 2021, 10:01 a.m. UTC | #4
On Thu, Apr 01, 2021 at 10:52:01AM -0500, Rob Herring wrote:
> On Tue, Mar 30, 2021 at 11:03:44PM +0530, Pratyush Yadav wrote:
> > TI's J721E uses the Cadence CSI2RX and DPHY peripherals to facilitate
> > capture over a CSI-2 bus. The TI CSI2RX platform driver glues all the
> > parts together.
> > 
> > Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
> > ---
> >  .../devicetree/bindings/media/ti,csi2rx.yaml  | 70 +++++++++++++++++++
> >  1 file changed, 70 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/media/ti,csi2rx.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/media/ti,csi2rx.yaml b/Documentation/devicetree/bindings/media/ti,csi2rx.yaml
> > new file mode 100644
> > index 000000000000..ebd894364391
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/ti,csi2rx.yaml
> > @@ -0,0 +1,70 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/media/ti,csi2rx.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: TI CSI2RX Wrapper Device Tree Bindings
> > +

A description would be useful, especially given that the TRM doesn't
mention "CSI2RX".

> > +maintainers:
> > +  - Pratyush Yadav <p.yadav@ti.com>
> > +
> > +properties:
> > +  compatible:
> > +    items:
> > +      - const: ti,csi2rx
> > +
> > +  dmas:
> > +    description: RX DMA Channel 0
> 
> items:
>   - description: RX DMA Channel 0
> 
> Or just 'maxItems: 1'
> 
> > +
> > +  dma-names:
> > +    items:
> > +      - const: rx0
> > +
> > +  reg:
> > +    maxItems: 1
> > +    description: Base address and size of the TI wrapper registers.
> 
> That's all 'reg' properties, drop 'description'.

According to SPRUIL1B, there are four register banks for the CSI_RX_IF,
and two register banks for the DPHY_RX. What's your plan to support
these ? Not everything need to be implemented at once, but backward
compatibility need to be taken into account in the design.

> > +
> > +  power-domains:
> > +    maxItems: 1
> > +    description:
> > +      PM domain provider node and an args specifier containing
> > +      the device id value.
> 
> Drop.
> 
> > +
> > +  ranges: true
> > +
> > +  "#address-cells":
> > +    const: 2
> > +
> > +  "#size-cells":
> > +    const: 2
> > +
> > +patternProperties:
> > +  "csi-bridge@":
> 
> "^csi-bridge@"
> 
> > +    type: object
> > +    description: CSI2 bridge node.
> 
> Just an empty node?

Even if the node is optional, it would be useful to include it in the
example below, to show how it's supposed to be used.

> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - dmas
> > +  - dma-names
> > +  - power-domains
> > +  - "#address-cells"
> > +  - "#size-cells"
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/soc/ti,sci_pm_domain.h>
> > +
> > +    ti_csi2rx0: ticsi2rx {
> > +        compatible = "ti,csi2rx";
> > +        dmas = <&main_udmap 0x4940>;
> > +        dma-names = "rx0";
> > +        reg = <0x0 0x4500000 0x0 0x1000>;
> > +        power-domains = <&k3_pds 26 TI_SCI_PD_EXCLUSIVE>;
> > +        #address-cells = <2>;
> > +        #size-cells = <2>;
> > +    };
Laurent Pinchart April 2, 2021, 10:53 a.m. UTC | #5
On Fri, Apr 02, 2021 at 01:01:22PM +0300, Laurent Pinchart wrote:
> On Thu, Apr 01, 2021 at 10:52:01AM -0500, Rob Herring wrote:
> > On Tue, Mar 30, 2021 at 11:03:44PM +0530, Pratyush Yadav wrote:
> > > TI's J721E uses the Cadence CSI2RX and DPHY peripherals to facilitate
> > > capture over a CSI-2 bus. The TI CSI2RX platform driver glues all the
> > > parts together.
> > > 
> > > Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
> > > ---
> > >  .../devicetree/bindings/media/ti,csi2rx.yaml  | 70 +++++++++++++++++++
> > >  1 file changed, 70 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/media/ti,csi2rx.yaml
> > > 
> > > diff --git a/Documentation/devicetree/bindings/media/ti,csi2rx.yaml b/Documentation/devicetree/bindings/media/ti,csi2rx.yaml
> > > new file mode 100644
> > > index 000000000000..ebd894364391
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/media/ti,csi2rx.yaml
> > > @@ -0,0 +1,70 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/media/ti,csi2rx.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: TI CSI2RX Wrapper Device Tree Bindings
> > > +
> 
> A description would be useful, especially given that the TRM doesn't
> mention "CSI2RX".
> 
> > > +maintainers:
> > > +  - Pratyush Yadav <p.yadav@ti.com>
> > > +
> > > +properties:
> > > +  compatible:
> > > +    items:
> > > +      - const: ti,csi2rx
> > > +
> > > +  dmas:
> > > +    description: RX DMA Channel 0
> > 
> > items:
> >   - description: RX DMA Channel 0
> > 
> > Or just 'maxItems: 1'
> > 
> > > +
> > > +  dma-names:
> > > +    items:
> > > +      - const: rx0
> > > +
> > > +  reg:
> > > +    maxItems: 1
> > > +    description: Base address and size of the TI wrapper registers.
> > 
> > That's all 'reg' properties, drop 'description'.
> 
> According to SPRUIL1B, there are four register banks for the CSI_RX_IF,
> and two register banks for the DPHY_RX. What's your plan to support
> these ? Not everything need to be implemented at once, but backward
> compatibility need to be taken into account in the design.
> 
> > > +
> > > +  power-domains:
> > > +    maxItems: 1
> > > +    description:
> > > +      PM domain provider node and an args specifier containing
> > > +      the device id value.
> > 
> > Drop.
> > 
> > > +
> > > +  ranges: true
> > > +
> > > +  "#address-cells":
> > > +    const: 2
> > > +
> > > +  "#size-cells":
> > > +    const: 2
> > > +
> > > +patternProperties:
> > > +  "csi-bridge@":
> > 
> > "^csi-bridge@"
> > 
> > > +    type: object
> > > +    description: CSI2 bridge node.
> > 
> > Just an empty node?
> 
> Even if the node is optional, it would be useful to include it in the
> example below, to show how it's supposed to be used.
> 
> > > +
> > > +required:
> > > +  - compatible
> > > +  - reg
> > > +  - dmas
> > > +  - dma-names
> > > +  - power-domains
> > > +  - "#address-cells"
> > > +  - "#size-cells"
> > > +
> > > +additionalProperties: false
> > > +
> > > +examples:
> > > +  - |
> > > +    #include <dt-bindings/soc/ti,sci_pm_domain.h>
> > > +
> > > +    ti_csi2rx0: ticsi2rx {
> > > +        compatible = "ti,csi2rx";
> > > +        dmas = <&main_udmap 0x4940>;
> > > +        dma-names = "rx0";
> > > +        reg = <0x0 0x4500000 0x0 0x1000>;
> > > +        power-domains = <&k3_pds 26 TI_SCI_PD_EXCLUSIVE>;
> > > +        #address-cells = <2>;
> > > +        #size-cells = <2>;
> > > +    };

It would also be useful to expand this to a full example that includes
integration with the PHY.
Pratyush Yadav April 6, 2021, 6 p.m. UTC | #6
On 01/04/21 10:52AM, Rob Herring wrote:
> On Tue, Mar 30, 2021 at 11:03:44PM +0530, Pratyush Yadav wrote:
> > TI's J721E uses the Cadence CSI2RX and DPHY peripherals to facilitate
> > capture over a CSI-2 bus. The TI CSI2RX platform driver glues all the
> > parts together.
> > 
> > Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
> > ---
> >  .../devicetree/bindings/media/ti,csi2rx.yaml  | 70 +++++++++++++++++++
> >  1 file changed, 70 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/media/ti,csi2rx.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/media/ti,csi2rx.yaml b/Documentation/devicetree/bindings/media/ti,csi2rx.yaml
> > new file mode 100644
> > index 000000000000..ebd894364391
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/ti,csi2rx.yaml
> > @@ -0,0 +1,70 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/media/ti,csi2rx.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: TI CSI2RX Wrapper Device Tree Bindings
> > +
> > +maintainers:
> > +  - Pratyush Yadav <p.yadav@ti.com>
> > +
> > +properties:
> > +  compatible:
> > +    items:
> > +      - const: ti,csi2rx
> > +
> > +  dmas:
> > +    description: RX DMA Channel 0
> 
> items:
>   - description: RX DMA Channel 0
> 
> Or just 'maxItems: 1'

Ok.

> 
> > +
> > +  dma-names:
> > +    items:
> > +      - const: rx0
> > +
> > +  reg:
> > +    maxItems: 1
> > +    description: Base address and size of the TI wrapper registers.
> 
> That's all 'reg' properties, drop 'description'.

Ok.

> 
> > +
> > +  power-domains:
> > +    maxItems: 1
> > +    description:
> > +      PM domain provider node and an args specifier containing
> > +      the device id value.
> 
> Drop.

Ok.

> 
> > +
> > +  ranges: true
> > +
> > +  "#address-cells":
> > +    const: 2
> > +
> > +  "#size-cells":
> > +    const: 2
> > +
> > +patternProperties:
> > +  "csi-bridge@":
> 
> "^csi-bridge@"

Ok.

> 
> > +    type: object
> > +    description: CSI2 bridge node.
> 
> Just an empty node?

No. It should be a node for the Cadence csi2rx IP (compatible 
"cdns,csi2rx"). I'm not sure how to model this. This subnode is needed 
but it should take its properties from the Cadence csi2rx schema. Will a 

  properties:
    allOf:
      - $ref: cdns,csi2rx.yaml#

be a good idea?

> 
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - dmas
> > +  - dma-names
> > +  - power-domains
> > +  - "#address-cells"
> > +  - "#size-cells"
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/soc/ti,sci_pm_domain.h>
> > +
> > +    ti_csi2rx0: ticsi2rx {
> > +        compatible = "ti,csi2rx";
> > +        dmas = <&main_udmap 0x4940>;
> > +        dma-names = "rx0";
> > +        reg = <0x0 0x4500000 0x0 0x1000>;
> > +        power-domains = <&k3_pds 26 TI_SCI_PD_EXCLUSIVE>;
> > +        #address-cells = <2>;
> > +        #size-cells = <2>;
> > +    };
> > -- 
> > 2.30.0
> >
Pratyush Yadav April 6, 2021, 6:13 p.m. UTC | #7
On 02/04/21 01:01PM, Laurent Pinchart wrote:
> On Thu, Apr 01, 2021 at 10:52:01AM -0500, Rob Herring wrote:
> > On Tue, Mar 30, 2021 at 11:03:44PM +0530, Pratyush Yadav wrote:
> > > TI's J721E uses the Cadence CSI2RX and DPHY peripherals to facilitate
> > > capture over a CSI-2 bus. The TI CSI2RX platform driver glues all the
> > > parts together.
> > > 
> > > Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
> > > ---
> > >  .../devicetree/bindings/media/ti,csi2rx.yaml  | 70 +++++++++++++++++++
> > >  1 file changed, 70 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/media/ti,csi2rx.yaml
> > > 
> > > diff --git a/Documentation/devicetree/bindings/media/ti,csi2rx.yaml b/Documentation/devicetree/bindings/media/ti,csi2rx.yaml
> > > new file mode 100644
> > > index 000000000000..ebd894364391
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/media/ti,csi2rx.yaml
> > > @@ -0,0 +1,70 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/media/ti,csi2rx.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: TI CSI2RX Wrapper Device Tree Bindings
> > > +
> 
> A description would be useful, especially given that the TRM doesn't
> mention "CSI2RX".

Ok.

> 
> > > +maintainers:
> > > +  - Pratyush Yadav <p.yadav@ti.com>
> > > +
> > > +properties:
> > > +  compatible:
> > > +    items:
> > > +      - const: ti,csi2rx
> > > +
> > > +  dmas:
> > > +    description: RX DMA Channel 0
> > 
> > items:
> >   - description: RX DMA Channel 0
> > 
> > Or just 'maxItems: 1'
> > 
> > > +
> > > +  dma-names:
> > > +    items:
> > > +      - const: rx0
> > > +
> > > +  reg:
> > > +    maxItems: 1
> > > +    description: Base address and size of the TI wrapper registers.
> > 
> > That's all 'reg' properties, drop 'description'.
> 
> According to SPRUIL1B, there are four register banks for the CSI_RX_IF,
> and two register banks for the DPHY_RX. What's your plan to support
> these ? Not everything need to be implemented at once, but backward
> compatibility need to be taken into account in the design.

The CSI_RX_IF0_ECC_AGGR_CFG register bank is for safety requirements. 
The driver does not use them. The CSI_RX_IF0_RX_SHIM_VBUSP_MMR_CSI2RXIF 
register bank is for the TI wrapper around the Cadence CSI2RX bridge 
that deals with DMA threads. This bank is what this binding is concerned 
with. The CSI_RX_IF0_VBUS2APB_WRAP_VBUS_APB_CSI2RX bank is for the 
Cadence CSI2RX bridge. The Cadence schema should deal with that. And 
lastly, I don't know what the CSI_RX_IF0_CP_INTD_CFG_INTD_CFG bank is 
for. The driver does not use it.

I don't forsee the first and last bank being used in Kernel, but if we 
want to be safe I can change maxItems to 3. Sounds good?

> 
> > > +
> > > +  power-domains:
> > > +    maxItems: 1
> > > +    description:
> > > +      PM domain provider node and an args specifier containing
> > > +      the device id value.
> > 
> > Drop.
> > 
> > > +
> > > +  ranges: true
> > > +
> > > +  "#address-cells":
> > > +    const: 2
> > > +
> > > +  "#size-cells":
> > > +    const: 2
> > > +
> > > +patternProperties:
> > > +  "csi-bridge@":
> > 
> > "^csi-bridge@"
> > 
> > > +    type: object
> > > +    description: CSI2 bridge node.
> > 
> > Just an empty node?
> 
> Even if the node is optional, it would be useful to include it in the
> example below, to show how it's supposed to be used.

It is not optional. It should be the Cadence CSI2RX bridge node. Will 
add it in the example. I also need to see if there is any way to make a 
patternProperty a required property.

> 
> > > +
> > > +required:
> > > +  - compatible
> > > +  - reg
> > > +  - dmas
> > > +  - dma-names
> > > +  - power-domains
> > > +  - "#address-cells"
> > > +  - "#size-cells"
> > > +
> > > +additionalProperties: false
> > > +
> > > +examples:
> > > +  - |
> > > +    #include <dt-bindings/soc/ti,sci_pm_domain.h>
> > > +
> > > +    ti_csi2rx0: ticsi2rx {
> > > +        compatible = "ti,csi2rx";
> > > +        dmas = <&main_udmap 0x4940>;
> > > +        dma-names = "rx0";
> > > +        reg = <0x0 0x4500000 0x0 0x1000>;
> > > +        power-domains = <&k3_pds 26 TI_SCI_PD_EXCLUSIVE>;
> > > +        #address-cells = <2>;
> > > +        #size-cells = <2>;
> > > +    };
> 
> -- 
> Regards,
> 
> Laurent Pinchart
Pratyush Yadav April 6, 2021, 6:15 p.m. UTC | #8
On 02/04/21 01:53PM, Laurent Pinchart wrote:
> On Fri, Apr 02, 2021 at 01:01:22PM +0300, Laurent Pinchart wrote:
> > On Thu, Apr 01, 2021 at 10:52:01AM -0500, Rob Herring wrote:
> > > On Tue, Mar 30, 2021 at 11:03:44PM +0530, Pratyush Yadav wrote:
> > > > TI's J721E uses the Cadence CSI2RX and DPHY peripherals to facilitate
> > > > capture over a CSI-2 bus. The TI CSI2RX platform driver glues all the
> > > > parts together.
> > > > 
> > > > Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
> > > > ---
> > > >  .../devicetree/bindings/media/ti,csi2rx.yaml  | 70 +++++++++++++++++++
> > > >  1 file changed, 70 insertions(+)
> > > >  create mode 100644 Documentation/devicetree/bindings/media/ti,csi2rx.yaml
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/media/ti,csi2rx.yaml b/Documentation/devicetree/bindings/media/ti,csi2rx.yaml
> > > > new file mode 100644
> > > > index 000000000000..ebd894364391
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/media/ti,csi2rx.yaml
> > > > @@ -0,0 +1,70 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/media/ti,csi2rx.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: TI CSI2RX Wrapper Device Tree Bindings
> > > > +
> > 
> > A description would be useful, especially given that the TRM doesn't
> > mention "CSI2RX".
> > 
> > > > +maintainers:
> > > > +  - Pratyush Yadav <p.yadav@ti.com>
> > > > +
> > > > +properties:
> > > > +  compatible:
> > > > +    items:
> > > > +      - const: ti,csi2rx
> > > > +
> > > > +  dmas:
> > > > +    description: RX DMA Channel 0
> > > 
> > > items:
> > >   - description: RX DMA Channel 0
> > > 
> > > Or just 'maxItems: 1'
> > > 
> > > > +
> > > > +  dma-names:
> > > > +    items:
> > > > +      - const: rx0
> > > > +
> > > > +  reg:
> > > > +    maxItems: 1
> > > > +    description: Base address and size of the TI wrapper registers.
> > > 
> > > That's all 'reg' properties, drop 'description'.
> > 
> > According to SPRUIL1B, there are four register banks for the CSI_RX_IF,
> > and two register banks for the DPHY_RX. What's your plan to support
> > these ? Not everything need to be implemented at once, but backward
> > compatibility need to be taken into account in the design.
> > 
> > > > +
> > > > +  power-domains:
> > > > +    maxItems: 1
> > > > +    description:
> > > > +      PM domain provider node and an args specifier containing
> > > > +      the device id value.
> > > 
> > > Drop.
> > > 
> > > > +
> > > > +  ranges: true
> > > > +
> > > > +  "#address-cells":
> > > > +    const: 2
> > > > +
> > > > +  "#size-cells":
> > > > +    const: 2
> > > > +
> > > > +patternProperties:
> > > > +  "csi-bridge@":
> > > 
> > > "^csi-bridge@"
> > > 
> > > > +    type: object
> > > > +    description: CSI2 bridge node.
> > > 
> > > Just an empty node?
> > 
> > Even if the node is optional, it would be useful to include it in the
> > example below, to show how it's supposed to be used.
> > 
> > > > +
> > > > +required:
> > > > +  - compatible
> > > > +  - reg
> > > > +  - dmas
> > > > +  - dma-names
> > > > +  - power-domains
> > > > +  - "#address-cells"
> > > > +  - "#size-cells"
> > > > +
> > > > +additionalProperties: false
> > > > +
> > > > +examples:
> > > > +  - |
> > > > +    #include <dt-bindings/soc/ti,sci_pm_domain.h>
> > > > +
> > > > +    ti_csi2rx0: ticsi2rx {
> > > > +        compatible = "ti,csi2rx";
> > > > +        dmas = <&main_udmap 0x4940>;
> > > > +        dma-names = "rx0";
> > > > +        reg = <0x0 0x4500000 0x0 0x1000>;
> > > > +        power-domains = <&k3_pds 26 TI_SCI_PD_EXCLUSIVE>;
> > > > +        #address-cells = <2>;
> > > > +        #size-cells = <2>;
> > > > +    };
> 
> It would also be useful to expand this to a full example that includes
> integration with the PHY.

Integration with PHY is Cadence CSI2RX schema's problem. But I will add 
the subnode here anyway so it should have the PHY related properties as 
well.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/media/ti,csi2rx.yaml b/Documentation/devicetree/bindings/media/ti,csi2rx.yaml
new file mode 100644
index 000000000000..ebd894364391
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/ti,csi2rx.yaml
@@ -0,0 +1,70 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/ti,csi2rx.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: TI CSI2RX Wrapper Device Tree Bindings
+
+maintainers:
+  - Pratyush Yadav <p.yadav@ti.com>
+
+properties:
+  compatible:
+    items:
+      - const: ti,csi2rx
+
+  dmas:
+    description: RX DMA Channel 0
+
+  dma-names:
+    items:
+      - const: rx0
+
+  reg:
+    maxItems: 1
+    description: Base address and size of the TI wrapper registers.
+
+  power-domains:
+    maxItems: 1
+    description:
+      PM domain provider node and an args specifier containing
+      the device id value.
+
+  ranges: true
+
+  "#address-cells":
+    const: 2
+
+  "#size-cells":
+    const: 2
+
+patternProperties:
+  "csi-bridge@":
+    type: object
+    description: CSI2 bridge node.
+
+required:
+  - compatible
+  - reg
+  - dmas
+  - dma-names
+  - power-domains
+  - "#address-cells"
+  - "#size-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/soc/ti,sci_pm_domain.h>
+
+    ti_csi2rx0: ticsi2rx {
+        compatible = "ti,csi2rx";
+        dmas = <&main_udmap 0x4940>;
+        dma-names = "rx0";
+        reg = <0x0 0x4500000 0x0 0x1000>;
+        power-domains = <&k3_pds 26 TI_SCI_PD_EXCLUSIVE>;
+        #address-cells = <2>;
+        #size-cells = <2>;
+    };