Messages in this thread | | | Subject | Re: [PATCH v3 10/13] ASoC: tegra: Add audio graph based card driver | From | Sameer Pujar <> | Date | Fri, 2 Oct 2020 14:38:24 +0530 |
| |
>> Add Tegra audio machine driver which is based on generic audio graph card >> driver. It re-uses most of the common stuff from audio graph driver and >> uses the same DT binding. Required Tegra specific customizations are done >> in the driver. > (snip) >> +static const struct snd_soc_ops tegra_audio_graph_ops = { >> + .startup = asoc_simple_startup, >> + .shutdown = asoc_simple_shutdown, >> + .hw_params = tegra_audio_graph_hw_params, >> +}; > This is just an idea, > but can we use hooks here somehow ? > > .ops_hook_pre > .ops_hook_func > .ops_hook_post > > if (priv->ops_hook_pre->func) > priv->ops_hook_pre->func_pre(...); > > if (priv->ops_hook_func->func) > priv->ops_hook_func->func(...); /* driver's function */ > else > graph_func(...); /* audio-graph function */ > > if (priv->ops_hook_post->func) > priv->ops_hook_post->func(...);
Right now I just required to populate some flags or structures and do not have any specific pre()/post() functions to be called. Can this be reserved for later?
> > >> +static int tegra_audio_graph_probe(struct platform_device *pdev) >> +{ >> + struct asoc_simple_priv *priv; >> + struct device *dev = &pdev->dev; >> + struct snd_soc_card *card; >> + struct link_info li; >> + int err; >> + >> + /* Allocate the private data and the DAI link array */ >> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); >> + if (!priv) >> + return -ENOMEM; >> + >> + priv->data = (struct tegra_audio_graph_data *) >> + devm_kzalloc(dev, sizeof(*priv->data), GFP_KERNEL); >> + if (!priv->data) >> + return -ENOMEM; >> + >> + card = simple_priv_to_card(priv); >> + >> + card->owner = THIS_MODULE; >> + card->dev = dev; >> + card->component_chaining = true; >> + card->probe = tegra_audio_graph_card_probe; >> + >> + priv->ops = &tegra_audio_graph_ops; >> + priv->force_dpcm = 1; >> + >> + memset(&li, 0, sizeof(li)); >> + graph_get_dais_count(priv, &li); >> + if (!li.link || !li.dais) >> + return -EINVAL; >> + >> + err = asoc_simple_init_priv(priv, &li); >> + if (err < 0) >> + return err; >> + >> + err = graph_parse_of(priv); >> + if (err < 0) { >> + if (err != -EPROBE_DEFER) >> + dev_err(dev, "Parse error %d\n", err); >> + goto cleanup; >> + } >> + >> + snd_soc_card_set_drvdata(card, priv); >> + >> + asoc_simple_debug_info(priv); >> + >> + err = devm_snd_soc_register_card(dev, card); >> + if (err < 0) >> + goto cleanup; >> + >> + return 0; >> + >> +cleanup: >> + asoc_simple_clean_reference(card); >> + >> + return err; >> +}
> These are almost same as graph_probe(). > Maybe we can separate graph_probe() and export function ?
Yes possible, I can move more stuff into graph_parse_of() which is already an exported function in the current series. This can be utilized by both generic audio graph and Tegra audio graph.
Something like below,
static int tegra_audio_graph_probe(struct platform_device *pdev) { struct tegra_audio_priv *priv; struct device *dev = &pdev->dev; struct snd_soc_card *card;
priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); if (!priv) return -ENOMEM;
card = simple_priv_to_card(&priv->simple);
card->owner = THIS_MODULE; card->dev = dev; card->probe = tegra_audio_graph_card_probe;
/* graph_parse_of() depends on below */ card->component_chaining = 1; priv->simple.ops = &tegra_audio_graph_ops; priv->simple.force_dpcm = 1;
return graph_parse_of(&priv->simple); }
Does this sound fine?
| |