linux-kernel - Re: [PATCH v3 3/4] ALSA: cs35l41: Add shared boost feature

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] [day] [month] [year] [list]

Message-ID: <20230208175034.GA12852@ediswmail.ad.cirrus.com>
Date:   Wed, 8 Feb 2023 17:50:34 +0000
From:   Charles Keepax <ckeepax@...nsource.cirrus.com>
To:     Lucas Tanure <lucas.tanure@...labora.com>
CC:     David Rhodes <david.rhodes@...rus.com>,
        Liam Girdwood <lgirdwood@...il.com>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Mark Brown <broonie@...nel.org>,
        "Rob Herring" <robh+dt@...nel.org>,
        Jaroslav Kysela <perex@...ex.cz>,
        Takashi Iwai <tiwai@...e.com>, <alsa-devel@...a-project.org>,
        <devicetree@...r.kernel.org>, <patches@...nsource.cirrus.com>,
        <linux-kernel@...r.kernel.org>, <kernel@...labora.com>
Subject: Re: [PATCH v3 3/4] ALSA: cs35l41: Add shared boost feature

On Wed, Feb 08, 2023 at 02:18:38PM +0000, Lucas Tanure wrote:
> Shared boost allows two amplifiers to share a single boost circuit by
> communicating on the MDSYNC bus.
> The passive amplifier does not control the boost and receives data from
> the active amplifier.
> 
> Shared Boost is not supported in HDA Systems.
> Based on David Rhodes shared boost patches.
> 
> Signed-off-by: Lucas Tanure <lucas.tanure@...labora.com>
> ---
> +		ret = wait_for_completion_timeout(pll_lock, msecs_to_jiffies(1000));
> +		if (ret == 0) {
> +			ret = -ETIMEDOUT;
> +		} else {
> +			regmap_read(regmap, CS35L41_PWR_CTRL3, &pwr_ctrl3);
> +			pwr_ctrl3 |= CS35L41_SYNC_EN_MASK;
> +			struct reg_sequence cs35l41_mdsync_up_seq[] = {
> +				{CS35L41_PWR_CTRL3,	pwr_ctrl3},
> +				{CS35L41_PWR_CTRL1,	0x00000000, 3000},
> +				{CS35L41_PWR_CTRL1,	0x00000001, 3000},
> +			};

I think this looks good as long as turning the DRE off wasn't an
important part of the sequence and was just accidentally
happening. Probably on David to confirm that one.

Although should really move the variable definition to the top of
the code block and put a blank line after it, to pass checkpatch.

Thanks,
Charles

> +			ret = regmap_multi_reg_write(cs35l45->regmap, cs35l41_mdsync_up_seq,
> +						     ARRAY_SIZE(cs35l41_mdsync_up_seq));