Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1301649 > unrolled thread

[PATCH v2 0/8] i2c mux cleanup and locking update

Started byPeter Rosin <peda@lysator.liu.se>
First post2016-01-05 17:00 +0100
Last post2016-01-07 09:20 +0100
Articles 8 — 4 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 0/8] i2c mux cleanup and locking update Peter Rosin <peda@lysator.liu.se> - 2016-01-05 17:00 +0100
    [PATCH v2 2/8] i2c-mux: move select and deselect ops to i2c_mux_core Peter Rosin <peda@lysator.liu.se> - 2016-01-05 17:10 +0100
    [PATCH v2 4/8] i2c-mux: remove the mux dev pointer from the mux per channel data Peter Rosin <peda@lysator.liu.se> - 2016-01-05 17:10 +0100
    Re: [PATCH v2 0/8] i2c mux cleanup and locking update Wolfram Sang <wsa@the-dreams.de> - 2016-01-05 20:00 +0100
      Re: [PATCH v2 0/8] i2c mux cleanup and locking update Peter Rosin <peda@lysator.liu.se> - 2016-01-05 20:10 +0100
      Re: [PATCH v2 0/8] i2c mux cleanup and locking update Crt Mori <cmo@melexis.com> - 2016-01-06 14:30 +0100
    Re: [PATCH v2 0/8] i2c mux cleanup and locking update Antti Palosaari <crope@iki.fi> - 2016-01-06 18:20 +0100
      Re: [PATCH v2 0/8] i2c mux cleanup and locking update Peter Rosin <peda@lysator.liu.se> - 2016-01-07 09:20 +0100

#1301649 — [PATCH v2 0/8] i2c mux cleanup and locking update

FromPeter Rosin <peda@lysator.liu.se>
Date2016-01-05 17:00 +0100
Subject[PATCH v2 0/8] i2c mux cleanup and locking update
Message-ID<qNCbE-4Sl-5@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, and 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 is 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 a dt property to i2c-mux-gpio and
i2c-mux-pinctrl that can be used to state that the mux is updated by means
of the muxed master bus, and that the select-transfer-deselect operations
should be locked individually. When this holds, the i2c bus *is* 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 i2s master bus). A lock
is added to the mux so that transfers through the mux 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 of 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.

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 the last three patches of the series, with the real
meat in 8/8.

PS. needs a bunch of testing, I do not have access to all the involved hw

Changes since v1:
- Allocate mux core and (optional) priv in a combined allocation.
- Killed dev_err messages triggered by memory allocation failure.
- Fix the device specific i2c muxes that I had overlooked.
- Rebased on top of v4.4-rc8 (was based on v4.4-rc6 previously).

Cheers,
Peter

Peter Rosin (8):
  i2c-mux: add common core data for every mux instance
  i2c-mux: move select and deselect ops to i2c_mux_core
  i2c-mux: move the slave side adapter management to i2c_mux_core
  i2c-mux: remove the mux dev pointer from the mux per channel data
  i2c-mux: pinctrl: get rid of the driver private struct device pointer
  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 i2c controlled
    muxing

 .../devicetree/bindings/i2c/i2c-mux-gpio.txt       |   2 +
 .../devicetree/bindings/i2c/i2c-mux-pinctrl.txt    |   4 +
 drivers/i2c/i2c-core.c                             |  59 ++---
 drivers/i2c/i2c-mux.c                              | 272 +++++++++++++++++----
 drivers/i2c/muxes/i2c-arb-gpio-challenge.c         |  46 ++--
 drivers/i2c/muxes/i2c-mux-gpio.c                   |  58 ++---
 drivers/i2c/muxes/i2c-mux-pca9541.c                |  58 +++--
 drivers/i2c/muxes/i2c-mux-pca954x.c                |  66 ++---
 drivers/i2c/muxes/i2c-mux-pinctrl.c                |  89 +++----
 drivers/i2c/muxes/i2c-mux-reg.c                    |  63 ++---
 drivers/iio/imu/inv_mpu6050/inv_mpu_core.c         |  33 +--
 drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h          |   2 +-
 drivers/media/dvb-frontends/m88ds3103.c            |  23 +-
 drivers/media/dvb-frontends/m88ds3103_priv.h       |   2 +-
 drivers/media/dvb-frontends/rtl2830.c              |  24 +-
 drivers/media/dvb-frontends/rtl2830_priv.h         |   2 +-
 drivers/media/dvb-frontends/rtl2832.c              |  30 ++-
 drivers/media/dvb-frontends/rtl2832_priv.h         |   2 +-
 drivers/media/dvb-frontends/si2168.c               |  29 ++-
 drivers/media/dvb-frontends/si2168_priv.h          |   2 +-
 drivers/media/usb/cx231xx/cx231xx-core.c           |   6 +-
 drivers/media/usb/cx231xx/cx231xx-i2c.c            |  48 ++--
 drivers/media/usb/cx231xx/cx231xx.h                |   4 +-
 drivers/of/unittest.c                              |  41 ++--
 include/linux/i2c-mux-gpio.h                       |   2 +
 include/linux/i2c-mux-pinctrl.h                    |   2 +
 include/linux/i2c-mux.h                            |  39 ++-
 include/linux/i2c.h                                |  28 ++-
 28 files changed, 612 insertions(+), 424 deletions(-)

-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1301657 — [PATCH v2 2/8] i2c-mux: move select and deselect ops to i2c_mux_core

FromPeter Rosin <peda@lysator.liu.se>
Date2016-01-05 17:10 +0100
Subject[PATCH v2 2/8] i2c-mux: move select and deselect ops to i2c_mux_core
Message-ID<qNClk-5bm-19@gated-at.bofh.it>
In reply to#1301649
From: Peter Rosin <peda@axentia.se>

The mux select and deselect ops are common to the mux most of the time,
so store the ops in the mux core.

Change the select and deselect op to work in terms of the mux core instead
of the child adapter. No driver uses the child adapter anyway, and if it
is needed in a future mux driver it can be worked out from the channel id.

i2c-arb-gpio-challenge is special in that it needs the mux device pointer
in the select op, so store that device pointer in the mux core as well.
This pointer is going to get further use in later commits.

i2c-mux-pca954x is special since it does not add its deselect op to all
its child adapters, handle this by adding a mask that makes the deselect
op a no-operation for child adapters not wishing to deselect the mux.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/i2c/i2c-mux.c                      | 31 ++++++++++--------------------
 drivers/i2c/muxes/i2c-arb-gpio-challenge.c | 18 ++++++++---------
 drivers/i2c/muxes/i2c-mux-gpio.c           | 19 +++++++++---------
 drivers/i2c/muxes/i2c-mux-pca9541.c        | 18 +++++++++--------
 drivers/i2c/muxes/i2c-mux-pca954x.c        | 30 +++++++++++++++++------------
 drivers/i2c/muxes/i2c-mux-pinctrl.c        | 20 ++++++++-----------
 drivers/i2c/muxes/i2c-mux-reg.c            | 21 +++++++++-----------
 drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 17 +++++++---------
 drivers/media/dvb-frontends/m88ds3103.c    |  8 ++++----
 drivers/media/dvb-frontends/rtl2830.c      |  8 ++++----
 drivers/media/dvb-frontends/rtl2832.c      | 15 ++++++++-------
 drivers/media/dvb-frontends/si2168.c       | 13 +++++++------
 drivers/media/usb/cx231xx/cx231xx-i2c.c    | 12 ++++--------
 drivers/of/unittest.c                      |  7 +++----
 include/linux/i2c-mux.h                    | 15 ++++++++-------
 15 files changed, 118 insertions(+), 134 deletions(-)

diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
index c2163f6b51d5..6c5cb9f7649b 100644
--- a/drivers/i2c/i2c-mux.c
+++ b/drivers/i2c/i2c-mux.c
@@ -32,13 +32,8 @@ struct i2c_mux_priv {
 	struct i2c_adapter adap;
 	struct i2c_algorithm algo;
 	struct i2c_mux_core *muxc;
-
 	struct device *mux_dev;
-	void *mux_priv;
 	u32 chan_id;
-
-	int (*select)(struct i2c_adapter *, void *mux_priv, u32 chan_id);
-	int (*deselect)(struct i2c_adapter *, void *mux_priv, u32 chan_id);
 };
 
 static int i2c_mux_master_xfer(struct i2c_adapter *adap,
@@ -51,11 +46,11 @@ static int i2c_mux_master_xfer(struct i2c_adapter *adap,
 
 	/* Switch to the right mux port and perform the transfer. */
 
-	ret = priv->select(parent, priv->mux_priv, priv->chan_id);
+	ret = muxc->select(muxc, priv->chan_id);
 	if (ret >= 0)
 		ret = __i2c_transfer(parent, msgs, num);
-	if (priv->deselect)
-		priv->deselect(parent, priv->mux_priv, priv->chan_id);
+	if (muxc->deselect)
+		muxc->deselect(muxc, priv->chan_id);
 
 	return ret;
 }
@@ -72,12 +67,12 @@ static int i2c_mux_smbus_xfer(struct i2c_adapter *adap,
 
 	/* Select the right mux port and perform the transfer. */
 
-	ret = priv->select(parent, priv->mux_priv, priv->chan_id);
+	ret = muxc->select(muxc, priv->chan_id);
 	if (ret >= 0)
 		ret = parent->algo->smbus_xfer(parent, addr, flags,
 					read_write, command, size, data);
-	if (priv->deselect)
-		priv->deselect(parent, priv->mux_priv, priv->chan_id);
+	if (muxc->deselect)
+		muxc->deselect(muxc, priv->chan_id);
 
 	return ret;
 }
@@ -113,18 +108,15 @@ struct i2c_mux_core *i2c_mux_alloc(struct device *dev, int sizeof_priv)
 		return NULL;
 	if (sizeof_priv)
 		muxc->priv = muxc + 1;
+	muxc->dev = dev;
 	return muxc;
 }
 EXPORT_SYMBOL_GPL(i2c_mux_alloc);
 
 struct i2c_adapter *i2c_add_mux_adapter(struct i2c_mux_core *muxc,
-				struct device *mux_dev,
-				void *mux_priv, u32 force_nr, u32 chan_id,
-				unsigned int class,
-				int (*select) (struct i2c_adapter *,
-					       void *, u32),
-				int (*deselect) (struct i2c_adapter *,
-						 void *, u32))
+					struct device *mux_dev,
+					u32 force_nr, u32 chan_id,
+					unsigned int class)
 {
 	struct i2c_adapter *parent = muxc->parent;
 	struct i2c_mux_priv *priv;
@@ -138,10 +130,7 @@ struct i2c_adapter *i2c_add_mux_adapter(struct i2c_mux_core *muxc,
 	/* Set up private adapter data */
 	priv->muxc = muxc;
 	priv->mux_dev = mux_dev;
-	priv->mux_priv = mux_priv;
 	priv->chan_id = chan_id;
-	priv->select = select;
-	priv->deselect = deselect;
 
 	/* Need to do algo dynamically because we don't know ahead
 	 * of time what sort of physical adapter we'll be dealing with.
diff --git a/drivers/i2c/muxes/i2c-arb-gpio-challenge.c b/drivers/i2c/muxes/i2c-arb-gpio-challenge.c
index 6e27ea4fb25a..1e1a479d5b61 100644
--- a/drivers/i2c/muxes/i2c-arb-gpio-challenge.c
+++ b/drivers/i2c/muxes/i2c-arb-gpio-challenge.c
@@ -58,9 +58,9 @@ struct i2c_arbitrator_data {
  *
  * Use the GPIO-based signalling protocol; return -EBUSY if we fail.
  */
-static int i2c_arbitrator_select(struct i2c_adapter *adap, void *data, u32 chan)
+static int i2c_arbitrator_select(struct i2c_mux_core *muxc, u32 chan)
 {
-	const struct i2c_arbitrator_data *arb = data;
+	const struct i2c_arbitrator_data *arb = i2c_mux_priv(muxc);
 	unsigned long stop_retry, stop_time;
 
 	/* Start a round of trying to claim the bus */
@@ -92,7 +92,7 @@ static int i2c_arbitrator_select(struct i2c_adapter *adap, void *data, u32 chan)
 	/* Give up, release our claim */
 	gpio_set_value(arb->our_gpio, arb->our_gpio_release);
 	udelay(arb->slew_delay_us);
-	dev_err(&adap->dev, "Could not claim bus, timeout\n");
+	dev_err(muxc->dev, "Could not claim bus, timeout\n");
 	return -EBUSY;
 }
 
@@ -101,10 +101,9 @@ static int i2c_arbitrator_select(struct i2c_adapter *adap, void *data, u32 chan)
  *
  * Release the I2C bus using the GPIO-based signalling protocol.
  */
-static int i2c_arbitrator_deselect(struct i2c_adapter *adap, void *data,
-				   u32 chan)
+static int i2c_arbitrator_deselect(struct i2c_mux_core *muxc, u32 chan)
 {
-	const struct i2c_arbitrator_data *arb = data;
+	const struct i2c_arbitrator_data *arb = i2c_mux_priv(muxc);
 
 	/* Release the bus and wait for the other master to notice */
 	gpio_set_value(arb->our_gpio, arb->our_gpio_release);
@@ -140,6 +139,9 @@ static int i2c_arbitrator_probe(struct platform_device *pdev)
 	arb = i2c_mux_priv(muxc);
 
 	platform_set_drvdata(pdev, muxc);
+
+	muxc->select = i2c_arbitrator_select,
+	muxc->deselect = i2c_arbitrator_deselect;
 	/* Request GPIOs */
 	ret = of_get_named_gpio_flags(np, "our-claim-gpio", 0, &gpio_flags);
 	if (!gpio_is_valid(ret)) {
@@ -203,9 +205,7 @@ static int i2c_arbitrator_probe(struct platform_device *pdev)
 	}
 
 	/* Actually add the mux adapter */
-	arb->child = i2c_add_mux_adapter(muxc, dev, arb, 0, 0, 0,
-					 i2c_arbitrator_select,
-					 i2c_arbitrator_deselect);
+	arb->child = i2c_add_mux_adapter(muxc, dev, 0, 0, 0);
 	if (!arb->child) {
 		dev_err(dev, "Failed to add adapter\n");
 		ret = -ENODEV;
diff --git a/drivers/i2c/muxes/i2c-mux-gpio.c b/drivers/i2c/muxes/i2c-mux-gpio.c
index ee43dd76a4d7..d89a0fbca4bc 100644
--- a/drivers/i2c/muxes/i2c-mux-gpio.c
+++ b/drivers/i2c/muxes/i2c-mux-gpio.c
@@ -32,18 +32,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);
 
@@ -138,7 +138,6 @@ 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;
 
@@ -180,6 +179,7 @@ static int i2c_mux_gpio_probe(struct platform_device *pdev)
 		return -EPROBE_DEFER;
 
 	muxc->parent = parent;
+	muxc->select = i2c_mux_gpio_select;
 	mux->gpio_base = gpio_base;
 
 	mux->adap = devm_kzalloc(&pdev->dev,
@@ -193,10 +193,10 @@ static int i2c_mux_gpio_probe(struct platform_device *pdev)
 
 	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;
+		muxc->deselect = NULL;
 	}
 
 	for (i = 0; i < mux->data.n_gpios; i++) {
@@ -222,9 +222,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(muxc, &pdev->dev, mux, nr,
-						   mux->data.values[i], class,
-						   i2c_mux_gpio_select, deselect);
+		mux->adap[i] = i2c_add_mux_adapter(muxc, &pdev->dev, nr,
+						   mux->data.values[i], class);
 		if (!mux->adap[i]) {
 			ret = -ENODEV;
 			dev_err(&pdev->dev, "Failed to add adapter %d\n", i);
diff --git a/drivers/i2c/muxes/i2c-mux-pca9541.c b/drivers/i2c/muxes/i2c-mux-pca9541.c
index 47ae2259d1ca..ae42039459d0 100644
--- a/drivers/i2c/muxes/i2c-mux-pca9541.c
+++ b/drivers/i2c/muxes/i2c-mux-pca9541.c
@@ -287,9 +287,10 @@ static int pca9541_arbitrate(struct i2c_client *client)
 	return 0;
 }
 
-static int pca9541_select_chan(struct i2c_adapter *adap, void *client, u32 chan)
+static int pca9541_select_chan(struct i2c_mux_core *muxc, u32 chan)
 {
-	struct pca9541 *data = i2c_get_clientdata(client);
+	struct pca9541 *data = i2c_mux_priv(muxc);
+	struct i2c_client *client = data->client;
 	int ret;
 	unsigned long timeout = jiffies + ARB2_TIMEOUT;
 		/* give up after this time */
@@ -311,9 +312,11 @@ static int pca9541_select_chan(struct i2c_adapter *adap, void *client, u32 chan)
 	return -ETIMEDOUT;
 }
 
-static int pca9541_release_chan(struct i2c_adapter *adap,
-				void *client, u32 chan)
+static int pca9541_release_chan(struct i2c_mux_core *muxc, u32 chan)
 {
+	struct pca9541 *data = i2c_mux_priv(muxc);
+	struct i2c_client *client = data->client;
+
 	pca9541_release_bus(client);
 	return 0;
 }
@@ -342,6 +345,8 @@ static int pca9541_probe(struct i2c_client *client,
 
 	data->client = client;
 	muxc->parent = adap;
+	muxc->select = pca9541_select_chan;
+	muxc->deselect = pca9541_release_chan;
 
 	/*
 	 * I2C accesses are unprotected here.
@@ -356,10 +361,7 @@ static int pca9541_probe(struct i2c_client *client,
 	force = 0;
 	if (pdata)
 		force = pdata->modes[0].adap_id;
-	data->mux_adap = i2c_add_mux_adapter(muxc, &client->dev, client,
-					     force, 0, 0,
-					     pca9541_select_chan,
-					     pca9541_release_chan);
+	data->mux_adap = i2c_add_mux_adapter(muxc, &client->dev, force, 0, 0);
 
 	if (data->mux_adap == NULL) {
 		dev_err(&client->dev, "failed to register master selector\n");
diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
index a4df831fae9d..9e9d708fb2cb 100644
--- a/drivers/i2c/muxes/i2c-mux-pca954x.c
+++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
@@ -63,6 +63,7 @@ struct pca954x {
 	struct i2c_adapter *virt_adaps[PCA954X_MAX_NCHANS];
 
 	u8 last_chan;		/* last register value */
+	u8 deselect;
 	struct i2c_client *client;
 };
 
@@ -147,10 +148,10 @@ static int pca954x_reg_write(struct i2c_adapter *adap,
 	return ret;
 }
 
-static int pca954x_select_chan(struct i2c_adapter *adap,
-			       void *client, u32 chan)
+static int pca954x_select_chan(struct i2c_mux_core *muxc, u32 chan)
 {
-	struct pca954x *data = i2c_get_clientdata(client);
+	struct pca954x *data = i2c_mux_priv(muxc);
+	struct i2c_client *client = data->client;
 	const struct chip_desc *chip = &chips[data->type];
 	u8 regval;
 	int ret = 0;
@@ -163,21 +164,24 @@ static int pca954x_select_chan(struct i2c_adapter *adap,
 
 	/* Only select the channel if its different from the last channel */
 	if (data->last_chan != regval) {
-		ret = pca954x_reg_write(adap, client, regval);
+		ret = pca954x_reg_write(muxc->parent, client, regval);
 		data->last_chan = regval;
 	}
 
 	return ret;
 }
 
-static int pca954x_deselect_mux(struct i2c_adapter *adap,
-				void *client, u32 chan)
+static int pca954x_deselect_mux(struct i2c_mux_core *muxc, u32 chan)
 {
-	struct pca954x *data = i2c_get_clientdata(client);
+	struct pca954x *data = i2c_mux_priv(muxc);
+	struct i2c_client *client = data->client;
+
+	if (!(data->deselect & (1 << chan)))
+		return 0;
 
 	/* Deselect active channel */
 	data->last_chan = 0;
-	return pca954x_reg_write(adap, client, data->last_chan);
+	return pca954x_reg_write(muxc->parent, client, data->last_chan);
 }
 
 /*
@@ -222,6 +226,8 @@ static int pca954x_probe(struct i2c_client *client,
 	}
 
 	muxc->parent = adap;
+	muxc->select = pca954x_select_chan;
+	muxc->deselect = pca954x_deselect_mux;
 	data->type = id->driver_data;
 	data->last_chan = 0;		   /* force the first selection */
 
@@ -243,13 +249,13 @@ static int pca954x_probe(struct i2c_client *client,
 				/* discard unconfigured channels */
 				break;
 			idle_disconnect_pd = pdata->modes[num].deselect_on_exit;
+			data->deselect |= (idle_disconnect_pd
+					   || idle_disconnect_dt) << num;
 		}
 
 		data->virt_adaps[num] =
-			i2c_add_mux_adapter(muxc, &client->dev, client,
-				force, num, class, pca954x_select_chan,
-				(idle_disconnect_pd || idle_disconnect_dt)
-					? pca954x_deselect_mux : NULL);
+			i2c_add_mux_adapter(muxc, &client->dev,
+					    force, num, class);
 
 		if (data->virt_adaps[num] == NULL) {
 			ret = -ENODEV;
diff --git a/drivers/i2c/muxes/i2c-mux-pinctrl.c b/drivers/i2c/muxes/i2c-mux-pinctrl.c
index 810f75f114a7..e87c8f77037a 100644
--- a/drivers/i2c/muxes/i2c-mux-pinctrl.c
+++ b/drivers/i2c/muxes/i2c-mux-pinctrl.c
@@ -34,18 +34,16 @@ struct i2c_mux_pinctrl {
 	struct i2c_adapter **busses;
 };
 
-static int i2c_mux_pinctrl_select(struct i2c_adapter *adap, void *data,
-				  u32 chan)
+static int i2c_mux_pinctrl_select(struct i2c_mux_core *muxc, u32 chan)
 {
-	struct i2c_mux_pinctrl *mux = data;
+	struct i2c_mux_pinctrl *mux = i2c_mux_priv(muxc);
 
 	return pinctrl_select_state(mux->pinctrl, mux->states[chan]);
 }
 
-static int i2c_mux_pinctrl_deselect(struct i2c_adapter *adap, void *data,
-				    u32 chan)
+static int i2c_mux_pinctrl_deselect(struct i2c_mux_core *muxc, u32 chan)
 {
-	struct i2c_mux_pinctrl *mux = data;
+	struct i2c_mux_pinctrl *mux = i2c_mux_priv(muxc);
 
 	return pinctrl_select_state(mux->pinctrl, mux->state_idle);
 }
@@ -132,7 +130,6 @@ static int i2c_mux_pinctrl_probe(struct platform_device *pdev)
 {
 	struct i2c_mux_core *muxc;
 	struct i2c_mux_pinctrl *mux;
-	int (*deselect)(struct i2c_adapter *, void *, u32);
 	int i, ret;
 
 	muxc = i2c_mux_alloc(&pdev->dev, sizeof(*mux));
@@ -203,10 +200,11 @@ static int i2c_mux_pinctrl_probe(struct platform_device *pdev)
 			goto err;
 		}
 
-		deselect = i2c_mux_pinctrl_deselect;
+		muxc->deselect = i2c_mux_pinctrl_deselect;
 	} else {
-		deselect = NULL;
+		muxc->deselect = NULL;
 	}
+	muxc->select = i2c_mux_pinctrl_select;
 
 	muxc->parent = i2c_get_adapter(mux->pdata->parent_bus_num);
 	if (!muxc->parent) {
@@ -221,9 +219,7 @@ static int i2c_mux_pinctrl_probe(struct platform_device *pdev)
 				(mux->pdata->base_bus_num + i) : 0;
 
 		mux->busses[i] = i2c_add_mux_adapter(muxc, &pdev->dev,
-						     mux, bus, i, 0,
-						     i2c_mux_pinctrl_select,
-						     deselect);
+						     bus, i, 0);
 		if (!mux->busses[i]) {
 			ret = -ENODEV;
 			dev_err(&pdev->dev, "Failed to add adapter %d\n", i);
diff --git a/drivers/i2c/muxes/i2c-mux-reg.c b/drivers/i2c/muxes/i2c-mux-reg.c
index 8bde4cfac512..3b01e7809a66 100644
--- a/drivers/i2c/muxes/i2c-mux-reg.c
+++ b/drivers/i2c/muxes/i2c-mux-reg.c
@@ -63,18 +63,16 @@ static int i2c_mux_reg_set(const struct regmux *mux, unsigned int chan_id)
 	return 0;
 }
 
-static int i2c_mux_reg_select(struct i2c_adapter *adap, void *data,
-			      unsigned int chan)
+static int i2c_mux_reg_select(struct i2c_mux_core *muxc, u32 chan)
 {
-	struct regmux *mux = data;
+	struct regmux *mux = i2c_mux_priv(muxc);
 
 	return i2c_mux_reg_set(mux, chan);
 }
 
-static int i2c_mux_reg_deselect(struct i2c_adapter *adap, void *data,
-				unsigned int chan)
+static int i2c_mux_reg_deselect(struct i2c_mux_core *muxc, u32 chan)
 {
-	struct regmux *mux = data;
+	struct regmux *mux = i2c_mux_priv(muxc);
 
 	if (mux->data.idle_in_use)
 		return i2c_mux_reg_set(mux, mux->data.idle);
@@ -173,7 +171,6 @@ static int i2c_mux_reg_probe(struct platform_device *pdev)
 	struct regmux *mux;
 	struct i2c_adapter *parent;
 	struct resource *res;
-	int (*deselect)(struct i2c_adapter *, void *, u32);
 	unsigned int class;
 	int i, ret, nr;
 
@@ -225,19 +222,19 @@ static int i2c_mux_reg_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
+	muxc->select = i2c_mux_reg_select;
 	if (mux->data.idle_in_use)
-		deselect = i2c_mux_reg_deselect;
+		muxc->deselect = i2c_mux_reg_deselect;
 	else
-		deselect = NULL;
+		muxc->deselect = NULL;
 
 	for (i = 0; i < mux->data.n_values; i++) {
 		nr = mux->data.base_nr ? (mux->data.base_nr + i) : 0;
 		class = mux->data.classes ? mux->data.classes[i] : 0;
 
-		mux->adap[i] = i2c_add_mux_adapter(muxc, &pdev->dev, mux,
+		mux->adap[i] = i2c_add_mux_adapter(muxc, &pdev->dev,
 						   nr, mux->data.values[i],
-						   class, i2c_mux_reg_select,
-						   deselect);
+						   class);
 		if (!mux->adap[i]) {
 			ret = -ENODEV;
 			dev_err(&pdev->dev, "Failed to add adapter %d\n", i);
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
index 3aab0d7a1bdc..0a47396bc5be 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
@@ -109,10 +109,9 @@ static int inv_mpu6050_write_reg_unlocked(struct inv_mpu6050_state *st,
 	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 iio_dev *indio_dev = mux_priv;
+	struct iio_dev *indio_dev = i2c_mux_priv(muxc);
 	struct inv_mpu6050_state *st = iio_priv(indio_dev);
 	int ret = 0;
 
@@ -138,10 +137,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 iio_dev *indio_dev = mux_priv;
+	struct iio_dev *indio_dev = i2c_mux_priv(muxc);
 	struct inv_mpu6050_state *st = iio_priv(indio_dev);
 
 	mutex_lock(&indio_dev->mlock);
@@ -849,13 +847,12 @@ static int inv_mpu_probe(struct i2c_client *client,
 	}
 	st->muxc->priv = indio_dev;
 	st->muxc->parent = client->adapter;
+	st->muxc->select = inv_mpu6050_select_bypass;
+	st->muxc->deselect = inv_mpu6050_deselect_bypass;
 
 	st->mux_adapter = i2c_add_mux_adapter(st->muxc,
 					      &client->dev,
-					      indio_dev,
-					      0, 0, 0,
-					      inv_mpu6050_select_bypass,
-					      inv_mpu6050_deselect_bypass);
+					      0, 0, 0);
 	if (!st->mux_adapter) {
 		result = -ENODEV;
 		goto out_unreg_device;
diff --git a/drivers/media/dvb-frontends/m88ds3103.c b/drivers/media/dvb-frontends/m88ds3103.c
index a0006aec6937..c9f8296ea421 100644
--- a/drivers/media/dvb-frontends/m88ds3103.c
+++ b/drivers/media/dvb-frontends/m88ds3103.c
@@ -1251,9 +1251,9 @@ static void m88ds3103_release(struct dvb_frontend *fe)
 	i2c_unregister_device(client);
 }
 
-static int m88ds3103_select(struct i2c_adapter *adap, void *mux_priv, u32 chan)
+static int m88ds3103_select(struct i2c_mux_core *muxc, u32 chan)
 {
-	struct m88ds3103_dev *dev = mux_priv;
+	struct m88ds3103_dev *dev = i2c_mux_priv(muxc);
 	struct i2c_client *client = dev->client;
 	int ret;
 	struct i2c_msg msg = {
@@ -1473,11 +1473,11 @@ static int m88ds3103_probe(struct i2c_client *client,
 	}
 	dev->muxc->priv = dev;
 	dev->muxc->parent = client->adapter;
+	dev->muxc->select = m88ds3103_select;
 
 	/* create mux i2c adapter for tuner */
 	dev->i2c_adapter = i2c_add_mux_adapter(dev->muxc, &client->dev,
-					       dev, 0, 0, 0, m88ds3103_select,
-					       NULL);
+					       0, 0, 0);
 	if (dev->i2c_adapter == NULL) {
 		ret = -ENOMEM;
 		goto err_kfree;
diff --git a/drivers/media/dvb-frontends/rtl2830.c b/drivers/media/dvb-frontends/rtl2830.c
index ebf28b49cab2..d6330e8d5fa4 100644
--- a/drivers/media/dvb-frontends/rtl2830.c
+++ b/drivers/media/dvb-frontends/rtl2830.c
@@ -677,9 +677,9 @@ err:
  * adapter lock is already taken by tuner driver.
  * Gate is closed automatically after single I2C transfer.
  */
-static int rtl2830_select(struct i2c_adapter *adap, void *mux_priv, u32 chan_id)
+static int rtl2830_select(struct i2c_mux_core *muxc, u32 chan_id)
 {
-	struct i2c_client *client = mux_priv;
+	struct i2c_client *client = i2c_mux_priv(muxc);
 	struct rtl2830_dev *dev = i2c_get_clientdata(client);
 	int ret;
 
@@ -871,10 +871,10 @@ static int rtl2830_probe(struct i2c_client *client,
 	}
 	dev->muxc->priv = client;
 	dev->muxc->parent = client->adapter;
+	dev->muxc->select = rtl2830_select;
 
 	/* create muxed i2c adapter for tuner */
-	dev->adapter = i2c_add_mux_adapter(dev->muxc, &client->dev,
-			client, 0, 0, 0, rtl2830_select, NULL);
+	dev->adapter = i2c_add_mux_adapter(dev->muxc, &client->dev, 0, 0, 0);
 	if (dev->adapter == NULL) {
 		ret = -ENODEV;
 		goto err_regmap_exit;
diff --git a/drivers/media/dvb-frontends/rtl2832.c b/drivers/media/dvb-frontends/rtl2832.c
index 38402ad3ecdd..c8fd990fdae8 100644
--- a/drivers/media/dvb-frontends/rtl2832.c
+++ b/drivers/media/dvb-frontends/rtl2832.c
@@ -866,9 +866,9 @@ err:
 	dev_dbg(&client->dev, "failed=%d\n", ret);
 }
 
-static int rtl2832_select(struct i2c_adapter *adap, void *mux_priv, u32 chan_id)
+static int rtl2832_select(struct i2c_mux_core *muxc, u32 chan_id)
 {
-	struct rtl2832_dev *dev = mux_priv;
+	struct rtl2832_dev *dev = i2c_mux_priv(muxc);
 	struct i2c_client *client = dev->client;
 	int ret;
 
@@ -889,10 +889,9 @@ err:
 	return ret;
 }
 
-static int rtl2832_deselect(struct i2c_adapter *adap, void *mux_priv,
-			    u32 chan_id)
+static int rtl2832_deselect(struct i2c_mux_core *muxc, u32 chan_id)
 {
-	struct rtl2832_dev *dev = mux_priv;
+	struct rtl2832_dev *dev = i2c_mux_priv(muxc);
 
 	schedule_delayed_work(&dev->i2c_gate_work, usecs_to_jiffies(100));
 	return 0;
@@ -1268,10 +1267,12 @@ static int rtl2832_probe(struct i2c_client *client,
 	}
 	dev->muxc->priv = dev;
 	dev->muxc->parent = i2c;
+	dev->muxc->select = rtl2832_select;
+	dev->muxc->deselect = rtl2832_deselect;
 
 	/* create muxed i2c adapter for demod tuner bus */
-	dev->i2c_adapter_tuner = i2c_add_mux_adapter(dev->muxc, &i2c->dev, dev,
-			0, 0, 0, rtl2832_select, rtl2832_deselect);
+	dev->i2c_adapter_tuner = i2c_add_mux_adapter(dev->muxc, &i2c->dev,
+						     0, 0, 0);
 	if (dev->i2c_adapter_tuner == NULL) {
 		ret = -ENODEV;
 		goto err_regmap_exit;
diff --git a/drivers/media/dvb-frontends/si2168.c b/drivers/media/dvb-frontends/si2168.c
index a52756bf9834..5b1872b1bbf4 100644
--- a/drivers/media/dvb-frontends/si2168.c
+++ b/drivers/media/dvb-frontends/si2168.c
@@ -615,9 +615,9 @@ static int si2168_get_tune_settings(struct dvb_frontend *fe,
  * 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_adapter *adap, void *mux_priv, u32 chan)
+static int si2168_select(struct i2c_mux_core *muxc, u32 chan)
 {
-	struct i2c_client *client = mux_priv;
+	struct i2c_client *client = i2c_mux_priv(muxc);
 	int ret;
 	struct si2168_cmd cmd;
 
@@ -635,9 +635,9 @@ err:
 	return ret;
 }
 
-static int si2168_deselect(struct i2c_adapter *adap, void *mux_priv, u32 chan)
+static int si2168_deselect(struct i2c_mux_core *muxc, u32 chan)
 {
-	struct i2c_client *client = mux_priv;
+	struct i2c_client *client = i2c_mux_priv(muxc);
 	int ret;
 	struct si2168_cmd cmd;
 
@@ -715,10 +715,11 @@ static int si2168_probe(struct i2c_client *client,
 	}
 	dev->muxc->priv = client;
 	dev->muxc->parent = client->adapter;
+	dev->muxc->select = si2168_select;
+	dev->muxc->deselect = si2168_deselect;
 
 	/* create mux i2c adapter for tuner */
-	dev->adapter = i2c_add_mux_adapter(dev->muxc, &client->dev,
-			client, 0, 0, 0, si2168_select, si2168_deselect);
+	dev->adapter = i2c_add_mux_adapter(dev->muxc, &client->dev, 0, 0, 0);
 	if (dev->adapter == NULL) {
 		ret = -ENODEV;
 		goto err_kfree;
diff --git a/drivers/media/usb/cx231xx/cx231xx-i2c.c b/drivers/media/usb/cx231xx/cx231xx-i2c.c
index 09c30e753ca5..51760bfc7cbc 100644
--- a/drivers/media/usb/cx231xx/cx231xx-i2c.c
+++ b/drivers/media/usb/cx231xx/cx231xx-i2c.c
@@ -557,10 +557,9 @@ int cx231xx_i2c_unregister(struct cx231xx_i2c *bus)
  * cx231xx_i2c_mux_select()
  * switch i2c master number 1 between port1 and port3
  */
-static int cx231xx_i2c_mux_select(struct i2c_adapter *adap,
-			void *mux_priv, u32 chan_id)
+static int cx231xx_i2c_mux_select(struct i2c_mux_core *muxc, u32 chan_id)
 {
-	struct cx231xx *dev = mux_priv;
+	struct cx231xx *dev = i2c_mux_priv(muxc);
 
 	return cx231xx_enable_i2c_port_3(dev, chan_id);
 }
@@ -572,6 +571,7 @@ int cx231xx_i2c_mux_create(struct cx231xx *dev)
 		return -ENOMEM;
 	dev->muxc->priv = dev;
 	dev->muxc->parent = &dev->i2c_bus[1].i2c_adap;
+	dev->muxc->select = cx231xx_i2c_mux_select;
 	return 0;
 }
 
@@ -582,13 +582,9 @@ int cx231xx_i2c_mux_register(struct cx231xx *dev, int mux_no)
 
 	dev->i2c_mux_adap[mux_no] = i2c_add_mux_adapter(dev->muxc,
 				mux_dev,
-				dev /* mux_priv */,
 				0,
 				mux_no /* chan_id */,
-				0 /* class */,
-				&cx231xx_i2c_mux_select,
-				NULL);
-
+				0 /* class */);
 	if (!dev->i2c_mux_adap[mux_no])
 		dev_warn(dev->dev,
 			 "i2c mux %d register FAILED\n", mux_no);
diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index 6bfc2f9a5a24..a4abd9b588b9 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -1682,8 +1682,7 @@ struct unittest_i2c_mux_data {
 	struct i2c_adapter *adap[];
 };
 
-static int unittest_i2c_mux_select_chan(struct i2c_adapter *adap,
-			       void *client, u32 chan)
+static int unittest_i2c_mux_select_chan(struct i2c_mux_core *muxc, u32 chan)
 {
 	return 0;
 }
@@ -1725,11 +1724,11 @@ static int unittest_i2c_mux_probe(struct i2c_client *client,
 	if (!muxc)
 		return -ENOMEM;
 	muxc->parent = adap;
+	muxc->select = unittest_i2c_mux_select_chan;
 	stm = i2c_mux_priv(muxc);
 	stm->nchans = nchans;
 	for (i = 0; i < nchans; i++) {
-		stm->adap[i] = i2c_add_mux_adapter(muxc, dev, client,
-				0, i, 0, unittest_i2c_mux_select_chan, NULL);
+		stm->adap[i] = i2c_add_mux_adapter(muxc, dev, 0, i, 0);
 		if (!stm->adap[i]) {
 			dev_err(dev, "Failed to register mux #%d\n", i);
 			for (i--; i >= 0; i--)
diff --git a/include/linux/i2c-mux.h b/include/linux/i2c-mux.h
index 3ca1783b86ac..5cd6e1e664e0 100644
--- a/include/linux/i2c-mux.h
+++ b/include/linux/i2c-mux.h
@@ -29,7 +29,12 @@
 
 struct i2c_mux_core {
 	struct i2c_adapter *parent;
+	struct device *dev;
+
 	void *priv;
+
+	int (*select)(struct i2c_mux_core *, u32 chan_id);
+	int (*deselect)(struct i2c_mux_core *, u32 chan_id);
 };
 
 struct i2c_mux_core *i2c_mux_alloc(struct device *dev, int sizeof_priv);
@@ -46,13 +51,9 @@ static inline void *i2c_mux_priv(struct i2c_mux_core *muxc)
  * mux control.
  */
 struct i2c_adapter *i2c_add_mux_adapter(struct i2c_mux_core *muxc,
-				struct device *mux_dev,
-				void *mux_priv, u32 force_nr, u32 chan_id,
-				unsigned int class,
-				int (*select) (struct i2c_adapter *,
-					       void *mux_dev, u32 chan_id),
-				int (*deselect) (struct i2c_adapter *,
-						 void *mux_dev, u32 chan_id));
+					struct device *mux_dev,
+					u32 force_nr, u32 chan_id,
+					unsigned int class);
 
 void i2c_del_mux_adapter(struct i2c_adapter *adap);
 
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1301660 — [PATCH v2 4/8] i2c-mux: remove the mux dev pointer from the mux per channel data

FromPeter Rosin <peda@lysator.liu.se>
Date2016-01-05 17:10 +0100
Subject[PATCH v2 4/8] i2c-mux: remove the mux dev pointer from the mux per channel data
Message-ID<qNCll-5bm-43@gated-at.bofh.it>
In reply to#1301649
From: Peter Rosin <peda@axentia.se>

The dev pointer is readily available in the mux core struct, no point in
keeping multiple copies around.

The patch also fixes a bug in rtl2832, which attached its mux slave
adapter to the device owning the mux parent adapter instead of
attaching it to its own device.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/i2c/i2c-mux.c                      | 24 ++++++++++++------------
 drivers/i2c/muxes/i2c-arb-gpio-challenge.c |  2 +-
 drivers/i2c/muxes/i2c-mux-gpio.c           |  3 +--
 drivers/i2c/muxes/i2c-mux-pca9541.c        |  2 +-
 drivers/i2c/muxes/i2c-mux-pca954x.c        |  3 +--
 drivers/i2c/muxes/i2c-mux-pinctrl.c        |  3 +--
 drivers/i2c/muxes/i2c-mux-reg.c            |  3 +--
 drivers/iio/imu/inv_mpu6050/inv_mpu_core.c |  2 +-
 drivers/media/dvb-frontends/m88ds3103.c    |  2 +-
 drivers/media/dvb-frontends/rtl2830.c      |  2 +-
 drivers/media/dvb-frontends/rtl2832.c      |  2 +-
 drivers/media/dvb-frontends/si2168.c       |  2 +-
 drivers/media/usb/cx231xx/cx231xx-i2c.c    |  3 ---
 drivers/of/unittest.c                      |  2 +-
 include/linux/i2c-mux.h                    |  1 -
 15 files changed, 24 insertions(+), 32 deletions(-)

diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
index 7ba0308537a8..5c1088079231 100644
--- a/drivers/i2c/i2c-mux.c
+++ b/drivers/i2c/i2c-mux.c
@@ -32,7 +32,6 @@ struct i2c_mux_priv {
 	struct i2c_adapter adap;
 	struct i2c_algorithm algo;
 	struct i2c_mux_core *muxc;
-	struct device *mux_dev;
 	u32 chan_id;
 };
 
@@ -137,7 +136,6 @@ struct i2c_mux_core *i2c_mux_alloc(struct device *dev, int sizeof_priv)
 EXPORT_SYMBOL_GPL(i2c_mux_alloc);
 
 int i2c_add_mux_adapter(struct i2c_mux_core *muxc,
-			struct device *mux_dev,
 			u32 force_nr, u32 chan_id,
 			unsigned int class)
 {
@@ -162,7 +160,6 @@ int i2c_add_mux_adapter(struct i2c_mux_core *muxc,
 
 	/* Set up private adapter data */
 	priv->muxc = muxc;
-	priv->mux_dev = mux_dev;
 	priv->chan_id = chan_id;
 
 	/* Need to do algo dynamically because we don't know ahead
@@ -197,11 +194,11 @@ int i2c_add_mux_adapter(struct i2c_mux_core *muxc,
 	 * Try to populate the mux adapter's of_node, expands to
 	 * nothing if !CONFIG_OF.
 	 */
-	if (mux_dev->of_node) {
+	if (muxc->dev->of_node) {
 		struct device_node *child;
 		u32 reg;
 
-		for_each_child_of_node(mux_dev->of_node, child) {
+		for_each_child_of_node(muxc->dev->of_node, child) {
 			ret = of_property_read_u32(child, "reg", &reg);
 			if (ret)
 				continue;
@@ -215,8 +212,9 @@ int i2c_add_mux_adapter(struct i2c_mux_core *muxc,
 	/*
 	 * Associate the mux channel with an ACPI node.
 	 */
-	if (has_acpi_companion(mux_dev))
-		acpi_preset_companion(&priv->adap.dev, ACPI_COMPANION(mux_dev),
+	if (has_acpi_companion(muxc->dev))
+		acpi_preset_companion(&priv->adap.dev,
+				      ACPI_COMPANION(muxc->dev),
 				      chan_id);
 
 	if (force_nr) {
@@ -233,12 +231,14 @@ int i2c_add_mux_adapter(struct i2c_mux_core *muxc,
 		return ret;
 	}
 
-	WARN(sysfs_create_link(&priv->adap.dev.kobj, &mux_dev->kobj, "mux_device"),
-			       "can't create symlink to mux device\n");
+	WARN(sysfs_create_link(&priv->adap.dev.kobj, &muxc->dev->kobj,
+			       "mux_device"),
+	     "can't create symlink to mux device\n");
 
 	snprintf(symlink_name, sizeof(symlink_name), "channel-%u", chan_id);
-	WARN(sysfs_create_link(&mux_dev->kobj, &priv->adap.dev.kobj, symlink_name),
-			       "can't create symlink for channel %u\n", chan_id);
+	WARN(sysfs_create_link(&muxc->dev->kobj, &priv->adap.dev.kobj,
+			       symlink_name),
+	     "can't create symlink for channel %u\n", chan_id);
 	dev_info(&parent->dev, "Added multiplexed i2c bus %d\n",
 		 i2c_adapter_id(&priv->adap));
 
@@ -259,7 +259,7 @@ void i2c_del_mux_adapters(struct i2c_mux_core *muxc)
 
 		snprintf(symlink_name, sizeof(symlink_name),
 			 "channel-%u", priv->chan_id);
-		sysfs_remove_link(&priv->mux_dev->kobj, symlink_name);
+		sysfs_remove_link(&muxc->dev->kobj, symlink_name);
 
 		sysfs_remove_link(&priv->adap.dev.kobj, "mux_device");
 		i2c_del_adapter(adap);
diff --git a/drivers/i2c/muxes/i2c-arb-gpio-challenge.c b/drivers/i2c/muxes/i2c-arb-gpio-challenge.c
index e0558e8a0e74..c2bc18c7921f 100644
--- a/drivers/i2c/muxes/i2c-arb-gpio-challenge.c
+++ b/drivers/i2c/muxes/i2c-arb-gpio-challenge.c
@@ -204,7 +204,7 @@ static int i2c_arbitrator_probe(struct platform_device *pdev)
 	}
 
 	/* Actually add the mux adapter */
-	ret = i2c_add_mux_adapter(muxc, dev, 0, 0, 0);
+	ret = i2c_add_mux_adapter(muxc, 0, 0, 0);
 	if (ret) {
 		dev_err(dev, "Failed to add adapter\n");
 		i2c_put_adapter(muxc->parent);
diff --git a/drivers/i2c/muxes/i2c-mux-gpio.c b/drivers/i2c/muxes/i2c-mux-gpio.c
index 6bd41ace81d4..e800c4597fa4 100644
--- a/drivers/i2c/muxes/i2c-mux-gpio.c
+++ b/drivers/i2c/muxes/i2c-mux-gpio.c
@@ -216,8 +216,7 @@ 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;
 
-		ret = i2c_add_mux_adapter(muxc, &pdev->dev, nr,
-					  mux->data.values[i], class);
+		ret = i2c_add_mux_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;
diff --git a/drivers/i2c/muxes/i2c-mux-pca9541.c b/drivers/i2c/muxes/i2c-mux-pca9541.c
index 80de0a0977a5..0e18d25334b5 100644
--- a/drivers/i2c/muxes/i2c-mux-pca9541.c
+++ b/drivers/i2c/muxes/i2c-mux-pca9541.c
@@ -361,7 +361,7 @@ static int pca9541_probe(struct i2c_client *client,
 	force = 0;
 	if (pdata)
 		force = pdata->modes[0].adap_id;
-	ret = i2c_add_mux_adapter(muxc, &client->dev, force, 0, 0);
+	ret = i2c_add_mux_adapter(muxc, force, 0, 0);
 	if (ret) {
 		dev_err(&client->dev, "failed to register master selector\n");
 		return ret;
diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
index 640670b604f5..2d15325b6282 100644
--- a/drivers/i2c/muxes/i2c-mux-pca954x.c
+++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
@@ -256,8 +256,7 @@ static int pca954x_probe(struct i2c_client *client,
 					   || idle_disconnect_dt) << num;
 		}
 
-		ret = i2c_add_mux_adapter(muxc, &client->dev,
-					  force, num, class);
+		ret = i2c_add_mux_adapter(muxc, force, num, class);
 
 		if (ret) {
 			dev_err(&client->dev,
diff --git a/drivers/i2c/muxes/i2c-mux-pinctrl.c b/drivers/i2c/muxes/i2c-mux-pinctrl.c
index 3bbb3fb1d693..0dc912898813 100644
--- a/drivers/i2c/muxes/i2c-mux-pinctrl.c
+++ b/drivers/i2c/muxes/i2c-mux-pinctrl.c
@@ -212,8 +212,7 @@ static int i2c_mux_pinctrl_probe(struct platform_device *pdev)
 		u32 bus = mux->pdata->base_bus_num ?
 				(mux->pdata->base_bus_num + i) : 0;
 
-		ret = i2c_add_mux_adapter(muxc, &pdev->dev,
-					  bus, i, 0);
+		ret = i2c_add_mux_adapter(muxc, bus, i, 0);
 		if (ret) {
 			dev_err(&pdev->dev, "Failed to add adapter %d\n", i);
 			goto err_del_adapter;
diff --git a/drivers/i2c/muxes/i2c-mux-reg.c b/drivers/i2c/muxes/i2c-mux-reg.c
index 5c004ff5b6ad..3c919e49260c 100644
--- a/drivers/i2c/muxes/i2c-mux-reg.c
+++ b/drivers/i2c/muxes/i2c-mux-reg.c
@@ -227,8 +227,7 @@ static int i2c_mux_reg_probe(struct platform_device *pdev)
 		nr = mux->data.base_nr ? (mux->data.base_nr + i) : 0;
 		class = mux->data.classes ? mux->data.classes[i] : 0;
 
-		ret = i2c_add_mux_adapter(muxc, &pdev->dev, nr,
-					  mux->data.values[i], class);
+		ret = i2c_add_mux_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;
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
index a9a163c1c22c..5cd3c48682e6 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
@@ -850,7 +850,7 @@ static int inv_mpu_probe(struct i2c_client *client,
 	st->muxc->select = inv_mpu6050_select_bypass;
 	st->muxc->deselect = inv_mpu6050_deselect_bypass;
 
-	result = i2c_add_mux_adapter(st->muxc, &client->dev, 0, 0, 0);
+	result = i2c_add_mux_adapter(st->muxc, 0, 0, 0);
 	if (result)
 		goto out_unreg_device;
 
diff --git a/drivers/media/dvb-frontends/m88ds3103.c b/drivers/media/dvb-frontends/m88ds3103.c
index deab5cdba01f..45ad3ef82b4f 100644
--- a/drivers/media/dvb-frontends/m88ds3103.c
+++ b/drivers/media/dvb-frontends/m88ds3103.c
@@ -1476,7 +1476,7 @@ static int m88ds3103_probe(struct i2c_client *client,
 	dev->muxc->select = m88ds3103_select;
 
 	/* create mux i2c adapter for tuner */
-	ret = i2c_add_mux_adapter(dev->muxc, &client->dev, 0, 0, 0);
+	ret = i2c_add_mux_adapter(dev->muxc, 0, 0, 0);
 	if (ret)
 		goto err_kfree;
 
diff --git a/drivers/media/dvb-frontends/rtl2830.c b/drivers/media/dvb-frontends/rtl2830.c
index 9864740722dd..1da8d2e22983 100644
--- a/drivers/media/dvb-frontends/rtl2830.c
+++ b/drivers/media/dvb-frontends/rtl2830.c
@@ -874,7 +874,7 @@ static int rtl2830_probe(struct i2c_client *client,
 	dev->muxc->select = rtl2830_select;
 
 	/* create muxed i2c adapter for tuner */
-	ret = i2c_add_mux_adapter(dev->muxc, &client->dev, 0, 0, 0);
+	ret = i2c_add_mux_adapter(dev->muxc, 0, 0, 0);
 	if (ret)
 		goto err_regmap_exit;
 
diff --git a/drivers/media/dvb-frontends/rtl2832.c b/drivers/media/dvb-frontends/rtl2832.c
index 99d8dbf66fd7..c586150623f7 100644
--- a/drivers/media/dvb-frontends/rtl2832.c
+++ b/drivers/media/dvb-frontends/rtl2832.c
@@ -1271,7 +1271,7 @@ static int rtl2832_probe(struct i2c_client *client,
 	dev->muxc->deselect = rtl2832_deselect;
 
 	/* create muxed i2c adapter for demod tuner bus */
-	ret = i2c_add_mux_adapter(dev->muxc, &i2c->dev, 0, 0, 0);
+	ret = i2c_add_mux_adapter(dev->muxc, 0, 0, 0);
 	if (ret)
 		goto err_regmap_exit;
 
diff --git a/drivers/media/dvb-frontends/si2168.c b/drivers/media/dvb-frontends/si2168.c
index 06aa496cc42c..ae217b5e6618 100644
--- a/drivers/media/dvb-frontends/si2168.c
+++ b/drivers/media/dvb-frontends/si2168.c
@@ -719,7 +719,7 @@ static int si2168_probe(struct i2c_client *client,
 	dev->muxc->deselect = si2168_deselect;
 
 	/* create mux i2c adapter for tuner */
-	ret = i2c_add_mux_adapter(dev->muxc, &client->dev, 0, 0, 0);
+	ret = i2c_add_mux_adapter(dev->muxc, 0, 0, 0);
 	if (ret)
 		goto err_kfree;
 
diff --git a/drivers/media/usb/cx231xx/cx231xx-i2c.c b/drivers/media/usb/cx231xx/cx231xx-i2c.c
index 2b5adb056827..bfa63cf69235 100644
--- a/drivers/media/usb/cx231xx/cx231xx-i2c.c
+++ b/drivers/media/usb/cx231xx/cx231xx-i2c.c
@@ -577,12 +577,9 @@ int cx231xx_i2c_mux_create(struct cx231xx *dev)
 
 int cx231xx_i2c_mux_register(struct cx231xx *dev, int mux_no)
 {
-	/* what is the correct mux_dev? */
-	struct device *mux_dev = dev->dev;
 	int rc;
 
 	rc = i2c_add_mux_adapter(dev->muxc,
-				 mux_dev,
 				 0,
 				 mux_no /* chan_id */,
 				 0 /* class */);
diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index 77ccc54cfdc9..46897ed4e396 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -1722,7 +1722,7 @@ static int unittest_i2c_mux_probe(struct i2c_client *client,
 	if (ret)
 		return ret;
 	for (i = 0; i < nchans; i++) {
-		ret = i2c_add_mux_adapter(muxc, dev, 0, i, 0);
+		ret = i2c_add_mux_adapter(muxc, 0, i, 0);
 		if (ret) {
 			dev_err(dev, "Failed to register mux #%d\n", i);
 			i2c_del_mux_adapters(muxc);
diff --git a/include/linux/i2c-mux.h b/include/linux/i2c-mux.h
index bfcdcc46f2a6..d88e0a3b6768 100644
--- a/include/linux/i2c-mux.h
+++ b/include/linux/i2c-mux.h
@@ -56,7 +56,6 @@ int i2c_mux_reserve_adapters(struct i2c_mux_core *muxc, int adapters);
  * mux control.
  */
 int i2c_add_mux_adapter(struct i2c_mux_core *muxc,
-			struct device *mux_dev,
 			u32 force_nr, u32 chan_id,
 			unsigned int class);
 
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1301889

FromWolfram Sang <wsa@the-dreams.de>
Date2016-01-05 20:00 +0100
Message-ID<qNEZR-6P2-51@gated-at.bofh.it>
In reply to#1301649

[Multipart message — attachments visible in raw view] — view raw

Peter,

> PS. needs a bunch of testing, I do not have access to all the involved hw

First of all, thanks for diving into this topic and the huge effort you
apparently have put into it.

It is obviously a quite intrusive series, so it needs careful review.
TBH, I can't really tell when I have the bandwidth to do that, so I hope
other people will step up. And yes, it needs serious testing.

To all: Although I appreciate any review support, I'd think the first
thing to be done should be a very high level review - is this series
worth the huge update? Is the path chosen proper? Stuff like this. I'd
appreciate Acks or Revs for that. Stuff like fixing checkpatch warnings
and other minor stuff should come later.

Thanks,

   Wolfram

[toc] | [prev] | [next] | [standalone]

#1301893

FromPeter Rosin <peda@lysator.liu.se>
Date2016-01-05 20:10 +0100
Message-ID<qNF9v-77Z-3@gated-at.bofh.it>
In reply to#1301889
Hi Wolfram,

On 2016-01-05 19:48, Wolfram Sang wrote:
> Peter,
> 
>> PS. needs a bunch of testing, I do not have access to all the involved hw
> 
> First of all, thanks for diving into this topic and the huge effort you
> apparently have put into it.

Yeah, I started with dipping just the toes, but now it rather feels like
I'm fully submerged at the deep end...

> It is obviously a quite intrusive series, so it needs careful review.
> TBH, I can't really tell when I have the bandwidth to do that, so I hope
> other people will step up. And yes, it needs serious testing.
> 
> To all: Although I appreciate any review support, I'd think the first
> thing to be done should be a very high level review - is this series
> worth the huge update? Is the path chosen proper? Stuff like this. I'd
> appreciate Acks or Revs for that. Stuff like fixing checkpatch warnings
> and other minor stuff should come later.

Right, I'll hold back on sending updates for trivial stuff until the
big picture stuff has been cleared.

Cheers,
Peter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1302760

FromCrt Mori <cmo@melexis.com>
Date2016-01-06 14:30 +0100
Message-ID<qNWk2-1HB-15@gated-at.bofh.it>
In reply to#1301889
Hi Wolfram and Peter,
I will give my opinion about the path chosen although it should be
taken lightly.

I can see that hardware guys missed the software guys again on the
development path, but since this happens more often than not, I would
say it seems OK to have support for this as long as it does not make
more complex (longer) standard i2c transfers. I would support to have
additional mutex before mux as that will make less chance that someone
forgets to lock mutex before mux and proposed solution seems valid.

Regards,
Crt

On 5 January 2016 at 19:48, Wolfram Sang <wsa@the-dreams.de> wrote:
> Peter,
>
>> PS. needs a bunch of testing, I do not have access to all the involved hw
>
> First of all, thanks for diving into this topic and the huge effort you
> apparently have put into it.
>
> It is obviously a quite intrusive series, so it needs careful review.
> TBH, I can't really tell when I have the bandwidth to do that, so I hope
> other people will step up. And yes, it needs serious testing.
>
> To all: Although I appreciate any review support, I'd think the first
> thing to be done should be a very high level review - is this series
> worth the huge update? Is the path chosen proper? Stuff like this. I'd
> appreciate Acks or Revs for that. Stuff like fixing checkpatch warnings
> and other minor stuff should come later.
>
> Thanks,
>
>    Wolfram
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1302908

FromAntti Palosaari <crope@iki.fi>
Date2016-01-06 18:20 +0100
Message-ID<qNZUD-476-13@gated-at.bofh.it>
In reply to#1301649
On 01/05/2016 05:57 PM, Peter Rosin wrote:
> 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)

Handling of I2C muxes that close channel automatically, after the first 
I2C stop (P) is seen?

For example channel is selected to BAT1 => there is EEPROM write => mux 
closes channel BAT1 => access to BAT1 will fail. Is it possible to lock 
whole adapter, but allow only traffic to i2c mux client?

regards
Antti

>
> 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, and 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 is 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 a dt property to i2c-mux-gpio and
> i2c-mux-pinctrl that can be used to state that the mux is updated by means
> of the muxed master bus, and that the select-transfer-deselect operations
> should be locked individually. When this holds, the i2c bus *is* 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 i2s master bus). A lock
> is added to the mux so that transfers through the mux 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 of 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.
>
> 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 the last three patches of the series, with the real
> meat in 8/8.
>
> PS. needs a bunch of testing, I do not have access to all the involved hw
>
> Changes since v1:
> - Allocate mux core and (optional) priv in a combined allocation.
> - Killed dev_err messages triggered by memory allocation failure.
> - Fix the device specific i2c muxes that I had overlooked.
> - Rebased on top of v4.4-rc8 (was based on v4.4-rc6 previously).
>
> Cheers,
> Peter
>
> Peter Rosin (8):
>    i2c-mux: add common core data for every mux instance
>    i2c-mux: move select and deselect ops to i2c_mux_core
>    i2c-mux: move the slave side adapter management to i2c_mux_core
>    i2c-mux: remove the mux dev pointer from the mux per channel data
>    i2c-mux: pinctrl: get rid of the driver private struct device pointer
>    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 i2c controlled
>      muxing
>
>   .../devicetree/bindings/i2c/i2c-mux-gpio.txt       |   2 +
>   .../devicetree/bindings/i2c/i2c-mux-pinctrl.txt    |   4 +
>   drivers/i2c/i2c-core.c                             |  59 ++---
>   drivers/i2c/i2c-mux.c                              | 272 +++++++++++++++++----
>   drivers/i2c/muxes/i2c-arb-gpio-challenge.c         |  46 ++--
>   drivers/i2c/muxes/i2c-mux-gpio.c                   |  58 ++---
>   drivers/i2c/muxes/i2c-mux-pca9541.c                |  58 +++--
>   drivers/i2c/muxes/i2c-mux-pca954x.c                |  66 ++---
>   drivers/i2c/muxes/i2c-mux-pinctrl.c                |  89 +++----
>   drivers/i2c/muxes/i2c-mux-reg.c                    |  63 ++---
>   drivers/iio/imu/inv_mpu6050/inv_mpu_core.c         |  33 +--
>   drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h          |   2 +-
>   drivers/media/dvb-frontends/m88ds3103.c            |  23 +-
>   drivers/media/dvb-frontends/m88ds3103_priv.h       |   2 +-
>   drivers/media/dvb-frontends/rtl2830.c              |  24 +-
>   drivers/media/dvb-frontends/rtl2830_priv.h         |   2 +-
>   drivers/media/dvb-frontends/rtl2832.c              |  30 ++-
>   drivers/media/dvb-frontends/rtl2832_priv.h         |   2 +-
>   drivers/media/dvb-frontends/si2168.c               |  29 ++-
>   drivers/media/dvb-frontends/si2168_priv.h          |   2 +-
>   drivers/media/usb/cx231xx/cx231xx-core.c           |   6 +-
>   drivers/media/usb/cx231xx/cx231xx-i2c.c            |  48 ++--
>   drivers/media/usb/cx231xx/cx231xx.h                |   4 +-
>   drivers/of/unittest.c                              |  41 ++--
>   include/linux/i2c-mux-gpio.h                       |   2 +
>   include/linux/i2c-mux-pinctrl.h                    |   2 +
>   include/linux/i2c-mux.h                            |  39 ++-
>   include/linux/i2c.h                                |  28 ++-
>   28 files changed, 612 insertions(+), 424 deletions(-)
>

-- 
http://palosaari.fi/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1303344

FromPeter Rosin <peda@lysator.liu.se>
Date2016-01-07 09:20 +0100
Message-ID<qOdXA-5pq-1@gated-at.bofh.it>
In reply to#1302908
Hi Antti,

On 2016-01-06 18:17, Antti Palosaari wrote:
> On 01/05/2016 05:57 PM, Peter Rosin wrote:
>> 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)
> 
> Handling of I2C muxes that close channel automatically, after the first I2C stop (P) is seen?
> 
> For example channel is selected to BAT1 => there is EEPROM write => mux closes channel BAT1 => access to BAT1 will fail.

The proposed locking changes affect gpio- and pinctrl-controlled muxes
only, and I can't see one of those actually know anything about the
i2c-signals that they mux. Such muxes certainly has to obey the pins
that control them, no?

That fact that other muxes might piggy-back and also use the new
i2c-controlled flag is a different question, and it may indeed not be
safe to declare a mux "i2c-controlled" just to avoid the locking, if
the locking is in fact required. Maybe the name "i2c-controlled" is
poor?

> Is it possible to lock whole adapter, but allow only traffic to i2c mux client?

This is basically what is there today, and which does not work for the
above i2c topology. So maybe you need to expand on what you meant?



There is no neat way for the i2c mux code to dip its fingers into all
relevant i2c-accesses to categorize them as "mux updates" or "mux slave
side accesses", when they come from generic subsystems such as gpio
och pinctrl.

The only thing I have been able to think of in that area is to add i2c
devices that are used to change the mux to a virtual slave adapter of
the mux, so that the mux can identify those accesses and bypass the
locking. But that isn't generic enough to cover the case where one
device is used to control more than one mux (since it needs to sit on
more than one adapter in that case), so I scrapped that idea pretty
early. But I also find that I have to scrap it again and again, since
everything else I cook up usually ends up being some variant of the
virtual slave adapter when I think some more about it. Maybe it *is*
possible to have the same device in different places in the adapter
tree (if all those places have a common ancestor)? But I don't think
so...

That does not address your concerns about extra accesses creeping
in between the mux select and the mux slave side access for auto-
closing muxes (or arbitrators), but I never pretended that there would
be protection from that. If there are such requirements you basically
have to lock the root adapter to prevent i2c "noise".

I can't see recursive locks helping either, because that would need
lock ownership to be propagated into gpio/pinctrl, and that is again
too ugly.

Cheers,
Peter

> regards
> Antti
> 
>>
>> 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, and 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 is 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 a dt property to i2c-mux-gpio and
>> i2c-mux-pinctrl that can be used to state that the mux is updated by means
>> of the muxed master bus, and that the select-transfer-deselect operations
>> should be locked individually. When this holds, the i2c bus *is* 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 i2s master bus). A lock
>> is added to the mux so that transfers through the mux 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 of 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.
>>
>> 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 the last three patches of the series, with the real
>> meat in 8/8.
>>
>> PS. needs a bunch of testing, I do not have access to all the involved hw
>>
>> Changes since v1:
>> - Allocate mux core and (optional) priv in a combined allocation.
>> - Killed dev_err messages triggered by memory allocation failure.
>> - Fix the device specific i2c muxes that I had overlooked.
>> - Rebased on top of v4.4-rc8 (was based on v4.4-rc6 previously).
>>
>> Cheers,
>> Peter
>>
>> Peter Rosin (8):
>>    i2c-mux: add common core data for every mux instance
>>    i2c-mux: move select and deselect ops to i2c_mux_core
>>    i2c-mux: move the slave side adapter management to i2c_mux_core
>>    i2c-mux: remove the mux dev pointer from the mux per channel data
>>    i2c-mux: pinctrl: get rid of the driver private struct device pointer
>>    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 i2c controlled
>>      muxing
>>
>>   .../devicetree/bindings/i2c/i2c-mux-gpio.txt       |   2 +
>>   .../devicetree/bindings/i2c/i2c-mux-pinctrl.txt    |   4 +
>>   drivers/i2c/i2c-core.c                             |  59 ++---
>>   drivers/i2c/i2c-mux.c                              | 272 +++++++++++++++++----
>>   drivers/i2c/muxes/i2c-arb-gpio-challenge.c         |  46 ++--
>>   drivers/i2c/muxes/i2c-mux-gpio.c                   |  58 ++---
>>   drivers/i2c/muxes/i2c-mux-pca9541.c                |  58 +++--
>>   drivers/i2c/muxes/i2c-mux-pca954x.c                |  66 ++---
>>   drivers/i2c/muxes/i2c-mux-pinctrl.c                |  89 +++----
>>   drivers/i2c/muxes/i2c-mux-reg.c                    |  63 ++---
>>   drivers/iio/imu/inv_mpu6050/inv_mpu_core.c         |  33 +--
>>   drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h          |   2 +-
>>   drivers/media/dvb-frontends/m88ds3103.c            |  23 +-
>>   drivers/media/dvb-frontends/m88ds3103_priv.h       |   2 +-
>>   drivers/media/dvb-frontends/rtl2830.c              |  24 +-
>>   drivers/media/dvb-frontends/rtl2830_priv.h         |   2 +-
>>   drivers/media/dvb-frontends/rtl2832.c              |  30 ++-
>>   drivers/media/dvb-frontends/rtl2832_priv.h         |   2 +-
>>   drivers/media/dvb-frontends/si2168.c               |  29 ++-
>>   drivers/media/dvb-frontends/si2168_priv.h          |   2 +-
>>   drivers/media/usb/cx231xx/cx231xx-core.c           |   6 +-
>>   drivers/media/usb/cx231xx/cx231xx-i2c.c            |  48 ++--
>>   drivers/media/usb/cx231xx/cx231xx.h                |   4 +-
>>   drivers/of/unittest.c                              |  41 ++--
>>   include/linux/i2c-mux-gpio.h                       |   2 +
>>   include/linux/i2c-mux-pinctrl.h                    |   2 +
>>   include/linux/i2c-mux.h                            |  39 ++-
>>   include/linux/i2c.h                                |  28 ++-
>>   28 files changed, 612 insertions(+), 424 deletions(-)
>>
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web