From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933619AbcCOC1J (ORCPT ); Mon, 14 Mar 2016 22:27:09 -0400 Received: from mailout1.w1.samsung.com ([210.118.77.11]:65162 "EHLO mailout1.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750912AbcCOC1H (ORCPT ); Mon, 14 Mar 2016 22:27:07 -0400 X-AuditID: cbfec7f5-f79b16d000005389-54-56e772f602be Subject: Re: [PATCH v3] rtc: s3c: Don't print an error on probe deferral To: Javier Martinez Canillas , linux-kernel@vger.kernel.org References: <1458005918-7893-1-git-send-email-javier@osg.samsung.com> <56E76A57.7070104@samsung.com> <56E76C6F.5020803@osg.samsung.com> Cc: Joe Perches , Alexandre Belloni , linux-samsung-soc@vger.kernel.org, rtc-linux@googlegroups.com From: Krzysztof Kozlowski X-Enigmail-Draft-Status: N1110 Message-id: <56E772F2.2030507@samsung.com> Date: Tue, 15 Mar 2016 11:26:58 +0900 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-version: 1.0 In-reply-to: <56E76C6F.5020803@osg.samsung.com> Content-type: text/plain; charset=windows-1252 Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFnrNLMWRmVeSWpSXmKPExsVy+t/xK7rfip6HGbSd57XouLaYyeLN2zVM FrPvP2axeP3C0OLyrjlsFjPO72Oy2N/ZwejA7vFk00VGjz0TT7J5bOm/y+7xZdU1Zo++LasY PT5vkgtgi+KySUnNySxLLdK3S+DKaNkjXTBdsmLr1ttMDYy9Il2MnBwSAiYShy79YIKwxSQu 3FvP1sXIxSEksJRRovX7XyjnKaPEzNmnWUGqhAU8JHbOf8oIYosIhEr8u3ibEaKok1Giu2kr M4jDLDCPUeLpwVnsIFVsAsYSm5cvYYPYISfR2z2JBcTmFdCSaN15EWwqi4CqxI0bc8FqRAUi JA53drFD1AhK/Jh8D6yeU0Bf4tSyHUDbOIAW6Encv6gFEmYWkJfYvOYt8wRGwVlIOmYhVM1C UrWAkXkVo2hqaXJBcVJ6rpFecWJucWleul5yfu4mRkgUfN3BuPSY1SFGAQ5GJR7eGVLPw4RY E8uKK3MPMUpwMCuJ8CYkAIV4UxIrq1KL8uOLSnNSiw8xSnOwKInzztz1PkRIID2xJDU7NbUg tQgmy8TBKdXA2BhRf6joafU6nXVJ20JE0mROCJybF8KwOeifyWtR/1/BgpudT31y+cc5Szo/ /rmYek6U7KEbEWxCK8yKr2/pvtLVcHfmMoeNynFHJzvUGBRo7J1+4B/Dqhk8iXNnb9tw1vii 9095JYUMrwv3j5RN8U45qRav8zle1/M1r8SB6QKrOJys/HvLlFiKMxINtZiLihMB2rb1SX4C AAA= Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 15.03.2016 10:59, Javier Martinez Canillas wrote: > Hello Krzysztof, > > On 03/14/2016 10:50 PM, Krzysztof Kozlowski wrote: >> On 15.03.2016 10:38, Javier Martinez Canillas wrote: >>> The clock and source clock looked up by the driver may not be available >>> just because the clock controller driver was not probed yet so printing >>> an error in this case is not correct and only adds confusion to users. >>> >>> However, knowing that a driver's probe was deferred may be useful so it >>> can be printed as a debug information. >>> >>> Signed-off-by: Javier Martinez Canillas >>> >>> --- >>> >>> Changes in v3: >>> - Change debug messages again as suggested by Joe Perches. >>> >>> Changes in v2: >>> - Improve debug messages as suggested by Joe Perches. >>> >>> drivers/rtc/rtc-s3c.c | 19 ++++++++++++++----- >>> 1 file changed, 14 insertions(+), 5 deletions(-) >>> >>> diff --git a/drivers/rtc/rtc-s3c.c b/drivers/rtc/rtc-s3c.c >>> index ffb860d18701..d01ad7e8078e 100644 >>> --- a/drivers/rtc/rtc-s3c.c >>> +++ b/drivers/rtc/rtc-s3c.c >>> @@ -501,18 +501,27 @@ static int s3c_rtc_probe(struct platform_device *pdev) >>> >>> info->rtc_clk = devm_clk_get(&pdev->dev, "rtc"); >>> if (IS_ERR(info->rtc_clk)) { >>> - dev_err(&pdev->dev, "failed to find rtc clock\n"); >>> - return PTR_ERR(info->rtc_clk); >>> + ret = PTR_ERR(info->rtc_clk); >>> + if (ret != -EPROBE_DEFER) >>> + dev_err(&pdev->dev, "failed to find rtc clock\n"); >>> + else >>> + dev_dbg(&pdev->dev, "probe deferred due to missing rtc clk\n"); >>> + return ret; >>> } >>> clk_prepare_enable(info->rtc_clk); >>> >>> if (info->data->needs_src_clk) { >>> info->rtc_src_clk = devm_clk_get(&pdev->dev, "rtc_src"); >>> if (IS_ERR(info->rtc_src_clk)) { >>> - dev_err(&pdev->dev, >>> - "failed to find rtc source clock\n"); >>> + ret = PTR_ERR(info->rtc_src_clk); >>> + if (ret != -EPROBE_DEFER) >>> + dev_err(&pdev->dev, >>> + "failed to find rtc source clock\n"); >>> + else >>> + dev_dbg(&pdev->dev, >>> + "probe deferred due to missing rtc src clk\n"); >>> clk_disable_unprepare(info->rtc_clk); >>> - return PTR_ERR(info->rtc_src_clk); >>> + return ret; >>> } >>> clk_prepare_enable(info->rtc_src_clk); >>> } >>> >> >> The error path starts looking complicated. This has now 4 indentation >> levels... >> > > Yeah, I don't think we can get rid of the 4 indentation levels since > the function already has 3 and a check for the errno code is needed. Probably handling of the clocks in the driver could be simplified a little bit (the if(needs_src_clk) appears in few places)... but this is out of scope for this patch. > >> I agree for removal of error in case of probe deferral because it might >> be misleading but I don't see much benefit of a debug message. >> > > But yes, we can at least get rid of the else statement. I don't have a > strong opinion about the debug information, I left it to avoid someone > to tell me that I was removing a useful log. Although dev_dbg doesn't harm... but isn't driver core printing debug message already? BR, Krzysztof