2011-05-06 09:29:11

by Haojian Zhuang

[permalink] [raw]
Subject: [PATCH 1/6] mfd: 88pm860x: support rtc

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

diff --git a/drivers/mfd/88pm860x-core.c b/drivers/mfd/88pm860x-core.c
index 7ba4aaf..f2cac92 100644
--- a/drivers/mfd/88pm860x-core.c
+++ b/drivers/mfd/88pm860x-core.c
@@ -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);
diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index e187887..286f51e 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -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
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index ca91c3c..e3d6fb0 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -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
diff --git a/drivers/rtc/rtc-88pm860x.c b/drivers/rtc/rtc-88pm860x.c
new file mode 100644
index 0000000..64b847b
--- /dev/null
+++ b/drivers/rtc/rtc-88pm860x.c
@@ -0,0 +1,427 @@
+/*
+ * Real Time Clock driver for Marvell 88PM860x PMIC
+ *
+ * Copyright (c) 2010 Marvell International Ltd.
+ * Author: Haojian Zhuang <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/mutex.h>
+#include <linux/rtc.h>
+#include <linux/delay.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/88pm860x.h>
+
+#define VRTC_CALIBRATION
+
+struct pm860x_rtc_info {
+ struct pm860x_chip *chip;
+ struct i2c_client *i2c;
+ struct rtc_device *rtc_dev;
+ struct device *dev;
+ struct delayed_work calib_work;
+
+ int irq;
+ int vrtc;
+ int (*sync)(unsigned int ticks);
+};
+
+#define REG_VRTC_MEAS1 0x7D
+
+#define REG0_ADDR 0xB0
+#define REG1_ADDR 0xB2
+#define REG2_ADDR 0xB4
+#define REG3_ADDR 0xB6
+
+#define REG0_DATA 0xB1
+#define REG1_DATA 0xB3
+#define REG2_DATA 0xB5
+#define REG3_DATA 0xB7
+
+/* bit definitions of Measurement Enable Register 2 (0x51) */
+#define MEAS2_VRTC (1 << 0)
+
+/* bit definitions of RTC Register 1 (0xA0) */
+#define ALARM_EN (1 << 3)
+#define ALARM_WAKEUP (1 << 4)
+#define ALARM (1 << 5)
+#define RTC1_USE_XO (1 << 7)
+
+#define VRTC_CALIB_INTERVAL (HZ * 60 * 10) /* 10 minutes */
+
+static irqreturn_t rtc_update_handler(int irq, void *data)
+{
+ struct pm860x_rtc_info *info = (struct pm860x_rtc_info *)data;
+ int mask;
+
+ mask = ALARM | ALARM_WAKEUP;
+ pm860x_set_bits(info->i2c, PM8607_RTC1, mask | ALARM_EN, mask);
+ rtc_update_irq(info->rtc_dev, 1, RTC_AF);
+ return IRQ_HANDLED;
+}
+
+static int pm860x_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled)
+{
+ struct pm860x_rtc_info *info = dev_get_drvdata(dev);
+
+ if (enabled)
+ pm860x_set_bits(info->i2c, PM8607_RTC1, ALARM, ALARM);
+ else
+ pm860x_set_bits(info->i2c, PM8607_RTC1, ALARM, 0);
+ return 0;
+}
+
+/*
+ * Calculate the next alarm time given the requested alarm time mask
+ * and the current time.
+ */
+static void rtc_next_alarm_time(struct rtc_time *next, struct rtc_time *now,
+ struct rtc_time *alrm)
+{
+ unsigned long next_time;
+ unsigned long now_time;
+
+ next->tm_year = now->tm_year;
+ next->tm_mon = now->tm_mon;
+ next->tm_mday = now->tm_mday;
+ next->tm_hour = alrm->tm_hour;
+ next->tm_min = alrm->tm_min;
+ next->tm_sec = alrm->tm_sec;
+
+ rtc_tm_to_time(now, &now_time);
+ rtc_tm_to_time(next, &next_time);
+
+ if (next_time < now_time) {
+ /* Advance one day */
+ next_time += 60 * 60 * 24;
+ rtc_time_to_tm(next_time, next);
+ }
+}
+
+static int pm860x_rtc_read_time(struct device *dev, struct rtc_time *tm)
+{
+ struct pm860x_rtc_info *info = dev_get_drvdata(dev);
+ unsigned char buf[8];
+ unsigned long ticks, base, data;
+
+ pm860x_page_bulk_read(info->i2c, REG0_ADDR, 8, buf);
+ dev_dbg(info->dev, "%x-%x-%x-%x-%x-%x-%x-%x\n", buf[0], buf[1],
+ buf[2], buf[3], buf[4], buf[5], buf[6], buf[7]);
+ base = (buf[1] << 24) | (buf[3] << 16) | (buf[5] << 8) | buf[7];
+
+ /* load 32-bit read-only counter */
+ pm860x_bulk_read(info->i2c, PM8607_RTC_COUNTER1, 4, buf);
+ data = (buf[3] << 24) | (buf[2] << 16) | (buf[1] << 8) | buf[0];
+ ticks = base + data;
+ dev_dbg(info->dev, "get base:0x%lx, RO count:0x%lx, ticks:0x%lx\n",
+ base, data, ticks);
+
+ rtc_time_to_tm(ticks, tm);
+
+ return 0;
+}
+
+static int pm860x_rtc_set_time(struct device *dev, struct rtc_time *tm)
+{
+ struct pm860x_rtc_info *info = dev_get_drvdata(dev);
+ unsigned char buf[4];
+ unsigned long ticks, base, data;
+
+ if ((tm->tm_year < 70) || (tm->tm_year > 138)) {
+ dev_dbg(info->dev, "Set time %d out of range. "
+ "Please set time between 1970 to 2038.\n",
+ 1900 + tm->tm_year);
+ return -EINVAL;
+ }
+ rtc_tm_to_time(tm, &ticks);
+
+ /* load 32-bit read-only counter */
+ pm860x_bulk_read(info->i2c, PM8607_RTC_COUNTER1, 4, buf);
+ data = (buf[3] << 24) | (buf[2] << 16) | (buf[1] << 8) | buf[0];
+ base = ticks - data;
+ dev_dbg(info->dev, "set base:0x%lx, RO count:0x%lx, ticks:0x%lx\n",
+ base, data, ticks);
+
+ pm860x_page_reg_write(info->i2c, REG0_DATA, (base >> 24) & 0xFF);
+ pm860x_page_reg_write(info->i2c, REG1_DATA, (base >> 16) & 0xFF);
+ pm860x_page_reg_write(info->i2c, REG2_DATA, (base >> 8) & 0xFF);
+ pm860x_page_reg_write(info->i2c, REG3_DATA, base & 0xFF);
+
+ if (info->sync)
+ info->sync(ticks);
+ return 0;
+}
+
+static int pm860x_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
+{
+ struct pm860x_rtc_info *info = dev_get_drvdata(dev);
+ unsigned char buf[8];
+ unsigned long ticks, base, data;
+ int ret;
+
+ pm860x_page_bulk_read(info->i2c, REG0_ADDR, 8, buf);
+ dev_dbg(info->dev, "%x-%x-%x-%x-%x-%x-%x-%x\n", buf[0], buf[1],
+ buf[2], buf[3], buf[4], buf[5], buf[6], buf[7]);
+ base = (buf[1] << 24) | (buf[3] << 16) | (buf[5] << 8) | buf[7];
+
+ pm860x_bulk_read(info->i2c, PM8607_RTC_EXPIRE1, 4, buf);
+ data = (buf[3] << 24) | (buf[2] << 16) | (buf[1] << 8) | buf[0];
+ ticks = base + data;
+ dev_dbg(info->dev, "get base:0x%lx, RO count:0x%lx, ticks:0x%lx\n",
+ base, data, ticks);
+
+ rtc_time_to_tm(ticks, &alrm->time);
+ ret = pm860x_reg_read(info->i2c, PM8607_RTC1);
+ alrm->enabled = (ret & ALARM_EN) ? 1 : 0;
+ alrm->pending = (ret & (ALARM | ALARM_WAKEUP)) ? 1 : 0;
+ return 0;
+}
+
+static int pm860x_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
+{
+ struct pm860x_rtc_info *info = dev_get_drvdata(dev);
+ struct rtc_time now_tm, alarm_tm;
+ unsigned long ticks, base, data;
+ unsigned char buf[8];
+ int mask;
+
+ pm860x_set_bits(info->i2c, PM8607_RTC1, ALARM_EN, 0);
+
+ pm860x_page_bulk_read(info->i2c, REG0_ADDR, 8, buf);
+ dev_dbg(info->dev, "%x-%x-%x-%x-%x-%x-%x-%x\n", buf[0], buf[1],
+ buf[2], buf[3], buf[4], buf[5], buf[6], buf[7]);
+ base = (buf[1] << 24) | (buf[3] << 16) | (buf[5] << 8) | buf[7];
+
+ /* load 32-bit read-only counter */
+ pm860x_bulk_read(info->i2c, PM8607_RTC_COUNTER1, 4, buf);
+ data = (buf[3] << 24) | (buf[2] << 16) | (buf[1] << 8) | buf[0];
+ ticks = base + data;
+ dev_dbg(info->dev, "get base:0x%lx, RO count:0x%lx, ticks:0x%lx\n",
+ base, data, ticks);
+
+ rtc_time_to_tm(ticks, &now_tm);
+ rtc_next_alarm_time(&alarm_tm, &now_tm, &alrm->time);
+ /* get new ticks for alarm in 24 hours */
+ rtc_tm_to_time(&alarm_tm, &ticks);
+ data = ticks - base;
+
+ buf[0] = data & 0xff;
+ buf[1] = (data >> 8) & 0xff;
+ buf[2] = (data >> 16) & 0xff;
+ buf[3] = (data >> 24) & 0xff;
+ pm860x_bulk_write(info->i2c, PM8607_RTC_EXPIRE1, 4, buf);
+ if (alrm->enabled) {
+ mask = ALARM | ALARM_WAKEUP | ALARM_EN;
+ pm860x_set_bits(info->i2c, PM8607_RTC1, mask, mask);
+ } else {
+ mask = ALARM | ALARM_WAKEUP | ALARM_EN;
+ pm860x_set_bits(info->i2c, PM8607_RTC1, mask,
+ ALARM | ALARM_WAKEUP);
+ }
+ return 0;
+}
+
+static const struct rtc_class_ops pm860x_rtc_ops = {
+ .read_time = pm860x_rtc_read_time,
+ .set_time = pm860x_rtc_set_time,
+ .read_alarm = pm860x_rtc_read_alarm,
+ .set_alarm = pm860x_rtc_set_alarm,
+ .alarm_irq_enable = pm860x_rtc_alarm_irq_enable,
+};
+
+#ifdef VRTC_CALIBRATION
+static void calibrate_vrtc_work(struct work_struct *work)
+{
+ struct pm860x_rtc_info *info = container_of(work,
+ struct pm860x_rtc_info, calib_work.work);
+ unsigned char buf[2];
+ unsigned int sum, data, mean, vrtc_set;
+ int i;
+
+ for (i = 0, sum = 0; i < 16; i++) {
+ msleep(100);
+ pm860x_bulk_read(info->i2c, REG_VRTC_MEAS1, 2, buf);
+ data = (buf[0] << 4) | buf[1];
+ data = (data * 5400) >> 12; /* convert to mv */
+ sum += data;
+ }
+ mean = sum >> 4;
+ vrtc_set = 2700 + (info->vrtc & 0x3) * 200;
+ dev_dbg(info->dev, "mean:%d, vrtc_set:%d\n", mean, vrtc_set);
+
+ sum = pm860x_reg_read(info->i2c, PM8607_RTC_MISC1);
+ data = sum & 0x3;
+ if ((mean + 200) < vrtc_set) {
+ /* try higher voltage */
+ if (++data == 4)
+ goto out;
+ data = (sum & 0xf8) | (data & 0x3);
+ pm860x_reg_write(info->i2c, PM8607_RTC_MISC1, data);
+ } else if ((mean - 200) > vrtc_set) {
+ /* try lower voltage */
+ if (data-- == 0)
+ goto out;
+ data = (sum & 0xf8) | (data & 0x3);
+ pm860x_reg_write(info->i2c, PM8607_RTC_MISC1, data);
+ } else
+ goto out;
+ dev_dbg(info->dev, "set 0x%x to RTC_MISC1\n", data);
+ /* trigger next calibration since VRTC is updated */
+ schedule_delayed_work(&info->calib_work, VRTC_CALIB_INTERVAL);
+ return;
+out:
+ /* disable measurement */
+ pm860x_set_bits(info->i2c, PM8607_MEAS_EN2, MEAS2_VRTC, 0);
+ dev_dbg(info->dev, "finish VRTC calibration\n");
+ return;
+}
+#endif
+
+static int __devinit pm860x_rtc_probe(struct platform_device *pdev)
+{
+ struct pm860x_chip *chip = dev_get_drvdata(pdev->dev.parent);
+ struct pm860x_rtc_pdata *pdata = NULL;
+ struct pm860x_rtc_info *info;
+ struct rtc_time tm;
+ unsigned long ticks = 0;
+ int ret;
+
+ pdata = pdev->dev.platform_data;
+ if (pdata == NULL)
+ dev_warn(&pdev->dev, "No platform data!\n");
+
+ info = kzalloc(sizeof(struct pm860x_rtc_info), GFP_KERNEL);
+ if (!info)
+ return -ENOMEM;
+ info->irq = platform_get_irq(pdev, 0);
+ if (info->irq < 0) {
+ dev_err(&pdev->dev, "No IRQ resource!\n");
+ ret = -EINVAL;
+ goto out;
+ }
+
+ info->chip = chip;
+ info->i2c = (chip->id == CHIP_PM8607) ? chip->client : chip->companion;
+ info->dev = &pdev->dev;
+ dev_set_drvdata(&pdev->dev, info);
+
+ ret = request_threaded_irq(info->irq, NULL, rtc_update_handler,
+ IRQF_ONESHOT, "rtc", info);
+ if (ret < 0) {
+ dev_err(chip->dev, "Failed to request IRQ: #%d: %d\n",
+ info->irq, ret);
+ goto out;
+ }
+
+ /* set addresses of 32-bit base value for RTC time */
+ pm860x_page_reg_write(info->i2c, REG0_ADDR, REG0_DATA);
+ pm860x_page_reg_write(info->i2c, REG1_ADDR, REG1_DATA);
+ pm860x_page_reg_write(info->i2c, REG2_ADDR, REG2_DATA);
+ pm860x_page_reg_write(info->i2c, REG3_ADDR, REG3_DATA);
+
+ ret = pm860x_rtc_read_time(&pdev->dev, &tm);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "Failed to read initial time.\n");
+ goto out_rtc;
+ }
+ if ((tm.tm_year < 70) || (tm.tm_year > 138)) {
+ tm.tm_year = 70;
+ tm.tm_mon = 0;
+ tm.tm_mday = 1;
+ tm.tm_hour = 0;
+ tm.tm_min = 0;
+ tm.tm_sec = 0;
+ ret = pm860x_rtc_set_time(&pdev->dev, &tm);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "Failed to set initial time.\n");
+ goto out_rtc;
+ }
+ }
+ rtc_tm_to_time(&tm, &ticks);
+ if (pdata && pdata->sync) {
+ pdata->sync(ticks);
+ info->sync = pdata->sync;
+ }
+
+ info->rtc_dev = rtc_device_register("88pm860x-rtc", &pdev->dev,
+ &pm860x_rtc_ops, THIS_MODULE);
+ ret = PTR_ERR(info->rtc_dev);
+ if (IS_ERR(info->rtc_dev)) {
+ dev_err(&pdev->dev, "Failed to register RTC device: %d\n", ret);
+ goto out_rtc;
+ }
+
+ /*
+ * enable internal XO instead of internal 3.25MHz clock since it can
+ * free running in PMIC power-down state.
+ */
+ pm860x_set_bits(info->i2c, PM8607_RTC1, RTC1_USE_XO, RTC1_USE_XO);
+
+#ifdef VRTC_CALIBRATION
+ /* <00> -- 2.7V, <01> -- 2.9V, <10> -- 3.1V, <11> -- 3.3V */
+ if (pdata && pdata->vrtc)
+ info->vrtc = pdata->vrtc & 0x3;
+ else
+ info->vrtc = 1;
+ pm860x_set_bits(info->i2c, PM8607_MEAS_EN2, MEAS2_VRTC, MEAS2_VRTC);
+
+ /* calibrate VRTC */
+ INIT_DELAYED_WORK(&info->calib_work, calibrate_vrtc_work);
+ schedule_delayed_work(&info->calib_work, VRTC_CALIB_INTERVAL);
+#endif /* VRTC_CALIBRATION */
+ return 0;
+out_rtc:
+ free_irq(info->irq, info);
+out:
+ kfree(info);
+ return ret;
+}
+
+static int __devexit pm860x_rtc_remove(struct platform_device *pdev)
+{
+ struct pm860x_rtc_info *info = platform_get_drvdata(pdev);
+
+#ifdef VRTC_CALIBRATION
+ flush_scheduled_work();
+ /* disable measurement */
+ pm860x_set_bits(info->i2c, PM8607_MEAS_EN2, MEAS2_VRTC, 0);
+#endif /* VRTC_CALIBRATION */
+
+ platform_set_drvdata(pdev, NULL);
+ rtc_device_unregister(info->rtc_dev);
+ free_irq(info->irq, info);
+ kfree(info);
+ return 0;
+}
+
+static struct platform_driver pm860x_rtc_driver = {
+ .driver = {
+ .name = "88pm860x-rtc",
+ .owner = THIS_MODULE,
+ },
+ .probe = pm860x_rtc_probe,
+ .remove = __devexit_p(pm860x_rtc_remove),
+};
+
+static int __init pm860x_rtc_init(void)
+{
+ return platform_driver_register(&pm860x_rtc_driver);
+}
+module_init(pm860x_rtc_init);
+
+static void __exit pm860x_rtc_exit(void)
+{
+ platform_driver_unregister(&pm860x_rtc_driver);
+}
+module_exit(pm860x_rtc_exit);
+
+MODULE_DESCRIPTION("Marvell 88PM860x RTC driver");
+MODULE_AUTHOR("Haojian Zhuang <[email protected]>");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/mfd/88pm860x.h b/include/linux/mfd/88pm860x.h
index 8fba797..63b4fb8 100644
--- a/include/linux/mfd/88pm860x.h
+++ b/include/linux/mfd/88pm860x.h
@@ -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;
--
1.5.6.5


2011-05-06 09:29:13

by Haojian Zhuang

[permalink] [raw]
Subject: [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(-)

diff --git a/drivers/mfd/88pm860x-core.c b/drivers/mfd/88pm860x-core.c
index f2cac92..04ab50c 100644
--- a/drivers/mfd/88pm860x-core.c
+++ b/drivers/mfd/88pm860x-core.c
@@ -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,
--
1.5.6.5

2011-05-06 09:30:29

by Haojian Zhuang

[permalink] [raw]
Subject: [PATCH 3/6] mfd: 88pm860x: enhance lock on i2c transaction

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(-)

diff --git a/drivers/mfd/88pm860x-i2c.c b/drivers/mfd/88pm860x-i2c.c
index e017dc8..67a34f4 100644
--- a/drivers/mfd/88pm860x-i2c.c
+++ b/drivers/mfd/88pm860x-i2c.c
@@ -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;
}
--
1.5.6.5

2011-05-06 09:29:14

by Haojian Zhuang

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

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(-)

diff --git a/drivers/mfd/88pm860x-core.c b/drivers/mfd/88pm860x-core.c
index 04ab50c..8e88fba 100644
--- a/drivers/mfd/88pm860x-core.c
+++ b/drivers/mfd/88pm860x-core.c
@@ -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;
diff --git a/drivers/regulator/88pm8607.c b/drivers/regulator/88pm8607.c
index 784ea77..1904be9 100644
--- a/drivers/regulator/88pm8607.c
+++ b/drivers/regulator/88pm8607.c
@@ -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)) {
--
1.5.6.5

2011-05-06 09:29:48

by Haojian Zhuang

[permalink] [raw]
Subject: [PATCH 5/6] mfd: 88pm860x: remove unused parameter

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(-)

diff --git a/drivers/mfd/88pm860x-core.c b/drivers/mfd/88pm860x-core.c
index 8e88fba..17dfe9b 100644
--- a/drivers/mfd/88pm860x-core.c
+++ b/drivers/mfd/88pm860x-core.c
@@ -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);
--
1.5.6.5

2011-05-06 09:29:27

by Haojian Zhuang

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

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(-)

diff --git a/drivers/mfd/max8925-core.c b/drivers/mfd/max8925-core.c
index 58cc5fd..e1e59c9 100644
--- a/drivers/mfd/max8925-core.c
+++ b/drivers/mfd/max8925-core.c
@@ -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);
--
1.5.6.5

2011-05-06 11:49:16

by Mark Brown

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

On Fri, May 06, 2011 at 05:21:25PM +0800, Haojian Zhuang wrote:
> 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]>

2011-05-06 13:06:34

by Mark Brown

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

On Fri, May 06, 2011 at 05:21:23PM +0800, Haojian Zhuang wrote:

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

2011-05-13 14:14:04

by Samuel Ortiz

[permalink] [raw]
Subject: Re: [PATCH 1/6] mfd: 88pm860x: support rtc

Hi Haojian,

On Fri, May 06, 2011 at 05:21:20PM +0800, Haojian Zhuang wrote:
> 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.

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

2011-05-13 14:14:39

by Samuel Ortiz

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

Hi Haojian,

On Fri, May 06, 2011 at 05:21:21PM +0800, Haojian Zhuang wrote:
> 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.

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

2011-05-13 14:21:03

by Samuel Ortiz

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

Hi Haojian,

On Fri, May 06, 2011 at 05:21:22PM +0800, Haojian Zhuang wrote:
> 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.

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

2011-05-13 14:21:59

by Samuel Ortiz

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

Hi Haojian,

On Fri, May 06, 2011 at 05:21:23PM +0800, Haojian Zhuang wrote:
> 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.

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

2011-05-13 14:24:15

by Samuel Ortiz

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

Hi Haojian,

On Fri, May 06, 2011 at 05:21:24PM +0800, Haojian Zhuang wrote:
> i2c_client parameter isn't used in some functions. Just remove it.
Patch applied, thanks a lot.

Cheers,
Samuel.

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

2011-05-13 14:24:42

by Samuel Ortiz

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

Hi Haojian,

On Fri, May 06, 2011 at 05:21:25PM +0800, Haojian Zhuang wrote:
> Since regulator[0] is always checking in mfd driver, it results in
> registration failure without regulator[0].
Patch applied as well, thanks.

Cheers,
Samuel.

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

2011-05-13 15:00:27

by Haojian Zhuang

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

On Fri, May 13, 2011 at 10:20 PM, Samuel Ortiz <[email protected]> wrote:
> Hi Haojian,
>
> On Fri, May 06, 2011 at 05:21:22PM +0800, Haojian Zhuang wrote:
>> 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

2011-05-13 18:27:37

by Samuel Ortiz

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

Hi Haojian,

On Fri, May 13, 2011 at 11:00:24PM +0800, Haojian Zhuang wrote:
> On Fri, May 13, 2011 at 10:20 PM, Samuel Ortiz <[email protected]> wrote:
> > Hi Haojian,
> >
> > On Fri, May 06, 2011 at 05:21:22PM +0800, Haojian Zhuang wrote:
> >> 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.
I see. Wouldn't i2c_transfer() fix your problem then ?

Cheers,
Samuel.

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

2011-05-14 13:40:19

by Haojian Zhuang

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

On Sat, May 14, 2011 at 2:27 AM, Samuel Ortiz <[email protected]> wrote:
> Hi Haojian,
>
> On Fri, May 13, 2011 at 11:00:24PM +0800, Haojian Zhuang wrote:
>> On Fri, May 13, 2011 at 10:20 PM, Samuel Ortiz <[email protected]> wrote:
>> > Hi Haojian,
>> >
>> > On Fri, May 06, 2011 at 05:21:22PM +0800, Haojian Zhuang wrote:
>> >> 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.
> 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