linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/3] dt-bindings: input: add GPIO controllable vibrator
@ 2019-04-07 15:58 Luca Weiss
  2019-04-07 15:58 ` [PATCH v2 2/3] Input: add a driver for GPIO controllable vibrators Luca Weiss
  2019-04-07 15:58 ` [PATCH v2 3/3] ARM: dts: msm8974-FP2: Add vibration motor Luca Weiss
  0 siblings, 2 replies; 5+ messages in thread
From: Luca Weiss @ 2019-04-07 15:58 UTC (permalink / raw)
  Cc: Dmitry Torokhov, Rob Herring, Mark Rutland,
	Mauro Carvalho Chehab, Pascal PAILLET-LME, Coly Li, Lee Jones,
	Xiaotong Lu, Brian Masney, Rob Herring, Baolin Wang, Andy Gross,
	David Brown, open list:ARM/QUALCOMM SUPPORT,
	open list:INPUT (KEYBOARD, MOUSE, JOYSTICK , TOUCHSCREEN)...,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list, Luca Weiss

Provide a simple driver for GPIO controllable vibrators.
It will be used by the Fairphone 2.

Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
---
Changes from v1:
- Mark vcc-supply as optional

 .../bindings/input/gpio-vibrator.txt          | 20 +++++++++++++++++++
 1 file changed, 20 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/gpio-vibrator.txt

diff --git a/Documentation/devicetree/bindings/input/gpio-vibrator.txt b/Documentation/devicetree/bindings/input/gpio-vibrator.txt
new file mode 100644
index 000000000000..93e5a8e7622d
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/gpio-vibrator.txt
@@ -0,0 +1,20 @@
+* GPIO vibrator device tree bindings
+
+Registers a GPIO device as vibrator, where the vibration motor just has the
+capability to turn on or off. If the device is connected to a pwm, you should
+use the pwm-vibrator driver instead.
+
+Required properties:
+- compatible: should contain "gpio-vibrator"
+- enable-gpios: Should contain a GPIO handle
+
+Optional properties:
+- vcc-supply: Phandle for the regulator supplying power
+
+Example from Fairphone 2:
+
+vibrator {
+	compatible = "gpio-vibrator";
+	enable-gpios = <&msmgpio 86 GPIO_ACTIVE_HIGH>;
+	vcc-supply = <&pm8941_l18>;
+};
-- 
2.21.0


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

* [PATCH v2 2/3] Input: add a driver for GPIO controllable vibrators
  2019-04-07 15:58 [PATCH v2 1/3] dt-bindings: input: add GPIO controllable vibrator Luca Weiss
@ 2019-04-07 15:58 ` Luca Weiss
  2019-04-07 22:18   ` Dmitry Torokhov
  2019-04-07 15:58 ` [PATCH v2 3/3] ARM: dts: msm8974-FP2: Add vibration motor Luca Weiss
  1 sibling, 1 reply; 5+ messages in thread
From: Luca Weiss @ 2019-04-07 15:58 UTC (permalink / raw)
  Cc: Dmitry Torokhov, Rob Herring, Mark Rutland,
	Mauro Carvalho Chehab, Pascal PAILLET-LME, Coly Li, Lee Jones,
	Xiaotong Lu, Brian Masney, Rob Herring, Baolin Wang, Andy Gross,
	David Brown, open list:ARM/QUALCOMM SUPPORT,
	open list:INPUT (KEYBOARD, MOUSE, JOYSTICK , TOUCHSCREEN)...,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list, Luca Weiss

Provide a simple driver for GPIO controllable vibrators.
It will be used by the Fairphone 2.

Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
---
 drivers/input/misc/Kconfig      |  12 ++
 drivers/input/misc/Makefile     |   1 +
 drivers/input/misc/gpio-vibra.c | 214 ++++++++++++++++++++++++++++++++
 3 files changed, 227 insertions(+)
 create mode 100644 drivers/input/misc/gpio-vibra.c

diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index e15ed1bb8558..6dfe9e2fe5b1 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -290,6 +290,18 @@ config INPUT_GPIO_DECODER
 	 To compile this driver as a module, choose M here: the module
 	 will be called gpio_decoder.
 
+config INPUT_GPIO_VIBRA
+	tristate "GPIO vibrator support"
+	depends on GPIOLIB || COMPILE_TEST
+	select INPUT_FF_MEMLESS
+	help
+	  Say Y here to get support for GPIO based vibrator devices.
+
+	  If unsure, say N.
+
+	  To compile this driver as a module, choose M here: the module will be
+	  called gpio-vibra.
+
 config INPUT_IXP4XX_BEEPER
 	tristate "IXP4XX Beeper support"
 	depends on ARCH_IXP4XX
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index b936c5b1d4ac..f38ebbdb05e2 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -36,6 +36,7 @@ obj-$(CONFIG_INPUT_DRV2667_HAPTICS)	+= drv2667.o
 obj-$(CONFIG_INPUT_GP2A)		+= gp2ap002a00f.o
 obj-$(CONFIG_INPUT_GPIO_BEEPER)		+= gpio-beeper.o
 obj-$(CONFIG_INPUT_GPIO_DECODER)	+= gpio_decoder.o
+obj-$(CONFIG_INPUT_GPIO_VIBRA)		+= gpio-vibra.o
 obj-$(CONFIG_INPUT_HISI_POWERKEY)	+= hisi_powerkey.o
 obj-$(CONFIG_HP_SDC_RTC)		+= hp_sdc_rtc.o
 obj-$(CONFIG_INPUT_IMS_PCU)		+= ims-pcu.o
diff --git a/drivers/input/misc/gpio-vibra.c b/drivers/input/misc/gpio-vibra.c
new file mode 100644
index 000000000000..14f9534668c8
--- /dev/null
+++ b/drivers/input/misc/gpio-vibra.c
@@ -0,0 +1,214 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ *  GPIO vibrator driver
+ *
+ *  Copyright (C) 2019 Luca Weiss <luca@z3ntu.xyz>
+ *
+ *  Based on PWM vibrator driver:
+ *  Copyright (C) 2017 Collabora Ltd.
+ *
+ *  Based on previous work from:
+ *  Copyright (C) 2012 Dmitry Torokhov <dmitry.torokhov@gmail.com>
+ *
+ *  Based on PWM beeper driver:
+ *  Copyright (C) 2010, Lars-Peter Clausen <lars@metafoo.de>
+ */
+
+#include <linux/gpio/consumer.h>
+#include <linux/input.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/regulator/consumer.h>
+#include <linux/slab.h>
+
+enum vibrator_state {
+	VIBRATOR_OFF,
+	VIBRATOR_ON
+};
+
+struct gpio_vibrator {
+	struct input_dev *input;
+	struct gpio_desc *gpio;
+	struct regulator *vcc;
+
+	struct work_struct play_work;
+	enum vibrator_state state;
+	bool vcc_on;
+};
+
+static int gpio_vibrator_start(struct gpio_vibrator *vibrator)
+{
+	struct device *pdev = vibrator->input->dev.parent;
+	int err;
+
+	if (!vibrator->vcc_on) {
+		err = regulator_enable(vibrator->vcc);
+		if (err) {
+			dev_err(pdev, "failed to enable regulator: %d", err);
+			return err;
+		}
+		vibrator->vcc_on = true;
+	}
+
+	gpiod_set_value(vibrator->gpio, 1);
+
+	return 0;
+}
+
+static void gpio_vibrator_stop(struct gpio_vibrator *vibrator)
+{
+	gpiod_set_value(vibrator->gpio, 0);
+
+	if (vibrator->vcc_on) {
+		regulator_disable(vibrator->vcc);
+		vibrator->vcc_on = false;
+	}
+}
+
+static void gpio_vibrator_play_work(struct work_struct *work)
+{
+	struct gpio_vibrator *vibrator = container_of(work,
+					struct gpio_vibrator, play_work);
+
+	if (vibrator->state == VIBRATOR_ON)
+		gpio_vibrator_start(vibrator);
+	else
+		gpio_vibrator_stop(vibrator);
+}
+
+static int gpio_vibrator_play_effect(struct input_dev *dev, void *data,
+				    struct ff_effect *effect)
+{
+	struct gpio_vibrator *vibrator = input_get_drvdata(dev);
+
+	int level = effect->u.rumble.strong_magnitude;
+
+	if (!level)
+		level = effect->u.rumble.weak_magnitude;
+
+	if (level)
+		vibrator->state = VIBRATOR_ON;
+	else
+		vibrator->state = VIBRATOR_OFF;
+
+	schedule_work(&vibrator->play_work);
+
+	return 0;
+}
+
+static void gpio_vibrator_close(struct input_dev *input)
+{
+	struct gpio_vibrator *vibrator = input_get_drvdata(input);
+
+	cancel_work_sync(&vibrator->play_work);
+	gpio_vibrator_stop(vibrator);
+}
+
+static int gpio_vibrator_probe(struct platform_device *pdev)
+{
+	struct gpio_vibrator *vibrator;
+	int err;
+
+	vibrator = devm_kzalloc(&pdev->dev, sizeof(*vibrator), GFP_KERNEL);
+	if (!vibrator)
+		return -ENOMEM;
+
+	vibrator->input = devm_input_allocate_device(&pdev->dev);
+	if (!vibrator->input)
+		return -ENOMEM;
+
+	vibrator->vcc = devm_regulator_get(&pdev->dev, "vcc");
+	err = PTR_ERR_OR_ZERO(vibrator->vcc);
+	if (err) {
+		if (err != -EPROBE_DEFER)
+			dev_err(&pdev->dev, "Failed to request regulator: %d",
+				err);
+		return err;
+	}
+
+	vibrator->gpio = devm_gpiod_get(&pdev->dev, "enable", GPIOD_OUT_LOW);
+	err = PTR_ERR_OR_ZERO(vibrator->gpio);
+	if (err) {
+		if (err != -EPROBE_DEFER)
+			dev_err(&pdev->dev, "Failed to request main gpio: %d",
+				err);
+		return err;
+	}
+
+	INIT_WORK(&vibrator->play_work, gpio_vibrator_play_work);
+
+	vibrator->input->name = "gpio-vibrator";
+	vibrator->input->id.bustype = BUS_HOST;
+	vibrator->input->dev.parent = &pdev->dev;
+	vibrator->input->close = gpio_vibrator_close;
+
+	input_set_drvdata(vibrator->input, vibrator);
+	input_set_capability(vibrator->input, EV_FF, FF_RUMBLE);
+
+	err = input_ff_create_memless(vibrator->input, NULL,
+				      gpio_vibrator_play_effect);
+	if (err) {
+		dev_err(&pdev->dev, "Couldn't create FF dev: %d", err);
+		return err;
+	}
+
+	err = input_register_device(vibrator->input);
+	if (err) {
+		dev_err(&pdev->dev, "Couldn't register input dev: %d", err);
+		return err;
+	}
+
+	platform_set_drvdata(pdev, vibrator);
+
+	return 0;
+}
+
+static int __maybe_unused gpio_vibrator_suspend(struct device *dev)
+{
+	struct gpio_vibrator *vibrator = dev_get_drvdata(dev);
+
+	cancel_work_sync(&vibrator->play_work);
+	if (vibrator->state == VIBRATOR_ON)
+		gpio_vibrator_stop(vibrator);
+
+	return 0;
+}
+
+static int __maybe_unused gpio_vibrator_resume(struct device *dev)
+{
+	struct gpio_vibrator *vibrator = dev_get_drvdata(dev);
+
+	if (vibrator->state == VIBRATOR_ON)
+		gpio_vibrator_start(vibrator);
+
+	return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(gpio_vibrator_pm_ops,
+			 gpio_vibrator_suspend, gpio_vibrator_resume);
+
+#ifdef CONFIG_OF
+static const struct of_device_id gpio_vibra_dt_match_table[] = {
+	{ .compatible = "gpio-vibrator" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, gpio_vibra_dt_match_table);
+#endif
+
+static struct platform_driver gpio_vibrator_driver = {
+	.probe	= gpio_vibrator_probe,
+	.driver	= {
+		.name	= "gpio-vibrator",
+		.pm	= &gpio_vibrator_pm_ops,
+		.of_match_table = of_match_ptr(gpio_vibra_dt_match_table),
+	},
+};
+module_platform_driver(gpio_vibrator_driver);
+
+MODULE_AUTHOR("Luca Weiss <luca@z3ntu.xy>");
+MODULE_DESCRIPTION("GPIO vibrator driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:gpio-vibrator");
-- 
2.21.0


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

* [PATCH v2 3/3] ARM: dts: msm8974-FP2: Add vibration motor
  2019-04-07 15:58 [PATCH v2 1/3] dt-bindings: input: add GPIO controllable vibrator Luca Weiss
  2019-04-07 15:58 ` [PATCH v2 2/3] Input: add a driver for GPIO controllable vibrators Luca Weiss
@ 2019-04-07 15:58 ` Luca Weiss
  1 sibling, 0 replies; 5+ messages in thread
From: Luca Weiss @ 2019-04-07 15:58 UTC (permalink / raw)
  Cc: Dmitry Torokhov, Rob Herring, Mark Rutland,
	Mauro Carvalho Chehab, Pascal PAILLET-LME, Coly Li, Lee Jones,
	Xiaotong Lu, Brian Masney, Rob Herring, Baolin Wang, Andy Gross,
	David Brown, open list:ARM/QUALCOMM SUPPORT,
	open list:INPUT (KEYBOARD, MOUSE, JOYSTICK , TOUCHSCREEN)...,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list, Luca Weiss

Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
---
 arch/arm/boot/dts/qcom-msm8974-fairphone-fp2.dts | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/qcom-msm8974-fairphone-fp2.dts b/arch/arm/boot/dts/qcom-msm8974-fairphone-fp2.dts
index 643c57f84818..bf402ae39226 100644
--- a/arch/arm/boot/dts/qcom-msm8974-fairphone-fp2.dts
+++ b/arch/arm/boot/dts/qcom-msm8974-fairphone-fp2.dts
@@ -50,6 +50,12 @@
 		};
 	};
 
+	vibrator {
+		compatible = "gpio-vibrator";
+		enable-gpios = <&msmgpio 86 GPIO_ACTIVE_HIGH>;
+		vcc-supply = <&pm8941_l18>;
+	};
+
 	smd {
 		rpm {
 			rpm_requests {
-- 
2.21.0


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

* Re: [PATCH v2 2/3] Input: add a driver for GPIO controllable vibrators
  2019-04-07 15:58 ` [PATCH v2 2/3] Input: add a driver for GPIO controllable vibrators Luca Weiss
@ 2019-04-07 22:18   ` Dmitry Torokhov
  2019-04-09 11:48     ` Luca Weiss
  0 siblings, 1 reply; 5+ messages in thread
From: Dmitry Torokhov @ 2019-04-07 22:18 UTC (permalink / raw)
  To: Luca Weiss
  Cc: Rob Herring, Mark Rutland, Mauro Carvalho Chehab,
	Pascal PAILLET-LME, Coly Li, Lee Jones, Xiaotong Lu,
	Brian Masney, Rob Herring, Baolin Wang, Andy Gross, David Brown,
	open list:ARM/QUALCOMM SUPPORT, open list:INPUT (KEYBOARD, MOUSE,
	JOYSTICK, TOUCHSCREEN)...,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list

Hi Luca,

On Sun, Apr 07, 2019 at 05:58:41PM +0200, Luca Weiss wrote:
> Provide a simple driver for GPIO controllable vibrators.
> It will be used by the Fairphone 2.
> 
> Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
> ---
>  drivers/input/misc/Kconfig      |  12 ++
>  drivers/input/misc/Makefile     |   1 +
>  drivers/input/misc/gpio-vibra.c | 214 ++++++++++++++++++++++++++++++++
>  3 files changed, 227 insertions(+)
>  create mode 100644 drivers/input/misc/gpio-vibra.c
> 
> diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
> index e15ed1bb8558..6dfe9e2fe5b1 100644
> --- a/drivers/input/misc/Kconfig
> +++ b/drivers/input/misc/Kconfig
> @@ -290,6 +290,18 @@ config INPUT_GPIO_DECODER
>  	 To compile this driver as a module, choose M here: the module
>  	 will be called gpio_decoder.
>  
> +config INPUT_GPIO_VIBRA
> +	tristate "GPIO vibrator support"
> +	depends on GPIOLIB || COMPILE_TEST
> +	select INPUT_FF_MEMLESS
> +	help
> +	  Say Y here to get support for GPIO based vibrator devices.
> +
> +	  If unsure, say N.
> +
> +	  To compile this driver as a module, choose M here: the module will be
> +	  called gpio-vibra.
> +
>  config INPUT_IXP4XX_BEEPER
>  	tristate "IXP4XX Beeper support"
>  	depends on ARCH_IXP4XX
> diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
> index b936c5b1d4ac..f38ebbdb05e2 100644
> --- a/drivers/input/misc/Makefile
> +++ b/drivers/input/misc/Makefile
> @@ -36,6 +36,7 @@ obj-$(CONFIG_INPUT_DRV2667_HAPTICS)	+= drv2667.o
>  obj-$(CONFIG_INPUT_GP2A)		+= gp2ap002a00f.o
>  obj-$(CONFIG_INPUT_GPIO_BEEPER)		+= gpio-beeper.o
>  obj-$(CONFIG_INPUT_GPIO_DECODER)	+= gpio_decoder.o
> +obj-$(CONFIG_INPUT_GPIO_VIBRA)		+= gpio-vibra.o
>  obj-$(CONFIG_INPUT_HISI_POWERKEY)	+= hisi_powerkey.o
>  obj-$(CONFIG_HP_SDC_RTC)		+= hp_sdc_rtc.o
>  obj-$(CONFIG_INPUT_IMS_PCU)		+= ims-pcu.o
> diff --git a/drivers/input/misc/gpio-vibra.c b/drivers/input/misc/gpio-vibra.c
> new file mode 100644
> index 000000000000..14f9534668c8
> --- /dev/null
> +++ b/drivers/input/misc/gpio-vibra.c
> @@ -0,0 +1,214 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + *  GPIO vibrator driver
> + *
> + *  Copyright (C) 2019 Luca Weiss <luca@z3ntu.xyz>
> + *
> + *  Based on PWM vibrator driver:
> + *  Copyright (C) 2017 Collabora Ltd.
> + *
> + *  Based on previous work from:
> + *  Copyright (C) 2012 Dmitry Torokhov <dmitry.torokhov@gmail.com>
> + *
> + *  Based on PWM beeper driver:
> + *  Copyright (C) 2010, Lars-Peter Clausen <lars@metafoo.de>
> + */
> +
> +#include <linux/gpio/consumer.h>
> +#include <linux/input.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/property.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/slab.h>
> +
> +enum vibrator_state {
> +	VIBRATOR_OFF,
> +	VIBRATOR_ON
> +};

I'd probably go with simple "bool running".

> +
> +struct gpio_vibrator {
> +	struct input_dev *input;
> +	struct gpio_desc *gpio;
> +	struct regulator *vcc;
> +
> +	struct work_struct play_work;
> +	enum vibrator_state state;
> +	bool vcc_on;
> +};
> +
> +static int gpio_vibrator_start(struct gpio_vibrator *vibrator)
> +{
> +	struct device *pdev = vibrator->input->dev.parent;
> +	int err;
> +
> +	if (!vibrator->vcc_on) {
> +		err = regulator_enable(vibrator->vcc);
> +		if (err) {
> +			dev_err(pdev, "failed to enable regulator: %d", err);
> +			return err;
> +		}
> +		vibrator->vcc_on = true;
> +	}
> +
> +	gpiod_set_value(vibrator->gpio, 1);

Since this is called by work item and thus from context that can sleep,
we can use gpiod_set_value_cansleep() so that "slow" GPIOs can be used
with the driver.

> +
> +	return 0;
> +}
> +
> +static void gpio_vibrator_stop(struct gpio_vibrator *vibrator)
> +{
> +	gpiod_set_value(vibrator->gpio, 0);
> +
> +	if (vibrator->vcc_on) {
> +		regulator_disable(vibrator->vcc);
> +		vibrator->vcc_on = false;
> +	}
> +}
> +
> +static void gpio_vibrator_play_work(struct work_struct *work)
> +{
> +	struct gpio_vibrator *vibrator = container_of(work,
> +					struct gpio_vibrator, play_work);
> +
> +	if (vibrator->state == VIBRATOR_ON)
> +		gpio_vibrator_start(vibrator);
> +	else
> +		gpio_vibrator_stop(vibrator);
> +}
> +
> +static int gpio_vibrator_play_effect(struct input_dev *dev, void *data,
> +				    struct ff_effect *effect)
> +{
> +	struct gpio_vibrator *vibrator = input_get_drvdata(dev);
> +
> +	int level = effect->u.rumble.strong_magnitude;
> +
> +	if (!level)
> +		level = effect->u.rumble.weak_magnitude;
> +
> +	if (level)
> +		vibrator->state = VIBRATOR_ON;
> +	else
> +		vibrator->state = VIBRATOR_OFF;
> +
> +	schedule_work(&vibrator->play_work);
> +
> +	return 0;
> +}
> +
> +static void gpio_vibrator_close(struct input_dev *input)
> +{
> +	struct gpio_vibrator *vibrator = input_get_drvdata(input);
> +
> +	cancel_work_sync(&vibrator->play_work);
> +	gpio_vibrator_stop(vibrator);

Do you want to update vibrator->state here?

> +}
> +
> +static int gpio_vibrator_probe(struct platform_device *pdev)
> +{
> +	struct gpio_vibrator *vibrator;
> +	int err;
> +
> +	vibrator = devm_kzalloc(&pdev->dev, sizeof(*vibrator), GFP_KERNEL);
> +	if (!vibrator)
> +		return -ENOMEM;
> +
> +	vibrator->input = devm_input_allocate_device(&pdev->dev);
> +	if (!vibrator->input)
> +		return -ENOMEM;
> +
> +	vibrator->vcc = devm_regulator_get(&pdev->dev, "vcc");
> +	err = PTR_ERR_OR_ZERO(vibrator->vcc);
> +	if (err) {
> +		if (err != -EPROBE_DEFER)
> +			dev_err(&pdev->dev, "Failed to request regulator: %d",
> +				err);
> +		return err;
> +	}
> +
> +	vibrator->gpio = devm_gpiod_get(&pdev->dev, "enable", GPIOD_OUT_LOW);
> +	err = PTR_ERR_OR_ZERO(vibrator->gpio);
> +	if (err) {
> +		if (err != -EPROBE_DEFER)
> +			dev_err(&pdev->dev, "Failed to request main gpio: %d",
> +				err);
> +		return err;
> +	}
> +
> +	INIT_WORK(&vibrator->play_work, gpio_vibrator_play_work);
> +
> +	vibrator->input->name = "gpio-vibrator";
> +	vibrator->input->id.bustype = BUS_HOST;
> +	vibrator->input->dev.parent = &pdev->dev;

No need to set as you used devm_input_device_alloc().

> +	vibrator->input->close = gpio_vibrator_close;
> +
> +	input_set_drvdata(vibrator->input, vibrator);
> +	input_set_capability(vibrator->input, EV_FF, FF_RUMBLE);
> +
> +	err = input_ff_create_memless(vibrator->input, NULL,
> +				      gpio_vibrator_play_effect);
> +	if (err) {
> +		dev_err(&pdev->dev, "Couldn't create FF dev: %d", err);
> +		return err;
> +	}
> +
> +	err = input_register_device(vibrator->input);
> +	if (err) {
> +		dev_err(&pdev->dev, "Couldn't register input dev: %d", err);
> +		return err;
> +	}
> +
> +	platform_set_drvdata(pdev, vibrator);
> +
> +	return 0;
> +}
> +
> +static int __maybe_unused gpio_vibrator_suspend(struct device *dev)
> +{
> +	struct gpio_vibrator *vibrator = dev_get_drvdata(dev);
> +
> +	cancel_work_sync(&vibrator->play_work);
> +	if (vibrator->state == VIBRATOR_ON)
> +		gpio_vibrator_stop(vibrator);
> +
> +	return 0;
> +}
> +
> +static int __maybe_unused gpio_vibrator_resume(struct device *dev)
> +{
> +	struct gpio_vibrator *vibrator = dev_get_drvdata(dev);
> +
> +	if (vibrator->state == VIBRATOR_ON)
> +		gpio_vibrator_start(vibrator);
> +
> +	return 0;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(gpio_vibrator_pm_ops,
> +			 gpio_vibrator_suspend, gpio_vibrator_resume);
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id gpio_vibra_dt_match_table[] = {
> +	{ .compatible = "gpio-vibrator" },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, gpio_vibra_dt_match_table);
> +#endif
> +
> +static struct platform_driver gpio_vibrator_driver = {
> +	.probe	= gpio_vibrator_probe,
> +	.driver	= {
> +		.name	= "gpio-vibrator",
> +		.pm	= &gpio_vibrator_pm_ops,
> +		.of_match_table = of_match_ptr(gpio_vibra_dt_match_table),
> +	},
> +};
> +module_platform_driver(gpio_vibrator_driver);
> +
> +MODULE_AUTHOR("Luca Weiss <luca@z3ntu.xy>");
> +MODULE_DESCRIPTION("GPIO vibrator driver");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:gpio-vibrator");
> -- 
> 2.21.0
> 

Thanks.

-- 
Dmitry

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

* Re: [PATCH v2 2/3] Input: add a driver for GPIO controllable vibrators
  2019-04-07 22:18   ` Dmitry Torokhov
@ 2019-04-09 11:48     ` Luca Weiss
  0 siblings, 0 replies; 5+ messages in thread
From: Luca Weiss @ 2019-04-09 11:48 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Rob Herring, Mark Rutland, Mauro Carvalho Chehab,
	Pascal PAILLET-LME, Coly Li, Lee Jones, Xiaotong Lu,
	Brian Masney, Rob Herring, Baolin Wang, Andy Gross, David Brown,
	open list:ARM/QUALCOMM SUPPORT, open list:INPUT (KEYBOARD, MOUSE,
	JOYSTICK, TOUCHSCREEN)...,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list

Hello Dmitry,

see inline comments - and:
your last two comments should probably be also applied to the pwm-vibra.c 
driver, as that's where these code blocks come from.

On Montag, 8. April 2019 00:18:57 CEST Dmitry Torokhov wrote:
> Hi Luca,
> 
> On Sun, Apr 07, 2019 at 05:58:41PM +0200, Luca Weiss wrote:
> > Provide a simple driver for GPIO controllable vibrators.
> > It will be used by the Fairphone 2.
> > 
> > Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
> > ---
> > 
> >  drivers/input/misc/Kconfig      |  12 ++
> >  drivers/input/misc/Makefile     |   1 +
> >  drivers/input/misc/gpio-vibra.c | 214 ++++++++++++++++++++++++++++++++
> >  3 files changed, 227 insertions(+)
> >  create mode 100644 drivers/input/misc/gpio-vibra.c
> > 
> > diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
> > index e15ed1bb8558..6dfe9e2fe5b1 100644
> > --- a/drivers/input/misc/Kconfig
> > +++ b/drivers/input/misc/Kconfig
> > @@ -290,6 +290,18 @@ config INPUT_GPIO_DECODER
> > 
> >  	 To compile this driver as a module, choose M here: the module
> >  	 will be called gpio_decoder.
> > 
> > +config INPUT_GPIO_VIBRA
> > +	tristate "GPIO vibrator support"
> > +	depends on GPIOLIB || COMPILE_TEST
> > +	select INPUT_FF_MEMLESS
> > +	help
> > +	  Say Y here to get support for GPIO based vibrator devices.
> > +
> > +	  If unsure, say N.
> > +
> > +	  To compile this driver as a module, choose M here: the module 
will be
> > +	  called gpio-vibra.
> > +
> > 
> >  config INPUT_IXP4XX_BEEPER
> >  
> >  	tristate "IXP4XX Beeper support"
> >  	depends on ARCH_IXP4XX
> > 
> > diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
> > index b936c5b1d4ac..f38ebbdb05e2 100644
> > --- a/drivers/input/misc/Makefile
> > +++ b/drivers/input/misc/Makefile
> > @@ -36,6 +36,7 @@ obj-$(CONFIG_INPUT_DRV2667_HAPTICS)	+= drv2667.o
> > 
> >  obj-$(CONFIG_INPUT_GP2A)		+= gp2ap002a00f.o
> >  obj-$(CONFIG_INPUT_GPIO_BEEPER)		+= gpio-beeper.o
> >  obj-$(CONFIG_INPUT_GPIO_DECODER)	+= gpio_decoder.o
> > 
> > +obj-$(CONFIG_INPUT_GPIO_VIBRA)		+= gpio-vibra.o
> > 
> >  obj-$(CONFIG_INPUT_HISI_POWERKEY)	+= hisi_powerkey.o
> >  obj-$(CONFIG_HP_SDC_RTC)		+= hp_sdc_rtc.o
> >  obj-$(CONFIG_INPUT_IMS_PCU)		+= ims-pcu.o
> > 
> > diff --git a/drivers/input/misc/gpio-vibra.c
> > b/drivers/input/misc/gpio-vibra.c new file mode 100644
> > index 000000000000..14f9534668c8
> > --- /dev/null
> > +++ b/drivers/input/misc/gpio-vibra.c
> > @@ -0,0 +1,214 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + *  GPIO vibrator driver
> > + *
> > + *  Copyright (C) 2019 Luca Weiss <luca@z3ntu.xyz>
> > + *
> > + *  Based on PWM vibrator driver:
> > + *  Copyright (C) 2017 Collabora Ltd.
> > + *
> > + *  Based on previous work from:
> > + *  Copyright (C) 2012 Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > + *
> > + *  Based on PWM beeper driver:
> > + *  Copyright (C) 2010, Lars-Peter Clausen <lars@metafoo.de>
> > + */
> > +
> > +#include <linux/gpio/consumer.h>
> > +#include <linux/input.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/of_device.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/property.h>
> > +#include <linux/regulator/consumer.h>
> > +#include <linux/slab.h>
> > +
> > +enum vibrator_state {
> > +	VIBRATOR_OFF,
> > +	VIBRATOR_ON
> > +};
> 
> I'd probably go with simple "bool running".
Changed.
> 
> > +
> > +struct gpio_vibrator {
> > +	struct input_dev *input;
> > +	struct gpio_desc *gpio;
> > +	struct regulator *vcc;
> > +
> > +	struct work_struct play_work;
> > +	enum vibrator_state state;
> > +	bool vcc_on;
> > +};
> > +
> > +static int gpio_vibrator_start(struct gpio_vibrator *vibrator)
> > +{
> > +	struct device *pdev = vibrator->input->dev.parent;
> > +	int err;
> > +
> > +	if (!vibrator->vcc_on) {
> > +		err = regulator_enable(vibrator->vcc);
> > +		if (err) {
> > +			dev_err(pdev, "failed to enable regulator: 
%d", err);
> > +			return err;
> > +		}
> > +		vibrator->vcc_on = true;
> > +	}
> > +
> > +	gpiod_set_value(vibrator->gpio, 1);
> 
> Since this is called by work item and thus from context that can sleep,
> we can use gpiod_set_value_cansleep() so that "slow" GPIOs can be used
> with the driver.
Changed.
> 
> > +
> > +	return 0;
> > +}
> > +
> > +static void gpio_vibrator_stop(struct gpio_vibrator *vibrator)
> > +{
> > +	gpiod_set_value(vibrator->gpio, 0);
> > +
> > +	if (vibrator->vcc_on) {
> > +		regulator_disable(vibrator->vcc);
> > +		vibrator->vcc_on = false;
> > +	}
> > +}
> > +
> > +static void gpio_vibrator_play_work(struct work_struct *work)
> > +{
> > +	struct gpio_vibrator *vibrator = container_of(work,
> > +					struct gpio_vibrator, 
play_work);
> > +
> > +	if (vibrator->state == VIBRATOR_ON)
> > +		gpio_vibrator_start(vibrator);
> > +	else
> > +		gpio_vibrator_stop(vibrator);
> > +}
> > +
> > +static int gpio_vibrator_play_effect(struct input_dev *dev, void *data,
> > +				    struct ff_effect *effect)
> > +{
> > +	struct gpio_vibrator *vibrator = input_get_drvdata(dev);
> > +
> > +	int level = effect->u.rumble.strong_magnitude;
> > +
> > +	if (!level)
> > +		level = effect->u.rumble.weak_magnitude;
> > +
> > +	if (level)
> > +		vibrator->state = VIBRATOR_ON;
> > +	else
> > +		vibrator->state = VIBRATOR_OFF;
> > +
> > +	schedule_work(&vibrator->play_work);
> > +
> > +	return 0;
> > +}
> > +
> > +static void gpio_vibrator_close(struct input_dev *input)
> > +{
> > +	struct gpio_vibrator *vibrator = input_get_drvdata(input);
> > +
> > +	cancel_work_sync(&vibrator->play_work);
> > +	gpio_vibrator_stop(vibrator);
> 
> Do you want to update vibrator->state here?
I've looked at that a bit and it makes sense when the vibrator is on, the 
input it closed (and the vibrator stopped but the variable is still 'on'), 
then the device is suspended and resumed, the vibrator will be turned on 
again, so I'll change that.
> 
> > +}
> > +
> > +static int gpio_vibrator_probe(struct platform_device *pdev)
> > +{
> > +	struct gpio_vibrator *vibrator;
> > +	int err;
> > +
> > +	vibrator = devm_kzalloc(&pdev->dev, sizeof(*vibrator), 
GFP_KERNEL);
> > +	if (!vibrator)
> > +		return -ENOMEM;
> > +
> > +	vibrator->input = devm_input_allocate_device(&pdev->dev);
> > +	if (!vibrator->input)
> > +		return -ENOMEM;
> > +
> > +	vibrator->vcc = devm_regulator_get(&pdev->dev, "vcc");
> > +	err = PTR_ERR_OR_ZERO(vibrator->vcc);
> > +	if (err) {
> > +		if (err != -EPROBE_DEFER)
> > +			dev_err(&pdev->dev, "Failed to request 
regulator: %d",
> > +				err);
> > +		return err;
> > +	}
> > +
> > +	vibrator->gpio = devm_gpiod_get(&pdev->dev, "enable", 
GPIOD_OUT_LOW);
> > +	err = PTR_ERR_OR_ZERO(vibrator->gpio);
> > +	if (err) {
> > +		if (err != -EPROBE_DEFER)
> > +			dev_err(&pdev->dev, "Failed to request main 
gpio: %d",
> > +				err);
> > +		return err;
> > +	}
> > +
> > +	INIT_WORK(&vibrator->play_work, gpio_vibrator_play_work);
> > +
> > +	vibrator->input->name = "gpio-vibrator";
> > +	vibrator->input->id.bustype = BUS_HOST;
> > +	vibrator->input->dev.parent = &pdev->dev;
> 
> No need to set as you used devm_input_device_alloc().
Changed.
> > +	vibrator->input->close = gpio_vibrator_close;
> > +
> > +	input_set_drvdata(vibrator->input, vibrator);
> > +	input_set_capability(vibrator->input, EV_FF, FF_RUMBLE);
> > +
> > +	err = input_ff_create_memless(vibrator->input, NULL,
> > +				      gpio_vibrator_play_effect);
> > +	if (err) {
> > +		dev_err(&pdev->dev, "Couldn't create FF dev: %d", err);
> > +		return err;
> > +	}
> > +
> > +	err = input_register_device(vibrator->input);
> > +	if (err) {
> > +		dev_err(&pdev->dev, "Couldn't register input dev: %d", 
err);
> > +		return err;
> > +	}
> > +
> > +	platform_set_drvdata(pdev, vibrator);
> > +
> > +	return 0;
> > +}
> > +
> > +static int __maybe_unused gpio_vibrator_suspend(struct device *dev)
> > +{
> > +	struct gpio_vibrator *vibrator = dev_get_drvdata(dev);
> > +
> > +	cancel_work_sync(&vibrator->play_work);
> > +	if (vibrator->state == VIBRATOR_ON)
> > +		gpio_vibrator_stop(vibrator);
> > +
> > +	return 0;
> > +}
> > +
> > +static int __maybe_unused gpio_vibrator_resume(struct device *dev)
> > +{
> > +	struct gpio_vibrator *vibrator = dev_get_drvdata(dev);
> > +
> > +	if (vibrator->state == VIBRATOR_ON)
> > +		gpio_vibrator_start(vibrator);
> > +
> > +	return 0;
> > +}
> > +
> > +static SIMPLE_DEV_PM_OPS(gpio_vibrator_pm_ops,
> > +			 gpio_vibrator_suspend, 
gpio_vibrator_resume);
> > +
> > +#ifdef CONFIG_OF
> > +static const struct of_device_id gpio_vibra_dt_match_table[] = {
> > +	{ .compatible = "gpio-vibrator" },
> > +	{},
> > +};
> > +MODULE_DEVICE_TABLE(of, gpio_vibra_dt_match_table);
> > +#endif
> > +
> > +static struct platform_driver gpio_vibrator_driver = {
> > +	.probe	= gpio_vibrator_probe,
> > +	.driver	= {
> > +		.name	= "gpio-vibrator",
> > +		.pm	= &gpio_vibrator_pm_ops,
> > +		.of_match_table = 
of_match_ptr(gpio_vibra_dt_match_table),
> > +	},
> > +};
> > +module_platform_driver(gpio_vibrator_driver);
> > +
> > +MODULE_AUTHOR("Luca Weiss <luca@z3ntu.xy>");
> > +MODULE_DESCRIPTION("GPIO vibrator driver");
> > +MODULE_LICENSE("GPL");
> > +MODULE_ALIAS("platform:gpio-vibrator");
> 
> Thanks.

Thank you for the review, will send a v3 soon!
Luca



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

end of thread, other threads:[~2019-04-09 11:48 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-07 15:58 [PATCH v2 1/3] dt-bindings: input: add GPIO controllable vibrator Luca Weiss
2019-04-07 15:58 ` [PATCH v2 2/3] Input: add a driver for GPIO controllable vibrators Luca Weiss
2019-04-07 22:18   ` Dmitry Torokhov
2019-04-09 11:48     ` Luca Weiss
2019-04-07 15:58 ` [PATCH v2 3/3] ARM: dts: msm8974-FP2: Add vibration motor Luca Weiss

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