Conversation
Gelbpunkt
left a comment
There was a problem hiding this comment.
I had a very rough look at this. It's better than your last revision, but still smells like downstream. Please make sure to also run clang-format on the driver.
| Say 'y' here to include support for the H-Bridge Stepper | ||
| Motor Driver. The MISC peripheral holds the pwm and the | ||
| driver provides an API to drive stepper motor. |
There was a problem hiding this comment.
This description and the config option name are way too generic. Describe the actual hardware.
There was a problem hiding this comment.
Added motor controller manufacturer and model. And info about its purpose.
| #define UP 1 | ||
| #define DOWN 0 | ||
|
|
||
| /* Move back add delta ms to guarantee go home success */ |
There was a problem hiding this comment.
What? This comment makes no sense
There was a problem hiding this comment.
I didn't actually change this file. It is straight from downstream. I think what they meant is to add a few ms to the motor work time to make sure that camera hid completely.
| pstate.period = pwm->period_ns; | ||
| pstate.duty_cycle = pwm->duty_ns; | ||
|
|
||
| pr_debug("enable %d, period %llu, duty %llu\n", pstate.enabled, pstate.period, pstate.duty_cycle); |
There was a problem hiding this comment.
I would recommend using dev_dbg instead of pr_debug
There was a problem hiding this comment.
I have wrapped dev_dbg in a macro to use function:line prefix formatting. Replaced all pr_debug with it.
| { | ||
| struct drv8846_soc_ctrl *mctrl = filp->private_data; | ||
|
|
||
| pr_debug("mctrl %p, application fasync!", mctrl); |
There was a problem hiding this comment.
These log messages do not add anything of value. Logs should be informative.
There was a problem hiding this comment.
Some debug messages have been removed and for some just added more context.
other necessary gpio nodes Signed-off-by: Stanislaw Dac <stadac@disroot.org> arm64: dts: qcom: sm7150-xiaomi-davinci: fix dts code style Signed-off-by: Stanislaw Dac <stadac@disroot.org>
…Motor Controller Signed-off-by: Stanislaw Dac <stadac@disroot.org> dt-bindings: pwm: ti,drv8846: remove the "ti" prefix from the properties Signed-off-by: Stanislaw Dac <stadac@disroot.org>
Signed-off-by: Stanislaw Dac <stadac@disroot.org> misc: drv8846: TI DRV8846 driver cleanup Signed-off-by: Stanislaw Dac <stadac@disroot.org>
237f378 to
a74334d
Compare
This PR introduces a DRV8846 motor driver (and DTS node) which is responsible for the pop-up camera mechanism.
It has been tested on Xiaomi 9T. Xiaomi 9T Pro (xiaomi-raphael) also uses this solution.
This will be useful for testing the selfie camera later.
Contains:
Can be tested with this GUI: https://gist.github.com/stsdc/20dcc6e83b5ae23f25523ac0c8567085
Proof of work: https://mas.to/@stsdc/116026121171513287