jayacotton / inettools-z80

Inet tools for z80-rc2014 tcp/ip stack code.
MIT License
16 stars 2 forks source link

Discussion on getting SPI_BURST_READ to work #2

Open durgadas311 opened 4 years ago

durgadas311 commented 4 years ago

Hi Jay, I was looking over this code, and realized that my confusion about how WIZCHIP_READ_BUF() worked was due to looking at the code under "#ifdef SPI_BURST_READ" but not realizing that that option is not enabled. Based on my experience with a similar SPI interface, this code will probably work if you add a "dummy" input on the data/shift port, like this:

diff --git a/w5500.c b/w5500.c
index 1a61ee8..85866bf 100644
--- a/w5500.c
+++ b/w5500.c
@@ -201,6 +201,10 @@ WIZCHIP_READ_BUF (uint32_t AddrSel, unsigned char *pBuf, uint16_t len)
   SpiSendData ((AddrSel & 0x000000FF) >> 0);
 #ifdef SPI_BURST_READ
   printf ("burst_read %d\n", len);
+  /* trigger shift of the first byte */
+#asm
+       in      _shift_rdtr
+#endasm
   p = pBuf;
   rs = spi_burst_read (p, len);
   p += rs;

That is essentially what I do on my code for the Heathkit computer adapter. This means that the hardware does an additional read past the end of the burst region, but that does not cause any problem on the W5500, and from what I hear other devices as well. As long as the device does not have any hidden side-effects for reads, it should be fine. For regions like the W5500 FIFO memory, that is certainly fine.

jayacotton commented 4 years ago

Cool, I'll have a look at that soon.

jayacotton commented 4 years ago

I had a try at this, it did not work. Got stuck in a loop trying to read 308 more bytes.
There must be more to this than meets the eye.

I went over to your repo but did not find an example of the code running with burst mode. Can you provide guidance ?

durgadas311 commented 4 years ago

I was just reviewing your spi_burst_read() routine, and notice that it has a maximum transfer size of 256 bytes. This is due to the INIR instruction being used. Not sure what your test was doing, when you say "trying to read 308 more bytes" do you mean that it was stuck while there were 308 more bytes to read? Or that the total size of the attempted transfer was 308 bytes?

My W5500 SNIOS for CP/NET is here: https://github.com/durgadas311/MmsCpm3/blob/master/net/src/snioswiz.asm. Note, that is written in Intel mnemonics (with Z80.LIB), so it may not be easy for you to read if you're into Zilog mnemonics.

Probably start at the "cpyin" routine, which reads a chunk of data from the W5500 FIFO in burst mode. The "cpsetup" routine issues the read command to the W5500, and then on line 295 you see a "dummy" input on the (equivalent of the) _shift_rdtr port. Then, the code gets a little twisted as I actually compensate for the 256-byte limit of INIR by splitting the transfer into multiple INIRs as needed. Actually, since it is for CP/NET (who's maximum packet size is 261), it does not handle an arbitrary length but will do a maximum of two INIRs.

Let me know if you have more questions on my code. Also, let me know more details of the test you were attempting. If you want some help adapting spi_burst_read() to handle more than 256 bytes, let me know. I can sketch out some possible changes.

durgadas311 commented 4 years ago

Oh, just noticed one thing in WIZCHIP_READ_BUF(). The loop that calls spi_burst_read() for additional remnants of the data (for transfers over 256 bytes) does not advance the buffer pointer 'p'. If your transfer never needs more than two calls to spi_burst_read(), then it would probably work. But if you do support more than 512 bytes then you'll need to fix that.

Also, I think there is a problem with setting the buffer address (in HL) in spi_burst_read(). I think that the "ld hl,_l_buffer" needs to be "ld hl,(_l_buffer)". The former is more like "HL = &l_buffer" and you need it to be "HL = l_buffer" since 'l_buffer' contains the address of the buffer, it is not the actual buffer.

jayacotton commented 4 years ago

o.k. I'll check that.

jayacotton commented 3 years ago

Well here it is almost a year latter, still have not gotten to this.

durgadas311 commented 3 years ago

I'm not sure how urgent this is, and also not certain how noticeable the increase in speed would be. It's just something that could be done. They are bugs lingering if anyone ever turns on burst mode.

jayacotton commented 1 year ago

Burst read is working now for rc2014. However the s100 version is not working. I suspect a hardware bug, but it could also be a bus decode logic error on the 22v10 chip.

On second thought. The 22v10 should not have anything to do with the problem, and the address decode is specific the the lower 8 address bits.

The burst read works using the INIR instruction. It will put the address out on the c register and the byte count out on the b register. Well, in fact it puts the i/o address out from the h/l register and then swaps it back. Kind of a busy instruction.

Need to study the manual on z80 actions and compare the instruction cycles.