diff mbox series

[v2] bus: mhi: core: Add support MHI EE FP for download firmware

Message ID 20201102122756.23452-1-carl.yin@quectel.com (mailing list archive)
State New, archived
Headers show
Series [v2] bus: mhi: core: Add support MHI EE FP for download firmware | expand

Commit Message

Carl Yin(殷张成) Nov. 2, 2020, 12:27 p.m. UTC 
From: "carl.yin" <carl.yin@quectel.com>

MHI wwan modems support download firmware to nand or emmc
by firehose protocol, process as next:
1. modem boot up and enter EE AMSS, create DIAG channels (4, 5) device
2. user space tool send EDL command via DIAG channel,
   then modem enter EE EDL
3. boot.c download 'flash programmer image' via BHI interface
4. modem enter EE FP, and create EDL channels (34, 35) device
5. user space tool download 'firmware image' to modem via EDL channels
   by firehose protocol

Signed-off-by: carl.yin <carl.yin@quectel.com>
---
 drivers/bus/mhi/core/init.c     |  2 ++
 drivers/bus/mhi/core/internal.h |  1 +
 drivers/bus/mhi/core/main.c     |  5 ++++-
 drivers/bus/mhi/core/pm.c       | 13 ++++++++++++-
 include/linux/mhi.h             |  4 +++-
 5 files changed, 22 insertions(+), 3 deletions(-)

Comments

Bhaumik Bhatt Nov. 2, 2020, 4:34 p.m. UTC | #1 
On 2020-11-02 04:27, carl.yin@quectel.com wrote:
> From: "carl.yin" <carl.yin@quectel.com>
> 
> MHI wwan modems support download firmware to nand or emmc
> by firehose protocol, process as next:
> 1. modem boot up and enter EE AMSS, create DIAG channels (4, 5) device
> 2. user space tool send EDL command via DIAG channel,
>    then modem enter EE EDL
> 3. boot.c download 'flash programmer image' via BHI interface
> 4. modem enter EE FP, and create EDL channels (34, 35) device
> 5. user space tool download 'firmware image' to modem via EDL channels
>    by firehose protocol
> 
> Signed-off-by: carl.yin <carl.yin@quectel.com>
> ---
>  drivers/bus/mhi/core/init.c     |  2 ++
>  drivers/bus/mhi/core/internal.h |  1 +
>  drivers/bus/mhi/core/main.c     |  5 ++++-
>  drivers/bus/mhi/core/pm.c       | 13 ++++++++++++-
>  include/linux/mhi.h             |  4 +++-
>  5 files changed, 22 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c
> index ac4aa5c..e34616b 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] = "FLASH PROGRAMMER",
>  	[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] = "FLASH_PROGRAMMER",
>  	[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..a1e1561 100644
> --- a/drivers/bus/mhi/core/main.c
> +++ b/drivers/bus/mhi/core/main.c
> @@ -422,7 +422,7 @@ irqreturn_t mhi_intvec_threaded_handler(int
> irq_number, void *priv)
>  		wake_up_all(&mhi_cntrl->state_event);
> 
>  		/* For fatal errors, we let controller decide next step */
> -		if (MHI_IN_PBL(ee))
> +		if (MHI_IN_PBL(mhi_cntrl->ee))
Let's please have this as a separate patch with a fixes tag, as it fixes 
a
pre-existing bug. I am sure Mani would want this.
>  			mhi_cntrl->status_cb(mhi_cntrl, MHI_CB_FATAL_ERROR);
>  		else
>  			mhi_pm_sys_err_handler(mhi_cntrl);
> @@ -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;
When do you get this EE event on the control event ring? Does it come by 
after you
have detected EE as FP from mhi_sync_power_up() and move to ready and 
then M0?
>  			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..2d68812 100644
> --- a/drivers/bus/mhi/core/pm.c
> +++ b/drivers/bus/mhi/core/pm.c
> @@ -658,6 +658,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;
> @@ -1077,10 +1083,15 @@ 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_FP ||
>  			   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;
> +	if (mhi_cntrl->ee == MHI_EE_FP)
> +		mhi_queue_state_transition(mhi_cntrl, DEV_ST_TRANSITION_READY);
> +	else
> +		ret = (MHI_IN_MISSION_MODE(mhi_cntrl->ee)) ? 0 : -ETIMEDOUT;
> +
>  	if (ret)
>  		mhi_power_down(mhi_cntrl, false);
> 
We should come up with a better design for this later on.
> 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,

Thanks,
Bhaumik
Manivannan Sadhasivam Nov. 16, 2020, 6:08 a.m. UTC | #2 
On Mon, Nov 02, 2020 at 08:34:14AM -0800, Bhaumik Bhatt wrote:
> On 2020-11-02 04:27, carl.yin@quectel.com wrote:
> > From: "carl.yin" <carl.yin@quectel.com>
> > 
> > MHI wwan modems support download firmware to nand or emmc
> > by firehose protocol, process as next:
> > 1. modem boot up and enter EE AMSS, create DIAG channels (4, 5) device
> > 2. user space tool send EDL command via DIAG channel,
> >    then modem enter EE EDL
> > 3. boot.c download 'flash programmer image' via BHI interface
> > 4. modem enter EE FP, and create EDL channels (34, 35) device
> > 5. user space tool download 'firmware image' to modem via EDL channels
> >    by firehose protocol
> > 
> > Signed-off-by: carl.yin <carl.yin@quectel.com>
> > ---
> >  drivers/bus/mhi/core/init.c     |  2 ++
> >  drivers/bus/mhi/core/internal.h |  1 +
> >  drivers/bus/mhi/core/main.c     |  5 ++++-
> >  drivers/bus/mhi/core/pm.c       | 13 ++++++++++++-
> >  include/linux/mhi.h             |  4 +++-
> >  5 files changed, 22 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c
> > index ac4aa5c..e34616b 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] = "FLASH PROGRAMMER",
> >  	[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] = "FLASH_PROGRAMMER",
> >  	[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..a1e1561 100644
> > --- a/drivers/bus/mhi/core/main.c
> > +++ b/drivers/bus/mhi/core/main.c
> > @@ -422,7 +422,7 @@ irqreturn_t mhi_intvec_threaded_handler(int
> > irq_number, void *priv)
> >  		wake_up_all(&mhi_cntrl->state_event);
> > 
> >  		/* For fatal errors, we let controller decide next step */
> > -		if (MHI_IN_PBL(ee))
> > +		if (MHI_IN_PBL(mhi_cntrl->ee))
> Let's please have this as a separate patch with a fixes tag, as it fixes a
> pre-existing bug. I am sure Mani would want this.

Yes. It is not recommended to club changes like this onto a single patch.

Thanks,
Mani

> >  			mhi_cntrl->status_cb(mhi_cntrl, MHI_CB_FATAL_ERROR);
> >  		else
> >  			mhi_pm_sys_err_handler(mhi_cntrl);
> > @@ -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;
> When do you get this EE event on the control event ring? Does it come by
> after you
> have detected EE as FP from mhi_sync_power_up() and move to ready and then
> M0?
> >  			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..2d68812 100644
> > --- a/drivers/bus/mhi/core/pm.c
> > +++ b/drivers/bus/mhi/core/pm.c
> > @@ -658,6 +658,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;
> > @@ -1077,10 +1083,15 @@ 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_FP ||
> >  			   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;
> > +	if (mhi_cntrl->ee == MHI_EE_FP)
> > +		mhi_queue_state_transition(mhi_cntrl, DEV_ST_TRANSITION_READY);
> > +	else
> > +		ret = (MHI_IN_MISSION_MODE(mhi_cntrl->ee)) ? 0 : -ETIMEDOUT;
> > +
> >  	if (ret)
> >  		mhi_power_down(mhi_cntrl, false);
> > 
> We should come up with a better design for this later on.
> > 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,
> 
> Thanks,
> Bhaumik
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
diff mbox series

Patch

diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c
index ac4aa5c..e34616b 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] = "FLASH PROGRAMMER",
 	[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] = "FLASH_PROGRAMMER",
 	[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..a1e1561 100644
--- a/drivers/bus/mhi/core/main.c
+++ b/drivers/bus/mhi/core/main.c
@@ -422,7 +422,7 @@  irqreturn_t mhi_intvec_threaded_handler(int irq_number, void *priv)
 		wake_up_all(&mhi_cntrl->state_event);
 
 		/* For fatal errors, we let controller decide next step */
-		if (MHI_IN_PBL(ee))
+		if (MHI_IN_PBL(mhi_cntrl->ee))
 			mhi_cntrl->status_cb(mhi_cntrl, MHI_CB_FATAL_ERROR);
 		else
 			mhi_pm_sys_err_handler(mhi_cntrl);
@@ -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..2d68812 100644
--- a/drivers/bus/mhi/core/pm.c
+++ b/drivers/bus/mhi/core/pm.c
@@ -658,6 +658,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;
@@ -1077,10 +1083,15 @@  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_FP ||
 			   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;
+	if (mhi_cntrl->ee == MHI_EE_FP)
+		mhi_queue_state_transition(mhi_cntrl, DEV_ST_TRANSITION_READY);
+	else
+		ret = (MHI_IN_MISSION_MODE(mhi_cntrl->ee)) ? 0 : -ETIMEDOUT;
+
 	if (ret)
 		mhi_power_down(mhi_cntrl, false);
 
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,