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


Groups > linux.kernel > #1349631 > unrolled thread

[PATCH v4 00/18] i2c mux cleanup and locking update

Started byPeter Rosin <peda@lysator.liu.se>
First post2016-03-03 23:30 +0100
Last post2016-03-15 18:10 +0100
Articles 4 on this page of 24 — 3 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH v4 00/18] i2c mux cleanup and locking update Peter Rosin <peda@lysator.liu.se> - 2016-03-03 23:30 +0100
    [PATCH v4 10/18] [media] rtl2830: convert to use an explicit i2c mux core Peter Rosin <peda@lysator.liu.se> - 2016-03-03 23:30 +0100
    [PATCH v4 12/18] [media] si2168: convert to use an explicit i2c mux core Peter Rosin <peda@lysator.liu.se> - 2016-03-03 23:30 +0100
    [PATCH v4 05/18] i2c: i2c-mux-pca9541: convert to use an explicit i2c mux core Peter Rosin <peda@lysator.liu.se> - 2016-03-03 23:30 +0100
    [PATCH v4 06/18] i2c: i2c-mux-pca954x: convert to use an explicit i2c mux core Peter Rosin <peda@lysator.liu.se> - 2016-03-03 23:30 +0100
    [PATCH v4 11/18] [media] rtl2832: convert to use an explicit i2c mux core Peter Rosin <peda@lysator.liu.se> - 2016-03-03 23:30 +0100
    [PATCH v4 04/18] i2c: i2c-arb-gpio-challenge: convert to use an explicit i2c mux core Peter Rosin <peda@lysator.liu.se> - 2016-03-03 23:30 +0100
    [PATCH v4 02/18] i2c: i2c-mux-gpio: convert to use an explicit i2c mux core Peter Rosin <peda@lysator.liu.se> - 2016-03-03 23:30 +0100
    [PATCH v4 03/18] i2c: i2c-mux-pinctrl: convert to use an explicit i2c mux core Peter Rosin <peda@lysator.liu.se> - 2016-03-03 23:30 +0100
    [PATCH v4 18/18] i2c-mux: relax locking of the top i2c adapter during i2c controlled muxing Peter Rosin <peda@lysator.liu.se> - 2016-03-03 23:40 +0100
      Re: [PATCH v4 18/18] i2c-mux: relax locking of the top i2c adapter  during i2c controlled muxing kbuild test robot <lkp@intel.com> - 2016-03-04 06:30 +0100
      Re: [PATCH v4 18/18] i2c-mux: relax locking of the top i2c adapter  during i2c controlled muxing Peter Rosin <peda@lysator.liu.se> - 2016-03-04 08:20 +0100
    [PATCH v4 15/18] i2c-mux: drop old unused i2c-mux api Peter Rosin <peda@lysator.liu.se> - 2016-03-03 23:40 +0100
    [PATCH v4 13/18] [media] cx231xx: convert to use an explicit i2c mux core Peter Rosin <peda@lysator.liu.se> - 2016-03-03 23:40 +0100
    [PATCH v4 09/18] [media] m88ds3103: convert to use an explicit i2c mux core Peter Rosin <peda@lysator.liu.se> - 2016-03-03 23:40 +0100
    [PATCH v4 16/18] i2c: allow adapter drivers to override the adapter locking Peter Rosin <peda@lysator.liu.se> - 2016-03-03 23:40 +0100
      Re: [PATCH v4 16/18] i2c: allow adapter drivers to override the  adapter locking kbuild test robot <lkp@intel.com> - 2016-03-04 07:00 +0100
      Re: [PATCH v4 16/18] i2c: allow adapter drivers to override the adapter  locking Peter Rosin <peda@lysator.liu.se> - 2016-03-04 10:40 +0100
    [PATCH v4 17/18] i2c: muxes always lock the parent adapter Peter Rosin <peda@lysator.liu.se> - 2016-03-03 23:40 +0100
    [PATCH v4 14/18] of/unittest: convert to use an explicit i2c mux core Peter Rosin <peda@lysator.liu.se> - 2016-03-03 23:40 +0100
    Re: [PATCH v4 00/18] i2c mux cleanup and locking update Peter Rosin <peda@lysator.liu.se> - 2016-03-04 12:10 +0100
      Re: [PATCH v4 00/18] i2c mux cleanup and locking update Peter Rosin <peda@lysator.liu.se> - 2016-03-04 15:50 +0100
    Re: [PATCH v4 00/18] i2c mux cleanup and locking update Peter Rosin <peda@lysator.liu.se> - 2016-03-15 15:20 +0100
      Re: [PATCH v4 00/18] i2c mux cleanup and locking update Antti Palosaari <crope@iki.fi> - 2016-03-15 18:10 +0100

Page 2 of 2 — ← Prev page 1 [2]


#1350160

FromPeter Rosin <peda@lysator.liu.se>
Date2016-03-04 12:10 +0100
Message-ID<r8VMm-5IQ-15@gated-at.bofh.it>
In reply to#1349631
I wrote:
> Concerns:
> - The locking is perhaps too complex?

Ok, to highlight the benefits of this series, I expect that patches such as
[1] and the one inlined below can follow up to clean up ad-hoc i2c locking
in drivers. Putting this locking in one place instead of having it spread
out in random drivers is a good thing.

PS. the inlined patch has not been tested as I have no hw, it's a proof of
concept. Maybe the simplifications can be extended into rtl2832_sdr.c as well?
Anyway, please do test this patch on top of the v4 series if you have the hw.

Cheers,
Peter

[1] https://lkml.org/lkml/2016/3/3/932

diff --git a/drivers/media/dvb-frontends/rtl2832.c b/drivers/media/dvb-frontends/rtl2832.c
index 775444898599..fd1f05e9e79a 100644
--- a/drivers/media/dvb-frontends/rtl2832.c
+++ b/drivers/media/dvb-frontends/rtl2832.c
@@ -158,36 +158,24 @@ static int rtl2832_bulk_write(struct i2c_client *client, unsigned int reg,
 			      const void *val, size_t val_count)
 {
 	struct rtl2832_dev *dev = i2c_get_clientdata(client);
-	int ret;
 
-	i2c_lock_adapter(client->adapter);
-	ret = regmap_bulk_write(dev->regmap, reg, val, val_count);
-	i2c_unlock_adapter(client->adapter);
-	return ret;
+	return regmap_bulk_write(dev->regmap, reg, val, val_count);
 }
 
 static int rtl2832_update_bits(struct i2c_client *client, unsigned int reg,
 			       unsigned int mask, unsigned int val)
 {
 	struct rtl2832_dev *dev = i2c_get_clientdata(client);
-	int ret;
 
-	i2c_lock_adapter(client->adapter);
-	ret = regmap_update_bits(dev->regmap, reg, mask, val);
-	i2c_unlock_adapter(client->adapter);
-	return ret;
+	return regmap_update_bits(dev->regmap, reg, mask, val);
 }
 
 static int rtl2832_bulk_read(struct i2c_client *client, unsigned int reg,
 			     void *val, size_t val_count)
 {
 	struct rtl2832_dev *dev = i2c_get_clientdata(client);
-	int ret;
 
-	i2c_lock_adapter(client->adapter);
-	ret = regmap_bulk_read(dev->regmap, reg, val, val_count);
-	i2c_unlock_adapter(client->adapter);
-	return ret;
+	return regmap_bulk_read(dev->regmap, reg, val, val_count);
 }
 
 static int rtl2832_rd_demod_reg(struct rtl2832_dev *dev, int reg, u32 *val)
@@ -204,7 +192,7 @@ static int rtl2832_rd_demod_reg(struct rtl2832_dev *dev, int reg, u32 *val)
 	len = (msb >> 3) + 1;
 	mask = REG_MASK(msb - lsb);
 
-	ret = rtl2832_bulk_read(client, reg_start_addr, reading, len);
+	ret = regmap_bulk_read(dev->regmap, reg_start_addr, reading, len);
 	if (ret)
 		goto err;
 
@@ -234,7 +222,7 @@ static int rtl2832_wr_demod_reg(struct rtl2832_dev *dev, int reg, u32 val)
 	len = (msb >> 3) + 1;
 	mask = REG_MASK(msb - lsb);
 
-	ret = rtl2832_bulk_read(client, reg_start_addr, reading, len);
+	ret = regmap_bulk_read(dev->regmap, reg_start_addr, reading, len);
 	if (ret)
 		goto err;
 
@@ -248,7 +236,7 @@ static int rtl2832_wr_demod_reg(struct rtl2832_dev *dev, int reg, u32 val)
 	for (i = 0; i < len; i++)
 		writing[i] = (writing_tmp >> ((len - 1 - i) * 8)) & 0xff;
 
-	ret = rtl2832_bulk_write(client, reg_start_addr, writing, len);
+	ret = regmap_bulk_write(dev->regmap, reg_start_addr, writing, len);
 	if (ret)
 		goto err;
 
@@ -492,7 +480,7 @@ static int rtl2832_set_frontend(struct dvb_frontend *fe)
 		fe->ops.tuner_ops.set_params(fe);
 
 	/* PIP mode related */
-	ret = rtl2832_bulk_write(client, 0x192, "\x00\x0f\xff", 3);
+	ret = regmap_bulk_write(dev->regmap, 0x192, "\x00\x0f\xff", 3);
 	if (ret)
 		goto err;
 
@@ -530,7 +518,7 @@ static int rtl2832_set_frontend(struct dvb_frontend *fe)
 	}
 
 	for (j = 0; j < sizeof(bw_params[0]); j++) {
-		ret = rtl2832_bulk_write(client, 0x11c + j, &bw_params[i][j], 1);
+		ret = regmap_bulk_write(dev->regmap, 0x11c + j, &bw_params[i][j], 1);
 		if (ret)
 			goto err;
 	}
@@ -586,11 +574,11 @@ static int rtl2832_get_frontend(struct dvb_frontend *fe)
 	if (dev->sleeping)
 		return 0;
 
-	ret = rtl2832_bulk_read(client, 0x33c, buf, 2);
+	ret = regmap_bulk_read(dev->regmap, 0x33c, buf, 2);
 	if (ret)
 		goto err;
 
-	ret = rtl2832_bulk_read(client, 0x351, &buf[2], 1);
+	ret = regmap_bulk_read(dev->regmap, 0x351, &buf[2], 1);
 	if (ret)
 		goto err;
 
@@ -757,7 +745,7 @@ static void rtl2832_stat_work(struct work_struct *work)
 	/* signal strength */
 	if (dev->fe_status & FE_HAS_SIGNAL) {
 		/* read digital AGC */
-		ret = rtl2832_bulk_read(client, 0x305, &u8tmp, 1);
+		ret = regmap_bulk_read(dev->regmap, 0x305, &u8tmp, 1);
 		if (ret)
 			goto err;
 
@@ -783,7 +771,7 @@ static void rtl2832_stat_work(struct work_struct *work)
 			{87659938, 87659938, 87885178, 88241743},
 		};
 
-		ret = rtl2832_bulk_read(client, 0x33c, &u8tmp, 1);
+		ret = regmap_bulk_read(dev->regmap, 0x33c, &u8tmp, 1);
 		if (ret)
 			goto err;
 
@@ -795,7 +783,7 @@ static void rtl2832_stat_work(struct work_struct *work)
 		if (hierarchy > HIERARCHY_NUM - 1)
 			goto err_schedule_delayed_work;
 
-		ret = rtl2832_bulk_read(client, 0x40c, buf, 2);
+		ret = regmap_bulk_read(dev->regmap, 0x40c, buf, 2);
 		if (ret)
 			goto err;
 
@@ -816,7 +804,7 @@ static void rtl2832_stat_work(struct work_struct *work)
 
 	/* BER */
 	if (dev->fe_status & FE_HAS_LOCK) {
-		ret = rtl2832_bulk_read(client, 0x34e, buf, 2);
+		ret = regmap_bulk_read(dev->regmap, 0x34e, buf, 2);
 		if (ret)
 			goto err;
 
@@ -844,8 +832,6 @@ err:
 
 /*
  * I2C gate/mux/repeater logic
- * We must use unlocked __i2c_transfer() here (through regmap) because of I2C
- * adapter lock is already taken by tuner driver.
  * There is delay mechanism to avoid unneeded I2C gate open / close. Gate close
  * is delayed here a little bit in order to see if there is sequence of I2C
  * messages sent to same I2C bus.
@@ -857,7 +843,7 @@ static void rtl2832_i2c_gate_work(struct work_struct *work)
 	int ret;
 
 	/* close gate */
-	ret = rtl2832_update_bits(dev->client, 0x101, 0x08, 0x00);
+	ret = regmap_update_bits(dev->regmap, 0x101, 0x08, 0x00);
 	if (ret)
 		goto err;
 
@@ -875,10 +861,7 @@ static int rtl2832_select(struct i2c_mux_core *muxc, u32 chan_id)
 	/* terminate possible gate closing */
 	cancel_delayed_work(&dev->i2c_gate_work);
 
-	/*
-	 * I2C adapter lock is already taken and due to that we will use
-	 * regmap_update_bits() which does not lock again I2C adapter.
-	 */
+	/* open gate */
 	ret = regmap_update_bits(dev->regmap, 0x101, 0x08, 0x08);
 	if (ret)
 		goto err;
@@ -950,120 +933,6 @@ static bool rtl2832_volatile_reg(struct device *dev, unsigned int reg)
 	return false;
 }
 
-/*
- * We implement own I2C access routines for regmap in order to get manual access
- * to I2C adapter lock, which is needed for I2C mux adapter.
- */
-static int rtl2832_regmap_read(void *context, const void *reg_buf,
-			       size_t reg_size, void *val_buf, size_t val_size)
-{
-	struct i2c_client *client = context;
-	int ret;
-	struct i2c_msg msg[2] = {
-		{
-			.addr = client->addr,
-			.flags = 0,
-			.len = reg_size,
-			.buf = (u8 *)reg_buf,
-		}, {
-			.addr = client->addr,
-			.flags = I2C_M_RD,
-			.len = val_size,
-			.buf = val_buf,
-		}
-	};
-
-	ret = __i2c_transfer(client->adapter, msg, 2);
-	if (ret != 2) {
-		dev_warn(&client->dev, "i2c reg read failed %d reg %02x\n",
-			 ret, *(u8 *)reg_buf);
-		if (ret >= 0)
-			ret = -EREMOTEIO;
-		return ret;
-	}
-	return 0;
-}
-
-static int rtl2832_regmap_write(void *context, const void *data, size_t count)
-{
-	struct i2c_client *client = context;
-	int ret;
-	struct i2c_msg msg[1] = {
-		{
-			.addr = client->addr,
-			.flags = 0,
-			.len = count,
-			.buf = (u8 *)data,
-		}
-	};
-
-	ret = __i2c_transfer(client->adapter, msg, 1);
-	if (ret != 1) {
-		dev_warn(&client->dev, "i2c reg write failed %d reg %02x\n",
-			 ret, *(u8 *)data);
-		if (ret >= 0)
-			ret = -EREMOTEIO;
-		return ret;
-	}
-	return 0;
-}
-
-static int rtl2832_regmap_gather_write(void *context, const void *reg,
-				       size_t reg_len, const void *val,
-				       size_t val_len)
-{
-	struct i2c_client *client = context;
-	int ret;
-	u8 buf[256];
-	struct i2c_msg msg[1] = {
-		{
-			.addr = client->addr,
-			.flags = 0,
-			.len = 1 + val_len,
-			.buf = buf,
-		}
-	};
-
-	buf[0] = *(u8 const *)reg;
-	memcpy(&buf[1], val, val_len);
-
-	ret = __i2c_transfer(client->adapter, msg, 1);
-	if (ret != 1) {
-		dev_warn(&client->dev, "i2c reg write failed %d reg %02x\n",
-			 ret, *(u8 const *)reg);
-		if (ret >= 0)
-			ret = -EREMOTEIO;
-		return ret;
-	}
-	return 0;
-}
-
-/*
- * FIXME: Hack. Implement own regmap locking in order to silence lockdep
- * recursive lock warning. That happens when regmap I2C client calls I2C mux
- * adapter, which leads demod I2C repeater enable via demod regmap. Operation
- * takes two regmap locks recursively - but those are different regmap instances
- * in a two different I2C drivers, so it is not deadlock. Proper fix is to make
- * regmap aware of lockdep.
- */
-static void rtl2832_regmap_lock(void *__dev)
-{
-	struct rtl2832_dev *dev = __dev;
-	struct i2c_client *client = dev->client;
-
-	dev_dbg(&client->dev, "\n");
-	mutex_lock(&dev->regmap_mutex);
-}
-
-static void rtl2832_regmap_unlock(void *__dev)
-{
-	struct rtl2832_dev *dev = __dev;
-	struct i2c_client *client = dev->client;
-
-	dev_dbg(&client->dev, "\n");
-	mutex_unlock(&dev->regmap_mutex);
-}
-
 static struct dvb_frontend *rtl2832_get_dvb_frontend(struct i2c_client *client)
 {
 	struct rtl2832_dev *dev = i2c_get_clientdata(client);
@@ -1087,7 +956,7 @@ static int rtl2832_enable_slave_ts(struct i2c_client *client)
 
 	dev_dbg(&client->dev, "\n");
 
-	ret = rtl2832_bulk_write(client, 0x10c, "\x5f\xff", 2);
+	ret = regmap_bulk_write(dev->regmap, 0x10c, "\x5f\xff", 2);
 	if (ret)
 		goto err;
 
@@ -1095,11 +964,11 @@ static int rtl2832_enable_slave_ts(struct i2c_client *client)
 	if (ret)
 		goto err;
 
-	ret = rtl2832_bulk_write(client, 0x0bc, "\x18", 1);
+	ret = regmap_bulk_write(dev->regmap, 0x0bc, "\x18", 1);
 	if (ret)
 		goto err;
 
-	ret = rtl2832_bulk_write(client, 0x192, "\x7f\xf7\xff", 3);
+	ret = regmap_bulk_write(dev->regmap, 0x192, "\x7f\xf7\xff", 3);
 	if (ret)
 		goto err;
 
@@ -1133,7 +1002,7 @@ static int rtl2832_pid_filter_ctrl(struct dvb_frontend *fe, int onoff)
 	else
 		u8tmp = 0x00;
 
-	ret = rtl2832_update_bits(client, 0x061, 0xc0, u8tmp);
+	ret = regmap_update_bits(dev->regmap, 0x061, 0xc0, u8tmp);
 	if (ret)
 		goto err;
 
@@ -1168,14 +1037,14 @@ static int rtl2832_pid_filter(struct dvb_frontend *fe, u8 index, u16 pid,
 	buf[1] = (dev->filters >>  8) & 0xff;
 	buf[2] = (dev->filters >> 16) & 0xff;
 	buf[3] = (dev->filters >> 24) & 0xff;
-	ret = rtl2832_bulk_write(client, 0x062, buf, 4);
+	ret = regmap_bulk_write(dev->regmap, 0x062, buf, 4);
 	if (ret)
 		goto err;
 
 	/* add PID */
 	buf[0] = (pid >> 8) & 0xff;
 	buf[1] = (pid >> 0) & 0xff;
-	ret = rtl2832_bulk_write(client, 0x066 + 2 * index, buf, 2);
+	ret = regmap_bulk_write(dev->regmap, 0x066 + 2 * index, buf, 2);
 	if (ret)
 		goto err;
 
@@ -1193,12 +1062,6 @@ static int rtl2832_probe(struct i2c_client *client,
 	struct rtl2832_dev *dev;
 	int ret;
 	u8 tmp;
-	static const struct regmap_bus regmap_bus = {
-		.read = rtl2832_regmap_read,
-		.write = rtl2832_regmap_write,
-		.gather_write = rtl2832_regmap_gather_write,
-		.val_format_endian_default = REGMAP_ENDIAN_NATIVE,
-	};
 	static const struct regmap_range_cfg regmap_range_cfg[] = {
 		{
 			.selector_reg     = 0x00,
@@ -1231,15 +1094,12 @@ static int rtl2832_probe(struct i2c_client *client,
 	mutex_init(&dev->regmap_mutex);
 	dev->regmap_config.reg_bits =  8,
 	dev->regmap_config.val_bits =  8,
-	dev->regmap_config.lock = rtl2832_regmap_lock,
-	dev->regmap_config.unlock = rtl2832_regmap_unlock,
-	dev->regmap_config.lock_arg = dev,
 	dev->regmap_config.volatile_reg = rtl2832_volatile_reg,
 	dev->regmap_config.max_register = 5 * 0x100,
 	dev->regmap_config.ranges = regmap_range_cfg,
 	dev->regmap_config.num_ranges = ARRAY_SIZE(regmap_range_cfg),
 	dev->regmap_config.cache_type = REGCACHE_NONE,
-	dev->regmap = regmap_init(&client->dev, &regmap_bus, client,
+	dev->regmap = regmap_init(&client->dev, NULL, client,
 				  &dev->regmap_config);
 	if (IS_ERR(dev->regmap)) {
 		ret = PTR_ERR(dev->regmap);
@@ -1247,12 +1107,13 @@ static int rtl2832_probe(struct i2c_client *client,
 	}
 
 	/* check if the demod is there */
-	ret = rtl2832_bulk_read(client, 0x000, &tmp, 1);
+	ret = regmap_bulk_read(dev->regmap, 0x000, &tmp, 1);
 	if (ret)
 		goto err_regmap_exit;
 
 	/* create muxed i2c adapter for demod tuner bus */
-	dev->muxc = i2c_mux_one_adapter(i2c, &i2c->dev, 0, 0, 0, 0, 0,
+	dev->muxc = i2c_mux_one_adapter(i2c, &i2c->dev, 0,
+					I2C_CONTROLLED_MUX, 0, 0, 0,
 					rtl2832_select, rtl2832_deselect);
 	if (IS_ERR(dev->muxc)) {
 		ret = PTR_ERR(dev->muxc);
-- 
2.1.4

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


#1350262

FromPeter Rosin <peda@lysator.liu.se>
Date2016-03-04 15:50 +0100
Message-ID<r8Zdf-89n-7@gated-at.bofh.it>
In reply to#1350160
I wrote:
> I wrote:
>> Concerns:
>> - The locking is perhaps too complex?
> 
> Ok, to highlight the benefits of this series, I expect that patches such as
> [1] and the one inlined below can follow up to clean up ad-hoc i2c locking
> in drivers. Putting this locking in one place instead of having it spread
> out in random drivers is a good thing.
> 
> PS. the inlined patch has not been tested as I have no hw, it's a proof of
> concept. Maybe the simplifications can be extended into rtl2832_sdr.c as well?
> Anyway, please do test this patch on top of the v4 series if you have the hw.

The untested patch in the previous message indeed has some obvious problems, so
I fixed that and folded some more removed code now that rtl2832_sdr does not
need to do anything special to access device registers.

The result builds for me, that's all the testing I have done.

diffstat for the two patches combined:
 drivers/media/dvb-frontends/rtl2832.c      | 216 ++++-------------------------
 drivers/media/dvb-frontends/rtl2832.h      |   4 +-
 drivers/media/dvb-frontends/rtl2832_priv.h |   1 -
 drivers/media/dvb-frontends/rtl2832_sdr.c  |  13 +-
 drivers/media/dvb-frontends/rtl2832_sdr.h  |   5 +-
 drivers/media/usb/dvb-usb-v2/rtl28xxu.c    |   5 +-
 6 files changed, 34 insertions(+), 210 deletions(-)

Cheers,
Peter

diff --git a/drivers/media/dvb-frontends/rtl2832.c b/drivers/media/dvb-frontends/rtl2832.c
index fd1f05e9e79a..6ade321503f0 100644
--- a/drivers/media/dvb-frontends/rtl2832.c
+++ b/drivers/media/dvb-frontends/rtl2832.c
@@ -153,31 +153,6 @@ static const struct rtl2832_reg_entry registers[] = {
 	[DVBT_REG_4MSEL]	= {0x013,  0, 0},
 };
 
-/* Our regmap is bypassing I2C adapter lock, thus we do it! */
-static int rtl2832_bulk_write(struct i2c_client *client, unsigned int reg,
-			      const void *val, size_t val_count)
-{
-	struct rtl2832_dev *dev = i2c_get_clientdata(client);
-
-	return regmap_bulk_write(dev->regmap, reg, val, val_count);
-}
-
-static int rtl2832_update_bits(struct i2c_client *client, unsigned int reg,
-			       unsigned int mask, unsigned int val)
-{
-	struct rtl2832_dev *dev = i2c_get_clientdata(client);
-
-	return regmap_update_bits(dev->regmap, reg, mask, val);
-}
-
-static int rtl2832_bulk_read(struct i2c_client *client, unsigned int reg,
-			     void *val, size_t val_count)
-{
-	struct rtl2832_dev *dev = i2c_get_clientdata(client);
-
-	return regmap_bulk_read(dev->regmap, reg, val, val_count);
-}
-
 static int rtl2832_rd_demod_reg(struct rtl2832_dev *dev, int reg, u32 *val)
 {
 	struct i2c_client *client = dev->client;
@@ -1091,7 +1066,6 @@ static int rtl2832_probe(struct i2c_client *client,
 	INIT_DELAYED_WORK(&dev->i2c_gate_work, rtl2832_i2c_gate_work);
 	INIT_DELAYED_WORK(&dev->stat_work, rtl2832_stat_work);
 	/* create regmap */
-	mutex_init(&dev->regmap_mutex);
 	dev->regmap_config.reg_bits =  8,
 	dev->regmap_config.val_bits =  8,
 	dev->regmap_config.volatile_reg = rtl2832_volatile_reg,
@@ -1099,8 +1073,7 @@ static int rtl2832_probe(struct i2c_client *client,
 	dev->regmap_config.ranges = regmap_range_cfg,
 	dev->regmap_config.num_ranges = ARRAY_SIZE(regmap_range_cfg),
 	dev->regmap_config.cache_type = REGCACHE_NONE,
-	dev->regmap = regmap_init(&client->dev, NULL, client,
-				  &dev->regmap_config);
+	dev->regmap = regmap_init_i2c(client, &dev->regmap_config);
 	if (IS_ERR(dev->regmap)) {
 		ret = PTR_ERR(dev->regmap);
 		goto err_kfree;
@@ -1131,9 +1104,7 @@ static int rtl2832_probe(struct i2c_client *client,
 	pdata->enable_slave_ts = rtl2832_enable_slave_ts;
 	pdata->pid_filter = rtl2832_pid_filter;
 	pdata->pid_filter_ctrl = rtl2832_pid_filter_ctrl;
-	pdata->bulk_read = rtl2832_bulk_read;
-	pdata->bulk_write = rtl2832_bulk_write;
-	pdata->update_bits = rtl2832_update_bits;
+	pdata->regmap = dev->regmap;
 
 	dev_info(&client->dev, "Realtek RTL2832 successfully attached\n");
 	return 0;
diff --git a/drivers/media/dvb-frontends/rtl2832.h b/drivers/media/dvb-frontends/rtl2832.h
index c29a4c2bf71a..fab2f99672d6 100644
--- a/drivers/media/dvb-frontends/rtl2832.h
+++ b/drivers/media/dvb-frontends/rtl2832.h
@@ -57,9 +57,7 @@ struct rtl2832_platform_data {
 	int (*pid_filter)(struct dvb_frontend *, u8, u16, int);
 	int (*pid_filter_ctrl)(struct dvb_frontend *, int);
 /* private: Register access for SDR module use only */
-	int (*bulk_read)(struct i2c_client *, unsigned int, void *, size_t);
-	int (*bulk_write)(struct i2c_client *, unsigned int, const void *, size_t);
-	int (*update_bits)(struct i2c_client *, unsigned int, unsigned int, unsigned int);
+	struct regmap *regmap;
 };
 
 #endif /* RTL2832_H */
diff --git a/drivers/media/dvb-frontends/rtl2832_priv.h b/drivers/media/dvb-frontends/rtl2832_priv.h
index 6b3cd23a2c26..8657e0e578c5 100644
--- a/drivers/media/dvb-frontends/rtl2832_priv.h
+++ b/drivers/media/dvb-frontends/rtl2832_priv.h
@@ -33,7 +33,6 @@
 struct rtl2832_dev {
 	struct rtl2832_platform_data *pdata;
 	struct i2c_client *client;
-	struct mutex regmap_mutex;
 	struct regmap_config regmap_config;
 	struct regmap *regmap;
 	struct i2c_mux_core *muxc;
diff --git a/drivers/media/dvb-frontends/rtl2832_sdr.c b/drivers/media/dvb-frontends/rtl2832_sdr.c
index b860f02a4e55..6a6b1debe277 100644
--- a/drivers/media/dvb-frontends/rtl2832_sdr.c
+++ b/drivers/media/dvb-frontends/rtl2832_sdr.c
@@ -35,6 +35,7 @@
 #include <linux/platform_device.h>
 #include <linux/jiffies.h>
 #include <linux/math64.h>
+#include <linux/regmap.h>
 
 static bool rtl2832_sdr_emulated_fmt;
 module_param_named(emulated_formats, rtl2832_sdr_emulated_fmt, bool, 0644);
@@ -169,9 +170,9 @@ static int rtl2832_sdr_wr_regs(struct rtl2832_sdr_dev *dev, u16 reg,
 {
 	struct platform_device *pdev = dev->pdev;
 	struct rtl2832_sdr_platform_data *pdata = pdev->dev.platform_data;
-	struct i2c_client *client = pdata->i2c_client;
+	struct regmap *regmap = pdata->regmap;
 
-	return pdata->bulk_write(client, reg, val, len);
+	return regmap_bulk_write(regmap, reg, val, len);
 }
 
 #if 0
@@ -181,9 +182,9 @@ static int rtl2832_sdr_rd_regs(struct rtl2832_sdr_dev *dev, u16 reg, u8 *val,
 {
 	struct platform_device *pdev = dev->pdev;
 	struct rtl2832_sdr_platform_data *pdata = pdev->dev.platform_data;
-	struct i2c_client *client = pdata->i2c_client;
+	struct regmap *regmap = pdata->regmap;
 
-	return pdata->bulk_read(client, reg, val, len);
+	return regmap_bulk_read(regmap, reg, val, len);
 }
 #endif
 
@@ -199,9 +200,9 @@ static int rtl2832_sdr_wr_reg_mask(struct rtl2832_sdr_dev *dev, u16 reg,
 {
 	struct platform_device *pdev = dev->pdev;
 	struct rtl2832_sdr_platform_data *pdata = pdev->dev.platform_data;
-	struct i2c_client *client = pdata->i2c_client;
+	struct regmap *regmap = pdata->regmap;
 
-	return pdata->update_bits(client, reg, mask, val);
+	return regmap_update_bits(regmap, reg, mask, val);
 }
 
 /* Private functions */
diff --git a/drivers/media/dvb-frontends/rtl2832_sdr.h b/drivers/media/dvb-frontends/rtl2832_sdr.h
index 342ea84860df..d8fc7e7212e3 100644
--- a/drivers/media/dvb-frontends/rtl2832_sdr.h
+++ b/drivers/media/dvb-frontends/rtl2832_sdr.h
@@ -56,10 +56,7 @@ struct rtl2832_sdr_platform_data {
 #define RTL2832_SDR_TUNER_R828D     0x2b
 	u8 tuner;
 
-	struct i2c_client *i2c_client;
-	int (*bulk_read)(struct i2c_client *, unsigned int, void *, size_t);
-	int (*bulk_write)(struct i2c_client *, unsigned int, const void *, size_t);
-	int (*update_bits)(struct i2c_client *, unsigned int, unsigned int, unsigned int);
+	struct regmap *regmap;
 	struct dvb_frontend *dvb_frontend;
 	struct v4l2_subdev *v4l2_subdev;
 	struct dvb_usb_device *dvb_usb_device;
diff --git a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
index eb5787a3191e..799def499f67 100644
--- a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
+++ b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
@@ -1325,10 +1325,7 @@ static int rtl2832u_tuner_attach(struct dvb_usb_adapter *adap)
 	case TUNER_RTL2832_R828D:
 		pdata.clk = dev->rtl2832_platform_data.clk;
 		pdata.tuner = dev->tuner;
-		pdata.i2c_client = dev->i2c_client_demod;
-		pdata.bulk_read = dev->rtl2832_platform_data.bulk_read;
-		pdata.bulk_write = dev->rtl2832_platform_data.bulk_write;
-		pdata.update_bits = dev->rtl2832_platform_data.update_bits;
+		pdata.regmap = dev->rtl2832_platform_data.regmap;
 		pdata.dvb_frontend = adap->fe[0];
 		pdata.dvb_usb_device = d;
 		pdata.v4l2_subdev = subdev;
-- 
2.1.4

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


#1358062

FromPeter Rosin <peda@lysator.liu.se>
Date2016-03-15 15:20 +0100
Message-ID<rcXZg-3bg-21@gated-at.bofh.it>
In reply to#1349631
Hi!

On 2016-03-03 23:27, 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)
> 
> 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 is plainly, I need support for it.

etc etc

I've made a few changes, but instead of posting another copy of a rather long
series, I decided to push it to github and only post a note about it here. The
plan is to then rebase (if needed) on top of v4.6-rc1 and at that point post
all the patches (this is, after all, at least 4.7 material). At the same time
I do not want to hide the latest version from anyone wanting to have a look or
test.

The series will be posted again for review. This is just a heads up.

v5 compared to v4:
- 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.


The series looks like this now:

The following changes since commit f6cede5b49e822ebc41a099fe41ab4989f64e2cb:

  Linux 4.5-rc7 (2016-03-06 14:48:03 -0800)

are available in the git repository at:

  https://github.com/peda-r/i2c-mux.git mux-core-and-locking-5

for you to fetch changes up to c1ef4a249b0bd45ba97e14f15f6ee89e7fbc0222:

  [media] rtl2832: regmap is aware of lockdep, drop local locking hack (2016-03-15 10:02:15 +0100)

----------------------------------------------------------------
Antti Palosaari (1):
      [media] si2168: declare that the i2c gate is self-locked

Peter Rosin (24):
      MAINTAINERS: add myself as i2c mux maintainer
      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 self-locked muxing
      i2c-mux: document i2c muxes and elaborate on parent-/self-locked muxes
      iio: imu: inv_mpu6050: declare that the i2c gate is self-locked
      [media] rtl2832: declare that the i2c gate is self-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               | 312 ++++++++++++++++++++++++
 MAINTAINERS                                  |  11 +
 drivers/i2c/i2c-core.c                       |  65 +++--
 drivers/i2c/i2c-mux.c                        | 347 ++++++++++++++++++++++-----
 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   |  75 ++----
 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        | 236 +++---------------
 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         |  82 ++-----
 drivers/media/dvb-frontends/si2168_priv.h    |   2 +-
 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                      |  65 ++++-
 include/linux/i2c.h                          |  28 ++-
 31 files changed, 1206 insertions(+), 903 deletions(-)
 create mode 100644 Documentation/i2c/i2c-topology

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


#1358152

FromAntti Palosaari <crope@iki.fi>
Date2016-03-15 18:10 +0100
Message-ID<rd0DN-4Yj-15@gated-at.bofh.it>
In reply to#1358062
On 03/15/2016 04:09 PM, Peter Rosin wrote:

> The series will be posted again for review. This is just a heads up.
>
> v5 compared to v4:
> - 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.
>
>
> The series looks like this now:
>
> The following changes since commit f6cede5b49e822ebc41a099fe41ab4989f64e2cb:
>
>    Linux 4.5-rc7 (2016-03-06 14:48:03 -0800)
>
> are available in the git repository at:
>
>    https://github.com/peda-r/i2c-mux.git mux-core-and-locking-5

I reviewed and tested these patches:

c1ef4a2 [media] rtl2832: regmap is aware of lockdep, drop local locking hack
6636178 [media] rtl2832_sdr: get rid of empty regmap wrappers
001ad6b [media] rtl2832: declare that the i2c gate is self-locked
e2e82e4 [media] si2168: declare that the i2c gate is self-locked
b52f766 [media] si2168: convert to use an explicit i2c mux core
4ba9115 [media] rtl2832: convert to use an explicit i2c mux core
3f1778b [media] rtl2830: convert to use an explicit i2c mux core
5c8bfc8 [media] m88ds3103: convert to use an explicit i2c mux core


Reviewed-by: Antti Palosaari <crope@iki.fi>
Tested-by: Antti Palosaari <crope@iki.fi>

regards
Antti

-- 
http://palosaari.fi/

[toc] | [prev] | [standalone]


Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel


csiph-web