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

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

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