-
Notifications
You must be signed in to change notification settings - Fork 8.2k
Make BFLB not use the SDK code #91977
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
Conversation
6e0c76d to
f2a098f
Compare
175cf3f to
324da9a
Compare
| } | ||
| /* clear interrupts that require ack*/ | ||
| tmp = sys_read32(cfg->base_reg + UART_INT_CLEAR_OFFSET); | ||
| tmp = tmp | UART_CR_URX_RTO_CLR; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there's more that needs clearing, e.g. you've enabled PCE so UART_CR_URX_PCE_CLR ; probably UART_CR_URX_END_CLR and UART_CR_UTX_END_CLR as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay no, deleted message was right but i looked at the wrong place UART_CR_URX_END_CLR and UART_CR_UTX_END_CLR 's interrupts should be masked and UART_CR_URX_PCE_CLR is handled in uart_bflb_err_check
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(masked = interrupt not thrown to CPU, but i think it was required for working?)
| void riscv_clic_irq_priority_set(unsigned int irq, unsigned int prio, uint32_t flags) | ||
| { | ||
| ARG_UNUSED(irq); | ||
| ARG_UNUSED(prio); | ||
| ARG_UNUSED(flags); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would be good to document why it's not being implemented? Or... should it be implemented?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should probably yes, will look into it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shell sample isnt working on this PR while it should anyway so i have some checks to do...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It works with debugger.... uuuurgh
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixing this has had the knock-off effect of somehow fixing the shell sample without the little wait I just added... Sometimes I really dont understand those chips...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
drivers/serial/uart_bflb.c
Outdated
| tmp = sys_read32(cfg->base_reg + UART_STATUS_OFFSET); | ||
| rc = tmp & UART_STS_UTX_BUS_BUSY; | ||
| tmp = sys_read32(cfg->base_reg + UART_FIFO_CONFIG_1_OFFSET); | ||
| rc += tmp & UART_TX_FIFO_CNT_MASK; | ||
| return (rc > 0 ? 1 : 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think you have the logic backwards?
irq_tx_complete returns 1 If nothing remains to be transmitted ; 0 If transmission is not completed.
tx is probably not complete if UART_STS_UTX_BUS_BUSY is set or if FIFO still contains data?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds like it yes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
Note tx fifo count is inverted, it counts free bytes.
| int uart_bflb_irq_tx_ready(const struct device *dev) | ||
| { | ||
| const struct bflb_config *const cfg = dev->config; | ||
| uint32_t tmp = 0; | ||
|
|
||
| tmp = sys_read32(cfg->base_reg + UART_FIFO_CONFIG_1_OFFSET); | ||
| return (tmp & UART_TX_FIFO_CNT_MASK) > 0; | ||
| } | ||
|
|
||
| int uart_bflb_irq_rx_ready(const struct device *dev) | ||
| { | ||
| const struct bflb_config *const cfg = dev->config; | ||
| uint32_t tmp = 0; | ||
|
|
||
| tmp = sys_read32(cfg->base_reg + UART_FIFO_CONFIG_1_OFFSET); | ||
| return (tmp & UART_RX_FIFO_CNT_MASK) > 0; | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you've setup thresholds so raw count shouldn't be used? I think this should instead check if UART_URX_FIFO_INT / UART_UTX_FIFO_INT are set/pending.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These interrupts are masked, functionally it's the same (And there might have been a reason, interrupts gave me, and continue to give me, a lot of trouble on this platform, I wouldnt remember but it's possible there was issues doing it with the interrupt).
| reg mstatus 0x7800 | ||
| reg mie 0x0 | ||
| # reg pc 0x23000000 | ||
| reg mstatus 0x80000000 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bit 31 of mstatus is read-only?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure where that's from, I see the same, maybe 0x0 is more appropriate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
418a18c to
382a12e
Compare
|
Rebased on main. |
83cb97c to
dd434c7
Compare
|
Squashed commits that couldn't build by themselves (so almost all of them...) |
nandojve
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see more points for improvement but since this is a big change already I think it is fine to move this way. Then we improve gradually to avoid stuck the PR.
This reverts 514258a which causes issues here Signed-off-by: Camille BAUD <[email protected]>
|
rebased on main |
Reorganize and update soc folder files for SDK-independance Reorganize and update hal_bouffalolab files for SDK-independance Reorganize and update soc dts files for SDK-independance Update serial and pinctrl driver files for SDK-independance Update ai_wb2_12f, bl604e_iot_dvk, and dt_bl10_dvk to new bl60x support and fixup openocd config of ai_wb2_12f Signed-off-by: Camille BAUD <[email protected]>
mostly makes things a little bit safer Signed-off-by: Camille BAUD <[email protected]>
|
|
ping @kartben |
|
regarding soc: bflb: re-add ATOMIC_OPERATIONS_C are you sure, that the riscv cpu has the atomic extention? |
Yes, it is supposed to work, but doesn't, so it is a problem for later. |



What's in the title.
Large update that cannot be cut without breaking functionality.
Some drivers are getting entirely revamped, such as uart and pinctrl, do not consider those as updates of the older drivers, but rather new drivers (which is true to some large extent).