QSPI multiple thread access

Report here problems in any of ChibiOS components. This forum is NOT for support.
steved
Posts: 823
Joined: Fri Nov 09, 2012 2:22 pm
Has thanked: 12 times
Been thanked: 135 times

QSPI multiple thread access

Postby steved » Thu Jul 09, 2020 2:04 pm

Maybe this is a matter of design philosophy, but at present it's not possible to do "simultaneous" access of NOR flash on the QSPI bus from multiple threads - an assert is triggered for the second thread.
There is already a mutex to manage access to the QSPI bus, which I would think should be sufficient. However it's after the state checking assert. There could be a relatively long delay for thread 2 if thread 1 has just started an erase cycle, but if that matters it could be managed by the application (or could remain a special case, as now).

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: QSPI multiple thread access

Postby Giovanni » Thu Jul 09, 2020 2:30 pm

Most drivers are not thread safe by design and have explicit acquire/release function.

Reason: grouping, often you want to perform series of operations in an atomic way.

Giovanni

steved
Posts: 823
Joined: Fri Nov 09, 2012 2:22 pm
Has thanked: 12 times
Been thanked: 135 times

Re: QSPI multiple thread access

Postby steved » Thu Jul 09, 2020 4:04 pm

I can see the potential need for grouping, and that is something that usually needs to be managed at an application level.

My point is really specifically for the NOR-flash QSPI driver - why assert when the driver is busy for any reason (other than erase, which potentially muddies the argument), when there is a mutex three lines later in the code which manages the access in a controlled manner? An assert when the driver is uninitialised makes perfect sense; less so otherwise. If the driver returns an error code when an erase is in progress, why not do likewise when the driver is busy for other reasons?

(In my particular situation, I shall have to either modify the driver, which I much prefer not to do, or add an extra mutex to manage access; potentially several mutexes if I use more than the read function).

User avatar
FXCoder
Posts: 384
Joined: Sun Jun 12, 2016 4:10 am
Location: Sydney, Australia
Has thanked: 180 times
Been thanked: 130 times

Re: QSPI multiple thread access

Postby FXCoder » Fri Jul 10, 2020 9:40 am

Hi Steve.
So the assert should let through FLASH_READY, FLASH_READ, FLASH_PGM & FLASH_ERASE.
Then if it is ERASE the status FLASH_BUSY_ERASING gets returned otherwise the calling thread will sit on the Mutex waiting for any current operation to finish.
That approach seems to make sense to me.

Giovanni,
Comments?

--
Bob

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: QSPI multiple thread access

Postby Giovanni » Fri Jul 10, 2020 10:25 am

Hi,

That it the idea, erasing is a polled operation so we cannot lock everything.

Giovanni

steved
Posts: 823
Joined: Fri Nov 09, 2012 2:22 pm
Has thanked: 12 times
Been thanked: 135 times

Re: QSPI multiple thread access

Postby steved » Fri Jul 10, 2020 1:19 pm

Giovanni wrote:That it the idea, erasing is a polled operation so we cannot lock everything.

Surely the mutex just blocks the calling thread?

Bob's summarised things well; although maybe just assert on FLASH_UNINIT and FLASH_STOP. Initially I thought that the routine could block on the mutex for erase as well as other operations, but on reflection its probably better that the caller has the opportunity to decide how to handle a device that's being erased.

I also wonder whether returning an error on FLASH_STOP (rather than asserting) might be more helpful. If it matters, I suppose the caller could check the device state before making the call.

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: QSPI multiple thread access

Postby Giovanni » Fri Jul 10, 2020 2:47 pm

Start and stop states are handled like all other drivers, the application is supposed to know if a driver is started, it should be part of the application "state", no need to check the driver state.

Giovanni

User avatar
FXCoder
Posts: 384
Joined: Sun Jun 12, 2016 4:10 am
Location: Sydney, Australia
Has thanked: 180 times
Been thanked: 130 times

Re: QSPI multiple thread access

Postby FXCoder » Fri Jul 10, 2020 4:04 pm

I haven't had time to look much but I expect the setting of SNOR_SHARED_BUS would also need to come into the equation.
i.e. The mutex may not really be there and an assert in development would be better than a driver/peripheral crash.

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: QSPI multiple thread access

Postby Giovanni » Fri Jul 10, 2020 4:21 pm

I wonder if it is right to have to call (w)spiStart() from outside when SNOR_SHARED_BUS==FALSE.

The most desirable behavior, I think, would be:

SNOR_SHARED_BUS==FALSE -> start the bus on snorStart(), stop it on snorStop.

SNOR_SHARED_BUS==TRUE -> start/stop the bus in each operation, this is for allowing shared SPIs basically.

Perhaps this should be considered a bug because the (W)SPI configuration is passed to the SNOR driver in both cases.

Giovanni

steved
Posts: 823
Joined: Fri Nov 09, 2012 2:22 pm
Has thanked: 12 times
Been thanked: 135 times

Re: QSPI multiple thread access

Postby steved » Fri Jul 10, 2020 8:53 pm

With other peripherals, start() and stop() are called from within the application, rather than within Chibi. This is good for the more straightforward applications (like mine!) where there is no bus/interrupt sharing and power consumption is not critical; just call start() during initialisation, and that's that. Ideally the same principle should IMO be followed; I'm not keen on the idea of starting and stopping the bus with each operation - it's a bit of avoidable overhead. In fact, why would one need to do this, even in a shared configuration? Unless the bus settings change, of course.
The mutex, when enabled, can have the dual purposes of managing access to the bus, and making those accesses thread-safe - as long as checks prior to the mutex don't interfere. (If grouped sequences are needed, they need to be managed at a higher level).

Maybe only enable certain checks if SNOR_SHARED_BUS==FALSE

I suspect this type of issue is going to arise more as Chibi acquires more complex drivers which sit on top of HAL.


Return to “Bug Reports”

Who is online

Users browsing this forum: No registered users and 18 guests