Bug in the m25qStop() function in m25q.c driver Topic is solved

Report here problems in any of ChibiOS components. This forum is NOT for support.
wpaul
Posts: 16
Joined: Wed Oct 12, 2016 10:06 pm
Been thanked: 3 times

Bug in the m25qStop() function in m25q.c driver  Topic is solved

Postby wpaul » Mon Sep 24, 2018 6:13 pm

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

wpaul
Posts: 16
Joined: Wed Oct 12, 2016 10:06 pm
Been thanked: 3 times

Re: Bug in the m25qStop() function in m25q.c driver

Postby wpaul » Mon Sep 24, 2018 10:08 pm

Oops, I just realized: instead of version 18.1.2, that should be 18.2.1. My bad.

-Bill

User avatar
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

Postby Giovanni » Tue Sep 25, 2018 12:17 pm

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

User avatar
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

Postby Giovanni » Thu Nov 15, 2018 11:09 am

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


Return to “Bug Reports”

Who is online

Users browsing this forum: No registered users and 20 guests