>>>> +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.
