| Message ID | 1403660878-56350-7-git-send-email-s-anna@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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. 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. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/mailbox/omap-mailbox.c b/drivers/mailbox/omap-mailbox.c index d71e84f..4847466 100644 --- a/drivers/mailbox/omap-mailbox.c +++ b/drivers/mailbox/omap-mailbox.c @@ -676,6 +676,28 @@ static const struct of_device_id omap_mailbox_of_match[] = { }; MODULE_DEVICE_TABLE(of, omap_mailbox_of_match); +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; +} + static int omap_mbox_probe(struct platform_device *pdev) { struct resource *mem; @@ -835,6 +857,7 @@ static int omap_mbox_probe(struct platform_device *pdev) mdev->controller.ops = &omap_mbox_chan_ops; mdev->controller.chans = chnls; mdev->controller.num_chans = info_count; + mdev->controller.of_xlate = omap_mbox_of_xlate; ret = omap_mbox_register(mdev); if (ret) return ret;
The mailbox framework currently does not support using the channel phandles directly in the mbox property of client nodes, and also expects a minimum value of 1 for the #mbox-cells in the mailbox controller device node. Implement a custom of_xlate function for the OMAP mailbox driver that allows phandles for the pargs specifier instead of indexing to avoid any channel registration order dependencies. Cc: Jassi Brar <jassisinghbrar@gmail.com> Signed-off-by: Suman Anna <s-anna@ti.com> --- drivers/mailbox/omap-mailbox.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+)