Closed olkal closed 5 months ago
Great addition! Will look into it later this week
This addition was on my todo list, so I'm happy you implemented and tested it.
The POR (power on reset) value of the SEQOP bit is zero ==> sequential read/write mode.
However as we will not be sure if the value is set to zero, I propose to clear that bit explicitly
Add in the begin() function. line 73++ something like
uint8_t reg = readReg(MCP23017_IOCR);
if (reg & MCP23017_IOCR_SEQOP) // check if already zero
{
reg &= ~MCP23017_IOCR_SEQOP; // clear SEQOP bit for sequential read/write
if (! writeReg(MCP23017_IOCR, reg)) return false;
}
Yes, I agree. Should I also remove line 73 then? // if (! writeReg(MCP23017_IOCR, MCP23017_IOCR_SEQOP)) return false;
I don't see any any reason to keep it.
Please remove line 73 as it has no function
LGTM The 16 bit calls now only transfer 4 bytes instead of 6 bytes which can be seen in your performance measurements. Will squash and merge it now and bump the version number tomorrow.
@olkal Did performance test on AVR UNO with the optimized 16 bit and the relative gain is again ~30%. Especially for SW SPI the absolute improvement is substantial.
function | 0.5.2 | 0.5.3 | notes |
---|---|---|---|
SW SPI write16 | 450 | 300 | |
SW SPI read16 | 446 | 298 | |
HW SPI write16 | 42 | 26 | 1 MHz |
HW SPI read16 | 42 | 28 | 1 MHz |
HW SPI write16 | 28 | 18 | 2 MHz |
HW SPI read16 | 30 | 18 | 2 MHz |
HW SPI write16 | 22 | 16 | 4 MHz |
HW SPI read16 | 24 | 16 | 4 MHz |
HW SPI write16 | 20 | 14 | 8 MHz |
HW SPI read16 | 20 | 14 | 8 MHz |
Hi, this PR is implementing sequential read/write for 16-bit operations as described in the data sheet 3.2.1 page 13. See test data below from running the MCP23S17_performance.ino sketch on an ESP32-S3@240mhz.