Messages in this thread | | | Date | Tue, 10 Jan 2023 09:57:40 -0800 | From | Elliot Berman <> | Subject | Re: [PATCH v8 07/28] mailbox: Allow direct registration to a channel |
| |
On 1/9/2023 1:34 PM, Alex Elder wrote: > On 12/19/22 4:58 PM, Elliot Berman wrote: >> Support virtual mailbox controllers and clients which are not platform >> devices or come from the devicetree by allowing them to match client to >> channel via some other mechanism. > > The new function behaves very much like mbox_request_channel() > did before. > > The new function differs from omap_mbox_request_channel() in that > it can change the if chan->txdone_method is TXDONE_BY_POLL, it > is changed to TXDONE_BY_ACK if the client's knows_txdone field is > set. Is this OK? Why?
Both of the current drivers that use mbox_bind_client use TXDONE_BY_IRQ, so this doesn't cause issue for checking whether the client has txdone_method.
> > It also assumes chan->mbox->ops->startup us non-null (though that > isn't really a problem). > >> >> Signed-off-by: Elliot Berman <quic_eberman@quicinc.com> >> --- >> drivers/mailbox/mailbox.c | 96 ++++++++++++++++++++++++---------- >> drivers/mailbox/omap-mailbox.c | 18 ++----- >> drivers/mailbox/pcc.c | 18 ++----- >> include/linux/mailbox_client.h | 1 + >> 4 files changed, 76 insertions(+), 57 deletions(-) >> >> diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c >> index 4229b9b5da98..adf36c05fa43 100644 >> --- a/drivers/mailbox/mailbox.c >> +++ b/drivers/mailbox/mailbox.c >> @@ -317,6 +317,71 @@ int mbox_flush(struct mbox_chan *chan, unsigned >> long timeout) >> } >> EXPORT_SYMBOL_GPL(mbox_flush); >> +static int __mbox_bind_client(struct mbox_chan *chan, struct >> mbox_client *cl) > > There should be an unbind_client() call. At a minimum, you are > calling try_module_get(), and the matching module_put() call > would belong there. And even though one might just call > module_put() elsewhere for this, it would be cleaner to have > a function that similarly encapsulates the shutdown call > as well.
The function for this is "mbox_free_channel".
Thanks, Elliot
| |