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

[Cezary Rojewski]

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.

>> +
>> +    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);