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.

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

Workaround for stm32can stuck in error passive state #726

Merged
merged 2 commits into from
Aug 19, 2023
Merged

Workaround for stm32can stuck in error passive state #726

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
84593d6
Cancel messages in the queue when we are in error passive mode.
balazsracz Aug 5, 2023
ed2cdc4
Cancels the transmit queue for both error passive and bus off states.
balazsracz Aug 5, 2023
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
27 changes: 22 additions & 5 deletions src/freertos_drivers/st/Stm32Can.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,19 @@ void Stm32Can::disable()
*/
void Stm32Can::tx_msg()
{
// If we are error passive, the last transmission ended with an error, and
// there are no free TX mailboxes, then we flush the input queue. This is a
// workaround because the STM32 CAN controller can get stuck in this state
// and never get to bus off if the TX attempts end up with no-ack (meaning
// the controller is alone on the bus).
if ((CAN->ESR & CAN_ESR_EPVF) && ((CAN->ESR & CAN_ESR_LEC_Msk) != 0) &&
((CAN->TSR & (CAN_TSR_TME0 | CAN_TSR_TME1 | CAN_TSR_TME2)) == 0))
{
txBuf->flush();
txBuf->signal_condition();
return;
Copy link
Owner

Choose a reason for hiding this comment

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

In this case, should we actually return, or should we allow the data to be queued up in the space created. I think it probably doesn't matter, and the return is okay. The data is likely to get stale anyways.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There are two invariants that we care about and should hold:

  • there shall be space in the txBuf for new frames to enter. We need this so that the stack is happy. This will be true after the flush and signal.
  • there shall be frames in the controller's TX mailboxes (no free tx mailbox). This is necessary so that when the cable is plugged in, the retries will eventually reach a tx success (even though the frame is outdated). The tx success will then exit the condition in two ways: the TEC will decrement and there will be an empty tx mailbox.

Once we are in here, there is no free tx mailbox, so return makes sense.
Another CAN driver (https://github.com/bakerstu/openmrn/blob/master/src/freertos_drivers/ti/TivaCan.cxx#L184) does the same, after flushing the queue they do nothing.

I guess we could rotate in the newer frames into some TX mailbox, but this would be pretty complex logic, and the frames in the TX mailbox would still be outdated when they do get transmitted, just somewhat less outdated.

}

/* see if we can send anything out */
struct can_frame *can_frame;

Expand Down Expand Up @@ -435,31 +448,35 @@ void Stm32Can::sce_interrupt_handler()
/* error interrupt has occured */
CAN->MSR |= CAN_MSR_ERRI; // clear flag

bool cancel_queue = false;

if (CAN->ESR & CAN_ESR_EWGF)
{
/* error warning condition */
state_ = CAN_STATE_BUS_WARNING;
CAN->ESR &= ~CAN_ESR_EWGF;
Copy link
Owner

Choose a reason for hiding this comment

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

Are we sure we want to remove this flag clearing, and the two below?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll add this from the git commit to the PR description:

Removes flag clear statements for error flags in the ESR register, because these flags are not writeable (can not be cleared in software).

}
if (CAN->ESR & CAN_ESR_EPVF)
{
/* error passive condition */
++softErrorCount;
state_ = CAN_STATE_BUS_PASSIVE;
CAN->ESR &= ~CAN_ESR_EPVF;
cancel_queue = true;
}
if (CAN->ESR & CAN_ESR_BOFF)
{
/* bus off error condition */
++busOffCount;
state_ = CAN_STATE_BUS_OFF;
cancel_queue = true;
}
if (cancel_queue)
{
CAN->TSR |= CAN_TSR_ABRQ2;
CAN->TSR |= CAN_TSR_ABRQ1;
CAN->TSR |= CAN_TSR_ABRQ0;
CAN->IER &= ~CAN_IER_TMEIE;
txBuf->flush();
txBuf->signal_condition_from_isr();
++busOffCount;
state_ = CAN_STATE_BUS_OFF;
CAN->ESR &= ~CAN_ESR_BOFF;
}
}
}
Expand Down