From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 353C8C43334 for ; Tue, 14 Jun 2022 11:54:50 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Message-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=/phTES0hK3HqPzHeg5ga1B6qE3Cspfub1F+hs92gnNg=; b=GdgfLqWXcRH3VL Y3Rn6XCzS+RRAhK/geQS1z+sWwn1PR63MaK6WNsNuPNPVft0g0iPAdlm+n9cnhTPtnAWbWgMAj7QH 3VuriemFayFKPYwfe/Uieko8cXHJESBWlFTfzgXOg/gMHVzlUWZln8ptvi5/FhVNCT0dgmu99Wq0l UlZiwLYN8xMTJgc3rKuM/kGGmKhL/YSkhT8/J+HXXaCA59PMUbEIg5X09vQDr7aIygPvnRgad1ZTT kS6b3i9BbC5N3BlT77iXXpLOjqeA61k38b3ZHamLBesC/A2v+kFnM4T2a0zDhWlTNPdVPLiZWedDI 1qWeWu9VLhoIJZbwSgdg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1o157D-009Kee-0t; Tue, 14 Jun 2022 11:53:47 +0000 Received: from ams.source.kernel.org ([145.40.68.75]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1o1579-009Kd4-5g for linux-arm-kernel@lists.infradead.org; Tue, 14 Jun 2022 11:53:44 +0000 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id A9076B8186E; Tue, 14 Jun 2022 11:53:41 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 8F399C3411B; Tue, 14 Jun 2022 11:53:35 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1655207620; bh=eEz9C3W3sm9wx9gzJgQmUp0IPoTdYsESOAuzf4ofhV8=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=JFLVXF5aq/bM8aDTLAKh6CruDhUXa5m/VL9QBtecheJnhFBgFzE0PJeO4vwRAIoS3 Kmg61dAuGQt4VrVpn3mq1JFh17zz9uDUcFN7whvPrwHKtc48a6Uv+uPbNCdODRPnj5 VnAA1wHMOPJ5MRo1xrbRNg1C9n50hOwotJgw0io0ssT9YXBeO6UoZDWOXozXrU5Em4 M2fFIMnyICMp0RHxgb3GzMjczSkbnJ8Zot0X8aYWRw9Wwvfhtu5Lr/w2gW0lwSkWnU N06iHshmvnSnCkCw5LBetxbmjljGSFDDlrmUEHPjWVopwcTBUy4ZiAagRS2bGr/pHO aqZJ39+TWBG+g== Date: Tue, 14 Jun 2022 13:02:49 +0100 From: Jonathan Cameron To: Subject: Re: [PATCH 11/16] iio: adc: at91-sama5d2_adc: add locking parameter to at91_adc_read_info_raw() Message-ID: <20220614130249.005ad51a@jic23-huawei> In-Reply-To: <2989a8a5-b350-aac1-dcfb-249ac5bb23aa@microchip.com> References: <20220609083213.1795019-1-claudiu.beznea@microchip.com> <20220609083213.1795019-12-claudiu.beznea@microchip.com> <20220611185851.4d266d5e@jic23-huawei> <2989a8a5-b350-aac1-dcfb-249ac5bb23aa@microchip.com> X-Mailer: Claws Mail 4.1.0 (GTK 3.24.34; x86_64-pc-linux-gnu) MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220614_045343_545210_D8CA053F X-CRM114-Status: GOOD ( 31.21 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: devicetree@vger.kernel.org, alexandre.belloni@bootlin.com, lars@metafoo.de, ludovic.desroches@atmel.com, linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org, robh+dt@kernel.org, Eugen.Hristev@microchip.com, krzk+dt@kernel.org, linux-arm-kernel@lists.infradead.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Tue, 14 Jun 2022 08:50:14 +0000 wrote: > On 11.06.2022 20:58, Jonathan Cameron wrote: > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > > > On Thu, 9 Jun 2022 11:32:08 +0300 > > Claudiu Beznea wrote: > > > >> Add a parameter to at91_adc_read_info_raw() to specify if st->lock mutex > >> need to be acquired. This prepares for the addition of temperature sensor > >> code which will re-use at91_adc_read_info_raw() function to read 2 voltages > >> for determining the real temperature. > > > > This looks like a potential lock dependency issue. > > iio_device_claim_direct_mode() takes an internal iio lock, and > > you then take st->lock. > > > > If you are going to invert that locking order in another path > > you have a deadlock. > > > > So rethink this. If you want to reuse the code you'll need to factor > > it out to a separate function that takes none of the locks then > > take all locks needed in each call path (in the same order). > > OK, I'll check it. Hi Claudia, Minor kernel mailing list etiquette thing is that there is no need to reply to say you'll check something or that you agree with review feedback. Just generates more emails to read. Reviewers assume anything you don't comment on their feedback will be addressed in next version of the code! Thanks, Jonathan > > > > > Jonathan > > > > > >> > >> Signed-off-by: Claudiu Beznea > >> --- > >> drivers/iio/adc/at91-sama5d2_adc.c | 15 ++++++++++----- > >> 1 file changed, 10 insertions(+), 5 deletions(-) > >> > >> diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c > >> index 1283bcf4e682..8f8fef42de84 100644 > >> --- a/drivers/iio/adc/at91-sama5d2_adc.c > >> +++ b/drivers/iio/adc/at91-sama5d2_adc.c > >> @@ -1583,7 +1583,8 @@ static irqreturn_t at91_adc_interrupt(int irq, void *private) > >> } > >> > >> static int at91_adc_read_info_raw(struct iio_dev *indio_dev, > >> - struct iio_chan_spec const *chan, int *val) > >> + struct iio_chan_spec const *chan, int *val, > >> + bool lock) > >> { > >> struct at91_adc_state *st = iio_priv(indio_dev); > >> int (*fn)(struct at91_adc_state *, int, u16 *) = NULL; > >> @@ -1602,13 +1603,15 @@ static int at91_adc_read_info_raw(struct iio_dev *indio_dev, > >> ret = iio_device_claim_direct_mode(indio_dev); > >> if (ret) > >> return ret; > >> - mutex_lock(&st->lock); > >> + if (lock) > >> + mutex_lock(&st->lock); > >> > >> if (fn) { > >> ret = fn(st, chan->channel, &tmp_val); > >> *val = tmp_val; > >> ret = at91_adc_adjust_val_osr(st, val); > >> - mutex_unlock(&st->lock); > >> + if (lock) > >> + mutex_unlock(&st->lock); > >> iio_device_release_direct_mode(indio_dev); > >> > >> return ret; > >> @@ -1644,7 +1647,8 @@ static int at91_adc_read_info_raw(struct iio_dev *indio_dev, > >> /* Needed to ACK the DRDY interruption */ > >> at91_adc_readl(st, LCDR); > >> > >> - mutex_unlock(&st->lock); > >> + if (lock) > >> + mutex_unlock(&st->lock); > >> > >> iio_device_release_direct_mode(indio_dev); > >> return ret; > >> @@ -1658,7 +1662,8 @@ static int at91_adc_read_raw(struct iio_dev *indio_dev, > >> > >> switch (mask) { > >> case IIO_CHAN_INFO_RAW: > >> - return at91_adc_read_info_raw(indio_dev, chan, val); > >> + return at91_adc_read_info_raw(indio_dev, chan, val, true); > >> + > >> case IIO_CHAN_INFO_SCALE: > >> *val = st->vref_uv / 1000; > >> if (chan->differential) > > > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel