From dbe9ea86ecba84d073e47e8029c06af642cf17c7 Mon Sep 17 00:00:00 2001 From: Ewen McNeill Date: Sat, 8 Jun 2019 12:43:05 +1200 Subject: [PATCH 1/4] tinyprog: Pad to full minor sector write size Either the TinyProg-BootLoader state machines, or the SPI flash chip used on the TinyFPGA BX, seems unable to *properly* program a partial minor sector (ie, writing less than 256 octets), leading to a failed write/verify cycle. This seems to happen at the end of any programming that is not an exact multiple of 256 octets. Since the whole 256 octets is already erased in program_sectors(), we pad out the short write to a full 256 octets (with 0xff, which is the value read for "erased", to reduce wear on on the flash cells). For more detail on diagnosing this, see: https://github.com/timvideos/litex-buildenv/issues/137 --- programmer/tinyprog/__init__.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/programmer/tinyprog/__init__.py b/programmer/tinyprog/__init__.py index ed585a9..a6b4260 100644 --- a/programmer/tinyprog/__init__.py +++ b/programmer/tinyprog/__init__.py @@ -499,6 +499,22 @@ def program_sectors(self, addr, data): for minor_offset in range(0, 4 * 1024, minor_sector_size): minor_write_data = current_write_data[ minor_offset:minor_offset + minor_sector_size] + + # The TinyFPGA firmware and/or flash chip does not handle + # partial minor sector writes properly, so pad out short + # writes to a whole minor sector. (This is safe because + # we explicitly erased the whole sector above, so the + # following bytes must be freshly erased. Usually it will + # only happen on the final minor sector to be written.) + # + if (len(minor_write_data) < minor_sector_size): + pad_len = minor_sector_size - len(minor_write_data) + padding = b'\xff' * pad_len + + minor_write_data = bytearray(minor_write_data) + minor_write_data.extend(padding) + assert(len(minor_write_data) == minor_sector_size) + self.write( current_addr + minor_offset, minor_write_data, From c15dd007ac6b043fc543a565da31a779cd6f4531 Mon Sep 17 00:00:00 2001 From: Ewen McNeill Date: Sat, 8 Jun 2019 12:52:37 +1200 Subject: [PATCH 2/4] tinyprog: Ignore pip-wheel-metadata "pip-wheel-metadata" is created alongside setup.py, by "pip install ." (used for development). See https://github.com/pypa/pip/issues/6213 for discussion of this clutter (issue currently open; might be moved to another location, eg build/pip-wheel-metadata or .pip-wheel-metadata, in a later version). --- programmer/.gitignore | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/programmer/.gitignore b/programmer/.gitignore index 25fe328..197949b 100644 --- a/programmer/.gitignore +++ b/programmer/.gitignore @@ -9,6 +9,10 @@ __pycache__/ *~ tinyprog/full_version.py +# due to pip install . (for development) +# https://github.com/pypa/pip/issues/6213 +pip-wheel-metadata + # due to using tox and pytest .tox .coverage From 959ab559f6f4123213230c4ee8c6550f7ab3b300 Mon Sep 17 00:00:00 2001 From: Ewen McNeill Date: Sat, 8 Jun 2019 13:12:47 +1200 Subject: [PATCH 3/4] tinyprog: (read)timeout=2.0 writeTimeout=5.0 Double the read timeout to reduce risk of short reads causing verify errors. Substantially increase the write timeout to try to reduce the risk of an incomplete write, or a write being abandoned when it was nearly finished due to a minor SPI flash delay. --- programmer/tinyprog/__init__.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/programmer/tinyprog/__init__.py b/programmer/tinyprog/__init__.py index a6b4260..b31e787 100644 --- a/programmer/tinyprog/__init__.py +++ b/programmer/tinyprog/__init__.py @@ -113,9 +113,20 @@ def __str__(self): return self.port_name def __enter__(self): + # Timeouts: + # - Read: 2.0 seconds (timeout) + # - Write: 5.0 seconds (writeTimeout) + # + # Rationale: hitting the writeTimeout is fatal, so it pays to be + # patient in case there is a brief delay; readTimeout is less + # fatal, but can result in short reads if it is hit, so we want + # a timeout high enough that is never hit normally. In practice + # 1.0 seconds is *usually* enough, so the chosen values are double + # and five times the "usually enough" values. + # try: self.ser = serial.Serial( - self.port_name, timeout=1.0, writeTimeout=1.0).__enter__() + self.port_name, timeout=2.0, writeTimeout=5.0).__enter__() except serial.SerialException as e: raise PortError("Failed to open serial port:\n%s" % str(e)) From 0b573f367b9d0c5cbde2b58437a24111f7f8a038 Mon Sep 17 00:00:00 2001 From: Ewen McNeill Date: Sat, 8 Jun 2019 15:45:27 +1200 Subject: [PATCH 4/4] tinyprog: limit trailing padding to aligned writes To avoid wrap around when writing beyond the 256 octet sector, if the write starts part way into the 256 octet sector, only do the padding to a full 256 octet sector if we are writing from the begining of the sector (and thus writing a full 256 octets will not wrap around). We also write 0xff for safety, as in most SPI flash that is the erased value, and effectively one can only write 0 bits, so writing twice to a cell (without erasing) writes (existing value AND new value), which for a new value of 0xff should be the existing value. --- programmer/tinyprog/__init__.py | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/programmer/tinyprog/__init__.py b/programmer/tinyprog/__init__.py index b31e787..3d7a4c9 100644 --- a/programmer/tinyprog/__init__.py +++ b/programmer/tinyprog/__init__.py @@ -512,13 +512,20 @@ def program_sectors(self, addr, data): minor_offset:minor_offset + minor_sector_size] # The TinyFPGA firmware and/or flash chip does not handle - # partial minor sector writes properly, so pad out short - # writes to a whole minor sector. (This is safe because - # we explicitly erased the whole sector above, so the - # following bytes must be freshly erased. Usually it will - # only happen on the final minor sector to be written.) + # partial minor sector writes properly, so pad out a final + # write of a partial to a whole minor sector, if we are + # writing aligned to the SPI flash internal minor sectors. # - if (len(minor_write_data) < minor_sector_size): + # Due to the way SPI flash works, writing 0xff *without + # erasing* should be a no-opt, because 0xff is what you + # get after erasing, and you can only write 0 bits. + # + current_minor_addr = current_addr + minor_offset + + if (((current_minor_addr % minor_sector_size) == 0) and + (len(minor_write_data) < minor_sector_size)): + assert((current_minor_addr % minor_sector_size) == 0) + pad_len = minor_sector_size - len(minor_write_data) padding = b'\xff' * pad_len