adafruit / circuitpython

CircuitPython - a Python implementation for teaching coding with microcontrollers
https://circuitpython.org
MIT License
3.95k stars 1.16k forks source link

ESP32-S2: glitchless deinit #3423

Open emard opened 3 years ago

emard commented 3 years ago

For my project I often have to deinit a pin from bitbanging, start SPI, deinit SPI, bitbang again etc... After a pin or spi is deinit(), and also when SPI is init() it would be the best if the pin hardware can keep its electrical state before deinit() and doesn't introduce any glitch. For my use, only CLK pin is important to be glitchless. Currently a 1-CLK glitch happens which is luckily of consistent shape and is workaroundable in the code but I'd like to have cleaner solution if possible.

ESP32 (non-S2) with micropyhton has some SPI pinout which don't introduce any gitch to CLK.

if there's some glitch-free fix for ESP32-S2 I'll be glad to test on real hardware.

tannewt commented 3 years ago

It doesn't look like we're returning pins to INPUT when reset. Would you mind trying to do

        gpio_set_direction(i, GPIO_MODE_DEF_INPUT);
        gpio_pullup_dis(i);
        gpio_pulldown_dis(i);

Here: https://github.com/adafruit/circuitpython/blob/main/ports/esp32s2/common-hal/microcontroller/Pin.c#L58-L73 ?

Setting the pin to INPUT should float it and preserve the charge.

emard commented 3 years ago

I don't think setting pin to input is good idea, because Rest of hardware may pull it differently and we can't rely in charge capacitance.

Correct way would be if clk pin was e.g. in output state and driving "1" out, after deinit() it should continue to drive "1", not float.

On 9/15/20, Scott Shawcroft notifications@github.com wrote:

It doesn't look like we're returning pins to INPUT when reset. Would you mind trying to do

        gpio_set_direction(i, GPIO_MODE_DEF_INPUT);
        gpio_pullup_dis(i);
        gpio_pulldown_dis(i);

Here: https://github.com/adafruit/circuitpython/blob/main/ports/esp32s2/common-hal/microcontroller/Pin.c#L58-L73 ?

Setting the pin to INPUT should float it and preserve the charge.

-- You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub: https://github.com/adafruit/circuitpython/issues/3423#issuecomment-692893246

tannewt commented 3 years ago

deinit should definitely float because you are done with the pin. That's what all of the other CP ports do already.

We don't actually support transitioning between bitbang and hardware output with any of our APIs like you are trying to do.

How are you switching your DigitalInOut to output? Perhaps it's causing the glitch.

emard commented 3 years ago

Yes It is causing the glitch.

IMHO, deinit() should at least have some parameter how to leave the pin or it should leave it in the state it found. When a pin is deinit() it doesn't mean we are done with the rest of hardware. Imagine half-bridge drivers, solid state relays and similar things... going into input mode is not good.

Source is here, an example line when workaround is done. https://github.com/emard/esp32ecp5/blob/master/circuitpython/ecp5f.py#L71

On 9/15/20, Scott Shawcroft notifications@github.com wrote:

deinit should definitely float because you are done with the pin. That's what all of the other CP ports do already.

We don't actually support transitioning between bitbang and hardware output with any of our APIs like you are trying to do.

How are you switching your DigitalInOut to output? Perhaps it's causing the glitch.

-- You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub: https://github.com/adafruit/circuitpython/issues/3423#issuecomment-692916816

tannewt commented 3 years ago

We're not going to change how deinit works. If hardware requires a particular default state a pull resistor should be used.

Thanks for a link to your source. I suspect you may want to change this: https://github.com/emard/esp32ecp5/blob/master/circuitpython/jtag.py#L43-L45

The clock line is POL = 1 which means the default state is high between transactions. (According to Wikipedia) Setting .direction turns on the output with a low signal. By using switch_to_output(value=True) the output should start high and prevent a glitch.

emard commented 3 years ago

Hardware itself controlled by python code may not always require identical electical condition as safe-to-leave state after deinit() because hardware has memory too

Imagine red and green traffic light semaphores on a street crossing and we want to deinit() for a while for some reason - to use different driving method e.g. pwm, to upgrade or out of memory happened and it will force deinit(). If there's no traffic some pull resistors will let all lights be red or all off, but once the traffic starts going thru crossing we can't do so simple with resistors anymore

On 9/16/20, Scott Shawcroft notifications@github.com wrote:

We're not going to change how deinit works. If hardware requires a particular default state a pull resistor should be used.

Thanks for a link to your source. I suspect you may want to change this: https://github.com/emard/esp32ecp5/blob/master/circuitpython/jtag.py#L43-L45

The clock line is POL = 1 which means the default state is high between transactions. (According to Wikipedia) Setting .direction turns on the output with a low signal. By using switch_to_output(value=True) the output should start high and prevent a glitch.

-- You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub: https://github.com/adafruit/circuitpython/issues/3423#issuecomment-693561245

emard commented 3 years ago

A practical question: Lets say pins ESP32-S2 gpio35-37 are used for bitbanging and SPI and I need glitchless transition between those two modes.

What's the recommended solution, can I avoid gpio_pin.deinit() del gpio_pin while not hwspi.try_lock():

and use something like switch_to_output(), switch_to_input()?

tannewt commented 3 years ago

Did you try my recommendation from https://github.com/adafruit/circuitpython/issues/3423#issuecomment-693561245 ?

emard commented 3 years ago

I don't understand how to apply your recommendation in my code. So protocol must first start in bitbanging mode and then switch to SPI. If I try to init SPI then, it will be rejected reporting some error like "pin already in use".

My solution to avoid this error was to deinit the pin and delete it. It is very slow then, I suppose I needs a lot to allocates objects and memory.

tannewt commented 3 years ago

Here is my suggested change: https://github.com/emard/esp32ecp5/pull/7

emard commented 3 years ago

I have applied your change but glitch still happens. It could happen somewhere between turning off bitbang mode and turning on SPI mode bitbang_jtag_off() # NOTE TCK glitch spi_jtag_on()

https://github.com/emard/esp32ecp5/blob/master/circuitpython/jtag.py#L40

tannewt commented 3 years ago

Bummer. It could be that we need polarity in the SPI constructor so that it doesn't enable the pins without switching it: https://github.com/emard/esp32ecp5/blob/master/circuitpython/jtag.py#L62-L65

Another alternative would be adding a native JTAG primitive that can do it under the hood. That would give you more flexibility.

emard commented 3 years ago

Yes, polarity would beter fit in the constructor.

If you could add JTAG accelerator module using SPI or something else, that would really be excellent and speed up FLASH over JTAG significantly!

JTAG traffic is 99.9% same as 8-bit SPI but at command transitons, it needs to shift few bits, typically 1-4 bits clocking data over TMS line.

This sometimes cannot be replaced by clocking full byte, 8-bits. The point that slows things is write page to FLASH. After sending 256 of page bytes, last TDI bit must be sent together with the changed state of TMS line, and SPI can't send this, so the bitbanging mode is currently the only solution.

tannewt commented 3 years ago

I'm not planning on adding a JTAG module myself. I would review and merge one though.