Re: [PATCH RFC v2 2/3] dt-bindings: mailbox: Add thead,th1520-mailbox bindings

[Michal Wilczynski]

On 9/28/24 00:32, Rob Herring wrote:
> On Fri, Sep 27, 2024 at 11:42:06AM +0200, Michal Wilczynski wrote:
>> Add bindings for the mailbox controller. This work is based on the vendor
>> kernel. [1]
>>
>> Link: https://protect2.fireeye.com/v1/url?k=5f89eff9-3e02facf-5f8864b6-74fe485cbff1-fe45b64bcc1d1a7f&q=1&e=ed6b721a-b6ae-4d7a-b1bf-4e0a7691061d&u=https%3A%2F%2Fgithub.com%2Frevyos%2Fthead-kernel.git [1]
>>
>> Signed-off-by: Michal Wilczynski <m.wilczynski@xxxxxxxxxxx>
>> ---
>>  .../bindings/mailbox/thead,th1520-mbox.yaml   | 84 +++++++++++++++++++
>>  MAINTAINERS                                   |  1 +
>>  2 files changed, 85 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/mailbox/thead,th1520-mbox.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/mailbox/thead,th1520-mbox.yaml b/Documentation/devicetree/bindings/mailbox/thead,th1520-mbox.yaml
>> new file mode 100644
>> index 000000000000..b517d0c6eb12
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mailbox/thead,th1520-mbox.yaml
>> @@ -0,0 +1,84 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +
>> +%YAML 1.2
>> +---
>> +$id: https://protect2.fireeye.com/v1/url?k=b15f6a95-d0d47fa3-b15ee1da-74fe485cbff1-a74b5ef04b0a5478&q=1&e=ed6b721a-b6ae-4d7a-b1bf-4e0a7691061d&u=http%3A%2F%2Fdevicetree.org%2Fschemas%2Fmailbox%2Fthead%2Cth1520-mbox.yaml%23
>> +$schema: https://protect2.fireeye.com/v1/url?k=8bf885f9-ea7390cf-8bf90eb6-74fe485cbff1-894b0d9aad275d0d&q=1&e=ed6b721a-b6ae-4d7a-b1bf-4e0a7691061d&u=http%3A%2F%2Fdevicetree.org%2Fmeta-schemas%2Fcore.yaml%23
>> +
>> +title: T-head TH1520 Mailbox Controller
>> +
>> +description:
>> +  The T-head mailbox controller enables communication and coordination between
>> +  cores within the SoC by passing messages (e.g., data, status, and control)
>> +  through mailbox channels. It also allows one core to signal another processor
>> +  using interrupts via the Interrupt Controller Unit (ICU).
>> +
>> +maintainers:
>> +  - Michal Wilczynski <m.wilczynski@xxxxxxxxxxx>
>> +
>> +properties:
>> +  compatible:
>> +    const: thead,th1520-mbox
>> +
>> +  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
>> +
>> +  reg-names:
>> +    items:
>> +      - const: local
>> +      - const: remote-icu0
>> +      - const: remote-icu1
>> +      - const: remote-icu2
>> +
>> +  interrupts:
>> +    maxItems: 1
>> +
>> +  thead,icu-cpu-id:
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    description: |
> 
> Don't need '|' if no formatting.
> 
>> +      CPU ID associated with the Interrupt Controller Unit (ICU). This ID should
>> +      correspond to a valid CPU core in the system. The value must be between 0
>> +      and (N - 1), where N is the total number of CPU cores in the SoC.
>> +    minimum: 0
>> +    maximum: 3
> 
> The normal way we associate a node with some subset of CPUs is the 
> 'cpus' property linking to the CPU node.
> 
> Or is this CPUs which are not part of what Linux is running on (i.e. the 
> SMP cores).

Well, for now, the only use case is talking to the processors which are not
managed by the Linux kernel, specifically to the E902 core, which helps with
power management and is able to power up the GPU power island.

The T-head manual states: "For the four CPU cores of C910T, E902, C906, and
C910R in this chip, there are four MBOX units whose corresponding CPU_IDX
are 0, 1, 2, and 3 respectively."

But on my system - the LicheePi 4A there are four C910T SMP cores on which
Linux is running, and those all share the same CPU_IDX of 0.

So using the cpu or cpus property wouldn't be correct here. I could drop the
thead,icu-cpu-id property altogether, as for now the only viable value of it
would be 0, and just hard-code the 0 in the driver as well.

> 
>> +
>> +  '#mbox-cells':
>> +    const: 2
>> +    description: |
>> +      Specifies the number of cells needed to encode the mailbox specifier.
>> +      The mailbox specifier consists of two cells:
>> +        - The first cell is the destination CPU ID.
>> +        - The second cell is the mailbox channel ID.
>> +
>> +additionalProperties: false
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - reg-names
>> +  - interrupts
>> +  - thead,icu-cpu-id
>> +  - '#mbox-cells'
>> +
>> +examples:
>> +  - |
>> +
>> +    soc {
>> +      #address-cells = <2>;
>> +      #size-cells = <2>;
>> +      mailbox@ffffc38000 {
>> +        compatible = "thead,th1520-mbox";
>> +        reg = <0xff 0xffc38000 0x0 0x4000>,
>> +              <0xff 0xffc44000 0x0 0x1000>,
>> +              <0xff 0xffc4c000 0x0 0x1000>,
>> +              <0xff 0xffc54000 0x0 0x1000>;
>> +        reg-names = "local", "remote-icu0", "remote-icu1", "remote-icu2";
>> +        interrupts = <28>;
>> +        thead,icu-cpu-id = <0>;
>> +        #mbox-cells = <2>;
>> +      };
>> +    };
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index df4d7be6cf35..a6028f850a25 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -19943,6 +19943,7 @@ L:	linux-riscv@xxxxxxxxxxxxxxxxxxx
>>  S:	Maintained
>>  T:	git https://protect2.fireeye.com/v1/url?k=b8a28277-d9299741-b8a30938-74fe485cbff1-54d1d6ede07254ed&q=1&e=ed6b721a-b6ae-4d7a-b1bf-4e0a7691061d&u=https%3A%2F%2Fgithub.com%2Fpdp7%2Flinux.git
>>  F:	Documentation/devicetree/bindings/clock/thead,th1520-clk-ap.yaml
>> +F:	Documentation/devicetree/bindings/mailbox/thead,th1520-mbox.yaml
>>  F:	arch/riscv/boot/dts/thead/
>>  F:	drivers/clk/thead/clk-th1520-ap.c
>>  F:	drivers/mailbox/mailbox-th1520.c
>> -- 
>> 2.34.1
>>
>