diff mbox series

[RESEND,v2,1/4] dt-bindings: bluetooth: broadcom: Fix clocks check

Message ID 20210924072756.869731-1-maxime@cerno.tech (mailing list archive)
State Awaiting Upstream
Delegated to: Netdev Maintainers
Headers show
Series [RESEND,v2,1/4] dt-bindings: bluetooth: broadcom: Fix clocks check | expand

Checks

Context Check Description
netdev/tree_selection success Not a local patch

Commit Message

Maxime Ripard Sept. 24, 2021, 7:27 a.m. UTC 
The original binding was mentioning that valid values for the clocks and
clock-names property were one or two clocks from extclk, txco and lpo,
with extclk being deprecated in favor of txco.

However, the current binding lists a valid array as extclk, txco and
lpo, with either one or two items.

While this looks similar, it actually enforces that all the device trees
use either ["extclk"], or ["extclk", "txco"]. That doesn't make much
sense, since the two clocks are said to be equivalent, with one
superseeding the other.

lpo is also not a valid clock anymore, and would be as the third clock
of the list, while we could have only this clock in the previous binding
(and in DTs).

Let's rework the clock clause to allow to have either:

 - extclk, and mark it a deprecated
 - txco alone
 - lpo alone
 - txco, lpo

While ["extclk", "lpo"] wouldn't be valid, it wasn't found in any device
tree so it's not an issue in practice.

Similarly, ["lpo", "txco"] is still considered invalid, but it's
generally considered as a best practice to fix the order of clocks.

Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: netdev@vger.kernel.org
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 .../bindings/net/broadcom-bluetooth.yaml        | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

Comments

Rob Herring (Arm) Oct. 12, 2021, 2:08 p.m. UTC | #1 
On Fri, Sep 24, 2021 at 09:27:53AM +0200, Maxime Ripard wrote:
> The original binding was mentioning that valid values for the clocks and
> clock-names property were one or two clocks from extclk, txco and lpo,
> with extclk being deprecated in favor of txco.
> 
> However, the current binding lists a valid array as extclk, txco and
> lpo, with either one or two items.
> 
> While this looks similar, it actually enforces that all the device trees
> use either ["extclk"], or ["extclk", "txco"]. That doesn't make much
> sense, since the two clocks are said to be equivalent, with one
> superseeding the other.
> 
> lpo is also not a valid clock anymore, and would be as the third clock
> of the list, while we could have only this clock in the previous binding
> (and in DTs).
> 
> Let's rework the clock clause to allow to have either:
> 
>  - extclk, and mark it a deprecated
>  - txco alone
>  - lpo alone
>  - txco, lpo
> 
> While ["extclk", "lpo"] wouldn't be valid, it wasn't found in any device
> tree so it's not an issue in practice.
> 
> Similarly, ["lpo", "txco"] is still considered invalid, but it's
> generally considered as a best practice to fix the order of clocks.
> 
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: netdev@vger.kernel.org
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Reviewed-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> ---
>  .../bindings/net/broadcom-bluetooth.yaml        | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)

I've applied this and the rest of the series.

Rob
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/net/broadcom-bluetooth.yaml b/Documentation/devicetree/bindings/net/broadcom-bluetooth.yaml
index fbdc2083bec4..5aac094fd217 100644
--- a/Documentation/devicetree/bindings/net/broadcom-bluetooth.yaml
+++ b/Documentation/devicetree/bindings/net/broadcom-bluetooth.yaml
@@ -50,16 +50,29 @@  properties:
       by interrupts and "host-wakeup" interrupt-names
 
   clocks:
+    minItems: 1
     maxItems: 2
     description: 1 or 2 clocks as defined in clock-names below,
       in that order
 
   clock-names:
     description: Names of the 1 to 2 supplied clocks
-    items:
+    oneOf:
+      - const: extclk
+        deprecated: true
+        description: Deprecated in favor of txco
+
       - const: txco
+        description: >
+          external reference clock (not a standalone crystal)
+
       - const: lpo
-      - const: extclk
+        description: >
+          external low power 32.768 kHz clock
+
+      - items:
+          - const: txco
+          - const: lpo
 
   vbat-supply:
     description: phandle to regulator supply for VBAT