[RFC] mailbox: Add Broadcom BCM2835 mailbox driver
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
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
How could this patch be progressed? Is there anything I could do?
More information about the linux-rpi-kernel