DRV2605 DT Property Cleanup #76969
DRV2605 DT Property Cleanup #76969
Conversation
Documents the enable GPI and the input/trigger GPI in the dt bindings. Signed-off-by: Ricardo Rivera-Matos <[email protected]>
Removes the "ti_" prefix form the the device tree properties. Signed-off-by: Ricardo Rivera-Matos <[email protected]>
fabiobaltieri
left a comment
fabiobaltieri
left a comment
There was a problem hiding this comment.
drv2605_pm_state also seems unused, also should this allow setting DRV2605_REG_RATED_VOLTAGE and DRV2605_REG_OVERDRIVE_CLAMP_VOLTAGE? I think they are fairly important parameters
Removes an unused enum defining the DRV2605 power states. This enum is dead code so it is being removed. Signed-off-by: Ricardo Rivera-Matos <[email protected]>
drivers/haptics/drv2605.c
Outdated
| .en_gpio = GPIO_DT_SPEC_INST_GET_OR(inst, en_gpios, {}), \ | ||
| .in_trig_gpio = GPIO_DT_SPEC_INST_GET_OR(inst, in_trig_gpios, {}), \ | ||
| .feedback_brake_factor = DT_INST_ENUM_IDX_OR(inst, feedback_brake_factor, 3), \ | ||
| .loop_gain = DT_INST_ENUM_IDX_OR(inst, loop_gain, 2), \ | ||
| .actuator_mode = DT_INST_ENUM_IDX_OR(inst, actuator_mode, 0), \ |
There was a problem hiding this comment.
The default values should actually be set (and maybe more importantly, documented) in the binding... I think?
There was a problem hiding this comment.
(I know this is not strictly related to this change, but I missed it in initial review and am catching this now as I'm actually starting to play with the driver). Thanks!
There was a problem hiding this comment.
The default values should actually be set (and maybe more importantly, documented) in the binding... I think?
Will do!
as I'm actually starting to play with the driver
Don't have too much fun buzzing around.
Will drop, good catch.
Sure, ack! |
rriveramcrus
force-pushed
the
drv2605-bindings-cleanup
branch
from
c22b70d to
55674ee
Compare
kartben
left a comment
kartben
left a comment
There was a problem hiding this comment.
Hopefully @fabiobaltieri can chime in an confirm I am not making unreasonable requests 😅
BTW the MikroE Vibro 4 click I'm using seems to use a motor that requires LRA open-loop mode. No idea what this actually means 😅 but it's true the motor vibrates more smoothly when I hard code CTRL3.LRA_OPEN_LOOP=1. It feels like there are many other flags from CTRL1/2/3 that would be worth exposing, and I wonder what's the best way to do this, since there's certainly a lot of things that can be tuned there (so to be clear: I'm not asking you to add them in this PR). Regarding open loop specifically, it feels like having an open-loop boolean that sets either LRA_OPEN_LOOP or ERM_OPEN_LOOP based on the current mode would be a good starting point?
Thanks!
rriveramcrus
force-pushed
the
drv2605-bindings-cleanup
branch
from
55674ee to
22635f3
Compare
I will hardcode this in the driver based on the actuator mode selected. I am opposed to adding a property to control open/closed loop operation without adding support for calibration and OTP burning. My DRV2605 breakout board does not make it easy to swap out actuators (it's cheap) and adding calibration support would require testing a couple different actuators. I defer adding calibration and OTP burning to a future PR. |
rriveramcrus
force-pushed
the
drv2605-bindings-cleanup
branch
2 times, most recently
from
096a881 to
14ca761
Compare
Requires the actuator-mode property to be set as there is no safe default value. Signed-off-by: Ricardo Rivera-Matos <[email protected]>
Adds the default value for the existing properties to the device tree and fall back on those values at initialization if necessary. Signed-off-by: Ricardo Rivera-Matos <[email protected]>
rriveramcrus
force-pushed
the
drv2605-bindings-cleanup
branch
from
14ca761 to
4680131
Compare
|
Hey played a bit with the driver in this PR, I have an unknown LRA salvaged from a phone connected, seems to work quite nicely, just the comment above about the macro. |
Adds support for boot time configuration of the rated voltage at initialization via "vib-rated-mv" devicetree property. Signed-off-by: Ricardo Rivera-Matos <[email protected]>
Adds "vib-overdrive-mv" device tree property to configure the overdrive clamp at initialization. Signed-off-by: Ricardo Rivera-Matos <[email protected]>
Enables open loop operation when initializing into LRA mode. Open loop operation is enabled by default for ERM mode. This is hard coded for LRA mode because calibration is currently unsupported. Signed-off-by: Ricardo Rivera-Matos <[email protected]>
rriveramcrus
force-pushed
the
drv2605-bindings-cleanup
branch
from
4680131 to
341aeeb
Compare
6 participants
#76343 (comment)
Address the issues raised by this comment: