lkml.org 
[lkml]   [2022]   [Jun]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v4 1/1] bus: mhi: host: Move IRQ allocation to controller registration phase
From
On 6/23/2022 5:54 AM, 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.

Looks good to me.

Reviewed-by: Jeffrey Hugo <quic_jhugo@quicinc.com>

>
> Thanks,
> Mani
>
>> ---
>> 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.
>>
>

\
 
 \ /
  Last update: 2022-06-23 19:22    [W:0.154 / U:1.924 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site