diff mbox series

[RESEND,v1,2/7] dt-bindings: can: mpfs: add missing required clock

Message ID 20231208-palpitate-passable-c79bacf2036c@spud
State New
Headers show
Series MPFS clock fixes required for correct CAN clock modeling | expand

Commit Message

Conor Dooley Dec. 8, 2023, 5:12 p.m. UTC
From: Conor Dooley <conor.dooley@microchip.com>

The CAN controller on PolarFire SoC has an AHB peripheral clock _and_ a
CAN bus clock. The bus clock was omitted when the binding was written,
but is required for operation. Make up for lost time and add it.

Cautionary tale in adding bindings without having implemented a real
user for them perhaps.

Fixes: c878d518d7b6 ("dt-bindings: can: mpfs: document the mpfs CAN controller")
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 .../devicetree/bindings/net/can/microchip,mpfs-can.yaml    | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Comments

Conor Dooley Dec. 8, 2023, 7:25 p.m. UTC | #1
On Fri, Dec 08, 2023 at 12:31:00PM -0600, Rob Herring wrote:
> 
> On Fri, 08 Dec 2023 17:12:24 +0000, Conor Dooley wrote:
> > From: Conor Dooley <conor.dooley@microchip.com>
> > 
> > The CAN controller on PolarFire SoC has an AHB peripheral clock _and_ a
> > CAN bus clock. The bus clock was omitted when the binding was written,
> > but is required for operation. Make up for lost time and add it.
> > 
> > Cautionary tale in adding bindings without having implemented a real
> > user for them perhaps.
> > 
> > Fixes: c878d518d7b6 ("dt-bindings: can: mpfs: document the mpfs CAN controller")
> > Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> > ---
> >  .../devicetree/bindings/net/can/microchip,mpfs-can.yaml    | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> > 
> 
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
> 
> yamllint warnings/errors:
> 
> dtschema/dtc warnings/errors:
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/can/microchip,mpfs-can.yaml: properties:clocks: {'maxItems': 2, 'items': [{'description': 'AHB peripheral clock'}, {'description': 'CAN bus clock'}]} should not be valid under {'required': ['maxItems']}
> 	hint: "maxItems" is not needed with an "items" list
> 	from schema $id: http://devicetree.org/meta-schemas/items.yaml#


Oh dear, me of all people.
Marc Kleine-Budde Dec. 12, 2023, 8:49 p.m. UTC | #2
On 08.12.2023 17:12:24, Conor Dooley wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
> 
> The CAN controller on PolarFire SoC has an AHB peripheral clock _and_ a
> CAN bus clock. The bus clock was omitted when the binding was written,
> but is required for operation. Make up for lost time and add it.
> 
> Cautionary tale in adding bindings without having implemented a real
> user for them perhaps.
> 
> Fixes: c878d518d7b6 ("dt-bindings: can: mpfs: document the mpfs CAN controller")
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
>  .../devicetree/bindings/net/can/microchip,mpfs-can.yaml    | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/can/microchip,mpfs-can.yaml b/Documentation/devicetree/bindings/net/can/microchip,mpfs-can.yaml
> index 45aa3de7cf01..05f680f15b17 100644
> --- a/Documentation/devicetree/bindings/net/can/microchip,mpfs-can.yaml
> +++ b/Documentation/devicetree/bindings/net/can/microchip,mpfs-can.yaml
> @@ -24,7 +24,10 @@ properties:
>      maxItems: 1
>  
>    clocks:
> -    maxItems: 1
> +    maxItems: 2
> +    items:
> +      - description: AHB peripheral clock
> +      - description: CAN bus clock

Do we we want to have a "clock-names" property, as we need the clock
rate of the CAN bus clock.

Marc

>  
>  required:
>    - compatible
> @@ -39,7 +42,7 @@ examples:
>      can@2010c000 {
>          compatible = "microchip,mpfs-can";
>          reg = <0x2010c000 0x1000>;
> -        clocks = <&clkcfg 17>;
> +        clocks = <&clkcfg 17>, <&clkcfg 37>;
>          interrupt-parent = <&plic>;
>          interrupts = <56>;
>      };

Marc
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/net/can/microchip,mpfs-can.yaml b/Documentation/devicetree/bindings/net/can/microchip,mpfs-can.yaml
index 45aa3de7cf01..05f680f15b17 100644
--- a/Documentation/devicetree/bindings/net/can/microchip,mpfs-can.yaml
+++ b/Documentation/devicetree/bindings/net/can/microchip,mpfs-can.yaml
@@ -24,7 +24,10 @@  properties:
     maxItems: 1
 
   clocks:
-    maxItems: 1
+    maxItems: 2
+    items:
+      - description: AHB peripheral clock
+      - description: CAN bus clock
 
 required:
   - compatible
@@ -39,7 +42,7 @@  examples:
     can@2010c000 {
         compatible = "microchip,mpfs-can";
         reg = <0x2010c000 0x1000>;
-        clocks = <&clkcfg 17>;
+        clocks = <&clkcfg 17>, <&clkcfg 37>;
         interrupt-parent = <&plic>;
         interrupts = <56>;
     };