[PATCH v4 03/17] ASoC: Intel: Introduce AVS driver

[Cezary Rojewski]

On 2022-03-09 10:58 PM, Pierre-Louis Bossart wrote:
>> diff --git a/sound/soc/intel/avs/Makefile b/sound/soc/intel/avs/Makefile
>> new file mode 100644
>> index 000000000000..5f7976a95fe2
>> --- /dev/null
>> +++ b/sound/soc/intel/avs/Makefile
>> @@ -0,0 +1,5 @@
>> +# SPDX-License-Identifier: GPL-2.0-only
>> +
>> +snd-soc-avs-objs := dsp.o
>> +
>> +obj-$(CONFIG_SND_SOC_INTEL_AVS) += snd-soc-avs.o
> 
> nit-pick: snd-soc-intel-avs?
> 
> avs is not a clear mapping to Intel platforms


We already have snd_soc_catpt : (

>> +/*
>> + * struct avs_dev - Intel HD-Audio driver data
>> + *
>> + * @dev: PCI device
>> + * @dsp_ba: DSP bar address
> 
> not sure it's only limited to DSP, is it?


Well, this space is classified as: "Host Device Memory Space (Audio DSP)".

>> + * @spec: platform-specific descriptor
>> + */
>> +struct avs_dev {
>> +    struct hda_bus base;
>> +    struct device *dev;
>> +
>> +    void __iomem *dsp_ba;
>> +    const struct avs_spec *spec;
>> +};
> 
>> +int avs_dsp_core_reset(struct avs_dev *adev, u32 core_mask, bool reset)
>> +{
>> +    u32 value, mask, reg;
>> +    int ret;
>> +
>> +    mask = AVS_ADSPCS_CRST_MASK(core_mask);
>> +    value = reset ? mask : 0;
>> +
>> +    snd_hdac_adsp_updatel(adev, AVS_ADSP_REG_ADSPCS, mask, value);
>> +
>> +    ret = snd_hdac_adsp_readl_poll(adev, AVS_ADSP_REG_ADSPCS,
>> +                       reg, (reg & mask) == value,
>> +                       AVS_ADSPCS_INTERVAL_US,
>> +                       AVS_ADSPCS_TIMEOUT_US);
>> +    if (ret)
>> +        dev_err(adev->dev, "core_mask %d %s reset failed: %d\n",
>> +            core_mask, reset ? "enter" : "exit", ret);
> 
> nit-pick: reset %s "entry" "exit"


I did align with what's already present in sound/hda/hdac_controller.c. 
It's done the other way around there. My personal opinion is: it sounds 
more logically the way it is currently - but I'm not a native English 
speaker.

>> +
>> +    return ret;
>> +}
>> +
>