devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Input: Add driver for Microchip's CAP1106
@ 2014-07-11  7:42 Daniel Mack
  2014-07-11  9:43 ` David Herrmann
  2014-07-11  9:47 ` David Herrmann
  0 siblings, 2 replies; 4+ messages in thread
From: Daniel Mack @ 2014-07-11  7:42 UTC (permalink / raw)
  To: dtor; +Cc: linux-input, broonie, devicetree, Daniel Mack

This patch adds a driver for Microchips CAP1106, an I2C driven, 6-channel
capacitive touch sensor.

For now, only the capacitive buttons are supported, and no specific
settings that can be tweaked for individual channels, except for the
device-wide sensitivity gain. The defaults seem to work just fine out of
the box, so I'll leave configurable parameters for someone who's in need
of them and who can actually measure the impact. All registers are
prepared, however. Many of them are just not used for now.

The implementation does not make any attempt to be compatible to platform
data driven boards, but fully depends on CONFIG_OF.

Power management functions are also left for volounteers with the ability
to actually test them.

Signed-off-by: Daniel Mack <zonque@gmail.com>
---
 .../devicetree/bindings/input/cap1106.txt          |  63 ++++
 drivers/input/keyboard/Kconfig                     |  10 +
 drivers/input/keyboard/Makefile                    |   1 +
 drivers/input/keyboard/cap1106.c                   | 376 +++++++++++++++++++++
 4 files changed, 450 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/cap1106.txt
 create mode 100644 drivers/input/keyboard/cap1106.c

diff --git a/Documentation/devicetree/bindings/input/cap1106.txt b/Documentation/devicetree/bindings/input/cap1106.txt
new file mode 100644
index 0000000..57f5af3
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/cap1106.txt
@@ -0,0 +1,63 @@
+Device tree bindings for Microchip CAP1106, 6 channel capacitive touch sensor
+
+The node for this driver must be a child of a I2C controller node, as the
+device communication via I2C only.
+
+Required properties:
+
+	compatible:		Must be "microchip,cap1106"
+	reg:			The I2C slave address of the device.
+				Only 0x28 is valid.
+	interrupt:		Node describing the interrupt line the device's
+				ALERT#/CM_IRQ# pin is connected to.
+
+Optional properties:
+
+	autorepeat:		Enables the Linux input system's autorepeat
+				feature on the input device.
+	microchip,sensor-gain:	Defines the gain of the sensor circuitry. This
+				effectively controls the sensitivity, as a
+				smaller delta capacitance is required to
+				generate the same delta count values.
+				Valid values are 1, 2, 4, and 8.
+				By default, a gain of 1 is set.
+
+To define details of each individual button channel, six subnodes can be
+specified. Inside each of those, the following property is valid:
+
+	linux,keycode:	Specify the numeric identifier of the keycode to be
+			generated when this channel is activated. If this
+			property is omitted, KEY_A, KEY_B, etc are used as
+			defaults.
+
+Example:
+
+i2c_controller {
+	cap1106@28 {
+		compatible = "microchip,cap1106";
+		interrupt-parent = <&gpio1>;
+		interrupts = <0 0>;
+		reg = <0x28>;
+		autorepeat;
+		microchip,sensor-gain = <2>;
+
+		up {
+			linux,keycode = <103>; /* KEY_UP */
+		};
+		right {
+			linux,keycode = <106>; /* KEY_RIGHT */
+		};
+		down {
+			linux,keycode = <108>; /* KEY_DOWN */
+		};
+		left {
+			linux,keycode = <105>; /* KEY_LEFT */
+		};
+		pagedown {
+			linux,keycode = <109>; /* KEY_PAGEDOWN */
+		};
+		pageup {
+			linux,keycode = <104>; /* KEY_PAGEUP */
+		};
+	};
+}
diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index f7e79b4..a3958c6 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -665,4 +665,14 @@ config KEYBOARD_CROS_EC
 	  To compile this driver as a module, choose M here: the
 	  module will be called cros_ec_keyb.
 
+config KEYBOARD_CAP1106
+	tristate "Microchip CAP1106 touch sensor"
+	depends on OF && I2C
+	select REGMAP_I2C
+	help
+	  Say Y here to enable the CAP1106 touch sensor driver.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called cap1106.
+
 endif
diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
index 7504ae1..0a33456 100644
--- a/drivers/input/keyboard/Makefile
+++ b/drivers/input/keyboard/Makefile
@@ -11,6 +11,7 @@ obj-$(CONFIG_KEYBOARD_AMIGA)		+= amikbd.o
 obj-$(CONFIG_KEYBOARD_ATARI)		+= atakbd.o
 obj-$(CONFIG_KEYBOARD_ATKBD)		+= atkbd.o
 obj-$(CONFIG_KEYBOARD_BFIN)		+= bf54x-keys.o
+obj-$(CONFIG_KEYBOARD_CAP1106)		+= cap1106.o
 obj-$(CONFIG_KEYBOARD_CLPS711X)		+= clps711x-keypad.o
 obj-$(CONFIG_KEYBOARD_CROS_EC)		+= cros_ec_keyb.o
 obj-$(CONFIG_KEYBOARD_DAVINCI)		+= davinci_keyscan.o
diff --git a/drivers/input/keyboard/cap1106.c b/drivers/input/keyboard/cap1106.c
new file mode 100644
index 0000000..ad0f0fb
--- /dev/null
+++ b/drivers/input/keyboard/cap1106.c
@@ -0,0 +1,376 @@
+/*
+ * Input driver for Microchip CAP1106, 6 channel capacitive touch sensor
+ *
+ * http://www.microchip.com/wwwproducts/Devices.aspx?product=CAP1106
+ *
+ * (c) 2014 Daniel Mack <linux@zonque.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
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/interrupt.h>
+#include <linux/input.h>
+#include <linux/of_irq.h>
+#include <linux/regmap.h>
+#include <linux/i2c.h>
+#include <linux/gpio/consumer.h>
+
+#define CAP1106_REG_MAIN_CONTROL	0x00
+#define CAP1106_REG_MAIN_CONTROL_GAIN_SHIFT	(6)
+#define CAP1106_REG_MAIN_CONTROL_GAIN_MASK	(0xc0)
+#define CAP1106_REG_MAIN_CONTROL_DLSEEP		BIT(4)
+#define CAP1106_REG_GENERAL_STATUS	0x02
+#define CAP1106_REG_SENSOR_INPUT	0x03
+#define CAP1106_REG_NOISE_FLAG_STATUS	0x0a
+#define CAP1106_REG_SENOR_DELTA(X)	(0x10 + (X))
+#define CAP1106_REG_SENSITIVITY_CONTROL	0x1f
+#define CAP1106_REG_CONFIG		0x20
+#define CAP1106_REG_SENSOR_ENABLE	0x21
+#define CAP1106_REG_SENSOR_CONFIG	0x22
+#define CAP1106_REG_SENSOR_CONFIG2	0x23
+#define CAP1106_REG_SAMPLING_CONFIG	0x24
+#define CAP1106_REG_CALIBRATION		0x25
+#define CAP1106_REG_INT_ENABLE		0x26
+#define CAP1106_REG_REPEAT_RATE		0x28
+#define CAP1106_REG_MT_CONFIG		0x2a
+#define CAP1106_REG_MT_PATTERN_CONFIG	0x2b
+#define CAP1106_REG_MT_PATTERN		0x2d
+#define CAP1106_REG_RECALIB_CONFIG	0x2f
+#define CAP1106_REG_SENSOR_THRESH(X)	(0x30 + (X))
+#define CAP1106_REG_SENSOR_NOISE_THRESH	0x38
+#define CAP1106_REG_STANDBY_CHANNEL	0x40
+#define CAP1106_REG_STANDBY_CONFIG	0x41
+#define CAP1106_REG_STANDBY_SENSITIVITY	0x42
+#define CAP1106_REG_STANDBY_THRESH	0x43
+#define CAP1106_REG_CONFIG2		0x44
+#define CAP1106_REG_SENSOR_BASE_CNT(X)	(0x50 + (X))
+#define CAP1106_REG_SENSOR_CALIB	(0xb1 + (X))
+#define CAP1106_REG_SENSOR_CALIB_LSB1	0xb9
+#define CAP1106_REG_SENSOR_CALIB_LSB2	0xba
+#define CAP1106_REG_PRODUCT_ID		0xfd
+#define CAP1106_REG_MANUFACTURER_ID	0xfe
+#define CAP1106_REG_REVISION		0xff
+
+#define CAP1106_NUM_CHN 6
+#define CAP1106_PRODUCT_ID	0x55
+#define CAP1106_MANUFACTURER_ID	0x5d
+
+struct cap1106_priv {
+	struct regmap *regmap;
+	struct input_dev *idev;
+	struct work_struct work;
+	spinlock_t lock;
+	int irq;
+
+	/* config */
+	unsigned int keycodes[CAP1106_NUM_CHN];
+};
+
+static const struct reg_default cap1106_reg_defaults[] = {
+	{ CAP1106_REG_MAIN_CONTROL,		0x00 },
+	{ CAP1106_REG_GENERAL_STATUS,		0x00 },
+	{ CAP1106_REG_SENSOR_INPUT,		0x00 },
+	{ CAP1106_REG_NOISE_FLAG_STATUS,	0x00 },
+	{ CAP1106_REG_SENSITIVITY_CONTROL,	0x2f },
+	{ CAP1106_REG_CONFIG,			0x20 },
+	{ CAP1106_REG_SENSOR_ENABLE,		0x3f },
+	{ CAP1106_REG_SENSOR_CONFIG,		0xa4 },
+	{ CAP1106_REG_SENSOR_CONFIG2,		0x07 },
+	{ CAP1106_REG_SAMPLING_CONFIG,		0x39 },
+	{ CAP1106_REG_CALIBRATION,		0x00 },
+	{ CAP1106_REG_INT_ENABLE,		0x3f },
+	{ CAP1106_REG_REPEAT_RATE,		0x3f },
+	{ CAP1106_REG_MT_CONFIG,		0x80 },
+	{ CAP1106_REG_MT_PATTERN_CONFIG,	0x00 },
+	{ CAP1106_REG_MT_PATTERN,		0x3f },
+	{ CAP1106_REG_RECALIB_CONFIG,		0x8a },
+	{ CAP1106_REG_SENSOR_THRESH(0),		0x40 },
+	{ CAP1106_REG_SENSOR_THRESH(1),		0x40 },
+	{ CAP1106_REG_SENSOR_THRESH(2),		0x40 },
+	{ CAP1106_REG_SENSOR_THRESH(3),		0x40 },
+	{ CAP1106_REG_SENSOR_THRESH(4),		0x40 },
+	{ CAP1106_REG_SENSOR_THRESH(5),		0x40 },
+	{ CAP1106_REG_SENSOR_NOISE_THRESH,	0x01 },
+	{ CAP1106_REG_STANDBY_CHANNEL,		0x00 },
+	{ CAP1106_REG_STANDBY_CONFIG,		0x39 },
+	{ CAP1106_REG_STANDBY_SENSITIVITY,	0x02 },
+	{ CAP1106_REG_STANDBY_THRESH,		0x40 },
+	{ CAP1106_REG_CONFIG2,			0x40 },
+	{ CAP1106_REG_SENSOR_CALIB_LSB1,	0x00 },
+	{ CAP1106_REG_SENSOR_CALIB_LSB2,	0x00 },
+};
+
+static bool cap1106_volatile_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case CAP1106_REG_MAIN_CONTROL:
+	case CAP1106_REG_SENSOR_INPUT:
+	case CAP1106_REG_SENOR_DELTA(0):
+	case CAP1106_REG_SENOR_DELTA(1):
+	case CAP1106_REG_SENOR_DELTA(2):
+	case CAP1106_REG_SENOR_DELTA(3):
+	case CAP1106_REG_SENOR_DELTA(4):
+	case CAP1106_REG_SENOR_DELTA(5):
+	case CAP1106_REG_PRODUCT_ID:
+	case CAP1106_REG_MANUFACTURER_ID:
+	case CAP1106_REG_REVISION:
+		return true;
+	}
+
+	return false;
+}
+
+static const struct regmap_config cap1106_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+
+	.max_register = CAP1106_REG_REVISION,
+	.reg_defaults = cap1106_reg_defaults,
+
+	.num_reg_defaults = ARRAY_SIZE(cap1106_reg_defaults),
+	.cache_type = REGCACHE_RBTREE,
+	.volatile_reg = cap1106_volatile_reg,
+};
+
+static void cap1106_work(struct work_struct *w)
+{
+	struct cap1106_priv *priv = container_of(w, struct cap1106_priv, work);
+	unsigned int status;
+	int ret, i;
+
+	spin_lock(&priv->lock);
+
+	/*
+	 * Deassert interrupt. This needs to be done before reading the status
+	 * registers, which will not carry valid values otherwise.
+	 */
+	ret = regmap_update_bits(priv->regmap, CAP1106_REG_MAIN_CONTROL, 1, 0);
+	if (ret < 0)
+		goto out;
+
+	ret = regmap_read(priv->regmap, CAP1106_REG_SENSOR_INPUT, &status);
+	if (ret < 0)
+		goto out;
+
+	for (i = 0; i < CAP1106_NUM_CHN; i++)
+		input_report_key(priv->idev, priv->keycodes[i],
+				 status & (1 << i));
+
+	input_sync(priv->idev);
+
+out:
+	enable_irq(priv->irq);
+	spin_unlock(&priv->lock);
+}
+
+static irqreturn_t cap1106_isr(int irq_num, void *data)
+{
+	struct cap1106_priv *priv = data;
+
+	spin_lock(&priv->lock);
+	disable_irq_nosync(priv->irq);
+	schedule_work(&priv->work);
+	spin_unlock(&priv->lock);
+
+	return IRQ_HANDLED;
+}
+
+static int cap1106_open(struct input_dev *input_dev)
+{
+	struct cap1106_priv *priv = input_get_drvdata(input_dev);
+	int ret;
+
+	/* disable deep sleep */
+	ret = regmap_update_bits(priv->regmap, CAP1106_REG_MAIN_CONTROL,
+				 CAP1106_REG_MAIN_CONTROL_DLSEEP, 0);
+
+	enable_irq(priv->irq);
+
+	return ret;
+}
+
+static void cap1106_close(struct input_dev *input_dev)
+{
+	struct cap1106_priv *priv = input_get_drvdata(input_dev);
+	unsigned long flags;
+
+	spin_lock_irqsave(&priv->lock, flags);
+	disable_irq(priv->irq);
+	cancel_work_sync(&priv->work);
+	spin_unlock_irqrestore(&priv->lock, flags);
+
+	/* enable deep sleep */
+	regmap_update_bits(priv->regmap, CAP1106_REG_MAIN_CONTROL,
+			   CAP1106_REG_MAIN_CONTROL_DLSEEP,
+			   CAP1106_REG_MAIN_CONTROL_DLSEEP);
+}
+
+static int cap1106_i2c_probe(struct i2c_client *i2c_client,
+			     const struct i2c_device_id *id)
+{
+	struct device *dev = &i2c_client->dev;
+	struct device_node *child, *node;
+	struct cap1106_priv *priv;
+	unsigned int val, rev;
+	u8 gain = 0;
+	int i, ret;
+	u32 tmp32;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->regmap = devm_regmap_init_i2c(i2c_client, &cap1106_regmap_config);
+	if (IS_ERR(priv->regmap))
+		return PTR_ERR(priv->regmap);
+
+	ret = regmap_read(priv->regmap, CAP1106_REG_PRODUCT_ID, &val);
+	if (ret < 0)
+		return ret;
+
+	if (val != CAP1106_PRODUCT_ID) {
+		dev_err(dev, "Product ID: Got 0x%02x, expected 0x%02x\n",
+			val, CAP1106_PRODUCT_ID);
+		return -ENODEV;
+	}
+
+	ret = regmap_read(priv->regmap, CAP1106_REG_MANUFACTURER_ID, &val);
+	if (ret < 0)
+		return ret;
+
+	if (val != CAP1106_MANUFACTURER_ID) {
+		dev_err(dev, "Manufacturer ID: Got 0x%02x, expected 0x%02x\n",
+			val, CAP1106_MANUFACTURER_ID);
+		return -ENODEV;
+	}
+
+	ret = regmap_read(priv->regmap, CAP1106_REG_REVISION, &rev);
+	if (ret < 0)
+		return ret;
+
+	dev_info(dev, "CAP1106 detected, revision 0x%02x\n", rev);
+	i2c_set_clientdata(i2c_client, priv);
+	INIT_WORK(&priv->work, cap1106_work);
+	spin_lock_init(&priv->lock);
+	node = dev->of_node;
+
+	if (!of_property_read_u32(node, "microchip,sensor-gain", &tmp32)) {
+		if (is_power_of_2(tmp32) && tmp32 <= 8)
+			gain = ilog2(tmp32);
+		else
+			dev_err(dev, "Invalid sensor-gain value %d\n", tmp32);
+	}
+
+	ret = regmap_update_bits(priv->regmap, CAP1106_REG_MAIN_CONTROL,
+				 CAP1106_REG_MAIN_CONTROL_GAIN_MASK,
+				 gain << CAP1106_REG_MAIN_CONTROL_GAIN_SHIFT);
+	if (ret < 0)
+		return ret;
+
+	/* disable autorepeat. The Linux input system has its own handling. */
+	ret = regmap_write(priv->regmap, CAP1106_REG_REPEAT_RATE, 0);
+	if (ret < 0)
+		return ret;
+
+	/* set the defaults */
+	for (i = 0; i < CAP1106_NUM_CHN; i++)
+		priv->keycodes[i] = KEY_A + i;
+
+	i = 0;
+	for_each_child_of_node(node, child) {
+		if (i == CAP1106_NUM_CHN) {
+			dev_err(dev, "Too many button nodes.\n");
+			return -EINVAL;
+		}
+
+		if (!of_property_read_u32(child, "linux,keycode", &tmp32))
+			priv->keycodes[i] = tmp32;
+
+		i++;
+	}
+
+	priv->idev = devm_input_allocate_device(dev);
+	if (!priv->idev)
+		return -ENOMEM;
+
+	priv->idev->name = "CAP1106 capacitive touch sensor";
+	priv->idev->id.bustype = BUS_I2C;
+	priv->idev->evbit[0] = BIT_MASK(EV_KEY);
+
+	if (of_get_property(node, "autorepeat", NULL))
+		__set_bit(EV_REP, priv->idev->evbit);
+
+	for (i = 0; i < CAP1106_NUM_CHN; i++) {
+		unsigned int code = priv->keycodes[i];
+		priv->idev->keybit[BIT_WORD(code)] |= BIT_MASK(code);
+	}
+
+	priv->idev->id.vendor = CAP1106_MANUFACTURER_ID;
+	priv->idev->id.product = CAP1106_PRODUCT_ID;
+	priv->idev->id.version = rev;
+
+	priv->idev->open = cap1106_open;
+	priv->idev->close = cap1106_close;
+
+	input_set_drvdata(priv->idev, priv);
+
+	ret = input_register_device(priv->idev);
+	if (ret < 0)
+		return ret;
+
+	priv->irq = irq_of_parse_and_map(node, 0);
+	if (!priv->irq) {
+		dev_err(dev, "Unable to parse or map IRQ\n");
+		return -ENXIO;
+	}
+
+	ret = devm_request_irq(dev, priv->irq, cap1106_isr, IRQF_DISABLED,
+			       dev_name(dev), priv);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+static int cap1106_i2c_remove(struct i2c_client *i2c_client)
+{
+	struct cap1106_priv *priv = i2c_get_clientdata(i2c_client);
+
+	input_unregister_device(priv->idev);
+
+	return 0;
+}
+
+static const struct of_device_id cap1106_dt_ids[] = {
+	{ .compatible = "microchip,cap1106", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, cap1106_dt_ids);
+
+static const struct i2c_device_id cap1106_i2c_ids[] = {
+	{ "cap1106", 0 },
+	{}
+};
+MODULE_DEVICE_TABLE(i2c, cap1106_i2c_ids);
+
+static struct i2c_driver cap1106_i2c_driver = {
+	.driver = {
+		.name	= "cap1106",
+		.owner	= THIS_MODULE,
+		.of_match_table = cap1106_dt_ids,
+	},
+	.id_table	= cap1106_i2c_ids,
+	.probe		= cap1106_i2c_probe,
+	.remove		= cap1106_i2c_remove,
+};
+
+module_i2c_driver(cap1106_i2c_driver);
+
+MODULE_ALIAS("platform:" DRV_NAME);
+MODULE_DESCRIPTION("Microchip CAP1106 driver");
+MODULE_AUTHOR("Daniel Mack <linux@zonque.org>");
+MODULE_LICENSE("GPL v2");
-- 
1.9.3


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

* Re: [PATCH] Input: Add driver for Microchip's CAP1106
  2014-07-11  7:42 [PATCH] Input: Add driver for Microchip's CAP1106 Daniel Mack
@ 2014-07-11  9:43 ` David Herrmann
       [not found]   ` <53BFB300.1040500@zonque.org>
  2014-07-11  9:47 ` David Herrmann
  1 sibling, 1 reply; 4+ messages in thread
From: David Herrmann @ 2014-07-11  9:43 UTC (permalink / raw)
  To: Daniel Mack; +Cc: dtor, open list:HID CORE LAYER, Mark Brown, devicetree

Hi

On Fri, Jul 11, 2014 at 9:42 AM, Daniel Mack <zonque@gmail.com> wrote:
> This patch adds a driver for Microchips CAP1106, an I2C driven, 6-channel
> capacitive touch sensor.
>
> For now, only the capacitive buttons are supported, and no specific
> settings that can be tweaked for individual channels, except for the
> device-wide sensitivity gain. The defaults seem to work just fine out of
> the box, so I'll leave configurable parameters for someone who's in need
> of them and who can actually measure the impact. All registers are
> prepared, however. Many of them are just not used for now.
>
> The implementation does not make any attempt to be compatible to platform
> data driven boards, but fully depends on CONFIG_OF.
>
> Power management functions are also left for volounteers with the ability
> to actually test them.
>
> Signed-off-by: Daniel Mack <zonque@gmail.com>
> ---
>  .../devicetree/bindings/input/cap1106.txt          |  63 ++++
>  drivers/input/keyboard/Kconfig                     |  10 +
>  drivers/input/keyboard/Makefile                    |   1 +
>  drivers/input/keyboard/cap1106.c                   | 376 +++++++++++++++++++++
>  4 files changed, 450 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/input/cap1106.txt
>  create mode 100644 drivers/input/keyboard/cap1106.c
>
> diff --git a/Documentation/devicetree/bindings/input/cap1106.txt b/Documentation/devicetree/bindings/input/cap1106.txt
> new file mode 100644
> index 0000000..57f5af3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/cap1106.txt
> @@ -0,0 +1,63 @@
> +Device tree bindings for Microchip CAP1106, 6 channel capacitive touch sensor
> +
> +The node for this driver must be a child of a I2C controller node, as the
> +device communication via I2C only.
> +
> +Required properties:
> +
> +       compatible:             Must be "microchip,cap1106"
> +       reg:                    The I2C slave address of the device.
> +                               Only 0x28 is valid.
> +       interrupt:              Node describing the interrupt line the device's
> +                               ALERT#/CM_IRQ# pin is connected to.
> +
> +Optional properties:
> +
> +       autorepeat:             Enables the Linux input system's autorepeat
> +                               feature on the input device.
> +       microchip,sensor-gain:  Defines the gain of the sensor circuitry. This
> +                               effectively controls the sensitivity, as a
> +                               smaller delta capacitance is required to
> +                               generate the same delta count values.
> +                               Valid values are 1, 2, 4, and 8.
> +                               By default, a gain of 1 is set.
> +
> +To define details of each individual button channel, six subnodes can be
> +specified. Inside each of those, the following property is valid:
> +
> +       linux,keycode:  Specify the numeric identifier of the keycode to be
> +                       generated when this channel is activated. If this
> +                       property is omitted, KEY_A, KEY_B, etc are used as
> +                       defaults.
> +
> +Example:
> +
> +i2c_controller {
> +       cap1106@28 {
> +               compatible = "microchip,cap1106";
> +               interrupt-parent = <&gpio1>;
> +               interrupts = <0 0>;
> +               reg = <0x28>;
> +               autorepeat;
> +               microchip,sensor-gain = <2>;
> +
> +               up {
> +                       linux,keycode = <103>; /* KEY_UP */
> +               };
> +               right {
> +                       linux,keycode = <106>; /* KEY_RIGHT */
> +               };
> +               down {
> +                       linux,keycode = <108>; /* KEY_DOWN */
> +               };
> +               left {
> +                       linux,keycode = <105>; /* KEY_LEFT */
> +               };
> +               pagedown {
> +                       linux,keycode = <109>; /* KEY_PAGEDOWN */
> +               };
> +               pageup {
> +                       linux,keycode = <104>; /* KEY_PAGEUP */
> +               };
> +       };
> +}
> diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
> index f7e79b4..a3958c6 100644
> --- a/drivers/input/keyboard/Kconfig
> +++ b/drivers/input/keyboard/Kconfig
> @@ -665,4 +665,14 @@ config KEYBOARD_CROS_EC
>           To compile this driver as a module, choose M here: the
>           module will be called cros_ec_keyb.
>
> +config KEYBOARD_CAP1106
> +       tristate "Microchip CAP1106 touch sensor"
> +       depends on OF && I2C
> +       select REGMAP_I2C
> +       help
> +         Say Y here to enable the CAP1106 touch sensor driver.
> +
> +         To compile this driver as a module, choose M here: the
> +         module will be called cap1106.
> +
>  endif
> diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
> index 7504ae1..0a33456 100644
> --- a/drivers/input/keyboard/Makefile
> +++ b/drivers/input/keyboard/Makefile
> @@ -11,6 +11,7 @@ obj-$(CONFIG_KEYBOARD_AMIGA)          += amikbd.o
>  obj-$(CONFIG_KEYBOARD_ATARI)           += atakbd.o
>  obj-$(CONFIG_KEYBOARD_ATKBD)           += atkbd.o
>  obj-$(CONFIG_KEYBOARD_BFIN)            += bf54x-keys.o
> +obj-$(CONFIG_KEYBOARD_CAP1106)         += cap1106.o
>  obj-$(CONFIG_KEYBOARD_CLPS711X)                += clps711x-keypad.o
>  obj-$(CONFIG_KEYBOARD_CROS_EC)         += cros_ec_keyb.o
>  obj-$(CONFIG_KEYBOARD_DAVINCI)         += davinci_keyscan.o
> diff --git a/drivers/input/keyboard/cap1106.c b/drivers/input/keyboard/cap1106.c
> new file mode 100644
> index 0000000..ad0f0fb
> --- /dev/null
> +++ b/drivers/input/keyboard/cap1106.c
> @@ -0,0 +1,376 @@
> +/*
> + * Input driver for Microchip CAP1106, 6 channel capacitive touch sensor
> + *
> + * http://www.microchip.com/wwwproducts/Devices.aspx?product=CAP1106
> + *
> + * (c) 2014 Daniel Mack <linux@zonque.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
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/interrupt.h>
> +#include <linux/input.h>
> +#include <linux/of_irq.h>
> +#include <linux/regmap.h>
> +#include <linux/i2c.h>
> +#include <linux/gpio/consumer.h>
> +
> +#define CAP1106_REG_MAIN_CONTROL       0x00
> +#define CAP1106_REG_MAIN_CONTROL_GAIN_SHIFT    (6)
> +#define CAP1106_REG_MAIN_CONTROL_GAIN_MASK     (0xc0)
> +#define CAP1106_REG_MAIN_CONTROL_DLSEEP                BIT(4)
> +#define CAP1106_REG_GENERAL_STATUS     0x02
> +#define CAP1106_REG_SENSOR_INPUT       0x03
> +#define CAP1106_REG_NOISE_FLAG_STATUS  0x0a
> +#define CAP1106_REG_SENOR_DELTA(X)     (0x10 + (X))
> +#define CAP1106_REG_SENSITIVITY_CONTROL        0x1f
> +#define CAP1106_REG_CONFIG             0x20
> +#define CAP1106_REG_SENSOR_ENABLE      0x21
> +#define CAP1106_REG_SENSOR_CONFIG      0x22
> +#define CAP1106_REG_SENSOR_CONFIG2     0x23
> +#define CAP1106_REG_SAMPLING_CONFIG    0x24
> +#define CAP1106_REG_CALIBRATION                0x25
> +#define CAP1106_REG_INT_ENABLE         0x26
> +#define CAP1106_REG_REPEAT_RATE                0x28
> +#define CAP1106_REG_MT_CONFIG          0x2a
> +#define CAP1106_REG_MT_PATTERN_CONFIG  0x2b
> +#define CAP1106_REG_MT_PATTERN         0x2d
> +#define CAP1106_REG_RECALIB_CONFIG     0x2f
> +#define CAP1106_REG_SENSOR_THRESH(X)   (0x30 + (X))
> +#define CAP1106_REG_SENSOR_NOISE_THRESH        0x38
> +#define CAP1106_REG_STANDBY_CHANNEL    0x40
> +#define CAP1106_REG_STANDBY_CONFIG     0x41
> +#define CAP1106_REG_STANDBY_SENSITIVITY        0x42
> +#define CAP1106_REG_STANDBY_THRESH     0x43
> +#define CAP1106_REG_CONFIG2            0x44
> +#define CAP1106_REG_SENSOR_BASE_CNT(X) (0x50 + (X))
> +#define CAP1106_REG_SENSOR_CALIB       (0xb1 + (X))
> +#define CAP1106_REG_SENSOR_CALIB_LSB1  0xb9
> +#define CAP1106_REG_SENSOR_CALIB_LSB2  0xba
> +#define CAP1106_REG_PRODUCT_ID         0xfd
> +#define CAP1106_REG_MANUFACTURER_ID    0xfe
> +#define CAP1106_REG_REVISION           0xff
> +
> +#define CAP1106_NUM_CHN 6
> +#define CAP1106_PRODUCT_ID     0x55
> +#define CAP1106_MANUFACTURER_ID        0x5d
> +
> +struct cap1106_priv {
> +       struct regmap *regmap;
> +       struct input_dev *idev;
> +       struct work_struct work;
> +       spinlock_t lock;
> +       int irq;
> +
> +       /* config */
> +       unsigned int keycodes[CAP1106_NUM_CHN];
> +};
> +
> +static const struct reg_default cap1106_reg_defaults[] = {
> +       { CAP1106_REG_MAIN_CONTROL,             0x00 },
> +       { CAP1106_REG_GENERAL_STATUS,           0x00 },
> +       { CAP1106_REG_SENSOR_INPUT,             0x00 },
> +       { CAP1106_REG_NOISE_FLAG_STATUS,        0x00 },
> +       { CAP1106_REG_SENSITIVITY_CONTROL,      0x2f },
> +       { CAP1106_REG_CONFIG,                   0x20 },
> +       { CAP1106_REG_SENSOR_ENABLE,            0x3f },
> +       { CAP1106_REG_SENSOR_CONFIG,            0xa4 },
> +       { CAP1106_REG_SENSOR_CONFIG2,           0x07 },
> +       { CAP1106_REG_SAMPLING_CONFIG,          0x39 },
> +       { CAP1106_REG_CALIBRATION,              0x00 },
> +       { CAP1106_REG_INT_ENABLE,               0x3f },
> +       { CAP1106_REG_REPEAT_RATE,              0x3f },
> +       { CAP1106_REG_MT_CONFIG,                0x80 },
> +       { CAP1106_REG_MT_PATTERN_CONFIG,        0x00 },
> +       { CAP1106_REG_MT_PATTERN,               0x3f },
> +       { CAP1106_REG_RECALIB_CONFIG,           0x8a },
> +       { CAP1106_REG_SENSOR_THRESH(0),         0x40 },
> +       { CAP1106_REG_SENSOR_THRESH(1),         0x40 },
> +       { CAP1106_REG_SENSOR_THRESH(2),         0x40 },
> +       { CAP1106_REG_SENSOR_THRESH(3),         0x40 },
> +       { CAP1106_REG_SENSOR_THRESH(4),         0x40 },
> +       { CAP1106_REG_SENSOR_THRESH(5),         0x40 },
> +       { CAP1106_REG_SENSOR_NOISE_THRESH,      0x01 },
> +       { CAP1106_REG_STANDBY_CHANNEL,          0x00 },
> +       { CAP1106_REG_STANDBY_CONFIG,           0x39 },
> +       { CAP1106_REG_STANDBY_SENSITIVITY,      0x02 },
> +       { CAP1106_REG_STANDBY_THRESH,           0x40 },
> +       { CAP1106_REG_CONFIG2,                  0x40 },
> +       { CAP1106_REG_SENSOR_CALIB_LSB1,        0x00 },
> +       { CAP1106_REG_SENSOR_CALIB_LSB2,        0x00 },
> +};
> +
> +static bool cap1106_volatile_reg(struct device *dev, unsigned int reg)
> +{
> +       switch (reg) {
> +       case CAP1106_REG_MAIN_CONTROL:
> +       case CAP1106_REG_SENSOR_INPUT:
> +       case CAP1106_REG_SENOR_DELTA(0):
> +       case CAP1106_REG_SENOR_DELTA(1):
> +       case CAP1106_REG_SENOR_DELTA(2):
> +       case CAP1106_REG_SENOR_DELTA(3):
> +       case CAP1106_REG_SENOR_DELTA(4):
> +       case CAP1106_REG_SENOR_DELTA(5):
> +       case CAP1106_REG_PRODUCT_ID:
> +       case CAP1106_REG_MANUFACTURER_ID:
> +       case CAP1106_REG_REVISION:
> +               return true;
> +       }
> +
> +       return false;
> +}
> +
> +static const struct regmap_config cap1106_regmap_config = {
> +       .reg_bits = 8,
> +       .val_bits = 8,
> +
> +       .max_register = CAP1106_REG_REVISION,
> +       .reg_defaults = cap1106_reg_defaults,
> +
> +       .num_reg_defaults = ARRAY_SIZE(cap1106_reg_defaults),
> +       .cache_type = REGCACHE_RBTREE,
> +       .volatile_reg = cap1106_volatile_reg,
> +};
> +
> +static void cap1106_work(struct work_struct *w)
> +{
> +       struct cap1106_priv *priv = container_of(w, struct cap1106_priv, work);
> +       unsigned int status;
> +       int ret, i;
> +
> +       spin_lock(&priv->lock);

If your worker started and is _about_ to take the spinlock, you might
dead-lock with cap1106_close(). They might already hold the spinlock
and then call cancel_work_sync(), thus waiting for this task to take
the lock end exit..

How about you add "bool closed : 1;" to your cap1106_priv structure
and drop the lock here, but..

> +
> +       /*
> +        * Deassert interrupt. This needs to be done before reading the status
> +        * registers, which will not carry valid values otherwise.
> +        */
> +       ret = regmap_update_bits(priv->regmap, CAP1106_REG_MAIN_CONTROL, 1, 0);
> +       if (ret < 0)
> +               goto out;
> +
> +       ret = regmap_read(priv->regmap, CAP1106_REG_SENSOR_INPUT, &status);
> +       if (ret < 0)
> +               goto out;
> +
> +       for (i = 0; i < CAP1106_NUM_CHN; i++)
> +               input_report_key(priv->idev, priv->keycodes[i],
> +                                status & (1 << i));
> +
> +       input_sync(priv->idev);
> +
> +out:
> +       enable_irq(priv->irq);
> +       spin_unlock(&priv->lock);

...change this to:

spin_lock(&priv->lock);
if (!priv->closed)
        enable_irq(priv->irq);
spin_unlock(&priv->lock);

> +}
> +
> +static irqreturn_t cap1106_isr(int irq_num, void *data)
> +{
> +       struct cap1106_priv *priv = data;
> +
> +       spin_lock(&priv->lock);
> +       disable_irq_nosync(priv->irq);
> +       schedule_work(&priv->work);
> +       spin_unlock(&priv->lock);

Locks here can be simply dropped. The IRQ is always stopped
synchronously before any worker is stopped, so this cannot race.

> +
> +       return IRQ_HANDLED;
> +}
> +
> +static int cap1106_open(struct input_dev *input_dev)
> +{
> +       struct cap1106_priv *priv = input_get_drvdata(input_dev);
> +       int ret;
> +
> +       /* disable deep sleep */
> +       ret = regmap_update_bits(priv->regmap, CAP1106_REG_MAIN_CONTROL,
> +                                CAP1106_REG_MAIN_CONTROL_DLSEEP, 0);
> +

You'd have to add:
priv->closed = false;

No need for locking as enable_irq and work-structs work as barriers.

> +       enable_irq(priv->irq);
> +
> +       return ret;
> +}
> +
> +static void cap1106_close(struct input_dev *input_dev)
> +{
> +       struct cap1106_priv *priv = input_get_drvdata(input_dev);
> +       unsigned long flags;
> +
> +       spin_lock_irqsave(&priv->lock, flags);
> +       disable_irq(priv->irq);
> +       cancel_work_sync(&priv->work);
> +       spin_unlock_irqrestore(&priv->lock, flags);

This would become:

spin_lock(&priv->lock);
priv->closed = true;
disable_irq(priv->irq);
spin_unlock(&priv->lock);

cancel_work_sync(&priv->work);

Btw., no need for irqsave/irqrestore here, ->closed is called in user-context.
Everything else looks good to me:

Thanks
David

> +
> +       /* enable deep sleep */
> +       regmap_update_bits(priv->regmap, CAP1106_REG_MAIN_CONTROL,
> +                          CAP1106_REG_MAIN_CONTROL_DLSEEP,
> +                          CAP1106_REG_MAIN_CONTROL_DLSEEP);
> +}
> +
> +static int cap1106_i2c_probe(struct i2c_client *i2c_client,
> +                            const struct i2c_device_id *id)
> +{
> +       struct device *dev = &i2c_client->dev;
> +       struct device_node *child, *node;
> +       struct cap1106_priv *priv;
> +       unsigned int val, rev;
> +       u8 gain = 0;
> +       int i, ret;
> +       u32 tmp32;
> +
> +       priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +       if (!priv)
> +               return -ENOMEM;
> +
> +       priv->regmap = devm_regmap_init_i2c(i2c_client, &cap1106_regmap_config);
> +       if (IS_ERR(priv->regmap))
> +               return PTR_ERR(priv->regmap);
> +
> +       ret = regmap_read(priv->regmap, CAP1106_REG_PRODUCT_ID, &val);
> +       if (ret < 0)
> +               return ret;
> +
> +       if (val != CAP1106_PRODUCT_ID) {
> +               dev_err(dev, "Product ID: Got 0x%02x, expected 0x%02x\n",
> +                       val, CAP1106_PRODUCT_ID);
> +               return -ENODEV;
> +       }
> +
> +       ret = regmap_read(priv->regmap, CAP1106_REG_MANUFACTURER_ID, &val);
> +       if (ret < 0)
> +               return ret;
> +
> +       if (val != CAP1106_MANUFACTURER_ID) {
> +               dev_err(dev, "Manufacturer ID: Got 0x%02x, expected 0x%02x\n",
> +                       val, CAP1106_MANUFACTURER_ID);
> +               return -ENODEV;
> +       }
> +
> +       ret = regmap_read(priv->regmap, CAP1106_REG_REVISION, &rev);
> +       if (ret < 0)
> +               return ret;
> +
> +       dev_info(dev, "CAP1106 detected, revision 0x%02x\n", rev);
> +       i2c_set_clientdata(i2c_client, priv);
> +       INIT_WORK(&priv->work, cap1106_work);
> +       spin_lock_init(&priv->lock);
> +       node = dev->of_node;
> +
> +       if (!of_property_read_u32(node, "microchip,sensor-gain", &tmp32)) {
> +               if (is_power_of_2(tmp32) && tmp32 <= 8)
> +                       gain = ilog2(tmp32);
> +               else
> +                       dev_err(dev, "Invalid sensor-gain value %d\n", tmp32);
> +       }
> +
> +       ret = regmap_update_bits(priv->regmap, CAP1106_REG_MAIN_CONTROL,
> +                                CAP1106_REG_MAIN_CONTROL_GAIN_MASK,
> +                                gain << CAP1106_REG_MAIN_CONTROL_GAIN_SHIFT);
> +       if (ret < 0)
> +               return ret;
> +
> +       /* disable autorepeat. The Linux input system has its own handling. */
> +       ret = regmap_write(priv->regmap, CAP1106_REG_REPEAT_RATE, 0);
> +       if (ret < 0)
> +               return ret;
> +
> +       /* set the defaults */
> +       for (i = 0; i < CAP1106_NUM_CHN; i++)
> +               priv->keycodes[i] = KEY_A + i;
> +
> +       i = 0;
> +       for_each_child_of_node(node, child) {
> +               if (i == CAP1106_NUM_CHN) {
> +                       dev_err(dev, "Too many button nodes.\n");
> +                       return -EINVAL;
> +               }
> +
> +               if (!of_property_read_u32(child, "linux,keycode", &tmp32))
> +                       priv->keycodes[i] = tmp32;
> +
> +               i++;
> +       }
> +
> +       priv->idev = devm_input_allocate_device(dev);
> +       if (!priv->idev)
> +               return -ENOMEM;
> +
> +       priv->idev->name = "CAP1106 capacitive touch sensor";
> +       priv->idev->id.bustype = BUS_I2C;
> +       priv->idev->evbit[0] = BIT_MASK(EV_KEY);
> +
> +       if (of_get_property(node, "autorepeat", NULL))
> +               __set_bit(EV_REP, priv->idev->evbit);
> +
> +       for (i = 0; i < CAP1106_NUM_CHN; i++) {
> +               unsigned int code = priv->keycodes[i];
> +               priv->idev->keybit[BIT_WORD(code)] |= BIT_MASK(code);
> +       }
> +
> +       priv->idev->id.vendor = CAP1106_MANUFACTURER_ID;
> +       priv->idev->id.product = CAP1106_PRODUCT_ID;
> +       priv->idev->id.version = rev;
> +
> +       priv->idev->open = cap1106_open;
> +       priv->idev->close = cap1106_close;
> +
> +       input_set_drvdata(priv->idev, priv);
> +
> +       ret = input_register_device(priv->idev);
> +       if (ret < 0)
> +               return ret;
> +
> +       priv->irq = irq_of_parse_and_map(node, 0);
> +       if (!priv->irq) {
> +               dev_err(dev, "Unable to parse or map IRQ\n");
> +               return -ENXIO;
> +       }
> +
> +       ret = devm_request_irq(dev, priv->irq, cap1106_isr, IRQF_DISABLED,
> +                              dev_name(dev), priv);
> +       if (ret < 0)
> +               return ret;
> +
> +       return 0;
> +}
> +
> +static int cap1106_i2c_remove(struct i2c_client *i2c_client)
> +{
> +       struct cap1106_priv *priv = i2c_get_clientdata(i2c_client);
> +
> +       input_unregister_device(priv->idev);
> +
> +       return 0;
> +}
> +
> +static const struct of_device_id cap1106_dt_ids[] = {
> +       { .compatible = "microchip,cap1106", },
> +       {}
> +};
> +MODULE_DEVICE_TABLE(of, cap1106_dt_ids);
> +
> +static const struct i2c_device_id cap1106_i2c_ids[] = {
> +       { "cap1106", 0 },
> +       {}
> +};
> +MODULE_DEVICE_TABLE(i2c, cap1106_i2c_ids);
> +
> +static struct i2c_driver cap1106_i2c_driver = {
> +       .driver = {
> +               .name   = "cap1106",
> +               .owner  = THIS_MODULE,
> +               .of_match_table = cap1106_dt_ids,
> +       },
> +       .id_table       = cap1106_i2c_ids,
> +       .probe          = cap1106_i2c_probe,
> +       .remove         = cap1106_i2c_remove,
> +};
> +
> +module_i2c_driver(cap1106_i2c_driver);
> +
> +MODULE_ALIAS("platform:" DRV_NAME);
> +MODULE_DESCRIPTION("Microchip CAP1106 driver");
> +MODULE_AUTHOR("Daniel Mack <linux@zonque.org>");
> +MODULE_LICENSE("GPL v2");
> --
> 1.9.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	[flat|nested] 4+ messages in thread

* Re: [PATCH] Input: Add driver for Microchip's CAP1106
  2014-07-11  7:42 [PATCH] Input: Add driver for Microchip's CAP1106 Daniel Mack
  2014-07-11  9:43 ` David Herrmann
@ 2014-07-11  9:47 ` David Herrmann
  1 sibling, 0 replies; 4+ messages in thread
From: David Herrmann @ 2014-07-11  9:47 UTC (permalink / raw)
  To: Daniel Mack; +Cc: dtor, open list:HID CORE LAYER, Mark Brown, devicetree

Hi

On Fri, Jul 11, 2014 at 9:42 AM, Daniel Mack <zonque@gmail.com> wrote:
> This patch adds a driver for Microchips CAP1106, an I2C driven, 6-channel
> capacitive touch sensor.
>
> For now, only the capacitive buttons are supported, and no specific
> settings that can be tweaked for individual channels, except for the
> device-wide sensitivity gain. The defaults seem to work just fine out of
> the box, so I'll leave configurable parameters for someone who's in need
> of them and who can actually measure the impact. All registers are
> prepared, however. Many of them are just not used for now.
>
> The implementation does not make any attempt to be compatible to platform
> data driven boards, but fully depends on CONFIG_OF.
>
> Power management functions are also left for volounteers with the ability
> to actually test them.
>
> Signed-off-by: Daniel Mack <zonque@gmail.com>
> ---
>  .../devicetree/bindings/input/cap1106.txt          |  63 ++++
>  drivers/input/keyboard/Kconfig                     |  10 +
>  drivers/input/keyboard/Makefile                    |   1 +
>  drivers/input/keyboard/cap1106.c                   | 376 +++++++++++++++++++++
>  4 files changed, 450 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/input/cap1106.txt
>  create mode 100644 drivers/input/keyboard/cap1106.c
>
> diff --git a/Documentation/devicetree/bindings/input/cap1106.txt b/Documentation/devicetree/bindings/input/cap1106.txt
> new file mode 100644
> index 0000000..57f5af3
...
> +       spin_lock_irqsave(&priv->lock, flags);
> +       disable_irq(priv->irq);
> +       cancel_work_sync(&priv->work);
> +       spin_unlock_irqrestore(&priv->lock, flags);

Btw., cancel_work_sync() has a might_sleep() annotation, so you really
cannot call it while holding spinlocks (see start_flush_work() in
workqueue.c).

Cheers
David

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

* Re: [PATCH] Input: Add driver for Microchip's CAP1106
       [not found]     ` <20140711173336.GA492@core.coreip.homeip.net>
@ 2014-07-11 17:42       ` Daniel Mack
  0 siblings, 0 replies; 4+ messages in thread
From: Daniel Mack @ 2014-07-11 17:42 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: David Herrmann, list@mail.zonque.de: HID CORE LAYER, Mark Brown,
	devicetree

On 07/11/2014 07:33 PM, Dmitry Torokhov wrote:
> On Fri, Jul 11, 2014 at 11:48:48AM +0200, Daniel Mack wrote:
>> On 07/11/2014 11:43 AM, David Herrmann wrote:
>>>> +static void cap1106_work(struct work_struct *w)
>>>>> +{
>>>>> +       struct cap1106_priv *priv = container_of(w, struct cap1106_priv, work);
>>>>> +       unsigned int status;
>>>>> +       int ret, i;
>>>>> +
>>>>> +       spin_lock(&priv->lock);
>>> If your worker started and is _about_ to take the spinlock, you might
>>> dead-lock with cap1106_close(). They might already hold the spinlock
>>> and then call cancel_work_sync(), thus waiting for this task to take
>>> the lock end exit..
>>>
>>> How about you add "bool closed : 1;" to your cap1106_priv structure
>>> and drop the lock here, but..
>>>
>>
>> Thanks for the review, David. Will look into it and send a v2 with these
>> changes soon.
> 
> Why aren't you using threaded oneshot IRQ for this? Then you would not
> need all this dancing around with worker and enabling/disabling
> interrupt.

Ah, right, that's much nicer indeed.
Thanks for the pointer! I'll post v3 later.


Daniel


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

end of thread, other threads:[~2014-07-11 17:42 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-07-11  7:42 [PATCH] Input: Add driver for Microchip's CAP1106 Daniel Mack
2014-07-11  9:43 ` David Herrmann
     [not found]   ` <53BFB300.1040500@zonque.org>
     [not found]     ` <20140711173336.GA492@core.coreip.homeip.net>
2014-07-11 17:42       ` Daniel Mack
2014-07-11  9:47 ` David Herrmann

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