[08/10] dt/bindings: Add binding for BCM2835 mailbox property channel driver

I was tempted to have the mailbox property channel support just be in
the 2835 mailbox driver itself, but mbox_request_channel() wants its
device to have the "mboxes" node, and that appears to be only intended
for mailbox clients, not controllers.

Signed-off-by: Eric Anholt <eric@anholt.net>
---
 .../bindings/mailbox/brcm,bcm2835-mbox-property.txt        | 14 ++++++++++++++
 1 file changed, 14 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mailbox/brcm,bcm2835-mbox-property.txt

On 03/02/2015 01:54 PM, Eric Anholt wrote:
> I was tempted to have the mailbox property channel support just be in
> the 2835 mailbox driver itself, but mbox_request_channel() wants its
> device to have the "mboxes" node, and that appears to be only intended
> for mailbox clients, not controllers.

This is more of a particular format/protocol of messages you can send
over the mailbox HW than a device. I wonder if it actually makes sense
to represent it in DT as a device at all?

If we do represent this as a device in DT, shouldn't it also look like a
mailbox device so that other drivers (clock, display, ...) can bind to
it and send messages using the mailbox API?

I might have expected to just add property support directly into the
basic bcm2835 mailbox driver itself. Perhaps some attempt might be made
to isolate the HW register level access in one file/driver, and expose
both the regular and property mailbox protocols as a higher level that
uses the low-level mailbox functionality? The concept of the lower 4
bits of mailbox data being a channel ID might belong in the higher
protocol level rather than the lower HW layer?
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Stephen Warren <swarren@wwwdotorg.org> writes:

> On 03/02/2015 01:54 PM, Eric Anholt wrote:
>> I was tempted to have the mailbox property channel support just be in
>> the 2835 mailbox driver itself, but mbox_request_channel() wants its
>> device to have the "mboxes" node, and that appears to be only intended
>> for mailbox clients, not controllers.
>
> This is more of a particular format/protocol of messages you can send
> over the mailbox HW than a device. I wonder if it actually makes sense
> to represent it in DT as a device at all?
>
> If we do represent this as a device in DT, shouldn't it also look like a
> mailbox device so that other drivers (clock, display, ...) can bind to
> it and send messages using the mailbox API?

I don't think it makes sense as a mailbox device.  mailbox devices can
only have one client per channel, while there's no reason to have that
limitation on the property channel.  You could imagine having the
individual tags be channels, except that again there's no reason to have
the one-client limitation, but more importantly the indivudual messages
to the firmware are composed of N tags.

> I might have expected to just add property support directly into the
> basic bcm2835 mailbox driver itself. Perhaps some attempt might be made
> to isolate the HW register level access in one file/driver, and expose
> both the regular and property mailbox protocols as a higher level that
> uses the low-level mailbox functionality? The concept of the lower 4
> bits of mailbox data being a channel ID might belong in the higher
> protocol level rather than the lower HW layer?

Yes, the lower 4 bits being the channel is firmware protocol.

The reason I didn't have it in the same device as the mailbox is that
you need the channel reference in order to set up a mailbox client, and
I didn't think it made sense to have the mailbox device dt reference
itself.

The higher-level interface I think might make sense would be a send_data
replacement that took your device and mbox channel index, and a u32 of
data (low 4 bits unused), and synchronously returned a u32 of response.
It would do the client setup/send/wait_for_completion/teardown for you.

I'm skeptical of putting much more work into mailboxes on this platform,
though.  All we've got for channels are:

0: power (present in this series)
1: fb (Won't be used in Linux)
2: unused
3: VCHIQ (I've heard skepticism that the kernel community would accept this
          interface)
4: unused
5: unused
6: unused
7: settings (not sure if there's anything here not exposed in properties)
8: property (present in this series)
9-15: unused

On 03/05/2015 12:50 PM, Eric Anholt wrote:
> Stephen Warren <swarren@wwwdotorg.org> writes:
> 
>> On 03/02/2015 01:54 PM, Eric Anholt wrote:
>>> I was tempted to have the mailbox property channel support just
>>> be in the 2835 mailbox driver itself, but
>>> mbox_request_channel() wants its device to have the "mboxes"
>>> node, and that appears to be only intended for mailbox clients,
>>> not controllers.
>> 
>> This is more of a particular format/protocol of messages you can
>> send over the mailbox HW than a device. I wonder if it actually
>> makes sense to represent it in DT as a device at all?
>> 
>> If we do represent this as a device in DT, shouldn't it also look
>> like a mailbox device so that other drivers (clock, display, ...)
>> can bind to it and send messages using the mailbox API?
> 
> I don't think it makes sense as a mailbox device.  mailbox devices
> can only have one client per channel, while there's no reason to
> have that limitation on the property channel.  You could imagine
> having the individual tags be channels, except that again there's
> no reason to have the one-client limitation, but more importantly
> the indivudual messages to the firmware are composed of N tags.

My inclination would be to structure everything as:

a) A mailbox driver/device. This doesn't implement any of the protocol
code. The device appears in DT, since it's real HW.

b) A RPi firmware protocol implementation. This is the only code that
talks directly to the mailbox driver. This would implement support for
all aspects of the RPi firmware protocol; both the non-property and
property channels; they're just different aspects of the one RPi firmware.

I guess there could be a DT node that represents the existence of the
RPi firmware. It would be a bit of a "virtual device" rather than an
actual piece of physical HW, but since firmware is an interface that
the SW interacts with, I guess a DT node makes sense.

This DT node would contain a property pointing at the physical mailbox
provider, so it could send messages.

c) Client drivers (clocks, power domains, ...) All client drivers talk
to (b) not (a). The DT nodes for this code might contain a property
that points at (b), but the API from c->b would likely be something
custom rather than the mailbox API, since the RPi firmware protocol
implements something custom rather than standard.

I'd be interested to hear opinions from people much more familiar with
other mailbox HW and protocol drivers. Perhaps they're all lumped
together on other platforms?
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Fri, Mar 6, 2015 at 10:45 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 03/05/2015 12:50 PM, Eric Anholt wrote:
>> Stephen Warren <swarren@wwwdotorg.org> writes:
>>
>>> On 03/02/2015 01:54 PM, Eric Anholt wrote:
>>>> I was tempted to have the mailbox property channel support just
>>>> be in the 2835 mailbox driver itself, but
>>>> mbox_request_channel() wants its device to have the "mboxes"
>>>> node, and that appears to be only intended for mailbox clients,
>>>> not controllers.
>>>
>>> This is more of a particular format/protocol of messages you can
>>> send over the mailbox HW than a device. I wonder if it actually
>>> makes sense to represent it in DT as a device at all?
>>>
>>> If we do represent this as a device in DT, shouldn't it also look
>>> like a mailbox device so that other drivers (clock, display, ...)
>>> can bind to it and send messages using the mailbox API?
>>
>> I don't think it makes sense as a mailbox device.  mailbox devices
>> can only have one client per channel, while there's no reason to
>> have that limitation on the property channel.  You could imagine
>> having the individual tags be channels, except that again there's
>> no reason to have the one-client limitation, but more importantly
>> the indivudual messages to the firmware are composed of N tags.
>
> My inclination would be to structure everything as:
>
> a) A mailbox driver/device. This doesn't implement any of the protocol
> code. The device appears in DT, since it's real HW.
>
Exactly.

> b) A RPi firmware protocol implementation. This is the only code that
> talks directly to the mailbox driver. This would implement support for
> all aspects of the RPi firmware protocol; both the non-property and
> property channels; they're just different aspects of the one RPi firmware.
>
Yes, I usually call that the 'server' driver - a gobal client driver
that serves other client subsystems.

> I guess there could be a DT node that represents the existence of the
> RPi firmware. It would be a bit of a "virtual device" rather than an
> actual piece of physical HW, but since firmware is an interface that
> the SW interacts with, I guess a DT node makes sense.
>
> This DT node would contain a property pointing at the physical mailbox
> provider, so it could send messages.
>
Yes, as long as a device has one physical resource IRQ, register,
clock I think it has the right to be in DT.
It will be a physical mailbox channel, so it qualifies.

> c) Client drivers (clocks, power domains, ...) All client drivers talk
> to (b) not (a). The DT nodes for this code might contain a property
> that points at (b), but the API from c->b would likely be something
> custom rather than the mailbox API, since the RPi firmware protocol
> implements something custom rather than standard.
>
Yes, c<->b protocol will be Linux specific.  b<->a will be
pre-determined by the remote firmware.

I haven't had the time to look deeply but from quick overview:
1) bcm2835-mailbox-power.c and bcm2835-mailbox-property.c are client
drivers and shouldn't reside in drivers/mailbox/

2) The bcm2835-mailbox-power should probably be a part of platform's
USB support - which should try to call remote to enable power upon
init. Or even better, let the 'server' driver (b) manage that mailbox
as well, if it could be used for other purposes on some other
platform.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html