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


Groups > linux.kernel > #1637222

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

From Bjorn Andersson <bjorn.andersson@linaro.org>
Newsgroups linux.kernel
Subject Re: [PATCH v4 3/5] soc: qcom: Introduce APCS IPC driver
Date 2017-05-08 08:00 +0200
Message-ID <tEJSa-896-3@gated-at.bofh.it> (permalink)
References (4 earlier) <tDR5o-5Ye-7@gated-at.bofh.it> <tDRyq-6eu-31@gated-at.bofh.it> <tDS1s-6E3-7@gated-at.bofh.it> <tDWy5-1bO-5@gated-at.bofh.it> <tDZPj-3qA-3@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Fri 05 May 21:48 PDT 2017, Jassi Brar wrote:

> On Sat, May 6, 2017 at 6:49 AM, Bjorn Andersson
> <bjorn.andersson@linaro.org> wrote:
> > On Fri 05 May 13:22 PDT 2017, Jassi Brar wrote:
> 
> >> How is it supposed to work if a client queues more than one request?
> >
> > One such example is found in patch 5 in this series. There are two FIFOs
> > in shared memory, one in each direction. Each fifo has a index-pair
> > associated; a write-index is used by the writer to inform the reader
> > where the valid data in the ring buffer ends and a read-index indicates
> > to the writer how far behind the read is.
> >
> > The writer will just push data into the FIFO, each time firing off an
> > APCS IPC interrupt and when the remote interrupt handler runs it will
> > consume all the messages from the read-index to the write-index. All
> > without the need for the reader to signal the writer that it has
> > received the interrupts.
> >
> > In the event that the write-index catches up with the read-index a
> > dedicated flag is set which will cause the reader to signal that the
> > read-index is updated - allowing the writer to sleep waiting for room in
> > the FIFO.
> >
> Interesting.Just for my enlightenment...
> 
>   Where does the writer sleep in the driver? I see it simply sets the
> bit and leave.
> Such a flag (or head and tail pointers matching) should be checked in
> last_tx_done()
> 

In the case of the FIFO based communication the flow control is
implemented one layer up. You can see this in glink_rpm_tx() (in patch
5/5), where we check to see if there's enough room between the writer
and the reader to store the new message. 

The remote side will process messages and increment the read index,
which will break us out of the loop.


Note that its possible to write any number of messages before invoking
the APCS IPC and there's no harm (beyond wasting a little bit of power)
in invoking it when there's no data written.

> If you think RPM will _always_ be ready to accept new messages (though
> we have seen that doesn't hold in some situations), then you don't
> need last_tx_done.

Whether the remote processor is ready to accept a new message or not is
irrelevant in the sense of APCS IPC. When a client sends the signal over
the APCS IPC it has already determined that there was space, filled in
the shared memory buffers and is now simply invoking the remote handler.


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.

But regardless of the protocol implemented ontop, the purpose of the
APCS IPC bit is _only_ to invoke some remote handler to consume some
data, somewhere - the event in itself does not carry any information.

> 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()?

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 :(

Regards,
Bjorn

Back to linux.kernel | Previous | NextPrevious 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