Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752042AbaLOJmM (ORCPT ); Mon, 15 Dec 2014 04:42:12 -0500 Received: from mail-lb0-f169.google.com ([209.85.217.169]:56103 "EHLO mail-lb0-f169.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750861AbaLOJmK (ORCPT ); Mon, 15 Dec 2014 04:42:10 -0500 Date: Mon, 15 Dec 2014 10:42:06 +0100 From: Johan Hovold To: George McCollister Cc: Johan Hovold , linux-usb@vger.kernel.org, open list , Greg Kroah-Hartman Subject: Re: [PATCH] USB: serial: add nt124 usb to serial driver Message-ID: <20141215094206.GA6778@localhost> References: <1418081057-25283-1-git-send-email-george.mccollister@gmail.com> <20141210130412.GJ14346@localhost> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.22 (2013-10-16) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Dec 12, 2014 at 09:01:03AM -0600, George McCollister wrote: > On Wed, Dec 10, 2014 at 7:04 AM, Johan Hovold wrote: > > On Mon, Dec 08, 2014 at 05:24:17PM -0600, George McCollister wrote: > >> + switch (termios->c_cflag & CSIZE) { > > > > C_CSIZE(tty) > Okay > > > >> + case CS5: > >> + newline.bDataBits = 5; > >> + break; > >> + case CS6: > >> + newline.bDataBits = 6; > >> + break; > I should probably just remove CS5 and CS6 since I don't think they > will work anyway. Ok. Remember to update the passed termios with the settings that you actually end up using (i.e. settings from old_termios). > >> +static int nt124_open(struct tty_struct *tty, > >> + struct usb_serial_port *port) > >> +{ > >> + struct nt124_private *priv = usb_get_serial_port_data(port); > >> + int result = 0; > >> + unsigned long flags; > >> + > >> + spin_lock_irqsave(&port->lock, flags); > >> + port->throttled = 0; > >> + port->throttle_req = 0; > >> + spin_unlock_irqrestore(&port->lock, flags); > >> + > >> + priv->flowctrl = 0; > > > > Drop this and keep the current settings. > Okay > > > >> + nt124_set_termios(tty, port, NULL); > >> + nt124_set_flowctrl(port, priv->flowctrl); > > > > Drop this. This is handled by tty and dtr_rts(). > Okay > > > >> + > >> + if (port->bulk_in_size) > >> + result = usb_serial_generic_submit_read_urbs(port, GFP_KERNEL); > > > > Call usb_serial_generic_open() instead (and skip the throttle-flags bit > > above). > Okay, so looks like the only thing I will do in this function is call > nt124_set_termios() followed by usb_serial_generic_open(). Yes, that should do it. > >> +static struct usb_serial_driver nt124_device = { > >> + .driver = { > >> + .owner = THIS_MODULE, > >> + .name = "nt124", > >> + }, > >> + .id_table = id_table, > >> + .num_ports = 1, > >> + .bulk_in_size = 32, > >> + .bulk_out_size = 32, > > > > Why do you reduce these buffer sizes? They can be bigger than the > > endpoint size for increased throughput. > I'll leave them out like most of the other drivers (and retest) unless > you have another suggestion. That's perfectly fine, and means that the buffer sizes will match the endpoint sizes (they will in fact never be smaller than that). Johan -- 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/