search

stnolting / neorv32

:desktop_computer: A small, customizable and extensible MCU-class 32-bit RISC-V soft-core CPU and microcontroller-like SoC written in platform-independent VHDL.
https://neorv32.org
BSD 3-Clause "New" or "Revised" License
1.54k stars 212 forks source link

[bug] uart issue after #883 upd #890

Closed Polaris-III closed 4 months ago

Polaris-III commented 4 months ago

Describe the bug After #883 i discover that neorv32 determines any printed character as "?". Looks like #883 uart_rx upd broke something

To Reproduce Check "sreg" bus of "rx_engine_t" record in "neorv32_uart.vhd" file. before #883 - sreg[9:0] after #883 - sreg[8:0] <- where this issue happened

Expected behavior Correct character determination

Screenshots I push "e" button to abort autoboot sequence Screen 1 - incorrect symbol. Screenshot from 2024-05-02 15-16-15

Screen 2 - correct after bus fix Screenshot from 2024-05-02 15-22-33

Environment:

Hardware:

Additional context I changed sreg back to "9 downto 0" and reimplement design - works fine

Unike267 commented 4 months ago

Hi everyone!

I've reproduced this bug in Arty A7 35t.

As shown in the following result via CuteCom:

issue_890

I hope it will be solved soon,

Cheers!

Edit:

Maybe the bug comes from here:

https://github.com/stnolting/neorv32/blob/e2fa5c7579444b1142de635ae0cf2fd82dd30ab5/sw/bootloader/bootloader.c#L395

Since it expects to get a char variable (8 bits) and gets other value. :thinking: That is, the problem is probably coming from the receiver.

Unike267 commented 4 months ago

Hi,

I've fixed the issue in local changing this line:

https://github.com/stnolting/neorv32/blob/e2fa5c7579444b1142de635ae0cf2fd82dd30ab5/rtl/core/neorv32_uart.vhd#L289

From rx_engine.sreg(8 downto 1); to rx_engine.sreg(7 downto 0);

Cheers! 😃

Edit:

@Polaris-III Could you test my change?

Polaris-III commented 4 months ago

Hi @Unike267, I'll check your solution tomorrow, thank you

stnolting commented 4 months ago

Seems like I have messed up the UART receiver 🙈

@Polaris-III thanks for finding this bug!

@Unike267 your fix looks great! Would you open a PR? :wink:

Unike267 commented 4 months ago

Done! 😃

When you merge it be careful with my commit description I should have written "on line 289" or "in line 289" instead of "in 289 line" sorry for that. 😅