Echo icon indicating copy to clipboard operation
Echo copied to clipboard

Fix echo_get_status() not returning ECHO_STAT_PCM, DIRBUSY or BUSY correctly

Open rhargreaves opened this issue 4 years ago • 2 comments

I believe there's a bug in echo_get_status() C API which is causing the function to never return ECHO_STAT_PCM, DIRBUSY or BUSY. The problem is in this bit of code:

   static const uint8_t and_flags[] = {
      0xFF,0xFF, 0xFF,0xFE,0xFF,0xFD, 0xFF,0xFF,0xFF
   };
   ...
   uint16_t status = 0;
   ...

   uint8_t command = z80_ram[0x1FFF];
   status &= and_flags[command];
   ...
   command = z80_ram[0x1FFB];
   status &= and_flags[command];
   ...

Status is a 16-bit integer, but the and_flags array returns a 8-bit mask. AND'ing these together truncates the value to 8-bit, stripping the upper byte's status.

Looking at the code, I believe the problem also occurs in the 68000 API, although I was not able to verify this as I do not use it.

rhargreaves avatar Feb 29 '20 23:02 rhargreaves

The asm routine is fine (byte-sized instructions will leave intact the higher bytes) and in fact your patch for that one is broken because it fails to account for the size difference (the value of d1 should be doubled before being used to look up into the table).

Also I need to know the licensing for that patch because I don't want to end up in a situation where I merge patches and then later everybody claims copyright on the driver and leads to a legal deadlock where nobody can decide what's OK to do with it later.

sikthehedgehog avatar Feb 29 '20 23:02 sikthehedgehog

I've removed the ASM patch. Oops! I'm happy for the rest of the patch to fall under your licence and for you to hold the copyright.

rhargreaves avatar Feb 29 '20 23:02 rhargreaves