zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.6k stars 6.49k forks source link

posix: can: dynamic interface name #75243

Closed Dalachowsky closed 2 months ago

Dalachowsky commented 3 months ago

Introduction

I'd like to implement a way to decouple CAN interface name from devicetree and provide command line option or a way to dynamically set it.

Problem description

I will explain the problem based on my example. I want to port my device to run on Linux. The device itself uses CAN interface. I'd like to run more than one program instance at once. However if I want to run two instances that use CAN I'd have to compile two separate programs with "can0" and "can1" set in their devicetree.

Proposed change

Add either a command-line option to change CAN interface name or some way to dynamically change it on system init.

Detailed RFC

Easiest solution would be adding a command line option --can-if which would change zephyr,canbus devicetree node's host-interface into whatever would be passed to it.

This solves my problem. However to future-proof it I'd like to propose a way to change it dynamically for each can device defined.

Proposed change (Detailed)

First off I think this feature should be enabled from Kconfig option CONFIG_CAN_NATIVE_LINUX_DYNAMIC_IF_NAME.

The interface name could be set using a function:

int can_native_linux_set_if_name(const struct device *dev, const char *if_name);

This function would have to be called before regular CAN initialization.

Since device configuration is const I think the best way to implement dynamic change would be to make if_name field a double pointer .

struct can_native_linux_config {
    const struct can_driver_config common;
-   const char *if_name;
+   const char **if_name;
};

This way we can make it point to some constant value or to a pointer that could be changed dynamically.

#define CAN_NATIVE_LINUX_INIT(inst)                     \
                                        \
+#if CONFIG_CAN_NATIVE_LINUX_DYNAMIC_IF_NAME \
+static char *can_native_if_name_##inst = DT_INST_PROP(inst, host_interface); \
+#else                                  \
+static const char *can_native_if_name_##inst = DT_INST_PROP(inst, host_interface); \
+#endif                                 \
                                        \
static const struct can_native_linux_config can_native_linux_cfg_##inst = { \
    .common = CAN_DT_DRIVER_CONFIG_INST_GET(inst, 0, 0),            \
-   .if_name = DT_INST_PROP(inst, host_interface),          \
+   .if_name = &can_native_if_name_##inst,              \
};                                      \
                                        \

Dependencies

Changing the if_name into a double pointer would require changing each occurence of it (Which essentially means only the init).

Concerns and Unresolved Questions

Since this is not a big change I do not have concerns regarding it.

Alternatives

One alternative I'm considering is adding the command line option and changing only the zephyr,canbus node. However I think that being able to freely set the name on any of the interfaces would be a lot more beneficial.

henrikbrixandersen commented 3 months ago

I'm fine with introducing a command line option for setting the native CAN IF name, but I fail to see the need for setting it dynamically from an application? Please elaborate on the use-case.

Dalachowsky commented 3 months ago

Ok so let's say we have a device with two can interfaces. We want to port it into linux and run few instances of the device. If we only add a command line option for setting the can interface name then it is impossible. If we introduce the can_native_linux_set_if_name() function then the user can add his own command line option with device index and for example set interface names to "can.INDEX.0" and "can.INDEX.1". Either way if we want to add the command line option the if_name field has to be made dynamic so exporting the can_native_linux_set_if_name() function to the user would be trivial and could be useful for some use-cases.

henrikbrixandersen commented 3 months ago

Ok so let's say we have a device with two can interfaces. We want to port it into linux and run few instances of the device.

Is this for testing purposes? "Port into linux" sounds more like you are trying to use Zephyr as an application framework for writing Linux applications? Hopefully that is not your use-case.

henrikbrixandersen commented 3 months ago

What I normally do when running multiple instances of the same CAN-enabled native_sim binary is to keep them all on the default "zcan0" interface via devicetree, create a virtual CAN network on the Linux host called "zcan0" and, if needed, route that to whichever external host CAN bus is needed for verification.

Dalachowsky commented 3 months ago

We are not using Zephyr to write Linux apps. Our primary target is the device we are working on, but we want to change our system architecture a bit and we are thinking about simulating our device on our main controller to see how it's working.

I must say I was not aware you could reopen the same CAN interface in more than one application. Now I get your point.

How would you prefer it to be implemented? Just a plain override in each initialization function when CONFIG_CAN_NATIVE_LINUX_DYNAMIC_IF_NAME (or more approperiately CONFIG_CAN_NATIVE_LINUX_IF_NAME_FROM_CMD_LINE) is active?

henrikbrixandersen commented 3 months ago

I must say I was not aware you could reopen the same CAN interface in more than one application. Now I get your point.

With this knowledge, do you still need a command line argument for setting the host IF?

Dalachowsky commented 3 months ago

I'd like to have the option to set it on the command line. Also since I have already looked through it and started writing my PR I might as well send it.

henrikbrixandersen commented 2 months ago

Implemented in https://github.com/zephyrproject-rtos/zephyr/pull/75416