[PATCH] Input: s6sy761 - fix resume ordering and restore sensing

[PATCH] Input: s6sy761 - fix resume ordering and restore sensing

[David Heidelberg via B4 Relay]

From: David Heidelberg <david@ixit.cz>

System suspend powers the controller off and resume powers it back on,
but the resume path enables the interrupt before s6sy761_power_on()
checks the boot. The firmware raises its boot-complete event on the
interrupt line, the threaded handler consumes it, s6sy761_power_on()
then reads an empty event and resume fails with -ENODEV, skipping the
touch function setup:

  s6sy761 2-0048: PM: dpm_run_callback(): s6sy761_resume [s6sy761] returns -19

Power the chip on first and only then unmask the interrupt. Once resume
completes the boot handshake the chip comes back with sensing off, as
at probe where input_open() turns it on, so the touchscreen stays dead
after resume. Send SENSE_ON again when the input device is open.

Tested on a Pixel 3 XL over several s2idle cycles: resume succeeds and
the touch function and sense status match the pre-suspend state.

Assisted-by: LLM
Cc: stable@vger.kernel.org
Fixes: 0145a7141e59 ("Input: add support for the Samsung S6SY761 touchscreen")
Signed-off-by: David Heidelberg <david@ixit.cz>
---
 drivers/input/touchscreen/s6sy761.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/input/touchscreen/s6sy761.c b/drivers/input/touchscreen/s6sy761.c
index 0f24a9b730635..c52c45a7d1029 100644
--- a/drivers/input/touchscreen/s6sy761.c
+++ b/drivers/input/touchscreen/s6sy761.c
@@ -495,20 +495,29 @@ static int s6sy761_suspend(struct device *dev)
 	s6sy761_power_off(sdata);
 
 	return 0;
 }
 
 static int s6sy761_resume(struct device *dev)
 {
 	struct s6sy761_data *sdata = dev_get_drvdata(dev);
+	int err;
 
+	err = s6sy761_power_on(sdata);
 	enable_irq(sdata->client->irq);
+	if (err)
+		return err;
 
-	return s6sy761_power_on(sdata);
+	guard(mutex)(&sdata->input->mutex);
+
+	if (!input_device_enabled(sdata->input))
+		return 0;
+
+	return i2c_smbus_write_byte(sdata->client, S6SY761_SENSE_ON);
 }
 
 static const struct dev_pm_ops s6sy761_pm_ops = {
 	SYSTEM_SLEEP_PM_OPS(s6sy761_suspend, s6sy761_resume)
 	RUNTIME_PM_OPS(s6sy761_runtime_suspend, s6sy761_runtime_resume, NULL)
 };
 
 #ifdef CONFIG_OF

---
base-commit: 7079a12d7506b07fb53b54a664bfad5fa9b16d70
change-id: 20260923-s6sy761-suspend-24674b0857ba

Best regards,
--  
David Heidelberg <david@ixit.cz>

Re: [PATCH] Input: s6sy761 - fix resume ordering and restore sensing

[Dmitry Torokhov]

Hi David,

On Wed, Sep 23, 2026 at 03:27:06PM +0200, David Heidelberg via B4 Relay wrote:
> From: David Heidelberg <david@ixit.cz>
> 
> System suspend powers the controller off and resume powers it back on,
> but the resume path enables the interrupt before s6sy761_power_on()
> checks the boot. The firmware raises its boot-complete event on the
> interrupt line, the threaded handler consumes it, s6sy761_power_on()
> then reads an empty event and resume fails with -ENODEV, skipping the
> touch function setup:
> 
>   s6sy761 2-0048: PM: dpm_run_callback(): s6sy761_resume [s6sy761] returns -19
> 
> Power the chip on first and only then unmask the interrupt. Once resume
> completes the boot handshake the chip comes back with sensing off, as
> at probe where input_open() turns it on, so the touchscreen stays dead
> after resume. Send SENSE_ON again when the input device is open.
> 
> Tested on a Pixel 3 XL over several s2idle cycles: resume succeeds and
> the touch function and sense status match the pre-suspend state.
> 
> Assisted-by: LLM
> Cc: stable@vger.kernel.org
> Fixes: 0145a7141e59 ("Input: add support for the Samsung S6SY761 touchscreen")
> Signed-off-by: David Heidelberg <david@ixit.cz>
> ---
>  drivers/input/touchscreen/s6sy761.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/input/touchscreen/s6sy761.c b/drivers/input/touchscreen/s6sy761.c
> index 0f24a9b730635..c52c45a7d1029 100644
> --- a/drivers/input/touchscreen/s6sy761.c
> +++ b/drivers/input/touchscreen/s6sy761.c
> @@ -495,20 +495,29 @@ static int s6sy761_suspend(struct device *dev)
>  	s6sy761_power_off(sdata);
>  
>  	return 0;
>  }
>  
>  static int s6sy761_resume(struct device *dev)
>  {
>  	struct s6sy761_data *sdata = dev_get_drvdata(dev);
> +	int err;
>  
> +	err = s6sy761_power_on(sdata);
>  	enable_irq(sdata->client->irq);

Why are we enabling the interrupt even if power on failed?

> +	if (err)
> +		return err;
>  
> -	return s6sy761_power_on(sdata);
> +	guard(mutex)(&sdata->input->mutex);
> +
> +	if (!input_device_enabled(sdata->input))
> +		return 0;
> +
> +	return i2c_smbus_write_byte(sdata->client, S6SY761_SENSE_ON);

Explicit return err/return 0 please - there are multiple failure points
in this function.

Thanks.

-- 
Dmitry

Re: [PATCH] Input: s6sy761 - fix resume ordering and restore sensing

[David Heidelberg]

On 26/09/2026 06:40, Dmitry Torokhov wrote:
> Hi David,
> 
> On Wed, Sep 23, 2026 at 03:27:06PM +0200, David Heidelberg via B4 Relay wrote:
>> From: David Heidelberg <david@ixit.cz>
>>
>> System suspend powers the controller off and resume powers it back on,
>> but the resume path enables the interrupt before s6sy761_power_on()
>> checks the boot. The firmware raises its boot-complete event on the
>> interrupt line, the threaded handler consumes it, s6sy761_power_on()
>> then reads an empty event and resume fails with -ENODEV, skipping the
>> touch function setup:
>>
>>    s6sy761 2-0048: PM: dpm_run_callback(): s6sy761_resume [s6sy761] returns -19
>>
>> Power the chip on first and only then unmask the interrupt. Once resume
>> completes the boot handshake the chip comes back with sensing off, as
>> at probe where input_open() turns it on, so the touchscreen stays dead
>> after resume. Send SENSE_ON again when the input device is open.
>>
>> Tested on a Pixel 3 XL over several s2idle cycles: resume succeeds and
>> the touch function and sense status match the pre-suspend state.
>>
>> Assisted-by: LLM
>> Cc: stable@vger.kernel.org
>> Fixes: 0145a7141e59 ("Input: add support for the Samsung S6SY761 touchscreen")
>> Signed-off-by: David Heidelberg <david@ixit.cz>
>> ---
>>   drivers/input/touchscreen/s6sy761.c | 11 ++++++++++-
>>   1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/input/touchscreen/s6sy761.c b/drivers/input/touchscreen/s6sy761.c
>> index 0f24a9b730635..c52c45a7d1029 100644
>> --- a/drivers/input/touchscreen/s6sy761.c
>> +++ b/drivers/input/touchscreen/s6sy761.c
>> @@ -495,20 +495,29 @@ static int s6sy761_suspend(struct device *dev)
>>   	s6sy761_power_off(sdata);
>>   
>>   	return 0;
>>   }
>>   
>>   static int s6sy761_resume(struct device *dev)
>>   {
>>   	struct s6sy761_data *sdata = dev_get_drvdata(dev);
>> +	int err;
>>   
>> +	err = s6sy761_power_on(sdata);
>>   	enable_irq(sdata->client->irq);
> 
> Why are we enabling the interrupt even if power on failed?

originally I moved the enable_irq after condition, but LLM kept convincing me 
it's wrong:

enable_irq() still runs when power-on fails. That is correct, because the next 
suspend or unbind calls disable_irq() again through s6sy761_power_off(), and the 
two calls must stay paired.

In case of failure the power_off will be run anyway, thus it make sense to me.

> 
>> +	if (err)
>> +		return err;
>>   
>> -	return s6sy761_power_on(sdata);
>> +	guard(mutex)(&sdata->input->mutex);
>> +
>> +	if (!input_device_enabled(sdata->input))
>> +		return 0;
>> +
>> +	return i2c_smbus_write_byte(sdata->client, S6SY761_SENSE_ON);
> 
> Explicit return err/return 0 please - there are multiple failure points
> in this function.

Sure, I'll send next version making the explicit return.

David

> 
> Thanks.
> 

Re: [PATCH] Input: s6sy761 - fix resume ordering and restore sensing

[Dmitry Torokhov]

On Sat, Sep 26, 2026 at 11:55:03AM +0200, David Heidelberg wrote:
> On 26/09/2026 06:40, Dmitry Torokhov wrote:
> > Hi David,
> > 
> > On Wed, Sep 23, 2026 at 03:27:06PM +0200, David Heidelberg via B4 Relay wrote:
> > > From: David Heidelberg <david@ixit.cz>
> > > 
> > > System suspend powers the controller off and resume powers it back on,
> > > but the resume path enables the interrupt before s6sy761_power_on()
> > > checks the boot. The firmware raises its boot-complete event on the
> > > interrupt line, the threaded handler consumes it, s6sy761_power_on()
> > > then reads an empty event and resume fails with -ENODEV, skipping the
> > > touch function setup:
> > > 
> > >    s6sy761 2-0048: PM: dpm_run_callback(): s6sy761_resume [s6sy761] returns -19
> > > 
> > > Power the chip on first and only then unmask the interrupt. Once resume
> > > completes the boot handshake the chip comes back with sensing off, as
> > > at probe where input_open() turns it on, so the touchscreen stays dead
> > > after resume. Send SENSE_ON again when the input device is open.
> > > 
> > > Tested on a Pixel 3 XL over several s2idle cycles: resume succeeds and
> > > the touch function and sense status match the pre-suspend state.
> > > 
> > > Assisted-by: LLM
> > > Cc: stable@vger.kernel.org
> > > Fixes: 0145a7141e59 ("Input: add support for the Samsung S6SY761 touchscreen")
> > > Signed-off-by: David Heidelberg <david@ixit.cz>
> > > ---
> > >   drivers/input/touchscreen/s6sy761.c | 11 ++++++++++-
> > >   1 file changed, 10 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/input/touchscreen/s6sy761.c b/drivers/input/touchscreen/s6sy761.c
> > > index 0f24a9b730635..c52c45a7d1029 100644
> > > --- a/drivers/input/touchscreen/s6sy761.c
> > > +++ b/drivers/input/touchscreen/s6sy761.c
> > > @@ -495,20 +495,29 @@ static int s6sy761_suspend(struct device *dev)
> > >   	s6sy761_power_off(sdata);
> > >   	return 0;
> > >   }
> > >   static int s6sy761_resume(struct device *dev)
> > >   {
> > >   	struct s6sy761_data *sdata = dev_get_drvdata(dev);
> > > +	int err;
> > > +	err = s6sy761_power_on(sdata);
> > >   	enable_irq(sdata->client->irq);
> > 
> > Why are we enabling the interrupt even if power on failed?
> 
> originally I moved the enable_irq after condition, but LLM kept convincing
> me it's wrong:
> 
> enable_irq() still runs when power-on fails. That is correct, because the
> next suspend or unbind calls disable_irq() again through
> s6sy761_power_off(), and the two calls must stay paired.
> 
> In case of failure the power_off will be run anyway, thus it make sense to me.

The LLM is wrong here. There should be no next suspend if resume fails.

On unbind it is OK to free a disabled interrupt as long as it is not
shared. The request_irq() path will reset the disable "depth" to 0 (or 1
if IRQF_NO_AUTOEN) and everything will work just fine.

This is different from clocks and regulators that do require matching
enable and disable.

Thanks.

-- 
Dmitry