On Thu, Dec 13, 2007 at 06:20:56PM -0800, Sarah Sharp wrote:
Hi Sarah,
> Signed-off-by: Sarah Sharp <[EMAIL PROTECTED]>
> ---
> drivers/usb/serial/pl2303.c | 42
> +++++++++++++++++++++---------------------
> 1 files changed, 21 insertions(+), 21 deletions(-)
There's a lot of code motion in the first four patches
(with no explanation) that seems to be greatly larger than
the net effect of applying all four patches.
I did so, just to see the end result, which was a lot more 'reviewable',
ending up with this..
diff --git a/drivers/usb/serial/pl2303.c b/drivers/usb/serial/pl2303.c
index cf8add9..15097a4 100644
--- a/drivers/usb/serial/pl2303.c
+++ b/drivers/usb/serial/pl2303.c
@@ -310,6 +310,28 @@ static unsigned int pl2303_buf_get(struct pl2303_buf *pb,
char *buf,
return count;
}
+static int pl2303_vendor_read(__u16 value, __u16 index,
+ struct usb_serial *serial, unsigned char *buf)
+{
+ int res = usb_control_msg(serial->dev, usb_rcvctrlpipe(serial->dev, 0),
+ VENDOR_READ_REQUEST, VENDOR_READ_REQUEST_TYPE,
+ value, index, buf, 1, 100);
+ dbg("0x%x:0x%x:0x%x:0x%x %d - %x", VENDOR_READ_REQUEST_TYPE,
+ VENDOR_READ_REQUEST, value, index, res, buf[0]);
+ return res;
+}
+
+static int pl2303_vendor_write(__u16 value, __u16 index,
+ struct usb_serial *serial)
+{
+ int res = usb_control_msg(serial->dev, usb_sndctrlpipe(serial->dev, 0),
+ VENDOR_WRITE_REQUEST, VENDOR_WRITE_REQUEST_TYPE,
+ value, index, NULL, 0, 100);
+ dbg("0x%x:0x%x:0x%x:0x%x %d", VENDOR_WRITE_REQUEST_TYPE,
+ VENDOR_WRITE_REQUEST, value, index, res);
+ return res;
+}
+
static int pl2303_startup(struct usb_serial *serial)
{
struct pl2303_private *priv;
@@ -671,7 +693,7 @@ static int pl2303_open(struct usb_serial_port *port, struct
file *filp)
struct ktermios tmp_termios;
struct usb_serial *serial = port->serial;
struct pl2303_private *priv = usb_get_serial_port_data(port);
- unsigned char *buf;
+ unsigned char buf;
int result;
dbg("%s - port %d", __FUNCTION__, port->number);
@@ -681,43 +703,27 @@ static int pl2303_open(struct usb_serial_port *port,
struct file *filp)
usb_clear_halt(serial->dev, port->read_urb->pipe);
}
- buf = kmalloc(10, GFP_KERNEL);
- if (buf==NULL)
- return -ENOMEM;
-
-#define FISH(a,b,c,d)
\
- result=usb_control_msg(serial->dev, usb_rcvctrlpipe(serial->dev,0),
\
- b, a, c, d, buf, 1, 100);
\
- dbg("0x%x:0x%x:0x%x:0x%x %d - %x",a,b,c,d,result,buf[0]);
-
-#define SOUP(a,b,c,d)
\
- result=usb_control_msg(serial->dev, usb_sndctrlpipe(serial->dev,0),
\
- b, a, c, d, NULL, 0, 100);
\
- dbg("0x%x:0x%x:0x%x:0x%x %d",a,b,c,d,result);
-
- FISH (VENDOR_READ_REQUEST_TYPE, VENDOR_READ_REQUEST, 0x8484, 0);
- SOUP (VENDOR_WRITE_REQUEST_TYPE, VENDOR_WRITE_REQUEST, 0x0404, 0);
- FISH (VENDOR_READ_REQUEST_TYPE, VENDOR_READ_REQUEST, 0x8484, 0);
- FISH (VENDOR_READ_REQUEST_TYPE, VENDOR_READ_REQUEST, 0x8383, 0);
- FISH (VENDOR_READ_REQUEST_TYPE, VENDOR_READ_REQUEST, 0x8484, 0);
- SOUP (VENDOR_WRITE_REQUEST_TYPE, VENDOR_WRITE_REQUEST, 0x0404, 1);
- FISH (VENDOR_READ_REQUEST_TYPE, VENDOR_READ_REQUEST, 0x8484, 0);
- FISH (VENDOR_READ_REQUEST_TYPE, VENDOR_READ_REQUEST, 0x8383, 0);
- SOUP (VENDOR_WRITE_REQUEST_TYPE, VENDOR_WRITE_REQUEST, 0, 1);
- SOUP (VENDOR_WRITE_REQUEST_TYPE, VENDOR_WRITE_REQUEST, 1, 0);
+ pl2303_vendor_read(0x8484, 0, serial, &buf);
+ pl2303_vendor_write(0x0404, 0, serial);
+ pl2303_vendor_read(0x8484, 0, serial, &buf);
+ pl2303_vendor_read(0x8383, 0, serial, &buf);
+ pl2303_vendor_read(0x8484, 0, serial, &buf);
+ pl2303_vendor_write(0x0404, 1, serial);
+ pl2303_vendor_read(0x8484, 0, serial, &buf);
+ pl2303_vendor_read(0x8383, 0, serial, &buf);
+ pl2303_vendor_write(0, 1, serial);
+ pl2303_vendor_write(1, 0, serial);
if (priv->type == HX) {
/* HX chip */
- SOUP (VENDOR_WRITE_REQUEST_TYPE, VENDOR_WRITE_REQUEST, 2, 0x44);
+ pl2303_vendor_write(2, 0x44, serial);
/* reset upstream data pipes */
- SOUP (VENDOR_WRITE_REQUEST_TYPE, VENDOR_WRITE_REQUEST, 8, 0);
- SOUP (VENDOR_WRITE_REQUEST_TYPE, VENDOR_WRITE_REQUEST, 9, 0);
+ pl2303_vendor_write(8, 0, serial);
+ pl2303_vendor_write(9, 0, serial);
} else {
- SOUP (VENDOR_WRITE_REQUEST_TYPE, VENDOR_WRITE_REQUEST, 2, 0x24);
+ pl2303_vendor_write(2, 0x24, serial);
}
- kfree(buf);
-
/* Setup termios */
if (port->tty) {
pl2303_set_termios(port, &tmp_termios);
After these changes, pl2303_vendor_read returns an int, and
we assign it to a char. Is that intentional ? Does it matter?
Looks like a step forward away from that macro sillyness though.
Dave
--
http://www.codemonkey.org.uk
-
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
