diff mbox series

[v1,08/10] bus: mhi: core: Move to an error state on any firmware load failure

Message ID 1600480955-16827-9-git-send-email-bbhatt@codeaurora.org (mailing list archive)
State Superseded
Headers show
Series Bug fixes and improvements for MHI power operations | expand

Commit Message

Bhaumik Bhatt Sept. 19, 2020, 2:02 a.m. UTC 
Move MHI to a firmware download error state for a failure to find
the firmware files or to load SBL or EBL image using BHI/BHIe. This
helps detect an error state sooner and shortens the wait for a
synchronous power up timeout.

Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
---
 drivers/bus/mhi/core/boot.c | 43 +++++++++++++++++++++++++------------------
 1 file changed, 25 insertions(+), 18 deletions(-)

Comments

Manivannan Sadhasivam Oct. 9, 2020, 4:42 p.m. UTC | #1 
On Fri, Sep 18, 2020 at 07:02:33PM -0700, Bhaumik Bhatt wrote:
> Move MHI to a firmware download error state for a failure to find
> the firmware files or to load SBL or EBL image using BHI/BHIe. This
> helps detect an error state sooner and shortens the wait for a
> synchronous power up timeout.
> 
> Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
> ---
>  drivers/bus/mhi/core/boot.c | 43 +++++++++++++++++++++++++------------------
>  1 file changed, 25 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/bus/mhi/core/boot.c b/drivers/bus/mhi/core/boot.c
> index 92b8dd3..fcc71f2 100644
> --- a/drivers/bus/mhi/core/boot.c
> +++ b/drivers/bus/mhi/core/boot.c

[...]

> -error_read:
> +error_ready_state:
>  	mhi_free_bhie_table(mhi_cntrl, mhi_cntrl->fbc_image);
>  	mhi_cntrl->fbc_image = NULL;
>  
> -error_alloc_fw_table:
> -	release_firmware(firmware);
> +error_fw_load:
> +	write_lock_irq(&mhi_cntrl->pm_lock);
> +	mhi_cntrl->pm_state = MHI_PM_FW_DL_ERR;
> +	wake_up_all(&mhi_cntrl->state_event);

Do you really need pm_lock for this?

Thanks,
Mani

> +	write_unlock_irq(&mhi_cntrl->pm_lock);
>  }
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
Bhaumik Bhatt Oct. 14, 2020, 1:37 a.m. UTC | #2 
On 2020-10-09 09:42, Manivannan Sadhasivam wrote:
> On Fri, Sep 18, 2020 at 07:02:33PM -0700, Bhaumik Bhatt wrote:
>> Move MHI to a firmware download error state for a failure to find
>> the firmware files or to load SBL or EBL image using BHI/BHIe. This
>> helps detect an error state sooner and shortens the wait for a
>> synchronous power up timeout.
>> 
>> Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
>> ---
>>  drivers/bus/mhi/core/boot.c | 43 
>> +++++++++++++++++++++++++------------------
>>  1 file changed, 25 insertions(+), 18 deletions(-)
>> 
>> diff --git a/drivers/bus/mhi/core/boot.c b/drivers/bus/mhi/core/boot.c
>> index 92b8dd3..fcc71f2 100644
>> --- a/drivers/bus/mhi/core/boot.c
>> +++ b/drivers/bus/mhi/core/boot.c
> 
> [...]
> 
>> -error_read:
>> +error_ready_state:
>>  	mhi_free_bhie_table(mhi_cntrl, mhi_cntrl->fbc_image);
>>  	mhi_cntrl->fbc_image = NULL;
>> 
>> -error_alloc_fw_table:
>> -	release_firmware(firmware);
>> +error_fw_load:
>> +	write_lock_irq(&mhi_cntrl->pm_lock);
>> +	mhi_cntrl->pm_state = MHI_PM_FW_DL_ERR;
>> +	wake_up_all(&mhi_cntrl->state_event);
> 
> Do you really need pm_lock for this?
> 
> Thanks,
> Mani
> 
Not really, the underlying usage does not matter if this lock is used or
not. We just want to error out so removing it.
>> +	write_unlock_irq(&mhi_cntrl->pm_lock);
>>  }
>> --
>> 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/boot.c b/drivers/bus/mhi/core/boot.c
index 92b8dd3..fcc71f2 100644
--- a/drivers/bus/mhi/core/boot.c
+++ b/drivers/bus/mhi/core/boot.c
@@ -425,13 +425,13 @@  void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl)
 						     !mhi_cntrl->seg_len))) {
 		dev_err(dev,
 			"No firmware image defined or !sbl_size || !seg_len\n");
-		return;
+		goto error_fw_load;
 	}
 
 	ret = request_firmware(&firmware, fw_name, dev);
 	if (ret) {
 		dev_err(dev, "Error loading firmware: %d\n", ret);
-		return;
+		goto error_fw_load;
 	}
 
 	size = (mhi_cntrl->fbc_download) ? mhi_cntrl->sbl_size : firmware->size;
@@ -443,7 +443,7 @@  void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl)
 	buf = mhi_alloc_coherent(mhi_cntrl, size, &dma_addr, GFP_KERNEL);
 	if (!buf) {
 		release_firmware(firmware);
-		return;
+		goto error_fw_load;
 	}
 
 	/* Download SBL or EDL image using BHI */
@@ -451,17 +451,17 @@  void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl)
 	ret = mhi_fw_load_bhi(mhi_cntrl, dma_addr, size);
 	mhi_free_coherent(mhi_cntrl, size, buf, dma_addr);
 
-	if (!mhi_cntrl->fbc_download || ret || mhi_cntrl->ee == MHI_EE_EDL)
-		release_firmware(firmware);
-
 	/* Error or in EDL mode, we're done */
 	if (ret) {
 		dev_err(dev, "MHI did not load SBL/EDL image, ret:%d\n", ret);
-		return;
+		release_firmware(firmware);
+		goto error_fw_load;
 	}
 
-	if (mhi_cntrl->ee == MHI_EE_EDL)
+	if (mhi_cntrl->ee == MHI_EE_EDL) {
+		release_firmware(firmware);
 		return;
+	}
 
 	write_lock_irq(&mhi_cntrl->pm_lock);
 	mhi_cntrl->dev_state = MHI_STATE_RESET;
@@ -474,13 +474,17 @@  void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl)
 	if (mhi_cntrl->fbc_download) {
 		ret = mhi_alloc_bhie_table(mhi_cntrl, &mhi_cntrl->fbc_image,
 					   firmware->size);
-		if (ret)
-			goto error_alloc_fw_table;
+		if (ret) {
+			release_firmware(firmware);
+			goto error_fw_load;
+		}
 
 		/* Load the firmware into BHIE vec table */
 		mhi_firmware_copy(mhi_cntrl, firmware, mhi_cntrl->fbc_image);
 	}
 
+	release_firmware(firmware);
+
 fw_load_ee_pthru:
 	/* Transitioning into MHI RESET->READY state */
 	ret = mhi_ready_state_transition(mhi_cntrl);
@@ -490,7 +494,7 @@  void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl)
 
 	if (ret) {
 		dev_err(dev, "MHI did not enter READY state\n");
-		goto error_read;
+		goto error_ready_state;
 	}
 
 	/* Wait for the SBL event */
@@ -501,7 +505,7 @@  void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl)
 
 	if (!ret || MHI_PM_IN_ERROR_STATE(mhi_cntrl->pm_state)) {
 		dev_err(dev, "MHI did not enter SBL\n");
-		goto error_read;
+		goto error_ready_state;
 	}
 
 	/* Start full firmware image download */
@@ -509,17 +513,20 @@  void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl)
 	ret = mhi_fw_load_bhie(mhi_cntrl,
 			       /* Vector table is the last entry */
 			       &image_info->mhi_buf[image_info->entries - 1]);
-	if (ret)
+	if (ret) {
 		dev_err(dev, "MHI did not load AMSS, ret:%d\n", ret);
-
-	release_firmware(firmware);
+		goto error_fw_load;
+	}
 
 	return;
 
-error_read:
+error_ready_state:
 	mhi_free_bhie_table(mhi_cntrl, mhi_cntrl->fbc_image);
 	mhi_cntrl->fbc_image = NULL;
 
-error_alloc_fw_table:
-	release_firmware(firmware);
+error_fw_load:
+	write_lock_irq(&mhi_cntrl->pm_lock);
+	mhi_cntrl->pm_state = MHI_PM_FW_DL_ERR;
+	wake_up_all(&mhi_cntrl->state_event);
+	write_unlock_irq(&mhi_cntrl->pm_lock);
 }