Received-SPF: pass (google.com: domain of linux-kernel+bounces-98572-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240308172726.225357-1-prabhakar.mahadev-lad.rj@bp.renesas.com>
 <20240308172726.225357-3-prabhakar.mahadev-lad.rj@bp.renesas.com>
 <7082ed3b-d6d0-4228-b7a6-7c0e0e46b8e2@linaro.org> <CA+V-a8tM29h10DULurMJtBZBnLK_ZF7pH_Y0bhZTvWO0O7-G-Q@mail.gmail.com>
 <2974085a-d9b4-4a66-b60f-c02a06a74647@linaro.org>
In-Reply-To: <2974085a-d9b4-4a66-b60f-c02a06a74647@linaro.org>
From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: Mon, 11 Mar 2024 10:00:23 +0100
Message-ID: <CAMuHMdVgp_vFnWr5ruzdyc1vNQKoCdM=pLZmgmkDjmUHvQBJJw@mail.gmail.com>
Subject: Re: [PATCH 2/5] dt-bindings: i2c: renesas,riic: Document R9A09G057 support
To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Cc: "Lad, Prabhakar" <prabhakar.csengg@gmail.com>, Chris Brandt <chris.brandt@renesas.com>, 
	Andi Shyti <andi.shyti@kernel.org>, Rob Herring <robh+dt@kernel.org>, 
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>, Conor Dooley <conor+dt@kernel.org>, 
	Magnus Damm <magnus.damm@gmail.com>, Wolfram Sang <wsa+renesas@sang-engineering.com>, 
	linux-renesas-soc@vger.kernel.org, linux-i2c@vger.kernel.org, 
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, 
	Fabrizio Castro <fabrizio.castro.jz@renesas.com>, 
	Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi Krzysztof,

On Sun, Mar 10, 2024 at 9:10=E2=80=AFAM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
> On 10/03/2024 00:28, Lad, Prabhakar wrote:
> > On Sat, Mar 9, 2024 at 12:00=E2=80=AFPM 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>
> >>>
> >>> Document support for the I2C Bus Interface (RIIC) available in the
> >>> Renesas RZ/V2H (R9A09G057) SoC.
> >>>
> >>> The RIIC interface in the Renesas RZ/V2H differs from RZ/A in a
> >>> couple of ways:
> >>> - Register offsets for the RZ/V2H SoC differ from those of the RZ/A S=
oC.
> >>> - RZ/V2H register access is 8-bit, whereas RZ/A supports 8/16/32-bit.
> >>> - RZ/V2H has some bit differences in the slave address register.
> >>>
> >>> To accommodate these differences in the existing driver, a new compat=
ible
> >>> string "renesas,riic-r9a09g057" is added.
> >>>
> >>> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com=
>
> >>> Reviewed-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>

> >>> --- a/Documentation/devicetree/bindings/i2c/renesas,riic.yaml
> >>> +++ b/Documentation/devicetree/bindings/i2c/renesas,riic.yaml
> >>> @@ -15,14 +15,19 @@ allOf:
> >>>
> >>>  properties:
> >>>    compatible:
> >>> -    items:
> >>> -      - enum:
> >>> -          - renesas,riic-r7s72100   # RZ/A1H
> >>> -          - renesas,riic-r7s9210    # RZ/A2M
> >>> -          - renesas,riic-r9a07g043  # RZ/G2UL and RZ/Five
> >>> -          - renesas,riic-r9a07g044  # RZ/G2{L,LC}
> >>> -          - renesas,riic-r9a07g054  # RZ/V2L
> >>> -      - const: renesas,riic-rz      # generic RIIC compatible
> >>> +    oneOf:
> >>> +      - items:
> >>> +          - enum:
> >>> +              - renesas,riic-r7s72100   # RZ/A1H
> >>> +              - renesas,riic-r7s9210    # RZ/A2M
> >>> +              - renesas,riic-r9a07g043  # RZ/G2UL and RZ/Five
> >>> +              - renesas,riic-r9a07g044  # RZ/G2{L,LC}
> >>> +              - renesas,riic-r9a07g054  # RZ/V2L
> >>> +          - const: renesas,riic-rz      # generic RIIC compatible
> >>> +
> >>> +      - items:
> >>> +          - enum:
> >>> +              - renesas,riic-r9a09g057  # RZ/V2H(P)
> >>
> >> No, that does not look right. If you added generic compatible for all
> >> RIIC then how can you add a new RIIC compatible which does not follow
> >> generic one?
> >>
> > The generic compatible above which was added previously was for the
> > RZ/(A) SoCs and not for all the RIICs. The RZ/G2L family was also
>
> No, it said: "generic RIIC compatible". It did not say "RIIC RZ/A". It
> said RIIC RZ

At the time the original bindings were written, only RZ/A1, RZ/T1,
and RZ/N1 existed, and all RIIC modules present in these SoCs were
identical.  Later, we got RZ/A2, which also included a compatible
RIIC block.

Somewhere along the timeline, the marketing department became creative,
and we got RZ/G1 (RZ/G1[HMNEC]) and RZ/G2 (RZ/G2[HMNE]), which were
unrelated to earlier RZ series :-(  When marketing started smoking
something different, we got RZ/G2L, which is unrelated to RZ/G2,
but reuses some parts from RZ/A.  Recently, we got RZ/G3S, which is
similar to RZ/G2L...

> > compatible hence they fallback to the generic RZ one.
>
> riic-r9a09g057 is also RIIC RZ, isn't it?

Yes, as in "it comes from the division that calls its products
RZ/something". But...

> >> This shows the ridiculousness of these generic compatibles. They are
> >> generic till you figure out the truth: oh crap, it's not generic.
> >>
> > Sorry I lack skills to predict the future of upcoming IP blocks which
> > fit in the SoC.
>
> So don't use generic compatibles as fallbacks. That's the point.

It's indeed difficult to predict the future. So SoC-specific compatible
values are safer.
At the same time, we want to avoid having to add compatible values for
each and every SoC to each driver, so we try to group SoCs per family.
For R-Car that worked out reasonably well, however, for RZ...

Gr{oetje,eeting}s,

                        Geert

--=20
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k=
org

In personal conversations with technical people, I call myself a hacker. Bu=
t
when I'm talking to journalists I just say "programmer" or something like t=
hat.
                                -- Linus Torvalds