Re: [PATCH] ARM: dts: qcom: msm8960: expressatt Add PWM vibrator

[Rudraksha Gupta <guptarud@gmail.com>]

Hello Konrad,


On 12/16/25 06:22, Konrad Dybcio wrote:
> On 12/11/25 11:23 AM, Rudraksha Gupta via B4 Relay wrote:
>> From: Rudraksha Gupta <guptarud@gmail.com>
>>
>> Add a pwm-vibrator to expressatt. Currently this vibrates only at 100%
>>
>> Signed-off-by: Rudraksha Gupta <guptarud@gmail.com>
>> ---
>> Add a pwm-vibrator to expressatt. Currently this vibrates only at 100%
>>
>> Link:
>> - https://github.com/LineageOS/android_kernel_samsung_d2/blob/stable/cm-12.0-YNG4N/arch/arm/mach-msm/board-express.c#L1767
>> - https://github.com/LineageOS/android_kernel_samsung_d2/blob/stable/cm-12.0-YNG4N/drivers/motor/Makefile#L5
>>
>> Test:
>> =====================
>> samsung-expressatt:~$ dmesg | grep vibra
>> [   79.892226] input: pwm-vibrator as /devices/platform/vibrator/input/input4
>> samsung-expressatt:~$ fftest /dev/input/event4
>> Force feedback test program.
>> HOLD FIRMLY YOUR WHEEL OR JOYSTICK TO PREVENT DAMAGES
>>
>> Device /dev/input/event4 opened
>> Features:
>>    * Absolute axes:
>>      [00 00 00 00 00 00 00 00 ]
>>    * Relative axes:
>>      [00 00 ]
>>    * Force feedback effects types: Periodic, Rumble, Gain,
>>      Force feedback periodic effects: Square, Triangle, Sine,
>>      [00 00 00 00 00 00 00 00 00 00 03 07 01 00 00 00 ]
>>    * Number of simultaneous effects: 16
>>
>> Setting master gain to 75% ... OK
>> Uploading effect #0 (Periodic sinusoidal) ... OK (id 0)
>> Uploading effect #1 (Constant) ... Error: Invalid argument
>> Uploading effect #2 (Spring) ... Error: Invalid argument
>> Uploading effect #3 (Damper) ... Error: Invalid argument
> Looks like this is because GP1_CLK does not implement .set_duty_cycle..
>
> The downstream driver you linked to does so in a terribly hacky way
> (from the vibrator driver and not the clock driver):
>
> https://github.com/LineageOS/android_kernel_samsung_d2/blob/cm-14.1/drivers/motor/immvibespi.c#L53
>
> Upstream, we have an implementation for clk_rcg*2*_ops, whereas 8960
> uses clk_rcg_ops.
>
> They look very similar though, perhaps you can hack it up..
>
> One thing to note is that you're passing GP1_CLK to the clk-pwm (as
> you should), but we need to do clk_set_duty_cycle on its parent,
> GP1_CLK*_SRC*. The framework will take care of this:
>
> ```
> --- drivers/clk/clk.c
> 	if (!core->ops->get_duty_cycle)
> 		return clk_core_update_duty_cycle_parent_nolock(core);
> ```
>
> so long as you add CLK_DUTY_CYCLE_PARENT to the child
>
> [...]


Sorry for the delay, but I'm struggling to get this implemented. I have 
my in progress work here:

https://codeberg.org/LogicalErzor/linux/commit/4e7f94d9fdd1e4e1d688b2eb518494d710f157fb


I was able to get rid of the -16 error that was getting spammed when 
using fftest, but it is still showing

Uploading effect #1 (Constant) ... Error: Invalid argument
Uploading effect #2 (Spring) ... Error: Invalid argument
Uploading effect #3 (Damper) ... Error: Invalid argument


If you have any other insight on how to make the vibrator variable in 
intensity (it's still either 0% or 100%), please feel free to let me 
know. I will continue trying to hack away at it to see if I can get the 
variable intensity to work via fftest.
>> +	/* TODO: Vary the frequency besides being 0% or 100% */
>> +	vibrator {
>> +		compatible = "pwm-vibrator";
>> +		pwms = <&gp1_pwm 0 54347 0>;  /* ~18.4 kHz */
>> +		pwm-names = "enable";
>> +		enable-gpios = <&pm8921_gpio 4 GPIO_ACTIVE_HIGH>;
>> +		vcc-supply = <&vdd_haptics>;
>> +
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&vib_pwm_gpio>;
> pinctrl-n
> pinctrl-names
>
> please, both occurences
>
>> +	};
>> +
>> +	vdd_haptics: vdd-haptics-regulator {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "vdd_haptics";
>> +		gpio = <&tlmm 47 GPIO_ACTIVE_HIGH>;
>> +		enable-active-high;
>> +		regulator-boot-on;
>> +
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&haptics_pwr_en>;
>> +	};
>>   };
>>   
>>   &gsbi2 {
>> @@ -241,6 +270,21 @@ touchkey_i2c_pins: touchkey-i2c-state {
>>   		bias-disable;
>>   	};
>>   
>> +	haptics_pwr_en: haptics-pwr-en-state {
>> +		pins = "gpio47";
>> +		function = "gpio";
>> +		drive-strength = <2>;
>> +		bias-disable;
>> +		output-low;
> Drop output-low, the driver should take care of setting the state
>
> Konrad