On Mon, 6 Dec 2021, Ricardo Martinez wrote: > From: Haijun Liu <haijun.liu@xxxxxxxxxxxx> > > Registers the t7xx device driver with the kernel. Setup all the core > components: PCIe layer, Modem Host Cross Core Interface (MHCCIF), > modem control operations, modem state machine, and build > infrastructure. > > * PCIe layer code implements driver probe and removal. > * MHCCIF provides interrupt channels to communicate events > such as handshake, PM and port enumeration. > * Modem control implements the entry point for modem init, > reset and exit. > * The modem status monitor is a state machine used by modem control > to complete initialization and stop. It is used also to propagate > exception events reported by other components. > > Signed-off-by: Haijun Liu <haijun.liu@xxxxxxxxxxxx> > Signed-off-by: Chandrashekar Devegowda <chandrashekar.devegowda@xxxxxxxxx> > Co-developed-by: Ricardo Martinez <ricardo.martinez@xxxxxxxxxxxxxxx> > Signed-off-by: Ricardo Martinez <ricardo.martinez@xxxxxxxxxxxxxxx> > + switch (reason) { > + case EXCEPTION_HS_TIMEOUT: > + dev_err(dev, "BOOT_HS_FAIL\n"); > + break; > + > + case EXCEPTION_EVENT: > + t7xx_fsm_broadcast_state(ctl, MD_STATE_EXCEPTION); > + t7xx_md_exception_handshake(ctl->md); > + > + cnt = 0; > + while (cnt < FSM_MD_EX_REC_OK_TIMEOUT_MS / FSM_EVENT_POLL_INTERVAL_MS) { > + if (kthread_should_stop()) > + return; > + > + spin_lock_irqsave(&ctl->event_lock, flags); > + event = list_first_entry_or_null(&ctl->event_queue, > + struct t7xx_fsm_event, entry); > + if (event) { > + if (event->event_id == FSM_EVENT_MD_EX) { > + fsm_del_kf_event(event); > + } else if (event->event_id == FSM_EVENT_MD_EX_REC_OK) { > + rec_ok = true; > + fsm_del_kf_event(event); > + } > + } > + > + spin_unlock_irqrestore(&ctl->event_lock, flags); > + if (rec_ok) > + break; > + > + cnt++; > + /* Try again after 20ms */ > + msleep(FSM_EVENT_POLL_INTERVAL_MS); > + } > + > + cnt = 0; > + while (cnt < FSM_MD_EX_PASS_TIMEOUT_MS / FSM_EVENT_POLL_INTERVAL_MS) { > + if (kthread_should_stop()) > + return; > + > + spin_lock_irqsave(&ctl->event_lock, flags); > + event = list_first_entry_or_null(&ctl->event_queue, > + struct t7xx_fsm_event, entry); > + if (event && event->event_id == FSM_EVENT_MD_EX_PASS) { > + pass = true; > + fsm_del_kf_event(event); > + } > + > + spin_unlock_irqrestore(&ctl->event_lock, flags); > + > + if (pass) > + break; > + cnt++; > + /* Try again after 20ms */ > + msleep(FSM_EVENT_POLL_INTERVAL_MS); > + } It hurts me a bit to see so much code duplication with only that one extra if branch + if condition right-hand sides being different. It would seem like something that could be solved with a helper taking those two things as parameters. I hope the ordering of FSM_EVENT_MD_EX, FSM_EVENT_MD_EX_REC_OK, and FSM_EVENT_MD_EX_PASS is guaranteed by something. Otherwise, the event being waited for might not become the first entry in the event_queue and the loop just keeps waiting until timeout? > +void t7xx_fsm_clr_event(struct t7xx_fsm_ctl *ctl, enum t7xx_fsm_event_state event_id) > +{ > + struct device *dev = &ctl->md->t7xx_dev->pdev->dev; > + struct t7xx_fsm_event *event, *evt_next; > + unsigned long flags; > + > + spin_lock_irqsave(&ctl->event_lock, flags); > + list_for_each_entry_safe(event, evt_next, &ctl->event_queue, entry) { > + dev_err(dev, "Unhandled event %d\n", event->event_id); > + > + if (event->event_id == event_id) > + fsm_del_kf_event(event); > + } It seems that only events matching to event_id are removed from the event_queue. Can that dev_err print the same event over and over again (I'm assuming here multiple calls to t7xx_fsm_clr_event can occur) because the other events still remaining in event_queue? > +struct t7xx_fsm_ctl { > + struct t7xx_modem *md; > + enum md_state md_state; > + unsigned int curr_state; > + unsigned int last_state; The value of last_state is never used. -- i.