linux-kernel - Re: [PATCH] ALSA: hda: Use maple tree register cache

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: <18bcfcba-a6ce-4595-bd2b-4d4ba761fd58@sirena.org.uk>
Date:   Tue, 13 Jun 2023 17:41:15 +0100
From:   Mark Brown <broonie@...nel.org>
To:     Takashi Iwai <tiwai@...e.de>
Cc:     Jaroslav Kysela <perex@...ex.cz>, Takashi Iwai <tiwai@...e.com>,
        alsa-devel@...a-project.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] ALSA: hda: Use maple tree register cache

On Tue, Jun 13, 2023 at 06:15:12PM +0200, Takashi Iwai wrote:
> Mark Brown wrote:
> > On Tue, Jun 13, 2023 at 04:24:28PM +0200, Takashi Iwai wrote:

> > > Since HD-audio codec has no known default values unlike normal codecs,
> > > it needs to initialize itself only at the first access, and this
> > > helper does it.

> > Ah, if it's just suppressing the write the code should just be removed.
> > regmap_update_bits() already suppresses noop writes so unless we might
> > write a different value to the register later the effect will be the
> > same.  I can send a patch.

> Oh, I'm afraid that we're seeing different things.  The code there is
> rather to *set* some initial value for each amp register (but only
> once), and it's not about optimization for writing a same value
> again.

> That is, the function helps to set an initial (mute) value on each amp
> when the driver parses the topology and finds an amp.  But if the
> driver already has parsed this amp beforehand by other paths, it skips
> the initialization, as the other path may have already unmuted the
> amp.

> Or I might have misunderstood what you mean about _update_bits()...

So it is possible that we might set two distinct values during setup
then and we're doing this intentionally?  It's not obvious that this
might happen.  A comment wouldn't hurt, and a big part of this is
confusing is that in the non-regmap case all we're doing is suppressing
duplicate writes, in that path it's just checking for changes in the
register value.

None of this is what the non-regmap path does, it just suppresses noop
writes to the hardware.

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)