Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Support for RM2 break out boards#16057

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
peterharperuk wants to merge5 commits intomicropython:master
base:master
Choose a base branch
Loading
frompeterharperuk:pico2_w_changes

Conversation

peterharperuk
Copy link
Contributor

Adds changes needed to support Pico 2 W. This requires the develop version of the sdk so I've kept the PR in draft.

dpgeorge, Gadgetoid, and nagyPista reacted with hooray emoji
@peterharperuk
Copy link
ContributorAuthor

I see one test failure for rp2040 Pico in rp2_dma which implies DMA has go slower. It does appear to be caused by the sleep change which doesn't make a lot of sense. I tried taking an average over a number of runs and get much better results - even if it's a loop of 1?!

@codecovCodecov
Copy link

codecovbot commentedOct 21, 2024
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.57%. Comparing base(39538e4) to head(c3570b2).

Additional details and impacted files
@@           Coverage Diff           @@##           master   #16057   +/-   ##=======================================  Coverage   98.57%   98.57%           =======================================  Files         164      164             Lines       21349    21349           =======================================  Hits        21045    21045             Misses        304      304

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report?Share it here.

@github-actionsGitHub Actions
Copy link

github-actionsbot commentedOct 21, 2024
edited
Loading

Code size report:

   bare-arm:    +0 +0.000% minimal x86:    +0 +0.000%    unix x64:    +0 +0.000% standard      stm32:    +0 +0.000% PYBV10     mimxrt:    +0 +0.000% TEENSY40        rp2: -1616 -0.177% RPI_PICO_W[incl +4(bss)]       samd:    +0 +0.000% ADAFRUIT_ITSYBITSY_M4_EXPRESS  qemu rv32:    +0 +0.000% VIRT_RV32

@dpgeorge
Copy link
Member

I see one test failure for rp2040 Pico in rp2_dma which implies DMA has go slower. It does appear to be caused by the sleep change which doesn't make a lot of sense. I tried taking an average over a number of runs and get much better results - even if it's a loop of 1?!

That could be due to flash caching effects, where the first run through code is slower but after that it's faster.

@peterharperuk
Copy link
ContributorAuthor

That could be due to flash caching effects, where the first run through code is slower but after that it's faster.

Maybe. Resetting src before the copy makes it twice as fast. Maybe that puts it in the cache? I don't quite understand why this is faster however...

for i in range(0,1):    dt = run_and_time_dma(dma)

@peterharperukpeterharperukforce-pushed thepico2_w_changes branch 3 times, most recently from9a47835 to57472a3CompareOctober 24, 2024 13:07
@peterharperuk
Copy link
ContributorAuthor

I've taken the liberty to add the ability to use cyw43 with normal Pico or Pico 2 devices as we're starting to see cyw43 breakout boards appearinghttps://shop.pimoroni.com/products/rm2-breakout?variant=53492995719547

If you wanted to do this you have to enable it and define the default pins to use...

cmaked .. -DPICO_BOARD=pico -DPICO_CYW43_SUPPORTED=1 -DCYW43_DEFAULT_PIN_WL_REG_ON=2 -DCYW43_DEFAULT_PIN_WL_DATA_OUT=4 -DCYW43_DEFAULT_PIN_WL_DATA_IN=4 -DCYW43_DEFAULT_PIN_WL_HOST_WAKE=4 -DCYW43_DEFAULT_PIN_WL_CLOCK=5 -DCYW43_DEFAULT_PIN_WL_CS=3 -DCYW43_PIO_CLOCK_DIV_INT=3

You can use different pins by configuring them at run time.

wlan = network.WLAN(network.STA_IF, pin_on=6, pin_out=8, pin_in=8, pin_wake=8, pin_clock=9, pin_cs=7, div=3)
Gadgetoid reacted with hooray emoji

@Gadgetoid
Copy link
Contributor

Awesome stuff. Will switch our wireless builds over to this and see how it fares!

peterharperuk reacted with thumbs up emoji

@Gadgetoid
Copy link
Contributor

Gadgetoid commentedNov 5, 2024
edited
Loading

Thispossibly breaks PPP support, though I can't seem to figure out why. Might be a configuration quirk.

(Doing some poking and rebasing in some commits from master to see if I can either fix it, or figure out what's awry... have some variations onppp.connect() either stalling altogether, or producing apppos_create failed message. My initial thought was something to do with LwIP but I can't find anything in this PR that might cause it.

Update: Went with my gut instinct and reverted50cfd54, seems to have fixed PPPbut has probably hosed WiFi...

@Gadgetoid
Copy link
Contributor

Also seem to be having some trouble with WiFi not working on soft reset. If I run the same code twice I need to press reset on a board or I'll get connection aborted orOSError -2.

I'm also having a lot of trouble establishing a connection at all which might be related to my router, or my early-stage development boards, or something else, but I know I'm not the only one encountering strangeness. I can't seem to quantify it any better than that- though- and without a Pico 2 W as a reference it's tricky to eliminate any external factors.

@peterharperuk
Copy link
ContributorAuthor

I've tested this with a Pico 2 and your RM2 breakout. I sometimes have to slow the clock div to 3. I believe this was the command line...

import network
wlan = network.WLAN(network.STA_IF, pin_on=2, pin_cs=3, pin_dat=4, pin_clock=5, div_int=3)

I'll check what happens after a soft reset.

@Gadgetoid
Copy link
Contributor

Weirdly no amount of messing with the clock divider seems to help me.

It will either stubbornly refuse to connect, or - seemingly out of the blue - connect and workfine all night until I run the code again at which point GOTO STUBBORN.

I can only conclude it's something up with my router (A Mikrotik L009 I use for 2.4GHz devices) but it feels like it's only got particularly bad recently.

I guess I should build this code for the Pico W and see what happens!

@Gadgetoid
Copy link
Contributor

Can't replicate on a Pico W, even if I enable dynamic pins. Either specifically a Pico 2 / RP2350 problem, or something up with our batteries-included builds.

@peterharperuk
Copy link
ContributorAuthor

The only problem I have "sometimes" is when I pull the plug on a connected device. It will often take a couple of attempts to reconnect "next time". But it sounds like you have a bigger issue. I can send you a Pico 2 W if you like. Otherwise I'll just spend some time playing around with your RM2 breakout.

@Gadgetoid
Copy link
Contributor

I think our builds were not configuring the reserved pin behaviour, which could explain an awful lot.

I've gone back to the proverbial drawing board and rebased my Plasma 2350 W config upon your Pico 2 W config and it seems to work. Will try the same with Pico Plus 2 (W). With your changes there's no reason I should be rolling a W vs non-W build at all so I'm beavering away combining everything (which dramatically simplifies things too, handy)

(I'd still love a Pico 2 W for testing, but it's not urgent 😆)

I think the issue I mentioned above regarding50cfd54 breaking PPP is still a problem, but I will re-test once I've finished all my mucking about. Thanks!

@Gadgetoid
Copy link
Contributor

We're still seeing an issue establishing a connection on the first try, which I'm working on narrowing down. Otherwise, things are working much, much better. It was definitely related to MicroPython's soft reset pin cleanup trouncing the comms pin. (Might be fun with a wireless module for Yukon 😆)

@Gadgetoid
Copy link
Contributor

Since it would probably be difficult to support multiple wireless interfaces (though it would be really cool), should thenetwork.WLAN() class currently prevent the user from creating more thanone instance?

@peterharperuk
Copy link
ContributorAuthor

multiple wireless interfaces

I wondered when someone was going to ask for that feature :) I guess it's more of a problem now you have the ability to change the pins.

@Gadgetoid
Copy link
Contributor

I've been trying very hard not to ask! (at least as much because I can't think of a sensible use-case other than intellectual curiosity, but that doesn't mean there isn't one)

It's possible, I think?, but potentially a bit of a nightmare refactor, and actually making use of multiple network interfaces is non-trivial.

@peterharperuk
Copy link
ContributorAuthor

Yeah it should work (I've had a wifi and lan connections working at the same time with a wiznet evb). The wifi driver relies on cyw43_state global all over the place - need to fix that somehow.

@anecdata
Copy link

@peterharperuk
Copy link
ContributorAuthor

@anecdata try raising an issue in herehttps://github.com/georgerobotics/cyw43-driver (note: you can already connect Wifi in STA and AP at the same time).

anecdata reacted with thumbs up emoji

@dpgeorge
Copy link
Member

@peterharperuk can you please rebase this now that#16313 is merged?

Gadgetoid reacted with eyes emoji

When building allow the port to be configured on the command line bypassing a value for CMAKE_ARGS.Signed-off-by: Peter Harper <peter.harper@raspberrypi.com>
Signed-off-by: Peter Harper <peter.harper@raspberrypi.com>
Signed-off-by: Peter Harper <peter.harper@raspberrypi.com>
Signed-off-by: Peter Harper <peter.harper@raspberrypi.com>
Allow cyw43 pins and clock to be configured at runtime.Move cyw43 init code from main.e.g.wlan = network.WLAN(network.STA_IF, pin_on=2, pin_cs=3, pin_dat=4,pin_clock=5, div_int=3)ble = bluetooth.BLE(pin_on=2, pin_cs=3, pin_dat=4, pin_clock=5, div_int=3)ext_led = Pin('WL_GPIO0', Pin.OUT, pin_on=2, pin_cs=3, pin_dat=4,pin_clock=5, div_int=3)Signed-off-by: Peter Harper <peter.harper@raspberrypi.com>
@peterharperukpeterharperuk changed the titlePico 2 W changes and support for RM2 break out boardsSupport for RM2 break out boardsDec 19, 2024
@peterharperuk
Copy link
ContributorAuthor

peterharperuk commentedDec 19, 2024
edited
Loading

I re-ran the tests on all devices. extmod/machine_i2s_rate.py failed once on pico2/rm2 but I couldn't repeat this. rp2_dma is failing on pico2/rm2 as noted before (I guess I should look info fixing this) but not on pico2_w (bit weird)?

I see the thread_stdin issue on riscv as well but failures of select_poll_eintr, stress_schedule and thread_sleep2 when cyw43 is enabled. These don't look like regressions. I think I missed these failures previously.

@peterharperukpeterharperuk marked this pull request as ready for reviewDecember 19, 2024 14:25
@peterharperuk
Copy link
ContributorAuthor

I've put the sleep changes over here#16454

@@ -34,26 +34,26 @@ ifeq ($(BUILD_VERBOSE),1)
MAKE_ARGS += VERBOSE=1 # Picked up in Makefile generated by CMake
endif

CMAKE_ARGS += -DMICROPY_BOARD=$(BOARD) -DMICROPY_BOARD_DIR="$(abspath $(BOARD_DIR))"
overrideCMAKE_ARGS += -DMICROPY_BOARD=$(BOARD) -DMICROPY_BOARD_DIR="$(abspath $(BOARD_DIR))"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I really don't think it's a good idea to complicate the Makefile by addingoverride. Combining make and cmake is already tricky enough.

Why exactly is this needed, what were you trying to achieve with it? Can we do it a different way, eg just call cmake directly in such cases?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I think the intent is to pass compiler definitions through to a port, effectively skipping the need to change config files? Possibly to optionally build Pico with RM2 support.

We have variants for this, though there are few examples of that in practise. Here's one for switching to a RISCV build:https://github.com/micropython/micropython/tree/master/ports/rp2/boards/SEEED_XIAO_RP2350

@dpgeorge
Copy link
Member

Now that the call tocyw43_init() is moved out of main and only called if/when the WiFi/BLE/LED is used, does that cause any issues with the WL GPIO left uninitialised? Mostly I'm thinking about WL_ON = GPIO23: shouldn't this be configured as output in a low state to keep the cyw43 off until it's needed?

@dpgeorge
Copy link
Member

I'm glad that the Pico 2 W changes were pulled out to a separate commit (and merged!) because now this PR is a lot cleaner, and it's clearer what it's trying to do (allow dynamic cyw43 config).

And I now realize the complexity that's added here to the constructors of WLAN, BLE and Pin. It wasn't clear to me before how deep these changes were, I thought they were only related to the WLAN part.

I really don't think themachine.Pin constructor should take cyw43 pin arguments (so the cyw43 can be set up to use the external GPIO). That seems really convoluted and a violation of layering (pins needing pins). I'm aware I suggested this, over the previous "set cyw43 pins" approach, but I didn't appreciate then that it would affect themachine.Pin constructor.

Taking a step back, what's needed here is a better abstraction/model of the hardware. The cyw43 peripheral is not just WLAN, it's also BLE and GPIO. So maybe it needs to be its own entity, and then you obtain the sub-entities from that. Eg:

importcyw43# construct a cyw43 instance (this will persist over soft reset)cyw=cyw43.CYW43(regon=2,cs=3,data=4,clock=5,div_int=3)# get the sub-systems from the cyw43 instancewlan=cyw.WLAN()ble=cyw.BLE()led=cyw.Pin(0)

The downside with this approach is that you can no longer usenetwork.WLAN() orbluetooth.BLE(). The former is not too bad, all the network tests will still work because they only accesssocket. But the BLE tests won't work because they needbluetooth.BLE(). Also, the user won't be able to access the LED (or other GPIO) viamachine.Pin. I don't think that's such a problem though, they know if the board has a cyw43 breakout or not and need to do things slightly differently constructing the objects.

Alternatively, when thecyw43.CYW43 instance is created, it could activate the WLAN and BLE so thatnetwork.WLAN() andbluetooth.BLE() would "just work" and return the cyw43's instance of those peripherals. In the case of multiple cyw43 breakout's (a future possibility) the WLAN and BLE entities could be specified by higher identifiers, egbluetooth.BLE(1). Eg:

importcyw43# construct a cyw43 instance (this will persist over soft reset)cyw=cyw43.CYW43(regon=2,cs=3,data=4,clock=5,div_int=3)# access sub-systems as normalwlan=network.WLAN()ble=bluetooth.BLE()led=machine.Pin("LED")

That's very similar to the "set cyw43 pins" approach, except that it's a cyw43 object that you create, rather than a function you call (to set the pins). The benefit of this is that you can just put acyw43.CYW43() line in boot.py/main.py somewhere, and then all existing code samples just work the same.

(Looking at hownetwork.WIZNET5K works, that's much easier because that peripheral is just supplying LAN. The CYW43 doesn't match this pattern so really needs something new, like suggested above.)

@eightycc
Copy link

Over in CircuitPython we've run into an issue (adafruit#9777) with the RM2 breakout that may need to be addressed in this issue. The RM2 (and probably the bare CYW43439) appears to automatically select phasing for RM2 to host transfers depending on the gSPI clock speed. Reducing the gSPI clock speed below a certain (unknown) threshold causes "normal mode" to be forced by the RM2. Writing a 1 to gSPI register 0x0000 bit 4 has no effect in this case.

@peterharperuk
Copy link
ContributorAuthor

@eightycc I've had to increase the pio divisor to 3 for reliable operation for reasons unknown. I don't know if that's the same issue.

@eightycc
Copy link

@peterharperuk I'd suspect signal issues for your case. The problem I'm reporting kicks in around divide by 10 and up. I hit it while increasing the divisor to compensate for a nasty fly wire bodge I've been using for tracing.

@peterharperuk
Copy link
ContributorAuthor

I was led to believe (maybe wrongly) that the firmware was built to only work for a specific SPI clock speed. And that's why we have to mess about with the divisor if we change the core clock speed.

@eightycc
Copy link

Once I changed fromspi_gap01_sample0 tospi_gap0_sample1 for the PIO program at slower clock speeds I've been able to go all the way down to 625kHz with no issues, so I don't believe there's a firmware issue.

@eightycc
Copy link

eightycc commentedDec 22, 2024
edited
Loading

@peterharperuk I've done some more testing with both the CYW43439 on the Pico W boards and the RM2 module.

The older CYW43439 operates as described in the Cypress Semi datasheet. It does not change gSPI data phasing at lower clock rates.

The newer RM2 modules change gSPI data phasing somewhere between 21.429MHz and 23.077MHz on the two RM2 modules I tested.

Assuming a 150MHz processor clock, a divisor of 3 results in a 25MHz gSPI clock. Depending on the accuracy of the RM2's gSPI clock measurement, a divisor of 3 could conceivably fail intermittently. At 23.077MHz (divisor 3, fraction 64) with a debug build I'm seeing instability.which leads me to think that there's a range of gSPI clock frequencies centered around 22MHz that are unusable.

Another factor that may affect the RM2's gSPI clock measurement leading to unpredictability is the PIO clock divider's insertion of "leap" cycles to maintain the specified clock rate. The resulting jitter may cause measurement of small clock samples by the RM2 to be inaccurate causing it to change phasing erratically.

Back to stable operation at 18.75MHz (divisor 4, fraction 0), but need to specifyCYW43_SPI_PROGRAM_NAME="spi_gap0_sample1" to accommodate the change in phasing.

@Gadgetoid
Copy link
Contributor

Gadgetoid commentedMar 26, 2025
edited
Loading

The benefit of this is that you can just put a cyw43.CYW43() line in boot.py/main.py somewhere, and then all existing code samples just work the same.

This makes sense.

It feels like there should be a better approach to the pins.csv wrangling. It's a lot of duplication to accomplish not very much. My gut feeling would be to omit them, but perhaps something could be hung off theCYW43 instance for manipulating these pins if they're needed.

Note: I've raised a PR for the pins.csv functionality, since I think it's a necessary and sensible addition to the variant supportanyway, that should at least remove a commit from this pile if it's accepted:#17028

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

@GadgetoidGadgetoidGadgetoid left review comments

@dpgeorgedpgeorgedpgeorge left review comments

Assignees
No one assigned
Labels
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

5 participants
@peterharperuk@dpgeorge@Gadgetoid@anecdata@eightycc

[8]ページ先頭

©2009-2025 Movatter.jp