linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/4] fpga: add devm managed create APIs
@ 2018-09-26 16:12 Alan Tull
  2018-09-26 16:12 ` [PATCH v3 1/4] fpga: mgr: add devm_fpga_mgr_create Alan Tull
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Alan Tull @ 2018-09-26 16:12 UTC (permalink / raw)
  To: Moritz Fischer, Jonathan Corbet
  Cc: Randy Dunlap, Federico Vaga, linux-kernel, linux-fpga, linux-doc,
	Alan Tull

Implement managed devm_fpga_(mgr|bridge|region)_create() functions.

Patchset is dependent on the patches that were acked/sent upstream:
   https://lkml.org/lkml/2018/9/12/639

v3 changes:
* take out unclear explanation fpga_(mgr|br|region)_unregister
* Add acks
* drop the separately resent "dt-bindings: fpga: fix freeze controller compatible in region doc"
* drop some of the acked patches that I send to Greg

Alan Tull (4):
  fpga: mgr: add devm_fpga_mgr_create
  fpga: bridge: add devm_fpga_bridge_create
  fpga: add devm_fpga_region_create
  docs: fpga: document programming fpgas using regions

 Documentation/driver-api/fpga/fpga-bridge.rst      |  37 ++----
 Documentation/driver-api/fpga/fpga-mgr.rst         | 126 +++------------------
 Documentation/driver-api/fpga/fpga-programming.rst | 107 +++++++++++++++++
 Documentation/driver-api/fpga/fpga-region.rst      |  91 +++++++--------
 Documentation/driver-api/fpga/index.rst            |   2 +
 Documentation/driver-api/fpga/intro.rst            |   2 +-
 drivers/fpga/altera-cvp.c                          |   8 +-
 drivers/fpga/altera-fpga2sdram.c                   |   8 +-
 drivers/fpga/altera-freeze-bridge.c                |  13 +--
 drivers/fpga/altera-hps2fpga.c                     |   7 +-
 drivers/fpga/altera-pr-ip-core.c                   |   9 +-
 drivers/fpga/altera-ps-spi.c                       |  11 +-
 drivers/fpga/dfl-fme-br.c                          |  11 +-
 drivers/fpga/dfl-fme-mgr.c                         |  11 +-
 drivers/fpga/dfl-fme-region.c                      |   6 +-
 drivers/fpga/dfl.c                                 |   6 +-
 drivers/fpga/fpga-bridge.c                         |  68 +++++++++--
 drivers/fpga/fpga-mgr.c                            |  64 +++++++++--
 drivers/fpga/fpga-region.c                         |  65 +++++++++--
 drivers/fpga/ice40-spi.c                           |  10 +-
 drivers/fpga/machxo2-spi.c                         |  11 +-
 drivers/fpga/of-fpga-region.c                      |   6 +-
 drivers/fpga/socfpga-a10.c                         |   5 +-
 drivers/fpga/socfpga.c                             |  10 +-
 drivers/fpga/ts73xx-fpga.c                         |  11 +-
 drivers/fpga/xilinx-pr-decoupler.c                 |   4 +-
 drivers/fpga/xilinx-spi.c                          |  12 +-
 drivers/fpga/zynq-fpga.c                           |   5 +-
 include/linux/fpga/fpga-bridge.h                   |   4 +
 include/linux/fpga/fpga-mgr.h                      |   4 +
 include/linux/fpga/fpga-region.h                   |   4 +
 31 files changed, 412 insertions(+), 326 deletions(-)
 create mode 100644 Documentation/driver-api/fpga/fpga-programming.rst

-- 
2.7.4


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

* [PATCH v3 1/4] fpga: mgr: add devm_fpga_mgr_create
  2018-09-26 16:12 [PATCH v3 0/4] fpga: add devm managed create APIs Alan Tull
@ 2018-09-26 16:12 ` Alan Tull
  2018-10-15 14:54   ` Alan Tull
  2018-09-26 16:12 ` [PATCH v3 2/4] fpga: bridge: add devm_fpga_bridge_create Alan Tull
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 11+ messages in thread
From: Alan Tull @ 2018-09-26 16:12 UTC (permalink / raw)
  To: Moritz Fischer, Jonathan Corbet
  Cc: Randy Dunlap, Federico Vaga, linux-kernel, linux-fpga, linux-doc,
	Alan Tull

Add devm_fpga_mgr_create() which is the managed
version of fpga_mgr_create().

Change current FPGA manager drivers to use
devm_fpga_mgr_create()

Signed-off-by: Alan Tull <atull@kernel.org>
Suggested-by: Federico Vaga <federico.vaga@cern.ch>
---
v2: add suggested-by
v3: remove some unclear documentation about fpga_mgr_unregister
---
 Documentation/driver-api/fpga/fpga-mgr.rst | 13 +++---
 drivers/fpga/altera-cvp.c                  |  8 ++--
 drivers/fpga/altera-pr-ip-core.c           |  9 +----
 drivers/fpga/altera-ps-spi.c               | 11 ++---
 drivers/fpga/dfl-fme-mgr.c                 | 11 ++---
 drivers/fpga/fpga-mgr.c                    | 64 ++++++++++++++++++++++++++----
 drivers/fpga/ice40-spi.c                   | 10 ++---
 drivers/fpga/machxo2-spi.c                 | 11 ++---
 drivers/fpga/socfpga-a10.c                 |  5 +--
 drivers/fpga/socfpga.c                     | 10 ++---
 drivers/fpga/ts73xx-fpga.c                 | 11 ++---
 drivers/fpga/xilinx-spi.c                  | 12 ++----
 drivers/fpga/zynq-fpga.c                   |  5 +--
 include/linux/fpga/fpga-mgr.h              |  4 ++
 14 files changed, 97 insertions(+), 87 deletions(-)

diff --git a/Documentation/driver-api/fpga/fpga-mgr.rst b/Documentation/driver-api/fpga/fpga-mgr.rst
index 82b6dbb..db8885e 100644
--- a/Documentation/driver-api/fpga/fpga-mgr.rst
+++ b/Documentation/driver-api/fpga/fpga-mgr.rst
@@ -49,18 +49,14 @@ probe function calls fpga_mgr_register(), such as::
 		 * them in priv
 		 */
 
-		mgr = fpga_mgr_create(dev, "Altera SOCFPGA FPGA Manager",
-				      &socfpga_fpga_ops, priv);
+		mgr = devm_fpga_mgr_create(dev, "Altera SOCFPGA FPGA Manager",
+					   &socfpga_fpga_ops, priv);
 		if (!mgr)
 			return -ENOMEM;
 
 		platform_set_drvdata(pdev, mgr);
 
-		ret = fpga_mgr_register(mgr);
-		if (ret)
-			fpga_mgr_free(mgr);
-
-		return ret;
+		return fpga_mgr_register(mgr);
 	}
 
 	static int socfpga_fpga_remove(struct platform_device *pdev)
@@ -170,6 +166,9 @@ API for implementing a new FPGA Manager driver
    :functions: fpga_manager_ops
 
 .. kernel-doc:: drivers/fpga/fpga-mgr.c
+   :functions: devm_fpga_mgr_create
+
+.. kernel-doc:: drivers/fpga/fpga-mgr.c
    :functions: fpga_mgr_create
 
 .. kernel-doc:: drivers/fpga/fpga-mgr.c
diff --git a/drivers/fpga/altera-cvp.c b/drivers/fpga/altera-cvp.c
index 7fa7936..610a155 100644
--- a/drivers/fpga/altera-cvp.c
+++ b/drivers/fpga/altera-cvp.c
@@ -453,8 +453,8 @@ 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 = fpga_mgr_create(&pdev->dev, conf->mgr_name,
-			      &altera_cvp_ops, conf);
+	mgr = devm_fpga_mgr_create(&pdev->dev, conf->mgr_name,
+				   &altera_cvp_ops, conf);
 	if (!mgr) {
 		ret = -ENOMEM;
 		goto err_unmap;
@@ -463,10 +463,8 @@ static int altera_cvp_probe(struct pci_dev *pdev,
 	pci_set_drvdata(pdev, mgr);
 
 	ret = fpga_mgr_register(mgr);
-	if (ret) {
-		fpga_mgr_free(mgr);
+	if (ret)
 		goto err_unmap;
-	}
 
 	ret = driver_create_file(&altera_cvp_driver.driver,
 				 &driver_attr_chkcfg);
diff --git a/drivers/fpga/altera-pr-ip-core.c b/drivers/fpga/altera-pr-ip-core.c
index 65e0b6a..a7a3bf0 100644
--- a/drivers/fpga/altera-pr-ip-core.c
+++ b/drivers/fpga/altera-pr-ip-core.c
@@ -177,7 +177,6 @@ int alt_pr_register(struct device *dev, void __iomem *reg_base)
 {
 	struct alt_pr_priv *priv;
 	struct fpga_manager *mgr;
-	int ret;
 	u32 val;
 
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
@@ -192,17 +191,13 @@ 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 = fpga_mgr_create(dev, dev_name(dev), &alt_pr_ops, priv);
+	mgr = devm_fpga_mgr_create(dev, dev_name(dev), &alt_pr_ops, priv);
 	if (!mgr)
 		return -ENOMEM;
 
 	dev_set_drvdata(dev, mgr);
 
-	ret = fpga_mgr_register(mgr);
-	if (ret)
-		fpga_mgr_free(mgr);
-
-	return ret;
+	return fpga_mgr_register(mgr);
 }
 EXPORT_SYMBOL_GPL(alt_pr_register);
 
diff --git a/drivers/fpga/altera-ps-spi.c b/drivers/fpga/altera-ps-spi.c
index 24b25c6..33aafda 100644
--- a/drivers/fpga/altera-ps-spi.c
+++ b/drivers/fpga/altera-ps-spi.c
@@ -239,7 +239,6 @@ static int altera_ps_probe(struct spi_device *spi)
 	struct altera_ps_conf *conf;
 	const struct of_device_id *of_id;
 	struct fpga_manager *mgr;
-	int ret;
 
 	conf = devm_kzalloc(&spi->dev, sizeof(*conf), GFP_KERNEL);
 	if (!conf)
@@ -275,18 +274,14 @@ 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 = fpga_mgr_create(&spi->dev, conf->mgr_name,
-			      &altera_ps_ops, conf);
+	mgr = devm_fpga_mgr_create(&spi->dev, conf->mgr_name,
+				   &altera_ps_ops, conf);
 	if (!mgr)
 		return -ENOMEM;
 
 	spi_set_drvdata(spi, mgr);
 
-	ret = fpga_mgr_register(mgr);
-	if (ret)
-		fpga_mgr_free(mgr);
-
-	return ret;
+	return fpga_mgr_register(mgr);
 }
 
 static int altera_ps_remove(struct spi_device *spi)
diff --git a/drivers/fpga/dfl-fme-mgr.c b/drivers/fpga/dfl-fme-mgr.c
index 9f045d0..76f3770 100644
--- a/drivers/fpga/dfl-fme-mgr.c
+++ b/drivers/fpga/dfl-fme-mgr.c
@@ -287,7 +287,6 @@ static int fme_mgr_probe(struct platform_device *pdev)
 	struct fme_mgr_priv *priv;
 	struct fpga_manager *mgr;
 	struct resource *res;
-	int ret;
 
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
@@ -309,19 +308,15 @@ static int fme_mgr_probe(struct platform_device *pdev)
 
 	fme_mgr_get_compat_id(priv->ioaddr, compat_id);
 
-	mgr = fpga_mgr_create(dev, "DFL FME FPGA Manager",
-			      &fme_mgr_ops, priv);
+	mgr = devm_fpga_mgr_create(dev, "DFL FME FPGA Manager",
+				   &fme_mgr_ops, priv);
 	if (!mgr)
 		return -ENOMEM;
 
 	mgr->compat_id = compat_id;
 	platform_set_drvdata(pdev, mgr);
 
-	ret = fpga_mgr_register(mgr);
-	if (ret)
-		fpga_mgr_free(mgr);
-
-	return ret;
+	return fpga_mgr_register(mgr);
 }
 
 static int fme_mgr_remove(struct platform_device *pdev)
diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c
index a41b07e..c386681 100644
--- a/drivers/fpga/fpga-mgr.c
+++ b/drivers/fpga/fpga-mgr.c
@@ -558,6 +558,9 @@ EXPORT_SYMBOL_GPL(fpga_mgr_unlock);
  * @mops:	pointer to structure of fpga manager ops
  * @priv:	fpga manager private data
  *
+ * The caller of this function is responsible for freeing the struct with
+ * fpga_mgr_free().  Using devm_fpga_mgr_create() instead is recommended.
+ *
  * Return: pointer to struct fpga_manager or NULL
  */
 struct fpga_manager *fpga_mgr_create(struct device *dev, const char *name,
@@ -618,8 +621,8 @@ struct fpga_manager *fpga_mgr_create(struct device *dev, const char *name,
 EXPORT_SYMBOL_GPL(fpga_mgr_create);
 
 /**
- * fpga_mgr_free - deallocate a FPGA manager
- * @mgr:	fpga manager struct created by fpga_mgr_create
+ * fpga_mgr_free - free a FPGA manager created with fpga_mgr_create()
+ * @mgr:	fpga manager struct
  */
 void fpga_mgr_free(struct fpga_manager *mgr)
 {
@@ -628,9 +631,55 @@ void fpga_mgr_free(struct fpga_manager *mgr)
 }
 EXPORT_SYMBOL_GPL(fpga_mgr_free);
 
+static void devm_fpga_mgr_release(struct device *dev, void *res)
+{
+	struct fpga_manager *mgr = *(struct fpga_manager **)res;
+
+	fpga_mgr_free(mgr);
+}
+
+/**
+ * devm_fpga_mgr_create - create and initialize a managed FPGA manager struct
+ * @dev:	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 a 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.
+ *
+ * Return: pointer to struct fpga_manager or NULL
+ */
+struct fpga_manager *devm_fpga_mgr_create(struct device *dev, const char *name,
+					  const struct fpga_manager_ops *mops,
+					  void *priv)
+{
+	struct fpga_manager **ptr, *mgr;
+
+	ptr = devres_alloc(devm_fpga_mgr_release, sizeof(*ptr), GFP_KERNEL);
+	if (!ptr)
+		return NULL;
+
+	mgr = fpga_mgr_create(dev, name, mops, priv);
+	if (!mgr) {
+		devres_free(ptr);
+	} else {
+		*ptr = mgr;
+		devres_add(dev, ptr);
+	}
+
+	return mgr;
+}
+EXPORT_SYMBOL_GPL(devm_fpga_mgr_create);
+
 /**
  * fpga_mgr_register - register a FPGA manager
- * @mgr:	fpga manager struct created by fpga_mgr_create
+ * @mgr: fpga manager struct
  *
  * Return: 0 on success, negative error code otherwise.
  */
@@ -661,8 +710,10 @@ int fpga_mgr_register(struct fpga_manager *mgr)
 EXPORT_SYMBOL_GPL(fpga_mgr_register);
 
 /**
- * fpga_mgr_unregister - unregister and free a FPGA manager
- * @mgr:	fpga manager struct
+ * fpga_mgr_unregister - unregister a FPGA manager
+ * @mgr: fpga manager struct
+ *
+ * This function is intended for use in a FPGA manager driver's remove function.
  */
 void fpga_mgr_unregister(struct fpga_manager *mgr)
 {
@@ -681,9 +732,6 @@ EXPORT_SYMBOL_GPL(fpga_mgr_unregister);
 
 static void fpga_mgr_dev_release(struct device *dev)
 {
-	struct fpga_manager *mgr = to_fpga_manager(dev);
-
-	fpga_mgr_free(mgr);
 }
 
 static int __init fpga_mgr_class_init(void)
diff --git a/drivers/fpga/ice40-spi.c b/drivers/fpga/ice40-spi.c
index 5981c7e..6154661 100644
--- a/drivers/fpga/ice40-spi.c
+++ b/drivers/fpga/ice40-spi.c
@@ -175,18 +175,14 @@ static int ice40_fpga_probe(struct spi_device *spi)
 		return ret;
 	}
 
-	mgr = fpga_mgr_create(dev, "Lattice iCE40 FPGA Manager",
-			      &ice40_fpga_ops, priv);
+	mgr = devm_fpga_mgr_create(dev, "Lattice iCE40 FPGA Manager",
+				   &ice40_fpga_ops, priv);
 	if (!mgr)
 		return -ENOMEM;
 
 	spi_set_drvdata(spi, mgr);
 
-	ret = fpga_mgr_register(mgr);
-	if (ret)
-		fpga_mgr_free(mgr);
-
-	return ret;
+	return fpga_mgr_register(mgr);
 }
 
 static int ice40_fpga_remove(struct spi_device *spi)
diff --git a/drivers/fpga/machxo2-spi.c b/drivers/fpga/machxo2-spi.c
index a582e00..4d8a876 100644
--- a/drivers/fpga/machxo2-spi.c
+++ b/drivers/fpga/machxo2-spi.c
@@ -356,25 +356,20 @@ static int machxo2_spi_probe(struct spi_device *spi)
 {
 	struct device *dev = &spi->dev;
 	struct fpga_manager *mgr;
-	int ret;
 
 	if (spi->max_speed_hz > MACHXO2_MAX_SPEED) {
 		dev_err(dev, "Speed is too high\n");
 		return -EINVAL;
 	}
 
-	mgr = fpga_mgr_create(dev, "Lattice MachXO2 SPI FPGA Manager",
-			      &machxo2_ops, spi);
+	mgr = devm_fpga_mgr_create(dev, "Lattice MachXO2 SPI FPGA Manager",
+				   &machxo2_ops, spi);
 	if (!mgr)
 		return -ENOMEM;
 
 	spi_set_drvdata(spi, mgr);
 
-	ret = fpga_mgr_register(mgr);
-	if (ret)
-		fpga_mgr_free(mgr);
-
-	return ret;
+	return fpga_mgr_register(mgr);
 }
 
 static int machxo2_spi_remove(struct spi_device *spi)
diff --git a/drivers/fpga/socfpga-a10.c b/drivers/fpga/socfpga-a10.c
index be30c48..573d88b 100644
--- a/drivers/fpga/socfpga-a10.c
+++ b/drivers/fpga/socfpga-a10.c
@@ -508,8 +508,8 @@ static int socfpga_a10_fpga_probe(struct platform_device *pdev)
 		return -EBUSY;
 	}
 
-	mgr = fpga_mgr_create(dev, "SoCFPGA Arria10 FPGA Manager",
-			      &socfpga_a10_fpga_mgr_ops, priv);
+	mgr = devm_fpga_mgr_create(dev, "SoCFPGA Arria10 FPGA Manager",
+				   &socfpga_a10_fpga_mgr_ops, priv);
 	if (!mgr)
 		return -ENOMEM;
 
@@ -517,7 +517,6 @@ static int socfpga_a10_fpga_probe(struct platform_device *pdev)
 
 	ret = fpga_mgr_register(mgr);
 	if (ret) {
-		fpga_mgr_free(mgr);
 		clk_disable_unprepare(priv->clk);
 		return ret;
 	}
diff --git a/drivers/fpga/socfpga.c b/drivers/fpga/socfpga.c
index 959d71f..4a8a2fc 100644
--- a/drivers/fpga/socfpga.c
+++ b/drivers/fpga/socfpga.c
@@ -571,18 +571,14 @@ static int socfpga_fpga_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	mgr = fpga_mgr_create(dev, "Altera SOCFPGA FPGA Manager",
-			      &socfpga_fpga_ops, priv);
+	mgr = devm_fpga_mgr_create(dev, "Altera SOCFPGA FPGA Manager",
+				   &socfpga_fpga_ops, priv);
 	if (!mgr)
 		return -ENOMEM;
 
 	platform_set_drvdata(pdev, mgr);
 
-	ret = fpga_mgr_register(mgr);
-	if (ret)
-		fpga_mgr_free(mgr);
-
-	return ret;
+	return fpga_mgr_register(mgr);
 }
 
 static int socfpga_fpga_remove(struct platform_device *pdev)
diff --git a/drivers/fpga/ts73xx-fpga.c b/drivers/fpga/ts73xx-fpga.c
index 08efd18..dc22a58 100644
--- a/drivers/fpga/ts73xx-fpga.c
+++ b/drivers/fpga/ts73xx-fpga.c
@@ -118,7 +118,6 @@ static int ts73xx_fpga_probe(struct platform_device *pdev)
 	struct ts73xx_fpga_priv *priv;
 	struct fpga_manager *mgr;
 	struct resource *res;
-	int ret;
 
 	priv = devm_kzalloc(kdev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
@@ -133,18 +132,14 @@ static int ts73xx_fpga_probe(struct platform_device *pdev)
 		return PTR_ERR(priv->io_base);
 	}
 
-	mgr = fpga_mgr_create(kdev, "TS-73xx FPGA Manager",
-			      &ts73xx_fpga_ops, priv);
+	mgr = devm_fpga_mgr_create(kdev, "TS-73xx FPGA Manager",
+				   &ts73xx_fpga_ops, priv);
 	if (!mgr)
 		return -ENOMEM;
 
 	platform_set_drvdata(pdev, mgr);
 
-	ret = fpga_mgr_register(mgr);
-	if (ret)
-		fpga_mgr_free(mgr);
-
-	return ret;
+	return fpga_mgr_register(mgr);
 }
 
 static int ts73xx_fpga_remove(struct platform_device *pdev)
diff --git a/drivers/fpga/xilinx-spi.c b/drivers/fpga/xilinx-spi.c
index 8d19459..469486b 100644
--- a/drivers/fpga/xilinx-spi.c
+++ b/drivers/fpga/xilinx-spi.c
@@ -144,7 +144,6 @@ static int xilinx_spi_probe(struct spi_device *spi)
 {
 	struct xilinx_spi_conf *conf;
 	struct fpga_manager *mgr;
-	int ret;
 
 	conf = devm_kzalloc(&spi->dev, sizeof(*conf), GFP_KERNEL);
 	if (!conf)
@@ -167,18 +166,15 @@ static int xilinx_spi_probe(struct spi_device *spi)
 		return PTR_ERR(conf->done);
 	}
 
-	mgr = fpga_mgr_create(&spi->dev, "Xilinx Slave Serial FPGA Manager",
-			      &xilinx_spi_ops, conf);
+	mgr = devm_fpga_mgr_create(&spi->dev,
+				   "Xilinx Slave Serial FPGA Manager",
+				   &xilinx_spi_ops, conf);
 	if (!mgr)
 		return -ENOMEM;
 
 	spi_set_drvdata(spi, mgr);
 
-	ret = fpga_mgr_register(mgr);
-	if (ret)
-		fpga_mgr_free(mgr);
-
-	return ret;
+	return fpga_mgr_register(mgr);
 }
 
 static int xilinx_spi_remove(struct spi_device *spi)
diff --git a/drivers/fpga/zynq-fpga.c b/drivers/fpga/zynq-fpga.c
index 3110e00..bb82efe 100644
--- a/drivers/fpga/zynq-fpga.c
+++ b/drivers/fpga/zynq-fpga.c
@@ -614,8 +614,8 @@ static int zynq_fpga_probe(struct platform_device *pdev)
 
 	clk_disable(priv->clk);
 
-	mgr = fpga_mgr_create(dev, "Xilinx Zynq FPGA Manager",
-			      &zynq_fpga_ops, priv);
+	mgr = devm_fpga_mgr_create(dev, "Xilinx Zynq FPGA Manager",
+				   &zynq_fpga_ops, priv);
 	if (!mgr)
 		return -ENOMEM;
 
@@ -624,7 +624,6 @@ static int zynq_fpga_probe(struct platform_device *pdev)
 	err = fpga_mgr_register(mgr);
 	if (err) {
 		dev_err(dev, "unable to register FPGA manager\n");
-		fpga_mgr_free(mgr);
 		clk_unprepare(priv->clk);
 		return err;
 	}
diff --git a/include/linux/fpga/fpga-mgr.h b/include/linux/fpga/fpga-mgr.h
index 8ab5df7..e8ca62b 100644
--- a/include/linux/fpga/fpga-mgr.h
+++ b/include/linux/fpga/fpga-mgr.h
@@ -198,4 +198,8 @@ 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 *devm_fpga_mgr_create(struct device *dev, const char *name,
+					  const struct fpga_manager_ops *mops,
+					  void *priv);
+
 #endif /*_LINUX_FPGA_MGR_H */
-- 
2.7.4


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

* [PATCH v3 2/4] fpga: bridge: add devm_fpga_bridge_create
  2018-09-26 16:12 [PATCH v3 0/4] fpga: add devm managed create APIs Alan Tull
  2018-09-26 16:12 ` [PATCH v3 1/4] fpga: mgr: add devm_fpga_mgr_create Alan Tull
@ 2018-09-26 16:12 ` Alan Tull
  2018-09-26 16:12 ` [PATCH v3 3/4] fpga: add devm_fpga_region_create Alan Tull
  2018-09-26 16:12 ` [PATCH v3 4/4] docs: fpga: document programming fpgas using regions Alan Tull
  3 siblings, 0 replies; 11+ messages in thread
From: Alan Tull @ 2018-09-26 16:12 UTC (permalink / raw)
  To: Moritz Fischer, Jonathan Corbet
  Cc: Randy Dunlap, Federico Vaga, linux-kernel, linux-fpga, linux-doc,
	Alan Tull

Add devm_fpga_bridge_create() which is the managed
version of fpga_bridge_create().

Change current bridge drivers to use
devm_fpga_bridge_create().

Signed-off-by: Alan Tull <atull@kernel.org>
Suggested-by: Federico Vaga <federico.vaga@cern.ch>
Acked-by: Moritz Fischer <mdf@kernel.org>
---
v2: add suggested-by
v3: remove some unclear documentation about fpga_bridge_unregister
    add Acked-by
---
 Documentation/driver-api/fpga/fpga-bridge.rst |  3 ++
 drivers/fpga/altera-fpga2sdram.c              |  8 ++--
 drivers/fpga/altera-freeze-bridge.c           | 13 ++---
 drivers/fpga/altera-hps2fpga.c                |  7 ++-
 drivers/fpga/dfl-fme-br.c                     | 11 ++---
 drivers/fpga/fpga-bridge.c                    | 68 +++++++++++++++++++++++----
 drivers/fpga/xilinx-pr-decoupler.c            |  4 +-
 include/linux/fpga/fpga-bridge.h              |  4 ++
 8 files changed, 80 insertions(+), 38 deletions(-)

diff --git a/Documentation/driver-api/fpga/fpga-bridge.rst b/Documentation/driver-api/fpga/fpga-bridge.rst
index 2c2aaca..ebbcbde 100644
--- a/Documentation/driver-api/fpga/fpga-bridge.rst
+++ b/Documentation/driver-api/fpga/fpga-bridge.rst
@@ -11,6 +11,9 @@ API to implement a new FPGA bridge
    :functions: fpga_bridge_ops
 
 .. kernel-doc:: drivers/fpga/fpga-bridge.c
+   :functions: devm_fpga_bridge_create
+
+.. kernel-doc:: drivers/fpga/fpga-bridge.c
    :functions: fpga_bridge_create
 
 .. kernel-doc:: drivers/fpga/fpga-bridge.c
diff --git a/drivers/fpga/altera-fpga2sdram.c b/drivers/fpga/altera-fpga2sdram.c
index 23660cc..a78e49c 100644
--- a/drivers/fpga/altera-fpga2sdram.c
+++ b/drivers/fpga/altera-fpga2sdram.c
@@ -121,18 +121,16 @@ 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 = fpga_bridge_create(dev, F2S_BRIDGE_NAME,
-				&altera_fpga2sdram_br_ops, priv);
+	br = devm_fpga_bridge_create(dev, F2S_BRIDGE_NAME,
+				     &altera_fpga2sdram_br_ops, priv);
 	if (!br)
 		return -ENOMEM;
 
 	platform_set_drvdata(pdev, br);
 
 	ret = fpga_bridge_register(br);
-	if (ret) {
-		fpga_bridge_free(br);
+	if (ret)
 		return ret;
-	}
 
 	dev_info(dev, "driver initialized with handoff %08x\n", priv->mask);
 
diff --git a/drivers/fpga/altera-freeze-bridge.c b/drivers/fpga/altera-freeze-bridge.c
index ffd586c..dd58c4a 100644
--- a/drivers/fpga/altera-freeze-bridge.c
+++ b/drivers/fpga/altera-freeze-bridge.c
@@ -213,7 +213,6 @@ static int altera_freeze_br_probe(struct platform_device *pdev)
 	struct fpga_bridge *br;
 	struct resource *res;
 	u32 status, revision;
-	int ret;
 
 	if (!np)
 		return -ENODEV;
@@ -245,20 +244,14 @@ static int altera_freeze_br_probe(struct platform_device *pdev)
 
 	priv->base_addr = base_addr;
 
-	br = fpga_bridge_create(dev, FREEZE_BRIDGE_NAME,
-				&altera_freeze_br_br_ops, priv);
+	br = devm_fpga_bridge_create(dev, FREEZE_BRIDGE_NAME,
+				     &altera_freeze_br_br_ops, priv);
 	if (!br)
 		return -ENOMEM;
 
 	platform_set_drvdata(pdev, br);
 
-	ret = fpga_bridge_register(br);
-	if (ret) {
-		fpga_bridge_free(br);
-		return ret;
-	}
-
-	return 0;
+	return fpga_bridge_register(br);
 }
 
 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 a974d3f..77b95f2 100644
--- a/drivers/fpga/altera-hps2fpga.c
+++ b/drivers/fpga/altera-hps2fpga.c
@@ -180,7 +180,8 @@ static int alt_fpga_bridge_probe(struct platform_device *pdev)
 		}
 	}
 
-	br = fpga_bridge_create(dev, priv->name, &altera_hps2fpga_br_ops, priv);
+	br = devm_fpga_bridge_create(dev, priv->name,
+				     &altera_hps2fpga_br_ops, priv);
 	if (!br) {
 		ret = -ENOMEM;
 		goto err;
@@ -190,12 +191,10 @@ static int alt_fpga_bridge_probe(struct platform_device *pdev)
 
 	ret = fpga_bridge_register(br);
 	if (ret)
-		goto err_free;
+		goto err;
 
 	return 0;
 
-err_free:
-	fpga_bridge_free(br);
 err:
 	clk_disable_unprepare(priv->clk);
 
diff --git a/drivers/fpga/dfl-fme-br.c b/drivers/fpga/dfl-fme-br.c
index 7cc041d..3ff9f3a 100644
--- a/drivers/fpga/dfl-fme-br.c
+++ b/drivers/fpga/dfl-fme-br.c
@@ -61,7 +61,6 @@ static int fme_br_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct fme_br_priv *priv;
 	struct fpga_bridge *br;
-	int ret;
 
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
@@ -69,18 +68,14 @@ static int fme_br_probe(struct platform_device *pdev)
 
 	priv->pdata = dev_get_platdata(dev);
 
-	br = fpga_bridge_create(dev, "DFL FPGA FME Bridge",
-				&fme_bridge_ops, priv);
+	br = devm_fpga_bridge_create(dev, "DFL FPGA FME Bridge",
+				     &fme_bridge_ops, priv);
 	if (!br)
 		return -ENOMEM;
 
 	platform_set_drvdata(pdev, br);
 
-	ret = fpga_bridge_register(br);
-	if (ret)
-		fpga_bridge_free(br);
-
-	return ret;
+	return fpga_bridge_register(br);
 }
 
 static int fme_br_remove(struct platform_device *pdev)
diff --git a/drivers/fpga/fpga-bridge.c b/drivers/fpga/fpga-bridge.c
index c983dac..80bd8f1 100644
--- a/drivers/fpga/fpga-bridge.c
+++ b/drivers/fpga/fpga-bridge.c
@@ -324,6 +324,9 @@ ATTRIBUTE_GROUPS(fpga_bridge);
  * @br_ops:	pointer to structure of fpga bridge ops
  * @priv:	FPGA bridge private data
  *
+ * 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
  */
 struct fpga_bridge *fpga_bridge_create(struct device *dev, const char *name,
@@ -378,8 +381,8 @@ struct fpga_bridge *fpga_bridge_create(struct device *dev, const char *name,
 EXPORT_SYMBOL_GPL(fpga_bridge_create);
 
 /**
- * fpga_bridge_free - free a fpga bridge and its id
- * @bridge:	FPGA bridge struct created by fpga_bridge_create
+ * fpga_bridge_free - free a fpga bridge created by fpga_bridge_create()
+ * @bridge:	FPGA bridge struct
  */
 void fpga_bridge_free(struct fpga_bridge *bridge)
 {
@@ -388,9 +391,56 @@ void fpga_bridge_free(struct fpga_bridge *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);
+}
+
 /**
- * fpga_bridge_register - register a fpga bridge
- * @bridge:	FPGA bridge struct created by fpga_bridge_create
+ * devm_fpga_bridge_create - create and init a managed struct fpga_bridge
+ * @dev:	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 a 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.
+ *
+ *  Return: struct fpga_bridge or NULL
+ */
+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 **ptr, *bridge;
+
+	ptr = devres_alloc(devm_fpga_bridge_release, sizeof(*ptr), GFP_KERNEL);
+	if (!ptr)
+		return NULL;
+
+	bridge = fpga_bridge_create(dev, name, br_ops, priv);
+	if (!bridge) {
+		devres_free(ptr);
+	} else {
+		*ptr = bridge;
+		devres_add(dev, ptr);
+	}
+
+	return bridge;
+}
+EXPORT_SYMBOL_GPL(devm_fpga_bridge_create);
+
+/**
+ * fpga_bridge_register - register a FPGA bridge
+ *
+ * @bridge: FPGA bridge struct
  *
  * Return: 0 for success, error code otherwise.
  */
@@ -412,8 +462,11 @@ int fpga_bridge_register(struct fpga_bridge *bridge)
 EXPORT_SYMBOL_GPL(fpga_bridge_register);
 
 /**
- * fpga_bridge_unregister - unregister and free a fpga bridge
- * @bridge:	FPGA bridge struct created by fpga_bridge_create
+ * fpga_bridge_unregister - unregister a FPGA bridge
+ *
+ * @bridge: FPGA bridge struct
+ *
+ * This function is intended for use in a FPGA bridge driver's remove function.
  */
 void fpga_bridge_unregister(struct fpga_bridge *bridge)
 {
@@ -430,9 +483,6 @@ EXPORT_SYMBOL_GPL(fpga_bridge_unregister);
 
 static void fpga_bridge_dev_release(struct device *dev)
 {
-	struct fpga_bridge *bridge = to_fpga_bridge(dev);
-
-	fpga_bridge_free(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 07ba153..6410361 100644
--- a/drivers/fpga/xilinx-pr-decoupler.c
+++ b/drivers/fpga/xilinx-pr-decoupler.c
@@ -121,8 +121,8 @@ static int xlnx_pr_decoupler_probe(struct platform_device *pdev)
 
 	clk_disable(priv->clk);
 
-	br = fpga_bridge_create(&pdev->dev, "Xilinx PR Decoupler",
-				&xlnx_pr_decoupler_br_ops, priv);
+	br = devm_fpga_bridge_create(&pdev->dev, "Xilinx PR Decoupler",
+				     &xlnx_pr_decoupler_br_ops, priv);
 	if (!br) {
 		err = -ENOMEM;
 		goto err_clk;
diff --git a/include/linux/fpga/fpga-bridge.h b/include/linux/fpga/fpga-bridge.h
index ce550fc..817600a 100644
--- a/include/linux/fpga/fpga-bridge.h
+++ b/include/linux/fpga/fpga-bridge.h
@@ -69,4 +69,8 @@ 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
+*devm_fpga_bridge_create(struct device *dev, const char *name,
+			 const struct fpga_bridge_ops *br_ops, void *priv);
+
 #endif /* _LINUX_FPGA_BRIDGE_H */
-- 
2.7.4


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

* [PATCH v3 3/4] fpga: add devm_fpga_region_create
  2018-09-26 16:12 [PATCH v3 0/4] fpga: add devm managed create APIs Alan Tull
  2018-09-26 16:12 ` [PATCH v3 1/4] fpga: mgr: add devm_fpga_mgr_create Alan Tull
  2018-09-26 16:12 ` [PATCH v3 2/4] fpga: bridge: add devm_fpga_bridge_create Alan Tull
@ 2018-09-26 16:12 ` Alan Tull
  2018-10-15 20:10   ` Moritz Fischer
  2018-09-26 16:12 ` [PATCH v3 4/4] docs: fpga: document programming fpgas using regions Alan Tull
  3 siblings, 1 reply; 11+ messages in thread
From: Alan Tull @ 2018-09-26 16:12 UTC (permalink / raw)
  To: Moritz Fischer, Jonathan Corbet
  Cc: Randy Dunlap, Federico Vaga, linux-kernel, linux-fpga, linux-doc,
	Alan Tull

Add devm_fpga_region_create() which is the
managed version of fpga_region_create().

Change current region drivers to use
devm_fpga_region_create().

Signed-off-by: Alan Tull <atull@kernel.org>
Suggested-by: Federico Vaga <federico.vaga@cern.ch>
---
v2: add suggested-by
v3: remove some unclear documentation about fpga_region_unregister
---
 Documentation/driver-api/fpga/fpga-region.rst |  3 ++
 drivers/fpga/dfl-fme-region.c                 |  6 +--
 drivers/fpga/dfl.c                            |  6 +--
 drivers/fpga/fpga-region.c                    | 65 +++++++++++++++++++++++----
 drivers/fpga/of-fpga-region.c                 |  6 +--
 include/linux/fpga/fpga-region.h              |  4 ++
 6 files changed, 70 insertions(+), 20 deletions(-)

diff --git a/Documentation/driver-api/fpga/fpga-region.rst b/Documentation/driver-api/fpga/fpga-region.rst
index f30333c..dc9f75c 100644
--- a/Documentation/driver-api/fpga/fpga-region.rst
+++ b/Documentation/driver-api/fpga/fpga-region.rst
@@ -90,6 +90,9 @@ API to add a new FPGA region
    :functions: fpga_region
 
 .. kernel-doc:: drivers/fpga/fpga-region.c
+   :functions: devm_fpga_region_create
+
+.. kernel-doc:: drivers/fpga/fpga-region.c
    :functions: fpga_region_create
 
 .. kernel-doc:: drivers/fpga/fpga-region.c
diff --git a/drivers/fpga/dfl-fme-region.c b/drivers/fpga/dfl-fme-region.c
index 3fa0de2..1eeb42a 100644
--- a/drivers/fpga/dfl-fme-region.c
+++ b/drivers/fpga/dfl-fme-region.c
@@ -39,7 +39,7 @@ static int fme_region_probe(struct platform_device *pdev)
 	if (IS_ERR(mgr))
 		return -EPROBE_DEFER;
 
-	region = fpga_region_create(dev, mgr, fme_region_get_bridges);
+	region = devm_fpga_region_create(dev, mgr, fme_region_get_bridges);
 	if (!region) {
 		ret = -ENOMEM;
 		goto eprobe_mgr_put;
@@ -51,14 +51,12 @@ static int fme_region_probe(struct platform_device *pdev)
 
 	ret = fpga_region_register(region);
 	if (ret)
-		goto region_free;
+		goto eprobe_mgr_put;
 
 	dev_dbg(dev, "DFL FME FPGA Region probed\n");
 
 	return 0;
 
-region_free:
-	fpga_region_free(region);
 eprobe_mgr_put:
 	fpga_mgr_put(mgr);
 	return ret;
diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
index a9b521b..2c09e50 100644
--- a/drivers/fpga/dfl.c
+++ b/drivers/fpga/dfl.c
@@ -899,7 +899,7 @@ dfl_fpga_feature_devs_enumerate(struct dfl_fpga_enum_info *info)
 	if (!cdev)
 		return ERR_PTR(-ENOMEM);
 
-	cdev->region = fpga_region_create(info->dev, NULL, NULL);
+	cdev->region = devm_fpga_region_create(info->dev, NULL, NULL);
 	if (!cdev->region) {
 		ret = -ENOMEM;
 		goto free_cdev_exit;
@@ -911,7 +911,7 @@ dfl_fpga_feature_devs_enumerate(struct dfl_fpga_enum_info *info)
 
 	ret = fpga_region_register(cdev->region);
 	if (ret)
-		goto free_region_exit;
+		goto free_cdev_exit;
 
 	/* create and init build info for enumeration */
 	binfo = devm_kzalloc(info->dev, sizeof(*binfo), GFP_KERNEL);
@@ -942,8 +942,6 @@ dfl_fpga_feature_devs_enumerate(struct dfl_fpga_enum_info *info)
 
 unregister_region_exit:
 	fpga_region_unregister(cdev->region);
-free_region_exit:
-	fpga_region_free(cdev->region);
 free_cdev_exit:
 	devm_kfree(info->dev, cdev);
 	return ERR_PTR(ret);
diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c
index 0d65220..bde5a9d 100644
--- a/drivers/fpga/fpga-region.c
+++ b/drivers/fpga/fpga-region.c
@@ -185,6 +185,10 @@ ATTRIBUTE_GROUPS(fpga_region);
  * @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.
+ *
  * Return: struct fpga_region or NULL
  */
 struct fpga_region
@@ -230,8 +234,8 @@ struct fpga_region
 EXPORT_SYMBOL_GPL(fpga_region_create);
 
 /**
- * fpga_region_free - free a struct fpga_region
- * @region: FPGA region created by fpga_region_create
+ * fpga_region_free - free a FPGA region created by fpga_region_create()
+ * @region: FPGA region
  */
 void fpga_region_free(struct fpga_region *region)
 {
@@ -240,21 +244,69 @@ void fpga_region_free(struct fpga_region *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);
+}
+
+/**
+ * devm_fpga_region_create - create and initialize a managed FPGA region struct
+ * @dev: 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 a 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.
+ *
+ * Return: struct fpga_region or NULL
+ */
+struct fpga_region
+*devm_fpga_region_create(struct device *dev,
+			 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(dev, mgr, get_bridges);
+	if (!region) {
+		devres_free(ptr);
+	} else {
+		*ptr = region;
+		devres_add(dev, ptr);
+	}
+
+	return region;
+}
+EXPORT_SYMBOL_GPL(devm_fpga_region_create);
+
 /**
  * fpga_region_register - register a FPGA region
- * @region: FPGA region created by fpga_region_create
+ * @region: FPGA region
+ *
  * Return: 0 or -errno
  */
 int fpga_region_register(struct fpga_region *region)
 {
 	return device_add(&region->dev);
-
 }
 EXPORT_SYMBOL_GPL(fpga_region_register);
 
 /**
- * fpga_region_unregister - unregister and free a FPGA region
+ * fpga_region_unregister - unregister a FPGA region
  * @region: FPGA region
+ *
+ * This function is intended for use in a FPGA region driver's remove function.
  */
 void fpga_region_unregister(struct fpga_region *region)
 {
@@ -264,9 +316,6 @@ EXPORT_SYMBOL_GPL(fpga_region_unregister);
 
 static void fpga_region_dev_release(struct device *dev)
 {
-	struct fpga_region *region = to_fpga_region(dev);
-
-	fpga_region_free(region);
 }
 
 /**
diff --git a/drivers/fpga/of-fpga-region.c b/drivers/fpga/of-fpga-region.c
index d6a70e4..75f64ab 100644
--- a/drivers/fpga/of-fpga-region.c
+++ b/drivers/fpga/of-fpga-region.c
@@ -410,7 +410,7 @@ static int of_fpga_region_probe(struct platform_device *pdev)
 	if (IS_ERR(mgr))
 		return -EPROBE_DEFER;
 
-	region = fpga_region_create(dev, mgr, of_fpga_region_get_bridges);
+	region = devm_fpga_region_create(dev, mgr, of_fpga_region_get_bridges);
 	if (!region) {
 		ret = -ENOMEM;
 		goto eprobe_mgr_put;
@@ -418,7 +418,7 @@ static int of_fpga_region_probe(struct platform_device *pdev)
 
 	ret = fpga_region_register(region);
 	if (ret)
-		goto eprobe_free;
+		goto eprobe_mgr_put;
 
 	of_platform_populate(np, fpga_region_of_match, NULL, &region->dev);
 	platform_set_drvdata(pdev, region);
@@ -427,8 +427,6 @@ static int of_fpga_region_probe(struct platform_device *pdev)
 
 	return 0;
 
-eprobe_free:
-	fpga_region_free(region);
 eprobe_mgr_put:
 	fpga_mgr_put(mgr);
 	return ret;
diff --git a/include/linux/fpga/fpga-region.h b/include/linux/fpga/fpga-region.h
index 0521b7f..27cb706 100644
--- a/include/linux/fpga/fpga-region.h
+++ b/include/linux/fpga/fpga-region.h
@@ -44,4 +44,8 @@ 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
+*devm_fpga_region_create(struct device *dev, struct fpga_manager *mgr,
+			int (*get_bridges)(struct fpga_region *));
+
 #endif /* _FPGA_REGION_H */
-- 
2.7.4


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

* [PATCH v3 4/4] docs: fpga: document programming fpgas using regions
  2018-09-26 16:12 [PATCH v3 0/4] fpga: add devm managed create APIs Alan Tull
                   ` (2 preceding siblings ...)
  2018-09-26 16:12 ` [PATCH v3 3/4] fpga: add devm_fpga_region_create Alan Tull
@ 2018-09-26 16:12 ` Alan Tull
  2018-10-15 20:11   ` Moritz Fischer
  3 siblings, 1 reply; 11+ messages in thread
From: Alan Tull @ 2018-09-26 16:12 UTC (permalink / raw)
  To: Moritz Fischer, Jonathan Corbet
  Cc: Randy Dunlap, Federico Vaga, linux-kernel, linux-fpga, linux-doc,
	Alan Tull

Clarify the intention that interfaces and upper layers use
regions rather than managers directly.

Rearrange API documentation to better group the API functions
used to create FPGA mgr/bridge/regions and the API used for
programming FPGAs.

Signed-off-by: Alan Tull <atull@kernel.org>
Suggested-by: Federico Vaga <federico.vaga@cern.ch>
---
v2: Add suggested-by
    s/a FPGA/an FPGA/
    document freeing the fpga_mgr_info on success path
    minor formatting fixes and suggested edits
v3: no changes
---
 Documentation/driver-api/fpga/fpga-bridge.rst      |  38 ++-----
 Documentation/driver-api/fpga/fpga-mgr.rst         | 117 ++-------------------
 Documentation/driver-api/fpga/fpga-programming.rst | 107 +++++++++++++++++++
 Documentation/driver-api/fpga/fpga-region.rst      |  92 ++++++++--------
 Documentation/driver-api/fpga/index.rst            |   2 +
 Documentation/driver-api/fpga/intro.rst            |   2 +-
 6 files changed, 171 insertions(+), 187 deletions(-)
 create mode 100644 Documentation/driver-api/fpga/fpga-programming.rst

diff --git a/Documentation/driver-api/fpga/fpga-bridge.rst b/Documentation/driver-api/fpga/fpga-bridge.rst
index ebbcbde..71c5a40 100644
--- a/Documentation/driver-api/fpga/fpga-bridge.rst
+++ b/Documentation/driver-api/fpga/fpga-bridge.rst
@@ -4,6 +4,12 @@ FPGA Bridge
 API to implement a new FPGA bridge
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
+* struct :c:type:`fpga_bridge` — The FPGA Bridge structure
+* struct :c:type:`fpga_bridge_ops` — Low level Bridge driver ops
+* :c:func:`devm_fpga_bridge_create()` — Allocate and init a bridge struct
+* :c:func:`fpga_bridge_register()` — Register a bridge
+* :c:func:`fpga_bridge_unregister()` — Unregister a bridge
+
 .. kernel-doc:: include/linux/fpga/fpga-bridge.h
    :functions: fpga_bridge
 
@@ -14,39 +20,7 @@ API to implement a new FPGA bridge
    :functions: devm_fpga_bridge_create
 
 .. kernel-doc:: drivers/fpga/fpga-bridge.c
-   :functions: fpga_bridge_create
-
-.. kernel-doc:: drivers/fpga/fpga-bridge.c
-   :functions: fpga_bridge_free
-
-.. kernel-doc:: drivers/fpga/fpga-bridge.c
    :functions: fpga_bridge_register
 
 .. kernel-doc:: drivers/fpga/fpga-bridge.c
    :functions: fpga_bridge_unregister
-
-API to control an FPGA bridge
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-
-You probably won't need these directly.  FPGA regions should handle this.
-
-.. kernel-doc:: drivers/fpga/fpga-bridge.c
-   :functions: of_fpga_bridge_get
-
-.. kernel-doc:: drivers/fpga/fpga-bridge.c
-   :functions: fpga_bridge_get
-
-.. kernel-doc:: drivers/fpga/fpga-bridge.c
-   :functions: fpga_bridge_put
-
-.. kernel-doc:: drivers/fpga/fpga-bridge.c
-   :functions: fpga_bridge_get_to_list
-
-.. kernel-doc:: drivers/fpga/fpga-bridge.c
-   :functions: of_fpga_bridge_get_to_list
-
-.. kernel-doc:: drivers/fpga/fpga-bridge.c
-   :functions: fpga_bridge_enable
-
-.. kernel-doc:: drivers/fpga/fpga-bridge.c
-   :functions: fpga_bridge_disable
diff --git a/Documentation/driver-api/fpga/fpga-mgr.rst b/Documentation/driver-api/fpga/fpga-mgr.rst
index db8885e..576f194 100644
--- a/Documentation/driver-api/fpga/fpga-mgr.rst
+++ b/Documentation/driver-api/fpga/fpga-mgr.rst
@@ -98,67 +98,19 @@ The ops include a .state function which will determine the state the FPGA is in
 and return a code of type enum fpga_mgr_states.  It doesn't result in a change
 in state.
 
-How to write an image buffer to a supported FPGA
-------------------------------------------------
-
-Some sample code::
-
-	#include <linux/fpga/fpga-mgr.h>
-
-	struct fpga_manager *mgr;
-	struct fpga_image_info *info;
-	int ret;
-
-	/*
-	 * Get a reference to FPGA manager.  The manager is not locked, so you can
-	 * hold onto this reference without it preventing programming.
-	 *
-	 * This example uses the device node of the manager.  Alternatively, use
-	 * fpga_mgr_get(dev) instead if you have the device.
-	 */
-	mgr = of_fpga_mgr_get(mgr_node);
-
-	/* struct with information about the FPGA image to program. */
-	info = fpga_image_info_alloc(dev);
-
-	/* flags indicates whether to do full or partial reconfiguration */
-	info->flags = FPGA_MGR_PARTIAL_RECONFIG;
-
-	/*
-	 * At this point, indicate where the image is. This is pseudo-code; you're
-	 * going to use one of these three.
-	 */
-	if (image is in a scatter gather table) {
-
-		info->sgt = [your scatter gather table]
-
-	} else if (image is in a buffer) {
-
-		info->buf = [your image buffer]
-		info->count = [image buffer size]
-
-	} else if (image is in a firmware file) {
-
-		info->firmware_name = devm_kstrdup(dev, firmware_name, GFP_KERNEL);
-
-	}
-
-	/* Get exclusive control of FPGA manager */
-	ret = fpga_mgr_lock(mgr);
-
-	/* Load the buffer to the FPGA */
-	ret = fpga_mgr_buf_load(mgr, &info, buf, count);
-
-	/* Release the FPGA manager */
-	fpga_mgr_unlock(mgr);
-	fpga_mgr_put(mgr);
-
-	/* Deallocate the image info if you're done with it */
-	fpga_image_info_free(info);
-
 API for implementing a new FPGA Manager driver
 ----------------------------------------------
 
+* ``fpga_mgr_states`` —  Values for :c:member:`fpga_manager->state`.
+* struct :c:type:`fpga_manager` —  the FPGA manager struct
+* struct :c:type:`fpga_manager_ops` —  Low level FPGA manager driver ops
+* :c:func:`devm_fpga_mgr_create` —  Allocate and init a manager struct
+* :c:func:`fpga_mgr_register` —  Register an FPGA manager
+* :c:func:`fpga_mgr_unregister` —  Unregister an FPGA manager
+
+.. kernel-doc:: include/linux/fpga/fpga-mgr.h
+   :functions: fpga_mgr_states
+
 .. kernel-doc:: include/linux/fpga/fpga-mgr.h
    :functions: fpga_manager
 
@@ -169,56 +121,7 @@ API for implementing a new FPGA Manager driver
    :functions: devm_fpga_mgr_create
 
 .. kernel-doc:: drivers/fpga/fpga-mgr.c
-   :functions: fpga_mgr_create
-
-.. kernel-doc:: drivers/fpga/fpga-mgr.c
-   :functions: fpga_mgr_free
-
-.. kernel-doc:: drivers/fpga/fpga-mgr.c
    :functions: fpga_mgr_register
 
 .. kernel-doc:: drivers/fpga/fpga-mgr.c
    :functions: fpga_mgr_unregister
-
-API for programming an FPGA
----------------------------
-
-FPGA Manager flags
-
-.. kernel-doc:: include/linux/fpga/fpga-mgr.h
-   :doc: FPGA Manager flags
-
-.. kernel-doc:: include/linux/fpga/fpga-mgr.h
-   :functions: fpga_image_info
-
-.. kernel-doc:: include/linux/fpga/fpga-mgr.h
-   :functions: fpga_mgr_states
-
-.. kernel-doc:: drivers/fpga/fpga-mgr.c
-   :functions: fpga_image_info_alloc
-
-.. kernel-doc:: drivers/fpga/fpga-mgr.c
-   :functions: fpga_image_info_free
-
-.. kernel-doc:: drivers/fpga/fpga-mgr.c
-   :functions: of_fpga_mgr_get
-
-.. kernel-doc:: drivers/fpga/fpga-mgr.c
-   :functions: fpga_mgr_get
-
-.. kernel-doc:: drivers/fpga/fpga-mgr.c
-   :functions: fpga_mgr_put
-
-.. kernel-doc:: drivers/fpga/fpga-mgr.c
-   :functions: fpga_mgr_lock
-
-.. kernel-doc:: drivers/fpga/fpga-mgr.c
-   :functions: fpga_mgr_unlock
-
-.. kernel-doc:: include/linux/fpga/fpga-mgr.h
-   :functions: fpga_mgr_states
-
-Note - use :c:func:`fpga_region_program_fpga()` instead of :c:func:`fpga_mgr_load()`
-
-.. kernel-doc:: drivers/fpga/fpga-mgr.c
-   :functions: fpga_mgr_load
diff --git a/Documentation/driver-api/fpga/fpga-programming.rst b/Documentation/driver-api/fpga/fpga-programming.rst
new file mode 100644
index 0000000..b5484df
--- /dev/null
+++ b/Documentation/driver-api/fpga/fpga-programming.rst
@@ -0,0 +1,107 @@
+In-kernel API for FPGA Programming
+==================================
+
+Overview
+--------
+
+The in-kernel API for FPGA programming is a combination of APIs from
+FPGA manager, bridge, and regions.  The actual function used to
+trigger FPGA programming is :c:func:`fpga_region_program_fpga()`.
+
+:c:func:`fpga_region_program_fpga()` uses functionality supplied by
+the FPGA manager and bridges.  It will:
+
+ * lock the region's mutex
+ * lock the mutex of the region's FPGA manager
+ * build a list of FPGA bridges if a method has been specified to do so
+ * disable the bridges
+ * program the FPGA using info passed in :c:member:`fpga_region->info`.
+ * re-enable the bridges
+ * release the locks
+
+The struct fpga_image_info specifies what FPGA image to program.  It is
+allocated/freed by :c:func:`fpga_image_info_alloc()` and freed with
+:c:func:`fpga_image_info_free()`
+
+How to program an FPGA using a region
+-------------------------------------
+
+When the FPGA region driver probed, it was given a pointer to an FPGA manager
+driver so it knows which manager to use.  The region also either has a list of
+bridges to control during programming or it has a pointer to a function that
+will generate that list.  Here's some sample code of what to do next::
+
+	#include <linux/fpga/fpga-mgr.h>
+	#include <linux/fpga/fpga-region.h>
+
+	struct fpga_image_info *info;
+	int ret;
+
+	/*
+	 * First, alloc the struct with information about the FPGA image to
+	 * program.
+	 */
+	info = fpga_image_info_alloc(dev);
+	if (!info)
+		return -ENOMEM;
+
+	/* Set flags as needed, such as: */
+	info->flags = FPGA_MGR_PARTIAL_RECONFIG;
+
+	/*
+	 * Indicate where the FPGA image is. This is pseudo-code; you're
+	 * going to use one of these three.
+	 */
+	if (image is in a scatter gather table) {
+
+		info->sgt = [your scatter gather table]
+
+	} else if (image is in a buffer) {
+
+		info->buf = [your image buffer]
+		info->count = [image buffer size]
+
+	} else if (image is in a firmware file) {
+
+		info->firmware_name = devm_kstrdup(dev, firmware_name,
+						   GFP_KERNEL);
+
+	}
+
+	/* Add info to region and do the programming */
+	region->info = info;
+	ret = fpga_region_program_fpga(region);
+
+	/* Deallocate the image info if you're done with it */
+	region->info = NULL;
+	fpga_image_info_free(info);
+
+	if (ret)
+		return ret;
+
+	/* Now enumerate whatever hardware has appeared in the FPGA. */
+
+API for programming an FPGA
+---------------------------
+
+* :c:func:`fpga_region_program_fpga` —  Program an FPGA
+* :c:type:`fpga_image_info` —  Specifies what FPGA image to program
+* :c:func:`fpga_image_info_alloc()` —  Allocate an FPGA image info struct
+* :c:func:`fpga_image_info_free()` —  Free an FPGA image info struct
+
+.. kernel-doc:: drivers/fpga/fpga-region.c
+   :functions: fpga_region_program_fpga
+
+FPGA Manager flags
+
+.. kernel-doc:: include/linux/fpga/fpga-mgr.h
+   :doc: FPGA Manager flags
+
+.. kernel-doc:: include/linux/fpga/fpga-mgr.h
+   :functions: fpga_image_info
+
+.. kernel-doc:: drivers/fpga/fpga-mgr.c
+   :functions: fpga_image_info_alloc
+
+.. kernel-doc:: drivers/fpga/fpga-mgr.c
+   :functions: fpga_image_info_free
diff --git a/Documentation/driver-api/fpga/fpga-region.rst b/Documentation/driver-api/fpga/fpga-region.rst
index dc9f75c..0529b2d 100644
--- a/Documentation/driver-api/fpga/fpga-region.rst
+++ b/Documentation/driver-api/fpga/fpga-region.rst
@@ -34,41 +34,6 @@ fpga_image_info including:
  * flags indicating specifics such as whether the image is for partial
    reconfiguration.
 
-How to program an FPGA using a region
--------------------------------------
-
-First, allocate the info struct::
-
-	info = fpga_image_info_alloc(dev);
-	if (!info)
-		return -ENOMEM;
-
-Set flags as needed, i.e.::
-
-	info->flags |= FPGA_MGR_PARTIAL_RECONFIG;
-
-Point to your FPGA image, such as::
-
-	info->sgt = &sgt;
-
-Add info to region and do the programming::
-
-	region->info = info;
-	ret = fpga_region_program_fpga(region);
-
-:c:func:`fpga_region_program_fpga()` operates on info passed in the
-fpga_image_info (region->info).  This function will attempt to:
-
- * lock the region's mutex
- * lock the region's FPGA manager
- * build a list of FPGA bridges if a method has been specified to do so
- * disable the bridges
- * program the FPGA
- * re-enable the bridges
- * release the locks
-
-Then you will want to enumerate whatever hardware has appeared in the FPGA.
-
 How to add a new FPGA region
 ----------------------------
 
@@ -77,15 +42,36 @@ An example of usage can be seen in the probe function of [#f2]_.
 .. [#f1] ../devicetree/bindings/fpga/fpga-region.txt
 .. [#f2] ../../drivers/fpga/of-fpga-region.c
 
-API to program an FPGA
-----------------------
-
-.. kernel-doc:: drivers/fpga/fpga-region.c
-   :functions: fpga_region_program_fpga
-
 API to add a new FPGA region
 ----------------------------
 
+* struct :c:type:`fpga_region` — The FPGA region struct
+* :c:func:`devm_fpga_region_create` — Allocate and init a region struct
+* :c:func:`fpga_region_register` —  Register an FPGA region
+* :c:func:`fpga_region_unregister` —  Unregister an FPGA region
+
+The FPGA region's probe function will need to get a reference to the FPGA
+Manager it will be using to do the programming.  This usually would happen
+during the region's probe function.
+
+* :c:func:`fpga_mgr_get` — Get a reference to an FPGA manager, raise ref count
+* :c:func:`of_fpga_mgr_get` —  Get a reference to an FPGA manager, raise ref count,
+  given a device node.
+* :c:func:`fpga_mgr_put` — Put an FPGA manager
+
+The FPGA region will need to specify which bridges to control while programming
+the FPGA.  The region driver can build a list of bridges during probe time
+(:c:member:`fpga_region->bridge_list`) or it can have a function that creates
+the list of bridges to program just before programming
+(:c:member:`fpga_region->get_bridges`).  The FPGA bridge framework supplies the
+following APIs to handle building or tearing down that list.
+
+* :c:func:`fpga_bridge_get_to_list` — Get a ref of an FPGA bridge, add it to a
+  list
+* :c:func:`of_fpga_bridge_get_to_list` — Get a ref of an FPGA bridge, add it to a
+  list, given a device node
+* :c:func:`fpga_bridges_put` — Given a list of bridges, put them
+
 .. kernel-doc:: include/linux/fpga/fpga-region.h
    :functions: fpga_region
 
@@ -93,13 +79,25 @@ API to add a new FPGA region
    :functions: devm_fpga_region_create
 
 .. kernel-doc:: drivers/fpga/fpga-region.c
-   :functions: fpga_region_create
-
-.. kernel-doc:: drivers/fpga/fpga-region.c
-   :functions: fpga_region_free
-
-.. kernel-doc:: drivers/fpga/fpga-region.c
    :functions: fpga_region_register
 
 .. kernel-doc:: drivers/fpga/fpga-region.c
    :functions: fpga_region_unregister
+
+.. kernel-doc:: drivers/fpga/fpga-mgr.c
+   :functions: fpga_mgr_get
+
+.. kernel-doc:: drivers/fpga/fpga-mgr.c
+   :functions: of_fpga_mgr_get
+
+.. kernel-doc:: drivers/fpga/fpga-mgr.c
+   :functions: fpga_mgr_put
+
+.. kernel-doc:: drivers/fpga/fpga-bridge.c
+   :functions: fpga_bridge_get_to_list
+
+.. kernel-doc:: drivers/fpga/fpga-bridge.c
+   :functions: of_fpga_bridge_get_to_list
+
+.. kernel-doc:: drivers/fpga/fpga-bridge.c
+   :functions: fpga_bridges_put
diff --git a/Documentation/driver-api/fpga/index.rst b/Documentation/driver-api/fpga/index.rst
index c51e5eb..31a4773 100644
--- a/Documentation/driver-api/fpga/index.rst
+++ b/Documentation/driver-api/fpga/index.rst
@@ -11,3 +11,5 @@ FPGA Subsystem
    fpga-mgr
    fpga-bridge
    fpga-region
+   fpga-programming
+
diff --git a/Documentation/driver-api/fpga/intro.rst b/Documentation/driver-api/fpga/intro.rst
index 50d1cab..f54c7da 100644
--- a/Documentation/driver-api/fpga/intro.rst
+++ b/Documentation/driver-api/fpga/intro.rst
@@ -44,7 +44,7 @@ FPGA Region
 -----------
 
 If you are adding a new interface to the FPGA framework, add it on top
-of an FPGA region to allow the most reuse of your interface.
+of an FPGA region.
 
 The FPGA Region framework (fpga-region.c) associates managers and
 bridges as reconfigurable regions.  A region may refer to the whole
-- 
2.7.4


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

* Re: [PATCH v3 1/4] fpga: mgr: add devm_fpga_mgr_create
  2018-09-26 16:12 ` [PATCH v3 1/4] fpga: mgr: add devm_fpga_mgr_create Alan Tull
@ 2018-10-15 14:54   ` Alan Tull
  2018-10-15 17:32     ` Moritz Fischer
  2018-10-16  7:02     ` Federico Vaga
  0 siblings, 2 replies; 11+ messages in thread
From: Alan Tull @ 2018-10-15 14:54 UTC (permalink / raw)
  To: Moritz Fischer, Jonathan Corbet
  Cc: Randy Dunlap, Federico Vaga, linux-kernel, linux-fpga,
	Linux Doc Mailing List

On Wed, Sep 26, 2018 at 11:12 AM Alan Tull <atull@kernel.org> wrote:

Any other comments on this patchset?

Alan

>
> Add devm_fpga_mgr_create() which is the managed
> version of fpga_mgr_create().
>
> Change current FPGA manager drivers to use
> devm_fpga_mgr_create()
>
> Signed-off-by: Alan Tull <atull@kernel.org>
> Suggested-by: Federico Vaga <federico.vaga@cern.ch>
> ---
> v2: add suggested-by
> v3: remove some unclear documentation about fpga_mgr_unregister
> ---
>  Documentation/driver-api/fpga/fpga-mgr.rst | 13 +++---
>  drivers/fpga/altera-cvp.c                  |  8 ++--
>  drivers/fpga/altera-pr-ip-core.c           |  9 +----
>  drivers/fpga/altera-ps-spi.c               | 11 ++---
>  drivers/fpga/dfl-fme-mgr.c                 | 11 ++---
>  drivers/fpga/fpga-mgr.c                    | 64 ++++++++++++++++++++++++++----
>  drivers/fpga/ice40-spi.c                   | 10 ++---
>  drivers/fpga/machxo2-spi.c                 | 11 ++---
>  drivers/fpga/socfpga-a10.c                 |  5 +--
>  drivers/fpga/socfpga.c                     | 10 ++---
>  drivers/fpga/ts73xx-fpga.c                 | 11 ++---
>  drivers/fpga/xilinx-spi.c                  | 12 ++----
>  drivers/fpga/zynq-fpga.c                   |  5 +--
>  include/linux/fpga/fpga-mgr.h              |  4 ++
>  14 files changed, 97 insertions(+), 87 deletions(-)
>
> diff --git a/Documentation/driver-api/fpga/fpga-mgr.rst b/Documentation/driver-api/fpga/fpga-mgr.rst
> index 82b6dbb..db8885e 100644
> --- a/Documentation/driver-api/fpga/fpga-mgr.rst
> +++ b/Documentation/driver-api/fpga/fpga-mgr.rst
> @@ -49,18 +49,14 @@ probe function calls fpga_mgr_register(), such as::
>                  * them in priv
>                  */
>
> -               mgr = fpga_mgr_create(dev, "Altera SOCFPGA FPGA Manager",
> -                                     &socfpga_fpga_ops, priv);
> +               mgr = devm_fpga_mgr_create(dev, "Altera SOCFPGA FPGA Manager",
> +                                          &socfpga_fpga_ops, priv);
>                 if (!mgr)
>                         return -ENOMEM;
>
>                 platform_set_drvdata(pdev, mgr);
>
> -               ret = fpga_mgr_register(mgr);
> -               if (ret)
> -                       fpga_mgr_free(mgr);
> -
> -               return ret;
> +               return fpga_mgr_register(mgr);
>         }
>
>         static int socfpga_fpga_remove(struct platform_device *pdev)
> @@ -170,6 +166,9 @@ API for implementing a new FPGA Manager driver
>     :functions: fpga_manager_ops
>
>  .. kernel-doc:: drivers/fpga/fpga-mgr.c
> +   :functions: devm_fpga_mgr_create
> +
> +.. kernel-doc:: drivers/fpga/fpga-mgr.c
>     :functions: fpga_mgr_create
>
>  .. kernel-doc:: drivers/fpga/fpga-mgr.c
> diff --git a/drivers/fpga/altera-cvp.c b/drivers/fpga/altera-cvp.c
> index 7fa7936..610a155 100644
> --- a/drivers/fpga/altera-cvp.c
> +++ b/drivers/fpga/altera-cvp.c
> @@ -453,8 +453,8 @@ 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 = fpga_mgr_create(&pdev->dev, conf->mgr_name,
> -                             &altera_cvp_ops, conf);
> +       mgr = devm_fpga_mgr_create(&pdev->dev, conf->mgr_name,
> +                                  &altera_cvp_ops, conf);
>         if (!mgr) {
>                 ret = -ENOMEM;
>                 goto err_unmap;
> @@ -463,10 +463,8 @@ static int altera_cvp_probe(struct pci_dev *pdev,
>         pci_set_drvdata(pdev, mgr);
>
>         ret = fpga_mgr_register(mgr);
> -       if (ret) {
> -               fpga_mgr_free(mgr);
> +       if (ret)
>                 goto err_unmap;
> -       }
>
>         ret = driver_create_file(&altera_cvp_driver.driver,
>                                  &driver_attr_chkcfg);
> diff --git a/drivers/fpga/altera-pr-ip-core.c b/drivers/fpga/altera-pr-ip-core.c
> index 65e0b6a..a7a3bf0 100644
> --- a/drivers/fpga/altera-pr-ip-core.c
> +++ b/drivers/fpga/altera-pr-ip-core.c
> @@ -177,7 +177,6 @@ int alt_pr_register(struct device *dev, void __iomem *reg_base)
>  {
>         struct alt_pr_priv *priv;
>         struct fpga_manager *mgr;
> -       int ret;
>         u32 val;
>
>         priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> @@ -192,17 +191,13 @@ 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 = fpga_mgr_create(dev, dev_name(dev), &alt_pr_ops, priv);
> +       mgr = devm_fpga_mgr_create(dev, dev_name(dev), &alt_pr_ops, priv);
>         if (!mgr)
>                 return -ENOMEM;
>
>         dev_set_drvdata(dev, mgr);
>
> -       ret = fpga_mgr_register(mgr);
> -       if (ret)
> -               fpga_mgr_free(mgr);
> -
> -       return ret;
> +       return fpga_mgr_register(mgr);
>  }
>  EXPORT_SYMBOL_GPL(alt_pr_register);
>
> diff --git a/drivers/fpga/altera-ps-spi.c b/drivers/fpga/altera-ps-spi.c
> index 24b25c6..33aafda 100644
> --- a/drivers/fpga/altera-ps-spi.c
> +++ b/drivers/fpga/altera-ps-spi.c
> @@ -239,7 +239,6 @@ static int altera_ps_probe(struct spi_device *spi)
>         struct altera_ps_conf *conf;
>         const struct of_device_id *of_id;
>         struct fpga_manager *mgr;
> -       int ret;
>
>         conf = devm_kzalloc(&spi->dev, sizeof(*conf), GFP_KERNEL);
>         if (!conf)
> @@ -275,18 +274,14 @@ 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 = fpga_mgr_create(&spi->dev, conf->mgr_name,
> -                             &altera_ps_ops, conf);
> +       mgr = devm_fpga_mgr_create(&spi->dev, conf->mgr_name,
> +                                  &altera_ps_ops, conf);
>         if (!mgr)
>                 return -ENOMEM;
>
>         spi_set_drvdata(spi, mgr);
>
> -       ret = fpga_mgr_register(mgr);
> -       if (ret)
> -               fpga_mgr_free(mgr);
> -
> -       return ret;
> +       return fpga_mgr_register(mgr);
>  }
>
>  static int altera_ps_remove(struct spi_device *spi)
> diff --git a/drivers/fpga/dfl-fme-mgr.c b/drivers/fpga/dfl-fme-mgr.c
> index 9f045d0..76f3770 100644
> --- a/drivers/fpga/dfl-fme-mgr.c
> +++ b/drivers/fpga/dfl-fme-mgr.c
> @@ -287,7 +287,6 @@ static int fme_mgr_probe(struct platform_device *pdev)
>         struct fme_mgr_priv *priv;
>         struct fpga_manager *mgr;
>         struct resource *res;
> -       int ret;
>
>         priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>         if (!priv)
> @@ -309,19 +308,15 @@ static int fme_mgr_probe(struct platform_device *pdev)
>
>         fme_mgr_get_compat_id(priv->ioaddr, compat_id);
>
> -       mgr = fpga_mgr_create(dev, "DFL FME FPGA Manager",
> -                             &fme_mgr_ops, priv);
> +       mgr = devm_fpga_mgr_create(dev, "DFL FME FPGA Manager",
> +                                  &fme_mgr_ops, priv);
>         if (!mgr)
>                 return -ENOMEM;
>
>         mgr->compat_id = compat_id;
>         platform_set_drvdata(pdev, mgr);
>
> -       ret = fpga_mgr_register(mgr);
> -       if (ret)
> -               fpga_mgr_free(mgr);
> -
> -       return ret;
> +       return fpga_mgr_register(mgr);
>  }
>
>  static int fme_mgr_remove(struct platform_device *pdev)
> diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c
> index a41b07e..c386681 100644
> --- a/drivers/fpga/fpga-mgr.c
> +++ b/drivers/fpga/fpga-mgr.c
> @@ -558,6 +558,9 @@ EXPORT_SYMBOL_GPL(fpga_mgr_unlock);
>   * @mops:      pointer to structure of fpga manager ops
>   * @priv:      fpga manager private data
>   *
> + * The caller of this function is responsible for freeing the struct with
> + * fpga_mgr_free().  Using devm_fpga_mgr_create() instead is recommended.
> + *
>   * Return: pointer to struct fpga_manager or NULL
>   */
>  struct fpga_manager *fpga_mgr_create(struct device *dev, const char *name,
> @@ -618,8 +621,8 @@ struct fpga_manager *fpga_mgr_create(struct device *dev, const char *name,
>  EXPORT_SYMBOL_GPL(fpga_mgr_create);
>
>  /**
> - * fpga_mgr_free - deallocate a FPGA manager
> - * @mgr:       fpga manager struct created by fpga_mgr_create
> + * fpga_mgr_free - free a FPGA manager created with fpga_mgr_create()
> + * @mgr:       fpga manager struct
>   */
>  void fpga_mgr_free(struct fpga_manager *mgr)
>  {
> @@ -628,9 +631,55 @@ void fpga_mgr_free(struct fpga_manager *mgr)
>  }
>  EXPORT_SYMBOL_GPL(fpga_mgr_free);
>
> +static void devm_fpga_mgr_release(struct device *dev, void *res)
> +{
> +       struct fpga_manager *mgr = *(struct fpga_manager **)res;
> +
> +       fpga_mgr_free(mgr);
> +}
> +
> +/**
> + * devm_fpga_mgr_create - create and initialize a managed FPGA manager struct
> + * @dev:       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 a 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.
> + *
> + * Return: pointer to struct fpga_manager or NULL
> + */
> +struct fpga_manager *devm_fpga_mgr_create(struct device *dev, const char *name,
> +                                         const struct fpga_manager_ops *mops,
> +                                         void *priv)
> +{
> +       struct fpga_manager **ptr, *mgr;
> +
> +       ptr = devres_alloc(devm_fpga_mgr_release, sizeof(*ptr), GFP_KERNEL);
> +       if (!ptr)
> +               return NULL;
> +
> +       mgr = fpga_mgr_create(dev, name, mops, priv);
> +       if (!mgr) {
> +               devres_free(ptr);
> +       } else {
> +               *ptr = mgr;
> +               devres_add(dev, ptr);
> +       }
> +
> +       return mgr;
> +}
> +EXPORT_SYMBOL_GPL(devm_fpga_mgr_create);
> +
>  /**
>   * fpga_mgr_register - register a FPGA manager
> - * @mgr:       fpga manager struct created by fpga_mgr_create
> + * @mgr: fpga manager struct
>   *
>   * Return: 0 on success, negative error code otherwise.
>   */
> @@ -661,8 +710,10 @@ int fpga_mgr_register(struct fpga_manager *mgr)
>  EXPORT_SYMBOL_GPL(fpga_mgr_register);
>
>  /**
> - * fpga_mgr_unregister - unregister and free a FPGA manager
> - * @mgr:       fpga manager struct
> + * fpga_mgr_unregister - unregister a FPGA manager
> + * @mgr: fpga manager struct
> + *
> + * This function is intended for use in a FPGA manager driver's remove function.
>   */
>  void fpga_mgr_unregister(struct fpga_manager *mgr)
>  {
> @@ -681,9 +732,6 @@ EXPORT_SYMBOL_GPL(fpga_mgr_unregister);
>
>  static void fpga_mgr_dev_release(struct device *dev)
>  {
> -       struct fpga_manager *mgr = to_fpga_manager(dev);
> -
> -       fpga_mgr_free(mgr);
>  }
>
>  static int __init fpga_mgr_class_init(void)
> diff --git a/drivers/fpga/ice40-spi.c b/drivers/fpga/ice40-spi.c
> index 5981c7e..6154661 100644
> --- a/drivers/fpga/ice40-spi.c
> +++ b/drivers/fpga/ice40-spi.c
> @@ -175,18 +175,14 @@ static int ice40_fpga_probe(struct spi_device *spi)
>                 return ret;
>         }
>
> -       mgr = fpga_mgr_create(dev, "Lattice iCE40 FPGA Manager",
> -                             &ice40_fpga_ops, priv);
> +       mgr = devm_fpga_mgr_create(dev, "Lattice iCE40 FPGA Manager",
> +                                  &ice40_fpga_ops, priv);
>         if (!mgr)
>                 return -ENOMEM;
>
>         spi_set_drvdata(spi, mgr);
>
> -       ret = fpga_mgr_register(mgr);
> -       if (ret)
> -               fpga_mgr_free(mgr);
> -
> -       return ret;
> +       return fpga_mgr_register(mgr);
>  }
>
>  static int ice40_fpga_remove(struct spi_device *spi)
> diff --git a/drivers/fpga/machxo2-spi.c b/drivers/fpga/machxo2-spi.c
> index a582e00..4d8a876 100644
> --- a/drivers/fpga/machxo2-spi.c
> +++ b/drivers/fpga/machxo2-spi.c
> @@ -356,25 +356,20 @@ static int machxo2_spi_probe(struct spi_device *spi)
>  {
>         struct device *dev = &spi->dev;
>         struct fpga_manager *mgr;
> -       int ret;
>
>         if (spi->max_speed_hz > MACHXO2_MAX_SPEED) {
>                 dev_err(dev, "Speed is too high\n");
>                 return -EINVAL;
>         }
>
> -       mgr = fpga_mgr_create(dev, "Lattice MachXO2 SPI FPGA Manager",
> -                             &machxo2_ops, spi);
> +       mgr = devm_fpga_mgr_create(dev, "Lattice MachXO2 SPI FPGA Manager",
> +                                  &machxo2_ops, spi);
>         if (!mgr)
>                 return -ENOMEM;
>
>         spi_set_drvdata(spi, mgr);
>
> -       ret = fpga_mgr_register(mgr);
> -       if (ret)
> -               fpga_mgr_free(mgr);
> -
> -       return ret;
> +       return fpga_mgr_register(mgr);
>  }
>
>  static int machxo2_spi_remove(struct spi_device *spi)
> diff --git a/drivers/fpga/socfpga-a10.c b/drivers/fpga/socfpga-a10.c
> index be30c48..573d88b 100644
> --- a/drivers/fpga/socfpga-a10.c
> +++ b/drivers/fpga/socfpga-a10.c
> @@ -508,8 +508,8 @@ static int socfpga_a10_fpga_probe(struct platform_device *pdev)
>                 return -EBUSY;
>         }
>
> -       mgr = fpga_mgr_create(dev, "SoCFPGA Arria10 FPGA Manager",
> -                             &socfpga_a10_fpga_mgr_ops, priv);
> +       mgr = devm_fpga_mgr_create(dev, "SoCFPGA Arria10 FPGA Manager",
> +                                  &socfpga_a10_fpga_mgr_ops, priv);
>         if (!mgr)
>                 return -ENOMEM;
>
> @@ -517,7 +517,6 @@ static int socfpga_a10_fpga_probe(struct platform_device *pdev)
>
>         ret = fpga_mgr_register(mgr);
>         if (ret) {
> -               fpga_mgr_free(mgr);
>                 clk_disable_unprepare(priv->clk);
>                 return ret;
>         }
> diff --git a/drivers/fpga/socfpga.c b/drivers/fpga/socfpga.c
> index 959d71f..4a8a2fc 100644
> --- a/drivers/fpga/socfpga.c
> +++ b/drivers/fpga/socfpga.c
> @@ -571,18 +571,14 @@ static int socfpga_fpga_probe(struct platform_device *pdev)
>         if (ret)
>                 return ret;
>
> -       mgr = fpga_mgr_create(dev, "Altera SOCFPGA FPGA Manager",
> -                             &socfpga_fpga_ops, priv);
> +       mgr = devm_fpga_mgr_create(dev, "Altera SOCFPGA FPGA Manager",
> +                                  &socfpga_fpga_ops, priv);
>         if (!mgr)
>                 return -ENOMEM;
>
>         platform_set_drvdata(pdev, mgr);
>
> -       ret = fpga_mgr_register(mgr);
> -       if (ret)
> -               fpga_mgr_free(mgr);
> -
> -       return ret;
> +       return fpga_mgr_register(mgr);
>  }
>
>  static int socfpga_fpga_remove(struct platform_device *pdev)
> diff --git a/drivers/fpga/ts73xx-fpga.c b/drivers/fpga/ts73xx-fpga.c
> index 08efd18..dc22a58 100644
> --- a/drivers/fpga/ts73xx-fpga.c
> +++ b/drivers/fpga/ts73xx-fpga.c
> @@ -118,7 +118,6 @@ static int ts73xx_fpga_probe(struct platform_device *pdev)
>         struct ts73xx_fpga_priv *priv;
>         struct fpga_manager *mgr;
>         struct resource *res;
> -       int ret;
>
>         priv = devm_kzalloc(kdev, sizeof(*priv), GFP_KERNEL);
>         if (!priv)
> @@ -133,18 +132,14 @@ static int ts73xx_fpga_probe(struct platform_device *pdev)
>                 return PTR_ERR(priv->io_base);
>         }
>
> -       mgr = fpga_mgr_create(kdev, "TS-73xx FPGA Manager",
> -                             &ts73xx_fpga_ops, priv);
> +       mgr = devm_fpga_mgr_create(kdev, "TS-73xx FPGA Manager",
> +                                  &ts73xx_fpga_ops, priv);
>         if (!mgr)
>                 return -ENOMEM;
>
>         platform_set_drvdata(pdev, mgr);
>
> -       ret = fpga_mgr_register(mgr);
> -       if (ret)
> -               fpga_mgr_free(mgr);
> -
> -       return ret;
> +       return fpga_mgr_register(mgr);
>  }
>
>  static int ts73xx_fpga_remove(struct platform_device *pdev)
> diff --git a/drivers/fpga/xilinx-spi.c b/drivers/fpga/xilinx-spi.c
> index 8d19459..469486b 100644
> --- a/drivers/fpga/xilinx-spi.c
> +++ b/drivers/fpga/xilinx-spi.c
> @@ -144,7 +144,6 @@ static int xilinx_spi_probe(struct spi_device *spi)
>  {
>         struct xilinx_spi_conf *conf;
>         struct fpga_manager *mgr;
> -       int ret;
>
>         conf = devm_kzalloc(&spi->dev, sizeof(*conf), GFP_KERNEL);
>         if (!conf)
> @@ -167,18 +166,15 @@ static int xilinx_spi_probe(struct spi_device *spi)
>                 return PTR_ERR(conf->done);
>         }
>
> -       mgr = fpga_mgr_create(&spi->dev, "Xilinx Slave Serial FPGA Manager",
> -                             &xilinx_spi_ops, conf);
> +       mgr = devm_fpga_mgr_create(&spi->dev,
> +                                  "Xilinx Slave Serial FPGA Manager",
> +                                  &xilinx_spi_ops, conf);
>         if (!mgr)
>                 return -ENOMEM;
>
>         spi_set_drvdata(spi, mgr);
>
> -       ret = fpga_mgr_register(mgr);
> -       if (ret)
> -               fpga_mgr_free(mgr);
> -
> -       return ret;
> +       return fpga_mgr_register(mgr);
>  }
>
>  static int xilinx_spi_remove(struct spi_device *spi)
> diff --git a/drivers/fpga/zynq-fpga.c b/drivers/fpga/zynq-fpga.c
> index 3110e00..bb82efe 100644
> --- a/drivers/fpga/zynq-fpga.c
> +++ b/drivers/fpga/zynq-fpga.c
> @@ -614,8 +614,8 @@ static int zynq_fpga_probe(struct platform_device *pdev)
>
>         clk_disable(priv->clk);
>
> -       mgr = fpga_mgr_create(dev, "Xilinx Zynq FPGA Manager",
> -                             &zynq_fpga_ops, priv);
> +       mgr = devm_fpga_mgr_create(dev, "Xilinx Zynq FPGA Manager",
> +                                  &zynq_fpga_ops, priv);
>         if (!mgr)
>                 return -ENOMEM;
>
> @@ -624,7 +624,6 @@ static int zynq_fpga_probe(struct platform_device *pdev)
>         err = fpga_mgr_register(mgr);
>         if (err) {
>                 dev_err(dev, "unable to register FPGA manager\n");
> -               fpga_mgr_free(mgr);
>                 clk_unprepare(priv->clk);
>                 return err;
>         }
> diff --git a/include/linux/fpga/fpga-mgr.h b/include/linux/fpga/fpga-mgr.h
> index 8ab5df7..e8ca62b 100644
> --- a/include/linux/fpga/fpga-mgr.h
> +++ b/include/linux/fpga/fpga-mgr.h
> @@ -198,4 +198,8 @@ 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 *devm_fpga_mgr_create(struct device *dev, const char *name,
> +                                         const struct fpga_manager_ops *mops,
> +                                         void *priv);
> +
>  #endif /*_LINUX_FPGA_MGR_H */
> --
> 2.7.4
>

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

* Re: [PATCH v3 1/4] fpga: mgr: add devm_fpga_mgr_create
  2018-10-15 14:54   ` Alan Tull
@ 2018-10-15 17:32     ` Moritz Fischer
  2018-10-16  7:02     ` Federico Vaga
  1 sibling, 0 replies; 11+ messages in thread
From: Moritz Fischer @ 2018-10-15 17:32 UTC (permalink / raw)
  To: Alan Tull
  Cc: Jonathan Corbet, Randy Dunlap, Federico Vaga,
	Linux Kernel Mailing List, linux-fpga, Linux Doc Mailing List

Hi Alan,

On Mon, Oct 15, 2018 at 7:55 AM Alan Tull <atull@kernel.org> wrote:
>
> On Wed, Sep 26, 2018 at 11:12 AM Alan Tull <atull@kernel.org> wrote:
>
> Any other comments on this patchset?

Looks good to me, Sorry for the delay

Acked-by: Moritz Fischer <mdf@kernel.org>

Cheers,

Moritz

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

* Re: [PATCH v3 3/4] fpga: add devm_fpga_region_create
  2018-09-26 16:12 ` [PATCH v3 3/4] fpga: add devm_fpga_region_create Alan Tull
@ 2018-10-15 20:10   ` Moritz Fischer
  0 siblings, 0 replies; 11+ messages in thread
From: Moritz Fischer @ 2018-10-15 20:10 UTC (permalink / raw)
  To: Alan Tull
  Cc: Jonathan Corbet, Randy Dunlap, Federico Vaga,
	Linux Kernel Mailing List, linux-fpga, Linux Doc Mailing List

On Wed, Sep 26, 2018 at 9:12 AM Alan Tull <atull@kernel.org> wrote:
>
> Add devm_fpga_region_create() which is the
> managed version of fpga_region_create().
>
> Change current region drivers to use
> devm_fpga_region_create().
>
> Signed-off-by: Alan Tull <atull@kernel.org>
> Suggested-by: Federico Vaga <federico.vaga@cern.ch>

Acked-by: Moritz Fischer <mdf@kernel.org>
> ---
> v2: add suggested-by
> v3: remove some unclear documentation about fpga_region_unregister
> ---
>  Documentation/driver-api/fpga/fpga-region.rst |  3 ++
>  drivers/fpga/dfl-fme-region.c                 |  6 +--
>  drivers/fpga/dfl.c                            |  6 +--
>  drivers/fpga/fpga-region.c                    | 65 +++++++++++++++++++++++----
>  drivers/fpga/of-fpga-region.c                 |  6 +--
>  include/linux/fpga/fpga-region.h              |  4 ++
>  6 files changed, 70 insertions(+), 20 deletions(-)
>
> diff --git a/Documentation/driver-api/fpga/fpga-region.rst b/Documentation/driver-api/fpga/fpga-region.rst
> index f30333c..dc9f75c 100644
> --- a/Documentation/driver-api/fpga/fpga-region.rst
> +++ b/Documentation/driver-api/fpga/fpga-region.rst
> @@ -90,6 +90,9 @@ API to add a new FPGA region
>     :functions: fpga_region
>
>  .. kernel-doc:: drivers/fpga/fpga-region.c
> +   :functions: devm_fpga_region_create
> +
> +.. kernel-doc:: drivers/fpga/fpga-region.c
>     :functions: fpga_region_create
>
>  .. kernel-doc:: drivers/fpga/fpga-region.c
> diff --git a/drivers/fpga/dfl-fme-region.c b/drivers/fpga/dfl-fme-region.c
> index 3fa0de2..1eeb42a 100644
> --- a/drivers/fpga/dfl-fme-region.c
> +++ b/drivers/fpga/dfl-fme-region.c
> @@ -39,7 +39,7 @@ static int fme_region_probe(struct platform_device *pdev)
>         if (IS_ERR(mgr))
>                 return -EPROBE_DEFER;
>
> -       region = fpga_region_create(dev, mgr, fme_region_get_bridges);
> +       region = devm_fpga_region_create(dev, mgr, fme_region_get_bridges);
>         if (!region) {
>                 ret = -ENOMEM;
>                 goto eprobe_mgr_put;
> @@ -51,14 +51,12 @@ static int fme_region_probe(struct platform_device *pdev)
>
>         ret = fpga_region_register(region);
>         if (ret)
> -               goto region_free;
> +               goto eprobe_mgr_put;
>
>         dev_dbg(dev, "DFL FME FPGA Region probed\n");
>
>         return 0;
>
> -region_free:
> -       fpga_region_free(region);
>  eprobe_mgr_put:
>         fpga_mgr_put(mgr);
>         return ret;
> diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
> index a9b521b..2c09e50 100644
> --- a/drivers/fpga/dfl.c
> +++ b/drivers/fpga/dfl.c
> @@ -899,7 +899,7 @@ dfl_fpga_feature_devs_enumerate(struct dfl_fpga_enum_info *info)
>         if (!cdev)
>                 return ERR_PTR(-ENOMEM);
>
> -       cdev->region = fpga_region_create(info->dev, NULL, NULL);
> +       cdev->region = devm_fpga_region_create(info->dev, NULL, NULL);
>         if (!cdev->region) {
>                 ret = -ENOMEM;
>                 goto free_cdev_exit;
> @@ -911,7 +911,7 @@ dfl_fpga_feature_devs_enumerate(struct dfl_fpga_enum_info *info)
>
>         ret = fpga_region_register(cdev->region);
>         if (ret)
> -               goto free_region_exit;
> +               goto free_cdev_exit;
>
>         /* create and init build info for enumeration */
>         binfo = devm_kzalloc(info->dev, sizeof(*binfo), GFP_KERNEL);
> @@ -942,8 +942,6 @@ dfl_fpga_feature_devs_enumerate(struct dfl_fpga_enum_info *info)
>
>  unregister_region_exit:
>         fpga_region_unregister(cdev->region);
> -free_region_exit:
> -       fpga_region_free(cdev->region);
>  free_cdev_exit:
>         devm_kfree(info->dev, cdev);
>         return ERR_PTR(ret);
> diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c
> index 0d65220..bde5a9d 100644
> --- a/drivers/fpga/fpga-region.c
> +++ b/drivers/fpga/fpga-region.c
> @@ -185,6 +185,10 @@ ATTRIBUTE_GROUPS(fpga_region);
>   * @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.
> + *
>   * Return: struct fpga_region or NULL
>   */
>  struct fpga_region
> @@ -230,8 +234,8 @@ struct fpga_region
>  EXPORT_SYMBOL_GPL(fpga_region_create);
>
>  /**
> - * fpga_region_free - free a struct fpga_region
> - * @region: FPGA region created by fpga_region_create
> + * fpga_region_free - free a FPGA region created by fpga_region_create()
> + * @region: FPGA region
>   */
>  void fpga_region_free(struct fpga_region *region)
>  {
> @@ -240,21 +244,69 @@ void fpga_region_free(struct fpga_region *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);
> +}
> +
> +/**
> + * devm_fpga_region_create - create and initialize a managed FPGA region struct
> + * @dev: 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 a 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.
> + *
> + * Return: struct fpga_region or NULL
> + */
> +struct fpga_region
> +*devm_fpga_region_create(struct device *dev,
> +                        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(dev, mgr, get_bridges);
> +       if (!region) {
> +               devres_free(ptr);
> +       } else {
> +               *ptr = region;
> +               devres_add(dev, ptr);
> +       }
> +
> +       return region;
> +}
> +EXPORT_SYMBOL_GPL(devm_fpga_region_create);
> +
>  /**
>   * fpga_region_register - register a FPGA region
> - * @region: FPGA region created by fpga_region_create
> + * @region: FPGA region
> + *
>   * Return: 0 or -errno
>   */
>  int fpga_region_register(struct fpga_region *region)
>  {
>         return device_add(&region->dev);
> -
>  }
>  EXPORT_SYMBOL_GPL(fpga_region_register);
>
>  /**
> - * fpga_region_unregister - unregister and free a FPGA region
> + * fpga_region_unregister - unregister a FPGA region
>   * @region: FPGA region
> + *
> + * This function is intended for use in a FPGA region driver's remove function.
>   */
>  void fpga_region_unregister(struct fpga_region *region)
>  {
> @@ -264,9 +316,6 @@ EXPORT_SYMBOL_GPL(fpga_region_unregister);
>
>  static void fpga_region_dev_release(struct device *dev)
>  {
> -       struct fpga_region *region = to_fpga_region(dev);
> -
> -       fpga_region_free(region);
>  }
>
>  /**
> diff --git a/drivers/fpga/of-fpga-region.c b/drivers/fpga/of-fpga-region.c
> index d6a70e4..75f64ab 100644
> --- a/drivers/fpga/of-fpga-region.c
> +++ b/drivers/fpga/of-fpga-region.c
> @@ -410,7 +410,7 @@ static int of_fpga_region_probe(struct platform_device *pdev)
>         if (IS_ERR(mgr))
>                 return -EPROBE_DEFER;
>
> -       region = fpga_region_create(dev, mgr, of_fpga_region_get_bridges);
> +       region = devm_fpga_region_create(dev, mgr, of_fpga_region_get_bridges);
>         if (!region) {
>                 ret = -ENOMEM;
>                 goto eprobe_mgr_put;
> @@ -418,7 +418,7 @@ static int of_fpga_region_probe(struct platform_device *pdev)
>
>         ret = fpga_region_register(region);
>         if (ret)
> -               goto eprobe_free;
> +               goto eprobe_mgr_put;
>
>         of_platform_populate(np, fpga_region_of_match, NULL, &region->dev);
>         platform_set_drvdata(pdev, region);
> @@ -427,8 +427,6 @@ static int of_fpga_region_probe(struct platform_device *pdev)
>
>         return 0;
>
> -eprobe_free:
> -       fpga_region_free(region);
>  eprobe_mgr_put:
>         fpga_mgr_put(mgr);
>         return ret;
> diff --git a/include/linux/fpga/fpga-region.h b/include/linux/fpga/fpga-region.h
> index 0521b7f..27cb706 100644
> --- a/include/linux/fpga/fpga-region.h
> +++ b/include/linux/fpga/fpga-region.h
> @@ -44,4 +44,8 @@ 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
> +*devm_fpga_region_create(struct device *dev, struct fpga_manager *mgr,
> +                       int (*get_bridges)(struct fpga_region *));
> +
>  #endif /* _FPGA_REGION_H */
> --
> 2.7.4
>

Thanks,
Moritz

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

* Re: [PATCH v3 4/4] docs: fpga: document programming fpgas using regions
  2018-09-26 16:12 ` [PATCH v3 4/4] docs: fpga: document programming fpgas using regions Alan Tull
@ 2018-10-15 20:11   ` Moritz Fischer
  2018-10-15 20:40     ` Alan Tull
  0 siblings, 1 reply; 11+ messages in thread
From: Moritz Fischer @ 2018-10-15 20:11 UTC (permalink / raw)
  To: Alan Tull
  Cc: Jonathan Corbet, Randy Dunlap, Federico Vaga,
	Linux Kernel Mailing List, linux-fpga, Linux Doc Mailing List

On Wed, Sep 26, 2018 at 9:12 AM Alan Tull <atull@kernel.org> wrote:
>
> Clarify the intention that interfaces and upper layers use
> regions rather than managers directly.
>
> Rearrange API documentation to better group the API functions
> used to create FPGA mgr/bridge/regions and the API used for
> programming FPGAs.
>
> Signed-off-by: Alan Tull <atull@kernel.org>
> Suggested-by: Federico Vaga <federico.vaga@cern.ch>

Acked-by: Moritz Fischer <mdf@kernel.org>
> ---
> v2: Add suggested-by
>     s/a FPGA/an FPGA/
>     document freeing the fpga_mgr_info on success path
>     minor formatting fixes and suggested edits
> v3: no changes
> ---
>  Documentation/driver-api/fpga/fpga-bridge.rst      |  38 ++-----
>  Documentation/driver-api/fpga/fpga-mgr.rst         | 117 ++-------------------
>  Documentation/driver-api/fpga/fpga-programming.rst | 107 +++++++++++++++++++
>  Documentation/driver-api/fpga/fpga-region.rst      |  92 ++++++++--------
>  Documentation/driver-api/fpga/index.rst            |   2 +
>  Documentation/driver-api/fpga/intro.rst            |   2 +-
>  6 files changed, 171 insertions(+), 187 deletions(-)
>  create mode 100644 Documentation/driver-api/fpga/fpga-programming.rst
>
> diff --git a/Documentation/driver-api/fpga/fpga-bridge.rst b/Documentation/driver-api/fpga/fpga-bridge.rst
> index ebbcbde..71c5a40 100644
> --- a/Documentation/driver-api/fpga/fpga-bridge.rst
> +++ b/Documentation/driver-api/fpga/fpga-bridge.rst
> @@ -4,6 +4,12 @@ FPGA Bridge
>  API to implement a new FPGA bridge
>  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> +* struct :c:type:`fpga_bridge` — The FPGA Bridge structure
> +* struct :c:type:`fpga_bridge_ops` — Low level Bridge driver ops
> +* :c:func:`devm_fpga_bridge_create()` — Allocate and init a bridge struct
> +* :c:func:`fpga_bridge_register()` — Register a bridge
> +* :c:func:`fpga_bridge_unregister()` — Unregister a bridge
> +
>  .. kernel-doc:: include/linux/fpga/fpga-bridge.h
>     :functions: fpga_bridge
>
> @@ -14,39 +20,7 @@ API to implement a new FPGA bridge
>     :functions: devm_fpga_bridge_create
>
>  .. kernel-doc:: drivers/fpga/fpga-bridge.c
> -   :functions: fpga_bridge_create
> -
> -.. kernel-doc:: drivers/fpga/fpga-bridge.c
> -   :functions: fpga_bridge_free
> -
> -.. kernel-doc:: drivers/fpga/fpga-bridge.c
>     :functions: fpga_bridge_register
>
>  .. kernel-doc:: drivers/fpga/fpga-bridge.c
>     :functions: fpga_bridge_unregister
> -
> -API to control an FPGA bridge
> -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> -
> -You probably won't need these directly.  FPGA regions should handle this.
> -
> -.. kernel-doc:: drivers/fpga/fpga-bridge.c
> -   :functions: of_fpga_bridge_get
> -
> -.. kernel-doc:: drivers/fpga/fpga-bridge.c
> -   :functions: fpga_bridge_get
> -
> -.. kernel-doc:: drivers/fpga/fpga-bridge.c
> -   :functions: fpga_bridge_put
> -
> -.. kernel-doc:: drivers/fpga/fpga-bridge.c
> -   :functions: fpga_bridge_get_to_list
> -
> -.. kernel-doc:: drivers/fpga/fpga-bridge.c
> -   :functions: of_fpga_bridge_get_to_list
> -
> -.. kernel-doc:: drivers/fpga/fpga-bridge.c
> -   :functions: fpga_bridge_enable
> -
> -.. kernel-doc:: drivers/fpga/fpga-bridge.c
> -   :functions: fpga_bridge_disable
> diff --git a/Documentation/driver-api/fpga/fpga-mgr.rst b/Documentation/driver-api/fpga/fpga-mgr.rst
> index db8885e..576f194 100644
> --- a/Documentation/driver-api/fpga/fpga-mgr.rst
> +++ b/Documentation/driver-api/fpga/fpga-mgr.rst
> @@ -98,67 +98,19 @@ The ops include a .state function which will determine the state the FPGA is in
>  and return a code of type enum fpga_mgr_states.  It doesn't result in a change
>  in state.
>
> -How to write an image buffer to a supported FPGA
> -------------------------------------------------
> -
> -Some sample code::
> -
> -       #include <linux/fpga/fpga-mgr.h>
> -
> -       struct fpga_manager *mgr;
> -       struct fpga_image_info *info;
> -       int ret;
> -
> -       /*
> -        * Get a reference to FPGA manager.  The manager is not locked, so you can
> -        * hold onto this reference without it preventing programming.
> -        *
> -        * This example uses the device node of the manager.  Alternatively, use
> -        * fpga_mgr_get(dev) instead if you have the device.
> -        */
> -       mgr = of_fpga_mgr_get(mgr_node);
> -
> -       /* struct with information about the FPGA image to program. */
> -       info = fpga_image_info_alloc(dev);
> -
> -       /* flags indicates whether to do full or partial reconfiguration */
> -       info->flags = FPGA_MGR_PARTIAL_RECONFIG;
> -
> -       /*
> -        * At this point, indicate where the image is. This is pseudo-code; you're
> -        * going to use one of these three.
> -        */
> -       if (image is in a scatter gather table) {
> -
> -               info->sgt = [your scatter gather table]
> -
> -       } else if (image is in a buffer) {
> -
> -               info->buf = [your image buffer]
> -               info->count = [image buffer size]
> -
> -       } else if (image is in a firmware file) {
> -
> -               info->firmware_name = devm_kstrdup(dev, firmware_name, GFP_KERNEL);
> -
> -       }
> -
> -       /* Get exclusive control of FPGA manager */
> -       ret = fpga_mgr_lock(mgr);
> -
> -       /* Load the buffer to the FPGA */
> -       ret = fpga_mgr_buf_load(mgr, &info, buf, count);
> -
> -       /* Release the FPGA manager */
> -       fpga_mgr_unlock(mgr);
> -       fpga_mgr_put(mgr);
> -
> -       /* Deallocate the image info if you're done with it */
> -       fpga_image_info_free(info);
> -
>  API for implementing a new FPGA Manager driver
>  ----------------------------------------------
>
> +* ``fpga_mgr_states`` —  Values for :c:member:`fpga_manager->state`.
> +* struct :c:type:`fpga_manager` —  the FPGA manager struct
> +* struct :c:type:`fpga_manager_ops` —  Low level FPGA manager driver ops
> +* :c:func:`devm_fpga_mgr_create` —  Allocate and init a manager struct
> +* :c:func:`fpga_mgr_register` —  Register an FPGA manager
> +* :c:func:`fpga_mgr_unregister` —  Unregister an FPGA manager
> +
> +.. kernel-doc:: include/linux/fpga/fpga-mgr.h
> +   :functions: fpga_mgr_states
> +
>  .. kernel-doc:: include/linux/fpga/fpga-mgr.h
>     :functions: fpga_manager
>
> @@ -169,56 +121,7 @@ API for implementing a new FPGA Manager driver
>     :functions: devm_fpga_mgr_create
>
>  .. kernel-doc:: drivers/fpga/fpga-mgr.c
> -   :functions: fpga_mgr_create
> -
> -.. kernel-doc:: drivers/fpga/fpga-mgr.c
> -   :functions: fpga_mgr_free
> -
> -.. kernel-doc:: drivers/fpga/fpga-mgr.c
>     :functions: fpga_mgr_register
>
>  .. kernel-doc:: drivers/fpga/fpga-mgr.c
>     :functions: fpga_mgr_unregister
> -
> -API for programming an FPGA
> ----------------------------
> -
> -FPGA Manager flags
> -
> -.. kernel-doc:: include/linux/fpga/fpga-mgr.h
> -   :doc: FPGA Manager flags
> -
> -.. kernel-doc:: include/linux/fpga/fpga-mgr.h
> -   :functions: fpga_image_info
> -
> -.. kernel-doc:: include/linux/fpga/fpga-mgr.h
> -   :functions: fpga_mgr_states
> -
> -.. kernel-doc:: drivers/fpga/fpga-mgr.c
> -   :functions: fpga_image_info_alloc
> -
> -.. kernel-doc:: drivers/fpga/fpga-mgr.c
> -   :functions: fpga_image_info_free
> -
> -.. kernel-doc:: drivers/fpga/fpga-mgr.c
> -   :functions: of_fpga_mgr_get
> -
> -.. kernel-doc:: drivers/fpga/fpga-mgr.c
> -   :functions: fpga_mgr_get
> -
> -.. kernel-doc:: drivers/fpga/fpga-mgr.c
> -   :functions: fpga_mgr_put
> -
> -.. kernel-doc:: drivers/fpga/fpga-mgr.c
> -   :functions: fpga_mgr_lock
> -
> -.. kernel-doc:: drivers/fpga/fpga-mgr.c
> -   :functions: fpga_mgr_unlock
> -
> -.. kernel-doc:: include/linux/fpga/fpga-mgr.h
> -   :functions: fpga_mgr_states
> -
> -Note - use :c:func:`fpga_region_program_fpga()` instead of :c:func:`fpga_mgr_load()`
> -
> -.. kernel-doc:: drivers/fpga/fpga-mgr.c
> -   :functions: fpga_mgr_load
> diff --git a/Documentation/driver-api/fpga/fpga-programming.rst b/Documentation/driver-api/fpga/fpga-programming.rst
> new file mode 100644
> index 0000000..b5484df
> --- /dev/null
> +++ b/Documentation/driver-api/fpga/fpga-programming.rst
> @@ -0,0 +1,107 @@
> +In-kernel API for FPGA Programming
> +==================================
> +
> +Overview
> +--------
> +
> +The in-kernel API for FPGA programming is a combination of APIs from
> +FPGA manager, bridge, and regions.  The actual function used to
> +trigger FPGA programming is :c:func:`fpga_region_program_fpga()`.
> +
> +:c:func:`fpga_region_program_fpga()` uses functionality supplied by
> +the FPGA manager and bridges.  It will:
> +
> + * lock the region's mutex
> + * lock the mutex of the region's FPGA manager
> + * build a list of FPGA bridges if a method has been specified to do so
> + * disable the bridges
> + * program the FPGA using info passed in :c:member:`fpga_region->info`.
> + * re-enable the bridges
> + * release the locks
> +
> +The struct fpga_image_info specifies what FPGA image to program.  It is
> +allocated/freed by :c:func:`fpga_image_info_alloc()` and freed with
> +:c:func:`fpga_image_info_free()`
> +
> +How to program an FPGA using a region
> +-------------------------------------
> +
> +When the FPGA region driver probed, it was given a pointer to an FPGA manager
> +driver so it knows which manager to use.  The region also either has a list of
> +bridges to control during programming or it has a pointer to a function that
> +will generate that list.  Here's some sample code of what to do next::
> +
> +       #include <linux/fpga/fpga-mgr.h>
> +       #include <linux/fpga/fpga-region.h>
> +
> +       struct fpga_image_info *info;
> +       int ret;
> +
> +       /*
> +        * First, alloc the struct with information about the FPGA image to
> +        * program.
> +        */
> +       info = fpga_image_info_alloc(dev);
> +       if (!info)
> +               return -ENOMEM;
> +
> +       /* Set flags as needed, such as: */
> +       info->flags = FPGA_MGR_PARTIAL_RECONFIG;
> +
> +       /*
> +        * Indicate where the FPGA image is. This is pseudo-code; you're
> +        * going to use one of these three.
> +        */
> +       if (image is in a scatter gather table) {
> +
> +               info->sgt = [your scatter gather table]
> +
> +       } else if (image is in a buffer) {
> +
> +               info->buf = [your image buffer]
> +               info->count = [image buffer size]
> +
> +       } else if (image is in a firmware file) {
> +
> +               info->firmware_name = devm_kstrdup(dev, firmware_name,
> +                                                  GFP_KERNEL);
> +
> +       }
> +
> +       /* Add info to region and do the programming */
> +       region->info = info;
> +       ret = fpga_region_program_fpga(region);
> +
> +       /* Deallocate the image info if you're done with it */
> +       region->info = NULL;
> +       fpga_image_info_free(info);
> +
> +       if (ret)
> +               return ret;
> +
> +       /* Now enumerate whatever hardware has appeared in the FPGA. */
> +
> +API for programming an FPGA
> +---------------------------
> +
> +* :c:func:`fpga_region_program_fpga` —  Program an FPGA
> +* :c:type:`fpga_image_info` —  Specifies what FPGA image to program
> +* :c:func:`fpga_image_info_alloc()` —  Allocate an FPGA image info struct
> +* :c:func:`fpga_image_info_free()` —  Free an FPGA image info struct
> +
> +.. kernel-doc:: drivers/fpga/fpga-region.c
> +   :functions: fpga_region_program_fpga
> +
> +FPGA Manager flags
> +
> +.. kernel-doc:: include/linux/fpga/fpga-mgr.h
> +   :doc: FPGA Manager flags
> +
> +.. kernel-doc:: include/linux/fpga/fpga-mgr.h
> +   :functions: fpga_image_info
> +
> +.. kernel-doc:: drivers/fpga/fpga-mgr.c
> +   :functions: fpga_image_info_alloc
> +
> +.. kernel-doc:: drivers/fpga/fpga-mgr.c
> +   :functions: fpga_image_info_free
> diff --git a/Documentation/driver-api/fpga/fpga-region.rst b/Documentation/driver-api/fpga/fpga-region.rst
> index dc9f75c..0529b2d 100644
> --- a/Documentation/driver-api/fpga/fpga-region.rst
> +++ b/Documentation/driver-api/fpga/fpga-region.rst
> @@ -34,41 +34,6 @@ fpga_image_info including:
>   * flags indicating specifics such as whether the image is for partial
>     reconfiguration.
>
> -How to program an FPGA using a region
> --------------------------------------
> -
> -First, allocate the info struct::
> -
> -       info = fpga_image_info_alloc(dev);
> -       if (!info)
> -               return -ENOMEM;
> -
> -Set flags as needed, i.e.::
> -
> -       info->flags |= FPGA_MGR_PARTIAL_RECONFIG;
> -
> -Point to your FPGA image, such as::
> -
> -       info->sgt = &sgt;
> -
> -Add info to region and do the programming::
> -
> -       region->info = info;
> -       ret = fpga_region_program_fpga(region);
> -
> -:c:func:`fpga_region_program_fpga()` operates on info passed in the
> -fpga_image_info (region->info).  This function will attempt to:
> -
> - * lock the region's mutex
> - * lock the region's FPGA manager
> - * build a list of FPGA bridges if a method has been specified to do so
> - * disable the bridges
> - * program the FPGA
> - * re-enable the bridges
> - * release the locks
> -
> -Then you will want to enumerate whatever hardware has appeared in the FPGA.
> -
>  How to add a new FPGA region
>  ----------------------------
>
> @@ -77,15 +42,36 @@ An example of usage can be seen in the probe function of [#f2]_.
>  .. [#f1] ../devicetree/bindings/fpga/fpga-region.txt
>  .. [#f2] ../../drivers/fpga/of-fpga-region.c
>
> -API to program an FPGA
> -----------------------
> -
> -.. kernel-doc:: drivers/fpga/fpga-region.c
> -   :functions: fpga_region_program_fpga
> -
>  API to add a new FPGA region
>  ----------------------------
>
> +* struct :c:type:`fpga_region` — The FPGA region struct
> +* :c:func:`devm_fpga_region_create` — Allocate and init a region struct
> +* :c:func:`fpga_region_register` —  Register an FPGA region
> +* :c:func:`fpga_region_unregister` —  Unregister an FPGA region
> +
> +The FPGA region's probe function will need to get a reference to the FPGA
> +Manager it will be using to do the programming.  This usually would happen
> +during the region's probe function.
> +
> +* :c:func:`fpga_mgr_get` — Get a reference to an FPGA manager, raise ref count
> +* :c:func:`of_fpga_mgr_get` —  Get a reference to an FPGA manager, raise ref count,
> +  given a device node.
> +* :c:func:`fpga_mgr_put` — Put an FPGA manager
> +
> +The FPGA region will need to specify which bridges to control while programming
> +the FPGA.  The region driver can build a list of bridges during probe time
> +(:c:member:`fpga_region->bridge_list`) or it can have a function that creates
> +the list of bridges to program just before programming
> +(:c:member:`fpga_region->get_bridges`).  The FPGA bridge framework supplies the
> +following APIs to handle building or tearing down that list.
> +
> +* :c:func:`fpga_bridge_get_to_list` — Get a ref of an FPGA bridge, add it to a
> +  list
> +* :c:func:`of_fpga_bridge_get_to_list` — Get a ref of an FPGA bridge, add it to a
> +  list, given a device node
> +* :c:func:`fpga_bridges_put` — Given a list of bridges, put them
> +
>  .. kernel-doc:: include/linux/fpga/fpga-region.h
>     :functions: fpga_region
>
> @@ -93,13 +79,25 @@ API to add a new FPGA region
>     :functions: devm_fpga_region_create
>
>  .. kernel-doc:: drivers/fpga/fpga-region.c
> -   :functions: fpga_region_create
> -
> -.. kernel-doc:: drivers/fpga/fpga-region.c
> -   :functions: fpga_region_free
> -
> -.. kernel-doc:: drivers/fpga/fpga-region.c
>     :functions: fpga_region_register
>
>  .. kernel-doc:: drivers/fpga/fpga-region.c
>     :functions: fpga_region_unregister
> +
> +.. kernel-doc:: drivers/fpga/fpga-mgr.c
> +   :functions: fpga_mgr_get
> +
> +.. kernel-doc:: drivers/fpga/fpga-mgr.c
> +   :functions: of_fpga_mgr_get
> +
> +.. kernel-doc:: drivers/fpga/fpga-mgr.c
> +   :functions: fpga_mgr_put
> +
> +.. kernel-doc:: drivers/fpga/fpga-bridge.c
> +   :functions: fpga_bridge_get_to_list
> +
> +.. kernel-doc:: drivers/fpga/fpga-bridge.c
> +   :functions: of_fpga_bridge_get_to_list
> +
> +.. kernel-doc:: drivers/fpga/fpga-bridge.c
> +   :functions: fpga_bridges_put
> diff --git a/Documentation/driver-api/fpga/index.rst b/Documentation/driver-api/fpga/index.rst
> index c51e5eb..31a4773 100644
> --- a/Documentation/driver-api/fpga/index.rst
> +++ b/Documentation/driver-api/fpga/index.rst
> @@ -11,3 +11,5 @@ FPGA Subsystem
>     fpga-mgr
>     fpga-bridge
>     fpga-region
> +   fpga-programming
> +
> diff --git a/Documentation/driver-api/fpga/intro.rst b/Documentation/driver-api/fpga/intro.rst
> index 50d1cab..f54c7da 100644
> --- a/Documentation/driver-api/fpga/intro.rst
> +++ b/Documentation/driver-api/fpga/intro.rst
> @@ -44,7 +44,7 @@ FPGA Region
>  -----------
>
>  If you are adding a new interface to the FPGA framework, add it on top
> -of an FPGA region to allow the most reuse of your interface.
> +of an FPGA region.
>
>  The FPGA Region framework (fpga-region.c) associates managers and
>  bridges as reconfigurable regions.  A region may refer to the whole
> --
> 2.7.4
>

Thanks,
Moritz

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

* Re: [PATCH v3 4/4] docs: fpga: document programming fpgas using regions
  2018-10-15 20:11   ` Moritz Fischer
@ 2018-10-15 20:40     ` Alan Tull
  0 siblings, 0 replies; 11+ messages in thread
From: Alan Tull @ 2018-10-15 20:40 UTC (permalink / raw)
  To: Moritz Fischer
  Cc: Jonathan Corbet, Randy Dunlap, Federico Vaga, linux-kernel,
	linux-fpga, Linux Doc Mailing List

On Mon, Oct 15, 2018 at 3:11 PM Moritz Fischer
<moritz.fischer.private@gmail.com> wrote:
>
> On Wed, Sep 26, 2018 at 9:12 AM Alan Tull <atull@kernel.org> wrote:
> >
> > Clarify the intention that interfaces and upper layers use
> > regions rather than managers directly.
> >
> > Rearrange API documentation to better group the API functions
> > used to create FPGA mgr/bridge/regions and the API used for
> > programming FPGAs.
> >
> > Signed-off-by: Alan Tull <atull@kernel.org>
> > Suggested-by: Federico Vaga <federico.vaga@cern.ch>
>
> Acked-by: Moritz Fischer <mdf@kernel.org>

Thanks Moritz!

Alan

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

* Re: [PATCH v3 1/4] fpga: mgr: add devm_fpga_mgr_create
  2018-10-15 14:54   ` Alan Tull
  2018-10-15 17:32     ` Moritz Fischer
@ 2018-10-16  7:02     ` Federico Vaga
  1 sibling, 0 replies; 11+ messages in thread
From: Federico Vaga @ 2018-10-16  7:02 UTC (permalink / raw)
  To: Alan Tull
  Cc: Moritz Fischer, Jonathan Corbet, Randy Dunlap, linux-kernel,
	linux-fpga, Linux Doc Mailing List

not comment here

On Monday, October 15, 2018 4:54:25 PM CEST Alan Tull wrote:
> On Wed, Sep 26, 2018 at 11:12 AM Alan Tull <atull@kernel.org> wrote:
> 
> Any other comments on this patchset?
> 
> Alan
> 
> > Add devm_fpga_mgr_create() which is the managed
> > version of fpga_mgr_create().
> > 
> > Change current FPGA manager drivers to use
> > devm_fpga_mgr_create()
> > 
> > Signed-off-by: Alan Tull <atull@kernel.org>
> > Suggested-by: Federico Vaga <federico.vaga@cern.ch>
> > ---
> > v2: add suggested-by
> > v3: remove some unclear documentation about fpga_mgr_unregister
> > ---
> > 
> >  Documentation/driver-api/fpga/fpga-mgr.rst | 13 +++---
> >  drivers/fpga/altera-cvp.c                  |  8 ++--
> >  drivers/fpga/altera-pr-ip-core.c           |  9 +----
> >  drivers/fpga/altera-ps-spi.c               | 11 ++---
> >  drivers/fpga/dfl-fme-mgr.c                 | 11 ++---
> >  drivers/fpga/fpga-mgr.c                    | 64
> >  ++++++++++++++++++++++++++---- drivers/fpga/ice40-spi.c                 
> >   | 10 ++---
> >  drivers/fpga/machxo2-spi.c                 | 11 ++---
> >  drivers/fpga/socfpga-a10.c                 |  5 +--
> >  drivers/fpga/socfpga.c                     | 10 ++---
> >  drivers/fpga/ts73xx-fpga.c                 | 11 ++---
> >  drivers/fpga/xilinx-spi.c                  | 12 ++----
> >  drivers/fpga/zynq-fpga.c                   |  5 +--
> >  include/linux/fpga/fpga-mgr.h              |  4 ++
> >  14 files changed, 97 insertions(+), 87 deletions(-)
> > 
> > diff --git a/Documentation/driver-api/fpga/fpga-mgr.rst
> > b/Documentation/driver-api/fpga/fpga-mgr.rst index 82b6dbb..db8885e
> > 100644
> > --- a/Documentation/driver-api/fpga/fpga-mgr.rst
> > +++ b/Documentation/driver-api/fpga/fpga-mgr.rst
> > 
> > @@ -49,18 +49,14 @@ probe function calls fpga_mgr_register(), such as::
> >                  * them in priv
> >                  */
> > 
> > -               mgr = fpga_mgr_create(dev, "Altera SOCFPGA FPGA Manager",
> > -                                     &socfpga_fpga_ops, priv);
> > +               mgr = devm_fpga_mgr_create(dev, "Altera SOCFPGA FPGA
> > Manager", +                                          &socfpga_fpga_ops,
> > priv);> 
> >                 if (!mgr)
> >                 
> >                         return -ENOMEM;
> >                 
> >                 platform_set_drvdata(pdev, mgr);
> > 
> > -               ret = fpga_mgr_register(mgr);
> > -               if (ret)
> > -                       fpga_mgr_free(mgr);
> > -
> > -               return ret;
> > +               return fpga_mgr_register(mgr);
> > 
> >         }
> >         
> >         static int socfpga_fpga_remove(struct platform_device *pdev)
> > 
> > @@ -170,6 +166,9 @@ API for implementing a new FPGA Manager driver
> > 
> >     :functions: fpga_manager_ops
> >  
> >  .. kernel-doc:: drivers/fpga/fpga-mgr.c
> > 
> > +   :functions: devm_fpga_mgr_create
> > +
> > +.. kernel-doc:: drivers/fpga/fpga-mgr.c
> > 
> >     :functions: fpga_mgr_create
> >  
> >  .. kernel-doc:: drivers/fpga/fpga-mgr.c
> > 
> > diff --git a/drivers/fpga/altera-cvp.c b/drivers/fpga/altera-cvp.c
> > index 7fa7936..610a155 100644
> > --- a/drivers/fpga/altera-cvp.c
> > +++ b/drivers/fpga/altera-cvp.c
> > @@ -453,8 +453,8 @@ 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 = fpga_mgr_create(&pdev->dev, conf->mgr_name,
> > -                             &altera_cvp_ops, conf);
> > +       mgr = devm_fpga_mgr_create(&pdev->dev, conf->mgr_name,
> > +                                  &altera_cvp_ops, conf);
> > 
> >         if (!mgr) {
> >         
> >                 ret = -ENOMEM;
> >                 goto err_unmap;
> > 
> > @@ -463,10 +463,8 @@ static int altera_cvp_probe(struct pci_dev *pdev,
> > 
> >         pci_set_drvdata(pdev, mgr);
> >         
> >         ret = fpga_mgr_register(mgr);
> > 
> > -       if (ret) {
> > -               fpga_mgr_free(mgr);
> > +       if (ret)
> > 
> >                 goto err_unmap;
> > 
> > -       }
> > 
> >         ret = driver_create_file(&altera_cvp_driver.driver,
> >         
> >                                  &driver_attr_chkcfg);
> > 
> > diff --git a/drivers/fpga/altera-pr-ip-core.c
> > b/drivers/fpga/altera-pr-ip-core.c index 65e0b6a..a7a3bf0 100644
> > --- a/drivers/fpga/altera-pr-ip-core.c
> > +++ b/drivers/fpga/altera-pr-ip-core.c
> > @@ -177,7 +177,6 @@ int alt_pr_register(struct device *dev, void __iomem
> > *reg_base)> 
> >  {
> >  
> >         struct alt_pr_priv *priv;
> >         struct fpga_manager *mgr;
> > 
> > -       int ret;
> > 
> >         u32 val;
> >         
> >         priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> > 
> > @@ -192,17 +191,13 @@ 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 = fpga_mgr_create(dev, dev_name(dev), &alt_pr_ops, priv);
> > +       mgr = devm_fpga_mgr_create(dev, dev_name(dev), &alt_pr_ops, priv);
> > 
> >         if (!mgr)
> >         
> >                 return -ENOMEM;
> >         
> >         dev_set_drvdata(dev, mgr);
> > 
> > -       ret = fpga_mgr_register(mgr);
> > -       if (ret)
> > -               fpga_mgr_free(mgr);
> > -
> > -       return ret;
> > +       return fpga_mgr_register(mgr);
> > 
> >  }
> >  EXPORT_SYMBOL_GPL(alt_pr_register);
> > 
> > diff --git a/drivers/fpga/altera-ps-spi.c b/drivers/fpga/altera-ps-spi.c
> > index 24b25c6..33aafda 100644
> > --- a/drivers/fpga/altera-ps-spi.c
> > +++ b/drivers/fpga/altera-ps-spi.c
> > @@ -239,7 +239,6 @@ static int altera_ps_probe(struct spi_device *spi)
> > 
> >         struct altera_ps_conf *conf;
> >         const struct of_device_id *of_id;
> >         struct fpga_manager *mgr;
> > 
> > -       int ret;
> > 
> >         conf = devm_kzalloc(&spi->dev, sizeof(*conf), GFP_KERNEL);
> >         if (!conf)
> > 
> > @@ -275,18 +274,14 @@ 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 = fpga_mgr_create(&spi->dev, conf->mgr_name,
> > -                             &altera_ps_ops, conf);
> > +       mgr = devm_fpga_mgr_create(&spi->dev, conf->mgr_name,
> > +                                  &altera_ps_ops, conf);
> > 
> >         if (!mgr)
> >         
> >                 return -ENOMEM;
> >         
> >         spi_set_drvdata(spi, mgr);
> > 
> > -       ret = fpga_mgr_register(mgr);
> > -       if (ret)
> > -               fpga_mgr_free(mgr);
> > -
> > -       return ret;
> > +       return fpga_mgr_register(mgr);
> > 
> >  }
> >  
> >  static int altera_ps_remove(struct spi_device *spi)
> > 
> > diff --git a/drivers/fpga/dfl-fme-mgr.c b/drivers/fpga/dfl-fme-mgr.c
> > index 9f045d0..76f3770 100644
> > --- a/drivers/fpga/dfl-fme-mgr.c
> > +++ b/drivers/fpga/dfl-fme-mgr.c
> > @@ -287,7 +287,6 @@ static int fme_mgr_probe(struct platform_device *pdev)
> > 
> >         struct fme_mgr_priv *priv;
> >         struct fpga_manager *mgr;
> >         struct resource *res;
> > 
> > -       int ret;
> > 
> >         priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> >         if (!priv)
> > 
> > @@ -309,19 +308,15 @@ static int fme_mgr_probe(struct platform_device
> > *pdev)> 
> >         fme_mgr_get_compat_id(priv->ioaddr, compat_id);
> > 
> > -       mgr = fpga_mgr_create(dev, "DFL FME FPGA Manager",
> > -                             &fme_mgr_ops, priv);
> > +       mgr = devm_fpga_mgr_create(dev, "DFL FME FPGA Manager",
> > +                                  &fme_mgr_ops, priv);
> > 
> >         if (!mgr)
> >         
> >                 return -ENOMEM;
> >         
> >         mgr->compat_id = compat_id;
> >         platform_set_drvdata(pdev, mgr);
> > 
> > -       ret = fpga_mgr_register(mgr);
> > -       if (ret)
> > -               fpga_mgr_free(mgr);
> > -
> > -       return ret;
> > +       return fpga_mgr_register(mgr);
> > 
> >  }
> >  
> >  static int fme_mgr_remove(struct platform_device *pdev)
> > 
> > diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c
> > index a41b07e..c386681 100644
> > --- a/drivers/fpga/fpga-mgr.c
> > +++ b/drivers/fpga/fpga-mgr.c
> > @@ -558,6 +558,9 @@ EXPORT_SYMBOL_GPL(fpga_mgr_unlock);
> > 
> >   * @mops:      pointer to structure of fpga manager ops
> >   * @priv:      fpga manager private data
> >   *
> > 
> > + * The caller of this function is responsible for freeing the struct with
> > + * fpga_mgr_free().  Using devm_fpga_mgr_create() instead is recommended.
> > + *
> > 
> >   * Return: pointer to struct fpga_manager or NULL
> >   */
> >  
> >  struct fpga_manager *fpga_mgr_create(struct device *dev, const char
> >  *name,
> > 
> > @@ -618,8 +621,8 @@ struct fpga_manager *fpga_mgr_create(struct device
> > *dev, const char *name,> 
> >  EXPORT_SYMBOL_GPL(fpga_mgr_create);
> >  
> >  /**
> > 
> > - * fpga_mgr_free - deallocate a FPGA manager
> > - * @mgr:       fpga manager struct created by fpga_mgr_create
> > + * fpga_mgr_free - free a FPGA manager created with fpga_mgr_create()
> > + * @mgr:       fpga manager struct
> > 
> >   */
> >  
> >  void fpga_mgr_free(struct fpga_manager *mgr)
> >  {
> > 
> > @@ -628,9 +631,55 @@ void fpga_mgr_free(struct fpga_manager *mgr)
> > 
> >  }
> >  EXPORT_SYMBOL_GPL(fpga_mgr_free);
> > 
> > +static void devm_fpga_mgr_release(struct device *dev, void *res)
> > +{
> > +       struct fpga_manager *mgr = *(struct fpga_manager **)res;
> > +
> > +       fpga_mgr_free(mgr);
> > +}
> > +
> > +/**
> > + * devm_fpga_mgr_create - create and initialize a managed FPGA manager
> > struct + * @dev:       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 a 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. + *
> > + * Return: pointer to struct fpga_manager or NULL
> > + */
> > +struct fpga_manager *devm_fpga_mgr_create(struct device *dev, const char
> > *name, +                                         const struct
> > fpga_manager_ops *mops, +                                         void
> > *priv)
> > +{
> > +       struct fpga_manager **ptr, *mgr;
> > +
> > +       ptr = devres_alloc(devm_fpga_mgr_release, sizeof(*ptr),
> > GFP_KERNEL); +       if (!ptr)
> > +               return NULL;
> > +
> > +       mgr = fpga_mgr_create(dev, name, mops, priv);
> > +       if (!mgr) {
> > +               devres_free(ptr);
> > +       } else {
> > +               *ptr = mgr;
> > +               devres_add(dev, ptr);
> > +       }
> > +
> > +       return mgr;
> > +}
> > +EXPORT_SYMBOL_GPL(devm_fpga_mgr_create);
> > +
> > 
> >  /**
> >  
> >   * fpga_mgr_register - register a FPGA manager
> > 
> > - * @mgr:       fpga manager struct created by fpga_mgr_create
> > + * @mgr: fpga manager struct
> > 
> >   *
> >   * Return: 0 on success, negative error code otherwise.
> >   */
> > 
> > @@ -661,8 +710,10 @@ int fpga_mgr_register(struct fpga_manager *mgr)
> > 
> >  EXPORT_SYMBOL_GPL(fpga_mgr_register);
> >  
> >  /**
> > 
> > - * fpga_mgr_unregister - unregister and free a FPGA manager
> > - * @mgr:       fpga manager struct
> > + * fpga_mgr_unregister - unregister a FPGA manager
> > + * @mgr: fpga manager struct
> > + *
> > + * This function is intended for use in a FPGA manager driver's remove
> > function.> 
> >   */
> >  
> >  void fpga_mgr_unregister(struct fpga_manager *mgr)
> >  {
> > 
> > @@ -681,9 +732,6 @@ EXPORT_SYMBOL_GPL(fpga_mgr_unregister);
> > 
> >  static void fpga_mgr_dev_release(struct device *dev)
> >  {
> > 
> > -       struct fpga_manager *mgr = to_fpga_manager(dev);
> > -
> > -       fpga_mgr_free(mgr);
> > 
> >  }
> >  
> >  static int __init fpga_mgr_class_init(void)
> > 
> > diff --git a/drivers/fpga/ice40-spi.c b/drivers/fpga/ice40-spi.c
> > index 5981c7e..6154661 100644
> > --- a/drivers/fpga/ice40-spi.c
> > +++ b/drivers/fpga/ice40-spi.c
> > @@ -175,18 +175,14 @@ static int ice40_fpga_probe(struct spi_device *spi)
> > 
> >                 return ret;
> >         
> >         }
> > 
> > -       mgr = fpga_mgr_create(dev, "Lattice iCE40 FPGA Manager",
> > -                             &ice40_fpga_ops, priv);
> > +       mgr = devm_fpga_mgr_create(dev, "Lattice iCE40 FPGA Manager",
> > +                                  &ice40_fpga_ops, priv);
> > 
> >         if (!mgr)
> >         
> >                 return -ENOMEM;
> >         
> >         spi_set_drvdata(spi, mgr);
> > 
> > -       ret = fpga_mgr_register(mgr);
> > -       if (ret)
> > -               fpga_mgr_free(mgr);
> > -
> > -       return ret;
> > +       return fpga_mgr_register(mgr);
> > 
> >  }
> >  
> >  static int ice40_fpga_remove(struct spi_device *spi)
> > 
> > diff --git a/drivers/fpga/machxo2-spi.c b/drivers/fpga/machxo2-spi.c
> > index a582e00..4d8a876 100644
> > --- a/drivers/fpga/machxo2-spi.c
> > +++ b/drivers/fpga/machxo2-spi.c
> > @@ -356,25 +356,20 @@ static int machxo2_spi_probe(struct spi_device *spi)
> > 
> >  {
> >  
> >         struct device *dev = &spi->dev;
> >         struct fpga_manager *mgr;
> > 
> > -       int ret;
> > 
> >         if (spi->max_speed_hz > MACHXO2_MAX_SPEED) {
> >         
> >                 dev_err(dev, "Speed is too high\n");
> >                 return -EINVAL;
> >         
> >         }
> > 
> > -       mgr = fpga_mgr_create(dev, "Lattice MachXO2 SPI FPGA Manager",
> > -                             &machxo2_ops, spi);
> > +       mgr = devm_fpga_mgr_create(dev, "Lattice MachXO2 SPI FPGA
> > Manager",
> > +                                  &machxo2_ops, spi);
> > 
> >         if (!mgr)
> >         
> >                 return -ENOMEM;
> >         
> >         spi_set_drvdata(spi, mgr);
> > 
> > -       ret = fpga_mgr_register(mgr);
> > -       if (ret)
> > -               fpga_mgr_free(mgr);
> > -
> > -       return ret;
> > +       return fpga_mgr_register(mgr);
> > 
> >  }
> >  
> >  static int machxo2_spi_remove(struct spi_device *spi)
> > 
> > diff --git a/drivers/fpga/socfpga-a10.c b/drivers/fpga/socfpga-a10.c
> > index be30c48..573d88b 100644
> > --- a/drivers/fpga/socfpga-a10.c
> > +++ b/drivers/fpga/socfpga-a10.c
> > @@ -508,8 +508,8 @@ static int socfpga_a10_fpga_probe(struct
> > platform_device *pdev)> 
> >                 return -EBUSY;
> >         
> >         }
> > 
> > -       mgr = fpga_mgr_create(dev, "SoCFPGA Arria10 FPGA Manager",
> > -                             &socfpga_a10_fpga_mgr_ops, priv);
> > +       mgr = devm_fpga_mgr_create(dev, "SoCFPGA Arria10 FPGA Manager",
> > +                                  &socfpga_a10_fpga_mgr_ops, priv);
> > 
> >         if (!mgr)
> >         
> >                 return -ENOMEM;
> > 
> > @@ -517,7 +517,6 @@ static int socfpga_a10_fpga_probe(struct
> > platform_device *pdev)> 
> >         ret = fpga_mgr_register(mgr);
> >         if (ret) {
> > 
> > -               fpga_mgr_free(mgr);
> > 
> >                 clk_disable_unprepare(priv->clk);
> >                 return ret;
> >         
> >         }
> > 
> > diff --git a/drivers/fpga/socfpga.c b/drivers/fpga/socfpga.c
> > index 959d71f..4a8a2fc 100644
> > --- a/drivers/fpga/socfpga.c
> > +++ b/drivers/fpga/socfpga.c
> > @@ -571,18 +571,14 @@ static int socfpga_fpga_probe(struct platform_device
> > *pdev)> 
> >         if (ret)
> >         
> >                 return ret;
> > 
> > -       mgr = fpga_mgr_create(dev, "Altera SOCFPGA FPGA Manager",
> > -                             &socfpga_fpga_ops, priv);
> > +       mgr = devm_fpga_mgr_create(dev, "Altera SOCFPGA FPGA Manager",
> > +                                  &socfpga_fpga_ops, priv);
> > 
> >         if (!mgr)
> >         
> >                 return -ENOMEM;
> >         
> >         platform_set_drvdata(pdev, mgr);
> > 
> > -       ret = fpga_mgr_register(mgr);
> > -       if (ret)
> > -               fpga_mgr_free(mgr);
> > -
> > -       return ret;
> > +       return fpga_mgr_register(mgr);
> > 
> >  }
> >  
> >  static int socfpga_fpga_remove(struct platform_device *pdev)
> > 
> > diff --git a/drivers/fpga/ts73xx-fpga.c b/drivers/fpga/ts73xx-fpga.c
> > index 08efd18..dc22a58 100644
> > --- a/drivers/fpga/ts73xx-fpga.c
> > +++ b/drivers/fpga/ts73xx-fpga.c
> > @@ -118,7 +118,6 @@ static int ts73xx_fpga_probe(struct platform_device
> > *pdev)> 
> >         struct ts73xx_fpga_priv *priv;
> >         struct fpga_manager *mgr;
> >         struct resource *res;
> > 
> > -       int ret;
> > 
> >         priv = devm_kzalloc(kdev, sizeof(*priv), GFP_KERNEL);
> >         if (!priv)
> > 
> > @@ -133,18 +132,14 @@ static int ts73xx_fpga_probe(struct platform_device
> > *pdev)> 
> >                 return PTR_ERR(priv->io_base);
> >         
> >         }
> > 
> > -       mgr = fpga_mgr_create(kdev, "TS-73xx FPGA Manager",
> > -                             &ts73xx_fpga_ops, priv);
> > +       mgr = devm_fpga_mgr_create(kdev, "TS-73xx FPGA Manager",
> > +                                  &ts73xx_fpga_ops, priv);
> > 
> >         if (!mgr)
> >         
> >                 return -ENOMEM;
> >         
> >         platform_set_drvdata(pdev, mgr);
> > 
> > -       ret = fpga_mgr_register(mgr);
> > -       if (ret)
> > -               fpga_mgr_free(mgr);
> > -
> > -       return ret;
> > +       return fpga_mgr_register(mgr);
> > 
> >  }
> >  
> >  static int ts73xx_fpga_remove(struct platform_device *pdev)
> > 
> > diff --git a/drivers/fpga/xilinx-spi.c b/drivers/fpga/xilinx-spi.c
> > index 8d19459..469486b 100644
> > --- a/drivers/fpga/xilinx-spi.c
> > +++ b/drivers/fpga/xilinx-spi.c
> > @@ -144,7 +144,6 @@ static int xilinx_spi_probe(struct spi_device *spi)
> > 
> >  {
> >  
> >         struct xilinx_spi_conf *conf;
> >         struct fpga_manager *mgr;
> > 
> > -       int ret;
> > 
> >         conf = devm_kzalloc(&spi->dev, sizeof(*conf), GFP_KERNEL);
> >         if (!conf)
> > 
> > @@ -167,18 +166,15 @@ static int xilinx_spi_probe(struct spi_device *spi)
> > 
> >                 return PTR_ERR(conf->done);
> >         
> >         }
> > 
> > -       mgr = fpga_mgr_create(&spi->dev, "Xilinx Slave Serial FPGA
> > Manager", -                             &xilinx_spi_ops, conf);
> > +       mgr = devm_fpga_mgr_create(&spi->dev,
> > +                                  "Xilinx Slave Serial FPGA Manager",
> > +                                  &xilinx_spi_ops, conf);
> > 
> >         if (!mgr)
> >         
> >                 return -ENOMEM;
> >         
> >         spi_set_drvdata(spi, mgr);
> > 
> > -       ret = fpga_mgr_register(mgr);
> > -       if (ret)
> > -               fpga_mgr_free(mgr);
> > -
> > -       return ret;
> > +       return fpga_mgr_register(mgr);
> > 
> >  }
> >  
> >  static int xilinx_spi_remove(struct spi_device *spi)
> > 
> > diff --git a/drivers/fpga/zynq-fpga.c b/drivers/fpga/zynq-fpga.c
> > index 3110e00..bb82efe 100644
> > --- a/drivers/fpga/zynq-fpga.c
> > +++ b/drivers/fpga/zynq-fpga.c
> > @@ -614,8 +614,8 @@ static int zynq_fpga_probe(struct platform_device
> > *pdev)> 
> >         clk_disable(priv->clk);
> > 
> > -       mgr = fpga_mgr_create(dev, "Xilinx Zynq FPGA Manager",
> > -                             &zynq_fpga_ops, priv);
> > +       mgr = devm_fpga_mgr_create(dev, "Xilinx Zynq FPGA Manager",
> > +                                  &zynq_fpga_ops, priv);
> > 
> >         if (!mgr)
> >         
> >                 return -ENOMEM;
> > 
> > @@ -624,7 +624,6 @@ static int zynq_fpga_probe(struct platform_device
> > *pdev)> 
> >         err = fpga_mgr_register(mgr);
> >         if (err) {
> >         
> >                 dev_err(dev, "unable to register FPGA manager\n");
> > 
> > -               fpga_mgr_free(mgr);
> > 
> >                 clk_unprepare(priv->clk);
> >                 return err;
> >         
> >         }
> > 
> > diff --git a/include/linux/fpga/fpga-mgr.h b/include/linux/fpga/fpga-mgr.h
> > index 8ab5df7..e8ca62b 100644
> > --- a/include/linux/fpga/fpga-mgr.h
> > +++ b/include/linux/fpga/fpga-mgr.h
> > @@ -198,4 +198,8 @@ 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 *devm_fpga_mgr_create(struct device *dev, const char
> > *name, +                                         const struct
> > fpga_manager_ops *mops, +                                         void
> > *priv);
> > +
> > 
> >  #endif /*_LINUX_FPGA_MGR_H */
> > 
> > --
> > 2.7.4





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

end of thread, other threads:[~2018-10-16  7:02 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-26 16:12 [PATCH v3 0/4] fpga: add devm managed create APIs Alan Tull
2018-09-26 16:12 ` [PATCH v3 1/4] fpga: mgr: add devm_fpga_mgr_create Alan Tull
2018-10-15 14:54   ` Alan Tull
2018-10-15 17:32     ` Moritz Fischer
2018-10-16  7:02     ` Federico Vaga
2018-09-26 16:12 ` [PATCH v3 2/4] fpga: bridge: add devm_fpga_bridge_create Alan Tull
2018-09-26 16:12 ` [PATCH v3 3/4] fpga: add devm_fpga_region_create Alan Tull
2018-10-15 20:10   ` Moritz Fischer
2018-09-26 16:12 ` [PATCH v3 4/4] docs: fpga: document programming fpgas using regions Alan Tull
2018-10-15 20:11   ` Moritz Fischer
2018-10-15 20:40     ` Alan Tull

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).