linux-arm-msm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeffrey Hugo <jhugo-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
To: Jassi Brar
	<jassisinghbrar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Bjorn Andersson
	<bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Andy Gross <andy.gross-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>,
	Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>,
	Ohad Ben-Cohen <ohad-Ix1uc/W3ht7QT0dZR+AlfA@public.gmane.org>,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-soc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Devicetree List
	<devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	Linux Kernel Mailing List
	<linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	linux-remoteproc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: [PATCH v4 3/5] soc: qcom: Introduce APCS IPC driver
Date: Fri, 5 May 2017 13:53:41 -0600	[thread overview]
Message-ID: <338c4262-cc6b-bed2-16fe-1767d1f0d5f6@codeaurora.org> (raw)
In-Reply-To: <CABb+yY03imq8911rF9dHJjG_a_aosksA=tgk7Rog5AZSezHDyg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On 5/5/2017 1:22 PM, Jassi Brar wrote:
> On Sat, May 6, 2017 at 12:07 AM, Bjorn Andersson
> <bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>> On Fri 05 May 03:26 PDT 2017, Jassi Brar wrote:
>>
>>> On Fri, May 5, 2017 at 1:35 AM, Bjorn Andersson
>>> <bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>>>
>>>> +
>>>> +static int qcom_apcs_ipc_send_data(struct mbox_chan *chan, void *data)
>>>> +{
>>>> +       struct qcom_apcs_ipc *apcs = container_of(chan->mbox,
>>>> +                                                 struct qcom_apcs_ipc, mbox);
>>>> +       unsigned long idx = (unsigned long)chan->con_priv;
>>>> +
>>>> +       writel(BIT(idx), apcs->base + apcs->offset);
>>>> +
>>> When/how does this bit get ever cleared again?
>>> You may want to add last_tx_done() callback to check if this bit is
>>> cleared before you can send the next interrupt. And set
>>> txdone_poll/irq accordingly.
>>>
>>
>> It's a write-only register, writing a bit fires off an edge triggered
>> interrupt on the specific remote processor, which will ack the
>> associated IRQ status and handle the interrupt.
>>
>> As the "message" is just a notification to the other side that it needs
>> to act on "something", there's no harm in notifying it multiple times
>> before it has a chance to ack the IRQ and a write after that will be
>> seen as a separate interrupt.
>>
> What causes it to return to '0'?

Technically nothing.  This is not a traditional register.  Its an 
address that is modeled as a register that the hardware knows, when 1 is 
written to the specific bit, the hardware is to take a specific action 
(generate the interrupt).

As Bjorn stated, its a write only address.  The hardware documentation 
states not to read it.  On most platforms, if you attempt to read the 
address, you will always get 0, although that is not guaranteed, and 
there have been platforms where attempting to read the address will 
result in a bus hang.

Conceptually, the hardware automatically resets it to "0" in the next 
clock cycle.  Software doesn't need to care.

> 
> I think the driver should wait for it to become 0 before writing 1.
> For example, the protocol has a command that says to remote cpu to
> increase the voltage supply by 0.1v. This command is filled in a
> structure and laid out in the shared memory before you ring the
> 'doorbell'.  In this situation you don't want the remote cpu to act
> twice on the same command. Also for a new command, you don't want to
> overwrite the last command packet before remote cpu has consumed it.

That doesn't apply here.  The doorbell in this case is a signal for the 
remote processor to go look at the FIFO in shared memory.  Since 
interrupts can be lost (ie the local processor sends multiple interrupts 
before the remote processor can service them, so the GIC drops them 
since the interrupt is already pending), the remote processor is 
required to drain the FIFO when it processes the interrupt.

There is no way to determine if the remote processor has observed a 
message, that does not involve pretty trivial race conditions.

I invented this protocol.  From what I've seen (granted I did not do a 
thorough review), Bjorn is doing the correct thing here.

-- 
Jeffrey Hugo
Qualcomm Datacenter Technologies as an affiliate of Qualcomm 
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

  parent reply	other threads:[~2017-05-05 19:53 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-04 20:05 [PATCH v4 1/5] mailbox: Make startup and shutdown ops optional Bjorn Andersson
2017-05-04 20:05 ` [PATCH v4 2/5] dt-bindings: mailbox: Introduce Qualcomm APCS global binding Bjorn Andersson
2017-05-04 20:05 ` [PATCH v4 3/5] soc: qcom: Introduce APCS IPC driver Bjorn Andersson
2017-05-05 10:26   ` Jassi Brar
2017-05-05 18:37     ` Bjorn Andersson
2017-05-05 19:22       ` Jassi Brar
     [not found]         ` <CABb+yY03imq8911rF9dHJjG_a_aosksA=tgk7Rog5AZSezHDyg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-05-05 19:53           ` Jeffrey Hugo [this message]
2017-05-05 20:22             ` Jassi Brar
2017-05-05 20:39               ` Jeffrey Hugo
     [not found]               ` <CABb+yY1K3Hyn7CzQCA6tHu-KrPigARc3cYgBdT8PNX7vjhPK0g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-05-06  1:19                 ` Bjorn Andersson
2017-05-06  4:48                   ` Jassi Brar
2017-05-08  5:54                     ` Bjorn Andersson
2017-05-08  6:47                       ` Jassi Brar
2017-05-08 19:11                         ` Bjorn Andersson
2017-05-09 16:41                           ` Jassi Brar
     [not found]                             ` <CABb+yY2uC6vNFTq1KtZwrrw8x4BEfc2cMjF5DwrLbGpYWBNBzQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-05-09 19:11                               ` Bjorn Andersson
2017-05-10  2:33                                 ` Jassi Brar
2017-05-10 19:00                                   ` Bjorn Andersson
2017-05-11  2:07                                     ` Jassi Brar
2017-05-12 22:48                                       ` Bjorn Andersson
2017-05-16 11:25                                         ` Jassi Brar
2017-05-04 20:05 ` [PATCH v4 4/5] soc: qcom: Add device tree binding for GLINK RPM Bjorn Andersson
2017-05-08 17:06   ` Rob Herring
2017-05-08 17:53     ` Bjorn Andersson
2017-05-04 20:05 ` [PATCH v4 5/5] rpmsg: Introduce Qualcomm RPM glink driver Bjorn Andersson
     [not found] ` <20170504200539.27027-1-bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2017-05-05  9:35   ` [PATCH v4 1/5] mailbox: Make startup and shutdown ops optional Sudeep Holla
2017-05-05 10:33   ` Jassi Brar
2017-05-05 18:21     ` Bjorn Andersson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=338c4262-cc6b-bed2-16fe-1767d1f0d5f6@codeaurora.org \
    --to=jhugo-sgv2jx0feol9jmxxk+q4oq@public.gmane.org \
    --cc=andy.gross-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \
    --cc=bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \
    --cc=devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=jassisinghbrar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-remoteproc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-soc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=mark.rutland-5wv7dgnIgG8@public.gmane.org \
    --cc=ohad-Ix1uc/W3ht7QT0dZR+AlfA@public.gmane.org \
    --cc=robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).