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

From: Peter Rosin
Date: Fri Mar 04 2016 - 09:49:47 EST


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