Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751664AbdGEN3W (ORCPT ); Wed, 5 Jul 2017 09:29:22 -0400 Received: from metis.ext.4.pengutronix.de ([92.198.50.35]:49057 "EHLO metis.ext.4.pengutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750893AbdGEN3V (ORCPT ); Wed, 5 Jul 2017 09:29:21 -0400 Date: Wed, 5 Jul 2017 15:29:10 +0200 From: Uwe =?iso-8859-1?Q?Kleine-K=F6nig?= To: Romain Perier Cc: Greg Kroah-Hartman , linux-serial@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Nandor Han Subject: Re: [PATCH v2 1/6] serial: imx: only set DMA rx-ing when DMA starts Message-ID: <20170705132910.nmixhnh4l6h2hlga@pengutronix.de> References: <20170705130706.10388-1-romain.perier@collabora.com> <20170705130706.10388-2-romain.perier@collabora.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20170705130706.10388-2-romain.perier@collabora.com> User-Agent: NeoMutt/20170113 (1.7.2) X-SA-Exim-Connect-IP: 2001:67c:670:100:5054:ff:fe2a:3aa X-SA-Exim-Mail-From: ukl@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: linux-kernel@vger.kernel.org Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1175 Lines: 35 Hello, On Wed, Jul 05, 2017 at 03:07:01PM +0200, Romain Perier wrote: > From: Nandor Han > > Avoid the situation when `dma_is_rxing` could incorrectly signal that > DMA RX channel is receiving data in case DMA preparation or sg mapping > fails. > > This commit fixes the issues by moving the assignment of dma_is_rxing > out of imx_disable_rx_int(), then the variable is set to 1 from > start_rx_dma() only when the preparation is correctly done. I'd write: There are a few issues with setting dma_is_rxing to 1 in imx_disable_rx_int: - Currently always after imx_disable_rx_int() the function start_rx_dma() is called. This dependency isn't obvious though. - start_rx_dma() does error checking and might exit without enabling DMA but keeping dma_is_rxing 1. So the more natural place for setting dma_is_rxing to 1 is in start_rx_dma after all errors are checked. If you use this, there is nothing left of Nandor Han's patch and you can drop his authorship. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-K?nig | Industrial Linux Solutions | http://www.pengutronix.de/ |