bitfixer / bf-romulator

Romulator - RAM/ROM replacement and debug for 6502 CPU systems
144 stars 21 forks source link

Parity Error Check Bug - When D1-mini sends 0x22 for a reread after a parity error the FPGA does not decrement the address by 8. #19

Open dennowiggle opened 1 year ago

dennowiggle commented 1 year ago

I tested using the incrementing byte pattern that vram is initialized too by file vram_test.txt, and then loading the canvas.html page on the standalone D1-mini programmer with the relevant "verbose = true" in function romulatorReadVramBlock() in module libRomulatorDebug.cpp to print out the returned data values.

To produce a parity error to test the FPGA I forced a send of 0x22 to the FPGA when a specific vram address was accessed and looked at the data returned by the FPGA.

The error in the FPGA verilog code is in the VERIFY_PARITY_BYTE section of the state machine in "diagnostics.v". Change line 219 from "if (rx_dv <= 1'b1)" to "if (rx_dv == 1'b1)"

VERIFY_PARITY_BYTE:
begin
  tx_dv <= 0;
  // if (rx_dv <= 1'b1)
  if (rx_dv == 1'b1)
  begin
    if (rx_byte == PARITY_ERROR)
    begin
      vram_address <= vram_address - 8;
    end
    state <= NEXT_VRAM_BYTE;
  end
end

Now that "if (rx_dv == 1'b1)" is functioning properly, the D1-mini code needs to be modified in file "libRomulatorDebug.cpp".

Between lines 356 and 357 add // Advance FPGA so it can prepare the next vram byte for sending over SPI. xfer(0x0);

A similar change should also be made on the Raspberry Pi code for that version of debugger.

dennowiggle commented 1 year ago

There are a few other typo's and I've noted them below.

In file "diagnostics.v" there is also an error on line 165 where an "=" is used instead of "<=" change line 165, from parity_byte[parity_counter] = vram_data[0] + vram_data[1] + vram_data[2] + vram_data[3] + vram_data[4] + vram_data[5] + vram_data[6] + vram_data[7]; to : parity_byte[parity_counter] <= vram_data[0] + vram_data[1] + vram_data[2] + vram_data[3] + vram_data[4] + vram_data[5] + vram_data[6] + vram_data[7];

Some of the other verilog files have similar typo's.

"enable_logic.v" Extra comma on line 337 can be removed .q(vram_output),

"spi_flash_reader.v" Line 180 change state = TX_IDLE; to state <= TX_IDLE;

line 194 from xfer_state = XFER_IDLE; to xfer_state <= XFER_IDLE;

line 218 from xfer_state = XFER_FLASHREAD_BLOCK; to xfer_state <= XFER_FLASHREAD_BLOCK;

line 223 from xfer_flash_blocks_to_read = xfer_flash_blocks_to_read - 1; to xfer_flash_blocks_to_read <= xfer_flash_blocks_to_read - 1;

line 235 from xfer_flash_blocks_to_read = 2; to
xfer_flash_blocks_to_read <= 2;

line 348 from ram_address = ram_address + 1; to ram_address <= ram_address + 1;

"simple_ram_dual_clock.v" - line 14 is missing a semi-colon at the end reg [DATA_WIDTH-1:0] ram [2ADDR_WIDTH-1:0] // is exponentiation

bitfixer commented 1 year ago

Thanks very much for the fixes! I am away from my computer for a few days but will take a closer look as soon as I'm back.

On Fri, Dec 9, 2022 at 2:34 PM dennowiggle @.***> wrote:

There are a few other typo's and I've noted them below.

In file "diagnostics.v" there is also an error on line 165 where an "=" is used instead of "<="

change line 165, from

parity_byte[parity_counter] = vram_data[0] + vram_data[1] + vram_data[2] + vram_data[3] + vram_data[4] + vram_data[5] + vram_data[6] + vram_data[7];

to :

parity_byte[parity_counter] <= vram_data[0] + vram_data[1] + vram_data[2]

  • vram_data[3] + vram_data[4] + vram_data[5] + vram_data[6] + vram_data[7];

Some of the other verilog files have similar typo's.

"enable_logic.v"

  • Extra comma on line 337 can be removed

    .q(vram_output),

"spi_flash_reader.v"

Line 180 change

state = TX_IDLE;

to

state <= TX_IDLE;

line 194 from

xfer_state = XFER_IDLE;

to

xfer_state <= XFER_IDLE;

line 218 from

xfer_state = XFER_FLASHREAD_BLOCK;

to

xfer_state <= XFER_FLASHREAD_BLOCK;

line 223 from

xfer_flash_blocks_to_read = xfer_flash_blocks_to_read - 1;

to

xfer_flash_blocks_to_read <= xfer_flash_blocks_to_read - 1;

line 235 from

xfer_flash_blocks_to_read = 2;

to

xfer_flash_blocks_to_read <= 2;

line 348 from

ram_address = ram_address + 1;

to

ram_address <= ram_address + 1;

"simple_ram_dual_clock.v" - line 14 is missing a semi-colon at the end

reg [DATA_WIDTH-1:0] ram [2ADDR_WIDTH-1:0] // is exponentiation

— Reply to this email directly, view it on GitHub https://github.com/bitfixer/bf-romulator/issues/19#issuecomment-1344852337, or unsubscribe https://github.com/notifications/unsubscribe-auth/AANYKOP2SVXBTXG7TKCINK3WMOXWVANCNFSM6AAAAAASZ2BZB4 . You are receiving this because you are subscribed to this thread.Message ID: @.***>

dennowiggle commented 1 year ago

Correction: File "spi_flash_reader.v"

Delete line 223 xfer_flash_blocks_to_read = xfer_flash_blocks_to_read - 1;

Add line between line 290 and 291 xfer_flash_blocks_to_read <= xfer_flash_blocks_to_read - 1;

New line should look like this XFER_READ_BYTES_DONE: begin spi_cs_reg <= 1; xfer_flash_blocks_to_read <= xfer_flash_blocks_to_read - 1; xfer_state <= XFER_FLASHREAD_NEXT; end

Changes have been tested to work with a 24MHz clock on an Apple IIe.