From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751997AbcGUK2o (ORCPT ); Thu, 21 Jul 2016 06:28:44 -0400 Received: from mailout3.samsung.com ([203.254.224.33]:49376 "EHLO mailout3.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751346AbcGUK2l convert rfc822-to-8bit (ORCPT ); Thu, 21 Jul 2016 06:28:41 -0400 X-AuditID: cbfee68d-f79876d000001436-37-5790a3d09d93 MIME-version: 1.0 Content-type: text/plain; charset=UTF-8 Content-transfer-encoding: 8BIT Subject: Re: [PATCH v4 4/4] ASoC: samsung: Add machine driver for Exynos5433 based TM2 board To: Sylwester Nawrocki References: <1467738877-31555-1-git-send-email-s.nawrocki@samsung.com> <5788721B.7010602@samsung.com> <578CB261.60507@samsung.com> Cc: broonie@kernel.org, robh@kernel.org, alsa-devel@alsa-project.org, devicetree@vger.kernel.org, ideal.song@samsung.com, inki.dae@samsung.com, b.zolnierkie@samsung.com, linux-samsung-soc@vger.kernel.org, linux-kernel@vger.kernel.org, Krzysztof Kozlowski From: Chanwoo Choi Message-id: <5790A3D0.4070500@samsung.com> Date: Thu, 21 Jul 2016 19:28:32 +0900 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 In-reply-to: <578CB261.60507@samsung.com> X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrBIsWRmVeSWpSXmKPExsWyRsSkUPfC4gnhBtd/KFhcuXiIyWLjjPWs FlMfPmGzmH/kHKvFrr/3GS0m3Z/AYvH6haHF5V1z2CxmnN/HZPF/zw52i8Nv2lkduD02fG5i 89i0qpPNo2/LKkaPz5vkAliiuGxSUnMyy1KL9O0SuDLaPhxhKrghVfFj7WL2BsZ2kS5GTg4J AROJzzt+MkPYYhIX7q1n62Lk4hASWMEo0d+5E8jhACu6s64WIj6LUWLugWY2kAZeAUGJH5Pv sYDYzALqEpPmLWIGqWcWEJH4cpcDIqwtsWzha7D5QgIPGCWe3owAsYUF4iTu7frIDmKLCOhL LFl1EWrvLUaJtxNWsYI4zALzmCS2/z8MtoxNQEti/4sbUIu1JLZe2Aa2mEVAVWLb9wVgk0QF IiROnX0LVsMpoClxvO0eO8ggCYG37BJT33yGahCQ+Db5EAvEZ7ISmw5AfS8pcXDFDZYJjOKz kPw2C8lvsxB+m4XktwWMLKsYRVMLkguKk9KLDPWKE3OLS/PS9ZLzczcxAiP49L9nvTsYbx+w PsQowMGoxMO74kV/uBBrYllxZe4hRlOggyYyS4km5wPTRF5JvKGxmZGFqYmpsZG5pZmSOK+i 1M9gIYH0xJLU7NTUgtSi+KLSnNTiQ4xMHJxSDYzFDyTZb67WqFxwLoLJ/FrHz0urXrwrO5J/ SXTfgfp6/fuXPObKalouK1xS1ho+I94n6YjKEnEWm6N/Dqvxvs6cmF/as7Kh6tDD/VJRbEut 5nEHTJjS91j44EGVeS/un709Xd75daFMenSHbHbovpuRlTxOfMl27e7nHq7J26zOwdxj8i/9 UqUSS3FGoqEWc1FxIgADj+tI2wIAAA== X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFvrDIsWRmVeSWpSXmKPExsVy+t9jQd0LiyeEG+z5LWVx5eIhJouNM9az Wkx9+ITNYv6Rc6wWu/7eZ7SYdH8Ci8XrF4YWl3fNYbOYcX4fk8X/PTvYLQ6/aWd14PbY8LmJ zWPTqk42j74tqxg9Pm+SC2CJamC0yUhNTEktUkjNS85PycxLt1XyDo53jjc1MzDUNbS0MFdS yEvMTbVVcvEJ0HXLzAG6SkmhLDGnFCgUkFhcrKRvh2lCaIibrgVMY4Sub0gQXI+RARpIWMOY cWjDAvaCFVIVc/+UNTBeF+5i5OCQEDCRuLOutouRE8gUk7hwbz1bFyMXh5DALEaJuQea2UAS vAKCEj8m32MBqWcWkJc4cikbJMwsoC4xad4iZhBbSOABo8TTmxEgtrBAnMS9XR/ZQWwRAX2J JasuQs28xSjxdsIqVhCHWWAek8T2/4fBFrAJaEnsf3EDapmWxNYL21hAbBYBVYlt3xeATRIV iJA4dfYtWA2ngKbE8bZ77BMYgc5EuG8Wwn2zkNy3gJF5FaNEakFyQXFSeq5hXmq5XnFibnFp Xrpecn7uJkZwzD+T2sF4cJf7IUYBDkYlHl4F9gnhQqyJZcWVuYcYJTiYlUR47RYChXhTEiur Uovy44tKc1KLDzGaAt04kVlKNDkfmI7ySuINjU3MjCyNzA0tjIzNlcR5H/9fFyYkkJ5Ykpqd mlqQWgTTx8TBKdXA6P9YS/Gq8HZ1PhP1dcV+qurm9b4b32SHR2TYqks26T475l13RqZQPTFp 0s+zvHed935b3cFf69423cQrrePQDC0t3bbyANUfb56wy383tmA5O3XmX+NJno9fPHrLv6l0 rYvdac8TjUqm2dI3JQSjvI/vDZ2wQq1S+tv+pZaG808dt/g75d9hJZbijERDLeai4kQA07l3 7A8DAAA= DLP-Filter: Pass X-MTR: 20000000000000000@CPGS X-CFilter-Loop: Reflected Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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