Skip to content
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.

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

drivers: can: rcar: inconsistent behavior when calling can_stop() with pending transmissions #50546

Closed
henrikbrixandersen opened this issue Sep 22, 2022 · 6 comments · Fixed by #57344
Assignees
Labels
area: CAN bug The issue is a bug, or the PR is fixing a bug platform: Renesas R-Car Renesas R-Car priority: low Low impact/importance bug

Comments

@henrikbrixandersen
Copy link
Member

Describe the bug
The Renesas R-Car CAN controller drivers likely exhibits inconsistent behavior when calling can_stop() with pending CAN frame transmissions similar as to what was reported in #50545. I have not been able to find the behavior of the Renesas R-Car CAN driver as I do not have access to the hardware.

To check
Steps to reproduce the behavior:

  1. Disconnect the CAN controller from the CAN bus.
  2. Using the CAN shell, call can start <device>.
  3. Using the CAN shell, call can send <device> 010.
  4. Using the CAN shell, call can stop <device>.
  5. Connect the CAN controller to a CAN bus with at least one other node for ACK'ing frames and monitor any received frames on that node (e.g. use candump on GNU/Linux).
  6. Using the CAN shell, call can start <device>.
  7. Observe the different behavior across the Zephyr CAN drivers as described above.

Expected behavior
The behavior should be the same across the Zephyr CAN drivers. Since some CAN controllers automatically abort any pending transmissions, the common behavior should be to abort any pending transmissions and notify the senders of this.

Impact
Unknown.

Environment (please complete the following information):

Additional context
The can_stop() API was introduced in #50102.

@henrikbrixandersen
Copy link
Member Author

@aaillet Ping

@github-actions
Copy link

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

@github-actions
Copy link

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

@github-actions github-actions bot added the Stale label Feb 27, 2023
@aaillet aaillet removed the Stale label Feb 27, 2023
@aaillet
Copy link
Member

aaillet commented Apr 20, 2023

Hi @henrikbrixandersen,

Sorry for this big delay...

I just verified can_stop() behavior on a random late Feb 2023 sha (dca33a1) on H3ULCB board and got these results:

#Board not plugged to CAN bus
uart:~$ can start can@e6c30000
starting can@e6c30000
uart:~$ can send can@e6c30000 01 DE AD BE EF
enqueuing CAN frame #0 with standard (11-bit) CAN ID 0x001, RTR 0, CAN-FD 0, BRS 0, DLC 4
uart:~$ can stop can@e6c30000 
stopping can@e6c30000
#Pluging board to CAN bus
uart:~$ can start can@e6c30000
starting can@e6c30000
CAN frame #0 successfully sent

I can confirm that the frame has successfully been received by the other CAN device (a computer using CAN USB adapter) as soon as can start() has been called.
I think that this is the behavior we are waiting for but I just wanted you to validate that !

Thank you in advance !

@henrikbrixandersen
Copy link
Member Author

I can confirm that the frame has successfully been received by the other CAN device (a computer using CAN USB adapter) as soon as can start() has been called. I think that this is the behavior we are waiting for but I just wanted you to validate that !

Unfortunately, that is the opposite of the desired behaviour. Calling can_stop() should abort any pending TX frames.

@aaillet
Copy link
Member

aaillet commented Apr 21, 2023

Unfortunately, that is the opposite of the desired behaviour. Calling can_stop() should abort any pending TX frames.

Oh ok, so we want the controller to empty FIFOs and clear everything that has to be cleared !
I'll work on fixing that, thanks for the answer !

aaillet added a commit to iotbzh/zephyr that referenced this issue Apr 27, 2023
Calling for can_stop() method was not emptying FIFOs
and tx_msgq.
We use controller reset mode to empty controller FIFOs.
We are emptying tx_msgq with "ENETDOWN" return code.

Fixes zephyrproject-rtos#50546

Signed-off-by: Aymeric Aillet <aymeric.aillet@iot.bzh>
@henrikbrixandersen henrikbrixandersen changed the title drivers: can: rcar: likely inconsistent behavior when calling can_stop() with pending transmissions drivers: can: rcar: inconsistent behavior when calling can_stop() with pending transmissions Apr 30, 2023
aaillet added a commit to iotbzh/zephyr that referenced this issue May 10, 2023
Calling for can_stop() method was not emptying FIFOs
and tx_msgq.
Resetting TX FIFO to empty it.
Emptying tx_msgq with "ENETDOWN" return code.

Fixes zephyrproject-rtos#50546

Signed-off-by: Aymeric Aillet <aymeric.aillet@iot.bzh>
aaillet added a commit to iotbzh/zephyr that referenced this issue May 10, 2023
Calling for can_stop() method was not emptying FIFOs
and tx_msgq.
Resetting TX FIFO to empty it.
Emptying tx_msgq with "ENETDOWN" return code.

Fixes zephyrproject-rtos#50546

Signed-off-by: Aymeric Aillet <aymeric.aillet@iot.bzh>
aaillet added a commit to iotbzh/zephyr that referenced this issue May 10, 2023
Calling for can_stop() method was not emptying FIFOs
and tx_msgq.
Resetting TX FIFO to empty it.
Emptying tx_msgq with "ENETDOWN" return code.

Fixes zephyrproject-rtos#50546

Signed-off-by: Aymeric Aillet <aymeric.aillet@iot.bzh>
aaillet added a commit to iotbzh/zephyr that referenced this issue May 24, 2023
Calling for can_stop() method was not emptying FIFOs
and tx_msgq.
Resetting TX FIFO to empty it.
Emptying tx_msgq with "ENETDOWN" return code.

Fixes zephyrproject-rtos#50546

Signed-off-by: Aymeric Aillet <aymeric.aillet@iot.bzh>
nashif pushed a commit that referenced this issue May 24, 2023
Calling for can_stop() method was not emptying FIFOs
and tx_msgq.
Resetting TX FIFO to empty it.
Emptying tx_msgq with "ENETDOWN" return code.

Fixes #50546

Signed-off-by: Aymeric Aillet <aymeric.aillet@iot.bzh>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: CAN bug The issue is a bug, or the PR is fixing a bug platform: Renesas R-Car Renesas R-Car priority: low Low impact/importance bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants