hwdb: quirks for accelerometers in HP laptops #5510

Merged
merged 2 commits into from Mar 4, 2017

Conversation

Projects
None yet
5 participants
Contributor

phomes commented Mar 1, 2017

Quirk for HP 8540w based on similar fix by @dsd in #5471. Also related to #5160

@poettering poettering added the hwdb label Mar 1, 2017

rules/60-sensor.rules
@@ -7,4 +7,8 @@ SUBSYSTEM=="iio", KERNEL=="iio*", SUBSYSTEMS=="usb|i2c", \
IMPORT{builtin}="hwdb 'sensor:modalias:$attr{modalias}:$attr{[dmi/id]modalias}'", \
GOTO="sensor_end"
+SUBSYSTEM=="input", ENV{ID_INPUT_ACCELEROMETER}=="1", SUBSYSTEMS=="platform", \
+ IMPORT{builtin}="hwdb 'sensor:modalias:platform:$id:$attr{[dmi/id]modalias}'", \
@poettering

poettering Mar 1, 2017

Owner

why not use $attr{modalias} here? why use platform:$id instead?

@phomes

phomes Mar 1, 2017

Contributor

That works for event7 and js0, but not for this one:


P: /devices/platform/lis3lv02d/input/input13
E: ABS=7
E: DEVPATH=/devices/platform/lis3lv02d/input/input13
E: EV=9
E: ID_FOR_SEAT=input-platform-lis3lv02d
E: ID_INPUT=1
E: ID_INPUT_ACCELEROMETER=1
E: ID_PATH=platform-lis3lv02d
E: ID_PATH_TAG=platform-lis3lv02d
E: IIO_SENSOR_PROXY_TYPE=input-accel
E: MODALIAS=input:b0019v0000p0000e0000-e0,3,kra0,1,2,mlsfw
E: NAME="ST LIS3LV02DL Accelerometer"
E: PHYS="lis3lv02d/input0"
E: PRODUCT=19/0/0/0
E: PROP=0
E: SUBSYSTEM=input
E: SYSTEMD_WANTS=iio-sensor-proxy.service
E: TAGS=:seat:systemd:
E: USEC_INITIALIZED=10640768
@poettering

poettering Mar 1, 2017

Owner

why doesn't it precisely? SUBSYSTEMS=="platform" would mean we always go up to the platform device before retrieving $attr{modalias}, no?

@phomes

phomes Mar 1, 2017

Contributor

I admit that I am not the world champ of udev rules. But as I read the documentation we grab the modalias of this device since it has one, and only go for the parent if it did not have one itself?

$attr{file}, %s{file}
The value of a sysfs attribute found at the device where all keys of the rule have matched. If the matching device does not have such an attribute, and a previous KERNELS, SUBSYSTEMS, DRIVERS, or ATTRS test selected a parent device, then the attribute from that parent device is used.

@dsd

dsd Mar 1, 2017

Contributor

No, it will only go up to the platform device if a modalias is not found on the node being matched. In this case we want the mount matrix to apply to the input device, but input devices do have a modalias and it's not the one we want.

       $attr{file}, %s{file}
           The value of a sysfs attribute found at the device where all keys
           of the rule have matched. If the matching device does not have such
           an attribute, and a previous KERNELS, SUBSYSTEMS, DRIVERS, or ATTRS
           test selected a parent device, then the attribute from that parent
           device is used.
Owner

poettering commented Mar 1, 2017

@hadess please have a look!

hwdb/60-sensor.hwdb
@@ -48,6 +48,12 @@ sensor:modalias:acpi:SMO8500*:dmi:*svn*ASUSTeK*:*pn*TP500LB*
ACCEL_MOUNT_MATRIX=0, 1, 0; 1, 0, 0; 0, 0, 0
#########################################
+# HP EliteBook 8540w
@hadess

hadess Mar 1, 2017

Contributor

This is supposed to be a brand name, not the model of the device.

@phomes

phomes Mar 2, 2017

Contributor

Fixed

hwdb/60-sensor.hwdb
@@ -48,6 +48,12 @@ sensor:modalias:acpi:SMO8500*:dmi:*svn*ASUSTeK*:*pn*TP500LB*
ACCEL_MOUNT_MATRIX=0, 1, 0; 1, 0, 0; 0, 0, 0
#########################################
+# HP EliteBook 8540w
+#########################################
+sensor:modalias:platform:lis3lv02d:dmi:*svn*Hewlett-Packard*:*pn*HPEliteBook8540w*
@hadess

hadess Mar 1, 2017

Contributor

This could also apply to any number of HP laptops. Could you please add a link to:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/platform/x86/hp_accel.c#n207
?

@phomes

phomes Mar 2, 2017

Contributor

I borrowed a 8560w and it too needs the same quirk. I added it to the patch now. Do you want that link within the .hwdb file? or in the commit message?

rules/60-sensor.rules
@@ -7,4 +7,8 @@ SUBSYSTEM=="iio", KERNEL=="iio*", SUBSYSTEMS=="usb|i2c", \
IMPORT{builtin}="hwdb 'sensor:modalias:$attr{modalias}:$attr{[dmi/id]modalias}'", \
GOTO="sensor_end"
+SUBSYSTEM=="input", ENV{ID_INPUT_ACCELEROMETER}=="1", SUBSYSTEMS=="platform", \
+ IMPORT{builtin}="hwdb 'sensor:modalias:platform:$id:$attr{[dmi/id]modalias}'", \
@hadess

hadess Mar 1, 2017

Contributor

Looks correct. Could you please pastebin the full output of udevadm info --export-db for that machine, with the rules applied?

Owner

poettering commented Mar 2, 2017

also, @phomes could you please rebase?

Contributor

phomes commented Mar 2, 2017

rebased to master

Contributor

hadess commented Mar 2, 2017

One last change. Could you please split up the first patch into a patch to the rules, and one to the hwdb file, as Daniel did in #5471 ?
You can add both rules for the 2 laptops in a single commit though.

phomes added some commits Mar 3, 2017

hwdb: quirks for accelerometers in HP laptops
This patch adds quirks for the two laptops I could test on
(8540w and 8560w). The accelerometer is configured in the
kernel to report values according to the base of the laptop.
We want the values according to the screen instead.

It is likely (but untested) to match all HP laptops with the
lis3lv02d accelerometer on this list:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/platform/x86/hp_accel.c#n207
Contributor

phomes commented Mar 3, 2017

@hadess I have updated the patches

@phomes phomes referenced this pull request Mar 3, 2017

Open

Inverted laptop screen for HP Envy 15-as006nf #5160

1 of 2 tasks complete
Contributor

hadess commented Mar 3, 2017

Looking good, will merge when tests have finished running.

Contributor

hadess commented Mar 4, 2017

@martinpitt Could you poke your buildbot please? So we get checks sorted out.

@hadess hadess changed the title from hwdb: Add quirk for HP 8540w accelerometer to hwdb: quirks for accelerometers in HP laptops Mar 4, 2017

Contributor

hadess commented Mar 4, 2017

I'll go out on a limb and say this is a transient error. Yell if I broke something.

@hadess hadess merged commit bf1c790 into systemd:master Mar 4, 2017

4 of 5 checks passed

xenial-amd64 autopkgtest finished (failure)
Details
default Build finished.
Details
semaphoreci The build passed on Semaphore.
Details
xenial-i386 autopkgtest finished (success)
Details
xenial-s390x autopkgtest finished (success)
Details
Contributor

martinpitt commented Mar 4, 2017

@hadess: I am looking at it indeed. There was a daily image with an uninstallable kernel and a quickfix broke the scsi_debug kernel causing the storage test to fail. These should be sorted out by now and I'm retrying tests. However, I don't know why the upstream test times out, that sounds unrelated to the above breakage. But I've seen it in other tests too, indeed not related to this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment