On 2022-03-04 5:54 PM, Ranjani Sridharan wrote:
On Fri, 2022-03-04 at 15:57 +0100, Cezary Rojewski wrote:
...
+static int avs_fw_manifest_strip_verify(struct avs_dev *adev, struct firmware *fw,
const struct avs_fw_version*min) +{
- struct avs_fw_manifest *man;
- int offset, ret;
- ret = avs_fw_ext_manifest_strip(fw);
- if (ret)
return ret;- offset = avs_fw_manifest_offset(fw);
- if (offset < 0)
return offset;- if (fw->size < offset + sizeof(*man))
return -EINVAL;- if (!min)
return 0;- man = (struct avs_fw_manifest *)(fw->data + offset);
- if (man->version.major != min->major ||
man->version.minor != min->minor ||man->version.hotfix != min->hotfix ||man->version.build < min->build) {Isnt this check a bit too strict? Isnt a check major enough?
Unfortunately not. I share the similar thinking but the build system has its history and several things which should not happen, had happened. There could be _large_ API changes without any meaningful version updates at all. To prevent any unwanted behavior, this check is as strict as it can get.
dev_warn(adev->dev, "bad FW version %d.%d.%d.%d,expected %d.%d.%d.%d or newer\n",
man->version.major, man->version.minor,man->version.hotfix, man->version.build,min->major, min->minor, min->hotfix, min-build);
if (!debug_ignore_fw_version)return -EINVAL;- }
- return 0;
+}
...
+int avs_dsp_boot_firmware(struct avs_dev *adev, bool purge) +{
- int ret, i;
- /* Full boot, clear cached data except for basefw (slot 0). */
Does this mean IMR restore is only available for base FW and not for module libraries? Do I understand this correctly?
Loop below just clears the data. The new snapshot will be received once the basefw and libraries get loaded. The execution of library loading is not part of this patch anymore as it is dependent on the avs-soc-component stuff. To make things easier to review, request was to split main series into chucks. I do believe it is easier to read and review indeed.
- for (i = 1; i < adev->fw_cfg.max_libs_count; i++)
memset(adev->lib_names[i], 0, AVS_LIB_NAME_SIZE);- avs_hda_clock_gating_enable(adev, false);
- avs_hda_l1sen_enable(adev, false);
- ret = avs_dsp_load_basefw(adev);
- avs_hda_l1sen_enable(adev, true);
- avs_hda_clock_gating_enable(adev, true);
- if (ret < 0)
return ret;- /* With all code loaded, refresh module information. */
- ret = avs_module_info_init(adev, true);
It is not clear if this required only after first boot or after a suspend/resume as well.
avs_dsp_boot_firmware() and avs_dsp_first_boot_firmware() (found just below this one) are two separate functions. Only the latter has things done once. Anything else can happen several times throughout the lifetime of the avs-driver.
Regards, Czarek