Merge pull request #14688 from MubeenHCLite/CAN_rxinterrupt_fix

Solution for mutex problem in CAN read ISR

Author: 0xc0170

drivers/include/drivers/RawCAN.h

@@ -0,0 +1,94 @@
+/*
+ *  Copyright (C) 2021, STMicroelectronics, All Rights Reserved
+ *  SPDX-License-Identifier: Apache-2.0
+ *
+ *  Licensed under the Apache License, Version 2.0 (the "License"); you may
+ *  not use this file except in compliance with the License.
+ *  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ *  WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+
+#ifndef RAWCAN_H
+#define RAWCAN_H
+
+#include "platform/platform.h"
+#include "drivers/CAN.h"
+
+#if DEVICE_CAN || defined(DOXYGEN_ONLY)
+
+#include "interfaces/InterfaceCAN.h"
+#include "hal/can_api.h"
+#include "platform/Callback.h"
+#include "platform/PlatformMutex.h"
+
+namespace mbed {
+#ifndef FEATURE_EXPERIMENTAL_API
+class RawCAN: public CAN {
+public:
+    /** Creates an unlocked CAN interface connected to specific pins.
+     *
+     *  @param rd read from transmitter
+     *  @param td transmit to transmitter
+     *
+     * Example:
+     * @code
+     * #include "mbed.h"
+     *
+     *
+     * Ticker ticker;
+     * DigitalOut led1(LED1);
+     * DigitalOut led2(LED2);
+     * //The constructor takes in RX, and TX pin respectively.
+     * //These pins, for this example, are defined in mbed_app.json
+     * RawCAN can1(MBED_CONF_APP_CAN1_RD, MBED_CONF_APP_CAN1_TD);
+     * RawCAN can2(MBED_CONF_APP_CAN2_RD, MBED_CONF_APP_CAN2_TD);
+     *
+     * unsigned char counter = 0;
+     *
+     * void send() {
+     *     if(can1.write(CANMessage(1337U, &counter, 1))) {
+     *         printf("Message sent: %d\n", counter);
+     *         counter++;
+     *     }
+     *     led1 = !led1;
+     * }
+     *
+     * int main() {
+     *     ticker.attach(&send, 1);
+     *     CANMessage msg;
+     *     while(1) {
+     *         if(can2.read(msg)) {
+     *             printf("Message received: %d\n\n", msg.data[0]);
+     *             led2 = !led2;
+     *         }
+     *         ThisThread::sleep_for(200);
+     *     }
+     * }
+     *
+     * @endcode
+     */
+
+    /* Note: The can apis are unlocked hence using this when multiple
+    *  threads are accessing a single instance of CAN will lead to
+    *  race conditions, can be used in single threaded CAN.
+    */
+    using CAN::CAN;
+
+
+    // override lock apis to create unlocked CAN
+    void lock() override {};
+    void unlock() override {};
+
+};
+#endif //FEATURE_EXPERIMENTAL_API
+}
+
+#endif
+#endif //RAWCAN_H

mbed.h

@@ -68,6 +68,7 @@
 #include "drivers/I2C.h"
 #include "drivers/I2CSlave.h"
 #include "drivers/CAN.h"
+#include "drivers/RawCAN.h"
 #include "drivers/UnbufferedSerial.h"
 #include "drivers/BufferedSerial.h"
 #include "drivers/FlashIAP.h"

targets/TARGET_STM/README.md

@@ -528,6 +528,63 @@ For application that require optimized maximum performance, the recommendation i
 The SPI DMA transfer support shall be implemented on a case-by-case based on below example
 https://github.com/ABOSTM/mbed-os/tree/I2C_SPI_DMA_IMPLEMENTATION_FOR_STM32L4
 
+### CAN receive interrupt problem due to mutex and resolution
+
+In bxCAN and earlier versions the receive interrupt flags can be cleared only on performing a read operation in ST MCUs
+But can_read() cannot be used in interrupt context as it is gaurded by lock operation and mbed does not allow locks in 
+interrupt context. Hence the Rx interrupt is disabled for a while and read is deferred to thread context, the interrupt is
+enabled on a successful read.
+
+As an other option RawCAN (with unlocked CAN apis) is also available and can be used directly, if only one thread is accessing
+the CAN interface.
+
+While using RxInterrupt with the CAN object the receive ISR callback registered should defer read to thread context.
+A simple example is as shown below:
+
+#include "mbed.h"
+
+Ticker ticker;
+Thread canReadThread;
+
+DigitalOut led1(LED1);
+DigitalOut led2(LED2);
+DigitalOut led3(LED3);
+
+CAN can1(PD_0 ,PD_1);
+
+EventQueue queue(32 * EVENTS_EVENT_SIZE);
+
+int counter = 0xABCD;
+CANMessage msg;
+
+void canRead(){
+        if(can1.read(msg)) {
+            if(msg.id==1100)
+                led2 = !led2;
+            if(msg.id==1102){
+                led3 = !led3;
+            }
+        }
+}
+
+void canISR(){
+    queue.call(canRead);
+    led3 = !led3;
+}
+
+int main() {
+
+    can1.frequency(100000);
+    can1.mode(CAN::Normal);
+
+    can1.attach(canISR, CAN::RxIrq);
+
+    canReadThread.start(callback(&queue, &EventQueue::dispatch_forever));
+
+    while(1) {
+    }
+}
+
 
 ## Mbed OS Wiki pages
 

targets/TARGET_STM/TARGET_STM32F0/objects.h

@@ -156,6 +156,7 @@ struct can_s {
     CAN_HandleTypeDef CanHandle;
     int index;
     int hz;
+    int rxIrqEnabled;
 };
 #endif
 

targets/TARGET_STM/TARGET_STM32F1/objects.h

@@ -145,6 +145,7 @@ struct can_s {
     CAN_HandleTypeDef CanHandle;
     int index;
     int hz;
+    int rxIrqEnabled;
 };
 #endif
 

targets/TARGET_STM/TARGET_STM32F2/objects.h

@@ -154,6 +154,7 @@ struct can_s {
     CAN_HandleTypeDef CanHandle;
     int index;
     int hz;
+    int rxIrqEnabled;
 };
 #endif
 

targets/TARGET_STM/TARGET_STM32F3/objects.h

@@ -143,6 +143,7 @@ struct can_s {
     CAN_HandleTypeDef CanHandle;
     int index;
     int hz;
+    int rxIrqEnabled;
 };
 #endif
 

targets/TARGET_STM/TARGET_STM32F4/objects.h

@@ -158,6 +158,7 @@ struct can_s {
     CAN_HandleTypeDef CanHandle;
     int index;
     int hz;
+    int rxIrqEnabled;
 };
 #endif
 

targets/TARGET_STM/TARGET_STM32F7/objects.h

@@ -167,6 +167,7 @@ struct can_s {
     CAN_HandleTypeDef CanHandle;
     int index;
     int hz;
+    int rxIrqEnabled;
 };
 #endif
 

targets/TARGET_STM/TARGET_STM32L4/objects.h

@@ -157,6 +157,7 @@ struct can_s {
     CAN_HandleTypeDef CanHandle;
     int index;
     int hz;
+    int rxIrqEnabled;
 };
 #endif
 

targets/TARGET_STM/TARGET_STM32L5/objects.h

@@ -157,6 +157,7 @@ struct can_s {
     CAN_HandleTypeDef CanHandle;
     int index;
     int hz;
+    int rxIrqEnabled;
 };
 #endif
 

targets/TARGET_STM/can_api.c

@@ -778,6 +778,7 @@ void can_irq_init(can_t *obj, can_irq_handler handler, uint32_t id)
 {
     irq_handler = handler;
     can_irq_ids[obj->index] = id;
+    obj->rxIrqEnabled = false;
 }
 
 void can_irq_free(can_t *obj)
@@ -787,6 +788,7 @@ void can_irq_free(can_t *obj)
     can->IER &= ~(CAN_IT_FMP0 | CAN_IT_FMP1 | CAN_IT_TME | \
                   CAN_IT_ERR | CAN_IT_EPV | CAN_IT_BOF);
     can_irq_ids[obj->index] = 0;
+    obj->rxIrqEnabled = false;
 }
 
 void can_free(can_t *obj)
@@ -1012,6 +1014,10 @@ int can_read(can_t *obj, CAN_Message *msg, int handle)
     /* Release the FIFO */
     can->RF0R |= CAN_RF0R_RFOM0;
 
+    if(obj->rxIrqEnabled == true) {
+        __HAL_CAN_ENABLE_IT(&obj->CanHandle, CAN_IT_FMP0);
+    }
+
     return 1;
 }
 
@@ -1025,6 +1031,7 @@ void can_reset(can_t *obj)
 
     /* restore registers state as saved in obj context */
     can_registers_init(obj);
+    obj->rxIrqEnabled = false;
 }
 
 unsigned char can_rderror(can_t *obj)
@@ -1177,6 +1184,12 @@ static void can_irq(CANName name, int id)
     tmp1 = __HAL_CAN_MSG_PENDING(&CanHandle, CAN_FIFO0);
     tmp2 = __HAL_CAN_GET_IT_SOURCE(&CanHandle, CAN_IT_FMP0);
 
+    // In legacy can (bxCAN and earlier), reading is the only way to clear rx interrupt. But can_read has mutex locks
+    // since mutexes cannot be used in ISR context, rx interrupt is masked here to temporary disable it
+    // rx interrupts will be unamsked in read operation. reads must be deffered to thread context.
+    // refer to the CAN receive interrupt problem due to mutex and resolution section of README doc.
+    __HAL_CAN_DISABLE_IT(&CanHandle, CAN_IT_FMP0);
+
     if ((tmp1 != 0) && tmp2) {
         irq_handler(can_irq_ids[id], IRQ_RX);
     }
@@ -1276,6 +1289,7 @@ void can_irq_set(can_t *obj, CanIrqType type, uint32_t enable)
                 ier = CAN_IT_FMP0;
                 irq_n = CAN1_IRQ_RX_IRQN;
                 vector = (uint32_t)&CAN1_IRQ_RX_VECT;
+                obj->rxIrqEnabled = true;
                 break;
             case IRQ_TX:
                 ier = CAN_IT_TME;
@@ -1308,6 +1322,7 @@ void can_irq_set(can_t *obj, CanIrqType type, uint32_t enable)
                 ier = CAN_IT_FMP0;
                 irq_n = CAN2_IRQ_RX_IRQN;
                 vector = (uint32_t)&CAN2_IRQ_RX_VECT;
+                obj->rxIrqEnabled = true;
                 break;
             case IRQ_TX:
                 ier = CAN_IT_TME;
@@ -1341,6 +1356,7 @@ void can_irq_set(can_t *obj, CanIrqType type, uint32_t enable)
                 ier = CAN_IT_FMP0;
                 irq_n = CAN3_IRQ_RX_IRQN;
                 vector = (uint32_t)&CAN3_IRQ_RX_VECT;
+                obj->rxIrqEnabled = true;
                 break;
             case IRQ_TX:
                 ier = CAN_IT_TME;