Linux-Media Archive mirror
 help / color / mirror / Atom feed
From: Geert Uytterhoeven <geert@linux-m68k.org>
To: Rob Herring <robh@kernel.org>, Conor Dooley <conor@kernel.org>
Cc: "Niklas Söderlund" <niklas.soderlund+renesas@ragnatech.se>,
	"Mauro Carvalho Chehab" <mchehab@kernel.org>,
	"Krzysztof Kozlowski" <krzysztof.kozlowski+dt@linaro.org>,
	"Conor Dooley" <conor+dt@kernel.org>,
	"Geert Uytterhoeven" <geert+renesas@glider.be>,
	linux-media@vger.kernel.org, devicetree@vger.kernel.org,
	linux-renesas-soc@vger.kernel.org
Subject: Re: [PATCH v2 1/2] dt-bindings: media: renesas,vin: Add binding for V4M
Date: Thu, 13 Jun 2024 21:35:13 +0200	[thread overview]
Message-ID: <CAMuHMdUQr0pzhL6Tq=R_TTUSu5wDZO-sWQHkuLg4C=xv9TyoWQ@mail.gmail.com> (raw)
In-Reply-To: <20240613165111.GA2005299-robh@kernel.org>

Hi Rob, Conor,

On Thu, Jun 13, 2024 at 6:51 PM Rob Herring <robh@kernel.org> wrote:
> On Tue, Jun 11, 2024 at 01:06:17PM +0200, Niklas Söderlund wrote:
> > On 2024-06-10 22:32:29 +0100, Conor Dooley wrote:
> > > On Mon, Jun 10, 2024 at 06:59:35PM +0200, Niklas Söderlund wrote:
> > > > On 2024-06-10 17:03:49 +0100, Conor Dooley wrote:
> > > > > On Mon, Jun 10, 2024 at 01:31:23PM +0200, Niklas Söderlund wrote:
> > > > > > Document support for the VIN module in the Renesas V4M (r8a779h0) SoC.
> > > > > >
> > > > > > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > > > > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > > > > > ---
> > > > > >  Documentation/devicetree/bindings/media/renesas,vin.yaml | 1 +
> > > > > >  1 file changed, 1 insertion(+)
> > > > > >
> > > > > > diff --git a/Documentation/devicetree/bindings/media/renesas,vin.yaml b/Documentation/devicetree/bindings/media/renesas,vin.yaml
> > > > > > index 5539d0f8e74d..168cb02f8abe 100644
> > > > > > --- a/Documentation/devicetree/bindings/media/renesas,vin.yaml
> > > > > > +++ b/Documentation/devicetree/bindings/media/renesas,vin.yaml
> > > > > > @@ -54,6 +54,7 @@ properties:
> > > > > >                - renesas,vin-r8a77995 # R-Car D3
> > > > > >                - renesas,vin-r8a779a0 # R-Car V3U
> > > > > >                - renesas,vin-r8a779g0 # R-Car V4H
> > > > > > +              - renesas,vin-r8a779h0 # R-Car V4M
> > > > >
> > > > > Your driver patch suggests that this is compatible with the g variant.
> > > >
> > > > Currently it is. But that not always be true, I tried to outline this in
> > > > to cover letter.
> > >
> > > To be honest, I don't usually read cover letters when reviewing bindings.
> > > Information about why things are/are not compatible should be in a
> > > commit itself.
> > >
> > > >     The V4M capture pipeline is similar to the other Gen4 SoC supported
> > > >     upstream already V4H. Currently all futures supported for VIN on V4M are
> > > >     also supported by V4H and the driver code can be shared. But as done for
> > > >     other R-Car IP bindings a new dedicated binding for V4M is created.
> > > >     This have proved prudent in the past where quirks are found even within
> > > >     the same generation as more advance use-cases are enabled.
> > >
> > > I don't understand how this precludes using the g variant as a fallback
> > > compatible. I'm not suggesting that you don't add a specific one for the
> > > h variant.
> >
> > The bindings have been around for a while and currently there are 25 SoC
> > specific compatibles, one for each SoC supported. Each compatible
> > consist of the SoC model number, not the VIN IP model/version number as
> > no such versioning schema exist.
> >
> > The datasheets are specific for each SoC and there are differences
> > between almost every SoC. There are of course lots of similarities
> > between the SoCs and the similarities are cluster around the 3
> > generations (Gen{2,3,4}) supported.
> >
> > Using the g variant as fallback in DTS for h variant even if we also add
> > a specific one for h is confusing. As g and h are two different SoC.
>
> Why? That is the very definition of how "compatible" is supposed to
> work.
>
> > The g variant is r8a779g0 which is the SoC name/number for V4H.
> > The h variant is r8a779h0 which is the SoC name/number for V4M.
> >
> > I think the core of the problem is that there are no versioning schema
> > for the individual IP blocks used on each SoC. For better or worse the
> > bindings for lots of Renesas IPs are centred around SoC name/number and
> > not the individual IP implementations.
>
> We've tried IP version based compatibles before. It doesn't work. Guess
> what, the IP version changes with nearly every SoC. Chip designers have
> no discipline.

The R-Car V4M capture pipeline is similar to e.g. the R-Car V4H capture
pipeline. But it is not identical, hence the different compatible values.
AFAIU, for the current feature-set, the driver does not need to handle
the differences.  But that may change later...

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-06-13 19:35 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-10 11:31 [PATCH v2 0/2] rcar-vin: Add support for R-Car V4M Niklas Söderlund
2024-06-10 11:31 ` [PATCH v2 1/2] dt-bindings: media: renesas,vin: Add binding for V4M Niklas Söderlund
2024-06-10 16:03   ` Conor Dooley
2024-06-10 16:59     ` Niklas Söderlund
2024-06-10 21:32       ` Conor Dooley
2024-06-11 11:06         ` Niklas Söderlund
2024-06-13 16:51           ` Rob Herring
2024-06-13 19:35             ` Geert Uytterhoeven [this message]
2024-06-18 13:57               ` Niklas Söderlund
2024-06-18 14:31                 ` Conor Dooley
2024-06-10 11:31 ` [PATCH v2 2/2] media: rcar-vin: Add support for R-Car V4M Niklas Söderlund
2024-06-10 12:14   ` Geert Uytterhoeven
2024-06-19  1:35   ` Laurent Pinchart

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='CAMuHMdUQr0pzhL6Tq=R_TTUSu5wDZO-sWQHkuLg4C=xv9TyoWQ@mail.gmail.com' \
    --to=geert@linux-m68k.org \
    --cc=conor+dt@kernel.org \
    --cc=conor@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=geert+renesas@glider.be \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-renesas-soc@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=niklas.soderlund+renesas@ragnatech.se \
    --cc=robh@kernel.org \
    /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).