[v2] Add support for Marvell MHU on CN9x and CN10x SoC

[PATCH v2 2/2] Documentation: add Marvell MHU driver bindings

Posted by Wojciech Zmuda 3 years, 1 month ago

From: Wojciech Bartczak <wbartczak@marvell.com>

Marvell Message Handling Unit is a mailbox controller present in
Marvell OcteonTx and OcteonTX2 SoC family.

Signed-off-by: Wojciech Bartczak <wbartczak@marvell.com>
Signed-off-by: Wojciech Zmuda <wzmuda@marvell.com>
---
 .../bindings/mailbox/marvell,mvl-mhu.yml      | 67 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 2 files changed, 68 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mailbox/marvell,mvl-mhu.yml

diff --git a/Documentation/devicetree/bindings/mailbox/marvell,mvl-mhu.yml b/Documentation/devicetree/bindings/mailbox/marvell,mvl-mhu.yml
new file mode 100644
index 000000000000..e06a17eab0f9
--- /dev/null
+++ b/Documentation/devicetree/bindings/mailbox/marvell,mvl-mhu.yml
@@ -0,0 +1,67 @@
+# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mailbox/marvell,mvl-mhu.yml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Marvell Message Handling Unit driver
+
+maintainers:
+  - Sujeet Baranwal <sbaranwal@marvell.com>
+  - Sunil Goutham <sgoutham@marvell.com>
+  - Wojciech Bartczak <wbartczak@marvell.com>
+
+description:
+  The Control-Processors Cluster (CPC) provides Arm-platform specification
+  entities for managing the system. On of the CPC processors is the System
+  Control Processor (SCP). The SCP is responsible, among others, for booting
+  the chip, clock and power initialization, controlling power consumption
+  through DVFS, monitoring temperature sensors and controlling AVS. The SCP,
+  as each XCP, contains mailboxes for software-to-software communications.
+  Mailbox writes cause an interrupt to the local XCP core or to the AP.
+  This driver exposes AP-SCP Message Handling Unit to the system, providing
+  the mailbox communication mechanism to the system, with the intention
+  of plugging into the SCMI framework. It is designed to work with Marvell
+  OcteonTX and OcteonTX2-based platforms.
+  Mailbox has no other usage than SCMI communication. In case of
+  configurations running without SCMI support it should be disabled.
+
+properties:
+  compatible:
+    items:
+      - const: marvell,mbox
+
+  reg:
+    maxItems: 1
+
+  "#mbox-cells":
+    description: Index of the channel
+    const: 1
+
+required:
+  - "#mbox-cells"
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+		/ {
+			mailbox: mailbox@28,0 {
+				compatible = "marvell,mbox";
+				#mbox-cells = <1>;
+				reg = <0xE000 0 0 0 0>;/*  DEVFN = 0xE0 (1C:0) */
+			};
+
+			/* ... */
+
+			firmware {
+				scmi {
+					compatible = "arm,scmi";
+					mboxes = <&mailbox 0>;
+					mbox-names = "scp_ap";
+					/* ... */
+				};
+			};
+		};
+
diff --git a/MAINTAINERS b/MAINTAINERS
index 650a198cce24..e53f001a15c3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11328,6 +11328,7 @@ M:	Sunil Goutham <sgoutham@marvell.com>
 M:	Wojciech Bartczak <wbartczak@marvell.com>
 L:	linux-kernel@vger.kernel.org
 S:	Maintained
+F:	Documentation/devicetree/bindings/mailbox/marvell,mvl-mhu.yml
 F:	drivers/mailbox/mvl_mhu.c
 
 MATROX FRAMEBUFFER DRIVER
-- 
2.17.1

Re: [PATCH v2 2/2] Documentation: add Marvell MHU driver bindings

Posted by Krzysztof Kozlowski 3 years, 1 month ago

On 03/01/2023 16:56, Wojciech Zmuda wrote:
> From: Wojciech Bartczak <wbartczak@marvell.com>
> 
> Marvell Message Handling Unit is a mailbox controller present in
> Marvell OcteonTx and OcteonTX2 SoC family.

1. Use subject prefixes matching the subsystem (which you can get for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching).

2. Subject: drop second, redundant "bindings".

3. Subject: drop "driver", unless your piece of hardware is called driver?


> 
> Signed-off-by: Wojciech Bartczak <wbartczak@marvell.com>
> Signed-off-by: Wojciech Zmuda <wzmuda@marvell.com>
> ---
>  .../bindings/mailbox/marvell,mvl-mhu.yml      | 67 +++++++++++++++++++
>  MAINTAINERS                                   |  1 +
>  2 files changed, 68 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mailbox/marvell,mvl-mhu.yml
> 
> diff --git a/Documentation/devicetree/bindings/mailbox/marvell,mvl-mhu.yml b/Documentation/devicetree/bindings/mailbox/marvell,mvl-mhu.yml
> new file mode 100644
> index 000000000000..e06a17eab0f9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mailbox/marvell,mvl-mhu.yml

Filenames should be based on compatibles, e.g. be the same.

> @@ -0,0 +1,67 @@
> +# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mailbox/marvell,mvl-mhu.yml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Marvell Message Handling Unit driver

Drop driver

> +
> +maintainers:
> +  - Sujeet Baranwal <sbaranwal@marvell.com>
> +  - Sunil Goutham <sgoutham@marvell.com>
> +  - Wojciech Bartczak <wbartczak@marvell.com>
> +
> +description:
> +  The Control-Processors Cluster (CPC) provides Arm-platform specification
> +  entities for managing the system. On of the CPC processors is the System

On->One?

> +  Control Processor (SCP). The SCP is responsible, among others, for booting
> +  the chip, clock and power initialization, controlling power consumption
> +  through DVFS, monitoring temperature sensors and controlling AVS. The SCP,
> +  as each XCP, contains mailboxes for software-to-software communications.
> +  Mailbox writes cause an interrupt to the local XCP core or to the AP.
> +  This driver exposes AP-SCP Message Handling Unit to the system, providing
> +  the mailbox communication mechanism to the system, with the intention
> +  of plugging into the SCMI framework. It is designed to work with Marvell
> +  OcteonTX and OcteonTX2-based platforms.
> +  Mailbox has no other usage than SCMI communication. In case of
> +  configurations running without SCMI support it should be disabled.
> +
> +properties:
> +  compatible:
> +    items:
> +      - const: marvell,mbox

This is too generic. Are you sure that all Marvel mailboxes - past,
current and future - will be exactly same as this one. No differences
for next 100 years? IOW, compatible has to be specific to hardware (SoC,
device etc).

> +
> +  reg:
> +    maxItems: 1
> +
> +  "#mbox-cells":
> +    description: Index of the channel
> +    const: 1

No interrupts?

> +
> +required:
> +  - "#mbox-cells"
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +		/ {

Use 4 spaces for example indentation.

> +			mailbox: mailbox@28,0 {
> +				compatible = "marvell,mbox";
> +				#mbox-cells = <1>;
> +				reg = <0xE000 0 0 0 0>;/*  DEVFN = 0xE0 (1C:0) */

1. reg is a second property, after compatible.
2. unit address is wrong.
3. lowercase hex.
4. Two spaces after /* but missing space before.

> +			};
> +
> +			/* ... */
> +
> +			firmware {
> +				scmi {
> +					compatible = "arm,scmi";
> +					mboxes = <&mailbox 0>;

Drop entire firmware example, unrelated.

> +					mbox-names = "scp_ap";
> +					/* ... */

Best regards,
Krzysztof

[PATCH v2 1/2] mailbox: mvl-mhu: add OcteonTX2 MHU mailbox driver
[PATCH v2 2/2] Documentation: add Marvell MHU driver bindings