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

cdc: fix line_coding aligment #2047

Merged
merged 2 commits into from
Apr 28, 2023
Merged

cdc: fix line_coding aligment #2047

merged 2 commits into from
Apr 28, 2023

Conversation

jbtheou
Copy link

@jbtheou jbtheou commented Apr 28, 2023

While calling tud_cdc_n_get_line_coding, the structure is copied into the destination.

Dump of assembler code for function tud_cdc_n_get_line_coding:
0x000193f4 <+0>: mov.w r2, #2112 @ 0x840
0x000193f8 <+4>: ldr r3, [pc, #20] @ (0x19410
<tud_cdc_n_get_line_coding+28>)
0x000193fa <+6>: mla r0, r2, r0, r3
=> 0x000193fe <+10>: ldr.w r3, [r0, #6]
0x00019402 <+14>: str r3, [r1, #0]

On some platform (tested on LPC55S28), the address needs to be 4-bytes aligned. Without this, the address is

(gdb) p &_cdcd_itf.line_coding
$3 = (cdc_line_coding_t *) 0x40100006 <_cdcd_itf+6>

which leads to a HardFault. With this fix

(gdb) p &_cdcd_itf.line_coding
$5 = (cdc_line_coding_t *) 0x40100008 <_cdcd_itf+8>

and the function can be called properly

Jean-Baptiste Theou and others added 2 commits April 27, 2023 15:48
While calling tud_cdc_n_get_line_coding, the structure is copied into
the destination.

Dump of assembler code for function tud_cdc_n_get_line_coding:
   0x000193f4 <+0>:	mov.w	r2, hathach#2112	@ 0x840
0x000193f8 <+4>:	ldr	r3, [pc, hathach#20]	@ (0x19410
<tud_cdc_n_get_line_coding+28>)
   0x000193fa <+6>:	mla	r0, r2, r0, r3
=> 0x000193fe <+10>:	ldr.w	r3, [r0, hathach#6]
   0x00019402 <+14>:	str	r3, [r1, #0]

On some platform (tested on LPC55S28), the address needs to be 4-bytes
aligned. Without this, the address is

(gdb) p &_cdcd_itf.line_coding
$3 = (cdc_line_coding_t *) 0x40100006 <_cdcd_itf+6>

which leads to a HardFault. With this fix

(gdb) p &_cdcd_itf.line_coding
$5 = (cdc_line_coding_t *) 0x40100008 <_cdcd_itf+8>

and the function can be called properly

Signed-off-by: Jean-Baptiste Theou <[email protected]>
Copy link
Owner

@hathach hathach left a comment

Choose a reason for hiding this comment

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

Thank you, I push an update to add aligned 4 for host driver as well. Actually, this isn't issue since M4+ can access non-aligned data. However iirc lpc55 eventhough is m33 but its highspeed usb can only access 4-bytes aligned memory region which cause the issue. (fs usb probably work just fine).

@jbtheou
Copy link
Author

jbtheou commented Apr 28, 2023

Thanks a lot for extending the fix.
Indeed, used it on SAMD21/51 in the past without issue, so I was a bit confused initially about the issue.

Thank you for the quick review and approval!

@hathach hathach merged commit a41ab41 into hathach:master Apr 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants