Skip to content

Commit

Permalink
usb: dwc_otg: fix system lockup when interrupts are threaded
Browse files Browse the repository at this point in the history 
Fix lockup in dwc_otg driver that leads to a system freeze of the
4-core Raspberry Pi board when RT Preempt kernel is in use or when
interrupts are threaded in general.
The lockup occurs when the irq handler thread gets preempted while it
holds the fiq spin lock.
The patch makes sure to disable local irq while fiq spin lock is held
irrespective of whether the interrupt is threaded or not.
The patch also unifies the use of the fiq spin lock outside the fiq
handler by introducing two function-like macros fiq_fsm_spin_lock_irqsave
and fiq_fsm_spin_unlock_irqrestore.

Under RT kernel, the bug can be reproduced in a few minutes by running
hackbench and cyclictest in this way
$ ( while true; do nice hackbench 30 >/dev/null; done )&
$ echo "run 'kill $!' to stop hackbench"
$ cyclictest -a -t -n -p 80

Signed-off-by: Oussama Ghorbel <ghorbel@gmail.com>
[local_irq_{save,restore}() -> local_irq_{save,restore}_nort()]
Signed-off-by: Tiejun Chen <tiejun.china@gmail.com>
  • Loading branch information
@oghorbel
oghorbel authored and Tiejun Chen committed Feb 25, 2018
1 parent 8e9f5e4 commit 05dd5c4
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 56 deletions.
14 changes: 14 additions & 0 deletions drivers/usb/host/dwc_otg/dwc_otg_fiq_fsm.h
View file
Expand Up @@ -393,4 +393,18 @@ extern void dwc_otg_fiq_fsm(struct fiq_state *state, int num_channels);

extern void dwc_otg_fiq_nop(struct fiq_state *state);

#define fiq_fsm_spin_lock_irqsave(lock, flags) \
do { \
local_fiq_disable(); \
local_irq_save_nort(flags); \
fiq_fsm_spin_lock(lock); \
} while (0)

#define fiq_fsm_spin_unlock_irqrestore(lock, flags) \
do { \
fiq_fsm_spin_unlock(lock); \
local_irq_restore_nort(flags); \
local_fiq_enable(); \
} while (0)

#endif /* DWC_OTG_FIQ_FSM_H_ */
41 changes: 17 additions & 24 deletions drivers/usb/host/dwc_otg/dwc_otg_hcd.c
View file
Expand Up @@ -1414,12 +1414,10 @@ static void assign_and_init_hc(dwc_otg_hcd_t * hcd, dwc_otg_qh_t * qh)

dwc_otg_hc_init(hcd->core_if, hc);

local_irq_save(flags);

if (fiq_enable) {
local_fiq_disable();
fiq_fsm_spin_lock(&hcd->fiq_state->lock);
}
if (fiq_enable)
fiq_fsm_spin_lock_irqsave(&hcd->fiq_state->lock, flags);
else
local_irq_save(flags);

/* Enable the top level host channel interrupt. */
intr_enable = (1 << hc->hc_num);
Expand All @@ -1429,12 +1427,10 @@ static void assign_and_init_hc(dwc_otg_hcd_t * hcd, dwc_otg_qh_t * qh)
gintmsk.b.hcintr = 1;
DWC_MODIFY_REG32(&hcd->core_if->core_global_regs->gintmsk, 0, gintmsk.d32);

if (fiq_enable) {
fiq_fsm_spin_unlock(&hcd->fiq_state->lock);
local_fiq_enable();
}

local_irq_restore(flags);
if (fiq_enable)
fiq_fsm_spin_unlock_irqrestore(&hcd->fiq_state->lock, flags);
else
local_irq_restore(flags);
hc->qh = qh;
}

Expand Down Expand Up @@ -1646,6 +1642,7 @@ int fiq_fsm_queue_isoc_transaction(dwc_otg_hcd_t *hcd, dwc_otg_qh_t *qh)
int xfer_len, nrpackets;
hcdma_data_t hcdma;
hfnum_data_t hfnum;
unsigned long flags;

if (st->fsm != FIQ_PASSTHROUGH)
return 0;
Expand Down Expand Up @@ -1721,8 +1718,7 @@ int fiq_fsm_queue_isoc_transaction(dwc_otg_hcd_t *hcd, dwc_otg_qh_t *qh)
fiq_print(FIQDBG_INT, hcd->fiq_state, "%08x", st->hctsiz_copy.d32);
fiq_print(FIQDBG_INT, hcd->fiq_state, "%08x", st->hcdma_copy.d32);
hfnum.d32 = DWC_READ_REG32(&hcd->core_if->host_if->host_global_regs->hfnum);
local_fiq_disable();
fiq_fsm_spin_lock(&hcd->fiq_state->lock);
fiq_fsm_spin_lock_irqsave(&hcd->fiq_state->lock, flags);
DWC_WRITE_REG32(&hc_regs->hctsiz, st->hctsiz_copy.d32);
DWC_WRITE_REG32(&hc_regs->hcsplt, st->hcsplt_copy.d32);
DWC_WRITE_REG32(&hc_regs->hcdma, st->hcdma_copy.d32);
Expand All @@ -1742,8 +1738,7 @@ int fiq_fsm_queue_isoc_transaction(dwc_otg_hcd_t *hcd, dwc_otg_qh_t *qh)
}
mb();
st->hcchar_copy.b.chen = 0;
fiq_fsm_spin_unlock(&hcd->fiq_state->lock);
local_fiq_enable();
fiq_fsm_spin_unlock_irqrestore(&hcd->fiq_state->lock, flags);
return 0;
}

Expand All @@ -1769,6 +1764,7 @@ int fiq_fsm_queue_split_transaction(dwc_otg_hcd_t *hcd, dwc_otg_qh_t *qh)
/* Program HC registers, setup FIQ_state, examine FIQ if periodic, start transfer (not if uframe 5) */
int hub_addr, port_addr, frame, uframe;
struct fiq_channel_state *st = &hcd->fiq_state->channel[hc->hc_num];
unsigned long flags;

/*
* Non-periodic channel assignments stay in the non_periodic_active queue.
Expand Down Expand Up @@ -1889,8 +1885,7 @@ int fiq_fsm_queue_split_transaction(dwc_otg_hcd_t *hcd, dwc_otg_qh_t *qh)
DWC_WRITE_REG32(&hc_regs->hcchar, st->hcchar_copy.d32);
DWC_WRITE_REG32(&hc_regs->hcintmsk, st->hcintmsk_copy.d32);

local_fiq_disable();
fiq_fsm_spin_lock(&hcd->fiq_state->lock);
fiq_fsm_spin_lock_irqsave(&hcd->fiq_state->lock, flags);

if (hc->ep_type & 0x1) {
hfnum.d32 = DWC_READ_REG32(&hcd->core_if->host_if->host_global_regs->hfnum);
Expand Down Expand Up @@ -1999,8 +1994,7 @@ int fiq_fsm_queue_split_transaction(dwc_otg_hcd_t *hcd, dwc_otg_qh_t *qh)
DWC_WRITE_REG32(&hc_regs->hcchar, st->hcchar_copy.d32);
}
mb();
fiq_fsm_spin_unlock(&hcd->fiq_state->lock);
local_fiq_enable();
fiq_fsm_spin_unlock_irqrestore(&hcd->fiq_state->lock, flags);
return 0;
}

Expand Down Expand Up @@ -2481,6 +2475,7 @@ static void process_non_periodic_channels(dwc_otg_hcd_t * hcd)
void dwc_otg_hcd_queue_transactions(dwc_otg_hcd_t * hcd,
dwc_otg_transaction_type_e tr_type)
{
unsigned long flags;
#ifdef DEBUG_SOF
DWC_DEBUGPL(DBG_HCD, "Queue Transactions\n");
#endif
Expand All @@ -2506,11 +2501,9 @@ void dwc_otg_hcd_queue_transactions(dwc_otg_hcd_t * hcd,
gintmsk.b.nptxfempty = 1;

if (fiq_enable) {
local_fiq_disable();
fiq_fsm_spin_lock(&hcd->fiq_state->lock);
fiq_fsm_spin_lock_irqsave(&hcd->fiq_state->lock, flags);
DWC_MODIFY_REG32(&hcd->core_if->core_global_regs->gintmsk, gintmsk.d32, 0);
fiq_fsm_spin_unlock(&hcd->fiq_state->lock);
local_fiq_enable();
fiq_fsm_spin_unlock_irqrestore(&hcd->fiq_state->lock, flags);
} else {
DWC_MODIFY_REG32(&hcd->core_if->core_global_regs->gintmsk, gintmsk.d32, 0);
}
Expand Down
40 changes: 16 additions & 24 deletions drivers/usb/host/dwc_otg/dwc_otg_hcd_intr.c
View file
Expand Up @@ -83,6 +83,7 @@ int32_t dwc_otg_hcd_handle_intr(dwc_otg_hcd_t * dwc_otg_hcd)
gintmsk_data_t gintmsk;
hfnum_data_t hfnum;
haintmsk_data_t haintmsk;
unsigned long flags;

#ifdef DEBUG
dwc_otg_core_global_regs_t *global_regs = core_if->core_global_regs;
Expand All @@ -100,8 +101,7 @@ int32_t dwc_otg_hcd_handle_intr(dwc_otg_hcd_t * dwc_otg_hcd)
/* Check if HOST Mode */
if (dwc_otg_is_host_mode(core_if)) {
if (fiq_enable) {
local_fiq_disable();
fiq_fsm_spin_lock(&dwc_otg_hcd->fiq_state->lock);
fiq_fsm_spin_lock_irqsave(&dwc_otg_hcd->fiq_state->lock, flags);
/* Pull in from the FIQ's disabled mask */
gintmsk.d32 = gintmsk.d32 | ~(dwc_otg_hcd->fiq_state->gintmsk_saved.d32);
dwc_otg_hcd->fiq_state->gintmsk_saved.d32 = ~0;
Expand All @@ -118,8 +118,7 @@ int32_t dwc_otg_hcd_handle_intr(dwc_otg_hcd_t * dwc_otg_hcd)
gintsts.d32 &= gintmsk.d32;

if (fiq_enable) {
fiq_fsm_spin_unlock(&dwc_otg_hcd->fiq_state->lock);
local_fiq_enable();
fiq_fsm_spin_unlock_irqrestore(&dwc_otg_hcd->fiq_state->lock, flags);
}

if (!gintsts.d32) {
Expand Down Expand Up @@ -166,11 +165,9 @@ int32_t dwc_otg_hcd_handle_intr(dwc_otg_hcd_t * dwc_otg_hcd)
gintmsk_data_t gintmsk = { .b.portintr = 1};
retval |= dwc_otg_hcd_handle_port_intr(dwc_otg_hcd);
if (fiq_enable) {
local_fiq_disable();
fiq_fsm_spin_lock(&dwc_otg_hcd->fiq_state->lock);
fiq_fsm_spin_lock_irqsave(&dwc_otg_hcd->fiq_state->lock, flags);
DWC_MODIFY_REG32(&dwc_otg_hcd->core_if->core_global_regs->gintmsk, 0, gintmsk.d32);
fiq_fsm_spin_unlock(&dwc_otg_hcd->fiq_state->lock);
local_fiq_enable();
fiq_fsm_spin_unlock_irqrestore(&dwc_otg_hcd->fiq_state->lock, flags);
} else {
DWC_MODIFY_REG32(&dwc_otg_hcd->core_if->core_global_regs->gintmsk, 0, gintmsk.d32);
}
Expand Down Expand Up @@ -210,8 +207,7 @@ int32_t dwc_otg_hcd_handle_intr(dwc_otg_hcd_t * dwc_otg_hcd)
if (fiq_enable) {
gintmsk_data_t gintmsk_new;
haintmsk_data_t haintmsk_new;
local_fiq_disable();
fiq_fsm_spin_lock(&dwc_otg_hcd->fiq_state->lock);
fiq_fsm_spin_lock_irqsave(&dwc_otg_hcd->fiq_state->lock, flags);
gintmsk_new.d32 = *(volatile uint32_t *)&dwc_otg_hcd->fiq_state->gintmsk_saved.d32;
if(fiq_fsm_enable)
haintmsk_new.d32 = *(volatile uint32_t *)&dwc_otg_hcd->fiq_state->haintmsk_saved.d32;
Expand All @@ -234,8 +230,7 @@ int32_t dwc_otg_hcd_handle_intr(dwc_otg_hcd_t * dwc_otg_hcd)
haintmsk.d32 = DWC_READ_REG32(&core_if->host_if->host_global_regs->haintmsk);
/* Re-enable interrupts that the FIQ masked (first time round) */
FIQ_WRITE(dwc_otg_hcd->fiq_state->dwc_regs_base + GINTMSK, gintmsk.d32);
fiq_fsm_spin_unlock(&dwc_otg_hcd->fiq_state->lock);
local_fiq_enable();
fiq_fsm_spin_unlock_irqrestore(&dwc_otg_hcd->fiq_state->lock, flags);

if ((jiffies / HZ) > last_time) {
//dwc_otg_qh_t *qh;
Expand Down Expand Up @@ -637,6 +632,7 @@ int32_t dwc_otg_hcd_handle_hc_intr(dwc_otg_hcd_t * dwc_otg_hcd)
{
int i;
int retval = 0;
unsigned long flags;
haint_data_t haint = { .d32 = 0 } ;

/* Clear appropriate bits in HCINTn to clear the interrupt bit in
Expand All @@ -649,12 +645,10 @@ int32_t dwc_otg_hcd_handle_hc_intr(dwc_otg_hcd_t * dwc_otg_hcd)
if(fiq_fsm_enable)
{
/* check the mask? */
local_fiq_disable();
fiq_fsm_spin_lock(&dwc_otg_hcd->fiq_state->lock);
fiq_fsm_spin_lock_irqsave(&dwc_otg_hcd->fiq_state->lock, flags);
haint.b2.chint |= ~(dwc_otg_hcd->fiq_state->haintmsk_saved.b2.chint);
dwc_otg_hcd->fiq_state->haintmsk_saved.b2.chint = ~0;
fiq_fsm_spin_unlock(&dwc_otg_hcd->fiq_state->lock);
local_fiq_enable();
fiq_fsm_spin_unlock_irqrestore(&dwc_otg_hcd->fiq_state->lock, flags);
}

for (i = 0; i < dwc_otg_hcd->core_if->core_params->host_channels; i++) {
Expand Down Expand Up @@ -1061,6 +1055,8 @@ static void halt_channel(dwc_otg_hcd_t * hcd,
dwc_hc_t * hc,
dwc_otg_qtd_t * qtd, dwc_otg_halt_status_e halt_status)
{
unsigned long flags;

if (hcd->core_if->dma_enable) {
release_channel(hcd, hc, qtd, halt_status);
return;
Expand All @@ -1083,11 +1079,9 @@ static void halt_channel(dwc_otg_hcd_t * hcd,
*/
gintmsk.b.nptxfempty = 1;
if (fiq_enable) {
local_fiq_disable();
fiq_fsm_spin_lock(&hcd->fiq_state->lock);
fiq_fsm_spin_lock_irqsave(&hcd->fiq_state->lock, flags);
DWC_MODIFY_REG32(&global_regs->gintmsk, 0, gintmsk.d32);
fiq_fsm_spin_unlock(&hcd->fiq_state->lock);
local_fiq_enable();
fiq_fsm_spin_unlock_irqrestore(&hcd->fiq_state->lock, flags);
} else {
DWC_MODIFY_REG32(&global_regs->gintmsk, 0, gintmsk.d32);
}
Expand All @@ -1108,11 +1102,9 @@ static void halt_channel(dwc_otg_hcd_t * hcd,
*/
gintmsk.b.ptxfempty = 1;
if (fiq_enable) {
local_fiq_disable();
fiq_fsm_spin_lock(&hcd->fiq_state->lock);
fiq_fsm_spin_lock_irqsave(&hcd->fiq_state->lock, flags);
DWC_MODIFY_REG32(&global_regs->gintmsk, 0, gintmsk.d32);
fiq_fsm_spin_unlock(&hcd->fiq_state->lock);
local_fiq_enable();
fiq_fsm_spin_unlock_irqrestore(&hcd->fiq_state->lock, flags);
} else {
DWC_MODIFY_REG32(&global_regs->gintmsk, 0, gintmsk.d32);
}
Expand Down
15 changes: 7 additions & 8 deletions drivers/usb/host/dwc_otg/dwc_otg_hcd_queue.c
View file
Expand Up @@ -676,6 +676,7 @@ static int schedule_periodic(dwc_otg_hcd_t * hcd, dwc_otg_qh_t * qh)
int dwc_otg_hcd_qh_add(dwc_otg_hcd_t * hcd, dwc_otg_qh_t * qh)
{
int status = 0;
unsigned long flags;
gintmsk_data_t intr_mask = {.d32 = 0 };

if (!DWC_LIST_EMPTY(&qh->qh_list_entry)) {
Expand All @@ -694,11 +695,9 @@ int dwc_otg_hcd_qh_add(dwc_otg_hcd_t * hcd, dwc_otg_qh_t * qh)
if ( !hcd->periodic_qh_count ) {
intr_mask.b.sofintr = 1;
if (fiq_enable) {
local_fiq_disable();
fiq_fsm_spin_lock(&hcd->fiq_state->lock);
fiq_fsm_spin_lock_irqsave(&hcd->fiq_state->lock, flags);
DWC_MODIFY_REG32(&hcd->core_if->core_global_regs->gintmsk, intr_mask.d32, intr_mask.d32);
fiq_fsm_spin_unlock(&hcd->fiq_state->lock);
local_fiq_enable();
fiq_fsm_spin_unlock_irqrestore(&hcd->fiq_state->lock, flags);
} else {
DWC_MODIFY_REG32(&hcd->core_if->core_global_regs->gintmsk, intr_mask.d32, intr_mask.d32);
}
Expand Down Expand Up @@ -742,6 +741,8 @@ static void deschedule_periodic(dwc_otg_hcd_t * hcd, dwc_otg_qh_t * qh)
* @param qh QH to remove from schedule. */
void dwc_otg_hcd_qh_remove(dwc_otg_hcd_t * hcd, dwc_otg_qh_t * qh)
{
unsigned long flags;

gintmsk_data_t intr_mask = {.d32 = 0 };

if (DWC_LIST_EMPTY(&qh->qh_list_entry)) {
Expand All @@ -763,11 +764,9 @@ void dwc_otg_hcd_qh_remove(dwc_otg_hcd_t * hcd, dwc_otg_qh_t * qh)
if( !hcd->periodic_qh_count && !fiq_fsm_enable ) {
intr_mask.b.sofintr = 1;
if (fiq_enable) {
local_fiq_disable();
fiq_fsm_spin_lock(&hcd->fiq_state->lock);
fiq_fsm_spin_lock_irqsave(&hcd->fiq_state->lock, flags);
DWC_MODIFY_REG32(&hcd->core_if->core_global_regs->gintmsk, intr_mask.d32, 0);
fiq_fsm_spin_unlock(&hcd->fiq_state->lock);
local_fiq_enable();
fiq_fsm_spin_unlock_irqrestore(&hcd->fiq_state->lock, flags);
} else {
DWC_MODIFY_REG32(&hcd->core_if->core_global_regs->gintmsk, intr_mask.d32, 0);
}
Expand Down

0 comments on commit 05dd5c4

Please sign in to comment.