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 |
> - - 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?
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
> 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.
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>
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.
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
> 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 --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:
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(-)