[alsa-devel] [PATCH v4 2/2] ASoC: Intel: Skylake: large_config_get overhaul

[Pierre-Louis Bossart]

On 8/8/19 11:52 AM, Cezary Rojewski wrote:
> On 2019-08-08 16:50, Pierre-Louis Bossart wrote:
>> Thanks Cezary, the split makes it much easier to review. I have a 
>> couple of minor comments below, looks good otherwise.
>>
>> On 8/8/19 5:24 AM, Cezary Rojewski wrote:
>>> LARGE_CONFIG_GET is mainly used to retrieve requested module parameters
>>> but it may also carry TX payload with them. Update its implementation to
>>> account for both TX and RX data.
>>> First reply.header carries total payload size within data_off_sizefield.
>>> Make use of reply.header to realloc returned buffer with correct size.
>>>
>>> Failure of IPC request is permissive - error-payload may be returned, an
>>> informative data why GET for given param failed - and thus function
>>> should not collapse before entire processing is finished. Caller is
>>> responsible for checking returned payload and bytes parameters.
>>
>> but that is the same as before, yes? this patch does not change the 
>> behavior on errors?
> 
> To evaluate this statement you have to take a look at old code (before 
> addition of reply saving):
> 
> ret = sst_ipc_tx_message_wait(ipc, *ipc_header, NULL, 0,
>                ((char *)param) + data_offset,
>                msg->param_data_size);
> if (ret < 0) {
>      dev_err(ipc->dev,
>          "ipc: get large config fail, err: %d\n", ret);
>      return ret;
> }
> 
> Third and four parameter for sst_ipc_tx_message_wait denoted tx_data and 
> tx_size in older version. As you can see, these are empty - no payload 
> is carried, _get_ only awaits reply and copied data retrieved (breaks 
> vendor case as described earlier).
> 
> Immediately after failure, function was collapsing. Some data might have 
> been appended to "param" but size is unknown. However, due to lack of 
> retrieval of said size, indeed one may say the behavior on errors did 
> not really change. It's vital to highlight that changes added here (2/2 
> patch) do not make function return right after receiving error though - 
> buffer and size are updated for the caller before leaving the scope.

ok, got it. The change actually happened in patch1, I couldn't see the 
change in that specific patch.
Thanks for the precisions.