From: Bhaumik Bhatt <bbhatt@codeaurora.org>
To: Loic Poulain <loic.poulain@linaro.org>
Cc: manivannan.sadhasivam@linaro.org, hemantk@codeaurora.org,
linux-arm-msm@vger.kernel.org, jhugo@codeaurora.org
Subject: Re: [PATCH v3] mhi: pci_generic: Ensure device readiness before starting MHI
Date: Thu, 11 Feb 2021 11:48:46 -0800 [thread overview]
Message-ID: <acdace3428d2850e6f5ecacc9888d2ca@codeaurora.org> (raw)
In-Reply-To: <1613071507-31489-1-git-send-email-loic.poulain@linaro.org>
Hi Loic,
On 2021-02-11 11:25 AM, Loic Poulain wrote:
> The PCI device may have not been bound from cold boot and be in
> undefined state, or simply not yet ready for MHI operations. This
> change ensures that the MHI layer is reset to initial state and
> ready for MHI initialization and power up.
>
> Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
> ---
> v2: reset only if necessary
> v3: do not wait for MHI readiness in PBL context
>
> drivers/bus/mhi/pci_generic.c | 33 +++++++++++++++++++++++++++++++++
> 1 file changed, 33 insertions(+)
>
> diff --git a/drivers/bus/mhi/pci_generic.c
> b/drivers/bus/mhi/pci_generic.c
> index c20f59e..87abd7c 100644
> --- a/drivers/bus/mhi/pci_generic.c
> +++ b/drivers/bus/mhi/pci_generic.c
> @@ -17,6 +17,8 @@
> #include <linux/timer.h>
> #include <linux/workqueue.h>
>
> +#include "core/internal.h"
> +
> #define MHI_PCI_DEFAULT_BAR_NUM 0
>
> #define MHI_POST_RESET_DELAY_MS 500
> @@ -256,6 +258,7 @@ static int mhi_pci_claim(struct mhi_controller
> *mhi_cntrl,
> return err;
> }
> mhi_cntrl->regs = pcim_iomap_table(pdev)[bar_num];
> + mhi_cntrl->bhi = mhi_cntrl->regs + readl(mhi_cntrl->regs + BHIOFF);
>
> err = pci_set_dma_mask(pdev, dma_mask);
> if (err) {
> @@ -391,6 +394,31 @@ static void health_check(struct timer_list *t)
> mod_timer(&mhi_pdev->health_check_timer, jiffies +
> HEALTH_CHECK_PERIOD);
> }
>
> +static void __mhi_sw_reset(struct mhi_controller *mhi_cntrl)
> +{
> + unsigned int max_wait_ready = 100;
> +
> + if (MHI_IN_PBL(mhi_get_exec_env(mhi_cntrl))) {
> + /* nothing to do, ready for BHI */
> + return;
> + }
> +
> + if (mhi_get_mhi_state(mhi_cntrl) >= MHI_STATE_M0) {
> + dev_warn(mhi_cntrl->cntrl_dev, "Need reset\n");
> + writel(MHICTRL_RESET_MASK, mhi_cntrl->regs + MHICTRL);
> + msleep(10);
> + }
> +
> + while (mhi_get_mhi_state(mhi_cntrl) != MHI_STATE_READY) {
> + if (!max_wait_ready--) {
> + dev_warn(mhi_cntrl->cntrl_dev, "Not ready (state %u)\n",
> + mhi_get_mhi_state(mhi_cntrl));
> + break;
> + }
> + msleep(50);
> + }
> +}
> +
> static int mhi_pci_probe(struct pci_dev *pdev, const struct
> pci_device_id *id)
> {
> const struct mhi_pci_dev_info *info = (struct mhi_pci_dev_info *)
> id->driver_data;
> @@ -451,6 +479,9 @@ static int mhi_pci_probe(struct pci_dev *pdev,
> const struct pci_device_id *id)
> goto err_unregister;
> }
>
> + /* Before starting MHI, ensure device is in good initial state */
> + __mhi_sw_reset(mhi_cntrl);
> +
> err = mhi_sync_power_up(mhi_cntrl);
> if (err) {
> dev_err(&pdev->dev, "failed to power up MHI controller\n");
> @@ -532,6 +563,8 @@ static void mhi_pci_reset_done(struct pci_dev
> *pdev)
> return;
> }
>
> + __mhi_sw_reset(mhi_cntrl);
> +
> err = mhi_sync_power_up(mhi_cntrl);
> if (err) {
> dev_err(&pdev->dev, "failed to power up MHI controller\n");
I have a set of changes coming in where you don't need to set BHI before
the
mhi_sync_power_up() call. I will be moving it to
mhi_prepare_for_power_up().
I will share the changes in the next patch series so you don't have to
set it.
Another thing I was thinking: we could expose a function to issue an MHI
RESET
so controllers have the freedom to use it if EE makes it possible.
This way we avoid the +#include "core/internal.h" and maintain a clear
distinction
between controller and core.
Thanks,
Bhaumik
---
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum,
a Linux Foundation Collaborative Project
next prev parent reply other threads:[~2021-02-11 19:50 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-11 19:25 [PATCH v3] mhi: pci_generic: Ensure device readiness before starting MHI Loic Poulain
2021-02-11 19:48 ` Bhaumik Bhatt [this message]
2021-02-12 1:41 ` Bhaumik Bhatt
2021-02-15 7:30 ` Loic Poulain
2021-02-16 17:37 ` Bhaumik Bhatt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=acdace3428d2850e6f5ecacc9888d2ca@codeaurora.org \
--to=bbhatt@codeaurora.org \
--cc=hemantk@codeaurora.org \
--cc=jhugo@codeaurora.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=loic.poulain@linaro.org \
--cc=manivannan.sadhasivam@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).