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 (mailing list archive)
State Accepted
Delegated to: Geert Uytterhoeven
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, 8:36 a.m. UTC | #2
Hi Wolfram,

On Thu, 10 Apr 2025 at 10:33, Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
> > -            - 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

You can run Linux on 6 MiB of RAM, if you try hard ;-)

> effort for other OS in the last 7 years? And making the remaining

... which does not mean there are no users.

> r9a06g032 just const? Why should we allow everything there? Do we want
> to support that?

We don't allow "everything". Valid compatible values are checked by
the normal rules below.

Gr{oetje,eeting}s,

                        Geert
Wolfram Sang April 10, 2025, 9:37 a.m. UTC | #3
> > What about simply dropping r9a06g033 which cannot run Linux (no RAM
> > controller, only 6MB internal RAM) and there hasn't been any upstreaming
> 
> You can run Linux on 6 MiB of RAM, if you try hard ;-)

Whoever tries that hard will have zero problems upstreaming the bindings
then again. I will happily ack them. But it will not happen.

> 
> > effort for other OS in the last 7 years? And making the remaining
> 
> ... which does not mean there are no users.

And these mysterious users will complain about a removed UART binding
while there is not even a clk binding upstream?

> 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

      - items:
          - enum:
              - renesas,r8a774a1-wdt     # RZ/G2M
              - renesas,r8a774b1-wdt     # RZ/G2N
              - renesas,r8a774c0-wdt     # RZ/G2E
              - renesas,r8a774e1-wdt     # RZ/G2H
              - renesas,r8a7795-wdt      # R-Car H3
              - renesas,r8a7796-wdt      # R-Car M3-W
              - renesas,r8a77961-wdt     # R-Car M3-W+
              - renesas,r8a77965-wdt     # R-Car M3-N
              - renesas,r8a77970-wdt     # R-Car V3M
              - renesas,r8a77980-wdt     # R-Car V3H
              - renesas,r8a77990-wdt     # R-Car E3
              - renesas,r8a77995-wdt     # R-Car D3
          - const: renesas,rcar-gen3-wdt # R-Car Gen3 and RZ/G2

      - items:
          - enum:
              - renesas,r8a779a0-wdt     # R-Car V3U
              - renesas,r8a779f0-wdt     # R-Car S4-8
              - renesas,r8a779g0-wdt     # R-Car V4H
              - renesas,r8a779h0-wdt     # R-Car V4M
          - const: renesas,rcar-gen4-wdt # R-Car Gen4
Geert Uytterhoeven April 10, 2025, 10:56 a.m. UTC | #4
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 | #5
> 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 | #6
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 | #7
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 | #8
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 | #9
> 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: