Introduce optional fw_node() callback which allows a mailbox controller
driver to provide controller specific mapping using fwnode.

The Linux OF framework already implements fwnode operations for the
Linux DD framework so the fw_xlate() callback works fine with device
tree as well.

Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
 drivers/mailbox/mailbox.c          | 45 +++++++++++++++++-------------
 include/linux/mailbox_controller.h |  3 ++
 2 files changed, 28 insertions(+), 20 deletions(-)

diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
index 5cd8ae222073..d1840eace725 100644
--- a/drivers/mailbox/mailbox.c
+++ b/drivers/mailbox/mailbox.c
@@ -15,6 +15,7 @@
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/of.h>
+#include <linux/property.h>
 #include <linux/spinlock.h>
 
 #include "mailbox.h"
@@ -383,34 +384,46 @@ EXPORT_SYMBOL_GPL(mbox_bind_client);
  */
 struct mbox_chan *mbox_request_channel(struct mbox_client *cl, int index)
 {
+	struct fwnode_reference_args fwspec;
 	struct device *dev = cl->dev;
 	struct mbox_controller *mbox;
 	struct of_phandle_args spec;
 	struct mbox_chan *chan;
+	unsigned int i;
 	int ret;
 
-	if (!dev || !dev->of_node) {
-		pr_debug("%s: No owner device node\n", __func__);
+	if (!dev || !dev_fwnode(dev)) {
+		pr_debug("No owner %s\n", dev ? "fwnode" : "device");
 		return ERR_PTR(-ENODEV);
 	}
 
-	ret = of_parse_phandle_with_args(dev->of_node, "mboxes", "#mbox-cells",
-					 index, &spec);
+	ret = fwnode_property_get_reference_args(dev_fwnode(dev), "mboxes",
+						 "#mbox-cells", 0, index, &fwspec);
 	if (ret) {
 		dev_err(dev, "%s: can't parse \"mboxes\" property\n", __func__);
 		return ERR_PTR(ret);
 	}
 
+	spec.np = to_of_node(fwspec.fwnode);
+	spec.args_count = fwspec.nargs;
+	for (i = 0; i < spec.args_count; i++)
+		spec.args[i] = fwspec.args[i];
+
 	scoped_guard(mutex, &con_mutex) {
 		chan = ERR_PTR(-EPROBE_DEFER);
-		list_for_each_entry(mbox, &mbox_cons, node)
-			if (mbox->dev->of_node == spec.np) {
+		list_for_each_entry(mbox, &mbox_cons, node) {
+			if (mbox->fw_xlate && dev_fwnode(mbox->dev) == fwspec.fwnode) {
+				chan = mbox->fw_xlate(mbox, &fwspec);
+				if (!IS_ERR(chan))
+					break;
+			} else if (mbox->of_xlate && mbox->dev->of_node == spec.np) {
 				chan = mbox->of_xlate(mbox, &spec);
 				if (!IS_ERR(chan))
 					break;
 			}
+		}
 
-		of_node_put(spec.np);
+		fwnode_handle_put(fwspec.fwnode);
 
 		if (IS_ERR(chan))
 			return chan;
@@ -427,15 +440,8 @@ EXPORT_SYMBOL_GPL(mbox_request_channel);
 struct mbox_chan *mbox_request_channel_byname(struct mbox_client *cl,
 					      const char *name)
 {
-	struct device_node *np = cl->dev->of_node;
-	int index;
-
-	if (!np) {
-		dev_err(cl->dev, "%s() currently only supports DT\n", __func__);
-		return ERR_PTR(-EINVAL);
-	}
+	int index = device_property_match_string(cl->dev, "mbox-names", name);
 
-	index = of_property_match_string(np, "mbox-names", name);
 	if (index < 0) {
 		dev_err(cl->dev, "%s() could not locate channel named \"%s\"\n",
 			__func__, name);
@@ -470,9 +476,8 @@ void mbox_free_channel(struct mbox_chan *chan)
 }
 EXPORT_SYMBOL_GPL(mbox_free_channel);
 
-static struct mbox_chan *
-of_mbox_index_xlate(struct mbox_controller *mbox,
-		    const struct of_phandle_args *sp)
+static struct mbox_chan *fw_mbox_index_xlate(struct mbox_controller *mbox,
+					     const struct fwnode_reference_args *sp)
 {
 	int ind = sp->args[0];
 
@@ -523,8 +528,8 @@ int mbox_controller_register(struct mbox_controller *mbox)
 		spin_lock_init(&chan->lock);
 	}
 
-	if (!mbox->of_xlate)
-		mbox->of_xlate = of_mbox_index_xlate;
+	if (!mbox->fw_xlate && !mbox->of_xlate)
+		mbox->fw_xlate = fw_mbox_index_xlate;
 
 	scoped_guard(mutex, &con_mutex)
 		list_add_tail(&mbox->node, &mbox_cons);
diff --git a/include/linux/mailbox_controller.h b/include/linux/mailbox_controller.h
index ad01c4082358..80a427c7ca29 100644
--- a/include/linux/mailbox_controller.h
+++ b/include/linux/mailbox_controller.h
@@ -66,6 +66,7 @@ struct mbox_chan_ops {
  *			no interrupt rises. Ignored if 'txdone_irq' is set.
  * @txpoll_period:	If 'txdone_poll' is in effect, the API polls for
  *			last TX's status after these many millisecs
+ * @fw_xlate:		Controller driver specific mapping of channel via fwnode
  * @of_xlate:		Controller driver specific mapping of channel via DT
  * @poll_hrt:		API private. hrtimer used to poll for TXDONE on all
  *			channels.
@@ -79,6 +80,8 @@ struct mbox_controller {
 	bool txdone_irq;
 	bool txdone_poll;
 	unsigned txpoll_period;
+	struct mbox_chan *(*fw_xlate)(struct mbox_controller *mbox,
+				      const struct fwnode_reference_args *sp);
 	struct mbox_chan *(*of_xlate)(struct mbox_controller *mbox,
 				      const struct of_phandle_args *sp);
 	/* Internal to API */
-- 
2.43.0

On Wed, Jun 11, 2025 at 11:52:20AM +0530, Anup Patel wrote:
> Introduce optional fw_node() callback which allows a mailbox controller
> driver to provide controller specific mapping using fwnode.
> 
> The Linux OF framework already implements fwnode operations for the
> Linux DD framework so the fw_xlate() callback works fine with device
> tree as well.

...

>  struct mbox_chan *mbox_request_channel(struct mbox_client *cl, int index)
>  {
> +	struct fwnode_reference_args fwspec;

Define

	struct fwnode_handle *fwnode;


>  	struct device *dev = cl->dev;

This better to be just a declaration.

>  	struct mbox_controller *mbox;
>  	struct of_phandle_args spec;
>  	struct mbox_chan *chan;
> +	unsigned int i;
>  	int ret;

With the above the below will look like...

> -	if (!dev || !dev->of_node) {
> -		pr_debug("%s: No owner device node\n", __func__);
> +	if (!dev || !dev_fwnode(dev)) {
> +		pr_debug("No owner %s\n", dev ? "fwnode" : "device");
>  		return ERR_PTR(-ENODEV);
>  	}
>  
> -	ret = of_parse_phandle_with_args(dev->of_node, "mboxes", "#mbox-cells",
> -					 index, &spec);
> +	ret = fwnode_property_get_reference_args(dev_fwnode(dev), "mboxes",
> +						 "#mbox-cells", 0, index, &fwspec);
>  	if (ret) {
>  		dev_err(dev, "%s: can't parse \"mboxes\" property\n", __func__);
>  		return ERR_PTR(ret);
>  	}

...this

	dev = cl->dev;
	if (!dev) {
		pr_debug("No owner device\n");
		return ERR_PTR(-ENODEV);
	}

	fwnode = dev_fwnode(dev);
	if (!fwnode) {
		dev_dbg(dev, "No owner fwnode\n");
		return ERR_PTR(-ENODEV);
	}

	ret = fwnode_property_get_reference_args(fwnode, "mboxes",
						 "#mbox-cells", 0, index, &fwspec);
	if (ret) {
		dev_err(dev, "%s: can't parse \"mboxes\" property\n", __func__);

You may save a few bytes by doing it as

		dev_err(dev, "%s: can't parse \"%s\" property\n", __func__, "mboxes");

		return ERR_PTR(ret);
	}

> +	spec.np = to_of_node(fwspec.fwnode);
> +	spec.args_count = fwspec.nargs;
> +	for (i = 0; i < spec.args_count; i++)
> +		spec.args[i] = fwspec.args[i];
> +
>  	scoped_guard(mutex, &con_mutex) {
>  		chan = ERR_PTR(-EPROBE_DEFER);
> -		list_for_each_entry(mbox, &mbox_cons, node)
> -			if (mbox->dev->of_node == spec.np) {
> +		list_for_each_entry(mbox, &mbox_cons, node) {

> +			if (mbox->fw_xlate && dev_fwnode(mbox->dev) == fwspec.fwnode) {

We have a helper device_match_fwnode()

> +				chan = mbox->fw_xlate(mbox, &fwspec);
> +				if (!IS_ERR(chan))
> +					break;
> +			} else if (mbox->of_xlate && mbox->dev->of_node == spec.np) {

No need to check OF node (again). Instead refactor as

			if (device_match_fwnode(...)) {
				if (fw_xlate) {
					...
				} else if (of_xlate) {
					...
				}
			}

>  				chan = mbox->of_xlate(mbox, &spec);
>  				if (!IS_ERR(chan))
>  					break;
>  			}
> +		}
>  
> -		of_node_put(spec.np);
> +		fwnode_handle_put(fwspec.fwnode);
>  
>  		if (IS_ERR(chan))
>  			return chan;


-- 
With Best Regards,
Andy Shevchenko

On Wed, Jun 11, 2025 at 9:05 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Wed, Jun 11, 2025 at 11:52:20AM +0530, Anup Patel wrote:
> > Introduce optional fw_node() callback which allows a mailbox controller
> > driver to provide controller specific mapping using fwnode.
> >
> > The Linux OF framework already implements fwnode operations for the
> > Linux DD framework so the fw_xlate() callback works fine with device
> > tree as well.
>
> ...
>
> >  struct mbox_chan *mbox_request_channel(struct mbox_client *cl, int index)
> >  {
> > +     struct fwnode_reference_args fwspec;
>
> Define
>
>         struct fwnode_handle *fwnode;
>
>
> >       struct device *dev = cl->dev;
>
> This better to be just a declaration.
>
> >       struct mbox_controller *mbox;
> >       struct of_phandle_args spec;
> >       struct mbox_chan *chan;
> > +     unsigned int i;
> >       int ret;
>
> With the above the below will look like...
>
> > -     if (!dev || !dev->of_node) {
> > -             pr_debug("%s: No owner device node\n", __func__);
> > +     if (!dev || !dev_fwnode(dev)) {
> > +             pr_debug("No owner %s\n", dev ? "fwnode" : "device");
> >               return ERR_PTR(-ENODEV);
> >       }
> >
> > -     ret = of_parse_phandle_with_args(dev->of_node, "mboxes", "#mbox-cells",
> > -                                      index, &spec);
> > +     ret = fwnode_property_get_reference_args(dev_fwnode(dev), "mboxes",
> > +                                              "#mbox-cells", 0, index, &fwspec);
> >       if (ret) {
> >               dev_err(dev, "%s: can't parse \"mboxes\" property\n", __func__);
> >               return ERR_PTR(ret);
> >       }
>
> ...this
>
>         dev = cl->dev;
>         if (!dev) {
>                 pr_debug("No owner device\n");
>                 return ERR_PTR(-ENODEV);
>         }
>
>         fwnode = dev_fwnode(dev);
>         if (!fwnode) {
>                 dev_dbg(dev, "No owner fwnode\n");
>                 return ERR_PTR(-ENODEV);
>         }
>
>         ret = fwnode_property_get_reference_args(fwnode, "mboxes",
>                                                  "#mbox-cells", 0, index, &fwspec);
>         if (ret) {
>                 dev_err(dev, "%s: can't parse \"mboxes\" property\n", __func__);
>
> You may save a few bytes by doing it as
>
>                 dev_err(dev, "%s: can't parse \"%s\" property\n", __func__, "mboxes");
>
>                 return ERR_PTR(ret);
>         }
>
> > +     spec.np = to_of_node(fwspec.fwnode);
> > +     spec.args_count = fwspec.nargs;
> > +     for (i = 0; i < spec.args_count; i++)
> > +             spec.args[i] = fwspec.args[i];
> > +
> >       scoped_guard(mutex, &con_mutex) {
> >               chan = ERR_PTR(-EPROBE_DEFER);
> > -             list_for_each_entry(mbox, &mbox_cons, node)
> > -                     if (mbox->dev->of_node == spec.np) {
> > +             list_for_each_entry(mbox, &mbox_cons, node) {
>
> > +                     if (mbox->fw_xlate && dev_fwnode(mbox->dev) == fwspec.fwnode) {
>
> We have a helper device_match_fwnode()
>
> > +                             chan = mbox->fw_xlate(mbox, &fwspec);
> > +                             if (!IS_ERR(chan))
> > +                                     break;
> > +                     } else if (mbox->of_xlate && mbox->dev->of_node == spec.np) {
>
> No need to check OF node (again). Instead refactor as
>
>                         if (device_match_fwnode(...)) {
>                                 if (fw_xlate) {
>                                         ...
>                                 } else if (of_xlate) {
>                                         ...
>                                 }
>                         }
>

Okay, I will update like you suggested.

Regards,
Anup