drivers: can: Extend attach_isr with void *arg and pass to the isr cb

This commit extends the CAN api attach_isr function by an argument
that is passed to the isr callback.
With this extension it is possible to distinguish between filter matches
when they use the same callback.

Signed-off-by: Alexander Wachter <alexander.wachter@student.tugraz.at>

drivers/can/mcp2515.c

@@ -177,41 +177,6 @@ const int mcp2515_set_mode(struct device *dev, u8_t mcp2515_mode)
 	return 0;
 }
 
-static int mcp2515_attach(struct device *dev, const struct zcan_filter *filter,
-			  void *response_ptr, u8_t is_type_msgq)
-{
-	struct mcp2515_data *dev_data = DEV_DATA(dev);
-	int filter_idx = 0;
-
-	__ASSERT(response_ptr != NULL, "response_ptr can not be null");
-
-	k_mutex_lock(&dev_data->filter_mutex, K_FOREVER);
-
-	/* find free filter */
-	while ((BIT(filter_idx) & dev_data->filter_usage)
-	       && (filter_idx < CONFIG_CAN_MCP2515_MAX_FILTER)) {
-		filter_idx++;
-	}
-
-	/* setup filter */
-	if (filter_idx < CONFIG_CAN_MCP2515_MAX_FILTER) {
-		dev_data->filter_usage |= BIT(filter_idx);
-		if (is_type_msgq) {
-			dev_data->filter_response_type |= BIT(filter_idx);
-		} else {
-			dev_data->filter_response_type &= ~BIT(filter_idx);
-		}
-		dev_data->filter[filter_idx] = *filter;
-		dev_data->filter_response[filter_idx] = response_ptr;
-	} else {
-		filter_idx = CAN_NO_FREE_FILTER;
-	}
-
-	k_mutex_unlock(&dev_data->filter_mutex);
-
-	return filter_idx;
-}
-
 static int mcp2515_configure(struct device *dev, enum can_mode mode,
 			     u32_t bitrate)
 {
@@ -300,7 +265,7 @@ static int mcp2515_configure(struct device *dev, enum can_mode mode,
 				mcp2515_convert_canmode_to_mcp2515mode(mode));
 }
 
-int mcp2515_send(struct device *dev, const struct zcan_frame *msg,
+static int mcp2515_send(struct device *dev, const struct zcan_frame *msg,
 		 s32_t timeout, can_tx_callback_t callback)
 {
 	struct mcp2515_data *dev_data = DEV_DATA(dev);
@@ -349,19 +314,41 @@ int mcp2515_send(struct device *dev, const struct zcan_frame *msg,
 	return 0;
 }
 
-int mcp2515_attach_msgq(struct device *dev, struct k_msgq *msgq,
+static int mcp2515_attach_isr(struct device *dev, can_rx_callback_t rx_cb,
-			const struct zcan_filter *filter)
+			      void *cb_arg,
+			      const struct zcan_filter *filter)
 {
-	return mcp2515_attach(dev, filter, (void *) msgq, 1);
+	struct mcp2515_data *dev_data = DEV_DATA(dev);
+	int filter_idx = 0;
+
+	__ASSERT(rx_cb != NULL, "response_ptr can not be null");
+
+	k_mutex_lock(&dev_data->filter_mutex, K_FOREVER);
+
+	/* find free filter */
+	while ((BIT(filter_idx) & dev_data->filter_usage)
+	       && (filter_idx < CONFIG_CAN_MCP2515_MAX_FILTER)) {
+		filter_idx++;
+	}
+
+	/* setup filter */
+	if (filter_idx < CONFIG_CAN_MCP2515_MAX_FILTER) {
+		dev_data->filter_usage |= BIT(filter_idx);
+
+		dev_data->filter[filter_idx] = *filter;
+		dev_data->rx_cb[filter_idx] = rx_cb;
+		dev_data->cb_arg[filter_idx] = cb_arg;
+
+	} else {
+		filter_idx = CAN_NO_FREE_FILTER;
+	}
+
+	k_mutex_unlock(&dev_data->filter_mutex);
+
+	return filter_idx;
 }
 
-int mcp2515_attach_isr(struct device *dev, can_rx_callback_t isr,
+static void mcp2515_detach(struct device *dev, int filter_nr)
-		       const struct zcan_filter *filter)
-{
-	return mcp2515_attach(dev, filter, (void *) isr, 0);
-}
-
-void mcp2515_detach(struct device *dev, int filter_nr)
 {
 	struct mcp2515_data *dev_data = DEV_DATA(dev);
 
@@ -398,6 +385,8 @@ static void mcp2515_rx_filter(struct device *dev, struct zcan_frame *msg)
 {
 	struct mcp2515_data *dev_data = DEV_DATA(dev);
 	u8_t filter_idx = 0U;
+	can_rx_callback_t callback;
+	struct zcan_frame tmp_msg;
 
 	k_mutex_lock(&dev_data->filter_mutex, K_FOREVER);
 
@@ -411,17 +400,11 @@ static void mcp2515_rx_filter(struct device *dev, struct zcan_frame *msg)
 			continue; /* filter did not match */
 		}
 
-		if (dev_data->filter_response_type & BIT(filter_idx)) {
+		callback = dev_data->rx_cb[filter_idx];
-			struct k_msgq *msg_q =
+		/*Make a temporary copy in case the user modifies the message*/
-				dev_data->filter_response[filter_idx];
+		tmp_msg = *msg;
 
-			k_msgq_put(msg_q, msg, K_NO_WAIT);
+		callback(&tmp_msg, dev_data->cb_arg[filter_idx]);
-		} else {
-			can_rx_callback_t callback =
-				dev_data->filter_response[filter_idx];
-
-			callback(msg);
-		}
 	}
 
 	k_mutex_unlock(&dev_data->filter_mutex);
@@ -517,7 +500,6 @@ static void mcp2515_int_gpio_callback(struct device *dev,
 static const struct can_driver_api can_api_funcs = {
 	.configure = mcp2515_configure,
 	.send = mcp2515_send,
-	.attach_msgq = mcp2515_attach_msgq,
 	.attach_isr = mcp2515_attach_isr,
 	.detach = mcp2515_detach
 };
@@ -601,8 +583,7 @@ static int mcp2515_init(struct device *dev)
 			NULL, NULL, K_PRIO_COOP(dev_cfg->int_thread_priority),
 			0, K_NO_WAIT);
 
-	(void)memset(dev_data->filter_response, 0,
+	(void)memset(dev_data->rx_cb, 0, sizeof(dev_data->rx_cb));
-		     sizeof(dev_data->filter_response));
 	(void)memset(dev_data->filter, 0, sizeof(dev_data->filter));
 
 	ret = mcp2515_configure(dev, CAN_NORMAL_MODE, dev_cfg->bus_speed);
@@ -622,7 +603,6 @@ static struct mcp2515_data mcp2515_data_1 = {
 	.tx_cb[2].cb = NULL,
 	.tx_busy_map = 0U,
 	.filter_usage = 0U,
-	.filter_response_type = 0U,
 };
 
 static const struct mcp2515_config mcp2515_config_1 = {

drivers/can/mcp2515.h

@@ -46,8 +46,8 @@ struct mcp2515_data {
 	/* filter data */
 	struct k_mutex filter_mutex;
 	u32_t filter_usage;
-	u32_t filter_response_type;
+	can_rx_callback_t rx_cb[CONFIG_CAN_MCP2515_MAX_FILTER];
-	void *filter_response[CONFIG_CAN_MCP2515_MAX_FILTER];
+	void *cb_arg[CONFIG_CAN_MCP2515_MAX_FILTER];
 	struct zcan_filter filter[CONFIG_CAN_MCP2515_MAX_FILTER];
 };
 

drivers/can/stm32_can.c

@@ -81,7 +81,7 @@ void can_stm32_rx_isr_handler(CAN_TypeDef *can, struct can_stm32_data *data)
 			} else {
 				can_rx_callback_t callback =
 					data->rx_response[filter_match_index];
-				callback(&msg);
+				callback(&msg, data->cb_arg[filter_match_index]);
 			}
 		}
 
@@ -302,6 +302,8 @@ static int can_stm32_init(struct device *dev)
 	data->filter_usage = (1ULL << CAN_MAX_NUMBER_OF_FILTERS) - 1ULL;
 	(void)memset(data->rx_response, 0,
 		     sizeof(void *) * CONFIG_CAN_MAX_FILTER);
+	(void)memset(data->cb_arg, 0,
+		     sizeof(void *) * CONFIG_CAN_MAX_FILTER);
 	data->response_type = 0U;
 
 	clock = device_get_binding(STM32_CLOCK_CONTROL_NAME);
@@ -713,6 +715,10 @@ static inline int can_stm32_set_filter(const struct zcan_filter *filter,
 						start_index,
 						shift_width);
 
+			res |= can_stm32_shift_arr(device_data->cb_arg,
+						   start_index,
+						   shift_width);
+
 			if (filter_index_new >= CONFIG_CAN_MAX_FILTER || res) {
 				LOG_INF("No space for a new filter!");
 				filter_nr = CAN_NO_FREE_FILTER;
@@ -748,6 +754,7 @@ done:
 
 
 static inline int can_stm32_attach(struct device *dev, void *response_ptr,
+				   void *cb_arg,
 				   const struct zcan_filter *filter,
 				   int *filter_index)
 {
@@ -760,6 +767,7 @@ static inline int can_stm32_attach(struct device *dev, void *response_ptr,
 	filter_nr = can_stm32_set_filter(filter, data, can, &filter_index_tmp);
 	if (filter_nr != CAN_NO_FREE_FILTER) {
 		data->rx_response[filter_index_tmp] = response_ptr;
+		data->cb_arg[filter_index_tmp] = cb_arg;
 	}
 
 	*filter_index = filter_index_tmp;
@@ -774,13 +782,14 @@ int can_stm32_attach_msgq(struct device *dev, struct k_msgq *msgq,
 	struct can_stm32_data *data = DEV_DATA(dev);
 
 	k_mutex_lock(&data->set_filter_mutex, K_FOREVER);
-	filter_nr = can_stm32_attach(dev, msgq, filter, &filter_index);
+	filter_nr = can_stm32_attach(dev, msgq, NULL, filter, &filter_index);
 	data->response_type |= (1ULL << filter_index);
 	k_mutex_unlock(&data->set_filter_mutex);
 	return filter_nr;
 }
 
 int can_stm32_attach_isr(struct device *dev, can_rx_callback_t isr,
+			 void *cb_arg,
 			 const struct zcan_filter *filter)
 {
 	struct can_stm32_data *data = DEV_DATA(dev);
@@ -788,7 +797,7 @@ int can_stm32_attach_isr(struct device *dev, can_rx_callback_t isr,
 	int filter_index;
 
 	k_mutex_lock(&data->set_filter_mutex, K_FOREVER);
-	filter_nr = can_stm32_attach(dev, isr, filter, &filter_index);
+	filter_nr = can_stm32_attach(dev, isr, cb_arg, filter, &filter_index);
 	data->response_type &= ~(1ULL << filter_index);
 	k_mutex_unlock(&data->set_filter_mutex);
 	return filter_nr;
@@ -839,6 +848,7 @@ void can_stm32_detach(struct device *dev, int filter_nr)
 
 	can->FMR &= ~(CAN_FMR_FINIT);
 	data->rx_response[filter_index] = NULL;
+	data->cb_arg[filter_index] = NULL;
 	data->response_type &= ~(1ULL << filter_index);
 
 	k_mutex_unlock(&data->set_filter_mutex);

drivers/can/stm32_can.h

@@ -64,6 +64,7 @@ struct can_stm32_data {
 	u64_t filter_usage;
 	u64_t response_type;
 	void *rx_response[CONFIG_CAN_MAX_FILTER];
+	void *cb_arg[CONFIG_CAN_MAX_FILTER];
 };
 
 struct can_stm32_config {

include/can.h

@@ -218,9 +218,10 @@ typedef void (*can_tx_callback_t)(u32_t error_flags);
  * @brief Define the application callback handler function signature
  *        for receiving.
  *
- * @param received message
+ * @param msg received message
+ * @param arg argument that was passed when the filter was attached
  */
-typedef void (*can_rx_callback_t)(struct zcan_frame *msg);
+typedef void (*can_rx_callback_t)(struct zcan_frame *msg, void *arg);
 
 typedef int (*can_configure_t)(struct device *dev, enum can_mode mode,
 				u32_t bitrate);
@@ -233,6 +234,7 @@ typedef int (*can_attach_msgq_t)(struct device *dev, struct k_msgq *msg_q,
 				 const struct zcan_filter *filter);
 
 typedef int (*can_attach_isr_t)(struct device *dev, can_rx_callback_t isr,
+				void *callback_arg,
 				const struct zcan_filter *filter);
 
 typedef void (*can_detach_t)(struct device *dev, int filter_id);
@@ -361,22 +363,24 @@ static inline int z_impl_can_attach_msgq(struct device *dev,
  * is hardware dependent.
  * A callback function can be attached to more than one filter.
  * *
- * @param dev    Pointer to the device structure for the driver instance.
+ * @param dev          Pointer to the device structure for the driver instance.
- * @param isr    Callback function pointer.
+ * @param isr          Callback function pointer.
- * @param filter Pointer to a zcan_filter structure defining the id
+ * @param callback_arg This will be passed whenever the isr is called.
- *               filtering.
+ * @param filter       Pointer to a zcan_filter structure defining the id
+ *                     filtering.
  *
  * @retval filter id on success.
  * @retval CAN_NO_FREE_FILTER if there is no filter left.
  */
 static inline int can_attach_isr(struct device *dev,
 				       can_rx_callback_t isr,
+				       void *callback_arg,
 				       const struct zcan_filter *filter)
 {
 	const struct can_driver_api *api =
 		(const struct can_driver_api *)dev->driver_api;
 
-	return api->attach_isr(dev, isr, filter);
+	return api->attach_isr(dev, isr, callback_arg, filter);
 }
 
 /**

samples/drivers/CAN/src/main.c

@@ -172,10 +172,12 @@ void led_thread(void *msgq, void *can_dev_param, void *gpio_dev_param)
 	}
 }
 
-void rx_button_isr(struct zcan_frame *msg)
+void rx_button_isr(struct zcan_frame *msg, void *arg)
 {
 	u16_t cnt = msg->data[0] | (msg->data[1] << 8);
 
+	ARG_UNUSED(arg);
+
 	printk("Button pressed %d times\n", cnt);
 }
 
@@ -235,7 +237,7 @@ void main(void)
 		printk("Error enabling callback!\n");
 	}
 
-	ret = can_attach_isr(can_dev, rx_button_isr, &filter);
+	ret = can_attach_isr(can_dev, rx_button_isr, 0, &filter);
 	if (ret == CAN_NO_FREE_FILTER) {
 		printk("Error, no filter available!\n");
 		return;

tests/drivers/can/api/src/main.c

@@ -146,27 +146,31 @@ static void tx_isr(u32_t error_flags)
 
 }
 
-static void rx_std_isr(struct zcan_frame *msg)
+static void rx_std_isr(struct zcan_frame *msg, void *arg)
 {
 	check_msg(msg, &test_std_msg, 0);
+	zassert_equal_ptr(arg, &test_std_filter, "arg does not match");
 	k_sem_give(&rx_isr_sem);
 }
 
-static void rx_std_mask_isr(struct zcan_frame *msg)
+static void rx_std_mask_isr(struct zcan_frame *msg, void *arg)
 {
 	check_msg(msg, &test_std_msg, 0x0F);
+	zassert_equal_ptr(arg, &test_std_masked_filter, "arg does not match");
 	k_sem_give(&rx_isr_sem);
 }
 
-static void rx_ext_isr(struct zcan_frame *msg)
+static void rx_ext_isr(struct zcan_frame *msg, void *arg)
 {
 	check_msg(msg, &test_ext_msg, 0);
+	zassert_equal_ptr(arg, &test_ext_filter, "arg does not match");
 	k_sem_give(&rx_isr_sem);
 }
 
-static void rx_ext_mask_isr(struct zcan_frame *msg)
+static void rx_ext_mask_isr(struct zcan_frame *msg, void *arg)
 {
 	check_msg(msg, &test_ext_msg, 0x0F);
+	zassert_equal_ptr(arg, &test_ext_masked_filter, "arg does not match");
 	k_sem_give(&rx_isr_sem);
 }
 
@@ -212,15 +216,19 @@ static inline int attach_isr(struct device *can_dev,
 
 	if (filter->id_type == CAN_STANDARD_IDENTIFIER) {
 		if (filter->std_id_mask == CAN_STD_ID_MASK) {
-			filter_id = can_attach_isr(can_dev, rx_std_isr, filter);
+			filter_id = can_attach_isr(can_dev, rx_std_isr,
+						   (void *)filter, filter);
 		} else {
-			filter_id = can_attach_isr(can_dev, rx_std_mask_isr, filter);
+			filter_id = can_attach_isr(can_dev, rx_std_mask_isr,
+						   (void *)filter, filter);
 		}
 	} else {
 		if (filter->ext_id_mask == CAN_EXT_ID_MASK) {
-			filter_id = can_attach_isr(can_dev, rx_ext_isr, filter);
+			filter_id = can_attach_isr(can_dev, rx_ext_isr,
+						   (void *)filter, filter);
 		} else {
-			filter_id = can_attach_isr(can_dev, rx_ext_mask_isr, filter);
+			filter_id = can_attach_isr(can_dev, rx_ext_mask_isr,
+						   (void *)filter, filter);
 		}
 	}