> +struct avs_dma_data {
> + struct avs_tplg_path_template *template;
> + struct avs_path *path;
> + /*
> + * link stream is stored within substream's runtime
> + * private_data to fulfill the needs of codec BE path
> + *
> + * host stream assigned
not able to parse that comment, what are you trying to say?
> + */
> + struct hdac_ext_stream *host_stream;
> +};
> +
> +static ssize_t topology_name_read(struct file *file, char __user *user_buf, size_t count,
> + loff_t *ppos)
> +{
> + struct snd_soc_component *component = file->private_data;
> + struct snd_soc_card *card = component->card;
> + struct snd_soc_acpi_mach *mach = dev_get_platdata(card->dev);
> + char buf[64];
> + size_t len;
> +
> + len = snprintf(buf, sizeof(buf), "%s/%s\n", component->driver->topology_name_prefix,
> + mach->tplg_filename);
> +
> + return simple_read_from_buffer(user_buf, count, ppos, buf, len);
> +}
> +
> +static const struct file_operations topology_name_fops = {
> + .open = simple_open,
> + .read = topology_name_read,
> + .llseek = default_llseek,
> +};
can you clarify why this is needed?
> +
> +static int avs_component_load_libraries(struct avs_soc_component *acomp)
> +{
> + struct avs_tplg *tplg = acomp->tplg;
> + struct avs_dev *adev = to_avs_dev(acomp->base.dev);
> + int ret;
> +
> + if (!tplg->num_libs)
> + return 0;
> +
> + /* Parent device may be asleep and library loading involves IPCs. */
> + ret = pm_runtime_get_sync(adev->dev);
> + if (ret < 0 && ret != -EACCES) {
> + pm_runtime_put_noidle(adev->dev);
> + return ret;
> + }
Mark recommends the use of pm_runtime_resume_and_get(), see patches from today.
> +
> + avs_hda_clock_gating_enable(adev, false);
> + avs_hda_l1sen_enable(adev, false);
> +
> + ret = avs_dsp_load_libraries(adev, tplg->libs, tplg->num_libs);
> +
> + avs_hda_l1sen_enable(adev, true);
> + avs_hda_clock_gating_enable(adev, true);
> +
> + if (!ret)
> + ret = avs_module_info_init(adev, false);
> +
> + pm_runtime_mark_last_busy(adev->dev);
> + pm_runtime_put_autosuspend(adev->dev);
> +
> + return ret;
> +}
> +
> +static int avs_component_probe(struct snd_soc_component *component)
> +{
> + struct snd_soc_card *card = component->card;
> + struct snd_soc_acpi_mach *mach;
> + struct avs_soc_component *acomp;
> + struct avs_dev *adev;
> + char *filename;
> + int ret;
> +
> + dev_dbg(card->dev, "probing %s card %s\n", component->name, card->name);
> + mach = dev_get_platdata(card->dev);
> + acomp = to_avs_soc_component(component);
> + adev = to_avs_dev(component->dev);
> +
> + acomp->tplg = avs_tplg_new(component);
> + if (!acomp->tplg)
> + return -ENOMEM;
> +
> + if (!mach->tplg_filename)
> + goto finalize;
> +
> + /* Load specified topology and create sysfs for it. */
> + filename = kasprintf(GFP_KERNEL, "%s/%s", component->driver->topology_name_prefix,
> + mach->tplg_filename);
what is the link between topology and sysfs?
> + if (!filename)
> + return -ENOMEM;
> +
> + ret = avs_load_topology(component, filename);
> + kfree(filename);
> + if (ret < 0)
> + return ret;
> +
> + ret = avs_component_load_libraries(acomp);
> + if (ret < 0) {
> + dev_err(card->dev, "libraries loading failed: %d\n", ret);
> + goto err_load_libs;
> + }
> +
> +finalize:
> + debugfs_create_file("topology_name", 0444, component->debugfs_root, component,
> + &topology_name_fops);
that's debugfs here, is this to make it possible to select an alternate topology file? If yes, a comment earlier wouldn't hurt.
> +
> + mutex_lock(&adev->comp_list_mutex);
> + list_add_tail(&acomp->node, &adev->comp_list);
> + mutex_unlock(&adev->comp_list_mutex);
> +
> + return 0;
> +
> +err_load_libs:
> + avs_remove_topology(component);
> + return ret;
> +}
> +
> +static const struct snd_soc_component_driver avs_component_driver = {
> + .name = "avs-pcm",
> + .probe = avs_component_probe,
> + .remove = avs_component_remove,
> + .open = avs_component_open,
> + .pointer = avs_component_pointer,
> + .mmap = avs_component_mmap,
> + .pcm_construct = avs_component_construct,
> + .module_get_upon_open = 1, /* increment refcount when a pcm is opened */
> + .topology_name_prefix = "intel/avs",
is this intentional that the firmware binaries and topologies will be stored in the same intel/avs directory?
> + .non_legacy_dai_naming = true,
is this needed? we've never used this for Intel?
> +};
> +
