From owner-freebsd-usb@FreeBSD.ORG  Thu Jul  3 15:24:08 2008
Return-Path: <owner-freebsd-usb@FreeBSD.ORG>
Delivered-To: usb@freebsd.org
Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34])
	by hub.freebsd.org (Postfix) with ESMTP id B264F106566C;
	Thu,  3 Jul 2008 15:24:08 +0000 (UTC)
	(envelope-from luigi@onelab2.iet.unipi.it)
Received: from onelab2.iet.unipi.it (onelab2.iet.unipi.it [131.114.9.129])
	by mx1.freebsd.org (Postfix) with ESMTP id 6E5828FC18;
	Thu,  3 Jul 2008 15:24:08 +0000 (UTC)
	(envelope-from luigi@onelab2.iet.unipi.it)
Received: by onelab2.iet.unipi.it (Postfix, from userid 275)
	id 4C6FA7308E; Thu,  3 Jul 2008 17:25:51 +0200 (CEST)
Date: Thu, 3 Jul 2008 17:25:51 +0200
From: Luigi Rizzo <rizzo@icir.org>
To: Gary Jennejohn <gary.jennejohn@freenet.de>
Message-ID: <20080703152551.GA73103@onelab2.iet.unipi.it>
References: <20080703140719.GA72315@onelab2.iet.unipi.it>
	<20080703170700.3b91b8c6@peedub.jennejohn.org>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20080703170700.3b91b8c6@peedub.jennejohn.org>
User-Agent: Mutt/1.4.2.3i
Cc: usb@freebsd.org, current@freebsd.org
Subject: Re: may I commit this small umodem patch ?
X-BeenThere: freebsd-usb@freebsd.org
X-Mailman-Version: 2.1.5
Precedence: list
List-Id: FreeBSD support for USB <freebsd-usb.freebsd.org>
List-Unsubscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-usb>,
	<mailto:freebsd-usb-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/freebsd-usb>
List-Post: <mailto:freebsd-usb@freebsd.org>
List-Help: <mailto:freebsd-usb-request@freebsd.org?subject=help>
List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-usb>,
	<mailto:freebsd-usb-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Thu, 03 Jul 2008 15:24:08 -0000

On Thu, Jul 03, 2008 at 05:07:00PM +0200, Gary Jennejohn wrote:
> On Thu, 3 Jul 2008 16:07:19 +0200
> Luigi Rizzo <rizzo@icir.org> wrote:
> 
> > There was a discussion back in september about adding
> > support for basic CDC tty devices in umodem.c.
> > This lets you talk to a number of usb devices built around
> > microcontrollers (e.g. Atmel), and puts us on par with
> > Linux and Windows in terms of supporting these devices.
> > 
> > Because this simply requires the small patch below to the
> > probe/attach routine, so if there are no objections I plan to add
> > this to the system (CURRENT then RELENG_7 and RELENG_6) in the next
> > few days.
> > 
> > > Index: umodem.c
> > > ===================================================================
> > > RCS file: /home/ncvs/src/sys/dev/usb/umodem.c,v
> > > retrieving revision 1.57
> > > diff -u -r1.57 umodem.c
> > > --- umodem.c	31 Jan 2005 13:58:10 -0000	1.57
> > > +++ umodem.c	20 Aug 2006 17:05:34 -0000
> > > @@ -256,6 +260,15 @@
> > >  	    id->bInterfaceProtocol == UIPROTO_CDC_AT)
> > >  		ret = UMATCH_IFACECLASS_IFACESUBCLASS_IFACEPROTO;
> > >  
> > > +#if 1
> > > +	if (ret == UMATCH_NONE &&
> > > +	    id->bInterfaceClass == UICLASS_CDC_DATA &&
> > > +	    id->bInterfaceSubClass == UISUBCLASS_DATA &&
> > > +	    id->bInterfaceProtocol == 0x00)
> > > +		ret = UMATCH_IFACECLASS_IFACESUBCLASS_IFACEPROTO;
> > > +		return ret;
> > > +#endif
> > > +
> > >  	if (ret == UMATCH_NONE)
> > >  		return (ret);
> > 
> 
> Is there any reason to keep the #if 1 ... #endif?  And why not just
> directly return UMATCH_IFACECLASS_IFACESUBCLASS_IFACEPROTO rather than
> assigning it to ret first?

in fact there are also missing braces that need to be
added -- as is, the code after the #endif is completely disabled.

thanks for the comment, but don't worry, the commit will be done
the right way.

cheers
luigi