linux-i2c.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Peter Rosin <peda-SamgB31n2u5IcsJQ0EH25Q@public.gmane.org>
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>,
	Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>,
	Peter Korsgaard
	<peter.korsgaard-ob4gmnvZ1/cAvxtiuMwx3w@public.gmane.org>,
	Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>,
	Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	Hartmut Knaack <knaack.h-Mmb7MZpHnFY@public.gmane.org>,
	Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>,
	Peter Meerwald <pmeerw-jW+XmwGofnusTnJN9+BGXg@public.gmane.org>,
	Antti Palosaari <crope-X3B1VOXEql0@public.gmane.org>,
	Mauro Carvalho Chehab
	<mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>,
	Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	Frank Rowand
	<frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Grant Likely
	<grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>,
	Viorel Suman
	<viorel.suman-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>,
	Krzysztof Kozlowski
	<k.kozlowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>,
	Terry Heo <terryheo-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>,
	Hans Verkuil
	<hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>,
	Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>,
	Tommi Rantala
	<tt.rantala-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-media-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: [PATCH v4 00/18] i2c mux cleanup and locking update
Date: Fri, 04 Mar 2016 15:49:30 +0100	[thread overview]
Message-ID: <56D9A07A.5040106@lysator.liu.se> (raw)
In-Reply-To: <56D96B15.8090806-SamgB31n2u5IcsJQ0EH25Q@public.gmane.org>

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

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

  parent reply	other threads:[~2016-03-04 14:49 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-03 22:27 [PATCH v4 00/18] i2c mux cleanup and locking update Peter Rosin
2016-03-03 22:27 ` [PATCH v4 01/18] i2c-mux: add common data for every i2c-mux instance Peter Rosin
2016-03-03 22:27 ` [PATCH v4 02/18] i2c: i2c-mux-gpio: convert to use an explicit i2c mux core Peter Rosin
2016-03-03 22:27 ` [PATCH v4 03/18] i2c: i2c-mux-pinctrl: " Peter Rosin
2016-03-03 22:27 ` [PATCH v4 04/18] i2c: i2c-arb-gpio-challenge: " Peter Rosin
2016-03-03 22:27 ` [PATCH v4 05/18] i2c: i2c-mux-pca9541: " Peter Rosin
2016-03-03 22:27 ` [PATCH v4 06/18] i2c: i2c-mux-pca954x: " Peter Rosin
2016-03-03 22:27 ` [PATCH v4 07/18] i2c: i2c-mux-reg: " Peter Rosin
2016-03-03 22:27 ` [PATCH v4 08/18] iio: imu: inv_mpu6050: " Peter Rosin
2016-03-03 22:27 ` [PATCH v4 09/18] [media] m88ds3103: " Peter Rosin
2016-03-03 22:27 ` [PATCH v4 10/18] [media] rtl2830: " Peter Rosin
2016-03-03 22:27 ` [PATCH v4 11/18] [media] rtl2832: " Peter Rosin
2016-03-03 22:27 ` [PATCH v4 12/18] [media] si2168: " Peter Rosin
2016-03-03 22:27 ` [PATCH v4 13/18] [media] cx231xx: " Peter Rosin
2016-03-03 22:27 ` [PATCH v4 14/18] of/unittest: " Peter Rosin
2016-03-03 22:27 ` [PATCH v4 15/18] i2c-mux: drop old unused i2c-mux api Peter Rosin
2016-03-03 22:27 ` [PATCH v4 16/18] i2c: allow adapter drivers to override the adapter locking Peter Rosin
     [not found]   ` <1457044050-15230-17-git-send-email-peda-SamgB31n2u5IcsJQ0EH25Q@public.gmane.org>
2016-03-04  5:58     ` kbuild test robot
2016-03-04  9:33     ` Peter Rosin
2016-03-03 22:27 ` [PATCH v4 17/18] i2c: muxes always lock the parent adapter Peter Rosin
2016-03-03 22:27 ` [PATCH v4 18/18] i2c-mux: relax locking of the top i2c adapter during i2c controlled muxing Peter Rosin
2016-03-04  5:21   ` kbuild test robot
     [not found]   ` <1457044050-15230-19-git-send-email-peda-SamgB31n2u5IcsJQ0EH25Q@public.gmane.org>
2016-03-04  7:10     ` Peter Rosin
2016-03-04 17:44   ` kbuild test robot
2016-03-04 11:01 ` [PATCH v4 00/18] i2c mux cleanup and locking update Peter Rosin
     [not found]   ` <56D96B15.8090806-SamgB31n2u5IcsJQ0EH25Q@public.gmane.org>
2016-03-04 14:49     ` Peter Rosin [this message]
2016-03-15 14:09 ` Peter Rosin
     [not found]   ` <56E817AE.2090005-SamgB31n2u5IcsJQ0EH25Q@public.gmane.org>
2016-03-15 17:08     ` Antti Palosaari

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=56D9A07A.5040106@lysator.liu.se \
    --to=peda-samgb31n2u5icsjq0eh25q@public.gmane.org \
    --cc=arnd-r2nGTMty4D4@public.gmane.org \
    --cc=crope-X3B1VOXEql0@public.gmane.org \
    --cc=devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \
    --cc=hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org \
    --cc=jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=k.kozlowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org \
    --cc=knaack.h-Mmb7MZpHnFY@public.gmane.org \
    --cc=lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org \
    --cc=linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org \
    --cc=linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-media-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org \
    --cc=peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org \
    --cc=peter.korsgaard-ob4gmnvZ1/cAvxtiuMwx3w@public.gmane.org \
    --cc=pmeerw-jW+XmwGofnusTnJN9+BGXg@public.gmane.org \
    --cc=robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=terryheo-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org \
    --cc=tt.rantala-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=viorel.suman-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org \
    --cc=wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).