On Sat, Dec 21, 2024 at 12:45 AM Tudor Ambarus <tudor.ambarus@xxxxxxxxxx> wrote: > > Hi, Jassi, > > Thanks for the review! > > On 12/21/24 2:19 AM, Jassi Brar wrote: > > On Thu, Dec 19, 2024 at 4:51 AM Tudor Ambarus <tudor.ambarus@xxxxxxxxxx> wrote: > >> > >> Hi, Krzysztof, Jassi, > >> > >> On 12/17/24 9:40 AM, Tudor Ambarus wrote: > >> > >>> diff --git a/Documentation/devicetree/bindings/mailbox/google,gs101-mbox.yaml b/Documentation/devicetree/bindings/mailbox/google,gs101-mbox.yaml > >> > >> cut > >> > >>> + > >>> + '#mbox-cells': > >>> + description: | > >>> + <&phandle type channel> > >>> + phandle : label name of controller. > >>> + type : channel type, doorbell or data-transfer. > >>> + channel : channel number. > >>> + > >>> + Here is how a client can reference them: > >>> + mboxes = <&ap2apm_mailbox DOORBELL 2>; > >>> + mboxes = <&ap2apm_mailbox DATA 3>; > >>> + const: 2 > >>> + > >> > >> Revisiting this, I think that for the ACPM interface mailbox client use > >> case, it would be better to introduce a mbox property where I reference > >> just the phandle to the controller: > >> mbox = <&ap2apm_mailbox>; > >> > >> The ACPM interface discovers the mailbox channel IDs at runtime by > >> parsing SRAM. And all ACPM's channels are of type DOORBELL, thus > >> specifying the type and channel in DT is redundant. > >> > >> It would require to extend a bit the mailbox core to provide a > >> mbox_request_channel_by_args() method. I already wrote a draft and > >> tested it. > >> > >> Do you find the idea fine? > >> > > Looking at v6, I prefer this version... maybe modify it a bit. > > Just to summarize for the readers, in the end I chose for the > controllers to allow #mbox-cells = <0>; and for the clients to still use > the mboxes property, but just to reference the phandle to the controller: > mboxes = <&ap2apm_mailbox>; > This was already supported, see drivers/mailbox/bcm2835-mailbox.c for example. > Then I updated the mailbox core to allow clients to request channels by > passing some args containing channel identifiers to the controllers, > that the controllers xlate() using their own method. > This is unnecessary. If you don't pass the doorbell number from DT, each channel populated by the driver is just a s/w construct or a 'virtual' channel. Make use of 'void *data' in send_data() to specify the doorbell. Cheers. Jassi
