From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pf1-f169.google.com (mail-pf1-f169.google.com [209.85.210.169]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id C4F693FC8 for ; Thu, 23 Sep 2021 08:59:32 +0000 (UTC) Received: by mail-pf1-f169.google.com with SMTP id k26so520849pfi.5 for ; Thu, 23 Sep 2021 01:59:32 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to; bh=IwZWh7TVy1EsJ4/Xl5ph0XhiwHhsFLWIFyxY44IHD6c=; b=EtCi7o76f+Y3DWLNDkwwIIvlAA4cVI0p3JtUo3aGd6gHqYvwHf5nl75xwRDeWi9NyX 04vxuYbauytQ0qUNsZBilOckJOidDrCsNThS00KJPtvOjD63yUtWG+JBvtIp7OIPpQM6 ZD/LS9uAIA1YOmyHt0b2n9l5BnOXO9wtgQpGzk3G0IAnm+IjPClViLRS0nH3h0BAb97p Gp+3xBRf2C66aNcS/Uy2Yz2bryZLoDolIg1QCpXz0tlDZEMdBLcJ8pANb0qejWWBxYCP VFIo1hlxn3CV59KkHhVS8tvGcnCMsfdmQG8h9SwRiE0sMNb3mcVfXJWUM6+MpfQigzIU h5rw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to; bh=IwZWh7TVy1EsJ4/Xl5ph0XhiwHhsFLWIFyxY44IHD6c=; b=VsdVFbFtqbUBR1ZGdde9DlsUZZKHGi4+T68hvkLlkvdc2bcKZEHdelD2Be/1OheSy+ nmot6s3KVxtoWBDGMpdY2xiM9obMEeR0/rJF2wtGdXHTGSVaaH8sidJfEIuOQ2YqmSxv mc1BKGwMxSo3/g2zogDCTk7HEqDMltGzHt9jr0VM5iMdhDCn4xx+DdnDfMsINSBjMYdR I8xDY4M9G8QL6bgLGj69L/ir77SBVOcayDLSp7Bbi+rGoRkQfPckfY7SHhZQNYQMoIVU PhBjPaInl0r6DOeZSVGSQR3eSbUVYrRMPUodfzE76ca+nFSQ4DMMmvdXrPvgrxE1r8v8 zmYg== X-Gm-Message-State: AOAM530OPhA2PvpOaKjFYazsZFP3peQltErSTXtQAPjnPd+7bNWAPaQq 1EULmUat1c1AuYZ/wP33sglz X-Google-Smtp-Source: ABdhPJwwaXILnaoOCOksr0wpBKXGHCF7fnZfoGHzfcogdAEo2/ZL/8T1fOEFbOYoPvUfpQ98obaaKw== X-Received: by 2002:a63:770b:: with SMTP id s11mr3133224pgc.275.1632387572197; Thu, 23 Sep 2021 01:59:32 -0700 (PDT) Received: from thinkpad ([117.193.215.78]) by smtp.gmail.com with ESMTPSA id s2sm8830002pjs.56.2021.09.23.01.59.28 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 23 Sep 2021 01:59:31 -0700 (PDT) Date: Thu, 23 Sep 2021 14:29:26 +0530 From: Manivannan Sadhasivam To: Carl Huang Cc: Kalle Valo , Loic Poulain , ath11k@lists.infradead.org, linux-arm-msm , linux-wireless@vger.kernel.org, regressions@lists.linux.dev Subject: Re: [regression] mhi: ath11k resume fails on some devices Message-ID: <20210923085926.GD6083@thinkpad> References: <871r5p0x2u.fsf@codeaurora.org> <20210916111218.GA12918@thinkpad> <20210916163529.GA9027@thinkpad> <87k0jgxyjp.fsf@codeaurora.org> <20210916171927.GB9027@thinkpad> Precedence: bulk X-Mailing-List: regressions@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: On Thu, Sep 23, 2021 at 04:34:43PM +0800, Carl Huang wrote: > On 2021-09-17 01:19, Manivannan Sadhasivam wrote: > > On Thu, Sep 16, 2021 at 07:42:02PM +0300, Kalle Valo wrote: > > > Manivannan Sadhasivam writes: > > > > > > > On Thu, Sep 16, 2021 at 01:18:22PM +0200, Loic Poulain wrote: > > > >> Le jeu. 16 sept. 2021 à 13:12, Manivannan Sadhasivam < > > > >> manivannan.sadhasivam@linaro.org> a écrit : > > > >> > > > > > > > > [...] > > > > > > > >> > If things seems to work fine without that patch, then it implies that > > > >> > setting M0 > > > >> > state works during resume. I think we should just revert that patch. > > > >> > > > > >> > Loic, did that patch fix any issue for you or it was a cosmetic fix only? > > > >> > > > >> > > > >> It fixes sdx modem resuming issue, without that we don’t know modem needs > > > >> to be reinitialized. > > > >> > > > > > > > > Okay. Then in that case, the recovery mechanism has to be added to the ath11k > > > > MHI controller. > > > > > > What does that mean in practise, do you have any pointers or > > > examples? I > > > have no clue what you are proposing :) > > > > > > > Take a look at the mhi_pci_recovery_work() function below: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/bus/mhi/pci_generic.c#n610 > > > > You need to implement something similar that basically powers up the MHI > > endpoint (QCA6390) in case pm_resume() fails. At minimum, you need to > > call > > below functions: > > > > # Check if the device is powered on. If yes, then power it down to bring > > it back > > mhi_power_down() > > mhi_unprepare_after_power_down() > > > > # Power up the device > > mhi_prepare_for_power_up() > > mhi_sync_power_up() > > > > This implies that the WLAN device has been powered off during suspend, > > so the > > resume fails and we are bringing the device back to working state. > > > This is fine for platform which doesn't provide power supply during suspend. > But NUC has power supply in suspend state. If NUC retains power supply during suspend then it should work with that commit. During resume, the device is expected to be in M3 state and that's what the commit verifies. If the device is in a different state, then most likely the device have power cycled. > QCA6390 on NUC works after just reverting this commit also proves NUC has > power supply in > suspend state. > That's because we allowed the device to be in any state during resume and if it responds to the M0 transition it worked. > The reason is MHI-STATUS register can't be read somehow in M3 state on NUC. No, that's not correct. > Does the MHI spec state that MHI-STATUS register can be read in M3 state? > Yes, all the MHI registers are accessible in all states. During M3, both MHI host and device (if supported) will transition to D3 Cold. Then during resume, host will switch to D0 link state and will also notify the device to enter D0. For aid debugging, please see the state the device is in during mhi_pm_resume(). You can use below diff: diff --git a/drivers/bus/mhi/core/pm.c b/drivers/bus/mhi/core/pm.c index fb99e3727155..482d55dd209e 100644 --- a/drivers/bus/mhi/core/pm.c +++ b/drivers/bus/mhi/core/pm.c @@ -898,6 +898,9 @@ int mhi_pm_resume(struct mhi_controller *mhi_cntrl) if (MHI_PM_IN_ERROR_STATE(mhi_cntrl->pm_state)) return -EIO; + dev_info(dev, "Device state: %s\n", + TO_MHI_STATE_STR(mhi_get_mhi_state(mhi_cntrl))); + if (mhi_get_mhi_state(mhi_cntrl) != MHI_STATE_M3) return -EINVAL; Thanks, Mani