Re: [PATCH v6] dt-bindings: net: can: fsl,mpc5xxx-mscan: Convert to DT schema

[Rob Herring <robh@kernel.org>]

On Wed, Sep 16, 2026 at 09:58:03AM +0800, Quchaosheng wrote:
> Convert the Freescale MPC5xxx MSCAN binding from free-form text to DT
> schema.
> 
> Describe fsl,mpc5200-mscan and fsl,mpc5121-mscan in separate sections, as
> the original text does. fsl,mpc5200-mscan accepts "ip" and "ref" for
> fsl,mscan-clock-source, fsl,mpc5121-mscan also accepts "sys". Enforce both
> with an if/then block on the compatible string.
> 
> Add "fsl,mpc5200b-mscan" as a fallback of "fsl,mpc5200-mscan", as
> mpc5200b.dtsi uses it.
> 
> Document clocks and clock-names. The MPC5121 driver looks up the "ipg",
> "ips", "sys", "ref" and "mclk" clocks by name, and mpc5121.dtsi provides
> all five. The original text did not describe them. MPC5200 obtains its
> frequency from the CDM block and does not use DT clocks at all.
> 
> clock-names accepts the names in any order and allows a subset: the driver
> looks each one up by name (devm_clk_get()) and only needs the clocks
> actually selected, so a node providing just the clocks in use is valid.
> 
> Keep the note about MPC5121 Rev. 1.
> 
> Signed-off-by: Quchaosheng <quchaosheng000406@163.com>
> ---
> v2:
>  - Add "fsl,mpc5200b-mscan" as a fallback compatible.
>  - Document clocks and clock-names.
> 
> v3:
>  - Split fsl,mscan-clock-source by compatible string, following the
>    separate sections in the original text. "sys" applies to
>    fsl,mpc5121-mscan only.
>  - Rewrite the commit message in imperative mood.
> 
> v6:
>  - No content change. Fix the patch format: the changelog was placed in the
>    commit message instead of after the '---' separator, which confused
>    Patchwork. Move it back where it belongs.
> 
>  - clock-names: accept the names in any order and allow a subset, as the
>    driver looks them up by name and only needs the clocks actually
>    selected (devm_clk_get()). The previous strict ordered 5-item list
>    rejected valid nodes that provide only the clocks in use.
> 
>  - clocks: add minItems: 1 and note that MPC5200 does not use DT clocks
>    at all; it obtains its frequency from the CDM block.
>  .../bindings/net/can/fsl,mpc5xxx-mscan.yaml   | 118 ++++++++++++++++++
>  .../bindings/net/can/mpc5xxx-mscan.txt        |  53 --------
>  2 files changed, 118 insertions(+), 53 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/net/can/fsl,mpc5xxx-mscan.yaml
>  delete mode 100644 Documentation/devicetree/bindings/net/can/mpc5xxx-mscan.txt
> 
> diff --git a/Documentation/devicetree/bindings/net/can/fsl,mpc5xxx-mscan.yaml b/Documentation/devicetree/bindings/net/can/fsl,mpc5xxx-mscan.yaml
> new file mode 100644
> index 000000000..7f01008f1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/can/fsl,mpc5xxx-mscan.yaml
> @@ -0,0 +1,118 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/can/fsl,mpc5xxx-mscan.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Freescale MPC5xxx MSCAN controller
> +
> +maintainers:
> +  - Marc Kleine-Budde <mkl@pengutronix.de>
> +
> +description:
> +  The MSCAN controller found on the Freescale MPC5200 and MPC5121 SoCs. The
> +  MPC5121 Rev. 1 processor is not supported.
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - items:
> +          - enum:
> +              - fsl,mpc5200b-mscan
> +          - const: fsl,mpc5200-mscan
> +      - enum:
> +          - fsl,mpc5200-mscan
> +          - fsl,mpc5121-mscan
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    description:
> +      Clocks feeding the controller. Only meaningful on MPC5121, where the
> +      driver looks the clocks up by name ("ipg" for register access, plus
> +      the clocks selected through fsl,mscan-clock-source). MPC5200 obtains
> +      its frequency from the CDM block and does not use DT clocks.
> +    minItems: 1
> +    maxItems: 5
> +
> +  clock-names:
> +    description:
> +      The driver looks clocks up by name, so the order is not significant,
> +      but each name may be given at most once.

There's only one defined order, so please define the order. We only 
don't define the order if the schema and/or users are too much of a mess 
to do so.

> +    minItems: 1
> +    maxItems: 5
> +    items:
> +      enum: [ipg, ips, sys, ref, mclk]
> +
> +  fsl,mscan-clock-source:
> +    description:
> +      Clock source used for the controller.
> +    enum: [ip, ref, sys]
> +
> +  fsl,mscan-clock-divider:
> +    description: Additional clock divider for the reference and system clocks.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    default: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +
> +allOf:
> +  - $ref: can-controller.yaml#
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: fsl,mpc5200-mscan
> +    then:
> +      properties:
> +        fsl,mscan-clock-source:
> +          description:
> +            Clock source used for the controller. "ref" is the default when
> +            the property is not present.
> +          enum: [ip, ref]
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: fsl,mpc5121-mscan
> +    then:
> +      properties:
> +        fsl,mscan-clock-source:
> +          description:
> +            Clock source used for the controller. When the property is not
> +            present, an optimal clock source and frequency based on the
> +            system clock is selected. If this is not possible, the reference
> +            clock is used.
> +          enum: [ip, ref, sys]

You can drop this if/then. It has no effect.

Though looks like this should define 'minItems: 5' for 
clocks/clock-names?

> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    can@1300 {
> +        compatible = "fsl,mpc5121-mscan";
> +        reg = <0x1300 0x80>;
> +        interrupts = <12 IRQ_TYPE_LEVEL_LOW>;
> +        interrupt-parent = <&ipic>;
> +    };
> +
> +    can@1380 {
> +        compatible = "fsl,mpc5121-mscan";
> +        reg = <0x1380 0x80>;
> +        interrupts = <13 IRQ_TYPE_LEVEL_LOW>;
> +        interrupt-parent = <&ipic>;
> +        fsl,mscan-clock-source = "ref";
> +        fsl,mscan-clock-divider = <3>;
> +    };

One example is enough.

Rob