diff mbox series

[v4,19/19] fpga: dfl: destroy/recreate feature platform device on port release/assign

Message ID 20241025223714.394533-20-peter.colberg@intel.com (mailing list archive)
State New
Headers show
Series fpga: dfl: fix kernel warning on port release/assign for SRIOV | expand

Commit Message

Peter Colberg Oct. 25, 2024, 10:37 p.m. UTC 
From: Xu Yilun <yilun.xu@intel.com>

Now that the internal DFL APIs have been converted to consume DFL
enumeration info from a separate structure, dfl_feature_dev_data, which
lifetime is independent of the feature device, proceed to completely
destroy and recreate the feature platform device on port release and
assign, respectively. This resolves a longstanding issue in the use of
platform_device_add(), which states to "not call this routine more than
once for any device structure" and which used to print a kernel warning.

The function feature_dev_unregister() resets the device pointer in the
feature data to NULL to signal that the feature platform device has been
destroyed. This substitutes the previous device_is_registered() checks.

Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Russ Weight <russell.h.weight@intel.com>
Signed-off-by: Peter Colberg <peter.colberg@intel.com>
Reviewed-by: Matthew Gerlach <matthew.gerlach@linux.intel.com>
Reviewed-by: Basheer Ahmed Muddebihal <basheer.ahmed.muddebihal@linux.intel.com>
---
Changes since v3:
- New patch extracted from last patch of v3 series.
---
 drivers/fpga/dfl.c | 24 ++++++------------------
 1 file changed, 6 insertions(+), 18 deletions(-)

Comments

Xu Yilun Nov. 18, 2024, 3:09 p.m. UTC | #1 
On Fri, Oct 25, 2024 at 06:37:14PM -0400, Peter Colberg wrote:
> From: Xu Yilun <yilun.xu@intel.com>
> 
> Now that the internal DFL APIs have been converted to consume DFL
> enumeration info from a separate structure, dfl_feature_dev_data, which
> lifetime is independent of the feature device, proceed to completely
> destroy and recreate the feature platform device on port release and
> assign, respectively. This resolves a longstanding issue in the use of
> platform_device_add(), which states to "not call this routine more than
> once for any device structure" and which used to print a kernel warning.
> 
> The function feature_dev_unregister() resets the device pointer in the
> feature data to NULL to signal that the feature platform device has been
> destroyed. This substitutes the previous device_is_registered() checks.

Acked-by: Xu Yilun <yilun.xu@intel.com>

for this patch and all other patches that I didn't comment in this
series.

Thanks.

> 
> Signed-off-by: Xu Yilun <yilun.xu@intel.com>
> Signed-off-by: Russ Weight <russell.h.weight@intel.com>
> Signed-off-by: Peter Colberg <peter.colberg@intel.com>
> Reviewed-by: Matthew Gerlach <matthew.gerlach@linux.intel.com>
> Reviewed-by: Basheer Ahmed Muddebihal <basheer.ahmed.muddebihal@linux.intel.com>
Peter Colberg Nov. 19, 2024, 11:46 p.m. UTC | #2 
On Mon, 2024-11-18 at 23:09 +0800, Xu Yilun wrote:
> On Fri, Oct 25, 2024 at 06:37:14PM -0400, Peter Colberg wrote:
> > From: Xu Yilun <yilun.xu@intel.com>
> > 
> > Now that the internal DFL APIs have been converted to consume DFL
> > enumeration info from a separate structure, dfl_feature_dev_data, which
> > lifetime is independent of the feature device, proceed to completely
> > destroy and recreate the feature platform device on port release and
> > assign, respectively. This resolves a longstanding issue in the use of
> > platform_device_add(), which states to "not call this routine more than
> > once for any device structure" and which used to print a kernel warning.
> > 
> > The function feature_dev_unregister() resets the device pointer in the
> > feature data to NULL to signal that the feature platform device has been
> > destroyed. This substitutes the previous device_is_registered() checks.
> 
> Acked-by: Xu Yilun <yilun.xu@intel.com>
> 
> for this patch and all other patches that I didn't comment in this
> series.

Thank you for the thorough review and your guidance in converting this
change into a series of small patches that are easy to reason about.

Peter

> 
> Thanks.
> 
> > 
> > Signed-off-by: Xu Yilun <yilun.xu@intel.com>
> > Signed-off-by: Russ Weight <russell.h.weight@intel.com>
> > Signed-off-by: Peter Colberg <peter.colberg@intel.com>
> > Reviewed-by: Matthew Gerlach <matthew.gerlach@linux.intel.com>
> > Reviewed-by: Basheer Ahmed Muddebihal <basheer.ahmed.muddebihal@linux.intel.com>
diff mbox series

Patch

diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
index ed38e5ec84b6..3b3b05ca3ddb 100644
--- a/drivers/fpga/dfl.c
+++ b/drivers/fpga/dfl.c
@@ -1620,22 +1620,10 @@  EXPORT_SYMBOL_GPL(dfl_fpga_feature_devs_enumerate);
  */
 void dfl_fpga_feature_devs_remove(struct dfl_fpga_cdev *cdev)
 {
-	struct dfl_feature_dev_data *fdata, *ptmp;
-
 	mutex_lock(&cdev->lock);
 	if (cdev->fme_dev)
 		put_device(cdev->fme_dev);
 
-	list_for_each_entry_safe(fdata, ptmp, &cdev->port_dev_list, node) {
-		struct platform_device *port_dev = fdata->dev;
-
-		/* remove released ports */
-		if (!device_is_registered(&port_dev->dev)) {
-			platform_device_put(port_dev);
-		}
-
-		list_del(&fdata->node);
-	}
 	mutex_unlock(&cdev->lock);
 
 	remove_feature_devs(cdev);
@@ -1715,7 +1703,7 @@  int dfl_fpga_cdev_release_port(struct dfl_fpga_cdev *cdev, int port_id)
 	if (!fdata)
 		goto unlock_exit;
 
-	if (!device_is_registered(&fdata->dev->dev)) {
+	if (!fdata->dev) {
 		ret = -EBUSY;
 		goto unlock_exit;
 	}
@@ -1726,7 +1714,7 @@  int dfl_fpga_cdev_release_port(struct dfl_fpga_cdev *cdev, int port_id)
 	if (ret)
 		goto unlock_exit;
 
-	platform_device_del(fdata->dev);
+	feature_dev_unregister(fdata);
 	cdev->released_port_num++;
 unlock_exit:
 	mutex_unlock(&cdev->lock);
@@ -1756,12 +1744,12 @@  int dfl_fpga_cdev_assign_port(struct dfl_fpga_cdev *cdev, int port_id)
 	if (!fdata)
 		goto unlock_exit;
 
-	if (device_is_registered(&fdata->dev->dev)) {
+	if (fdata->dev) {
 		ret = -EBUSY;
 		goto unlock_exit;
 	}
 
-	ret = platform_device_add(fdata->dev);
+	ret = feature_dev_register(fdata);
 	if (ret)
 		goto unlock_exit;
 
@@ -1811,7 +1799,7 @@  void dfl_fpga_cdev_config_ports_pf(struct dfl_fpga_cdev *cdev)
 
 	mutex_lock(&cdev->lock);
 	list_for_each_entry(fdata, &cdev->port_dev_list, node) {
-		if (device_is_registered(&fdata->dev->dev))
+		if (fdata->dev)
 			continue;
 
 		config_port_pf_mode(cdev->fme_dev, fdata->id);
@@ -1848,7 +1836,7 @@  int dfl_fpga_cdev_config_ports_vf(struct dfl_fpga_cdev *cdev, int num_vfs)
 	}
 
 	list_for_each_entry(fdata, &cdev->port_dev_list, node) {
-		if (device_is_registered(&fdata->dev->dev))
+		if (fdata->dev)
 			continue;
 
 		config_port_vf_mode(cdev->fme_dev, fdata->id);