diff mbox series

[v4,1/6] dt-bindings: media: platform: visconti: Add Toshiba Visconti Video Input Interface bindings

Message ID 20230110014143.18684-2-yuji2.ishikawa@toshiba.co.jp
State New
Headers show
Series [v4,1/6] dt-bindings: media: platform: visconti: Add Toshiba Visconti Video Input Interface bindings | expand

Commit Message

Yuji Ishikawa Jan. 10, 2023, 1:41 a.m. UTC 
Adds the Device Tree binding documentation that allows to describe
the Video Input Interface found in Toshiba Visconti SoCs.

Signed-off-by: Yuji Ishikawa <yuji2.ishikawa@toshiba.co.jp>
Reviewed-by: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
---
Changelog v2:
- no change

Changelog v3:
- no change

Changelog v4:
- fix style problems at the v3 patch
- remove "index" member
- update example
---
 .../bindings/media/toshiba,visconti-viif.yaml | 98 +++++++++++++++++++
 1 file changed, 98 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/toshiba,visconti-viif.yaml

Comments

Yuji Ishikawa Jan. 11, 2023, 1:21 p.m. UTC | #1 
Hello Krzysztof,

Sorry, I missed your comments following the topic of recipients list.

Below are the rest of the responses

> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: Wednesday, January 11, 2023 4:31 AM
> To: ishikawa yuji(石川 悠司 ○ＲＤＣ□ＡＩＴＣ○ＥＡ開)
> <yuji2.ishikawa@toshiba.co.jp>; Hans Verkuil <hverkuil@xs4all.nl>; Laurent
> Pinchart <laurent.pinchart@ideasonboard.com>; Mauro Carvalho Chehab
> <mchehab@kernel.org>; iwamatsu nobuhiro(岩松 信洋 □ＳＷＣ◯ＡＣＴ)
> <nobuhiro1.iwamatsu@toshiba.co.jp>
> Cc: linux-media@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> linux-kernel@vger.kernel.org; devicetree@vger.kernel.org
> Subject: Re: [PATCH v4 1/6] dt-bindings: media: platform: visconti: Add Toshiba
> Visconti Video Input Interface bindings
> 
> On 10/01/2023 02:41, Yuji Ishikawa wrote:
> > Adds the Device Tree binding documentation that allows to describe the
> > Video Input Interface found in Toshiba Visconti SoCs.
> >
> > Signed-off-by: Yuji Ishikawa <yuji2.ishikawa@toshiba.co.jp>
> > Reviewed-by: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
> 
> 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 missed few of them, so clearly this was not sent correctly.
> 
> 
> Subject: drop second/last, redundant "bindings". The "dt-bindings"
> prefix is already stating that these are bindings.
> 
> > ---
> > Changelog v2:
> > - no change
> >
> > Changelog v3:
> > - no change
> >
> > Changelog v4:
> > - fix style problems at the v3 patch
> > - remove "index" member
> > - update example
> > ---
> >  .../bindings/media/toshiba,visconti-viif.yaml | 98
> > +++++++++++++++++++
> >  1 file changed, 98 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/media/toshiba,visconti-viif.yaml
> >
> > diff --git
> > a/Documentation/devicetree/bindings/media/toshiba,visconti-viif.yaml
> > b/Documentation/devicetree/bindings/media/toshiba,visconti-viif.yaml
> > new file mode 100644
> > index 00000000000..71442724d1a
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/toshiba,visconti-viif.ya
> > +++ ml
> > @@ -0,0 +1,98 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/media/toshiba,visconti-viif.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Toshiba Visconti5 SoC Video Input Interface Device Tree
> > +Bindings
> 
> Drop "Device Tree Bindings"

I'll drop it.

> 
> > +
> > +maintainers:
> > +  - Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
> > +
> > +description:
> > +  Toshiba Visconti5 SoC Video Input Interface (VIIF)
> > +  receives MIPI CSI2 video stream,
> > +  processes the stream with embedded image signal processor (L1ISP,
> > +L2ISP),
> > +  then stores pictures to main memory.
> 
> Fix wrapping.

I'll fix this.
I didn't realize a new line has been added through formatting a patch.
 
> > +
> > +properties:
> > +  compatible:
> > +    const: toshiba,visconti-viif
> 
> Compatible must be specific. You called your SoC visconti5, didn't you?
> 

The Video Input Interface hardware is likely to be used at future SoCs of
Visconti Architecture.
Does compatible have to be specific to SoC's model name 
rather than architecture name?

> > +
> > +  reg:
> > +    items:
> > +      - description: registers for capture control
> > +      - description: registers for CSI2 receiver control
> > +
> > +  interrupts:
> > +    items:
> > +      - description: Sync Interrupt
> > +      - description: Status (Error) Interrupt
> > +      - description: CSI2 Receiver Interrupt
> > +      - description: L1ISP Interrupt
> > +
> 
> 
> 
> Best regards,
> Krzysztof

Regards,
Yuji
Krzysztof Kozlowski Jan. 11, 2023, 1:28 p.m. UTC | #2 
On 11/01/2023 14:21, yuji2.ishikawa@toshiba.co.jp wrote:
> Hello Krzysztof,
> 
> Sorry, I missed your comments following the topic of recipients list.
> 
> Below are the rest of the responses
> 
>> -----Original Message-----
>> From: Krzysztof Kozlowski <krzk@kernel.org>
>> Sent: Wednesday, January 11, 2023 4:31 AM
>> To: ishikawa yuji(石川 悠司 ○ＲＤＣ□ＡＩＴＣ○ＥＡ開)
>> <yuji2.ishikawa@toshiba.co.jp>; Hans Verkuil <hverkuil@xs4all.nl>; Laurent
>> Pinchart <laurent.pinchart@ideasonboard.com>; Mauro Carvalho Chehab
>> <mchehab@kernel.org>; iwamatsu nobuhiro(岩松 信洋 □ＳＷＣ◯ＡＣＴ)
>> <nobuhiro1.iwamatsu@toshiba.co.jp>
>> Cc: linux-media@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
>> linux-kernel@vger.kernel.org; devicetree@vger.kernel.org
>> Subject: Re: [PATCH v4 1/6] dt-bindings: media: platform: visconti: Add Toshiba
>> Visconti Video Input Interface bindings
>>
>> On 10/01/2023 02:41, Yuji Ishikawa wrote:
>>> Adds the Device Tree binding documentation that allows to describe the
>>> Video Input Interface found in Toshiba Visconti SoCs.
>>>
>>> Signed-off-by: Yuji Ishikawa <yuji2.ishikawa@toshiba.co.jp>
>>> Reviewed-by: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
>>
>> 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 missed few of them, so clearly this was not sent correctly.
>>
>>
>> Subject: drop second/last, redundant "bindings". The "dt-bindings"
>> prefix is already stating that these are bindings.

Follow all comments.

>>
>>> ---
>>> Changelog v2:
>>> - no change
>>>
>>> Changelog v3:
>>> - no change
>>>
>>> Changelog v4:
>>> - fix style problems at the v3 patch
>>> - remove "index" member
>>> - update example
>>> ---
>>>  .../bindings/media/toshiba,visconti-viif.yaml | 98
>>> +++++++++++++++++++
>>>  1 file changed, 98 insertions(+)
>>>  create mode 100644
>>> Documentation/devicetree/bindings/media/toshiba,visconti-viif.yaml
>>>
>>> diff --git
>>> a/Documentation/devicetree/bindings/media/toshiba,visconti-viif.yaml
>>> b/Documentation/devicetree/bindings/media/toshiba,visconti-viif.yaml
>>> new file mode 100644
>>> index 00000000000..71442724d1a
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/media/toshiba,visconti-viif.ya
>>> +++ ml
>>> @@ -0,0 +1,98 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/media/toshiba,visconti-viif.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Toshiba Visconti5 SoC Video Input Interface Device Tree
>>> +Bindings
>>
>> Drop "Device Tree Bindings"
> 
> I'll drop it.
> 
>>
>>> +
>>> +maintainers:
>>> +  - Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
>>> +
>>> +description:
>>> +  Toshiba Visconti5 SoC Video Input Interface (VIIF)
>>> +  receives MIPI CSI2 video stream,
>>> +  processes the stream with embedded image signal processor (L1ISP,
>>> +L2ISP),
>>> +  then stores pictures to main memory.
>>
>> Fix wrapping.
> 
> I'll fix this.
> I didn't realize a new line has been added through formatting a patch.
>  
>>> +
>>> +properties:
>>> +  compatible:
>>> +    const: toshiba,visconti-viif
>>
>> Compatible must be specific. You called your SoC visconti5, didn't you?
>>
> 
> The Video Input Interface hardware is likely to be used at future SoCs of
> Visconti Architecture.
> Does compatible have to be specific to SoC's model name 
> rather than architecture name?

Compatibles should always be specific to SoC model name. Adding more
generic family fallback is also good idea when it is applicable.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/media/toshiba,visconti-viif.yaml b/Documentation/devicetree/bindings/media/toshiba,visconti-viif.yaml
new file mode 100644
index 00000000000..71442724d1a
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/toshiba,visconti-viif.yaml
@@ -0,0 +1,98 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/toshiba,visconti-viif.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Toshiba Visconti5 SoC Video Input Interface Device Tree Bindings
+
+maintainers:
+  - Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
+
+description:
+  Toshiba Visconti5 SoC Video Input Interface (VIIF)
+  receives MIPI CSI2 video stream,
+  processes the stream with embedded image signal processor (L1ISP, L2ISP),
+  then stores pictures to main memory.
+
+properties:
+  compatible:
+    const: toshiba,visconti-viif
+
+  reg:
+    items:
+      - description: registers for capture control
+      - description: registers for CSI2 receiver control
+
+  interrupts:
+    items:
+      - description: Sync Interrupt
+      - description: Status (Error) Interrupt
+      - description: CSI2 Receiver Interrupt
+      - description: L1ISP Interrupt
+
+  port:
+    $ref: /schemas/graph.yaml#/$defs/port-base
+    unevaluatedProperties: false
+    description: Input port, single endpoint describing the CSI-2 transmitter.
+
+    properties:
+      endpoint:
+        $ref: video-interfaces.yaml#
+        unevaluatedProperties: false
+
+        properties:
+          data-lanes:
+            description: VIIF supports 2 or 4 data lines
+            $ref: /schemas/types.yaml#/definitions/uint32-array
+            minItems: 1
+            maxItems: 4
+            items:
+              minimum: 1
+              maximum: 4
+
+          clock-lanes:
+            description: VIIF supports 1 clock line
+            const: 0
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - port
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        viif@1c000000 {
+            compatible = "toshiba,visconti-viif";
+            reg = <0 0x1c000000 0 0x6000>,
+                  <0 0x1c008000 0 0x400>;
+            interrupts = <GIC_SPI 64 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 73 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH>;
+
+            port {
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                csi_in0: endpoint {
+                    remote-endpoint = <&imx219_out0>;
+                    bus-type = <4>;
+                    data-lanes = <1 2>;
+                    clock-lanes = <0>;
+                    clock-noncontinuous;
+                    link-frequencies = /bits/ 64 <456000000>;
+                };
+            };
+        };
+    };