linux-kernel - Re: [PATCH 06/13] ASoC: tegra: Add Tegra210 based MVC driver

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]

Date:   Fri, 3 Sep 2021 19:13:26 +0100
From:   Mark Brown <broonie@...nel.org>
To:     Sameer Pujar <spujar@...dia.com>
Cc:     lgirdwood@...il.com, robh+dt@...nel.org, thierry.reding@...il.com,
        jonathanh@...dia.com, catalin.marinas@....com, will@...nel.org,
        perex@...ex.cz, tiwai@...e.com, kuninori.morimoto.gx@...esas.com,
        sharadg@...dia.com, alsa-devel@...a-project.org,
        devicetree@...r.kernel.org, linux-tegra@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 06/13] ASoC: tegra: Add Tegra210 based MVC driver

On Fri, Aug 27, 2021 at 03:03:52PM +0530, Sameer Pujar wrote:
> The Master Volume Control (MVC) provides gain or attenuation to a digital
> signal path. It can be used in input or output signal path for per-stream
> volume control or it can be used as master volume control. The MVC block
> has one input and one output. The input digital stream can be mono or
> multi-channel (up to 7.1 channels) stream. An independent mute control is
> also included in the MVC block.

Looks like it's also got a little bit of other DSP in there (a simple
EQ?).  Not that it really matters.

> +	if (reg == TEGRA210_MVC_CTRL) {
> +		u32 val;
> +		u8 mute_mask;

> +	} else {
> +		u8 chan = (reg - TEGRA210_MVC_TARGET_VOL) / REG_SIZE;
> +		s32 val = mvc->volume[chan];

It's not clear to me why we're using the same callbacks for the volume
and mute settings - there's no shared code on the read path and only a
tiny bit on the write path.

> +	err |= regmap_update_bits(mvc->regmap, TEGRA210_MVC_SWITCH,
> +			TEGRA210_MVC_VOLUME_SWITCH_MASK,
> +			TEGRA210_MVC_VOLUME_SWITCH_TRIGGER);
> +
> +end:
> +	pm_runtime_put(cmpnt->dev);
> +	return err;
> +}

_put() should return 0 if there's no change or 1 for a change.

> +	/* SW reset */
> +	regmap_write(mvc->regmap, TEGRA210_MVC_SOFT_RESET, 1);

What about all the cached values in the regmap, won't they get out of
sync?  Especially things like volume and mute, it looks like the mute
just gets written directly to the regmap and not otherwise saved.

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