From patchwork Wed Nov 27 23:15:41 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nishanth Menon X-Patchwork-Id: 3250511 Return-Path: X-Original-To: patchwork-linux-omap@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 0470F9F526 for ; Wed, 27 Nov 2013 23:17:32 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id C106520595 for ; Wed, 27 Nov 2013 23:17:29 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id AA8DB20453 for ; Wed, 27 Nov 2013 23:17:28 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758249Ab3K0XRS (ORCPT ); Wed, 27 Nov 2013 18:17:18 -0500 Received: from arroyo.ext.ti.com ([192.94.94.40]:39586 "EHLO arroyo.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757549Ab3K0XRQ (ORCPT ); Wed, 27 Nov 2013 18:17:16 -0500 Received: from dflxv15.itg.ti.com ([128.247.5.124]) by arroyo.ext.ti.com (8.13.7/8.13.7) with ESMTP id rARNFgkA021668; Wed, 27 Nov 2013 17:15:42 -0600 Received: from DLEE71.ent.ti.com (dlee71.ent.ti.com [157.170.170.114]) by dflxv15.itg.ti.com (8.14.3/8.13.8) with ESMTP id rARNFgVb014261; Wed, 27 Nov 2013 17:15:42 -0600 Received: from dlep33.itg.ti.com (157.170.170.75) by DLEE71.ent.ti.com (157.170.170.114) with Microsoft SMTP Server id 14.2.342.3; Wed, 27 Nov 2013 17:15:41 -0600 Received: from localhost (ileax41-snat.itg.ti.com [10.172.224.153]) by dlep33.itg.ti.com (8.14.3/8.13.8) with ESMTP id rARNFf8Q027748; Wed, 27 Nov 2013 17:15:41 -0600 Date: Wed, 27 Nov 2013 17:15:41 -0600 From: Nishanth Menon To: Sekhar Nori , CC: Daniel Mack , , , , , , , , , Kevin Hilman , , , , , , , , Subject: pm_runtime functions and IS_ERR_VALUE (was Re: [PATCH v6] ARM: omap: edma: add suspend suspend/resume hooks) Message-ID: <20131127231541.GA8418@kahuna> References: <1384726754-27875-1-git-send-email-zonque@gmail.com> <5295F53C.8040101@ti.com> <5295F806.9030900@gmail.com> <5295F99F.6010203@ti.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <5295F99F.6010203@ti.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-omap-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-omap@vger.kernel.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Change in subject line + wider forum On 19:24-20131127, Sekhar Nori wrote: > On Wednesday 27 November 2013 07:17 PM, Daniel Mack wrote: > > On 11/27/2013 02:35 PM, Sekhar Nori wrote: > >> On Monday 18 November 2013 03:49 AM, Daniel Mack wrote: > >>> +static int edma_pm_suspend(struct device *dev) > >>> +{ > >>> + int j, r; > >>> + > >>> + r = pm_runtime_get_sync(dev); > >>> + if (IS_ERR_VALUE(r)) { > >> > >> So IS_ERR_VALUE() is only for functions which may return a negative > >> number outside of MAX_ERRNO as a success indication. > >> pm_runtime_get_sync() does not appear to be one of them so just use" > >> > >> if (r < 0) { .. } > > > > That's true. Thanks for catching this, I'll fix it. However, grepping > > through the tree, there are quite a lot places where the same mistake is > > made. > > Yes, this is a common fallacy. Russell cleaned up a bunch of these a > while back. Thinking a little more on this front, to prevent recurrence and fixing the ones we already have, how about something like the following patch? For example, on 3.13-rc1, with omap2plus_defconfig, I see the following: http://pastebin.mozilla.org/3681911 -->8-- From b7946d214fab72b2e18cd67eec01c377f1cddee8 Mon Sep 17 00:00:00 2001 From: Nishanth Menon Date: Wed, 27 Nov 2013 17:01:20 -0600 Subject: [RFC PATCH] scripts: Coccinelle script for pm_runtime_* return checks with IS_ERR_VALUE As indicated by Sekhar in [1], there seems to be a tendency to use IS_ERR_VALUE to check the error result for pm_runtime_* functions which make no sense considering commit c48cd65 (ARM: OMAP: use consistent error checking) - the error values can either be < 0 for error OR 0, 1 in cases where we have success. So, setup a coccinelle script to help identify the same. [1] http://marc.info/?t=138472678100003&r=1&w=2 Cc: Russell King Reported-by: Sekhar Nori Signed-off-by: Nishanth Menon --- scripts/coccinelle/api/pm_runtime.cocci | 109 +++++++++++++++++++++++++++++++ 1 file changed, 109 insertions(+) create mode 100644 scripts/coccinelle/api/pm_runtime.cocci diff --git a/scripts/coccinelle/api/pm_runtime.cocci b/scripts/coccinelle/api/pm_runtime.cocci new file mode 100644 index 0000000..f01789e --- /dev/null +++ b/scripts/coccinelle/api/pm_runtime.cocci @@ -0,0 +1,109 @@ +/// Make sure pm_runtime_* calls does not use unnecessary IS_ERR_VALUE +// +// Keywords: pm_runtime +// Confidence: Medium +// Copyright (C) 2013 Texas Instruments Incorporated - GPLv2. +// URL: http://coccinelle.lip6.fr/ +// Options: --include-headers + +virtual patch +virtual context +virtual org +virtual report + +//---------------------------------------------------------- +// Detection +//---------------------------------------------------------- + +@runtime_bad_err_handle exists@ +expression ret; +@@ +( +ret = \(pm_runtime_idle\| + pm_runtime_suspend\| + pm_runtime_autosuspend\| + pm_runtime_resume\| + pm_request_idle\| + pm_request_resume\| + pm_request_autosuspend\| + pm_runtime_get\| + pm_runtime_get_sync\| + pm_runtime_put\| + pm_runtime_put_autosuspend\| + pm_runtime_put_sync\| + pm_runtime_put_sync_suspend\| + pm_runtime_put_sync_autosuspend\| + pm_runtime_set_active\| + pm_schedule_suspend\| + pm_runtime_barrier\| + pm_generic_runtime_suspend\| + pm_generic_runtime_resume\)(...); +... +IS_ERR_VALUE(ret) +... +) + +//---------------------------------------------------------- +// For context mode +//---------------------------------------------------------- + +@depends on runtime_bad_err_handle && context@ +identifier pm_runtime_api; +expression ret; +@@ +( +ret = pm_runtime_api(...); +... +* IS_ERR_VALUE(ret) +... +) + +//---------------------------------------------------------- +// For patch mode +//---------------------------------------------------------- + +@depends on runtime_bad_err_handle && patch@ +identifier pm_runtime_api; +expression ret; +@@ +( +ret = pm_runtime_api(...); +... +- IS_ERR_VALUE(ret) ++ ret < 0 +... +) + +//---------------------------------------------------------- +// For org and report mode +//---------------------------------------------------------- + +@r depends on runtime_bad_err_handle exists@ +position p1, p2; +identifier pm_runtime_api; +expression ret; +@@ +( +ret = pm_runtime_api@p1(...); +... +IS_ERR_VALUE@p2(ret) +... +) + +@script:python depends on org@ +p1 << r.p1; +p2 << r.p2; +pm_runtime_api << r.pm_runtime_api; +@@ + +cocci.print_main(pm_runtime_api,p1) +cocci.print_secs("IS_ERR_VALUE",p2) + +@script:python depends on report@ +p1 << r.p1; +p2 << r.p2; +pm_runtime_api << r.pm_runtime_api; +@@ + +msg = "%s returns < 0 as error. Unecessary IS_ERR_VALUE at line %s" % (pm_runtime_api, p2[0].line) +coccilib.report.print_report(p1[0],msg)