On 12/31/2013 01:22 PM, Markus Mayer wrote:
This change introduces debugfs support for the BCM281xx watchdog driver.

Signed-off-by: Markus Mayer <markus.ma...@linaro.org>
---
  drivers/watchdog/Kconfig        |   10 ++++
  drivers/watchdog/bcm_kona_wdt.c |  116 +++++++++++++++++++++++++++++++++++++--
  2 files changed, 122 insertions(+), 4 deletions(-)

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 10d188a..9b8b96d 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -1152,6 +1152,16 @@ config BCM_KONA_WDT
          Say 'Y' or 'M' here to enable the driver. The module will be called
          bcm_kona_wdt.

+config BCM_KONA_WDT_DEBUG
+       bool "DEBUGFS support for BCM Kona Watchdog"
+       depends on BCM_KONA_WDT
+       help
+         If enabled, adds /sys/kernel/debug/bcm-kona-wdt/info which provides
+         access to the driver's internal data structures as well as watchdog
+         timer hardware registres.
+
Unless Wim already applied this patch, I would suggest to merge the followup 
patch
with this one.

+         If in doubt, say 'N'.
+
  config LANTIQ_WDT
        tristate "Lantiq SoC watchdog"
        depends on LANTIQ
diff --git a/drivers/watchdog/bcm_kona_wdt.c b/drivers/watchdog/bcm_kona_wdt.c
index 7e41a83..d3aa473 100644
--- a/drivers/watchdog/bcm_kona_wdt.c
+++ b/drivers/watchdog/bcm_kona_wdt.c
@@ -11,6 +11,7 @@
   * GNU General Public License for more details.
   */

+#include <linux/debugfs.h>
  #include <linux/delay.h>
  #include <linux/err.h>
  #include <linux/io.h>
@@ -55,9 +56,13 @@ struct bcm_kona_wdt {
         */
        int resolution;
        spinlock_t lock;
+#ifdef CONFIG_BCM_KONA_WDT_DEBUG
+       unsigned long busy_count;
+       struct dentry *debugfs;
+#endif
  };

-static int secure_register_read(void __iomem *addr)
+static int secure_register_read(struct bcm_kona_wdt *wdt, uint32_t offset)
  {
        uint32_t val;
        unsigned count = 0;
@@ -70,10 +75,16 @@ static int secure_register_read(void __iomem *addr)
        do {
                if (unlikely(count > 1))
                        udelay(5);
-               val = readl_relaxed(addr);
+               val = readl_relaxed(wdt->base + offset);
                count++;
        } while ((val & SECWDOG_WD_LOAD_FLAG) && count < SECWDOG_MAX_TRY);

+#ifdef CONFIG_BCM_KONA_WDT_DEBUG
+       /* Remember the maximum number iterations due to WD_LOAD_FLAG */
+       if (count > wdt->busy_count)
+               wdt->busy_count = count;
+#endif
+
        /* This is the only place we return a negative value. */
        if (val & SECWDOG_WD_LOAD_FLAG)
                return -ETIMEDOUT;
@@ -84,6 +95,80 @@ static int secure_register_read(void __iomem *addr)
        return val;
  }

+#ifdef CONFIG_BCM_KONA_WDT_DEBUG
+
+static int bcm_kona_wdt_dbg_show(struct seq_file *s, void *data)
+{
+       int ctl_val, cur_val, ret;
+       unsigned long flags;
+       struct bcm_kona_wdt *wdt = s->private;
+
+       if (!wdt)
+               return seq_puts(s, "No device pointer\n");
+
+       spin_lock_irqsave(&wdt->lock, flags);
+       ctl_val = secure_register_read(wdt, SECWDOG_CTRL_REG);
+       cur_val = secure_register_read(wdt, SECWDOG_COUNT_REG);
+       spin_unlock_irqrestore(&wdt->lock, flags);
+
+       if (ctl_val < 0 || cur_val < 0) {
+               ret = seq_puts(s, "Error accessing hardware\n");
+       } else {
+               int ctl, cur, ctl_sec, cur_sec, res;
+
+               ctl = ctl_val & SECWDOG_COUNT_MASK;
+               res = (ctl_val & SECWDOG_RES_MASK) >> SECWDOG_CLKS_SHIFT;
+               cur = cur_val & SECWDOG_COUNT_MASK;
+               ctl_sec = TICKS_TO_SECS(ctl, wdt);
+               cur_sec = TICKS_TO_SECS(cur, wdt);
+               ret = seq_printf(s, "Resolution: %d / %d\n"
+                               "Control: %d s / %d (%#x) ticks\n"
+                               "Current: %d s / %d (%#x) ticks\n"
+                               "Busy count: %lu\n", res,
+                               wdt->resolution, ctl_sec, ctl, ctl, cur_sec,
+                               cur, cur, wdt->busy_count);
+       }
+
+       return ret;
+}
+
+static int bcm_kona_dbg_open(struct inode *inode, struct file *file)
+{
+       return single_open(file, bcm_kona_wdt_dbg_show, inode->i_private);
+}
+
+static const struct file_operations bcm_kona_dbg_operations = {
+       .open           = bcm_kona_dbg_open,
+       .read           = seq_read,
+       .llseek         = seq_lseek,
+       .release        = single_release,
+};
+
+static struct dentry *bcm_kona_wdt_debugfs_init(struct bcm_kona_wdt *wdt,
+       struct watchdog_device *wdd)
+{
+       struct dentry *dir;
+
+       dir = debugfs_create_dir(BCM_KONA_WDT_NAME, NULL);
+       if (IS_ERR_OR_NULL(dir))
+               return NULL;
+
+       if (debugfs_create_file("info", S_IFREG | S_IRUGO, dir, wdt,
+                               &bcm_kona_dbg_operations))
+               return dir;
+
+       /* Clean up */
+       debugfs_remove_recursive(dir);
+       return NULL;
+}
+
+static void bcm_kona_debugfs_exit(struct dentry *dir)
+{
+       debugfs_remove_recursive(dir);
+}
+
+#endif /* CONFIG_BCM_KONA_WDT_DEBUG */
+
  static int bcm_kona_wdt_ctrl_reg_modify(struct bcm_kona_wdt *wdt,
                                        unsigned mask, unsigned newval)
  {
@@ -93,7 +178,7 @@ static int bcm_kona_wdt_ctrl_reg_modify(struct bcm_kona_wdt 
*wdt,

        spin_lock_irqsave(&wdt->lock, flags);

-       val = secure_register_read(wdt->base + SECWDOG_CTRL_REG);
+       val = secure_register_read(wdt, SECWDOG_CTRL_REG);
        if (val < 0) {
                ret = val;
        } else {
@@ -140,7 +225,7 @@ static unsigned int bcm_kona_wdt_get_timeleft(struct 
watchdog_device *wdog)
        unsigned long flags;

        spin_lock_irqsave(&wdt->lock, flags);
-       val = secure_register_read(wdt->base + SECWDOG_COUNT_REG);
+       val = secure_register_read(wdt, SECWDOG_COUNT_REG);
        spin_unlock_irqrestore(&wdt->lock, flags);

        if (val < 0)
@@ -190,6 +275,27 @@ static void bcm_kona_wdt_shutdown(struct platform_device 
*pdev)
        bcm_kona_wdt_stop(&bcm_kona_wdt_wdd);
  }

+#ifdef CONFIG_BCM_KONA_WDT_DEBUG
+static void bcm_kona_wdt_debug_init(struct platform_device *pdev)
+{
+       struct bcm_kona_wdt *wdt = platform_get_drvdata(pdev);
+
+       wdt->debugfs = bcm_kona_wdt_debugfs_init(wdt, &bcm_kona_wdt_wdd);
+}
+
+static void bcm_kona_wdt_debug_exit(struct platform_device *pdev)
+{
+       struct bcm_kona_wdt *wdt = platform_get_drvdata(pdev);
+
+       if (wdt->debugfs)
+               bcm_kona_debugfs_exit(wdt->debugfs);
+}
+
+#else
+static void bcm_kona_wdt_debug_init(struct platform_device *pdev) {}
+static void bcm_kona_wdt_debug_exit(struct platform_device *pdev) {}
+#endif
+

Can you fold those functions into bcm_kona_wdt_debugfs_init() and 
bcm_kona_debugfs_exit() ?
Then you only need a sinfle #ifdef / #else combination. Only functional change 
would be to
set wdt->debugfs in bcm_kona_wdt_debugfs_init() and to check it in 
bcm_kona_debugfs_exit().

Thanks,
Guenter

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to