[RFC] mailbox: Add Broadcom BCM2835 mailbox driver

Craig McGeachie slapdau at yahoo.com.au
Mon Aug 19 03:28:39 EDT 2013


On 08/17/2013 11:47 PM, Craig McGeachie wrote:
> I think Lubomir's implementation, with the atomic write/read interface,
> might be the safest, but that it gains safety at the cost of
> performance. I say performance a bit tongue in cheek because I don't see
> the need for that many mailbox calls.  The only place where I thought it
> might be an issue, vchiq, makes the call once to set an alternative call
> mechanism.  Unless somebody is checking the chip temperature every 5us,
> I can't see it being an issue.

I take it all back.  Lubomir's implementation looks close to ideal. With 
the following caveats.

1. There is a safety problem. bcm2835_mbox_io can have NULL passed for 
out28. Line 138 then goes to out: and releases the channel lock. A 
subsequent call to bcm2835_mbox_io before bcm2835_mbox_irq was called to 
service the reply would enter the mutex section, and pick up the 
completion and reply message. Then all messages and replies would be out 
of step with each other.  This would be a very rare race condition, but 
it could happen, and it would be nasty to track down.

2. I'm not sure what will happen the wait_for_completion_timeout 
actually times out, if it could have the same problem

2. The patch documents the proposed device tree bindings but doesn't 
include any.

3. The example reg definition looks large. Mailbox 0 is at 
0x7e00b880-0x7e00b89f and mailbox 1 is at 0x7e00b8a0-0x7e00b8bf.

4. I got slightly confused by line 38,
+#define MAIL0_WRT  (ARM_0_MAIL1 + 0x00)
After reading the other drivers, I get the point.  I was just surprised 
to see code writing to MAIL0_WRT after I'd read the other drivers and 
seen the expected behaviour.

Otherwise, it neatly queues up different channels as much as possible 
any comments I made about performance were ill informed rubbish.  Sorry.

But I still don't think it should be in drivers/mailbox or make use of 
include/linux/mailbox.h.

How could this patch be progressed?  Is there anything I could do?

Cheers,
Craig.




More information about the linux-rpi-kernel mailing list