diff mbox

[7/7] docg3: Fix miuse of seq_printf return value

Message ID c1a1138f1b05e87a3fbd8737af2f2c0be6e2724b.1412031505.git.joe@perches.com
State Accepted
Commit 8c98d2554f715260efb3e779a9cfe045c4af8aa0
Headers show

Commit Message

Joe Perches Sept. 29, 2014, 11:08 p.m. UTC
seq_printf doesn't return a useful value, so remove
these misuses.

Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/mtd/devices/docg3.c | 112 ++++++++++++++++++++------------------------
 1 file changed, 52 insertions(+), 60 deletions(-)

Comments

Brian Norris Oct. 22, 2014, 8:29 a.m. UTC | #1
On Mon, Sep 29, 2014 at 04:08:27PM -0700, Joe Perches wrote:
> seq_printf doesn't return a useful value, so remove
> these misuses.

Good catch. So it looks like this driver always had some form of
wrongness (returning a character count) in its debugfs callbacks, but
nobody noticed.

Applied to l2-mtd.git. Thanks!

Brian

> Signed-off-by: Joe Perches <joe@perches.com>
> ---
>  drivers/mtd/devices/docg3.c | 112 ++++++++++++++++++++------------------------
>  1 file changed, 52 insertions(+), 60 deletions(-)
> 
> diff --git a/drivers/mtd/devices/docg3.c b/drivers/mtd/devices/docg3.c
> index 21cc4b6..68ff83c 100644
> --- a/drivers/mtd/devices/docg3.c
> +++ b/drivers/mtd/devices/docg3.c
> @@ -1655,22 +1655,21 @@ static int dbg_flashctrl_show(struct seq_file *s, void *p)
>  {
>  	struct docg3 *docg3 = (struct docg3 *)s->private;
>  
> -	int pos = 0;
>  	u8 fctrl;
>  
>  	mutex_lock(&docg3->cascade->lock);
>  	fctrl = doc_register_readb(docg3, DOC_FLASHCONTROL);
>  	mutex_unlock(&docg3->cascade->lock);
>  
> -	pos += seq_printf(s,
> -		 "FlashControl : 0x%02x (%s,CE# %s,%s,%s,flash %s)\n",
> -		 fctrl,
> -		 fctrl & DOC_CTRL_VIOLATION ? "protocol violation" : "-",
> -		 fctrl & DOC_CTRL_CE ? "active" : "inactive",
> -		 fctrl & DOC_CTRL_PROTECTION_ERROR ? "protection error" : "-",
> -		 fctrl & DOC_CTRL_SEQUENCE_ERROR ? "sequence error" : "-",
> -		 fctrl & DOC_CTRL_FLASHREADY ? "ready" : "not ready");
> -	return pos;
> +	seq_printf(s, "FlashControl : 0x%02x (%s,CE# %s,%s,%s,flash %s)\n",
> +		   fctrl,
> +		   fctrl & DOC_CTRL_VIOLATION ? "protocol violation" : "-",
> +		   fctrl & DOC_CTRL_CE ? "active" : "inactive",
> +		   fctrl & DOC_CTRL_PROTECTION_ERROR ? "protection error" : "-",
> +		   fctrl & DOC_CTRL_SEQUENCE_ERROR ? "sequence error" : "-",
> +		   fctrl & DOC_CTRL_FLASHREADY ? "ready" : "not ready");
> +
> +	return 0;
>  }
>  DEBUGFS_RO_ATTR(flashcontrol, dbg_flashctrl_show);
>  
> @@ -1678,58 +1677,56 @@ static int dbg_asicmode_show(struct seq_file *s, void *p)
>  {
>  	struct docg3 *docg3 = (struct docg3 *)s->private;
>  
> -	int pos = 0, pctrl, mode;
> +	int pctrl, mode;
>  
>  	mutex_lock(&docg3->cascade->lock);
>  	pctrl = doc_register_readb(docg3, DOC_ASICMODE);
>  	mode = pctrl & 0x03;
>  	mutex_unlock(&docg3->cascade->lock);
>  
> -	pos += seq_printf(s,
> -			 "%04x : RAM_WE=%d,RSTIN_RESET=%d,BDETCT_RESET=%d,WRITE_ENABLE=%d,POWERDOWN=%d,MODE=%d%d (",
> -			 pctrl,
> -			 pctrl & DOC_ASICMODE_RAM_WE ? 1 : 0,
> -			 pctrl & DOC_ASICMODE_RSTIN_RESET ? 1 : 0,
> -			 pctrl & DOC_ASICMODE_BDETCT_RESET ? 1 : 0,
> -			 pctrl & DOC_ASICMODE_MDWREN ? 1 : 0,
> -			 pctrl & DOC_ASICMODE_POWERDOWN ? 1 : 0,
> -			 mode >> 1, mode & 0x1);
> +	seq_printf(s,
> +		   "%04x : RAM_WE=%d,RSTIN_RESET=%d,BDETCT_RESET=%d,WRITE_ENABLE=%d,POWERDOWN=%d,MODE=%d%d (",
> +		   pctrl,
> +		   pctrl & DOC_ASICMODE_RAM_WE ? 1 : 0,
> +		   pctrl & DOC_ASICMODE_RSTIN_RESET ? 1 : 0,
> +		   pctrl & DOC_ASICMODE_BDETCT_RESET ? 1 : 0,
> +		   pctrl & DOC_ASICMODE_MDWREN ? 1 : 0,
> +		   pctrl & DOC_ASICMODE_POWERDOWN ? 1 : 0,
> +		   mode >> 1, mode & 0x1);
>  
>  	switch (mode) {
>  	case DOC_ASICMODE_RESET:
> -		pos += seq_puts(s, "reset");
> +		seq_puts(s, "reset");
>  		break;
>  	case DOC_ASICMODE_NORMAL:
> -		pos += seq_puts(s, "normal");
> +		seq_puts(s, "normal");
>  		break;
>  	case DOC_ASICMODE_POWERDOWN:
> -		pos += seq_puts(s, "powerdown");
> +		seq_puts(s, "powerdown");
>  		break;
>  	}
> -	pos += seq_puts(s, ")\n");
> -	return pos;
> +	seq_puts(s, ")\n");
> +	return 0;
>  }
>  DEBUGFS_RO_ATTR(asic_mode, dbg_asicmode_show);
>  
>  static int dbg_device_id_show(struct seq_file *s, void *p)
>  {
>  	struct docg3 *docg3 = (struct docg3 *)s->private;
> -	int pos = 0;
>  	int id;
>  
>  	mutex_lock(&docg3->cascade->lock);
>  	id = doc_register_readb(docg3, DOC_DEVICESELECT);
>  	mutex_unlock(&docg3->cascade->lock);
>  
> -	pos += seq_printf(s, "DeviceId = %d\n", id);
> -	return pos;
> +	seq_printf(s, "DeviceId = %d\n", id);
> +	return 0;
>  }
>  DEBUGFS_RO_ATTR(device_id, dbg_device_id_show);
>  
>  static int dbg_protection_show(struct seq_file *s, void *p)
>  {
>  	struct docg3 *docg3 = (struct docg3 *)s->private;
> -	int pos = 0;
>  	int protect, dps0, dps0_low, dps0_high, dps1, dps1_low, dps1_high;
>  
>  	mutex_lock(&docg3->cascade->lock);
> @@ -1742,45 +1739,40 @@ static int dbg_protection_show(struct seq_file *s, void *p)
>  	dps1_high = doc_register_readw(docg3, DOC_DPS1_ADDRHIGH);
>  	mutex_unlock(&docg3->cascade->lock);
>  
> -	pos += seq_printf(s, "Protection = 0x%02x (",
> -			 protect);
> +	seq_printf(s, "Protection = 0x%02x (", protect);
>  	if (protect & DOC_PROTECT_FOUNDRY_OTP_LOCK)
> -		pos += seq_puts(s, "FOUNDRY_OTP_LOCK,");
> +		seq_puts(s, "FOUNDRY_OTP_LOCK,");
>  	if (protect & DOC_PROTECT_CUSTOMER_OTP_LOCK)
> -		pos += seq_puts(s, "CUSTOMER_OTP_LOCK,");
> +		seq_puts(s, "CUSTOMER_OTP_LOCK,");
>  	if (protect & DOC_PROTECT_LOCK_INPUT)
> -		pos += seq_puts(s, "LOCK_INPUT,");
> +		seq_puts(s, "LOCK_INPUT,");
>  	if (protect & DOC_PROTECT_STICKY_LOCK)
> -		pos += seq_puts(s, "STICKY_LOCK,");
> +		seq_puts(s, "STICKY_LOCK,");
>  	if (protect & DOC_PROTECT_PROTECTION_ENABLED)
> -		pos += seq_puts(s, "PROTECTION ON,");
> +		seq_puts(s, "PROTECTION ON,");
>  	if (protect & DOC_PROTECT_IPL_DOWNLOAD_LOCK)
> -		pos += seq_puts(s, "IPL_DOWNLOAD_LOCK,");
> +		seq_puts(s, "IPL_DOWNLOAD_LOCK,");
>  	if (protect & DOC_PROTECT_PROTECTION_ERROR)
> -		pos += seq_puts(s, "PROTECT_ERR,");
> +		seq_puts(s, "PROTECT_ERR,");
>  	else
> -		pos += seq_puts(s, "NO_PROTECT_ERR");
> -	pos += seq_puts(s, ")\n");
> -
> -	pos += seq_printf(s, "DPS0 = 0x%02x : "
> -			 "Protected area [0x%x - 0x%x] : OTP=%d, READ=%d, "
> -			 "WRITE=%d, HW_LOCK=%d, KEY_OK=%d\n",
> -			 dps0, dps0_low, dps0_high,
> -			 !!(dps0 & DOC_DPS_OTP_PROTECTED),
> -			 !!(dps0 & DOC_DPS_READ_PROTECTED),
> -			 !!(dps0 & DOC_DPS_WRITE_PROTECTED),
> -			 !!(dps0 & DOC_DPS_HW_LOCK_ENABLED),
> -			 !!(dps0 & DOC_DPS_KEY_OK));
> -	pos += seq_printf(s, "DPS1 = 0x%02x : "
> -			 "Protected area [0x%x - 0x%x] : OTP=%d, READ=%d, "
> -			 "WRITE=%d, HW_LOCK=%d, KEY_OK=%d\n",
> -			 dps1, dps1_low, dps1_high,
> -			 !!(dps1 & DOC_DPS_OTP_PROTECTED),
> -			 !!(dps1 & DOC_DPS_READ_PROTECTED),
> -			 !!(dps1 & DOC_DPS_WRITE_PROTECTED),
> -			 !!(dps1 & DOC_DPS_HW_LOCK_ENABLED),
> -			 !!(dps1 & DOC_DPS_KEY_OK));
> -	return pos;
> +		seq_puts(s, "NO_PROTECT_ERR");
> +	seq_puts(s, ")\n");
> +
> +	seq_printf(s, "DPS0 = 0x%02x : Protected area [0x%x - 0x%x] : OTP=%d, READ=%d, WRITE=%d, HW_LOCK=%d, KEY_OK=%d\n",
> +		   dps0, dps0_low, dps0_high,
> +		   !!(dps0 & DOC_DPS_OTP_PROTECTED),
> +		   !!(dps0 & DOC_DPS_READ_PROTECTED),
> +		   !!(dps0 & DOC_DPS_WRITE_PROTECTED),
> +		   !!(dps0 & DOC_DPS_HW_LOCK_ENABLED),
> +		   !!(dps0 & DOC_DPS_KEY_OK));
> +	seq_printf(s, "DPS1 = 0x%02x : Protected area [0x%x - 0x%x] : OTP=%d, READ=%d, WRITE=%d, HW_LOCK=%d, KEY_OK=%d\n",
> +		   dps1, dps1_low, dps1_high,
> +		   !!(dps1 & DOC_DPS_OTP_PROTECTED),
> +		   !!(dps1 & DOC_DPS_READ_PROTECTED),
> +		   !!(dps1 & DOC_DPS_WRITE_PROTECTED),
> +		   !!(dps1 & DOC_DPS_HW_LOCK_ENABLED),
> +		   !!(dps1 & DOC_DPS_KEY_OK));
> +	return 0;
>  }
>  DEBUGFS_RO_ATTR(protection, dbg_protection_show);
>
Steven Rostedt Oct. 28, 2014, 3:13 p.m. UTC | #2
On Wed, 22 Oct 2014 01:29:16 -0700
Brian Norris <computersforpeace@gmail.com> wrote:

> On Mon, Sep 29, 2014 at 04:08:27PM -0700, Joe Perches wrote:
> > seq_printf doesn't return a useful value, so remove
> > these misuses.
> 
> Good catch. So it looks like this driver always had some form of
> wrongness (returning a character count) in its debugfs callbacks, but
> nobody noticed.
> 
> Applied to l2-mtd.git. Thanks!
> 

Note, I'm going to be working on changes to remove the return value of
seq_printf() and friends. I need this change as well. If you
already applied it to your git tree, if you can still rebase, can you
make a separate branch off of Linus's tree that I can pull to do work
on?

Or I can take this patch as well, with your Acked-by.

Thanks!

-- Steve
Joe Perches Oct. 28, 2014, 3:57 p.m. UTC | #3
On Tue, 2014-10-28 at 11:13 -0400, Steven Rostedt wrote:
> I'm going to be working on changes to remove the return value of
> seq_printf() and friends.

I'm sure you know all of this, but for anyone else:

This doesn't need to be done in a single pass.

The seq_<foo> functions themselves can wait until
all the uses are converted.

Here are files that likely need to change:

$ git grep --name-only -E "[\(=]\s*seq_(vprintf|printf|puts|putc|escape|write|put_decimal_\w+)\s*\("
arch/arm/plat-pxa/dma.c
arch/cris/arch-v10/kernel/fasttimer.c
arch/cris/arch-v32/kernel/fasttimer.c
arch/microblaze/kernel/cpu/mb.c
arch/x86/kernel/cpu/mtrr/if.c
drivers/base/power/wakeup.c
drivers/mfd/ab8500-debugfs.c
drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c
drivers/parisc/ccio-dma.c
drivers/parisc/sba_iommu.c
drivers/regulator/dbx500-prcmu.c
drivers/staging/lustre/lustre/fid/lproc_fid.c
drivers/staging/lustre/lustre/llite/lproc_llite.c
drivers/staging/lustre/lustre/mdc/lproc_mdc.c
drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
drivers/staging/lustre/lustre/osc/lproc_osc.c
drivers/staging/lustre/lustre/ptlrpc/lproc_ptlrpc.c
drivers/staging/lustre/lustre/ptlrpc/sec_bulk.c
drivers/usb/gadget/udc/goku_udc.c
drivers/usb/gadget/udc/pxa27x_udc.c
drivers/watchdog/bcm_kona_wdt.c
fs/debugfs/file.c
fs/dlm/debug_fs.c
fs/eventfd.c
fs/eventpoll.c
fs/notify/fdinfo.c
fs/proc/base.c
fs/seq_file.c
kernel/trace/trace_seq.c
net/batman-adv/gateway_client.c
net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
net/netfilter/nf_conntrack_standalone.c
net/netfilter/nf_log.c
net/netfilter/xt_hashlimit.c
Steven Rostedt Oct. 28, 2014, 4:05 p.m. UTC | #4
On Tue, 28 Oct 2014 08:57:57 -0700
Joe Perches <joe@perches.com> wrote:

> On Tue, 2014-10-28 at 11:13 -0400, Steven Rostedt wrote:
> > I'm going to be working on changes to remove the return value of
> > seq_printf() and friends.
> 
> I'm sure you know all of this, but for anyone else:
> 
> This doesn't need to be done in a single pass.

Yeah, I took a look at all the places, and I've decided to take it off
in chunks. I'm starting with the ones you posted, and will try to get
acks for them. And then go after other chunks as I have time.

I would like to get this done before I do my merge of trace_seq and
seq_file, but I'm thinking I may have to do that in parallel.

-- Steve
Brian Norris Oct. 28, 2014, 5:14 p.m. UTC | #5
On Tue, Oct 28, 2014 at 12:05:52PM -0400, Steven Rostedt wrote:
> On Tue, 28 Oct 2014 08:57:57 -0700
> Joe Perches <joe@perches.com> wrote:
> 
> > On Tue, 2014-10-28 at 11:13 -0400, Steven Rostedt wrote:
> > > I'm going to be working on changes to remove the return value of
> > > seq_printf() and friends.
> > 
> > I'm sure you know all of this, but for anyone else:
> > 
> > This doesn't need to be done in a single pass.
> 
> Yeah, I took a look at all the places, and I've decided to take it off
> in chunks. I'm starting with the ones you posted, and will try to get
> acks for them. And then go after other chunks as I have time.

I'll keep this in my tree as-is for now. Let me know if you still need
something changed. I can give my 'Ack' if you really want to pull this
into a separate branch.

BTW, I just noticed the $subject has a typo: s/miuse/misuse/

Brian
Steven Rostedt Oct. 28, 2014, 5:26 p.m. UTC | #6
On Tue, 28 Oct 2014 10:14:09 -0700
Brian Norris <computersforpeace@gmail.com> wrote:

> On Tue, Oct 28, 2014 at 12:05:52PM -0400, Steven Rostedt wrote:
> > On Tue, 28 Oct 2014 08:57:57 -0700
> > Joe Perches <joe@perches.com> wrote:
> > 
> > > On Tue, 2014-10-28 at 11:13 -0400, Steven Rostedt wrote:
> > > > I'm going to be working on changes to remove the return value of
> > > > seq_printf() and friends.
> > > 
> > > I'm sure you know all of this, but for anyone else:
> > > 
> > > This doesn't need to be done in a single pass.
> > 
> > Yeah, I took a look at all the places, and I've decided to take it off
> > in chunks. I'm starting with the ones you posted, and will try to get
> > acks for them. And then go after other chunks as I have time.
> 
> I'll keep this in my tree as-is for now. Let me know if you still need
> something changed. I can give my 'Ack' if you really want to pull this
> into a separate branch.

If I get to a point where I can change the seq_printf() to return void,
then I'll want it, otherwise without it, it will cause my branch to fail
to compile on that code.

That's why I would like it. If we keep it in your tree and have that for
the next release, it will matter which tree goes into Linus's tree
first. If mine goes in first, then it will break his build.

Now, that's assuming I get this ready for 3.19, as I'll need quite a
few Acked-by's for the changes that I'll be making. If I don't get it
by 3.19, then it wont be an issue if that change goes in with your tree.

-- Steve

> 
> BTW, I just noticed the $subject has a typo: s/miuse/misuse/
Joe Perches Oct. 28, 2014, 5:27 p.m. UTC | #7
On Tue, 2014-10-28 at 12:05 -0400, Steven Rostedt wrote:
> On Tue, 28 Oct 2014 08:57:57 -0700
> Joe Perches <joe@perches.com> wrote:
> 
> > On Tue, 2014-10-28 at 11:13 -0400, Steven Rostedt wrote:
> > > I'm going to be working on changes to remove the return value of
> > > seq_printf() and friends.
> > 
> > I'm sure you know all of this, but for anyone else:
> > 
> > This doesn't need to be done in a single pass.
> 
> Yeah, I took a look at all the places, and I've decided to take it off
> in chunks. I'm starting with the ones you posted, and will try to get
> acks for them. And then go after other chunks as I have time.
> 
> I would like to get this done before I do my merge of trace_seq and
> seq_file, but I'm thinking I may have to do that in parallel.

I think the most important thing is to get the
seq_is_overflown (or seq_has_overflowed or whatever
other name is chosen) function added then the rest
of the patches can be applied whenever maintainers
(or Andrew or trivial or ...) pick them up.
Steven Rostedt Oct. 28, 2014, 5:32 p.m. UTC | #8
On Tue, 28 Oct 2014 10:27:40 -0700
Joe Perches <joe@perches.com> wrote:

> On Tue, 2014-10-28 at 12:05 -0400, Steven Rostedt wrote:
> > On Tue, 28 Oct 2014 08:57:57 -0700
> > Joe Perches <joe@perches.com> wrote:
> > 
> > > On Tue, 2014-10-28 at 11:13 -0400, Steven Rostedt wrote:
> > > > I'm going to be working on changes to remove the return value of
> > > > seq_printf() and friends.
> > > 
> > > I'm sure you know all of this, but for anyone else:
> > > 
> > > This doesn't need to be done in a single pass.
> > 
> > Yeah, I took a look at all the places, and I've decided to take it off
> > in chunks. I'm starting with the ones you posted, and will try to get
> > acks for them. And then go after other chunks as I have time.
> > 
> > I would like to get this done before I do my merge of trace_seq and
> > seq_file, but I'm thinking I may have to do that in parallel.
> 
> I think the most important thing is to get the
> seq_is_overflown (or seq_has_overflowed or whatever
> other name is chosen) function added then the rest
> of the patches can be applied whenever maintainers
> (or Andrew or trivial or ...) pick them up.

I can get that in without much of an issue. But the merge of trace_seq
and seq_file would be easier if I didn't have to worry about return
values. Which is why I want to get this in quickly.

-- Steve
Brian Norris Oct. 28, 2014, 5:36 p.m. UTC | #9
On Tue, Oct 28, 2014 at 10:32 AM, Steven Rostedt <rostedt@goodmis.org> wrote:
> On Tue, 28 Oct 2014 10:27:40 -0700
> Joe Perches <joe@perches.com> wrote:
>> On Tue, 2014-10-28 at 12:05 -0400, Steven Rostedt wrote:
>> > On Tue, 28 Oct 2014 08:57:57 -0700
>> > Joe Perches <joe@perches.com> wrote:
>> >
>> > > On Tue, 2014-10-28 at 11:13 -0400, Steven Rostedt wrote:
>> > > > I'm going to be working on changes to remove the return value of
>> > > > seq_printf() and friends.
>> > >
>> > > I'm sure you know all of this, but for anyone else:
>> > >
>> > > This doesn't need to be done in a single pass.
>> >
>> > Yeah, I took a look at all the places, and I've decided to take it off
>> > in chunks. I'm starting with the ones you posted, and will try to get
>> > acks for them. And then go after other chunks as I have time.
>> >
>> > I would like to get this done before I do my merge of trace_seq and
>> > seq_file, but I'm thinking I may have to do that in parallel.
>>
>> I think the most important thing is to get the
>> seq_is_overflown (or seq_has_overflowed or whatever
>> other name is chosen) function added then the rest
>> of the patches can be applied whenever maintainers
>> (or Andrew or trivial or ...) pick them up.
>
> I can get that in without much of an issue. But the merge of trace_seq
> and seq_file would be easier if I didn't have to worry about return
> values. Which is why I want to get this in quickly.

Whatever you'd like. Please, have my ack and keep the patch in your own branch!

Acked-by: Brian Norris <computersforpeace@gmail.com>

It's no problem if we both have the patch, is it? Or if I see it in
linux-next, then I may drop it from my tree.

Regards,
Brian
Joe Perches Oct. 28, 2014, 5:38 p.m. UTC | #10
On Tue, 2014-10-28 at 13:32 -0400, Steven Rostedt wrote:
> On Tue, 28 Oct 2014 10:27:40 -0700 Joe Perches <joe@perches.com> wrote:
> > On Tue, 2014-10-28 at 12:05 -0400, Steven Rostedt wrote:
> > > I would like to get this done before I do my merge of trace_seq and
> > > seq_file, but I'm thinking I may have to do that in parallel.
> > 
> > I think the most important thing is to get the
> > seq_is_overflown (or seq_has_overflowed or whatever
> > other name is chosen) function added then the rest
> > of the patches can be applied whenever maintainers
> > (or Andrew or trivial or ...) pick them up.
> 
> I can get that in without much of an issue. But the merge of trace_seq
> and seq_file would be easier if I didn't have to worry about return
> values. Which is why I want to get this in quickly.

What is the issue there?
diff mbox

Patch

diff --git a/drivers/mtd/devices/docg3.c b/drivers/mtd/devices/docg3.c
index 21cc4b6..68ff83c 100644
--- a/drivers/mtd/devices/docg3.c
+++ b/drivers/mtd/devices/docg3.c
@@ -1655,22 +1655,21 @@  static int dbg_flashctrl_show(struct seq_file *s, void *p)
 {
 	struct docg3 *docg3 = (struct docg3 *)s->private;
 
-	int pos = 0;
 	u8 fctrl;
 
 	mutex_lock(&docg3->cascade->lock);
 	fctrl = doc_register_readb(docg3, DOC_FLASHCONTROL);
 	mutex_unlock(&docg3->cascade->lock);
 
-	pos += seq_printf(s,
-		 "FlashControl : 0x%02x (%s,CE# %s,%s,%s,flash %s)\n",
-		 fctrl,
-		 fctrl & DOC_CTRL_VIOLATION ? "protocol violation" : "-",
-		 fctrl & DOC_CTRL_CE ? "active" : "inactive",
-		 fctrl & DOC_CTRL_PROTECTION_ERROR ? "protection error" : "-",
-		 fctrl & DOC_CTRL_SEQUENCE_ERROR ? "sequence error" : "-",
-		 fctrl & DOC_CTRL_FLASHREADY ? "ready" : "not ready");
-	return pos;
+	seq_printf(s, "FlashControl : 0x%02x (%s,CE# %s,%s,%s,flash %s)\n",
+		   fctrl,
+		   fctrl & DOC_CTRL_VIOLATION ? "protocol violation" : "-",
+		   fctrl & DOC_CTRL_CE ? "active" : "inactive",
+		   fctrl & DOC_CTRL_PROTECTION_ERROR ? "protection error" : "-",
+		   fctrl & DOC_CTRL_SEQUENCE_ERROR ? "sequence error" : "-",
+		   fctrl & DOC_CTRL_FLASHREADY ? "ready" : "not ready");
+
+	return 0;
 }
 DEBUGFS_RO_ATTR(flashcontrol, dbg_flashctrl_show);
 
@@ -1678,58 +1677,56 @@  static int dbg_asicmode_show(struct seq_file *s, void *p)
 {
 	struct docg3 *docg3 = (struct docg3 *)s->private;
 
-	int pos = 0, pctrl, mode;
+	int pctrl, mode;
 
 	mutex_lock(&docg3->cascade->lock);
 	pctrl = doc_register_readb(docg3, DOC_ASICMODE);
 	mode = pctrl & 0x03;
 	mutex_unlock(&docg3->cascade->lock);
 
-	pos += seq_printf(s,
-			 "%04x : RAM_WE=%d,RSTIN_RESET=%d,BDETCT_RESET=%d,WRITE_ENABLE=%d,POWERDOWN=%d,MODE=%d%d (",
-			 pctrl,
-			 pctrl & DOC_ASICMODE_RAM_WE ? 1 : 0,
-			 pctrl & DOC_ASICMODE_RSTIN_RESET ? 1 : 0,
-			 pctrl & DOC_ASICMODE_BDETCT_RESET ? 1 : 0,
-			 pctrl & DOC_ASICMODE_MDWREN ? 1 : 0,
-			 pctrl & DOC_ASICMODE_POWERDOWN ? 1 : 0,
-			 mode >> 1, mode & 0x1);
+	seq_printf(s,
+		   "%04x : RAM_WE=%d,RSTIN_RESET=%d,BDETCT_RESET=%d,WRITE_ENABLE=%d,POWERDOWN=%d,MODE=%d%d (",
+		   pctrl,
+		   pctrl & DOC_ASICMODE_RAM_WE ? 1 : 0,
+		   pctrl & DOC_ASICMODE_RSTIN_RESET ? 1 : 0,
+		   pctrl & DOC_ASICMODE_BDETCT_RESET ? 1 : 0,
+		   pctrl & DOC_ASICMODE_MDWREN ? 1 : 0,
+		   pctrl & DOC_ASICMODE_POWERDOWN ? 1 : 0,
+		   mode >> 1, mode & 0x1);
 
 	switch (mode) {
 	case DOC_ASICMODE_RESET:
-		pos += seq_puts(s, "reset");
+		seq_puts(s, "reset");
 		break;
 	case DOC_ASICMODE_NORMAL:
-		pos += seq_puts(s, "normal");
+		seq_puts(s, "normal");
 		break;
 	case DOC_ASICMODE_POWERDOWN:
-		pos += seq_puts(s, "powerdown");
+		seq_puts(s, "powerdown");
 		break;
 	}
-	pos += seq_puts(s, ")\n");
-	return pos;
+	seq_puts(s, ")\n");
+	return 0;
 }
 DEBUGFS_RO_ATTR(asic_mode, dbg_asicmode_show);
 
 static int dbg_device_id_show(struct seq_file *s, void *p)
 {
 	struct docg3 *docg3 = (struct docg3 *)s->private;
-	int pos = 0;
 	int id;
 
 	mutex_lock(&docg3->cascade->lock);
 	id = doc_register_readb(docg3, DOC_DEVICESELECT);
 	mutex_unlock(&docg3->cascade->lock);
 
-	pos += seq_printf(s, "DeviceId = %d\n", id);
-	return pos;
+	seq_printf(s, "DeviceId = %d\n", id);
+	return 0;
 }
 DEBUGFS_RO_ATTR(device_id, dbg_device_id_show);
 
 static int dbg_protection_show(struct seq_file *s, void *p)
 {
 	struct docg3 *docg3 = (struct docg3 *)s->private;
-	int pos = 0;
 	int protect, dps0, dps0_low, dps0_high, dps1, dps1_low, dps1_high;
 
 	mutex_lock(&docg3->cascade->lock);
@@ -1742,45 +1739,40 @@  static int dbg_protection_show(struct seq_file *s, void *p)
 	dps1_high = doc_register_readw(docg3, DOC_DPS1_ADDRHIGH);
 	mutex_unlock(&docg3->cascade->lock);
 
-	pos += seq_printf(s, "Protection = 0x%02x (",
-			 protect);
+	seq_printf(s, "Protection = 0x%02x (", protect);
 	if (protect & DOC_PROTECT_FOUNDRY_OTP_LOCK)
-		pos += seq_puts(s, "FOUNDRY_OTP_LOCK,");
+		seq_puts(s, "FOUNDRY_OTP_LOCK,");
 	if (protect & DOC_PROTECT_CUSTOMER_OTP_LOCK)
-		pos += seq_puts(s, "CUSTOMER_OTP_LOCK,");
+		seq_puts(s, "CUSTOMER_OTP_LOCK,");
 	if (protect & DOC_PROTECT_LOCK_INPUT)
-		pos += seq_puts(s, "LOCK_INPUT,");
+		seq_puts(s, "LOCK_INPUT,");
 	if (protect & DOC_PROTECT_STICKY_LOCK)
-		pos += seq_puts(s, "STICKY_LOCK,");
+		seq_puts(s, "STICKY_LOCK,");
 	if (protect & DOC_PROTECT_PROTECTION_ENABLED)
-		pos += seq_puts(s, "PROTECTION ON,");
+		seq_puts(s, "PROTECTION ON,");
 	if (protect & DOC_PROTECT_IPL_DOWNLOAD_LOCK)
-		pos += seq_puts(s, "IPL_DOWNLOAD_LOCK,");
+		seq_puts(s, "IPL_DOWNLOAD_LOCK,");
 	if (protect & DOC_PROTECT_PROTECTION_ERROR)
-		pos += seq_puts(s, "PROTECT_ERR,");
+		seq_puts(s, "PROTECT_ERR,");
 	else
-		pos += seq_puts(s, "NO_PROTECT_ERR");
-	pos += seq_puts(s, ")\n");
-
-	pos += seq_printf(s, "DPS0 = 0x%02x : "
-			 "Protected area [0x%x - 0x%x] : OTP=%d, READ=%d, "
-			 "WRITE=%d, HW_LOCK=%d, KEY_OK=%d\n",
-			 dps0, dps0_low, dps0_high,
-			 !!(dps0 & DOC_DPS_OTP_PROTECTED),
-			 !!(dps0 & DOC_DPS_READ_PROTECTED),
-			 !!(dps0 & DOC_DPS_WRITE_PROTECTED),
-			 !!(dps0 & DOC_DPS_HW_LOCK_ENABLED),
-			 !!(dps0 & DOC_DPS_KEY_OK));
-	pos += seq_printf(s, "DPS1 = 0x%02x : "
-			 "Protected area [0x%x - 0x%x] : OTP=%d, READ=%d, "
-			 "WRITE=%d, HW_LOCK=%d, KEY_OK=%d\n",
-			 dps1, dps1_low, dps1_high,
-			 !!(dps1 & DOC_DPS_OTP_PROTECTED),
-			 !!(dps1 & DOC_DPS_READ_PROTECTED),
-			 !!(dps1 & DOC_DPS_WRITE_PROTECTED),
-			 !!(dps1 & DOC_DPS_HW_LOCK_ENABLED),
-			 !!(dps1 & DOC_DPS_KEY_OK));
-	return pos;
+		seq_puts(s, "NO_PROTECT_ERR");
+	seq_puts(s, ")\n");
+
+	seq_printf(s, "DPS0 = 0x%02x : Protected area [0x%x - 0x%x] : OTP=%d, READ=%d, WRITE=%d, HW_LOCK=%d, KEY_OK=%d\n",
+		   dps0, dps0_low, dps0_high,
+		   !!(dps0 & DOC_DPS_OTP_PROTECTED),
+		   !!(dps0 & DOC_DPS_READ_PROTECTED),
+		   !!(dps0 & DOC_DPS_WRITE_PROTECTED),
+		   !!(dps0 & DOC_DPS_HW_LOCK_ENABLED),
+		   !!(dps0 & DOC_DPS_KEY_OK));
+	seq_printf(s, "DPS1 = 0x%02x : Protected area [0x%x - 0x%x] : OTP=%d, READ=%d, WRITE=%d, HW_LOCK=%d, KEY_OK=%d\n",
+		   dps1, dps1_low, dps1_high,
+		   !!(dps1 & DOC_DPS_OTP_PROTECTED),
+		   !!(dps1 & DOC_DPS_READ_PROTECTED),
+		   !!(dps1 & DOC_DPS_WRITE_PROTECTED),
+		   !!(dps1 & DOC_DPS_HW_LOCK_ENABLED),
+		   !!(dps1 & DOC_DPS_KEY_OK));
+	return 0;
 }
 DEBUGFS_RO_ATTR(protection, dbg_protection_show);