mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Peter Rosin <peda@axentia.se>
To: <linux-kernel@vger.kernel.org>
Cc: Peter Rosin <peda@axentia.se>, Wolfram Sang <wsa@the-dreams.de>,
	Jonathan Corbet <corbet@lwn.net>,
	Peter Korsgaard <peter.korsgaard@barco.com>,
	Guenter Roeck <linux@roeck-us.net>,
	Jonathan Cameron <jic23@kernel.org>,
	Hartmut Knaack <knaack.h@gmx.de>,
	Lars-Peter Clausen <lars@metafoo.de>,
	Peter Meerwald <pmeerw@pmeerw.net>,
	Antti Palosaari <crope@iki.fi>,
	Mauro Carvalho Chehab <mchehab@osg.samsung.com>,
	Rob Herring <robh+dt@kernel.org>,
	Frank Rowand <frowand.list@gmail.com>,
	Grant Likely <grant.likely@linaro.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	"David S. Miller" <davem@davemloft.net>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Kalle Valo <kvalo@codeaurora.org>, Jiri Slaby <jslaby@suse.com>,
	Daniel Baluta <daniel.baluta@intel.com>,
	Lucas De Marchi <lucas.demarchi@intel.com>,
	Matt Ranostay <matt.ranostay@intel.com>,
	Krzysztof Kozlowski <k.kozlowski@samsung.com>,
	Hans Verkuil <hans.verkuil@cisco.com>,
	Terry Heo <terryheo@google.com>, Arnd Bergmann <arnd@arndb.de>,
	Tommi Rantala <tt.rantala@gmail.com>,
	Crestez Dan Leonard <leonard.crestez@intel.com>,
	<linux-i2c@vger.kernel.org>, <linux-doc@vger.kernel.org>,
	<linux-iio@vger.kernel.org>, <linux-media@vger.kernel.org>,
	<devicetree@vger.kernel.org>
Subject: [PATCH v9 5/9] iio: imu: inv_mpu6050: change the i2c gate to be mux-locked
Date: Wed, 4 May 2016 22:15:31 +0200	[thread overview]
Message-ID: <1462392935-28011-6-git-send-email-peda@axentia.se> (raw)
In-Reply-To: <1462392935-28011-1-git-send-email-peda@axentia.se>

The root i2c adapter lock is then no longer held by the i2c mux during
accesses behind the i2c gate, and such accesses need to take that lock
just like any other ordinary i2c accesses do.

So, declare the i2c gate mux-locked, and zap the code that makes the
unlocked i2c accesses and just use ordinary regmap_write accesses.

This also happens to fix the deadlock described in
http://patchwork.ozlabs.org/patch/584776/ authored by
Adriana Reus <adriana.reus@intel.com> and submitted by
Daniel Baluta <daniel.baluta@intel.com>

----------8<----------
iio: imu: inv_mpu6050: Fix deadlock between i2c adapter lock and mpu lock

This deadlock occurs if the accel/gyro and the sensor on the auxiliary
I2C (in my setup it's an ak8975) are working at the same time.

Scenario:

      T1					T2
     ====				       ====
inv_mpu6050_read_fifo                  aux sensor op (eg. ak8975_read_raw)
        |                                     |
mutex_lock(&indio_dev->mlock)           i2c_transfer
        |                                     |
i2c transaction                         i2c adapter lock
        |                                     |
i2c adapter lock                        i2c_mux_master_xfer
                                              |
                                        inv_mpu6050_select_bypass
                                              |
                                        mutex_lock(&indio_dev->mlock)

When we operate on an mpu sensor the order of locking is mpu lock
followed by the i2c adapter lock. However, when we operate the auxiliary
sensor the order of locking is the other way around.

...
----------8<----------

The reason this patch fixes the deadlock is that T2 does not grab the
i2c adapter lock until the very end (and grabs the newfangled i2c mux
lock where it previously grabbed the i2c adapter lock).

Acked-by: Jonathan Cameron <jic23@kernel.org>
Acked-by: Daniel Baluta <daniel.baluta@intel.com>
Tested-by: Crestez Dan Leonard <leonard.crestez@intel.com>
Signed-off-by: Peter Rosin <peda@axentia.se>
---
 Documentation/i2c/i2c-topology            |  2 +-
 drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c | 52 ++++++-------------------------
 2 files changed, 11 insertions(+), 43 deletions(-)

diff --git a/Documentation/i2c/i2c-topology b/Documentation/i2c/i2c-topology
index 27bfd682808d..69b008518454 100644
--- a/Documentation/i2c/i2c-topology
+++ b/Documentation/i2c/i2c-topology
@@ -50,7 +50,7 @@ i2c-mux-pinctrl           Normally parent-locked, mux-locked iff
 i2c-mux-reg               Parent-locked
 
 In drivers/iio/
-imu/inv_mpu6050/          Parent-locked
+imu/inv_mpu6050/          Mux-locked
 
 In drivers/media/
 dvb-frontends/m88ds3103   Parent-locked
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
index 3a078df84224..e25f7ef7f0ea 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
@@ -24,45 +24,16 @@ static const struct regmap_config inv_mpu_regmap_config = {
 	.val_bits = 8,
 };
 
-/*
- * The i2c read/write needs to happen in unlocked mode. As the parent
- * adapter is common. If we use locked versions, it will fail as
- * the mux adapter will lock the parent i2c adapter, while calling
- * select/deselect functions.
- */
-static int inv_mpu6050_write_reg_unlocked(struct i2c_client *client,
-					  u8 reg, u8 d)
-{
-	int ret;
-	u8 buf[2] = {reg, d};
-	struct i2c_msg msg[1] = {
-		{
-			.addr = client->addr,
-			.flags = 0,
-			.len = sizeof(buf),
-			.buf = buf,
-		}
-	};
-
-	ret = __i2c_transfer(client->adapter, msg, 1);
-	if (ret != 1)
-		return ret;
-
-	return 0;
-}
-
 static int inv_mpu6050_select_bypass(struct i2c_mux_core *muxc, u32 chan_id)
 {
-	struct i2c_client *client = i2c_mux_priv(muxc);
-	struct iio_dev *indio_dev = dev_get_drvdata(&client->dev);
+	struct iio_dev *indio_dev = i2c_mux_priv(muxc);
 	struct inv_mpu6050_state *st = iio_priv(indio_dev);
 	int ret = 0;
 
 	/* Use the same mutex which was used everywhere to protect power-op */
 	mutex_lock(&indio_dev->mlock);
 	if (!st->powerup_count) {
-		ret = inv_mpu6050_write_reg_unlocked(client,
-						     st->reg->pwr_mgmt_1, 0);
+		ret = regmap_write(st->map, st->reg->pwr_mgmt_1, 0);
 		if (ret)
 			goto write_error;
 
@@ -71,10 +42,9 @@ static int inv_mpu6050_select_bypass(struct i2c_mux_core *muxc, u32 chan_id)
 	}
 	if (!ret) {
 		st->powerup_count++;
-		ret = inv_mpu6050_write_reg_unlocked(client,
-						     st->reg->int_pin_cfg,
-						     INV_MPU6050_INT_PIN_CFG |
-						     INV_MPU6050_BIT_BYPASS_EN);
+		ret = regmap_write(st->map, st->reg->int_pin_cfg,
+				   INV_MPU6050_INT_PIN_CFG |
+				   INV_MPU6050_BIT_BYPASS_EN);
 	}
 write_error:
 	mutex_unlock(&indio_dev->mlock);
@@ -84,18 +54,16 @@ write_error:
 
 static int inv_mpu6050_deselect_bypass(struct i2c_mux_core *muxc, u32 chan_id)
 {
-	struct i2c_client *client = i2c_mux_priv(muxc);
-	struct iio_dev *indio_dev = dev_get_drvdata(&client->dev);
+	struct iio_dev *indio_dev = i2c_mux_priv(muxc);
 	struct inv_mpu6050_state *st = iio_priv(indio_dev);
 
 	mutex_lock(&indio_dev->mlock);
 	/* It doesn't really mattter, if any of the calls fails */
-	inv_mpu6050_write_reg_unlocked(client, st->reg->int_pin_cfg,
-				       INV_MPU6050_INT_PIN_CFG);
+	regmap_write(st->map, st->reg->int_pin_cfg, INV_MPU6050_INT_PIN_CFG);
 	st->powerup_count--;
 	if (!st->powerup_count)
-		inv_mpu6050_write_reg_unlocked(client, st->reg->pwr_mgmt_1,
-					       INV_MPU6050_BIT_SLEEP);
+		regmap_write(st->map, st->reg->pwr_mgmt_1,
+			     INV_MPU6050_BIT_SLEEP);
 	mutex_unlock(&indio_dev->mlock);
 
 	return 0;
@@ -134,7 +102,7 @@ static int inv_mpu_probe(struct i2c_client *client,
 
 	st = iio_priv(dev_get_drvdata(&client->dev));
 	st->muxc = i2c_mux_alloc(client->adapter, &client->dev,
-				 1, 0, 0,
+				 1, 0, I2C_MUX_LOCKED,
 				 inv_mpu6050_select_bypass,
 				 inv_mpu6050_deselect_bypass);
 	if (!st->muxc) {
-- 
2.1.4

  parent reply	other threads:[~2016-05-04 20:18 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-04 20:15 [PATCH v9 0/9] i2c mux cleanup and locking update Peter Rosin
2016-05-04 20:15 ` [PATCH v9 1/9] i2c: allow adapter drivers to override the adapter locking Peter Rosin
2016-05-04 20:15 ` [PATCH v9 2/9] i2c: muxes always lock the parent adapter Peter Rosin
2016-05-04 20:15 ` [PATCH v9 3/9] i2c-mux: relax locking of the top i2c adapter during mux-locked muxing Peter Rosin
2016-05-04 20:15 ` [PATCH v9 4/9] i2c-mux: document i2c muxes and elaborate on parent-/mux-locked muxes Peter Rosin
2016-05-04 20:15 ` Peter Rosin [this message]
2016-05-04 20:15 ` [PATCH v9 6/9] [media] si2168: change the i2c gate to be mux-locked Peter Rosin
2016-05-04 20:15 ` [PATCH v9 7/9] [media] rtl2832: " Peter Rosin
2016-05-04 20:15 ` [PATCH v9 8/9] [media] rtl2832_sdr: get rid of empty regmap wrappers Peter Rosin
2016-05-04 20:15 ` [PATCH v9 9/9] [media] rtl2832: regmap is aware of lockdep, drop local locking hack Peter Rosin
2016-05-04 20:42 ` [PATCH v9 0/9] i2c mux cleanup and locking update Wolfram Sang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1462392935-28011-6-git-send-email-peda@axentia.se \
    --to=peda@axentia.se \
    --cc=akpm@linux-foundation.org \
    --cc=arnd@arndb.de \
    --cc=corbet@lwn.net \
    --cc=crope@iki.fi \
    --cc=daniel.baluta@intel.com \
    --cc=davem@davemloft.net \
    --cc=devicetree@vger.kernel.org \
    --cc=frowand.list@gmail.com \
    --cc=grant.likely@linaro.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=hans.verkuil@cisco.com \
    --cc=jic23@kernel.org \
    --cc=jslaby@suse.com \
    --cc=k.kozlowski@samsung.com \
    --cc=knaack.h@gmx.de \
    --cc=kvalo@codeaurora.org \
    --cc=lars@metafoo.de \
    --cc=leonard.crestez@intel.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-i2c@vger.kernel.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=lucas.demarchi@intel.com \
    --cc=matt.ranostay@intel.com \
    --cc=mchehab@osg.samsung.com \
    --cc=peter.korsgaard@barco.com \
    --cc=pmeerw@pmeerw.net \
    --cc=robh+dt@kernel.org \
    --cc=terryheo@google.com \
    --cc=tt.rantala@gmail.com \
    --cc=wsa@the-dreams.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®