From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753210AbcDQNe2 (ORCPT ); Sun, 17 Apr 2016 09:34:28 -0400 Received: from mail-lf0-f65.google.com ([209.85.215.65]:33277 "EHLO mail-lf0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753025AbcDQNeZ (ORCPT ); Sun, 17 Apr 2016 09:34:25 -0400 Date: Sun, 17 Apr 2016 15:34:21 +0200 From: Krzysztof Kozlowski To: Javier Martinez Canillas Cc: linux-kernel@vger.kernel.org, Anand Moon , Kukjin Kim , linux-samsung-soc@vger.kernel.org, Wolfram Sang , Krzysztof Kozlowski , linux-i2c@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v2 2/2] i2c: exynos5: Fix possible ABBA deadlock by keeping I2C clock prepared Message-ID: <20160417133421.GC3620@kozik-lap> References: <1460855693-28225-1-git-send-email-javier@osg.samsung.com> <1460855693-28225-2-git-send-email-javier@osg.samsung.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <1460855693-28225-2-git-send-email-javier@osg.samsung.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sat, Apr 16, 2016 at 09:14:53PM -0400, Javier Martinez Canillas wrote: > The exynos5 I2C controller driver always prepares and enables a clock > before using it and then disables unprepares it when the clock is not > used anymore. > > But this can cause a possible ABBA deadlock in some scenarios since a > driver that uses regmap to access its I2C registers, will first grab > the regmap lock and then the I2C xfer function will grab the prepare > lock when preparing the I2C clock. But since the clock driver also > uses regmap for I2C accesses, preparing a clock will first grab the > prepare lock and then the regmap lock when using the regmap API. > > An example of this happens on the Exynos5422 Odroid XU4 board where a > s2mps11 PMIC is used and both the s2mps11 regulators and clk drivers > share the same I2C regmap. > > The possible deadlock is reported by the kernel lockdep: > > Possible unsafe locking scenario: > > CPU0 CPU1 > ---- ---- > lock(sec_core:428:(regmap)->lock); > lock(prepare_lock); > lock(sec_core:428:(regmap)->lock); > lock(prepare_lock); > > *** DEADLOCK *** > > Fix it by leaving the code prepared on probe and use {en,dis}able in > the I2C transfer function. > > This patch is similar to commit 34e81ad5f0b6 ("i2c: s3c2410: fix ABBA > deadlock by keeping clock prepared") that fixes the same bug in other > driver for an I2C controller found in Samsung SoCs. > > Reported-by: Anand Moon > Signed-off-by: Javier Martinez Canillas > Reviewed-by: Anand Moon > Tested-by: Anand Moon > > --- > > Changes in v2: > - Add Anand Moon's Reviewed and Tested by tags. > - Unprepare the clock on suspend. Suggested by Krzysztof Kozlowski. > > drivers/i2c/busses/i2c-exynos5.c | 14 +++++++++++--- > 1 file changed, 11 insertions(+), 3 deletions(-) Hi, Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof From mboxrd@z Thu Jan 1 00:00:00 1970 From: k.kozlowski@samsung.com (Krzysztof Kozlowski) Date: Sun, 17 Apr 2016 15:34:21 +0200 Subject: [PATCH v2 2/2] i2c: exynos5: Fix possible ABBA deadlock by keeping I2C clock prepared In-Reply-To: <1460855693-28225-2-git-send-email-javier@osg.samsung.com> References: <1460855693-28225-1-git-send-email-javier@osg.samsung.com> <1460855693-28225-2-git-send-email-javier@osg.samsung.com> Message-ID: <20160417133421.GC3620@kozik-lap> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Sat, Apr 16, 2016 at 09:14:53PM -0400, Javier Martinez Canillas wrote: > The exynos5 I2C controller driver always prepares and enables a clock > before using it and then disables unprepares it when the clock is not > used anymore. > > But this can cause a possible ABBA deadlock in some scenarios since a > driver that uses regmap to access its I2C registers, will first grab > the regmap lock and then the I2C xfer function will grab the prepare > lock when preparing the I2C clock. But since the clock driver also > uses regmap for I2C accesses, preparing a clock will first grab the > prepare lock and then the regmap lock when using the regmap API. > > An example of this happens on the Exynos5422 Odroid XU4 board where a > s2mps11 PMIC is used and both the s2mps11 regulators and clk drivers > share the same I2C regmap. > > The possible deadlock is reported by the kernel lockdep: > > Possible unsafe locking scenario: > > CPU0 CPU1 > ---- ---- > lock(sec_core:428:(regmap)->lock); > lock(prepare_lock); > lock(sec_core:428:(regmap)->lock); > lock(prepare_lock); > > *** DEADLOCK *** > > Fix it by leaving the code prepared on probe and use {en,dis}able in > the I2C transfer function. > > This patch is similar to commit 34e81ad5f0b6 ("i2c: s3c2410: fix ABBA > deadlock by keeping clock prepared") that fixes the same bug in other > driver for an I2C controller found in Samsung SoCs. > > Reported-by: Anand Moon > Signed-off-by: Javier Martinez Canillas > Reviewed-by: Anand Moon > Tested-by: Anand Moon > > --- > > Changes in v2: > - Add Anand Moon's Reviewed and Tested by tags. > - Unprepare the clock on suspend. Suggested by Krzysztof Kozlowski. > > drivers/i2c/busses/i2c-exynos5.c | 14 +++++++++++--- > 1 file changed, 11 insertions(+), 3 deletions(-) Hi, Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof