[PATCH v4 0/3] device property: Remove device_add_properties()

[PATCH v4 0/3] device property: Remove device_add_properties()

[Heikki Krogerus]

Hi,

One more version. Hopefully the commit messages are now OK. No other
changes since v3:

https://lore.kernel.org/lkml/20211006112643.77684-1-heikki.krogerus@linux.intel.com/


v3 cover letter:

In this third version of this series, the second patch is now split in
two. The device_remove_properties() call is first removed from
device_del() in its own patch, and the
device_add/remove_properties() API is removed separately in the last
patch. I hope the commit messages are clear enough this time.


v2 cover letter:

This is the second version where I only modified the commit message of
the first patch according to comments from Bjorn.


Original cover letter:

There is one user left for the API, so converting that to use software
node API instead, and removing the function.


thanks,

Heikki Krogerus (3):
  PCI: Convert to device_create_managed_software_node()
  driver core: Don't call device_remove_properties() from device_del()
  device property: Remove device_add_properties() API

 drivers/base/core.c      |  1 -
 drivers/base/property.c  | 48 ----------------------------------------
 drivers/pci/quirks.c     |  2 +-
 include/linux/property.h |  4 ----
 4 files changed, 1 insertion(+), 54 deletions(-)

-- 
2.33.0

[PATCH v4 1/3] PCI: Convert to device_create_managed_software_node()

[Heikki Krogerus]

In quirk_huawei_pcie_sva(), device_add_properties() is used to
inject additional device properties, but there is no
device_remove_properties() call anywhere to remove those
properties. The assumption is most likely that the device is
never removed, and the properties therefore do not also never
need to be removed.

Even though it is unlikely that the device is ever removed in
this case, it is safer to make sure that the properties are
also removed if the device ever does get unregistered.

To achieve this, instead of adding a separate quirk for the
case of device removal where device_remove_properties() is
called, using device_create_managed_software_node() instead of
device_add_properties(). Both functions create a software node
(a type of fwnode) that holds the device properties, which is
then assigned to the device very much the same way.

The difference between the two functions is, that
device_create_managed_software_node() guarantees that the
software node (together with the properties) is removed when
the device is removed. The function device_add_property() does
not guarantee that, so the properties added with it should
always be removed with device_remove_properties().

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Acked-by: Zhangfei Gao <zhangfei.gao@linaro.org>
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/pci/quirks.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 003950c738d26..b4cb658cce2b1 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -1850,7 +1850,7 @@ static void quirk_huawei_pcie_sva(struct pci_dev *pdev)
 	 * can set it directly.
 	 */
 	if (!pdev->dev.of_node &&
-	    device_add_properties(&pdev->dev, properties))
+	    device_create_managed_software_node(&pdev->dev, properties, NULL))
 		pci_warn(pdev, "could not add stall property");
 }
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_HUAWEI, 0xa250, quirk_huawei_pcie_sva);
-- 
2.33.0

[PATCH v4 2/3] driver core: Don't call device_remove_properties() from device_del()

[Heikki Krogerus]

All the drivers that relied on device_del() to call
device_remove_properties() have now been converted to either
use device_create_managed_software_node() instead of
device_add_properties(), or to register the software node
completely separately from the device.

This will make it finally possible to share and reuse the
software nodes that hold the additional device properties.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/base/core.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index fd034d7424472..a40b6fb1ebb01 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -3582,7 +3582,6 @@ void device_del(struct device *dev)
 	device_pm_remove(dev);
 	driver_deferred_probe_del(dev);
 	device_platform_notify_remove(dev);
-	device_remove_properties(dev);
 	device_links_purge(dev);
 
 	if (dev->bus)
-- 
2.33.0

[PATCH v4 3/3] device property: Remove device_add_properties() API

[Heikki Krogerus]

There are no more users for it.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/base/property.c  | 48 ----------------------------------------
 include/linux/property.h |  4 ----
 2 files changed, 52 deletions(-)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index f1f35b48ab8b9..d0960a9e89741 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -507,54 +507,6 @@ struct fwnode_handle *fwnode_find_reference(const struct fwnode_handle *fwnode,
 }
 EXPORT_SYMBOL_GPL(fwnode_find_reference);
 
-/**
- * device_remove_properties - Remove properties from a device object.
- * @dev: Device whose properties to remove.
- *
- * The function removes properties previously associated to the device
- * firmware node with device_add_properties(). Memory allocated to the
- * properties will also be released.
- */
-void device_remove_properties(struct device *dev)
-{
-	struct fwnode_handle *fwnode = dev_fwnode(dev);
-
-	if (!fwnode)
-		return;
-
-	if (is_software_node(fwnode->secondary)) {
-		fwnode_remove_software_node(fwnode->secondary);
-		set_secondary_fwnode(dev, NULL);
-	}
-}
-EXPORT_SYMBOL_GPL(device_remove_properties);
-
-/**
- * device_add_properties - Add a collection of properties to a device object.
- * @dev: Device to add properties to.
- * @properties: Collection of properties to add.
- *
- * Associate a collection of device properties represented by @properties with
- * @dev. The function takes a copy of @properties.
- *
- * WARNING: The callers should not use this function if it is known that there
- * is no real firmware node associated with @dev! In that case the callers
- * should create a software node and assign it to @dev directly.
- */
-int device_add_properties(struct device *dev,
-			  const struct property_entry *properties)
-{
-	struct fwnode_handle *fwnode;
-
-	fwnode = fwnode_create_software_node(properties, NULL);
-	if (IS_ERR(fwnode))
-		return PTR_ERR(fwnode);
-
-	set_secondary_fwnode(dev, fwnode);
-	return 0;
-}
-EXPORT_SYMBOL_GPL(device_add_properties);
-
 /**
  * fwnode_get_name - Return the name of a node
  * @fwnode: The firmware node
diff --git a/include/linux/property.h b/include/linux/property.h
index 88fa726a76df7..16f736c698a2d 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -378,10 +378,6 @@ property_entries_dup(const struct property_entry *properties);
 
 void property_entries_free(const struct property_entry *properties);
 
-int device_add_properties(struct device *dev,
-			  const struct property_entry *properties);
-void device_remove_properties(struct device *dev);
-
 bool device_dma_supported(struct device *dev);
 
 enum dev_dma_attr device_get_dma_attr(struct device *dev);
-- 
2.33.0

Re: [PATCH v4 2/3] driver core: Don't call device_remove_properties() from device_del()

[Andy Shevchenko]

On Mon, Nov 15, 2021 at 03:10:00PM +0300, Heikki Krogerus wrote:
> All the drivers that relied on device_del() to call
> device_remove_properties() have now been converted to either
> use device_create_managed_software_node() instead of
> device_add_properties(), or to register the software node
> completely separately from the device.
> 
> This will make it finally possible to share and reuse the
> software nodes that hold the additional device properties.

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

> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
>  drivers/base/core.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index fd034d7424472..a40b6fb1ebb01 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -3582,7 +3582,6 @@ void device_del(struct device *dev)
>  	device_pm_remove(dev);
>  	driver_deferred_probe_del(dev);
>  	device_platform_notify_remove(dev);
> -	device_remove_properties(dev);
>  	device_links_purge(dev);
>  
>  	if (dev->bus)
> -- 
> 2.33.0
> 

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 0/3] device property: Remove device_add_properties()

[Rafael J. Wysocki]

On Mon, Nov 15, 2021 at 1:10 PM Heikki Krogerus
<heikki.krogerus@linux.intel.com> wrote:
>
> Hi,
>
> One more version. Hopefully the commit messages are now OK. No other
> changes since v3:
>
> https://lore.kernel.org/lkml/20211006112643.77684-1-heikki.krogerus@linux.intel.com/
>
>
> v3 cover letter:
>
> In this third version of this series, the second patch is now split in
> two. The device_remove_properties() call is first removed from
> device_del() in its own patch, and the
> device_add/remove_properties() API is removed separately in the last
> patch. I hope the commit messages are clear enough this time.
>
>
> v2 cover letter:
>
> This is the second version where I only modified the commit message of
> the first patch according to comments from Bjorn.
>
>
> Original cover letter:
>
> There is one user left for the API, so converting that to use software
> node API instead, and removing the function.
>
>
> thanks,
>
> Heikki Krogerus (3):
>   PCI: Convert to device_create_managed_software_node()
>   driver core: Don't call device_remove_properties() from device_del()
>   device property: Remove device_add_properties() API
>
>  drivers/base/core.c      |  1 -
>  drivers/base/property.c  | 48 ----------------------------------------
>  drivers/pci/quirks.c     |  2 +-
>  include/linux/property.h |  4 ----
>  4 files changed, 1 insertion(+), 54 deletions(-)

Has this been picked up already or am I expected to pick it up?

Re: [PATCH v4 0/3] device property: Remove device_add_properties()

[Heikki Krogerus]

On Wed, Nov 24, 2021 at 02:59:01PM +0100, Rafael J. Wysocki wrote:
> On Mon, Nov 15, 2021 at 1:10 PM Heikki Krogerus
> <heikki.krogerus@linux.intel.com> wrote:
> >
> > Hi,
> >
> > One more version. Hopefully the commit messages are now OK. No other
> > changes since v3:
> >
> > https://lore.kernel.org/lkml/20211006112643.77684-1-heikki.krogerus@linux.intel.com/
> >
> >
> > v3 cover letter:
> >
> > In this third version of this series, the second patch is now split in
> > two. The device_remove_properties() call is first removed from
> > device_del() in its own patch, and the
> > device_add/remove_properties() API is removed separately in the last
> > patch. I hope the commit messages are clear enough this time.
> >
> >
> > v2 cover letter:
> >
> > This is the second version where I only modified the commit message of
> > the first patch according to comments from Bjorn.
> >
> >
> > Original cover letter:
> >
> > There is one user left for the API, so converting that to use software
> > node API instead, and removing the function.
> >
> >
> > thanks,
> >
> > Heikki Krogerus (3):
> >   PCI: Convert to device_create_managed_software_node()
> >   driver core: Don't call device_remove_properties() from device_del()
> >   device property: Remove device_add_properties() API
> >
> >  drivers/base/core.c      |  1 -
> >  drivers/base/property.c  | 48 ----------------------------------------
> >  drivers/pci/quirks.c     |  2 +-
> >  include/linux/property.h |  4 ----
> >  4 files changed, 1 insertion(+), 54 deletions(-)
> 
> Has this been picked up already or am I expected to pick it up?

It hasn't been picked up by anybody, so if you can take these, that
would be great.

thanks,

-- 
heikki

Re: [PATCH v4 0/3] device property: Remove device_add_properties()

[Rafael J. Wysocki]

On Wed, Nov 24, 2021 at 3:15 PM Heikki Krogerus
<heikki.krogerus@linux.intel.com> wrote:
>
> On Wed, Nov 24, 2021 at 02:59:01PM +0100, Rafael J. Wysocki wrote:
> > On Mon, Nov 15, 2021 at 1:10 PM Heikki Krogerus
> > <heikki.krogerus@linux.intel.com> wrote:
> > >
> > > Hi,
> > >
> > > One more version. Hopefully the commit messages are now OK. No other
> > > changes since v3:
> > >
> > > https://lore.kernel.org/lkml/20211006112643.77684-1-heikki.krogerus@linux.intel.com/
> > >
> > >
> > > v3 cover letter:
> > >
> > > In this third version of this series, the second patch is now split in
> > > two. The device_remove_properties() call is first removed from
> > > device_del() in its own patch, and the
> > > device_add/remove_properties() API is removed separately in the last
> > > patch. I hope the commit messages are clear enough this time.
> > >
> > >
> > > v2 cover letter:
> > >
> > > This is the second version where I only modified the commit message of
> > > the first patch according to comments from Bjorn.
> > >
> > >
> > > Original cover letter:
> > >
> > > There is one user left for the API, so converting that to use software
> > > node API instead, and removing the function.
> > >
> > >
> > > thanks,
> > >
> > > Heikki Krogerus (3):
> > >   PCI: Convert to device_create_managed_software_node()
> > >   driver core: Don't call device_remove_properties() from device_del()
> > >   device property: Remove device_add_properties() API
> > >
> > >  drivers/base/core.c      |  1 -
> > >  drivers/base/property.c  | 48 ----------------------------------------
> > >  drivers/pci/quirks.c     |  2 +-
> > >  include/linux/property.h |  4 ----
> > >  4 files changed, 1 insertion(+), 54 deletions(-)
> >
> > Has this been picked up already or am I expected to pick it up?
>
> It hasn't been picked up by anybody, so if you can take these, that
> would be great.

OK, applied as 5.17 material now.

Greg, please let me know if you'd rather take these patches yourself
and I'll drop them in that case.

Thanks!

Re: [PATCH v4 0/3] device property: Remove device_add_properties()

[Greg Kroah-Hartman]

On Wed, Nov 24, 2021 at 03:30:53PM +0100, Rafael J. Wysocki wrote:
> On Wed, Nov 24, 2021 at 3:15 PM Heikki Krogerus
> <heikki.krogerus@linux.intel.com> wrote:
> >
> > On Wed, Nov 24, 2021 at 02:59:01PM +0100, Rafael J. Wysocki wrote:
> > > On Mon, Nov 15, 2021 at 1:10 PM Heikki Krogerus
> > > <heikki.krogerus@linux.intel.com> wrote:
> > > >
> > > > Hi,
> > > >
> > > > One more version. Hopefully the commit messages are now OK. No other
> > > > changes since v3:
> > > >
> > > > https://lore.kernel.org/lkml/20211006112643.77684-1-heikki.krogerus@linux.intel.com/
> > > >
> > > >
> > > > v3 cover letter:
> > > >
> > > > In this third version of this series, the second patch is now split in
> > > > two. The device_remove_properties() call is first removed from
> > > > device_del() in its own patch, and the
> > > > device_add/remove_properties() API is removed separately in the last
> > > > patch. I hope the commit messages are clear enough this time.
> > > >
> > > >
> > > > v2 cover letter:
> > > >
> > > > This is the second version where I only modified the commit message of
> > > > the first patch according to comments from Bjorn.
> > > >
> > > >
> > > > Original cover letter:
> > > >
> > > > There is one user left for the API, so converting that to use software
> > > > node API instead, and removing the function.
> > > >
> > > >
> > > > thanks,
> > > >
> > > > Heikki Krogerus (3):
> > > >   PCI: Convert to device_create_managed_software_node()
> > > >   driver core: Don't call device_remove_properties() from device_del()
> > > >   device property: Remove device_add_properties() API
> > > >
> > > >  drivers/base/core.c      |  1 -
> > > >  drivers/base/property.c  | 48 ----------------------------------------
> > > >  drivers/pci/quirks.c     |  2 +-
> > > >  include/linux/property.h |  4 ----
> > > >  4 files changed, 1 insertion(+), 54 deletions(-)
> > >
> > > Has this been picked up already or am I expected to pick it up?
> >
> > It hasn't been picked up by anybody, so if you can take these, that
> > would be great.
> 
> OK, applied as 5.17 material now.
> 
> Greg, please let me know if you'd rather take these patches yourself
> and I'll drop them in that case.

No problem, you can take them, thanks!

greg k-h