diff mbox series

[1/5] dt-bindings: i2c: renesas,riic: Update comment for fallback string

Message ID 20240308172726.225357-2-prabhakar.mahadev-lad.rj@bp.renesas.com (mailing list archive)
State Superseded
Delegated to: Geert Uytterhoeven
Headers show
Series Add RIIC support for Renesas RZ/V2H SoC | expand

Commit Message

Lad, Prabhakar March 8, 2024, 5:27 p.m. UTC 
From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

With the fallback string being utilized by multiple other SoCs, this
patch updates the comment for the generic compatible string.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Reviewed-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
---
 Documentation/devicetree/bindings/i2c/renesas,riic.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Krzysztof Kozlowski March 9, 2024, 11:58 a.m. UTC | #1 
On 08/03/2024 18:27, Prabhakar wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
> With the fallback string being utilized by multiple other SoCs, this
> patch updates the comment for the generic compatible string.
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Reviewed-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>

Really, you review a comment change? Internally?

Is this some sort of company policy? Are these even true reviews?

> ---
>  Documentation/devicetree/bindings/i2c/renesas,riic.yaml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/renesas,riic.yaml b/Documentation/devicetree/bindings/i2c/renesas,riic.yaml
> index 2291a7cd619b..63ac5fe3208d 100644
> --- a/Documentation/devicetree/bindings/i2c/renesas,riic.yaml
> +++ b/Documentation/devicetree/bindings/i2c/renesas,riic.yaml
> @@ -22,7 +22,7 @@ properties:
>            - renesas,riic-r9a07g043  # RZ/G2UL and RZ/Five
>            - renesas,riic-r9a07g044  # RZ/G2{L,LC}
>            - renesas,riic-r9a07g054  # RZ/V2L
> -      - const: renesas,riic-rz      # RZ/A or RZ/G2L
> +      - const: renesas,riic-rz      # generic RIIC compatible

Just drop the comment instead.

Best regards,
Krzysztof
Lad, Prabhakar March 9, 2024, 11:05 p.m. UTC | #2 
Hi Krzysztof,

Thank you for the review.

On Sat, Mar 9, 2024 at 11:58 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 08/03/2024 18:27, Prabhakar wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > With the fallback string being utilized by multiple other SoCs, this
> > patch updates the comment for the generic compatible string.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > Reviewed-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
>
> Really, you review a comment change? Internally?
>
> Is this some sort of company policy? Are these even true reviews?
>
Yes this patch was reviewed internally and it's "real". Unfortunately
I cannot share the repo externally where this review was done but I
can assure it was reviewed. As this is not a single patch all the
patches in this series were internally reviewed. Is it bad to review a
comment change?
BTW what makes you think I have added fake review tags?

Is there any guideline you can point me to that states what needs to
be done when the code has been internally reviewed please. I'll make
sure I'll follow it.

> > ---
> >  Documentation/devicetree/bindings/i2c/renesas,riic.yaml | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/i2c/renesas,riic.yaml b/Documentation/devicetree/bindings/i2c/renesas,riic.yaml
> > index 2291a7cd619b..63ac5fe3208d 100644
> > --- a/Documentation/devicetree/bindings/i2c/renesas,riic.yaml
> > +++ b/Documentation/devicetree/bindings/i2c/renesas,riic.yaml
> > @@ -22,7 +22,7 @@ properties:
> >            - renesas,riic-r9a07g043  # RZ/G2UL and RZ/Five
> >            - renesas,riic-r9a07g044  # RZ/G2{L,LC}
> >            - renesas,riic-r9a07g054  # RZ/V2L
> > -      - const: renesas,riic-rz      # RZ/A or RZ/G2L
> > +      - const: renesas,riic-rz      # generic RIIC compatible
>
> Just drop the comment instead.
>
Ok, I will drop it.

Cheers,
Prabhakar
Geert Uytterhoeven March 14, 2024, 2:33 p.m. UTC | #3 
Hi Prabhakar,

On Fri, Mar 8, 2024 at 6:28 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> With the fallback string being utilized by multiple other SoCs, this
> patch updates the comment for the generic compatible string.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Reviewed-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>

Thanks for your patch!

> --- a/Documentation/devicetree/bindings/i2c/renesas,riic.yaml
> +++ b/Documentation/devicetree/bindings/i2c/renesas,riic.yaml
> @@ -22,7 +22,7 @@ properties:
>            - renesas,riic-r9a07g043  # RZ/G2UL and RZ/Five
>            - renesas,riic-r9a07g044  # RZ/G2{L,LC}
>            - renesas,riic-r9a07g054  # RZ/V2L
> -      - const: renesas,riic-rz      # RZ/A or RZ/G2L
> +      - const: renesas,riic-rz      # generic RIIC compatible

Please drop this patch, as this is not a truly generic RIIC compatible,
but applies to a subset of the RZ series only.

Gr{oetje,eeting}s,

                        Geert
Andi Shyti March 19, 2024, 9:19 p.m. UTC | #4 
Hi Prabhakar,

On Sat, Mar 09, 2024 at 11:05:40PM +0000, Lad, Prabhakar wrote:
> On Sat, Mar 9, 2024 at 11:58 AM Krzysztof Kozlowski
> > On 08/03/2024 18:27, Prabhakar wrote:
> > > With the fallback string being utilized by multiple other SoCs, this
> > > patch updates the comment for the generic compatible string.
> > >
> > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > Reviewed-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
> >
> > Really, you review a comment change? Internally?
> >
> > Is this some sort of company policy? Are these even true reviews?
> >
> Yes this patch was reviewed internally and it's "real". Unfortunately
> I cannot share the repo externally where this review was done but I
> can assure it was reviewed. As this is not a single patch all the
> patches in this series were internally reviewed. Is it bad to review a
> comment change?
> BTW what makes you think I have added fake review tags?

I don't believe Krzysztof is questioning the validity of your
offline reviews, but the community is unaware of what happens
in your closed environment.

If you submit a patch with the r-b tag, it holds little
significance for me since I haven't witnessed the review process
myself. However, you are, of course, free to include it; I have
no objections to that.

My suggestion is for Fabrizio to publicly express his review on
this mailing list, which would add more value to the time he
spent reviewing your patch.

By the way, there are other companies that do this.

Andi
Krzysztof Kozlowski March 20, 2024, 9:40 a.m. UTC | #5 
On 19/03/2024 22:19, Andi Shyti wrote:
> Hi Prabhakar,
> 
> On Sat, Mar 09, 2024 at 11:05:40PM +0000, Lad, Prabhakar wrote:
>> On Sat, Mar 9, 2024 at 11:58 AM Krzysztof Kozlowski
>>> On 08/03/2024 18:27, Prabhakar wrote:
>>>> With the fallback string being utilized by multiple other SoCs, this
>>>> patch updates the comment for the generic compatible string.
>>>>
>>>> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>>>> Reviewed-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
>>>
>>> Really, you review a comment change? Internally?
>>>
>>> Is this some sort of company policy? Are these even true reviews?
>>>
>> Yes this patch was reviewed internally and it's "real". Unfortunately
>> I cannot share the repo externally where this review was done but I
>> can assure it was reviewed. As this is not a single patch all the
>> patches in this series were internally reviewed. Is it bad to review a
>> comment change?
>> BTW what makes you think I have added fake review tags?
> 
> I don't believe Krzysztof is questioning the validity of your
> offline reviews, but the community is unaware of what happens
> in your closed environment.
> 
> If you submit a patch with the r-b tag, it holds little
> significance for me since I haven't witnessed the review process
> myself. However, you are, of course, free to include it; I have
> no objections to that.
> 
> My suggestion is for Fabrizio to publicly express his review on
> this mailing list, which would add more value to the time he
> spent reviewing your patch.
> 
> By the way, there are other companies that do this.
> 

To me seeing such reviews of a trivial comment patch means reviews are
fake, just to fulfill the process. Especially done internally. Kind of
"patchset looks good, so +1 in Gerrit" (it does not matter if you use
Gerrit or not...). I don't consider them reviews, but useless company
policies. Provide real review or do not provide one at all. And provide
it public, so work with the community, not your inside systems.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/i2c/renesas,riic.yaml b/Documentation/devicetree/bindings/i2c/renesas,riic.yaml
index 2291a7cd619b..63ac5fe3208d 100644
--- a/Documentation/devicetree/bindings/i2c/renesas,riic.yaml
+++ b/Documentation/devicetree/bindings/i2c/renesas,riic.yaml
@@ -22,7 +22,7 @@  properties:
           - renesas,riic-r9a07g043  # RZ/G2UL and RZ/Five
           - renesas,riic-r9a07g044  # RZ/G2{L,LC}
           - renesas,riic-r9a07g054  # RZ/V2L
-      - const: renesas,riic-rz      # RZ/A or RZ/G2L
+      - const: renesas,riic-rz      # generic RIIC compatible
 
   reg:
     maxItems: 1