Re: [PATCH v3 10/13] ASoC: tegra: Add audio graph based card driver

...

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?