Re: [PATCHv8 2/2] mailbox: Introduce framework for mailbox

On 16 July 2014 21:39, Suman Anna [off-list ref] wrote:

Hi,

On 07/16/2014 09:18 AM, Jassi Brar wrote:

quoted

On 16 July 2014 19:38, Arnd Bergmann [off-list ref] wrote:

quoted

On Wednesday 16 July 2014 18:42:22 Jassi Brar wrote:

quoted

On 16 July 2014 18:39, Arnd Bergmann [off-list ref] wrote:

quoted

On Wednesday 16 July 2014 18:35:33 Jassi Brar wrote:

quoted

On 16 July 2014 18:15, Arnd Bergmann [off-list ref] wrote:

quoted

On Wednesday 16 July 2014 18:07:04 Jassi Brar wrote:

quoted

On 16 July 2014 15:46, Arnd Bergmann [off-list ref] wrote:

quoted

On Wednesday 16 July 2014 10:40:19 Sudeep Holla wrote:

quoted

quoted

+
+Required property:
+- mbox: List of phandle and mailbox channel specifier.
+
+- mbox-names: List of identifier strings for each mailbox channel
+               required by the client.
+

IMO the mailbox names are more associated with the controller channels/
mailbox rather than the clients using it. Does it make sense to move
this under controller. It also avoid each client replicating the names.

I think it would be best to just make the mbox-names property optional,
like we have for other subsystems.

A very similar subsystem - DMAEngine also has 'dma-names' as a
required property.

If a client is assigned only 1 mbox in DT, we can do without
mbox-names. But I am not sure what to do if a client needs two or more
differently capable mboxes? Simply allocating in order of mbox request
doesn't seem very robust.

Traditionally, these things (regs, interrupts, ...) are just accessed
by index. The reason why dmaengine requires the name is that some machines
can use multiple DMA engine devices attached to the same request line,
so the dmaengine subsystem can pick any of them that has a matching
name.

And also, I think, when a client needs 2 different dma channels, say
for RX and TX each. The api can't assign the first channel specified
in 'dmas' property to the first channel request that comes to it,
unless we assume client driver always requests dma channels in the
order written in its DT node.  And this is the main reason I see for
having mbox-names property.

Most subsystems require passing an explicit index in this case.

quoted

  If we make mbox-names optional, do we assume client driver must
request mbox in the order specified in its DT node?

Correct.

OK. So how about we drop mbox-names altogether and expect client
driver to simply provide an index of the mbox needed?

That would be fine with me, but I think a lot of people like
the idea of identifying things by name, and are used to that
from the other subsystems.

Maybe you can leave the mbox-names property defined as 'optional'
in the generic mbox binding but remove the code in Linux? That way
we can always put it back at a later point without changing the
binding in an incompatible way.

I like this too, especially given that the framework uses this only to
retrieve the index. The mbox_client structure currently requires the
chan_name to be filled in, which means both the client driver and the
framework end up having to parse the property.

Yeah let us discourage the use of mbox-names.

While we are at this, how about we change the property name from "mbox"
to a plural-form "mboxes" to be inline with gpios, dmas etc.

OK, will change mbox to mboxes.

-jassi
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html