2018-08-14 19:18:44

by Alan Tull

Subject: [PATCH 0/8] fpga: Add managed create in FPGA API

Fix one functional FPGA bug.

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

More documentation cleanup to make the documentation more helpful
and to emphasize the use of FPGA regions for FPGA programming.

Fix a few minor documentation corrections.

Alan Tull (8):
fpga: do not access region struct after fpga_region_unregister
fpga: mgr: add devm_fpga_mgr_create
fpga: bridge: add devm_fpga_bridge_create
fpga: add devm_fpga_region_create
dt-bindings: fpga: fix freeze controller compatible in region doc
fpga: bridge: fix obvious function documentation error
docs: fpga: document fpga manager flags
docs: fpga: document programming fpgas using regions

.../devicetree/bindings/fpga/fpga-region.txt | 4 +-
Documentation/driver-api/fpga/fpga-bridge.rst | 37 ++-----
Documentation/driver-api/fpga/fpga-mgr.rst | 121 +++------------------
Documentation/driver-api/fpga/fpga-programming.rst | 103 ++++++++++++++++++
Documentation/driver-api/fpga/fpga-region.rst | 91 ++++++++--------
Documentation/driver-api/fpga/index.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 | 10 +-
drivers/fpga/dfl.c | 6 +-
drivers/fpga/fpga-bridge.c | 74 +++++++++++--
drivers/fpga/fpga-mgr.c | 67 ++++++++++--
drivers/fpga/fpga-region.c | 68 ++++++++++--
drivers/fpga/ice40-spi.c | 10 +-
drivers/fpga/machxo2-spi.c | 11 +-
drivers/fpga/of-fpga-region.c | 9 +-
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 | 22 +++-
include/linux/fpga/fpga-region.h | 4 +
31 files changed, 437 insertions(+), 331 deletions(-)
create mode 100644 Documentation/driver-api/fpga/fpga-programming.rst

--
2.7.4

2018-08-14 19:17:00

by Alan Tull

Subject: [PATCH 7/8] docs: fpga: document fpga manager flags

Add flags #defines to kerneldoc documentation in a
useful place.

Signed-off-by: Alan Tull <[email protected]>
---
Documentation/driver-api/fpga/fpga-mgr.rst | 5 +++++
include/linux/fpga/fpga-mgr.h | 18 ++++++++++++------
2 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/Documentation/driver-api/fpga/fpga-mgr.rst b/Documentation/driver-api/fpga/fpga-mgr.rst
index 431556d..db8885e 100644
--- a/Documentation/driver-api/fpga/fpga-mgr.rst
+++ b/Documentation/driver-api/fpga/fpga-mgr.rst
@@ -183,6 +183,11 @@ API for implementing a new FPGA Manager driver
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

diff --git a/include/linux/fpga/fpga-mgr.h b/include/linux/fpga/fpga-mgr.h
index 1ca02ce..777c502 100644
--- a/include/linux/fpga/fpga-mgr.h
+++ b/include/linux/fpga/fpga-mgr.h
@@ -53,12 +53,18 @@ enum fpga_mgr_states {
FPGA_MGR_STATE_OPERATING,
};

-/*
- * FPGA Manager flags
- * FPGA_MGR_PARTIAL_RECONFIG: do partial reconfiguration if supported
- * FPGA_MGR_EXTERNAL_CONFIG: FPGA has been configured prior to Linux booting
- * FPGA_MGR_BITSTREAM_LSB_FIRST: SPI bitstream bit order is LSB first
- * FPGA_MGR_COMPRESSED_BITSTREAM: FPGA bitstream is compressed
+/**
+ * DOC: FPGA Manager flags
+ *
+ * Flags used in the &fpga_image_info->flags field
+ *
+ * %FPGA_MGR_PARTIAL_RECONFIG: do partial reconfiguration if supported
+ *
+ * %FPGA_MGR_EXTERNAL_CONFIG: FPGA has been configured prior to Linux booting
+ *
+ * %FPGA_MGR_BITSTREAM_LSB_FIRST: SPI bitstream bit order is LSB first
+ *
+ * %FPGA_MGR_COMPRESSED_BITSTREAM: FPGA bitstream is compressed
*/
#define FPGA_MGR_PARTIAL_RECONFIG BIT(0)
#define FPGA_MGR_EXTERNAL_CONFIG BIT(1)
--
2.7.4

2018-08-14 19:17:05

by Alan Tull

Subject: [PATCH 5/8] dt-bindings: fpga: fix freeze controller compatible in region doc

Documentation/devicetree/bindings/fpga/fpga-region.txt has an error
regarding the freeze controller bridge, secifically:

compatible = "altr,freeze-bridge";

The compatibility string should be "altr,freeze-bridge-controller"
as set forth in altera-freeze-bridge.txt.

Signed-off-by: Alan Tull <[email protected]>
Reported-by: Dalon Westergreen <[email protected]>
---
Documentation/devicetree/bindings/fpga/fpga-region.txt | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/fpga/fpga-region.txt b/Documentation/devicetree/bindings/fpga/fpga-region.txt
index 6db8aed..90c4469 100644
--- a/Documentation/devicetree/bindings/fpga/fpga-region.txt
+++ b/Documentation/devicetree/bindings/fpga/fpga-region.txt
@@ -415,7 +415,7 @@ DT Overlay contains:
firmware-name = "base.rbf";

fpga-bridge@4400 {
- compatible = "altr,freeze-bridge";
+ compatible = "altr,freeze-bridge-controller";
reg = <0x4400 0x10>;

fpga_region1: fpga-region1 {
@@ -427,7 +427,7 @@ DT Overlay contains:
};

fpga-bridge@4420 {
- compatible = "altr,freeze-bridge";
+ compatible = "altr,freeze-bridge-controller";
reg = <0x4420 0x10>;

fpga_region2: fpga-region2 {
--
2.7.4

2018-08-14 19:17:20

by Alan Tull

Subject: [PATCH 4/8] 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]>
---
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 | 68 +++++++++++++++++++++++----
drivers/fpga/of-fpga-region.c | 6 +--
include/linux/fpga/fpga-region.h | 4 ++
6 files changed, 73 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 51a5ac2..ec134ec 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..56f773d 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,72 @@ 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.
+ * If the region was created by devm_fpga_region_create(), it will be
+ * automatically freed. If the region was created by fpga_region_create(), the
+ * caller is responsible for freeing it with fpga_region_free().
*/
void fpga_region_unregister(struct fpga_region *region)
{
@@ -264,9 +319,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 052a134..122286f 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);
dev_set_drvdata(dev, 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-08-14 19:18:08

by Alan Tull

Subject: [PATCH 3/8] 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]>
---
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 | 72 +++++++++++++++++++++++----
drivers/fpga/xilinx-pr-decoupler.c | 4 +-
include/linux/fpga/fpga-bridge.h | 4 ++
8 files changed, 84 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 24b8f98..c39d35f 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,15 @@ 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.
+ * If the bridge was created with devm_fpga_bridge_create(), the bridge struct
+ * will be automatically freed. If the bridge was created with
+ * fpga_bridge_create(), the caller is responsible for freeing the bridge with
+ * fpga_bridge_free().
*/
void fpga_bridge_unregister(struct fpga_bridge *bridge)
{
@@ -430,9 +487,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-08-14 19:18:17

by Alan Tull

Subject: [PATCH 2/8] 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]>
---
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 | 67 ++++++++++++++++++++++++++----
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, 100 insertions(+), 87 deletions(-)

diff --git a/Documentation/driver-api/fpga/fpga-mgr.rst b/Documentation/driver-api/fpga/fpga-mgr.rst
index 4b3825d..431556d 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 b5ef405..617ce0b 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..c8684bc 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,13 @@ 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.
+ * If the manager was created by devm_fpga_mgr_create(), it will be
+ * automatically freed. If the manager was created by fpga_mgr_create(), the
+ * caller is responsible for freeing it with fpga_mgr_free().
*/
void fpga_mgr_unregister(struct fpga_manager *mgr)
{
@@ -681,9 +735,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 8942e61..1ca02ce 100644
--- a/include/linux/fpga/fpga-mgr.h
+++ b/include/linux/fpga/fpga-mgr.h
@@ -190,4 +190,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-08-14 19:18:30

by Alan Tull

Subject: [PATCH 1/8] fpga: do not access region struct after fpga_region_unregister

A couple drivers were accessing the region struct after it had been
freed. Save off the pointer to the mgr before the region struct gets
freed.

Signed-off-by: Alan Tull <[email protected]>
---
drivers/fpga/dfl-fme-region.c | 4 +++-
drivers/fpga/of-fpga-region.c | 3 ++-
2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/fpga/dfl-fme-region.c b/drivers/fpga/dfl-fme-region.c
index 0b7e19c..51a5ac2 100644
--- a/drivers/fpga/dfl-fme-region.c
+++ b/drivers/fpga/dfl-fme-region.c
@@ -14,6 +14,7 @@
*/

#include <linux/module.h>
+#include <linux/fpga/fpga-mgr.h>
#include <linux/fpga/fpga-region.h>

#include "dfl-fme-pr.h"
@@ -66,9 +67,10 @@ static int fme_region_probe(struct platform_device *pdev)
static int fme_region_remove(struct platform_device *pdev)
{
struct fpga_region *region = dev_get_drvdata(&pdev->dev);
+ struct fpga_manager *mgr = region->mgr;

fpga_region_unregister(region);
- fpga_mgr_put(region->mgr);
+ fpga_mgr_put(mgr);

return 0;
}
diff --git a/drivers/fpga/of-fpga-region.c b/drivers/fpga/of-fpga-region.c
index 35fabb8..052a134 100644
--- a/drivers/fpga/of-fpga-region.c
+++ b/drivers/fpga/of-fpga-region.c
@@ -437,9 +437,10 @@ static int of_fpga_region_probe(struct platform_device *pdev)
static int of_fpga_region_remove(struct platform_device *pdev)
{
struct fpga_region *region = platform_get_drvdata(pdev);
+ struct fpga_manager *mgr = region->mgr;

fpga_region_unregister(region);
- fpga_mgr_put(region->mgr);
+ fpga_mgr_put(mgr);

return 0;
}
--
2.7.4

2018-08-14 19:31:38

by Alan Tull

Subject: [PATCH 6/8] fpga: bridge: fix obvious function documentation error

fpga_bridge_dev_match() returns a FPGA bridge struct, not a
FPGA manager struct so s/manager/bridge/.

Signed-off-by: Alan Tull <[email protected]>
---
drivers/fpga/fpga-bridge.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/fpga/fpga-bridge.c b/drivers/fpga/fpga-bridge.c
index c39d35f..7b0071a 100644
--- a/drivers/fpga/fpga-bridge.c
+++ b/drivers/fpga/fpga-bridge.c
@@ -125,7 +125,7 @@ static int fpga_bridge_dev_match(struct device *dev, const void *data)
*
* Given a device, get an exclusive reference to a fpga bridge.
*
- * Return: fpga manager struct or IS_ERR() condition containing error code.
+ * Return: fpga bridge struct or IS_ERR() condition containing error code.
*/
struct fpga_bridge *fpga_bridge_get(struct device *dev,
struct fpga_image_info *info)
--
2.7.4

2018-08-14 19:49:42

by Alan Tull

Subject: [PATCH 8/8] 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]>
---
Documentation/driver-api/fpga/fpga-bridge.rst | 38 ++-----
Documentation/driver-api/fpga/fpga-mgr.rst | 117 ++-------------------
Documentation/driver-api/fpga/fpga-programming.rst | 103 ++++++++++++++++++
Documentation/driver-api/fpga/fpga-region.rst | 92 ++++++++--------
Documentation/driver-api/fpga/index.rst | 2 +
5 files changed, 166 insertions(+), 186 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..1c1dc54 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 a FPGA manager
+* :c:func:`fpga_mgr_unregister` — Unregister a 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..cc34d17
--- /dev/null
+++ b/Documentation/driver-api/fpga/fpga-programming.rst
@@ -0,0 +1,103 @@
+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 a 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);
+ if (ret)
+ return ret;
+
+ /* Deallocate the image info if you're done with it */
+ fpga_image_info_free(info);
+
+ /* Now enumerate whatever hardware has appeared in the FPGA. */
+
+API for programming an FPGA
+---------------------------
+
+* :c:func:`fpga_region_program_fpga` — Program a FPGA
+* :c:type:`fpga_image_info` — Specifies what FPGA image to program
+* :c:func:`fpga_image_info_alloc()` — Allocate a FPGA image info struct
+* :c:func:`fpga_image_info_free()` — Free a 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..90d95bb 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 a FPGA region
+* :c:func:`fpga_region_unregister` — Unregister a 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 a FPGA manager, raise ref count
+* :c:func:`of_fpga_mgr_get` — Get a reference to a FPGA manager, raise ref count,
+ given a device node.
+* :c:func:`fpga_mgr_put` — Put a 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 a FPGA bridge, add it to a
+ list
+* :c:func:`of_fpga_bridge_get_to_list` — Get a ref of a 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
+
--
2.7.4

2018-08-14 21:45:59

by Federico Vaga

Subject: Re: [PATCH 0/8] fpga: Add managed create in FPGA API

Hi Alan,

I think you forgot to add:

Suggested-by: Federico Vaga <[email protected]>

on patches 2,3,4 which were suggested here

https://lkml.org/lkml/2018/7/26/145

and patch 8 here:

https://lkml.org/lkml/2018/7/18/959

On Tuesday, August 14, 2018 9:15:18 PM CEST Alan Tull wrote:
> Fix one functional FPGA bug.
>
> Implement managed devm_fpga_(mgr|bridge|region)_create() functions.
>
> More documentation cleanup to make the documentation more helpful
> and to emphasize the use of FPGA regions for FPGA programming.
>
> Fix a few minor documentation corrections.
>
> Alan Tull (8):
> fpga: do not access region struct after fpga_region_unregister
> fpga: mgr: add devm_fpga_mgr_create
> fpga: bridge: add devm_fpga_bridge_create
> fpga: add devm_fpga_region_create
> dt-bindings: fpga: fix freeze controller compatible in region doc
> fpga: bridge: fix obvious function documentation error
> docs: fpga: document fpga manager flags
> docs: fpga: document programming fpgas using regions
>
> .../devicetree/bindings/fpga/fpga-region.txt | 4 +-
> Documentation/driver-api/fpga/fpga-bridge.rst | 37 ++-----
> Documentation/driver-api/fpga/fpga-mgr.rst | 121
> +++------------------ Documentation/driver-api/fpga/fpga-programming.rst
> | 103 ++++++++++++++++++ Documentation/driver-api/fpga/fpga-region.rst
> | 91 ++++++++-------- Documentation/driver-api/fpga/index.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 | 10 +-
> drivers/fpga/dfl.c | 6 +-
> drivers/fpga/fpga-bridge.c | 74 +++++++++++--
> drivers/fpga/fpga-mgr.c | 67 ++++++++++--
> drivers/fpga/fpga-region.c | 68 ++++++++++--
> drivers/fpga/ice40-spi.c | 10 +-
> drivers/fpga/machxo2-spi.c | 11 +-
> drivers/fpga/of-fpga-region.c | 9 +-
> 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 | 22 +++-
> include/linux/fpga/fpga-region.h | 4 +
> 31 files changed, 437 insertions(+), 331 deletions(-)
> create mode 100644 Documentation/driver-api/fpga/fpga-programming.rst


--
Federico Vaga
[BE-CO-HT]

2018-08-14 23:48:28

by Alan Tull

Subject: Re: [PATCH 0/8] fpga: Add managed create in FPGA API

On Tue, Aug 14, 2018 at 4:44 PM, Federico Vaga <[email protected]> wrote:
> Hi Alan,
>
> I think you forgot to add:
>
> Suggested-by: Federico Vaga <[email protected]>
>
> on patches 2,3,4 which were suggested here
>
> https://lkml.org/lkml/2018/7/26/145
>
> and patch 8 here:
>
> https://lkml.org/lkml/2018/7/18/959

You are right and I appreciate the suggestions. I credited you on the
patch that this replaces [1]. I'll fix this in v2.

Alan

[1] https://lkml.org/lkml/2018/7/25/721

>
> On Tuesday, August 14, 2018 9:15:18 PM CEST Alan Tull wrote:
>> Fix one functional FPGA bug.
>>
>> Implement managed devm_fpga_(mgr|bridge|region)_create() functions.
>>
>> More documentation cleanup to make the documentation more helpful
>> and to emphasize the use of FPGA regions for FPGA programming.
>>
>> Fix a few minor documentation corrections.
>>
>> Alan Tull (8):
>> fpga: do not access region struct after fpga_region_unregister
>> fpga: mgr: add devm_fpga_mgr_create
>> fpga: bridge: add devm_fpga_bridge_create
>> fpga: add devm_fpga_region_create
>> dt-bindings: fpga: fix freeze controller compatible in region doc
>> fpga: bridge: fix obvious function documentation error
>> docs: fpga: document fpga manager flags
>> docs: fpga: document programming fpgas using regions
>>
>> .../devicetree/bindings/fpga/fpga-region.txt | 4 +-
>> Documentation/driver-api/fpga/fpga-bridge.rst | 37 ++-----
>> Documentation/driver-api/fpga/fpga-mgr.rst | 121
>> +++------------------ Documentation/driver-api/fpga/fpga-programming.rst
>> | 103 ++++++++++++++++++ Documentation/driver-api/fpga/fpga-region.rst
>> | 91 ++++++++-------- Documentation/driver-api/fpga/index.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 | 10 +-
>> drivers/fpga/dfl.c | 6 +-
>> drivers/fpga/fpga-bridge.c | 74 +++++++++++--
>> drivers/fpga/fpga-mgr.c | 67 ++++++++++--
>> drivers/fpga/fpga-region.c | 68 ++++++++++--
>> drivers/fpga/ice40-spi.c | 10 +-
>> drivers/fpga/machxo2-spi.c | 11 +-
>> drivers/fpga/of-fpga-region.c | 9 +-
>> 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 | 22 +++-
>> include/linux/fpga/fpga-region.h | 4 +
>> 31 files changed, 437 insertions(+), 331 deletions(-)
>> create mode 100644 Documentation/driver-api/fpga/fpga-programming.rst
>
>
> --
> Federico Vaga
> [BE-CO-HT]
>
>

2018-08-15 02:42:08

by Randy Dunlap

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

On 08/14/2018 12:15 PM, Alan Tull 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]>
> ---
> Documentation/driver-api/fpga/fpga-bridge.rst | 38 ++-----
> Documentation/driver-api/fpga/fpga-mgr.rst | 117 ++-------------------
> Documentation/driver-api/fpga/fpga-programming.rst | 103 ++++++++++++++++++
> Documentation/driver-api/fpga/fpga-region.rst | 92 ++++++++--------
> Documentation/driver-api/fpga/index.rst | 2 +
> 5 files changed, 166 insertions(+), 186 deletions(-)
> create mode 100644 Documentation/driver-api/fpga/fpga-programming.rst
>

Hi,
A few comments below...


> diff --git a/Documentation/driver-api/fpga/fpga-programming.rst b/Documentation/driver-api/fpga/fpga-programming.rst
> new file mode 100644
> index 0000000..cc34d17
> --- /dev/null
> +++ b/Documentation/driver-api/fpga/fpga-programming.rst
> @@ -0,0 +1,103 @@
> +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 a FPGA manager

to an FPGA

> +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);
> + if (ret)
> + return ret;
> +

always deallocate and then do:
if (ret)
return ret;
?

> + /* Deallocate the image info if you're done with it */
> + fpga_image_info_free(info);
> +
> + /* Now enumerate whatever hardware has appeared in the FPGA. */
> +
> +API for programming an FPGA
> +---------------------------
> +
> +* :c:func:`fpga_region_program_fpga` — Program a FPGA
> +* :c:type:`fpga_image_info` — Specifies what FPGA image to program
> +* :c:func:`fpga_image_info_alloc()` — Allocate a FPGA image info struct
> +* :c:func:`fpga_image_info_free()` — Free a 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

[snip]

> 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 a FPGA region

an FPGA

> +* :c:func:`fpga_region_unregister` — Unregister a FPGA region

an FPGA

> +
> +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 a FPGA manager, raise ref count

an FPGA

> +* :c:func:`of_fpga_mgr_get` — Get a reference to a FPGA manager, raise ref count,

an FPGA

> + given a device node.
> +* :c:func:`fpga_mgr_put` — Put a FPGA manager

an FPGA

> +
> +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 a FPGA bridge, add it to a
> + list
> +* :c:func:`of_fpga_bridge_get_to_list` — Get a ref of a 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
>



--
~Randy

2018-08-15 16:33:56

by Alan Tull

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

On Tue, Aug 14, 2018 at 9:40 PM, Randy Dunlap <[email protected]> wrote:

Hi Randy,

> On 08/14/2018 12:15 PM, Alan Tull 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]>
>> ---
>> Documentation/driver-api/fpga/fpga-bridge.rst | 38 ++-----
>> Documentation/driver-api/fpga/fpga-mgr.rst | 117 ++-------------------
>> Documentation/driver-api/fpga/fpga-programming.rst | 103 ++++++++++++++++++
>> Documentation/driver-api/fpga/fpga-region.rst | 92 ++++++++--------
>> Documentation/driver-api/fpga/index.rst | 2 +
>> 5 files changed, 166 insertions(+), 186 deletions(-)
>> create mode 100644 Documentation/driver-api/fpga/fpga-programming.rst
>>
>
> Hi,
> A few comments below...
>
>
>> diff --git a/Documentation/driver-api/fpga/fpga-programming.rst b/Documentation/driver-api/fpga/fpga-programming.rst
>> new file mode 100644
>> index 0000000..cc34d17
>> --- /dev/null
>> +++ b/Documentation/driver-api/fpga/fpga-programming.rst
>> @@ -0,0 +1,103 @@
>> +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 a FPGA manager
>
> to an FPGA

Yes

>
>> +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);
>> + if (ret)
>> + return ret;
>> +
>
> always deallocate and then do:
> if (ret)
> return ret;
> ?

Yep, I'll fix it.

>
>> + /* Deallocate the image info if you're done with it */
>> + fpga_image_info_free(info);
>> +
>> + /* Now enumerate whatever hardware has appeared in the FPGA. */
>> +
>> +API for programming an FPGA
>> +---------------------------
>> +
>> +* :c:func:`fpga_region_program_fpga` — Program a FPGA
>> +* :c:type:`fpga_image_info` — Specifies what FPGA image to program
>> +* :c:func:`fpga_image_info_alloc()` — Allocate a FPGA image info struct
>> +* :c:func:`fpga_image_info_free()` — Free a 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
>
> [snip]
>
>> 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 a FPGA region
>
> an FPGA
>
>> +* :c:func:`fpga_region_unregister` — Unregister a FPGA region
>
> an FPGA
>
>> +
>> +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 a FPGA manager, raise ref count
>
> an FPGA
>
>> +* :c:func:`of_fpga_mgr_get` — Get a reference to a FPGA manager, raise ref count,
>
> an FPGA
>
>> + given a device node.
>> +* :c:func:`fpga_mgr_put` — Put a FPGA manager
>
> an FPGA

I'll fix these too; I'll go through this folder and s/a FPGA/an FPGA/g

>
>> +
>> +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 a FPGA bridge, add it to a
>> + list
>> +* :c:func:`of_fpga_bridge_get_to_list` — Get a ref of a 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
>>
>
>
>
> --
> ~Randy

Thanks for the review!

Alan

2018-08-16 19:04:16

by Moritz Fischer

Subject: Re: [PATCH 7/8] docs: fpga: document fpga manager flags

Hi Alan,

On Tue, Aug 14, 2018 at 02:15:25PM -0500, Alan Tull wrote:
> Add flags #defines to kerneldoc documentation in a
> useful place.
>
> Signed-off-by: Alan Tull <[email protected]>
> ---
> Documentation/driver-api/fpga/fpga-mgr.rst | 5 +++++
> include/linux/fpga/fpga-mgr.h | 18 ++++++++++++------
> 2 files changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/driver-api/fpga/fpga-mgr.rst b/Documentation/driver-api/fpga/fpga-mgr.rst
> index 431556d..db8885e 100644
> --- a/Documentation/driver-api/fpga/fpga-mgr.rst
> +++ b/Documentation/driver-api/fpga/fpga-mgr.rst
> @@ -183,6 +183,11 @@ API for implementing a new FPGA Manager driver
> 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
>
> diff --git a/include/linux/fpga/fpga-mgr.h b/include/linux/fpga/fpga-mgr.h
> index 1ca02ce..777c502 100644
> --- a/include/linux/fpga/fpga-mgr.h
> +++ b/include/linux/fpga/fpga-mgr.h
> @@ -53,12 +53,18 @@ enum fpga_mgr_states {
> FPGA_MGR_STATE_OPERATING,
> };
>
> -/*
> - * FPGA Manager flags
> - * FPGA_MGR_PARTIAL_RECONFIG: do partial reconfiguration if supported
> - * FPGA_MGR_EXTERNAL_CONFIG: FPGA has been configured prior to Linux booting
> - * FPGA_MGR_BITSTREAM_LSB_FIRST: SPI bitstream bit order is LSB first

We are actually missing the ENCRYPTED flag here, can you add that while
we're at it? I must've forgotten to add that back then ... derp.

> - * FPGA_MGR_COMPRESSED_BITSTREAM: FPGA bitstream is compressed
> +/**
> + * DOC: FPGA Manager flags
> + *
> + * Flags used in the &fpga_image_info->flags field
> + *
> + * %FPGA_MGR_PARTIAL_RECONFIG: do partial reconfiguration if supported
> + *
> + * %FPGA_MGR_EXTERNAL_CONFIG: FPGA has been configured prior to Linux booting
> + *
> + * %FPGA_MGR_BITSTREAM_LSB_FIRST: SPI bitstream bit order is LSB first
> + *
> + * %FPGA_MGR_COMPRESSED_BITSTREAM: FPGA bitstream is compressed
> */
> #define FPGA_MGR_PARTIAL_RECONFIG BIT(0)
> #define FPGA_MGR_EXTERNAL_CONFIG BIT(1)
> --
> 2.7.4
>

Thanks,
Moritz

2018-08-16 20:22:18

by Alan Tull

Subject: Re: [PATCH 7/8] docs: fpga: document fpga manager flags

On Thu, Aug 16, 2018 at 2:02 PM, Moritz Fischer <[email protected]> wrote:
Hi Moritz,

> Hi Alan,
>
> On Tue, Aug 14, 2018 at 02:15:25PM -0500, Alan Tull wrote:
>> Add flags #defines to kerneldoc documentation in a
>> useful place.
>>
>> Signed-off-by: Alan Tull <[email protected]>
>> ---
>> Documentation/driver-api/fpga/fpga-mgr.rst | 5 +++++
>> include/linux/fpga/fpga-mgr.h | 18 ++++++++++++------
>> 2 files changed, 17 insertions(+), 6 deletions(-)
>>
>> diff --git a/Documentation/driver-api/fpga/fpga-mgr.rst b/Documentation/driver-api/fpga/fpga-mgr.rst
>> index 431556d..db8885e 100644
>> --- a/Documentation/driver-api/fpga/fpga-mgr.rst
>> +++ b/Documentation/driver-api/fpga/fpga-mgr.rst
>> @@ -183,6 +183,11 @@ API for implementing a new FPGA Manager driver
>> 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
>>
>> diff --git a/include/linux/fpga/fpga-mgr.h b/include/linux/fpga/fpga-mgr.h
>> index 1ca02ce..777c502 100644
>> --- a/include/linux/fpga/fpga-mgr.h
>> +++ b/include/linux/fpga/fpga-mgr.h
>> @@ -53,12 +53,18 @@ enum fpga_mgr_states {
>> FPGA_MGR_STATE_OPERATING,
>> };
>>
>> -/*
>> - * FPGA Manager flags
>> - * FPGA_MGR_PARTIAL_RECONFIG: do partial reconfiguration if supported
>> - * FPGA_MGR_EXTERNAL_CONFIG: FPGA has been configured prior to Linux booting
>> - * FPGA_MGR_BITSTREAM_LSB_FIRST: SPI bitstream bit order is LSB first
>
> We are actually missing the ENCRYPTED flag here, can you add that while
> we're at it? I must've forgotten to add that back then ... derp.

Sure!

>
>> - * FPGA_MGR_COMPRESSED_BITSTREAM: FPGA bitstream is compressed
>> +/**
>> + * DOC: FPGA Manager flags
>> + *
>> + * Flags used in the &fpga_image_info->flags field
>> + *
>> + * %FPGA_MGR_PARTIAL_RECONFIG: do partial reconfiguration if supported
>> + *
>> + * %FPGA_MGR_EXTERNAL_CONFIG: FPGA has been configured prior to Linux booting

How about this description?

+ *
+ * %FPGA_MGR_ENCRYPTED_BITSTREAM: indicates bitstream is encrypted

>> + *
>> + * %FPGA_MGR_BITSTREAM_LSB_FIRST: SPI bitstream bit order is LSB first
>> + *
>> + * %FPGA_MGR_COMPRESSED_BITSTREAM: FPGA bitstream is compressed
>> */
>> #define FPGA_MGR_PARTIAL_RECONFIG BIT(0)
>> #define FPGA_MGR_EXTERNAL_CONFIG BIT(1)
>> --
>> 2.7.4
>>
>
> Thanks,
> Moritz

Thanks,
Alan

2018-08-16 22:04:06

by Moritz Fischer

Subject: Re: [PATCH 7/8] docs: fpga: document fpga manager flags

On Thu, Aug 16, 2018 at 03:20:12PM -0500, Alan Tull wrote:
> On Thu, Aug 16, 2018 at 2:02 PM, Moritz Fischer <[email protected]> wrote:
> Hi Moritz,
>
> > Hi Alan,
> >
> > On Tue, Aug 14, 2018 at 02:15:25PM -0500, Alan Tull wrote:
> >> Add flags #defines to kerneldoc documentation in a
> >> useful place.
> >>
> >> Signed-off-by: Alan Tull <[email protected]>
> >> ---
> >> Documentation/driver-api/fpga/fpga-mgr.rst | 5 +++++
> >> include/linux/fpga/fpga-mgr.h | 18 ++++++++++++------
> >> 2 files changed, 17 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/Documentation/driver-api/fpga/fpga-mgr.rst b/Documentation/driver-api/fpga/fpga-mgr.rst
> >> index 431556d..db8885e 100644
> >> --- a/Documentation/driver-api/fpga/fpga-mgr.rst
> >> +++ b/Documentation/driver-api/fpga/fpga-mgr.rst
> >> @@ -183,6 +183,11 @@ API for implementing a new FPGA Manager driver
> >> 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
> >>
> >> diff --git a/include/linux/fpga/fpga-mgr.h b/include/linux/fpga/fpga-mgr.h
> >> index 1ca02ce..777c502 100644
> >> --- a/include/linux/fpga/fpga-mgr.h
> >> +++ b/include/linux/fpga/fpga-mgr.h
> >> @@ -53,12 +53,18 @@ enum fpga_mgr_states {
> >> FPGA_MGR_STATE_OPERATING,
> >> };
> >>
> >> -/*
> >> - * FPGA Manager flags
> >> - * FPGA_MGR_PARTIAL_RECONFIG: do partial reconfiguration if supported
> >> - * FPGA_MGR_EXTERNAL_CONFIG: FPGA has been configured prior to Linux booting
> >> - * FPGA_MGR_BITSTREAM_LSB_FIRST: SPI bitstream bit order is LSB first
> >
> > We are actually missing the ENCRYPTED flag here, can you add that while
> > we're at it? I must've forgotten to add that back then ... derp.
>
> Sure!
>
> >
> >> - * FPGA_MGR_COMPRESSED_BITSTREAM: FPGA bitstream is compressed
> >> +/**
> >> + * DOC: FPGA Manager flags
> >> + *
> >> + * Flags used in the &fpga_image_info->flags field
> >> + *
> >> + * %FPGA_MGR_PARTIAL_RECONFIG: do partial reconfiguration if supported
> >> + *
> >> + * %FPGA_MGR_EXTERNAL_CONFIG: FPGA has been configured prior to Linux booting
>
> How about this description?
>
> + *
> + * %FPGA_MGR_ENCRYPTED_BITSTREAM: indicates bitstream is encrypted

Works for me :)
>
> >> + *
> >> + * %FPGA_MGR_BITSTREAM_LSB_FIRST: SPI bitstream bit order is LSB first
> >> + *
> >> + * %FPGA_MGR_COMPRESSED_BITSTREAM: FPGA bitstream is compressed
> >> */
> >> #define FPGA_MGR_PARTIAL_RECONFIG BIT(0)
> >> #define FPGA_MGR_EXTERNAL_CONFIG BIT(1)
> >> --
> >> 2.7.4
> >>
> >
> > Thanks,
> > Moritz
>
> Thanks,
> Alan