mfd: 88pm860x: support rtc

PATCH In review Haojian Zhuang 4 participants 17 messages May 6, 2011 – May 14, 2011 1 message not in the archive

Subsystems (3)

Multifunction devices (MFD) Voltage and current regulator framework Real time clock (RTC)

lore

Share & tools

lore

Haojian Zhuang

author

2011-05-06 09:29 UTC #1 lore raw

PATCH 1/6 mfd: 88pm860x: support rtc

In reply to a message not in the archive

Enable rtc function in 88pm860x PMIC.

Signed-off-by Haojian Zhuang <[email protected]>

Cc Samuel Ortiz <[email protected]>

Cc Alessandro Zummo <[email protected]>

---
 drivers/mfd/88pm860x-core.c  |   29 +++
 drivers/rtc/Kconfig          |   10 +
 drivers/rtc/Makefile         |    1 +
 drivers/rtc/rtc-88pm860x.c   |  427 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/88pm860x.h |    6 +
 5 files changed, 473 insertions(+), 0 deletions(-)
 create mode 100644 drivers/rtc/rtc-88pm860x.c

drivers/mfd/88pm860x-core.c +29 -0

index 7ba4aaf..f2cac92 100644
@@ -90,6 +90,10 @@ static struct resource charger_resources[] __devinitdata = {
 	{PM8607_IRQ_VCHG, PM8607_IRQ_VCHG, "vchg voltage",    IORESOURCE_IRQ,},
 };
 
+static struct resource rtc_resources[] __devinitdata = {
+	{PM8607_IRQ_RTC, PM8607_IRQ_RTC, "rtc", IORESOURCE_IRQ,},
+};
+
 static struct mfd_cell bk_devs[] = {
 	{"88pm860x-backlight", 0,},
 	{"88pm860x-backlight", 1,},

@@ -143,6 +147,10 @@ static struct mfd_cell power_devs[] = {
 	{"88pm860x-charger", -1,},
 };
 
+static struct mfd_cell rtc_devs[] = {
+	{"88pm860x-rtc", -1,},
+};
+
 static struct pm860x_backlight_pdata bk_pdata[ARRAY_SIZE(bk_devs)];
 static struct pm860x_led_pdata led_pdata[ARRAY_SIZE(led_devs)];
 static struct regulator_init_data regulator_pdata[ARRAY_SIZE(regulator_devs)];

@@ -635,6 +643,26 @@ out:
 	return;
 }
 
+static void __devinit device_rtc_init(struct pm860x_chip *chip,
+				      struct i2c_client *i2c,
+				      struct pm860x_platform_data *pdata)
+{
+	int ret;
+
+	if ((pdata == NULL))
+		return;
+
+	rtc_devs[0].platform_data = pdata->rtc;
+	rtc_devs[0].pdata_size = sizeof(struct pm860x_rtc_pdata);
+	rtc_devs[0].num_resources = ARRAY_SIZE(rtc_resources);
+	rtc_devs[0].resources = &rtc_resources[0];
+	ret = mfd_add_devices(chip->dev, 0, &rtc_devs[0],
+			      ARRAY_SIZE(rtc_devs), &rtc_resources[0],
+			      chip->irq_base);
+	if (ret < 0)
+		dev_err(chip->dev, "Failed to add rtc subdev\n");
+}
+
 static void __devinit device_touch_init(struct pm860x_chip *chip,
 					struct i2c_client *i2c,
 					struct pm860x_platform_data *pdata)

@@ -770,6 +798,7 @@ static void __devinit device_8607_init(struct pm860x_chip *chip,
 		goto out;
 
 	device_regulator_init(chip, i2c, pdata);
+	device_rtc_init(chip, i2c, pdata);
 	device_onkey_init(chip, i2c, pdata);
 	device_touch_init(chip, i2c, pdata);
 	device_power_init(chip, i2c, pdata);

drivers/rtc/Kconfig +10 -0

index e187887..286f51e 100644
@@ -128,6 +128,16 @@ comment "I2C RTC drivers"
 
 if I2C
 
+config RTC_DRV_88PM860X
+	tristate "Marvell 88PM860x"
+	depends on RTC_CLASS && I2C && MFD_88PM860X
+	help
+	  If you say yes here you get support for RTC function in Marvell
+	  88PM860x chips.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called rtc-88pm860x.
+
 config RTC_DRV_DS1307
 	tristate "Dallas/Maxim DS1307/37/38/39/40, ST M41T00, EPSON RX-8025"
 	help

drivers/rtc/Makefile +1 -0

index ca91c3c..e3d6fb0 100644
@@ -15,6 +15,7 @@ rtc-core-$(CONFIG_RTC_INTF_SYSFS) += rtc-sysfs.o
 
 # Keep the list ordered.
 
+obj-$(CONFIG_RTC_DRV_88PM860X)  += rtc-88pm860x.o
 obj-$(CONFIG_RTC_DRV_AB3100)	+= rtc-ab3100.o
 obj-$(CONFIG_RTC_DRV_AB8500)	+= rtc-ab8500.o
 obj-$(CONFIG_RTC_DRV_AT32AP700X)+= rtc-at32ap700x.o

drivers/rtc/rtc-88pm860x.c new file +427 -0

427 lines not on this page · raw message

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

index 8fba797..63b4fb8 100644
@@ -330,6 +330,11 @@ struct pm860x_led_pdata {
 	unsigned long	flags;
 };
 
+struct pm860x_rtc_pdata {
+	int		(*sync)(unsigned int ticks);
+	int		vrtc;
+};
+
 struct pm860x_touch_pdata {
 	int		gpadc_prebias;
 	int		slot_cycle;

@@ -349,6 +354,7 @@ struct pm860x_power_pdata {
 struct pm860x_platform_data {
 	struct pm860x_backlight_pdata	*backlight;
 	struct pm860x_led_pdata		*led;
+	struct pm860x_rtc_pdata		*rtc;
 	struct pm860x_touch_pdata	*touch;
 	struct pm860x_power_pdata	*power;
 	struct regulator_init_data	*regulator;

Signature · 1 line

1.5.6.5

Haojian Zhuang

author

2011-05-06 09:29 UTC #2 lore raw

PATCH 2/6 mfd: 88pm860x: avoid to allocate static platform data

Maybe multiple PMIC devices are installed into one board. Static variable can only make driver logic mess. So remove these variable, and use platform data from platform driver instead.

Signed-off-by Haojian Zhuang <[email protected]>

Cc Samuel Ortiz <[email protected]>

---
 drivers/mfd/88pm860x-core.c |   45 +++++++++++++++---------------------------
 1 files changed, 16 insertions(+), 29 deletions(-)

drivers/mfd/88pm860x-core.c +16 ~~-29~~

index f2cac92..04ab50c 100644
@@ -151,11 +151,6 @@ static struct mfd_cell rtc_devs[] = {
 	{"88pm860x-rtc", -1,},
 };
 
-static struct pm860x_backlight_pdata bk_pdata[ARRAY_SIZE(bk_devs)];
-static struct pm860x_led_pdata led_pdata[ARRAY_SIZE(led_devs)];
-static struct regulator_init_data regulator_pdata[ARRAY_SIZE(regulator_devs)];
-static struct pm860x_touch_pdata touch_pdata;
-static struct pm860x_power_pdata power_pdata;
 
 struct pm860x_irq_data {
 	int	reg;

@@ -522,14 +517,12 @@ static void __devinit device_bk_init(struct pm860x_chip *chip,
 		pdata->num_backlights = ARRAY_SIZE(bk_devs);
 
 	for (i = 0; i < pdata->num_backlights; i++) {
-		memcpy(&bk_pdata[i], &pdata->backlight[i],
-			sizeof(struct pm860x_backlight_pdata));
-		bk_devs[i].platform_data = &bk_pdata[i];
-		bk_devs[i].pdata_size = sizeof(bk_pdata[i]);
+		bk_devs[i].platform_data = &pdata->backlight[i];
+		bk_devs[i].pdata_size = sizeof(struct pm860x_backlight_pdata);
 
 		for (j = 0; j < ARRAY_SIZE(bk_devs); j++) {
 			id = bk_resources[j].start;
-			if (bk_pdata[i].flags != id)
+			if (pdata->backlight[i].flags != id)
 				continue;
 
 			bk_devs[i].num_resources = 1;

@@ -560,14 +553,12 @@ static void __devinit device_led_init(struct pm860x_chip *chip,
 		pdata->num_leds = ARRAY_SIZE(led_devs);
 
 	for (i = 0; i < pdata->num_leds; i++) {
-		memcpy(&led_pdata[i], &pdata->led[i],
-			sizeof(struct pm860x_led_pdata));
-		led_devs[i].platform_data = &led_pdata[i];
-		led_devs[i].pdata_size = sizeof(led_pdata[i]);
+		led_devs[i].platform_data = &pdata->led[i];
+		led_devs[i].pdata_size = sizeof(struct pm860x_led_pdata);
 
 		for (j = 0; j < ARRAY_SIZE(led_devs); j++) {
 			id = led_resources[j].start;
-			if (led_pdata[i].flags != id)
+			if (pdata->led[i].flags != id)
 				continue;
 
 			led_devs[i].num_resources = 1;

@@ -625,10 +616,8 @@ static void __devinit device_regulator_init(struct pm860x_chip *chip,
 				initdata->constraints.name);
 			goto out;
 		}
-		memcpy(&regulator_pdata[i], &pdata->regulator[i],
-			sizeof(struct regulator_init_data));
-		regulator_devs[i].platform_data = &regulator_pdata[i];
-		regulator_devs[i].pdata_size = sizeof(regulator_pdata[i]);
+		regulator_devs[i].platform_data = &pdata->regulator[i];
+		regulator_devs[i].pdata_size = sizeof(struct regulator_init_data);
 		regulator_devs[i].num_resources = 1;
 		regulator_devs[i].resources = &regulator_resources[j];

@@ -669,12 +658,11 @@ static void __devinit device_touch_init(struct pm860x_chip *chip,
 {
 	int ret;
 
-	if ((pdata == NULL) || (pdata->touch == NULL))
+	if (pdata == NULL)
 		return;
 
-	memcpy(&touch_pdata, pdata->touch, sizeof(struct pm860x_touch_pdata));
-	touch_devs[0].platform_data = &touch_pdata;
-	touch_devs[0].pdata_size = sizeof(touch_pdata);
+	touch_devs[0].platform_data = pdata->touch;
+	touch_devs[0].pdata_size = sizeof(struct pm860x_touch_pdata);
 	touch_devs[0].num_resources = ARRAY_SIZE(touch_resources);
 	touch_devs[0].resources = &touch_resources[0];
 	ret = mfd_add_devices(chip->dev, 0, &touch_devs[0],

@@ -690,12 +678,11 @@ static void __devinit device_power_init(struct pm860x_chip *chip,
 {
 	int ret;
 
-	if ((pdata == NULL) || (pdata->power == NULL))
+	if (pdata == NULL)
 		return;
 
-	memcpy(&power_pdata, pdata->power, sizeof(struct pm860x_power_pdata));
-	power_devs[0].platform_data = &power_pdata;
-	power_devs[0].pdata_size = sizeof(power_pdata);
+	power_devs[0].platform_data = pdata->power;
+	power_devs[0].pdata_size = sizeof(struct pm860x_power_pdata);
 	power_devs[0].num_resources = ARRAY_SIZE(battery_resources);
 	power_devs[0].resources = &battery_resources[0],
 	ret = mfd_add_devices(chip->dev, 0, &power_devs[0], 1,

@@ -703,8 +690,8 @@ static void __devinit device_power_init(struct pm860x_chip *chip,
 	if (ret < 0)
 		dev_err(chip->dev, "Failed to add battery subdev\n");
 
-	power_devs[1].platform_data = &power_pdata;
-	power_devs[0].pdata_size = sizeof(power_pdata);
+	power_devs[1].platform_data = pdata->power;
+	power_devs[1].pdata_size = sizeof(struct pm860x_power_pdata);
 	power_devs[1].num_resources = ARRAY_SIZE(charger_resources);
 	power_devs[1].resources = &charger_resources[0],
 	ret = mfd_add_devices(chip->dev, 0, &power_devs[1], 1,

Signature · 1 line

1.5.6.5

Haojian Zhuang

author

2011-05-06 09:29 UTC #4 lore raw

PATCH 4/6 mfd: 88pm860x: avoid to use constraint name in regulator driver

In reply to Haojian Zhuang

Avoid to use constraint name in regulator driver. So use regulator id is used instead in platform driver.

Signed-off-by Haojian Zhuang <[email protected]>

Cc Samuel Ortiz <[email protected]>

Cc Liam Girdwood <[email protected]>

Cc Mark Brown <[email protected]>

---
 drivers/mfd/88pm860x-core.c  |   35 ++++++++---------------------------
 drivers/regulator/88pm8607.c |   25 +++++++++++++------------
 2 files changed, 21 insertions(+), 39 deletions(-)

drivers/mfd/88pm860x-core.c +8 ~~-27~~

index 04ab50c..8e88fba 100644
@@ -581,7 +581,7 @@ static void __devinit device_regulator_init(struct pm860x_chip *chip,
 {
 	struct regulator_init_data *initdata;
 	int ret;
-	int i, j;
+	int i, seq;
 
 	if ((pdata == NULL) || (pdata->regulator == NULL))
 		return;

@@ -589,40 +589,21 @@ static void __devinit device_regulator_init(struct pm860x_chip *chip,
 	if (pdata->num_regulators > ARRAY_SIZE(regulator_devs))
 		pdata->num_regulators = ARRAY_SIZE(regulator_devs);
 
-	for (i = 0, j = -1; i < pdata->num_regulators; i++) {
+	for (i = 0, seq = -1; i < pdata->num_regulators; i++) {
 		initdata = &pdata->regulator[i];
-		if (strstr(initdata->constraints.name, "BUCK")) {
-			sscanf(initdata->constraints.name, "BUCK%d", &j);
-			/* BUCK1 ~ BUCK3 */
-			if ((j < 1) || (j > 3)) {
-				dev_err(chip->dev, "Failed to add constraint "
-					"(%s)\n", initdata->constraints.name);
-				goto out;
-			}
-			j = (j - 1) + PM8607_ID_BUCK1;
-		}
-		if (strstr(initdata->constraints.name, "LDO")) {
-			sscanf(initdata->constraints.name, "LDO%d", &j);
-			/* LDO1 ~ LDO15 */
-			if ((j < 1) || (j > 15)) {
-				dev_err(chip->dev, "Failed to add constraint "
-					"(%s)\n", initdata->constraints.name);
-				goto out;
-			}
-			j = (j - 1) + PM8607_ID_LDO1;
-		}
-		if (j == -1) {
-			dev_err(chip->dev, "Failed to add constraint (%s)\n",
-				initdata->constraints.name);
+		seq = *(unsigned int *)initdata->driver_data;
+		if ((seq < 0) || (seq > PM8607_ID_RG_MAX)) {
+			dev_err(chip->dev, "Wrong ID(%d) on regulator(%s)\n",
+				seq, initdata->constraints.name);
 			goto out;
 		}
 		regulator_devs[i].platform_data = &pdata->regulator[i];
 		regulator_devs[i].pdata_size = sizeof(struct regulator_init_data);
 		regulator_devs[i].num_resources = 1;
-		regulator_devs[i].resources = &regulator_resources[j];
+		regulator_devs[i].resources = &regulator_resources[seq];
 
 		ret = mfd_add_devices(chip->dev, 0, &regulator_devs[i], 1,
-				      &regulator_resources[j], 0);
+				      &regulator_resources[seq], 0);
 		if (ret < 0) {
 			dev_err(chip->dev, "Failed to add regulator subdev\n");
 			goto out;

drivers/regulator/88pm8607.c +13 ~~-12~~

index 784ea77..1904be9 100644
@@ -398,32 +398,33 @@ static int __devinit pm8607_regulator_probe(struct platform_device *pdev)
 {
 	struct pm860x_chip *chip = dev_get_drvdata(pdev->dev.parent);
 	struct pm8607_regulator_info *info = NULL;
-	struct regulator_init_data *pdata;
+	struct regulator_init_data *pdata = pdev->dev.platform_data;
+	struct resource *res;
 	int i;
 
-	pdata = pdev->dev.platform_data;
-	if (pdata == NULL)
+	res = platform_get_resource(pdev, IORESOURCE_IO, 0);
+	if (res == NULL) {
+		dev_err(&pdev->dev, "No I/O resource!\n");
 		return -EINVAL;
-
+	}
 	for (i = 0; i < ARRAY_SIZE(pm8607_regulator_info); i++) {
 		info = &pm8607_regulator_info[i];
-		if (!strcmp(info->desc.name, pdata->constraints.name))
+		if (info->desc.id == res->start)
 			break;
 	}
-	if (i > ARRAY_SIZE(pm8607_regulator_info)) {
-		dev_err(&pdev->dev, "Failed to find regulator %s\n",
-			pdata->constraints.name);
+	if ((i < 0) || (i > PM8607_ID_RG_MAX)) {
+		dev_err(&pdev->dev, "Failed to find regulator %d\n",
+			res->start);
 		return -EINVAL;
 	}
-
 	info->i2c = (chip->id == CHIP_PM8607) ? chip->client : chip->companion;
 	info->chip = chip;
 
 	/* check DVC ramp slope double */
-	if (!strcmp(info->desc.name, "BUCK3"))
-		if (info->chip->buck3_double)
-			info->slope_double = 1;
+	if ((i == PM8607_ID_BUCK3) && info->chip->buck3_double)
+		info->slope_double = 1;
 
+	/* replace driver_data with info */
 	info->regulator = regulator_register(&info->desc, &pdev->dev,
 					     pdata, info);
 	if (IS_ERR(info->regulator)) {

Signature · 1 line

1.5.6.5

Haojian Zhuang

author

2011-05-06 09:29 UTC #6 lore raw

PATCH 6/6 mfd: max8925: remove checking on regulator[0]

In reply to Haojian Zhuang

Since regulator[0] is always checking in mfd driver, it results in registration failure without regulator[0].

Signed-off-by Haojian Zhuang <[email protected]>

Cc Samuel Ortiz <[email protected]>

---
 drivers/mfd/max8925-core.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

drivers/mfd/max8925-core.c +1 -1

index 58cc5fd..e1e59c9 100644
@@ -627,7 +627,7 @@ int __devinit max8925_device_init(struct max8925_chip *chip,
 		goto out_dev;
 	}
 
-	if (pdata && pdata->regulator[0]) {
+	if (pdata) {
 		ret = mfd_add_devices(chip->dev, 0, &regulator_devs[0],
 				      ARRAY_SIZE(regulator_devs),
 				      &regulator_resources[0], 0);

Signature · 1 line

1.5.6.5

Haojian Zhuang

author

2011-05-06 09:29 UTC #5 lore raw

PATCH 5/6 mfd: 88pm860x: remove unused parameter

In reply to Haojian Zhuang

i2c_client parameter isn't used in some functions. Just remove it.

Signed-off-by Haojian Zhuang <[email protected]>

Cc Samuel Ortiz <[email protected]>

---
 drivers/mfd/88pm860x-core.c |   28 ++++++++++------------------
 1 files changed, 10 insertions(+), 18 deletions(-)

drivers/mfd/88pm860x-core.c +10 ~~-18~~

index 8e88fba..17dfe9b 100644
@@ -504,7 +504,6 @@ static void device_irq_exit(struct pm860x_chip *chip)
 }
 
 static void __devinit device_bk_init(struct pm860x_chip *chip,
-				     struct i2c_client *i2c,
 				     struct pm860x_platform_data *pdata)
 {
 	int ret;

@@ -540,7 +539,6 @@ static void __devinit device_bk_init(struct pm860x_chip *chip,
 }
 
 static void __devinit device_led_init(struct pm860x_chip *chip,
-				      struct i2c_client *i2c,
 				      struct pm860x_platform_data *pdata)
 {
 	int ret;

@@ -576,7 +574,6 @@ static void __devinit device_led_init(struct pm860x_chip *chip,
 }
 
 static void __devinit device_regulator_init(struct pm860x_chip *chip,
-					    struct i2c_client *i2c,
 					    struct pm860x_platform_data *pdata)
 {
 	struct regulator_init_data *initdata;

@@ -614,7 +611,6 @@ out:
 }
 
 static void __devinit device_rtc_init(struct pm860x_chip *chip,
-				      struct i2c_client *i2c,
 				      struct pm860x_platform_data *pdata)
 {
 	int ret;

@@ -634,7 +630,6 @@ static void __devinit device_rtc_init(struct pm860x_chip *chip,
 }
 
 static void __devinit device_touch_init(struct pm860x_chip *chip,
-					struct i2c_client *i2c,
 					struct pm860x_platform_data *pdata)
 {
 	int ret;

@@ -654,7 +649,6 @@ static void __devinit device_touch_init(struct pm860x_chip *chip,
 }
 
 static void __devinit device_power_init(struct pm860x_chip *chip,
-					struct i2c_client *i2c,
 					struct pm860x_platform_data *pdata)
 {
 	int ret;

@@ -682,7 +676,6 @@ static void __devinit device_power_init(struct pm860x_chip *chip,
 }
 
 static void __devinit device_onkey_init(struct pm860x_chip *chip,
-					struct i2c_client *i2c,
 					struct pm860x_platform_data *pdata)
 {
 	int ret;

@@ -697,7 +690,6 @@ static void __devinit device_onkey_init(struct pm860x_chip *chip,
 }
 
 static void __devinit device_codec_init(struct pm860x_chip *chip,
-					struct i2c_client *i2c,
 					struct pm860x_platform_data *pdata)
 {
 	int ret;

@@ -765,12 +757,12 @@ static void __devinit device_8607_init(struct pm860x_chip *chip,
 	if (ret < 0)
 		goto out;
 
-	device_regulator_init(chip, i2c, pdata);
-	device_rtc_init(chip, i2c, pdata);
-	device_onkey_init(chip, i2c, pdata);
-	device_touch_init(chip, i2c, pdata);
-	device_power_init(chip, i2c, pdata);
-	device_codec_init(chip, i2c, pdata);
+	device_regulator_init(chip, pdata);
+	device_rtc_init(chip, pdata);
+	device_onkey_init(chip, pdata);
+	device_touch_init(chip, pdata);
+	device_power_init(chip, pdata);
+	device_codec_init(chip, pdata);
 out:
 	return;
 }

@@ -782,8 +774,8 @@ int __devinit pm860x_device_init(struct pm860x_chip *chip,
 
 	switch (chip->id) {
 	case CHIP_PM8606:
-		device_bk_init(chip, chip->client, pdata);
-		device_led_init(chip, chip->client, pdata);
+		device_bk_init(chip, pdata);
+		device_led_init(chip, pdata);
 		break;
 	case CHIP_PM8607:
 		device_8607_init(chip, chip->client, pdata);

@@ -793,8 +785,8 @@ int __devinit pm860x_device_init(struct pm860x_chip *chip,
 	if (chip->companion) {
 		switch (chip->id) {
 		case CHIP_PM8607:
-			device_bk_init(chip, chip->companion, pdata);
-			device_led_init(chip, chip->companion, pdata);
+			device_bk_init(chip, pdata);
+			device_led_init(chip, pdata);
 			break;
 		case CHIP_PM8606:
 			device_8607_init(chip, chip->companion, pdata);

Signature · 1 line

1.5.6.5

Haojian Zhuang

author

2011-05-06 09:30 UTC #3 lore raw

PATCH 3/6 mfd: 88pm860x: enhance lock on i2c transaction

In reply to Haojian Zhuang

Accessing test page in 88pm860x is a sequence of read/write on i2c bus. Bus lock is used in each small i2c transaction. But it may result the whole sequence interrupted by other i2c client transaction.

Use i2c_lock_adapter()/i2c_unlock_adapter() to protect the sequence and use master_xfer() to send i2c message.

Signed-off-by Haojian Zhuang <[email protected]>

Cc Samuel Ortiz <[email protected]>

Cc Jean Delvare <[email protected]>

Cc Ben Dooks <[email protected]>

---
 drivers/mfd/88pm860x-i2c.c |  112 +++++++++++++++++++++++++++++++------------
 1 files changed, 81 insertions(+), 31 deletions(-)

drivers/mfd/88pm860x-i2c.c +81 ~~-31~~

index e017dc8..67a34f4 100644
@@ -126,6 +126,51 @@ out:
 }
 EXPORT_SYMBOL(pm860x_set_bits);
 
+static int read_device(struct i2c_client *i2c, int reg,
+		       int bytes, void *dest)
+{
+	unsigned char msgbuf0[I2C_SMBUS_BLOCK_MAX + 3];
+	unsigned char msgbuf1[I2C_SMBUS_BLOCK_MAX + 2];
+	struct i2c_adapter *adap = i2c->adapter;
+	struct i2c_msg msg[2] = {{i2c->addr, i2c->flags, 1, msgbuf0},
+				 {i2c->addr, i2c->flags | I2C_M_RD, 0, msgbuf1},
+				};
+	int num = 1, ret = 0;
+
+	if (dest == NULL)
+		return -EINVAL;
+	msgbuf0[0] = (unsigned char)reg;	/* command */
+	msg[1].len = bytes;
+
+	/* if data needs to read back, num should be 2 */
+	if (bytes > 0)
+		num = 2;
+	ret = adap->algo->master_xfer(adap, msg, num);
+	memcpy(dest, msgbuf1, bytes);
+	return ret;
+}
+
+static int write_device(struct i2c_client *i2c, int reg,
+			int bytes, void *src)
+{
+	unsigned char buf[bytes + 1];
+	struct i2c_adapter *adap = i2c->adapter;
+	struct i2c_msg msg;
+	int ret;
+
+	buf[0] = (unsigned char)reg;
+	memcpy(&buf[1], src, bytes);
+	msg.addr = i2c->addr;
+	msg.flags = i2c->flags;
+	msg.len = bytes + 1;
+	msg.buf = buf;
+
+	ret = adap->algo->master_xfer(adap, &msg, 1);
+	if (ret < 0)
+		return ret;
+	return 0;
+}
+
 int pm860x_page_reg_read(struct i2c_client *i2c, int reg)
 {
 	struct pm860x_chip *chip = i2c_get_clientdata(i2c);

@@ -134,14 +179,15 @@ int pm860x_page_reg_read(struct i2c_client *i2c, int reg)
 	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);
+	i2c_lock_adapter(i2c->adapter);
+	read_device(i2c, 0xFA, 0, &zero);
+	read_device(i2c, 0xFB, 0, &zero);
+	read_device(i2c, 0xFF, 0, &zero);
+	ret = read_device(i2c, reg, 1, &data);
 	if (ret >= 0)
 		ret = (int)data;
-	pm860x_write_device(i2c, 0xFE, 0, &zero);
-	pm860x_write_device(i2c, 0xFC, 0, &zero);
+	read_device(i2c, 0xFC, 0, &zero);
+	i2c_unlock_adapter(i2c->adapter);
 	mutex_unlock(&chip->io_lock);
 	return ret;
 }

@@ -155,12 +201,13 @@ int pm860x_page_reg_write(struct i2c_client *i2c, int reg,
 	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);
+	i2c_lock_adapter(i2c->adapter);
+	read_device(i2c, 0xFA, 0, &zero);
+	read_device(i2c, 0xFB, 0, &zero);
+	read_device(i2c, 0xFF, 0, &zero);
+	ret = write_device(i2c, reg, 1, &data);
+	read_device(i2c, 0xFC, 0, &zero);
+	i2c_unlock_adapter(i2c->adapter);
 	mutex_unlock(&chip->io_lock);
 	return ret;
 }

@@ -174,12 +221,13 @@ int pm860x_page_bulk_read(struct i2c_client *i2c, int reg,
 	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);
+	i2c_lock_adapter(i2c->adapter);
+	read_device(i2c, 0xFA, 0, &zero);
+	read_device(i2c, 0xFB, 0, &zero);
+	read_device(i2c, 0xFF, 0, &zero);
+	ret = read_device(i2c, reg, count, buf);
+	read_device(i2c, 0xFC, 0, &zero);
+	i2c_unlock_adapter(i2c->adapter);
 	mutex_unlock(&chip->io_lock);
 	return ret;
 }

@@ -193,12 +241,13 @@ int pm860x_page_bulk_write(struct i2c_client *i2c, int reg,
 	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);
+	i2c_lock_adapter(i2c->adapter);
+	read_device(i2c, 0xFA, 0, &zero);
+	read_device(i2c, 0xFB, 0, &zero);
+	read_device(i2c, 0xFF, 0, &zero);
+	ret = write_device(i2c, reg, count, buf);
+	read_device(i2c, 0xFC, 0, &zero);
+	i2c_unlock_adapter(i2c->adapter);
 	mutex_unlock(&chip->io_lock);
 	return ret;
 }

@@ -213,18 +262,19 @@ int pm860x_page_set_bits(struct i2c_client *i2c, int reg,
 	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);
+	i2c_lock_adapter(i2c->adapter);
+	read_device(i2c, 0xFA, 0, &zero);
+	read_device(i2c, 0xFB, 0, &zero);
+	read_device(i2c, 0xFF, 0, &zero);
+	ret = read_device(i2c, reg, 1, &value);
 	if (ret < 0)
 		goto out;
 	value &= ~mask;
 	value |= data;
-	ret = pm860x_write_device(i2c, reg, 1, &value);
+	ret = write_device(i2c, reg, 1, &value);
 out:
-	pm860x_write_device(i2c, 0xFE, 0, &zero);
-	pm860x_write_device(i2c, 0xFC, 0, &zero);
+	read_device(i2c, 0xFC, 0, &zero);
+	i2c_unlock_adapter(i2c->adapter);
 	mutex_unlock(&chip->io_lock);
 	return ret;
 }

Signature · 1 line

1.5.6.5

Mark Brown

Reviewed-by on v1 6/6

2011-05-06 11:49 UTC #7 lore raw

Re: [PATCH 6/6] mfd: max8925: remove checking on regulator[0]

In reply to Haojian Zhuang

Quoted 5 lines · Haojian Zhuang

Since regulator[0] is always checking in mfd driver, it results in registration failure without regulator[0].
Signed-off-by Haojian Zhuang <[email protected]>
Cc Samuel Ortiz <[email protected]>

Reviewed-by Mark Brown <[email protected]>

Mark Brown

Acked-by on v1 4/6

2011-05-06 13:06 UTC #10 lore raw

Re: [PATCH 4/6] mfd: 88pm860x: avoid to use constraint name in regulator driver

In reply to Haojian Zhuang

Quoted 2 lines · Haojian Zhuang

Avoid to use constraint name in regulator driver. So use regulator id is used instead in platform driver.

Looks like there's several machines using this which will also need updating. Otherwise looks OK

Acked-by Mark Brown <[email protected]>

Samuel Ortiz

2011-05-13 14:14 UTC #17 lore raw

In reply to Haojian Zhuang

Hi Haojian,

Quoted 1 line · Haojian Zhuang

Enable rtc function in 88pm860x PMIC.

I'm applying this patch for now, hoping Alessandro will be able to review it at some point (and merge it from his tree if he prefers so).

Cheers,
Samuel.

Signature · 2 lines

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

Samuel Ortiz

2011-05-13 14:14 UTC #16 lore raw

Re: [PATCH 2/6] mfd: 88pm860x: avoid to allocate static platform data

In reply to Haojian Zhuang

Hi Haojian,

Quoted 3 lines · Haojian Zhuang

Maybe multiple PMIC devices are installed into one board. Static variable can only make driver logic mess. So remove these variable, and use platform data from platform driver instead.

Very good, thanks a lot.
Applied to my for-next branch.

Cheers,
Samuel.

Signature · 2 lines

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

Samuel Ortiz

2011-05-13 14:21 UTC #12 lore raw

Re: [PATCH 3/6] mfd: 88pm860x: enhance lock on i2c transaction

In reply to Haojian Zhuang

Hi Haojian,

Quoted 3 lines · Haojian Zhuang

Accessing test page in 88pm860x is a sequence of read/write on i2c bus. Bus lock is used in each small i2c transaction. But it may result the whole sequence interrupted by other i2c client transaction.

Sure, but what you mainly want is your MFD i2c IO calls to be serialized, and that's already being taken care of by the current code.
Are other i2c clients (non MFD ones) touching the same i2c registers than the MFD ones ?

Cheers,
Samuel.

Signature · 2 lines

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

Samuel Ortiz

2011-05-13 14:21 UTC #11 lore raw

Re: [PATCH 4/6] mfd: 88pm860x: avoid to use constraint name in regulator driver

In reply to Haojian Zhuang

Hi Haojian,

Quoted 2 lines · Haojian Zhuang

Avoid to use constraint name in regulator driver. So use regulator id is used instead in platform driver.

Patch applied to my for-next branch, thanks.

Cheers,
Samuel.

Signature · 2 lines

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

Samuel Ortiz

2011-05-13 14:24 UTC #9 lore raw

Re: [PATCH 5/6] mfd: 88pm860x: remove unused parameter

In reply to Haojian Zhuang

Hi Haojian,

Quoted 1 line · Haojian Zhuang

i2c_client parameter isn't used in some functions. Just remove it.

Patch applied, thanks a lot.

Cheers,
Samuel.

Signature · 2 lines

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

Samuel Ortiz

2011-05-13 14:24 UTC #8 lore raw

Re: [PATCH 6/6] mfd: max8925: remove checking on regulator[0]

In reply to Haojian Zhuang

Hi Haojian,

Quoted 2 lines · Haojian Zhuang

Since regulator[0] is always checking in mfd driver, it results in registration failure without regulator[0].

Patch applied as well, thanks.

Cheers,
Samuel.

Signature · 2 lines

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

Haojian Zhuang

2011-05-13 15:00 UTC #13 lore raw

Re: [PATCH 3/6] mfd: 88pm860x: enhance lock on i2c transaction

In reply to Samuel Ortiz

Quoted 11 lines · Samuel Ortiz

Hi Haojian,
Quoted 3 lines · Haojian Zhuang
Accessing test page in 88pm860x is a sequence of read/write on i2c bus. Bus lock is used in each small i2c transaction. But it may result the whole sequence interrupted by other i2c client transaction.
Sure, but what you mainly want is your MFD i2c IO calls to be serialized, and that's already being taken care of by the current code.
Are other i2c clients (non MFD ones) touching the same i2c registers than the MFD ones ?

Other process may not access the same register. But they may access same i2c bus. What I did is used to protect bus operation.

Even accessing one register in test page is composed by a sequence of accessing test page.

For example, read one byte of 0xbc in test page.
1) i2c read zero byte from 0xFA
2) i2c read zero byte from 0xFB
3) i2c read zero byte from 0xFF
4) i2c read one byte from 0xbc (desired operation)
5) i2c read zero byte from 0xFC

Step #1 to #3 is used to enter test page mode. Step 4 is used to read desired data.
Step #5 is used to exit test page mode. If all these five steps are using standard
i2c read operation, bus lock in i2c driver will be held and released five times. If another
process is also accessing i2c bus, it may interrupt the sequence and import error to
pmic.

So the potential issue is releasing bus lock too early. If I just add bus lock protection
in test page operation, I'll meet dead lock issue since bus lock will be used in standard
i2c operation. I have to implement my i2c frame and send it out directly.

Best Regards
Haojian

Samuel Ortiz

2011-05-13 18:27 UTC #14 lore raw

Re: [PATCH 3/6] mfd: 88pm860x: enhance lock on i2c transaction

Hi Haojian,

Quoted 25 lines · Haojian Zhuang

Quoted 11 lines · Samuel Ortiz
Hi Haojian,
Quoted 3 lines · Haojian Zhuang
Accessing test page in 88pm860x is a sequence of read/write on i2c bus. Bus lock is used in each small i2c transaction. But it may result the whole sequence interrupted by other i2c client transaction.
Sure, but what you mainly want is your MFD i2c IO calls to be serialized, and that's already being taken care of by the current code.
Are other i2c clients (non MFD ones) touching the same i2c registers than the MFD ones ?
Other process may not access the same register. But they may access same i2c bus. What I did is used to protect bus operation.
Even accessing one register in test page is composed by a sequence of accessing test page.
For example, read one byte of 0xbc in test page.
1) i2c read zero byte from 0xFA
2) i2c read zero byte from 0xFB
3) i2c read zero byte from 0xFF
4) i2c read one byte from 0xbc (desired operation)
5) i2c read zero byte from 0xFC

Quoted 9 lines

Step #1 to #3 is used to enter test page mode. Step 4 is used to read desired data.
Step #5 is used to exit test page mode. If all these five steps are using standard
i2c read operation, bus lock in i2c driver will be held and released five times. If another
process is also accessing i2c bus, it may interrupt the sequence and import error to
pmic.

I see. Wouldn't i2c_transfer() fix your problem then ?

Cheers,
Samuel.

Signature · 2 lines

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

Haojian Zhuang

2011-05-14 13:40 UTC #15 lore raw

Re: [PATCH 3/6] mfd: 88pm860x: enhance lock on i2c transaction

Quoted 37 lines · Samuel Ortiz

Hi Haojian,
Quoted 25 lines · Haojian Zhuang
Quoted 11 lines · Samuel Ortiz
Hi Haojian,
Quoted 3 lines · Haojian Zhuang
Accessing test page in 88pm860x is a sequence of read/write on i2c bus. Bus lock is used in each small i2c transaction. But it may result the whole sequence interrupted by other i2c client transaction.
Sure, but what you mainly want is your MFD i2c IO calls to be serialized, and that's already being taken care of by the current code.
Are other i2c clients (non MFD ones) touching the same i2c registers than the MFD ones ?
Other process may not access the same register. But they may access same i2c bus. What I did is used to protect bus operation.
Even accessing one register in test page is composed by a sequence of accessing test page.
For example, read one byte of 0xbc in test page.
1) i2c read zero byte from 0xFA
2) i2c read zero byte from 0xFB
3) i2c read zero byte from 0xFF
4) i2c read one byte from 0xbc (desired operation)
5) i2c read zero byte from 0xFC
Quoted 9 lines
Step #1 to #3 is used to enter test page mode. Step 4 is used to read desired data.
Step #5 is used to exit test page mode. If all these five steps are using standard
i2c read operation, bus lock in i2c driver will be held and released five times. If another
process is also accessing i2c bus, it may interrupt the sequence and import error to
pmic.

Quoted 2 lines

I see. Wouldn't i2c_transfer() fix your problem then ?

No, i2c_transfer() can't fix my problem.

All atomic i2c frame are composed by start signal, i2c address, i2c data (optional),
end signal. i2c_transfer() is used to send one i2c frame, so it only produce one start signal and one end signal.

What I need is a sequence of i2c frames. Nearly five start signals and five end signals should be produced in the sequence of test page. So i2c_transfer() can't handle this scenario, and I have to use my i2c frames.

Best Regards
Haojian