TLDR: _XTAL_FREQ must be derived from PLL clock, not XTAL clock
I got some time to investigate why AT89C51 erase isn't reliable. Some early testing showed we aren't generating a correct 10 ms erase cycle, but instead get around 3.33 ms. This explains why erases aren't always valid.
I discovered we have "_XTAL_FREQ 16000000", which matches the crystal on board. However, main.c starts with enabling the PLL ("OSCTUNEbits.PLLEN = 1;"), which means _XTAL_FREQ used by delay() family functions must be based on that instead (see https://www.microchip.com/forums/m1060232.aspx). It seems like we actually need "_XTAL_FREQ 48000000" which implies CPDIV is dividing by 2 or something like that. I'm fairly certain we just need to make the _XTAL_FREQ change and move on, although we should be more aware of it for the future.
TLDR: _XTAL_FREQ must be derived from PLL clock, not XTAL clock
I got some time to investigate why AT89C51 erase isn't reliable. Some early testing showed we aren't generating a correct 10 ms erase cycle, but instead get around 3.33 ms. This explains why erases aren't always valid.
I discovered we have "_XTAL_FREQ 16000000", which matches the crystal on board. However, main.c starts with enabling the PLL ("OSCTUNEbits.PLLEN = 1;"), which means _XTAL_FREQ used by delay() family functions must be based on that instead (see https://www.microchip.com/forums/m1060232.aspx). It seems like we actually need "_XTAL_FREQ 48000000" which implies CPDIV is dividing by 2 or something like that. I'm fairly certain we just need to make the _XTAL_FREQ change and move on, although we should be more aware of it for the future.