Skip to contentSkip to conversation

Update kernel to latest FMC bus release

PATCH Superseded Pat Riehecky 3 participants 16 messages Jun 30, 2017 – Jul 17, 2017

lore
Share & tools
lore

Pat Riehecky

author
#0 lore raw

cover Update kernel to latest FMC bus release

The following patches sync up the mainline kernel's FMC bus with the upstream release tagged v2017-06.

Commits (written by Federico Vaga) have been squashed into logical units.

These patches address kernel bug 195653.

These specific changes have been live within the FMC repo since 2015 and in active use by FMC projects since then.

The patches include 1 bugfix and 4 enhancements:

Bugfix:
0001-fmc-remove-unused-variable.patch - remove an unused int

Enhancement:
0002-fmc-hide-fmc-operations-behind-helpers.patch - user helper functions to hide internal semantics 0003-fmc-The-only-way-to-dump-the-SDB-is-from-debugfs.patch - use debugfs to dump the device 0004-fmc-change-registration-prototype.patch - add a second prototype for device registration 0005-fmc-carrier-can-program-FPGA-on-registration.patch - permit programming the FPGA on registration

Upstream git repo: http://www.ohwr.org/projects/fmc-bus/repository/show?rev=fmc-bus-v2017-06

FMC maintainer: Alessandro Rubini <[email protected]> Initial approver of FMC: Greg KH <[email protected]> Bugzilla assigned to: [email protected]

Federico Vaga (5):
drivers/fmc: remove unused variable
drivers/fmc: hide fmc operations behind helpers drivers/fmc: The only way to dump the SDB is from debugfs drivers/fmc: change registration prototype drivers/fmc: carrier can program FPGA on registration

 drivers/fmc/Makefile           |   1 +
 drivers/fmc/fmc-chardev.c      |   3 +-
 drivers/fmc/fmc-core.c         |  95 ++++++++++++++++++++--
 drivers/fmc/fmc-debug.c        | 173 +++++++++++++++++++++++++++++++++++++++++
 drivers/fmc/fmc-dump.c         |  41 ----------
 drivers/fmc/fmc-match.c        |   2 +-
 drivers/fmc/fmc-private.h      |   9 +++
 drivers/fmc/fmc-sdb.c          | 119 ++++++----------------------
 drivers/fmc/fmc-trivial.c      |  20 ++---
 drivers/fmc/fmc-write-eeprom.c |   8 +-
 drivers/fmc/fru-parse.c        |   3 +-
 include/linux/fmc.h            |  39 +++++++++-
 12 files changed, 349 insertions(+), 164 deletions(-)
 create mode 100644 drivers/fmc/fmc-debug.c
 create mode 100644 drivers/fmc/fmc-private.h

--
1.8.3.1

author
#1 lore raw

PATCH 1/5 drivers/fmc: remove unused variable

Signed-off-by Federico Vaga <[email protected]>
---
 drivers/fmc/fru-parse.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/fmc/fru-parse.c +1 -2
index cb46263..eb21480 100644
@@ -31,12 +31,11 @@ static char *__fru_alloc_get_tl(struct fru_common_header *header, int nr)
 {
 	struct fru_type_length *tl;
 	char *res;
-	int len;
 
 	tl = __fru_get_board_tl(header, nr);
 	if (!tl)
 		return NULL;
-	len = fru_strlen(tl);
+
 	res = fru_alloc(fru_strlen(tl) + 1);
 	if (!res)
 		return NULL;

--
1.8.3.1

author
#2 lore raw

PATCH 2/5 drivers/fmc: hide fmc operations behind helpers

In reply to Pat Riehecky

This gave us more freedom to change/add/remove operations without recompiling all device driver.

Typically, Carrier board implement the fmc operations, so they will not use these helpers.

Signed-off-by Federico Vaga <[email protected]>
---
 drivers/fmc/fmc-chardev.c      |  3 +--
 drivers/fmc/fmc-core.c         | 55 ++++++++++++++++++++++++++++++++++++++++++
 drivers/fmc/fmc-match.c        |  2 +-
 drivers/fmc/fmc-trivial.c      | 20 ++++++---------
 drivers/fmc/fmc-write-eeprom.c |  8 +++---
 include/linux/fmc.h            | 18 ++++++++++++++
 6 files changed, 87 insertions(+), 19 deletions(-)
drivers/fmc/fmc-chardev.c +1 -2
index ace6ef2..5ecf409 100644
@@ -129,8 +129,7 @@ static int fc_probe(struct fmc_device *fmc)
 
 	struct fc_instance *fc;
 
-	if (fmc->op->validate)
-		index = fmc->op->validate(fmc, &fc_drv);
+	index = fmc_validate(fmc, &fc_drv);
 	if (index < 0)
 		return -EINVAL; /* not our device: invalid */
drivers/fmc/fmc-core.c +55 -0

61 lines not on this page · raw message

drivers/fmc/fmc-match.c +1 -1
index 104a5ef..a0956d1 100644
@@ -63,7 +63,7 @@ int fmc_fill_id_info(struct fmc_device *fmc)
 		if (!fmc->eeprom)
 			return -ENOMEM;
 		allocated = 1;
-		ret = fmc->op->read_ee(fmc, 0, fmc->eeprom, fmc->eeprom_len);
+		ret = fmc_read_ee(fmc, 0, fmc->eeprom, fmc->eeprom_len);
 		if (ret < 0)
 			goto out;
 	}
drivers/fmc/fmc-trivial.c +8 -12
index 6c590f5..8defdee 100644
@@ -24,7 +24,7 @@ static irqreturn_t t_handler(int irq, void *dev_id)
 {
 	struct fmc_device *fmc = dev_id;
 
-	fmc->op->irq_ack(fmc);
+	fmc_irq_ack(fmc);
 	dev_info(&fmc->dev, "received irq %i\n", irq);
 	return IRQ_HANDLED;
 }

@@ -46,25 +46,21 @@ static int t_probe(struct fmc_device *fmc)
 	int ret;
 	int index = 0;
 
-	if (fmc->op->validate)
-		index = fmc->op->validate(fmc, &t_drv);
+	index = fmc_validate(fmc, &t_drv);
 	if (index < 0)
 		return -EINVAL; /* not our device: invalid */
 
-	ret = fmc->op->irq_request(fmc, t_handler, "fmc-trivial", IRQF_SHARED);
+	ret = fmc_irq_request(fmc, t_handler, "fmc-trivial", IRQF_SHARED);
 	if (ret < 0)
 		return ret;
 	/* ignore error code of call below, we really don't care */
-	fmc->op->gpio_config(fmc, t_gpio, ARRAY_SIZE(t_gpio));
+	fmc_gpio_config(fmc, t_gpio, ARRAY_SIZE(t_gpio));
 
-	/* Reprogram, if asked to. ESRCH == no filename specified */
-	ret = -ESRCH;
-	if (fmc->op->reprogram)
-		ret = fmc->op->reprogram(fmc, &t_drv, "");
-	if (ret == -ESRCH)
+	ret = fmc_reprogram(fmc, &t_drv, "", 0);
+	if (ret == -EPERM) /* programming not supported */
 		ret = 0;
 	if (ret < 0)
-		fmc->op->irq_free(fmc);
+		fmc_irq_free(fmc);
 
 	/* FIXME: reprogram LM32 too */
 	return ret;

@@ -72,7 +68,7 @@ static int t_probe(struct fmc_device *fmc)
 
 static int t_remove(struct fmc_device *fmc)
 {
-	fmc->op->irq_free(fmc);
+	fmc_irq_free(fmc);
 	return 0;
 }
drivers/fmc/fmc-write-eeprom.c +4 -4
index 9bb2cbd..3eb81bb 100644
@@ -50,7 +50,7 @@ static int fwe_run_tlv(struct fmc_device *fmc, const struct firmware *fw,
 		if (write) {
 			dev_info(&fmc->dev, "write %i bytes at 0x%04x\n",
 				 thislen, thisaddr);
-			err = fmc->op->write_ee(fmc, thisaddr, p + 5, thislen);
+			err = fmc_write_ee(fmc, thisaddr, p + 5, thislen);
 		}
 		if (err < 0) {
 			dev_err(&fmc->dev, "write failure @0x%04x\n",

@@ -70,7 +70,7 @@ static int fwe_run_bin(struct fmc_device *fmc, const struct firmware *fw)
 	int ret;
 
 	dev_info(&fmc->dev, "programming %zi bytes\n", fw->size);
-	ret = fmc->op->write_ee(fmc, 0, (void *)fw->data, fw->size);
+	ret = fmc_write_ee(fmc, 0, (void *)fw->data, fw->size);
 	if (ret < 0) {
 		dev_info(&fmc->dev, "write_eeprom: error %i\n", ret);
 		return ret;

@@ -115,8 +115,8 @@ static int fwe_probe(struct fmc_device *fmc)
 			KBUILD_MODNAME);
 		return -ENODEV;
 	}
-	if (fmc->op->validate)
-		index = fmc->op->validate(fmc, &fwe_drv);
+
+	index = fmc_validate(fmc, &fwe_drv);
 	if (index < 0) {
 		pr_err("%s: refusing device \"%s\"\n", KBUILD_MODNAME,
 		       dev_name(dev));
include/linux/fmc.h +18 -0
index a5f0aa5..8bb4a15 100644
@@ -234,4 +234,22 @@ static inline void fmc_set_drvdata(struct fmc_device *fmc, void *data)
 extern void fmc_dump_eeprom(const struct fmc_device *fmc);
 extern void fmc_dump_sdb(const struct fmc_device *fmc);
 
+/* helpers for FMC operations */
+extern int fmc_irq_request(struct fmc_device *fmc, irq_handler_t h,
+			   char *name, int flags);
+extern void fmc_irq_free(struct fmc_device *fmc);
+extern void fmc_irq_ack(struct fmc_device *fmc);
+extern int fmc_validate(struct fmc_device *fmc, struct fmc_driver *drv);
+extern int fmc_gpio_config(struct fmc_device *fmc, struct fmc_gpio *gpio,
+			   int ngpio);
+extern int fmc_read_ee(struct fmc_device *fmc, int pos, void *d, int l);
+extern int fmc_write_ee(struct fmc_device *fmc, int pos, const void *d, int l);
+
+/* helpers for FMC operations */
+extern int fmc_irq_request(struct fmc_device *fmc, irq_handler_t h,
+			   char *name, int flags);
+extern void fmc_irq_free(struct fmc_device *fmc);
+extern void fmc_irq_ack(struct fmc_device *fmc);
+extern int fmc_validate(struct fmc_device *fmc, struct fmc_driver *drv);
+
 #endif /* __LINUX_FMC_H__ */

--
1.8.3.1

author
#3 lore raw

PATCH 3/5 drivers/fmc: The only way to dump the SDB is from debugfs

In reply to Pat Riehecky

Driver should not call fmc_sdb_dump() anymore. (actually they can but the operation is not supported, so it will print an error message)

Signed-off-by Federico Vaga <[email protected]>
---
 drivers/fmc/Makefile      |   1 +
 drivers/fmc/fmc-core.c    |   7 +-
 drivers/fmc/fmc-debug.c   | 173 ++++++++++++++++++++++++++++++++++++++++++++++
 drivers/fmc/fmc-dump.c    |  41 -----------
 drivers/fmc/fmc-private.h |   9 +++
 drivers/fmc/fmc-sdb.c     |  99 +-------------------------
 include/linux/fmc.h       |   4 +-
 7 files changed, 195 insertions(+), 139 deletions(-)
 create mode 100644 drivers/fmc/fmc-debug.c
 create mode 100644 drivers/fmc/fmc-private.h
drivers/fmc/Makefile +1 -0
index b945291..e809322 100644
@@ -6,6 +6,7 @@ fmc-y += fmc-match.o
 fmc-y += fmc-sdb.o
 fmc-y += fru-parse.o
 fmc-y += fmc-dump.o
+fmc-y += fmc-debug.o
 
 obj-$(CONFIG_FMC_FAKEDEV) += fmc-fakedev.o
 obj-$(CONFIG_FMC_TRIVIAL) += fmc-trivial.o
drivers/fmc/fmc-core.c +6 -1
index 5263d06..ef6d8ac 100644
@@ -13,6 +13,9 @@
 #include <linux/init.h>
 #include <linux/device.h>
 #include <linux/fmc.h>
+#include <linux/fmc-sdb.h>
+
+#include "fmc-private.h"
 
 static int fmc_check_version(unsigned long version, const char *name)
 {

@@ -289,7 +292,7 @@ int fmc_device_register_n(struct fmc_device **devs, int n)
 		}
 		/* This device went well, give information to the user */
 		fmc_dump_eeprom(fmc);
-		fmc_dump_sdb(fmc);
+		fmc_debug_init(fmc);
 	}
 	return 0;
 

@@ -301,6 +304,7 @@ int fmc_device_register_n(struct fmc_device **devs, int n)
 
 	kfree(devarray);
 	for (i--; i >= 0; i--) {
+		fmc_debug_exit(devs[i]);
 		sysfs_remove_bin_file(&devs[i]->dev.kobj, &fmc_eeprom_attr);
 		device_del(&devs[i]->dev);
 		fmc_free_id_info(devs[i]);

@@ -328,6 +332,7 @@ void fmc_device_unregister_n(struct fmc_device **devs, int n)
 	kfree(devs[0]->devarray);
 
 	for (i = 0; i < n; i++) {
+		fmc_debug_exit(devs[i]);
 		sysfs_remove_bin_file(&devs[i]->dev.kobj, &fmc_eeprom_attr);
 		device_del(&devs[i]->dev);
 		fmc_free_id_info(devs[i]);
drivers/fmc/fmc-debug.c new file +173 -0

173 lines not on this page · raw message

drivers/fmc/fmc-dump.c +0 -41

50 lines not on this page · raw message

drivers/fmc/fmc-private.h new file +9 -0
new file mode 100644
index 0000000..1e51366
@@ -0,0 +1,9 @@
+/*
+ * Copyright (C) 2015 CERN (www.cern.ch)
+ * Author: Federico Vaga <[email protected]>
+ *
+ * Released according to the GNU GPL, version 2 or any later version.
+ */
+
+extern int fmc_debug_init(struct fmc_device *fmc);
+extern void fmc_debug_exit(struct fmc_device *fmc);
drivers/fmc/fmc-sdb.c +3 -96

111 lines not on this page · raw message

include/linux/fmc.h +3 -1
index 8bb4a15..5c8df0c 100644
@@ -180,6 +180,9 @@ struct fmc_device {
 	uint32_t device_id;		/* Filled by the device */
 	char *mezzanine_name;		/* Defaults to ``fmc'' */
 	void *mezzanine_data;
+
+	struct dentry *dbg_dir;
+	struct dentry *dbg_sdb_dump;
 };
 #define to_fmc_device(x) container_of((x), struct fmc_device, dev)
 

@@ -232,7 +235,6 @@ static inline void fmc_set_drvdata(struct fmc_device *fmc, void *data)
 extern int fmc_fill_id_info(struct fmc_device *fmc);
 extern void fmc_free_id_info(struct fmc_device *fmc);
 extern void fmc_dump_eeprom(const struct fmc_device *fmc);
-extern void fmc_dump_sdb(const struct fmc_device *fmc);
 
 /* helpers for FMC operations */
 extern int fmc_irq_request(struct fmc_device *fmc, irq_handler_t h,

--
1.8.3.1

author
#4 lore raw

PATCH 4/5 drivers/fmc: change registration prototype

In reply to Pat Riehecky

Permit use of either fmc_device_register_n or fmc_device_register_n_gw depending on the type of device in use.

Signed-off-by Federico Vaga <[email protected]>
---
 drivers/fmc/fmc-core.c | 15 ++++++++++++++-
 include/linux/fmc.h    | 13 +++++++++++--
 2 files changed, 25 insertions(+), 3 deletions(-)
drivers/fmc/fmc-core.c +14 -1
index ef6d8ac..eabeac0 100644
@@ -199,7 +199,8 @@ void fmc_driver_unregister(struct fmc_driver *drv)
  * When a device set is registered, all eeproms must be read
  * and all FRUs must be parsed
  */
-int fmc_device_register_n(struct fmc_device **devs, int n)
+int fmc_device_register_n_gw(struct fmc_device **devs, int n,
+			  struct fmc_gateware *gw)
 {
 	struct fmc_device *fmc, **devarray;
 	uint32_t device_id;

@@ -313,8 +314,20 @@ int fmc_device_register_n(struct fmc_device **devs, int n)
 	return ret;
 
 }
+EXPORT_SYMBOL(fmc_device_register_n_gw);
+
+int fmc_device_register_n(struct fmc_device **devs, int n)
+{
+	return fmc_device_register_n_gw(devs, n, NULL);
+}
 EXPORT_SYMBOL(fmc_device_register_n);
 
+int fmc_device_register_gw(struct fmc_device *fmc, struct fmc_gateware *gw)
+{
+	return fmc_device_register_n_gw(&fmc, 1, gw);
+}
+EXPORT_SYMBOL(fmc_device_register_gw);
+
 int fmc_device_register(struct fmc_device *fmc)
 {
 	return fmc_device_register_n(&fmc, 1);
include/linux/fmc.h +11 -2
index 5c8df0c..b6c73d5 100644
@@ -220,14 +220,23 @@ static inline void fmc_set_drvdata(struct fmc_device *fmc, void *data)
 	dev_set_drvdata(&fmc->dev, data);
 }
 
-/* The 4 access points */
+struct fmc_gateware {
+	void *bitstream;
+	unsigned long len;
+};
+
+/* The 5 access points */
 extern int fmc_driver_register(struct fmc_driver *drv);
 extern void fmc_driver_unregister(struct fmc_driver *drv);
 extern int fmc_device_register(struct fmc_device *tdev);
+extern int fmc_device_register_gw(struct fmc_device *tdev,
+				  struct fmc_gateware *gw);
 extern void fmc_device_unregister(struct fmc_device *tdev);
 
-/* Two more for device sets, all driven by the same FPGA */
+/* Three more for device sets, all driven by the same FPGA */
 extern int fmc_device_register_n(struct fmc_device **devs, int n);
+extern int fmc_device_register_n_gw(struct fmc_device **devs, int n,
+				    struct fmc_gateware *gw);
 extern void fmc_device_unregister_n(struct fmc_device **devs, int n);
 
 /* Internal cross-calls between files; not exported to other modules */

--
1.8.3.1

author
#5 lore raw

PATCH 5/5 drivers/fmc: carrier can program FPGA on registration

In reply to Pat Riehecky

The initial FPGA may require programming before it is useful.

Signed-off-by Federico Vaga <[email protected]>
---
 drivers/fmc/fmc-core.c | 18 +++++++++++++++---
 drivers/fmc/fmc-sdb.c  | 24 ++++++++++++++++++++++++
 include/linux/fmc.h    |  4 ++++
 3 files changed, 43 insertions(+), 3 deletions(-)
drivers/fmc/fmc-core.c +15 -3
index eabeac0..cec3b8d 100644
@@ -280,6 +280,21 @@ int fmc_device_register_n_gw(struct fmc_device **devs, int n,
 		else
 			dev_set_name(&fmc->dev, "%s-%04x", fmc->mezzanine_name,
 				     device_id);
+
+		if (gw) {
+			/*
+			 * The carrier already know the bitstream to load
+			 * for this set of FMC mezzanines.
+			 */
+			ret = fmc->op->reprogram_raw(fmc, NULL,
+						     gw->bitstream, gw->len);
+			if (ret) {
+				dev_warn(fmc->hwdev,
+					 "Invalid gateware for FMC mezzanine\n");
+				goto out;
+			}
+		}
+
 		ret = device_add(&fmc->dev);
 		if (ret < 0) {
 			dev_err(fmc->hwdev, "Slot %i: Failed in registering "

@@ -300,9 +315,6 @@ int fmc_device_register_n_gw(struct fmc_device **devs, int n,
 out1:
 	device_del(&fmc->dev);
 out:
-	fmc_free_id_info(fmc);
-	put_device(&fmc->dev);
-
 	kfree(devarray);
 	for (i--; i >= 0; i--) {
 		fmc_debug_exit(devs[i]);
drivers/fmc/fmc-sdb.c +24 -0
index 89e37a6..ffdc176 100644
@@ -127,6 +127,30 @@ int fmc_free_sdb_tree(struct fmc_device *fmc)
 EXPORT_SYMBOL(fmc_free_sdb_tree);
 
 /* This helper calls reprogram and inizialized sdb as well */
+int fmc_reprogram_raw(struct fmc_device *fmc, struct fmc_driver *d,
+		      void *gw, unsigned long len, int sdb_entry)
+{
+	int ret;
+
+	ret = fmc->op->reprogram_raw(fmc, d, gw, len);
+	if (ret < 0)
+		return ret;
+	if (sdb_entry < 0)
+		return ret;
+
+	/* We are required to find SDB at a given offset */
+	ret = fmc_scan_sdb_tree(fmc, sdb_entry);
+	if (ret < 0) {
+		dev_err(&fmc->dev, "Can't find SDB at address 0x%x\n",
+			sdb_entry);
+		return -ENODEV;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(fmc_reprogram_raw);
+
+/* This helper calls reprogram and inizialized sdb as well */
 int fmc_reprogram(struct fmc_device *fmc, struct fmc_driver *d, char *gw,
 			 int sdb_entry)
 {
include/linux/fmc.h +4 -0
index b6c73d5..3dc8a1b 100644
@@ -132,6 +132,8 @@ struct fmc_operations {
 	uint32_t (*read32)(struct fmc_device *fmc, int offset);
 	void (*write32)(struct fmc_device *fmc, uint32_t value, int offset);
 	int (*validate)(struct fmc_device *fmc, struct fmc_driver *drv);
+	int (*reprogram_raw)(struct fmc_device *f, struct fmc_driver *d,
+			     void *gw, unsigned long len);
 	int (*reprogram)(struct fmc_device *f, struct fmc_driver *d, char *gw);
 	int (*irq_request)(struct fmc_device *fmc, irq_handler_t h,
 			   char *name, int flags);

@@ -144,6 +146,8 @@ struct fmc_operations {
 };
 
 /* Prefer this helper rather than calling of fmc->reprogram directly */
+int fmc_reprogram_raw(struct fmc_device *fmc, struct fmc_driver *d,
+		      void *gw, unsigned long len, int sdb_entry);
 extern int fmc_reprogram(struct fmc_device *f, struct fmc_driver *d, char *gw,
 		     int sdb_entry);

--
1.8.3.1

#6 lore raw
In reply to Pat Riehecky

Hello.

Thank you Pat for taking care of these patches.

Quoted 6 lines

Federico Vaga (5):
drivers/fmc: remove unused variable
drivers/fmc: hide fmc operations behind helpers drivers/fmc: The only way to dump the SDB is from debugfs drivers/fmc: change registration prototype drivers/fmc: carrier can program FPGA on registration

The main problem with your patches, as submitted, is that they miss the "From:" line to attribute Federico as author. Then I think you might add your own "Tested-by:".

I want to ack them but I'd love to run them first to have a second check. I should definitely manage it in a pair of days more. Meanwhile, if you want to post V2 with the fixes above that would be great. If needed, you can write me offlist.

thankyou for your submission and your patience
/alessandro

author
#7 lore raw

v2 cover Update kernel to latest FMC bus release

The version 2 patch set includes correct From: and Tested-by: entries.

The following patches sync up the mainline kernel's FMC bus with the upstream release tagged v2017-06.

Commits (written by Federico Vaga) have been squashed into logical units.

These patches address kernel bug 195653.

These specific changes have been live within the FMC repo since 2015 and in active use by FMC projects since then.

The patches include 1 bugfix and 4 enhancements:

Bugfix:
0001-fmc-remove-unused-variable.patch - remove an unused int

Enhancement:
0002-fmc-hide-fmc-operations-behind-helpers.patch - user helper functions to hide internal semantics 0003-fmc-The-only-way-to-dump-the-SDB-is-from-debugfs.patch - use debugfs to dump the device 0004-fmc-change-registration-prototype.patch - add a second prototype for device registration 0005-fmc-carrier-can-program-FPGA-on-registration.patch - permit programming the FPGA on registration

Upstream git repo: http://www.ohwr.org/projects/fmc-bus/repository/show?rev=fmc-bus-v2017-06

FMC maintainer: Alessandro Rubini <[email protected]> Initial approver of FMC: Greg KH <[email protected]> Bugzilla assigned to: [email protected]

author
#8 lore raw

PATCH v2 1/5 drivers/fmc: remove unused variable

From: Federico Vaga <[email protected]>

Signed-off-by Federico Vaga <[email protected]>
Tested-by Pat Riehecky <[email protected]>
---
 drivers/fmc/fru-parse.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/fmc/fru-parse.c +1 -2
index cb46263..eb21480 100644
@@ -31,12 +31,11 @@ static char *__fru_alloc_get_tl(struct fru_common_header *header, int nr)
 {
 	struct fru_type_length *tl;
 	char *res;
-	int len;
 
 	tl = __fru_get_board_tl(header, nr);
 	if (!tl)
 		return NULL;
-	len = fru_strlen(tl);
+
 	res = fru_alloc(fru_strlen(tl) + 1);
 	if (!res)
 		return NULL;

--
1.8.3.1

author
#10 lore raw

PATCH v2 2/5 drivers/fmc: hide fmc operations behind helpers

In reply to Pat Riehecky

This gave us more freedom to change/add/remove operations without recompiling all device driver.

Typically, Carrier board implement the fmc operations, so they will not use these helpers.

From: Federico Vaga <[email protected]>

Signed-off-by Federico Vaga <[email protected]>
Tested-by Pat Riehecky <[email protected]>
---
 drivers/fmc/fmc-chardev.c       |  3 +--
 drivers/fmc/fmc-core.c          | 55 ++++++++++++++++++++++++++++++++++++++++++++++
 drivers/fmc/fmc-match.c         |  2 +-
 drivers/fmc/fmc-trivial.c       | 20 +++++++----------
 drivers/fmc/fmc-write-eeprom.c  |  8 +++----
 include/linux/fmc.h | 18 +++++++++++++++
 6 files changed, 87 insertions(+), 19 deletions(-)
drivers/fmc/fmc-chardev.c +1 -2
index 38aa7c4..b3eb6a2 100644
@@ -129,8 +129,7 @@ static int fc_probe(struct fmc_device *fmc)
 
 	struct fc_instance *fc;
 
-	if (fmc->op->validate)
-		index = fmc->op->validate(fmc, &fc_drv);
+	index = fmc_validate(fmc, &fc_drv);
 	if (index < 0)
 		return -EINVAL; /* not our device: invalid */
drivers/fmc/fmc-core.c +55 -0

61 lines not on this page · raw message

drivers/fmc/fmc-match.c +1 -1
index 104a5ef..a0956d1 100644
@@ -63,7 +63,7 @@ int fmc_fill_id_info(struct fmc_device *fmc)
 		if (!fmc->eeprom)
 			return -ENOMEM;
 		allocated = 1;
-		ret = fmc->op->read_ee(fmc, 0, fmc->eeprom, fmc->eeprom_len);
+		ret = fmc_read_ee(fmc, 0, fmc->eeprom, fmc->eeprom_len);
 		if (ret < 0)
 			goto out;
 	}
drivers/fmc/fmc-trivial.c +8 -12
index d26a174..7c03946 100644
@@ -18,7 +18,7 @@ static irqreturn_t t_handler(int irq, void *dev_id)
 {
 	struct fmc_device *fmc = dev_id;
 
-	fmc->op->irq_ack(fmc);
+	fmc_irq_ack(fmc);
 	dev_info(&fmc->dev, "received irq %i\n", irq);
 	return IRQ_HANDLED;
 }

@@ -40,25 +40,21 @@ static int t_probe(struct fmc_device *fmc)
 	int ret;
 	int index = 0;
 
-	if (fmc->op->validate)
-		index = fmc->op->validate(fmc, &t_drv);
+	index = fmc_validate(fmc, &t_drv);
 	if (index < 0)
 		return -EINVAL; /* not our device: invalid */
 
-	ret = fmc->op->irq_request(fmc, t_handler, "fmc-trivial", IRQF_SHARED);
+	ret = fmc_irq_request(fmc, t_handler, "fmc-trivial", IRQF_SHARED);
 	if (ret < 0)
 		return ret;
 	/* ignore error code of call below, we really don't care */
-	fmc->op->gpio_config(fmc, t_gpio, ARRAY_SIZE(t_gpio));
+	fmc_gpio_config(fmc, t_gpio, ARRAY_SIZE(t_gpio));
 
-	/* Reprogram, if asked to. ESRCH == no filename specified */
-	ret = -ESRCH;
-	if (fmc->op->reprogram)
-		ret = fmc->op->reprogram(fmc, &t_drv, "");
-	if (ret == -ESRCH)
+	ret = fmc_reprogram(fmc, &t_drv, "", 0);
+	if (ret == -EPERM) /* programming not supported */
 		ret = 0;
 	if (ret < 0)
-		fmc->op->irq_free(fmc);
+		fmc_irq_free(fmc);
 
 	/* FIXME: reprogram LM32 too */
 	return ret;

@@ -66,7 +62,7 @@ static int t_probe(struct fmc_device *fmc)
 
 static int t_remove(struct fmc_device *fmc)
 {
-	fmc->op->irq_free(fmc);
+	fmc_irq_free(fmc);
 	return 0;
 }
drivers/fmc/fmc-write-eeprom.c +4 -4
index aa88f02..de1c113 100644
@@ -50,7 +50,7 @@ static int fwe_run_tlv(struct fmc_device *fmc, const struct firmware *fw,
 		if (write) {
 			dev_info(&fmc->dev, "write %i bytes at 0x%04x\n",
 				 thislen, thisaddr);
-			err = fmc->op->write_ee(fmc, thisaddr, p + 5, thislen);
+			err = fmc_write_ee(fmc, thisaddr, p + 5, thislen);
 		}
 		if (err < 0) {
 			dev_err(&fmc->dev, "write failure @0x%04x\n",

@@ -70,7 +70,7 @@ static int fwe_run_bin(struct fmc_device *fmc, const struct firmware *fw)
 	int ret;
 
 	dev_info(&fmc->dev, "programming %zi bytes\n", fw->size);
-	ret = fmc->op->write_ee(fmc, 0, (void *)fw->data, fw->size);
+	ret = fmc_write_ee(fmc, 0, (void *)fw->data, fw->size);
 	if (ret < 0) {
 		dev_info(&fmc->dev, "write_eeprom: error %i\n", ret);
 		return ret;

@@ -115,8 +115,8 @@ int fwe_probe(struct fmc_device *fmc)
 			KBUILD_MODNAME);
 		return -ENODEV;
 	}
-	if (fmc->op->validate)
-		index = fmc->op->validate(fmc, &fwe_drv);
+
+	index = fmc_validate(fmc, &fwe_drv);
 	if (index < 0) {
 		pr_err("%s: refusing device \"%s\"\n", KBUILD_MODNAME,
 		       dev_name(dev));
include/linux/fmc.h +18 -0
index a5f0aa5..8bb4a15 100644
@@ -234,4 +234,22 @@ extern void fmc_free_id_info(struct fmc_device *fmc);
 extern void fmc_dump_eeprom(const struct fmc_device *fmc);
 extern void fmc_dump_sdb(const struct fmc_device *fmc);
 
+/* helpers for FMC operations */
+extern int fmc_irq_request(struct fmc_device *fmc, irq_handler_t h,
+			   char *name, int flags);
+extern void fmc_irq_free(struct fmc_device *fmc);
+extern void fmc_irq_ack(struct fmc_device *fmc);
+extern int fmc_validate(struct fmc_device *fmc, struct fmc_driver *drv);
+extern int fmc_gpio_config(struct fmc_device *fmc, struct fmc_gpio *gpio,
+			   int ngpio);
+extern int fmc_read_ee(struct fmc_device *fmc, int pos, void *d, int l);
+extern int fmc_write_ee(struct fmc_device *fmc, int pos, const void *d, int l);
+
+/* helpers for FMC operations */
+extern int fmc_irq_request(struct fmc_device *fmc, irq_handler_t h,
+			   char *name, int flags);
+extern void fmc_irq_free(struct fmc_device *fmc);
+extern void fmc_irq_ack(struct fmc_device *fmc);
+extern int fmc_validate(struct fmc_device *fmc, struct fmc_driver *drv);
+
 #endif /* __LINUX_FMC_H__ */

--
1.8.3.1

author
#11 lore raw

PATCH v2 3/5 drivers/fmc: The only way to dump the SDB is from debugfs

In reply to Pat Riehecky

Driver should not call fmc_sdb_dump() anymore. (actually they can but the operation is not supported, so it will print an error message)

From: Federico Vaga <[email protected]>

Signed-off-by Federico Vaga <[email protected]>
Tested-by Pat Riehecky <[email protected]>
---
 drivers/fmc/Makefile                |   2 +
 drivers/fmc/fmc-core.c              |   7 +-
 drivers/fmc/fmc-debug.c             | 171 +++++++++++++++++++++++++++++++++++++++++
 drivers/fmc/fmc-dump.c              |  41 ----------
 drivers/fmc/fmc-private.h           |   9 +++
 drivers/fmc/fmc-sdb.c               |  99 +-----------------------
 include/linux/fmc-sdb.h |   1 +
 include/linux/fmc.h     |   4 +-
 8 files changed, 195 insertions(+), 139 deletions(-)
 create mode 100644 drivers/fmc/fmc-debug.c
 create mode 100644 drivers/fmc/fmc-private.h
drivers/fmc/Makefile +1 -0
index bb6efcd..6e2ad7b 100644
@@ -6,6 +6,7 @@ fmc-y += fmc-match.o
 fmc-y += fmc-sdb.o
 fmc-y += fru-parse.o
 fmc-y += fmc-dump.o
+fmc-y += fmc-debug.o
 
 obj-$(CONFIG_FMC_FAKEDEV) += fmc-fakedev.o
 obj-$(CONFIG_FMC_TRIVIAL) += fmc-trivial.o
drivers/fmc/fmc-core.c +6 -1
index 60e568d..50e18ba 100644
@@ -13,6 +13,9 @@
 #include <linux/init.h>
 #include <linux/device.h>
 #include <linux/fmc.h>
+#include <linux/fmc-sdb.h>
+
+#include "fmc-private.h"
 
 static int fmc_check_version(unsigned long version, const char *name)
 {

@@ -289,7 +292,7 @@ int fmc_device_register_n(struct fmc_device **devs, int n)
 		}
 		/* This device went well, give information to the user */
 		fmc_dump_eeprom(fmc);
-		fmc_dump_sdb(fmc);
+		fmc_debug_init(fmc);
 	}
 	return 0;
 

@@ -301,6 +304,7 @@ out:
 
 	kfree(devarray);
 	for (i--; i >= 0; i--) {
+		fmc_debug_exit(devs[i]);
 		sysfs_remove_bin_file(&devs[i]->dev.kobj, &fmc_eeprom_attr);
 		device_del(&devs[i]->dev);
 		fmc_free_id_info(devs[i]);

@@ -328,6 +332,7 @@ void fmc_device_unregister_n(struct fmc_device **devs, int n)
 	kfree(devs[0]->devarray);
 
 	for (i = 0; i < n; i++) {
+		fmc_debug_exit(devs[i]);
 		sysfs_remove_bin_file(&devs[i]->dev.kobj, &fmc_eeprom_attr);
 		device_del(&devs[i]->dev);
 		fmc_free_id_info(devs[i]);
drivers/fmc/fmc-debug.c new file +173 -0

173 lines not on this page · raw message

drivers/fmc/fmc-dump.c +0 -41

50 lines not on this page · raw message

drivers/fmc/fmc-private.h new file +9 -0
new file mode 100644
index 0000000..1e51366
@@ -0,0 +1,9 @@
+/*
+ * Copyright (C) 2015 CERN (www.cern.ch)
+ * Author: Federico Vaga <[email protected]>
+ *
+ * Released according to the GNU GPL, version 2 or any later version.
+ */
+
+extern int fmc_debug_init(struct fmc_device *fmc);
+extern void fmc_debug_exit(struct fmc_device *fmc);
drivers/fmc/fmc-sdb.c +3 -96

111 lines not on this page · raw message

include/linux/fmc.h +3 -1
index 8bb4a15..5c8df0c 100644
@@ -180,6 +180,9 @@ struct fmc_device {
 	uint32_t device_id;		/* Filled by the device */
 	char *mezzanine_name;		/* Defaults to ``fmc'' */
 	void *mezzanine_data;
+
+	struct dentry *dbg_dir;
+	struct dentry *dbg_sdb_dump;
 };
 #define to_fmc_device(x) container_of((x), struct fmc_device, dev)
 

@@ -232,7 +235,6 @@ extern int fmc_match(struct device *dev, struct device_driver *drv);
 extern int fmc_fill_id_info(struct fmc_device *fmc);
 extern void fmc_free_id_info(struct fmc_device *fmc);
 extern void fmc_dump_eeprom(const struct fmc_device *fmc);
-extern void fmc_dump_sdb(const struct fmc_device *fmc);
 
 /* helpers for FMC operations */
 extern int fmc_irq_request(struct fmc_device *fmc, irq_handler_t h,

--
1.8.3.1

author
#12 lore raw

PATCH v2 4/5 drivers/fmc: change registration prototype

In reply to Pat Riehecky

Permit use of either fmc_device_register_n or fmc_device_register_n_gw depending on the type of device in use.

From: Federico Vaga <[email protected]>

Signed-off-by Federico Vaga <[email protected]>
Tested-by Pat Riehecky <[email protected]>
---
 drivers/fmc/fmc-core.c          | 15 ++++++++++++++-
 include/linux/fmc.h | 13 +++++++++++--
 2 files changed, 25 insertions(+), 3 deletions(-)
drivers/fmc/fmc-core.c +14 -1
index 50e18ba..be411e3 100644
@@ -199,7 +199,8 @@ EXPORT_SYMBOL(fmc_driver_unregister);
  * When a device set is registered, all eeproms must be read
  * and all FRUs must be parsed
  */
-int fmc_device_register_n(struct fmc_device **devs, int n)
+int fmc_device_register_n_gw(struct fmc_device **devs, int n,
+			  struct fmc_gateware *gw)
 {
 	struct fmc_device *fmc, **devarray;
 	uint32_t device_id;

@@ -313,8 +314,20 @@ out:
 	return ret;
 
 }
+EXPORT_SYMBOL(fmc_device_register_n_gw);
+
+int fmc_device_register_n(struct fmc_device **devs, int n)
+{
+	return fmc_device_register_n_gw(devs, n, NULL);
+}
 EXPORT_SYMBOL(fmc_device_register_n);
 
+int fmc_device_register_gw(struct fmc_device *fmc, struct fmc_gateware *gw)
+{
+	return fmc_device_register_n_gw(&fmc, 1, gw);
+}
+EXPORT_SYMBOL(fmc_device_register_gw);
+
 int fmc_device_register(struct fmc_device *fmc)
 {
 	return fmc_device_register_n(&fmc, 1);
include/linux/fmc.h +11 -2
index 5c8df0c..b6c73d5 100644
@@ -220,14 +220,23 @@ static inline void fmc_set_drvdata(struct fmc_device *fmc, void *data)
 	dev_set_drvdata(&fmc->dev, data);
 }
 
-/* The 4 access points */
+struct fmc_gateware {
+	void *bitstream;
+	unsigned long len;
+};
+
+/* The 5 access points */
 extern int fmc_driver_register(struct fmc_driver *drv);
 extern void fmc_driver_unregister(struct fmc_driver *drv);
 extern int fmc_device_register(struct fmc_device *tdev);
+extern int fmc_device_register_gw(struct fmc_device *tdev,
+				  struct fmc_gateware *gw);
 extern void fmc_device_unregister(struct fmc_device *tdev);
 
-/* Two more for device sets, all driven by the same FPGA */
+/* Three more for device sets, all driven by the same FPGA */
 extern int fmc_device_register_n(struct fmc_device **devs, int n);
+extern int fmc_device_register_n_gw(struct fmc_device **devs, int n,
+				    struct fmc_gateware *gw);
 extern void fmc_device_unregister_n(struct fmc_device **devs, int n);
 
 /* Internal cross-calls between files; not exported to other modules */

--
1.8.3.1

author
#13 lore raw

PATCH v2 5/5 drivers/fmc: carrier can program FPGA on registration

In reply to Pat Riehecky

The initial FPGA may require programming before it is useful.

From: Federico Vaga <[email protected]>

Signed-off-by Federico Vaga <[email protected]>
Tested-by Pat Riehecky <[email protected]>
---
 drivers/fmc/fmc-core.c | 18 +++++++++++++++---
 drivers/fmc/fmc-sdb.c  | 24 ++++++++++++++++++++++++
 include/linux/fmc.h    |  4 ++++
 3 files changed, 43 insertions(+), 3 deletions(-)
drivers/fmc/fmc-core.c +15 -3
index eabeac0..cec3b8d 100644
@@ -280,6 +280,21 @@ int fmc_device_register_n_gw(struct fmc_device **devs, int n,
 		else
 			dev_set_name(&fmc->dev, "%s-%04x", fmc->mezzanine_name,
 				     device_id);
+
+		if (gw) {
+			/*
+			 * The carrier already know the bitstream to load
+			 * for this set of FMC mezzanines.
+			 */
+			ret = fmc->op->reprogram_raw(fmc, NULL,
+						     gw->bitstream, gw->len);
+			if (ret) {
+				dev_warn(fmc->hwdev,
+					 "Invalid gateware for FMC mezzanine\n");
+				goto out;
+			}
+		}
+
 		ret = device_add(&fmc->dev);
 		if (ret < 0) {
 			dev_err(fmc->hwdev, "Slot %i: Failed in registering "

@@ -300,9 +315,6 @@ int fmc_device_register_n_gw(struct fmc_device **devs, int n,
 out1:
 	device_del(&fmc->dev);
 out:
-	fmc_free_id_info(fmc);
-	put_device(&fmc->dev);
-
 	kfree(devarray);
 	for (i--; i >= 0; i--) {
 		fmc_debug_exit(devs[i]);
drivers/fmc/fmc-sdb.c +24 -0
index 89e37a6..ffdc176 100644
@@ -127,6 +127,30 @@ int fmc_free_sdb_tree(struct fmc_device *fmc)
 EXPORT_SYMBOL(fmc_free_sdb_tree);
 
 /* This helper calls reprogram and inizialized sdb as well */
+int fmc_reprogram_raw(struct fmc_device *fmc, struct fmc_driver *d,
+		      void *gw, unsigned long len, int sdb_entry)
+{
+	int ret;
+
+	ret = fmc->op->reprogram_raw(fmc, d, gw, len);
+	if (ret < 0)
+		return ret;
+	if (sdb_entry < 0)
+		return ret;
+
+	/* We are required to find SDB at a given offset */
+	ret = fmc_scan_sdb_tree(fmc, sdb_entry);
+	if (ret < 0) {
+		dev_err(&fmc->dev, "Can't find SDB at address 0x%x\n",
+			sdb_entry);
+		return -ENODEV;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(fmc_reprogram_raw);
+
+/* This helper calls reprogram and inizialized sdb as well */
 int fmc_reprogram(struct fmc_device *fmc, struct fmc_driver *d, char *gw,
 			 int sdb_entry)
 {
include/linux/fmc.h +4 -0
index b6c73d5..3dc8a1b 100644
@@ -132,6 +132,8 @@ struct fmc_operations {
 	uint32_t (*read32)(struct fmc_device *fmc, int offset);
 	void (*write32)(struct fmc_device *fmc, uint32_t value, int offset);
 	int (*validate)(struct fmc_device *fmc, struct fmc_driver *drv);
+	int (*reprogram_raw)(struct fmc_device *f, struct fmc_driver *d,
+			     void *gw, unsigned long len);
 	int (*reprogram)(struct fmc_device *f, struct fmc_driver *d, char *gw);
 	int (*irq_request)(struct fmc_device *fmc, irq_handler_t h,
 			   char *name, int flags);

@@ -144,6 +146,8 @@ struct fmc_operations {
 };
 
 /* Prefer this helper rather than calling of fmc->reprogram directly */
+int fmc_reprogram_raw(struct fmc_device *fmc, struct fmc_driver *d,
+		      void *gw, unsigned long len, int sdb_entry);
 extern int fmc_reprogram(struct fmc_device *f, struct fmc_driver *d, char *gw,
 		     int sdb_entry);

--
1.8.3.1

#15 lore raw

Re: [PATCH v2 05/05] drivers/fmc: carrier can program FPGA on registration

Quoted 2 lines

This is not how the "From:" line works, please read SubmittingPatches for the correct usage...

I'm submitting V3 with the correct From line, and my acked-by, since now I managed to actually test them. It will happen later today.

Thanks greg for considering the patches.

/alessandro