[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <8f47229a-2785-5f54-f7d1-a42299de64fe@quicinc.com>
Date: Tue, 10 Jan 2023 09:57:40 -0800
From: Elliot Berman <quic_eberman@...cinc.com>
To: Alex Elder <elder@...aro.org>,
Bjorn Andersson <quic_bjorande@...cinc.com>,
Jassi Brar <jassisinghbrar@...il.com>,
"Sudeep Holla" <sudeep.holla@....com>
CC: Murali Nalajala <quic_mnalajal@...cinc.com>,
Trilok Soni <quic_tsoni@...cinc.com>,
Srivatsa Vaddagiri <quic_svaddagi@...cinc.com>,
Carl van Schaik <quic_cvanscha@...cinc.com>,
Prakruthi Deepak Heragu <quic_pheragu@...cinc.com>,
Arnd Bergmann <arnd@...db.de>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Rob Herring <robh+dt@...nel.org>,
"Krzysztof Kozlowski" <krzysztof.kozlowski+dt@...aro.org>,
Jonathan Corbet <corbet@....net>,
Bagas Sanjaya <bagasdotme@...il.com>,
Catalin Marinas <catalin.marinas@....com>,
Will Deacon <will@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Lorenzo Pieralisi <lpieralisi@...nel.org>,
"Dmitry Baryshkov" <dmitry.baryshkov@...aro.org>,
<linux-arm-msm@...r.kernel.org>, <devicetree@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <linux-doc@...r.kernel.org>,
<linux-arm-kernel@...ts.infradead.org>,
<linux-acpi@...r.kernel.org>
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@...cinc.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
Powered by blists - more mailing lists