Skip to content

Some Issues Need to be Fixed #1

@kazutoiris

Description

@kazutoiris

Issues to be Fixed

  1. There is a spelling error in the comment.

    https://github.com/oscc-ip/uart/blob/0e33d4465fc19e624f9e5c418b4d0cd0b193cc45/rtl/uart_define.sv#L44-L45

  2. parity_err is always cleared.

    https://github.com/oscc-ip/uart/blob/0e33d4465fc19e624f9e5c418b4d0cd0b193cc45/rtl/apb4_uart.sv#L200-L201

  3. Besides clearing the Parity Error bit, other bits are also cleared (which should not be cleared).

    https://github.com/oscc-ip/uart/blob/0e33d4465fc19e624f9e5c418b4d0cd0b193cc45/rtl/apb4_uart.sv#L133-L136

    https://github.com/oscc-ip/uart/blob/0e33d4465fc19e624f9e5c418b4d0cd0b193cc45/rtl/uart_irq.sv#L58-L59

Recommendations for Improvement

  1. LCR is used to refer to Line Control Register, which may cause confusion, suggest extracting PEIE, TXIE and RXIE to Interrupt Enable Register (IER) or renaming the register's name.

    https://github.com/oscc-ip/uart/blob/0e33d4465fc19e624f9e5c418b4d0cd0b193cc45/rtl/uart_define.sv#L15-L18

  2. Suggest renaming s_rx_push_ready to s_rx_fifo_full to avoid confusion.

    https://github.com/oscc-ip/uart/blob/0e33d4465fc19e624f9e5c418b4d0cd0b193cc45/rtl/apb4_uart.sv#L183

  3. pready is always true, suggest connecting !tx_fifo_full to pready for FIFO status detection.

    https://github.com/oscc-ip/uart/blob/0e33d4465fc19e624f9e5c418b4d0cd0b193cc45/rtl/apb4_uart.sv#L57

    https://github.com/oscc-ip/uart/blob/0e33d4465fc19e624f9e5c418b4d0cd0b193cc45/rtl/apb4_uart.sv#L151

BTW, I suggest continuing to use the SHL-0.51 license, which is widely used by projects such as cva6 and has detailed licensing terms. The MulanPSL v2 license is still not mature enough, with vague terms and some unanswered issues (osslab-pku/OpenSourceLicense-FQA#4), so raising concerns about the prospect of maintenance being discontinued.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions