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

SN32: USB packet drops hotfix #49

Open
wants to merge 3 commits into
base: sn32_master
Choose a base branch
from
Open

SN32: USB packet drops hotfix #49

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
de57cb4
Revert "Fixed the sn32F2xx usb packet drops"
Jpe230 May 17, 2022
11e3bb9
Merge 2abb160c
Jpe230 May 17, 2022
1366c95
USB hotfix
Jpe230 Jul 7, 2022
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
97 changes: 84 additions & 13 deletions os/hal/ports/SN32/LLD/SN32F2xx/USB/hal_usb_lld.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,13 +209,34 @@ static void sn32_usb_write_fifo(usbep_t ep, const uint8_t *buf, size_t sz, bool
*/
static void usb_lld_serve_interrupt(USBDriver *usbp) {
uint32_t iwIntFlag;

/* Get Interrupt Status and clear immediately. */
iwIntFlag = SN32_USB->INSTS;
/* Keep only PRESETUP & ERR_SETUP & ACK and NAK flags. */
SN32_USB->INSTSC = ~(mskEP0_PRESETUP|mskERR_SETUP
|mskEP6_ACK|mskEP5_ACK|mskEP4_ACK|mskEP3_ACK|mskEP2_ACK|mskEP1_ACK
|mskEP6_NAK|mskEP5_NAK|mskEP4_NAK|mskEP3_NAK|mskEP2_NAK|mskEP1_NAK);

/* Keep only SETUP, TIMEOUT and reserved flags. */
Copy link

Choose a reason for hiding this comment

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

Either the comment or the clearing is wrong, because you clear the setup flag.
Also, 27 and 28 are reserved, so why do you clear those (They are read only, so no real clearing takes places)?

SN32_USB->INSTSC = (
mskEP1_NAK |
mskEP2_NAK |
mskEP3_NAK |
mskEP4_NAK |
mskEP1_ACK |
mskEP2_ACK |
mskEP3_ACK |
mskEP4_ACK |
mskEP0_OUT_STALL |
mskEP0_IN_STALL |
mskEP0_OUT |
mskEP0_IN |
mskEP0_SETUP |
mskEP0_PRESETUP |
mskBUS_WAKEUP |
mskUSB_SOF |
mskBUS_RESUME |
(0x1<<27) |
(0x1<<28) |
mskBUS_SUSPEND |
mskBUS_RESET
);


if (iwIntFlag == 0) {
//@20160902 add for EMC protection
Expand Down Expand Up @@ -353,10 +374,11 @@ static void usb_lld_serve_interrupt(USBDriver *usbp) {
SN32_USB->INSTSC = (mskEP0_OUT_STALL);
}
}

/////////////////////////////////////////////////
/* Device Status Interrupt (EPnACK) */
/////////////////////////////////////////////////
else if (iwIntFlag & (mskEP6_ACK|mskEP5_ACK|mskEP4_ACK|mskEP3_ACK|mskEP2_ACK|mskEP1_ACK)) {
if (iwIntFlag & (mskEP6_ACK|mskEP5_ACK|mskEP4_ACK|mskEP3_ACK|mskEP2_ACK|mskEP1_ACK)) {
// Determine the interrupting endpoint, direction, and clear the interrupt flag
for(usbep_t ep = 1; ep <= USB_MAX_ENDPOINTS; ep++) {
if (iwIntFlag & mskEPn_ACK(ep)){
Expand All @@ -365,7 +387,7 @@ static void usb_lld_serve_interrupt(USBDriver *usbp) {
}
}
}
else if (iwIntFlag & (mskEP6_NAK|mskEP5_NAK|mskEP4_NAK|mskEP3_NAK|mskEP2_NAK|mskEP1_NAK)) {
if (iwIntFlag & (mskEP6_NAK|mskEP5_NAK|mskEP4_NAK|mskEP3_NAK|mskEP2_NAK|mskEP1_NAK)) {
// Determine the interrupting endpoint, direction, and clear the interrupt flag
for(usbep_t ep = 1; ep <= USB_MAX_ENDPOINTS; ep++) {
if (iwIntFlag & mskEPn_NAK(ep)){
Expand Down Expand Up @@ -465,21 +487,68 @@ void handleACK(USBDriver* usbp, usbep_t ep) {
void handleNAK(USBDriver *usbp, usbep_t ep) {
uint8_t out = 0;

if(ep > 0 && ep <= USB_MAX_ENDPOINTS)
{
// Determine the interrupting endpoint direction
if(ep > 0 && ep <= USB_MAX_ENDPOINTS) {
out = ( SN32_USB->CFG & mskEPn_DIR(ep) ) == mskEPn_DIR(ep);
}
else {
return;
}

if(!out) // ack only required for in endpoints

if(out)
{
// no ack required here
}
else
{
// This is not a retransmission, retransmission is transparent and happens on phy layer
// NAK happens when host polls IN EP and device has nothing to send
// It has been observed that sometimes USB phy doesn't generate ACK (unknown why)
// (count ACK interrupts didn't match count of usb_lld_start_in calls per EP)
// However while USB transmitting and qmk thread wants to send another packet qmk goes to
// infinite sleep, expecting that successfull USB transmission will wake it up
// If USB transmission never completes (no ACK) then qmk never wakes up and keyboard locks up
// To prevent this every NAK (1ms or 8ms depending on host poll interval) was calling
// callbacks and wake up function to wake up qmk thread, however packet was not delivered to host
// (for unknown reason) and thus we have seen:
// 1) stuck keypresses when usb packets to press key delivered but key release packet lost
// 2) untyped key when usb packet to press key was lost but key release packet delivered
// Because callback was called every NAK some qmk features didnt work such as CONSOLE
// since callback might release buffers and endup in deadlock via disabled interrupts
// callback for keyboard is empty thus its repated calling is harmless
#if defined(SN32_USB_ORIGINAL_NAK_HANDLING)
_usb_isr_invoke_in_cb(usbp, ep);
#else
//To fake missing ACK we can send 0 sized packet
//however (again for unknown reason) packets now being delivered to host as well!
//- value 2 has been selected to allow at least 2 NAK delivery (2ms or 16ms depending on
//host polling interval) between moment qmk called start_in and moment USB phy actually
//started transmission
//- value 10 was selected arbitrary.
//- values 3-10 we are delivering 0 sized packet trying to get at least one ack
if (nakcnt[ep] > 0) {
//qmk called start_in
if (nakcnt[ep] > 10) {
//11-....
//consider packet undeliverable but ack it to the qmk
nakcnt[ep] = 0;
_usb_isr_invoke_in_cb(usbp, ep);
}
else if (nakcnt[ep] > 2) {
//3-10
nakcnt[ep]++;
EPCTL_SET_STAT_ACK(ep, 0);
}
else {
//1-2
//give it sometime to deliver the packet
nakcnt[ep]++;
}
}
#endif
}

}

/*===========================================================================*/
/* Driver interrupt handlers and threads. */
/*===========================================================================*/
Expand Down Expand Up @@ -642,6 +711,8 @@ void usb_lld_init_endpoint(USBDriver *usbp, usbep_t ep) {
usb_lld_stall_in(usbp, 0);
}
else if(ep <= USB_MAX_ENDPOINTS) {
/* Set endpoint direction flag in USB configuration register.*/
SN32_USB->CFG &= ~mskEPn_DIR(ep);
/* Set endpoint PMA buffer offset in USB configuration register.*/
uint32_t buff_addr = usb_pm_alloc(usbp, epcp->in_maxsize);
USB_SET_BUFFER_OFST(ep,buff_addr);
Expand Down Expand Up @@ -886,4 +957,4 @@ void usb_lld_clear_in(USBDriver *usbp, usbep_t ep) {

#endif /* HAL_USE_USB == TRUE */

/** @} */
/** @} */