mfd: Convert pm860x to use regmap api

PATCH Superseded Jett.Zhou 4 participants 4 messages Nov 10, 2011 – Dec 12, 2011 Multifunction devices (MFD)

Share & tools

Jett.Zhou

author

2011-11-10 11:43 UTC #0 lore raw

PATCH 1/1 mfd: Convert pm860x to use regmap api

Convert the register read/write code to use the register map API. We still need some pm860x specific code and locking in place to make sure test page read/write can not be interrupted by other i2c operation based on pm860x chip;

Change-Id I4d5c616d021ff62672477b9e08e118985c4d6af0

Signed-off-by Jett.Zhou <[email protected]>

---
 drivers/mfd/88pm860x-i2c.c   |  162 +++++++++++++++++++++---------------------
 drivers/mfd/Kconfig          |    1 +
 include/linux/mfd/88pm860x.h |    2 +
 3 files changed, 83 insertions(+), 82 deletions(-)

drivers/mfd/88pm860x-i2c.c +80 ~~-82~~

index e017dc8..fd3e7f7 100644
@@ -12,49 +12,21 @@
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/i2c.h>
+#include <linux/err.h>
 #include <linux/mfd/88pm860x.h>
 #include <linux/slab.h>
-
-static inline int pm860x_read_device(struct i2c_client *i2c,
-				     int reg, int bytes, void *dest)
-{
-	unsigned char data;
-	int ret;
-
-	data = (unsigned char)reg;
-	ret = i2c_master_send(i2c, &data, 1);
-	if (ret < 0)
-		return ret;
-
-	ret = i2c_master_recv(i2c, dest, bytes);
-	if (ret < 0)
-		return ret;
-	return 0;
-}
-
-static inline int pm860x_write_device(struct i2c_client *i2c,
-				      int reg, int bytes, void *src)
-{
-	unsigned char buf[bytes + 1];
-	int ret;
-
-	buf[0] = (unsigned char)reg;
-	memcpy(&buf[1], src, bytes);
-
-	ret = i2c_master_send(i2c, buf, bytes + 1);
-	if (ret < 0)
-		return ret;
-	return 0;
-}
+#include <linux/regmap.h>
 
 int pm860x_reg_read(struct i2c_client *i2c, int reg)
 {
 	struct pm860x_chip *chip = i2c_get_clientdata(i2c);
-	unsigned char data;
+	struct regmap *map = (i2c == chip->client) ? chip->regmap
+					: chip->regmap_companion;
+	unsigned int data;
 	int ret;
 
 	mutex_lock(&chip->io_lock);
-	ret = pm860x_read_device(i2c, reg, 1, &data);
+	ret = regmap_read(map, reg, &data);
 	mutex_unlock(&chip->io_lock);
 
 	if (ret < 0)

@@ -68,10 +40,12 @@ int pm860x_reg_write(struct i2c_client *i2c, int reg,
 		     unsigned char data)
 {
 	struct pm860x_chip *chip = i2c_get_clientdata(i2c);
+	struct regmap *map = (i2c == chip->client) ? chip->regmap
+					: chip->regmap_companion;
 	int ret;
 
 	mutex_lock(&chip->io_lock);
-	ret = pm860x_write_device(i2c, reg, 1, &data);
+	ret = regmap_write(map, reg, data);
 	mutex_unlock(&chip->io_lock);
 
 	return ret;

@@ -82,10 +56,12 @@ int pm860x_bulk_read(struct i2c_client *i2c, int reg,
 		     int count, unsigned char *buf)
 {
 	struct pm860x_chip *chip = i2c_get_clientdata(i2c);
+	struct regmap *map = (i2c == chip->client) ? chip->regmap
+					: chip->regmap_companion;
 	int ret;
 
 	mutex_lock(&chip->io_lock);
-	ret = pm860x_read_device(i2c, reg, count, buf);
+	ret = regmap_raw_read(map, reg, buf, count);
 	mutex_unlock(&chip->io_lock);
 
 	return ret;

@@ -96,10 +72,12 @@ int pm860x_bulk_write(struct i2c_client *i2c, int reg,
 		      int count, unsigned char *buf)
 {
 	struct pm860x_chip *chip = i2c_get_clientdata(i2c);
+	struct regmap *map = (i2c == chip->client) ? chip->regmap
+					: chip->regmap_companion;
 	int ret;
 
 	mutex_lock(&chip->io_lock);
-	ret = pm860x_write_device(i2c, reg, count, buf);
+	ret = regmap_raw_write(map, reg, buf, count);
 	mutex_unlock(&chip->io_lock);
 
 	return ret;

@@ -110,17 +88,12 @@ int pm860x_set_bits(struct i2c_client *i2c, int reg,
 		    unsigned char mask, unsigned char data)
 {
 	struct pm860x_chip *chip = i2c_get_clientdata(i2c);
-	unsigned char value;
+	struct regmap *map = (i2c == chip->client) ? chip->regmap
+					: chip->regmap_companion;
 	int ret;
 
 	mutex_lock(&chip->io_lock);
-	ret = pm860x_read_device(i2c, reg, 1, &value);
-	if (ret < 0)
-		goto out;
-	value &= ~mask;
-	value |= data;
-	ret = pm860x_write_device(i2c, reg, 1, &value);
-out:
+	ret = regmap_update_bits(map, reg, mask, data);
 	mutex_unlock(&chip->io_lock);
 	return ret;
 }

@@ -129,19 +102,21 @@ EXPORT_SYMBOL(pm860x_set_bits);
 int pm860x_page_reg_read(struct i2c_client *i2c, int reg)
 {
 	struct pm860x_chip *chip = i2c_get_clientdata(i2c);
+	struct regmap *map = (i2c == chip->client) ? chip->regmap
+					: chip->regmap_companion;
 	unsigned char zero = 0;
 	unsigned char data;
 	int ret;
 
 	mutex_lock(&chip->io_lock);
-	pm860x_write_device(i2c, 0xFA, 0, &zero);
-	pm860x_write_device(i2c, 0xFB, 0, &zero);
-	pm860x_write_device(i2c, 0xFF, 0, &zero);
-	ret = pm860x_read_device(i2c, reg, 1, &data);
+	regmap_raw_write(map, 0xFA, &zero, 0);
+	regmap_raw_write(map, 0xFB, &zero, 0);
+	regmap_raw_write(map, 0xFF, &zero, 0);
+	ret = regmap_raw_read(map, reg, &data, 1);
 	if (ret >= 0)
 		ret = (int)data;
-	pm860x_write_device(i2c, 0xFE, 0, &zero);
-	pm860x_write_device(i2c, 0xFC, 0, &zero);
+	regmap_raw_write(map, 0xFE, &zero, 0);
+	regmap_raw_write(map, 0xFC, &zero, 0);
 	mutex_unlock(&chip->io_lock);
 	return ret;
 }

@@ -151,16 +126,18 @@ int pm860x_page_reg_write(struct i2c_client *i2c, int reg,
 			  unsigned char data)
 {
 	struct pm860x_chip *chip = i2c_get_clientdata(i2c);
+	struct regmap *map = (i2c == chip->client) ? chip->regmap
+					: chip->regmap_companion;
 	unsigned char zero;
 	int ret;
 
 	mutex_lock(&chip->io_lock);
-	pm860x_write_device(i2c, 0xFA, 0, &zero);
-	pm860x_write_device(i2c, 0xFB, 0, &zero);
-	pm860x_write_device(i2c, 0xFF, 0, &zero);
-	ret = pm860x_write_device(i2c, reg, 1, &data);
-	pm860x_write_device(i2c, 0xFE, 0, &zero);
-	pm860x_write_device(i2c, 0xFC, 0, &zero);
+	regmap_raw_write(map, 0xFA, &zero, 0);
+	regmap_raw_write(map, 0xFB, &zero, 0);
+	regmap_raw_write(map, 0xFF, &zero, 0);
+	ret = regmap_raw_write(map, reg, &data, 1);
+	regmap_raw_write(map, 0xFE, &zero, 0);
+	regmap_raw_write(map, 0xFC, &zero, 0);
 	mutex_unlock(&chip->io_lock);
 	return ret;
 }

@@ -170,16 +147,18 @@ int pm860x_page_bulk_read(struct i2c_client *i2c, int reg,
 			  int count, unsigned char *buf)
 {
 	struct pm860x_chip *chip = i2c_get_clientdata(i2c);
+	struct regmap *map = (i2c == chip->client) ? chip->regmap
+					: chip->regmap_companion;
 	unsigned char zero = 0;
 	int ret;
 
 	mutex_lock(&chip->io_lock);
-	pm860x_write_device(i2c, 0xFA, 0, &zero);
-	pm860x_write_device(i2c, 0xFB, 0, &zero);
-	pm860x_write_device(i2c, 0xFF, 0, &zero);
-	ret = pm860x_read_device(i2c, reg, count, buf);
-	pm860x_write_device(i2c, 0xFE, 0, &zero);
-	pm860x_write_device(i2c, 0xFC, 0, &zero);
+	regmap_raw_write(map, 0xFA, &zero, 0);
+	regmap_raw_write(map, 0xFB, &zero, 0);
+	regmap_raw_write(map, 0xFF, &zero, 0);
+	ret = regmap_raw_read(map, reg, buf, count);
+	regmap_raw_write(map, 0xFE, &zero, 0);
+	regmap_raw_write(map, 0xFC, &zero, 0);
 	mutex_unlock(&chip->io_lock);
 	return ret;
 }

@@ -189,16 +168,18 @@ int pm860x_page_bulk_write(struct i2c_client *i2c, int reg,
 			   int count, unsigned char *buf)
 {
 	struct pm860x_chip *chip = i2c_get_clientdata(i2c);
+	struct regmap *map = (i2c == chip->client) ? chip->regmap
+					: chip->regmap_companion;
 	unsigned char zero = 0;
 	int ret;
 
 	mutex_lock(&chip->io_lock);
-	pm860x_write_device(i2c, 0xFA, 0, &zero);
-	pm860x_write_device(i2c, 0xFB, 0, &zero);
-	pm860x_write_device(i2c, 0xFF, 0, &zero);
-	ret = pm860x_write_device(i2c, reg, count, buf);
-	pm860x_write_device(i2c, 0xFE, 0, &zero);
-	pm860x_write_device(i2c, 0xFC, 0, &zero);
+	regmap_raw_write(map, 0xFA, &zero, 0);
+	regmap_raw_write(map, 0xFB, &zero, 0);
+	regmap_raw_write(map, 0xFF, &zero, 0);
+	ret = regmap_raw_write(map, reg, buf, count);
+	regmap_raw_write(map, 0xFE, &zero, 0);
+	regmap_raw_write(map, 0xFC, &zero, 0);
 	mutex_unlock(&chip->io_lock);
 	return ret;
 }

@@ -208,23 +189,18 @@ int pm860x_page_set_bits(struct i2c_client *i2c, int reg,
 			 unsigned char mask, unsigned char data)
 {
 	struct pm860x_chip *chip = i2c_get_clientdata(i2c);
+	struct regmap *map = (i2c == chip->client) ? chip->regmap
+					: chip->regmap_companion;
 	unsigned char zero;
-	unsigned char value;
 	int ret;
 
 	mutex_lock(&chip->io_lock);
-	pm860x_write_device(i2c, 0xFA, 0, &zero);
-	pm860x_write_device(i2c, 0xFB, 0, &zero);
-	pm860x_write_device(i2c, 0xFF, 0, &zero);
-	ret = pm860x_read_device(i2c, reg, 1, &value);
-	if (ret < 0)
-		goto out;
-	value &= ~mask;
-	value |= data;
-	ret = pm860x_write_device(i2c, reg, 1, &value);
-out:
-	pm860x_write_device(i2c, 0xFE, 0, &zero);
-	pm860x_write_device(i2c, 0xFC, 0, &zero);
+	regmap_raw_write(map, 0xFA, &zero, 0);
+	regmap_raw_write(map, 0xFB, &zero, 0);
+	regmap_raw_write(map, 0xFF, &zero, 0);
+	ret = regmap_update_bits(map, reg, mask, data);
+	regmap_raw_write(map, 0xFE, &zero, 0);
+	regmap_raw_write(map, 0xFC, &zero, 0);
 	mutex_unlock(&chip->io_lock);
 	return ret;
 }

@@ -257,11 +233,17 @@ static int verify_addr(struct i2c_client *i2c)
 	return 0;
 }
 
+static struct regmap_config pm860x_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+};
+
 static int __devinit pm860x_probe(struct i2c_client *client,
 				  const struct i2c_device_id *id)
 {
 	struct pm860x_platform_data *pdata = client->dev.platform_data;
 	struct pm860x_chip *chip;
+	int ret;
 
 	if (!pdata) {
 		pr_info("No platform data in %s!\n", __func__);

@@ -273,6 +255,14 @@ static int __devinit pm860x_probe(struct i2c_client *client,
 		return -ENOMEM;
 
 	chip->id = verify_addr(client);
+
+	chip->regmap = regmap_init_i2c(client, &pm860x_regmap_config);
+	if (IS_ERR(chip->regmap)) {
+		ret = PTR_ERR(chip->regmap);
+		dev_err(&client->dev, "Failed to allocate register map: %d\n",
+				ret);
+		return ret;
+	}
 	chip->client = client;
 	i2c_set_clientdata(client, chip);
 	chip->dev = &client->dev;

@@ -290,6 +280,14 @@ static int __devinit pm860x_probe(struct i2c_client *client,
 		chip->companion_addr = pdata->companion_addr;
 		chip->companion = i2c_new_dummy(chip->client->adapter,
 						chip->companion_addr);
+		chip->regmap_companion = regmap_init_i2c(chip->companion,
+							&pm860x_regmap_config);
+		if (IS_ERR(chip->regmap_companion)) {
+			ret = PTR_ERR(chip->regmap_companion);
+			dev_err(&chip->companion->dev,
+				"Failed to allocate register map: %d\n", ret);
+			return ret;
+		}
 		i2c_set_clientdata(chip->companion, chip);
 	}

drivers/mfd/Kconfig +1 -0

index a67adcb..1973b23 100644
@@ -27,6 +27,7 @@ config MFD_CORE
 config MFD_88PM860X
 	bool "Support Marvell 88PM8606/88PM8607"
 	depends on I2C=y && GENERIC_HARDIRQS
+	select REGMAP_I2C
 	select MFD_CORE
 	help
 	  This supports for Marvell 88PM8606/88PM8607 Power Management IC.

include/linux/mfd/88pm860x.h +2 -0

index 63b4fb8..f860547 100644
@@ -301,6 +301,8 @@ struct pm860x_chip {
 	struct mutex		irq_lock;
 	struct i2c_client	*client;
 	struct i2c_client	*companion;	/* companion chip client */
+	struct regmap           *regmap;
+	struct regmap           *regmap_companion;
 
 	int			buck3_double;	/* DVC ramp slope double */
 	unsigned short		companion_addr;

Signature · 1 line

1.7.0.4

Mark Brown

2011-11-10 13:02 UTC #1 lore raw

PATCH 1/1 mfd: Convert pm860x to use regmap api

Quoted 4 lines · Jett.Zhou

Convert the register read/write code to use the register map API. We still need some pm860x specific code and locking in place to make sure test page read/write can not be interrupted by other i2c operation based on pm860x chip;

Reviewed-by Mark Brown <[email protected]>

I'm assuming that the interruptions come from the fact that there's two I2C clients?

Quoted 1 line

Change-Id: I4d5c616d021ff62672477b9e08e118985c4d6af0

Don't include these in kernel submissions, they're only meaningful within your private Gerrit install.

Haojian Zhuang

2011-11-11 02:08 UTC #2 lore raw

PATCH 1/1 mfd: Convert pm860x to use regmap api

Quoted 11 lines · Mark Brown

Quoted 4 lines · Jett.Zhou
Convert the register read/write code to use the register map API. We still need some pm860x specific code and locking in place to make sure test page read/write can not be interrupted by other i2c operation based on pm860x chip;
Reviewed-by Mark Brown <[email protected]>
I'm assuming that the interruptions come from the fact that there's two I2C clients?

The patch is still invalid. It can't prevent pm860x i2c operation be interrupted by other clients.

There're two banks in 88pm8607. One is the normal bank, and the other one is the test bank. Accessing the test bank need a special I2C sequence.

Touching to 0xFA address
Touching to 0xFB address
Touching to 0xFF address
Accessing bank register
Touching to 0xFE address
Touching 0xFC address

This sequence can't be interrupted. It means that we can't use i2c_master_send() to implement touching 0xFA address. Otherwise, other i2c operation may be inserted into 0xFA and 0xFB operation since the lock of i2c_adapter is already released.

So you need to fix this issue for accessing test bank first. Then you can replace i2c read/write implementation with regmap for only normal bank in 88pm8607.

Thanks
Haojian

Samuel Ortiz

2011-12-12 11:48 UTC #3 lore raw

PATCH 1/1 mfd: Convert pm860x to use regmap api

Quoted 28 lines · Haojian Zhuang

Quoted 11 lines · Mark Brown
Quoted 4 lines · Jett.Zhou
Convert the register read/write code to use the register map API. We still need some pm860x specific code and locking in place to make sure test page read/write can not be interrupted by other i2c operation based on pm860x chip;
Reviewed-by Mark Brown <[email protected]>
I'm assuming that the interruptions come from the fact that there's two I2C clients?
The patch is still invalid. It can't prevent pm860x i2c operation be interrupted by other clients.
There're two banks in 88pm8607. One is the normal bank, and the other one is the test bank. Accessing the test bank need a special I2C sequence.
Touching to 0xFA address
Touching to 0xFB address
Touching to 0xFF address
Accessing bank register
Touching to 0xFE address
Touching 0xFC address

Quoted 8 lines

This sequence can't be interrupted. It means that we can't use i2c_master_send() to implement touching 0xFA address. Otherwise, other i2c operation may be inserted into 0xFA and 0xFB operation since the lock of i2c_adapter is already released.
So you need to fix this issue for accessing test bank first. Then you can replace i2c read/write implementation with regmap for only normal bank in 88pm8607.

Jett, could you please look at that ?

Cheers,
Samuel.

Signature · 2 lines

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