[PATCH v2 0/4] fixes for twl6040-vibra

[PATCH v2 0/4] fixes for twl6040-vibra

[H. Nikolaus Schaller]

Changes V2:
* remove 2 patches already accepted
* keep DT memory management patch until we know if there is a better solution in the library
* suggestions by Dmitry Torokhov:
	* add cancel_work to make removal of mutex safe
	* replace "unregister handler" patch by "do not reparent" patch
* new: add fix for atomic schedule bug

V1: 2016-04-18 21:55:44: There are some small bugs in the twl6040-vibra driver and differences to how twl4030 vibra works.

This patch series addresses them.


Dmitry Torokhov (1):
  Input: twl6040-vibra - do not reparent to grandparent

H. Nikolaus Schaller (3):
  input: twl6040-vibra: fix DT node memory management
  input: twl6040-vibra: remove mutex
  input: twl6040-vibra: fix atomic schedule panic

 drivers/input/misc/twl6040-vibra.c | 31 ++++++++++---------------------
 1 file changed, 10 insertions(+), 21 deletions(-)

-- 
2.7.3

[PATCH v2 1/4] input: twl6040-vibra: fix DT node memory management

[H. Nikolaus Schaller]

commit e7ec014a47e4 ("Input: twl6040-vibra - update for device tree support")

made the separate vibra DT node to a subnode of the twl6040.

It now calls of_find_node_by_name() to locate the "vibra" subnode.
This function has a side effect to call of_node_put on() for the twl6040
parent node passed in as a parameter. This causes trouble later on.

Solution: we must call of_node_get() before of_find_node_by_name()

Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 drivers/input/misc/twl6040-vibra.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/input/misc/twl6040-vibra.c b/drivers/input/misc/twl6040-vibra.c
index 0c853c2..ad15498 100644
--- a/drivers/input/misc/twl6040-vibra.c
+++ b/drivers/input/misc/twl6040-vibra.c
@@ -257,6 +257,7 @@ static int twl6040_vibra_probe(struct platform_device *pdev)
 	int vddvibr_uV = 0;
 	int error;
 
+	of_node_get(twl6040_core_dev->of_node);
 	twl6040_core_node = of_find_node_by_name(twl6040_core_dev->of_node,
 						 "vibra");
 	if (!twl6040_core_node) {
-- 
2.7.3

[PATCH v2 2/4] input: twl6040-vibra: remove mutex

[H. Nikolaus Schaller]

The mutex does not seem to be needed. twl4030-vibra doesn't
use one either.

Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 drivers/input/misc/twl6040-vibra.c | 15 ++-------------
 1 file changed, 2 insertions(+), 13 deletions(-)

diff --git a/drivers/input/misc/twl6040-vibra.c b/drivers/input/misc/twl6040-vibra.c
index ad15498..f7e0d17 100644
--- a/drivers/input/misc/twl6040-vibra.c
+++ b/drivers/input/misc/twl6040-vibra.c
@@ -46,7 +46,7 @@ struct vibra_info {
 	struct device *dev;
 	struct input_dev *input_dev;
 	struct work_struct play_work;
-	struct mutex mutex;
+
 	int irq;
 
 	bool enabled;
@@ -182,8 +182,6 @@ static void vibra_play_work(struct work_struct *work)
 	struct vibra_info *info = container_of(work,
 				struct vibra_info, play_work);
 
-	mutex_lock(&info->mutex);
-
 	if (info->weak_speed || info->strong_speed) {
 		if (!info->enabled)
 			twl6040_vibra_enable(info);
@@ -192,7 +190,6 @@ static void vibra_play_work(struct work_struct *work)
 	} else if (info->enabled)
 		twl6040_vibra_disable(info);
 
-	mutex_unlock(&info->mutex);
 }
 
 static int vibra_play(struct input_dev *input, void *data,
@@ -223,12 +220,8 @@ static void twl6040_vibra_close(struct input_dev *input)
 
 	cancel_work_sync(&info->play_work);
 
-	mutex_lock(&info->mutex);
-
 	if (info->enabled)
 		twl6040_vibra_disable(info);
-
-	mutex_unlock(&info->mutex);
 }
 
 static int __maybe_unused twl6040_vibra_suspend(struct device *dev)
@@ -236,13 +229,11 @@ static int __maybe_unused twl6040_vibra_suspend(struct device *dev)
 	struct platform_device *pdev = to_platform_device(dev);
 	struct vibra_info *info = platform_get_drvdata(pdev);
 
-	mutex_lock(&info->mutex);
+	cancel_work_sync(&info->play_work);
 
 	if (info->enabled)
 		twl6040_vibra_disable(info);
 
-	mutex_unlock(&info->mutex);
-
 	return 0;
 }
 
@@ -301,8 +292,6 @@ static int twl6040_vibra_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	mutex_init(&info->mutex);
-
 	error = devm_request_threaded_irq(&pdev->dev, info->irq, NULL,
 					  twl6040_vib_irq_handler,
 					  IRQF_ONESHOT,
-- 
2.7.3

[PATCH v2 3/4] Input: twl6040-vibra - do not reparent to grandparent

[H. Nikolaus Schaller]

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>

---
 drivers/input/misc/twl6040-vibra.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/input/misc/twl6040-vibra.c b/drivers/input/misc/twl6040-vibra.c
index f7e0d17..f4384b7 100644
--- a/drivers/input/misc/twl6040-vibra.c
+++ b/drivers/input/misc/twl6040-vibra.c
@@ -347,7 +347,6 @@ static int twl6040_vibra_probe(struct platform_device *pdev)
 
 	info->input_dev->name = "twl6040:vibrator";
 	info->input_dev->id.version = 1;
-	info->input_dev->dev.parent = pdev->dev.parent;
 	info->input_dev->close = twl6040_vibra_close;
 	__set_bit(FF_RUMBLE, info->input_dev->ffbit);
 
-- 
2.7.3

[PATCH v2 4/4] input: twl6040-vibra: fix atomic schedule panic

[H. Nikolaus Schaller]

commit c6f39257c952 ("mfd: twl6040: Use regmap for register cache")

did remove the private cache for the vibra control registers and replaced
access within twl6040_get_vibralr_status() by calls to regmap. This is ok,
as long as twl6040_get_vibralr_status() uses already cached values or is
not called from interrupt context. But we call this in vibra_play() for checking
that the vibrator is not configured for audio mode.

The result is a "BUG: scheduling while atomic" if the first use of the
twl6040 is a vibra effect, because the first fetch is by reading the
twl6040 registers through (blocking) i2c and not from the cache.

As soon as the regmap has cached the status, further calls are fine.

The solution is to move the condition to the work() function which
runs in context that can block.

The original code returns -EBUSY, but the return value of ->play()
functions is ignored anyways. Hence, we do not loose functionality
by not returning an error but just reporting the issue to INFO loglevel.

Tested-on: Pyra (omap5) prototype
Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 drivers/input/misc/twl6040-vibra.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/input/misc/twl6040-vibra.c b/drivers/input/misc/twl6040-vibra.c
index f4384b7..5690eb7 100644
--- a/drivers/input/misc/twl6040-vibra.c
+++ b/drivers/input/misc/twl6040-vibra.c
@@ -181,6 +181,14 @@ static void vibra_play_work(struct work_struct *work)
 {
 	struct vibra_info *info = container_of(work,
 				struct vibra_info, play_work);
+	int ret;
+
+	/* Do not allow effect, while the routing is set to use audio */
+	ret = twl6040_get_vibralr_status(info->twl6040);
+	if (ret & TWL6040_VIBSEL) {
+		dev_info(info->dev, "Vibra is configured for audio\n");
+		return;
+	}
 
 	if (info->weak_speed || info->strong_speed) {
 		if (!info->enabled)
@@ -196,14 +204,6 @@ static int vibra_play(struct input_dev *input, void *data,
 		      struct ff_effect *effect)
 {
 	struct vibra_info *info = input_get_drvdata(input);
-	int ret;
-
-	/* Do not allow effect, while the routing is set to use audio */
-	ret = twl6040_get_vibralr_status(info->twl6040);
-	if (ret & TWL6040_VIBSEL) {
-		dev_info(&input->dev, "Vibra is configured for audio\n");
-		return -EBUSY;
-	}
 
 	info->weak_speed = effect->u.rumble.weak_magnitude;
 	info->strong_speed = effect->u.rumble.strong_magnitude;
-- 
2.7.3