Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1638502
| From | Jassi Brar <jassisinghbrar@gmail.com> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PATCH v4 3/5] soc: qcom: Introduce APCS IPC driver |
| Date | 2017-05-10 04:40 +0200 |
| Message-ID | <tFpHH-2eO-7@gated-at.bofh.it> (permalink) |
| References | (6 earlier) <tEJSa-896-3@gated-at.bofh.it> <tEKEx-pS-1@gated-at.bofh.it> <tEWmm-80A-7@gated-at.bofh.it> <tFguJ-4li-7@gated-at.bofh.it> <tFiPT-62w-9@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
On Wed, May 10, 2017 at 12:41 AM, Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
> On Tue 09 May 09:41 PDT 2017, Jassi Brar wrote:
>>
>> >> >> The client should call mbox_client_txdone() after
>> >> >> mbox_send_message().
>> >> >
>> >> > So every time we call mbox_send_message() from any of the client drivers
>> >> > we also needs to call mbox_client_txdone()?
>> >> >
>> >> Yes.
>> >>
>> >> > This seems like an awkward side effect of using the mailbox framework -
>> >> > which has to be spread out in at least 6 different client drivers :(
>> >> >
>> >> No. Mailbox or whatever you implement - you must (and do) tick the
>> >> state machine to keep the messages moving.
>> >
>> > But the state you have in the other mailbox drivers is not a concern of
>> > the APCS IPC.
>> >
>> No, as you say above you check for space before writing the next
>> message, this is what I call ticking the state machine.
>>
>
> Sure, but you're talking about the mailbox state machine. The APCS IPC
> doesn't have states. The _only_ thing that the APCS IPC provides is a
> mechanism for informing the other side that "hey there's something to
> do". So it doesn't matter if there's already a pending "hey there's
> something to do", because adding another will still only be "hey there's
> something to do".
>
> I'm just trying to describe the big picture, but you keep confusing the
> mailbox/doorbell responsibilities with the client's responsibilities.
>
I think I do understand the bigger picture...
The client driver sets up data packet in SHM and submit a "doorbell"
to be ringed. The controller driver simply sets some bit to trigger an
irq on the remote side (doorbell). And before submitting a "doorbell"
the client makes sure there is some space for data packet to be
written. Right? You see, in the big picture you do have a
state-machine.
[Message to send]
|
|
|-------------->|
| No |
| |
|___[Space Available?]
|
|Yes
|
|
[ Setup Data in SHM]
|
V
[Ring Doorbell]
Mailbox framework supports this whole picture. There is even a
callback (tx_prepare) to setup data packet just before the doorbell is
to be rung.
>> BTW, this is an option only if your client driver doesn't want to
>> explicitly tick the state machine by calling mbox_client_txdone()...
>> which I think should be done in the first place.
>>
>
> There is no state of the APCS IPC, so the overhead is created by the
> mailbox framework.
>
Overhead remains the same if you move the check from your client
drivers to last_tx_done.
OR your client driver, rightfully, drive the state machine by calling
mbox_client_txdone() like other platforms.
[Message to send]<----------|
| |
| |
|-------------->| |
| No | |
| | |
|___[Space Available?] |
| |
|Yes |
| |
V |
[Setup Data in SHM] |
| |
V |
mbox_send_message() |
| |
V |
mbox_client_txdone() |
| |
V______________|
> The part where this piece of hardware differs from the other mailboxes
> is that TX is done as send_data() returns and in the realm of the
> mailbox there is no such thing as "tx done". So how about we extend the
> framework to handle stateless and message-less doorbells?
>
This is a very common usecase. It would be unfair to other platforms
to modify the API just because you find it awkward to call
mbox_client_txdone() right after mbox_send_message(). For example,
drivers/firmware/tegra/bpmp.c
I'd much rather have mbox_send_message_and_tick() than implant a new api.
Thanks.
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[PATCH v4 1/5] mailbox: Make startup and shutdown ops optional Bjorn Andersson <bjorn.andersson@linaro.org> - 2017-05-04 22:10 +0200
[PATCH v4 2/5] dt-bindings: mailbox: Introduce Qualcomm APCS global binding Bjorn Andersson <bjorn.andersson@linaro.org> - 2017-05-04 22:10 +0200
[PATCH v4 4/5] soc: qcom: Add device tree binding for GLINK RPM Bjorn Andersson <bjorn.andersson@linaro.org> - 2017-05-04 22:10 +0200
Re: [PATCH v4 4/5] soc: qcom: Add device tree binding for GLINK RPM Rob Herring <robh@kernel.org> - 2017-05-08 19:10 +0200
Re: [PATCH v4 4/5] soc: qcom: Add device tree binding for GLINK RPM Bjorn Andersson <bjorn.andersson@linaro.org> - 2017-05-08 20:00 +0200
[PATCH v4 3/5] soc: qcom: Introduce APCS IPC driver Bjorn Andersson <bjorn.andersson@linaro.org> - 2017-05-04 22:10 +0200
Re: [PATCH v4 3/5] soc: qcom: Introduce APCS IPC driver Jassi Brar <jassisinghbrar@gmail.com> - 2017-05-05 12:30 +0200
Re: [PATCH v4 3/5] soc: qcom: Introduce APCS IPC driver Bjorn Andersson <bjorn.andersson@linaro.org> - 2017-05-05 20:40 +0200
Re: [PATCH v4 3/5] soc: qcom: Introduce APCS IPC driver Jassi Brar <jassisinghbrar@gmail.com> - 2017-05-05 21:30 +0200
Re: [PATCH v4 3/5] soc: qcom: Introduce APCS IPC driver Jeffrey Hugo <jhugo@codeaurora.org> - 2017-05-05 22:00 +0200
Re: [PATCH v4 3/5] soc: qcom: Introduce APCS IPC driver Jassi Brar <jassisinghbrar@gmail.com> - 2017-05-05 22:30 +0200
Re: [PATCH v4 3/5] soc: qcom: Introduce APCS IPC driver Jeffrey Hugo <jhugo@codeaurora.org> - 2017-05-05 22:40 +0200
Re: [PATCH v4 3/5] soc: qcom: Introduce APCS IPC driver Bjorn Andersson <bjorn.andersson@linaro.org> - 2017-05-06 03:20 +0200
Re: [PATCH v4 3/5] soc: qcom: Introduce APCS IPC driver Jassi Brar <jassisinghbrar@gmail.com> - 2017-05-06 06:50 +0200
Re: [PATCH v4 3/5] soc: qcom: Introduce APCS IPC driver Bjorn Andersson <bjorn.andersson@linaro.org> - 2017-05-08 08:00 +0200
Re: [PATCH v4 3/5] soc: qcom: Introduce APCS IPC driver Jassi Brar <jassisinghbrar@gmail.com> - 2017-05-08 08:50 +0200
Re: [PATCH v4 3/5] soc: qcom: Introduce APCS IPC driver Bjorn Andersson <bjorn.andersson@linaro.org> - 2017-05-08 21:20 +0200
Re: [PATCH v4 3/5] soc: qcom: Introduce APCS IPC driver Jassi Brar <jassisinghbrar@gmail.com> - 2017-05-09 18:50 +0200
Re: [PATCH v4 3/5] soc: qcom: Introduce APCS IPC driver Bjorn Andersson <bjorn.andersson@linaro.org> - 2017-05-09 21:20 +0200
Re: [PATCH v4 3/5] soc: qcom: Introduce APCS IPC driver Jassi Brar <jassisinghbrar@gmail.com> - 2017-05-10 04:40 +0200
Re: [PATCH v4 3/5] soc: qcom: Introduce APCS IPC driver Bjorn Andersson <bjorn.andersson@linaro.org> - 2017-05-10 21:10 +0200
Re: [PATCH v4 3/5] soc: qcom: Introduce APCS IPC driver Jassi Brar <jassisinghbrar@gmail.com> - 2017-05-11 04:10 +0200
[PATCH v4 5/5] rpmsg: Introduce Qualcomm RPM glink driver Bjorn Andersson <bjorn.andersson@linaro.org> - 2017-05-04 22:10 +0200
Re: [PATCH v4 1/5] mailbox: Make startup and shutdown ops optional Sudeep Holla <sudeep.holla@arm.com> - 2017-05-05 11:40 +0200
Re: [PATCH v4 1/5] mailbox: Make startup and shutdown ops optional Jassi Brar <jassisinghbrar@gmail.com> - 2017-05-05 12:40 +0200
Re: [PATCH v4 1/5] mailbox: Make startup and shutdown ops optional Bjorn Andersson <bjorn.andersson@linaro.org> - 2017-05-05 20:30 +0200
csiph-web