lkml.org 
[lkml]   [2015]   [May]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH] mailbox/bcm2835: Fix mailbox full detection.
From
On Fri, May 29, 2015 at 3:16 AM, Eric Anholt <eric@anholt.net> wrote:
> Stephen Warren <swarren@wwwdotorg.org> writes:
>
>> On 05/13/2015 02:10 PM, Eric Anholt wrote:
>>> With the VC reader blocked and the ARM writing, MAIL0_STA reads empty
>>> permanently while MAIL1_STA goes from empty (0x40000000) to non-empty
>>> (0x00000001-0x00000007) to full (0x80000008).
>>>
>>> This bug ended up having no effect on us, because all of our
>>> transactions in the client driver were synchronous and under a mutex.
>>
>> If you could get someone at the RPi Foundation or Broadcom to update the
>> register descriptions and example code at the following URLs, that would
>> be rather useful. Otherwise, this code will appear incorrect when
>> compared against the documentation:
>>
>> https://github.com/raspberrypi/firmware/wiki/Mailboxes
>> ("Mailbox registers" at the bottom)
>>
>> https://github.com/raspberrypi/firmware/wiki/Accessing-mailboxes
>> ("Sample code")
>
> Since it's a wiki, I went ahead and edited the first one. Hopefully
> that clarifies how the c++ in the other page is supposed to be used.
>
>>> diff --git a/drivers/mailbox/bcm2835-mailbox.c b/drivers/mailbox/bcm2835-mailbox.c
>>
>>> @@ -117,7 +118,7 @@ static bool bcm2835_last_tx_done(struct mbox_chan *link)
>>> bool ret;
>>>
>>> spin_lock(&mbox->lock);
>>> - ret = !(readl(mbox->regs + MAIL0_STA) & ARM_MS_FULL);
>>> + ret = !(readl(mbox->regs + MAIL1_STA) & ARM_MS_FULL);
>>
>> What does "tx done" mean semantically?
>>
>> If "tx done" means "remote side received all our messages", then surely
>> this should check MAIL1_STA for emptiness, which is different to the
>> "not full" check implemented here?
>>
Mailbox core queues messages internally. The controller driver is
provided one message at a time.

>> If "tx done" means "there's space to transmit more messages", then
>> consider this:
>
> The mailbox core appears to use this hook as "there's space to transmit
> more messages." The name does seem really confusing.
>
'tx_done' => 'is tx done' / 'tx is done'. 'tx' is the last submitted
message to be transferred.
So 'tx_done' marks if the last transmitted message was received by the
remote. That event could be indicated by some register's bit cleared
or even a message(ack) returned by remote (client does
mbox_client_txdone). And if last tx was done, we should be able to
send next message.


\
 
 \ /
  Last update: 2015-05-29 09:41    [W:0.043 / U:0.124 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site