1. Patchew
  2. linux
  3. [PATCH RESEND v1 0/7] MPFS clock fixes required for correct CAN clock modeling
  4. View patch

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

Conor Dooley posted 7 patches 2 years ago
There is a newer version of this series
[PATCH RESEND v1 2/7] dt-bindings: can: mpfs: add missing required clock
Posted by Conor Dooley 2 years ago 
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
 
 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>;
     };
-- 
2.39.2
Re: [PATCH RESEND v1 2/7] dt-bindings: can: mpfs: add missing required clock
Posted by Marc Kleine-Budde 2 years ago 
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

-- 
Pengutronix e.K.                 | Marc Kleine-Budde          |
Embedded Linux                   | https://www.pengutronix.de |
Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |
Re: [PATCH RESEND v1 2/7] dt-bindings: can: mpfs: add missing required clock
Posted by Conor Dooley 2 years ago 
On Tue, Dec 12, 2023 at 09:49:41PM +0100, Marc Kleine-Budde wrote:
> 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.

We should not need the clock-names property to be able to get both of
the clocks. clk_bulk_get_all() for example should be usable here.

Cheers,
Conor.
Re: [PATCH RESEND v1 2/7] dt-bindings: can: mpfs: add missing required clock
Posted by Marc Kleine-Budde 2 years ago 
On 13.12.2023 13:02:49, Conor Dooley wrote:
> On Tue, Dec 12, 2023 at 09:49:41PM +0100, Marc Kleine-Budde wrote:
> > 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.
> 
> We should not need the clock-names property to be able to get both of
> the clocks. clk_bulk_get_all() for example should be usable here.

ACK, but we need the clock rate of CAN clock. Does this binding check
that the CAN clock rate is the 2nd one?

regards,
Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde          |
Embedded Linux                   | https://www.pengutronix.de |
Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |
Re: [PATCH RESEND v1 2/7] dt-bindings: can: mpfs: add missing required clock
Posted by Conor Dooley 2 years ago 
On Thu, Dec 14, 2023 at 12:31:04PM +0100, Marc Kleine-Budde wrote:
> On 13.12.2023 13:02:49, Conor Dooley wrote:
> > On Tue, Dec 12, 2023 at 09:49:41PM +0100, Marc Kleine-Budde wrote:
> > > 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.
> > 
> > We should not need the clock-names property to be able to get both of
> > the clocks. clk_bulk_get_all() for example should be usable here.
> 
> ACK, but we need the clock rate of CAN clock. Does this binding check
> that the CAN clock rate is the 2nd one?

The items list requires that the can clock be the second one, so drivers
etc can rely on that ordering.
Re: [PATCH RESEND v1 2/7] dt-bindings: can: mpfs: add missing required clock
Posted by Marc Kleine-Budde 2 years ago 
On 14.12.2023 13:16:55, Conor Dooley wrote:
> > > > > --- 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.
> > > 
> > > We should not need the clock-names property to be able to get both of
> > > the clocks. clk_bulk_get_all() for example should be usable here.
> > 
> > ACK, but we need the clock rate of CAN clock. Does this binding check
> > that the CAN clock rate is the 2nd one?
> 
> The items list requires that the can clock be the second one, so drivers
> etc can rely on that ordering.

Thanks for the clarification,
Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde          |
Embedded Linux                   | https://www.pengutronix.de |
Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |
Re: [PATCH RESEND v1 2/7] dt-bindings: can: mpfs: add missing required clock
Posted by Rob Herring 2 years ago 
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#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20231208-palpitate-passable-c79bacf2036c@spud

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
Re: [PATCH RESEND v1 2/7] dt-bindings: can: mpfs: add missing required clock
Posted by Conor Dooley 2 years ago 
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.
Re: [PATCH RESEND v1 2/7] dt-bindings: can: mpfs: add missing required clock
Posted by Rob Herring 2 years ago 
On Fri, Dec 08, 2023 at 07:25:39PM +0000, Conor Dooley wrote:
> 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.

Happens to the best of us. :)

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.