Messages in this thread | ![/](/images/icornerl.gif) | | Date | Tue, 11 Jul 2023 11:45:27 +0530 | Subject | Re: [PATCH v3 9/9] bus: mhi: ep: wake up host is the MHI state is in M3 | From | Krishna Chaitanya Chundru <> |
| |
On 7/7/2023 8:42 PM, Bjorn Helgaas wrote: > On Fri, Jul 07, 2023 at 02:41:57PM +0300, Dan Carpenter wrote: >> On Fri, Jul 07, 2023 at 04:33:56PM +0530, Krishna chaitanya chundru wrote: >>> If the MHI state is in M3 then the most probably the host kept the >>> device in D3 hot or D3 cold, due to that endpoint transctions will not >>> be read by the host, so endpoint wakes up host to bring the host to D0 >>> which eventually bring back the MHI state to M0. >>> >>> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com> >>> --- >>> drivers/bus/mhi/ep/main.c | 28 ++++++++++++++++++++++++++++ >>> 1 file changed, 28 insertions(+) >>> >>> diff --git a/drivers/bus/mhi/ep/main.c b/drivers/bus/mhi/ep/main.c >>> index 6008818..46a8a3c 100644 >>> --- a/drivers/bus/mhi/ep/main.c >>> +++ b/drivers/bus/mhi/ep/main.c >>> @@ -25,6 +25,27 @@ static DEFINE_IDA(mhi_ep_cntrl_ida); >>> static int mhi_ep_create_device(struct mhi_ep_cntrl *mhi_cntrl, u32 ch_id); >>> static int mhi_ep_destroy_device(struct device *dev, void *data); >>> >>> +static bool mhi_ep_wake_host(struct mhi_ep_cntrl *mhi_cntrl) >>> +{ >>> + enum mhi_state state; >>> + bool mhi_reset; >>> + u32 count = 0; >>> + >>> + mhi_cntrl->wakeup_host(mhi_cntrl); >>> + >>> + /* Wait for Host to set the M0 state */ >>> + do { >>> + msleep(M0_WAIT_DELAY_MS); >>> + mhi_ep_mmio_get_mhi_state(mhi_cntrl, &state, &mhi_reset); >>> + count++; >>> + } while (state != MHI_STATE_M0 && count < M0_WAIT_COUNT); >>> + >>> + if (state != MHI_STATE_M0) >>> + return false; >> Functions which return false on success are an abomination. Also >> boolean functions should be named for the question they answer such >> as access_ok() or has_feature() etc. > +1. Also nice if boolean functions do not have side effects, so in > this case, where mhi_ep_wake_host() *does* something that might fail, > I think "return 0 for success or negative error value" is easier to > read.
sure Dan and Bjorn I will replace bool with int return type as suggested in the next patch series.
- KC
>> Write it like this: >> >> static int mhi_ep_wake_host(struct mhi_ep_cntrl *mhi_cntrl) >> { >> enum mhi_state state; >> bool mhi_reset; >> int count = 0; >> >> mhi_cntrl->wakeup_host(mhi_cntrl); >> >> while (count++ < M0_WAIT_COUNT) { >> msleep(M0_WAIT_DELAY_MS); >> >> mhi_ep_mmio_get_mhi_state(mhi_cntrl, &state, &mhi_reset); >> if (state == MHI_STATE_M0) >> return 0; >> } >> return -ENODEV; >> }
| ![\](/images/icornerr.gif) |