On Thu, 11 Feb 2021 at 18:13, Jeffrey Hugo <jhugo@xxxxxxxxxxxxxx> wrote:
>
> On 2/11/2021 10:08 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@xxxxxxxxxx>
> > ---
> > drivers/bus/mhi/pci_generic.c | 23 +++++++++++++++++++++++
> > 1 file changed, 23 insertions(+)
> >
> > diff --git a/drivers/bus/mhi/pci_generic.c b/drivers/bus/mhi/pci_generic.c
> > index c20f59e..bfa0a1e 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
> > @@ -391,6 +393,22 @@ 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 = 200;
> > +
> > + mhi_pci_write_reg(mhi_cntrl, mhi_cntrl->regs + MHICTRL,
> > + MHICTRL_RESET_MASK);
> > +
> > + while (mhi_get_mhi_state(mhi_cntrl) != MHI_STATE_READY) {
> > + if (!max_wait_ready--) {
> > + dev_warn(mhi_cntrl->cntrl_dev, "Not ready\n");
> > + 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 +469,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 +553,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");
> >
>
> So, I'm curious, how does this actually work?
>
> From what I can see, you define SBL images. If those get loaded by the
> PBL, it doesn't happen over MHI. PBL will not move MHI to ready state,
> except in the specific instance of a fatal error.
I defined generic SBL images for flashless controller versions, but
mine is not, and so it boots directly in mission mode.
>
> Your above change works if the device comes up straight in mission mode
> (AMSS), but if it comes up in PBL, you are going to hit the timeout and
> dev_warn() every time.
Ok, I thought we should get into MHI ready state, whatever the
'execution environment'... So I definitely need to take that into
consideration. thanks.
Regards,
Loic
