Message-ID: <b4216bc75530702e9e0bb951f566c04e959ac7a6.camel@icenowy.me>
Date: Thu, 14 Aug 2025 15:34:27 +0800
From: Icenowy Zheng <uwu@...nowy.me>
To: Krzysztof Kozlowski <krzk@...nel.org>, 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

在 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?

> 
> > 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.

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?

> 
> > +
> >    '#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@...fc38000 {
> > +      mbox_c910t: mailbox@...fc38000 {
> 
> No, don't add unused labels. This is not improving the binding.

Sorry.

> 
> 
> Best regards,
> Krzysztof

Powered by blists - more mailing lists