[RFC] mailbox: Add Broadcom BCM2835 mailbox driver

[Stephen Warren]

On 04/17/2013 06:56 AM, Lubomir Rintel wrote:
> Hello!
> 
> This adds a driver for mailbox IPC mechanism present on Broadcom
> BCM2835 SoC, used in Raspberry Pi and Roku 2 devices.

Is this driver for the upstream kernel, or downstream?

Patches for upstream should also be posted to a few other mailing lists;
see the output of get_maintainers.pl. In particular,

The ARM kernel mailing list, since this touches ARM.
The device tree mailing list, since this introduces a new binging.
Perhaps a mailbox mailing list if there is one.

Note that I'm away for 2 weeks starting later today. I'm not sure if
I'll be able to review anything during that time.

> Some work-in-progress drivers (as well as device tree bindings, etc.) that
> utilize this are here:
> 
> https://github.com/hackerspace/rpi-linux/tree/lr-raspberry-pi
> https://github.com/hackerspace/rpi-linux/tree/lr-vchiq
> 
> I intend to post them for review/inclusion once this driver is in good shape.
> 
> I'm really not sure about some decisions made implementing this and am wondering
> if you could take a look at this:
> 
> 1.) How to ensure this driver is loaded before the devices that need it? Is
> arch_initcall() appropriate? Shouldn't devices that communicate via mailbox
> (thermal, cpufreq, vchiq, framebuffer...) be present underneath the mbox in the
> device hierarchy so that we could register them in our probe() only once we've
> set up the mailbox?

Deferred probe should be used. Using initcall levels for this purpose
won't work with device tree anyway in general, and is rather fragile.

The basic idea is that drivers using the mailbox API should attempt to
initialize the mailbox API, and if that fails because the mailbox
driver/provider hasn't yet completed probe, then the client should
return -EPROBE_DEFERRED from their probe().

> 2.) Is the global struct device for mbox appropriate or should we extend the
> exported functions with dev argument and let our users discover the mbox device
> via device tree?

I haven't looked at the code, but as a general rule, globals aren't a
good idea. Clients of resources must certainly look up the provider of
that resource using device tree. There are perhaps two ways to do that:

a) (most common for simple resources): Add a property to each client
device that contains the phandle of the mailbox provider, plus any
required properties (e.g. channels), and then provide an API in the
mailbox driver to look that up and convert it so whatever internal
objects are required. See GPIOs, interrupts, regulators, pinctrl, ...
for examples.

b) (probably most appopriate here): Represent the mailbox device as a
bus, and make each device that communicates over that bus be a child of
the mailbox driver. Mailbox will initialize, then cause all its children
to be probed()d, and they can get a handle to the mailbox provider by
passing their parent struct device * to some mailbox API for conversion.
This is probably most appropriate, since one can view the mailbox
communication layer as a bus that contains a bunch of child devices, in
just the same way as various virtual machine buses work. You'd use
of_platform_populate() to instantiate the children. See e.g.
sound/soc/tegra/tegra30_ahub.c for an example.

Also, see if there are any existing mailbox bindings, and try to be
consistent with them if it makes sense. It may also make sense to create
a standardized mailbox binding.