All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
To: Jeffery Miller <jefferymiller@google.com>
Cc: "Jonathan Denose" <jdenose@chromium.org>,
	jdenose@google.com, "Lyude Paul" <lyude@redhat.com>,
	benjamin.tissoires@redhat.com, "Andrew Duggan" <andrew@duggan.us>,
	loic.poulain@linaro.org, "Andrew Duggan" <aduggan@synaptics.com>,
	"Javier Martinez Canillas" <javierm@redhat.com>,
	"Jeremy Kerr" <jk@codeconstruct.com.au>,
	"Jonathan Cameron" <Jonathan.Cameron@huawei.com>,
	"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
	linux-input@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3] Input: synaptics-rmi4 - retry reading SMBus version on resume
Date: Fri, 21 Jul 2023 16:53:48 -0700	[thread overview]
Message-ID: <ZLsajIm2qTcLE+O7@google.com> (raw)
In-Reply-To: <20230608210404.722123-1-jefferymiller@google.com>

Hi Jeffery,

On Thu, Jun 08, 2023 at 09:04:00PM +0000, Jeffery Miller wrote:
> On resume there can be a period of time after the
> preceding serio_resume -> psmouse_deactivate call
> where calls to rmi_smb_get_version fail with
> -ENXIO.
> 
> The call path in rmi_smb_resume is rmi_smb_resume -> rmi_smb_reset ->
> rmi_smb_enable_smbus_mode -> rmi_smb_get_version where
> this failure would occur.
> 
> Add a 30ms delay and retry in the ENXIO case to ensure the following
> rmi_driver_resume calls in rmi_smbus_resume can succeed.
> 
> This behavior was seen on a Lenovo T440p machine that required
> a delay of approximately 7-12ms.
> 
> The 30ms delay was chosen based on the linux-input message:
> Link: https://lore.kernel.org/all/BYAPR03MB47572F2C65E52ED673238D41B2439@BYAPR03MB4757.namprd03.prod.outlook.com/

I do not quite like putting these retries in RMI code. I wonder if we
should not move the delay into psmouse_smbus_reconnect():

	if (smbdev->need_deactivate) {
		psmouse_deactivate(psmouse);
		/* Give the device time to switch to SMBus mode */
		msleep(30);
	}

or even factor it out into psmouse_activate_smbus_mode() and call it
from psmouse_smbus_reconnect() as well as psmouse_smbus_init().

Thanks.

or even factor it out into psmouse_activate_smbus_mode() and call it
from psmouse_smbus_reconnect() as well as psmouse_smbus_init().

Thanks.

-- 
Dmitry

  reply	other threads:[~2023-07-21 23:53 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-08 21:04 [PATCH v3] Input: synaptics-rmi4 - retry reading SMBus version on resume Jeffery Miller
2023-07-21 23:53 ` Dmitry Torokhov [this message]
  -- strict thread matches above, loose matches on Subject: below --
2023-06-09 19:47 kernel test robot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZLsajIm2qTcLE+O7@google.com \
    --to=dmitry.torokhov@gmail.com \
    --cc=Jonathan.Cameron@huawei.com \
    --cc=aduggan@synaptics.com \
    --cc=andrew@duggan.us \
    --cc=benjamin.tissoires@redhat.com \
    --cc=javierm@redhat.com \
    --cc=jdenose@chromium.org \
    --cc=jdenose@google.com \
    --cc=jefferymiller@google.com \
    --cc=jk@codeconstruct.com.au \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=loic.poulain@linaro.org \
    --cc=lyude@redhat.com \
    --cc=u.kleine-koenig@pengutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.