Linux-i2c Archive mirror
 help / color / mirror / Atom feed
From: Geert Uytterhoeven <geert@linux-m68k.org>
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>
Subject: Re: [PATCH 2/5] dt-bindings: i2c: renesas,riic: Document R9A09G057 support
Date: Mon, 11 Mar 2024 10:00:23 +0100	[thread overview]
Message-ID: <CAMuHMdVgp_vFnWr5ruzdyc1vNQKoCdM=pLZmgmkDjmUHvQBJJw@mail.gmail.com> (raw)
In-Reply-To: <2974085a-d9b4-4a66-b60f-c02a06a74647@linaro.org>

Hi Krzysztof,

On Sun, Mar 10, 2024 at 9:10 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
> On 10/03/2024 00:28, Lad, Prabhakar wrote:
> > On Sat, Mar 9, 2024 at 12:00 PM 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 SoC.
> >>> - 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 compatible
> >>> 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

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

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

  reply	other threads:[~2024-03-11  9:00 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-08 17:27 [PATCH 0/5] Add RIIC support for Renesas RZ/V2H SoC Prabhakar
2024-03-08 17:27 ` [PATCH 1/5] dt-bindings: i2c: renesas,riic: Update comment for fallback string Prabhakar
2024-03-09 11:58   ` Krzysztof Kozlowski
2024-03-09 23:05     ` Lad, Prabhakar
2024-03-19 21:19       ` Andi Shyti
2024-03-20  9:40         ` Krzysztof Kozlowski
2024-03-14 14:33   ` Geert Uytterhoeven
2024-03-08 17:27 ` [PATCH 2/5] dt-bindings: i2c: renesas,riic: Document R9A09G057 support Prabhakar
2024-03-09 12:00   ` Krzysztof Kozlowski
2024-03-09 23:28     ` Lad, Prabhakar
2024-03-10  8:10       ` Krzysztof Kozlowski
2024-03-11  9:00         ` Geert Uytterhoeven [this message]
2024-03-12 11:04           ` Krzysztof Kozlowski
2024-03-12 14:06             ` Geert Uytterhoeven
2024-03-12 15:05               ` Krzysztof Kozlowski
2024-03-10  8:05   ` Biju Das
2024-03-08 17:27 ` [PATCH 3/5] i2c: riic: Introduce helper functions for I2C read/write operations Prabhakar
2024-03-08 19:47   ` Geert Uytterhoeven
2024-03-08 21:00     ` Lad, Prabhakar
2024-03-08 17:27 ` [PATCH 4/5] i2c: riic: Pass register offsets and chip details as OF data Prabhakar
2024-03-08 17:36   ` Biju Das
2024-03-08 18:03     ` Lad, Prabhakar
2024-03-08 18:15       ` Biju Das
2024-03-08 20:59         ` Lad, Prabhakar
2024-03-08 17:27 ` [PATCH 5/5] i2c: riic: Add support for R9A09G057 SoC Prabhakar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAMuHMdVgp_vFnWr5ruzdyc1vNQKoCdM=pLZmgmkDjmUHvQBJJw@mail.gmail.com' \
    --to=geert@linux-m68k.org \
    --cc=andi.shyti@kernel.org \
    --cc=chris.brandt@renesas.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=fabrizio.castro.jz@renesas.com \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=krzysztof.kozlowski@linaro.org \
    --cc=linux-i2c@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-renesas-soc@vger.kernel.org \
    --cc=magnus.damm@gmail.com \
    --cc=prabhakar.csengg@gmail.com \
    --cc=prabhakar.mahadev-lad.rj@bp.renesas.com \
    --cc=robh+dt@kernel.org \
    --cc=wsa+renesas@sang-engineering.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).