[PATCH] mfd: Convert pcf50633 to use new register map API

[PATCH] mfd: Convert pcf50633 to use new register map API

[Mark Brown]

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---

The regmap API has gone into mainline now so this can be merged into
-next.

 drivers/mfd/Kconfig               |    1 +
 drivers/mfd/pcf50633-core.c       |  114 +++++++++---------------------------
 include/linux/mfd/pcf50633/core.h |    3 +-
 3 files changed, 32 insertions(+), 86 deletions(-)

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index a67adcb..a992948 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -503,6 +503,7 @@ config MFD_WM8994
 config MFD_PCF50633
 	tristate "Support for NXP PCF50633"
 	depends on I2C
+	select REGMAP_I2C
 	help
 	  Say yes here if you have NXP PCF50633 chip on your board.
 	  This core driver provides register access and IRQ handling
diff --git a/drivers/mfd/pcf50633-core.c b/drivers/mfd/pcf50633-core.c
index 57868416..77ca3b3 100644
--- a/drivers/mfd/pcf50633-core.c
+++ b/drivers/mfd/pcf50633-core.c
@@ -23,45 +23,16 @@
 #include <linux/i2c.h>
 #include <linux/pm.h>
 #include <linux/slab.h>
+#include <linux/regmap.h>
+#include <linux/err.h>
 
 #include <linux/mfd/pcf50633/core.h>
 
-static int __pcf50633_read(struct pcf50633 *pcf, u8 reg, int num, u8 *data)
-{
-	int ret;
-
-	ret = i2c_smbus_read_i2c_block_data(pcf->i2c_client, reg,
-				num, data);
-	if (ret < 0)
-		dev_err(pcf->dev, "Error reading %d regs at %d\n", num, reg);
-
-	return ret;
-}
-
-static int __pcf50633_write(struct pcf50633 *pcf, u8 reg, int num, u8 *data)
-{
-	int ret;
-
-	ret = i2c_smbus_write_i2c_block_data(pcf->i2c_client, reg,
-				num, data);
-	if (ret < 0)
-		dev_err(pcf->dev, "Error writing %d regs at %d\n", num, reg);
-
-	return ret;
-
-}
-
 /* Read a block of up to 32 regs  */
 int pcf50633_read_block(struct pcf50633 *pcf, u8 reg,
 					int nr_regs, u8 *data)
 {
-	int ret;
-
-	mutex_lock(&pcf->lock);
-	ret = __pcf50633_read(pcf, reg, nr_regs, data);
-	mutex_unlock(&pcf->lock);
-
-	return ret;
+	return regmap_raw_read(pcf->regmap, reg, data, nr_regs);
 }
 EXPORT_SYMBOL_GPL(pcf50633_read_block);
 
@@ -69,23 +40,18 @@ EXPORT_SYMBOL_GPL(pcf50633_read_block);
 int pcf50633_write_block(struct pcf50633 *pcf , u8 reg,
 					int nr_regs, u8 *data)
 {
-	int ret;
-
-	mutex_lock(&pcf->lock);
-	ret = __pcf50633_write(pcf, reg, nr_regs, data);
-	mutex_unlock(&pcf->lock);
-
-	return ret;
+	return regmap_raw_write(pcf->regmap, reg, data, nr_regs);
 }
 EXPORT_SYMBOL_GPL(pcf50633_write_block);
 
 u8 pcf50633_reg_read(struct pcf50633 *pcf, u8 reg)
 {
-	u8 val;
+	unsigned int val;
+	int ret;
 
-	mutex_lock(&pcf->lock);
-	__pcf50633_read(pcf, reg, 1, &val);
-	mutex_unlock(&pcf->lock);
+	ret = regmap_read(pcf->regmap, reg, &val);
+	if (ret < 0)
+		return -1;
 
 	return val;
 }
@@ -93,56 +59,19 @@ EXPORT_SYMBOL_GPL(pcf50633_reg_read);
 
 int pcf50633_reg_write(struct pcf50633 *pcf, u8 reg, u8 val)
 {
-	int ret;
-
-	mutex_lock(&pcf->lock);
-	ret = __pcf50633_write(pcf, reg, 1, &val);
-	mutex_unlock(&pcf->lock);
-
-	return ret;
+	return regmap_write(pcf->regmap, reg, val);
 }
 EXPORT_SYMBOL_GPL(pcf50633_reg_write);
 
 int pcf50633_reg_set_bit_mask(struct pcf50633 *pcf, u8 reg, u8 mask, u8 val)
 {
-	int ret;
-	u8 tmp;
-
-	val &= mask;
-
-	mutex_lock(&pcf->lock);
-	ret = __pcf50633_read(pcf, reg, 1, &tmp);
-	if (ret < 0)
-		goto out;
-
-	tmp &= ~mask;
-	tmp |= val;
-	ret = __pcf50633_write(pcf, reg, 1, &tmp);
-
-out:
-	mutex_unlock(&pcf->lock);
-
-	return ret;
+	return regmap_update_bits(pcf->regmap, reg, mask, val);
 }
 EXPORT_SYMBOL_GPL(pcf50633_reg_set_bit_mask);
 
 int pcf50633_reg_clear_bits(struct pcf50633 *pcf, u8 reg, u8 val)
 {
-	int ret;
-	u8 tmp;
-
-	mutex_lock(&pcf->lock);
-	ret = __pcf50633_read(pcf, reg, 1, &tmp);
-	if (ret < 0)
-		goto out;
-
-	tmp &= ~val;
-	ret = __pcf50633_write(pcf, reg, 1, &tmp);
-
-out:
-	mutex_unlock(&pcf->lock);
-
-	return ret;
+	return regmap_update_bits(pcf->regmap, reg, val, 0);
 }
 EXPORT_SYMBOL_GPL(pcf50633_reg_clear_bits);
 
@@ -251,6 +180,11 @@ static int pcf50633_resume(struct device *dev)
 
 static SIMPLE_DEV_PM_OPS(pcf50633_pm, pcf50633_suspend, pcf50633_resume);
 
+static struct regmap_config pcf50633_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+};
+
 static int __devinit pcf50633_probe(struct i2c_client *client,
 				const struct i2c_device_id *ids)
 {
@@ -272,16 +206,23 @@ static int __devinit pcf50633_probe(struct i2c_client *client,
 
 	mutex_init(&pcf->lock);
 
+	pcf->regmap = regmap_init_i2c(client, &pcf50633_regmap_config);
+	if (IS_ERR(pcf->regmap)) {
+		ret = PTR_ERR(pcf->regmap);
+		dev_err(pcf->dev, "Failed to allocate register map: %d\n",
+			ret);
+		goto err_free;
+	}
+
 	i2c_set_clientdata(client, pcf);
 	pcf->dev = &client->dev;
-	pcf->i2c_client = client;
 
 	version = pcf50633_reg_read(pcf, 0);
 	variant = pcf50633_reg_read(pcf, 1);
 	if (version < 0 || variant < 0) {
 		dev_err(pcf->dev, "Unable to probe pcf50633\n");
 		ret = -ENODEV;
-		goto err_free;
+		goto err_regmap;
 	}
 
 	dev_info(pcf->dev, "Probed device version %d variant %d\n",
@@ -328,6 +269,8 @@ static int __devinit pcf50633_probe(struct i2c_client *client,
 
 	return 0;
 
+err_regmap:
+	regmap_exit(pcf->regmap);
 err_free:
 	kfree(pcf);
 
@@ -351,6 +294,7 @@ static int __devexit pcf50633_remove(struct i2c_client *client)
 	for (i = 0; i < PCF50633_NUM_REGULATORS; i++)
 		platform_device_unregister(pcf->regulator_pdev[i]);
 
+	regmap_exit(pcf->regmap);
 	kfree(pcf);
 
 	return 0;
diff --git a/include/linux/mfd/pcf50633/core.h b/include/linux/mfd/pcf50633/core.h
index 50d4a04..a808407 100644
--- a/include/linux/mfd/pcf50633/core.h
+++ b/include/linux/mfd/pcf50633/core.h
@@ -21,6 +21,7 @@
 #include <linux/mfd/pcf50633/backlight.h>
 
 struct pcf50633;
+struct regmap;
 
 #define PCF50633_NUM_REGULATORS	11
 
@@ -134,7 +135,7 @@ enum {
 
 struct pcf50633 {
 	struct device *dev;
-	struct i2c_client *i2c_client;
+	struct regmap *regmap;
 
 	struct pcf50633_platform_data *pdata;
 	int irq;
-- 
1.7.5.4

Re: [PATCH] mfd: Convert pcf50633 to use new register map API

[Lars-Peter Clausen]

>  #include <linux/mfd/pcf50633/core.h>
>  
[...]
>  /* Read a block of up to 32 regs  */
>  int pcf50633_read_block(struct pcf50633 *pcf, u8 reg,
>  					int nr_regs, u8 *data)
>  {
> -	int ret;
> -
> -	mutex_lock(&pcf->lock);
> -	ret = __pcf50633_read(pcf, reg, nr_regs, data);
> -	mutex_unlock(&pcf->lock);
> -
> -	return ret;
> +	return regmap_raw_read(pcf->regmap, reg, data, nr_regs);
>  }
>  EXPORT_SYMBOL_GPL(pcf50633_read_block);

There are callers which expect pcf50633_read_block to return the number of
bytes read. We could change the wrapper to return nr_regs if regmap_raw_read
returns 0. But I guess it is best to just update the callers. Incremental patch
which does this at the end of the mail.

>  
> @@ -69,23 +40,18 @@ EXPORT_SYMBOL_GPL(pcf50633_read_block);
>  int pcf50633_write_block(struct pcf50633 *pcf , u8 reg,
>  					int nr_regs, u8 *data)
>  {
> -	int ret;
> -
> -	mutex_lock(&pcf->lock);
> -	ret = __pcf50633_write(pcf, reg, nr_regs, data);
> -	mutex_unlock(&pcf->lock);
> -
> -	return ret;
> +	return regmap_raw_write(pcf->regmap, reg, data, nr_regs);
>  }
>  EXPORT_SYMBOL_GPL(pcf50633_write_block);
>  
>  u8 pcf50633_reg_read(struct pcf50633 *pcf, u8 reg)
>  {
> -	u8 val;
> +	unsigned int val;
> +	int ret;
>  
> -	mutex_lock(&pcf->lock);
> -	__pcf50633_read(pcf, reg, 1, &val);
> -	mutex_unlock(&pcf->lock);
> +	ret = regmap_read(pcf->regmap, reg, &val);
> +	if (ret < 0)
> +		return -1;
>  
>  	return val;
>  }
> @@ -93,56 +59,19 @@ EXPORT_SYMBOL_GPL(pcf50633_reg_read);
>  
>  int pcf50633_reg_write(struct pcf50633 *pcf, u8 reg, u8 val)
>  {
> -	int ret;
> -
> -	mutex_lock(&pcf->lock);
> -	ret = __pcf50633_write(pcf, reg, 1, &val);
> -	mutex_unlock(&pcf->lock);
> -
> -	return ret;
> +	return regmap_write(pcf->regmap, reg, val);
>  }
>  EXPORT_SYMBOL_GPL(pcf50633_reg_write);
>  
>  int pcf50633_reg_set_bit_mask(struct pcf50633 *pcf, u8 reg, u8 mask, u8 val)
>  {
> -	int ret;
> -	u8 tmp;
> -
> -	val &= mask;
> -
> -	mutex_lock(&pcf->lock);
> -	ret = __pcf50633_read(pcf, reg, 1, &tmp);
> -	if (ret < 0)
> -		goto out;
> -
> -	tmp &= ~mask;
> -	tmp |= val;
> -	ret = __pcf50633_write(pcf, reg, 1, &tmp);
> -
> -out:
> -	mutex_unlock(&pcf->lock);
> -
> -	return ret;
> +	return regmap_update_bits(pcf->regmap, reg, mask, val);
>  }
>  EXPORT_SYMBOL_GPL(pcf50633_reg_set_bit_mask);
>  
>  int pcf50633_reg_clear_bits(struct pcf50633 *pcf, u8 reg, u8 val)
>  {
> -	int ret;
> -	u8 tmp;
> -
> -	mutex_lock(&pcf->lock);
> -	ret = __pcf50633_read(pcf, reg, 1, &tmp);
> -	if (ret < 0)
> -		goto out;
> -
> -	tmp &= ~val;
> -	ret = __pcf50633_write(pcf, reg, 1, &tmp);
> -
> -out:
> -	mutex_unlock(&pcf->lock);
> -
> -	return ret;
> +	return regmap_update_bits(pcf->regmap, reg, val, 0);
>  }
>  EXPORT_SYMBOL_GPL(pcf50633_reg_clear_bits);

I would prefer making the wrappers static inline functions.

>  
> @@ -251,6 +180,11 @@ static int pcf50633_resume(struct device *dev)
>  
>  static SIMPLE_DEV_PM_OPS(pcf50633_pm, pcf50633_suspend, pcf50633_resume);
>  
> +static struct regmap_config pcf50633_regmap_config = {

const

> +	.reg_bits = 8,
> +	.val_bits = 8,
> +};
> +

------

diff --git a/drivers/base/regmap/regmap-i2c.c b/drivers/base/regmap/regmap-i2c.c
index c2231ff..ba107db 100644
--- a/drivers/base/regmap/regmap-i2c.c
+++ b/drivers/base/regmap/regmap-i2c.c
@@ -70,6 +70,8 @@ static int regmap_i2c_read(struct device *dev,
 	struct i2c_msg xfer[2];
 	int ret;

+	printk("regmap_read: %d %d\n", reg_size, val_size);
+
 	xfer[0].addr = i2c->addr;
 	xfer[0].flags = 0;
 	xfer[0].len = reg_size;
diff --git a/drivers/mfd/pcf50633-irq.c b/drivers/mfd/pcf50633-irq.c
index 4b8269c..edc1897 100644
--- a/drivers/mfd/pcf50633-irq.c
+++ b/drivers/mfd/pcf50633-irq.c
@@ -96,7 +96,7 @@ static irqreturn_t pcf50633_irq(int irq, void *data)
 	/* Read the 5 INT regs in one transaction */
 	ret = pcf50633_read_block(pcf, PCF50633_REG_INT1,
 						ARRAY_SIZE(pcf_int), pcf_int);
-	if (ret != ARRAY_SIZE(pcf_int)) {
+	if (ret) {
 		dev_err(pcf->dev, "Error reading INT registers\n");

 		/*
diff --git a/drivers/rtc/rtc-pcf50633.c b/drivers/rtc/rtc-pcf50633.c
index b0dc8c2..5bcc8e2 100644
--- a/drivers/rtc/rtc-pcf50633.c
+++ b/drivers/rtc/rtc-pcf50633.c
@@ -114,9 +114,9 @@ static int pcf50633_rtc_read_time(struct device *dev,
 	ret = pcf50633_read_block(rtc->pcf, PCF50633_REG_RTCSC,
 					    PCF50633_TI_EXTENT,
 					    &pcf_tm.time[0]);
-	if (ret != PCF50633_TI_EXTENT) {
+	if (ret) {
 		dev_err(dev, "Failed to read time\n");
-		return -EIO;
+		return ret;
 	}

 	dev_dbg(dev, "PCF_TIME: %02x.%02x.%02x %02x:%02x:%02x\n",
@@ -186,9 +186,9 @@ static int pcf50633_rtc_read_alarm(struct device *dev,

 	ret = pcf50633_read_block(rtc->pcf, PCF50633_REG_RTCSCA,
 				PCF50633_TI_EXTENT, &pcf_tm.time[0]);
-	if (ret != PCF50633_TI_EXTENT) {
+	if (ret) {
 		dev_err(dev, "Failed to read time\n");
-		return -EIO;
+		return ret;
 	}

 	pcf2rtc_time(&alrm->time, &pcf_tm);

Re: [PATCH] mfd: Convert pcf50633 to use new register map API

[Mark Brown]

On Mon, Aug 08, 2011 at 05:29:27AM +0200, Lars-Peter Clausen wrote:

> >  EXPORT_SYMBOL_GPL(pcf50633_reg_clear_bits);

> I would prefer making the wrappers static inline functions.

I considered doing this for all the conversions but preferred to go for
a minimal diff for ease of review, it seems better to do such code
motion changes later on.  Similarly for the return value stuff which
I'll update shortly.

> --- a/drivers/base/regmap/regmap-i2c.c
> +++ b/drivers/base/regmap/regmap-i2c.c
> @@ -70,6 +70,8 @@ static int regmap_i2c_read(struct device *dev,
>  	struct i2c_msg xfer[2];
>  	int ret;
> 
> +	printk("regmap_read: %d %d\n", reg_size, val_size);
> +

Clearly not ideal...

Re: [PATCH] mfd: Convert pcf50633 to use new register map API

[Samuel Ortiz]

Hi Lars,

On Mon, Aug 08, 2011 at 05:29:27AM +0200, Lars-Peter Clausen wrote:
> 
> >  #include <linux/mfd/pcf50633/core.h>
> >  
> [...]
> >  /* Read a block of up to 32 regs  */
> >  int pcf50633_read_block(struct pcf50633 *pcf, u8 reg,
> >  					int nr_regs, u8 *data)
> >  {
> > -	int ret;
> > -
> > -	mutex_lock(&pcf->lock);
> > -	ret = __pcf50633_read(pcf, reg, nr_regs, data);
> > -	mutex_unlock(&pcf->lock);
> > -
> > -	return ret;
> > +	return regmap_raw_read(pcf->regmap, reg, data, nr_regs);
> >  }
> >  EXPORT_SYMBOL_GPL(pcf50633_read_block);
> 
> There are callers which expect pcf50633_read_block to return the number of
> bytes read. We could change the wrapper to return nr_regs if regmap_raw_read
> returns 0. But I guess it is best to just update the callers. Incremental patch
> which does this at the end of the mail.
I'd like to apply Mark's patch and yours. Could you please remove the
unnecesary printk in your patch and I'll go ahead.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

Re: [PATCH] mfd: Convert pcf50633 to use new register map API

[Samuel Ortiz]

On Mon, Aug 22, 2011 at 11:03:13AM +0200, Samuel Ortiz wrote:
> Hi Lars,
> 
> On Mon, Aug 08, 2011 at 05:29:27AM +0200, Lars-Peter Clausen wrote:
> > 
> > >  #include <linux/mfd/pcf50633/core.h>
> > >  
> > [...]
> > >  /* Read a block of up to 32 regs  */
> > >  int pcf50633_read_block(struct pcf50633 *pcf, u8 reg,
> > >  					int nr_regs, u8 *data)
> > >  {
> > > -	int ret;
> > > -
> > > -	mutex_lock(&pcf->lock);
> > > -	ret = __pcf50633_read(pcf, reg, nr_regs, data);
> > > -	mutex_unlock(&pcf->lock);
> > > -
> > > -	return ret;
> > > +	return regmap_raw_read(pcf->regmap, reg, data, nr_regs);
> > >  }
> > >  EXPORT_SYMBOL_GPL(pcf50633_read_block);
> > 
> > There are callers which expect pcf50633_read_block to return the number of
> > bytes read. We could change the wrapper to return nr_regs if regmap_raw_read
> > returns 0. But I guess it is best to just update the callers. Incremental patch
> > which does this at the end of the mail.
> I'd like to apply Mark's patch and yours. Could you please remove the
> unnecesary printk in your patch and I'll go ahead.
Nevermind, I just saw Mark's updated patch.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

Re: [PATCH] mfd: Convert pcf50633 to use new register map API

[Samuel Ortiz]

Hi Mark,

On Mon, Aug 08, 2011 at 05:04:40PM +0900, Mark Brown wrote:
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
Patch applied, thanks.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

Re: [PATCH] mfd: Convert pcf50633 to use new register map API

[Lars-Peter Clausen]

On 08/08/2011 10:04 AM, Mark Brown wrote:
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

Tested-by: Lars-Peter Clausen <lars@metafoo.de>

> ---
>  drivers/mfd/Kconfig               |    1 +
>  drivers/mfd/pcf50633-core.c       |  114 +++++++++++-------------------------
>  include/linux/mfd/pcf50633/core.h |    3 +-
>  3 files changed, 38 insertions(+), 80 deletions(-)
>  [...]

[PATCH] mfd: Convert pcf50633 to use new register map API

[Mark Brown]

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 drivers/mfd/Kconfig               |    1 +
 drivers/mfd/pcf50633-core.c       |  114 +++++++++++-------------------------
 include/linux/mfd/pcf50633/core.h |    3 +-
 3 files changed, 38 insertions(+), 80 deletions(-)

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 21574bd..13f0040 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -500,6 +500,7 @@ config MFD_WM8994
 config MFD_PCF50633
 	tristate "Support for NXP PCF50633"
 	depends on I2C
+	select REGMAP_I2C
 	help
 	  Say yes here if you have NXP PCF50633 chip on your board.
 	  This core driver provides register access and IRQ handling
diff --git a/drivers/mfd/pcf50633-core.c b/drivers/mfd/pcf50633-core.c
index 57868416..ff1a7e7 100644
--- a/drivers/mfd/pcf50633-core.c
+++ b/drivers/mfd/pcf50633-core.c
@@ -23,45 +23,22 @@
 #include <linux/i2c.h>
 #include <linux/pm.h>
 #include <linux/slab.h>
+#include <linux/regmap.h>
+#include <linux/err.h>
 
 #include <linux/mfd/pcf50633/core.h>
 
-static int __pcf50633_read(struct pcf50633 *pcf, u8 reg, int num, u8 *data)
-{
-	int ret;
-
-	ret = i2c_smbus_read_i2c_block_data(pcf->i2c_client, reg,
-				num, data);
-	if (ret < 0)
-		dev_err(pcf->dev, "Error reading %d regs at %d\n", num, reg);
-
-	return ret;
-}
-
-static int __pcf50633_write(struct pcf50633 *pcf, u8 reg, int num, u8 *data)
-{
-	int ret;
-
-	ret = i2c_smbus_write_i2c_block_data(pcf->i2c_client, reg,
-				num, data);
-	if (ret < 0)
-		dev_err(pcf->dev, "Error writing %d regs at %d\n", num, reg);
-
-	return ret;
-
-}
-
 /* Read a block of up to 32 regs  */
 int pcf50633_read_block(struct pcf50633 *pcf, u8 reg,
 					int nr_regs, u8 *data)
 {
 	int ret;
 
-	mutex_lock(&pcf->lock);
-	ret = __pcf50633_read(pcf, reg, nr_regs, data);
-	mutex_unlock(&pcf->lock);
+	ret = regmap_raw_read(pcf->regmap, reg, data, nr_regs);
+	if (ret != 0)
+		return ret;
 
-	return ret;
+	return nr_regs;
 }
 EXPORT_SYMBOL_GPL(pcf50633_read_block);
 
@@ -71,21 +48,22 @@ int pcf50633_write_block(struct pcf50633 *pcf , u8 reg,
 {
 	int ret;
 
-	mutex_lock(&pcf->lock);
-	ret = __pcf50633_write(pcf, reg, nr_regs, data);
-	mutex_unlock(&pcf->lock);
+	ret = regmap_raw_write(pcf->regmap, reg, data, nr_regs);
+	if (ret != 0)
+		return ret;
 
-	return ret;
+	return nr_regs;
 }
 EXPORT_SYMBOL_GPL(pcf50633_write_block);
 
 u8 pcf50633_reg_read(struct pcf50633 *pcf, u8 reg)
 {
-	u8 val;
+	unsigned int val;
+	int ret;
 
-	mutex_lock(&pcf->lock);
-	__pcf50633_read(pcf, reg, 1, &val);
-	mutex_unlock(&pcf->lock);
+	ret = regmap_read(pcf->regmap, reg, &val);
+	if (ret < 0)
+		return -1;
 
 	return val;
 }
@@ -93,56 +71,19 @@ EXPORT_SYMBOL_GPL(pcf50633_reg_read);
 
 int pcf50633_reg_write(struct pcf50633 *pcf, u8 reg, u8 val)
 {
-	int ret;
-
-	mutex_lock(&pcf->lock);
-	ret = __pcf50633_write(pcf, reg, 1, &val);
-	mutex_unlock(&pcf->lock);
-
-	return ret;
+	return regmap_write(pcf->regmap, reg, val);
 }
 EXPORT_SYMBOL_GPL(pcf50633_reg_write);
 
 int pcf50633_reg_set_bit_mask(struct pcf50633 *pcf, u8 reg, u8 mask, u8 val)
 {
-	int ret;
-	u8 tmp;
-
-	val &= mask;
-
-	mutex_lock(&pcf->lock);
-	ret = __pcf50633_read(pcf, reg, 1, &tmp);
-	if (ret < 0)
-		goto out;
-
-	tmp &= ~mask;
-	tmp |= val;
-	ret = __pcf50633_write(pcf, reg, 1, &tmp);
-
-out:
-	mutex_unlock(&pcf->lock);
-
-	return ret;
+	return regmap_update_bits(pcf->regmap, reg, mask, val);
 }
 EXPORT_SYMBOL_GPL(pcf50633_reg_set_bit_mask);
 
 int pcf50633_reg_clear_bits(struct pcf50633 *pcf, u8 reg, u8 val)
 {
-	int ret;
-	u8 tmp;
-
-	mutex_lock(&pcf->lock);
-	ret = __pcf50633_read(pcf, reg, 1, &tmp);
-	if (ret < 0)
-		goto out;
-
-	tmp &= ~val;
-	ret = __pcf50633_write(pcf, reg, 1, &tmp);
-
-out:
-	mutex_unlock(&pcf->lock);
-
-	return ret;
+	return regmap_update_bits(pcf->regmap, reg, val, 0);
 }
 EXPORT_SYMBOL_GPL(pcf50633_reg_clear_bits);
 
@@ -251,6 +192,11 @@ static int pcf50633_resume(struct device *dev)
 
 static SIMPLE_DEV_PM_OPS(pcf50633_pm, pcf50633_suspend, pcf50633_resume);
 
+static struct regmap_config pcf50633_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+};
+
 static int __devinit pcf50633_probe(struct i2c_client *client,
 				const struct i2c_device_id *ids)
 {
@@ -272,16 +218,23 @@ static int __devinit pcf50633_probe(struct i2c_client *client,
 
 	mutex_init(&pcf->lock);
 
+	pcf->regmap = regmap_init_i2c(client, &pcf50633_regmap_config);
+	if (IS_ERR(pcf->regmap)) {
+		ret = PTR_ERR(pcf->regmap);
+		dev_err(pcf->dev, "Failed to allocate register map: %d\n",
+			ret);
+		goto err_free;
+	}
+
 	i2c_set_clientdata(client, pcf);
 	pcf->dev = &client->dev;
-	pcf->i2c_client = client;
 
 	version = pcf50633_reg_read(pcf, 0);
 	variant = pcf50633_reg_read(pcf, 1);
 	if (version < 0 || variant < 0) {
 		dev_err(pcf->dev, "Unable to probe pcf50633\n");
 		ret = -ENODEV;
-		goto err_free;
+		goto err_regmap;
 	}
 
 	dev_info(pcf->dev, "Probed device version %d variant %d\n",
@@ -328,6 +281,8 @@ static int __devinit pcf50633_probe(struct i2c_client *client,
 
 	return 0;
 
+err_regmap:
+	regmap_exit(pcf->regmap);
 err_free:
 	kfree(pcf);
 
@@ -351,6 +306,7 @@ static int __devexit pcf50633_remove(struct i2c_client *client)
 	for (i = 0; i < PCF50633_NUM_REGULATORS; i++)
 		platform_device_unregister(pcf->regulator_pdev[i]);
 
+	regmap_exit(pcf->regmap);
 	kfree(pcf);
 
 	return 0;
diff --git a/include/linux/mfd/pcf50633/core.h b/include/linux/mfd/pcf50633/core.h
index 50d4a04..a808407 100644
--- a/include/linux/mfd/pcf50633/core.h
+++ b/include/linux/mfd/pcf50633/core.h
@@ -21,6 +21,7 @@
 #include <linux/mfd/pcf50633/backlight.h>
 
 struct pcf50633;
+struct regmap;
 
 #define PCF50633_NUM_REGULATORS	11
 
@@ -134,7 +135,7 @@ enum {
 
 struct pcf50633 {
 	struct device *dev;
-	struct i2c_client *i2c_client;
+	struct regmap *regmap;
 
 	struct pcf50633_platform_data *pdata;
 	int irq;
-- 
1.7.5.4

Re: [PATCH] mfd: Convert pcf50633 to use new register map API

[Lars-Peter Clausen]

>  /* Read a block of up to 32 regs  */
>  int pcf50633_read_block(struct pcf50633 *pcf, u8 reg,
>  					int nr_regs, u8 *data)
>  {
> -	int ret;
> -
> -	mutex_lock(&pcf->lock);
> -	ret = __pcf50633_read(pcf, reg, nr_regs, data);
> -	mutex_unlock(&pcf->lock);
> -
> -	return ret;
> +	return regmap_raw_read(pcf->regmap, reg, data, nr_regs);
>  }
>  EXPORT_SYMBOL_GPL(pcf50633_read_block);
>  
> @@ -71,21 +42,22 @@ int pcf50633_write_block(struct pcf50633 *pcf , u8 reg,
>  {
>  	int ret;
>  
> -	mutex_lock(&pcf->lock);
> -	ret = __pcf50633_write(pcf, reg, nr_regs, data);
> -	mutex_unlock(&pcf->lock);
> +	ret = regmap_raw_write(pcf->regmap, reg, data, nr_regs);
> +	if (ret != 0)
> +		return ret;
>  
> -	return ret;
> +	return nr_regs;
>  }
>  EXPORT_SYMBOL_GPL(pcf50633_write_block);

We need to return the number of read regs in case of success for read_block
too. (Actually we only need this for read_block, but I guess it is nice to be
consistent here)

- Lars

[PATCH] mfd: Convert pcf50633 to use new register map API

[Mark Brown]

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 drivers/mfd/Kconfig               |    1 +
 drivers/mfd/pcf50633-core.c       |  114 ++++++++++--------------------------
 include/linux/mfd/pcf50633/core.h |    3 +-
 3 files changed, 35 insertions(+), 83 deletions(-)

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 21574bd..13f0040 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -500,6 +500,7 @@ config MFD_WM8994
 config MFD_PCF50633
 	tristate "Support for NXP PCF50633"
 	depends on I2C
+	select REGMAP_I2C
 	help
 	  Say yes here if you have NXP PCF50633 chip on your board.
 	  This core driver provides register access and IRQ handling
diff --git a/drivers/mfd/pcf50633-core.c b/drivers/mfd/pcf50633-core.c
index 57868416..c6720dc 100644
--- a/drivers/mfd/pcf50633-core.c
+++ b/drivers/mfd/pcf50633-core.c
@@ -23,45 +23,16 @@
 #include <linux/i2c.h>
 #include <linux/pm.h>
 #include <linux/slab.h>
+#include <linux/regmap.h>
+#include <linux/err.h>
 
 #include <linux/mfd/pcf50633/core.h>
 
-static int __pcf50633_read(struct pcf50633 *pcf, u8 reg, int num, u8 *data)
-{
-	int ret;
-
-	ret = i2c_smbus_read_i2c_block_data(pcf->i2c_client, reg,
-				num, data);
-	if (ret < 0)
-		dev_err(pcf->dev, "Error reading %d regs at %d\n", num, reg);
-
-	return ret;
-}
-
-static int __pcf50633_write(struct pcf50633 *pcf, u8 reg, int num, u8 *data)
-{
-	int ret;
-
-	ret = i2c_smbus_write_i2c_block_data(pcf->i2c_client, reg,
-				num, data);
-	if (ret < 0)
-		dev_err(pcf->dev, "Error writing %d regs at %d\n", num, reg);
-
-	return ret;
-
-}
-
 /* Read a block of up to 32 regs  */
 int pcf50633_read_block(struct pcf50633 *pcf, u8 reg,
 					int nr_regs, u8 *data)
 {
-	int ret;
-
-	mutex_lock(&pcf->lock);
-	ret = __pcf50633_read(pcf, reg, nr_regs, data);
-	mutex_unlock(&pcf->lock);
-
-	return ret;
+	return regmap_raw_read(pcf->regmap, reg, data, nr_regs);
 }
 EXPORT_SYMBOL_GPL(pcf50633_read_block);
 
@@ -71,21 +42,22 @@ int pcf50633_write_block(struct pcf50633 *pcf , u8 reg,
 {
 	int ret;
 
-	mutex_lock(&pcf->lock);
-	ret = __pcf50633_write(pcf, reg, nr_regs, data);
-	mutex_unlock(&pcf->lock);
+	ret = regmap_raw_write(pcf->regmap, reg, data, nr_regs);
+	if (ret != 0)
+		return ret;
 
-	return ret;
+	return nr_regs;
 }
 EXPORT_SYMBOL_GPL(pcf50633_write_block);
 
 u8 pcf50633_reg_read(struct pcf50633 *pcf, u8 reg)
 {
-	u8 val;
+	unsigned int val;
+	int ret;
 
-	mutex_lock(&pcf->lock);
-	__pcf50633_read(pcf, reg, 1, &val);
-	mutex_unlock(&pcf->lock);
+	ret = regmap_read(pcf->regmap, reg, &val);
+	if (ret < 0)
+		return -1;
 
 	return val;
 }
@@ -93,56 +65,19 @@ EXPORT_SYMBOL_GPL(pcf50633_reg_read);
 
 int pcf50633_reg_write(struct pcf50633 *pcf, u8 reg, u8 val)
 {
-	int ret;
-
-	mutex_lock(&pcf->lock);
-	ret = __pcf50633_write(pcf, reg, 1, &val);
-	mutex_unlock(&pcf->lock);
-
-	return ret;
+	return regmap_write(pcf->regmap, reg, val);
 }
 EXPORT_SYMBOL_GPL(pcf50633_reg_write);
 
 int pcf50633_reg_set_bit_mask(struct pcf50633 *pcf, u8 reg, u8 mask, u8 val)
 {
-	int ret;
-	u8 tmp;
-
-	val &= mask;
-
-	mutex_lock(&pcf->lock);
-	ret = __pcf50633_read(pcf, reg, 1, &tmp);
-	if (ret < 0)
-		goto out;
-
-	tmp &= ~mask;
-	tmp |= val;
-	ret = __pcf50633_write(pcf, reg, 1, &tmp);
-
-out:
-	mutex_unlock(&pcf->lock);
-
-	return ret;
+	return regmap_update_bits(pcf->regmap, reg, mask, val);
 }
 EXPORT_SYMBOL_GPL(pcf50633_reg_set_bit_mask);
 
 int pcf50633_reg_clear_bits(struct pcf50633 *pcf, u8 reg, u8 val)
 {
-	int ret;
-	u8 tmp;
-
-	mutex_lock(&pcf->lock);
-	ret = __pcf50633_read(pcf, reg, 1, &tmp);
-	if (ret < 0)
-		goto out;
-
-	tmp &= ~val;
-	ret = __pcf50633_write(pcf, reg, 1, &tmp);
-
-out:
-	mutex_unlock(&pcf->lock);
-
-	return ret;
+	return regmap_update_bits(pcf->regmap, reg, val, 0);
 }
 EXPORT_SYMBOL_GPL(pcf50633_reg_clear_bits);
 
@@ -251,6 +186,11 @@ static int pcf50633_resume(struct device *dev)
 
 static SIMPLE_DEV_PM_OPS(pcf50633_pm, pcf50633_suspend, pcf50633_resume);
 
+static struct regmap_config pcf50633_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+};
+
 static int __devinit pcf50633_probe(struct i2c_client *client,
 				const struct i2c_device_id *ids)
 {
@@ -272,16 +212,23 @@ static int __devinit pcf50633_probe(struct i2c_client *client,
 
 	mutex_init(&pcf->lock);
 
+	pcf->regmap = regmap_init_i2c(client, &pcf50633_regmap_config);
+	if (IS_ERR(pcf->regmap)) {
+		ret = PTR_ERR(pcf->regmap);
+		dev_err(pcf->dev, "Failed to allocate register map: %d\n",
+			ret);
+		goto err_free;
+	}
+
 	i2c_set_clientdata(client, pcf);
 	pcf->dev = &client->dev;
-	pcf->i2c_client = client;
 
 	version = pcf50633_reg_read(pcf, 0);
 	variant = pcf50633_reg_read(pcf, 1);
 	if (version < 0 || variant < 0) {
 		dev_err(pcf->dev, "Unable to probe pcf50633\n");
 		ret = -ENODEV;
-		goto err_free;
+		goto err_regmap;
 	}
 
 	dev_info(pcf->dev, "Probed device version %d variant %d\n",
@@ -328,6 +275,8 @@ static int __devinit pcf50633_probe(struct i2c_client *client,
 
 	return 0;
 
+err_regmap:
+	regmap_exit(pcf->regmap);
 err_free:
 	kfree(pcf);
 
@@ -351,6 +300,7 @@ static int __devexit pcf50633_remove(struct i2c_client *client)
 	for (i = 0; i < PCF50633_NUM_REGULATORS; i++)
 		platform_device_unregister(pcf->regulator_pdev[i]);
 
+	regmap_exit(pcf->regmap);
 	kfree(pcf);
 
 	return 0;
diff --git a/include/linux/mfd/pcf50633/core.h b/include/linux/mfd/pcf50633/core.h
index 50d4a04..a808407 100644
--- a/include/linux/mfd/pcf50633/core.h
+++ b/include/linux/mfd/pcf50633/core.h
@@ -21,6 +21,7 @@
 #include <linux/mfd/pcf50633/backlight.h>
 
 struct pcf50633;
+struct regmap;
 
 #define PCF50633_NUM_REGULATORS	11
 
@@ -134,7 +135,7 @@ enum {
 
 struct pcf50633 {
 	struct device *dev;
-	struct i2c_client *i2c_client;
+	struct regmap *regmap;
 
 	struct pcf50633_platform_data *pdata;
 	int irq;
-- 
1.7.5.4