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

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

[Daniele Palmas]

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.

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

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

[Manivannan Sadhasivam]

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.

- 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
> 

-- 
மணிவண்ணன் சதாசிவம்

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

[Daniele Palmas]

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
> >
>
> --
> மணிவண்ணன் சதாசிவம்

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

[Manivannan Sadhasivam]

On Tue, May 30, 2023 at 01:12:59PM +0200, Daniele Palmas wrote:
> 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)
> 

Both left Qualcomm a while ago...

> 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.
> 

So IIUC, while updating SBL over SAHARA channel, the firmware update could be
corrupted and the device would get stuck in SBL EE. In that case, the SAHARA
channel exposed by PBL will still be open and that could be used to retry the
firmware update. Am I right?

Looks like PBL is doing a fail safe upgrade!

> 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.
> 

If my above understanding is correct, then this patch will benefit other
platforms too.

- Mani

> 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
> > >
> >
> > --
> > மணிவண்ணன் சதாசிவம்

-- 
மணிவண்ணன் சதாசிவம்

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

[Daniele Palmas]

Il giorno mar 30 mag 2023 alle ore 15:56 Manivannan Sadhasivam
<mani@kernel.org> ha scritto:
>
> On Tue, May 30, 2023 at 01:12:59PM +0200, Daniele Palmas wrote:
> > 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)
> >
>
> Both left Qualcomm a while ago...
>
> > 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.
> >
>
> So IIUC, while updating SBL over SAHARA channel, the firmware update could be
> corrupted and the device would get stuck in SBL EE. In that case, the SAHARA
> channel exposed by PBL will still be open and that could be used to retry the
> firmware update. Am I right?
>
> Looks like PBL is doing a fail safe upgrade!
>

Yes, the scenario is a broken firmware update.

Telit has implemented custom code in the SBL for allowing the firmware
update through a custom protocol using the SAHARA channels: this has
been mainly done to avoid sharing with customers Qualcomm IP about
firmware update, since it is usually problematic.

Firmware update is fail safe in the sense that it is guaranteed that
the modem will always power-up at least in SBL, in which the firmware
update can always be retried. I don't have the exact details of what
is happening inside the modem, but, if required, I can get them.

> > 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.
> >
>
> If my above understanding is correct, then this patch will benefit other
> platforms too.
>

Well... I can't comment on that, since I don't know if the above
described behavior is due to Telit custom SBL.

Anyway, maybe there could be other scenarios like, for example, modem
falling into core dump mode in a very early stage? Not sure...

Regards,
Daniele

> - Mani
>
> > 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
> > > >
> > >
> > > --
> > > மணிவண்ணன் சதாசிவம்
>
> --
> மணிவண்ணன் சதாசிவம்

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

[Manivannan Sadhasivam]

On Wed, May 31, 2023 at 01:44:24PM +0200, Daniele Palmas wrote:
> Il giorno mar 30 mag 2023 alle ore 15:56 Manivannan Sadhasivam
> <mani@kernel.org> ha scritto:
> >
> > On Tue, May 30, 2023 at 01:12:59PM +0200, Daniele Palmas wrote:
> > > 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)
> > >
> >
> > Both left Qualcomm a while ago...
> >
> > > 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.
> > >
> >
> > So IIUC, while updating SBL over SAHARA channel, the firmware update could be
> > corrupted and the device would get stuck in SBL EE. In that case, the SAHARA
> > channel exposed by PBL will still be open and that could be used to retry the
> > firmware update. Am I right?
> >
> > Looks like PBL is doing a fail safe upgrade!
> >
> 
> Yes, the scenario is a broken firmware update.
> 
> Telit has implemented custom code in the SBL for allowing the firmware
> update through a custom protocol using the SAHARA channels: this has
> been mainly done to avoid sharing with customers Qualcomm IP about
> firmware update, since it is usually problematic.
> 
> Firmware update is fail safe in the sense that it is guaranteed that
> the modem will always power-up at least in SBL, in which the firmware
> update can always be retried. I don't have the exact details of what
> is happening inside the modem, but, if required, I can get them.
> 

Okay, I didn't now that it is a custom SBL, so yeah it may not benefit other
platforms. But what is the userspace tool that you are using to download
firmware over SAHARA? Can QDL be used?

- Mani

> > > 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.
> > >
> >
> > If my above understanding is correct, then this patch will benefit other
> > platforms too.
> >
> 
> Well... I can't comment on that, since I don't know if the above
> described behavior is due to Telit custom SBL.
> 
> Anyway, maybe there could be other scenarios like, for example, modem
> falling into core dump mode in a very early stage? Not sure...
> 
> Regards,
> Daniele
> 
> > - Mani
> >
> > > 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
> > > > >
> > > >
> > > > --
> > > > மணிவண்ணன் சதாசிவம்
> >
> > --
> > மணிவண்ணன் சதாசிவம்

-- 
மணிவண்ணன் சதாசிவம்

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

[Jeffrey Hugo]

On 5/31/2023 7:01 AM, Manivannan Sadhasivam wrote:
> On Wed, May 31, 2023 at 01:44:24PM +0200, Daniele Palmas wrote:
>> Il giorno mar 30 mag 2023 alle ore 15:56 Manivannan Sadhasivam
>> <mani@kernel.org> ha scritto:
>>>
>>> On Tue, May 30, 2023 at 01:12:59PM +0200, Daniele Palmas wrote:
>>>> 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)
>>>>
>>>
>>> Both left Qualcomm a while ago...
>>>
>>>> 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.
>>>>
>>>
>>> So IIUC, while updating SBL over SAHARA channel, the firmware update could be
>>> corrupted and the device would get stuck in SBL EE. In that case, the SAHARA
>>> channel exposed by PBL will still be open and that could be used to retry the
>>> firmware update. Am I right?
>>>
>>> Looks like PBL is doing a fail safe upgrade!
>>>
>>
>> Yes, the scenario is a broken firmware update.
>>
>> Telit has implemented custom code in the SBL for allowing the firmware
>> update through a custom protocol using the SAHARA channels: this has
>> been mainly done to avoid sharing with customers Qualcomm IP about
>> firmware update, since it is usually problematic.
>>
>> Firmware update is fail safe in the sense that it is guaranteed that
>> the modem will always power-up at least in SBL, in which the firmware
>> update can always be retried. I don't have the exact details of what
>> is happening inside the modem, but, if required, I can get them.
>>
> 
> Okay, I didn't now that it is a custom SBL, so yeah it may not benefit other
> platforms. But what is the userspace tool that you are using to download
> firmware over SAHARA? Can QDL be used?

Yeah, this looks like a weird quirk to me.  The device should never 
"start" in SBL.  AIC100 has a weird flow where it can be there 
(interrupted flashless boot, remove driver, modprobe driver), but that 
gets handled in the QAIC driver.

Since this for flash loading, I would expect EDL or FP EEs to be used. 
It sounds like Telit has integrated a flash programmer into the SBL 
instead of having it be loaded via EDL.  That is not inherently invalid, 
but the device should still mimic the right flows instead of inventing 
its own.

> 
> - Mani
> 
>>>> 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.
>>>>
>>>
>>> If my above understanding is correct, then this patch will benefit other
>>> platforms too.
>>>
>>
>> Well... I can't comment on that, since I don't know if the above
>> described behavior is due to Telit custom SBL.
>>
>> Anyway, maybe there could be other scenarios like, for example, modem
>> falling into core dump mode in a very early stage? Not sure...
>>
>> Regards,
>> Daniele
>>
>>> - Mani
>>>
>>>> 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
>>>>>>
>>>>>
>>>>> --
>>>>> மணிவண்ணன் சதாசிவம்
>>>
>>> --
>>> மணிவண்ணன் சதாசிவம்
> 

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

[Daniele Palmas]

Il giorno mer 31 mag 2023 alle ore 15:01 Manivannan Sadhasivam
<mani@kernel.org> ha scritto:
>
> On Wed, May 31, 2023 at 01:44:24PM +0200, Daniele Palmas wrote:
> > Il giorno mar 30 mag 2023 alle ore 15:56 Manivannan Sadhasivam
> > <mani@kernel.org> ha scritto:
> > >
> > > On Tue, May 30, 2023 at 01:12:59PM +0200, Daniele Palmas wrote:
> > > > 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)
> > > >
> > >
> > > Both left Qualcomm a while ago...
> > >
> > > > 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.
> > > >
> > >
> > > So IIUC, while updating SBL over SAHARA channel, the firmware update could be
> > > corrupted and the device would get stuck in SBL EE. In that case, the SAHARA
> > > channel exposed by PBL will still be open and that could be used to retry the
> > > firmware update. Am I right?
> > >
> > > Looks like PBL is doing a fail safe upgrade!
> > >
> >
> > Yes, the scenario is a broken firmware update.
> >
> > Telit has implemented custom code in the SBL for allowing the firmware
> > update through a custom protocol using the SAHARA channels: this has
> > been mainly done to avoid sharing with customers Qualcomm IP about
> > firmware update, since it is usually problematic.
> >
> > Firmware update is fail safe in the sense that it is guaranteed that
> > the modem will always power-up at least in SBL, in which the firmware
> > update can always be retried. I don't have the exact details of what
> > is happening inside the modem, but, if required, I can get them.
> >
>
> Okay, I didn't now that it is a custom SBL, so yeah it may not benefit other
> platforms. But what is the userspace tool that you are using to download
> firmware over SAHARA?

The userspace tool is not public at the moment, but will check about
the plans for open-sourcing.

> Can QDL be used?

I will check that with firmware developers.

Reading also Jeffrey's reply, since this looks like a Telit modem
quirk, I'm wondering if it could make sense to deal with this again
once the quirk framework at
https://lore.kernel.org/mhi/20230519163902.4170-2-quic_jhugo@quicinc.com/T/#u
gets merged, otherwise I guess we can continue living with custom
patches.

Thanks,
Daniele

> - Mani
>
> > > > 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.
> > > >
> > >
> > > If my above understanding is correct, then this patch will benefit other
> > > platforms too.
> > >
> >
> > Well... I can't comment on that, since I don't know if the above
> > described behavior is due to Telit custom SBL.
> >
> > Anyway, maybe there could be other scenarios like, for example, modem
> > falling into core dump mode in a very early stage? Not sure...
> >
> > Regards,
> > Daniele
> >
> > > - Mani
> > >
> > > > 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
> > > > > >
> > > > >
> > > > > --
> > > > > மணிவண்ணன் சதாசிவம்
> > >
> > > --
> > > மணிவண்ணன் சதாசிவம்
>
> --
> மணிவண்ணன் சதாசிவம்