Update 88pm860x and max8925 MFD driver

PATCH In review Haojian Zhuang 7 participants 52 messages Apr 18, 2011 – May 22, 2011 1 message not in the archive

Subsystems (5)

Input drivers Multifunction devices (MFD) Voltage and current regulator framework Real time clock (RTC) +1 more subsystems

lore

Share & tools

lore

A message that is not in the archive (sent off-list or not yet received): 2011041801

Haojian Zhuang

author

2011-04-18 14:09 UTC #1 lore raw

v2 cover Update 88pm860x and max8925 MFD driver

Changelog:
v2:
1. Fix the calculation issue on time in touch driver of 88pm860x.
2. Always checking return code in open() of touch driver of 88pm860x.
3. Avoid to check constraint name in regulator driver. Check driver_data instead.

Haojian Zhuang

author

2011-04-18 14:09 UTC #2 lore raw

PATCH v2 1/14 input: touchscreen: use polling mode in 88pm860x

In reply to a message not in the archive

Measuring point on touchscreen with IRQ mode can only monitor pen-down event. If finger is moving on touchscreen, it can't be monitored by IRQ pen-down event. So switch to polling mode after pen-down event.

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

Cc Dmitry Torokhov <[email protected]>

---
 drivers/input/touchscreen/88pm860x-ts.c |   79 ++++++++++++++++++++++++------
 1 files changed, 63 insertions(+), 16 deletions(-)

drivers/input/touchscreen/88pm860x-ts.c +63 ~~-16~~

index b3aebc2..dbb3435 100644
@@ -12,13 +12,25 @@
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/i2c.h>
+#include <linux/slab.h>
 #include <linux/input.h>
 #include <linux/mfd/88pm860x.h>
-#include <linux/slab.h>
 
 #define MEAS_LEN		(8)
 #define ACCURATE_BIT		(12)
 
+/*
+ * While 32KHz hardware timer is used for scheduler, we always assign HZ
+ * to 128. It means that 1 tick costs 7.8msec.
+ */
+#define MEASURE_INTERVAL_MS	(7)
+
+/* debounce register */
+#define DEBOUNCE		(0x0A)
+
+#define PD_DEBOUNCE_0MSEC	(0)
+#define PD_DEBOUNCE_MASK	(3)
+
 /* touch register */
 #define MEAS_EN3		(0x52)

@@ -39,22 +51,27 @@
 #define MEAS_TSIZ2_EN		(1 << 7)
 
 struct pm860x_touch {
-	struct input_dev *idev;
-	struct i2c_client *i2c;
-	struct pm860x_chip *chip;
-	int irq;
-	int res_x;		/* resistor of Xplate */
+	struct input_dev	*idev;
+	struct i2c_client	*i2c;
+	struct pm860x_chip	*chip;
+	struct delayed_work	poll_work;
+	struct mutex		lock;
+
+	int			irq;
+	int			res_x;		/* resistor of Xplate */
 };
 
-static irqreturn_t pm860x_touch_handler(int irq, void *data)
+static void pm860x_poll_work(struct work_struct *work)
 {
-	struct pm860x_touch *touch = data;
+	struct pm860x_touch *touch = container_of(work, struct pm860x_touch,
+						  poll_work.work);
 	struct pm860x_chip *chip = touch->chip;
+	int x, y, z1, z2, rt = 0;
+	int ret, pen_down, interval;
 	unsigned char buf[MEAS_LEN];
-	int x, y, pen_down;
-	int z1, z2, rt = 0;
-	int ret;
+	struct timeval start, end;
 
+	do_gettimeofday(&start);
 	ret = pm860x_bulk_read(touch->i2c, MEAS_TSIX_1, MEAS_LEN, buf);
 	if (ret < 0)
 		goto out;

@@ -77,14 +94,39 @@ static irqreturn_t pm860x_touch_handler(int irq, void *data)
 		input_report_abs(touch->idev, ABS_PRESSURE, rt);
 		input_report_key(touch->idev, BTN_TOUCH, 1);
 		dev_dbg(chip->dev, "pen down at [%d, %d].\n", x, y);
+
+		do_gettimeofday(&end);
+		interval = ((end.tv_sec - start.tv_sec) * 1000000 +
+			(end.tv_usec - start.tv_usec)) / 1000;
+		interval = (interval < MEASURE_INTERVAL_MS)
+			? (MEASURE_INTERVAL_MS - interval) : 0;
+		schedule_delayed_work(&touch->poll_work,
+				      msecs_to_jiffies(interval));
 	} else {
 		input_report_abs(touch->idev, ABS_PRESSURE, 0);
 		input_report_key(touch->idev, BTN_TOUCH, 0);
 		dev_dbg(chip->dev, "pen release\n");
+		mutex_unlock(&touch->lock);
 	}
 	input_sync(touch->idev);
 
 out:
+	return;
+}
+
+static irqreturn_t pm860x_touch_handler(int irq, void *data)
+{
+	struct pm860x_touch *touch = data;
+	int ret;
+
+	ret = pm860x_reg_read(touch->i2c, PM8607_STATUS_1);
+	dev_dbg(touch->chip->dev, "pen status:%d\n", (ret & PM8607_STATUS_PEN)
+		? 1 : 0);
+	if ((ret & PM8607_STATUS_PEN) == 0)
+		return IRQ_HANDLED;
+
+	mutex_lock(&touch->lock);
+	pm860x_poll_work(&touch->poll_work.work);
 	return IRQ_HANDLED;
 }

@@ -93,12 +135,14 @@ static int pm860x_touch_open(struct input_dev *dev)
 	struct pm860x_touch *touch = input_get_drvdata(dev);
 	int data, ret;
 
+	/* set debounce time with 0ms */
+	ret = pm860x_set_bits(touch->i2c, DEBOUNCE, PD_DEBOUNCE_MASK,
+			      PD_DEBOUNCE_0MSEC);
+	if (ret < 0)
+		goto out;
 	data = MEAS_PD_EN | MEAS_TSIX_EN | MEAS_TSIY_EN
 		| MEAS_TSIZ1_EN | MEAS_TSIZ2_EN;
 	ret = pm860x_set_bits(touch->i2c, MEAS_EN3, data, data);
-	if (ret < 0)
-		goto out;
-	return 0;
 out:
 	return ret;
 }

@@ -152,16 +196,17 @@ static int __devinit pm860x_touch_probe(struct platform_device *pdev)
 	}
 
 	touch->idev->name = "88pm860x-touch";
-	touch->idev->phys = "88pm860x/input0";
+	touch->idev->phys = "88pm860x/input1";
 	touch->idev->id.bustype = BUS_I2C;
 	touch->idev->dev.parent = &pdev->dev;
 	touch->idev->open = pm860x_touch_open;
 	touch->idev->close = pm860x_touch_close;
 	touch->chip = chip;
 	touch->i2c = (chip->id == CHIP_PM8607) ? chip->client : chip->companion;
-	touch->irq = irq + chip->irq_base;
+	touch->irq = irq;
 	touch->res_x = pdata->res_x;
 	input_set_drvdata(touch->idev, touch);
+	mutex_init(&touch->lock);
 
 	ret = request_threaded_irq(touch->irq, NULL, pm860x_touch_handler,
 				   IRQF_ONESHOT, "touch", touch);

@@ -188,6 +233,7 @@ static int __devinit pm860x_touch_probe(struct platform_device *pdev)
 	}
 
 	platform_set_drvdata(pdev, touch);
+	INIT_DELAYED_WORK(&touch->poll_work, pm860x_poll_work);
 	return 0;
 out_rg:
 	free_irq(touch->irq, touch);

@@ -202,6 +248,7 @@ static int __devexit pm860x_touch_remove(struct platform_device *pdev)
 {
 	struct pm860x_touch *touch = platform_get_drvdata(pdev);
 
+	flush_scheduled_work();
 	input_unregister_device(touch->idev);
 	free_irq(touch->irq, touch);
 	platform_set_drvdata(pdev, NULL);

Signature · 1 line

1.5.6.5

Haojian Zhuang

author

2011-04-18 14:10 UTC #3 lore raw

PATCH v2 2/14 input: touchscreen: move initialization in 88pm860x

In reply to a message not in the archive

Move the GPADC initialization code from mfd driver to touch driver.

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

Cc Dmitry Torokhov <[email protected]>

---
 drivers/input/touchscreen/88pm860x-ts.c |   44 +++++++++++++++++++++-----
 drivers/mfd/88pm860x-core.c             |   51 -------------------------------
 2 files changed, 35 insertions(+), 60 deletions(-)

drivers/input/touchscreen/88pm860x-ts.c +35 -9

index dbb3435..8ae2856 100644
@@ -14,6 +14,7 @@
 #include <linux/i2c.h>
 #include <linux/slab.h>
 #include <linux/input.h>
+#include <linux/mfd/core.h>
 #include <linux/mfd/88pm860x.h>
 
 #define MEAS_LEN		(8)

@@ -157,11 +158,40 @@ static void pm860x_touch_close(struct input_dev *dev)
 	pm860x_set_bits(touch->i2c, MEAS_EN3, data, 0);
 }
 
+static void __devinit pm860x_gpadc_init(struct i2c_client *i2c,
+					struct pm860x_touch_pdata *pdata)
+{
+	int data;
+
+	/* set GPADC MISC1 register */
+	data = 0;
+	data |= (pdata->gpadc_prebias << 1) & PM8607_GPADC_PREBIAS_MASK;
+	data |= (pdata->slot_cycle << 3) & PM8607_GPADC_SLOT_CYCLE_MASK;
+	data |= (pdata->off_scale << 5) & PM8607_GPADC_OFF_SCALE_MASK;
+	data |= (pdata->sw_cal << 7) & PM8607_GPADC_SW_CAL_MASK;
+	if (data)
+		pm860x_reg_write(i2c, PM8607_GPADC_MISC1, data);
+
+	/* set tsi prebias time */
+	if (pdata->tsi_prebias) {
+		data = pdata->tsi_prebias;
+		pm860x_reg_write(i2c, PM8607_TSI_PREBIAS, data);
+	}
+
+	/* set prebias & prechg time of pen detect */
+	data = 0;
+	data |= pdata->pen_prebias & PM8607_PD_PREBIAS_MASK;
+	data |= (pdata->pen_prechg << 5) & PM8607_PD_PRECHG_MASK;
+	if (data)
+		pm860x_reg_write(i2c, PM8607_PD_PREBIAS, data);
+
+	pm860x_set_bits(i2c, PM8607_GPADC_MISC1,
+			PM8607_GPADC_EN, PM8607_GPADC_EN);
+}
+
 static int __devinit pm860x_touch_probe(struct platform_device *pdev)
 {
 	struct pm860x_chip *chip = dev_get_drvdata(pdev->dev.parent);
-	struct pm860x_platform_data *pm860x_pdata =		\
-				pdev->dev.parent->platform_data;
 	struct pm860x_touch_pdata *pdata = NULL;
 	struct pm860x_touch *touch;
 	int irq, ret;

@@ -172,13 +202,8 @@ static int __devinit pm860x_touch_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	if (!pm860x_pdata) {
-		dev_err(&pdev->dev, "platform data is missing\n");
-		return -EINVAL;
-	}
-
-	pdata = pm860x_pdata->touch;
-	if (!pdata) {
+	pdata = pdev->dev.platform_data;
+	if (pdata == NULL) {
 		dev_err(&pdev->dev, "touchscreen data is missing\n");
 		return -EINVAL;
 	}

@@ -232,6 +257,7 @@ static int __devinit pm860x_touch_probe(struct platform_device *pdev)
 		goto out_rg;
 	}
 
+	pm860x_gpadc_init(touch->i2c, pdata);
 	platform_set_drvdata(pdev, touch);
 	INIT_DELAYED_WORK(&touch->poll_work, pm860x_poll_work);
 	return 0;

drivers/mfd/88pm860x-core.c +0 ~~-51~~

63 lines not on this page · raw message

Signature · 1 line

1.5.6.5

Samuel Ortiz

Acked-by on v2 2/14

2011-04-26 08:58 UTC #4 lore raw

Re: [PATCH 02/14] input: touchscreen: move initialization in 88pm860x

Hi Haojian,

Quoted 1 line · Haojian Zhuang

Move the GPADC initialization code from mfd driver to touch driver.

I think this should go through Dmitry's tree. If he's ok with that:

Acked-by Samuel Ortiz <[email protected]>

Cheers,
Samuel.

Signature · 2 lines

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

Haojian Zhuang

author

2011-04-18 14:10 UTC #5 lore raw

PATCH v2 3/14 rtc: add 88pm860x rtc

In reply to a message not in the archive

Support RTC component in 88PM860x. Also support synchronize time from 88PM860x RTC to another RTC device. It should be implemented in sync callback API.

While 88PM860x RTC is enabled, it can do RTC calibration to gurantee enough power is supplied on VRTC domain of 88PM860x.

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

Cc Alessandro Zummo <[email protected]>

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

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

index b03cdd3..f114ec7 100644
@@ -57,6 +57,10 @@ static struct resource regulator_resources[] __initdata = {
 	{PM8607_ID_LDO15, PM8607_ID_LDO15, "ldo-15", IORESOURCE_IO,},
 };
 
+static struct resource rtc_resources[] __devinitdata = {
+	{PM8607_IRQ_RTC, PM8607_IRQ_RTC, "rtc", IORESOURCE_IRQ,},
+};
+
 static struct resource touch_resources[] __initdata = {
 	{PM8607_IRQ_PEN, PM8607_IRQ_PEN, "touch", IORESOURCE_IRQ,},
 };

@@ -126,6 +130,10 @@ static struct mfd_cell regulator_devs[] __initdata = {
 	{"88pm860x-regulator", 17,},
 };
 
+static struct mfd_cell rtc_devs[] = {
+	{"88pm860x-rtc", -1,},
+};
+
 static struct mfd_cell touch_devs[] __initdata = {
 	{"88pm860x-touch", -1,},
 };

@@ -148,6 +156,7 @@ 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;
+static struct pm860x_rtc_pdata rtc_pdata;
 
 struct pm860x_irq_data {
 	int	reg;

@@ -588,6 +597,27 @@ 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) || (pdata->rtc == NULL))
+		return;
+
+	memcpy(&rtc_pdata, pdata->rtc, sizeof(struct pm860x_rtc_pdata));
+	rtc_devs[0].platform_data = &rtc_pdata;
+	rtc_devs[0].pdata_size = sizeof(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)

@@ -719,6 +749,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 +426 -0

426 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

Samuel Ortiz

Acked-by on v2 3/14

2011-04-26 08:59 UTC #6 lore raw

Re: [PATCH 03/14] rtc: add 88pm860x rtc

Hi Haojian,

Quoted 6 lines · Haojian Zhuang

Support RTC component in 88PM860x. Also support synchronize time from 88PM860x RTC to another RTC device. It should be implemented in sync callback API.
While 88PM860x RTC is enabled, it can do RTC calibration to gurantee enough power is supplied on VRTC domain of 88PM860x.

For the MFD part:

Acked-by Samuel Ortiz <[email protected]>

Cheers,
Samuel.

Signature · 2 lines

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

Haojian Zhuang

author

2011-04-18 14:10 UTC #7 lore raw

PATCH v2 4/14 input: set the long press detection in 88pm860x onkey

In reply to a message not in the archive

Move the long-press detection from onkey handler to probe function. Avoid unnecessary I2C operation in onkey handler.

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

Cc Dmitry Torokhov <[email protected]>

---
 drivers/input/misc/88pm860x_onkey.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

drivers/input/misc/88pm860x_onkey.c +3 -3

index 3dca3c1..1abc77e 100644
@@ -50,9 +50,6 @@ static irqreturn_t pm860x_onkey_handler(int irq, void *data)
 	ret &= ONKEY_STATUS;
 	input_report_key(info->idev, KEY_POWER, ret);
 	input_sync(info->idev);
-
-	/* Enable 8-second long onkey detection */
-	pm860x_set_bits(info->i2c, PM8607_WAKEUP, 3, LONG_ONKEY_EN);
 	return IRQ_HANDLED;
 }

@@ -105,6 +102,9 @@ static int __devinit pm860x_onkey_probe(struct platform_device *pdev)
 	}
 
 	platform_set_drvdata(pdev, info);
+
+	/* Enable 8-second long onkey detection */
+	pm860x_set_bits(info->i2c, PM8607_WAKEUP, 3, LONG_ONKEY_EN);
 	return 0;
 
 out_irq:

Signature · 1 line

1.5.6.5

Haojian Zhuang

author

2011-04-18 14:10 UTC #8 lore raw

PATCH v2 5/14 w1: add DS278x slave driver

In reply to a message not in the archive

Append DS278x slave driver.

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

Cc Evgeniy Polyakov <[email protected]>

---
 drivers/w1/slaves/Kconfig     |   13 +++
 drivers/w1/slaves/Makefile    |    1 +
 drivers/w1/slaves/w1_ds278x.c |  184 +++++++++++++++++++++++++++++++++++++++++
 drivers/w1/w1_family.h        |    3 +
 4 files changed, 201 insertions(+), 0 deletions(-)
 create mode 100644 drivers/w1/slaves/w1_ds278x.c

drivers/w1/slaves/Kconfig +13 -0

index f0c9096..d737aa9 100644
@@ -61,6 +61,19 @@ config W1_SLAVE_DS2760
 
 	  If you are unsure, say N.
 
+config W1_SLAVE_DS278x
+	tristate "Dallas 278x battery monitor chip"
+	depends on W1
+	help
+	  If you enable this you will have the DS278x battery monitor
+	  chip support.
+
+	  The battery monitor chip is used in many batteries/devices
+	  as the one who is responsible for charging/discharging/monitoring
+	  Li+ batteries.
+
+	  If you are unsure, say N.
+
 config W1_SLAVE_BQ27000
 	tristate "BQ27000 slave support"
 	depends on W1

drivers/w1/slaves/Makefile +1 -0

index 3c76350..bcc134b 100644
@@ -8,4 +8,5 @@ obj-$(CONFIG_W1_SLAVE_DS2423)	+= w1_ds2423.o
 obj-$(CONFIG_W1_SLAVE_DS2431)	+= w1_ds2431.o
 obj-$(CONFIG_W1_SLAVE_DS2433)	+= w1_ds2433.o
 obj-$(CONFIG_W1_SLAVE_DS2760)	+= w1_ds2760.o
+obj-$(CONFIG_W1_SLAVE_DS278x)	+= w1_ds278x.o
 obj-$(CONFIG_W1_SLAVE_BQ27000)	+= w1_bq27000.o

drivers/w1/slaves/w1_ds278x.c new file +184 -0

184 lines not on this page · raw message

drivers/w1/w1_family.h +3 -0

index f3b636d..0f0b58c 100644
@@ -36,6 +36,9 @@
 #define W1_THERM_DS18B20 	0x28
 #define W1_EEPROM_DS2431	0x2D
 #define W1_FAMILY_DS2760	0x30
+#define W1_BATTMON_DS2780	0x32
+#define W1_BATTMON_DS2781	0x3D
+#define W1_BATTMON_DS2783	0x34
 
 #define MAXNAMELEN		32

Signature · 1 line

1.5.6.5

Haojian Zhuang

author

2011-04-18 14:12 UTC #9 lore raw

PATCH v2 6/14 mfd: pxa-w1: MFD driver for PXA 1wire control + DS1WM chip

In reply to a message not in the archive

This driver provides registers and IRQ of PXA3xx chips to the ds1wm driver.

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

Cc Evgeniy Polyakov <[email protected]>

---
 drivers/mfd/Kconfig  |    7 ++
 drivers/mfd/Makefile |    1 +
 drivers/mfd/pxa-w1.c |  150 ++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 158 insertions(+), 0 deletions(-)
 create mode 100644 drivers/mfd/pxa-w1.c

drivers/mfd/Kconfig +7 -0

index e2fea58..b6ecf90 100644
@@ -34,6 +34,13 @@ config MFD_88PM860X
 	  select individual components like voltage regulators, RTC and
 	  battery-charger under the corresponding menus.
 
+config MFD_PXA_DS1WM
+	tristate "Support DS1WM chip on Marvell silicons"
+	select MFD_CORE
+	help
+	  This core driver provides register access for PXA_DS1WM.
+	  Actual functionality is handled by the ds1wm drivers.
+
 config MFD_SM501
 	tristate "Support for Silicon Motion SM501"
 	 ---help---

drivers/mfd/Makefile +1 -0

index 419caa9..4f8d1d2 100644
@@ -4,6 +4,7 @@
 
 88pm860x-objs			:= 88pm860x-core.o 88pm860x-i2c.o
 obj-$(CONFIG_MFD_88PM860X)	+= 88pm860x.o
+obj-$(CONFIG_MFD_PXA_DS1WM)	+= pxa-w1.o
 obj-$(CONFIG_MFD_SM501)		+= sm501.o
 obj-$(CONFIG_MFD_ASIC3)		+= asic3.o tmio_core.o

drivers/mfd/pxa-w1.c new file +150 -0

150 lines not on this page · raw message

Signature · 1 line

1.5.6.5

Samuel Ortiz

2011-04-26 09:48 UTC #10 lore raw

Re: [PATCH 06/14] mfd: pxa-w1: MFD driver for PXA 1wire control + DS1WM chip

Hi Haojian,

Quoted 1 line · Haojian Zhuang

This driver provides registers and IRQ of PXA3xx chips to the ds1wm driver.

I see why this is needed, but I really think the ds1wm driver should be converted to a regular platform device. The MFD driver below is just adding a useless middle layer between your platform code and the ds1wm one.

Cheers,
Samuel.

Signature · 2 lines

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

Haojian Zhuang

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

Re: [PATCH 06/14] mfd: pxa-w1: MFD driver for PXA 1wire control + DS1WM chip

Quoted 8 lines · Samuel Ortiz

Hi Haojian,
Quoted 1 line · Haojian Zhuang
This driver provides registers and IRQ of PXA3xx chips to the ds1wm driver.
I see why this is needed, but I really think the ds1wm driver should be converted to a regular platform device. The MFD driver below is just adding a useless middle layer between your platform code and the ds1wm one.

I'm OK to use the regulator platform device for ds1wm driver except for one concern.
ds1wm driver is designed for mfd cell driver. I need to update it as regulator platform
driver first. cell->enable() / cell->disable() is used to enable clock for ds1wm driver.
So I need to change the API from cell->enable/disable to clk_enable() / clk_disable().

But the key issue is that there's no common structure for clkdev. Now clkdev is only
designed for deeply machine depend.

Russell King - ARM Linux

2011-05-14 19:40 UTC #12 lore raw

Re: [PATCH 06/14] mfd: pxa-w1: MFD driver for PXA 1wire control + DS1WM chip

Quoted 4 lines · Haojian Zhuang

But the key issue is that there's no common structure for clkdev. Now clkdev is only designed for deeply machine depend.
Quoted 1 line
From my view is that we need a common structure for clkdev.

You mean struct clk.

As I said on our call on Thursday, everyone knows this, and it's something that has been worked on for the last year. Concensus had been reached and everyone had settled on a proposal, and just when we were almost ready to merge it, the concensus fell apart again.

As a result of that, a common struct clk has been delayed - it's not going to be in place for 2.6.40 (three months) but maybe we can reach concensus again for the .41 merge window.

As I also said, this issue was talked about in a session at the Linaro conference, and it will be interesting to hear what the result of that session was. At the moment, I'm entirely out of the loop on many things that were discussed there (except for a message from tglx.)

Mark Brown

2011-05-14 22:23 UTC #13 lore raw

Re: [PATCH 06/14] mfd: pxa-w1: MFD driver for PXA 1wire control + DS1WM chip

Quoted 4 lines · Russell King - ARM Linux

As I also said, this issue was talked about in a session at the Linaro conference, and it will be interesting to hear what the result of that session was. At the moment, I'm entirely out of the loop on many things that were discussed there (except for a message from tglx.)

Everything should have had some live noted minutes linked off this site:

http://summit.ubuntu.com/uds-o/

probably everything interesting is on this page:

http://summit.ubuntu.com/uds-o/track/linaro-kernel/

(in etherpad, you want the icon that looks a bit like a notepad on the left of the box in the talk). They won't always be accurate as they were done live and not 100% reviewed but might be helpful.

Several people had actions to bring things to the list so I'd expect to see some more digested mails hitting in the next week or two.

Samuel Ortiz

2011-05-22 20:00 UTC #14 lore raw

Re: [PATCH 06/14] mfd: pxa-w1: MFD driver for PXA 1wire control + DS1WM chip

In reply to Haojian Zhuang

Hi Haojian,

Quoted 18 lines · Haojian Zhuang

Quoted 8 lines · Samuel Ortiz
Hi Haojian,
Quoted 1 line · Haojian Zhuang
This driver provides registers and IRQ of PXA3xx chips to the ds1wm driver.
I see why this is needed, but I really think the ds1wm driver should be converted to a regular platform device. The MFD driver below is just adding a useless middle layer between your platform code and the ds1wm one.
I'm OK to use the regulator platform device for ds1wm driver except for one concern.
ds1wm driver is designed for mfd cell driver. I need to update it as regulator platform
driver first. cell->enable() / cell->disable() is used to enable clock for ds1wm driver.
So I need to change the API from cell->enable/disable to clk_enable() / clk_disable().

Quoted 8 lines

But the key issue is that there's no common structure for clkdev. Now clkdev is only
designed for deeply machine depend.
From my view is that we need a common structure for clkdev. Then mfd driver can get benefit from this. Some client driver can be written as regulator platform device.
And mfd driver can use them seamless. What's your opinion?

I may be missing your point, but what is preventing you from calling the clock API from drivers/w1/masters/ds1wm.c, the same way you're calling it from this patch ?

Cheers,
Samuel.

Signature · 2 lines

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

Haojian Zhuang

author

2011-04-18 14:11 UTC #15 lore raw

PATCH v2 7/14 mfd: fix build warning on 88pm860x

In reply to a message not in the archive

WARNING: vmlinux.o(.devinit.text+0x6c4): Section mismatch in reference from the function device_onkey_init() to the (unknown reference) .init.data:(unknown)
The function __devinit device_onkey_init() references a (unknown reference)
__initdata (unknown).
If (unknown) is only used by device_onkey_init then annotate (unknown) with a matching annotation.

It's caused by using __initdata on mfd cell resources. Replace __initdata with __devinitdata.

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

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

drivers/mfd/88pm860x-core.c +14 ~~-14~~

index f114ec7..4fbc69b 100644
@@ -21,13 +21,13 @@
 
 #define INT_STATUS_NUM			3
 
-static struct resource bk_resources[] __initdata = {
+static struct resource bk_resources[] __devinitdata = {
 	{PM8606_BACKLIGHT1, PM8606_BACKLIGHT1, "backlight-0", IORESOURCE_IO,},
 	{PM8606_BACKLIGHT2, PM8606_BACKLIGHT2, "backlight-1", IORESOURCE_IO,},
 	{PM8606_BACKLIGHT3, PM8606_BACKLIGHT3, "backlight-2", IORESOURCE_IO,},
 };
 
-static struct resource led_resources[] __initdata = {
+static struct resource led_resources[] __devinitdata = {
 	{PM8606_LED1_RED,   PM8606_LED1_RED,   "led0-red",   IORESOURCE_IO,},
 	{PM8606_LED1_GREEN, PM8606_LED1_GREEN, "led0-green", IORESOURCE_IO,},
 	{PM8606_LED1_BLUE,  PM8606_LED1_BLUE,  "led0-blue",  IORESOURCE_IO,},

@@ -36,7 +36,7 @@ static struct resource led_resources[] __initdata = {
 	{PM8606_LED2_BLUE,  PM8606_LED2_BLUE,  "led1-blue",  IORESOURCE_IO,},
 };
 
-static struct resource regulator_resources[] __initdata = {
+static struct resource regulator_resources[] __devinitdata = {
 	{PM8607_ID_BUCK1, PM8607_ID_BUCK1, "buck-1", IORESOURCE_IO,},
 	{PM8607_ID_BUCK2, PM8607_ID_BUCK2, "buck-2", IORESOURCE_IO,},
 	{PM8607_ID_BUCK3, PM8607_ID_BUCK3, "buck-3", IORESOURCE_IO,},

@@ -61,15 +61,15 @@ static struct resource rtc_resources[] __devinitdata = {
 	{PM8607_IRQ_RTC, PM8607_IRQ_RTC, "rtc", IORESOURCE_IRQ,},
 };
 
-static struct resource touch_resources[] __initdata = {
+static struct resource touch_resources[] __devinitdata = {
 	{PM8607_IRQ_PEN, PM8607_IRQ_PEN, "touch", IORESOURCE_IRQ,},
 };
 
-static struct resource onkey_resources[] __initdata = {
+static struct resource onkey_resources[] __devinitdata = {
 	{PM8607_IRQ_ONKEY, PM8607_IRQ_ONKEY, "onkey", IORESOURCE_IRQ,},
 };
 
-static struct resource codec_resources[] __initdata = {
+static struct resource codec_resources[] __devinitdata = {
 	/* Headset microphone insertion or removal */
 	{PM8607_IRQ_MICIN,   PM8607_IRQ_MICIN,   "micin",   IORESOURCE_IRQ,},
 	/* Hook-switch press or release */

@@ -80,12 +80,12 @@ static struct resource codec_resources[] __initdata = {
 	{PM8607_IRQ_AUDIO_SHORT, PM8607_IRQ_AUDIO_SHORT, "audio-short", IORESOURCE_IRQ,},
 };
 
-static struct resource battery_resources[] __initdata = {
+static struct resource battery_resources[] __devinitdata = {
 	{PM8607_IRQ_CC,  PM8607_IRQ_CC,  "columb counter", IORESOURCE_IRQ,},
 	{PM8607_IRQ_BAT, PM8607_IRQ_BAT, "battery",        IORESOURCE_IRQ,},
 };
 
-static struct resource charger_resources[] __initdata = {
+static struct resource charger_resources[] __devinitdata = {
 	{PM8607_IRQ_CHG,  PM8607_IRQ_CHG,  "charger detect",  IORESOURCE_IRQ,},
 	{PM8607_IRQ_CHG_DONE,  PM8607_IRQ_CHG_DONE,  "charging done",       IORESOURCE_IRQ,},
 	{PM8607_IRQ_CHG_FAULT, PM8607_IRQ_CHG_FAULT, "charging timeout",    IORESOURCE_IRQ,},

@@ -94,13 +94,13 @@ static struct resource charger_resources[] __initdata = {
 	{PM8607_IRQ_VCHG, PM8607_IRQ_VCHG, "vchg voltage",    IORESOURCE_IRQ,},
 };
 
-static struct mfd_cell bk_devs[] __initdata = {
+static struct mfd_cell bk_devs[] = {
 	{"88pm860x-backlight", 0,},
 	{"88pm860x-backlight", 1,},
 	{"88pm860x-backlight", 2,},
 };
 
-static struct mfd_cell led_devs[] __initdata = {
+static struct mfd_cell led_devs[] = {
 	{"88pm860x-led", 0,},
 	{"88pm860x-led", 1,},
 	{"88pm860x-led", 2,},

@@ -109,7 +109,7 @@ static struct mfd_cell led_devs[] __initdata = {
 	{"88pm860x-led", 5,},
 };
 
-static struct mfd_cell regulator_devs[] __initdata = {
+static struct mfd_cell regulator_devs[] = {
 	{"88pm860x-regulator", 0,},
 	{"88pm860x-regulator", 1,},
 	{"88pm860x-regulator", 2,},

@@ -134,15 +134,15 @@ static struct mfd_cell rtc_devs[] = {
 	{"88pm860x-rtc", -1,},
 };
 
-static struct mfd_cell touch_devs[] __initdata = {
+static struct mfd_cell touch_devs[] = {
 	{"88pm860x-touch", -1,},
 };
 
-static struct mfd_cell onkey_devs[] __initdata = {
+static struct mfd_cell onkey_devs[] = {
 	{"88pm860x-onkey", -1,},
 };
 
-static struct mfd_cell codec_devs[] __initdata = {
+static struct mfd_cell codec_devs[] = {
 	{"88pm860x-codec", -1,},
 };

Signature · 1 line

1.5.6.5

Samuel Ortiz

2011-04-26 09:49 UTC #16 lore raw

Re: [PATCH 07/14] mfd: fix build warning on 88pm860x

Hi Haojian,

Quoted 10 lines · Haojian Zhuang

WARNING: vmlinux.o(.devinit.text+0x6c4): Section mismatch in reference from the function device_onkey_init() to the (unknown reference) .init.data:(unknown)
The function __devinit device_onkey_init() references a (unknown reference)
__initdata (unknown).
If (unknown) is only used by device_onkey_init then annotate (unknown) with a matching annotation.
It's caused by using __initdata on mfd cell resources. Replace __initdata with __devinitdata.

Patch applied, many thanks.

Cheers,
Samuel.

Signature · 2 lines

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

Haojian Zhuang

2011-04-26 16:44 UTC #17 lore raw

Re: [PATCH 07/14] mfd: fix build warning on 88pm860x

Quoted 11 lines · Samuel Ortiz

Hi Haojian,
Quoted 8 lines · Haojian Zhuang
WARNING: vmlinux.o(.devinit.text+0x6c4): Section mismatch in reference from the function device_onkey_init() to the (unknown reference) .init.data:(unknown)
The function __devinit device_onkey_init() references a (unknown reference) __initdata (unknown).
If (unknown) is only used by device_onkey_init then annotate (unknown) with a matching annotation.

Quoted 4 lines

Quoted 2 lines
It's caused by using __initdata on mfd cell resources. Replace __initdata with __devinitdata.
Patch applied, many thanks.

Thanks a lot. But I can't find it in your mfd tree. Could you update your tree? Since
I need to rebase it and format new patches.

Best Regards
Haojian

Samuel Ortiz

2011-04-26 16:47 UTC #18 lore raw

Re: [PATCH 07/14] mfd: fix build warning on 88pm860x

Hi Haojian,

Quoted 12 lines · Haojian Zhuang

Quoted 11 lines · Samuel Ortiz
Hi Haojian,
Quoted 8 lines · Haojian Zhuang
WARNING: vmlinux.o(.devinit.text+0x6c4): Section mismatch in reference from the function device_onkey_init() to the (unknown reference) .init.data:(unknown)
The function __devinit device_onkey_init() references a (unknown reference) __initdata (unknown).
If (unknown) is only used by device_onkey_init then annotate (unknown) with a matching annotation.

Quoted 7 lines

Quoted 4 lines
Quoted 2 lines
It's caused by using __initdata on mfd cell resources. Replace __initdata with __devinitdata.
Patch applied, many thanks.
Thanks a lot. But I can't find it in your mfd tree. Could you update your tree? Since
I need to rebase it and format new patches.

I just pushed to kernel.org. It's going to be there soon.

Cheers,
Samuel.

Signature · 2 lines

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

Haojian Zhuang

author

2011-04-18 14:11 UTC #19 lore raw

PATCH v2 8/14 mfd: use platform_data in max8925

In reply to a message not in the archive

Use platform_data to pass parameters to client driver.

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

---
 drivers/mfd/max8925-core.c  |  467 +++++++++++++++++++++++++------------------
 include/linux/mfd/max8925.h |    9 +-
 2 files changed, 274 insertions(+), 202 deletions(-)

drivers/mfd/max8925-core.c +269 ~~-198~~

index 58cc5fd..044801c 100644
@@ -17,169 +17,141 @@
 #include <linux/platform_device.h>
 #include <linux/mfd/core.h>
 #include <linux/mfd/max8925.h>
+#include <linux/regulator/machine.h>
 
-static struct resource backlight_resources[] = {
-	{
-		.name	= "max8925-backlight",
-		.start	= MAX8925_WLED_MODE_CNTL,
-		.end	= MAX8925_WLED_CNTL,
-		.flags	= IORESOURCE_IO,
-	},
+static struct resource bk_resources[] __devinitdata = {
+	{0, 0, "max8925-backlight", IORESOURCE_IO,},
 };
 
-static struct mfd_cell backlight_devs[] = {
-	{
-		.name		= "max8925-backlight",
-		.num_resources	= 1,
-		.resources	= &backlight_resources[0],
-		.id		= -1,
-	},
+static struct resource touch_resources[] __devinitdata = {
+	{MAX8925_IRQ_TSC_STICK,  MAX8925_IRQ_TSC_STICK,  "touch stick",
+	 IORESOURCE_IRQ,},
+	{MAX8925_IRQ_TSC_NSTICK, MAX8925_IRQ_TSC_NSTICK, "touch nstick",
+	 IORESOURCE_IRQ,},
 };
 
-static struct resource touch_resources[] = {
-	{
-		.name	= "max8925-tsc",
-		.start	= MAX8925_TSC_IRQ,
-		.end	= MAX8925_ADC_RES_END,
-		.flags	= IORESOURCE_IO,
-	},
+static struct resource power_resources[] __devinitdata = {
+	{MAX8925_IRQ_VCHG_DC_OVP,    MAX8925_IRQ_VCHG_DC_OVP,		\
+	 "dc overvoltage",	IORESOURCE_IRQ,},
+	{MAX8925_IRQ_VCHG_DC_F,      MAX8925_IRQ_VCHG_DC_F,		\
+	 "dc falling",		IORESOURCE_IRQ,},
+	{MAX8925_IRQ_VCHG_DC_R,      MAX8925_IRQ_VCHG_DC_R,		\
+	 "dc rising",		IORESOURCE_IRQ,},
+	{MAX8925_IRQ_VCHG_USB_OVP,   MAX8925_IRQ_VCHG_USB_OVP,		\
+	 "usb overvoltage",	IORESOURCE_IRQ,},
+	{MAX8925_IRQ_VCHG_USB_F,     MAX8925_IRQ_VCHG_USB_F,		\
+	 "usb falling",		IORESOURCE_IRQ,},
+	{MAX8925_IRQ_VCHG_USB_R,     MAX8925_IRQ_VCHG_USB_R,		\
+	 "usb rising",		IORESOURCE_IRQ,},
+	{MAX8925_IRQ_VCHG_THM_OK_R,  MAX8925_IRQ_VCHG_THM_OK_R,		\
+	 "bat temp valid",	IORESOURCE_IRQ,},
+	{MAX8925_IRQ_VCHG_THM_OK_F,  MAX8925_IRQ_VCHG_THM_OK_F,		\
+	 "bat temp invalid",	IORESOURCE_IRQ,},
+	{MAX8925_IRQ_VCHG_SYSLOW_F, MAX8925_IRQ_VCHG_SYSLOW_F,		\
+	 "VSYSLOW falling",	IORESOURCE_IRQ,},
+	{MAX8925_IRQ_VCHG_SYSLOW_R, MAX8925_IRQ_VCHG_SYSLOW_R,		\
+	 "VSYSLOW rising", IORESOURCE_IRQ,},
+	{MAX8925_IRQ_VCHG_RST,       MAX8925_IRQ_VCHG_RST,		\
+	 "charger restart",	IORESOURCE_IRQ,},
+	{MAX8925_IRQ_VCHG_DONE,      MAX8925_IRQ_VCHG_DONE,		\
+	 "charger done",	IORESOURCE_IRQ,},
+	{MAX8925_IRQ_VCHG_TOPOFF,    MAX8925_IRQ_VCHG_TOPOFF,		\
+	 "charger topoff",	IORESOURCE_IRQ,},
+	{MAX8925_IRQ_VCHG_TMR_FAULT, MAX8925_IRQ_VCHG_TMR_FAULT,	\
+	 "charger expire",	IORESOURCE_IRQ,},
 };
 
-static struct mfd_cell touch_devs[] = {
-	{
-		.name		= "max8925-touch",
-		.num_resources	= 1,
-		.resources	= &touch_resources[0],
-		.id		= -1,
-	},
+static struct resource rtc_resources[] __devinitdata = {
+	{MAX8925_IRQ_RTC_ALARM0, MAX8925_IRQ_RTC_ALARM0, "rtc-alarm0",
+	 IORESOURCE_IRQ,},
+	{MAX8925_IRQ_RTC_ALARM1, MAX8925_IRQ_RTC_ALARM1, "rtc-alarm1",
+	 IORESOURCE_IRQ,},
 };
 
-static struct resource power_supply_resources[] = {
-	{
-		.name	= "max8925-power",
-		.start	= MAX8925_CHG_IRQ1,
-		.end	= MAX8925_CHG_IRQ1_MASK,
-		.flags	= IORESOURCE_IO,
-	},
+static struct resource onkey_resources[] __devinitdata = {
+	{MAX8925_IRQ_GPM_SW_R, MAX8925_IRQ_GPM_SW_R, "onkey R",
+	 IORESOURCE_IRQ,},
+	{MAX8925_IRQ_GPM_SW_F, MAX8925_IRQ_GPM_SW_F, "onkey F",
+	 IORESOURCE_IRQ,},
 };
 
-static struct mfd_cell power_devs[] = {
-	{
-		.name		= "max8925-power",
-		.num_resources	= 1,
-		.resources	= &power_supply_resources[0],
-		.id		= -1,
-	},
+static struct resource regulator_resources[] __devinitdata = {
+	{MAX8925_ID_SD1,   MAX8925_ID_SD1,   "SD1",   IORESOURCE_IO,},
+	{MAX8925_ID_SD2,   MAX8925_ID_SD2,   "SD2",   IORESOURCE_IO,},
+	{MAX8925_ID_SD3,   MAX8925_ID_SD3,   "SD3",   IORESOURCE_IO,},
+	{MAX8925_ID_LDO1,  MAX8925_ID_LDO1,  "LDO01", IORESOURCE_IO,},
+	{MAX8925_ID_LDO2,  MAX8925_ID_LDO2,  "LDO02", IORESOURCE_IO,},
+	{MAX8925_ID_LDO3,  MAX8925_ID_LDO3,  "LDO03", IORESOURCE_IO,},
+	{MAX8925_ID_LDO4,  MAX8925_ID_LDO4,  "LDO04", IORESOURCE_IO,},
+	{MAX8925_ID_LDO5,  MAX8925_ID_LDO5,  "LDO05", IORESOURCE_IO,},
+	{MAX8925_ID_LDO6,  MAX8925_ID_LDO6,  "LDO06", IORESOURCE_IO,},
+	{MAX8925_ID_LDO7,  MAX8925_ID_LDO7,  "LDO07", IORESOURCE_IO,},
+	{MAX8925_ID_LDO8,  MAX8925_ID_LDO8,  "LDO08", IORESOURCE_IO,},
+	{MAX8925_ID_LDO9,  MAX8925_ID_LDO9,  "LDO09", IORESOURCE_IO,},
+	{MAX8925_ID_LDO10, MAX8925_ID_LDO10, "LDO10", IORESOURCE_IO,},
+	{MAX8925_ID_LDO11, MAX8925_ID_LDO11, "LDO11", IORESOURCE_IO,},
+	{MAX8925_ID_LDO12, MAX8925_ID_LDO12, "LDO12", IORESOURCE_IO,},
+	{MAX8925_ID_LDO13, MAX8925_ID_LDO13, "LDO13", IORESOURCE_IO,},
+	{MAX8925_ID_LDO14, MAX8925_ID_LDO14, "LDO14", IORESOURCE_IO,},
+	{MAX8925_ID_LDO15, MAX8925_ID_LDO15, "LDO15", IORESOURCE_IO,},
+	{MAX8925_ID_LDO16, MAX8925_ID_LDO16, "LDO16", IORESOURCE_IO,},
+	{MAX8925_ID_LDO17, MAX8925_ID_LDO17, "LDO17", IORESOURCE_IO,},
+	{MAX8925_ID_LDO18, MAX8925_ID_LDO18, "LDO18", IORESOURCE_IO,},
+	{MAX8925_ID_LDO19, MAX8925_ID_LDO19, "LDO19", IORESOURCE_IO,},
+	{MAX8925_ID_LDO20, MAX8925_ID_LDO20, "LDO20", IORESOURCE_IO,},
 };
 
-static struct resource rtc_resources[] = {
-	{
-		.name	= "max8925-rtc",
-		.start	= MAX8925_RTC_IRQ,
-		.end	= MAX8925_RTC_IRQ_MASK,
-		.flags	= IORESOURCE_IO,
-	},
+static struct mfd_cell bk_devs[] = {
+	{"max8925-backlight", -1,},
 };
 
-static struct mfd_cell rtc_devs[] = {
-	{
-		.name		= "max8925-rtc",
-		.num_resources	= 1,
-		.resources	= &rtc_resources[0],
-		.id		= -1,
-	},
+static struct mfd_cell touch_devs[] = {
+	{"max8925-touch", -1,},
 };
 
-static struct resource onkey_resources[] = {
-	{
-		.name	= "max8925-onkey",
-		.start	= MAX8925_IRQ_GPM_SW_R,
-		.end	= MAX8925_IRQ_GPM_SW_R,
-		.flags	= IORESOURCE_IRQ,
-	}, {
-		.name	= "max8925-onkey",
-		.start	= MAX8925_IRQ_GPM_SW_F,
-		.end	= MAX8925_IRQ_GPM_SW_F,
-		.flags	= IORESOURCE_IRQ,
-	},
+static struct mfd_cell power_devs[] = {
+	{"max8925-power", -1,},
 };
 
-static struct mfd_cell onkey_devs[] = {
-	{
-		.name		= "max8925-onkey",
-		.num_resources	= 2,
-		.resources	= &onkey_resources[0],
-		.id		= -1,
-	},
+static struct mfd_cell rtc_devs[] = {
+	{"max8925-rtc", -1,},
 };
 
-#define MAX8925_REG_RESOURCE(_start, _end)	\
-{						\
-	.start	= MAX8925_##_start,		\
-	.end	= MAX8925_##_end,		\
-	.flags	= IORESOURCE_IO,		\
-}
-
-static struct resource regulator_resources[] = {
-	MAX8925_REG_RESOURCE(SDCTL1, SDCTL1),
-	MAX8925_REG_RESOURCE(SDCTL2, SDCTL2),
-	MAX8925_REG_RESOURCE(SDCTL3, SDCTL3),
-	MAX8925_REG_RESOURCE(LDOCTL1, LDOCTL1),
-	MAX8925_REG_RESOURCE(LDOCTL2, LDOCTL2),
-	MAX8925_REG_RESOURCE(LDOCTL3, LDOCTL3),
-	MAX8925_REG_RESOURCE(LDOCTL4, LDOCTL4),
-	MAX8925_REG_RESOURCE(LDOCTL5, LDOCTL5),
-	MAX8925_REG_RESOURCE(LDOCTL6, LDOCTL6),
-	MAX8925_REG_RESOURCE(LDOCTL7, LDOCTL7),
-	MAX8925_REG_RESOURCE(LDOCTL8, LDOCTL8),
-	MAX8925_REG_RESOURCE(LDOCTL9, LDOCTL9),
-	MAX8925_REG_RESOURCE(LDOCTL10, LDOCTL10),
-	MAX8925_REG_RESOURCE(LDOCTL11, LDOCTL11),
-	MAX8925_REG_RESOURCE(LDOCTL12, LDOCTL12),
-	MAX8925_REG_RESOURCE(LDOCTL13, LDOCTL13),
-	MAX8925_REG_RESOURCE(LDOCTL14, LDOCTL14),
-	MAX8925_REG_RESOURCE(LDOCTL15, LDOCTL15),
-	MAX8925_REG_RESOURCE(LDOCTL16, LDOCTL16),
-	MAX8925_REG_RESOURCE(LDOCTL17, LDOCTL17),
-	MAX8925_REG_RESOURCE(LDOCTL18, LDOCTL18),
-	MAX8925_REG_RESOURCE(LDOCTL19, LDOCTL19),
-	MAX8925_REG_RESOURCE(LDOCTL20, LDOCTL20),
+static struct mfd_cell onkey_devs[] = {
+	{"max8925-onkey", -1,},
 };
 
-#define MAX8925_REG_DEVS(_id)						\
-{									\
-	.name		= "max8925-regulator",				\
-	.num_resources	= 1,						\
-	.resources	= &regulator_resources[MAX8925_ID_##_id],	\
-	.id		= MAX8925_ID_##_id,				\
-}
-
 static struct mfd_cell regulator_devs[] = {
-	MAX8925_REG_DEVS(SD1),
-	MAX8925_REG_DEVS(SD2),
-	MAX8925_REG_DEVS(SD3),
-	MAX8925_REG_DEVS(LDO1),
-	MAX8925_REG_DEVS(LDO2),
-	MAX8925_REG_DEVS(LDO3),
-	MAX8925_REG_DEVS(LDO4),
-	MAX8925_REG_DEVS(LDO5),
-	MAX8925_REG_DEVS(LDO6),
-	MAX8925_REG_DEVS(LDO7),
-	MAX8925_REG_DEVS(LDO8),
-	MAX8925_REG_DEVS(LDO9),
-	MAX8925_REG_DEVS(LDO10),
-	MAX8925_REG_DEVS(LDO11),
-	MAX8925_REG_DEVS(LDO12),
-	MAX8925_REG_DEVS(LDO13),
-	MAX8925_REG_DEVS(LDO14),
-	MAX8925_REG_DEVS(LDO15),
-	MAX8925_REG_DEVS(LDO16),
-	MAX8925_REG_DEVS(LDO17),
-	MAX8925_REG_DEVS(LDO18),
-	MAX8925_REG_DEVS(LDO19),
-	MAX8925_REG_DEVS(LDO20),
+	{"max8925-regulator", 0,},
+	{"max8925-regulator", 1,},
+	{"max8925-regulator", 2,},
+	{"max8925-regulator", 3,},
+	{"max8925-regulator", 4,},
+	{"max8925-regulator", 5,},
+	{"max8925-regulator", 6,},
+	{"max8925-regulator", 7,},
+	{"max8925-regulator", 8,},
+	{"max8925-regulator", 9,},
+	{"max8925-regulator", 10,},
+	{"max8925-regulator", 11,},
+	{"max8925-regulator", 12,},
+	{"max8925-regulator", 13,},
+	{"max8925-regulator", 14,},
+	{"max8925-regulator", 15,},
+	{"max8925-regulator", 16,},
+	{"max8925-regulator", 17,},
+	{"max8925-regulator", 18,},
+	{"max8925-regulator", 19,},
+	{"max8925-regulator", 20,},
+	{"max8925-regulator", 21,},
+	{"max8925-regulator", 22,},
 };
 
+static struct max8925_backlight_pdata bk_pdata;
+static struct max8925_touch_pdata touch_pdata;
+static struct max8925_power_pdata power_pdata;
+static struct regulator_init_data regulator_pdata[ARRAY_SIZE(regulator_devs)];
+
 enum {
 	FLAGS_ADC = 1,	/* register in ADC component */
 	FLAGS_RTC,	/* register in RTC component */

@@ -588,6 +560,159 @@ tsc_irq:
 	return 0;
 }
 
+static void __devinit device_bk_init(struct max8925_chip *chip,
+				     struct i2c_client *i2c,
+				     struct max8925_platform_data *pdata)
+{
+	int ret;
+
+	if ((pdata == NULL) || (pdata->backlight == NULL))
+		return;
+
+	memcpy(&bk_pdata, pdata->backlight,
+		sizeof(struct max8925_backlight_pdata));
+	bk_devs[0].platform_data = &bk_pdata;
+	bk_devs[0].pdata_size = sizeof(bk_pdata);
+
+	bk_devs[0].num_resources = 1;
+	bk_devs[0].resources = &bk_resources[0];
+	ret = mfd_add_devices(chip->dev, 0, &bk_devs[0], 1,
+			      &bk_resources[0], 0);
+	if (ret < 0)
+		dev_err(chip->dev, "Failed to add backlight subdev\n");
+}
+
+static void __devinit device_rtc_init(struct max8925_chip *chip,
+				      struct i2c_client *i2c,
+				      struct max8925_platform_data *pdata)
+{
+	int ret;
+
+	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], 1,
+			      &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 max8925_chip *chip,
+					struct i2c_client *i2c,
+					struct max8925_platform_data *pdata)
+{
+	int ret;
+
+	if ((pdata == NULL) || (pdata->touch == NULL))
+		return;
+
+	memcpy(&touch_pdata, pdata->touch,
+		sizeof(struct max8925_touch_pdata));
+	touch_devs[0].platform_data = &touch_pdata;
+	touch_devs[0].pdata_size = sizeof(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],
+			      ARRAY_SIZE(touch_devs),
+			      &touch_resources[0], chip->irq_base);
+	if (ret < 0)
+		dev_err(chip->dev, "Failed to add touch subdev\n");
+}
+
+static void __devinit device_power_init(struct max8925_chip *chip,
+					struct i2c_client *i2c,
+					struct max8925_platform_data *pdata)
+{
+	int ret;
+
+	if ((pdata == NULL) || (pdata->power == NULL))
+		return;
+
+	memcpy(&power_pdata, pdata->power,
+		sizeof(struct max8925_power_pdata));
+	power_devs[0].platform_data = &power_pdata;
+	power_devs[0].pdata_size = sizeof(power_pdata);
+	power_devs[0].num_resources = ARRAY_SIZE(power_resources);
+	power_devs[0].resources = &power_resources[0];
+	ret = mfd_add_devices(chip->dev, 0, &power_devs[0],
+				ARRAY_SIZE(power_devs),
+				&power_resources[0], chip->irq_base);
+	if (ret < 0)
+		dev_err(chip->dev, "Failed to add power supply subdev\n");
+}
+
+static void __devinit device_onkey_init(struct max8925_chip *chip,
+					struct i2c_client *i2c,
+					struct max8925_platform_data *pdata)
+{
+	int ret;
+
+	onkey_devs[0].num_resources = ARRAY_SIZE(onkey_resources);
+	onkey_devs[0].resources = &onkey_resources[0];
+	ret = mfd_add_devices(chip->dev, 0, &onkey_devs[0],
+			      ARRAY_SIZE(onkey_devs),
+			      &onkey_resources[0], chip->irq_base);
+	if (ret < 0)
+		dev_err(chip->dev, "Failed to add onkey subdev\n");
+}
+
+static void __devinit device_regulator_init(struct max8925_chip *chip,
+					struct i2c_client *i2c,
+					struct max8925_platform_data *pdata)
+{
+	struct regulator_init_data *initdata;
+	int ret, i, j;
+
+	if ((pdata == NULL) || (pdata->regulator == NULL))
+		return;
+
+	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++) {
+		initdata = &pdata->regulator[i];
+		if (strstr(initdata->constraints.name, "SD")) {
+			sscanf(initdata->constraints.name, "SD%d", &j);
+			/* SD1 ~ SD3 */
+			if ((j < 1) || (j > 3)) {
+				dev_err(chip->dev, "Failed to add constraint "
+					"(%s)\n", initdata->constraints.name);
+				goto out;
+			}
+			j = (j - 1) + MAX8925_ID_SD1;
+		}
+		if (strstr(initdata->constraints.name, "LDO")) {
+			sscanf(initdata->constraints.name, "LDO%d", &j);
+			/* LDO01 ~ LDO20 */
+			if ((j < 1) || (j > 20)) {
+				dev_err(chip->dev, "Failed to add constraint "
+					"(%s)\n", initdata->constraints.name);
+				goto out;
+			}
+			j = (j - 1) + MAX8925_ID_LDO1;
+		}
+		if (j == -1) {
+			dev_err(chip->dev, "Failed to add constraint (%s)\n",
+				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].num_resources = 1;
+		regulator_devs[i].resources = &regulator_resources[j];
+
+		ret = mfd_add_devices(chip->dev, 0, &regulator_devs[i], 1,
+				      &regulator_resources[j], 0);
+		if (ret < 0) {
+			dev_err(chip->dev, "Failed to add regulator subdev\n");
+			goto out;
+		}
+	}
+out:
+	return;
+}
+
 int __devinit max8925_device_init(struct max8925_chip *chip,
 				  struct max8925_platform_data *pdata)
 {

@@ -604,75 +729,21 @@ int __devinit max8925_device_init(struct max8925_chip *chip,
 		do {
 			ret = max8925_reg_read(chip->adc, MAX8925_TSC_IRQ);
 		} while (ret & MAX8925_NREF_OK);
-		/* enaable ADC scheduler, interval is 1 second */
+		/* enable ADC scheduler, interval is 1 second */
 		max8925_set_bits(chip->adc, MAX8925_ADC_SCHED, 3, 2);
 	}
 
 	/* enable Momentary Power Loss */
 	max8925_set_bits(chip->rtc, MAX8925_MPL_CNTL, 1 << 4, 1 << 4);
 
-	ret = mfd_add_devices(chip->dev, 0, &rtc_devs[0],
-			      ARRAY_SIZE(rtc_devs),
-			      &rtc_resources[0], 0);
-	if (ret < 0) {
-		dev_err(chip->dev, "Failed to add rtc subdev\n");
-		goto out;
-	}
-
-	ret = mfd_add_devices(chip->dev, 0, &onkey_devs[0],
-			      ARRAY_SIZE(onkey_devs),
-			      &onkey_resources[0], 0);
-	if (ret < 0) {
-		dev_err(chip->dev, "Failed to add onkey subdev\n");
-		goto out_dev;
-	}
-
-	if (pdata && pdata->regulator[0]) {
-		ret = mfd_add_devices(chip->dev, 0, &regulator_devs[0],
-				      ARRAY_SIZE(regulator_devs),
-				      &regulator_resources[0], 0);
-		if (ret < 0) {
-			dev_err(chip->dev, "Failed to add regulator subdev\n");
-			goto out_dev;
-		}
-	}
-
-	if (pdata && pdata->backlight) {
-		ret = mfd_add_devices(chip->dev, 0, &backlight_devs[0],
-				      ARRAY_SIZE(backlight_devs),
-				      &backlight_resources[0], 0);
-		if (ret < 0) {
-			dev_err(chip->dev, "Failed to add backlight subdev\n");
-			goto out_dev;
-		}
-	}
-
-	if (pdata && pdata->power) {
-		ret = mfd_add_devices(chip->dev, 0, &power_devs[0],
-					ARRAY_SIZE(power_devs),
-					&power_supply_resources[0], 0);
-		if (ret < 0) {
-			dev_err(chip->dev, "Failed to add power supply "
-				"subdev\n");
-			goto out_dev;
-		}
-	}
-
-	if (pdata && pdata->touch) {
-		ret = mfd_add_devices(chip->dev, 0, &touch_devs[0],
-				      ARRAY_SIZE(touch_devs),
-				      &touch_resources[0], 0);
-		if (ret < 0) {
-			dev_err(chip->dev, "Failed to add touch subdev\n");
-			goto out_dev;
-		}
-	}
+	device_bk_init(chip, chip->i2c, pdata);
+	device_rtc_init(chip, chip->rtc, pdata);
+	device_touch_init(chip, chip->adc, pdata);
+	device_power_init(chip, chip->adc, pdata);
+	device_onkey_init(chip, chip->i2c, pdata);
+	device_regulator_init(chip, chip->i2c, pdata);
 
 	return 0;
-out_dev:
-	mfd_remove_devices(chip->dev);
-out:
-	return ret;
 }
 
 void __devexit max8925_device_exit(struct max8925_chip *chip)

include/linux/mfd/max8925.h +5 -4

index 5259dfe..51578e2 100644
@@ -209,9 +209,9 @@ struct max8925_chip {
 };
 
 struct max8925_backlight_pdata {
-	int	lxw_scl;	/* 0/1 -- 0.8Ohm/0.4Ohm */
-	int	lxw_freq;	/* 700KHz ~ 1400KHz */
-	int	dual_string;	/* 0/1 -- single/dual string */
+	int		lxw_scl;	/* 0/1 -- 0.8Ohm/0.4Ohm */
+	int		lxw_freq;	/* 700KHz ~ 1400KHz */
+	int		dual_string;	/* 0/1 -- single/dual string */
 };
 
 struct max8925_touch_pdata {

@@ -233,10 +233,11 @@ struct max8925_platform_data {
 	struct max8925_backlight_pdata	*backlight;
 	struct max8925_touch_pdata	*touch;
 	struct max8925_power_pdata	*power;
-	struct regulator_init_data	*regulator[MAX8925_MAX_REGULATOR];
+	struct regulator_init_data	*regulator;
 
 	int		irq_base;
 	int		tsc_irq;
+	int		num_regulators;
 };
 
 extern int max8925_reg_read(struct i2c_client *, int);

Signature · 1 line

1.5.6.5

Haojian Zhuang

author

2011-04-20 12:00 UTC #20 lore raw

PATCH v2 8/14 mfd: use platform_data in max8925

Use platform_data to pass parameters to client driver.

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

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

drivers/mfd/max8925-core.c +162 ~~-125~~

index 58cc5fd..a974711 100644
@@ -17,102 +17,88 @@
 #include <linux/platform_device.h>
 #include <linux/mfd/core.h>
 #include <linux/mfd/max8925.h>
+#include <linux/regulator/machine.h>
 
-static struct resource backlight_resources[] = {
-	{
-		.name	= "max8925-backlight",
-		.start	= MAX8925_WLED_MODE_CNTL,
-		.end	= MAX8925_WLED_CNTL,
-		.flags	= IORESOURCE_IO,
-	},
+static struct resource bk_resources[] __devinitdata = {
+	{0, 0, "max8925-backlight", IORESOURCE_IO,},
 };
 
-static struct mfd_cell backlight_devs[] = {
-	{
-		.name		= "max8925-backlight",
-		.num_resources	= 1,
-		.resources	= &backlight_resources[0],
-		.id		= -1,
-	},
+static struct resource touch_resources[] __devinitdata = {
+	{MAX8925_IRQ_TSC_STICK,  MAX8925_IRQ_TSC_STICK,  "touch stick",
+	 IORESOURCE_IRQ,},
+	{MAX8925_IRQ_TSC_NSTICK, MAX8925_IRQ_TSC_NSTICK, "touch nstick",
+	 IORESOURCE_IRQ,},
 };
 
-static struct resource touch_resources[] = {
-	{
-		.name	= "max8925-tsc",
-		.start	= MAX8925_TSC_IRQ,
-		.end	= MAX8925_ADC_RES_END,
-		.flags	= IORESOURCE_IO,
-	},
+static struct resource power_resources[] __devinitdata = {
+	{MAX8925_IRQ_VCHG_DC_OVP,    MAX8925_IRQ_VCHG_DC_OVP,		\
+	 "dc overvoltage",	IORESOURCE_IRQ,},
+	{MAX8925_IRQ_VCHG_DC_F,      MAX8925_IRQ_VCHG_DC_F,		\
+	 "dc falling",		IORESOURCE_IRQ,},
+	{MAX8925_IRQ_VCHG_DC_R,      MAX8925_IRQ_VCHG_DC_R,		\
+	 "dc rising",		IORESOURCE_IRQ,},
+	{MAX8925_IRQ_VCHG_USB_OVP,   MAX8925_IRQ_VCHG_USB_OVP,		\
+	 "usb overvoltage",	IORESOURCE_IRQ,},
+	{MAX8925_IRQ_VCHG_USB_F,     MAX8925_IRQ_VCHG_USB_F,		\
+	 "usb falling",		IORESOURCE_IRQ,},
+	{MAX8925_IRQ_VCHG_USB_R,     MAX8925_IRQ_VCHG_USB_R,		\
+	 "usb rising",		IORESOURCE_IRQ,},
+	{MAX8925_IRQ_VCHG_THM_OK_R,  MAX8925_IRQ_VCHG_THM_OK_R,		\
+	 "bat temp valid",	IORESOURCE_IRQ,},
+	{MAX8925_IRQ_VCHG_THM_OK_F,  MAX8925_IRQ_VCHG_THM_OK_F,		\
+	 "bat temp invalid",	IORESOURCE_IRQ,},
+	{MAX8925_IRQ_VCHG_SYSLOW_F, MAX8925_IRQ_VCHG_SYSLOW_F,		\
+	 "VSYSLOW falling",	IORESOURCE_IRQ,},
+	{MAX8925_IRQ_VCHG_SYSLOW_R, MAX8925_IRQ_VCHG_SYSLOW_R,		\
+	 "VSYSLOW rising", IORESOURCE_IRQ,},
+	{MAX8925_IRQ_VCHG_RST,       MAX8925_IRQ_VCHG_RST,		\
+	 "charger restart",	IORESOURCE_IRQ,},
+	{MAX8925_IRQ_VCHG_DONE,      MAX8925_IRQ_VCHG_DONE,		\
+	 "charger done",	IORESOURCE_IRQ,},
+	{MAX8925_IRQ_VCHG_TOPOFF,    MAX8925_IRQ_VCHG_TOPOFF,		\
+	 "charger topoff",	IORESOURCE_IRQ,},
+	{MAX8925_IRQ_VCHG_TMR_FAULT, MAX8925_IRQ_VCHG_TMR_FAULT,	\
+	 "charger expire",	IORESOURCE_IRQ,},
 };
 
-static struct mfd_cell touch_devs[] = {
-	{
-		.name		= "max8925-touch",
-		.num_resources	= 1,
-		.resources	= &touch_resources[0],
-		.id		= -1,
-	},
+static struct resource rtc_resources[] __devinitdata = {
+	{MAX8925_IRQ_RTC_ALARM0, MAX8925_IRQ_RTC_ALARM0, "rtc-alarm0",
+	 IORESOURCE_IRQ,},
+	{MAX8925_IRQ_RTC_ALARM1, MAX8925_IRQ_RTC_ALARM1, "rtc-alarm1",
+	 IORESOURCE_IRQ,},
 };
 
-static struct resource power_supply_resources[] = {
-	{
-		.name	= "max8925-power",
-		.start	= MAX8925_CHG_IRQ1,
-		.end	= MAX8925_CHG_IRQ1_MASK,
-		.flags	= IORESOURCE_IO,
-	},
+static struct resource onkey_resources[] __devinitdata = {
+	{MAX8925_IRQ_GPM_SW_R, MAX8925_IRQ_GPM_SW_R, "onkey R",
+	 IORESOURCE_IRQ,},
+	{MAX8925_IRQ_GPM_SW_F, MAX8925_IRQ_GPM_SW_F, "onkey F",
+	 IORESOURCE_IRQ,},
 };
 
-static struct mfd_cell power_devs[] = {
-	{
-		.name		= "max8925-power",
-		.num_resources	= 1,
-		.resources	= &power_supply_resources[0],
-		.id		= -1,
-	},
+static struct mfd_cell bk_devs[] = {
+	{"max8925-backlight", -1,},
 };
 
-static struct resource rtc_resources[] = {
-	{
-		.name	= "max8925-rtc",
-		.start	= MAX8925_RTC_IRQ,
-		.end	= MAX8925_RTC_IRQ_MASK,
-		.flags	= IORESOURCE_IO,
-	},
+static struct mfd_cell touch_devs[] = {
+	{"max8925-touch", -1,},
 };
 
-static struct mfd_cell rtc_devs[] = {
-	{
-		.name		= "max8925-rtc",
-		.num_resources	= 1,
-		.resources	= &rtc_resources[0],
-		.id		= -1,
-	},
+static struct mfd_cell power_devs[] = {
+	{"max8925-power", -1,},
 };
 
-static struct resource onkey_resources[] = {
-	{
-		.name	= "max8925-onkey",
-		.start	= MAX8925_IRQ_GPM_SW_R,
-		.end	= MAX8925_IRQ_GPM_SW_R,
-		.flags	= IORESOURCE_IRQ,
-	}, {
-		.name	= "max8925-onkey",
-		.start	= MAX8925_IRQ_GPM_SW_F,
-		.end	= MAX8925_IRQ_GPM_SW_F,
-		.flags	= IORESOURCE_IRQ,
-	},
+static struct mfd_cell rtc_devs[] = {
+	{"max8925-rtc", -1,},
 };
 
 static struct mfd_cell onkey_devs[] = {
-	{
-		.name		= "max8925-onkey",
-		.num_resources	= 2,
-		.resources	= &onkey_resources[0],
-		.id		= -1,
-	},
+	{"max8925-onkey", -1,},
 };
 
+static struct max8925_backlight_pdata bk_pdata;
+static struct max8925_touch_pdata touch_pdata;
+static struct max8925_power_pdata power_pdata;
+
 #define MAX8925_REG_RESOURCE(_start, _end)	\
 {						\
 	.start	= MAX8925_##_start,		\

@@ -588,6 +574,101 @@ tsc_irq:
 	return 0;
 }
 
+static void __devinit device_bk_init(struct max8925_chip *chip,
+				     struct i2c_client *i2c,
+				     struct max8925_platform_data *pdata)
+{
+	int ret;
+
+	if ((pdata == NULL) || (pdata->backlight == NULL))
+		return;
+
+	memcpy(&bk_pdata, pdata->backlight,
+		sizeof(struct max8925_backlight_pdata));
+	bk_devs[0].platform_data = &bk_pdata;
+	bk_devs[0].pdata_size = sizeof(bk_pdata);
+
+	bk_devs[0].num_resources = 1;
+	bk_devs[0].resources = &bk_resources[0];
+	ret = mfd_add_devices(chip->dev, 0, &bk_devs[0], 1,
+			      &bk_resources[0], 0);
+	if (ret < 0)
+		dev_err(chip->dev, "Failed to add backlight subdev\n");
+}
+
+static void __devinit device_rtc_init(struct max8925_chip *chip,
+				      struct i2c_client *i2c,
+				      struct max8925_platform_data *pdata)
+{
+	int ret;
+
+	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], 1,
+			      &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 max8925_chip *chip,
+					struct i2c_client *i2c,
+					struct max8925_platform_data *pdata)
+{
+	int ret;
+
+	if ((pdata == NULL) || (pdata->touch == NULL))
+		return;
+
+	memcpy(&touch_pdata, pdata->touch,
+		sizeof(struct max8925_touch_pdata));
+	touch_devs[0].platform_data = &touch_pdata;
+	touch_devs[0].pdata_size = sizeof(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],
+			      ARRAY_SIZE(touch_devs),
+			      &touch_resources[0], chip->irq_base);
+	if (ret < 0)
+		dev_err(chip->dev, "Failed to add touch subdev\n");
+}
+
+static void __devinit device_power_init(struct max8925_chip *chip,
+					struct i2c_client *i2c,
+					struct max8925_platform_data *pdata)
+{
+	int ret;
+
+	if ((pdata == NULL) || (pdata->power == NULL))
+		return;
+
+	memcpy(&power_pdata, pdata->power,
+		sizeof(struct max8925_power_pdata));
+	power_devs[0].platform_data = &power_pdata;
+	power_devs[0].pdata_size = sizeof(power_pdata);
+	power_devs[0].num_resources = ARRAY_SIZE(power_resources);
+	power_devs[0].resources = &power_resources[0];
+	ret = mfd_add_devices(chip->dev, 0, &power_devs[0],
+				ARRAY_SIZE(power_devs),
+				&power_resources[0], chip->irq_base);
+	if (ret < 0)
+		dev_err(chip->dev, "Failed to add power supply subdev\n");
+}
+
+static void __devinit device_onkey_init(struct max8925_chip *chip,
+					struct i2c_client *i2c,
+					struct max8925_platform_data *pdata)
+{
+	int ret;
+
+	onkey_devs[0].num_resources = ARRAY_SIZE(onkey_resources);
+	onkey_devs[0].resources = &onkey_resources[0];
+	ret = mfd_add_devices(chip->dev, 0, &onkey_devs[0],
+			      ARRAY_SIZE(onkey_devs),
+			      &onkey_resources[0], chip->irq_base);
+	if (ret < 0)
+		dev_err(chip->dev, "Failed to add onkey subdev\n");
+}
+
 int __devinit max8925_device_init(struct max8925_chip *chip,
 				  struct max8925_platform_data *pdata)
 {

@@ -604,28 +685,18 @@ int __devinit max8925_device_init(struct max8925_chip *chip,
 		do {
 			ret = max8925_reg_read(chip->adc, MAX8925_TSC_IRQ);
 		} while (ret & MAX8925_NREF_OK);
-		/* enaable ADC scheduler, interval is 1 second */
+		/* enable ADC scheduler, interval is 1 second */
 		max8925_set_bits(chip->adc, MAX8925_ADC_SCHED, 3, 2);
 	}
 
 	/* enable Momentary Power Loss */
 	max8925_set_bits(chip->rtc, MAX8925_MPL_CNTL, 1 << 4, 1 << 4);
 
-	ret = mfd_add_devices(chip->dev, 0, &rtc_devs[0],
-			      ARRAY_SIZE(rtc_devs),
-			      &rtc_resources[0], 0);
-	if (ret < 0) {
-		dev_err(chip->dev, "Failed to add rtc subdev\n");
-		goto out;
-	}
-
-	ret = mfd_add_devices(chip->dev, 0, &onkey_devs[0],
-			      ARRAY_SIZE(onkey_devs),
-			      &onkey_resources[0], 0);
-	if (ret < 0) {
-		dev_err(chip->dev, "Failed to add onkey subdev\n");
-		goto out_dev;
-	}
+	device_bk_init(chip, chip->i2c, pdata);
+	device_rtc_init(chip, chip->rtc, pdata);
+	device_touch_init(chip, chip->adc, pdata);
+	device_power_init(chip, chip->adc, pdata);
+	device_onkey_init(chip, chip->i2c, pdata);
 
 	if (pdata && pdata->regulator[0]) {
 		ret = mfd_add_devices(chip->dev, 0, &regulator_devs[0],

@@ -637,42 +708,8 @@ int __devinit max8925_device_init(struct max8925_chip *chip,
 		}
 	}
 
-	if (pdata && pdata->backlight) {
-		ret = mfd_add_devices(chip->dev, 0, &backlight_devs[0],
-				      ARRAY_SIZE(backlight_devs),
-				      &backlight_resources[0], 0);
-		if (ret < 0) {
-			dev_err(chip->dev, "Failed to add backlight subdev\n");
-			goto out_dev;
-		}
-	}
-
-	if (pdata && pdata->power) {
-		ret = mfd_add_devices(chip->dev, 0, &power_devs[0],
-					ARRAY_SIZE(power_devs),
-					&power_supply_resources[0], 0);
-		if (ret < 0) {
-			dev_err(chip->dev, "Failed to add power supply "
-				"subdev\n");
-			goto out_dev;
-		}
-	}
-
-	if (pdata && pdata->touch) {
-		ret = mfd_add_devices(chip->dev, 0, &touch_devs[0],
-				      ARRAY_SIZE(touch_devs),
-				      &touch_resources[0], 0);
-		if (ret < 0) {
-			dev_err(chip->dev, "Failed to add touch subdev\n");
-			goto out_dev;
-		}
-	}
-
-	return 0;
 out_dev:
-	mfd_remove_devices(chip->dev);
-out:
-	return ret;
+	return 0;
 }
 
 void __devexit max8925_device_exit(struct max8925_chip *chip)

Signature · 1 line

1.5.6.5

Samuel Ortiz

2011-04-26 09:45 UTC #21 lore raw

Re: [PATCH 08/14] mfd: use platform_data in max8925

Hi Haojian,

Quoted 1 line · Haojian Zhuang

Use platform_data to pass parameters to client driver.

You need to provide more details about why this is needed, and how the sub devices resources can change from one platform to another.

Also, some additional comments:

drivers/mfd/max8925-core.c line 24

Quoted 3 lines

drivers/mfd/max8925-core.c +2 -0

+static struct resource bk_resources[] __devinitdata = {
+	{0, 0, "max8925-backlight", IORESOURCE_IO,},
 };

Do you really need to define this empty resource ? Can't you do that from the device_*_init() routines ?

drivers/mfd/max8925-core.c tsc_irq: · line 579

Quoted 3 lines

drivers/mfd/max8925-core.c +3 -0

+static void __devinit device_bk_init(struct max8925_chip *chip,
+				     struct i2c_client *i2c,
+				     struct max8925_platform_data *pdata)

I don't think you need to pass the i2c pointer here.

Cheers,
Samuel.

Signature · 2 lines

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

Haojian Zhuang

author

2011-04-18 14:11 UTC #22 lore raw

PATCH v2 9/14 input: get irq from resource in max8925 onkey

In reply to a message not in the archive

Avoid to use hardcoding irq numbers in max8925 onkey driver. Get irq from resources instead.

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

Cc Dmitry Torokhov <[email protected]>

---
 drivers/input/misc/max8925_onkey.c |   36 ++++++++++++++++--------------------
 1 files changed, 16 insertions(+), 20 deletions(-)

drivers/input/misc/max8925_onkey.c +16 ~~-20~~

index 7de0ded..835cc2a 100644
@@ -69,18 +69,7 @@ static int __devinit max8925_onkey_probe(struct platform_device *pdev)
 {
 	struct max8925_chip *chip = dev_get_drvdata(pdev->dev.parent);
 	struct max8925_onkey_info *info;
-	int irq[2], error;
-
-	irq[0] = platform_get_irq(pdev, 0);
-	if (irq[0] < 0) {
-		dev_err(&pdev->dev, "No IRQ resource!\n");
-		return -EINVAL;
-	}
-	irq[1] = platform_get_irq(pdev, 1);
-	if (irq[1] < 0) {
-		dev_err(&pdev->dev, "No IRQ resource!\n");
-		return -EINVAL;
-	}
+	int error = -EINVAL;
 
 	info = kzalloc(sizeof(struct max8925_onkey_info), GFP_KERNEL);
 	if (!info)

@@ -88,21 +77,30 @@ static int __devinit max8925_onkey_probe(struct platform_device *pdev)
 
 	info->i2c = chip->i2c;
 	info->dev = &pdev->dev;
-	irq[0] += chip->irq_base;
-	irq[1] += chip->irq_base;
 
-	error = request_threaded_irq(irq[0], NULL, max8925_onkey_handler,
+	info->irq[0] = platform_get_irq(pdev, 0);
+	if (info->irq[0] < 0) {
+		dev_err(&pdev->dev, "No IRQ resource!\n");
+		goto out;
+	}
+	info->irq[1] = platform_get_irq(pdev, 1);
+	if (info->irq[1] < 0) {
+		dev_err(&pdev->dev, "No IRQ resource!\n");
+		goto out;
+	}
+
+	error = request_threaded_irq(info->irq[0], NULL, max8925_onkey_handler,
 				     IRQF_ONESHOT, "onkey-down", info);
 	if (error < 0) {
 		dev_err(chip->dev, "Failed to request IRQ: #%d: %d\n",
-			irq[0], error);
+			info->irq[0], error);
 		goto out;
 	}
-	error = request_threaded_irq(irq[1], NULL, max8925_onkey_handler,
+	error = request_threaded_irq(info->irq[1], NULL, max8925_onkey_handler,
 				     IRQF_ONESHOT, "onkey-up", info);
 	if (error < 0) {
 		dev_err(chip->dev, "Failed to request IRQ: #%d: %d\n",
-			irq[1], error);
+			info->irq[1], error);
 		goto out_irq;
 	}

@@ -117,8 +115,6 @@ static int __devinit max8925_onkey_probe(struct platform_device *pdev)
 	info->idev->phys = "max8925_on/input0";
 	info->idev->id.bustype = BUS_I2C;
 	info->idev->dev.parent = &pdev->dev;
-	info->irq[0] = irq[0];
-	info->irq[1] = irq[1];
 	info->idev->evbit[0] = BIT_MASK(EV_KEY);
 	info->idev->keybit[BIT_WORD(KEY_POWER)] = BIT_MASK(KEY_POWER);

Signature · 1 line

1.5.6.5

Haojian Zhuang

author

2011-04-18 14:11 UTC #23 lore raw

PATCH v2 10/14 rtc: avoid to use hardcoding irq number in max8925

In reply to a message not in the archive

Avoid to use hardcoding irq number in max8925 rtc driver. Use irq number from resources instead.

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

Cc Alessandro Zummo <[email protected]>

---
 drivers/rtc/rtc-max8925.c |   22 +++++++++++++---------
 1 files changed, 13 insertions(+), 9 deletions(-)

drivers/rtc/rtc-max8925.c +13 -9

index 174036d..c460152 100644
@@ -69,6 +69,7 @@ struct max8925_rtc_info {
 	struct max8925_chip	*chip;
 	struct i2c_client	*rtc;
 	struct device		*dev;
+	int			irq;
 };
 
 static irqreturn_t rtc_update_handler(int irq, void *data)

@@ -239,7 +240,7 @@ static int __devinit max8925_rtc_probe(struct platform_device *pdev)
 {
 	struct max8925_chip *chip = dev_get_drvdata(pdev->dev.parent);
 	struct max8925_rtc_info *info;
-	int irq, ret;
+	int ret = -EINVAL;
 
 	info = kzalloc(sizeof(struct max8925_rtc_info), GFP_KERNEL);
 	if (!info)

@@ -247,16 +248,22 @@ static int __devinit max8925_rtc_probe(struct platform_device *pdev)
 	info->chip = chip;
 	info->rtc = chip->rtc;
 	info->dev = &pdev->dev;
-	irq = chip->irq_base + MAX8925_IRQ_RTC_ALARM0;
+	info->irq = platform_get_irq(pdev, 0);
+	if (info->irq < 0) {
+		dev_err(chip->dev, "Failed to get IRQ resource\n");
+		goto out_irq;
+	}
 
-	ret = request_threaded_irq(irq, NULL, rtc_update_handler,
+	ret = request_threaded_irq(info->irq, NULL, rtc_update_handler,
 				   IRQF_ONESHOT, "rtc-alarm0", info);
 	if (ret < 0) {
 		dev_err(chip->dev, "Failed to request IRQ: #%d: %d\n",
-			irq, ret);
+			info->irq, ret);
 		goto out_irq;
 	}
 
+	dev_set_drvdata(&pdev->dev, info);
+	platform_set_drvdata(pdev, info);
 	info->rtc_dev = rtc_device_register("max8925-rtc", &pdev->dev,
 					&max8925_rtc_ops, THIS_MODULE);
 	ret = PTR_ERR(info->rtc_dev);

@@ -265,12 +272,9 @@ static int __devinit max8925_rtc_probe(struct platform_device *pdev)
 		goto out_rtc;
 	}
 
-	dev_set_drvdata(&pdev->dev, info);
-	platform_set_drvdata(pdev, info);
-
 	return 0;
 out_rtc:
-	free_irq(chip->irq_base + MAX8925_IRQ_RTC_ALARM0, info);
+	free_irq(info->irq, info);
 out_irq:
 	kfree(info);
 	return ret;

@@ -281,7 +285,7 @@ static int __devexit max8925_rtc_remove(struct platform_device *pdev)
 	struct max8925_rtc_info *info = platform_get_drvdata(pdev);
 
 	if (info) {
-		free_irq(info->chip->irq_base + MAX8925_IRQ_RTC_ALARM0, info);
+		free_irq(info->irq, info);
 		rtc_device_unregister(info->rtc_dev);
 		kfree(info);
 	}

Signature · 1 line

1.5.6.5

Haojian Zhuang

author

2011-04-18 14:11 UTC #24 lore raw

PATCH v2 11/14 power_supply: max8925: use platform_data from cell

In reply to a message not in the archive

Avoid to get platform_data from parent device. Get it from mfd cell device instead.

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

Cc Anton Vorontsov <[email protected]>

Cc David Woodhouse <[email protected]>

---
 drivers/power/max8925_power.c |   10 ++--------
 1 files changed, 2 insertions(+), 8 deletions(-)

drivers/power/max8925_power.c +2 -8

index 8e5aec2..a70e16d 100644
@@ -425,16 +425,11 @@ static __devexit int max8925_deinit_charger(struct max8925_power_info *info)
 static __devinit int max8925_power_probe(struct platform_device *pdev)
 {
 	struct max8925_chip *chip = dev_get_drvdata(pdev->dev.parent);
-	struct max8925_platform_data *max8925_pdata;
 	struct max8925_power_pdata *pdata = NULL;
 	struct max8925_power_info *info;
 	int ret;
 
-	if (pdev->dev.parent->platform_data) {
-		max8925_pdata = pdev->dev.parent->platform_data;
-		pdata = max8925_pdata->power;
-	}
-
+	pdata = pdev->dev.platform_data;
 	if (!pdata) {
 		dev_err(&pdev->dev, "platform data isn't assigned to "
 			"power supply\n");

@@ -447,6 +442,7 @@ static __devinit int max8925_power_probe(struct platform_device *pdev)
 	info->chip = chip;
 	info->gpm = chip->i2c;
 	info->adc = chip->adc;
+	platform_set_drvdata(pdev, info);
 
 	info->ac.name = "max8925-ac";
 	info->ac.type = POWER_SUPPLY_TYPE_MAINS;

@@ -482,8 +478,6 @@ static __devinit int max8925_power_probe(struct platform_device *pdev)
 	info->topoff_threshold = pdata->topoff_threshold;
 	info->fast_charge = pdata->fast_charge;
 	info->set_charger = pdata->set_charger;
-	dev_set_drvdata(&pdev->dev, info);
-	platform_set_drvdata(pdev, info);
 
 	max8925_init_charger(chip, info);
 	return 0;

Signature · 1 line

1.5.6.5

Anton Vorontsov

Acked-by on v2 11/14

2011-04-18 21:49 UTC #25 lore raw

Re: [PATCH 11/14] power_supply: max8925: use platform_data from cell

Quoted 7 lines · Haojian Zhuang

Avoid to get platform_data from parent device. Get it from mfd cell device instead.
Signed-off-by Haojian Zhuang <[email protected]>
Cc Anton Vorontsov <[email protected]>
Cc David Woodhouse <[email protected]>
---

Acked-by Anton Vorontsov <[email protected]>

Thanks,

Signature · 2 lines

Anton Vorontsov
Email: [email protected]

Samuel Ortiz

2011-05-02 15:15 UTC #26 lore raw

Re: [PATCH 11/14] power_supply: max8925: use platform_data from cell

Hi Haojian, Anton,

Quoted 10 lines · Anton Vorontsov

Quoted 7 lines · Haojian Zhuang
Avoid to get platform_data from parent device. Get it from mfd cell device instead.
Signed-off-by Haojian Zhuang <[email protected]>
Cc Anton Vorontsov <[email protected]>
Cc David Woodhouse <[email protected]>
---
Acked-by Anton Vorontsov <[email protected]>

Thanks, patch applied to my for-next branch.

Cheers,
Samuel.

Signature · 2 lines

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

Haojian Zhuang

author

2011-04-18 14:11 UTC #27 lore raw

PATCH v2 12/14 regulator: max8925: enable i2c sequence for control

In reply to a message not in the archive

Force to enable i2c as power up/down sequence. Otherwise, SD/LDO can't be enabled or disabled via accessing i2c bus.

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

Cc Liam Girdwood <[email protected]>

Cc Mark Brown <[email protected]>

---
 drivers/regulator/max8925-regulator.c |   11 ++++++++---
 1 files changed, 8 insertions(+), 3 deletions(-)

drivers/regulator/max8925-regulator.c +8 -3

index 8ae1475..e4dbd66 100644
@@ -23,6 +23,10 @@
 #define SD1_DVM_SHIFT		5		/* SDCTL1 bit5 */
 #define SD1_DVM_EN		6		/* SDV1 bit 6 */
 
+/* bit definitions in SD & LDO control registers */
+#define OUT_ENABLE   		0x1f		/* Power U/D sequence as I2C */
+#define OUT_DISABLE		0x1e		/* Power U/D sequence as I2C */
+
 struct max8925_regulator_info {
 	struct regulator_desc	desc;
 	struct regulator_dev	*regulator;

@@ -93,8 +97,8 @@ static int max8925_enable(struct regulator_dev *rdev)
 	struct max8925_regulator_info *info = rdev_get_drvdata(rdev);
 
 	return max8925_set_bits(info->i2c, info->enable_reg,
-				1 << info->enable_bit,
-				1 << info->enable_bit);
+				OUT_ENABLE << info->enable_bit,
+				OUT_ENABLE << info->enable_bit);
 }
 
 static int max8925_disable(struct regulator_dev *rdev)

@@ -102,7 +106,8 @@ static int max8925_disable(struct regulator_dev *rdev)
 	struct max8925_regulator_info *info = rdev_get_drvdata(rdev);
 
 	return max8925_set_bits(info->i2c, info->enable_reg,
-				1 << info->enable_bit, 0);
+				OUT_ENABLE << info->enable_bit,
+				OUT_DISABLE << info->enable_bit);
 }
 
 static int max8925_is_enabled(struct regulator_dev *rdev)

Signature · 1 line

1.5.6.5

Mark Brown

Acked-by on v2 12/14

2011-04-18 14:27 UTC #28 lore raw

Re: [PATCH 12/14] regulator: max8925: enable i2c sequence for control

Quoted 2 lines · Haojian Zhuang

Force to enable i2c as power up/down sequence. Otherwise, SD/LDO can't be enabled or disabled via accessing i2c bus.

Acked-by Mark Brown <[email protected]>

but please work on the quality of your changelogs.

Samuel Ortiz

2011-05-02 15:16 UTC #29 lore raw

Re: [PATCH 12/14] regulator: max8925: enable i2c sequence for control

In reply to Haojian Zhuang

Hi Haojian,

Quoted 3 lines · Haojian Zhuang

Force to enable i2c as power up/down sequence. Otherwise, SD/LDO can't be enabled or disabled via accessing i2c bus.

Patch applied to my for-next branch, thanks.

Cheers,
Samuel.

Signature · 2 lines

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

Haojian Zhuang

author

2011-04-18 14:10 UTC #30 lore raw

PATCH v2 13/14 regulator: check name in initialization of max8925

In reply to a message not in the archive

Check name in initialization of max8925 regulator driver.

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

Cc Liam Girdwood <[email protected]>

Cc Mark Brown <[email protected]>

---
 drivers/regulator/max8925-regulator.c |   39 ++++++++++++++++-----------------
 1 files changed, 19 insertions(+), 20 deletions(-)

drivers/regulator/max8925-regulator.c +19 ~~-20~~

index e4dbd66..7ce8147 100644
@@ -174,7 +174,7 @@ static struct regulator_ops max8925_regulator_ldo_ops = {
 #define MAX8925_SDV(_id, min, max, step)			\
 {								\
 	.desc	= {						\
-		.name	= "SDV" #_id,				\
+		.name	= "SD" #_id,				\
 		.ops	= &max8925_regulator_sdv_ops,		\
 		.type	= REGULATOR_VOLTAGE,			\
 		.id	= MAX8925_ID_SD##_id,			\

@@ -236,39 +236,38 @@ static struct max8925_regulator_info max8925_regulator_info[] = {
 	MAX8925_LDO(20, 750, 3900, 50),
 };
 
-static struct max8925_regulator_info * __devinit find_regulator_info(int id)
+static int __devinit max8925_regulator_probe(struct platform_device *pdev)
 {
-	struct max8925_regulator_info *ri;
+	struct max8925_chip *chip = dev_get_drvdata(pdev->dev.parent);
+	struct max8925_regulator_info *ri = NULL;
+	struct regulator_init_data *pdata;
+	struct regulator_dev *rdev;
 	int i;
 
+	pdata = pdev->dev.platform_data;
+	if ((pdata == NULL) || (pdata->driver_data == NULL))
+		return -EINVAL;
+
 	for (i = 0; i < ARRAY_SIZE(max8925_regulator_info); i++) {
 		ri = &max8925_regulator_info[i];
-		if (ri->desc.id == id)
-			return ri;
+		/* pdata->driver_data stores the name of regulator */
+		if (!strcmp(ri->desc.name, pdata->driver_data))
+			break;
 	}
-	return NULL;
-}
-
-static int __devinit max8925_regulator_probe(struct platform_device *pdev)
-{
-	struct max8925_chip *chip = dev_get_drvdata(pdev->dev.parent);
-	struct max8925_platform_data *pdata = chip->dev->platform_data;
-	struct max8925_regulator_info *ri;
-	struct regulator_dev *rdev;
-
-	ri = find_regulator_info(pdev->id);
-	if (ri == NULL) {
-		dev_err(&pdev->dev, "invalid regulator ID specified\n");
+	if (i > ARRAY_SIZE(max8925_regulator_info)) {
+		dev_err(&pdev->dev, "Failed to find regulator %s\n",
+			pdata->constraints.name);
 		return -EINVAL;
 	}
 	ri->i2c = chip->i2c;
 	ri->chip = chip;
 
+	/* replace driver_data with ri */
 	rdev = regulator_register(&ri->desc, &pdev->dev,
-				  pdata->regulator[pdev->id], ri);
+				  pdata, ri);
 	if (IS_ERR(rdev)) {
 		dev_err(&pdev->dev, "failed to register regulator %s\n",
-				ri->desc.name);
+			ri->desc.name);
 		return PTR_ERR(rdev);
 	}

Signature · 1 line

1.5.6.5

Mark Brown

2011-04-18 14:29 UTC #31 lore raw

Re: [PATCH 13/14] regulator: check name in initialization of max8925

Quoted 1 line · Haojian Zhuang

Check name in initialization of max8925 regulator driver.

What name are we checking and why do we need to check it? I've no idea what the patch is supposed to do which makes it hard to review.

drivers/regulator/max8925-regulator.c static struct regulator_ops max8925_regulator_ldo_ops = { · line 177

Quoted 7 lines

+++ b/drivers/regulator/max8925-regulator.c

drivers/regulator/max8925-regulator.c +1 -1

@@ -174,7 +174,7 @@ static struct regulator_ops max8925_regulator_ldo_ops = {
 #define MAX8925_SDV(_id, min, max, step)			\
 {								\
 	.desc	= {						\
-		.name	= "SDV" #_id,				\
+		.name	= "SD" #_id,				\

The above isn't obviously correct - it changes the name that's assigned from matching the macro used to something different.

Haojian Zhuang

2011-04-18 15:33 UTC #32 lore raw

Re: [PATCH 13/14] regulator: check name in initialization of max8925

Quoted 6 lines · Mark Brown

Quoted 1 line · Haojian Zhuang
Check name in initialization of max8925 regulator driver.
What name are we checking and why do we need to check it? I've no idea what the patch is supposed to do which makes it hard to review.

Actually, I didn't submit any platform driver before. So I can't attach the related patch on
platform driver. After 2.6.39, I'll upload patches on platform driver.

In this original design, regulator data is assigned in platform driver separately with index.
If I missed to define regulator[0] in platform data, the regulator driver will meet failure
because of checking in max8925-core.c. The regulator[0] always means Buck0.

So I want to avoid to use the index and check regulator[] one by one. I use a pointer to link
all regulator data together. I just need to check whether the regulator pointer is valid or not.

Quoted 11 lines

drivers/regulator/max8925-regulator.c static struct regulator_ops max8925_regulator_ldo_ops = { · line 177
Quoted 7 lines
+++ b/drivers/regulator/max8925-regulator.c
drivers/regulator/max8925-regulator.c +1 -1
@@ -174,7 +174,7 @@ static struct regulator_ops max8925_regulator_ldo_ops = {
  #define MAX8925_SDV(_id, min, max, step)                     \
  {                                                            \
       .desc   = {                                             \
-             .name   = "SDV" #_id,                           \
+             .name   = "SD" #_id,                            \
The above isn't obviously correct - it changes the name that's assigned from matching the macro used to something different.

The name isn't used by others. So I change it to SDx in order to compare regulator name.

Mark Brown

2011-04-18 16:04 UTC #33 lore raw

Re: [PATCH 13/14] regulator: check name in initialization of max8925

Quoted 4 lines · Haojian Zhuang

So I want to avoid to use the index and check regulator[] one by one. I use a pointer to link
all regulator data together. I just need to check whether the regulator pointer is valid or not.

Why? I don't understand what the goal of this change is.

Haojian Zhuang

2011-04-19 02:46 UTC #34 lore raw

RE: [PATCH 13/14] regulator: check name in initialization of max8925

Quoted 12 lines

-----Original Message-----
From: Mark Brown [mailto:[email protected]]
Sent: 2011年4月19日 12:04 AM
To: Haojian Zhuang
Cc: Haojian Zhuang; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH 13/14] regulator: check name in initialization of max8925

Quoted 8 lines

Quoted 4 lines · Haojian Zhuang
So I want to avoid to use the index and check regulator[] one by one. I use a pointer to link
all regulator data together. I just need to check whether the regulator pointer is valid or not.
Why? I don't understand what the goal of this change is.

Original implementation:

	Max8925_core.c:
		If ((pdata == NULL) || (pdata->regulator[0] == NULL))
			Return -EINVAL;
	Machine driver:
		Platform data:
			.regulator[0]	= xxx
			.regulator[1]	= xxx

	The index of regulator array is id of buck and ldo.
	There's issue in max8925_core.c since we can't assume regulator[0] always declared in machine driver.

Current implementation:

	Max8925_core.c:
		If ((pdata == NULL) || (pdata->regulator == NULL))
			Return -EINVAL;
	Machine driver:
		Platform data:
			Use two parameters: num_regulators and regulator pointer. The index of new regulator array isn't id of buck and ldo any more.

�翳簕.n�+壏煯壄�+%娝�遍荻w�簕.n�+壏{炳G珴�妠ay�蕠跈�,jf＂穐殢飦�戧鐉_璁(殠娸"濟�m��G珴�⒏?櫒璀�&ｘ忈秈O曟瑉窔v豝�m��鹅�⒏?朓�

Mark Brown

2011-04-19 08:09 UTC #35 lore raw

Re: [PATCH 13/14] regulator: check name in initialization of max8925

Quoted 7 lines · Haojian Zhuang

	Machine driver:
		Platform data:
			.regulator[0]	= xxx
			.regulator[1]	= xxx

	The index of regulator array is id of buck and ldo.
	There's issue in max8925_core.c since we can't assume regulator[0] always declared in machine driver.

What is the issue? It's trivial to skip the regulator if the data is null.

Haojian Zhuang

2011-04-19 08:30 UTC #36 lore raw

RE: [PATCH 13/14] regulator: check name in initialization of max8925

Quoted 18 lines

-----Original Message-----
From: Mark Brown [mailto:[email protected]]
Sent: 2011年4月19日 4:09 PM
To: Haojian Zhuang
Cc: Haojian Zhuang; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH 13/14] regulator: check name in initialization of max8925
Quoted 4 lines · Haojian Zhuang
	Machine driver:
		Platform data:
			.regulator[0]	= xxx
			.regulator[1]	= xxx

Quoted 6 lines

Quoted 2 lines
The index of regulator array is id of buck and ldo. There's issue in max8925_core.c since we can't assume regulator[0]
always declared in machine driver.
What is the issue? It's trivial to skip the regulator if the data is null.

It skiped all regulators if regulator[0] isn't decleared.

	(pdata == NULL) || (pdata->regulator[0] == NULL)

So it's a bug. �翳簕.n�+壏煯壄�+%娝�遍荻w�簕.n�+壏{炳G珴�妠ay�蕠跈�,jf＂穐殢飦�戧鐉_璁(殠娸"濟�m��G珴�⒏?櫒璀�&ｘ忈秈O曟瑉窔v豝�m��鹅�⒏?朓�

Mark Brown

2011-04-19 10:51 UTC #37 lore raw

Re: [PATCH 13/14] regulator: check name in initialization of max8925

Quoted 10 lines · Haojian Zhuang

Quoted 5 lines
Quoted 1 line
There's issue in max8925_core.c since we can't assume regulator[0]
always declared in machine driver.
What is the issue? It's trivial to skip the regulator if the data is null.
It skiped all regulators if regulator[0] isn't decleared.
	(pdata == NULL) || (pdata->regulator[0] == NULL)
So it's a bug.

Surely the obvious fix is to fix the issue and not skip all the regulators if an individual regulator has no data?

Haojian Zhuang

2011-04-19 11:35 UTC #38 lore raw

RE: [PATCH 13/14] regulator: check name in initialization of max8925

Quoted 20 lines

-----Original Message-----
From: Mark Brown [mailto:[email protected]]
Sent: 2011年4月19日 6:51 PM
To: Haojian Zhuang
Cc: Haojian Zhuang; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH 13/14] regulator: check name in initialization of max8925
Quoted 5 lines · Haojian Zhuang
Quoted 5 lines
Quoted 1 line
There's issue in max8925_core.c since we can't assume regulator[0]
always declared in machine driver.
What is the issue? It's trivial to skip the regulator if the data is null.

Quoted 7 lines

Quoted 4 lines
It skiped all regulators if regulator[0] isn't decleared.
	(pdata == NULL) || (pdata->regulator[0] == NULL)
So it's a bug.
Surely the obvious fix is to fix the issue and not skip all the regulators if an individual regulator has no data?

Yes. �翳簕.n�+壏煯壄�+%娝�遍荻w�簕.n�+壏{炳G珴�妠ay�蕠跈�,jf＂穐殢飦�戧鐉_璁(殠娸"濟�m��G珴�⒏?櫒璀�&ｘ忈秈O曟瑉窔v豝�m��鹅�⒏?朓�

Haojian Zhuang

author

2011-04-20 12:01 UTC #39 lore raw

PATCH v2 13/14 regulator: max8925: fix not add device if missing init data

In reply to Haojian Zhuang

If regulator[0] is missed in init data, all regulators of max8925 won't be initialized.

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

Cc Liam Girdwood <[email protected]>

Cc Mark Brown <[email protected]>

---
 drivers/mfd/max8925-core.c            |  171 ++++++++++++++++++---------------
 drivers/regulator/max8925-regulator.c |   37 ++++----
 include/linux/mfd/max8925.h           |    3 +-
 3 files changed, 112 insertions(+), 99 deletions(-)

drivers/mfd/max8925-core.c +93 ~~-78~~

index a974711..93f2731 100644
@@ -75,6 +75,32 @@ static struct resource onkey_resources[] __devinitdata = {
 	 IORESOURCE_IRQ,},
 };
 
+static struct resource regulator_resources[] __devinitdata = {
+	{MAX8925_ID_SD1,   MAX8925_ID_SD1,   "SD1",   IORESOURCE_IO,},
+	{MAX8925_ID_SD2,   MAX8925_ID_SD2,   "SD2",   IORESOURCE_IO,},
+	{MAX8925_ID_SD3,   MAX8925_ID_SD3,   "SD3",   IORESOURCE_IO,},
+	{MAX8925_ID_LDO1,  MAX8925_ID_LDO1,  "LDO01", IORESOURCE_IO,},
+	{MAX8925_ID_LDO2,  MAX8925_ID_LDO2,  "LDO02", IORESOURCE_IO,},
+	{MAX8925_ID_LDO3,  MAX8925_ID_LDO3,  "LDO03", IORESOURCE_IO,},
+	{MAX8925_ID_LDO4,  MAX8925_ID_LDO4,  "LDO04", IORESOURCE_IO,},
+	{MAX8925_ID_LDO5,  MAX8925_ID_LDO5,  "LDO05", IORESOURCE_IO,},
+	{MAX8925_ID_LDO6,  MAX8925_ID_LDO6,  "LDO06", IORESOURCE_IO,},
+	{MAX8925_ID_LDO7,  MAX8925_ID_LDO7,  "LDO07", IORESOURCE_IO,},
+	{MAX8925_ID_LDO8,  MAX8925_ID_LDO8,  "LDO08", IORESOURCE_IO,},
+	{MAX8925_ID_LDO9,  MAX8925_ID_LDO9,  "LDO09", IORESOURCE_IO,},
+	{MAX8925_ID_LDO10, MAX8925_ID_LDO10, "LDO10", IORESOURCE_IO,},
+	{MAX8925_ID_LDO11, MAX8925_ID_LDO11, "LDO11", IORESOURCE_IO,},
+	{MAX8925_ID_LDO12, MAX8925_ID_LDO12, "LDO12", IORESOURCE_IO,},
+	{MAX8925_ID_LDO13, MAX8925_ID_LDO13, "LDO13", IORESOURCE_IO,},
+	{MAX8925_ID_LDO14, MAX8925_ID_LDO14, "LDO14", IORESOURCE_IO,},
+	{MAX8925_ID_LDO15, MAX8925_ID_LDO15, "LDO15", IORESOURCE_IO,},
+	{MAX8925_ID_LDO16, MAX8925_ID_LDO16, "LDO16", IORESOURCE_IO,},
+	{MAX8925_ID_LDO17, MAX8925_ID_LDO17, "LDO17", IORESOURCE_IO,},
+	{MAX8925_ID_LDO18, MAX8925_ID_LDO18, "LDO18", IORESOURCE_IO,},
+	{MAX8925_ID_LDO19, MAX8925_ID_LDO19, "LDO19", IORESOURCE_IO,},
+	{MAX8925_ID_LDO20, MAX8925_ID_LDO20, "LDO20", IORESOURCE_IO,},
+};
+
 static struct mfd_cell bk_devs[] = {
 	{"max8925-backlight", -1,},
 };

@@ -95,76 +121,36 @@ static struct mfd_cell onkey_devs[] = {
 	{"max8925-onkey", -1,},
 };
 
+static struct mfd_cell regulator_devs[] = {
+	{"max8925-regulator", 0,},
+	{"max8925-regulator", 1,},
+	{"max8925-regulator", 2,},
+	{"max8925-regulator", 3,},
+	{"max8925-regulator", 4,},
+	{"max8925-regulator", 5,},
+	{"max8925-regulator", 6,},
+	{"max8925-regulator", 7,},
+	{"max8925-regulator", 8,},
+	{"max8925-regulator", 9,},
+	{"max8925-regulator", 10,},
+	{"max8925-regulator", 11,},
+	{"max8925-regulator", 12,},
+	{"max8925-regulator", 13,},
+	{"max8925-regulator", 14,},
+	{"max8925-regulator", 15,},
+	{"max8925-regulator", 16,},
+	{"max8925-regulator", 17,},
+	{"max8925-regulator", 18,},
+	{"max8925-regulator", 19,},
+	{"max8925-regulator", 20,},
+	{"max8925-regulator", 21,},
+	{"max8925-regulator", 22,},
+};
+
 static struct max8925_backlight_pdata bk_pdata;
 static struct max8925_touch_pdata touch_pdata;
 static struct max8925_power_pdata power_pdata;
-
-#define MAX8925_REG_RESOURCE(_start, _end)	\
-{						\
-	.start	= MAX8925_##_start,		\
-	.end	= MAX8925_##_end,		\
-	.flags	= IORESOURCE_IO,		\
-}
-
-static struct resource regulator_resources[] = {
-	MAX8925_REG_RESOURCE(SDCTL1, SDCTL1),
-	MAX8925_REG_RESOURCE(SDCTL2, SDCTL2),
-	MAX8925_REG_RESOURCE(SDCTL3, SDCTL3),
-	MAX8925_REG_RESOURCE(LDOCTL1, LDOCTL1),
-	MAX8925_REG_RESOURCE(LDOCTL2, LDOCTL2),
-	MAX8925_REG_RESOURCE(LDOCTL3, LDOCTL3),
-	MAX8925_REG_RESOURCE(LDOCTL4, LDOCTL4),
-	MAX8925_REG_RESOURCE(LDOCTL5, LDOCTL5),
-	MAX8925_REG_RESOURCE(LDOCTL6, LDOCTL6),
-	MAX8925_REG_RESOURCE(LDOCTL7, LDOCTL7),
-	MAX8925_REG_RESOURCE(LDOCTL8, LDOCTL8),
-	MAX8925_REG_RESOURCE(LDOCTL9, LDOCTL9),
-	MAX8925_REG_RESOURCE(LDOCTL10, LDOCTL10),
-	MAX8925_REG_RESOURCE(LDOCTL11, LDOCTL11),
-	MAX8925_REG_RESOURCE(LDOCTL12, LDOCTL12),
-	MAX8925_REG_RESOURCE(LDOCTL13, LDOCTL13),
-	MAX8925_REG_RESOURCE(LDOCTL14, LDOCTL14),
-	MAX8925_REG_RESOURCE(LDOCTL15, LDOCTL15),
-	MAX8925_REG_RESOURCE(LDOCTL16, LDOCTL16),
-	MAX8925_REG_RESOURCE(LDOCTL17, LDOCTL17),
-	MAX8925_REG_RESOURCE(LDOCTL18, LDOCTL18),
-	MAX8925_REG_RESOURCE(LDOCTL19, LDOCTL19),
-	MAX8925_REG_RESOURCE(LDOCTL20, LDOCTL20),
-};
-
-#define MAX8925_REG_DEVS(_id)						\
-{									\
-	.name		= "max8925-regulator",				\
-	.num_resources	= 1,						\
-	.resources	= &regulator_resources[MAX8925_ID_##_id],	\
-	.id		= MAX8925_ID_##_id,				\
-}
-
-static struct mfd_cell regulator_devs[] = {
-	MAX8925_REG_DEVS(SD1),
-	MAX8925_REG_DEVS(SD2),
-	MAX8925_REG_DEVS(SD3),
-	MAX8925_REG_DEVS(LDO1),
-	MAX8925_REG_DEVS(LDO2),
-	MAX8925_REG_DEVS(LDO3),
-	MAX8925_REG_DEVS(LDO4),
-	MAX8925_REG_DEVS(LDO5),
-	MAX8925_REG_DEVS(LDO6),
-	MAX8925_REG_DEVS(LDO7),
-	MAX8925_REG_DEVS(LDO8),
-	MAX8925_REG_DEVS(LDO9),
-	MAX8925_REG_DEVS(LDO10),
-	MAX8925_REG_DEVS(LDO11),
-	MAX8925_REG_DEVS(LDO12),
-	MAX8925_REG_DEVS(LDO13),
-	MAX8925_REG_DEVS(LDO14),
-	MAX8925_REG_DEVS(LDO15),
-	MAX8925_REG_DEVS(LDO16),
-	MAX8925_REG_DEVS(LDO17),
-	MAX8925_REG_DEVS(LDO18),
-	MAX8925_REG_DEVS(LDO19),
-	MAX8925_REG_DEVS(LDO20),
-};
+static struct regulator_init_data regulator_pdata[ARRAY_SIZE(regulator_devs)];
 
 enum {
 	FLAGS_ADC = 1,	/* register in ADC component */

@@ -669,6 +655,45 @@ static void __devinit device_onkey_init(struct max8925_chip *chip,
 		dev_err(chip->dev, "Failed to add onkey subdev\n");
 }
 
+static void __devinit device_regulator_init(struct max8925_chip *chip,
+					struct i2c_client *i2c,
+					struct max8925_platform_data *pdata)
+{
+	struct regulator_init_data *initdata;
+	int ret, i, seq;
+
+	if ((pdata == NULL) || (pdata->regulator == NULL))
+		return;
+
+	if (pdata->num_regulators > ARRAY_SIZE(regulator_devs))
+		pdata->num_regulators = ARRAY_SIZE(regulator_devs);
+
+	for (i = 0, seq = -1; i < pdata->num_regulators; i++) {
+		initdata = &pdata->regulator[i];
+		seq = *(unsigned int *)initdata->driver_data;
+		if ((seq < 0) || (seq > MAX8925_ID_MAX)) {
+			dev_err(chip->dev, "Wrong ID(%d) on regulator(%s)\n",
+				seq, 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].num_resources = 1;
+		regulator_devs[i].resources = &regulator_resources[seq];
+
+		ret = mfd_add_devices(chip->dev, 0, &regulator_devs[i], 1,
+				      &regulator_resources[seq], 0);
+		if (ret < 0) {
+			dev_err(chip->dev, "Failed to add regulator subdev\n");
+			goto out;
+		}
+	}
+out:
+	return;
+}
+
 int __devinit max8925_device_init(struct max8925_chip *chip,
 				  struct max8925_platform_data *pdata)
 {

@@ -697,18 +722,8 @@ int __devinit max8925_device_init(struct max8925_chip *chip,
 	device_touch_init(chip, chip->adc, pdata);
 	device_power_init(chip, chip->adc, pdata);
 	device_onkey_init(chip, chip->i2c, pdata);
+	device_regulator_init(chip, chip->i2c, pdata);
 
-	if (pdata && pdata->regulator[0]) {
-		ret = mfd_add_devices(chip->dev, 0, &regulator_devs[0],
-				      ARRAY_SIZE(regulator_devs),
-				      &regulator_resources[0], 0);
-		if (ret < 0) {
-			dev_err(chip->dev, "Failed to add regulator subdev\n");
-			goto out_dev;
-		}
-	}
-
-out_dev:
 	return 0;
 }

drivers/regulator/max8925-regulator.c +17 ~~-20~~

index e4dbd66..733dcd2 100644
@@ -236,39 +236,36 @@ static struct max8925_regulator_info max8925_regulator_info[] = {
 	MAX8925_LDO(20, 750, 3900, 50),
 };
 
-static struct max8925_regulator_info * __devinit find_regulator_info(int id)
-{
-	struct max8925_regulator_info *ri;
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(max8925_regulator_info); i++) {
-		ri = &max8925_regulator_info[i];
-		if (ri->desc.id == id)
-			return ri;
-	}
-	return NULL;
-}
-
 static int __devinit max8925_regulator_probe(struct platform_device *pdev)
 {
 	struct max8925_chip *chip = dev_get_drvdata(pdev->dev.parent);
-	struct max8925_platform_data *pdata = chip->dev->platform_data;
-	struct max8925_regulator_info *ri;
+	struct max8925_regulator_info *ri = NULL;
+	struct regulator_init_data *pdata = pdev->dev.platform_data;
 	struct regulator_dev *rdev;
+	struct resource *res;
+	int i;
 
-	ri = find_regulator_info(pdev->id);
-	if (ri == NULL) {
-		dev_err(&pdev->dev, "invalid regulator ID specified\n");
+	res = platform_get_resource(pdev, IORESOURCE_IO, 0);
+	if (res == NULL) {
+		dev_err(&pdev->dev, "No I/O resource!\n");
+		return -EINVAL;
+	}
+	i = res->start;
+	if ((i < 0) || (i > MAX8925_ID_MAX)) {
+		dev_err(&pdev->dev, "Failed to find regulator %d\n",
+			res->start);
 		return -EINVAL;
 	}
+	ri = &max8925_regulator_info[i];
 	ri->i2c = chip->i2c;
 	ri->chip = chip;
 
+	/* replace driver_data with ri */
 	rdev = regulator_register(&ri->desc, &pdev->dev,
-				  pdata->regulator[pdev->id], ri);
+				  pdata, ri);
 	if (IS_ERR(rdev)) {
 		dev_err(&pdev->dev, "failed to register regulator %s\n",
-				ri->desc.name);
+			ri->desc.name);
 		return PTR_ERR(rdev);
 	}

include/linux/mfd/max8925.h +2 -1

index 5259dfe..438a734 100644
@@ -233,10 +233,11 @@ struct max8925_platform_data {
 	struct max8925_backlight_pdata	*backlight;
 	struct max8925_touch_pdata	*touch;
 	struct max8925_power_pdata	*power;
-	struct regulator_init_data	*regulator[MAX8925_MAX_REGULATOR];
+	struct regulator_init_data	*regulator;
 
 	int		irq_base;
 	int		tsc_irq;
+	int		num_regulators;
 };
 
 extern int max8925_reg_read(struct i2c_client *, int);

Signature · 1 line

1.5.6.5

Mark Brown

2011-04-21 10:42 UTC #40 lore raw

Re: [PATCH 13/14] regulator: max8925: fix not add device if missing init data

Quoted 2 lines · Haojian Zhuang

If regulator[0] is missed in init data, all regulators of max8925 won't be initialized.

With a changelog like this I'd expect a small change to an error check in the startup code or something but this is a very big change to the driver initialisation.

Quoted 1 line

+static struct regulator_init_data regulator_pdata[ARRAY_SIZE(regulator_devs)];

That looks really suspicious, what happens if there's two of these devices in the system?

drivers/mfd/max8925-core.c static void __devinit device_onkey_init(struct max8925_chip *chip, · line 690

Quoted 12 lines

drivers/mfd/max8925-core.c +12 -0

+		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].num_resources = 1;
+		regulator_devs[i].resources = &regulator_resources[seq];
+
+		ret = mfd_add_devices(chip->dev, 0, &regulator_devs[i], 1,
+				      &regulator_resources[seq], 0);
+		if (ret < 0) {
+			dev_err(chip->dev, "Failed to add regulator subdev\n");
+			goto out;

It's really unclear why the array is needed at all if you're registering the devices one at a time.

drivers/mfd/max8925-core.c int __devinit max8925_device_init(struct max8925_chip *chip, · line 708

Quoted 8 lines

drivers/mfd/max8925-core.c +0 -8

-	if (pdata && pdata->regulator[0]) {
-		ret = mfd_add_devices(chip->dev, 0, &regulator_devs[0],
-				      ARRAY_SIZE(regulator_devs),
-				      &regulator_resources[0], 0);
-		if (ret < 0) {
-			dev_err(chip->dev, "Failed to add regulator subdev\n");
-			goto out_dev;
-		}

Surely the only change that's needed here is to remove the check to see if pdata->regulator[0] is non-null?

Haojian Zhuang

2011-04-21 11:13 UTC #41 lore raw

RE: [PATCH 13/14] regulator: max8925: fix not add device if missing init data

Quoted 20 lines

-----Original Message-----
From: Mark Brown [mailto:[email protected]]
Sent: 2011年4月21日 6:42 PM
To: Haojian Zhuang
Cc: [email protected]; [email protected]; linux- [email protected]; [email protected]
Subject: Re: [PATCH 13/14] regulator: max8925: fix not add device if missing init data
Quoted 1 line · Haojian Zhuang
If regulator[0] is missed in init data, all regulators of max8925
won't
Quoted 1 line
be initialized.
With a changelog like this I'd expect a small change to an error check in the startup code or something but this is a very big change to the driver initialisation.

Quoted 6 lines

Quoted 1 line
+static struct regulator_init_data
regulator_pdata[ARRAY_SIZE(regulator_devs)];
That looks really suspicious, what happens if there's two of these devices in the system?

It's impossible to use two PMIC in one system. At least, I didn't hear.

Quoted 7 lines

drivers/mfd/max8925-core.c +7 -0

+		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].num_resources = 1;
+		regulator_devs[i].resources = &regulator_resources[seq];
+

Quoted 9 lines

drivers/mfd/max8925-core.c static void __devinit device_onkey_init(struct max8925_chip *chip, · line 690
Quoted 5 lines
drivers/mfd/max8925-core.c +5 -0
+		ret = mfd_add_devices(chip->dev, 0, &regulator_devs[i], 1,
+				      &regulator_resources[seq], 0);
+		if (ret < 0) {
+			dev_err(chip->dev, "Failed to add regulator subdev\n");
+			goto out;
It's really unclear why the array is needed at all if you're registering the devices one at a time.

Although there're a lot of regulators, maybe only parts of them are used. So only add the necessary one. Others will be passed. The number is got from platform data.

Quoted 11 lines

drivers/mfd/max8925-core.c int __devinit max8925_device_init(struct max8925_chip *chip, · line 708
Quoted 8 lines
drivers/mfd/max8925-core.c +0 -8
-	if (pdata && pdata->regulator[0]) {
-		ret = mfd_add_devices(chip->dev, 0, &regulator_devs[0],
-				      ARRAY_SIZE(regulator_devs),
-				      &regulator_resources[0], 0);
-		if (ret < 0) {
-			dev_err(chip->dev, "Failed to add regulator subdev\n");
-			goto out_dev;
-		}
Surely the only change that's needed here is to remove the check to see if pdata->regulator[0] is non-null?

All above changes are necessary to me.

Thanks
Haojian �翳簕.n�+壏煯壄�+%娝�遍荻w�簕.n�+壏{炳G珴�妠ay�蕠跈�,jf＂穐殢飦�戧鐉_璁(殠娸"濟�m��G珴�⒏?櫒璀�&ｘ忈秈O曟瑉窔v豝�m��鹅�⒏?朓�

Mark Brown

2011-04-21 12:03 UTC #42 lore raw

Re: [PATCH 13/14] regulator: max8925: fix not add device if missing init data

Quoted 7 lines · Haojian Zhuang

Quoted 5 lines
Quoted 1 line
+static struct regulator_init_data
regulator_pdata[ARRAY_SIZE(regulator_devs)];
That looks really suspicious, what happens if there's two of these devices in the system?
It's impossible to use two PMIC in one system. At least, I didn't hear.

I don't believe that. It's not going to be a common case but it will be possible.

Quoted 6 lines

Quoted 2 lines
It's really unclear why the array is needed at all if you're registering the devices one at a time.
Although there're a lot of regulators, maybe only parts of them are used. So only add the necessary one. Others will be passed. The number is got from platform data.

You're missing the point here. You're registering a device at a time but for some reason the platform data is being stored in an array in order to pass it in, even though only a single element in the array is in use.

Quoted 12 lines

Quoted 11 lines
drivers/mfd/max8925-core.c int __devinit max8925_device_init(struct max8925_chip *chip, · line 708
Quoted 8 lines
drivers/mfd/max8925-core.c +0 -8
-	if (pdata && pdata->regulator[0]) {
-		ret = mfd_add_devices(chip->dev, 0, &regulator_devs[0],
-				      ARRAY_SIZE(regulator_devs),
-				      &regulator_resources[0], 0);
-		if (ret < 0) {
-			dev_err(chip->dev, "Failed to add regulator subdev\n");
-			goto out_dev;
-		}
Surely the only change that's needed here is to remove the check to see if pdata->regulator[0] is non-null?

Quoted 1 line

All above changes are necessary to me.

What makes you say this? If you remove the check for the first entry being null then the driver will go and register all the regulators which seems fine. That seems like it fixes the problem with skipping all the regulators if the first one has no platform data.

You're really not explaining the changes you're trying to make here terribly well, the changes you're making seem very substantial relative to what you say you're trying to do.

Haojian Zhuang

2011-04-21 15:23 UTC #43 lore raw

Re: [PATCH 13/14] regulator: max8925: fix not add device if missing init data

Quoted 5 lines · Mark Brown

Quoted 2 lines · Haojian Zhuang
Quoted 2 lines
Quoted 1 line
+static struct regulator_init_data
regulator_pdata[ARRAY_SIZE(regulator_devs)];

Quoted 8 lines

Quoted 4 lines
Quoted 2 lines
That looks really suspicious, what happens if there's two of these devices in the system?
It's impossible to use two PMIC in one system. At least, I didn't hear.
I don't believe that. It's not going to be a common case but it will be possible.

OK, I can allocate memory to handle this.

Quoted 12 lines

Quoted 6 lines
Quoted 2 lines
It's really unclear why the array is needed at all if you're registering the devices one at a time.
Although there're a lot of regulators, maybe only parts of them are used. So only add the necessary one. Others will be passed. The number is got from platform data.
You're missing the point here. You're registering a device at a time but for some reason the platform data is being stored in an array in order to pass it in, even though only a single element in the array is in use.

I can allocate memory for platform data.

Quoted 12 lines

Quoted 11 lines
Quoted 11 lines
Quoted 8 lines
- if (pdata && pdata->regulator[0]) {
- ret = mfd_add_devices(chip->dev, 0, &regulator_devs[0],
- ARRAY_SIZE(regulator_devs),
- &regulator_resources[0], 0);
- if (ret < 0) {
- dev_err(chip->dev, "Failed to add regulator subdev\n");
- goto out_dev;
- }
Surely the only change that's needed here is to remove the check to see if pdata->regulator[0] is non-null?

Quoted 7 lines

Quoted 1 line
All above changes are necessary to me.
What makes you say this? If you remove the check for the first entry being null then the driver will go and register all the regulators which seems fine. That seems like it fixes the problem with skipping all the regulators if the first one has no platform data.

If I keep original method and only fix the null issue on regulator[0], I can also register regulators. But it'll display error messages since regulator_init_data is null while it's registers.

After switching to the current method, I only register the ones that I really need. So I can avoid some error messages in boot time.

An array seems not good, and I can switch to allocate memory.

Quoted 4 lines

You're really not explaining the changes you're trying to make here terribly well, the changes you're making seem very substantial relative to what you say you're trying to do.

Yes, I'll list all changes in log.

Mark Brown

2011-04-21 18:32 UTC #44 lore raw

Re: [PATCH 13/14] regulator: max8925: fix not add device if missing init data

Quoted 10 lines · Haojian Zhuang

On Thu, Apr 21, 2011 at 8:03 PM, Mark Brown
Quoted 4 lines
What makes you say this? If you remove the check for the first entry being null then the driver will go and register all the regulators which seems fine. That seems like it fixes the problem with skipping all the regulators if the first one has no platform data.
If I keep original method and only fix the null issue on regulator[0], I can also register regulators. But it'll display error messages since regulator_init_data is null while it's registers.

Registering regulators with null init data seems like a reasonable thing to want to do - let's just remove the error message.

Haojian Zhuang

author

2011-04-18 14:10 UTC #45 lore raw

PATCH v2 14/14 regulator: 88pm8607: avoid to use constraint name in driver

In reply to a message not in the archive

Avoid to use constraint name in driver. So use driver_data instead to store regulator name in platform driver.

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

Cc Liam Girdwood <[email protected]>

Cc Mark Brown <[email protected]>

---
 drivers/regulator/88pm8607.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

drivers/regulator/88pm8607.c +4 -2

index 784ea77..63fc34e 100644
@@ -402,12 +402,13 @@ static int __devinit pm8607_regulator_probe(struct platform_device *pdev)
 	int i;
 
 	pdata = pdev->dev.platform_data;
-	if (pdata == NULL)
+	if ((pdata == NULL) || (pdata->driver_data == NULL))
 		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))
+		/* pdata->driver_data stores the name of regulator */
+		if (!strcmp(info->desc.name, pdata->driver_data))
 			break;
 	}
 	if (i > ARRAY_SIZE(pm8607_regulator_info)) {

@@ -424,6 +425,7 @@ static int __devinit pm8607_regulator_probe(struct platform_device *pdev)
 		if (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

Mark Brown

2011-04-18 14:30 UTC #46 lore raw

Re: [PATCH 14/14] regulator: 88pm8607: avoid to use constraint name in driver

Quoted 2 lines · Haojian Zhuang

Avoid to use constraint name in driver. So use driver_data instead to store regulator name in platform driver.

If this is something that's present in the current driver it should be fixed for 2.6.39 so should be a patch at the start of the series which can be applied separately to the others.

drivers/regulator/88pm8607.c static int __devinit pm8607_regulator_probe(struct platform_device *pdev) · line 412

Quoted 3 lines

drivers/regulator/88pm8607.c +2 -0

+		/* pdata->driver_data stores the name of regulator */
+		if (!strcmp(info->desc.name, pdata->driver_data))
 			break;

Why not just use #defines in the header? It'd be less error prone if nothing else.

Haojian Zhuang

2011-04-18 15:38 UTC #47 lore raw

Re: [PATCH 14/14] regulator: 88pm8607: avoid to use constraint name in driver

Quoted 8 lines · Mark Brown

Quoted 2 lines · Haojian Zhuang
Avoid to use constraint name in driver. So use driver_data instead to store regulator name in platform driver.
If this is something that's present in the current driver it should be fixed for 2.6.39 so should be a patch at the start of the series which can be applied separately to the others.

Quoted 7 lines

Quoted 3 lines
+ /* pdata->driver_data stores the name of regulator */
+ if (!strcmp(info->desc.name, pdata->driver_data)) break;
Why not just use #defines in the header? It'd be less error prone if nothing else.

I don't understand this. desc.name is assigned in regulator driver. pdata->driver_data is assigned in platform driver. I need to use same name to link them together.

Mark Brown

2011-04-18 16:02 UTC #48 lore raw

Re: [PATCH 14/14] regulator: 88pm8607: avoid to use constraint name in driver

Quoted 8 lines · Haojian Zhuang

On Mon, Apr 18, 2011 at 10:30 PM, Mark Brown
Quoted 2 lines
Why not just use #defines in the header? It'd be less error prone if nothing else.
I don't understand this. desc.name is assigned in regulator driver. pdata->driver_data is assigned in platform driver. I need to use same name to link them together.

I assume that by "platform driver" you mean the machine driver for the system? What I'm saying is that instead of using a string to match it'd be more normal to use an enum in the driver header, that way if someone typos the string or something then there will be a build failure whereas with strings like this there will only be a runtime failure.

Part of the problem here is that your changelogs aren't really saying why you're making these changes...

Haojian Zhuang

2011-04-19 02:47 UTC #49 lore raw

RE: [PATCH 14/14] regulator: 88pm8607: avoid to use constraint name in driver

Quoted 23 lines

-----Original Message-----
From: Mark Brown [mailto:[email protected]]
Sent: 2011年4月19日 12:03 AM
To: Haojian Zhuang
Cc: Haojian Zhuang; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH 14/14] regulator: 88pm8607: avoid to use constraint name in driver
Quoted 3 lines · Haojian Zhuang
On Mon, Apr 18, 2011 at 10:30 PM, Mark Brown
Quoted 1 line
Why not just use #defines in the header? It'd be less error prone
if
Quoted 5 lines
Quoted 1 line
nothing else.
I don't understand this. desc.name is assigned in regulator driver. pdata->driver_data is assigned in platform driver. I need to use same name to link them together.

Quoted 6 lines

I assume that by "platform driver" you mean the machine driver for the system? What I'm saying is that instead of using a string to match it'd be more normal to use an enum in the driver header, that way if someone typos the string or something then there will be a build failure whereas with strings like this there will only be a runtime failure.

It's nearly same whatever it's string or index number. In machine driver, it's more convenient on using string. Otherwise, I still need to create a new integer array to store index since I transfer them into regulator data by driver_data parameter in regulator_init_data.

Quoted 2 lines

Part of the problem here is that your changelogs aren't really saying why you're making these changes...

Yes, the changelog is too simple. ��{.n�+��+%��ݶ��w��{.n�+��{��G��{ay�ʇڙ�,j��f��h��z_�(�階�ݢj"��m��G��?��&��~��iO��z��v�^�m��?�I�

Mark Brown

2011-04-19 08:12 UTC #50 lore raw

Re: [PATCH 14/14] regulator: 88pm8607: avoid to use constraint name in driver

Quoted 8 lines · Haojian Zhuang

Quoted 5 lines
I assume that by "platform driver" you mean the machine driver for the system? What I'm saying is that instead of using a string to match it'd be more normal to use an enum in the driver header, that way if someone typos the string or something then there will be a build failure whereas with strings like this there will only be a runtime failure.
It's nearly same whatever it's string or index number. In machine driver, it's more convenient on using string. Otherwise, I still need to create

You're saying it's more convenient but you're not saying why. I'd say that the lack of build time checking I mentioned is actually unhelpful.

Quoted 2 lines

a new integer array to store index since I transfer them into regulator data by driver_data parameter in regulator_init_data.

You already need to add a char * to store the name, adding an enum instead isn't a substantial difference...

Haojian Zhuang

2011-04-19 11:37 UTC #51 lore raw

RE: [PATCH 14/14] regulator: 88pm8607: avoid to use constraint name in driver

Quoted 32 lines

-----Original Message-----
From: Mark Brown [mailto:[email protected]]
Sent: 2011年4月19日 4:13 PM
To: Haojian Zhuang
Cc: Haojian Zhuang; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH 14/14] regulator: 88pm8607: avoid to use constraint name in driver
Quoted 1 line · Haojian Zhuang
Quoted 1 line
I assume that by "platform driver" you mean the machine driver for
the
Quoted 1 line
Quoted 1 line
system? What I'm saying is that instead of using a string to match
it'd
Quoted 1 line
Quoted 1 line
be more normal to use an enum in the driver header, that way if
someone
Quoted 1 line
Quoted 1 line
typos the string or something then there will be a build failure
whereas
Quoted 3 lines
Quoted 1 line
with strings like this there will only be a runtime failure.
It's nearly same whatever it's string or index number. In machine
driver,
Quoted 1 line
it's more convenient on using string. Otherwise, I still need to
create
You're saying it's more convenient but you're not saying why. I'd say that the lack of build time checking I mentioned is actually unhelpful.

Quoted 6 lines

Quoted 1 line
a new integer array to store index since I transfer them into
regulator data
Quoted 1 line
by driver_data parameter in regulator_init_data.
You already need to add a char * to store the name, adding an enum instead isn't a substantial difference...

OK. So I just need to switch to ID instead of name on these two patches.
Is it right? �翳簕.n�+壏煯壄�+%娝�遍荻w�簕.n�+壏{炳G珴�妠ay�蕠跈�,jf＂穐殢飦�戧鐉_璁(殠娸"濟�m��G珴�⒏?櫒璀�&ｘ忈秈O曟瑉窔v豝�m��鹅�⒏?朓�

Haojian Zhuang

author

2011-04-20 12:01 UTC #52 lore raw

PATCH v2 14/14 regulator: 88pm8607: avoid to use constraint name in driver

In reply to Haojian Zhuang

Avoid to use constraint name in driver. So use driver_data instead to store regulator id in platform driver.

Signed-off-by Haojian Zhuang <[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 4fbc69b..3147c41 100644
@@ -544,7 +544,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;

@@ -552,31 +552,12 @@ 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;
 		}
 		memcpy(&regulator_pdata[i], &pdata->regulator[i],

@@ -584,10 +565,10 @@ static void __devinit device_regulator_init(struct pm860x_chip *chip,
 		regulator_devs[i].platform_data = &regulator_pdata[i];
 		regulator_devs[i].pdata_size = sizeof(regulator_pdata[i]);
 		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