| Message ID | 20241219-mbox_request_channel_by_args-v1-1-617a6910f842@linaro.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mailbox: add support for clients to request channels by arguments | expand |
On Thu, Dec 19, 2024 at 01:07:46PM +0000, Tudor Ambarus wrote: > There are mailbox clients that can discover the mailbox channel ID at > run-time. For such cases passing the channel identifier via DT is > redundant. Add support for referencing controllers solely by node. I don't really get your implementation, why not just allow #mbox-cells = 0? That's what's done for things like fixed frequency clocks that only have a single output. Cheers, Conor. > > Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org> > --- > Documentation/devicetree/bindings/mailbox/mailbox.txt | 19 +++++++++++++++---- > 1 file changed, 15 insertions(+), 4 deletions(-) > > diff --git a/Documentation/devicetree/bindings/mailbox/mailbox.txt b/Documentation/devicetree/bindings/mailbox/mailbox.txt > index af8ecee2ac68..0c4295a62f61 100644 > --- a/Documentation/devicetree/bindings/mailbox/mailbox.txt > +++ b/Documentation/devicetree/bindings/mailbox/mailbox.txt > @@ -5,9 +5,10 @@ assign appropriate mailbox channel to client drivers. > > * Mailbox Controller > > -Required property: > -- #mbox-cells: Must be at least 1. Number of cells in a mailbox > - specifier. > +Optional property: > +- #mbox-cells: Must be at least 1. Number of cells in a mailbox specifier. > + The property becomes mandatory for the cases where the clients > + reference the controller via the mboxes property. > > Example: > mailbox: mailbox { > @@ -19,7 +20,11 @@ Example: > * Mailbox Client > > Required property: > +Clients must reference the mailbox controller either via the mboxes or mbox > +properties. > - mboxes: List of phandle and mailbox channel specifiers. > +- mbox: phandle pointing to the controller. Used by clients that can discover > + the channel identifiers at runtime. > > Optional property: > - mbox-names: List of identifier strings for each mailbox channel. > @@ -29,7 +34,13 @@ Optional property: > communication between the mailbox client and the remote. > > > -Example: > +Example using mbox: > + power-management { > + ... > + mbox = <&mailbox>; > + }; > + > +Example using mboxes: > pwr_cntrl: power { > ... > mbox-names = "pwr-ctrl", "rpc"; > > -- > 2.47.1.613.gc27f4b7a9f-goog >
Hi, Conor, On 12/19/24 2:11 PM, Conor Dooley wrote: >> There are mailbox clients that can discover the mailbox channel ID at >> run-time. For such cases passing the channel identifier via DT is >> redundant. Add support for referencing controllers solely by node. > I don't really get your implementation, why not just allow #mbox-cells = 0? > That's what's done for things like fixed frequency clocks that only have > a single output. Ah, indeed! instead of: of_parse_phandle(dev->of_node, "mbox", 0); I can do a: of_parse_phandle_with_args(dev->of_node, "mboxes", "#mbox-cells", 0, &of_args) where #mbox-cells = 0; Or ... can I pass NULL for cells_name and make the #mbox-cells property optional and still keeping its requirement of being at least 1? Thanks! ta
On Thu, Dec 19, 2024 at 03:42:11PM +0000, Tudor Ambarus wrote: > Hi, Conor, > > On 12/19/24 2:11 PM, Conor Dooley wrote: > >> There are mailbox clients that can discover the mailbox channel ID at > >> run-time. For such cases passing the channel identifier via DT is > >> redundant. Add support for referencing controllers solely by node. > > I don't really get your implementation, why not just allow #mbox-cells = 0? > > That's what's done for things like fixed frequency clocks that only have > > a single output. > > Ah, indeed! > > instead of: > of_parse_phandle(dev->of_node, "mbox", 0); > I can do a: > of_parse_phandle_with_args(dev->of_node, "mboxes", > "#mbox-cells", 0, &of_args) > where #mbox-cells = 0; > > Or ... can I pass NULL for cells_name and make the #mbox-cells property > optional and still keeping its requirement of being at least 1? I think the mbox-cells = 0 approach is preferred, that property is what marks it as a mailbox controller after all. Perhaps Rob or Krzysztof can comment?
On 12/19/24 6:58 PM, Conor Dooley wrote: > On Thu, Dec 19, 2024 at 03:42:11PM +0000, Tudor Ambarus wrote: >> Hi, Conor, >> >> On 12/19/24 2:11 PM, Conor Dooley wrote: >>>> There are mailbox clients that can discover the mailbox channel ID at >>>> run-time. For such cases passing the channel identifier via DT is >>>> redundant. Add support for referencing controllers solely by node. >>> I don't really get your implementation, why not just allow #mbox-cells = 0? >>> That's what's done for things like fixed frequency clocks that only have >>> a single output. >> >> Ah, indeed! >> >> instead of: >> of_parse_phandle(dev->of_node, "mbox", 0); >> I can do a: >> of_parse_phandle_with_args(dev->of_node, "mboxes", >> "#mbox-cells", 0, &of_args) >> where #mbox-cells = 0; >> >> Or ... can I pass NULL for cells_name and make the #mbox-cells property >> optional and still keeping its requirement of being at least 1? > > I think the mbox-cells = 0 approach is preferred, that property is what > marks it as a mailbox controller after all. Perhaps Rob or Krzysztof can > comment? I think using mbox-cells = 0 is better indeed. In my proposal I considered the list to always have a single phandle, thus a reference to a single mailbox controller, whereas it may be possible that clients to reference multiple mailbox controllers. If so, #mbox-cells needs to be defined in all the controllers, for consistency reasons, similar to what happens with fixed clocks, as you already mentioned. Thus I'll change the method to: struct mbox_chan *mbox_request_channel_by_args(struct mbox_client *cl, int index, const struct mbox_xlate_args *spec); and use of_parse_phandle_with_args() in it. Thanks, Conor! ta
On Fri, Dec 20, 2024 at 07:51:32AM +0000, Tudor Ambarus wrote: > > > On 12/19/24 6:58 PM, Conor Dooley wrote: > > On Thu, Dec 19, 2024 at 03:42:11PM +0000, Tudor Ambarus wrote: > >> Hi, Conor, > >> > >> On 12/19/24 2:11 PM, Conor Dooley wrote: > >>>> There are mailbox clients that can discover the mailbox channel ID at > >>>> run-time. For such cases passing the channel identifier via DT is > >>>> redundant. Add support for referencing controllers solely by node. > >>> I don't really get your implementation, why not just allow #mbox-cells = 0? > >>> That's what's done for things like fixed frequency clocks that only have > >>> a single output. > >> > >> Ah, indeed! > >> > >> instead of: > >> of_parse_phandle(dev->of_node, "mbox", 0); > >> I can do a: > >> of_parse_phandle_with_args(dev->of_node, "mboxes", > >> "#mbox-cells", 0, &of_args) > >> where #mbox-cells = 0; > >> > >> Or ... can I pass NULL for cells_name and make the #mbox-cells property > >> optional and still keeping its requirement of being at least 1? > > > > I think the mbox-cells = 0 approach is preferred, that property is what > > marks it as a mailbox controller after all. Perhaps Rob or Krzysztof can > > comment? > > I think using mbox-cells = 0 is better indeed. In my proposal I > considered the list to always have a single phandle, thus a reference to > a single mailbox controller, whereas it may be possible that clients to > reference multiple mailbox controllers. If so, #mbox-cells needs to be > defined in all the controllers, for consistency reasons, similar to what > happens with fixed clocks, as you already mentioned. Oh right, I had totally forgotten about that, that's a solid argument for the mbox-cells = 0 approach for sure.
diff --git a/Documentation/devicetree/bindings/mailbox/mailbox.txt b/Documentation/devicetree/bindings/mailbox/mailbox.txt index af8ecee2ac68..0c4295a62f61 100644 --- a/Documentation/devicetree/bindings/mailbox/mailbox.txt +++ b/Documentation/devicetree/bindings/mailbox/mailbox.txt @@ -5,9 +5,10 @@ assign appropriate mailbox channel to client drivers. * Mailbox Controller -Required property: -- #mbox-cells: Must be at least 1. Number of cells in a mailbox - specifier. +Optional property: +- #mbox-cells: Must be at least 1. Number of cells in a mailbox specifier. + The property becomes mandatory for the cases where the clients + reference the controller via the mboxes property. Example: mailbox: mailbox { @@ -19,7 +20,11 @@ Example: * Mailbox Client Required property: +Clients must reference the mailbox controller either via the mboxes or mbox +properties. - mboxes: List of phandle and mailbox channel specifiers. +- mbox: phandle pointing to the controller. Used by clients that can discover + the channel identifiers at runtime. Optional property: - mbox-names: List of identifier strings for each mailbox channel. @@ -29,7 +34,13 @@ Optional property: communication between the mailbox client and the remote. -Example: +Example using mbox: + power-management { + ... + mbox = <&mailbox>; + }; + +Example using mboxes: pwr_cntrl: power { ... mbox-names = "pwr-ctrl", "rpc";
There are mailbox clients that can discover the mailbox channel ID at run-time. For such cases passing the channel identifier via DT is redundant. Add support for referencing controllers solely by node. Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org> --- Documentation/devicetree/bindings/mailbox/mailbox.txt | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-)