mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Benjamin Tissoires <benjamin.tissoires@redhat.com>
To: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Andrew Duggan <aduggan@synaptics.com>,
	linux-kernel@vger.kernel.org, linux-input@vger.kernel.org
Subject: Re: [PATCH 0/8] PS
Date: Fri, 10 Mar 2017 19:04:22 +0100	[thread overview]
Message-ID: <20170310180422.GB1221@mail.corp.redhat.com> (raw)
In-Reply-To: <20170310175206.GA21811@dtor-ws>

On Mar 10 2017 or thereabouts, Dmitry Torokhov wrote:
> On Fri, Mar 10, 2017 at 04:57:35PM +0100, Benjamin Tissoires wrote:
> > Hi Dmitry,
> > 
> > On Mar 09 2017 or thereabouts, Dmitry Torokhov wrote:
> > > Hi,
> > > 
> > > This is refresh of Benjamin's patches trying to bridge PS/2 and SMbus
> > > devices for better support of Synaptics RMI4 touchpads (and Elans later).
> > 
> > Thanks!
> > 
> > I have some issues/comments and am still working on those. Here are some
> > general comments:
> > 
> > > 
> > > The main difference is that we do not have platform device, as it only
> > > adds another indirection level, and have psmouse create SMBus companion
> > 
> > The purpose of having the platform device was to not have dependency
> > between psmouse and I2C. Right now I think that patch 6/8 will fail to
> > compile if I2C=m and PSMOUSE=y (I may be wrong).
> 
> This is taken care by the following guards in users if MOUSE_PS2_SMBUS:
> 
> depends on I2C=y || I2C=MOUSE_PS2

I can see this guards for MOUSE_PS2_SYNAPTICS_SMBUS, but not for
MOUSE_PS2_SMBUS. So unless I am completely missing the point, if users
disable SYNAPTICS_SMBUS but keep PS2_SMBUS there might be a problem.

> 
> I am perfectly fine to tie psmouse to I2C *core*, we do not need to have
> adapters loaded for it to work (hopefully).

K.

> 
> > 
> > > directly. Because serio ports complete registration asynchronously, we do
> > > not deadlock on psmouse_mutex when even if we have a pass-through port.
> > > (Frankly we need to revisit this whole serio and psmouse thing, use of
> > > global serio_mutex and psmouse_mutex is hurting us; they were needed when
> > > driver core could not recursively iterate over device and driver lists).
> > 
> > Agree, this is a giant PITA.
> > 
> > > 
> > > We also do not allow overriding serio driver, instead we teach psmouse
> > > about "special" devices and let it continue own the serio port and make
> > > sure nobody else touches it.
> > > 
> > > To work around issue with psmouse_reconnect() running sometimes too late,
> > > we add "fast reconnect" option to serio. Not too pretty, but gets the job
> > > done. We may need to revisit whole serio PM story later and stop "cheating"
> > > and pretending that device is resumed when it is not, but for that we need
> > > to teach PM core about devices that are OK not to wait for before resuming
> > > userspace. Anyway, much bigger topic for later.
> > 
> > I thought there was already the ability to say that a driver needs to be
> > run in a different thread for PM functions (IIRC i2c-hid uses
> > device_enable_async_suspend(&client->dev); and this "should" do the
> > trick).
> 
> The issue is that currently asynchronous resume still has to complete
> before we start resuming userspace, as PS/2 is way too slow. So the
> current solution marks device as resumed right away, and mouse may
> become responsive 2 seconds later, but that is good as we do not idly
> sit and wait but have userspace start turning on the screen and do other
> useful stuff. Maybe user can already start typing their password into
> screen locker.

Oh, I see. I haven't thought at the userspace issue.

> 
> We would need to give a way to drivers to indicate to PM core just how
> asynchronous our resume can be.
> 
> > 
> > > 
> > > This seems to be working on X1 Carbon and also not breaking my HP 1040 with
> > > forcepad (unfortunately it seems to be using some other SMBus controller
> > > for connecting Synaptics, as I see nothing at 0x2c when loading i2c-i801).
> > 
> > Well, on my T450, the SMBus connection is dead too. I can't seem to talk
> > to the device at all. This happens when the firmware believes it needs
> > to stay on PS/2 and gets completely deaf to I2C. I solved this by
> > calling psmouse_deactivate(), but this time, it looks like some other
> > function needs to be called.
> > 
> > I'll keep investigating and report back.
> 
> I've heard a rumors that HP 1020 uses a Microtech SMbus controller for
> its touchpad, it could be that 1040 is similar.

Oh, yes, I do remember this. However, I think the device was i2c_hid,
not SMBus. You can check if that's the case by looking at the DSDT, if
it has a HID over I2C touchpad, then that the same issue.

> 
> When your SMBus connection is dead do you see anything on the bus? At
> that address? Or it is completely unresponsive?

Completely unresponsive, nothing on the bus (as if there was nothing
physically wired).

I managed to discover that using psmouse as a module, not in bzImage
allows to have the bus properly set. So I guess there is some timing
issue (and that's going to be a pain to figure out).

In addition to the pdata fix I just sent in reply to 6/8, I have one
extra fix for rmi30 in case the pdata gets corrupted (or if f30 has
been deliberately disabled).

Cheers,
Benjamin

> 
> Thanks.
> 
> -- 
> Dmitry

  reply	other threads:[~2017-03-10 18:04 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-09 22:16 Dmitry Torokhov
2017-03-09 22:16 ` [PATCH 1/8] i2c: export i2c_client_type structure Dmitry Torokhov
2017-03-09 23:12   ` Wolfram Sang
2017-03-09 23:46     ` Dmitry Torokhov
2017-03-09 23:51       ` Dmitry Torokhov
2017-03-13 13:50     ` Jean Delvare
2017-03-15 23:50       ` Dmitry Torokhov
2017-04-01 16:06   ` Wolfram Sang
2017-04-01 16:43     ` Dmitry Torokhov
2017-03-09 22:16 ` [PATCH 2/8] Input: serio - add fast reconnect option Dmitry Torokhov
2017-03-09 22:16 ` [PATCH 3/8] Input: psmouse - implement " Dmitry Torokhov
2017-03-09 22:16 ` [PATCH 4/8] Input: psmouse - store pointer to current protocol Dmitry Torokhov
2017-03-09 22:16 ` [PATCH 5/8] Input: psmouse - introduce notion of SMBus companions Dmitry Torokhov
2017-03-09 22:16 ` [PATCH 6/8] Input: psmouse - add support for " Dmitry Torokhov
2017-03-11 15:13   ` kbuild test robot
2017-03-09 22:16 ` [PATCH 7/8] Input: synaptics - split device info into a separate structure Dmitry Torokhov
2017-03-09 22:16 ` [PATCH 8/8] Input: synaptics - add support for Intertouch devices Dmitry Torokhov
2017-03-09 23:53 ` [PATCH 6/8] Input: psmouse - add support for SMBus companions Dmitry Torokhov
2017-03-10 17:55   ` Benjamin Tissoires
2017-03-10 18:16     ` Dmitry Torokhov
2017-03-10 15:57 ` [PATCH 0/8] PS Benjamin Tissoires
2017-03-10 17:52   ` Dmitry Torokhov
2017-03-10 18:04     ` Benjamin Tissoires [this message]
2017-03-10 18:10       ` Dmitry Torokhov
2017-03-10 20:25         ` Dmitry Torokhov
2017-03-10 18:56     ` Andrew Duggan
2017-03-10 20:12       ` Dmitry Torokhov
2017-03-10 20:31         ` Andrew Duggan

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=20170310180422.GB1221@mail.corp.redhat.com \
    --to=benjamin.tissoires@redhat.com \
    --cc=aduggan@synaptics.com \
    --cc=dmitry.torokhov@gmail.com \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®