On Tue, Dec 17, 2024 at 01:51:54PM +0100, Krzysztof Kozlowski wrote: > On 17/12/2024 10:45, Laurent Pinchart wrote: > > On Tue, Dec 17, 2024 at 06:43:22AM +0100, Krzysztof Kozlowski wrote: > >> On 17/12/2024 01:00, yuji2.ishikawa@xxxxxxxxxxxxx wrote: > >>> Hello Krzysztof > >>> > >>> Thank you for your review > >>> > >>>> -----Original Message----- > >>>> From: Krzysztof Kozlowski <krzk@xxxxxxxxxx> > >>>> Sent: Monday, November 25, 2024 7:08 PM > >>>> To: ishikawa yuji(石川 悠司 ○ＲＤＣ□ＡＩＴＣ○ＥＡ開) > >>>> <yuji2.ishikawa@xxxxxxxxxxxxx>; Laurent Pinchart > >>>> <laurent.pinchart@xxxxxxxxxxxxxxxx>; Mauro Carvalho Chehab > >>>> <mchehab@xxxxxxxxxx>; Rob Herring <robh@xxxxxxxxxx>; Krzysztof Kozlowski > >>>> <krzk+dt@xxxxxxxxxx>; Conor Dooley <conor+dt@xxxxxxxxxx>; Sakari Ailus > >>>> <sakari.ailus@xxxxxxxxxxxxxxx>; Hans Verkuil <hverkuil-cisco@xxxxxxxxx>; > >>>> iwamatsu nobuhiro(岩松 信洋 ○ＤＩＴＣ□ＤＩＴ○ＯＳＴ) > >>>> <nobuhiro1.iwamatsu@xxxxxxxxxxxxx> > >>>> Cc: linux-media@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; > >>>> linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx > >>>> 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: > >>>>> 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@xxxxxxxxxxxxx> > >>>>> Reviewed-by: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@xxxxxxxxxxxxx> > >>>> > >>>> 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. > >> > >>> 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
