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: st: replace i2/3c_burst_write #79347

Merged
merged 1 commit into from
Jan 29, 2025

Conversation

XenuIsWatching
Copy link
Member

i2/3c_burst_write comes with a warning where this combined write synthesized by thsi API may not be supported by all I2/3C devices. Replace with i2c_write instead with a buffer combining the address and data.

teburd
teburd previously approved these changes Oct 3, 2024
ubieda
ubieda previously approved these changes Oct 16, 2024
@kartben
Copy link
Collaborator

kartben commented Nov 30, 2024

@MaureenHelm please revisit

@kartben
Copy link
Collaborator

kartben commented Jan 10, 2025

@MaureenHelm please revisit

@MaureenHelm ping :)
You might also find this useful:) https://pr-dashboard.zephyrproject.io/?username=MaureenHelm

Copy link
Collaborator

@avisconti avisconti left a comment

Choose a reason for hiding this comment

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

I think that in this PR the stmemsc Kconfig is missing...

@@ -21,6 +21,12 @@ int stmemsc_i3c_write(void *stmemsc,
uint8_t reg_addr, uint8_t *value, uint8_t len)
{
struct i3c_device_desc *target = **(struct i3c_device_desc ***)stmemsc;
uint8_t buf[17];
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use the macro also here. Maybe using a common name among I2C and I3C?

Copy link
Member Author

Choose a reason for hiding this comment

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

oh.. right, oops

@@ -21,7 +21,14 @@ int stmemsc_i2c_read(const struct i2c_dt_spec *stmemsc,
int stmemsc_i2c_write(const struct i2c_dt_spec *stmemsc,
uint8_t reg_addr, uint8_t *value, uint8_t len)
{
return i2c_burst_write_dt(stmemsc, reg_addr, value, len);
uint8_t buf[CONFIG_STMEMSC_I2C_WRITE_BUFFER_SIZE];
Copy link
Collaborator

Choose a reason for hiding this comment

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

Actually I was expecting to use it also externally to stmemsc, so it would be better (maybe) to use it in this way:

#define STMEMSC_WRITE_BUFFER_SIZE CONFIG_STMEMSC_I2C_WRITE_BUFFER_SIZE

Or maybe it is not necessary...

i2/3c_burst_write comes with a warning where this combined write
synthesized by thsi API may not be supported by all I2/3C
devices. Replace with i2c_write instead with a buffer combining
the address and data.

The Kconfig STMEMSC_I2C_WRITE_BUFFER_SIZE was added to set the
size of the buffer pushed to the stack.

Signed-off-by: Ryan McClelland <[email protected]>
@avisconti avisconti requested a review from ubieda January 29, 2025 10:11
@kartben kartben merged commit 8f0ffbb into zephyrproject-rtos:main Jan 29, 2025
24 checks passed
5ae29c added a commit to 5ae29c/zephyr that referenced this pull request Feb 3, 2025
Previously, the Kconfig option CONFIG_STMEMSC_I3C_I2C_WRITE_BUFFER_SIZE has been introduced to replace i2c_burst_write with i2c_write using a buffer combining the address and data. See zephyrproject-rtos#79347

Add missing buffer size config to stmemsc_i2c_write_incr to replace fixed buffer size.

Signed-off-by: Jonas Spinner <[email protected]>
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.

7 participants