Skip to content

Conversation

@alexanderwachter
Copy link
Member

@alexanderwachter alexanderwachter commented Dec 19, 2021

Make sure that all access to the msg_sram is 32 bit aligned.

Fix for #41255 and #41074

@alexanderwachter alexanderwachter added area: CAN Hotfix Fix for issues blocking development, i.e. upstream CI issues, tests failing in upstream CI , etc. labels Dec 19, 2021
@alexanderwachter
Copy link
Member Author

alexanderwachter commented Dec 19, 2021

@pdietl Have made this hotfix. We can switch to your memcpy when it is through.

@alexanderwachter alexanderwachter force-pushed the can/fixMcanAlignment branch 2 times, most recently from a5b3483 to c3f8df2 Compare December 19, 2021 14:08
Copy link
Member

@henrikbrixandersen henrikbrixandersen left a comment

Choose a reason for hiding this comment

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

Thanks.

@alexanderwachter alexanderwachter force-pushed the can/fixMcanAlignment branch 2 times, most recently from c91f7db to 9c70154 Compare December 19, 2021 15:40
Make sure that all access to the msg_sram
is 32 bit aligned.

Signed-off-by: Alexander Wachter <alexander@wachter.cloud>
Copy link
Member

@martinjaeger martinjaeger left a comment

Choose a reason for hiding this comment

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

Thanks. Agreed as a hotfix, though I would prefer to finally put the generic memcpy32 functions into lib/ as suggested in #41273 by @pdietl.

@alexanderwachter
Copy link
Member Author

The generic memcpy32 is not generic enough. I prefer a version with volatile qualifier so that the compiler is forced to copy 32bit in the order specified.

@erwango
Copy link
Member

erwango commented Dec 20, 2021

The generic memcpy32 is not generic enough. I prefer a version with volatile qualifier so that the compiler is forced to copy 32bit in the order specified.

I'm also puzzled to have such function implemented in a driver. If memcpy32 is not enough, maybe volatile_memcpy32 could also be added to the lib ?

@alexanderwachter
Copy link
Member Author

@erwango this is a hotfix. As soon as we have something generic, we will switch to that instead.

@carlescufi carlescufi merged commit 11d340f into zephyrproject-rtos:main Dec 20, 2021
@carlescufi
Copy link
Member

I merged this to get the hotfix in, but I completely agree we need to move those functions out of the driver.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: CAN Hotfix Fix for issues blocking development, i.e. upstream CI issues, tests failing in upstream CI , etc.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants