Re: [PATCH v2 1/3] dt-bindings: mailbox: add Sophgo cv18x SoCs mailbox

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On Tue, Jul 16, 2024 at 12:15 AM, Conor Dooley <conor@xxxxxxxxxx> wrote:
On Mon, Jul 15, 2024 at 10:56:38AM +0800, Inochi Amaoto wrote:
 On Mon, Jul 15, 2024 at 12:36:45AM GMT, Yuntao Dai wrote:
> Add devicetree bindings documentation for Sophgo cv18x SoCs mailbox
 >
 > Signed-off-by: Yuntao Dai <d1581209858@xxxxxxxx>
 > ---
> .../mailbox/sophgo,cv1800-mailbox.yaml | 61 +++++++++++++++++++
 >  1 file changed, 61 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mailbox/sophgo,cv1800-mailbox.yaml
 >
> diff --git a/Documentation/devicetree/bindings/mailbox/sophgo,cv1800-mailbox.yaml b/Documentation/devicetree/bindings/mailbox/sophgo,cv1800-mailbox.yaml
 > new file mode 100644
 > index 000000000..05099d819
 > --- /dev/null
> +++ b/Documentation/devicetree/bindings/mailbox/sophgo,cv1800-mailbox.yaml
 > @@ -0,0 +1,61 @@
 > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
 > +%YAML 1.2
 > +---
> +$id: http://devicetree.org/schemas/mailbox/sophgo,cv1800-mailbox.yaml#
 > +$schema: http://devicetree.org/meta-schemas/core.yaml#
 > +
 > +title: Sophgo cv1800 mailbox controller
 > +
 > +maintainers:
 > +  - Yuntao Dai <d1581209858@xxxxxxxx>
 > +
 > +description:
> + The Sophgo cv18x SoCs mailbox has 8 channels and 8 bytes per channel for > + different processors. Any processer can write data in a channel, and > + set co-responding register to raise interrupt to notice another processor,
 > +  and it is allowed to send data to itself.
 > +  Sophgo cv18x SoCs has 3 processors and numbered as
 > +  <1> C906L
 > +  <2> C906B
 > +  <3> 8051
 > +

Unify the "cv18x" (at least it should be cv18xx) and cv1800 in your binding.

 > +properties:
 > +  compatible:
 > +    enum:
 > +      - sophgo,cv1800-mailbox
 > +
 > +  reg:
 > +    maxItems: 1
 > +
 > +  interrupts:
 > +    maxItems: 1
 > +
 > +  interrupt-names:
 > +    const: mailbox

 I think this is not necessary, there is only one interrupt.

 > +
 > +  "#mbox-cells":
 > +    const: 2
 > +    description:
> + The first cell indicates which channel is used, the second cell indicates
 > +      sending to which processor
 > +
 > +required:
 > +  - compatible
 > +  - reg
 > +  - interrupts
 > +  - "#mbox-cells"
 > +
 > +additionalProperties: false
 > +
 > +examples:
 > +  - |
 > +    #include <dt-bindings/interrupt-controller/irq.h>
 > +
 > +    mailbox: mailbox@1900000 {

And while you're fixing the things Inochi pointed out, drop the
"mailbox:" label, you've got no references to it.


Yes, in this patch there is no references to it, but mailbox client node will need it.
I tested this patch using mailbox-test as the client, like:
"
mailbox-test {
	compatible = "mailbox-test";
	mbox-names = "rx", "tx";
	mboxes = <&mailbox 0 1 &mailbox 1 2>;
};
"

If you think it is better to drop the label in this patch, I will drop it.

 > +        compatible = "sophgo,cv1800-mailbox";
 > +        reg = <0x01900000 0x1000>;
 > +        interrupts = <101 IRQ_TYPE_LEVEL_HIGH>;
 > +        interrupt-names = "mailbox";
 > +        interrupt-parent = <&plic>;

 Remove "interrupt-parent".

 > +        #mbox-cells = <2>;
 > +    };
 > --
 > 2.17.1
 >





[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux