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: i2c_ll_stm32: Use I2C API flags #4572

Merged
merged 2 commits into from
Nov 6, 2017

Conversation

ydamigos
Copy link
Collaborator

STM32 I2C driver doesn't use the I2C API flags STOP/RESTART,
instead it uses its own RESTART flag. As a result, I2C API's
i2c_burst_write* funtions doesn't work. This patch makes
STM32 I2C driver to use I2C API flags.

Tested on: 96b_carbon, olimexino_stm32 (i2c_ll_stm32_v1)
Tested on: stm32f3_disco, disco_l475_iot1 (i2c_ll_stm32_v2)

Fixes: #4459

Signed-off-by: Yannis Damigos [email protected]
Signed-off-by: Daniel Wagenknecht [email protected]

@ydamigos
Copy link
Collaborator Author

@erwango @dwagenk Could you please check it?

@ydamigos
Copy link
Collaborator Author

@ldts Could you also check it?

Copy link
Contributor

@dwagenk dwagenk left a comment

Choose a reason for hiding this comment

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

Checked v2 driver on stm32f3_disco in interrupt and non-interrupt mode again.
Works as expected, so good to merge.

if (OPERATION(current) != OPERATION(next)) {
flags = I2C_MSG_RESTART;
if (!(next->flags & I2C_MSG_RESTART)) {
return -EINVAL;
Copy link
Collaborator

Choose a reason for hiding this comment

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

This seems broken for me, this means if the messages given to the API is broken, this will leave the bus in an inconsistent state

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated. Thanks for catching this.


/* Stop condition is required for the last message */
if ((num_msgs == 1) && !(current->flags & I2C_MSG_STOP)) {
return -EINVAL;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated.

while (!LL_I2C_IsActiveFlag_SB(i2c)) {
;
}
if (msg->flags & I2C_MSG_RESTART) {
Copy link
Member

Choose a reason for hiding this comment

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

Would you add empty lines between logical sections?
This whole function has gotten hard to read

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

@galak galak self-assigned this Oct 31, 2017
@galak galak force-pushed the arm branch 2 times, most recently from 8842a2b to d60d80f Compare November 3, 2017 18:54
STM32 I2C driver doesn't use the I2C API flags STOP/RESTART,
instead it uses its own RESTART flag. As a result, I2C API's
i2c_burst_write* funtions doesn't work. This patch makes
STM32 I2C driver to use I2C API flags.

Tested on: 96b_carbon, olimexino_stm32 (i2c_ll_stm32_v1)
Tested on: stm32f3_disco, disco_l475_iot1 (i2c_ll_stm32_v2)

Fixes: zephyrproject-rtos#4459

Signed-off-by: Yannis Damigos <[email protected]>
Signed-off-by: Daniel Wagenknecht <[email protected]>
Event ISR checks if the TX/RX interrupts is enabled instead
of the TXIS/RXNE interrupt status flags. Use the TXIS/RXNE
interrupt status flags to check which interrupt event
happened.

Signed-off-by: Yannis Damigos <[email protected]>
Signed-off-by: Daniel Wagenknecht <[email protected]>
@ydamigos
Copy link
Collaborator Author

ydamigos commented Nov 4, 2017

Rebased to the latest arm branch. @superna9999 @erwango Any other comments?

Copy link
Member

@erwango erwango left a comment

Choose a reason for hiding this comment

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

Would enjoy a bit more blank lines here and there, but otherwise LGTM.

@galak galak merged commit d818a8d into zephyrproject-rtos:arm Nov 6, 2017
@ydamigos ydamigos deleted the stm32-i2c branch November 6, 2017 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
platform: STM32 ST Micro STM32
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants