* [PATCH v3] mhi: pci_generic: Ensure device readiness before starting MHI
@ 2021-02-11 19:25 Loic Poulain
2021-02-11 19:48 ` Bhaumik Bhatt
2021-02-12 1:41 ` Bhaumik Bhatt
0 siblings, 2 replies; 5+ messages in thread
From: Loic Poulain @ 2021-02-11 19:25 UTC (permalink / raw)
To: manivannan.sadhasivam, hemantk; +Cc: linux-arm-msm, jhugo, Loic Poulain
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");
--
2.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v3] mhi: pci_generic: Ensure device readiness before starting MHI
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
2021-02-12 1:41 ` Bhaumik Bhatt
1 sibling, 0 replies; 5+ messages in thread
From: Bhaumik Bhatt @ 2021-02-11 19:48 UTC (permalink / raw)
To: Loic Poulain; +Cc: manivannan.sadhasivam, hemantk, linux-arm-msm, jhugo
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3] mhi: pci_generic: Ensure device readiness before starting MHI
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
@ 2021-02-12 1:41 ` Bhaumik Bhatt
2021-02-15 7:30 ` Loic Poulain
1 sibling, 1 reply; 5+ messages in thread
From: Bhaumik Bhatt @ 2021-02-12 1:41 UTC (permalink / raw)
To: Loic Poulain; +Cc: manivannan.sadhasivam, hemantk, linux-arm-msm, jhugo
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");
Can you share logs of what you're seeing as it is not clear why you
would need
this patch.
We have a mechanism in place that Jeff added a while back [1], to check
if device
is in SYS_ERROR state and do the same: issue reset and later, wait for
ready from
within mhi_sync_power_up() API.
Note that the MHI_IN_PBL() macro includes EDL and Pass Through modes as
well and
we do expect an MHI READY state move after Pass Through.
Thanks,
Bhaumik
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/bus/mhi/core?h=v5.11-rc7&id=e18d4e9fa79bb27de6447c0c172bb1c428a52bb2
---
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3] mhi: pci_generic: Ensure device readiness before starting MHI
2021-02-12 1:41 ` Bhaumik Bhatt
@ 2021-02-15 7:30 ` Loic Poulain
2021-02-16 17:37 ` Bhaumik Bhatt
0 siblings, 1 reply; 5+ messages in thread
From: Loic Poulain @ 2021-02-15 7:30 UTC (permalink / raw)
To: Bhaumik Bhatt
Cc: Manivannan Sadhasivam, Hemant Kumar, linux-arm-msm, Jeffrey Hugo
Hi Bhaumik,
On Fri, 12 Feb 2021 at 02:41, Bhaumik Bhatt <bbhatt@codeaurora.org> wrote:
>
> 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>
[...]
> > +
> > err = mhi_sync_power_up(mhi_cntrl);
> > if (err) {
> > dev_err(&pdev->dev, "failed to power up MHI controller\n");
>
> Can you share logs of what you're seeing as it is not clear why you
> would need
> this patch.
>
> We have a mechanism in place that Jeff added a while back [1], to check
> if device
> is in SYS_ERROR state and do the same: issue reset and later, wait for
> ready from
> within mhi_sync_power_up() API.
>
> Note that the MHI_IN_PBL() macro includes EDL and Pass Through modes as
> well and
> we do expect an MHI READY state move after Pass Through.
I think this is a mix of several issues, that could be fixed by latest
Jeffrey's patch and
this one: "mhi: core: Move to polling method to wait for MHI ready".
I assume it would be easier if you send this last one as standalone
fix, for review and merge.
Regards,
Loic
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3] mhi: pci_generic: Ensure device readiness before starting MHI
2021-02-15 7:30 ` Loic Poulain
@ 2021-02-16 17:37 ` Bhaumik Bhatt
0 siblings, 0 replies; 5+ messages in thread
From: Bhaumik Bhatt @ 2021-02-16 17:37 UTC (permalink / raw)
To: Loic Poulain
Cc: Manivannan Sadhasivam, Hemant Kumar, linux-arm-msm, Jeffrey Hugo
On 2021-02-14 11:30 PM, Loic Poulain wrote:
> Hi Bhaumik,
>
> On Fri, 12 Feb 2021 at 02:41, Bhaumik Bhatt <bbhatt@codeaurora.org>
> wrote:
>>
>> 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>
> [...]
>> > +
>> > err = mhi_sync_power_up(mhi_cntrl);
>> > if (err) {
>> > dev_err(&pdev->dev, "failed to power up MHI controller\n");
>>
>> Can you share logs of what you're seeing as it is not clear why you
>> would need
>> this patch.
>>
>> We have a mechanism in place that Jeff added a while back [1], to
>> check
>> if device
>> is in SYS_ERROR state and do the same: issue reset and later, wait for
>> ready from
>> within mhi_sync_power_up() API.
>>
>> Note that the MHI_IN_PBL() macro includes EDL and Pass Through modes
>> as
>> well and
>> we do expect an MHI READY state move after Pass Through.
>
> I think this is a mix of several issues, that could be fixed by latest
> Jeffrey's patch and
> this one: "mhi: core: Move to polling method to wait for MHI ready".
>
> I assume it would be easier if you send this last one as standalone
> fix, for review and merge.
>
Will do.
> Regards,
> Loic
Thanks,
Bhaumik
---
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-02-16 17:37 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
2021-02-12 1:41 ` Bhaumik Bhatt
2021-02-15 7:30 ` Loic Poulain
2021-02-16 17:37 ` Bhaumik Bhatt
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).