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;
Message-ID: <1d0bd2ec-ce27-02fa-b538-4636ca9ac289@collabora.com>
Date:   Fri, 26 Aug 2022 01:12:17 +0300
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.12.0
Subject: Re: [PATCH] ASoC: wm_adsp: Silent parsing error on loading speaker
 protection fw
Content-Language: en-US
To:     Charles Keepax <ckeepax@opensource.cirrus.com>
Cc:     Liam Girdwood <lgirdwood@gmail.com>,
        Mark Brown <broonie@kernel.org>,
        Jaroslav Kysela <perex@perex.cz>,
        Takashi Iwai <tiwai@suse.com>,
        Vlad Karpovich <Vlad.Karpovich@cirrus.com>,
        patches@opensource.cirrus.com, alsa-devel@alsa-project.org,
        linux-kernel@vger.kernel.org, kernel@collabora.com
References: <20220823133347.919706-1-cristian.ciocaltea@collabora.com>
 <20220825124715.GM92394@ediswmail.ad.cirrus.com>
From:   Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
In-Reply-To: <20220825124715.GM92394@ediswmail.ad.cirrus.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 8/25/22 15:47, Charles Keepax wrote:
> On Tue, Aug 23, 2022 at 04:33:47PM +0300, Cristian Ciocaltea wrote:
>> The tracing capabilities for the speaker protection fw enabled via
>> commit c55b3e46cb99 ("ASoC: wm_adsp: Add trace caps to speaker
>> protection FW") are not be available on all platforms, such as the
>> Valve's Steam Deck which is based on the Halo Core DSP.
>>
>> As a consequence, whenever the firmware is loaded, a rather misleading
>> 'Failed to parse legacy: -19' error message is written to the kernel
>> ring buffer:
>>
>> [  288.977412] steamdeck kernel: cs35l41 spi-VLV1776:01: DSP1: Firmware version: 3
>> [  288.978002] steamdeck kernel: cs35l41 spi-VLV1776:01: DSP1: cs35l41-dsp1-spk-prot.wmfw: Fri 02 Apr 2021 21:03:50 W. Europe Daylight Time
>> [  289.094065] steamdeck kernel: cs35l41 spi-VLV1776:01: DSP1: Firmware: 400a4 vendor: 0x2 v0.33.0, 2 algorithms
>> [  289.095073] steamdeck kernel: cs35l41 spi-VLV1776:01: DSP1: 0: ID cd v29.53.0 XM@94 YM@e
>> [  289.095665] steamdeck kernel: cs35l41 spi-VLV1776:01: DSP1: 1: ID f20b v0.0.1 XM@170 YM@0
>> [  289.096275] steamdeck kernel: cs35l41 spi-VLV1776:01: DSP1: Protection: C:\Users\ocanavan\Desktop\cirrusTune_july2021.bin
>> [  291.172383] steamdeck kernel: cs35l41 spi-VLV1776:01: DSP1: Failed to parse legacy: -19
>>
>> Update wm_adsp_buffer_init() to *not* report the ENODEV error when the
>> firmware type is WM_ADSP_FW_SPK_PROT.
>>
>> Fixes: c55b3e46cb99 ("ASoC: wm_adsp: Add trace caps to speaker protection FW")
>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
>> ---
>>   sound/soc/codecs/wm_adsp.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/sound/soc/codecs/wm_adsp.c b/sound/soc/codecs/wm_adsp.c
>> index cfaa45ede916..7514fc03b468 100644
>> --- a/sound/soc/codecs/wm_adsp.c
>> +++ b/sound/soc/codecs/wm_adsp.c
>> @@ -1602,7 +1602,7 @@ static int wm_adsp_buffer_init(struct wm_adsp *dsp)
>>   	if (list_empty(&dsp->buffer_list)) {
>>   		/* Fall back to legacy support */
>>   		ret = wm_adsp_buffer_parse_legacy(dsp);
>> -		if (ret)
>> +		if (ret && (dsp->fw != WM_ADSP_FW_SPK_PROT || ret != -ENODEV))
>>   			adsp_warn(dsp, "Failed to parse legacy: %d\n", ret);
> 
> Fixing this for a single firmware probably doesn't really make
> the most sense, if we are treating buffers as optional these days
> I guess really the best solution would be to make this either an
> info and slightly rephrase the message or make it a dbg message.

Indeed, I have just submitted v2 for a more generic handling of the issue:

https://lore.kernel.org/all/20220825220530.1205141-1-cristian.ciocaltea@collabora.com/

Thanks for reviewing,
Cristian

> Thanks,
> Charles