[PATCH 1/2] dt-bindings: mailbox : marvell,mbox: Add bindings

[PATCH 1/2] dt-bindings: mailbox : marvell,mbox: Add bindings

[Wojciech Bartczak]

This patch adds device tree binding for Marvell Message Handling Unit
(MHU) controller.

Signed-off-by: Wojciech Bartczak <wbartczak@marvell.com>
---
 .../mailbox/marvell,mbox-mailbox.yaml         | 112 ++++++++++++++++++
 1 file changed, 112 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mailbox/marvell,mbox-mailbox.yaml

diff --git a/Documentation/devicetree/bindings/mailbox/marvell,mbox-mailbox.yaml b/Documentation/devicetree/bindings/mailbox/marvell,mbox-mailbox.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..d9a6e14dcb12da6c3a9db2dfafc075ccefa8711c
--- /dev/null
+++ b/Documentation/devicetree/bindings/mailbox/marvell,mbox-mailbox.yaml
@@ -0,0 +1,112 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mailbox/marvell,mbox-mailbox.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Marvell Message Handling Unit
+
+maintainers:
+  - Wojciech Bartczak <wbartczak@marvell.com>
+  - Piyush Malgujar <pmalgujar@marvell.com>
+
+description:
+  The Marvell's Message Handling Unit is a mailbox controller
+  with a single channel used to communicate with System Control Processor.
+  Driver supports series of cn9x and cn10x SoC.
+  Sole purpose of the link is to exchange SCMI related data with SCP.
+  The link has hardwired configuration, it uses simple notification scheme
+  over shared memory block to push data back and forth.
+  Interrupts used by mailbox may be configured in two ways,
+  as SPI interrupts, then driver uses platform device forntend.
+  Other way is to use PCI bus frontend with LPI interrupts.
+
+properties:
+  compatible:
+    items:
+      - const: marvell,mbox
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    items:
+      - description: interrupt type
+        const: 0
+      - description: interrupt number
+      - description: interrupt trigger type
+        const: 1
+
+  '#mbox-cells':
+    description: Index of the channel
+    const: 1
+
+  shmem:
+    description:
+      List of phandle pointing to the shared memory area between SCP and AP.
+      The area is used to exchange additional information not covered by SCMI.
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - '#mbox-cells'
+  - shmem
+
+additionalProperties: false
+
+examples:
+  - |
+    soc@0 {
+      reg = <0 0>;
+       #address-cells = <2>;
+       #size-cells = <2>;
+
+      sram@36,0 {
+        compatible = "cpc-shmem";
+        reg = <0x86d0 0xdd400 0 0x200>;
+        #address-cells = <2>;
+        #size-cells = <1>;
+        ranges = <0 0x0 0x86d0 0xdd400 0x200>;
+
+        scp_to_cpu0: scp-shmame@0 {
+          compatible = "cpc-shmem";
+          reg = <0x0 0x0 0x200>;
+        };
+      };
+
+      mailbox0: mailbox@28,0 {
+        #mbox-cells = <1>;
+        compatible = "marvell,mbox";
+        reg = <0xe000 0 0 0>;
+        shmem = <&scp_to_cpu0>;
+      };
+    };
+  - |
+    soc@1 {
+      reg = <1 0>;
+       #address-cells = <2>;
+       #size-cells = <2>;
+
+      sram@36,0 {
+        compatible = "cpc-shmem";
+        reg = <0x86d0 0xdd400 0 0x200>;
+        #address-cells = <2>;
+        #size-cells = <1>;
+        ranges = <0 0x0 0x86d0 0xdd400 0x200>;
+
+        scp_to_cpu1: scp-shmame@1 {
+          compatible = "cpc-shmem";
+          reg = <0x0 0x0 0x200>;
+        };
+      };
+
+      mailbox1: mailbox@82c000000000 {
+        #mbox-cells = <1>;
+        compatible = "marvell,mbox";
+        reg = <0x82c0 0x00000000 0x0 0x100000>;
+        interrupt-parent = <&gic0>;
+        interrupts = <0 59 1>;
+        shmem = <&scp_to_cpu1>;
+      };
+    };
-- 
2.17.1

Re: [PATCH 1/2] dt-bindings: mailbox : marvell,mbox: Add bindings

[Krzysztof Kozlowski]

On 14/07/2022 14:12, Wojciech Bartczak wrote:
> This patch adds device tree binding for Marvell Message Handling Unit
> (MHU) controller.
> 
> Signed-off-by: Wojciech Bartczak <wbartczak@marvell.com>
> ---
>  .../mailbox/marvell,mbox-mailbox.yaml         | 112 ++++++++++++++++++
>  1 file changed, 112 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mailbox/marvell,mbox-mailbox.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mailbox/marvell,mbox-mailbox.yaml b/Documentation/devicetree/bindings/mailbox/marvell,mbox-mailbox.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..d9a6e14dcb12da6c3a9db2dfafc075ccefa8711c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mailbox/marvell,mbox-mailbox.yaml
> @@ -0,0 +1,112 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mailbox/marvell,mbox-mailbox.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Marvell Message Handling Unit
> +
> +maintainers:
> +  - Wojciech Bartczak <wbartczak@marvell.com>
> +  - Piyush Malgujar <pmalgujar@marvell.com>
> +
> +description:
> +  The Marvell's Message Handling Unit is a mailbox controller
> +  with a single channel used to communicate with System Control Processor.
> +  Driver supports series of cn9x and cn10x SoC.


Driver is not related to bindings, remove implementation details.

> +  Sole purpose of the link is to exchange SCMI related data with SCP.
> +  The link has hardwired configuration, it uses simple notification scheme
> +  over shared memory block to push data back and forth.
> +  Interrupts used by mailbox may be configured in two ways,
> +  as SPI interrupts, then driver uses platform device forntend.
> +  Other way is to use PCI bus frontend with LPI interrupts.
> +
> +properties:
> +  compatible:
> +    items:

No "items", you have just one here, but see comment below.

> +      - const: marvell,mbox

Need SoC or model specific compatible. Generic fallbacks are accepted if
always prepended with specific compatible.

Are you sure that all, absolutely all, including ones made in 20 years,
of Marvell mboxes will be 100% compatible with "marvell,mbox"?

> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    items:
> +      - description: interrupt type
> +        const: 0
> +      - description: interrupt number
> +      - description: interrupt trigger type
> +        const: 1

This looks wrong. You just specified here three interrupts, which are
fixed... It does not make really sense.


> +
> +  '#mbox-cells':
> +    description: Index of the channel

Skip description, obvious.

> +    const: 1
> +
> +  shmem:
> +    description:
> +      List of phandle pointing to the shared memory area between SCP and AP.
> +      The area is used to exchange additional information not covered by SCMI.
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - '#mbox-cells'
> +  - shmem
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    soc@0 {
> +      reg = <0 0>;
> +       #address-cells = <2>;

Messed up indentation. While fixing it, convert to 4 space for DTS
example, as recommended.

> +       #size-cells = <2>;
> +
> +      sram@36,0 {
> +        compatible = "cpc-shmem";
> +        reg = <0x86d0 0xdd400 0 0x200>;
> +        #address-cells = <2>;
> +        #size-cells = <1>;
> +        ranges = <0 0x0 0x86d0 0xdd400 0x200>;
> +
> +        scp_to_cpu0: scp-shmame@0 {
> +          compatible = "cpc-shmem";
> +          reg = <0x0 0x0 0x200>;
> +        };
> +      };

Isn't smem a generic property and generic use case? If so, then drop
this part, unless it brings anything specific to your mailbox example.

> +
> +      mailbox0: mailbox@28,0 {
> +        #mbox-cells = <1>;

First compatible, then reg then the rest of properties.

> +        compatible = "marvell,mbox";
> +        reg = <0xe000 0 0 0>;
> +        shmem = <&scp_to_cpu0>;
> +      };
> +    };
> +  - |
> +    soc@1 {
> +      reg = <1 0>;
> +       #address-cells = <2>;
> +       #size-cells = <2>;
> +
> +      sram@36,0 {
> +        compatible = "cpc-shmem";
> +        reg = <0x86d0 0xdd400 0 0x200>;
> +        #address-cells = <2>;
> +        #size-cells = <1>;
> +        ranges = <0 0x0 0x86d0 0xdd400 0x200>;
> +
> +        scp_to_cpu1: scp-shmame@1 {
> +          compatible = "cpc-shmem";
> +          reg = <0x0 0x0 0x200>;
> +        };
> +      };
> +
> +      mailbox1: mailbox@82c000000000 {
> +        #mbox-cells = <1>;
> +        compatible = "marvell,mbox";
> +        reg = <0x82c0 0x00000000 0x0 0x100000>;
> +        interrupt-parent = <&gic0>;
> +        interrupts = <0 59 1>;

These look like GIC and interrupt flags, so definitely not three
interrupts. Use proper defines.


> +        shmem = <&scp_to_cpu1>;

This is the same example as before, drop it or merge with previous.

> +      };
> +    };


Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: mailbox : marvell,mbox: Add bindings

[Rob Herring]

On Thu, 14 Jul 2022 05:12:12 -0700, Wojciech Bartczak wrote:
> This patch adds device tree binding for Marvell Message Handling Unit
> (MHU) controller.
> 
> Signed-off-by: Wojciech Bartczak <wbartczak@marvell.com>
> ---
>  .../mailbox/marvell,mbox-mailbox.yaml         | 112 ++++++++++++++++++
>  1 file changed, 112 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mailbox/marvell,mbox-mailbox.yaml
> 

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/linux-dt-review/Documentation/devicetree/bindings/mailbox/marvell,mbox-mailbox.yaml: properties: 'mboxes' is a dependency of 'shmem'
	from schema $id: http://devicetree.org/meta-schemas/mailbox.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mailbox/marvell,mbox-mailbox.yaml: properties: 'mboxes' is a dependency of 'shmem'
	from schema $id: http://devicetree.org/meta-schemas/mailbox.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mailbox/marvell,mbox-mailbox.yaml: ignoring, error in schema: properties
Documentation/devicetree/bindings/mailbox/marvell,mbox-mailbox.example.dtb:0:0: /example-0/soc@0/sram@36,0: failed to match any schema with compatible: ['cpc-shmem']
Documentation/devicetree/bindings/mailbox/marvell,mbox-mailbox.example.dtb:0:0: /example-0/soc@0/sram@36,0/scp-shmame@0: failed to match any schema with compatible: ['cpc-shmem']
Documentation/devicetree/bindings/mailbox/marvell,mbox-mailbox.example.dtb:0:0: /example-0/soc@0/mailbox@28,0: failed to match any schema with compatible: ['marvell,mbox']
Documentation/devicetree/bindings/mailbox/marvell,mbox-mailbox.example.dtb:0:0: /example-1/soc@1/sram@36,0: failed to match any schema with compatible: ['cpc-shmem']
Documentation/devicetree/bindings/mailbox/marvell,mbox-mailbox.example.dtb:0:0: /example-1/soc@1/sram@36,0/scp-shmame@1: failed to match any schema with compatible: ['cpc-shmem']
Documentation/devicetree/bindings/mailbox/marvell,mbox-mailbox.example.dtb:0:0: /example-1/soc@1/mailbox@82c000000000: failed to match any schema with compatible: ['marvell,mbox']

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

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.

Re: [PATCH 1/2] dt-bindings: mailbox : marvell,mbox: Add bindings

[Jassi Brar]

On Thu, Jul 14, 2022 at 7:13 AM Wojciech Bartczak <wbartczak@marvell.com> wrote:

...
> +description:
> +  The Marvell's Message Handling Unit is a mailbox controller
> +  with a single channel used to communicate with System Control Processor.
> +  Driver supports series of cn9x and cn10x SoC.
> +  Sole purpose of the link is to exchange SCMI related data with SCP.
> +  The link has hardwired configuration, it uses simple notification scheme
> +  over shared memory block to push data back and forth.
> +  Interrupts used by mailbox may be configured in two ways,
> +  as SPI interrupts, then driver uses platform device forntend.
> +  Other way is to use PCI bus frontend with LPI interrupts.
> +
Also have a provision of SPI vs LPI mode hint via DT.

...
> +
> +examples:
> +  - |
> +    soc@0 {
> +      reg = <0 0>;
> +       #address-cells = <2>;
> +       #size-cells = <2>;
> +
> +      sram@36,0 {
> +        compatible = "cpc-shmem";
> +        reg = <0x86d0 0xdd400 0 0x200>;
> +        #address-cells = <2>;
> +        #size-cells = <1>;
> +        ranges = <0 0x0 0x86d0 0xdd400 0x200>;
> +
> +        scp_to_cpu0: scp-shmame@0 {
>
Just curious, what does 'scp-shmame' stand for?

thanks.