Skip to content
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

Don't check for corrupt heap too early; Fix QSPI timing #1418

Merged
merged 2 commits into from
Dec 26, 2018

Conversation

dhalbert
Copy link
Collaborator

  1. RGB led was being used before stack_init(), so assert_heap_ok() was being called too early. Ignore heap check if stack not allocated yet.

  2. Multiple issues with nrf qspi_flash.c:

  • Two bitfield setting bugs: wrong or missing Pos values.
  • read function didn't return false if error
  • minor cleanup.
  • 32 MHz QSPI frequency was too fast when paired with GD25Q16C, even though that chip is supposed to be good up to 104 MHz. Not clear if it was SPI flash chip's fault or an issue with the peripheral or the driving pins. It's not clear to me that the nrfx QSPI driver sets the pins to high drive even though that's recommended. Made frequency be a max of 16 MHz for any chip.

Symptoms are that MSC gives errors on enumeration. May appear as an unlabeled drive or not at all.

PCA10056 flash chip is only 8MHz, so we didn't see this problem.

@hathach and @ladyada: either or both review.

hathach
hathach previously approved these changes Dec 21, 2018
@dhalbert
Copy link
Collaborator Author

dhalbert commented Dec 21, 2018

@jerryneedell points out this will affect the Particle boards too. It will cut their QSPI speed in half. They are using MX25L which has 133 MHz max, so they'll run at 16 MHz instead of 32 MHz.

I don't know why they appear to work but the Feather does not. I could special-case the Feather but it would be good at some point to understand what's going on.

@jerryneedell
Copy link
Collaborator

jerryneedell commented Dec 21, 2018

I downloaded this PR and built images for the Particle Xenon and Argon boards. Both appear work normally. No obvious impact. Note that neither board was exhibiting any problems without this PR either.

I tested MSC by uploading the full py Bundle /lib and comparing it to the one already in place - no errors.

I also tried changing the setting for the Argon in mpconfigboard.mk from using

SPI_FLASH_FILESYSTEM = 1
to
QSPI_FLASH_FILESYSTEM = 1
and rebuilt. This also works normally on the Argon
Was this a "typo". Should it be changed?
If I am not mistaken the Argon schematic shows that it is wired for QSPI
https://docs.particle.io/datasheets/wi-fi/argon-datasheet/#schematic
The docs say it is "QSPI compatible"
FYI - I accidentally loaded the Xenon build to the Argon and it worked fine except for the missing pin definitions...

@ghost
Copy link

ghost commented Dec 21, 2018

The particle argon and xenon eagle files spec different flash chips. Not clear what is actually on the PCBs or if this matters (flash on both boards works with CP, but I tested with slower speeds than the CP repo).

Would it be feasible to check which chip is actually on the board, rather than getting this information from a configuration file?

@dhalbert
Copy link
Collaborator Author

@bboser We do actually check the chip id. We just list the possible chips for each board in mpconfigboard.mk, and we only include the settings for chips known to be on the board in the firmware for that board. That saves a little space. For many boards, it's just one chip, for some of our boards, we've used more than one chip.

@dhalbert
Copy link
Collaborator Author

I tried setting the pin drives to high and it didn't help. Leaving at 16 MHz for now. Did a new push with some minor cleanup.

@dhalbert
Copy link
Collaborator Author

@ladyada Could you approve this if it's OK with you for now? That will get a working Feather 52840 build into master. Thanks.

@ladyada
Copy link
Member

ladyada commented Dec 26, 2018

checked!

@dhalbert dhalbert merged commit 6d60b81 into adafruit:master Dec 26, 2018
@dhalbert dhalbert deleted the feather52840-rgb-qspi-fixes branch December 26, 2018 22:53
@tannewt
Copy link
Member

tannewt commented Jan 3, 2019

The Argon was SPI because QSPI wasn't working on mine even though it should have. I bet this PR fixes the issue I was hitting.

I also removed the can from one Argon and one Xenon to verify the chip they shipped with.

@jamesmunns
Copy link

Hey all, sorry to revive a dead thread, but I'm bringing up the 52840 Feather for a Rust project and ran into the same 16MHz QSPI limitation.

This was one of the few places I found that mentioned the limitation. Did anyone figure a way to get it working at 32MHz?

@dhalbert
Copy link
Collaborator Author

dhalbert commented Apr 2, 2022

Did anyone figure a way to get it working at 32MHz?

We didn't. I assume your low-level code is completely different, so it's not just some problem in our setup. You could look in https://devzone.nordicsemi.com/ to see if this kind of thing was mentioned by anyone else, or even open a thread about it.

@jamesmunns
Copy link

Okay, thanks! My low level code is indeed different, and I have gotten 32MHz QSPI working with other flash parts using the same low level code, specifically the W25Q128JVS (though with a different nRF52 SOM, the Minew MS88SF2). So my best guess is it is something particular to this flash chip, the SOM in use, or the routing between the SOM and flash chip.

Thankfully 16MHz is "fast enough" for my current application, so I probably won't chase this down any further.

Thanks again for the confirmation!

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.

6 participants