Thank you, @alpernebbi! This PR will ensure Gnome Desktop Environment and other iio-sensor-proxy dependent software will work correctly out-of-the-box on Chromebooks.

and only on one the board that I have

I can confirm this is the correct transform matrix for all chromebooks and that this matrix is also consistent across chromebooks. See W3C and Android docs for more details on this.

Note: the one nautilus board before this patch was probably wrong, it should have been the same as the rest.
Note 2: Seems like gnome and iio-sensor-proxy all expect the accelerometers to come in the HID format, which is exactly inversed from what W3C and android provide. Something something disagreement on convention about newton's third law and what the sensor actually measures. This is why this chromebook transform matrix is just full of -1s.

Reviewed-by: Alexandru Stan <[email protected]>

@gwendalcr might be into this patch, he's the person that taught me all about sensors for chromebooks.

lgtm, but could you add a comment to the entry explaining why such a broad match makes sense here (i.e. say what you wrote above there in terse words)

Rebased and added this comment, hope it's good enough:

+# CrOS EC & kernel drivers internally correct for per-board sensor orientations,
+# but they return values in the inverse direction (Android & W3C specs vs HID).

The patch is correct for the lid accel (label:accel-display) (the one that matters for rotation).
For the base (label:accel-base), since gnome expects the 2 sensors to report the same data when the device is closed (lid angle is 0), but natural orientation/chrome expects the display and base sensors to report the same data when device is open (lid angle 180), we need an extra rotation along the X axis:

[[-1, 0, 0],    [[ 1, 0, 0],    [[-1, 0, 0],
 [0, -1, 0],  X  [0, -1, 0],  =  [ 0, 1, 0],
 [0, 0, -1]]     [0, 0, -1]]     [ 0, 0, 1]]

For the base accelerometer, the mount matrix is ACCEL_MOUNT_MATRIX=-1, 0, 0; 0, 1, 0; 0, 0, 1 - a quite common matrix for other base sensors.

I don't know how to reference the iio label sysfs entry in the sensor modalias to distinguish the base from the display sensor.

chrome expects the display and base sensors to report the same data when device is open (lid angle 180), we need an extra rotation along the X axis:

I see this is the case with cat /sys/bus/iio/devices/iio:device{1,3}/{location,in_accel_{x,y,z}_raw} on rk3399-gru-kevin.

                | iio:device1 | iio:device3
----------------+-------------+------------
location        | base        | lid
in_accel_x_raw  | -677        | 128
in_accel_y_raw  | -429        | 240
in_accel_z_raw  | 16296       | 16672

I don't know how to reference the iio label sysfs entry in the sensor modalias to distinguish the base from the display sensor.

I might be wrong, but it looks like what we can match in hwdb.d originates from rules.d, more specifically:

And maybe we can add the location sysfs file to that hwdb query, since that's what $attr does according to udev(7)

$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.

But I don't know what happens when it finds no value for other devices. I'm assuming adding an extra IMPORT line would be easiest, have to test.

I got something that works, but not sure what's the best rule here.

# /etc/udev/rules.d/61-cros-ec-accel.rules
ACTION=="remove", GOTO="sensor_end"

SUBSYSTEM=="iio", KERNEL=="iio*", SUBSYSTEMS=="usb|i2c|platform", \
  IMPORT{builtin}="hwdb 'sensor:modalias:$attr{modalias}:location:$attr{location}:$attr{[dmi/id]modalias}'", \
  GOTO="sensor_end"

LABEL="sensor_end"

# /etc/udev/hwdb.d/61-cros-ec-accel.hwdb
sensor:modalias:platform:cros-ec-accel:location:lid:*
 ACCEL_MOUNT_MATRIX=-1, 0, 0; 0, -1, 0; 0, 0, -1
 ACCEL_LOCATION=display

sensor:modalias:platform:cros-ec-accel:location:base:*
 ACCEL_MOUNT_MATRIX=-1, 0, 0; 0, 1, 0; 0, 0, 1
 ACCEL_LOCATION=base

This one also works, without the hwdb entries:

# /etc/udev/rules.d/61-cros-ec-accel.rules
ACTION=="remove", GOTO="sensor_end"

SUBSYSTEM=="iio", KERNEL=="iio*", SUBSYSTEMS=="platform", \
  ATTRS{modalias}=="platform:cros-ec-accel", ATTR{location}=="base", \
  ENV{ACCEL_MOUNT_MATRIX}="-1, 0, 0; 0, 1, 0; 0, 0, 1", \
  ENV{ACCEL_LOCATION}="base", \
  GOTO="sensor_end"

SUBSYSTEM=="iio", KERNEL=="iio*", SUBSYSTEMS=="platform", \
  ATTRS{modalias}=="platform:cros-ec-accel", ATTR{location}=="lid", \
  ENV{ACCEL_MOUNT_MATRIX}="-1, 0, 0; 0, -1, 0; 0, 0, -1", \
  ENV{ACCEL_LOCATION}="display", \
  GOTO="sensor_end"

LABEL="sensor_end"

I do not have affected laptops, so I cannot follow the discussions. But feel free to open any follow-up PRs. Thank you.

Filed #24353 as a follow-up.