diff mbox series

dt-bindings: serial: snps-dw-apb-uart: Simplify DMA-less RZ/N1 rule

Message ID 90c7aa143beb6a28255b24e8ef8c96180d869cbb.1744271974.git.geert+renesas@glider.be
State New
Headers show
Series dt-bindings: serial: snps-dw-apb-uart: Simplify DMA-less RZ/N1 rule | expand

Commit Message

Geert Uytterhoeven April 10, 2025, 8 a.m. UTC
There is no need to repeat all SoC-specific compatible values in the
rule for DMA-less RZ/N1 variants.  Use wildcard "{}" instead, to ease
maintenance.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 .../devicetree/bindings/serial/snps-dw-apb-uart.yaml          | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

Comments

Wolfram Sang April 10, 2025, 8:33 a.m. UTC | #1
> -            - enum:
> -                - renesas,r9a06g032-uart
> -                - renesas,r9a06g033-uart
> +            - {}

What about simply dropping r9a06g033 which cannot run Linux (no RAM
controller, only 6MB internal RAM) and there hasn't been any upstreaming
effort for other OS in the last 7 years? And making the remaining
r9a06g032 just const? Why should we allow everything there? Do we want
to support that?
Geert Uytterhoeven April 10, 2025, 10:56 a.m. UTC | #2
Hi Wolfram,

On Thu, 10 Apr 2025 at 11:37, Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
> > We don't allow "everything". Valid compatible values are checked by
> > the normal rules below.
>
> Why don't we use '{}' with all the bindings then? Would simplify so
> much. From the watchdog driver:
>
>
>       - items:
>           - enum:
>               - renesas,r8a7742-wdt      # RZ/G1H
>               - renesas,r8a7743-wdt      # RZ/G1M
>               - renesas,r8a7744-wdt      # RZ/G1N
>               - renesas,r8a7745-wdt      # RZ/G1E
>               - renesas,r8a77470-wdt     # RZ/G1C
>               - renesas,r8a7790-wdt      # R-Car H2
>               - renesas,r8a7791-wdt      # R-Car M2-W
>               - renesas,r8a7792-wdt      # R-Car V2H
>               - renesas,r8a7793-wdt      # R-Car M2-N
>               - renesas,r8a7794-wdt      # R-Car E2
>           - const: renesas,rcar-gen2-wdt # R-Car Gen2 and RZ/G1

[...]

The watchdog bindings do not have an extra rule that lists all
compatible values a second time.

Gr{oetje,eeting}s,

                        Geert
Wolfram Sang April 10, 2025, 8:29 p.m. UTC | #3
> The watchdog bindings do not have an extra rule that lists all
> compatible values a second time.

I see, this only simplifies the 'if' condition preventing the dma
properties. For me, that is just another reason to drop 'r9a06g033'
altogether because that would simplify both occurences and make it all
more readable, not less.

And I still think the other two points which you decided to not quote
still stand.
Rob Herring (Arm) April 11, 2025, 1:38 p.m. UTC | #4
On Thu, Apr 10, 2025 at 3:23 AM Geert Uytterhoeven
<geert+renesas@glider.be> wrote:
>
> There is no need to repeat all SoC-specific compatible values in the
> rule for DMA-less RZ/N1 variants.  Use wildcard "{}" instead, to ease
> maintenance.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  .../devicetree/bindings/serial/snps-dw-apb-uart.yaml          | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Wolfram Sang April 11, 2025, 3:37 p.m. UTC | #5
On Fri, Apr 11, 2025 at 08:38:58AM -0500, Rob Herring wrote:
> On Thu, Apr 10, 2025 at 3:23 AM Geert Uytterhoeven
> <geert+renesas@glider.be> wrote:
> >
> > There is no need to repeat all SoC-specific compatible values in the
> > rule for DMA-less RZ/N1 variants.  Use wildcard "{}" instead, to ease
> > maintenance.
> >
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > ---
> >  .../devicetree/bindings/serial/snps-dw-apb-uart.yaml          | 4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> Reviewed-by: Rob Herring (Arm) <robh@kernel.org>

I'll send my counterpatch in some minutes.
Rob Herring (Arm) April 11, 2025, 4:16 p.m. UTC | #6
On Fri, Apr 11, 2025 at 05:37:09PM +0200, Wolfram Sang wrote:
> On Fri, Apr 11, 2025 at 08:38:58AM -0500, Rob Herring wrote:
> > On Thu, Apr 10, 2025 at 3:23 AM Geert Uytterhoeven
> > <geert+renesas@glider.be> wrote:
> > >
> > > There is no need to repeat all SoC-specific compatible values in the
> > > rule for DMA-less RZ/N1 variants.  Use wildcard "{}" instead, to ease
> > > maintenance.
> > >
> > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > > ---
> > >  .../devicetree/bindings/serial/snps-dw-apb-uart.yaml          | 4 +---
> > >  1 file changed, 1 insertion(+), 3 deletions(-)
> > 
> > Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
> 
> I'll send my counterpatch in some minutes.

IMO, whether you drop the platform is orthogonal to this patch. 

Whether or not the platform can run Linux is irrelevant to whether there 
are bindings. Can it run u-boot? Now, if no one is going to make the 
bindings complete and upstream a .dts for it, then remove it.

Rob
Wolfram Sang April 11, 2025, 7:12 p.m. UTC | #7
> IMO, whether you drop the platform is orthogonal to this patch. 

Ok.

> Whether or not the platform can run Linux is irrelevant to whether there 

Yes and no. I know what you mean with "irrelevant" and I agree to that.
But...

> are bindings. Can it run u-boot? Now, if no one is going to make the 
> bindings complete and upstream a .dts for it, then remove it.

... also this. If the SoC can hardly run Linux (if at all) it is a lot
less likely in practice that someone will complete the upstream support,
no?
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml b/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
index 1aa3480d8d818e99..1ffe3834b0a85085 100644
--- a/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
+++ b/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
@@ -17,9 +17,7 @@  allOf:
       properties:
         compatible:
           items:
-            - enum:
-                - renesas,r9a06g032-uart
-                - renesas,r9a06g033-uart
+            - {}
             - const: renesas,rzn1-uart
             - const: snps,dw-apb-uart
     then: