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

[Krzysztof Kozlowski <krzk@kernel.org>]

On 28/08/2024 12:50, Laurent Pinchart 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@ragnatech.se>
>>>>>> ---
>>>>>> * 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@ragnatech.se/
>>>
>>> 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.
>>
>> 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!
>>
>> So let me actually NAK it - you got multiple comments on VIN to use
>> specific compatible.
> 
> Krzysztof, this tone is not acceptable, regardless of the technical
> argument. Period.

Except elevated arguments I don't think the tone is not acceptable.

Anyway, please provide references supporting your statement that Conor
and Rob encouraged using generic (not specific) fallback compatible.

I provided what I found, so I keep the discussion based on facts. I
expect the same from you.


Best regards,
Krzysztof