-
Notifications
You must be signed in to change notification settings - Fork 8.2k
sensors: lis2dh: disable SDO/SA0 pull-up in case of LIS3DH #19624
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
sensors: lis2dh: disable SDO/SA0 pull-up in case of LIS3DH #19624
Conversation
5424175 to
fa97deb
Compare
pabigot
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fine with the commit that unconditionally builds the helper function.
As for the pull-up control we should be avoiding Kconfig as devicetree is the "way forward". For my suggested approach see below in context.
It would be nice to move the existing irq-gpios and proposed disconnect-sdo-sda0-pull-up into a new st,lis2dh-common.yaml file that is included by both the SPI and I2C variants.
Create a common binding file that will be included by all bindings handled by lis2dh.c driver. For now this includes optional irq-gpios property. Use introduced st,lis2dh-common.yaml in st,lsm303dlhc-accel.yaml in order to support defining irq-gpios. Also improve description of st,lis2dh-i2c.yaml to better match what can be found in st,lis2dh-spi.yaml. Signed-off-by: Marcin Niestroj <[email protected]>
There is little reason to compile lis2dh_reg_field_update() function conditionally, based on enabled features. If it is not used, then linker will drop it anyway. Signed-off-by: Marcin Niestroj <[email protected]>
fa97deb to
3095b94
Compare
|
Based on the review comments I have introduced new device-tree property to disconnect SDO/SA0 pull-up when specified to be fully controlable by user. Hence previous logic based on compatible string has been dropped. Also a commit |
pabigot
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using IS_ENABLED(M) is standard for Kconfig and devicetree macros that have 0 or 1 as their value. It also evaluates to syntactically acceptable content when the macro definition is absent, as with frdm_k64f on test_build_sensors_i_z where there is no define for any LIS2DH devicetree entries. I think you should put it back which should fix the build problems.
Some chips supported by lis2dh driver (such as LIS2DH12 and LIS3DH) contain CTRL_REG0 (1Eh) register to control internal pull-up on SDO/SA0 line (enabled by default). Add disconnect-sdo-sa0-pull-up boolean device-tree property to allow disconnecting pull-up during driver initialization. This allows to save around 180uA at 3.6V in accelerometer power-down mode. Signed-off-by: Marcin Niestroj <[email protected]>
3095b94 to
5fe4844
Compare
avisconti
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
LIS3DH contains CTRL_REG0 (1Eh) register to control internal pull-up on
SDO/SA0 line (enabled by default). Add a helper
LIS2DH_HAS_SDO_SA0_PULL_UP macro, which is defined only when "st,lis3dh"
was passed to compatible list in device-tree. Based on that macro
disable internal pull-up registers, which consume around 180uA at 3.6V
in accelerometer power-down mode.
Signed-off-by: Marcin Niestroj [email protected]