linux-kernel - Re: [PATCH] ASoC: codecs: lpass-va-macro: set the default codec version for sm8250

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: <de0a9cf2-c656-430e-8c56-ca2975c73c0e@linaro.org>
Date: Fri, 16 Aug 2024 09:59:18 +0100
From: Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>, broonie@...nel.org
Cc: perex@...ex.cz, tiwai@...e.com, alsa-devel@...a-project.org,
 linux-arm-msm@...r.kernel.org, linux-sound@...r.kernel.org,
 linux-kernel@...r.kernel.org, amit.pundir@...aro.org
Subject: Re: [PATCH] ASoC: codecs: lpass-va-macro: set the default codec
 version for sm8250



On 15/08/2024 22:22, Dmitry Baryshkov wrote:
> On August 16, 2024 4:07:10 AM GMT+07:00, Dmitry Baryshkov <dmitry.baryshkov@...aro.org> wrote:
>> On August 15, 2024 11:49:03 PM GMT+07:00, srinivas.kandagatla@...aro.org wrote:
>>> From: Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
>>>
>>> sm8250 and sc7280 have lpass codec version 1.0, as these are very old
>>> platforms, they do not have a reliable way to get the codec version
>> >from core_id registers.
> 
> I wrote that it looked good, but maybe you can also describe, why core_id registers are not reliable? Are they just not present on those platforms or is there any other issue?
> 
Sure, the comment is correct because the registers are available to read 
however the values of those registers are not fit for dynamically 
detecting the version, like what we do in the driver.

one of the reasons is that the codec evolved over time, I think starting 
from v2 it has values made more sense to determine the version info from 
these registers. This is also evident in the current code.


Let me add this detail the commit log and send a v2.

--srini
> 
>>>
>>> Add the version info into of_data, so that it does not need to use
>>> core_id registers to get version number.
>>>
>>> Fixes: 378918d59181 ("ASoC: codecs: lpass-macro: add helpers to get codec version")
>>> Fixes: dbacef05898d ("ASoC: codec: lpass-rx-macro: prepare driver to accomdate new codec versions")
>>> Fixes: 727de4fbc546 ("ASoC: codecs: lpass-wsa-macro: Correct support for newer v2.5 version")
>>
>> Which commit introduced the issue? I think having just the first tag is enough.
>>
>> LGTM otherwise.
>>
>>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
>>> ---
>>> sound/soc/codecs/lpass-va-macro.c | 11 ++++++++++-
>>> 1 file changed, 10 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/sound/soc/codecs/lpass-va-macro.c b/sound/soc/codecs/lpass-va-macro.c
>>> index 8454193ed22a..e95d1f29ef18 100644
>>> --- a/sound/soc/codecs/lpass-va-macro.c
>>> +++ b/sound/soc/codecs/lpass-va-macro.c
>>> @@ -228,11 +228,13 @@ struct va_macro {
>>> struct va_macro_data {
>>> 	bool has_swr_master;
>>> 	bool has_npl_clk;
>>> +	int version;
>>> };
>>>
>>> static const struct va_macro_data sm8250_va_data = {
>>> 	.has_swr_master = false,
>>> 	.has_npl_clk = false,
>>> +	.version = LPASS_CODEC_VERSION_1_0,
>>> };
>>>
>>> static const struct va_macro_data sm8450_va_data = {
>>> @@ -1587,7 +1589,14 @@ static int va_macro_probe(struct platform_device *pdev)
>>> 			goto err_npl;
>>> 	}
>>>
>>> -	va_macro_set_lpass_codec_version(va);
>>> +	/**
>>> +	 * old version of codecs do not have a reliable way to determine the
>>> +	 * version from registers, get them from soc specific data
>>> +	 */
>>> +	if (data->version)
>>> +		lpass_macro_set_codec_version(data->version);
>>> +	else /* read version from register */
>>> +		va_macro_set_lpass_codec_version(va);
>>>
>>> 	if (va->has_swr_master) {
>>> 		/* Set default CLK div to 1 */
>>
>>
> 
>