Open mrbell321 opened 6 years ago
Very good idea indeed (sorry for the long delay).
However it would have been easier to split the various improvements into several PRs.
I'm not sure about the whitespaces around +
, -
, /
, *
(see this discussion for example).
Ok, I've cherry-picked most of your changes and adapted your PR here: https://github.com/eblot/pyspiflash/compare/typehints
I cannot yet deliver it to the master branch, as I do not have enough HW to test it for now, but I hopefully will in a couple of weeks. Stay tuned :-)
Indeed, it was not my intention to put them all on one PR, but it was kind of symbiotic between adding AT25SF041 functional level that I needed and adding typehints/PEP8.
Interesting about the whitespaces, though, I was unaware of the differences in PEP8 over time. I was just using what Pycharm told me was PEP8. If I get ambitious today, I'll add a PR for pylint to setup.py. ...if I get ambitious...
https://github.com/eblot/pyspiflash/compare/typehints looks good to me. I'll try to test it out on my HW. I'll close this PR if it works.
On another topic, it is possible to use bit-banging to communicate SPI on FTDI chips that don't have the MPSSE engine. Do you think that's a worthwhile effort to add? I have some C++ code that does this(of course based on the libftdi dll which isn't the same as pyftdi...) and I've been considering porting it over. Depending on your point of view, it might clutter up the codebase and it'll be a fairly big effort so I want to make sure you'll consider it before I put the effort in.
That would be really nice if you can test (and fix any stupid mistake I made), yes.
About bit banging: I think it would be deadly slow, as it would likely mean to exchange several USB frames to read or write a single SPI bit. I never try this, but I think you will not be able to achieve a "usable" speed, except for maybe erasing the whole chip or reading a couple of bytes. However if you have a working C(++) implementation, would you already have some figures to publish?
The FIFO mode could help (not sure how many FTDIs support this), but unfortunately I have not yet implemented this mode in PyFtdi.
About cluttering the codebase: it could be mostly implemented as an SPI-over-GPIO solution, i.e. a wrapper file - that could even belong to PyFtdi BTW, providing a SpiPort duck typing API. That would not change the PySpiFlash I would believe.
Actually, the bit-bang mode(in C) wasn't appreciably slower than MPSSE for the 2 chips that I use. I'll have to dig through the C++ code to remember exactly, but I think the translation overhead is something on the order of 8 USB bytes per SPI bit(one byte per pin plus a couple extra, maybe?). I think you can load about 4k into the FTDI buffer and let it run at SPI signalling rate. That gives you time to batch up the next 4k. So the limitation for the SPI flash chips I have is the SPI baud rate more than the USB frame rate.
It is likely not the bit-bang mode I had in mind, for which there is no buffering available. If you have the C++ extract (which libftdi API you call and most importantly how you configure the FTDI w/o MPSSE), it would be great to see it.
SST25 are by far the worst flash devices of the 25-series to program, because they do not have internal RAM buffer to receive data to write. As you have to poll the device for its busy state while it is programming, buffering do not really help anyway. It helps a lot to read out data, but is mostly useless to write them.
This is why https://github.com/eblot/pyspiflash#supported-spi-flash-devices write bandwidth for SST25 look so bad. Write w/o polling could be possible with adding TBP delay (20 µs) between each data byte to program, but I'm not sure how to achieve this over USB+FTDI.
Anyway, if you have an example of bit-banging + FTDI buffering w/o MPSSE, I'm interested in.
Since there's no documentation on how to use this, I went through and added type hints. This will break python 2.x compatibility, but, for me, that's fine.
Additionally, I improved the PEP8 format compliance slightly.
Finally, I think there may be some potential bugs lurking in
_SpiFlashDevice.erase
that I haven't fully thought through yet. I'll dig in later, but someone who knows more may want to review the different *sector_size variables and their assignment permutations.