* [PATCH 0/2] Add regulator-haptic driver
@ 2014-11-20 13:31 Jaewon Kim
2014-11-20 13:31 ` [PATCH 1/2] Input: add regulator haptic driver Jaewon Kim
2014-11-20 13:31 ` [PATCH 2/2] ARM: dts: Add regulator-haptic device node for exynos3250-rinato Jaewon Kim
0 siblings, 2 replies; 10+ messages in thread
From: Jaewon Kim @ 2014-11-20 13:31 UTC (permalink / raw)
To: Kukjin Kim, Dmitry Torokhov
Cc: Chanwoo Choi, linux-kernel, linux-input, linux-samsung-soc, Jaewon Kim
This patch series adds regulator-haptic driver.
The regulator-haptic has haptic motor and it is controlled by
voltage of regulator via force feedback framework.
Jaewon Kim (2):
Input: add regulator haptic driver
ARM: dts: Add regulator-haptic device node for exynos3250-rinato
.../devicetree/bindings/input/regulator-haptic.txt | 24 ++
arch/arm/boot/dts/exynos3250-rinato.dts | 7 +
drivers/input/misc/Kconfig | 11 +
drivers/input/misc/Makefile | 1 +
drivers/input/misc/regulator-haptic.c | 241 ++++++++++++++++++++
5 files changed, 284 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/regulator-haptic.txt
create mode 100644 drivers/input/misc/regulator-haptic.c
--
1.7.9.5
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] Input: add regulator haptic driver
2014-11-20 13:31 [PATCH 0/2] Add regulator-haptic driver Jaewon Kim
@ 2014-11-20 13:31 ` Jaewon Kim
2014-11-20 14:33 ` Pankaj Dubey
[not found] ` <20141128125051.GA3643@sirena.org.uk>
2014-11-20 13:31 ` [PATCH 2/2] ARM: dts: Add regulator-haptic device node for exynos3250-rinato Jaewon Kim
1 sibling, 2 replies; 10+ messages in thread
From: Jaewon Kim @ 2014-11-20 13:31 UTC (permalink / raw)
To: Kukjin Kim, Dmitry Torokhov
Cc: Chanwoo Choi, linux-kernel, linux-input, linux-samsung-soc,
Jaewon Kim, Hyunhee Kim
This patch adds support for haptic driver controlled by
voltage of regulator. And this driver support for
Force Feedback interface from input framework
Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com>
Signed-off-by: Hyunhee Kim <hyunhee.kim@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
---
.../devicetree/bindings/input/regulator-haptic.txt | 24 ++
drivers/input/misc/Kconfig | 11 +
drivers/input/misc/Makefile | 1 +
drivers/input/misc/regulator-haptic.c | 241 ++++++++++++++++++++
4 files changed, 277 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/regulator-haptic.txt
create mode 100644 drivers/input/misc/regulator-haptic.c
diff --git a/Documentation/devicetree/bindings/input/regulator-haptic.txt b/Documentation/devicetree/bindings/input/regulator-haptic.txt
new file mode 100644
index 0000000..9f60e17
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/regulator-haptic.txt
@@ -0,0 +1,24 @@
+* Requlator Haptic Device Tree Bindings
+
+The regulator haptic driver controlled by voltage of regulator.
+This driver implemented via Force Feedback interface.
+
+Required Properties:
+ - compatible : Should be "regulator-haptic"
+ - haptic-supply : Power supply for the haptic motor.
+ [*] refer Documentation/devicetree/bindings/regulator/regulator.txt
+
+ - max-microvolt : The maximum voltage value supplied to haptic motor.
+ [The unit of the voltage is a micro]
+
+ - min-microvolt : The minimum voltage value in which haptic motor reacts.
+ [The unit of the voltage is a micro]
+
+Example:
+
+ regulator-haptic {
+ compatible = "regulator-haptic";
+ haptic-supply = <&motor_regulator>;
+ max-microvolt = <2700000>;
+ min-microvolt = <1100000>;
+ };
diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 23297ab..e5e556d 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -394,6 +394,17 @@ config INPUT_CM109
To compile this driver as a module, choose M here: the module will be
called cm109.
+config INPUT_REGULATOR_HAPTIC
+ tristate "regulator haptics support"
+ select INPUT_FF_MEMLESS
+ help
+ This option enables device driver support for the haptic controlled
+ by regulator. This driver supports ff-memless interface
+ from input framework.
+
+ To compile this driver as a module, choose M here: the
+ module will be called regulator-haptic.
+
config INPUT_RETU_PWRBUTTON
tristate "Retu Power button Driver"
depends on MFD_RETU
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index 19c7603..1f135af 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -53,6 +53,7 @@ obj-$(CONFIG_INPUT_PMIC8XXX_PWRKEY) += pmic8xxx-pwrkey.o
obj-$(CONFIG_INPUT_POWERMATE) += powermate.o
obj-$(CONFIG_INPUT_PWM_BEEPER) += pwm-beeper.o
obj-$(CONFIG_INPUT_RB532_BUTTON) += rb532_button.o
+obj-$(CONFIG_INPUT_REGULATOR_HAPTIC) += regulator-haptic.o
obj-$(CONFIG_INPUT_RETU_PWRBUTTON) += retu-pwrbutton.o
obj-$(CONFIG_INPUT_GPIO_ROTARY_ENCODER) += rotary_encoder.o
obj-$(CONFIG_INPUT_SGI_BTNS) += sgi_btns.o
diff --git a/drivers/input/misc/regulator-haptic.c b/drivers/input/misc/regulator-haptic.c
new file mode 100644
index 0000000..1a83ecb
--- /dev/null
+++ b/drivers/input/misc/regulator-haptic.c
@@ -0,0 +1,241 @@
+/*
+ * Regulator haptic driver
+ *
+ * Copyright (c) 2014 Samsung Electronics Co., Ltd.
+ * Author: Jaewon Kim <jaewon02.kim@samsung.com>
+ * Author: Hyunhee Kim <hyunhee.kim@samsung.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
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/input.h>
+#include <linux/slab.h>
+#include <linux/regulator/consumer.h>
+#include <linux/of.h>
+
+#define MAX_MAGNITUDE_SHIFT 16
+
+struct regulator_haptic {
+ struct device *dev;
+ struct input_dev *input_dev;
+ struct regulator *regulator;
+ struct work_struct work;
+
+ bool enabled;
+ bool suspend_state;
+ unsigned int max_volt;
+ unsigned int min_volt;
+ unsigned int intensity;
+ unsigned int magnitude;
+};
+
+static void regulator_haptic_enable(struct regulator_haptic *haptic)
+{
+ int error;
+
+ if (haptic->enabled)
+ return;
+
+ error = regulator_enable(haptic->regulator);
+ if (error) {
+ dev_err(haptic->dev, "cannot enable regulator\n");
+ return;
+ }
+
+ haptic->enabled = true;
+}
+
+static void regulator_haptic_disable(struct regulator_haptic *haptic)
+{
+ int error;
+
+ if (!haptic->enabled)
+ return;
+
+ error = regulator_disable(haptic->regulator);
+ if (error) {
+ dev_err(haptic->dev, "cannot disable regulator\n");
+ return;
+ }
+
+ haptic->enabled = false;
+}
+
+static void regulator_haptic_work(struct work_struct *work)
+{
+ struct regulator_haptic *haptic = container_of(work,
+ struct regulator_haptic, work);
+ int error;
+
+ error = regulator_set_voltage(haptic->regulator,
+ haptic->intensity + haptic->min_volt, haptic->max_volt);
+ if (error) {
+ dev_err(haptic->dev, "cannot set regulator voltage\n");
+ return;
+ }
+
+ if (haptic->magnitude)
+ regulator_haptic_enable(haptic);
+ else
+ regulator_haptic_disable(haptic);
+}
+
+static int regulator_haptic_play_effect(struct input_dev *input, void *data,
+ struct ff_effect *effect)
+{
+ struct regulator_haptic *haptic = input_get_drvdata(input);
+ u64 volt_mag_multi;
+
+ haptic->magnitude = effect->u.rumble.strong_magnitude;
+ if (!haptic->magnitude)
+ haptic->magnitude = effect->u.rumble.weak_magnitude;
+
+
+ volt_mag_multi = (u64)(haptic->max_volt - haptic->min_volt) *
+ haptic->magnitude;
+ haptic->intensity = (unsigned int)(volt_mag_multi >>
+ MAX_MAGNITUDE_SHIFT);
+
+ schedule_work(&haptic->work);
+
+ return 0;
+}
+
+static void regulator_haptic_close(struct input_dev *input)
+{
+ struct regulator_haptic *haptic = input_get_drvdata(input);
+
+ cancel_work_sync(&haptic->work);
+ regulator_haptic_disable(haptic);
+}
+
+static int regulator_haptic_parse_dt(struct regulator_haptic *haptic)
+{
+ struct device_node *node = haptic->dev->of_node;
+ int error;
+
+ error = of_property_read_u32(node, "max-microvolt", &haptic->max_volt);
+ if (error) {
+ dev_err(haptic->dev, "cannot parse max-microvolt\n");
+ return error;
+ }
+
+ error = of_property_read_u32(node, "min-microvolt", &haptic->min_volt);
+ if (error) {
+ dev_err(haptic->dev, "cannot parse min-microvolt\n");
+ return error;
+ }
+
+ return 0;
+}
+
+static int regulator_haptic_probe(struct platform_device *pdev)
+{
+ struct regulator_haptic *haptic;
+ struct input_dev *input_dev;
+ int error;
+
+ haptic = devm_kzalloc(&pdev->dev, sizeof(*haptic), GFP_KERNEL);
+ if (!haptic)
+ return -ENOMEM;
+
+ haptic->dev = &pdev->dev;
+ haptic->enabled = false;
+ haptic->suspend_state = false;
+ INIT_WORK(&haptic->work, regulator_haptic_work);
+
+ error = regulator_haptic_parse_dt(haptic);
+ if (error) {
+ dev_err(&pdev->dev, "failed to parse device tree\n");
+ return error;
+ }
+
+ haptic->regulator = devm_regulator_get(&pdev->dev, "haptic");
+ if (IS_ERR(haptic->regulator)) {
+ dev_err(&pdev->dev, "failed to get regulator\n");
+ return PTR_ERR(haptic->regulator);
+ }
+
+ input_dev = devm_input_allocate_device(&pdev->dev);
+ if (!input_dev)
+ return -ENOMEM;
+
+ haptic->input_dev = input_dev;
+ haptic->input_dev->name = "regulator-haptic";
+ haptic->input_dev->dev.parent = &pdev->dev;
+ haptic->input_dev->close = regulator_haptic_close;
+ input_set_drvdata(haptic->input_dev, haptic);
+ input_set_capability(haptic->input_dev, EV_FF, FF_RUMBLE);
+
+ error = input_ff_create_memless(input_dev, NULL,
+ regulator_haptic_play_effect);
+ if (error) {
+ dev_err(&pdev->dev, "failed to create force-feedback\n");
+ return error;
+ }
+
+ error = input_register_device(haptic->input_dev);
+ if (error) {
+ dev_err(&pdev->dev, "failed to register input device\n");
+ return error;
+ }
+
+ platform_set_drvdata(pdev, haptic);
+
+ return 0;
+}
+
+static int __maybe_unused regulator_haptic_suspend(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct regulator_haptic *haptic = platform_get_drvdata(pdev);
+
+ if (haptic->enabled) {
+ regulator_haptic_disable(haptic);
+ haptic->suspend_state = true;
+ }
+
+ return 0;
+}
+
+static int __maybe_unused regulator_haptic_resume(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct regulator_haptic *haptic = platform_get_drvdata(pdev);
+
+ if (haptic->enabled) {
+ regulator_haptic_enable(haptic);
+ haptic->suspend_state = false;
+ }
+
+ return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(regulator_haptic_pm_ops,
+ regulator_haptic_suspend, regulator_haptic_resume);
+
+static struct of_device_id regulator_haptic_dt_match[] = {
+ { .compatible = "regulator-haptic" },
+ {},
+};
+
+static struct platform_driver regulator_haptic_driver = {
+ .probe = regulator_haptic_probe,
+ .driver = {
+ .name = "regulator-haptic",
+ .owner = THIS_MODULE,
+ .of_match_table = regulator_haptic_dt_match,
+ .pm = ®ulator_haptic_pm_ops,
+ },
+};
+module_platform_driver(regulator_haptic_driver);
+
+MODULE_AUTHOR("Jaewon Kim <jaewon02.kim@samsung.com>");
+MODULE_AUTHOR("Hyunhee Kim <hyunhee.kim@samsung.com>");
+MODULE_ALIAS("platform:regulator-haptic");
+MODULE_DESCRIPTION("Regulator haptic driver");
+MODULE_LICENSE("GPL");
--
1.7.9.5
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/2] ARM: dts: Add regulator-haptic device node for exynos3250-rinato
2014-11-20 13:31 [PATCH 0/2] Add regulator-haptic driver Jaewon Kim
2014-11-20 13:31 ` [PATCH 1/2] Input: add regulator haptic driver Jaewon Kim
@ 2014-11-20 13:31 ` Jaewon Kim
1 sibling, 0 replies; 10+ messages in thread
From: Jaewon Kim @ 2014-11-20 13:31 UTC (permalink / raw)
To: Kukjin Kim, Dmitry Torokhov
Cc: Chanwoo Choi, linux-kernel, linux-input, linux-samsung-soc, Jaewon Kim
This patch adds regulator-haptic device node controlled by regulator.
Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com>
---
arch/arm/boot/dts/exynos3250-rinato.dts | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/arm/boot/dts/exynos3250-rinato.dts b/arch/arm/boot/dts/exynos3250-rinato.dts
index 84380fa..8172815 100644
--- a/arch/arm/boot/dts/exynos3250-rinato.dts
+++ b/arch/arm/boot/dts/exynos3250-rinato.dts
@@ -104,6 +104,13 @@
};
};
};
+
+ regulator-haptic {
+ compatible = "regulator-haptic";
+ haptic-supply = <&motor_reg>;
+ min-microvolt = <1100000>;
+ max-microvolt = <2700000>;
+ };
};
&adc {
--
1.7.9.5
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] Input: add regulator haptic driver
2014-11-20 13:31 ` [PATCH 1/2] Input: add regulator haptic driver Jaewon Kim
@ 2014-11-20 14:33 ` Pankaj Dubey
2014-11-20 15:09 ` Dan Murphy
2014-11-21 1:22 ` Jaewon Kim
[not found] ` <20141128125051.GA3643@sirena.org.uk>
1 sibling, 2 replies; 10+ messages in thread
From: Pankaj Dubey @ 2014-11-20 14:33 UTC (permalink / raw)
To: Jaewon Kim
Cc: Kukjin Kim, Dmitry Torokhov, Chanwoo Choi, linux-kernel,
linux-input, linux-samsung-soc, Hyunhee Kim
Hi Jaewon,
On 20 November 2014 19:01, Jaewon Kim <jaewon02.kim@samsung.com> wrote:
> This patch adds support for haptic driver controlled by
> voltage of regulator. And this driver support for
> Force Feedback interface from input framework
>
> Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com>
> Signed-off-by: Hyunhee Kim <hyunhee.kim@samsung.com>
> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
> .../devicetree/bindings/input/regulator-haptic.txt | 24 ++
> drivers/input/misc/Kconfig | 11 +
> drivers/input/misc/Makefile | 1 +
> drivers/input/misc/regulator-haptic.c | 241 ++++++++++++++++++++
> 4 files changed, 277 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/input/regulator-haptic.txt
> create mode 100644 drivers/input/misc/regulator-haptic.c
>
> diff --git a/Documentation/devicetree/bindings/input/regulator-haptic.txt b/Documentation/devicetree/bindings/input/regulator-haptic.txt
> new file mode 100644
> index 0000000..9f60e17
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/regulator-haptic.txt
> @@ -0,0 +1,24 @@
> +* Requlator Haptic Device Tree Bindings
> +
> +The regulator haptic driver controlled by voltage of regulator.
> +This driver implemented via Force Feedback interface.
> +
> +Required Properties:
> + - compatible : Should be "regulator-haptic"
> + - haptic-supply : Power supply for the haptic motor.
> + [*] refer Documentation/devicetree/bindings/regulator/regulator.txt
> +
> + - max-microvolt : The maximum voltage value supplied to haptic motor.
> + [The unit of the voltage is a micro]
> +
> + - min-microvolt : The minimum voltage value in which haptic motor reacts.
> + [The unit of the voltage is a micro]
> +
> +Example:
> +
> + regulator-haptic {
> + compatible = "regulator-haptic";
> + haptic-supply = <&motor_regulator>;
> + max-microvolt = <2700000>;
> + min-microvolt = <1100000>;
> + };
> diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
> index 23297ab..e5e556d 100644
> --- a/drivers/input/misc/Kconfig
> +++ b/drivers/input/misc/Kconfig
> @@ -394,6 +394,17 @@ config INPUT_CM109
> To compile this driver as a module, choose M here: the module will be
> called cm109.
>
> +config INPUT_REGULATOR_HAPTIC
> + tristate "regulator haptics support"
> + select INPUT_FF_MEMLESS
> + help
> + This option enables device driver support for the haptic controlled
> + by regulator. This driver supports ff-memless interface
> + from input framework.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called regulator-haptic.
> +
> config INPUT_RETU_PWRBUTTON
> tristate "Retu Power button Driver"
> depends on MFD_RETU
> diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
> index 19c7603..1f135af 100644
> --- a/drivers/input/misc/Makefile
> +++ b/drivers/input/misc/Makefile
> @@ -53,6 +53,7 @@ obj-$(CONFIG_INPUT_PMIC8XXX_PWRKEY) += pmic8xxx-pwrkey.o
> obj-$(CONFIG_INPUT_POWERMATE) += powermate.o
> obj-$(CONFIG_INPUT_PWM_BEEPER) += pwm-beeper.o
> obj-$(CONFIG_INPUT_RB532_BUTTON) += rb532_button.o
> +obj-$(CONFIG_INPUT_REGULATOR_HAPTIC) += regulator-haptic.o
> obj-$(CONFIG_INPUT_RETU_PWRBUTTON) += retu-pwrbutton.o
> obj-$(CONFIG_INPUT_GPIO_ROTARY_ENCODER) += rotary_encoder.o
> obj-$(CONFIG_INPUT_SGI_BTNS) += sgi_btns.o
> diff --git a/drivers/input/misc/regulator-haptic.c b/drivers/input/misc/regulator-haptic.c
> new file mode 100644
> index 0000000..1a83ecb
> --- /dev/null
> +++ b/drivers/input/misc/regulator-haptic.c
> @@ -0,0 +1,241 @@
> +/*
> + * Regulator haptic driver
> + *
> + * Copyright (c) 2014 Samsung Electronics Co., Ltd.
> + * Author: Jaewon Kim <jaewon02.kim@samsung.com>
> + * Author: Hyunhee Kim <hyunhee.kim@samsung.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
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/input.h>
> +#include <linux/slab.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/of.h>
> +
> +#define MAX_MAGNITUDE_SHIFT 16
> +
> +struct regulator_haptic {
> + struct device *dev;
> + struct input_dev *input_dev;
> + struct regulator *regulator;
> + struct work_struct work;
> +
> + bool enabled;
> + bool suspend_state;
> + unsigned int max_volt;
> + unsigned int min_volt;
> + unsigned int intensity;
> + unsigned int magnitude;
> +};
> +
> +static void regulator_haptic_enable(struct regulator_haptic *haptic)
> +{
> + int error;
> +
> + if (haptic->enabled)
> + return;
> +
> + error = regulator_enable(haptic->regulator);
> + if (error) {
> + dev_err(haptic->dev, "cannot enable regulator\n");
> + return;
> + }
> +
> + haptic->enabled = true;
> +}
> +
> +static void regulator_haptic_disable(struct regulator_haptic *haptic)
> +{
> + int error;
> +
> + if (!haptic->enabled)
> + return;
> +
> + error = regulator_disable(haptic->regulator);
> + if (error) {
> + dev_err(haptic->dev, "cannot disable regulator\n");
> + return;
> + }
> +
> + haptic->enabled = false;
> +}
> +
> +static void regulator_haptic_work(struct work_struct *work)
> +{
> + struct regulator_haptic *haptic = container_of(work,
> + struct regulator_haptic, work);
> + int error;
> +
> + error = regulator_set_voltage(haptic->regulator,
> + haptic->intensity + haptic->min_volt, haptic->max_volt);
> + if (error) {
> + dev_err(haptic->dev, "cannot set regulator voltage\n");
> + return;
> + }
> +
> + if (haptic->magnitude)
> + regulator_haptic_enable(haptic);
> + else
> + regulator_haptic_disable(haptic);
> +}
> +
> +static int regulator_haptic_play_effect(struct input_dev *input, void *data,
> + struct ff_effect *effect)
> +{
> + struct regulator_haptic *haptic = input_get_drvdata(input);
> + u64 volt_mag_multi;
> +
> + haptic->magnitude = effect->u.rumble.strong_magnitude;
> + if (!haptic->magnitude)
> + haptic->magnitude = effect->u.rumble.weak_magnitude;
> +
> +
> + volt_mag_multi = (u64)(haptic->max_volt - haptic->min_volt) *
> + haptic->magnitude;
> + haptic->intensity = (unsigned int)(volt_mag_multi >>
> + MAX_MAGNITUDE_SHIFT);
> +
> + schedule_work(&haptic->work);
> +
> + return 0;
> +}
> +
> +static void regulator_haptic_close(struct input_dev *input)
> +{
> + struct regulator_haptic *haptic = input_get_drvdata(input);
> +
> + cancel_work_sync(&haptic->work);
> + regulator_haptic_disable(haptic);
> +}
> +
> +static int regulator_haptic_parse_dt(struct regulator_haptic *haptic)
> +{
> + struct device_node *node = haptic->dev->of_node;
> + int error;
> +
> + error = of_property_read_u32(node, "max-microvolt", &haptic->max_volt);
> + if (error) {
> + dev_err(haptic->dev, "cannot parse max-microvolt\n");
> + return error;
> + }
> +
> + error = of_property_read_u32(node, "min-microvolt", &haptic->min_volt);
> + if (error) {
> + dev_err(haptic->dev, "cannot parse min-microvolt\n");
> + return error;
> + }
> +
> + return 0;
> +}
> +
> +static int regulator_haptic_probe(struct platform_device *pdev)
> +{
> + struct regulator_haptic *haptic;
> + struct input_dev *input_dev;
> + int error;
> +
> + haptic = devm_kzalloc(&pdev->dev, sizeof(*haptic), GFP_KERNEL);
> + if (!haptic)
> + return -ENOMEM;
> +
> + haptic->dev = &pdev->dev;
> + haptic->enabled = false;
> + haptic->suspend_state = false;
> + INIT_WORK(&haptic->work, regulator_haptic_work);
> +
> + error = regulator_haptic_parse_dt(haptic);
> + if (error) {
> + dev_err(&pdev->dev, "failed to parse device tree\n");
> + return error;
> + }
> +
> + haptic->regulator = devm_regulator_get(&pdev->dev, "haptic");
> + if (IS_ERR(haptic->regulator)) {
> + dev_err(&pdev->dev, "failed to get regulator\n");
> + return PTR_ERR(haptic->regulator);
> + }
> +
> + input_dev = devm_input_allocate_device(&pdev->dev);
> + if (!input_dev)
> + return -ENOMEM;
> +
> + haptic->input_dev = input_dev;
> + haptic->input_dev->name = "regulator-haptic";
> + haptic->input_dev->dev.parent = &pdev->dev;
> + haptic->input_dev->close = regulator_haptic_close;
> + input_set_drvdata(haptic->input_dev, haptic);
> + input_set_capability(haptic->input_dev, EV_FF, FF_RUMBLE);
> +
> + error = input_ff_create_memless(input_dev, NULL,
> + regulator_haptic_play_effect);
> + if (error) {
> + dev_err(&pdev->dev, "failed to create force-feedback\n");
> + return error;
> + }
> +
> + error = input_register_device(haptic->input_dev);
> + if (error) {
> + dev_err(&pdev->dev, "failed to register input device\n");
> + return error;
> + }
> +
> + platform_set_drvdata(pdev, haptic);
> +
> + return 0;
> +}
> +
> +static int __maybe_unused regulator_haptic_suspend(struct device *dev)
> +{
> + struct platform_device *pdev = to_platform_device(dev);
> + struct regulator_haptic *haptic = platform_get_drvdata(pdev);
> +
> + if (haptic->enabled) {
> + regulator_haptic_disable(haptic);
> + haptic->suspend_state = true;
> + }
> +
> + return 0;
> +}
> +
> +static int __maybe_unused regulator_haptic_resume(struct device *dev)
> +{
> + struct platform_device *pdev = to_platform_device(dev);
> + struct regulator_haptic *haptic = platform_get_drvdata(pdev);
> +
> + if (haptic->enabled) {
> + regulator_haptic_enable(haptic);
> + haptic->suspend_state = false;
> + }
> +
> + return 0;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(regulator_haptic_pm_ops,
> + regulator_haptic_suspend, regulator_haptic_resume);
> +
> +static struct of_device_id regulator_haptic_dt_match[] = {
> + { .compatible = "regulator-haptic" },
> + {},
> +};
> +
> +static struct platform_driver regulator_haptic_driver = {
> + .probe = regulator_haptic_probe,
> + .driver = {
> + .name = "regulator-haptic",
> + .owner = THIS_MODULE,
.owner is no more required for drivers using module_platform_driver()
Thanks,
Pankaj Dubey
> + .of_match_table = regulator_haptic_dt_match,
> + .pm = ®ulator_haptic_pm_ops,
> + },
> +};
> +module_platform_driver(regulator_haptic_driver);
> +
> +MODULE_AUTHOR("Jaewon Kim <jaewon02.kim@samsung.com>");
> +MODULE_AUTHOR("Hyunhee Kim <hyunhee.kim@samsung.com>");
> +MODULE_ALIAS("platform:regulator-haptic");
> +MODULE_DESCRIPTION("Regulator haptic driver");
> +MODULE_LICENSE("GPL");
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" 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] 10+ messages in thread
* Re: [PATCH 1/2] Input: add regulator haptic driver
2014-11-20 14:33 ` Pankaj Dubey
@ 2014-11-20 15:09 ` Dan Murphy
2014-11-21 1:22 ` Jaewon Kim
1 sibling, 0 replies; 10+ messages in thread
From: Dan Murphy @ 2014-11-20 15:09 UTC (permalink / raw)
To: Pankaj Dubey, Jaewon Kim
Cc: Kukjin Kim, Dmitry Torokhov, Chanwoo Choi, linux-kernel,
linux-input, linux-samsung-soc, Hyunhee Kim
Hi
On 11/20/2014 08:33 AM, Pankaj Dubey wrote:
> Hi Jaewon,
>
> On 20 November 2014 19:01, Jaewon Kim <jaewon02.kim@samsung.com> wrote:
>> This patch adds support for haptic driver controlled by
>> voltage of regulator. And this driver support for
>> Force Feedback interface from input framework
>>
>> Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com>
>> Signed-off-by: Hyunhee Kim <hyunhee.kim@samsung.com>
>> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
>> ---
>> .../devicetree/bindings/input/regulator-haptic.txt | 24 ++
>> drivers/input/misc/Kconfig | 11 +
>> drivers/input/misc/Makefile | 1 +
>> drivers/input/misc/regulator-haptic.c | 241 ++++++++++++++++++++
>> 4 files changed, 277 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/input/regulator-haptic.txt
>> create mode 100644 drivers/input/misc/regulator-haptic.c
>>
>> diff --git a/Documentation/devicetree/bindings/input/regulator-haptic.txt b/Documentation/devicetree/bindings/input/regulator-haptic.txt
>> new file mode 100644
>> index 0000000..9f60e17
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/input/regulator-haptic.txt
>> @@ -0,0 +1,24 @@
>> +* Requlator Haptic Device Tree Bindings
>> +
>> +The regulator haptic driver controlled by voltage of regulator.
>> +This driver implemented via Force Feedback interface.
>> +
>> +Required Properties:
>> + - compatible : Should be "regulator-haptic"
>> + - haptic-supply : Power supply for the haptic motor.
>> + [*] refer Documentation/devicetree/bindings/regulator/regulator.txt
>> +
>> + - max-microvolt : The maximum voltage value supplied to haptic motor.
>> + [The unit of the voltage is a micro]
>> +
>> + - min-microvolt : The minimum voltage value in which haptic motor reacts.
>> + [The unit of the voltage is a micro]
>> +
>> +Example:
>> +
>> + regulator-haptic {
Should this be more about the function and not the driver name?
Somehting like
haptics {
>> + compatible = "regulator-haptic";
>> + haptic-supply = <&motor_regulator>;
>> + max-microvolt = <2700000>;
>> + min-microvolt = <1100000>;
>> + };
>> diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
>> index 23297ab..e5e556d 100644
>> --- a/drivers/input/misc/Kconfig
>> +++ b/drivers/input/misc/Kconfig
>> @@ -394,6 +394,17 @@ config INPUT_CM109
>> To compile this driver as a module, choose M here: the module will be
>> called cm109.
>>
>> +config INPUT_REGULATOR_HAPTIC
>> + tristate "regulator haptics support"
>> + select INPUT_FF_MEMLESS
>> + help
>> + This option enables device driver support for the haptic controlled
>> + by regulator. This driver supports ff-memless interface
>> + from input framework.
>> +
>> + To compile this driver as a module, choose M here: the
>> + module will be called regulator-haptic.
>> +
>> config INPUT_RETU_PWRBUTTON
>> tristate "Retu Power button Driver"
>> depends on MFD_RETU
>> diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
>> index 19c7603..1f135af 100644
>> --- a/drivers/input/misc/Makefile
>> +++ b/drivers/input/misc/Makefile
>> @@ -53,6 +53,7 @@ obj-$(CONFIG_INPUT_PMIC8XXX_PWRKEY) += pmic8xxx-pwrkey.o
>> obj-$(CONFIG_INPUT_POWERMATE) += powermate.o
>> obj-$(CONFIG_INPUT_PWM_BEEPER) += pwm-beeper.o
>> obj-$(CONFIG_INPUT_RB532_BUTTON) += rb532_button.o
>> +obj-$(CONFIG_INPUT_REGULATOR_HAPTIC) += regulator-haptic.o
>> obj-$(CONFIG_INPUT_RETU_PWRBUTTON) += retu-pwrbutton.o
>> obj-$(CONFIG_INPUT_GPIO_ROTARY_ENCODER) += rotary_encoder.o
>> obj-$(CONFIG_INPUT_SGI_BTNS) += sgi_btns.o
>> diff --git a/drivers/input/misc/regulator-haptic.c b/drivers/input/misc/regulator-haptic.c
>> new file mode 100644
>> index 0000000..1a83ecb
>> --- /dev/null
>> +++ b/drivers/input/misc/regulator-haptic.c
>> @@ -0,0 +1,241 @@
>> +/*
>> + * Regulator haptic driver
>> + *
>> + * Copyright (c) 2014 Samsung Electronics Co., Ltd.
>> + * Author: Jaewon Kim <jaewon02.kim@samsung.com>
>> + * Author: Hyunhee Kim <hyunhee.kim@samsung.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
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/input.h>
>> +#include <linux/slab.h>
>> +#include <linux/regulator/consumer.h>
>> +#include <linux/of.h>
>> +
>> +#define MAX_MAGNITUDE_SHIFT 16
>> +
>> +struct regulator_haptic {
>> + struct device *dev;
>> + struct input_dev *input_dev;
>> + struct regulator *regulator;
>> + struct work_struct work;
>> +
>> + bool enabled;
>> + bool suspend_state;
I don't understand what you are tracking here. You only set it and unset
this value but never do any checking
>> + unsigned int max_volt;
>> + unsigned int min_volt;
>> + unsigned int intensity;
>> + unsigned int magnitude;
>> +};
>> +
>> +static void regulator_haptic_enable(struct regulator_haptic *haptic)
>> +{
>> + int error;
>> +
>> + if (haptic->enabled)
>> + return;
>> +
>> + error = regulator_enable(haptic->regulator);
>> + if (error) {
>> + dev_err(haptic->dev, "cannot enable regulator\n");
>> + return;
>> + }
>> +
>> + haptic->enabled = true;
>> +}
>> +
>> +static void regulator_haptic_disable(struct regulator_haptic *haptic)
>> +{
>> + int error;
>> +
>> + if (!haptic->enabled)
>> + return;
>> +
>> + error = regulator_disable(haptic->regulator);
>> + if (error) {
>> + dev_err(haptic->dev, "cannot disable regulator\n");
>> + return;
>> + }
>> +
>> + haptic->enabled = false;
>> +}
Can the enable/disable just be one function with an argument?
Then you can centralize the check and reduce the code footprint.
>> +
>> +static void regulator_haptic_work(struct work_struct *work)
>> +{
>> + struct regulator_haptic *haptic = container_of(work,
>> + struct regulator_haptic, work);
>> + int error;
>> +
>> + error = regulator_set_voltage(haptic->regulator,
>> + haptic->intensity + haptic->min_volt, haptic->max_volt);
>> + if (error) {
>> + dev_err(haptic->dev, "cannot set regulator voltage\n");
>> + return;
>> + }
>> +
>> + if (haptic->magnitude)
>> + regulator_haptic_enable(haptic);
>> + else
>> + regulator_haptic_disable(haptic);
>> +}
>> +
>> +static int regulator_haptic_play_effect(struct input_dev *input, void *data,
>> + struct ff_effect *effect)
>> +{
>> + struct regulator_haptic *haptic = input_get_drvdata(input);
>> + u64 volt_mag_multi;
>> +
>> + haptic->magnitude = effect->u.rumble.strong_magnitude;
>> + if (!haptic->magnitude)
>> + haptic->magnitude = effect->u.rumble.weak_magnitude;
>> +
>> +
>> + volt_mag_multi = (u64)(haptic->max_volt - haptic->min_volt) *
>> + haptic->magnitude;
>> + haptic->intensity = (unsigned int)(volt_mag_multi >>
>> + MAX_MAGNITUDE_SHIFT);
>> +
>> + schedule_work(&haptic->work);
>> +
>> + return 0;
>> +}
>> +
>> +static void regulator_haptic_close(struct input_dev *input)
>> +{
>> + struct regulator_haptic *haptic = input_get_drvdata(input);
>> +
>> + cancel_work_sync(&haptic->work);
>> + regulator_haptic_disable(haptic);
>> +}
>> +
>> +static int regulator_haptic_parse_dt(struct regulator_haptic *haptic)
>> +{
>> + struct device_node *node = haptic->dev->of_node;
>> + int error;
>> +
>> + error = of_property_read_u32(node, "max-microvolt", &haptic->max_volt);
>> + if (error) {
>> + dev_err(haptic->dev, "cannot parse max-microvolt\n");
>> + return error;
>> + }
>> +
>> + error = of_property_read_u32(node, "min-microvolt", &haptic->min_volt);
>> + if (error) {
>> + dev_err(haptic->dev, "cannot parse min-microvolt\n");
>> + return error;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int regulator_haptic_probe(struct platform_device *pdev)
>> +{
>> + struct regulator_haptic *haptic;
>> + struct input_dev *input_dev;
>> + int error;
>> +
>> + haptic = devm_kzalloc(&pdev->dev, sizeof(*haptic), GFP_KERNEL);
>> + if (!haptic)
>> + return -ENOMEM;
>> +
>> + haptic->dev = &pdev->dev;
>> + haptic->enabled = false;
>> + haptic->suspend_state = false;
>> + INIT_WORK(&haptic->work, regulator_haptic_work);
>> +
>> + error = regulator_haptic_parse_dt(haptic);
Do you need platform data support for platforms that do not currently support
dt?
>> + if (error) {
>> + dev_err(&pdev->dev, "failed to parse device tree\n");
>> + return error;
>> + }
>> +
>> + haptic->regulator = devm_regulator_get(&pdev->dev, "haptic");
>> + if (IS_ERR(haptic->regulator)) {
>> + dev_err(&pdev->dev, "failed to get regulator\n");
>> + return PTR_ERR(haptic->regulator);
>> + }
>> +
>> + input_dev = devm_input_allocate_device(&pdev->dev);
>> + if (!input_dev)
>> + return -ENOMEM;
>> +
>> + haptic->input_dev = input_dev;
>> + haptic->input_dev->name = "regulator-haptic";
>> + haptic->input_dev->dev.parent = &pdev->dev;
>> + haptic->input_dev->close = regulator_haptic_close;
>> + input_set_drvdata(haptic->input_dev, haptic);
>> + input_set_capability(haptic->input_dev, EV_FF, FF_RUMBLE);
>> +
>> + error = input_ff_create_memless(input_dev, NULL,
>> + regulator_haptic_play_effect);
>> + if (error) {
>> + dev_err(&pdev->dev, "failed to create force-feedback\n");
>> + return error;
>> + }
>> +
>> + error = input_register_device(haptic->input_dev);
>> + if (error) {
>> + dev_err(&pdev->dev, "failed to register input device\n");
>> + return error;
>> + }
>> +
>> + platform_set_drvdata(pdev, haptic);
>> +
>> + return 0;
>> +}
>> +
>> +static int __maybe_unused regulator_haptic_suspend(struct device *dev)
>> +{
>> + struct platform_device *pdev = to_platform_device(dev);
>> + struct regulator_haptic *haptic = platform_get_drvdata(pdev);
>> +
>> + if (haptic->enabled) {
>> + regulator_haptic_disable(haptic);
>> + haptic->suspend_state = true;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int __maybe_unused regulator_haptic_resume(struct device *dev)
>> +{
>> + struct platform_device *pdev = to_platform_device(dev);
>> + struct regulator_haptic *haptic = platform_get_drvdata(pdev);
>> +
>> + if (haptic->enabled) {
>> + regulator_haptic_enable(haptic);
We enable the regulator when we do the haptics work if magnitude is > 0.
And during suspend haptic->enabled may get set to false if it is true
so wouldn't haptic->enabled always be false here?
Plus do we want to continue the haptic response when coming out of suspend state?
>> + haptic->suspend_state = false;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static SIMPLE_DEV_PM_OPS(regulator_haptic_pm_ops,
>> + regulator_haptic_suspend, regulator_haptic_resume);
>> +
>> +static struct of_device_id regulator_haptic_dt_match[] = {
>> + { .compatible = "regulator-haptic" },
>> + {},
>> +};
>> +
>> +static struct platform_driver regulator_haptic_driver = {
>> + .probe = regulator_haptic_probe,
>> + .driver = {
>> + .name = "regulator-haptic",
>> + .owner = THIS_MODULE,
> .owner is no more required for drivers using module_platform_driver()
>
> Thanks,
> Pankaj Dubey
>
>> + .of_match_table = regulator_haptic_dt_match,
>> + .pm = ®ulator_haptic_pm_ops,
>> + },
>> +};
>> +module_platform_driver(regulator_haptic_driver);
>> +
>> +MODULE_AUTHOR("Jaewon Kim <jaewon02.kim@samsung.com>");
>> +MODULE_AUTHOR("Hyunhee Kim <hyunhee.kim@samsung.com>");
>> +MODULE_ALIAS("platform:regulator-haptic");
>> +MODULE_DESCRIPTION("Regulator haptic driver");
>> +MODULE_LICENSE("GPL");
>> --
>> 1.7.9.5
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
> --
> 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
--
------------------
Dan Murphy
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] Input: add regulator haptic driver
@ 2014-11-20 15:09 ` Dan Murphy
0 siblings, 0 replies; 10+ messages in thread
From: Dan Murphy @ 2014-11-20 15:09 UTC (permalink / raw)
To: Pankaj Dubey, Jaewon Kim
Cc: Kukjin Kim, Dmitry Torokhov, Chanwoo Choi, linux-kernel,
linux-input, linux-samsung-soc, Hyunhee Kim
Hi
On 11/20/2014 08:33 AM, Pankaj Dubey wrote:
> Hi Jaewon,
>
> On 20 November 2014 19:01, Jaewon Kim <jaewon02.kim@samsung.com> wrote:
>> This patch adds support for haptic driver controlled by
>> voltage of regulator. And this driver support for
>> Force Feedback interface from input framework
>>
>> Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com>
>> Signed-off-by: Hyunhee Kim <hyunhee.kim@samsung.com>
>> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
>> ---
>> .../devicetree/bindings/input/regulator-haptic.txt | 24 ++
>> drivers/input/misc/Kconfig | 11 +
>> drivers/input/misc/Makefile | 1 +
>> drivers/input/misc/regulator-haptic.c | 241 ++++++++++++++++++++
>> 4 files changed, 277 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/input/regulator-haptic.txt
>> create mode 100644 drivers/input/misc/regulator-haptic.c
>>
>> diff --git a/Documentation/devicetree/bindings/input/regulator-haptic.txt b/Documentation/devicetree/bindings/input/regulator-haptic.txt
>> new file mode 100644
>> index 0000000..9f60e17
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/input/regulator-haptic.txt
>> @@ -0,0 +1,24 @@
>> +* Requlator Haptic Device Tree Bindings
>> +
>> +The regulator haptic driver controlled by voltage of regulator.
>> +This driver implemented via Force Feedback interface.
>> +
>> +Required Properties:
>> + - compatible : Should be "regulator-haptic"
>> + - haptic-supply : Power supply for the haptic motor.
>> + [*] refer Documentation/devicetree/bindings/regulator/regulator.txt
>> +
>> + - max-microvolt : The maximum voltage value supplied to haptic motor.
>> + [The unit of the voltage is a micro]
>> +
>> + - min-microvolt : The minimum voltage value in which haptic motor reacts.
>> + [The unit of the voltage is a micro]
>> +
>> +Example:
>> +
>> + regulator-haptic {
Should this be more about the function and not the driver name?
Somehting like
haptics {
>> + compatible = "regulator-haptic";
>> + haptic-supply = <&motor_regulator>;
>> + max-microvolt = <2700000>;
>> + min-microvolt = <1100000>;
>> + };
>> diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
>> index 23297ab..e5e556d 100644
>> --- a/drivers/input/misc/Kconfig
>> +++ b/drivers/input/misc/Kconfig
>> @@ -394,6 +394,17 @@ config INPUT_CM109
>> To compile this driver as a module, choose M here: the module will be
>> called cm109.
>>
>> +config INPUT_REGULATOR_HAPTIC
>> + tristate "regulator haptics support"
>> + select INPUT_FF_MEMLESS
>> + help
>> + This option enables device driver support for the haptic controlled
>> + by regulator. This driver supports ff-memless interface
>> + from input framework.
>> +
>> + To compile this driver as a module, choose M here: the
>> + module will be called regulator-haptic.
>> +
>> config INPUT_RETU_PWRBUTTON
>> tristate "Retu Power button Driver"
>> depends on MFD_RETU
>> diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
>> index 19c7603..1f135af 100644
>> --- a/drivers/input/misc/Makefile
>> +++ b/drivers/input/misc/Makefile
>> @@ -53,6 +53,7 @@ obj-$(CONFIG_INPUT_PMIC8XXX_PWRKEY) += pmic8xxx-pwrkey.o
>> obj-$(CONFIG_INPUT_POWERMATE) += powermate.o
>> obj-$(CONFIG_INPUT_PWM_BEEPER) += pwm-beeper.o
>> obj-$(CONFIG_INPUT_RB532_BUTTON) += rb532_button.o
>> +obj-$(CONFIG_INPUT_REGULATOR_HAPTIC) += regulator-haptic.o
>> obj-$(CONFIG_INPUT_RETU_PWRBUTTON) += retu-pwrbutton.o
>> obj-$(CONFIG_INPUT_GPIO_ROTARY_ENCODER) += rotary_encoder.o
>> obj-$(CONFIG_INPUT_SGI_BTNS) += sgi_btns.o
>> diff --git a/drivers/input/misc/regulator-haptic.c b/drivers/input/misc/regulator-haptic.c
>> new file mode 100644
>> index 0000000..1a83ecb
>> --- /dev/null
>> +++ b/drivers/input/misc/regulator-haptic.c
>> @@ -0,0 +1,241 @@
>> +/*
>> + * Regulator haptic driver
>> + *
>> + * Copyright (c) 2014 Samsung Electronics Co., Ltd.
>> + * Author: Jaewon Kim <jaewon02.kim@samsung.com>
>> + * Author: Hyunhee Kim <hyunhee.kim@samsung.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
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/input.h>
>> +#include <linux/slab.h>
>> +#include <linux/regulator/consumer.h>
>> +#include <linux/of.h>
>> +
>> +#define MAX_MAGNITUDE_SHIFT 16
>> +
>> +struct regulator_haptic {
>> + struct device *dev;
>> + struct input_dev *input_dev;
>> + struct regulator *regulator;
>> + struct work_struct work;
>> +
>> + bool enabled;
>> + bool suspend_state;
I don't understand what you are tracking here. You only set it and unset
this value but never do any checking
>> + unsigned int max_volt;
>> + unsigned int min_volt;
>> + unsigned int intensity;
>> + unsigned int magnitude;
>> +};
>> +
>> +static void regulator_haptic_enable(struct regulator_haptic *haptic)
>> +{
>> + int error;
>> +
>> + if (haptic->enabled)
>> + return;
>> +
>> + error = regulator_enable(haptic->regulator);
>> + if (error) {
>> + dev_err(haptic->dev, "cannot enable regulator\n");
>> + return;
>> + }
>> +
>> + haptic->enabled = true;
>> +}
>> +
>> +static void regulator_haptic_disable(struct regulator_haptic *haptic)
>> +{
>> + int error;
>> +
>> + if (!haptic->enabled)
>> + return;
>> +
>> + error = regulator_disable(haptic->regulator);
>> + if (error) {
>> + dev_err(haptic->dev, "cannot disable regulator\n");
>> + return;
>> + }
>> +
>> + haptic->enabled = false;
>> +}
Can the enable/disable just be one function with an argument?
Then you can centralize the check and reduce the code footprint.
>> +
>> +static void regulator_haptic_work(struct work_struct *work)
>> +{
>> + struct regulator_haptic *haptic = container_of(work,
>> + struct regulator_haptic, work);
>> + int error;
>> +
>> + error = regulator_set_voltage(haptic->regulator,
>> + haptic->intensity + haptic->min_volt, haptic->max_volt);
>> + if (error) {
>> + dev_err(haptic->dev, "cannot set regulator voltage\n");
>> + return;
>> + }
>> +
>> + if (haptic->magnitude)
>> + regulator_haptic_enable(haptic);
>> + else
>> + regulator_haptic_disable(haptic);
>> +}
>> +
>> +static int regulator_haptic_play_effect(struct input_dev *input, void *data,
>> + struct ff_effect *effect)
>> +{
>> + struct regulator_haptic *haptic = input_get_drvdata(input);
>> + u64 volt_mag_multi;
>> +
>> + haptic->magnitude = effect->u.rumble.strong_magnitude;
>> + if (!haptic->magnitude)
>> + haptic->magnitude = effect->u.rumble.weak_magnitude;
>> +
>> +
>> + volt_mag_multi = (u64)(haptic->max_volt - haptic->min_volt) *
>> + haptic->magnitude;
>> + haptic->intensity = (unsigned int)(volt_mag_multi >>
>> + MAX_MAGNITUDE_SHIFT);
>> +
>> + schedule_work(&haptic->work);
>> +
>> + return 0;
>> +}
>> +
>> +static void regulator_haptic_close(struct input_dev *input)
>> +{
>> + struct regulator_haptic *haptic = input_get_drvdata(input);
>> +
>> + cancel_work_sync(&haptic->work);
>> + regulator_haptic_disable(haptic);
>> +}
>> +
>> +static int regulator_haptic_parse_dt(struct regulator_haptic *haptic)
>> +{
>> + struct device_node *node = haptic->dev->of_node;
>> + int error;
>> +
>> + error = of_property_read_u32(node, "max-microvolt", &haptic->max_volt);
>> + if (error) {
>> + dev_err(haptic->dev, "cannot parse max-microvolt\n");
>> + return error;
>> + }
>> +
>> + error = of_property_read_u32(node, "min-microvolt", &haptic->min_volt);
>> + if (error) {
>> + dev_err(haptic->dev, "cannot parse min-microvolt\n");
>> + return error;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int regulator_haptic_probe(struct platform_device *pdev)
>> +{
>> + struct regulator_haptic *haptic;
>> + struct input_dev *input_dev;
>> + int error;
>> +
>> + haptic = devm_kzalloc(&pdev->dev, sizeof(*haptic), GFP_KERNEL);
>> + if (!haptic)
>> + return -ENOMEM;
>> +
>> + haptic->dev = &pdev->dev;
>> + haptic->enabled = false;
>> + haptic->suspend_state = false;
>> + INIT_WORK(&haptic->work, regulator_haptic_work);
>> +
>> + error = regulator_haptic_parse_dt(haptic);
Do you need platform data support for platforms that do not currently support
dt?
>> + if (error) {
>> + dev_err(&pdev->dev, "failed to parse device tree\n");
>> + return error;
>> + }
>> +
>> + haptic->regulator = devm_regulator_get(&pdev->dev, "haptic");
>> + if (IS_ERR(haptic->regulator)) {
>> + dev_err(&pdev->dev, "failed to get regulator\n");
>> + return PTR_ERR(haptic->regulator);
>> + }
>> +
>> + input_dev = devm_input_allocate_device(&pdev->dev);
>> + if (!input_dev)
>> + return -ENOMEM;
>> +
>> + haptic->input_dev = input_dev;
>> + haptic->input_dev->name = "regulator-haptic";
>> + haptic->input_dev->dev.parent = &pdev->dev;
>> + haptic->input_dev->close = regulator_haptic_close;
>> + input_set_drvdata(haptic->input_dev, haptic);
>> + input_set_capability(haptic->input_dev, EV_FF, FF_RUMBLE);
>> +
>> + error = input_ff_create_memless(input_dev, NULL,
>> + regulator_haptic_play_effect);
>> + if (error) {
>> + dev_err(&pdev->dev, "failed to create force-feedback\n");
>> + return error;
>> + }
>> +
>> + error = input_register_device(haptic->input_dev);
>> + if (error) {
>> + dev_err(&pdev->dev, "failed to register input device\n");
>> + return error;
>> + }
>> +
>> + platform_set_drvdata(pdev, haptic);
>> +
>> + return 0;
>> +}
>> +
>> +static int __maybe_unused regulator_haptic_suspend(struct device *dev)
>> +{
>> + struct platform_device *pdev = to_platform_device(dev);
>> + struct regulator_haptic *haptic = platform_get_drvdata(pdev);
>> +
>> + if (haptic->enabled) {
>> + regulator_haptic_disable(haptic);
>> + haptic->suspend_state = true;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int __maybe_unused regulator_haptic_resume(struct device *dev)
>> +{
>> + struct platform_device *pdev = to_platform_device(dev);
>> + struct regulator_haptic *haptic = platform_get_drvdata(pdev);
>> +
>> + if (haptic->enabled) {
>> + regulator_haptic_enable(haptic);
We enable the regulator when we do the haptics work if magnitude is > 0.
And during suspend haptic->enabled may get set to false if it is true
so wouldn't haptic->enabled always be false here?
Plus do we want to continue the haptic response when coming out of suspend state?
>> + haptic->suspend_state = false;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static SIMPLE_DEV_PM_OPS(regulator_haptic_pm_ops,
>> + regulator_haptic_suspend, regulator_haptic_resume);
>> +
>> +static struct of_device_id regulator_haptic_dt_match[] = {
>> + { .compatible = "regulator-haptic" },
>> + {},
>> +};
>> +
>> +static struct platform_driver regulator_haptic_driver = {
>> + .probe = regulator_haptic_probe,
>> + .driver = {
>> + .name = "regulator-haptic",
>> + .owner = THIS_MODULE,
> .owner is no more required for drivers using module_platform_driver()
>
> Thanks,
> Pankaj Dubey
>
>> + .of_match_table = regulator_haptic_dt_match,
>> + .pm = ®ulator_haptic_pm_ops,
>> + },
>> +};
>> +module_platform_driver(regulator_haptic_driver);
>> +
>> +MODULE_AUTHOR("Jaewon Kim <jaewon02.kim@samsung.com>");
>> +MODULE_AUTHOR("Hyunhee Kim <hyunhee.kim@samsung.com>");
>> +MODULE_ALIAS("platform:regulator-haptic");
>> +MODULE_DESCRIPTION("Regulator haptic driver");
>> +MODULE_LICENSE("GPL");
>> --
>> 1.7.9.5
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
> --
> 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
--
------------------
Dan Murphy
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] Input: add regulator haptic driver
2014-11-20 14:33 ` Pankaj Dubey
2014-11-20 15:09 ` Dan Murphy
@ 2014-11-21 1:22 ` Jaewon Kim
1 sibling, 0 replies; 10+ messages in thread
From: Jaewon Kim @ 2014-11-21 1:22 UTC (permalink / raw)
To: Pankaj Dubey
Cc: Kukjin Kim, Dmitry Torokhov, Chanwoo Choi, linux-kernel,
linux-input, linux-samsung-soc, Hyunhee Kim
Hi Pankaj,
2014년 11월 20일 23:33에 Pankaj Dubey 이(가) 쓴 글:
> Hi Jaewon,
>
> On 20 November 2014 19:01, Jaewon Kim <jaewon02.kim@samsung.com> wrote:
>> This patch adds support for haptic driver controlled by
>> voltage of regulator. And this driver support for
>> Force Feedback interface from input framework
>>
>> Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com>
>> Signed-off-by: Hyunhee Kim <hyunhee.kim@samsung.com>
>> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
>> ---
>> .../devicetree/bindings/input/regulator-haptic.txt | 24 ++
>> drivers/input/misc/Kconfig | 11 +
>> drivers/input/misc/Makefile | 1 +
>> drivers/input/misc/regulator-haptic.c | 241 ++++++++++++++++++++
>> 4 files changed, 277 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/input/regulator-haptic.txt
>> create mode 100644 drivers/input/misc/regulator-haptic.c
>>
>> diff --git a/Documentation/devicetree/bindings/input/regulator-haptic.txt b/Documentation/devicetree/bindings/input/regulator-haptic.txt
>> new file mode 100644
>> index 0000000..9f60e17
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/input/regulator-haptic.txt
>> @@ -0,0 +1,24 @@
>> +* Requlator Haptic Device Tree Bindings
>> +
>> +The regulator haptic driver controlled by voltage of regulator.
>> +This driver implemented via Force Feedback interface.
>> +
>> +Required Properties:
>> + - compatible : Should be "regulator-haptic"
>> + - haptic-supply : Power supply for the haptic motor.
>> + [*] refer Documentation/devicetree/bindings/regulator/regulator.txt
>> +
>> + - max-microvolt : The maximum voltage value supplied to haptic motor.
>> + [The unit of the voltage is a micro]
>> +
>> + - min-microvolt : The minimum voltage value in which haptic motor reacts.
>> + [The unit of the voltage is a micro]
>> +
>> +Example:
>> +
>> + regulator-haptic {
>> + compatible = "regulator-haptic";
>> + haptic-supply = <&motor_regulator>;
>> + max-microvolt = <2700000>;
>> + min-microvolt = <1100000>;
>> + };
>> diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
>> index 23297ab..e5e556d 100644
>> --- a/drivers/input/misc/Kconfig
>> +++ b/drivers/input/misc/Kconfig
>> @@ -394,6 +394,17 @@ config INPUT_CM109
>> To compile this driver as a module, choose M here: the module will be
>> called cm109.
>>
>> +config INPUT_REGULATOR_HAPTIC
>> + tristate "regulator haptics support"
>> + select INPUT_FF_MEMLESS
>> + help
>> + This option enables device driver support for the haptic controlled
>> + by regulator. This driver supports ff-memless interface
>> + from input framework.
>> +
>> + To compile this driver as a module, choose M here: the
>> + module will be called regulator-haptic.
>> +
>> config INPUT_RETU_PWRBUTTON
>> tristate "Retu Power button Driver"
>> depends on MFD_RETU
>> diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
>> index 19c7603..1f135af 100644
>> --- a/drivers/input/misc/Makefile
>> +++ b/drivers/input/misc/Makefile
>> @@ -53,6 +53,7 @@ obj-$(CONFIG_INPUT_PMIC8XXX_PWRKEY) += pmic8xxx-pwrkey.o
>> obj-$(CONFIG_INPUT_POWERMATE) += powermate.o
>> obj-$(CONFIG_INPUT_PWM_BEEPER) += pwm-beeper.o
>> obj-$(CONFIG_INPUT_RB532_BUTTON) += rb532_button.o
>> +obj-$(CONFIG_INPUT_REGULATOR_HAPTIC) += regulator-haptic.o
>> obj-$(CONFIG_INPUT_RETU_PWRBUTTON) += retu-pwrbutton.o
>> obj-$(CONFIG_INPUT_GPIO_ROTARY_ENCODER) += rotary_encoder.o
>> obj-$(CONFIG_INPUT_SGI_BTNS) += sgi_btns.o
>> diff --git a/drivers/input/misc/regulator-haptic.c b/drivers/input/misc/regulator-haptic.c
>> new file mode 100644
>> index 0000000..1a83ecb
>> --- /dev/null
>> +++ b/drivers/input/misc/regulator-haptic.c
>> @@ -0,0 +1,241 @@
>> +/*
>> + * Regulator haptic driver
>> + *
>> + * Copyright (c) 2014 Samsung Electronics Co., Ltd.
>> + * Author: Jaewon Kim <jaewon02.kim@samsung.com>
>> + * Author: Hyunhee Kim <hyunhee.kim@samsung.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
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/input.h>
>> +#include <linux/slab.h>
>> +#include <linux/regulator/consumer.h>
>> +#include <linux/of.h>
>> +
>> +#define MAX_MAGNITUDE_SHIFT 16
>> +
>> +struct regulator_haptic {
>> + struct device *dev;
>> + struct input_dev *input_dev;
>> + struct regulator *regulator;
>> + struct work_struct work;
>> +
>> + bool enabled;
>> + bool suspend_state;
>> + unsigned int max_volt;
>> + unsigned int min_volt;
>> + unsigned int intensity;
>> + unsigned int magnitude;
>> +};
>> +
>> +static void regulator_haptic_enable(struct regulator_haptic *haptic)
>> +{
>> + int error;
>> +
>> + if (haptic->enabled)
>> + return;
>> +
>> + error = regulator_enable(haptic->regulator);
>> + if (error) {
>> + dev_err(haptic->dev, "cannot enable regulator\n");
>> + return;
>> + }
>> +
>> + haptic->enabled = true;
>> +}
>> +
>> +static void regulator_haptic_disable(struct regulator_haptic *haptic)
>> +{
>> + int error;
>> +
>> + if (!haptic->enabled)
>> + return;
>> +
>> + error = regulator_disable(haptic->regulator);
>> + if (error) {
>> + dev_err(haptic->dev, "cannot disable regulator\n");
>> + return;
>> + }
>> +
>> + haptic->enabled = false;
>> +}
>> +
>> +static void regulator_haptic_work(struct work_struct *work)
>> +{
>> + struct regulator_haptic *haptic = container_of(work,
>> + struct regulator_haptic, work);
>> + int error;
>> +
>> + error = regulator_set_voltage(haptic->regulator,
>> + haptic->intensity + haptic->min_volt, haptic->max_volt);
>> + if (error) {
>> + dev_err(haptic->dev, "cannot set regulator voltage\n");
>> + return;
>> + }
>> +
>> + if (haptic->magnitude)
>> + regulator_haptic_enable(haptic);
>> + else
>> + regulator_haptic_disable(haptic);
>> +}
>> +
>> +static int regulator_haptic_play_effect(struct input_dev *input, void *data,
>> + struct ff_effect *effect)
>> +{
>> + struct regulator_haptic *haptic = input_get_drvdata(input);
>> + u64 volt_mag_multi;
>> +
>> + haptic->magnitude = effect->u.rumble.strong_magnitude;
>> + if (!haptic->magnitude)
>> + haptic->magnitude = effect->u.rumble.weak_magnitude;
>> +
>> +
>> + volt_mag_multi = (u64)(haptic->max_volt - haptic->min_volt) *
>> + haptic->magnitude;
>> + haptic->intensity = (unsigned int)(volt_mag_multi >>
>> + MAX_MAGNITUDE_SHIFT);
>> +
>> + schedule_work(&haptic->work);
>> +
>> + return 0;
>> +}
>> +
>> +static void regulator_haptic_close(struct input_dev *input)
>> +{
>> + struct regulator_haptic *haptic = input_get_drvdata(input);
>> +
>> + cancel_work_sync(&haptic->work);
>> + regulator_haptic_disable(haptic);
>> +}
>> +
>> +static int regulator_haptic_parse_dt(struct regulator_haptic *haptic)
>> +{
>> + struct device_node *node = haptic->dev->of_node;
>> + int error;
>> +
>> + error = of_property_read_u32(node, "max-microvolt", &haptic->max_volt);
>> + if (error) {
>> + dev_err(haptic->dev, "cannot parse max-microvolt\n");
>> + return error;
>> + }
>> +
>> + error = of_property_read_u32(node, "min-microvolt", &haptic->min_volt);
>> + if (error) {
>> + dev_err(haptic->dev, "cannot parse min-microvolt\n");
>> + return error;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int regulator_haptic_probe(struct platform_device *pdev)
>> +{
>> + struct regulator_haptic *haptic;
>> + struct input_dev *input_dev;
>> + int error;
>> +
>> + haptic = devm_kzalloc(&pdev->dev, sizeof(*haptic), GFP_KERNEL);
>> + if (!haptic)
>> + return -ENOMEM;
>> +
>> + haptic->dev = &pdev->dev;
>> + haptic->enabled = false;
>> + haptic->suspend_state = false;
>> + INIT_WORK(&haptic->work, regulator_haptic_work);
>> +
>> + error = regulator_haptic_parse_dt(haptic);
>> + if (error) {
>> + dev_err(&pdev->dev, "failed to parse device tree\n");
>> + return error;
>> + }
>> +
>> + haptic->regulator = devm_regulator_get(&pdev->dev, "haptic");
>> + if (IS_ERR(haptic->regulator)) {
>> + dev_err(&pdev->dev, "failed to get regulator\n");
>> + return PTR_ERR(haptic->regulator);
>> + }
>> +
>> + input_dev = devm_input_allocate_device(&pdev->dev);
>> + if (!input_dev)
>> + return -ENOMEM;
>> +
>> + haptic->input_dev = input_dev;
>> + haptic->input_dev->name = "regulator-haptic";
>> + haptic->input_dev->dev.parent = &pdev->dev;
>> + haptic->input_dev->close = regulator_haptic_close;
>> + input_set_drvdata(haptic->input_dev, haptic);
>> + input_set_capability(haptic->input_dev, EV_FF, FF_RUMBLE);
>> +
>> + error = input_ff_create_memless(input_dev, NULL,
>> + regulator_haptic_play_effect);
>> + if (error) {
>> + dev_err(&pdev->dev, "failed to create force-feedback\n");
>> + return error;
>> + }
>> +
>> + error = input_register_device(haptic->input_dev);
>> + if (error) {
>> + dev_err(&pdev->dev, "failed to register input device\n");
>> + return error;
>> + }
>> +
>> + platform_set_drvdata(pdev, haptic);
>> +
>> + return 0;
>> +}
>> +
>> +static int __maybe_unused regulator_haptic_suspend(struct device *dev)
>> +{
>> + struct platform_device *pdev = to_platform_device(dev);
>> + struct regulator_haptic *haptic = platform_get_drvdata(pdev);
>> +
>> + if (haptic->enabled) {
>> + regulator_haptic_disable(haptic);
>> + haptic->suspend_state = true;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int __maybe_unused regulator_haptic_resume(struct device *dev)
>> +{
>> + struct platform_device *pdev = to_platform_device(dev);
>> + struct regulator_haptic *haptic = platform_get_drvdata(pdev);
>> +
>> + if (haptic->enabled) {
>> + regulator_haptic_enable(haptic);
>> + haptic->suspend_state = false;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static SIMPLE_DEV_PM_OPS(regulator_haptic_pm_ops,
>> + regulator_haptic_suspend, regulator_haptic_resume);
>> +
>> +static struct of_device_id regulator_haptic_dt_match[] = {
>> + { .compatible = "regulator-haptic" },
>> + {},
>> +};
>> +
>> +static struct platform_driver regulator_haptic_driver = {
>> + .probe = regulator_haptic_probe,
>> + .driver = {
>> + .name = "regulator-haptic",
>> + .owner = THIS_MODULE,
> .owner is no more required for drivers using module_platform_driver()
>
> Thanks,
> Pankaj Dubey
Thanks to review my patches.
I will remove next version.
>
>> + .of_match_table = regulator_haptic_dt_match,
>> + .pm = ®ulator_haptic_pm_ops,
>> + },
>> +};
>> +module_platform_driver(regulator_haptic_driver);
>> +
>> +MODULE_AUTHOR("Jaewon Kim <jaewon02.kim@samsung.com>");
>> +MODULE_AUTHOR("Hyunhee Kim <hyunhee.kim@samsung.com>");
>> +MODULE_ALIAS("platform:regulator-haptic");
>> +MODULE_DESCRIPTION("Regulator haptic driver");
>> +MODULE_LICENSE("GPL");
>> --
>> 1.7.9.5
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
Regards,
Jaewon Kim
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] Input: add regulator haptic driver
2014-11-20 15:09 ` Dan Murphy
@ 2014-11-21 1:49 ` Jaewon Kim
-1 siblings, 0 replies; 10+ messages in thread
From: Jaewon Kim @ 2014-11-21 1:49 UTC (permalink / raw)
To: Dan Murphy, Pankaj Dubey
Cc: Kukjin Kim, Dmitry Torokhov, Chanwoo Choi, linux-kernel,
linux-input, linux-samsung-soc, Hyunhee Kim
Hi Dan,
2014년 11월 21일 00:09에 Dan Murphy 이(가) 쓴 글:
> Hi
>
> On 11/20/2014 08:33 AM, Pankaj Dubey wrote:
>> Hi Jaewon,
>>
>> On 20 November 2014 19:01, Jaewon Kim <jaewon02.kim@samsung.com> wrote:
>>> This patch adds support for haptic driver controlled by
>>> voltage of regulator. And this driver support for
>>> Force Feedback interface from input framework
>>>
>>> Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com>
>>> Signed-off-by: Hyunhee Kim <hyunhee.kim@samsung.com>
>>> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
>>> ---
>>> .../devicetree/bindings/input/regulator-haptic.txt | 24 ++
>>> drivers/input/misc/Kconfig | 11 +
>>> drivers/input/misc/Makefile | 1 +
>>> drivers/input/misc/regulator-haptic.c | 241 ++++++++++++++++++++
>>> 4 files changed, 277 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/input/regulator-haptic.txt
>>> create mode 100644 drivers/input/misc/regulator-haptic.c
>>>
>>> diff --git a/Documentation/devicetree/bindings/input/regulator-haptic.txt b/Documentation/devicetree/bindings/input/regulator-haptic.txt
>>> new file mode 100644
>>> index 0000000..9f60e17
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/input/regulator-haptic.txt
>>> @@ -0,0 +1,24 @@
>>> +* Requlator Haptic Device Tree Bindings
>>> +
>>> +The regulator haptic driver controlled by voltage of regulator.
>>> +This driver implemented via Force Feedback interface.
>>> +
>>> +Required Properties:
>>> + - compatible : Should be "regulator-haptic"
>>> + - haptic-supply : Power supply for the haptic motor.
>>> + [*] refer Documentation/devicetree/bindings/regulator/regulator.txt
>>> +
>>> + - max-microvolt : The maximum voltage value supplied to haptic motor.
>>> + [The unit of the voltage is a micro]
>>> +
>>> + - min-microvolt : The minimum voltage value in which haptic motor reacts.
>>> + [The unit of the voltage is a micro]
>>> +
>>> +Example:
>>> +
>>> + regulator-haptic {
> Should this be more about the function and not the driver name?
> Somehting like
>
> haptics {
Yes, haptics is better than driver name.
>
>>> + compatible = "regulator-haptic";
>>> + haptic-supply = <&motor_regulator>;
>>> + max-microvolt = <2700000>;
>>> + min-microvolt = <1100000>;
>>> + };
>>> diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
>>> index 23297ab..e5e556d 100644
>>> --- a/drivers/input/misc/Kconfig
>>> +++ b/drivers/input/misc/Kconfig
>>> @@ -394,6 +394,17 @@ config INPUT_CM109
>>> To compile this driver as a module, choose M here: the module will be
>>> called cm109.
>>>
>>> +config INPUT_REGULATOR_HAPTIC
>>> + tristate "regulator haptics support"
>>> + select INPUT_FF_MEMLESS
>>> + help
>>> + This option enables device driver support for the haptic controlled
>>> + by regulator. This driver supports ff-memless interface
>>> + from input framework.
>>> +
>>> + To compile this driver as a module, choose M here: the
>>> + module will be called regulator-haptic.
>>> +
>>> config INPUT_RETU_PWRBUTTON
>>> tristate "Retu Power button Driver"
>>> depends on MFD_RETU
>>> diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
>>> index 19c7603..1f135af 100644
>>> --- a/drivers/input/misc/Makefile
>>> +++ b/drivers/input/misc/Makefile
>>> @@ -53,6 +53,7 @@ obj-$(CONFIG_INPUT_PMIC8XXX_PWRKEY) += pmic8xxx-pwrkey.o
>>> obj-$(CONFIG_INPUT_POWERMATE) += powermate.o
>>> obj-$(CONFIG_INPUT_PWM_BEEPER) += pwm-beeper.o
>>> obj-$(CONFIG_INPUT_RB532_BUTTON) += rb532_button.o
>>> +obj-$(CONFIG_INPUT_REGULATOR_HAPTIC) += regulator-haptic.o
>>> obj-$(CONFIG_INPUT_RETU_PWRBUTTON) += retu-pwrbutton.o
>>> obj-$(CONFIG_INPUT_GPIO_ROTARY_ENCODER) += rotary_encoder.o
>>> obj-$(CONFIG_INPUT_SGI_BTNS) += sgi_btns.o
>>> diff --git a/drivers/input/misc/regulator-haptic.c b/drivers/input/misc/regulator-haptic.c
>>> new file mode 100644
>>> index 0000000..1a83ecb
>>> --- /dev/null
>>> +++ b/drivers/input/misc/regulator-haptic.c
>>> @@ -0,0 +1,241 @@
>>> +/*
>>> + * Regulator haptic driver
>>> + *
>>> + * Copyright (c) 2014 Samsung Electronics Co., Ltd.
>>> + * Author: Jaewon Kim <jaewon02.kim@samsung.com>
>>> + * Author: Hyunhee Kim <hyunhee.kim@samsung.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
>>> + * published by the Free Software Foundation.
>>> + */
>>> +
>>> +#include <linux/module.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/input.h>
>>> +#include <linux/slab.h>
>>> +#include <linux/regulator/consumer.h>
>>> +#include <linux/of.h>
>>> +
>>> +#define MAX_MAGNITUDE_SHIFT 16
>>> +
>>> +struct regulator_haptic {
>>> + struct device *dev;
>>> + struct input_dev *input_dev;
>>> + struct regulator *regulator;
>>> + struct work_struct work;
>>> +
>>> + bool enabled;
>>> + bool suspend_state;
> I don't understand what you are tracking here. You only set it and unset
> this value but never do any checking
>
>>> + unsigned int max_volt;
>>> + unsigned int min_volt;
>>> + unsigned int intensity;
>>> + unsigned int magnitude;
>>> +};
>>> +
>>> +static void regulator_haptic_enable(struct regulator_haptic *haptic)
>>> +{
>>> + int error;
>>> +
>>> + if (haptic->enabled)
>>> + return;
>>> +
>>> + error = regulator_enable(haptic->regulator);
>>> + if (error) {
>>> + dev_err(haptic->dev, "cannot enable regulator\n");
>>> + return;
>>> + }
>>> +
>>> + haptic->enabled = true;
>>> +}
>>> +
>>> +static void regulator_haptic_disable(struct regulator_haptic *haptic)
>>> +{
>>> + int error;
>>> +
>>> + if (!haptic->enabled)
>>> + return;
>>> +
>>> + error = regulator_disable(haptic->regulator);
>>> + if (error) {
>>> + dev_err(haptic->dev, "cannot disable regulator\n");
>>> + return;
>>> + }
>>> +
>>> + haptic->enabled = false;
>>> +}
> Can the enable/disable just be one function with an argument?
> Then you can centralize the check and reduce the code footprint.
Ok. I will merge two function with a boolean parameter.
>
>>> +
>>> +static void regulator_haptic_work(struct work_struct *work)
>>> +{
>>> + struct regulator_haptic *haptic = container_of(work,
>>> + struct regulator_haptic, work);
>>> + int error;
>>> +
>>> + error = regulator_set_voltage(haptic->regulator,
>>> + haptic->intensity + haptic->min_volt, haptic->max_volt);
>>> + if (error) {
>>> + dev_err(haptic->dev, "cannot set regulator voltage\n");
>>> + return;
>>> + }
>>> +
>>> + if (haptic->magnitude)
>>> + regulator_haptic_enable(haptic);
>>> + else
>>> + regulator_haptic_disable(haptic);
>>> +}
>>> +
>>> +static int regulator_haptic_play_effect(struct input_dev *input, void *data,
>>> + struct ff_effect *effect)
>>> +{
>>> + struct regulator_haptic *haptic = input_get_drvdata(input);
>>> + u64 volt_mag_multi;
>>> +
>>> + haptic->magnitude = effect->u.rumble.strong_magnitude;
>>> + if (!haptic->magnitude)
>>> + haptic->magnitude = effect->u.rumble.weak_magnitude;
>>> +
>>> +
>>> + volt_mag_multi = (u64)(haptic->max_volt - haptic->min_volt) *
>>> + haptic->magnitude;
>>> + haptic->intensity = (unsigned int)(volt_mag_multi >>
>>> + MAX_MAGNITUDE_SHIFT);
>>> +
>>> + schedule_work(&haptic->work);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static void regulator_haptic_close(struct input_dev *input)
>>> +{
>>> + struct regulator_haptic *haptic = input_get_drvdata(input);
>>> +
>>> + cancel_work_sync(&haptic->work);
>>> + regulator_haptic_disable(haptic);
>>> +}
>>> +
>>> +static int regulator_haptic_parse_dt(struct regulator_haptic *haptic)
>>> +{
>>> + struct device_node *node = haptic->dev->of_node;
>>> + int error;
>>> +
>>> + error = of_property_read_u32(node, "max-microvolt", &haptic->max_volt);
>>> + if (error) {
>>> + dev_err(haptic->dev, "cannot parse max-microvolt\n");
>>> + return error;
>>> + }
>>> +
>>> + error = of_property_read_u32(node, "min-microvolt", &haptic->min_volt);
>>> + if (error) {
>>> + dev_err(haptic->dev, "cannot parse min-microvolt\n");
>>> + return error;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int regulator_haptic_probe(struct platform_device *pdev)
>>> +{
>>> + struct regulator_haptic *haptic;
>>> + struct input_dev *input_dev;
>>> + int error;
>>> +
>>> + haptic = devm_kzalloc(&pdev->dev, sizeof(*haptic), GFP_KERNEL);
>>> + if (!haptic)
>>> + return -ENOMEM;
>>> +
>>> + haptic->dev = &pdev->dev;
>>> + haptic->enabled = false;
>>> + haptic->suspend_state = false;
>>> + INIT_WORK(&haptic->work, regulator_haptic_work);
>>> +
>>> + error = regulator_haptic_parse_dt(haptic);
> Do you need platform data support for platforms that do not currently support
> dt?
Ok I will support platform data in next version.
>
>>> + if (error) {
>>> + dev_err(&pdev->dev, "failed to parse device tree\n");
>>> + return error;
>>> + }
>>> +
>>> + haptic->regulator = devm_regulator_get(&pdev->dev, "haptic");
>>> + if (IS_ERR(haptic->regulator)) {
>>> + dev_err(&pdev->dev, "failed to get regulator\n");
>>> + return PTR_ERR(haptic->regulator);
>>> + }
>>> +
>>> + input_dev = devm_input_allocate_device(&pdev->dev);
>>> + if (!input_dev)
>>> + return -ENOMEM;
>>> +
>>> + haptic->input_dev = input_dev;
>>> + haptic->input_dev->name = "regulator-haptic";
>>> + haptic->input_dev->dev.parent = &pdev->dev;
>>> + haptic->input_dev->close = regulator_haptic_close;
>>> + input_set_drvdata(haptic->input_dev, haptic);
>>> + input_set_capability(haptic->input_dev, EV_FF, FF_RUMBLE);
>>> +
>>> + error = input_ff_create_memless(input_dev, NULL,
>>> + regulator_haptic_play_effect);
>>> + if (error) {
>>> + dev_err(&pdev->dev, "failed to create force-feedback\n");
>>> + return error;
>>> + }
>>> +
>>> + error = input_register_device(haptic->input_dev);
>>> + if (error) {
>>> + dev_err(&pdev->dev, "failed to register input device\n");
>>> + return error;
>>> + }
>>> +
>>> + platform_set_drvdata(pdev, haptic);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int __maybe_unused regulator_haptic_suspend(struct device *dev)
>>> +{
>>> + struct platform_device *pdev = to_platform_device(dev);
>>> + struct regulator_haptic *haptic = platform_get_drvdata(pdev);
>>> +
>>> + if (haptic->enabled) {
>>> + regulator_haptic_disable(haptic);
>>> + haptic->suspend_state = true;
>>>
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int __maybe_unused regulator_haptic_resume(struct device *dev)
>>> +{
>>> + struct platform_device *pdev = to_platform_device(dev);
>>> + struct regulator_haptic *haptic = platform_get_drvdata(pdev);
>>> +
>>> + if (haptic->enabled) {
>>> + regulator_haptic_enable(haptic);
> We enable the regulator when we do the haptics work if magnitude is > 0.
> And during suspend haptic->enabled may get set to false if it is true
> so wouldn't haptic->enabled always be false here?
You are right, It was my typo. It is "if (haptic->suspend_state)".
My intention is restore past state.
thanks to point error.
>
> Plus do we want to continue the haptic response when coming out of suspend state?
>
>>> + haptic->suspend_state = false;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static SIMPLE_DEV_PM_OPS(regulator_haptic_pm_ops,
>>> + regulator_haptic_suspend, regulator_haptic_resume);
>>> +
>>> +static struct of_device_id regulator_haptic_dt_match[] = {
>>> + { .compatible = "regulator-haptic" },
>>> + {},
>>> +};
>>> +
>>> +static struct platform_driver regulator_haptic_driver = {
>>> + .probe = regulator_haptic_probe,
>>> + .driver = {
>>> + .name = "regulator-haptic",
>>> + .owner = THIS_MODULE,
>> .owner is no more required for drivers using module_platform_driver()
>>
>> Thanks,
>> Pankaj Dubey
>>
>>> + .of_match_table = regulator_haptic_dt_match,
>>> + .pm = ®ulator_haptic_pm_ops,
>>> + },
>>> +};
>>> +module_platform_driver(regulator_haptic_driver);
>>> +
>>> +MODULE_AUTHOR("Jaewon Kim <jaewon02.kim@samsung.com>");
>>> +MODULE_AUTHOR("Hyunhee Kim <hyunhee.kim@samsung.com>");
>>> +MODULE_ALIAS("platform:regulator-haptic");
>>> +MODULE_DESCRIPTION("Regulator haptic driver");
>>> +MODULE_LICENSE("GPL");
>>> --
>>> 1.7.9.5
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> --
>> 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] 10+ messages in thread
* Re: [PATCH 1/2] Input: add regulator haptic driver
@ 2014-11-21 1:49 ` Jaewon Kim
0 siblings, 0 replies; 10+ messages in thread
From: Jaewon Kim @ 2014-11-21 1:49 UTC (permalink / raw)
To: Dan Murphy, Pankaj Dubey
Cc: Kukjin Kim, Dmitry Torokhov, Chanwoo Choi, linux-kernel,
linux-input, linux-samsung-soc, Hyunhee Kim
Hi Dan,
2014년 11월 21일 00:09에 Dan Murphy 이(가) 쓴 글:
> Hi
>
> On 11/20/2014 08:33 AM, Pankaj Dubey wrote:
>> Hi Jaewon,
>>
>> On 20 November 2014 19:01, Jaewon Kim <jaewon02.kim@samsung.com> wrote:
>>> This patch adds support for haptic driver controlled by
>>> voltage of regulator. And this driver support for
>>> Force Feedback interface from input framework
>>>
>>> Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com>
>>> Signed-off-by: Hyunhee Kim <hyunhee.kim@samsung.com>
>>> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
>>> ---
>>> .../devicetree/bindings/input/regulator-haptic.txt | 24 ++
>>> drivers/input/misc/Kconfig | 11 +
>>> drivers/input/misc/Makefile | 1 +
>>> drivers/input/misc/regulator-haptic.c | 241 ++++++++++++++++++++
>>> 4 files changed, 277 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/input/regulator-haptic.txt
>>> create mode 100644 drivers/input/misc/regulator-haptic.c
>>>
>>> diff --git a/Documentation/devicetree/bindings/input/regulator-haptic.txt b/Documentation/devicetree/bindings/input/regulator-haptic.txt
>>> new file mode 100644
>>> index 0000000..9f60e17
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/input/regulator-haptic.txt
>>> @@ -0,0 +1,24 @@
>>> +* Requlator Haptic Device Tree Bindings
>>> +
>>> +The regulator haptic driver controlled by voltage of regulator.
>>> +This driver implemented via Force Feedback interface.
>>> +
>>> +Required Properties:
>>> + - compatible : Should be "regulator-haptic"
>>> + - haptic-supply : Power supply for the haptic motor.
>>> + [*] refer Documentation/devicetree/bindings/regulator/regulator.txt
>>> +
>>> + - max-microvolt : The maximum voltage value supplied to haptic motor.
>>> + [The unit of the voltage is a micro]
>>> +
>>> + - min-microvolt : The minimum voltage value in which haptic motor reacts.
>>> + [The unit of the voltage is a micro]
>>> +
>>> +Example:
>>> +
>>> + regulator-haptic {
> Should this be more about the function and not the driver name?
> Somehting like
>
> haptics {
Yes, haptics is better than driver name.
>
>>> + compatible = "regulator-haptic";
>>> + haptic-supply = <&motor_regulator>;
>>> + max-microvolt = <2700000>;
>>> + min-microvolt = <1100000>;
>>> + };
>>> diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
>>> index 23297ab..e5e556d 100644
>>> --- a/drivers/input/misc/Kconfig
>>> +++ b/drivers/input/misc/Kconfig
>>> @@ -394,6 +394,17 @@ config INPUT_CM109
>>> To compile this driver as a module, choose M here: the module will be
>>> called cm109.
>>>
>>> +config INPUT_REGULATOR_HAPTIC
>>> + tristate "regulator haptics support"
>>> + select INPUT_FF_MEMLESS
>>> + help
>>> + This option enables device driver support for the haptic controlled
>>> + by regulator. This driver supports ff-memless interface
>>> + from input framework.
>>> +
>>> + To compile this driver as a module, choose M here: the
>>> + module will be called regulator-haptic.
>>> +
>>> config INPUT_RETU_PWRBUTTON
>>> tristate "Retu Power button Driver"
>>> depends on MFD_RETU
>>> diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
>>> index 19c7603..1f135af 100644
>>> --- a/drivers/input/misc/Makefile
>>> +++ b/drivers/input/misc/Makefile
>>> @@ -53,6 +53,7 @@ obj-$(CONFIG_INPUT_PMIC8XXX_PWRKEY) += pmic8xxx-pwrkey.o
>>> obj-$(CONFIG_INPUT_POWERMATE) += powermate.o
>>> obj-$(CONFIG_INPUT_PWM_BEEPER) += pwm-beeper.o
>>> obj-$(CONFIG_INPUT_RB532_BUTTON) += rb532_button.o
>>> +obj-$(CONFIG_INPUT_REGULATOR_HAPTIC) += regulator-haptic.o
>>> obj-$(CONFIG_INPUT_RETU_PWRBUTTON) += retu-pwrbutton.o
>>> obj-$(CONFIG_INPUT_GPIO_ROTARY_ENCODER) += rotary_encoder.o
>>> obj-$(CONFIG_INPUT_SGI_BTNS) += sgi_btns.o
>>> diff --git a/drivers/input/misc/regulator-haptic.c b/drivers/input/misc/regulator-haptic.c
>>> new file mode 100644
>>> index 0000000..1a83ecb
>>> --- /dev/null
>>> +++ b/drivers/input/misc/regulator-haptic.c
>>> @@ -0,0 +1,241 @@
>>> +/*
>>> + * Regulator haptic driver
>>> + *
>>> + * Copyright (c) 2014 Samsung Electronics Co., Ltd.
>>> + * Author: Jaewon Kim <jaewon02.kim@samsung.com>
>>> + * Author: Hyunhee Kim <hyunhee.kim@samsung.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
>>> + * published by the Free Software Foundation.
>>> + */
>>> +
>>> +#include <linux/module.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/input.h>
>>> +#include <linux/slab.h>
>>> +#include <linux/regulator/consumer.h>
>>> +#include <linux/of.h>
>>> +
>>> +#define MAX_MAGNITUDE_SHIFT 16
>>> +
>>> +struct regulator_haptic {
>>> + struct device *dev;
>>> + struct input_dev *input_dev;
>>> + struct regulator *regulator;
>>> + struct work_struct work;
>>> +
>>> + bool enabled;
>>> + bool suspend_state;
> I don't understand what you are tracking here. You only set it and unset
> this value but never do any checking
>
>>> + unsigned int max_volt;
>>> + unsigned int min_volt;
>>> + unsigned int intensity;
>>> + unsigned int magnitude;
>>> +};
>>> +
>>> +static void regulator_haptic_enable(struct regulator_haptic *haptic)
>>> +{
>>> + int error;
>>> +
>>> + if (haptic->enabled)
>>> + return;
>>> +
>>> + error = regulator_enable(haptic->regulator);
>>> + if (error) {
>>> + dev_err(haptic->dev, "cannot enable regulator\n");
>>> + return;
>>> + }
>>> +
>>> + haptic->enabled = true;
>>> +}
>>> +
>>> +static void regulator_haptic_disable(struct regulator_haptic *haptic)
>>> +{
>>> + int error;
>>> +
>>> + if (!haptic->enabled)
>>> + return;
>>> +
>>> + error = regulator_disable(haptic->regulator);
>>> + if (error) {
>>> + dev_err(haptic->dev, "cannot disable regulator\n");
>>> + return;
>>> + }
>>> +
>>> + haptic->enabled = false;
>>> +}
> Can the enable/disable just be one function with an argument?
> Then you can centralize the check and reduce the code footprint.
Ok. I will merge two function with a boolean parameter.
>
>>> +
>>> +static void regulator_haptic_work(struct work_struct *work)
>>> +{
>>> + struct regulator_haptic *haptic = container_of(work,
>>> + struct regulator_haptic, work);
>>> + int error;
>>> +
>>> + error = regulator_set_voltage(haptic->regulator,
>>> + haptic->intensity + haptic->min_volt, haptic->max_volt);
>>> + if (error) {
>>> + dev_err(haptic->dev, "cannot set regulator voltage\n");
>>> + return;
>>> + }
>>> +
>>> + if (haptic->magnitude)
>>> + regulator_haptic_enable(haptic);
>>> + else
>>> + regulator_haptic_disable(haptic);
>>> +}
>>> +
>>> +static int regulator_haptic_play_effect(struct input_dev *input, void *data,
>>> + struct ff_effect *effect)
>>> +{
>>> + struct regulator_haptic *haptic = input_get_drvdata(input);
>>> + u64 volt_mag_multi;
>>> +
>>> + haptic->magnitude = effect->u.rumble.strong_magnitude;
>>> + if (!haptic->magnitude)
>>> + haptic->magnitude = effect->u.rumble.weak_magnitude;
>>> +
>>> +
>>> + volt_mag_multi = (u64)(haptic->max_volt - haptic->min_volt) *
>>> + haptic->magnitude;
>>> + haptic->intensity = (unsigned int)(volt_mag_multi >>
>>> + MAX_MAGNITUDE_SHIFT);
>>> +
>>> + schedule_work(&haptic->work);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static void regulator_haptic_close(struct input_dev *input)
>>> +{
>>> + struct regulator_haptic *haptic = input_get_drvdata(input);
>>> +
>>> + cancel_work_sync(&haptic->work);
>>> + regulator_haptic_disable(haptic);
>>> +}
>>> +
>>> +static int regulator_haptic_parse_dt(struct regulator_haptic *haptic)
>>> +{
>>> + struct device_node *node = haptic->dev->of_node;
>>> + int error;
>>> +
>>> + error = of_property_read_u32(node, "max-microvolt", &haptic->max_volt);
>>> + if (error) {
>>> + dev_err(haptic->dev, "cannot parse max-microvolt\n");
>>> + return error;
>>> + }
>>> +
>>> + error = of_property_read_u32(node, "min-microvolt", &haptic->min_volt);
>>> + if (error) {
>>> + dev_err(haptic->dev, "cannot parse min-microvolt\n");
>>> + return error;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int regulator_haptic_probe(struct platform_device *pdev)
>>> +{
>>> + struct regulator_haptic *haptic;
>>> + struct input_dev *input_dev;
>>> + int error;
>>> +
>>> + haptic = devm_kzalloc(&pdev->dev, sizeof(*haptic), GFP_KERNEL);
>>> + if (!haptic)
>>> + return -ENOMEM;
>>> +
>>> + haptic->dev = &pdev->dev;
>>> + haptic->enabled = false;
>>> + haptic->suspend_state = false;
>>> + INIT_WORK(&haptic->work, regulator_haptic_work);
>>> +
>>> + error = regulator_haptic_parse_dt(haptic);
> Do you need platform data support for platforms that do not currently support
> dt?
Ok I will support platform data in next version.
>
>>> + if (error) {
>>> + dev_err(&pdev->dev, "failed to parse device tree\n");
>>> + return error;
>>> + }
>>> +
>>> + haptic->regulator = devm_regulator_get(&pdev->dev, "haptic");
>>> + if (IS_ERR(haptic->regulator)) {
>>> + dev_err(&pdev->dev, "failed to get regulator\n");
>>> + return PTR_ERR(haptic->regulator);
>>> + }
>>> +
>>> + input_dev = devm_input_allocate_device(&pdev->dev);
>>> + if (!input_dev)
>>> + return -ENOMEM;
>>> +
>>> + haptic->input_dev = input_dev;
>>> + haptic->input_dev->name = "regulator-haptic";
>>> + haptic->input_dev->dev.parent = &pdev->dev;
>>> + haptic->input_dev->close = regulator_haptic_close;
>>> + input_set_drvdata(haptic->input_dev, haptic);
>>> + input_set_capability(haptic->input_dev, EV_FF, FF_RUMBLE);
>>> +
>>> + error = input_ff_create_memless(input_dev, NULL,
>>> + regulator_haptic_play_effect);
>>> + if (error) {
>>> + dev_err(&pdev->dev, "failed to create force-feedback\n");
>>> + return error;
>>> + }
>>> +
>>> + error = input_register_device(haptic->input_dev);
>>> + if (error) {
>>> + dev_err(&pdev->dev, "failed to register input device\n");
>>> + return error;
>>> + }
>>> +
>>> + platform_set_drvdata(pdev, haptic);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int __maybe_unused regulator_haptic_suspend(struct device *dev)
>>> +{
>>> + struct platform_device *pdev = to_platform_device(dev);
>>> + struct regulator_haptic *haptic = platform_get_drvdata(pdev);
>>> +
>>> + if (haptic->enabled) {
>>> + regulator_haptic_disable(haptic);
>>> + haptic->suspend_state = true;
>>>
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int __maybe_unused regulator_haptic_resume(struct device *dev)
>>> +{
>>> + struct platform_device *pdev = to_platform_device(dev);
>>> + struct regulator_haptic *haptic = platform_get_drvdata(pdev);
>>> +
>>> + if (haptic->enabled) {
>>> + regulator_haptic_enable(haptic);
> We enable the regulator when we do the haptics work if magnitude is > 0.
> And during suspend haptic->enabled may get set to false if it is true
> so wouldn't haptic->enabled always be false here?
You are right, It was my typo. It is "if (haptic->suspend_state)".
My intention is restore past state.
thanks to point error.
>
> Plus do we want to continue the haptic response when coming out of suspend state?
>
>>> + haptic->suspend_state = false;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static SIMPLE_DEV_PM_OPS(regulator_haptic_pm_ops,
>>> + regulator_haptic_suspend, regulator_haptic_resume);
>>> +
>>> +static struct of_device_id regulator_haptic_dt_match[] = {
>>> + { .compatible = "regulator-haptic" },
>>> + {},
>>> +};
>>> +
>>> +static struct platform_driver regulator_haptic_driver = {
>>> + .probe = regulator_haptic_probe,
>>> + .driver = {
>>> + .name = "regulator-haptic",
>>> + .owner = THIS_MODULE,
>> .owner is no more required for drivers using module_platform_driver()
>>
>> Thanks,
>> Pankaj Dubey
>>
>>> + .of_match_table = regulator_haptic_dt_match,
>>> + .pm = ®ulator_haptic_pm_ops,
>>> + },
>>> +};
>>> +module_platform_driver(regulator_haptic_driver);
>>> +
>>> +MODULE_AUTHOR("Jaewon Kim <jaewon02.kim@samsung.com>");
>>> +MODULE_AUTHOR("Hyunhee Kim <hyunhee.kim@samsung.com>");
>>> +MODULE_ALIAS("platform:regulator-haptic");
>>> +MODULE_DESCRIPTION("Regulator haptic driver");
>>> +MODULE_LICENSE("GPL");
>>> --
>>> 1.7.9.5
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> --
>> 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
>
--
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] 10+ messages in thread
* Re: [PATCH 1/2] Input: add regulator haptic driver
[not found] ` <20141128125051.GA3643@sirena.org.uk>
@ 2014-12-01 1:30 ` Jaewon Kim
0 siblings, 0 replies; 10+ messages in thread
From: Jaewon Kim @ 2014-12-01 1:30 UTC (permalink / raw)
To: Mark Brown
Cc: Kukjin Kim, Dmitry Torokhov, Chanwoo Choi, linux-kernel,
linux-input, linux-samsung-soc, Hyunhee Kim
Hi Mark,
2014년 11월 28일 21:50에 Mark Brown 이(가) 쓴 글:
> On Thu, Nov 20, 2014 at 10:31:39PM +0900, Jaewon Kim wrote:
>
>> + haptic->regulator = devm_regulator_get(&pdev->dev, "haptic");
>> + if (IS_ERR(haptic->regulator)) {
>> + dev_err(&pdev->dev, "failed to get regulator\n");
>> + return PTR_ERR(haptic->regulator);
>> + }
> This needs to be _get_exclusive() - the driver will be broken if
> something else shares the regualtor since it relies on both enabling and
> disabling the regulator and on setting the voltage for effects. It's
> not like a power supply where leaving the device powered when it could
> be off is going to have no effect, if the power is left on then the
> haptic motor will continue to operate.
Thanks to review my patch.
Haptic regulator has to only be used in this driver.
So, it is right to change like your advice.
I will fix next version.
Thanks
Jaewon Kim.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2014-12-01 1:30 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-11-20 13:31 [PATCH 0/2] Add regulator-haptic driver Jaewon Kim
2014-11-20 13:31 ` [PATCH 1/2] Input: add regulator haptic driver Jaewon Kim
2014-11-20 14:33 ` Pankaj Dubey
2014-11-20 15:09 ` Dan Murphy
2014-11-20 15:09 ` Dan Murphy
2014-11-21 1:49 ` Jaewon Kim
2014-11-21 1:49 ` Jaewon Kim
2014-11-21 1:22 ` Jaewon Kim
[not found] ` <20141128125051.GA3643@sirena.org.uk>
2014-12-01 1:30 ` Jaewon Kim
2014-11-20 13:31 ` [PATCH 2/2] ARM: dts: Add regulator-haptic device node for exynos3250-rinato Jaewon Kim
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.