search

PabloPL / linux

Linux kernel source tree
Other
17 stars 0 forks source link

Fix vibrator warning #9

Closed xc-racer99 closed 5 years ago

xc-racer99 commented 6 years ago

See https://github.com/PabloPL/linux/commits/for-upstream/vibrator

@PabloPL could you please test this on the i9000? You can compile the script at https://git.collabora.com/cgit/user/sre/rumble-test.git/plain/rumble-test.c or use fftest.

It needs CONFIG_PWM=y CONFIG_PWM_SAMSUNG=y CONFIG_INPUT_MISC=y CONFIG_INPUT_PWM_VIBRA=y CONFIG_REGULATOR_FIXED_VOLTAGE=y in the defconfig

PabloPL commented 6 years ago

After adding also CONFIG_REGULATOR_FIXED_VOLTAGE=y it is working fine for me on i9000.

You can add to commit "ARM: dts: s5pv210: aries: Add support for pwm-vibrator" my "Tested-by" (like it's done for example https://patchwork.kernel.org/patch/10354345/)

Of course we'll need separate comit which will enable those options (in defconfig), when submiting to mainline.

Thanks for great work.

xc-racer99 commented 6 years ago

Did you want to squish the defconfig and dts changes into v2 of your aries patchset? I don't mind if you do squish them into the main aries dtb commit.

I've updated the branch with the Tested-by and the defconfig changes.

PabloPL commented 6 years ago

What about patch "input: misc: pwm-vibra: Prevent unbalanced regulator" ? Will this work without it? I could squash defconfig and dts, but i'm not sure if it's good idea to send them in this v2. I think it will be better to send them in next patchset (maybe there will be more things to add, like for example touchkeys, lcd etc). Let's have aries in mainline first.

xc-racer99 commented 6 years ago

Well, it works without the unbalanced regulator patch, it just throws up a big warning about unbalanced regular during probe and whenever in use.

xc-racer99 commented 5 years ago

Vibrator has been submitted and accepted along with Aries V2, but throws a warning about unbalanced regulators. https://github.com/PabloPL/linux/commit/a753bd7a26e02d2750cb1a7a7fdcfc9fb041fca0 should probably be submitted upstream at some point.

PabloPL commented 5 years ago

Offtopic: maybe we could use regulator_is_enabled rather than storing regulator state (which it has internally probably) in another variable vcc_on (need testing and verification). It would look little "nicer". Whole fix is good, but maybe we could make it look better :).

xc-racer99 commented 5 years ago

Definitely could look into that. I basically renamed the amplifier_on property from the pwm-beeper driver.

PabloPL commented 5 years ago

I just found this api (regulator_is_enabled) now.

PabloPL commented 5 years ago

Updated patch (using regulator_is_enabled) on branch for-upstream/pwm-vibra-fix. Patch send to mainline for review.

PabloPL commented 5 years ago

Had to get back to original look of patch (using local variable to store information if regulator is enabled). Resend for review (also added patch which changes order of disable calls in pwm-vibra driver, sugested in review).

PabloPL commented 5 years ago

Patches accepted in mainline (currently in linux-next).

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=3ca232df9921f083c3b37ba5fbc76f4d9046268b Input: pwm-vibra - prevent unbalanced regulator https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=94803aef3533676194c772383472636c453e3147 Input: pwm-vibra - stop regulator after disabling pwm, not before (sugested after review of first one)