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

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

[Alessandro Rubini]

From: Federico Vaga <federico.vaga@cern.ch>

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 <federico.vaga@cern.ch>
Tested-by: Pat Riehecky <riehecky@fnal.gov>
Acked-by: Alessandro Rubini <rubini@gnudd.com>
---

V3 (Alessandro): fixed From line in patch, added alessandro's acked-by

V2 (Pat): added Tested-by and incorrect From line

V1 (Pat): picked from ohwr.org repo, where most fmc users pick from.

 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

diff --git a/drivers/fmc/Makefile b/drivers/fmc/Makefile
index b945291..e809322 100644
--- a/drivers/fmc/Makefile
+++ b/drivers/fmc/Makefile
@@ -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
diff --git a/drivers/fmc/fmc-core.c b/drivers/fmc/fmc-core.c
index 5263d06..ef6d8ac 100644
--- a/drivers/fmc/fmc-core.c
+++ b/drivers/fmc/fmc-core.c
@@ -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]);
diff --git a/drivers/fmc/fmc-debug.c b/drivers/fmc/fmc-debug.c
new file mode 100644
index 0000000..3293072
--- /dev/null
+++ b/drivers/fmc/fmc-debug.c
@@ -0,0 +1,173 @@
+/*
+ * Copyright (C) 2015 CERN (www.cern.ch)
+ * Author: Federico Vaga <federico.vaga@cern.ch>
+ *
+ * Released according to the GNU GPL, version 2 or any later version.
+ */
+
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/init.h>
+#include <linux/fs.h>
+#include <linux/debugfs.h>
+#include <linux/seq_file.h>
+#include <asm/byteorder.h>
+
+#include <linux/fmc.h>
+#include <linux/sdb.h>
+#include <linux/fmc-sdb.h>
+
+#define FMC_DBG_SDB_DUMP "dump_sdb"
+
+static char *__strip_trailing_space(char *buf, char *str, int len)
+{
+	int i = len - 1;
+
+	memcpy(buf, str, len);
+	buf[len] = '\0';
+	while (i >= 0 && buf[i] == ' ')
+		buf[i--] = '\0';
+	return buf;
+}
+
+#define __sdb_string(buf, field) ({			\
+	BUILD_BUG_ON(sizeof(buf) < sizeof(field));	\
+	__strip_trailing_space(buf, (void *)(field), sizeof(field));	\
+		})
+
+/**
+ * We do not check seq_printf() errors because we want to see things in any case
+ */
+static void fmc_sdb_dump_recursive(struct fmc_device *fmc, struct seq_file *s,
+				   const struct sdb_array *arr)
+{
+	unsigned long base = arr->baseaddr;
+	int i, j, n = arr->len, level = arr->level;
+	char tmp[64];
+
+	for (i = 0; i < n; i++) {
+		union  sdb_record *r;
+		struct sdb_product *p;
+		struct sdb_component *c;
+
+		r = &arr->record[i];
+		c = &r->dev.sdb_component;
+		p = &c->product;
+
+		for (j = 0; j < level; j++)
+			seq_printf(s, "   ");
+		switch (r->empty.record_type) {
+		case sdb_type_interconnect:
+			seq_printf(s, "%08llx:%08x %.19s\n",
+				   __be64_to_cpu(p->vendor_id),
+				   __be32_to_cpu(p->device_id),
+				   p->name);
+			break;
+		case sdb_type_device:
+			seq_printf(s, "%08llx:%08x %.19s (%08llx-%08llx)\n",
+				   __be64_to_cpu(p->vendor_id),
+				   __be32_to_cpu(p->device_id),
+				   p->name,
+				   __be64_to_cpu(c->addr_first) + base,
+				   __be64_to_cpu(c->addr_last) + base);
+			break;
+		case sdb_type_bridge:
+			seq_printf(s, "%08llx:%08x %.19s (bridge: %08llx)\n",
+				   __be64_to_cpu(p->vendor_id),
+				   __be32_to_cpu(p->device_id),
+				   p->name,
+				   __be64_to_cpu(c->addr_first) + base);
+			if (IS_ERR(arr->subtree[i])) {
+				seq_printf(s, "SDB: (bridge error %li)\n",
+					 PTR_ERR(arr->subtree[i]));
+				break;
+			}
+			fmc_sdb_dump_recursive(fmc, s, arr->subtree[i]);
+			break;
+		case sdb_type_integration:
+			seq_printf(s, "integration\n");
+			break;
+		case sdb_type_repo_url:
+			seq_printf(s, "Synthesis repository: %s\n",
+					  __sdb_string(tmp, r->repo_url.repo_url));
+			break;
+		case sdb_type_synthesis:
+			seq_printf(s, "Bitstream '%s' ",
+					  __sdb_string(tmp, r->synthesis.syn_name));
+			seq_printf(s, "synthesized %08x by %s ",
+					  __be32_to_cpu(r->synthesis.date),
+					  __sdb_string(tmp, r->synthesis.user_name));
+			seq_printf(s, "(%s version %x), ",
+					  __sdb_string(tmp, r->synthesis.tool_name),
+					  __be32_to_cpu(r->synthesis.tool_version));
+			seq_printf(s, "commit %pm\n",
+					  r->synthesis.commit_id);
+			break;
+		case sdb_type_empty:
+			seq_printf(s, "empty\n");
+			break;
+		default:
+			seq_printf(s, "UNKNOWN TYPE 0x%02x\n",
+				   r->empty.record_type);
+			break;
+		}
+	}
+}
+
+static int fmc_sdb_dump(struct seq_file *s, void *offset)
+{
+	struct fmc_device *fmc = s->private;
+
+	if (!fmc->sdb) {
+		seq_printf(s, "no SDB information\n");
+		return 0;
+	}
+
+	seq_printf(s, "FMC: %s (%s), slot %i, device %s\n", dev_name(fmc->hwdev),
+	fmc->carrier_name, fmc->slot_id, dev_name(&fmc->dev));
+	/* Dump SDB information */
+	fmc_sdb_dump_recursive(fmc, s, fmc->sdb);
+
+	return 0;
+}
+
+
+static int fmc_sdb_dump_open(struct inode *inode, struct file *file)
+{
+	struct fmc_device *fmc = inode->i_private;
+
+	return single_open(file, fmc_sdb_dump, fmc);
+}
+
+
+const struct file_operations fmc_dbgfs_sdb_dump = {
+	.owner = THIS_MODULE,
+	.open  = fmc_sdb_dump_open,
+	.read = seq_read,
+	.llseek = seq_lseek,
+	.release = single_release,
+};
+
+int fmc_debug_init(struct fmc_device *fmc)
+{
+	fmc->dbg_dir = debugfs_create_dir(dev_name(&fmc->dev), NULL);
+	if (IS_ERR_OR_NULL(fmc->dbg_dir)) {
+		pr_err("FMC: Cannot create debugfs\n");
+		return PTR_ERR(fmc->dbg_dir);
+	}
+
+	fmc->dbg_sdb_dump = debugfs_create_file(FMC_DBG_SDB_DUMP, 0444,
+						fmc->dbg_dir, fmc,
+						&fmc_dbgfs_sdb_dump);
+	if (IS_ERR_OR_NULL(fmc->dbg_sdb_dump))
+		pr_err("FMC: Cannot create debugfs file %s\n",
+		       FMC_DBG_SDB_DUMP);
+
+	return 0;
+}
+
+void fmc_debug_exit(struct fmc_device *fmc)
+{
+	if (fmc->dbg_dir)
+		debugfs_remove_recursive(fmc->dbg_dir);
+}
diff --git a/drivers/fmc/fmc-dump.c b/drivers/fmc/fmc-dump.c
index c91afd6..cd1df47 100644
--- a/drivers/fmc/fmc-dump.c
+++ b/drivers/fmc/fmc-dump.c
@@ -15,8 +15,6 @@
 
 static int fmc_must_dump_eeprom;
 module_param_named(dump_eeprom, fmc_must_dump_eeprom, int, 0644);
-static int fmc_must_dump_sdb;
-module_param_named(dump_sdb, fmc_must_dump_sdb, int, 0644);
 
 #define LINELEN 16
 
@@ -59,42 +57,3 @@ void fmc_dump_eeprom(const struct fmc_device *fmc)
 	for (i = 0; i < fmc->eeprom_len; i += LINELEN, line += LINELEN)
 		prev = dump_line(i, line, prev);
 }
-
-void fmc_dump_sdb(const struct fmc_device *fmc)
-{
-	const uint8_t *line, *prev;
-	int i, len;
-
-	if (!fmc->sdb)
-		return;
-	if (!fmc_must_dump_sdb)
-		return;
-
-	/* If the argument is not-zero, do simple dump (== show) */
-	if (fmc_must_dump_sdb > 0)
-		fmc_show_sdb_tree(fmc);
-
-	if (fmc_must_dump_sdb == 1)
-		return;
-
-	/* If bigger than 1, dump it seriously, to help debugging */
-
-	/*
-	 * Here we should really use libsdbfs (which is designed to
-	 * work in kernel space as well) , but it doesn't support
-	 * directories yet, and it requires better intergration (it
-	 * should be used instead of fmc-specific code).
-	 *
-	 * So, lazily, just dump the top-level array
-	 */
-	pr_info("FMC: %s (%s), slot %i, device %s\n", dev_name(fmc->hwdev),
-		fmc->carrier_name, fmc->slot_id, dev_name(&fmc->dev));
-	pr_info("FMC: poor dump of sdb first level:\n");
-
-	len = fmc->sdb->len * sizeof(union sdb_record);
-	line = (void *)fmc->sdb->record;
-	prev = NULL;
-	for (i = 0; i < len; i += LINELEN, line += LINELEN)
-		prev = dump_line(i, line, prev);
-	return;
-}
diff --git a/drivers/fmc/fmc-private.h b/drivers/fmc/fmc-private.h
new file mode 100644
index 0000000..1e51366
--- /dev/null
+++ b/drivers/fmc/fmc-private.h
@@ -0,0 +1,9 @@
+/*
+ * Copyright (C) 2015 CERN (www.cern.ch)
+ * Author: Federico Vaga <federico.vaga@cern.ch>
+ *
+ * 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);
diff --git a/drivers/fmc/fmc-sdb.c b/drivers/fmc/fmc-sdb.c
index 4603fdb..89e37a6 100644
--- a/drivers/fmc/fmc-sdb.c
+++ b/drivers/fmc/fmc-sdb.c
@@ -145,108 +145,15 @@ int fmc_reprogram(struct fmc_device *fmc, struct fmc_driver *d, char *gw,
 			sdb_entry);
 		return -ENODEV;
 	}
-	fmc_dump_sdb(fmc);
+
 	return 0;
 }
 EXPORT_SYMBOL(fmc_reprogram);
 
-static char *__strip_trailing_space(char *buf, char *str, int len)
-{
-	int i = len - 1;
-
-	memcpy(buf, str, len);
-	while(i >= 0 && buf[i] == ' ')
-		buf[i--] = '\0';
-	return buf;
-}
-
-#define __sdb_string(buf, field) ({			\
-	BUILD_BUG_ON(sizeof(buf) < sizeof(field));	\
-	__strip_trailing_space(buf, (void *)(field), sizeof(field));	\
-		})
-
-static void __fmc_show_sdb_tree(const struct fmc_device *fmc,
-				const struct sdb_array *arr)
-{
-	unsigned long base = arr->baseaddr;
-	int i, j, n = arr->len, level = arr->level;
-	char buf[64];
-
-	for (i = 0; i < n; i++) {
-		union  sdb_record *r;
-		struct sdb_product *p;
-		struct sdb_component *c;
-		r = &arr->record[i];
-		c = &r->dev.sdb_component;
-		p = &c->product;
-
-		dev_info(&fmc->dev, "SDB: ");
-
-		for (j = 0; j < level; j++)
-			printk(KERN_CONT "   ");
-		switch (r->empty.record_type) {
-		case sdb_type_interconnect:
-			printk(KERN_CONT "%08llx:%08x %.19s\n",
-			       __be64_to_cpu(p->vendor_id),
-			       __be32_to_cpu(p->device_id),
-			       p->name);
-			break;
-		case sdb_type_device:
-			printk(KERN_CONT "%08llx:%08x %.19s (%08llx-%08llx)\n",
-			       __be64_to_cpu(p->vendor_id),
-			       __be32_to_cpu(p->device_id),
-			       p->name,
-			       __be64_to_cpu(c->addr_first) + base,
-			       __be64_to_cpu(c->addr_last) + base);
-			break;
-		case sdb_type_bridge:
-			printk(KERN_CONT "%08llx:%08x %.19s (bridge: %08llx)\n",
-			       __be64_to_cpu(p->vendor_id),
-			       __be32_to_cpu(p->device_id),
-			       p->name,
-			       __be64_to_cpu(c->addr_first) + base);
-			if (IS_ERR(arr->subtree[i])) {
-				dev_info(&fmc->dev, "SDB: (bridge error %li)\n",
-					 PTR_ERR(arr->subtree[i]));
-				break;
-			}
-			__fmc_show_sdb_tree(fmc, arr->subtree[i]);
-			break;
-		case sdb_type_integration:
-			printk(KERN_CONT "integration\n");
-			break;
-		case sdb_type_repo_url:
-			printk(KERN_CONT "Synthesis repository: %s\n",
-			       __sdb_string(buf, r->repo_url.repo_url));
-			break;
-		case sdb_type_synthesis:
-			printk(KERN_CONT "Bitstream '%s' ",
-			       __sdb_string(buf, r->synthesis.syn_name));
-			printk(KERN_CONT "synthesized %08x by %s ",
-			       __be32_to_cpu(r->synthesis.date),
-			       __sdb_string(buf, r->synthesis.user_name));
-			printk(KERN_CONT "(%s version %x), ",
-			       __sdb_string(buf, r->synthesis.tool_name),
-			       __be32_to_cpu(r->synthesis.tool_version));
-			printk(KERN_CONT "commit %pm\n",
-			       r->synthesis.commit_id);
-			break;
-		case sdb_type_empty:
-			printk(KERN_CONT "empty\n");
-			break;
-		default:
-			printk(KERN_CONT "UNKNOWN TYPE 0x%02x\n",
-			       r->empty.record_type);
-			break;
-		}
-	}
-}
-
 void fmc_show_sdb_tree(const struct fmc_device *fmc)
 {
-	if (!fmc->sdb)
-		return;
-	__fmc_show_sdb_tree(fmc, fmc->sdb);
+	pr_err("%s: not supported anymore, use debugfs to dump SDB\n",
+		__func__);
 }
 EXPORT_SYMBOL(fmc_show_sdb_tree);
 
diff --git a/include/linux/fmc.h b/include/linux/fmc.h
index 8bb4a154..5c8df0c 100644
--- a/include/linux/fmc.h
+++ b/include/linux/fmc.h
@@ -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,
-- 
2.1.4