Re: [PATCH v2 1/6] dt-bindings: media: renesas,isp: Add Gen4 family fallback

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

 



On Wed, Aug 28, 2024 at 4:46 PM Rob Herring <robh@xxxxxxxxxx> wrote:
> On Wed, Aug 28, 2024 at 07:36:35AM +0200, Krzysztof Kozlowski wrote:
> > On 27/08/2024 23:34, Laurent Pinchart wrote:
> > > On Tue, Aug 27, 2024 at 10:12:33AM +0200, Niklas Söderlund wrote:
> > >> On 2024-08-27 08:31:22 +0200, Krzysztof Kozlowski wrote:
> > >>> On Mon, Aug 26, 2024 at 04:43:47PM +0200, Niklas Söderlund wrote:
> > >>>> The ISP Channel Selector IP is the same for all current Gen4 devices.
> > >>>> This was not known when adding support for V3U and V4H and a single SoC
> > >>>> specific compatible was used.
> > >>>>
> > >>>> Before adding more SoC specific bindings for V4M add a family compatible
> > >>>> fallback for Gen4. That way the driver only needs to be updated once for
> > >>>> Gen4, and we still have the option to fix any problems in the driver if
> > >>>> any testable differences between the SoCs are found.
> > >>>>
> > >>>> There are already DTS files using the V3U and V4H compatibles which
> > >>>> needs to be updated to not produce a warning for DTS checks. The driver
> > >>>> also needs to kept the compatible values to be backward compatible , but
> > >>>> for new Gen4 SoCs such as V4M we can avoid this.
> > >>>>
> > >>>> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx>
> > >>>> ---
> > >>>> * Changes since v1
> > >>>> - New in v2.
> > >>>> ---
> > >>>>  Documentation/devicetree/bindings/media/renesas,isp.yaml | 3 ++-
> > >>>>  1 file changed, 2 insertions(+), 1 deletion(-)
> > >>>>
> > >>>> diff --git a/Documentation/devicetree/bindings/media/renesas,isp.yaml b/Documentation/devicetree/bindings/media/renesas,isp.yaml
> > >>>> index 33650a1ea034..730c86f2d7b1 100644
> > >>>> --- a/Documentation/devicetree/bindings/media/renesas,isp.yaml
> > >>>> +++ b/Documentation/devicetree/bindings/media/renesas,isp.yaml
> > >>>> @@ -22,6 +22,7 @@ properties:
> > >>>>        - enum:
> > >>>>            - renesas,r8a779a0-isp # V3U
> > >>>>            - renesas,r8a779g0-isp # V4H
> > >>>> +      - const: renesas,rcar-gen4-isp # Generic R-Car Gen4
> > >>>
> > >>> Adding generic fallback post-factum is odd, does not feel reliable.
> > >>> Instead use specific compatibles as fallbacks.
> > >>
> > >> I agree, it feels a bit odd. But this was the road we hammered out at
> > >> great pain for how to be able to move forward with this issue for the
> > >> other IP block involved in video capture for R-Car Gen4, VIN [1]. This
> > >> just mirrors that long discussion decision for the R-Car CSISP.
> > >>
> > >> I would hate to have different solutions for the two.
> > >>
> > >> 1. [PATCH v5 0/6] rcar-vin: Add support for R-Car V4M
> > >>    https://lore.kernel.org/all/20240704161620.1425409-1-niklas.soderlund+renesas@xxxxxxxxxxxx/
> > >
> > > The compatible fallback for VIN has been added following a request from
> > > Conor and Rob, so it would be nice if the three of you could agree to
> > > achieve consistency in the bindings :-)
> >
> > Don't twist our answers. You need fallback, but specific, not family.
> > There was a countless number of answers from Rob that specific
> > compatibles are preferred.
>
> Preferred, definitely. But preferred is not absolute. The Renesas
> bindings have consistently followed the above style for some time. For
> the most part that has worked out it seems (based on Geert's slides
> linked in one of the threads). If you want to continue that here, it's
> not something I care to argue about.
>
> However, I have to agree that adding the fallback after the fact is not
> ideal. Why design it where you have to carry renesas,r8a779g0-isp and
> renesas,rcar-gen4-isp in the driver forever when you could have 0 driver
> changes instead? The problem with genericish fallbacks is you have to
> know the future. Am I going to have a family of chips with the same
> block? It's much easier to just say "oh, this new chip is compatible
> with this old chip".

I agree it is not ideal, but it worked well for us.

R-Car Gen4 was a bit special, as its first member was named R-Car V3U,
and at that time it wasn't clear whether R-Car V3U was some sort
of one-off intermediate between Gen3 and Gen4, or the real thing.
In addition, the second Gen4 SoC (R-Car S4-8) is network-oriented,
and doesn't have any media support at all.  So we had to wait for
R-Car V4H and V4M to get a better picture of media commonalities.

Note that unlike on R-Car Gen2, R-Car Gen3 Video-In uses only
SoC-specific compatible values, as the VIN module on Gen3 had
SoC-specific routings, and a generic compatible value thus couldn't
work.  Hence for R-Car Gen4, we also started with only SoC-specific
compatible values, which turned out not needed (so far ;-) due to the
introduction of a new player in the pipeline (the ISP).

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