2018-09-26 16:12:54

by Alan Tull

Subject: [PATCH v3 0/4] fpga: add devm managed create APIs

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

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

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

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

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

--
2.7.4

2018-09-26 16:13:00

by Alan Tull

Subject: [PATCH v3 4/4] docs: fpga: document programming fpgas using regions

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

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

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

diff --git a/Documentation/driver-api/fpga/fpga-bridge.rst b/Documentation/driver-api/fpga/fpga-bridge.rst
index ebbcbde..71c5a40 100644
--- a/Documentation/driver-api/fpga/fpga-bridge.rst
+++ b/Documentation/driver-api/fpga/fpga-bridge.rst
@@ -4,6 +4,12 @@ FPGA Bridge
API to implement a new FPGA bridge
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

+* struct :c:type:`fpga_bridge` — The FPGA Bridge structure
+* struct :c:type:`fpga_bridge_ops` — Low level Bridge driver ops
+* :c:func:`devm_fpga_bridge_create()` — Allocate and init a bridge struct
+* :c:func:`fpga_bridge_register()` — Register a bridge
+* :c:func:`fpga_bridge_unregister()` — Unregister a bridge
+
.. kernel-doc:: include/linux/fpga/fpga-bridge.h
:functions: fpga_bridge

@@ -14,39 +20,7 @@ API to implement a new FPGA bridge
:functions: devm_fpga_bridge_create

.. kernel-doc:: drivers/fpga/fpga-bridge.c
- :functions: fpga_bridge_create
-
-.. kernel-doc:: drivers/fpga/fpga-bridge.c
- :functions: fpga_bridge_free
-
-.. kernel-doc:: drivers/fpga/fpga-bridge.c
:functions: fpga_bridge_register

.. kernel-doc:: drivers/fpga/fpga-bridge.c
:functions: fpga_bridge_unregister
-
-API to control an FPGA bridge
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-
-You probably won't need these directly. FPGA regions should handle this.
-
-.. kernel-doc:: drivers/fpga/fpga-bridge.c
- :functions: of_fpga_bridge_get
-
-.. kernel-doc:: drivers/fpga/fpga-bridge.c
- :functions: fpga_bridge_get
-
-.. kernel-doc:: drivers/fpga/fpga-bridge.c
- :functions: fpga_bridge_put
-
-.. kernel-doc:: drivers/fpga/fpga-bridge.c
- :functions: fpga_bridge_get_to_list
-
-.. kernel-doc:: drivers/fpga/fpga-bridge.c
- :functions: of_fpga_bridge_get_to_list
-
-.. kernel-doc:: drivers/fpga/fpga-bridge.c
- :functions: fpga_bridge_enable
-
-.. kernel-doc:: drivers/fpga/fpga-bridge.c
- :functions: fpga_bridge_disable
diff --git a/Documentation/driver-api/fpga/fpga-mgr.rst b/Documentation/driver-api/fpga/fpga-mgr.rst
index db8885e..576f194 100644
--- a/Documentation/driver-api/fpga/fpga-mgr.rst
+++ b/Documentation/driver-api/fpga/fpga-mgr.rst
@@ -98,67 +98,19 @@ The ops include a .state function which will determine the state the FPGA is in
and return a code of type enum fpga_mgr_states. It doesn't result in a change
in state.

-How to write an image buffer to a supported FPGA
-------------------------------------------------
-
-Some sample code::
-
- #include <linux/fpga/fpga-mgr.h>
-
- struct fpga_manager *mgr;
- struct fpga_image_info *info;
- int ret;
-
- /*
- * Get a reference to FPGA manager. The manager is not locked, so you can
- * hold onto this reference without it preventing programming.
- *
- * This example uses the device node of the manager. Alternatively, use
- * fpga_mgr_get(dev) instead if you have the device.
- */
- mgr = of_fpga_mgr_get(mgr_node);
-
- /* struct with information about the FPGA image to program. */
- info = fpga_image_info_alloc(dev);
-
- /* flags indicates whether to do full or partial reconfiguration */
- info->flags = FPGA_MGR_PARTIAL_RECONFIG;
-
- /*
- * At this point, indicate where the image is. This is pseudo-code; you're
- * going to use one of these three.
- */
- if (image is in a scatter gather table) {
-
- info->sgt = [your scatter gather table]
-
- } else if (image is in a buffer) {
-
- info->buf = [your image buffer]
- info->count = [image buffer size]
-
- } else if (image is in a firmware file) {
-
- info->firmware_name = devm_kstrdup(dev, firmware_name, GFP_KERNEL);
-
- }
-
- /* Get exclusive control of FPGA manager */
- ret = fpga_mgr_lock(mgr);
-
- /* Load the buffer to the FPGA */
- ret = fpga_mgr_buf_load(mgr, &info, buf, count);
-
- /* Release the FPGA manager */
- fpga_mgr_unlock(mgr);
- fpga_mgr_put(mgr);
-
- /* Deallocate the image info if you're done with it */
- fpga_image_info_free(info);
-
API for implementing a new FPGA Manager driver
----------------------------------------------

+* ``fpga_mgr_states`` — Values for :c:member:`fpga_manager->state`.
+* struct :c:type:`fpga_manager` — the FPGA manager struct
+* struct :c:type:`fpga_manager_ops` — Low level FPGA manager driver ops
+* :c:func:`devm_fpga_mgr_create` — Allocate and init a manager struct
+* :c:func:`fpga_mgr_register` — Register an FPGA manager
+* :c:func:`fpga_mgr_unregister` — Unregister an FPGA manager
+
+.. kernel-doc:: include/linux/fpga/fpga-mgr.h
+ :functions: fpga_mgr_states
+
.. kernel-doc:: include/linux/fpga/fpga-mgr.h
:functions: fpga_manager

@@ -169,56 +121,7 @@ API for implementing a new FPGA Manager driver
:functions: devm_fpga_mgr_create

.. kernel-doc:: drivers/fpga/fpga-mgr.c
- :functions: fpga_mgr_create
-
-.. kernel-doc:: drivers/fpga/fpga-mgr.c
- :functions: fpga_mgr_free
-
-.. kernel-doc:: drivers/fpga/fpga-mgr.c
:functions: fpga_mgr_register

.. kernel-doc:: drivers/fpga/fpga-mgr.c
:functions: fpga_mgr_unregister
-
-API for programming an FPGA
----------------------------
-
-FPGA Manager flags
-
-.. kernel-doc:: include/linux/fpga/fpga-mgr.h
- :doc: FPGA Manager flags
-
-.. kernel-doc:: include/linux/fpga/fpga-mgr.h
- :functions: fpga_image_info
-
-.. kernel-doc:: include/linux/fpga/fpga-mgr.h
- :functions: fpga_mgr_states
-
-.. kernel-doc:: drivers/fpga/fpga-mgr.c
- :functions: fpga_image_info_alloc
-
-.. kernel-doc:: drivers/fpga/fpga-mgr.c
- :functions: fpga_image_info_free
-
-.. kernel-doc:: drivers/fpga/fpga-mgr.c
- :functions: of_fpga_mgr_get
-
-.. kernel-doc:: drivers/fpga/fpga-mgr.c
- :functions: fpga_mgr_get
-
-.. kernel-doc:: drivers/fpga/fpga-mgr.c
- :functions: fpga_mgr_put
-
-.. kernel-doc:: drivers/fpga/fpga-mgr.c
- :functions: fpga_mgr_lock
-
-.. kernel-doc:: drivers/fpga/fpga-mgr.c
- :functions: fpga_mgr_unlock
-
-.. kernel-doc:: include/linux/fpga/fpga-mgr.h
- :functions: fpga_mgr_states
-
-Note - use :c:func:`fpga_region_program_fpga()` instead of :c:func:`fpga_mgr_load()`
-
-.. kernel-doc:: drivers/fpga/fpga-mgr.c
- :functions: fpga_mgr_load
diff --git a/Documentation/driver-api/fpga/fpga-programming.rst b/Documentation/driver-api/fpga/fpga-programming.rst
new file mode 100644
index 0000000..b5484df
--- /dev/null
+++ b/Documentation/driver-api/fpga/fpga-programming.rst
@@ -0,0 +1,107 @@
+In-kernel API for FPGA Programming
+==================================
+
+Overview
+--------
+
+The in-kernel API for FPGA programming is a combination of APIs from
+FPGA manager, bridge, and regions. The actual function used to
+trigger FPGA programming is :c:func:`fpga_region_program_fpga()`.
+
+:c:func:`fpga_region_program_fpga()` uses functionality supplied by
+the FPGA manager and bridges. It will:
+
+ * lock the region's mutex
+ * lock the mutex of the region's FPGA manager
+ * build a list of FPGA bridges if a method has been specified to do so
+ * disable the bridges
+ * program the FPGA using info passed in :c:member:`fpga_region->info`.
+ * re-enable the bridges
+ * release the locks
+
+The struct fpga_image_info specifies what FPGA image to program. It is
+allocated/freed by :c:func:`fpga_image_info_alloc()` and freed with
+:c:func:`fpga_image_info_free()`
+
+How to program an FPGA using a region
+-------------------------------------
+
+When the FPGA region driver probed, it was given a pointer to an FPGA manager
+driver so it knows which manager to use. The region also either has a list of
+bridges to control during programming or it has a pointer to a function that
+will generate that list. Here's some sample code of what to do next::
+
+ #include <linux/fpga/fpga-mgr.h>
+ #include <linux/fpga/fpga-region.h>
+
+ struct fpga_image_info *info;
+ int ret;
+
+ /*
+ * First, alloc the struct with information about the FPGA image to
+ * program.
+ */
+ info = fpga_image_info_alloc(dev);
+ if (!info)
+ return -ENOMEM;
+
+ /* Set flags as needed, such as: */
+ info->flags = FPGA_MGR_PARTIAL_RECONFIG;
+
+ /*
+ * Indicate where the FPGA image is. This is pseudo-code; you're
+ * going to use one of these three.
+ */
+ if (image is in a scatter gather table) {
+
+ info->sgt = [your scatter gather table]
+
+ } else if (image is in a buffer) {
+
+ info->buf = [your image buffer]
+ info->count = [image buffer size]
+
+ } else if (image is in a firmware file) {
+
+ info->firmware_name = devm_kstrdup(dev, firmware_name,
+ GFP_KERNEL);
+
+ }
+
+ /* Add info to region and do the programming */
+ region->info = info;
+ ret = fpga_region_program_fpga(region);
+
+ /* Deallocate the image info if you're done with it */
+ region->info = NULL;
+ fpga_image_info_free(info);
+
+ if (ret)
+ return ret;
+
+ /* Now enumerate whatever hardware has appeared in the FPGA. */
+
+API for programming an FPGA
+---------------------------
+
+* :c:func:`fpga_region_program_fpga` — Program an FPGA
+* :c:type:`fpga_image_info` — Specifies what FPGA image to program
+* :c:func:`fpga_image_info_alloc()` — Allocate an FPGA image info struct
+* :c:func:`fpga_image_info_free()` — Free an FPGA image info struct
+
+.. kernel-doc:: drivers/fpga/fpga-region.c
+ :functions: fpga_region_program_fpga
+
+FPGA Manager flags
+
+.. kernel-doc:: include/linux/fpga/fpga-mgr.h
+ :doc: FPGA Manager flags
+
+.. kernel-doc:: include/linux/fpga/fpga-mgr.h
+ :functions: fpga_image_info
+
+.. kernel-doc:: drivers/fpga/fpga-mgr.c
+ :functions: fpga_image_info_alloc
+
+.. kernel-doc:: drivers/fpga/fpga-mgr.c
+ :functions: fpga_image_info_free
diff --git a/Documentation/driver-api/fpga/fpga-region.rst b/Documentation/driver-api/fpga/fpga-region.rst
index dc9f75c..0529b2d 100644
--- a/Documentation/driver-api/fpga/fpga-region.rst
+++ b/Documentation/driver-api/fpga/fpga-region.rst
@@ -34,41 +34,6 @@ fpga_image_info including:
* flags indicating specifics such as whether the image is for partial
reconfiguration.

-How to program an FPGA using a region
--------------------------------------
-
-First, allocate the info struct::
-
- info = fpga_image_info_alloc(dev);
- if (!info)
- return -ENOMEM;
-
-Set flags as needed, i.e.::
-
- info->flags |= FPGA_MGR_PARTIAL_RECONFIG;
-
-Point to your FPGA image, such as::
-
- info->sgt = &sgt;
-
-Add info to region and do the programming::
-
- region->info = info;
- ret = fpga_region_program_fpga(region);
-
-:c:func:`fpga_region_program_fpga()` operates on info passed in the
-fpga_image_info (region->info). This function will attempt to:
-
- * lock the region's mutex
- * lock the region's FPGA manager
- * build a list of FPGA bridges if a method has been specified to do so
- * disable the bridges
- * program the FPGA
- * re-enable the bridges
- * release the locks
-
-Then you will want to enumerate whatever hardware has appeared in the FPGA.
-
How to add a new FPGA region
----------------------------

@@ -77,15 +42,36 @@ An example of usage can be seen in the probe function of [#f2]_.
.. [#f1] ../devicetree/bindings/fpga/fpga-region.txt
.. [#f2] ../../drivers/fpga/of-fpga-region.c

-API to program an FPGA
-----------------------
-
-.. kernel-doc:: drivers/fpga/fpga-region.c
- :functions: fpga_region_program_fpga
-
API to add a new FPGA region
----------------------------

+* struct :c:type:`fpga_region` — The FPGA region struct
+* :c:func:`devm_fpga_region_create` — Allocate and init a region struct
+* :c:func:`fpga_region_register` — Register an FPGA region
+* :c:func:`fpga_region_unregister` — Unregister an FPGA region
+
+The FPGA region's probe function will need to get a reference to the FPGA
+Manager it will be using to do the programming. This usually would happen
+during the region's probe function.
+
+* :c:func:`fpga_mgr_get` — Get a reference to an FPGA manager, raise ref count
+* :c:func:`of_fpga_mgr_get` — Get a reference to an FPGA manager, raise ref count,
+ given a device node.
+* :c:func:`fpga_mgr_put` — Put an FPGA manager
+
+The FPGA region will need to specify which bridges to control while programming
+the FPGA. The region driver can build a list of bridges during probe time
+(:c:member:`fpga_region->bridge_list`) or it can have a function that creates
+the list of bridges to program just before programming
+(:c:member:`fpga_region->get_bridges`). The FPGA bridge framework supplies the
+following APIs to handle building or tearing down that list.
+
+* :c:func:`fpga_bridge_get_to_list` — Get a ref of an FPGA bridge, add it to a
+ list
+* :c:func:`of_fpga_bridge_get_to_list` — Get a ref of an FPGA bridge, add it to a
+ list, given a device node
+* :c:func:`fpga_bridges_put` — Given a list of bridges, put them
+
.. kernel-doc:: include/linux/fpga/fpga-region.h
:functions: fpga_region

@@ -93,13 +79,25 @@ API to add a new FPGA region
:functions: devm_fpga_region_create

.. kernel-doc:: drivers/fpga/fpga-region.c
- :functions: fpga_region_create
-
-.. kernel-doc:: drivers/fpga/fpga-region.c
- :functions: fpga_region_free
-
-.. kernel-doc:: drivers/fpga/fpga-region.c
:functions: fpga_region_register

.. kernel-doc:: drivers/fpga/fpga-region.c
:functions: fpga_region_unregister
+
+.. kernel-doc:: drivers/fpga/fpga-mgr.c
+ :functions: fpga_mgr_get
+
+.. kernel-doc:: drivers/fpga/fpga-mgr.c
+ :functions: of_fpga_mgr_get
+
+.. kernel-doc:: drivers/fpga/fpga-mgr.c
+ :functions: fpga_mgr_put
+
+.. kernel-doc:: drivers/fpga/fpga-bridge.c
+ :functions: fpga_bridge_get_to_list
+
+.. kernel-doc:: drivers/fpga/fpga-bridge.c
+ :functions: of_fpga_bridge_get_to_list
+
+.. kernel-doc:: drivers/fpga/fpga-bridge.c
+ :functions: fpga_bridges_put
diff --git a/Documentation/driver-api/fpga/index.rst b/Documentation/driver-api/fpga/index.rst
index c51e5eb..31a4773 100644
--- a/Documentation/driver-api/fpga/index.rst
+++ b/Documentation/driver-api/fpga/index.rst
@@ -11,3 +11,5 @@ FPGA Subsystem
fpga-mgr
fpga-bridge
fpga-region
+ fpga-programming
+
diff --git a/Documentation/driver-api/fpga/intro.rst b/Documentation/driver-api/fpga/intro.rst
index 50d1cab..f54c7da 100644
--- a/Documentation/driver-api/fpga/intro.rst
+++ b/Documentation/driver-api/fpga/intro.rst
@@ -44,7 +44,7 @@ FPGA Region
-----------

If you are adding a new interface to the FPGA framework, add it on top
-of an FPGA region to allow the most reuse of your interface.
+of an FPGA region.

The FPGA Region framework (fpga-region.c) associates managers and
bridges as reconfigurable regions. A region may refer to the whole
--
2.7.4

2018-09-26 16:13:07

by Alan Tull

Subject: [PATCH v3 2/4] fpga: bridge: add devm_fpga_bridge_create

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

Change current bridge drivers to use
devm_fpga_bridge_create().

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

diff --git a/Documentation/driver-api/fpga/fpga-bridge.rst b/Documentation/driver-api/fpga/fpga-bridge.rst
index 2c2aaca..ebbcbde 100644
--- a/Documentation/driver-api/fpga/fpga-bridge.rst
+++ b/Documentation/driver-api/fpga/fpga-bridge.rst
@@ -11,6 +11,9 @@ API to implement a new FPGA bridge
:functions: fpga_bridge_ops

.. kernel-doc:: drivers/fpga/fpga-bridge.c
+ :functions: devm_fpga_bridge_create
+
+.. kernel-doc:: drivers/fpga/fpga-bridge.c
:functions: fpga_bridge_create

.. kernel-doc:: drivers/fpga/fpga-bridge.c
diff --git a/drivers/fpga/altera-fpga2sdram.c b/drivers/fpga/altera-fpga2sdram.c
index 23660cc..a78e49c 100644
--- a/drivers/fpga/altera-fpga2sdram.c
+++ b/drivers/fpga/altera-fpga2sdram.c
@@ -121,18 +121,16 @@ static int alt_fpga_bridge_probe(struct platform_device *pdev)
/* Get f2s bridge configuration saved in handoff register */
regmap_read(sysmgr, SYSMGR_ISWGRP_HANDOFF3, &priv->mask);

- br = fpga_bridge_create(dev, F2S_BRIDGE_NAME,
- &altera_fpga2sdram_br_ops, priv);
+ br = devm_fpga_bridge_create(dev, F2S_BRIDGE_NAME,
+ &altera_fpga2sdram_br_ops, priv);
if (!br)
return -ENOMEM;

platform_set_drvdata(pdev, br);

ret = fpga_bridge_register(br);
- if (ret) {
- fpga_bridge_free(br);
+ if (ret)
return ret;
- }

dev_info(dev, "driver initialized with handoff %08x\n", priv->mask);

diff --git a/drivers/fpga/altera-freeze-bridge.c b/drivers/fpga/altera-freeze-bridge.c
index ffd586c..dd58c4a 100644
--- a/drivers/fpga/altera-freeze-bridge.c
+++ b/drivers/fpga/altera-freeze-bridge.c
@@ -213,7 +213,6 @@ static int altera_freeze_br_probe(struct platform_device *pdev)
struct fpga_bridge *br;
struct resource *res;
u32 status, revision;
- int ret;

if (!np)
return -ENODEV;
@@ -245,20 +244,14 @@ static int altera_freeze_br_probe(struct platform_device *pdev)

priv->base_addr = base_addr;

- br = fpga_bridge_create(dev, FREEZE_BRIDGE_NAME,
- &altera_freeze_br_br_ops, priv);
+ br = devm_fpga_bridge_create(dev, FREEZE_BRIDGE_NAME,
+ &altera_freeze_br_br_ops, priv);
if (!br)
return -ENOMEM;

platform_set_drvdata(pdev, br);

- ret = fpga_bridge_register(br);
- if (ret) {
- fpga_bridge_free(br);
- return ret;
- }
-
- return 0;
+ return fpga_bridge_register(br);
}

static int altera_freeze_br_remove(struct platform_device *pdev)
diff --git a/drivers/fpga/altera-hps2fpga.c b/drivers/fpga/altera-hps2fpga.c
index a974d3f..77b95f2 100644
--- a/drivers/fpga/altera-hps2fpga.c
+++ b/drivers/fpga/altera-hps2fpga.c
@@ -180,7 +180,8 @@ static int alt_fpga_bridge_probe(struct platform_device *pdev)
}
}

- br = fpga_bridge_create(dev, priv->name, &altera_hps2fpga_br_ops, priv);
+ br = devm_fpga_bridge_create(dev, priv->name,
+ &altera_hps2fpga_br_ops, priv);
if (!br) {
ret = -ENOMEM;
goto err;
@@ -190,12 +191,10 @@ static int alt_fpga_bridge_probe(struct platform_device *pdev)

ret = fpga_bridge_register(br);
if (ret)
- goto err_free;
+ goto err;

return 0;

-err_free:
- fpga_bridge_free(br);
err:
clk_disable_unprepare(priv->clk);

diff --git a/drivers/fpga/dfl-fme-br.c b/drivers/fpga/dfl-fme-br.c
index 7cc041d..3ff9f3a 100644
--- a/drivers/fpga/dfl-fme-br.c
+++ b/drivers/fpga/dfl-fme-br.c
@@ -61,7 +61,6 @@ static int fme_br_probe(struct platform_device *pdev)
struct device *dev = &pdev->dev;
struct fme_br_priv *priv;
struct fpga_bridge *br;
- int ret;

priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
if (!priv)
@@ -69,18 +68,14 @@ static int fme_br_probe(struct platform_device *pdev)

priv->pdata = dev_get_platdata(dev);

- br = fpga_bridge_create(dev, "DFL FPGA FME Bridge",
- &fme_bridge_ops, priv);
+ br = devm_fpga_bridge_create(dev, "DFL FPGA FME Bridge",
+ &fme_bridge_ops, priv);
if (!br)
return -ENOMEM;

platform_set_drvdata(pdev, br);

- ret = fpga_bridge_register(br);
- if (ret)
- fpga_bridge_free(br);
-
- return ret;
+ return fpga_bridge_register(br);
}

static int fme_br_remove(struct platform_device *pdev)
diff --git a/drivers/fpga/fpga-bridge.c b/drivers/fpga/fpga-bridge.c
index c983dac..80bd8f1 100644
--- a/drivers/fpga/fpga-bridge.c
+++ b/drivers/fpga/fpga-bridge.c
@@ -324,6 +324,9 @@ ATTRIBUTE_GROUPS(fpga_bridge);
* @br_ops: pointer to structure of fpga bridge ops
* @priv: FPGA bridge private data
*
+ * The caller of this function is responsible for freeing the bridge with
+ * fpga_bridge_free(). Using devm_fpga_bridge_create() instead is recommended.
+ *
* Return: struct fpga_bridge or NULL
*/
struct fpga_bridge *fpga_bridge_create(struct device *dev, const char *name,
@@ -378,8 +381,8 @@ struct fpga_bridge *fpga_bridge_create(struct device *dev, const char *name,
EXPORT_SYMBOL_GPL(fpga_bridge_create);

/**
- * fpga_bridge_free - free a fpga bridge and its id
- * @bridge: FPGA bridge struct created by fpga_bridge_create
+ * fpga_bridge_free - free a fpga bridge created by fpga_bridge_create()
+ * @bridge: FPGA bridge struct
*/
void fpga_bridge_free(struct fpga_bridge *bridge)
{
@@ -388,9 +391,56 @@ void fpga_bridge_free(struct fpga_bridge *bridge)
}
EXPORT_SYMBOL_GPL(fpga_bridge_free);

+static void devm_fpga_bridge_release(struct device *dev, void *res)
+{
+ struct fpga_bridge *bridge = *(struct fpga_bridge **)res;
+
+ fpga_bridge_free(bridge);
+}
+
/**
- * fpga_bridge_register - register a fpga bridge
- * @bridge: FPGA bridge struct created by fpga_bridge_create
+ * devm_fpga_bridge_create - create and init a managed struct fpga_bridge
+ * @dev: FPGA bridge device from pdev
+ * @name: FPGA bridge name
+ * @br_ops: pointer to structure of fpga bridge ops
+ * @priv: FPGA bridge private data
+ *
+ * This function is intended for use in a FPGA bridge driver's probe function.
+ * After the bridge driver creates the struct with devm_fpga_bridge_create(), it
+ * should register the bridge with fpga_bridge_register(). The bridge driver's
+ * remove function should call fpga_bridge_unregister(). The bridge struct
+ * allocated with this function will be freed automatically on driver detach.
+ * This includes the case of a probe function returning error before calling
+ * fpga_bridge_register(), the struct will still get cleaned up.
+ *
+ * Return: struct fpga_bridge or NULL
+ */
+struct fpga_bridge
+*devm_fpga_bridge_create(struct device *dev, const char *name,
+ const struct fpga_bridge_ops *br_ops, void *priv)
+{
+ struct fpga_bridge **ptr, *bridge;
+
+ ptr = devres_alloc(devm_fpga_bridge_release, sizeof(*ptr), GFP_KERNEL);
+ if (!ptr)
+ return NULL;
+
+ bridge = fpga_bridge_create(dev, name, br_ops, priv);
+ if (!bridge) {
+ devres_free(ptr);
+ } else {
+ *ptr = bridge;
+ devres_add(dev, ptr);
+ }
+
+ return bridge;
+}
+EXPORT_SYMBOL_GPL(devm_fpga_bridge_create);
+
+/**
+ * fpga_bridge_register - register a FPGA bridge
+ *
+ * @bridge: FPGA bridge struct
*
* Return: 0 for success, error code otherwise.
*/
@@ -412,8 +462,11 @@ int fpga_bridge_register(struct fpga_bridge *bridge)
EXPORT_SYMBOL_GPL(fpga_bridge_register);

/**
- * fpga_bridge_unregister - unregister and free a fpga bridge
- * @bridge: FPGA bridge struct created by fpga_bridge_create
+ * fpga_bridge_unregister - unregister a FPGA bridge
+ *
+ * @bridge: FPGA bridge struct
+ *
+ * This function is intended for use in a FPGA bridge driver's remove function.
*/
void fpga_bridge_unregister(struct fpga_bridge *bridge)
{
@@ -430,9 +483,6 @@ EXPORT_SYMBOL_GPL(fpga_bridge_unregister);

static void fpga_bridge_dev_release(struct device *dev)
{
- struct fpga_bridge *bridge = to_fpga_bridge(dev);
-
- fpga_bridge_free(bridge);
}

static int __init fpga_bridge_dev_init(void)
diff --git a/drivers/fpga/xilinx-pr-decoupler.c b/drivers/fpga/xilinx-pr-decoupler.c
index 07ba153..6410361 100644
--- a/drivers/fpga/xilinx-pr-decoupler.c
+++ b/drivers/fpga/xilinx-pr-decoupler.c
@@ -121,8 +121,8 @@ static int xlnx_pr_decoupler_probe(struct platform_device *pdev)

clk_disable(priv->clk);

- br = fpga_bridge_create(&pdev->dev, "Xilinx PR Decoupler",
- &xlnx_pr_decoupler_br_ops, priv);
+ br = devm_fpga_bridge_create(&pdev->dev, "Xilinx PR Decoupler",
+ &xlnx_pr_decoupler_br_ops, priv);
if (!br) {
err = -ENOMEM;
goto err_clk;
diff --git a/include/linux/fpga/fpga-bridge.h b/include/linux/fpga/fpga-bridge.h
index ce550fc..817600a 100644
--- a/include/linux/fpga/fpga-bridge.h
+++ b/include/linux/fpga/fpga-bridge.h
@@ -69,4 +69,8 @@ void fpga_bridge_free(struct fpga_bridge *br);
int fpga_bridge_register(struct fpga_bridge *br);
void fpga_bridge_unregister(struct fpga_bridge *br);

+struct fpga_bridge
+*devm_fpga_bridge_create(struct device *dev, const char *name,
+ const struct fpga_bridge_ops *br_ops, void *priv);
+
#endif /* _LINUX_FPGA_BRIDGE_H */
--
2.7.4

2018-09-26 16:13:15

by Alan Tull

Subject: [PATCH v3 3/4] fpga: add devm_fpga_region_create

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

Change current region drivers to use
devm_fpga_region_create().

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

diff --git a/Documentation/driver-api/fpga/fpga-region.rst b/Documentation/driver-api/fpga/fpga-region.rst
index f30333c..dc9f75c 100644
--- a/Documentation/driver-api/fpga/fpga-region.rst
+++ b/Documentation/driver-api/fpga/fpga-region.rst
@@ -90,6 +90,9 @@ API to add a new FPGA region
:functions: fpga_region

.. kernel-doc:: drivers/fpga/fpga-region.c
+ :functions: devm_fpga_region_create
+
+.. kernel-doc:: drivers/fpga/fpga-region.c
:functions: fpga_region_create

.. kernel-doc:: drivers/fpga/fpga-region.c
diff --git a/drivers/fpga/dfl-fme-region.c b/drivers/fpga/dfl-fme-region.c
index 3fa0de2..1eeb42a 100644
--- a/drivers/fpga/dfl-fme-region.c
+++ b/drivers/fpga/dfl-fme-region.c
@@ -39,7 +39,7 @@ static int fme_region_probe(struct platform_device *pdev)
if (IS_ERR(mgr))
return -EPROBE_DEFER;

- region = fpga_region_create(dev, mgr, fme_region_get_bridges);
+ region = devm_fpga_region_create(dev, mgr, fme_region_get_bridges);
if (!region) {
ret = -ENOMEM;
goto eprobe_mgr_put;
@@ -51,14 +51,12 @@ static int fme_region_probe(struct platform_device *pdev)

ret = fpga_region_register(region);
if (ret)
- goto region_free;
+ goto eprobe_mgr_put;

dev_dbg(dev, "DFL FME FPGA Region probed\n");

return 0;

-region_free:
- fpga_region_free(region);
eprobe_mgr_put:
fpga_mgr_put(mgr);
return ret;
diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
index a9b521b..2c09e50 100644
--- a/drivers/fpga/dfl.c
+++ b/drivers/fpga/dfl.c
@@ -899,7 +899,7 @@ dfl_fpga_feature_devs_enumerate(struct dfl_fpga_enum_info *info)
if (!cdev)
return ERR_PTR(-ENOMEM);

- cdev->region = fpga_region_create(info->dev, NULL, NULL);
+ cdev->region = devm_fpga_region_create(info->dev, NULL, NULL);
if (!cdev->region) {
ret = -ENOMEM;
goto free_cdev_exit;
@@ -911,7 +911,7 @@ dfl_fpga_feature_devs_enumerate(struct dfl_fpga_enum_info *info)

ret = fpga_region_register(cdev->region);
if (ret)
- goto free_region_exit;
+ goto free_cdev_exit;

/* create and init build info for enumeration */
binfo = devm_kzalloc(info->dev, sizeof(*binfo), GFP_KERNEL);
@@ -942,8 +942,6 @@ dfl_fpga_feature_devs_enumerate(struct dfl_fpga_enum_info *info)

unregister_region_exit:
fpga_region_unregister(cdev->region);
-free_region_exit:
- fpga_region_free(cdev->region);
free_cdev_exit:
devm_kfree(info->dev, cdev);
return ERR_PTR(ret);
diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c
index 0d65220..bde5a9d 100644
--- a/drivers/fpga/fpga-region.c
+++ b/drivers/fpga/fpga-region.c
@@ -185,6 +185,10 @@ ATTRIBUTE_GROUPS(fpga_region);
* @mgr: manager that programs this region
* @get_bridges: optional function to get bridges to a list
*
+ * The caller of this function is responsible for freeing the resulting region
+ * struct with fpga_region_free(). Using devm_fpga_region_create() instead is
+ * recommended.
+ *
* Return: struct fpga_region or NULL
*/
struct fpga_region
@@ -230,8 +234,8 @@ struct fpga_region
EXPORT_SYMBOL_GPL(fpga_region_create);

/**
- * fpga_region_free - free a struct fpga_region
- * @region: FPGA region created by fpga_region_create
+ * fpga_region_free - free a FPGA region created by fpga_region_create()
+ * @region: FPGA region
*/
void fpga_region_free(struct fpga_region *region)
{
@@ -240,21 +244,69 @@ void fpga_region_free(struct fpga_region *region)
}
EXPORT_SYMBOL_GPL(fpga_region_free);

+static void devm_fpga_region_release(struct device *dev, void *res)
+{
+ struct fpga_region *region = *(struct fpga_region **)res;
+
+ fpga_region_free(region);
+}
+
+/**
+ * devm_fpga_region_create - create and initialize a managed FPGA region struct
+ * @dev: device parent
+ * @mgr: manager that programs this region
+ * @get_bridges: optional function to get bridges to a list
+ *
+ * This function is intended for use in a FPGA region driver's probe function.
+ * After the region driver creates the region struct with
+ * devm_fpga_region_create(), it should register it with fpga_region_register().
+ * The region driver's remove function should call fpga_region_unregister().
+ * The region struct allocated with this function will be freed automatically on
+ * driver detach. This includes the case of a probe function returning error
+ * before calling fpga_region_register(), the struct will still get cleaned up.
+ *
+ * Return: struct fpga_region or NULL
+ */
+struct fpga_region
+*devm_fpga_region_create(struct device *dev,
+ struct fpga_manager *mgr,
+ int (*get_bridges)(struct fpga_region *))
+{
+ struct fpga_region **ptr, *region;
+
+ ptr = devres_alloc(devm_fpga_region_release, sizeof(*ptr), GFP_KERNEL);
+ if (!ptr)
+ return NULL;
+
+ region = fpga_region_create(dev, mgr, get_bridges);
+ if (!region) {
+ devres_free(ptr);
+ } else {
+ *ptr = region;
+ devres_add(dev, ptr);
+ }
+
+ return region;
+}
+EXPORT_SYMBOL_GPL(devm_fpga_region_create);
+
/**
* fpga_region_register - register a FPGA region
- * @region: FPGA region created by fpga_region_create
+ * @region: FPGA region
+ *
* Return: 0 or -errno
*/
int fpga_region_register(struct fpga_region *region)
{
return device_add(&region->dev);
-
}
EXPORT_SYMBOL_GPL(fpga_region_register);

/**
- * fpga_region_unregister - unregister and free a FPGA region
+ * fpga_region_unregister - unregister a FPGA region
* @region: FPGA region
+ *
+ * This function is intended for use in a FPGA region driver's remove function.
*/
void fpga_region_unregister(struct fpga_region *region)
{
@@ -264,9 +316,6 @@ EXPORT_SYMBOL_GPL(fpga_region_unregister);

static void fpga_region_dev_release(struct device *dev)
{
- struct fpga_region *region = to_fpga_region(dev);
-
- fpga_region_free(region);
}

/**
diff --git a/drivers/fpga/of-fpga-region.c b/drivers/fpga/of-fpga-region.c
index d6a70e4..75f64ab 100644
--- a/drivers/fpga/of-fpga-region.c
+++ b/drivers/fpga/of-fpga-region.c
@@ -410,7 +410,7 @@ static int of_fpga_region_probe(struct platform_device *pdev)
if (IS_ERR(mgr))
return -EPROBE_DEFER;

- region = fpga_region_create(dev, mgr, of_fpga_region_get_bridges);
+ region = devm_fpga_region_create(dev, mgr, of_fpga_region_get_bridges);
if (!region) {
ret = -ENOMEM;
goto eprobe_mgr_put;
@@ -418,7 +418,7 @@ static int of_fpga_region_probe(struct platform_device *pdev)

ret = fpga_region_register(region);
if (ret)
- goto eprobe_free;
+ goto eprobe_mgr_put;

of_platform_populate(np, fpga_region_of_match, NULL, &region->dev);
platform_set_drvdata(pdev, region);
@@ -427,8 +427,6 @@ static int of_fpga_region_probe(struct platform_device *pdev)

return 0;

-eprobe_free:
- fpga_region_free(region);
eprobe_mgr_put:
fpga_mgr_put(mgr);
return ret;
diff --git a/include/linux/fpga/fpga-region.h b/include/linux/fpga/fpga-region.h
index 0521b7f..27cb706 100644
--- a/include/linux/fpga/fpga-region.h
+++ b/include/linux/fpga/fpga-region.h
@@ -44,4 +44,8 @@ void fpga_region_free(struct fpga_region *region);
int fpga_region_register(struct fpga_region *region);
void fpga_region_unregister(struct fpga_region *region);

+struct fpga_region
+*devm_fpga_region_create(struct device *dev, struct fpga_manager *mgr,
+ int (*get_bridges)(struct fpga_region *));
+
#endif /* _FPGA_REGION_H */
--
2.7.4

2018-09-26 16:13:39

by Alan Tull

Subject: [PATCH v3 1/4] fpga: mgr: add devm_fpga_mgr_create

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

Change current FPGA manager drivers to use
devm_fpga_mgr_create()

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

diff --git a/Documentation/driver-api/fpga/fpga-mgr.rst b/Documentation/driver-api/fpga/fpga-mgr.rst
index 82b6dbb..db8885e 100644
--- a/Documentation/driver-api/fpga/fpga-mgr.rst
+++ b/Documentation/driver-api/fpga/fpga-mgr.rst
@@ -49,18 +49,14 @@ probe function calls fpga_mgr_register(), such as::
* them in priv
*/

- mgr = fpga_mgr_create(dev, "Altera SOCFPGA FPGA Manager",
- &socfpga_fpga_ops, priv);
+ mgr = devm_fpga_mgr_create(dev, "Altera SOCFPGA FPGA Manager",
+ &socfpga_fpga_ops, priv);
if (!mgr)
return -ENOMEM;

platform_set_drvdata(pdev, mgr);

- ret = fpga_mgr_register(mgr);
- if (ret)
- fpga_mgr_free(mgr);
-
- return ret;
+ return fpga_mgr_register(mgr);
}

static int socfpga_fpga_remove(struct platform_device *pdev)
@@ -170,6 +166,9 @@ API for implementing a new FPGA Manager driver
:functions: fpga_manager_ops

.. kernel-doc:: drivers/fpga/fpga-mgr.c
+ :functions: devm_fpga_mgr_create
+
+.. kernel-doc:: drivers/fpga/fpga-mgr.c
:functions: fpga_mgr_create

.. kernel-doc:: drivers/fpga/fpga-mgr.c
diff --git a/drivers/fpga/altera-cvp.c b/drivers/fpga/altera-cvp.c
index 7fa7936..610a155 100644
--- a/drivers/fpga/altera-cvp.c
+++ b/drivers/fpga/altera-cvp.c
@@ -453,8 +453,8 @@ static int altera_cvp_probe(struct pci_dev *pdev,
snprintf(conf->mgr_name, sizeof(conf->mgr_name), "%s @%s",
ALTERA_CVP_MGR_NAME, pci_name(pdev));

- mgr = fpga_mgr_create(&pdev->dev, conf->mgr_name,
- &altera_cvp_ops, conf);
+ mgr = devm_fpga_mgr_create(&pdev->dev, conf->mgr_name,
+ &altera_cvp_ops, conf);
if (!mgr) {
ret = -ENOMEM;
goto err_unmap;
@@ -463,10 +463,8 @@ static int altera_cvp_probe(struct pci_dev *pdev,
pci_set_drvdata(pdev, mgr);

ret = fpga_mgr_register(mgr);
- if (ret) {
- fpga_mgr_free(mgr);
+ if (ret)
goto err_unmap;
- }

ret = driver_create_file(&altera_cvp_driver.driver,
&driver_attr_chkcfg);
diff --git a/drivers/fpga/altera-pr-ip-core.c b/drivers/fpga/altera-pr-ip-core.c
index 65e0b6a..a7a3bf0 100644
--- a/drivers/fpga/altera-pr-ip-core.c
+++ b/drivers/fpga/altera-pr-ip-core.c
@@ -177,7 +177,6 @@ int alt_pr_register(struct device *dev, void __iomem *reg_base)
{
struct alt_pr_priv *priv;
struct fpga_manager *mgr;
- int ret;
u32 val;

priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
@@ -192,17 +191,13 @@ int alt_pr_register(struct device *dev, void __iomem *reg_base)
(val & ALT_PR_CSR_STATUS_MSK) >> ALT_PR_CSR_STATUS_SFT,
(int)(val & ALT_PR_CSR_PR_START));

- mgr = fpga_mgr_create(dev, dev_name(dev), &alt_pr_ops, priv);
+ mgr = devm_fpga_mgr_create(dev, dev_name(dev), &alt_pr_ops, priv);
if (!mgr)
return -ENOMEM;

dev_set_drvdata(dev, mgr);

- ret = fpga_mgr_register(mgr);
- if (ret)
- fpga_mgr_free(mgr);
-
- return ret;
+ return fpga_mgr_register(mgr);
}
EXPORT_SYMBOL_GPL(alt_pr_register);

diff --git a/drivers/fpga/altera-ps-spi.c b/drivers/fpga/altera-ps-spi.c
index 24b25c6..33aafda 100644
--- a/drivers/fpga/altera-ps-spi.c
+++ b/drivers/fpga/altera-ps-spi.c
@@ -239,7 +239,6 @@ static int altera_ps_probe(struct spi_device *spi)
struct altera_ps_conf *conf;
const struct of_device_id *of_id;
struct fpga_manager *mgr;
- int ret;

conf = devm_kzalloc(&spi->dev, sizeof(*conf), GFP_KERNEL);
if (!conf)
@@ -275,18 +274,14 @@ static int altera_ps_probe(struct spi_device *spi)
snprintf(conf->mgr_name, sizeof(conf->mgr_name), "%s %s",
dev_driver_string(&spi->dev), dev_name(&spi->dev));

- mgr = fpga_mgr_create(&spi->dev, conf->mgr_name,
- &altera_ps_ops, conf);
+ mgr = devm_fpga_mgr_create(&spi->dev, conf->mgr_name,
+ &altera_ps_ops, conf);
if (!mgr)
return -ENOMEM;

spi_set_drvdata(spi, mgr);

- ret = fpga_mgr_register(mgr);
- if (ret)
- fpga_mgr_free(mgr);
-
- return ret;
+ return fpga_mgr_register(mgr);
}

static int altera_ps_remove(struct spi_device *spi)
diff --git a/drivers/fpga/dfl-fme-mgr.c b/drivers/fpga/dfl-fme-mgr.c
index 9f045d0..76f3770 100644
--- a/drivers/fpga/dfl-fme-mgr.c
+++ b/drivers/fpga/dfl-fme-mgr.c
@@ -287,7 +287,6 @@ static int fme_mgr_probe(struct platform_device *pdev)
struct fme_mgr_priv *priv;
struct fpga_manager *mgr;
struct resource *res;
- int ret;

priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
if (!priv)
@@ -309,19 +308,15 @@ static int fme_mgr_probe(struct platform_device *pdev)

fme_mgr_get_compat_id(priv->ioaddr, compat_id);

- mgr = fpga_mgr_create(dev, "DFL FME FPGA Manager",
- &fme_mgr_ops, priv);
+ mgr = devm_fpga_mgr_create(dev, "DFL FME FPGA Manager",
+ &fme_mgr_ops, priv);
if (!mgr)
return -ENOMEM;

mgr->compat_id = compat_id;
platform_set_drvdata(pdev, mgr);

- ret = fpga_mgr_register(mgr);
- if (ret)
- fpga_mgr_free(mgr);
-
- return ret;
+ return fpga_mgr_register(mgr);
}

static int fme_mgr_remove(struct platform_device *pdev)
diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c
index a41b07e..c386681 100644
--- a/drivers/fpga/fpga-mgr.c
+++ b/drivers/fpga/fpga-mgr.c
@@ -558,6 +558,9 @@ EXPORT_SYMBOL_GPL(fpga_mgr_unlock);
* @mops: pointer to structure of fpga manager ops
* @priv: fpga manager private data
*
+ * The caller of this function is responsible for freeing the struct with
+ * fpga_mgr_free(). Using devm_fpga_mgr_create() instead is recommended.
+ *
* Return: pointer to struct fpga_manager or NULL
*/
struct fpga_manager *fpga_mgr_create(struct device *dev, const char *name,
@@ -618,8 +621,8 @@ struct fpga_manager *fpga_mgr_create(struct device *dev, const char *name,
EXPORT_SYMBOL_GPL(fpga_mgr_create);

/**
- * fpga_mgr_free - deallocate a FPGA manager
- * @mgr: fpga manager struct created by fpga_mgr_create
+ * fpga_mgr_free - free a FPGA manager created with fpga_mgr_create()
+ * @mgr: fpga manager struct
*/
void fpga_mgr_free(struct fpga_manager *mgr)
{
@@ -628,9 +631,55 @@ void fpga_mgr_free(struct fpga_manager *mgr)
}
EXPORT_SYMBOL_GPL(fpga_mgr_free);

+static void devm_fpga_mgr_release(struct device *dev, void *res)
+{
+ struct fpga_manager *mgr = *(struct fpga_manager **)res;
+
+ fpga_mgr_free(mgr);
+}
+
+/**
+ * devm_fpga_mgr_create - create and initialize a managed FPGA manager struct
+ * @dev: fpga manager device from pdev
+ * @name: fpga manager name
+ * @mops: pointer to structure of fpga manager ops
+ * @priv: fpga manager private data
+ *
+ * This function is intended for use in a FPGA manager driver's probe function.
+ * After the manager driver creates the manager struct with
+ * devm_fpga_mgr_create(), it should register it with fpga_mgr_register(). The
+ * manager driver's remove function should call fpga_mgr_unregister(). The
+ * manager struct allocated with this function will be freed automatically on
+ * driver detach. This includes the case of a probe function returning error
+ * before calling fpga_mgr_register(), the struct will still get cleaned up.
+ *
+ * Return: pointer to struct fpga_manager or NULL
+ */
+struct fpga_manager *devm_fpga_mgr_create(struct device *dev, const char *name,
+ const struct fpga_manager_ops *mops,
+ void *priv)
+{
+ struct fpga_manager **ptr, *mgr;
+
+ ptr = devres_alloc(devm_fpga_mgr_release, sizeof(*ptr), GFP_KERNEL);
+ if (!ptr)
+ return NULL;
+
+ mgr = fpga_mgr_create(dev, name, mops, priv);
+ if (!mgr) {
+ devres_free(ptr);
+ } else {
+ *ptr = mgr;
+ devres_add(dev, ptr);
+ }
+
+ return mgr;
+}
+EXPORT_SYMBOL_GPL(devm_fpga_mgr_create);
+
/**
* fpga_mgr_register - register a FPGA manager
- * @mgr: fpga manager struct created by fpga_mgr_create
+ * @mgr: fpga manager struct
*
* Return: 0 on success, negative error code otherwise.
*/
@@ -661,8 +710,10 @@ int fpga_mgr_register(struct fpga_manager *mgr)
EXPORT_SYMBOL_GPL(fpga_mgr_register);

/**
- * fpga_mgr_unregister - unregister and free a FPGA manager
- * @mgr: fpga manager struct
+ * fpga_mgr_unregister - unregister a FPGA manager
+ * @mgr: fpga manager struct
+ *
+ * This function is intended for use in a FPGA manager driver's remove function.
*/
void fpga_mgr_unregister(struct fpga_manager *mgr)
{
@@ -681,9 +732,6 @@ EXPORT_SYMBOL_GPL(fpga_mgr_unregister);

static void fpga_mgr_dev_release(struct device *dev)
{
- struct fpga_manager *mgr = to_fpga_manager(dev);
-
- fpga_mgr_free(mgr);
}

static int __init fpga_mgr_class_init(void)
diff --git a/drivers/fpga/ice40-spi.c b/drivers/fpga/ice40-spi.c
index 5981c7e..6154661 100644
--- a/drivers/fpga/ice40-spi.c
+++ b/drivers/fpga/ice40-spi.c
@@ -175,18 +175,14 @@ static int ice40_fpga_probe(struct spi_device *spi)
return ret;
}

- mgr = fpga_mgr_create(dev, "Lattice iCE40 FPGA Manager",
- &ice40_fpga_ops, priv);
+ mgr = devm_fpga_mgr_create(dev, "Lattice iCE40 FPGA Manager",
+ &ice40_fpga_ops, priv);
if (!mgr)
return -ENOMEM;

spi_set_drvdata(spi, mgr);

- ret = fpga_mgr_register(mgr);
- if (ret)
- fpga_mgr_free(mgr);
-
- return ret;
+ return fpga_mgr_register(mgr);
}

static int ice40_fpga_remove(struct spi_device *spi)
diff --git a/drivers/fpga/machxo2-spi.c b/drivers/fpga/machxo2-spi.c
index a582e00..4d8a876 100644
--- a/drivers/fpga/machxo2-spi.c
+++ b/drivers/fpga/machxo2-spi.c
@@ -356,25 +356,20 @@ static int machxo2_spi_probe(struct spi_device *spi)
{
struct device *dev = &spi->dev;
struct fpga_manager *mgr;
- int ret;

if (spi->max_speed_hz > MACHXO2_MAX_SPEED) {
dev_err(dev, "Speed is too high\n");
return -EINVAL;
}

- mgr = fpga_mgr_create(dev, "Lattice MachXO2 SPI FPGA Manager",
- &machxo2_ops, spi);
+ mgr = devm_fpga_mgr_create(dev, "Lattice MachXO2 SPI FPGA Manager",
+ &machxo2_ops, spi);
if (!mgr)
return -ENOMEM;

spi_set_drvdata(spi, mgr);

- ret = fpga_mgr_register(mgr);
- if (ret)
- fpga_mgr_free(mgr);
-
- return ret;
+ return fpga_mgr_register(mgr);
}

static int machxo2_spi_remove(struct spi_device *spi)
diff --git a/drivers/fpga/socfpga-a10.c b/drivers/fpga/socfpga-a10.c
index be30c48..573d88b 100644
--- a/drivers/fpga/socfpga-a10.c
+++ b/drivers/fpga/socfpga-a10.c
@@ -508,8 +508,8 @@ static int socfpga_a10_fpga_probe(struct platform_device *pdev)
return -EBUSY;
}

- mgr = fpga_mgr_create(dev, "SoCFPGA Arria10 FPGA Manager",
- &socfpga_a10_fpga_mgr_ops, priv);
+ mgr = devm_fpga_mgr_create(dev, "SoCFPGA Arria10 FPGA Manager",
+ &socfpga_a10_fpga_mgr_ops, priv);
if (!mgr)
return -ENOMEM;

@@ -517,7 +517,6 @@ static int socfpga_a10_fpga_probe(struct platform_device *pdev)

ret = fpga_mgr_register(mgr);
if (ret) {
- fpga_mgr_free(mgr);
clk_disable_unprepare(priv->clk);
return ret;
}
diff --git a/drivers/fpga/socfpga.c b/drivers/fpga/socfpga.c
index 959d71f..4a8a2fc 100644
--- a/drivers/fpga/socfpga.c
+++ b/drivers/fpga/socfpga.c
@@ -571,18 +571,14 @@ static int socfpga_fpga_probe(struct platform_device *pdev)
if (ret)
return ret;

- mgr = fpga_mgr_create(dev, "Altera SOCFPGA FPGA Manager",
- &socfpga_fpga_ops, priv);
+ mgr = devm_fpga_mgr_create(dev, "Altera SOCFPGA FPGA Manager",
+ &socfpga_fpga_ops, priv);
if (!mgr)
return -ENOMEM;

platform_set_drvdata(pdev, mgr);

- ret = fpga_mgr_register(mgr);
- if (ret)
- fpga_mgr_free(mgr);
-
- return ret;
+ return fpga_mgr_register(mgr);
}

static int socfpga_fpga_remove(struct platform_device *pdev)
diff --git a/drivers/fpga/ts73xx-fpga.c b/drivers/fpga/ts73xx-fpga.c
index 08efd18..dc22a58 100644
--- a/drivers/fpga/ts73xx-fpga.c
+++ b/drivers/fpga/ts73xx-fpga.c
@@ -118,7 +118,6 @@ static int ts73xx_fpga_probe(struct platform_device *pdev)
struct ts73xx_fpga_priv *priv;
struct fpga_manager *mgr;
struct resource *res;
- int ret;

priv = devm_kzalloc(kdev, sizeof(*priv), GFP_KERNEL);
if (!priv)
@@ -133,18 +132,14 @@ static int ts73xx_fpga_probe(struct platform_device *pdev)
return PTR_ERR(priv->io_base);
}

- mgr = fpga_mgr_create(kdev, "TS-73xx FPGA Manager",
- &ts73xx_fpga_ops, priv);
+ mgr = devm_fpga_mgr_create(kdev, "TS-73xx FPGA Manager",
+ &ts73xx_fpga_ops, priv);
if (!mgr)
return -ENOMEM;

platform_set_drvdata(pdev, mgr);

- ret = fpga_mgr_register(mgr);
- if (ret)
- fpga_mgr_free(mgr);
-
- return ret;
+ return fpga_mgr_register(mgr);
}

static int ts73xx_fpga_remove(struct platform_device *pdev)
diff --git a/drivers/fpga/xilinx-spi.c b/drivers/fpga/xilinx-spi.c
index 8d19459..469486b 100644
--- a/drivers/fpga/xilinx-spi.c
+++ b/drivers/fpga/xilinx-spi.c
@@ -144,7 +144,6 @@ static int xilinx_spi_probe(struct spi_device *spi)
{
struct xilinx_spi_conf *conf;
struct fpga_manager *mgr;
- int ret;

conf = devm_kzalloc(&spi->dev, sizeof(*conf), GFP_KERNEL);
if (!conf)
@@ -167,18 +166,15 @@ static int xilinx_spi_probe(struct spi_device *spi)
return PTR_ERR(conf->done);
}

- mgr = fpga_mgr_create(&spi->dev, "Xilinx Slave Serial FPGA Manager",
- &xilinx_spi_ops, conf);
+ mgr = devm_fpga_mgr_create(&spi->dev,
+ "Xilinx Slave Serial FPGA Manager",
+ &xilinx_spi_ops, conf);
if (!mgr)
return -ENOMEM;

spi_set_drvdata(spi, mgr);

- ret = fpga_mgr_register(mgr);
- if (ret)
- fpga_mgr_free(mgr);
-
- return ret;
+ return fpga_mgr_register(mgr);
}

static int xilinx_spi_remove(struct spi_device *spi)
diff --git a/drivers/fpga/zynq-fpga.c b/drivers/fpga/zynq-fpga.c
index 3110e00..bb82efe 100644
--- a/drivers/fpga/zynq-fpga.c
+++ b/drivers/fpga/zynq-fpga.c
@@ -614,8 +614,8 @@ static int zynq_fpga_probe(struct platform_device *pdev)

clk_disable(priv->clk);

- mgr = fpga_mgr_create(dev, "Xilinx Zynq FPGA Manager",
- &zynq_fpga_ops, priv);
+ mgr = devm_fpga_mgr_create(dev, "Xilinx Zynq FPGA Manager",
+ &zynq_fpga_ops, priv);
if (!mgr)
return -ENOMEM;

@@ -624,7 +624,6 @@ static int zynq_fpga_probe(struct platform_device *pdev)
err = fpga_mgr_register(mgr);
if (err) {
dev_err(dev, "unable to register FPGA manager\n");
- fpga_mgr_free(mgr);
clk_unprepare(priv->clk);
return err;
}
diff --git a/include/linux/fpga/fpga-mgr.h b/include/linux/fpga/fpga-mgr.h
index 8ab5df7..e8ca62b 100644
--- a/include/linux/fpga/fpga-mgr.h
+++ b/include/linux/fpga/fpga-mgr.h
@@ -198,4 +198,8 @@ void fpga_mgr_free(struct fpga_manager *mgr);
int fpga_mgr_register(struct fpga_manager *mgr);
void fpga_mgr_unregister(struct fpga_manager *mgr);

+struct fpga_manager *devm_fpga_mgr_create(struct device *dev, const char *name,
+ const struct fpga_manager_ops *mops,
+ void *priv);
+
#endif /*_LINUX_FPGA_MGR_H */
--
2.7.4

2018-10-15 14:55:49

by Alan Tull

Subject: Re: [PATCH v3 1/4] fpga: mgr: add devm_fpga_mgr_create

On Wed, Sep 26, 2018 at 11:12 AM Alan Tull <[email protected]> wrote:

Any other comments on this patchset?

Alan

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

2018-10-15 17:43:28

by Moritz Fischer

Subject: Re: [PATCH v3 1/4] fpga: mgr: add devm_fpga_mgr_create

Hi Alan,

On Mon, Oct 15, 2018 at 7:55 AM Alan Tull <[email protected]> wrote:
>
> On Wed, Sep 26, 2018 at 11:12 AM Alan Tull <[email protected]> wrote:
>
> Any other comments on this patchset?

Looks good to me, Sorry for the delay

Acked-by: Moritz Fischer <[email protected]>

Cheers,

Moritz

2018-10-15 20:11:42

by Moritz Fischer

Subject: Re: [PATCH v3 3/4] fpga: add devm_fpga_region_create

On Wed, Sep 26, 2018 at 9:12 AM Alan Tull <[email protected]> wrote:
>
> Add devm_fpga_region_create() which is the
> managed version of fpga_region_create().
>
> Change current region drivers to use
> devm_fpga_region_create().
>
> Signed-off-by: Alan Tull <[email protected]>
> Suggested-by: Federico Vaga <[email protected]>

Acked-by: Moritz Fischer <[email protected]>
> ---
> v2: add suggested-by
> v3: remove some unclear documentation about fpga_region_unregister
> ---
> Documentation/driver-api/fpga/fpga-region.rst | 3 ++
> drivers/fpga/dfl-fme-region.c | 6 +--
> drivers/fpga/dfl.c | 6 +--
> drivers/fpga/fpga-region.c | 65 +++++++++++++++++++++++----
> drivers/fpga/of-fpga-region.c | 6 +--
> include/linux/fpga/fpga-region.h | 4 ++
> 6 files changed, 70 insertions(+), 20 deletions(-)
>
> diff --git a/Documentation/driver-api/fpga/fpga-region.rst b/Documentation/driver-api/fpga/fpga-region.rst
> index f30333c..dc9f75c 100644
> --- a/Documentation/driver-api/fpga/fpga-region.rst
> +++ b/Documentation/driver-api/fpga/fpga-region.rst
> @@ -90,6 +90,9 @@ API to add a new FPGA region
> :functions: fpga_region
>
> .. kernel-doc:: drivers/fpga/fpga-region.c
> + :functions: devm_fpga_region_create
> +
> +.. kernel-doc:: drivers/fpga/fpga-region.c
> :functions: fpga_region_create
>
> .. kernel-doc:: drivers/fpga/fpga-region.c
> diff --git a/drivers/fpga/dfl-fme-region.c b/drivers/fpga/dfl-fme-region.c
> index 3fa0de2..1eeb42a 100644
> --- a/drivers/fpga/dfl-fme-region.c
> +++ b/drivers/fpga/dfl-fme-region.c
> @@ -39,7 +39,7 @@ static int fme_region_probe(struct platform_device *pdev)
> if (IS_ERR(mgr))
> return -EPROBE_DEFER;
>
> - region = fpga_region_create(dev, mgr, fme_region_get_bridges);
> + region = devm_fpga_region_create(dev, mgr, fme_region_get_bridges);
> if (!region) {
> ret = -ENOMEM;
> goto eprobe_mgr_put;
> @@ -51,14 +51,12 @@ static int fme_region_probe(struct platform_device *pdev)
>
> ret = fpga_region_register(region);
> if (ret)
> - goto region_free;
> + goto eprobe_mgr_put;
>
> dev_dbg(dev, "DFL FME FPGA Region probed\n");
>
> return 0;
>
> -region_free:
> - fpga_region_free(region);
> eprobe_mgr_put:
> fpga_mgr_put(mgr);
> return ret;
> diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
> index a9b521b..2c09e50 100644
> --- a/drivers/fpga/dfl.c
> +++ b/drivers/fpga/dfl.c
> @@ -899,7 +899,7 @@ dfl_fpga_feature_devs_enumerate(struct dfl_fpga_enum_info *info)
> if (!cdev)
> return ERR_PTR(-ENOMEM);
>
> - cdev->region = fpga_region_create(info->dev, NULL, NULL);
> + cdev->region = devm_fpga_region_create(info->dev, NULL, NULL);
> if (!cdev->region) {
> ret = -ENOMEM;
> goto free_cdev_exit;
> @@ -911,7 +911,7 @@ dfl_fpga_feature_devs_enumerate(struct dfl_fpga_enum_info *info)
>
> ret = fpga_region_register(cdev->region);
> if (ret)
> - goto free_region_exit;
> + goto free_cdev_exit;
>
> /* create and init build info for enumeration */
> binfo = devm_kzalloc(info->dev, sizeof(*binfo), GFP_KERNEL);
> @@ -942,8 +942,6 @@ dfl_fpga_feature_devs_enumerate(struct dfl_fpga_enum_info *info)
>
> unregister_region_exit:
> fpga_region_unregister(cdev->region);
> -free_region_exit:
> - fpga_region_free(cdev->region);
> free_cdev_exit:
> devm_kfree(info->dev, cdev);
> return ERR_PTR(ret);
> diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c
> index 0d65220..bde5a9d 100644
> --- a/drivers/fpga/fpga-region.c
> +++ b/drivers/fpga/fpga-region.c
> @@ -185,6 +185,10 @@ ATTRIBUTE_GROUPS(fpga_region);
> * @mgr: manager that programs this region
> * @get_bridges: optional function to get bridges to a list
> *
> + * The caller of this function is responsible for freeing the resulting region
> + * struct with fpga_region_free(). Using devm_fpga_region_create() instead is
> + * recommended.
> + *
> * Return: struct fpga_region or NULL
> */
> struct fpga_region
> @@ -230,8 +234,8 @@ struct fpga_region
> EXPORT_SYMBOL_GPL(fpga_region_create);
>
> /**
> - * fpga_region_free - free a struct fpga_region
> - * @region: FPGA region created by fpga_region_create
> + * fpga_region_free - free a FPGA region created by fpga_region_create()
> + * @region: FPGA region
> */
> void fpga_region_free(struct fpga_region *region)
> {
> @@ -240,21 +244,69 @@ void fpga_region_free(struct fpga_region *region)
> }
> EXPORT_SYMBOL_GPL(fpga_region_free);
>
> +static void devm_fpga_region_release(struct device *dev, void *res)
> +{
> + struct fpga_region *region = *(struct fpga_region **)res;
> +
> + fpga_region_free(region);
> +}
> +
> +/**
> + * devm_fpga_region_create - create and initialize a managed FPGA region struct
> + * @dev: device parent
> + * @mgr: manager that programs this region
> + * @get_bridges: optional function to get bridges to a list
> + *
> + * This function is intended for use in a FPGA region driver's probe function.
> + * After the region driver creates the region struct with
> + * devm_fpga_region_create(), it should register it with fpga_region_register().
> + * The region driver's remove function should call fpga_region_unregister().
> + * The region struct allocated with this function will be freed automatically on
> + * driver detach. This includes the case of a probe function returning error
> + * before calling fpga_region_register(), the struct will still get cleaned up.
> + *
> + * Return: struct fpga_region or NULL
> + */
> +struct fpga_region
> +*devm_fpga_region_create(struct device *dev,
> + struct fpga_manager *mgr,
> + int (*get_bridges)(struct fpga_region *))
> +{
> + struct fpga_region **ptr, *region;
> +
> + ptr = devres_alloc(devm_fpga_region_release, sizeof(*ptr), GFP_KERNEL);
> + if (!ptr)
> + return NULL;
> +
> + region = fpga_region_create(dev, mgr, get_bridges);
> + if (!region) {
> + devres_free(ptr);
> + } else {
> + *ptr = region;
> + devres_add(dev, ptr);
> + }
> +
> + return region;
> +}
> +EXPORT_SYMBOL_GPL(devm_fpga_region_create);
> +
> /**
> * fpga_region_register - register a FPGA region
> - * @region: FPGA region created by fpga_region_create
> + * @region: FPGA region
> + *
> * Return: 0 or -errno
> */
> int fpga_region_register(struct fpga_region *region)
> {
> return device_add(&region->dev);
> -
> }
> EXPORT_SYMBOL_GPL(fpga_region_register);
>
> /**
> - * fpga_region_unregister - unregister and free a FPGA region
> + * fpga_region_unregister - unregister a FPGA region
> * @region: FPGA region
> + *
> + * This function is intended for use in a FPGA region driver's remove function.
> */
> void fpga_region_unregister(struct fpga_region *region)
> {
> @@ -264,9 +316,6 @@ EXPORT_SYMBOL_GPL(fpga_region_unregister);
>
> static void fpga_region_dev_release(struct device *dev)
> {
> - struct fpga_region *region = to_fpga_region(dev);
> -
> - fpga_region_free(region);
> }
>
> /**
> diff --git a/drivers/fpga/of-fpga-region.c b/drivers/fpga/of-fpga-region.c
> index d6a70e4..75f64ab 100644
> --- a/drivers/fpga/of-fpga-region.c
> +++ b/drivers/fpga/of-fpga-region.c
> @@ -410,7 +410,7 @@ static int of_fpga_region_probe(struct platform_device *pdev)
> if (IS_ERR(mgr))
> return -EPROBE_DEFER;
>
> - region = fpga_region_create(dev, mgr, of_fpga_region_get_bridges);
> + region = devm_fpga_region_create(dev, mgr, of_fpga_region_get_bridges);
> if (!region) {
> ret = -ENOMEM;
> goto eprobe_mgr_put;
> @@ -418,7 +418,7 @@ static int of_fpga_region_probe(struct platform_device *pdev)
>
> ret = fpga_region_register(region);
> if (ret)
> - goto eprobe_free;
> + goto eprobe_mgr_put;
>
> of_platform_populate(np, fpga_region_of_match, NULL, &region->dev);
> platform_set_drvdata(pdev, region);
> @@ -427,8 +427,6 @@ static int of_fpga_region_probe(struct platform_device *pdev)
>
> return 0;
>
> -eprobe_free:
> - fpga_region_free(region);
> eprobe_mgr_put:
> fpga_mgr_put(mgr);
> return ret;
> diff --git a/include/linux/fpga/fpga-region.h b/include/linux/fpga/fpga-region.h
> index 0521b7f..27cb706 100644
> --- a/include/linux/fpga/fpga-region.h
> +++ b/include/linux/fpga/fpga-region.h
> @@ -44,4 +44,8 @@ void fpga_region_free(struct fpga_region *region);
> int fpga_region_register(struct fpga_region *region);
> void fpga_region_unregister(struct fpga_region *region);
>
> +struct fpga_region
> +*devm_fpga_region_create(struct device *dev, struct fpga_manager *mgr,
> + int (*get_bridges)(struct fpga_region *));
> +
> #endif /* _FPGA_REGION_H */
> --
> 2.7.4
>

Thanks,
Moritz

2018-10-15 20:13:40

by Moritz Fischer

Subject: Re: [PATCH v3 4/4] docs: fpga: document programming fpgas using regions

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

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

Thanks,
Moritz

2018-10-15 20:41:37

by Alan Tull

Subject: Re: [PATCH v3 4/4] docs: fpga: document programming fpgas using regions

On Mon, Oct 15, 2018 at 3:11 PM Moritz Fischer
<[email protected]> wrote:
>
> On Wed, Sep 26, 2018 at 9:12 AM Alan Tull <[email protected]> wrote:
> >
> > Clarify the intention that interfaces and upper layers use
> > regions rather than managers directly.
> >
> > Rearrange API documentation to better group the API functions
> > used to create FPGA mgr/bridge/regions and the API used for
> > programming FPGAs.
> >
> > Signed-off-by: Alan Tull <[email protected]>
> > Suggested-by: Federico Vaga <[email protected]>
>
> Acked-by: Moritz Fischer <[email protected]>

Thanks Moritz!

Alan

2018-10-16 07:03:28

by Federico Vaga

Subject: Re: [PATCH v3 1/4] fpga: mgr: add devm_fpga_mgr_create

not comment here

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