Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1638279

Re: [PATCH v4 3/5] soc: qcom: Introduce APCS IPC driver

Show all headers | View raw

On Tue, May 9, 2017 at 12:41 AM, Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
> On Sun 07 May 23:47 PDT 2017, Jassi Brar wrote:
>
>> On Mon, May 8, 2017 at 11:24 AM, Bjorn Andersson
>> <bjorn.andersson@linaro.org> wrote:
>> > On Fri 05 May 21:48 PDT 2017, Jassi Brar wrote:
>> >
>> > The APCS IPC register serves the basis for all inter-processor
>> > communication in a Qualcomm platform, so it's not only the RPM driver
>> > discussed earlier that uses this. It's also used for other non-FIFO
>> > based communication channels, where the signalled information either
>> > isn't acked at all or acked on a system-level.
>> >
>> Something has to indicate consumption of data or "requested action
>> taken". Otherwise the protocol is design-wise broken.
>>
>
> The SMD and GLINK protocols work by providing two independent one-way
> pipes that higher levels can use to send and receive messages. When some
> driver pushes a message into the transmit-pipe we check if there's
> space, then write the message, signal the remote (APCS IPC) and then
> return.
>
"we check if there's space"  -> this is what mailbox api tries to do
with last_tx_done before starting the next message.


>> >> 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.


>>   Best designs have some interrupt occurring when the message has been
>> consumed by the remote. Some designs have a flag set which needs to be
>> polled to detect completion. Very few (like yours) that support
>> neither irq nor polling, have to be driven by the upper protocol layer
>> by some ack packet (or tracking read/write pointers like you do).
>> These three cases are denoted by TXDONE_BY_IRQ, TXDONE_BY_POLL and
>> TXDONE_BY_ACK respectively.
>>
>
> You're confusing the APCS IPC with the larger communication mechanism,
>
Maybe. I am not versed with QCom technologies like RPM, SMD, GLINK, APCS etc.
Controller driver is what physically transmits a signal to remote.
Users above the mailbox api are client drivers.

>
> This is why I suggested that this is a doorbell, rather than a mailbox.
> Your argumentation of how a mailbox should work makes perfect sense, but
> it's not how the Qualcomm IPC works.
>
Mailbox framework is designed to support, what you call doorbell type
of communication, just fine. There is no need to define another class.

>
> Setting TXDONE_BY_POLL and specifying a dummy last_tx_done() comes with
> a crazy overhead. To set a single bit in a register we will take the
> channel spinlock 4 times, start a timer, iterate over all registered
> channels and the client must be marked as blocking so we will get at
> least 2 additional context switches.
>
How often does the platform send messages for it to be a considerable load?
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.

thanks

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

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