ectool motionsense broken on glimmer |
|||||||||
Issue descriptionSince 4b154c6f95 (cl//226880), ectool expects EC to support EC_CMD_MOTION_SENSE_CMD version 1 or above, but glimmer does not. In consequence, all commands ectool motionsense ... fails on glimmer (and most likely all baytrail)
,
Apr 7 2017
,
Apr 8 2017
Actually, looking with Alex, even kernel 3.10 expects EC_CMD_MOTION_SENSE_CMD version 1 or above to work properly. We can not change the frequency and other parameters. Luckily, chrome is only interested in reading accel data, which is done via ACPI io memory, no EC command required.
,
Apr 12 2017
I was looking at porting the 3.10 driver to 4.4. Eg: amstan@amstan:~/cros/src/third_party/kernel/v4.4 % cp ../v3.10/drivers/iio/accel/cros_ec_accel.c drivers/iio/accel/cros_ec_accel.c Seems rather hard, with errors such as: /mnt/host/source/src/third_party/kernel/v4.4/drivers/iio/accel/cros_ec_accel.c:187:4: error: 'struct cros_ec_device' has no member named 'cmd_read_u8' /mnt/host/source/src/third_party/kernel/v4.4/drivers/iio/accel/cros_ec_accel.c:613:11: error: too many arguments to function 'iio_kfifo_allocate' /mnt/host/source/src/third_party/kernel/v4.4/drivers/iio/accel/cros_ec_accel.c:678:12: error: 'struct cros_ec_command' has no member named 'outdata' The issue is that the cros_ec driver is different upstream due the the rewriting work of Javier. We can't just load our old accel driver and expect it to work with the new cros_ec driver. While it might be possible to massage it, we should ask ourselves if it's worth it(to have this CHROMIUM: cros-ec-legacy driver that's only enabled on glimmer). Maybe we should look into what dtor suggested and just make the new sensors driver support protocol version 1.
,
Apr 12 2017
In terms of making the new driver support accel protocol version 1... Right now the driver responsible for loading all the other ec drivers, stops pretty early on: [ 2.368390] cros-ec-ctl cros-ec-ctl.1.auto: cannot get EC features: 0/1 Because the EC doesn't support the inventory command. Normally it would provide this information: localhost sys # ectool inventory EC supported features: 1 : Flash support 5 : LED support 6 : Motion Sensors support 10 : Thermal management support 13 : Host event support 14 : GPIO support 15 : I2C master support 16 : Charger support 17 : Simple Battery support 18 : Smart Battery support 22 : USB Cros Power Delievery support 24 : FIFO for Motion Sensors events support 26 : Host-controlled USB-C SS mux support 27 : Real-time clock support Then for each feature the driver cares about, it defined new mfd devices and allows the subdrivers(like the cros_ec_sensors, via cros_ec_sensors_register()) to bind to them. If we wanted to support accel version 1, we would have to somehow assume that it's present (ignoring all the inventory features that we added) and just try it.
,
Apr 17 2017
#4: current glimmer firmware does not even support version 1, only version 0. As I said in #3, all EC_CMD_MOTION_SENSE_CMD commands sent by the 3.10 driver to the EC currently fails (the driver use version 1): we only communicate via the ACPI interface thru IO registers: it is enough to confirm presence of accels and gather data. It is not for changing frequency or calibration. The 3.10 driver is hard coded to expect 2 accelerometers. We can add support: in cros_ec_dev.c: ec_device_probe(): after cros_ec_check_features() fails, check EC_MEMMAP_ACC_STATUS_PRESENCE_BIT in EC_MEMMAP_ACC_STATUS: if true, build a 2 mfd_cell array to call a new simple driver, using cros_ec_sensor_platform to present one accel per iio device, hard-coding location. [Having one device per iio would help remove some dead code in chrome to support older statck]. By introducing a new driver, we need to alter accel udev script to be sure it creates the link chrome needs.
,
Apr 20 2017
Working on a new driver cros-ec-accel-legacy, without EC changes. ETA end of April, so in the meantime, we need glimmer convertible to stay on 3.10.
,
Apr 26 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/3bf98755f9c670c5c10ca05cba22848d65117cb2 commit 3bf98755f9c670c5c10ca05cba22848d65117cb2 Author: Gwendal Grignou <gwendal@chromium.org> Date: Wed Apr 26 01:40:07 2017 CHROMIUM: iio: accel: Add cros_ec_accel_legacy driver. Add driver to support older EC firmware that only support deprecated ec command. Rely on ACPI memory map register to access sensor information. Present same interface as the regular cros_ec sensor stack: - one iio device per accelerometer - use HTML5 axis definition - use iio abi units - accept calibration calls, but do nothing Chrome can use the same code than regular cros_ec sensor stack to calculate orientation and lid angle. BUG= chromium:709642 TEST=Check sensor values with script: DIR=/sys/bus/iio/devices/ for i in 0 1 ; do cat $DIR/iio:device$i/location for j in x y z; do echo -n $j: ; cat $DIR/iio:device$i/in_accel_${j}_raw done done Check chrome goes into tablet mode. Change-Id: I1856e41cca3993e4f1604b5f6234d813c60b0973 Signed-off-by: Gwendal Grignou <gwendal@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/483965 Reviewed-by: Dmitry Torokhov <dtor@chromium.org> [modify] https://crrev.com/3bf98755f9c670c5c10ca05cba22848d65117cb2/drivers/iio/accel/Kconfig [modify] https://crrev.com/3bf98755f9c670c5c10ca05cba22848d65117cb2/drivers/platform/chrome/cros_ec_dev.c [modify] https://crrev.com/3bf98755f9c670c5c10ca05cba22848d65117cb2/drivers/iio/accel/Makefile [add] https://crrev.com/3bf98755f9c670c5c10ca05cba22848d65117cb2/drivers/iio/accel/cros_ec_accel_legacy.c
,
Apr 26 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/85f58fe31f725f4405444c4cbe7382e7cacca63f commit 85f58fe31f725f4405444c4cbe7382e7cacca63f Author: Gwendal Grignou <gwendal@chromium.org> Date: Wed Apr 26 18:25:59 2017 CHROMIUM: Add support for accelerometers on glimmer Add cros-ec-accel-legacy as valid cros-ec accelerometer driver. CQ-DEPEND=CL:483965 BUG= chromium:709642 TEST=Check sensor values with script: DIR=/sys/bus/iio/devices/ for i in 0 1 ; do cat $DIR/iio:device$i/location for j in x y z; do echo -n $j: ; cat $DIR/iio:device$i/in_accel_${j}_raw done done Check /dev/cros-ec-accel links are created properly Check trigger and access rights. Check chrome goes into tablet mode when needed. Change-Id: I1457a18f998eb394aea8fde8f3711467cdab6b49 Signed-off-by: Gwendal Grignou <gwendal@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/483942 Reviewed-by: Bernie Thompson <bhthompson@chromium.org> [modify] https://crrev.com/85f58fe31f725f4405444c4cbe7382e7cacca63f/chromeos-base/chromeos-accelerometer-init/files/udev/99-cros-ec-accel.rules [modify] https://crrev.com/85f58fe31f725f4405444c4cbe7382e7cacca63f/chromeos-base/chromeos-accelerometer-init/files/init/cros-ec-accel.conf [rename] https://crrev.com/85f58fe31f725f4405444c4cbe7382e7cacca63f/chromeos-base/chromeos-accelerometer-init/chromeos-accelerometer-init-0.0.1-r15.ebuild
,
Apr 28 2017
,
Apr 28 2017
The CLs above are more for chromium:709252
,
Apr 28 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/efad54b2e2c2dc700015a5e7cd89e0122f35eaf6 commit efad54b2e2c2dc700015a5e7cd89e0122f35eaf6 Author: Gwendal Grignou <gwendal@chromium.org> Date: Fri Apr 28 21:07:08 2017 CHROMIUM: config: x86_64: Add cros_ec_accel_legacy driver Add support for accelerometer for Glimmer. BUG= chromium:709642 TEST=Check sensor values with script: DIR=/sys/bus/iio/devices/ for i in 0 1 ; do cat $DIR/iio:device$i/location for j in x y z; do echo -n $j: ; cat $DIR/iio:device$i/in_accel_${j}_raw done done Change-Id: I483e340f83ebcbb61a9828b32ea9d7ab57f79f33 Signed-off-by: Gwendal Grignou <gwendal@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/483964 Reviewed-by: Dmitry Torokhov <dtor@chromium.org> (cherry picked from commit f85f0c104dad52a85b386b49b10f55116867fcd4) Reviewed-on: https://chromium-review.googlesource.com/490829 [modify] https://crrev.com/efad54b2e2c2dc700015a5e7cd89e0122f35eaf6/chromeos/config/x86_64/common.config
,
Apr 28 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/2292d7429f3ec10cc9a4252fa877b597276859b1 commit 2292d7429f3ec10cc9a4252fa877b597276859b1 Author: Gwendal Grignou <gwendal@chromium.org> Date: Fri Apr 28 21:07:09 2017 CHROMIUM: Add support for accelerometers on glimmer Add cros-ec-accel-legacy as valid cros-ec accelerometer driver. CQ-DEPEND=CL:483965 BUG= chromium:709642 TEST=Check sensor values with script: DIR=/sys/bus/iio/devices/ for i in 0 1 ; do cat $DIR/iio:device$i/location for j in x y z; do echo -n $j: ; cat $DIR/iio:device$i/in_accel_${j}_raw done done Check /dev/cros-ec-accel links are created properly Check trigger and access rights. Check chrome goes into tablet mode when needed. Change-Id: I1457a18f998eb394aea8fde8f3711467cdab6b49 Signed-off-by: Gwendal Grignou <gwendal@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/483942 Reviewed-by: Bernie Thompson <bhthompson@chromium.org> (cherry picked from commit 85f58fe31f725f4405444c4cbe7382e7cacca63f) Reviewed-on: https://chromium-review.googlesource.com/490847 [modify] https://crrev.com/2292d7429f3ec10cc9a4252fa877b597276859b1/chromeos-base/chromeos-accelerometer-init/files/udev/99-cros-ec-accel.rules [modify] https://crrev.com/2292d7429f3ec10cc9a4252fa877b597276859b1/chromeos-base/chromeos-accelerometer-init/files/init/cros-ec-accel.conf [rename] https://crrev.com/2292d7429f3ec10cc9a4252fa877b597276859b1/chromeos-base/chromeos-accelerometer-init/chromeos-accelerometer-init-0.0.1-r15.ebuild
,
Apr 28 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/83d7d925177b0cb734db954464401a498a222fe2 commit 83d7d925177b0cb734db954464401a498a222fe2 Author: Gwendal Grignou <gwendal@chromium.org> Date: Fri Apr 28 21:07:13 2017 CHROMIUM: iio: accel: Add cros_ec_accel_legacy driver. Add driver to support older EC firmware that only support deprecated ec command. Rely on ACPI memory map register to access sensor information. Present same interface as the regular cros_ec sensor stack: - one iio device per accelerometer - use HTML5 axis definition - use iio abi units - accept calibration calls, but do nothing Chrome can use the same code than regular cros_ec sensor stack to calculate orientation and lid angle. BUG= chromium:709642 TEST=Check sensor values with script: DIR=/sys/bus/iio/devices/ for i in 0 1 ; do cat $DIR/iio:device$i/location for j in x y z; do echo -n $j: ; cat $DIR/iio:device$i/in_accel_${j}_raw done done Check chrome goes into tablet mode. Change-Id: I1856e41cca3993e4f1604b5f6234d813c60b0973 Signed-off-by: Gwendal Grignou <gwendal@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/483965 Reviewed-by: Dmitry Torokhov <dtor@chromium.org> (cherry picked from commit 3bf98755f9c670c5c10ca05cba22848d65117cb2) Reviewed-on: https://chromium-review.googlesource.com/490828 [modify] https://crrev.com/83d7d925177b0cb734db954464401a498a222fe2/drivers/iio/accel/Kconfig [modify] https://crrev.com/83d7d925177b0cb734db954464401a498a222fe2/drivers/platform/chrome/cros_ec_dev.c [modify] https://crrev.com/83d7d925177b0cb734db954464401a498a222fe2/drivers/iio/accel/Makefile [add] https://crrev.com/83d7d925177b0cb734db954464401a498a222fe2/drivers/iio/accel/cros_ec_accel_legacy.c
,
May 1 2017
Can this be closed?
,
May 1 2017
We are not fixing ectool motionsense on glimmer, but now, the sensors with the current EC show up in 4.4.
,
May 2 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/05842b0e6433314a3a0abe755f1ffcb77ce96f94 commit 05842b0e6433314a3a0abe755f1ffcb77ce96f94 Author: Guenter Roeck <groeck@chromium.org> Date: Tue May 02 18:45:39 2017 FIXUP: CHROMIUM: iio: accel: Add cros_ec_accel_legacy driver Fix the following build warnings. drivers/iio/accel/cros_ec_accel_legacy.c:223:6-12: WARNING: Unsigned expression compared with zero: status < 0 drivers/iio/accel/cros_ec_accel_legacy.c:307:13: warning: no previous prototype for cros_ec_accel_legacy_capture BUG= chromium:709642 TEST=Build with W=1 Change-Id: I0d09ce476afb0afdb7f253045912f7fa44624766 Signed-off-by: Guenter Roeck <groeck@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/490747 Reviewed-by: Gwendal Grignou <gwendal@chromium.org> [modify] https://crrev.com/05842b0e6433314a3a0abe755f1ffcb77ce96f94/drivers/iio/accel/cros_ec_accel_legacy.c
,
May 13 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform2/+/b16b2f6897b5a53627d46499a68e4a160388372f commit b16b2f6897b5a53627d46499a68e4a160388372f Author: Gwendal Grignou <gwendal@chromium.org> Date: Sat May 13 08:04:43 2017 init: Exclude cros-ec-accel AND cros-ec-accel-legacy devices These devices are already claimed by cros-ec-accel.conf. Adding * to the match to include cros-ec-accel-legacy. BUG=b:38189383, chromium:709642 TEST=Check accelerometer is matched in time on eve. Change-Id: I6d695fc9cee6a023f6ed5dbc2f098a6c9c05db3a Signed-off-by: Gwendal Grignou <gwendal@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/502172 Tested-by: Daniel Kurtz <djkurtz@chromium.org> Reviewed-by: Daniel Kurtz <djkurtz@chromium.org> Reviewed-by: Mike Frysinger <vapier@chromium.org> [modify] https://crrev.com/b16b2f6897b5a53627d46499a68e4a160388372f/init/upstart/udev-trigger.conf
,
May 13 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform2/+/df8c5bb29df16ba386d80b687935208dd8b51c8a commit df8c5bb29df16ba386d80b687935208dd8b51c8a Author: Gwendal Grignou <gwendal@chromium.org> Date: Sat May 13 17:56:43 2017 init: Exclude cros-ec-accel AND cros-ec-accel-legacy devices These devices are already claimed by cros-ec-accel.conf. Adding * to the match to include cros-ec-accel-legacy. BUG=b:38189383, chromium:709642 TEST=Check accelerometer is matched in time on eve. Change-Id: I6d695fc9cee6a023f6ed5dbc2f098a6c9c05db3a Signed-off-by: Gwendal Grignou <gwendal@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/502172 Tested-by: Daniel Kurtz <djkurtz@chromium.org> Reviewed-by: Daniel Kurtz <djkurtz@chromium.org> Reviewed-by: Mike Frysinger <vapier@chromium.org> (cherry picked from commit b16b2f6897b5a53627d46499a68e4a160388372f) Reviewed-on: https://chromium-review.googlesource.com/505423 [modify] https://crrev.com/df8c5bb29df16ba386d80b687935208dd8b51c8a/init/upstart/udev-trigger.conf
,
Nov 16 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/f2b141a242e59017dbc774dc916748670a41da0b commit f2b141a242e59017dbc774dc916748670a41da0b Author: Gwendal Grignou <gwendal@chromium.org> Date: Thu Nov 16 06:37:51 2017 FIXUP: CHROMIUM: iio: accel: Add cros_ec_accel_legacy driver Because we read the LPC memory directly, we would read the primary EC while setting up the downstream EC, incorrectly re-discovering the accelerometers connected to the primary EC and associating them with the secondary EC. BUG= chromium:709642 TEST=On Samus with 4.4: Before on the secondary EC (cros_pd), at /sys/devices/.../PNP0C09:00/GOOG0004:00/cros-ec-ctl.2.auto/ We have chromeos, cros-ec-accel-legacy.0, cros-ec-accel-legacy.1 and cros-usb-pd-charger. In iio/devices, we have cros-ec-accel and cros-ec-accel-legacy pointing to the same accelerometer. Now, only we have only chromeos and cros-usb-pd-charger. Change-Id: I5f9b8c026ee0a9700d7e31a1c46357763c17b634 Signed-off-by: Gwendal Grignou <gwendal@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/772807 Reviewed-by: Guenter Roeck <groeck@chromium.org> [modify] https://crrev.com/f2b141a242e59017dbc774dc916748670a41da0b/drivers/platform/chrome/cros_ec_dev.c |
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by gwendal@chromium.org
, Apr 7 2017