[PATCH 0/4] Support additional regulators on BCM590xx

[PATCH 0/4] Support additional regulators on BCM590xx

[Matt Porter]

This series enables support for 7 additional regulators on the BCM59056
PMU. These regulators are accessed via a secondary I2C slave address.
The MFD driver exposes an additional regmap descriptor for the additional
address space and the regulator implements support for GPLDO1-6 and VBUS
regulators.

Matt Porter (4):
  mfd: bcm590xx: update binding with additional BCM59056 regulators
  mfd: bcm590xx: add support for second i2c slave address space
  regulator: bcm590xx: add support for regulators on secondary i2c slave
  ARM: dts: bcm590xx: add support for GPLDO and VBUS regulators

 Documentation/devicetree/bindings/mfd/bcm590xx.txt |  4 +-
 arch/arm/boot/dts/bcm59056.dtsi                    | 21 +++++
 drivers/mfd/bcm590xx.c                             | 60 ++++++++++----
 drivers/regulator/bcm590xx-regulator.c             | 92 +++++++++++++++++++---
 include/linux/mfd/bcm590xx.h                       |  9 ++-
 5 files changed, 158 insertions(+), 28 deletions(-)

-- 
1.8.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

[PATCH 1/4] mfd: bcm590xx: update binding with additional BCM59056 regulators

[Matt Porter]

The BCM59056 supports GPLDO1-6 and VBUS regulators in a secondary
I2C slave address space. Add these regulators to the list of valid
regulator node names for BCM59056.

Signed-off-by: Matt Porter <mporter-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 Documentation/devicetree/bindings/mfd/bcm590xx.txt | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/mfd/bcm590xx.txt b/Documentation/devicetree/bindings/mfd/bcm590xx.txt
index 1fe30e2..be51a15 100644
--- a/Documentation/devicetree/bindings/mfd/bcm590xx.txt
+++ b/Documentation/devicetree/bindings/mfd/bcm590xx.txt
@@ -19,7 +19,9 @@ Optional child nodes:
   The valid regulator node names for BCM59056 are:
   	rfldo, camldo1, camldo2, simldo1, simldo2, sdldo, sdxldo,
 	mmcldo1, mmcldo2, audldo, micldo, usbldo, vibldo,
-	csr, iosr1, iosr2, msr, sdsr1, sdsr2, vsr
+	csr, iosr1, iosr2, msr, sdsr1, sdsr2, vsr,
+	gpldo1, gpldo2, gpldo3, gpldo4, gpldo5, gpldo6,
+	vbus
 
 Example:
 	pmu: bcm59056@8 {
-- 
1.8.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

[PATCH 2/4] mfd: bcm590xx: add support for second i2c slave address space

[Matt Porter]

BCM590xx utilizes a second i2c slave address to access additional
register space. Add support for the second address space by
instantiated a dummy i2c device with the appropriate secondary
i2c slave address. Expose a second 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(-)

diff --git a/drivers/mfd/bcm590xx.c b/drivers/mfd/bcm590xx.c
index e9a33c7..b710ffa 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_0 = {
 	.reg_bits	= 8,
 	.val_bits	= 8,
-	.max_register	= BCM590XX_MAX_REGISTER,
+	.max_register	= BCM590XX_MAX_REGISTER_0,
 	.cache_type	= REGCACHE_RBTREE,
 };
 
-static int bcm590xx_i2c_probe(struct i2c_client *i2c,
+static const struct regmap_config bcm590xx_regmap_config_1 = {
+	.reg_bits	= 8,
+	.val_bits	= 8,
+	.max_register	= BCM590XX_MAX_REGISTER_1,
+	.cache_type	= REGCACHE_RBTREE,
+};
+
+static int bcm590xx_i2c_probe(struct i2c_client *addmap0,
 			      const struct i2c_device_id *id)
 {
 	struct bcm590xx *bcm590xx;
 	int ret;
 
-	bcm590xx = devm_kzalloc(&i2c->dev, sizeof(*bcm590xx), GFP_KERNEL);
+	bcm590xx = devm_kzalloc(&addmap0->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(addmap0, bcm590xx);
+	bcm590xx->dev = &addmap0->dev;
+	bcm590xx->addmap0 = addmap0;
 
-	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->regmap0 = devm_regmap_init_i2c(addmap0,
+						 &bcm590xx_regmap_config_0);
+	if (IS_ERR(bcm590xx->regmap0)) {
+		ret = PTR_ERR(bcm590xx->regmap0);
+		dev_err(&addmap0->dev, "regmap 0 init failed: %d\n", ret);
 		return ret;
 	}
 
-	ret = mfd_add_devices(&i2c->dev, -1, bcm590xx_devs,
+	/* Second I2C slave address is the base address with A(2) asserted */
+	bcm590xx->addmap1 = i2c_new_dummy(addmap0->adapter,
+					  addmap0->addr | BIT(2));
+	if (IS_ERR_OR_NULL(bcm590xx->addmap1)) {
+		dev_err(&addmap0->dev, "failed to add address map 1 device\n");
+		return -ENODEV;
+	}
+	i2c_set_clientdata(bcm590xx->addmap1, bcm590xx);
+
+	bcm590xx->regmap1 = devm_regmap_init_i2c(bcm590xx->addmap1,
+						&bcm590xx_regmap_config_1);
+	if (IS_ERR(bcm590xx->regmap1)) {
+		ret = PTR_ERR(bcm590xx->regmap1);
+		dev_err(&bcm590xx->addmap1->dev,
+			"regmap 1 init failed: %d\n", ret);
+		goto err;
+	}
+
+	ret = mfd_add_devices(&addmap0->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(&addmap0->dev, "failed to add sub-devices: %d\n", ret);
+		goto err;
+	}
+
+	return 0;
 
+err:
+	i2c_unregister_device(bcm590xx->addmap1);
 	return ret;
 }
 
diff --git a/include/linux/mfd/bcm590xx.h b/include/linux/mfd/bcm590xx.h
index 434df2d..a2723f2 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_0	0xe7
+#define BCM590XX_MAX_REGISTER_1	0xf0
 
 struct bcm590xx {
 	struct device *dev;
-	struct i2c_client *i2c_client;
-	struct regmap *regmap;
+	struct i2c_client *addmap0;
+	struct i2c_client *addmap1;
+	struct regmap *regmap0;
+	struct regmap *regmap1;
 	unsigned int id;
 };
 
-- 
1.8.4

Re: [PATCH 2/4] mfd: bcm590xx: add support for second i2c slave address space

[Lee Jones]

On Mon, 14 Apr 2014, Matt Porter wrote:

> BCM590xx utilizes a second i2c slave address to access additional

s/i2c/I2C

> register space. Add support for the second address space by
> instantiated a dummy i2c device with the appropriate secondary

s/instantiated/instantiating

> i2c slave address. Expose a second regmap register space so that

s/i2c/I2C

Exposing?

s/regmap/Regmap

> mfd drivers can access this secondary i2c slave address space.

s/mfd/MFD

s/i2c/I2C

> Signed-off-by: Matt Porter <mporter-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  drivers/mfd/bcm590xx.c       | 60 +++++++++++++++++++++++++++++++++-----------
>  include/linux/mfd/bcm590xx.h |  9 ++++---
>  2 files changed, 52 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/mfd/bcm590xx.c b/drivers/mfd/bcm590xx.c
> index e9a33c7..b710ffa 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_0 = {

Not loving _0 and _1 appendages.

Is one of them {primary|master} and the other {secondary|slave}?

>  	.reg_bits	= 8,
>  	.val_bits	= 8,
> -	.max_register	= BCM590XX_MAX_REGISTER,
> +	.max_register	= BCM590XX_MAX_REGISTER_0,
>  	.cache_type	= REGCACHE_RBTREE,
>  };
>  
> -static int bcm590xx_i2c_probe(struct i2c_client *i2c,
> +static const struct regmap_config bcm590xx_regmap_config_1 = {
> +	.reg_bits	= 8,
> +	.val_bits	= 8,
> +	.max_register	= BCM590XX_MAX_REGISTER_1,
> +	.cache_type	= REGCACHE_RBTREE,
> +};
> +
> +static int bcm590xx_i2c_probe(struct i2c_client *addmap0,

Would this be best left as i2c, then naming the other one
i2c_secondary for instance?

addmap{0,1} doesn't quite sit right with me.

REVISIT: Ah, it's address-map, rather than add map. Okay, not as bad
as I first thought, but still, is there a better naming convention you
could use?

>  			      const struct i2c_device_id *id)
>  {
>  	struct bcm590xx *bcm590xx;
>  	int ret;
>  
> -	bcm590xx = devm_kzalloc(&i2c->dev, sizeof(*bcm590xx), GFP_KERNEL);
> +	bcm590xx = devm_kzalloc(&addmap0->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(addmap0, bcm590xx);
> +	bcm590xx->dev = &addmap0->dev;
> +	bcm590xx->addmap0 = addmap0;
>  
> -	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->regmap0 = devm_regmap_init_i2c(addmap0,
> +						 &bcm590xx_regmap_config_0);
> +	if (IS_ERR(bcm590xx->regmap0)) {
> +		ret = PTR_ERR(bcm590xx->regmap0);
> +		dev_err(&addmap0->dev, "regmap 0 init failed: %d\n", ret);
>  		return ret;
>  	}
>  
> -	ret = mfd_add_devices(&i2c->dev, -1, bcm590xx_devs,
> +	/* Second I2C slave address is the base address with A(2) asserted */
> +	bcm590xx->addmap1 = i2c_new_dummy(addmap0->adapter,
> +					  addmap0->addr | BIT(2));
> +	if (IS_ERR_OR_NULL(bcm590xx->addmap1)) {
> +		dev_err(&addmap0->dev, "failed to add address map 1 device\n");
> +		return -ENODEV;
> +	}
> +	i2c_set_clientdata(bcm590xx->addmap1, bcm590xx);
> +
> +	bcm590xx->regmap1 = devm_regmap_init_i2c(bcm590xx->addmap1,
> +						&bcm590xx_regmap_config_1);
> +	if (IS_ERR(bcm590xx->regmap1)) {
> +		ret = PTR_ERR(bcm590xx->regmap1);
> +		dev_err(&bcm590xx->addmap1->dev,
> +			"regmap 1 init failed: %d\n", ret);
> +		goto err;
> +	}
> +
> +	ret = mfd_add_devices(&addmap0->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(&addmap0->dev, "failed to add sub-devices: %d\n", ret);
> +		goto err;
> +	}
> +
> +	return 0;
>  
> +err:
> +	i2c_unregister_device(bcm590xx->addmap1);
>  	return ret;
>  }
>  
> diff --git a/include/linux/mfd/bcm590xx.h b/include/linux/mfd/bcm590xx.h
> index 434df2d..a2723f2 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_0	0xe7
> +#define BCM590XX_MAX_REGISTER_1	0xf0
>  
>  struct bcm590xx {
>  	struct device *dev;
> -	struct i2c_client *i2c_client;
> -	struct regmap *regmap;
> +	struct i2c_client *addmap0;
> +	struct i2c_client *addmap1;
> +	struct regmap *regmap0;
> +	struct regmap *regmap1;
>  	unsigned int id;
>  };
>  

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
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

Re: [PATCH 2/4] mfd: bcm590xx: add support for second i2c slave address space

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 379 bytes --]

On Wed, Apr 16, 2014 at 12:06:03PM +0100, Lee Jones wrote:

> s/regmap/Regmap

It's consistently written regmap in all the documentation and so on :)

> addmap{0,1} doesn't quite sit right with me.

> REVISIT: Ah, it's address-map, rather than add map. Okay, not as bad
> as I first thought, but still, is there a better naming convention you
> could use?

addrmap or something?

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 2/4] mfd: bcm590xx: add support for second i2c slave address space

[Lee Jones]

> > s/regmap/Regmap
> 
> It's consistently written regmap in all the documentation and so on :)

Furry muff; but the comments still stand for the acronyms.

> > addmap{0,1} doesn't quite sit right with me.
> 
> > REVISIT: Ah, it's address-map, rather than add map. Okay, not as bad
> > as I first thought, but still, is there a better naming convention you
> > could use?
> 
> addrmap or something?

Right, that was what I was thinking. However, I prefer something along
the lines of 'i2c' and 'i2c_sec' or 'client' and 'client_slv' etc.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
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

Re: [PATCH 2/4] mfd: bcm590xx: add support for second i2c slave address space

[Matt Porter]

On Thu, Apr 17, 2014 at 07:57:53AM +0100, Lee Jones wrote:
> > > s/regmap/Regmap
> > 
> > It's consistently written regmap in all the documentation and so on :)
> 
> Furry muff; but the comments still stand for the acronyms.
> 
> > > addmap{0,1} doesn't quite sit right with me.
> > 
> > > REVISIT: Ah, it's address-map, rather than add map. Okay, not as bad
> > > as I first thought, but still, is there a better naming convention you
> > > could use?
> > 
> > addrmap or something?
> 
> Right, that was what I was thinking. However, I prefer something along
> the lines of 'i2c' and 'i2c_sec' or 'client' and 'client_slv' etc.

FWIW, the reason it's addmap{0,1} is that the datasheet has documents
ADDMAP=0 and the first bank of registers and ADDMAP=1 as the second bank
of registers. I adopted that to match the docs for the part.

I guess we could do i2c and i2c_sec, I'll just have to put a comment
correlating it to the h/w. Calling it 'slv' implies something else
so we should avoid that here. The notion of a "secondary" i2c device
is completely a Linux I2C subsystem fabrication which wouldn't exist
if it allowed multiple slave addresses per device. From a h/w
perspective there is really no primary and secondary relationship.

I'm fine with i2c/i2c_sec or addrmap0/1 and I will just comment to
correlate with the datasheet..pick one.

-Matt
> 
> -- 
> Lee Jones
> Linaro STMicroelectronics Landing Team Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog
--
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

Re: [PATCH 2/4] mfd: bcm590xx: add support for second i2c slave address space

[Lee Jones]

> > > > s/regmap/Regmap
> > > 
> > > It's consistently written regmap in all the documentation and so on :)
> > 
> > Furry muff; but the comments still stand for the acronyms.
> > 
> > > > addmap{0,1} doesn't quite sit right with me.
> > > 
> > > > REVISIT: Ah, it's address-map, rather than add map. Okay, not as bad
> > > > as I first thought, but still, is there a better naming convention you
> > > > could use?
> > > 
> > > addrmap or something?
> > 
> > Right, that was what I was thinking. However, I prefer something along
> > the lines of 'i2c' and 'i2c_sec' or 'client' and 'client_slv' etc.
> 
> FWIW, the reason it's addmap{0,1} is that the datasheet has documents
> ADDMAP=0 and the first bank of registers and ADDMAP=1 as the second bank
> of registers. I adopted that to match the docs for the part.
> 
> I guess we could do i2c and i2c_sec, I'll just have to put a comment
> correlating it to the h/w. Calling it 'slv' implies something else
> so we should avoid that here. The notion of a "secondary" i2c device
> is completely a Linux I2C subsystem fabrication which wouldn't exist
> if it allowed multiple slave addresses per device. From a h/w
> perspective there is really no primary and secondary relationship.
> 
> I'm fine with i2c/i2c_sec or addrmap0/1 and I will just comment to
> correlate with the datasheet..pick one.

Let's stick method fabricated by the I2C subsystem. It may seem strange
from a h/w perspective, but it is the way we (you) have coded it, as
the first parameter of i2c_new_dummy() is the 'managing' (primary,
parent, master, whatever) device, so '_sec' would suit as an
identifying appendage for the resultant device.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 2/4] mfd: bcm590xx: add support for second i2c slave address space

[Matt Porter]

On Tue, Apr 22, 2014 at 09:21:39AM +0100, Lee Jones wrote:
> > > > > s/regmap/Regmap
> > > > 
> > > > It's consistently written regmap in all the documentation and so on :)
> > > 
> > > Furry muff; but the comments still stand for the acronyms.
> > > 
> > > > > addmap{0,1} doesn't quite sit right with me.
> > > > 
> > > > > REVISIT: Ah, it's address-map, rather than add map. Okay, not as bad
> > > > > as I first thought, but still, is there a better naming convention you
> > > > > could use?
> > > > 
> > > > addrmap or something?
> > > 
> > > Right, that was what I was thinking. However, I prefer something along
> > > the lines of 'i2c' and 'i2c_sec' or 'client' and 'client_slv' etc.
> > 
> > FWIW, the reason it's addmap{0,1} is that the datasheet has documents
> > ADDMAP=0 and the first bank of registers and ADDMAP=1 as the second bank
> > of registers. I adopted that to match the docs for the part.
> > 
> > I guess we could do i2c and i2c_sec, I'll just have to put a comment
> > correlating it to the h/w. Calling it 'slv' implies something else
> > so we should avoid that here. The notion of a "secondary" i2c device
> > is completely a Linux I2C subsystem fabrication which wouldn't exist
> > if it allowed multiple slave addresses per device. From a h/w
> > perspective there is really no primary and secondary relationship.
> > 
> > I'm fine with i2c/i2c_sec or addrmap0/1 and I will just comment to
> > correlate with the datasheet..pick one.
> 
> Let's stick method fabricated by the I2C subsystem. It may seem strange
> from a h/w perspective, but it is the way we (you) have coded it, as
> the first parameter of i2c_new_dummy() is the 'managing' (primary,
> parent, master, whatever) device, so '_sec' would suit as an
> identifying appendage for the resultant device.

That works, I'll also switch to addrmap_[pri|sec] which touches the
regulator driver as well. That will keep the relationship between device
and regmap clear.

-Matt

Re: [PATCH 2/4] mfd: bcm590xx: add support for second i2c slave address space

[Matt Porter]

On Wed, Apr 23, 2014 at 06:01:26PM -0400, Matt Porter wrote:
> On Tue, Apr 22, 2014 at 09:21:39AM +0100, Lee Jones wrote:
> > > > > > s/regmap/Regmap
> > > > > 
> > > > > It's consistently written regmap in all the documentation and so on :)
> > > > 
> > > > Furry muff; but the comments still stand for the acronyms.
> > > > 
> > > > > > addmap{0,1} doesn't quite sit right with me.
> > > > > 
> > > > > > REVISIT: Ah, it's address-map, rather than add map. Okay, not as bad
> > > > > > as I first thought, but still, is there a better naming convention you
> > > > > > could use?
> > > > > 
> > > > > addrmap or something?
> > > > 
> > > > Right, that was what I was thinking. However, I prefer something along
> > > > the lines of 'i2c' and 'i2c_sec' or 'client' and 'client_slv' etc.
> > > 
> > > FWIW, the reason it's addmap{0,1} is that the datasheet has documents
> > > ADDMAP=0 and the first bank of registers and ADDMAP=1 as the second bank
> > > of registers. I adopted that to match the docs for the part.
> > > 
> > > I guess we could do i2c and i2c_sec, I'll just have to put a comment
> > > correlating it to the h/w. Calling it 'slv' implies something else
> > > so we should avoid that here. The notion of a "secondary" i2c device
> > > is completely a Linux I2C subsystem fabrication which wouldn't exist
> > > if it allowed multiple slave addresses per device. From a h/w
> > > perspective there is really no primary and secondary relationship.
> > > 
> > > I'm fine with i2c/i2c_sec or addrmap0/1 and I will just comment to
> > > correlate with the datasheet..pick one.
> > 
> > Let's stick method fabricated by the I2C subsystem. It may seem strange
> > from a h/w perspective, but it is the way we (you) have coded it, as
> > the first parameter of i2c_new_dummy() is the 'managing' (primary,
> > parent, master, whatever) device, so '_sec' would suit as an
> > identifying appendage for the resultant device.
> 
> That works, I'll also switch to addrmap_[pri|sec] which touches the
> regulator driver as well. That will keep the relationship  between device
> and regmap clear.

Misspoke...I'm switching regmap[0|1] to regmap_[pri|sec] to keep that
synced with i2c_[pri|sec]

-Matt

Re: [PATCH 2/4] mfd: bcm590xx: add support for second i2c slave address space

[Lee Jones]

> > > > > > > s/regmap/Regmap
> > > > > > 
> > > > > > It's consistently written regmap in all the documentation and so on :)
> > > > > 
> > > > > Furry muff; but the comments still stand for the acronyms.
> > > > > 
> > > > > > > addmap{0,1} doesn't quite sit right with me.
> > > > > > 
> > > > > > > REVISIT: Ah, it's address-map, rather than add map. Okay, not as bad
> > > > > > > as I first thought, but still, is there a better naming convention you
> > > > > > > could use?
> > > > > > 
> > > > > > addrmap or something?
> > > > > 
> > > > > Right, that was what I was thinking. However, I prefer something along
> > > > > the lines of 'i2c' and 'i2c_sec' or 'client' and 'client_slv' etc.
> > > > 
> > > > FWIW, the reason it's addmap{0,1} is that the datasheet has documents
> > > > ADDMAP=0 and the first bank of registers and ADDMAP=1 as the second bank
> > > > of registers. I adopted that to match the docs for the part.
> > > > 
> > > > I guess we could do i2c and i2c_sec, I'll just have to put a comment
> > > > correlating it to the h/w. Calling it 'slv' implies something else
> > > > so we should avoid that here. The notion of a "secondary" i2c device
> > > > is completely a Linux I2C subsystem fabrication which wouldn't exist
> > > > if it allowed multiple slave addresses per device. From a h/w
> > > > perspective there is really no primary and secondary relationship.
> > > > 
> > > > I'm fine with i2c/i2c_sec or addrmap0/1 and I will just comment to
> > > > correlate with the datasheet..pick one.
> > > 
> > > Let's stick method fabricated by the I2C subsystem. It may seem strange
> > > from a h/w perspective, but it is the way we (you) have coded it, as
> > > the first parameter of i2c_new_dummy() is the 'managing' (primary,
> > > parent, master, whatever) device, so '_sec' would suit as an
> > > identifying appendage for the resultant device.
> > 
> > That works, I'll also switch to addrmap_[pri|sec] which touches the
> > regulator driver as well. That will keep the relationship  between device
> > and regmap clear.
> 
> Misspoke...I'm switching regmap[0|1] to regmap_[pri|sec] to keep that
> synced with i2c_[pri|sec]

Sounds good, thanks Matt.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
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

Re: [PATCH 2/4] mfd: bcm590xx: add support for second i2c slave address space

[Matt Porter]

On Wed, Apr 16, 2014 at 12:06:03PM +0100, Lee Jones wrote:
> On Mon, 14 Apr 2014, Matt Porter wrote:
> 
> > BCM590xx utilizes a second i2c slave address to access additional
> 
> s/i2c/I2C
> 
> > register space. Add support for the second address space by
> > instantiated a dummy i2c device with the appropriate secondary
> 
> s/instantiated/instantiating
> 
> > i2c slave address. Expose a second regmap register space so that
> 
> s/i2c/I2C
> 
> Exposing?
> 
> s/regmap/Regmap
> 
> > mfd drivers can access this secondary i2c slave address space.
> 
> s/mfd/MFD
> 
> s/i2c/I2C

Ok, I'll fix the capitalization and wording..except for regmap as
noted by Mark.

> 
> > Signed-off-by: Matt Porter <mporter-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > ---
> >  drivers/mfd/bcm590xx.c       | 60 +++++++++++++++++++++++++++++++++-----------
> >  include/linux/mfd/bcm590xx.h |  9 ++++---
> >  2 files changed, 52 insertions(+), 17 deletions(-)
> > 
> > diff --git a/drivers/mfd/bcm590xx.c b/drivers/mfd/bcm590xx.c
> > index e9a33c7..b710ffa 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_0 = {
> 
> Not loving _0 and _1 appendages.
> 
> Is one of them {primary|master} and the other {secondary|slave}?

I guess from a Linux I2C subsystem, we can view _1 as the
"secondary"...it does correspond the the i2c_new_dummy() device
that we create in the mfd probe. That device corresponds to the
ADDMAP=1 address on the PMU. This is why I used those appendages.

-Matt
--
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

[PATCH 3/4] regulator: bcm590xx: add support for regulators on secondary i2c slave

[Matt Porter]

The bcm590xx mfd driver now exposes a second regmap descriptor making
the registers for regulators on the secondary i2c slave address
available.  Add support for GPLDO1-6 and VBUS regulators found within
this register range.

Signed-off-by: Matt Porter <mporter@linaro.org>
---
 drivers/regulator/bcm590xx-regulator.c | 92 ++++++++++++++++++++++++++++++----
 1 file changed, 82 insertions(+), 10 deletions(-)

diff --git a/drivers/regulator/bcm590xx-regulator.c b/drivers/regulator/bcm590xx-regulator.c
index c3750c5..4f0b5d9 100644
--- a/drivers/regulator/bcm590xx-regulator.c
+++ b/drivers/regulator/bcm590xx-regulator.c
@@ -22,7 +22,7 @@
 #include <linux/regulator/of_regulator.h>
 #include <linux/slab.h>
 
-/* Register defs */
+/* I2C slave 0 registers */
 #define BCM590XX_RFLDOPMCTRL1	0x60
 #define BCM590XX_IOSR1PMCTRL1	0x7a
 #define BCM590XX_IOSR2PMCTRL1	0x7c
@@ -31,13 +31,34 @@
 #define BCM590XX_SDSR2PMCTRL1	0x86
 #define BCM590XX_MSRPMCTRL1	0x8a
 #define BCM590XX_VSRPMCTRL1	0x8e
-#define BCM590XX_REG_ENABLE	BIT(7)
-
 #define BCM590XX_RFLDOCTRL	0x96
 #define BCM590XX_CSRVOUT1	0xc0
+
+/* I2C slave 1 registers */
+#define BCM590XX_GPLDO5PMCTRL1	0x16
+#define BCM590XX_GPLDO6PMCTRL1	0x18
+#define BCM590XX_GPLDO1CTRL	0x1a
+#define BCM590XX_GPLDO2CTRL	0x1b
+#define BCM590XX_GPLDO3CTRL	0x1c
+#define BCM590XX_GPLDO4CTRL	0x1d
+#define BCM590XX_GPLDO5CTRL	0x1e
+#define BCM590XX_GPLDO6CTRL	0x1f
+#define BCM590XX_OTG_CTRL	0x40
+#define BCM590XX_GPLDO1PMCTRL1	0x57
+#define BCM590XX_GPLDO2PMCTRL1	0x59
+#define BCM590XX_GPLDO3PMCTRL1	0x5b
+#define BCM590XX_GPLDO4PMCTRL1	0x5d
+
+#define BCM590XX_REG_ENABLE	BIT(7)
+#define BCM590XX_VBUS_ENABLE	BIT(2)
 #define BCM590XX_LDO_VSEL_MASK	GENMASK(5, 3)
 #define BCM590XX_SR_VSEL_MASK	GENMASK(5, 0)
 
+/*
+ * RFLDO to VSR regulators are
+ * accessed via I2C slave 0
+ */
+
 /* LDO regulator IDs */
 #define BCM590XX_REG_RFLDO	0
 #define BCM590XX_REG_CAMLDO1	1
@@ -62,9 +83,25 @@
 #define BCM590XX_REG_SDSR2	18
 #define BCM590XX_REG_VSR	19
 
-#define BCM590XX_NUM_REGS	20
+/*
+ * GPLDO1 to VBUS regulators are
+ * accessed via I2C slave 1
+ */
+
+#define BCM590XX_REG_GPLDO1	20
+#define BCM590XX_REG_GPLDO2	21
+#define BCM590XX_REG_GPLDO3	22
+#define BCM590XX_REG_GPLDO4	23
+#define BCM590XX_REG_GPLDO5	24
+#define BCM590XX_REG_GPLDO6	25
+#define BCM590XX_REG_VBUS	26
+
+#define BCM590XX_NUM_REGS	27
 
 #define BCM590XX_REG_IS_LDO(n)	(n < BCM590XX_REG_CSR)
+#define BCM590XX_REG_IS_GPLDO(n) \
+	((n > BCM590XX_REG_VSR) && (n < BCM590XX_REG_VBUS))
+#define BCM590XX_REG_IS_VBUS(n)	(n == BCM590XX_REG_VBUS)
 
 struct bcm590xx_board {
 	struct regulator_init_data *bcm590xx_pmu_init_data[BCM590XX_NUM_REGS];
@@ -149,6 +186,12 @@ static struct bcm590xx_info bcm590xx_regs[] = {
 	BCM590XX_REG_RANGES(sdsr1, dcdc_sdsr1_ranges),
 	BCM590XX_REG_RANGES(sdsr2, dcdc_iosr1_ranges),
 	BCM590XX_REG_RANGES(vsr, dcdc_iosr1_ranges),
+	BCM590XX_REG_TABLE(gpldo1, ldo_a_table),
+	BCM590XX_REG_TABLE(gpldo2, ldo_a_table),
+	BCM590XX_REG_TABLE(gpldo3, ldo_a_table),
+	BCM590XX_REG_TABLE(gpldo4, ldo_a_table),
+	BCM590XX_REG_TABLE(gpldo5, ldo_a_table),
+	BCM590XX_REG_TABLE(gpldo6, ldo_a_table),
 };
 
 struct bcm590xx_reg {
@@ -161,6 +204,8 @@ static int bcm590xx_get_vsel_register(int id)
 {
 	if (BCM590XX_REG_IS_LDO(id))
 		return BCM590XX_RFLDOCTRL + id;
+	else if (BCM590XX_REG_IS_GPLDO(id))
+		return BCM590XX_GPLDO1CTRL + id;
 	else
 		return BCM590XX_CSRVOUT1 + (id - BCM590XX_REG_CSR) * 3;
 }
@@ -171,6 +216,8 @@ static int bcm590xx_get_enable_register(int id)
 
 	if (BCM590XX_REG_IS_LDO(id))
 		reg = BCM590XX_RFLDOPMCTRL1 + id * 2;
+	else if (BCM590XX_REG_IS_GPLDO(id))
+		reg = BCM590XX_GPLDO1PMCTRL1 + id * 2;
 	else
 		switch (id) {
 		case BCM590XX_REG_CSR:
@@ -191,8 +238,11 @@ static int bcm590xx_get_enable_register(int id)
 		case BCM590XX_REG_SDSR2:
 			reg = BCM590XX_SDSR2PMCTRL1;
 			break;
+		case BCM590XX_REG_VBUS:
+			reg = BCM590XX_OTG_CTRL;
 		};
 
+
 	return reg;
 }
 
@@ -216,6 +266,12 @@ static struct regulator_ops bcm590xx_ops_dcdc = {
 	.map_voltage		= regulator_map_voltage_linear_range,
 };
 
+static struct regulator_ops bcm590xx_ops_vbus = {
+	.is_enabled		= regulator_is_enabled_regmap,
+	.enable			= regulator_enable_regmap,
+	.disable		= regulator_disable_regmap,
+};
+
 #define BCM590XX_MATCH(_name, _id) \
 	{ \
 		.name = #_name, \
@@ -243,6 +299,13 @@ static struct of_regulator_match bcm590xx_matches[] = {
 	BCM590XX_MATCH(sdsr1, SDSR1),
 	BCM590XX_MATCH(sdsr2, SDSR2),
 	BCM590XX_MATCH(vsr, VSR),
+	BCM590XX_MATCH(gpldo1, GPLDO1),
+	BCM590XX_MATCH(gpldo2, GPLDO2),
+	BCM590XX_MATCH(gpldo3, GPLDO3),
+	BCM590XX_MATCH(gpldo4, GPLDO4),
+	BCM590XX_MATCH(gpldo5, GPLDO5),
+	BCM590XX_MATCH(gpldo6, GPLDO6),
+	BCM590XX_MATCH(vbus, VBUS),
 };
 
 static struct bcm590xx_board *bcm590xx_parse_dt_reg_data(
@@ -353,17 +416,23 @@ static int bcm590xx_probe(struct platform_device *pdev)
 		pmu->desc[i].linear_ranges = info->linear_ranges;
 		pmu->desc[i].n_linear_ranges = info->n_linear_ranges;
 
-		if (BCM590XX_REG_IS_LDO(i)) {
+		if ((BCM590XX_REG_IS_LDO(i)) || (BCM590XX_REG_IS_GPLDO(i))) {
 			pmu->desc[i].ops = &bcm590xx_ops_ldo;
 			pmu->desc[i].vsel_mask = BCM590XX_LDO_VSEL_MASK;
-		} else {
+		} else if (BCM590XX_REG_IS_VBUS(i))
+			pmu->desc[i].ops = &bcm590xx_ops_vbus;
+		else {
 			pmu->desc[i].ops = &bcm590xx_ops_dcdc;
 			pmu->desc[i].vsel_mask = BCM590XX_SR_VSEL_MASK;
 		}
 
-		pmu->desc[i].vsel_reg = bcm590xx_get_vsel_register(i);
-		pmu->desc[i].enable_is_inverted = true;
-		pmu->desc[i].enable_mask = BCM590XX_REG_ENABLE;
+		if (BCM590XX_REG_IS_VBUS(i))
+			pmu->desc[i].enable_mask = BCM590XX_VBUS_ENABLE;
+		else {
+			pmu->desc[i].vsel_reg = bcm590xx_get_vsel_register(i);
+			pmu->desc[i].enable_is_inverted = true;
+			pmu->desc[i].enable_mask = BCM590XX_REG_ENABLE;
+		}
 		pmu->desc[i].enable_reg = bcm590xx_get_enable_register(i);
 		pmu->desc[i].type = REGULATOR_VOLTAGE;
 		pmu->desc[i].owner = THIS_MODULE;
@@ -371,7 +440,10 @@ static int bcm590xx_probe(struct platform_device *pdev)
 		config.dev = bcm590xx->dev;
 		config.init_data = reg_data;
 		config.driver_data = pmu;
-		config.regmap = bcm590xx->regmap;
+		if (BCM590XX_REG_IS_GPLDO(i) || BCM590XX_REG_IS_VBUS(i))
+			config.regmap = bcm590xx->regmap1;
+		else
+			config.regmap = bcm590xx->regmap0;
 
 		if (bcm590xx_reg_matches)
 			config.of_node = bcm590xx_reg_matches[i].of_node;
-- 
1.8.4

Re: [PATCH 3/4] regulator: bcm590xx: add support for regulators on secondary i2c slave

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 517 bytes --]

On Mon, Apr 14, 2014 at 02:50:27PM -0400, Matt Porter wrote:
> The bcm590xx mfd driver now exposes a second regmap descriptor making
> the registers for regulators on the secondary i2c slave address
> available.  Add support for GPLDO1-6 and VBUS regulators found within
> this register range.
> 
> Signed-off-by: Matt Porter <mporter@linaro.org>

Acked-by: Mark Brown <broonie@linaro.org>

though if possible I'd prefer to merge this into the regulator tree
which would need a cross tree merge with MFD.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH 4/4] ARM: dts: bcm590xx: add support for GPLDO and VBUS regulators

[Matt Porter]

Adds additional nodes to support GPLDO1-6 and VBUS regulators which
are not supported in the bcm590xx regulator driver.

Signed-off-by: Matt Porter <mporter@linaro.org>
---
 arch/arm/boot/dts/bcm59056.dtsi | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/arch/arm/boot/dts/bcm59056.dtsi b/arch/arm/boot/dts/bcm59056.dtsi
index dfadaaa..066adfb 100644
--- a/arch/arm/boot/dts/bcm59056.dtsi
+++ b/arch/arm/boot/dts/bcm59056.dtsi
@@ -70,5 +70,26 @@
 
 		vsr_reg: vsr {
 		};
+
+		gpldo1_reg: gpldo1 {
+		};
+
+		gpldo2_reg: gpldo2 {
+		};
+
+		gpldo3_reg: gpldo3 {
+		};
+
+		gpldo4_reg: gpldo4 {
+		};
+
+		gpldo5_reg: gpldo5 {
+		};
+
+		gpldo6_reg: gpldo6 {
+		};
+
+		vbus_reg: vbus {
+		};
 	};
 };
-- 
1.8.4