Re: [PATCH 01/14] ASoC: Intel: avs: Account for libraries when booting basefw

[Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>]

>>>>   +int avs_dsp_load_libraries(struct avs_dev *adev, struct
>>>> avs_tplg_library *libs, u32 num_libs)
>>>> +{
>>>> +    int start, id, i = 0;
>>>> +    int ret;
>>>> +
>>>> +    /* Calculate the id to assign for the next lib. */
>>>> +    for (id = 0; id < adev->fw_cfg.max_libs_count; id++)
>>>> +        if (adev->lib_names[id][0] == '\0')
>>>> +            break;
>>>> +    if (id + num_libs >= adev->fw_cfg.max_libs_count)
>>>> +        return -EINVAL;
>>>
>>> use ida_alloc_max() ?
>>
>>
>> After reading this one couple of times I'm keen to agree that IDA
>> should have been used for library ID allocation and a at the same
>> time, surprised it has't done that already. Till now we used IDA
>> 'only' when allocating pipeline IDs and module instance IDs. Pipeline
>> allocation is good comparison here - makes use of ida_alloc_max()
>> already - library one should follow.
>>
>> This finding is much appreciated, Pierre.
> 
> I think that using ida here is a bit of an overkill. Ida works fine when
> there can be both id allocation and freeing and that's how it work with
> pipelines and modules IDs in avs. However there is no mechanism for
> unloading libraries in cAVS firmware, therefore ida would be used here
> only to increase the ID, so it needlessly complicates the code while not
> giving much of a benefit. Also our approach to check if we can load all
> libraries before the loop makes it problematic with ida because we would
> need to allocate an id at start and calculate if all libs would fit and
> then either free it instantly or complicate the loop to use id allocated
> before. Therefore I suggest to leave this code unchanged. I've synced
> with Cezary on this and provided explanation convinced him too.

That's fine, you should however capture this design decision with a
comment or a clarification in the commit message. "libraries" mean
different things to different people, and it's hard to review without
context.