Hi,
There is one user left for the API, so converting that to use software
node API instead, and removing the function.
thanks,
Heikki Krogerus (2):
PCI: Use software node API with additional device properties
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
Using device_create_managed_software_node() to inject the
properties in quirk_huawei_pcie_sva() instead of with the
old device_add_properties() API.
Signed-off-by: Heikki Krogerus <[email protected]>
---
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 b6b4c803bdc94..fe5eedba47908 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
There are no more users for it.
Signed-off-by: Heikki Krogerus <[email protected]>
---
drivers/base/core.c | 1 -
drivers/base/property.c | 48 ----------------------------------------
include/linux/property.h | 4 ----
3 files changed, 53 deletions(-)
diff --git a/drivers/base/core.c b/drivers/base/core.c
index 7758223f040c8..7935ee642fa3f 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -3573,7 +3573,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)
diff --git a/drivers/base/property.c b/drivers/base/property.c
index 453918eb7390c..1f1eee37817e0 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -508,54 +508,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 357513a977e5d..daf0b5841286f 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -377,10 +377,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
On Wed, Sep 29, 2021 at 5:18 PM Heikki Krogerus
<[email protected]> wrote:
>
> Hi,
>
> There is one user left for the API, so converting that to use software
> node API instead, and removing the function.
All look good to me, thank you for removing the old API!
Reviewed-by: Andy Shevchenko <[email protected]>
> thanks,
>
> Heikki Krogerus (2):
> PCI: Use software node API with additional device properties
> 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(-)
--
With Best Regards,
Andy Shevchenko
[+cc Zhangfei, author of 8304a3a199ee ("PCI: Set dma-can-stall for
HiSilicon chips"), which added this]
On Wed, Sep 29, 2021 at 04:37:28PM +0300, Heikki Krogerus wrote:
> Using device_create_managed_software_node() to inject the
> properties in quirk_huawei_pcie_sva() instead of with the
> old device_add_properties() API.
>
> Signed-off-by: Heikki Krogerus <[email protected]>
This is fine with me, but please update the subject line and commit
log something like this:
PCI: Convert to device_create_managed_software_node()
In quirk_huawei_pcie_sva(), use device_create_managed_software_node()
instead of device_add_properties() to set the "dma-can-stall"
property.
This resolves a software node lifetime issue (see 151f6ff78cdf
("software node: Provide replacement for device_add_properties()"))
and paves the way for removing device_add_properties() completely.
Actually, 8304a3a199ee was merged during the v5.15 merge window, so if
this does in fact fix a lifetime issue, I can merge this before
v5.15-final.
I know *this* quirk applies to AMBA devices, and I assume they cannot
be removed, so there's no actual lifetime problem in this particular
case, but in general it looks like a problem for PCI devices.
> ---
> 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 b6b4c803bdc94..fe5eedba47908 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
>
On 2021/9/30 上午1:08, Bjorn Helgaas wrote:
> [+cc Zhangfei, author of 8304a3a199ee ("PCI: Set dma-can-stall for
> HiSilicon chips"), which added this]
>
> On Wed, Sep 29, 2021 at 04:37:28PM +0300, Heikki Krogerus wrote:
>> Using device_create_managed_software_node() to inject the
>> properties in quirk_huawei_pcie_sva() instead of with the
>> old device_add_properties() API.
>>
>> Signed-off-by: Heikki Krogerus <[email protected]>
> This is fine with me, but please update the subject line and commit
> log something like this:
>
> PCI: Convert to device_create_managed_software_node()
>
> In quirk_huawei_pcie_sva(), use device_create_managed_software_node()
> instead of device_add_properties() to set the "dma-can-stall"
> property.
>
> This resolves a software node lifetime issue (see 151f6ff78cdf
> ("software node: Provide replacement for device_add_properties()"))
> and paves the way for removing device_add_properties() completely.
>
> Actually, 8304a3a199ee was merged during the v5.15 merge window, so if
> this does in fact fix a lifetime issue, I can merge this before
> v5.15-final.
>
> I know *this* quirk applies to AMBA devices, and I assume they cannot
> be removed, so there's no actual lifetime problem in this particular
> case, but in general it looks like a problem for PCI devices.
Thanks Bjorn
This patch also works, though the quirk is for platform devices and not
removed.
Acked-by: Zhangfei Gao <[email protected]>
>
>> ---
>> 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 b6b4c803bdc94..fe5eedba47908 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
>>
Hi guys,
On Thu, Sep 30, 2021 at 10:33:27AM +0800, Zhangfei Gao wrote:
> On 2021/9/30 上午1:08, Bjorn Helgaas wrote:
> > [+cc Zhangfei, author of 8304a3a199ee ("PCI: Set dma-can-stall for
> > HiSilicon chips"), which added this]
> >
> > On Wed, Sep 29, 2021 at 04:37:28PM +0300, Heikki Krogerus wrote:
> > > Using device_create_managed_software_node() to inject the
> > > properties in quirk_huawei_pcie_sva() instead of with the
> > > old device_add_properties() API.
> > >
> > > Signed-off-by: Heikki Krogerus <[email protected]>
> > This is fine with me, but please update the subject line and commit
> > log something like this:
> >
> > PCI: Convert to device_create_managed_software_node()
> >
> > In quirk_huawei_pcie_sva(), use device_create_managed_software_node()
> > instead of device_add_properties() to set the "dma-can-stall"
> > property.
> >
> > This resolves a software node lifetime issue (see 151f6ff78cdf
> > ("software node: Provide replacement for device_add_properties()"))
> > and paves the way for removing device_add_properties() completely.
> >
> > Actually, 8304a3a199ee was merged during the v5.15 merge window, so if
> > this does in fact fix a lifetime issue, I can merge this before
> > v5.15-final.
It does not fix lifetime issues. This is because device_del() called
device_remove_properties() unconditionally with every device.
There should be no functional impact.
> > I know *this* quirk applies to AMBA devices, and I assume they cannot
> > be removed, so there's no actual lifetime problem in this particular
> > case, but in general it looks like a problem for PCI devices.
> Thanks Bjorn
> This patch also works, though the quirk is for platform devices and not
> removed.
If the device is really never removed, then we could also constify the
node and the properties in it. Then the patch would look like this:
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index b6b4c803bdc94..3dc7a1c62bf24 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -1833,13 +1833,17 @@ DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_VENDOR_ID_HUAWEI, 0x1610, PCI_CLASS_BRIDGE_PCI
* even when a "PCI" device turns out to be a regular old SoC device
* dressed up as a RCiEP and normal rules don't apply.
*/
+static const struct property_entry huawei_pcie_sva_props[] = {
+ PROPERTY_ENTRY_BOOL("dma-can-stall"),
+ { }
+};
+
+static const struct software_node huawei_pcie_sva_swnode = {
+ .properties = huawei_pcie_sva_props,
+};
+
static void quirk_huawei_pcie_sva(struct pci_dev *pdev)
{
- struct property_entry properties[] = {
- PROPERTY_ENTRY_BOOL("dma-can-stall"),
- {},
- };
-
if (pdev->revision != 0x21 && pdev->revision != 0x30)
return;
@@ -1850,7 +1854,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_add_software_node(&pdev->dev, &huawei_pcie_sva_swnode))
pci_warn(pdev, "could not add stall property");
}
DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_HUAWEI, 0xa250, quirk_huawei_pcie_sva);
Let me know if you prefer it that way.
thanks,
--
heikki
On Thu, Sep 30, 2021 at 1:06 PM Heikki Krogerus
<[email protected]> wrote:
> On Thu, Sep 30, 2021 at 10:33:27AM +0800, Zhangfei Gao wrote:
...
> If the device is really never removed, then we could also constify the
> node and the properties in it. Then the patch would look like this:
I'm not sure the user can't force removal of the device (via PCI
rescan, for example,, or via unbind/bind cycle). I guess this way
should be really taken carefully.
--
With Best Regards,
Andy Shevchenko
On Thu, Sep 30, 2021 at 12:20 PM Andy Shevchenko
<[email protected]> wrote:
>
> On Thu, Sep 30, 2021 at 1:06 PM Heikki Krogerus
> <[email protected]> wrote:
> > On Thu, Sep 30, 2021 at 10:33:27AM +0800, Zhangfei Gao wrote:
>
> ...
>
> > If the device is really never removed, then we could also constify the
> > node and the properties in it. Then the patch would look like this:
>
> I'm not sure the user can't force removal of the device (via PCI
> rescan, for example,, or via unbind/bind cycle).
The sysfs unbind doesn't remove the device, though, AFAICS. It just
unbinds the driver from it, if any.
> I guess this way should be really taken carefully.
But I agree.
On Thu, Sep 30, 2021 at 12:37:27PM +0200, Rafael J. Wysocki wrote:
> On Thu, Sep 30, 2021 at 12:20 PM Andy Shevchenko
> <[email protected]> wrote:
> >
> > On Thu, Sep 30, 2021 at 1:06 PM Heikki Krogerus
> > <[email protected]> wrote:
> > > On Thu, Sep 30, 2021 at 10:33:27AM +0800, Zhangfei Gao wrote:
> >
> > ...
> >
> > > If the device is really never removed, then we could also constify the
> > > node and the properties in it. Then the patch would look like this:
> >
> > I'm not sure the user can't force removal of the device (via PCI
> > rescan, for example,, or via unbind/bind cycle).
>
> The sysfs unbind doesn't remove the device, though, AFAICS. It just
> unbinds the driver from it, if any.
>
> > I guess this way should be really taken carefully.
>
> But I agree.
OK. Makes sense.
Thanks guys,
--
heikki