Content-Type: text/plain;
        charset=us-ascii
Mime-Version: 1.0 (1.0)
Subject: Re: bug fix for registers debugfs file implementation [RFC]
From: Variksla <variksla@gmail.com>
In-Reply-To: <20170421172709.dqz7vq3vor56a5qg@sirena.org.uk>
Date: Sat, 22 Apr 2017 13:28:15 -0700
Cc: linux-kernel@vger.kernel.org, gregkh@linuxfoundation.org
Message-Id: <A6A78A0A-155B-42E2-B6CA-2182FD4C997B@gmail.com>
References: <CAES_P+-Y1eocM043HDtU1T6Aj=UaX80pj0z37SmYhVyhY1YZsw@mail.gmail.com> <20170421172709.dqz7vq3vor56a5qg@sirena.org.uk>
To: Mark Brown <broonie@kernel.org>, ckeepax@opensource.wolfsonmicro.com
Sender: linux-kernel-owner@vger.kernel.org
Content-Transfer-Encoding: 8bit
Content-Length: 2209
Lines: 50


> 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?
> 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?

> something if we end a register exactly on a page boundary or something?
> 
> Also your patch is completely tab/space mangled so it's not appliable.
> 
>> +static void regmap_debugfs_stop(struct seq_file *s, void *v)
>> +{
>> +}
> 
> The need for empty functions is worrying...

In several other places in kernel code the usage is similar.