Re: [alsa-devel] [PATCH v4 4/4] ASoC: samsung: Add machine driver for Exynos5433 based TM2 board

Hi Sylwester,

[snip]

+static int tm2_probe(struct platform_device *pdev) +{

• struct device *dev = &pdev->dev;
• struct snd_soc_card *card = &tm2_card;
• struct tm2_machine_priv *priv;
• struct device_node *cpu_dai_node, *codec_dai_node;
• int ret, i;
• if (!dev->of_node) {

• dev_err(dev, "DT node is missing\n");
  

• return -ENODEV;
  

• }
• priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
• if (!priv)

• return -ENOMEM;
  

• snd_soc_card_set_drvdata(card, priv);
• card->dev = dev;
• priv->gpio_mic_bias = devm_gpiod_get(dev, "mic-bias",

• 				GPIOF_OUT_INIT_LOW);
  

• if (IS_ERR(priv->gpio_mic_bias)) {

• dev_err(dev, "Failed to get mic bias gpio\n");
  

• return PTR_ERR(priv->gpio_mic_bias);
  

• }
• ret = snd_soc_of_parse_card_name(card, "model");
• if (ret < 0) {

• dev_err(dev, "Card name is not specified\n");
  

• return ret;
  

• }
• ret = snd_soc_of_parse_audio_routing(card, "samsung,audio-routing");
• if (ret < 0) {

• dev_err(dev, "Audio routing is not specified or invalid\n");
  

• return ret;
  

• }
• card->aux_dev[0].codec_of_node = of_parse_phandle(dev->of_node,

• 					"audio-amplifier", 0);
  

• if (!card->aux_dev[0].codec_of_node) {

• dev_err(dev, "audio-amplifier property invalid or missing\n");
  

• return -EINVAL;
  

• }
• cpu_dai_node = of_parse_phandle(dev->of_node, "i2s-controller", 0);
• if (!cpu_dai_node) {

• dev_err(dev, "i2s-controllers property invalid or missing\n");
  

• ret = -EINVAL;
  

• goto err_put_amp;
  

• }
• codec_dai_node = of_parse_phandle(dev->of_node, "audio-codec", 0);
• if (!codec_dai_node) {

• dev_err(dev, "audio-codec property invalid or missing\n");
  

• ret = -EINVAL;
  

• goto err_put_cpu_dai;
  

• }
• for (i = 0; i < card->num_links; i++) {

• card->dai_link[i].cpu_dai_name = NULL;
  

• card->dai_link[i].cpu_name = NULL;
  

• card->dai_link[i].platform_name = NULL;
  

• card->dai_link[i].codec_of_node = codec_dai_node;
  

• card->dai_link[i].cpu_of_node = cpu_dai_node;
  

• card->dai_link[i].platform_of_node = cpu_dai_node;
  

• }
• priv->codec_mclk1 = of_clk_get_by_name(codec_dai_node, "mclk1");
• if (IS_ERR(priv->codec_mclk1)) {

• dev_err(dev, "Failed to get mclk1 clock\n");
  

• ret = PTR_ERR(priv->codec_mclk1);
  

• goto err_put_codec_dai;
  

• }

I think that you better to use the devm_clk_get() instead of of_clk_get_by_name() because you don't need to handle the clk_put() when error happen and remove the this driver.

priv->codec_mclk1 = devm_clk_get(dev, "mclk1");

• /* mclk2 is optional */
• priv->codec_mclk2 = of_clk_get_by_name(codec_dai_node, "mclk2");
• if (IS_ERR(priv->codec_mclk2))

• dev_info(dev, "Not using mclk2 clock\n");
  

priv->codec_mclk2 = devm_clk_get(dev, "mclk2");

• ret = devm_snd_soc_register_component(dev, &tm2_component,

• 		tm2_ext_dai, ARRAY_SIZE(tm2_ext_dai));
  

• if (ret < 0) {

• dev_err(dev, "Failed to register component: %d\n", ret);
  

• goto err_put_mclk;
  

• }
• ret = devm_snd_soc_register_card(dev, card);
• if (ret < 0) {

• dev_err(dev, "Failed to register card: %d\n", ret);
  

• goto err_put_mclk;
  

• }
• return 0;

+err_put_mclk:

• clk_put(priv->codec_mclk1);
• if (!IS_ERR(priv->codec_mclk2))

• clk_put(priv->codec_mclk2);
  

If using the devm_clk_get(), clk_put() is not needed

+err_put_codec_dai:

• of_node_put(codec_dai_node);

+err_put_cpu_dai:

• of_node_put(cpu_dai_node);

+err_put_amp:

• of_node_put(card->aux_dev[0].codec_of_node);
• return ret;

+}

+static int tm2_remove(struct platform_device *pdev) +{

• struct snd_soc_card *card = &tm2_card;
• struct tm2_machine_priv *priv = snd_soc_card_get_drvdata(card);
• clk_put(priv->codec_mclk1);
• if (!IS_ERR(priv->codec_mclk2))

• clk_put(priv->codec_mclk2);
  

ditto.

• of_node_put(card->dai_link[0].codec_of_node);
• of_node_put(card->dai_link[0].cpu_of_node);
• of_node_put(card->aux_dev[0].codec_of_node);
• return 0;

+}

+static const struct of_device_id tm2_of_match[] = {

• { .compatible = "samsung,tm2-audio" },
• { },

+}; +MODULE_DEVICE_TABLE(of, tm2_of_match);

+static struct platform_driver tm2_driver = {

• .driver = {

• .name		= "tm2-audio",
  

• .pm		= &snd_soc_pm_ops,
  

• .of_match_table	= tm2_of_match,
  

• },
• .probe = tm2_probe,
• .remove = tm2_remove,

+};

+module_platform_driver(tm2_driver);

+MODULE_AUTHOR("Inha Song ideal.song@samsung.com"); +MODULE_DESCRIPTION("ALSA SoC Exynos TM2 Audio Support"); +MODULE_LICENSE("GPL v2");

Thanks, Chanwoo Choi