Hi Sylwester, On 2016년 07월 18일 19:41, Sylwester Nawrocki wrote: > Hi Chanwoo, > > On 07/15/2016 07:18 AM, Chanwoo Choi wrote: >>> +static int tm2_probe(struct platform_device *pdev) >>> +{ >>> + struct device *dev = &pdev->dev; > >>> + 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; >>> + } > >>> + 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"); > > The clocks are from the CODEC DT node, for which we don't have struct > device pointer here, that's why I used of_clk_get_by_name(). When I test it, I can get the clock pointer by devm_clk_get() as following: diff --git a/sound/soc/samsung/tm2_wm5110.c b/sound/soc/samsung/tm2_wm5110.c index 9728b3c5927f..5de4fc554aec 100644 --- a/sound/soc/samsung/tm2_wm5110.c +++ b/sound/soc/samsung/tm2_wm5110.c @@ -500,7 +500,7 @@ static int tm2_probe(struct platform_device *pdev) card->dai_link[i].platform_of_node = cpu_dai_node; } - priv->codec_mclk1 = of_clk_get_by_name(codec_dai_node, "mclk1"); + priv->codec_mclk1 = devm_clk_get(dev, "mclk1"); if (IS_ERR(priv->codec_mclk1)) { dev_err(dev, "Failed to get mclk1 clock\n"); ret = PTR_ERR(priv->codec_mclk1); @@ -508,7 +508,7 @@ static int tm2_probe(struct platform_device *pdev) } /* mclk2 is optional */ - priv->codec_mclk2 = of_clk_get_by_name(codec_dai_node, "mclk2"); + priv->codec_mclk2 = devm_clk_get(dev, "mclk2"); if (IS_ERR(priv->codec_mclk2)) dev_info(dev, "Not using mclk2 clock\n"); @@ -516,21 +516,17 @@ static int tm2_probe(struct platform_device *pdev) 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; + goto err_put_codec_dai; } 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; + goto err_put_codec_dai; } return 0; -err_put_mclk: - clk_put(priv->codec_mclk1); - if (!IS_ERR(priv->codec_mclk2)) - clk_put(priv->codec_mclk2); err_put_codec_dai: of_node_put(codec_dai_node); err_put_cpu_dai: @@ -543,11 +539,6 @@ err_put_amp: 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); Regards, Chanwoo Choi -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html