Received: by 2002:a05:7412:e794:b0:fa:551:50a7 with SMTP id o20csp2984184rdd; Sat, 13 Jan 2024 09:46:47 -0800 (PST) X-Google-Smtp-Source: AGHT+IFNrIC0AONZcXpVOMkxnO0XgwqZZgwq69QpKYKDtNC5ujWkAvK+ABa3B0Z4ajhvVYhS3RLS X-Received: by 2002:a05:6870:6f0c:b0:206:a09e:5ffd with SMTP id qw12-20020a0568706f0c00b00206a09e5ffdmr4215073oab.14.1705168007006; Sat, 13 Jan 2024 09:46:47 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1705168006; cv=none; d=google.com; s=arc-20160816; b=B62mCHJx/4/fsXhwTWLIYz62O9dOG8S//rRkVjWk17bdqbveV7jSawXCwreom9139T iX7KQTjjCzRMyDY8jm7QA7Y/Rr+IylyoHDlatOT6E6Rf6Fd9TbZpjchRKUcVhonGWYbS jCtyvzG6/XQBEdNhScJrxsoPA+onzIQEMYvp4/YeV4TzMUZwnByVFsNJPUU557dBi2YR fM52+q5PkWf2tfnnzujLSUS+zxUq2LZ/mVH4RZ20peNT8LOZUo9j8yMacqBPJDAkg72k tBgoWyGzV0YRracIHI9Wv+67LDngJrU+//qMUnPA2yqgbbC4eZ98TUkhBzTB+y621kxJ xucw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:mime-version:list-unsubscribe :list-subscribe:list-id:precedence:references:in-reply-to:message-id :subject:cc:to:from:date:dkim-signature; bh=TAdgY9nRGXfwo8rujhJ+dWdpLFZ+ragR1XDVvOaWlzA=; fh=ulEsowzlAtPDy3g4/fQyA2ZCVAC7qp3vvIu5ug5ACmY=; b=d1279ARDgA5YyPRAFtlP1iDMMlE37d5eIH4vQew+/UoOzNPUoKd/34cDeOvWwMPywF n9MBSCl6xppkF/oNPwixho4clLW8DqqhcTpHBCMuUqrGFzBvTRMiHO4PXUyp+pD8d3Rh OT8bUds7Br8xgFr+P2JiEeLMBLSaOQbdgn0eGi+y+jtPAbjkvMctZmTPdMhlGDoN+MpT t7ZXGPgg1cC54gtBMmtgudlcQsr/YABeRfouiijzF3txsjnaDy3M8i8zYfPOUBYN4et/ HKO6OlTB+D0IRWOAlf75Hl35M4c30/jmVr/1OMloPNJ71Ts9lRJ+fJTHCWtOy0rabFOi CkgA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b="le3Wx2T/"; spf=pass (google.com: domain of linux-kernel+bounces-25343-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) smtp.mailfrom="linux-kernel+bounces-25343-linux.lists.archive=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Return-Path: Received: from sv.mirrors.kernel.org (sv.mirrors.kernel.org. [2604:1380:45e3:2400::1]) by mx.google.com with ESMTPS id y63-20020a638a42000000b005cdec41efddsi5527191pgd.150.2024.01.13.09.46.46 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 13 Jan 2024 09:46:46 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel+bounces-25343-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b="le3Wx2T/"; spf=pass (google.com: domain of linux-kernel+bounces-25343-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) smtp.mailfrom="linux-kernel+bounces-25343-linux.lists.archive=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by sv.mirrors.kernel.org (Postfix) with ESMTPS id D682C282FF3 for ; Sat, 13 Jan 2024 17:46:45 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 84E555681; Sat, 13 Jan 2024 17:46:38 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="le3Wx2T/" Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id AD2815663; Sat, 13 Jan 2024 17:46:37 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 9D8C8C433C7; Sat, 13 Jan 2024 17:46:35 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1705167997; bh=0aCEhZFZIhna+PX3Z+MkI3nkB05GmNoXnwGBZrWka8w=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=le3Wx2T/O/pRlZfxR/qYM0M0IBrHkfmIZLEgmSzK6YDuaoJ+7gzL2eQ4EwrtNSC9v 6gvwGrsFyYd3OWK5bWyL9K4/miSlwf+U5T0JbZGNudFHYym+cqExYreMozkCKOcWtv /bSwUYbZimmBvwzxuwY/p1PvCxFm37FgNRJC4tN4zbAXnlpsFvIZanUG8BjG3lRt2G 0hzixJiO/tmfeiu578B40n7idViC9A37Z+mJ6qw5HkNdtN20pG67eLrDFtlP9OqAue C8LkWxgIZ5iHyt/VErYLoRMvp0mcim0OfvENjzD6paVaZwLn2PN+7zU58z+bYe6+cN 8mMQJgFyI9CWg== Date: Sat, 13 Jan 2024 17:46:25 +0000 From: Jonathan Cameron To: Denis Benato Cc: Lars-Peter Clausen , linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org, Jagath Jog J , Uwe =?UTF-8?B?S2xlaW5lLUvDtm5pZw==?= Subject: Re: iio: iio-trig-hrtimer bug on suspend/resume when used with bmi160 and bmi323 Message-ID: <20240113174351.47a20239@jic23-huawei> In-Reply-To: <31d7f7aa-e834-4fd0-a66a-e0ff528425dc@gmail.com> References: <31d7f7aa-e834-4fd0-a66a-e0ff528425dc@gmail.com> X-Mailer: Claws Mail 4.2.0 (GTK 3.24.39; x86_64-pc-linux-gnu) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit On Wed, 10 Jan 2024 23:35:01 +0100 Denis Benato wrote: > Hello, > > With this mail I am submitting bug report that is probably related to > iio-trig-hrtimer but there is also the possibility for it to be > specific to bmi160 and bmi323. > > The described problem have been reproduced on my handheld PC (Asus > RC71L) and in another handheld PC with two different gyroscope > drivers: bmi323 (backported by me on v6.7, on RC71L) and bmi160. > > My target hardware (RC71L that yeld to this discovery) has a bmi323 > chip that does not have any interrupt pins reaching the CPU, yet I > need to fetch periodically data from said device, therefore I used > iio-trig-hrtimer: created a trigger, set the device and trigger > sampling frequencies, bound the trigger to the device and enabled > buffer: data is being read and available over /dev/iio:device0. > > While in this state if I suspend my handheld I receive (from dmesg) > the warning reported below and at resume data is not coming out of > the iio device and the hrtimer appears to not be working. If I create > a new trigger and bind the new trigger to said iio device and > re-enable buffer data does come out of /dev/iio:device0 once more, > until the next sleep. > > Since this is important to me I have taken the time to look at both > drivers and iio-trig-hrtimer and I have identified three possible > reasons: > > 1) iio-trig-hrtimer won't work after suspend regardless of how it is > used (this is what I believe is the cause) me too. > 2) iio-trig-hrtimer is stopped by the -ESHTDOWN returned by the > function printing "Transfer while suspended", however that stack > trace does not include function calls related to iio-trig-hrtimer and > this seems less plausible 3) bmi160 and bmi323 appears to be similar > and maybe are sharing a common bug with suspend (this is also why I > have maintainers of those drivers in the recipient list) > > Thanks for your time, patience and understanding, Hi Denis, I suspect this is the legacy of the platform I used to test the hrtimer and similar triggers on never had working suspend and resume (we ripped support for that board out of the kernel a while back now...) Hence those paths were never tested by me and others may not have cared about this particular case. Anyhow, so I think what is going on is fairly simple. There is no way for a driver to indicate to a trigger provided by a separate module / hardware device that it should stop triggering data capture. The driver in question doesn't block data capture when suspended, which would be easy enough to add but doesn't feel like the right solution. So my initial thought is that we should add suspend and resume callbacks to iio_trigger_ops and call them manually from iio device drivers in their suspend and resume callbacks. These would simply pause whatever the trigger source was so that no attempts are made to trigger the use of the device when it is suspended. It gets a little messy though as triggers can be shared between multiple devices so we'd need to reference count suspend and resume for the trigger to make sure we only resume once all consumers of the trigger have said they are ready to cope with triggers again. As mentioned, the alternative would be to block the triggers at ingress to the bmi323 and bmi160 drivers. There may be a helpful pm flag that could be used but if not, then setting an is_suspended flag under the data->mutex to avoid races. and reading it in the trigger_handler to decide whether to talk to the device should work. I'd kind of like the generic solution of actually letting the trigger know, but not sure how much work it would turn out to be. Either way there are a lot of drivers to fix this problem in as in theory most triggers can be used with most drivers that support buffered data capture. There may also be fun corners where a hardware trigger from one IIO device A is being used by another B and the suspend timing is such that B finishing with the trigger results in A taking an action (in the try_reenable callback) that could result in bus traffic. That one is going to be much more fiddly to handle than the simpler case you have run into. Thanks for the detailed bug report btw. To get it fixed a few questions: 1) Are you happy to test proposed fixes? 2) Do you want to have a go at fixing it yourself? (I'd suggest trying the fix in the bmi323 driver first rather than looking at the other solution) If we eventually implement the more general version, then a bmi323 additional protection against this problem would not be a problem. Clearly I'd like the answers to be yes to both questions, but up to you! Jonathan