[PATCH 0/2] software node: Two fixes

[PATCH 0/2] software node: Two fixes

[Heikki Krogerus]

Hi,

The second one only affects 5.12-rc1.


Heikki Krogerus (2):
  software node: Fix node registration
  software node: Fix device_add_software_node()

 drivers/base/swnode.c    | 29 ++++++++++++++++++++---------
 include/linux/property.h |  2 +-
 2 files changed, 21 insertions(+), 10 deletions(-)

-- 
2.30.1

[PATCH 1/2] software node: Fix node registration

[Heikki Krogerus]

Software node can not be registered before its parent.

Fixes: 80488a6b1d3c ("software node: Add support for static node descriptors")
Cc: stable@vger.kernel.org
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/base/swnode.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
index 37179a8b1ceba..74db8c971db74 100644
--- a/drivers/base/swnode.c
+++ b/drivers/base/swnode.c
@@ -938,6 +938,9 @@ int software_node_register(const struct software_node *node)
 	if (software_node_to_swnode(node))
 		return -EEXIST;
 
+	if (node->parent && !parent)
+		return -EINVAL;
+
 	return PTR_ERR_OR_ZERO(swnode_register(node, parent, 0));
 }
 EXPORT_SYMBOL_GPL(software_node_register);
-- 
2.30.1

Re: [PATCH 1/2] software node: Fix node registration

[Andy Shevchenko]

On Mon, Mar 01, 2021 at 05:30:11PM +0300, Heikki Krogerus wrote:
> Software node can not be registered before its parent.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Tested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

(On Intel Galileo Gen 2 with some custom patches to convert gpio-dwapb et al.
 to use swnodes. Those patches a subject to further submission.)

Thanks!

> Fixes: 80488a6b1d3c ("software node: Add support for static node descriptors")
> Cc: stable@vger.kernel.org
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
>  drivers/base/swnode.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
> index 37179a8b1ceba..74db8c971db74 100644
> --- a/drivers/base/swnode.c
> +++ b/drivers/base/swnode.c
> @@ -938,6 +938,9 @@ int software_node_register(const struct software_node *node)
>  	if (software_node_to_swnode(node))
>  		return -EEXIST;
>  
> +	if (node->parent && !parent)
> +		return -EINVAL;
> +
>  	return PTR_ERR_OR_ZERO(swnode_register(node, parent, 0));
>  }
>  EXPORT_SYMBOL_GPL(software_node_register);
> -- 
> 2.30.1
> 

-- 
With Best Regards,
Andy Shevchenko

[PATCH 2/2] software node: Fix device_add_software_node()

[Heikki Krogerus]

The function device_add_software_node() was meant to
register the node supplied to it, but only if that node
wasn't already registered. Right now the function attempts
to always register the node. That will cause a failure with
nodes that are already registered.

Fixing that by incrementing the reference count of the nodes
that have already been registered, and only registering the
new nodes. Also, clarifying the behaviour in the function
documentation.

Fixes: e68d0119e328 ("software node: Introduce device_add_software_node()")
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/base/swnode.c    | 26 +++++++++++++++++---------
 include/linux/property.h |  2 +-
 2 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
index 74db8c971db74..fa3719ef80e4d 100644
--- a/drivers/base/swnode.c
+++ b/drivers/base/swnode.c
@@ -1005,25 +1005,33 @@ EXPORT_SYMBOL_GPL(fwnode_remove_software_node);
 /**
  * device_add_software_node - Assign software node to a device
  * @dev: The device the software node is meant for.
- * @swnode: The software node.
+ * @node: The software node.
  *
- * This function will register @swnode and make it the secondary firmware node
- * pointer of @dev. If @dev has no primary node, then @swnode will become the primary
- * node.
+ * This function will make @node the secondary firmware node pointer of @dev. If
+ * @dev has no primary node, then @node will become the primary node. The
+ * function will register @node automatically if it wasn't already registered.
  */
-int device_add_software_node(struct device *dev, const struct software_node *swnode)
+int device_add_software_node(struct device *dev, const struct software_node *node)
 {
+	struct swnode *swnode;
 	int ret;
 
 	/* Only one software node per device. */
 	if (dev_to_swnode(dev))
 		return -EBUSY;
 
-	ret = software_node_register(swnode);
-	if (ret)
-		return ret;
+	swnode = software_node_to_swnode(node);
+	if (swnode) {
+		kobject_get(&swnode->kobj);
+	} else {
+		ret = software_node_register(node);
+		if (ret)
+			return ret;
+
+		swnode = software_node_to_swnode(node);
+	}
 
-	set_secondary_fwnode(dev, software_node_fwnode(swnode));
+	set_secondary_fwnode(dev, &swnode->fwnode);
 
 	return 0;
 }
diff --git a/include/linux/property.h b/include/linux/property.h
index dafccfce02624..dd4687b562393 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -488,7 +488,7 @@ fwnode_create_software_node(const struct property_entry *properties,
 			    const struct fwnode_handle *parent);
 void fwnode_remove_software_node(struct fwnode_handle *fwnode);
 
-int device_add_software_node(struct device *dev, const struct software_node *swnode);
+int device_add_software_node(struct device *dev, const struct software_node *node);
 void device_remove_software_node(struct device *dev);
 
 int device_create_managed_software_node(struct device *dev,
-- 
2.30.1

Re: [PATCH 2/2] software node: Fix device_add_software_node()

[Andy Shevchenko]

On Mon, Mar 01, 2021 at 05:30:12PM +0300, Heikki Krogerus wrote:
> The function device_add_software_node() was meant to
> register the node supplied to it, but only if that node
> wasn't already registered. Right now the function attempts
> to always register the node. That will cause a failure with
> nodes that are already registered.
> 
> Fixing that by incrementing the reference count of the nodes
> that have already been registered, and only registering the
> new nodes. Also, clarifying the behaviour in the function
> documentation.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Tested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

(On Intel Galileo Gen 2 with some custom patches to convert gpio-dwapb et al.
  to use swnodes. Those patches a subject to further submission.)

Thanks!

> Fixes: e68d0119e328 ("software node: Introduce device_add_software_node()")
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
>  drivers/base/swnode.c    | 26 +++++++++++++++++---------
>  include/linux/property.h |  2 +-
>  2 files changed, 18 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
> index 74db8c971db74..fa3719ef80e4d 100644
> --- a/drivers/base/swnode.c
> +++ b/drivers/base/swnode.c
> @@ -1005,25 +1005,33 @@ EXPORT_SYMBOL_GPL(fwnode_remove_software_node);
>  /**
>   * device_add_software_node - Assign software node to a device
>   * @dev: The device the software node is meant for.
> - * @swnode: The software node.
> + * @node: The software node.
>   *
> - * This function will register @swnode and make it the secondary firmware node
> - * pointer of @dev. If @dev has no primary node, then @swnode will become the primary
> - * node.
> + * This function will make @node the secondary firmware node pointer of @dev. If
> + * @dev has no primary node, then @node will become the primary node. The
> + * function will register @node automatically if it wasn't already registered.
>   */
> -int device_add_software_node(struct device *dev, const struct software_node *swnode)
> +int device_add_software_node(struct device *dev, const struct software_node *node)
>  {
> +	struct swnode *swnode;
>  	int ret;
>  
>  	/* Only one software node per device. */
>  	if (dev_to_swnode(dev))
>  		return -EBUSY;
>  
> -	ret = software_node_register(swnode);
> -	if (ret)
> -		return ret;
> +	swnode = software_node_to_swnode(node);
> +	if (swnode) {
> +		kobject_get(&swnode->kobj);
> +	} else {
> +		ret = software_node_register(node);
> +		if (ret)
> +			return ret;
> +
> +		swnode = software_node_to_swnode(node);
> +	}
>  
> -	set_secondary_fwnode(dev, software_node_fwnode(swnode));
> +	set_secondary_fwnode(dev, &swnode->fwnode);
>  
>  	return 0;
>  }
> diff --git a/include/linux/property.h b/include/linux/property.h
> index dafccfce02624..dd4687b562393 100644
> --- a/include/linux/property.h
> +++ b/include/linux/property.h
> @@ -488,7 +488,7 @@ fwnode_create_software_node(const struct property_entry *properties,
>  			    const struct fwnode_handle *parent);
>  void fwnode_remove_software_node(struct fwnode_handle *fwnode);
>  
> -int device_add_software_node(struct device *dev, const struct software_node *swnode);
> +int device_add_software_node(struct device *dev, const struct software_node *node);
>  void device_remove_software_node(struct device *dev);
>  
>  int device_create_managed_software_node(struct device *dev,
> -- 
> 2.30.1
> 

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 0/2] software node: Two fixes

[Andy Shevchenko]

On Mon, Mar 01, 2021 at 05:30:10PM +0300, Heikki Krogerus wrote:
> Hi,
> 
> The second one only affects 5.12-rc1.

Rafael, Greg, can this be applied for v5.12-rcX?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 0/2] software node: Two fixes

[Rafael J. Wysocki]

On Tue, Mar 9, 2021 at 11:51 AM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Mon, Mar 01, 2021 at 05:30:10PM +0300, Heikki Krogerus wrote:
> > Hi,
> >
> > The second one only affects 5.12-rc1.
>
> Rafael, Greg, can this be applied for v5.12-rcX?

Do you have a pointer to this?

Re: [PATCH 0/2] software node: Two fixes

[Heikki Krogerus]

On Tue, Mar 09, 2021 at 02:51:22PM +0100, Rafael J. Wysocki wrote:
> On Tue, Mar 9, 2021 at 11:51 AM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > On Mon, Mar 01, 2021 at 05:30:10PM +0300, Heikki Krogerus wrote:
> > > Hi,
> > >
> > > The second one only affects 5.12-rc1.
> >
> > Rafael, Greg, can this be applied for v5.12-rcX?
> 
> Do you have a pointer to this?

Pointer?

thanks,

-- 
heikki

Re: [PATCH 0/2] software node: Two fixes

[Rafael J. Wysocki]

On Wed, Mar 10, 2021 at 8:36 AM Heikki Krogerus
<heikki.krogerus@linux.intel.com> wrote:
>
> On Tue, Mar 09, 2021 at 02:51:22PM +0100, Rafael J. Wysocki wrote:
> > On Tue, Mar 9, 2021 at 11:51 AM Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > >
> > > On Mon, Mar 01, 2021 at 05:30:10PM +0300, Heikki Krogerus wrote:
> > > > Hi,
> > > >
> > > > The second one only affects 5.12-rc1.
> > >
> > > Rafael, Greg, can this be applied for v5.12-rcX?
> >
> > Do you have a pointer to this?
>
> Pointer?

Never mind, I'll pick them up from the archive.

Re: [PATCH 0/2] software node: Two fixes

[Rafael J. Wysocki]

On Wed, Mar 10, 2021 at 3:16 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Wed, Mar 10, 2021 at 8:36 AM Heikki Krogerus
> <heikki.krogerus@linux.intel.com> wrote:
> >
> > On Tue, Mar 09, 2021 at 02:51:22PM +0100, Rafael J. Wysocki wrote:
> > > On Tue, Mar 9, 2021 at 11:51 AM Andy Shevchenko
> > > <andriy.shevchenko@linux.intel.com> wrote:
> > > >
> > > > On Mon, Mar 01, 2021 at 05:30:10PM +0300, Heikki Krogerus wrote:
> > > > > Hi,
> > > > >
> > > > > The second one only affects 5.12-rc1.
> > > >
> > > > Rafael, Greg, can this be applied for v5.12-rcX?
> > >
> > > Do you have a pointer to this?
> >
> > Pointer?
>
> Never mind, I'll pick them up from the archive.

Both [1-2/2] applied as 5.12-rc material now, thanks!

However, I would appreciate CCing swnode-related material to linux-acpi.

Re: [PATCH 0/2] software node: Two fixes

[Heikki Krogerus]

On Wed, Mar 10, 2021 at 03:30:09PM +0100, Rafael J. Wysocki wrote:
> On Wed, Mar 10, 2021 at 3:16 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
> >
> > On Wed, Mar 10, 2021 at 8:36 AM Heikki Krogerus
> > <heikki.krogerus@linux.intel.com> wrote:
> > >
> > > On Tue, Mar 09, 2021 at 02:51:22PM +0100, Rafael J. Wysocki wrote:
> > > > On Tue, Mar 9, 2021 at 11:51 AM Andy Shevchenko
> > > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > >
> > > > > On Mon, Mar 01, 2021 at 05:30:10PM +0300, Heikki Krogerus wrote:
> > > > > > Hi,
> > > > > >
> > > > > > The second one only affects 5.12-rc1.
> > > > >
> > > > > Rafael, Greg, can this be applied for v5.12-rcX?
> > > >
> > > > Do you have a pointer to this?
> > >
> > > Pointer?
> >
> > Never mind, I'll pick them up from the archive.
> 
> Both [1-2/2] applied as 5.12-rc material now, thanks!
> 
> However, I would appreciate CCing swnode-related material to linux-acpi.

Sure think. Maybe it would be better to add an entry for the thing to
the MAINTAINERS file. That way we can make linux-acpi the ml for it. I
have half automated the preparation of patches and I rely on the
get_maintainer.pl script, so it would make at least my life a bit
easier :-).

I'll send you the patch. I'll make myself and Andy the reviewers.

thanks,

-- 
heikki