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 B63B9C433F5 for ; Thu, 5 May 2022 15:19:23 +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:In-Reply-To:MIME-Version:References: 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=yTyqu9XYd1XYKR87xgekR7wVoXOdwHbjG7PwZvbIye8=; b=InoVKEq78b4whE qgibH3CUC+iYfDo26eaOHqaReEiFKUjPTyA0KoIv+ZjOdDUaVn8CxAKAXH8olIeE1RT8tyT4Iqg/p QJwrte+Ir3LCv2Um8+VWAhxeP0W8b3GzUtIoqxs/T3+lsKujCHJLGkvtJFfTuClP86AMkqFahZ65O EP0GEcCkW7sUSddpu0fhNZVa9sxQyTJvUo89zYA16M0HsNUMRHSdImGqJ0v0xBEwEz4lvFEHUS7az dn3SC+csTorAnxrpQQUOeiB+4pYIL9Iwe1DllMRHg76k81UMBEKOMFdCt6Aij2SINccNOV77C8lDx UStpHb8GLelX+bEQ0JwQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nmdFI-00GcHp-EE; Thu, 05 May 2022 15:18:24 +0000 Received: from dfw.source.kernel.org ([2604:1380:4641:c500::1]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1nmdFE-00GcGp-Uk for linux-arm-kernel@lists.infradead.org; Thu, 05 May 2022 15:18:22 +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 dfw.source.kernel.org (Postfix) with ESMTPS id 0FA7C61CF4; Thu, 5 May 2022 15:18:20 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id CBF23C385AE; Thu, 5 May 2022 15:18:18 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1651763899; bh=BSLHwZBU7CEcphBe/qi0nQ6hse+0DaLEQ9dWC+R/3Fc=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=EuJrI7SOU5Y/UcoEhlXrTw6pKOQ0AMRNAdrLI8NhFbPYVnDqpyFZBpA1gqepEkxZN bvvH6XGUgEOAU5/weZia3rDkML8glIxPAzFij2pwQP75xBfwZ3xJo7SMqMVg+R09Sn 5rkCau7yV2ibfVTurh6YLgJvoOLa4vTdhHcnxmI9PCGpM2y3FTeST6TcsDGdWLz5OX Yk0K99EeTAcCAcSvI/A40vnGeicpMjeK42bmvMZ+Xa8d64Fi+Digwj1UrhWo4U/nwz flMRmQgjS0gH5ApSf35HEqY+HAV+dx9b7oeI2Fg/mVW5uaecVh9/q/5gmYq5tQ7Fdw 5AVkxyu8gFfDA== Date: Thu, 5 May 2022 08:18:17 -0700 From: Nathan Chancellor To: Michael Walle Subject: Re: [PATCH v2] i2c: at91: use dma safe buffers Message-ID: References: <20220407150828.202513-1-michael@walle.cc> <54f02090e6621460b66dc9d233d53283@walle.cc> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <54f02090e6621460b66dc9d233d53283@walle.cc> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220505_081821_119644_8450BD98 X-CRM114-Status: GOOD ( 30.76 ) 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: Alexandre Belloni , llvm@lists.linux.dev, linux-kernel@vger.kernel.org, Wolfram Sang , linux-i2c@vger.kernel.org, Codrin Ciubotariu , Claudiu Beznea , 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 Thu, May 05, 2022 at 05:11:30PM +0200, Michael Walle wrote: > [+ Wolfram] > > Am 2022-05-05 16:51, schrieb Nathan Chancellor: > > On Thu, Apr 07, 2022 at 05:08:28PM +0200, Michael Walle wrote: > > > The supplied buffer might be on the stack and we get the following > > > error > > > message: > > > [ 3.312058] at91_i2c e0070600.i2c: rejecting DMA map of vmalloc > > > memory > > > > > > Use i2c_{get,put}_dma_safe_msg_buf() to get a DMA-able memory region > > > if > > > necessary. > > > > > > Fixes: 60937b2cdbf9 ("i2c: at91: add dma support") > > > Signed-off-by: Michael Walle > > > Reviewed-by: Codrin Ciubotariu > > > --- > > > changes since v1: > > > - remove extra empty line > > > - add fixes tag as suggested by Codrin > > > > > > drivers/i2c/busses/i2c-at91-master.c | 11 +++++++++++ > > > 1 file changed, 11 insertions(+) > > > > > > diff --git a/drivers/i2c/busses/i2c-at91-master.c > > > b/drivers/i2c/busses/i2c-at91-master.c > > > index b0eae94909f4..5eca3b3bb609 100644 > > > --- a/drivers/i2c/busses/i2c-at91-master.c > > > +++ b/drivers/i2c/busses/i2c-at91-master.c > > > @@ -656,6 +656,7 @@ static int at91_twi_xfer(struct i2c_adapter > > > *adap, struct i2c_msg *msg, int num) > > > unsigned int_addr_flag = 0; > > > struct i2c_msg *m_start = msg; > > > bool is_read; > > > + u8 *dma_buf; > > > > > > dev_dbg(&adap->dev, "at91_xfer: processing %d messages:\n", num); > > > > > > @@ -703,7 +704,17 @@ static int at91_twi_xfer(struct i2c_adapter > > > *adap, struct i2c_msg *msg, int num) > > > dev->msg = m_start; > > > dev->recv_len_abort = false; > > > > > > + if (dev->use_dma) { > > > + dma_buf = i2c_get_dma_safe_msg_buf(m_start, 1); > > > + if (!dma_buf) { > > > + ret = -ENOMEM; > > > + goto out; > > > + } > > > + dev->buf = dma_buf; > > > + } > > > + > > > ret = at91_do_twi_transfer(dev); > > > + i2c_put_dma_safe_msg_buf(dma_buf, m_start, !ret); > > > > > > ret = (ret < 0) ? ret : num; > > > out: > > > -- > > > 2.30.2 > > > > > > > > > > This change as commit 03fbb903c8bf ("i2c: at91: use dma safe buffers") > > causes the following clang warning: > > Ugh.. > > > drivers/i2c/busses/i2c-at91-master.c:707:6: error: variable 'dma_buf' > > is used uninitialized whenever 'if' condition is false > > [-Werror,-Wsometimes-uninitialized] > > if (dev->use_dma) { > > ^~~~~~~~~~~~ > > drivers/i2c/busses/i2c-at91-master.c:717:27: note: uninitialized use > > occurs here > > i2c_put_dma_safe_msg_buf(dma_buf, m_start, !ret); > > ^~~~~~~ > > drivers/i2c/busses/i2c-at91-master.c:707:2: note: remove the 'if' if > > its condition is always true > > if (dev->use_dma) { > > ^~~~~~~~~~~~~~~~~~ > > drivers/i2c/busses/i2c-at91-master.c:659:13: note: initialize the > > variable 'dma_buf' to silence this warning > > u8 *dma_buf; > > ^ > > = NULL > > 1 error generated. > > > > Should this variable be initialized or should the call to > > i2c_put_dma_safe_msg_buf() be moved into the if statement? > > Initializing dma_buf with NULL should to the trick because > i2c_put_dma_safe_msg_buf() is handling NULL gracefully. No > need for an extra "if (dma_buf)". > > Will you take care or should I send a patch? I can send a patch, thank you for the input! Cheers, Nathan _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel