Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1370151 > unrolled thread
| Started by | Peter Rosin <peda@lysator.liu.se> |
|---|---|
| First post | 2016-04-03 11:00 +0200 |
| Last post | 2016-04-11 18:00 +0200 |
| Articles | 10 — 3 participants |
Back to article view | Back to linux.kernel
[PATCH v6 00/24] i2c mux cleanup and locking update Peter Rosin <peda@lysator.liu.se> - 2016-04-03 11:00 +0200
[PATCH v6 08/24] iio: imu: inv_mpu6050: convert to use an explicit i2c mux core Peter Rosin <peda@lysator.liu.se> - 2016-04-03 11:00 +0200
Re: [PATCH v6 08/24] iio: imu: inv_mpu6050: convert to use an explicit i2c mux core Jonathan Cameron <jic23@kernel.org> - 2016-04-03 13:00 +0200
Re: [PATCH v6 08/24] iio: imu: inv_mpu6050: convert to use an explicit i2c mux core Peter Rosin <peda@lysator.liu.se> - 2016-04-03 14:00 +0200
Re: [PATCH v6 08/24] iio: imu: inv_mpu6050: convert to use an explicit i2c mux core Jonathan Cameron <jic23@kernel.org> - 2016-04-10 16:20 +0200
[PATCH v6 02/24] i2c: i2c-mux-gpio: convert to use an explicit i2c mux core Peter Rosin <peda@lysator.liu.se> - 2016-04-03 11:00 +0200
[PATCH v6 21/24] [media] si2168: change the i2c gate to be mux-locked Peter Rosin <peda@lysator.liu.se> - 2016-04-03 11:00 +0200
Re: [PATCH v6 00/24] i2c mux cleanup and locking update Wolfram Sang <wsa@the-dreams.de> - 2016-04-11 14:50 +0200
Re: [PATCH v6 00/24] i2c mux cleanup and locking update Peter Rosin <peda@lysator.liu.se> - 2016-04-11 15:40 +0200
Re: [PATCH v6 00/24] i2c mux cleanup and locking update Wolfram Sang <wsa@the-dreams.de> - 2016-04-11 18:00 +0200
| From | Peter Rosin <peda@lysator.liu.se> |
|---|---|
| Date | 2016-04-03 11:00 +0200 |
| Subject | [PATCH v6 00/24] i2c mux cleanup and locking update |
| Message-ID | <rjM30-Ns-3@gated-at.bofh.it> |
From: Peter Rosin <peda@axentia.se>
Hi!
I have a pair of boards with this i2c topology:
GPIO ---| ------ BAT1
| v /
I2C -----+------B---+---- MUX
| \
EEPROM ------ BAT2
(B denotes the boundary between the boards)
The problem with this is that the GPIO controller sits on the same i2c bus
that it MUXes. For pca954x devices this is worked around by using unlocked
transfers when updating the MUX. I have no such luck as the GPIO is a general
purpose IO expander and the MUX is just a random bidirectional MUX, unaware
of the fact that it is muxing an i2c bus. Extending unlocked transfers
into the GPIO subsystem is too ugly to even think about. But the general hw
approach is sane in my opinion, with the number of connections between the
two boards minimized. To put it plainly, I need support for it.
So, I observe that while it is needed to have the i2c bus locked during the
actual MUX update in order to avoid random garbage on the slave side, it
is not strictly a must to have it locked over the whole sequence of a full
select-transfer-deselect operation. The MUX itself needs to be locked, so
transfers to clients behind the mux are serialized, and the MUX needs to be
stable during all i2c traffic (otherwise individual mux slave segments
might see garbage).
This series accomplishes this by adding code to i2c-mux-gpio and
i2c-mux-pinctrl that determines if all involved devices used to update the
mux are controlled by the same root i2c adapter that is muxed. When this
is the case, the select-transfer-deselect operations should be locked
individually to avoid the deadlock. The i2c bus *is* still locked
during muxing, since the muxing happens as part of i2c transfers. This
is true even if the MUX is updated with several transfers to the GPIO (at
least as long as *all* MUX changes are using the i2c master bus). A lock
is added to i2c adapters that muxes on that adapter grab, so that transfers
through the muxes are serialized.
Concerns:
- The locking is perhaps too complex?
- I worry about the priority inheritance aspect of the adapter lock. When
the transfers behind the mux are divided into select-transfer-deselect all
locked individually, low priority transfers get more chances to interfere
with high priority transfers.
- When doing an i2c_transfer() in_atomic() context or with irqs_disabled(),
there is a higher possibility that the mux is not returned to its idle
state after a failed (-EAGAIN) transfer due to trylock.
- Is the detection of i2c-controlled gpios and pinctrls sane (i.e. the
usage of the new i2c_root_adapter() function in 18/24)?
To summarize the series, there's some i2c-mux infrastructure cleanup work
first (I think that part stands by itself as desireable regardless), the
locking changes are in 16/24 and after with the real meat in 18/24. There
is some documentation added in 19/24 while 20/24 and after are cleanups to
existing drivers utilizing the new stuff.
PS. needs a bunch of testing, I do not have access to all the involved hw.
Specifically, thank you Antti for testing v5, but I did not add any
Tested-by for v6 since I moved the lock from the mux itself to the mux
parent adapter. I did this to cope with the situation I described in
http://marc.info/?l=linux-i2c&m=145875234525803&w=2
thus making it possible to get rid of the unlocked accesses in the
si2168 driver (patch 21/24). I also didn't add any Reviewed-by for
the parts of the rtl2832 driver changes that suffered the most from
driver updates since v4.5-rc7, so please review that again as well.
This series can also be pulled from github, if that is preferred:
---------------------
The following changes since commit f55532a0c0b8bb6148f4e07853b876ef73bc69ca:
Linux 4.6-rc1 (2016-03-26 16:03:24 -0700)
are available in the git repository at:
https://github.com/peda-r/i2c-mux.git mux-core-and-locking-6
for you to fetch changes up to 81830e43de2bc849848b939166103217ac444df5:
[media] rtl2832: regmap is aware of lockdep, drop local locking hack (2016-04-03 09:35:52 +0200)
---------------------
v6 compared to v5:
- Rebase on top of v4.6-rc1
- Adjust to gpio subsystem overhaul.
- Adjust to changes in the inv_mpu6050 driver.
- Adjust to changes in the rtl2832 driver.
- Fix some new trivial checkpatch issues.
- Rename "self-locked" muxes "mux-locked" instead, since the lock has
been moved to the parent adapter and is common for all muxes with
the same parent adapter. The advantage is that address collisions
behind sibling muxes are handled. Parent-locked muxes also grab this
new mux-lock so that parent-locked and mux-locked siblings interact
better.
- Firmware mutex added to the si2168 driver.
v5 compared to v4 (only published as a git branch):
- Rebase on top of v4.5-rc7.
- A new patch making me maintainer of i2c muxes (also sent separately).
- A new file Documentation/i2c/i2c-topology that describes various muxing
issues.
- Rename "i2c-controlled" muxes "self-locked" instead, as it is perfectly
reasonable to have i2c-controlled muxes that use the pre-existing locking
scheme. The pre-existing locking scheme for i2c muxes is from here on
called "parent-locked".
- Rename i2c-mux.c:i2c_mux_master_xfer to __i2c_mux_master_xfer since it
calls __i2c_transfer, which leaves room for a new i2c_mux_master_xfer
that calls i2c_transfer. Similar rename shuffle for i2c_mux_smbus_xfer.
- Use sizeof(*priv) instead of sizeof(struct i2c_mux_priv). One instance.
- Some follow-up patches that were posted in response to v2-v4 cleaning up
and simplifying various i2c muxes outside drivers/i2c/, among those is
an unrelated cleanup patch to drivers/media/dvb-frontends/rtl2832.c that
I carry here since it conflicts (trivially) with this series. That
unrelated patch is (currently) the last patch in the series.
v4 compared to v3:
- Rebase on top of v4.5-rc6.
- Update to add new i2c-mux interfaces in 01/18 including glue to implement
the old interfaces in terms of the new interfaces, then change the
mux users over to the new interfaces one by one (in 02/18 through 14/18),
and finally removing the old interfaces in 15/18. I.e. the first 15
patches of v4 replaces the first 5 patches of v3, with the following
points describing changes in the end result. Each patch is now touching
only one subsystem.
- Rename i2c_add_mux_adapter and i2c_del_mux_adapters to i2c_mux_add_adapter
and i2c_mux_del_adapters (so that the old functions can live on during the
transition).
- Make i2c_mux_alloc take a parent and the select/deselect ops as
arguments. Also add a flags argument to prevent churn later on.
- Add a new interface i2c_mux_one_adapter(). Make use of it in suitable
mux users with a single child adapter.
- Adjust to a rename in struct gpio_chip.
- Update a couple of comments to match the new code.
v3 compared to v2:
- Fix devm_kfree of a NULL pointer in i2c_mux_reserve_adapters().
- Remove device tree "i2c-controlled" property and determine this by walking
the dev tree instead.
- Fix compile problems with inv_mpu_acpi.c
- Wait with adding the client pointer to patch 2/8 for pca9541 and pca954x.
v2 compared to v1:
- Allocate mux core and (optional) priv in a combined allocation.
- Kill dev_err messages triggered by memory allocation failure.
- Fix the device specific i2c muxes that I had overlooked.
- Rebase on top of v4.4-rc8 (was based on v4.4-rc6 previously).
- Drop the last two patches in the series.
Cheers,
Peter
Antti Palosaari (1):
[media] si2168: change the i2c gate to be mux-locked
Peter Rosin (23):
i2c-mux: add common data for every i2c-mux instance
i2c: i2c-mux-gpio: convert to use an explicit i2c mux core
i2c: i2c-mux-pinctrl: convert to use an explicit i2c mux core
i2c: i2c-arb-gpio-challenge: convert to use an explicit i2c mux core
i2c: i2c-mux-pca9541: convert to use an explicit i2c mux core
i2c: i2c-mux-pca954x: convert to use an explicit i2c mux core
i2c: i2c-mux-reg: convert to use an explicit i2c mux core
iio: imu: inv_mpu6050: convert to use an explicit i2c mux core
[media] m88ds3103: convert to use an explicit i2c mux core
[media] rtl2830: convert to use an explicit i2c mux core
[media] rtl2832: convert to use an explicit i2c mux core
[media] si2168: convert to use an explicit i2c mux core
[media] cx231xx: convert to use an explicit i2c mux core
of/unittest: convert to use an explicit i2c mux core
i2c-mux: drop old unused i2c-mux api
i2c: allow adapter drivers to override the adapter locking
i2c: muxes always lock the parent adapter
i2c-mux: relax locking of the top i2c adapter during mux-locked muxing
i2c-mux: document i2c muxes and elaborate on parent-/mux-locked muxes
iio: imu: inv_mpu6050: change the i2c gate to be mux-locked
[media] rtl2832: change the i2c gate to be mux-locked
[media] rtl2832_sdr: get rid of empty regmap wrappers
[media] rtl2832: regmap is aware of lockdep, drop local locking hack
Documentation/i2c/i2c-topology | 370 +++++++++++++++++++++++++++
MAINTAINERS | 1 +
drivers/i2c/i2c-core.c | 66 +++--
drivers/i2c/i2c-mux.c | 350 ++++++++++++++++++++-----
drivers/i2c/muxes/i2c-arb-gpio-challenge.c | 47 ++--
drivers/i2c/muxes/i2c-mux-gpio.c | 72 +++---
drivers/i2c/muxes/i2c-mux-pca9541.c | 55 ++--
drivers/i2c/muxes/i2c-mux-pca954x.c | 64 ++---
drivers/i2c/muxes/i2c-mux-pinctrl.c | 124 +++++----
drivers/i2c/muxes/i2c-mux-reg.c | 63 ++---
drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c | 2 +-
drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 1 -
drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c | 78 ++----
drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h | 3 +-
drivers/media/dvb-frontends/m88ds3103.c | 18 +-
drivers/media/dvb-frontends/m88ds3103_priv.h | 2 +-
drivers/media/dvb-frontends/rtl2830.c | 17 +-
drivers/media/dvb-frontends/rtl2830_priv.h | 2 +-
drivers/media/dvb-frontends/rtl2832.c | 241 +++--------------
drivers/media/dvb-frontends/rtl2832.h | 4 +-
drivers/media/dvb-frontends/rtl2832_priv.h | 3 +-
drivers/media/dvb-frontends/rtl2832_sdr.c | 303 ++++++++++------------
drivers/media/dvb-frontends/rtl2832_sdr.h | 5 +-
drivers/media/dvb-frontends/si2168.c | 103 +++-----
drivers/media/dvb-frontends/si2168_priv.h | 3 +-
drivers/media/usb/cx231xx/cx231xx-core.c | 6 +-
drivers/media/usb/cx231xx/cx231xx-i2c.c | 47 ++--
drivers/media/usb/cx231xx/cx231xx.h | 4 +-
drivers/media/usb/dvb-usb-v2/rtl28xxu.c | 5 +-
drivers/of/unittest.c | 40 ++-
include/linux/i2c-mux.h | 64 ++++-
include/linux/i2c.h | 29 ++-
32 files changed, 1277 insertions(+), 915 deletions(-)
create mode 100644 Documentation/i2c/i2c-topology
--
2.1.4
[toc] | [next] | [standalone]
| From | Peter Rosin <peda@lysator.liu.se> |
|---|---|
| Date | 2016-04-03 11:00 +0200 |
| Subject | [PATCH v6 08/24] iio: imu: inv_mpu6050: convert to use an explicit i2c mux core |
| Message-ID | <rjM32-Ns-49@gated-at.bofh.it> |
| In reply to | #1370151 |
From: Peter Rosin <peda@axentia.se>
Allocate an explicit i2c mux core to handle parent and child adapters
etc. Update the select/deselect ops to be in terms of the i2c mux core
instead of the child adapter.
Signed-off-by: Peter Rosin <peda@axentia.se>
---
drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c | 2 +-
drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 1 -
drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c | 32 +++++++++++++-----------------
drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h | 3 ++-
4 files changed, 17 insertions(+), 21 deletions(-)
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c
index 2771106fd650..f62b8bd9ad7e 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c
@@ -183,7 +183,7 @@ int inv_mpu_acpi_create_mux_client(struct i2c_client *client)
} else
return 0; /* no secondary addr, which is OK */
}
- st->mux_client = i2c_new_device(st->mux_adapter, &info);
+ st->mux_client = i2c_new_device(st->muxc->adapter[0], &info);
if (!st->mux_client)
return -ENODEV;
}
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
index d192953e9a38..0c2bded2b5b7 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
@@ -23,7 +23,6 @@
#include <linux/kfifo.h>
#include <linux/spinlock.h>
#include <linux/iio/iio.h>
-#include <linux/i2c-mux.h>
#include <linux/acpi.h>
#include "inv_mpu_iio.h"
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
index f581256d9d4c..0d429d788106 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
@@ -15,7 +15,6 @@
#include <linux/delay.h>
#include <linux/err.h>
#include <linux/i2c.h>
-#include <linux/i2c-mux.h>
#include <linux/iio/iio.h>
#include <linux/module.h>
#include "inv_mpu_iio.h"
@@ -52,10 +51,9 @@ static int inv_mpu6050_write_reg_unlocked(struct i2c_client *client,
return 0;
}
-static int inv_mpu6050_select_bypass(struct i2c_adapter *adap, void *mux_priv,
- u32 chan_id)
+static int inv_mpu6050_select_bypass(struct i2c_mux_core *muxc, u32 chan_id)
{
- struct i2c_client *client = mux_priv;
+ struct i2c_client *client = i2c_mux_priv(muxc);
struct iio_dev *indio_dev = dev_get_drvdata(&client->dev);
struct inv_mpu6050_state *st = iio_priv(indio_dev);
int ret = 0;
@@ -84,10 +82,9 @@ write_error:
return ret;
}
-static int inv_mpu6050_deselect_bypass(struct i2c_adapter *adap,
- void *mux_priv, u32 chan_id)
+static int inv_mpu6050_deselect_bypass(struct i2c_mux_core *muxc, u32 chan_id)
{
- struct i2c_client *client = mux_priv;
+ struct i2c_client *client = i2c_mux_priv(muxc);
struct iio_dev *indio_dev = dev_get_drvdata(&client->dev);
struct inv_mpu6050_state *st = iio_priv(indio_dev);
@@ -136,16 +133,15 @@ static int inv_mpu_probe(struct i2c_client *client,
return result;
st = iio_priv(dev_get_drvdata(&client->dev));
- st->mux_adapter = i2c_add_mux_adapter(client->adapter,
- &client->dev,
- client,
- 0, 0, 0,
- inv_mpu6050_select_bypass,
- inv_mpu6050_deselect_bypass);
- if (!st->mux_adapter) {
- result = -ENODEV;
+ st->muxc = i2c_mux_one_adapter(client->adapter, &client->dev, 0, 0,
+ 0, 0, 0,
+ inv_mpu6050_select_bypass,
+ inv_mpu6050_deselect_bypass);
+ if (IS_ERR(st->muxc)) {
+ result = PTR_ERR(st->muxc);
goto out_unreg_device;
}
+ st->muxc->priv = client;
result = inv_mpu_acpi_create_mux_client(client);
if (result)
@@ -154,7 +150,7 @@ static int inv_mpu_probe(struct i2c_client *client,
return 0;
out_del_mux:
- i2c_del_mux_adapter(st->mux_adapter);
+ i2c_mux_del_adapters(st->muxc);
out_unreg_device:
inv_mpu_core_remove(&client->dev);
return result;
@@ -162,11 +158,11 @@ out_unreg_device:
static int inv_mpu_remove(struct i2c_client *client)
{
- struct iio_dev *indio_dev = i2c_get_clientdata(client);
+ struct iio_dev *indio_dev = dev_get_drvdata(&client->dev);
struct inv_mpu6050_state *st = iio_priv(indio_dev);
inv_mpu_acpi_delete_mux_client(client);
- i2c_del_mux_adapter(st->mux_adapter);
+ i2c_mux_del_adapters(st->muxc);
return inv_mpu_core_remove(&client->dev);
}
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
index e302a49703bf..bb3cef6d7059 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
@@ -11,6 +11,7 @@
* GNU General Public License for more details.
*/
#include <linux/i2c.h>
+#include <linux/i2c-mux.h>
#include <linux/kfifo.h>
#include <linux/spinlock.h>
#include <linux/iio/iio.h>
@@ -127,7 +128,7 @@ struct inv_mpu6050_state {
const struct inv_mpu6050_hw *hw;
enum inv_devices chip_type;
spinlock_t time_stamp_lock;
- struct i2c_adapter *mux_adapter;
+ struct i2c_mux_core *muxc;
struct i2c_client *mux_client;
unsigned int powerup_count;
struct inv_mpu6050_platform_data plat_data;
--
2.1.4
[toc] | [prev] | [next] | [standalone]
| From | Jonathan Cameron <jic23@kernel.org> |
|---|---|
| Date | 2016-04-03 13:00 +0200 |
| Subject | Re: [PATCH v6 08/24] iio: imu: inv_mpu6050: convert to use an explicit i2c mux core |
| Message-ID | <rjNV8-274-11@gated-at.bofh.it> |
| In reply to | #1370152 |
On 03/04/16 09:52, Peter Rosin wrote:
> From: Peter Rosin <peda@axentia.se>
>
> Allocate an explicit i2c mux core to handle parent and child adapters
> etc. Update the select/deselect ops to be in terms of the i2c mux core
> instead of the child adapter.
>
> Signed-off-by: Peter Rosin <peda@axentia.se>
I'm mostly fine with this (though one unrelated change seems to have snuck
in). However, I'm not set up to test it - hence other than fixing the change
you can have my ack, but ideal would be a tested by from someone with
relevant hardware... However, it looks to be a fairly mechanical change so
if no one is currently setup to test it, then don't let it hold up the
series too long!
Acked-by: Jonathan Cameron <jic23@kernel.org>
Jonathan
> ---
> drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c | 2 +-
> drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 1 -
> drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c | 32 +++++++++++++-----------------
> drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h | 3 ++-
> 4 files changed, 17 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c
> index 2771106fd650..f62b8bd9ad7e 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c
> @@ -183,7 +183,7 @@ int inv_mpu_acpi_create_mux_client(struct i2c_client *client)
> } else
> return 0; /* no secondary addr, which is OK */
> }
> - st->mux_client = i2c_new_device(st->mux_adapter, &info);
> + st->mux_client = i2c_new_device(st->muxc->adapter[0], &info);
> if (!st->mux_client)
> return -ENODEV;
> }
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> index d192953e9a38..0c2bded2b5b7 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> @@ -23,7 +23,6 @@
> #include <linux/kfifo.h>
> #include <linux/spinlock.h>
> #include <linux/iio/iio.h>
> -#include <linux/i2c-mux.h>
> #include <linux/acpi.h>
> #include "inv_mpu_iio.h"
>
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> index f581256d9d4c..0d429d788106 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> @@ -15,7 +15,6 @@
> #include <linux/delay.h>
> #include <linux/err.h>
> #include <linux/i2c.h>
> -#include <linux/i2c-mux.h>
> #include <linux/iio/iio.h>
> #include <linux/module.h>
> #include "inv_mpu_iio.h"
> @@ -52,10 +51,9 @@ static int inv_mpu6050_write_reg_unlocked(struct i2c_client *client,
> return 0;
> }
>
> -static int inv_mpu6050_select_bypass(struct i2c_adapter *adap, void *mux_priv,
> - u32 chan_id)
> +static int inv_mpu6050_select_bypass(struct i2c_mux_core *muxc, u32 chan_id)
> {
> - struct i2c_client *client = mux_priv;
> + struct i2c_client *client = i2c_mux_priv(muxc);
> struct iio_dev *indio_dev = dev_get_drvdata(&client->dev);
> struct inv_mpu6050_state *st = iio_priv(indio_dev);
> int ret = 0;
> @@ -84,10 +82,9 @@ write_error:
> return ret;
> }
>
> -static int inv_mpu6050_deselect_bypass(struct i2c_adapter *adap,
> - void *mux_priv, u32 chan_id)
> +static int inv_mpu6050_deselect_bypass(struct i2c_mux_core *muxc, u32 chan_id)
> {
> - struct i2c_client *client = mux_priv;
> + struct i2c_client *client = i2c_mux_priv(muxc);
> struct iio_dev *indio_dev = dev_get_drvdata(&client->dev);
> struct inv_mpu6050_state *st = iio_priv(indio_dev);
>
> @@ -136,16 +133,15 @@ static int inv_mpu_probe(struct i2c_client *client,
> return result;
>
> st = iio_priv(dev_get_drvdata(&client->dev));
> - st->mux_adapter = i2c_add_mux_adapter(client->adapter,
> - &client->dev,
> - client,
> - 0, 0, 0,
> - inv_mpu6050_select_bypass,
> - inv_mpu6050_deselect_bypass);
> - if (!st->mux_adapter) {
> - result = -ENODEV;
> + st->muxc = i2c_mux_one_adapter(client->adapter, &client->dev, 0, 0,
> + 0, 0, 0,
> + inv_mpu6050_select_bypass,
> + inv_mpu6050_deselect_bypass);
> + if (IS_ERR(st->muxc)) {
> + result = PTR_ERR(st->muxc);
> goto out_unreg_device;
> }
> + st->muxc->priv = client;
>
> result = inv_mpu_acpi_create_mux_client(client);
> if (result)
> @@ -154,7 +150,7 @@ static int inv_mpu_probe(struct i2c_client *client,
> return 0;
>
> out_del_mux:
> - i2c_del_mux_adapter(st->mux_adapter);
> + i2c_mux_del_adapters(st->muxc);
> out_unreg_device:
> inv_mpu_core_remove(&client->dev);
> return result;
> @@ -162,11 +158,11 @@ out_unreg_device:
>
> static int inv_mpu_remove(struct i2c_client *client)
> {
> - struct iio_dev *indio_dev = i2c_get_clientdata(client);
> + struct iio_dev *indio_dev = dev_get_drvdata(&client->dev);
Why this change? Seems unrelated.
> struct inv_mpu6050_state *st = iio_priv(indio_dev);
>
> inv_mpu_acpi_delete_mux_client(client);
> - i2c_del_mux_adapter(st->mux_adapter);
> + i2c_mux_del_adapters(st->muxc);
>
> return inv_mpu_core_remove(&client->dev);
> }
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> index e302a49703bf..bb3cef6d7059 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> @@ -11,6 +11,7 @@
> * GNU General Public License for more details.
> */
> #include <linux/i2c.h>
> +#include <linux/i2c-mux.h>
> #include <linux/kfifo.h>
> #include <linux/spinlock.h>
> #include <linux/iio/iio.h>
> @@ -127,7 +128,7 @@ struct inv_mpu6050_state {
> const struct inv_mpu6050_hw *hw;
> enum inv_devices chip_type;
> spinlock_t time_stamp_lock;
> - struct i2c_adapter *mux_adapter;
> + struct i2c_mux_core *muxc;
> struct i2c_client *mux_client;
> unsigned int powerup_count;
> struct inv_mpu6050_platform_data plat_data;
>
[toc] | [prev] | [next] | [standalone]
| From | Peter Rosin <peda@lysator.liu.se> |
|---|---|
| Date | 2016-04-03 14:00 +0200 |
| Subject | Re: [PATCH v6 08/24] iio: imu: inv_mpu6050: convert to use an explicit i2c mux core |
| Message-ID | <rjORc-2Nz-1@gated-at.bofh.it> |
| In reply to | #1370197 |
On 2016-04-03 12:51, Jonathan Cameron wrote:
> On 03/04/16 09:52, Peter Rosin wrote:
>> From: Peter Rosin <peda@axentia.se>
>>
>> Allocate an explicit i2c mux core to handle parent and child adapters
>> etc. Update the select/deselect ops to be in terms of the i2c mux core
>> instead of the child adapter.
>>
>> Signed-off-by: Peter Rosin <peda@axentia.se>
> I'm mostly fine with this (though one unrelated change seems to have snuck
> in). However, I'm not set up to test it - hence other than fixing the change
> you can have my ack, but ideal would be a tested by from someone with
> relevant hardware... However, it looks to be a fairly mechanical change so
> if no one is currently setup to test it, then don't let it hold up the
> series too long!
>
> Acked-by: Jonathan Cameron <jic23@kernel.org>
Thanks for your acks!
> Jonathan
>> ---
>> drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c | 2 +-
>> drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 1 -
>> drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c | 32 +++++++++++++-----------------
>> drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h | 3 ++-
>> 4 files changed, 17 insertions(+), 21 deletions(-)
>>
>> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c
>> index 2771106fd650..f62b8bd9ad7e 100644
>> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c
>> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c
>> @@ -183,7 +183,7 @@ int inv_mpu_acpi_create_mux_client(struct i2c_client *client)
>> } else
>> return 0; /* no secondary addr, which is OK */
>> }
>> - st->mux_client = i2c_new_device(st->mux_adapter, &info);
>> + st->mux_client = i2c_new_device(st->muxc->adapter[0], &info);
>> if (!st->mux_client)
>> return -ENODEV;
>> }
>> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
>> index d192953e9a38..0c2bded2b5b7 100644
>> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
>> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
>> @@ -23,7 +23,6 @@
>> #include <linux/kfifo.h>
>> #include <linux/spinlock.h>
>> #include <linux/iio/iio.h>
>> -#include <linux/i2c-mux.h>
>> #include <linux/acpi.h>
>> #include "inv_mpu_iio.h"
>>
>> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
>> index f581256d9d4c..0d429d788106 100644
>> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
>> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
>> @@ -15,7 +15,6 @@
>> #include <linux/delay.h>
>> #include <linux/err.h>
>> #include <linux/i2c.h>
>> -#include <linux/i2c-mux.h>
>> #include <linux/iio/iio.h>
>> #include <linux/module.h>
>> #include "inv_mpu_iio.h"
>> @@ -52,10 +51,9 @@ static int inv_mpu6050_write_reg_unlocked(struct i2c_client *client,
>> return 0;
>> }
>>
>> -static int inv_mpu6050_select_bypass(struct i2c_adapter *adap, void *mux_priv,
>> - u32 chan_id)
>> +static int inv_mpu6050_select_bypass(struct i2c_mux_core *muxc, u32 chan_id)
>> {
>> - struct i2c_client *client = mux_priv;
>> + struct i2c_client *client = i2c_mux_priv(muxc);
>> struct iio_dev *indio_dev = dev_get_drvdata(&client->dev);
Here, the existing code uses drv_get_drvdata to get from i2c_client to iio_dev...
>> struct inv_mpu6050_state *st = iio_priv(indio_dev);
>> int ret = 0;
>> @@ -84,10 +82,9 @@ write_error:
>> return ret;
>> }
>>
>> -static int inv_mpu6050_deselect_bypass(struct i2c_adapter *adap,
>> - void *mux_priv, u32 chan_id)
>> +static int inv_mpu6050_deselect_bypass(struct i2c_mux_core *muxc, u32 chan_id)
>> {
>> - struct i2c_client *client = mux_priv;
>> + struct i2c_client *client = i2c_mux_priv(muxc);
>> struct iio_dev *indio_dev = dev_get_drvdata(&client->dev);
...and here too...
>> struct inv_mpu6050_state *st = iio_priv(indio_dev);
>>
>> @@ -136,16 +133,15 @@ static int inv_mpu_probe(struct i2c_client *client,
>> return result;
>>
>> st = iio_priv(dev_get_drvdata(&client->dev));
>> - st->mux_adapter = i2c_add_mux_adapter(client->adapter,
>> - &client->dev,
>> - client,
>> - 0, 0, 0,
>> - inv_mpu6050_select_bypass,
>> - inv_mpu6050_deselect_bypass);
>> - if (!st->mux_adapter) {
>> - result = -ENODEV;
>> + st->muxc = i2c_mux_one_adapter(client->adapter, &client->dev, 0, 0,
>> + 0, 0, 0,
>> + inv_mpu6050_select_bypass,
>> + inv_mpu6050_deselect_bypass);
>> + if (IS_ERR(st->muxc)) {
>> + result = PTR_ERR(st->muxc);
>> goto out_unreg_device;
>> }
>> + st->muxc->priv = client;
>>
>> result = inv_mpu_acpi_create_mux_client(client);
>> if (result)
>> @@ -154,7 +150,7 @@ static int inv_mpu_probe(struct i2c_client *client,
>> return 0;
>>
>> out_del_mux:
>> - i2c_del_mux_adapter(st->mux_adapter);
>> + i2c_mux_del_adapters(st->muxc);
>> out_unreg_device:
>> inv_mpu_core_remove(&client->dev);
>> return result;
>> @@ -162,11 +158,11 @@ out_unreg_device:
>>
>> static int inv_mpu_remove(struct i2c_client *client)
>> {
>> - struct iio_dev *indio_dev = i2c_get_clientdata(client);
>> + struct iio_dev *indio_dev = dev_get_drvdata(&client->dev);
> Why this change? Seems unrelated.
...which is why I made this change. Maybe a bad call, but the inconsistency
disturbed me and I was changing the function anyway. I could split it out
to its own commit I suppose, or should I just not bother at all?
Cheers,
Peter
>> struct inv_mpu6050_state *st = iio_priv(indio_dev);
>>
>> inv_mpu_acpi_delete_mux_client(client);
>> - i2c_del_mux_adapter(st->mux_adapter);
>> + i2c_mux_del_adapters(st->muxc);
>>
>> return inv_mpu_core_remove(&client->dev);
>> }
>> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
>> index e302a49703bf..bb3cef6d7059 100644
>> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
>> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
>> @@ -11,6 +11,7 @@
>> * GNU General Public License for more details.
>> */
>> #include <linux/i2c.h>
>> +#include <linux/i2c-mux.h>
>> #include <linux/kfifo.h>
>> #include <linux/spinlock.h>
>> #include <linux/iio/iio.h>
>> @@ -127,7 +128,7 @@ struct inv_mpu6050_state {
>> const struct inv_mpu6050_hw *hw;
>> enum inv_devices chip_type;
>> spinlock_t time_stamp_lock;
>> - struct i2c_adapter *mux_adapter;
>> + struct i2c_mux_core *muxc;
>> struct i2c_client *mux_client;
>> unsigned int powerup_count;
>> struct inv_mpu6050_platform_data plat_data;
>>
>
[toc] | [prev] | [next] | [standalone]
| From | Jonathan Cameron <jic23@kernel.org> |
|---|---|
| Date | 2016-04-10 16:20 +0200 |
| Subject | Re: [PATCH v6 08/24] iio: imu: inv_mpu6050: convert to use an explicit i2c mux core |
| Message-ID | <rmonw-6i9-13@gated-at.bofh.it> |
| In reply to | #1370203 |
On 03/04/16 12:51, Peter Rosin wrote:
> On 2016-04-03 12:51, Jonathan Cameron wrote:
>> On 03/04/16 09:52, Peter Rosin wrote:
>>> From: Peter Rosin <peda@axentia.se>
>>>
>>> Allocate an explicit i2c mux core to handle parent and child adapters
>>> etc. Update the select/deselect ops to be in terms of the i2c mux core
>>> instead of the child adapter.
>>>
>>> Signed-off-by: Peter Rosin <peda@axentia.se>
>> I'm mostly fine with this (though one unrelated change seems to have snuck
>> in). However, I'm not set up to test it - hence other than fixing the change
>> you can have my ack, but ideal would be a tested by from someone with
>> relevant hardware... However, it looks to be a fairly mechanical change so
>> if no one is currently setup to test it, then don't let it hold up the
>> series too long!
>>
>> Acked-by: Jonathan Cameron <jic23@kernel.org>
>
> Thanks for your acks!
>
>> Jonathan
>>> ---
>>> drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c | 2 +-
>>> drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 1 -
>>> drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c | 32 +++++++++++++-----------------
>>> drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h | 3 ++-
>>> 4 files changed, 17 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c
>>> index 2771106fd650..f62b8bd9ad7e 100644
>>> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c
>>> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c
>>> @@ -183,7 +183,7 @@ int inv_mpu_acpi_create_mux_client(struct i2c_client *client)
>>> } else
>>> return 0; /* no secondary addr, which is OK */
>>> }
>>> - st->mux_client = i2c_new_device(st->mux_adapter, &info);
>>> + st->mux_client = i2c_new_device(st->muxc->adapter[0], &info);
>>> if (!st->mux_client)
>>> return -ENODEV;
>>> }
>>> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
>>> index d192953e9a38..0c2bded2b5b7 100644
>>> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
>>> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
>>> @@ -23,7 +23,6 @@
>>> #include <linux/kfifo.h>
>>> #include <linux/spinlock.h>
>>> #include <linux/iio/iio.h>
>>> -#include <linux/i2c-mux.h>
>>> #include <linux/acpi.h>
>>> #include "inv_mpu_iio.h"
>>>
>>> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
>>> index f581256d9d4c..0d429d788106 100644
>>> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
>>> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
>>> @@ -15,7 +15,6 @@
>>> #include <linux/delay.h>
>>> #include <linux/err.h>
>>> #include <linux/i2c.h>
>>> -#include <linux/i2c-mux.h>
>>> #include <linux/iio/iio.h>
>>> #include <linux/module.h>
>>> #include "inv_mpu_iio.h"
>>> @@ -52,10 +51,9 @@ static int inv_mpu6050_write_reg_unlocked(struct i2c_client *client,
>>> return 0;
>>> }
>>>
>>> -static int inv_mpu6050_select_bypass(struct i2c_adapter *adap, void *mux_priv,
>>> - u32 chan_id)
>>> +static int inv_mpu6050_select_bypass(struct i2c_mux_core *muxc, u32 chan_id)
>>> {
>>> - struct i2c_client *client = mux_priv;
>>> + struct i2c_client *client = i2c_mux_priv(muxc);
>>> struct iio_dev *indio_dev = dev_get_drvdata(&client->dev);
>
> Here, the existing code uses drv_get_drvdata to get from i2c_client to iio_dev...
>
>>> struct inv_mpu6050_state *st = iio_priv(indio_dev);
>>> int ret = 0;
>>> @@ -84,10 +82,9 @@ write_error:
>>> return ret;
>>> }
>>>
>>> -static int inv_mpu6050_deselect_bypass(struct i2c_adapter *adap,
>>> - void *mux_priv, u32 chan_id)
>>> +static int inv_mpu6050_deselect_bypass(struct i2c_mux_core *muxc, u32 chan_id)
>>> {
>>> - struct i2c_client *client = mux_priv;
>>> + struct i2c_client *client = i2c_mux_priv(muxc);
>>> struct iio_dev *indio_dev = dev_get_drvdata(&client->dev);
>
> ...and here too...
>
>>> struct inv_mpu6050_state *st = iio_priv(indio_dev);
>>>
>>> @@ -136,16 +133,15 @@ static int inv_mpu_probe(struct i2c_client *client,
>>> return result;
>>>
>>> st = iio_priv(dev_get_drvdata(&client->dev));
>>> - st->mux_adapter = i2c_add_mux_adapter(client->adapter,
>>> - &client->dev,
>>> - client,
>>> - 0, 0, 0,
>>> - inv_mpu6050_select_bypass,
>>> - inv_mpu6050_deselect_bypass);
>>> - if (!st->mux_adapter) {
>>> - result = -ENODEV;
>>> + st->muxc = i2c_mux_one_adapter(client->adapter, &client->dev, 0, 0,
>>> + 0, 0, 0,
>>> + inv_mpu6050_select_bypass,
>>> + inv_mpu6050_deselect_bypass);
>>> + if (IS_ERR(st->muxc)) {
>>> + result = PTR_ERR(st->muxc);
>>> goto out_unreg_device;
>>> }
>>> + st->muxc->priv = client;
>>>
>>> result = inv_mpu_acpi_create_mux_client(client);
>>> if (result)
>>> @@ -154,7 +150,7 @@ static int inv_mpu_probe(struct i2c_client *client,
>>> return 0;
>>>
>>> out_del_mux:
>>> - i2c_del_mux_adapter(st->mux_adapter);
>>> + i2c_mux_del_adapters(st->muxc);
>>> out_unreg_device:
>>> inv_mpu_core_remove(&client->dev);
>>> return result;
>>> @@ -162,11 +158,11 @@ out_unreg_device:
>>>
>>> static int inv_mpu_remove(struct i2c_client *client)
>>> {
>>> - struct iio_dev *indio_dev = i2c_get_clientdata(client);
>>> + struct iio_dev *indio_dev = dev_get_drvdata(&client->dev);
>> Why this change? Seems unrelated.
>
> ...which is why I made this change. Maybe a bad call, but the inconsistency
> disturbed me and I was changing the function anyway. I could split it out
> to its own commit I suppose, or should I just not bother at all?
Funny thing is I'd say the i2c_get_clientdata option is the better of the two!
I don't really care though either way.
J
>
> Cheers,
> Peter
>
>>> struct inv_mpu6050_state *st = iio_priv(indio_dev);
>>>
>>> inv_mpu_acpi_delete_mux_client(client);
>>> - i2c_del_mux_adapter(st->mux_adapter);
>>> + i2c_mux_del_adapters(st->muxc);
>>>
>>> return inv_mpu_core_remove(&client->dev);
>>> }
>>> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
>>> index e302a49703bf..bb3cef6d7059 100644
>>> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
>>> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
>>> @@ -11,6 +11,7 @@
>>> * GNU General Public License for more details.
>>> */
>>> #include <linux/i2c.h>
>>> +#include <linux/i2c-mux.h>
>>> #include <linux/kfifo.h>
>>> #include <linux/spinlock.h>
>>> #include <linux/iio/iio.h>
>>> @@ -127,7 +128,7 @@ struct inv_mpu6050_state {
>>> const struct inv_mpu6050_hw *hw;
>>> enum inv_devices chip_type;
>>> spinlock_t time_stamp_lock;
>>> - struct i2c_adapter *mux_adapter;
>>> + struct i2c_mux_core *muxc;
>>> struct i2c_client *mux_client;
>>> unsigned int powerup_count;
>>> struct inv_mpu6050_platform_data plat_data;
>>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
[toc] | [prev] | [next] | [standalone]
| From | Peter Rosin <peda@lysator.liu.se> |
|---|---|
| Date | 2016-04-03 11:00 +0200 |
| Subject | [PATCH v6 02/24] i2c: i2c-mux-gpio: convert to use an explicit i2c mux core |
| Message-ID | <rjM32-Ns-57@gated-at.bofh.it> |
| In reply to | #1370151 |
From: Peter Rosin <peda@axentia.se>
Allocate an explicit i2c mux core to handle parent and child adapters
etc. Update the select/deselect ops to be in terms of the i2c mux core
instead of the child adapter.
Signed-off-by: Peter Rosin <peda@axentia.se>
---
drivers/i2c/muxes/i2c-mux-gpio.c | 54 ++++++++++++++++------------------------
1 file changed, 21 insertions(+), 33 deletions(-)
diff --git a/drivers/i2c/muxes/i2c-mux-gpio.c b/drivers/i2c/muxes/i2c-mux-gpio.c
index b8e11c16d98c..1bcc26737359 100644
--- a/drivers/i2c/muxes/i2c-mux-gpio.c
+++ b/drivers/i2c/muxes/i2c-mux-gpio.c
@@ -18,8 +18,6 @@
#include <linux/of_gpio.h>
struct gpiomux {
- struct i2c_adapter *parent;
- struct i2c_adapter **adap; /* child busses */
struct i2c_mux_gpio_platform_data data;
unsigned gpio_base;
};
@@ -33,18 +31,18 @@ static void i2c_mux_gpio_set(const struct gpiomux *mux, unsigned val)
val & (1 << i));
}
-static int i2c_mux_gpio_select(struct i2c_adapter *adap, void *data, u32 chan)
+static int i2c_mux_gpio_select(struct i2c_mux_core *muxc, u32 chan)
{
- struct gpiomux *mux = data;
+ struct gpiomux *mux = i2c_mux_priv(muxc);
i2c_mux_gpio_set(mux, chan);
return 0;
}
-static int i2c_mux_gpio_deselect(struct i2c_adapter *adap, void *data, u32 chan)
+static int i2c_mux_gpio_deselect(struct i2c_mux_core *muxc, u32 chan)
{
- struct gpiomux *mux = data;
+ struct gpiomux *mux = i2c_mux_priv(muxc);
i2c_mux_gpio_set(mux, mux->data.idle);
@@ -136,19 +134,19 @@ static int i2c_mux_gpio_probe_dt(struct gpiomux *mux,
static int i2c_mux_gpio_probe(struct platform_device *pdev)
{
+ struct i2c_mux_core *muxc;
struct gpiomux *mux;
struct i2c_adapter *parent;
- int (*deselect) (struct i2c_adapter *, void *, u32);
unsigned initial_state, gpio_base;
int i, ret;
- mux = devm_kzalloc(&pdev->dev, sizeof(*mux), GFP_KERNEL);
- if (!mux) {
- dev_err(&pdev->dev, "Cannot allocate gpiomux structure");
+ muxc = i2c_mux_alloc(NULL, &pdev->dev, sizeof(*mux), 0,
+ i2c_mux_gpio_select, NULL);
+ if (!muxc)
return -ENOMEM;
- }
+ mux = i2c_mux_priv(muxc);
- platform_set_drvdata(pdev, mux);
+ platform_set_drvdata(pdev, muxc);
if (!dev_get_platdata(&pdev->dev)) {
ret = i2c_mux_gpio_probe_dt(mux, pdev);
@@ -180,24 +178,18 @@ static int i2c_mux_gpio_probe(struct platform_device *pdev)
if (!parent)
return -EPROBE_DEFER;
- mux->parent = parent;
+ muxc->parent = parent;
mux->gpio_base = gpio_base;
- mux->adap = devm_kzalloc(&pdev->dev,
- sizeof(*mux->adap) * mux->data.n_values,
- GFP_KERNEL);
- if (!mux->adap) {
- dev_err(&pdev->dev, "Cannot allocate i2c_adapter structure");
- ret = -ENOMEM;
+ ret = i2c_mux_reserve_adapters(muxc, mux->data.n_values);
+ if (ret)
goto alloc_failed;
- }
if (mux->data.idle != I2C_MUX_GPIO_NO_IDLE) {
initial_state = mux->data.idle;
- deselect = i2c_mux_gpio_deselect;
+ muxc->deselect = i2c_mux_gpio_deselect;
} else {
initial_state = mux->data.values[0];
- deselect = NULL;
}
for (i = 0; i < mux->data.n_gpios; i++) {
@@ -223,11 +215,8 @@ static int i2c_mux_gpio_probe(struct platform_device *pdev)
u32 nr = mux->data.base_nr ? (mux->data.base_nr + i) : 0;
unsigned int class = mux->data.classes ? mux->data.classes[i] : 0;
- mux->adap[i] = i2c_add_mux_adapter(parent, &pdev->dev, mux, nr,
- mux->data.values[i], class,
- i2c_mux_gpio_select, deselect);
- if (!mux->adap[i]) {
- ret = -ENODEV;
+ ret = i2c_mux_add_adapter(muxc, nr, mux->data.values[i], class);
+ if (ret) {
dev_err(&pdev->dev, "Failed to add adapter %d\n", i);
goto add_adapter_failed;
}
@@ -239,8 +228,7 @@ static int i2c_mux_gpio_probe(struct platform_device *pdev)
return 0;
add_adapter_failed:
- for (; i > 0; i--)
- i2c_del_mux_adapter(mux->adap[i - 1]);
+ i2c_mux_del_adapters(muxc);
i = mux->data.n_gpios;
err_request_gpio:
for (; i > 0; i--)
@@ -253,16 +241,16 @@ alloc_failed:
static int i2c_mux_gpio_remove(struct platform_device *pdev)
{
- struct gpiomux *mux = platform_get_drvdata(pdev);
+ struct i2c_mux_core *muxc = platform_get_drvdata(pdev);
+ struct gpiomux *mux = i2c_mux_priv(muxc);
int i;
- for (i = 0; i < mux->data.n_values; i++)
- i2c_del_mux_adapter(mux->adap[i]);
+ i2c_mux_del_adapters(muxc);
for (i = 0; i < mux->data.n_gpios; i++)
gpio_free(mux->gpio_base + mux->data.gpios[i]);
- i2c_put_adapter(mux->parent);
+ i2c_put_adapter(muxc->parent);
return 0;
}
--
2.1.4
[toc] | [prev] | [next] | [standalone]
| From | Peter Rosin <peda@lysator.liu.se> |
|---|---|
| Date | 2016-04-03 11:00 +0200 |
| Subject | [PATCH v6 21/24] [media] si2168: change the i2c gate to be mux-locked |
| Message-ID | <rjM32-Ns-55@gated-at.bofh.it> |
| In reply to | #1370151 |
From: Antti Palosaari <crope@iki.fi>
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
i2c accesses unlocked. But add a mutex so that firmware commands are
still serialized.
Signed-off-by: Antti Palosaari <crope@iki.fi>
Signed-off-by: Peter Rosin <peda@axentia.se>
---
Documentation/i2c/i2c-topology | 2 +-
drivers/media/dvb-frontends/si2168.c | 85 ++++++++-----------------------
drivers/media/dvb-frontends/si2168_priv.h | 1 +
3 files changed, 23 insertions(+), 65 deletions(-)
diff --git a/Documentation/i2c/i2c-topology b/Documentation/i2c/i2c-topology
index 346623a80bd1..a9ca97df7661 100644
--- a/Documentation/i2c/i2c-topology
+++ b/Documentation/i2c/i2c-topology
@@ -56,7 +56,7 @@ In drivers/media/
dvb-frontends/m88ds3103 Parent-locked
dvb-frontends/rtl2830 Parent-locked
dvb-frontends/rtl2832 Parent-locked
-dvb-frontends/si2168 Parent-locked
+dvb-frontends/si2168 Mux-locked
usb/cx231xx/ Parent-locked
diff --git a/drivers/media/dvb-frontends/si2168.c b/drivers/media/dvb-frontends/si2168.c
index ca455d01c71d..2eecbb2ef48e 100644
--- a/drivers/media/dvb-frontends/si2168.c
+++ b/drivers/media/dvb-frontends/si2168.c
@@ -18,53 +18,23 @@
static const struct dvb_frontend_ops si2168_ops;
-/* Own I2C adapter locking is needed because of I2C gate logic. */
-static int si2168_i2c_master_send_unlocked(const struct i2c_client *client,
- const char *buf, int count)
-{
- int ret;
- struct i2c_msg msg = {
- .addr = client->addr,
- .flags = 0,
- .len = count,
- .buf = (char *)buf,
- };
-
- ret = __i2c_transfer(client->adapter, &msg, 1);
- return (ret == 1) ? count : ret;
-}
-
-static int si2168_i2c_master_recv_unlocked(const struct i2c_client *client,
- char *buf, int count)
-{
- int ret;
- struct i2c_msg msg = {
- .addr = client->addr,
- .flags = I2C_M_RD,
- .len = count,
- .buf = buf,
- };
-
- ret = __i2c_transfer(client->adapter, &msg, 1);
- return (ret == 1) ? count : ret;
-}
-
/* execute firmware command */
-static int si2168_cmd_execute_unlocked(struct i2c_client *client,
- struct si2168_cmd *cmd)
+static int si2168_cmd_execute(struct i2c_client *client, struct si2168_cmd *cmd)
{
+ struct si2168_dev *dev = i2c_get_clientdata(client);
int ret;
unsigned long timeout;
+ mutex_lock(&dev->i2c_mutex);
+
if (cmd->wlen) {
/* write cmd and args for firmware */
- ret = si2168_i2c_master_send_unlocked(client, cmd->args,
- cmd->wlen);
+ ret = i2c_master_send(client, cmd->args, cmd->wlen);
if (ret < 0) {
- goto err;
+ goto err_mutex_unlock;
} else if (ret != cmd->wlen) {
ret = -EREMOTEIO;
- goto err;
+ goto err_mutex_unlock;
}
}
@@ -73,13 +43,12 @@ static int si2168_cmd_execute_unlocked(struct i2c_client *client,
#define TIMEOUT 70
timeout = jiffies + msecs_to_jiffies(TIMEOUT);
while (!time_after(jiffies, timeout)) {
- ret = si2168_i2c_master_recv_unlocked(client, cmd->args,
- cmd->rlen);
+ ret = i2c_master_recv(client, cmd->args, cmd->rlen);
if (ret < 0) {
- goto err;
+ goto err_mutex_unlock;
} else if (ret != cmd->rlen) {
ret = -EREMOTEIO;
- goto err;
+ goto err_mutex_unlock;
}
/* firmware ready? */
@@ -94,32 +63,23 @@ static int si2168_cmd_execute_unlocked(struct i2c_client *client,
/* error bit set? */
if ((cmd->args[0] >> 6) & 0x01) {
ret = -EREMOTEIO;
- goto err;
+ goto err_mutex_unlock;
}
if (!((cmd->args[0] >> 7) & 0x01)) {
ret = -ETIMEDOUT;
- goto err;
+ goto err_mutex_unlock;
}
}
+ mutex_unlock(&dev->i2c_mutex);
return 0;
-err:
+err_mutex_unlock:
+ mutex_unlock(&dev->i2c_mutex);
dev_dbg(&client->dev, "failed=%d\n", ret);
return ret;
}
-static int si2168_cmd_execute(struct i2c_client *client, struct si2168_cmd *cmd)
-{
- int ret;
-
- i2c_lock_adapter(client->adapter);
- ret = si2168_cmd_execute_unlocked(client, cmd);
- i2c_unlock_adapter(client->adapter);
-
- return ret;
-}
-
static int si2168_read_status(struct dvb_frontend *fe, enum fe_status *status)
{
struct i2c_client *client = fe->demodulator_priv;
@@ -610,11 +570,6 @@ static int si2168_get_tune_settings(struct dvb_frontend *fe,
return 0;
}
-/*
- * I2C gate logic
- * We must use unlocked I2C I/O because I2C adapter lock is already taken
- * by the caller (usually tuner driver).
- */
static int si2168_select(struct i2c_mux_core *muxc, u32 chan)
{
struct i2c_client *client = i2c_mux_priv(muxc);
@@ -625,7 +580,7 @@ static int si2168_select(struct i2c_mux_core *muxc, u32 chan)
memcpy(cmd.args, "\xc0\x0d\x01", 3);
cmd.wlen = 3;
cmd.rlen = 0;
- ret = si2168_cmd_execute_unlocked(client, &cmd);
+ ret = si2168_cmd_execute(client, &cmd);
if (ret)
goto err;
@@ -645,7 +600,7 @@ static int si2168_deselect(struct i2c_mux_core *muxc, u32 chan)
memcpy(cmd.args, "\xc0\x0d\x00", 3);
cmd.wlen = 3;
cmd.rlen = 0;
- ret = si2168_cmd_execute_unlocked(client, &cmd);
+ ret = si2168_cmd_execute(client, &cmd);
if (ret)
goto err;
@@ -708,9 +663,11 @@ static int si2168_probe(struct i2c_client *client,
goto err;
}
+ mutex_init(&dev->i2c_mutex);
+
/* create mux i2c adapter for tuner */
- dev->muxc = i2c_mux_one_adapter(client->adapter, &client->dev, 0, 0,
- 0, 0, 0,
+ dev->muxc = i2c_mux_one_adapter(client->adapter, &client->dev, 0,
+ I2C_MUX_LOCKED, 0, 0, 0,
si2168_select, si2168_deselect);
if (IS_ERR(dev->muxc)) {
ret = PTR_ERR(dev->muxc);
diff --git a/drivers/media/dvb-frontends/si2168_priv.h b/drivers/media/dvb-frontends/si2168_priv.h
index 165bf1412063..8a1f36d2014d 100644
--- a/drivers/media/dvb-frontends/si2168_priv.h
+++ b/drivers/media/dvb-frontends/si2168_priv.h
@@ -29,6 +29,7 @@
/* state struct */
struct si2168_dev {
+ struct mutex i2c_mutex;
struct i2c_mux_core *muxc;
struct dvb_frontend fe;
enum fe_delivery_system delivery_system;
--
2.1.4
[toc] | [prev] | [next] | [standalone]
| From | Wolfram Sang <wsa@the-dreams.de> |
|---|---|
| Date | 2016-04-11 14:50 +0200 |
| Message-ID | <rmJrX-61n-3@gated-at.bofh.it> |
| In reply to | #1370151 |
[Multipart message — attachments visible in raw view] — view raw
Hi Peter, > To summarize the series, there's some i2c-mux infrastructure cleanup work > first (I think that part stands by itself as desireable regardless), the > locking changes are in 16/24 and after with the real meat in 18/24. There > is some documentation added in 19/24 while 20/24 and after are cleanups to > existing drivers utilizing the new stuff. My idea is to review and pull in the infrastructure work for 4.7 and the locking changes to 4.8. This gives us one cycle to fix regressions (if any) in the infrastructure work first. Is that okay with you? Thanks, Wolfram
[toc] | [prev] | [next] | [standalone]
| From | Peter Rosin <peda@lysator.liu.se> |
|---|---|
| Date | 2016-04-11 15:40 +0200 |
| Message-ID | <rmKel-6GA-13@gated-at.bofh.it> |
| In reply to | #1375789 |
Hi! On 2016-04-11 14:39, Wolfram Sang wrote: > Hi Peter, > >> To summarize the series, there's some i2c-mux infrastructure cleanup work >> first (I think that part stands by itself as desireable regardless), the >> locking changes are in 16/24 and after with the real meat in 18/24. There >> is some documentation added in 19/24 while 20/24 and after are cleanups to >> existing drivers utilizing the new stuff. > > My idea is to review and pull in the infrastructure work for 4.7 and the > locking changes to 4.8. This gives us one cycle to fix regressions (if > any) in the infrastructure work first. Is that okay with you? I was just thinking about how it appears impossible to get it all of it merged in one go and what to do about it... The untra-defensive approach is only merge stuff that has actually been tested on real hw, and to hold off the rest until someone has tested. That can obviously take forever. At the same time, many of the patches are kind of mechanical, and feels rather safe. 1 i2c-mux: add common data for every i2c-mux instance 2 i2c: i2c-mux-gpio: convert to use an explicit i2c mux core 3 i2c: i2c-mux-pinctrl: convert to use an explicit i2c mux core 4 i2c: i2c-arb-gpio-challenge: convert to use an explicit i2c mux core 5 i2c: i2c-mux-pca9541: convert to use an explicit i2c mux core 6 i2c: i2c-mux-pca954x: convert to use an explicit i2c mux core 7 i2c: i2c-mux-reg: convert to use an explicit i2c mux core 8 iio: imu: inv_mpu6050: convert to use an explicit i2c mux core 9 [media] m88ds3103: convert to use an explicit i2c mux core 10 [media] rtl2830: convert to use an explicit i2c mux core 11 [media] rtl2832: convert to use an explicit i2c mux core 12 [media] si2168: convert to use an explicit i2c mux core 13 [media] cx231xx: convert to use an explicit i2c mux core 14 of/unittest: convert to use an explicit i2c mux core 15 i2c-mux: drop old unused i2c-mux api 16 i2c: allow adapter drivers to override the adapter locking 17 i2c: muxes always lock the parent adapter 18 i2c-mux: relax locking of the top i2c adapter during mux-locked muxing 19 i2c-mux: document i2c muxes and elaborate on parent-/mux-locked muxes 20 iio: imu: inv_mpu6050: change the i2c gate to be mux-locked 21 [media] si2168: change the i2c gate to be mux-locked 22 [media] rtl2832: change the i2c gate to be mux-locked 23 [media] rtl2832_sdr: get rid of empty regmap wrappers 24 [media] rtl2832: regmap is aware of lockdep, drop local locking hack I have tested 1, 2, 14, 16, 17 and 18 on real hw. Antti has tested 9, 10, 11, 12, 16, 17, 21, 22, 23 and 24, but they have been rebased since and 22 in particular is not a pure rebase since the driver moved underneath me. And the locking was not 100% the same either, not that I expect things to have gone south, but... 15 is a functional no-op once 2-14 are merged. 19 is docs only. Jonathan acked 8 and 20, but 20 needs a tested-by from Daniel Baluta and/or Adriana Reus. That leaves 3, 4, 5, 6, 7, 13, but all those fall in the mechanical category, with the possible exception of 13 which is more complex than the other patches in the 2-14 range. But 13 builds, and besides, what can possibly go wrong with a patch with that number? :-) IIUC, your suggestion is to push 1-15 for 4.7, i.e. hope for the best with 3, 4, 5, 6, 7 and 13. And then hopefully get testing for 20, and retesting for 21, 22, 23 and 24 and merge 16-24 for 4.8. That seems like a good plan to me. Maybe we should give Antti some more time to re-add his tested-by tags on 9-12 before they are merged into non-rewritable branches? Cheers, Peter
[toc] | [prev] | [next] | [standalone]
| From | Wolfram Sang <wsa@the-dreams.de> |
|---|---|
| Date | 2016-04-11 18:00 +0200 |
| Message-ID | <rmMpR-8jk-57@gated-at.bofh.it> |
| In reply to | #1375864 |
[Multipart message — attachments visible in raw view] — view raw
> can obviously take forever. At the same time, many of the patches are kind > of mechanical, and feels rather safe. I agree about the mechanical stuff, thus my suggestion. We do what we can about testing and reviewing. And once it reaches linux-next (hopefully next week latest), test coverage will increase significantly and we can fix issues incrementally from there on. Same goes when it finally hits Linus' tree, coverage will increase more, but we should be really at a very sane level then ;) I will also pick up patch 15 (the removal) for 4.8. So we have the full 4.7 cycle to revert if something goes very wrong. > Maybe we should give Antti some more time to re-add his tested-by tags > on 9-12 before they are merged into non-rewritable branches? Yes. That would be great. I need to sync with the media maintainers anyhow. I'd like to push all the patches via my tree. Thanks, Wolfram
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web