Skip to content

Conversation

@rustypig91
Copy link
Contributor

Update sam0 i2c driver to directly send/receive next message if it is
in the same direction and the current message has no stop or restart
flags. Seems like in some drivers this is the expected behaviour.

Fixes #36857

Signed-off-by: Christoffer Zakrisson [email protected]

@cfriedt
Copy link
Member

cfriedt commented Sep 21, 2021

It's not immediately obvious - what necessitated all of the changes from foo to data->foo, and how are the foo local variables still being used?

@rustypig91
Copy link
Contributor Author

rustypig91 commented Sep 21, 2021

It's not immediately obvious - what necessitated all of the changes from foo to data->foo, and how are the foo local variables still being used?

data->msgs and data->num_msgs are now shared variables with the isr. The isr will update these variables if it directly continues on with the next message.

This could be changed so that the isr instead only receives pointers to the old variables for less code change.

@rustypig91 rustypig91 force-pushed the fix_sam0_i2c_burst_write branch from ee7d890 to f6cd68b Compare September 21, 2021 17:17
Copy link
Member

Choose a reason for hiding this comment

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

The restart flag should be checked on the next message, not the current one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed, also updated line 149 to make it more readable.

@cfriedt
Copy link
Member

cfriedt commented Sep 22, 2021

It's not immediately obvious - what necessitated all of the changes from foo to data->foo, and how are the foo local variables still being used?

data->msgs and data->num_msgs are now shared variables with the isr. The isr will update these variables if it directly continues on with the next message.

This could be changed so that the isr instead only receives pointers to the old variables for less code change.

Hmm... Zephyr-style ISR handlers do have a data pointer but presumably that would used by the driver instance. I see no reason to block (once @anangl's comment is addressed), but it would be great if you could look into that later @rustypig91.

@cfriedt cfriedt self-requested a review September 22, 2021 13:18
@rustypig91 rustypig91 force-pushed the fix_sam0_i2c_burst_write branch from f6cd68b to 4d1f6dd Compare September 22, 2021 15:35
Copy link
Member

Choose a reason for hiding this comment

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

This comment became inaccurate after the recent change regarding the restart flag. It would be nice to update it.

Update sam0 i2c driver to directly send/receive next message if it is
in the same direction and the current message has no stop or restart
flags. Seems like in some drivers this is the expected behaviour.

Fixes zephyrproject-rtos#36857

Signed-off-by: Christoffer Zakrisson <[email protected]>
@rustypig91 rustypig91 force-pushed the fix_sam0_i2c_burst_write branch from 4d1f6dd to 625a97e Compare September 22, 2021 17:51
@cfriedt cfriedt merged commit 2ec4986 into zephyrproject-rtos:main Sep 27, 2021
@bryan-hunt
Copy link

bryan-hunt commented Sep 27, 2021

How was this tested? This change causes every byte read to get NACK'd if num_msgs == 1

@rustypig91
Copy link
Contributor Author

How was this tested? This change causes every byte read to get NACK'd if num_msgs == 1

Tested against an ssd1306 so unfortunately limited testing on read messages :/
#38881 should fix this issue (it works when talking between two Arduino boards).

rustypig91 added a commit to rustypig91/zephyr that referenced this pull request Dec 12, 2021
PR zephyrproject-rtos#38482 made the sam0 i2c send NACK when receiving a single message

Fixes zephyrproject-rtos#38878
Fixes zephyrproject-rtos#41016

Signed-off-by: Christoffer Zakrisson <[email protected]>
cfriedt pushed a commit that referenced this pull request Jan 1, 2022
PR #38482 made the sam0 i2c send NACK when receiving a single message

Fixes #38878
Fixes #41016

Signed-off-by: Christoffer Zakrisson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

i2c_samd0.c burst write not compatible with ssd1306.c

4 participants