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: <87a5x3cp9r.wl-tiwai@suse.de>
Date:   Tue, 13 Jun 2023 18:15:12 +0200
From:   Takashi Iwai <tiwai@...e.de>
To:     Mark Brown <broonie@...nel.org>
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, 13 Jun 2023 17:49:41 +0200,
Mark Brown wrote:
> 
> On Tue, Jun 13, 2023 at 04:24:28PM +0200, Takashi Iwai wrote:
> > Mark Brown wrote:
> 
> > > BTW I was just looking at reg_raw_update_once() and I can't figure out
> > > why it's trying to do what it's doing - it does a read to check if it's
> > > seen the register before and then does an _update_bits() if the register
> > > hasn't been cached yet, apparently trying suppress duplicate writes but
> > > possibly deliberately discarding changes to multiple bitfields in the
> > > same register.  That's not what the non-regmap path does, it'll only
> > > discard noop changes to the same bitfield.
> 
> > Yes, it's a quite hackish way of optimization of the initialization.
> 
> > 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()...


thanks,

Takashi