| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <52C8480C.9040409@roeck-us.net>
Date: Sat, 04 Jan 2014 09:42:36 -0800
From: Guenter Roeck <linux@...ck-us.net>
To: Markus Mayer <markus.mayer@...aro.org>,
Wim Van Sebroeck <wim@...ana.be>,
Christian Daudt <bcm@...thebug.org>
CC: Linaro Patches <patches@...aro.org>,
Linux Watchdog List <linux-watchdog@...r.kernel.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/2] watchdog: bcm281xx: Debugfs support
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.mayer@...aro.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 majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists