RE: [PATCH 4/6] fjes: Implement debug mode for fjes driver

[Izumi, Taku]

Dear David,

> -Original Message-
> From: David Miller [mailto:da...@davemloft.net]
> Sent: Thursday, October 13, 2016 11:20 PM
> To: Izumi, Taku/泉 拓
> Cc: netdev@vger.kernel.org
> Subject: Re: [PATCH 4/6] fjes: Implement debug mode for fjes driver
> 
> From: Taku Izumi <izumi.t...@jp.fujitsu.com>
> Date: Tue, 11 Oct 2016 17:55:20 +0900
> 
> > This patch implements debug mode for fjes driver.
> > You can get firmware activity information by enabling
> > debug mode. This is useful for debugging.
> >
> > To enable debug mode, write value of debugging mode to
> > debug_mode file in debugfs:
> >
> >   # echo 1 > /sys/kernel/debug/fjes/fjes.0/debug_mode
> >
> > To disable debug mode, write 0 to debug_mode file in debugfs:
> >
> >   # echo 0 > /sys/kernel/debug/fjes/fjes.0/debug_mode
> >
> > Firmware activity information can be retrieved via
> > /sys/kernel/debug/fjes/fjes.0/debug_data file.
> >
> > Signed-off-by: Taku Izumi <izumi.t...@jp.fujitsu.com>
> 
> There is no reason to use debugfs for this, we have facilities such
> as ETHTOOL_SET_DUMP et al. that you can use to implement this.

  Thank you for reviewing.
  I see. I'll rewrite this patch to use ethtool.

Sincerely,
Taku Izumi

Re: [PATCH 4/6] fjes: Implement debug mode for fjes driver

[David Miller]

From: Taku Izumi 
Date: Tue, 11 Oct 2016 17:55:20 +0900

> This patch implements debug mode for fjes driver.
> You can get firmware activity information by enabling
> debug mode. This is useful for debugging.
> 
> To enable debug mode, write value of debugging mode to
> debug_mode file in debugfs:
> 
>   # echo 1 > /sys/kernel/debug/fjes/fjes.0/debug_mode
> 
> To disable debug mode, write 0 to debug_mode file in debugfs:
> 
>   # echo 0 > /sys/kernel/debug/fjes/fjes.0/debug_mode
> 
> Firmware activity information can be retrieved via
> /sys/kernel/debug/fjes/fjes.0/debug_data file.
> 
> Signed-off-by: Taku Izumi 

There is no reason to use debugfs for this, we have facilities such
as ETHTOOL_SET_DUMP et al. that you can use to implement this.

[PATCH 4/6] fjes: Implement debug mode for fjes driver

[Taku Izumi]

This patch implements debug mode for fjes driver.
You can get firmware activity information by enabling
debug mode. This is useful for debugging.

To enable debug mode, write value of debugging mode to
debug_mode file in debugfs:

  # echo 1 > /sys/kernel/debug/fjes/fjes.0/debug_mode

To disable debug mode, write 0 to debug_mode file in debugfs:

  # echo 0 > /sys/kernel/debug/fjes/fjes.0/debug_mode

Firmware activity information can be retrieved via
/sys/kernel/debug/fjes/fjes.0/debug_data file.

Signed-off-by: Taku Izumi 
---
 drivers/net/fjes/Makefile   |   2 +-
 drivers/net/fjes/fjes.h |  18 +
 drivers/net/fjes/fjes_debugfs.c | 169 
 drivers/net/fjes/fjes_hw.c  | 122 +
 drivers/net/fjes/fjes_hw.h  |  15 
 drivers/net/fjes/fjes_main.c|  12 ++-
 drivers/net/fjes/fjes_trace.h   |  69 
 7 files changed, 405 insertions(+), 2 deletions(-)
 create mode 100644 drivers/net/fjes/fjes_debugfs.c

diff --git a/drivers/net/fjes/Makefile b/drivers/net/fjes/Makefile
index 6705d1b..bc47b35 100644
--- a/drivers/net/fjes/Makefile
+++ b/drivers/net/fjes/Makefile
@@ -27,4 +27,4 @@
 
 obj-$(CONFIG_FUJITSU_ES) += fjes.o
 
-fjes-objs := fjes_main.o fjes_hw.o fjes_ethtool.o fjes_trace.o
+fjes-objs := fjes_main.o fjes_hw.o fjes_ethtool.o fjes_trace.o fjes_debugfs.o
diff --git a/drivers/net/fjes/fjes.h b/drivers/net/fjes/fjes.h
index a592fe2..3f4dc73 100644
--- a/drivers/net/fjes/fjes.h
+++ b/drivers/net/fjes/fjes.h
@@ -23,6 +23,7 @@
 #define FJES_H_
 
 #include 
+#include 
 
 #include "fjes_hw.h"
 
@@ -66,6 +67,11 @@ struct fjes_adapter {
bool interrupt_watch_enable;
 
struct fjes_hw hw;
+
+#ifdef CONFIG_DEBUG_FS
+   struct dentry *dbg_adapter;
+   struct debugfs_blob_wrapper blob;
+#endif /* CONFIG_DEBUG_FS */
 };
 
 extern char fjes_driver_name[];
@@ -74,4 +80,16 @@ extern const u32 fjes_support_mtu[];
 
 void fjes_set_ethtool_ops(struct net_device *);
 
+#ifdef CONFIG_DEBUG_FS
+void fjes_dbg_adapter_init(struct fjes_adapter *adapter);
+void fjes_dbg_adapter_exit(struct fjes_adapter *adapter);
+void fjes_dbg_init(void);
+void fjes_dbg_exit(void);
+#else
+static inline void fjes_dbg_adapter_init(struct fjes_adapter *adapter) {}
+static inline void fjes_dbg_adapter_exit(struct fjes_adapter *adapter) {}
+static inline void fjes_dbg_init(void) {}
+static inline void fjes_dbg_exit(void) {}
+#endif /* CONFIG_DEBUG_FS */
+
 #endif /* FJES_H_ */
diff --git a/drivers/net/fjes/fjes_debugfs.c b/drivers/net/fjes/fjes_debugfs.c
new file mode 100644
index 000..d868fe7
--- /dev/null
+++ b/drivers/net/fjes/fjes_debugfs.c
@@ -0,0 +1,169 @@
+/*
+ *  FUJITSU Extended Socket Network Device driver
+ *  Copyright (c) 2015-2016 FUJITSU LIMITED
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; if not, see .
+ *
+ * The full GNU General Public License is included in this distribution in
+ * the file called "COPYING".
+ *
+ */
+
+/* debugfs support for fjes driver */
+
+#ifdef CONFIG_DEBUG_FS
+
+#include 
+#include 
+#include 
+
+#include "fjes.h"
+
+static struct dentry *fjes_debug_root;
+
+static ssize_t fjes_dbg_dbg_mode_read(struct file *file, char __user *ubuf,
+ size_t count, loff_t *ppos)
+{
+   struct fjes_adapter *adapter = file->private_data;
+   struct fjes_hw *hw = >hw;
+   char buf[64];
+   int size;
+
+   size = sprintf(buf, "%d\n", hw->debug_mode);
+
+   return simple_read_from_buffer(ubuf, count, ppos, buf, size);
+}
+
+static ssize_t fjes_dbg_dbg_mode_write(struct file *file,
+  const char __user *ubuf, size_t count,
+  loff_t *ppos)
+{
+   struct fjes_adapter *adapter = file->private_data;
+   struct fjes_hw *hw = >hw;
+   unsigned int value;
+   int ret;
+
+   ret = kstrtouint_from_user(ubuf, count, 10, );
+   if (ret)
+   return ret;
+
+   if (value) {
+   if (hw->debug_mode)
+   return -EPERM;
+
+   hw->debug_mode = value;
+
+   /* enable debug mode */
+   mutex_lock(>hw_info.lock);
+   ret = fjes_hw_start_debug(hw);
+   mutex_unlock(>hw_info.lock);
+
+   if (ret) {
+   hw->debug_mode = 0;
+   return ret;
+