Message ID | 1353645902-7467-2-git-send-email-sachin.kamat@linaro.org (mailing list archive) |
---|---|
State | Superseded, archived |
Headers |
Received: from mail.tu-berlin.de ([130.149.7.33]) by www.linuxtv.org with esmtp (Exim 4.72) (envelope-from <linux-media-owner@vger.kernel.org>) id 1TblEm-0002VP-3q for patchwork@linuxtv.org; Fri, 23 Nov 2012 05:51:36 +0100 X-tubIT-Incoming-IP: 209.132.180.67 Received: from vger.kernel.org ([209.132.180.67]) by mail.tu-berlin.de (exim-4.75/mailfrontend-4) with esmtp for <patchwork@linuxtv.org> id 1TblEl-0002ee-B7; Fri, 23 Nov 2012 05:51:35 +0100 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755783Ab2KWEv0 (ORCPT <rfc822;patchwork@linuxtv.org>); Thu, 22 Nov 2012 23:51:26 -0500 Received: from mail-da0-f46.google.com ([209.85.210.46]:62866 "EHLO mail-da0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755727Ab2KWEvZ (ORCPT <rfc822;linux-media@vger.kernel.org>); Thu, 22 Nov 2012 23:51:25 -0500 Received: by mail-da0-f46.google.com with SMTP id p5so2471532dak.19 for <linux-media@vger.kernel.org>; Thu, 22 Nov 2012 20:51:25 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=from:to:cc:subject:date:message-id:x-mailer:in-reply-to:references :x-gm-message-state; bh=hslxmHaTT8tswuvZVRiqQcXDiuhoG76yBPLTBHOYgsg=; b=nkonjBlLm5qqPHTOWnA/qtfReWG66w8wdPzf6O/xamNdIrXna1DWFEUhaRqa3my2+t PBwRFS1sVA1kSlf1idP1p5unphoHF1bu2OYTcjpms8UFeFztraT0ccwDYSpxE7mG/74E hkdagQQ7Vinpdg1QMyYCadA7O8zSFClAIK01k49S0h28tpKwrjZR50Oxb/k+Zz7yjlLX buWedwAq1V3bkIn6VnbVX/9O4uMdJpGBc1zECfFuwjitGhx3xn7Y0BzZGaM3mMNqnXfp uTtaH3pEqEb5+VOVnwR0N72uWl6uWsSBe/bXNjG4P8nwue2DjOKplUUyYB3VB2gqbU8w J5jQ== Received: by 10.68.189.138 with SMTP id gi10mr10708715pbc.165.1353646285451; Thu, 22 Nov 2012 20:51:25 -0800 (PST) Received: from localhost.localdomain ([115.113.119.130]) by mx.google.com with ESMTPS id x8sm3029746paw.16.2012.11.22.20.51.22 (version=TLSv1/SSLv3 cipher=OTHER); Thu, 22 Nov 2012 20:51:24 -0800 (PST) From: Sachin Kamat <sachin.kamat@linaro.org> To: linux-media@vger.kernel.org Cc: s.nawrocki@samsung.com, sachin.kamat@linaro.org, patches@linaro.org, Shaik Ameer Basha <shaik.ameer@samsung.com> Subject: [PATCH 1/4] [media] exynos-gsc: Rearrange error messages for valid prints Date: Fri, 23 Nov 2012 10:14:59 +0530 Message-Id: <1353645902-7467-2-git-send-email-sachin.kamat@linaro.org> X-Mailer: git-send-email 1.7.4.1 In-Reply-To: <1353645902-7467-1-git-send-email-sachin.kamat@linaro.org> References: <1353645902-7467-1-git-send-email-sachin.kamat@linaro.org> X-Gm-Message-State: ALoCoQliVjznpJ7AhQkbhMK468jgn/ONlf3pJyBFeHtZFxNn0qdVNaRM2OcGcsUtIi9x8w+X3xB7 Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: <linux-media.vger.kernel.org> X-Mailing-List: linux-media@vger.kernel.org X-PMX-Version: 5.6.1.2065439, Antispam-Engine: 2.7.2.376379, Antispam-Data: 2012.11.23.43916 X-PMX-Spam: Gauge=IIIIIIII, Probability=8%, Report=' MULTIPLE_RCPTS 0.1, HTML_00_01 0.05, HTML_00_10 0.05, BODYTEXTP_SIZE_3000_LESS 0, BODY_SIZE_1700_1799 0, BODY_SIZE_2000_LESS 0, BODY_SIZE_5000_LESS 0, BODY_SIZE_7000_LESS 0, URI_ENDS_IN_HTML 0, __ANY_URI 0, __CP_URI_IN_BODY 0, __HAS_FROM 0, __HAS_MSGID 0, __HAS_X_MAILER 0, __HAS_X_MAILING_LIST 0, __MIME_TEXT_ONLY 0, __MULTIPLE_RCPTS_CC_X2 0, __SANE_MSGID 0, __SUBJ_ALPHA_END 0, __TO_MALFORMED_2 0, __TO_NO_NAME 0, __URI_NO_WWW 0, __URI_NS , __YOUTUBE_RCVD 0' |
Commit Message
Sachin Kamat
Nov. 23, 2012, 4:44 a.m. UTC
In case of clk_prepare failure, the function gsc_clk_get also prints
"failed to get clock" which is not correct. Hence move the error
messages to their respective blocks. While at it, also renamed the labels
meaningfully.
Cc: Shaik Ameer Basha <shaik.ameer@samsung.com>
Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
drivers/media/platform/exynos-gsc/gsc-core.c | 19 ++++++++++---------
1 files changed, 10 insertions(+), 9 deletions(-)
Comments
Hi Sachin, Thanks for the patches. On 11/23/2012 05:44 AM, Sachin Kamat wrote: > In case of clk_prepare failure, the function gsc_clk_get also prints > "failed to get clock" which is not correct. Hence move the error > messages to their respective blocks. While at it, also renamed the labels > meaningfully. > > Cc: Shaik Ameer Basha <shaik.ameer@samsung.com> > Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org> > --- > drivers/media/platform/exynos-gsc/gsc-core.c | 19 ++++++++++--------- > 1 files changed, 10 insertions(+), 9 deletions(-) > > diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c b/drivers/media/platform/exynos-gsc/gsc-core.c > index 6d6f65d..45bcfa7 100644 > --- a/drivers/media/platform/exynos-gsc/gsc-core.c > +++ b/drivers/media/platform/exynos-gsc/gsc-core.c > @@ -1017,25 +1017,26 @@ static int gsc_clk_get(struct gsc_dev *gsc) > dev_dbg(&gsc->pdev->dev, "gsc_clk_get Called\n"); > > gsc->clock = clk_get(&gsc->pdev->dev, GSC_CLOCK_GATE_NAME); > - if (IS_ERR(gsc->clock)) > - goto err_print; > + if (IS_ERR(gsc->clock)) { > + dev_err(&gsc->pdev->dev, "failed to get clock~~~: %s\n", > + GSC_CLOCK_GATE_NAME); > + goto err_clk_get; You could also just return PTR_ERR(gsc->clock) here and remove err_clk_get label entirely. > + } > > ret = clk_prepare(gsc->clock); > if (ret < 0) { > + dev_err(&gsc->pdev->dev, "clock prepare failed for clock: %s\n", > + GSC_CLOCK_GATE_NAME); > clk_put(gsc->clock); > gsc->clock = NULL; > - goto err; > + goto err_clk_prepare; > } > > return 0; > > -err: > - dev_err(&gsc->pdev->dev, "clock prepare failed for clock: %s\n", > - GSC_CLOCK_GATE_NAME); > +err_clk_prepare: > gsc_clk_put(gsc); This call can be removed too. I would remove all labels and gotos in this function. Since there is only one clock, you need to only call clk_put when clk_prepare() fails, there is no need for gsc_clk_put(). > -err_print: > - dev_err(&gsc->pdev->dev, "failed to get clock~~~: %s\n", > - GSC_CLOCK_GATE_NAME); > +err_clk_get: > return -ENXIO; > } As a general remark, I think changes like in this series have to be validated before we can think of applying it. I guess Shaik or somebody else would need to test it. I still have no board I could test Exynos5 Gscaler IP. -- Regards, Sylwester -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Sylwester, Thanks for the review. On 23 November 2012 15:01, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote: > Hi Sachin, > > Thanks for the patches. > > On 11/23/2012 05:44 AM, Sachin Kamat wrote: >> In case of clk_prepare failure, the function gsc_clk_get also prints >> "failed to get clock" which is not correct. Hence move the error >> messages to their respective blocks. While at it, also renamed the labels >> meaningfully. >> >> Cc: Shaik Ameer Basha <shaik.ameer@samsung.com> >> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org> >> --- >> drivers/media/platform/exynos-gsc/gsc-core.c | 19 ++++++++++--------- >> 1 files changed, 10 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c b/drivers/media/platform/exynos-gsc/gsc-core.c >> index 6d6f65d..45bcfa7 100644 >> --- a/drivers/media/platform/exynos-gsc/gsc-core.c >> +++ b/drivers/media/platform/exynos-gsc/gsc-core.c >> @@ -1017,25 +1017,26 @@ static int gsc_clk_get(struct gsc_dev *gsc) >> dev_dbg(&gsc->pdev->dev, "gsc_clk_get Called\n"); >> >> gsc->clock = clk_get(&gsc->pdev->dev, GSC_CLOCK_GATE_NAME); >> - if (IS_ERR(gsc->clock)) >> - goto err_print; >> + if (IS_ERR(gsc->clock)) { >> + dev_err(&gsc->pdev->dev, "failed to get clock~~~: %s\n", >> + GSC_CLOCK_GATE_NAME); >> + goto err_clk_get; > > You could also just return PTR_ERR(gsc->clock) here and remove > err_clk_get label entirely. OK. > >> + } >> >> ret = clk_prepare(gsc->clock); >> if (ret < 0) { >> + dev_err(&gsc->pdev->dev, "clock prepare failed for clock: %s\n", >> + GSC_CLOCK_GATE_NAME); >> clk_put(gsc->clock); >> gsc->clock = NULL; >> - goto err; >> + goto err_clk_prepare; >> } >> >> return 0; >> >> -err: >> - dev_err(&gsc->pdev->dev, "clock prepare failed for clock: %s\n", >> - GSC_CLOCK_GATE_NAME); >> +err_clk_prepare: >> gsc_clk_put(gsc); > > This call can be removed too. I would remove all labels and gotos in > this function. Since there is only one clock, you need to only call > clk_put when clk_prepare() fails, there is no need for gsc_clk_put(). I have removed gsc_clk_put() in the subsequent patch in this series. I will probably incorporate your previous comment and remove the label altogether (in patch 3) and send it again. > >> -err_print: >> - dev_err(&gsc->pdev->dev, "failed to get clock~~~: %s\n", >> - GSC_CLOCK_GATE_NAME); >> +err_clk_get: >> return -ENXIO; >> } > > As a general remark, I think changes like in this series have to be > validated before we can think of applying it. I guess Shaik or > somebody else would need to test it. I still have no board I could > test Exynos5 Gscaler IP. Yes you are right. I have already talked to Shaik about it. He has agreed to test the same. > > -- > > Regards, > Sylwester
On 11/23/2012 10:47 AM, Sachin Kamat wrote: >>> +err_clk_prepare: >>> gsc_clk_put(gsc); >> >> This call can be removed too. I would remove all labels and gotos in >> this function. Since there is only one clock, you need to only call >> clk_put when clk_prepare() fails, there is no need for gsc_clk_put(). > > I have removed gsc_clk_put() in the subsequent patch in this series. > I will probably incorporate your previous comment and remove the label > altogether (in patch 3) and send it again. OK, sounds good to me that way too. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c b/drivers/media/platform/exynos-gsc/gsc-core.c index 6d6f65d..45bcfa7 100644 --- a/drivers/media/platform/exynos-gsc/gsc-core.c +++ b/drivers/media/platform/exynos-gsc/gsc-core.c @@ -1017,25 +1017,26 @@ static int gsc_clk_get(struct gsc_dev *gsc) dev_dbg(&gsc->pdev->dev, "gsc_clk_get Called\n"); gsc->clock = clk_get(&gsc->pdev->dev, GSC_CLOCK_GATE_NAME); - if (IS_ERR(gsc->clock)) - goto err_print; + if (IS_ERR(gsc->clock)) { + dev_err(&gsc->pdev->dev, "failed to get clock~~~: %s\n", + GSC_CLOCK_GATE_NAME); + goto err_clk_get; + } ret = clk_prepare(gsc->clock); if (ret < 0) { + dev_err(&gsc->pdev->dev, "clock prepare failed for clock: %s\n", + GSC_CLOCK_GATE_NAME); clk_put(gsc->clock); gsc->clock = NULL; - goto err; + goto err_clk_prepare; } return 0; -err: - dev_err(&gsc->pdev->dev, "clock prepare failed for clock: %s\n", - GSC_CLOCK_GATE_NAME); +err_clk_prepare: gsc_clk_put(gsc); -err_print: - dev_err(&gsc->pdev->dev, "failed to get clock~~~: %s\n", - GSC_CLOCK_GATE_NAME); +err_clk_get: return -ENXIO; }