Messages in this thread | | | Date | Thu, 23 Jun 2022 20:49:00 +0800 | Subject | Re: [PATCH v4 1/1] bus: mhi: host: Move IRQ allocation to controller registration phase | From | Qiang Yu <> |
| |
On 6/23/2022 7:54 PM, Manivannan Sadhasivam wrote: > On Thu, Jun 23, 2022 at 10:43:03AM +0800, Qiang Yu wrote: >> During runtime, the MHI endpoint may be powered up/down several times. >> So instead of allocating and destroying the IRQs all the time, let's just >> enable/disable IRQs during power up/down. >> >> The IRQs will be allocated during mhi_register_controller() and freed >> during mhi_unregister_controller(). This works well for things like PCI >> hotplug also as once the PCI device gets removed, the controller will >> get unregistered. And once it comes back, it will get registered back >> and even if the IRQ configuration changes (MSI), that will get accounted. >> >> Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com> > I thought I already gave my r-o-b. But anyway, > > Reviewed-by: Manivannan Sadhasivam <mani@kernel.org> > > I'll wait for a review from Jeff before applying. > > Thanks, > Mani
I apologize for that. Thanks for your time and patience.
> >> --- >> v3->v4: move mhi_init_irq_setup() above mhi_alloc_device() >> v2->v3: change commit text and comments. >> v1->v2: Rewrite commit text. Remove a random change. Use >> inline enables. >> >> drivers/bus/mhi/host/init.c | 17 ++++++++++++++++- >> drivers/bus/mhi/host/pm.c | 19 +++++++++++++------ >> 2 files changed, 29 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/bus/mhi/host/init.c b/drivers/bus/mhi/host/init.c >> index cbb86b2..a1d37da 100644 >> --- a/drivers/bus/mhi/host/init.c >> +++ b/drivers/bus/mhi/host/init.c >> @@ -179,6 +179,12 @@ int mhi_init_irq_setup(struct mhi_controller *mhi_cntrl) >> "bhi", mhi_cntrl); >> if (ret) >> return ret; >> + /* >> + * IRQs should be enabled during mhi_async_power_up(), so disable them explicitly here. >> + * Due to the use of IRQF_SHARED flag as default while requesting IRQs, we assume that >> + * IRQ_NOAUTOEN is not applicable. >> + */ >> + disable_irq(mhi_cntrl->irq[0]); >> >> for (i = 0; i < mhi_cntrl->total_ev_rings; i++, mhi_event++) { >> if (mhi_event->offload_ev) >> @@ -200,6 +206,8 @@ int mhi_init_irq_setup(struct mhi_controller *mhi_cntrl) >> mhi_cntrl->irq[mhi_event->irq], i); >> goto error_request; >> } >> + >> + disable_irq(mhi_cntrl->irq[mhi_event->irq]); >> } >> >> return 0; >> @@ -979,12 +987,16 @@ int mhi_register_controller(struct mhi_controller *mhi_cntrl, >> goto err_destroy_wq; >> } >> >> + ret = mhi_init_irq_setup(mhi_cntrl); >> + if (ret) >> + goto err_ida_free; >> + >> /* Register controller with MHI bus */ >> mhi_dev = mhi_alloc_device(mhi_cntrl); >> if (IS_ERR(mhi_dev)) { >> dev_err(mhi_cntrl->cntrl_dev, "Failed to allocate MHI device\n"); >> ret = PTR_ERR(mhi_dev); >> - goto err_ida_free; >> + goto error_setup_irq; >> } >> >> mhi_dev->dev_type = MHI_DEVICE_CONTROLLER; >> @@ -1007,6 +1019,8 @@ int mhi_register_controller(struct mhi_controller *mhi_cntrl, >> >> err_release_dev: >> put_device(&mhi_dev->dev); >> +error_setup_irq: >> + mhi_deinit_free_irq(mhi_cntrl); >> err_ida_free: >> ida_free(&mhi_controller_ida, mhi_cntrl->index); >> err_destroy_wq: >> @@ -1027,6 +1041,7 @@ void mhi_unregister_controller(struct mhi_controller *mhi_cntrl) >> struct mhi_chan *mhi_chan = mhi_cntrl->mhi_chan; >> unsigned int i; >> >> + mhi_deinit_free_irq(mhi_cntrl); >> mhi_destroy_debugfs(mhi_cntrl); >> >> destroy_workqueue(mhi_cntrl->hiprio_wq); >> diff --git a/drivers/bus/mhi/host/pm.c b/drivers/bus/mhi/host/pm.c >> index dc2e8ff..4a42186 100644 >> --- a/drivers/bus/mhi/host/pm.c >> +++ b/drivers/bus/mhi/host/pm.c >> @@ -500,7 +500,7 @@ static void mhi_pm_disable_transition(struct mhi_controller *mhi_cntrl) >> for (i = 0; i < mhi_cntrl->total_ev_rings; i++, mhi_event++) { >> if (mhi_event->offload_ev) >> continue; >> - free_irq(mhi_cntrl->irq[mhi_event->irq], mhi_event); >> + disable_irq(mhi_cntrl->irq[mhi_event->irq]); >> tasklet_kill(&mhi_event->task); >> } >> >> @@ -1060,12 +1060,13 @@ static void mhi_deassert_dev_wake(struct mhi_controller *mhi_cntrl, >> >> int mhi_async_power_up(struct mhi_controller *mhi_cntrl) >> { >> + struct mhi_event *mhi_event = mhi_cntrl->mhi_event; >> enum mhi_state state; >> enum mhi_ee_type current_ee; >> enum dev_st_transition next_state; >> struct device *dev = &mhi_cntrl->mhi_dev->dev; >> u32 interval_us = 25000; /* poll register field every 25 milliseconds */ >> - int ret; >> + int ret, i; >> >> dev_info(dev, "Requested to power ON\n"); >> >> @@ -1117,9 +1118,15 @@ int mhi_async_power_up(struct mhi_controller *mhi_cntrl) >> mhi_write_reg(mhi_cntrl, mhi_cntrl->bhi, BHI_INTVEC, 0); >> } >> >> - ret = mhi_init_irq_setup(mhi_cntrl); >> - if (ret) >> - goto error_exit; >> + /* IRQs have been requested during probe, so we just need to enable them. */ >> + enable_irq(mhi_cntrl->irq[0]); >> + >> + for (i = 0; i < mhi_cntrl->total_ev_rings; i++, mhi_event++) { >> + if (mhi_event->offload_ev) >> + continue; >> + >> + enable_irq(mhi_cntrl->irq[mhi_event->irq]); >> + } >> >> /* Transition to next state */ >> next_state = MHI_IN_PBL(current_ee) ? >> @@ -1182,7 +1189,7 @@ void mhi_power_down(struct mhi_controller *mhi_cntrl, bool graceful) >> /* Wait for shutdown to complete */ >> flush_work(&mhi_cntrl->st_worker); >> >> - free_irq(mhi_cntrl->irq[0], mhi_cntrl); >> + disable_irq(mhi_cntrl->irq[0]); >> } >> EXPORT_SYMBOL_GPL(mhi_power_down); >> >> -- >> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project. >>
| |