[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250729-reshuffle-contented-e6def76b540b@spud>
Date: Tue, 29 Jul 2025 18:54:07 +0100
From: Conor Dooley <conor@...nel.org>
To: Yixun Lan <dlan@...too.org>
Cc: Alex Elder <elder@...cstar.com>, gregkh@...uxfoundation.org,
jirislaby@...nel.org, robh@...nel.org, krzk+dt@...nel.org,
conor+dt@...nel.org, matthias.bgg@...il.com,
angelogioacchino.delregno@...labora.com, lkundrak@...sk,
devicetree@...r.kernel.org, linux-serial@...r.kernel.org,
spacemit@...ts.linux.dev, linux-mediatek@...ts.infradead.org,
linux-riscv@...ts.infradead.org,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
kernel test robot <lkp@...el.com>
Subject: Re: [PATCH] dt-bindings: serial: 8250: allow "main" and "uart" as
clock names
On Tue, Jul 29, 2025 at 06:53:19AM +0800, Yixun Lan wrote:
> Hi Alex,
>
> On 17:00 Mon 28 Jul , Alex Elder wrote:
> > There are two compatible strings defined in "8250.yaml" that require
> > two clocks to be specified, along with their names:
> > - "spacemit,k1-uart", used in "spacemit/k1.dtsi"
> > - "nxp,lpc1850-uart", used in "lpc/lpc18xx.dtsi"
> >
> > When only one clock is used, the name is not required. However there
> > are two places that do specify a name:
> > - In "mediatek/mt7623.dtsi", the clock for the "mediatek,mtk-btif"
> > compatible serial device is named "main"
> > - In "qca/ar9132.dtsi", the clock for the "ns8250" compatible
> > serial device is named "uart"
> >
> > In commit d2db0d7815444 ("dt-bindings: serial: 8250: allow clock 'uartclk'
> > and 'reg' for nxp,lpc1850-uart"), Frank Li added the restriction that two
> > named clocks be used for the NXP platform mentioned above. Extend that
> > so that the two named clocks used by the SpacemiT platform are similarly
> > restricted.
> >
> > Add "main" and "uart" as allowed names when a single clock is specified.
> >
> > Fixes: 2c0594f9f0629 ("dt-bindings: serial: 8250: support an optional second clock")
> > Reported-by: kernel test robot <lkp@...el.com>
> > Closes: https://lore.kernel.org/oe-kbuild-all/202507160314.wrC51lXX-lkp@intel.com/
> > Signed-off-by: Alex Elder <elder@...cstar.com>
> > ---
> > .../devicetree/bindings/serial/8250.yaml | 19 ++++++++++++++-----
> > 1 file changed, 14 insertions(+), 5 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/serial/8250.yaml b/Documentation/devicetree/bindings/serial/8250.yaml
> > index e46bee8d25bf0..cef52ebd8f7da 100644
> > --- a/Documentation/devicetree/bindings/serial/8250.yaml
> > +++ b/Documentation/devicetree/bindings/serial/8250.yaml
> > @@ -61,11 +61,17 @@ allOf:
> > - const: uartclk
> > - const: reg
> ..
> > else:
> would it be better to drop this 'else', and moving following 'if' block
> to the same level with "nxp,lpc1850-uart"?
>
> the reason here would avoid too many indentions if add more constraint in
> the future if other SoC uart need different clock-names..
I agree, it's more typical to do it that way I think to boot.
Also, why is there a k1/lpc conditional bit that is not part of the
allOf in addition to the bits in the allOf? Can that get merged with the
allOf please?
Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)
Powered by blists - more mailing lists