Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751446AbdDAJNo (ORCPT ); Sat, 1 Apr 2017 05:13:44 -0400 Received: from mail-yw0-f193.google.com ([209.85.161.193]:34790 "EHLO mail-yw0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751095AbdDAJNm (ORCPT ); Sat, 1 Apr 2017 05:13:42 -0400 MIME-Version: 1.0 From: noman pouigt Date: Sat, 1 Apr 2017 02:13:41 -0700 Message-ID: Subject: bug fix for registers debugfs file implementation [RFC] To: "broonie@kernel.org" , linux-kernel@vger.kernel.org Cc: gregkh@linuxfoundation.org Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 8474 Lines: 275 Current registers debugfs file implementation doesn't handle if the size exceeds 4k. It just dumps 4k of registers. Fix this by using the seq_file which already handles the file offset logic instead of reinventing the wheel. I am wondering if there is any issue is doing below which I am not aware of? Signed-off-by: variksla --- drivers/base/regmap/regmap-debugfs.c | 217 ++++++++++------------------------- 1 file changed, 61 insertions(+), 156 deletions(-) diff --git a/drivers/base/regmap/regmap-debugfs.c b/drivers/base/regmap/regmap-debugfs.c index 36ce351..0bec69c 100644 --- a/drivers/base/regmap/regmap-debugfs.c +++ b/drivers/base/regmap/regmap-debugfs.c @@ -88,98 +88,7 @@ static bool regmap_printable(struct regmap *map, unsigned int reg) return true; } -/* - * Work out where the start offset maps into register numbers, bearing - * in mind that we suppress hidden registers. - */ -static unsigned int regmap_debugfs_get_dump_start(struct regmap *map, - unsigned int base, - loff_t from, - loff_t *pos) -{ - struct regmap_debugfs_off_cache *c = NULL; - loff_t p = 0; - unsigned int i, ret; - unsigned int fpos_offset; - unsigned int reg_offset; - - /* Suppress the cache if we're using a subrange */ - if (base) - return base; - - /* - * If we don't have a cache build one so we don't have to do a - * linear scan each time. - */ - mutex_lock(&map->cache_lock); - i = base; - if (list_empty(&map->debugfs_off_cache)) { - for (; i <= map->max_register; i += map->reg_stride) { - /* Skip unprinted registers, closing off cache entry */ - if (!regmap_printable(map, i)) { - if (c) { - c->max = p - 1; - c->max_reg = i - map->reg_stride; - list_add_tail(&c->list, - &map->debugfs_off_cache); - c = NULL; - } - - continue; - } - - /* No cache entry? Start a new one */ - if (!c) { - c = kzalloc(sizeof(*c), GFP_KERNEL); - if (!c) { - regmap_debugfs_free_dump_cache(map); - mutex_unlock(&map->cache_lock); - return base; - } - c->min = p; - c->base_reg = i; - } - - p += map->debugfs_tot_len; - } - } - - /* Close the last entry off if we didn't scan beyond it */ - if (c) { - c->max = p - 1; - c->max_reg = i - map->reg_stride; - list_add_tail(&c->list, - &map->debugfs_off_cache); - } - - /* - * This should never happen; we return above if we fail to - * allocate and we should never be in this code if there are - * no registers at all. - */ - WARN_ON(list_empty(&map->debugfs_off_cache)); - ret = base; - - /* Find the relevant block:offset */ - list_for_each_entry(c, &map->debugfs_off_cache, list) { - if (from >= c->min && from <= c->max) { - fpos_offset = from - c->min; - reg_offset = fpos_offset / map->debugfs_tot_len; - *pos = c->min + (reg_offset * map->debugfs_tot_len); - mutex_unlock(&map->cache_lock); - return c->base_reg + (reg_offset * map->reg_stride); - } - - *pos = c->max; - ret = c->max_reg; - } - mutex_unlock(&map->cache_lock); - - return ret; -} - -static inline void regmap_calc_tot_len(struct regmap *map, - void *buf, size_t count) +static inline void regmap_calc_tot_len(struct regmap *map) { /* Calculate the length of a fixed format */ if (!map->debugfs_tot_len) { @@ -190,84 +99,80 @@ static inline void regmap_calc_tot_len(struct regmap *map, } } -static ssize_t regmap_read_debugfs(struct regmap *map, unsigned int from, - unsigned int to, char __user *user_buf, - size_t count, loff_t *ppos) +static void *regmap_debugfs_start(struct seq_file *s, loff_t *pos) { - size_t buf_pos = 0; - loff_t p = *ppos; - ssize_t ret; - int i; - char *buf; - unsigned int val, start_reg; - - if (*ppos < 0 || !count) - return -EINVAL; + struct regmap *map = s->private; + regmap_calc_tot_len(map); + return pos; +} - buf = kmalloc(count, GFP_KERNEL); - if (!buf) - return -ENOMEM; +static void *regmap_debugfs_next(struct seq_file *s, void *v, loff_t *pos) +{ + struct regmap *map = s->private; - regmap_calc_tot_len(map, buf, count); + (*pos)++; + if (*pos >= map->max_register) { + return NULL; + } + return pos; +} - /* Work out which register we're starting at */ - start_reg = regmap_debugfs_get_dump_start(map, from, *ppos, &p); +static void regmap_debugfs_stop(struct seq_file *s, void *v) +{ +} - for (i = start_reg; i <= to; i += map->reg_stride) { - if (!regmap_readable(map, i) && !regmap_cached(map, i)) - continue; +static int regmap_debugfs_show(struct seq_file *s, void *v) +{ + struct regmap *map = s->private; + unsigned int val, current_reg = *(loff_t *)v; + int ret; - if (regmap_precious(map, i)) - continue; + if (!regmap_printable(map, current_reg)) + return 0; - /* If we're in the region the user is trying to read */ - if (p >= *ppos) { - /* ...but not beyond it */ - if (buf_pos + map->debugfs_tot_len > count) - break; + /* Format the register */ + seq_printf(s, "%.*x: ", map->debugfs_reg_len, current_reg); - /* Format the register */ - snprintf(buf + buf_pos, count - buf_pos, "%.*x: ", - map->debugfs_reg_len, i - from); - buf_pos += map->debugfs_reg_len + 2; - - /* Format the value, write all X if we can't read */ - ret = regmap_read(map, i, &val); - if (ret == 0) - snprintf(buf + buf_pos, count - buf_pos, - "%.*x", map->debugfs_val_len, val); - else - memset(buf + buf_pos, 'X', - map->debugfs_val_len); - buf_pos += 2 * map->format.val_bytes; - - buf[buf_pos++] = '\n'; - } - p += map->debugfs_tot_len; + /* Format the value, write all X if we can't read */ + ret = regmap_read(map, current_reg, &val); + if (ret == 0) { + seq_printf(s, "%.*x\n", map->debugfs_val_len, val); + } else { + char buf[20]; + memset(buf, 'X', map->debugfs_val_len); + seq_printf(s, "%.*s\n", map->debugfs_val_len, buf); } + return 0; +} - ret = buf_pos; +static struct seq_operations regmap_debugfs_seq_ops = { + .start = regmap_debugfs_start, + .next = regmap_debugfs_next, + .stop = regmap_debugfs_stop, + .show = regmap_debugfs_show +}; - if (copy_to_user(user_buf, buf, buf_pos)) { - ret = -EFAULT; - goto out; +static int debugfs_open(struct inode *inode, struct file *file) +{ + int ret; + ret = seq_open(file, ®map_debugfs_seq_ops); + if (!ret) { + struct seq_file *seq; + /* seq_open will have modified file->private_data */ + seq = file->private_data; + seq->private = inode->i_private; } - *ppos += buf_pos; - -out: - kfree(buf); return ret; -} - -static ssize_t regmap_map_read_file(struct file *file, char __user *user_buf, - size_t count, loff_t *ppos) -{ - struct regmap *map = file->private_data; +}; - return regmap_read_debugfs(map, 0, map->max_register, user_buf, - count, ppos); -} +static struct file_operations regmap_debugfs_fops = { + .owner = THIS_MODULE, + .open = debugfs_open, + .read = seq_read, + .llseek = seq_lseek, + .release = seq_release +}; #undef REGMAP_ALLOW_WRITE_DEBUGFS #ifdef REGMAP_ALLOW_WRITE_DEBUGFS @@ -579,7 +484,7 @@ void regmap_debugfs_init(struct regmap *map, const char *name) #endif debugfs_create_file("registers", registers_mode, map->debugfs, - map, ®map_map_fops); + map, ®map_debugfs_fops); debugfs_create_file("access", 0400, map->debugfs, map, ®map_access_fops); } -- 2.7.4