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

Issue 639341 link

Starred by 1 user

Issue metadata

Status: Archived
Owner:
Last visit > 30 days ago
Closed: Mar 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

cros_ec_ring: simplify buffer

Project Member Reported by gwendal@chromium.org, Aug 19 2016

Issue description

As suggested by Jonathan Cameron <jic23@kernel.org>, cros_ec_ring uses a trigger_buffer, but it is not required given we always enable the trigger.

Use a simpler kfifo buffer:
  indio_dev->modes = INDIO_BUFFER_SOFTWARE;
  
  buffer = devm_iio_kfifo_allocate(&device->dev);
  if (!buffer) 
    return -ENOMEM;

  iio_device_attach_buffer(indio_dev, buffer);

In the notifier, use cros_ec_ring_handler() directly.
We won't trigger the interrupt on another thread, the notifier will appear stuck, but we are using the EC bus anyway, other notified won't be able to do anything with the EC anyway.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 28 2016

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

commit 357907813a8aaabaa813f71269348302cea3e496
Author: Gwendal Grignou <gwendal@chromium.org>
Date: Sat Aug 20 19:06:40 2016

CHROMIUM: iio: cros_ec: Remove cros_ec_sensor_ring trigger

Using a IRQ trigger is not needed for cros_ec_sensor_ring.
When the notifier indicates there are event, within that thread, collect
from the EC and process all the datum.
Using a separate thread is not useful, given it will take ownership of
the EC and prevents other notified from getting their data.

BUG= chromium:639341 
TEST=Check on Kevin with ARC++ that sensor data are still collected
properly.

Change-Id: I5dda516a88d8108ed2b020ae92c6053c718518f7
Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/373562
Reviewed-by: Guenter Roeck <groeck@chromium.org>

[modify] https://crrev.com/357907813a8aaabaa813f71269348302cea3e496/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_ring.c
[modify] https://crrev.com/357907813a8aaabaa813f71269348302cea3e496/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.h
[modify] https://crrev.com/357907813a8aaabaa813f71269348302cea3e496/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c

Comment 2 by moch@chromium.org, Aug 31 2016

Labels: Merge-Request-54

Comment 3 by dimu@chromium.org, Aug 31 2016

Labels: -Merge-Request-54 Merge-Approved-54 Hotlist-Merge-Approved
Owner: moch@chromium.org
Your change meets the bar and is auto-approved for M54 (branch: 2840)
Project Member

Comment 4 by bugdroid1@chromium.org, Aug 31 2016

Labels: merge-merged-release-R54-8743.B-chromeos-4.4
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/568b2e39ad26cb3a3258170efd19d268921671a9

commit 568b2e39ad26cb3a3258170efd19d268921671a9
Author: Gwendal Grignou <gwendal@chromium.org>
Date: Sat Aug 20 19:06:40 2016

CHROMIUM: iio: cros_ec: Remove cros_ec_sensor_ring trigger

Using a IRQ trigger is not needed for cros_ec_sensor_ring.
When the notifier indicates there are event, within that thread, collect
from the EC and process all the datum.
Using a separate thread is not useful, given it will take ownership of
the EC and prevents other notified from getting their data.

BUG= chromium:639341 
TEST=Check on Kevin with ARC++ that sensor data are still collected
properly.

Change-Id: I5dda516a88d8108ed2b020ae92c6053c718518f7
Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/373562
Reviewed-by: Guenter Roeck <groeck@chromium.org>
(cherry picked from commit 357907813a8aaabaa813f71269348302cea3e496)
Reviewed-on: https://chromium-review.googlesource.com/379563
Reviewed-by: Mohammed Habibulla <moch@google.com>
Commit-Queue: Mohammed Habibulla <moch@google.com>
Tested-by: Mohammed Habibulla <moch@google.com>

[modify] https://crrev.com/568b2e39ad26cb3a3258170efd19d268921671a9/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_ring.c
[modify] https://crrev.com/568b2e39ad26cb3a3258170efd19d268921671a9/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.h
[modify] https://crrev.com/568b2e39ad26cb3a3258170efd19d268921671a9/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c

Project Member

Comment 5 by bugdroid1@chromium.org, Aug 31 2016

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

commit 568b2e39ad26cb3a3258170efd19d268921671a9
Author: Gwendal Grignou <gwendal@chromium.org>
Date: Sat Aug 20 19:06:40 2016

CHROMIUM: iio: cros_ec: Remove cros_ec_sensor_ring trigger

Using a IRQ trigger is not needed for cros_ec_sensor_ring.
When the notifier indicates there are event, within that thread, collect
from the EC and process all the datum.
Using a separate thread is not useful, given it will take ownership of
the EC and prevents other notified from getting their data.

BUG= chromium:639341 
TEST=Check on Kevin with ARC++ that sensor data are still collected
properly.

Change-Id: I5dda516a88d8108ed2b020ae92c6053c718518f7
Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/373562
Reviewed-by: Guenter Roeck <groeck@chromium.org>
(cherry picked from commit 357907813a8aaabaa813f71269348302cea3e496)
Reviewed-on: https://chromium-review.googlesource.com/379563
Reviewed-by: Mohammed Habibulla <moch@google.com>
Commit-Queue: Mohammed Habibulla <moch@google.com>
Tested-by: Mohammed Habibulla <moch@google.com>

[modify] https://crrev.com/568b2e39ad26cb3a3258170efd19d268921671a9/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_ring.c
[modify] https://crrev.com/568b2e39ad26cb3a3258170efd19d268921671a9/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.h
[modify] https://crrev.com/568b2e39ad26cb3a3258170efd19d268921671a9/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c

Project Member

Comment 6 by bugdroid1@chromium.org, Sep 1 2016

Labels: merge-merged-factory-gru-8652.B-chromeos-4.4
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/4888b42a1659fcb4b54353d80655646a432665f2

commit 4888b42a1659fcb4b54353d80655646a432665f2
Author: Gwendal Grignou <gwendal@chromium.org>
Date: Sat Aug 20 19:06:40 2016

CHROMIUM: iio: cros_ec: Remove cros_ec_sensor_ring trigger

Using a IRQ trigger is not needed for cros_ec_sensor_ring.
When the notifier indicates there are event, within that thread, collect
from the EC and process all the datum.
Using a separate thread is not useful, given it will take ownership of
the EC and prevents other notified from getting their data.

BUG= chromium:639341 
TEST=Check on Kevin with ARC++ that sensor data are still collected
properly.

Change-Id: I5dda516a88d8108ed2b020ae92c6053c718518f7
Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/373562
Reviewed-by: Guenter Roeck <groeck@chromium.org>
(cherry picked from commit 357907813a8aaabaa813f71269348302cea3e496)
Reviewed-on: https://chromium-review.googlesource.com/377739
Tested-by: Jongpil Jung <jongpil19.jung@samsung.com>
Reviewed-by: Philip Chen <philipchen@chromium.org>
Commit-Queue: Philip Chen <philipchen@chromium.org>

[modify] https://crrev.com/4888b42a1659fcb4b54353d80655646a432665f2/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_ring.c
[modify] https://crrev.com/4888b42a1659fcb4b54353d80655646a432665f2/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.h
[modify] https://crrev.com/4888b42a1659fcb4b54353d80655646a432665f2/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c

Project Member

Comment 7 by sheriffbot@chromium.org, Sep 4 2016

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 8 by sheriffbot@chromium.org, Sep 7 2016

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 9 by sheriffbot@chromium.org, Oct 20 2016

Labels: -Merge-Approved-54
This issue hasn't been updated in the last 6 weeks, so removing its merge approval label. Please re-request a merge if needed.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 10 by moch@chromium.org, Mar 4 2017

Status: Fixed (was: Untriaged)

Comment 11 by dchan@google.com, May 30 2017

Labels: VerifyIn-60
Labels: VerifyIn-61

Comment 13 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Sign in to add a comment