Date:   Wed, 7 Jun 2017 16:31:15 -0700
From:   noman pouigt <variksla@...il.com>
To:     Charles Keepax <ckeepax@...nsource.wolfsonmicro.com>
Cc:     Mark Brown <broonie@...nel.org>, linux-kernel@...r.kernel.org,
        gregkh@...uxfoundation.org
Subject: Re: bug fix for registers debugfs file implementation [RFC]

On Mon, Apr 24, 2017 at 2:02 AM, Charles Keepax
<ckeepax@...nsource.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@...nel.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

Powered by blists - more mailing lists