On 2022-03-04 7:56 PM, Pierre-Louis Bossart wrote:
Compared to SKL and KBL, younger cAVS platforms are meant to re-use
one
Younger? you mean newer?
Isn't the meaning of the two quite similar in this context?
younger doesn't sound quite right to me.
'cAVS platforms more recent that SKL and KBL...'
Sure, can reword.
Module loading handler is dummy as library and module code became
inseparable in later firmware generations.
replace dummy with "stub" maybe? lets use inclusive terms
Is 'dummy' categorized as non-inclusive? We have several usages of
'dummy' even within /sound (e.g. soc-utils.c).
Intel and plenty of other companies recommend a different wording.
mock-up, stub, placeholder, etc. see e.g.
https://developers.google.com/style/inclusive-documentation
Yes we have plenty of existing uses of 'dummy', but that doesn't mean we
should add new ones.
No problem with rewording this one in v4.
+static int
+avs_hda_init_rom(struct avs_dev *adev, unsigned int dma_id, bool
purge)
+{
+ const struct avs_spec *const spec = adev->spec;
+ unsigned int corex_mask, reg;
+ int ret;
+
+ corex_mask = spec->core_init_mask & ~AVS_MAIN_CORE_MASK;
+
+ ret = avs_dsp_op(adev, power, spec->core_init_mask, true);
+ if (ret < 0)
+ goto err;
+
+ ret = avs_dsp_op(adev, reset, AVS_MAIN_CORE_MASK, false);
+ if (ret < 0)
+ goto err;
+
+ reinit_completion(&adev->fw_ready);
+ avs_dsp_op(adev, int_control, true);
+
+ /* set boot config */
+ ret = avs_ipc_set_boot_config(adev, dma_id, purge);
+ if (ret) {
+ ret = AVS_IPC_RET(ret);
+ goto err;
+ }
+
+ /* await ROM init */
+ ret = snd_hdac_adsp_readq_poll(adev, spec->rom_status, reg,
+ (reg & 0xF) == AVS_ROM_INIT_DONE
||
+ (reg & 0xF) ==
APL_ROM_FW_ENTERED,
+ AVS_ROM_INIT_POLLING_US,
APL_ROM_INIT_TIMEOUT_US);
+ if (ret < 0) {
+ dev_err(adev->dev, "rom init timeout: %d\n", ret);
+ goto err;
+ }
+
+ /* power down non-main cores */
+ if (corex_mask)
+ avs_dsp_op(adev, power, corex_mask, false);
What if this fails?
We are still in happy path, worst thing could happen here is increased
power consumption.
'happy path'?
The question is whether we value user experience over error-checking
this case. I've never seen this step failing and the step itself is
quite.. extraordinary : ) It's also invoked on very limited number of
cAVS platforms, for everything else it's just: 'return 0'.
Regards,
Czarek
