diff mbox series

[v5,1/6] dt-bindings: media: renesas,vin: Add Gen4 family fallback

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

Commit Message

Niklas Söderlund July 4, 2024, 4:16 p.m. UTC 
  The Gen4 SoCs V3U, V4H and V4M have a similar video capture pipelines.

The datasheets for the SoCs have small nuances around the Pre-Clip
registers ELPrC and EPPrC in three use-cases, interlaced images,
embedded data and RAW8 input. On V4H the values written to the registers
are based on odd numbers while on V4M they are even numbers, values are
based on the input image size. No board that uses these SoCs which also
have the external peripherals to test these nuances exists. Most likely
this is an issue in the datasheet.

Before adding 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>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
* Changes since v4
- Added V3U to the list of Gen4 devices.

* Changes since v3
- New in v4.
---
 Documentation/devicetree/bindings/media/renesas,vin.yaml | 3 +++
 1 file changed, 3 insertions(+)

Comments

Laurent Pinchart Aug. 23, 2024, 1:37 p.m. UTC | #1 
Hi Niklas,

Thank you for the patch.

On Thu, Jul 04, 2024 at 06:16:15PM +0200, Niklas Söderlund wrote:
> The Gen4 SoCs V3U, V4H and V4M have a similar video capture pipelines.
> 
> The datasheets for the SoCs have small nuances around the Pre-Clip
> registers ELPrC and EPPrC in three use-cases, interlaced images,
> embedded data and RAW8 input. On V4H the values written to the registers
> are based on odd numbers while on V4M they are even numbers, values are
> based on the input image size. No board that uses these SoCs which also
> have the external peripherals to test these nuances exists. Most likely
> this is an issue in the datasheet.
> 
> Before adding 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

s/needs/need/

> also needs to kept the compatible values to be backward compatible , but

s/kept/keep/

> for new Gen4 SoCs such as V4M we can avoid this.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> Acked-by: Conor Dooley <conor.dooley@microchip.com>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> ---
> * Changes since v4
> - Added V3U to the list of Gen4 devices.
> 
> * Changes since v3
> - New in v4.
> ---
>  Documentation/devicetree/bindings/media/renesas,vin.yaml | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/renesas,vin.yaml b/Documentation/devicetree/bindings/media/renesas,vin.yaml
> index 5539d0f8e74d..aa16f1528f2d 100644
> --- a/Documentation/devicetree/bindings/media/renesas,vin.yaml
> +++ b/Documentation/devicetree/bindings/media/renesas,vin.yaml
> @@ -52,8 +52,11 @@ properties:
>                - renesas,vin-r8a77980 # R-Car V3H
>                - renesas,vin-r8a77990 # R-Car E3
>                - renesas,vin-r8a77995 # R-Car D3
> +      - items:
> +          - enum:
>                - renesas,vin-r8a779a0 # R-Car V3U
>                - renesas,vin-r8a779g0 # R-Car V4H
> +          - const: renesas,rcar-gen4-vin # Generic R-Car Gen4
>  
>    reg:
>      maxItems: 1
Krzysztof Kozlowski Aug. 28, 2024, 11:26 a.m. UTC | #2 
On 04/07/2024 18:16, Niklas Söderlund wrote:
> The Gen4 SoCs V3U, V4H and V4M have a similar video capture pipelines.
> 
> The datasheets for the SoCs have small nuances around the Pre-Clip
> registers ELPrC and EPPrC in three use-cases, interlaced images,
> embedded data and RAW8 input. On V4H the values written to the registers
> are based on odd numbers while on V4M they are even numbers, values are
> based on the input image size. No board that uses these SoCs which also
> have the external peripherals to test these nuances exists. Most likely
> this is an issue in the datasheet.
> 
> Before adding 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>
> Acked-by: Conor Dooley <conor.dooley@microchip.com>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> * Changes since v4
> - Added V3U to the list of Gen4 devices.
> 
> * Changes since v3
> - New in v4.
> ---
>  Documentation/devicetree/bindings/media/renesas,vin.yaml | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/renesas,vin.yaml b/Documentation/devicetree/bindings/media/renesas,vin.yaml
> index 5539d0f8e74d..aa16f1528f2d 100644
> --- a/Documentation/devicetree/bindings/media/renesas,vin.yaml
> +++ b/Documentation/devicetree/bindings/media/renesas,vin.yaml
> @@ -52,8 +52,11 @@ properties:
>                - renesas,vin-r8a77980 # R-Car V3H
>                - renesas,vin-r8a77990 # R-Car E3
>                - renesas,vin-r8a77995 # R-Car D3
> +      - items:
> +          - enum:
>                - renesas,vin-r8a779a0 # R-Car V3U
>                - renesas,vin-r8a779g0 # R-Car V4H
> +          - const: renesas,rcar-gen4-vin # Generic R-Car Gen4

I believe it was requested in earlier comments to use SoC specific
compatible as fallback.

See:
https://lore.kernel.org/all/20240624-rented-danger-300652ab8eeb@wendy/
https://lore.kernel.org/all/20240610-screen-wolverine-78370c66d40f@spud/
https://lore.kernel.org/all/20240620-gating-coherent-af984389b2d7@spud/

You ignored these suggestions and decided to go with other approach.

I don't agree with this.

Please carry:

Nacked-by: Krzysztof Kozlowski <krzk@kernel.org>

so my objection and your avoidance of earlier suggestions will be
clearly documented.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/media/renesas,vin.yaml b/Documentation/devicetree/bindings/media/renesas,vin.yaml
index 5539d0f8e74d..aa16f1528f2d 100644
--- a/Documentation/devicetree/bindings/media/renesas,vin.yaml
+++ b/Documentation/devicetree/bindings/media/renesas,vin.yaml
@@ -52,8 +52,11 @@  properties:
               - renesas,vin-r8a77980 # R-Car V3H
               - renesas,vin-r8a77990 # R-Car E3
               - renesas,vin-r8a77995 # R-Car D3
+      - items:
+          - enum:
               - renesas,vin-r8a779a0 # R-Car V3U
               - renesas,vin-r8a779g0 # R-Car V4H
+          - const: renesas,rcar-gen4-vin # Generic R-Car Gen4
 
   reg:
     maxItems: 1