[PATCH RESEND v1] HID: Intel-thc-hid: Intel-quickspi: Correct device state after S4

[PATCH RESEND v1] HID: Intel-thc-hid: Intel-quickspi: Correct device state after S4

[Even Xu]

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(+)

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;
 }
 
-- 
2.40.1

Re: [PATCH RESEND v1] HID: Intel-thc-hid: Intel-quickspi: Correct device state after S4

[Jiri Kosina]

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,

-- 
Jiri Kosina
SUSE Labs

RE: [PATCH RESEND v1] HID: Intel-thc-hid: Intel-quickspi: Correct device state after S4

[Xu, Even]

> -----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

RE: [PATCH RESEND v1] HID: Intel-thc-hid: Intel-quickspi: Correct device state after S4

[Jiri Kosina]

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 Kosina
SUSE Labs

RE: [PATCH RESEND v1] HID: Intel-thc-hid: Intel-quickspi: Correct device state after S4

[Xu, Even]

> -----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

RE: [PATCH RESEND v1] HID: Intel-thc-hid: Intel-quickspi: Correct device state after S4

[Jiri Kosina]

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

RE: [PATCH RESEND v1] HID: Intel-thc-hid: Intel-quickspi: Correct device state after S4

[Xu, Even]

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

RE: [PATCH RESEND v1] HID: Intel-thc-hid: Intel-quickspi: Correct device state after S4

[Xu, Even]

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

RE: [PATCH RESEND v1] HID: Intel-thc-hid: Intel-quickspi: Correct device state after S4

[Jiri Kosina]

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

RE: [PATCH RESEND v1] HID: Intel-thc-hid: Intel-quickspi: Correct device state after S4

[Xu, Even]

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