linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/3] mc13783: add pwr button support
@ 2011-07-09 23:06 Philippe Rétornaz
  2011-07-09 23:06 ` [PATCH 1/3] mc13783: add power " Philippe Rétornaz
  0 siblings, 1 reply; 8+ messages in thread
From: Philippe Rétornaz @ 2011-07-09 23:06 UTC (permalink / raw)
  To: s.hauer
  Cc: linux-arm-kernel, dmitry.torokhov, sameo, linux-input, broonie,
	u.kleine-koenig, philippe.retornaz

Hello

This add button handling for the MC13783 PMIC.
The first patch modify the mc13783 MFD driver to add a button
subdevice and also adds the mc13783-pwrbutton driver using a misc
input device.

The second patch adds the power on button support to the mx31moboard boards.

The third patch is optional and modify the mc13xxx driver to use the platform 
data to check if the leds and buttons subdevices should be enabled.

We need to keep the regulator flags as the regulator platform data can be NULL
on mc13892.

v2: first patch modified according to comments by Dmitry Torokhov
v3: Implement review by Uwe

Philippe Rétornaz (3):
  mc13783: add power button support
  mx31moboard: Add MC13783 power button support
  mc13xxx: implicitly enable leds and buttons

 arch/arm/mach-imx/mach-mx31moboard.c   |   10 +-
 drivers/input/misc/Kconfig             |   10 +
 drivers/input/misc/Makefile            |    1 +
 drivers/input/misc/mc13783-pwrbutton.c |  288 ++++++++++++++++++++++++++++++++
 drivers/mfd/mc13xxx-core.c             |    6 +-
 include/linux/mfd/mc13783.h            |    2 +-
 include/linux/mfd/mc13xxx.h            |   18 ++-
 7 files changed, 331 insertions(+), 4 deletions(-)
 create mode 100644 drivers/input/misc/mc13783-pwrbutton.c

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/3] mc13783: add power button support
  2011-07-09 23:06 [PATCH v3 0/3] mc13783: add pwr button support Philippe Rétornaz
@ 2011-07-09 23:06 ` Philippe Rétornaz
  2011-07-09 23:06   ` [PATCH 2/3] mx31moboard: Add MC13783 " Philippe Rétornaz
  0 siblings, 1 reply; 8+ messages in thread
From: Philippe Rétornaz @ 2011-07-09 23:06 UTC (permalink / raw)
  To: s.hauer
  Cc: linux-arm-kernel, dmitry.torokhov, sameo, linux-input, broonie,
	u.kleine-koenig, philippe.retornaz

This adds support for the power-on buttons of MC13783 PMIC.
This is done using a misc input device.

Signed-off-by: Philippe Rétornaz <philippe.retornaz@epfl.ch>
---
 drivers/input/misc/Kconfig             |   10 +
 drivers/input/misc/Makefile            |    1 +
 drivers/input/misc/mc13783-pwrbutton.c |  288 ++++++++++++++++++++++++++++++++
 drivers/mfd/mc13xxx-core.c             |    4 +
 include/linux/mfd/mc13783.h            |    1 +
 include/linux/mfd/mc13xxx.h            |   18 ++
 6 files changed, 322 insertions(+), 0 deletions(-)
 create mode 100644 drivers/input/misc/mc13783-pwrbutton.c

diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 45dc6aa..4272658 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -100,6 +100,16 @@ config INPUT_MAX8925_ONKEY
 	  To compile this driver as a module, choose M here: the module
 	  will be called max8925_onkey.
 
+config INPUT_MC13783_PWRBUTTON
+	tristate "MC13783 ON buttons"
+	depends on MFD_MC13783
+	help
+	  Support the ON buttons of MC13783 PMIC as an input device
+	  reporting power button status.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called mc13783-pwrbutton.
+
 config INPUT_APANEL
 	tristate "Fujitsu Lifebook Application Panel buttons"
 	depends on X86 && I2C && LEDS_CLASS
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index 38efb2c..ecced4c 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -27,6 +27,7 @@ obj-$(CONFIG_INPUT_IXP4XX_BEEPER)	+= ixp4xx-beeper.o
 obj-$(CONFIG_INPUT_KEYSPAN_REMOTE)	+= keyspan_remote.o
 obj-$(CONFIG_INPUT_M68K_BEEP)		+= m68kspkr.o
 obj-$(CONFIG_INPUT_MAX8925_ONKEY)	+= max8925_onkey.o
+obj-$(CONFIG_INPUT_MC13783_PWRBUTTON)	+= mc13783-pwrbutton.o
 obj-$(CONFIG_INPUT_PCAP)		+= pcap_keys.o
 obj-$(CONFIG_INPUT_PCF50633_PMU)	+= pcf50633-input.o
 obj-$(CONFIG_INPUT_PCF8574)		+= pcf8574_keypad.o
diff --git a/drivers/input/misc/mc13783-pwrbutton.c b/drivers/input/misc/mc13783-pwrbutton.c
new file mode 100644
index 0000000..1fe308b
--- /dev/null
+++ b/drivers/input/misc/mc13783-pwrbutton.c
@@ -0,0 +1,288 @@
+/**
+ * mc13783-pwrbutton.c - MC13783 Power Button Input Driver
+ *
+ * Copyright (C) 2011 Philippe Rétornaz
+ *
+ * Based on twl4030-pwrbutton driver by:
+ *     Peter De Schrijver <peter.de-schrijver@nokia.com>
+ *     Felipe Balbi <felipe.balbi@nokia.com>
+ *
+ * This file is subject to the terms and conditions of the GNU General
+ * Public License. See the file "COPYING" in the main directory of this
+ * archive for more details.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Suite 500, Boston, MA 02110-1335  USA
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/input.h>
+#include <linux/interrupt.h>
+#include <linux/platform_device.h>
+#include <linux/mfd/mc13783.h>
+#include <linux/sched.h>
+#include <linux/slab.h>
+
+struct mc13783_pwrb {
+	struct input_dev *pwr;
+	struct mc13xxx *mc13783;
+#define MC13783_PWRB_B1_POL_INVERT	(1 << 0)
+#define MC13783_PWRB_B2_POL_INVERT	(1 << 1)
+#define MC13783_PWRB_B3_POL_INVERT	(1 << 2)
+	int flags;
+	unsigned short keymap[3];
+};
+
+#define MC13783_REG_INTERRUPT_SENSE_1		5
+#define MC13783_IRQSENSE1_ONOFD1S		(1 << 3)
+#define MC13783_IRQSENSE1_ONOFD2S		(1 << 4)
+#define MC13783_IRQSENSE1_ONOFD3S		(1 << 5)
+
+#define MC13783_REG_POWER_CONTROL_2		15
+#define MC13783_POWER_CONTROL_2_ON1BDBNC	4
+#define MC13783_POWER_CONTROL_2_ON2BDBNC	6
+#define MC13783_POWER_CONTROL_2_ON3BDBNC	8
+#define MC13783_POWER_CONTROL_2_ON1BRSTEN	(1 << 1)
+#define MC13783_POWER_CONTROL_2_ON2BRSTEN	(1 << 2)
+#define MC13783_POWER_CONTROL_2_ON3BRSTEN	(1 << 3)
+
+static irqreturn_t button_irq(int irq, void *_priv)
+{
+	struct mc13783_pwrb *priv = _priv;
+	int val;
+	mc13xxx_irq_ack(priv->mc13783, irq);
+	mc13xxx_reg_read(priv->mc13783, MC13783_REG_INTERRUPT_SENSE_1, &val);
+
+	switch (irq) {
+	case MC13783_IRQ_ONOFD1:
+		val = val & MC13783_IRQSENSE1_ONOFD1S ? 1 : 0;
+		if (priv->flags & MC13783_PWRB_B1_POL_INVERT)
+			val ^= 1;
+		input_report_key(priv->pwr, priv->keymap[0], val);
+		break;
+
+	case MC13783_IRQ_ONOFD2:
+		val = val & MC13783_IRQSENSE1_ONOFD2S ? 1 : 0;
+		if (priv->flags & MC13783_PWRB_B2_POL_INVERT)
+			val ^= 1;
+		input_report_key(priv->pwr, priv->keymap[1], val);
+		break;
+
+	case MC13783_IRQ_ONOFD3:
+		val = val & MC13783_IRQSENSE1_ONOFD3S ? 1 : 0;
+		if (priv->flags & MC13783_PWRB_B3_POL_INVERT)
+			val ^= 1;
+		input_report_key(priv->pwr, priv->keymap[2], val);
+		break;
+	}
+
+	input_sync(priv->pwr);
+
+	return IRQ_HANDLED;
+}
+
+static int __devinit mc13783_pwrbutton_probe(struct platform_device *pdev)
+{
+	struct mc13xxx_buttons_platform_data *pdata;
+	struct mc13xxx *mc13783 = dev_get_drvdata(pdev->dev.parent);
+	struct input_dev *pwr;
+	struct mc13783_pwrb *priv;
+	int err = 0;
+	int reg = 0;
+
+	pdata = dev_get_platdata(&pdev->dev);
+	if (pdata == NULL) {
+		dev_err(&pdev->dev, "missing platform data\n");
+		return -ENODEV;
+	}
+
+	pwr = input_allocate_device();
+	if (!pwr) {
+		dev_dbg(&pdev->dev, "Can't allocate power button\n");
+		return -ENOMEM;
+	}
+
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+
+	if (!priv) {
+		err = -ENOMEM;
+		dev_dbg(&pdev->dev, "Can't allocate power button\n");
+		goto free_input_dev;
+	}
+
+	pwr->evbit[0] = BIT_MASK(EV_KEY);
+
+	reg |= (pdata->b1on_flags & 0x3) << MC13783_POWER_CONTROL_2_ON1BDBNC;
+	reg |= (pdata->b2on_flags & 0x3) << MC13783_POWER_CONTROL_2_ON2BDBNC;
+	reg |= (pdata->b3on_flags & 0x3) << MC13783_POWER_CONTROL_2_ON3BDBNC;
+
+	priv->pwr = pwr;
+	priv->mc13783 = mc13783;
+
+	mc13xxx_lock(mc13783);
+
+	if (pdata->b1on_flags & MC13783_BUTTON_ENABLE) {
+		priv->keymap[0] = pdata->b1on_key;
+		if (pdata->b1on_key != KEY_RESERVED)
+			__set_bit(pdata->b1on_key, pwr->keybit);
+
+		if (pdata->b1on_flags & MC13783_BUTTON_POL_INVERT)
+			priv->flags |= MC13783_PWRB_B1_POL_INVERT;
+
+		if (pdata->b1on_flags & MC13783_BUTTON_RESET_EN)
+			reg |= MC13783_POWER_CONTROL_2_ON1BRSTEN;
+
+		err = mc13xxx_irq_request(mc13783, MC13783_IRQ_ONOFD1,
+					  button_irq, "b1on", priv);
+
+		if (err) {
+			mc13xxx_unlock(mc13783);
+
+			dev_dbg(&pdev->dev, "Can't request irq\n");
+			goto free_priv;
+		}
+	}
+
+	if (pdata->b2on_flags & MC13783_BUTTON_ENABLE) {
+		priv->keymap[0] = pdata->b2on_key;
+		if (pdata->b2on_key != KEY_RESERVED)
+			__set_bit(pdata->b2on_key, pwr->keybit);
+
+		if (pdata->b2on_flags & MC13783_BUTTON_POL_INVERT)
+			priv->flags |= MC13783_PWRB_B2_POL_INVERT;
+
+		if (pdata->b2on_flags & MC13783_BUTTON_RESET_EN)
+			reg |= MC13783_POWER_CONTROL_2_ON2BRSTEN;
+
+		err = mc13xxx_irq_request(mc13783, MC13783_IRQ_ONOFD2,
+					  button_irq, "b2on", priv);
+
+		if (err) {
+			dev_dbg(&pdev->dev, "Can't request irq\n");
+			goto free_irq_b1;
+		}
+	}
+
+	if (pdata->b3on_flags & MC13783_BUTTON_ENABLE) {
+		priv->keymap[0] = pdata->b3on_key;
+		if (pdata->b3on_key != KEY_RESERVED)
+			__set_bit(pdata->b3on_key, pwr->keybit);
+
+		if (pdata->b3on_flags & MC13783_BUTTON_POL_INVERT)
+			priv->flags |= MC13783_PWRB_B3_POL_INVERT;
+
+		if (pdata->b3on_flags & MC13783_BUTTON_RESET_EN)
+			reg |= MC13783_POWER_CONTROL_2_ON3BRSTEN;
+
+		err = mc13xxx_irq_request(mc13783, MC13783_IRQ_ONOFD3,
+					  button_irq, "b3on", priv);
+
+		if (err) {
+			dev_dbg(&pdev->dev, "Can't request irq: %d\n", err);
+			goto free_irq_b2;
+		}
+	}
+
+	mc13xxx_reg_rmw(mc13783, MC13783_REG_POWER_CONTROL_2, 0x3FE, reg);
+
+	mc13xxx_unlock(mc13783);
+
+	pwr->name = "mc13783_pwrbutton";
+	pwr->phys = "mc13783_pwrbutton/input0";
+	pwr->dev.parent = &pdev->dev;
+
+	pwr->keycode = priv->keymap;
+	pwr->keycodemax = ARRAY_SIZE(priv->keymap);
+	pwr->keycodesize = sizeof(priv->keymap[0]);
+
+	err = input_register_device(pwr);
+	if (err) {
+		dev_dbg(&pdev->dev, "Can't register power button: %d\n", err);
+		goto free_irq;
+	}
+
+	platform_set_drvdata(pdev, priv);
+
+	return 0;
+
+free_irq:
+	mc13xxx_lock(mc13783);
+
+	if (pdata->b3on_flags & MC13783_BUTTON_ENABLE)
+		mc13xxx_irq_free(mc13783, MC13783_IRQ_ONOFD3, priv);
+free_irq_b2:
+	if (pdata->b2on_flags & MC13783_BUTTON_ENABLE)
+		mc13xxx_irq_free(mc13783, MC13783_IRQ_ONOFD2, priv);
+
+free_irq_b1:
+	if (pdata->b1on_flags & MC13783_BUTTON_ENABLE)
+		mc13xxx_irq_free(mc13783, MC13783_IRQ_ONOFD1, priv);
+
+	mc13xxx_unlock(mc13783);
+
+free_priv:
+	kfree(priv);
+
+free_input_dev:
+	input_free_device(pwr);
+
+	return err;
+}
+
+static int __devexit mc13783_pwrbutton_remove(struct platform_device *pdev)
+{
+	struct mc13783_pwrb *priv = platform_get_drvdata(pdev);
+	struct mc13xxx_buttons_platform_data *pdata;
+	pdata = dev_get_platdata(&pdev->dev);
+
+	mc13xxx_lock(priv->mc13783);
+
+	if (pdata->b3on_flags & MC13783_BUTTON_ENABLE)
+		mc13xxx_irq_free(priv->mc13783, MC13783_IRQ_ONOFD3, priv);
+	if (pdata->b2on_flags & MC13783_BUTTON_ENABLE)
+		mc13xxx_irq_free(priv->mc13783, MC13783_IRQ_ONOFD2, priv);
+	if (pdata->b1on_flags & MC13783_BUTTON_ENABLE)
+		mc13xxx_irq_free(priv->mc13783, MC13783_IRQ_ONOFD1, priv);
+
+	mc13xxx_unlock(priv->mc13783);
+
+	input_unregister_device(priv->pwr);
+	kfree(priv);
+
+	return 0;
+}
+
+struct platform_driver mc13783_pwrbutton_driver = {
+	.probe		= mc13783_pwrbutton_probe,
+	.remove		= __devexit_p(mc13783_pwrbutton_remove),
+	.driver		= {
+		.name	= "mc13783-pwrbutton",
+		.owner	= THIS_MODULE,
+	},
+};
+
+static int __init mc13783_pwrbutton_init(void)
+{
+	return platform_driver_register(&mc13783_pwrbutton_driver);
+}
+module_init(mc13783_pwrbutton_init);
+
+static void __exit mc13783_pwrbutton_exit(void)
+{
+	platform_driver_unregister(&mc13783_pwrbutton_driver);
+}
+module_exit(mc13783_pwrbutton_exit);
+
+MODULE_ALIAS("platform:mc13783-pwrbutton");
+MODULE_DESCRIPTION("MC13783 Power Button");
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Philippe Retornaz");
diff --git a/drivers/mfd/mc13xxx-core.c b/drivers/mfd/mc13xxx-core.c
index 7e4d44b..44edf78 100644
--- a/drivers/mfd/mc13xxx-core.c
+++ b/drivers/mfd/mc13xxx-core.c
@@ -778,6 +778,10 @@ err_revision:
 		mc13xxx_add_subdevice_pdata(mc13xxx, "%s-led",
 				pdata->leds, sizeof(*pdata->leds));
 
+	if (pdata->flags & MC13XXX_USE_BUTTON)
+		mc13xxx_add_subdevice_pdata(mc13xxx, "%s-pwrbutton",
+				pdata->buttons, sizeof(*pdata->buttons));
+
 	return 0;
 }
 
diff --git a/include/linux/mfd/mc13783.h b/include/linux/mfd/mc13783.h
index 7d0f3d6..bd4fbac 100644
--- a/include/linux/mfd/mc13783.h
+++ b/include/linux/mfd/mc13783.h
@@ -106,6 +106,7 @@ static inline int mc13783_irq_ack(struct mc13783 *mc13783, int irq)
 #define mc13783_regulator_platform_data mc13xxx_regulator_platform_data
 #define mc13783_led_platform_data mc13xxx_led_platform_data
 #define mc13783_leds_platform_data mc13xxx_leds_platform_data
+#define mc13783_buttons_platform_data mc13xxx_buttons_platform_data
 
 #define mc13783_platform_data mc13xxx_platform_data
 #define MC13783_USE_TOUCHSCREEN	MC13XXX_USE_TOUCHSCREEN
diff --git a/include/linux/mfd/mc13xxx.h b/include/linux/mfd/mc13xxx.h
index c064bea..22d8930 100644
--- a/include/linux/mfd/mc13xxx.h
+++ b/include/linux/mfd/mc13xxx.h
@@ -137,6 +137,22 @@ struct mc13xxx_leds_platform_data {
 	char tc3_period;
 };
 
+struct mc13xxx_buttons_platform_data {
+#define MC13783_BUTTON_DBNC_0MS		0
+#define MC13783_BUTTON_DBNC_30MS	1
+#define MC13783_BUTTON_DBNC_150MS	2
+#define MC13783_BUTTON_DBNC_750MS	3
+#define MC13783_BUTTON_ENABLE		(1 << 2)
+#define MC13783_BUTTON_POL_INVERT	(1 << 3)
+#define MC13783_BUTTON_RESET_EN		(1 << 4)
+	int b1on_flags;
+	unsigned short b1on_key;
+	int b2on_flags;
+	unsigned short b2on_key;
+	int b3on_flags;
+	unsigned short b3on_key;
+};
+
 struct mc13xxx_platform_data {
 #define MC13XXX_USE_TOUCHSCREEN (1 << 0)
 #define MC13XXX_USE_CODEC	(1 << 1)
@@ -144,10 +160,12 @@ struct mc13xxx_platform_data {
 #define MC13XXX_USE_RTC		(1 << 3)
 #define MC13XXX_USE_REGULATOR	(1 << 4)
 #define MC13XXX_USE_LED		(1 << 5)
+#define MC13XXX_USE_BUTTON	(1 << 6)
 	unsigned int flags;
 
 	struct mc13xxx_regulator_platform_data regulators;
 	struct mc13xxx_leds_platform_data *leds;
+	struct mc13xxx_buttons_platform_data *buttons;
 };
 
 #endif /* ifndef __LINUX_MFD_MC13XXX_H */
-- 
1.6.3.3

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/3] mx31moboard: Add MC13783 power button support
  2011-07-09 23:06 ` [PATCH 1/3] mc13783: add power " Philippe Rétornaz
@ 2011-07-09 23:06   ` Philippe Rétornaz
  2011-07-09 23:06     ` [PATCH 3/3] mc13xxx: implicitly enable leds and buttons Philippe Rétornaz
  0 siblings, 1 reply; 8+ messages in thread
From: Philippe Rétornaz @ 2011-07-09 23:06 UTC (permalink / raw)
  To: s.hauer
  Cc: linux-arm-kernel, dmitry.torokhov, sameo, linux-input, broonie,
	u.kleine-koenig, philippe.retornaz

Add the power-on button on mx31moboard using MC13783 PMIC.

Signed-off-by: Philippe Rétornaz <philippe.retornaz@epfl.ch>
---
 arch/arm/mach-imx/mach-mx31moboard.c |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-imx/mach-mx31moboard.c b/arch/arm/mach-imx/mach-mx31moboard.c
index 12ee755..447143b 100644
--- a/arch/arm/mach-imx/mach-mx31moboard.c
+++ b/arch/arm/mach-imx/mach-mx31moboard.c
@@ -28,6 +28,7 @@
 #include <linux/spi/spi.h>
 #include <linux/types.h>
 #include <linux/memblock.h>
+#include <linux/input.h>
 
 #include <linux/usb/otg.h>
 #include <linux/usb/ulpi.h>
@@ -264,14 +265,21 @@ static struct mc13783_leds_platform_data moboard_leds = {
 	.tc2_period = MC13783_LED_PERIOD_10MS,
 };
 
+static struct mc13783_buttons_platform_data moboard_buttons = {
+	.b1on_flags = MC13783_BUTTON_DBNC_750MS | MC13783_BUTTON_ENABLE |
+			MC13783_BUTTON_POL_INVERT,
+	.b1on_key = KEY_POWER,
+};
+
 static struct mc13xxx_platform_data moboard_pmic = {
 	.regulators = {
 		.regulators = moboard_regulators,
 		.num_regulators = ARRAY_SIZE(moboard_regulators),
 	},
 	.leds = &moboard_leds,
+	.buttons = &moboard_buttons,
 	.flags = MC13XXX_USE_REGULATOR | MC13XXX_USE_RTC |
-		MC13XXX_USE_ADC | MC13XXX_USE_LED,
+		MC13XXX_USE_ADC | MC13XXX_USE_LED | MC13XXX_USE_BUTTON,
 };
 
 static struct spi_board_info moboard_spi_board_info[] __initdata = {
-- 
1.6.3.3

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 3/3] mc13xxx: implicitly enable leds and buttons
  2011-07-09 23:06   ` [PATCH 2/3] mx31moboard: Add MC13783 " Philippe Rétornaz
@ 2011-07-09 23:06     ` Philippe Rétornaz
  2011-07-10  8:04       ` Uwe Kleine-König
  0 siblings, 1 reply; 8+ messages in thread
From: Philippe Rétornaz @ 2011-07-09 23:06 UTC (permalink / raw)
  To: s.hauer
  Cc: linux-arm-kernel, dmitry.torokhov, sameo, linux-input, broonie,
	u.kleine-koenig, philippe.retornaz

The leds and buttons subdevices cannot be used without additional
platform data.

Use the presence of the platform data to enable the device instead
of an additional flag.

Signed-off-by: Philippe Rétornaz <philippe.retornaz@epfl.ch>
---
 arch/arm/mach-imx/mach-mx31moboard.c |    2 +-
 drivers/mfd/mc13xxx-core.c           |    4 ++--
 include/linux/mfd/mc13783.h          |    1 -
 include/linux/mfd/mc13xxx.h          |    2 --
 4 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-imx/mach-mx31moboard.c b/arch/arm/mach-imx/mach-mx31moboard.c
index 447143b..c4cd6b8 100644
--- a/arch/arm/mach-imx/mach-mx31moboard.c
+++ b/arch/arm/mach-imx/mach-mx31moboard.c
@@ -279,7 +279,7 @@ static struct mc13xxx_platform_data moboard_pmic = {
 	.leds = &moboard_leds,
 	.buttons = &moboard_buttons,
 	.flags = MC13XXX_USE_REGULATOR | MC13XXX_USE_RTC |
-		MC13XXX_USE_ADC | MC13XXX_USE_LED | MC13XXX_USE_BUTTON,
+		MC13XXX_USE_ADC,
 };
 
 static struct spi_board_info moboard_spi_board_info[] __initdata = {
diff --git a/drivers/mfd/mc13xxx-core.c b/drivers/mfd/mc13xxx-core.c
index 44edf78..130781f 100644
--- a/drivers/mfd/mc13xxx-core.c
+++ b/drivers/mfd/mc13xxx-core.c
@@ -774,11 +774,11 @@ err_revision:
 	if (pdata->flags & MC13XXX_USE_TOUCHSCREEN)
 		mc13xxx_add_subdevice(mc13xxx, "%s-ts");
 
-	if (pdata->flags & MC13XXX_USE_LED)
+	if (pdata->leds)
 		mc13xxx_add_subdevice_pdata(mc13xxx, "%s-led",
 				pdata->leds, sizeof(*pdata->leds));
 
-	if (pdata->flags & MC13XXX_USE_BUTTON)
+	if (pdata->buttons)
 		mc13xxx_add_subdevice_pdata(mc13xxx, "%s-pwrbutton",
 				pdata->buttons, sizeof(*pdata->buttons));
 
diff --git a/include/linux/mfd/mc13783.h b/include/linux/mfd/mc13783.h
index bd4fbac..61dca4c 100644
--- a/include/linux/mfd/mc13783.h
+++ b/include/linux/mfd/mc13783.h
@@ -114,7 +114,6 @@ static inline int mc13783_irq_ack(struct mc13783 *mc13783, int irq)
 #define MC13783_USE_ADC		MC13XXX_USE_ADC
 #define MC13783_USE_RTC		MC13XXX_USE_RTC
 #define MC13783_USE_REGULATOR	MC13XXX_USE_REGULATOR
-#define MC13783_USE_LED		MC13XXX_USE_LED
 
 #define MC13783_ADC_MODE_TS		1
 #define MC13783_ADC_MODE_SINGLE_CHAN	2
diff --git a/include/linux/mfd/mc13xxx.h b/include/linux/mfd/mc13xxx.h
index 22d8930..6c4b854 100644
--- a/include/linux/mfd/mc13xxx.h
+++ b/include/linux/mfd/mc13xxx.h
@@ -159,8 +159,6 @@ struct mc13xxx_platform_data {
 #define MC13XXX_USE_ADC		(1 << 2)
 #define MC13XXX_USE_RTC		(1 << 3)
 #define MC13XXX_USE_REGULATOR	(1 << 4)
-#define MC13XXX_USE_LED		(1 << 5)
-#define MC13XXX_USE_BUTTON	(1 << 6)
 	unsigned int flags;
 
 	struct mc13xxx_regulator_platform_data regulators;
-- 
1.6.3.3

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 3/3] mc13xxx: implicitly enable leds and buttons
  2011-07-09 23:06     ` [PATCH 3/3] mc13xxx: implicitly enable leds and buttons Philippe Rétornaz
@ 2011-07-10  8:04       ` Uwe Kleine-König
  2011-07-10  8:37         ` Mark Brown
  2011-07-10 11:33         ` Philippe Rétornaz
  0 siblings, 2 replies; 8+ messages in thread
From: Uwe Kleine-König @ 2011-07-10  8:04 UTC (permalink / raw)
  To: Philippe Rétornaz
  Cc: s.hauer, linux-arm-kernel, dmitry.torokhov, sameo, linux-input,
	broonie

Hello,

On Sun, Jul 10, 2011 at 01:06:36AM +0200, Philippe Rétornaz wrote:
> The leds and buttons subdevices cannot be used without additional
> platform data.
> 
> Use the presence of the platform data to enable the device instead
> of an additional flag.
I guess you could make some people happy by splitting this patch into:

	- mfd/mc13xxx: implicitly enable leds and buttons (and
	  regulators?)
	- drop MC13XXX_USE_... in arch code

because the patches have different paths into mainline (unless it's
handled otherwise).

And I wonder why you introduce MC13XXX_USE_BUTTON in the first place.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 3/3] mc13xxx: implicitly enable leds and buttons
  2011-07-10  8:04       ` Uwe Kleine-König
@ 2011-07-10  8:37         ` Mark Brown
  2011-07-10 11:33         ` Philippe Rétornaz
  1 sibling, 0 replies; 8+ messages in thread
From: Mark Brown @ 2011-07-10  8:37 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Philippe Rétornaz, s.hauer, linux-arm-kernel,
	dmitry.torokhov, sameo, linux-input

On Sun, Jul 10, 2011 at 10:04:50AM +0200, Uwe Kleine-König wrote:

> 	- mfd/mc13xxx: implicitly enable leds and buttons (and
> 	  regulators?)

It's nicer to enable regulators unconditionally, even if they're not
controlled by software in the system reading their state can still be
useful.
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 3/3] mc13xxx: implicitly enable leds and buttons
  2011-07-10  8:04       ` Uwe Kleine-König
  2011-07-10  8:37         ` Mark Brown
@ 2011-07-10 11:33         ` Philippe Rétornaz
  2011-07-10 14:15           ` Uwe Kleine-König
  1 sibling, 1 reply; 8+ messages in thread
From: Philippe Rétornaz @ 2011-07-10 11:33 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: s.hauer, linux-arm-kernel, dmitry.torokhov, sameo, linux-input,
	broonie

Le dimanche 10 juillet 2011 10:04:50, Uwe Kleine-König a écrit :
> Hello,
> 
> On Sun, Jul 10, 2011 at 01:06:36AM +0200, Philippe Rétornaz wrote:
> > The leds and buttons subdevices cannot be used without additional
> > platform data.
> > 
> > Use the presence of the platform data to enable the device instead
> > of an additional flag.
> 
> I guess you could make some people happy by splitting this patch into:
> 
> 	- mfd/mc13xxx: implicitly enable leds and buttons (and
> 	  regulators?)
> 	- drop MC13XXX_USE_... in arch code

But this mean that between the two commit the build will be broken or the 
behavior will change.  Which will break bissections.

For the regulator stuff, I don't want to change the behavior of others board as 
some (mx31lite) enable the regualtor without platform data. That's why I did 
not changed it. 
Moreover for the mc13892 chip (which share the same flag), the probe() does 
modify some registers, so it's not just some cosmetic change, it will change 
the behavior on some boards.

BTW I will be on holydays next week so I won't be able to produce new patches 
soon.

Thanks,

Philippe
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 3/3] mc13xxx: implicitly enable leds and buttons
  2011-07-10 11:33         ` Philippe Rétornaz
@ 2011-07-10 14:15           ` Uwe Kleine-König
  0 siblings, 0 replies; 8+ messages in thread
From: Uwe Kleine-König @ 2011-07-10 14:15 UTC (permalink / raw)
  To: Philippe Rétornaz
  Cc: s.hauer, linux-arm-kernel, dmitry.torokhov, sameo, linux-input,
	broonie

Hello Philippe,

On Sun, Jul 10, 2011 at 01:33:09PM +0200, Philippe Rétornaz wrote:
> Le dimanche 10 juillet 2011 10:04:50, Uwe Kleine-König a écrit :
> > Hello,
> > 
> > On Sun, Jul 10, 2011 at 01:06:36AM +0200, Philippe Rétornaz wrote:
> > > The leds and buttons subdevices cannot be used without additional
> > > platform data.
> > > 
> > > Use the presence of the platform data to enable the device instead
> > > of an additional flag.
> > 
> > I guess you could make some people happy by splitting this patch into:
> > 
> > 	- mfd/mc13xxx: implicitly enable leds and buttons (and
> > 	  regulators?)
> > 	- drop MC13XXX_USE_... in arch code
> 
> But this mean that between the two commit the build will be broken or the 
> behavior will change.  Which will break bissections.
If you don't remove the MC13XXX_USE_... constants the unconverted boards
should still compile.
> 
> For the regulator stuff, I don't want to change the behavior of others
> board as some (mx31lite) enable the regualtor without platform data.
> That's why I did not changed it. 
If having pdata=NULL is a valid config then either we need to keep the
constant for regulators or (as Mark suggested) register unconditionally.

> Moreover for the mc13892 chip (which share the same flag), the probe() does 
> modify some registers, so it's not just some cosmetic change, it will change 
> the behavior on some boards.
no fear, that's what the -rc phase is there for. When improving
something you often have to change behaviour.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2011-07-10 14:15 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-07-09 23:06 [PATCH v3 0/3] mc13783: add pwr button support Philippe Rétornaz
2011-07-09 23:06 ` [PATCH 1/3] mc13783: add power " Philippe Rétornaz
2011-07-09 23:06   ` [PATCH 2/3] mx31moboard: Add MC13783 " Philippe Rétornaz
2011-07-09 23:06     ` [PATCH 3/3] mc13xxx: implicitly enable leds and buttons Philippe Rétornaz
2011-07-10  8:04       ` Uwe Kleine-König
2011-07-10  8:37         ` Mark Brown
2011-07-10 11:33         ` Philippe Rétornaz
2011-07-10 14:15           ` Uwe Kleine-König

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).