Am Freitag, 19. Juni 2009 13:51:25 schrieb Richard Ash:
> In particular, the USB 2.0 series devices have only on bulk in and out
> endpoint, which is shared between the multiple ports using headers on
> each URB (as far as I can see based on the vendor driver code). I'm not
> clear what the best way of handling this is - do I need to initialise
> the endpoints / URBs, and if so, what function do it do it in? Will
> keeping the URBs in port 0's structure and then referencing them there
> as required be a good idea or a bad one?
In this case it is easiest to allocate the write URBs dynamically
and free them on completion.
Regards
Oliver
> +/* structure in which to keep all the messy stuff that this driver
> needs
> + * alongside the usb_serial_port structure */
> +struct quatech2_port {
> + int magic;
> + char active; /* someone has this device open */
> + unsigned char *xfer_to_tty_buffer;
> + wait_queue_head_t wait;
> + int open_count; /* number of times this port has been opened */
> + struct semaphore sem; /* locks this structure */
> + __u8 shadowLCR; /* last LCR value received */
> + __u8 shadowMCR; /* last MCR value received */
> + __u8 shadowMSR; /* last MSR value received */
> + __u8 shadowLSR; /* last LSR value received */
> + char open_ports; /* ports open on whole device */
> + char RxHolding;
> + char Rcv_Flush;
> + char Xmit_Flush;
> + char closePending;
> + char fifo_empty_flag;
> + int xmit_pending_bytes;
> + int xmit_fifo_room_bytes;
> + struct semaphore pend_xmit_sem; /* locks this structure */
> + spinlock_t lock;
> +};
What do you need two semaphores for?
> +static int qt2_attach(struct usb_serial *serial)
> +{
> + struct usb_serial_port *port;
> + struct quatech2_port *qt2_port;
> + int i;
> +
> + /* Now setup per port private data, which replaces all the things
> + * that quatech added to standard kernel structures in their driver */
> + for (i = 0; i < serial->num_ports; i++) {
> + port = serial->port[i];
> + qt2_port = kzalloc(sizeof(*qt2_port), GFP_KERNEL);
> + if (!qt2_port) {
> + dbg("%s: kmalloc for quatech2_port (%d) failed!.",
> + __func__, i);
> + return -ENOMEM;
memory leak in error case
> + }
> + spin_lock_init(&qt2_port->lock);
> + if (i == 0)
> + qt2_port->open_ports = 0; /* no ports */
> + else
> + qt2_port->open_ports = -1; /* unused */
> +
> + usb_set_serial_port_data(port, qt2_port);
> +
> + }
> + /* switch on power to the hardware */
> + if (qt2_boxpoweron(serial) < 0) {
> + dbg("qt2_boxpoweron() failed");
> + goto startup_error;
> + }
> + /* set all ports to RS232 mode */
> + for (i = 0; i < serial->num_ports; ++i) {
> + if (qt2_boxsetQMCR(serial, i, QU2BOX232) < 0) {
> + dbg("qt2_boxsetQMCR() on port %d failed",
> + i);
> + goto startup_error;
> + }
> + }
> +
> + return 0;
> +
> +startup_error:
> + for (i = 0; i < serial->num_ports; i++) {
> + port = serial->port[i];
> + qt2_port = qt2_get_port_private(port);
> + kfree(qt2_port);
> + usb_set_serial_port_data(port, NULL);
> + }
> +
> + dbg("Exit fail %s\n", __func__);
> + return -EIO;
> +}
> +
> +static void qt2_release(struct usb_serial *serial)
> +{
> + struct usb_serial_port *port;
> + struct quatech2_port *qt_port;
> + int i;
> +
> + dbg("enterting %s", __func__);
> +
> + for (i = 0; i < serial->num_ports; i++) {
> + port = serial->port[i];
> + if (!port)
> + continue;
> +
> + qt_port = usb_get_serial_port_data(port);
> + kfree(qt_port);
> + usb_set_serial_port_data(port, NULL);
> + }
> +}
> +/* This function is called once per serial port on the device.
> + * The tty_struct and the usb_serial_port belong to this port,
> + * i.e. there are multiple ones for a multi-port device.
> + * However the usb_serial_port structure has a back-pointer
> + * to the parent usb_serial structure which belongs to the device,
> + * so we can access either the device-wide information or
> + * any other port's information (because there are also forward
> + * pointers) via that pointer.
> + * This is most helpful if the device shares resources (e.g. end
> + * points) between different ports
> + */
> +int qt2_open(struct tty_struct *tty,
> + struct usb_serial_port *port, struct file *filp)
> +{
> + struct usb_serial *serial; /* device structure */
> + struct usb_serial_port *port0; /* first port structure on device */
> + struct quatech2_port *port_extra; /* extra data for this port */
> + struct quatech2_port *port0_extra; /* extra data for first port */
> + struct qt2_status_data ChannelData;
> + unsigned short default_divisor = QU2BOXSPD9600;
> + unsigned char default_LCR = SERIAL_8_DATA;
> + int status;
> +
> + if (port_paranoia_check(port, __func__))
> + return -ENODEV;
> +
> + dbg("%s - port %d\n", __func__, port->number);
> +
> + serial = port->serial; /* get the parent device structure */
> + if (serial_paranoia_check(serial, __func__))
> + return -ENODEV;
> + port0 = serial->port[0]; /* get the first port's device structure */
> +
> + port_extra = qt2_get_port_private(port);
> + port0_extra = qt2_get_port_private(port0);
> +
> + if (port_extra == NULL || port0_extra == NULL)
> + return -ENODEV;
> +
> + usb_clear_halt(serial->dev, port->write_urb->pipe);
> + usb_clear_halt(serial->dev, port->read_urb->pipe);
Is this really needed?
> + port0_extra->open_ports++;
> +
> + /* FIXME: are these needed? Does it even do anything useful? */
> + /* get the modem and line status values from the UART */
> + status = qt2_openboxchannel(serial, port->number,
> + &ChannelData);
> + if (status < 0) {
> + dbg("qt2_openboxchannel on channel %d failed",
> + port->number);
> + return status;
> + }
> + port_extra->shadowLSR = ChannelData.line_status &
> + (SERIAL_LSR_OE | SERIAL_LSR_PE | SERIAL_LSR_FE |
> + SERIAL_LSR_BI);
> +
> + port_extra->shadowMSR = ChannelData.modem_status &
> + (SERIAL_MSR_CTS | SERIAL_MSR_DSR | SERIAL_MSR_RI |
> + SERIAL_MSR_CD);
> +
> + port_extra->fifo_empty_flag = true;
> + dbg("qt2_openboxchannel on channel %d completed.",
> + port->number);
> +
> + /* Set Baud rate to default and turn off flow control here */
> + status = qt2_conf_uart(serial, port->number, default_divisor,
> + default_LCR);
> + if (status < 0) {
> + dbg("qt2_conf_uart() failed on channel %d",
> + port->number);
> + return status;
> + }
> + dbg("qt2_conf_uart() completed on channel %d",
> + port->number);
> +
> + dbg("port number is %d", port->number);
> + dbg("serial number is %d", port->serial->minor);
> +
> + /* We need to set up URBs and endpoints here. We only
> + * have one pair of endpoints per device, so in fact
> + * we only need to set up endpoints on the first time
> + * round, not subsequent ones.
> + * When we do a write to a port, we will use the same URBs
> + * regadless of the port, with a 5-byte header added on to
> + * tell the box which port it should eventually come out of,
> + * so the same URBs need to be visible to write calls
> + * regardless of which port is being written.
> + * To this end we actually keep the relevant endpoints and
> + * URBs in port 0's structure, because that's always there
> + * and avoids providing our own duplicate members in some
> + * user data structure for the same purpose.
> + */
> + if (port0_extra->open_ports == 1) {
> + /* this is first port to be opened */
> + }
What's the purpose of this?
> +
> + dbg("Bulkin endpoint is %d", port->bulk_in_endpointAddress);
> + dbg("BulkOut endpoint is %d", port->bulk_out_endpointAddress);
> + dbg("Interrupt endpoint is %d", port->interrupt_in_endpointAddress);
> +
> + /* initialize our wait queues */
> + init_waitqueue_head(&port_extra->wait);
Should be done in attach()
> +
> + /* remember to store port_extra and port0 back again at end !*/
> + qt2_set_port_private(port, port_extra);
> + qt2_set_port_private(serial->port[0], port0_extra);
Where do you change them?
> +
> + return 0;
> +}
> +
> +/* internal, private helper functions for the driver */
> +
> +/* Power up the FPGA in the box to get it working */
> +static int qt2_boxpoweron(struct usb_serial *serial)
> +{
> + int result;
> + __u8 Direcion;
> + unsigned int pipe;
> + Direcion = USBD_TRANSFER_DIRECTION_OUT;
> + pipe = usb_rcvctrlpipe(serial->dev, 0);
> + result = usb_control_msg(serial->dev, pipe, QT_SET_GET_DEVICE,
> + Direcion, QU2BOXPWRON, 0x00, NULL, 0x00,
> + 5000);
> + return result;
> +}
> +
> +/*
> + * qt2_boxsetQMCR Issue a QT_GET_SET_QMCR vendor-spcific request on the
> + * default control pipe. If successful return the number of bytes
> written,
> + * otherwise return a negative error number of the problem.
> + */
> +static int qt2_boxsetQMCR(struct usb_serial *serial, __u16 Uart_Number,
> + __u8 QMCR_Value)
> +{
> + int result;
> + __u16 PortSettings;
> +
> + PortSettings = (__u16)(QMCR_Value);
> +
> + dbg("%s(): Port = %d, PortSettings = 0x%x", __func__,
> + Uart_Number, PortSettings);
> +
> + result = usb_control_msg(serial->dev, usb_sndctrlpipe(serial->dev, 0),
> + QT_GET_SET_QMCR, 0x40, PortSettings,
> + (__u16)Uart_Number, NULL, 0, 5000);
> + return result;
> +}
> +static inline struct quatech2_port *qt2_get_port_private(struct
> usb_serial_port
> + *port)
> +{
> + return (struct quatech2_port *)usb_get_serial_port_data(port);
> +}
> +
> +static inline void qt2_set_port_private(struct usb_serial_port *port,
> + struct quatech2_port *data)
> +{
> + usb_set_serial_port_data(port, (void *)data);
> +}
These two functions serve no purpose.
> +static void __exit quausb2_usb_exit(void)
> +{
> + usb_deregister(&quausb2_usb_driver);
> + usb_serial_deregister(&quatech2_device);
> +}
I suggest you first deregister the serial device.
> +
> +module_init(quausb2_usb_init);
> +module_exit(quausb2_usb_exit);
> +
> +MODULE_AUTHOR(DRIVER_AUTHOR);
> +MODULE_DESCRIPTION(DRIVER_DESC);
> +MODULE_LICENSE("GPL");
> +
> +module_param(debug, bool, S_IRUGO | S_IWUSR);
> +MODULE_PARM_DESC(debug, "Debug enabled or not");
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
