| Message ID | 20250304032255.1131067-1-even.xu@intel.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Jiri Kosina |
| Headers | show |
| Series | [RESEND,v1] HID: Intel-thc-hid: Intel-quickspi: Correct device state after S4 | expand |
On Tue, 4 Mar 2025, Even Xu wrote: > During S4 retore flow, quickspi device was resetted by driver and state > was changed to RESETTED. It is needed to be change to ENABLED state > after S4 re-initialization finished, otherwise, device will run in wrong > state and HID input data will be dropped. Um, RESETTED, really? In the code the flag is called QUICKI2C_RESETED, but that seems to be gramatically incorrect as well, right? I'll now apply this as-is because the code is already in, but perhaps renaming the flag to QUICKI2C_RESET would be in order. Thanks,
> -----Original Message----- > From: Jiri Kosina <jikos@kernel.org> > Sent: Wednesday, March 5, 2025 4:54 AM > To: Xu, Even <even.xu@intel.com> > Cc: bentiss@kernel.org; srinivas.pandruvada@linux.intel.com; mpearson- > lenovo@squebb.ca; linux-input@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH RESEND v1] HID: Intel-thc-hid: Intel-quickspi: Correct device > state after S4 > > On Tue, 4 Mar 2025, Even Xu wrote: > > > During S4 retore flow, quickspi device was resetted by driver and > > state was changed to RESETTED. It is needed to be change to ENABLED > > state after S4 re-initialization finished, otherwise, device will run > > in wrong state and HID input data will be dropped. > > Um, RESETTED, really? Sorry, it's my bad, it should be RESET. > > In the code the flag is called QUICKI2C_RESETED, but that seems to be > gramatically incorrect as well, right? Yes, you are right, let me create a patch to correct it. > > I'll now apply this as-is because the code is already in, but perhaps renaming the > flag to QUICKI2C_RESET would be in order. Current patch is still needed, quickspi device init flow is: init -> resetting -> reset -> enabled. Exiting code in pm restore() callback takes reset operation and puts device into reset state, but forgets move to enabled state after init flow is done. Thanks for your suggestion! Let me refine the patch in V2. Best Regards, Even Xu > > Thanks, > > -- > Jiri Kosina > SUSE Labs
On Wed, 5 Mar 2025, Xu, Even wrote: > > I'll now apply this as-is because the code is already in, but perhaps renaming the > > flag to QUICKI2C_RESET would be in order. > > Current patch is still needed, quickspi device init flow is: init -> > resetting -> reset -> enabled. Exiting code in pm restore() callback > takes reset operation and puts device into reset state, but forgets move > to enabled state after init flow is done. > > Thanks for your suggestion! Let me refine the patch in V2. I have already applied your v1 patch to upstream-fixes queue so that it could go to Linus quickly, as an important functional fix. So please base the naming fixup on top of that. Thanks,
> -----Original Message----- > From: Jiri Kosina <jikos@kernel.org> > Sent: Wednesday, March 5, 2025 1:36 PM > To: Xu, Even <even.xu@intel.com> > Cc: bentiss@kernel.org; srinivas.pandruvada@linux.intel.com; mpearson- > lenovo@squebb.ca; linux-input@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: RE: [PATCH RESEND v1] HID: Intel-thc-hid: Intel-quickspi: Correct device > state after S4 > > On Wed, 5 Mar 2025, Xu, Even wrote: > > > > I'll now apply this as-is because the code is already in, but > > > perhaps renaming the flag to QUICKI2C_RESET would be in order. > > > > Current patch is still needed, quickspi device init flow is: init -> > > resetting -> reset -> enabled. Exiting code in pm restore() callback > > takes reset operation and puts device into reset state, but forgets > > move to enabled state after init flow is done. > > > > Thanks for your suggestion! Let me refine the patch in V2. > > I have already applied your v1 patch to upstream-fixes queue so that it could go to > Linus quickly, as an important functional fix. > > So please base the naming fixup on top of that. Thanks Jiri! I didn't realize v1 patch already got applied, just sent out v2 patch this morning. If so, could you just pick " [PATCH v2 1/2] HID: Intel-thc-hid: Intel-quickspi: Correct device state names gramatically" from v2 patch set for the naming fix? Those two patches have no confliction/dependence. Thank you very much! Best Regards, Even Xu > > Thanks, > > -- > Jiri Kosina > SUSE Labs
On Wed, 5 Mar 2025, Xu, Even wrote: > I didn't realize v1 patch already got applied, just sent out v2 patch > this morning. If so, could you just pick " [PATCH v2 1/2] HID: > Intel-thc-hid: Intel-quickspi: Correct device state names gramatically" > from v2 patch set for the naming fix? Those two patches have no > confliction/dependence. 2/2 now in hid.git#for-6.15/intel-thc. Thanks,
Thank you very much, Jiri! Best Regards, Even Xu > -----Original Message----- > From: Jiri Kosina <jikos@kernel.org> > Sent: Wednesday, March 5, 2025 4:44 PM > To: Xu, Even <even.xu@intel.com> > Cc: bentiss@kernel.org; srinivas.pandruvada@linux.intel.com; mpearson- > lenovo@squebb.ca; linux-input@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: RE: [PATCH RESEND v1] HID: Intel-thc-hid: Intel-quickspi: Correct device > state after S4 > > On Wed, 5 Mar 2025, Xu, Even wrote: > > > I didn't realize v1 patch already got applied, just sent out v2 patch > > this morning. If so, could you just pick " [PATCH v2 1/2] HID: > > Intel-thc-hid: Intel-quickspi: Correct device state names gramatically" > > from v2 patch set for the naming fix? Those two patches have no > > confliction/dependence. > > 2/2 now in hid.git#for-6.15/intel-thc. Thanks, > > -- > Jiri Kosina > SUSE Labs
Hi, Jiri, Just recognized you applied [2/2], could you also pick [1/2] for name fixing? Thanks! Best Regards, Even Xu > -----Original Message----- > From: Jiri Kosina <jikos@kernel.org> > Sent: Wednesday, March 5, 2025 4:44 PM > To: Xu, Even <even.xu@intel.com> > Cc: bentiss@kernel.org; srinivas.pandruvada@linux.intel.com; mpearson- > lenovo@squebb.ca; linux-input@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: RE: [PATCH RESEND v1] HID: Intel-thc-hid: Intel-quickspi: Correct device > state after S4 > > On Wed, 5 Mar 2025, Xu, Even wrote: > > > I didn't realize v1 patch already got applied, just sent out v2 patch > > this morning. If so, could you just pick " [PATCH v2 1/2] HID: > > Intel-thc-hid: Intel-quickspi: Correct device state names gramatically" > > from v2 patch set for the naming fix? Those two patches have no > > confliction/dependence. > > 2/2 now in hid.git#for-6.15/intel-thc. Thanks, > > -- > Jiri Kosina > SUSE Labs
On Thu, 6 Mar 2025, Xu, Even wrote:
> Just recognized you applied [2/2], could you also pick [1/2] for name fixing?
The functional one is in Linus' tree already [1], the one fixing grammar
of the enum naming is queued for 6.15 [2].
[1] https://git.kernel.org/linus/db52926fb0be40e1d588a346df73f5ea3a34a4c6
[2] https://web.git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git/commit/?h=for-6.15/intel-thc
Got it, Thank you very much, Jiri! Best Regards, Even Xu > -----Original Message----- > From: Jiri Kosina <jikos@kernel.org> > Sent: Friday, March 7, 2025 4:11 PM > To: Xu, Even <even.xu@intel.com> > Cc: bentiss@kernel.org; srinivas.pandruvada@linux.intel.com; mpearson- > lenovo@squebb.ca; linux-input@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: RE: [PATCH RESEND v1] HID: Intel-thc-hid: Intel-quickspi: Correct device > state after S4 > > On Thu, 6 Mar 2025, Xu, Even wrote: > > > Just recognized you applied [2/2], could you also pick [1/2] for name fixing? > > The functional one is in Linus' tree already [1], the one fixing grammar of the > enum naming is queued for 6.15 [2]. > > [1] https://git.kernel.org/linus/db52926fb0be40e1d588a346df73f5ea3a34a4c6 > [2] > https://web.git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git/commit/?h=for- > 6.15/intel-thc > > -- > Jiri Kosina > SUSE Labs
diff --git a/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c b/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c index 4641e818dfa4..6b2c7620be2b 100644 --- a/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c +++ b/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c @@ -909,6 +909,8 @@ static int quickspi_restore(struct device *device) thc_change_ltr_mode(qsdev->thc_hw, THC_LTR_MODE_ACTIVE); + qsdev->state = QUICKSPI_ENABLED; + return 0; }
During S4 retore flow, quickspi device was resetted by driver and state was changed to RESETTED. It is needed to be change to ENABLED state after S4 re-initialization finished, otherwise, device will run in wrong state and HID input data will be dropped. Signed-off-by: Even Xu <even.xu@intel.com> Fixes: 6912aaf3fd24 ("HID: intel-thc-hid: intel-quickspi: Add PM implementation") --- drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c | 2 ++ 1 file changed, 2 insertions(+)