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
