diff mbox series

[v2,01/21] dt-bindings: display: tegra: add Tegra20 VIP

Message ID 20221128152336.133953-2-luca.ceresoli@bootlin.com
State Changes Requested, archived
Headers show
Series Add Tegra20 parallel video input capture | expand

Checks

Context Check Description
robh/checkpatch success
robh/patch-applied success
robh/dtbs-check warning build log
robh/dt-meta-schema success
robh/checkpatch success
robh/patch-applied success
robh/dtbs-check warning build log
robh/dt-meta-schema success

Commit Message

Luca Ceresoli Nov. 28, 2022, 3:23 p.m. UTC 
VIP is the parallel video capture component within the video input
subsystem of Tegra20 (and other Tegra chips, apparently).

Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

---

Changed in v2 (suggested by Krzysztof Kozlowski):
- remove redundant "bindings" from subject line
- remove $nodename
- add channel@0 description
- add reg: const: 0
---
 .../display/tegra/nvidia,tegra20-vip.yaml     | 63 +++++++++++++++++++
 MAINTAINERS                                   |  7 +++
 2 files changed, 70 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vip.yaml

Comments

Rob Herring Dec. 1, 2022, 11:19 p.m. UTC | #1 
On Mon, Nov 28, 2022 at 04:23:16PM +0100, Luca Ceresoli wrote:
> VIP is the parallel video capture component within the video input
> subsystem of Tegra20 (and other Tegra chips, apparently).
> 
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> 
> ---
> 
> Changed in v2 (suggested by Krzysztof Kozlowski):
> - remove redundant "bindings" from subject line
> - remove $nodename
> - add channel@0 description
> - add reg: const: 0
> ---
>  .../display/tegra/nvidia,tegra20-vip.yaml     | 63 +++++++++++++++++++
>  MAINTAINERS                                   |  7 +++
>  2 files changed, 70 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vip.yaml
> 
> diff --git a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vip.yaml b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vip.yaml
> new file mode 100644
> index 000000000000..44be2e16c9b4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vip.yaml
> @@ -0,0 +1,63 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/tegra/nvidia,tegra20-vip.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NVIDIA Tegra VIP (parallel video capture) controller
> +
> +maintainers:
> +  - Luca Ceresoli <luca.ceresoli@bootlin.com>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - nvidia,tegra20-vip
> +
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 0
> +
> +  channel@0:

Kind of odd there is only 1 channel with a unit-address. Are more 
channels coming? Please make the binding as complete as possible even if 
no driver support yet.


> +    description: parallel video capture interface for the VI
> +    type: object
> +
> +    properties:
> +      reg:
> +        const: 0
> +
> +      ports:
> +        $ref: /schemas/graph.yaml#/properties/ports
> +
> +        properties:
> +          port@0:
> +            $ref: /schemas/graph.yaml#/properties/port
> +            description:
> +              Port receiving the video stream from the sensor
> +
> +          port@1:
> +            $ref: /schemas/graph.yaml#/properties/port
> +            description:
> +              Port sending the video stream to the VI
> +
> +        required:
> +          - port@0
> +          - port@1
> +
> +    additionalProperties: false

A bit easier to read the indented cases if this is above 'properties'.

> +
> +    required:
> +      - reg
> +      - ports
> +
> +unevaluatedProperties: false
> +
> +required:
> +  - compatible
> +  - "#address-cells"
> +  - "#size-cells"
> +  - channel@0
> +
> +# see nvidia,tegra20-vi.yaml for an example
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 69565ac0c224..92c762f85f17 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -20315,6 +20315,13 @@ S:	Maintained
>  F:	Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml
>  F:	drivers/staging/media/tegra-video/
>  
> +TEGRA VIDEO DRIVER FOR TEGRA20 VIP (PARALLEL VIDEO CAPTURE)
> +M:	Luca Ceresoli <luca.ceresoli@bootlin.com>
> +L:	linux-media@vger.kernel.org
> +L:	linux-tegra@vger.kernel.org
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vip.yaml
> +
>  TEGRA XUSB PADCTL DRIVER
>  M:	JC Kuo <jckuo@nvidia.com>
>  S:	Supported
> -- 
> 2.34.1
> 
>
Luca Ceresoli Dec. 2, 2022, 8:11 a.m. UTC | #2 
Hello Rob,

Thanks for your review.

On Thu, 1 Dec 2022 17:19:36 -0600
Rob Herring <robh@kernel.org> wrote:

> On Mon, Nov 28, 2022 at 04:23:16PM +0100, Luca Ceresoli wrote:
> > VIP is the parallel video capture component within the video input
> > subsystem of Tegra20 (and other Tegra chips, apparently).
> > 
> > Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> > 
> > ---
> > 
> > Changed in v2 (suggested by Krzysztof Kozlowski):
> > - remove redundant "bindings" from subject line
> > - remove $nodename
> > - add channel@0 description
> > - add reg: const: 0
> > ---
> >  .../display/tegra/nvidia,tegra20-vip.yaml     | 63 +++++++++++++++++++
> >  MAINTAINERS                                   |  7 +++
> >  2 files changed, 70 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vip.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vip.yaml b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vip.yaml
> > new file mode 100644
> > index 000000000000..44be2e16c9b4
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vip.yaml
> > @@ -0,0 +1,63 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/display/tegra/nvidia,tegra20-vip.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: NVIDIA Tegra VIP (parallel video capture) controller
> > +
> > +maintainers:
> > +  - Luca Ceresoli <luca.ceresoli@bootlin.com>
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - nvidia,tegra20-vip
> > +
> > +  "#address-cells":
> > +    const: 1
> > +
> > +  "#size-cells":
> > +    const: 0
> > +
> > +  channel@0:  
> 
> Kind of odd there is only 1 channel with a unit-address. Are more 
> channels coming? Please make the binding as complete as possible even if 
> no driver support yet.

This was discussed in v1 with Krzysztof and the outcome was that it's
OK because it's likely that other SoCs have more, but the documentation
is not public so I cannot add examples.

Full discussion (pretty short indeed):

https://lore.kernel.org/linux-devicetree/5292cc1b-c951-c5c5-b2ef-c154baf6d7fd@linaro.org/

Do you agree that the unit-address should be kept?

> > +    description: parallel video capture interface for the VI
> > +    type: object
> > +
> > +    properties:
> > +      reg:
> > +        const: 0
> > +
> > +      ports:
> > +        $ref: /schemas/graph.yaml#/properties/ports
> > +
> > +        properties:
> > +          port@0:
> > +            $ref: /schemas/graph.yaml#/properties/port
> > +            description:
> > +              Port receiving the video stream from the sensor
> > +
> > +          port@1:
> > +            $ref: /schemas/graph.yaml#/properties/port
> > +            description:
> > +              Port sending the video stream to the VI
> > +
> > +        required:
> > +          - port@0
> > +          - port@1
> > +
> > +    additionalProperties: false  
> 
> A bit easier to read the indented cases if this is above 'properties'.

Sure, will do in v3.
Dmitry Osipenko Dec. 20, 2022, 8:13 p.m. UTC | #3 
02.12.2022 11:11, Luca Ceresoli пишет:
> Hello Rob,
> 
> Thanks for your review.
> 
> On Thu, 1 Dec 2022 17:19:36 -0600
> Rob Herring <robh@kernel.org> wrote:
> 
>> On Mon, Nov 28, 2022 at 04:23:16PM +0100, Luca Ceresoli wrote:
>>> VIP is the parallel video capture component within the video input
>>> subsystem of Tegra20 (and other Tegra chips, apparently).
>>>
>>> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
>>>
>>> ---
>>>
>>> Changed in v2 (suggested by Krzysztof Kozlowski):
>>> - remove redundant "bindings" from subject line
>>> - remove $nodename
>>> - add channel@0 description
>>> - add reg: const: 0
>>> ---
>>>  .../display/tegra/nvidia,tegra20-vip.yaml     | 63 +++++++++++++++++++
>>>  MAINTAINERS                                   |  7 +++
>>>  2 files changed, 70 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vip.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vip.yaml b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vip.yaml
>>> new file mode 100644
>>> index 000000000000..44be2e16c9b4
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vip.yaml
>>> @@ -0,0 +1,63 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/display/tegra/nvidia,tegra20-vip.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: NVIDIA Tegra VIP (parallel video capture) controller
>>> +
>>> +maintainers:
>>> +  - Luca Ceresoli <luca.ceresoli@bootlin.com>
>>> +
>>> +properties:
>>> +  compatible:
>>> +    enum:
>>> +      - nvidia,tegra20-vip
>>> +
>>> +  "#address-cells":
>>> +    const: 1
>>> +
>>> +  "#size-cells":
>>> +    const: 0
>>> +
>>> +  channel@0:  
>> Kind of odd there is only 1 channel with a unit-address. Are more 
>> channels coming? Please make the binding as complete as possible even if 
>> no driver support yet.
> This was discussed in v1 with Krzysztof and the outcome was that it's
> OK because it's likely that other SoCs have more, but the documentation
> is not public so I cannot add examples.
> 
> Full discussion (pretty short indeed):
> 
> https://lore.kernel.org/linux-devicetree/5292cc1b-c951-c5c5-b2ef-c154baf6d7fd@linaro.org/
> 
> Do you agree that the unit-address should be kept?

It's doubtful that there is a SoC having a VIP with multiple channels.
I'd expect it to be multiple VIPs rather than channels. There are NVIDIA
people to confirm that.

The "channel" itself looks redundant to me, i.e. the reg and ports
should be moved to the vip node.
Luca Ceresoli Dec. 22, 2022, 9:03 a.m. UTC | #4 
Hello Dmitry,

On Tue, 20 Dec 2022 23:13:05 +0300
Dmitry Osipenko <digetx@gmail.com> wrote:

> 02.12.2022 11:11, Luca Ceresoli пишет:

...

> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vip.yaml
> >>> @@ -0,0 +1,63 @@
> >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >>> +%YAML 1.2
> >>> +---
> >>> +$id: http://devicetree.org/schemas/display/tegra/nvidia,tegra20-vip.yaml#
> >>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >>> +
> >>> +title: NVIDIA Tegra VIP (parallel video capture) controller
> >>> +
> >>> +maintainers:
> >>> +  - Luca Ceresoli <luca.ceresoli@bootlin.com>
> >>> +
> >>> +properties:
> >>> +  compatible:
> >>> +    enum:
> >>> +      - nvidia,tegra20-vip
> >>> +
> >>> +  "#address-cells":
> >>> +    const: 1
> >>> +
> >>> +  "#size-cells":
> >>> +    const: 0
> >>> +
> >>> +  channel@0:    
> >> Kind of odd there is only 1 channel with a unit-address. Are more 
> >> channels coming? Please make the binding as complete as possible even if 
> >> no driver support yet.  
> > This was discussed in v1 with Krzysztof and the outcome was that it's
> > OK because it's likely that other SoCs have more, but the documentation
> > is not public so I cannot add examples.
> > 
> > Full discussion (pretty short indeed):
> > 
> > https://lore.kernel.org/linux-devicetree/5292cc1b-c951-c5c5-b2ef-c154baf6d7fd@linaro.org/
> > 
> > Do you agree that the unit-address should be kept?  
> 
> It's doubtful that there is a SoC having a VIP with multiple channels.
> I'd expect it to be multiple VIPs rather than channels. There are NVIDIA
> people to confirm that.
> 
> The "channel" itself looks redundant to me, i.e. the reg and ports
> should be moved to the vip node.

OK, will do in v3 unless there are different opinions.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vip.yaml b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vip.yaml
new file mode 100644
index 000000000000..44be2e16c9b4
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vip.yaml
@@ -0,0 +1,63 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/tegra/nvidia,tegra20-vip.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NVIDIA Tegra VIP (parallel video capture) controller
+
+maintainers:
+  - Luca Ceresoli <luca.ceresoli@bootlin.com>
+
+properties:
+  compatible:
+    enum:
+      - nvidia,tegra20-vip
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0
+
+  channel@0:
+    description: parallel video capture interface for the VI
+    type: object
+
+    properties:
+      reg:
+        const: 0
+
+      ports:
+        $ref: /schemas/graph.yaml#/properties/ports
+
+        properties:
+          port@0:
+            $ref: /schemas/graph.yaml#/properties/port
+            description:
+              Port receiving the video stream from the sensor
+
+          port@1:
+            $ref: /schemas/graph.yaml#/properties/port
+            description:
+              Port sending the video stream to the VI
+
+        required:
+          - port@0
+          - port@1
+
+    additionalProperties: false
+
+    required:
+      - reg
+      - ports
+
+unevaluatedProperties: false
+
+required:
+  - compatible
+  - "#address-cells"
+  - "#size-cells"
+  - channel@0
+
+# see nvidia,tegra20-vi.yaml for an example
diff --git a/MAINTAINERS b/MAINTAINERS
index 69565ac0c224..92c762f85f17 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -20315,6 +20315,13 @@  S:	Maintained
 F:	Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml
 F:	drivers/staging/media/tegra-video/
 
+TEGRA VIDEO DRIVER FOR TEGRA20 VIP (PARALLEL VIDEO CAPTURE)
+M:	Luca Ceresoli <luca.ceresoli@bootlin.com>
+L:	linux-media@vger.kernel.org
+L:	linux-tegra@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vip.yaml
+
 TEGRA XUSB PADCTL DRIVER
 M:	JC Kuo <jckuo@nvidia.com>
 S:	Supported