Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
From:   Guillermo Rodriguez Garcia <guille.rodriguez@gmail.com>
Date:   Thu, 24 Nov 2022 10:57:34 +0100
Message-ID: <CABDcavYdsk-O4x3oPX4i4+T5wsoZV26_kpEq6JvpD8A_cAGHxg@mail.gmail.com>
Subject: SOC_DOUBLE_R_SX_TLV controls broken in cs24l51 driver
To:     linux-kernel@vger.kernel.org
Cc:     =?UTF-8?Q?Tan_Nay=C4=B1r?= <tannayir@gmail.com>,
        Srinivas Kandagatla <srinivas.kandagatla@linaro.org>,
        Banajit Goswami <bgoswami@codeaurora.org>,
        Richard Fitzgerald <rf@opensource.cirrus.com>,
        Lucas Tanure <tanureal@opensource.cirrus.com>,
        David Rhodes <david.rhodes@cirrus.com>,
        James Schulman <james.schulman@cirrus.com>,
        Charles Keepax <ckeepax@opensource.cirrus.com>,
        patches@opensource.cirrus.com, alsa-devel@alsa-project.org,
        Mark Brown <broonie@kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi all,

I am using a dev board with a Cirrus Logic cs24l51 codec.

This used to work fine prior to kernel version 5.x, however after 5.x
it is not possible to set certain values for ALSA controls from
userspace.

I believe this is related to the input validation that is mentioned in
this thread: https://lore.kernel.org/all/Yph8C3bRxcr6ogW7@sirena.org.uk/T/,
and possibly in this commit: 4f1e50d6a9cf9c1b8c859d449b5031cacfa8404e
("ASoC: ops: Reject out of bounds values in snd_soc_put_volsw_sx()")

For the cs24l51, all the controls that fail are using the
SOC_DOUBLE_R_SX_TLV macro.

I have traced this to the checks in snd_soc_put_volsw_sx, specifically
the (val > max - min) check:

pr_warn("Max: %d, Min: %d, Value: %d", max, min, val);
pr_warn("platform_max: %d", mc->platform_max);
if (mc->platform_max && val > mc->platform_max)
{
    return -EINVAL;
}
if (val > max - min){
    pr_warn("(val > max - min) check failed");
    return -EINVAL;
}
if (val < 0)
    return -EINVAL;

According to the datasheet of the CS24L51, section 6.13, page 61, the
PCMMIXX_VOL registers accept the following range of values:

Binary Code / Volume Setting
001 1000 / +12.0 dB
=C2=B7=C2=B7=C2=B7 =C2=B7=C2=B7=C2=B7
000 0000 / 0 dB
111 1111 / -0.5 dB
111 1110 / -1.0 dB
=C2=B7=C2=B7=C2=B7 =C2=B7=C2=B7=C2=B7
001 1001 / -51.5 dB

Minimum value is 0x19 (001 1001) corresponding to -51.5 dB, then there
are 127 possible gain settings from -51.5 dB to +12.0 dB, in 0.5 dB
steps.

This is declared in the driver as follows:

SOC_DOUBLE_R_SX_TLV("PCM Playback Volume",
CS42L51_PCMA_VOL, CS42L51_PCMB_VOL,
0, 0x19, 0x7F, adc_pcm_tlv),

0x19 =3D min value
0x7F =3D number of gain settings

This seems to be correct according to the semantics of the
SOC_DOUBLE_R_SX_TLV macro as described in commit
34198710f55b5f359f43e67d9a08fe5aadfbca1b ("ASoC: Add info callback for
SX_TLV controls").

However, the (val > max - min) check in snd_soc_put_volsw_sx fails in
the above example because val =3D 127, max - min =3D 127 - 25 =3D 102.

So I am not sure how this should be fixed. Is the SX_TLV macro being
used incorrectly here? Is the check in snd_soc_put_volsw_sx wrong?

Any pointers are welcome.

Thanks,

(If possible, please CC me in any replies)

Guillermo Rodriguez Garcia
guille.rodriguez@gmail.com