1. Patchew
  2. linux
  3. View series

[PATCH] dt-bindings: serial: 8250: allow "main" and "uart" as clock names

Alex Elder posted 1 patch 2 months, 1 week ago
There is a newer version of this series
.../devicetree/bindings/serial/8250.yaml      | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)
[PATCH] dt-bindings: serial: 8250: allow "main" and "uart" as clock names
Posted by Alex Elder 2 months, 1 week ago 
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@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202507160314.wrC51lXX-lkp@intel.com/
Signed-off-by: Alex Elder <elder@riscstar.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:
-      properties:
-        clock-names:
-          items:
-            - const: core
-            - const: bus
+      if:
+        properties:
+          compatible:
+            contains:
+              const: spacemit,k1-uart
+      then:
+        properties:
+          clock-names:
+            items:
+              - const: core
+              - const: bus
 
 properties:
   compatible:
@@ -162,6 +168,9 @@ properties:
     minItems: 1
     maxItems: 2
     oneOf:
+      - enum:
+          - main
+          - uart
       - items:
           - const: core
           - const: bus

base-commit: 0b90c3b6d76ea512dc3dac8fb30215e175b0019a
-- 
2.48.1
Re: [PATCH] dt-bindings: serial: 8250: allow "main" and "uart" as clock names
Posted by Yixun Lan 2 months, 1 week ago 
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@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202507160314.wrC51lXX-lkp@intel.com/
> Signed-off-by: Alex Elder <elder@riscstar.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..

> -      properties:
> -        clock-names:
> -          items:
> -            - const: core
> -            - const: bus
> +      if:
> +        properties:
> +          compatible:
> +            contains:
> +              const: spacemit,k1-uart
> +      then:
> +        properties:
> +          clock-names:
> +            items:
> +              - const: core
> +              - const: bus
>  
>  properties:
>    compatible:
> @@ -162,6 +168,9 @@ properties:
>      minItems: 1
>      maxItems: 2
>      oneOf:
> +      - enum:
> +          - main
> +          - uart
>        - items:
>            - const: core
>            - const: bus
> 
> base-commit: 0b90c3b6d76ea512dc3dac8fb30215e175b0019a
> -- 
> 2.48.1
> 

-- 
Yixun Lan (dlan)
Re: [PATCH] dt-bindings: serial: 8250: allow "main" and "uart" as clock names
Posted by Conor Dooley 2 months, 1 week ago 
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@intel.com>
> > Closes: https://lore.kernel.org/oe-kbuild-all/202507160314.wrC51lXX-lkp@intel.com/
> > Signed-off-by: Alex Elder <elder@riscstar.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?
Re: [PATCH] dt-bindings: serial: 8250: allow "main" and "uart" as clock names
Posted by Alex Elder 2 months, 1 week ago 
On 7/29/25 12:54 PM, Conor Dooley wrote:
> 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@intel.com>
>>> Closes: https://lore.kernel.org/oe-kbuild-all/202507160314.wrC51lXX-lkp@intel.com/
>>> Signed-off-by: Alex Elder <elder@riscstar.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?

Are you talking about the blank line here?

     then:
       oneOf:
         - required: [ clock-frequency ]
         - required: [ clocks ]
                            <------ this blank line
   - if:
       properties:
         compatible:
           contains:
             const: nxp,lpc1850-uart
     then:

I didn't notice that before.  It got inserted with commit
d2db0d7815444 ("dt-bindings: serial: 8250: allow clock
'uartclk' and 'reg' for nxp,lpc1850-uart").

If so, yes I'll remove that as well when I update the patch to
get rid of the else as Yixun suggests.

Greg won't take this for a couple weeks so I'll hold off sending
v2 for a while.

					-Alex
Re: [PATCH] dt-bindings: serial: 8250: allow "main" and "uart" as clock names
Posted by Conor Dooley 2 months, 1 week ago 
On Tue, Jul 29, 2025 at 04:04:05PM -0500, Alex Elder wrote:
> On 7/29/25 12:54 PM, Conor Dooley wrote:
> > 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@intel.com>
> > > > Closes: https://lore.kernel.org/oe-kbuild-all/202507160314.wrC51lXX-lkp@intel.com/
> > > > Signed-off-by: Alex Elder <elder@riscstar.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?
> 
> Are you talking about the blank line here?

No, I'm talking about what's down around line 270 in the binding.

> 
>     then:
>       oneOf:
>         - required: [ clock-frequency ]
>         - required: [ clocks ]
>                            <------ this blank line
>   - if:
>       properties:
>         compatible:
>           contains:
>             const: nxp,lpc1850-uart
>     then:
> 
> I didn't notice that before.  It got inserted with commit
> d2db0d7815444 ("dt-bindings: serial: 8250: allow clock
> 'uartclk' and 'reg' for nxp,lpc1850-uart").
> 
> If so, yes I'll remove that as well when I update the patch to
> get rid of the else as Yixun suggests.
> 
> Greg won't take this for a couple weeks so I'll hold off sending
> v2 for a while.
> 
> 					-Alex
>
Re: [PATCH] dt-bindings: serial: 8250: allow "main" and "uart" as clock names
Posted by Alex Elder 2 months, 1 week ago 
On 7/29/25 4:08 PM, Conor Dooley wrote:
> On Tue, Jul 29, 2025 at 04:04:05PM -0500, Alex Elder wrote:
>> On 7/29/25 12:54 PM, Conor Dooley wrote:
>>> 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@intel.com>
>>>>> Closes: https://lore.kernel.org/oe-kbuild-all/202507160314.wrC51lXX-lkp@intel.com/
>>>>> Signed-off-by: Alex Elder <elder@riscstar.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?
>>
>> Are you talking about the blank line here?
> 
> No, I'm talking about what's down around line 270 in the binding.

Oh wow that's in a weird spot, and it might be redundant?

Anyway I'll work on getting that fixed too before I send
my next version.

					-Alex

>>
>>      then:
>>        oneOf:
>>          - required: [ clock-frequency ]
>>          - required: [ clocks ]
>>                             <------ this blank line
>>    - if:
>>        properties:
>>          compatible:
>>            contains:
>>              const: nxp,lpc1850-uart
>>      then:
>>
>> I didn't notice that before.  It got inserted with commit
>> d2db0d7815444 ("dt-bindings: serial: 8250: allow clock
>> 'uartclk' and 'reg' for nxp,lpc1850-uart").
>>
>> If so, yes I'll remove that as well when I update the patch to
>> get rid of the else as Yixun suggests.
>>
>> Greg won't take this for a couple weeks so I'll hold off sending
>> v2 for a while.
>>
>> 					-Alex
>>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.