| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <236a5ed2-566c-9ee8-4189-8c647044a82f@quicinc.com>
Date: Thu, 23 Jun 2022 11:00:35 -0600
From: Jeffrey Hugo <quic_jhugo@...cinc.com>
To: Manivannan Sadhasivam <mani@...nel.org>,
Qiang Yu <quic_qianyu@...cinc.com>
CC: <quic_hemantk@...cinc.com>, <loic.poulain@...aro.org>,
<mhi@...ts.linux.dev>, <linux-arm-msm@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <quic_cang@...cinc.com>
Subject: Re: [PATCH v4 1/1] bus: mhi: host: Move IRQ allocation to controller
registration phase
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@...cinc.com>
>
> I thought I already gave my r-o-b. But anyway,
>
> Reviewed-by: Manivannan Sadhasivam <mani@...nel.org>
>
> I'll wait for a review from Jeff before applying.
Looks good to me.
Reviewed-by: Jeffrey Hugo <quic_jhugo@...cinc.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.
>>
>
Powered by blists - more mailing lists