lkml.org 
[lkml]   [2017]   [Apr]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: bug fix for registers debugfs file implementation [RFC]
From
Date


> 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.
\
 
 \ /
  Last update: 2017-04-22 22:29    [W:0.056 / U:0.068 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site