From patchwork Mon Oct 15 01:53:36 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Kelly X-Patchwork-Id: 10641003 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 6DD8E157A for ; Mon, 15 Oct 2018 01:53:50 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 95BB328B58 for ; Mon, 15 Oct 2018 01:53:50 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 850C428DF7; Mon, 15 Oct 2018 01:53:50 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 912BD28B58 for ; Mon, 15 Oct 2018 01:53:49 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726526AbeJOJgu (ORCPT ); Mon, 15 Oct 2018 05:36:50 -0400 Received: from mail-pg1-f195.google.com ([209.85.215.195]:43667 "EHLO mail-pg1-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726524AbeJOJgu (ORCPT ); Mon, 15 Oct 2018 05:36:50 -0400 Received: by mail-pg1-f195.google.com with SMTP id 80-v6so8363594pgh.10 for ; Sun, 14 Oct 2018 18:53:47 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=martingkelly-com.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:date:message-id; bh=23fNynnvkb3Efhf+lkXDhQBvJiXD8nYuvDb6Qs9uXdk=; b=mQwuDhMjK4pYHHJYXye8PEUDgJue+nc3L4zFuBMgi1SPHPmbE1CAWN3GLNnSPIuNOP 9CHAPES5Cq/rgwlGHzx7zBc4vvZtlsK9P42GqACq4HB0uv6WFOq5Utmq36KeTpw+joBb tf3Xc9+/YljYDMcunmBHJiocJMoajjoF4TQ4Aeyy11vogD12TevAQc26AvCXE95WGRPL jlxXc4MZBsu/ZvjO0Fd4aGRZ5gIcPBBDJ1c9qk3Kl5pTDgRu/zlVn7x/Ufnuw8yZMj5Y wUpoAb87EV08qZ/QhfvYhuBYFK7YMnGsU+CbZFsf66lWnxsM3sR2khp2ZTtcPx3OUsNK 8DaQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=23fNynnvkb3Efhf+lkXDhQBvJiXD8nYuvDb6Qs9uXdk=; b=j/8ohGq5Lt5b7Wg6o80cCUeyvrb4DfniwrYihylYMQucopZfz25G1SEf+nIZ/q+u6k 3NAb4K6GImQKcoMQWHq9qzuO01gdNPCsR3JDfc3ubmtcENY//CBGLIHuL4EaxCMjmSjn SpGTxz528/vPzY9Pn8v4xJ7OVWxz4+Qc3Wgz6U0aDcqJzkdD8/v51tDAoJwnFoqXwt6c HCIBX6MvJ4wiEB460JSPnBTqbcDX1JKftuziIRiXUvYrz8leHIyOaP1N2zpVvSCOu3fo d9PxA90gmeQE7VAHml/AvVKQmjKgxc+06zkIAtm7BoNzJmHbEH5eNWErCf56F1EUcsIx CCJw== X-Gm-Message-State: ABuFfoh4OSY4KwvOki4DJ30gAym4xwZfSxW4ZFhlbcY72obWJIkI9lSX S94+j+8Vsu+8tR5dPg8yin2gvCOwtHo= X-Google-Smtp-Source: ACcGV61ZJWjR9TsgSjU3xgyGF0wLNpJWIw1N/wg2wHGfm5a9dsdyIT1eSUuo3KpCvdYLciaVgBitag== X-Received: by 2002:a63:a119:: with SMTP id b25-v6mr14233801pgf.186.1539568426943; Sun, 14 Oct 2018 18:53:46 -0700 (PDT) Received: from cascade.Home (71-212-17-184.tukw.qwest.net. [71.212.17.184]) by smtp.gmail.com with ESMTPSA id t22-v6sm12312916pfk.141.2018.10.14.18.53.46 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sun, 14 Oct 2018 18:53:46 -0700 (PDT) From: Martin Kelly X-Google-Original-From: Martin Kelly , Hartmut Knaack , Lars-Peter Clausen , Peter Meerwald-Stadler , Martin Kelly Subject: [RFC][PATCH] iio:st_magn: enable device after trigger Date: Sun, 14 Oct 2018 18:53:36 -0700 Message-Id: <20181015015336.21066-1-martin@martingkelly.com> X-Mailer: git-send-email 2.11.0 Sender: linux-iio-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-iio@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP From: Martin Kelly I'd like to get others' opinions on this patch before I submit it. It fixes an issue I've seen with a driver I'm working on (LSM9DS1 magnetometer driver, waiting on this before I send a patch). However, I have a few concerns about the generality of this approach: (1) Possible data races between st_sensors_set_enable and IRQ handlers, since both could run concurrently. Although we can protect this with a lock, we'd need to touch each driver, which will be tricky to test. Alternatively, we could audit each driver and see if there are any real races. The only property touched by st_sensors_set_enable appears to be the "enabled" property, which is unlikely to be read/written by any IRQ handlers. That said, the fact that the race exists makes me concerned about potential mistakes in the future. (2) If this is a good approach to solve the issue, then we should make the same change to the ST accelerometer, gyroscope, and pressure drivers. But again, we'd need to re-test every driver, which is tricky. I'd be happy to hear opinions or suggested alternative approaches. Thanks! Patch: -------------------------------------------------------------------------- Currently, we enable the device before we enable the device trigger. At high frequencies, this can cause interrupts that don't yet have a poll function associated with them and are thus treated as spurious. At high frequencies with level interrupts, this can even cause an interrupt storm of repeated spurious interrupts (~100,000 on my Beagleboard with the LSM9DS1 magnetometer). If these repeat too much, the interrupt will get disabled and the device will stop functioning. To prevent these problems, enable the device prior to enabling the device trigger, and disable the divec prior to disabling the trigger. This means there's no window of time during which the device creates interrupts but we have no trigger to answer them. Signed-off-by: Martin Kelly --- drivers/iio/magnetometer/st_magn_buffer.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) -- 2.11.0 diff --git a/drivers/iio/magnetometer/st_magn_buffer.c b/drivers/iio/magnetometer/st_magn_buffer.c index 0a9e8fadfa9d..37ab30566464 100644 --- a/drivers/iio/magnetometer/st_magn_buffer.c +++ b/drivers/iio/magnetometer/st_magn_buffer.c @@ -30,11 +30,6 @@ int st_magn_trig_set_state(struct iio_trigger *trig, bool state) return st_sensors_set_dataready_irq(indio_dev, state); } -static int st_magn_buffer_preenable(struct iio_dev *indio_dev) -{ - return st_sensors_set_enable(indio_dev, true); -} - static int st_magn_buffer_postenable(struct iio_dev *indio_dev) { int err; @@ -50,7 +45,7 @@ static int st_magn_buffer_postenable(struct iio_dev *indio_dev) if (err < 0) goto st_magn_buffer_postenable_error; - return err; + return st_sensors_set_enable(indio_dev, true); st_magn_buffer_postenable_error: kfree(mdata->buffer_data); @@ -63,11 +58,11 @@ static int st_magn_buffer_predisable(struct iio_dev *indio_dev) int err; struct st_sensor_data *mdata = iio_priv(indio_dev); - err = iio_triggered_buffer_predisable(indio_dev); + err = st_sensors_set_enable(indio_dev, false); if (err < 0) goto st_magn_buffer_predisable_error; - err = st_sensors_set_enable(indio_dev, false); + err = iio_triggered_buffer_predisable(indio_dev); st_magn_buffer_predisable_error: kfree(mdata->buffer_data); @@ -75,7 +70,6 @@ static int st_magn_buffer_predisable(struct iio_dev *indio_dev) } static const struct iio_buffer_setup_ops st_magn_buffer_setup_ops = { - .preenable = &st_magn_buffer_preenable, .postenable = &st_magn_buffer_postenable, .predisable = &st_magn_buffer_predisable, };