Hi Arnd,
On 06/25/2014 03:39 AM, Arnd Bergmann wrote:
> On Tuesday 24 June 2014 20:47:58 Suman Anna wrote:
>> +static struct mbox_chan *omap_mbox_of_xlate(struct mbox_controller *controller,
>> + const struct of_phandle_args *sp)
>> +{
>> + phandle phandle = sp->args[0];
>> + struct device_node *node;
>> + struct omap_mbox_device *mdev;
>> + struct omap_mbox *mbox;
>> +
>> + node = of_find_node_by_phandle(phandle);
>> + if (!node) {
>> + pr_err("could not find node phandle 0x%x\n", phandle);
>> + return NULL;
>> + }
>> +
>> + mdev = container_of(controller, struct omap_mbox_device, controller);
>> + if (WARN_ON(!mdev))
>> + return NULL;
>> +
>> + mbox = omap_mbox_device_find(mdev, node->name);
>> + return mbox ? mbox->chan : NULL;
>>
>
> Wouldn't it be easier to change the omap_mbox_device_find() function to
> take a phandle argument directly? You shouldn't have to walk all
> nodes in the system, or match it by name if you already have the
> device node.
The omap_mbox_device_find() function is also used on the non-DT mailbox
client path (for OMAP3) at the moment where we don't have the device
node pointers. I am merely reusing the function for the DT lookup path,
and once I remove the non-DT support, I can surely do the matching logic
just based on the device node pointer. I just chose to minimize the
changes to the omap_mbox_device_find() to do both variants now when I
know that I am gonna remove the non-DT part (name lookup) later on.
The current expected DT usage model for the OMAP mailbox client is (with
the v7 mailbox framework)
mbox = <&controller_phandle &channel_phandle>
So, this is not walking through all the nodes in the system, only the
channel/sub-mailbox nodes present on the particular controller node.
Surely, it can done as mbox = <&channel_phandle> as the parent
controller node relationship is inherent, but this requires changes to
the mailbox framework, and I am not sure if every platform
implementation would implement the child channels as their own nodes.
>
> Also, the xlate function is normally the place where you read out
> the other arguments and set them as members in your omap_mbox
> structure.
The current flow for the xlate function is during the
mailbox_request_channel. The channels themselves would have been
registered during the controller node probe, so this is merely a lookup
for the correct channel. Should we be renaming the xlate function, if
the behavior is not what one expects out of a standard xlate?
regards
Suman
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
