Date: Mon, 24 Apr 2017 10:02:31 +0100
From: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
To: Variksla <variksla@gmail.com>
CC: Mark Brown <broonie@kernel.org>, <linux-kernel@vger.kernel.org>,
        <gregkh@linuxfoundation.org>
Subject: Re: bug fix for registers debugfs file implementation [RFC]
Message-ID: <20170424090231.GF1594@localhost.localdomain>
References: <CAES_P+-Y1eocM043HDtU1T6Aj=UaX80pj0z37SmYhVyhY1YZsw@mail.gmail.com>
 <20170421172709.dqz7vq3vor56a5qg@sirena.org.uk>
 <A6A78A0A-155B-42E2-B6CA-2182FD4C997B@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <A6A78A0A-155B-42E2-B6CA-2182FD4C997B@gmail.com>
User-Agent: Mutt/1.5.23 (2014-03-12)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2984
Lines: 67

On Sat, Apr 22, 2017 at 01:28:15PM -0700, Variksla wrote:
> 
> 
> > On Apr 21, 2017, at 10:27 AM, Mark Brown <broonie@kernel.org> wrote:
> 
> Thanks for the response.
> > 
> >> On Sat, Apr 01, 2017 at 02:13:41AM -0700, noman pouigt wrote:
> >> 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?
> > 
> > If I remember correctly this is done the way it is because seq_file has
> > to iterate through the entire file to get to the point being read by the
> > application.  This is a *very* big overhead for some applications (like
> > monitoring some registers to see what they're doing) on bigger devices,
> 
> Wondering why would the user space application be monitoring the registers?

We do have tooling that accesses and occasionally monitors
registers using the debugfs files. It is often used by hardware
types while testing/debugging issues. And as Mark points out
speed is quite a priority for us on this front so we would be
very keen to avoid anything that slows down the debugfs interface
to the regmap and the ability to seek and acccess just part of
the file is absolutely vital.

> > especially if there's lots of uncached stuff and the reads have to go to
> > the hardware.  With the current code you can open the file, seek to the
> > registers you're interested in and read only them.  You'll notice that
> > the other debug files have been converted over to seq_file as they're
> > pure software and there's less reason to repeatedly read them.
> 
> Yes. I noticed that and that is why I realized that I am doing something wrong.
> > 
> > I'm also very surprised that this is failing for you as I know this code
> > has been fairly heavily exercised with devices with very large register
> > maps much larger than 4k and my own testing is mainly doing cats which
> > seemed to work last time I tried and should be iterating over the 4k
> > boundary...  what's the actual failure mode?  I'm guessing it's
> 
> I have integrated Florida codec(wm8281) from cirrus and it has more than 4K registers. I could not dump more than 4K with the existing interface.
> 
> Charles, are you able to dump all the registers?
> 

I seem to be able to dump all the registers just fine here (all
500k of them). You say it only dumps 4k worth of registers then
just stops? What version of the kernel are you testing this
on? The test I just ran was on Linus's tree, is this an older
kernel or for-next?

If its an older kernel are you perhaps missing one of these
fixes:

commit 26ee47411ae22caa07d3f3b63ca6d097cba6681b
regmap: debugfs: Fix continued read from registers file

commit 213fa5d9685b985e0c61a8db1883a3abf94b18d7
regmap: debugfs: Fix return from regmap_debugfs_get_dump_start

Thanks,
Charles