Fix for stm32 serial driver not returning -ENOTSUP. #31798
Conversation
zisisadamos
force-pushed
the
stm32_uart9bit_init_fix
branch
from
77e7e94
to
cb3b2a1
Compare
drivers/serial/uart_stm32.c
Outdated
| @@ -336,10 +336,10 @@ static int uart_stm32_configure(const struct device *dev, | |||
| /* Driver doesn't support 5 or 6 databits and potentially 7 or 9 */ | |||
| if ((UART_CFG_DATA_BITS_5 == cfg->data_bits) || | |||
| (UART_CFG_DATA_BITS_6 == cfg->data_bits) | |||
| #ifndef LL_USART_DATAWIDTH_7B | |||
| #ifdef LL_USART_DATAWIDTH_7B | |||
There was a problem hiding this comment.
This code returns -ENOTSUP when LL_USART_DATAWIDTH_7B/9B is not defined (which is the case on some parts). So, from this point of view, this code is correct.
You're modifying in "return -ENOTSUP when LL_USART_DATAWIDTH_7B/9B is defined", which doens't seem correct.
There might be things to fix, but I don't think this is the correct fix.
There was a problem hiding this comment.
You are right I havent taken mcu that dont support 9bit mode into account. I can just do a check on configuration and then return -ENOTSUP. I can have a change ready on the afternoon.
There was a problem hiding this comment.
I propose to purely remove this change.
There was a problem hiding this comment.
Tthis change was removed and requested changes have been made.
drivers/serial/uart_stm32.c
Outdated
| @@ -336,10 +336,10 @@ static int uart_stm32_configure(const struct device *dev, | |||
| /* Driver doesn't support 5 or 6 databits and potentially 7 or 9 */ | |||
| if ((UART_CFG_DATA_BITS_5 == cfg->data_bits) || | |||
| (UART_CFG_DATA_BITS_6 == cfg->data_bits) | |||
| #ifndef LL_USART_DATAWIDTH_7B | |||
| #ifdef LL_USART_DATAWIDTH_7B | |||
There was a problem hiding this comment.
I propose to purely remove this change.
zisisadamos
force-pushed
the
stm32_uart9bit_init_fix
branch
6 times, most recently
from
4f5fa6b
to
271893d
Compare
…TSUP. Fixes driver incorrectly indicating that drive r supports 7 and 9 bit modes. Fixes zephyrproject-rtos#31799 Signed-off-by: Zisis Adamos <[email protected]>
zisisadamos
force-pushed
the
stm32_uart9bit_init_fix
branch
from
271893d
to
0ead6c6
Compare
| ) { | ||
| if ((cfg->data_bits == UART_CFG_DATA_BITS_5) || | ||
| (cfg->data_bits == UART_CFG_DATA_BITS_6) || | ||
| (cfg->data_bits == UART_CFG_DATA_BITS_7) || |
There was a problem hiding this comment.
Some devices support UART_CFG_DATA_BITS_7, and depending on application, 7B transaction could work.
So I'm not sure what it the right thing to do here.
If dedicated functions are required to support transaction different from 8B, then I guess the problem is that API has to be reviewed. Otherwise, I don't see why we should prevent 7B trasactions.
| if ((cfg->data_bits == UART_CFG_DATA_BITS_5) || | ||
| (cfg->data_bits == UART_CFG_DATA_BITS_6) || | ||
| (cfg->data_bits == UART_CFG_DATA_BITS_7) || | ||
| (cfg->data_bits == UART_CFG_DATA_BITS_9)) { |
There was a problem hiding this comment.
To be clear I think we should limit the change to the following for now
if ((UART_CFG_DATA_BITS_5 == cfg->data_bits) ||
(UART_CFG_DATA_BITS_6 == cfg->data_bits)
#ifndef LL_USART_DATAWIDTH_7B
|| (UART_CFG_DATA_BITS_7 == cfg->data_bits)
#endif /* LL_USART_DATAWIDTH_7B */
|| (UART_CFG_DATA_BITS_9 == cfg->data_bits)
) {
|
Fixed by #32204 |
Signed-off-by: Zisis Adamos [email protected]