Message-ID: <ad3f03da-696d-4e87-a218-9c9f26f830f7@kernel.org>
Date: Thu, 14 Aug 2025 09:48:12 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Icenowy Zheng <uwu@...nowy.me>, Drew Fustini <fustini@...nel.org>,
 Guo Ren <guoren@...nel.org>, Fu Wei <wefu@...hat.com>,
 Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>, Jassi Brar <jassisinghbrar@...il.com>,
 Michal Wilczynski <m.wilczynski@...sung.com>
Cc: Han Gao <rabenda.cn@...il.com>, Inochi Amaoto <inochiama@...il.com>,
 Yao Zi <ziyao@...root.org>, linux-riscv@...ts.infradead.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 1/4] dt-bindings: mailbox: thead,th1520-mbox: retrofit
 for other mailboxes

On 14/08/2025 09:34, Icenowy Zheng wrote:
> 在 2025-08-14星期四的 09:18 +0200，Krzysztof Kozlowski写道：
>> 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.
> 
> See PATCH 3/4 in this patchset.
> 
> Or do you mean I need to add new compatibles as I said below?

I don't know... you say there are some other mailboxes (I understand as
mailbox controllers) but I don't see anything being added here.


> 
>>
>>> 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@...nowy.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@...sung.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.
> 
> The problem is that the mailbox cannot send to itself, so the "sending
> to self" slot is redefined to other meaning.

I don't know what is sending to self...

> 
> Or should I assign different compatible strings to all 4 mailboxes
> because they have different registers in the "sending to self" slot,
> which have different offset because of instance ID?

Commit msg is quite vague in describing hardware. It has a lot of text
but no answers to questions. What are the differences? What is the hardware?

if you have different programming model, then you have different
compatibles, usually.


Best regards,
Krzysztof

Powered by blists - more mailing lists