Hello:
This weekend I wrote a QSPI driver for ChibiOS for a project I'm working on. I got things to work, but along the way I discovered a bug in the m25qStop() function which I thought I should report.
Relevant details:
OS version: ChibiOS 18.1.2 release
Platform: Nordic nRF52840 DK development board
MCU: Nordic nRF52840 (1MB flash, 256KB RAM)
CPU: ARM Cortex-M4 running at 64MHz
QSPI flash: Macronix MX25R6435F
Compiler: GCC 7.1.0
Driver modules: hal_qspi, hal_flash, m25q
I'm using a modified version of the NRF52 platform support code from the ChibiOS community repo. (The NRF52840 is very similar to the NRF52832; it just has more flash/RAM and some added peripherals.) While I used the 18.1.2 release tarball for my project, the bug seems to be present in the latest code in the git repo too:
https://github.com/ChibiOS/ChibiOS/blob ... ron/m25q.c
Here we can see that the m25qStop() routine looks like this:
[...]
void m25qStop(M25QDriver *devp) {
osalDbgCheck(devp != NULL);
osalDbgAssert(devp->state != FLASH_UNINIT, "invalid state");
if (devp->state != FLASH_STOP) {
/* Bus acquisition.*/
jesd216_bus_acquire(devp->config->busp, devp->config->buscfg);
/* Stopping bus device.*/
jesd216_stop(devp->config->busp);
/* Deleting current configuration.*/
devp->config = NULL;
/* Driver stopped.*/
devp->state = FLASH_STOP;
/* Bus release.*/
jesd216_bus_release(devp->config->busp);
}
}
[...]
The bug is pretty simple: the devp->config pointer is set to NULL, but then two lines later it's dereferenced again in the call to jesd216_bus_release(). At least on the ARM processor in the Nordic MCU, this triggers a hard fault trap. In my local sources I corrected this by moving the "Deleting current configuration" part after the "Bus release" part.
Aside from this, things worked pretty much as expected. Note that I'm aware that the m25q driver is for Micron parts and my board has a Macronix part. But the m25q driver seems to work well enough that all the functionality I need works (read/erase/program/memmap).
-Bill
Bug in the m25qStop() function in m25q.c driver Topic is solved
Re: Bug in the m25qStop() function in m25q.c driver
Oops, I just realized: instead of version 18.1.2, that should be 18.2.1. My bad.
-Bill
-Bill
- Giovanni
- Site Admin
- Posts: 14444
- Joined: Wed May 27, 2009 8:48 am
- Location: Salerno, Italy
- Has thanked: 1074 times
- Been thanked: 921 times
- Contact:
Re: Bug in the m25qStop() function in m25q.c driver
Hi,
Note that under \os\hal\lib\complex\m25q there is a new version of the driver that allows to support multiple vendors, it will replace the current one. Probably it has the same bug, have to verify.
Giovanni
Note that under \os\hal\lib\complex\m25q there is a new version of the driver that allows to support multiple vendors, it will replace the current one. Probably it has the same bug, have to verify.
Giovanni
- Giovanni
- Site Admin
- Posts: 14444
- Joined: Wed May 27, 2009 8:48 am
- Location: Salerno, Italy
- Has thanked: 1074 times
- Been thanked: 921 times
- Contact:
Re: Bug in the m25qStop() function in m25q.c driver
Fixed as bug #988.
Note that the whole flash infrastructure has been modified in trunk code, you may want to give it a try.
Giovanni
Note that the whole flash infrastructure has been modified in trunk code, you may want to give it a try.
Giovanni
Who is online
Users browsing this forum: No registered users and 20 guests