Hello,
On 21/09/2022 10:36, Marc Kleine-Budde wrote:
> The dev->can.state is set to CAN_STATE_ERROR_ACTIVE, after the device
> has been started. On busy networks the CAN controller might receive
> CAN frame between and go into an error state before the dev->can.state
> is assigned.
>
> Assign dev->can.state before starting the controller to close the race
> window.
>
> Fixes: d08e973a77d1 ("can: gs_usb: Added support for the GS_USB CAN devices")
> Link: https://lore.kernel.org/all/20220920195216.232481-1-mkl@xxxxxxxxxxxxxx
> Signed-off-by: Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx>
FYI, we got a small conflict when merging -net in net-next in the MPTCP
tree due to this patch applied in -net:
5440428b3da6 ("can: gs_usb: gs_can_open(): fix race dev->can.state
condition")
and this one from net-next:
45dfa45f52e6 ("can: gs_usb: add RX and TX hardware timestamp support")
The conflict has been resolved on our side[1] and the resolution we
suggest is attached to this email.
Cheers,
Matt
[1] https://github.com/multipath-tcp/mptcp_net-next/commit/671f1521b564
--
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net
diff --cc drivers/net/can/usb/gs_usb.c
index cc363f1935ce,c1ff3c046d62..5e0d280b0cd3
--- a/drivers/net/can/usb/gs_usb.c
+++ b/drivers/net/can/usb/gs_usb.c
@@@ -956,11 -823,8 +956,12 @@@ static int gs_can_open(struct net_devic
if (ctrlmode & CAN_CTRLMODE_3_SAMPLES)
flags |= GS_CAN_MODE_TRIPLE_SAMPLE;
+ /* if hardware supports timestamps, enable it */
+ if (dev->feature & GS_CAN_FEATURE_HW_TIMESTAMP)
+ flags |= GS_CAN_MODE_HW_TIMESTAMP;
+
/* finally start device */
+ dev->can.state = CAN_STATE_ERROR_ACTIVE;
dm->mode = cpu_to_le32(GS_CAN_MODE_START);
dm->flags = cpu_to_le32(flags);
rc = usb_control_msg(interface_to_usbdev(dev->iface),
@@@ -977,12 -842,6 +979,10 @@@
kfree(dm);
+ /* start polling timestamp */
+ if (dev->feature & GS_CAN_FEATURE_HW_TIMESTAMP)
+ gs_usb_timestamp_init(dev);
+
- dev->can.state = CAN_STATE_ERROR_ACTIVE;
-
parent->active_channels++;
if (!(dev->can.ctrlmode & CAN_CTRLMODE_LISTENONLY))
netif_start_queue(netdev);