svn commit: r202243 - in head: share/man/man4 sys/dev/usb sys/dev/usb/net

Ben Kaduk minimarmot at gmail.com
Wed Jan 13 21:27:30 UTC 2010


Lots of grammar nits.  See below.

On Wed, Jan 13, 2010 at 3:54 PM, Andrew Thompson <thompsa at freebsd.org> wrote:
> Author: thompsa
> Date: Wed Jan 13 20:54:18 2010
> New Revision: 202243
> URL: http://svn.freebsd.org/changeset/base/202243
>
> Log:
>  Update to Fredrik's latest uhso driver. This changes port detection, adds
>  comments and other code nits.
>
>  Submitted by: Fredrik Lindberg <fli at shapeshifter.se>
>
> Modified:
>  head/share/man/man4/uhso.4
>  head/sys/dev/usb/net/uhso.c
>  head/sys/dev/usb/usbdevs
>
> Modified: head/share/man/man4/uhso.4
> ==============================================================================
> --- head/share/man/man4/uhso.4  Wed Jan 13 20:51:23 2010        (r202242)
> +++ head/share/man/man4/uhso.4  Wed Jan 13 20:54:18 2010        (r202243)
> @@ -23,11 +23,11 @@
>  .\"
>  .\" $FreeBSD$
>  .\"
> -.Dd Aug 12, 2009
> +.Dd January 14, 2010
>  .Os
>  .Dt UHSO 4
>  .Sh NAME
> -.Nm hso
> +.Nm uhso
>  .Nd support for several HSxPA devices from Option N.V.
>  .Sh SYNOPSIS
>  The module can be loaded at boot time by placing the following line in
> @@ -47,12 +47,12 @@ driver which makes them behave like a
>  .Xr tty 4 .
>  The packet interface is exposed as a network interface.
>  .Pp
> -To establish a connection on the packet interface the use of the proprietary
> +Connection of the packet interface is achieved by using the proprietary

This sentence sounds subtly wrong to me.  I think that just saying
"connection to"
instead of "of" will make it grammatically correct, but I worry that
it may no longer
be factually correct, then.  Perhaps "A connection on [...]", instead?

>  AT commands
>  .Dq Li AT_OWANCALL
>  and
>  .Dq Li AT_OWANDATA
> -are required on any of the serial ports.
> +on any of the available serial ports.
>  .Pp
>  The network interface must be configured manually using the data obtain from
>  these calls.
> @@ -70,12 +70,23 @@ driver supports at least the following c
>  Option GlobeSurfer iCON 7.2 (new firmware)
>  .It
>  Option iCON 225
> +.It
> +Option iCON 505
>  .El
>  .Pp
>  The device features a mass storage device referred to as
>  .Dq Zero-CD
> -which contains drivers for Microsoft Windows.
> -The driver automatically switches the device to modem mode.
> +which contains drivers for Microsoft Windows, this is the default

Comma splice.  I would put a semicolon, but a full stop is acceptable, too.

> +mode for the device.
> +The
> +.Nm
> +driver automatically switches the device from
> +.Dq Zero-CD
> +mode to modem mode.
> +This behavior can be disabled by setting
> +.Va hw.usb.uhso.auto_switch
> +to 0 using
> +.Xr sysctl 8
>  .Sh EXAMPLES
>  Establishing a packet interface connection
>  .Bd -literal -offset indent
>
> Modified: head/sys/dev/usb/net/uhso.c
> ==============================================================================
> --- head/sys/dev/usb/net/uhso.c Wed Jan 13 20:51:23 2010        (r202242)
> +++ head/sys/dev/usb/net/uhso.c Wed Jan 13 20:54:18 2010        (r202243)
> @@ -145,12 +146,14 @@ struct uhso_softc {
>  #define UHSO_MPORT_TYPE_APP    0x01    /* Application */
>  #define UHSO_MPORT_TYPE_PCSC   0x02
>  #define UHSO_MPORT_TYPE_GPS    0x03
> -#define UHSO_MPORT_TYPE_APP2   0x04
> +#define UHSO_MPORT_TYPE_APP2   0x04    /* Secondary application */
>  #define UHSO_MPORT_TYPE_MAX    UHSO_MPORT_TYPE_APP2
>  #define UHSO_MPORT_TYPE_NOMAX  8       /* Max number of mux ports */
>
>  /*
>  * Port definitions
> + * Note that these definitions are arbitray and doesn't match
> + * values returned by the auto config descriptor.

"arbitrary"; "don't"; "the values"

>  */
>  #define UHSO_PORT_TYPE_CTL     0x01
>  #define UHSO_PORT_TYPE_APP     0x02
> @@ -560,6 +567,18 @@ uhso_attach(device_t self)
>            CTLFLAG_RD, uhso_port[UHSO_IFACE_PORT(sc->sc_type)], 0,
>            "Port available at this interface");
>
> +       /*
> +        * The default interface description on most Option devices aren't

"isn't"

> +        * very helpful. So we skip device_set_usb_desc and set the
> +        * device description manually.
> +        */
> +       device_set_desc_copy(self, uhso_port_type[UHSO_IFACE_PORT_TYPE(sc->sc_type)]);
> +       /* Announce device */
> +       device_printf(self, "<%s port> at <%s %s> on %s\n",
> +           uhso_port_type[UHSO_IFACE_PORT_TYPE(sc->sc_type)],
> +           uaa->device->manufacturer, uaa->device->product,
> +           device_get_nameunit(uaa->device->bus->bdev));
> +
>        if (sc->sc_ttys > 0) {
>                SYSCTL_ADD_INT(sctx, SYSCTL_CHILDREN(soid), OID_AUTO, "ports",
>                    CTLFLAG_RD, &sc->sc_ttys, 0, "Number of attached serial ports");
> @@ -798,27 +832,41 @@ uhso_probe_iface(struct uhso_softc *sc,
>        sc->sc_type = type;
>        iface = usbd_get_iface(sc->sc_udev, index);
>
> -       if (UHSO_IFACE_USB_TYPE(type) & (UHSO_IF_MUX | UHSO_IF_NET)) {
> -               error0 = uhso_attach_muxserial(sc, iface, type);
> +       if (UHSO_IFACE_PORT_TYPE(type) == UHSO_PORT_TYPE_NETWORK) {
>                error = uhso_attach_ifnet(sc, iface, type);
> -
> -               if (error0 && error)
> +               if (error) {
> +                       UHSO_DPRINTF(1, "uhso_attach_ifnet failed");
>                        return (ENXIO);
> +               }
>
> -               if (sc->sc_ttys > 0) {
> +               /*
> +                * If there is an additional interrupt endpoint on this
> +                * interface we most likley have a multiplexed serial port

"likely".  I might put a comma before "we", while you're here.

> +                * available.
> +                */
> +               if (iface->idesc->bNumEndpoints < 3) {
> +                       sc->sc_type = UHSO_IFACE_SPEC(
> +                           UHSO_IFACE_USB_TYPE(type) & ~UHSO_IF_MUX,
> +                           UHSO_IFACE_PORT(type) & ~UHSO_PORT_SERIAL,
> +                           UHSO_IFACE_PORT_TYPE(type));
> +                       return (0);
> +               }
> +
> +               UHSO_DPRINTF(1, "Trying to attach mux. serial\n");
> +               error = uhso_attach_muxserial(sc, iface, type);
> +               if (error == 0 && sc->sc_ttys > 0) {
>                        error = ucom_attach(&sc->sc_super_ucom, sc->sc_ucom,
>                            sc->sc_ttys, sc, &uhso_ucom_callback, &sc->sc_mtx);
>                        if (error) {
>                                device_printf(sc->sc_dev, "ucom_attach failed\n");
>                                return (ENXIO);
>                        }
> -               }
>
> -               mtx_lock(&sc->sc_mtx);
> -               usbd_transfer_start(sc->sc_xfer[UHSO_MUX_ENDPT_INTR]);
> -               mtx_unlock(&sc->sc_mtx);
> -       }
> -       else if ((UHSO_IFACE_USB_TYPE(type) & UHSO_IF_BULK) &&
> +                       mtx_lock(&sc->sc_mtx);
> +                       usbd_transfer_start(sc->sc_xfer[UHSO_MUX_ENDPT_INTR]);
> +                       mtx_unlock(&sc->sc_mtx);
> +               }
> +       } else if ((UHSO_IFACE_USB_TYPE(type) & UHSO_IF_BULK) &&
>            UHSO_IFACE_PORT(type) & UHSO_PORT_SERIAL) {
>
>                error = uhso_attach_bulkserial(sc, iface, type);
> @@ -914,6 +977,10 @@ uhso_attach_muxserial(struct uhso_softc
>        return (0);
>  }
>
> +/*
> + * Interrupt callback for the multiplexed serial port. Indicates
> + * which serial port that has data waiting.

"that" is unnecessary, here.

> + */
>  static void
>  uhso_mux_intr_callback(struct usb_xfer *xfer, usb_error_t error)
>  {
> @@ -1470,7 +1529,11 @@ tr_setup:
>  }
>
>  /*
> - * Defered RX processing, called with mutex locked.
> + * Deferred RX processing, called with mutex locked.
> + *
> + * Each frame we receive might contain several small ip-packets aswell

"as well"

-Ben Kaduk


More information about the svn-src-head mailing list