Wrong "spi.gpio" pin direction reconfig, and fix suggestion !

[Bnjamn]

Hello eblot,

I'm using this pyftdi library for a professionnal project. It is very great. Thanks you for that, it's a big help !

But I want to report an issue on the spi.gpio "pins reconfiguration direction". I noticed that if I try to reconfigure one pin direction (and I want to keep previous config for others), it erase the previous config and keeps only the config for the pin I requested...

Going through the code (spi.py) I found (in SpiController() class) the following function : -----> _set_gpio_direction
The last line is :
-----> self._gpio_mask = gpio_mask & pins
But this is wrong (I guess) Instead of using the "&" operator, I suggest to use the "|", and it actualise much better the _gpio_mask. I helps to keeps the previous config of other pins which was erasing before by using "&"... :-)

[eblot]

Hi. Good catch!

I think the line should be removed. There is no reason to change the GPIO mask here, only the direction of the selected pins.

[Bnjamn]

Hi back,

By trying to remove the line, it doesn't work anymore...

The "@property direction" return a correct feedback according to my re-configuration. But at the end I'm not able to read anymore the bits of my gpio port....

So I prefer to keep the "or" operand on that line (701) : -----> self._gpio_mask = gpio_mask | pins

Actually, the "read_gpio" function DO USE "self._gpio_mask" So it must be updated, isn't it ? (just trying to understand a bit better, sorry ^^)

[zzts]

If you follow the code, this is the only place the _gpio_mask is set and/or updated except for the initialization to 0 at the beginning of the class. At this point in the code gpio_mask is FFF0 for a 16 bit port and F0 for an 8 bit port. The current code "forgets" any previously configured pins. If you remove line 701, _gpio.mask will never be changed. If you OR it you get nothing new (since it is FFF0). Prior to running the set_direction for the first time, gpio.pins returns FFF0 or F0 indicating that all pins are configured. This a result of line 432 432 self._set_gpio_direction(16, (~self._spi_mask) & 0xFFFF, io_dir) which is run to configure the spi interface. This done to allow the initial setting of direction and pin state with the configure command.

After setting pins and directions, gpio.pins returns only newly configured pins, which is ok the first time around if you didn't set direction and pin state with the configure command.

self._gpio_mask |= pins will add the new pins and retains previously configured pins, however it does not allow you remove pins from configuration and retains every pin as configured except the spi port pins. This is also not acceptable.

I'm not sure yet how to fix the situation in the code, so I have to keep track of the pins and directions in my code and resend the ones I want to retain to the set_direction. ie. gpio = GPIO port assignment

config = gpio.pins
direct = gpio.direction
config &= ~newPins  
config |=  newPins
direct &= ~newdirect
direct |= newDirect
gpio.set_direction(config, direct)

To deconfig a pin and leave the remainder as is: gpio.set_direction(gpio.pins & ~ pins_to_release, gpio.direction & ~pins_to_release)

[TedKus]

I tried a few cases with the gpio pins. The trouble is going into and out of tri-state (I'm using I2C) if the direction is set to 0 (input or tri-state) then that pin cannot be driven again. if not careful about it, none of them can be driven again. My branch has a fix for the I2C side and the following code works to update a specific pin gpio_position = pin + 3 # bit number 3 pin_mask = 1 << gpio_position pins_states = self.gpio().read(with_output=True) & 0x78 if state:

Note: drive = 1 is HIGH

         new_pins_states = pins_states | pin_mask

else:

Note: drive = 0 is LOW

         new_pins_states = pins_states & ~pin_mask

try: self.gpio().write(new_pins_states) except I2cIOError: temp = ((self.gpio().direction >> gpio_position) & 1) print(f"gpio:{pin} is Tri-State\n" f"the pin direction is: {temp}\n" f"the .direction is {bin(self.gpio().direction)}\n" f"the .value is {bin(new_pins_states)}") raise

with my patch in place, and catching the exception: by printing the binary like that it was easy to see that read was returning 16 bits, so the inner function failed unless I added the & with 0x78 to keep it to the lower bits (all the C232 can reach). There may be some cases where wide-port devices are not really addressable.

I guess what I'm saying is that you were right. the mask should be in the configure, and it doesn't need to be altered (after all, they're fixed pins!) in configure: gpio_mask = (1 << 16) - 1 # set mask for wide port (16-bit) gpio_mask &= ~self._i2c_mask self._gpio_mask = gpio_mask self._set_gpio_direction(io_out, io_dir)

[TedKus]

Pull #260 addresses similar issue in I2C. I will be using SPI later this year and will look at this then, but the changes would be similar.