Skip to content

ENC28J60: add missing end() required by LwipIntfDev - #9323

Merged
mcspr merged 1 commit into
esp8266:masterfrom
cl445:fix/enc28j60-end
Aug 27, 2026
Merged

ENC28J60: add missing end() required by LwipIntfDev#9323
mcspr merged 1 commit into
esp8266:masterfrom
cl445:fix/enc28j60-end

Conversation

@cl445

@cl445 cl445 commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

LwipIntfDev<RawDev> calls RawDev::end() in LwipIntfDev::end() and in the two error paths of LwipIntfDev::begin() (netif_add failure, dhcp_start() returning ERR_IF). Wiznet5500 provides end(), ENC28J60 does not, so LwipIntfDev<ENC28J60> (i.e. ENC28J60lwIP) no longer compiles:

LwipIntfDev.h:280:20: error: 'end' is not a member of 'ENC28J60'
LwipIntfDev.h:294:24: error: 'end' is not a member of 'ENC28J60'

The calls were introduced in #9023 ("LwipIntfDev - method end() to enable repeated begin"), which only touched cores/esp8266/LwipIntfDev.h. The ENC28J60 driver was not updated at the time.

It showed up in firmware that instantiates both ENC28J60lwIP and Wiznet5500lwIP and selects the interface at runtime: the W5500 translation unit builds, the ENC28J60 one does not.

Wiznet5500::end() closes socket 0, clears the socket interrupt flags and waits for SOCK_CLOSED. It stops the data path but leaves the chip powered and configured, because a following begin() performs a software reset and reopens the socket anyway. The ENC28J60 equivalent of "close the socket" is clearing ECON1.RXEN, the bit that ENC28J60::reset() sets as the last step of initialization to enable reception. ECON1 is mapped into all banks, so no bank switch is required, and no transmission can be in flight because sendFrame() polls ECON1.TXRTS until the frame has left.

A full software reset (SPI command 0xFF) was considered. It goes beyond what Wiznet5500::end() does, and it would have to carry the erratum #2 delay of at least 1 ms before the chip may be accessed again. Since begin() already issues a software reset via reset(), the reset in end() adds nothing, so the minimal variant is used. Happy to switch if a hard reset in end() is preferred.

With this change ENC28J60lwIP compiles again and the repeated-begin() behaviour intended by #9023 also works for the ENC28J60.

Testing

Built a firmware that instantiates both drivers against this branch. Without the patch it stops with the two errors above; with it the same build links successfully (RAM 50.8 %, flash 58.5 %). clang-format with tests/clang-format-core.yaml leaves both files unchanged.

Not yet verified on hardware: calling end(), then begin(), then passing traffic on a real ENC28J60.

LwipIntfDev<RawDev> calls RawDev::end() from LwipIntfDev::end() and from the two
error paths of LwipIntfDev::begin(). Wiznet5500 provides end(), ENC28J60 does
not, so LwipIntfDev<ENC28J60> - i.e. ENC28J60lwIP - no longer compiles:

  LwipIntfDev.h:280:20: error: 'end' is not a member of 'ENC28J60'
  LwipIntfDev.h:294:24: error: 'end' is not a member of 'ENC28J60'

The calls came in with esp8266#9023, which only touched cores/esp8266/LwipIntfDev.h.

Wiznet5500::end() closes socket 0 and leaves the chip powered and configured,
since a following begin() resets and reopens it anyway. The ENC28J60 equivalent
of closing the socket is clearing ECON1.RXEN, the bit that reset() sets last to
enable reception. ECON1 is mapped into every bank, so no bank switch is needed,
and no frame can be in flight because sendFrame() polls ECON1.TXRTS until the
frame has left.
@cl445
cl445 force-pushed the fix/enc28j60-end branch from 56cddec to b01e7d0 Compare August 15, 2026 13:08
@cl445
cl445 marked this pull request as ready for review August 15, 2026 13:08
@mcspr
mcspr merged commit 1475ed7 into esp8266:master Aug 27, 2026
28 checks passed
@mcspr

mcspr commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Thanks.

CI probably should handle this, since we currently only build one version of the ethernet wrapper header through the example .ino
Just not sure what would be the most appropriate way for this besides simply duplicating example code per driver

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants