On Thursday 22 January 2015 13:52:46 Benjamin Tissoires wrote:
> On Jan 21 2015 or thereabouts, Peter Wu wrote:
> > On Wednesday 21 January 2015 10:16:04 Benjamin Tissoires wrote:
> > > Hi Peter,
> > >
> > > On Jan 21 2015 or thereabouts, Peter Wu wrote:
> > > > Linux 3.19 changed the way in which devices get registered by
> > > > introducing a hid-logitech-hidpp module. The driver name therefore needs
> > > > to be adjusted.
> > > >
> > > > Signed-off-by: Peter Wu <pe...@lekensteyn.nl>
> > > > ---
> > > > Hi,
> > > >
> > > > This patch makes UPower recognize Logitech Unifying receivers again.
> > > >
> > > > Benjamin, I have CC'ed you to let you know that there is possibly
> > > > userspace breakage due to the hid-logitech-hidpp rework.
> > >
> > > Thanks for letting me know. I forgot about the upower handling, and the
> > > rule indeed needs a patch.
> > >
> > > >
> > > > Kind regards,
> > > > Peter
> > > > ---
> > > > rules/95-upower-csr.rules | 3 +++
> > > > 1 file changed, 3 insertions(+)
> > > >
> > > > diff --git a/rules/95-upower-csr.rules b/rules/95-upower-csr.rules
> > > > index b476660..39539ca 100644
> > > > --- a/rules/95-upower-csr.rules
> > > > +++ b/rules/95-upower-csr.rules
> > > > @@ -25,5 +25,8 @@ SUBSYSTEM!="hid", GOTO="up_unifying_end"
> > > > ATTRS{idVendor}=="046d", ENV{UPOWER_VENDOR}="Logitech, Inc."
> > > > ATTRS{idVendor}=="046d", ATTRS{idProduct}=="c52b",
> > > > DRIVER=="logitech-djdevice", ENV{UPOWER_BATTERY_TYPE}="unifying"
> > > > ATTRS{idVendor}=="046d", ATTRS{idProduct}=="c532",
> > > > DRIVER=="logitech-djdevice", ENV{UPOWER_BATTERY_TYPE}="unifying"
> > > > +# These devices bind to the hid-logitech-hidpp module since Linux 3.19
> > > > +ATTRS{idVendor}=="046d", ATTRS{idProduct}=="c52b",
> > > > DRIVER=="logitech-hidpp-device", ENV{UPOWER_BATTERY_TYPE}="unifying"
> > > > +ATTRS{idVendor}=="046d", ATTRS{idProduct}=="c532",
> > > > DRIVER=="logitech-hidpp-device", ENV{UPOWER_BATTERY_TYPE}="unifying"
> > >
> > > This seems wrong.
> > > The Pids c52b and c532 are now solely used by the receiver. So they are
> > > using logitech-djreceiver, and so we should keep the match on the
> > > driver for the current string.
> >
> > The 'S' in ATTRS matches parent devices so it will correctly match child
> > devices using the logitech-hidpp-device driver.
> >
> > > OTOH, the devices changed of driver, but they are also now not using the
> > > PIDs from the receiver (well, nothing prevent Logitech from reusing c52b
> > > and c53b as a wireless PID).
> > >
> > > So I think we should remove the PID match here too. The name of the
> > > driver and the VID match is sufficient to guarantee that the unifying
> > > protocol can be used.
> >
> > I see what you mean, the receivers bind themselves to the two PIDs, but
> > the devices themselves are loaded based on the Wireless PIDs which are
> > read from the receiver. See v2 below (tested with a 0xc52b receiver).
> > > Cheers.
> > > Benjamin
> > >
> > > PS: not sure I am clear enough in my explanations... It looks like it's
> > > difficult for me to align 2 thought properly this morning :)
> > >
> > > > ATTRS{idVendor}=="046d", ATTRS{idProduct}=="c52f",
> > > > ENV{UPOWER_BATTERY_TYPE}="lg-wireless"
> > > > LABEL="up_unifying_end"
> > >
> > ---
> > From 00d5062b744abb7bd73d6a98f08ed5ab359e79ff Mon Sep 17 00:00:00 2001
> > From: Peter Wu <pe...@lekensteyn.nl>
> > Date: Wed, 21 Jan 2015 15:48:47 +0100
> > Subject: [PATCH v2] rules: support Logitech Unifying in Linux 3.19
> >
> > Linux 3.19 changed the way in which devices get registered by
> > introducing a hid-logitech-hidpp module. The driver name therefore needs
> > to be adjusted.
> >
> > As the driver only binds to Logitech HID++ devices which are below a
> > receiver, it should be safe to drop the precise idProduct. These IDs
> > would require all Unifying devices to be connected through a receiver
> > which matches the two product IDs.
> >
> > Signed-off-by: Peter Wu <pe...@lekensteyn.nl>
> > ---
> > rules/95-upower-csr.rules | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/rules/95-upower-csr.rules b/rules/95-upower-csr.rules
> > index b476660..22f3ee0 100644
> > --- a/rules/95-upower-csr.rules
> > +++ b/rules/95-upower-csr.rules
> > @@ -25,5 +25,7 @@ SUBSYSTEM!="hid", GOTO="up_unifying_end"
> > ATTRS{idVendor}=="046d", ENV{UPOWER_VENDOR}="Logitech, Inc."
> > ATTRS{idVendor}=="046d", ATTRS{idProduct}=="c52b",
> > DRIVER=="logitech-djdevice", ENV{UPOWER_BATTERY_TYPE}="unifying"
> > ATTRS{idVendor}=="046d", ATTRS{idProduct}=="c532",
> > DRIVER=="logitech-djdevice", ENV{UPOWER_BATTERY_TYPE}="unifying"
> > +# These devices bind to the hid-logitech-hidpp module since Linux 3.19
> > +ATTRS{idVendor}=="046d", DRIVER=="logitech-hidpp-device",
> > ENV{UPOWER_BATTERY_TYPE}="unifying"
>
> By having the rule actually written made me wondered what would happen
> with a Bluetooth T651 touchpad. And actually, the battery_type gets
> assigned, but upower ignores it as this is not a dj device (it expects
> usb and a receiver as a parent, whereas bluetooth hidpp devices are on
> their own).
>
> So this looks fine to me. When (if) the actual support of bluetooth
> hidpp devices gets added to upower, the rule will already be in place,
> so we should be good.
IIRC, the new hid-logitech-hidpp module allows you to write to the
hidraw devices to bypass the receiver. Bluetooth probably does not have
a receiver that understands the HID++ protocol, since the wireless
transport is handled by Bluetooth.
Looking at the implementation (up-device-unifying.c, hidpp-device.c) it
looks relatively simple to remove the assumption of a receiver.
Currently the receiver is used to read the device type
(mouse/keyboard/...), short model name ("M525", "K800") and a 32-bit
serial number.
At least the model name can now be read from sysfs. Would it make sense
to extend the hid-logitech-hidpp driver to add a HID_UNIQ field
matching the serial number?
> FWIW, Reviewed-by: Benjamin Tissoires <benjamin.tissoi...@redhat.com>
Thanks!
Kind regards,
Peter
> Cheers,
> Benjamin
>
> > ATTRS{idVendor}=="046d", ATTRS{idProduct}=="c52f",
> > ENV{UPOWER_BATTERY_TYPE}="lg-wireless"
> > LABEL="up_unifying_end"
>
_______________________________________________
devkit-devel mailing list
devkit-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/devkit-devel
