From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jarkko Nikula Subject: Re: [PATCH 02/10] i2c: designware-platdrv: Gate clk in error path in ->probe() Date: Wed, 15 Jun 2016 15:07:02 +0300 Message-ID: <0b2b63b3-6c66-fba2-028d-ab366b9df1af@linux.intel.com> References: <1465916848-8207-1-git-send-email-ulf.hansson@linaro.org> <1465916848-8207-3-git-send-email-ulf.hansson@linaro.org> <1465917768.30123.65.camel@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Return-path: Received: from mga14.intel.com ([192.55.52.115]:10227 "EHLO mga14.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751903AbcFOMHh (ORCPT ); Wed, 15 Jun 2016 08:07:37 -0400 In-Reply-To: Sender: linux-i2c-owner@vger.kernel.org List-Id: linux-i2c@vger.kernel.org To: Ulf Hansson , Andy Shevchenko Cc: Wolfram Sang , "linux-i2c@vger.kernel.org" , Mika Westerberg , John Stultz , Guodong Xu , "linux-arm-kernel@lists.infradead.org" On 06/15/2016 10:16 AM, Ulf Hansson wrote: > On 14 June 2016 at 17:22, Andy Shevchenko > wrote: >> On Tue, 2016-06-14 at 17:07 +0200, Ulf Hansson wrote: >>> The call to i2c_dw_probe() may fail in ->probe() in which case the >>> clock >>> remains ungated. Fix the error path by gating the clock before the >>> error >>> code is returned. >>> >>> Signed-off-by: Ulf Hansson >>> --- >>> drivers/i2c/busses/i2c-designware-platdrv.c | 10 ++++++++-- >>> 1 file changed, 8 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c >>> b/drivers/i2c/busses/i2c-designware-platdrv.c >>> index e39962b..19174e7 100644 >>> --- a/drivers/i2c/busses/i2c-designware-platdrv.c >>> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c >>> @@ -235,6 +235,7 @@ static int dw_i2c_plat_probe(struct >>> platform_device *pdev) >>> ACPI_COMPANION_SET(&adap->dev, ACPI_COMPANION(&pdev->dev)); >>> adap->dev.of_node = pdev->dev.of_node; >>> >> >> >>> + pm_runtime_get_noresume(&pdev->dev); >> >>> + pm_runtime_put(&pdev->dev); >> >> I don't see an explanation of these add-ons. > > As we explicitly do clk_disable_unprepare() in the error path, we must > prevent the ->runtime_suspend() callback to be called during this > period. > > This is a common pattern used by many drivers deploying runtime PM > support, which is probably why I left out the explanation. > What would cause here runtime suspend during probe? Also pairing pm_runtime_get_noresume() with pm_runtime_put() instead of pm_runtime_put_noidle() looks suspicious to me. -- Jarkko