Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1946796AbbHHPkK (ORCPT ); Sat, 8 Aug 2015 11:40:10 -0400 Received: from out1-smtp.messagingengine.com ([66.111.4.25]:41874 "EHLO out1-smtp.messagingengine.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1946779AbbHHPkG (ORCPT ); Sat, 8 Aug 2015 11:40:06 -0400 X-Sasl-enc: Y/Pu2sDMftDTGKtWhqOyH84ZJgflsrmFFzWlvEG4gwzz 1439048402 Date: Sat, 8 Aug 2015 08:40:01 -0700 From: Greg KH To: Sebastian Andrzej Siewior Cc: Peter Hurley , Vinod Koul , Russell King , Dan Williams , dmaengine@vger.kernel.org, linux-kernel@vger.kernel.org, nsekhar@ti.com, linux-omap@vger.kernel.org, linux-serial@vger.kernel.org, john.ogness@linutronix.de, Peter Ujfalusi Subject: Re: [PATCH 1/3] tty: serial: 8250_omap: do not use RX DMA if pause is not supported Message-ID: <20150808154001.GB11851@kroah.com> References: <1438977619-15488-1-git-send-email-bigeasy@linutronix.de> <1438977619-15488-2-git-send-email-bigeasy@linutronix.de> <55C54D49.3090406@hurleysoftware.com> <55C5CC95.6030901@linutronix.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <55C5CC95.6030901@linutronix.de> User-Agent: Mutt/1.5.23+102 (2ca89bed6448) (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1849 Lines: 43 On Sat, Aug 08, 2015 at 11:32:05AM +0200, Sebastian Andrzej Siewior wrote: > On 08/08/2015 02:28 AM, Peter Hurley wrote: > >> diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c > >> index 0340ee6ba970..07a11e0935e4 100644 > >> --- a/drivers/tty/serial/8250/8250_omap.c > >> +++ b/drivers/tty/serial/8250/8250_omap.c > >> @@ -769,7 +771,9 @@ static void omap_8250_rx_dma_flush(struct uart_8250_port *p) > >> return; > >> } > >> > >> - dmaengine_pause(dma->rxchan); > >> + ret = dmaengine_pause(dma->rxchan); > >> + if (WARN_ON_ONCE(ret)) > >> + priv->rx_dma_broken = true; > > > > No offense, Sebastian, but it boggles my mind that anyone could defend this > > as solid api design. We're in the middle of an interrupt handler and the > > slave dma driver is /just/ telling us now that it doesn't implement this > > functionality?!!? > > This is the best thing I could come up with. But to be fair: This > happens once _very_ early in the process and is 100% reproducible. The > WARN_ON should trigger user attention. Never expect a _user_ to do anything with a WARN_ON except ignore it. WARN_ON is for developers when something really bad went wrong that you want to be notified so that you can fix the code to prevent that from ever happening again. So this isn't ok, sorry, a user would not know what to do with this at all except email me asking why the driver is broken because a kernel "oops" just happened, when in reality, their hardware is broken in a way that you already know about when you wrote the patch. You know better than this. greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/