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

Message ID 20240610113124.2396688-2-niklas.soderlund+renesas@ragnatech.se (mailing list archive)
State New
Headers
Series rcar-vin: Add support for R-Car V4M |

Commit Message

Niklas Söderlund June 10, 2024, 11:31 a.m. UTC
  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(+)
  

Comments

Conor Dooley June 10, 2024, 4:03 p.m. UTC | #1
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.
  
Niklas Söderlund June 10, 2024, 4:59 p.m. UTC | #2
Hi Conor,

Thanks for your feedback.

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.

    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.
  
Conor Dooley June 10, 2024, 9:32 p.m. UTC | #3
On Mon, Jun 10, 2024 at 06:59:35PM +0200, Niklas Söderlund wrote:
> Hi Conor,
> 
> Thanks for your feedback.
> 
> 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.

Thanks,
Conor.
  
Niklas Söderlund June 11, 2024, 11:06 a.m. UTC | #4
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:
> > Hi Conor,
> > 
> > Thanks for your feedback.
> > 
> > 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.

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.
  
Rob Herring June 13, 2024, 4:51 p.m. UTC | #5
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:
> > > Hi Conor,
> > > 
> > > Thanks for your feedback.
> > > 
> > > 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.

Rob
  
Geert Uytterhoeven June 13, 2024, 7:35 p.m. UTC | #6
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
  

Patch

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
 
   reg:
     maxItems: 1