Added active high polarity option for CS and ability to work with bi-…

[sgoadhouse]

…directional data for MicroWire support. These code changes were specifically done so that I could use a FTDI device to directly write and read a 93LC56 MicroWire EEPROM used by another FTDI device (yea, I bricked my FT4232 and used pyftdi to fix it). The MicroWire protocol is similar enough to SPI that spi mode can be used but MicroWire has an active high chip select (CS) instead of the active low CS of SPI. Also, in the case of fixing the EEPROM of a FT4232H-56Q Mini Module, I also needed bi-directional data support, which MicroWire can handle. These code changes were done so that they should not negatively impact code not using active high CS nor bi-directional data but these features can be extended to new users.

[eblot]

I may merge this improved implementation, but I think there are a couple of issues to address first:

1. __init__ argument order
   Compatibility with previous version should be preserved, the API should not introduce
   any compatibility break. I'm afraid the position of cs_bits argument may introduce an issue here, for clients that use unnamed arguments.

• cs_pol seems to be a boolean, so its default value would be True/False, not 1
• I do not get the exact meaning of cs_bits. You probably want to check cs_bits as well for validity so it does not cause any trouble if the value is invalid. It also should have a default value for client that do not want to deal with this new setting.
• 80-column wide rule is broken
• double space chars are required before # comment that follow a line with Python code
• self._bidir and False or duplex cannot work: it always evals to duplex.
• I'm not sure what happens with self._last_prolog_ctrl whenever two slave
  devices are accessed once after another (i.e. bidir interleaved with
  non-bidir). This introduces a stateful behaviour to the SPI engine...
  Maybe this should be handled @ SpiPort level?
• spi.rst should also be updated to reflect the improved API
• some kind of test(s) would be nice to check how the new code behaves - addressing another 93Cx6 EEPROM would be a useful example.

[sgoadhouse]

Very good points. I will take a look at it and improve things.

[sgoadhouse]

I added a test to tests/spi.py. I will address the other points and test against my new test.

[sgoadhouse]

• self._bidir and False or duplex was embarrassingly stupid; I fixed it and checked that it was correct
• I fixed all other issues except what to do with self._last_prolog_ctrl and updating spi.rst. I will take care of those next.
• Originally cs_bits was simply copied over from SpiController but I found that sending cs_count is a better API that is more understandable. cs_bits is recomputed in SpiPort, which duplicates code at the expense of a cleaner API. The reason SpiPort even needs this is because the CS bits are in the same byte as the other signals. If CS is Active High, you only need to make it high in __init__ in SpiPort while all of the other CS are low to keep them inactive. This is simple to do by shifting the single bit mask as was done. However, if CS is Active Low, you need to make the chosen CS low while ALL of the other CS are high, or inactive. Since the other signals are in the same byte, you cannot do a simple bit inversion of the mask. You must only invert the cs_bits. So now cs_count is sent to __init__ in SpiPort so only the CS bits get inverted.

[sgoadhouse]

spi.rst updated with MicroWire example using active high CS and bi-directional data

[sgoadhouse]

I am open to suggestions on how to handle self._last_prolog_ctrl. What I need is a way to change the data line from an output to an input. The only way I know how to do that is with the SET_BITS_LOW command which takes a value and a direction. This means that I need to know that last value sent in a SET_BITS_LOW command so I can send the same value but change the direction bits so that the DATA line is now an input. Perhaps I can remove from _exchange_full_duplex() where I save to self._last_prolog_ctrl and only save to self._last_prolog_ctrl in _exchange_half_duplex() when bidir is set to True? Do you think that will work?

[eblot]

I'm not sure how to understand how the microwire works, so it is hard to give a valid answer to this question, I first need to read some docs, I believe :-)

[eblot]

Just to be sure on how you have connected the EEPROM to the FTDI: you went for the same connection kludge as with I2C: MOSI & MISO are connected together on FTDI side, and connected to SISO on EEPROM side? You also need to disable MOSI as an output, so when you need to read from the EEPROM, FTDI/MOSI does not force a level on the bi-directional line?

If this is the case, and the FTDI is always the master - therefore the slave cannot send data on the bi-dir line without a clock cycle from the master - is it not possible to only force FTDI/MOSI as input (e.g. hi-z) on a read sequence (EEPROM --> FTDI/MISO), read out the values, then revert back to FTDI/MOSI as output as soon as the read sequence is over - i.e. leave the FTDI device in its default setup (MOSI: out, MISO: in) except during the read sequence?

I guess I missed something at some point :P

[sgoadhouse]

You have it exactly right. During a read when the slave (EEPROM) is expected to drive the data line, the MOSI line is changed to be an input so that the MISO line can receive from the slave. This is exactly what my code does. The problem is that I cannot find a way to tell the FTDI to change direction of the MOSI line without also having to tell it what state ALL of the outputs must be in. So in order to change the direction of the MOSI line, I must know what value was last sent with the command SET_BITS_LOW so that I do not cause an output to change state while trying to make MOSI an input. So I use self._last_prolog_ctrl to try to save the state that the outputs were last set to. With the ability to make the prolog optional (ie. no start bit), this complicates matters a little.

[eblot]

I must know what value was last sent with the command SET_BITS_LOW so that I do not cause an output to change state while trying to make MOSI an input.
This is true the FTDI lacks several useful feature (such as precise clocking without altering the clock output to generate proper delays, and SET/GET of bits instead of bytes...)

Nevertheless, the SCLK state is known (as it only depends on CPOL/CPHA), the MOSI/MISO should be don't care are both are to be set as inputs. Do you have to preserve the state between two write to the EEPROM - just asking, I'm not sure to get it?

[sgoadhouse]

Sorry for the delay in responding. You make a very good point. MOSI/MISO does not matter then since they will both be set to inputs, CS must remain active and SCLK can be known through CPOL/CPHA. There is also the GPIO, but I believe I know how to handle that.

…

-- Stephen Goadhouse Electronics Engineer University of Virginia Physics Department office: room 265 434-982-5594 sgoadhouse@virginia.edu<mailto:sgoadhouse@virginia.edu> http://faculty.virginia.edu/phys-electronics<http://faculty.virginia.edu/phys-electronics/> For any Electronics Shop requests, please email the team at phys-electronics@Virginia.EDU<mailto:physics-electronics-shop@Virginia.EDU> or feel free to come see Larry St. John in the Electronics Shop in Physics Rm 115 On Feb 23, 2018, at 06:54, Emmanuel Blot <notifications@github.com<mailto:notifications@github.com>> wrote: I must know what value was last sent with the command SET_BITS_LOW so that I do not cause an output to change state while trying to make MOSI an input. This is true the FTDI lacks several useful feature (such as precise clocking without altering the clock output to generate proper delays, and SET/GET of bits instead of bytes...) Nevertheless, the SCLK state is known (as it only depends on CPOL/CPHA), the MOSI/MISO should be don't care are both are to be set as inputs. Do you have to preserve the state between two write to the EEPROM - just asking, I'm not sure to get it? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub<#86 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AAfWSvRl4NZJgiFqxGbevadoqBAkqt1Zks5tXqb9gaJpZM4SLC6F>.

[eblot]

Just to let you know, I've committed some fixes with SPI & GPIO earlier today. I do not think you need to rebase your code on this new version (v0.28.7), but you might be bitten with the issues I just fixed - if you test GPIO access.

[sgoadhouse]

Thanks for the heads up. I made the code changes to eliminate the state with _last_prolog_ctrl and I also found a bug in my test case (tests/microwire.py) and fixed it. I rebased and just submitted the changes. Hopefully these can meet approval. Thanks for helping me to improve this code and being willing to work with me until I got it right.

[eblot]

Thanks. I need to set up a working SPI/µWire environment to run the tests. I will merge you work once it's done, and keep you posted.