Re: [PATCH v12 2/8] dt-bindings: media: platform: visconti: Add Toshiba Visconti Video Input Interface

On Tue, Dec 17, 2024 at 01:51:54PM +0100, Krzysztof Kozlowski wrote:

On 17/12/2024 10:45, Laurent Pinchart wrote:

quoted

On Tue, Dec 17, 2024 at 06:43:22AM +0100, Krzysztof Kozlowski wrote:

quoted

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

quoted

Hello Krzysztof

Thank you for your review

quoted

-----Original Message-----
From: Krzysztof Kozlowski <krzk@kernel.org>
Sent: Monday, November 25, 2024 7:08 PM
To: ishikawa yuji(石川 悠司 ○ＲＤＣ□ＡＩＴＣ○ＥＡ開)
[off-list ref]; Laurent Pinchart
[off-list ref]; Mauro Carvalho Chehab
[off-list ref]; Rob Herring [off-list ref]; Krzysztof Kozlowski
[off-list ref]; Conor Dooley [off-list ref]; Sakari Ailus
[off-list ref]; Hans Verkuil [off-list ref];
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 2/8] dt-bindings: media: platform: visconti: Add
Toshiba Visconti Video Input Interface

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

quoted

Adds the Device Tree binding documentation that allows to describe the
Video Input Interface found in Toshiba Visconti SoCs.

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

Why this tag stayed and other was removed? What was the reason of tag
removal?

The stayed tag is due to internal review.

Did the internal review really happened? How is it that immediately new
version has internal review without any traces?

I have doubts this review happened in the context of reviewer's
statement of oversight.

quoted

The removed tag is due to code's change (split of csi2rx part) after the last review.
If the code is largely changed following the instruction of another reviewer
after obtaining the tags, how should the tags be handled?

Drop all reviews and perform reviews on the list.

Such internal review appearing afterwards is rather a proof it you are
adding just the tags to satisfy your process. I have no way to even
verify whether that person performed any reasonable review or maybe just
acked your patch.

How do you verify that for public reviews ?

By quality or amount of comments. Or timing. Or reviewing cover letter
without any feedback on individual patches.

There are many, many ways. Considering how many companies were adding
fake manager-review-tags in the past (or fake SoBs), I am pretty picky
on that.

On the other hand I've heard numerous complains about patches being sent
by new developers from large companies to upstream lists without first
being reviewed internally by more experienced developers. You can't ask
people to review patches internally before submitting them upstream and
at the same time complain about R-b tags coming from internal reviews.

The value of a R-b tag, regardless of whether or not it comes from an
internal review, ultimately depends on the trust you have on the
reviewer. You can take that into account to decide when you consider a
patch to be ready to be merged, or to skip reviewing it if enough
reviewers you trust have looked at it first.

-- 
Regards,

Laurent Pinchart