[BUG ?] regmap: debugfs: WARN_ON at regmap-debugfs.c:151

[BUG ?] regmap: debugfs: WARN_ON at regmap-debugfs.c:151

[Sergej Sawazki]

[-- Attachment #1: Type: text/plain, Size: 587 bytes --]

Hello all,

when I try to read the register values of a i2c ASoC codec through
debugfs, (eg: cat /sys/kernel/debug/regmap/1-001a/registers)
I am getting an WARN_ON at drivers/base/regmap/regmap-debugfs.c:151.
Please see attached dmesg.txt for details.

I did some debugging and found that regmap_readable(), at regmap-
debugfs.c:109, returns "false" for all registers, hence the 
debugfs_off_cache list remains empty and we get an warning in line 151.

Reverting commit 4191f1979 "regmap: if format_write is used, declare
all registers as "unreadable"" fixed the issue.

Regards,
Sergej

[-- Attachment #2: dmesg.txt --]
[-- Type: text/plain, Size: 1572 bytes --]

[  507.037105] ------------[ cut here ]------------
[  507.037173] WARNING: CPU: 0 PID: 2609 at drivers/base/regmap/regmap-debugfs.c:151 regmap_debugfs_get_dump_start.part.0+0x210/0x26c()
[  507.037188] Modules linked in: 8192cu snd_soc_wm8741 regmap_spi snd_soc_bcm2708_i2s regmap_mmio snd_soc_tau_dac
snd_soc_core snd_compress snd_pcm_dmaengine snd_pcm_oss snd_mixer_oss snd_pcm snd_seq_dummy snd_seq_midi snd_rawmidi snd_seq_oss snd_seq_midi_ev
ent snd_seq snd_seq_device snd_timer clk_si5351 regmap_i2c snd i2c_bcm2708 sg uio_pdrv_genirq uio
[  507.037320] CPU: 0 PID: 2609 Comm: cat Tainted: G        W      3.18.7-tau-dac-00032-g604bbbc-dirty #54
[  507.037388] [<c00158e4>] (unwind_backtrace) from [<c0012c40>] (show_stack+0x20/0x24)
[  507.037438] [<c0012c40>] (show_stack) from [<c0554008>] (dump_stack+0x20/0x28)
[  507.037480] [<c0554008>] (dump_stack) from [<c0023608>] (warn_slowpath_common+0x7c/0x9c)
[  507.037511] [<c0023608>] (warn_slowpath_common) from [<c0023654>] (warn_slowpath_null+0x2c/0x34)
[  507.037539] [<c0023654>] (warn_slowpath_null) from [<c0375a30>] (regmap_debugfs_get_dump_start.part.0+0x210/0x26c)
[  507.037569] [<c0375a30>] (regmap_debugfs_get_dump_start.part.0) from [<c0375b30>] (regmap_reg_ranges_read_file+0xa4/0x280)
[  507.037609] [<c0375b30>] (regmap_reg_ranges_read_file) from [<c0136a0c>] (vfs_read+0x98/0x184)
[  507.037639] [<c0136a0c>] (vfs_read) from [<c0136b44>] (SyS_read+0x4c/0x98)
[  507.037668] [<c0136b44>] (SyS_read) from [<c000f580>] (ret_fast_syscall+0x0/0x48)
[  507.037684] ---[ end trace b52574e72eb4e3d1 ]---

Re: [BUG ?] regmap: debugfs: WARN_ON at regmap-debugfs.c:151

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 884 bytes --]

On Sat, Jun 13, 2015 at 03:21:02PM +0200, Sergej Sawazki wrote:

> when I try to read the register values of a i2c ASoC codec through
> debugfs, (eg: cat /sys/kernel/debug/regmap/1-001a/registers)
> I am getting an WARN_ON at drivers/base/regmap/regmap-debugfs.c:151.

Please provide some plain text description of which line of code you're
referring to when reporting things - line numbers do change as things
move on (though I see this one).

> I did some debugging and found that regmap_readable(), at regmap-
> debugfs.c:109, returns "false" for all registers, hence the
> debugfs_off_cache list remains empty and we get an warning in line 151.

We need to change that code to special case write only register maps
like this and just skip having a cache for those devices.  They normally
have very small register maps anyway so it shouldn't have too big an
impact on performance.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [BUG ?] regmap: debugfs: WARN_ON at regmap-debugfs.c:151

[Sergej Sawazki]

Am 15. Juni 2015 11:49:22 MESZ, schrieb Mark Brown <broonie@kernel.org>:
>On Sat, Jun 13, 2015 at 03:21:02PM +0200, Sergej Sawazki wrote:
>
>> I did some debugging and found that regmap_readable(), at regmap-
>> debugfs.c:109, returns "false" for all registers, hence the
>> debugfs_off_cache list remains empty and we get an warning in line
>151.
>
>We need to change that code to special case write only register maps
>like this and just skip having a cache for those devices.  They
>normally
>have very small register maps anyway so it shouldn't have too big an
>impact on performance.

I'm having this issue with a wm8741 DAC, its register map is not "write only", it has readable and writeable registers.

-- 
Sergej

Re: [BUG ?] regmap: debugfs: WARN_ON at regmap-debugfs.c:151

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 812 bytes --]

On Mon, Jun 15, 2015 at 02:42:30PM +0200, Sergej Sawazki wrote:
> Am 15. Juni 2015 11:49:22 MESZ, schrieb Mark Brown <broonie@kernel.org>:

> >We need to change that code to special case write only register maps
> >like this and just skip having a cache for those devices.  They
> >normally
> >have very small register maps anyway so it shouldn't have too big an
> >impact on performance.

> I'm having this issue with a wm8741 DAC, its register map is not
> "write only", it has readable and writeable registers.

Are you *sure* there are physically readable registers on the device?
It's difficult to implement for 7x9 devices given that the top register
bit is in the byte used to send the register address, I expect you'll
find that the registers marked as readable are actually those that
should be cached.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [BUG ?] regmap: debugfs: WARN_ON at regmap-debugfs.c:151

[Sergej Sawazki]

On Mon, 15 Jun 2015 15:49:33 +0100, Mark Brown wrote:
> On Mon, Jun 15, 2015 at 02:42:30PM +0200, Sergej Sawazki wrote:
>> Am 15. Juni 2015 11:49:22 MESZ, schrieb Mark Brown <broonie@kernel.org>:
>
>>> We need to change that code to special case write only register maps
>>> like this and just skip having a cache for those devices.  They
>>> normally
>>> have very small register maps anyway so it shouldn't have too big an
>>> impact on performance.
>
>> I'm having this issue with a wm8741 DAC, its register map is not
>> "write only", it has readable and writeable registers.
>
> Are you *sure* there are physically readable registers on the device?
> It's difficult to implement for 7x9 devices given that the top register
> bit is in the byte used to send the register address, I expect you'll
> find that the registers marked as readable are actually those that
> should be cached.
>

You are right, the registers are not physically readable. Sorry for the
confusion.

What I actually was trying to do is to read the cached register values.

Re: [BUG ?] regmap: debugfs: WARN_ON at regmap-debugfs.c:151

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 826 bytes --]

On Wed, Jun 24, 2015 at 10:49:59PM +0200, Sergej Sawazki wrote:
> On Mon, 15 Jun 2015 15:49:33 +0100, Mark Brown wrote:

> >Are you *sure* there are physically readable registers on the device?
> >It's difficult to implement for 7x9 devices given that the top register
> >bit is in the byte used to send the register address, I expect you'll
> >find that the registers marked as readable are actually those that
> >should be cached.

> You are right, the registers are not physically readable. Sorry for the
> confusion.

> What I actually was trying to do is to read the cached register values.

OK, so what we need to do here is rearrange things so we check the cache
before we check for readability.  That way if there is a cached value
then we'll return it and the readability check will still reflect the
hardware state.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]