[PATCH v5 0/3] mfd: tps65217: Add power-button and IRQ support

[PATCH v5 0/3] mfd: tps65217: Add power-button and IRQ support

[Marcin Niestroj]

Hi,

This is a resend of v4, rebased on v4.8-rc4, with dropped two patches:
 * "power_supply: tps65217-charger: Fix NULL deref during property export"
   (already in mainline)
 * "power_supply: tps65217-charger: Add support for IRQs" (queued into
   power-supply's -next branch)

The following patches add tps65217 power button support and make it usable
with am335x boards. It was developed and rebased on v4.8-rc4.

Changes v4 -> v5:
 * Dropped two patches (they were already dropped in v4 resend):
   * "power_supply: tps65217-charger: Fix NULL deref during property export"
     (already in mainline)
   * "power_supply: tps65217-charger: Add support for IRQs" (queued into
     power-supply's -next branch)
 * Combine tps65217 and tps65218 power button drivers (as suggested by Dmitry)
 * Combine device-tree documentation for tps65217 and tps65218 power buttons

Changes v3 -> v4:
 * Add irq_set_parent() during irq init (suggested by Grygorii)
 * Remove hardcoded IRQ trigger type, rely on DT bindings instead
   (suggested by Grygorii)
 * Rebased and tested on top of v4.7-rc4

Changes v2 -> v3 (suggested by Lee):
 * Alphabetical reorder of includes
 * Rename enum tps65217_irqs -> tps65217_irq_type, so we won't confuse
   it with structure object with the same name.
 * Fix inconsistent order of irq_enable and irq_disable
 * Remove redundant 'else' when returning from tps65217_irq_thread

Changes v1 -> v2:
 * Added information about parent device in tps65217 power button
   device-tree binding documentation (suggested by Rob)
 * Rebased and tested on top of v4.7-rc3

Marcin Niestroj (3):
  mfd: tps65217: Add support for IRQs
  mfd: tps65217: Add power button as subdevice
  Input: Add support for tps65217 power button

 .../bindings/input/tps65218-pwrbutton.txt          |  17 +-
 drivers/input/misc/Kconfig                         |   4 +-
 drivers/input/misc/tps65218-pwrbutton.c            | 102 +++++++---
 drivers/mfd/Kconfig                                |   1 +
 drivers/mfd/tps65217.c                             | 205 ++++++++++++++++++++-
 include/linux/mfd/tps65217.h                       |  12 ++
 6 files changed, 298 insertions(+), 43 deletions(-)

-- 
2.9.3

[PATCH v5 1/3] mfd: tps65217: Add support for IRQs

[Marcin Niestroj]

Add support for handling IRQs: power button, AC and USB power state
changes. Mask and interrupt bits are shared within one register, which
prevents us to use regmap_irq implementation. New irq_domain is created in
order to add interrupt handling for each tps65217's subsystem. IRQ
resources have been added for charger subsystem to be able to notify about
AC and USB state changes.

Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
Acked-by: Lee Jones <lee.jones@linaro.org>
Reviewed-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
Changes v4 -> v5: none

Changes v3 -> v4 (suggested by Grygorii):
 * Add irq_set_parent() during irq init
 * Remove hardcoded IRQ trigger type, rely on DT bindings instead

Changes v2 -> v3 (suggested by Lee):
 * Alphabetical reorder of includes
 * Rename enum tps65217_irqs -> tps65217_irq_type, so we won't confuse
   it with structure object with the same name.
 * Fix inconsistent order of irq_enable and irq_disable
 * Remove redundant 'else' when returning from tps65217_irq_thread

Changes v1 -> v2: none

 drivers/mfd/Kconfig          |   1 +
 drivers/mfd/tps65217.c       | 195 +++++++++++++++++++++++++++++++++++++++++--
 include/linux/mfd/tps65217.h |  12 +++
 3 files changed, 200 insertions(+), 8 deletions(-)

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 2d1fb64..35e9c9a 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1214,6 +1214,7 @@ config MFD_TPS65217
 	depends on I2C
 	select MFD_CORE
 	select REGMAP_I2C
+	select IRQ_DOMAIN
 	help
 	  If you say yes here you get support for the TPS65217 series of
 	  Power Management / White LED chips.
diff --git a/drivers/mfd/tps65217.c b/drivers/mfd/tps65217.c
index 049a6fc..41b5d59 100644
--- a/drivers/mfd/tps65217.c
+++ b/drivers/mfd/tps65217.c
@@ -15,22 +15,99 @@
  * GNU General Public License for more details.
  */
 
-#include <linux/kernel.h>
 #include <linux/device.h>
-#include <linux/module.h>
-#include <linux/platform_device.h>
+#include <linux/err.h>
 #include <linux/init.h>
+#include <linux/interrupt.h>
 #include <linux/i2c.h>
-#include <linux/slab.h>
-#include <linux/regmap.h>
-#include <linux/err.h>
+#include <linux/irq.h>
+#include <linux/irqdomain.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
 
 #include <linux/mfd/core.h>
 #include <linux/mfd/tps65217.h>
 
-static const struct mfd_cell tps65217s[] = {
+static struct resource charger_resources[] = {
+	DEFINE_RES_IRQ_NAMED(TPS65217_IRQ_AC, "AC"),
+	DEFINE_RES_IRQ_NAMED(TPS65217_IRQ_USB, "USB"),
+};
+
+struct tps65217_irq {
+	int mask;
+	int interrupt;
+};
+
+static const struct tps65217_irq tps65217_irqs[] = {
+	[TPS65217_IRQ_PB] = {
+		.mask = TPS65217_INT_PBM,
+		.interrupt = TPS65217_INT_PBI,
+	},
+	[TPS65217_IRQ_AC] = {
+		.mask = TPS65217_INT_ACM,
+		.interrupt = TPS65217_INT_ACI,
+	},
+	[TPS65217_IRQ_USB] = {
+		.mask = TPS65217_INT_USBM,
+		.interrupt = TPS65217_INT_USBI,
+	},
+};
+
+static void tps65217_irq_lock(struct irq_data *data)
+{
+	struct tps65217 *tps = irq_data_get_irq_chip_data(data);
+
+	mutex_lock(&tps->irq_lock);
+}
+
+static void tps65217_irq_sync_unlock(struct irq_data *data)
+{
+	struct tps65217 *tps = irq_data_get_irq_chip_data(data);
+	int ret;
+
+	ret = tps65217_reg_write(tps, TPS65217_REG_INT, tps->irq_mask,
+				TPS65217_PROTECT_NONE);
+	if (ret != 0)
+		dev_err(tps->dev, "Failed to sync IRQ masks\n");
+
+	mutex_unlock(&tps->irq_lock);
+}
+
+static const inline struct tps65217_irq *
+irq_to_tps65217_irq(struct tps65217 *tps, struct irq_data *data)
+{
+	return &tps65217_irqs[data->hwirq];
+}
+
+static void tps65217_irq_enable(struct irq_data *data)
+{
+	struct tps65217 *tps = irq_data_get_irq_chip_data(data);
+	const struct tps65217_irq *irq_data = irq_to_tps65217_irq(tps, data);
+
+	tps->irq_mask &= ~irq_data->mask;
+}
+
+static void tps65217_irq_disable(struct irq_data *data)
+{
+	struct tps65217 *tps = irq_data_get_irq_chip_data(data);
+	const struct tps65217_irq *irq_data = irq_to_tps65217_irq(tps, data);
+
+	tps->irq_mask |= irq_data->mask;
+}
+
+static struct irq_chip tps65217_irq_chip = {
+	.irq_bus_lock		= tps65217_irq_lock,
+	.irq_bus_sync_unlock	= tps65217_irq_sync_unlock,
+	.irq_enable		= tps65217_irq_enable,
+	.irq_disable		= tps65217_irq_disable,
+};
+
+static struct mfd_cell tps65217s[] = {
 	{
 		.name = "tps65217-pmic",
 		.of_compatible = "ti,tps65217-pmic",
@@ -41,10 +118,90 @@ static const struct mfd_cell tps65217s[] = {
 	},
 	{
 		.name = "tps65217-charger",
+		.num_resources = ARRAY_SIZE(charger_resources),
+		.resources = charger_resources,
 		.of_compatible = "ti,tps65217-charger",
 	},
 };
 
+static irqreturn_t tps65217_irq_thread(int irq, void *data)
+{
+	struct tps65217 *tps = data;
+	unsigned int status;
+	bool handled = false;
+	int i;
+	int ret;
+
+	ret = tps65217_reg_read(tps, TPS65217_REG_INT, &status);
+	if (ret < 0) {
+		dev_err(tps->dev, "Failed to read IRQ status: %d\n",
+			ret);
+		return IRQ_NONE;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(tps65217_irqs); i++) {
+		if (status & tps65217_irqs[i].interrupt) {
+			handle_nested_irq(irq_find_mapping(tps->irq_domain, i));
+			handled = true;
+		}
+	}
+
+	if (handled)
+		return IRQ_HANDLED;
+
+	return IRQ_NONE;
+}
+
+static int tps65217_irq_map(struct irq_domain *h, unsigned int virq,
+			irq_hw_number_t hw)
+{
+	struct tps65217 *tps = h->host_data;
+
+	irq_set_chip_data(virq, tps);
+	irq_set_chip_and_handler(virq, &tps65217_irq_chip, handle_edge_irq);
+	irq_set_nested_thread(virq, 1);
+	irq_set_parent(virq, tps->irq);
+	irq_set_noprobe(virq);
+
+	return 0;
+}
+
+static const struct irq_domain_ops tps65217_irq_domain_ops = {
+	.map = tps65217_irq_map,
+};
+
+static int tps65217_irq_init(struct tps65217 *tps, int irq)
+{
+	int ret;
+
+	mutex_init(&tps->irq_lock);
+	tps->irq = irq;
+
+	/* Mask all interrupt sources */
+	tps->irq_mask = (TPS65217_INT_RESERVEDM | TPS65217_INT_PBM
+			| TPS65217_INT_ACM | TPS65217_INT_USBM);
+	tps65217_reg_write(tps, TPS65217_REG_INT, tps->irq_mask,
+			TPS65217_PROTECT_NONE);
+
+	tps->irq_domain = irq_domain_add_linear(tps->dev->of_node,
+		TPS65217_NUM_IRQ, &tps65217_irq_domain_ops, tps);
+	if (!tps->irq_domain) {
+		dev_err(tps->dev, "Could not create IRQ domain\n");
+		return -ENOMEM;
+	}
+
+	ret = devm_request_threaded_irq(tps->dev, irq, NULL,
+					tps65217_irq_thread, IRQF_ONESHOT,
+					"tps65217-irq", tps);
+	if (ret) {
+		dev_err(tps->dev, "Failed to request IRQ %d: %d\n",
+			irq, ret);
+		return ret;
+	}
+
+	return 0;
+}
+
 /**
  * tps65217_reg_read: Read a single tps65217 register.
  *
@@ -149,11 +306,22 @@ int tps65217_clear_bits(struct tps65217 *tps, unsigned int reg,
 }
 EXPORT_SYMBOL_GPL(tps65217_clear_bits);
 
+static bool tps65217_volatile_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case TPS65217_REG_INT:
+		return true;
+	default:
+		return false;
+	}
+}
+
 static const struct regmap_config tps65217_regmap_config = {
 	.reg_bits = 8,
 	.val_bits = 8,
 
 	.max_register = TPS65217_REG_MAX,
+	.volatile_reg = tps65217_volatile_reg,
 };
 
 static const struct of_device_id tps65217_of_match[] = {
@@ -205,8 +373,19 @@ static int tps65217_probe(struct i2c_client *client,
 		return ret;
 	}
 
+	if (client->irq) {
+		tps65217_irq_init(tps, client->irq);
+	} else {
+		int i;
+
+		/* Don't tell children about IRQ resources which won't fire */
+		for (i = 0; i < ARRAY_SIZE(tps65217s); i++)
+			tps65217s[i].num_resources = 0;
+	}
+
 	ret = devm_mfd_add_devices(tps->dev, -1, tps65217s,
-				   ARRAY_SIZE(tps65217s), NULL, 0, NULL);
+				   ARRAY_SIZE(tps65217s), NULL, 0,
+				   tps->irq_domain);
 	if (ret < 0) {
 		dev_err(tps->dev, "mfd_add_devices failed: %d\n", ret);
 		return ret;
diff --git a/include/linux/mfd/tps65217.h b/include/linux/mfd/tps65217.h
index 1c88231..4ccda89 100644
--- a/include/linux/mfd/tps65217.h
+++ b/include/linux/mfd/tps65217.h
@@ -73,6 +73,7 @@
 #define TPS65217_PPATH_AC_CURRENT_MASK	0x0C
 #define TPS65217_PPATH_USB_CURRENT_MASK	0x03
 
+#define TPS65217_INT_RESERVEDM		BIT(7)
 #define TPS65217_INT_PBM		BIT(6)
 #define TPS65217_INT_ACM		BIT(5)
 #define TPS65217_INT_USBM		BIT(4)
@@ -233,6 +234,13 @@ struct tps65217_bl_pdata {
 	int dft_brightness;
 };
 
+enum tps65217_irq_type {
+	TPS65217_IRQ_PB,
+	TPS65217_IRQ_AC,
+	TPS65217_IRQ_USB,
+	TPS65217_NUM_IRQ
+};
+
 /**
  * struct tps65217_board - packages regulator init data
  * @tps65217_regulator_data: regulator initialization values
@@ -258,6 +266,10 @@ struct tps65217 {
 	struct regulator_desc desc[TPS65217_NUM_REGULATOR];
 	struct regmap *regmap;
 	u8 *strobes;
+	struct irq_domain *irq_domain;
+	struct mutex irq_lock;
+	u8 irq_mask;
+	int irq;
 };
 
 static inline struct tps65217 *dev_to_tps65217(struct device *dev)
-- 
2.9.3

Re: [PATCH v5 1/3] mfd: tps65217: Add support for IRQs

[Tony Lindgren]

* Marcin Niestroj <m.niestroj@grinn-global.com> [160908 02:59]:
> Add support for handling IRQs: power button, AC and USB power state
> changes. Mask and interrupt bits are shared within one register, which
> prevents us to use regmap_irq implementation. New irq_domain is created in
> order to add interrupt handling for each tps65217's subsystem. IRQ
> resources have been added for charger subsystem to be able to notify about
> AC and USB state changes.
> 
> Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
> Acked-by: Lee Jones <lee.jones@linaro.org>
> Reviewed-by: Grygorii Strashko <grygorii.strashko@ti.com>

Gave this a try with the peripheral USB port on BeagleBone black,
works fine for me to detect cable events. Will post the musb dsps
related patches after some clean up in few days. Anyways, feel
free to add:

Tested-by: Tony Lindgren <tony@atomide.com>

[PATCH v5 2/3] mfd: tps65217: Add power button as subdevice

[Marcin Niestroj]

Add tps65217 power buttor subdevice with assigned IRQ resources.

Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
Acked-by: Lee Jones <lee.jones@linaro.org>
---
Depends on patch 1 in series

Changes v1 -> v5: none

 drivers/mfd/tps65217.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/mfd/tps65217.c b/drivers/mfd/tps65217.c
index 41b5d59..57c8741 100644
--- a/drivers/mfd/tps65217.c
+++ b/drivers/mfd/tps65217.c
@@ -38,6 +38,10 @@ static struct resource charger_resources[] = {
 	DEFINE_RES_IRQ_NAMED(TPS65217_IRQ_USB, "USB"),
 };
 
+static struct resource pb_resources[] = {
+	DEFINE_RES_IRQ_NAMED(TPS65217_IRQ_PB, "PB"),
+};
+
 struct tps65217_irq {
 	int mask;
 	int interrupt;
@@ -122,6 +126,12 @@ static struct mfd_cell tps65217s[] = {
 		.resources = charger_resources,
 		.of_compatible = "ti,tps65217-charger",
 	},
+	{
+		.name = "tps65217-pwrbutton",
+		.num_resources = ARRAY_SIZE(pb_resources),
+		.resources = pb_resources,
+		.of_compatible = "ti,tps65217-pwrbutton",
+	},
 };
 
 static irqreturn_t tps65217_irq_thread(int irq, void *data)
-- 
2.9.3

Re: [PATCH v5 2/3] mfd: tps65217: Add power button as subdevice

[Tony Lindgren]

* Marcin Niestroj <m.niestroj@grinn-global.com> [160908 02:59]:
> Add tps65217 power buttor subdevice with assigned IRQ resources.
> 
> Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
> Acked-by: Lee Jones <lee.jones@linaro.org>

Acked-by: Tony Lindgren <tony@atomide.com>

[PATCH v5 3/3] Input: Add support for tps65217 power button

[Marcin Niestroj]

This driver enables us to use tps65217's power button as KEY_POWER on
am335x boards (directly connected button in chiliboard, accessible pin
via expansion header in beaglebone). This patch has been tested with
chiliboard.

Signed-off-by: Marcin Niestroj <m.niestroj-z3quKL4iOrmQ6ZAhV5LmOA@public.gmane.org>
---
Depends on patches 1-2 in series and commit
47d7d5ed68d877269003a392b7008905d65650bb
("power_supply: tps65217-charger: Add support for IRQs")
in power-supply's -next branch.

Changes v4 -> v5:
 * Add support for tps65217 power button in tps65218-pwrbutton driver
   (as suggested by Dmitry)
 * Device-tree documentation was combined with tps65218-pwrbutton

Changes v2 -> v4: none

Changes v1 -> v2:
 * Added information about parent device in tps65217 power button
   device-tree binding documentation (suggested by Rob)

 .../bindings/input/tps65218-pwrbutton.txt          |  17 +++-
 drivers/input/misc/Kconfig                         |   4 +-
 drivers/input/misc/tps65218-pwrbutton.c            | 102 +++++++++++++++------
 3 files changed, 88 insertions(+), 35 deletions(-)

diff --git a/Documentation/devicetree/bindings/input/tps65218-pwrbutton.txt b/Documentation/devicetree/bindings/input/tps65218-pwrbutton.txt
index e30e0b9..3e5b979 100644
--- a/Documentation/devicetree/bindings/input/tps65218-pwrbutton.txt
+++ b/Documentation/devicetree/bindings/input/tps65218-pwrbutton.txt
@@ -1,13 +1,24 @@
-Texas Instruments TPS65218 power button
+Texas Instruments TPS65217 and TPS65218 power button
+
+This module is part of the TPS65217/TPS65218. For more details about the whole
+TPS65217 chip see Documentation/devicetree/bindings/regulator/tps65217.txt.
 
 This driver provides a simple power button event via an Interrupt.
 
 Required properties:
-- compatible: should be "ti,tps65218-pwrbutton"
+- compatible: should be "ti,tps65217-pwrbutton" or "ti,tps65218-pwrbutton"
+
+Required properties for TPS65218:
 - interrupts: should be one of the following
    - <3 IRQ_TYPE_EDGE_BOTH>: For controllers compatible with tps65218
 
-Example:
+Examples:
+
+&tps {
+	tps65217-pwrbutton {
+		compatible = "ti,tps65217-pwrbutton";
+	};
+};
 
 &tps {
 	power-button {
diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index efb0ca8..dbd5b75 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -454,10 +454,10 @@ config INPUT_RETU_PWRBUTTON
 
 config INPUT_TPS65218_PWRBUTTON
 	tristate "TPS65218 Power button driver"
-	depends on MFD_TPS65218
+	depends on (MFD_TPS65217 || MFD_TPS65218)
 	help
 	  Say Y here if you want to enable power buttong reporting for
-	  the TPS65218 Power Management IC device.
+	  TPS65217 and TPS65218 Power Management IC device.
 
 	  To compile this driver as a module, choose M here. The module will
 	  be called tps65218-pwrbutton.
diff --git a/drivers/input/misc/tps65218-pwrbutton.c b/drivers/input/misc/tps65218-pwrbutton.c
index a39b626..b7964d1 100644
--- a/drivers/input/misc/tps65218-pwrbutton.c
+++ b/drivers/input/misc/tps65218-pwrbutton.c
@@ -1,8 +1,9 @@
 /*
- * Texas Instruments' TPS65218 Power Button Input Driver
+ * Texas Instruments' TPS65217 and TPS65218 Power Button Input Driver
  *
  * Copyright (C) 2014 Texas Instruments Incorporated - http://www.ti.com/
  * Author: Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>
+ * Author: Marcin Niestroj <m.niestroj-z3quKL4iOrmQ6ZAhV5LmOA@public.gmane.org>
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 as
@@ -18,31 +19,64 @@
 #include <linux/input.h>
 #include <linux/interrupt.h>
 #include <linux/kernel.h>
+#include <linux/mfd/tps65217.h>
 #include <linux/mfd/tps65218.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
+#include <linux/regmap.h>
 #include <linux/slab.h>
 
-struct tps65218_pwrbutton {
+struct tps6521x_data {
+	size_t regmap_offset;
+	unsigned int reg_status;
+	unsigned int pb_mask;
+	const char *name;
+};
+
+static const struct tps6521x_data tps65217_data = {
+	.regmap_offset = offsetof(struct tps65217, regmap),
+	.reg_status = TPS65217_REG_STATUS,
+	.pb_mask = TPS65217_STATUS_PB,
+	.name = "tps65217_pwrbutton",
+};
+
+static const struct tps6521x_data tps65218_data = {
+	.regmap_offset = offsetof(struct tps65218, regmap),
+	.reg_status = TPS65218_REG_STATUS,
+	.pb_mask = TPS65218_STATUS_PB_STATE,
+	.name = "tps65218_pwrbutton",
+};
+
+struct tps6521x_pwrbutton {
 	struct device *dev;
-	struct tps65218 *tps;
+	struct regmap *regmap;
 	struct input_dev *idev;
+	const struct tps6521x_data *data;
+	char phys[32];
+};
+
+static const struct of_device_id of_tps6521x_pb_match[] = {
+	{ .compatible = "ti,tps65217-pwrbutton", .data = &tps65217_data },
+	{ .compatible = "ti,tps65218-pwrbutton", .data = &tps65218_data },
+	{ },
 };
+MODULE_DEVICE_TABLE(of, of_tps6521x_pb_match);
 
-static irqreturn_t tps65218_pwr_irq(int irq, void *_pwr)
+static irqreturn_t tps6521x_pb_irq(int irq, void *_pwr)
 {
-	struct tps65218_pwrbutton *pwr = _pwr;
+	struct tps6521x_pwrbutton *pwr = _pwr;
+	const struct tps6521x_data *tps_data = pwr->data;
 	unsigned int reg;
 	int error;
 
-	error = tps65218_reg_read(pwr->tps, TPS65218_REG_STATUS, &reg);
+	error = regmap_read(pwr->regmap, tps_data->reg_status, &reg);
 	if (error) {
 		dev_err(pwr->dev, "can't read register: %d\n", error);
 		goto out;
 	}
 
-	if (reg & TPS65218_STATUS_PB_STATE) {
+	if (reg & tps_data->pb_mask) {
 		input_report_key(pwr->idev, KEY_POWER, 1);
 		pm_wakeup_event(pwr->dev, 0);
 	} else {
@@ -55,42 +89,56 @@ out:
 	return IRQ_HANDLED;
 }
 
-static int tps65218_pwron_probe(struct platform_device *pdev)
+static int tps6521x_pb_probe(struct platform_device *pdev)
 {
-	struct tps65218 *tps = dev_get_drvdata(pdev->dev.parent);
+	void *tps = dev_get_drvdata(pdev->dev.parent);
 	struct device *dev = &pdev->dev;
-	struct tps65218_pwrbutton *pwr;
+	struct tps6521x_pwrbutton *pwr;
 	struct input_dev *idev;
+	const struct of_device_id *match;
 	int error;
 	int irq;
 
+	match = of_match_node(of_tps6521x_pb_match, pdev->dev.of_node);
+	if (!match)
+		return -ENXIO;
+
 	pwr = devm_kzalloc(dev, sizeof(*pwr), GFP_KERNEL);
 	if (!pwr)
 		return -ENOMEM;
 
+	pwr->data = match->data;
+
 	idev = devm_input_allocate_device(dev);
 	if (!idev)
 		return -ENOMEM;
 
-	idev->name = "tps65218_pwrbutton";
-	idev->phys = "tps65218_pwrbutton/input0";
+	idev->name = pwr->data->name;
+	snprintf(pwr->phys, sizeof(pwr->phys), "%s/input0",
+		pwr->data->name);
+	idev->phys = pwr->phys;
 	idev->dev.parent = dev;
 	idev->id.bustype = BUS_I2C;
 
 	input_set_capability(idev, EV_KEY, KEY_POWER);
 
-	pwr->tps = tps;
+	pwr->regmap = *(struct regmap **) (tps + pwr->data->regmap_offset);
 	pwr->dev = dev;
 	pwr->idev = idev;
 	platform_set_drvdata(pdev, pwr);
 	device_init_wakeup(dev, true);
 
 	irq = platform_get_irq(pdev, 0);
-	error = devm_request_threaded_irq(dev, irq, NULL, tps65218_pwr_irq,
-					  IRQF_TRIGGER_RISING |
-						IRQF_TRIGGER_FALLING |
-						IRQF_ONESHOT,
-					  "tps65218-pwrbutton", pwr);
+	if (irq < 0) {
+		dev_err(dev, "No IRQ resource!\n");
+		return -EINVAL;
+	}
+
+	error = devm_request_threaded_irq(dev, irq, NULL, tps6521x_pb_irq,
+					IRQF_TRIGGER_RISING |
+					IRQF_TRIGGER_FALLING |
+					IRQF_ONESHOT,
+					pwr->data->name, pwr);
 	if (error) {
 		dev_err(dev, "failed to request IRQ #%d: %d\n",
 			irq, error);
@@ -106,21 +154,15 @@ static int tps65218_pwron_probe(struct platform_device *pdev)
 	return 0;
 }
 
-static const struct of_device_id of_tps65218_pwr_match[] = {
-	{ .compatible = "ti,tps65218-pwrbutton" },
-	{ },
-};
-MODULE_DEVICE_TABLE(of, of_tps65218_pwr_match);
-
-static struct platform_driver tps65218_pwron_driver = {
-	.probe	= tps65218_pwron_probe,
+static struct platform_driver tps6521x_pb_driver = {
+	.probe	= tps6521x_pb_probe,
 	.driver	= {
-		.name	= "tps65218_pwrbutton",
-		.of_match_table = of_tps65218_pwr_match,
+		.name	= "tps6521x_pwrbutton",
+		.of_match_table = of_tps6521x_pb_match,
 	},
 };
-module_platform_driver(tps65218_pwron_driver);
+module_platform_driver(tps6521x_pb_driver);
 
-MODULE_DESCRIPTION("TPS65218 Power Button");
+MODULE_DESCRIPTION("TPS6521X Power Button");
 MODULE_LICENSE("GPL v2");
 MODULE_AUTHOR("Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>");
-- 
2.9.3

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

Re: [PATCH v5 3/3] Input: Add support for tps65217 power button

[Dmitry Torokhov]

On Thu, Sep 08, 2016 at 11:59:00AM +0200, Marcin Niestroj wrote:
> This driver enables us to use tps65217's power button as KEY_POWER on
> am335x boards (directly connected button in chiliboard, accessible pin
> via expansion header in beaglebone). This patch has been tested with
> chiliboard.
> 
> Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
> ---
> Depends on patches 1-2 in series and commit
> 47d7d5ed68d877269003a392b7008905d65650bb
> ("power_supply: tps65217-charger: Add support for IRQs")
> in power-supply's -next branch.
> 
> Changes v4 -> v5:
>  * Add support for tps65217 power button in tps65218-pwrbutton driver
>    (as suggested by Dmitry)
>  * Device-tree documentation was combined with tps65218-pwrbutton
> 
> Changes v2 -> v4: none
> 
> Changes v1 -> v2:
>  * Added information about parent device in tps65217 power button
>    device-tree binding documentation (suggested by Rob)
> 
>  .../bindings/input/tps65218-pwrbutton.txt          |  17 +++-
>  drivers/input/misc/Kconfig                         |   4 +-
>  drivers/input/misc/tps65218-pwrbutton.c            | 102 +++++++++++++++------
>  3 files changed, 88 insertions(+), 35 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/input/tps65218-pwrbutton.txt b/Documentation/devicetree/bindings/input/tps65218-pwrbutton.txt
> index e30e0b9..3e5b979 100644
> --- a/Documentation/devicetree/bindings/input/tps65218-pwrbutton.txt
> +++ b/Documentation/devicetree/bindings/input/tps65218-pwrbutton.txt
> @@ -1,13 +1,24 @@
> -Texas Instruments TPS65218 power button
> +Texas Instruments TPS65217 and TPS65218 power button
> +
> +This module is part of the TPS65217/TPS65218. For more details about the whole
> +TPS65217 chip see Documentation/devicetree/bindings/regulator/tps65217.txt.
>  
>  This driver provides a simple power button event via an Interrupt.
>  
>  Required properties:
> -- compatible: should be "ti,tps65218-pwrbutton"
> +- compatible: should be "ti,tps65217-pwrbutton" or "ti,tps65218-pwrbutton"
> +
> +Required properties for TPS65218:
>  - interrupts: should be one of the following
>     - <3 IRQ_TYPE_EDGE_BOTH>: For controllers compatible with tps65218
>  
> -Example:
> +Examples:
> +
> +&tps {
> +	tps65217-pwrbutton {
> +		compatible = "ti,tps65217-pwrbutton";
> +	};
> +};
>  
>  &tps {
>  	power-button {
> diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
> index efb0ca8..dbd5b75 100644
> --- a/drivers/input/misc/Kconfig
> +++ b/drivers/input/misc/Kconfig
> @@ -454,10 +454,10 @@ config INPUT_RETU_PWRBUTTON
>  
>  config INPUT_TPS65218_PWRBUTTON
>  	tristate "TPS65218 Power button driver"
> -	depends on MFD_TPS65218
> +	depends on (MFD_TPS65217 || MFD_TPS65218)
>  	help
>  	  Say Y here if you want to enable power buttong reporting for
> -	  the TPS65218 Power Management IC device.
> +	  TPS65217 and TPS65218 Power Management IC device.
>  
>  	  To compile this driver as a module, choose M here. The module will
>  	  be called tps65218-pwrbutton.
> diff --git a/drivers/input/misc/tps65218-pwrbutton.c b/drivers/input/misc/tps65218-pwrbutton.c
> index a39b626..b7964d1 100644
> --- a/drivers/input/misc/tps65218-pwrbutton.c
> +++ b/drivers/input/misc/tps65218-pwrbutton.c
> @@ -1,8 +1,9 @@
>  /*
> - * Texas Instruments' TPS65218 Power Button Input Driver
> + * Texas Instruments' TPS65217 and TPS65218 Power Button Input Driver
>   *
>   * Copyright (C) 2014 Texas Instruments Incorporated - http://www.ti.com/
>   * Author: Felipe Balbi <balbi@ti.com>
> + * Author: Marcin Niestroj <m.niestroj@grinn-global.com>
>   *
>   * This program is free software; you can redistribute it and/or modify
>   * it under the terms of the GNU General Public License version 2 as
> @@ -18,31 +19,64 @@
>  #include <linux/input.h>
>  #include <linux/interrupt.h>
>  #include <linux/kernel.h>
> +#include <linux/mfd/tps65217.h>
>  #include <linux/mfd/tps65218.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/platform_device.h>
> +#include <linux/regmap.h>
>  #include <linux/slab.h>
>  
> -struct tps65218_pwrbutton {
> +struct tps6521x_data {
> +	size_t regmap_offset;
> +	unsigned int reg_status;
> +	unsigned int pb_mask;
> +	const char *name;
> +};
> +
> +static const struct tps6521x_data tps65217_data = {
> +	.regmap_offset = offsetof(struct tps65217, regmap),
> +	.reg_status = TPS65217_REG_STATUS,
> +	.pb_mask = TPS65217_STATUS_PB,
> +	.name = "tps65217_pwrbutton",
> +};
> +
> +static const struct tps6521x_data tps65218_data = {
> +	.regmap_offset = offsetof(struct tps65218, regmap),
> +	.reg_status = TPS65218_REG_STATUS,
> +	.pb_mask = TPS65218_STATUS_PB_STATE,
> +	.name = "tps65218_pwrbutton",
> +};
> +
> +struct tps6521x_pwrbutton {
>  	struct device *dev;
> -	struct tps65218 *tps;
> +	struct regmap *regmap;
>  	struct input_dev *idev;
> +	const struct tps6521x_data *data;
> +	char phys[32];
> +};
> +
> +static const struct of_device_id of_tps6521x_pb_match[] = {
> +	{ .compatible = "ti,tps65217-pwrbutton", .data = &tps65217_data },
> +	{ .compatible = "ti,tps65218-pwrbutton", .data = &tps65218_data },
> +	{ },
>  };
> +MODULE_DEVICE_TABLE(of, of_tps6521x_pb_match);
>  
> -static irqreturn_t tps65218_pwr_irq(int irq, void *_pwr)
> +static irqreturn_t tps6521x_pb_irq(int irq, void *_pwr)
>  {
> -	struct tps65218_pwrbutton *pwr = _pwr;
> +	struct tps6521x_pwrbutton *pwr = _pwr;
> +	const struct tps6521x_data *tps_data = pwr->data;
>  	unsigned int reg;
>  	int error;
>  
> -	error = tps65218_reg_read(pwr->tps, TPS65218_REG_STATUS, &reg);
> +	error = regmap_read(pwr->regmap, tps_data->reg_status, &reg);
>  	if (error) {
>  		dev_err(pwr->dev, "can't read register: %d\n", error);
>  		goto out;
>  	}
>  
> -	if (reg & TPS65218_STATUS_PB_STATE) {
> +	if (reg & tps_data->pb_mask) {
>  		input_report_key(pwr->idev, KEY_POWER, 1);
>  		pm_wakeup_event(pwr->dev, 0);
>  	} else {
> @@ -55,42 +89,56 @@ out:
>  	return IRQ_HANDLED;
>  }
>  
> -static int tps65218_pwron_probe(struct platform_device *pdev)
> +static int tps6521x_pb_probe(struct platform_device *pdev)
>  {
> -	struct tps65218 *tps = dev_get_drvdata(pdev->dev.parent);
> +	void *tps = dev_get_drvdata(pdev->dev.parent);
>  	struct device *dev = &pdev->dev;
> -	struct tps65218_pwrbutton *pwr;
> +	struct tps6521x_pwrbutton *pwr;
>  	struct input_dev *idev;
> +	const struct of_device_id *match;
>  	int error;
>  	int irq;
>  
> +	match = of_match_node(of_tps6521x_pb_match, pdev->dev.of_node);
> +	if (!match)
> +		return -ENXIO;
> +
>  	pwr = devm_kzalloc(dev, sizeof(*pwr), GFP_KERNEL);
>  	if (!pwr)
>  		return -ENOMEM;
>  
> +	pwr->data = match->data;
> +
>  	idev = devm_input_allocate_device(dev);
>  	if (!idev)
>  		return -ENOMEM;
>  
> -	idev->name = "tps65218_pwrbutton";
> -	idev->phys = "tps65218_pwrbutton/input0";
> +	idev->name = pwr->data->name;
> +	snprintf(pwr->phys, sizeof(pwr->phys), "%s/input0",
> +		pwr->data->name);
> +	idev->phys = pwr->phys;
>  	idev->dev.parent = dev;
>  	idev->id.bustype = BUS_I2C;
>  
>  	input_set_capability(idev, EV_KEY, KEY_POWER);
>  
> -	pwr->tps = tps;
> +	pwr->regmap = *(struct regmap **) (tps + pwr->data->regmap_offset);

Please never ever do this stuff, it is extremely fragile and is bound to
break. Do this instead:

	regmap = dev_get_regmap(pdev->dev.parent, NULL);


>  	pwr->dev = dev;
>  	pwr->idev = idev;
>  	platform_set_drvdata(pdev, pwr);
>  	device_init_wakeup(dev, true);
>  
>  	irq = platform_get_irq(pdev, 0);
> -	error = devm_request_threaded_irq(dev, irq, NULL, tps65218_pwr_irq,
> -					  IRQF_TRIGGER_RISING |
> -						IRQF_TRIGGER_FALLING |
> -						IRQF_ONESHOT,
> -					  "tps65218-pwrbutton", pwr);
> +	if (irq < 0) {
> +		dev_err(dev, "No IRQ resource!\n");
> +		return -EINVAL;
> +	}
> +
> +	error = devm_request_threaded_irq(dev, irq, NULL, tps6521x_pb_irq,
> +					IRQF_TRIGGER_RISING |
> +					IRQF_TRIGGER_FALLING |
> +					IRQF_ONESHOT,

Prefer to keep the old formatting for flags to be able to see easily
that they are not separate arguments, but a single expression.

> +					pwr->data->name, pwr);
>  	if (error) {
>  		dev_err(dev, "failed to request IRQ #%d: %d\n",
>  			irq, error);
> @@ -106,21 +154,15 @@ static int tps65218_pwron_probe(struct platform_device *pdev)
>  	return 0;
>  }
>  
> -static const struct of_device_id of_tps65218_pwr_match[] = {
> -	{ .compatible = "ti,tps65218-pwrbutton" },
> -	{ },
> -};
> -MODULE_DEVICE_TABLE(of, of_tps65218_pwr_match);
> -
> -static struct platform_driver tps65218_pwron_driver = {
> -	.probe	= tps65218_pwron_probe,
> +static struct platform_driver tps6521x_pb_driver = {
> +	.probe	= tps6521x_pb_probe,
>  	.driver	= {
> -		.name	= "tps65218_pwrbutton",
> -		.of_match_table = of_tps65218_pwr_match,
> +		.name	= "tps6521x_pwrbutton",
> +		.of_match_table = of_tps6521x_pb_match,
>  	},
>  };
> -module_platform_driver(tps65218_pwron_driver);
> +module_platform_driver(tps6521x_pb_driver);
>  
> -MODULE_DESCRIPTION("TPS65218 Power Button");
> +MODULE_DESCRIPTION("TPS6521X Power Button");
>  MODULE_LICENSE("GPL v2");
>  MODULE_AUTHOR("Felipe Balbi <balbi@ti.com>");
> -- 
> 2.9.3
> 

Thanks.

-- 
Dmitry

Re: [PATCH v5 3/3] Input: Add support for tps65217 power button

[Tony Lindgren]

* Dmitry Torokhov <dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> [160908 10:04]:
> On Thu, Sep 08, 2016 at 11:59:00AM +0200, Marcin Niestroj wrote:
> > -	pwr->tps = tps;
> > +	pwr->regmap = *(struct regmap **) (tps + pwr->data->regmap_offset);
> 
> Please never ever do this stuff, it is extremely fragile and is bound to
> break. Do this instead:
> 
> 	regmap = dev_get_regmap(pdev->dev.parent, NULL);

I wonder if we should add dev_get_parent_regmap(dev, "desc")?

This passing of pdev->dev.parent always makes me wonder why
we're tweaking the parent resources..

Regards,

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