[PATCH 0/5] Improvements for drv260x driver and add to Huawei Watch

[PATCH 0/5] Improvements for drv260x driver and add to Huawei Watch

[Luca Weiss]

Clean up some issues with the driver I've noticed while working on it,
then fix the magnitude handling which previously resulted the driver
discarding the upper 8 bits of the magnitude value, and finally we can
add the drv260x to the APQ8026 Huawei Watch.

While the "fix magnitude handling" commit technically changes behavior
and could change behavior of existing user space applications that deal
with this quirky behavior, it's definitely not correct and results in
very unexpected behavior if the user space doesn't look out for it
(meaning only use 0x00-0xFF magnitude, and also know that the magnitude
was interpreted as signed value, see more details in the commit message
there).

There's currently only one other upstream user of the driver,
qcom/msm8996-xiaomi-gemini where I've added the dts authors to this
patch series (Raffaele & Yassine) and some tests on that device would be
nice since it's operating in LRA mode, mine does in ERM mode.

Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
---
Luca Weiss (5):
      Input: drv260x - fix typo in register value define
      Input: drv260x - sleep between polling GO bit
      Input: drv260x - remove unused .reg_defaults
      Input: drv260x - fix magnitude handling
      ARM: dts: qcom: apq8026-huawei-sturgeon: Add vibrator

 arch/arm/boot/dts/qcom-apq8026-huawei-sturgeon.dts | 28 +++++++++++
 drivers/input/misc/drv260x.c                       | 56 ++++------------------
 2 files changed, 37 insertions(+), 47 deletions(-)
---
base-commit: fa55d47c433364df7361fd4f9b169b5845b06914
change-id: 20230430-drv260x-improvements-e218894a0c5c

Best regards,
-- 
Luca Weiss <luca@z3ntu.xyz>

[PATCH 1/5] Input: drv260x - fix typo in register value define

[Luca Weiss]

ANANLOG should be ANALOG. Fix the typo.

Fixes: 7132fe4f5687 ("Input: drv260x - add TI drv260x haptics driver")
Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
---
 drivers/input/misc/drv260x.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/input/misc/drv260x.c b/drivers/input/misc/drv260x.c
index 8a9ebfc04a2d..e95c4e775b5d 100644
--- a/drivers/input/misc/drv260x.c
+++ b/drivers/input/misc/drv260x.c
@@ -149,7 +149,7 @@
 
 /* Control 3 Register */
 #define DRV260X_LRA_OPEN_LOOP		(1 << 0)
-#define DRV260X_ANANLOG_IN			(1 << 1)
+#define DRV260X_ANALOG_IN			(1 << 1)
 #define DRV260X_LRA_DRV_MODE		(1 << 2)
 #define DRV260X_RTP_UNSIGNED_DATA	(1 << 3)
 #define DRV260X_SUPPLY_COMP_DIS		(1 << 4)
@@ -322,7 +322,7 @@ static const struct reg_sequence drv260x_lra_init_regs[] = {
 		DRV260X_BEMF_GAIN_3 },
 	{ DRV260X_CTRL1, DRV260X_STARTUP_BOOST },
 	{ DRV260X_CTRL2, DRV260X_SAMP_TIME_250 },
-	{ DRV260X_CTRL3, DRV260X_NG_THRESH_2 | DRV260X_ANANLOG_IN },
+	{ DRV260X_CTRL3, DRV260X_NG_THRESH_2 | DRV260X_ANALOG_IN },
 	{ DRV260X_CTRL4, DRV260X_AUTOCAL_TIME_500MS },
 };
 

-- 
2.40.1

[PATCH 2/5] Input: drv260x - sleep between polling GO bit

[Luca Weiss]

When doing the initial startup there's no need to poll without any
delay and spam the I2C bus.

Let's sleep 15ms between each attempt, which is the same time as used
in the vendor driver.

Fixes: 7132fe4f5687 ("Input: drv260x - add TI drv260x haptics driver")
Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
---
 drivers/input/misc/drv260x.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/input/misc/drv260x.c b/drivers/input/misc/drv260x.c
index e95c4e775b5d..884d43eb4b61 100644
--- a/drivers/input/misc/drv260x.c
+++ b/drivers/input/misc/drv260x.c
@@ -435,6 +435,7 @@ static int drv260x_init(struct drv260x_data *haptics)
 	}
 
 	do {
+		usleep_range(15000, 15500);
 		error = regmap_read(haptics->regmap, DRV260X_GO, &cal_buf);
 		if (error) {
 			dev_err(&haptics->client->dev,

-- 
2.40.1

[PATCH 3/5] Input: drv260x - remove unused .reg_defaults

[Luca Weiss]

Since the driver has disabled regmap caching with REGCACHE_NONE, it's
warning us that we provide defaults that are not used. Remove them.

  [    0.561159] drv260x-haptics 0-005a: No cache used with register defaults set!

Fixes: 7132fe4f5687 ("Input: drv260x - add TI drv260x haptics driver")
Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
---
 drivers/input/misc/drv260x.c | 40 ----------------------------------------
 1 file changed, 40 deletions(-)

diff --git a/drivers/input/misc/drv260x.c b/drivers/input/misc/drv260x.c
index 884d43eb4b61..a7e3120bdc13 100644
--- a/drivers/input/misc/drv260x.c
+++ b/drivers/input/misc/drv260x.c
@@ -193,44 +193,6 @@ struct drv260x_data {
 	int overdrive_voltage;
 };
 
-static const struct reg_default drv260x_reg_defs[] = {
-	{ DRV260X_STATUS, 0xe0 },
-	{ DRV260X_MODE, 0x40 },
-	{ DRV260X_RT_PB_IN, 0x00 },
-	{ DRV260X_LIB_SEL, 0x00 },
-	{ DRV260X_WV_SEQ_1, 0x01 },
-	{ DRV260X_WV_SEQ_2, 0x00 },
-	{ DRV260X_WV_SEQ_3, 0x00 },
-	{ DRV260X_WV_SEQ_4, 0x00 },
-	{ DRV260X_WV_SEQ_5, 0x00 },
-	{ DRV260X_WV_SEQ_6, 0x00 },
-	{ DRV260X_WV_SEQ_7, 0x00 },
-	{ DRV260X_WV_SEQ_8, 0x00 },
-	{ DRV260X_GO, 0x00 },
-	{ DRV260X_OVERDRIVE_OFF, 0x00 },
-	{ DRV260X_SUSTAIN_P_OFF, 0x00 },
-	{ DRV260X_SUSTAIN_N_OFF, 0x00 },
-	{ DRV260X_BRAKE_OFF, 0x00 },
-	{ DRV260X_A_TO_V_CTRL, 0x05 },
-	{ DRV260X_A_TO_V_MIN_INPUT, 0x19 },
-	{ DRV260X_A_TO_V_MAX_INPUT, 0xff },
-	{ DRV260X_A_TO_V_MIN_OUT, 0x19 },
-	{ DRV260X_A_TO_V_MAX_OUT, 0xff },
-	{ DRV260X_RATED_VOLT, 0x3e },
-	{ DRV260X_OD_CLAMP_VOLT, 0x8c },
-	{ DRV260X_CAL_COMP, 0x0c },
-	{ DRV260X_CAL_BACK_EMF, 0x6c },
-	{ DRV260X_FEEDBACK_CTRL, 0x36 },
-	{ DRV260X_CTRL1, 0x93 },
-	{ DRV260X_CTRL2, 0xfa },
-	{ DRV260X_CTRL3, 0xa0 },
-	{ DRV260X_CTRL4, 0x20 },
-	{ DRV260X_CTRL5, 0x80 },
-	{ DRV260X_LRA_LOOP_PERIOD, 0x33 },
-	{ DRV260X_VBAT_MON, 0x00 },
-	{ DRV260X_LRA_RES_PERIOD, 0x00 },
-};
-
 #define DRV260X_DEF_RATED_VOLT		0x90
 #define DRV260X_DEF_OD_CLAMP_VOLT	0x90
 
@@ -453,8 +415,6 @@ static const struct regmap_config drv260x_regmap_config = {
 	.val_bits = 8,
 
 	.max_register = DRV260X_MAX_REG,
-	.reg_defaults = drv260x_reg_defs,
-	.num_reg_defaults = ARRAY_SIZE(drv260x_reg_defs),
 	.cache_type = REGCACHE_NONE,
 };
 

-- 
2.40.1

[PATCH 4/5] Input: drv260x - fix magnitude handling

[Luca Weiss]

First of all, previously the 16-bit magnitude was written as-is to the
device which actually discarded the upper 8 bits since the device has
8-bit registers only. This meant that a strong_magnitude of 0xFF00 would
result in 0. To correct this shift the strong_magnitude / weak_magnitude
input values so we discard the lower 8 bits and keep the upper bits
instead.

Secondly the RTP mode that is used by default interprets the values as
signed (2s complement), so 0x81 = 0%, 0x00 = 50%, 0x7F = 100%. This
doesn't match the FF_RUMBLE interface at all, so let's tell the device
to interpret the data as unsigned instead which gets us 0x00 = 0% and
0xFF = 100%.

As last change switch ERM to using "Closed-Loop Mode, Unidirectional"
instead of "Open-Loop Mode" since it's recommended by the datasheet
compared to open loop and better matches our use case of 0% - 100%
vibration.

Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
---
 drivers/input/misc/drv260x.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/input/misc/drv260x.c b/drivers/input/misc/drv260x.c
index a7e3120bdc13..f5e96b36acda 100644
--- a/drivers/input/misc/drv260x.c
+++ b/drivers/input/misc/drv260x.c
@@ -186,7 +186,7 @@ struct drv260x_data {
 	struct work_struct work;
 	struct gpio_desc *enable_gpio;
 	struct regulator *regulator;
-	u32 magnitude;
+	u8 magnitude;
 	u32 mode;
 	u32 library;
 	int rated_voltage;
@@ -237,10 +237,11 @@ static int drv260x_haptics_play(struct input_dev *input, void *data,
 
 	haptics->mode = DRV260X_LRA_NO_CAL_MODE;
 
+	/* Scale u16 magnitude into u8 register value */
 	if (effect->u.rumble.strong_magnitude > 0)
-		haptics->magnitude = effect->u.rumble.strong_magnitude;
+		haptics->magnitude = effect->u.rumble.strong_magnitude >> 8;
 	else if (effect->u.rumble.weak_magnitude > 0)
-		haptics->magnitude = effect->u.rumble.weak_magnitude;
+		haptics->magnitude = effect->u.rumble.weak_magnitude >> 8;
 	else
 		haptics->magnitude = 0;
 
@@ -266,7 +267,7 @@ static void drv260x_close(struct input_dev *input)
 
 static const struct reg_sequence drv260x_lra_cal_regs[] = {
 	{ DRV260X_MODE, DRV260X_AUTO_CAL },
-	{ DRV260X_CTRL3, DRV260X_NG_THRESH_2 },
+	{ DRV260X_CTRL3, DRV260X_NG_THRESH_2 | DRV260X_RTP_UNSIGNED_DATA },
 	{ DRV260X_FEEDBACK_CTRL, DRV260X_FB_REG_LRA_MODE |
 		DRV260X_BRAKE_FACTOR_4X | DRV260X_LOOP_GAIN_HIGH },
 };
@@ -284,7 +285,7 @@ static const struct reg_sequence drv260x_lra_init_regs[] = {
 		DRV260X_BEMF_GAIN_3 },
 	{ DRV260X_CTRL1, DRV260X_STARTUP_BOOST },
 	{ DRV260X_CTRL2, DRV260X_SAMP_TIME_250 },
-	{ DRV260X_CTRL3, DRV260X_NG_THRESH_2 | DRV260X_ANALOG_IN },
+	{ DRV260X_CTRL3, DRV260X_NG_THRESH_2 | DRV260X_RTP_UNSIGNED_DATA | DRV260X_ANALOG_IN },
 	{ DRV260X_CTRL4, DRV260X_AUTOCAL_TIME_500MS },
 };
 
@@ -299,7 +300,7 @@ static const struct reg_sequence drv260x_erm_cal_regs[] = {
 	{ DRV260X_CTRL1, DRV260X_STARTUP_BOOST },
 	{ DRV260X_CTRL2, DRV260X_SAMP_TIME_250 | DRV260X_BLANK_TIME_75 |
 		DRV260X_IDISS_TIME_75 },
-	{ DRV260X_CTRL3, DRV260X_NG_THRESH_2 | DRV260X_ERM_OPEN_LOOP },
+	{ DRV260X_CTRL3, DRV260X_NG_THRESH_2 | DRV260X_RTP_UNSIGNED_DATA },
 	{ DRV260X_CTRL4, DRV260X_AUTOCAL_TIME_500MS },
 };
 

-- 
2.40.1

[PATCH 5/5] ARM: dts: qcom: apq8026-huawei-sturgeon: Add vibrator

[Luca Weiss]

The watch has a DRV2605 for haptics. Add a node for it based on the
values found in the downstream board file.

Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
---
 arch/arm/boot/dts/qcom-apq8026-huawei-sturgeon.dts | 28 ++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/arch/arm/boot/dts/qcom-apq8026-huawei-sturgeon.dts b/arch/arm/boot/dts/qcom-apq8026-huawei-sturgeon.dts
index d64096028ab1..eb73b992a696 100644
--- a/arch/arm/boot/dts/qcom-apq8026-huawei-sturgeon.dts
+++ b/arch/arm/boot/dts/qcom-apq8026-huawei-sturgeon.dts
@@ -7,6 +7,7 @@
 
 #include "qcom-msm8226.dtsi"
 #include "qcom-pm8226.dtsi"
+#include <dt-bindings/input/ti-drv260x.h>
 
 /delete-node/ &adsp_region;
 
@@ -68,6 +69,26 @@ &adsp {
 	status = "okay";
 };
 
+&blsp1_i2c2 {
+	clock-frequency = <384000>;
+
+	status = "okay";
+
+	vibrator@5a {
+		compatible = "ti,drv2605";
+		reg = <0x5a>;
+		enable-gpios = <&tlmm 60 GPIO_ACTIVE_HIGH>;
+
+		mode = <DRV260X_ERM_MODE>;
+		library-sel = <DRV260X_ERM_LIB_D>;
+		vib-rated-mv = <2765>;
+		vib-overdrive-mv = <3525>;
+
+		pinctrl-0 = <&vibrator_default_state>;
+		pinctrl-names = "default";
+	};
+};
+
 &blsp1_i2c5 {
 	clock-frequency = <384000>;
 
@@ -347,6 +368,13 @@ reset-pins {
 		};
 	};
 
+	vibrator_default_state: vibrator-default-state {
+		pins = "gpio59", "gpio60";
+		function = "gpio";
+		drive-strength = <2>;
+		bias-pull-down;
+	};
+
 	wlan_hostwake_default_state: wlan-hostwake-default-state {
 		pins = "gpio66";
 		function = "gpio";

-- 
2.40.1

Re: [PATCH 5/5] ARM: dts: qcom: apq8026-huawei-sturgeon: Add vibrator

[Konrad Dybcio]

On 30.04.2023 20:20, Luca Weiss wrote:
> The watch has a DRV2605 for haptics. Add a node for it based on the
> values found in the downstream board file.
> 
> Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Konrad
>  arch/arm/boot/dts/qcom-apq8026-huawei-sturgeon.dts | 28 ++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/qcom-apq8026-huawei-sturgeon.dts b/arch/arm/boot/dts/qcom-apq8026-huawei-sturgeon.dts
> index d64096028ab1..eb73b992a696 100644
> --- a/arch/arm/boot/dts/qcom-apq8026-huawei-sturgeon.dts
> +++ b/arch/arm/boot/dts/qcom-apq8026-huawei-sturgeon.dts
> @@ -7,6 +7,7 @@
>  
>  #include "qcom-msm8226.dtsi"
>  #include "qcom-pm8226.dtsi"
> +#include <dt-bindings/input/ti-drv260x.h>
>  
>  /delete-node/ &adsp_region;
>  
> @@ -68,6 +69,26 @@ &adsp {
>  	status = "okay";
>  };
>  
> +&blsp1_i2c2 {
> +	clock-frequency = <384000>;
> +
> +	status = "okay";
> +
> +	vibrator@5a {
> +		compatible = "ti,drv2605";
> +		reg = <0x5a>;
> +		enable-gpios = <&tlmm 60 GPIO_ACTIVE_HIGH>;
> +
> +		mode = <DRV260X_ERM_MODE>;
> +		library-sel = <DRV260X_ERM_LIB_D>;
> +		vib-rated-mv = <2765>;
> +		vib-overdrive-mv = <3525>;
> +
> +		pinctrl-0 = <&vibrator_default_state>;
> +		pinctrl-names = "default";
> +	};
> +};
> +
>  &blsp1_i2c5 {
>  	clock-frequency = <384000>;
>  
> @@ -347,6 +368,13 @@ reset-pins {
>  		};
>  	};
>  
> +	vibrator_default_state: vibrator-default-state {
> +		pins = "gpio59", "gpio60";
> +		function = "gpio";
> +		drive-strength = <2>;
> +		bias-pull-down;
> +	};
> +
>  	wlan_hostwake_default_state: wlan-hostwake-default-state {
>  		pins = "gpio66";
>  		function = "gpio";
> 

Re: [PATCH 0/5] Improvements for drv260x driver and add to Huawei Watch

[Dmitry Torokhov]

On Sun, Apr 30, 2023 at 08:20:52PM +0200, Luca Weiss wrote:
> Clean up some issues with the driver I've noticed while working on it,
> then fix the magnitude handling which previously resulted the driver
> discarding the upper 8 bits of the magnitude value, and finally we can
> add the drv260x to the APQ8026 Huawei Watch.
> 
> While the "fix magnitude handling" commit technically changes behavior
> and could change behavior of existing user space applications that deal
> with this quirky behavior, it's definitely not correct and results in
> very unexpected behavior if the user space doesn't look out for it
> (meaning only use 0x00-0xFF magnitude, and also know that the magnitude
> was interpreted as signed value, see more details in the commit message
> there).
> 
> There's currently only one other upstream user of the driver,
> qcom/msm8996-xiaomi-gemini where I've added the dts authors to this
> patch series (Raffaele & Yassine) and some tests on that device would be
> nice since it's operating in LRA mode, mine does in ERM mode.
> 
> Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
> ---
> Luca Weiss (5):
>       Input: drv260x - fix typo in register value define
>       Input: drv260x - sleep between polling GO bit
>       Input: drv260x - remove unused .reg_defaults
>       Input: drv260x - fix magnitude handling
>       ARM: dts: qcom: apq8026-huawei-sturgeon: Add vibrator

Applied patches 1-4.

Thanks.

-- 
Dmitry

Re: (subset) [PATCH 0/5] Improvements for drv260x driver and add to Huawei Watch

[Bjorn Andersson]

On Sun, 30 Apr 2023 20:20:52 +0200, Luca Weiss wrote:
> Clean up some issues with the driver I've noticed while working on it,
> then fix the magnitude handling which previously resulted the driver
> discarding the upper 8 bits of the magnitude value, and finally we can
> add the drv260x to the APQ8026 Huawei Watch.
> 
> While the "fix magnitude handling" commit technically changes behavior
> and could change behavior of existing user space applications that deal
> with this quirky behavior, it's definitely not correct and results in
> very unexpected behavior if the user space doesn't look out for it
> (meaning only use 0x00-0xFF magnitude, and also know that the magnitude
> was interpreted as signed value, see more details in the commit message
> there).
> 
> [...]

Applied, thanks!

[5/5] ARM: dts: qcom: apq8026-huawei-sturgeon: Add vibrator
      commit: ad318f9ce5a2e815816bfcf7f187a3ac32905523

Best regards,
-- 
Bjorn Andersson <andersson@kernel.org>