Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1660453 > unrolled thread

Re: bug fix for registers debugfs file implementation [RFC]

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: bug fix for registers debugfs file implementation [RFC] noman pouigt <variksla@gmail.com> - 2017-06-08 01:40 +0200
    Re: bug fix for registers debugfs file implementation [RFC] Charles Keepax <ckeepax@opensource.wolfsonmicro.com> - 2017-06-08 10:50 +0200

#1660453 — Re: bug fix for registers debugfs file implementation [RFC]

On Mon, Apr 24, 2017 at 2:02 AM, Charles Keepax
<ckeepax@opensource.wolfsonmicro.com> wrote:

Thanks for replying.

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

I am able to dump till 0x00058a address.

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

Using
https://github.com/CirrusLogic/linux-drivers/tree/v3.18-arizona/drivers

> fixes:

I have both the patches mentioned here in my tree.
>
> commit 26ee47411ae22caa07d3f3b63ca6d097cba6681b
> regmap: debugfs: Fix continued read from registers file
>
> commit 213fa5d9685b985e0c61a8db1883a3abf94b18d7
> regmap: debugfs: Fix return from regmap_debugfs_get_dump_start
>
> Thanks,
> Charles

[toc] | [next] | [standalone]

#1660947

On Wed, Jun 07, 2017 at 04:31:15PM -0700, noman pouigt wrote:
> On Mon, Apr 24, 2017 at 2:02 AM, Charles Keepax
> <ckeepax@opensource.wolfsonmicro.com> wrote:
> >> > 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
> 
> Using
> https://github.com/CirrusLogic/linux-drivers/tree/v3.18-arizona/drivers
> 

I am able to dump all the registers if I merge in that branch
into my stock 3.18 setup here. My guess would be there must be
some vendor kernel excitement or something going on here. If you
can ping through a copy of your
drivers/base/regmap/regmap-debugfs.c I can have a look at that
see how it compares to mine.

But I suspect you might need to work through and work out where
it is bombing out in regmap_read_debugfs.

Thanks,
Charles

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web