* [PATCH] bus: mhi: core: Add support MHI EE FP for download firmware
@ 2020-10-27 9:43 carl.yin
2020-10-27 15:11 ` Jeffrey Hugo
0 siblings, 1 reply; 11+ messages in thread
From: carl.yin @ 2020-10-27 9:43 UTC (permalink / raw)
To: manivannan.sadhasivam, hemantk, sfr
Cc: linux-arm-msm, linux-kernel, carl.yin, naveen.kumar
From: "carl.yin" <carl.yin@quectel.com>
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@quectel.com>
---
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,
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] bus: mhi: core: Add support MHI EE FP for download firmware
2020-10-27 9:43 [PATCH] bus: mhi: core: Add support MHI EE FP for download firmware carl.yin
@ 2020-10-27 15:11 ` Jeffrey Hugo
2020-10-27 23:01 ` Hemant Kumar
0 siblings, 1 reply; 11+ messages in thread
From: Jeffrey Hugo @ 2020-10-27 15:11 UTC (permalink / raw)
To: carl.yin, manivannan.sadhasivam, hemantk, sfr
Cc: linux-arm-msm, linux-kernel, naveen.kumar
On 10/27/2020 3:43 AM, 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. 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@quectel.com>
> ---
> 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?
--
Jeffrey Hugo
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] bus: mhi: core: Add support MHI EE FP for download firmware
2020-10-27 15:11 ` Jeffrey Hugo
@ 2020-10-27 23:01 ` Hemant Kumar
2020-10-28 1:39 ` 答复: " Carl Yin(殷张成)
2020-10-28 14:19 ` Jeffrey Hugo
0 siblings, 2 replies; 11+ messages in thread
From: Hemant Kumar @ 2020-10-27 23:01 UTC (permalink / raw)
To: Jeffrey Hugo, carl.yin, manivannan.sadhasivam, sfr
Cc: linux-arm-msm, linux-kernel, naveen.kumar
Hi Jeff,
On 10/27/20 8:11 AM, Jeffrey Hugo wrote:
> On 10/27/2020 3:43 AM, 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. 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@quectel.com>
>> ---
>> 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.
>> 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 :-
@@ -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
^ permalink raw reply [flat|nested] 11+ messages in thread
* 答复: [PATCH] bus: mhi: core: Add support MHI EE FP for download firmware
2020-10-27 23:01 ` Hemant Kumar
@ 2020-10-28 1:39 ` Carl Yin(殷张成)
2020-10-28 14:24 ` Jeffrey Hugo
2020-10-28 14:19 ` Jeffrey Hugo
1 sibling, 1 reply; 11+ messages in thread
From: Carl Yin(殷张成) @ 2020-10-28 1:39 UTC (permalink / raw)
To: Hemant Kumar, Jeffrey Hugo, manivannan.sadhasivam, sfr
Cc: linux-arm-msm, linux-kernel, Naveen Kumar
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@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. 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@quectel.com>
> >> ---
> >> 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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] bus: mhi: core: Add support MHI EE FP for download firmware
2020-10-27 23:01 ` Hemant Kumar
2020-10-28 1:39 ` 答复: " Carl Yin(殷张成)
@ 2020-10-28 14:19 ` Jeffrey Hugo
1 sibling, 0 replies; 11+ messages in thread
From: Jeffrey Hugo @ 2020-10-28 14:19 UTC (permalink / raw)
To: Hemant Kumar, carl.yin, manivannan.sadhasivam, sfr
Cc: linux-arm-msm, linux-kernel, naveen.kumar
On 10/27/2020 5:01 PM, Hemant Kumar wrote:
> Hi Jeff,
>
> On 10/27/20 8:11 AM, Jeffrey Hugo wrote:
>> On 10/27/2020 3:43 AM, 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. 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@quectel.com>
>>> ---
>>> 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.
<sigh> Alright I withdraw my nack based on your word.
> 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.
Intresting. Seems simple enough.
>
> >> 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 :-
This seems sane to me. I'll review v2 when it gets posted.
>
> @@ -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
>
--
Jeffrey Hugo
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: 答复: [PATCH] bus: mhi: core: Add support MHI EE FP for download firmware
2020-10-28 1:39 ` 答复: " Carl Yin(殷张成)
@ 2020-10-28 14:24 ` Jeffrey Hugo
2020-10-29 1:12 ` 答复: " Carl Yin(殷张成)
0 siblings, 1 reply; 11+ messages in thread
From: Jeffrey Hugo @ 2020-10-28 14:24 UTC (permalink / raw)
To: Carl Yin(殷张成),
Hemant Kumar, manivannan.sadhasivam, sfr
Cc: linux-arm-msm, linux-kernel, Naveen Kumar
On 10/27/2020 7:39 PM, Carl Yin(殷张成) wrote:
> 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@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. 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@quectel.com>
>>>> ---
>>>> 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.
Linaro has an opensource utility that implements firehose -
https://github.com/andersson/qdl
Did you test with that?
--
Jeffrey Hugo
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply [flat|nested] 11+ messages in thread
* 答复: 答复: [PATCH] bus: mhi: core: Add support MHI EE FP for download firmware
2020-10-28 14:24 ` Jeffrey Hugo
@ 2020-10-29 1:12 ` Carl Yin(殷张成)
2020-10-29 14:20 ` Jeffrey Hugo
0 siblings, 1 reply; 11+ messages in thread
From: Carl Yin(殷张成) @ 2020-10-29 1:12 UTC (permalink / raw)
To: Jeffrey Hugo, Hemant Kumar, manivannan.sadhasivam, sfr
Cc: linux-arm-msm, linux-kernel, Naveen Kumar
Hi Jeffery:
On Wednesday, October 28, 2020 10:24 PM, jhugo wrote:
> On 10/27/2020 7:39 PM, Carl Yin(殷张成) wrote:
> > 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@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. 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@quectel.com>
> >>>> ---
> >>>> 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.
>
> Linaro has an opensource utility that implements firehose -
> https://github.com/andersson/qdl
>
> Did you test with that?
[carl.yin] I am test with https://github.com/carlyin/mhi_for_quectel_modem/blob/main/tools/firehose_protocol.c
Firehose protocol is same for mhi and usb device,
and different on download edl image.
For usb device, use space tool download edl image by sahara protocol,
For mhi device, mhi driver download edl image by BHI interface,
For wwan device, it is better do it in user space.
>
> --
> Jeffrey Hugo
> Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux
> Foundation Collaborative Project.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: 答复: 答复: [PATCH] bus: mhi: core: Add support MHI EE FP for download firmware
2020-10-29 1:12 ` 答复: " Carl Yin(殷张成)
@ 2020-10-29 14:20 ` Jeffrey Hugo
0 siblings, 0 replies; 11+ messages in thread
From: Jeffrey Hugo @ 2020-10-29 14:20 UTC (permalink / raw)
To: Carl Yin(殷张成),
Hemant Kumar, manivannan.sadhasivam, sfr
Cc: linux-arm-msm, linux-kernel, Naveen Kumar, Bjorn Andersson
On 10/28/2020 7:12 PM, Carl Yin(殷张成) wrote:
> Hi Jeffery:
>
> On Wednesday, October 28, 2020 10:24 PM, jhugo wrote:
>> On 10/27/2020 7:39 PM, Carl Yin(殷张成) wrote:
>>> 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@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. 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@quectel.com>
>>>>>> ---
>>>>>> 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.
>>
>> Linaro has an opensource utility that implements firehose -
>> https://github.com/andersson/qdl
>>
>> Did you test with that?
> [carl.yin] I am test with https://github.com/carlyin/mhi_for_quectel_modem/blob/main/tools/firehose_protocol.c
> Firehose protocol is same for mhi and usb device,
> and different on download edl image.
> For usb device, use space tool download edl image by sahara protocol,
> For mhi device, mhi driver download edl image by BHI interface,
> For wwan device, it is better do it in user space.
Your utility has no license on it, so it looks like no one but you can
use it. This is not good, particularly when an opensource alternative
exists. I would like to use EDL over MHI in future, but right now it
looks like that is not possible.
Additionally, we shouldn't have N tools, when one would do. Having
different tools for usb/mhi/wwan seems like a poor user experience.
Please test with the Linaro implementation. If it doesn't work, please
work with Linaro to update it.
--
Jeffrey Hugo
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: 答复: [PATCH] bus: mhi: core: Add support MHI EE FP for download firmware
2020-10-28 1:57 ` 答复: " Carl Yin(殷张成)
2020-10-28 14:22 ` Jeffrey Hugo
@ 2020-10-28 21:40 ` Bhaumik Bhatt
1 sibling, 0 replies; 11+ messages in thread
From: Bhaumik Bhatt @ 2020-10-28 21:40 UTC (permalink / raw)
To: Carl Yin(殷张成)
Cc: Hemant Kumar, Jeffrey Hugo, manivannan.sadhasivam, sfr,
linux-arm-msm, linux-kernel, Naveen Kumar,
hemantk=codeaurora.org
On 2020-10-27 18:57, Carl Yin wrote:
> Hi bbhatt:
>
> On Wednesday, October 28, 2020 9:34 AM, bbhatt wrote:
>> Hi Carl,
>> On 2020-10-27 16:01, Hemant Kumar wrote:
>> > Hi Jeff,
>> >
>> > On 10/27/20 8:11 AM, Jeffrey Hugo wrote:
>> >> On 10/27/2020 3:43 AM, 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. 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@quectel.com>
>> >>> ---
>> >>> 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",
>> Longer description here would be nice (FLASH PROGRAMMER).
> [carl.yin] yes, for SDX24 and SDX55 chip, 'FIREHOSE PROGRAMMER' is more
> better.
I agree with Jeff, should be generic.
>>
>> >>> [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.
>> >
>> >>> 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 :-
>> >
>> The callback that Hemant refers to is for MHI_CB_FATAL_ERROR. It
>> should be
>> seen by the controller once device enters EDL EE with MHI in SYS_ERROR
>> state.
>>
>> Can you confirm you see this? If yes, it should be used to power down
>> and power
>> up MHI with EE set to EDL.
> [carl.yin] from AMSS EE to SBL EE or EDL EE, I think it is not a
> really ' FATAL_ERROR '.
> It is a normal EE state change, no need to power down and power up
> device.
> When ' FATAL_ERROR ' occurs, we can get really EE state from the MHI
> devices.
> Then just stay here, let the user space tools to decide how to handle
> this new EE.
>
Why do you think it is a normal EE change? FATAL ERROR callback is a
means to
notify the controller that the device is back to one of the PBL EEs
(PBL/EDL/
PTHRU) and a clean-up must be done. Hence, the emphasis on doing an MHI
power
down and power up.
Because any move to from AMSS -> PBL is essentially treated as a reboot
of the
device, MHI host should instruct the controller to do the same for MHI.
Once
the next power up happens, you are free to load the EDL image because EE
will
be EDL if mhi_cntrl->edl_image is set as
'firehose/prog_firehose_sdx55.mbn'.
This if check is not a good idea. If you want to load the EDL image,
just use
what already exists.
>>
>> This, along with the mhi_cntrl->edl_image set from the controller,
>> will result in
>> MHI loading the EDL image for you.
>> > @@ -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);
>> >>> + }
> [carl.yin] I am working on SDX24 and SDX55 chip, maybe the process is
> different form MHI wlan chips?
> For these chips, SBL EE means the chip enter RAMDUMP state. mhi driver
> just need export SAHARA chan
> to user space, then user space tool to catch RADDUMP.
>
>> >
>> > Thanks,
>> > Hemant
>>
>> Thanks,
>> Bhaumik
>> --
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
>> Forum, a
>> Linux Foundation Collaborative Project
Thanks,
Bhaumik
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: 答复: [PATCH] bus: mhi: core: Add support MHI EE FP for download firmware
2020-10-28 1:57 ` 答复: " Carl Yin(殷张成)
@ 2020-10-28 14:22 ` Jeffrey Hugo
2020-10-28 21:40 ` Bhaumik Bhatt
1 sibling, 0 replies; 11+ messages in thread
From: Jeffrey Hugo @ 2020-10-28 14:22 UTC (permalink / raw)
To: Carl Yin(殷张成), bbhatt, Hemant Kumar
Cc: manivannan.sadhasivam, sfr, linux-arm-msm, linux-kernel,
Naveen Kumar, hemantk=codeaurora.org
On 10/27/2020 7:57 PM, Carl Yin(殷张成) wrote:
> Hi bbhatt:
>
> On Wednesday, October 28, 2020 9:34 AM, bbhatt wrote:
>> Hi Carl,
>> On 2020-10-27 16:01, Hemant Kumar wrote:
>>> Hi Jeff,
>>>
>>> On 10/27/20 8:11 AM, Jeffrey Hugo wrote:
>>>> On 10/27/2020 3:43 AM, 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. 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@quectel.com>
>>>>> ---
>>>>> 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",
>> Longer description here would be nice (FLASH PROGRAMMER).
> [carl.yin] yes, for SDX24 and SDX55 chip, 'FIREHOSE PROGRAMMER' is more better.
"Flash programmer" is more generic. It means that the EE can be used
with a different protocol than firehose if there is a use for that.
--
Jeffrey Hugo
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply [flat|nested] 11+ messages in thread
* 答复: [PATCH] bus: mhi: core: Add support MHI EE FP for download firmware
[not found] ` <27de7fd88ffeb7f95b2a709f0eaec6c5@codeaurora.org>
@ 2020-10-28 1:57 ` Carl Yin(殷张成)
2020-10-28 14:22 ` Jeffrey Hugo
2020-10-28 21:40 ` Bhaumik Bhatt
0 siblings, 2 replies; 11+ messages in thread
From: Carl Yin(殷张成) @ 2020-10-28 1:57 UTC (permalink / raw)
To: bbhatt, Hemant Kumar
Cc: Jeffrey Hugo, manivannan.sadhasivam, sfr, linux-arm-msm,
linux-kernel, Naveen Kumar, hemantk=codeaurora.org
Hi bbhatt:
On Wednesday, October 28, 2020 9:34 AM, bbhatt wrote:
> Hi Carl,
> On 2020-10-27 16:01, Hemant Kumar wrote:
> > Hi Jeff,
> >
> > On 10/27/20 8:11 AM, Jeffrey Hugo wrote:
> >> On 10/27/2020 3:43 AM, 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. 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@quectel.com>
> >>> ---
> >>> 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",
> Longer description here would be nice (FLASH PROGRAMMER).
[carl.yin] yes, for SDX24 and SDX55 chip, 'FIREHOSE PROGRAMMER' is more better.
>
> >>> [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.
> >
> >>> 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 :-
> >
> The callback that Hemant refers to is for MHI_CB_FATAL_ERROR. It should be
> seen by the controller once device enters EDL EE with MHI in SYS_ERROR state.
>
> Can you confirm you see this? If yes, it should be used to power down and power
> up MHI with EE set to EDL.
[carl.yin] from AMSS EE to SBL EE or EDL EE, I think it is not a really ' FATAL_ERROR '.
It is a normal EE state change, no need to power down and power up device.
When ' FATAL_ERROR ' occurs, we can get really EE state from the MHI devices.
Then just stay here, let the user space tools to decide how to handle this new EE.
>
> This, along with the mhi_cntrl->edl_image set from the controller, will result in
> MHI loading the EDL image for you.
> > @@ -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);
> >>> + }
[carl.yin] I am working on SDX24 and SDX55 chip, maybe the process is different form MHI wlan chips?
For these chips, SBL EE means the chip enter RAMDUMP state. mhi driver just need export SAHARA chan
to user space, then user space tool to catch RADDUMP.
> >
> > Thanks,
> > Hemant
>
> Thanks,
> Bhaumik
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a
> Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2020-10-29 14:33 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-27 9:43 [PATCH] bus: mhi: core: Add support MHI EE FP for download firmware carl.yin
2020-10-27 15:11 ` Jeffrey Hugo
2020-10-27 23:01 ` Hemant Kumar
2020-10-28 1:39 ` 答复: " Carl Yin(殷张成)
2020-10-28 14:24 ` Jeffrey Hugo
2020-10-29 1:12 ` 答复: " Carl Yin(殷张成)
2020-10-29 14:20 ` Jeffrey Hugo
2020-10-28 14:19 ` Jeffrey Hugo
[not found] <632f397f8c8bdc4df85a26d78beb58f3@sslemail.net>
[not found] ` <27de7fd88ffeb7f95b2a709f0eaec6c5@codeaurora.org>
2020-10-28 1:57 ` 答复: " Carl Yin(殷张成)
2020-10-28 14:22 ` Jeffrey Hugo
2020-10-28 21:40 ` Bhaumik Bhatt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).