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

Update regs dictionary; Remove LIB. #24

Merged
merged 6 commits into from
Oct 27, 2023

Conversation

arturum1
Copy link
Contributor

@arturum1 arturum1 commented Sep 1, 2023

- Update uart16550_init function to set the correct initial value for the Line Control Register. The fixed initial value is 0x03. This value should be the same as the reset value from hardware (according to the UART16550 Core Technical Manual). However, if we don't initialize it in software, it seems that the hardware reset value is sometimes not correct. Without this software fix, the uart16550 core would sometimes transmit with 5 bits per character, instead of the correct 8 bits per character. This behaviour was probably due to the wrong init value in the LCR register.

Update:

  • Update regs dictionary;
  • Remove LIB.

Used for IObundle/iob-soc-opencryptolinux#53

- Update `uart16550_init` function to set the correct initial value for the Line Control Register.
  The fixed initial value is 0x03. This value should be the same as the
  reset value from hardware (according to the UART16550 Core Technical
  Manual). However, if we don't initialize it in software, it seems that
  the hardware reset value is sometimes not correct.
  Without this software fix, the uart16550 core would sometimes transmit with 5 bits
  per character, instead of the correct 8 bits per character. This behaviour was probably
  due to the wrong init value in the LCR register.
@PedroAntunes178
Copy link
Contributor

Could be accepted. I do not know if it is the correct approach, tho.
In:

if (wb_rst_i) lcr <= #1 8'b00000011; // 8n1 setting
it seems as if the register is being correctly reset.
What could be occurring is some problem when the register is read.
Maybe a problem with the iob2wishbone interface in the read. Should be interesting to see if the register is ready to be read immediately when "ack" is signaled.

@PedroAntunes178
Copy link
Contributor

@arturum1 I changed the way rdata is passed on the iob2wishbone wrapper. Could you update the UART16550 and test if the error talked in this commit is persistent?

@PedroAntunes178
Copy link
Contributor

PedroAntunes178 commented Oct 25, 2023

@arturum1 @jjts
Before merging this PR please revert the changes made to software/src/iob-uart16550.c.
Then verify if the issue still presists. I have made an alteration to the wishbone converter that might solve the issue.
Then give feedback.

@jjts
Copy link
Contributor

jjts commented Oct 25, 2023

what changes what issue? I will not revert changes . Can it be accepted and then you change it @PedroAntunes178 ?

@PedroAntunes178
Copy link
Contributor

what changes what issue? I will not revert changes . Can it be accepted and then you change it @PedroAntunes178 ?

This PR was initially formed to solve an issue with the LCR register. I did not have that problem and therefore can not replicate it. I had a similar problem when updating the CPU on OpenCryptoLinux and made a change to the wishbone wrapper that fixed it. That change is already upstream.

In this PR it was presented a software solution to the issue. But it has not been tested with the hardware fix I made. I do not agree with the software fix because it does not read the value with which the register is inicialized.

I can accept the PR and then change the software. However, that would be simillar to reverting the change like I asked @arturum1. And I have no way of knowing if the issue presists for his case. If he made those alteration he could test and check if my fix also works for him.

@arturum1 arturum1 marked this pull request as draft October 26, 2023 10:22
@arturum1 arturum1 changed the title fix(driver): Fix LCR register init value. Update regs dictionary; Remove LIB. Oct 26, 2023
@arturum1 arturum1 marked this pull request as ready for review October 27, 2023 14:17
@PedroAntunes178 PedroAntunes178 merged commit 091455b into IObundle:master Oct 27, 2023
1 check passed
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.

3 participants