public inbox for driver-core@lists.linux.dev
 help / color / mirror / Atom feed
* [PATCH] software node: provide wrappers around kobject_get/put()
@ 2026-04-24 12:42 Bartosz Golaszewski
  2026-04-24 13:33 ` Andy Shevchenko
  0 siblings, 1 reply; 6+ messages in thread
From: Bartosz Golaszewski @ 2026-04-24 12:42 UTC (permalink / raw)
  To: Andy Shevchenko, Daniel Scally, Heikki Krogerus, Sakari Ailus,
	Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich
  Cc: linux-acpi, driver-core, linux-kernel, brgl, Bartosz Golaszewski

Make the code more readable by avoid constant dereferencing of the
swnode's kobject when managing references. Provide wrappers that take
struct swnode * as argument and make them hide that logic.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
 drivers/base/swnode.c | 36 ++++++++++++++++++++++--------------
 1 file changed, 22 insertions(+), 14 deletions(-)

diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
index a19f8f722bc8e..503908c04ae66 100644
--- a/drivers/base/swnode.c
+++ b/drivers/base/swnode.c
@@ -374,20 +374,28 @@ EXPORT_SYMBOL_GPL(property_entries_free);
 /* -------------------------------------------------------------------------- */
 /* fwnode operations */
 
+static void swnode_get(struct swnode *swnode)
+{
+	kobject_get(&swnode->kobj);
+}
+
+static void swnode_put(struct swnode *swnode)
+{
+	kobject_put(&swnode->kobj);
+}
+
 static struct fwnode_handle *software_node_get(struct fwnode_handle *fwnode)
 {
 	struct swnode *swnode = to_swnode(fwnode);
 
-	kobject_get(&swnode->kobj);
+	swnode_get(swnode);
 
 	return &swnode->fwnode;
 }
 
 static void software_node_put(struct fwnode_handle *fwnode)
 {
-	struct swnode *swnode = to_swnode(fwnode);
-
-	kobject_put(&swnode->kobj);
+	swnode_put(to_swnode(fwnode));
 }
 
 static bool software_node_property_present(const struct fwnode_handle *fwnode,
@@ -493,7 +501,7 @@ software_node_get_named_child_node(const struct fwnode_handle *fwnode,
 
 	list_for_each_entry(child, &swnode->children, entry) {
 		if (!strcmp(childname, kobject_name(&child->kobj))) {
-			kobject_get(&child->kobj);
+			swnode_get(child);
 			return &child->fwnode;
 		}
 	}
@@ -737,7 +745,7 @@ software_node_find_by_name(const struct software_node *parent, const char *name)
 		swnode = kobj_to_swnode(k);
 		if (parent == swnode->node->parent && swnode->node->name &&
 		    !strcmp(name, swnode->node->name)) {
-			kobject_get(&swnode->kobj);
+			swnode_get(swnode);
 			break;
 		}
 		swnode = NULL;
@@ -835,13 +843,13 @@ swnode_register(const struct software_node *node, struct swnode *parent,
 					   parent ? &parent->kobj : NULL,
 					   "node%d", swnode->id);
 	if (ret) {
-		kobject_put(&swnode->kobj);
+		swnode_put(swnode);
 		return ERR_PTR(ret);
 	}
 
 	/*
 	 * Assign the flag only in the successful case, so
-	 * the above kobject_put() won't mess up with properties.
+	 * the above swnode_put() won't mess up with properties.
 	 */
 	swnode->allocated = allocated;
 
@@ -978,7 +986,7 @@ void fwnode_remove_software_node(struct fwnode_handle *fwnode)
 	if (!swnode)
 		return;
 
-	kobject_put(&swnode->kobj);
+	swnode_put(swnode);
 }
 EXPORT_SYMBOL_GPL(fwnode_remove_software_node);
 
@@ -1002,7 +1010,7 @@ int device_add_software_node(struct device *dev, const struct software_node *nod
 
 	swnode = software_node_to_swnode(node);
 	if (swnode) {
-		kobject_get(&swnode->kobj);
+		swnode_get(swnode);
 	} else {
 		ret = software_node_register(node);
 		if (ret)
@@ -1044,7 +1052,7 @@ void device_remove_software_node(struct device *dev)
 		software_node_notify_remove(dev);
 
 	set_secondary_fwnode(dev, NULL);
-	kobject_put(&swnode->kobj);
+	swnode_put(swnode);
 }
 EXPORT_SYMBOL_GPL(device_remove_software_node);
 
@@ -1097,7 +1105,7 @@ void software_node_notify(struct device *dev)
 	if (!swnode)
 		return;
 
-	kobject_get(&swnode->kobj);
+	swnode_get(swnode);
 	ret = sysfs_create_link(&dev->kobj, &swnode->kobj, "software_node");
 	if (ret)
 		return;
@@ -1119,11 +1127,11 @@ void software_node_notify_remove(struct device *dev)
 
 	sysfs_remove_link(&swnode->kobj, dev_name(dev));
 	sysfs_remove_link(&dev->kobj, "software_node");
-	kobject_put(&swnode->kobj);
+	swnode_put(swnode);
 
 	if (swnode->managed) {
 		set_secondary_fwnode(dev, NULL);
-		kobject_put(&swnode->kobj);
+		swnode_put(swnode);
 	}
 }
 
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] software node: provide wrappers around kobject_get/put()
  2026-04-24 12:42 [PATCH] software node: provide wrappers around kobject_get/put() Bartosz Golaszewski
@ 2026-04-24 13:33 ` Andy Shevchenko
  2026-04-24 13:43   ` Bartosz Golaszewski
  0 siblings, 1 reply; 6+ messages in thread
From: Andy Shevchenko @ 2026-04-24 13:33 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Daniel Scally, Heikki Krogerus, Sakari Ailus, Greg Kroah-Hartman,
	Rafael J. Wysocki, Danilo Krummrich, linux-acpi, driver-core,
	linux-kernel, brgl

On Fri, Apr 24, 2026 at 02:42:50PM +0200, Bartosz Golaszewski wrote:
> Make the code more readable by avoid constant dereferencing of the
> swnode's kobject when managing references. Provide wrappers that take
> struct swnode * as argument and make them hide that logic.

...

> +static void swnode_get(struct swnode *swnode)
> +{
> +	kobject_get(&swnode->kobj);
> +}

Isn't a pattern to return the object itself so the code can bump the reference
in a single statement (if appropriate)?

...

>  static struct fwnode_handle *software_node_get(struct fwnode_handle *fwnode)
>  {
>  	struct swnode *swnode = to_swnode(fwnode);
>  
> -	kobject_get(&swnode->kobj);
> +	swnode_get(swnode);

Like

	struct swnode *swnode = swnode_get(to_swnode(fwnode));

>  	return &swnode->fwnode;
>  }

...

>  	list_for_each_entry(child, &swnode->children, entry) {
>  		if (!strcmp(childname, kobject_name(&child->kobj))) {
> -			kobject_get(&child->kobj);
> +			swnode_get(child);

Becomes inconsistent with kobject_name()...

>  			return &child->fwnode;
>  		}
>  	}

...

>  		swnode = kobj_to_swnode(k);
>  		if (parent == swnode->node->parent && swnode->node->name &&
>  		    !strcmp(name, swnode->node->name)) {
> -			kobject_get(&swnode->kobj);
> +			swnode_get(swnode);

Also not sure. Maybe use 'k'?

>  			break;
>  		}

...

Otherwise LGTM and makes sense.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] software node: provide wrappers around kobject_get/put()
  2026-04-24 13:33 ` Andy Shevchenko
@ 2026-04-24 13:43   ` Bartosz Golaszewski
  2026-04-24 15:32     ` Andy Shevchenko
  0 siblings, 1 reply; 6+ messages in thread
From: Bartosz Golaszewski @ 2026-04-24 13:43 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Bartosz Golaszewski, Daniel Scally, Heikki Krogerus, Sakari Ailus,
	Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
	linux-acpi, driver-core, linux-kernel

On Fri, Apr 24, 2026 at 3:33 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Fri, Apr 24, 2026 at 02:42:50PM +0200, Bartosz Golaszewski wrote:
> > Make the code more readable by avoid constant dereferencing of the
> > swnode's kobject when managing references. Provide wrappers that take
> > struct swnode * as argument and make them hide that logic.
>
> ...
>
> > +static void swnode_get(struct swnode *swnode)
> > +{
> > +     kobject_get(&swnode->kobj);
> > +}
>
> Isn't a pattern to return the object itself so the code can bump the reference
> in a single statement (if appropriate)?
>

Typically yeah but nobody here would use it right now. I can change it
if you prefer it.

>
> ...
>
> >       list_for_each_entry(child, &swnode->children, entry) {
> >               if (!strcmp(childname, kobject_name(&child->kobj))) {
> > -                     kobject_get(&child->kobj);
> > +                     swnode_get(child);
>
> Becomes inconsistent with kobject_name()...
>

What do you mean?

> >                       return &child->fwnode;
> >               }
> >       }
>
> ...
>
> >               swnode = kobj_to_swnode(k);
> >               if (parent == swnode->node->parent && swnode->node->name &&
> >                   !strcmp(name, swnode->node->name)) {
> > -                     kobject_get(&swnode->kobj);
> > +                     swnode_get(swnode);
>
> Also not sure. Maybe use 'k'?
>

I'm not following either, please rephrase.

Bart

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] software node: provide wrappers around kobject_get/put()
  2026-04-24 13:43   ` Bartosz Golaszewski
@ 2026-04-24 15:32     ` Andy Shevchenko
  2026-04-27  8:15       ` Bartosz Golaszewski
  0 siblings, 1 reply; 6+ messages in thread
From: Andy Shevchenko @ 2026-04-24 15:32 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Bartosz Golaszewski, Daniel Scally, Heikki Krogerus, Sakari Ailus,
	Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
	linux-acpi, driver-core, linux-kernel

On Fri, Apr 24, 2026 at 03:43:39PM +0200, Bartosz Golaszewski wrote:
> On Fri, Apr 24, 2026 at 3:33 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Fri, Apr 24, 2026 at 02:42:50PM +0200, Bartosz Golaszewski wrote:

...

> > Isn't a pattern to return the object itself so the code can bump the reference
> > in a single statement (if appropriate)?
> 
> Typically yeah but nobody here would use it right now. I can change it
> if you prefer it.

I showed below the example (it's now cut from the context).

...

> > >       list_for_each_entry(child, &swnode->children, entry) {
> > >               if (!strcmp(childname, kobject_name(&child->kobj))) {
> > > -                     kobject_get(&child->kobj);
> > > +                     swnode_get(child);
> >
> > Becomes inconsistent with kobject_name()...
> 
> What do you mean?
> 
> > >                       return &child->fwnode;
> > >               }
> > >       }

The same piece of code uses kobj and child either way of this conversion happen
or not. To be fully consistent we need something like swnode_match_name() or
alike instead of that strcmp().

...

> > >               swnode = kobj_to_swnode(k);
> > >               if (parent == swnode->node->parent && swnode->node->name &&
> > >                   !strcmp(name, swnode->node->name)) {
> > > -                     kobject_get(&swnode->kobj);
> > > +                     swnode_get(swnode);
> >
> > Also not sure. Maybe use 'k'?
> 
> I'm not following either, please rephrase.

                     kobject_get(k);

should also work. And here I referred to the fact that we already have a kobject
instance available. Yes, my counter example is just to reveal inconsistency,
I kinda agree that using swnode specific API might be clearer approach.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] software node: provide wrappers around kobject_get/put()
  2026-04-24 15:32     ` Andy Shevchenko
@ 2026-04-27  8:15       ` Bartosz Golaszewski
  2026-04-27  8:34         ` Andy Shevchenko
  0 siblings, 1 reply; 6+ messages in thread
From: Bartosz Golaszewski @ 2026-04-27  8:15 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Bartosz Golaszewski, Daniel Scally, Heikki Krogerus, Sakari Ailus,
	Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
	linux-acpi, driver-core, linux-kernel

On Fri, Apr 24, 2026 at 5:32 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> should also work. And here I referred to the fact that we already have a kobject
> instance available. Yes, my counter example is just to reveal inconsistency,
> I kinda agree that using swnode specific API might be clearer approach.
>

Let's do kobject_name() -> swnode_name() separately?

Bart

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] software node: provide wrappers around kobject_get/put()
  2026-04-27  8:15       ` Bartosz Golaszewski
@ 2026-04-27  8:34         ` Andy Shevchenko
  0 siblings, 0 replies; 6+ messages in thread
From: Andy Shevchenko @ 2026-04-27  8:34 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Bartosz Golaszewski, Daniel Scally, Heikki Krogerus, Sakari Ailus,
	Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
	linux-acpi, driver-core, linux-kernel

On Mon, Apr 27, 2026 at 10:15:54AM +0200, Bartosz Golaszewski wrote:
> On Fri, Apr 24, 2026 at 5:32 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > should also work. And here I referred to the fact that we already have a kobject
> > instance available. Yes, my counter example is just to reveal inconsistency,
> > I kinda agree that using swnode specific API might be clearer approach.
> 
> Let's do kobject_name() -> swnode_name() separately?

OK.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-04-27  8:34 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-24 12:42 [PATCH] software node: provide wrappers around kobject_get/put() Bartosz Golaszewski
2026-04-24 13:33 ` Andy Shevchenko
2026-04-24 13:43   ` Bartosz Golaszewski
2026-04-24 15:32     ` Andy Shevchenko
2026-04-27  8:15       ` Bartosz Golaszewski
2026-04-27  8:34         ` Andy Shevchenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox