From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-17.7 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id F2704C43460 for ; Tue, 27 Apr 2021 08:06:38 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id C23BE60FF3 for ; Tue, 27 Apr 2021 08:06:38 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234992AbhD0IHU (ORCPT ); Tue, 27 Apr 2021 04:07:20 -0400 Received: from mail.kernel.org ([198.145.29.99]:48506 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230348AbhD0IHS (ORCPT ); Tue, 27 Apr 2021 04:07:18 -0400 Received: by mail.kernel.org (Postfix) with ESMTPSA id A56B261289; Tue, 27 Apr 2021 08:06:33 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1619510795; bh=uDqPXc72hL4aKi2U6+mtvOcmn5lhmGyP0bhksbuvE9Y=; h=Subject:To:Cc:References:From:Date:In-Reply-To:From; b=ayD8QkQDQGRBR36w01ROgZE2eWW1yO3dh7mBaWtwJnt+PFf1oheG6EwMvqdxdbOhT cJ3g8Ok6jFCFhHJidRZmX3PuF/k/ZR5unL1eJ2ILyL/96nWqq7yh3VQaOu3Q0ek1Rt WGJx2vOpRQQ1iZL6/iIpKcFj5onx4WtX+4ngNLu+GFqxMRz1AW/Dyo9bnSxBainHa+ 2rErKGsFVd44GwLCxAHSVCly+83Vd/3iSxUXVkQhmQV70yyPPOVaLCcGMR44AbGHja /LEMlMhJdQDXbMkE9NPg8XG4tQoaYqiTmKqYFroVSre3NaQyo6562hL/ynCIpruE3p 8PVbUMJ5oeMnA== Subject: Re: [PATCH 57/78] media: exynos4-is: use pm_runtime_resume_and_get() To: Mauro Carvalho Chehab Cc: linuxarm@huawei.com, mauro.chehab@huawei.com, Krzysztof Kozlowski , Mauro Carvalho Chehab , Sylwester Nawrocki , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-media@vger.kernel.org, linux-samsung-soc@vger.kernel.org References: <091915bb1cbec13b566d129f85ae229fcb92e2e4.1619191723.git.mchehab+huawei@kernel.org> <45068e81-8f9b-fea8-b7bc-bdd0443ba7e6@kernel.org> <20210426151224.2b677d1b@coco.lan> From: Sylwester Nawrocki Message-ID: <1a98b4b7-78c2-810d-a4ba-762c1a9576b3@kernel.org> Date: Tue, 27 Apr 2021 10:06:31 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.1 MIME-Version: 1.0 In-Reply-To: <20210426151224.2b677d1b@coco.lan> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 26.04.2021 15:12, Mauro Carvalho Chehab wrote: > Em Sun, 25 Apr 2021 22:57:25 +0200 > Sylwester Nawrocki escreveu: > >> On 24.04.2021 08:45, Mauro Carvalho Chehab wrote: >>> Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter") >>> added pm_runtime_resume_and_get() in order to automatically handle >>> dev->power.usage_count decrement on errors. >>> >>> Use the new API, in order to cleanup the error check logic. >>> >>> Signed-off-by: Mauro Carvalho Chehab >>> diff --git a/drivers/media/platform/exynos4-is/fimc-is.c b/drivers/media/platform/exynos4-is/fimc-is.c >>> index 972d9601d236..bca35866cc74 100644 >>> --- a/drivers/media/platform/exynos4-is/fimc-is.c >>> +++ b/drivers/media/platform/exynos4-is/fimc-is.c >>> @@ -828,7 +828,7 @@ static int fimc_is_probe(struct platform_device *pdev) >>> goto err_irq; >>> } >>> >>> - ret = pm_runtime_get_sync(dev); >>> + ret = pm_runtime_resume_and_get(dev); >>> if (ret < 0) >>> goto err_pm; >> >> It seems you intended to use err_suspend label here. We don't need >> a new label though, instead of err_pm we can jump to err_irq when >> pm_runtime_resume_and_get() fails. > > Thanks! Will fix at the next version. > >> Note that when runtime PM is >> disabled pm_runtime_resume_and_get() always returns 0. > > Ok, but there are a couple of conditions at rpm_resume() function > at drivers/base/power/runtime.c (which is the code that actually > handles those PM macros) that could make it to return errors, > which are independent on the PM callbacks, like those: > > if (dev->power.runtime_error) > retval = -EINVAL; > else if (dev->power.disable_depth > 0) > retval = -EACCES; > > and more might be added as the PM core changes. Right, I looked only at !CONFIG_PM case, this is what the "if (!pm_runtime_enabled(dev))" test and explicit fimc_is_runtime_{resume,suspend} calls were originally for. Agreed, better not to rely too much on internal implementation as there is no specific guarantees about return value at the API documentation. Regards, Sylwester >>> @@ -862,6 +862,7 @@ static int fimc_is_probe(struct platform_device *pdev) >>> fimc_is_unregister_subdevs(is); >>> err_pm: >>> pm_runtime_put_noidle(dev); >>> +err_suspend: >>> if (!pm_runtime_enabled(dev)) >>> fimc_is_runtime_suspend(dev); >>> err_irq: >> From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.7 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI, NICE_REPLY_A,SPF_HELO_NONE,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id DE93FC433B4 for ; Tue, 27 Apr 2021 08:08:46 +0000 (UTC) Received: from desiato.infradead.org (desiato.infradead.org [90.155.92.199]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 30CEA6101B for ; Tue, 27 Apr 2021 08:08:44 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 30CEA6101B Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=desiato.20200630; h=Sender:Content-Type: Content-Transfer-Encoding:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date:Message-ID:From: References:Cc:To:Subject:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=h1y3xsicdV5O6+rJeyneq+DnjOHMORnfakmxBODwUm8=; b=oe/mnqvf0Ed7SDzDjT9IvRIX+ aMtH1X+BLosI2fNT6QBxS9QA2Hm9EYUTDBfhxsW8w1JUNBW8X+civpYAiFb7T/9qe/oHCV6GUFuNi mLFK3bSeRCVR7CCpmVLPyItylo+XWYnTGBf1Cr5bhXlT0zkWEwflq0es1+RdrF/uBRKz4pOs27VfP nSI3/C5p0UHML5nOAqIbJ/VnyTXXigN6wVfFu5csko1k8I8i+vTHJAGJegF3/hltMiWDEnhfeDYYp ktOEWfxQ3zsrt8u+Gh9pDA6AzU03UNgmmHTSnZoIzjPM+WqVBc2nxDmlXOoPkvpA7i6MUcUOu4EkF r1L0Co/Sg==; Received: from localhost ([::1] helo=desiato.infradead.org) by desiato.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1lbIjz-0014QU-9I; Tue, 27 Apr 2021 08:06:46 +0000 Received: from bombadil.infradead.org ([2607:7c80:54:e::133]) by desiato.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1lbIjv-0014QA-IR for linux-arm-kernel@desiato.infradead.org; Tue, 27 Apr 2021 08:06:40 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20210309; h=Content-Transfer-Encoding: Content-Type:In-Reply-To:MIME-Version:Date:Message-ID:From:References:Cc:To: Subject:Sender:Reply-To:Content-ID:Content-Description; bh=Gyz/bfFyrOc+6R+azd2xG5nVmqTkOopbCXrwVEqphgw=; b=qhg3B4gvYbwfRejTfxB8LyUZL7 arzrwYHDYxFTuj2qTSU7olaWxcDaIrT9MquZ7bUaleAILX0z3i7mwFigaINTUht6PTr+m7hS+UTnO XD4A+cPj3RIZcYsiQ/fsYAGkLN4Cdaqv8JOY2QEwoAZez6bzd2h1uYtsr1U0UkFyD7Uu4YJJlYb0+ LtzdOKuYz8awm6VFcoAMj7xmdyFCAW7zUWlPI2CnBwCb7j8WFo/8YwygMJSYGJkXvJNu0/uCSu2rb Hio7wnmQYjJS25MYIP8csvs2Q8KQf6gtMevylaAKHQjswu8b3DofsSJIaSRJAnTFD1UT4nNzSQgzy Rf0EB3Jg==; Received: from mail.kernel.org ([198.145.29.99]) by bombadil.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1lbIjs-00GWqc-V5 for linux-arm-kernel@lists.infradead.org; Tue, 27 Apr 2021 08:06:38 +0000 Received: by mail.kernel.org (Postfix) with ESMTPSA id A56B261289; Tue, 27 Apr 2021 08:06:33 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1619510795; bh=uDqPXc72hL4aKi2U6+mtvOcmn5lhmGyP0bhksbuvE9Y=; h=Subject:To:Cc:References:From:Date:In-Reply-To:From; b=ayD8QkQDQGRBR36w01ROgZE2eWW1yO3dh7mBaWtwJnt+PFf1oheG6EwMvqdxdbOhT cJ3g8Ok6jFCFhHJidRZmX3PuF/k/ZR5unL1eJ2ILyL/96nWqq7yh3VQaOu3Q0ek1Rt WGJx2vOpRQQ1iZL6/iIpKcFj5onx4WtX+4ngNLu+GFqxMRz1AW/Dyo9bnSxBainHa+ 2rErKGsFVd44GwLCxAHSVCly+83Vd/3iSxUXVkQhmQV70yyPPOVaLCcGMR44AbGHja /LEMlMhJdQDXbMkE9NPg8XG4tQoaYqiTmKqYFroVSre3NaQyo6562hL/ynCIpruE3p 8PVbUMJ5oeMnA== Subject: Re: [PATCH 57/78] media: exynos4-is: use pm_runtime_resume_and_get() To: Mauro Carvalho Chehab Cc: linuxarm@huawei.com, mauro.chehab@huawei.com, Krzysztof Kozlowski , Mauro Carvalho Chehab , Sylwester Nawrocki , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-media@vger.kernel.org, linux-samsung-soc@vger.kernel.org References: <091915bb1cbec13b566d129f85ae229fcb92e2e4.1619191723.git.mchehab+huawei@kernel.org> <45068e81-8f9b-fea8-b7bc-bdd0443ba7e6@kernel.org> <20210426151224.2b677d1b@coco.lan> From: Sylwester Nawrocki Message-ID: <1a98b4b7-78c2-810d-a4ba-762c1a9576b3@kernel.org> Date: Tue, 27 Apr 2021 10:06:31 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.1 MIME-Version: 1.0 In-Reply-To: <20210426151224.2b677d1b@coco.lan> Content-Language: en-US X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210427_010637_114242_92610F78 X-CRM114-Status: GOOD ( 20.50 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 26.04.2021 15:12, Mauro Carvalho Chehab wrote: > Em Sun, 25 Apr 2021 22:57:25 +0200 > Sylwester Nawrocki escreveu: > >> On 24.04.2021 08:45, Mauro Carvalho Chehab wrote: >>> Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter") >>> added pm_runtime_resume_and_get() in order to automatically handle >>> dev->power.usage_count decrement on errors. >>> >>> Use the new API, in order to cleanup the error check logic. >>> >>> Signed-off-by: Mauro Carvalho Chehab >>> diff --git a/drivers/media/platform/exynos4-is/fimc-is.c b/drivers/media/platform/exynos4-is/fimc-is.c >>> index 972d9601d236..bca35866cc74 100644 >>> --- a/drivers/media/platform/exynos4-is/fimc-is.c >>> +++ b/drivers/media/platform/exynos4-is/fimc-is.c >>> @@ -828,7 +828,7 @@ static int fimc_is_probe(struct platform_device *pdev) >>> goto err_irq; >>> } >>> >>> - ret = pm_runtime_get_sync(dev); >>> + ret = pm_runtime_resume_and_get(dev); >>> if (ret < 0) >>> goto err_pm; >> >> It seems you intended to use err_suspend label here. We don't need >> a new label though, instead of err_pm we can jump to err_irq when >> pm_runtime_resume_and_get() fails. > > Thanks! Will fix at the next version. > >> Note that when runtime PM is >> disabled pm_runtime_resume_and_get() always returns 0. > > Ok, but there are a couple of conditions at rpm_resume() function > at drivers/base/power/runtime.c (which is the code that actually > handles those PM macros) that could make it to return errors, > which are independent on the PM callbacks, like those: > > if (dev->power.runtime_error) > retval = -EINVAL; > else if (dev->power.disable_depth > 0) > retval = -EACCES; > > and more might be added as the PM core changes. Right, I looked only at !CONFIG_PM case, this is what the "if (!pm_runtime_enabled(dev))" test and explicit fimc_is_runtime_{resume,suspend} calls were originally for. Agreed, better not to rely too much on internal implementation as there is no specific guarantees about return value at the API documentation. Regards, Sylwester >>> @@ -862,6 +862,7 @@ static int fimc_is_probe(struct platform_device *pdev) >>> fimc_is_unregister_subdevs(is); >>> err_pm: >>> pm_runtime_put_noidle(dev); >>> +err_suspend: >>> if (!pm_runtime_enabled(dev)) >>> fimc_is_runtime_suspend(dev); >>> err_irq: >> _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel