RE: [PATCH v12 1/8] dt-bindings: media: platform: visconti: Add Toshiba Visconti MIPI CSI-2 Receiver

-----Original Message-----
From: Krzysztof Kozlowski <krzk@kernel.org>
Sent: Tuesday, December 17, 2024 2:45 PM
To: ishikawa yuji(石川 悠司 ○ＲＤＣ□ＡＩＴＣ○ＥＡ開)
[off-list ref]; laurent.pinchart@ideasonboard.com;
mchehab@kernel.org; robh@kernel.org; krzk+dt@kernel.org;
conor+dt@kernel.org; sakari.ailus@linux.intel.com; hverkuil-cisco@xs4all.nl;
iwamatsu nobuhiro(岩松 信洋 ○ＤＩＴＣ□ＤＩＴ○ＯＳＴ)
[off-list ref]
Cc: linux-media@vger.kernel.org; linux-kernel@vger.kernel.org;
linux-arm-kernel@lists.infradead.org; devicetree@vger.kernel.org
Subject: Re: [PATCH v12 1/8] dt-bindings: media: platform: visconti: Add
Toshiba Visconti MIPI CSI-2 Receiver

On 17/12/2024 00:57, yuji2.ishikawa@toshiba.co.jp wrote:

quoted

quoted

On 25/11/2024 10:21, Yuji Ishikawa wrote:

quoted

Adds the Device Tree binding documentation that allows to describe
the MIPI CSI-2 Receiver found in Toshiba Visconti SoCs.

Signed-off-by: Yuji Ishikawa <redacted>
Reviewed-by: Nobuhiro Iwamatsu <redacted>

How newly added patch can have already Rb tag? Was this review
really, really performed internally or you just satisfy some internal
managers requirements and fake the stats?

I added this Reviewed-by tag because the patch was reviewed internally.

What issues were identified by internal review, especially in the context of
bindings?

The review was insufficient. We discussed the splitting of drivers, but overlooked the schema.
I should have been more careful not to add an inappropriate tag.
I apologize for not understanding the attention and respect that should be given to the Reviewed-by tag.
From now on, I will add the tag to reviews discussed in the open mailing list.

quoted

quoted

quoted

---

Changelog v12:
- Newly add bindings for CSI2RX driver

 .../media/toshiba,visconti5-csi2rx.yaml       | 104

++++++++++++++++++

quoted

 1 file changed, 104 insertions(+)
 create mode 100644
Documentation/devicetree/bindings/media/toshiba,visconti5-csi2rx.yam
l

diff --git
a/Documentation/devicetree/bindings/media/toshiba,visconti5-csi2rx.y
am
l
b/Documentation/devicetree/bindings/media/toshiba,visconti5-csi2rx.y
am
l
new file mode 100644
index 000000000000..5488072bc82a

--- /dev/null
+++

b/Documentation/devicetree/bindings/media/toshiba,visconti5-csi2

quoted

quoted

quoted

+++ rx
+++ .yaml

@@ -0,0 +1,104 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML

1.2

quoted

quoted

quoted

+---
+$id:
+http://devicetree.org/schemas/media/toshiba,visconti5-csi2rx.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Toshiba Visconti5 SoC MIPI CSI-2 receiver
+
+maintainers:
+  - Nobuhiro Iwamatsu [off-list ref]
+
+description: |-

Drop |-

I'll drop "|-"

quoted

quoted

+  Toshiba Visconti5 SoC MIPI CSI-2 receiver device receives MIPI
+ CSI-2 video  stream. Use with VIIF device. T.B.D
+
+properties:
+  compatible:
+    const: toshiba,visconti5-csi2rx

Why this is called "RX"? Can you have a TX? I had impression that one

cannot.

quoted

quoted

VIIF has only MIPI CSI2 receiver (CSI2RX). There's no TX for it.

So this device cannot be anything else? Then drop rx.

I'll drop "rx". The new compatible string will be "toshiba,visconti5-csi2".
I'll also update the name of bindings document file.

quoted

Visconti also has VOIF (Video Output Interface) hardware which has MIPI

CSI2 (not DSI) transmitter (CSI2TX).

Or this can be something else? Confusing.

VOIF has only TX. There's no RX for it.
I just wanted to inform you that there is a node with similar name and function.
Please forget about it.

Best regards,
Krzysztof

Best regards,
Yuji Ishikawa