by Manivannan Sadhasivam

[permalink] [raw]

Subject: Re: [PATCH v7 6/8] bus: mhi: core: Return appropriate error codes for AMSS load failure

On Fri, May 08, 2020 at 07:26:46PM -0700, Bhaumik Bhatt wrote:
> When loading AMSS firmware using BHIe protocol, return -ETIMEDOUT if no
> response is received within the timeout or return -EIO in case of a
> protocol returned failure or an MHI error state.
>
> Signed-off-by: Bhaumik Bhatt <[email protected]>
> Reviewed-by: Jeffrey Hugo <[email protected]>

Reviewed-by: Manivannan Sadhasivam <[email protected]>

Thanks,
Mani

> ---
> drivers/bus/mhi/core/boot.c | 23 ++++++++++++-----------
> 1 file changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/bus/mhi/core/boot.c b/drivers/bus/mhi/core/boot.c
> index 17c636b..cf6dc5a 100644
> --- a/drivers/bus/mhi/core/boot.c
> +++ b/drivers/bus/mhi/core/boot.c
> @@ -176,6 +176,7 @@ static int mhi_fw_load_amss(struct mhi_controller *mhi_cntrl,
> void __iomem *base = mhi_cntrl->bhie;
> rwlock_t *pm_lock = &mhi_cntrl->pm_lock;
> u32 tx_status, sequence_id;
> + int ret;
>
> read_lock_bh(pm_lock);
> if (!MHI_REG_ACCESS_VALID(mhi_cntrl->pm_state)) {
> @@ -198,19 +199,19 @@ static int mhi_fw_load_amss(struct mhi_controller *mhi_cntrl,
> read_unlock_bh(pm_lock);
>
> /* Wait for the image download to complete */
> - wait_event_timeout(mhi_cntrl->state_event,
> - MHI_PM_IN_ERROR_STATE(mhi_cntrl->pm_state) ||
> - mhi_read_reg_field(mhi_cntrl, base,
> - BHIE_TXVECSTATUS_OFFS,
> - BHIE_TXVECSTATUS_STATUS_BMSK,
> - BHIE_TXVECSTATUS_STATUS_SHFT,
> - &tx_status) || tx_status,
> - msecs_to_jiffies(mhi_cntrl->timeout_ms));
> -
> - if (MHI_PM_IN_ERROR_STATE(mhi_cntrl->pm_state))
> + ret = wait_event_timeout(mhi_cntrl->state_event,
> + MHI_PM_IN_ERROR_STATE(mhi_cntrl->pm_state) ||
> + mhi_read_reg_field(mhi_cntrl, base,
> + BHIE_TXVECSTATUS_OFFS,
> + BHIE_TXVECSTATUS_STATUS_BMSK,
> + BHIE_TXVECSTATUS_STATUS_SHFT,
> + &tx_status) || tx_status,
> + msecs_to_jiffies(mhi_cntrl->timeout_ms));
> + if (MHI_PM_IN_ERROR_STATE(mhi_cntrl->pm_state) ||
> + tx_status != BHIE_TXVECSTATUS_STATUS_XFER_COMPL)
> return -EIO;
>
> - return (tx_status == BHIE_TXVECSTATUS_STATUS_XFER_COMPL) ? 0 : -EIO;
> + return (!ret) ? -ETIMEDOUT : 0;
> }
>
> static int mhi_fw_load_sbl(struct mhi_controller *mhi_cntrl,
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project

2020-05-09 08:34:18

by Manivannan Sadhasivam

[permalink] [raw]

Subject: Re: [PATCH v7 0/8] Bug fixes and improved logging in MHI

Hi Bhaumik,

On Fri, May 08, 2020 at 07:26:40PM -0700, Bhaumik Bhatt wrote:
> A set of patches for bug fixes and improved logging in mhi/core/boot.c.
> Verified on x86 and arm64 platforms.
>

Series applied to mhi-next! I'll wait for one more -rc before sending the
final series to Greg for v5.8. If any other series gets reviewed by that point,
I'll club them together for the final one.

Thanks,
Mani

> v7:
> -Updated commit text for macro inclusion
> -Updated channel ID bound checks
> -Fixed non-uniform placement of function parameters to be within 80 characters
> -Sent to correct Maintainer email ID
>
> v6:
> -Updated the MHI_RANDOM_U32_NONZERO to only give a random number upto the
> supplied bitmask
>
> v5:
> -Updated the macro MHI_RANDOM_U32_NONZERO to take a bitmask as the input
> parameter and output a non-zero value between 1 and U32_MAX
>
> v4:
> -Dropped the change: bus: mhi: core: WARN_ON for malformed vector table
> -Updated bus: mhi: core: Read transfer length from an event properly to include
> parse rsc events
> -Use prandom_u32_max() instead of prandom_u32 to avoid if check in
> bus: mhi: core: Ensure non-zero session or sequence ID values are used
>
> v3:
> -Fixed signed-off-by tags
> -Add a refactor patch for MHI queue APIs
> -Commit text fix in bus: mhi: core: Read transfer length from an event properly
> -Fix channel ID range check for ctrl and data event rings processing
>
> v2:
> -Fix channel ID range check potential infinite loop
> -Add appropriate signed-off-by tags
>
> Bhaumik Bhatt (4):
> bus: mhi: core: Handle firmware load using state worker
> bus: mhi: core: Return appropriate error codes for AMSS load failure
> bus: mhi: core: Improve debug logs for loading firmware
> bus: mhi: core: Ensure non-zero session or sequence ID values are used
>
> Hemant Kumar (4):
> bus: mhi: core: Refactor mhi queue APIs
> bus: mhi: core: Cache intmod from mhi event to mhi channel
> bus: mhi: core: Add range check for channel id received in event ring
> bus: mhi: core: Read transfer length from an event properly
>
> drivers/bus/mhi/core/boot.c | 75 +++++++++----------
> drivers/bus/mhi/core/init.c | 5 +-
> drivers/bus/mhi/core/internal.h | 5 +-
> drivers/bus/mhi/core/main.c | 156 +++++++++++++++++++++-------------------
> drivers/bus/mhi/core/pm.c | 6 +-
> include/linux/mhi.h | 2 -
> 6 files changed, 129 insertions(+), 120 deletions(-)
>
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project