Messages in this thread | | | Subject | Re: [PATCH 1/3] bus: mhi: core: Handle syserr during power_up | From | Jeffrey Hugo <> | Date | Tue, 7 Apr 2020 09:11:33 -0600 |
| |
On 4/7/2020 12:26 AM, Manivannan Sadhasivam wrote: > On Mon, Apr 06, 2020 at 03:04:35PM -0600, Jeffrey Hugo wrote: >> The MHI device may be in the syserr state when we attempt to init it in >> power_up(). Since we have no local state, the handling is simple - >> reset the device and wait for it to transition out of the reset state. >> >> Signed-off-by: Jeffrey Hugo <jhugo@codeaurora.org> >> --- >> drivers/bus/mhi/core/pm.c | 20 ++++++++++++++++++++ >> 1 file changed, 20 insertions(+) >> >> diff --git a/drivers/bus/mhi/core/pm.c b/drivers/bus/mhi/core/pm.c >> index 52690cb..cd6ba23 100644 >> --- a/drivers/bus/mhi/core/pm.c >> +++ b/drivers/bus/mhi/core/pm.c >> @@ -9,6 +9,7 @@ >> #include <linux/dma-direction.h> >> #include <linux/dma-mapping.h> >> #include <linux/interrupt.h> >> +#include <linux/iopoll.h> >> #include <linux/list.h> >> #include <linux/mhi.h> >> #include <linux/module.h> >> @@ -760,6 +761,7 @@ static void mhi_deassert_dev_wake(struct mhi_controller *mhi_cntrl, >> >> int mhi_async_power_up(struct mhi_controller *mhi_cntrl) >> { >> + enum mhi_state state; >> enum mhi_ee_type current_ee; >> enum dev_st_transition next_state; >> struct device *dev = &mhi_cntrl->mhi_dev->dev; >> @@ -829,6 +831,24 @@ int mhi_async_power_up(struct mhi_controller *mhi_cntrl) >> goto error_bhi_offset; >> } >> >> + state = mhi_get_mhi_state(mhi_cntrl); >> + if (state == MHI_STATE_SYS_ERR) { >> + mhi_set_mhi_state(mhi_cntrl, MHI_STATE_RESET); >> + ret = readl_poll_timeout(mhi_cntrl->regs + MHICTRL, val, >> + !(val & MHICTRL_RESET_MASK), 1000, > > Hmm. Do we really need a max 1ms delay between each read? I'd prefer to have > 100ns to reduce the wait time.
I assume you mean 100us since that's the units of the parameter, and usleep_range is the actual delay mechanism. Please correct me if that is a bad assumption.
I chose 1ms to try to avoid flooding the bus, since on one system we care about, the round trip time was observed to be ~1ms. However, that is fairly arbitrary, so a factor of 10 reduction don't seem like a significant issue.
> >> + mhi_cntrl->timeout_ms * 1000); >> + if (ret) { >> + dev_info(dev, "Failed to reset syserr\n"); > > dev_info(dev, "Failed to reset MHI due to syserr state\n"); ? >
Ah yes, that is clearer. Thanks
-- Jeffrey Hugo Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
| |