<snip> > > > > + mboxes: > > > > + description: > > > > + List of phandle of 2 MU channels for TXDB, 2 MU channels for RXDB > > > > + (see mailbox/fsl,mu.txt) > > > > + maxItems: 1 > > > > > > Should be 4? > > > > Actually is just a list with 1 item. I think is the terminology: > > > > You can have an example here of the mboxes defined for SCU. > > https://github.com/torvalds/linux/blob/master/arch/arm64/boot/dts/freescale/imx8qxp.dtsi#L123 > > mboxes = <&lsio_mu1 0 0 > &lsio_mu1 0 1 > &lsio_mu1 0 2 > &lsio_mu1 0 3 > &lsio_mu1 1 0 > &lsio_mu1 1 1 > &lsio_mu1 1 2 > &lsio_mu1 1 3 > &lsio_mu1 3 3>; > > Logically, this is 9 entries and each entry is 3 cells ( or phandle > plus 2 cells). More below... Ok.. > > > > > + > > > > + mbox-names > > Also, missing a ':' here. This won't build. Make sure you build this > (make dt_binding_check). See > Documentation/devicetree/writing-schemas.md. > Fixed in v2. Awesome! I thought that Documentation/devicetree/bindings/dsp/fsl,dsp_ipc.yaml is purely decorative and used as an example. But it's actually the schema for the newly yaml dts, right? Used make dt_binding_check everything looks OK now. > > > > + description: > > > > + Mailboxes names > > > > + allOf: > > > > + - $ref: "/schemas/types.yaml#/definitions/string" > > > > > > No need for this, '*-names' already has a defined type. > > So, should I remove the above two lines ? > > Actually, all 4. There's no need to describe what 'mbox-names' is. > > > > > + - enum: [ "txdb0", "txdb1", "rxdb0", "rxdb1" ] > > > > > > Should be an 'items' list with 4 entries? > > > > Let me better read the yaml spec. But "items" list indeed sounds better. > > What you should end up with is: > > items: > - const: txdb0 > - const: txdb1 > - const: rxdb0 > - const: rxdb1 > > This is saying you have 4 strings in the listed order. The enum you > had would be a single string of one of the 4 values. > I see! Thanks. > > > > +required: > > > > + - compatible > > > > + - mboxes > > > > + - mbox-names > > > > > > This seems incomplete. How does one boot the DSP? Load firmware? No > > > resources that Linux has to manage. Shared memory? > > > > This is only the IPC mailboxes used by DSP to communicate with Linux. The > > loading of the firmware, the resources needed to be managed by Linux, etc > > are part of the DSP node. > > You should just add the mailboxes to the DSP node then. I suppose you > didn't because you want 2 drivers? If so, that's the OS's problem and > not part of DT. A Linux driver can instantiate devices for other > drivers. Yes, I want the DSP IPC driver to be separated. And then the SOF Linux driver that needs to communicate with DSP just gets a handle to DSP IPC driver and does the communication. dts relevant nodes look like this now: » dsp_ipc: dsp_ipc { » » compatible = "fsl,imx8qxp-dsp"; » » mbox-names = "txdb0", "txdb1", » » » "rxdb0", "rxdb1"; » » mboxes = <&lsio_mu13 2 0>, » » » <&lsio_mu13 2 1>, » » » <&lsio_mu13 3 0>, » » » <&lsio_mu13 3 1>; » }; » adma_dsp: dsp@596e8000 { » » compatible = "fsl,imx8qxp-sof-dsp"; » » reg = <0x596e8000 0x88000>; » » reserved-region = <&dsp_reserved>; » » ipc = <&dsp_ipc>; » }; Your suggeston would be to have something like this: » adma_dsp: dsp@596e8000 { » » compatible = "fsl,imx8qxp-sof-dsp"; » » reg = <0x596e8000 0x88000>; » » reserved-region = <&dsp_reserved>; » mbox-names = "txdb0", "txdb1", » » » "rxdb0", "rxdb1"; » » mboxes = <&lsio_mu13 2 0>, » » » <&lsio_mu13 2 1>, » » » <&lsio_mu13 3 0>, » » » <&lsio_mu13 3 1>; » }; Not sure exactly how to instantiate IPC DSP driver then. I already have prepared v2 with most of your feedback incorporated, but not this latest change with moving mboxes inside dsp driver. More than that I have followed the model of SCFW IPC and having to different approach for similar IPC mechanism is a little bit confusing. Anyhow, will try to address your further feedback, will send v2 now to have more feedback from Oleksij. thanks, Daniel.
