* Re: [PATCH 1/2] dt-bindings: mailbox : marvell,mbox: Add bindings
2022-07-14 12:12 ` [PATCH 1/2] dt-bindings: mailbox : marvell,mbox: Add bindings Wojciech Bartczak
@ 2022-07-14 12:33 ` Krzysztof Kozlowski
2022-07-14 15:58 ` Rob Herring
2022-07-17 14:33 ` Jassi Brar
2 siblings, 0 replies; 4+ messages in thread
From: Krzysztof Kozlowski @ 2022-07-14 12:33 UTC (permalink / raw)
To: Wojciech Bartczak, linux-kernel
Cc: wbartczak, Piyush Malgujar, Sunil Goutham, Jassi Brar,
Rob Herring, Krzysztof Kozlowski, devicetree
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
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH 1/2] dt-bindings: mailbox : marvell,mbox: Add bindings
2022-07-14 12:12 ` [PATCH 1/2] dt-bindings: mailbox : marvell,mbox: Add bindings Wojciech Bartczak
2022-07-14 12:33 ` Krzysztof Kozlowski
@ 2022-07-14 15:58 ` Rob Herring
2022-07-17 14:33 ` Jassi Brar
2 siblings, 0 replies; 4+ messages in thread
From: Rob Herring @ 2022-07-14 15:58 UTC (permalink / raw)
To: Wojciech Bartczak
Cc: Rob Herring, devicetree, Sunil Goutham, Piyush Malgujar,
Jassi Brar, Krzysztof Kozlowski, linux-kernel, wbartczak
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.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] dt-bindings: mailbox : marvell,mbox: Add bindings
2022-07-14 12:12 ` [PATCH 1/2] dt-bindings: mailbox : marvell,mbox: Add bindings Wojciech Bartczak
2022-07-14 12:33 ` Krzysztof Kozlowski
2022-07-14 15:58 ` Rob Herring
@ 2022-07-17 14:33 ` Jassi Brar
2 siblings, 0 replies; 4+ messages in thread
From: Jassi Brar @ 2022-07-17 14:33 UTC (permalink / raw)
To: Wojciech Bartczak
Cc: Linux Kernel Mailing List, wbartczak, Piyush Malgujar,
Sunil Goutham, Rob Herring, Krzysztof Kozlowski, Devicetree List
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.
^ permalink raw reply [flat|nested] 4+ messages in thread