[PATCH v2 0/8] fpga: Populate dev_release functions

[PATCH v2 0/8] fpga: Populate dev_release functions

[Russ Weight]

The FPGA framework has a convention of using managed resource functions
to allow parent drivers to manage the data structures allocated by the
class drivers. They use an empty *_dev_release() function to satisfy the
class driver.

This is inconsistent with linux driver model.

This is a complete re-do of the previous patch set entitled
"fpga: Use standard class dev_release function". These changes populate the
class dev_release callback functions while maintaining the current API.
Additional changes are made to maintain consistency with the driver model.

For more context on these changes, refer to this email thread:

https://marc.info/?l=linux-fpga&m=162127412218557&w=2

Changelog v1 -> v2:
  - Moved the renaming of "dev" to "parent" into a separate patch each for
    fpga-mgr, fpga-bridge, fpga-region.
  - Restored the call to fpga_mgr_free() in devm_*_mgr_release() instead of 
    changing it to put_device().
  - Replaced patch "fpga: altera-pr-ip: Remove function alt_pr_unregister"
    with "fpga: altera-pr-ip: Remove function alt_pr_unregister". This patch
    removes the alt_pr_unregister() function altogether, instead of just
    removing portions of it.

Russ Weight (8):
  fpga: altera-pr-ip: Remove function alt_pr_unregister
  fpga: stratix10-soc: Add missing fpga_mgr_free() call
  fpga: mgr: Rename dev to parent for parent device
  fpga: bridge: Rename dev to parent for parent device
  fpga: region: Rename dev to parent for parent device
  fpga: mgr: Use standard dev_release for class driver
  fpga: bridge: Use standard dev_release for class driver
  fpga: region: Use standard dev_release for class driver

 drivers/fpga/altera-pr-ip-core.c       | 10 -----
 drivers/fpga/fpga-bridge.c             | 46 ++++++++++-----------
 drivers/fpga/fpga-mgr.c                | 55 ++++++++++++--------------
 drivers/fpga/fpga-region.c             | 44 ++++++++++-----------
 drivers/fpga/stratix10-soc.c           |  1 +
 include/linux/fpga/altera-pr-ip-core.h |  1 -
 6 files changed, 71 insertions(+), 86 deletions(-)

-- 
2.25.1

[PATCH v2 1/8] fpga: altera-pr-ip: Remove function alt_pr_unregister

[Russ Weight]

Remove the alt_pr_unregister() function; it is no longer used.

Signed-off-by: Russ Weight <russell.h.weight@intel.com>
---
v2:
  - The first version of this patch was entitled:
    "fpga: altera-pr-ip: Remove function alt_pr_unregister". This version of the
    patch removes the alt_pr_unregister() function altogether, instead of just
    removing portions of it.
---
 drivers/fpga/altera-pr-ip-core.c       | 10 ----------
 include/linux/fpga/altera-pr-ip-core.h |  1 -
 2 files changed, 11 deletions(-)

diff --git a/drivers/fpga/altera-pr-ip-core.c b/drivers/fpga/altera-pr-ip-core.c
index 5b130c4d9882..dfdf21ed34c4 100644
--- a/drivers/fpga/altera-pr-ip-core.c
+++ b/drivers/fpga/altera-pr-ip-core.c
@@ -199,16 +199,6 @@ int alt_pr_register(struct device *dev, void __iomem *reg_base)
 }
 EXPORT_SYMBOL_GPL(alt_pr_register);
 
-void alt_pr_unregister(struct device *dev)
-{
-	struct fpga_manager *mgr = dev_get_drvdata(dev);
-
-	dev_dbg(dev, "%s\n", __func__);
-
-	fpga_mgr_unregister(mgr);
-}
-EXPORT_SYMBOL_GPL(alt_pr_unregister);
-
 MODULE_AUTHOR("Matthew Gerlach <matthew.gerlach@linux.intel.com>");
 MODULE_DESCRIPTION("Altera Partial Reconfiguration IP Core");
 MODULE_LICENSE("GPL v2");
diff --git a/include/linux/fpga/altera-pr-ip-core.h b/include/linux/fpga/altera-pr-ip-core.h
index 0b08ac20ab16..a6b4c07858cc 100644
--- a/include/linux/fpga/altera-pr-ip-core.h
+++ b/include/linux/fpga/altera-pr-ip-core.h
@@ -13,6 +13,5 @@
 #include <linux/io.h>
 
 int alt_pr_register(struct device *dev, void __iomem *reg_base);
-void alt_pr_unregister(struct device *dev);
 
 #endif /* _ALT_PR_IP_CORE_H */
-- 
2.25.1

Re: [PATCH v2 1/8] fpga: altera-pr-ip: Remove function alt_pr_unregister

[Xu Yilun]

Looks good to me.

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

On Wed, Jun 09, 2021 at 03:11:28PM -0700, Russ Weight wrote:
> Remove the alt_pr_unregister() function; it is no longer used.
> 
> Signed-off-by: Russ Weight <russell.h.weight@intel.com>
> ---
> v2:
>   - The first version of this patch was entitled:
>     "fpga: altera-pr-ip: Remove function alt_pr_unregister". This version of the
>     patch removes the alt_pr_unregister() function altogether, instead of just
>     removing portions of it.
> ---
>  drivers/fpga/altera-pr-ip-core.c       | 10 ----------
>  include/linux/fpga/altera-pr-ip-core.h |  1 -
>  2 files changed, 11 deletions(-)
> 
> diff --git a/drivers/fpga/altera-pr-ip-core.c b/drivers/fpga/altera-pr-ip-core.c
> index 5b130c4d9882..dfdf21ed34c4 100644
> --- a/drivers/fpga/altera-pr-ip-core.c
> +++ b/drivers/fpga/altera-pr-ip-core.c
> @@ -199,16 +199,6 @@ int alt_pr_register(struct device *dev, void __iomem *reg_base)
>  }
>  EXPORT_SYMBOL_GPL(alt_pr_register);
>  
> -void alt_pr_unregister(struct device *dev)
> -{
> -	struct fpga_manager *mgr = dev_get_drvdata(dev);
> -
> -	dev_dbg(dev, "%s\n", __func__);
> -
> -	fpga_mgr_unregister(mgr);
> -}
> -EXPORT_SYMBOL_GPL(alt_pr_unregister);
> -
>  MODULE_AUTHOR("Matthew Gerlach <matthew.gerlach@linux.intel.com>");
>  MODULE_DESCRIPTION("Altera Partial Reconfiguration IP Core");
>  MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/fpga/altera-pr-ip-core.h b/include/linux/fpga/altera-pr-ip-core.h
> index 0b08ac20ab16..a6b4c07858cc 100644
> --- a/include/linux/fpga/altera-pr-ip-core.h
> +++ b/include/linux/fpga/altera-pr-ip-core.h
> @@ -13,6 +13,5 @@
>  #include <linux/io.h>
>  
>  int alt_pr_register(struct device *dev, void __iomem *reg_base);
> -void alt_pr_unregister(struct device *dev);
>  
>  #endif /* _ALT_PR_IP_CORE_H */
> -- 
> 2.25.1

[PATCH v2 2/8] fpga: stratix10-soc: Add missing fpga_mgr_free() call

[Russ Weight]

The stratix10-soc driver uses fpga_mgr_create() function and is therefore
responsible to call fpga_mgr_free() to release the class driver resources.
Add a missing call to fpga_mgr_free in the s10_remove() function.

Signed-off-by: Russ Weight <russell.h.weight@intel.com>
Reviewed-by: Xu Yilun <yilun.xu@intel.com>
---
v2:
  - Added reviewed-by tag
---
 drivers/fpga/stratix10-soc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/fpga/stratix10-soc.c b/drivers/fpga/stratix10-soc.c
index 657a70c5fc99..9e34bbbce26e 100644
--- a/drivers/fpga/stratix10-soc.c
+++ b/drivers/fpga/stratix10-soc.c
@@ -454,6 +454,7 @@ static int s10_remove(struct platform_device *pdev)
 	struct s10_priv *priv = mgr->priv;
 
 	fpga_mgr_unregister(mgr);
+	fpga_mgr_free(mgr);
 	stratix10_svc_free_channel(priv->chan);
 
 	return 0;
-- 
2.25.1

[PATCH v2 3/8] fpga: mgr: Rename dev to parent for parent device

[Russ Weight]

Rename variable "dev" to "parent" in cases where it represents the parent
device.

Signed-off-by: Russ Weight <russell.h.weight@intel.com>
---
v2:
  - This patch contains the renaming of "dev" to "parent" that was previously
    part of the patch: "fpga: mgr: Use standard dev_release for class driver"
---
 drivers/fpga/fpga-mgr.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c
index b85bc47c91a9..42ddc0844781 100644
--- a/drivers/fpga/fpga-mgr.c
+++ b/drivers/fpga/fpga-mgr.c
@@ -551,7 +551,7 @@ EXPORT_SYMBOL_GPL(fpga_mgr_unlock);
 
 /**
  * fpga_mgr_create - create and initialize a FPGA manager struct
- * @dev:	fpga manager device from pdev
+ * @parent:	fpga manager device from pdev
  * @name:	fpga manager name
  * @mops:	pointer to structure of fpga manager ops
  * @priv:	fpga manager private data
@@ -561,7 +561,7 @@ EXPORT_SYMBOL_GPL(fpga_mgr_unlock);
  *
  * Return: pointer to struct fpga_manager or NULL
  */
-struct fpga_manager *fpga_mgr_create(struct device *dev, const char *name,
+struct fpga_manager *fpga_mgr_create(struct device *parent, const char *name,
 				     const struct fpga_manager_ops *mops,
 				     void *priv)
 {
@@ -571,12 +571,12 @@ struct fpga_manager *fpga_mgr_create(struct device *dev, const char *name,
 	if (!mops || !mops->write_complete || !mops->state ||
 	    !mops->write_init || (!mops->write && !mops->write_sg) ||
 	    (mops->write && mops->write_sg)) {
-		dev_err(dev, "Attempt to register without fpga_manager_ops\n");
+		dev_err(parent, "Attempt to register without fpga_manager_ops\n");
 		return NULL;
 	}
 
 	if (!name || !strlen(name)) {
-		dev_err(dev, "Attempt to register with no name!\n");
+		dev_err(parent, "Attempt to register with no name!\n");
 		return NULL;
 	}
 
@@ -597,8 +597,8 @@ struct fpga_manager *fpga_mgr_create(struct device *dev, const char *name,
 	device_initialize(&mgr->dev);
 	mgr->dev.class = fpga_mgr_class;
 	mgr->dev.groups = mops->groups;
-	mgr->dev.parent = dev;
-	mgr->dev.of_node = dev->of_node;
+	mgr->dev.parent = parent;
+	mgr->dev.of_node = parent->of_node;
 	mgr->dev.id = id;
 
 	ret = dev_set_name(&mgr->dev, "fpga%d", id);
@@ -636,7 +636,7 @@ static void devm_fpga_mgr_release(struct device *dev, void *res)
 
 /**
  * devm_fpga_mgr_create - create and initialize a managed FPGA manager struct
- * @dev:	fpga manager device from pdev
+ * @parent:	fpga manager device from pdev
  * @name:	fpga manager name
  * @mops:	pointer to structure of fpga manager ops
  * @priv:	fpga manager private data
@@ -651,7 +651,7 @@ static void devm_fpga_mgr_release(struct device *dev, void *res)
  *
  * Return: pointer to struct fpga_manager or NULL
  */
-struct fpga_manager *devm_fpga_mgr_create(struct device *dev, const char *name,
+struct fpga_manager *devm_fpga_mgr_create(struct device *parent, const char *name,
 					  const struct fpga_manager_ops *mops,
 					  void *priv)
 {
@@ -661,13 +661,13 @@ struct fpga_manager *devm_fpga_mgr_create(struct device *dev, const char *name,
 	if (!dr)
 		return NULL;
 
-	dr->mgr = fpga_mgr_create(dev, name, mops, priv);
+	dr->mgr = fpga_mgr_create(parent, name, mops, priv);
 	if (!dr->mgr) {
 		devres_free(dr);
 		return NULL;
 	}
 
-	devres_add(dev, dr);
+	devres_add(parent, dr);
 
 	return dr->mgr;
 }
-- 
2.25.1

Re: [PATCH v2 3/8] fpga: mgr: Rename dev to parent for parent device

[Xu Yilun]

Looks good to me.

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

On Wed, Jun 09, 2021 at 03:11:30PM -0700, Russ Weight wrote:
> Rename variable "dev" to "parent" in cases where it represents the parent
> device.
> 
> Signed-off-by: Russ Weight <russell.h.weight@intel.com>
> ---
> v2:
>   - This patch contains the renaming of "dev" to "parent" that was previously
>     part of the patch: "fpga: mgr: Use standard dev_release for class driver"
> ---
>  drivers/fpga/fpga-mgr.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c
> index b85bc47c91a9..42ddc0844781 100644
> --- a/drivers/fpga/fpga-mgr.c
> +++ b/drivers/fpga/fpga-mgr.c
> @@ -551,7 +551,7 @@ EXPORT_SYMBOL_GPL(fpga_mgr_unlock);
>  
>  /**
>   * fpga_mgr_create - create and initialize a FPGA manager struct
> - * @dev:	fpga manager device from pdev
> + * @parent:	fpga manager device from pdev
>   * @name:	fpga manager name
>   * @mops:	pointer to structure of fpga manager ops
>   * @priv:	fpga manager private data
> @@ -561,7 +561,7 @@ EXPORT_SYMBOL_GPL(fpga_mgr_unlock);
>   *
>   * Return: pointer to struct fpga_manager or NULL
>   */
> -struct fpga_manager *fpga_mgr_create(struct device *dev, const char *name,
> +struct fpga_manager *fpga_mgr_create(struct device *parent, const char *name,
>  				     const struct fpga_manager_ops *mops,
>  				     void *priv)
>  {
> @@ -571,12 +571,12 @@ struct fpga_manager *fpga_mgr_create(struct device *dev, const char *name,
>  	if (!mops || !mops->write_complete || !mops->state ||
>  	    !mops->write_init || (!mops->write && !mops->write_sg) ||
>  	    (mops->write && mops->write_sg)) {
> -		dev_err(dev, "Attempt to register without fpga_manager_ops\n");
> +		dev_err(parent, "Attempt to register without fpga_manager_ops\n");
>  		return NULL;
>  	}
>  
>  	if (!name || !strlen(name)) {
> -		dev_err(dev, "Attempt to register with no name!\n");
> +		dev_err(parent, "Attempt to register with no name!\n");
>  		return NULL;
>  	}
>  
> @@ -597,8 +597,8 @@ struct fpga_manager *fpga_mgr_create(struct device *dev, const char *name,
>  	device_initialize(&mgr->dev);
>  	mgr->dev.class = fpga_mgr_class;
>  	mgr->dev.groups = mops->groups;
> -	mgr->dev.parent = dev;
> -	mgr->dev.of_node = dev->of_node;
> +	mgr->dev.parent = parent;
> +	mgr->dev.of_node = parent->of_node;
>  	mgr->dev.id = id;
>  
>  	ret = dev_set_name(&mgr->dev, "fpga%d", id);
> @@ -636,7 +636,7 @@ static void devm_fpga_mgr_release(struct device *dev, void *res)
>  
>  /**
>   * devm_fpga_mgr_create - create and initialize a managed FPGA manager struct
> - * @dev:	fpga manager device from pdev
> + * @parent:	fpga manager device from pdev
>   * @name:	fpga manager name
>   * @mops:	pointer to structure of fpga manager ops
>   * @priv:	fpga manager private data
> @@ -651,7 +651,7 @@ static void devm_fpga_mgr_release(struct device *dev, void *res)
>   *
>   * Return: pointer to struct fpga_manager or NULL
>   */
> -struct fpga_manager *devm_fpga_mgr_create(struct device *dev, const char *name,
> +struct fpga_manager *devm_fpga_mgr_create(struct device *parent, const char *name,
>  					  const struct fpga_manager_ops *mops,
>  					  void *priv)
>  {
> @@ -661,13 +661,13 @@ struct fpga_manager *devm_fpga_mgr_create(struct device *dev, const char *name,
>  	if (!dr)
>  		return NULL;
>  
> -	dr->mgr = fpga_mgr_create(dev, name, mops, priv);
> +	dr->mgr = fpga_mgr_create(parent, name, mops, priv);
>  	if (!dr->mgr) {
>  		devres_free(dr);
>  		return NULL;
>  	}
>  
> -	devres_add(dev, dr);
> +	devres_add(parent, dr);
>  
>  	return dr->mgr;
>  }
> -- 
> 2.25.1

[PATCH v2 4/8] fpga: bridge: Rename dev to parent for parent device

[Russ Weight]

Rename variable "dev" to "parent" in cases where it represents the parent
device.

Signed-off-by: Russ Weight <russell.h.weight@intel.com>
---
v2:
  - This patch contains the renaming of "dev" to "parent" that was previously
    part of the patch: "fpga: bridge: Use standard dev_release for class driver"
---
 drivers/fpga/fpga-bridge.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/fpga/fpga-bridge.c b/drivers/fpga/fpga-bridge.c
index 05c6d4f2d043..6c56afc66a6d 100644
--- a/drivers/fpga/fpga-bridge.c
+++ b/drivers/fpga/fpga-bridge.c
@@ -313,7 +313,7 @@ ATTRIBUTE_GROUPS(fpga_bridge);
 
 /**
  * fpga_bridge_create - create and initialize a struct fpga_bridge
- * @dev:	FPGA bridge device from pdev
+ * @parent:	FPGA bridge device from pdev
  * @name:	FPGA bridge name
  * @br_ops:	pointer to structure of fpga bridge ops
  * @priv:	FPGA bridge private data
@@ -323,7 +323,7 @@ ATTRIBUTE_GROUPS(fpga_bridge);
  *
  * Return: struct fpga_bridge or NULL
  */
-struct fpga_bridge *fpga_bridge_create(struct device *dev, const char *name,
+struct fpga_bridge *fpga_bridge_create(struct device *parent, const char *name,
 				       const struct fpga_bridge_ops *br_ops,
 				       void *priv)
 {
@@ -331,7 +331,7 @@ struct fpga_bridge *fpga_bridge_create(struct device *dev, const char *name,
 	int id, ret;
 
 	if (!name || !strlen(name)) {
-		dev_err(dev, "Attempt to register with no name!\n");
+		dev_err(parent, "Attempt to register with no name!\n");
 		return NULL;
 	}
 
@@ -353,8 +353,8 @@ struct fpga_bridge *fpga_bridge_create(struct device *dev, const char *name,
 	device_initialize(&bridge->dev);
 	bridge->dev.groups = br_ops->groups;
 	bridge->dev.class = fpga_bridge_class;
-	bridge->dev.parent = dev;
-	bridge->dev.of_node = dev->of_node;
+	bridge->dev.parent = parent;
+	bridge->dev.of_node = parent->of_node;
 	bridge->dev.id = id;
 
 	ret = dev_set_name(&bridge->dev, "br%d", id);
@@ -392,7 +392,7 @@ static void devm_fpga_bridge_release(struct device *dev, void *res)
 
 /**
  * devm_fpga_bridge_create - create and init a managed struct fpga_bridge
- * @dev:	FPGA bridge device from pdev
+ * @parent:	FPGA bridge device from pdev
  * @name:	FPGA bridge name
  * @br_ops:	pointer to structure of fpga bridge ops
  * @priv:	FPGA bridge private data
@@ -408,7 +408,7 @@ static void devm_fpga_bridge_release(struct device *dev, void *res)
  *  Return: struct fpga_bridge or NULL
  */
 struct fpga_bridge
-*devm_fpga_bridge_create(struct device *dev, const char *name,
+*devm_fpga_bridge_create(struct device *parent, const char *name,
 			 const struct fpga_bridge_ops *br_ops, void *priv)
 {
 	struct fpga_bridge **ptr, *bridge;
@@ -417,12 +417,12 @@ struct fpga_bridge
 	if (!ptr)
 		return NULL;
 
-	bridge = fpga_bridge_create(dev, name, br_ops, priv);
+	bridge = fpga_bridge_create(parent, name, br_ops, priv);
 	if (!bridge) {
 		devres_free(ptr);
 	} else {
 		*ptr = bridge;
-		devres_add(dev, ptr);
+		devres_add(parent, ptr);
 	}
 
 	return bridge;
-- 
2.25.1

Re: [PATCH v2 4/8] fpga: bridge: Rename dev to parent for parent device

[Xu Yilun]

Looks good to me.

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

On Wed, Jun 09, 2021 at 03:11:31PM -0700, Russ Weight wrote:
> Rename variable "dev" to "parent" in cases where it represents the parent
> device.
> 
> Signed-off-by: Russ Weight <russell.h.weight@intel.com>
> ---
> v2:
>   - This patch contains the renaming of "dev" to "parent" that was previously
>     part of the patch: "fpga: bridge: Use standard dev_release for class driver"
> ---
>  drivers/fpga/fpga-bridge.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/fpga/fpga-bridge.c b/drivers/fpga/fpga-bridge.c
> index 05c6d4f2d043..6c56afc66a6d 100644
> --- a/drivers/fpga/fpga-bridge.c
> +++ b/drivers/fpga/fpga-bridge.c
> @@ -313,7 +313,7 @@ ATTRIBUTE_GROUPS(fpga_bridge);
>  
>  /**
>   * fpga_bridge_create - create and initialize a struct fpga_bridge
> - * @dev:	FPGA bridge device from pdev
> + * @parent:	FPGA bridge device from pdev
>   * @name:	FPGA bridge name
>   * @br_ops:	pointer to structure of fpga bridge ops
>   * @priv:	FPGA bridge private data
> @@ -323,7 +323,7 @@ ATTRIBUTE_GROUPS(fpga_bridge);
>   *
>   * Return: struct fpga_bridge or NULL
>   */
> -struct fpga_bridge *fpga_bridge_create(struct device *dev, const char *name,
> +struct fpga_bridge *fpga_bridge_create(struct device *parent, const char *name,
>  				       const struct fpga_bridge_ops *br_ops,
>  				       void *priv)
>  {
> @@ -331,7 +331,7 @@ struct fpga_bridge *fpga_bridge_create(struct device *dev, const char *name,
>  	int id, ret;
>  
>  	if (!name || !strlen(name)) {
> -		dev_err(dev, "Attempt to register with no name!\n");
> +		dev_err(parent, "Attempt to register with no name!\n");
>  		return NULL;
>  	}
>  
> @@ -353,8 +353,8 @@ struct fpga_bridge *fpga_bridge_create(struct device *dev, const char *name,
>  	device_initialize(&bridge->dev);
>  	bridge->dev.groups = br_ops->groups;
>  	bridge->dev.class = fpga_bridge_class;
> -	bridge->dev.parent = dev;
> -	bridge->dev.of_node = dev->of_node;
> +	bridge->dev.parent = parent;
> +	bridge->dev.of_node = parent->of_node;
>  	bridge->dev.id = id;
>  
>  	ret = dev_set_name(&bridge->dev, "br%d", id);
> @@ -392,7 +392,7 @@ static void devm_fpga_bridge_release(struct device *dev, void *res)
>  
>  /**
>   * devm_fpga_bridge_create - create and init a managed struct fpga_bridge
> - * @dev:	FPGA bridge device from pdev
> + * @parent:	FPGA bridge device from pdev
>   * @name:	FPGA bridge name
>   * @br_ops:	pointer to structure of fpga bridge ops
>   * @priv:	FPGA bridge private data
> @@ -408,7 +408,7 @@ static void devm_fpga_bridge_release(struct device *dev, void *res)
>   *  Return: struct fpga_bridge or NULL
>   */
>  struct fpga_bridge
> -*devm_fpga_bridge_create(struct device *dev, const char *name,
> +*devm_fpga_bridge_create(struct device *parent, const char *name,
>  			 const struct fpga_bridge_ops *br_ops, void *priv)
>  {
>  	struct fpga_bridge **ptr, *bridge;
> @@ -417,12 +417,12 @@ struct fpga_bridge
>  	if (!ptr)
>  		return NULL;
>  
> -	bridge = fpga_bridge_create(dev, name, br_ops, priv);
> +	bridge = fpga_bridge_create(parent, name, br_ops, priv);
>  	if (!bridge) {
>  		devres_free(ptr);
>  	} else {
>  		*ptr = bridge;
> -		devres_add(dev, ptr);
> +		devres_add(parent, ptr);
>  	}
>  
>  	return bridge;
> -- 
> 2.25.1

[PATCH v2 5/8] fpga: region: Rename dev to parent for parent device

[Russ Weight]

Rename variable "dev" to "parent" in cases where it represents the parent
device.

Signed-off-by: Russ Weight <russell.h.weight@intel.com>
---
v2:
  - This patch contains the renaming of "dev" to "parent" that was previously
    part of the patch: "fpga: region: Use standard dev_release for class driver"
---
 drivers/fpga/fpga-region.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c
index c3134b89c3fe..563626d66e56 100644
--- a/drivers/fpga/fpga-region.c
+++ b/drivers/fpga/fpga-region.c
@@ -181,7 +181,7 @@ ATTRIBUTE_GROUPS(fpga_region);
 
 /**
  * fpga_region_create - alloc and init a struct fpga_region
- * @dev: device parent
+ * @parent: device parent
  * @mgr: manager that programs this region
  * @get_bridges: optional function to get bridges to a list
  *
@@ -192,7 +192,7 @@ ATTRIBUTE_GROUPS(fpga_region);
  * Return: struct fpga_region or NULL
  */
 struct fpga_region
-*fpga_region_create(struct device *dev,
+*fpga_region_create(struct device *parent,
 		    struct fpga_manager *mgr,
 		    int (*get_bridges)(struct fpga_region *))
 {
@@ -214,8 +214,8 @@ struct fpga_region
 
 	device_initialize(&region->dev);
 	region->dev.class = fpga_region_class;
-	region->dev.parent = dev;
-	region->dev.of_node = dev->of_node;
+	region->dev.parent = parent;
+	region->dev.of_node = parent->of_node;
 	region->dev.id = id;
 
 	ret = dev_set_name(&region->dev, "region%d", id);
@@ -268,7 +268,7 @@ static void devm_fpga_region_release(struct device *dev, void *res)
  * Return: struct fpga_region or NULL
  */
 struct fpga_region
-*devm_fpga_region_create(struct device *dev,
+*devm_fpga_region_create(struct device *parent,
 			 struct fpga_manager *mgr,
 			 int (*get_bridges)(struct fpga_region *))
 {
@@ -278,12 +278,12 @@ struct fpga_region
 	if (!ptr)
 		return NULL;
 
-	region = fpga_region_create(dev, mgr, get_bridges);
+	region = fpga_region_create(parent, mgr, get_bridges);
 	if (!region) {
 		devres_free(ptr);
 	} else {
 		*ptr = region;
-		devres_add(dev, ptr);
+		devres_add(parent, ptr);
 	}
 
 	return region;
-- 
2.25.1

Re: [PATCH v2 5/8] fpga: region: Rename dev to parent for parent device

[Xu Yilun]

Looks good to me.

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

On Wed, Jun 09, 2021 at 03:11:32PM -0700, Russ Weight wrote:
> Rename variable "dev" to "parent" in cases where it represents the parent
> device.
> 
> Signed-off-by: Russ Weight <russell.h.weight@intel.com>
> ---
> v2:
>   - This patch contains the renaming of "dev" to "parent" that was previously
>     part of the patch: "fpga: region: Use standard dev_release for class driver"
> ---
>  drivers/fpga/fpga-region.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c
> index c3134b89c3fe..563626d66e56 100644
> --- a/drivers/fpga/fpga-region.c
> +++ b/drivers/fpga/fpga-region.c
> @@ -181,7 +181,7 @@ ATTRIBUTE_GROUPS(fpga_region);
>  
>  /**
>   * fpga_region_create - alloc and init a struct fpga_region
> - * @dev: device parent
> + * @parent: device parent
>   * @mgr: manager that programs this region
>   * @get_bridges: optional function to get bridges to a list
>   *
> @@ -192,7 +192,7 @@ ATTRIBUTE_GROUPS(fpga_region);
>   * Return: struct fpga_region or NULL
>   */
>  struct fpga_region
> -*fpga_region_create(struct device *dev,
> +*fpga_region_create(struct device *parent,
>  		    struct fpga_manager *mgr,
>  		    int (*get_bridges)(struct fpga_region *))
>  {
> @@ -214,8 +214,8 @@ struct fpga_region
>  
>  	device_initialize(&region->dev);
>  	region->dev.class = fpga_region_class;
> -	region->dev.parent = dev;
> -	region->dev.of_node = dev->of_node;
> +	region->dev.parent = parent;
> +	region->dev.of_node = parent->of_node;
>  	region->dev.id = id;
>  
>  	ret = dev_set_name(&region->dev, "region%d", id);
> @@ -268,7 +268,7 @@ static void devm_fpga_region_release(struct device *dev, void *res)
>   * Return: struct fpga_region or NULL
>   */
>  struct fpga_region
> -*devm_fpga_region_create(struct device *dev,
> +*devm_fpga_region_create(struct device *parent,
>  			 struct fpga_manager *mgr,
>  			 int (*get_bridges)(struct fpga_region *))
>  {
> @@ -278,12 +278,12 @@ struct fpga_region
>  	if (!ptr)
>  		return NULL;
>  
> -	region = fpga_region_create(dev, mgr, get_bridges);
> +	region = fpga_region_create(parent, mgr, get_bridges);
>  	if (!region) {
>  		devres_free(ptr);
>  	} else {
>  		*ptr = region;
> -		devres_add(dev, ptr);
> +		devres_add(parent, ptr);
>  	}
>  
>  	return region;
> -- 
> 2.25.1

Re: [PATCH v2 5/8] fpga: region: Rename dev to parent for parent device

[Xu Yilun]

Sorry, one more minor comment inline.

On Wed, Jun 09, 2021 at 03:11:32PM -0700, Russ Weight wrote:
> Rename variable "dev" to "parent" in cases where it represents the parent
> device.
> 
> Signed-off-by: Russ Weight <russell.h.weight@intel.com>
> ---
> v2:
>   - This patch contains the renaming of "dev" to "parent" that was previously
>     part of the patch: "fpga: region: Use standard dev_release for class driver"
> ---
>  drivers/fpga/fpga-region.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c
> index c3134b89c3fe..563626d66e56 100644
> --- a/drivers/fpga/fpga-region.c
> +++ b/drivers/fpga/fpga-region.c
> @@ -181,7 +181,7 @@ ATTRIBUTE_GROUPS(fpga_region);
>  
>  /**
>   * fpga_region_create - alloc and init a struct fpga_region
> - * @dev: device parent
> + * @parent: device parent
>   * @mgr: manager that programs this region
>   * @get_bridges: optional function to get bridges to a list
>   *
> @@ -192,7 +192,7 @@ ATTRIBUTE_GROUPS(fpga_region);
>   * Return: struct fpga_region or NULL
>   */
>  struct fpga_region
> -*fpga_region_create(struct device *dev,
> +*fpga_region_create(struct device *parent,
>  		    struct fpga_manager *mgr,
>  		    int (*get_bridges)(struct fpga_region *))
>  {
> @@ -214,8 +214,8 @@ struct fpga_region
>  
>  	device_initialize(&region->dev);
>  	region->dev.class = fpga_region_class;
> -	region->dev.parent = dev;
> -	region->dev.of_node = dev->of_node;
> +	region->dev.parent = parent;
> +	region->dev.of_node = parent->of_node;
>  	region->dev.id = id;
>  
>  	ret = dev_set_name(&region->dev, "region%d", id);
> @@ -268,7 +268,7 @@ static void devm_fpga_region_release(struct device *dev, void *res)
>   * Return: struct fpga_region or NULL
>   */
>  struct fpga_region
> -*devm_fpga_region_create(struct device *dev,
> +*devm_fpga_region_create(struct device *parent,

You missed the function description doc change in Patch #8

Thanks,
Yilun

>  			 struct fpga_manager *mgr,
>  			 int (*get_bridges)(struct fpga_region *))



>  {
> @@ -278,12 +278,12 @@ struct fpga_region
>  	if (!ptr)
>  		return NULL;
>  
> -	region = fpga_region_create(dev, mgr, get_bridges);
> +	region = fpga_region_create(parent, mgr, get_bridges);
>  	if (!region) {
>  		devres_free(ptr);
>  	} else {
>  		*ptr = region;
> -		devres_add(dev, ptr);
> +		devres_add(parent, ptr);
>  	}
>  
>  	return region;
> -- 
> 2.25.1

Re: [PATCH v2 5/8] fpga: region: Rename dev to parent for parent device

[Russ Weight]

On 6/10/21 1:25 AM, Xu Yilun wrote:
> Sorry, one more minor comment inline.
>
> On Wed, Jun 09, 2021 at 03:11:32PM -0700, Russ Weight wrote:
>> Rename variable "dev" to "parent" in cases where it represents the parent
>> device.
>>
>> Signed-off-by: Russ Weight <russell.h.weight@intel.com>
>> ---
>> v2:
>>   - This patch contains the renaming of "dev" to "parent" that was previously
>>     part of the patch: "fpga: region: Use standard dev_release for class driver"
>> ---
>>  drivers/fpga/fpga-region.c | 14 +++++++-------
>>  1 file changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c
>> index c3134b89c3fe..563626d66e56 100644
>> --- a/drivers/fpga/fpga-region.c
>> +++ b/drivers/fpga/fpga-region.c
>> @@ -181,7 +181,7 @@ ATTRIBUTE_GROUPS(fpga_region);
>>  
>>  /**
>>   * fpga_region_create - alloc and init a struct fpga_region
>> - * @dev: device parent
>> + * @parent: device parent
>>   * @mgr: manager that programs this region
>>   * @get_bridges: optional function to get bridges to a list
>>   *
>> @@ -192,7 +192,7 @@ ATTRIBUTE_GROUPS(fpga_region);
>>   * Return: struct fpga_region or NULL
>>   */
>>  struct fpga_region
>> -*fpga_region_create(struct device *dev,
>> +*fpga_region_create(struct device *parent,
>>  		    struct fpga_manager *mgr,
>>  		    int (*get_bridges)(struct fpga_region *))
>>  {
>> @@ -214,8 +214,8 @@ struct fpga_region
>>  
>>  	device_initialize(&region->dev);
>>  	region->dev.class = fpga_region_class;
>> -	region->dev.parent = dev;
>> -	region->dev.of_node = dev->of_node;
>> +	region->dev.parent = parent;
>> +	region->dev.of_node = parent->of_node;
>>  	region->dev.id = id;
>>  
>>  	ret = dev_set_name(&region->dev, "region%d", id);
>> @@ -268,7 +268,7 @@ static void devm_fpga_region_release(struct device *dev, void *res)
>>   * Return: struct fpga_region or NULL
>>   */
>>  struct fpga_region
>> -*devm_fpga_region_create(struct device *dev,
>> +*devm_fpga_region_create(struct device *parent,
> You missed the function description doc change in Patch #8

Thanks for catching that Yilun. I have fixed it for the next patchset.

- Russ
>
> Thanks,
> Yilun
>
>>  			 struct fpga_manager *mgr,
>>  			 int (*get_bridges)(struct fpga_region *))
>
>
>>  {
>> @@ -278,12 +278,12 @@ struct fpga_region
>>  	if (!ptr)
>>  		return NULL;
>>  
>> -	region = fpga_region_create(dev, mgr, get_bridges);
>> +	region = fpga_region_create(parent, mgr, get_bridges);
>>  	if (!region) {
>>  		devres_free(ptr);
>>  	} else {
>>  		*ptr = region;
>> -		devres_add(dev, ptr);
>> +		devres_add(parent, ptr);
>>  	}
>>  
>>  	return region;
>> -- 
>> 2.25.1

[PATCH v2 6/8] fpga: mgr: Use standard dev_release for class driver

[Russ Weight]

The FPGA manager class driver data structure is being treated as a
managed resource instead of using the class.dev_release call-back
function to release the class data structure. This change populates
the class.dev_release function, changes the fpga_mgr_free() function
to call put_device() and changes the fpga_mgr_unregister() function
to call device_del() instead of device_unregister().

Signed-off-by: Russ Weight <russell.h.weight@intel.com>
---
v2:
  - Moved the renaming of "dev" to "parent" to a separate patch
  - Call fpga_mgr_free() in devm_fpga_mgr_release() instead of put_device()
---
 drivers/fpga/fpga-mgr.c | 35 +++++++++++++++--------------------
 1 file changed, 15 insertions(+), 20 deletions(-)

diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c
index 42ddc0844781..9f6c3760b6ff 100644
--- a/drivers/fpga/fpga-mgr.c
+++ b/drivers/fpga/fpga-mgr.c
@@ -585,8 +585,10 @@ struct fpga_manager *fpga_mgr_create(struct device *parent, const char *name,
 		return NULL;
 
 	id = ida_simple_get(&fpga_mgr_ida, 0, 0, GFP_KERNEL);
-	if (id < 0)
-		goto error_kfree;
+	if (id < 0) {
+		kfree(mgr);
+		return NULL;
+	}
 
 	mutex_init(&mgr->ref_mutex);
 
@@ -602,17 +604,12 @@ struct fpga_manager *fpga_mgr_create(struct device *parent, const char *name,
 	mgr->dev.id = id;
 
 	ret = dev_set_name(&mgr->dev, "fpga%d", id);
-	if (ret)
-		goto error_device;
+	if (ret) {
+		put_device(&mgr->dev);
+		return NULL;
+	}
 
 	return mgr;
-
-error_device:
-	ida_simple_remove(&fpga_mgr_ida, id);
-error_kfree:
-	kfree(mgr);
-
-	return NULL;
 }
 EXPORT_SYMBOL_GPL(fpga_mgr_create);
 
@@ -622,8 +619,7 @@ EXPORT_SYMBOL_GPL(fpga_mgr_create);
  */
 void fpga_mgr_free(struct fpga_manager *mgr)
 {
-	ida_simple_remove(&fpga_mgr_ida, mgr->dev.id);
-	kfree(mgr);
+	put_device(&mgr->dev);
 }
 EXPORT_SYMBOL_GPL(fpga_mgr_free);
 
@@ -692,16 +688,11 @@ int fpga_mgr_register(struct fpga_manager *mgr)
 
 	ret = device_add(&mgr->dev);
 	if (ret)
-		goto error_device;
+		return ret;
 
 	dev_info(&mgr->dev, "%s registered\n", mgr->name);
 
 	return 0;
-
-error_device:
-	ida_simple_remove(&fpga_mgr_ida, mgr->dev.id);
-
-	return ret;
 }
 EXPORT_SYMBOL_GPL(fpga_mgr_register);
 
@@ -722,7 +713,7 @@ void fpga_mgr_unregister(struct fpga_manager *mgr)
 	if (mgr->mops->fpga_remove)
 		mgr->mops->fpga_remove(mgr);
 
-	device_unregister(&mgr->dev);
+	device_del(&mgr->dev);
 }
 EXPORT_SYMBOL_GPL(fpga_mgr_unregister);
 
@@ -781,6 +772,10 @@ EXPORT_SYMBOL_GPL(devm_fpga_mgr_register);
 
 static void fpga_mgr_dev_release(struct device *dev)
 {
+	struct fpga_manager *mgr = to_fpga_manager(dev);
+
+	ida_simple_remove(&fpga_mgr_ida, mgr->dev.id);
+	kfree(mgr);
 }
 
 static int __init fpga_mgr_class_init(void)
-- 
2.25.1

Re: [PATCH v2 6/8] fpga: mgr: Use standard dev_release for class driver

[Xu Yilun]

Looks good to me.

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

On Wed, Jun 09, 2021 at 03:11:33PM -0700, Russ Weight wrote:
> The FPGA manager class driver data structure is being treated as a
> managed resource instead of using the class.dev_release call-back
> function to release the class data structure. This change populates
> the class.dev_release function, changes the fpga_mgr_free() function
> to call put_device() and changes the fpga_mgr_unregister() function
> to call device_del() instead of device_unregister().
> 
> Signed-off-by: Russ Weight <russell.h.weight@intel.com>
> ---
> v2:
>   - Moved the renaming of "dev" to "parent" to a separate patch
>   - Call fpga_mgr_free() in devm_fpga_mgr_release() instead of put_device()
> ---
>  drivers/fpga/fpga-mgr.c | 35 +++++++++++++++--------------------
>  1 file changed, 15 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c
> index 42ddc0844781..9f6c3760b6ff 100644
> --- a/drivers/fpga/fpga-mgr.c
> +++ b/drivers/fpga/fpga-mgr.c
> @@ -585,8 +585,10 @@ struct fpga_manager *fpga_mgr_create(struct device *parent, const char *name,
>  		return NULL;
>  
>  	id = ida_simple_get(&fpga_mgr_ida, 0, 0, GFP_KERNEL);
> -	if (id < 0)
> -		goto error_kfree;
> +	if (id < 0) {
> +		kfree(mgr);
> +		return NULL;
> +	}
>  
>  	mutex_init(&mgr->ref_mutex);
>  
> @@ -602,17 +604,12 @@ struct fpga_manager *fpga_mgr_create(struct device *parent, const char *name,
>  	mgr->dev.id = id;
>  
>  	ret = dev_set_name(&mgr->dev, "fpga%d", id);
> -	if (ret)
> -		goto error_device;
> +	if (ret) {
> +		put_device(&mgr->dev);
> +		return NULL;
> +	}
>  
>  	return mgr;
> -
> -error_device:
> -	ida_simple_remove(&fpga_mgr_ida, id);
> -error_kfree:
> -	kfree(mgr);
> -
> -	return NULL;
>  }
>  EXPORT_SYMBOL_GPL(fpga_mgr_create);
>  
> @@ -622,8 +619,7 @@ EXPORT_SYMBOL_GPL(fpga_mgr_create);
>   */
>  void fpga_mgr_free(struct fpga_manager *mgr)
>  {
> -	ida_simple_remove(&fpga_mgr_ida, mgr->dev.id);
> -	kfree(mgr);
> +	put_device(&mgr->dev);
>  }
>  EXPORT_SYMBOL_GPL(fpga_mgr_free);
>  
> @@ -692,16 +688,11 @@ int fpga_mgr_register(struct fpga_manager *mgr)
>  
>  	ret = device_add(&mgr->dev);
>  	if (ret)
> -		goto error_device;
> +		return ret;
>  
>  	dev_info(&mgr->dev, "%s registered\n", mgr->name);
>  
>  	return 0;
> -
> -error_device:
> -	ida_simple_remove(&fpga_mgr_ida, mgr->dev.id);
> -
> -	return ret;
>  }
>  EXPORT_SYMBOL_GPL(fpga_mgr_register);
>  
> @@ -722,7 +713,7 @@ void fpga_mgr_unregister(struct fpga_manager *mgr)
>  	if (mgr->mops->fpga_remove)
>  		mgr->mops->fpga_remove(mgr);
>  
> -	device_unregister(&mgr->dev);
> +	device_del(&mgr->dev);
>  }
>  EXPORT_SYMBOL_GPL(fpga_mgr_unregister);
>  
> @@ -781,6 +772,10 @@ EXPORT_SYMBOL_GPL(devm_fpga_mgr_register);
>  
>  static void fpga_mgr_dev_release(struct device *dev)
>  {
> +	struct fpga_manager *mgr = to_fpga_manager(dev);
> +
> +	ida_simple_remove(&fpga_mgr_ida, mgr->dev.id);
> +	kfree(mgr);
>  }
>  
>  static int __init fpga_mgr_class_init(void)
> -- 
> 2.25.1

[PATCH v2 7/8] fpga: bridge: Use standard dev_release for class driver

[Russ Weight]

The FPGA bridge class driver data structure is being treated as a managed
resource instead of using standard dev_release call-back to release the
class data structure. This change populates the class.dev_release function
and changes the fpga_bridge_free() function to call put_device(). It also
changes fpga_bridge_unregister() to call device_del() instead of
device_unregister().

Signed-off-by: Russ Weight <russell.h.weight@intel.com>
---
v2:
  - Moved the renaming of "dev" to "parent" to a separate patch
  - Call fpga_bridge_free() in devm_fpga_bridge_release() instead of
    put_device()
---
 drivers/fpga/fpga-bridge.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/fpga/fpga-bridge.c b/drivers/fpga/fpga-bridge.c
index 6c56afc66a6d..b40f4b4228e7 100644
--- a/drivers/fpga/fpga-bridge.c
+++ b/drivers/fpga/fpga-bridge.c
@@ -340,8 +340,10 @@ struct fpga_bridge *fpga_bridge_create(struct device *parent, const char *name,
 		return NULL;
 
 	id = ida_simple_get(&fpga_bridge_ida, 0, 0, GFP_KERNEL);
-	if (id < 0)
-		goto error_kfree;
+	if (id < 0) {
+		kfree(bridge);
+		return NULL;
+	}
 
 	mutex_init(&bridge->mutex);
 	INIT_LIST_HEAD(&bridge->node);
@@ -358,17 +360,12 @@ struct fpga_bridge *fpga_bridge_create(struct device *parent, const char *name,
 	bridge->dev.id = id;
 
 	ret = dev_set_name(&bridge->dev, "br%d", id);
-	if (ret)
-		goto error_device;
+	if (ret) {
+		put_device(&bridge->dev);
+		return NULL;
+	}
 
 	return bridge;
-
-error_device:
-	ida_simple_remove(&fpga_bridge_ida, id);
-error_kfree:
-	kfree(bridge);
-
-	return NULL;
 }
 EXPORT_SYMBOL_GPL(fpga_bridge_create);
 
@@ -378,8 +375,7 @@ EXPORT_SYMBOL_GPL(fpga_bridge_create);
  */
 void fpga_bridge_free(struct fpga_bridge *bridge)
 {
-	ida_simple_remove(&fpga_bridge_ida, bridge->dev.id);
-	kfree(bridge);
+	put_device(&bridge->dev);
 }
 EXPORT_SYMBOL_GPL(fpga_bridge_free);
 
@@ -469,12 +465,16 @@ void fpga_bridge_unregister(struct fpga_bridge *bridge)
 	if (bridge->br_ops && bridge->br_ops->fpga_bridge_remove)
 		bridge->br_ops->fpga_bridge_remove(bridge);
 
-	device_unregister(&bridge->dev);
+	device_del(&bridge->dev);
 }
 EXPORT_SYMBOL_GPL(fpga_bridge_unregister);
 
 static void fpga_bridge_dev_release(struct device *dev)
 {
+	struct fpga_bridge *bridge = to_fpga_bridge(dev);
+
+	ida_simple_remove(&fpga_bridge_ida, bridge->dev.id);
+	kfree(bridge);
 }
 
 static int __init fpga_bridge_dev_init(void)
-- 
2.25.1

Re: [PATCH v2 7/8] fpga: bridge: Use standard dev_release for class driver

[Xu Yilun]

Looks good to me.

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

On Wed, Jun 09, 2021 at 03:11:34PM -0700, Russ Weight wrote:
> The FPGA bridge class driver data structure is being treated as a managed
> resource instead of using standard dev_release call-back to release the
> class data structure. This change populates the class.dev_release function
> and changes the fpga_bridge_free() function to call put_device(). It also
> changes fpga_bridge_unregister() to call device_del() instead of
> device_unregister().
> 
> Signed-off-by: Russ Weight <russell.h.weight@intel.com>
> ---
> v2:
>   - Moved the renaming of "dev" to "parent" to a separate patch
>   - Call fpga_bridge_free() in devm_fpga_bridge_release() instead of
>     put_device()
> ---
>  drivers/fpga/fpga-bridge.c | 28 ++++++++++++++--------------
>  1 file changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/fpga/fpga-bridge.c b/drivers/fpga/fpga-bridge.c
> index 6c56afc66a6d..b40f4b4228e7 100644
> --- a/drivers/fpga/fpga-bridge.c
> +++ b/drivers/fpga/fpga-bridge.c
> @@ -340,8 +340,10 @@ struct fpga_bridge *fpga_bridge_create(struct device *parent, const char *name,
>  		return NULL;
>  
>  	id = ida_simple_get(&fpga_bridge_ida, 0, 0, GFP_KERNEL);
> -	if (id < 0)
> -		goto error_kfree;
> +	if (id < 0) {
> +		kfree(bridge);
> +		return NULL;
> +	}
>  
>  	mutex_init(&bridge->mutex);
>  	INIT_LIST_HEAD(&bridge->node);
> @@ -358,17 +360,12 @@ struct fpga_bridge *fpga_bridge_create(struct device *parent, const char *name,
>  	bridge->dev.id = id;
>  
>  	ret = dev_set_name(&bridge->dev, "br%d", id);
> -	if (ret)
> -		goto error_device;
> +	if (ret) {
> +		put_device(&bridge->dev);
> +		return NULL;
> +	}
>  
>  	return bridge;
> -
> -error_device:
> -	ida_simple_remove(&fpga_bridge_ida, id);
> -error_kfree:
> -	kfree(bridge);
> -
> -	return NULL;
>  }
>  EXPORT_SYMBOL_GPL(fpga_bridge_create);
>  
> @@ -378,8 +375,7 @@ EXPORT_SYMBOL_GPL(fpga_bridge_create);
>   */
>  void fpga_bridge_free(struct fpga_bridge *bridge)
>  {
> -	ida_simple_remove(&fpga_bridge_ida, bridge->dev.id);
> -	kfree(bridge);
> +	put_device(&bridge->dev);
>  }
>  EXPORT_SYMBOL_GPL(fpga_bridge_free);
>  
> @@ -469,12 +465,16 @@ void fpga_bridge_unregister(struct fpga_bridge *bridge)
>  	if (bridge->br_ops && bridge->br_ops->fpga_bridge_remove)
>  		bridge->br_ops->fpga_bridge_remove(bridge);
>  
> -	device_unregister(&bridge->dev);
> +	device_del(&bridge->dev);
>  }
>  EXPORT_SYMBOL_GPL(fpga_bridge_unregister);
>  
>  static void fpga_bridge_dev_release(struct device *dev)
>  {
> +	struct fpga_bridge *bridge = to_fpga_bridge(dev);
> +
> +	ida_simple_remove(&fpga_bridge_ida, bridge->dev.id);
> +	kfree(bridge);
>  }
>  
>  static int __init fpga_bridge_dev_init(void)
> -- 
> 2.25.1

[PATCH v2 8/8] fpga: region: Use standard dev_release for class driver

[Russ Weight]

The FPGA region class driver data structure is being treated as a managed
resource instead of using standard dev_release call-back to release the
class data structure. This change populates the class.dev_release function
and changes the fpga_region_free() function to call put_device().
It also changes fpga_region_unregister() to call device_del() instead
of device_unregister().

Signed-off-by: Russ Weight <russell.h.weight@intel.com>
---
v2:
  - Moved the renaming of "dev" to "parent" to a separate patch
  - Call fpga_region_free() in devm_fpga_region_release() instead of put_device()
---
 drivers/fpga/fpga-region.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c
index 563626d66e56..bdc15fab60c0 100644
--- a/drivers/fpga/fpga-region.c
+++ b/drivers/fpga/fpga-region.c
@@ -204,8 +204,10 @@ struct fpga_region
 		return NULL;
 
 	id = ida_simple_get(&fpga_region_ida, 0, 0, GFP_KERNEL);
-	if (id < 0)
-		goto err_free;
+	if (id < 0) {
+		kfree(region);
+		return NULL;
+	}
 
 	region->mgr = mgr;
 	region->get_bridges = get_bridges;
@@ -219,17 +221,12 @@ struct fpga_region
 	region->dev.id = id;
 
 	ret = dev_set_name(&region->dev, "region%d", id);
-	if (ret)
-		goto err_remove;
+	if (ret) {
+		put_device(&region->dev);
+		return NULL;
+	}
 
 	return region;
-
-err_remove:
-	ida_simple_remove(&fpga_region_ida, id);
-err_free:
-	kfree(region);
-
-	return NULL;
 }
 EXPORT_SYMBOL_GPL(fpga_region_create);
 
@@ -239,8 +236,7 @@ EXPORT_SYMBOL_GPL(fpga_region_create);
  */
 void fpga_region_free(struct fpga_region *region)
 {
-	ida_simple_remove(&fpga_region_ida, region->dev.id);
-	kfree(region);
+	put_device(&region->dev);
 }
 EXPORT_SYMBOL_GPL(fpga_region_free);
 
@@ -253,7 +249,7 @@ static void devm_fpga_region_release(struct device *dev, void *res)
 
 /**
  * devm_fpga_region_create - create and initialize a managed FPGA region struct
- * @dev: device parent
+ * @parent: device parent
  * @mgr: manager that programs this region
  * @get_bridges: optional function to get bridges to a list
  *
@@ -310,12 +306,16 @@ EXPORT_SYMBOL_GPL(fpga_region_register);
  */
 void fpga_region_unregister(struct fpga_region *region)
 {
-	device_unregister(&region->dev);
+	device_del(&region->dev);
 }
 EXPORT_SYMBOL_GPL(fpga_region_unregister);
 
 static void fpga_region_dev_release(struct device *dev)
 {
+	struct fpga_region *region = to_fpga_region(dev);
+
+	ida_simple_remove(&fpga_region_ida, region->dev.id);
+	kfree(region);
 }
 
 /**
-- 
2.25.1

Re: [PATCH v2 8/8] fpga: region: Use standard dev_release for class driver

[Xu Yilun]

On Wed, Jun 09, 2021 at 03:11:35PM -0700, Russ Weight wrote:
> The FPGA region class driver data structure is being treated as a managed
> resource instead of using standard dev_release call-back to release the
> class data structure. This change populates the class.dev_release function
> and changes the fpga_region_free() function to call put_device().
> It also changes fpga_region_unregister() to call device_del() instead
> of device_unregister().
> 
> Signed-off-by: Russ Weight <russell.h.weight@intel.com>
> ---
> v2:
>   - Moved the renaming of "dev" to "parent" to a separate patch
>   - Call fpga_region_free() in devm_fpga_region_release() instead of put_device()
> ---
>  drivers/fpga/fpga-region.c | 30 +++++++++++++++---------------
>  1 file changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c
> index 563626d66e56..bdc15fab60c0 100644
> --- a/drivers/fpga/fpga-region.c
> +++ b/drivers/fpga/fpga-region.c
> @@ -204,8 +204,10 @@ struct fpga_region
>  		return NULL;
>  
>  	id = ida_simple_get(&fpga_region_ida, 0, 0, GFP_KERNEL);
> -	if (id < 0)
> -		goto err_free;
> +	if (id < 0) {
> +		kfree(region);
> +		return NULL;
> +	}
>  
>  	region->mgr = mgr;
>  	region->get_bridges = get_bridges;
> @@ -219,17 +221,12 @@ struct fpga_region
>  	region->dev.id = id;
>  
>  	ret = dev_set_name(&region->dev, "region%d", id);
> -	if (ret)
> -		goto err_remove;
> +	if (ret) {
> +		put_device(&region->dev);
> +		return NULL;
> +	}
>  
>  	return region;
> -
> -err_remove:
> -	ida_simple_remove(&fpga_region_ida, id);
> -err_free:
> -	kfree(region);
> -
> -	return NULL;
>  }
>  EXPORT_SYMBOL_GPL(fpga_region_create);
>  
> @@ -239,8 +236,7 @@ EXPORT_SYMBOL_GPL(fpga_region_create);
>   */
>  void fpga_region_free(struct fpga_region *region)
>  {
> -	ida_simple_remove(&fpga_region_ida, region->dev.id);
> -	kfree(region);
> +	put_device(&region->dev);
>  }
>  EXPORT_SYMBOL_GPL(fpga_region_free);
>  
> @@ -253,7 +249,7 @@ static void devm_fpga_region_release(struct device *dev, void *res)
>  
>  /**
>   * devm_fpga_region_create - create and initialize a managed FPGA region struct
> - * @dev: device parent
> + * @parent: device parent

This should be in Patch #5.

Others look good to me. You could append my reviewed-by after this fix.

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

>   * @mgr: manager that programs this region
>   * @get_bridges: optional function to get bridges to a list
>   *
> @@ -310,12 +306,16 @@ EXPORT_SYMBOL_GPL(fpga_region_register);
>   */
>  void fpga_region_unregister(struct fpga_region *region)
>  {
> -	device_unregister(&region->dev);
> +	device_del(&region->dev);
>  }
>  EXPORT_SYMBOL_GPL(fpga_region_unregister);
>  
>  static void fpga_region_dev_release(struct device *dev)
>  {
> +	struct fpga_region *region = to_fpga_region(dev);
> +
> +	ida_simple_remove(&fpga_region_ida, region->dev.id);
> +	kfree(region);
>  }
>  
>  /**
> -- 
> 2.25.1

Re: [PATCH v2 8/8] fpga: region: Use standard dev_release for class driver

[Russ Weight]

On 6/10/21 1:29 AM, Xu Yilun wrote:
> On Wed, Jun 09, 2021 at 03:11:35PM -0700, Russ Weight wrote:
>> The FPGA region class driver data structure is being treated as a managed
>> resource instead of using standard dev_release call-back to release the
>> class data structure. This change populates the class.dev_release function
>> and changes the fpga_region_free() function to call put_device().
>> It also changes fpga_region_unregister() to call device_del() instead
>> of device_unregister().
>>
>> Signed-off-by: Russ Weight <russell.h.weight@intel.com>
>> ---
>> v2:
>>   - Moved the renaming of "dev" to "parent" to a separate patch
>>   - Call fpga_region_free() in devm_fpga_region_release() instead of put_device()
>> ---
>>  drivers/fpga/fpga-region.c | 30 +++++++++++++++---------------
>>  1 file changed, 15 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c
>> index 563626d66e56..bdc15fab60c0 100644
>> --- a/drivers/fpga/fpga-region.c
>> +++ b/drivers/fpga/fpga-region.c
>> @@ -204,8 +204,10 @@ struct fpga_region
>>  		return NULL;
>>  
>>  	id = ida_simple_get(&fpga_region_ida, 0, 0, GFP_KERNEL);
>> -	if (id < 0)
>> -		goto err_free;
>> +	if (id < 0) {
>> +		kfree(region);
>> +		return NULL;
>> +	}
>>  
>>  	region->mgr = mgr;
>>  	region->get_bridges = get_bridges;
>> @@ -219,17 +221,12 @@ struct fpga_region
>>  	region->dev.id = id;
>>  
>>  	ret = dev_set_name(&region->dev, "region%d", id);
>> -	if (ret)
>> -		goto err_remove;
>> +	if (ret) {
>> +		put_device(&region->dev);
>> +		return NULL;
>> +	}
>>  
>>  	return region;
>> -
>> -err_remove:
>> -	ida_simple_remove(&fpga_region_ida, id);
>> -err_free:
>> -	kfree(region);
>> -
>> -	return NULL;
>>  }
>>  EXPORT_SYMBOL_GPL(fpga_region_create);
>>  
>> @@ -239,8 +236,7 @@ EXPORT_SYMBOL_GPL(fpga_region_create);
>>   */
>>  void fpga_region_free(struct fpga_region *region)
>>  {
>> -	ida_simple_remove(&fpga_region_ida, region->dev.id);
>> -	kfree(region);
>> +	put_device(&region->dev);
>>  }
>>  EXPORT_SYMBOL_GPL(fpga_region_free);
>>  
>> @@ -253,7 +249,7 @@ static void devm_fpga_region_release(struct device *dev, void *res)
>>  
>>  /**
>>   * devm_fpga_region_create - create and initialize a managed FPGA region struct
>> - * @dev: device parent
>> + * @parent: device parent
> This should be in Patch #5.
>
> Others look good to me. You could append my reviewed-by after this fix.

Thanks Yilun. I made the fix.

- Russ

>
> Reviewed-by: Xu Yilun <yilun.xu@intel.com>
>
>>   * @mgr: manager that programs this region
>>   * @get_bridges: optional function to get bridges to a list
>>   *
>> @@ -310,12 +306,16 @@ EXPORT_SYMBOL_GPL(fpga_region_register);
>>   */
>>  void fpga_region_unregister(struct fpga_region *region)
>>  {
>> -	device_unregister(&region->dev);
>> +	device_del(&region->dev);
>>  }
>>  EXPORT_SYMBOL_GPL(fpga_region_unregister);
>>  
>>  static void fpga_region_dev_release(struct device *dev)
>>  {
>> +	struct fpga_region *region = to_fpga_region(dev);
>> +
>> +	ida_simple_remove(&fpga_region_ida, region->dev.id);
>> +	kfree(region);
>>  }
>>  
>>  /**
>> -- 
>> 2.25.1