linux-kernel - Re: [PATCH] ASoC: pcm512x: Mend accesses to the I2S_1 and I2S

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <8ee31fd2-3fbe-f5da-a030-c6b677c7f398@axentia.se>
Date:   Tue, 21 Sep 2021 08:37:28 +0200
From:   Peter Rosin <peda@...ntia.se>
To:     Mark Brown <broonie@...nel.org>
Cc:     Péter Ujfalusi <peter.ujfalusi@...il.com>,
        "alsa-devel@...a-project.org" <alsa-devel@...a-project.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Takashi Iwai <tiwai@...e.com>,
        Liam Girdwood <lgirdwood@...il.com>,
        Kirill Marinushkin <kmarinushkin@...dec.com>
Subject: Re: [PATCH] ASoC: pcm512x: Mend accesses to the I2S_1 and I2S_2
 registers

On 2021-09-20 23:29, Mark Brown wrote:
> On Mon, Sep 20, 2021 at 09:37:37PM +0200, Peter Rosin wrote:
> 
>> compare if the update was needed at all. But marking volatile wasn't
>> enough. I also tried to set both a default and mark as volatile,
>> but it seems every read fails with -16 (EBUSY). I don't get why, to me
>> it almost feels like a regmap issue of some sort (probably the regmap
>> config is bad in some way), but I'm not fluent in regmap...
> 
> Having a default for a volatile register isn't really a sensible
> configuration since the whole point with volatile registers is
> that they change underneath us, I'd not be surprised if we had
> some error checking code in there that was trying to tell you
> there was a problem though it does seem like it should at least
> be more verbose about it since returning -EBUSY isn't exactly
> helpful by itself.

I totally agree that it's not a sensible config to set up a register
with a default when it's marked as volatile. That was just a wild
attempt.

I expected it to just work to mark the register as readable and do
without the default value (i.e. the way it was before my patch). What
I don't understand is why regmap returns -EBUSY in that case. That
doesn't make sense to me. Perhaps that -EBUSY is propagated from the
I2C layer, but in that case, why is it then ok to do a write to
another register at the same spot in the code? So, why -EBUSY?

Something is going on that is not understood. At least not by me.

Cheers,
Peter