[PATCH] mfd: ADP5520 Multifunction LCD Backlight and Keypad Input Device Driver

[PATCH] mfd: ADP5520 Multifunction LCD Backlight and Keypad Input Device Driver

[Mike Frysinger]

From: Michael Hennerich <michael.hennerich@analog.com>

Base driver for Analog Devices ADP5520/ADP5501 MFD PMICs

Subdevs:
LCD Backlight   : drivers/video/backlight/adp5520_bl
LEDs            : drivers/led/leds-adp5520
GPIO            : drivers/gpio/adp5520-gpio (ADP5520 only)
Keys            : drivers/input/keyboard/adp5520-keys (ADP5520 only)

Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
Signed-off-by: Bryan Wu <cooloney@kernel.org>
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 drivers/mfd/Kconfig         |   10 ++
 drivers/mfd/Makefile        |    1 +
 drivers/mfd/adp5520.c       |  371 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/adp5520.h |  303 +++++++++++++++++++++++++++++++++++
 4 files changed, 685 insertions(+), 0 deletions(-)
 create mode 100644 drivers/mfd/adp5520.c
 create mode 100644 include/linux/mfd/adp5520.h

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 491ac0f..421438c 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -147,6 +147,16 @@ config PMIC_DA903X
 	  individual components like LCD backlight, voltage regulators,
 	  LEDs and battery-charger under the corresponding menus.
 
+config PMIC_ADP5520
+	bool "Analog Devices ADP5520/01 MFD PMIC Core Support"
+	depends on I2C=y
+	help
+	  Say yes here to add support for Analog Devices AD5520 and ADP5501,
+	  Multifunction Power Management IC. This includes
+	  the I2C driver and the core APIs _only_, you have to select
+	  individual components like LCD backlight, LEDs, GPIOs and Kepad
+	  under the corresponding menus.
+
 config MFD_WM8400
 	tristate "Support Wolfson Microelectronics WM8400"
 	select MFD_CORE
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 6f8a9a1..981092c 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -44,3 +44,4 @@ obj-$(CONFIG_MFD_PCF50633)	+= pcf50633-core.o
 obj-$(CONFIG_PCF50633_ADC)	+= pcf50633-adc.o
 obj-$(CONFIG_PCF50633_GPIO)	+= pcf50633-gpio.o
 obj-$(CONFIG_AB3100_CORE)	+= ab3100-core.o
+obj-$(CONFIG_PMIC_ADP5520)	+= adp5520.o
diff --git a/drivers/mfd/adp5520.c b/drivers/mfd/adp5520.c
new file mode 100644
index 0000000..7b538df
--- /dev/null
+++ b/drivers/mfd/adp5520.c
@@ -0,0 +1,371 @@
+/*
+ * Base driver for Analog Devices ADP5520/ADP5501 MFD PMICs
+ * LCD Backlight: drivers/video/backlight/adp5520_bl
+ * LEDs		: drivers/led/leds-adp5520
+ * GPIO		: drivers/gpio/adp5520-gpio (ADP5520 only)
+ * Keys		: drivers/input/keyboard/adp5520-keys (ADP5520 only)
+ *
+ * Copyright 2009 Analog Devices Inc.
+ *
+ * Derived from da903x:
+ * Copyright (C) 2008 Compulab, Ltd.
+ * 	Mike Rapoport <mike@compulab.co.il>
+ *
+ * Copyright (C) 2006-2008 Marvell International Ltd.
+ * 	Eric Miao <eric.miao@marvell.com>
+ *
+ * Licensed under the GPL-2 or later.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/workqueue.h>
+#include <linux/i2c.h>
+
+#include <linux/mfd/adp5520.h>
+
+struct adp5520_chip {
+	struct i2c_client *client;
+	struct device *dev;
+	struct mutex lock;
+	struct work_struct irq_work;
+	struct blocking_notifier_head notifier_list;
+	int irq;
+};
+
+static int __adp5520_read(struct i2c_client *client,
+				int reg, uint8_t *val)
+{
+	int ret;
+
+	ret = i2c_smbus_read_byte_data(client, reg);
+	if (ret < 0) {
+		dev_err(&client->dev, "failed reading at 0x%02x\n", reg);
+		return ret;
+	}
+
+	*val = (uint8_t)ret;
+	return 0;
+}
+
+static int __adp5520_write(struct i2c_client *client,
+				 int reg, uint8_t val)
+{
+	int ret;
+
+	ret = i2c_smbus_write_byte_data(client, reg, val);
+	if (ret < 0) {
+		dev_err(&client->dev, "failed writing 0x%02x to 0x%02x\n",
+				val, reg);
+		return ret;
+	}
+	return 0;
+}
+
+int __adp5520_ack_bits(struct i2c_client *client, int reg, uint8_t bit_mask)
+{
+	struct adp5520_chip *chip = i2c_get_clientdata(client);
+	uint8_t reg_val;
+	int ret;
+
+	mutex_lock(&chip->lock);
+
+	ret = __adp5520_read(client, reg, &reg_val);
+
+	if (!ret) {
+		reg_val |= bit_mask;
+		ret = __adp5520_write(client, reg, reg_val);
+	}
+
+	mutex_unlock(&chip->lock);
+	return ret;
+}
+
+int adp5520_write(struct device *dev, int reg, uint8_t val)
+{
+	return __adp5520_write(to_i2c_client(dev), reg, val);
+}
+EXPORT_SYMBOL_GPL(adp5520_write);
+
+int adp5520_read(struct device *dev, int reg, uint8_t *val)
+{
+	return __adp5520_read(to_i2c_client(dev), reg, val);
+}
+EXPORT_SYMBOL_GPL(adp5520_read);
+
+int adp5520_set_bits(struct device *dev, int reg, uint8_t bit_mask)
+{
+	struct adp5520_chip *chip = dev_get_drvdata(dev);
+	uint8_t reg_val;
+	int ret;
+
+	mutex_lock(&chip->lock);
+
+	ret = __adp5520_read(chip->client, reg, &reg_val);
+
+	if (!ret && ((reg_val & bit_mask) == 0)) {
+		reg_val |= bit_mask;
+		ret = __adp5520_write(chip->client, reg, reg_val);
+	}
+
+	mutex_unlock(&chip->lock);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(adp5520_set_bits);
+
+int adp5520_clr_bits(struct device *dev, int reg, uint8_t bit_mask)
+{
+	struct adp5520_chip *chip = dev_get_drvdata(dev);
+	uint8_t reg_val;
+	int ret;
+
+	mutex_lock(&chip->lock);
+
+	ret = __adp5520_read(chip->client, reg, &reg_val);
+
+	if (!ret && (reg_val & bit_mask)) {
+		reg_val &= ~bit_mask;
+		ret = __adp5520_write(chip->client, reg, reg_val);
+	}
+
+	mutex_unlock(&chip->lock);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(adp5520_clr_bits);
+
+int adp5520_register_notifier(struct device *dev, struct notifier_block *nb,
+				unsigned int events)
+{
+	struct adp5520_chip *chip = dev_get_drvdata(dev);
+
+	if (chip->irq) {
+		adp5520_set_bits(chip->dev, INTERRUPT_ENABLE,
+			events & (KP_IEN | KR_IEN | OVP_IEN | CMPR_IEN));
+
+		return blocking_notifier_chain_register(&chip->notifier_list,
+			 nb);
+	}
+
+	return -ENODEV;
+}
+EXPORT_SYMBOL_GPL(adp5520_register_notifier);
+
+int adp5520_unregister_notifier(struct device *dev, struct notifier_block *nb,
+				unsigned int events)
+{
+	struct adp5520_chip *chip = dev_get_drvdata(dev);
+
+	adp5520_clr_bits(chip->dev, INTERRUPT_ENABLE,
+		events & (KP_IEN | KR_IEN | OVP_IEN | CMPR_IEN));
+
+	return blocking_notifier_chain_unregister(&chip->notifier_list, nb);
+}
+EXPORT_SYMBOL_GPL(adp5520_unregister_notifier);
+
+static void adp5520_irq_work(struct work_struct *work)
+{
+	struct adp5520_chip *chip =
+		container_of(work, struct adp5520_chip, irq_work);
+	unsigned int events;
+	uint8_t reg_val;
+	int ret;
+
+	ret = __adp5520_read(chip->client, MODE_STATUS, &reg_val);
+	if (ret)
+		goto out;
+
+	events =  reg_val & (OVP_INT | CMPR_INT | GPI_INT | KR_INT | KP_INT);
+
+	blocking_notifier_call_chain(&chip->notifier_list, events, NULL);
+	/* ACK, Sticky bits are W1C */
+	__adp5520_ack_bits(chip->client, MODE_STATUS, events);
+
+out:
+	enable_irq(chip->client->irq);
+}
+
+static int adp5520_irq_handler(int irq, void *data)
+{
+	struct adp5520_chip *chip = data;
+
+	disable_irq_nosync(irq);
+	schedule_work(&chip->irq_work);
+
+	return IRQ_HANDLED;
+}
+
+static int __remove_subdev(struct device *dev, void *unused)
+{
+	platform_device_unregister(to_platform_device(dev));
+	return 0;
+}
+
+static int adp5520_remove_subdevs(struct adp5520_chip *chip)
+{
+	return device_for_each_child(chip->dev, NULL, __remove_subdev);
+}
+
+static int __devinit adp5520_add_subdevs(struct adp5520_chip *chip,
+					struct adp5520_platform_data *pdata)
+{
+	struct adp5520_subdev_info *subdev;
+	struct platform_device *pdev;
+	int i, ret = 0;
+
+	for (i = 0; i < pdata->num_subdevs; i++) {
+		subdev = &pdata->subdevs[i];
+
+		pdev = platform_device_alloc(subdev->name, subdev->id);
+
+		pdev->dev.parent = chip->dev;
+		pdev->dev.platform_data = subdev->platform_data;
+
+		ret = platform_device_add(pdev);
+		if (ret)
+			goto failed;
+	}
+	return 0;
+
+failed:
+	adp5520_remove_subdevs(chip);
+	return ret;
+}
+
+static int __devinit adp5520_probe(struct i2c_client *client,
+					const struct i2c_device_id *id)
+{
+	struct adp5520_platform_data *pdata = client->dev.platform_data;
+	struct adp5520_chip *chip;
+	int ret;
+
+	if (!i2c_check_functionality(client->adapter,
+					I2C_FUNC_SMBUS_BYTE_DATA)) {
+		dev_err(&client->dev, "SMBUS Word Data not Supported\n");
+		return -EIO;
+	}
+
+	if (pdata == NULL) {
+		dev_err(&client->dev, "missing platform data\n");
+		return -ENODEV;
+	}
+
+	chip = kzalloc(sizeof(*chip), GFP_KERNEL);
+	if (!chip)
+		return -ENOMEM;
+
+	i2c_set_clientdata(client, chip);
+	chip->client = client;
+
+	chip->dev = &client->dev;
+	chip->irq = client->irq;
+	mutex_init(&chip->lock);
+
+	if (chip->irq) {
+		INIT_WORK(&chip->irq_work, adp5520_irq_work);
+		BLOCKING_INIT_NOTIFIER_HEAD(&chip->notifier_list);
+
+		ret = request_irq(chip->irq, adp5520_irq_handler,
+				IRQF_DISABLED | IRQF_TRIGGER_LOW,
+				"adp5520", chip);
+		if (ret) {
+			dev_err(&client->dev, "failed to request irq %d\n",
+					chip->irq);
+			goto out_free_chip;
+		}
+	}
+
+	ret = adp5520_write(chip->dev, MODE_STATUS, nSTNBY);
+	if (ret) {
+		dev_err(&client->dev, "failed to write\n");
+		goto out_free_irq;
+	}
+
+	ret = adp5520_add_subdevs(chip, pdata);
+
+	if (!ret)
+		return ret;
+
+out_free_irq:
+	if (chip->irq)
+		free_irq(chip->irq, chip);
+
+out_free_chip:
+	i2c_set_clientdata(client, NULL);
+	kfree(chip);
+
+	return ret;
+}
+
+static int __devexit adp5520_remove(struct i2c_client *client)
+{
+	struct adp5520_chip *chip = dev_get_drvdata(&client->dev);
+
+	if (chip->irq)
+		free_irq(chip->irq, chip);
+
+	adp5520_remove_subdevs(chip);
+	adp5520_write(chip->dev, MODE_STATUS, 0);
+	i2c_set_clientdata(client, NULL);
+	kfree(chip);
+	return 0;
+}
+
+#ifdef CONFIG_PM
+static int adp5520_suspend(struct i2c_client *client,
+				 pm_message_t state)
+{
+	struct adp5520_chip *chip = dev_get_drvdata(&client->dev);
+
+	adp5520_clr_bits(chip->dev, MODE_STATUS, nSTNBY);
+	return 0;
+}
+
+static int adp5520_resume(struct i2c_client *client)
+{
+	struct adp5520_chip *chip = dev_get_drvdata(&client->dev);
+
+	adp5520_set_bits(chip->dev, MODE_STATUS, nSTNBY);
+	return 0;
+}
+#else
+#define adp5520_suspend	NULL
+#define adp5520_resume	NULL
+#endif
+
+static const struct i2c_device_id adp5520_id[] = {
+	{ "pmic-adp5520", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, adp5520_id);
+
+static struct i2c_driver adp5520_driver = {
+	.driver = {
+		.name	= "adp5520",
+		.owner	= THIS_MODULE,
+	},
+	.probe		= adp5520_probe,
+	.remove		= __devexit_p(adp5520_remove),
+	.suspend	= adp5520_suspend,
+	.resume		= adp5520_resume,
+	.id_table 	= adp5520_id,
+};
+
+static int __init adp5520_init(void)
+{
+	return i2c_add_driver(&adp5520_driver);
+}
+module_init(adp5520_init);
+
+static void __exit adp5520_exit(void)
+{
+	i2c_del_driver(&adp5520_driver);
+}
+module_exit(adp5520_exit);
+
+MODULE_AUTHOR("Michael Hennerich <hennerich@blackfin.uclinux.org>");
+MODULE_DESCRIPTION("ADP5520(01) PMIC-MFD Driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/mfd/adp5520.h b/include/linux/mfd/adp5520.h
new file mode 100644
index 0000000..fffa1db
--- /dev/null
+++ b/include/linux/mfd/adp5520.h
@@ -0,0 +1,303 @@
+/*
+ * Definitions and platfrom data for Analog Devices
+ * ADP5520/ADP5501 MFD PMICs (Backlight, LED, GPIO and Keys)
+ *
+ * Copyright 2009 Analog Devices Inc.
+ *
+ * Licensed under the GPL-2 or later.
+ */
+
+
+#ifndef __LINUX_MFD_ADP5520_H
+#define __LINUX_MFD_ADP5520_H
+
+#define ID_ADP5520		5520
+#define ID_ADP5501		5501
+
+/*
+ * ADP5520/ADP5501 Register Map
+ */
+
+#define MODE_STATUS 		0x00
+#define INTERRUPT_ENABLE 	0x01
+#define BL_CONTROL 		0x02
+#define BL_TIME 		0x03
+#define BL_FADE 		0x04
+#define DAYLIGHT_MAX 		0x05
+#define DAYLIGHT_DIM 		0x06
+#define OFFICE_MAX 		0x07
+#define OFFICE_DIM 		0x08
+#define DARK_MAX 		0x09
+#define DARK_DIM 		0x0A
+#define BL_VALUE 		0x0B
+#define ALS_CMPR_CFG 		0x0C
+#define L2_TRIP 		0x0D
+#define L2_HYS 			0x0E
+#define L3_TRIP 		0x0F
+#define L3_HYS 			0x10
+#define LED_CONTROL 		0x11
+#define LED_TIME 		0x12
+#define LED_FADE 		0x13
+#define LED1_CURRENT 		0x14
+#define LED2_CURRENT 		0x15
+#define LED3_CURRENT 		0x16
+
+/*
+ * ADP5520 Register Map
+ */
+
+#define GPIO_CFG_1 		0x17
+#define GPIO_CFG_2 		0x18
+#define GPIO_IN 		0x19
+#define GPIO_OUT 		0x1A
+#define GPIO_INT_EN 		0x1B
+#define GPIO_INT_STAT 		0x1C
+#define GPIO_INT_LVL 		0x1D
+#define GPIO_DEBOUNCE 		0x1E
+#define GPIO_PULLUP 		0x1F
+#define KP_INT_STAT_1 		0x20
+#define KP_INT_STAT_2 		0x21
+#define KR_INT_STAT_1 		0x22
+#define KR_INT_STAT_2 		0x23
+#define KEY_STAT_1 		0x24
+#define KEY_STAT_2 		0x25
+
+/*
+ * MODE_STATUS bits
+ */
+
+#define nSTNBY		(1 << 7)
+#define BL_EN           (1 << 6)
+#define DIM_EN          (1 << 5)
+#define OVP_INT         (1 << 4)
+#define CMPR_INT        (1 << 3)
+#define GPI_INT         (1 << 2)
+#define KR_INT          (1 << 1)
+#define KP_INT          (1 << 0)
+
+/*
+ * INTERRUPT_ENABLE bits
+ */
+
+#define AUTO_LD_EN      (1 << 4)
+#define CMPR_IEN        (1 << 3)
+#define OVP_IEN         (1 << 2)
+#define KR_IEN          (1 << 1)
+#define KP_IEN          (1 << 0)
+
+/*
+ * BL_CONTROL bits
+ */
+
+#define BL_LVL          ((x) << 5)
+#define BL_LAW          ((x) << 4)
+#define BL_AUTO_ADJ     (1 << 3)
+#define OVP_EN          (1 << 2)
+#define FOVR            (1 << 1)
+#define KP_BL_EN        (1 << 0)
+
+/*
+ * ALS_CMPR_CFG bits
+ */
+
+#define L3_OUT		(1 << 3)
+#define L2_OUT		(1 << 2)
+#define L3_EN		(1 << 1)
+
+#define ADP5020_MAX_BRIGHTNESS	0x7F
+
+#define FADE_VAL(in, out)	((0xF & (in)) | ((0xF & (out)) << 4))
+#define BL_CTRL_VAL(law, auto)	(((1 & (auto)) << 3)) | ((0x3 & (law)) << 4))
+#define ALS_CMPR_CFG_VAL(filt, l3_en)	(((0x7 & filt) << 5) | l3_en)
+
+/*
+ * LEDs subdevice bits and masks
+ */
+
+#define ADP5520_01_MAXLEDS 3
+
+#define FLAG_LED_MASK 		0x3
+#define FLAG_OFFT_SHIFT 	8
+#define FLAG_OFFT_MASK 		0x3
+
+#define R3_MODE		(1 << 5)
+#define C3_MODE		(1 << 4)
+#define LED_LAW		(1 << 3)
+#define LED3_EN		(1 << 2)
+#define LED2_EN		(1 << 1)
+#define LED1_EN		(1 << 0)
+
+/*
+ * GPIO subdevice bits and masks
+ */
+
+#define ADP5520_MAXGPIOS	8
+
+#define GPIO_C3		(1 << 7)	/* LED2 or GPIO7 aka C3 */
+#define GPIO_C2		(1 << 6)
+#define GPIO_C1		(1 << 5)
+#define GPIO_C0		(1 << 4)
+#define GPIO_R3		(1 << 3)	/* LED3 or GPIO3 aka R3 */
+#define GPIO_R2		(1 << 2)
+#define GPIO_R1		(1 << 1)
+#define GPIO_R0		(1 << 0)
+
+struct adp5520_gpio_platfrom_data {
+	unsigned gpio_start;
+	u8 gpio_en_mask;
+	u8 gpio_pullup_mask;
+};
+
+/*
+ * Keypad subdevice bits and masks
+ */
+
+#define ADP5520_MAXKEYS	16
+
+#define COL_C3 		(1 << 7)	/* LED2 or GPIO7 aka C3 */
+#define COL_C2		(1 << 6)
+#define COL_C1		(1 << 5)
+#define COL_C0		(1 << 4)
+#define ROW_R3		(1 << 3)	/* LED3 or GPIO3 aka R3 */
+#define ROW_R2		(1 << 2)
+#define ROW_R1		(1 << 1)
+#define ROW_R0		(1 << 0)
+
+#define KEY(row, col) (col + row * 4)
+#define ADP5520_KEYMAPSIZE	ADP5520_MAXKEYS
+
+struct adp5520_keys_platfrom_data {
+	int rows_en_mask;		/* Number of rows */
+	int cols_en_mask;		/* Number of columns */
+	const unsigned short *keymap;	/* Pointer to keymap */
+	unsigned short keymapsize;	/* Keymap size */
+	unsigned repeat:1;		/* Enable key repeat */
+};
+
+
+/*
+ * LEDs subdevice platfrom data
+ */
+
+#define FLAG_ID_ADP5520_LED1_ADP5501_LED0 	1	/* ADP5520 PIN ILED */
+#define FLAG_ID_ADP5520_LED2_ADP5501_LED1 	2	/* ADP5520 PIN C3 */
+#define FLAG_ID_ADP5520_LED3_ADP5501_LED2 	3	/* ADP5520 PIN R3 */
+
+#define LED_DIS_BLINK	(0 << FLAG_OFFT_SHIFT)
+#define LED_OFFT_600ms	(1 << FLAG_OFFT_SHIFT)
+#define LED_OFFT_800ms	(2 << FLAG_OFFT_SHIFT)
+#define LED_OFFT_1200ms	(3 << FLAG_OFFT_SHIFT)
+
+#define LED_ONT_200ms	0
+#define LED_ONT_600ms	1
+#define LED_ONT_800ms	2
+#define LED_ONT_1200ms	3
+
+struct adp5520_leds_platfrom_data {
+	int num_leds;
+	struct led_info	*leds;
+	u8 fade_in;		/* Backlight Fade-In Timer */
+	u8 fade_out;		/* Backlight Fade-Out Timer */
+	u8 led_on_time;
+};
+
+/*
+ * Backlight subdevice platfrom data
+ */
+
+#define FADE_T_DIS	0	/* Fade Timer Disabled */
+#define FADE_T_300ms	1	/* 0.3 Sec */
+#define FADE_T_600ms	2
+#define FADE_T_900ms	3
+#define FADE_T_1200ms	4
+#define FADE_T_1500ms	5
+#define FADE_T_1800ms	6
+#define FADE_T_2100ms	7
+#define FADE_T_2400ms	8
+#define FADE_T_2700ms	9
+#define FADE_T_3000ms	10
+#define FADE_T_3500ms	11
+#define FADE_T_4000ms	12
+#define FADE_T_4500ms	13
+#define FADE_T_5000ms	14
+#define FADE_T_5500ms	15	/* 5.5 Sec */
+
+#define BL_LAW_LINEAR 	0
+#define BL_LAW_SQUARE 	1
+#define BL_LAW_CUBIC1 	2
+#define BL_LAW_CUBIC2 	3
+
+#define BL_AMBL_FILT_80ms 	0	/* Light sensor filter time */
+#define BL_AMBL_FILT_160ms 	1
+#define BL_AMBL_FILT_320ms 	2
+#define BL_AMBL_FILT_640ms 	3
+#define BL_AMBL_FILT_1280ms 	4
+#define BL_AMBL_FILT_2560ms 	5
+#define BL_AMBL_FILT_5120ms 	6
+#define BL_AMBL_FILT_10240ms 	7	/* 10.24 sec */
+
+	/*
+	 * Blacklight current 0..30mA
+	 */
+#define BL_CUR_mA(I)		((I * 127) / 30)
+
+	/*
+	 * L2 comparator current 0..1000uA
+	 */
+#define L2_COMP_CURR_uA(I)	((I * 255) / 1000)
+
+	/*
+	 * L3 comparator current 0..127uA
+	 */
+#define L3_COMP_CURR_uA(I)	((I * 255) / 127)
+
+struct adp5520_backlight_platfrom_data {
+	u8 fade_in;		/* Backlight Fade-In Timer */
+	u8 fade_out;		/* Backlight Fade-Out Timer */
+	u8 fade_led_law;	/* fade-on/fade-off transfer characteristic */
+
+	u8 en_ambl_sens;	/* 1 = enable ambient light sensor */
+	u8 abml_filt;		/* Light sensor filter time */
+	u8 l1_daylight_max;	/* use BL_CUR_mA(I) 0 <= I <= 30 mA */
+	u8 l1_daylight_dim;	/* typ = 0, use BL_CUR_mA(I) 0 <= I <= 30 mA */
+	u8 l2_office_max;	/* use BL_CUR_mA(I) 0 <= I <= 30 mA */
+	u8 l2_office_dim;	/* typ = 0, use BL_CUR_mA(I) 0 <= I <= 30 mA */
+	u8 l3_dark_max;		/* use BL_CUR_mA(I) 0 <= I <= 30 mA */
+	u8 l3_dark_dim;		/* typ = 0, use BL_CUR_mA(I) 0 <= I <= 30 mA */
+	u8 l2_trip;		/* use L2_COMP_CURR_uA(I) 0 <= I <= 1000 uA */
+	u8 l2_hyst;		/* use L2_COMP_CURR_uA(I) 0 <= I <= 1000 uA */
+	u8 l3_trip;		/* use L3_COMP_CURR_uA(I) 0 <= I <= 127 uA */
+	u8 l3_hyst;		/* use L3_COMP_CURR_uA(I) 0 <= I <= 127 uA */
+};
+
+/*
+ * MFD chip platfrom data
+ */
+
+struct adp5520_subdev_info {
+	int		id;
+	const char	*name;
+	void		*platform_data;
+};
+
+struct adp5520_platform_data {
+	int num_subdevs;
+	struct adp5520_subdev_info *subdevs;
+};
+
+/*
+ * MFD chip functions
+ */
+
+extern int adp5520_read(struct device *dev, int reg, uint8_t *val);
+extern int adp5520_write(struct device *dev, int reg, u8 val);
+extern int adp5520_clr_bits(struct device *dev, int reg, uint8_t bit_mask);
+extern int adp5520_set_bits(struct device *dev, int reg, uint8_t bit_mask);
+
+extern int adp5520_register_notifier(struct device *dev,
+		 struct notifier_block *nb, unsigned int events);
+
+extern int adp5520_unregister_notifier(struct device *dev,
+		struct notifier_block *nb, unsigned int events);
+
+#endif /* __LINUX_MFD_ADP5520_H */
-- 
1.6.5.rc1

[PATCH v2] mfd: ADP5520 Multifunction LCD Backlight and Keypad Input Device Driver

[Mike Frysinger]

From: Michael Hennerich <michael.hennerich@analog.com>

Base driver for Analog Devices ADP5520/ADP5501 MFD PMICs

Subdevs:
LCD Backlight   : drivers/video/backlight/adp5520_bl
LEDs            : drivers/led/leds-adp5520
GPIO            : drivers/gpio/adp5520-gpio (ADP5520 only)
Keys            : drivers/input/keyboard/adp5520-keys (ADP5520 only)

Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
Signed-off-by: Bryan Wu <cooloney@kernel.org>
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
v2
	- fix return type of irq handler
	- fix unbalanced paren in BL_CTRL_VAL() macro

 drivers/mfd/Kconfig         |   10 ++
 drivers/mfd/Makefile        |    1 +
 drivers/mfd/adp5520.c       |  371 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/adp5520.h |  303 +++++++++++++++++++++++++++++++++++
 4 files changed, 685 insertions(+), 0 deletions(-)
 create mode 100644 drivers/mfd/adp5520.c
 create mode 100644 include/linux/mfd/adp5520.h

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 570be13..34e8595 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -160,6 +160,16 @@ config PMIC_DA903X
 	  individual components like LCD backlight, voltage regulators,
 	  LEDs and battery-charger under the corresponding menus.
 
+config PMIC_ADP5520
+	bool "Analog Devices ADP5520/01 MFD PMIC Core Support"
+	depends on I2C=y
+	help
+	  Say yes here to add support for Analog Devices AD5520 and ADP5501,
+	  Multifunction Power Management IC. This includes
+	  the I2C driver and the core APIs _only_, you have to select
+	  individual components like LCD backlight, LEDs, GPIOs and Kepad
+	  under the corresponding menus.
+
 config MFD_WM8400
 	tristate "Support Wolfson Microelectronics WM8400"
 	select MFD_CORE
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index f3b277b..a42248e 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -50,3 +50,4 @@ obj-$(CONFIG_PCF50633_ADC)	+= pcf50633-adc.o
 obj-$(CONFIG_PCF50633_GPIO)	+= pcf50633-gpio.o
 obj-$(CONFIG_AB3100_CORE)	+= ab3100-core.o
 obj-$(CONFIG_AB3100_OTP)	+= ab3100-otp.o
+obj-$(CONFIG_PMIC_ADP5520)	+= adp5520.o
diff --git a/drivers/mfd/adp5520.c b/drivers/mfd/adp5520.c
new file mode 100644
index 0000000..1083626
--- /dev/null
+++ b/drivers/mfd/adp5520.c
@@ -0,0 +1,371 @@
+/*
+ * Base driver for Analog Devices ADP5520/ADP5501 MFD PMICs
+ * LCD Backlight: drivers/video/backlight/adp5520_bl
+ * LEDs		: drivers/led/leds-adp5520
+ * GPIO		: drivers/gpio/adp5520-gpio (ADP5520 only)
+ * Keys		: drivers/input/keyboard/adp5520-keys (ADP5520 only)
+ *
+ * Copyright 2009 Analog Devices Inc.
+ *
+ * Derived from da903x:
+ * Copyright (C) 2008 Compulab, Ltd.
+ * 	Mike Rapoport <mike@compulab.co.il>
+ *
+ * Copyright (C) 2006-2008 Marvell International Ltd.
+ * 	Eric Miao <eric.miao@marvell.com>
+ *
+ * Licensed under the GPL-2 or later.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/workqueue.h>
+#include <linux/i2c.h>
+
+#include <linux/mfd/adp5520.h>
+
+struct adp5520_chip {
+	struct i2c_client *client;
+	struct device *dev;
+	struct mutex lock;
+	struct work_struct irq_work;
+	struct blocking_notifier_head notifier_list;
+	int irq;
+};
+
+static int __adp5520_read(struct i2c_client *client,
+				int reg, uint8_t *val)
+{
+	int ret;
+
+	ret = i2c_smbus_read_byte_data(client, reg);
+	if (ret < 0) {
+		dev_err(&client->dev, "failed reading at 0x%02x\n", reg);
+		return ret;
+	}
+
+	*val = (uint8_t)ret;
+	return 0;
+}
+
+static int __adp5520_write(struct i2c_client *client,
+				 int reg, uint8_t val)
+{
+	int ret;
+
+	ret = i2c_smbus_write_byte_data(client, reg, val);
+	if (ret < 0) {
+		dev_err(&client->dev, "failed writing 0x%02x to 0x%02x\n",
+				val, reg);
+		return ret;
+	}
+	return 0;
+}
+
+int __adp5520_ack_bits(struct i2c_client *client, int reg, uint8_t bit_mask)
+{
+	struct adp5520_chip *chip = i2c_get_clientdata(client);
+	uint8_t reg_val;
+	int ret;
+
+	mutex_lock(&chip->lock);
+
+	ret = __adp5520_read(client, reg, &reg_val);
+
+	if (!ret) {
+		reg_val |= bit_mask;
+		ret = __adp5520_write(client, reg, reg_val);
+	}
+
+	mutex_unlock(&chip->lock);
+	return ret;
+}
+
+int adp5520_write(struct device *dev, int reg, uint8_t val)
+{
+	return __adp5520_write(to_i2c_client(dev), reg, val);
+}
+EXPORT_SYMBOL_GPL(adp5520_write);
+
+int adp5520_read(struct device *dev, int reg, uint8_t *val)
+{
+	return __adp5520_read(to_i2c_client(dev), reg, val);
+}
+EXPORT_SYMBOL_GPL(adp5520_read);
+
+int adp5520_set_bits(struct device *dev, int reg, uint8_t bit_mask)
+{
+	struct adp5520_chip *chip = dev_get_drvdata(dev);
+	uint8_t reg_val;
+	int ret;
+
+	mutex_lock(&chip->lock);
+
+	ret = __adp5520_read(chip->client, reg, &reg_val);
+
+	if (!ret && ((reg_val & bit_mask) == 0)) {
+		reg_val |= bit_mask;
+		ret = __adp5520_write(chip->client, reg, reg_val);
+	}
+
+	mutex_unlock(&chip->lock);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(adp5520_set_bits);
+
+int adp5520_clr_bits(struct device *dev, int reg, uint8_t bit_mask)
+{
+	struct adp5520_chip *chip = dev_get_drvdata(dev);
+	uint8_t reg_val;
+	int ret;
+
+	mutex_lock(&chip->lock);
+
+	ret = __adp5520_read(chip->client, reg, &reg_val);
+
+	if (!ret && (reg_val & bit_mask)) {
+		reg_val &= ~bit_mask;
+		ret = __adp5520_write(chip->client, reg, reg_val);
+	}
+
+	mutex_unlock(&chip->lock);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(adp5520_clr_bits);
+
+int adp5520_register_notifier(struct device *dev, struct notifier_block *nb,
+				unsigned int events)
+{
+	struct adp5520_chip *chip = dev_get_drvdata(dev);
+
+	if (chip->irq) {
+		adp5520_set_bits(chip->dev, INTERRUPT_ENABLE,
+			events & (KP_IEN | KR_IEN | OVP_IEN | CMPR_IEN));
+
+		return blocking_notifier_chain_register(&chip->notifier_list,
+			 nb);
+	}
+
+	return -ENODEV;
+}
+EXPORT_SYMBOL_GPL(adp5520_register_notifier);
+
+int adp5520_unregister_notifier(struct device *dev, struct notifier_block *nb,
+				unsigned int events)
+{
+	struct adp5520_chip *chip = dev_get_drvdata(dev);
+
+	adp5520_clr_bits(chip->dev, INTERRUPT_ENABLE,
+		events & (KP_IEN | KR_IEN | OVP_IEN | CMPR_IEN));
+
+	return blocking_notifier_chain_unregister(&chip->notifier_list, nb);
+}
+EXPORT_SYMBOL_GPL(adp5520_unregister_notifier);
+
+static void adp5520_irq_work(struct work_struct *work)
+{
+	struct adp5520_chip *chip =
+		container_of(work, struct adp5520_chip, irq_work);
+	unsigned int events;
+	uint8_t reg_val;
+	int ret;
+
+	ret = __adp5520_read(chip->client, MODE_STATUS, &reg_val);
+	if (ret)
+		goto out;
+
+	events =  reg_val & (OVP_INT | CMPR_INT | GPI_INT | KR_INT | KP_INT);
+
+	blocking_notifier_call_chain(&chip->notifier_list, events, NULL);
+	/* ACK, Sticky bits are W1C */
+	__adp5520_ack_bits(chip->client, MODE_STATUS, events);
+
+out:
+	enable_irq(chip->client->irq);
+}
+
+static irqreturn_t adp5520_irq_handler(int irq, void *data)
+{
+	struct adp5520_chip *chip = data;
+
+	disable_irq_nosync(irq);
+	schedule_work(&chip->irq_work);
+
+	return IRQ_HANDLED;
+}
+
+static int __remove_subdev(struct device *dev, void *unused)
+{
+	platform_device_unregister(to_platform_device(dev));
+	return 0;
+}
+
+static int adp5520_remove_subdevs(struct adp5520_chip *chip)
+{
+	return device_for_each_child(chip->dev, NULL, __remove_subdev);
+}
+
+static int __devinit adp5520_add_subdevs(struct adp5520_chip *chip,
+					struct adp5520_platform_data *pdata)
+{
+	struct adp5520_subdev_info *subdev;
+	struct platform_device *pdev;
+	int i, ret = 0;
+
+	for (i = 0; i < pdata->num_subdevs; i++) {
+		subdev = &pdata->subdevs[i];
+
+		pdev = platform_device_alloc(subdev->name, subdev->id);
+
+		pdev->dev.parent = chip->dev;
+		pdev->dev.platform_data = subdev->platform_data;
+
+		ret = platform_device_add(pdev);
+		if (ret)
+			goto failed;
+	}
+	return 0;
+
+failed:
+	adp5520_remove_subdevs(chip);
+	return ret;
+}
+
+static int __devinit adp5520_probe(struct i2c_client *client,
+					const struct i2c_device_id *id)
+{
+	struct adp5520_platform_data *pdata = client->dev.platform_data;
+	struct adp5520_chip *chip;
+	int ret;
+
+	if (!i2c_check_functionality(client->adapter,
+					I2C_FUNC_SMBUS_BYTE_DATA)) {
+		dev_err(&client->dev, "SMBUS Word Data not Supported\n");
+		return -EIO;
+	}
+
+	if (pdata == NULL) {
+		dev_err(&client->dev, "missing platform data\n");
+		return -ENODEV;
+	}
+
+	chip = kzalloc(sizeof(*chip), GFP_KERNEL);
+	if (!chip)
+		return -ENOMEM;
+
+	i2c_set_clientdata(client, chip);
+	chip->client = client;
+
+	chip->dev = &client->dev;
+	chip->irq = client->irq;
+	mutex_init(&chip->lock);
+
+	if (chip->irq) {
+		INIT_WORK(&chip->irq_work, adp5520_irq_work);
+		BLOCKING_INIT_NOTIFIER_HEAD(&chip->notifier_list);
+
+		ret = request_irq(chip->irq, adp5520_irq_handler,
+				IRQF_DISABLED | IRQF_TRIGGER_LOW,
+				"adp5520", chip);
+		if (ret) {
+			dev_err(&client->dev, "failed to request irq %d\n",
+					chip->irq);
+			goto out_free_chip;
+		}
+	}
+
+	ret = adp5520_write(chip->dev, MODE_STATUS, nSTNBY);
+	if (ret) {
+		dev_err(&client->dev, "failed to write\n");
+		goto out_free_irq;
+	}
+
+	ret = adp5520_add_subdevs(chip, pdata);
+
+	if (!ret)
+		return ret;
+
+out_free_irq:
+	if (chip->irq)
+		free_irq(chip->irq, chip);
+
+out_free_chip:
+	i2c_set_clientdata(client, NULL);
+	kfree(chip);
+
+	return ret;
+}
+
+static int __devexit adp5520_remove(struct i2c_client *client)
+{
+	struct adp5520_chip *chip = dev_get_drvdata(&client->dev);
+
+	if (chip->irq)
+		free_irq(chip->irq, chip);
+
+	adp5520_remove_subdevs(chip);
+	adp5520_write(chip->dev, MODE_STATUS, 0);
+	i2c_set_clientdata(client, NULL);
+	kfree(chip);
+	return 0;
+}
+
+#ifdef CONFIG_PM
+static int adp5520_suspend(struct i2c_client *client,
+				 pm_message_t state)
+{
+	struct adp5520_chip *chip = dev_get_drvdata(&client->dev);
+
+	adp5520_clr_bits(chip->dev, MODE_STATUS, nSTNBY);
+	return 0;
+}
+
+static int adp5520_resume(struct i2c_client *client)
+{
+	struct adp5520_chip *chip = dev_get_drvdata(&client->dev);
+
+	adp5520_set_bits(chip->dev, MODE_STATUS, nSTNBY);
+	return 0;
+}
+#else
+#define adp5520_suspend	NULL
+#define adp5520_resume	NULL
+#endif
+
+static const struct i2c_device_id adp5520_id[] = {
+	{ "pmic-adp5520", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, adp5520_id);
+
+static struct i2c_driver adp5520_driver = {
+	.driver = {
+		.name	= "adp5520",
+		.owner	= THIS_MODULE,
+	},
+	.probe		= adp5520_probe,
+	.remove		= __devexit_p(adp5520_remove),
+	.suspend	= adp5520_suspend,
+	.resume		= adp5520_resume,
+	.id_table 	= adp5520_id,
+};
+
+static int __init adp5520_init(void)
+{
+	return i2c_add_driver(&adp5520_driver);
+}
+module_init(adp5520_init);
+
+static void __exit adp5520_exit(void)
+{
+	i2c_del_driver(&adp5520_driver);
+}
+module_exit(adp5520_exit);
+
+MODULE_AUTHOR("Michael Hennerich <hennerich@blackfin.uclinux.org>");
+MODULE_DESCRIPTION("ADP5520(01) PMIC-MFD Driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/mfd/adp5520.h b/include/linux/mfd/adp5520.h
new file mode 100644
index 0000000..fffa1db
--- /dev/null
+++ b/include/linux/mfd/adp5520.h
@@ -0,0 +1,303 @@
+/*
+ * Definitions and platfrom data for Analog Devices
+ * ADP5520/ADP5501 MFD PMICs (Backlight, LED, GPIO and Keys)
+ *
+ * Copyright 2009 Analog Devices Inc.
+ *
+ * Licensed under the GPL-2 or later.
+ */
+
+
+#ifndef __LINUX_MFD_ADP5520_H
+#define __LINUX_MFD_ADP5520_H
+
+#define ID_ADP5520		5520
+#define ID_ADP5501		5501
+
+/*
+ * ADP5520/ADP5501 Register Map
+ */
+
+#define MODE_STATUS 		0x00
+#define INTERRUPT_ENABLE 	0x01
+#define BL_CONTROL 		0x02
+#define BL_TIME 		0x03
+#define BL_FADE 		0x04
+#define DAYLIGHT_MAX 		0x05
+#define DAYLIGHT_DIM 		0x06
+#define OFFICE_MAX 		0x07
+#define OFFICE_DIM 		0x08
+#define DARK_MAX 		0x09
+#define DARK_DIM 		0x0A
+#define BL_VALUE 		0x0B
+#define ALS_CMPR_CFG 		0x0C
+#define L2_TRIP 		0x0D
+#define L2_HYS 			0x0E
+#define L3_TRIP 		0x0F
+#define L3_HYS 			0x10
+#define LED_CONTROL 		0x11
+#define LED_TIME 		0x12
+#define LED_FADE 		0x13
+#define LED1_CURRENT 		0x14
+#define LED2_CURRENT 		0x15
+#define LED3_CURRENT 		0x16
+
+/*
+ * ADP5520 Register Map
+ */
+
+#define GPIO_CFG_1 		0x17
+#define GPIO_CFG_2 		0x18
+#define GPIO_IN 		0x19
+#define GPIO_OUT 		0x1A
+#define GPIO_INT_EN 		0x1B
+#define GPIO_INT_STAT 		0x1C
+#define GPIO_INT_LVL 		0x1D
+#define GPIO_DEBOUNCE 		0x1E
+#define GPIO_PULLUP 		0x1F
+#define KP_INT_STAT_1 		0x20
+#define KP_INT_STAT_2 		0x21
+#define KR_INT_STAT_1 		0x22
+#define KR_INT_STAT_2 		0x23
+#define KEY_STAT_1 		0x24
+#define KEY_STAT_2 		0x25
+
+/*
+ * MODE_STATUS bits
+ */
+
+#define nSTNBY		(1 << 7)
+#define BL_EN           (1 << 6)
+#define DIM_EN          (1 << 5)
+#define OVP_INT         (1 << 4)
+#define CMPR_INT        (1 << 3)
+#define GPI_INT         (1 << 2)
+#define KR_INT          (1 << 1)
+#define KP_INT          (1 << 0)
+
+/*
+ * INTERRUPT_ENABLE bits
+ */
+
+#define AUTO_LD_EN      (1 << 4)
+#define CMPR_IEN        (1 << 3)
+#define OVP_IEN         (1 << 2)
+#define KR_IEN          (1 << 1)
+#define KP_IEN          (1 << 0)
+
+/*
+ * BL_CONTROL bits
+ */
+
+#define BL_LVL          ((x) << 5)
+#define BL_LAW          ((x) << 4)
+#define BL_AUTO_ADJ     (1 << 3)
+#define OVP_EN          (1 << 2)
+#define FOVR            (1 << 1)
+#define KP_BL_EN        (1 << 0)
+
+/*
+ * ALS_CMPR_CFG bits
+ */
+
+#define L3_OUT		(1 << 3)
+#define L2_OUT		(1 << 2)
+#define L3_EN		(1 << 1)
+
+#define ADP5020_MAX_BRIGHTNESS	0x7F
+
+#define FADE_VAL(in, out)	((0xF & (in)) | ((0xF & (out)) << 4))
+#define BL_CTRL_VAL(law, auto)	(((1 & (auto)) << 3) | ((0x3 & (law)) << 4))
+#define ALS_CMPR_CFG_VAL(filt, l3_en)	(((0x7 & filt) << 5) | l3_en)
+
+/*
+ * LEDs subdevice bits and masks
+ */
+
+#define ADP5520_01_MAXLEDS 3
+
+#define FLAG_LED_MASK 		0x3
+#define FLAG_OFFT_SHIFT 	8
+#define FLAG_OFFT_MASK 		0x3
+
+#define R3_MODE		(1 << 5)
+#define C3_MODE		(1 << 4)
+#define LED_LAW		(1 << 3)
+#define LED3_EN		(1 << 2)
+#define LED2_EN		(1 << 1)
+#define LED1_EN		(1 << 0)
+
+/*
+ * GPIO subdevice bits and masks
+ */
+
+#define ADP5520_MAXGPIOS	8
+
+#define GPIO_C3		(1 << 7)	/* LED2 or GPIO7 aka C3 */
+#define GPIO_C2		(1 << 6)
+#define GPIO_C1		(1 << 5)
+#define GPIO_C0		(1 << 4)
+#define GPIO_R3		(1 << 3)	/* LED3 or GPIO3 aka R3 */
+#define GPIO_R2		(1 << 2)
+#define GPIO_R1		(1 << 1)
+#define GPIO_R0		(1 << 0)
+
+struct adp5520_gpio_platfrom_data {
+	unsigned gpio_start;
+	u8 gpio_en_mask;
+	u8 gpio_pullup_mask;
+};
+
+/*
+ * Keypad subdevice bits and masks
+ */
+
+#define ADP5520_MAXKEYS	16
+
+#define COL_C3 		(1 << 7)	/* LED2 or GPIO7 aka C3 */
+#define COL_C2		(1 << 6)
+#define COL_C1		(1 << 5)
+#define COL_C0		(1 << 4)
+#define ROW_R3		(1 << 3)	/* LED3 or GPIO3 aka R3 */
+#define ROW_R2		(1 << 2)
+#define ROW_R1		(1 << 1)
+#define ROW_R0		(1 << 0)
+
+#define KEY(row, col) (col + row * 4)
+#define ADP5520_KEYMAPSIZE	ADP5520_MAXKEYS
+
+struct adp5520_keys_platfrom_data {
+	int rows_en_mask;		/* Number of rows */
+	int cols_en_mask;		/* Number of columns */
+	const unsigned short *keymap;	/* Pointer to keymap */
+	unsigned short keymapsize;	/* Keymap size */
+	unsigned repeat:1;		/* Enable key repeat */
+};
+
+
+/*
+ * LEDs subdevice platfrom data
+ */
+
+#define FLAG_ID_ADP5520_LED1_ADP5501_LED0 	1	/* ADP5520 PIN ILED */
+#define FLAG_ID_ADP5520_LED2_ADP5501_LED1 	2	/* ADP5520 PIN C3 */
+#define FLAG_ID_ADP5520_LED3_ADP5501_LED2 	3	/* ADP5520 PIN R3 */
+
+#define LED_DIS_BLINK	(0 << FLAG_OFFT_SHIFT)
+#define LED_OFFT_600ms	(1 << FLAG_OFFT_SHIFT)
+#define LED_OFFT_800ms	(2 << FLAG_OFFT_SHIFT)
+#define LED_OFFT_1200ms	(3 << FLAG_OFFT_SHIFT)
+
+#define LED_ONT_200ms	0
+#define LED_ONT_600ms	1
+#define LED_ONT_800ms	2
+#define LED_ONT_1200ms	3
+
+struct adp5520_leds_platfrom_data {
+	int num_leds;
+	struct led_info	*leds;
+	u8 fade_in;		/* Backlight Fade-In Timer */
+	u8 fade_out;		/* Backlight Fade-Out Timer */
+	u8 led_on_time;
+};
+
+/*
+ * Backlight subdevice platfrom data
+ */
+
+#define FADE_T_DIS	0	/* Fade Timer Disabled */
+#define FADE_T_300ms	1	/* 0.3 Sec */
+#define FADE_T_600ms	2
+#define FADE_T_900ms	3
+#define FADE_T_1200ms	4
+#define FADE_T_1500ms	5
+#define FADE_T_1800ms	6
+#define FADE_T_2100ms	7
+#define FADE_T_2400ms	8
+#define FADE_T_2700ms	9
+#define FADE_T_3000ms	10
+#define FADE_T_3500ms	11
+#define FADE_T_4000ms	12
+#define FADE_T_4500ms	13
+#define FADE_T_5000ms	14
+#define FADE_T_5500ms	15	/* 5.5 Sec */
+
+#define BL_LAW_LINEAR 	0
+#define BL_LAW_SQUARE 	1
+#define BL_LAW_CUBIC1 	2
+#define BL_LAW_CUBIC2 	3
+
+#define BL_AMBL_FILT_80ms 	0	/* Light sensor filter time */
+#define BL_AMBL_FILT_160ms 	1
+#define BL_AMBL_FILT_320ms 	2
+#define BL_AMBL_FILT_640ms 	3
+#define BL_AMBL_FILT_1280ms 	4
+#define BL_AMBL_FILT_2560ms 	5
+#define BL_AMBL_FILT_5120ms 	6
+#define BL_AMBL_FILT_10240ms 	7	/* 10.24 sec */
+
+	/*
+	 * Blacklight current 0..30mA
+	 */
+#define BL_CUR_mA(I)		((I * 127) / 30)
+
+	/*
+	 * L2 comparator current 0..1000uA
+	 */
+#define L2_COMP_CURR_uA(I)	((I * 255) / 1000)
+
+	/*
+	 * L3 comparator current 0..127uA
+	 */
+#define L3_COMP_CURR_uA(I)	((I * 255) / 127)
+
+struct adp5520_backlight_platfrom_data {
+	u8 fade_in;		/* Backlight Fade-In Timer */
+	u8 fade_out;		/* Backlight Fade-Out Timer */
+	u8 fade_led_law;	/* fade-on/fade-off transfer characteristic */
+
+	u8 en_ambl_sens;	/* 1 = enable ambient light sensor */
+	u8 abml_filt;		/* Light sensor filter time */
+	u8 l1_daylight_max;	/* use BL_CUR_mA(I) 0 <= I <= 30 mA */
+	u8 l1_daylight_dim;	/* typ = 0, use BL_CUR_mA(I) 0 <= I <= 30 mA */
+	u8 l2_office_max;	/* use BL_CUR_mA(I) 0 <= I <= 30 mA */
+	u8 l2_office_dim;	/* typ = 0, use BL_CUR_mA(I) 0 <= I <= 30 mA */
+	u8 l3_dark_max;		/* use BL_CUR_mA(I) 0 <= I <= 30 mA */
+	u8 l3_dark_dim;		/* typ = 0, use BL_CUR_mA(I) 0 <= I <= 30 mA */
+	u8 l2_trip;		/* use L2_COMP_CURR_uA(I) 0 <= I <= 1000 uA */
+	u8 l2_hyst;		/* use L2_COMP_CURR_uA(I) 0 <= I <= 1000 uA */
+	u8 l3_trip;		/* use L3_COMP_CURR_uA(I) 0 <= I <= 127 uA */
+	u8 l3_hyst;		/* use L3_COMP_CURR_uA(I) 0 <= I <= 127 uA */
+};
+
+/*
+ * MFD chip platfrom data
+ */
+
+struct adp5520_subdev_info {
+	int		id;
+	const char	*name;
+	void		*platform_data;
+};
+
+struct adp5520_platform_data {
+	int num_subdevs;
+	struct adp5520_subdev_info *subdevs;
+};
+
+/*
+ * MFD chip functions
+ */
+
+extern int adp5520_read(struct device *dev, int reg, uint8_t *val);
+extern int adp5520_write(struct device *dev, int reg, u8 val);
+extern int adp5520_clr_bits(struct device *dev, int reg, uint8_t bit_mask);
+extern int adp5520_set_bits(struct device *dev, int reg, uint8_t bit_mask);
+
+extern int adp5520_register_notifier(struct device *dev,
+		 struct notifier_block *nb, unsigned int events);
+
+extern int adp5520_unregister_notifier(struct device *dev,
+		struct notifier_block *nb, unsigned int events);
+
+#endif /* __LINUX_MFD_ADP5520_H */
-- 
1.6.5.rc1

Re: [Uclinux-dist-devel] [PATCH v2] mfd: ADP5520 Multifunction LCD Backlight and Keypad Input Device Driver

[Mike Frysinger]

On Wed, Sep 23, 2009 at 01:11, Mike Frysinger wrote:
> From: Michael Hennerich <michael.hennerich@analog.com>
>
> Base driver for Analog Devices ADP5520/ADP5501 MFD PMICs

could you check this out/pick up Andrew ?
-mike

Re: [Uclinux-dist-devel] [PATCH v2] mfd: ADP5520 Multifunction LCD Backlight and Keypad Input Device Driver

[Andrew Morton]

On Tue, 29 Sep 2009 17:04:21 -0400
Mike Frysinger <vapier.adi@gmail.com> wrote:

> On Wed, Sep 23, 2009 at 01:11, Mike Frysinger wrote:
> > From: Michael Hennerich <michael.hennerich@analog.com>
> >
> > Base driver for Analog Devices ADP5520/ADP5501 MFD PMICs
> 
> could you check this out/pick up Andrew ?

Spose so.  Is Samuel not around?

Does this patch have any dependencies on other stuff we weren't told about?

Re: [Uclinux-dist-devel] [PATCH v2] mfd: ADP5520 Multifunction LCD Backlight and Keypad Input Device Driver

[Mike Frysinger]

On Tue, Sep 29, 2009 at 17:14, Andrew Morton wrote:
> On Tue, 29 Sep 2009 17:04:21 -0400 Mike Frysinger wrote:
>> On Wed, Sep 23, 2009 at 01:11, Mike Frysinger wrote:
>> > From: Michael Hennerich <michael.hennerich@analog.com>
>> >
>> > Base driver for Analog Devices ADP5520/ADP5501 MFD PMICs
>>
>> could you check this out/pick up Andrew ?
>
> Spose so.  Is Samuel not around?

all the other MFD patches were noticed/reviewed/picked up

> Does this patch have any dependencies on other stuff we weren't told about?

the other way around.  this is the core.
-mike

Re: [PATCH v2] mfd: ADP5520 Multifunction LCD Backlight and Keypad Input Device Driver

[Andrew Morton]

On Wed, 23 Sep 2009 01:11:04 -0400
Mike Frysinger <vapier@gentoo.org> wrote:

> +static void adp5520_irq_work(struct work_struct *work)
> +{
> +	struct adp5520_chip *chip =
> +		container_of(work, struct adp5520_chip, irq_work);
> +	unsigned int events;
> +	uint8_t reg_val;
> +	int ret;
> +
> +	ret = __adp5520_read(chip->client, MODE_STATUS, &reg_val);
> +	if (ret)
> +		goto out;
> +
> +	events =  reg_val & (OVP_INT | CMPR_INT | GPI_INT | KR_INT | KP_INT);
> +
> +	blocking_notifier_call_chain(&chip->notifier_list, events, NULL);
> +	/* ACK, Sticky bits are W1C */
> +	__adp5520_ack_bits(chip->client, MODE_STATUS, events);
> +
> +out:
> +	enable_irq(chip->client->irq);
> +}
> +
> +static irqreturn_t adp5520_irq_handler(int irq, void *data)
> +{
> +	struct adp5520_chip *chip = data;
> +
> +	disable_irq_nosync(irq);
> +	schedule_work(&chip->irq_work);
> +
> +	return IRQ_HANDLED;
> +}

Disabling interrutps for an arbitrarily long period of time is pretty
nasty.  Especially if some poor innocent device is trying to share that
irq (can this happen?).

Is there no other way?

Re: [Uclinux-dist-devel] [PATCH v2] mfd: ADP5520 Multifunction LCD Backlight and Keypad Input Device Driver

[Samuel Ortiz]

On Tue, Sep 29, 2009 at 02:14:50PM -0700, Andrew Morton wrote:
> On Tue, 29 Sep 2009 17:04:21 -0400
> Mike Frysinger <vapier.adi@gmail.com> wrote:
> 
> > On Wed, Sep 23, 2009 at 01:11, Mike Frysinger wrote:
> > > From: Michael Hennerich <michael.hennerich@analog.com>
> > >
> > > Base driver for Analog Devices ADP5520/ADP5501 MFD PMICs
> > 
> > could you check this out/pick up Andrew ?
> 
> Spose so.  Is Samuel not around?
I'm around, just too busy those days to look at mfd stuff. Sorry for the
latency...
I'll grab my mfd hat tomorrow, look at the various patches pending and update
my tree.

Cheers,
Samuel.
 
> Does this patch have any dependencies on other stuff we weren't told about?
> 

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

RE: [PATCH v2] mfd: ADP5520 Multifunction LCD Backlight and Keypad Input Device Driver

[Hennerich, Michael]

>From: Andrew Morton [mailto:akpm@linux-foundation.org]
ject: Re: [PATCH v2] mfd: ADP5520 Multifunction LCD Backlight and Keypad
Input Device Driver
>
>On Wed, 23 Sep 2009 01:11:04 -0400
>Mike Frysinger <vapier@gentoo.org> wrote:
>
>> +static void adp5520_irq_work(struct work_struct *work)
>> +{
>> +	struct adp5520_chip *chip =
>> +		container_of(work, struct adp5520_chip, irq_work);
>> +	unsigned int events;
>> +	uint8_t reg_val;
>> +	int ret;
>> +
>> +	ret = __adp5520_read(chip->client, MODE_STATUS, &reg_val);
>> +	if (ret)
>> +		goto out;
>> +
>> +	events =  reg_val & (OVP_INT | CMPR_INT | GPI_INT | KR_INT |
KP_INT);
>> +
>> +	blocking_notifier_call_chain(&chip->notifier_list, events,
NULL);
>> +	/* ACK, Sticky bits are W1C */
>> +	__adp5520_ack_bits(chip->client, MODE_STATUS, events);
>> +
>> +out:
>> +	enable_irq(chip->client->irq);
>> +}
>> +
>> +static irqreturn_t adp5520_irq_handler(int irq, void *data)
>> +{
>> +	struct adp5520_chip *chip = data;
>> +
>> +	disable_irq_nosync(irq);
>> +	schedule_work(&chip->irq_work);
>> +
>> +	return IRQ_HANDLED;
>> +}
>
>Disabling interrutps for an arbitrarily long period of time is pretty
>nasty.  Especially if some poor innocent device is trying to share that
>irq (can this happen?).
>
>Is there no other way?

Well - incredible slow peripherals such as I2C can't be directly
serviced from non-sleepy context such as IRQ - in case someone wants to
use level sensitive IRQs - disabling the interrupt source until serviced
used to be the common way. Typically work queues are serviced in the
shadow of the IRQ caused the Interrupt. Shared IRQs are in addition
somehow OR'ed with each other. So it really doesn't matter, if another
device asserts a SHARED IRQ; 1 OR 1 = 1  - the kernel just needs to make
sure    
that IRQs are serviced until absence... (While in return the device
caused the IRQ de-asserts its signal to the host)

This driver targets mostly embedded devices, they typically feature
dedicated interrupt channels for each Interrupt source.   

>(can this happen?)

I don't think so 

-Michael

 

Re: [PATCH v2] mfd: ADP5520 Multifunction LCD Backlight and Keypad Input Device Driver

[Samuel Ortiz]

Hi Mike,

Some comments on this patch:

On Wed, Sep 23, 2009 at 01:11:04AM -0400, Mike Frysinger wrote:
> From: Michael Hennerich <michael.hennerich@analog.com>
> 
> Base driver for Analog Devices ADP5520/ADP5501 MFD PMICs
> 
> Subdevs:
> LCD Backlight   : drivers/video/backlight/adp5520_bl
> LEDs            : drivers/led/leds-adp5520
> GPIO            : drivers/gpio/adp5520-gpio (ADP5520 only)
> Keys            : drivers/input/keyboard/adp5520-keys (ADP5520 only)
> 
> Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
> Signed-off-by: Bryan Wu <cooloney@kernel.org>
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> ---
> v2
> 	- fix return type of irq handler
> 	- fix unbalanced paren in BL_CTRL_VAL() macro
> 
>  drivers/mfd/Kconfig         |   10 ++
>  drivers/mfd/Makefile        |    1 +
>  drivers/mfd/adp5520.c       |  371 +++++++++++++++++++++++++++++++++++++++++++
>  include/linux/mfd/adp5520.h |  303 +++++++++++++++++++++++++++++++++++
>  4 files changed, 685 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/mfd/adp5520.c
>  create mode 100644 include/linux/mfd/adp5520.h
> 
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 570be13..34e8595 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -160,6 +160,16 @@ config PMIC_DA903X
>  	  individual components like LCD backlight, voltage regulators,
>  	  LEDs and battery-charger under the corresponding menus.
>  
> +config PMIC_ADP5520
> +	bool "Analog Devices ADP5520/01 MFD PMIC Core Support"
> +	depends on I2C=y
> +	help
> +	  Say yes here to add support for Analog Devices AD5520 and ADP5501,
> +	  Multifunction Power Management IC. This includes
> +	  the I2C driver and the core APIs _only_, you have to select
> +	  individual components like LCD backlight, LEDs, GPIOs and Kepad
> +	  under the corresponding menus.
> +
>  config MFD_WM8400
>  	tristate "Support Wolfson Microelectronics WM8400"
>  	select MFD_CORE
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index f3b277b..a42248e 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -50,3 +50,4 @@ obj-$(CONFIG_PCF50633_ADC)	+= pcf50633-adc.o
>  obj-$(CONFIG_PCF50633_GPIO)	+= pcf50633-gpio.o
>  obj-$(CONFIG_AB3100_CORE)	+= ab3100-core.o
>  obj-$(CONFIG_AB3100_OTP)	+= ab3100-otp.o
> +obj-$(CONFIG_PMIC_ADP5520)	+= adp5520.o
> diff --git a/drivers/mfd/adp5520.c b/drivers/mfd/adp5520.c
> new file mode 100644
> index 0000000..1083626
> --- /dev/null
> +++ b/drivers/mfd/adp5520.c
> @@ -0,0 +1,371 @@
> +/*
> + * Base driver for Analog Devices ADP5520/ADP5501 MFD PMICs
> + * LCD Backlight: drivers/video/backlight/adp5520_bl
> + * LEDs		: drivers/led/leds-adp5520
> + * GPIO		: drivers/gpio/adp5520-gpio (ADP5520 only)
> + * Keys		: drivers/input/keyboard/adp5520-keys (ADP5520 only)
> + *
> + * Copyright 2009 Analog Devices Inc.
> + *
> + * Derived from da903x:
> + * Copyright (C) 2008 Compulab, Ltd.
> + * 	Mike Rapoport <mike@compulab.co.il>
> + *
> + * Copyright (C) 2006-2008 Marvell International Ltd.
> + * 	Eric Miao <eric.miao@marvell.com>
> + *
> + * Licensed under the GPL-2 or later.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/workqueue.h>
> +#include <linux/i2c.h>
> +
> +#include <linux/mfd/adp5520.h>
> +
> +struct adp5520_chip {
> +	struct i2c_client *client;
> +	struct device *dev;
> +	struct mutex lock;
> +	struct work_struct irq_work;
> +	struct blocking_notifier_head notifier_list;
> +	int irq;
> +};
> +
> +static int __adp5520_read(struct i2c_client *client,
> +				int reg, uint8_t *val)
> +{
> +	int ret;
> +
> +	ret = i2c_smbus_read_byte_data(client, reg);
> +	if (ret < 0) {
> +		dev_err(&client->dev, "failed reading at 0x%02x\n", reg);
> +		return ret;
> +	}
> +
> +	*val = (uint8_t)ret;
> +	return 0;
> +}
> +
> +static int __adp5520_write(struct i2c_client *client,
> +				 int reg, uint8_t val)
> +{
> +	int ret;
> +
> +	ret = i2c_smbus_write_byte_data(client, reg, val);
> +	if (ret < 0) {
> +		dev_err(&client->dev, "failed writing 0x%02x to 0x%02x\n",
> +				val, reg);
> +		return ret;
> +	}
> +	return 0;
> +}
> +
> +int __adp5520_ack_bits(struct i2c_client *client, int reg, uint8_t bit_mask)
> +{
> +	struct adp5520_chip *chip = i2c_get_clientdata(client);
> +	uint8_t reg_val;
> +	int ret;
> +
> +	mutex_lock(&chip->lock);
> +
> +	ret = __adp5520_read(client, reg, &reg_val);
> +
> +	if (!ret) {
> +		reg_val |= bit_mask;
> +		ret = __adp5520_write(client, reg, reg_val);
> +	}
> +
> +	mutex_unlock(&chip->lock);
> +	return ret;
> +}
> +
> +int adp5520_write(struct device *dev, int reg, uint8_t val)
> +{
> +	return __adp5520_write(to_i2c_client(dev), reg, val);
> +}
> +EXPORT_SYMBOL_GPL(adp5520_write);
> +
> +int adp5520_read(struct device *dev, int reg, uint8_t *val)
> +{
> +	return __adp5520_read(to_i2c_client(dev), reg, val);
> +}
> +EXPORT_SYMBOL_GPL(adp5520_read);
> +
> +int adp5520_set_bits(struct device *dev, int reg, uint8_t bit_mask)
> +{
> +	struct adp5520_chip *chip = dev_get_drvdata(dev);
> +	uint8_t reg_val;
> +	int ret;
> +
> +	mutex_lock(&chip->lock);
> +
> +	ret = __adp5520_read(chip->client, reg, &reg_val);
> +
> +	if (!ret && ((reg_val & bit_mask) == 0)) {
> +		reg_val |= bit_mask;
> +		ret = __adp5520_write(chip->client, reg, reg_val);
> +	}
> +
> +	mutex_unlock(&chip->lock);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(adp5520_set_bits);
> +
> +int adp5520_clr_bits(struct device *dev, int reg, uint8_t bit_mask)
> +{
> +	struct adp5520_chip *chip = dev_get_drvdata(dev);
> +	uint8_t reg_val;
> +	int ret;
> +
> +	mutex_lock(&chip->lock);
> +
> +	ret = __adp5520_read(chip->client, reg, &reg_val);
> +
> +	if (!ret && (reg_val & bit_mask)) {
> +		reg_val &= ~bit_mask;
> +		ret = __adp5520_write(chip->client, reg, reg_val);
> +	}
> +
> +	mutex_unlock(&chip->lock);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(adp5520_clr_bits);
> +
> +int adp5520_register_notifier(struct device *dev, struct notifier_block *nb,
> +				unsigned int events)
> +{
> +	struct adp5520_chip *chip = dev_get_drvdata(dev);
> +
> +	if (chip->irq) {
> +		adp5520_set_bits(chip->dev, INTERRUPT_ENABLE,
> +			events & (KP_IEN | KR_IEN | OVP_IEN | CMPR_IEN));
> +
> +		return blocking_notifier_chain_register(&chip->notifier_list,
> +			 nb);
> +	}
> +
> +	return -ENODEV;
> +}
> +EXPORT_SYMBOL_GPL(adp5520_register_notifier);
> +
> +int adp5520_unregister_notifier(struct device *dev, struct notifier_block *nb,
> +				unsigned int events)
> +{
> +	struct adp5520_chip *chip = dev_get_drvdata(dev);
> +
> +	adp5520_clr_bits(chip->dev, INTERRUPT_ENABLE,
> +		events & (KP_IEN | KR_IEN | OVP_IEN | CMPR_IEN));
> +
> +	return blocking_notifier_chain_unregister(&chip->notifier_list, nb);
> +}
> +EXPORT_SYMBOL_GPL(adp5520_unregister_notifier);
> +
> +static void adp5520_irq_work(struct work_struct *work)
> +{
> +	struct adp5520_chip *chip =
> +		container_of(work, struct adp5520_chip, irq_work);
> +	unsigned int events;
> +	uint8_t reg_val;
> +	int ret;
> +
> +	ret = __adp5520_read(chip->client, MODE_STATUS, &reg_val);
> +	if (ret)
> +		goto out;
> +
> +	events =  reg_val & (OVP_INT | CMPR_INT | GPI_INT | KR_INT | KP_INT);
> +
> +	blocking_notifier_call_chain(&chip->notifier_list, events, NULL);
> +	/* ACK, Sticky bits are W1C */
> +	__adp5520_ack_bits(chip->client, MODE_STATUS, events);
> +
> +out:
> +	enable_irq(chip->client->irq);
> +}
> +
> +static irqreturn_t adp5520_irq_handler(int irq, void *data)
> +{
> +	struct adp5520_chip *chip = data;
> +
> +	disable_irq_nosync(irq);
> +	schedule_work(&chip->irq_work);
Have you considered using a threaded irq handler here ?


> +	return IRQ_HANDLED;
> +}
> +
> +static int __remove_subdev(struct device *dev, void *unused)
> +{
> +	platform_device_unregister(to_platform_device(dev));
> +	return 0;
> +}
> +
> +static int adp5520_remove_subdevs(struct adp5520_chip *chip)
> +{
> +	return device_for_each_child(chip->dev, NULL, __remove_subdev);
> +}
> +
> +static int __devinit adp5520_add_subdevs(struct adp5520_chip *chip,
> +					struct adp5520_platform_data *pdata)
> +{
> +	struct adp5520_subdev_info *subdev;
> +	struct platform_device *pdev;
> +	int i, ret = 0;
> +
> +	for (i = 0; i < pdata->num_subdevs; i++) {
> +		subdev = &pdata->subdevs[i];
> +
> +		pdev = platform_device_alloc(subdev->name, subdev->id);
> +
> +		pdev->dev.parent = chip->dev;
> +		pdev->dev.platform_data = subdev->platform_data;
> +
> +		ret = platform_device_add(pdev);
> +		if (ret)
> +			goto failed;
> +	}
> +	return 0;
Here I would expect the MFD core driver to know about all the potential
subdevices and add them in that routine, and not take the subdevices list from
the platform definition.
I realize da903x has the same issue btw.

Also, please note that you could use the mfd-core API for adding devices, but
that's just optional.

Cheers,
Samuel.

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

RE: [PATCH v2] mfd: ADP5520 Multifunction LCD Backlight and Keypad Input Device Driver

[Hennerich, Michael]

Hi Samuel,

>From: Samuel Ortiz [mailto:sameo@linux.intel.com]
>Hi Mike,
>
>Some comments on this patch:
>
>On Wed, Sep 23, 2009 at 01:11:04AM -0400, Mike Frysinger wrote:
>> From: Michael Hennerich <michael.hennerich@analog.com>
>>
>> Base driver for Analog Devices ADP5520/ADP5501 MFD PMICs
>>
>> Subdevs:
>> LCD Backlight   : drivers/video/backlight/adp5520_bl
>> LEDs            : drivers/led/leds-adp5520
>> GPIO            : drivers/gpio/adp5520-gpio (ADP5520 only)
>> Keys            : drivers/input/keyboard/adp5520-keys (ADP5520 only)
>>
>> Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
>> Signed-off-by: Bryan Wu <cooloney@kernel.org>
>> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
>> ---
>> v2
>> 	- fix return type of irq handler
>> 	- fix unbalanced paren in BL_CTRL_VAL() macro
>>
>>  drivers/mfd/Kconfig         |   10 ++
>>  drivers/mfd/Makefile        |    1 +
>>  drivers/mfd/adp5520.c       |  371
+++++++++++++++++++++++++++++++++++++++++++
>>  include/linux/mfd/adp5520.h |  303
+++++++++++++++++++++++++++++++++++
>>  4 files changed, 685 insertions(+), 0 deletions(-)
>>  create mode 100644 drivers/mfd/adp5520.c
>>  create mode 100644 include/linux/mfd/adp5520.h
>>
>> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
>> index 570be13..34e8595 100644
>> --- a/drivers/mfd/Kconfig
>> +++ b/drivers/mfd/Kconfig
>> @@ -160,6 +160,16 @@ config PMIC_DA903X
>>  	  individual components like LCD backlight, voltage regulators,
>>  	  LEDs and battery-charger under the corresponding menus.
>>
>> +config PMIC_ADP5520
>> +	bool "Analog Devices ADP5520/01 MFD PMIC Core Support"
>> +	depends on I2C=y
>> +	help
>> +	  Say yes here to add support for Analog Devices AD5520 and
ADP5501,
>> +	  Multifunction Power Management IC. This includes
>> +	  the I2C driver and the core APIs _only_, you have to select
>> +	  individual components like LCD backlight, LEDs, GPIOs and
Kepad
>> +	  under the corresponding menus.
>> +
>>  config MFD_WM8400
>>  	tristate "Support Wolfson Microelectronics WM8400"
>>  	select MFD_CORE
>> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
>> index f3b277b..a42248e 100644
>> --- a/drivers/mfd/Makefile
>> +++ b/drivers/mfd/Makefile
>> @@ -50,3 +50,4 @@ obj-$(CONFIG_PCF50633_ADC)	+= pcf50633-adc.o
>>  obj-$(CONFIG_PCF50633_GPIO)	+= pcf50633-gpio.o
>>  obj-$(CONFIG_AB3100_CORE)	+= ab3100-core.o
>>  obj-$(CONFIG_AB3100_OTP)	+= ab3100-otp.o
>> +obj-$(CONFIG_PMIC_ADP5520)	+= adp5520.o
>> diff --git a/drivers/mfd/adp5520.c b/drivers/mfd/adp5520.c
>> new file mode 100644
>> index 0000000..1083626
>> --- /dev/null
>> +++ b/drivers/mfd/adp5520.c
>> @@ -0,0 +1,371 @@
>> +/*
>> + * Base driver for Analog Devices ADP5520/ADP5501 MFD PMICs
>> + * LCD Backlight: drivers/video/backlight/adp5520_bl
>> + * LEDs		: drivers/led/leds-adp5520
>> + * GPIO		: drivers/gpio/adp5520-gpio (ADP5520 only)
>> + * Keys		: drivers/input/keyboard/adp5520-keys (ADP5520
only)
>> + *
>> + * Copyright 2009 Analog Devices Inc.
>> + *
>> + * Derived from da903x:
>> + * Copyright (C) 2008 Compulab, Ltd.
>> + * 	Mike Rapoport <mike@compulab.co.il>
>> + *
>> + * Copyright (C) 2006-2008 Marvell International Ltd.
>> + * 	Eric Miao <eric.miao@marvell.com>
>> + *
>> + * Licensed under the GPL-2 or later.
>> + */
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/init.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/irq.h>
>> +#include <linux/workqueue.h>
>> +#include <linux/i2c.h>
>> +
>> +#include <linux/mfd/adp5520.h>
>> +
>> +struct adp5520_chip {
>> +	struct i2c_client *client;
>> +	struct device *dev;
>> +	struct mutex lock;
>> +	struct work_struct irq_work;
>> +	struct blocking_notifier_head notifier_list;
>> +	int irq;
>> +};
>> +
>> +static int __adp5520_read(struct i2c_client *client,
>> +				int reg, uint8_t *val)
>> +{
>> +	int ret;
>> +
>> +	ret = i2c_smbus_read_byte_data(client, reg);
>> +	if (ret < 0) {
>> +		dev_err(&client->dev, "failed reading at 0x%02x\n",
reg);
>> +		return ret;
>> +	}
>> +
>> +	*val = (uint8_t)ret;
>> +	return 0;
>> +}
>> +
>> +static int __adp5520_write(struct i2c_client *client,
>> +				 int reg, uint8_t val)
>> +{
>> +	int ret;
>> +
>> +	ret = i2c_smbus_write_byte_data(client, reg, val);
>> +	if (ret < 0) {
>> +		dev_err(&client->dev, "failed writing 0x%02x to
0x%02x\n",
>> +				val, reg);
>> +		return ret;
>> +	}
>> +	return 0;
>> +}
>> +
>> +int __adp5520_ack_bits(struct i2c_client *client, int reg, uint8_t
bit_mask)
>> +{
>> +	struct adp5520_chip *chip = i2c_get_clientdata(client);
>> +	uint8_t reg_val;
>> +	int ret;
>> +
>> +	mutex_lock(&chip->lock);
>> +
>> +	ret = __adp5520_read(client, reg, &reg_val);
>> +
>> +	if (!ret) {
>> +		reg_val |= bit_mask;
>> +		ret = __adp5520_write(client, reg, reg_val);
>> +	}
>> +
>> +	mutex_unlock(&chip->lock);
>> +	return ret;
>> +}
>> +
>> +int adp5520_write(struct device *dev, int reg, uint8_t val)
>> +{
>> +	return __adp5520_write(to_i2c_client(dev), reg, val);
>> +}
>> +EXPORT_SYMBOL_GPL(adp5520_write);
>> +
>> +int adp5520_read(struct device *dev, int reg, uint8_t *val)
>> +{
>> +	return __adp5520_read(to_i2c_client(dev), reg, val);
>> +}
>> +EXPORT_SYMBOL_GPL(adp5520_read);
>> +
>> +int adp5520_set_bits(struct device *dev, int reg, uint8_t bit_mask)
>> +{
>> +	struct adp5520_chip *chip = dev_get_drvdata(dev);
>> +	uint8_t reg_val;
>> +	int ret;
>> +
>> +	mutex_lock(&chip->lock);
>> +
>> +	ret = __adp5520_read(chip->client, reg, &reg_val);
>> +
>> +	if (!ret && ((reg_val & bit_mask) == 0)) {
>> +		reg_val |= bit_mask;
>> +		ret = __adp5520_write(chip->client, reg, reg_val);
>> +	}
>> +
>> +	mutex_unlock(&chip->lock);
>> +	return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(adp5520_set_bits);
>> +
>> +int adp5520_clr_bits(struct device *dev, int reg, uint8_t bit_mask)
>> +{
>> +	struct adp5520_chip *chip = dev_get_drvdata(dev);
>> +	uint8_t reg_val;
>> +	int ret;
>> +
>> +	mutex_lock(&chip->lock);
>> +
>> +	ret = __adp5520_read(chip->client, reg, &reg_val);
>> +
>> +	if (!ret && (reg_val & bit_mask)) {
>> +		reg_val &= ~bit_mask;
>> +		ret = __adp5520_write(chip->client, reg, reg_val);
>> +	}
>> +
>> +	mutex_unlock(&chip->lock);
>> +	return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(adp5520_clr_bits);
>> +
>> +int adp5520_register_notifier(struct device *dev, struct
notifier_block *nb,
>> +				unsigned int events)
>> +{
>> +	struct adp5520_chip *chip = dev_get_drvdata(dev);
>> +
>> +	if (chip->irq) {
>> +		adp5520_set_bits(chip->dev, INTERRUPT_ENABLE,
>> +			events & (KP_IEN | KR_IEN | OVP_IEN |
CMPR_IEN));
>> +
>> +		return
blocking_notifier_chain_register(&chip->notifier_list,
>> +			 nb);
>> +	}
>> +
>> +	return -ENODEV;
>> +}
>> +EXPORT_SYMBOL_GPL(adp5520_register_notifier);
>> +
>> +int adp5520_unregister_notifier(struct device *dev, struct
notifier_block *nb,
>> +				unsigned int events)
>> +{
>> +	struct adp5520_chip *chip = dev_get_drvdata(dev);
>> +
>> +	adp5520_clr_bits(chip->dev, INTERRUPT_ENABLE,
>> +		events & (KP_IEN | KR_IEN | OVP_IEN | CMPR_IEN));
>> +
>> +	return blocking_notifier_chain_unregister(&chip->notifier_list,
nb);
>> +}
>> +EXPORT_SYMBOL_GPL(adp5520_unregister_notifier);
>> +
>> +static void adp5520_irq_work(struct work_struct *work)
>> +{
>> +	struct adp5520_chip *chip =
>> +		container_of(work, struct adp5520_chip, irq_work);
>> +	unsigned int events;
>> +	uint8_t reg_val;
>> +	int ret;
>> +
>> +	ret = __adp5520_read(chip->client, MODE_STATUS, &reg_val);
>> +	if (ret)
>> +		goto out;
>> +
>> +	events =  reg_val & (OVP_INT | CMPR_INT | GPI_INT | KR_INT |
KP_INT);
>> +
>> +	blocking_notifier_call_chain(&chip->notifier_list, events,
NULL);
>> +	/* ACK, Sticky bits are W1C */
>> +	__adp5520_ack_bits(chip->client, MODE_STATUS, events);
>> +
>> +out:
>> +	enable_irq(chip->client->irq);
>> +}
>> +
>> +static irqreturn_t adp5520_irq_handler(int irq, void *data)
>> +{
>> +	struct adp5520_chip *chip = data;
>> +
>> +	disable_irq_nosync(irq);
>> +	schedule_work(&chip->irq_work);
>Have you considered using a threaded irq handler here ?

The Linux version I developed this driver on didn't feature threaded irq
handlers.
But thanks I'm going to take a look here.

>
>
>> +	return IRQ_HANDLED;
>> +}
>> +
>> +static int __remove_subdev(struct device *dev, void *unused)
>> +{
>> +	platform_device_unregister(to_platform_device(dev));
>> +	return 0;
>> +}
>> +
>> +static int adp5520_remove_subdevs(struct adp5520_chip *chip)
>> +{
>> +	return device_for_each_child(chip->dev, NULL, __remove_subdev);
>> +}
>> +
>> +static int __devinit adp5520_add_subdevs(struct adp5520_chip *chip,
>> +					struct adp5520_platform_data
*pdata)
>> +{
>> +	struct adp5520_subdev_info *subdev;
>> +	struct platform_device *pdev;
>> +	int i, ret = 0;
>> +
>> +	for (i = 0; i < pdata->num_subdevs; i++) {
>> +		subdev = &pdata->subdevs[i];
>> +
>> +		pdev = platform_device_alloc(subdev->name, subdev->id);
>> +
>> +		pdev->dev.parent = chip->dev;
>> +		pdev->dev.platform_data = subdev->platform_data;
>> +
>> +		ret = platform_device_add(pdev);
>> +		if (ret)
>> +			goto failed;
>> +	}
>> +	return 0;
>Here I would expect the MFD core driver to know about all the potential
>subdevices and add them in that routine, and not take the subdevices
list from
>the platform definition.
>I realize da903x has the same issue btw.

The ADP5520 is an I2C device and gets registered via struct
i2c_board_info.
How about having multiple ADP5520 in a system with different I2C salve
addresses?
Each ADP5520 having different Keypad, Backlight and GPIO configurations
passed in platform_data?
How will they map? The MFD core is struct resource centric, which is not
going to help here.

I could be doing something like this:
                           
Index: drivers/mfd/adp5520.c

===================================================================

--- drivers/mfd/adp5520.c       (revision 7535)

+++ drivers/mfd/adp5520.c       (working copy)

@@ -25,6 +25,7 @@

 #include <linux/irq.h>

 #include <linux/workqueue.h>

 #include <linux/i2c.h>

+#include <linux/mfd/core.h>

 

 #include <linux/mfd/adp5520.h>

 

@@ -235,6 +236,21 @@

        return ret;

 }

 

+static struct mfd_cell __devinitdata adp5520_cells[] = {

+       {

+               .name = "adp5520-backlight",

+       },

+       {

+               .name = "adp5520-led",

+       },

+       {
+               .name = "adp5520-gpio",
+       },
+       {
+               .name = "adp5520-keys",
+       },
+};
+
 static int __devinit adp5520_probe(struct i2c_client *client,
                                        const struct i2c_device_id *id)
 {
@@ -284,11 +300,20 @@
                goto out_free_irq;
        }

+#if 0
        ret = adp5520_add_subdevs(chip, pdata);

        if (!ret)
                return ret;
+#endif

+       ret = mfd_add_devices(&chip->dev, id->driver_data,
+                       adp5520_cells, ARRAY_SIZE(adp5520_cells),
+                       NULL, client->irq);
+
+       if (!ret)
+               return ret;
+
 out_free_irq:
        if (chip->irq)
                free_irq(chip->irq, chip);
@@ -337,7 +362,8 @@
 #endif

 static const struct i2c_device_id adp5520_id[] = {
-       { "pmic-adp5520", 0 },
+       { "adp5520", ID_ADP5520 },
+       { "adp5501", ID_ADP5501 },
        { }
 };
 MODULE_DEVICE_TABLE(i2c, adp5520_id); 


However this would just work for exactly one ADP5520 in a system.
A way out could be to append the I2C salve address to the cell .name?

Comments appreciated.

>
>Also, please note that you could use the mfd-core API for adding
devices, but
>that's just optional.
>
>Cheers,
>Samuel.
>
>--
>Intel Open Source Technology Centre
>http://oss.intel.com/

Best regards,
Michael

Re: [PATCH v2] mfd: ADP5520 Multifunction LCD Backlight and Keypad Input Device Driver

[Samuel Ortiz]

Hi Michael,

On Fri, Oct 02, 2009 at 10:38:16AM +0100, Hennerich, Michael wrote:
> >> +static int __devinit adp5520_add_subdevs(struct adp5520_chip *chip,
> >> +					struct adp5520_platform_data
> *pdata)
> >> +{
> >> +	struct adp5520_subdev_info *subdev;
> >> +	struct platform_device *pdev;
> >> +	int i, ret = 0;
> >> +
> >> +	for (i = 0; i < pdata->num_subdevs; i++) {
> >> +		subdev = &pdata->subdevs[i];
> >> +
> >> +		pdev = platform_device_alloc(subdev->name, subdev->id);
> >> +
> >> +		pdev->dev.parent = chip->dev;
> >> +		pdev->dev.platform_data = subdev->platform_data;
> >> +
> >> +		ret = platform_device_add(pdev);
> >> +		if (ret)
> >> +			goto failed;
> >> +	}
> >> +	return 0;
> >Here I would expect the MFD core driver to know about all the potential
> >subdevices and add them in that routine, and not take the subdevices
> list from
> >the platform definition.
> >I realize da903x has the same issue btw.
> 
> The ADP5520 is an I2C device and gets registered via struct
> i2c_board_info.
> How about having multiple ADP5520 in a system with different I2C salve
> addresses?
> Each ADP5520 having different Keypad, Backlight and GPIO configurations
> passed in platform_data?
> How will they map? The MFD core is struct resource centric, which is not
> going to help here.
With 2 of those devices, your board file would look like that:

static struct i2c_board_info __initdata board_i2c_board_info[] = {
	{	/* APD5520 #1 */
		I2C_BOARD_INFO("pmic-adp5520", SLAVE_ADDR_1),
		.platform_data	= &apd5520_platform_1,
	},
	{	/* APD5520 #2 */
		I2C_BOARD_INFO("pmic-adp5520", SLAVE_ADDR_2),
		.platform_data	= &apd5520_platform_2,
	},
};

and your platform data would be an aggregation of all subdevices platform data
structures:

struct adp5520_platform_data {
	struct adp5520_leds_platfrom_data *leds;
	struct adp5520_keys_platfrom_data *keyp;
	struct adp5520_gpio_platfrom_data *gpio;
}

Then, your mfd/adp5520.c will have that piece of code:

static struct platform_device adp5520_gpio_device = {
	.name	= "adp5520-gpio",
	.id	= -1,
}

static struct platform_device *adp5520_subdevs[] = {
	&adp5520_gpio_device,
	&adp5520_leds_device,
	&adp5520_keyp_device,
}

Finally, your i2c probe routine would assign platform_data pointers to te
right devices:

static int __devinit adp5520_probe(struct i2c_client *client,
					const struct i2c_device_id *id)
{
	struct adp5520_platform_data *pdata = client->dev.platform_data; 
[...]
	for (i = 0; i < ARRAY_SIZE(adp5520_subdevs); i++) {
		adp5520_subdevs[i]->dev.parent = &client->dev;
		adp5520_assign_pdata(&adp5520_subdevs[i], pdata);
	}

	platform_add_devices(adp5520_subdevs,
			ARRAY_SIZE(adp5520_subdevs)); 
}

Where adp5520_assign_pdata() is a routine setting the platform_device's
platform_data pointer according e.g. to the platform_device name field.

Would that make sense to you ?

Cheers,
Samuel.

> I could be doing something like this:
>                            
> Index: drivers/mfd/adp5520.c
> 
> ===================================================================
> 
> --- drivers/mfd/adp5520.c       (revision 7535)
> 
> +++ drivers/mfd/adp5520.c       (working copy)
> 
> @@ -25,6 +25,7 @@
> 
>  #include <linux/irq.h>
> 
>  #include <linux/workqueue.h>
> 
>  #include <linux/i2c.h>
> 
> +#include <linux/mfd/core.h>
> 
>  
> 
>  #include <linux/mfd/adp5520.h>
> 
>  
> 
> @@ -235,6 +236,21 @@
> 
>         return ret;
> 
>  }
> 
>  
> 
> +static struct mfd_cell __devinitdata adp5520_cells[] = {
> 
> +       {
> 
> +               .name = "adp5520-backlight",
> 
> +       },
> 
> +       {
> 
> +               .name = "adp5520-led",
> 
> +       },
> 
> +       {
> +               .name = "adp5520-gpio",
> +       },
> +       {
> +               .name = "adp5520-keys",
> +       },
> +};
> +
>  static int __devinit adp5520_probe(struct i2c_client *client,
>                                         const struct i2c_device_id *id)
>  {
> @@ -284,11 +300,20 @@
>                 goto out_free_irq;
>         }
> 
> +#if 0
>         ret = adp5520_add_subdevs(chip, pdata);
> 
>         if (!ret)
>                 return ret;
> +#endif
> 
> +       ret = mfd_add_devices(&chip->dev, id->driver_data,
> +                       adp5520_cells, ARRAY_SIZE(adp5520_cells),
> +                       NULL, client->irq);
> +
> +       if (!ret)
> +               return ret;
> +
>  out_free_irq:
>         if (chip->irq)
>                 free_irq(chip->irq, chip);
> @@ -337,7 +362,8 @@
>  #endif
> 
>  static const struct i2c_device_id adp5520_id[] = {
> -       { "pmic-adp5520", 0 },
> +       { "adp5520", ID_ADP5520 },
> +       { "adp5501", ID_ADP5501 },
>         { }
>  };
>  MODULE_DEVICE_TABLE(i2c, adp5520_id); 
> 
> 
> However this would just work for exactly one ADP5520 in a system.
> A way out could be to append the I2C salve address to the cell .name?
> 
> Comments appreciated.
> 
> >
> >Also, please note that you could use the mfd-core API for adding
> devices, but
> >that's just optional.
> >
> >Cheers,
> >Samuel.
> >
> >--
> >Intel Open Source Technology Centre
> >http://oss.intel.com/
> 
> Best regards,
> Michael

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

RE: [Uclinux-dist-devel] [PATCH v2] mfd: ADP5520 Multifunction LCDBacklight and Keypad Input Device Driver

[Hennerich, Michael]

>From: uclinux-dist-devel-bounces@blackfin.uclinux.org
[mailto:uclinux-dist-devel-
>bounces@blackfin.uclinux.org] On Behalf Of Hennerich, Michael
>Device Driver
>
>Hi Samuel,
>
>>From: Samuel Ortiz [mailto:sameo@linux.intel.com]
>>Hi Mike,
>>
>>Some comments on this patch:
>>
>>On Wed, Sep 23, 2009 at 01:11:04AM -0400, Mike Frysinger wrote:
>>> +static irqreturn_t adp5520_irq_handler(int irq, void *data)
>>> +{
>>> +	struct adp5520_chip *chip = data;
>>> +
>>> +	disable_irq_nosync(irq);
>>> +	schedule_work(&chip->irq_work);
>>Have you considered using a threaded irq handler here ?
>
>The Linux version I developed this driver on didn't feature threaded
irq
>handlers.
>But thanks I'm going to take a look here.

Hi Samuel,

Well the threaded irq handlers are no option here, since we use a Level
Sensitive Interrupt. 
The work queue here is to schedule the main irq handler outside hardirq
context.
I2C can't we invoked form none sleepy context, so we can't clear the
interrupt. 
This will cause that we execute the hardirq over and over again,
preventing the irq thread to be run.

The threaded irqs with its current implementation also doesn't allow me
to disable the irq in the hardirq handler.

There have been some discussions about this on lkml recently.
Until there is a way to workaround this issue (handle_level_oneshot_irq,
etc.),
I like to stick with: 

>>> +	disable_irq_nosync(irq);
>>> +	schedule_work(&chip->irq_work);

Best regards,
Michael

Re: [Uclinux-dist-devel] [PATCH v2] mfd: ADP5520 Multifunction LCDBacklight and Keypad Input Device Driver

[Samuel Ortiz]

Hi Michael,

On Fri, Oct 02, 2009 at 02:48:27PM +0100, Hennerich, Michael wrote:
> >From: uclinux-dist-devel-bounces@blackfin.uclinux.org
> [mailto:uclinux-dist-devel-
> >bounces@blackfin.uclinux.org] On Behalf Of Hennerich, Michael
> >Device Driver
> >
> >Hi Samuel,
> >
> >>From: Samuel Ortiz [mailto:sameo@linux.intel.com]
> >>Hi Mike,
> >>
> >>Some comments on this patch:
> >>
> >>On Wed, Sep 23, 2009 at 01:11:04AM -0400, Mike Frysinger wrote:
> >>> +static irqreturn_t adp5520_irq_handler(int irq, void *data)
> >>> +{
> >>> +	struct adp5520_chip *chip = data;
> >>> +
> >>> +	disable_irq_nosync(irq);
> >>> +	schedule_work(&chip->irq_work);
> >>Have you considered using a threaded irq handler here ?
> >
> >The Linux version I developed this driver on didn't feature threaded
> irq
> >handlers.
> >But thanks I'm going to take a look here.
> 
> Hi Samuel,
> 
> Well the threaded irq handlers are no option here, since we use a Level
> Sensitive Interrupt. 
> The work queue here is to schedule the main irq handler outside hardirq
> context.
> I2C can't we invoked form none sleepy context, so we can't clear the
> interrupt. 
> This will cause that we execute the hardirq over and over again,
> preventing the irq thread to be run.
> 
> The threaded irqs with its current implementation also doesn't allow me
> to disable the irq in the hardirq handler.
Ok, that's the piece I missed. Thanks for the clarification.


> There have been some discussions about this on lkml recently.
> Until there is a way to workaround this issue (handle_level_oneshot_irq,
> etc.),
> I like to stick with: 
Fine with me, yes.

Cheers,
Samuel.


> >>> +	disable_irq_nosync(irq);
> >>> +	schedule_work(&chip->irq_work);
> 
> Best regards,
> Michael
> 

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

Re: [Uclinux-dist-devel] [PATCH v2] mfd: ADP5520 Multifunction LCDBacklight and Keypad Input Device Driver

[Mark Brown]

On Fri, Oct 02, 2009 at 02:48:27PM +0100, Hennerich, Michael wrote:

> Well the threaded irq handlers are no option here, since we use a Level
> Sensitive Interrupt. 
> The work queue here is to schedule the main irq handler outside hardirq
> context.
> I2C can't we invoked form none sleepy context, so we can't clear the
> interrupt. 
> This will cause that we execute the hardirq over and over again,
> preventing the irq thread to be run.

> The threaded irqs with its current implementation also doesn't allow me
> to disable the irq in the hardirq handler.

This should all work perfectly fine.  If you don't supply a hard IRQ
handler then the genirq infrastructure will disable the IRQ and schedule
the threaded handler, reenabling the IRQ when the threaded handler
finishes.  The threaded handler runs in a non-atomic context so it can
happily access I2C devices.

> There have been some discussions about this on lkml recently.
> Until there is a way to workaround this issue (handle_level_oneshot_irq,
> etc.),
> I like to stick with: 

> >>> +	disable_irq_nosync(irq);
> >>> +	schedule_work(&chip->irq_work);

This is essentially what a threaded IRQ handler does with current
mainline.  There were issues in 2.6.31 but I believe all Thomas' fixes
have been merged now.

RE: [Uclinux-dist-devel] [PATCH v2] mfd: ADP5520 MultifunctionLCDBacklight and Keypad Input Device Driver

[Hennerich, Michael]

>-----Original Message-----
>From: Mark Brown [mailto:broonie@opensource.wolfsonmicro.com]
>Sent: Friday, October 02, 2009 4:27 PM
>To: Hennerich, Michael
>Cc: Samuel Ortiz; Mike Frysinger; tglx@linutronix.de;
uclinux-dist-devel@blackfin.uclinux.org; linux-
>kernel@vger.kernel.org
>Subject: Re: [Uclinux-dist-devel] [PATCH v2] mfd: ADP5520
MultifunctionLCDBacklight and Keypad Input
>Device Driver
>
>On Fri, Oct 02, 2009 at 02:48:27PM +0100, Hennerich, Michael wrote:
>
>> Well the threaded irq handlers are no option here, since we use a
Level
>> Sensitive Interrupt.
>> The work queue here is to schedule the main irq handler outside
hardirq
>> context.
>> I2C can't we invoked form none sleepy context, so we can't clear the
>> interrupt.
>> This will cause that we execute the hardirq over and over again,
>> preventing the irq thread to be run.
>
>> The threaded irqs with its current implementation also doesn't allow
me
>> to disable the irq in the hardirq handler.
>
>This should all work perfectly fine.  If you don't supply a hard IRQ
>handler then the genirq infrastructure will disable the IRQ and
schedule
>the threaded handler, reenabling the IRQ when the threaded handler
>finishes.  The threaded handler runs in a non-atomic context so it can
>happily access I2C devices.

Hi Mark,

I saw your patch: mfd: Convert WM8350 to use request_threaded_irq()

And was wondering how this ever worked.
I'm using: Linux release 2.6.31.1-ADI-2010R1-pre-svn7535, build #10835
Fri Oct 2 14:48:19 CEST 2009

int request_threaded_irq(unsigned int irq, irq_handler_t handler,
			 irq_handler_t thread_fn, unsigned long
irqflags,
			 const char *devname, void *dev_id)
{
--- snip ---

	if (!handler)
		return -EINVAL;

}

So I guess your patch won't work on 2.6.31

>
>> There have been some discussions about this on lkml recently.
>> Until there is a way to workaround this issue
(handle_level_oneshot_irq,
>> etc.),
>> I like to stick with:
>
>> >>> +	disable_irq_nosync(irq);
>> >>> +	schedule_work(&chip->irq_work);
>
>This is essentially what a threaded IRQ handler does with current
>mainline.  There were issues in 2.6.31 but I believe all Thomas' fixes
>have been merged now.

Do you know when they merged?
They are not in latest staple 2.6.31.1.

Best regards,
Michael

Re: [Uclinux-dist-devel] [PATCH v2] mfd: ADP5520 MultifunctionLCDBacklight and Keypad Input Device Driver

[Mark Brown]

On Fri, Oct 02, 2009 at 03:37:15PM +0100, Hennerich, Michael wrote:

> So I guess your patch won't work on 2.6.31

Yes, it doesn't work in 2.6.31 - I waited until the relevant genirq
stuff had hit mainline before I submitted it.

> >This is essentially what a threaded IRQ handler does with current
> >mainline.  There were issues in 2.6.31 but I believe all Thomas' fixes
> >have been merged now.

> Do you know when they merged?
> They are not in latest staple 2.6.31.1.

2.6.32.

RE: [PATCH v2] mfd: ADP5520 Multifunction LCD Backlight and Keypad Input Device Driver

[Hennerich, Michael]

>With 2 of those devices, your board file would look like that:
>
>static struct i2c_board_info __initdata board_i2c_board_info[] = {
>	{	/* APD5520 #1 */
>		I2C_BOARD_INFO("pmic-adp5520", SLAVE_ADDR_1),
>		.platform_data	= &apd5520_platform_1,
>	},
>	{	/* APD5520 #2 */
>		I2C_BOARD_INFO("pmic-adp5520", SLAVE_ADDR_2),
>		.platform_data	= &apd5520_platform_2,
>	},
>};
>
>and your platform data would be an aggregation of all subdevices
platform data
>structures:
>
>struct adp5520_platform_data {
>	struct adp5520_leds_platfrom_data *leds;
>	struct adp5520_keys_platfrom_data *keyp;
>	struct adp5520_gpio_platfrom_data *gpio;
>}
>
>Then, your mfd/adp5520.c will have that piece of code:
>
>static struct platform_device adp5520_gpio_device = {
>	.name	= "adp5520-gpio",
>	.id	= -1,
>}
>
>static struct platform_device *adp5520_subdevs[] = {
>	&adp5520_gpio_device,
>	&adp5520_leds_device,
>	&adp5520_keyp_device,
>}
>
>Finally, your i2c probe routine would assign platform_data pointers to
te
>right devices:
>
>static int __devinit adp5520_probe(struct i2c_client *client,
>					const struct i2c_device_id *id)
>{
>	struct adp5520_platform_data *pdata = client->dev.platform_data;
>[...]
>	for (i = 0; i < ARRAY_SIZE(adp5520_subdevs); i++) {
>		adp5520_subdevs[i]->dev.parent = &client->dev;
>		adp5520_assign_pdata(&adp5520_subdevs[i], pdata);
>	}
>
>	platform_add_devices(adp5520_subdevs,
>			ARRAY_SIZE(adp5520_subdevs));
>}
>
>Where adp5520_assign_pdata() is a routine setting the platform_device's
>platform_data pointer according e.g. to the platform_device name field.
>
>Would that make sense to you ?
>
>Cheers,
>Samuel.

I think that should work.
Thanks,

Michael

RE: [Uclinux-dist-devel] [PATCH v2] mfd: ADP5520MultifunctionLCDBacklight and Keypad Input Device Driver

[Hennerich, Michael]

>From: Mark Brown [mailto:broonie@opensource.wolfsonmicro.com]
>On Fri, Oct 02, 2009 at 03:37:15PM +0100, Hennerich, Michael wrote:
>
>> So I guess your patch won't work on 2.6.31
>
>Yes, it doesn't work in 2.6.31 - I waited until the relevant genirq
>stuff had hit mainline before I submitted it.
>
>> >This is essentially what a threaded IRQ handler does with current
>> >mainline.  There were issues in 2.6.31 but I believe all Thomas'
fixes
>> >have been merged now.
>
>> Do you know when they merged?
>> They are not in latest staple 2.6.31.1.
>
>2.6.32.

Thanks! IRQF_ONESHOT does the trick.

-Michael

[PATCH v3] mfd: ADP5520 Multifunction LCD Backlight and Keypad Input Device Driver

[Mike Frysinger]

From: Michael Hennerich <michael.hennerich@analog.com>

Base driver for Analog Devices ADP5520/ADP5501 MFD PMICs

Subdevs:
LCD Backlight   : drivers/video/backlight/adp5520_bl
LEDs            : drivers/led/leds-adp5520
GPIO            : drivers/gpio/adp5520-gpio (ADP5520 only)
Keys            : drivers/input/keyboard/adp5520-keys (ADP5520 only)

Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
Signed-off-by: Bryan Wu <cooloney@kernel.org>
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
v3
	- switch to threaded irq
	- integrate subdevices via board resources

 drivers/mfd/Kconfig         |   10 ++
 drivers/mfd/Makefile        |    1 +
 drivers/mfd/adp5520.c       |  375 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/adp5520.h |  299 ++++++++++++++++++++++++++++++++++
 4 files changed, 685 insertions(+), 0 deletions(-)
 create mode 100644 drivers/mfd/adp5520.c
 create mode 100644 include/linux/mfd/adp5520.h

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 570be13..34e8595 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -160,6 +160,16 @@ config PMIC_DA903X
 	  individual components like LCD backlight, voltage regulators,
 	  LEDs and battery-charger under the corresponding menus.
 
+config PMIC_ADP5520
+	bool "Analog Devices ADP5520/01 MFD PMIC Core Support"
+	depends on I2C=y
+	help
+	  Say yes here to add support for Analog Devices AD5520 and ADP5501,
+	  Multifunction Power Management IC. This includes
+	  the I2C driver and the core APIs _only_, you have to select
+	  individual components like LCD backlight, LEDs, GPIOs and Kepad
+	  under the corresponding menus.
+
 config MFD_WM8400
 	tristate "Support Wolfson Microelectronics WM8400"
 	select MFD_CORE
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index f3b277b..a42248e 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -50,3 +50,4 @@ obj-$(CONFIG_PCF50633_ADC)	+= pcf50633-adc.o
 obj-$(CONFIG_PCF50633_GPIO)	+= pcf50633-gpio.o
 obj-$(CONFIG_AB3100_CORE)	+= ab3100-core.o
 obj-$(CONFIG_AB3100_OTP)	+= ab3100-otp.o
+obj-$(CONFIG_PMIC_ADP5520)	+= adp5520.o
diff --git a/drivers/mfd/adp5520.c b/drivers/mfd/adp5520.c
new file mode 100644
index 0000000..eea5e5b
--- /dev/null
+++ b/drivers/mfd/adp5520.c
@@ -0,0 +1,375 @@
+/*
+ * Base driver for Analog Devices ADP5520/ADP5501 MFD PMICs
+ * LCD Backlight: drivers/video/backlight/adp5520_bl
+ * LEDs		: drivers/led/leds-adp5520
+ * GPIO		: drivers/gpio/adp5520-gpio (ADP5520 only)
+ * Keys		: drivers/input/keyboard/adp5520-keys (ADP5520 only)
+ *
+ * Copyright 2009 Analog Devices Inc.
+ *
+ * Derived from da903x:
+ * Copyright (C) 2008 Compulab, Ltd.
+ * 	Mike Rapoport <mike@compulab.co.il>
+ *
+ * Copyright (C) 2006-2008 Marvell International Ltd.
+ * 	Eric Miao <eric.miao@marvell.com>
+ *
+ * Licensed under the GPL-2 or later.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/err.h>
+#include <linux/i2c.h>
+
+#include <linux/mfd/adp5520.h>
+
+struct adp5520_chip {
+	struct i2c_client *client;
+	struct device *dev;
+	struct mutex lock;
+	struct blocking_notifier_head notifier_list;
+	int irq;
+	unsigned long id;
+};
+
+static int __adp5520_read(struct i2c_client *client,
+				int reg, uint8_t *val)
+{
+	int ret;
+
+	ret = i2c_smbus_read_byte_data(client, reg);
+	if (ret < 0) {
+		dev_err(&client->dev, "failed reading at 0x%02x\n", reg);
+		return ret;
+	}
+
+	*val = (uint8_t)ret;
+	return 0;
+}
+
+static int __adp5520_write(struct i2c_client *client,
+				 int reg, uint8_t val)
+{
+	int ret;
+
+	ret = i2c_smbus_write_byte_data(client, reg, val);
+	if (ret < 0) {
+		dev_err(&client->dev, "failed writing 0x%02x to 0x%02x\n",
+				val, reg);
+		return ret;
+	}
+	return 0;
+}
+
+int __adp5520_ack_bits(struct i2c_client *client, int reg, uint8_t bit_mask)
+{
+	struct adp5520_chip *chip = i2c_get_clientdata(client);
+	uint8_t reg_val;
+	int ret;
+
+	mutex_lock(&chip->lock);
+
+	ret = __adp5520_read(client, reg, &reg_val);
+
+	if (!ret) {
+		reg_val |= bit_mask;
+		ret = __adp5520_write(client, reg, reg_val);
+	}
+
+	mutex_unlock(&chip->lock);
+	return ret;
+}
+
+int adp5520_write(struct device *dev, int reg, uint8_t val)
+{
+	return __adp5520_write(to_i2c_client(dev), reg, val);
+}
+EXPORT_SYMBOL_GPL(adp5520_write);
+
+int adp5520_read(struct device *dev, int reg, uint8_t *val)
+{
+	return __adp5520_read(to_i2c_client(dev), reg, val);
+}
+EXPORT_SYMBOL_GPL(adp5520_read);
+
+int adp5520_set_bits(struct device *dev, int reg, uint8_t bit_mask)
+{
+	struct adp5520_chip *chip = dev_get_drvdata(dev);
+	uint8_t reg_val;
+	int ret;
+
+	mutex_lock(&chip->lock);
+
+	ret = __adp5520_read(chip->client, reg, &reg_val);
+
+	if (!ret && ((reg_val & bit_mask) == 0)) {
+		reg_val |= bit_mask;
+		ret = __adp5520_write(chip->client, reg, reg_val);
+	}
+
+	mutex_unlock(&chip->lock);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(adp5520_set_bits);
+
+int adp5520_clr_bits(struct device *dev, int reg, uint8_t bit_mask)
+{
+	struct adp5520_chip *chip = dev_get_drvdata(dev);
+	uint8_t reg_val;
+	int ret;
+
+	mutex_lock(&chip->lock);
+
+	ret = __adp5520_read(chip->client, reg, &reg_val);
+
+	if (!ret && (reg_val & bit_mask)) {
+		reg_val &= ~bit_mask;
+		ret = __adp5520_write(chip->client, reg, reg_val);
+	}
+
+	mutex_unlock(&chip->lock);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(adp5520_clr_bits);
+
+int adp5520_register_notifier(struct device *dev, struct notifier_block *nb,
+				unsigned int events)
+{
+	struct adp5520_chip *chip = dev_get_drvdata(dev);
+
+	if (chip->irq) {
+		adp5520_set_bits(chip->dev, INTERRUPT_ENABLE,
+			events & (KP_IEN | KR_IEN | OVP_IEN | CMPR_IEN));
+
+		return blocking_notifier_chain_register(&chip->notifier_list,
+			 nb);
+	}
+
+	return -ENODEV;
+}
+EXPORT_SYMBOL_GPL(adp5520_register_notifier);
+
+int adp5520_unregister_notifier(struct device *dev, struct notifier_block *nb,
+				unsigned int events)
+{
+	struct adp5520_chip *chip = dev_get_drvdata(dev);
+
+	adp5520_clr_bits(chip->dev, INTERRUPT_ENABLE,
+		events & (KP_IEN | KR_IEN | OVP_IEN | CMPR_IEN));
+
+	return blocking_notifier_chain_unregister(&chip->notifier_list, nb);
+}
+EXPORT_SYMBOL_GPL(adp5520_unregister_notifier);
+
+static irqreturn_t adp5520_irq_thread(int irq, void *data)
+{
+	struct adp5520_chip *chip = data;
+	unsigned int events;
+	uint8_t reg_val;
+	int ret;
+
+	ret = __adp5520_read(chip->client, MODE_STATUS, &reg_val);
+	if (ret)
+		goto out;
+
+	events =  reg_val & (OVP_INT | CMPR_INT | GPI_INT | KR_INT | KP_INT);
+
+	blocking_notifier_call_chain(&chip->notifier_list, events, NULL);
+	/* ACK, Sticky bits are W1C */
+	__adp5520_ack_bits(chip->client, MODE_STATUS, events);
+
+out:
+	return IRQ_HANDLED;
+}
+
+static int __remove_subdev(struct device *dev, void *unused)
+{
+	platform_device_unregister(to_platform_device(dev));
+	return 0;
+}
+
+static int adp5520_remove_subdevs(struct adp5520_chip *chip)
+{
+	return device_for_each_child(chip->dev, NULL, __remove_subdev);
+}
+
+static int __devinit adp5520_probe(struct i2c_client *client,
+					const struct i2c_device_id *id)
+{
+	struct adp5520_platform_data *pdata = client->dev.platform_data;
+	struct platform_device *pdev;
+	struct adp5520_chip *chip;
+	int ret;
+
+	if (!i2c_check_functionality(client->adapter,
+					I2C_FUNC_SMBUS_BYTE_DATA)) {
+		dev_err(&client->dev, "SMBUS Word Data not Supported\n");
+		return -EIO;
+	}
+
+	if (pdata == NULL) {
+		dev_err(&client->dev, "missing platform data\n");
+		return -ENODEV;
+	}
+
+	chip = kzalloc(sizeof(*chip), GFP_KERNEL);
+	if (!chip)
+		return -ENOMEM;
+
+	i2c_set_clientdata(client, chip);
+	chip->client = client;
+
+	chip->dev = &client->dev;
+	chip->irq = client->irq;
+	chip->id = id->driver_data;
+	mutex_init(&chip->lock);
+
+	if (chip->irq) {
+		BLOCKING_INIT_NOTIFIER_HEAD(&chip->notifier_list);
+
+		ret = request_threaded_irq(chip->irq, NULL, adp5520_irq_thread,
+				IRQF_TRIGGER_LOW | IRQF_ONESHOT,
+				"adp5520", chip);
+		if (ret) {
+			dev_err(&client->dev, "failed to request irq %d\n",
+					chip->irq);
+			goto out_free_chip;
+		}
+	}
+
+	ret = adp5520_write(chip->dev, MODE_STATUS, nSTNBY);
+	if (ret) {
+		dev_err(&client->dev, "failed to write\n");
+		goto out_free_irq;
+	}
+
+	if (pdata->keys) {
+		pdev = platform_device_register_data(chip->dev, "adp5520-keys",
+				chip->id, pdata->keys, sizeof(*pdata->keys));
+		if (IS_ERR(pdev)) {
+			ret = PTR_ERR(pdev);
+			goto out_remove_subdevs;
+		}
+	}
+
+	if (pdata->gpio) {
+		pdev = platform_device_register_data(chip->dev, "adp5520-gpio",
+				chip->id, pdata->gpio, sizeof(*pdata->gpio));
+		if (IS_ERR(pdev)) {
+			ret = PTR_ERR(pdev);
+			goto out_remove_subdevs;
+		}
+	}
+
+	if (pdata->leds) {
+		pdev = platform_device_register_data(chip->dev, "adp5520-led",
+				chip->id, pdata->leds, sizeof(*pdata->leds));
+		if (IS_ERR(pdev)) {
+			ret = PTR_ERR(pdev);
+			goto out_remove_subdevs;
+		}
+	}
+
+	if (pdata->backlight) {
+		pdev = platform_device_register_data(chip->dev,
+						"adp5520-backlight",
+						chip->id,
+						pdata->backlight,
+						sizeof(*pdata->backlight));
+		if (IS_ERR(pdev)) {
+			ret = PTR_ERR(pdev);
+			goto out_remove_subdevs;
+		}
+	}
+
+	return 0;
+
+out_remove_subdevs:
+	adp5520_remove_subdevs(chip);
+
+out_free_irq:
+	if (chip->irq)
+		free_irq(chip->irq, chip);
+
+out_free_chip:
+	i2c_set_clientdata(client, NULL);
+	kfree(chip);
+
+	return ret;
+}
+
+static int __devexit adp5520_remove(struct i2c_client *client)
+{
+	struct adp5520_chip *chip = dev_get_drvdata(&client->dev);
+
+	if (chip->irq)
+		free_irq(chip->irq, chip);
+
+	adp5520_remove_subdevs(chip);
+	adp5520_write(chip->dev, MODE_STATUS, 0);
+	i2c_set_clientdata(client, NULL);
+	kfree(chip);
+	return 0;
+}
+
+#ifdef CONFIG_PM
+static int adp5520_suspend(struct i2c_client *client,
+				 pm_message_t state)
+{
+	struct adp5520_chip *chip = dev_get_drvdata(&client->dev);
+
+	adp5520_clr_bits(chip->dev, MODE_STATUS, nSTNBY);
+	return 0;
+}
+
+static int adp5520_resume(struct i2c_client *client)
+{
+	struct adp5520_chip *chip = dev_get_drvdata(&client->dev);
+
+	adp5520_set_bits(chip->dev, MODE_STATUS, nSTNBY);
+	return 0;
+}
+#else
+#define adp5520_suspend	NULL
+#define adp5520_resume	NULL
+#endif
+
+static const struct i2c_device_id adp5520_id[] = {
+	{ "pmic-adp5520", ID_ADP5520 },
+	{ "pmic-adp5501", ID_ADP5501 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, adp5520_id);
+
+static struct i2c_driver adp5520_driver = {
+	.driver = {
+		.name	= "adp5520",
+		.owner	= THIS_MODULE,
+	},
+	.probe		= adp5520_probe,
+	.remove		= __devexit_p(adp5520_remove),
+	.suspend	= adp5520_suspend,
+	.resume		= adp5520_resume,
+	.id_table 	= adp5520_id,
+};
+
+static int __init adp5520_init(void)
+{
+	return i2c_add_driver(&adp5520_driver);
+}
+module_init(adp5520_init);
+
+static void __exit adp5520_exit(void)
+{
+	i2c_del_driver(&adp5520_driver);
+}
+module_exit(adp5520_exit);
+
+MODULE_AUTHOR("Michael Hennerich <hennerich@blackfin.uclinux.org>");
+MODULE_DESCRIPTION("ADP5520(01) PMIC-MFD Driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/mfd/adp5520.h b/include/linux/mfd/adp5520.h
new file mode 100644
index 0000000..ed4d9f2
--- /dev/null
+++ b/include/linux/mfd/adp5520.h
@@ -0,0 +1,299 @@
+/*
+ * Definitions and platfrom data for Analog Devices
+ * ADP5520/ADP5501 MFD PMICs (Backlight, LED, GPIO and Keys)
+ *
+ * Copyright 2009 Analog Devices Inc.
+ *
+ * Licensed under the GPL-2 or later.
+ */
+
+
+#ifndef __LINUX_MFD_ADP5520_H
+#define __LINUX_MFD_ADP5520_H
+
+#define ID_ADP5520		5520
+#define ID_ADP5501		5501
+
+/*
+ * ADP5520/ADP5501 Register Map
+ */
+
+#define MODE_STATUS 		0x00
+#define INTERRUPT_ENABLE 	0x01
+#define BL_CONTROL 		0x02
+#define BL_TIME 		0x03
+#define BL_FADE 		0x04
+#define DAYLIGHT_MAX 		0x05
+#define DAYLIGHT_DIM 		0x06
+#define OFFICE_MAX 		0x07
+#define OFFICE_DIM 		0x08
+#define DARK_MAX 		0x09
+#define DARK_DIM 		0x0A
+#define BL_VALUE 		0x0B
+#define ALS_CMPR_CFG 		0x0C
+#define L2_TRIP 		0x0D
+#define L2_HYS 			0x0E
+#define L3_TRIP 		0x0F
+#define L3_HYS 			0x10
+#define LED_CONTROL 		0x11
+#define LED_TIME 		0x12
+#define LED_FADE 		0x13
+#define LED1_CURRENT 		0x14
+#define LED2_CURRENT 		0x15
+#define LED3_CURRENT 		0x16
+
+/*
+ * ADP5520 Register Map
+ */
+
+#define GPIO_CFG_1 		0x17
+#define GPIO_CFG_2 		0x18
+#define GPIO_IN 		0x19
+#define GPIO_OUT 		0x1A
+#define GPIO_INT_EN 		0x1B
+#define GPIO_INT_STAT 		0x1C
+#define GPIO_INT_LVL 		0x1D
+#define GPIO_DEBOUNCE 		0x1E
+#define GPIO_PULLUP 		0x1F
+#define KP_INT_STAT_1 		0x20
+#define KP_INT_STAT_2 		0x21
+#define KR_INT_STAT_1 		0x22
+#define KR_INT_STAT_2 		0x23
+#define KEY_STAT_1 		0x24
+#define KEY_STAT_2 		0x25
+
+/*
+ * MODE_STATUS bits
+ */
+
+#define nSTNBY		(1 << 7)
+#define BL_EN           (1 << 6)
+#define DIM_EN          (1 << 5)
+#define OVP_INT         (1 << 4)
+#define CMPR_INT        (1 << 3)
+#define GPI_INT         (1 << 2)
+#define KR_INT          (1 << 1)
+#define KP_INT          (1 << 0)
+
+/*
+ * INTERRUPT_ENABLE bits
+ */
+
+#define AUTO_LD_EN      (1 << 4)
+#define CMPR_IEN        (1 << 3)
+#define OVP_IEN         (1 << 2)
+#define KR_IEN          (1 << 1)
+#define KP_IEN          (1 << 0)
+
+/*
+ * BL_CONTROL bits
+ */
+
+#define BL_LVL          ((x) << 5)
+#define BL_LAW          ((x) << 4)
+#define BL_AUTO_ADJ     (1 << 3)
+#define OVP_EN          (1 << 2)
+#define FOVR            (1 << 1)
+#define KP_BL_EN        (1 << 0)
+
+/*
+ * ALS_CMPR_CFG bits
+ */
+
+#define L3_OUT		(1 << 3)
+#define L2_OUT		(1 << 2)
+#define L3_EN		(1 << 1)
+
+#define ADP5020_MAX_BRIGHTNESS	0x7F
+
+#define FADE_VAL(in, out)	((0xF & (in)) | ((0xF & (out)) << 4))
+#define BL_CTRL_VAL(law, auto)	(((1 & (auto)) << 3) | ((0x3 & (law)) << 4))
+#define ALS_CMPR_CFG_VAL(filt, l3_en)	(((0x7 & filt) << 5) | l3_en)
+
+/*
+ * LEDs subdevice bits and masks
+ */
+
+#define ADP5520_01_MAXLEDS 3
+
+#define FLAG_LED_MASK 		0x3
+#define FLAG_OFFT_SHIFT 	8
+#define FLAG_OFFT_MASK 		0x3
+
+#define R3_MODE		(1 << 5)
+#define C3_MODE		(1 << 4)
+#define LED_LAW		(1 << 3)
+#define LED3_EN		(1 << 2)
+#define LED2_EN		(1 << 1)
+#define LED1_EN		(1 << 0)
+
+/*
+ * GPIO subdevice bits and masks
+ */
+
+#define ADP5520_MAXGPIOS	8
+
+#define GPIO_C3		(1 << 7)	/* LED2 or GPIO7 aka C3 */
+#define GPIO_C2		(1 << 6)
+#define GPIO_C1		(1 << 5)
+#define GPIO_C0		(1 << 4)
+#define GPIO_R3		(1 << 3)	/* LED3 or GPIO3 aka R3 */
+#define GPIO_R2		(1 << 2)
+#define GPIO_R1		(1 << 1)
+#define GPIO_R0		(1 << 0)
+
+struct adp5520_gpio_platfrom_data {
+	unsigned gpio_start;
+	u8 gpio_en_mask;
+	u8 gpio_pullup_mask;
+};
+
+/*
+ * Keypad subdevice bits and masks
+ */
+
+#define ADP5520_MAXKEYS	16
+
+#define COL_C3 		(1 << 7)	/* LED2 or GPIO7 aka C3 */
+#define COL_C2		(1 << 6)
+#define COL_C1		(1 << 5)
+#define COL_C0		(1 << 4)
+#define ROW_R3		(1 << 3)	/* LED3 or GPIO3 aka R3 */
+#define ROW_R2		(1 << 2)
+#define ROW_R1		(1 << 1)
+#define ROW_R0		(1 << 0)
+
+#define KEY(row, col) (col + row * 4)
+#define ADP5520_KEYMAPSIZE	ADP5520_MAXKEYS
+
+struct adp5520_keys_platfrom_data {
+	int rows_en_mask;		/* Number of rows */
+	int cols_en_mask;		/* Number of columns */
+	const unsigned short *keymap;	/* Pointer to keymap */
+	unsigned short keymapsize;	/* Keymap size */
+	unsigned repeat:1;		/* Enable key repeat */
+};
+
+
+/*
+ * LEDs subdevice platfrom data
+ */
+
+#define FLAG_ID_ADP5520_LED1_ADP5501_LED0 	1	/* ADP5520 PIN ILED */
+#define FLAG_ID_ADP5520_LED2_ADP5501_LED1 	2	/* ADP5520 PIN C3 */
+#define FLAG_ID_ADP5520_LED3_ADP5501_LED2 	3	/* ADP5520 PIN R3 */
+
+#define LED_DIS_BLINK	(0 << FLAG_OFFT_SHIFT)
+#define LED_OFFT_600ms	(1 << FLAG_OFFT_SHIFT)
+#define LED_OFFT_800ms	(2 << FLAG_OFFT_SHIFT)
+#define LED_OFFT_1200ms	(3 << FLAG_OFFT_SHIFT)
+
+#define LED_ONT_200ms	0
+#define LED_ONT_600ms	1
+#define LED_ONT_800ms	2
+#define LED_ONT_1200ms	3
+
+struct adp5520_leds_platfrom_data {
+	int num_leds;
+	struct led_info	*leds;
+	u8 fade_in;		/* Backlight Fade-In Timer */
+	u8 fade_out;		/* Backlight Fade-Out Timer */
+	u8 led_on_time;
+};
+
+/*
+ * Backlight subdevice platfrom data
+ */
+
+#define FADE_T_DIS	0	/* Fade Timer Disabled */
+#define FADE_T_300ms	1	/* 0.3 Sec */
+#define FADE_T_600ms	2
+#define FADE_T_900ms	3
+#define FADE_T_1200ms	4
+#define FADE_T_1500ms	5
+#define FADE_T_1800ms	6
+#define FADE_T_2100ms	7
+#define FADE_T_2400ms	8
+#define FADE_T_2700ms	9
+#define FADE_T_3000ms	10
+#define FADE_T_3500ms	11
+#define FADE_T_4000ms	12
+#define FADE_T_4500ms	13
+#define FADE_T_5000ms	14
+#define FADE_T_5500ms	15	/* 5.5 Sec */
+
+#define BL_LAW_LINEAR 	0
+#define BL_LAW_SQUARE 	1
+#define BL_LAW_CUBIC1 	2
+#define BL_LAW_CUBIC2 	3
+
+#define BL_AMBL_FILT_80ms 	0	/* Light sensor filter time */
+#define BL_AMBL_FILT_160ms 	1
+#define BL_AMBL_FILT_320ms 	2
+#define BL_AMBL_FILT_640ms 	3
+#define BL_AMBL_FILT_1280ms 	4
+#define BL_AMBL_FILT_2560ms 	5
+#define BL_AMBL_FILT_5120ms 	6
+#define BL_AMBL_FILT_10240ms 	7	/* 10.24 sec */
+
+	/*
+	 * Blacklight current 0..30mA
+	 */
+#define BL_CUR_mA(I)		((I * 127) / 30)
+
+	/*
+	 * L2 comparator current 0..1000uA
+	 */
+#define L2_COMP_CURR_uA(I)	((I * 255) / 1000)
+
+	/*
+	 * L3 comparator current 0..127uA
+	 */
+#define L3_COMP_CURR_uA(I)	((I * 255) / 127)
+
+struct adp5520_backlight_platfrom_data {
+	u8 fade_in;		/* Backlight Fade-In Timer */
+	u8 fade_out;		/* Backlight Fade-Out Timer */
+	u8 fade_led_law;	/* fade-on/fade-off transfer characteristic */
+
+	u8 en_ambl_sens;	/* 1 = enable ambient light sensor */
+	u8 abml_filt;		/* Light sensor filter time */
+	u8 l1_daylight_max;	/* use BL_CUR_mA(I) 0 <= I <= 30 mA */
+	u8 l1_daylight_dim;	/* typ = 0, use BL_CUR_mA(I) 0 <= I <= 30 mA */
+	u8 l2_office_max;	/* use BL_CUR_mA(I) 0 <= I <= 30 mA */
+	u8 l2_office_dim;	/* typ = 0, use BL_CUR_mA(I) 0 <= I <= 30 mA */
+	u8 l3_dark_max;		/* use BL_CUR_mA(I) 0 <= I <= 30 mA */
+	u8 l3_dark_dim;		/* typ = 0, use BL_CUR_mA(I) 0 <= I <= 30 mA */
+	u8 l2_trip;		/* use L2_COMP_CURR_uA(I) 0 <= I <= 1000 uA */
+	u8 l2_hyst;		/* use L2_COMP_CURR_uA(I) 0 <= I <= 1000 uA */
+	u8 l3_trip;		/* use L3_COMP_CURR_uA(I) 0 <= I <= 127 uA */
+	u8 l3_hyst;		/* use L3_COMP_CURR_uA(I) 0 <= I <= 127 uA */
+};
+
+/*
+ * MFD chip platfrom data
+ */
+
+struct adp5520_platform_data {
+	struct adp5520_keys_platfrom_data *keys;
+	struct adp5520_gpio_platfrom_data *gpio;
+	struct adp5520_leds_platfrom_data *leds;
+	struct adp5520_backlight_platfrom_data *backlight;
+};
+
+/*
+ * MFD chip functions
+ */
+
+extern int adp5520_read(struct device *dev, int reg, uint8_t *val);
+extern int adp5520_write(struct device *dev, int reg, u8 val);
+extern int adp5520_clr_bits(struct device *dev, int reg, uint8_t bit_mask);
+extern int adp5520_set_bits(struct device *dev, int reg, uint8_t bit_mask);
+
+extern int adp5520_register_notifier(struct device *dev,
+		 struct notifier_block *nb, unsigned int events);
+
+extern int adp5520_unregister_notifier(struct device *dev,
+		struct notifier_block *nb, unsigned int events);
+
+#endif /* __LINUX_MFD_ADP5520_H */
-- 
1.6.5.rc2

Re: [PATCH v3] mfd: ADP5520 Multifunction LCD Backlight and Keypad Input Device Driver

[Mark Brown]

On Tue, Oct 06, 2009 at 03:44:31AM -0400, Mike Frysinger wrote:

> +int adp5520_register_notifier(struct device *dev, struct notifier_block *nb,
> +				unsigned int events)
> +{

This notifier stuff looks an awful lot like an interrupt controller
driver.  Now that it's possible to implement support for an I2C/SPI
driven interrupt controller it'd be good to use that rather than having
a custom API if that's possible.

> +#define GPIO_R3		(1 << 3)	/* LED3 or GPIO3 aka R3 */
> +#define GPIO_R2		(1 << 2)
> +#define GPIO_R1		(1 << 1)
> +#define GPIO_R0		(1 << 0)

> +struct adp5520_gpio_platfrom_data {
> +	unsigned gpio_start;
> +	u8 gpio_en_mask;
> +	u8 gpio_pullup_mask;
> +};

Since there's platform data in here it'd probably be better to namespace
the #defines or put them in a separate header in case there are
collisions with some other chip used on a board.

> +struct adp5520_leds_platfrom_data {

There's some 'platfrom' rather than 'platform' typos.

> +	int num_leds;
> +	struct led_info	*leds;
> +	u8 fade_in;		/* Backlight Fade-In Timer */
> +	u8 fade_out;		/* Backlight Fade-Out Timer */
> +	u8 led_on_time;

I don't know exactly what the on_time option does but if it controls
hardware-implemented blinking there's actually a callback function the
LED driver can implement to allow triggers to use the hardware assisted
blinking at runtime.  If it returns an error or isn't implemented
there's a software fallback.

RE: [PATCH v3] mfd: ADP5520 Multifunction LCD Backlight and KeypadInput Device Driver

[Hennerich, Michael]

Hi Mark,

>From: Mark Brown [mailto:broonie@opensource.wolfsonmicro.com]
>On Tue, Oct 06, 2009 at 03:44:31AM -0400, Mike Frysinger wrote:
>
>> +int adp5520_register_notifier(struct device *dev, struct
notifier_block *nb,
>> +				unsigned int events)
>> +{
>
>This notifier stuff looks an awful lot like an interrupt controller
>driver.  Now that it's possible to implement support for an I2C/SPI
>driven interrupt controller it'd be good to use that rather than having
>a custom API if that's possible.

Honestly this notifier chain is a clean approach and serves its purpose
here pretty well.
IMHO it's much more preferable than pretending there is a virtual GPIO
that doesn't exist and a MFD subdev could request.

>
>> +#define GPIO_R3		(1 << 3)	/* LED3 or GPIO3 aka R3
*/
>> +#define GPIO_R2		(1 << 2)
>> +#define GPIO_R1		(1 << 1)
>> +#define GPIO_R0		(1 << 0)
>
>> +struct adp5520_gpio_platfrom_data {
>> +	unsigned gpio_start;
>> +	u8 gpio_en_mask;
>> +	u8 gpio_pullup_mask;
>> +};
>
>Since there's platform data in here it'd probably be better to
namespace
>the #defines or put them in a separate header in case there are
>collisions with some other chip used on a board.

Will do.


>
>> +struct adp5520_leds_platfrom_data {
>
>There's some 'platfrom' rather than 'platform' typos.

Good catch - this happens to me all the time


>
>> +	int num_leds;
>> +	struct led_info	*leds;
>> +	u8 fade_in;		/* Backlight Fade-In Timer */
>> +	u8 fade_out;		/* Backlight Fade-Out Timer */
>> +	u8 led_on_time;
>
>I don't know exactly what the on_time option does but if it controls
>hardware-implemented blinking there's actually a callback function the
>LED driver can implement to allow triggers to use the hardware assisted
>blinking at runtime.  If it returns an error or isn't implemented
>there's a software fallback.

Yes its hardware controlled blinking. I noticed the leds timer trigger
driver.
People can still use it - the downside with the hardware assisted
blinking is that all LEDs share the same on_time. So I decided against
using the callback.

-Michael 

Re: [PATCH v3] mfd: ADP5520 Multifunction LCD Backlight and KeypadInput Device Driver

[Mark Brown]

On Tue, Oct 06, 2009 at 01:23:52PM +0100, Hennerich, Michael wrote:
> >From: Mark Brown [mailto:broonie@opensource.wolfsonmicro.com]

> >This notifier stuff looks an awful lot like an interrupt controller
> >driver.  Now that it's possible to implement support for an I2C/SPI
> >driven interrupt controller it'd be good to use that rather than having
> >a custom API if that's possible.

> Honestly this notifier chain is a clean approach and serves its purpose
> here pretty well.
> IMHO it's much more preferable than pretending there is a virtual GPIO
> that doesn't exist and a MFD subdev could request.

I'm not sure what the association with virtual gpios is?  This is all
separate to gpiolib except in that it would mean that a gpio driver for
the device would be able to export these interrupts to its clients.

> >> +	u8 led_on_time;

> >I don't know exactly what the on_time option does but if it controls
> >hardware-implemented blinking there's actually a callback function the

> Yes its hardware controlled blinking. I noticed the leds timer trigger
> driver.
> People can still use it - the downside with the hardware assisted
> blinking is that all LEDs share the same on_time. So I decided against
> using the callback.

Ah, if it affects all LEDs then it isn't suitable for the callback at
all.

RE: [PATCH v3] mfd: ADP5520 Multifunction LCD Backlight and KeypadInput Device Driver

[Hennerich, Michael]

>From: Mark Brown [mailto:broonie@opensource.wolfsonmicro.com]
>On Tue, Oct 06, 2009 at 01:23:52PM +0100, Hennerich, Michael wrote:
>> >From: Mark Brown [mailto:broonie@opensource.wolfsonmicro.com]
>
>> >This notifier stuff looks an awful lot like an interrupt controller
>> >driver.  Now that it's possible to implement support for an I2C/SPI
>> >driven interrupt controller it'd be good to use that rather than
having
>> >a custom API if that's possible.
>
>> Honestly this notifier chain is a clean approach and serves its
purpose
>> here pretty well.
>> IMHO it's much more preferable than pretending there is a virtual
GPIO
>> that doesn't exist and a MFD subdev could request.
>
>I'm not sure what the association with virtual gpios is?  This is all
>separate to gpiolib except in that it would mean that a gpio driver for
>the device would be able to export these interrupts to its clients.

This is what I meant.
So you propose having the MFD Core as well as its subdevs requesting the
ADP5520 IRQ (client->irq) IRQF_SHARED? I think we already excluded us
from using this option when we were asked to move to the NEW threaded
irqs?


-Michael

Re: [PATCH v3] mfd: ADP5520 Multifunction LCD Backlight and KeypadInput Device Driver

[Mark Brown]

On Tue, Oct 06, 2009 at 01:55:45PM +0100, Hennerich, Michael wrote:
> >From: Mark Brown [mailto:broonie@opensource.wolfsonmicro.com]

> >I'm not sure what the association with virtual gpios is?  This is all
> >separate to gpiolib except in that it would mean that a gpio driver for
> >the device would be able to export these interrupts to its clients.

> This is what I meant.
> So you propose having the MFD Core as well as its subdevs requesting the
> ADP5520 IRQ (client->irq) IRQF_SHARED? I think we already excluded us
> from using this option when we were asked to move to the NEW threaded
> irqs?

No, I'm suggesting implementing an IRQ controller driver for the device
- register an irq_chip for the interrupt controller on it.  Support for
doing this on I2C devices was added at pretty much the same time as the
IRQ_ONESHOT support.  This gives access to all the genirq infrastrure
features rather than having to implement a custom IRQ handling stack.

Like I say, I'm not sure what you meant when you were talking about
virtual gpios.

RE: [PATCH v3] mfd: ADP5520 Multifunction LCD Backlight and KeypadInput Device Driver

[Hennerich, Michael]

>From: Mark Brown [mailto:broonie@opensource.wolfsonmicro.com]
>On Tue, Oct 06, 2009 at 01:55:45PM +0100, Hennerich, Michael wrote:
>> >From: Mark Brown [mailto:broonie@opensource.wolfsonmicro.com]
>
>> >I'm not sure what the association with virtual gpios is?  This is
all
>> >separate to gpiolib except in that it would mean that a gpio driver
for
>> >the device would be able to export these interrupts to its clients.
>
>> This is what I meant.
>> So you propose having the MFD Core as well as its subdevs requesting
the
>> ADP5520 IRQ (client->irq) IRQF_SHARED? I think we already excluded us
>> from using this option when we were asked to move to the NEW threaded
>> irqs?
>
>No, I'm suggesting implementing an IRQ controller driver for the device
>- register an irq_chip for the interrupt controller on it.  Support for
>doing this on I2C devices was added at pretty much the same time as the
>IRQ_ONESHOT support.  This gives access to all the genirq infrastrure
>features rather than having to implement a custom IRQ handling stack.
>
>Like I say, I'm not sure what you meant when you were talking about
>virtual gpios.

This is not an interrupt controller.
The only ADP5520 subdev that needs to be notified is the adp5520-keys
input driver, if present.
Sounds like overshoot, registering a irq_chip using
set_irq_chip_and_handler() and friends, for exactly one dedicated and
known consumer.

-Michael

Re: [PATCH v3] mfd: ADP5520 Multifunction LCD Backlight and KeypadInput Device Driver

[Mark Brown]

On Tue, Oct 06, 2009 at 03:32:56PM +0100, Hennerich, Michael wrote:

> This is not an interrupt controller.
> The only ADP5520 subdev that needs to be notified is the adp5520-keys
> input driver, if present.
> Sounds like overshoot, registering a irq_chip using
> set_irq_chip_and_handler() and friends, for exactly one dedicated and
> known consumer.

According to the datasheet the GPIOs, light sensor and regulator can
also generate interrupts?  The GPIOs in particular would benefit from
this since this would mean that their interrupts would be usable by
generic gpiolib based drivers.  

RE: [PATCH v3] mfd: ADP5520 Multifunction LCD Backlight and KeypadInput Device Driver

[Hennerich, Michael]

>From: Mark Brown [mailto:broonie@opensource.wolfsonmicro.com]
>On Tue, Oct 06, 2009 at 03:32:56PM +0100, Hennerich, Michael wrote:
>
>> This is not an interrupt controller.
>> The only ADP5520 subdev that needs to be notified is the adp5520-keys
>> input driver, if present.
>> Sounds like overshoot, registering a irq_chip using
>> set_irq_chip_and_handler() and friends, for exactly one dedicated and
>> known consumer.
>
>According to the datasheet the GPIOs, light sensor and regulator can
>also generate interrupts?

Right - I know - but none of the subdevs are currently using this
functionality.

>The GPIOs in particular would benefit from
>this since this would mean that their interrupts would be usable by
>generic gpiolib based drivers.

Right - and I didn't say that I'm not going to add this functionality
later.
All the ADP5520 subdevs already merged in 2.6.32 - the MFD core is the
only remaining part.
Time is running short and major changes are going to require another
round of full testing.

-Michael

Re: [PATCH v3] mfd: ADP5520 Multifunction LCD Backlight and KeypadInput Device Driver

[Mark Brown]

On Tue, Oct 06, 2009 at 04:05:45PM +0100, Hennerich, Michael wrote:
> >From: Mark Brown [mailto:broonie@opensource.wolfsonmicro.com]

> >According to the datasheet the GPIOs, light sensor and regulator can
> >also generate interrupts?

> Right - I know - but none of the subdevs are currently using this
> functionality.

You weren't very clear on the difference between the current state of
the drivers and the capabilities of the chip there.

> >The GPIOs in particular would benefit from
> >this since this would mean that their interrupts would be usable by
> >generic gpiolib based drivers.

> Right - and I didn't say that I'm not going to add this functionality
> later.
> All the ADP5520 subdevs already merged in 2.6.32 - the MFD core is the
> only remaining part.
> Time is running short and major changes are going to require another
> round of full testing.

Oh, well, that's sad.  It would have been worth at least considering 
doing something that at least looks like the IRQ infrastructure but now
there's going to be a merge issue with the input tree if anything gets
changed now.

RE: [PATCH v3] mfd: ADP5520 Multifunction LCD Backlight and KeypadInput Device Driver

[Hennerich, Michael]

>From: Mark Brown [mailto:broonie@opensource.wolfsonmicro.com]
>On Tue, Oct 06, 2009 at 04:05:45PM +0100, Hennerich, Michael wrote:
>> >From: Mark Brown [mailto:broonie@opensource.wolfsonmicro.com]
>
>> >According to the datasheet the GPIOs, light sensor and regulator can
>> >also generate interrupts?
>
>> Right - I know - but none of the subdevs are currently using this
>> functionality.
>
>You weren't very clear on the difference between the current state of
>the drivers and the capabilities of the chip there.

As you said there are chip internal interrupt sources for I/Os, keypad
presses and releases, ambient light sensor comparator states, and
overvoltage conditions.
The current state of the driver uses only interrupts for the keypad.
I think you agree that its common practice to only implement
functionality for chip features that are typically used.
There are exactly 8 GP signals which are muxed with Keypad and GPIO. In
case you use a 4x4 Keypad there is no GPIO left.
In case you use a 3x4 Keypad there is exactly 1 GPIO that can be exposed
to the gpiolib.

In one of your earlier posts you mentioned: "register an irq_chip for
the interrupt controller on it.  Support for doing this on I2C devices
was added at pretty much the same time as the IRQ_ONESHOT support."

Can you point me to what exactly was added to support this on I2C/SPI
devices? 

-Michael

Re: [PATCH v3] mfd: ADP5520 Multifunction LCD Backlight and KeypadInput Device Driver

[Mark Brown]

On Wed, Oct 07, 2009 at 09:50:43AM +0100, Hennerich, Michael wrote:

> As you said there are chip internal interrupt sources for I/Os, keypad
> presses and releases, ambient light sensor comparator states, and
> overvoltage conditions.
> The current state of the driver uses only interrupts for the keypad.
> I think you agree that its common practice to only implement
> functionality for chip features that are typically used.

Yes, but equally well it's good to avoid future issues as the driver is
enhanced.  To be honest the main thing that made me notice when reading
the patch was the use of the blocking notifier pattern to implement the
interrupt infrastructure, mostly since it doesn't look like an IRQ
driver and the naming hides the fat that that's what it is.

I think I forgot to mention it previously but there's some work on
getting a standard ALS interface in the kernel too.  I'd really expect
the GPIOs to end up being used as GPIOs in some designs as well.

> In one of your earlier posts you mentioned: "register an irq_chip for
> the interrupt controller on it.  Support for doing this on I2C devices
> was added at pretty much the same time as the IRQ_ONESHOT support."

> Can you point me to what exactly was added to support this on I2C/SPI
> devices? 

These commits (which were merged during the merge window):

4dbc9ca genirq: Do not mask oneshot edge type interrupts
399b5da genirq: Support nested threaded irq handling
70aedd2 genirq: Add buslock support

There may be some other fixup patches afterwards too, but the buslock
and nested threaded handers are the key ones.

RE: [PATCH v3] mfd: ADP5520 Multifunction LCD Backlight and KeypadInput Device Driver

[Hennerich, Michael]

>From: Mark Brown [mailto:broonie@opensource.wolfsonmicro.com]
>Subject: Re: [PATCH v3] mfd: ADP5520 Multifunction LCD Backlight and
KeypadInput Device Driver
>
>On Wed, Oct 07, 2009 at 09:50:43AM +0100, Hennerich, Michael wrote:
>
>> As you said there are chip internal interrupt sources for I/Os,
keypad
>> presses and releases, ambient light sensor comparator states, and
>> overvoltage conditions.
>> The current state of the driver uses only interrupts for the keypad.
>> I think you agree that its common practice to only implement
>> functionality for chip features that are typically used.
>
>Yes, but equally well it's good to avoid future issues as the driver is
>enhanced.  To be honest the main thing that made me notice when reading
>the patch was the use of the blocking notifier pattern to implement the
>interrupt infrastructure, mostly since it doesn't look like an IRQ
>driver and the naming hides the fat that that's what it is.

In case I would have done the whole ADP5520 in a single file exposing
functionality to the input, backlight, led and gpio infrastructure - I
probably wouldn't find a subtree maintainer that is likely to merge this
blob.
But apart from the GPIO interrupt capabilities - I wouldn't need doing
an interrupt controller.
I think you agree.

So this notifier chain seemed like a good approach to notifiy the
input/keypad/adp5520-keys about some work. BTW - this approach is used
by other drivers for exactly the same reason too.

Honestly - I'm not yet convinced that this new irq stuff really works in
combination with my ADP5520 Low Level IRQ. 
My chained_handler (for demux) as well as irq_desc .mask .unmask .ack
and .set_type need to also be allowed to invoke sleeping i2c
transfers!!!?

If I understood you right:

Instead of request_threaded_irq() in my MFD core:

I should do following: (unfortunately this is all on the bleeding edge
of technology, with no example driver actually using this craft)


static struct irq_chip adp5520_irqchip = {
	.name		= "ADP5520",
	.mask		= adp5520_irq_mask, /* MAYSLEEP */
	.unmask	= adp5520_irq_unmask, /* MAYSLEEP */
	.set_type	= adp5520_irq_type, /* MAYSLEEP */
};

static void adp5520_irq_handler(unsigned irq, struct irq_desc *desc)
{

	/* MAYSLEEP */

}

--snip--

	set_irq_data(client->irq, chip);
	set_irq_chained_handler(client->irq, adp5520_irq_handler);

Or instead of using the set_irq_chained_handler 
- I should use request_threaded_irq() and do whatever I wanted to do in
adp5520_irq_handler(unsigned        irq, struct irq_desc *desc), in my
irq_thread???

For the additional interrupts exposed by the ADP5520 MFD Core Interrupt
Controller I do this:


	set_irq_chip_and_handler_name(chip->irq_base +
ADP5520_KEYPAD_IRQ, &adp5520_irqchip,
					handle_nested_irq,
"adp5520-demux");
	set_irq_chip_data(chip->irq_base + ADP5520_KEYPAD_IRQ, chip);

	set_irq_nested_thread(chip->irq_base + ADP5520_KEYPAD_IRQ, 1);

	for (i = ADP5520_KEYPAD_IRQ; i < ngpio; i++) {
		set_irq_chip_and_handler_name(i + chip->irq_base,
&adp5520_irqchip,
					handle_nested_irq,
"adp5520-demux");
		set_irq_chip_data(i + chip->irq_base, chip);
		set_irq_nested_thread(chip->irq_base + i, 1);
	}




>
>I think I forgot to mention it previously but there's some work on
>getting a standard ALS interface in the kernel too.  I'd really expect
>the GPIOs to end up being used as GPIOs in some designs as well.

This is really interesting. Do you know where this discussion currently
takes place, and who is taking the lead (came up with a proposal)?

-Michael

>
>> In one of your earlier posts you mentioned: "register an irq_chip for
>> the interrupt controller on it.  Support for doing this on I2C
devices
>> was added at pretty much the same time as the IRQ_ONESHOT support."
>
>> Can you point me to what exactly was added to support this on I2C/SPI
>> devices?
>
>These commits (which were merged during the merge window):
>
>4dbc9ca genirq: Do not mask oneshot edge type interrupts
>399b5da genirq: Support nested threaded irq handling
>70aedd2 genirq: Add buslock support
>
>There may be some other fixup patches afterwards too, but the buslock
>and nested threaded handers are the key ones.

RE: [PATCH v3] mfd: ADP5520 Multifunction LCD Backlight and KeypadInput Device Driver

[Hennerich, Michael]

>From: Hennerich, Michael
>>From: Mark Brown [mailto:broonie@opensource.wolfsonmicro.com]
>>Subject: Re: [PATCH v3] mfd: ADP5520 Multifunction LCD Backlight and
KeypadInput Device Driver
>>
>>On Wed, Oct 07, 2009 at 09:50:43AM +0100, Hennerich, Michael wrote:
>>
>>> As you said there are chip internal interrupt sources for I/Os,
keypad
>>> presses and releases, ambient light sensor comparator states, and
>>> overvoltage conditions.
>>> The current state of the driver uses only interrupts for the keypad.
>>> I think you agree that its common practice to only implement
>>> functionality for chip features that are typically used.
>>
>>Yes, but equally well it's good to avoid future issues as the driver
is
>>enhanced.  To be honest the main thing that made me notice when
reading
>>the patch was the use of the blocking notifier pattern to implement
the
>>interrupt infrastructure, mostly since it doesn't look like an IRQ
>>driver and the naming hides the fat that that's what it is.
>
>In case I would have done the whole ADP5520 in a single file exposing
functionality to the input,
>backlight, led and gpio infrastructure - I probably wouldn't find a
subtree maintainer that is likely
>to merge this blob.
>But apart from the GPIO interrupt capabilities - I wouldn't need doing
an interrupt controller.
>I think you agree.
>
>So this notifier chain seemed like a good approach to notifiy the
input/keypad/adp5520-keys about
>some work. BTW - this approach is used by other drivers for exactly the
same reason too.
>
>Honestly - I'm not yet convinced that this new irq stuff really works
in combination with my ADP5520
>Low Level IRQ.
>My chained_handler (for demux) as well as irq_desc .mask .unmask .ack
and .set_type need to also be
>allowed to invoke sleeping i2c transfers!!!?
>
>If I understood you right:
>
>Instead of request_threaded_irq() in my MFD core:
>
>I should do following: (unfortunately this is all on the bleeding edge
of technology, with no example
>driver actually using this craft)
>
>
>static struct irq_chip adp5520_irqchip = {
>	.name		= "ADP5520",
>	.mask		= adp5520_irq_mask, /* MAYSLEEP */
>	.unmask	= adp5520_irq_unmask, /* MAYSLEEP */
>	.set_type	= adp5520_irq_type, /* MAYSLEEP */
>};
>
>static void adp5520_irq_handler(unsigned irq, struct irq_desc *desc)
>{
>
>	/* MAYSLEEP */
>
>}
>
>--snip--
>
>	set_irq_data(client->irq, chip);
>	set_irq_chained_handler(client->irq, adp5520_irq_handler);
>
>Or instead of using the set_irq_chained_handler
>- I should use request_threaded_irq() and do whatever I wanted to do in
adp5520_irq_handler(unsigned
>irq, struct irq_desc *desc), in my irq_thread???
>
>For the additional interrupts exposed by the ADP5520 MFD Core Interrupt
Controller I do this:
>
>
>	set_irq_chip_and_handler_name(chip->irq_base +
ADP5520_KEYPAD_IRQ, &adp5520_irqchip,
>					handle_nested_irq,
"adp5520-demux");
>	set_irq_chip_data(chip->irq_base + ADP5520_KEYPAD_IRQ, chip);
>
>	set_irq_nested_thread(chip->irq_base + ADP5520_KEYPAD_IRQ, 1);
>
>	for (i = ADP5520_KEYPAD_IRQ; i < ngpio; i++) {
>		set_irq_chip_and_handler_name(i + chip->irq_base,
&adp5520_irqchip,
>					handle_nested_irq,
"adp5520-demux");
>		set_irq_chip_data(i + chip->irq_base, chip);
>		set_irq_nested_thread(chip->irq_base + i, 1);
>	}
>
>

Mark,

Most architectures define NR_IRQS to exactly the number of on-chip IRQs.
Therefore irq_desc pointers > NR_IRQS will fail on most architectures.
This driver should be usable out of the box - but I guess this may
prevent this.

-Michael

Re: [PATCH v3] mfd: ADP5520 Multifunction LCD Backlight and KeypadInput Device Driver

[Mark Brown]

On Wed, Oct 07, 2009 at 01:11:40PM +0100, Hennerich, Michael wrote:

> In case I would have done the whole ADP5520 in a single file exposing
> functionality to the input, backlight, led and gpio infrastructure - I
> probably wouldn't find a subtree maintainer that is likely to merge this
> blob.

Why do you say this?

> But apart from the GPIO interrupt capabilities - I wouldn't need doing
> an interrupt controller.
> I think you agree.

I'm not sure I do, TBH - if there's more than one

> So this notifier chain seemed like a good approach to notifiy the
> input/keypad/adp5520-keys about some work. BTW - this approach is used
> by other drivers for exactly the same reason too.

The only ones using the specific approach are da903x and ab3100, both of
which predate the availability of the genirq improvements.

> Honestly - I'm not yet convinced that this new irq stuff really works in
> combination with my ADP5520 Low Level IRQ. 
> My chained_handler (for demux) as well as irq_desc .mask .unmask .ack
> and .set_type need to also be allowed to invoke sleeping i2c
> transfers!!!?

You don't need to use chained_handler explicitly.  You can just use a
regular threaded IRQ handler for the primary IRQ, register an IRQ chip
for the IRQs it provides then call handle_nested_irq() from within the
primary IRQ handler.  The chaining is only needed if running in hard
IRQ context.

The bus_lock stuff is there so that mask, unmask and ack don't need to
do I2C interactions.  The idea is that you update local variables in
those and then when sync_unlock() is called you write out all the
changes to the device.  The framework is set up to cope with this.

> I should do following: (unfortunately this is all on the bleeding edge
> of technology, with no example driver actually using this craft)

I have a patch for wm831x which does the switchover to the new model - I
hope to be able to publish it very soon, I did some blind changes that I
need to test.  I'll include you in the CCs when I post it.

> >I think I forgot to mention it previously but there's some work on >
>getting a standard ALS interface in the kernel too.  I'd really expect
> >the GPIOs to end up being used as GPIOs in some designs as well.

> This is really interesting. Do you know where this discussion
currently > takes place, and who is taking the lead (came up with a
proposal)?

Not precisely, though there's an active thread 'New home for DS1682
driver' on the I2C list with some mutterings about it - it should at
least give some pointers for further archive trawling.

Re: [PATCH v3] mfd: ADP5520 Multifunction LCD Backlight and KeypadInput Device Driver

[Mark Brown]

On Wed, Oct 07, 2009 at 02:01:42PM +0100, Hennerich, Michael wrote:

> Most architectures define NR_IRQS to exactly the number of on-chip IRQs.
> Therefore irq_desc pointers > NR_IRQS will fail on most architectures.
> This driver should be usable out of the box - but I guess this may
> prevent this.

Indeed, that's rather unfortunate.  This wouldn't be the only driver
with that dependency, though, so I'd expect to see the interesting
architectures getting fixed.

RE: [PATCH v3] mfd: ADP5520 Multifunction LCD Backlight and KeypadInput Device Driver

[Hennerich, Michael]

>From: Mark Brown [mailto:broonie@opensource.wolfsonmicro.com]
>
>On Wed, Oct 07, 2009 at 02:01:42PM +0100, Hennerich, Michael wrote:
>
>> Most architectures define NR_IRQS to exactly the number of on-chip
IRQs.
>> Therefore irq_desc pointers > NR_IRQS will fail on most
architectures.
>> This driver should be usable out of the box - but I guess this may
>> prevent this.
>
>Indeed, that's rather unfortunate.  This wouldn't be the only driver
>with that dependency, though, so I'd expect to see the interesting
>architectures getting fixed.

We have an update patch that fixes the platform typo and namespace
pollution in the header file.
I think we send out this patch as final for mainline inclusion.
For the interrupt controller feature - I think I can wait until the
interesting architectures get fixed.

Thanks for your feedback and help
best regards,
Michael