[alsa-devel] [PATCH 04/35] ASoC: Intel: Skylake: Unify firmware loading mechanism

[Pierre-Louis Bossart]

On 8/24/19 4:34 AM, Cezary Rojewski wrote:
> On 2019-08-23 20:40, Pierre-Louis Bossart wrote:
>>
>>> -int skl_sst_init_fw(struct device *dev, struct skl_dev *skl)
>>> +int skl_sst_init_fw(struct skl_dev *skl)
>>>   {
>>> -    int ret;
>>>       struct sst_dsp *sst = skl->dsp;
>>> +    struct device *dev = skl->dev;
>>> +    int (*lp_check)(struct sst_dsp *dsp, bool state);
>>> +    int ret;
>>> +
>>> +    lp_check = skl->ipc.ops.check_dsp_lp_on;
>>> +    skl->enable_miscbdcge(dev, false);
>>> +    skl->clock_power_gating(dev, false);
>>>       ret = sst->fw_ops.load_fw(sst);
>>>       if (ret < 0) {
>>>           dev_err(dev, "Load base fw failed : %d\n", ret);
>>> -        return ret;
>>> +        goto exit;
>>> +    }
>>> +
>>> +    if (!skl->is_first_boot)
>>> +        goto library_load;
>>> +    /* Disable power check during cfg setup */
>>> +    skl->ipc.ops.check_dsp_lp_on = NULL;
>>
>> It's very odd to play with .ops callback dynamically. Usually ops are 
>> constant, and if you want to disable them you add a flag.
>>
> 
> Yeye, keen eye! Can't do everything at once though :/
> The power check is APL+ specific and should not be part of generic ipc 
> framework at all (found in /sound/soc/intel/common/sst-ipc.c). Different 
> fate awaits said check. For now, in this single case it seems best to 
> simply disable the check and reapply it once setup is done.

What's the difference with having this callback do nothing for APL-?

> 
>>> +
>>> +    ret = skl_ipc_fw_cfg_get(&skl->ipc, &skl->fw_cfg);
>>> +    if (ret < 0) {
>>> +        dev_err(dev, "Failed to get fw cfg: %d\n", ret);
>>> +        goto exit;
>>> +    }
>>> +
>>> +    ret = skl_ipc_hw_cfg_get(&skl->ipc, &skl->hw_cfg);
>>> +    if (ret < 0) {
>>> +        dev_err(dev, "Failed to get hw cfg: %d\n", ret);
>>> +        goto exit;
>>>       }
>>>       skl_dsp_init_core_state(sst);
>>> +library_load:
>>>       if (skl->lib_count > 1) {
>>>           ret = sst->fw_ops.load_library(sst, skl->lib_info,
>>>                           skl->lib_count);
>>>           if (ret < 0) {
>>> -            dev_err(dev, "Load Library failed : %x\n", ret);
>>> -            return ret;
>>> +            dev_err(dev, "Load library failed : %x\n", ret);
>>> +            goto exit;
>>>           }
>>>       }
>>> +
>>>       skl->is_first_boot = false;
>>> +exit:
>>> +    skl->ipc.ops.check_dsp_lp_on = lp_check;
>>> +    skl->enable_miscbdcge(dev, true);
>>> +    skl->clock_power_gating(dev, true);
>>> -    return 0;
>>> +    return ret;
>>>   }
>>>   EXPORT_SYMBOL_GPL(skl_sst_init_fw);