From owner-svn-src-all@FreeBSD.ORG Tue Mar 11 07:22:24 2014 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id A571173E; Tue, 11 Mar 2014 07:22:24 +0000 (UTC) Received: from mail108.syd.optusnet.com.au (mail108.syd.optusnet.com.au [211.29.132.59]) by mx1.freebsd.org (Postfix) with ESMTP id 4E697CCF; Tue, 11 Mar 2014 07:22:23 +0000 (UTC) Received: from c122-106-147-133.carlnfd1.nsw.optusnet.com.au (c122-106-147-133.carlnfd1.nsw.optusnet.com.au [122.106.147.133]) by mail108.syd.optusnet.com.au (Postfix) with ESMTPS id D8B741A2325; Tue, 11 Mar 2014 18:22:15 +1100 (EST) Date: Tue, 11 Mar 2014 18:22:14 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Marcel Moolenaar Subject: Re: svn commit: r262996 - head/sys/dev/uart In-Reply-To: <201403110320.s2B3KBqu023414@svn.freebsd.org> Message-ID: <20140311173639.K2716@besplex.bde.org> References: <201403110320.s2B3KBqu023414@svn.freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.1 cv=ddC5gxne c=1 sm=1 tr=0 a=7NqvjVvQucbO2RlWB8PEog==:117 a=PO7r1zJSAAAA:8 a=jYpDk86rP-AA:10 a=kj9zAlcOel0A:10 a=JzwRw_2MAAAA:8 a=aEWq4vkuDa4fM4Uz3EUA:9 a=CjuIK1q_8ugA:10 Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 11 Mar 2014 07:22:24 -0000 On Tue, 11 Mar 2014, Marcel Moolenaar wrote: > Log: > Pass the actual baudrate to tty_init_console(). This defines the initial > baudrate of the device special file, and makes sure that on open(2) the > UART is programmed with the correct baudrate. This then eliminates the > need in uart_tty_param() to override the speed setting. Here are my old patches (except the one for uart, and larger updates for sio) to do this for more console drivers. Actually, I mainly added tty_init_console() calls that are completely missing (these are more needed when the non-console case is fixed so that the default isn't for consoles). Most of the drivers are virtual ones that don't have a speed. Only sio and uart were tested at runtime. bvm seems to be the only new console driver which is missing a tty_init_console() call after these patches. % diff -c2 ./dev/cfe/cfe_console.c~ ./dev/cfe/cfe_console.c % *** ./dev/cfe/cfe_console.c~ Sat Dec 24 05:21:14 2011 % --- ./dev/cfe/cfe_console.c Sat Dec 24 05:21:16 2011 % *************** % *** 90,93 **** % --- 90,94 ---- % cfe_consdev.cn_name[0] != '\0') { % tp = tty_alloc(&cfe_ttydevsw, NULL); % + tty_init_console(tp, 0); % tty_makedev(tp, NULL, "cfecons"); % } % diff -c2 ./dev/ofw/ofw_console.c~ ./dev/ofw/ofw_console.c % *** ./dev/ofw/ofw_console.c~ Sat Dec 24 05:21:27 2011 % --- ./dev/ofw/ofw_console.c Sat Dec 24 05:21:29 2011 % *************** % *** 98,101 **** % --- 98,102 ---- % */ % tp = tty_alloc(&ofw_ttydevsw, NULL); % + tty_init_console(tp, 0); % tty_makedev(tp, NULL, "%s", output); % tty_makealias(tp, "ofwcons"); % diff -c2 ./dev/syscons/syscons.c~ ./dev/syscons/syscons.c % *** ./dev/syscons/syscons.c~ Tue Apr 10 01:48:10 2012 % --- ./dev/syscons/syscons.c Tue Apr 10 01:48:11 2012 % *************** % *** 560,564 **** % */ % } % ! % dev = make_dev(&consolectl_devsw, 0, UID_ROOT, GID_WHEEL, 0600, % "consolectl"); % --- 562,567 ---- % */ % } % ! if (sc_console_unit == unit) % ! tty_init_console(sc->dev[0], 0); % dev = make_dev(&consolectl_devsw, 0, UID_ROOT, GID_WHEEL, 0600, % "consolectl"); % diff -c2 ./dev/usb/serial/usb_serial.c~ ./dev/usb/serial/usb_serial.c % *** ./dev/usb/serial/usb_serial.c~ Tue Apr 10 01:48:34 2012 % --- ./dev/usb/serial/usb_serial.c Tue Apr 10 01:48:57 2012 % *************** % *** 331,336 **** % % tp = tty_alloc_mutex(&ucom_class, sc, sc->sc_mtx); % - if (tp == NULL) % - return (ENOMEM); % % /* Check if the client has a custom TTY name */ -current already has my patches for usb only, except for this hunk. % --- 331,334 ---- % *************** % *** 369,376 **** % ucom_cons_softc = sc; % % ! memset(&t, 0, sizeof(t)); % ! t.c_ispeed = ucom_cons_baud; % ! t.c_ospeed = t.c_ispeed; % t.c_cflag = CS8; % % mtx_lock(ucom_cons_softc->sc_mtx); % --- 367,375 ---- % ucom_cons_softc = sc; % % ! tty_init_console(tp, ucom_cons_baud); % ! t = tp->t_termios_init_in; % ! #ifdef GARBAGE /* maybe usb can't do all modes, but CS8 by itself is invalid */ % t.c_cflag = CS8; % + #endif % % mtx_lock(ucom_cons_softc->sc_mtx); % *************** % *** 1080,1084 **** % % /* XXX the TTY layer should call "open()" first! */ % ! % error = ucom_open(tp); % if (error) { % --- 1079,1087 ---- % % /* XXX the TTY layer should call "open()" first! */ % ! /* % ! * Not quite; its ordering is partly backwards, but some % ! * parameters must be set early in ttydev_open(), possibly % ! * before calling ttydevsw_open(). % ! */ % error = ucom_open(tp); % if (error) { % *************** % *** 1091,1094 **** % --- 1094,1098 ---- % /* Check requested parameters. */ % if (t->c_ispeed && (t->c_ispeed != t->c_ospeed)) { % + /* XXX c_ospeed == 0 is perfectly valid. */ % DPRINTF("mismatch ispeed and ospeed\n"); % error = EINVAL; -current has the rest of these usb patches to a fault. It has this XXX comment about the broken ospeed (ospeed == 0 means hang up), but not the fix. Testing ispeed is also strange -- ispeed == 0 has no special meaning. uart gets this wrong in a lesser way: when ospeed == 0, it returns after hanging up without setting any other parameters. tcsetattr() is specified to attempt set _all_ the supported attributes requested. Then if it succeeds in setting a single attribute, it must return success, and if it failed to set a single attribute then it must return failure. When it succeeds in setting ospeed == 0, that is success for the call. The error handling for this is onerous for applications and most don't do it on success. On success, non-broken error handling has to do a tcgetattr() and check which attributes are actually set and decide whether it cares about others. For the ospeed == 0 case, it would see that no attributes were changed and would notice the uart bug if it requested a change of another attribute that it cares about (it has to discard the change to ospeed == 0 before comparing with the old attributes, since ospeed == 0 is physically impossible so it would be another bug for this setting to be sticky at the software level). If it doesn't want to change anything but just wants to hang up, then it should use tcgetattr() before tcsetattr and only request changing ospeed. % diff -c2 ./dev/xen/console/console.c~ ./dev/xen/console/console.c % *** ./dev/xen/console/console.c~ Sat Dec 24 05:20:43 2011 % --- ./dev/xen/console/console.c Sat Dec 24 05:20:45 2011 % *************** % *** 234,237 **** % --- 234,238 ---- % % xccons = tty_alloc(&xc_ttydevsw, NULL); % + tty_init_console(xccons, 0); % tty_makedev(xccons, NULL, "xc%r", 0); % % diff -c2 ./ia64/ia64/ssc.c~ ./ia64/ia64/ssc.c % *** ./ia64/ia64/ssc.c~ Sat Dec 24 05:24:12 2011 % --- ./ia64/ia64/ssc.c Sat Dec 24 05:24:13 2011 % *************** % *** 118,121 **** % --- 118,122 ---- % % tp = tty_alloc(&ssc_class, NULL); % + tty_init_console(tp, 0); % tty_makedev(tp, NULL, "ssccons"); % } Bruce