Re: [PATCH v2 1/2] dt-bindings: media: renesas,vin: Add binding for V4M

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Rob, Conor,

On Thu, Jun 13, 2024 at 6:51 PM Rob Herring <robh@xxxxxxxxxx> 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@xxxxxxxxxxxx>
> > > > > > Reviewed-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
> > > > > > ---
> > > > > >  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@xxxxxxxxxxxxxx

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





[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux