diff mbox

[v2,2/4] mfd: bcm590xx: add support for secondary I2C slave address

Message ID 1398295293-11033-3-git-send-email-mporter@linaro.org (mailing list archive)
State New, archived
Headers show

Commit Message

Matt Porter April 23, 2014, 11:21 p.m. UTC
BCM590xx utilizes a secondary I2C slave address to access additional
register space. Add support for the secondary address space by
instantiating a dummy I2C device with the appropriate secondary
I2C slave address. Also expose a secondary regmap register space so
that MFD drivers can access this secondary i2c slave address space.

Signed-off-by: Matt Porter <mporter@linaro.org>
---
 drivers/mfd/bcm590xx.c       | 60 +++++++++++++++++++++++++++++++++-----------
 include/linux/mfd/bcm590xx.h |  9 ++++---
 2 files changed, 52 insertions(+), 17 deletions(-)

Comments

Lee Jones April 28, 2014, 11:56 a.m. UTC | #1
On Wed, 23 Apr 2014, Matt Porter wrote:

> BCM590xx utilizes a secondary I2C slave address to access additional
> register space. Add support for the secondary address space by
> instantiating a dummy I2C device with the appropriate secondary
> I2C slave address. Also expose a secondary regmap register space so
> that MFD drivers can access this secondary i2c slave address space.
> 
> Signed-off-by: Matt Porter <mporter@linaro.org>
> ---
>  drivers/mfd/bcm590xx.c       | 60 +++++++++++++++++++++++++++++++++-----------
>  include/linux/mfd/bcm590xx.h |  9 ++++---
>  2 files changed, 52 insertions(+), 17 deletions(-)

Nice, flows much better now.

Can I just apply the two MFD patches, or are there cross-deps?
Matt Porter April 28, 2014, 2:09 p.m. UTC | #2
On Mon, Apr 28, 2014 at 12:56:27PM +0100, Lee Jones wrote:
> On Wed, 23 Apr 2014, Matt Porter wrote:
> 
> > BCM590xx utilizes a secondary I2C slave address to access additional
> > register space. Add support for the secondary address space by
> > instantiating a dummy I2C device with the appropriate secondary
> > I2C slave address. Also expose a secondary regmap register space so
> > that MFD drivers can access this secondary i2c slave address space.
> > 
> > Signed-off-by: Matt Porter <mporter@linaro.org>
> > ---
> >  drivers/mfd/bcm590xx.c       | 60 +++++++++++++++++++++++++++++++++-----------
> >  include/linux/mfd/bcm590xx.h |  9 ++++---
> >  2 files changed, 52 insertions(+), 17 deletions(-)
> 
> Nice, flows much better now.
> 
> Can I just apply the two MFD patches, or are there cross-deps?

There's cross-deps so the regulator driver will need to go with it.
Mark mentioned earlier in the thread that he wanted to merge through
the regulator tree though.

-Matt
Lee Jones May 20, 2014, 4:11 p.m. UTC | #3
> > > BCM590xx utilizes a secondary I2C slave address to access additional
> > > register space. Add support for the secondary address space by
> > > instantiating a dummy I2C device with the appropriate secondary
> > > I2C slave address. Also expose a secondary regmap register space so
> > > that MFD drivers can access this secondary i2c slave address space.
> > > 
> > > Signed-off-by: Matt Porter <mporter@linaro.org>
> > > ---
> > >  drivers/mfd/bcm590xx.c       | 60 +++++++++++++++++++++++++++++++++-----------
> > >  include/linux/mfd/bcm590xx.h |  9 ++++---
> > >  2 files changed, 52 insertions(+), 17 deletions(-)
> > 
> > Nice, flows much better now.
> > 
> > Can I just apply the two MFD patches, or are there cross-deps?
> 
> There's cross-deps so the regulator driver will need to go with it.
> Mark mentioned earlier in the thread that he wanted to merge through
> the regulator tree though.

Still nothing.

Mark,
  Can I apply this set and supply you with a pull-request?
Mark Brown May 20, 2014, 7 p.m. UTC | #4
On Tue, May 20, 2014 at 05:11:31PM +0100, Lee Jones wrote:

> > > >  drivers/mfd/bcm590xx.c       | 60 +++++++++++++++++++++++++++++++++-----------
> > > >  include/linux/mfd/bcm590xx.h |  9 ++++---
> > > >  2 files changed, 52 insertions(+), 17 deletions(-)

> > There's cross-deps so the regulator driver will need to go with it.
> > Mark mentioned earlier in the thread that he wanted to merge through
> > the regulator tree though.

> Still nothing.

> Mark,
>   Can I apply this set and supply you with a pull-request?

Can someone please send me whatever it is you want me to look at, the
above diffstat doesn't look relevant.  Given the frequency of respins
I'm not always paying a huge amount of attention to MFD serieses which
look like they're going to be resent.
Matt Porter May 20, 2014, 8:29 p.m. UTC | #5
On Tue, May 20, 2014 at 08:00:55PM +0100, Mark Brown wrote:
> On Tue, May 20, 2014 at 05:11:31PM +0100, Lee Jones wrote:
> 
> > > > >  drivers/mfd/bcm590xx.c       | 60 +++++++++++++++++++++++++++++++++-----------
> > > > >  include/linux/mfd/bcm590xx.h |  9 ++++---
> > > > >  2 files changed, 52 insertions(+), 17 deletions(-)
> 
> > > There's cross-deps so the regulator driver will need to go with it.
> > > Mark mentioned earlier in the thread that he wanted to merge through
> > > the regulator tree though.
> 
> > Still nothing.
> 
> > Mark,
> >   Can I apply this set and supply you with a pull-request?
> 
> Can someone please send me whatever it is you want me to look at, the
> above diffstat doesn't look relevant.  Given the frequency of respins
> I'm not always paying a huge amount of attention to MFD serieses which
> look like they're going to be resent.

Just resent the v2 series.

-Matt
Mark Brown May 20, 2014, 9:44 p.m. UTC | #6
On Tue, May 20, 2014 at 04:29:23PM -0400, Matt Porter wrote:

> Just resent the v2 series.

I looked at that but it seems I already acked the regulator part of the
series and nothing else looked immediately relevant?
Matt Porter May 20, 2014, 9:57 p.m. UTC | #7
On Tue, May 20, 2014 at 10:44:39PM +0100, Mark Brown wrote:
> On Tue, May 20, 2014 at 04:29:23PM -0400, Matt Porter wrote:
> 
> > Just resent the v2 series.
> 
> I looked at that but it seems I already acked the regulator part of the
> series and nothing else looked immediately relevant?

The series has cross dependencies (shared header include) and thus needs
to have both the mfd and regulator portions merged together. You had
mentioned in the v1 version that you'd like to take it through the
regulator tree and so Lee's comments earlier were with regard to you
taking the mfd portions.

-Matt
Mark Brown May 20, 2014, 10:17 p.m. UTC | #8
On Tue, May 20, 2014 at 05:57:26PM -0400, Matt Porter wrote:
> On Tue, May 20, 2014 at 10:44:39PM +0100, Mark Brown wrote:

> > I looked at that but it seems I already acked the regulator part of the
> > series and nothing else looked immediately relevant?

> The series has cross dependencies (shared header include) and thus needs
> to have both the mfd and regulator portions merged together. You had
> mentioned in the v1 version that you'd like to take it through the
> regulator tree and so Lee's comments earlier were with regard to you
> taking the mfd portions.

Right, OK - because it depends on the earlier MFD patches (I think?) I
was expecting it to be applied in MFD and merged over to regulator.
Lee Jones May 21, 2014, 9:26 a.m. UTC | #9
On Tue, 20 May 2014, Mark Brown wrote:

> On Tue, May 20, 2014 at 05:57:26PM -0400, Matt Porter wrote:
> > On Tue, May 20, 2014 at 10:44:39PM +0100, Mark Brown wrote:
> 
> > > I looked at that but it seems I already acked the regulator part of the
> > > series and nothing else looked immediately relevant?
> 
> > The series has cross dependencies (shared header include) and thus needs
> > to have both the mfd and regulator portions merged together. You had
> > mentioned in the v1 version that you'd like to take it through the
> > regulator tree and so Lee's comments earlier were with regard to you
> > taking the mfd portions.
> 
> Right, OK - because it depends on the earlier MFD patches (I think?) I
> was expecting it to be applied in MFD and merged over to regulator.

Great, that's all I needed to know.  I'm on it.
diff mbox

Patch

diff --git a/drivers/mfd/bcm590xx.c b/drivers/mfd/bcm590xx.c
index e9a33c7..43cba1a 100644
--- a/drivers/mfd/bcm590xx.c
+++ b/drivers/mfd/bcm590xx.c
@@ -28,39 +28,71 @@  static const struct mfd_cell bcm590xx_devs[] = {
 	},
 };
 
-static const struct regmap_config bcm590xx_regmap_config = {
+static const struct regmap_config bcm590xx_regmap_config_pri = {
 	.reg_bits	= 8,
 	.val_bits	= 8,
-	.max_register	= BCM590XX_MAX_REGISTER,
+	.max_register	= BCM590XX_MAX_REGISTER_PRI,
 	.cache_type	= REGCACHE_RBTREE,
 };
 
-static int bcm590xx_i2c_probe(struct i2c_client *i2c,
+static const struct regmap_config bcm590xx_regmap_config_sec = {
+	.reg_bits	= 8,
+	.val_bits	= 8,
+	.max_register	= BCM590XX_MAX_REGISTER_SEC,
+	.cache_type	= REGCACHE_RBTREE,
+};
+
+static int bcm590xx_i2c_probe(struct i2c_client *i2c_pri,
 			      const struct i2c_device_id *id)
 {
 	struct bcm590xx *bcm590xx;
 	int ret;
 
-	bcm590xx = devm_kzalloc(&i2c->dev, sizeof(*bcm590xx), GFP_KERNEL);
+	bcm590xx = devm_kzalloc(&i2c_pri->dev, sizeof(*bcm590xx), GFP_KERNEL);
 	if (!bcm590xx)
 		return -ENOMEM;
 
-	i2c_set_clientdata(i2c, bcm590xx);
-	bcm590xx->dev = &i2c->dev;
-	bcm590xx->i2c_client = i2c;
+	i2c_set_clientdata(i2c_pri, bcm590xx);
+	bcm590xx->dev = &i2c_pri->dev;
+	bcm590xx->i2c_pri = i2c_pri;
 
-	bcm590xx->regmap = devm_regmap_init_i2c(i2c, &bcm590xx_regmap_config);
-	if (IS_ERR(bcm590xx->regmap)) {
-		ret = PTR_ERR(bcm590xx->regmap);
-		dev_err(&i2c->dev, "regmap initialization failed: %d\n", ret);
+	bcm590xx->regmap_pri = devm_regmap_init_i2c(i2c_pri,
+						 &bcm590xx_regmap_config_pri);
+	if (IS_ERR(bcm590xx->regmap_pri)) {
+		ret = PTR_ERR(bcm590xx->regmap_pri);
+		dev_err(&i2c_pri->dev, "primary regmap init failed: %d\n", ret);
 		return ret;
 	}
 
-	ret = mfd_add_devices(&i2c->dev, -1, bcm590xx_devs,
+	/* Secondary I2C slave address is the base address with A(2) asserted */
+	bcm590xx->i2c_sec = i2c_new_dummy(i2c_pri->adapter,
+					  i2c_pri->addr | BIT(2));
+	if (IS_ERR_OR_NULL(bcm590xx->i2c_sec)) {
+		dev_err(&i2c_pri->dev, "failed to add secondary I2C device\n");
+		return -ENODEV;
+	}
+	i2c_set_clientdata(bcm590xx->i2c_sec, bcm590xx);
+
+	bcm590xx->regmap_sec = devm_regmap_init_i2c(bcm590xx->i2c_sec,
+						&bcm590xx_regmap_config_sec);
+	if (IS_ERR(bcm590xx->regmap_sec)) {
+		ret = PTR_ERR(bcm590xx->regmap_sec);
+		dev_err(&bcm590xx->i2c_sec->dev,
+			"secondary regmap init failed: %d\n", ret);
+		goto err;
+	}
+
+	ret = mfd_add_devices(&i2c_pri->dev, -1, bcm590xx_devs,
 			      ARRAY_SIZE(bcm590xx_devs), NULL, 0, NULL);
-	if (ret < 0)
-		dev_err(&i2c->dev, "failed to add sub-devices: %d\n", ret);
+	if (ret < 0) {
+		dev_err(&i2c_pri->dev, "failed to add sub-devices: %d\n", ret);
+		goto err;
+	}
+
+	return 0;
 
+err:
+	i2c_unregister_device(bcm590xx->i2c_sec);
 	return ret;
 }
 
diff --git a/include/linux/mfd/bcm590xx.h b/include/linux/mfd/bcm590xx.h
index 434df2d..267aede 100644
--- a/include/linux/mfd/bcm590xx.h
+++ b/include/linux/mfd/bcm590xx.h
@@ -19,12 +19,15 @@ 
 #include <linux/regmap.h>
 
 /* max register address */
-#define BCM590XX_MAX_REGISTER	0xe7
+#define BCM590XX_MAX_REGISTER_PRI	0xe7
+#define BCM590XX_MAX_REGISTER_SEC	0xf0
 
 struct bcm590xx {
 	struct device *dev;
-	struct i2c_client *i2c_client;
-	struct regmap *regmap;
+	struct i2c_client *i2c_pri;
+	struct i2c_client *i2c_sec;
+	struct regmap *regmap_pri;
+	struct regmap *regmap_sec;
 	unsigned int id;
 };