答复: [PATCH] bus: mhi: core: Add support MHI EE FP for download firmware

[Carl Yin(殷张成)]

Hi Hemant and Jeffery:

On Wednesday, October 28, 2020 7:02 AM, hemantk wrote:
> Hi Jeff,
> 
> On 10/27/20 8:11 AM, Jeffrey Hugo wrote:
> > On 10/27/2020 3:43 AM, carl.yin@xxxxxxxxxxx wrote:
> >> From: "carl.yin" <carl.yin@xxxxxxxxxxx>
> >>
> >> MHI wwan modems support download firmware to nand or emmc by firehose
> >> protocol, process as next:
> >> 1. wwan modem normal bootup and enter EE AMSS, create mhi DIAG chan
> >> device 2. send EDL cmd via DIAG chan, then modem enter EE EDL 3.
> >> boot.c download 'firehose/prog_firehose_sdx55.mbn' via BHI interface
> >> 4. modem enter EE FP, and create mhi EDL chan device 5. user space
> >> tool download FW to modem via EDL chan by firehose protocol
> >>
> >> Signed-off-by: carl.yin <carl.yin@xxxxxxxxxxx>
> >> ---
> >>   drivers/bus/mhi/core/boot.c     |  4 +++-
> >>   drivers/bus/mhi/core/init.c     |  2 ++
> >>   drivers/bus/mhi/core/internal.h |  1 +
> >>   drivers/bus/mhi/core/main.c     |  3 +++
> >>   drivers/bus/mhi/core/pm.c       | 16 +++++++++++++++-
> >>   include/linux/mhi.h             |  4 +++-
> >>   6 files changed, 27 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/bus/mhi/core/boot.c
> >> b/drivers/bus/mhi/core/boot.c index 24422f5..ab39ad6 100644
> >> --- a/drivers/bus/mhi/core/boot.c
> >> +++ b/drivers/bus/mhi/core/boot.c
> >> @@ -460,8 +460,10 @@ void mhi_fw_load_handler(struct mhi_controller
> >> *mhi_cntrl)
> >>           return;
> >>       }
> >> -    if (mhi_cntrl->ee == MHI_EE_EDL)
> >> +    if (mhi_cntrl->ee == MHI_EE_EDL) {
> >> +        mhi_ready_state_transition(mhi_cntrl);
> >>           return;
> >> +    }
> >>       write_lock_irq(&mhi_cntrl->pm_lock);
> >>       mhi_cntrl->dev_state = MHI_STATE_RESET; diff --git
> >> a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c index
> >> ac4aa5c..9c2c2f3 100644
> >> --- a/drivers/bus/mhi/core/init.c
> >> +++ b/drivers/bus/mhi/core/init.c
> >> @@ -26,6 +26,7 @@ const char * const mhi_ee_str[MHI_EE_MAX] = {
> >>       [MHI_EE_WFW] = "WFW",
> >>       [MHI_EE_PTHRU] = "PASS THRU",
> >>       [MHI_EE_EDL] = "EDL",
> >> +    [MHI_EE_FP] = "FP",
> >>       [MHI_EE_DISABLE_TRANSITION] = "DISABLE",
> >>       [MHI_EE_NOT_SUPPORTED] = "NOT SUPPORTED",
> >>   };
> >> @@ -35,6 +36,7 @@ const char * const
> >> dev_state_tran_str[DEV_ST_TRANSITION_MAX] = {
> >>       [DEV_ST_TRANSITION_READY] = "READY",
> >>       [DEV_ST_TRANSITION_SBL] = "SBL",
> >>       [DEV_ST_TRANSITION_MISSION_MODE] = "MISSION_MODE",
> >> +    [DEV_ST_TRANSITION_FP] = "FP",
> >>       [DEV_ST_TRANSITION_SYS_ERR] = "SYS_ERR",
> >>       [DEV_ST_TRANSITION_DISABLE] = "DISABLE",
> >>   };
> >> diff --git a/drivers/bus/mhi/core/internal.h
> >> b/drivers/bus/mhi/core/internal.h index 4abf0cf..6ae897a 100644
> >> --- a/drivers/bus/mhi/core/internal.h
> >> +++ b/drivers/bus/mhi/core/internal.h
> >> @@ -386,6 +386,7 @@ enum dev_st_transition {
> >>       DEV_ST_TRANSITION_READY,
> >>       DEV_ST_TRANSITION_SBL,
> >>       DEV_ST_TRANSITION_MISSION_MODE,
> >> +    DEV_ST_TRANSITION_FP,
> >>       DEV_ST_TRANSITION_SYS_ERR,
> >>       DEV_ST_TRANSITION_DISABLE,
> >>       DEV_ST_TRANSITION_MAX,
> >> diff --git a/drivers/bus/mhi/core/main.c
> >> b/drivers/bus/mhi/core/main.c index 3950792..e307b58 100644
> >> --- a/drivers/bus/mhi/core/main.c
> >> +++ b/drivers/bus/mhi/core/main.c
> >> @@ -782,6 +782,9 @@ int mhi_process_ctrl_ev_ring(struct
> >> mhi_controller *mhi_cntrl,
> >>               case MHI_EE_SBL:
> >>                   st = DEV_ST_TRANSITION_SBL;
> >>                   break;
> >> +            case MHI_EE_FP:
> >> +                st = DEV_ST_TRANSITION_FP;
> >> +                break;
> >>               case MHI_EE_WFW:
> >>               case MHI_EE_AMSS:
> >>                   st = DEV_ST_TRANSITION_MISSION_MODE; diff
> --git
> >> a/drivers/bus/mhi/core/pm.c b/drivers/bus/mhi/core/pm.c index
> >> 3de7b16..3c95a5d 100644
> >> --- a/drivers/bus/mhi/core/pm.c
> >> +++ b/drivers/bus/mhi/core/pm.c
> >> @@ -563,7 +563,15 @@ static void mhi_pm_disable_transition(struct
> >> mhi_controller *mhi_cntrl,
> >>       }
> >>       if (cur_state == MHI_PM_SYS_ERR_PROCESS) {
> >> -        mhi_ready_state_transition(mhi_cntrl);
> >> +        if (mhi_get_exec_env(mhi_cntrl) == MHI_EE_EDL
> >> +            && mhi_get_mhi_state(mhi_cntrl) == MHI_STATE_RESET) {
> >> +            write_lock_irq(&mhi_cntrl->pm_lock);
> >> +            cur_state = mhi_tryset_pm_state(mhi_cntrl,
> MHI_PM_POR);
> >> +            write_unlock_irq(&mhi_cntrl->pm_lock);
> >> +            mhi_queue_state_transition(mhi_cntrl,
> >> DEV_ST_TRANSITION_PBL);
> >> +        } else {
> >> +            mhi_ready_state_transition(mhi_cntrl);
> >> +        }
> >>       } else {
> >>           /* Move to disable state */
> >>           write_lock_irq(&mhi_cntrl->pm_lock);
> >> @@ -658,6 +666,12 @@ void mhi_pm_st_worker(struct work_struct *work)
> >>           case DEV_ST_TRANSITION_MISSION_MODE:
> >>               mhi_pm_mission_mode_transition(mhi_cntrl);
> >>               break;
> >> +        case DEV_ST_TRANSITION_FP:
> >> +            write_lock_irq(&mhi_cntrl->pm_lock);
> >> +            mhi_cntrl->ee = MHI_EE_FP;
> >> +            write_unlock_irq(&mhi_cntrl->pm_lock);
> >> +            mhi_create_devices(mhi_cntrl);
> >> +            break;
> >>           case DEV_ST_TRANSITION_READY:
> >>               mhi_ready_state_transition(mhi_cntrl);
> >>               break;
> >> diff --git a/include/linux/mhi.h b/include/linux/mhi.h index
> >> 6e1122c..4620af8 100644
> >> --- a/include/linux/mhi.h
> >> +++ b/include/linux/mhi.h
> >> @@ -120,6 +120,7 @@ struct mhi_link_info {
> >>    * @MHI_EE_WFW: WLAN firmware mode
> >>    * @MHI_EE_PTHRU: Passthrough
> >>    * @MHI_EE_EDL: Embedded downloader
> >> + * @MHI_EE_FP, Flash Programmer Environment
> >>    */
> >>   enum mhi_ee_type {
> >>       MHI_EE_PBL,
> >> @@ -129,7 +130,8 @@ enum mhi_ee_type {
> >>       MHI_EE_WFW,
> >>       MHI_EE_PTHRU,
> >>       MHI_EE_EDL,
> >> -    MHI_EE_MAX_SUPPORTED = MHI_EE_EDL,
> >> +    MHI_EE_FP,
> >> +    MHI_EE_MAX_SUPPORTED = MHI_EE_FP,
> >>       MHI_EE_DISABLE_TRANSITION, /* local EE, not related to mhi spec
> >> */
> >>       MHI_EE_NOT_SUPPORTED,
> >>       MHI_EE_MAX,
> >>
> >
> > This gets a NACK from me.  I don't see the FP_EE that this patch
> > introduces defined in the spec.  Where did it come from?
> >
> There is indeed a FP EE, BHI spec will be updated with this EE next month.
> 
> Basically, once device goes to EDL, flash programmer image is downloaded using
> BHI protocol (same as we download SBL image using BHI from PBL in current use
> case). Once it is downloaded intvec sends EE change event for FP. Also event is
> generated for the same which is used to create EDL channels (34, 35) which is
> used by flash programmer to flash image for AMSS.
> 
[carl.yin] I am refer to QUALLCOMM's windows MHI driver.
And the patch work well on my SDX24&SDX55 modems.
I can use it to upgrade my modems.

>  >> 2. send EDL cmd via DIAG chan, then modem enter EE EDL
> #2 needs to be done in cleaner way. From AMSS when diag cmd is sent to switch
> to EDL, device would send SYS_ERR which we can use to do a call back to mhi
> controller to perform power down and power up. Instead of moving pm state to
> POR from disable transition :-
[carl.yin] when the MHI EE switch, from AMSS to SBL, or from AMSS to EDL.
I not sure if we will do mhi controller power down and power up.
But from my test result, it works well now.
Next is the mhi core log.
[66519.934318] mhi 0000:03:00.0: Chan: 4 successfully moved to start state
[66519.934324] mhi 0000:03:00.0: Preparing channel: 5
[66519.942675] mhi 0000:03:00.0: Chan: 5 successfully moved to start state
[66521.817279] mhi 0000:03:00.0: local ee:EDL device ee:AMSS dev_state:SYS_ERR
[66521.817284] mhi 0000:03:00.0: System error detected
[66521.817304] mhi 0000:03:00.0: Handling state transition: SYS_ERR
[66521.817308] mhi 0000:03:00.0: Transitioning from PM state: SYS_ERR Detect to: SYS_ERR Process
[66521.817311] mhi 0000:03:00.0: Triggering MHI Reset in device
[66521.817451] mhi 0000:03:00.0: local ee:EDL device ee:DISABLE dev_state:RESET
[66521.817469] mhi 0000:03:00.0: Waiting for all pending event ring processing to complete
[66521.817472] mhi 0000:03:00.0: Waiting for all pending threads to complete
[66521.817474] mhi 0000:03:00.0: Reset all active channels and remove MHI devices
 [66521.817777] mhi 0000:03:00.0: destroy device for chan:DIAG
[66521.817809] mhi 0000:03:00.0: Marking all events for chan: 5 as stale
[66521.817812] mhi 0000:03:00.0: Finished marking events as stale events
[66521.817857] mhi 0000:03:00.0: Marking all events for chan: 4 as stale
[66521.817860] mhi 0000:03:00.0: Finished marking events as stale events
[66521.818036] mhi 0000:03:00.0: destroy device for chan:MBIM
[66521.818062] mhi 0000:03:00.0: Marking all events for chan: 13 as stale
[66521.818064] mhi 0000:03:00.0: Finished marking events as stale events
[66521.818066] mhi 0000:03:00.0: Marking all events for chan: 12 as stale
[66521.818069] mhi 0000:03:00.0: Finished marking events as stale events
[66521.818471] mhi 0000:03:00.0: destroy device for chan:IP_HW0_MBIM
[66521.818497] mhi 0000:03:00.0: Marking all events for chan: 101 as stale
[66521.818500] mhi 0000:03:00.0: Finished marking events as stale events
[66521.818503] mhi 0000:03:00.0: Marking all events for chan: 100 as stale
[66521.818505] mhi 0000:03:00.0: Finished marking events as stale events
[66521.855633] mhi 0000:03:00.0: Entered: unprepare channel:101
[66521.855635] mhi 0000:03:00.0: Entered: unprepare channel:100
[66521.855669] mhi 0000:03:00.0: Resetting EV CTXT and CMD CTXT
[66521.855678] mhi 0000:03:00.0: Exiting with PM state: POR, MHI state: RESET
[66521.855679] mhi 0000:03:00.0: Handling state transition: PBL
[66521.855848] mhi 0000:03:00.0: Starting SBL download via BHI. Session ID:703272956
[66521.878480] mhi 0000:03:00.0: local ee:EDL device ee:EDL dev_state:RESET
[66522.902542] mhi 0000:03:00.0: local ee:FP device ee:EDL dev_state:READY
[66522.902563] mhi 0000:03:00.0: Device in READY State
[66522.902566] mhi 0000:03:00.0: Initializing MHI registers
[66522.906563] mhi 0000:03:00.0: State change event to state: M0
[66522.907481] mhi 0000:03:00.0: Received EE event: FP
[66522.907530] mhi 0000:03:00.0: Handling state transition: FP
[66523.818829] mhi 0000:03:00.0: Preparing channel: 34
[66523.822219] mhi 0000:03:00.0: Chan: 34 successfully moved to start state
[66523.822225] mhi 0000:03:00.0: Preparing channel: 35
[66523.825642] mhi 0000:03:00.0: Chan: 35 successfully moved to start state

> 
> @@ -563,7 +563,15 @@ static void mhi_pm_disable_transition(struct  >>
> mhi_controller *mhi_cntrl,
>  >>       }
>  >>       if (cur_state == MHI_PM_SYS_ERR_PROCESS) {
>  >> -        mhi_ready_state_transition(mhi_cntrl);
>  >> +        if (mhi_get_exec_env(mhi_cntrl) == MHI_EE_EDL
>  >> +            && mhi_get_mhi_state(mhi_cntrl) == MHI_STATE_RESET) {
>  >> +            write_lock_irq(&mhi_cntrl->pm_lock);
>  >> +            cur_state = mhi_tryset_pm_state(mhi_cntrl, MHI_PM_POR);
>  >> +            write_unlock_irq(&mhi_cntrl->pm_lock);
>  >> +            mhi_queue_state_transition(mhi_cntrl,
>  >> DEV_ST_TRANSITION_PBL);
>  >> +        } else {
>  >> +            mhi_ready_state_transition(mhi_cntrl);
>  >> +        }
> 
> Thanks,
> Hemant
> 
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a
> Linux Foundation Collaborative Project