On Wed, Aug 28, 2024 at 09:46:44AM -0500, Rob Herring 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". Yep, that's what I said pretty much. When I acked it I did so with the comment: | Same caveat here. Using the g model as a fallback is, as we already | discussed, an option too and would be less disruptive. | (at https://lore.kernel.org/all/20240626-unnatural-ember-26ae8895c008@spud/) But... > > Look, Conor's reply: > > > > https://lore.kernel.org/all/20240620-gating-coherent-af984389b2d7@spud/ > > Do you see family fallback? I think "r8a779g0" is SoC. > > > > Look here: > > https://lore.kernel.org/all/20240610-screen-wolverine-78370c66d40f@spud/ > > > > Or here > > https://lore.kernel.org/all/20240624-rented-danger-300652ab8eeb@wendy/ > > where Conor agrees against! > > But he ultimately Acked it. ...since Geert was happy enough with taking the modifications to existing devicetrees, and well aware of the pros/cons of each approach, I figured I had argued it enough and let it be.
Attachment:
signature.asc
Description: PGP signature
