Skip to content

stm32: Use lib/wiznet5k instead of drivers/wiznet5k. #9019

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 4 commits into from

Conversation

robert-hh
Copy link
Contributor

This PR uses lib/wiznet5k instead of drivers/wiznet5k, like the rp2040 port already does. The change was initiated by @omogenot, and I picked up the baton for test, bug fixing and completion. See the thread at issue #9013. I tested the code change with a Wiznet W5500 breakout, with and without LWIP support. The performance is equivalent to the W5500 support for RP2040.
Two notes:

  • The last commit in this PR consists of the combined changes of PR extmod/network_wiznet5k.c: Five small bug fixes. #8974, so testing runs well. This last commit can either be removed, or the other PR gets obsolete.
  • There is a hiccup in the CI of this PR. If I include lib/wiznet5k as a submodule only in builds for W5x00 devices, then it is not pulled in the CI build and it fails because of the missing files. If I included lib/wiznet5k unconditionally, the build with cc3k support fails. What could be done?

The object retuned for pyb.spi is not changed. This function is also used
for the cc3300 support.
- Register nic when the LWIP stack is used.
  That was missing, and network.route() returned an empty list.
- Drop an obsolete argument check.
  The proper checks are done further down in the code.
- Deinit the nic before (re-)initialisation.
  If nic.active(True) is called several times in a row, the device may
  lock up. Even if that is bad coding practice, calling wiznet5k_deinit()
  in wiznet5k_init() prevents the lock.
- Check for the absence of keyword arguments.
  Using keyword arguments raises an error now.
- Schedule clearing of interrupt flags.
  Avoiding conflicts between the IRQ and an active transfers.
  Before the change, the device could lock up in heavy traffic situations.
  Fix found and code supplied by @omogenot.
- Rearrange the function wiznet5k_poll().
  To have just one exit and a more compact flag test. This is just a
  style change without impact to the function.
- Clear interrupt for W5100S.
@omogenot
Copy link
Contributor

omogenot commented Aug 5, 2022

@robert-hh
What about:

ifneq ($(MICROPY_PY_CC3K),1)
GIT_SUBMODULES += lib/wiznet5k
endif

in Makefile after line 38:
GIT_SUBMODULES += lib/libhydrogen lib/lwip lib/mbedtls lib/stm32lib

@robert-hh
Copy link
Contributor Author

robert-hh commented Aug 5, 2022

There's already a conditional GIT_SUBMODULES += lib/wiznet5k in the block of ifneq ($(MICROPY_PY_NETWORK_WIZNET5K),0). And I placed that for tests already after the other GIT_SUBMODULES statement. at line 38. No change.
It's more a problem in the way CI builds the test environments, by not (always) including the required submodules. resp. not setting the proper flags when doing so. So it looks like it uses the same build environment for all Wiznet and CC3K variants.

@omogenot
Copy link
Contributor

omogenot commented Aug 5, 2022

There's already a conditional GIT_SUBMODULES += lib/wiznet5k in the block of ifneq ($(MICROPY_PY_NETWORK_WIZNET5K),0). And I placed that for tests already after the other GIT_SUBMODULES statement. at line 38. No change.

I propose to do a conditional GIT_SUBMODULES += lib/wiznet5k with a CC3K condition instead:

# CC3K vs WIZNET driver incompatibility workaround
ifneq ($(MICROPY_PY_CC3K),1)
GIT_SUBMODULES += lib/wiznet5k
endif

It might not be an obvious workaround if someone reads the makefile in the future, but I kind of remember that you told me that this CC3K was obsolete, no?

@robert-hh
Copy link
Contributor Author

robert-hh commented Aug 5, 2022

No. That does not help. It looks like the problem is in ci.sh. The respective section is below:

function ci_stm32_pyb_build {
    make ${MAKEOPTS} -C mpy-cross
    make ${MAKEOPTS} -C ports/stm32 submodules
    git submodule update --init lib/btstack
    git submodule update --init lib/mynewt-nimble
    make ${MAKEOPTS} -C ports/stm32 BOARD=PYBV11 MICROPY_PY_NETWORK_WIZNET5K=5200 MICROPY_PY_CC3K=1 USER_C_MODULES=../../examples/usercmodule
    make ${MAKEOPTS} -C ports/stm32 BOARD=PYBD_SF2
    make ${MAKEOPTS} -C ports/stm32 BOARD=PYBD_SF6 NANBOX=1 MICROPY_BLUETOOTH_NIMBLE=0 MICROPY_BLUETOOTH_BTSTACK=1
    make ${MAKEOPTS} -C ports/stm32/mboot BOARD=PYBV10 CFLAGS_EXTRA='-DMBOOT_FSLOAD=1 -DMBOOT_VFS_LFS2=1'
    make ${MAKEOPTS} -C ports/stm32/mboot BOARD=PYBD_SF6
    make ${MAKEOPTS} -C ports/stm32/mboot BOARD=STM32F769DISC CFLAGS_EXTRA='-DMBOOT_ADDRESS_SPACE_64BIT=1 -DMBOOT_SDCARD_ADDR=0x100000000ULL -DMBOOT_SDCARD_BYTE_SIZE=0x400000000ULL -DMBOOT_FSLOAD=1 -DMBOOT_VFS_FAT=1'

If I run the line
make ${MAKEOPTS} -C ports/stm32 BOARD=PYBV11 MICROPY_PY_NETWORK_WIZNET5K=5200 MICROPY_PY_CC3K=1 USER_C_MODULES=../../examples/usercmodule
locally, I get the same error messages. MICROPY_PY_NETWORK_WIZNET5K=5200 and MICROPY_PY_CC3K=1must not be combined in one compile.

@robert-hh
Copy link
Contributor Author

I'll close that for now to do a silent test for ci.sh changes.

@robert-hh robert-hh closed this Aug 5, 2022
@omogenot
Copy link
Contributor

omogenot commented Aug 5, 2022

Apparently the main issues are:

drivers/cc3000/inc/socket.h:67: error: "SOCK_STREAM" redefined
drivers/cc3000/inc/socket.h:67: error: "SOCK_DGRAM" redefined

I will try to do a #ifdef #undef to see if it goes further

Edit: No chance. It's a matter of include path precedence... Impossible to solve without, as you said, prevent make call with multiple network drivers.

By the way, the MICROPY_PY_NETWORK_CYW43=1 does not seem to compile, at least on my machine for a missing definition of symbol pyb_pin_WL_SDIO_1 the compiler is proposing to use pyb_pin_WL_SD_D1 instead. Was eventually this pin renamed amongst the various versions?
Our work looks like the Danaïdes' barrel :-). Every time you fix a thing a new one blows up.

@robert-hh
Copy link
Contributor Author

I have a proper set-up, ci passes. I'm just cleaning it now and run a further test.

@robert-hh
Copy link
Contributor Author

robert-hh commented Aug 5, 2022

I changed ci.sh in splitting the builds for wiznet5k and cc3k. But to be safe, I opened a new PR. I could not tell that after re-opening all commits were included.

RetiredWizard pushed a commit to RetiredWizard/micropython that referenced this pull request Mar 8, 2024
…03-07

update frozen libraries for 9.0.0-rc.0
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