From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Yuji Ishikawa <yuji2.ishikawa@toshiba.co.jp>
Cc: Hans Verkuil <hverkuil@xs4all.nl>,
Mauro Carvalho Chehab <mchehab@kernel.org>,
Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>,
Rob Herring <robh+dt@kernel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
"Rafael J . Wysocki" <rafael.j.wysocki@intel.com>,
Mark Brown <broonie@kernel.org>,
linux-media@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org
Subject: Re: [PATCH v5 1/6] dt-bindings: media: platform: visconti: Add Toshiba Visconti Video Input Interface bindings
Date: Tue, 17 Jan 2023 17:26:22 +0200 [thread overview]
Message-ID: <Y8a+Hk2jFOjbkIvZ@pendragon.ideasonboard.com> (raw)
In-Reply-To: <20230111022433.25950-2-yuji2.ishikawa@toshiba.co.jp>
Hi Yuji,
Thank you for the patch.
On Wed, Jan 11, 2023 at 11:24:28AM +0900, 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>
> ---
> Changelog v2:
> - no change
>
> Changelog v3:
> - no change
>
> Changelog v4:
> - fix style problems at the v3 patch
> - remove "index" member
> - update example
>
> Changelog v5:
> - no change
> ---
> .../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.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
Nitpicking, s/registers/Registers/ in the two lines above as you
capitalize the descriptions below.
> +
> + 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.
I would write
description:
CSI-2 input port, with a single endpoint connected to the CSI-2
transmitter.
> +
> + properties:
> + endpoint:
> + $ref: video-interfaces.yaml#
> + unevaluatedProperties: false
> +
> + properties:
> + data-lanes:
> + description: VIIF supports 2 or 4 data lines
s/lines/lanes/
> + $ref: /schemas/types.yaml#/definitions/uint32-array
You can drop this line, it's already handled by video-interfaces.yaml.
> + minItems: 1
> + maxItems: 4
If only 2 or 4 data lanes are supported, shouldn't minItems be 2 ?
> + items:
> + minimum: 1
> + maximum: 4
Can the CSI-2 receiver reorder the data lanes ? If not, I think you can
write
items:
- const: 1
- const: 2
- const: 3
- const: 4
> +
> + clock-lanes:
> + description: VIIF supports 1 clock line
s/line/lane/
> + const: 0
I would also add
clock-noncontinuous: true
link-frequencies: true
to indicate that the above two properties are used by this device.
Also, mark the properties that are required:
required:
- data-lanes
- clock-lanes
I'm wondering, though, if clock-lanes shouldn't be simply omitted. If
the hardware doesn't support any other option than using lane 0 for the
clock lane (as in, no lane remapping), then you can drop the clock-lanes
property completely.
> +
> +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>;
Does the hardware support any other bus type ? If not, you can drop the
bus-type. If it does, bus-type should be added to the binding, with the
value set to "const: 4".
> + data-lanes = <1 2>;
> + clock-lanes = <0>;
> + clock-noncontinuous;
> + link-frequencies = /bits/ 64 <456000000>;
> + };
> + };
> + };
> + };
--
Regards,
Laurent Pinchart
next prev parent reply other threads:[~2023-01-17 15:26 UTC|newest]
Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-11 2:24 [PATCH v5 0/6] Add Toshiba Visconti Video Input Interface driver Yuji Ishikawa
2023-01-11 2:24 ` [PATCH v5 1/6] dt-bindings: media: platform: visconti: Add Toshiba Visconti Video Input Interface bindings Yuji Ishikawa
2023-01-11 9:19 ` Krzysztof Kozlowski
2023-01-11 12:48 ` yuji2.ishikawa
2023-01-17 15:26 ` Laurent Pinchart [this message]
2023-01-17 15:42 ` Krzysztof Kozlowski
2023-01-17 15:58 ` Laurent Pinchart
2023-01-17 17:01 ` Krzysztof Kozlowski
2023-01-22 19:25 ` Laurent Pinchart
2023-01-30 9:06 ` yuji2.ishikawa
2023-02-01 9:45 ` Laurent Pinchart
2023-02-01 11:24 ` yuji2.ishikawa
2023-01-11 2:24 ` [PATCH v5 2/6] media: platform: visconti: Add Toshiba Visconti Video Input Interface driver Yuji Ishikawa
2023-01-11 15:30 ` kernel test robot
2023-01-11 22:55 ` kernel test robot
2023-01-17 10:01 ` Hans Verkuil
2023-01-25 12:12 ` yuji2.ishikawa
2023-01-17 22:39 ` Sakari Ailus
2023-02-01 2:02 ` yuji2.ishikawa
2023-02-01 9:41 ` Laurent Pinchart
2023-02-01 11:22 ` yuji2.ishikawa
2023-01-18 0:52 ` Laurent Pinchart
2023-02-02 4:37 ` yuji2.ishikawa
2023-01-11 2:24 ` [PATCH v5 3/6] media: platform: visconti: Add Toshiba Visconti Video Input Interface driver user interace Yuji Ishikawa
2023-01-17 11:47 ` Hans Verkuil
2023-01-18 1:04 ` Laurent Pinchart
2023-01-25 10:20 ` yuji2.ishikawa
2023-01-26 20:49 ` Laurent Pinchart
2023-02-02 4:52 ` yuji2.ishikawa
2023-02-02 7:58 ` Laurent Pinchart
2023-01-26 1:25 ` yuji2.ishikawa
2023-01-26 8:01 ` Hans Verkuil
2023-01-27 12:47 ` yuji2.ishikawa
2023-01-11 2:24 ` [PATCH v5 4/6] media: platform: visconti: Add Toshiba Visconti Video Input Interface driver v4l2 controls handler Yuji Ishikawa
2023-01-17 11:19 ` Hans Verkuil
2023-01-26 0:38 ` yuji2.ishikawa
2023-01-26 8:39 ` Hans Verkuil
2023-01-26 11:35 ` Laurent Pinchart
2023-02-03 1:35 ` yuji2.ishikawa
2023-02-02 12:42 ` yuji2.ishikawa
2023-01-11 2:24 ` [PATCH v5 5/6] documentation: media: add documentation for Toshiba Visconti Video Input Interface driver Yuji Ishikawa
2023-01-11 2:24 ` [PATCH v5 6/6] MAINTAINERS: Add entries for Toshiba Visconti Video Input Interface Yuji Ishikawa
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Y8a+Hk2jFOjbkIvZ@pendragon.ideasonboard.com \
--to=laurent.pinchart@ideasonboard.com \
--cc=broonie@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=hverkuil@xs4all.nl \
--cc=krzysztof.kozlowski+dt@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=mchehab@kernel.org \
--cc=nobuhiro1.iwamatsu@toshiba.co.jp \
--cc=rafael.j.wysocki@intel.com \
--cc=robh+dt@kernel.org \
--cc=yuji2.ishikawa@toshiba.co.jp \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).