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