Re: [PATCH 1/1] bus: mhi: host: allow SBL as initial EE

[Daniele Palmas <dnlplm@gmail.com>]

Hi Mani,

Il giorno mar 30 mag 2023 alle ore 12:31 Manivannan Sadhasivam
<mani@kernel.org> ha scritto:
>
> On Tue, May 30, 2023 at 11:13:40AM +0200, Daniele Palmas wrote:
> > There are situations in which SBL is a legitimate initial execution
> > environment (e.g. modem stuck in SBL due to a firmware failure...), but
> > mhi refuses to start:
> >
> > mhi-pci-generic 0000:01:00.0: MHI PCI device found: foxconn-sdx55
> > mhi-pci-generic 0000:01:00.0: BAR 0: assigned
> > mhi-pci-generic 0000:01:00.0: enabling device (0000 -> 0002)
> > mhi mhi0: Requested to power ON
> > mhi mhi0: SECONDARY BOOTLOADER is not a valid EE for power on
> > mhi-pci-generic 0000:01:00.0: failed to power up MHI controller
> > mhi-pci-generic: probe of 0000:01:00.0 failed with error -5
> >
> > Fix this by adding SBL as an allowed initial execution environment.
> >
>
> What can you do with the modem when firmware failure happens? If there is a
> usecase, please explain.

(removing Siddartha and Sujeev due to addresses not working)

A possible scenario for a Telit modem not being able to go to mission
mode is when a firmware update does not work properly: in this case it
remains stuck in SBL, but the SAHARA device can be used for retrying
the firmware update.

Telit FN990 supports the SAHARA channels in pci_generic. It's true
that there's still missing the exposed device for userspace, something
that we are currently managing with out of tree patches, but I see
that there's some ongoing effort for that
https://lore.kernel.org/mhi/20230522190459.13790-1-quic_jhugo@quicinc.com/

I'm not sure if non-Telit modems have other reasonable use-cases.

Regards,
Daniele

>
> - Mani
>
> > Fixes: 3000f85b8f47 ("bus: mhi: core: Add support for basic PM operations")
> > Signed-off-by: Daniele Palmas <dnlplm@gmail.com>
> > ---
> >  drivers/bus/mhi/host/internal.h | 2 +-
> >  drivers/bus/mhi/host/pm.c       | 3 ++-
> >  2 files changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/bus/mhi/host/internal.h b/drivers/bus/mhi/host/internal.h
> > index 2e139e76de4c..3bdcd2321aa5 100644
> > --- a/drivers/bus/mhi/host/internal.h
> > +++ b/drivers/bus/mhi/host/internal.h
> > @@ -56,7 +56,7 @@ extern const char * const mhi_ee_str[MHI_EE_MAX];
> >
> >  #define MHI_IN_PBL(ee) (ee == MHI_EE_PBL || ee == MHI_EE_PTHRU || \
> >                       ee == MHI_EE_EDL)
> > -#define MHI_POWER_UP_CAPABLE(ee) (MHI_IN_PBL(ee) || ee == MHI_EE_AMSS)
> > +#define MHI_POWER_UP_CAPABLE(ee) (MHI_IN_PBL(ee) || ee == MHI_EE_AMSS || ee == MHI_EE_SBL)
> >  #define MHI_FW_LOAD_CAPABLE(ee) (ee == MHI_EE_PBL || ee == MHI_EE_EDL)
> >  #define MHI_IN_MISSION_MODE(ee) (ee == MHI_EE_AMSS || ee == MHI_EE_WFW || \
> >                                ee == MHI_EE_FP)
> > diff --git a/drivers/bus/mhi/host/pm.c b/drivers/bus/mhi/host/pm.c
> > index 083459028a4b..18872c5017be 100644
> > --- a/drivers/bus/mhi/host/pm.c
> > +++ b/drivers/bus/mhi/host/pm.c
> > @@ -1203,10 +1203,11 @@ int mhi_sync_power_up(struct mhi_controller *mhi_cntrl)
> >
> >       wait_event_timeout(mhi_cntrl->state_event,
> >                          MHI_IN_MISSION_MODE(mhi_cntrl->ee) ||
> > +                        mhi_cntrl->ee == MHI_EE_SBL ||
> >                          MHI_PM_IN_ERROR_STATE(mhi_cntrl->pm_state),
> >                          msecs_to_jiffies(mhi_cntrl->timeout_ms));
> >
> > -     ret = (MHI_IN_MISSION_MODE(mhi_cntrl->ee)) ? 0 : -ETIMEDOUT;
> > +     ret = (MHI_IN_MISSION_MODE(mhi_cntrl->ee) || mhi_cntrl->ee == MHI_EE_SBL) ? 0 : -ETIMEDOUT;
> >       if (ret)
> >               mhi_power_down(mhi_cntrl, false);
> >
> > --
> > 2.37.1
> >
>
> --
> மணிவண்ணன் சதாசிவம்