Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932512AbaJNOox (ORCPT ); Tue, 14 Oct 2014 10:44:53 -0400 Received: from icebox.esperi.org.uk ([81.187.191.129]:45603 "EHLO mail.esperi.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932147AbaJNOov (ORCPT ); Tue, 14 Oct 2014 10:44:51 -0400 From: Nix To: Johan Hovold Cc: Paul Martin , Oliver Neukum , Greg Kroah-Hartman , linux-kernel@vger.kernel.org Subject: Re: [3.16.1 BISECTED REGRESSION]: Simtec Entropy Key (cdc-acm) broken in 3.16 References: <878um4tg09.fsf@spindle.srvr.nix> <1409569752.24385.12.camel@linux-fkkt.site> <874mwnosz1.fsf@spindle.srvr.nix> <871tqe2zqt.fsf_-_@spindle.srvr.nix> <20141011195108.GA8275@thinkpad.nowster.org.uk> <87y4smz1l0.fsf@spindle.srvr.nix> <20141012185845.GB2786@localhost> <878uklynq9.fsf@spindle.srvr.nix> <20141014083432.GB7958@localhost> Emacs: featuring the world's first municipal garbage collector! Date: Tue, 14 Oct 2014 15:44:40 +0100 In-Reply-To: <20141014083432.GB7958@localhost> (Johan Hovold's message of "Tue, 14 Oct 2014 10:34:32 +0200") Message-ID: <87eguayalj.fsf@spindle.srvr.nix> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.4.50 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-DCC-dcc1-Metrics: spindle 1182; Body=5 Fuz1=5 Fuz2=5 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 14 Oct 2014, Johan Hovold spake thusly: > On Sun, Oct 12, 2014 at 10:36:30PM +0100, Nix wrote: >> On 12 Oct 2014, Johan Hovold verbalised: >> >> > On Sat, Oct 11, 2014 at 11:24:59PM +0100, Nix wrote: >> >> On 11 Oct 2014, Paul Martin spake thusly: >> >> >> >> > Having been privy to the firmware of the eKey, it is very simplisting, >> >> > with no implementation whatsoever of any flow control. >> >> >> >> That's what I thought. (Why would something that just provides data at a >> >> constant rate way below that of even the slowest USB bus *need* flow >> >> control?) >> >> >> >> One presumes therefore that the kernel suddenly trying to do flow >> >> control on shutdown would fubar the firmware's internal state, leading >> >> to the symptoms I see. >> > >> > The cdc-acm driver was dropping DTR/RTS on shutdown (close) also before >> > the commit you refer to. One thing it did change however is that this is >> > now only done if HUPCL is set. Might setting that flag be enough to >> > prevent the device firmware from crashing? >> >> If I read the ekeyd 1.1.5 source code correctly, this is already >> happening: >> >> ,----[ host/stream.c:estream_open() ] >> | } else if (S_ISCHR(sbuf.st_mode)) { >> | /* Open the file as a character device/tty */ >> | fd = open(uri, O_RDWR | O_NOCTTY); >> | if ((fd != -1) && (isatty(fd))) { >> | if (tcgetattr(fd, &settings) == 0 ) { >> | settings.c_cflag &= ~(CSIZE | CSTOPB | PARENB | CLOCAL | >> | CREAD | PARODD | CRTSCTS); >> | settings.c_iflag &= ~(BRKINT | IGNPAR | PARMRK | INPCK | >> | ISTRIP | INLCR | IGNCR | ICRNL | IXON | >> | IXOFF | IXANY | IMAXBEL); >> | settings.c_iflag |= IGNBRK; >> | settings.c_oflag &= ~(OPOST | OCRNL | ONOCR | ONLRET); >> | settings.c_lflag &= ~(ISIG | ICANON | IEXTEN | ECHO | >> | ECHOE | ECHOK | ECHONL | NOFLSH | >> | TOSTOP | ECHOPRT | ECHOCTL | ECHOKE); >> | settings.c_cflag |= CS8 | HUPCL | CREAD | CLOCAL; >> | #ifdef EKEY_FULL_TERMIOS >> | settings.c_cflag &= ~(CBAUD); >> | settings.c_iflag &= ~(IUTF8 | IUCLC); >> | settings.c_oflag &= ~(OFILL | OFDEL | NLDLY | CRDLY | TABDLY | >> | BSDLY | VTDLY | FFDLY | OLCUC ); >> | settings.c_oflag |= NL0 | CR0 | TAB0 | BS0 | VT0 | FF0; >> | settings.c_lflag &= ~(XCASE); >> | #endif >> | settings.c_cflag |= B115200; >> | if (tcsetattr(fd, TCSANOW, &settings) < 0) { >> `---- >> >> Note the HUPCL in there. >> >> I have checked: this code is being executed against a symlink that >> points to /dev/ttyACM0, and the tcsetattr() succeeds. (At least, it's >> succeeding on the kernel I'm running now, but of course that's 3.16.5 >> with this commit reverted...) > > You could verify that by enabling debugging in the cdc-acm driver and > making sure that the corresponding control messages are indeed sent on > close. Yeah, good idea -- in the specific context of the system rebooting, in particular (though I'll also see if just killing the daemon causes this problem, something I haven't tested). I was assuming it would be hard to see it before the reboot process cleared the screen -- but of course this machine has a serial console so that's not important. > But you haven't seen any fw crashes since you reverted the commit in > question? Not one. > Another thing you could try is to add back the > > acm_set_control(acm, 0); > > just after the dev_info message in probe. I'll try that tonight. -- NULL && (void) -- 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/