All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v8 0/3] fpga: Use standard class dev_release function
@ 2021-06-24 20:53 Russ Weight
  2021-06-24 20:53 ` [PATCH v8 1/3] fpga: mgr: Use standard dev_release for class driver Russ Weight
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Russ Weight @ 2021-06-24 20:53 UTC (permalink / raw)
  To: mdf, linux-fpga
  Cc: trix, lgoncalv, yilun.xu, hao.wu, matthew.gerlach, richard.gong,
	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.

These changes remove the managed resource functions and populate the class
dev_release callback functions. They also merge the create() and register()
functions into a single register() or register_simple() function for each of
the fpga-mgr, fpga-region, and fpga-bridge class drivers.

The new *register() functions accept an info data structure to provide
flexibility in passing optional parameters. The *register_simple()
functions support the legacy parameter list for users that don't require
the use of optional parameters.

For more context, refer to this email thread:

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

I turned on the configs assocated with each of the modified files, but I
must have been missing some dependencies, because not all of them compiled.
I did a run-time test specifically with the dfl-fme infrastructure. This
would have exercised the region, bridge, and fpga-mgr frameworks.

Changelog v7 -> v8:
  - Added reviewed-by tags.
  - Updated Documentation/driver-api/fpga/ files: fpga-mgr.rst,
    fpga-bridge.rst, and fpga-region.rst.

Changelog v6 -> v7:
  - Update the commit messages to describe the new parameters for the
    *register() functions and to mention the *register_simple() functions.
  - Fix function prototypes in header file to rename dev to parent.
  - Make use of the PTR_ERR_OR_ZERO() macro when possible.
  - Some cleanup of comments.
  - Update function defintions/prototypes to apply const to the new info
    parameter.
  - Verify that info->br_ops is non-null in the fpga_bridge_register()
    function.
  - Verify a non-null info pointer in the fpga_region_register() function.

Changelog v5 -> v6:
  - Moved FPGA manager/bridge/region optional parameters out of the ops
    structure and back into the FPGA class driver structure.
  - Changed fpga_*_register() function parameters to accept an info data
    structure to provide flexibility in passing optional parameters.
  - Added fpga_*_register_simple() functions to support current parameters
    for users that don't require use of optional parameters.

Changelog v4 -> v5:
  - Rebased on top of recently accepted patches.
  - Removed compat_id from the fpga_mgr_register() parameter list
    and added it to the fpga_manager_ops structure. This also required
    dynamically allocating the dfl-fme-ops structure in order to add
    the appropriate compat_id.
  - Created the fpga_region_ops data structure which is optionally passed
    to fpga_region_register(). compat_id, the get_bridges() pointer, and
    the priv pointer are included in the fpga_region_ops structure.

Changelog v3 -> v4:
  - Added the compat_id parameter to fpga_mgr_register() and
    devm_fpga_mgr_register() to ensure that the compat_id is set before
    the device_register() call.
  - Added the compat_id parameter to fpga_region_register() to ensure
    that the compat_id is set before the device_register() call.
  - Modified the dfl_fpga_feature_devs_enumerate() function to restore
    the fpga_region_register() call to the correct location.

Changelog v2 -> v3:
  - Cleaned up comment headers for fpga_mgr_register(), fpga_bridge_register(),
    and fpga_region_register().
  - Fixed error return on ida_simple_get() failure for fpga_mgr_register(),
    fpga_bridge_register(), and fpga_region_register().
  - Fixed error return value for fpga_bridge_register(): ERR_PTR(ret) instead
    of NULL.

Changelog v1 -> v2:
  - Restored devm_fpga_mgr_register() functionality to the fpga-mgr
    class driver, adapted for the combined create/register functionality.
  - All previous callers of devm_fpga_mgr_register() will continue to call
    devm_fpga_mgr_register().
  - replaced unnecessary ternary operators in return statements with
    standard if conditions.

Russ Weight (3):
  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

 Documentation/driver-api/fpga/fpga-bridge.rst |   8 +-
 Documentation/driver-api/fpga/fpga-mgr.rst    |  39 ++--
 Documentation/driver-api/fpga/fpga-region.rst |   8 +-
 drivers/fpga/altera-cvp.c                     |  12 +-
 drivers/fpga/altera-fpga2sdram.c              |  12 +-
 drivers/fpga/altera-freeze-bridge.c           |  10 +-
 drivers/fpga/altera-hps2fpga.c                |  12 +-
 drivers/fpga/altera-pr-ip-core.c              |   8 +-
 drivers/fpga/altera-ps-spi.c                  |   9 +-
 drivers/fpga/dfl-fme-br.c                     |  10 +-
 drivers/fpga/dfl-fme-mgr.c                    |  22 +-
 drivers/fpga/dfl-fme-region.c                 |  17 +-
 drivers/fpga/dfl.c                            |  12 +-
 drivers/fpga/fpga-bridge.c                    | 138 ++++-------
 drivers/fpga/fpga-mgr.c                       | 214 ++++++++----------
 drivers/fpga/fpga-region.c                    | 121 ++++------
 drivers/fpga/ice40-spi.c                      |   9 +-
 drivers/fpga/machxo2-spi.c                    |  10 +-
 drivers/fpga/of-fpga-region.c                 |  11 +-
 drivers/fpga/socfpga-a10.c                    |  16 +-
 drivers/fpga/socfpga.c                        |   9 +-
 drivers/fpga/stratix10-soc.c                  |  16 +-
 drivers/fpga/ts73xx-fpga.c                    |   9 +-
 drivers/fpga/xilinx-pr-decoupler.c            |  17 +-
 drivers/fpga/xilinx-spi.c                     |  11 +-
 drivers/fpga/zynq-fpga.c                      |  16 +-
 drivers/fpga/zynqmp-fpga.c                    |   9 +-
 include/linux/fpga/fpga-bridge.h              |  33 ++-
 include/linux/fpga/fpga-mgr.h                 |  62 +++--
 include/linux/fpga/fpga-region.h              |  36 ++-
 30 files changed, 396 insertions(+), 520 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v8 1/3] fpga: mgr: Use standard dev_release for class driver
  2021-06-24 20:53 [PATCH v8 0/3] fpga: Use standard class dev_release function Russ Weight
@ 2021-06-24 20:53 ` Russ Weight
  2021-06-28  3:49   ` Wu, Hao
  2021-06-28  4:39   ` Xu Yilun
  2021-06-24 20:53 ` [PATCH v8 2/3] fpga: bridge: " Russ Weight
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 10+ messages in thread
From: Russ Weight @ 2021-06-24 20:53 UTC (permalink / raw)
  To: mdf, linux-fpga
  Cc: trix, lgoncalv, yilun.xu, hao.wu, matthew.gerlach, richard.gong,
	Russ Weight

The FPGA manager class driver data structure is being treated as a
managed resource instead of using the standard dev_release call-back
function to release the class data structure. This change removes
the managed resource code for the freeing of the class data structure
and combines the create() and register() functions into a single
register() or register_simple() function.

The register() function accepts an info data structure to provide
flexibility in passing optional parameters. The register_simple()
function supports the current parameter list for users that don't
require the use of optional parameters.

The devm_fpga_mgr_register() function is retained, and the
devm_fpga_mgr_register_simple() function is added.

Signed-off-by: Russ Weight <russell.h.weight@intel.com>
Reviewed-by: Xu Yilun <yilun.xu@intel.com>
---
v8:
  - Added reviewed-by tag.
  - Updated Documentation/driver-api/fpga/fpga-mgr.rst documentation.
v7:
  - Update the commit message to describe the new parameters for the
    *fpga_mgr_register() functions and to mention the
    *fpga_mgr_register_simple() functions.
  - Fix function prototypes in header file to rename dev to parent.
  - Make use of the PTR_ERR_OR_ZERO() macro when possible.
  - Some cleanup of comments.
  - Update function defintions/prototypes to apply const to the new info
    parameter.
v6:
  - Moved FPGA manager optional parameters out of the ops structure and
    back into the FPGA manager structure.
  - Changed fpga_mgr_register()/devm_fpga_mgr_register() parameters to
    accept an info data structure to provide flexibility in passing optional
    parameters.
  - Added fpga_mgr_register_simple()/devm_fpga_mgr_register_simple()
    functions to support current parameters for users that don't require
    the use of optional parameters.
v5:
  - Rebased on top of recently accepted patches.
  - Removed compat_id from the fpga_mgr_register() parameter list
    and added it to the fpga_manager_ops structure. This also required
    dynamically allocating the dfl-fme-ops structure in order to add
    the appropriate compat_id.
v4:
  - Added the compat_id parameter to fpga_mgr_register() and
    devm_fpga_mgr_register() to ensure that the compat_id is set before
    the device_register() call.
v3:
  - Cleaned up comment header for fpga_mgr_register()
  - Fix error return on ida_simple_get() failure
v2:
  - Restored devm_fpga_mgr_register() functionality, adapted for the combined
    create/register functionality.
  - All previous callers of devm_fpga_mgr_register() will continue to call
    devm_fpga_mgr_register().
  - replaced unnecessary ternary operators in return statements with standard
    if conditions.
---
 Documentation/driver-api/fpga/fpga-mgr.rst |  39 ++--
 drivers/fpga/altera-cvp.c                  |  12 +-
 drivers/fpga/altera-pr-ip-core.c           |   8 +-
 drivers/fpga/altera-ps-spi.c               |   9 +-
 drivers/fpga/dfl-fme-mgr.c                 |  22 +--
 drivers/fpga/fpga-mgr.c                    | 214 +++++++++------------
 drivers/fpga/ice40-spi.c                   |   9 +-
 drivers/fpga/machxo2-spi.c                 |  10 +-
 drivers/fpga/socfpga-a10.c                 |  16 +-
 drivers/fpga/socfpga.c                     |   9 +-
 drivers/fpga/stratix10-soc.c               |  16 +-
 drivers/fpga/ts73xx-fpga.c                 |   9 +-
 drivers/fpga/xilinx-spi.c                  |  11 +-
 drivers/fpga/zynq-fpga.c                   |  16 +-
 drivers/fpga/zynqmp-fpga.c                 |   9 +-
 include/linux/fpga/fpga-mgr.h              |  62 ++++--
 16 files changed, 204 insertions(+), 267 deletions(-)

diff --git a/Documentation/driver-api/fpga/fpga-mgr.rst b/Documentation/driver-api/fpga/fpga-mgr.rst
index 4d926b452cb3..fbb7f07cae9f 100644
--- a/Documentation/driver-api/fpga/fpga-mgr.rst
+++ b/Documentation/driver-api/fpga/fpga-mgr.rst
@@ -24,7 +24,8 @@ How to support a new FPGA device
 --------------------------------
 
 To add another FPGA manager, write a driver that implements a set of ops.  The
-probe function calls fpga_mgr_register(), such as::
+probe function calls devm_fpga_mgr_register() or
+devm_fpga_mgr_register_simple(), such as::
 
 	static const struct fpga_manager_ops socfpga_fpga_ops = {
 		.write_init = socfpga_fpga_ops_configure_init,
@@ -49,23 +50,9 @@ probe function calls fpga_mgr_register(), such as::
 		 * them in priv
 		 */
 
-		mgr = devm_fpga_mgr_create(dev, "Altera SOCFPGA FPGA Manager",
-					   &socfpga_fpga_ops, priv);
-		if (!mgr)
-			return -ENOMEM;
-
-		platform_set_drvdata(pdev, mgr);
-
-		return fpga_mgr_register(mgr);
-	}
-
-	static int socfpga_fpga_remove(struct platform_device *pdev)
-	{
-		struct fpga_manager *mgr = platform_get_drvdata(pdev);
-
-		fpga_mgr_unregister(mgr);
-
-		return 0;
+		mgr = devm_fpga_mgr_register_simple(dev, "Altera SOCFPGA FPGA Manager",
+						    &socfpga_fpga_ops, priv);
+		return PTR_ERR_OR_ZERO(mgr);
 	}
 
 
@@ -104,8 +91,10 @@ API for implementing a new FPGA Manager driver
 * ``fpga_mgr_states`` -  Values for :c:expr:`fpga_manager->state`.
 * struct fpga_manager -  the FPGA manager struct
 * struct fpga_manager_ops -  Low level FPGA manager driver ops
-* devm_fpga_mgr_create() -  Allocate and init a manager struct
-* fpga_mgr_register() -  Register an FPGA manager
+* fpga_mgr_register() -  Create and register an FPGA manager
+* fpga_mgr_register_simple() -  Create and register an FPGA manager
+* devm_fpga_mgr_register() -  Create and register an FPGA manager
+* devm_fpga_mgr_register_simple() -  Create and register an FPGA manager
 * fpga_mgr_unregister() -  Unregister an FPGA manager
 
 .. kernel-doc:: include/linux/fpga/fpga-mgr.h
@@ -118,10 +107,16 @@ API for implementing a new FPGA Manager driver
    :functions: fpga_manager_ops
 
 .. kernel-doc:: drivers/fpga/fpga-mgr.c
-   :functions: devm_fpga_mgr_create
+   :functions: fpga_mgr_register
 
 .. kernel-doc:: drivers/fpga/fpga-mgr.c
-   :functions: fpga_mgr_register
+   :functions: fpga_mgr_register_simple
+
+.. kernel-doc:: drivers/fpga/fpga-mgr.c
+   :functions: devm_fpga_mgr_register
+
+.. kernel-doc:: drivers/fpga/fpga-mgr.c
+   :functions: devm_fpga_mgr_register_simple
 
 .. kernel-doc:: drivers/fpga/fpga-mgr.c
    :functions: fpga_mgr_unregister
diff --git a/drivers/fpga/altera-cvp.c b/drivers/fpga/altera-cvp.c
index ccf4546eff29..ddf2ffe3f138 100644
--- a/drivers/fpga/altera-cvp.c
+++ b/drivers/fpga/altera-cvp.c
@@ -652,19 +652,15 @@ static int altera_cvp_probe(struct pci_dev *pdev,
 	snprintf(conf->mgr_name, sizeof(conf->mgr_name), "%s @%s",
 		 ALTERA_CVP_MGR_NAME, pci_name(pdev));
 
-	mgr = devm_fpga_mgr_create(&pdev->dev, conf->mgr_name,
-				   &altera_cvp_ops, conf);
-	if (!mgr) {
-		ret = -ENOMEM;
+	mgr = fpga_mgr_register_simple(&pdev->dev, conf->mgr_name,
+				       &altera_cvp_ops, conf);
+	if (IS_ERR(mgr)) {
+		ret = PTR_ERR(mgr);
 		goto err_unmap;
 	}
 
 	pci_set_drvdata(pdev, mgr);
 
-	ret = fpga_mgr_register(mgr);
-	if (ret)
-		goto err_unmap;
-
 	return 0;
 
 err_unmap:
diff --git a/drivers/fpga/altera-pr-ip-core.c b/drivers/fpga/altera-pr-ip-core.c
index dfdf21ed34c4..e0baf3392b67 100644
--- a/drivers/fpga/altera-pr-ip-core.c
+++ b/drivers/fpga/altera-pr-ip-core.c
@@ -191,11 +191,9 @@ int alt_pr_register(struct device *dev, void __iomem *reg_base)
 		(val & ALT_PR_CSR_STATUS_MSK) >> ALT_PR_CSR_STATUS_SFT,
 		(int)(val & ALT_PR_CSR_PR_START));
 
-	mgr = devm_fpga_mgr_create(dev, dev_name(dev), &alt_pr_ops, priv);
-	if (!mgr)
-		return -ENOMEM;
-
-	return devm_fpga_mgr_register(dev, mgr);
+	mgr = devm_fpga_mgr_register_simple(dev, dev_name(dev),
+					    &alt_pr_ops, priv);
+	return PTR_ERR_OR_ZERO(mgr);
 }
 EXPORT_SYMBOL_GPL(alt_pr_register);
 
diff --git a/drivers/fpga/altera-ps-spi.c b/drivers/fpga/altera-ps-spi.c
index 23bfd4d1ad0f..f544de2a9f6b 100644
--- a/drivers/fpga/altera-ps-spi.c
+++ b/drivers/fpga/altera-ps-spi.c
@@ -302,12 +302,9 @@ static int altera_ps_probe(struct spi_device *spi)
 	snprintf(conf->mgr_name, sizeof(conf->mgr_name), "%s %s",
 		 dev_driver_string(&spi->dev), dev_name(&spi->dev));
 
-	mgr = devm_fpga_mgr_create(&spi->dev, conf->mgr_name,
-				   &altera_ps_ops, conf);
-	if (!mgr)
-		return -ENOMEM;
-
-	return devm_fpga_mgr_register(&spi->dev, mgr);
+	mgr = devm_fpga_mgr_register_simple(&spi->dev, conf->mgr_name,
+					    &altera_ps_ops, conf);
+	return PTR_ERR_OR_ZERO(mgr);
 }
 
 static const struct spi_device_id altera_ps_spi_ids[] = {
diff --git a/drivers/fpga/dfl-fme-mgr.c b/drivers/fpga/dfl-fme-mgr.c
index d5861d13b306..20b1cf3d957f 100644
--- a/drivers/fpga/dfl-fme-mgr.c
+++ b/drivers/fpga/dfl-fme-mgr.c
@@ -282,7 +282,7 @@ static void fme_mgr_get_compat_id(void __iomem *fme_pr,
 static int fme_mgr_probe(struct platform_device *pdev)
 {
 	struct dfl_fme_mgr_pdata *pdata = dev_get_platdata(&pdev->dev);
-	struct fpga_compat_id *compat_id;
+	struct fpga_manager_info info = { 0 };
 	struct device *dev = &pdev->dev;
 	struct fme_mgr_priv *priv;
 	struct fpga_manager *mgr;
@@ -302,20 +302,16 @@ static int fme_mgr_probe(struct platform_device *pdev)
 			return PTR_ERR(priv->ioaddr);
 	}
 
-	compat_id = devm_kzalloc(dev, sizeof(*compat_id), GFP_KERNEL);
-	if (!compat_id)
+	info.name = "DFL FME FPGA Manager";
+	info.mops = &fme_mgr_ops;
+	info.priv = priv;
+	info.compat_id = devm_kzalloc(dev, sizeof(*info.compat_id), GFP_KERNEL);
+	if (!info.compat_id)
 		return -ENOMEM;
 
-	fme_mgr_get_compat_id(priv->ioaddr, compat_id);
-
-	mgr = devm_fpga_mgr_create(dev, "DFL FME FPGA Manager",
-				   &fme_mgr_ops, priv);
-	if (!mgr)
-		return -ENOMEM;
-
-	mgr->compat_id = compat_id;
-
-	return devm_fpga_mgr_register(dev, mgr);
+	fme_mgr_get_compat_id(priv->ioaddr, info.compat_id);
+	mgr = devm_fpga_mgr_register(dev, &info);
+	return PTR_ERR_OR_ZERO(mgr);
 }
 
 static struct platform_driver fme_mgr_driver = {
diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c
index ecb4c3c795fa..6bd65a1b84fb 100644
--- a/drivers/fpga/fpga-mgr.c
+++ b/drivers/fpga/fpga-mgr.c
@@ -550,21 +550,19 @@ void fpga_mgr_unlock(struct fpga_manager *mgr)
 EXPORT_SYMBOL_GPL(fpga_mgr_unlock);
 
 /**
- * fpga_mgr_create - create and initialize an FPGA manager struct
+ * fpga_mgr_register - create and register an FPGA Manager device
  * @parent:	fpga manager device from pdev
- * @name:	fpga manager name
- * @mops:	pointer to structure of fpga manager ops
- * @priv:	fpga manager private data
+ * @info:	parameters for fpga manager
  *
- * The caller of this function is responsible for freeing the struct with
- * fpga_mgr_free().  Using devm_fpga_mgr_create() instead is recommended.
+ * The caller of this function is responsible for calling fpga_mgr_unregister().
+ * Using devm_fpga_mgr_register() instead is recommended.
  *
- * Return: pointer to struct fpga_manager or NULL
+ * Return: pointer to struct fpga_manager pointer or ERR_PTR()
  */
-struct fpga_manager *fpga_mgr_create(struct device *parent, const char *name,
-				     const struct fpga_manager_ops *mops,
-				     void *priv)
+struct fpga_manager *
+fpga_mgr_register(struct device *parent, const struct fpga_manager_info *info)
 {
+	const struct fpga_manager_ops *mops = info->mops;
 	struct fpga_manager *mgr;
 	int id, ret;
 
@@ -572,29 +570,31 @@ struct fpga_manager *fpga_mgr_create(struct device *parent, const char *name,
 	    !mops->write_init || (!mops->write && !mops->write_sg) ||
 	    (mops->write && mops->write_sg)) {
 		dev_err(parent, "Attempt to register without fpga_manager_ops\n");
-		return NULL;
+		return ERR_PTR(-EINVAL);
 	}
 
-	if (!name || !strlen(name)) {
+	if (!info->name || !strlen(info->name)) {
 		dev_err(parent, "Attempt to register with no name!\n");
-		return NULL;
+		return ERR_PTR(-EINVAL);
 	}
 
 	mgr = kzalloc(sizeof(*mgr), GFP_KERNEL);
 	if (!mgr)
-		return NULL;
+		return ERR_PTR(-ENOMEM);
 
 	id = ida_simple_get(&fpga_mgr_ida, 0, 0, GFP_KERNEL);
-	if (id < 0)
+	if (id < 0) {
+		ret = id;
 		goto error_kfree;
+	}
 
 	mutex_init(&mgr->ref_mutex);
 
-	mgr->name = name;
-	mgr->mops = mops;
-	mgr->priv = priv;
+	mgr->name = info->name;
+	mgr->mops = info->mops;
+	mgr->priv = info->priv;
+	mgr->compat_id = info->compat_id;
 
-	device_initialize(&mgr->dev);
 	mgr->dev.class = fpga_mgr_class;
 	mgr->dev.groups = mops->groups;
 	mgr->dev.parent = parent;
@@ -605,6 +605,19 @@ struct fpga_manager *fpga_mgr_create(struct device *parent, const char *name,
 	if (ret)
 		goto error_device;
 
+	/*
+	 * Initialize framework state by requesting low level driver read state
+	 * from device.  FPGA may be in reset mode or may have been programmed
+	 * by bootloader or EEPROM.
+	 */
+	mgr->state = mgr->mops->state(mgr);
+
+	ret = device_register(&mgr->dev);
+	if (ret) {
+		put_device(&mgr->dev);
+		return ERR_PTR(ret);
+	}
+
 	return mgr;
 
 error_device:
@@ -612,98 +625,38 @@ struct fpga_manager *fpga_mgr_create(struct device *parent, const char *name,
 error_kfree:
 	kfree(mgr);
 
-	return NULL;
-}
-EXPORT_SYMBOL_GPL(fpga_mgr_create);
-
-/**
- * fpga_mgr_free - free an FPGA manager created with fpga_mgr_create()
- * @mgr:	fpga manager struct
- */
-void fpga_mgr_free(struct fpga_manager *mgr)
-{
-	ida_simple_remove(&fpga_mgr_ida, mgr->dev.id);
-	kfree(mgr);
-}
-EXPORT_SYMBOL_GPL(fpga_mgr_free);
-
-static void devm_fpga_mgr_release(struct device *dev, void *res)
-{
-	struct fpga_mgr_devres *dr = res;
-
-	fpga_mgr_free(dr->mgr);
+	return ERR_PTR(ret);
 }
+EXPORT_SYMBOL_GPL(fpga_mgr_register);
 
 /**
- * devm_fpga_mgr_create - create and initialize a managed FPGA manager struct
+ * fpga_mgr_register_simple - create and register an FPGA Manager device
  * @parent:	fpga manager device from pdev
  * @name:	fpga manager name
  * @mops:	pointer to structure of fpga manager ops
  * @priv:	fpga manager private data
  *
- * This function is intended for use in an FPGA manager driver's probe function.
- * After the manager driver creates the manager struct with
- * devm_fpga_mgr_create(), it should register it with fpga_mgr_register().  The
- * manager driver's remove function should call fpga_mgr_unregister().  The
- * manager struct allocated with this function will be freed automatically on
- * driver detach.  This includes the case of a probe function returning error
- * before calling fpga_mgr_register(), the struct will still get cleaned up.
+ * The caller of this function is responsible for calling fpga_mgr_unregister().
+ * Using devm_fpga_mgr_register_simple() instead is recommended. This simple
+ * version of the register function should be sufficient for most users. The
+ * fpga_mgr_register() function is available for users that need to pass
+ * additional, optional parameters.
  *
- * Return: pointer to struct fpga_manager or NULL
+ * Return: pointer to struct fpga_manager pointer or ERR_PTR()
  */
-struct fpga_manager *devm_fpga_mgr_create(struct device *parent, const char *name,
-					  const struct fpga_manager_ops *mops,
-					  void *priv)
+struct fpga_manager *
+fpga_mgr_register_simple(struct device *parent, const char *name,
+			 const struct fpga_manager_ops *mops, void *priv)
 {
-	struct fpga_mgr_devres *dr;
+	struct fpga_manager_info info = { 0 };
 
-	dr = devres_alloc(devm_fpga_mgr_release, sizeof(*dr), GFP_KERNEL);
-	if (!dr)
-		return NULL;
-
-	dr->mgr = fpga_mgr_create(parent, name, mops, priv);
-	if (!dr->mgr) {
-		devres_free(dr);
-		return NULL;
-	}
-
-	devres_add(parent, dr);
+	info.name = name;
+	info.mops = mops;
+	info.priv = priv;
 
-	return dr->mgr;
+	return fpga_mgr_register(parent, &info);
 }
-EXPORT_SYMBOL_GPL(devm_fpga_mgr_create);
-
-/**
- * fpga_mgr_register - register an FPGA manager
- * @mgr: fpga manager struct
- *
- * Return: 0 on success, negative error code otherwise.
- */
-int fpga_mgr_register(struct fpga_manager *mgr)
-{
-	int ret;
-
-	/*
-	 * Initialize framework state by requesting low level driver read state
-	 * from device.  FPGA may be in reset mode or may have been programmed
-	 * by bootloader or EEPROM.
-	 */
-	mgr->state = mgr->mops->state(mgr);
-
-	ret = device_add(&mgr->dev);
-	if (ret)
-		goto error_device;
-
-	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);
+EXPORT_SYMBOL_GPL(fpga_mgr_register_simple);
 
 /**
  * fpga_mgr_unregister - unregister an FPGA manager
@@ -726,14 +679,6 @@ void fpga_mgr_unregister(struct fpga_manager *mgr)
 }
 EXPORT_SYMBOL_GPL(fpga_mgr_unregister);
 
-static int fpga_mgr_devres_match(struct device *dev, void *res,
-				 void *match_data)
-{
-	struct fpga_mgr_devres *dr = res;
-
-	return match_data == dr->mgr;
-}
-
 static void devm_fpga_mgr_unregister(struct device *dev, void *res)
 {
 	struct fpga_mgr_devres *dr = res;
@@ -743,44 +688,67 @@ static void devm_fpga_mgr_unregister(struct device *dev, void *res)
 
 /**
  * devm_fpga_mgr_register - resource managed variant of fpga_mgr_register()
- * @dev: managing device for this FPGA manager
- * @mgr: fpga manager struct
+ * @parent:	fpga manager device from pdev
+ * @info:	parameters for fpga manager
  *
  * This is the devres variant of fpga_mgr_register() for which the unregister
  * function will be called automatically when the managing device is detached.
  */
-int devm_fpga_mgr_register(struct device *dev, struct fpga_manager *mgr)
+struct fpga_manager *
+devm_fpga_mgr_register(struct device *parent, const struct fpga_manager_info *info)
 {
 	struct fpga_mgr_devres *dr;
-	int ret;
-
-	/*
-	 * Make sure that the struct fpga_manager * that is passed in is
-	 * managed itself.
-	 */
-	if (WARN_ON(!devres_find(dev, devm_fpga_mgr_release,
-				 fpga_mgr_devres_match, mgr)))
-		return -EINVAL;
+	struct fpga_manager *mgr;
 
 	dr = devres_alloc(devm_fpga_mgr_unregister, sizeof(*dr), GFP_KERNEL);
 	if (!dr)
-		return -ENOMEM;
+		return ERR_PTR(-ENOMEM);
 
-	ret = fpga_mgr_register(mgr);
-	if (ret) {
+	mgr = fpga_mgr_register(parent, info);
+	if (IS_ERR(mgr)) {
 		devres_free(dr);
-		return ret;
+		return mgr;
 	}
 
 	dr->mgr = mgr;
-	devres_add(dev, dr);
+	devres_add(parent, dr);
 
-	return 0;
+	return mgr;
 }
 EXPORT_SYMBOL_GPL(devm_fpga_mgr_register);
 
+/**
+ * devm_fpga_mgr_register_simple - resource managed variant of
+ *				   fpga_mgr_register_simple()
+ * @parent:	fpga manager device from pdev
+ * @name:	fpga manager name
+ * @mops:	pointer to structure of fpga manager ops
+ * @priv:	fpga manager private data
+ *
+ * This is the devres variant of fpga_mgr_register_simple() for which the
+ * unregister function will be called automatically when the managing
+ * device is detached.
+ */
+struct fpga_manager *
+devm_fpga_mgr_register_simple(struct device *parent, const char *name,
+			      const struct fpga_manager_ops *mops, void *priv)
+{
+	struct fpga_manager_info info = { 0 };
+
+	info.name = name;
+	info.mops = mops;
+	info.priv = priv;
+
+	return devm_fpga_mgr_register(parent, &info);
+}
+EXPORT_SYMBOL_GPL(devm_fpga_mgr_register_simple);
+
 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)
diff --git a/drivers/fpga/ice40-spi.c b/drivers/fpga/ice40-spi.c
index 69dec5af23c3..448eb6f4e509 100644
--- a/drivers/fpga/ice40-spi.c
+++ b/drivers/fpga/ice40-spi.c
@@ -178,12 +178,9 @@ static int ice40_fpga_probe(struct spi_device *spi)
 		return ret;
 	}
 
-	mgr = devm_fpga_mgr_create(dev, "Lattice iCE40 FPGA Manager",
-				   &ice40_fpga_ops, priv);
-	if (!mgr)
-		return -ENOMEM;
-
-	return devm_fpga_mgr_register(dev, mgr);
+	mgr = devm_fpga_mgr_register_simple(dev, "Lattice iCE40 FPGA Manager",
+					    &ice40_fpga_ops, priv);
+	return PTR_ERR_OR_ZERO(mgr);
 }
 
 static const struct of_device_id ice40_fpga_of_match[] = {
diff --git a/drivers/fpga/machxo2-spi.c b/drivers/fpga/machxo2-spi.c
index 114a64d2b7a4..77be11155402 100644
--- a/drivers/fpga/machxo2-spi.c
+++ b/drivers/fpga/machxo2-spi.c
@@ -366,12 +366,10 @@ static int machxo2_spi_probe(struct spi_device *spi)
 		return -EINVAL;
 	}
 
-	mgr = devm_fpga_mgr_create(dev, "Lattice MachXO2 SPI FPGA Manager",
-				   &machxo2_ops, spi);
-	if (!mgr)
-		return -ENOMEM;
-
-	return devm_fpga_mgr_register(dev, mgr);
+	mgr = devm_fpga_mgr_register_simple(dev,
+					    "Lattice MachXO2 SPI FPGA Manager",
+					    &machxo2_ops, spi);
+	return PTR_ERR_OR_ZERO(mgr);
 }
 
 static const struct of_device_id of_match[] = {
diff --git a/drivers/fpga/socfpga-a10.c b/drivers/fpga/socfpga-a10.c
index 573d88bdf730..5ffefaa3eb07 100644
--- a/drivers/fpga/socfpga-a10.c
+++ b/drivers/fpga/socfpga-a10.c
@@ -508,19 +508,15 @@ static int socfpga_a10_fpga_probe(struct platform_device *pdev)
 		return -EBUSY;
 	}
 
-	mgr = devm_fpga_mgr_create(dev, "SoCFPGA Arria10 FPGA Manager",
-				   &socfpga_a10_fpga_mgr_ops, priv);
-	if (!mgr)
-		return -ENOMEM;
-
-	platform_set_drvdata(pdev, mgr);
-
-	ret = fpga_mgr_register(mgr);
-	if (ret) {
+	mgr = fpga_mgr_register_simple(dev, "SoCFPGA Arria10 FPGA Manager",
+				       &socfpga_a10_fpga_mgr_ops, priv);
+	if (IS_ERR(mgr)) {
 		clk_disable_unprepare(priv->clk);
-		return ret;
+		return PTR_ERR(mgr);
 	}
 
+	platform_set_drvdata(pdev, mgr);
+
 	return 0;
 }
 
diff --git a/drivers/fpga/socfpga.c b/drivers/fpga/socfpga.c
index 1f467173fc1f..9280db4364d4 100644
--- a/drivers/fpga/socfpga.c
+++ b/drivers/fpga/socfpga.c
@@ -571,12 +571,9 @@ static int socfpga_fpga_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	mgr = devm_fpga_mgr_create(dev, "Altera SOCFPGA FPGA Manager",
-				   &socfpga_fpga_ops, priv);
-	if (!mgr)
-		return -ENOMEM;
-
-	return devm_fpga_mgr_register(dev, mgr);
+	mgr = devm_fpga_mgr_register_simple(dev, "Altera SOCFPGA FPGA Manager",
+					    &socfpga_fpga_ops, priv);
+	return PTR_ERR_OR_ZERO(mgr);
 }
 
 #ifdef CONFIG_OF
diff --git a/drivers/fpga/stratix10-soc.c b/drivers/fpga/stratix10-soc.c
index a2cea500f7cc..fb84d88d4ce9 100644
--- a/drivers/fpga/stratix10-soc.c
+++ b/drivers/fpga/stratix10-soc.c
@@ -425,18 +425,11 @@ static int s10_probe(struct platform_device *pdev)
 
 	init_completion(&priv->status_return_completion);
 
-	mgr = fpga_mgr_create(dev, "Stratix10 SOC FPGA Manager",
-			      &s10_ops, priv);
-	if (!mgr) {
-		dev_err(dev, "unable to create FPGA manager\n");
-		ret = -ENOMEM;
-		goto probe_err;
-	}
-
-	ret = fpga_mgr_register(mgr);
-	if (ret) {
+	mgr = fpga_mgr_register_simple(dev, "Stratix10 SOC FPGA Manager",
+				       &s10_ops, priv);
+	if (IS_ERR(mgr)) {
 		dev_err(dev, "unable to register FPGA manager\n");
-		fpga_mgr_free(mgr);
+		ret = PTR_ERR(mgr);
 		goto probe_err;
 	}
 
@@ -454,7 +447,6 @@ 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;
diff --git a/drivers/fpga/ts73xx-fpga.c b/drivers/fpga/ts73xx-fpga.c
index 101f016c6ed8..009d5cfa1074 100644
--- a/drivers/fpga/ts73xx-fpga.c
+++ b/drivers/fpga/ts73xx-fpga.c
@@ -122,12 +122,9 @@ static int ts73xx_fpga_probe(struct platform_device *pdev)
 	if (IS_ERR(priv->io_base))
 		return PTR_ERR(priv->io_base);
 
-	mgr = devm_fpga_mgr_create(kdev, "TS-73xx FPGA Manager",
-				   &ts73xx_fpga_ops, priv);
-	if (!mgr)
-		return -ENOMEM;
-
-	return devm_fpga_mgr_register(kdev, mgr);
+	mgr = devm_fpga_mgr_register_simple(kdev, "TS-73xx FPGA Manager",
+					    &ts73xx_fpga_ops, priv);
+	return PTR_ERR_OR_ZERO(mgr);
 }
 
 static struct platform_driver ts73xx_fpga_driver = {
diff --git a/drivers/fpga/xilinx-spi.c b/drivers/fpga/xilinx-spi.c
index fee4d0abf6bf..6f568a757cf2 100644
--- a/drivers/fpga/xilinx-spi.c
+++ b/drivers/fpga/xilinx-spi.c
@@ -247,13 +247,10 @@ static int xilinx_spi_probe(struct spi_device *spi)
 		return dev_err_probe(&spi->dev, PTR_ERR(conf->done),
 				     "Failed to get DONE gpio\n");
 
-	mgr = devm_fpga_mgr_create(&spi->dev,
-				   "Xilinx Slave Serial FPGA Manager",
-				   &xilinx_spi_ops, conf);
-	if (!mgr)
-		return -ENOMEM;
-
-	return devm_fpga_mgr_register(&spi->dev, mgr);
+	mgr = devm_fpga_mgr_register_simple(&spi->dev,
+					    "Xilinx Slave Serial FPGA Manager",
+					    &xilinx_spi_ops, conf);
+	return PTR_ERR_OR_ZERO(mgr);
 }
 
 static const struct of_device_id xlnx_spi_of_match[] = {
diff --git a/drivers/fpga/zynq-fpga.c b/drivers/fpga/zynq-fpga.c
index 9b75bd4f93d8..a3de365aadc6 100644
--- a/drivers/fpga/zynq-fpga.c
+++ b/drivers/fpga/zynq-fpga.c
@@ -609,20 +609,16 @@ static int zynq_fpga_probe(struct platform_device *pdev)
 
 	clk_disable(priv->clk);
 
-	mgr = devm_fpga_mgr_create(dev, "Xilinx Zynq FPGA Manager",
-				   &zynq_fpga_ops, priv);
-	if (!mgr)
-		return -ENOMEM;
-
-	platform_set_drvdata(pdev, mgr);
-
-	err = fpga_mgr_register(mgr);
-	if (err) {
+	mgr = fpga_mgr_register_simple(dev, "Xilinx Zynq FPGA Manager",
+				       &zynq_fpga_ops, priv);
+	if (IS_ERR(mgr)) {
 		dev_err(dev, "unable to register FPGA manager\n");
 		clk_unprepare(priv->clk);
-		return err;
+		return PTR_ERR(mgr);
 	}
 
+	platform_set_drvdata(pdev, mgr);
+
 	return 0;
 }
 
diff --git a/drivers/fpga/zynqmp-fpga.c b/drivers/fpga/zynqmp-fpga.c
index 125743c9797f..53a7b36f61e8 100644
--- a/drivers/fpga/zynqmp-fpga.c
+++ b/drivers/fpga/zynqmp-fpga.c
@@ -102,12 +102,9 @@ static int zynqmp_fpga_probe(struct platform_device *pdev)
 
 	priv->dev = dev;
 
-	mgr = devm_fpga_mgr_create(dev, "Xilinx ZynqMP FPGA Manager",
-				   &zynqmp_fpga_ops, priv);
-	if (!mgr)
-		return -ENOMEM;
-
-	return devm_fpga_mgr_register(dev, mgr);
+	mgr = devm_fpga_mgr_register_simple(dev, "Xilinx ZynqMP FPGA Manager",
+					    &zynqmp_fpga_ops, priv);
+	return PTR_ERR_OR_ZERO(mgr);
 }
 
 static const struct of_device_id zynqmp_fpga_of_match[] = {
diff --git a/include/linux/fpga/fpga-mgr.h b/include/linux/fpga/fpga-mgr.h
index 474c1f506307..9efe55be93bc 100644
--- a/include/linux/fpga/fpga-mgr.h
+++ b/include/linux/fpga/fpga-mgr.h
@@ -105,6 +105,36 @@ struct fpga_image_info {
 #endif
 };
 
+/**
+ * struct fpga_compat_id - id for compatibility check
+ *
+ * @id_h: high 64bit of the compat_id
+ * @id_l: low 64bit of the compat_id
+ */
+struct fpga_compat_id {
+	u64 id_h;
+	u64 id_l;
+};
+
+/**
+ * struct fpga_manager_info - collection of parameters for an FPGA Manager
+ * @name: fpga manager name
+ * @compat_id: FPGA manager id for compatibility check.
+ * @mops: pointer to structure of fpga manager ops
+ * @priv: fpga manager private data
+ *
+ * fpga_manager_info contains parameters for the register function. These
+ * are separated into an info structure because they some are optional
+ * others could be added to in the future. The info structure facilitates
+ * maintaining a stable API.
+ */
+struct fpga_manager_info {
+	const char *name;
+	struct fpga_compat_id *compat_id;
+	const struct fpga_manager_ops *mops;
+	void *priv;
+};
+
 /**
  * struct fpga_manager_ops - ops for low level fpga manager drivers
  * @initial_header_size: Maximum number of bytes that should be passed into write_init
@@ -143,17 +173,6 @@ struct fpga_manager_ops {
 #define FPGA_MGR_STATUS_IP_PROTOCOL_ERR		BIT(3)
 #define FPGA_MGR_STATUS_FIFO_OVERFLOW_ERR	BIT(4)
 
-/**
- * struct fpga_compat_id - id for compatibility check
- *
- * @id_h: high 64bit of the compat_id
- * @id_l: low 64bit of the compat_id
- */
-struct fpga_compat_id {
-	u64 id_h;
-	u64 id_l;
-};
-
 /**
  * struct fpga_manager - fpga manager structure
  * @name: name of low level fpga manager
@@ -191,17 +210,18 @@ struct fpga_manager *fpga_mgr_get(struct device *dev);
 
 void fpga_mgr_put(struct fpga_manager *mgr);
 
-struct fpga_manager *fpga_mgr_create(struct device *dev, const char *name,
-				     const struct fpga_manager_ops *mops,
-				     void *priv);
-void fpga_mgr_free(struct fpga_manager *mgr);
-int fpga_mgr_register(struct fpga_manager *mgr);
-void fpga_mgr_unregister(struct fpga_manager *mgr);
+struct fpga_manager *
+fpga_mgr_register(struct device *parent, const struct fpga_manager_info *info);
 
-int devm_fpga_mgr_register(struct device *dev, struct fpga_manager *mgr);
+struct fpga_manager *
+fpga_mgr_register_simple(struct device *parent, const char *name,
+			 const struct fpga_manager_ops *mops, void *priv);
+void fpga_mgr_unregister(struct fpga_manager *mgr);
 
-struct fpga_manager *devm_fpga_mgr_create(struct device *dev, const char *name,
-					  const struct fpga_manager_ops *mops,
-					  void *priv);
+struct fpga_manager *
+devm_fpga_mgr_register(struct device *parent, const struct fpga_manager_info *info);
+struct fpga_manager *
+devm_fpga_mgr_register_simple(struct device *parent, const char *name,
+			      const struct fpga_manager_ops *mops, void *priv);
 
 #endif /*_LINUX_FPGA_MGR_H */
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v8 2/3] fpga: bridge: Use standard dev_release for class driver
  2021-06-24 20:53 [PATCH v8 0/3] fpga: Use standard class dev_release function Russ Weight
  2021-06-24 20:53 ` [PATCH v8 1/3] fpga: mgr: Use standard dev_release for class driver Russ Weight
@ 2021-06-24 20:53 ` Russ Weight
  2021-06-28  5:31   ` Xu Yilun
  2021-06-24 20:53 ` [PATCH v8 3/3] fpga: region: " Russ Weight
  2021-06-27 21:05 ` [PATCH v8 0/3] fpga: Use standard class dev_release function Russ Weight
  3 siblings, 1 reply; 10+ messages in thread
From: Russ Weight @ 2021-06-24 20:53 UTC (permalink / raw)
  To: mdf, linux-fpga
  Cc: trix, lgoncalv, yilun.xu, hao.wu, matthew.gerlach, richard.gong,
	Russ Weight

The FPGA bridge class driver data structure is being treated as a
managed resource instead of using the standard dev_release call-back
function to release the class data structure. This change removes
the managed resource code and combines the create() and register()
functions into a single register() or register_simple() function.

The register() function accepts an info data structure to provide
flexibility in passing optional parameters. The register_simple()
function supports the current parameter list for users that don't
require the use of optional parameters.

Signed-off-by: Russ Weight <russell.h.weight@intel.com>
Reviewed-by: Xu Yilun <yilun.xu@intel.com>
---
v8:
  - Added reviewed-by tag.
  - Updated Documentation/driver-api/fpga/fpga-bridge.rst documentation.
v7:
  - Update the commit message to describe the new parameters for the
    fpga_bridge_register() function and to mention the
    fpga_bridge_register_simple() function.
  - Fix function prototypes in header file to rename dev to parent.
  - Some cleanup of comments.
  - Update function defintions/prototypes to apply const to the new info
    parameter.
  - Verify that info->br_ops is non-null in register functions.
v6:
  - Changed fpga_bridge_register() parameters to accept an info data
    structure to provide flexibility in passing optional parameters.
  - Added fpga_bridge_register_simple() function to support current
    parameters for users that don't require the use of optional
    parameters.
v5:
  - Rebased on top of recently accepted patches.
v4:
  - Restore the previous format for the Return value in the comment header
    for fpga_bridge_register()
v3:
  - Cleaned up comment header for fpga_bridge_register()
  - Fix error return values for fpga_bridge_register()
v2:
  - No changes
---
 Documentation/driver-api/fpga/fpga-bridge.rst |   8 +-
 drivers/fpga/altera-fpga2sdram.c              |  12 +-
 drivers/fpga/altera-freeze-bridge.c           |  10 +-
 drivers/fpga/altera-hps2fpga.c                |  12 +-
 drivers/fpga/dfl-fme-br.c                     |  10 +-
 drivers/fpga/fpga-bridge.c                    | 138 ++++++------------
 drivers/fpga/xilinx-pr-decoupler.c            |  17 +--
 include/linux/fpga/fpga-bridge.h              |  33 +++--
 8 files changed, 100 insertions(+), 140 deletions(-)

diff --git a/Documentation/driver-api/fpga/fpga-bridge.rst b/Documentation/driver-api/fpga/fpga-bridge.rst
index 8d650b4e2ce6..d87aafc60ad6 100644
--- a/Documentation/driver-api/fpga/fpga-bridge.rst
+++ b/Documentation/driver-api/fpga/fpga-bridge.rst
@@ -6,8 +6,8 @@ API to implement a new FPGA bridge
 
 * struct fpga_bridge - The FPGA Bridge structure
 * struct fpga_bridge_ops - Low level Bridge driver ops
-* devm_fpga_bridge_create() - Allocate and init a bridge struct
-* fpga_bridge_register() - Register a bridge
+* fpga_bridge_register() - Create and register a bridge
+* fpga_bridge_register_simple() - Create and register a bridge
 * fpga_bridge_unregister() - Unregister a bridge
 
 .. kernel-doc:: include/linux/fpga/fpga-bridge.h
@@ -17,10 +17,10 @@ API to implement a new FPGA bridge
    :functions: fpga_bridge_ops
 
 .. kernel-doc:: drivers/fpga/fpga-bridge.c
-   :functions: devm_fpga_bridge_create
+   :functions: fpga_bridge_register
 
 .. kernel-doc:: drivers/fpga/fpga-bridge.c
-   :functions: fpga_bridge_register
+   :functions: fpga_bridge_register_simple
 
 .. kernel-doc:: drivers/fpga/fpga-bridge.c
    :functions: fpga_bridge_unregister
diff --git a/drivers/fpga/altera-fpga2sdram.c b/drivers/fpga/altera-fpga2sdram.c
index a78e49c63c64..e165440ebbab 100644
--- a/drivers/fpga/altera-fpga2sdram.c
+++ b/drivers/fpga/altera-fpga2sdram.c
@@ -121,17 +121,13 @@ static int alt_fpga_bridge_probe(struct platform_device *pdev)
 	/* Get f2s bridge configuration saved in handoff register */
 	regmap_read(sysmgr, SYSMGR_ISWGRP_HANDOFF3, &priv->mask);
 
-	br = devm_fpga_bridge_create(dev, F2S_BRIDGE_NAME,
-				     &altera_fpga2sdram_br_ops, priv);
-	if (!br)
-		return -ENOMEM;
+	br = fpga_bridge_register_simple(dev, F2S_BRIDGE_NAME,
+					 &altera_fpga2sdram_br_ops, priv);
+	if (IS_ERR(br))
+		return PTR_ERR(mgr);
 
 	platform_set_drvdata(pdev, br);
 
-	ret = fpga_bridge_register(br);
-	if (ret)
-		return ret;
-
 	dev_info(dev, "driver initialized with handoff %08x\n", priv->mask);
 
 	if (!of_property_read_u32(dev->of_node, "bridge-enable", &enable)) {
diff --git a/drivers/fpga/altera-freeze-bridge.c b/drivers/fpga/altera-freeze-bridge.c
index dd58c4aea92e..4e39b5475630 100644
--- a/drivers/fpga/altera-freeze-bridge.c
+++ b/drivers/fpga/altera-freeze-bridge.c
@@ -244,14 +244,14 @@ static int altera_freeze_br_probe(struct platform_device *pdev)
 
 	priv->base_addr = base_addr;
 
-	br = devm_fpga_bridge_create(dev, FREEZE_BRIDGE_NAME,
-				     &altera_freeze_br_br_ops, priv);
-	if (!br)
-		return -ENOMEM;
+	br = fpga_bridge_register_simple(dev, FREEZE_BRIDGE_NAME,
+					 &altera_freeze_br_br_ops, priv);
+	if (IS_ERR(br))
+		return PTR_ERR(br);
 
 	platform_set_drvdata(pdev, br);
 
-	return fpga_bridge_register(br);
+	return 0;
 }
 
 static int altera_freeze_br_remove(struct platform_device *pdev)
diff --git a/drivers/fpga/altera-hps2fpga.c b/drivers/fpga/altera-hps2fpga.c
index 77b95f251821..a564eb29349c 100644
--- a/drivers/fpga/altera-hps2fpga.c
+++ b/drivers/fpga/altera-hps2fpga.c
@@ -180,19 +180,15 @@ static int alt_fpga_bridge_probe(struct platform_device *pdev)
 		}
 	}
 
-	br = devm_fpga_bridge_create(dev, priv->name,
-				     &altera_hps2fpga_br_ops, priv);
-	if (!br) {
-		ret = -ENOMEM;
+	br = fpga_bridge_register_simple(dev, priv->name,
+					 &altera_hps2fpga_br_ops, priv);
+	if (IS_ERR(br)) {
+		ret = PTR_ERR(br);
 		goto err;
 	}
 
 	platform_set_drvdata(pdev, br);
 
-	ret = fpga_bridge_register(br);
-	if (ret)
-		goto err;
-
 	return 0;
 
 err:
diff --git a/drivers/fpga/dfl-fme-br.c b/drivers/fpga/dfl-fme-br.c
index 3ff9f3a687ce..0ad39e502142 100644
--- a/drivers/fpga/dfl-fme-br.c
+++ b/drivers/fpga/dfl-fme-br.c
@@ -68,14 +68,14 @@ static int fme_br_probe(struct platform_device *pdev)
 
 	priv->pdata = dev_get_platdata(dev);
 
-	br = devm_fpga_bridge_create(dev, "DFL FPGA FME Bridge",
-				     &fme_bridge_ops, priv);
-	if (!br)
-		return -ENOMEM;
+	br = fpga_bridge_register_simple(dev, "DFL FPGA FME Bridge",
+					 &fme_bridge_ops, priv);
+	if (IS_ERR(br))
+		return PTR_ERR(br);
 
 	platform_set_drvdata(pdev, br);
 
-	return fpga_bridge_register(br);
+	return 0;
 }
 
 static int fme_br_remove(struct platform_device *pdev)
diff --git a/drivers/fpga/fpga-bridge.c b/drivers/fpga/fpga-bridge.c
index 798f55670646..6f2f16fd5025 100644
--- a/drivers/fpga/fpga-bridge.c
+++ b/drivers/fpga/fpga-bridge.c
@@ -312,55 +312,62 @@ static struct attribute *fpga_bridge_attrs[] = {
 ATTRIBUTE_GROUPS(fpga_bridge);
 
 /**
- * fpga_bridge_create - create and initialize a struct fpga_bridge
+ * fpga_bridge_register - create and register an FPGA Bridge device
  * @parent:	FPGA bridge device from pdev
- * @name:	FPGA bridge name
- * @br_ops:	pointer to structure of fpga bridge ops
- * @priv:	FPGA bridge private data
+ * @info:	parameters for FPGA Bridge
  *
- * The caller of this function is responsible for freeing the bridge with
- * fpga_bridge_free().  Using devm_fpga_bridge_create() instead is recommended.
- *
- * Return: struct fpga_bridge or NULL
+ * Return: struct fpga_bridge pointer or ERR_PTR()
  */
-struct fpga_bridge *fpga_bridge_create(struct device *parent, const char *name,
-				       const struct fpga_bridge_ops *br_ops,
-				       void *priv)
+struct fpga_bridge *fpga_bridge_register(struct device *parent,
+					 const struct fpga_bridge_info *info)
 {
 	struct fpga_bridge *bridge;
 	int id, ret;
 
-	if (!name || !strlen(name)) {
+	if (!info->br_ops) {
+		dev_err(parent, "Attempt to register without fpga_bridge_ops\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	if (!info->name || !strlen(info->name)) {
 		dev_err(parent, "Attempt to register with no name!\n");
-		return NULL;
+		return ERR_PTR(-EINVAL);
 	}
 
 	bridge = kzalloc(sizeof(*bridge), GFP_KERNEL);
 	if (!bridge)
-		return NULL;
+		return ERR_PTR(-ENOMEM);
 
 	id = ida_simple_get(&fpga_bridge_ida, 0, 0, GFP_KERNEL);
-	if (id < 0)
+	if (id < 0) {
+		ret = id;
 		goto error_kfree;
+	}
 
 	mutex_init(&bridge->mutex);
 	INIT_LIST_HEAD(&bridge->node);
 
-	bridge->name = name;
-	bridge->br_ops = br_ops;
-	bridge->priv = priv;
+	bridge->name = info->name;
+	bridge->br_ops = info->br_ops;
+	bridge->priv = info->priv;
 
-	device_initialize(&bridge->dev);
-	bridge->dev.groups = br_ops->groups;
+	bridge->dev.groups = info->br_ops->groups;
 	bridge->dev.class = fpga_bridge_class;
 	bridge->dev.parent = parent;
 	bridge->dev.of_node = parent->of_node;
 	bridge->dev.id = id;
+	of_platform_populate(bridge->dev.of_node, NULL, NULL, &bridge->dev);
 
 	ret = dev_set_name(&bridge->dev, "br%d", id);
 	if (ret)
 		goto error_device;
 
+	ret = device_register(&bridge->dev);
+	if (ret) {
+		put_device(&bridge->dev);
+		return ERR_PTR(ret);
+	}
+
 	return bridge;
 
 error_device:
@@ -368,90 +375,37 @@ struct fpga_bridge *fpga_bridge_create(struct device *parent, const char *name,
 error_kfree:
 	kfree(bridge);
 
-	return NULL;
-}
-EXPORT_SYMBOL_GPL(fpga_bridge_create);
-
-/**
- * fpga_bridge_free - free an fpga bridge created by fpga_bridge_create()
- * @bridge:	FPGA bridge struct
- */
-void fpga_bridge_free(struct fpga_bridge *bridge)
-{
-	ida_simple_remove(&fpga_bridge_ida, bridge->dev.id);
-	kfree(bridge);
-}
-EXPORT_SYMBOL_GPL(fpga_bridge_free);
-
-static void devm_fpga_bridge_release(struct device *dev, void *res)
-{
-	struct fpga_bridge *bridge = *(struct fpga_bridge **)res;
-
-	fpga_bridge_free(bridge);
+	return ERR_PTR(ret);
 }
+EXPORT_SYMBOL_GPL(fpga_bridge_register);
 
 /**
- * devm_fpga_bridge_create - create and init a managed struct fpga_bridge
+ * fpga_bridge_register_simple - create and register an FPGA Bridge device
  * @parent:	FPGA bridge device from pdev
  * @name:	FPGA bridge name
  * @br_ops:	pointer to structure of fpga bridge ops
  * @priv:	FPGA bridge private data
  *
- * This function is intended for use in an FPGA bridge driver's probe function.
- * After the bridge driver creates the struct with devm_fpga_bridge_create(), it
- * should register the bridge with fpga_bridge_register().  The bridge driver's
- * remove function should call fpga_bridge_unregister().  The bridge struct
- * allocated with this function will be freed automatically on driver detach.
- * This includes the case of a probe function returning error before calling
- * fpga_bridge_register(), the struct will still get cleaned up.
+ * This simple version of the register should be sufficient for most users.
+ * The fpga_bridge_register() function is available for users that need to pass
+ * additional, optional parameters.
  *
- *  Return: struct fpga_bridge or NULL
+ * Return: struct fpga_bridge pointer or ERR_PTR()
  */
-struct fpga_bridge
-*devm_fpga_bridge_create(struct device *parent, const char *name,
-			 const struct fpga_bridge_ops *br_ops, void *priv)
+struct fpga_bridge *
+fpga_bridge_register_simple(struct device *parent, const char *name,
+			    const struct fpga_bridge_ops *br_ops,
+			    void *priv)
 {
-	struct fpga_bridge **ptr, *bridge;
-
-	ptr = devres_alloc(devm_fpga_bridge_release, sizeof(*ptr), GFP_KERNEL);
-	if (!ptr)
-		return NULL;
-
-	bridge = fpga_bridge_create(parent, name, br_ops, priv);
-	if (!bridge) {
-		devres_free(ptr);
-	} else {
-		*ptr = bridge;
-		devres_add(parent, ptr);
-	}
-
-	return bridge;
-}
-EXPORT_SYMBOL_GPL(devm_fpga_bridge_create);
-
-/**
- * fpga_bridge_register - register an FPGA bridge
- *
- * @bridge: FPGA bridge struct
- *
- * Return: 0 for success, error code otherwise.
- */
-int fpga_bridge_register(struct fpga_bridge *bridge)
-{
-	struct device *dev = &bridge->dev;
-	int ret;
-
-	ret = device_add(dev);
-	if (ret)
-		return ret;
-
-	of_platform_populate(dev->of_node, NULL, NULL, dev);
+	struct fpga_bridge_info info = { 0 };
 
-	dev_info(dev->parent, "fpga bridge [%s] registered\n", bridge->name);
+	info.name = name;
+	info.br_ops = br_ops;
+	info.priv = priv;
 
-	return 0;
+	return fpga_bridge_register(parent, &info);
 }
-EXPORT_SYMBOL_GPL(fpga_bridge_register);
+EXPORT_SYMBOL_GPL(fpga_bridge_register_simple);
 
 /**
  * fpga_bridge_unregister - unregister an FPGA bridge
@@ -475,6 +429,10 @@ 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)
diff --git a/drivers/fpga/xilinx-pr-decoupler.c b/drivers/fpga/xilinx-pr-decoupler.c
index ea2bde6e5bc4..24bc16a86091 100644
--- a/drivers/fpga/xilinx-pr-decoupler.c
+++ b/drivers/fpga/xilinx-pr-decoupler.c
@@ -138,22 +138,17 @@ static int xlnx_pr_decoupler_probe(struct platform_device *pdev)
 
 	clk_disable(priv->clk);
 
-	br = devm_fpga_bridge_create(&pdev->dev, priv->ipconfig->name,
-				     &xlnx_pr_decoupler_br_ops, priv);
-	if (!br) {
-		err = -ENOMEM;
-		goto err_clk;
-	}
-
-	platform_set_drvdata(pdev, br);
-
-	err = fpga_bridge_register(br);
-	if (err) {
+	br = fpga_bridge_register_simple(&pdev->dev, priv->ipconfig->name,
+					 &xlnx_pr_decoupler_br_ops, priv);
+	if (IS_ERR(br)) {
+		err = PTR_ERR(br);
 		dev_err(&pdev->dev, "unable to register %s",
 			priv->ipconfig->name);
 		goto err_clk;
 	}
 
+	platform_set_drvdata(pdev, br);
+
 	return 0;
 
 err_clk:
diff --git a/include/linux/fpga/fpga-bridge.h b/include/linux/fpga/fpga-bridge.h
index 6c3c28806ff1..6b5584530c92 100644
--- a/include/linux/fpga/fpga-bridge.h
+++ b/include/linux/fpga/fpga-bridge.h
@@ -22,6 +22,23 @@ struct fpga_bridge_ops {
 	const struct attribute_group **groups;
 };
 
+/**
+ * struct fpga_bridge_info - collection of parameters an FPGA Bridge
+ * @name: fpga bridge name
+ * @br_ops: pointer to structure of fpga bridge ops
+ * @priv: fpga bridge private data
+ *
+ * fpga_bridge_info contains parameters for the register function. These
+ * are separated into an info structure because they some are optional
+ * others could be added to in the future. The info structure facilitates
+ * maintaining a stable API.
+ */
+struct fpga_bridge_info {
+	const char *name;
+	const struct fpga_bridge_ops *br_ops;
+	void *priv;
+};
+
 /**
  * struct fpga_bridge - FPGA bridge structure
  * @name: name of low level FPGA bridge
@@ -62,15 +79,13 @@ int of_fpga_bridge_get_to_list(struct device_node *np,
 			       struct fpga_image_info *info,
 			       struct list_head *bridge_list);
 
-struct fpga_bridge *fpga_bridge_create(struct device *dev, const char *name,
-				       const struct fpga_bridge_ops *br_ops,
-				       void *priv);
-void fpga_bridge_free(struct fpga_bridge *br);
-int fpga_bridge_register(struct fpga_bridge *br);
-void fpga_bridge_unregister(struct fpga_bridge *br);
+struct fpga_bridge *
+fpga_bridge_register(struct device *parent, const struct fpga_bridge_info *info);
 
-struct fpga_bridge
-*devm_fpga_bridge_create(struct device *dev, const char *name,
-			 const struct fpga_bridge_ops *br_ops, void *priv);
+struct fpga_bridge *
+fpga_bridge_register_simple(struct device *parent, const char *name,
+			    const struct fpga_bridge_ops *br_ops,
+			    void *priv);
+void fpga_bridge_unregister(struct fpga_bridge *br);
 
 #endif /* _LINUX_FPGA_BRIDGE_H */
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v8 3/3] fpga: region: Use standard dev_release for class driver
  2021-06-24 20:53 [PATCH v8 0/3] fpga: Use standard class dev_release function Russ Weight
  2021-06-24 20:53 ` [PATCH v8 1/3] fpga: mgr: Use standard dev_release for class driver Russ Weight
  2021-06-24 20:53 ` [PATCH v8 2/3] fpga: bridge: " Russ Weight
@ 2021-06-24 20:53 ` Russ Weight
  2021-06-27 21:05 ` [PATCH v8 0/3] fpga: Use standard class dev_release function Russ Weight
  3 siblings, 0 replies; 10+ messages in thread
From: Russ Weight @ 2021-06-24 20:53 UTC (permalink / raw)
  To: mdf, linux-fpga
  Cc: trix, lgoncalv, yilun.xu, hao.wu, matthew.gerlach, richard.gong,
	Russ Weight

The FPGA region class driver data structure is being treated as a
managed resource instead of using the standard dev_release call-back
function to release the class data structure. This change removes the
managed resource code and combines the create() and register()
functions into a single register() or register_simple() function.

The register() function accepts an info data structure to provide
flexibility in passing optional parameters. The register_simple()
function supports the current parameter list for users that don't
require the use of optional parameters.

Signed-off-by: Russ Weight <russell.h.weight@intel.com>
Reviewed-by: Xu Yilun <yilun.xu@intel.com>
---
v8:
  - Added reviewed-by tag.
  - Updated Documentation/driver-api/fpga/fpga-region.rst documentation.
v7:
  - Update the commit message to describe the new parameters for the
    fpga_region_register() function and to mention the
    fpga_region_register_simple() function.
  - Fix function prototypes in header file to rename dev to parent.
  - Some cleanup of comments.
  - Update function defintions/prototypes to apply const to the new info
    parameter.
  - Verify a non-null info pointer in the register() functions.
v6:
  - Moved FPGA manager optional parameters out of the ops structure and
    back into the FPGA Region structure.
  - Changed fpga_region_register() parameters to accept an info data
    structure to provide flexibility in passing optional parameters.
  - Added fpga_region_register_simple() functions to support current
    parameters for users that don't require the use of optional parameters.
v5:
  - Rebased on top of recently accepted patches.
  - Created the fpga_region_ops data structure which is optionally passed
    to fpga_region_register(). compat_id, the get_bridges() pointer, and
    the priv pointer are included in the fpga_region_ops structure.
v4:
  - Added the compat_id parameter to fpga_region_register() to ensure
    that the compat_id is set before the device_register() call.
  - Modified the dfl_fpga_feature_devs_enumerate() function to restore
    the fpga_region_register() call to the correct location.
v3:
  - Cleaned up comment header for fpga_region_register()
  - Fix fpga_region_register() error return on ida_simple_get() failure
v2:
  - No changes
---
 Documentation/driver-api/fpga/fpga-region.rst |   8 +-
 drivers/fpga/dfl-fme-region.c                 |  17 ++-
 drivers/fpga/dfl.c                            |  12 +-
 drivers/fpga/fpga-region.c                    | 121 +++++++-----------
 drivers/fpga/of-fpga-region.c                 |  11 +-
 include/linux/fpga/fpga-region.h              |  36 ++++--
 6 files changed, 92 insertions(+), 113 deletions(-)

diff --git a/Documentation/driver-api/fpga/fpga-region.rst b/Documentation/driver-api/fpga/fpga-region.rst
index 2636a27c11b2..ee1cacf0fe1a 100644
--- a/Documentation/driver-api/fpga/fpga-region.rst
+++ b/Documentation/driver-api/fpga/fpga-region.rst
@@ -46,8 +46,8 @@ API to add a new FPGA region
 ----------------------------
 
 * struct fpga_region - The FPGA region struct
-* devm_fpga_region_create() - Allocate and init a region struct
-* fpga_region_register() -  Register an FPGA region
+* fpga_region_register() -  Create and register an FPGA region
+* fpga_region_register_simple() -  Create and register an FPGA region
 * fpga_region_unregister() -  Unregister an FPGA region
 
 The FPGA region's probe function will need to get a reference to the FPGA
@@ -76,10 +76,10 @@ following APIs to handle building or tearing down that list.
    :functions: fpga_region
 
 .. kernel-doc:: drivers/fpga/fpga-region.c
-   :functions: devm_fpga_region_create
+   :functions: fpga_region_register
 
 .. kernel-doc:: drivers/fpga/fpga-region.c
-   :functions: fpga_region_register
+   :functions: fpga_region_register_simple
 
 .. kernel-doc:: drivers/fpga/fpga-region.c
    :functions: fpga_region_unregister
diff --git a/drivers/fpga/dfl-fme-region.c b/drivers/fpga/dfl-fme-region.c
index 1eeb42af1012..36bf6865bbc3 100644
--- a/drivers/fpga/dfl-fme-region.c
+++ b/drivers/fpga/dfl-fme-region.c
@@ -30,6 +30,7 @@ static int fme_region_get_bridges(struct fpga_region *region)
 static int fme_region_probe(struct platform_device *pdev)
 {
 	struct dfl_fme_region_pdata *pdata = dev_get_platdata(&pdev->dev);
+	struct fpga_region_info info = { 0 };
 	struct device *dev = &pdev->dev;
 	struct fpga_region *region;
 	struct fpga_manager *mgr;
@@ -39,20 +40,18 @@ static int fme_region_probe(struct platform_device *pdev)
 	if (IS_ERR(mgr))
 		return -EPROBE_DEFER;
 
-	region = devm_fpga_region_create(dev, mgr, fme_region_get_bridges);
-	if (!region) {
-		ret = -ENOMEM;
+	info.mgr = mgr;
+	info.compat_id = mgr->compat_id;
+	info.get_bridges = fme_region_get_bridges;
+	info.priv = pdata;
+	region = fpga_region_register(dev, &info);
+	if (IS_ERR(region)) {
+		ret = PTR_ERR(region);
 		goto eprobe_mgr_put;
 	}
 
-	region->priv = pdata;
-	region->compat_id = mgr->compat_id;
 	platform_set_drvdata(pdev, region);
 
-	ret = fpga_region_register(region);
-	if (ret)
-		goto eprobe_mgr_put;
-
 	dev_dbg(dev, "DFL FME FPGA Region probed\n");
 
 	return 0;
diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
index 511b20ff35a3..a4269d40586e 100644
--- a/drivers/fpga/dfl.c
+++ b/drivers/fpga/dfl.c
@@ -1400,19 +1400,15 @@ dfl_fpga_feature_devs_enumerate(struct dfl_fpga_enum_info *info)
 	if (!cdev)
 		return ERR_PTR(-ENOMEM);
 
-	cdev->region = devm_fpga_region_create(info->dev, NULL, NULL);
-	if (!cdev->region) {
-		ret = -ENOMEM;
-		goto free_cdev_exit;
-	}
-
 	cdev->parent = info->dev;
 	mutex_init(&cdev->lock);
 	INIT_LIST_HEAD(&cdev->port_dev_list);
 
-	ret = fpga_region_register(cdev->region);
-	if (ret)
+	cdev->region = fpga_region_register_simple(info->dev, NULL, NULL);
+	if (IS_ERR(cdev->region)) {
+		ret = PTR_ERR(cdev->region);
 		goto free_cdev_exit;
+	}
 
 	/* create and init build info for enumeration */
 	binfo = devm_kzalloc(info->dev, sizeof(*binfo), GFP_KERNEL);
diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c
index a4838715221f..7009f366e27a 100644
--- a/drivers/fpga/fpga-region.c
+++ b/drivers/fpga/fpga-region.c
@@ -180,39 +180,42 @@ static struct attribute *fpga_region_attrs[] = {
 ATTRIBUTE_GROUPS(fpga_region);
 
 /**
- * fpga_region_create - alloc and init a struct fpga_region
+ * fpga_region_register - create and register an FPGA Region device
  * @parent: device parent
- * @mgr: manager that programs this region
- * @get_bridges: optional function to get bridges to a list
- *
- * The caller of this function is responsible for freeing the resulting region
- * struct with fpga_region_free().  Using devm_fpga_region_create() instead is
- * recommended.
+ * @info: parameters for FPGA Region
  *
- * Return: struct fpga_region or NULL
+ * Return: struct fpga_region or ERR_PTR()
  */
-struct fpga_region
-*fpga_region_create(struct device *parent,
-		    struct fpga_manager *mgr,
-		    int (*get_bridges)(struct fpga_region *))
+struct fpga_region *
+fpga_region_register(struct device *parent, const struct fpga_region_info *info)
 {
 	struct fpga_region *region;
 	int id, ret = 0;
 
+	if (!info) {
+		dev_err(parent,
+			"Attempt to register without required info structure\n");
+		return ERR_PTR(-EINVAL);
+	}
+
 	region = kzalloc(sizeof(*region), GFP_KERNEL);
 	if (!region)
-		return NULL;
+		return ERR_PTR(-ENOMEM);
 
 	id = ida_simple_get(&fpga_region_ida, 0, 0, GFP_KERNEL);
-	if (id < 0)
+	if (id < 0) {
+		ret = id;
 		goto err_free;
+	}
+
+	region->mgr = info->mgr;
+	region->compat_id = info->compat_id;
+	region->priv = info->priv;
+	region->get_bridges = info->get_bridges;
 
-	region->mgr = mgr;
-	region->get_bridges = get_bridges;
 	mutex_init(&region->mutex);
 	INIT_LIST_HEAD(&region->bridge_list);
 
-	device_initialize(&region->dev);
 	region->dev.class = fpga_region_class;
 	region->dev.parent = parent;
 	region->dev.of_node = parent->of_node;
@@ -222,6 +225,12 @@ struct fpga_region
 	if (ret)
 		goto err_remove;
 
+	ret = device_register(&region->dev);
+	if (ret) {
+		put_device(&region->dev);
+		return ERR_PTR(ret);
+	}
+
 	return region;
 
 err_remove:
@@ -229,78 +238,34 @@ struct fpga_region
 err_free:
 	kfree(region);
 
-	return NULL;
-}
-EXPORT_SYMBOL_GPL(fpga_region_create);
-
-/**
- * fpga_region_free - free an FPGA region created by fpga_region_create()
- * @region: FPGA region
- */
-void fpga_region_free(struct fpga_region *region)
-{
-	ida_simple_remove(&fpga_region_ida, region->dev.id);
-	kfree(region);
-}
-EXPORT_SYMBOL_GPL(fpga_region_free);
-
-static void devm_fpga_region_release(struct device *dev, void *res)
-{
-	struct fpga_region *region = *(struct fpga_region **)res;
-
-	fpga_region_free(region);
+	return ERR_PTR(ret);
 }
+EXPORT_SYMBOL_GPL(fpga_region_register);
 
 /**
- * devm_fpga_region_create - create and initialize a managed FPGA region struct
+ * fpga_region_register_simple - create and register an FPGA Region device
  * @parent: device parent
  * @mgr: manager that programs this region
  * @get_bridges: optional function to get bridges to a list
  *
- * This function is intended for use in an FPGA region driver's probe function.
- * After the region driver creates the region struct with
- * devm_fpga_region_create(), it should register it with fpga_region_register().
- * The region driver's remove function should call fpga_region_unregister().
- * The region struct allocated with this function will be freed automatically on
- * driver detach.  This includes the case of a probe function returning error
- * before calling fpga_region_register(), the struct will still get cleaned up.
+ * This simple version of the register should be sufficient for most users.
+ * The fpga_region_register() function is available for users that need to
+ * pass additional, optional parameters.
  *
- * Return: struct fpga_region or NULL
+ * Return: struct fpga_region or ERR_PTR()
  */
-struct fpga_region
-*devm_fpga_region_create(struct device *parent,
-			 struct fpga_manager *mgr,
-			 int (*get_bridges)(struct fpga_region *))
+struct fpga_region *
+fpga_region_register_simple(struct device *parent, struct fpga_manager *mgr,
+			    int (*get_bridges)(struct fpga_region *))
 {
-	struct fpga_region **ptr, *region;
-
-	ptr = devres_alloc(devm_fpga_region_release, sizeof(*ptr), GFP_KERNEL);
-	if (!ptr)
-		return NULL;
-
-	region = fpga_region_create(parent, mgr, get_bridges);
-	if (!region) {
-		devres_free(ptr);
-	} else {
-		*ptr = region;
-		devres_add(parent, ptr);
-	}
+	struct fpga_region_info info = { 0 };
 
-	return region;
-}
-EXPORT_SYMBOL_GPL(devm_fpga_region_create);
+	info.mgr = mgr;
+	info.get_bridges = get_bridges;
 
-/**
- * fpga_region_register - register an FPGA region
- * @region: FPGA region
- *
- * Return: 0 or -errno
- */
-int fpga_region_register(struct fpga_region *region)
-{
-	return device_add(&region->dev);
+	return fpga_region_register(parent, &info);
 }
-EXPORT_SYMBOL_GPL(fpga_region_register);
+EXPORT_SYMBOL_GPL(fpga_region_register_simple);
 
 /**
  * fpga_region_unregister - unregister an FPGA region
@@ -316,6 +281,10 @@ 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);
 }
 
 /**
diff --git a/drivers/fpga/of-fpga-region.c b/drivers/fpga/of-fpga-region.c
index e3c25576b6b9..377d19585d8e 100644
--- a/drivers/fpga/of-fpga-region.c
+++ b/drivers/fpga/of-fpga-region.c
@@ -405,16 +405,13 @@ static int of_fpga_region_probe(struct platform_device *pdev)
 	if (IS_ERR(mgr))
 		return -EPROBE_DEFER;
 
-	region = devm_fpga_region_create(dev, mgr, of_fpga_region_get_bridges);
-	if (!region) {
-		ret = -ENOMEM;
+	region = fpga_region_register_simple(dev, mgr,
+					     of_fpga_region_get_bridges);
+	if (IS_ERR(region)) {
+		ret = PTR_ERR(region);
 		goto eprobe_mgr_put;
 	}
 
-	ret = fpga_region_register(region);
-	if (ret)
-		goto eprobe_mgr_put;
-
 	of_platform_populate(np, fpga_region_of_match, NULL, &region->dev);
 	platform_set_drvdata(pdev, region);
 
diff --git a/include/linux/fpga/fpga-region.h b/include/linux/fpga/fpga-region.h
index 27cb706275db..6c0da49265cb 100644
--- a/include/linux/fpga/fpga-region.h
+++ b/include/linux/fpga/fpga-region.h
@@ -7,6 +7,27 @@
 #include <linux/fpga/fpga-mgr.h>
 #include <linux/fpga/fpga-bridge.h>
 
+struct fpga_region;
+
+/**
+ * struct fpga_region_info - collection of parameters an FPGA Region
+ * @mgr: fpga region manager
+ * @compat_id: FPGA region id for compatibility check.
+ * @priv: fpga region private data
+ * @get_bridges: optional function to get bridges to a list
+ *
+ * fpga_region_info contains parameters for the register function. These
+ * are separated into an info structure because they some are optional
+ * others could be added to in the future. The info structure facilitates
+ * maintaining a stable API.
+ */
+struct fpga_region_info {
+	struct fpga_manager *mgr;
+	struct fpga_compat_id *compat_id;
+	void *priv;
+	int (*get_bridges)(struct fpga_region *region);
+};
+
 /**
  * struct fpga_region - FPGA Region structure
  * @dev: FPGA Region device
@@ -37,15 +58,12 @@ struct fpga_region *fpga_region_class_find(
 
 int fpga_region_program_fpga(struct fpga_region *region);
 
-struct fpga_region
-*fpga_region_create(struct device *dev, struct fpga_manager *mgr,
-		    int (*get_bridges)(struct fpga_region *));
-void fpga_region_free(struct fpga_region *region);
-int fpga_region_register(struct fpga_region *region);
-void fpga_region_unregister(struct fpga_region *region);
+struct fpga_region *
+fpga_region_register(struct device *parent, const struct fpga_region_info *info);
 
-struct fpga_region
-*devm_fpga_region_create(struct device *dev, struct fpga_manager *mgr,
-			int (*get_bridges)(struct fpga_region *));
+struct fpga_region *
+fpga_region_register_simple(struct device *parent, struct fpga_manager *mgr,
+			    int (*get_bridges)(struct fpga_region *));
+void fpga_region_unregister(struct fpga_region *region);
 
 #endif /* _FPGA_REGION_H */
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v8 0/3] fpga: Use standard class dev_release function
  2021-06-24 20:53 [PATCH v8 0/3] fpga: Use standard class dev_release function Russ Weight
                   ` (2 preceding siblings ...)
  2021-06-24 20:53 ` [PATCH v8 3/3] fpga: region: " Russ Weight
@ 2021-06-27 21:05 ` Russ Weight
  3 siblings, 0 replies; 10+ messages in thread
From: Russ Weight @ 2021-06-27 21:05 UTC (permalink / raw)
  To: mdf, linux-fpga
  Cc: trix, lgoncalv, yilun.xu, hao.wu, matthew.gerlach, richard.gong

I think this version may be ready to go. Please take a minute to review and either provide
a review tag or let me know of any issues you see.

- Russ

On 6/24/21 1:53 PM, Russ Weight wrote:
> 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.
>
> These changes remove the managed resource functions and populate the class
> dev_release callback functions. They also merge the create() and register()
> functions into a single register() or register_simple() function for each of
> the fpga-mgr, fpga-region, and fpga-bridge class drivers.
>
> The new *register() functions accept an info data structure to provide
> flexibility in passing optional parameters. The *register_simple()
> functions support the legacy parameter list for users that don't require
> the use of optional parameters.
>
> For more context, refer to this email thread:
>
> https://marc.info/?l=linux-fpga&m=162127412218557&w=2
>
> I turned on the configs assocated with each of the modified files, but I
> must have been missing some dependencies, because not all of them compiled.
> I did a run-time test specifically with the dfl-fme infrastructure. This
> would have exercised the region, bridge, and fpga-mgr frameworks.
>
> Changelog v7 -> v8:
>   - Added reviewed-by tags.
>   - Updated Documentation/driver-api/fpga/ files: fpga-mgr.rst,
>     fpga-bridge.rst, and fpga-region.rst.
>
> Changelog v6 -> v7:
>   - Update the commit messages to describe the new parameters for the
>     *register() functions and to mention the *register_simple() functions.
>   - Fix function prototypes in header file to rename dev to parent.
>   - Make use of the PTR_ERR_OR_ZERO() macro when possible.
>   - Some cleanup of comments.
>   - Update function defintions/prototypes to apply const to the new info
>     parameter.
>   - Verify that info->br_ops is non-null in the fpga_bridge_register()
>     function.
>   - Verify a non-null info pointer in the fpga_region_register() function.
>
> Changelog v5 -> v6:
>   - Moved FPGA manager/bridge/region optional parameters out of the ops
>     structure and back into the FPGA class driver structure.
>   - Changed fpga_*_register() function parameters to accept an info data
>     structure to provide flexibility in passing optional parameters.
>   - Added fpga_*_register_simple() functions to support current parameters
>     for users that don't require use of optional parameters.
>
> Changelog v4 -> v5:
>   - Rebased on top of recently accepted patches.
>   - Removed compat_id from the fpga_mgr_register() parameter list
>     and added it to the fpga_manager_ops structure. This also required
>     dynamically allocating the dfl-fme-ops structure in order to add
>     the appropriate compat_id.
>   - Created the fpga_region_ops data structure which is optionally passed
>     to fpga_region_register(). compat_id, the get_bridges() pointer, and
>     the priv pointer are included in the fpga_region_ops structure.
>
> Changelog v3 -> v4:
>   - Added the compat_id parameter to fpga_mgr_register() and
>     devm_fpga_mgr_register() to ensure that the compat_id is set before
>     the device_register() call.
>   - Added the compat_id parameter to fpga_region_register() to ensure
>     that the compat_id is set before the device_register() call.
>   - Modified the dfl_fpga_feature_devs_enumerate() function to restore
>     the fpga_region_register() call to the correct location.
>
> Changelog v2 -> v3:
>   - Cleaned up comment headers for fpga_mgr_register(), fpga_bridge_register(),
>     and fpga_region_register().
>   - Fixed error return on ida_simple_get() failure for fpga_mgr_register(),
>     fpga_bridge_register(), and fpga_region_register().
>   - Fixed error return value for fpga_bridge_register(): ERR_PTR(ret) instead
>     of NULL.
>
> Changelog v1 -> v2:
>   - Restored devm_fpga_mgr_register() functionality to the fpga-mgr
>     class driver, adapted for the combined create/register functionality.
>   - All previous callers of devm_fpga_mgr_register() will continue to call
>     devm_fpga_mgr_register().
>   - replaced unnecessary ternary operators in return statements with
>     standard if conditions.
>
> Russ Weight (3):
>   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
>
>  Documentation/driver-api/fpga/fpga-bridge.rst |   8 +-
>  Documentation/driver-api/fpga/fpga-mgr.rst    |  39 ++--
>  Documentation/driver-api/fpga/fpga-region.rst |   8 +-
>  drivers/fpga/altera-cvp.c                     |  12 +-
>  drivers/fpga/altera-fpga2sdram.c              |  12 +-
>  drivers/fpga/altera-freeze-bridge.c           |  10 +-
>  drivers/fpga/altera-hps2fpga.c                |  12 +-
>  drivers/fpga/altera-pr-ip-core.c              |   8 +-
>  drivers/fpga/altera-ps-spi.c                  |   9 +-
>  drivers/fpga/dfl-fme-br.c                     |  10 +-
>  drivers/fpga/dfl-fme-mgr.c                    |  22 +-
>  drivers/fpga/dfl-fme-region.c                 |  17 +-
>  drivers/fpga/dfl.c                            |  12 +-
>  drivers/fpga/fpga-bridge.c                    | 138 ++++-------
>  drivers/fpga/fpga-mgr.c                       | 214 ++++++++----------
>  drivers/fpga/fpga-region.c                    | 121 ++++------
>  drivers/fpga/ice40-spi.c                      |   9 +-
>  drivers/fpga/machxo2-spi.c                    |  10 +-
>  drivers/fpga/of-fpga-region.c                 |  11 +-
>  drivers/fpga/socfpga-a10.c                    |  16 +-
>  drivers/fpga/socfpga.c                        |   9 +-
>  drivers/fpga/stratix10-soc.c                  |  16 +-
>  drivers/fpga/ts73xx-fpga.c                    |   9 +-
>  drivers/fpga/xilinx-pr-decoupler.c            |  17 +-
>  drivers/fpga/xilinx-spi.c                     |  11 +-
>  drivers/fpga/zynq-fpga.c                      |  16 +-
>  drivers/fpga/zynqmp-fpga.c                    |   9 +-
>  include/linux/fpga/fpga-bridge.h              |  33 ++-
>  include/linux/fpga/fpga-mgr.h                 |  62 +++--
>  include/linux/fpga/fpga-region.h              |  36 ++-
>  30 files changed, 396 insertions(+), 520 deletions(-)
>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [PATCH v8 1/3] fpga: mgr: Use standard dev_release for class driver
  2021-06-24 20:53 ` [PATCH v8 1/3] fpga: mgr: Use standard dev_release for class driver Russ Weight
@ 2021-06-28  3:49   ` Wu, Hao
  2021-06-28 18:51     ` Moritz Fischer
  2021-06-30 22:12     ` Russ Weight
  2021-06-28  4:39   ` Xu Yilun
  1 sibling, 2 replies; 10+ messages in thread
From: Wu, Hao @ 2021-06-28  3:49 UTC (permalink / raw)
  To: Weight, Russell H, mdf, linux-fpga
  Cc: trix, lgoncalv, Xu, Yilun, Gerlach, Matthew, Gong, Richard

> -----Original Message-----
> From: Weight, Russell H <russell.h.weight@intel.com>
> Sent: Friday, June 25, 2021 4:54 AM
> To: mdf@kernel.org; linux-fpga@vger.kernel.org
> Cc: trix@redhat.com; lgoncalv@redhat.com; Xu, Yilun <yilun.xu@intel.com>;
> Wu, Hao <hao.wu@intel.com>; Gerlach, Matthew
> <matthew.gerlach@intel.com>; Gong, Richard <richard.gong@intel.com>;
> Weight, Russell H <russell.h.weight@intel.com>
> Subject: [PATCH v8 1/3] fpga: mgr: Use standard dev_release for class driver
> 
> The FPGA manager class driver data structure is being treated as a
> managed resource instead of using the standard dev_release call-back
> function to release the class data structure. This change removes
> the managed resource code for the freeing of the class data structure
> and combines the create() and register() functions into a single
> register() or register_simple() function.
> 

Maybe register() and register_full() is better. it keeps the most used
function name the same (and shorter than register_simple()). Just
my 2 cents. Let's wait for review comments from Moritz to decide. : )

Thanks
Hao

> The register() function accepts an info data structure to provide
> flexibility in passing optional parameters. The register_simple()
> function supports the current parameter list for users that don't
> require the use of optional parameters.
> 
> The devm_fpga_mgr_register() function is retained, and the
> devm_fpga_mgr_register_simple() function is added.
> 
> Signed-off-by: Russ Weight <russell.h.weight@intel.com>
> Reviewed-by: Xu Yilun <yilun.xu@intel.com>
> ---
> v8:
>   - Added reviewed-by tag.
>   - Updated Documentation/driver-api/fpga/fpga-mgr.rst documentation.
> v7:
>   - Update the commit message to describe the new parameters for the
>     *fpga_mgr_register() functions and to mention the
>     *fpga_mgr_register_simple() functions.
>   - Fix function prototypes in header file to rename dev to parent.
>   - Make use of the PTR_ERR_OR_ZERO() macro when possible.
>   - Some cleanup of comments.
>   - Update function defintions/prototypes to apply const to the new info
>     parameter.
> v6:
>   - Moved FPGA manager optional parameters out of the ops structure and
>     back into the FPGA manager structure.
>   - Changed fpga_mgr_register()/devm_fpga_mgr_register() parameters to
>     accept an info data structure to provide flexibility in passing optional
>     parameters.
>   - Added fpga_mgr_register_simple()/devm_fpga_mgr_register_simple()
>     functions to support current parameters for users that don't require
>     the use of optional parameters.
> v5:
>   - Rebased on top of recently accepted patches.
>   - Removed compat_id from the fpga_mgr_register() parameter list
>     and added it to the fpga_manager_ops structure. This also required
>     dynamically allocating the dfl-fme-ops structure in order to add
>     the appropriate compat_id.
> v4:
>   - Added the compat_id parameter to fpga_mgr_register() and
>     devm_fpga_mgr_register() to ensure that the compat_id is set before
>     the device_register() call.
> v3:
>   - Cleaned up comment header for fpga_mgr_register()
>   - Fix error return on ida_simple_get() failure
> v2:
>   - Restored devm_fpga_mgr_register() functionality, adapted for the combined
>     create/register functionality.
>   - All previous callers of devm_fpga_mgr_register() will continue to call
>     devm_fpga_mgr_register().
>   - replaced unnecessary ternary operators in return statements with standard
>     if conditions.
> ---
>  Documentation/driver-api/fpga/fpga-mgr.rst |  39 ++--
>  drivers/fpga/altera-cvp.c                  |  12 +-
>  drivers/fpga/altera-pr-ip-core.c           |   8 +-
>  drivers/fpga/altera-ps-spi.c               |   9 +-
>  drivers/fpga/dfl-fme-mgr.c                 |  22 +--
>  drivers/fpga/fpga-mgr.c                    | 214 +++++++++------------
>  drivers/fpga/ice40-spi.c                   |   9 +-
>  drivers/fpga/machxo2-spi.c                 |  10 +-
>  drivers/fpga/socfpga-a10.c                 |  16 +-
>  drivers/fpga/socfpga.c                     |   9 +-
>  drivers/fpga/stratix10-soc.c               |  16 +-
>  drivers/fpga/ts73xx-fpga.c                 |   9 +-
>  drivers/fpga/xilinx-spi.c                  |  11 +-
>  drivers/fpga/zynq-fpga.c                   |  16 +-
>  drivers/fpga/zynqmp-fpga.c                 |   9 +-
>  include/linux/fpga/fpga-mgr.h              |  62 ++++--
>  16 files changed, 204 insertions(+), 267 deletions(-)
> 
> diff --git a/Documentation/driver-api/fpga/fpga-mgr.rst
> b/Documentation/driver-api/fpga/fpga-mgr.rst
> index 4d926b452cb3..fbb7f07cae9f 100644
> --- a/Documentation/driver-api/fpga/fpga-mgr.rst
> +++ b/Documentation/driver-api/fpga/fpga-mgr.rst
> @@ -24,7 +24,8 @@ How to support a new FPGA device
>  --------------------------------
> 
>  To add another FPGA manager, write a driver that implements a set of ops.  The
> -probe function calls fpga_mgr_register(), such as::
> +probe function calls devm_fpga_mgr_register() or
> +devm_fpga_mgr_register_simple(), such as::
> 
>  	static const struct fpga_manager_ops socfpga_fpga_ops = {
>  		.write_init = socfpga_fpga_ops_configure_init,
> @@ -49,23 +50,9 @@ probe function calls fpga_mgr_register(), such as::
>  		 * them in priv
>  		 */
> 
> -		mgr = devm_fpga_mgr_create(dev, "Altera SOCFPGA FPGA
> Manager",
> -					   &socfpga_fpga_ops, priv);
> -		if (!mgr)
> -			return -ENOMEM;
> -
> -		platform_set_drvdata(pdev, mgr);
> -
> -		return fpga_mgr_register(mgr);
> -	}
> -
> -	static int socfpga_fpga_remove(struct platform_device *pdev)
> -	{
> -		struct fpga_manager *mgr = platform_get_drvdata(pdev);
> -
> -		fpga_mgr_unregister(mgr);
> -
> -		return 0;
> +		mgr = devm_fpga_mgr_register_simple(dev, "Altera SOCFPGA
> FPGA Manager",
> +						    &socfpga_fpga_ops, priv);
> +		return PTR_ERR_OR_ZERO(mgr);
>  	}
> 
> 
> @@ -104,8 +91,10 @@ API for implementing a new FPGA Manager driver
>  * ``fpga_mgr_states`` -  Values for :c:expr:`fpga_manager->state`.
>  * struct fpga_manager -  the FPGA manager struct
>  * struct fpga_manager_ops -  Low level FPGA manager driver ops
> -* devm_fpga_mgr_create() -  Allocate and init a manager struct
> -* fpga_mgr_register() -  Register an FPGA manager
> +* fpga_mgr_register() -  Create and register an FPGA manager
> +* fpga_mgr_register_simple() -  Create and register an FPGA manager
> +* devm_fpga_mgr_register() -  Create and register an FPGA manager
> +* devm_fpga_mgr_register_simple() -  Create and register an FPGA manager
>  * fpga_mgr_unregister() -  Unregister an FPGA manager
> 
>  .. kernel-doc:: include/linux/fpga/fpga-mgr.h
> @@ -118,10 +107,16 @@ API for implementing a new FPGA Manager driver
>     :functions: fpga_manager_ops
> 
>  .. kernel-doc:: drivers/fpga/fpga-mgr.c
> -   :functions: devm_fpga_mgr_create
> +   :functions: fpga_mgr_register
> 
>  .. kernel-doc:: drivers/fpga/fpga-mgr.c
> -   :functions: fpga_mgr_register
> +   :functions: fpga_mgr_register_simple
> +
> +.. kernel-doc:: drivers/fpga/fpga-mgr.c
> +   :functions: devm_fpga_mgr_register
> +
> +.. kernel-doc:: drivers/fpga/fpga-mgr.c
> +   :functions: devm_fpga_mgr_register_simple
> 
>  .. kernel-doc:: drivers/fpga/fpga-mgr.c
>     :functions: fpga_mgr_unregister
> diff --git a/drivers/fpga/altera-cvp.c b/drivers/fpga/altera-cvp.c
> index ccf4546eff29..ddf2ffe3f138 100644
> --- a/drivers/fpga/altera-cvp.c
> +++ b/drivers/fpga/altera-cvp.c
> @@ -652,19 +652,15 @@ static int altera_cvp_probe(struct pci_dev *pdev,
>  	snprintf(conf->mgr_name, sizeof(conf->mgr_name), "%s @%s",
>  		 ALTERA_CVP_MGR_NAME, pci_name(pdev));
> 
> -	mgr = devm_fpga_mgr_create(&pdev->dev, conf->mgr_name,
> -				   &altera_cvp_ops, conf);
> -	if (!mgr) {
> -		ret = -ENOMEM;
> +	mgr = fpga_mgr_register_simple(&pdev->dev, conf->mgr_name,
> +				       &altera_cvp_ops, conf);
> +	if (IS_ERR(mgr)) {
> +		ret = PTR_ERR(mgr);
>  		goto err_unmap;
>  	}
> 
>  	pci_set_drvdata(pdev, mgr);
> 
> -	ret = fpga_mgr_register(mgr);
> -	if (ret)
> -		goto err_unmap;
> -
>  	return 0;
> 
>  err_unmap:
> diff --git a/drivers/fpga/altera-pr-ip-core.c b/drivers/fpga/altera-pr-ip-core.c
> index dfdf21ed34c4..e0baf3392b67 100644
> --- a/drivers/fpga/altera-pr-ip-core.c
> +++ b/drivers/fpga/altera-pr-ip-core.c
> @@ -191,11 +191,9 @@ int alt_pr_register(struct device *dev, void __iomem
> *reg_base)
>  		(val & ALT_PR_CSR_STATUS_MSK) >> ALT_PR_CSR_STATUS_SFT,
>  		(int)(val & ALT_PR_CSR_PR_START));
> 
> -	mgr = devm_fpga_mgr_create(dev, dev_name(dev), &alt_pr_ops, priv);
> -	if (!mgr)
> -		return -ENOMEM;
> -
> -	return devm_fpga_mgr_register(dev, mgr);
> +	mgr = devm_fpga_mgr_register_simple(dev, dev_name(dev),
> +					    &alt_pr_ops, priv);
> +	return PTR_ERR_OR_ZERO(mgr);
>  }
>  EXPORT_SYMBOL_GPL(alt_pr_register);
> 
> diff --git a/drivers/fpga/altera-ps-spi.c b/drivers/fpga/altera-ps-spi.c
> index 23bfd4d1ad0f..f544de2a9f6b 100644
> --- a/drivers/fpga/altera-ps-spi.c
> +++ b/drivers/fpga/altera-ps-spi.c
> @@ -302,12 +302,9 @@ static int altera_ps_probe(struct spi_device *spi)
>  	snprintf(conf->mgr_name, sizeof(conf->mgr_name), "%s %s",
>  		 dev_driver_string(&spi->dev), dev_name(&spi->dev));
> 
> -	mgr = devm_fpga_mgr_create(&spi->dev, conf->mgr_name,
> -				   &altera_ps_ops, conf);
> -	if (!mgr)
> -		return -ENOMEM;
> -
> -	return devm_fpga_mgr_register(&spi->dev, mgr);
> +	mgr = devm_fpga_mgr_register_simple(&spi->dev, conf->mgr_name,
> +					    &altera_ps_ops, conf);
> +	return PTR_ERR_OR_ZERO(mgr);
>  }
> 
>  static const struct spi_device_id altera_ps_spi_ids[] = {
> diff --git a/drivers/fpga/dfl-fme-mgr.c b/drivers/fpga/dfl-fme-mgr.c
> index d5861d13b306..20b1cf3d957f 100644
> --- a/drivers/fpga/dfl-fme-mgr.c
> +++ b/drivers/fpga/dfl-fme-mgr.c
> @@ -282,7 +282,7 @@ static void fme_mgr_get_compat_id(void __iomem
> *fme_pr,
>  static int fme_mgr_probe(struct platform_device *pdev)
>  {
>  	struct dfl_fme_mgr_pdata *pdata = dev_get_platdata(&pdev->dev);
> -	struct fpga_compat_id *compat_id;
> +	struct fpga_manager_info info = { 0 };
>  	struct device *dev = &pdev->dev;
>  	struct fme_mgr_priv *priv;
>  	struct fpga_manager *mgr;
> @@ -302,20 +302,16 @@ static int fme_mgr_probe(struct platform_device
> *pdev)
>  			return PTR_ERR(priv->ioaddr);
>  	}
> 
> -	compat_id = devm_kzalloc(dev, sizeof(*compat_id), GFP_KERNEL);
> -	if (!compat_id)
> +	info.name = "DFL FME FPGA Manager";
> +	info.mops = &fme_mgr_ops;
> +	info.priv = priv;
> +	info.compat_id = devm_kzalloc(dev, sizeof(*info.compat_id),
> GFP_KERNEL);
> +	if (!info.compat_id)
>  		return -ENOMEM;
> 
> -	fme_mgr_get_compat_id(priv->ioaddr, compat_id);
> -
> -	mgr = devm_fpga_mgr_create(dev, "DFL FME FPGA Manager",
> -				   &fme_mgr_ops, priv);
> -	if (!mgr)
> -		return -ENOMEM;
> -
> -	mgr->compat_id = compat_id;
> -
> -	return devm_fpga_mgr_register(dev, mgr);
> +	fme_mgr_get_compat_id(priv->ioaddr, info.compat_id);
> +	mgr = devm_fpga_mgr_register(dev, &info);
> +	return PTR_ERR_OR_ZERO(mgr);
>  }
> 
>  static struct platform_driver fme_mgr_driver = {
> diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c
> index ecb4c3c795fa..6bd65a1b84fb 100644
> --- a/drivers/fpga/fpga-mgr.c
> +++ b/drivers/fpga/fpga-mgr.c
> @@ -550,21 +550,19 @@ void fpga_mgr_unlock(struct fpga_manager *mgr)
>  EXPORT_SYMBOL_GPL(fpga_mgr_unlock);
> 
>  /**
> - * fpga_mgr_create - create and initialize an FPGA manager struct
> + * fpga_mgr_register - create and register an FPGA Manager device
>   * @parent:	fpga manager device from pdev
> - * @name:	fpga manager name
> - * @mops:	pointer to structure of fpga manager ops
> - * @priv:	fpga manager private data
> + * @info:	parameters for fpga manager
>   *
> - * The caller of this function is responsible for freeing the struct with
> - * fpga_mgr_free().  Using devm_fpga_mgr_create() instead is recommended.
> + * The caller of this function is responsible for calling fpga_mgr_unregister().
> + * Using devm_fpga_mgr_register() instead is recommended.
>   *
> - * Return: pointer to struct fpga_manager or NULL
> + * Return: pointer to struct fpga_manager pointer or ERR_PTR()
>   */
> -struct fpga_manager *fpga_mgr_create(struct device *parent, const char
> *name,
> -				     const struct fpga_manager_ops *mops,
> -				     void *priv)
> +struct fpga_manager *
> +fpga_mgr_register(struct device *parent, const struct fpga_manager_info
> *info)
>  {
> +	const struct fpga_manager_ops *mops = info->mops;
>  	struct fpga_manager *mgr;
>  	int id, ret;
> 
> @@ -572,29 +570,31 @@ struct fpga_manager *fpga_mgr_create(struct device
> *parent, const char *name,
>  	    !mops->write_init || (!mops->write && !mops->write_sg) ||
>  	    (mops->write && mops->write_sg)) {
>  		dev_err(parent, "Attempt to register without
> fpga_manager_ops\n");
> -		return NULL;
> +		return ERR_PTR(-EINVAL);
>  	}
> 
> -	if (!name || !strlen(name)) {
> +	if (!info->name || !strlen(info->name)) {
>  		dev_err(parent, "Attempt to register with no name!\n");
> -		return NULL;
> +		return ERR_PTR(-EINVAL);
>  	}
> 
>  	mgr = kzalloc(sizeof(*mgr), GFP_KERNEL);
>  	if (!mgr)
> -		return NULL;
> +		return ERR_PTR(-ENOMEM);
> 
>  	id = ida_simple_get(&fpga_mgr_ida, 0, 0, GFP_KERNEL);
> -	if (id < 0)
> +	if (id < 0) {
> +		ret = id;
>  		goto error_kfree;
> +	}
> 
>  	mutex_init(&mgr->ref_mutex);
> 
> -	mgr->name = name;
> -	mgr->mops = mops;
> -	mgr->priv = priv;
> +	mgr->name = info->name;
> +	mgr->mops = info->mops;
> +	mgr->priv = info->priv;
> +	mgr->compat_id = info->compat_id;
> 
> -	device_initialize(&mgr->dev);
>  	mgr->dev.class = fpga_mgr_class;
>  	mgr->dev.groups = mops->groups;
>  	mgr->dev.parent = parent;
> @@ -605,6 +605,19 @@ struct fpga_manager *fpga_mgr_create(struct device
> *parent, const char *name,
>  	if (ret)
>  		goto error_device;
> 
> +	/*
> +	 * Initialize framework state by requesting low level driver read state
> +	 * from device.  FPGA may be in reset mode or may have been
> programmed
> +	 * by bootloader or EEPROM.
> +	 */
> +	mgr->state = mgr->mops->state(mgr);
> +
> +	ret = device_register(&mgr->dev);
> +	if (ret) {
> +		put_device(&mgr->dev);
> +		return ERR_PTR(ret);
> +	}
> +
>  	return mgr;
> 
>  error_device:
> @@ -612,98 +625,38 @@ struct fpga_manager *fpga_mgr_create(struct device
> *parent, const char *name,
>  error_kfree:
>  	kfree(mgr);
> 
> -	return NULL;
> -}
> -EXPORT_SYMBOL_GPL(fpga_mgr_create);
> -
> -/**
> - * fpga_mgr_free - free an FPGA manager created with fpga_mgr_create()
> - * @mgr:	fpga manager struct
> - */
> -void fpga_mgr_free(struct fpga_manager *mgr)
> -{
> -	ida_simple_remove(&fpga_mgr_ida, mgr->dev.id);
> -	kfree(mgr);
> -}
> -EXPORT_SYMBOL_GPL(fpga_mgr_free);
> -
> -static void devm_fpga_mgr_release(struct device *dev, void *res)
> -{
> -	struct fpga_mgr_devres *dr = res;
> -
> -	fpga_mgr_free(dr->mgr);
> +	return ERR_PTR(ret);
>  }
> +EXPORT_SYMBOL_GPL(fpga_mgr_register);
> 
>  /**
> - * devm_fpga_mgr_create - create and initialize a managed FPGA manager
> struct
> + * fpga_mgr_register_simple - create and register an FPGA Manager device
>   * @parent:	fpga manager device from pdev
>   * @name:	fpga manager name
>   * @mops:	pointer to structure of fpga manager ops
>   * @priv:	fpga manager private data
>   *
> - * This function is intended for use in an FPGA manager driver's probe function.
> - * After the manager driver creates the manager struct with
> - * devm_fpga_mgr_create(), it should register it with fpga_mgr_register().  The
> - * manager driver's remove function should call fpga_mgr_unregister().  The
> - * manager struct allocated with this function will be freed automatically on
> - * driver detach.  This includes the case of a probe function returning error
> - * before calling fpga_mgr_register(), the struct will still get cleaned up.
> + * The caller of this function is responsible for calling fpga_mgr_unregister().
> + * Using devm_fpga_mgr_register_simple() instead is recommended. This
> simple
> + * version of the register function should be sufficient for most users. The
> + * fpga_mgr_register() function is available for users that need to pass
> + * additional, optional parameters.
>   *
> - * Return: pointer to struct fpga_manager or NULL
> + * Return: pointer to struct fpga_manager pointer or ERR_PTR()
>   */
> -struct fpga_manager *devm_fpga_mgr_create(struct device *parent, const
> char *name,
> -					  const struct fpga_manager_ops
> *mops,
> -					  void *priv)
> +struct fpga_manager *
> +fpga_mgr_register_simple(struct device *parent, const char *name,
> +			 const struct fpga_manager_ops *mops, void *priv)
>  {
> -	struct fpga_mgr_devres *dr;
> +	struct fpga_manager_info info = { 0 };
> 
> -	dr = devres_alloc(devm_fpga_mgr_release, sizeof(*dr), GFP_KERNEL);
> -	if (!dr)
> -		return NULL;
> -
> -	dr->mgr = fpga_mgr_create(parent, name, mops, priv);
> -	if (!dr->mgr) {
> -		devres_free(dr);
> -		return NULL;
> -	}
> -
> -	devres_add(parent, dr);
> +	info.name = name;
> +	info.mops = mops;
> +	info.priv = priv;
> 
> -	return dr->mgr;
> +	return fpga_mgr_register(parent, &info);
>  }
> -EXPORT_SYMBOL_GPL(devm_fpga_mgr_create);
> -
> -/**
> - * fpga_mgr_register - register an FPGA manager
> - * @mgr: fpga manager struct
> - *
> - * Return: 0 on success, negative error code otherwise.
> - */
> -int fpga_mgr_register(struct fpga_manager *mgr)
> -{
> -	int ret;
> -
> -	/*
> -	 * Initialize framework state by requesting low level driver read state
> -	 * from device.  FPGA may be in reset mode or may have been
> programmed
> -	 * by bootloader or EEPROM.
> -	 */
> -	mgr->state = mgr->mops->state(mgr);
> -
> -	ret = device_add(&mgr->dev);
> -	if (ret)
> -		goto error_device;
> -
> -	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);
> +EXPORT_SYMBOL_GPL(fpga_mgr_register_simple);
> 
>  /**
>   * fpga_mgr_unregister - unregister an FPGA manager
> @@ -726,14 +679,6 @@ void fpga_mgr_unregister(struct fpga_manager *mgr)
>  }
>  EXPORT_SYMBOL_GPL(fpga_mgr_unregister);
> 
> -static int fpga_mgr_devres_match(struct device *dev, void *res,
> -				 void *match_data)
> -{
> -	struct fpga_mgr_devres *dr = res;
> -
> -	return match_data == dr->mgr;
> -}
> -
>  static void devm_fpga_mgr_unregister(struct device *dev, void *res)
>  {
>  	struct fpga_mgr_devres *dr = res;
> @@ -743,44 +688,67 @@ static void devm_fpga_mgr_unregister(struct device
> *dev, void *res)
> 
>  /**
>   * devm_fpga_mgr_register - resource managed variant of fpga_mgr_register()
> - * @dev: managing device for this FPGA manager
> - * @mgr: fpga manager struct
> + * @parent:	fpga manager device from pdev
> + * @info:	parameters for fpga manager
>   *
>   * This is the devres variant of fpga_mgr_register() for which the unregister
>   * function will be called automatically when the managing device is detached.
>   */
> -int devm_fpga_mgr_register(struct device *dev, struct fpga_manager *mgr)
> +struct fpga_manager *
> +devm_fpga_mgr_register(struct device *parent, const struct
> fpga_manager_info *info)
>  {
>  	struct fpga_mgr_devres *dr;
> -	int ret;
> -
> -	/*
> -	 * Make sure that the struct fpga_manager * that is passed in is
> -	 * managed itself.
> -	 */
> -	if (WARN_ON(!devres_find(dev, devm_fpga_mgr_release,
> -				 fpga_mgr_devres_match, mgr)))
> -		return -EINVAL;
> +	struct fpga_manager *mgr;
> 
>  	dr = devres_alloc(devm_fpga_mgr_unregister, sizeof(*dr), GFP_KERNEL);
>  	if (!dr)
> -		return -ENOMEM;
> +		return ERR_PTR(-ENOMEM);
> 
> -	ret = fpga_mgr_register(mgr);
> -	if (ret) {
> +	mgr = fpga_mgr_register(parent, info);
> +	if (IS_ERR(mgr)) {
>  		devres_free(dr);
> -		return ret;
> +		return mgr;
>  	}
> 
>  	dr->mgr = mgr;
> -	devres_add(dev, dr);
> +	devres_add(parent, dr);
> 
> -	return 0;
> +	return mgr;
>  }
>  EXPORT_SYMBOL_GPL(devm_fpga_mgr_register);
> 
> +/**
> + * devm_fpga_mgr_register_simple - resource managed variant of
> + *				   fpga_mgr_register_simple()
> + * @parent:	fpga manager device from pdev
> + * @name:	fpga manager name
> + * @mops:	pointer to structure of fpga manager ops
> + * @priv:	fpga manager private data
> + *
> + * This is the devres variant of fpga_mgr_register_simple() for which the
> + * unregister function will be called automatically when the managing
> + * device is detached.
> + */
> +struct fpga_manager *
> +devm_fpga_mgr_register_simple(struct device *parent, const char *name,
> +			      const struct fpga_manager_ops *mops, void *priv)
> +{
> +	struct fpga_manager_info info = { 0 };
> +
> +	info.name = name;
> +	info.mops = mops;
> +	info.priv = priv;
> +
> +	return devm_fpga_mgr_register(parent, &info);
> +}
> +EXPORT_SYMBOL_GPL(devm_fpga_mgr_register_simple);
> +
>  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)
> diff --git a/drivers/fpga/ice40-spi.c b/drivers/fpga/ice40-spi.c
> index 69dec5af23c3..448eb6f4e509 100644
> --- a/drivers/fpga/ice40-spi.c
> +++ b/drivers/fpga/ice40-spi.c
> @@ -178,12 +178,9 @@ static int ice40_fpga_probe(struct spi_device *spi)
>  		return ret;
>  	}
> 
> -	mgr = devm_fpga_mgr_create(dev, "Lattice iCE40 FPGA Manager",
> -				   &ice40_fpga_ops, priv);
> -	if (!mgr)
> -		return -ENOMEM;
> -
> -	return devm_fpga_mgr_register(dev, mgr);
> +	mgr = devm_fpga_mgr_register_simple(dev, "Lattice iCE40 FPGA
> Manager",
> +					    &ice40_fpga_ops, priv);
> +	return PTR_ERR_OR_ZERO(mgr);
>  }
> 
>  static const struct of_device_id ice40_fpga_of_match[] = {
> diff --git a/drivers/fpga/machxo2-spi.c b/drivers/fpga/machxo2-spi.c
> index 114a64d2b7a4..77be11155402 100644
> --- a/drivers/fpga/machxo2-spi.c
> +++ b/drivers/fpga/machxo2-spi.c
> @@ -366,12 +366,10 @@ static int machxo2_spi_probe(struct spi_device *spi)
>  		return -EINVAL;
>  	}
> 
> -	mgr = devm_fpga_mgr_create(dev, "Lattice MachXO2 SPI FPGA
> Manager",
> -				   &machxo2_ops, spi);
> -	if (!mgr)
> -		return -ENOMEM;
> -
> -	return devm_fpga_mgr_register(dev, mgr);
> +	mgr = devm_fpga_mgr_register_simple(dev,
> +					    "Lattice MachXO2 SPI FPGA
> Manager",
> +					    &machxo2_ops, spi);
> +	return PTR_ERR_OR_ZERO(mgr);
>  }
> 
>  static const struct of_device_id of_match[] = {
> diff --git a/drivers/fpga/socfpga-a10.c b/drivers/fpga/socfpga-a10.c
> index 573d88bdf730..5ffefaa3eb07 100644
> --- a/drivers/fpga/socfpga-a10.c
> +++ b/drivers/fpga/socfpga-a10.c
> @@ -508,19 +508,15 @@ static int socfpga_a10_fpga_probe(struct
> platform_device *pdev)
>  		return -EBUSY;
>  	}
> 
> -	mgr = devm_fpga_mgr_create(dev, "SoCFPGA Arria10 FPGA Manager",
> -				   &socfpga_a10_fpga_mgr_ops, priv);
> -	if (!mgr)
> -		return -ENOMEM;
> -
> -	platform_set_drvdata(pdev, mgr);
> -
> -	ret = fpga_mgr_register(mgr);
> -	if (ret) {
> +	mgr = fpga_mgr_register_simple(dev, "SoCFPGA Arria10 FPGA Manager",
> +				       &socfpga_a10_fpga_mgr_ops, priv);
> +	if (IS_ERR(mgr)) {
>  		clk_disable_unprepare(priv->clk);
> -		return ret;
> +		return PTR_ERR(mgr);
>  	}
> 
> +	platform_set_drvdata(pdev, mgr);
> +
>  	return 0;
>  }
> 
> diff --git a/drivers/fpga/socfpga.c b/drivers/fpga/socfpga.c
> index 1f467173fc1f..9280db4364d4 100644
> --- a/drivers/fpga/socfpga.c
> +++ b/drivers/fpga/socfpga.c
> @@ -571,12 +571,9 @@ static int socfpga_fpga_probe(struct platform_device
> *pdev)
>  	if (ret)
>  		return ret;
> 
> -	mgr = devm_fpga_mgr_create(dev, "Altera SOCFPGA FPGA Manager",
> -				   &socfpga_fpga_ops, priv);
> -	if (!mgr)
> -		return -ENOMEM;
> -
> -	return devm_fpga_mgr_register(dev, mgr);
> +	mgr = devm_fpga_mgr_register_simple(dev, "Altera SOCFPGA FPGA
> Manager",
> +					    &socfpga_fpga_ops, priv);
> +	return PTR_ERR_OR_ZERO(mgr);
>  }
> 
>  #ifdef CONFIG_OF
> diff --git a/drivers/fpga/stratix10-soc.c b/drivers/fpga/stratix10-soc.c
> index a2cea500f7cc..fb84d88d4ce9 100644
> --- a/drivers/fpga/stratix10-soc.c
> +++ b/drivers/fpga/stratix10-soc.c
> @@ -425,18 +425,11 @@ static int s10_probe(struct platform_device *pdev)
> 
>  	init_completion(&priv->status_return_completion);
> 
> -	mgr = fpga_mgr_create(dev, "Stratix10 SOC FPGA Manager",
> -			      &s10_ops, priv);
> -	if (!mgr) {
> -		dev_err(dev, "unable to create FPGA manager\n");
> -		ret = -ENOMEM;
> -		goto probe_err;
> -	}
> -
> -	ret = fpga_mgr_register(mgr);
> -	if (ret) {
> +	mgr = fpga_mgr_register_simple(dev, "Stratix10 SOC FPGA Manager",
> +				       &s10_ops, priv);
> +	if (IS_ERR(mgr)) {
>  		dev_err(dev, "unable to register FPGA manager\n");
> -		fpga_mgr_free(mgr);
> +		ret = PTR_ERR(mgr);
>  		goto probe_err;
>  	}
> 
> @@ -454,7 +447,6 @@ 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;
> diff --git a/drivers/fpga/ts73xx-fpga.c b/drivers/fpga/ts73xx-fpga.c
> index 101f016c6ed8..009d5cfa1074 100644
> --- a/drivers/fpga/ts73xx-fpga.c
> +++ b/drivers/fpga/ts73xx-fpga.c
> @@ -122,12 +122,9 @@ static int ts73xx_fpga_probe(struct platform_device
> *pdev)
>  	if (IS_ERR(priv->io_base))
>  		return PTR_ERR(priv->io_base);
> 
> -	mgr = devm_fpga_mgr_create(kdev, "TS-73xx FPGA Manager",
> -				   &ts73xx_fpga_ops, priv);
> -	if (!mgr)
> -		return -ENOMEM;
> -
> -	return devm_fpga_mgr_register(kdev, mgr);
> +	mgr = devm_fpga_mgr_register_simple(kdev, "TS-73xx FPGA Manager",
> +					    &ts73xx_fpga_ops, priv);
> +	return PTR_ERR_OR_ZERO(mgr);
>  }
> 
>  static struct platform_driver ts73xx_fpga_driver = {
> diff --git a/drivers/fpga/xilinx-spi.c b/drivers/fpga/xilinx-spi.c
> index fee4d0abf6bf..6f568a757cf2 100644
> --- a/drivers/fpga/xilinx-spi.c
> +++ b/drivers/fpga/xilinx-spi.c
> @@ -247,13 +247,10 @@ static int xilinx_spi_probe(struct spi_device *spi)
>  		return dev_err_probe(&spi->dev, PTR_ERR(conf->done),
>  				     "Failed to get DONE gpio\n");
> 
> -	mgr = devm_fpga_mgr_create(&spi->dev,
> -				   "Xilinx Slave Serial FPGA Manager",
> -				   &xilinx_spi_ops, conf);
> -	if (!mgr)
> -		return -ENOMEM;
> -
> -	return devm_fpga_mgr_register(&spi->dev, mgr);
> +	mgr = devm_fpga_mgr_register_simple(&spi->dev,
> +					    "Xilinx Slave Serial FPGA Manager",
> +					    &xilinx_spi_ops, conf);
> +	return PTR_ERR_OR_ZERO(mgr);
>  }
> 
>  static const struct of_device_id xlnx_spi_of_match[] = {
> diff --git a/drivers/fpga/zynq-fpga.c b/drivers/fpga/zynq-fpga.c
> index 9b75bd4f93d8..a3de365aadc6 100644
> --- a/drivers/fpga/zynq-fpga.c
> +++ b/drivers/fpga/zynq-fpga.c
> @@ -609,20 +609,16 @@ static int zynq_fpga_probe(struct platform_device
> *pdev)
> 
>  	clk_disable(priv->clk);
> 
> -	mgr = devm_fpga_mgr_create(dev, "Xilinx Zynq FPGA Manager",
> -				   &zynq_fpga_ops, priv);
> -	if (!mgr)
> -		return -ENOMEM;
> -
> -	platform_set_drvdata(pdev, mgr);
> -
> -	err = fpga_mgr_register(mgr);
> -	if (err) {
> +	mgr = fpga_mgr_register_simple(dev, "Xilinx Zynq FPGA Manager",
> +				       &zynq_fpga_ops, priv);
> +	if (IS_ERR(mgr)) {
>  		dev_err(dev, "unable to register FPGA manager\n");
>  		clk_unprepare(priv->clk);
> -		return err;
> +		return PTR_ERR(mgr);
>  	}
> 
> +	platform_set_drvdata(pdev, mgr);
> +
>  	return 0;
>  }
> 
> diff --git a/drivers/fpga/zynqmp-fpga.c b/drivers/fpga/zynqmp-fpga.c
> index 125743c9797f..53a7b36f61e8 100644
> --- a/drivers/fpga/zynqmp-fpga.c
> +++ b/drivers/fpga/zynqmp-fpga.c
> @@ -102,12 +102,9 @@ static int zynqmp_fpga_probe(struct platform_device
> *pdev)
> 
>  	priv->dev = dev;
> 
> -	mgr = devm_fpga_mgr_create(dev, "Xilinx ZynqMP FPGA Manager",
> -				   &zynqmp_fpga_ops, priv);
> -	if (!mgr)
> -		return -ENOMEM;
> -
> -	return devm_fpga_mgr_register(dev, mgr);
> +	mgr = devm_fpga_mgr_register_simple(dev, "Xilinx ZynqMP FPGA
> Manager",
> +					    &zynqmp_fpga_ops, priv);
> +	return PTR_ERR_OR_ZERO(mgr);
>  }
> 
>  static const struct of_device_id zynqmp_fpga_of_match[] = {
> diff --git a/include/linux/fpga/fpga-mgr.h b/include/linux/fpga/fpga-mgr.h
> index 474c1f506307..9efe55be93bc 100644
> --- a/include/linux/fpga/fpga-mgr.h
> +++ b/include/linux/fpga/fpga-mgr.h
> @@ -105,6 +105,36 @@ struct fpga_image_info {
>  #endif
>  };
> 
> +/**
> + * struct fpga_compat_id - id for compatibility check
> + *
> + * @id_h: high 64bit of the compat_id
> + * @id_l: low 64bit of the compat_id
> + */
> +struct fpga_compat_id {
> +	u64 id_h;
> +	u64 id_l;
> +};
> +
> +/**
> + * struct fpga_manager_info - collection of parameters for an FPGA Manager
> + * @name: fpga manager name
> + * @compat_id: FPGA manager id for compatibility check.
> + * @mops: pointer to structure of fpga manager ops
> + * @priv: fpga manager private data
> + *
> + * fpga_manager_info contains parameters for the register function. These
> + * are separated into an info structure because they some are optional
> + * others could be added to in the future. The info structure facilitates
> + * maintaining a stable API.
> + */
> +struct fpga_manager_info {
> +	const char *name;
> +	struct fpga_compat_id *compat_id;
> +	const struct fpga_manager_ops *mops;
> +	void *priv;
> +};
> +
>  /**
>   * struct fpga_manager_ops - ops for low level fpga manager drivers
>   * @initial_header_size: Maximum number of bytes that should be passed into
> write_init
> @@ -143,17 +173,6 @@ struct fpga_manager_ops {
>  #define FPGA_MGR_STATUS_IP_PROTOCOL_ERR		BIT(3)
>  #define FPGA_MGR_STATUS_FIFO_OVERFLOW_ERR	BIT(4)
> 
> -/**
> - * struct fpga_compat_id - id for compatibility check
> - *
> - * @id_h: high 64bit of the compat_id
> - * @id_l: low 64bit of the compat_id
> - */
> -struct fpga_compat_id {
> -	u64 id_h;
> -	u64 id_l;
> -};
> -
>  /**
>   * struct fpga_manager - fpga manager structure
>   * @name: name of low level fpga manager
> @@ -191,17 +210,18 @@ struct fpga_manager *fpga_mgr_get(struct device
> *dev);
> 
>  void fpga_mgr_put(struct fpga_manager *mgr);
> 
> -struct fpga_manager *fpga_mgr_create(struct device *dev, const char *name,
> -				     const struct fpga_manager_ops *mops,
> -				     void *priv);
> -void fpga_mgr_free(struct fpga_manager *mgr);
> -int fpga_mgr_register(struct fpga_manager *mgr);
> -void fpga_mgr_unregister(struct fpga_manager *mgr);
> +struct fpga_manager *
> +fpga_mgr_register(struct device *parent, const struct fpga_manager_info
> *info);
> 
> -int devm_fpga_mgr_register(struct device *dev, struct fpga_manager *mgr);
> +struct fpga_manager *
> +fpga_mgr_register_simple(struct device *parent, const char *name,
> +			 const struct fpga_manager_ops *mops, void *priv);
> +void fpga_mgr_unregister(struct fpga_manager *mgr);
> 
> -struct fpga_manager *devm_fpga_mgr_create(struct device *dev, const char
> *name,
> -					  const struct fpga_manager_ops
> *mops,
> -					  void *priv);
> +struct fpga_manager *
> +devm_fpga_mgr_register(struct device *parent, const struct
> fpga_manager_info *info);
> +struct fpga_manager *
> +devm_fpga_mgr_register_simple(struct device *parent, const char *name,
> +			      const struct fpga_manager_ops *mops, void *priv);
> 
>  #endif /*_LINUX_FPGA_MGR_H */
> --
> 2.25.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v8 1/3] fpga: mgr: Use standard dev_release for class driver
  2021-06-24 20:53 ` [PATCH v8 1/3] fpga: mgr: Use standard dev_release for class driver Russ Weight
  2021-06-28  3:49   ` Wu, Hao
@ 2021-06-28  4:39   ` Xu Yilun
  1 sibling, 0 replies; 10+ messages in thread
From: Xu Yilun @ 2021-06-28  4:39 UTC (permalink / raw)
  To: Russ Weight
  Cc: mdf, linux-fpga, trix, lgoncalv, hao.wu, matthew.gerlach, richard.gong

On Thu, Jun 24, 2021 at 01:53:53PM -0700, Russ Weight wrote:
> The FPGA manager class driver data structure is being treated as a
> managed resource instead of using the standard dev_release call-back
> function to release the class data structure. This change removes
> the managed resource code for the freeing of the class data structure
> and combines the create() and register() functions into a single
> register() or register_simple() function.
> 
> The register() function accepts an info data structure to provide
> flexibility in passing optional parameters. The register_simple()
> function supports the current parameter list for users that don't
> require the use of optional parameters.
> 
> The devm_fpga_mgr_register() function is retained, and the
> devm_fpga_mgr_register_simple() function is added.
> 
> Signed-off-by: Russ Weight <russell.h.weight@intel.com>
> Reviewed-by: Xu Yilun <yilun.xu@intel.com>
> ---
> v8:
>   - Added reviewed-by tag.
>   - Updated Documentation/driver-api/fpga/fpga-mgr.rst documentation.
> v7:
>   - Update the commit message to describe the new parameters for the
>     *fpga_mgr_register() functions and to mention the
>     *fpga_mgr_register_simple() functions.
>   - Fix function prototypes in header file to rename dev to parent.
>   - Make use of the PTR_ERR_OR_ZERO() macro when possible.
>   - Some cleanup of comments.
>   - Update function defintions/prototypes to apply const to the new info
>     parameter.
> v6:
>   - Moved FPGA manager optional parameters out of the ops structure and
>     back into the FPGA manager structure.
>   - Changed fpga_mgr_register()/devm_fpga_mgr_register() parameters to
>     accept an info data structure to provide flexibility in passing optional
>     parameters.
>   - Added fpga_mgr_register_simple()/devm_fpga_mgr_register_simple()
>     functions to support current parameters for users that don't require
>     the use of optional parameters.
> v5:
>   - Rebased on top of recently accepted patches.
>   - Removed compat_id from the fpga_mgr_register() parameter list
>     and added it to the fpga_manager_ops structure. This also required
>     dynamically allocating the dfl-fme-ops structure in order to add
>     the appropriate compat_id.
> v4:
>   - Added the compat_id parameter to fpga_mgr_register() and
>     devm_fpga_mgr_register() to ensure that the compat_id is set before
>     the device_register() call.
> v3:
>   - Cleaned up comment header for fpga_mgr_register()
>   - Fix error return on ida_simple_get() failure
> v2:
>   - Restored devm_fpga_mgr_register() functionality, adapted for the combined
>     create/register functionality.
>   - All previous callers of devm_fpga_mgr_register() will continue to call
>     devm_fpga_mgr_register().
>   - replaced unnecessary ternary operators in return statements with standard
>     if conditions.
> ---
>  Documentation/driver-api/fpga/fpga-mgr.rst |  39 ++--
>  drivers/fpga/altera-cvp.c                  |  12 +-
>  drivers/fpga/altera-pr-ip-core.c           |   8 +-
>  drivers/fpga/altera-ps-spi.c               |   9 +-
>  drivers/fpga/dfl-fme-mgr.c                 |  22 +--
>  drivers/fpga/fpga-mgr.c                    | 214 +++++++++------------
>  drivers/fpga/ice40-spi.c                   |   9 +-
>  drivers/fpga/machxo2-spi.c                 |  10 +-
>  drivers/fpga/socfpga-a10.c                 |  16 +-
>  drivers/fpga/socfpga.c                     |   9 +-
>  drivers/fpga/stratix10-soc.c               |  16 +-
>  drivers/fpga/ts73xx-fpga.c                 |   9 +-
>  drivers/fpga/xilinx-spi.c                  |  11 +-
>  drivers/fpga/zynq-fpga.c                   |  16 +-
>  drivers/fpga/zynqmp-fpga.c                 |   9 +-
>  include/linux/fpga/fpga-mgr.h              |  62 ++++--
>  16 files changed, 204 insertions(+), 267 deletions(-)
> 
> diff --git a/Documentation/driver-api/fpga/fpga-mgr.rst b/Documentation/driver-api/fpga/fpga-mgr.rst
> index 4d926b452cb3..fbb7f07cae9f 100644
> --- a/Documentation/driver-api/fpga/fpga-mgr.rst
> +++ b/Documentation/driver-api/fpga/fpga-mgr.rst
> @@ -24,7 +24,8 @@ How to support a new FPGA device
>  --------------------------------
>  
>  To add another FPGA manager, write a driver that implements a set of ops.  The
> -probe function calls fpga_mgr_register(), such as::
> +probe function calls devm_fpga_mgr_register() or
> +devm_fpga_mgr_register_simple(), such as::

It makes me feel the devm functions are the only choice.

Is it better we describe the non-devm functions first, which is fundamental
to developers. Then introduce the devm versions and give the example.

Thanks,
Yilun

>  
>  	static const struct fpga_manager_ops socfpga_fpga_ops = {
>  		.write_init = socfpga_fpga_ops_configure_init,
> @@ -49,23 +50,9 @@ probe function calls fpga_mgr_register(), such as::
>  		 * them in priv
>  		 */
>  
> -		mgr = devm_fpga_mgr_create(dev, "Altera SOCFPGA FPGA Manager",
> -					   &socfpga_fpga_ops, priv);
> -		if (!mgr)
> -			return -ENOMEM;
> -
> -		platform_set_drvdata(pdev, mgr);
> -
> -		return fpga_mgr_register(mgr);
> -	}
> -
> -	static int socfpga_fpga_remove(struct platform_device *pdev)
> -	{
> -		struct fpga_manager *mgr = platform_get_drvdata(pdev);
> -
> -		fpga_mgr_unregister(mgr);
> -
> -		return 0;
> +		mgr = devm_fpga_mgr_register_simple(dev, "Altera SOCFPGA FPGA Manager",
> +						    &socfpga_fpga_ops, priv);
> +		return PTR_ERR_OR_ZERO(mgr);
>  	}
>  
>  
> @@ -104,8 +91,10 @@ API for implementing a new FPGA Manager driver
>  * ``fpga_mgr_states`` -  Values for :c:expr:`fpga_manager->state`.
>  * struct fpga_manager -  the FPGA manager struct
>  * struct fpga_manager_ops -  Low level FPGA manager driver ops
> -* devm_fpga_mgr_create() -  Allocate and init a manager struct
> -* fpga_mgr_register() -  Register an FPGA manager
> +* fpga_mgr_register() -  Create and register an FPGA manager
> +* fpga_mgr_register_simple() -  Create and register an FPGA manager
> +* devm_fpga_mgr_register() -  Create and register an FPGA manager
> +* devm_fpga_mgr_register_simple() -  Create and register an FPGA manager
>  * fpga_mgr_unregister() -  Unregister an FPGA manager
>  
>  .. kernel-doc:: include/linux/fpga/fpga-mgr.h
> @@ -118,10 +107,16 @@ API for implementing a new FPGA Manager driver
>     :functions: fpga_manager_ops
>  
>  .. kernel-doc:: drivers/fpga/fpga-mgr.c
> -   :functions: devm_fpga_mgr_create
> +   :functions: fpga_mgr_register
>  
>  .. kernel-doc:: drivers/fpga/fpga-mgr.c
> -   :functions: fpga_mgr_register
> +   :functions: fpga_mgr_register_simple
> +
> +.. kernel-doc:: drivers/fpga/fpga-mgr.c
> +   :functions: devm_fpga_mgr_register
> +
> +.. kernel-doc:: drivers/fpga/fpga-mgr.c
> +   :functions: devm_fpga_mgr_register_simple
>  
>  .. kernel-doc:: drivers/fpga/fpga-mgr.c
>     :functions: fpga_mgr_unregister
> diff --git a/drivers/fpga/altera-cvp.c b/drivers/fpga/altera-cvp.c
> index ccf4546eff29..ddf2ffe3f138 100644
> --- a/drivers/fpga/altera-cvp.c
> +++ b/drivers/fpga/altera-cvp.c
> @@ -652,19 +652,15 @@ static int altera_cvp_probe(struct pci_dev *pdev,
>  	snprintf(conf->mgr_name, sizeof(conf->mgr_name), "%s @%s",
>  		 ALTERA_CVP_MGR_NAME, pci_name(pdev));
>  
> -	mgr = devm_fpga_mgr_create(&pdev->dev, conf->mgr_name,
> -				   &altera_cvp_ops, conf);
> -	if (!mgr) {
> -		ret = -ENOMEM;
> +	mgr = fpga_mgr_register_simple(&pdev->dev, conf->mgr_name,
> +				       &altera_cvp_ops, conf);
> +	if (IS_ERR(mgr)) {
> +		ret = PTR_ERR(mgr);
>  		goto err_unmap;
>  	}
>  
>  	pci_set_drvdata(pdev, mgr);
>  
> -	ret = fpga_mgr_register(mgr);
> -	if (ret)
> -		goto err_unmap;
> -
>  	return 0;
>  
>  err_unmap:
> diff --git a/drivers/fpga/altera-pr-ip-core.c b/drivers/fpga/altera-pr-ip-core.c
> index dfdf21ed34c4..e0baf3392b67 100644
> --- a/drivers/fpga/altera-pr-ip-core.c
> +++ b/drivers/fpga/altera-pr-ip-core.c
> @@ -191,11 +191,9 @@ int alt_pr_register(struct device *dev, void __iomem *reg_base)
>  		(val & ALT_PR_CSR_STATUS_MSK) >> ALT_PR_CSR_STATUS_SFT,
>  		(int)(val & ALT_PR_CSR_PR_START));
>  
> -	mgr = devm_fpga_mgr_create(dev, dev_name(dev), &alt_pr_ops, priv);
> -	if (!mgr)
> -		return -ENOMEM;
> -
> -	return devm_fpga_mgr_register(dev, mgr);
> +	mgr = devm_fpga_mgr_register_simple(dev, dev_name(dev),
> +					    &alt_pr_ops, priv);
> +	return PTR_ERR_OR_ZERO(mgr);
>  }
>  EXPORT_SYMBOL_GPL(alt_pr_register);
>  
> diff --git a/drivers/fpga/altera-ps-spi.c b/drivers/fpga/altera-ps-spi.c
> index 23bfd4d1ad0f..f544de2a9f6b 100644
> --- a/drivers/fpga/altera-ps-spi.c
> +++ b/drivers/fpga/altera-ps-spi.c
> @@ -302,12 +302,9 @@ static int altera_ps_probe(struct spi_device *spi)
>  	snprintf(conf->mgr_name, sizeof(conf->mgr_name), "%s %s",
>  		 dev_driver_string(&spi->dev), dev_name(&spi->dev));
>  
> -	mgr = devm_fpga_mgr_create(&spi->dev, conf->mgr_name,
> -				   &altera_ps_ops, conf);
> -	if (!mgr)
> -		return -ENOMEM;
> -
> -	return devm_fpga_mgr_register(&spi->dev, mgr);
> +	mgr = devm_fpga_mgr_register_simple(&spi->dev, conf->mgr_name,
> +					    &altera_ps_ops, conf);
> +	return PTR_ERR_OR_ZERO(mgr);
>  }
>  
>  static const struct spi_device_id altera_ps_spi_ids[] = {
> diff --git a/drivers/fpga/dfl-fme-mgr.c b/drivers/fpga/dfl-fme-mgr.c
> index d5861d13b306..20b1cf3d957f 100644
> --- a/drivers/fpga/dfl-fme-mgr.c
> +++ b/drivers/fpga/dfl-fme-mgr.c
> @@ -282,7 +282,7 @@ static void fme_mgr_get_compat_id(void __iomem *fme_pr,
>  static int fme_mgr_probe(struct platform_device *pdev)
>  {
>  	struct dfl_fme_mgr_pdata *pdata = dev_get_platdata(&pdev->dev);
> -	struct fpga_compat_id *compat_id;
> +	struct fpga_manager_info info = { 0 };
>  	struct device *dev = &pdev->dev;
>  	struct fme_mgr_priv *priv;
>  	struct fpga_manager *mgr;
> @@ -302,20 +302,16 @@ static int fme_mgr_probe(struct platform_device *pdev)
>  			return PTR_ERR(priv->ioaddr);
>  	}
>  
> -	compat_id = devm_kzalloc(dev, sizeof(*compat_id), GFP_KERNEL);
> -	if (!compat_id)
> +	info.name = "DFL FME FPGA Manager";
> +	info.mops = &fme_mgr_ops;
> +	info.priv = priv;
> +	info.compat_id = devm_kzalloc(dev, sizeof(*info.compat_id), GFP_KERNEL);
> +	if (!info.compat_id)
>  		return -ENOMEM;
>  
> -	fme_mgr_get_compat_id(priv->ioaddr, compat_id);
> -
> -	mgr = devm_fpga_mgr_create(dev, "DFL FME FPGA Manager",
> -				   &fme_mgr_ops, priv);
> -	if (!mgr)
> -		return -ENOMEM;
> -
> -	mgr->compat_id = compat_id;
> -
> -	return devm_fpga_mgr_register(dev, mgr);
> +	fme_mgr_get_compat_id(priv->ioaddr, info.compat_id);
> +	mgr = devm_fpga_mgr_register(dev, &info);
> +	return PTR_ERR_OR_ZERO(mgr);
>  }
>  
>  static struct platform_driver fme_mgr_driver = {
> diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c
> index ecb4c3c795fa..6bd65a1b84fb 100644
> --- a/drivers/fpga/fpga-mgr.c
> +++ b/drivers/fpga/fpga-mgr.c
> @@ -550,21 +550,19 @@ void fpga_mgr_unlock(struct fpga_manager *mgr)
>  EXPORT_SYMBOL_GPL(fpga_mgr_unlock);
>  
>  /**
> - * fpga_mgr_create - create and initialize an FPGA manager struct
> + * fpga_mgr_register - create and register an FPGA Manager device
>   * @parent:	fpga manager device from pdev
> - * @name:	fpga manager name
> - * @mops:	pointer to structure of fpga manager ops
> - * @priv:	fpga manager private data
> + * @info:	parameters for fpga manager
>   *
> - * The caller of this function is responsible for freeing the struct with
> - * fpga_mgr_free().  Using devm_fpga_mgr_create() instead is recommended.
> + * The caller of this function is responsible for calling fpga_mgr_unregister().
> + * Using devm_fpga_mgr_register() instead is recommended.
>   *
> - * Return: pointer to struct fpga_manager or NULL
> + * Return: pointer to struct fpga_manager pointer or ERR_PTR()
>   */
> -struct fpga_manager *fpga_mgr_create(struct device *parent, const char *name,
> -				     const struct fpga_manager_ops *mops,
> -				     void *priv)
> +struct fpga_manager *
> +fpga_mgr_register(struct device *parent, const struct fpga_manager_info *info)
>  {
> +	const struct fpga_manager_ops *mops = info->mops;
>  	struct fpga_manager *mgr;
>  	int id, ret;
>  
> @@ -572,29 +570,31 @@ struct fpga_manager *fpga_mgr_create(struct device *parent, const char *name,
>  	    !mops->write_init || (!mops->write && !mops->write_sg) ||
>  	    (mops->write && mops->write_sg)) {
>  		dev_err(parent, "Attempt to register without fpga_manager_ops\n");
> -		return NULL;
> +		return ERR_PTR(-EINVAL);
>  	}
>  
> -	if (!name || !strlen(name)) {
> +	if (!info->name || !strlen(info->name)) {
>  		dev_err(parent, "Attempt to register with no name!\n");
> -		return NULL;
> +		return ERR_PTR(-EINVAL);
>  	}
>  
>  	mgr = kzalloc(sizeof(*mgr), GFP_KERNEL);
>  	if (!mgr)
> -		return NULL;
> +		return ERR_PTR(-ENOMEM);
>  
>  	id = ida_simple_get(&fpga_mgr_ida, 0, 0, GFP_KERNEL);
> -	if (id < 0)
> +	if (id < 0) {
> +		ret = id;
>  		goto error_kfree;
> +	}
>  
>  	mutex_init(&mgr->ref_mutex);
>  
> -	mgr->name = name;
> -	mgr->mops = mops;
> -	mgr->priv = priv;
> +	mgr->name = info->name;
> +	mgr->mops = info->mops;
> +	mgr->priv = info->priv;
> +	mgr->compat_id = info->compat_id;
>  
> -	device_initialize(&mgr->dev);
>  	mgr->dev.class = fpga_mgr_class;
>  	mgr->dev.groups = mops->groups;
>  	mgr->dev.parent = parent;
> @@ -605,6 +605,19 @@ struct fpga_manager *fpga_mgr_create(struct device *parent, const char *name,
>  	if (ret)
>  		goto error_device;
>  
> +	/*
> +	 * Initialize framework state by requesting low level driver read state
> +	 * from device.  FPGA may be in reset mode or may have been programmed
> +	 * by bootloader or EEPROM.
> +	 */
> +	mgr->state = mgr->mops->state(mgr);
> +
> +	ret = device_register(&mgr->dev);
> +	if (ret) {
> +		put_device(&mgr->dev);
> +		return ERR_PTR(ret);
> +	}
> +
>  	return mgr;
>  
>  error_device:
> @@ -612,98 +625,38 @@ struct fpga_manager *fpga_mgr_create(struct device *parent, const char *name,
>  error_kfree:
>  	kfree(mgr);
>  
> -	return NULL;
> -}
> -EXPORT_SYMBOL_GPL(fpga_mgr_create);
> -
> -/**
> - * fpga_mgr_free - free an FPGA manager created with fpga_mgr_create()
> - * @mgr:	fpga manager struct
> - */
> -void fpga_mgr_free(struct fpga_manager *mgr)
> -{
> -	ida_simple_remove(&fpga_mgr_ida, mgr->dev.id);
> -	kfree(mgr);
> -}
> -EXPORT_SYMBOL_GPL(fpga_mgr_free);
> -
> -static void devm_fpga_mgr_release(struct device *dev, void *res)
> -{
> -	struct fpga_mgr_devres *dr = res;
> -
> -	fpga_mgr_free(dr->mgr);
> +	return ERR_PTR(ret);
>  }
> +EXPORT_SYMBOL_GPL(fpga_mgr_register);
>  
>  /**
> - * devm_fpga_mgr_create - create and initialize a managed FPGA manager struct
> + * fpga_mgr_register_simple - create and register an FPGA Manager device
>   * @parent:	fpga manager device from pdev
>   * @name:	fpga manager name
>   * @mops:	pointer to structure of fpga manager ops
>   * @priv:	fpga manager private data
>   *
> - * This function is intended for use in an FPGA manager driver's probe function.
> - * After the manager driver creates the manager struct with
> - * devm_fpga_mgr_create(), it should register it with fpga_mgr_register().  The
> - * manager driver's remove function should call fpga_mgr_unregister().  The
> - * manager struct allocated with this function will be freed automatically on
> - * driver detach.  This includes the case of a probe function returning error
> - * before calling fpga_mgr_register(), the struct will still get cleaned up.
> + * The caller of this function is responsible for calling fpga_mgr_unregister().
> + * Using devm_fpga_mgr_register_simple() instead is recommended. This simple
> + * version of the register function should be sufficient for most users. The
> + * fpga_mgr_register() function is available for users that need to pass
> + * additional, optional parameters.
>   *
> - * Return: pointer to struct fpga_manager or NULL
> + * Return: pointer to struct fpga_manager pointer or ERR_PTR()
>   */
> -struct fpga_manager *devm_fpga_mgr_create(struct device *parent, const char *name,
> -					  const struct fpga_manager_ops *mops,
> -					  void *priv)
> +struct fpga_manager *
> +fpga_mgr_register_simple(struct device *parent, const char *name,
> +			 const struct fpga_manager_ops *mops, void *priv)
>  {
> -	struct fpga_mgr_devres *dr;
> +	struct fpga_manager_info info = { 0 };
>  
> -	dr = devres_alloc(devm_fpga_mgr_release, sizeof(*dr), GFP_KERNEL);
> -	if (!dr)
> -		return NULL;
> -
> -	dr->mgr = fpga_mgr_create(parent, name, mops, priv);
> -	if (!dr->mgr) {
> -		devres_free(dr);
> -		return NULL;
> -	}
> -
> -	devres_add(parent, dr);
> +	info.name = name;
> +	info.mops = mops;
> +	info.priv = priv;
>  
> -	return dr->mgr;
> +	return fpga_mgr_register(parent, &info);
>  }
> -EXPORT_SYMBOL_GPL(devm_fpga_mgr_create);
> -
> -/**
> - * fpga_mgr_register - register an FPGA manager
> - * @mgr: fpga manager struct
> - *
> - * Return: 0 on success, negative error code otherwise.
> - */
> -int fpga_mgr_register(struct fpga_manager *mgr)
> -{
> -	int ret;
> -
> -	/*
> -	 * Initialize framework state by requesting low level driver read state
> -	 * from device.  FPGA may be in reset mode or may have been programmed
> -	 * by bootloader or EEPROM.
> -	 */
> -	mgr->state = mgr->mops->state(mgr);
> -
> -	ret = device_add(&mgr->dev);
> -	if (ret)
> -		goto error_device;
> -
> -	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);
> +EXPORT_SYMBOL_GPL(fpga_mgr_register_simple);
>  
>  /**
>   * fpga_mgr_unregister - unregister an FPGA manager
> @@ -726,14 +679,6 @@ void fpga_mgr_unregister(struct fpga_manager *mgr)
>  }
>  EXPORT_SYMBOL_GPL(fpga_mgr_unregister);
>  
> -static int fpga_mgr_devres_match(struct device *dev, void *res,
> -				 void *match_data)
> -{
> -	struct fpga_mgr_devres *dr = res;
> -
> -	return match_data == dr->mgr;
> -}
> -
>  static void devm_fpga_mgr_unregister(struct device *dev, void *res)
>  {
>  	struct fpga_mgr_devres *dr = res;
> @@ -743,44 +688,67 @@ static void devm_fpga_mgr_unregister(struct device *dev, void *res)
>  
>  /**
>   * devm_fpga_mgr_register - resource managed variant of fpga_mgr_register()
> - * @dev: managing device for this FPGA manager
> - * @mgr: fpga manager struct
> + * @parent:	fpga manager device from pdev
> + * @info:	parameters for fpga manager
>   *
>   * This is the devres variant of fpga_mgr_register() for which the unregister
>   * function will be called automatically when the managing device is detached.
>   */
> -int devm_fpga_mgr_register(struct device *dev, struct fpga_manager *mgr)
> +struct fpga_manager *
> +devm_fpga_mgr_register(struct device *parent, const struct fpga_manager_info *info)
>  {
>  	struct fpga_mgr_devres *dr;
> -	int ret;
> -
> -	/*
> -	 * Make sure that the struct fpga_manager * that is passed in is
> -	 * managed itself.
> -	 */
> -	if (WARN_ON(!devres_find(dev, devm_fpga_mgr_release,
> -				 fpga_mgr_devres_match, mgr)))
> -		return -EINVAL;
> +	struct fpga_manager *mgr;
>  
>  	dr = devres_alloc(devm_fpga_mgr_unregister, sizeof(*dr), GFP_KERNEL);
>  	if (!dr)
> -		return -ENOMEM;
> +		return ERR_PTR(-ENOMEM);
>  
> -	ret = fpga_mgr_register(mgr);
> -	if (ret) {
> +	mgr = fpga_mgr_register(parent, info);
> +	if (IS_ERR(mgr)) {
>  		devres_free(dr);
> -		return ret;
> +		return mgr;
>  	}
>  
>  	dr->mgr = mgr;
> -	devres_add(dev, dr);
> +	devres_add(parent, dr);
>  
> -	return 0;
> +	return mgr;
>  }
>  EXPORT_SYMBOL_GPL(devm_fpga_mgr_register);
>  
> +/**
> + * devm_fpga_mgr_register_simple - resource managed variant of
> + *				   fpga_mgr_register_simple()
> + * @parent:	fpga manager device from pdev
> + * @name:	fpga manager name
> + * @mops:	pointer to structure of fpga manager ops
> + * @priv:	fpga manager private data
> + *
> + * This is the devres variant of fpga_mgr_register_simple() for which the
> + * unregister function will be called automatically when the managing
> + * device is detached.
> + */
> +struct fpga_manager *
> +devm_fpga_mgr_register_simple(struct device *parent, const char *name,
> +			      const struct fpga_manager_ops *mops, void *priv)
> +{
> +	struct fpga_manager_info info = { 0 };
> +
> +	info.name = name;
> +	info.mops = mops;
> +	info.priv = priv;
> +
> +	return devm_fpga_mgr_register(parent, &info);
> +}
> +EXPORT_SYMBOL_GPL(devm_fpga_mgr_register_simple);
> +
>  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)
> diff --git a/drivers/fpga/ice40-spi.c b/drivers/fpga/ice40-spi.c
> index 69dec5af23c3..448eb6f4e509 100644
> --- a/drivers/fpga/ice40-spi.c
> +++ b/drivers/fpga/ice40-spi.c
> @@ -178,12 +178,9 @@ static int ice40_fpga_probe(struct spi_device *spi)
>  		return ret;
>  	}
>  
> -	mgr = devm_fpga_mgr_create(dev, "Lattice iCE40 FPGA Manager",
> -				   &ice40_fpga_ops, priv);
> -	if (!mgr)
> -		return -ENOMEM;
> -
> -	return devm_fpga_mgr_register(dev, mgr);
> +	mgr = devm_fpga_mgr_register_simple(dev, "Lattice iCE40 FPGA Manager",
> +					    &ice40_fpga_ops, priv);
> +	return PTR_ERR_OR_ZERO(mgr);
>  }
>  
>  static const struct of_device_id ice40_fpga_of_match[] = {
> diff --git a/drivers/fpga/machxo2-spi.c b/drivers/fpga/machxo2-spi.c
> index 114a64d2b7a4..77be11155402 100644
> --- a/drivers/fpga/machxo2-spi.c
> +++ b/drivers/fpga/machxo2-spi.c
> @@ -366,12 +366,10 @@ static int machxo2_spi_probe(struct spi_device *spi)
>  		return -EINVAL;
>  	}
>  
> -	mgr = devm_fpga_mgr_create(dev, "Lattice MachXO2 SPI FPGA Manager",
> -				   &machxo2_ops, spi);
> -	if (!mgr)
> -		return -ENOMEM;
> -
> -	return devm_fpga_mgr_register(dev, mgr);
> +	mgr = devm_fpga_mgr_register_simple(dev,
> +					    "Lattice MachXO2 SPI FPGA Manager",
> +					    &machxo2_ops, spi);
> +	return PTR_ERR_OR_ZERO(mgr);
>  }
>  
>  static const struct of_device_id of_match[] = {
> diff --git a/drivers/fpga/socfpga-a10.c b/drivers/fpga/socfpga-a10.c
> index 573d88bdf730..5ffefaa3eb07 100644
> --- a/drivers/fpga/socfpga-a10.c
> +++ b/drivers/fpga/socfpga-a10.c
> @@ -508,19 +508,15 @@ static int socfpga_a10_fpga_probe(struct platform_device *pdev)
>  		return -EBUSY;
>  	}
>  
> -	mgr = devm_fpga_mgr_create(dev, "SoCFPGA Arria10 FPGA Manager",
> -				   &socfpga_a10_fpga_mgr_ops, priv);
> -	if (!mgr)
> -		return -ENOMEM;
> -
> -	platform_set_drvdata(pdev, mgr);
> -
> -	ret = fpga_mgr_register(mgr);
> -	if (ret) {
> +	mgr = fpga_mgr_register_simple(dev, "SoCFPGA Arria10 FPGA Manager",
> +				       &socfpga_a10_fpga_mgr_ops, priv);
> +	if (IS_ERR(mgr)) {
>  		clk_disable_unprepare(priv->clk);
> -		return ret;
> +		return PTR_ERR(mgr);
>  	}
>  
> +	platform_set_drvdata(pdev, mgr);
> +
>  	return 0;
>  }
>  
> diff --git a/drivers/fpga/socfpga.c b/drivers/fpga/socfpga.c
> index 1f467173fc1f..9280db4364d4 100644
> --- a/drivers/fpga/socfpga.c
> +++ b/drivers/fpga/socfpga.c
> @@ -571,12 +571,9 @@ static int socfpga_fpga_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>  
> -	mgr = devm_fpga_mgr_create(dev, "Altera SOCFPGA FPGA Manager",
> -				   &socfpga_fpga_ops, priv);
> -	if (!mgr)
> -		return -ENOMEM;
> -
> -	return devm_fpga_mgr_register(dev, mgr);
> +	mgr = devm_fpga_mgr_register_simple(dev, "Altera SOCFPGA FPGA Manager",
> +					    &socfpga_fpga_ops, priv);
> +	return PTR_ERR_OR_ZERO(mgr);
>  }
>  
>  #ifdef CONFIG_OF
> diff --git a/drivers/fpga/stratix10-soc.c b/drivers/fpga/stratix10-soc.c
> index a2cea500f7cc..fb84d88d4ce9 100644
> --- a/drivers/fpga/stratix10-soc.c
> +++ b/drivers/fpga/stratix10-soc.c
> @@ -425,18 +425,11 @@ static int s10_probe(struct platform_device *pdev)
>  
>  	init_completion(&priv->status_return_completion);
>  
> -	mgr = fpga_mgr_create(dev, "Stratix10 SOC FPGA Manager",
> -			      &s10_ops, priv);
> -	if (!mgr) {
> -		dev_err(dev, "unable to create FPGA manager\n");
> -		ret = -ENOMEM;
> -		goto probe_err;
> -	}
> -
> -	ret = fpga_mgr_register(mgr);
> -	if (ret) {
> +	mgr = fpga_mgr_register_simple(dev, "Stratix10 SOC FPGA Manager",
> +				       &s10_ops, priv);
> +	if (IS_ERR(mgr)) {
>  		dev_err(dev, "unable to register FPGA manager\n");
> -		fpga_mgr_free(mgr);
> +		ret = PTR_ERR(mgr);
>  		goto probe_err;
>  	}
>  
> @@ -454,7 +447,6 @@ 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;
> diff --git a/drivers/fpga/ts73xx-fpga.c b/drivers/fpga/ts73xx-fpga.c
> index 101f016c6ed8..009d5cfa1074 100644
> --- a/drivers/fpga/ts73xx-fpga.c
> +++ b/drivers/fpga/ts73xx-fpga.c
> @@ -122,12 +122,9 @@ static int ts73xx_fpga_probe(struct platform_device *pdev)
>  	if (IS_ERR(priv->io_base))
>  		return PTR_ERR(priv->io_base);
>  
> -	mgr = devm_fpga_mgr_create(kdev, "TS-73xx FPGA Manager",
> -				   &ts73xx_fpga_ops, priv);
> -	if (!mgr)
> -		return -ENOMEM;
> -
> -	return devm_fpga_mgr_register(kdev, mgr);
> +	mgr = devm_fpga_mgr_register_simple(kdev, "TS-73xx FPGA Manager",
> +					    &ts73xx_fpga_ops, priv);
> +	return PTR_ERR_OR_ZERO(mgr);
>  }
>  
>  static struct platform_driver ts73xx_fpga_driver = {
> diff --git a/drivers/fpga/xilinx-spi.c b/drivers/fpga/xilinx-spi.c
> index fee4d0abf6bf..6f568a757cf2 100644
> --- a/drivers/fpga/xilinx-spi.c
> +++ b/drivers/fpga/xilinx-spi.c
> @@ -247,13 +247,10 @@ static int xilinx_spi_probe(struct spi_device *spi)
>  		return dev_err_probe(&spi->dev, PTR_ERR(conf->done),
>  				     "Failed to get DONE gpio\n");
>  
> -	mgr = devm_fpga_mgr_create(&spi->dev,
> -				   "Xilinx Slave Serial FPGA Manager",
> -				   &xilinx_spi_ops, conf);
> -	if (!mgr)
> -		return -ENOMEM;
> -
> -	return devm_fpga_mgr_register(&spi->dev, mgr);
> +	mgr = devm_fpga_mgr_register_simple(&spi->dev,
> +					    "Xilinx Slave Serial FPGA Manager",
> +					    &xilinx_spi_ops, conf);
> +	return PTR_ERR_OR_ZERO(mgr);
>  }
>  
>  static const struct of_device_id xlnx_spi_of_match[] = {
> diff --git a/drivers/fpga/zynq-fpga.c b/drivers/fpga/zynq-fpga.c
> index 9b75bd4f93d8..a3de365aadc6 100644
> --- a/drivers/fpga/zynq-fpga.c
> +++ b/drivers/fpga/zynq-fpga.c
> @@ -609,20 +609,16 @@ static int zynq_fpga_probe(struct platform_device *pdev)
>  
>  	clk_disable(priv->clk);
>  
> -	mgr = devm_fpga_mgr_create(dev, "Xilinx Zynq FPGA Manager",
> -				   &zynq_fpga_ops, priv);
> -	if (!mgr)
> -		return -ENOMEM;
> -
> -	platform_set_drvdata(pdev, mgr);
> -
> -	err = fpga_mgr_register(mgr);
> -	if (err) {
> +	mgr = fpga_mgr_register_simple(dev, "Xilinx Zynq FPGA Manager",
> +				       &zynq_fpga_ops, priv);
> +	if (IS_ERR(mgr)) {
>  		dev_err(dev, "unable to register FPGA manager\n");
>  		clk_unprepare(priv->clk);
> -		return err;
> +		return PTR_ERR(mgr);
>  	}
>  
> +	platform_set_drvdata(pdev, mgr);
> +
>  	return 0;
>  }
>  
> diff --git a/drivers/fpga/zynqmp-fpga.c b/drivers/fpga/zynqmp-fpga.c
> index 125743c9797f..53a7b36f61e8 100644
> --- a/drivers/fpga/zynqmp-fpga.c
> +++ b/drivers/fpga/zynqmp-fpga.c
> @@ -102,12 +102,9 @@ static int zynqmp_fpga_probe(struct platform_device *pdev)
>  
>  	priv->dev = dev;
>  
> -	mgr = devm_fpga_mgr_create(dev, "Xilinx ZynqMP FPGA Manager",
> -				   &zynqmp_fpga_ops, priv);
> -	if (!mgr)
> -		return -ENOMEM;
> -
> -	return devm_fpga_mgr_register(dev, mgr);
> +	mgr = devm_fpga_mgr_register_simple(dev, "Xilinx ZynqMP FPGA Manager",
> +					    &zynqmp_fpga_ops, priv);
> +	return PTR_ERR_OR_ZERO(mgr);
>  }
>  
>  static const struct of_device_id zynqmp_fpga_of_match[] = {
> diff --git a/include/linux/fpga/fpga-mgr.h b/include/linux/fpga/fpga-mgr.h
> index 474c1f506307..9efe55be93bc 100644
> --- a/include/linux/fpga/fpga-mgr.h
> +++ b/include/linux/fpga/fpga-mgr.h
> @@ -105,6 +105,36 @@ struct fpga_image_info {
>  #endif
>  };
>  
> +/**
> + * struct fpga_compat_id - id for compatibility check
> + *
> + * @id_h: high 64bit of the compat_id
> + * @id_l: low 64bit of the compat_id
> + */
> +struct fpga_compat_id {
> +	u64 id_h;
> +	u64 id_l;
> +};
> +
> +/**
> + * struct fpga_manager_info - collection of parameters for an FPGA Manager
> + * @name: fpga manager name
> + * @compat_id: FPGA manager id for compatibility check.
> + * @mops: pointer to structure of fpga manager ops
> + * @priv: fpga manager private data
> + *
> + * fpga_manager_info contains parameters for the register function. These
> + * are separated into an info structure because they some are optional
> + * others could be added to in the future. The info structure facilitates
> + * maintaining a stable API.
> + */
> +struct fpga_manager_info {
> +	const char *name;
> +	struct fpga_compat_id *compat_id;
> +	const struct fpga_manager_ops *mops;
> +	void *priv;
> +};
> +
>  /**
>   * struct fpga_manager_ops - ops for low level fpga manager drivers
>   * @initial_header_size: Maximum number of bytes that should be passed into write_init
> @@ -143,17 +173,6 @@ struct fpga_manager_ops {
>  #define FPGA_MGR_STATUS_IP_PROTOCOL_ERR		BIT(3)
>  #define FPGA_MGR_STATUS_FIFO_OVERFLOW_ERR	BIT(4)
>  
> -/**
> - * struct fpga_compat_id - id for compatibility check
> - *
> - * @id_h: high 64bit of the compat_id
> - * @id_l: low 64bit of the compat_id
> - */
> -struct fpga_compat_id {
> -	u64 id_h;
> -	u64 id_l;
> -};
> -
>  /**
>   * struct fpga_manager - fpga manager structure
>   * @name: name of low level fpga manager
> @@ -191,17 +210,18 @@ struct fpga_manager *fpga_mgr_get(struct device *dev);
>  
>  void fpga_mgr_put(struct fpga_manager *mgr);
>  
> -struct fpga_manager *fpga_mgr_create(struct device *dev, const char *name,
> -				     const struct fpga_manager_ops *mops,
> -				     void *priv);
> -void fpga_mgr_free(struct fpga_manager *mgr);
> -int fpga_mgr_register(struct fpga_manager *mgr);
> -void fpga_mgr_unregister(struct fpga_manager *mgr);
> +struct fpga_manager *
> +fpga_mgr_register(struct device *parent, const struct fpga_manager_info *info);
>  
> -int devm_fpga_mgr_register(struct device *dev, struct fpga_manager *mgr);
> +struct fpga_manager *
> +fpga_mgr_register_simple(struct device *parent, const char *name,
> +			 const struct fpga_manager_ops *mops, void *priv);
> +void fpga_mgr_unregister(struct fpga_manager *mgr);
>  
> -struct fpga_manager *devm_fpga_mgr_create(struct device *dev, const char *name,
> -					  const struct fpga_manager_ops *mops,
> -					  void *priv);
> +struct fpga_manager *
> +devm_fpga_mgr_register(struct device *parent, const struct fpga_manager_info *info);
> +struct fpga_manager *
> +devm_fpga_mgr_register_simple(struct device *parent, const char *name,
> +			      const struct fpga_manager_ops *mops, void *priv);
>  
>  #endif /*_LINUX_FPGA_MGR_H */
> -- 
> 2.25.1

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v8 2/3] fpga: bridge: Use standard dev_release for class driver
  2021-06-24 20:53 ` [PATCH v8 2/3] fpga: bridge: " Russ Weight
@ 2021-06-28  5:31   ` Xu Yilun
  0 siblings, 0 replies; 10+ messages in thread
From: Xu Yilun @ 2021-06-28  5:31 UTC (permalink / raw)
  To: Russ Weight
  Cc: mdf, linux-fpga, trix, lgoncalv, hao.wu, matthew.gerlach, richard.gong

On Thu, Jun 24, 2021 at 01:53:54PM -0700, Russ Weight wrote:
> The FPGA bridge class driver data structure is being treated as a
> managed resource instead of using the standard dev_release call-back
> function to release the class data structure. This change removes
> the managed resource code and combines the create() and register()
> functions into a single register() or register_simple() function.
> 
> The register() function accepts an info data structure to provide
> flexibility in passing optional parameters. The register_simple()
> function supports the current parameter list for users that don't
> require the use of optional parameters.
> 
> Signed-off-by: Russ Weight <russell.h.weight@intel.com>
> Reviewed-by: Xu Yilun <yilun.xu@intel.com>
> ---
> v8:
>   - Added reviewed-by tag.
>   - Updated Documentation/driver-api/fpga/fpga-bridge.rst documentation.
> v7:
>   - Update the commit message to describe the new parameters for the
>     fpga_bridge_register() function and to mention the
>     fpga_bridge_register_simple() function.
>   - Fix function prototypes in header file to rename dev to parent.
>   - Some cleanup of comments.
>   - Update function defintions/prototypes to apply const to the new info
>     parameter.
>   - Verify that info->br_ops is non-null in register functions.
> v6:
>   - Changed fpga_bridge_register() parameters to accept an info data
>     structure to provide flexibility in passing optional parameters.
>   - Added fpga_bridge_register_simple() function to support current
>     parameters for users that don't require the use of optional
>     parameters.
> v5:
>   - Rebased on top of recently accepted patches.
> v4:
>   - Restore the previous format for the Return value in the comment header
>     for fpga_bridge_register()
> v3:
>   - Cleaned up comment header for fpga_bridge_register()
>   - Fix error return values for fpga_bridge_register()
> v2:
>   - No changes
> ---
>  Documentation/driver-api/fpga/fpga-bridge.rst |   8 +-
>  drivers/fpga/altera-fpga2sdram.c              |  12 +-
>  drivers/fpga/altera-freeze-bridge.c           |  10 +-
>  drivers/fpga/altera-hps2fpga.c                |  12 +-
>  drivers/fpga/dfl-fme-br.c                     |  10 +-
>  drivers/fpga/fpga-bridge.c                    | 138 ++++++------------
>  drivers/fpga/xilinx-pr-decoupler.c            |  17 +--
>  include/linux/fpga/fpga-bridge.h              |  33 +++--
>  8 files changed, 100 insertions(+), 140 deletions(-)
> 
> diff --git a/Documentation/driver-api/fpga/fpga-bridge.rst b/Documentation/driver-api/fpga/fpga-bridge.rst
> index 8d650b4e2ce6..d87aafc60ad6 100644
> --- a/Documentation/driver-api/fpga/fpga-bridge.rst
> +++ b/Documentation/driver-api/fpga/fpga-bridge.rst
> @@ -6,8 +6,8 @@ API to implement a new FPGA bridge
>  
>  * struct fpga_bridge - The FPGA Bridge structure
>  * struct fpga_bridge_ops - Low level Bridge driver ops
> -* devm_fpga_bridge_create() - Allocate and init a bridge struct
> -* fpga_bridge_register() - Register a bridge
> +* fpga_bridge_register() - Create and register a bridge
> +* fpga_bridge_register_simple() - Create and register a bridge

Identical descriptions may confuse developers, may provide a little
details to distinguish each other.

The same for fpga-mgr.rst & fpga-region.rst

Thanks,
Yilun

>  * fpga_bridge_unregister() - Unregister a bridge
>  
>  .. kernel-doc:: include/linux/fpga/fpga-bridge.h
> @@ -17,10 +17,10 @@ API to implement a new FPGA bridge
>     :functions: fpga_bridge_ops
>  
>  .. kernel-doc:: drivers/fpga/fpga-bridge.c
> -   :functions: devm_fpga_bridge_create
> +   :functions: fpga_bridge_register
>  
>  .. kernel-doc:: drivers/fpga/fpga-bridge.c
> -   :functions: fpga_bridge_register
> +   :functions: fpga_bridge_register_simple
>  
>  .. kernel-doc:: drivers/fpga/fpga-bridge.c
>     :functions: fpga_bridge_unregister
> diff --git a/drivers/fpga/altera-fpga2sdram.c b/drivers/fpga/altera-fpga2sdram.c
> index a78e49c63c64..e165440ebbab 100644
> --- a/drivers/fpga/altera-fpga2sdram.c
> +++ b/drivers/fpga/altera-fpga2sdram.c
> @@ -121,17 +121,13 @@ static int alt_fpga_bridge_probe(struct platform_device *pdev)
>  	/* Get f2s bridge configuration saved in handoff register */
>  	regmap_read(sysmgr, SYSMGR_ISWGRP_HANDOFF3, &priv->mask);
>  
> -	br = devm_fpga_bridge_create(dev, F2S_BRIDGE_NAME,
> -				     &altera_fpga2sdram_br_ops, priv);
> -	if (!br)
> -		return -ENOMEM;
> +	br = fpga_bridge_register_simple(dev, F2S_BRIDGE_NAME,
> +					 &altera_fpga2sdram_br_ops, priv);
> +	if (IS_ERR(br))
> +		return PTR_ERR(mgr);
>  
>  	platform_set_drvdata(pdev, br);
>  
> -	ret = fpga_bridge_register(br);
> -	if (ret)
> -		return ret;
> -
>  	dev_info(dev, "driver initialized with handoff %08x\n", priv->mask);
>  
>  	if (!of_property_read_u32(dev->of_node, "bridge-enable", &enable)) {
> diff --git a/drivers/fpga/altera-freeze-bridge.c b/drivers/fpga/altera-freeze-bridge.c
> index dd58c4aea92e..4e39b5475630 100644
> --- a/drivers/fpga/altera-freeze-bridge.c
> +++ b/drivers/fpga/altera-freeze-bridge.c
> @@ -244,14 +244,14 @@ static int altera_freeze_br_probe(struct platform_device *pdev)
>  
>  	priv->base_addr = base_addr;
>  
> -	br = devm_fpga_bridge_create(dev, FREEZE_BRIDGE_NAME,
> -				     &altera_freeze_br_br_ops, priv);
> -	if (!br)
> -		return -ENOMEM;
> +	br = fpga_bridge_register_simple(dev, FREEZE_BRIDGE_NAME,
> +					 &altera_freeze_br_br_ops, priv);
> +	if (IS_ERR(br))
> +		return PTR_ERR(br);
>  
>  	platform_set_drvdata(pdev, br);
>  
> -	return fpga_bridge_register(br);
> +	return 0;
>  }
>  
>  static int altera_freeze_br_remove(struct platform_device *pdev)
> diff --git a/drivers/fpga/altera-hps2fpga.c b/drivers/fpga/altera-hps2fpga.c
> index 77b95f251821..a564eb29349c 100644
> --- a/drivers/fpga/altera-hps2fpga.c
> +++ b/drivers/fpga/altera-hps2fpga.c
> @@ -180,19 +180,15 @@ static int alt_fpga_bridge_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> -	br = devm_fpga_bridge_create(dev, priv->name,
> -				     &altera_hps2fpga_br_ops, priv);
> -	if (!br) {
> -		ret = -ENOMEM;
> +	br = fpga_bridge_register_simple(dev, priv->name,
> +					 &altera_hps2fpga_br_ops, priv);
> +	if (IS_ERR(br)) {
> +		ret = PTR_ERR(br);
>  		goto err;
>  	}
>  
>  	platform_set_drvdata(pdev, br);
>  
> -	ret = fpga_bridge_register(br);
> -	if (ret)
> -		goto err;
> -
>  	return 0;
>  
>  err:
> diff --git a/drivers/fpga/dfl-fme-br.c b/drivers/fpga/dfl-fme-br.c
> index 3ff9f3a687ce..0ad39e502142 100644
> --- a/drivers/fpga/dfl-fme-br.c
> +++ b/drivers/fpga/dfl-fme-br.c
> @@ -68,14 +68,14 @@ static int fme_br_probe(struct platform_device *pdev)
>  
>  	priv->pdata = dev_get_platdata(dev);
>  
> -	br = devm_fpga_bridge_create(dev, "DFL FPGA FME Bridge",
> -				     &fme_bridge_ops, priv);
> -	if (!br)
> -		return -ENOMEM;
> +	br = fpga_bridge_register_simple(dev, "DFL FPGA FME Bridge",
> +					 &fme_bridge_ops, priv);
> +	if (IS_ERR(br))
> +		return PTR_ERR(br);
>  
>  	platform_set_drvdata(pdev, br);
>  
> -	return fpga_bridge_register(br);
> +	return 0;
>  }
>  
>  static int fme_br_remove(struct platform_device *pdev)
> diff --git a/drivers/fpga/fpga-bridge.c b/drivers/fpga/fpga-bridge.c
> index 798f55670646..6f2f16fd5025 100644
> --- a/drivers/fpga/fpga-bridge.c
> +++ b/drivers/fpga/fpga-bridge.c
> @@ -312,55 +312,62 @@ static struct attribute *fpga_bridge_attrs[] = {
>  ATTRIBUTE_GROUPS(fpga_bridge);
>  
>  /**
> - * fpga_bridge_create - create and initialize a struct fpga_bridge
> + * fpga_bridge_register - create and register an FPGA Bridge device
>   * @parent:	FPGA bridge device from pdev
> - * @name:	FPGA bridge name
> - * @br_ops:	pointer to structure of fpga bridge ops
> - * @priv:	FPGA bridge private data
> + * @info:	parameters for FPGA Bridge
>   *
> - * The caller of this function is responsible for freeing the bridge with
> - * fpga_bridge_free().  Using devm_fpga_bridge_create() instead is recommended.
> - *
> - * Return: struct fpga_bridge or NULL
> + * Return: struct fpga_bridge pointer or ERR_PTR()
>   */
> -struct fpga_bridge *fpga_bridge_create(struct device *parent, const char *name,
> -				       const struct fpga_bridge_ops *br_ops,
> -				       void *priv)
> +struct fpga_bridge *fpga_bridge_register(struct device *parent,
> +					 const struct fpga_bridge_info *info)
>  {
>  	struct fpga_bridge *bridge;
>  	int id, ret;
>  
> -	if (!name || !strlen(name)) {
> +	if (!info->br_ops) {
> +		dev_err(parent, "Attempt to register without fpga_bridge_ops\n");
> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	if (!info->name || !strlen(info->name)) {
>  		dev_err(parent, "Attempt to register with no name!\n");
> -		return NULL;
> +		return ERR_PTR(-EINVAL);
>  	}
>  
>  	bridge = kzalloc(sizeof(*bridge), GFP_KERNEL);
>  	if (!bridge)
> -		return NULL;
> +		return ERR_PTR(-ENOMEM);
>  
>  	id = ida_simple_get(&fpga_bridge_ida, 0, 0, GFP_KERNEL);
> -	if (id < 0)
> +	if (id < 0) {
> +		ret = id;
>  		goto error_kfree;
> +	}
>  
>  	mutex_init(&bridge->mutex);
>  	INIT_LIST_HEAD(&bridge->node);
>  
> -	bridge->name = name;
> -	bridge->br_ops = br_ops;
> -	bridge->priv = priv;
> +	bridge->name = info->name;
> +	bridge->br_ops = info->br_ops;
> +	bridge->priv = info->priv;
>  
> -	device_initialize(&bridge->dev);
> -	bridge->dev.groups = br_ops->groups;
> +	bridge->dev.groups = info->br_ops->groups;
>  	bridge->dev.class = fpga_bridge_class;
>  	bridge->dev.parent = parent;
>  	bridge->dev.of_node = parent->of_node;
>  	bridge->dev.id = id;
> +	of_platform_populate(bridge->dev.of_node, NULL, NULL, &bridge->dev);
>  
>  	ret = dev_set_name(&bridge->dev, "br%d", id);
>  	if (ret)
>  		goto error_device;
>  
> +	ret = device_register(&bridge->dev);
> +	if (ret) {
> +		put_device(&bridge->dev);
> +		return ERR_PTR(ret);
> +	}
> +
>  	return bridge;
>  
>  error_device:
> @@ -368,90 +375,37 @@ struct fpga_bridge *fpga_bridge_create(struct device *parent, const char *name,
>  error_kfree:
>  	kfree(bridge);
>  
> -	return NULL;
> -}
> -EXPORT_SYMBOL_GPL(fpga_bridge_create);
> -
> -/**
> - * fpga_bridge_free - free an fpga bridge created by fpga_bridge_create()
> - * @bridge:	FPGA bridge struct
> - */
> -void fpga_bridge_free(struct fpga_bridge *bridge)
> -{
> -	ida_simple_remove(&fpga_bridge_ida, bridge->dev.id);
> -	kfree(bridge);
> -}
> -EXPORT_SYMBOL_GPL(fpga_bridge_free);
> -
> -static void devm_fpga_bridge_release(struct device *dev, void *res)
> -{
> -	struct fpga_bridge *bridge = *(struct fpga_bridge **)res;
> -
> -	fpga_bridge_free(bridge);
> +	return ERR_PTR(ret);
>  }
> +EXPORT_SYMBOL_GPL(fpga_bridge_register);
>  
>  /**
> - * devm_fpga_bridge_create - create and init a managed struct fpga_bridge
> + * fpga_bridge_register_simple - create and register an FPGA Bridge device
>   * @parent:	FPGA bridge device from pdev
>   * @name:	FPGA bridge name
>   * @br_ops:	pointer to structure of fpga bridge ops
>   * @priv:	FPGA bridge private data
>   *
> - * This function is intended for use in an FPGA bridge driver's probe function.
> - * After the bridge driver creates the struct with devm_fpga_bridge_create(), it
> - * should register the bridge with fpga_bridge_register().  The bridge driver's
> - * remove function should call fpga_bridge_unregister().  The bridge struct
> - * allocated with this function will be freed automatically on driver detach.
> - * This includes the case of a probe function returning error before calling
> - * fpga_bridge_register(), the struct will still get cleaned up.
> + * This simple version of the register should be sufficient for most users.
> + * The fpga_bridge_register() function is available for users that need to pass
> + * additional, optional parameters.
>   *
> - *  Return: struct fpga_bridge or NULL
> + * Return: struct fpga_bridge pointer or ERR_PTR()
>   */
> -struct fpga_bridge
> -*devm_fpga_bridge_create(struct device *parent, const char *name,
> -			 const struct fpga_bridge_ops *br_ops, void *priv)
> +struct fpga_bridge *
> +fpga_bridge_register_simple(struct device *parent, const char *name,
> +			    const struct fpga_bridge_ops *br_ops,
> +			    void *priv)
>  {
> -	struct fpga_bridge **ptr, *bridge;
> -
> -	ptr = devres_alloc(devm_fpga_bridge_release, sizeof(*ptr), GFP_KERNEL);
> -	if (!ptr)
> -		return NULL;
> -
> -	bridge = fpga_bridge_create(parent, name, br_ops, priv);
> -	if (!bridge) {
> -		devres_free(ptr);
> -	} else {
> -		*ptr = bridge;
> -		devres_add(parent, ptr);
> -	}
> -
> -	return bridge;
> -}
> -EXPORT_SYMBOL_GPL(devm_fpga_bridge_create);
> -
> -/**
> - * fpga_bridge_register - register an FPGA bridge
> - *
> - * @bridge: FPGA bridge struct
> - *
> - * Return: 0 for success, error code otherwise.
> - */
> -int fpga_bridge_register(struct fpga_bridge *bridge)
> -{
> -	struct device *dev = &bridge->dev;
> -	int ret;
> -
> -	ret = device_add(dev);
> -	if (ret)
> -		return ret;
> -
> -	of_platform_populate(dev->of_node, NULL, NULL, dev);
> +	struct fpga_bridge_info info = { 0 };
>  
> -	dev_info(dev->parent, "fpga bridge [%s] registered\n", bridge->name);
> +	info.name = name;
> +	info.br_ops = br_ops;
> +	info.priv = priv;
>  
> -	return 0;
> +	return fpga_bridge_register(parent, &info);
>  }
> -EXPORT_SYMBOL_GPL(fpga_bridge_register);
> +EXPORT_SYMBOL_GPL(fpga_bridge_register_simple);
>  
>  /**
>   * fpga_bridge_unregister - unregister an FPGA bridge
> @@ -475,6 +429,10 @@ 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)
> diff --git a/drivers/fpga/xilinx-pr-decoupler.c b/drivers/fpga/xilinx-pr-decoupler.c
> index ea2bde6e5bc4..24bc16a86091 100644
> --- a/drivers/fpga/xilinx-pr-decoupler.c
> +++ b/drivers/fpga/xilinx-pr-decoupler.c
> @@ -138,22 +138,17 @@ static int xlnx_pr_decoupler_probe(struct platform_device *pdev)
>  
>  	clk_disable(priv->clk);
>  
> -	br = devm_fpga_bridge_create(&pdev->dev, priv->ipconfig->name,
> -				     &xlnx_pr_decoupler_br_ops, priv);
> -	if (!br) {
> -		err = -ENOMEM;
> -		goto err_clk;
> -	}
> -
> -	platform_set_drvdata(pdev, br);
> -
> -	err = fpga_bridge_register(br);
> -	if (err) {
> +	br = fpga_bridge_register_simple(&pdev->dev, priv->ipconfig->name,
> +					 &xlnx_pr_decoupler_br_ops, priv);
> +	if (IS_ERR(br)) {
> +		err = PTR_ERR(br);
>  		dev_err(&pdev->dev, "unable to register %s",
>  			priv->ipconfig->name);
>  		goto err_clk;
>  	}
>  
> +	platform_set_drvdata(pdev, br);
> +
>  	return 0;
>  
>  err_clk:
> diff --git a/include/linux/fpga/fpga-bridge.h b/include/linux/fpga/fpga-bridge.h
> index 6c3c28806ff1..6b5584530c92 100644
> --- a/include/linux/fpga/fpga-bridge.h
> +++ b/include/linux/fpga/fpga-bridge.h
> @@ -22,6 +22,23 @@ struct fpga_bridge_ops {
>  	const struct attribute_group **groups;
>  };
>  
> +/**
> + * struct fpga_bridge_info - collection of parameters an FPGA Bridge
> + * @name: fpga bridge name
> + * @br_ops: pointer to structure of fpga bridge ops
> + * @priv: fpga bridge private data
> + *
> + * fpga_bridge_info contains parameters for the register function. These
> + * are separated into an info structure because they some are optional
> + * others could be added to in the future. The info structure facilitates
> + * maintaining a stable API.
> + */
> +struct fpga_bridge_info {
> +	const char *name;
> +	const struct fpga_bridge_ops *br_ops;
> +	void *priv;
> +};
> +
>  /**
>   * struct fpga_bridge - FPGA bridge structure
>   * @name: name of low level FPGA bridge
> @@ -62,15 +79,13 @@ int of_fpga_bridge_get_to_list(struct device_node *np,
>  			       struct fpga_image_info *info,
>  			       struct list_head *bridge_list);
>  
> -struct fpga_bridge *fpga_bridge_create(struct device *dev, const char *name,
> -				       const struct fpga_bridge_ops *br_ops,
> -				       void *priv);
> -void fpga_bridge_free(struct fpga_bridge *br);
> -int fpga_bridge_register(struct fpga_bridge *br);
> -void fpga_bridge_unregister(struct fpga_bridge *br);
> +struct fpga_bridge *
> +fpga_bridge_register(struct device *parent, const struct fpga_bridge_info *info);
>  
> -struct fpga_bridge
> -*devm_fpga_bridge_create(struct device *dev, const char *name,
> -			 const struct fpga_bridge_ops *br_ops, void *priv);
> +struct fpga_bridge *
> +fpga_bridge_register_simple(struct device *parent, const char *name,
> +			    const struct fpga_bridge_ops *br_ops,
> +			    void *priv);
> +void fpga_bridge_unregister(struct fpga_bridge *br);
>  
>  #endif /* _LINUX_FPGA_BRIDGE_H */
> -- 
> 2.25.1

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v8 1/3] fpga: mgr: Use standard dev_release for class driver
  2021-06-28  3:49   ` Wu, Hao
@ 2021-06-28 18:51     ` Moritz Fischer
  2021-06-30 22:12     ` Russ Weight
  1 sibling, 0 replies; 10+ messages in thread
From: Moritz Fischer @ 2021-06-28 18:51 UTC (permalink / raw)
  To: Wu, Hao
  Cc: Weight, Russell H, mdf, linux-fpga, trix, lgoncalv, Xu, Yilun,
	Gerlach, Matthew, Gong, Richard

On Mon, Jun 28, 2021 at 03:49:46AM +0000, Wu, Hao wrote:
> > -----Original Message-----
> > From: Weight, Russell H <russell.h.weight@intel.com>
> > Sent: Friday, June 25, 2021 4:54 AM
> > To: mdf@kernel.org; linux-fpga@vger.kernel.org
> > Cc: trix@redhat.com; lgoncalv@redhat.com; Xu, Yilun <yilun.xu@intel.com>;
> > Wu, Hao <hao.wu@intel.com>; Gerlach, Matthew
> > <matthew.gerlach@intel.com>; Gong, Richard <richard.gong@intel.com>;
> > Weight, Russell H <russell.h.weight@intel.com>
> > Subject: [PATCH v8 1/3] fpga: mgr: Use standard dev_release for class driver
> > 
> > The FPGA manager class driver data structure is being treated as a
> > managed resource instead of using the standard dev_release call-back
> > function to release the class data structure. This change removes
> > the managed resource code for the freeing of the class data structure
> > and combines the create() and register() functions into a single
> > register() or register_simple() function.
> > 
> 
> Maybe register() and register_full() is better. it keeps the most used
> function name the same (and shorter than register_simple()). Just
> my 2 cents. Let's wait for review comments from Moritz to decide. : )

I like the register() and register_full() idea.
> 
> Thanks
> Hao
> 
> > The register() function accepts an info data structure to provide
> > flexibility in passing optional parameters. The register_simple()
> > function supports the current parameter list for users that don't
> > require the use of optional parameters.
> > 
> > The devm_fpga_mgr_register() function is retained, and the
> > devm_fpga_mgr_register_simple() function is added.
> > 
> > Signed-off-by: Russ Weight <russell.h.weight@intel.com>
> > Reviewed-by: Xu Yilun <yilun.xu@intel.com>
> > ---
> > v8:
> >   - Added reviewed-by tag.
> >   - Updated Documentation/driver-api/fpga/fpga-mgr.rst documentation.
> > v7:
> >   - Update the commit message to describe the new parameters for the
> >     *fpga_mgr_register() functions and to mention the
> >     *fpga_mgr_register_simple() functions.
> >   - Fix function prototypes in header file to rename dev to parent.
> >   - Make use of the PTR_ERR_OR_ZERO() macro when possible.
> >   - Some cleanup of comments.
> >   - Update function defintions/prototypes to apply const to the new info
> >     parameter.
> > v6:
> >   - Moved FPGA manager optional parameters out of the ops structure and
> >     back into the FPGA manager structure.
> >   - Changed fpga_mgr_register()/devm_fpga_mgr_register() parameters to
> >     accept an info data structure to provide flexibility in passing optional
> >     parameters.
> >   - Added fpga_mgr_register_simple()/devm_fpga_mgr_register_simple()
> >     functions to support current parameters for users that don't require
> >     the use of optional parameters.
> > v5:
> >   - Rebased on top of recently accepted patches.
> >   - Removed compat_id from the fpga_mgr_register() parameter list
> >     and added it to the fpga_manager_ops structure. This also required
> >     dynamically allocating the dfl-fme-ops structure in order to add
> >     the appropriate compat_id.
> > v4:
> >   - Added the compat_id parameter to fpga_mgr_register() and
> >     devm_fpga_mgr_register() to ensure that the compat_id is set before
> >     the device_register() call.
> > v3:
> >   - Cleaned up comment header for fpga_mgr_register()
> >   - Fix error return on ida_simple_get() failure
> > v2:
> >   - Restored devm_fpga_mgr_register() functionality, adapted for the combined
> >     create/register functionality.
> >   - All previous callers of devm_fpga_mgr_register() will continue to call
> >     devm_fpga_mgr_register().
> >   - replaced unnecessary ternary operators in return statements with standard
> >     if conditions.
> > ---
> >  Documentation/driver-api/fpga/fpga-mgr.rst |  39 ++--
> >  drivers/fpga/altera-cvp.c                  |  12 +-
> >  drivers/fpga/altera-pr-ip-core.c           |   8 +-
> >  drivers/fpga/altera-ps-spi.c               |   9 +-
> >  drivers/fpga/dfl-fme-mgr.c                 |  22 +--
> >  drivers/fpga/fpga-mgr.c                    | 214 +++++++++------------
> >  drivers/fpga/ice40-spi.c                   |   9 +-
> >  drivers/fpga/machxo2-spi.c                 |  10 +-
> >  drivers/fpga/socfpga-a10.c                 |  16 +-
> >  drivers/fpga/socfpga.c                     |   9 +-
> >  drivers/fpga/stratix10-soc.c               |  16 +-
> >  drivers/fpga/ts73xx-fpga.c                 |   9 +-
> >  drivers/fpga/xilinx-spi.c                  |  11 +-
> >  drivers/fpga/zynq-fpga.c                   |  16 +-
> >  drivers/fpga/zynqmp-fpga.c                 |   9 +-
> >  include/linux/fpga/fpga-mgr.h              |  62 ++++--
> >  16 files changed, 204 insertions(+), 267 deletions(-)
> > 
> > diff --git a/Documentation/driver-api/fpga/fpga-mgr.rst
> > b/Documentation/driver-api/fpga/fpga-mgr.rst
> > index 4d926b452cb3..fbb7f07cae9f 100644
> > --- a/Documentation/driver-api/fpga/fpga-mgr.rst
> > +++ b/Documentation/driver-api/fpga/fpga-mgr.rst
> > @@ -24,7 +24,8 @@ How to support a new FPGA device
> >  --------------------------------
> > 
> >  To add another FPGA manager, write a driver that implements a set of ops.  The
> > -probe function calls fpga_mgr_register(), such as::
> > +probe function calls devm_fpga_mgr_register() or
> > +devm_fpga_mgr_register_simple(), such as::
> > 
> >  	static const struct fpga_manager_ops socfpga_fpga_ops = {
> >  		.write_init = socfpga_fpga_ops_configure_init,
> > @@ -49,23 +50,9 @@ probe function calls fpga_mgr_register(), such as::
> >  		 * them in priv
> >  		 */
> > 
> > -		mgr = devm_fpga_mgr_create(dev, "Altera SOCFPGA FPGA
> > Manager",
> > -					   &socfpga_fpga_ops, priv);
> > -		if (!mgr)
> > -			return -ENOMEM;
> > -
> > -		platform_set_drvdata(pdev, mgr);
> > -
> > -		return fpga_mgr_register(mgr);
> > -	}
> > -
> > -	static int socfpga_fpga_remove(struct platform_device *pdev)
> > -	{
> > -		struct fpga_manager *mgr = platform_get_drvdata(pdev);
> > -
> > -		fpga_mgr_unregister(mgr);
> > -
> > -		return 0;
> > +		mgr = devm_fpga_mgr_register_simple(dev, "Altera SOCFPGA
> > FPGA Manager",
> > +						    &socfpga_fpga_ops, priv);
> > +		return PTR_ERR_OR_ZERO(mgr);
> >  	}
> > 
> > 
> > @@ -104,8 +91,10 @@ API for implementing a new FPGA Manager driver
> >  * ``fpga_mgr_states`` -  Values for :c:expr:`fpga_manager->state`.
> >  * struct fpga_manager -  the FPGA manager struct
> >  * struct fpga_manager_ops -  Low level FPGA manager driver ops
> > -* devm_fpga_mgr_create() -  Allocate and init a manager struct
> > -* fpga_mgr_register() -  Register an FPGA manager
> > +* fpga_mgr_register() -  Create and register an FPGA manager
> > +* fpga_mgr_register_simple() -  Create and register an FPGA manager
> > +* devm_fpga_mgr_register() -  Create and register an FPGA manager
> > +* devm_fpga_mgr_register_simple() -  Create and register an FPGA manager
> >  * fpga_mgr_unregister() -  Unregister an FPGA manager
> > 
> >  .. kernel-doc:: include/linux/fpga/fpga-mgr.h
> > @@ -118,10 +107,16 @@ API for implementing a new FPGA Manager driver
> >     :functions: fpga_manager_ops
> > 
> >  .. kernel-doc:: drivers/fpga/fpga-mgr.c
> > -   :functions: devm_fpga_mgr_create
> > +   :functions: fpga_mgr_register
> > 
> >  .. kernel-doc:: drivers/fpga/fpga-mgr.c
> > -   :functions: fpga_mgr_register
> > +   :functions: fpga_mgr_register_simple
> > +
> > +.. kernel-doc:: drivers/fpga/fpga-mgr.c
> > +   :functions: devm_fpga_mgr_register
> > +
> > +.. kernel-doc:: drivers/fpga/fpga-mgr.c
> > +   :functions: devm_fpga_mgr_register_simple
> > 
> >  .. kernel-doc:: drivers/fpga/fpga-mgr.c
> >     :functions: fpga_mgr_unregister
> > diff --git a/drivers/fpga/altera-cvp.c b/drivers/fpga/altera-cvp.c
> > index ccf4546eff29..ddf2ffe3f138 100644
> > --- a/drivers/fpga/altera-cvp.c
> > +++ b/drivers/fpga/altera-cvp.c
> > @@ -652,19 +652,15 @@ static int altera_cvp_probe(struct pci_dev *pdev,
> >  	snprintf(conf->mgr_name, sizeof(conf->mgr_name), "%s @%s",
> >  		 ALTERA_CVP_MGR_NAME, pci_name(pdev));
> > 
> > -	mgr = devm_fpga_mgr_create(&pdev->dev, conf->mgr_name,
> > -				   &altera_cvp_ops, conf);
> > -	if (!mgr) {
> > -		ret = -ENOMEM;
> > +	mgr = fpga_mgr_register_simple(&pdev->dev, conf->mgr_name,
> > +				       &altera_cvp_ops, conf);
> > +	if (IS_ERR(mgr)) {
> > +		ret = PTR_ERR(mgr);
> >  		goto err_unmap;
> >  	}
> > 
> >  	pci_set_drvdata(pdev, mgr);
> > 
> > -	ret = fpga_mgr_register(mgr);
> > -	if (ret)
> > -		goto err_unmap;
> > -
> >  	return 0;
> > 
> >  err_unmap:
> > diff --git a/drivers/fpga/altera-pr-ip-core.c b/drivers/fpga/altera-pr-ip-core.c
> > index dfdf21ed34c4..e0baf3392b67 100644
> > --- a/drivers/fpga/altera-pr-ip-core.c
> > +++ b/drivers/fpga/altera-pr-ip-core.c
> > @@ -191,11 +191,9 @@ int alt_pr_register(struct device *dev, void __iomem
> > *reg_base)
> >  		(val & ALT_PR_CSR_STATUS_MSK) >> ALT_PR_CSR_STATUS_SFT,
> >  		(int)(val & ALT_PR_CSR_PR_START));
> > 
> > -	mgr = devm_fpga_mgr_create(dev, dev_name(dev), &alt_pr_ops, priv);
> > -	if (!mgr)
> > -		return -ENOMEM;
> > -
> > -	return devm_fpga_mgr_register(dev, mgr);
> > +	mgr = devm_fpga_mgr_register_simple(dev, dev_name(dev),
> > +					    &alt_pr_ops, priv);
> > +	return PTR_ERR_OR_ZERO(mgr);
> >  }
> >  EXPORT_SYMBOL_GPL(alt_pr_register);
> > 
> > diff --git a/drivers/fpga/altera-ps-spi.c b/drivers/fpga/altera-ps-spi.c
> > index 23bfd4d1ad0f..f544de2a9f6b 100644
> > --- a/drivers/fpga/altera-ps-spi.c
> > +++ b/drivers/fpga/altera-ps-spi.c
> > @@ -302,12 +302,9 @@ static int altera_ps_probe(struct spi_device *spi)
> >  	snprintf(conf->mgr_name, sizeof(conf->mgr_name), "%s %s",
> >  		 dev_driver_string(&spi->dev), dev_name(&spi->dev));
> > 
> > -	mgr = devm_fpga_mgr_create(&spi->dev, conf->mgr_name,
> > -				   &altera_ps_ops, conf);
> > -	if (!mgr)
> > -		return -ENOMEM;
> > -
> > -	return devm_fpga_mgr_register(&spi->dev, mgr);
> > +	mgr = devm_fpga_mgr_register_simple(&spi->dev, conf->mgr_name,
> > +					    &altera_ps_ops, conf);
> > +	return PTR_ERR_OR_ZERO(mgr);
> >  }
> > 
> >  static const struct spi_device_id altera_ps_spi_ids[] = {
> > diff --git a/drivers/fpga/dfl-fme-mgr.c b/drivers/fpga/dfl-fme-mgr.c
> > index d5861d13b306..20b1cf3d957f 100644
> > --- a/drivers/fpga/dfl-fme-mgr.c
> > +++ b/drivers/fpga/dfl-fme-mgr.c
> > @@ -282,7 +282,7 @@ static void fme_mgr_get_compat_id(void __iomem
> > *fme_pr,
> >  static int fme_mgr_probe(struct platform_device *pdev)
> >  {
> >  	struct dfl_fme_mgr_pdata *pdata = dev_get_platdata(&pdev->dev);
> > -	struct fpga_compat_id *compat_id;
> > +	struct fpga_manager_info info = { 0 };
> >  	struct device *dev = &pdev->dev;
> >  	struct fme_mgr_priv *priv;
> >  	struct fpga_manager *mgr;
> > @@ -302,20 +302,16 @@ static int fme_mgr_probe(struct platform_device
> > *pdev)
> >  			return PTR_ERR(priv->ioaddr);
> >  	}
> > 
> > -	compat_id = devm_kzalloc(dev, sizeof(*compat_id), GFP_KERNEL);
> > -	if (!compat_id)
> > +	info.name = "DFL FME FPGA Manager";
> > +	info.mops = &fme_mgr_ops;
> > +	info.priv = priv;
> > +	info.compat_id = devm_kzalloc(dev, sizeof(*info.compat_id),
> > GFP_KERNEL);
> > +	if (!info.compat_id)
> >  		return -ENOMEM;
> > 
> > -	fme_mgr_get_compat_id(priv->ioaddr, compat_id);
> > -
> > -	mgr = devm_fpga_mgr_create(dev, "DFL FME FPGA Manager",
> > -				   &fme_mgr_ops, priv);
> > -	if (!mgr)
> > -		return -ENOMEM;
> > -
> > -	mgr->compat_id = compat_id;
> > -
> > -	return devm_fpga_mgr_register(dev, mgr);
> > +	fme_mgr_get_compat_id(priv->ioaddr, info.compat_id);
> > +	mgr = devm_fpga_mgr_register(dev, &info);
> > +	return PTR_ERR_OR_ZERO(mgr);
> >  }
> > 
> >  static struct platform_driver fme_mgr_driver = {
> > diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c
> > index ecb4c3c795fa..6bd65a1b84fb 100644
> > --- a/drivers/fpga/fpga-mgr.c
> > +++ b/drivers/fpga/fpga-mgr.c
> > @@ -550,21 +550,19 @@ void fpga_mgr_unlock(struct fpga_manager *mgr)
> >  EXPORT_SYMBOL_GPL(fpga_mgr_unlock);
> > 
> >  /**
> > - * fpga_mgr_create - create and initialize an FPGA manager struct
> > + * fpga_mgr_register - create and register an FPGA Manager device
> >   * @parent:	fpga manager device from pdev
> > - * @name:	fpga manager name
> > - * @mops:	pointer to structure of fpga manager ops
> > - * @priv:	fpga manager private data
> > + * @info:	parameters for fpga manager
> >   *
> > - * The caller of this function is responsible for freeing the struct with
> > - * fpga_mgr_free().  Using devm_fpga_mgr_create() instead is recommended.
> > + * The caller of this function is responsible for calling fpga_mgr_unregister().
> > + * Using devm_fpga_mgr_register() instead is recommended.
> >   *
> > - * Return: pointer to struct fpga_manager or NULL
> > + * Return: pointer to struct fpga_manager pointer or ERR_PTR()
> >   */
> > -struct fpga_manager *fpga_mgr_create(struct device *parent, const char
> > *name,
> > -				     const struct fpga_manager_ops *mops,
> > -				     void *priv)
> > +struct fpga_manager *
> > +fpga_mgr_register(struct device *parent, const struct fpga_manager_info
> > *info)
> >  {
> > +	const struct fpga_manager_ops *mops = info->mops;
> >  	struct fpga_manager *mgr;
> >  	int id, ret;
> > 
> > @@ -572,29 +570,31 @@ struct fpga_manager *fpga_mgr_create(struct device
> > *parent, const char *name,
> >  	    !mops->write_init || (!mops->write && !mops->write_sg) ||
> >  	    (mops->write && mops->write_sg)) {
> >  		dev_err(parent, "Attempt to register without
> > fpga_manager_ops\n");
> > -		return NULL;
> > +		return ERR_PTR(-EINVAL);
> >  	}
> > 
> > -	if (!name || !strlen(name)) {
> > +	if (!info->name || !strlen(info->name)) {
> >  		dev_err(parent, "Attempt to register with no name!\n");
> > -		return NULL;
> > +		return ERR_PTR(-EINVAL);
> >  	}
> > 
> >  	mgr = kzalloc(sizeof(*mgr), GFP_KERNEL);
> >  	if (!mgr)
> > -		return NULL;
> > +		return ERR_PTR(-ENOMEM);
> > 
> >  	id = ida_simple_get(&fpga_mgr_ida, 0, 0, GFP_KERNEL);
> > -	if (id < 0)
> > +	if (id < 0) {
> > +		ret = id;
> >  		goto error_kfree;
> > +	}
> > 
> >  	mutex_init(&mgr->ref_mutex);
> > 
> > -	mgr->name = name;
> > -	mgr->mops = mops;
> > -	mgr->priv = priv;
> > +	mgr->name = info->name;
> > +	mgr->mops = info->mops;
> > +	mgr->priv = info->priv;
> > +	mgr->compat_id = info->compat_id;
> > 
> > -	device_initialize(&mgr->dev);
> >  	mgr->dev.class = fpga_mgr_class;
> >  	mgr->dev.groups = mops->groups;
> >  	mgr->dev.parent = parent;
> > @@ -605,6 +605,19 @@ struct fpga_manager *fpga_mgr_create(struct device
> > *parent, const char *name,
> >  	if (ret)
> >  		goto error_device;
> > 
> > +	/*
> > +	 * Initialize framework state by requesting low level driver read state
> > +	 * from device.  FPGA may be in reset mode or may have been
> > programmed
> > +	 * by bootloader or EEPROM.
> > +	 */
> > +	mgr->state = mgr->mops->state(mgr);
> > +
> > +	ret = device_register(&mgr->dev);
> > +	if (ret) {
> > +		put_device(&mgr->dev);
> > +		return ERR_PTR(ret);
> > +	}
> > +
> >  	return mgr;
> > 
> >  error_device:
> > @@ -612,98 +625,38 @@ struct fpga_manager *fpga_mgr_create(struct device
> > *parent, const char *name,
> >  error_kfree:
> >  	kfree(mgr);
> > 
> > -	return NULL;
> > -}
> > -EXPORT_SYMBOL_GPL(fpga_mgr_create);
> > -
> > -/**
> > - * fpga_mgr_free - free an FPGA manager created with fpga_mgr_create()
> > - * @mgr:	fpga manager struct
> > - */
> > -void fpga_mgr_free(struct fpga_manager *mgr)
> > -{
> > -	ida_simple_remove(&fpga_mgr_ida, mgr->dev.id);
> > -	kfree(mgr);
> > -}
> > -EXPORT_SYMBOL_GPL(fpga_mgr_free);
> > -
> > -static void devm_fpga_mgr_release(struct device *dev, void *res)
> > -{
> > -	struct fpga_mgr_devres *dr = res;
> > -
> > -	fpga_mgr_free(dr->mgr);
> > +	return ERR_PTR(ret);
> >  }
> > +EXPORT_SYMBOL_GPL(fpga_mgr_register);
> > 
> >  /**
> > - * devm_fpga_mgr_create - create and initialize a managed FPGA manager
> > struct
> > + * fpga_mgr_register_simple - create and register an FPGA Manager device
> >   * @parent:	fpga manager device from pdev
> >   * @name:	fpga manager name
> >   * @mops:	pointer to structure of fpga manager ops
> >   * @priv:	fpga manager private data
> >   *
> > - * This function is intended for use in an FPGA manager driver's probe function.
> > - * After the manager driver creates the manager struct with
> > - * devm_fpga_mgr_create(), it should register it with fpga_mgr_register().  The
> > - * manager driver's remove function should call fpga_mgr_unregister().  The
> > - * manager struct allocated with this function will be freed automatically on
> > - * driver detach.  This includes the case of a probe function returning error
> > - * before calling fpga_mgr_register(), the struct will still get cleaned up.
> > + * The caller of this function is responsible for calling fpga_mgr_unregister().
> > + * Using devm_fpga_mgr_register_simple() instead is recommended. This
> > simple
> > + * version of the register function should be sufficient for most users. The
> > + * fpga_mgr_register() function is available for users that need to pass
> > + * additional, optional parameters.
> >   *
> > - * Return: pointer to struct fpga_manager or NULL
> > + * Return: pointer to struct fpga_manager pointer or ERR_PTR()
> >   */
> > -struct fpga_manager *devm_fpga_mgr_create(struct device *parent, const
> > char *name,
> > -					  const struct fpga_manager_ops
> > *mops,
> > -					  void *priv)
> > +struct fpga_manager *
> > +fpga_mgr_register_simple(struct device *parent, const char *name,
> > +			 const struct fpga_manager_ops *mops, void *priv)
> >  {
> > -	struct fpga_mgr_devres *dr;
> > +	struct fpga_manager_info info = { 0 };
> > 
> > -	dr = devres_alloc(devm_fpga_mgr_release, sizeof(*dr), GFP_KERNEL);
> > -	if (!dr)
> > -		return NULL;
> > -
> > -	dr->mgr = fpga_mgr_create(parent, name, mops, priv);
> > -	if (!dr->mgr) {
> > -		devres_free(dr);
> > -		return NULL;
> > -	}
> > -
> > -	devres_add(parent, dr);
> > +	info.name = name;
> > +	info.mops = mops;
> > +	info.priv = priv;
> > 
> > -	return dr->mgr;
> > +	return fpga_mgr_register(parent, &info);
> >  }
> > -EXPORT_SYMBOL_GPL(devm_fpga_mgr_create);
> > -
> > -/**
> > - * fpga_mgr_register - register an FPGA manager
> > - * @mgr: fpga manager struct
> > - *
> > - * Return: 0 on success, negative error code otherwise.
> > - */
> > -int fpga_mgr_register(struct fpga_manager *mgr)
> > -{
> > -	int ret;
> > -
> > -	/*
> > -	 * Initialize framework state by requesting low level driver read state
> > -	 * from device.  FPGA may be in reset mode or may have been
> > programmed
> > -	 * by bootloader or EEPROM.
> > -	 */
> > -	mgr->state = mgr->mops->state(mgr);
> > -
> > -	ret = device_add(&mgr->dev);
> > -	if (ret)
> > -		goto error_device;
> > -
> > -	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);
> > +EXPORT_SYMBOL_GPL(fpga_mgr_register_simple);
> > 
> >  /**
> >   * fpga_mgr_unregister - unregister an FPGA manager
> > @@ -726,14 +679,6 @@ void fpga_mgr_unregister(struct fpga_manager *mgr)
> >  }
> >  EXPORT_SYMBOL_GPL(fpga_mgr_unregister);
> > 
> > -static int fpga_mgr_devres_match(struct device *dev, void *res,
> > -				 void *match_data)
> > -{
> > -	struct fpga_mgr_devres *dr = res;
> > -
> > -	return match_data == dr->mgr;
> > -}
> > -
> >  static void devm_fpga_mgr_unregister(struct device *dev, void *res)
> >  {
> >  	struct fpga_mgr_devres *dr = res;
> > @@ -743,44 +688,67 @@ static void devm_fpga_mgr_unregister(struct device
> > *dev, void *res)
> > 
> >  /**
> >   * devm_fpga_mgr_register - resource managed variant of fpga_mgr_register()
> > - * @dev: managing device for this FPGA manager
> > - * @mgr: fpga manager struct
> > + * @parent:	fpga manager device from pdev
> > + * @info:	parameters for fpga manager
> >   *
> >   * This is the devres variant of fpga_mgr_register() for which the unregister
> >   * function will be called automatically when the managing device is detached.
> >   */
> > -int devm_fpga_mgr_register(struct device *dev, struct fpga_manager *mgr)
> > +struct fpga_manager *
> > +devm_fpga_mgr_register(struct device *parent, const struct
> > fpga_manager_info *info)
> >  {
> >  	struct fpga_mgr_devres *dr;
> > -	int ret;
> > -
> > -	/*
> > -	 * Make sure that the struct fpga_manager * that is passed in is
> > -	 * managed itself.
> > -	 */
> > -	if (WARN_ON(!devres_find(dev, devm_fpga_mgr_release,
> > -				 fpga_mgr_devres_match, mgr)))
> > -		return -EINVAL;
> > +	struct fpga_manager *mgr;
> > 
> >  	dr = devres_alloc(devm_fpga_mgr_unregister, sizeof(*dr), GFP_KERNEL);
> >  	if (!dr)
> > -		return -ENOMEM;
> > +		return ERR_PTR(-ENOMEM);
> > 
> > -	ret = fpga_mgr_register(mgr);
> > -	if (ret) {
> > +	mgr = fpga_mgr_register(parent, info);
> > +	if (IS_ERR(mgr)) {
> >  		devres_free(dr);
> > -		return ret;
> > +		return mgr;
> >  	}
> > 
> >  	dr->mgr = mgr;
> > -	devres_add(dev, dr);
> > +	devres_add(parent, dr);
> > 
> > -	return 0;
> > +	return mgr;
> >  }
> >  EXPORT_SYMBOL_GPL(devm_fpga_mgr_register);
> > 
> > +/**
> > + * devm_fpga_mgr_register_simple - resource managed variant of
> > + *				   fpga_mgr_register_simple()
> > + * @parent:	fpga manager device from pdev
> > + * @name:	fpga manager name
> > + * @mops:	pointer to structure of fpga manager ops
> > + * @priv:	fpga manager private data
> > + *
> > + * This is the devres variant of fpga_mgr_register_simple() for which the
> > + * unregister function will be called automatically when the managing
> > + * device is detached.
> > + */
> > +struct fpga_manager *
> > +devm_fpga_mgr_register_simple(struct device *parent, const char *name,
> > +			      const struct fpga_manager_ops *mops, void *priv)
> > +{
> > +	struct fpga_manager_info info = { 0 };
> > +
> > +	info.name = name;
> > +	info.mops = mops;
> > +	info.priv = priv;
> > +
> > +	return devm_fpga_mgr_register(parent, &info);
> > +}
> > +EXPORT_SYMBOL_GPL(devm_fpga_mgr_register_simple);
> > +
> >  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)
> > diff --git a/drivers/fpga/ice40-spi.c b/drivers/fpga/ice40-spi.c
> > index 69dec5af23c3..448eb6f4e509 100644
> > --- a/drivers/fpga/ice40-spi.c
> > +++ b/drivers/fpga/ice40-spi.c
> > @@ -178,12 +178,9 @@ static int ice40_fpga_probe(struct spi_device *spi)
> >  		return ret;
> >  	}
> > 
> > -	mgr = devm_fpga_mgr_create(dev, "Lattice iCE40 FPGA Manager",
> > -				   &ice40_fpga_ops, priv);
> > -	if (!mgr)
> > -		return -ENOMEM;
> > -
> > -	return devm_fpga_mgr_register(dev, mgr);
> > +	mgr = devm_fpga_mgr_register_simple(dev, "Lattice iCE40 FPGA
> > Manager",
> > +					    &ice40_fpga_ops, priv);
> > +	return PTR_ERR_OR_ZERO(mgr);
> >  }
> > 
> >  static const struct of_device_id ice40_fpga_of_match[] = {
> > diff --git a/drivers/fpga/machxo2-spi.c b/drivers/fpga/machxo2-spi.c
> > index 114a64d2b7a4..77be11155402 100644
> > --- a/drivers/fpga/machxo2-spi.c
> > +++ b/drivers/fpga/machxo2-spi.c
> > @@ -366,12 +366,10 @@ static int machxo2_spi_probe(struct spi_device *spi)
> >  		return -EINVAL;
> >  	}
> > 
> > -	mgr = devm_fpga_mgr_create(dev, "Lattice MachXO2 SPI FPGA
> > Manager",
> > -				   &machxo2_ops, spi);
> > -	if (!mgr)
> > -		return -ENOMEM;
> > -
> > -	return devm_fpga_mgr_register(dev, mgr);
> > +	mgr = devm_fpga_mgr_register_simple(dev,
> > +					    "Lattice MachXO2 SPI FPGA
> > Manager",
> > +					    &machxo2_ops, spi);
> > +	return PTR_ERR_OR_ZERO(mgr);
> >  }
> > 
> >  static const struct of_device_id of_match[] = {
> > diff --git a/drivers/fpga/socfpga-a10.c b/drivers/fpga/socfpga-a10.c
> > index 573d88bdf730..5ffefaa3eb07 100644
> > --- a/drivers/fpga/socfpga-a10.c
> > +++ b/drivers/fpga/socfpga-a10.c
> > @@ -508,19 +508,15 @@ static int socfpga_a10_fpga_probe(struct
> > platform_device *pdev)
> >  		return -EBUSY;
> >  	}
> > 
> > -	mgr = devm_fpga_mgr_create(dev, "SoCFPGA Arria10 FPGA Manager",
> > -				   &socfpga_a10_fpga_mgr_ops, priv);
> > -	if (!mgr)
> > -		return -ENOMEM;
> > -
> > -	platform_set_drvdata(pdev, mgr);
> > -
> > -	ret = fpga_mgr_register(mgr);
> > -	if (ret) {
> > +	mgr = fpga_mgr_register_simple(dev, "SoCFPGA Arria10 FPGA Manager",
> > +				       &socfpga_a10_fpga_mgr_ops, priv);
> > +	if (IS_ERR(mgr)) {
> >  		clk_disable_unprepare(priv->clk);
> > -		return ret;
> > +		return PTR_ERR(mgr);
> >  	}
> > 
> > +	platform_set_drvdata(pdev, mgr);
> > +
> >  	return 0;
> >  }
> > 
> > diff --git a/drivers/fpga/socfpga.c b/drivers/fpga/socfpga.c
> > index 1f467173fc1f..9280db4364d4 100644
> > --- a/drivers/fpga/socfpga.c
> > +++ b/drivers/fpga/socfpga.c
> > @@ -571,12 +571,9 @@ static int socfpga_fpga_probe(struct platform_device
> > *pdev)
> >  	if (ret)
> >  		return ret;
> > 
> > -	mgr = devm_fpga_mgr_create(dev, "Altera SOCFPGA FPGA Manager",
> > -				   &socfpga_fpga_ops, priv);
> > -	if (!mgr)
> > -		return -ENOMEM;
> > -
> > -	return devm_fpga_mgr_register(dev, mgr);
> > +	mgr = devm_fpga_mgr_register_simple(dev, "Altera SOCFPGA FPGA
> > Manager",
> > +					    &socfpga_fpga_ops, priv);
> > +	return PTR_ERR_OR_ZERO(mgr);
> >  }
> > 
> >  #ifdef CONFIG_OF
> > diff --git a/drivers/fpga/stratix10-soc.c b/drivers/fpga/stratix10-soc.c
> > index a2cea500f7cc..fb84d88d4ce9 100644
> > --- a/drivers/fpga/stratix10-soc.c
> > +++ b/drivers/fpga/stratix10-soc.c
> > @@ -425,18 +425,11 @@ static int s10_probe(struct platform_device *pdev)
> > 
> >  	init_completion(&priv->status_return_completion);
> > 
> > -	mgr = fpga_mgr_create(dev, "Stratix10 SOC FPGA Manager",
> > -			      &s10_ops, priv);
> > -	if (!mgr) {
> > -		dev_err(dev, "unable to create FPGA manager\n");
> > -		ret = -ENOMEM;
> > -		goto probe_err;
> > -	}
> > -
> > -	ret = fpga_mgr_register(mgr);
> > -	if (ret) {
> > +	mgr = fpga_mgr_register_simple(dev, "Stratix10 SOC FPGA Manager",
> > +				       &s10_ops, priv);
> > +	if (IS_ERR(mgr)) {
> >  		dev_err(dev, "unable to register FPGA manager\n");
> > -		fpga_mgr_free(mgr);
> > +		ret = PTR_ERR(mgr);
> >  		goto probe_err;
> >  	}
> > 
> > @@ -454,7 +447,6 @@ 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;
> > diff --git a/drivers/fpga/ts73xx-fpga.c b/drivers/fpga/ts73xx-fpga.c
> > index 101f016c6ed8..009d5cfa1074 100644
> > --- a/drivers/fpga/ts73xx-fpga.c
> > +++ b/drivers/fpga/ts73xx-fpga.c
> > @@ -122,12 +122,9 @@ static int ts73xx_fpga_probe(struct platform_device
> > *pdev)
> >  	if (IS_ERR(priv->io_base))
> >  		return PTR_ERR(priv->io_base);
> > 
> > -	mgr = devm_fpga_mgr_create(kdev, "TS-73xx FPGA Manager",
> > -				   &ts73xx_fpga_ops, priv);
> > -	if (!mgr)
> > -		return -ENOMEM;
> > -
> > -	return devm_fpga_mgr_register(kdev, mgr);
> > +	mgr = devm_fpga_mgr_register_simple(kdev, "TS-73xx FPGA Manager",
> > +					    &ts73xx_fpga_ops, priv);
> > +	return PTR_ERR_OR_ZERO(mgr);
> >  }
> > 
> >  static struct platform_driver ts73xx_fpga_driver = {
> > diff --git a/drivers/fpga/xilinx-spi.c b/drivers/fpga/xilinx-spi.c
> > index fee4d0abf6bf..6f568a757cf2 100644
> > --- a/drivers/fpga/xilinx-spi.c
> > +++ b/drivers/fpga/xilinx-spi.c
> > @@ -247,13 +247,10 @@ static int xilinx_spi_probe(struct spi_device *spi)
> >  		return dev_err_probe(&spi->dev, PTR_ERR(conf->done),
> >  				     "Failed to get DONE gpio\n");
> > 
> > -	mgr = devm_fpga_mgr_create(&spi->dev,
> > -				   "Xilinx Slave Serial FPGA Manager",
> > -				   &xilinx_spi_ops, conf);
> > -	if (!mgr)
> > -		return -ENOMEM;
> > -
> > -	return devm_fpga_mgr_register(&spi->dev, mgr);
> > +	mgr = devm_fpga_mgr_register_simple(&spi->dev,
> > +					    "Xilinx Slave Serial FPGA Manager",
> > +					    &xilinx_spi_ops, conf);
> > +	return PTR_ERR_OR_ZERO(mgr);
> >  }
> > 
> >  static const struct of_device_id xlnx_spi_of_match[] = {
> > diff --git a/drivers/fpga/zynq-fpga.c b/drivers/fpga/zynq-fpga.c
> > index 9b75bd4f93d8..a3de365aadc6 100644
> > --- a/drivers/fpga/zynq-fpga.c
> > +++ b/drivers/fpga/zynq-fpga.c
> > @@ -609,20 +609,16 @@ static int zynq_fpga_probe(struct platform_device
> > *pdev)
> > 
> >  	clk_disable(priv->clk);
> > 
> > -	mgr = devm_fpga_mgr_create(dev, "Xilinx Zynq FPGA Manager",
> > -				   &zynq_fpga_ops, priv);
> > -	if (!mgr)
> > -		return -ENOMEM;
> > -
> > -	platform_set_drvdata(pdev, mgr);
> > -
> > -	err = fpga_mgr_register(mgr);
> > -	if (err) {
> > +	mgr = fpga_mgr_register_simple(dev, "Xilinx Zynq FPGA Manager",
> > +				       &zynq_fpga_ops, priv);
> > +	if (IS_ERR(mgr)) {
> >  		dev_err(dev, "unable to register FPGA manager\n");
> >  		clk_unprepare(priv->clk);
> > -		return err;
> > +		return PTR_ERR(mgr);
> >  	}
> > 
> > +	platform_set_drvdata(pdev, mgr);
> > +
> >  	return 0;
> >  }
> > 
> > diff --git a/drivers/fpga/zynqmp-fpga.c b/drivers/fpga/zynqmp-fpga.c
> > index 125743c9797f..53a7b36f61e8 100644
> > --- a/drivers/fpga/zynqmp-fpga.c
> > +++ b/drivers/fpga/zynqmp-fpga.c
> > @@ -102,12 +102,9 @@ static int zynqmp_fpga_probe(struct platform_device
> > *pdev)
> > 
> >  	priv->dev = dev;
> > 
> > -	mgr = devm_fpga_mgr_create(dev, "Xilinx ZynqMP FPGA Manager",
> > -				   &zynqmp_fpga_ops, priv);
> > -	if (!mgr)
> > -		return -ENOMEM;
> > -
> > -	return devm_fpga_mgr_register(dev, mgr);
> > +	mgr = devm_fpga_mgr_register_simple(dev, "Xilinx ZynqMP FPGA
> > Manager",
> > +					    &zynqmp_fpga_ops, priv);
> > +	return PTR_ERR_OR_ZERO(mgr);
> >  }
> > 
> >  static const struct of_device_id zynqmp_fpga_of_match[] = {
> > diff --git a/include/linux/fpga/fpga-mgr.h b/include/linux/fpga/fpga-mgr.h
> > index 474c1f506307..9efe55be93bc 100644
> > --- a/include/linux/fpga/fpga-mgr.h
> > +++ b/include/linux/fpga/fpga-mgr.h
> > @@ -105,6 +105,36 @@ struct fpga_image_info {
> >  #endif
> >  };
> > 
> > +/**
> > + * struct fpga_compat_id - id for compatibility check
> > + *
> > + * @id_h: high 64bit of the compat_id
> > + * @id_l: low 64bit of the compat_id
> > + */
> > +struct fpga_compat_id {
> > +	u64 id_h;
> > +	u64 id_l;
> > +};
> > +
> > +/**
> > + * struct fpga_manager_info - collection of parameters for an FPGA Manager
> > + * @name: fpga manager name
> > + * @compat_id: FPGA manager id for compatibility check.
> > + * @mops: pointer to structure of fpga manager ops
> > + * @priv: fpga manager private data
> > + *
> > + * fpga_manager_info contains parameters for the register function. These
> > + * are separated into an info structure because they some are optional
> > + * others could be added to in the future. The info structure facilitates
> > + * maintaining a stable API.
> > + */
> > +struct fpga_manager_info {
> > +	const char *name;
> > +	struct fpga_compat_id *compat_id;
> > +	const struct fpga_manager_ops *mops;
> > +	void *priv;
> > +};
> > +
> >  /**
> >   * struct fpga_manager_ops - ops for low level fpga manager drivers
> >   * @initial_header_size: Maximum number of bytes that should be passed into
> > write_init
> > @@ -143,17 +173,6 @@ struct fpga_manager_ops {
> >  #define FPGA_MGR_STATUS_IP_PROTOCOL_ERR		BIT(3)
> >  #define FPGA_MGR_STATUS_FIFO_OVERFLOW_ERR	BIT(4)
> > 
> > -/**
> > - * struct fpga_compat_id - id for compatibility check
> > - *
> > - * @id_h: high 64bit of the compat_id
> > - * @id_l: low 64bit of the compat_id
> > - */
> > -struct fpga_compat_id {
> > -	u64 id_h;
> > -	u64 id_l;
> > -};
> > -
> >  /**
> >   * struct fpga_manager - fpga manager structure
> >   * @name: name of low level fpga manager
> > @@ -191,17 +210,18 @@ struct fpga_manager *fpga_mgr_get(struct device
> > *dev);
> > 
> >  void fpga_mgr_put(struct fpga_manager *mgr);
> > 
> > -struct fpga_manager *fpga_mgr_create(struct device *dev, const char *name,
> > -				     const struct fpga_manager_ops *mops,
> > -				     void *priv);
> > -void fpga_mgr_free(struct fpga_manager *mgr);
> > -int fpga_mgr_register(struct fpga_manager *mgr);
> > -void fpga_mgr_unregister(struct fpga_manager *mgr);
> > +struct fpga_manager *
> > +fpga_mgr_register(struct device *parent, const struct fpga_manager_info
> > *info);
> > 
> > -int devm_fpga_mgr_register(struct device *dev, struct fpga_manager *mgr);
> > +struct fpga_manager *
> > +fpga_mgr_register_simple(struct device *parent, const char *name,
> > +			 const struct fpga_manager_ops *mops, void *priv);
> > +void fpga_mgr_unregister(struct fpga_manager *mgr);
> > 
> > -struct fpga_manager *devm_fpga_mgr_create(struct device *dev, const char
> > *name,
> > -					  const struct fpga_manager_ops
> > *mops,
> > -					  void *priv);
> > +struct fpga_manager *
> > +devm_fpga_mgr_register(struct device *parent, const struct
> > fpga_manager_info *info);
> > +struct fpga_manager *
> > +devm_fpga_mgr_register_simple(struct device *parent, const char *name,
> > +			      const struct fpga_manager_ops *mops, void *priv);
> > 
> >  #endif /*_LINUX_FPGA_MGR_H */
> > --
> > 2.25.1
> 

- Moritz

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v8 1/3] fpga: mgr: Use standard dev_release for class driver
  2021-06-28  3:49   ` Wu, Hao
  2021-06-28 18:51     ` Moritz Fischer
@ 2021-06-30 22:12     ` Russ Weight
  1 sibling, 0 replies; 10+ messages in thread
From: Russ Weight @ 2021-06-30 22:12 UTC (permalink / raw)
  To: Wu, Hao, mdf, linux-fpga
  Cc: trix, lgoncalv, Xu, Yilun, Gerlach, Matthew, Gong, Richard



On 6/27/21 8:49 PM, Wu, Hao wrote:
>> -----Original Message-----
>> From: Weight, Russell H <russell.h.weight@intel.com>
>> Sent: Friday, June 25, 2021 4:54 AM
>> To: mdf@kernel.org; linux-fpga@vger.kernel.org
>> Cc: trix@redhat.com; lgoncalv@redhat.com; Xu, Yilun <yilun.xu@intel.com>;
>> Wu, Hao <hao.wu@intel.com>; Gerlach, Matthew
>> <matthew.gerlach@intel.com>; Gong, Richard <richard.gong@intel.com>;
>> Weight, Russell H <russell.h.weight@intel.com>
>> Subject: [PATCH v8 1/3] fpga: mgr: Use standard dev_release for class driver
>>
>> The FPGA manager class driver data structure is being treated as a
>> managed resource instead of using the standard dev_release call-back
>> function to release the class data structure. This change removes
>> the managed resource code for the freeing of the class data structure
>> and combines the create() and register() functions into a single
>> register() or register_simple() function.
>>
> Maybe register() and register_full() is better. it keeps the most used
> function name the same (and shorter than register_simple()). Just
> my 2 cents. Let's wait for review comments from Moritz to decide. : )

OK - I'm making this changes for the next version.

- Russ

>
> Thanks
> Hao
>
>> The register() function accepts an info data structure to provide
>> flexibility in passing optional parameters. The register_simple()
>> function supports the current parameter list for users that don't
>> require the use of optional parameters.
>>
>> The devm_fpga_mgr_register() function is retained, and the
>> devm_fpga_mgr_register_simple() function is added.
>>
>> Signed-off-by: Russ Weight <russell.h.weight@intel.com>
>> Reviewed-by: Xu Yilun <yilun.xu@intel.com>
>> ---
>> v8:
>>   - Added reviewed-by tag.
>>   - Updated Documentation/driver-api/fpga/fpga-mgr.rst documentation.
>> v7:
>>   - Update the commit message to describe the new parameters for the
>>     *fpga_mgr_register() functions and to mention the
>>     *fpga_mgr_register_simple() functions.
>>   - Fix function prototypes in header file to rename dev to parent.
>>   - Make use of the PTR_ERR_OR_ZERO() macro when possible.
>>   - Some cleanup of comments.
>>   - Update function defintions/prototypes to apply const to the new info
>>     parameter.
>> v6:
>>   - Moved FPGA manager optional parameters out of the ops structure and
>>     back into the FPGA manager structure.
>>   - Changed fpga_mgr_register()/devm_fpga_mgr_register() parameters to
>>     accept an info data structure to provide flexibility in passing optional
>>     parameters.
>>   - Added fpga_mgr_register_simple()/devm_fpga_mgr_register_simple()
>>     functions to support current parameters for users that don't require
>>     the use of optional parameters.
>> v5:
>>   - Rebased on top of recently accepted patches.
>>   - Removed compat_id from the fpga_mgr_register() parameter list
>>     and added it to the fpga_manager_ops structure. This also required
>>     dynamically allocating the dfl-fme-ops structure in order to add
>>     the appropriate compat_id.
>> v4:
>>   - Added the compat_id parameter to fpga_mgr_register() and
>>     devm_fpga_mgr_register() to ensure that the compat_id is set before
>>     the device_register() call.
>> v3:
>>   - Cleaned up comment header for fpga_mgr_register()
>>   - Fix error return on ida_simple_get() failure
>> v2:
>>   - Restored devm_fpga_mgr_register() functionality, adapted for the combined
>>     create/register functionality.
>>   - All previous callers of devm_fpga_mgr_register() will continue to call
>>     devm_fpga_mgr_register().
>>   - replaced unnecessary ternary operators in return statements with standard
>>     if conditions.
>> ---
>>  Documentation/driver-api/fpga/fpga-mgr.rst |  39 ++--
>>  drivers/fpga/altera-cvp.c                  |  12 +-
>>  drivers/fpga/altera-pr-ip-core.c           |   8 +-
>>  drivers/fpga/altera-ps-spi.c               |   9 +-
>>  drivers/fpga/dfl-fme-mgr.c                 |  22 +--
>>  drivers/fpga/fpga-mgr.c                    | 214 +++++++++------------
>>  drivers/fpga/ice40-spi.c                   |   9 +-
>>  drivers/fpga/machxo2-spi.c                 |  10 +-
>>  drivers/fpga/socfpga-a10.c                 |  16 +-
>>  drivers/fpga/socfpga.c                     |   9 +-
>>  drivers/fpga/stratix10-soc.c               |  16 +-
>>  drivers/fpga/ts73xx-fpga.c                 |   9 +-
>>  drivers/fpga/xilinx-spi.c                  |  11 +-
>>  drivers/fpga/zynq-fpga.c                   |  16 +-
>>  drivers/fpga/zynqmp-fpga.c                 |   9 +-
>>  include/linux/fpga/fpga-mgr.h              |  62 ++++--
>>  16 files changed, 204 insertions(+), 267 deletions(-)
>>
>> diff --git a/Documentation/driver-api/fpga/fpga-mgr.rst
>> b/Documentation/driver-api/fpga/fpga-mgr.rst
>> index 4d926b452cb3..fbb7f07cae9f 100644
>> --- a/Documentation/driver-api/fpga/fpga-mgr.rst
>> +++ b/Documentation/driver-api/fpga/fpga-mgr.rst
>> @@ -24,7 +24,8 @@ How to support a new FPGA device
>>  --------------------------------
>>
>>  To add another FPGA manager, write a driver that implements a set of ops.  The
>> -probe function calls fpga_mgr_register(), such as::
>> +probe function calls devm_fpga_mgr_register() or
>> +devm_fpga_mgr_register_simple(), such as::
>>
>>       static const struct fpga_manager_ops socfpga_fpga_ops = {
>>               .write_init = socfpga_fpga_ops_configure_init,
>> @@ -49,23 +50,9 @@ probe function calls fpga_mgr_register(), such as::
>>                * them in priv
>>                */
>>
>> -             mgr = devm_fpga_mgr_create(dev, "Altera SOCFPGA FPGA
>> Manager",
>> -                                        &socfpga_fpga_ops, priv);
>> -             if (!mgr)
>> -                     return -ENOMEM;
>> -
>> -             platform_set_drvdata(pdev, mgr);
>> -
>> -             return fpga_mgr_register(mgr);
>> -     }
>> -
>> -     static int socfpga_fpga_remove(struct platform_device *pdev)
>> -     {
>> -             struct fpga_manager *mgr = platform_get_drvdata(pdev);
>> -
>> -             fpga_mgr_unregister(mgr);
>> -
>> -             return 0;
>> +             mgr = devm_fpga_mgr_register_simple(dev, "Altera SOCFPGA
>> FPGA Manager",
>> +                                                 &socfpga_fpga_ops, priv);
>> +             return PTR_ERR_OR_ZERO(mgr);
>>       }
>>
>>
>> @@ -104,8 +91,10 @@ API for implementing a new FPGA Manager driver
>>  * ``fpga_mgr_states`` -  Values for :c:expr:`fpga_manager->state`.
>>  * struct fpga_manager -  the FPGA manager struct
>>  * struct fpga_manager_ops -  Low level FPGA manager driver ops
>> -* devm_fpga_mgr_create() -  Allocate and init a manager struct
>> -* fpga_mgr_register() -  Register an FPGA manager
>> +* fpga_mgr_register() -  Create and register an FPGA manager
>> +* fpga_mgr_register_simple() -  Create and register an FPGA manager
>> +* devm_fpga_mgr_register() -  Create and register an FPGA manager
>> +* devm_fpga_mgr_register_simple() -  Create and register an FPGA manager
>>  * fpga_mgr_unregister() -  Unregister an FPGA manager
>>
>>  .. kernel-doc:: include/linux/fpga/fpga-mgr.h
>> @@ -118,10 +107,16 @@ API for implementing a new FPGA Manager driver
>>     :functions: fpga_manager_ops
>>
>>  .. kernel-doc:: drivers/fpga/fpga-mgr.c
>> -   :functions: devm_fpga_mgr_create
>> +   :functions: fpga_mgr_register
>>
>>  .. kernel-doc:: drivers/fpga/fpga-mgr.c
>> -   :functions: fpga_mgr_register
>> +   :functions: fpga_mgr_register_simple
>> +
>> +.. kernel-doc:: drivers/fpga/fpga-mgr.c
>> +   :functions: devm_fpga_mgr_register
>> +
>> +.. kernel-doc:: drivers/fpga/fpga-mgr.c
>> +   :functions: devm_fpga_mgr_register_simple
>>
>>  .. kernel-doc:: drivers/fpga/fpga-mgr.c
>>     :functions: fpga_mgr_unregister
>> diff --git a/drivers/fpga/altera-cvp.c b/drivers/fpga/altera-cvp.c
>> index ccf4546eff29..ddf2ffe3f138 100644
>> --- a/drivers/fpga/altera-cvp.c
>> +++ b/drivers/fpga/altera-cvp.c
>> @@ -652,19 +652,15 @@ static int altera_cvp_probe(struct pci_dev *pdev,
>>       snprintf(conf->mgr_name, sizeof(conf->mgr_name), "%s @%s",
>>                ALTERA_CVP_MGR_NAME, pci_name(pdev));
>>
>> -     mgr = devm_fpga_mgr_create(&pdev->dev, conf->mgr_name,
>> -                                &altera_cvp_ops, conf);
>> -     if (!mgr) {
>> -             ret = -ENOMEM;
>> +     mgr = fpga_mgr_register_simple(&pdev->dev, conf->mgr_name,
>> +                                    &altera_cvp_ops, conf);
>> +     if (IS_ERR(mgr)) {
>> +             ret = PTR_ERR(mgr);
>>               goto err_unmap;
>>       }
>>
>>       pci_set_drvdata(pdev, mgr);
>>
>> -     ret = fpga_mgr_register(mgr);
>> -     if (ret)
>> -             goto err_unmap;
>> -
>>       return 0;
>>
>>  err_unmap:
>> diff --git a/drivers/fpga/altera-pr-ip-core.c b/drivers/fpga/altera-pr-ip-core.c
>> index dfdf21ed34c4..e0baf3392b67 100644
>> --- a/drivers/fpga/altera-pr-ip-core.c
>> +++ b/drivers/fpga/altera-pr-ip-core.c
>> @@ -191,11 +191,9 @@ int alt_pr_register(struct device *dev, void __iomem
>> *reg_base)
>>               (val & ALT_PR_CSR_STATUS_MSK) >> ALT_PR_CSR_STATUS_SFT,
>>               (int)(val & ALT_PR_CSR_PR_START));
>>
>> -     mgr = devm_fpga_mgr_create(dev, dev_name(dev), &alt_pr_ops, priv);
>> -     if (!mgr)
>> -             return -ENOMEM;
>> -
>> -     return devm_fpga_mgr_register(dev, mgr);
>> +     mgr = devm_fpga_mgr_register_simple(dev, dev_name(dev),
>> +                                         &alt_pr_ops, priv);
>> +     return PTR_ERR_OR_ZERO(mgr);
>>  }
>>  EXPORT_SYMBOL_GPL(alt_pr_register);
>>
>> diff --git a/drivers/fpga/altera-ps-spi.c b/drivers/fpga/altera-ps-spi.c
>> index 23bfd4d1ad0f..f544de2a9f6b 100644
>> --- a/drivers/fpga/altera-ps-spi.c
>> +++ b/drivers/fpga/altera-ps-spi.c
>> @@ -302,12 +302,9 @@ static int altera_ps_probe(struct spi_device *spi)
>>       snprintf(conf->mgr_name, sizeof(conf->mgr_name), "%s %s",
>>                dev_driver_string(&spi->dev), dev_name(&spi->dev));
>>
>> -     mgr = devm_fpga_mgr_create(&spi->dev, conf->mgr_name,
>> -                                &altera_ps_ops, conf);
>> -     if (!mgr)
>> -             return -ENOMEM;
>> -
>> -     return devm_fpga_mgr_register(&spi->dev, mgr);
>> +     mgr = devm_fpga_mgr_register_simple(&spi->dev, conf->mgr_name,
>> +                                         &altera_ps_ops, conf);
>> +     return PTR_ERR_OR_ZERO(mgr);
>>  }
>>
>>  static const struct spi_device_id altera_ps_spi_ids[] = {
>> diff --git a/drivers/fpga/dfl-fme-mgr.c b/drivers/fpga/dfl-fme-mgr.c
>> index d5861d13b306..20b1cf3d957f 100644
>> --- a/drivers/fpga/dfl-fme-mgr.c
>> +++ b/drivers/fpga/dfl-fme-mgr.c
>> @@ -282,7 +282,7 @@ static void fme_mgr_get_compat_id(void __iomem
>> *fme_pr,
>>  static int fme_mgr_probe(struct platform_device *pdev)
>>  {
>>       struct dfl_fme_mgr_pdata *pdata = dev_get_platdata(&pdev->dev);
>> -     struct fpga_compat_id *compat_id;
>> +     struct fpga_manager_info info = { 0 };
>>       struct device *dev = &pdev->dev;
>>       struct fme_mgr_priv *priv;
>>       struct fpga_manager *mgr;
>> @@ -302,20 +302,16 @@ static int fme_mgr_probe(struct platform_device
>> *pdev)
>>                       return PTR_ERR(priv->ioaddr);
>>       }
>>
>> -     compat_id = devm_kzalloc(dev, sizeof(*compat_id), GFP_KERNEL);
>> -     if (!compat_id)
>> +     info.name = "DFL FME FPGA Manager";
>> +     info.mops = &fme_mgr_ops;
>> +     info.priv = priv;
>> +     info.compat_id = devm_kzalloc(dev, sizeof(*info.compat_id),
>> GFP_KERNEL);
>> +     if (!info.compat_id)
>>               return -ENOMEM;
>>
>> -     fme_mgr_get_compat_id(priv->ioaddr, compat_id);
>> -
>> -     mgr = devm_fpga_mgr_create(dev, "DFL FME FPGA Manager",
>> -                                &fme_mgr_ops, priv);
>> -     if (!mgr)
>> -             return -ENOMEM;
>> -
>> -     mgr->compat_id = compat_id;
>> -
>> -     return devm_fpga_mgr_register(dev, mgr);
>> +     fme_mgr_get_compat_id(priv->ioaddr, info.compat_id);
>> +     mgr = devm_fpga_mgr_register(dev, &info);
>> +     return PTR_ERR_OR_ZERO(mgr);
>>  }
>>
>>  static struct platform_driver fme_mgr_driver = {
>> diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c
>> index ecb4c3c795fa..6bd65a1b84fb 100644
>> --- a/drivers/fpga/fpga-mgr.c
>> +++ b/drivers/fpga/fpga-mgr.c
>> @@ -550,21 +550,19 @@ void fpga_mgr_unlock(struct fpga_manager *mgr)
>>  EXPORT_SYMBOL_GPL(fpga_mgr_unlock);
>>
>>  /**
>> - * fpga_mgr_create - create and initialize an FPGA manager struct
>> + * fpga_mgr_register - create and register an FPGA Manager device
>>   * @parent:  fpga manager device from pdev
>> - * @name:    fpga manager name
>> - * @mops:    pointer to structure of fpga manager ops
>> - * @priv:    fpga manager private data
>> + * @info:    parameters for fpga manager
>>   *
>> - * The caller of this function is responsible for freeing the struct with
>> - * fpga_mgr_free().  Using devm_fpga_mgr_create() instead is recommended.
>> + * The caller of this function is responsible for calling fpga_mgr_unregister().
>> + * Using devm_fpga_mgr_register() instead is recommended.
>>   *
>> - * Return: pointer to struct fpga_manager or NULL
>> + * Return: pointer to struct fpga_manager pointer or ERR_PTR()
>>   */
>> -struct fpga_manager *fpga_mgr_create(struct device *parent, const char
>> *name,
>> -                                  const struct fpga_manager_ops *mops,
>> -                                  void *priv)
>> +struct fpga_manager *
>> +fpga_mgr_register(struct device *parent, const struct fpga_manager_info
>> *info)
>>  {
>> +     const struct fpga_manager_ops *mops = info->mops;
>>       struct fpga_manager *mgr;
>>       int id, ret;
>>
>> @@ -572,29 +570,31 @@ struct fpga_manager *fpga_mgr_create(struct device
>> *parent, const char *name,
>>           !mops->write_init || (!mops->write && !mops->write_sg) ||
>>           (mops->write && mops->write_sg)) {
>>               dev_err(parent, "Attempt to register without
>> fpga_manager_ops\n");
>> -             return NULL;
>> +             return ERR_PTR(-EINVAL);
>>       }
>>
>> -     if (!name || !strlen(name)) {
>> +     if (!info->name || !strlen(info->name)) {
>>               dev_err(parent, "Attempt to register with no name!\n");
>> -             return NULL;
>> +             return ERR_PTR(-EINVAL);
>>       }
>>
>>       mgr = kzalloc(sizeof(*mgr), GFP_KERNEL);
>>       if (!mgr)
>> -             return NULL;
>> +             return ERR_PTR(-ENOMEM);
>>
>>       id = ida_simple_get(&fpga_mgr_ida, 0, 0, GFP_KERNEL);
>> -     if (id < 0)
>> +     if (id < 0) {
>> +             ret = id;
>>               goto error_kfree;
>> +     }
>>
>>       mutex_init(&mgr->ref_mutex);
>>
>> -     mgr->name = name;
>> -     mgr->mops = mops;
>> -     mgr->priv = priv;
>> +     mgr->name = info->name;
>> +     mgr->mops = info->mops;
>> +     mgr->priv = info->priv;
>> +     mgr->compat_id = info->compat_id;
>>
>> -     device_initialize(&mgr->dev);
>>       mgr->dev.class = fpga_mgr_class;
>>       mgr->dev.groups = mops->groups;
>>       mgr->dev.parent = parent;
>> @@ -605,6 +605,19 @@ struct fpga_manager *fpga_mgr_create(struct device
>> *parent, const char *name,
>>       if (ret)
>>               goto error_device;
>>
>> +     /*
>> +      * Initialize framework state by requesting low level driver read state
>> +      * from device.  FPGA may be in reset mode or may have been
>> programmed
>> +      * by bootloader or EEPROM.
>> +      */
>> +     mgr->state = mgr->mops->state(mgr);
>> +
>> +     ret = device_register(&mgr->dev);
>> +     if (ret) {
>> +             put_device(&mgr->dev);
>> +             return ERR_PTR(ret);
>> +     }
>> +
>>       return mgr;
>>
>>  error_device:
>> @@ -612,98 +625,38 @@ struct fpga_manager *fpga_mgr_create(struct device
>> *parent, const char *name,
>>  error_kfree:
>>       kfree(mgr);
>>
>> -     return NULL;
>> -}
>> -EXPORT_SYMBOL_GPL(fpga_mgr_create);
>> -
>> -/**
>> - * fpga_mgr_free - free an FPGA manager created with fpga_mgr_create()
>> - * @mgr:     fpga manager struct
>> - */
>> -void fpga_mgr_free(struct fpga_manager *mgr)
>> -{
>> -     ida_simple_remove(&fpga_mgr_ida, mgr->dev.id);
>> -     kfree(mgr);
>> -}
>> -EXPORT_SYMBOL_GPL(fpga_mgr_free);
>> -
>> -static void devm_fpga_mgr_release(struct device *dev, void *res)
>> -{
>> -     struct fpga_mgr_devres *dr = res;
>> -
>> -     fpga_mgr_free(dr->mgr);
>> +     return ERR_PTR(ret);
>>  }
>> +EXPORT_SYMBOL_GPL(fpga_mgr_register);
>>
>>  /**
>> - * devm_fpga_mgr_create - create and initialize a managed FPGA manager
>> struct
>> + * fpga_mgr_register_simple - create and register an FPGA Manager device
>>   * @parent:  fpga manager device from pdev
>>   * @name:    fpga manager name
>>   * @mops:    pointer to structure of fpga manager ops
>>   * @priv:    fpga manager private data
>>   *
>> - * This function is intended for use in an FPGA manager driver's probe function.
>> - * After the manager driver creates the manager struct with
>> - * devm_fpga_mgr_create(), it should register it with fpga_mgr_register().  The
>> - * manager driver's remove function should call fpga_mgr_unregister().  The
>> - * manager struct allocated with this function will be freed automatically on
>> - * driver detach.  This includes the case of a probe function returning error
>> - * before calling fpga_mgr_register(), the struct will still get cleaned up.
>> + * The caller of this function is responsible for calling fpga_mgr_unregister().
>> + * Using devm_fpga_mgr_register_simple() instead is recommended. This
>> simple
>> + * version of the register function should be sufficient for most users. The
>> + * fpga_mgr_register() function is available for users that need to pass
>> + * additional, optional parameters.
>>   *
>> - * Return: pointer to struct fpga_manager or NULL
>> + * Return: pointer to struct fpga_manager pointer or ERR_PTR()
>>   */
>> -struct fpga_manager *devm_fpga_mgr_create(struct device *parent, const
>> char *name,
>> -                                       const struct fpga_manager_ops
>> *mops,
>> -                                       void *priv)
>> +struct fpga_manager *
>> +fpga_mgr_register_simple(struct device *parent, const char *name,
>> +                      const struct fpga_manager_ops *mops, void *priv)
>>  {
>> -     struct fpga_mgr_devres *dr;
>> +     struct fpga_manager_info info = { 0 };
>>
>> -     dr = devres_alloc(devm_fpga_mgr_release, sizeof(*dr), GFP_KERNEL);
>> -     if (!dr)
>> -             return NULL;
>> -
>> -     dr->mgr = fpga_mgr_create(parent, name, mops, priv);
>> -     if (!dr->mgr) {
>> -             devres_free(dr);
>> -             return NULL;
>> -     }
>> -
>> -     devres_add(parent, dr);
>> +     info.name = name;
>> +     info.mops = mops;
>> +     info.priv = priv;
>>
>> -     return dr->mgr;
>> +     return fpga_mgr_register(parent, &info);
>>  }
>> -EXPORT_SYMBOL_GPL(devm_fpga_mgr_create);
>> -
>> -/**
>> - * fpga_mgr_register - register an FPGA manager
>> - * @mgr: fpga manager struct
>> - *
>> - * Return: 0 on success, negative error code otherwise.
>> - */
>> -int fpga_mgr_register(struct fpga_manager *mgr)
>> -{
>> -     int ret;
>> -
>> -     /*
>> -      * Initialize framework state by requesting low level driver read state
>> -      * from device.  FPGA may be in reset mode or may have been
>> programmed
>> -      * by bootloader or EEPROM.
>> -      */
>> -     mgr->state = mgr->mops->state(mgr);
>> -
>> -     ret = device_add(&mgr->dev);
>> -     if (ret)
>> -             goto error_device;
>> -
>> -     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);
>> +EXPORT_SYMBOL_GPL(fpga_mgr_register_simple);
>>
>>  /**
>>   * fpga_mgr_unregister - unregister an FPGA manager
>> @@ -726,14 +679,6 @@ void fpga_mgr_unregister(struct fpga_manager *mgr)
>>  }
>>  EXPORT_SYMBOL_GPL(fpga_mgr_unregister);
>>
>> -static int fpga_mgr_devres_match(struct device *dev, void *res,
>> -                              void *match_data)
>> -{
>> -     struct fpga_mgr_devres *dr = res;
>> -
>> -     return match_data == dr->mgr;
>> -}
>> -
>>  static void devm_fpga_mgr_unregister(struct device *dev, void *res)
>>  {
>>       struct fpga_mgr_devres *dr = res;
>> @@ -743,44 +688,67 @@ static void devm_fpga_mgr_unregister(struct device
>> *dev, void *res)
>>
>>  /**
>>   * devm_fpga_mgr_register - resource managed variant of fpga_mgr_register()
>> - * @dev: managing device for this FPGA manager
>> - * @mgr: fpga manager struct
>> + * @parent:  fpga manager device from pdev
>> + * @info:    parameters for fpga manager
>>   *
>>   * This is the devres variant of fpga_mgr_register() for which the unregister
>>   * function will be called automatically when the managing device is detached.
>>   */
>> -int devm_fpga_mgr_register(struct device *dev, struct fpga_manager *mgr)
>> +struct fpga_manager *
>> +devm_fpga_mgr_register(struct device *parent, const struct
>> fpga_manager_info *info)
>>  {
>>       struct fpga_mgr_devres *dr;
>> -     int ret;
>> -
>> -     /*
>> -      * Make sure that the struct fpga_manager * that is passed in is
>> -      * managed itself.
>> -      */
>> -     if (WARN_ON(!devres_find(dev, devm_fpga_mgr_release,
>> -                              fpga_mgr_devres_match, mgr)))
>> -             return -EINVAL;
>> +     struct fpga_manager *mgr;
>>
>>       dr = devres_alloc(devm_fpga_mgr_unregister, sizeof(*dr), GFP_KERNEL);
>>       if (!dr)
>> -             return -ENOMEM;
>> +             return ERR_PTR(-ENOMEM);
>>
>> -     ret = fpga_mgr_register(mgr);
>> -     if (ret) {
>> +     mgr = fpga_mgr_register(parent, info);
>> +     if (IS_ERR(mgr)) {
>>               devres_free(dr);
>> -             return ret;
>> +             return mgr;
>>       }
>>
>>       dr->mgr = mgr;
>> -     devres_add(dev, dr);
>> +     devres_add(parent, dr);
>>
>> -     return 0;
>> +     return mgr;
>>  }
>>  EXPORT_SYMBOL_GPL(devm_fpga_mgr_register);
>>
>> +/**
>> + * devm_fpga_mgr_register_simple - resource managed variant of
>> + *                              fpga_mgr_register_simple()
>> + * @parent:  fpga manager device from pdev
>> + * @name:    fpga manager name
>> + * @mops:    pointer to structure of fpga manager ops
>> + * @priv:    fpga manager private data
>> + *
>> + * This is the devres variant of fpga_mgr_register_simple() for which the
>> + * unregister function will be called automatically when the managing
>> + * device is detached.
>> + */
>> +struct fpga_manager *
>> +devm_fpga_mgr_register_simple(struct device *parent, const char *name,
>> +                           const struct fpga_manager_ops *mops, void *priv)
>> +{
>> +     struct fpga_manager_info info = { 0 };
>> +
>> +     info.name = name;
>> +     info.mops = mops;
>> +     info.priv = priv;
>> +
>> +     return devm_fpga_mgr_register(parent, &info);
>> +}
>> +EXPORT_SYMBOL_GPL(devm_fpga_mgr_register_simple);
>> +
>>  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)
>> diff --git a/drivers/fpga/ice40-spi.c b/drivers/fpga/ice40-spi.c
>> index 69dec5af23c3..448eb6f4e509 100644
>> --- a/drivers/fpga/ice40-spi.c
>> +++ b/drivers/fpga/ice40-spi.c
>> @@ -178,12 +178,9 @@ static int ice40_fpga_probe(struct spi_device *spi)
>>               return ret;
>>       }
>>
>> -     mgr = devm_fpga_mgr_create(dev, "Lattice iCE40 FPGA Manager",
>> -                                &ice40_fpga_ops, priv);
>> -     if (!mgr)
>> -             return -ENOMEM;
>> -
>> -     return devm_fpga_mgr_register(dev, mgr);
>> +     mgr = devm_fpga_mgr_register_simple(dev, "Lattice iCE40 FPGA
>> Manager",
>> +                                         &ice40_fpga_ops, priv);
>> +     return PTR_ERR_OR_ZERO(mgr);
>>  }
>>
>>  static const struct of_device_id ice40_fpga_of_match[] = {
>> diff --git a/drivers/fpga/machxo2-spi.c b/drivers/fpga/machxo2-spi.c
>> index 114a64d2b7a4..77be11155402 100644
>> --- a/drivers/fpga/machxo2-spi.c
>> +++ b/drivers/fpga/machxo2-spi.c
>> @@ -366,12 +366,10 @@ static int machxo2_spi_probe(struct spi_device *spi)
>>               return -EINVAL;
>>       }
>>
>> -     mgr = devm_fpga_mgr_create(dev, "Lattice MachXO2 SPI FPGA
>> Manager",
>> -                                &machxo2_ops, spi);
>> -     if (!mgr)
>> -             return -ENOMEM;
>> -
>> -     return devm_fpga_mgr_register(dev, mgr);
>> +     mgr = devm_fpga_mgr_register_simple(dev,
>> +                                         "Lattice MachXO2 SPI FPGA
>> Manager",
>> +                                         &machxo2_ops, spi);
>> +     return PTR_ERR_OR_ZERO(mgr);
>>  }
>>
>>  static const struct of_device_id of_match[] = {
>> diff --git a/drivers/fpga/socfpga-a10.c b/drivers/fpga/socfpga-a10.c
>> index 573d88bdf730..5ffefaa3eb07 100644
>> --- a/drivers/fpga/socfpga-a10.c
>> +++ b/drivers/fpga/socfpga-a10.c
>> @@ -508,19 +508,15 @@ static int socfpga_a10_fpga_probe(struct
>> platform_device *pdev)
>>               return -EBUSY;
>>       }
>>
>> -     mgr = devm_fpga_mgr_create(dev, "SoCFPGA Arria10 FPGA Manager",
>> -                                &socfpga_a10_fpga_mgr_ops, priv);
>> -     if (!mgr)
>> -             return -ENOMEM;
>> -
>> -     platform_set_drvdata(pdev, mgr);
>> -
>> -     ret = fpga_mgr_register(mgr);
>> -     if (ret) {
>> +     mgr = fpga_mgr_register_simple(dev, "SoCFPGA Arria10 FPGA Manager",
>> +                                    &socfpga_a10_fpga_mgr_ops, priv);
>> +     if (IS_ERR(mgr)) {
>>               clk_disable_unprepare(priv->clk);
>> -             return ret;
>> +             return PTR_ERR(mgr);
>>       }
>>
>> +     platform_set_drvdata(pdev, mgr);
>> +
>>       return 0;
>>  }
>>
>> diff --git a/drivers/fpga/socfpga.c b/drivers/fpga/socfpga.c
>> index 1f467173fc1f..9280db4364d4 100644
>> --- a/drivers/fpga/socfpga.c
>> +++ b/drivers/fpga/socfpga.c
>> @@ -571,12 +571,9 @@ static int socfpga_fpga_probe(struct platform_device
>> *pdev)
>>       if (ret)
>>               return ret;
>>
>> -     mgr = devm_fpga_mgr_create(dev, "Altera SOCFPGA FPGA Manager",
>> -                                &socfpga_fpga_ops, priv);
>> -     if (!mgr)
>> -             return -ENOMEM;
>> -
>> -     return devm_fpga_mgr_register(dev, mgr);
>> +     mgr = devm_fpga_mgr_register_simple(dev, "Altera SOCFPGA FPGA
>> Manager",
>> +                                         &socfpga_fpga_ops, priv);
>> +     return PTR_ERR_OR_ZERO(mgr);
>>  }
>>
>>  #ifdef CONFIG_OF
>> diff --git a/drivers/fpga/stratix10-soc.c b/drivers/fpga/stratix10-soc.c
>> index a2cea500f7cc..fb84d88d4ce9 100644
>> --- a/drivers/fpga/stratix10-soc.c
>> +++ b/drivers/fpga/stratix10-soc.c
>> @@ -425,18 +425,11 @@ static int s10_probe(struct platform_device *pdev)
>>
>>       init_completion(&priv->status_return_completion);
>>
>> -     mgr = fpga_mgr_create(dev, "Stratix10 SOC FPGA Manager",
>> -                           &s10_ops, priv);
>> -     if (!mgr) {
>> -             dev_err(dev, "unable to create FPGA manager\n");
>> -             ret = -ENOMEM;
>> -             goto probe_err;
>> -     }
>> -
>> -     ret = fpga_mgr_register(mgr);
>> -     if (ret) {
>> +     mgr = fpga_mgr_register_simple(dev, "Stratix10 SOC FPGA Manager",
>> +                                    &s10_ops, priv);
>> +     if (IS_ERR(mgr)) {
>>               dev_err(dev, "unable to register FPGA manager\n");
>> -             fpga_mgr_free(mgr);
>> +             ret = PTR_ERR(mgr);
>>               goto probe_err;
>>       }
>>
>> @@ -454,7 +447,6 @@ 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;
>> diff --git a/drivers/fpga/ts73xx-fpga.c b/drivers/fpga/ts73xx-fpga.c
>> index 101f016c6ed8..009d5cfa1074 100644
>> --- a/drivers/fpga/ts73xx-fpga.c
>> +++ b/drivers/fpga/ts73xx-fpga.c
>> @@ -122,12 +122,9 @@ static int ts73xx_fpga_probe(struct platform_device
>> *pdev)
>>       if (IS_ERR(priv->io_base))
>>               return PTR_ERR(priv->io_base);
>>
>> -     mgr = devm_fpga_mgr_create(kdev, "TS-73xx FPGA Manager",
>> -                                &ts73xx_fpga_ops, priv);
>> -     if (!mgr)
>> -             return -ENOMEM;
>> -
>> -     return devm_fpga_mgr_register(kdev, mgr);
>> +     mgr = devm_fpga_mgr_register_simple(kdev, "TS-73xx FPGA Manager",
>> +                                         &ts73xx_fpga_ops, priv);
>> +     return PTR_ERR_OR_ZERO(mgr);
>>  }
>>
>>  static struct platform_driver ts73xx_fpga_driver = {
>> diff --git a/drivers/fpga/xilinx-spi.c b/drivers/fpga/xilinx-spi.c
>> index fee4d0abf6bf..6f568a757cf2 100644
>> --- a/drivers/fpga/xilinx-spi.c
>> +++ b/drivers/fpga/xilinx-spi.c
>> @@ -247,13 +247,10 @@ static int xilinx_spi_probe(struct spi_device *spi)
>>               return dev_err_probe(&spi->dev, PTR_ERR(conf->done),
>>                                    "Failed to get DONE gpio\n");
>>
>> -     mgr = devm_fpga_mgr_create(&spi->dev,
>> -                                "Xilinx Slave Serial FPGA Manager",
>> -                                &xilinx_spi_ops, conf);
>> -     if (!mgr)
>> -             return -ENOMEM;
>> -
>> -     return devm_fpga_mgr_register(&spi->dev, mgr);
>> +     mgr = devm_fpga_mgr_register_simple(&spi->dev,
>> +                                         "Xilinx Slave Serial FPGA Manager",
>> +                                         &xilinx_spi_ops, conf);
>> +     return PTR_ERR_OR_ZERO(mgr);
>>  }
>>
>>  static const struct of_device_id xlnx_spi_of_match[] = {
>> diff --git a/drivers/fpga/zynq-fpga.c b/drivers/fpga/zynq-fpga.c
>> index 9b75bd4f93d8..a3de365aadc6 100644
>> --- a/drivers/fpga/zynq-fpga.c
>> +++ b/drivers/fpga/zynq-fpga.c
>> @@ -609,20 +609,16 @@ static int zynq_fpga_probe(struct platform_device
>> *pdev)
>>
>>       clk_disable(priv->clk);
>>
>> -     mgr = devm_fpga_mgr_create(dev, "Xilinx Zynq FPGA Manager",
>> -                                &zynq_fpga_ops, priv);
>> -     if (!mgr)
>> -             return -ENOMEM;
>> -
>> -     platform_set_drvdata(pdev, mgr);
>> -
>> -     err = fpga_mgr_register(mgr);
>> -     if (err) {
>> +     mgr = fpga_mgr_register_simple(dev, "Xilinx Zynq FPGA Manager",
>> +                                    &zynq_fpga_ops, priv);
>> +     if (IS_ERR(mgr)) {
>>               dev_err(dev, "unable to register FPGA manager\n");
>>               clk_unprepare(priv->clk);
>> -             return err;
>> +             return PTR_ERR(mgr);
>>       }
>>
>> +     platform_set_drvdata(pdev, mgr);
>> +
>>       return 0;
>>  }
>>
>> diff --git a/drivers/fpga/zynqmp-fpga.c b/drivers/fpga/zynqmp-fpga.c
>> index 125743c9797f..53a7b36f61e8 100644
>> --- a/drivers/fpga/zynqmp-fpga.c
>> +++ b/drivers/fpga/zynqmp-fpga.c
>> @@ -102,12 +102,9 @@ static int zynqmp_fpga_probe(struct platform_device
>> *pdev)
>>
>>       priv->dev = dev;
>>
>> -     mgr = devm_fpga_mgr_create(dev, "Xilinx ZynqMP FPGA Manager",
>> -                                &zynqmp_fpga_ops, priv);
>> -     if (!mgr)
>> -             return -ENOMEM;
>> -
>> -     return devm_fpga_mgr_register(dev, mgr);
>> +     mgr = devm_fpga_mgr_register_simple(dev, "Xilinx ZynqMP FPGA
>> Manager",
>> +                                         &zynqmp_fpga_ops, priv);
>> +     return PTR_ERR_OR_ZERO(mgr);
>>  }
>>
>>  static const struct of_device_id zynqmp_fpga_of_match[] = {
>> diff --git a/include/linux/fpga/fpga-mgr.h b/include/linux/fpga/fpga-mgr.h
>> index 474c1f506307..9efe55be93bc 100644
>> --- a/include/linux/fpga/fpga-mgr.h
>> +++ b/include/linux/fpga/fpga-mgr.h
>> @@ -105,6 +105,36 @@ struct fpga_image_info {
>>  #endif
>>  };
>>
>> +/**
>> + * struct fpga_compat_id - id for compatibility check
>> + *
>> + * @id_h: high 64bit of the compat_id
>> + * @id_l: low 64bit of the compat_id
>> + */
>> +struct fpga_compat_id {
>> +     u64 id_h;
>> +     u64 id_l;
>> +};
>> +
>> +/**
>> + * struct fpga_manager_info - collection of parameters for an FPGA Manager
>> + * @name: fpga manager name
>> + * @compat_id: FPGA manager id for compatibility check.
>> + * @mops: pointer to structure of fpga manager ops
>> + * @priv: fpga manager private data
>> + *
>> + * fpga_manager_info contains parameters for the register function. These
>> + * are separated into an info structure because they some are optional
>> + * others could be added to in the future. The info structure facilitates
>> + * maintaining a stable API.
>> + */
>> +struct fpga_manager_info {
>> +     const char *name;
>> +     struct fpga_compat_id *compat_id;
>> +     const struct fpga_manager_ops *mops;
>> +     void *priv;
>> +};
>> +
>>  /**
>>   * struct fpga_manager_ops - ops for low level fpga manager drivers
>>   * @initial_header_size: Maximum number of bytes that should be passed into
>> write_init
>> @@ -143,17 +173,6 @@ struct fpga_manager_ops {
>>  #define FPGA_MGR_STATUS_IP_PROTOCOL_ERR              BIT(3)
>>  #define FPGA_MGR_STATUS_FIFO_OVERFLOW_ERR    BIT(4)
>>
>> -/**
>> - * struct fpga_compat_id - id for compatibility check
>> - *
>> - * @id_h: high 64bit of the compat_id
>> - * @id_l: low 64bit of the compat_id
>> - */
>> -struct fpga_compat_id {
>> -     u64 id_h;
>> -     u64 id_l;
>> -};
>> -
>>  /**
>>   * struct fpga_manager - fpga manager structure
>>   * @name: name of low level fpga manager
>> @@ -191,17 +210,18 @@ struct fpga_manager *fpga_mgr_get(struct device
>> *dev);
>>
>>  void fpga_mgr_put(struct fpga_manager *mgr);
>>
>> -struct fpga_manager *fpga_mgr_create(struct device *dev, const char *name,
>> -                                  const struct fpga_manager_ops *mops,
>> -                                  void *priv);
>> -void fpga_mgr_free(struct fpga_manager *mgr);
>> -int fpga_mgr_register(struct fpga_manager *mgr);
>> -void fpga_mgr_unregister(struct fpga_manager *mgr);
>> +struct fpga_manager *
>> +fpga_mgr_register(struct device *parent, const struct fpga_manager_info
>> *info);
>>
>> -int devm_fpga_mgr_register(struct device *dev, struct fpga_manager *mgr);
>> +struct fpga_manager *
>> +fpga_mgr_register_simple(struct device *parent, const char *name,
>> +                      const struct fpga_manager_ops *mops, void *priv);
>> +void fpga_mgr_unregister(struct fpga_manager *mgr);
>>
>> -struct fpga_manager *devm_fpga_mgr_create(struct device *dev, const char
>> *name,
>> -                                       const struct fpga_manager_ops
>> *mops,
>> -                                       void *priv);
>> +struct fpga_manager *
>> +devm_fpga_mgr_register(struct device *parent, const struct
>> fpga_manager_info *info);
>> +struct fpga_manager *
>> +devm_fpga_mgr_register_simple(struct device *parent, const char *name,
>> +                           const struct fpga_manager_ops *mops, void *priv);
>>
>>  #endif /*_LINUX_FPGA_MGR_H */
>> --
>> 2.25.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2021-06-30 22:12 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-24 20:53 [PATCH v8 0/3] fpga: Use standard class dev_release function Russ Weight
2021-06-24 20:53 ` [PATCH v8 1/3] fpga: mgr: Use standard dev_release for class driver Russ Weight
2021-06-28  3:49   ` Wu, Hao
2021-06-28 18:51     ` Moritz Fischer
2021-06-30 22:12     ` Russ Weight
2021-06-28  4:39   ` Xu Yilun
2021-06-24 20:53 ` [PATCH v8 2/3] fpga: bridge: " Russ Weight
2021-06-28  5:31   ` Xu Yilun
2021-06-24 20:53 ` [PATCH v8 3/3] fpga: region: " Russ Weight
2021-06-27 21:05 ` [PATCH v8 0/3] fpga: Use standard class dev_release function Russ Weight

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.