da7280: haptic driver submission

drivers/input/misc/Kconfig +0 -0

index 362e8a0..06dc5a3 100644
@@ -869,4 +869,17 @@ config INPUT_STPMIC1_ONKEY
 	  To compile this driver as a module, choose M here: the
 	  module will be called stpmic1_onkey.

Quoted 8 lines

drivers/input/misc/Kconfig +0 -0

index 362e8a0..06dc5a3 100644
@@ -869,4 +869,17 @@ config INPUT_STPMIC1_ONKEY
 	  To compile this driver as a module, choose M here: the
 	  module will be called stpmic1_onkey.
 

drivers/input/misc/Kconfig config INPUT_STPMIC1_ONKEY · line 880

Quoted 9 lines

drivers/input/misc/Kconfig +9 -0

+config INPUT_DA7280_HAPTICS
+	tristate "Dialog Semiconductor DA7280 haptics support"
+	depends on INPUT && I2C
+	select INPUT_FF_MEMLESS
+	select REGMAP_I2C
+	help
+	  Say Y to enable support for the Dialog DA7280 haptics driver.
+	  The haptics can be controlled by I2C communication,
+	  or by PWM input, or by GPI.

Is that GPIO.
?

Can the haptics be controlled only by PWM or only by GPI(O)?

Just curious: why is I2C required to build the driver if a user is only controlling the device by PWM or GPI?

drivers/input/misc/Kconfig config INPUT_STPMIC1_ONKEY · line 885

Quoted 7 lines

drivers/input/misc/Kconfig +4 -0

+
+	  To compile this driver as a module, choose M here: the
+	  module will be called da7280.
+
 endif

thanks.
--
~Randy

On Fri, July 24, 2020 5:51 AM, Randy Dunlap wrote

Quoted 9 lines · Roy Im

Quoted 8 lines

drivers/input/misc/Kconfig +0 -0

index 362e8a0..06dc5a3 100644
@@ -869,4 +869,17 @@ config INPUT_STPMIC1_ONKEY
 	  To compile this driver as a module, choose M here: the
 	  module will be called stpmic1_onkey.
 

Quoted 12 lines

drivers/input/misc/Kconfig config INPUT_STPMIC1_ONKEY · line 880

Quoted 9 lines

drivers/input/misc/Kconfig +9 -0

+config INPUT_DA7280_HAPTICS
+	tristate "Dialog Semiconductor DA7280 haptics support"
+	depends on INPUT && I2C
+	select INPUT_FF_MEMLESS
+	select REGMAP_I2C
+	help
+	  Say Y to enable support for the Dialog DA7280 haptics driver.
+	  The haptics can be controlled by I2C communication,
+	  or by PWM input, or by GPI.

Is that GPIO.
?

The Haptics can be working by GPI(if see from the haptic device), but from the Host it is GPO. Do you think the GPIO is correct?

Quoted 4 lines

Can the haptics be controlled only by PWM or only by GPI(O)?

Just curious: why is I2C required to build the driver if a user is only controlling the device by PWM or GPI?

I2C is required to control registers and it can be triggered by I2C or PWM or GPI(controlled by host outside this driver), so PWM and GPI are optional. With your comments, I think it's better to remove below lines(//remove) to avoid confusion and add PWM as below if you agree.

         // remove
	  The haptics can be controlled by I2C communication,
	  or by PWM input, or by GPI.
         // update, adding || PWM
         depends on (INPUT && I2C) || PWM

Quoted 22 lines

On Fri, July 24, 2020 5:51 AM, Randy Dunlap wrote

Quoted 21 lines · Roy Im

Quoted 7 lines

drivers/input/misc/Kconfig +0 -0

index 362e8a0..06dc5a3 100644
@@ -869,4 +869,17 @@ config INPUT_STPMIC1_ONKEY
 	  To compile this driver as a module, choose M here: the
 	  module will be called stpmic1_onkey.

drivers/input/misc/Kconfig config INPUT_STPMIC1_ONKEY · line 880

Quoted 10 lines

drivers/input/misc/Kconfig +9 -0

 
+config INPUT_DA7280_HAPTICS
+	tristate "Dialog Semiconductor DA7280 haptics support"
+	depends on INPUT && I2C
+	select INPUT_FF_MEMLESS
+	select REGMAP_I2C
+	help
+	  Say Y to enable support for the Dialog DA7280 haptics driver.
+	  The haptics can be controlled by I2C communication,
+	  or by PWM input, or by GPI.

Is that GPIO.
?

Quoted 1 line

The Haptics can be working by GPI(if see from the haptic device), but from the Host it is GPO. Do you think the GPIO is

correct?

To me it needs to represent what services/interfaces/facilities are used by this driver that are provided by the Linux kernel. If it uses Linux GPIO services, then it should say GPIO -- although I don't see it using any Linux GPIO services.

Quoted 7 lines

Quoted 5 lines

Can the haptics be controlled only by PWM or only by GPI(O)?

Just curious: why is I2C required to build the driver if a user is only controlling the device by PWM or GPI?

I2C is required to control registers and it can be triggered by I2C or PWM or GPI(controlled by host outside this driver),

so PWM and GPI are optional.

Quoted 1 line

With your comments, I think it's better to remove below lines(//remove) to avoid confusion and add PWM as below if

you agree.

Quoted 5 lines

         // remove
	  The haptics can be controlled by I2C communication,
	  or by PWM input, or by GPI.
         // update, adding || PWM
         depends on (INPUT && I2C) || PWM

Since <linux/pwm.h> provides stubs for when CONFIG_PWM is not enabled, it appears that "depends on PWM" is not required.

I'll leave it up to you. I was just trying to understand better. It may be that no changes are needed.

thanks.
--
~Randy

OK, thanks for your comments. To be clearer, I would like to update as below if you agree

	depends on INPUT && I2C

...
The haptics can be controlled by PWM or GPIO with I2C communication.

Adds support for the Dialog DA7280 LRA/ERM Haptic Driver with multiple mode and integrated waveform memory and wideband support. It communicates via an I2C bus to the device.

Reviewed-by Jes Sorensen <[email protected]>.

Signed-off-by Roy Im <[email protected]>

---

 drivers/input/misc/Kconfig  |   13 +
 drivers/input/misc/Makefile |    1 +
 drivers/input/misc/da7280.c | 1840 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 1854 insertions(+)
 create mode 100644 drivers/input/misc/da7280.c

drivers/input/misc/da7280.c new file +20 -0

new file mode 100644
index 0000000..6e3ead5
@@ -0,0 +1,1840 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * DA7280 Haptic device driver
+ *
+ * Copyright (c) 2020 Dialog Semiconductor.
+ * Author: Roy Im <[email protected]>
+ */
+
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/input.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/pwm.h>
+#include <linux/regmap.h>
+#include <linux/workqueue.h>
+#include <linux/uaccess.h>
+#include <linux/bitops.h>
+#include <linux/bitfield.h>
+

drivers/input/misc/da7280.c new file +12 -0

+static int da7280_haptic_set_pwm(struct da7280_haptic *haptics, bool enabled)
+{
+	struct pwm_state state;
+	u64 period_mag_multi;
+	int error;
+
+	if (!haptics->gain && enabled) {
+		dev_err(haptics->dev,
+			"Please set the gain first for the pwm mode\n");
+		return -EINVAL;
+	}
+

Quoted 43 lines · Roy Im

Adds support for the Dialog DA7280 LRA/ERM Haptic Driver with multiple mode and integrated waveform memory and wideband support. It communicates via an I2C bus to the device.

Reviewed-by Jes Sorensen <[email protected]>.

Signed-off-by Roy Im <[email protected]>

---

 drivers/input/misc/Kconfig  |   13 +
 drivers/input/misc/Makefile |    1 +
 drivers/input/misc/da7280.c | 1840
+++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 1854 insertions(+)
 create mode 100644 drivers/input/misc/da7280.c

drivers/input/misc/da7280.c new file +19 -0

new file mode 100644 index 0000000..6e3ead5
@@ -0,0 +1,1840 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * DA7280 Haptic device driver
+ *
+ * Copyright (c) 2020 Dialog Semiconductor.
+ * Author: Roy Im <[email protected]>  */
+
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/input.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/pwm.h>
+#include <linux/regmap.h>
+#include <linux/workqueue.h>
+#include <linux/uaccess.h>
+#include <linux/bitops.h>
+#include <linux/bitfield.h>
+

...

Quoted 12 lines

drivers/input/misc/da7280.c new file +12 -0

+static int da7280_haptic_set_pwm(struct da7280_haptic *haptics, bool
+enabled) {
+	struct pwm_state state;
+	u64 period_mag_multi;
+	int error;
+
+	if (!haptics->gain && enabled) {
+		dev_err(haptics->dev,
+			"Please set the gain first for the pwm mode\n");
+		return -EINVAL;
+	}
+

drivers/input/misc/da7280.c line 344

Quoted 10 lines

drivers/input/misc/da7280.c new file +10 -0

+	pwm_get_state(haptics->pwm_dev, &state);
+	state.enabled = enabled;
+	if (enabled) {
+		period_mag_multi = (u64)state.period * haptics->gain;
+		period_mag_multi >>= MAX_MAGNITUDE_SHIFT;
+
+		/* The interpretation of duty cycle depends on the acc_en,
+		 * it should be between 50% and 100% for acc_en = 0.
+		 * See datasheet 'PWM mode' section.
+		 */

from coding-style.rst:

/*
* This is the preferred style for multi-line
* comments in the Linux kernel source code.
* Please use it consistently.
*
* Description: A column of asterisks on the left side,
* with beginning and ending almost-blank lines. */

(except for networking code)

Please fix multiple locations.

thanks.
--
~Randy