Hello.
On 24-02-2011 16:50, Martin wrote:
From: Martin Jansen<martin.jansen@xxxxxxxxxxx>
Add support for RTS and CTS line status
This should be in the patch's subject.
Signed-off-by: Martin Jansen<martin.jansen@xxxxxxxxxxx>
---
--- linux-2.6.37.1/drivers/usb/serial/opticon.c.orig 2011-02-24
04:24:09.678554999 -0800
+++ linux-2.6.37.1/drivers/usb/serial/opticon.c 2011-02-24
05:35:13.859088028 -0800
[...]
@@ -21,6 +22,16 @@
#include<linux/usb/serial.h>
#include<linux/uaccess.h>
+#define CONTROL_RTS 0x02
+#define RESEND_CTS_STATE 0x03
+
+/* max number of write urbs in flight */
+#define URB_UPPER_LIMIT 8
+
+/* This driver works for the Opticon 1D barcode reader
+ * an examples of 1D barcode types are EAN, UPC, Code39, IATA etc.. */
Well, the preferred style of the multi-line comments is this:
/*
* bla
* bla
*/
@@ -57,6 +68,7 @@ static void opticon_bulk_callback(struct
struct tty_struct *tty;
int result;
int data_length;
+ unsigned long flags;
dbg("%s - port %d", __func__, port->number);
@@ -87,10 +99,10 @@ static void opticon_bulk_callback(struct
* Data from the device comes with a 2 byte header:
*
*<0x00><0x00>data...
- * This is real data to be sent to the tty layer
+ * This is real data to be sent to the tty layer
*<0x00><0x01)level
- * This is a RTS level change, the third byte is the RTS
- * value (0 for low, 1 for high).
+ * This is a CTS level change, the third byte is the CTS
+ * value (0 for low, 1 for high).
Please avoid random whitespace changes. Doing that in a sperate patch is OK.
@@ -140,6 +155,24 @@ exit:
spin_unlock(&priv->lock);
}
+static int send_control_msg(struct usb_serial_port *port, u8 requesttype,
+ u8 val)
+{
+ struct usb_serial *serial = port->serial;
+ int retval;
+ u8 buffer[2];
+
+ buffer[0] = val;
Hm, why 2-byte buffer if you're only sending 1 byte? Also, the buffer
should probably be allocated with kmalloc() as DMA off stack is dangerous on
non-x86 arches.
+ /* Send the message to the vendor control endpoint
+ * of the connected device */
Comment style. You're close to it though. :-)
+ retval = usb_control_msg(serial->dev, usb_sndctrlpipe(serial->dev, 0),
+ requesttype,
+ USB_DIR_OUT|USB_TYPE_VENDOR|USB_RECIP_INTERFACE,
Well, need spaces around | for the coding style to be consistent...
+ 0, 0, buffer, 1, 0);
+
+ return retval;
Why not just directly return the result of usb_control_msg()?
@@ -152,19 +185,30 @@ static int opticon_open(struct tty_struc
[...]
if (result)
dev_err(&port->dev,
"%s - failed resubmitting read urb, error %d\n",
__func__, result);
+ /* Request CTS line state, sometimes during opening the current
+ * CTS state can be missed. */
Comment style...
+ send_control_msg(port, RESEND_CTS_STATE, 1);
return result;
}
@@ -178,7 +222,7 @@ static void opticon_close(struct usb_ser
usb_kill_urb(priv->bulk_read_urb);
}
-static void opticon_write_bulk_callback(struct urb *urb)
+static void opticon_write_control_callback(struct urb *urb)
{
struct opticon_private *priv = urb->context;
int status = urb->status;
@@ -210,6 +254,7 @@ static int opticon_write(struct tty_stru
unsigned char *buffer;
unsigned long flags;
int status;
+ struct usb_ctrlrequest *dr;
dbg("%s - port %d", __func__, port->number);
@@ -226,6 +271,7 @@ static int opticon_write(struct tty_stru
if (!buffer) {
dev_err(&port->dev, "out of memory\n");
count = -ENOMEM;
+
Avoid random whitespace changes please.
goto error_no_buffer;
}
@@ -240,35 +286,28 @@ static int opticon_write(struct tty_stru
usb_serial_debug_data(debug,&port->dev, __func__, count, buffer);
- if (port->bulk_out_endpointAddress) {
- usb_fill_bulk_urb(urb, serial->dev,
- usb_sndbulkpipe(serial->dev,
- port->bulk_out_endpointAddress),
- buffer, count, opticon_write_bulk_callback, priv);
- } else {
- struct usb_ctrlrequest *dr;
+ /* The conncected devices do not have a bulk write endpoint,
+ * to transmit data to de barcode device the control endpoint is used */
Comment style...
+ dr = kmalloc(sizeof(struct usb_ctrlrequest), GFP_NOIO);
+ if (!dr)
+ return -ENOMEM;
- dr = kmalloc(sizeof(struct usb_ctrlrequest), GFP_NOIO);
- if (!dr)
- return -ENOMEM;
-
- dr->bRequestType = USB_TYPE_VENDOR | USB_RECIP_INTERFACE | USB_DIR_OUT;
- dr->bRequest = 0x01;
- dr->wValue = 0;
- dr->wIndex = 0;
- dr->wLength = cpu_to_le16(count);
-
- usb_fill_control_urb(urb, serial->dev,
- usb_sndctrlpipe(serial->dev, 0),
- (unsigned char *)dr, buffer, count,
- opticon_write_bulk_callback, priv);
- }
+ dr->bRequestType = USB_TYPE_VENDOR | USB_RECIP_INTERFACE | USB_DIR_OUT;
+ dr->bRequest = 0x01;
+ dr->wValue = 0;
+ dr->wIndex = 0;
+ dr->wLength = cpu_to_le16(count);
+
+ usb_fill_control_urb(urb, serial->dev,
+ usb_sndctrlpipe(serial->dev, 0),
+ (unsigned char *)dr, buffer, count,
+ opticon_write_control_callback, priv);
/* send it down the pipe */
status = usb_submit_urb(urb, GFP_ATOMIC);
if (status) {
dev_err(&port->dev,
- "%s - usb_submit_urb(write bulk) failed with status = %d\n",
+ "%s - usb_submit_urb(write endpoint) failed status = %d\n",
No, don't move this line to the left. Contunation lines should be aligned
more to the right.
__func__, status);
count = status;
goto error;
@@ -360,16 +399,49 @@ static int opticon_tiocmget(struct tty_s
int result = 0;
dbg("%s - port %d", __func__, port->number);
+ if (!usb_get_intfdata(port->serial->interface))
+ return -ENODEV;
Does this change have to do with the patch's purpose?
spin_lock_irqsave(&priv->lock, flags);
if (priv->rts)
- result = TIOCM_RTS;
+ result |= TIOCM_RTS;
+ if (priv->cts)
+ result |= TIOCM_CTS;
spin_unlock_irqrestore(&priv->lock, flags);
dbg("%s - %x", __func__, result);
return result;
}
+static int opticon_tiocmset(struct tty_struct *tty, struct file *file,
+ unsigned int set, unsigned int clear)
+{
+ struct usb_serial_port *port = tty->driver_data;
+ struct opticon_private *priv = usb_get_serial_data(port->serial);
+ unsigned long flags;
+ bool rts;
+ bool changed = false;
+
+ if (!usb_get_intfdata(port->serial->interface))
+ return -ENODEV;
+ /* We only support RTS so we only handle that */
This comment has nothing to do with the code immediately below it -- maybe
it should be moved?
@@ -431,6 +503,7 @@ static int opticon_startup(struct usb_se
priv->serial = serial;
priv->port = serial->port[0];
priv->udev = serial->dev;
+ priv->outstanding_urbs = 0; /* Init the outstanding urbs */
Does this change has anything to do with the patch's purpose?
@@ -581,6 +648,7 @@ static void __exit opticon_exit(void)
module_init(opticon_init);
module_exit(opticon_exit);
+MODULE_DESCRIPTION(DRIVER_DESC);
And this one?
WBR, Sergei
--
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
