1. Patchew
  2. linux
  3. [PATCH 0/2] mailbox: Add support for bcm74110 mailbox
  4. View patch

[PATCH 2/2] dt-bindings: mailbox: Add devicetree binding for bcm74110 mbox

justin.chen@broadcom.com posted 2 patches 8 months, 3 weeks ago
There is a newer version of this series
[PATCH 2/2] dt-bindings: mailbox: Add devicetree binding for bcm74110 mbox
Posted by justin.chen@broadcom.com 8 months, 3 weeks ago 
From: Justin Chen <justin.chen@broadcom.com>

Add devicetree YAML binding for brcmstb bcm74110 mailbox used
for communicating with a co-processor.

Signed-off-by: Justin Chen <justin.chen@broadcom.com>
---
 .../bindings/mailbox/brcm,bcm74110-mbox.yaml  | 68 +++++++++++++++++++
 1 file changed, 68 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mailbox/brcm,bcm74110-mbox.yaml

diff --git a/Documentation/devicetree/bindings/mailbox/brcm,bcm74110-mbox.yaml b/Documentation/devicetree/bindings/mailbox/brcm,bcm74110-mbox.yaml
new file mode 100644
index 000000000000..139728a35303
--- /dev/null
+++ b/Documentation/devicetree/bindings/mailbox/brcm,bcm74110-mbox.yaml
@@ -0,0 +1,68 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mailbox/brcm,bcm74110-mbox.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Broadcom BCM74110 Mailbox Driver
+
+maintainers:
+  - Justin Chen <justin.chen@broadcom.com>
+  - Florian Fainelli <florian.fainelli@broadcom.com>
+
+description: Broadcom mailbox driver first introduced with 74110
+
+properties:
+  compatible:
+    enum:
+      - brcm,bcm74110-mbox
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  "#mbox-cells":
+    const: 2
+    description:
+      The first cell is channel type and second cell is shared memory slot
+
+  brcm,mbox_tx:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: Mailbox transmit doorbell
+
+  brcm,mbox_rx:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: Mailbox receive doorbell
+
+  brcm,mbox_shmem:
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    maxItems: 2
+    description: Mailbox shared memory region and size
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - "#mbox-cells"
+  - brcm,mbox_tx
+  - brcm,mbox_rx
+  - brcm,mbox_shmem
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+        brcm_pmc_mailbox: brcm_pmc_mailbox@a552000 {
+                #mbox-cells = <2>;
+                compatible = "brcm,bcm74110-mbox";
+                reg = <0xa552000 0x1100>;
+                brcm,mbox_tx = <0x6>;
+                brcm,mbox_rx = <0x7>;
+                brcm,mbox_shmem = <0x3000 0x400>;
+                interrupts = <0x0 0x67 0x4>;
+                interrupt-parent = <&intc>;
+        };
-- 
2.34.1
Re: [PATCH 2/2] dt-bindings: mailbox: Add devicetree binding for bcm74110 mbox
Posted by Krzysztof Kozlowski 8 months, 3 weeks ago 
On 27/03/2025 23:16, justin.chen@broadcom.com wrote:
> From: Justin Chen <justin.chen@broadcom.com>
> 
> Add devicetree YAML binding for brcmstb bcm74110 mailbox used
> for communicating with a co-processor.
> 
> Signed-off-by: Justin Chen <justin.chen@broadcom.com>

Bindings are before users, see DT submitting patches.

> ---
>  .../bindings/mailbox/brcm,bcm74110-mbox.yaml  | 68 +++++++++++++++++++
>  1 file changed, 68 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mailbox/brcm,bcm74110-mbox.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mailbox/brcm,bcm74110-mbox.yaml b/Documentation/devicetree/bindings/mailbox/brcm,bcm74110-mbox.yaml
> new file mode 100644
> index 000000000000..139728a35303
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mailbox/brcm,bcm74110-mbox.yaml
> @@ -0,0 +1,68 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mailbox/brcm,bcm74110-mbox.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Broadcom BCM74110 Mailbox Driver

Driver as Linux driver? Drop, bindings describe hardware.

> +
> +maintainers:
> +  - Justin Chen <justin.chen@broadcom.com>
> +  - Florian Fainelli <florian.fainelli@broadcom.com>
> +
> +description: Broadcom mailbox driver first introduced with 74110

Same comments.

> +
> +properties:
> +  compatible:
> +    enum:
> +      - brcm,bcm74110-mbox
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  "#mbox-cells":
> +    const: 2
> +    description:
> +      The first cell is channel type and second cell is shared memory slot
> +
> +  brcm,mbox_tx:

No underscores. See DTS coding style.

> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Mailbox transmit doorbell

Why is this needed in DT? How many instances do you have in one SoC?
Where is the SoC DTS?

> +
> +  brcm,mbox_rx:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Mailbox receive doorbell
> +
> +  brcm,mbox_shmem:
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    maxItems: 2
> +    description: Mailbox shared memory region and size

No, use existing properties, e.g. memory region.

> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - "#mbox-cells"
> +  - brcm,mbox_tx
> +  - brcm,mbox_rx
> +  - brcm,mbox_shmem
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +        brcm_pmc_mailbox: brcm_pmc_mailbox@a552000 {

Use indentation we expect. See writing schema, example-schema.

> +                #mbox-cells = <2>;
> +                compatible = "brcm,bcm74110-mbox";

Fix order, see DTS coding style.



Best regards,
Krzysztof
Re: [PATCH 2/2] dt-bindings: mailbox: Add devicetree binding for bcm74110 mbox
Posted by Justin Chen 8 months, 3 weeks ago 

On 3/28/25 12:31 AM, Krzysztof Kozlowski wrote:
> On 27/03/2025 23:16, justin.chen@broadcom.com wrote:
>> From: Justin Chen <justin.chen@broadcom.com>
>>
>> Add devicetree YAML binding for brcmstb bcm74110 mailbox used
>> for communicating with a co-processor.
>>
>> Signed-off-by: Justin Chen <justin.chen@broadcom.com>
> 
> Bindings are before users, see DT submitting patches.
> 
>> ---
>>   .../bindings/mailbox/brcm,bcm74110-mbox.yaml  | 68 +++++++++++++++++++
>>   1 file changed, 68 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/mailbox/brcm,bcm74110-mbox.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/mailbox/brcm,bcm74110-mbox.yaml b/Documentation/devicetree/bindings/mailbox/brcm,bcm74110-mbox.yaml
>> new file mode 100644
>> index 000000000000..139728a35303
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mailbox/brcm,bcm74110-mbox.yaml
>> @@ -0,0 +1,68 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/mailbox/brcm,bcm74110-mbox.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Broadcom BCM74110 Mailbox Driver
> 
> Driver as Linux driver? Drop, bindings describe hardware.
> 
>> +
>> +maintainers:
>> +  - Justin Chen <justin.chen@broadcom.com>
>> +  - Florian Fainelli <florian.fainelli@broadcom.com>
>> +
>> +description: Broadcom mailbox driver first introduced with 74110
> 
> Same comments.
> 
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - brcm,bcm74110-mbox
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  interrupts:
>> +    maxItems: 1
>> +
>> +  "#mbox-cells":
>> +    const: 2
>> +    description:
>> +      The first cell is channel type and second cell is shared memory slot
>> +
>> +  brcm,mbox_tx:
> 
> No underscores. See DTS coding style.
> 

Acked. I already had this fixed in the driver, but not in the doc. Woops!

>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    description: Mailbox transmit doorbell
> 
> Why is this needed in DT? How many instances do you have in one SoC?
> Where is the SoC DTS?
> 

We have 3 possible instances in our current SoC. We currently only 
implement one. arm,scmi. But more will come in the future. I'll put a 
sample arm,scmi node as an example consumer in v2.

>> +
>> +  brcm,mbox_rx:
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    description: Mailbox receive doorbell
>> +
>> +  brcm,mbox_shmem:
>> +    $ref: /schemas/types.yaml#/definitions/uint32-array
>> +    maxItems: 2
>> +    description: Mailbox shared memory region and size
> 
> No, use existing properties, e.g. memory region.
> 

This is a region from the on chip memory. I will rename to be clear. It 
lies in a different address space as reserved-memory. I can still use 
memory region if you prefer, but I will need to manually strip the 
offset in my driver as the API agreed upon with the FW is an offset from 
the beginning of on chip memory. IMHO this makes things unnecessarily 
complicated.

>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - interrupts
>> +  - "#mbox-cells"
>> +  - brcm,mbox_tx
>> +  - brcm,mbox_rx
>> +  - brcm,mbox_shmem
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/interrupt-controller/irq.h>
>> +
>> +        brcm_pmc_mailbox: brcm_pmc_mailbox@a552000 {
> 
> Use indentation we expect. See writing schema, example-schema.
> 
>> +                #mbox-cells = <2>;
>> +                compatible = "brcm,bcm74110-mbox";
> 
> Fix order, see DTS coding style.
> 
> 
> 

Thanks for the review.
Justin

> Best regards,
> Krzysztof
Re: [PATCH 2/2] dt-bindings: mailbox: Add devicetree binding for bcm74110 mbox
Posted by Krzysztof Kozlowski 8 months, 3 weeks ago 
On 28/03/2025 19:36, Justin Chen wrote:
>>> +    const: 2
>>> +    description:
>>> +      The first cell is channel type and second cell is shared memory slot
>>> +
>>> +  brcm,mbox_tx:
>>
>> No underscores. See DTS coding style.
>>
> 
> Acked. I already had this fixed in the driver, but not in the doc. Woops!

So this did not even work and you did not test DTS.

Where is the DTS?

> 
>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>> +    description: Mailbox transmit doorbell
>>
>> Why is this needed in DT? How many instances do you have in one SoC?
>> Where is the SoC DTS?
>>
> 
> We have 3 possible instances in our current SoC. We currently only 
> implement one. arm,scmi. But more will come in the future. I'll put a 
> sample arm,scmi node as an example consumer in v2.

No. Post your DTS instead.

> 
>>> +
>>> +  brcm,mbox_rx:
>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>> +    description: Mailbox receive doorbell
>>> +
>>> +  brcm,mbox_shmem:
>>> +    $ref: /schemas/types.yaml#/definitions/uint32-array
>>> +    maxItems: 2
>>> +    description: Mailbox shared memory region and size
>>
>> No, use existing properties, e.g. memory region.
>>
> 
> This is a region from the on chip memory. I will rename to be clear. It 

memory? so reserved region is for what? Not memory?

If this is memory, then reserved region. If this is not memory, but
MMIO, then you have reg for that.


Best regards,
Krzysztof

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.