Skip to content
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

drivers: sensor: ams/tsl2591: Remove initial reset #83508

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mabembedded
Copy link

@mabembedded mabembedded commented Jan 2, 2025

The TSL2591 driver fails to initialize because the sensor responds with a NACK on the initial RESET. Although the datasheet from Adafruit claims that this should be valid (RESET is part of the CONTROL register), other sample non-Zephyr drivers provided by Arduino don't explicitly reset the sensor on initialization (see
https://github.com/adafruit/Adafruit_TSL2591_Library/blob/master/Adafruit_TSL2591.cpp).

After removing this initial RESET, the driver initializes successfully.

The TSL2591 driver fails to initialize because the sensor responds with a
NACK on the initial RESET. Although the datasheet from Adafruit claims that
this is operation should be valid (RESET is part of the CONTROL register),
other sample non-Zephyr drivers provided by Arduino don't explicitly reset
the sensor on initialization (see
https://github.com/adafruit/Adafruit_TSL2591_Library/blob/master/Adafruit_TSL2591.cpp).

After removing this initial RESET, the driver initializes successfully.

Signed-off-by: Mohammed Billoo <[email protected]>
@jeppenodgaard
Copy link
Collaborator

Thank you for contributing @mabembedded!

There has been no significant changes to the driver since the initial version, so I guess it only fails under certain conditions. Do you know why it fails?

The reset is probably done to ensure all registers are reset to their default value. Does retrying work or does it continue to fail?

@mabembedded
Copy link
Author

Hi @jeppenodgaard ,

I have an STM32 Disco L475 IOT board (https://www.st.com/en/evaluation-tools/b-l475e-iot01a.html) connected to the Adafruit variant of this sensor (https://www.adafruit.com/product/1980) over I2C, which I think should be a pretty standard connection. I added another attempt to reset the sensor, but it still fails. After tracing the issue down, the STM32 I2C driver reports a NACK response from the sensor for this particular transaction.

Since a retry also fails with a NACK, this could mean that a NACK is expected for this particular transfer, and we shouldn't error out in the driver. Or, it could mean that a RESET is not supported by the sensor, which would be unlikely.

I can connect this sensor to a Linux ARM SBC that I have, modify the upstream Linux driver so that it also performs a RESET first (the Linux driver doesn't perform a RESET), and see if that also results in a NACK. If so, it probably means that a NACK is expected for a RESET?

@jeppenodgaard
Copy link
Collaborator

Since a retry also fails with a NACK, this could mean that a NACK is expected for this particular transfer, and we shouldn't error out in the driver. Or, it could mean that a RESET is not supported by the sensor, which would be unlikely.

Maybe device restarts before it responds with an ACK. Is there another way to detect if the reset is successful?

I can connect this sensor to a Linux ARM SBC that I have, modify the upstream Linux driver so that it also performs a RESET first (the Linux driver doesn't perform a RESET), and see if that also results in a NACK. If so, it probably means that a NACK is expected for a RESET?

I have a feeling it acts the same.
You can also try to fiddle with it with the Zephyr i2c shell command.

@mabembedded
Copy link
Author

I can set the gain through the config register, reset the sensor, and read back the gain to see if it was indeed reset. In any case, do you agree that we should ignore the response from the RESET? Are you able to test this driver to see if you observe the same behavior?

@jeppenodgaard
Copy link
Collaborator

Yes. It seems like it should be ignored.

I do not have the sensor.

Copy link
Member

@MaureenHelm MaureenHelm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or you could do this?

/* Software reset the sensor. Upon issuing a software
* reset command over the I2C interface, the sensor
* immediately resets and does not send any
* acknowledgment (ACK) of the written byte to the
* master. Therefore, do not check the return code of
* the I2C transaction.
*/
config->ops->byte_write(dev, FXOS8700_REG_CTRLREG2,
FXOS8700_CTRLREG2_RST_MASK);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants