Re: [RFC PATCH 1/4] dt-bindings: mailbox: thead,th1520-mbox: retrofit for other mailboxes

[Krzysztof Kozlowski <krzk@kernel.org>]

On 14/08/2025 09:07, Icenowy Zheng wrote:
> The current binding of thead,th1520-mbox can only apply to the C910T
> mailbox (which has an ID of 0).
> 
> Because of the weird mailbox register mapping practice for world
> seperation on TH1520, the binding needs some reword, in addition to add
> a property for mailbox ID, to describe other mailboxes.
> 
> Update the binding, in order to make it suitable to describe other

But I do not see any new device being added.

> mailboxes. The example is also updated, with an addition of mbox_c910t
> label to show that the example describes this specfiic mailbox, mailbox
> ID added and the register window sizes updated to the values from the
> manual (previously the remote-icu0 register windows is declared to be
> overly small that it would never work).
> 
> Signed-off-by: Icenowy Zheng <uwu@icenowy.me>
> ---
>  .../bindings/mailbox/thead,th1520-mbox.yaml   | 49 ++++++++++++++-----
>  1 file changed, 36 insertions(+), 13 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mailbox/thead,th1520-mbox.yaml b/Documentation/devicetree/bindings/mailbox/thead,th1520-mbox.yaml
> index 0971fb97896ef..5a24d2e8a6a8c 100644
> --- a/Documentation/devicetree/bindings/mailbox/thead,th1520-mbox.yaml
> +++ b/Documentation/devicetree/bindings/mailbox/thead,th1520-mbox.yaml
> @@ -12,6 +12,17 @@ description:
>    through mailbox channels. It also allows one core to signal another processor
>    using interrupts via the Interrupt Controller Unit (ICU).
>  
> +  The SoC is divided to two worlds, REE and TEE, although it's currently unknown
> +  how to enable the seperation between worlds so the seperation does not exist
> +  yet. However each mailbox is assigned to a certain world, and register windows
> +  for mailboxes are assigned to different worlds too. In a certain world's
> +  register windows for mailboxes, only mailboxes assigned to this world will
> +  have the local ICU part mapped (in addition to the remote ICU part of the
> +  other same-world mailbox), and mailboxes assigned to the other world have
> +  only the coressponding remote ICU part mapped to this world. Two mailboxes
> +  (C910T and E902) are assigned to the TEE world and two mailboxes (C906 and
> +  C910R) are assigned to the REE world.
> +
>  maintainers:
>    - Michal Wilczynski <m.wilczynski@samsung.com>
>  
> @@ -22,9 +33,9 @@ properties:
>    clocks:
>      items:
>        - description: Clock for the local mailbox
> -      - description: Clock for remote ICU 0
> -      - description: Clock for remote ICU 1
> -      - description: Clock for remote ICU 2
> +      - description: Clock for the other mailbox in the same world
> +      - description: Clock for the first mailbox in the other world
> +      - description: Clock for the second mailbox in the other world
>  
>    clock-names:
>      items:
> @@ -35,10 +46,14 @@ properties:
>  
>    reg:
>      items:
> -      - description: Mailbox local base address
> -      - description: Remote ICU 0 base address
> -      - description: Remote ICU 1 base address
> -      - description: Remote ICU 2 base address
> +      - description: Base address of this specific mailbox
> +      - description: Base address of the other mailbox in the same world
> +      - description:
> +          Base address of the register window in this world corresponding to the
> +          first other-world mailbox.
> +      - description:
> +          Base address of the register window in this world corresponding to the
> +          second other-world mailbox.

This feels like ABI change.

>  
>    reg-names:
>      items:
> @@ -50,10 +65,17 @@ properties:
>    interrupts:
>      maxItems: 1
>  
> +  thead,mbox-id:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      The ID of this specific mailbox that this device tree node describes. For
> +      compatibility with old device trees, if missing, the ID is default to 0,
> +      the C910T mailbox.

No, you cannot have instance IDs. It's even explicitly documented in
writing bindings.

> +
>    '#mbox-cells':
>      const: 1
>      description:
> -      The one and only cell describes destination CPU ID.
> +      The one and only cell describes destination mailbox ID.
>  
>  required:
>    - compatible
> @@ -72,12 +94,12 @@ examples:
>      soc {
>        #address-cells = <2>;
>        #size-cells = <2>;
> -      mailbox@ffffc38000 {
> +      mbox_c910t: mailbox@ffffc38000 {

No, don't add unused labels. This is not improving the binding.


Best regards,
Krzysztof