drivers: can: replace canbus_np_context with socket_can_context

Move struct socket_can_context to socket_can_context.h to ensure
struct socket_can_context can be included without replying on
socket_can_generic.h.

Change rx thread of canbus to be preemptive to avoid can frame loss.

Update naming of _ENABLE to _ENABLED to follow convention in
Kconfig.native_posix.

Signed-off-by: Pin Loon Lee <pinloon.lee@westonrobot.com>

Author: pllee4-weston

drivers/can/Kconfig.native_posix

@@ -1,10 +1,7 @@
-# Kconfig.native_posix - native_posix CAN configuration options
+# Native posix CAN configuration options
 
-#
 # Copyright (c) 2019 Intel Corporation
-#
 # SPDX-License-Identifier: Apache-2.0
-#
 
 config CAN_NATIVE_POSIX
 	bool "native_posix CAN Driver"
@@ -24,51 +21,51 @@ config CAN_MAX_FILTER
 	  Must be at least the size of concurrent reads.
 
 ## Interface 1
-config CAN_NATIVE_POSIX_INTERFACE_1_ENABLE
+config CAN_NATIVE_POSIX_INTERFACE_1_ENABLED
 	bool "CANBUS interface 1"
-    default y
-   	depends on CAN_NATIVE_POSIX
+	default y
+	depends on CAN_NATIVE_POSIX
 	help
 	  This option enables the CANBUS network interface for Native POSIX board.
 
 config CAN_NATIVE_POSIX_INTERFACE_1_NAME
 	string "CANBUS interface 1 name on Linux side"
-    depends on CAN_NATIVE_POSIX_INTERFACE_1_ENABLE
+	depends on CAN_NATIVE_POSIX_INTERFACE_1_ENABLED
 	default "vcan0"
 	help
 	  This option sets the CANBUS network interface 1 name in host system.
 
 config CAN_NATIVE_POSIX_INTERFACE_1_SOCKETCAN_NAME
 	string "Network device name"
-    depends on CAN_NATIVE_POSIX_INTERFACE_1_ENABLE
+	depends on CAN_NATIVE_POSIX_INTERFACE_1_ENABLED
 	default "SOCKET_CAN_1"
 	help
 	  Name of the network device driver for SocketCAN.
 
 ## Interface 2
-config CAN_NATIVE_POSIX_INTERFACE_2_ENABLE
+config CAN_NATIVE_POSIX_INTERFACE_2_ENABLED
 	bool "CANBUS interface 2"
-    default y
-  	depends on CAN_NATIVE_POSIX
+	default n
+	depends on CAN_NATIVE_POSIX
 	help
 	  This option enables the CANBUS network interface for Native POSIX board.
 
-if CAN_NATIVE_POSIX_INTERFACE_2_ENABLE
+if CAN_NATIVE_POSIX_INTERFACE_2_ENABLED
 
 config CAN_NATIVE_POSIX_INTERFACE_2_NAME
 	string "CANBUS interface 2 name on Linux side"
-    depends on CAN_NATIVE_POSIX_INTERFACE_2_ENABLE
+	depends on CAN_NATIVE_POSIX_INTERFACE_2_ENABLED
 	default "vcan1"
 	help
-	  This option sets the CANBUS network interface 1 name in host system.
+	  This option sets the CANBUS network interface 2 name in host system.
 
 config CAN_NATIVE_POSIX_INTERFACE_2_SOCKETCAN_NAME
 	string "Network device name"
-    depends on CAN_NATIVE_POSIX_INTERFACE_2_ENABLE
+	depends on CAN_NATIVE_POSIX_INTERFACE_2_ENABLED
 	default "SOCKET_CAN_2"
 	help
 	  Name of the network device driver for SocketCAN.
-      
-endif # CAN_NATIVE_POSIX_INTERFACE_2_ENABLE
 
-endif # CAN_NATIVE_POSIX
+endif #CAN_NATIVE_POSIX_INTERFACE_2_ENABLED
+
+endif #CAN_NATIVE_POSIX

drivers/can/can_native_posix.c

@@ -35,32 +35,27 @@ LOG_MODULE_REGISTER(LOG_MODULE_NAME);
 #include <net/socket_can.h>
 
 #include "can_native_posix_priv.h"
+#include "socket_can_context.h"
 
 #define NET_BUF_TIMEOUT K_MSEC(100)
 
-struct canbus_np_context {
-	const struct device *can_dev;
-	struct k_msgq *msgq;
-	struct net_if *iface;
-	const char *if_name;
-
-	int dev_fd;
-	struct can_frame frame;
-};
-
-static int read_data(struct canbus_np_context *ctx, int fd)
+static int read_data(struct socket_can_context *ctx, int fd)
 {
 	struct net_pkt *pkt;
 	int count;
 
-	count = canbus_np_read_data(fd, (void *)(&ctx->frame),
-				    sizeof(ctx->frame));
+	struct can_frame frame;
+
+	count = canbus_np_read_data(fd, (void *)(&frame), sizeof(frame));
+
 	if (count <= 0) {
 		return 0;
 	}
 
 	struct zcan_frame zframe;
-	can_copy_frame_to_zframe(&ctx->frame, &zframe);
+
+	can_copy_frame_to_zframe(&frame, &zframe);
+
 	pkt = net_pkt_rx_alloc_with_buffer(ctx->iface, sizeof(zframe), AF_CAN,
 					   0, NET_BUF_TIMEOUT);
 	if (!pkt) {
@@ -79,7 +74,7 @@ static int read_data(struct canbus_np_context *ctx, int fd)
 	return 0;
 }
 
-static void canbus_np_rx(struct canbus_np_context *ctx)
+static void canbus_np_rx(struct socket_can_context *ctx)
 {
 	LOG_DBG("Starting ZCAN RX thread");
 
@@ -98,7 +93,7 @@ static int canbus_np_send(const struct device *dev,
 			  const struct zcan_frame *msg, k_timeout_t timeout,
 			  can_tx_callback_t callback_isr, void *callback_arg)
 {
-	struct canbus_np_context *ctx = dev->data;
+	struct socket_can_context *ctx = dev->data;
 	int ret = -ENODEV;
 
 	ARG_UNUSED(timeout);
@@ -161,15 +156,15 @@ static const struct can_driver_api can_api_funcs = {
 	.register_state_change_isr = canbus_np_register_state_change_isr
 };
 
-#ifdef CONFIG_CAN_NATIVE_POSIX_INTERFACE_1_ENABLE
+#ifdef CONFIG_CAN_NATIVE_POSIX_INTERFACE_1_ENABLED
 K_KERNEL_STACK_DEFINE(canbus_rx_stack1,
 		      CONFIG_ARCH_POSIX_RECOMMENDED_STACK_SIZE);
 static struct k_thread rx_thread_data1;
-static struct canbus_np_context canbus_context_data1;
+static struct socket_can_context canbus_context_data1;
 
 static int canbus_np1_init(const struct device *dev)
 {
-	struct canbus_np_context *ctx = dev->data;
+	struct socket_can_context *ctx = dev->data;
 
 	ctx->if_name = CONFIG_CAN_NATIVE_POSIX_INTERFACE_1_NAME;
 
@@ -181,7 +176,7 @@ static int canbus_np1_init(const struct device *dev)
 		k_thread_create(&rx_thread_data1, canbus_rx_stack1,
 				K_THREAD_STACK_SIZEOF(canbus_rx_stack1),
 				(k_thread_entry_t)canbus_np_rx, ctx, NULL, NULL,
-				K_PRIO_COOP(14), 0, K_NO_WAIT);
+				K_PRIO_PREEMPT(14), 0, K_NO_WAIT);
 	}
 
 	return 0;
@@ -191,17 +186,17 @@ DEVICE_DT_INST_DEFINE(0, &canbus_np1_init, NULL,
 	    (void *)&canbus_context_data1, NULL,
 	    POST_KERNEL, CONFIG_KERNEL_INIT_PRIORITY_DEVICE,
 	    &can_api_funcs);
-#endif /* CONFIG_CAN_NATIVE_POSIX_INTERFACE_1_ENABLE */
+#endif /* CONFIG_CAN_NATIVE_POSIX_INTERFACE_1_ENABLED */
 
-#ifdef CONFIG_CAN_NATIVE_POSIX_INTERFACE_2_ENABLE
+#ifdef CONFIG_CAN_NATIVE_POSIX_INTERFACE_2_ENABLED
 K_KERNEL_STACK_DEFINE(canbus_rx_stack2,
 		      CONFIG_ARCH_POSIX_RECOMMENDED_STACK_SIZE);
 static struct k_thread rx_thread_data2;
-static struct canbus_np_context canbus_context_data2;
+static struct socket_can_context canbus_context_data2;
 
 static int canbus_np2_init(const struct device *dev)
 {
-	struct canbus_np_context *ctx = dev->data;
+	struct socket_can_context *ctx = dev->data;
 
 	ctx->if_name = CONFIG_CAN_NATIVE_POSIX_INTERFACE_2_NAME;
 
@@ -213,7 +208,7 @@ static int canbus_np2_init(const struct device *dev)
 		k_thread_create(&rx_thread_data2, canbus_rx_stack2,
 				K_THREAD_STACK_SIZEOF(canbus_rx_stack2),
 				(k_thread_entry_t)canbus_np_rx, ctx, NULL, NULL,
-				K_PRIO_COOP(14), 0, K_NO_WAIT);
+				K_PRIO_PREEMPT(14), 0, K_NO_WAIT);
 	}
 
 	return 0;
@@ -223,61 +218,20 @@ DEVICE_DT_INST_DEFINE(1, &canbus_np2_init, NULL,
 	    (void *)&canbus_context_data2, NULL,
 	    POST_KERNEL, CONFIG_KERNEL_INIT_PRIORITY_DEVICE,
 	    &can_api_funcs);
-#endif /* CONFIG_CAN_NATIVE_POSIX_INTERFACE_2_ENABLE */
+#endif /* CONFIG_CAN_NATIVE_POSIX_INTERFACE_2_ENABLED */
 
-#if defined(CONFIG_CAN_NATIVE_POSIX_INTERFACE_1_ENABLE) ||                            \
-	defined(CONFIG_CAN_NATIVE_POSIX_INTERFACE_2_ENABLE)
+#if defined(CONFIG_CAN_NATIVE_POSIX_INTERFACE_1_ENABLED) ||                            \
+	defined(CONFIG_CAN_NATIVE_POSIX_INTERFACE_2_ENABLED)
 
 #if defined(CONFIG_NET_SOCKETS_CAN)
 
-#define SEND_TIMEOUT K_MSEC(100)
-#define BUF_ALLOC_TIMEOUT K_MSEC(50)
+#include "socket_can_generic.h"
 
-static void socket_can_iface_init(struct net_if *iface)
-{
-	const struct device *dev = net_if_get_device(iface);
-	struct canbus_np_context *socket_context = dev->data;
-
-	socket_context->iface = iface;
-
-	LOG_DBG("Init CAN interface %p dev %p", iface, dev);
-}
-
-static void tx_irq_callback(uint32_t error_flags, void *arg)
-{
-	if (error_flags) {
-		LOG_DBG("Callback! error-code: %d", error_flags);
-	}
-}
-
-/* This is called by net_if.c when packet is about to be sent */
-static int socket_can_send(const struct device *dev, struct net_pkt *pkt)
-{
-	struct canbus_np_context *socket_context = dev->data;
-	int ret;
-
-	if (net_pkt_family(pkt) != AF_CAN) {
-		return -EPFNOSUPPORT;
-	}
-
-	ret = can_send(socket_context->can_dev,
-		       (struct zcan_frame *)pkt->frags->data, SEND_TIMEOUT,
-		       tx_irq_callback, NULL);
-	if (ret) {
-		LOG_DBG("Cannot send socket CAN msg (%d)", ret);
-	}
-
-	/* If something went wrong, then we need to return negative value to
-	 * net_if.c:net_if_tx() so that the net_pkt will get released.
-	 */
-	return -ret;
-}
-
-static int socket_can_setsockopt(const struct device *dev, void *obj, int level,
+static int socket_can_np_setsockopt(const struct device *dev, void *obj, int level,
 				 int optname, const void *optval,
 				 socklen_t optlen)
 {
-	struct canbus_np_context *socket_context = dev->data;
+	struct socket_can_context *socket_context = dev->data;
 	struct can_filter filter;
 
 	if (level != SOL_CAN_RAW && optname != CAN_RAW_FILTER) {
@@ -305,26 +259,24 @@ static int socket_can_setsockopt(const struct device *dev, void *obj, int level,
 				    &filter, sizeof(filter));
 }
 
-static void socket_can_close(const struct device *dev, int filter_id)
-{
-	struct canbus_np_context *socket_context = dev->data;
-
-	can_detach(socket_context->can_dev, filter_id);
-}
-
-static struct canbus_api socket_can_api = {
+static struct canbus_api socket_can_np_api = {
 	.iface_api.init = socket_can_iface_init,
 	.send = socket_can_send,
 	.close = socket_can_close,
-	.setsockopt = socket_can_setsockopt,
+	.setsockopt = socket_can_np_setsockopt,
 };
 
-#ifdef CONFIG_CAN_NATIVE_POSIX_INTERFACE_1_ENABLE
-// static struct socket_can_context socket_can_context_1;
+#ifdef CONFIG_CAN_NATIVE_POSIX_INTERFACE_1_ENABLED
+
 static int socket_can_init_1(const struct device *dev)
 {
+	/* To avoid warning, use socket_can_api defined in socket_can_generic.h.
+	 * For native posix, use socket_can_np_api instead.
+	 */
+	(void)socket_can_api;
+
 	const struct device *can_dev = DEVICE_DT_INST_GET(0);
-	struct canbus_np_context *socket_context = dev->data;
+	struct socket_can_context *socket_context = dev->data;
 
 	LOG_DBG("Init socket CAN device %p (%s) for dev %p (%s)", dev,
 		dev->name, can_dev, can_dev->name);
@@ -338,15 +290,21 @@ NET_DEVICE_INIT_INSTANCE(socket_can_native_posix_1,
 			CONFIG_CAN_NATIVE_POSIX_INTERFACE_1_SOCKETCAN_NAME, 0,
 			socket_can_init_1, NULL,
 			&canbus_context_data1, NULL,
-			CONFIG_KERNEL_INIT_PRIORITY_DEVICE, &socket_can_api,
+			CONFIG_KERNEL_INIT_PRIORITY_DEVICE, &socket_can_np_api,
 			CANBUS_RAW_L2, NET_L2_GET_CTX_TYPE(CANBUS_RAW_L2), CAN_MTU);
-#endif /* CONFIG_CAN_NATIVE_POSIX_INTERFACE_1_ENABLE */
+#endif /* CONFIG_CAN_NATIVE_POSIX_INTERFACE_1_ENABLED */
+
+#ifdef CONFIG_CAN_NATIVE_POSIX_INTERFACE_2_ENABLED
 
-#ifdef CONFIG_CAN_NATIVE_POSIX_INTERFACE_2_ENABLE
 static int socket_can_init_2(const struct device *dev)
 {
+	/* To avoid warning, use socket_can_api defined in socket_can_generic.h.
+	 * For native posix, use socket_can_np_api instead.
+	 */
+	(void)socket_can_api;
+
 	const struct device *can_dev = DEVICE_DT_INST_GET(1);
-	struct canbus_np_context *socket_context = dev->data;
+	struct socket_can_context *socket_context = dev->data;
 
 	LOG_DBG("Init socket CAN device %p (%s) for dev %p (%s)", dev,
 		dev->name, can_dev, can_dev->name);
@@ -360,10 +318,10 @@ NET_DEVICE_INIT_INSTANCE(socket_can_native_posix_2,
 			CONFIG_CAN_NATIVE_POSIX_INTERFACE_2_SOCKETCAN_NAME, 1,
 			socket_can_init_2, NULL,
 			&canbus_context_data2, NULL,
-			CONFIG_KERNEL_INIT_PRIORITY_DEVICE, &socket_can_api,
+			CONFIG_KERNEL_INIT_PRIORITY_DEVICE, &socket_can_np_api,
 			CANBUS_RAW_L2, NET_L2_GET_CTX_TYPE(CANBUS_RAW_L2), CAN_MTU);
-#endif /* CONFIG_CAN_NATIVE_POSIX_INTERFACE_2_ENABLE */
+#endif /* CONFIG_CAN_NATIVE_POSIX_INTERFACE_2_ENABLED */
 
 #endif /* CONFIG_NET_SOCKETS_CAN */
 
-#endif /* CONFIG_CAN_NATIVE_POSIX_INTERFACE_1 */
+#endif /* CAN_NATIVE_POSIX_INTERFACE_1_ENABLED || CAN_NATIVE_POSIX_INTERFACE_2_ENABLED */

drivers/can/socket_can_context.h

@@ -0,0 +1,32 @@
+/*
+ * Copyright (c) 2019 Intel Corporation.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ */
+
+/* CANBUS related functions that are generic in all the drivers. */
+
+#include <net/net_pkt.h>
+#include <net/socket_can.h>
+
+#ifndef ZEPHYR_DRIVERS_CAN_SOCKET_CAN_CONTEXT_H_
+#define ZEPHYR_DRIVERS_CAN_SOCKET_CAN_CONTEXT_H_
+
+struct socket_can_context {
+	const struct device *can_dev;
+	struct net_if *iface;
+
+    /* for can on native posix */
+	const char *if_name;
+	int dev_fd;
+
+    /* for can on embedded board */
+	struct k_msgq *msgq;
+
+	/* TODO: remove the thread and push data to net directly from rx isr */
+	k_tid_t rx_tid;
+	struct k_thread rx_thread_data;
+};
+
+#endif /* ZEPHYR_DRIVERS_CAN_SOCKET_CAN_CONTEXT_H_ */