All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hans de Goede <hdegoede@redhat.com>
To: Jonathan Cameron <jic23@kernel.org>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Mark Brown <broonie@kernel.org>
Cc: Hans de Goede <hdegoede@redhat.com>,
	Lars-Peter Clausen <lars@metafoo.de>,
	Jeremy Cline <jeremy@jcline.org>,
	linux-iio@vger.kernel.org,
	Charles Keepax <ckeepax@opensource.cirrus.com>,
	patches@opensource.cirrus.com, alsa-devel@alsa-project.org
Subject: [PATCH 2/8] iio: accel: bmc150: Don't make the remove function of the second accelerometer unregister itself
Date: Fri, 21 May 2021 19:14:12 +0200	[thread overview]
Message-ID: <20210521171418.393871-3-hdegoede@redhat.com> (raw)
In-Reply-To: <20210521171418.393871-1-hdegoede@redhat.com>

On machines with dual accelerometers described in a single ACPI fwnode,
the bmc150_accel_probe() instantiates a second i2c-client for the second
accelerometer.

A pointer to this manually instantiated second i2c-client is stored
inside the iio_dev's private-data through bmc150_set_second_device(),
so that the i2c-client can be unregistered from bmc150_accel_remove().

Before this commit bmc150_set_second_device() took only 1 argument so it
would store the pointer in private-data of the iio_dev belonging to the
manually instantiated i2c-client, leading to the bmc150_accel_remove()
call for the second_dev trying to unregister *itself* while it was
being removed, leading to a deadlock and rmmod hanging.

Change bmc150_set_second_device() to take 2 arguments: 1. The i2c-client
which is instantiating the second i2c-client for the 2nd accelerometer and
2. The second-device pointer itself (which also is an i2c-client).

This will store the second_device pointer in the private data of the
iio_dev belonging to the (ACPI instantiated) i2c-client for the first
accelerometer and will make bmc150_accel_remove() unregister the
second_device i2c-client when called for the first client,
avoiding the deadlock.

Fixes: 5bfb3a4bd8f6 ("iio: accel: bmc150: Check for a second ACPI device for BOSC0200")
Cc: Jeremy Cline <jeremy@jcline.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/iio/accel/bmc150-accel-core.c | 4 ++--
 drivers/iio/accel/bmc150-accel-i2c.c  | 2 +-
 drivers/iio/accel/bmc150-accel.h      | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/accel/bmc150-accel-core.c b/drivers/iio/accel/bmc150-accel-core.c
index 3a3f67930165..8ff358c37a81 100644
--- a/drivers/iio/accel/bmc150-accel-core.c
+++ b/drivers/iio/accel/bmc150-accel-core.c
@@ -1815,11 +1815,11 @@ struct i2c_client *bmc150_get_second_device(struct i2c_client *client)
 }
 EXPORT_SYMBOL_GPL(bmc150_get_second_device);
 
-void bmc150_set_second_device(struct i2c_client *client)
+void bmc150_set_second_device(struct i2c_client *client, struct i2c_client *second_dev)
 {
 	struct bmc150_accel_data *data = iio_priv(i2c_get_clientdata(client));
 
-	data->second_device = client;
+	data->second_device = second_dev;
 }
 EXPORT_SYMBOL_GPL(bmc150_set_second_device);
 
diff --git a/drivers/iio/accel/bmc150-accel-i2c.c b/drivers/iio/accel/bmc150-accel-i2c.c
index 69f709319484..2afaae0294ee 100644
--- a/drivers/iio/accel/bmc150-accel-i2c.c
+++ b/drivers/iio/accel/bmc150-accel-i2c.c
@@ -70,7 +70,7 @@ static int bmc150_accel_probe(struct i2c_client *client,
 
 		second_dev = i2c_acpi_new_device(&client->dev, 1, &board_info);
 		if (!IS_ERR(second_dev))
-			bmc150_set_second_device(second_dev);
+			bmc150_set_second_device(client, second_dev);
 	}
 #endif
 
diff --git a/drivers/iio/accel/bmc150-accel.h b/drivers/iio/accel/bmc150-accel.h
index 6024f15b9700..e30c1698f6fb 100644
--- a/drivers/iio/accel/bmc150-accel.h
+++ b/drivers/iio/accel/bmc150-accel.h
@@ -18,7 +18,7 @@ int bmc150_accel_core_probe(struct device *dev, struct regmap *regmap, int irq,
 			    const char *name, bool block_supported);
 int bmc150_accel_core_remove(struct device *dev);
 struct i2c_client *bmc150_get_second_device(struct i2c_client *second_device);
-void bmc150_set_second_device(struct i2c_client *second_device);
+void bmc150_set_second_device(struct i2c_client *client, struct i2c_client *second_dev);
 extern const struct dev_pm_ops bmc150_accel_pm_ops;
 extern const struct regmap_config bmc150_regmap_conf;
 
-- 
2.31.1


WARNING: multiple messages have this Message-ID (diff)
From: Hans de Goede <hdegoede@redhat.com>
To: Jonathan Cameron <jic23@kernel.org>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Mark Brown <broonie@kernel.org>
Cc: alsa-devel@alsa-project.org, Lars-Peter Clausen <lars@metafoo.de>,
	Charles Keepax <ckeepax@opensource.cirrus.com>,
	linux-iio@vger.kernel.org, patches@opensource.cirrus.com,
	Jeremy Cline <jeremy@jcline.org>,
	Hans de Goede <hdegoede@redhat.com>
Subject: [PATCH 2/8] iio: accel: bmc150: Don't make the remove function of the second accelerometer unregister itself
Date: Fri, 21 May 2021 19:14:12 +0200	[thread overview]
Message-ID: <20210521171418.393871-3-hdegoede@redhat.com> (raw)
In-Reply-To: <20210521171418.393871-1-hdegoede@redhat.com>

On machines with dual accelerometers described in a single ACPI fwnode,
the bmc150_accel_probe() instantiates a second i2c-client for the second
accelerometer.

A pointer to this manually instantiated second i2c-client is stored
inside the iio_dev's private-data through bmc150_set_second_device(),
so that the i2c-client can be unregistered from bmc150_accel_remove().

Before this commit bmc150_set_second_device() took only 1 argument so it
would store the pointer in private-data of the iio_dev belonging to the
manually instantiated i2c-client, leading to the bmc150_accel_remove()
call for the second_dev trying to unregister *itself* while it was
being removed, leading to a deadlock and rmmod hanging.

Change bmc150_set_second_device() to take 2 arguments: 1. The i2c-client
which is instantiating the second i2c-client for the 2nd accelerometer and
2. The second-device pointer itself (which also is an i2c-client).

This will store the second_device pointer in the private data of the
iio_dev belonging to the (ACPI instantiated) i2c-client for the first
accelerometer and will make bmc150_accel_remove() unregister the
second_device i2c-client when called for the first client,
avoiding the deadlock.

Fixes: 5bfb3a4bd8f6 ("iio: accel: bmc150: Check for a second ACPI device for BOSC0200")
Cc: Jeremy Cline <jeremy@jcline.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/iio/accel/bmc150-accel-core.c | 4 ++--
 drivers/iio/accel/bmc150-accel-i2c.c  | 2 +-
 drivers/iio/accel/bmc150-accel.h      | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/accel/bmc150-accel-core.c b/drivers/iio/accel/bmc150-accel-core.c
index 3a3f67930165..8ff358c37a81 100644
--- a/drivers/iio/accel/bmc150-accel-core.c
+++ b/drivers/iio/accel/bmc150-accel-core.c
@@ -1815,11 +1815,11 @@ struct i2c_client *bmc150_get_second_device(struct i2c_client *client)
 }
 EXPORT_SYMBOL_GPL(bmc150_get_second_device);
 
-void bmc150_set_second_device(struct i2c_client *client)
+void bmc150_set_second_device(struct i2c_client *client, struct i2c_client *second_dev)
 {
 	struct bmc150_accel_data *data = iio_priv(i2c_get_clientdata(client));
 
-	data->second_device = client;
+	data->second_device = second_dev;
 }
 EXPORT_SYMBOL_GPL(bmc150_set_second_device);
 
diff --git a/drivers/iio/accel/bmc150-accel-i2c.c b/drivers/iio/accel/bmc150-accel-i2c.c
index 69f709319484..2afaae0294ee 100644
--- a/drivers/iio/accel/bmc150-accel-i2c.c
+++ b/drivers/iio/accel/bmc150-accel-i2c.c
@@ -70,7 +70,7 @@ static int bmc150_accel_probe(struct i2c_client *client,
 
 		second_dev = i2c_acpi_new_device(&client->dev, 1, &board_info);
 		if (!IS_ERR(second_dev))
-			bmc150_set_second_device(second_dev);
+			bmc150_set_second_device(client, second_dev);
 	}
 #endif
 
diff --git a/drivers/iio/accel/bmc150-accel.h b/drivers/iio/accel/bmc150-accel.h
index 6024f15b9700..e30c1698f6fb 100644
--- a/drivers/iio/accel/bmc150-accel.h
+++ b/drivers/iio/accel/bmc150-accel.h
@@ -18,7 +18,7 @@ int bmc150_accel_core_probe(struct device *dev, struct regmap *regmap, int irq,
 			    const char *name, bool block_supported);
 int bmc150_accel_core_remove(struct device *dev);
 struct i2c_client *bmc150_get_second_device(struct i2c_client *second_device);
-void bmc150_set_second_device(struct i2c_client *second_device);
+void bmc150_set_second_device(struct i2c_client *client, struct i2c_client *second_dev);
 extern const struct dev_pm_ops bmc150_accel_pm_ops;
 extern const struct regmap_config bmc150_regmap_conf;
 
-- 
2.31.1


  parent reply	other threads:[~2021-05-21 17:14 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-21 17:14 [PATCH 0/8] iio: accel: bmc150: Add support for yoga's with dual accelerometers with an ACPI HID of DUAL250E Hans de Goede
2021-05-21 17:14 ` Hans de Goede
2021-05-21 17:14 ` [PATCH 1/8] iio: accel: bmc150: Fix dereferencing the wrong pointer in bmc150_get/set_second_device Hans de Goede
2021-05-21 17:14   ` Hans de Goede
2021-05-21 17:14 ` Hans de Goede [this message]
2021-05-21 17:14   ` [PATCH 2/8] iio: accel: bmc150: Don't make the remove function of the second accelerometer unregister itself Hans de Goede
2021-05-22 18:06   ` Andy Shevchenko
2021-05-22 18:06     ` Andy Shevchenko
2021-05-22 18:10     ` Hans de Goede
2021-05-22 18:10       ` Hans de Goede
2021-05-21 17:14 ` [PATCH 3/8] iio: accel: bmc150: Move check for second ACPI device into a separate function Hans de Goede
2021-05-21 17:14   ` Hans de Goede
2021-05-22 17:37   ` Jonathan Cameron
2021-05-22 17:37     ` Jonathan Cameron
2021-05-22 17:39     ` Hans de Goede
2021-05-22 17:39       ` Hans de Goede
2021-05-22 18:09   ` Andy Shevchenko
2021-05-22 18:09     ` Andy Shevchenko
2021-05-22 18:57     ` Hans de Goede
2021-05-22 18:57       ` Hans de Goede
2021-05-21 17:14 ` [PATCH 4/8] iio: accel: bmc150: Add support for dual-accelerometers with a DUAL250E HID Hans de Goede
2021-05-21 17:14   ` Hans de Goede
2021-05-22 17:43   ` Jonathan Cameron
2021-05-22 17:43     ` Jonathan Cameron
2021-05-22 17:44     ` Hans de Goede
2021-05-22 17:44       ` Hans de Goede
2021-05-22 17:56       ` Jonathan Cameron
2021-05-22 17:56         ` Jonathan Cameron
2021-05-22 18:11   ` Andy Shevchenko
2021-05-22 18:11     ` Andy Shevchenko
2021-05-22 18:59     ` Hans de Goede
2021-05-22 18:59       ` Hans de Goede
2021-05-21 17:14 ` [PATCH 5/8] iio: accel: bmc150: Move struct bmc150_accel_data definition to bmc150-accel.h Hans de Goede
2021-05-21 17:14   ` Hans de Goede
2021-05-21 17:14 ` [PATCH 6/8] iio: accel: bmc150: Remove bmc150_set/get_second_device() accessor functions Hans de Goede
2021-05-21 17:14   ` Hans de Goede
2021-05-21 17:14 ` [PATCH 7/8] iio: accel: bmc150: Add support for DUAL250E ACPI DSM for setting the hinge angle Hans de Goede
2021-05-21 17:14   ` Hans de Goede
2021-05-22 17:53   ` Jonathan Cameron
2021-05-22 17:53     ` Jonathan Cameron
2021-05-22 18:21   ` Andy Shevchenko
2021-05-22 18:21     ` Andy Shevchenko
2021-05-22 19:02     ` Hans de Goede
2021-05-22 19:02       ` Hans de Goede
2021-05-21 17:14 ` [PATCH 8/8] iio: accel: bmc150: Set label based on accel-location for ACPI DUAL250E fwnodes Hans de Goede
2021-05-21 17:14   ` Hans de Goede
2021-05-22 18:34   ` Andy Shevchenko
2021-05-22 18:34     ` Andy Shevchenko
2021-05-22 19:04     ` Hans de Goede
2021-05-22 19:04       ` Hans de Goede
2021-05-22 18:01 ` [PATCH 0/8] iio: accel: bmc150: Add support for yoga's with dual accelerometers with an ACPI HID of DUAL250E Jonathan Cameron
2021-05-22 18:01   ` Jonathan Cameron
2021-05-22 18:03   ` Andy Shevchenko
2021-05-22 18:03     ` Andy Shevchenko
2021-05-22 18:49     ` Jonathan Cameron
2021-05-22 18:49       ` Jonathan Cameron
2021-05-23 11:05   ` Hans de Goede
2021-05-23 11:05     ` Hans de Goede

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=20210521171418.393871-3-hdegoede@redhat.com \
    --to=hdegoede@redhat.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@kernel.org \
    --cc=ckeepax@opensource.cirrus.com \
    --cc=jeremy@jcline.org \
    --cc=jic23@kernel.org \
    --cc=lars@metafoo.de \
    --cc=lgirdwood@gmail.com \
    --cc=linux-iio@vger.kernel.org \
    --cc=patches@opensource.cirrus.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.