| 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: <20220623115436.GA15542@thinkpad>
Date: Thu, 23 Jun 2022 17:24:36 +0530
From: Manivannan Sadhasivam <mani@...nel.org>
To: Qiang Yu <quic_qianyu@...cinc.com>
Cc: quic_hemantk@...cinc.com, loic.poulain@...aro.org,
quic_jhugo@...cinc.com, 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 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.
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