[WIP] Drivers: CAN: Implementation of a send timeout for can_send(...)

[alexanderwachter]

This PR implements a timeout for outgoing CAN frames.

At the moment only the stm32 driver implements the new functionality. Other drivers will follow by the end of the week.

It fixes issue #19502

[alexanderwachter]

ping @nixward

[alexanderwachter]

I'm not sure if mb_timeout - time_spent is allowed as timeout parameter here since I assume milliseconds and not use K_MSEC(). Maybe @pabigot can clarify.

[pabigot]

This is exactly the sort of code that can't be automatically converted without risk. The question is whether the driver wants to maintain mb_timeout as a count of milliseconds, or as a value to be passed to the kernel API.

Looking at the way the driver seems to do things, and assuming there's no need for timeouts measured in microseconds or anything finer than a millisecond, I would say that the variables used to pass timeouts should remain integers, documented as being specified in milliseconds.

In this code that would mean that mb_timeout == K_NO_WAIT would become mb_timeout == 0, and the value passed to k_sem_take would be K_MSEC(mb_timeout - time_spent).

[zephyrbot]

All checks passed.

checkpatch (informational only, not a failure)

-:83: WARNING:LONG_LINE: line over 80 characters
#83: FILE: drivers/can/can_mcp2515.c:37:
+static int mcp2515_cmd_bit_modify(struct mcp2515_data *dev_data, u8_t reg_addr, u8_t mask,

-:287: WARNING:LONG_LINE: line over 80 characters
#287: FILE: drivers/can/can_mcp2515.c:437:
+static inline struct mcp2515_tx_cb *mcp2515_get_cb_from_work(struct k_work *work)

-:360: WARNING:LONG_LINE: line over 80 characters
#360: FILE: drivers/can/can_mcp2515.c:504:
+		if (!atomic_test_and_set_bit(dev_data->tx_busy_map, BIT(tx_idx))) {

-:667: WARNING:LONG_LINE: line over 80 characters
#667: FILE: drivers/can/can_mcp2515.c:888:
+	k_delayed_work_init(&dev_data->tx_cb0.to_work, mcp2515_tx0_abort_handler);

-:668: WARNING:LONG_LINE: line over 80 characters
#668: FILE: drivers/can/can_mcp2515.c:889:
+	k_delayed_work_init(&dev_data->tx_cb1.to_work, mcp2515_tx1_abort_handler);

-:669: WARNING:LONG_LINE: line over 80 characters
#669: FILE: drivers/can/can_mcp2515.c:890:
+	k_delayed_work_init(&dev_data->tx_cb2.to_work, mcp2515_tx2_abort_handler);

-:984: WARNING:LONG_LINE: line over 80 characters
#984: FILE: drivers/can/can_mcux_flexcan.c:575:
+			data->tx_cbs[alloc].status = mcux_flexcan_get_err_no(dev);

-:1352: WARNING:LONG_LINE: line over 80 characters
#1352: FILE: drivers/can/can_stm32.c:552:
+	struct can_stm32_data *data = CONTAINER_OF(mb, struct can_stm32_data, mb0);

-:1363: WARNING:LONG_LINE: line over 80 characters
#1363: FILE: drivers/can/can_stm32.c:563:
+	struct can_stm32_data *data = CONTAINER_OF(mb, struct can_stm32_data, mb1);

-:1374: WARNING:LONG_LINE: line over 80 characters
#1374: FILE: drivers/can/can_stm32.c:574:
+	struct can_stm32_data *data = CONTAINER_OF(mb, struct can_stm32_data, mb2);

-:1494: WARNING:LONG_LINE: line over 80 characters
#1494: FILE: drivers/can/can_stm32.c:705:
+		z_add_timeout(&mb->timeout, to_func, z_ms_to_ticks(send_timeout));

- total: 0 errors, 11 warnings, 1947 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

Your patch has style problems, please review.

NOTE: Ignored message types: AVOID_EXTERNS BRACES CONFIG_EXPERIMENTAL CONST_STRUCT DATE_TIME FILE_PATH_CHANGES MINMAX NETWORKING_BLOCK_COMMENT_STYLE PRINTK_WITHOUT_KERN_LEVEL SPLIT_STRING VOLATILE

NOTE: If any of the errors are false positives, please report
      them to the maintainers.

Tip: The bot edits this comment instead of posting a new one, so you can check the comment's history to see earlier messages.

[erwango]

@alexanderwachter any plans for this PR ?

[alexanderwachter]

@erwango the plan is to get it in somewhen. But my priority is on the CAN-FD support at the moment.

[pabigot]

Could be superseded by #24511 if global driver timeouts get traction.

[alexanderwachter]

@pabigot I would love to see a common solution for all APIs. Never the less, this PR implements the mechanisms to cancel a transmission.