Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933956AbdC3OOt (ORCPT ); Thu, 30 Mar 2017 10:14:49 -0400 Received: from imap.thunk.org ([74.207.234.97]:40316 "EHLO imap.thunk.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933107AbdC3OOn (ORCPT ); Thu, 30 Mar 2017 10:14:43 -0400 Date: Thu, 30 Mar 2017 10:11:53 -0400 From: "Theodore Ts'o" To: Olliver Schinagl Cc: Vignesh R , Greg Kroah-Hartman , Jiri Slaby , Laxman Dewangan , Stephen Warren , Thierry Reding , Alexandre Courbot , "David S . Miller" , "dev@linux-sunxi.org" , Ed Blake , Andy Shevchenko , Alexander Sverdlin , Yegor Yefremov , Wan Ahmad Zainie , Kefeng Wang , Heikki Krogerus , Heiko Stuebner , Jason Uy , Douglas Anderson , Peter Hurley , Tony Lindgren , Thor Thayer , David Lechner , Jan Kiszka , "linux-serial@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "linux-tegra@vger.kernel.org" , "sparclinux@vger.kernel.org" Subject: Re: [PATCH] serial: Do not treat the IIR register as a bitfield Message-ID: <20170330141153.mvxa6iqj72hnhlu3@thunk.org> Mail-Followup-To: Theodore Ts'o , Olliver Schinagl , Vignesh R , Greg Kroah-Hartman , Jiri Slaby , Laxman Dewangan , Stephen Warren , Thierry Reding , Alexandre Courbot , "David S . Miller" , "dev@linux-sunxi.org" , Ed Blake , Andy Shevchenko , Alexander Sverdlin , Yegor Yefremov , Wan Ahmad Zainie , Kefeng Wang , Heikki Krogerus , Heiko Stuebner , Jason Uy , Douglas Anderson , Peter Hurley , Tony Lindgren , Thor Thayer , David Lechner , Jan Kiszka , "linux-serial@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "linux-tegra@vger.kernel.org" , "sparclinux@vger.kernel.org" References: <20170329184431.6226-1-oliver@schinagl.nl> <6be65e8b-eea4-08ed-0b30-5c0608764a83@ti.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: NeoMutt/20170113 (1.7.2) X-SA-Exim-Connect-IP: X-SA-Exim-Mail-From: tytso@thunk.org X-SA-Exim-Scanned: No (on imap.thunk.org); SAEximRunCond expanded to false Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1575 Lines: 46 While you're fixing this, there's a bug in samples/vfio-mdev/mtty.c: u8 ier = mdev_state->s[index].uart_reg[UART_IER]; *buf = 0; mutex_lock(&mdev_state->rxtx_lock); /* Interrupt priority 1: Parity, overrun, framing or break */ if ((ier & UART_IER_RLSI) && mdev_state->s[index].overrun) *buf |= UART_IIR_RLSI; /* Interrupt priority 2: Fifo trigger level reached */ if ((ier & UART_IER_RDI) && (mdev_state->s[index].rxtx.count == mdev_state->s[index].intr_trigger_level)) *buf |= UART_IIR_RDI; /* Interrupt priotiry 3: transmitter holding register empty */ if ((ier & UART_IER_THRI) && (mdev_state->s[index].rxtx.head == mdev_state->s[index].rxtx.tail)) *buf |= UART_IIR_THRI; /* Interrupt priotiry 4: Modem status: CTS, DSR, RI or DCD */ if ((ier & UART_IER_MSI) && (mdev_state->s[index].uart_reg[UART_MCR] & (UART_MCR_RTS | UART_MCR_DTR))) *buf |= UART_IIR_MSI; /* bit0: 0=> interrupt pending, 1=> no interrupt is pending */ if (*buf == 0) *buf = UART_IIR_NO_INT; It's treating the UART_IIR_* fields as a bitmask which is bad enough, but in the "Interrupt priority 4" case, UART_IIR_MSI is zero, so "*buf |= UART_IIR_MSI" is a no-op. And in the case where the modem status interrupt is the only thing set, *buf will be 0, and UART_IIR_NO_INT gets set erroneously. So this is another example of the bug of trying to treat the UART_IIR_* fields as a bitmask.... Yes, it's only sample code, but best fix it now before it gets copied elsewhere and metastisizes. :-) - Ted