New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 829603 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug-Regression



Sign in to add a comment

ThingM blink device is not available to chrome.hid.getDevices

Project Member Reported by asaha@google.com, Apr 5 2018

Issue description

UserAgent: Mozilla/5.0 (X11; CrOS x86_64 10323.58.0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/65.0.3325.167 Safari/537.36
Platform: 10323.58.0 (Official Build) stable-channel guado

Steps to reproduce the problem:
ThingM blink device is working on older CrOS versions. Its not working with kernel 3.8 and higher. chrome.hid.getDevices is not returning this device.

What is the expected behavior?

What went wrong?
Last known working version: 10176.68.0 (Official Build) stable-channel falco

The ThingM is marked as having a dedicated driver in kernels 3.14+. But its not enabled, so HID core might be skipping it. Can this please be enabled for kernel versions 3.14+.

Thanks.

Did this work before? Yes ThingM blink device is not recognized by chrome.hid.getDevices

Chrome version: 65.0.3325.167  Channel: n/a
OS Version: 10323.58.0
Flash Version: 29.0.0.134
 

Comment 1 by w...@chromium.org, Apr 5 2018

Components: OS>Kernel IO>USB
Labels: M-65
Owner: grundler@chromium.org
Status: Assigned (was: Unconfirmed)
Cc: dtor@chromium.org w...@chromium.org
Labels: -M-65 M-66
Status: Started (was: Assigned)
To clarify: ThingM is claimed by hid_generic on  Falco (3.8) kernel but not kernels which added HID_THINGM config option (including 3.10 and newer).

I'll enable this for 3.14 and newer kernels to keep behavior consistent:
   https://chromium-review.googlesource.com/q/topic:%22enable_HID_THINGM%22+(status:open%20OR%20status:merged)

This should land in R66 (not R65) since Falco behavior won't change and this isn't a regression in any particular Chrome OS release.
Summary: ThingM blink device is not available to chrome.hid.getDevices (was: ThingM blink device is not recognized by chrome.hid.getDevices)
Arpita,
Can you please test the image that I've built for guado?

First put a guado device in developer mode:
https://www.chromium.org/a/chromium.org/dev/chromium-os/developer-information-for-chrome-os-devices/generic

Enable USB boot as directed by the above web page.

Then copy the test image to USB storage stick using your linux workstation:
 wget https://x20web.corp.google.com/users/gr/grundler/chromeos_images/guado/R67-10540.0.2018_04_05_1749-a1/chromiumos_test_image.bin

sudo dd if=chromiumos_test_image.bin of=/dev/sdX bs=1M

where "/dev/sdX" is the USB stick (e.g. /dev/sdb or /dev/sdc)
(Tip: "sudo apt-get install lsscsi" and lsscsi can tell you the name)

Then insert the USB stick into the guado machine and type "reboot" at tty2.
When the "scary reboot screen" appears type "ctl-u" to boot from USB.

ThingM should be available now. Please reply to this bug with what actually happens.

When you are done testing, put the device back into "normal mode" described in "Leaving" section of the same "Chrome OS Device Instructions" web page.
For posterity: to test this is working, can use the sample chrome app posted on github:
   https://github.com/GoogleChrome/chrome-app-samples/tree/master/samples/blink1
Cc: olivery@google.com
Oliver tested my build (remotely for me) and the sample app doesn't work. The THingM blink device is now discovered by hid-thingm driver and advertised as /dev/hidraw1 to user space.

# dmesg | grep hidraw
[    1.405846] hidraw: raw HID events driver (C) Jiri Kosina
[    1.828672] hid-generic 0003:046D:C077.0001: input,hidraw0: USB HID v1.11 Mouse [Logitech USB Optical Mouse] on usb-0000:00:14.0-2/input0
[    3.126238] thingm 0003:27B8:01ED.0002: hidraw1: USB HID v1.01 Device [ThingM blink(1) mk2] on usb-0000:00:14.0-5/input0
[    3.477558] hid-generic 0003:1050:0200.0003: hiddev0,hidraw2: USB HID v1.00 Device [Yubico Yubico Gnubby (gnubby1)] on usb-0000:00:14.0-6.1/input0
[    3.660485] apple 0003:05AC:024F.0004: input,hidraw3: USB HID v1.11 Keyboard [Apple Inc. Apple Keyboard] on usb-0000:00:14.0-6.2/input0
[    3.661520] apple 0003:05AC:024F.0005: input,hidraw4: USB HID v1.11 Device [Apple Inc. Apple Keyboard] on usb-0000:00:14.0-6.2/input1
[   48.822429] thingm 0003:27B8:01ED.0006: hidraw1: USB HID v1.01 Device [ThingM blink(1) mk2] on usb-0000:00:14.0-5/input0


Default /dev/hidraw1 permissions seem wrong:
# ls -l /dev/hidraw*
crw-rw---- 1 root root 248, 0 Apr  6 17:16 /dev/hidraw0
crw-rw---- 1 root root 248, 1 Apr  6 17:17 /dev/hidraw1
crw-rw---- 1 root root 248, 2 Apr  6 17:16 /dev/hidraw2
crw-rw---- 1 root root 248, 3 Apr  6 17:16 /dev/hidraw3
crw-rw---- 1 root root 248, 4 Apr  6 17:16 /dev/hidraw4

But changing those to 666 (for hidraw1 only) didn't help. The sample code suggests normal linux machines need a udev rule like this:

# Make the blink(1) accessible to plugdev via hidraw.
SUBSYSTEM=="hidraw", SUBSYSTEMS=="usb", ATTRS{idVendor}=="27b8", ATTRS{idProduct}=="01ed", MODE="0660", GROUP="plugdev"

Changed that to "chronos" (instead of plugdev) and tried "0666" - while both might be required, they aren't sufficient to get the sample app to work.


FTR, the 3.14 kernel output is now similar to the original 3.8 kernel (falco) dmesg output:
  hid-generic 0003:27B8:01ED.0001: hiddev0,hidraw0: USB HID v1.01 Device [ThingM blink(1) mk2] on usb-0000:00:14.0-2/input0


Someone from Arpita's team (oliver?) will have debug the user space side of this further after they've moved this weekend. It looks like the kernel is now working correctly.

To be more confident, I'd like to remotely "cros flash" a new (cleaner) build to the guado test device. Please let me know when the system is powered up again.
Can you check the system log for messages from the permission broker? If the ThingM driver is loading then it might be triggering this rule,

https://chromium.googlesource.com/chromiumos/platform2/+/master/permission_broker/deny_claimed_hidraw_device_rule.cc
It's not obvious to me why hidraw3 is being denied since udev thinks hidraw3 is subsystem "hidraw".

I'm still suspicious of the /dev/hidraw* file permissions since I don't think cronos or any other normal user can read those. But I don't think that should affect the policy decision. Something more subtle/silly is going on here and I expect we'll need to tweak the permission_broker policy in deny_claimed_hidraw_device_rule.cc (link below).


Kernel discovers ThingM device and binds to hidraw2:
2018-04-09T10:20:40.627049-07:00 INFO kernel: [    2.636771] usb 1-5: new full-speed USB device number 5 using xhci_hcd
2018-04-09T10:20:40.792114-07:00 WARNING kernel: [    2.802781] usb 1-5: config 1 interface 0 altsetting 0 has 2 endpoint descriptors, different from the i
nterface descriptor's value: 1
2018-04-09T10:20:40.794070-07:00 INFO kernel: [    2.803950] usb 1-5: New USB device found, idVendor=27b8, idProduct=01ed, bcdDevice= 0.02
2018-04-09T10:20:40.794096-07:00 INFO kernel: [    2.803977] usb 1-5: New USB device strings: Mfr=1, Product=2, SerialNumber=3
2018-04-09T10:20:40.794101-07:00 INFO kernel: [    2.803997] usb 1-5: Product: blink(1) mk2
2018-04-09T10:20:40.794104-07:00 INFO kernel: [    2.804010] usb 1-5: Manufacturer: ThingM
2018-04-09T10:20:40.794109-07:00 INFO kernel: [    2.804022] usb 1-5: SerialNumber: 20002A8F
...
2018-04-09T10:20:40.807510-07:00 INFO kernel: [    2.817655] thingm 0003:27B8:01ED.0004: hidraw3: USB HID v1.01 Device [ThingM blink(1) mk2] on usb-0000:00
:14.0-5/input0


# ls -l /dev/hidraw*
crw-rw---- 1 root root 248, 0 Apr  9 10:22 /dev/hidraw0
crw-rw---- 1 root root 248, 1 Apr  9 10:22 /dev/hidraw1
crw-rw---- 1 root root 248, 2 Apr  9 10:22 /dev/hidraw2
crw-rw---- 1 root root 248, 3 Apr  9 10:22 /dev/hidraw3


permissions_broker says "DENY":
2018-04-09T10:20:51.955708-07:00 INFO permission_broker[1153]: ProcessPath(/dev/hidraw3)
2018-04-09T10:20:51.994439-07:00 INFO permission_broker[1153]:   AllowUsbDeviceRule: IGNORE
2018-04-09T10:20:51.994451-07:00 INFO permission_broker[1153]:   AllowTtyDeviceRule: IGNORE
2018-04-09T10:20:51.994458-07:00 INFO permission_broker[1153]:   DenyClaimedUsbDeviceRule: IGNORE
2018-04-09T10:20:51.994481-07:00 INFO permission_broker[1153]:   DenyUninitializedDeviceRule: IGNORE
2018-04-09T10:20:51.994487-07:00 INFO permission_broker[1153]:   DenyUsbDeviceClassRule: IGNORE
2018-04-09T10:20:51.994493-07:00 INFO permission_broker[1153]:   DenyUsbDeviceClassRule: IGNORE
2018-04-09T10:20:51.994499-07:00 INFO permission_broker[1153]:   DenyUsbVendorIdRule: IGNORE
2018-04-09T10:20:51.994505-07:00 INFO permission_broker[1153]:   AllowHidrawDeviceRule: ALLOW
2018-04-09T10:20:51.994512-07:00 INFO permission_broker[1153]:   AllowGroupTtyDeviceRule: IGNORE
2018-04-09T10:20:51.994518-07:00 INFO permission_broker[1153]:   DenyGroupTtyDeviceRule: IGNORE
2018-04-09T10:20:51.994532-07:00 INFO permission_broker[1153]: message repeated 2 times: [   DenyGroupTtyDeviceRule: IGNORE]
2018-04-09T10:20:52.025400-07:00 INFO permission_broker[1153]:   DenyClaimedHidrawDeviceRule: DENY
2018-04-09T10:20:52.025419-07:00 INFO permission_broker[1153]: Verdict for /dev/hidraw3: DENY
2018-04-09T10:20:52.025490-07:00 ERR permission_broker[1153]: OpenPath(...): Domain=permission_broker, Code=permission_denied, Message=Permission to open '/dev/hidraw3' denied

Source for this rule:
https://chromium.googlesource.com/chromiumos/platform2/+/master/permission_broker/deny_claimed_hidraw_device_rule.cc

# udevadm info /dev/hidraw3
P: /devices/pci0000:00/0000:00:14.0/usb1/1-5/1-5:1.0/0003:27B8:01ED.0004/hidraw/hidraw3
N: hidraw3
E: DEVNAME=/dev/hidraw3
E: DEVPATH=/devices/pci0000:00/0000:00:14.0/usb1/1-5/1-5:1.0/0003:27B8:01ED.0004/hidraw/hidraw3
E: MAJOR=248
E: MINOR=3
E: SUBSYSTEM=hidraw

### chronos user can read udev attributes as well
# udevadm info -a /dev/hidraw3

Udevadm info starts with the device specified by the devpath and then
walks up the chain of parent devices. It prints for every device
found, all possible attributes in the udev rules key format.
A rule to match, can be composed by the attributes of the device
and the attributes from one single parent device.

  looking at device '/devices/pci0000:00/0000:00:14.0/usb1/1-5/1-5:1.0/0003:27B8:01ED.0004/hidraw/hidraw3':
    KERNEL=="hidraw3"
    SUBSYSTEM=="hidraw"
    DRIVER==""

  looking at parent device '/devices/pci0000:00/0000:00:14.0/usb1/1-5/1-5:1.0/0003:27B8:01ED.0004':
    KERNELS=="0003:27B8:01ED.0004"
    SUBSYSTEMS=="hid"
    DRIVERS=="thingm"

  looking at parent device '/devices/pci0000:00/0000:00:14.0/usb1/1-5/1-5:1.0':
    KERNELS=="1-5:1.0"
    SUBSYSTEMS=="usb"
    DRIVERS=="usbhid"
    ATTRS{bAlternateSetting}==" 0"
    ATTRS{bInterfaceClass}=="03"
    ATTRS{bInterfaceNumber}=="00"
    ATTRS{bInterfaceProtocol}=="00"
    ATTRS{bInterfaceSubClass}=="00"
    ATTRS{bNumEndpoints}=="01"
    ATTRS{supports_autosuspend}=="1"

  looking at parent device '/devices/pci0000:00/0000:00:14.0/usb1/1-5':
    KERNELS=="1-5"
    SUBSYSTEMS=="usb"
    DRIVERS=="usb"
    ATTRS{authorized}=="1"
    ATTRS{avoid_reset_quirk}=="0"
    ATTRS{bConfigurationValue}=="1"
    ATTRS{bDeviceClass}=="00"
    ATTRS{bDeviceProtocol}=="00"
    ATTRS{bDeviceSubClass}=="00"
    ATTRS{bMaxPacketSize0}=="8"
    ATTRS{bMaxPower}=="120mA"
    ATTRS{bNumConfigurations}=="1"
    ATTRS{bNumInterfaces}==" 1"
    ATTRS{bcdDevice}=="0002"
    ATTRS{bmAttributes}=="80"
    ATTRS{busnum}=="1"
    ATTRS{configuration}==""
    ATTRS{devnum}=="5"
    ATTRS{devpath}=="5"
    ATTRS{idProduct}=="01ed"
    ATTRS{idVendor}=="27b8"
    ATTRS{ltm_capable}=="no"
    ATTRS{manufacturer}=="ThingM"
    ATTRS{maxchild}=="0"
    ATTRS{product}=="blink(1) mk2"
    ATTRS{quirks}=="0x0"
    ATTRS{removable}=="unknown"
    ATTRS{serial}=="20002A8F"
    ATTRS{speed}=="12"
    ATTRS{urbnum}=="16"
    ATTRS{version}==" 2.00"

  looking at parent device '/devices/pci0000:00/0000:00:14.0/usb1':
    KERNELS=="usb1"
    SUBSYSTEMS=="usb"
    DRIVERS=="usb"
    ATTRS{authorized}=="1"
    ATTRS{authorized_default}=="1"
    ATTRS{avoid_reset_quirk}=="0"
    ATTRS{bConfigurationValue}=="1"
    ATTRS{bDeviceClass}=="09"
    ATTRS{bDeviceProtocol}=="01"
    ATTRS{bDeviceSubClass}=="00"
    ATTRS{bMaxPacketSize0}=="64"
    ATTRS{bMaxPower}=="0mA"
    ATTRS{bNumConfigurations}=="1"
    ATTRS{bNumInterfaces}==" 1"
    ATTRS{bcdDevice}=="0314"
    ATTRS{bmAttributes}=="e0"
    ATTRS{busnum}=="1"
    ATTRS{configuration}==""
    ATTRS{devnum}=="1"
    ATTRS{devpath}=="0"
    ATTRS{idProduct}=="0002"
    ATTRS{idVendor}=="1d6b"
    ATTRS{ltm_capable}=="no"
    ATTRS{manufacturer}=="Linux 3.14.0 xhci-hcd"
    ATTRS{maxchild}=="11"
    ATTRS{product}=="xHCI Host Controller"
    ATTRS{quirks}=="0x0"
    ATTRS{removable}=="unknown"
    ATTRS{serial}=="0000:00:14.0"
    ATTRS{speed}=="480"
    ATTRS{urbnum}=="82"
    ATTRS{version}==" 2.00"

  looking at parent device '/devices/pci0000:00/0000:00:14.0':
    KERNELS=="0000:00:14.0"
    SUBSYSTEMS=="pci"
    DRIVERS=="xhci_hcd"
    ATTRS{broken_parity_status}=="0"
    ATTRS{class}=="0x0c0330"
    ATTRS{consistent_dma_mask_bits}=="64"
    ATTRS{d3cold_allowed}=="1"
    ATTRS{device}=="0x9cb1"
    ATTRS{dma_mask_bits}=="64"
    ATTRS{enabled}=="1"
    ATTRS{irq}=="61"
    ATTRS{local_cpulist}=="0-3"
    ATTRS{local_cpus}=="f"
    ATTRS{msi_bus}==""
    ATTRS{subsystem_device}=="0x9cb1"
    ATTRS{subsystem_vendor}=="0x8086"
    ATTRS{vendor}=="0x8086"

  looking at parent device '/devices/pci0000:00':
    KERNELS=="pci0000:00"
    SUBSYSTEMS==""
    DRIVERS==""


Cc: asaha@google.com bendera@google.com
> I'm still suspicious of the /dev/hidraw* file permissions since I don't think cronos or any other normal user can read those.

These file permissions are expected because the purpose of the permission broker is to open these files and pass the file descriptor to Chrome.

I think what's failing here is ShouldSiblingSubsystemExcludeHidAccess when it discovers that there's an additional "thingm" subsystem attached to the device.

I knew this issue sounded familiar, years ago I even proposed a patch to fix it but it never got merged:

https://patchwork.kernel.org/patch/6876051/
Components: -IO>USB IO>HID

Comment 11 by w...@chromium.org, Apr 9 2018

Cc: chenal@google.com
Thanks Reilly!  Awesome that you remembered this from 3 years ago!

I can see why the patch was ignored/rejected: it's always been a big no-no to have two different drivers possibly claim the same device.  This can lead to subtle differences in behavior and bug reports due to those differences. Since the kernel is open source, expectation is users can rebuild from the sources themselves to include any additional drivers. While this isn't true for most users (including Chrome OS users) for many different reasons, the attitude is "fix the distro".

However, there are exceptions to maintain backwards compatibility:
   https://patchwork.kernel.org/patch/9976657/

I can rebase and resubmit your patch upstream (and dtor can tell us now if I should do that). But it's essentially the same amount of work as changing the config to include the driver and fixing the offending permission_broker code to accept the fact that some devices will have an additional "subsystem" between hidraw and hid.

Dmitry, can we get guidance on your preferred path?

Comment 13 by dtor@google.com, Apr 9 2018

I think the kernel's position is that there is no benefit in keeping generic fallback: the specific driver was created for a reason (extended functionality) and is the one that supposed to handle the device. If one wants that hardware be supported they should simply enable the driver and that is what we need to do here.

As far as permission broker rules go I think we should recognize that "thingm" is safe and allow using the device even though it is not handled by hid-generic.

BTW, do we allow access to LED subsystem through Chrome? Because that is the intended interface to ThingM devices in newer kernels.
Dmitry - yeah, that seems reasonable. Can you review the following kernel CLs?
   https://chromium-review.googlesource.com/q/topic:%22enable_HID_THINGM%22+(status:open%20OR%20status:merged)

bendera, reillyg, can either of you propose a change to permissions broker to recognize another driver between hid and hidraw?
I'm an interested observer in this process (we use the blink device in one
of our internal systems), but know next to nothing about the code involved
here (and havent written c++ in 10 years). Reilly, if this isn't something
you can handle let me know and I will see if I can find a C++-fluent
engineer who can help out.
Cc: reillyg@chromium.org
Adding myself to CC because Monorail doesn't seem to automatically do this anymore for issues I comment on. Sorry for the delay.

The change to permission_broker is trivial, we already have a list of drivers here that we ignore, just add another case to the if statement:

https://chromium.googlesource.com/chromiumos/platform2/+/master/permission_broker/deny_claimed_hidraw_device_rule.cc#157

It's unfortunate that we have to ship code and load a driver just so we can ignore it. Isn't this a case of not breaking userspace? If the user didn't change their kernel configuration to include the ThingM driver then the kernel behavior shouldn't change in a way that breaks user applications.

Comment 17 by dtor@chromium.org, Apr 13 2018

No, in this case userspace pokes into kernel internals (looking at the driver names) and so all bets are off. Names of drivers that handle particular device are not part of ABI.
I'm not referring to the CONFIG_HID_THINGM=m configuration. I agree there that we are poking into kernel internals and all best are off. My frustration is that in the default CONFIG_HID_THINGM=n configuration the kernel behavior changed such that the hidraw driver no longer loads and therefore /dev/hidrawN is not available.

Either way, I don't care enough to push back on shipping this small driver just to make this work. Since I'm going to be working on another permission_broker-related project shortly I'll take this as an opportunity to dust off my Chromium OS development environment and create the patch to ignore it.
Project Member

Comment 19 by bugdroid1@chromium.org, Apr 17 2018

Labels: merge-merged-chromeos-3.14
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/c3d787957a44489585c47142616f8f7ad890ebc9

commit c3d787957a44489585c47142616f8f7ad890ebc9
Author: Grant Grundler <grundler@chromium.org>
Date: Tue Apr 17 00:57:55 2018

CHROMIUM: config: add HID_THINGM=m

ThingM LED status bar was claimed by hid-generic in 3.8 kernel.
Since HID_THINGM config option was added, hid-generic won't claim the
device. Enabling HID_THINGM should restore previous behavior.

BUG= chromium:829603 
TEST=tested by asaha@ on guado (3.14 kernel)

Change-Id: I30fbcb72531206b66f9e688e90bd3c1c619d036b
Signed-off-by: Grant Grundler <grundler@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/999051
Reviewed-by: Dmitry Torokhov <dtor@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>

[modify] https://crrev.com/c3d787957a44489585c47142616f8f7ad890ebc9/chromeos/config/mips/common.config
[modify] https://crrev.com/c3d787957a44489585c47142616f8f7ad890ebc9/chromeos/config/i386/common.config
[modify] https://crrev.com/c3d787957a44489585c47142616f8f7ad890ebc9/chromeos/config/armel/chromiumos-arm.flavour.config
[modify] https://crrev.com/c3d787957a44489585c47142616f8f7ad890ebc9/chromeos/config/armel/chromiumos-rockchip.flavour.config
[modify] https://crrev.com/c3d787957a44489585c47142616f8f7ad890ebc9/chromeos/config/arm64/common.config
[modify] https://crrev.com/c3d787957a44489585c47142616f8f7ad890ebc9/chromeos/config/armel/chromiumos-cygnus.flavour.config
[modify] https://crrev.com/c3d787957a44489585c47142616f8f7ad890ebc9/chromeos/config/x86_64/common.config

Project Member

Comment 20 by bugdroid1@chromium.org, Apr 17 2018

Labels: merge-merged-chromeos-3.18
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/8b88cb35fbc41df5767c26607cbfc073e19922c8

commit 8b88cb35fbc41df5767c26607cbfc073e19922c8
Author: Grant Grundler <grundler@chromium.org>
Date: Tue Apr 17 00:57:52 2018

CHROMIUM: config: add HID_THINGM=m

ThingM LED status bar was claimed by hid-generic in 3.8 kernel.
Since HID_THINGM config option was added, hid-generic won't claim the
device. Enabling HID_THINGM should restore previous behavior.

BUG= chromium:829603 
TEST=tested by asaha@ on guado (3.14 kernel)

Change-Id: I6dff74556633677ec6e0aa00e16fec4521472344
Signed-off-by: Grant Grundler <grundler@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/999153
Reviewed-by: Dmitry Torokhov <dtor@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>

[modify] https://crrev.com/8b88cb35fbc41df5767c26607cbfc073e19922c8/chromeos/config/base.config

Project Member

Comment 21 by bugdroid1@chromium.org, Apr 17 2018

Labels: merge-merged-chromeos-4.14
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/df94c7aa2f900b4dbe875b7935c29fad4064047d

commit df94c7aa2f900b4dbe875b7935c29fad4064047d
Author: Grant Grundler <grundler@chromium.org>
Date: Tue Apr 17 04:26:55 2018

CHROMIUM: config: add HID_THINGM=m

ThingM LED status bar was claimed by hid-generic in 3.8 kernel.
Since HID_THINGM config option was added, hid-generic won't claim the
device. Enabling HID_THINGM should restore previous behavior.

BUG= chromium:829603 
TEST=tested by asaha@ on guado (3.14 kernel)

Change-Id: I607d5cdb4e4b1facf9474f3398f3f750a97feebd
Signed-off-by: Grant Grundler <grundler@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/999156
Reviewed-by: Dmitry Torokhov <dtor@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>

[modify] https://crrev.com/df94c7aa2f900b4dbe875b7935c29fad4064047d/chromeos/config/base.config

Project Member

Comment 22 by bugdroid1@chromium.org, Apr 17 2018

Labels: merge-merged-chromeos-4.4
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/5a8db585f0c0300c1aee3288475987c410f9d067

commit 5a8db585f0c0300c1aee3288475987c410f9d067
Author: Grant Grundler <grundler@chromium.org>
Date: Tue Apr 17 04:26:38 2018

CHROMIUM: config: add HID_THINGM=m

ThingM LED status bar was claimed by hid-generic in 3.8 kernel.
Since HID_THINGM config option was added, hid-generic won't claim the
device. Enabling HID_THINGM should restore previous behavior.

BUG= chromium:829603 
TEST=tested by asaha@ on guado (3.14 kernel)

Change-Id: I743c5d1295f4f1b100f5ff7e49ba687505bf604a
Signed-off-by: Grant Grundler <grundler@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/999154
Reviewed-by: Dmitry Torokhov <dtor@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>

[modify] https://crrev.com/5a8db585f0c0300c1aee3288475987c410f9d067/chromeos/config/base.config

Project Member

Comment 23 by bugdroid1@chromium.org, Apr 17 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/dbc7c4128c52344bd80ca83764d5dcbf007640ee

commit dbc7c4128c52344bd80ca83764d5dcbf007640ee
Author: Daniel Kurtz <djkurtz@chromium.org>
Date: Tue Apr 17 22:47:25 2018

CHROMIUM: config: renormalize

Run: ./chromeos/scripts/kernelconfig olddefconfig

CONFIG_HID_LED:
  Commit df94c7aa2f90 ("CHROMIUM: config: add HID_THINGM=m") did as
advertised and added HID_THINGM=m to base.config.  Unfortunately, it looks
like this patch wasn't normalized before being submitted on chromeos-4.14,
since it didn't pick up the dependent change which now sets
CONFIG_HID_LED=m. In fact, that's all HID_THINGM=m does:

config HID_THINGM
        tristate "ThingM blink(1) USB RGB LED"
        depends on HID
        depends on LEDS_CLASS
        select HID_LED
        ---help---
        Support for the ThingM blink(1) USB RGB LED. This driver has been
        merged into the generic hid led driver. Config symbol HID_THINGM
        just selects HID_LED and will be removed soon.

Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>

BUG= chromium:829603 
TEST=none

Change-Id: Ib62745d122eca5b3d305f18113e4f376ea5b88a5
Reviewed-on: https://chromium-review.googlesource.com/1015296
Commit-Ready: Daniel Kurtz <djkurtz@chromium.org>
Tested-by: Daniel Kurtz <djkurtz@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Grant Grundler <grundler@chromium.org>

[modify] https://crrev.com/dbc7c4128c52344bd80ca83764d5dcbf007640ee/chromeos/config/base.config

Owner: reillyg@chromium.org
TL;DR CONFIG_HID_LED only exists in v4.14 branch. No additional kernel commits needed. Reassigning to reillyg as reminder to submit permission_broker change.

I ran "./chromeos/scripts/kernelconfig olddefconfig" in v3.14, v3.18, and v4.4 source trees and to verify there was no other change in the config files.

Daniel - thanks for submitting the v4.14 patch and reminding to run the normalization in the same patch.

Comment 25 by bendera@google.com, Apr 24 2018

If I understand the last few messages correctly, it sounds like the fix has been submitted... if so Woot! Thanks for all the help getting this landed. When might I be able to test this out on a chromebox near me?
I'm still waiting on a review for the permission_broker patch:

https://chromium-review.googlesource.com/c/chromiumos/platform2/+/1022116

Comment 27 by w...@chromium.org, Apr 26 2018

Looks like the fix failed to land due to a repo/infrastructure issue?
Project Member

Comment 28 by bugdroid1@chromium.org, Apr 26 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/dc56b6e7501ea499738676c24b71dba0449246c9

commit dc56b6e7501ea499738676c24b71dba0449246c9
Author: Reilly Grant <reillyg@chromium.org>
Date: Thu Apr 26 20:01:49 2018

permission_broker: Allow hidraw connections to ThingM device

Newer kernel versions have a driver for ThingM devices and we must
ignore it in order to continue to allow Chrome Apps to connect to this
device with the chrome.hid API.

BUG= chromium:829603 
TEST=Install the blink(1) demo app and verified the permission_broker
allows the connection and the new log message is printed.

Change-Id: If7c6026b4825d154b3dfc522b8d490bda2ce36a2
Reviewed-on: https://chromium-review.googlesource.com/1022116
Commit-Ready: Reilly Grant <reillyg@chromium.org>
Tested-by: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org>

[modify] https://crrev.com/dc56b6e7501ea499738676c24b71dba0449246c9/permission_broker/deny_claimed_hidraw_device_rule.cc

Status: Fixed (was: Started)

Comment 30 by asaha@google.com, May 4 2018

I tested this in latest Chrome canary and it is not working. 

crosh> uname -a
Linux localhost 3.14.0 #1 SMP PREEMPT Wed May 2 04:09:41 PDT 2018 x86_64 Intel(R) Core(TM) i7-5500U CPU @ 2.40GHz GenuineIntel GNU/Linux

Comment 31 by asaha@google.com, May 4 2018

Correction: I upgraded to latest canary and this is working.

Version 68.0.3419.0 (Official Build) canary (64-bit)

Sign in to add a comment