adafruit / Adafruit_CircuitPython_Wiznet5k

Pure-Python interface for WIZNET 5k Ethernet modules
Other
16 stars 36 forks source link

W5100S-EVB-Pico reset pin and reset logic #51

Closed chabala closed 2 years ago

chabala commented 2 years ago

Hello. I'm new to microcontrollers and CircuitPython, but I think I may have found a bug in the reset logic here: https://github.com/adafruit/Adafruit_CircuitPython_Wiznet5k/blob/cd08178ad2a929ca6414e10e885fb826c21c144b/adafruit_wiznet5k/adafruit_wiznet5k.py#L167-L172

I'm using the W5100S-EVB-Pico, and have run the test code I found here without issue: https://github.com/adafruit/Adafruit_CircuitPython_Wiznet5k/pull/49#issuecomment-1009195699

I've also run some similar test code from WIZnet without issue: https://github.com/Wiznet/RP2040-HAT-CircuitPython/blob/master/examples/Network/W5x00_Ping_Test.py

What I noticed is that they differ in what pin they use for the reset line:

GP15: https://gist.github.com/anecdata/3c6f37c05a91f7b769dad7517bfe3aa1#file-code-py-L13

vs

GP20: https://github.com/Wiznet/RP2040-HAT-CircuitPython/blob/54cc1871fc3f86bccd4bb59817024af5ab687668/examples/Network/W5x00_Ping_Test.py#L13-L14

According to the Getting Started guide, GP20 is the correct pin. However, changing the reset pin to GP20 in the gist example caused the module to fail to initialize:

code.py output: Hard reset... Init... Traceback (most recent call last): File "code.py", line 39, in File "/lib/adafruit_wiznet5k/adafruit_wiznet5k.py", line 180, in init AssertionError: Failed to initialize WIZnet module.

Looking closer at the WIZnet example, it succeeds using GP20 by not passing it into the module, thus skipping the reset block shown above. Both examples implement their own reset logic external to the module, and I assert that the only reason the GP15 example doesn't fail when it specifies the reset pin into the module, is because it happens to be the wrong pin.

I changed my local copy of adafruit_wiznet5k.py so that the reset logic works like the external reset in the gist example, False, wait 1 second, True, and now it still initializes when GP20 is specified.

I'm uncertain if this change would negatively effect other boards that are sharing this logic however, otherwise I would have provided a PR.

anecdata commented 2 years ago

Good catch. I do have the wrong pin (not sure of the origin of that), and resetting an unused pin does nothing. If I understand your comment, the longer delay on the correct active-low pin is what makes it work. The only two boards / modules that use this library are the W5100S (e.g., W5100S-EVB-Pico and the Hat version of that), and the W5500 (e.g.,, Adafruit Ethernet FeatherWing and similar, plus I think the Particle ethernet featherwing). The FeatherWing does not have reset connected by default. Reset is only used in the library during init, so I don't see downside to giving it a delay that works.

The datasheet calls for 560ns active-low then 60.3ms max to stabilize, but for some reason 100ms (python time) wasn't enough. Perhaps it could be made smaller than 1s.

Would you like to do a PR? I have the two wiznet boards and an ethernet featherwing and can test.

chabala commented 2 years ago

Would you like to do a PR?

Sure, I'll give it a go. I'll test a bit and see if I can get away with less than 1s. Perhaps it's just high->low vs low->high transitions that make the difference.