Sorry for the spam. Re-sending as plain text.
On 11/2/2021 8:46 AM, Andy Shevchenko wrote:
On Sun, Oct 31, 2021 at 08:56:24PM -0700, Ricardo Martinez wrote:
From: Haijun Lio<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.
+#define CCCI_HEADER_NO_DATA 0xffffffff
Is this internal value to Linux or something which is given by hardware?
It is hardware defined
...
+ spin_lock_irqsave(&md_info->exp_spinlock, flags);
Can it be called outside of IRQ context?
Actually, it is not in IRQ context, this function is called by the
thread_fn passed to request_threaded_irq(),
Maybe spin_lock_bh makes more sense.
+ int_sta = get_interrupt_status(mtk_dev);
+ md_info->exp_id |= int_sta;
+
+ if (md_info->exp_id & D2H_INT_PORT_ENUM) {
+ md_info->exp_id &= ~D2H_INT_PORT_ENUM;
+ if (ctl->curr_state == CCCI_FSM_INIT ||
+ ctl->curr_state == CCCI_FSM_PRE_START ||
+ ctl->curr_state == CCCI_FSM_STOPPED)
+ ccci_fsm_recv_md_interrupt(MD_IRQ_PORT_ENUM);
+ }
+
+ if (md_info->exp_id & D2H_INT_EXCEPTION_INIT) {
+ if (ctl->md_state == MD_STATE_INVALID ||
+ ctl->md_state == MD_STATE_WAITING_FOR_HS1 ||
+ ctl->md_state == MD_STATE_WAITING_FOR_HS2 ||
+ ctl->md_state == MD_STATE_READY) {
+ md_info->exp_id &= ~D2H_INT_EXCEPTION_INIT;
+ ccci_fsm_recv_md_interrupt(MD_IRQ_CCIF_EX);
+ }
+ } else if (ctl->md_state == MD_STATE_WAITING_FOR_HS1) {
+ /* start handshake if MD not assert */
+ mask = mhccif_mask_get(mtk_dev);
+ if ((md_info->exp_id & D2H_INT_ASYNC_MD_HK) && !(mask &
D2H_INT_ASYNC_MD_HK)) {
+ md_info->exp_id &= ~D2H_INT_ASYNC_MD_HK;
+ queue_work(md->handshake_wq, &md->handshake_work);
+ }
+ }
+
+ spin_unlock_irqrestore(&md_info->exp_spinlock, flags);
+
+ return 0;
+}
...
+ cmd = kmalloc(sizeof(*cmd),
+ (in_irq() || in_softirq() || irqs_disabled()) ? GFP_ATOMIC :
GFP_KERNEL);
Hmm...
Looks like we can just use in_interrupt(), was that the concern?
+ if (!cmd)
+ return -ENOMEM;
+ if (in_irq() || irqs_disabled())
+ flag &= ~FSM_CMD_FLAG_WAITING_TO_COMPLETE;
Even more hmm...
+ if (flag & FSM_CMD_FLAG_WAITING_TO_COMPLETE) {
+ wait_event(cmd->complete_wq, cmd->result != FSM_CMD_RESULT_PENDING);
Is it okay in IRQ context?
We know that the caller that sets FSM_CMD_FLAG_WAITING_TO_COMPLETE flag
it is not in IRQ context.
If that's good enough then we could also remove the check that clears
that flag few lines above.
The only calls using FSM_CMD_FLAG_WAITING_TO_COMPLETE are coming from
dev_pm_ops callbacks, and
we are not setting pm_runtime_irq_safe().
Otherwise we can use in_interrupt() to check here as well.
+ if (cmd->result != FSM_CMD_RESULT_OK)
+ result = -EINVAL;
