Skip to content

Multiple SocketCAN instance support. #51457

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+31 −6
Open

Multiple SocketCAN instance support. #51457

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion boards/arm/rddrone_fmuk66/rddrone_fmuk66.dts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@
zephyr,console = &lpuart0;
zephyr,shell-uart = &lpuart0;
zephyr,uart-pipe = &lpuart0;
zephyr,canbus = &flexcan0;
zephyr,canbus0 = &flexcan0;
zephyr,canbus1 = &flexcan1;
Comment on lines -38 to +39
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are introducing a new naming for chosen nodes here. This will break other subsystems and samples relying on the current naming.

Why are these new chosen nodes needed?

Copy link
Contributor Author

@sumitbatra-nxp sumitbatra-nxp Nov 6, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the current Zephyr code,
I can see some Device Trees have defined CAN nodes with node id's- can##x while others have used flexcan##x
In short, I there is no generic way of creating node id's for a CAN node.
Now, I wanted to create a generic application which uses multiple CAN nodes, so I thought that the application writer (based on the number of CAN PHYs on his board) could create n number of CHOSEN nodes like zephyr_canbus##x.
I know that this will break other subsystems, and I can think more over it, but I feel that the original naming wasn't written with multiple CAN instances in mind.
@henrikbrixandersen I welcome your thoughts on this, please free to correct my understanding.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "node id" you refer to is called the nodelabel, which matches (or at least should match) what the given IP block is called in the SoC datasheet.

For out-of-tree applications, you can add whatever chosen nodes and/or devicetree aliases you need in a devicetree overlay. For in-tree boards, samples, and tests we use the Zephyr-specific chosen nodes as documented here: https://docs.zephyrproject.org/latest/build/dts/api/api.html#zephyr-specific-chosen-nodes The rationale behind only supporting one CAN bus chosen node in Zephyr is that no in-tree code have yet needed to be able to support more than one CAN controller.

If you would like to propose a change to the naming of the Zephyr-specific chosen nodes, please open a separate RFC for this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sumitbatra-nxp ping

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would take back this chosen node change, since I feel it makes more sense to choose just one instance of a subsystem (out of many)

};

leds {
Expand Down
21 changes: 18 additions & 3 deletions drivers/net/canbus.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
*
*/

#define DT_DRV_COMPAT net_canbus

#include <zephyr/net/net_pkt.h>
#include <zephyr/net/canbus.h>
#include <zephyr/net/socketcan.h>
Expand Down Expand Up @@ -158,6 +160,19 @@ static const struct net_canbus_config net_canbus_cfg = {
.can_dev = DEVICE_DT_GET(DT_CHOSEN(zephyr_canbus))
};

NET_DEVICE_INIT(net_canbus, "NET_CANBUS", net_canbus_init, NULL, &net_canbus_ctx, &net_canbus_cfg,
CONFIG_NET_CANBUS_INIT_PRIORITY, &net_canbus_api, CANBUS_RAW_L2,
NET_L2_GET_CTX_TYPE(CANBUS_RAW_L2), CAN_MTU);
#define NET_CANBUS_INIT(n) \
static struct net_canbus_context net_canbus##n##_ctx; \
\
static const struct net_canbus_config net_canbus##n##_cfg = { \
.can_dev = DEVICE_DT_GET(DT_PARENT(DT_DRV_INST(n))) \
}; \
\
NET_DEVICE_DT_INST_DEFINE(n, net_canbus_init, NULL, \
&net_canbus##n##_ctx, \
&net_canbus##n##_cfg, \
CONFIG_NET_CANBUS_INIT_PRIORITY, \
&net_canbus_api, CANBUS_RAW_L2, \
NET_L2_GET_CTX_TYPE(CANBUS_RAW_L2), \
CAN_MTU); \

DT_INST_FOREACH_STATUS_OKAY(NET_CANBUS_INIT)
4 changes: 4 additions & 0 deletions dts/arm/nxp/nxp_k66.dtsi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@
sjw = <1>;
sample-point = <875>;
status = "disabled";
net_canbus1 {
compatible = "net-canbus";
status = "okay";
};
};
};
};
4 changes: 4 additions & 0 deletions dts/arm/nxp/nxp_k6x.dtsi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -506,6 +506,10 @@
sjw = <1>;
sample-point = <875>;
status = "disabled";
net_canbus0 {
compatible = "net-canbus";
status = "okay";
};
};

edma0: dma-controller@40008000 {
Expand Down
5 changes: 3 additions & 2 deletions samples/net/sockets/can/src/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ LOG_MODULE_REGISTER(net_socket_can_sample, LOG_LEVEL_DBG);
#define PRIORITY k_thread_priority_get(k_current_get())
#define STACKSIZE 1024
#define SLEEP_PERIOD K_SECONDS(1)
#define SOCKETCAN_NODE_0 DT_INST(0, net_canbus)

static k_tid_t tx_tid;
static K_THREAD_STACK_DEFINE(tx_stack, STACKSIZE);
Expand Down Expand Up @@ -171,7 +172,7 @@ static int setup_socket(void)

socketcan_from_can_filter(&zfilter, &sfilter);

iface = net_if_get_first_by_type(&NET_L2_GET_NAME(CANBUS_RAW));
iface = net_if_lookup_by_dev(DEVICE_DT_GET(SOCKETCAN_NODE_0));
if (!iface) {
LOG_ERR("No CANBUS network interface found!");
return -ENOENT;
Expand Down Expand Up @@ -256,7 +257,7 @@ static int setup_socket(void)

void main(void)
{
const struct device *dev = DEVICE_DT_GET(DT_CHOSEN(zephyr_canbus));
const struct device *dev = DEVICE_DT_GET(DT_CHOSEN(zephyr_canbus0));
int ret;
int fd;

Expand Down