| Message ID | 20241025223714.394533-17-peter.colberg@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | fpga: dfl: fix kernel warning on port release/assign for SRIOV | expand |
On Fri, Oct 25, 2024 at 06:37:11PM -0400, Peter Colberg wrote: > Delay calling platform_device_alloc() from build_info_create_dev() to > feature_dev_register(), now that the feature device data contains all > necessary data to create the feature device. This completes the new > function feature_dev_register(), which will be reused in a subsequent > commit to fully recreate the feature device when assigning a port. > > 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 | 59 +++++++++++++++++----------------------------- > 1 file changed, 22 insertions(+), 37 deletions(-) > > diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c > index a9ec37278b2d..d9cef150ed0d 100644 > --- a/drivers/fpga/dfl.c > +++ b/drivers/fpga/dfl.c > @@ -681,7 +681,6 @@ EXPORT_SYMBOL_GPL(dfl_fpga_dev_ops_unregister); > * @nr_irqs: number of irqs for all feature devices. > * @irq_table: Linux IRQ numbers for all irqs, indexed by local irq index of > * this device. > - * @feature_dev: current feature device. > * @type: the current FIU type. > * @ioaddr: header register region address of current FIU in enumeration. > * @start: register resource start of current FIU. > @@ -695,7 +694,6 @@ struct build_feature_devs_info { > unsigned int nr_irqs; > int *irq_table; > > - struct platform_device *feature_dev; > enum dfl_id_type type; > void __iomem *ioaddr; > resource_size_t start; > @@ -750,7 +748,6 @@ static void dfl_id_free_action(void *arg) > static struct dfl_feature_dev_data * > binfo_create_feature_dev_data(struct build_feature_devs_info *binfo) > { > - struct platform_device *fdev = binfo->feature_dev; > enum dfl_id_type type = binfo->type; > struct dfl_feature_info *finfo, *p; > struct dfl_feature_dev_data *fdata; > @@ -773,7 +770,6 @@ binfo_create_feature_dev_data(struct build_feature_devs_info *binfo) > if (!fdata->resources) > return ERR_PTR(-ENOMEM); > > - fdata->dev = fdev; > fdata->type = type; > > fdata->pdev_id = dfl_id_alloc(type, binfo->dev); > @@ -784,8 +780,6 @@ binfo_create_feature_dev_data(struct build_feature_devs_info *binfo) > 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; > @@ -809,7 +803,6 @@ binfo_create_feature_dev_data(struct build_feature_devs_info *binfo) > unsigned int i; > > /* save resource information for each feature */ > - feature->dev = fdev; > feature->id = finfo->fid; > feature->revision = finfo->revision; > feature->dfh_version = finfo->dfh_version; > @@ -868,18 +861,6 @@ binfo_create_feature_dev_data(struct build_feature_devs_info *binfo) > static int > build_info_create_dev(struct build_feature_devs_info *binfo) > { > - enum dfl_id_type type = binfo->type; > - struct platform_device *fdev; > - > - /* > - * we use -ENODEV as the initialization indicator which indicates > - * whether the id need to be reclaimed > - */ > - fdev = platform_device_alloc(dfl_devs[type].name, -ENODEV); > - if (!fdev) > - return -ENOMEM; > - > - binfo->feature_dev = fdev; > binfo->feature_num = 0; > > INIT_LIST_HEAD(&binfo->sub_features); > @@ -895,27 +876,43 @@ build_info_create_dev(struct build_feature_devs_info *binfo) > static int feature_dev_register(struct dfl_feature_dev_data *fdata) > { > struct dfl_feature_platform_data pdata = {}; > - struct platform_device *fdev = fdata->dev; > + struct platform_device *fdev; > + struct dfl_feature *feature; > int ret; > > + fdev = platform_device_alloc(fdata->pdev_name, fdata->pdev_id); > + if (!fdev) > + return -ENOMEM; > + > + fdata->dev = fdev; > + > fdev->dev.parent = &fdata->dfl_cdev->region->dev; > fdev->dev.devt = dfl_get_devt(dfl_devs[fdata->type].devt_type, fdev->id); > > + dfl_fpga_dev_for_each_feature(fdata, feature) > + feature->dev = fdev; > + > ret = platform_device_add_resources(fdev, fdata->resources, > fdata->resource_num); > if (ret) > - return ret; > + goto err_put_dev; > > pdata.fdata = fdata; > ret = platform_device_add_data(fdev, &pdata, sizeof(pdata)); > if (ret) > - return ret; > + goto err_put_dev; > > ret = platform_device_add(fdev); > if (ret) > - return ret; > + goto err_put_dev; > > return 0; > + > +err_put_dev: > + platform_device_put(fdev); > + fdata->dev = NULL; Do we also need to clean up all the feature->dev? Thanks, Yilun > + > + return ret; > } > > static void feature_dev_unregister(struct dfl_feature_dev_data *fdata) > @@ -940,16 +937,7 @@ static int build_info_commit_dev(struct build_feature_devs_info *binfo) > if (binfo->type == PORT_ID) > dfl_fpga_cdev_add_port_data(binfo->cdev, fdata); > else > - binfo->cdev->fme_dev = get_device(&binfo->feature_dev->dev); > - > - /* > - * reset it to avoid build_info_free() freeing their resource. > - * > - * The resource of successfully registered feature devices > - * will be freed by platform_device_unregister(). See the > - * comments in build_info_create_dev(). > - */ > - binfo->feature_dev = NULL; > + binfo->cdev->fme_dev = get_device(&fdata->dev->dev); > > /* reset the binfo for next FIU */ > binfo->type = DFL_ID_MAX; > @@ -966,8 +954,6 @@ static void build_info_free(struct build_feature_devs_info *binfo) > kfree(finfo); > } > > - platform_device_put(binfo->feature_dev); > - > devm_kfree(binfo->dev, binfo); > } > > @@ -1262,8 +1248,7 @@ static int parse_feature_afu(struct build_feature_devs_info *binfo, > case PORT_ID: > return parse_feature_port_afu(binfo, ofst); > default: > - dev_info(binfo->dev, "AFU belonging to FIU %s is not supported yet.\n", > - binfo->feature_dev->name); > + dev_info(binfo->dev, "AFU belonging to FIU is not supported yet.\n"); > } > > return 0; > -- > 2.47.0 > >
On Mon, 2024-11-18 at 22:51 +0800, Xu Yilun wrote: > On Fri, Oct 25, 2024 at 06:37:11PM -0400, Peter Colberg wrote: > > Delay calling platform_device_alloc() from build_info_create_dev() to > > feature_dev_register(), now that the feature device data contains all > > necessary data to create the feature device. This completes the new > > function feature_dev_register(), which will be reused in a subsequent > > commit to fully recreate the feature device when assigning a port. > > > > 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 | 59 +++++++++++++++++----------------------------- > > 1 file changed, 22 insertions(+), 37 deletions(-) > > > > diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c > > index a9ec37278b2d..d9cef150ed0d 100644 > > --- a/drivers/fpga/dfl.c > > +++ b/drivers/fpga/dfl.c > > @@ -681,7 +681,6 @@ EXPORT_SYMBOL_GPL(dfl_fpga_dev_ops_unregister); > > * @nr_irqs: number of irqs for all feature devices. > > * @irq_table: Linux IRQ numbers for all irqs, indexed by local irq index of > > * this device. > > - * @feature_dev: current feature device. > > * @type: the current FIU type. > > * @ioaddr: header register region address of current FIU in enumeration. > > * @start: register resource start of current FIU. > > @@ -695,7 +694,6 @@ struct build_feature_devs_info { > > unsigned int nr_irqs; > > int *irq_table; > > > > - struct platform_device *feature_dev; > > enum dfl_id_type type; > > void __iomem *ioaddr; > > resource_size_t start; > > @@ -750,7 +748,6 @@ static void dfl_id_free_action(void *arg) > > static struct dfl_feature_dev_data * > > binfo_create_feature_dev_data(struct build_feature_devs_info *binfo) > > { > > - struct platform_device *fdev = binfo->feature_dev; > > enum dfl_id_type type = binfo->type; > > struct dfl_feature_info *finfo, *p; > > struct dfl_feature_dev_data *fdata; > > @@ -773,7 +770,6 @@ binfo_create_feature_dev_data(struct build_feature_devs_info *binfo) > > if (!fdata->resources) > > return ERR_PTR(-ENOMEM); > > > > - fdata->dev = fdev; > > fdata->type = type; > > > > fdata->pdev_id = dfl_id_alloc(type, binfo->dev); > > @@ -784,8 +780,6 @@ binfo_create_feature_dev_data(struct build_feature_devs_info *binfo) > > 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; > > @@ -809,7 +803,6 @@ binfo_create_feature_dev_data(struct build_feature_devs_info *binfo) > > unsigned int i; > > > > /* save resource information for each feature */ > > - feature->dev = fdev; > > feature->id = finfo->fid; > > feature->revision = finfo->revision; > > feature->dfh_version = finfo->dfh_version; > > @@ -868,18 +861,6 @@ binfo_create_feature_dev_data(struct build_feature_devs_info *binfo) > > static int > > build_info_create_dev(struct build_feature_devs_info *binfo) > > { > > - enum dfl_id_type type = binfo->type; > > - struct platform_device *fdev; > > - > > - /* > > - * we use -ENODEV as the initialization indicator which indicates > > - * whether the id need to be reclaimed > > - */ > > - fdev = platform_device_alloc(dfl_devs[type].name, -ENODEV); > > - if (!fdev) > > - return -ENOMEM; > > - > > - binfo->feature_dev = fdev; > > binfo->feature_num = 0; > > > > INIT_LIST_HEAD(&binfo->sub_features); > > @@ -895,27 +876,43 @@ build_info_create_dev(struct build_feature_devs_info *binfo) > > static int feature_dev_register(struct dfl_feature_dev_data *fdata) > > { > > struct dfl_feature_platform_data pdata = {}; > > - struct platform_device *fdev = fdata->dev; > > + struct platform_device *fdev; > > + struct dfl_feature *feature; > > int ret; > > > > + fdev = platform_device_alloc(fdata->pdev_name, fdata->pdev_id); > > + if (!fdev) > > + return -ENOMEM; > > + > > + fdata->dev = fdev; > > + > > fdev->dev.parent = &fdata->dfl_cdev->region->dev; > > fdev->dev.devt = dfl_get_devt(dfl_devs[fdata->type].devt_type, fdev->id); > > > > + dfl_fpga_dev_for_each_feature(fdata, feature) > > + feature->dev = fdev; > > + > > ret = platform_device_add_resources(fdev, fdata->resources, > > fdata->resource_num); > > if (ret) > > - return ret; > > + goto err_put_dev; > > > > pdata.fdata = fdata; > > ret = platform_device_add_data(fdev, &pdata, sizeof(pdata)); > > if (ret) > > - return ret; > > + goto err_put_dev; > > > > ret = platform_device_add(fdev); > > if (ret) > > - return ret; > > + goto err_put_dev; > > > > return 0; > > + > > +err_put_dev: > > + platform_device_put(fdev); > > + fdata->dev = NULL; > > Do we also need to clean up all the feature->dev? Yes, thank you for the suggestion. It is not needed for functional correctness since feature->dev is exclusively used when non-NULL, but it seems a good idea for consistency. Thanks, Peter > > Thanks, > Yilun > > > + > > + return ret; > > } > > > > static void feature_dev_unregister(struct dfl_feature_dev_data *fdata) > > @@ -940,16 +937,7 @@ static int build_info_commit_dev(struct build_feature_devs_info *binfo) > > if (binfo->type == PORT_ID) > > dfl_fpga_cdev_add_port_data(binfo->cdev, fdata); > > else > > - binfo->cdev->fme_dev = get_device(&binfo->feature_dev->dev); > > - > > - /* > > - * reset it to avoid build_info_free() freeing their resource. > > - * > > - * The resource of successfully registered feature devices > > - * will be freed by platform_device_unregister(). See the > > - * comments in build_info_create_dev(). > > - */ > > - binfo->feature_dev = NULL; > > + binfo->cdev->fme_dev = get_device(&fdata->dev->dev); > > > > /* reset the binfo for next FIU */ > > binfo->type = DFL_ID_MAX; > > @@ -966,8 +954,6 @@ static void build_info_free(struct build_feature_devs_info *binfo) > > kfree(finfo); > > } > > > > - platform_device_put(binfo->feature_dev); > > - > > devm_kfree(binfo->dev, binfo); > > } > > > > @@ -1262,8 +1248,7 @@ static int parse_feature_afu(struct build_feature_devs_info *binfo, > > case PORT_ID: > > return parse_feature_port_afu(binfo, ofst); > > default: > > - dev_info(binfo->dev, "AFU belonging to FIU %s is not supported yet.\n", > > - binfo->feature_dev->name); > > + dev_info(binfo->dev, "AFU belonging to FIU is not supported yet.\n"); > > } > > > > return 0; > > -- > > 2.47.0 > > > >
On Mon, 2024-11-18 at 23:04 +0000, Colberg, Peter wrote: > On Mon, 2024-11-18 at 22:51 +0800, Xu Yilun wrote: > > On Fri, Oct 25, 2024 at 06:37:11PM -0400, Peter Colberg wrote: > > > Delay calling platform_device_alloc() from build_info_create_dev() to > > > feature_dev_register(), now that the feature device data contains all > > > necessary data to create the feature device. This completes the new > > > function feature_dev_register(), which will be reused in a subsequent > > > commit to fully recreate the feature device when assigning a port. > > > > > > 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 | 59 +++++++++++++++++----------------------------- > > > 1 file changed, 22 insertions(+), 37 deletions(-) > > > > > > diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c > > > index a9ec37278b2d..d9cef150ed0d 100644 > > > --- a/drivers/fpga/dfl.c > > > +++ b/drivers/fpga/dfl.c > > > @@ -681,7 +681,6 @@ EXPORT_SYMBOL_GPL(dfl_fpga_dev_ops_unregister); > > > * @nr_irqs: number of irqs for all feature devices. > > > * @irq_table: Linux IRQ numbers for all irqs, indexed by local irq index of > > > * this device. > > > - * @feature_dev: current feature device. > > > * @type: the current FIU type. > > > * @ioaddr: header register region address of current FIU in enumeration. > > > * @start: register resource start of current FIU. > > > @@ -695,7 +694,6 @@ struct build_feature_devs_info { > > > unsigned int nr_irqs; > > > int *irq_table; > > > > > > - struct platform_device *feature_dev; > > > enum dfl_id_type type; > > > void __iomem *ioaddr; > > > resource_size_t start; > > > @@ -750,7 +748,6 @@ static void dfl_id_free_action(void *arg) > > > static struct dfl_feature_dev_data * > > > binfo_create_feature_dev_data(struct build_feature_devs_info *binfo) > > > { > > > - struct platform_device *fdev = binfo->feature_dev; > > > enum dfl_id_type type = binfo->type; > > > struct dfl_feature_info *finfo, *p; > > > struct dfl_feature_dev_data *fdata; > > > @@ -773,7 +770,6 @@ binfo_create_feature_dev_data(struct build_feature_devs_info *binfo) > > > if (!fdata->resources) > > > return ERR_PTR(-ENOMEM); > > > > > > - fdata->dev = fdev; > > > fdata->type = type; > > > > > > fdata->pdev_id = dfl_id_alloc(type, binfo->dev); > > > @@ -784,8 +780,6 @@ binfo_create_feature_dev_data(struct build_feature_devs_info *binfo) > > > 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; > > > @@ -809,7 +803,6 @@ binfo_create_feature_dev_data(struct build_feature_devs_info *binfo) > > > unsigned int i; > > > > > > /* save resource information for each feature */ > > > - feature->dev = fdev; > > > feature->id = finfo->fid; > > > feature->revision = finfo->revision; > > > feature->dfh_version = finfo->dfh_version; > > > @@ -868,18 +861,6 @@ binfo_create_feature_dev_data(struct build_feature_devs_info *binfo) > > > static int > > > build_info_create_dev(struct build_feature_devs_info *binfo) > > > { > > > - enum dfl_id_type type = binfo->type; > > > - struct platform_device *fdev; > > > - > > > - /* > > > - * we use -ENODEV as the initialization indicator which indicates > > > - * whether the id need to be reclaimed > > > - */ > > > - fdev = platform_device_alloc(dfl_devs[type].name, -ENODEV); > > > - if (!fdev) > > > - return -ENOMEM; > > > - > > > - binfo->feature_dev = fdev; > > > binfo->feature_num = 0; > > > > > > INIT_LIST_HEAD(&binfo->sub_features); > > > @@ -895,27 +876,43 @@ build_info_create_dev(struct build_feature_devs_info *binfo) > > > static int feature_dev_register(struct dfl_feature_dev_data *fdata) > > > { > > > struct dfl_feature_platform_data pdata = {}; > > > - struct platform_device *fdev = fdata->dev; > > > + struct platform_device *fdev; > > > + struct dfl_feature *feature; > > > int ret; > > > > > > + fdev = platform_device_alloc(fdata->pdev_name, fdata->pdev_id); > > > + if (!fdev) > > > + return -ENOMEM; > > > + > > > + fdata->dev = fdev; > > > + > > > fdev->dev.parent = &fdata->dfl_cdev->region->dev; > > > fdev->dev.devt = dfl_get_devt(dfl_devs[fdata->type].devt_type, fdev->id); > > > > > > + dfl_fpga_dev_for_each_feature(fdata, feature) > > > + feature->dev = fdev; > > > + > > > ret = platform_device_add_resources(fdev, fdata->resources, > > > fdata->resource_num); > > > if (ret) > > > - return ret; > > > + goto err_put_dev; > > > > > > pdata.fdata = fdata; > > > ret = platform_device_add_data(fdev, &pdata, sizeof(pdata)); > > > if (ret) > > > - return ret; > > > + goto err_put_dev; > > > > > > ret = platform_device_add(fdev); > > > if (ret) > > > - return ret; > > > + goto err_put_dev; > > > > > > return 0; > > > + > > > +err_put_dev: > > > + platform_device_put(fdev); > > > + fdata->dev = NULL; > > > > Do we also need to clean up all the feature->dev? > > Yes, thank you for the suggestion. It is not needed for functional > correctness since feature->dev is exclusively used when non-NULL, but > it seems a good idea for consistency. This has been implemented in [PATCH v5 15/18]. Thanks, Peter > > Thanks, > Peter > > > > > Thanks, > > Yilun > > > > > + > > > + return ret; > > > } > > > > > > static void feature_dev_unregister(struct dfl_feature_dev_data *fdata) > > > @@ -940,16 +937,7 @@ static int build_info_commit_dev(struct build_feature_devs_info *binfo) > > > if (binfo->type == PORT_ID) > > > dfl_fpga_cdev_add_port_data(binfo->cdev, fdata); > > > else > > > - binfo->cdev->fme_dev = get_device(&binfo->feature_dev->dev); > > > - > > > - /* > > > - * reset it to avoid build_info_free() freeing their resource. > > > - * > > > - * The resource of successfully registered feature devices > > > - * will be freed by platform_device_unregister(). See the > > > - * comments in build_info_create_dev(). > > > - */ > > > - binfo->feature_dev = NULL; > > > + binfo->cdev->fme_dev = get_device(&fdata->dev->dev); > > > > > > /* reset the binfo for next FIU */ > > > binfo->type = DFL_ID_MAX; > > > @@ -966,8 +954,6 @@ static void build_info_free(struct build_feature_devs_info *binfo) > > > kfree(finfo); > > > } > > > > > > - platform_device_put(binfo->feature_dev); > > > - > > > devm_kfree(binfo->dev, binfo); > > > } > > > > > > @@ -1262,8 +1248,7 @@ static int parse_feature_afu(struct build_feature_devs_info *binfo, > > > case PORT_ID: > > > return parse_feature_port_afu(binfo, ofst); > > > default: > > > - dev_info(binfo->dev, "AFU belonging to FIU %s is not supported yet.\n", > > > - binfo->feature_dev->name); > > > + dev_info(binfo->dev, "AFU belonging to FIU is not supported yet.\n"); > > > } > > > > > > return 0; > > > -- > > > 2.47.0 > > > > > > >
diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c index a9ec37278b2d..d9cef150ed0d 100644 --- a/drivers/fpga/dfl.c +++ b/drivers/fpga/dfl.c @@ -681,7 +681,6 @@ EXPORT_SYMBOL_GPL(dfl_fpga_dev_ops_unregister); * @nr_irqs: number of irqs for all feature devices. * @irq_table: Linux IRQ numbers for all irqs, indexed by local irq index of * this device. - * @feature_dev: current feature device. * @type: the current FIU type. * @ioaddr: header register region address of current FIU in enumeration. * @start: register resource start of current FIU. @@ -695,7 +694,6 @@ struct build_feature_devs_info { unsigned int nr_irqs; int *irq_table; - struct platform_device *feature_dev; enum dfl_id_type type; void __iomem *ioaddr; resource_size_t start; @@ -750,7 +748,6 @@ static void dfl_id_free_action(void *arg) static struct dfl_feature_dev_data * binfo_create_feature_dev_data(struct build_feature_devs_info *binfo) { - struct platform_device *fdev = binfo->feature_dev; enum dfl_id_type type = binfo->type; struct dfl_feature_info *finfo, *p; struct dfl_feature_dev_data *fdata; @@ -773,7 +770,6 @@ binfo_create_feature_dev_data(struct build_feature_devs_info *binfo) if (!fdata->resources) return ERR_PTR(-ENOMEM); - fdata->dev = fdev; fdata->type = type; fdata->pdev_id = dfl_id_alloc(type, binfo->dev); @@ -784,8 +780,6 @@ binfo_create_feature_dev_data(struct build_feature_devs_info *binfo) 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; @@ -809,7 +803,6 @@ binfo_create_feature_dev_data(struct build_feature_devs_info *binfo) unsigned int i; /* save resource information for each feature */ - feature->dev = fdev; feature->id = finfo->fid; feature->revision = finfo->revision; feature->dfh_version = finfo->dfh_version; @@ -868,18 +861,6 @@ binfo_create_feature_dev_data(struct build_feature_devs_info *binfo) static int build_info_create_dev(struct build_feature_devs_info *binfo) { - enum dfl_id_type type = binfo->type; - struct platform_device *fdev; - - /* - * we use -ENODEV as the initialization indicator which indicates - * whether the id need to be reclaimed - */ - fdev = platform_device_alloc(dfl_devs[type].name, -ENODEV); - if (!fdev) - return -ENOMEM; - - binfo->feature_dev = fdev; binfo->feature_num = 0; INIT_LIST_HEAD(&binfo->sub_features); @@ -895,27 +876,43 @@ build_info_create_dev(struct build_feature_devs_info *binfo) static int feature_dev_register(struct dfl_feature_dev_data *fdata) { struct dfl_feature_platform_data pdata = {}; - struct platform_device *fdev = fdata->dev; + struct platform_device *fdev; + struct dfl_feature *feature; int ret; + fdev = platform_device_alloc(fdata->pdev_name, fdata->pdev_id); + if (!fdev) + return -ENOMEM; + + fdata->dev = fdev; + fdev->dev.parent = &fdata->dfl_cdev->region->dev; fdev->dev.devt = dfl_get_devt(dfl_devs[fdata->type].devt_type, fdev->id); + dfl_fpga_dev_for_each_feature(fdata, feature) + feature->dev = fdev; + ret = platform_device_add_resources(fdev, fdata->resources, fdata->resource_num); if (ret) - return ret; + goto err_put_dev; pdata.fdata = fdata; ret = platform_device_add_data(fdev, &pdata, sizeof(pdata)); if (ret) - return ret; + goto err_put_dev; ret = platform_device_add(fdev); if (ret) - return ret; + goto err_put_dev; return 0; + +err_put_dev: + platform_device_put(fdev); + fdata->dev = NULL; + + return ret; } static void feature_dev_unregister(struct dfl_feature_dev_data *fdata) @@ -940,16 +937,7 @@ static int build_info_commit_dev(struct build_feature_devs_info *binfo) if (binfo->type == PORT_ID) dfl_fpga_cdev_add_port_data(binfo->cdev, fdata); else - binfo->cdev->fme_dev = get_device(&binfo->feature_dev->dev); - - /* - * reset it to avoid build_info_free() freeing their resource. - * - * The resource of successfully registered feature devices - * will be freed by platform_device_unregister(). See the - * comments in build_info_create_dev(). - */ - binfo->feature_dev = NULL; + binfo->cdev->fme_dev = get_device(&fdata->dev->dev); /* reset the binfo for next FIU */ binfo->type = DFL_ID_MAX; @@ -966,8 +954,6 @@ static void build_info_free(struct build_feature_devs_info *binfo) kfree(finfo); } - platform_device_put(binfo->feature_dev); - devm_kfree(binfo->dev, binfo); } @@ -1262,8 +1248,7 @@ static int parse_feature_afu(struct build_feature_devs_info *binfo, case PORT_ID: return parse_feature_port_afu(binfo, ofst); default: - dev_info(binfo->dev, "AFU belonging to FIU %s is not supported yet.\n", - binfo->feature_dev->name); + dev_info(binfo->dev, "AFU belonging to FIU is not supported yet.\n"); } return 0;