Questions surrounding __HAL_LOCK

[matthew2399]

Posted on April 26, 2016 at 00:12

I’m an engineer at Fluke, and we’re using an STM32F4xx seriesmicrocontroller (together with its HAL drivers) as the basis for a newproduct. The HAL contains a “locking�? mechanism, where eachsubsystem—I²C, USB, UART, and so on—has a “locking object�?. My team hasbeen working on the assumption that this mechanism’s goal is to providesome mutual exclusion for memory-mapped hardware registers and relatedstate so that, for example, an interrupt doesn’t modify the same hardware state being manipulated in the main “thread�? of exectution.

What’s confused us, however, is how this is done. The driver code found in stm32f4xx_hal_def.h defines the locking mechanism as follows:[1]

typedef
enum
{
HAL_UNLOCKED = 0x00U,
HAL_LOCKED = 0x01U
} HAL_LockTypeDef;
#if (USE_RTOS == 1)
/* Reserved for future use */
#error ''USE_RTOS should be 0 in the current HAL release''
#else
#define __HAL_LOCK(__HANDLE__) \
do
{ \
if
((__HANDLE__)->Lock == HAL_LOCKED) \
{ \
return
HAL_BUSY; \
} \
else
\
{ \
(__HANDLE__)->Lock = HAL_LOCKED; \
} \
}
while
(0)
#define __HAL_UNLOCK(__HANDLE__) \
do
{ \
(__HANDLE__)->Lock = HAL_UNLOCKED; \
}
while
(0)
#endif /* USE_RTOS */

In summary, “locking�? consists of setting a flag, or returning an error(indicating the system is busy) when the flag is already set.“Unlocking�? consists of unconditionally clearing the flag. What throws us is the immediate return of HAL_BUSY in the case ofcontention. If a HAL lock is providing mutual exclusion between the mainthread and an interrupt handler, what should be done if the interrupt isthe second caller of __HAL_LOCK and gets HAL_BUSY? An interrupt cannotpause its execution and start again when the lock is no longercontended, like a userspace thread could in a multitasking OS. (The bestone can do is have the interrupt schedule the work for some later time,but this is not always feasible.) Additionally, many read-modify-write sequences (such as ORing bits) are performed on hardware registers without any additional protection besides these ''locks''. What is to keep an interrupt (or, if one is running an RTOS, another task) from executing in the middle of one of these sequences? Based on these observations, we tried redefining __HAL_LOCK and__HAL_UNLOCK to be a global “interrupt lock�?, i.e., the former disablesinterrupts and the latter re-enables them. For cases where several callsto these functions nest (whether intentionally or unintentionally), wekeep track of a nesting count. Unfortunately this doesn’t work well withthe I²C driver, since it contains several code paths where, on atimeout, __HAL_UNLOCK is called twice for a single call to __HAL_LOCK,unbalancing our nesting count to disastrous effect. How is the __HAL_LOCK/__HAL_UNLOCK system meant to work? It would seemwe’re not using it according to its design goals. [1] All code is taken from STM32CubeF4, version 1.0 #hal #stm32f4

[markb]

Posted on April 26, 2016 at 09:50 Hi Matt, It has been mentioned several times before here, the HAL_LOCK mechanism is badly broken. As you say, it doesn't work for interrupt handlers. What I have done for UART and CAN is copy code out of the HAL library into my own functions so that it can be called from an interrupt, for example, my HAL_CAN_RxCpltCallback(CAN_HandleTypeDef* hcan) function contains the following:

// ideally, we should be able to call HAL_CAN_Receive_IT() here to set up for another
// receive but the API is flawed because that function will fail if HAL_CAN_Transmit*()
// had already locked the handle when the receive interrupt occurred - so we do what
// HAL_CAN_Receive_IT() would do
if(hcan->State == HAL_CAN_STATE_BUSY_TX)
hcan->State = HAL_CAN_STATE_BUSY_TX_RX;
else {
hcan->State = HAL_CAN_STATE_BUSY_RX;
/* Set CAN error code to none */
hcan->ErrorCode = HAL_CAN_ERROR_NONE;
/* Enable Error warning Interrupt */
__HAL_CAN_ENABLE_IT(hcan, CAN_IT_EWG);
/* Enable Error passive Interrupt */
__HAL_CAN_ENABLE_IT(hcan, CAN_IT_EPV);
/* Enable Bus-off Interrupt */
__HAL_CAN_ENABLE_IT(hcan, CAN_IT_BOF);
/* Enable Last error code Interrupt */
__HAL_CAN_ENABLE_IT(hcan, CAN_IT_LEC);
/* Enable Error Interrupt */
__HAL_CAN_ENABLE_IT(hcan, CAN_IT_ERR);
}
// Enable FIFO 0 message pending Interrupt
__HAL_CAN_ENABLE_IT(hcan, CAN_IT_FMP0);

Note that I am not using interrupts for CAN tx, just receive. I hope this helps. Cheers, Mark

[Walid FTITI_O]

Posted on April 26, 2016 at 13:00

Hi Matt Kline,

Thanks for your feedback and contribution. It is a known limitation and a new LOCK and UNLOCK process under preparation. Sorry for the inconvenience that may bring -Hannibal-

[taraben]

Posted on May 01, 2016 at 17:32

Hello Friends,

this

__HAL_LOCK(__HANDLE__)

implementation is ... let's call it NAIVE. we can only trust the LOCK when calling from one main thread. If we call LOCK from main thread and from interrupt we have the well known data conflict. So we need to make sure never mixing LOCK from main and from ISR. Especially when we use HAL functions in Callback functions from HAL_***_IT or HAL_***_DMA we need kind of thread safety. One simple solution would be to use the ARM exclusive LDREX STREX here. Something similar as below

#define __HAL_LOCK(__HANDLE__) \
do
{ \
if
((__LDREX(&((__HANDLE__)->Lock) == HAL_LOCKED) \
{ \
return
HAL_BUSY; \
} \
else
\
{ \
if(__STREX(HAL_LOCKED, &((__HANDLE__)->Lock)) != 0) return HAL_BUSY; \
} \
}
while
(0)

Please ST comment and implement some more failsave version of HAL_LOCK. Regards, Adib. --

[Tesla DeLorean]

Posted on May 01, 2016 at 18:05

let's call it NAIVE

There's a lot of stuff in there that's ill conceived, and that's been apparent to anyone with any multi-threaded and multi-tasking experience, for several years. 

[richardst9]

Posted on September 24, 2016 at 09:25

It sounds like we are saying that that we cannot call any ST library code that invokes HAL_LOCK from within an interrupt routine.

In my USB case, I need to call HAL_PCD_EP_Receive() after unpacking data from the USB buffer so that more data can be received. In order not to massively impact the data transfer rate, I should do this at the end of the USB Receive Interrupt routine. But if a USB Receive Interrupt happened at a time that HAL_LOCK was ''locked'' (e.g. my main code was queueing some data for transmit), then the HAL_PCD_EP_Receive will not happen (i.e. will be HAL_LOCKed out) and the USB bus is stalled. What is the recommended workaround for this? When can we expect a fix?

[richardst9]

Posted on September 25, 2016 at 19:09

>we cannot call any ST library code that invokes HAL_LOCK from within an interrupt routine.

Might we go so far as to say the the USB Receive Interrupt is a pointless and futile waste of time? If all we can do is set a flag in the interrupt and poll for it in our User thread, then why would we bother? Why wouldn't we just poll the USB peripheral instead. There must be something that I'm missing here, because the scheme that seems to be forced upon developers by the HAL libraries appears, at best, poorly considered, and more realistically...well you can see where I'm going with this. Richard

[Elijah Brown]

Posted on February 07, 2017 at 01:37

I have exactly this problem with USB receive.  The HAL_Lock logic is hopelessly broken and it's littered throughout the ST libraries causing goodness knows what other race conditions.  I am also interested in a solution...

[Selso LIBERADO]

Posted on March 10, 2017 at 11:51

I've just check in the STM32F4Cube V1.14 and the code has not changed.

Yes the actual code is not thread-safe.

I think your solution as a good one, we could also use a SWI if we can implement it on these MCU (I come from ARM7TDMI world .

But your suggestion is the best one.

Is there a way to create issue, or suggest patch on ST code ? I did not find out where to do it, I also found an I2C API error and would like to suggest a fix.

[Imen.D]

Posted on May 18, 2017 at 12:33

Hello

LIBERADO.Selso

,

You can post your issue/patch in this STM32 forum.

ST moderators will take in charge your request.

Thanks

Imen