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