[PATCH v5 07/28] driver core: fw_devlink: Introduce fw_devlink_set_device()

Herve Codina posted 28 patches 1 month, 1 week ago
There is a newer version of this series
[PATCH v5 07/28] driver core: fw_devlink: Introduce fw_devlink_set_device()
Posted by Herve Codina 1 month, 1 week ago
Setting fwnode->dev is specific to fw_devlink.

In order to avoid having a direct 'fwnode->dev = dev;' in several
place in the kernel, introduce fw_devlink_set_device() helper to perform
this operation.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 include/linux/fwnode.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h
index a921ca2fe940..a1345e274125 100644
--- a/include/linux/fwnode.h
+++ b/include/linux/fwnode.h
@@ -231,4 +231,10 @@ void fw_devlink_purge_absent_suppliers(struct fwnode_handle *fwnode);
 void fw_devlink_refresh_fwnode(struct fwnode_handle *fwnode);
 bool fw_devlink_is_strict(void);
 
+static inline void fw_devlink_set_device(struct fwnode_handle *fwnode,
+					 struct device *dev)
+{
+	fwnode->dev = dev;
+}
+
 #endif
-- 
2.53.0
Re: [PATCH v5 07/28] driver core: fw_devlink: Introduce fw_devlink_set_device()
Posted by Jonathan Cameron 1 month ago
On Fri, 27 Feb 2026 14:54:04 +0100
Herve Codina <herve.codina@bootlin.com> wrote:

> Setting fwnode->dev is specific to fw_devlink.
> 
> In order to avoid having a direct 'fwnode->dev = dev;' in several
> place in the kernel, introduce fw_devlink_set_device() helper to perform
> this operation.
> 
I don't mind the helper, but the description could do with a little
detail on why.  Is it just to avoid visibility of internal details, or
is there a stronger reason?

> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>  include/linux/fwnode.h | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h
> index a921ca2fe940..a1345e274125 100644
> --- a/include/linux/fwnode.h
> +++ b/include/linux/fwnode.h
> @@ -231,4 +231,10 @@ void fw_devlink_purge_absent_suppliers(struct fwnode_handle *fwnode);
>  void fw_devlink_refresh_fwnode(struct fwnode_handle *fwnode);
>  bool fw_devlink_is_strict(void);
>  
> +static inline void fw_devlink_set_device(struct fwnode_handle *fwnode,
> +					 struct device *dev)
> +{
> +	fwnode->dev = dev;
> +}
> +
>  #endif
Re: [PATCH v5 07/28] driver core: fw_devlink: Introduce fw_devlink_set_device()
Posted by Herve Codina 1 month ago
Hi Jonathan,

On Mon, 2 Mar 2026 12:23:36 +0000
Jonathan Cameron <jonathan.cameron@huawei.com> wrote:

> On Fri, 27 Feb 2026 14:54:04 +0100
> Herve Codina <herve.codina@bootlin.com> wrote:
> 
> > Setting fwnode->dev is specific to fw_devlink.
> > 
> > In order to avoid having a direct 'fwnode->dev = dev;' in several
> > place in the kernel, introduce fw_devlink_set_device() helper to perform
> > this operation.
> >   
> I don't mind the helper, but the description could do with a little
> detail on why.  Is it just to avoid visibility of internal details, or
> is there a stronger reason?

I think the idea was to avoid visibility.

It cames from feedback received on my first iteration
  https://lore.kernel.org/all/20250408145139.293c79a2@bootlin.com/

I found the idea relevant and so I did the patch.

Best regards,
Hervé

Re: [PATCH v5 07/28] driver core: fw_devlink: Introduce fw_devlink_set_device()
Posted by Andy Shevchenko 1 month ago
On Tue, Mar 03, 2026 at 03:12:10PM +0100, Herve Codina wrote:
> On Mon, 2 Mar 2026 12:23:36 +0000
> Jonathan Cameron <jonathan.cameron@huawei.com> wrote:
> > On Fri, 27 Feb 2026 14:54:04 +0100
> > Herve Codina <herve.codina@bootlin.com> wrote:
> > 
> > > Setting fwnode->dev is specific to fw_devlink.
> > > 
> > > In order to avoid having a direct 'fwnode->dev = dev;' in several
> > > place in the kernel, introduce fw_devlink_set_device() helper to perform
> > > this operation.
> > >   
> > I don't mind the helper, but the description could do with a little
> > detail on why.  Is it just to avoid visibility of internal details, or
> > is there a stronger reason?
> 
> I think the idea was to avoid visibility.
> 
> It cames from feedback received on my first iteration
>   https://lore.kernel.org/all/20250408145139.293c79a2@bootlin.com/
> 
> I found the idea relevant and so I did the patch.

Yes, the idea is to hide the fwnode devlink related stuff behind the getters
and setters. Ideally, everything in fwnode_handle related to devlinks should
be marked as __private.

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH v5 07/28] driver core: fw_devlink: Introduce fw_devlink_set_device()
Posted by Charles Keepax 1 month, 1 week ago
On Fri, Feb 27, 2026 at 02:54:04PM +0100, Herve Codina wrote:
> Setting fwnode->dev is specific to fw_devlink.
> 
> In order to avoid having a direct 'fwnode->dev = dev;' in several
> place in the kernel, introduce fw_devlink_set_device() helper to perform
> this operation.
> 
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---

Reviewed-by: Charles Keepax <ckeepax@opensource.cirrus.com>

Thanks,
Charles