Re: [PATCH v6 4/5] Input: add haptic drvier on max77843
HI Dmitry,
On 28/02/2015 02:49, Dmitry Torokhov wrote:
On Thu, Feb 26, 2015 at 11:49:36AM +0900, Jaewon Kim wrote:
Hi Dmitry,
On 26/02/2015 10:23, Dmitry Torokhov wrote:
Hi Jaewon,
On Tue, Feb 24, 2015 at 10:29:07AM +0900, Jaewon Kim wrote:
+static void max77843_haptic_play_work(struct work_struct *work)
+{
+ struct max77843_haptic *haptic =
+ container_of(work, struct max77843_haptic, work);
+ int error;
+
+ mutex_lock(&haptic->mutex);
+
+ if (haptic->suspended) {
+ goto err_play;
+ }
+
You do not need braces around single statement. Also, this is not error
that you are handling, I'd prefer if we called this label out_unlock.
You are right.
I will change label name and remove braces.
+ error = max77843_haptic_set_duty_cycle(haptic);
+ if (error) {
+ dev_err(haptic->dev, "failed to set duty cycle: %d\n", error);
+ goto err_play;
+ }
Do you need to configure duty cycle if you stopping the playback? Or
maybe disabling pwm is enough?
It do not need to set duty cycle requisitely when disabling haptic.
I will move this function to front of max77843_haptic_enable().
+
+ if (haptic->magnitude) {
+ error = max77843_haptic_enable(haptic);
+ if (error)
+ dev_err(haptic->dev,
+ "cannot enable haptic: %d\n", error);
+ } else {
+ max77843_haptic_disable(haptic);
+ if (error)
+ dev_err(haptic->dev,
+ "cannot disable haptic: %d\n", error);
What error? You did not assign it...
Detailed error message printed in enable/disable() function.
What I was trying to say is that you do not assign new value to 'error'
variable in this path; it still carries the value from
max77843_haptic_set_duty_cycle() above and so this "if" statement will
never work and the message will never show up.
I never image at all that i am not assign 'error' variable.
I will assign it.
Thanks.
Thanks,
Jaewon Kim
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 4/5] Input: add haptic drvier on max77843
On Thu, Feb 26, 2015 at 11:49:36AM +0900, Jaewon Kim wrote:
> Hi Dmitry,
>
> On 26/02/2015 10:23, Dmitry Torokhov wrote:
> >Hi Jaewon,
> >
> >On Tue, Feb 24, 2015 at 10:29:07AM +0900, Jaewon Kim wrote:
> >>+static void max77843_haptic_play_work(struct work_struct *work)
> >>+{
> >>+ struct max77843_haptic *haptic =
> >>+ container_of(work, struct max77843_haptic, work);
> >>+ int error;
> >>+
> >>+ mutex_lock(&haptic->mutex);
> >>+
> >>+ if (haptic->suspended) {
> >>+ goto err_play;
> >>+ }
> >>+
> >You do not need braces around single statement. Also, this is not error
> >that you are handling, I'd prefer if we called this label out_unlock.
> You are right.
> I will change label name and remove braces.
> >>+ error = max77843_haptic_set_duty_cycle(haptic);
> >>+ if (error) {
> >>+ dev_err(haptic->dev, "failed to set duty cycle: %d\n", error);
> >>+ goto err_play;
> >>+ }
> >Do you need to configure duty cycle if you stopping the playback? Or
> >maybe disabling pwm is enough?
>
> It do not need to set duty cycle requisitely when disabling haptic.
>
> I will move this function to front of max77843_haptic_enable().
>
> >
> >>+
> >>+ if (haptic->magnitude) {
> >>+ error = max77843_haptic_enable(haptic);
> >>+ if (error)
> >>+ dev_err(haptic->dev,
> >>+ "cannot enable haptic: %d\n", error);
> >>+ } else {
> >>+ max77843_haptic_disable(haptic);
> >>+ if (error)
> >>+ dev_err(haptic->dev,
> >>+ "cannot disable haptic: %d\n", error);
> >What error? You did not assign it...
> Detailed error message printed in enable/disable() function.
What I was trying to say is that you do not assign new value to 'error'
variable in this path; it still carries the value from
max77843_haptic_set_duty_cycle() above and so this "if" statement will
never work and the message will never show up.
Thanks.
--
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 4/5] Input: add haptic drvier on max77843
Hi Dmitry,
On 26/02/2015 10:23, Dmitry Torokhov wrote:
Hi Jaewon,
On Tue, Feb 24, 2015 at 10:29:07AM +0900, Jaewon Kim wrote:
+static void max77843_haptic_play_work(struct work_struct *work)
+{
+ struct max77843_haptic *haptic =
+ container_of(work, struct max77843_haptic, work);
+ int error;
+
+ mutex_lock(&haptic->mutex);
+
+ if (haptic->suspended) {
+ goto err_play;
+ }
+
You do not need braces around single statement. Also, this is not error
that you are handling, I'd prefer if we called this label out_unlock.
You are right.
I will change label name and remove braces.
+ error = max77843_haptic_set_duty_cycle(haptic);
+ if (error) {
+ dev_err(haptic->dev, "failed to set duty cycle: %d\n", error);
+ goto err_play;
+ }
Do you need to configure duty cycle if you stopping the playback? Or
maybe disabling pwm is enough?
It do not need to set duty cycle requisitely when disabling haptic.
I will move this function to front of max77843_haptic_enable().
+
+ if (haptic->magnitude) {
+ error = max77843_haptic_enable(haptic);
+ if (error)
+ dev_err(haptic->dev,
+ "cannot enable haptic: %d\n", error);
+ } else {
+ max77843_haptic_disable(haptic);
+ if (error)
+ dev_err(haptic->dev,
+ "cannot disable haptic: %d\n", error);
What error? You did not assign it...
Detailed error message printed in enable/disable() function.
Thanks.
Thanks to review my patch.
Thanks,
Jaewon Kim
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 4/5] Input: add haptic drvier on max77843
Hi Jaewon,
On Tue, Feb 24, 2015 at 10:29:07AM +0900, Jaewon Kim wrote:
> +static void max77843_haptic_play_work(struct work_struct *work)
> +{
> + struct max77843_haptic *haptic =
> + container_of(work, struct max77843_haptic, work);
> + int error;
> +
> + mutex_lock(&haptic->mutex);
> +
> + if (haptic->suspended) {
> + goto err_play;
> + }
> +
You do not need braces around single statement. Also, this is not error
that you are handling, I'd prefer if we called this label out_unlock.
> + error = max77843_haptic_set_duty_cycle(haptic);
> + if (error) {
> + dev_err(haptic->dev, "failed to set duty cycle: %d\n", error);
> + goto err_play;
> + }
Do you need to configure duty cycle if you stopping the playback? Or
maybe disabling pwm is enough?
> +
> + if (haptic->magnitude) {
> + error = max77843_haptic_enable(haptic);
> + if (error)
> + dev_err(haptic->dev,
> + "cannot enable haptic: %d\n", error);
> + } else {
> + max77843_haptic_disable(haptic);
> + if (error)
> + dev_err(haptic->dev,
> + "cannot disable haptic: %d\n", error);
What error? You did not assign it...
Thanks.
--
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 4/5] Input: add haptic drvier on max77843
This patch adds support for haptic driver on max77843
MFD(Multi Function Device) with PMIC, MUIC, LED, CHARGER.
This driver supports external pwm and LRA(Linear Resonant Actuator) motor.
And it supports ff-memless interface from inpu framework.
Cc: Dmitry Torokhov
Signed-off-by: Jaewon Kim
---
drivers/input/misc/Kconfig | 12 ++
drivers/input/misc/Makefile |1 +
drivers/input/misc/max77843-haptic.c | 358 ++
3 files changed, 371 insertions(+)
create mode 100644 drivers/input/misc/max77843-haptic.c
diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 6deb8da..aa8c072 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -165,6 +165,18 @@ config INPUT_MAX77693_HAPTIC
To compile this driver as module, choose M here: the
module will be called max77693-haptic.
+config INPUT_MAX77843_HAPTIC
+tristate "MAXIM MAX77843 haptic controller support"
+depends on MFD_MAX77843 && PWM
+select INPUT_FF_MEMLESS
+help
+ This option enables support for the haptic controller on
+ MAXIM MAX77843 chip. The driver supports ff-memless interface
+ from input framework.
+
+ To compile this driver as module, choose M here: the
+ module will be called max77843-haptic.
+
config INPUT_MAX8925_ONKEY
tristate "MAX8925 ONKEY support"
depends on MFD_MAX8925
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index 403a1a5..75b5884 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -39,6 +39,7 @@ obj-$(CONFIG_INPUT_KEYSPAN_REMOTE)+= keyspan_remote.o
obj-$(CONFIG_INPUT_KXTJ9) += kxtj9.o
obj-$(CONFIG_INPUT_M68K_BEEP) += m68kspkr.o
obj-$(CONFIG_INPUT_MAX77693_HAPTIC)+= max77693-haptic.o
+obj-$(CONFIG_INPUT_MAX77843_HAPTIC)+= max77843-haptic.o
obj-$(CONFIG_INPUT_MAX8925_ONKEY) += max8925_onkey.o
obj-$(CONFIG_INPUT_MAX8997_HAPTIC) += max8997_haptic.o
obj-$(CONFIG_INPUT_MC13783_PWRBUTTON) += mc13783-pwrbutton.o
diff --git a/drivers/input/misc/max77843-haptic.c
b/drivers/input/misc/max77843-haptic.c
new file mode 100644
index 000..0005d0a
--- /dev/null
+++ b/drivers/input/misc/max77843-haptic.c
@@ -0,0 +1,358 @@
+/*
+ * MAXIM MAX77693 Haptic device driver
+ *
+ * Copyright (C) 2015 Samsung Electronics
+ * Author: Jaewon Kim
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define MAX_MAGNITUDE_SHIFT16
+
+enum max77843_haptic_motor_type {
+ MAX77843_HAPTIC_ERM = 0,
+ MAX77843_HAPTIC_LRA,
+};
+
+enum max77843_haptic_pwm_divisor {
+ MAX77843_HAPTIC_PWM_DIVISOR_32 = 0,
+ MAX77843_HAPTIC_PWM_DIVISOR_64,
+ MAX77843_HAPTIC_PWM_DIVISOR_128,
+ MAX77843_HAPTIC_PWM_DIVISOR_256,
+};
+
+struct max77843_haptic {
+ struct regmap *regmap_haptic;
+ struct device *dev;
+ struct input_dev *input_dev;
+ struct pwm_device *pwm_dev;
+ struct regulator *motor_reg;
+ struct work_struct work;
+ struct mutex mutex;
+
+ unsigned int magnitude;
+ unsigned int pwm_duty;
+
+ bool active;
+ bool suspended;
+
+ enum max77843_haptic_motor_type type;
+ enum max77843_haptic_pwm_divisor pwm_divisor;
+};
+
+static int max77843_haptic_set_duty_cycle(struct max77843_haptic *haptic)
+{
+ int delta = (haptic->pwm_dev->period + haptic->pwm_duty) / 2;
+ int error;
+
+ error = pwm_config(haptic->pwm_dev, delta, haptic->pwm_dev->period);
+ if (error) {
+ dev_err(haptic->dev, "failed to configure pwm: %d\n", error);
+ return error;
+ }
+
+ return 0;
+}
+
+static int max77843_haptic_bias(struct max77843_haptic *haptic, bool on)
+{
+ int error;
+
+ error = regmap_update_bits(haptic->regmap_haptic,
+ MAX77843_SYS_REG_MAINCTRL1,
+ MAX77843_MAINCTRL1_BIASEN_MASK,
+ on << MAINCTRL1_BIASEN_SHIFT);
+ if (error) {
+ dev_err(haptic->dev, "failed to %s bias: %d\n",
+ on ? "enable" : "disable", error);
+ return error;
+ }
+
+ return 0;
+}
+
+static int max77843_haptic_config(struct max77843_haptic *haptic, bool enable)
+{
+ unsigned int value;
+ int error;
+
+ value = ((haptic->type << MCONFIG_MODE_SHIFT) |
+ (enable << MCONFIG_MEN_SHIFT) |
+ (haptic->pwm_divisor << MCONFIG_PDIV_SHIFT));
+
+ error = regmap_write(haptic->regmap_haptic,
+ MAX77843_HAP_REG_MCONFIG, value)
