All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] FPGA Manager Patches for 4.18
@ 2018-04-27  1:26 ` Moritz Fischer
  0 siblings, 0 replies; 24+ messages in thread
From: Moritz Fischer @ 2018-04-27  1:26 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-arm-kernel, linux-doc, linux-fpga, michal.simek, corbet,
	atull, Moritz Fischer

Hi Greg,

Here's Alan's reworked patchset changing the API
for creating and registering FPGA Managers, Bridges and
Regions following your suggestions on the API.

These go on top of Paolo and Alan's patches that you
queued up the other day.

Thanks,

Moritz

Alan Tull (4):
  fpga: region: don't use drvdata in common fpga code
  fpga: manager: change api, don't use drvdata
  fpga: bridge: change api, don't use drvdata
  fpga: region: change api, add fpga_region_create/free

 Documentation/fpga/fpga-mgr.txt     | 35 ++++++++++---
 Documentation/fpga/fpga-region.txt  |  3 +-
 drivers/fpga/altera-cvp.c           | 19 +++++--
 drivers/fpga/altera-fpga2sdram.c    | 21 ++++++--
 drivers/fpga/altera-freeze-bridge.c | 22 ++++++--
 drivers/fpga/altera-hps2fpga.c      | 24 +++++++--
 drivers/fpga/altera-pr-ip-core.c    | 18 ++++++-
 drivers/fpga/altera-ps-spi.c        | 20 ++++++--
 drivers/fpga/fpga-bridge.c          | 70 +++++++++++++++++---------
 drivers/fpga/fpga-mgr.c             | 78 +++++++++++++++++++----------
 drivers/fpga/fpga-region.c          | 69 +++++++++++++++++++------
 drivers/fpga/ice40-spi.c            | 21 ++++++--
 drivers/fpga/machxo2-spi.c          | 20 ++++++--
 drivers/fpga/of-fpga-region.c       | 14 +++---
 drivers/fpga/socfpga-a10.c          | 14 ++++--
 drivers/fpga/socfpga.c              | 19 +++++--
 drivers/fpga/ts73xx-fpga.c          | 20 ++++++--
 drivers/fpga/xilinx-pr-decoupler.c  | 22 ++++++--
 drivers/fpga/xilinx-spi.c           | 20 ++++++--
 drivers/fpga/zynq-fpga.c            | 14 ++++--
 include/linux/fpga/fpga-bridge.h    |  9 ++--
 include/linux/fpga/fpga-mgr.h       | 10 ++--
 include/linux/fpga/fpga-region.h    | 11 ++--
 23 files changed, 429 insertions(+), 144 deletions(-)

-- 
2.17.0

^ permalink raw reply	[flat|nested] 24+ messages in thread
* [PATCH 0/4] FPGA Manager Patches for 4.18
@ 2018-04-27  1:26 ` Moritz Fischer
  0 siblings, 0 replies; 24+ messages in thread
From: Moritz Fischer @ 2018-04-27  1:26 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-arm-kernel, linux-doc, linux-fpga, michal.simek, corbet,
	atull, Moritz Fischer

Hi Greg,

Here's Alan's reworked patchset changing the API
for creating and registering FPGA Managers, Bridges and
Regions following your suggestions on the API.

These go on top of Paolo and Alan's patches that you
queued up the other day.

Thanks,

Moritz

Alan Tull (4):
  fpga: region: don't use drvdata in common fpga code
  fpga: manager: change api, don't use drvdata
  fpga: bridge: change api, don't use drvdata
  fpga: region: change api, add fpga_region_create/free

 Documentation/fpga/fpga-mgr.txt     | 35 ++++++++++---
 Documentation/fpga/fpga-region.txt  |  3 +-
 drivers/fpga/altera-cvp.c           | 19 +++++--
 drivers/fpga/altera-fpga2sdram.c    | 21 ++++++--
 drivers/fpga/altera-freeze-bridge.c | 22 ++++++--
 drivers/fpga/altera-hps2fpga.c      | 24 +++++++--
 drivers/fpga/altera-pr-ip-core.c    | 18 ++++++-
 drivers/fpga/altera-ps-spi.c        | 20 ++++++--
 drivers/fpga/fpga-bridge.c          | 70 +++++++++++++++++---------
 drivers/fpga/fpga-mgr.c             | 78 +++++++++++++++++++----------
 drivers/fpga/fpga-region.c          | 69 +++++++++++++++++++------
 drivers/fpga/ice40-spi.c            | 21 ++++++--
 drivers/fpga/machxo2-spi.c          | 20 ++++++--
 drivers/fpga/of-fpga-region.c       | 14 +++---
 drivers/fpga/socfpga-a10.c          | 14 ++++--
 drivers/fpga/socfpga.c              | 19 +++++--
 drivers/fpga/ts73xx-fpga.c          | 20 ++++++--
 drivers/fpga/xilinx-pr-decoupler.c  | 22 ++++++--
 drivers/fpga/xilinx-spi.c           | 20 ++++++--
 drivers/fpga/zynq-fpga.c            | 14 ++++--
 include/linux/fpga/fpga-bridge.h    |  9 ++--
 include/linux/fpga/fpga-mgr.h       | 10 ++--
 include/linux/fpga/fpga-region.h    | 11 ++--
 23 files changed, 429 insertions(+), 144 deletions(-)

-- 
2.17.0

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 24+ messages in thread
* [PATCH 0/4] FPGA Manager Patches for 4.18
@ 2018-04-27  1:26 ` Moritz Fischer
  0 siblings, 0 replies; 24+ messages in thread
From: Moritz Fischer @ 2018-04-27  1:26 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Greg,

Here's Alan's reworked patchset changing the API
for creating and registering FPGA Managers, Bridges and
Regions following your suggestions on the API.

These go on top of Paolo and Alan's patches that you
queued up the other day.

Thanks,

Moritz

Alan Tull (4):
  fpga: region: don't use drvdata in common fpga code
  fpga: manager: change api, don't use drvdata
  fpga: bridge: change api, don't use drvdata
  fpga: region: change api, add fpga_region_create/free

 Documentation/fpga/fpga-mgr.txt     | 35 ++++++++++---
 Documentation/fpga/fpga-region.txt  |  3 +-
 drivers/fpga/altera-cvp.c           | 19 +++++--
 drivers/fpga/altera-fpga2sdram.c    | 21 ++++++--
 drivers/fpga/altera-freeze-bridge.c | 22 ++++++--
 drivers/fpga/altera-hps2fpga.c      | 24 +++++++--
 drivers/fpga/altera-pr-ip-core.c    | 18 ++++++-
 drivers/fpga/altera-ps-spi.c        | 20 ++++++--
 drivers/fpga/fpga-bridge.c          | 70 +++++++++++++++++---------
 drivers/fpga/fpga-mgr.c             | 78 +++++++++++++++++++----------
 drivers/fpga/fpga-region.c          | 69 +++++++++++++++++++------
 drivers/fpga/ice40-spi.c            | 21 ++++++--
 drivers/fpga/machxo2-spi.c          | 20 ++++++--
 drivers/fpga/of-fpga-region.c       | 14 +++---
 drivers/fpga/socfpga-a10.c          | 14 ++++--
 drivers/fpga/socfpga.c              | 19 +++++--
 drivers/fpga/ts73xx-fpga.c          | 20 ++++++--
 drivers/fpga/xilinx-pr-decoupler.c  | 22 ++++++--
 drivers/fpga/xilinx-spi.c           | 20 ++++++--
 drivers/fpga/zynq-fpga.c            | 14 ++++--
 include/linux/fpga/fpga-bridge.h    |  9 ++--
 include/linux/fpga/fpga-mgr.h       | 10 ++--
 include/linux/fpga/fpga-region.h    | 11 ++--
 23 files changed, 429 insertions(+), 144 deletions(-)

-- 
2.17.0

^ permalink raw reply	[flat|nested] 24+ messages in thread
* [PATCH 1/4] fpga: region: don't use drvdata in common fpga code
  2018-04-27  1:26 ` Moritz Fischer
  (?)
@ 2018-04-27  1:26   ` Moritz Fischer
  -1 siblings, 0 replies; 24+ messages in thread
From: Moritz Fischer @ 2018-04-27  1:26 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-arm-kernel, linux-doc, linux-fpga, michal.simek, corbet,
	atull, Moritz Fischer

From: Alan Tull <atull@kernel.org>

Changes to fpga_region_register function to not set drvdata.

Setting drvdata is fine for DT based devices that will have one region
per platform device.  However PCIe based devices may have multiple
FPGA regions under one PCIe device.  Without these changes, the PCIe
solution has to create an extra device for each child region to hold
drvdata.

Signed-off-by: Alan Tull <atull@kernel.org>
Reported-by: Jiuyue Ma <majiuyue@huawei.com>
Signed-off-by: Moritz Fischer <mdf@kernel.org>
---
 drivers/fpga/fpga-region.c    | 1 -
 drivers/fpga/of-fpga-region.c | 1 +
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c
index cb0603e07ff8..f634a8ed5e2c 100644
--- a/drivers/fpga/fpga-region.c
+++ b/drivers/fpga/fpga-region.c
@@ -183,7 +183,6 @@ int fpga_region_register(struct device *dev, struct fpga_region *region)
 	region->dev.parent = dev;
 	region->dev.of_node = dev->of_node;
 	region->dev.id = id;
-	dev_set_drvdata(dev, region);
 
 	ret = dev_set_name(&region->dev, "region%d", id);
 	if (ret)
diff --git a/drivers/fpga/of-fpga-region.c b/drivers/fpga/of-fpga-region.c
index 119ff75522f1..35e7e8c4a0cb 100644
--- a/drivers/fpga/of-fpga-region.c
+++ b/drivers/fpga/of-fpga-region.c
@@ -438,6 +438,7 @@ static int of_fpga_region_probe(struct platform_device *pdev)
 		goto eprobe_mgr_put;
 
 	of_platform_populate(np, fpga_region_of_match, NULL, &region->dev);
+	dev_set_drvdata(dev, region);
 
 	dev_info(dev, "FPGA Region probed\n");
 
-- 
2.17.0

^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH 1/4] fpga: region: don't use drvdata in common fpga code
@ 2018-04-27  1:26   ` Moritz Fischer
  0 siblings, 0 replies; 24+ messages in thread
From: Moritz Fischer @ 2018-04-27  1:26 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-arm-kernel, linux-doc, linux-fpga, michal.simek, corbet,
	atull, Moritz Fischer

From: Alan Tull <atull@kernel.org>

Changes to fpga_region_register function to not set drvdata.

Setting drvdata is fine for DT based devices that will have one region
per platform device.  However PCIe based devices may have multiple
FPGA regions under one PCIe device.  Without these changes, the PCIe
solution has to create an extra device for each child region to hold
drvdata.

Signed-off-by: Alan Tull <atull@kernel.org>
Reported-by: Jiuyue Ma <majiuyue@huawei.com>
Signed-off-by: Moritz Fischer <mdf@kernel.org>
---
 drivers/fpga/fpga-region.c    | 1 -
 drivers/fpga/of-fpga-region.c | 1 +
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c
index cb0603e07ff8..f634a8ed5e2c 100644
--- a/drivers/fpga/fpga-region.c
+++ b/drivers/fpga/fpga-region.c
@@ -183,7 +183,6 @@ int fpga_region_register(struct device *dev, struct fpga_region *region)
 	region->dev.parent = dev;
 	region->dev.of_node = dev->of_node;
 	region->dev.id = id;
-	dev_set_drvdata(dev, region);
 
 	ret = dev_set_name(&region->dev, "region%d", id);
 	if (ret)
diff --git a/drivers/fpga/of-fpga-region.c b/drivers/fpga/of-fpga-region.c
index 119ff75522f1..35e7e8c4a0cb 100644
--- a/drivers/fpga/of-fpga-region.c
+++ b/drivers/fpga/of-fpga-region.c
@@ -438,6 +438,7 @@ static int of_fpga_region_probe(struct platform_device *pdev)
 		goto eprobe_mgr_put;
 
 	of_platform_populate(np, fpga_region_of_match, NULL, &region->dev);
+	dev_set_drvdata(dev, region);
 
 	dev_info(dev, "FPGA Region probed\n");
 
-- 
2.17.0

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH 1/4] fpga: region: don't use drvdata in common fpga code
@ 2018-04-27  1:26   ` Moritz Fischer
  0 siblings, 0 replies; 24+ messages in thread
From: Moritz Fischer @ 2018-04-27  1:26 UTC (permalink / raw)
  To: linux-arm-kernel

From: Alan Tull <atull@kernel.org>

Changes to fpga_region_register function to not set drvdata.

Setting drvdata is fine for DT based devices that will have one region
per platform device.  However PCIe based devices may have multiple
FPGA regions under one PCIe device.  Without these changes, the PCIe
solution has to create an extra device for each child region to hold
drvdata.

Signed-off-by: Alan Tull <atull@kernel.org>
Reported-by: Jiuyue Ma <majiuyue@huawei.com>
Signed-off-by: Moritz Fischer <mdf@kernel.org>
---
 drivers/fpga/fpga-region.c    | 1 -
 drivers/fpga/of-fpga-region.c | 1 +
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c
index cb0603e07ff8..f634a8ed5e2c 100644
--- a/drivers/fpga/fpga-region.c
+++ b/drivers/fpga/fpga-region.c
@@ -183,7 +183,6 @@ int fpga_region_register(struct device *dev, struct fpga_region *region)
 	region->dev.parent = dev;
 	region->dev.of_node = dev->of_node;
 	region->dev.id = id;
-	dev_set_drvdata(dev, region);
 
 	ret = dev_set_name(&region->dev, "region%d", id);
 	if (ret)
diff --git a/drivers/fpga/of-fpga-region.c b/drivers/fpga/of-fpga-region.c
index 119ff75522f1..35e7e8c4a0cb 100644
--- a/drivers/fpga/of-fpga-region.c
+++ b/drivers/fpga/of-fpga-region.c
@@ -438,6 +438,7 @@ static int of_fpga_region_probe(struct platform_device *pdev)
 		goto eprobe_mgr_put;
 
 	of_platform_populate(np, fpga_region_of_match, NULL, &region->dev);
+	dev_set_drvdata(dev, region);
 
 	dev_info(dev, "FPGA Region probed\n");
 
-- 
2.17.0

^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH 2/4] fpga: manager: change api, don't use drvdata
  2018-04-27  1:26 ` Moritz Fischer
  (?)
@ 2018-04-27  1:26   ` Moritz Fischer
  -1 siblings, 0 replies; 24+ messages in thread
From: Moritz Fischer @ 2018-04-27  1:26 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-arm-kernel, linux-doc, linux-fpga, michal.simek, corbet,
	atull, Moritz Fischer

From: Alan Tull <atull@kernel.org>

Change fpga_mgr_register to not set or use drvdata.  This supports
the case where a PCIe device has more than one manager.

Add fpga_mgr_create/free functions.  Change fpga_mgr_register and
fpga_mgr_unregister functions to take the mgr struct as their only
parameter.

  struct fpga_manager *fpga_mgr_create(struct device *dev,
                const char *name,
                const struct fpga_manager_ops *mops,
                void *priv);
  void fpga_mgr_free(struct fpga_manager *mgr);
  int fpga_mgr_register(struct fpga_manager *mgr);
  void fpga_mgr_unregister(struct fpga_manager *mgr);

Update the drivers that call fpga_mgr_register with the new API.

Signed-off-by: Alan Tull <atull@kernel.org>
[Moritz: Fixup whitespace issue]
Reported-by: Jiuyue Ma <majiuyue@huawei.com>
Signed-off-by: Moritz Fischer <mdf@kernel.org>
---
 Documentation/fpga/fpga-mgr.txt  | 35 ++++++++++----
 drivers/fpga/altera-cvp.c        | 19 ++++++--
 drivers/fpga/altera-pr-ip-core.c | 18 +++++++-
 drivers/fpga/altera-ps-spi.c     | 20 ++++++--
 drivers/fpga/fpga-mgr.c          | 78 +++++++++++++++++++++-----------
 drivers/fpga/ice40-spi.c         | 21 +++++++--
 drivers/fpga/machxo2-spi.c       | 20 ++++++--
 drivers/fpga/socfpga-a10.c       | 14 ++++--
 drivers/fpga/socfpga.c           | 19 ++++++--
 drivers/fpga/ts73xx-fpga.c       | 20 ++++++--
 drivers/fpga/xilinx-spi.c        | 20 ++++++--
 drivers/fpga/zynq-fpga.c         | 14 ++++--
 include/linux/fpga/fpga-mgr.h    | 10 ++--
 13 files changed, 237 insertions(+), 71 deletions(-)

diff --git a/Documentation/fpga/fpga-mgr.txt b/Documentation/fpga/fpga-mgr.txt
index cc6413ed6fc9..86b6df66a905 100644
--- a/Documentation/fpga/fpga-mgr.txt
+++ b/Documentation/fpga/fpga-mgr.txt
@@ -63,17 +63,23 @@ The user should call fpga_mgr_lock and verify that it returns 0 before
 attempting to program the FPGA.  Likewise, the user should call
 fpga_mgr_unlock when done programming the FPGA.
 
+To alloc/free a FPGA manager struct:
+------------------------------------
+
+	struct fpga_manager *fpga_mgr_create(struct device *dev,
+					     const char *name,
+					     const struct fpga_manager_ops *mops,
+					     void *priv);
+	void fpga_mgr_free(struct fpga_manager *mgr);
 
 To register or unregister the low level FPGA-specific driver:
 -------------------------------------------------------------
 
-	int fpga_mgr_register(struct device *dev, const char *name,
-			      const struct fpga_manager_ops *mops,
-			      void *priv);
+	int fpga_mgr_register(struct fpga_manager *mgr);
 
-	void fpga_mgr_unregister(struct device *dev);
+	void fpga_mgr_unregister(struct fpga_manager *mgr);
 
-Use of these two functions is described below in "How To Support a new FPGA
+Use of these functions is described below in "How To Support a new FPGA
 device."
 
 
@@ -148,6 +154,7 @@ static int socfpga_fpga_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct socfpga_fpga_priv *priv;
+	struct fpga_manager *mgr;
 	int ret;
 
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
@@ -157,13 +164,25 @@ static int socfpga_fpga_probe(struct platform_device *pdev)
 	/* ... do ioremaps, get interrupts, etc. and save
 	   them in priv... */
 
-	return fpga_mgr_register(dev, "Altera SOCFPGA FPGA Manager",
-				 &socfpga_fpga_ops, priv);
+	mgr = 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;
 }
 
 static int socfpga_fpga_remove(struct platform_device *pdev)
 {
-	fpga_mgr_unregister(&pdev->dev);
+	struct fpga_manager *mgr = platform_get_drvdata(pdev);
+
+	fpga_mgr_unregister(mgr);
 
 	return 0;
 }
diff --git a/drivers/fpga/altera-cvp.c b/drivers/fpga/altera-cvp.c
index 77b04e4b3254..dd4edd8f22ce 100644
--- a/drivers/fpga/altera-cvp.c
+++ b/drivers/fpga/altera-cvp.c
@@ -401,6 +401,7 @@ static int altera_cvp_probe(struct pci_dev *pdev,
 			    const struct pci_device_id *dev_id)
 {
 	struct altera_cvp_conf *conf;
+	struct fpga_manager *mgr;
 	u16 cmd, val;
 	int ret;
 
@@ -452,16 +453,24 @@ 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));
 
-	ret = fpga_mgr_register(&pdev->dev, conf->mgr_name,
-				&altera_cvp_ops, conf);
-	if (ret)
+	mgr = fpga_mgr_create(&pdev->dev, conf->mgr_name,
+			      &altera_cvp_ops, conf);
+	if (!mgr)
+		return -ENOMEM;
+
+	pci_set_drvdata(pdev, mgr);
+
+	ret = fpga_mgr_register(mgr);
+	if (ret) {
+		fpga_mgr_free(mgr);
 		goto err_unmap;
+	}
 
 	ret = driver_create_file(&altera_cvp_driver.driver,
 				 &driver_attr_chkcfg);
 	if (ret) {
 		dev_err(&pdev->dev, "Can't create sysfs chkcfg file\n");
-		fpga_mgr_unregister(&pdev->dev);
+		fpga_mgr_unregister(mgr);
 		goto err_unmap;
 	}
 
@@ -483,7 +492,7 @@ static void altera_cvp_remove(struct pci_dev *pdev)
 	u16 cmd;
 
 	driver_remove_file(&altera_cvp_driver.driver, &driver_attr_chkcfg);
-	fpga_mgr_unregister(&pdev->dev);
+	fpga_mgr_unregister(mgr);
 	pci_iounmap(pdev, conf->map);
 	pci_release_region(pdev, CVP_BAR);
 	pci_read_config_word(pdev, PCI_COMMAND, &cmd);
diff --git a/drivers/fpga/altera-pr-ip-core.c b/drivers/fpga/altera-pr-ip-core.c
index a7b31f9797ce..eea521774cf6 100644
--- a/drivers/fpga/altera-pr-ip-core.c
+++ b/drivers/fpga/altera-pr-ip-core.c
@@ -187,6 +187,8 @@ static const struct fpga_manager_ops alt_pr_ops = {
 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);
@@ -201,15 +203,27 @@ 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));
 
-	return fpga_mgr_register(dev, dev_name(dev), &alt_pr_ops, priv);
+	mgr = 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;
 }
 EXPORT_SYMBOL_GPL(alt_pr_register);
 
 int alt_pr_unregister(struct device *dev)
 {
+	struct fpga_manager *mgr = dev_get_drvdata(dev);
+
 	dev_dbg(dev, "%s\n", __func__);
 
-	fpga_mgr_unregister(dev);
+	fpga_mgr_unregister(mgr);
 
 	return 0;
 }
diff --git a/drivers/fpga/altera-ps-spi.c b/drivers/fpga/altera-ps-spi.c
index 06d212a3d49d..24b25c626036 100644
--- a/drivers/fpga/altera-ps-spi.c
+++ b/drivers/fpga/altera-ps-spi.c
@@ -238,6 +238,8 @@ 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)
@@ -273,13 +275,25 @@ 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));
 
-	return fpga_mgr_register(&spi->dev, conf->mgr_name,
-				 &altera_ps_ops, conf);
+	mgr = 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;
 }
 
 static int altera_ps_remove(struct spi_device *spi)
 {
-	fpga_mgr_unregister(&spi->dev);
+	struct fpga_manager *mgr = spi_get_drvdata(spi);
+
+	fpga_mgr_unregister(mgr);
 
 	return 0;
 }
diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c
index 9939d2cbc9a6..0a5181db3e2b 100644
--- a/drivers/fpga/fpga-mgr.c
+++ b/drivers/fpga/fpga-mgr.c
@@ -515,17 +515,17 @@ void fpga_mgr_unlock(struct fpga_manager *mgr)
 EXPORT_SYMBOL_GPL(fpga_mgr_unlock);
 
 /**
- * fpga_mgr_register - register a low level fpga manager driver
+ * fpga_mgr_create - create and initialize a 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
  *
- * Return: 0 on success, negative error code otherwise.
+ * Return: pointer to struct fpga_manager or NULL
  */
-int fpga_mgr_register(struct device *dev, const char *name,
-		      const struct fpga_manager_ops *mops,
-		      void *priv)
+struct fpga_manager *fpga_mgr_create(struct device *dev, const char *name,
+				     const struct fpga_manager_ops *mops,
+				     void *priv)
 {
 	struct fpga_manager *mgr;
 	int id, ret;
@@ -534,17 +534,17 @@ int fpga_mgr_register(struct device *dev, const char *name,
 	    !mops->write_init || (!mops->write && !mops->write_sg) ||
 	    (mops->write && mops->write_sg)) {
 		dev_err(dev, "Attempt to register without fpga_manager_ops\n");
-		return -EINVAL;
+		return NULL;
 	}
 
 	if (!name || !strlen(name)) {
 		dev_err(dev, "Attempt to register with no name!\n");
-		return -EINVAL;
+		return NULL;
 	}
 
 	mgr = kzalloc(sizeof(*mgr), GFP_KERNEL);
 	if (!mgr)
-		return -ENOMEM;
+		return NULL;
 
 	id = ida_simple_get(&fpga_mgr_ida, 0, 0, GFP_KERNEL);
 	if (id < 0) {
@@ -558,25 +558,56 @@ int fpga_mgr_register(struct device *dev, const char *name,
 	mgr->mops = mops;
 	mgr->priv = priv;
 
-	/*
-	 * Initialize framework state by requesting low level driver read state
-	 * from device.  FPGA may be in reset mode or may have been programmed
-	 * by bootloader or EEPROM.
-	 */
-	mgr->state = mgr->mops->state(mgr);
-
 	device_initialize(&mgr->dev);
 	mgr->dev.class = fpga_mgr_class;
 	mgr->dev.groups = mops->groups;
 	mgr->dev.parent = dev;
 	mgr->dev.of_node = dev->of_node;
 	mgr->dev.id = id;
-	dev_set_drvdata(dev, mgr);
 
 	ret = dev_set_name(&mgr->dev, "fpga%d", id);
 	if (ret)
 		goto error_device;
 
+	return mgr;
+
+error_device:
+	ida_simple_remove(&fpga_mgr_ida, id);
+error_kfree:
+	kfree(mgr);
+
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(fpga_mgr_create);
+
+/**
+ * fpga_mgr_free - deallocate a FPGA manager
+ * @mgr:	fpga manager struct created by fpga_mgr_create
+ */
+void fpga_mgr_free(struct fpga_manager *mgr)
+{
+	ida_simple_remove(&fpga_mgr_ida, mgr->dev.id);
+	kfree(mgr);
+}
+EXPORT_SYMBOL_GPL(fpga_mgr_free);
+
+/**
+ * fpga_mgr_register - register a FPGA manager
+ * @mgr:	fpga manager struct created by fpga_mgr_create
+ *
+ * Return: 0 on success, negative error code otherwise.
+ */
+int fpga_mgr_register(struct fpga_manager *mgr)
+{
+	int ret;
+
+	/*
+	 * Initialize framework state by requesting low level driver read state
+	 * from device.  FPGA may be in reset mode or may have been programmed
+	 * by bootloader or EEPROM.
+	 */
+	mgr->state = mgr->mops->state(mgr);
+
 	ret = device_add(&mgr->dev);
 	if (ret)
 		goto error_device;
@@ -586,22 +617,18 @@ int fpga_mgr_register(struct device *dev, const char *name,
 	return 0;
 
 error_device:
-	ida_simple_remove(&fpga_mgr_ida, id);
-error_kfree:
-	kfree(mgr);
+	ida_simple_remove(&fpga_mgr_ida, mgr->dev.id);
 
 	return ret;
 }
 EXPORT_SYMBOL_GPL(fpga_mgr_register);
 
 /**
- * fpga_mgr_unregister - unregister a low level fpga manager driver
- * @dev:	fpga manager device from pdev
+ * fpga_mgr_unregister - unregister a FPGA manager
+ * @mgr:	fpga manager struct
  */
-void fpga_mgr_unregister(struct device *dev)
+void fpga_mgr_unregister(struct fpga_manager *mgr)
 {
-	struct fpga_manager *mgr = dev_get_drvdata(dev);
-
 	dev_info(&mgr->dev, "%s %s\n", __func__, mgr->name);
 
 	/*
@@ -619,8 +646,7 @@ static void fpga_mgr_dev_release(struct device *dev)
 {
 	struct fpga_manager *mgr = to_fpga_manager(dev);
 
-	ida_simple_remove(&fpga_mgr_ida, mgr->dev.id);
-	kfree(mgr);
+	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 7fca82023062..5981c7ee7a7d 100644
--- a/drivers/fpga/ice40-spi.c
+++ b/drivers/fpga/ice40-spi.c
@@ -133,6 +133,7 @@ static int ice40_fpga_probe(struct spi_device *spi)
 {
 	struct device *dev = &spi->dev;
 	struct ice40_fpga_priv *priv;
+	struct fpga_manager *mgr;
 	int ret;
 
 	priv = devm_kzalloc(&spi->dev, sizeof(*priv), GFP_KERNEL);
@@ -174,14 +175,26 @@ static int ice40_fpga_probe(struct spi_device *spi)
 		return ret;
 	}
 
-	/* Register with the FPGA manager */
-	return fpga_mgr_register(dev, "Lattice iCE40 FPGA Manager",
-				 &ice40_fpga_ops, priv);
+	mgr = 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;
 }
 
 static int ice40_fpga_remove(struct spi_device *spi)
 {
-	fpga_mgr_unregister(&spi->dev);
+	struct fpga_manager *mgr = spi_get_drvdata(spi);
+
+	fpga_mgr_unregister(mgr);
+
 	return 0;
 }
 
diff --git a/drivers/fpga/machxo2-spi.c b/drivers/fpga/machxo2-spi.c
index 8e95ec9c5c9a..a582e0000c97 100644
--- a/drivers/fpga/machxo2-spi.c
+++ b/drivers/fpga/machxo2-spi.c
@@ -355,21 +355,33 @@ static const struct fpga_manager_ops machxo2_ops = {
 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;
 	}
 
-	return fpga_mgr_register(dev, "Lattice MachXO2 SPI FPGA Manager",
-				 &machxo2_ops, spi);
+	mgr = 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;
 }
 
 static int machxo2_spi_remove(struct spi_device *spi)
 {
-	struct device *dev = &spi->dev;
+	struct fpga_manager *mgr = spi_get_drvdata(spi);
 
-	fpga_mgr_unregister(dev);
+	fpga_mgr_unregister(mgr);
 
 	return 0;
 }
diff --git a/drivers/fpga/socfpga-a10.c b/drivers/fpga/socfpga-a10.c
index a46e343a5b72..dec3db5cdab1 100644
--- a/drivers/fpga/socfpga-a10.c
+++ b/drivers/fpga/socfpga-a10.c
@@ -482,6 +482,7 @@ static int socfpga_a10_fpga_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct a10_fpga_priv *priv;
 	void __iomem *reg_base;
+	struct fpga_manager *mgr;
 	struct resource *res;
 	int ret;
 
@@ -519,9 +520,16 @@ static int socfpga_a10_fpga_probe(struct platform_device *pdev)
 		return -EBUSY;
 	}
 
-	ret = fpga_mgr_register(dev, "SoCFPGA Arria10 FPGA Manager",
-				 &socfpga_a10_fpga_mgr_ops, priv);
+	mgr = fpga_mgr_create(dev, "SoCFPGA Arria10 FPGA Manager",
+			      &socfpga_a10_fpga_mgr_ops, priv);
+	if (!mgr)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, mgr);
+
+	ret = fpga_mgr_register(mgr);
 	if (ret) {
+		fpga_mgr_free(mgr);
 		clk_disable_unprepare(priv->clk);
 		return ret;
 	}
@@ -534,7 +542,7 @@ static int socfpga_a10_fpga_remove(struct platform_device *pdev)
 	struct fpga_manager *mgr = platform_get_drvdata(pdev);
 	struct a10_fpga_priv *priv = mgr->priv;
 
-	fpga_mgr_unregister(&pdev->dev);
+	fpga_mgr_unregister(mgr);
 	clk_disable_unprepare(priv->clk);
 
 	return 0;
diff --git a/drivers/fpga/socfpga.c b/drivers/fpga/socfpga.c
index b6672e66cda6..51efaf9e0e03 100644
--- a/drivers/fpga/socfpga.c
+++ b/drivers/fpga/socfpga.c
@@ -555,6 +555,7 @@ static int socfpga_fpga_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct socfpga_fpga_priv *priv;
+	struct fpga_manager *mgr;
 	struct resource *res;
 	int ret;
 
@@ -581,13 +582,25 @@ static int socfpga_fpga_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	return fpga_mgr_register(dev, "Altera SOCFPGA FPGA Manager",
-				 &socfpga_fpga_ops, priv);
+	mgr = 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;
 }
 
 static int socfpga_fpga_remove(struct platform_device *pdev)
 {
-	fpga_mgr_unregister(&pdev->dev);
+	struct fpga_manager *mgr = platform_get_drvdata(pdev);
+
+	fpga_mgr_unregister(mgr);
 
 	return 0;
 }
diff --git a/drivers/fpga/ts73xx-fpga.c b/drivers/fpga/ts73xx-fpga.c
index f6a96b42e2ca..08efd1895b1b 100644
--- a/drivers/fpga/ts73xx-fpga.c
+++ b/drivers/fpga/ts73xx-fpga.c
@@ -116,7 +116,9 @@ static int ts73xx_fpga_probe(struct platform_device *pdev)
 {
 	struct device *kdev = &pdev->dev;
 	struct ts73xx_fpga_priv *priv;
+	struct fpga_manager *mgr;
 	struct resource *res;
+	int ret;
 
 	priv = devm_kzalloc(kdev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
@@ -131,13 +133,25 @@ static int ts73xx_fpga_probe(struct platform_device *pdev)
 		return PTR_ERR(priv->io_base);
 	}
 
-	return fpga_mgr_register(kdev, "TS-73xx FPGA Manager",
-				 &ts73xx_fpga_ops, priv);
+	mgr = 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;
 }
 
 static int ts73xx_fpga_remove(struct platform_device *pdev)
 {
-	fpga_mgr_unregister(&pdev->dev);
+	struct fpga_manager *mgr = platform_get_drvdata(pdev);
+
+	fpga_mgr_unregister(mgr);
 
 	return 0;
 }
diff --git a/drivers/fpga/xilinx-spi.c b/drivers/fpga/xilinx-spi.c
index 9b62a4c2a3df..8d1945966533 100644
--- a/drivers/fpga/xilinx-spi.c
+++ b/drivers/fpga/xilinx-spi.c
@@ -143,6 +143,8 @@ static const struct fpga_manager_ops xilinx_spi_ops = {
 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)
@@ -165,13 +167,25 @@ static int xilinx_spi_probe(struct spi_device *spi)
 		return PTR_ERR(conf->done);
 	}
 
-	return fpga_mgr_register(&spi->dev, "Xilinx Slave Serial FPGA Manager",
-				 &xilinx_spi_ops, conf);
+	mgr = 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;
 }
 
 static int xilinx_spi_remove(struct spi_device *spi)
 {
-	fpga_mgr_unregister(&spi->dev);
+	struct fpga_manager *mgr = spi_get_drvdata(spi);
+
+	fpga_mgr_unregister(mgr);
 
 	return 0;
 }
diff --git a/drivers/fpga/zynq-fpga.c b/drivers/fpga/zynq-fpga.c
index 70b15b303471..3110e00121ca 100644
--- a/drivers/fpga/zynq-fpga.c
+++ b/drivers/fpga/zynq-fpga.c
@@ -558,6 +558,7 @@ static int zynq_fpga_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct zynq_fpga_priv *priv;
+	struct fpga_manager *mgr;
 	struct resource *res;
 	int err;
 
@@ -613,10 +614,17 @@ static int zynq_fpga_probe(struct platform_device *pdev)
 
 	clk_disable(priv->clk);
 
-	err = fpga_mgr_register(dev, "Xilinx Zynq FPGA Manager",
-				&zynq_fpga_ops, priv);
+	mgr = fpga_mgr_create(dev, "Xilinx Zynq FPGA Manager",
+			      &zynq_fpga_ops, priv);
+	if (!mgr)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, mgr);
+
+	err = fpga_mgr_register(mgr);
 	if (err) {
 		dev_err(dev, "unable to register FPGA manager\n");
+		fpga_mgr_free(mgr);
 		clk_unprepare(priv->clk);
 		return err;
 	}
@@ -632,7 +640,7 @@ static int zynq_fpga_remove(struct platform_device *pdev)
 	mgr = platform_get_drvdata(pdev);
 	priv = mgr->priv;
 
-	fpga_mgr_unregister(&pdev->dev);
+	fpga_mgr_unregister(mgr);
 
 	clk_unprepare(priv->clk);
 
diff --git a/include/linux/fpga/fpga-mgr.h b/include/linux/fpga/fpga-mgr.h
index 3c6de23aabdf..1266c1108e70 100644
--- a/include/linux/fpga/fpga-mgr.h
+++ b/include/linux/fpga/fpga-mgr.h
@@ -170,9 +170,11 @@ struct fpga_manager *fpga_mgr_get(struct device *dev);
 
 void fpga_mgr_put(struct fpga_manager *mgr);
 
-int fpga_mgr_register(struct device *dev, const char *name,
-		      const struct fpga_manager_ops *mops, void *priv);
-
-void fpga_mgr_unregister(struct device *dev);
+struct fpga_manager *fpga_mgr_create(struct device *dev, const char *name,
+				     const struct fpga_manager_ops *mops,
+				     void *priv);
+void fpga_mgr_free(struct fpga_manager *mgr);
+int fpga_mgr_register(struct fpga_manager *mgr);
+void fpga_mgr_unregister(struct fpga_manager *mgr);
 
 #endif /*_LINUX_FPGA_MGR_H */
-- 
2.17.0

^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH 2/4] fpga: manager: change api, don't use drvdata
@ 2018-04-27  1:26   ` Moritz Fischer
  0 siblings, 0 replies; 24+ messages in thread
From: Moritz Fischer @ 2018-04-27  1:26 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-arm-kernel, linux-doc, linux-fpga, michal.simek, corbet,
	atull, Moritz Fischer

From: Alan Tull <atull@kernel.org>

Change fpga_mgr_register to not set or use drvdata.  This supports
the case where a PCIe device has more than one manager.

Add fpga_mgr_create/free functions.  Change fpga_mgr_register and
fpga_mgr_unregister functions to take the mgr struct as their only
parameter.

  struct fpga_manager *fpga_mgr_create(struct device *dev,
                const char *name,
                const struct fpga_manager_ops *mops,
                void *priv);
  void fpga_mgr_free(struct fpga_manager *mgr);
  int fpga_mgr_register(struct fpga_manager *mgr);
  void fpga_mgr_unregister(struct fpga_manager *mgr);

Update the drivers that call fpga_mgr_register with the new API.

Signed-off-by: Alan Tull <atull@kernel.org>
[Moritz: Fixup whitespace issue]
Reported-by: Jiuyue Ma <majiuyue@huawei.com>
Signed-off-by: Moritz Fischer <mdf@kernel.org>
---
 Documentation/fpga/fpga-mgr.txt  | 35 ++++++++++----
 drivers/fpga/altera-cvp.c        | 19 ++++++--
 drivers/fpga/altera-pr-ip-core.c | 18 +++++++-
 drivers/fpga/altera-ps-spi.c     | 20 ++++++--
 drivers/fpga/fpga-mgr.c          | 78 +++++++++++++++++++++-----------
 drivers/fpga/ice40-spi.c         | 21 +++++++--
 drivers/fpga/machxo2-spi.c       | 20 ++++++--
 drivers/fpga/socfpga-a10.c       | 14 ++++--
 drivers/fpga/socfpga.c           | 19 ++++++--
 drivers/fpga/ts73xx-fpga.c       | 20 ++++++--
 drivers/fpga/xilinx-spi.c        | 20 ++++++--
 drivers/fpga/zynq-fpga.c         | 14 ++++--
 include/linux/fpga/fpga-mgr.h    | 10 ++--
 13 files changed, 237 insertions(+), 71 deletions(-)

diff --git a/Documentation/fpga/fpga-mgr.txt b/Documentation/fpga/fpga-mgr.txt
index cc6413ed6fc9..86b6df66a905 100644
--- a/Documentation/fpga/fpga-mgr.txt
+++ b/Documentation/fpga/fpga-mgr.txt
@@ -63,17 +63,23 @@ The user should call fpga_mgr_lock and verify that it returns 0 before
 attempting to program the FPGA.  Likewise, the user should call
 fpga_mgr_unlock when done programming the FPGA.
 
+To alloc/free a FPGA manager struct:
+------------------------------------
+
+	struct fpga_manager *fpga_mgr_create(struct device *dev,
+					     const char *name,
+					     const struct fpga_manager_ops *mops,
+					     void *priv);
+	void fpga_mgr_free(struct fpga_manager *mgr);
 
 To register or unregister the low level FPGA-specific driver:
 -------------------------------------------------------------
 
-	int fpga_mgr_register(struct device *dev, const char *name,
-			      const struct fpga_manager_ops *mops,
-			      void *priv);
+	int fpga_mgr_register(struct fpga_manager *mgr);
 
-	void fpga_mgr_unregister(struct device *dev);
+	void fpga_mgr_unregister(struct fpga_manager *mgr);
 
-Use of these two functions is described below in "How To Support a new FPGA
+Use of these functions is described below in "How To Support a new FPGA
 device."
 
 
@@ -148,6 +154,7 @@ static int socfpga_fpga_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct socfpga_fpga_priv *priv;
+	struct fpga_manager *mgr;
 	int ret;
 
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
@@ -157,13 +164,25 @@ static int socfpga_fpga_probe(struct platform_device *pdev)
 	/* ... do ioremaps, get interrupts, etc. and save
 	   them in priv... */
 
-	return fpga_mgr_register(dev, "Altera SOCFPGA FPGA Manager",
-				 &socfpga_fpga_ops, priv);
+	mgr = 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;
 }
 
 static int socfpga_fpga_remove(struct platform_device *pdev)
 {
-	fpga_mgr_unregister(&pdev->dev);
+	struct fpga_manager *mgr = platform_get_drvdata(pdev);
+
+	fpga_mgr_unregister(mgr);
 
 	return 0;
 }
diff --git a/drivers/fpga/altera-cvp.c b/drivers/fpga/altera-cvp.c
index 77b04e4b3254..dd4edd8f22ce 100644
--- a/drivers/fpga/altera-cvp.c
+++ b/drivers/fpga/altera-cvp.c
@@ -401,6 +401,7 @@ static int altera_cvp_probe(struct pci_dev *pdev,
 			    const struct pci_device_id *dev_id)
 {
 	struct altera_cvp_conf *conf;
+	struct fpga_manager *mgr;
 	u16 cmd, val;
 	int ret;
 
@@ -452,16 +453,24 @@ 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));
 
-	ret = fpga_mgr_register(&pdev->dev, conf->mgr_name,
-				&altera_cvp_ops, conf);
-	if (ret)
+	mgr = fpga_mgr_create(&pdev->dev, conf->mgr_name,
+			      &altera_cvp_ops, conf);
+	if (!mgr)
+		return -ENOMEM;
+
+	pci_set_drvdata(pdev, mgr);
+
+	ret = fpga_mgr_register(mgr);
+	if (ret) {
+		fpga_mgr_free(mgr);
 		goto err_unmap;
+	}
 
 	ret = driver_create_file(&altera_cvp_driver.driver,
 				 &driver_attr_chkcfg);
 	if (ret) {
 		dev_err(&pdev->dev, "Can't create sysfs chkcfg file\n");
-		fpga_mgr_unregister(&pdev->dev);
+		fpga_mgr_unregister(mgr);
 		goto err_unmap;
 	}
 
@@ -483,7 +492,7 @@ static void altera_cvp_remove(struct pci_dev *pdev)
 	u16 cmd;
 
 	driver_remove_file(&altera_cvp_driver.driver, &driver_attr_chkcfg);
-	fpga_mgr_unregister(&pdev->dev);
+	fpga_mgr_unregister(mgr);
 	pci_iounmap(pdev, conf->map);
 	pci_release_region(pdev, CVP_BAR);
 	pci_read_config_word(pdev, PCI_COMMAND, &cmd);
diff --git a/drivers/fpga/altera-pr-ip-core.c b/drivers/fpga/altera-pr-ip-core.c
index a7b31f9797ce..eea521774cf6 100644
--- a/drivers/fpga/altera-pr-ip-core.c
+++ b/drivers/fpga/altera-pr-ip-core.c
@@ -187,6 +187,8 @@ static const struct fpga_manager_ops alt_pr_ops = {
 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);
@@ -201,15 +203,27 @@ 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));
 
-	return fpga_mgr_register(dev, dev_name(dev), &alt_pr_ops, priv);
+	mgr = 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;
 }
 EXPORT_SYMBOL_GPL(alt_pr_register);
 
 int alt_pr_unregister(struct device *dev)
 {
+	struct fpga_manager *mgr = dev_get_drvdata(dev);
+
 	dev_dbg(dev, "%s\n", __func__);
 
-	fpga_mgr_unregister(dev);
+	fpga_mgr_unregister(mgr);
 
 	return 0;
 }
diff --git a/drivers/fpga/altera-ps-spi.c b/drivers/fpga/altera-ps-spi.c
index 06d212a3d49d..24b25c626036 100644
--- a/drivers/fpga/altera-ps-spi.c
+++ b/drivers/fpga/altera-ps-spi.c
@@ -238,6 +238,8 @@ 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)
@@ -273,13 +275,25 @@ 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));
 
-	return fpga_mgr_register(&spi->dev, conf->mgr_name,
-				 &altera_ps_ops, conf);
+	mgr = 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;
 }
 
 static int altera_ps_remove(struct spi_device *spi)
 {
-	fpga_mgr_unregister(&spi->dev);
+	struct fpga_manager *mgr = spi_get_drvdata(spi);
+
+	fpga_mgr_unregister(mgr);
 
 	return 0;
 }
diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c
index 9939d2cbc9a6..0a5181db3e2b 100644
--- a/drivers/fpga/fpga-mgr.c
+++ b/drivers/fpga/fpga-mgr.c
@@ -515,17 +515,17 @@ void fpga_mgr_unlock(struct fpga_manager *mgr)
 EXPORT_SYMBOL_GPL(fpga_mgr_unlock);
 
 /**
- * fpga_mgr_register - register a low level fpga manager driver
+ * fpga_mgr_create - create and initialize a 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
  *
- * Return: 0 on success, negative error code otherwise.
+ * Return: pointer to struct fpga_manager or NULL
  */
-int fpga_mgr_register(struct device *dev, const char *name,
-		      const struct fpga_manager_ops *mops,
-		      void *priv)
+struct fpga_manager *fpga_mgr_create(struct device *dev, const char *name,
+				     const struct fpga_manager_ops *mops,
+				     void *priv)
 {
 	struct fpga_manager *mgr;
 	int id, ret;
@@ -534,17 +534,17 @@ int fpga_mgr_register(struct device *dev, const char *name,
 	    !mops->write_init || (!mops->write && !mops->write_sg) ||
 	    (mops->write && mops->write_sg)) {
 		dev_err(dev, "Attempt to register without fpga_manager_ops\n");
-		return -EINVAL;
+		return NULL;
 	}
 
 	if (!name || !strlen(name)) {
 		dev_err(dev, "Attempt to register with no name!\n");
-		return -EINVAL;
+		return NULL;
 	}
 
 	mgr = kzalloc(sizeof(*mgr), GFP_KERNEL);
 	if (!mgr)
-		return -ENOMEM;
+		return NULL;
 
 	id = ida_simple_get(&fpga_mgr_ida, 0, 0, GFP_KERNEL);
 	if (id < 0) {
@@ -558,25 +558,56 @@ int fpga_mgr_register(struct device *dev, const char *name,
 	mgr->mops = mops;
 	mgr->priv = priv;
 
-	/*
-	 * Initialize framework state by requesting low level driver read state
-	 * from device.  FPGA may be in reset mode or may have been programmed
-	 * by bootloader or EEPROM.
-	 */
-	mgr->state = mgr->mops->state(mgr);
-
 	device_initialize(&mgr->dev);
 	mgr->dev.class = fpga_mgr_class;
 	mgr->dev.groups = mops->groups;
 	mgr->dev.parent = dev;
 	mgr->dev.of_node = dev->of_node;
 	mgr->dev.id = id;
-	dev_set_drvdata(dev, mgr);
 
 	ret = dev_set_name(&mgr->dev, "fpga%d", id);
 	if (ret)
 		goto error_device;
 
+	return mgr;
+
+error_device:
+	ida_simple_remove(&fpga_mgr_ida, id);
+error_kfree:
+	kfree(mgr);
+
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(fpga_mgr_create);
+
+/**
+ * fpga_mgr_free - deallocate a FPGA manager
+ * @mgr:	fpga manager struct created by fpga_mgr_create
+ */
+void fpga_mgr_free(struct fpga_manager *mgr)
+{
+	ida_simple_remove(&fpga_mgr_ida, mgr->dev.id);
+	kfree(mgr);
+}
+EXPORT_SYMBOL_GPL(fpga_mgr_free);
+
+/**
+ * fpga_mgr_register - register a FPGA manager
+ * @mgr:	fpga manager struct created by fpga_mgr_create
+ *
+ * Return: 0 on success, negative error code otherwise.
+ */
+int fpga_mgr_register(struct fpga_manager *mgr)
+{
+	int ret;
+
+	/*
+	 * Initialize framework state by requesting low level driver read state
+	 * from device.  FPGA may be in reset mode or may have been programmed
+	 * by bootloader or EEPROM.
+	 */
+	mgr->state = mgr->mops->state(mgr);
+
 	ret = device_add(&mgr->dev);
 	if (ret)
 		goto error_device;
@@ -586,22 +617,18 @@ int fpga_mgr_register(struct device *dev, const char *name,
 	return 0;
 
 error_device:
-	ida_simple_remove(&fpga_mgr_ida, id);
-error_kfree:
-	kfree(mgr);
+	ida_simple_remove(&fpga_mgr_ida, mgr->dev.id);
 
 	return ret;
 }
 EXPORT_SYMBOL_GPL(fpga_mgr_register);
 
 /**
- * fpga_mgr_unregister - unregister a low level fpga manager driver
- * @dev:	fpga manager device from pdev
+ * fpga_mgr_unregister - unregister a FPGA manager
+ * @mgr:	fpga manager struct
  */
-void fpga_mgr_unregister(struct device *dev)
+void fpga_mgr_unregister(struct fpga_manager *mgr)
 {
-	struct fpga_manager *mgr = dev_get_drvdata(dev);
-
 	dev_info(&mgr->dev, "%s %s\n", __func__, mgr->name);
 
 	/*
@@ -619,8 +646,7 @@ static void fpga_mgr_dev_release(struct device *dev)
 {
 	struct fpga_manager *mgr = to_fpga_manager(dev);
 
-	ida_simple_remove(&fpga_mgr_ida, mgr->dev.id);
-	kfree(mgr);
+	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 7fca82023062..5981c7ee7a7d 100644
--- a/drivers/fpga/ice40-spi.c
+++ b/drivers/fpga/ice40-spi.c
@@ -133,6 +133,7 @@ static int ice40_fpga_probe(struct spi_device *spi)
 {
 	struct device *dev = &spi->dev;
 	struct ice40_fpga_priv *priv;
+	struct fpga_manager *mgr;
 	int ret;
 
 	priv = devm_kzalloc(&spi->dev, sizeof(*priv), GFP_KERNEL);
@@ -174,14 +175,26 @@ static int ice40_fpga_probe(struct spi_device *spi)
 		return ret;
 	}
 
-	/* Register with the FPGA manager */
-	return fpga_mgr_register(dev, "Lattice iCE40 FPGA Manager",
-				 &ice40_fpga_ops, priv);
+	mgr = 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;
 }
 
 static int ice40_fpga_remove(struct spi_device *spi)
 {
-	fpga_mgr_unregister(&spi->dev);
+	struct fpga_manager *mgr = spi_get_drvdata(spi);
+
+	fpga_mgr_unregister(mgr);
+
 	return 0;
 }
 
diff --git a/drivers/fpga/machxo2-spi.c b/drivers/fpga/machxo2-spi.c
index 8e95ec9c5c9a..a582e0000c97 100644
--- a/drivers/fpga/machxo2-spi.c
+++ b/drivers/fpga/machxo2-spi.c
@@ -355,21 +355,33 @@ static const struct fpga_manager_ops machxo2_ops = {
 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;
 	}
 
-	return fpga_mgr_register(dev, "Lattice MachXO2 SPI FPGA Manager",
-				 &machxo2_ops, spi);
+	mgr = 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;
 }
 
 static int machxo2_spi_remove(struct spi_device *spi)
 {
-	struct device *dev = &spi->dev;
+	struct fpga_manager *mgr = spi_get_drvdata(spi);
 
-	fpga_mgr_unregister(dev);
+	fpga_mgr_unregister(mgr);
 
 	return 0;
 }
diff --git a/drivers/fpga/socfpga-a10.c b/drivers/fpga/socfpga-a10.c
index a46e343a5b72..dec3db5cdab1 100644
--- a/drivers/fpga/socfpga-a10.c
+++ b/drivers/fpga/socfpga-a10.c
@@ -482,6 +482,7 @@ static int socfpga_a10_fpga_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct a10_fpga_priv *priv;
 	void __iomem *reg_base;
+	struct fpga_manager *mgr;
 	struct resource *res;
 	int ret;
 
@@ -519,9 +520,16 @@ static int socfpga_a10_fpga_probe(struct platform_device *pdev)
 		return -EBUSY;
 	}
 
-	ret = fpga_mgr_register(dev, "SoCFPGA Arria10 FPGA Manager",
-				 &socfpga_a10_fpga_mgr_ops, priv);
+	mgr = fpga_mgr_create(dev, "SoCFPGA Arria10 FPGA Manager",
+			      &socfpga_a10_fpga_mgr_ops, priv);
+	if (!mgr)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, mgr);
+
+	ret = fpga_mgr_register(mgr);
 	if (ret) {
+		fpga_mgr_free(mgr);
 		clk_disable_unprepare(priv->clk);
 		return ret;
 	}
@@ -534,7 +542,7 @@ static int socfpga_a10_fpga_remove(struct platform_device *pdev)
 	struct fpga_manager *mgr = platform_get_drvdata(pdev);
 	struct a10_fpga_priv *priv = mgr->priv;
 
-	fpga_mgr_unregister(&pdev->dev);
+	fpga_mgr_unregister(mgr);
 	clk_disable_unprepare(priv->clk);
 
 	return 0;
diff --git a/drivers/fpga/socfpga.c b/drivers/fpga/socfpga.c
index b6672e66cda6..51efaf9e0e03 100644
--- a/drivers/fpga/socfpga.c
+++ b/drivers/fpga/socfpga.c
@@ -555,6 +555,7 @@ static int socfpga_fpga_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct socfpga_fpga_priv *priv;
+	struct fpga_manager *mgr;
 	struct resource *res;
 	int ret;
 
@@ -581,13 +582,25 @@ static int socfpga_fpga_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	return fpga_mgr_register(dev, "Altera SOCFPGA FPGA Manager",
-				 &socfpga_fpga_ops, priv);
+	mgr = 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;
 }
 
 static int socfpga_fpga_remove(struct platform_device *pdev)
 {
-	fpga_mgr_unregister(&pdev->dev);
+	struct fpga_manager *mgr = platform_get_drvdata(pdev);
+
+	fpga_mgr_unregister(mgr);
 
 	return 0;
 }
diff --git a/drivers/fpga/ts73xx-fpga.c b/drivers/fpga/ts73xx-fpga.c
index f6a96b42e2ca..08efd1895b1b 100644
--- a/drivers/fpga/ts73xx-fpga.c
+++ b/drivers/fpga/ts73xx-fpga.c
@@ -116,7 +116,9 @@ static int ts73xx_fpga_probe(struct platform_device *pdev)
 {
 	struct device *kdev = &pdev->dev;
 	struct ts73xx_fpga_priv *priv;
+	struct fpga_manager *mgr;
 	struct resource *res;
+	int ret;
 
 	priv = devm_kzalloc(kdev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
@@ -131,13 +133,25 @@ static int ts73xx_fpga_probe(struct platform_device *pdev)
 		return PTR_ERR(priv->io_base);
 	}
 
-	return fpga_mgr_register(kdev, "TS-73xx FPGA Manager",
-				 &ts73xx_fpga_ops, priv);
+	mgr = 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;
 }
 
 static int ts73xx_fpga_remove(struct platform_device *pdev)
 {
-	fpga_mgr_unregister(&pdev->dev);
+	struct fpga_manager *mgr = platform_get_drvdata(pdev);
+
+	fpga_mgr_unregister(mgr);
 
 	return 0;
 }
diff --git a/drivers/fpga/xilinx-spi.c b/drivers/fpga/xilinx-spi.c
index 9b62a4c2a3df..8d1945966533 100644
--- a/drivers/fpga/xilinx-spi.c
+++ b/drivers/fpga/xilinx-spi.c
@@ -143,6 +143,8 @@ static const struct fpga_manager_ops xilinx_spi_ops = {
 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)
@@ -165,13 +167,25 @@ static int xilinx_spi_probe(struct spi_device *spi)
 		return PTR_ERR(conf->done);
 	}
 
-	return fpga_mgr_register(&spi->dev, "Xilinx Slave Serial FPGA Manager",
-				 &xilinx_spi_ops, conf);
+	mgr = 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;
 }
 
 static int xilinx_spi_remove(struct spi_device *spi)
 {
-	fpga_mgr_unregister(&spi->dev);
+	struct fpga_manager *mgr = spi_get_drvdata(spi);
+
+	fpga_mgr_unregister(mgr);
 
 	return 0;
 }
diff --git a/drivers/fpga/zynq-fpga.c b/drivers/fpga/zynq-fpga.c
index 70b15b303471..3110e00121ca 100644
--- a/drivers/fpga/zynq-fpga.c
+++ b/drivers/fpga/zynq-fpga.c
@@ -558,6 +558,7 @@ static int zynq_fpga_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct zynq_fpga_priv *priv;
+	struct fpga_manager *mgr;
 	struct resource *res;
 	int err;
 
@@ -613,10 +614,17 @@ static int zynq_fpga_probe(struct platform_device *pdev)
 
 	clk_disable(priv->clk);
 
-	err = fpga_mgr_register(dev, "Xilinx Zynq FPGA Manager",
-				&zynq_fpga_ops, priv);
+	mgr = fpga_mgr_create(dev, "Xilinx Zynq FPGA Manager",
+			      &zynq_fpga_ops, priv);
+	if (!mgr)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, mgr);
+
+	err = fpga_mgr_register(mgr);
 	if (err) {
 		dev_err(dev, "unable to register FPGA manager\n");
+		fpga_mgr_free(mgr);
 		clk_unprepare(priv->clk);
 		return err;
 	}
@@ -632,7 +640,7 @@ static int zynq_fpga_remove(struct platform_device *pdev)
 	mgr = platform_get_drvdata(pdev);
 	priv = mgr->priv;
 
-	fpga_mgr_unregister(&pdev->dev);
+	fpga_mgr_unregister(mgr);
 
 	clk_unprepare(priv->clk);
 
diff --git a/include/linux/fpga/fpga-mgr.h b/include/linux/fpga/fpga-mgr.h
index 3c6de23aabdf..1266c1108e70 100644
--- a/include/linux/fpga/fpga-mgr.h
+++ b/include/linux/fpga/fpga-mgr.h
@@ -170,9 +170,11 @@ struct fpga_manager *fpga_mgr_get(struct device *dev);
 
 void fpga_mgr_put(struct fpga_manager *mgr);
 
-int fpga_mgr_register(struct device *dev, const char *name,
-		      const struct fpga_manager_ops *mops, void *priv);
-
-void fpga_mgr_unregister(struct device *dev);
+struct fpga_manager *fpga_mgr_create(struct device *dev, const char *name,
+				     const struct fpga_manager_ops *mops,
+				     void *priv);
+void fpga_mgr_free(struct fpga_manager *mgr);
+int fpga_mgr_register(struct fpga_manager *mgr);
+void fpga_mgr_unregister(struct fpga_manager *mgr);
 
 #endif /*_LINUX_FPGA_MGR_H */
-- 
2.17.0

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH 2/4] fpga: manager: change api, don't use drvdata
@ 2018-04-27  1:26   ` Moritz Fischer
  0 siblings, 0 replies; 24+ messages in thread
From: Moritz Fischer @ 2018-04-27  1:26 UTC (permalink / raw)
  To: linux-arm-kernel

From: Alan Tull <atull@kernel.org>

Change fpga_mgr_register to not set or use drvdata.  This supports
the case where a PCIe device has more than one manager.

Add fpga_mgr_create/free functions.  Change fpga_mgr_register and
fpga_mgr_unregister functions to take the mgr struct as their only
parameter.

  struct fpga_manager *fpga_mgr_create(struct device *dev,
                const char *name,
                const struct fpga_manager_ops *mops,
                void *priv);
  void fpga_mgr_free(struct fpga_manager *mgr);
  int fpga_mgr_register(struct fpga_manager *mgr);
  void fpga_mgr_unregister(struct fpga_manager *mgr);

Update the drivers that call fpga_mgr_register with the new API.

Signed-off-by: Alan Tull <atull@kernel.org>
[Moritz: Fixup whitespace issue]
Reported-by: Jiuyue Ma <majiuyue@huawei.com>
Signed-off-by: Moritz Fischer <mdf@kernel.org>
---
 Documentation/fpga/fpga-mgr.txt  | 35 ++++++++++----
 drivers/fpga/altera-cvp.c        | 19 ++++++--
 drivers/fpga/altera-pr-ip-core.c | 18 +++++++-
 drivers/fpga/altera-ps-spi.c     | 20 ++++++--
 drivers/fpga/fpga-mgr.c          | 78 +++++++++++++++++++++-----------
 drivers/fpga/ice40-spi.c         | 21 +++++++--
 drivers/fpga/machxo2-spi.c       | 20 ++++++--
 drivers/fpga/socfpga-a10.c       | 14 ++++--
 drivers/fpga/socfpga.c           | 19 ++++++--
 drivers/fpga/ts73xx-fpga.c       | 20 ++++++--
 drivers/fpga/xilinx-spi.c        | 20 ++++++--
 drivers/fpga/zynq-fpga.c         | 14 ++++--
 include/linux/fpga/fpga-mgr.h    | 10 ++--
 13 files changed, 237 insertions(+), 71 deletions(-)

diff --git a/Documentation/fpga/fpga-mgr.txt b/Documentation/fpga/fpga-mgr.txt
index cc6413ed6fc9..86b6df66a905 100644
--- a/Documentation/fpga/fpga-mgr.txt
+++ b/Documentation/fpga/fpga-mgr.txt
@@ -63,17 +63,23 @@ The user should call fpga_mgr_lock and verify that it returns 0 before
 attempting to program the FPGA.  Likewise, the user should call
 fpga_mgr_unlock when done programming the FPGA.
 
+To alloc/free a FPGA manager struct:
+------------------------------------
+
+	struct fpga_manager *fpga_mgr_create(struct device *dev,
+					     const char *name,
+					     const struct fpga_manager_ops *mops,
+					     void *priv);
+	void fpga_mgr_free(struct fpga_manager *mgr);
 
 To register or unregister the low level FPGA-specific driver:
 -------------------------------------------------------------
 
-	int fpga_mgr_register(struct device *dev, const char *name,
-			      const struct fpga_manager_ops *mops,
-			      void *priv);
+	int fpga_mgr_register(struct fpga_manager *mgr);
 
-	void fpga_mgr_unregister(struct device *dev);
+	void fpga_mgr_unregister(struct fpga_manager *mgr);
 
-Use of these two functions is described below in "How To Support a new FPGA
+Use of these functions is described below in "How To Support a new FPGA
 device."
 
 
@@ -148,6 +154,7 @@ static int socfpga_fpga_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct socfpga_fpga_priv *priv;
+	struct fpga_manager *mgr;
 	int ret;
 
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
@@ -157,13 +164,25 @@ static int socfpga_fpga_probe(struct platform_device *pdev)
 	/* ... do ioremaps, get interrupts, etc. and save
 	   them in priv... */
 
-	return fpga_mgr_register(dev, "Altera SOCFPGA FPGA Manager",
-				 &socfpga_fpga_ops, priv);
+	mgr = 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;
 }
 
 static int socfpga_fpga_remove(struct platform_device *pdev)
 {
-	fpga_mgr_unregister(&pdev->dev);
+	struct fpga_manager *mgr = platform_get_drvdata(pdev);
+
+	fpga_mgr_unregister(mgr);
 
 	return 0;
 }
diff --git a/drivers/fpga/altera-cvp.c b/drivers/fpga/altera-cvp.c
index 77b04e4b3254..dd4edd8f22ce 100644
--- a/drivers/fpga/altera-cvp.c
+++ b/drivers/fpga/altera-cvp.c
@@ -401,6 +401,7 @@ static int altera_cvp_probe(struct pci_dev *pdev,
 			    const struct pci_device_id *dev_id)
 {
 	struct altera_cvp_conf *conf;
+	struct fpga_manager *mgr;
 	u16 cmd, val;
 	int ret;
 
@@ -452,16 +453,24 @@ 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));
 
-	ret = fpga_mgr_register(&pdev->dev, conf->mgr_name,
-				&altera_cvp_ops, conf);
-	if (ret)
+	mgr = fpga_mgr_create(&pdev->dev, conf->mgr_name,
+			      &altera_cvp_ops, conf);
+	if (!mgr)
+		return -ENOMEM;
+
+	pci_set_drvdata(pdev, mgr);
+
+	ret = fpga_mgr_register(mgr);
+	if (ret) {
+		fpga_mgr_free(mgr);
 		goto err_unmap;
+	}
 
 	ret = driver_create_file(&altera_cvp_driver.driver,
 				 &driver_attr_chkcfg);
 	if (ret) {
 		dev_err(&pdev->dev, "Can't create sysfs chkcfg file\n");
-		fpga_mgr_unregister(&pdev->dev);
+		fpga_mgr_unregister(mgr);
 		goto err_unmap;
 	}
 
@@ -483,7 +492,7 @@ static void altera_cvp_remove(struct pci_dev *pdev)
 	u16 cmd;
 
 	driver_remove_file(&altera_cvp_driver.driver, &driver_attr_chkcfg);
-	fpga_mgr_unregister(&pdev->dev);
+	fpga_mgr_unregister(mgr);
 	pci_iounmap(pdev, conf->map);
 	pci_release_region(pdev, CVP_BAR);
 	pci_read_config_word(pdev, PCI_COMMAND, &cmd);
diff --git a/drivers/fpga/altera-pr-ip-core.c b/drivers/fpga/altera-pr-ip-core.c
index a7b31f9797ce..eea521774cf6 100644
--- a/drivers/fpga/altera-pr-ip-core.c
+++ b/drivers/fpga/altera-pr-ip-core.c
@@ -187,6 +187,8 @@ static const struct fpga_manager_ops alt_pr_ops = {
 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);
@@ -201,15 +203,27 @@ 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));
 
-	return fpga_mgr_register(dev, dev_name(dev), &alt_pr_ops, priv);
+	mgr = 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;
 }
 EXPORT_SYMBOL_GPL(alt_pr_register);
 
 int alt_pr_unregister(struct device *dev)
 {
+	struct fpga_manager *mgr = dev_get_drvdata(dev);
+
 	dev_dbg(dev, "%s\n", __func__);
 
-	fpga_mgr_unregister(dev);
+	fpga_mgr_unregister(mgr);
 
 	return 0;
 }
diff --git a/drivers/fpga/altera-ps-spi.c b/drivers/fpga/altera-ps-spi.c
index 06d212a3d49d..24b25c626036 100644
--- a/drivers/fpga/altera-ps-spi.c
+++ b/drivers/fpga/altera-ps-spi.c
@@ -238,6 +238,8 @@ 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)
@@ -273,13 +275,25 @@ 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));
 
-	return fpga_mgr_register(&spi->dev, conf->mgr_name,
-				 &altera_ps_ops, conf);
+	mgr = 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;
 }
 
 static int altera_ps_remove(struct spi_device *spi)
 {
-	fpga_mgr_unregister(&spi->dev);
+	struct fpga_manager *mgr = spi_get_drvdata(spi);
+
+	fpga_mgr_unregister(mgr);
 
 	return 0;
 }
diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c
index 9939d2cbc9a6..0a5181db3e2b 100644
--- a/drivers/fpga/fpga-mgr.c
+++ b/drivers/fpga/fpga-mgr.c
@@ -515,17 +515,17 @@ void fpga_mgr_unlock(struct fpga_manager *mgr)
 EXPORT_SYMBOL_GPL(fpga_mgr_unlock);
 
 /**
- * fpga_mgr_register - register a low level fpga manager driver
+ * fpga_mgr_create - create and initialize a 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
  *
- * Return: 0 on success, negative error code otherwise.
+ * Return: pointer to struct fpga_manager or NULL
  */
-int fpga_mgr_register(struct device *dev, const char *name,
-		      const struct fpga_manager_ops *mops,
-		      void *priv)
+struct fpga_manager *fpga_mgr_create(struct device *dev, const char *name,
+				     const struct fpga_manager_ops *mops,
+				     void *priv)
 {
 	struct fpga_manager *mgr;
 	int id, ret;
@@ -534,17 +534,17 @@ int fpga_mgr_register(struct device *dev, const char *name,
 	    !mops->write_init || (!mops->write && !mops->write_sg) ||
 	    (mops->write && mops->write_sg)) {
 		dev_err(dev, "Attempt to register without fpga_manager_ops\n");
-		return -EINVAL;
+		return NULL;
 	}
 
 	if (!name || !strlen(name)) {
 		dev_err(dev, "Attempt to register with no name!\n");
-		return -EINVAL;
+		return NULL;
 	}
 
 	mgr = kzalloc(sizeof(*mgr), GFP_KERNEL);
 	if (!mgr)
-		return -ENOMEM;
+		return NULL;
 
 	id = ida_simple_get(&fpga_mgr_ida, 0, 0, GFP_KERNEL);
 	if (id < 0) {
@@ -558,25 +558,56 @@ int fpga_mgr_register(struct device *dev, const char *name,
 	mgr->mops = mops;
 	mgr->priv = priv;
 
-	/*
-	 * Initialize framework state by requesting low level driver read state
-	 * from device.  FPGA may be in reset mode or may have been programmed
-	 * by bootloader or EEPROM.
-	 */
-	mgr->state = mgr->mops->state(mgr);
-
 	device_initialize(&mgr->dev);
 	mgr->dev.class = fpga_mgr_class;
 	mgr->dev.groups = mops->groups;
 	mgr->dev.parent = dev;
 	mgr->dev.of_node = dev->of_node;
 	mgr->dev.id = id;
-	dev_set_drvdata(dev, mgr);
 
 	ret = dev_set_name(&mgr->dev, "fpga%d", id);
 	if (ret)
 		goto error_device;
 
+	return mgr;
+
+error_device:
+	ida_simple_remove(&fpga_mgr_ida, id);
+error_kfree:
+	kfree(mgr);
+
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(fpga_mgr_create);
+
+/**
+ * fpga_mgr_free - deallocate a FPGA manager
+ * @mgr:	fpga manager struct created by fpga_mgr_create
+ */
+void fpga_mgr_free(struct fpga_manager *mgr)
+{
+	ida_simple_remove(&fpga_mgr_ida, mgr->dev.id);
+	kfree(mgr);
+}
+EXPORT_SYMBOL_GPL(fpga_mgr_free);
+
+/**
+ * fpga_mgr_register - register a FPGA manager
+ * @mgr:	fpga manager struct created by fpga_mgr_create
+ *
+ * Return: 0 on success, negative error code otherwise.
+ */
+int fpga_mgr_register(struct fpga_manager *mgr)
+{
+	int ret;
+
+	/*
+	 * Initialize framework state by requesting low level driver read state
+	 * from device.  FPGA may be in reset mode or may have been programmed
+	 * by bootloader or EEPROM.
+	 */
+	mgr->state = mgr->mops->state(mgr);
+
 	ret = device_add(&mgr->dev);
 	if (ret)
 		goto error_device;
@@ -586,22 +617,18 @@ int fpga_mgr_register(struct device *dev, const char *name,
 	return 0;
 
 error_device:
-	ida_simple_remove(&fpga_mgr_ida, id);
-error_kfree:
-	kfree(mgr);
+	ida_simple_remove(&fpga_mgr_ida, mgr->dev.id);
 
 	return ret;
 }
 EXPORT_SYMBOL_GPL(fpga_mgr_register);
 
 /**
- * fpga_mgr_unregister - unregister a low level fpga manager driver
- * @dev:	fpga manager device from pdev
+ * fpga_mgr_unregister - unregister a FPGA manager
+ * @mgr:	fpga manager struct
  */
-void fpga_mgr_unregister(struct device *dev)
+void fpga_mgr_unregister(struct fpga_manager *mgr)
 {
-	struct fpga_manager *mgr = dev_get_drvdata(dev);
-
 	dev_info(&mgr->dev, "%s %s\n", __func__, mgr->name);
 
 	/*
@@ -619,8 +646,7 @@ static void fpga_mgr_dev_release(struct device *dev)
 {
 	struct fpga_manager *mgr = to_fpga_manager(dev);
 
-	ida_simple_remove(&fpga_mgr_ida, mgr->dev.id);
-	kfree(mgr);
+	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 7fca82023062..5981c7ee7a7d 100644
--- a/drivers/fpga/ice40-spi.c
+++ b/drivers/fpga/ice40-spi.c
@@ -133,6 +133,7 @@ static int ice40_fpga_probe(struct spi_device *spi)
 {
 	struct device *dev = &spi->dev;
 	struct ice40_fpga_priv *priv;
+	struct fpga_manager *mgr;
 	int ret;
 
 	priv = devm_kzalloc(&spi->dev, sizeof(*priv), GFP_KERNEL);
@@ -174,14 +175,26 @@ static int ice40_fpga_probe(struct spi_device *spi)
 		return ret;
 	}
 
-	/* Register with the FPGA manager */
-	return fpga_mgr_register(dev, "Lattice iCE40 FPGA Manager",
-				 &ice40_fpga_ops, priv);
+	mgr = 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;
 }
 
 static int ice40_fpga_remove(struct spi_device *spi)
 {
-	fpga_mgr_unregister(&spi->dev);
+	struct fpga_manager *mgr = spi_get_drvdata(spi);
+
+	fpga_mgr_unregister(mgr);
+
 	return 0;
 }
 
diff --git a/drivers/fpga/machxo2-spi.c b/drivers/fpga/machxo2-spi.c
index 8e95ec9c5c9a..a582e0000c97 100644
--- a/drivers/fpga/machxo2-spi.c
+++ b/drivers/fpga/machxo2-spi.c
@@ -355,21 +355,33 @@ static const struct fpga_manager_ops machxo2_ops = {
 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;
 	}
 
-	return fpga_mgr_register(dev, "Lattice MachXO2 SPI FPGA Manager",
-				 &machxo2_ops, spi);
+	mgr = 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;
 }
 
 static int machxo2_spi_remove(struct spi_device *spi)
 {
-	struct device *dev = &spi->dev;
+	struct fpga_manager *mgr = spi_get_drvdata(spi);
 
-	fpga_mgr_unregister(dev);
+	fpga_mgr_unregister(mgr);
 
 	return 0;
 }
diff --git a/drivers/fpga/socfpga-a10.c b/drivers/fpga/socfpga-a10.c
index a46e343a5b72..dec3db5cdab1 100644
--- a/drivers/fpga/socfpga-a10.c
+++ b/drivers/fpga/socfpga-a10.c
@@ -482,6 +482,7 @@ static int socfpga_a10_fpga_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct a10_fpga_priv *priv;
 	void __iomem *reg_base;
+	struct fpga_manager *mgr;
 	struct resource *res;
 	int ret;
 
@@ -519,9 +520,16 @@ static int socfpga_a10_fpga_probe(struct platform_device *pdev)
 		return -EBUSY;
 	}
 
-	ret = fpga_mgr_register(dev, "SoCFPGA Arria10 FPGA Manager",
-				 &socfpga_a10_fpga_mgr_ops, priv);
+	mgr = fpga_mgr_create(dev, "SoCFPGA Arria10 FPGA Manager",
+			      &socfpga_a10_fpga_mgr_ops, priv);
+	if (!mgr)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, mgr);
+
+	ret = fpga_mgr_register(mgr);
 	if (ret) {
+		fpga_mgr_free(mgr);
 		clk_disable_unprepare(priv->clk);
 		return ret;
 	}
@@ -534,7 +542,7 @@ static int socfpga_a10_fpga_remove(struct platform_device *pdev)
 	struct fpga_manager *mgr = platform_get_drvdata(pdev);
 	struct a10_fpga_priv *priv = mgr->priv;
 
-	fpga_mgr_unregister(&pdev->dev);
+	fpga_mgr_unregister(mgr);
 	clk_disable_unprepare(priv->clk);
 
 	return 0;
diff --git a/drivers/fpga/socfpga.c b/drivers/fpga/socfpga.c
index b6672e66cda6..51efaf9e0e03 100644
--- a/drivers/fpga/socfpga.c
+++ b/drivers/fpga/socfpga.c
@@ -555,6 +555,7 @@ static int socfpga_fpga_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct socfpga_fpga_priv *priv;
+	struct fpga_manager *mgr;
 	struct resource *res;
 	int ret;
 
@@ -581,13 +582,25 @@ static int socfpga_fpga_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	return fpga_mgr_register(dev, "Altera SOCFPGA FPGA Manager",
-				 &socfpga_fpga_ops, priv);
+	mgr = 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;
 }
 
 static int socfpga_fpga_remove(struct platform_device *pdev)
 {
-	fpga_mgr_unregister(&pdev->dev);
+	struct fpga_manager *mgr = platform_get_drvdata(pdev);
+
+	fpga_mgr_unregister(mgr);
 
 	return 0;
 }
diff --git a/drivers/fpga/ts73xx-fpga.c b/drivers/fpga/ts73xx-fpga.c
index f6a96b42e2ca..08efd1895b1b 100644
--- a/drivers/fpga/ts73xx-fpga.c
+++ b/drivers/fpga/ts73xx-fpga.c
@@ -116,7 +116,9 @@ static int ts73xx_fpga_probe(struct platform_device *pdev)
 {
 	struct device *kdev = &pdev->dev;
 	struct ts73xx_fpga_priv *priv;
+	struct fpga_manager *mgr;
 	struct resource *res;
+	int ret;
 
 	priv = devm_kzalloc(kdev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
@@ -131,13 +133,25 @@ static int ts73xx_fpga_probe(struct platform_device *pdev)
 		return PTR_ERR(priv->io_base);
 	}
 
-	return fpga_mgr_register(kdev, "TS-73xx FPGA Manager",
-				 &ts73xx_fpga_ops, priv);
+	mgr = 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;
 }
 
 static int ts73xx_fpga_remove(struct platform_device *pdev)
 {
-	fpga_mgr_unregister(&pdev->dev);
+	struct fpga_manager *mgr = platform_get_drvdata(pdev);
+
+	fpga_mgr_unregister(mgr);
 
 	return 0;
 }
diff --git a/drivers/fpga/xilinx-spi.c b/drivers/fpga/xilinx-spi.c
index 9b62a4c2a3df..8d1945966533 100644
--- a/drivers/fpga/xilinx-spi.c
+++ b/drivers/fpga/xilinx-spi.c
@@ -143,6 +143,8 @@ static const struct fpga_manager_ops xilinx_spi_ops = {
 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)
@@ -165,13 +167,25 @@ static int xilinx_spi_probe(struct spi_device *spi)
 		return PTR_ERR(conf->done);
 	}
 
-	return fpga_mgr_register(&spi->dev, "Xilinx Slave Serial FPGA Manager",
-				 &xilinx_spi_ops, conf);
+	mgr = 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;
 }
 
 static int xilinx_spi_remove(struct spi_device *spi)
 {
-	fpga_mgr_unregister(&spi->dev);
+	struct fpga_manager *mgr = spi_get_drvdata(spi);
+
+	fpga_mgr_unregister(mgr);
 
 	return 0;
 }
diff --git a/drivers/fpga/zynq-fpga.c b/drivers/fpga/zynq-fpga.c
index 70b15b303471..3110e00121ca 100644
--- a/drivers/fpga/zynq-fpga.c
+++ b/drivers/fpga/zynq-fpga.c
@@ -558,6 +558,7 @@ static int zynq_fpga_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct zynq_fpga_priv *priv;
+	struct fpga_manager *mgr;
 	struct resource *res;
 	int err;
 
@@ -613,10 +614,17 @@ static int zynq_fpga_probe(struct platform_device *pdev)
 
 	clk_disable(priv->clk);
 
-	err = fpga_mgr_register(dev, "Xilinx Zynq FPGA Manager",
-				&zynq_fpga_ops, priv);
+	mgr = fpga_mgr_create(dev, "Xilinx Zynq FPGA Manager",
+			      &zynq_fpga_ops, priv);
+	if (!mgr)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, mgr);
+
+	err = fpga_mgr_register(mgr);
 	if (err) {
 		dev_err(dev, "unable to register FPGA manager\n");
+		fpga_mgr_free(mgr);
 		clk_unprepare(priv->clk);
 		return err;
 	}
@@ -632,7 +640,7 @@ static int zynq_fpga_remove(struct platform_device *pdev)
 	mgr = platform_get_drvdata(pdev);
 	priv = mgr->priv;
 
-	fpga_mgr_unregister(&pdev->dev);
+	fpga_mgr_unregister(mgr);
 
 	clk_unprepare(priv->clk);
 
diff --git a/include/linux/fpga/fpga-mgr.h b/include/linux/fpga/fpga-mgr.h
index 3c6de23aabdf..1266c1108e70 100644
--- a/include/linux/fpga/fpga-mgr.h
+++ b/include/linux/fpga/fpga-mgr.h
@@ -170,9 +170,11 @@ struct fpga_manager *fpga_mgr_get(struct device *dev);
 
 void fpga_mgr_put(struct fpga_manager *mgr);
 
-int fpga_mgr_register(struct device *dev, const char *name,
-		      const struct fpga_manager_ops *mops, void *priv);
-
-void fpga_mgr_unregister(struct device *dev);
+struct fpga_manager *fpga_mgr_create(struct device *dev, const char *name,
+				     const struct fpga_manager_ops *mops,
+				     void *priv);
+void fpga_mgr_free(struct fpga_manager *mgr);
+int fpga_mgr_register(struct fpga_manager *mgr);
+void fpga_mgr_unregister(struct fpga_manager *mgr);
 
 #endif /*_LINUX_FPGA_MGR_H */
-- 
2.17.0

^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH 3/4] fpga: bridge: change api, don't use drvdata
  2018-04-27  1:26 ` Moritz Fischer
  (?)
@ 2018-04-27  1:26   ` Moritz Fischer
  -1 siblings, 0 replies; 24+ messages in thread
From: Moritz Fischer @ 2018-04-27  1:26 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-arm-kernel, linux-doc, linux-fpga, michal.simek, corbet,
	atull, Moritz Fischer

From: Alan Tull <atull@kernel.org>

Change fpga_bridge_register to not set drvdata.  This is to support
the case where a PCIe device can have more than one bridge.

Add API functions to create/free the fpga bridge struct. Change
fpga_bridge_register/unregister to take FPGA bridge struct as
the only parameter.

  struct fpga_bridge
  *fpga_bridge_create(struct device *dev, const char *name,
                      const struct fpga_bridge_ops *br_ops,
                      void *priv);
  void fpga_bridge_free(struct fpga_bridge *br);
  int fpga_bridge_register(struct fpga_bridge *br);
  void fpga_bridge_unregister(struct fpga_bridge *br);

Update the drivers that call fpga_bridge_register with the new API.

Signed-off-by: Alan Tull <atull@kernel.org>
Reported-by: Jiuyue Ma <majiuyue@huawei.com>
Signed-off-by: Moritz Fischer <mdf@kernel.org>
---
 drivers/fpga/altera-fpga2sdram.c    | 21 ++++++---
 drivers/fpga/altera-freeze-bridge.c | 22 +++++++--
 drivers/fpga/altera-hps2fpga.c      | 24 +++++++---
 drivers/fpga/fpga-bridge.c          | 70 +++++++++++++++++++----------
 drivers/fpga/xilinx-pr-decoupler.c  | 22 ++++++---
 include/linux/fpga/fpga-bridge.h    |  9 ++--
 6 files changed, 123 insertions(+), 45 deletions(-)

diff --git a/drivers/fpga/altera-fpga2sdram.c b/drivers/fpga/altera-fpga2sdram.c
index d4eeb74388da..5a29ab6e3b28 100644
--- a/drivers/fpga/altera-fpga2sdram.c
+++ b/drivers/fpga/altera-fpga2sdram.c
@@ -106,6 +106,7 @@ static int alt_fpga_bridge_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct alt_fpga2sdram_data *priv;
+	struct fpga_bridge *br;
 	u32 enable;
 	struct regmap *sysmgr;
 	int ret = 0;
@@ -131,10 +132,18 @@ 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);
 
-	ret = fpga_bridge_register(dev, F2S_BRIDGE_NAME,
-				   &altera_fpga2sdram_br_ops, priv);
-	if (ret)
+	br = 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);
 		return ret;
+	}
 
 	dev_info(dev, "driver initialized with handoff %08x\n", priv->mask);
 
@@ -146,7 +155,7 @@ static int alt_fpga_bridge_probe(struct platform_device *pdev)
 				 (enable ? "enabling" : "disabling"));
 			ret = _alt_fpga2sdram_enable_set(priv, enable);
 			if (ret) {
-				fpga_bridge_unregister(&pdev->dev);
+				fpga_bridge_unregister(br);
 				return ret;
 			}
 		}
@@ -157,7 +166,9 @@ static int alt_fpga_bridge_probe(struct platform_device *pdev)
 
 static int alt_fpga_bridge_remove(struct platform_device *pdev)
 {
-	fpga_bridge_unregister(&pdev->dev);
+	struct fpga_bridge *br = platform_get_drvdata(pdev);
+
+	fpga_bridge_unregister(br);
 
 	return 0;
 }
diff --git a/drivers/fpga/altera-freeze-bridge.c b/drivers/fpga/altera-freeze-bridge.c
index 6159cfcf78a2..fa4b693cf4be 100644
--- a/drivers/fpga/altera-freeze-bridge.c
+++ b/drivers/fpga/altera-freeze-bridge.c
@@ -221,8 +221,10 @@ static int altera_freeze_br_probe(struct platform_device *pdev)
 	struct device_node *np = pdev->dev.of_node;
 	void __iomem *base_addr;
 	struct altera_freeze_br_data *priv;
+	struct fpga_bridge *br;
 	struct resource *res;
 	u32 status, revision;
+	int ret;
 
 	if (!np)
 		return -ENODEV;
@@ -254,13 +256,27 @@ static int altera_freeze_br_probe(struct platform_device *pdev)
 
 	priv->base_addr = base_addr;
 
-	return fpga_bridge_register(dev, FREEZE_BRIDGE_NAME,
-				    &altera_freeze_br_br_ops, priv);
+	br = 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;
 }
 
 static int altera_freeze_br_remove(struct platform_device *pdev)
 {
-	fpga_bridge_unregister(&pdev->dev);
+	struct fpga_bridge *br = platform_get_drvdata(pdev);
+
+	fpga_bridge_unregister(br);
 
 	return 0;
 }
diff --git a/drivers/fpga/altera-hps2fpga.c b/drivers/fpga/altera-hps2fpga.c
index 406d2f10741f..e4d39f0a7572 100644
--- a/drivers/fpga/altera-hps2fpga.c
+++ b/drivers/fpga/altera-hps2fpga.c
@@ -139,6 +139,7 @@ static int alt_fpga_bridge_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct altera_hps2fpga_data *priv;
 	const struct of_device_id *of_id;
+	struct fpga_bridge *br;
 	u32 enable;
 	int ret;
 
@@ -190,11 +191,24 @@ static int alt_fpga_bridge_probe(struct platform_device *pdev)
 		}
 	}
 
-	ret = fpga_bridge_register(dev, priv->name, &altera_hps2fpga_br_ops,
-				   priv);
-err:
+	br = fpga_bridge_create(dev, priv->name, &altera_hps2fpga_br_ops, priv);
+	if (!br) {
+		ret = -ENOMEM;
+		goto err;
+	}
+
+	platform_set_drvdata(pdev, br);
+
+	ret = fpga_bridge_register(br);
 	if (ret)
-		clk_disable_unprepare(priv->clk);
+		goto err_free;
+
+	return 0;
+
+err_free:
+	fpga_bridge_free(br);
+err:
+	clk_disable_unprepare(priv->clk);
 
 	return ret;
 }
@@ -204,7 +218,7 @@ static int alt_fpga_bridge_remove(struct platform_device *pdev)
 	struct fpga_bridge *bridge = platform_get_drvdata(pdev);
 	struct altera_hps2fpga_data *priv = bridge->priv;
 
-	fpga_bridge_unregister(&pdev->dev);
+	fpga_bridge_unregister(bridge);
 
 	clk_disable_unprepare(priv->clk);
 
diff --git a/drivers/fpga/fpga-bridge.c b/drivers/fpga/fpga-bridge.c
index 31bd2c59c305..2db1573507eb 100644
--- a/drivers/fpga/fpga-bridge.c
+++ b/drivers/fpga/fpga-bridge.c
@@ -328,28 +328,29 @@ static struct attribute *fpga_bridge_attrs[] = {
 ATTRIBUTE_GROUPS(fpga_bridge);
 
 /**
- * fpga_bridge_register - register a fpga bridge driver
+ * fpga_bridge_create - create and initialize a 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
  *
- * Return: 0 for success, error code otherwise.
+ * Return: struct fpga_bridge or NULL
  */
-int fpga_bridge_register(struct device *dev, const char *name,
-			 const struct fpga_bridge_ops *br_ops, void *priv)
+struct fpga_bridge *fpga_bridge_create(struct device *dev, const char *name,
+				       const struct fpga_bridge_ops *br_ops,
+				       void *priv)
 {
 	struct fpga_bridge *bridge;
 	int id, ret = 0;
 
 	if (!name || !strlen(name)) {
 		dev_err(dev, "Attempt to register with no name!\n");
-		return -EINVAL;
+		return NULL;
 	}
 
 	bridge = kzalloc(sizeof(*bridge), GFP_KERNEL);
 	if (!bridge)
-		return -ENOMEM;
+		return NULL;
 
 	id = ida_simple_get(&fpga_bridge_ida, 0, 0, GFP_KERNEL);
 	if (id < 0) {
@@ -370,40 +371,62 @@ int fpga_bridge_register(struct device *dev, const char *name,
 	bridge->dev.parent = dev;
 	bridge->dev.of_node = dev->of_node;
 	bridge->dev.id = id;
-	dev_set_drvdata(dev, bridge);
 
 	ret = dev_set_name(&bridge->dev, "br%d", id);
 	if (ret)
 		goto error_device;
 
-	ret = device_add(&bridge->dev);
-	if (ret)
-		goto error_device;
-
-	of_platform_populate(dev->of_node, NULL, NULL, dev);
-
-	dev_info(bridge->dev.parent, "fpga bridge [%s] registered\n",
-		 bridge->name);
-
-	return 0;
+	return bridge;
 
 error_device:
 	ida_simple_remove(&fpga_bridge_ida, id);
 error_kfree:
 	kfree(bridge);
 
-	return ret;
+	return NULL;
+}
+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
+ */
+void fpga_bridge_free(struct fpga_bridge *bridge)
+{
+	ida_simple_remove(&fpga_bridge_ida, bridge->dev.id);
+	kfree(bridge);
+}
+EXPORT_SYMBOL_GPL(fpga_bridge_free);
+
+/**
+ * fpga_bridge_register - register a fpga bridge
+ * @bridge:	FPGA bridge struct created by fpga_bridge_create
+ *
+ * Return: 0 for success, error code otherwise.
+ */
+int fpga_bridge_register(struct fpga_bridge *bridge)
+{
+	struct device *dev = &bridge->dev;
+	int ret;
+
+	ret = device_add(dev);
+	if (ret)
+		return ret;
+
+	of_platform_populate(dev->of_node, NULL, NULL, dev);
+
+	dev_info(dev->parent, "fpga bridge [%s] registered\n", bridge->name);
+
+	return 0;
 }
 EXPORT_SYMBOL_GPL(fpga_bridge_register);
 
 /**
  * fpga_bridge_unregister - unregister a fpga bridge driver
- * @dev: FPGA bridge device from pdev
+ * @bridge:	FPGA bridge struct created by fpga_bridge_create
  */
-void fpga_bridge_unregister(struct device *dev)
+void fpga_bridge_unregister(struct fpga_bridge *bridge)
 {
-	struct fpga_bridge *bridge = dev_get_drvdata(dev);
-
 	/*
 	 * If the low level driver provides a method for putting bridge into
 	 * a desired state upon unregister, do it.
@@ -419,8 +442,7 @@ static void fpga_bridge_dev_release(struct device *dev)
 {
 	struct fpga_bridge *bridge = to_fpga_bridge(dev);
 
-	ida_simple_remove(&fpga_bridge_ida, bridge->dev.id);
-	kfree(bridge);
+	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 0d7743089414..07ba1539e82c 100644
--- a/drivers/fpga/xilinx-pr-decoupler.c
+++ b/drivers/fpga/xilinx-pr-decoupler.c
@@ -94,6 +94,7 @@ MODULE_DEVICE_TABLE(of, xlnx_pr_decoupler_of_match);
 static int xlnx_pr_decoupler_probe(struct platform_device *pdev)
 {
 	struct xlnx_pr_decoupler_data *priv;
+	struct fpga_bridge *br;
 	int err;
 	struct resource *res;
 
@@ -120,16 +121,27 @@ static int xlnx_pr_decoupler_probe(struct platform_device *pdev)
 
 	clk_disable(priv->clk);
 
-	err = fpga_bridge_register(&pdev->dev, "Xilinx PR Decoupler",
-				   &xlnx_pr_decoupler_br_ops, priv);
+	br = fpga_bridge_create(&pdev->dev, "Xilinx PR Decoupler",
+				&xlnx_pr_decoupler_br_ops, priv);
+	if (!br) {
+		err = -ENOMEM;
+		goto err_clk;
+	}
+
+	platform_set_drvdata(pdev, br);
 
+	err = fpga_bridge_register(br);
 	if (err) {
 		dev_err(&pdev->dev, "unable to register Xilinx PR Decoupler");
-		clk_unprepare(priv->clk);
-		return err;
+		goto err_clk;
 	}
 
 	return 0;
+
+err_clk:
+	clk_unprepare(priv->clk);
+
+	return err;
 }
 
 static int xlnx_pr_decoupler_remove(struct platform_device *pdev)
@@ -137,7 +149,7 @@ static int xlnx_pr_decoupler_remove(struct platform_device *pdev)
 	struct fpga_bridge *bridge = platform_get_drvdata(pdev);
 	struct xlnx_pr_decoupler_data *p = bridge->priv;
 
-	fpga_bridge_unregister(&pdev->dev);
+	fpga_bridge_unregister(bridge);
 
 	clk_unprepare(p->clk);
 
diff --git a/include/linux/fpga/fpga-bridge.h b/include/linux/fpga/fpga-bridge.h
index 3694821a6d2d..ce550fcf6360 100644
--- a/include/linux/fpga/fpga-bridge.h
+++ b/include/linux/fpga/fpga-bridge.h
@@ -62,8 +62,11 @@ int of_fpga_bridge_get_to_list(struct device_node *np,
 			       struct fpga_image_info *info,
 			       struct list_head *bridge_list);
 
-int fpga_bridge_register(struct device *dev, const char *name,
-			 const struct fpga_bridge_ops *br_ops, void *priv);
-void fpga_bridge_unregister(struct device *dev);
+struct fpga_bridge *fpga_bridge_create(struct device *dev, const char *name,
+				       const struct fpga_bridge_ops *br_ops,
+				       void *priv);
+void fpga_bridge_free(struct fpga_bridge *br);
+int fpga_bridge_register(struct fpga_bridge *br);
+void fpga_bridge_unregister(struct fpga_bridge *br);
 
 #endif /* _LINUX_FPGA_BRIDGE_H */
-- 
2.17.0

^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH 3/4] fpga: bridge: change api, don't use drvdata
@ 2018-04-27  1:26   ` Moritz Fischer
  0 siblings, 0 replies; 24+ messages in thread
From: Moritz Fischer @ 2018-04-27  1:26 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-arm-kernel, linux-doc, linux-fpga, michal.simek, corbet,
	atull, Moritz Fischer

From: Alan Tull <atull@kernel.org>

Change fpga_bridge_register to not set drvdata.  This is to support
the case where a PCIe device can have more than one bridge.

Add API functions to create/free the fpga bridge struct. Change
fpga_bridge_register/unregister to take FPGA bridge struct as
the only parameter.

  struct fpga_bridge
  *fpga_bridge_create(struct device *dev, const char *name,
                      const struct fpga_bridge_ops *br_ops,
                      void *priv);
  void fpga_bridge_free(struct fpga_bridge *br);
  int fpga_bridge_register(struct fpga_bridge *br);
  void fpga_bridge_unregister(struct fpga_bridge *br);

Update the drivers that call fpga_bridge_register with the new API.

Signed-off-by: Alan Tull <atull@kernel.org>
Reported-by: Jiuyue Ma <majiuyue@huawei.com>
Signed-off-by: Moritz Fischer <mdf@kernel.org>
---
 drivers/fpga/altera-fpga2sdram.c    | 21 ++++++---
 drivers/fpga/altera-freeze-bridge.c | 22 +++++++--
 drivers/fpga/altera-hps2fpga.c      | 24 +++++++---
 drivers/fpga/fpga-bridge.c          | 70 +++++++++++++++++++----------
 drivers/fpga/xilinx-pr-decoupler.c  | 22 ++++++---
 include/linux/fpga/fpga-bridge.h    |  9 ++--
 6 files changed, 123 insertions(+), 45 deletions(-)

diff --git a/drivers/fpga/altera-fpga2sdram.c b/drivers/fpga/altera-fpga2sdram.c
index d4eeb74388da..5a29ab6e3b28 100644
--- a/drivers/fpga/altera-fpga2sdram.c
+++ b/drivers/fpga/altera-fpga2sdram.c
@@ -106,6 +106,7 @@ static int alt_fpga_bridge_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct alt_fpga2sdram_data *priv;
+	struct fpga_bridge *br;
 	u32 enable;
 	struct regmap *sysmgr;
 	int ret = 0;
@@ -131,10 +132,18 @@ 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);
 
-	ret = fpga_bridge_register(dev, F2S_BRIDGE_NAME,
-				   &altera_fpga2sdram_br_ops, priv);
-	if (ret)
+	br = 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);
 		return ret;
+	}
 
 	dev_info(dev, "driver initialized with handoff %08x\n", priv->mask);
 
@@ -146,7 +155,7 @@ static int alt_fpga_bridge_probe(struct platform_device *pdev)
 				 (enable ? "enabling" : "disabling"));
 			ret = _alt_fpga2sdram_enable_set(priv, enable);
 			if (ret) {
-				fpga_bridge_unregister(&pdev->dev);
+				fpga_bridge_unregister(br);
 				return ret;
 			}
 		}
@@ -157,7 +166,9 @@ static int alt_fpga_bridge_probe(struct platform_device *pdev)
 
 static int alt_fpga_bridge_remove(struct platform_device *pdev)
 {
-	fpga_bridge_unregister(&pdev->dev);
+	struct fpga_bridge *br = platform_get_drvdata(pdev);
+
+	fpga_bridge_unregister(br);
 
 	return 0;
 }
diff --git a/drivers/fpga/altera-freeze-bridge.c b/drivers/fpga/altera-freeze-bridge.c
index 6159cfcf78a2..fa4b693cf4be 100644
--- a/drivers/fpga/altera-freeze-bridge.c
+++ b/drivers/fpga/altera-freeze-bridge.c
@@ -221,8 +221,10 @@ static int altera_freeze_br_probe(struct platform_device *pdev)
 	struct device_node *np = pdev->dev.of_node;
 	void __iomem *base_addr;
 	struct altera_freeze_br_data *priv;
+	struct fpga_bridge *br;
 	struct resource *res;
 	u32 status, revision;
+	int ret;
 
 	if (!np)
 		return -ENODEV;
@@ -254,13 +256,27 @@ static int altera_freeze_br_probe(struct platform_device *pdev)
 
 	priv->base_addr = base_addr;
 
-	return fpga_bridge_register(dev, FREEZE_BRIDGE_NAME,
-				    &altera_freeze_br_br_ops, priv);
+	br = 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;
 }
 
 static int altera_freeze_br_remove(struct platform_device *pdev)
 {
-	fpga_bridge_unregister(&pdev->dev);
+	struct fpga_bridge *br = platform_get_drvdata(pdev);
+
+	fpga_bridge_unregister(br);
 
 	return 0;
 }
diff --git a/drivers/fpga/altera-hps2fpga.c b/drivers/fpga/altera-hps2fpga.c
index 406d2f10741f..e4d39f0a7572 100644
--- a/drivers/fpga/altera-hps2fpga.c
+++ b/drivers/fpga/altera-hps2fpga.c
@@ -139,6 +139,7 @@ static int alt_fpga_bridge_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct altera_hps2fpga_data *priv;
 	const struct of_device_id *of_id;
+	struct fpga_bridge *br;
 	u32 enable;
 	int ret;
 
@@ -190,11 +191,24 @@ static int alt_fpga_bridge_probe(struct platform_device *pdev)
 		}
 	}
 
-	ret = fpga_bridge_register(dev, priv->name, &altera_hps2fpga_br_ops,
-				   priv);
-err:
+	br = fpga_bridge_create(dev, priv->name, &altera_hps2fpga_br_ops, priv);
+	if (!br) {
+		ret = -ENOMEM;
+		goto err;
+	}
+
+	platform_set_drvdata(pdev, br);
+
+	ret = fpga_bridge_register(br);
 	if (ret)
-		clk_disable_unprepare(priv->clk);
+		goto err_free;
+
+	return 0;
+
+err_free:
+	fpga_bridge_free(br);
+err:
+	clk_disable_unprepare(priv->clk);
 
 	return ret;
 }
@@ -204,7 +218,7 @@ static int alt_fpga_bridge_remove(struct platform_device *pdev)
 	struct fpga_bridge *bridge = platform_get_drvdata(pdev);
 	struct altera_hps2fpga_data *priv = bridge->priv;
 
-	fpga_bridge_unregister(&pdev->dev);
+	fpga_bridge_unregister(bridge);
 
 	clk_disable_unprepare(priv->clk);
 
diff --git a/drivers/fpga/fpga-bridge.c b/drivers/fpga/fpga-bridge.c
index 31bd2c59c305..2db1573507eb 100644
--- a/drivers/fpga/fpga-bridge.c
+++ b/drivers/fpga/fpga-bridge.c
@@ -328,28 +328,29 @@ static struct attribute *fpga_bridge_attrs[] = {
 ATTRIBUTE_GROUPS(fpga_bridge);
 
 /**
- * fpga_bridge_register - register a fpga bridge driver
+ * fpga_bridge_create - create and initialize a 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
  *
- * Return: 0 for success, error code otherwise.
+ * Return: struct fpga_bridge or NULL
  */
-int fpga_bridge_register(struct device *dev, const char *name,
-			 const struct fpga_bridge_ops *br_ops, void *priv)
+struct fpga_bridge *fpga_bridge_create(struct device *dev, const char *name,
+				       const struct fpga_bridge_ops *br_ops,
+				       void *priv)
 {
 	struct fpga_bridge *bridge;
 	int id, ret = 0;
 
 	if (!name || !strlen(name)) {
 		dev_err(dev, "Attempt to register with no name!\n");
-		return -EINVAL;
+		return NULL;
 	}
 
 	bridge = kzalloc(sizeof(*bridge), GFP_KERNEL);
 	if (!bridge)
-		return -ENOMEM;
+		return NULL;
 
 	id = ida_simple_get(&fpga_bridge_ida, 0, 0, GFP_KERNEL);
 	if (id < 0) {
@@ -370,40 +371,62 @@ int fpga_bridge_register(struct device *dev, const char *name,
 	bridge->dev.parent = dev;
 	bridge->dev.of_node = dev->of_node;
 	bridge->dev.id = id;
-	dev_set_drvdata(dev, bridge);
 
 	ret = dev_set_name(&bridge->dev, "br%d", id);
 	if (ret)
 		goto error_device;
 
-	ret = device_add(&bridge->dev);
-	if (ret)
-		goto error_device;
-
-	of_platform_populate(dev->of_node, NULL, NULL, dev);
-
-	dev_info(bridge->dev.parent, "fpga bridge [%s] registered\n",
-		 bridge->name);
-
-	return 0;
+	return bridge;
 
 error_device:
 	ida_simple_remove(&fpga_bridge_ida, id);
 error_kfree:
 	kfree(bridge);
 
-	return ret;
+	return NULL;
+}
+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
+ */
+void fpga_bridge_free(struct fpga_bridge *bridge)
+{
+	ida_simple_remove(&fpga_bridge_ida, bridge->dev.id);
+	kfree(bridge);
+}
+EXPORT_SYMBOL_GPL(fpga_bridge_free);
+
+/**
+ * fpga_bridge_register - register a fpga bridge
+ * @bridge:	FPGA bridge struct created by fpga_bridge_create
+ *
+ * Return: 0 for success, error code otherwise.
+ */
+int fpga_bridge_register(struct fpga_bridge *bridge)
+{
+	struct device *dev = &bridge->dev;
+	int ret;
+
+	ret = device_add(dev);
+	if (ret)
+		return ret;
+
+	of_platform_populate(dev->of_node, NULL, NULL, dev);
+
+	dev_info(dev->parent, "fpga bridge [%s] registered\n", bridge->name);
+
+	return 0;
 }
 EXPORT_SYMBOL_GPL(fpga_bridge_register);
 
 /**
  * fpga_bridge_unregister - unregister a fpga bridge driver
- * @dev: FPGA bridge device from pdev
+ * @bridge:	FPGA bridge struct created by fpga_bridge_create
  */
-void fpga_bridge_unregister(struct device *dev)
+void fpga_bridge_unregister(struct fpga_bridge *bridge)
 {
-	struct fpga_bridge *bridge = dev_get_drvdata(dev);
-
 	/*
 	 * If the low level driver provides a method for putting bridge into
 	 * a desired state upon unregister, do it.
@@ -419,8 +442,7 @@ static void fpga_bridge_dev_release(struct device *dev)
 {
 	struct fpga_bridge *bridge = to_fpga_bridge(dev);
 
-	ida_simple_remove(&fpga_bridge_ida, bridge->dev.id);
-	kfree(bridge);
+	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 0d7743089414..07ba1539e82c 100644
--- a/drivers/fpga/xilinx-pr-decoupler.c
+++ b/drivers/fpga/xilinx-pr-decoupler.c
@@ -94,6 +94,7 @@ MODULE_DEVICE_TABLE(of, xlnx_pr_decoupler_of_match);
 static int xlnx_pr_decoupler_probe(struct platform_device *pdev)
 {
 	struct xlnx_pr_decoupler_data *priv;
+	struct fpga_bridge *br;
 	int err;
 	struct resource *res;
 
@@ -120,16 +121,27 @@ static int xlnx_pr_decoupler_probe(struct platform_device *pdev)
 
 	clk_disable(priv->clk);
 
-	err = fpga_bridge_register(&pdev->dev, "Xilinx PR Decoupler",
-				   &xlnx_pr_decoupler_br_ops, priv);
+	br = fpga_bridge_create(&pdev->dev, "Xilinx PR Decoupler",
+				&xlnx_pr_decoupler_br_ops, priv);
+	if (!br) {
+		err = -ENOMEM;
+		goto err_clk;
+	}
+
+	platform_set_drvdata(pdev, br);
 
+	err = fpga_bridge_register(br);
 	if (err) {
 		dev_err(&pdev->dev, "unable to register Xilinx PR Decoupler");
-		clk_unprepare(priv->clk);
-		return err;
+		goto err_clk;
 	}
 
 	return 0;
+
+err_clk:
+	clk_unprepare(priv->clk);
+
+	return err;
 }
 
 static int xlnx_pr_decoupler_remove(struct platform_device *pdev)
@@ -137,7 +149,7 @@ static int xlnx_pr_decoupler_remove(struct platform_device *pdev)
 	struct fpga_bridge *bridge = platform_get_drvdata(pdev);
 	struct xlnx_pr_decoupler_data *p = bridge->priv;
 
-	fpga_bridge_unregister(&pdev->dev);
+	fpga_bridge_unregister(bridge);
 
 	clk_unprepare(p->clk);
 
diff --git a/include/linux/fpga/fpga-bridge.h b/include/linux/fpga/fpga-bridge.h
index 3694821a6d2d..ce550fcf6360 100644
--- a/include/linux/fpga/fpga-bridge.h
+++ b/include/linux/fpga/fpga-bridge.h
@@ -62,8 +62,11 @@ int of_fpga_bridge_get_to_list(struct device_node *np,
 			       struct fpga_image_info *info,
 			       struct list_head *bridge_list);
 
-int fpga_bridge_register(struct device *dev, const char *name,
-			 const struct fpga_bridge_ops *br_ops, void *priv);
-void fpga_bridge_unregister(struct device *dev);
+struct fpga_bridge *fpga_bridge_create(struct device *dev, const char *name,
+				       const struct fpga_bridge_ops *br_ops,
+				       void *priv);
+void fpga_bridge_free(struct fpga_bridge *br);
+int fpga_bridge_register(struct fpga_bridge *br);
+void fpga_bridge_unregister(struct fpga_bridge *br);
 
 #endif /* _LINUX_FPGA_BRIDGE_H */
-- 
2.17.0

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH 3/4] fpga: bridge: change api, don't use drvdata
@ 2018-04-27  1:26   ` Moritz Fischer
  0 siblings, 0 replies; 24+ messages in thread
From: Moritz Fischer @ 2018-04-27  1:26 UTC (permalink / raw)
  To: linux-arm-kernel

From: Alan Tull <atull@kernel.org>

Change fpga_bridge_register to not set drvdata.  This is to support
the case where a PCIe device can have more than one bridge.

Add API functions to create/free the fpga bridge struct. Change
fpga_bridge_register/unregister to take FPGA bridge struct as
the only parameter.

  struct fpga_bridge
  *fpga_bridge_create(struct device *dev, const char *name,
                      const struct fpga_bridge_ops *br_ops,
                      void *priv);
  void fpga_bridge_free(struct fpga_bridge *br);
  int fpga_bridge_register(struct fpga_bridge *br);
  void fpga_bridge_unregister(struct fpga_bridge *br);

Update the drivers that call fpga_bridge_register with the new API.

Signed-off-by: Alan Tull <atull@kernel.org>
Reported-by: Jiuyue Ma <majiuyue@huawei.com>
Signed-off-by: Moritz Fischer <mdf@kernel.org>
---
 drivers/fpga/altera-fpga2sdram.c    | 21 ++++++---
 drivers/fpga/altera-freeze-bridge.c | 22 +++++++--
 drivers/fpga/altera-hps2fpga.c      | 24 +++++++---
 drivers/fpga/fpga-bridge.c          | 70 +++++++++++++++++++----------
 drivers/fpga/xilinx-pr-decoupler.c  | 22 ++++++---
 include/linux/fpga/fpga-bridge.h    |  9 ++--
 6 files changed, 123 insertions(+), 45 deletions(-)

diff --git a/drivers/fpga/altera-fpga2sdram.c b/drivers/fpga/altera-fpga2sdram.c
index d4eeb74388da..5a29ab6e3b28 100644
--- a/drivers/fpga/altera-fpga2sdram.c
+++ b/drivers/fpga/altera-fpga2sdram.c
@@ -106,6 +106,7 @@ static int alt_fpga_bridge_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct alt_fpga2sdram_data *priv;
+	struct fpga_bridge *br;
 	u32 enable;
 	struct regmap *sysmgr;
 	int ret = 0;
@@ -131,10 +132,18 @@ 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);
 
-	ret = fpga_bridge_register(dev, F2S_BRIDGE_NAME,
-				   &altera_fpga2sdram_br_ops, priv);
-	if (ret)
+	br = 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);
 		return ret;
+	}
 
 	dev_info(dev, "driver initialized with handoff %08x\n", priv->mask);
 
@@ -146,7 +155,7 @@ static int alt_fpga_bridge_probe(struct platform_device *pdev)
 				 (enable ? "enabling" : "disabling"));
 			ret = _alt_fpga2sdram_enable_set(priv, enable);
 			if (ret) {
-				fpga_bridge_unregister(&pdev->dev);
+				fpga_bridge_unregister(br);
 				return ret;
 			}
 		}
@@ -157,7 +166,9 @@ static int alt_fpga_bridge_probe(struct platform_device *pdev)
 
 static int alt_fpga_bridge_remove(struct platform_device *pdev)
 {
-	fpga_bridge_unregister(&pdev->dev);
+	struct fpga_bridge *br = platform_get_drvdata(pdev);
+
+	fpga_bridge_unregister(br);
 
 	return 0;
 }
diff --git a/drivers/fpga/altera-freeze-bridge.c b/drivers/fpga/altera-freeze-bridge.c
index 6159cfcf78a2..fa4b693cf4be 100644
--- a/drivers/fpga/altera-freeze-bridge.c
+++ b/drivers/fpga/altera-freeze-bridge.c
@@ -221,8 +221,10 @@ static int altera_freeze_br_probe(struct platform_device *pdev)
 	struct device_node *np = pdev->dev.of_node;
 	void __iomem *base_addr;
 	struct altera_freeze_br_data *priv;
+	struct fpga_bridge *br;
 	struct resource *res;
 	u32 status, revision;
+	int ret;
 
 	if (!np)
 		return -ENODEV;
@@ -254,13 +256,27 @@ static int altera_freeze_br_probe(struct platform_device *pdev)
 
 	priv->base_addr = base_addr;
 
-	return fpga_bridge_register(dev, FREEZE_BRIDGE_NAME,
-				    &altera_freeze_br_br_ops, priv);
+	br = 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;
 }
 
 static int altera_freeze_br_remove(struct platform_device *pdev)
 {
-	fpga_bridge_unregister(&pdev->dev);
+	struct fpga_bridge *br = platform_get_drvdata(pdev);
+
+	fpga_bridge_unregister(br);
 
 	return 0;
 }
diff --git a/drivers/fpga/altera-hps2fpga.c b/drivers/fpga/altera-hps2fpga.c
index 406d2f10741f..e4d39f0a7572 100644
--- a/drivers/fpga/altera-hps2fpga.c
+++ b/drivers/fpga/altera-hps2fpga.c
@@ -139,6 +139,7 @@ static int alt_fpga_bridge_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct altera_hps2fpga_data *priv;
 	const struct of_device_id *of_id;
+	struct fpga_bridge *br;
 	u32 enable;
 	int ret;
 
@@ -190,11 +191,24 @@ static int alt_fpga_bridge_probe(struct platform_device *pdev)
 		}
 	}
 
-	ret = fpga_bridge_register(dev, priv->name, &altera_hps2fpga_br_ops,
-				   priv);
-err:
+	br = fpga_bridge_create(dev, priv->name, &altera_hps2fpga_br_ops, priv);
+	if (!br) {
+		ret = -ENOMEM;
+		goto err;
+	}
+
+	platform_set_drvdata(pdev, br);
+
+	ret = fpga_bridge_register(br);
 	if (ret)
-		clk_disable_unprepare(priv->clk);
+		goto err_free;
+
+	return 0;
+
+err_free:
+	fpga_bridge_free(br);
+err:
+	clk_disable_unprepare(priv->clk);
 
 	return ret;
 }
@@ -204,7 +218,7 @@ static int alt_fpga_bridge_remove(struct platform_device *pdev)
 	struct fpga_bridge *bridge = platform_get_drvdata(pdev);
 	struct altera_hps2fpga_data *priv = bridge->priv;
 
-	fpga_bridge_unregister(&pdev->dev);
+	fpga_bridge_unregister(bridge);
 
 	clk_disable_unprepare(priv->clk);
 
diff --git a/drivers/fpga/fpga-bridge.c b/drivers/fpga/fpga-bridge.c
index 31bd2c59c305..2db1573507eb 100644
--- a/drivers/fpga/fpga-bridge.c
+++ b/drivers/fpga/fpga-bridge.c
@@ -328,28 +328,29 @@ static struct attribute *fpga_bridge_attrs[] = {
 ATTRIBUTE_GROUPS(fpga_bridge);
 
 /**
- * fpga_bridge_register - register a fpga bridge driver
+ * fpga_bridge_create - create and initialize a 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
  *
- * Return: 0 for success, error code otherwise.
+ * Return: struct fpga_bridge or NULL
  */
-int fpga_bridge_register(struct device *dev, const char *name,
-			 const struct fpga_bridge_ops *br_ops, void *priv)
+struct fpga_bridge *fpga_bridge_create(struct device *dev, const char *name,
+				       const struct fpga_bridge_ops *br_ops,
+				       void *priv)
 {
 	struct fpga_bridge *bridge;
 	int id, ret = 0;
 
 	if (!name || !strlen(name)) {
 		dev_err(dev, "Attempt to register with no name!\n");
-		return -EINVAL;
+		return NULL;
 	}
 
 	bridge = kzalloc(sizeof(*bridge), GFP_KERNEL);
 	if (!bridge)
-		return -ENOMEM;
+		return NULL;
 
 	id = ida_simple_get(&fpga_bridge_ida, 0, 0, GFP_KERNEL);
 	if (id < 0) {
@@ -370,40 +371,62 @@ int fpga_bridge_register(struct device *dev, const char *name,
 	bridge->dev.parent = dev;
 	bridge->dev.of_node = dev->of_node;
 	bridge->dev.id = id;
-	dev_set_drvdata(dev, bridge);
 
 	ret = dev_set_name(&bridge->dev, "br%d", id);
 	if (ret)
 		goto error_device;
 
-	ret = device_add(&bridge->dev);
-	if (ret)
-		goto error_device;
-
-	of_platform_populate(dev->of_node, NULL, NULL, dev);
-
-	dev_info(bridge->dev.parent, "fpga bridge [%s] registered\n",
-		 bridge->name);
-
-	return 0;
+	return bridge;
 
 error_device:
 	ida_simple_remove(&fpga_bridge_ida, id);
 error_kfree:
 	kfree(bridge);
 
-	return ret;
+	return NULL;
+}
+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
+ */
+void fpga_bridge_free(struct fpga_bridge *bridge)
+{
+	ida_simple_remove(&fpga_bridge_ida, bridge->dev.id);
+	kfree(bridge);
+}
+EXPORT_SYMBOL_GPL(fpga_bridge_free);
+
+/**
+ * fpga_bridge_register - register a fpga bridge
+ * @bridge:	FPGA bridge struct created by fpga_bridge_create
+ *
+ * Return: 0 for success, error code otherwise.
+ */
+int fpga_bridge_register(struct fpga_bridge *bridge)
+{
+	struct device *dev = &bridge->dev;
+	int ret;
+
+	ret = device_add(dev);
+	if (ret)
+		return ret;
+
+	of_platform_populate(dev->of_node, NULL, NULL, dev);
+
+	dev_info(dev->parent, "fpga bridge [%s] registered\n", bridge->name);
+
+	return 0;
 }
 EXPORT_SYMBOL_GPL(fpga_bridge_register);
 
 /**
  * fpga_bridge_unregister - unregister a fpga bridge driver
- * @dev: FPGA bridge device from pdev
+ * @bridge:	FPGA bridge struct created by fpga_bridge_create
  */
-void fpga_bridge_unregister(struct device *dev)
+void fpga_bridge_unregister(struct fpga_bridge *bridge)
 {
-	struct fpga_bridge *bridge = dev_get_drvdata(dev);
-
 	/*
 	 * If the low level driver provides a method for putting bridge into
 	 * a desired state upon unregister, do it.
@@ -419,8 +442,7 @@ static void fpga_bridge_dev_release(struct device *dev)
 {
 	struct fpga_bridge *bridge = to_fpga_bridge(dev);
 
-	ida_simple_remove(&fpga_bridge_ida, bridge->dev.id);
-	kfree(bridge);
+	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 0d7743089414..07ba1539e82c 100644
--- a/drivers/fpga/xilinx-pr-decoupler.c
+++ b/drivers/fpga/xilinx-pr-decoupler.c
@@ -94,6 +94,7 @@ MODULE_DEVICE_TABLE(of, xlnx_pr_decoupler_of_match);
 static int xlnx_pr_decoupler_probe(struct platform_device *pdev)
 {
 	struct xlnx_pr_decoupler_data *priv;
+	struct fpga_bridge *br;
 	int err;
 	struct resource *res;
 
@@ -120,16 +121,27 @@ static int xlnx_pr_decoupler_probe(struct platform_device *pdev)
 
 	clk_disable(priv->clk);
 
-	err = fpga_bridge_register(&pdev->dev, "Xilinx PR Decoupler",
-				   &xlnx_pr_decoupler_br_ops, priv);
+	br = fpga_bridge_create(&pdev->dev, "Xilinx PR Decoupler",
+				&xlnx_pr_decoupler_br_ops, priv);
+	if (!br) {
+		err = -ENOMEM;
+		goto err_clk;
+	}
+
+	platform_set_drvdata(pdev, br);
 
+	err = fpga_bridge_register(br);
 	if (err) {
 		dev_err(&pdev->dev, "unable to register Xilinx PR Decoupler");
-		clk_unprepare(priv->clk);
-		return err;
+		goto err_clk;
 	}
 
 	return 0;
+
+err_clk:
+	clk_unprepare(priv->clk);
+
+	return err;
 }
 
 static int xlnx_pr_decoupler_remove(struct platform_device *pdev)
@@ -137,7 +149,7 @@ static int xlnx_pr_decoupler_remove(struct platform_device *pdev)
 	struct fpga_bridge *bridge = platform_get_drvdata(pdev);
 	struct xlnx_pr_decoupler_data *p = bridge->priv;
 
-	fpga_bridge_unregister(&pdev->dev);
+	fpga_bridge_unregister(bridge);
 
 	clk_unprepare(p->clk);
 
diff --git a/include/linux/fpga/fpga-bridge.h b/include/linux/fpga/fpga-bridge.h
index 3694821a6d2d..ce550fcf6360 100644
--- a/include/linux/fpga/fpga-bridge.h
+++ b/include/linux/fpga/fpga-bridge.h
@@ -62,8 +62,11 @@ int of_fpga_bridge_get_to_list(struct device_node *np,
 			       struct fpga_image_info *info,
 			       struct list_head *bridge_list);
 
-int fpga_bridge_register(struct device *dev, const char *name,
-			 const struct fpga_bridge_ops *br_ops, void *priv);
-void fpga_bridge_unregister(struct device *dev);
+struct fpga_bridge *fpga_bridge_create(struct device *dev, const char *name,
+				       const struct fpga_bridge_ops *br_ops,
+				       void *priv);
+void fpga_bridge_free(struct fpga_bridge *br);
+int fpga_bridge_register(struct fpga_bridge *br);
+void fpga_bridge_unregister(struct fpga_bridge *br);
 
 #endif /* _LINUX_FPGA_BRIDGE_H */
-- 
2.17.0

^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH 4/4] fpga: region: change api, add fpga_region_create/free
  2018-04-27  1:26 ` Moritz Fischer
  (?)
@ 2018-04-27  1:26   ` Moritz Fischer
  -1 siblings, 0 replies; 24+ messages in thread
From: Moritz Fischer @ 2018-04-27  1:26 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-arm-kernel, linux-doc, linux-fpga, michal.simek, corbet,
	atull, Moritz Fischer

From: Alan Tull <atull@kernel.org>

Add fpga_region_create/free API functions.

Change fpga_region_register to take FPGA region struct as the only
parameter.  Change fpga_region_unregister to return void.

  struct fpga_region *fpga_region_create(struct device *dev,
                        struct fpga_manager *mgr,
                        int (*get_bridges)(struct fpga_region *));
  void fpga_region_free(struct fpga_region *region);
  int fpga_region_register(struct fpga_region *region);
  void fpga_region_unregister(struct fpga_region *region);

Remove groups storage from struct fpga_region, it's not
needed.  Callers can just "region->dev.groups = groups;"
after calling fpga_region_create.

Update the drivers that call fpga_region_register with the new API.

Signed-off-by: Alan Tull <atull@kernel.org>
Signed-off-by: Moritz Fischer <mdf@kernel.org>
---
 Documentation/fpga/fpga-region.txt |  3 +-
 drivers/fpga/fpga-region.c         | 68 ++++++++++++++++++++++++------
 drivers/fpga/of-fpga-region.c      | 13 +++---
 include/linux/fpga/fpga-region.h   | 11 +++--
 4 files changed, 68 insertions(+), 27 deletions(-)

diff --git a/Documentation/fpga/fpga-region.txt b/Documentation/fpga/fpga-region.txt
index 139a02ba1ff6..d38fa3b4154a 100644
--- a/Documentation/fpga/fpga-region.txt
+++ b/Documentation/fpga/fpga-region.txt
@@ -42,8 +42,7 @@ The FPGA region API
 To register or unregister a region:
 -----------------------------------
 
-	int fpga_region_register(struct device *dev,
-				 struct fpga_region *region);
+	int fpga_region_register(struct fpga_region *region);
 	int fpga_region_unregister(struct fpga_region *region);
 
 An example of usage can be seen in the probe function of [3]
diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c
index f634a8ed5e2c..b3ba3e40c44b 100644
--- a/drivers/fpga/fpga-region.c
+++ b/drivers/fpga/fpga-region.c
@@ -167,18 +167,36 @@ int fpga_region_program_fpga(struct fpga_region *region)
 }
 EXPORT_SYMBOL_GPL(fpga_region_program_fpga);
 
-int fpga_region_register(struct device *dev, struct fpga_region *region)
+/**
+ * fpga_region_create - alloc and init a struct fpga_region
+ * @dev: device parent
+ * @mgr: manager that programs this region
+ * @get_bridges: optional function to get bridges to a list
+ *
+ * Return: struct fpga_region or NULL
+ */
+struct fpga_region
+*fpga_region_create(struct device *dev,
+		    struct fpga_manager *mgr,
+		    int (*get_bridges)(struct fpga_region *))
 {
+	struct fpga_region *region;
 	int id, ret = 0;
 
+	region = kzalloc(sizeof(*region), GFP_KERNEL);
+	if (!region)
+		return NULL;
+
 	id = ida_simple_get(&fpga_region_ida, 0, 0, GFP_KERNEL);
 	if (id < 0)
-		return id;
+		goto err_free;
 
+	region->mgr = mgr;
+	region->get_bridges = get_bridges;
 	mutex_init(&region->mutex);
 	INIT_LIST_HEAD(&region->bridge_list);
+
 	device_initialize(&region->dev);
-	region->dev.groups = region->groups;
 	region->dev.class = fpga_region_class;
 	region->dev.parent = dev;
 	region->dev.of_node = dev->of_node;
@@ -188,23 +206,47 @@ int fpga_region_register(struct device *dev, struct fpga_region *region)
 	if (ret)
 		goto err_remove;
 
-	ret = device_add(&region->dev);
-	if (ret)
-		goto err_remove;
-
-	return 0;
+	return region;
 
 err_remove:
 	ida_simple_remove(&fpga_region_ida, id);
-	return ret;
+err_free:
+	kfree(region);
+
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(fpga_region_create);
+
+/**
+ * fpga_region_free - free a struct fpga_region
+ * @region: FPGA region created by fpga_region_create
+ */
+void fpga_region_free(struct fpga_region *region)
+{
+	ida_simple_remove(&fpga_region_ida, region->dev.id);
+	kfree(region);
+}
+EXPORT_SYMBOL_GPL(fpga_region_free);
+
+/*
+ * fpga_region_register - register a FPGA region
+ * @region: FPGA region created by fpga_region_create
+ * Return: 0 or -errno
+ */
+int fpga_region_register(struct fpga_region *region)
+{
+	return device_add(&region->dev);
+
 }
 EXPORT_SYMBOL_GPL(fpga_region_register);
 
-int fpga_region_unregister(struct fpga_region *region)
+/*
+ * fpga_region_unregister - unregister a FPGA region
+ * @region: FPGA region
+ */
+void fpga_region_unregister(struct fpga_region *region)
 {
 	device_unregister(&region->dev);
-
-	return 0;
 }
 EXPORT_SYMBOL_GPL(fpga_region_unregister);
 
@@ -212,7 +254,7 @@ static void fpga_region_dev_release(struct device *dev)
 {
 	struct fpga_region *region = to_fpga_region(dev);
 
-	ida_simple_remove(&fpga_region_ida, region->dev.id);
+	fpga_region_free(region);
 }
 
 /**
diff --git a/drivers/fpga/of-fpga-region.c b/drivers/fpga/of-fpga-region.c
index 35e7e8c4a0cb..9d681a1c5738 100644
--- a/drivers/fpga/of-fpga-region.c
+++ b/drivers/fpga/of-fpga-region.c
@@ -422,20 +422,15 @@ static int of_fpga_region_probe(struct platform_device *pdev)
 	if (IS_ERR(mgr))
 		return -EPROBE_DEFER;
 
-	region = devm_kzalloc(dev, sizeof(*region), GFP_KERNEL);
+	region = fpga_region_create(dev, mgr, of_fpga_region_get_bridges);
 	if (!region) {
 		ret = -ENOMEM;
 		goto eprobe_mgr_put;
 	}
 
-	region->mgr = mgr;
-
-	/* Specify how to get bridges for this type of region. */
-	region->get_bridges = of_fpga_region_get_bridges;
-
-	ret = fpga_region_register(dev, region);
+	ret = fpga_region_register(region);
 	if (ret)
-		goto eprobe_mgr_put;
+		goto eprobe_free;
 
 	of_platform_populate(np, fpga_region_of_match, NULL, &region->dev);
 	dev_set_drvdata(dev, region);
@@ -444,6 +439,8 @@ 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 b6520318ab9c..f2e215bd1330 100644
--- a/include/linux/fpga/fpga-region.h
+++ b/include/linux/fpga/fpga-region.h
@@ -14,7 +14,6 @@
  * @info: FPGA image info
  * @priv: private data
  * @get_bridges: optional function to get bridges to a list
- * @groups: optional attribute groups.
  */
 struct fpga_region {
 	struct device dev;
@@ -24,7 +23,6 @@ struct fpga_region {
 	struct fpga_image_info *info;
 	void *priv;
 	int (*get_bridges)(struct fpga_region *region);
-	const struct attribute_group **groups;
 };
 
 #define to_fpga_region(d) container_of(d, struct fpga_region, dev)
@@ -34,7 +32,12 @@ struct fpga_region *fpga_region_class_find(
 	int (*match)(struct device *, const void *));
 
 int fpga_region_program_fpga(struct fpga_region *region);
-int fpga_region_register(struct device *dev, struct fpga_region *region);
-int fpga_region_unregister(struct fpga_region *region);
+
+struct fpga_region
+*fpga_region_create(struct device *dev, struct fpga_manager *mgr,
+		    int (*get_bridges)(struct fpga_region *));
+void fpga_region_free(struct fpga_region *region);
+int fpga_region_register(struct fpga_region *region);
+void fpga_region_unregister(struct fpga_region *region);
 
 #endif /* _FPGA_REGION_H */
-- 
2.17.0

^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH 4/4] fpga: region: change api, add fpga_region_create/free
@ 2018-04-27  1:26   ` Moritz Fischer
  0 siblings, 0 replies; 24+ messages in thread
From: Moritz Fischer @ 2018-04-27  1:26 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-arm-kernel, linux-doc, linux-fpga, michal.simek, corbet,
	atull, Moritz Fischer

From: Alan Tull <atull@kernel.org>

Add fpga_region_create/free API functions.

Change fpga_region_register to take FPGA region struct as the only
parameter.  Change fpga_region_unregister to return void.

  struct fpga_region *fpga_region_create(struct device *dev,
                        struct fpga_manager *mgr,
                        int (*get_bridges)(struct fpga_region *));
  void fpga_region_free(struct fpga_region *region);
  int fpga_region_register(struct fpga_region *region);
  void fpga_region_unregister(struct fpga_region *region);

Remove groups storage from struct fpga_region, it's not
needed.  Callers can just "region->dev.groups = groups;"
after calling fpga_region_create.

Update the drivers that call fpga_region_register with the new API.

Signed-off-by: Alan Tull <atull@kernel.org>
Signed-off-by: Moritz Fischer <mdf@kernel.org>
---
 Documentation/fpga/fpga-region.txt |  3 +-
 drivers/fpga/fpga-region.c         | 68 ++++++++++++++++++++++++------
 drivers/fpga/of-fpga-region.c      | 13 +++---
 include/linux/fpga/fpga-region.h   | 11 +++--
 4 files changed, 68 insertions(+), 27 deletions(-)

diff --git a/Documentation/fpga/fpga-region.txt b/Documentation/fpga/fpga-region.txt
index 139a02ba1ff6..d38fa3b4154a 100644
--- a/Documentation/fpga/fpga-region.txt
+++ b/Documentation/fpga/fpga-region.txt
@@ -42,8 +42,7 @@ The FPGA region API
 To register or unregister a region:
 -----------------------------------
 
-	int fpga_region_register(struct device *dev,
-				 struct fpga_region *region);
+	int fpga_region_register(struct fpga_region *region);
 	int fpga_region_unregister(struct fpga_region *region);
 
 An example of usage can be seen in the probe function of [3]
diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c
index f634a8ed5e2c..b3ba3e40c44b 100644
--- a/drivers/fpga/fpga-region.c
+++ b/drivers/fpga/fpga-region.c
@@ -167,18 +167,36 @@ int fpga_region_program_fpga(struct fpga_region *region)
 }
 EXPORT_SYMBOL_GPL(fpga_region_program_fpga);
 
-int fpga_region_register(struct device *dev, struct fpga_region *region)
+/**
+ * fpga_region_create - alloc and init a struct fpga_region
+ * @dev: device parent
+ * @mgr: manager that programs this region
+ * @get_bridges: optional function to get bridges to a list
+ *
+ * Return: struct fpga_region or NULL
+ */
+struct fpga_region
+*fpga_region_create(struct device *dev,
+		    struct fpga_manager *mgr,
+		    int (*get_bridges)(struct fpga_region *))
 {
+	struct fpga_region *region;
 	int id, ret = 0;
 
+	region = kzalloc(sizeof(*region), GFP_KERNEL);
+	if (!region)
+		return NULL;
+
 	id = ida_simple_get(&fpga_region_ida, 0, 0, GFP_KERNEL);
 	if (id < 0)
-		return id;
+		goto err_free;
 
+	region->mgr = mgr;
+	region->get_bridges = get_bridges;
 	mutex_init(&region->mutex);
 	INIT_LIST_HEAD(&region->bridge_list);
+
 	device_initialize(&region->dev);
-	region->dev.groups = region->groups;
 	region->dev.class = fpga_region_class;
 	region->dev.parent = dev;
 	region->dev.of_node = dev->of_node;
@@ -188,23 +206,47 @@ int fpga_region_register(struct device *dev, struct fpga_region *region)
 	if (ret)
 		goto err_remove;
 
-	ret = device_add(&region->dev);
-	if (ret)
-		goto err_remove;
-
-	return 0;
+	return region;
 
 err_remove:
 	ida_simple_remove(&fpga_region_ida, id);
-	return ret;
+err_free:
+	kfree(region);
+
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(fpga_region_create);
+
+/**
+ * fpga_region_free - free a struct fpga_region
+ * @region: FPGA region created by fpga_region_create
+ */
+void fpga_region_free(struct fpga_region *region)
+{
+	ida_simple_remove(&fpga_region_ida, region->dev.id);
+	kfree(region);
+}
+EXPORT_SYMBOL_GPL(fpga_region_free);
+
+/*
+ * fpga_region_register - register a FPGA region
+ * @region: FPGA region created by fpga_region_create
+ * Return: 0 or -errno
+ */
+int fpga_region_register(struct fpga_region *region)
+{
+	return device_add(&region->dev);
+
 }
 EXPORT_SYMBOL_GPL(fpga_region_register);
 
-int fpga_region_unregister(struct fpga_region *region)
+/*
+ * fpga_region_unregister - unregister a FPGA region
+ * @region: FPGA region
+ */
+void fpga_region_unregister(struct fpga_region *region)
 {
 	device_unregister(&region->dev);
-
-	return 0;
 }
 EXPORT_SYMBOL_GPL(fpga_region_unregister);
 
@@ -212,7 +254,7 @@ static void fpga_region_dev_release(struct device *dev)
 {
 	struct fpga_region *region = to_fpga_region(dev);
 
-	ida_simple_remove(&fpga_region_ida, region->dev.id);
+	fpga_region_free(region);
 }
 
 /**
diff --git a/drivers/fpga/of-fpga-region.c b/drivers/fpga/of-fpga-region.c
index 35e7e8c4a0cb..9d681a1c5738 100644
--- a/drivers/fpga/of-fpga-region.c
+++ b/drivers/fpga/of-fpga-region.c
@@ -422,20 +422,15 @@ static int of_fpga_region_probe(struct platform_device *pdev)
 	if (IS_ERR(mgr))
 		return -EPROBE_DEFER;
 
-	region = devm_kzalloc(dev, sizeof(*region), GFP_KERNEL);
+	region = fpga_region_create(dev, mgr, of_fpga_region_get_bridges);
 	if (!region) {
 		ret = -ENOMEM;
 		goto eprobe_mgr_put;
 	}
 
-	region->mgr = mgr;
-
-	/* Specify how to get bridges for this type of region. */
-	region->get_bridges = of_fpga_region_get_bridges;
-
-	ret = fpga_region_register(dev, region);
+	ret = fpga_region_register(region);
 	if (ret)
-		goto eprobe_mgr_put;
+		goto eprobe_free;
 
 	of_platform_populate(np, fpga_region_of_match, NULL, &region->dev);
 	dev_set_drvdata(dev, region);
@@ -444,6 +439,8 @@ 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 b6520318ab9c..f2e215bd1330 100644
--- a/include/linux/fpga/fpga-region.h
+++ b/include/linux/fpga/fpga-region.h
@@ -14,7 +14,6 @@
  * @info: FPGA image info
  * @priv: private data
  * @get_bridges: optional function to get bridges to a list
- * @groups: optional attribute groups.
  */
 struct fpga_region {
 	struct device dev;
@@ -24,7 +23,6 @@ struct fpga_region {
 	struct fpga_image_info *info;
 	void *priv;
 	int (*get_bridges)(struct fpga_region *region);
-	const struct attribute_group **groups;
 };
 
 #define to_fpga_region(d) container_of(d, struct fpga_region, dev)
@@ -34,7 +32,12 @@ struct fpga_region *fpga_region_class_find(
 	int (*match)(struct device *, const void *));
 
 int fpga_region_program_fpga(struct fpga_region *region);
-int fpga_region_register(struct device *dev, struct fpga_region *region);
-int fpga_region_unregister(struct fpga_region *region);
+
+struct fpga_region
+*fpga_region_create(struct device *dev, struct fpga_manager *mgr,
+		    int (*get_bridges)(struct fpga_region *));
+void fpga_region_free(struct fpga_region *region);
+int fpga_region_register(struct fpga_region *region);
+void fpga_region_unregister(struct fpga_region *region);
 
 #endif /* _FPGA_REGION_H */
-- 
2.17.0

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH 4/4] fpga: region: change api, add fpga_region_create/free
@ 2018-04-27  1:26   ` Moritz Fischer
  0 siblings, 0 replies; 24+ messages in thread
From: Moritz Fischer @ 2018-04-27  1:26 UTC (permalink / raw)
  To: linux-arm-kernel

From: Alan Tull <atull@kernel.org>

Add fpga_region_create/free API functions.

Change fpga_region_register to take FPGA region struct as the only
parameter.  Change fpga_region_unregister to return void.

  struct fpga_region *fpga_region_create(struct device *dev,
                        struct fpga_manager *mgr,
                        int (*get_bridges)(struct fpga_region *));
  void fpga_region_free(struct fpga_region *region);
  int fpga_region_register(struct fpga_region *region);
  void fpga_region_unregister(struct fpga_region *region);

Remove groups storage from struct fpga_region, it's not
needed.  Callers can just "region->dev.groups = groups;"
after calling fpga_region_create.

Update the drivers that call fpga_region_register with the new API.

Signed-off-by: Alan Tull <atull@kernel.org>
Signed-off-by: Moritz Fischer <mdf@kernel.org>
---
 Documentation/fpga/fpga-region.txt |  3 +-
 drivers/fpga/fpga-region.c         | 68 ++++++++++++++++++++++++------
 drivers/fpga/of-fpga-region.c      | 13 +++---
 include/linux/fpga/fpga-region.h   | 11 +++--
 4 files changed, 68 insertions(+), 27 deletions(-)

diff --git a/Documentation/fpga/fpga-region.txt b/Documentation/fpga/fpga-region.txt
index 139a02ba1ff6..d38fa3b4154a 100644
--- a/Documentation/fpga/fpga-region.txt
+++ b/Documentation/fpga/fpga-region.txt
@@ -42,8 +42,7 @@ The FPGA region API
 To register or unregister a region:
 -----------------------------------
 
-	int fpga_region_register(struct device *dev,
-				 struct fpga_region *region);
+	int fpga_region_register(struct fpga_region *region);
 	int fpga_region_unregister(struct fpga_region *region);
 
 An example of usage can be seen in the probe function of [3]
diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c
index f634a8ed5e2c..b3ba3e40c44b 100644
--- a/drivers/fpga/fpga-region.c
+++ b/drivers/fpga/fpga-region.c
@@ -167,18 +167,36 @@ int fpga_region_program_fpga(struct fpga_region *region)
 }
 EXPORT_SYMBOL_GPL(fpga_region_program_fpga);
 
-int fpga_region_register(struct device *dev, struct fpga_region *region)
+/**
+ * fpga_region_create - alloc and init a struct fpga_region
+ * @dev: device parent
+ * @mgr: manager that programs this region
+ * @get_bridges: optional function to get bridges to a list
+ *
+ * Return: struct fpga_region or NULL
+ */
+struct fpga_region
+*fpga_region_create(struct device *dev,
+		    struct fpga_manager *mgr,
+		    int (*get_bridges)(struct fpga_region *))
 {
+	struct fpga_region *region;
 	int id, ret = 0;
 
+	region = kzalloc(sizeof(*region), GFP_KERNEL);
+	if (!region)
+		return NULL;
+
 	id = ida_simple_get(&fpga_region_ida, 0, 0, GFP_KERNEL);
 	if (id < 0)
-		return id;
+		goto err_free;
 
+	region->mgr = mgr;
+	region->get_bridges = get_bridges;
 	mutex_init(&region->mutex);
 	INIT_LIST_HEAD(&region->bridge_list);
+
 	device_initialize(&region->dev);
-	region->dev.groups = region->groups;
 	region->dev.class = fpga_region_class;
 	region->dev.parent = dev;
 	region->dev.of_node = dev->of_node;
@@ -188,23 +206,47 @@ int fpga_region_register(struct device *dev, struct fpga_region *region)
 	if (ret)
 		goto err_remove;
 
-	ret = device_add(&region->dev);
-	if (ret)
-		goto err_remove;
-
-	return 0;
+	return region;
 
 err_remove:
 	ida_simple_remove(&fpga_region_ida, id);
-	return ret;
+err_free:
+	kfree(region);
+
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(fpga_region_create);
+
+/**
+ * fpga_region_free - free a struct fpga_region
+ * @region: FPGA region created by fpga_region_create
+ */
+void fpga_region_free(struct fpga_region *region)
+{
+	ida_simple_remove(&fpga_region_ida, region->dev.id);
+	kfree(region);
+}
+EXPORT_SYMBOL_GPL(fpga_region_free);
+
+/*
+ * fpga_region_register - register a FPGA region
+ * @region: FPGA region created by fpga_region_create
+ * Return: 0 or -errno
+ */
+int fpga_region_register(struct fpga_region *region)
+{
+	return device_add(&region->dev);
+
 }
 EXPORT_SYMBOL_GPL(fpga_region_register);
 
-int fpga_region_unregister(struct fpga_region *region)
+/*
+ * fpga_region_unregister - unregister a FPGA region
+ * @region: FPGA region
+ */
+void fpga_region_unregister(struct fpga_region *region)
 {
 	device_unregister(&region->dev);
-
-	return 0;
 }
 EXPORT_SYMBOL_GPL(fpga_region_unregister);
 
@@ -212,7 +254,7 @@ static void fpga_region_dev_release(struct device *dev)
 {
 	struct fpga_region *region = to_fpga_region(dev);
 
-	ida_simple_remove(&fpga_region_ida, region->dev.id);
+	fpga_region_free(region);
 }
 
 /**
diff --git a/drivers/fpga/of-fpga-region.c b/drivers/fpga/of-fpga-region.c
index 35e7e8c4a0cb..9d681a1c5738 100644
--- a/drivers/fpga/of-fpga-region.c
+++ b/drivers/fpga/of-fpga-region.c
@@ -422,20 +422,15 @@ static int of_fpga_region_probe(struct platform_device *pdev)
 	if (IS_ERR(mgr))
 		return -EPROBE_DEFER;
 
-	region = devm_kzalloc(dev, sizeof(*region), GFP_KERNEL);
+	region = fpga_region_create(dev, mgr, of_fpga_region_get_bridges);
 	if (!region) {
 		ret = -ENOMEM;
 		goto eprobe_mgr_put;
 	}
 
-	region->mgr = mgr;
-
-	/* Specify how to get bridges for this type of region. */
-	region->get_bridges = of_fpga_region_get_bridges;
-
-	ret = fpga_region_register(dev, region);
+	ret = fpga_region_register(region);
 	if (ret)
-		goto eprobe_mgr_put;
+		goto eprobe_free;
 
 	of_platform_populate(np, fpga_region_of_match, NULL, &region->dev);
 	dev_set_drvdata(dev, region);
@@ -444,6 +439,8 @@ 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 b6520318ab9c..f2e215bd1330 100644
--- a/include/linux/fpga/fpga-region.h
+++ b/include/linux/fpga/fpga-region.h
@@ -14,7 +14,6 @@
  * @info: FPGA image info
  * @priv: private data
  * @get_bridges: optional function to get bridges to a list
- * @groups: optional attribute groups.
  */
 struct fpga_region {
 	struct device dev;
@@ -24,7 +23,6 @@ struct fpga_region {
 	struct fpga_image_info *info;
 	void *priv;
 	int (*get_bridges)(struct fpga_region *region);
-	const struct attribute_group **groups;
 };
 
 #define to_fpga_region(d) container_of(d, struct fpga_region, dev)
@@ -34,7 +32,12 @@ struct fpga_region *fpga_region_class_find(
 	int (*match)(struct device *, const void *));
 
 int fpga_region_program_fpga(struct fpga_region *region);
-int fpga_region_register(struct device *dev, struct fpga_region *region);
-int fpga_region_unregister(struct fpga_region *region);
+
+struct fpga_region
+*fpga_region_create(struct device *dev, struct fpga_manager *mgr,
+		    int (*get_bridges)(struct fpga_region *));
+void fpga_region_free(struct fpga_region *region);
+int fpga_region_register(struct fpga_region *region);
+void fpga_region_unregister(struct fpga_region *region);
 
 #endif /* _FPGA_REGION_H */
-- 
2.17.0

^ permalink raw reply related	[flat|nested] 24+ messages in thread
* Re: [PATCH 2/4] fpga: manager: change api, don't use drvdata
  2018-04-27  1:26   ` Moritz Fischer
  (?)
@ 2018-04-27 18:26     ` Florian Fainelli
  -1 siblings, 0 replies; 24+ messages in thread
From: Florian Fainelli @ 2018-04-27 18:26 UTC (permalink / raw)
  To: Moritz Fischer, linux-kernel
  Cc: corbet, linux-fpga, atull, linux-doc, michal.simek, linux-arm-kernel

On 04/26/2018 06:26 PM, Moritz Fischer wrote:
> From: Alan Tull <atull@kernel.org>
> 
> Change fpga_mgr_register to not set or use drvdata.  This supports
> the case where a PCIe device has more than one manager.
> 
> Add fpga_mgr_create/free functions.  Change fpga_mgr_register and
> fpga_mgr_unregister functions to take the mgr struct as their only
> parameter.
> 
>   struct fpga_manager *fpga_mgr_create(struct device *dev,
>                 const char *name,
>                 const struct fpga_manager_ops *mops,
>                 void *priv);
>   void fpga_mgr_free(struct fpga_manager *mgr);
>   int fpga_mgr_register(struct fpga_manager *mgr);
>   void fpga_mgr_unregister(struct fpga_manager *mgr);
> 
> Update the drivers that call fpga_mgr_register with the new API.

Apologies for chiming in so late, this commit does not make it clear
that fpga_mgr_unregister() now also free the 'mgr' argument by calling
fpga_mgr_free(), this is kind of detail, but an API should make that
clear IMHO.

Thanks
-- 
Florian

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH 2/4] fpga: manager: change api, don't use drvdata
@ 2018-04-27 18:26     ` Florian Fainelli
  0 siblings, 0 replies; 24+ messages in thread
From: Florian Fainelli @ 2018-04-27 18:26 UTC (permalink / raw)
  To: Moritz Fischer, linux-kernel
  Cc: corbet, linux-fpga, atull, linux-doc, michal.simek, linux-arm-kernel

On 04/26/2018 06:26 PM, Moritz Fischer wrote:
> From: Alan Tull <atull@kernel.org>
> 
> Change fpga_mgr_register to not set or use drvdata.  This supports
> the case where a PCIe device has more than one manager.
> 
> Add fpga_mgr_create/free functions.  Change fpga_mgr_register and
> fpga_mgr_unregister functions to take the mgr struct as their only
> parameter.
> 
>   struct fpga_manager *fpga_mgr_create(struct device *dev,
>                 const char *name,
>                 const struct fpga_manager_ops *mops,
>                 void *priv);
>   void fpga_mgr_free(struct fpga_manager *mgr);
>   int fpga_mgr_register(struct fpga_manager *mgr);
>   void fpga_mgr_unregister(struct fpga_manager *mgr);
> 
> Update the drivers that call fpga_mgr_register with the new API.

Apologies for chiming in so late, this commit does not make it clear
that fpga_mgr_unregister() now also free the 'mgr' argument by calling
fpga_mgr_free(), this is kind of detail, but an API should make that
clear IMHO.

Thanks
-- 
Florian
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 24+ messages in thread
* [PATCH 2/4] fpga: manager: change api, don't use drvdata
@ 2018-04-27 18:26     ` Florian Fainelli
  0 siblings, 0 replies; 24+ messages in thread
From: Florian Fainelli @ 2018-04-27 18:26 UTC (permalink / raw)
  To: linux-arm-kernel

On 04/26/2018 06:26 PM, Moritz Fischer wrote:
> From: Alan Tull <atull@kernel.org>
> 
> Change fpga_mgr_register to not set or use drvdata.  This supports
> the case where a PCIe device has more than one manager.
> 
> Add fpga_mgr_create/free functions.  Change fpga_mgr_register and
> fpga_mgr_unregister functions to take the mgr struct as their only
> parameter.
> 
>   struct fpga_manager *fpga_mgr_create(struct device *dev,
>                 const char *name,
>                 const struct fpga_manager_ops *mops,
>                 void *priv);
>   void fpga_mgr_free(struct fpga_manager *mgr);
>   int fpga_mgr_register(struct fpga_manager *mgr);
>   void fpga_mgr_unregister(struct fpga_manager *mgr);
> 
> Update the drivers that call fpga_mgr_register with the new API.

Apologies for chiming in so late, this commit does not make it clear
that fpga_mgr_unregister() now also free the 'mgr' argument by calling
fpga_mgr_free(), this is kind of detail, but an API should make that
clear IMHO.

Thanks
-- 
Florian

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH 2/4] fpga: manager: change api, don't use drvdata
  2018-04-27 18:26     ` Florian Fainelli
  (?)
@ 2018-04-27 23:30       ` Alan Tull
  -1 siblings, 0 replies; 24+ messages in thread
From: Alan Tull @ 2018-04-27 23:30 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Moritz Fischer, linux-kernel, Jonathan Corbet, linux-fpga,
	linux-doc, Michal Simek,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE

On Fri, Apr 27, 2018 at 1:26 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> On 04/26/2018 06:26 PM, Moritz Fischer wrote:
>> From: Alan Tull <atull@kernel.org>
>>
>> Change fpga_mgr_register to not set or use drvdata.  This supports
>> the case where a PCIe device has more than one manager.
>>
>> Add fpga_mgr_create/free functions.  Change fpga_mgr_register and
>> fpga_mgr_unregister functions to take the mgr struct as their only
>> parameter.
>>
>>   struct fpga_manager *fpga_mgr_create(struct device *dev,
>>                 const char *name,
>>                 const struct fpga_manager_ops *mops,
>>                 void *priv);
>>   void fpga_mgr_free(struct fpga_manager *mgr);
>>   int fpga_mgr_register(struct fpga_manager *mgr);
>>   void fpga_mgr_unregister(struct fpga_manager *mgr);
>>
>> Update the drivers that call fpga_mgr_register with the new API.
>
> Apologies for chiming in so late, this commit does not make it clear
> that fpga_mgr_unregister() now also free the 'mgr' argument by calling
> fpga_mgr_free(), this is kind of detail, but an API should make that
> clear IMHO.

If people follow the usage information, in
Documentation/fpga/fpga-mgr.txt, they'll do the right thing.  But I
can add a patch that clarifies the description of fpga_mgr_unregister
in fpga-mgr.c that it "unregisters and frees" the manager.

>
> Thanks
> --
> Florian

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH 2/4] fpga: manager: change api, don't use drvdata
@ 2018-04-27 23:30       ` Alan Tull
  0 siblings, 0 replies; 24+ messages in thread
From: Alan Tull @ 2018-04-27 23:30 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Moritz Fischer, linux-kernel, Jonathan Corbet, linux-fpga,
	linux-doc, Michal Simek,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE

On Fri, Apr 27, 2018 at 1:26 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> On 04/26/2018 06:26 PM, Moritz Fischer wrote:
>> From: Alan Tull <atull@kernel.org>
>>
>> Change fpga_mgr_register to not set or use drvdata.  This supports
>> the case where a PCIe device has more than one manager.
>>
>> Add fpga_mgr_create/free functions.  Change fpga_mgr_register and
>> fpga_mgr_unregister functions to take the mgr struct as their only
>> parameter.
>>
>>   struct fpga_manager *fpga_mgr_create(struct device *dev,
>>                 const char *name,
>>                 const struct fpga_manager_ops *mops,
>>                 void *priv);
>>   void fpga_mgr_free(struct fpga_manager *mgr);
>>   int fpga_mgr_register(struct fpga_manager *mgr);
>>   void fpga_mgr_unregister(struct fpga_manager *mgr);
>>
>> Update the drivers that call fpga_mgr_register with the new API.
>
> Apologies for chiming in so late, this commit does not make it clear
> that fpga_mgr_unregister() now also free the 'mgr' argument by calling
> fpga_mgr_free(), this is kind of detail, but an API should make that
> clear IMHO.

If people follow the usage information, in
Documentation/fpga/fpga-mgr.txt, they'll do the right thing.  But I
can add a patch that clarifies the description of fpga_mgr_unregister
in fpga-mgr.c that it "unregisters and frees" the manager.

>
> Thanks
> --
> Florian
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 24+ messages in thread
* [PATCH 2/4] fpga: manager: change api, don't use drvdata
@ 2018-04-27 23:30       ` Alan Tull
  0 siblings, 0 replies; 24+ messages in thread
From: Alan Tull @ 2018-04-27 23:30 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Apr 27, 2018 at 1:26 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> On 04/26/2018 06:26 PM, Moritz Fischer wrote:
>> From: Alan Tull <atull@kernel.org>
>>
>> Change fpga_mgr_register to not set or use drvdata.  This supports
>> the case where a PCIe device has more than one manager.
>>
>> Add fpga_mgr_create/free functions.  Change fpga_mgr_register and
>> fpga_mgr_unregister functions to take the mgr struct as their only
>> parameter.
>>
>>   struct fpga_manager *fpga_mgr_create(struct device *dev,
>>                 const char *name,
>>                 const struct fpga_manager_ops *mops,
>>                 void *priv);
>>   void fpga_mgr_free(struct fpga_manager *mgr);
>>   int fpga_mgr_register(struct fpga_manager *mgr);
>>   void fpga_mgr_unregister(struct fpga_manager *mgr);
>>
>> Update the drivers that call fpga_mgr_register with the new API.
>
> Apologies for chiming in so late, this commit does not make it clear
> that fpga_mgr_unregister() now also free the 'mgr' argument by calling
> fpga_mgr_free(), this is kind of detail, but an API should make that
> clear IMHO.

If people follow the usage information, in
Documentation/fpga/fpga-mgr.txt, they'll do the right thing.  But I
can add a patch that clarifies the description of fpga_mgr_unregister
in fpga-mgr.c that it "unregisters and frees" the manager.

>
> Thanks
> --
> Florian

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH 2/4] fpga: manager: change api, don't use drvdata
  2018-04-27 23:30       ` Alan Tull
  (?)
@ 2018-04-27 23:38         ` Florian Fainelli
  -1 siblings, 0 replies; 24+ messages in thread
From: Florian Fainelli @ 2018-04-27 23:38 UTC (permalink / raw)
  To: Alan Tull
  Cc: Moritz Fischer, linux-kernel, Jonathan Corbet, linux-fpga,
	linux-doc, Michal Simek,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE

On 04/27/2018 04:30 PM, Alan Tull wrote:
> On Fri, Apr 27, 2018 at 1:26 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
>> On 04/26/2018 06:26 PM, Moritz Fischer wrote:
>>> From: Alan Tull <atull@kernel.org>
>>>
>>> Change fpga_mgr_register to not set or use drvdata.  This supports
>>> the case where a PCIe device has more than one manager.
>>>
>>> Add fpga_mgr_create/free functions.  Change fpga_mgr_register and
>>> fpga_mgr_unregister functions to take the mgr struct as their only
>>> parameter.
>>>
>>>   struct fpga_manager *fpga_mgr_create(struct device *dev,
>>>                 const char *name,
>>>                 const struct fpga_manager_ops *mops,
>>>                 void *priv);
>>>   void fpga_mgr_free(struct fpga_manager *mgr);
>>>   int fpga_mgr_register(struct fpga_manager *mgr);
>>>   void fpga_mgr_unregister(struct fpga_manager *mgr);
>>>
>>> Update the drivers that call fpga_mgr_register with the new API.
>>
>> Apologies for chiming in so late, this commit does not make it clear
>> that fpga_mgr_unregister() now also free the 'mgr' argument by calling
>> fpga_mgr_free(), this is kind of detail, but an API should make that
>> clear IMHO.
> 
> If people follow the usage information, in
> Documentation/fpga/fpga-mgr.txt, they'll do the right thing.  But I
> can add a patch that clarifies the description of fpga_mgr_unregister
> in fpga-mgr.c that it "unregisters and frees" the manager.

Just mentioning that because not all APIs do this, take the network
devices: there is an unregister_netdev() and a free_netdev(). Either way
is fine with me as long as it is documented as such, I had to look at
the API implementation to figure out that, no, all the drivers were not
leaking their fpga_manager instance in their .remove() function :)
-- 
Florian

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH 2/4] fpga: manager: change api, don't use drvdata
@ 2018-04-27 23:38         ` Florian Fainelli
  0 siblings, 0 replies; 24+ messages in thread
From: Florian Fainelli @ 2018-04-27 23:38 UTC (permalink / raw)
  To: Alan Tull
  Cc: Moritz Fischer, linux-kernel, Jonathan Corbet, linux-fpga,
	linux-doc, Michal Simek,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE

On 04/27/2018 04:30 PM, Alan Tull wrote:
> On Fri, Apr 27, 2018 at 1:26 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
>> On 04/26/2018 06:26 PM, Moritz Fischer wrote:
>>> From: Alan Tull <atull@kernel.org>
>>>
>>> Change fpga_mgr_register to not set or use drvdata.  This supports
>>> the case where a PCIe device has more than one manager.
>>>
>>> Add fpga_mgr_create/free functions.  Change fpga_mgr_register and
>>> fpga_mgr_unregister functions to take the mgr struct as their only
>>> parameter.
>>>
>>>   struct fpga_manager *fpga_mgr_create(struct device *dev,
>>>                 const char *name,
>>>                 const struct fpga_manager_ops *mops,
>>>                 void *priv);
>>>   void fpga_mgr_free(struct fpga_manager *mgr);
>>>   int fpga_mgr_register(struct fpga_manager *mgr);
>>>   void fpga_mgr_unregister(struct fpga_manager *mgr);
>>>
>>> Update the drivers that call fpga_mgr_register with the new API.
>>
>> Apologies for chiming in so late, this commit does not make it clear
>> that fpga_mgr_unregister() now also free the 'mgr' argument by calling
>> fpga_mgr_free(), this is kind of detail, but an API should make that
>> clear IMHO.
> 
> If people follow the usage information, in
> Documentation/fpga/fpga-mgr.txt, they'll do the right thing.  But I
> can add a patch that clarifies the description of fpga_mgr_unregister
> in fpga-mgr.c that it "unregisters and frees" the manager.

Just mentioning that because not all APIs do this, take the network
devices: there is an unregister_netdev() and a free_netdev(). Either way
is fine with me as long as it is documented as such, I had to look at
the API implementation to figure out that, no, all the drivers were not
leaking their fpga_manager instance in their .remove() function :)
-- 
Florian
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 24+ messages in thread
* [PATCH 2/4] fpga: manager: change api, don't use drvdata
@ 2018-04-27 23:38         ` Florian Fainelli
  0 siblings, 0 replies; 24+ messages in thread
From: Florian Fainelli @ 2018-04-27 23:38 UTC (permalink / raw)
  To: linux-arm-kernel

On 04/27/2018 04:30 PM, Alan Tull wrote:
> On Fri, Apr 27, 2018 at 1:26 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
>> On 04/26/2018 06:26 PM, Moritz Fischer wrote:
>>> From: Alan Tull <atull@kernel.org>
>>>
>>> Change fpga_mgr_register to not set or use drvdata.  This supports
>>> the case where a PCIe device has more than one manager.
>>>
>>> Add fpga_mgr_create/free functions.  Change fpga_mgr_register and
>>> fpga_mgr_unregister functions to take the mgr struct as their only
>>> parameter.
>>>
>>>   struct fpga_manager *fpga_mgr_create(struct device *dev,
>>>                 const char *name,
>>>                 const struct fpga_manager_ops *mops,
>>>                 void *priv);
>>>   void fpga_mgr_free(struct fpga_manager *mgr);
>>>   int fpga_mgr_register(struct fpga_manager *mgr);
>>>   void fpga_mgr_unregister(struct fpga_manager *mgr);
>>>
>>> Update the drivers that call fpga_mgr_register with the new API.
>>
>> Apologies for chiming in so late, this commit does not make it clear
>> that fpga_mgr_unregister() now also free the 'mgr' argument by calling
>> fpga_mgr_free(), this is kind of detail, but an API should make that
>> clear IMHO.
> 
> If people follow the usage information, in
> Documentation/fpga/fpga-mgr.txt, they'll do the right thing.  But I
> can add a patch that clarifies the description of fpga_mgr_unregister
> in fpga-mgr.c that it "unregisters and frees" the manager.

Just mentioning that because not all APIs do this, take the network
devices: there is an unregister_netdev() and a free_netdev(). Either way
is fine with me as long as it is documented as such, I had to look at
the API implementation to figure out that, no, all the drivers were not
leaking their fpga_manager instance in their .remove() function :)
-- 
Florian

^ permalink raw reply	[flat|nested] 24+ messages in thread
end of thread, other threads:[~2018-04-27 23:38 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-27  1:26 [PATCH 0/4] FPGA Manager Patches for 4.18 Moritz Fischer
2018-04-27  1:26 ` Moritz Fischer
2018-04-27  1:26 ` Moritz Fischer
2018-04-27  1:26 ` [PATCH 1/4] fpga: region: don't use drvdata in common fpga code Moritz Fischer
2018-04-27  1:26   ` Moritz Fischer
2018-04-27  1:26   ` Moritz Fischer
2018-04-27  1:26 ` [PATCH 2/4] fpga: manager: change api, don't use drvdata Moritz Fischer
2018-04-27  1:26   ` Moritz Fischer
2018-04-27  1:26   ` Moritz Fischer
2018-04-27 18:26   ` Florian Fainelli
2018-04-27 18:26     ` Florian Fainelli
2018-04-27 18:26     ` Florian Fainelli
2018-04-27 23:30     ` Alan Tull
2018-04-27 23:30       ` Alan Tull
2018-04-27 23:30       ` Alan Tull
2018-04-27 23:38       ` Florian Fainelli
2018-04-27 23:38         ` Florian Fainelli
2018-04-27 23:38         ` Florian Fainelli
2018-04-27  1:26 ` [PATCH 3/4] fpga: bridge: " Moritz Fischer
2018-04-27  1:26   ` Moritz Fischer
2018-04-27  1:26   ` Moritz Fischer
2018-04-27  1:26 ` [PATCH 4/4] fpga: region: change api, add fpga_region_create/free Moritz Fischer
2018-04-27  1:26   ` Moritz Fischer
2018-04-27  1:26   ` Moritz Fischer

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