Re: [PATCH v7 7/8] bus: mhi: core: Improve debug logs for loading firmware

[Manivannan Sadhasivam]

On Fri, May 08, 2020 at 07:26:47PM -0700, Bhaumik Bhatt wrote:
> Add log messages to track boot flow errors and timeouts in SBL or AMSS
> firmware loading to aid in debug.
> 
> Signed-off-by: Bhaumik Bhatt <bbhatt@xxxxxxxxxxxxxx>
> Reviewed-by: Jeffrey Hugo <jhugo@xxxxxxxxxxxxxx>

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx>

Thanks,
Mani

> ---
>  drivers/bus/mhi/core/boot.c | 21 ++++++++++++++++++---
>  1 file changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/bus/mhi/core/boot.c b/drivers/bus/mhi/core/boot.c
> index cf6dc5a..80e4d76 100644
> --- a/drivers/bus/mhi/core/boot.c
> +++ b/drivers/bus/mhi/core/boot.c
> @@ -121,7 +121,8 @@ static int __mhi_download_rddm_in_panic(struct mhi_controller *mhi_cntrl)
>  		ee = mhi_get_exec_env(mhi_cntrl);
>  	}
>  
> -	dev_dbg(dev, "Waiting for image download completion, current EE: %s\n",
> +	dev_dbg(dev,
> +		"Waiting for RDDM image download via BHIe, current EE:%s\n",
>  		TO_MHI_EXEC_STR(ee));
>  
>  	while (retry--) {
> @@ -152,11 +153,14 @@ static int __mhi_download_rddm_in_panic(struct mhi_controller *mhi_cntrl)
>  int mhi_download_rddm_img(struct mhi_controller *mhi_cntrl, bool in_panic)
>  {
>  	void __iomem *base = mhi_cntrl->bhie;
> +	struct device *dev = &mhi_cntrl->mhi_dev->dev;
>  	u32 rx_status;
>  
>  	if (in_panic)
>  		return __mhi_download_rddm_in_panic(mhi_cntrl);
>  
> +	dev_dbg(dev, "Waiting for RDDM image download via BHIe\n");
> +
>  	/* Wait for the image download to complete */
>  	wait_event_timeout(mhi_cntrl->state_event,
>  			   mhi_read_reg_field(mhi_cntrl, base,
> @@ -174,6 +178,7 @@ static int mhi_fw_load_amss(struct mhi_controller *mhi_cntrl,
>  			    const struct mhi_buf *mhi_buf)
>  {
>  	void __iomem *base = mhi_cntrl->bhie;
> +	struct device *dev = &mhi_cntrl->mhi_dev->dev;
>  	rwlock_t *pm_lock = &mhi_cntrl->pm_lock;
>  	u32 tx_status, sequence_id;
>  	int ret;
> @@ -184,6 +189,7 @@ static int mhi_fw_load_amss(struct mhi_controller *mhi_cntrl,
>  		return -EIO;
>  	}
>  
> +	dev_dbg(dev, "Starting AMSS download via BHIe\n");
>  	mhi_write_reg(mhi_cntrl, base, BHIE_TXVECADDR_HIGH_OFFS,
>  		      upper_32_bits(mhi_buf->dma_addr));
>  
> @@ -435,7 +441,12 @@ void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl)
>  		release_firmware(firmware);
>  
>  	/* Error or in EDL mode, we're done */
> -	if (ret || mhi_cntrl->ee == MHI_EE_EDL)
> +	if (ret) {
> +		dev_err(dev, "MHI did not load SBL, ret:%d\n", ret);
> +		return;
> +	}
> +
> +	if (mhi_cntrl->ee == MHI_EE_EDL)
>  		return;
>  
>  	write_lock_irq(&mhi_cntrl->pm_lock);
> @@ -463,8 +474,10 @@ void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl)
>  	if (!mhi_cntrl->fbc_download)
>  		return;
>  
> -	if (ret)
> +	if (ret) {
> +		dev_err(dev, "MHI did not enter READY state\n");
>  		goto error_read;
> +	}
>  
>  	/* Wait for the SBL event */
>  	ret = wait_event_timeout(mhi_cntrl->state_event,
> @@ -482,6 +495,8 @@ void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl)
>  	ret = mhi_fw_load_amss(mhi_cntrl,
>  			       /* Vector table is the last entry */
>  			       &image_info->mhi_buf[image_info->entries - 1]);
> +	if (ret)
> +		dev_err(dev, "MHI did not load AMSS, ret:%d\n", ret);
>  
>  	release_firmware(firmware);
>  
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project