diff mbox series

[v4,14/19] fpga: dfl: store platform device id in feature device data

Message ID 20241025223714.394533-15-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 
Delay the feature device id allocation from build_info_create_dev() to
binfo_create_feature_dev_data() and store the id in the feature device
data before copying it to the device. This will allow reusing the same
id in a subsequent commit which completely destroys and recreates the
feature device when releasing and reassigning the corresponding port.

Instead of manually freeing the id when no longer needed, use a
device-managed resource with a custom action to automatically free
the id right before the feature device data is freed. The id registry
is guaranteed to be allocated when dfl_id_free_action() is invoked,
since the DFL PCIe device and its device-managed resources will be
destroyed before dfl_ids_destroy() is called in dfl_fpga_exit().

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 | 43 ++++++++++++++++++++++---------------------
 drivers/fpga/dfl.h |  2 ++
 2 files changed, 24 insertions(+), 21 deletions(-)

Comments

Xu Yilun Nov. 18, 2024, 2:34 p.m. UTC | #1 
> @@ -1658,7 +1660,6 @@ void dfl_fpga_feature_devs_remove(struct dfl_fpga_cdev *cdev)
>  
>  		/* remove released ports */
>  		if (!device_is_registered(&port_dev->dev)) {
> -			dfl_id_free(fdata->type, port_dev->id);

Also remove the braces.

Thanks,
Yilun

>  			platform_device_put(port_dev);
>  		}
Peter Colberg Nov. 19, 2024, 11:34 p.m. UTC | #2 
On Mon, 2024-11-18 at 22:34 +0800, Xu Yilun wrote:
> > @@ -1658,7 +1660,6 @@ void dfl_fpga_feature_devs_remove(struct dfl_fpga_cdev *cdev)
> >  
> > 
> > 
> > 
> >  		/* remove released ports */
> >  		if (!device_is_registered(&port_dev->dev)) {
> > -			dfl_id_free(fdata->type, port_dev->id);
> 
> Also remove the braces.

Thanks, this has been resolved in [PATCH v5 14/18].

Peter

> 
> Thanks,
> Yilun
> 
> >  			platform_device_put(port_dev);
> >  		}
diff mbox series

Patch

diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
index 560d35857405..758673b0290a 100644
--- a/drivers/fpga/dfl.c
+++ b/drivers/fpga/dfl.c
@@ -740,6 +740,13 @@  static void dfl_fpga_cdev_add_port_data(struct dfl_fpga_cdev *cdev,
 	mutex_unlock(&cdev->lock);
 }
 
+static void dfl_id_free_action(void *arg)
+{
+	struct dfl_feature_dev_data *fdata = arg;
+
+	dfl_id_free(fdata->type, fdata->pdev_id);
+}
+
 static struct dfl_feature_dev_data *
 binfo_create_feature_dev_data(struct build_feature_devs_info *binfo)
 {
@@ -747,7 +754,7 @@  binfo_create_feature_dev_data(struct build_feature_devs_info *binfo)
 	enum dfl_id_type type = binfo->type;
 	struct dfl_feature_info *finfo, *p;
 	struct dfl_feature_dev_data *fdata;
-	int index = 0, res_idx = 0;
+	int ret, index = 0, res_idx = 0;
 
 	if (WARN_ON_ONCE(type >= DFL_ID_MAX))
 		return ERR_PTR(-EINVAL);
@@ -768,6 +775,17 @@  binfo_create_feature_dev_data(struct build_feature_devs_info *binfo)
 
 	fdata->dev = fdev;
 	fdata->type = type;
+
+	fdata->pdev_id = dfl_id_alloc(type, binfo->dev);
+	if (fdata->pdev_id < 0)
+		return ERR_PTR(fdata->pdev_id);
+
+	ret = devm_add_action_or_reset(binfo->dev, dfl_id_free_action, fdata);
+	if (ret)
+		return ERR_PTR(ret);
+
+	fdev->id = fdata->pdev_id;
+
 	fdata->pdev_name = dfl_devs[type].name;
 	fdata->num = binfo->feature_num;
 	fdata->dfl_cdev = binfo->cdev;
@@ -866,10 +884,6 @@  build_info_create_dev(struct build_feature_devs_info *binfo)
 
 	INIT_LIST_HEAD(&binfo->sub_features);
 
-	fdev->id = dfl_id_alloc(type, &fdev->dev);
-	if (fdev->id < 0)
-		return fdev->id;
-
 	return 0;
 }
 
@@ -947,17 +961,9 @@  static void build_info_free(struct build_feature_devs_info *binfo)
 {
 	struct dfl_feature_info *finfo, *p;
 
-	/*
-	 * it is a valid id, free it. See comments in
-	 * build_info_create_dev()
-	 */
-	if (binfo->feature_dev && binfo->feature_dev->id >= 0) {
-		dfl_id_free(binfo->type, binfo->feature_dev->id);
-
-		list_for_each_entry_safe(finfo, p, &binfo->sub_features, node) {
-			list_del(&finfo->node);
-			kfree(finfo);
-		}
+	list_for_each_entry_safe(finfo, p, &binfo->sub_features, node) {
+		list_del(&finfo->node);
+		kfree(finfo);
 	}
 
 	platform_device_put(binfo->feature_dev);
@@ -1548,13 +1554,9 @@  EXPORT_SYMBOL_GPL(dfl_fpga_enum_info_add_irq);
 static int remove_feature_dev(struct device *dev, void *data)
 {
 	struct dfl_feature_dev_data *fdata = to_dfl_feature_dev_data(dev);
-	struct platform_device *pdev = to_platform_device(dev);
-	int id = pdev->id;
 
 	feature_dev_unregister(fdata);
 
-	dfl_id_free(fdata->type, id);
-
 	return 0;
 }
 
@@ -1658,7 +1660,6 @@  void dfl_fpga_feature_devs_remove(struct dfl_fpga_cdev *cdev)
 
 		/* remove released ports */
 		if (!device_is_registered(&port_dev->dev)) {
-			dfl_id_free(fdata->type, port_dev->id);
 			platform_device_put(port_dev);
 		}
 
diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h
index cbff5d543c44..2e38c42b3920 100644
--- a/drivers/fpga/dfl.h
+++ b/drivers/fpga/dfl.h
@@ -309,6 +309,7 @@  struct dfl_feature {
  * @lock: mutex to protect feature dev data.
  * @dev: ptr to the feature's platform device linked with this structure.
  * @type: type of DFL FIU for the feature dev. See enum dfl_id_type.
+ * @pdev_id: platform device id for the feature dev.
  * @pdev_name: platform device name for the feature dev.
  * @dfl_cdev: ptr to container device.
  * @id: id used for the feature device.
@@ -326,6 +327,7 @@  struct dfl_feature_dev_data {
 	struct mutex lock;
 	struct platform_device *dev;
 	enum dfl_id_type type;
+	int pdev_id;
 	const char *pdev_name;
 	struct dfl_fpga_cdev *dfl_cdev;
 	int id;