kernel: 4.14 update cros_ec_sensor_ring |
||||
Issue description4.14 sensor_ring is behind its 4.4 counterpart: git log --oneline --left-right --graph --cherry-pick cros/chromeos-4.4...HEAD drivers/iio/common/cros_ec_sensors/cros_ec_sensors_ring.c Shows the latest changes for ARCore are missing: < 78ea86c66f254 CHROMIUM: cros_ec: enable tight_timestamps automatically in sensor_ring < 49864518ef212 cros_ec: fix the timestamp comparator used to obtain medians < ff122fe0872dd (cros/stabilize-11101.B-chromeos-4.4) cros_ec: flip the comparison used to clear the filter history < 9d940d7b699e5 CHROMIUM: iio: cros_ec: Reset filter when there've been no recent interrupts < 92a6a71a1e5ab CHROMIUM: cros_ec_sensor_ring: Deal with interleaved samples < 7f2196441f4a8 CHROMIUM: iio: cros_ec: Add back the legacy timestamp processing code < 5c206eb768bd7 FIXUP: CHROMIUM: iio: cros_ec: Add predictive filter on fifo_timestamp < 4f2d6d214d832 CHROMIUM: iio: cros_ec: Add predictive filter on fifo_timestamp < a51a04df4c512 FIXUP: CHROMIUM: iio: cros_ec: Spread timestamps better < 3af8b8cf09074 CHROMIUM: iio: cros_ec: Spread timestamps better < 597d29e5f00d5 CHROMIUM: cros-ec: record event timestamp in the hard irq Having 4.14 up to date will make 4.19 rebase easier.
,
Oct 17
,
Oct 17
4.19 will have the same problem very soon (probably more important).
,
Oct 17
How soon is "very soon"?
,
Oct 30
,
Oct 31
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/448f9550647ca51d00a999a96003cd28a553df4a commit 448f9550647ca51d00a999a96003cd28a553df4a Author: Alexandru M Stan <amstan@chromium.org> Date: Wed Oct 31 19:42:21 2018 CHROMIUM: iio: cros_ec: Spread timestamps better Sometimes the EC receives only one interrupt (hence timestamp) for a batch of samples. Only the first sample will have the correct timestamp. So we must interpolate the other samples. We use the previous batch timestamp and our current batch timestamp as a way to calculate period, then spread the samples evenly. s0 int, 0ms s1 int, 10ms s2 int, 20ms 30ms point goes by, no interrupt, previous one is still asserted downloading s2 and s3 s3 sample, 20ms (incorrect timestamp) s4 int, 40ms The batches are [(s0), (s1), (s2, s3), (s4)]. Since the 3rd batch has 2 samples in them, we adjust the timestamp of s3. s2 - s1 = 10ms, so s3 must be s2 + 10ms => 20ms. If s1 would have been part of a bigger batch things would have gotten a little more complicated. BUG=b:73551961, b:67743747, chromium:896347 TEST="help" or "gpioget" on the EC does not cause a blip on the period graph Change-Id: Ie0dc866bb909144dc20e8e59463abd855add2e75 Signed-off-by: Alexandru M Stan <amstan@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/1064477 Reviewed-by: Gwendal Grignou <gwendal@chromium.org> FIXUP: CHROMIUM: iio: cros_ec: Spread timestamps better Commit 3af8b8cf09074 ("CHROMIUM: iio: cros_ec: Spread timestamps better") causes test builds to fail. drivers/iio/common/cros_ec_sensors/cros_ec_sensors_ring.c: In function 'cros_ec_ring_fifo_toggle': drivers/iio/common/cros_ec_sensors/cros_ec_sensors_ring.c:107:2: error: 'for' loop initial declarations are only allowed in C99 or C11 mode Furthermore, there are build warnings. drivers/iio/common/cros_ec_sensors/cros_ec_sensors_ring.c: In function cros_ec_ring_handler: drivers/iio/common/cros_ec_sensors/cros_ec_sensors_ring.c:352:28: warning: format %d expects argument of type int, but argument 6 has type s64 {aka long long int} BUG=b:73551961, b:67743747, chromium:896347 TEST="Build with allmodconfig kernel configurations" Change-Id: Ib9d64c1c21dafecbda405822339e171a6e992cb9 Signed-off-by: Guenter Roeck <groeck@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/1085382 Reviewed-by: Alexandru M Stan <amstan@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/1308055 Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com> Tested-by: Enrico Granata <egranata@chromium.org> [modify] https://crrev.com/448f9550647ca51d00a999a96003cd28a553df4a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_ring.c
,
Nov 20
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/9af26e57e8a528204d727072911f5788a25651b3 commit 9af26e57e8a528204d727072911f5788a25651b3 Author: Alexandru M Stan <amstan@chromium.org> Date: Tue Nov 20 03:13:46 2018 CHROMIUM: iio: cros_ec: Add predictive filter on fifo_timestamp fifo_timestamp is recorded in the AP interrupt (which sometimes has higher latency), if we were to just do a' = c - b + a, we would couple all the jitter from the interrupt into the timestamps of sensor samples from different bursts of data. Here's an attempt at decomposing (c[n], b[n]), (c[n-1], b[n-1]) into the slope (labeled m), median filtering the last 30 ms. calculate a good offset too (also using a median filter), then figure out a'. BUG=b:67743747, b:73552314, chromium:896347 TEST=Graph the period of a sensor, note how jitter stays under 200us peak-to-peak, especially when system is doing other stuff TEST=Graph c-b*1000, see the line be pretty jaggedy, graph cros_ec_ring_ts_filter(b)-b*1000, see it be super smooth Change-Id: I8a9ea489fe793655feba4ed89359155dfd7a359b Suggested-by: Gwendal Grignou <gwendal@chromium.org> Suggested-by: David Schneider <dnschneid@chromium.org> Signed-off-by: Alexandru M Stan <amstan@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/899776 Reviewed-by: Gwendal Grignou <gwendal@chromium.org> FIXUP: CHROMIUM: iio: cros_ec: Add predictive filter on fifo_timestamp Avoid ERROR: "__aeabi_ldivmod" [drivers/iio/common/cros_ec_sensors/cros_ec_sensors_ring.ko] undefined! by using 64-bit divide operations where needed. Also fix the following sparse warnings. drivers/iio/common/cros_ec_sensors/cros_ec_sensors_ring.c:161:67: sparse: Using plain integer as NULL pointer drivers/iio/common/cros_ec_sensors/cros_ec_sensors_ring.c:220:6: sparse: symbol 'cros_ec_ring_ts_filter_update' was not declared. Should it be static? drivers/iio/common/cros_ec_sensors/cros_ec_sensors_ring.c:301:5: sparse: symbol 'cros_ec_ring_ts_filter' was not declared. Should it be static? drivers/iio/common/cros_ec_sensors/cros_ec_sensors_ring.c:808:25: sparse: symbol 'cros_ec_ring_pm_ops' was not declared. Should it be static? BUG=b:67743747, b:73552314, chromium:896347 TEST=Build on 32-bit target Change-Id: I43140614eafeaa2004546bbf3fdf084869703a37 Signed-off-by: Guenter Roeck <groeck@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/1094983 Reviewed-by: Alexandru M Stan <amstan@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/1308057 Commit-Ready: Enrico Granata <egranata@chromium.org> Tested-by: Enrico Granata <egranata@chromium.org> Reviewed-by: Gwendal Grignou <gwendal@chromium.org> [modify] https://crrev.com/9af26e57e8a528204d727072911f5788a25651b3/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_ring.c
,
Nov 20
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/874a5da607c32610ae03d627340401f7e45ee4d1 commit 874a5da607c32610ae03d627340401f7e45ee4d1 Author: Alexandru M Stan <amstan@chromium.org> Date: Tue Nov 20 03:13:48 2018 CHROMIUM: iio: cros_ec: Add back the legacy timestamp processing code Turns out the new code is not happy if used with the old firmware. "New code" consists of: * CHROMIUM: iio: cros_ec: Spread timestamps better * CHROMIUM: iio: cros_ec: Add predictive filter on fifo_timestamp We'll have to keep the old code since there's a ton of corner cases for the old firmware behavior that's required to pass CTS. The new code can be enabled based on the tight_timestamps variable. Ideally we want the new/old kernel behavior based on a feature flag that the EC exposes (TODO), but for now we allow it to be switched via the "tight_timestamps" userspace attribute in /sys/bus/iio/devices. Summary of changes: * Moved new spreading code from inline to cros_ec_ring_spread_add() * Copied old spreading code from before commit 3af8b8cf0907 ("CHROMIUM: iio: cros_ec: Spread timestamps better") to cros_ec_ring_spread_add_legacy() * Added state->tight_timestamps variable, default false * Added IIO_DEVICE_ATTR for tight_timestamps * Based on if tight_timestamps==1 * Conditionally used timestamp filtering * Conditionally used cros_ec_ring_spread_add{,legacy}() * last_batch_len[] is now an int, no need for it to be 64 bits. * div64 in places * Small fixups CQ-DEPEND=CL:1128257 TEST=With old scarlet firmware, CTS still passes TEST=With new scarlet firmware (and udev hack, CL:1128257), timestamps are up to the 100us jitter spec BUG=b/111079027, b/109786990, chromium:896347 Change-Id: I6e8b4d53f7172fa47ba8e9725387ec4b0ddbc374 Signed-off-by: Alexandru M Stan <amstan@chromium.org> Signed-off-by: Gwendal Grignou <gwendal@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/1123651 Reviewed-on: https://chromium-review.googlesource.com/1308058 Commit-Ready: Enrico Granata <egranata@chromium.org> Tested-by: Enrico Granata <egranata@chromium.org> [modify] https://crrev.com/874a5da607c32610ae03d627340401f7e45ee4d1/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_ring.c
,
Nov 20
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/fad4e8e8eeb62fef62813de4220ca72003ccdfef commit fad4e8e8eeb62fef62813de4220ca72003ccdfef Author: Gwendal Grignou <gwendal@chromium.org> Date: Tue Nov 20 03:13:49 2018 CHROMIUM: cros_ec_sensor_ring: Deal with interleaved samples In the new timestamp spreading algoirthm deal with samples of different types that come from the same sensor: TS ACCEL1 GYRO1 ACCEL2 GYRO2 .. Before, ACCEL1 and ACCEL2 would have the same TS timestamp. After, ACCEL1 and ACCEL2 are spread. BUG=b:73551961, chromium:896347 TEST=Compile, CTS tests that passed still do. Checked with tight_timestamps set to 1. TEST=Start 2 sensor types, graph 1, cause a batch with "help" on the EC, stuff is correct now Change-Id: I9de978a43d444fb7e6237bfb2e37e7eca5e89e2a Signed-off-by: Gwendal Grignou <gwendal@chromium.org> Signed-off-by: Alexandru M Stan <amstan@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/1128501 Reviewed-on: https://chromium-review.googlesource.com/1308059 Commit-Ready: Enrico Granata <egranata@chromium.org> Tested-by: Enrico Granata <egranata@chromium.org> [modify] https://crrev.com/fad4e8e8eeb62fef62813de4220ca72003ccdfef/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_ring.c
,
Nov 20
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/2c07c775c8db176f09c82b4b60405ad9b6235b6c commit 2c07c775c8db176f09c82b4b60405ad9b6235b6c Author: Alexandru M Stan <amstan@chromium.org> Date: Tue Nov 20 03:13:51 2018 CHROMIUM: iio: cros_ec: Reset filter when there've been no recent interrupts When something (ex: CTS) turns off sensors for a while, changes the state of the CPU (eg: load => heat => clock drift) then starts the sensors again, the filter is not going to be able to smoothly transition from the old drift to the new one with the sudden change in drift. We should just start from scratch to make sure there's no major discontinuity in the data. Hopefully there are no big irq jitters around that time. For now every time there's a bigger gap than 500ms, we clear the filter and only start using it again after it has at least 8 irq entries in the history. TEST=Change TS_HISTORY_BORED_US to something much lower, now a simple batch will cause it to clear history, note how there's no big discontinuity (beyond the irq jitter that the filter is supposed to solve) in the period graph. TEST=CTS should fail less, especially around the beginning of a test. BUG=b:67743747, chromium:896347 Change-Id: Ie638451091287d4a7486021d9dff83f394d983c0 Signed-off-by: Alexandru M Stan <amstan@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/1136000 Tested-by: Gwendal Grignou <gwendal@chromium.org> Reviewed-by: Gwendal Grignou <gwendal@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/1308060 Commit-Ready: Enrico Granata <egranata@chromium.org> Tested-by: Enrico Granata <egranata@chromium.org> [modify] https://crrev.com/2c07c775c8db176f09c82b4b60405ad9b6235b6c/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_ring.c
,
Nov 20
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/5a9f7be497c0026a911119b71b1731d3626bad5b commit 5a9f7be497c0026a911119b71b1731d3626bad5b Author: Enrico Granata <egranata@chromium.org> Date: Tue Nov 20 03:13:52 2018 CHROMIUM: cros_ec: flip the comparison used to clear the filter history The filter history should be cleared when *more* than 500ms have elapsed. The code as written did the opposite of the intent. Reverse the direction of the comparison in order to be true to the intended meaning. BUG=b:112111610, chromium:896347 TEST=checked incorrect behavior on Nocturne Change-Id: Iab10b98ba9a08d8c93c333408c0cb32a48c67bfe Signed-off-by: Enrico Granata <egranata@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/1237603 Reviewed-by: Gwendal Grignou <gwendal@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/1308061 [modify] https://crrev.com/5a9f7be497c0026a911119b71b1731d3626bad5b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_ring.c
,
Nov 20
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/f9fe66f76f88c26c5df47efd75aab19952fa6682 commit f9fe66f76f88c26c5df47efd75aab19952fa6682 Author: Enrico Granata <egranata@chromium.org> Date: Tue Nov 20 03:13:53 2018 CHROMIUM: cros_ec: fix the timestamp comparator used to obtain medians Implementing the comparator by the difference of the timestamps has the potential to return an incorrect result due to signed integer truncation rules. Fix this by spelling out what we truly want the comparator to do. BUG=b:112111610, chromium:896347 TEST=observe no bad sorts in filter output on Nocturne Change-Id: Ifdceee6c579f51260771ffbf3f46c1fb4a883a3f Signed-off-by: Enrico Granata <egranata@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/1229185 Reviewed-by: Gwendal Grignou <gwendal@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/1308062 [modify] https://crrev.com/f9fe66f76f88c26c5df47efd75aab19952fa6682/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_ring.c
,
Jan 10
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/ec03d15d862b5a44fa426eb072b671999d351f86 commit ec03d15d862b5a44fa426eb072b671999d351f86 Author: Enrico Granata <egranata@chromium.org> Date: Thu Jan 10 21:58:40 2019 CHROMIUM: cros_ec: enable tight_timestamps automatically in sensor_ring This commit teaches cros_ec_sensors_ring how to read the EC feature flags to detect support for tight timestamps and - if support is detected - automatically enables tight_timestamps. BUG=b:112111610, chromium:896347 TEST=tight_timestamps automatically enabled on Nocturne Conflict: Unmerged paths: (use "git add/rm <file>..." as appropriate to mark resolution) deleted by us: drivers/platform/chrome/cros_ec_dev.c Resolved: cros_ec_dev.c sits at drivers/mfd in kernel 4.14 Conflict: include/linux/mfd/cros_ec.h Resolved: moved cros_ec_remove comment below Reviewed-on: https://chromium-review.googlesource.com/1255904 Reviewed-by: Alexandru M Stan <amstan@chromium.org> Change-Id: I275640a58e38fd10b71bc56567d2b22b03e93dba Signed-off-by: Enrico Granata <egranata@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/1308064 Commit-Ready: Jett Rink <jettrink@chromium.org> Reviewed-by: Aaron Durbin <adurbin@chromium.org> [modify] https://crrev.com/ec03d15d862b5a44fa426eb072b671999d351f86/include/linux/mfd/cros_ec.h [modify] https://crrev.com/ec03d15d862b5a44fa426eb072b671999d351f86/drivers/mfd/cros_ec_dev.c [modify] https://crrev.com/ec03d15d862b5a44fa426eb072b671999d351f86/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_ring.c |
||||
►
Sign in to add a comment |
||||
Comment 1 by gwendal@chromium.org
, Oct 17