From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dmitry Torokhov Subject: Re: [Letux-kernel] [PATCH 5/5] input: twl6040-vibra: remove mutex Date: Wed, 20 Apr 2016 10:49:51 -0700 Message-ID: <20160420174951.GB28831@dtor-ws> References: <592912a81a86fe0c9b5eaf3407cc5c4e1bfef9fe.1461009340.git.hns@goldelico.com> <20160418212048.GB1006@dtor-ws> <6E9FA6EF-660C-44AE-8BF4-03A21F0DB043@goldelico.com> <20160419080150.GB13933@dtor-ws> <3B181EC4-29FA-4C65-B33A-CC0D09E26E4D@goldelico.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Received: from mail-pa0-f53.google.com ([209.85.220.53]:33793 "EHLO mail-pa0-f53.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751002AbcDTRt6 (ORCPT ); Wed, 20 Apr 2016 13:49:58 -0400 Content-Disposition: inline In-Reply-To: Sender: linux-input-owner@vger.kernel.org List-Id: linux-input@vger.kernel.org To: "H. Nikolaus Schaller" Cc: Fabio Estevam , linux-input@vger.kernel.org, LKML , Peter Ujfalusi , kernel@pyra-handheld.com, Discussions about the Letux Kernel On Wed, Apr 20, 2016 at 11:22:53AM +0200, H. Nikolaus Schaller wrote: > > > Am 19.04.2016 um 10:08 schrieb H. Nikolaus Schaller : > > > > > >> Am 19.04.2016 um 10:01 schrieb Dmitry Torokhov : > >> > >> On Tue, Apr 19, 2016 at 09:49:01AM +0200, H. Nikolaus Schaller wrote: > >>> > >>>> Am 18.04.2016 um 23:20 schrieb Dmitry Torokhov : > >>>> > >>>> On Mon, Apr 18, 2016 at 09:55:41PM +0200, H. Nikolaus Schaller wrote: > >>>>> The mutex does not seem to be needed. > >>>> > >>>> twl6040_vibra_suspend() and vibra_play_work() may run concurrently, no? > >>> > >>> Hm. I don't know about the rule that would give an answer to this question... > >> > >> Sorry, that was actually a statement, not really a question. > > > > Indeed. In doubt about the answer we should take measures for the worst case. > > > >> It is > >> possible (although very unlikely) that userspace posts play request and > >> workqueue will not run until after suspend callback. > >> > >> Thinking about it some more I wonder if we better do what > >> twl6040_vibra_close() does and cancel the work before shutting off the > >> device, so that there is no chance of work executing after suspend > >> callback and reenabling the device. This way we can indeed remove the > >> mutex. > > > > Ok, I am fine with this. > > > > Will post an update ASAP. > > While doing testing I did run again into another locking related issue which I > had not yet tried to address with my patch set. It is a: > > BUG: scheduling while atomic > > report which sometimes ends in a kernel panic. > > I have attached such a log. vibra.py is here: > > http://git.goldelico.com/?p=gta04-kernel.git;a=blob;f=Letux/root/vibra.py;hb=refs/heads/letux-4.6-rc4 > > Basically it does an ioctl(EVIOCSFF) which triggers vibra_play. > > Maybe, can you decipher from the log what the reason could be? > > I only conjecture that it happens when vibra_play tries to read the regmap > of the twl6040 to get twl6040_get_vibralr_status (which has no pendant > in the twl4030 driver). > > Do we have to configure the twl6040 regmap differently? Right, vibra_play is called with interrupts disabled (that's why we have workqueue to enable/disable regulators, etc, when we start or stop vibration), so twl6040_get_vibralr_status() should not sleep, but apparently it does. Maybe the check for audio configuration should be moved into vibra_play_work(). Thanks. -- Dmitry