On Sat, Dec 21, 2013 at 05:57:41PM +0100, Andrew Lunn wrote:
> +/* Get the version of the firmware currently running. */
> +static int mxuport_get_fw_version(struct usb_serial *serial, u32 *version)
> +{
> + u8 *ver_buf;
> + int err;
> +
> + ver_buf = kzalloc(4, GFP_KERNEL);
> + if (!ver_buf)
> + return -ENOMEM;
> +
> + /* Get firmware version from SDRAM */
> + err = mxuport_recv_ctrl_urb(serial, RQ_VENDOR_GET_VERSION, 0, 0,
> + ver_buf, 4);
> + if (err != 4)
> + goto out;
> +
> + *version = (ver_buf[0] << 16) | (ver_buf[1] << 8) | ver_buf[2];
> + err = 0;
> +out:
> + kfree(ver_buf);
> + return err;
> +}
> +static int mxuport_probe(struct usb_serial *serial,
> + const struct usb_device_id *id)
> +{
> + u16 productid = le16_to_cpu(serial->dev->descriptor.idProduct);
> + const struct firmware *fw_p = NULL;
> + u32 version;
> + int local_ver;
> + char buf[32];
> + int err;
> +
> + /* Load our firmware */
> + err = mxuport_send_ctrl_urb(serial, RQ_VENDOR_QUERY_FW_CONFIG, 0, 0);
> + if (err) {
> + mxuport_send_ctrl_urb(serial, RQ_VENDOR_RESET_DEVICE, 0, 0);
> + return err;
> + }
> +
> + err = mxuport_get_fw_version(serial, &version);
> + if (err < 0)
> + return err;
I got the following compilation error with v8:
In file included from /home/johan/work/omicron/src/linux/include/linux/usb.h:18:0,
from /home/johan/work/omicron/src/linux/drivers/usb/serial/mxuport.c:31:
/home/johan/work/omicron/src/linux/drivers/usb/serial/mxuport.c: In function 'mxuport_probe':
/home/johan/work/omicron/src/linux/include/linux/device.h:1086:45: warning: 'version' may be used uninitialized in this function [-Wmaybe-uninitialized]
#define dev_info(dev, fmt, arg...) _dev_info(dev, fmt, ##arg)
^
/home/johan/work/omicron/src/linux/drivers/usb/serial/mxuport.c:1041:6: note: 'version' was declared here
u32 version;
^
and it turns out there was indeed a bug in the error handling above as
mxuport_get_fw_version could return err >= 0 also on errors (and in
which case version would be uninitialised).
> +
> + dev_dbg(&serial->interface->dev, "Device firmware version v%x.%x.%x\n",
> + (version & 0xff0000) >> 16,
> + (version & 0xff00) >> 8,
> + (version & 0xff));
> +
> + snprintf(buf, sizeof(buf) - 1, "moxa/moxa-%04x.fw", productid);
> +
> + err = request_firmware(&fw_p, buf, &serial->interface->dev);
> + if (err) {
> + dev_warn(&serial->interface->dev, "Firmware %s not found\n",
> + buf);
> +
> + /* Use the firmware already in the device */
> + err = 0;
> + } else {
> + local_ver = ((fw_p->data[VER_ADDR_1] << 16) |
> + (fw_p->data[VER_ADDR_2] << 8) |
> + fw_p->data[VER_ADDR_3]);
> + dev_dbg(&serial->interface->dev,
> + "Available firmware version v%x.%x.%x\n",
> + fw_p->data[VER_ADDR_1], fw_p->data[VER_ADDR_2],
> + fw_p->data[VER_ADDR_3]);
> + if (local_ver > version) {
> + err = mxuport_download_fw(serial, fw_p);
> + if (err)
> + goto out;
> + err = mxuport_get_fw_version(serial, &version);
> + if (err < 0)
> + goto out;
> + }
> + }
> +
> + dev_info(&serial->interface->dev,
> + "Using device firmware version v%x.%x.%x\n",
> + (version & 0xff0000) >> 16,
> + (version & 0xff00) >> 8,
> + (version & 0xff));
> +
> + /*
> + * Contains the features of this hardware. Store away for
> + * later use, eg, number of ports.
> + */
> + usb_set_serial_data(serial, (void *)id->driver_info);
> +out:
> + if (fw_p)
> + release_firmware(fw_p);
> + return err;
> +}
There was also a small typo in two error messages, an unused variable,
and some trailing whitespace at the end of the file.
All very minor so I figured I'll just fix it up here, along with a few
style issues (see full diff below).
I hope it's OK that I submit the patch with the below changes to Greg
for inclusion in v3.14. Let me know if you prefer to respin yourself.
Thanks for all your work with fixing up and mainlining this driver!
Happy new year,
Johan
diff --git a/drivers/usb/serial/mxuport.c b/drivers/usb/serial/mxuport.c
index 4b7a5bf57526..17e4f40b6306 100644
--- a/drivers/usb/serial/mxuport.c
+++ b/drivers/usb/serial/mxuport.c
@@ -229,13 +229,14 @@ static int mxuport_recv_ctrl_urb(struct usb_serial *serial,
USB_CTRL_GET_TIMEOUT);
if (status < 0) {
dev_err(&serial->interface->dev,
- "%s - recv usb_control_msg failed. (%d)\n",
+ "%s - usb_control_msg failed (%d)\n",
__func__, status);
return status;
}
if (status != size) {
- dev_err(&serial->interface->dev, "%s - sort read (%d / %zd)\n",
+ dev_err(&serial->interface->dev,
+ "%s - short read (%d / %zd)\n",
__func__, status, size);
return -EIO;
}
@@ -260,13 +261,14 @@ static int mxuport_send_ctrl_data_urb(struct usb_serial *serial,
USB_CTRL_SET_TIMEOUT);
if (status < 0) {
dev_err(&serial->interface->dev,
- "%s - send usb_control_msg failed. (%d)\n",
+ "%s - usb_control_msg failed (%d)\n",
__func__, status);
return status;
}
if (status != size) {
- dev_err(&serial->interface->dev, "%s - sort write (%d / %zd)\n",
+ dev_err(&serial->interface->dev,
+ "%s - short write (%d / %zd)\n",
__func__, status, size);
return -EIO;
}
@@ -974,8 +976,10 @@ static int mxuport_get_fw_version(struct usb_serial *serial, u32 *version)
/* Get firmware version from SDRAM */
err = mxuport_recv_ctrl_urb(serial, RQ_VENDOR_GET_VERSION, 0, 0,
ver_buf, 4);
- if (err != 4)
+ if (err != 4) {
+ err = -EIO;
goto out;
+ }
*version = (ver_buf[0] << 16) | (ver_buf[1] << 8) | ver_buf[2];
err = 0;
@@ -1137,7 +1141,10 @@ static int mxuport_port_probe(struct usb_serial_port *port)
err = mxuport_send_ctrl_urb(serial, RQ_VENDOR_SET_INTERFACE,
MX_INT_RS232,
port->port_number);
- return err;
+ if (err)
+ return err;
+
+ return 0;
}
static int mxuport_alloc_write_urb(struct usb_serial *serial,
@@ -1240,13 +1247,14 @@ static int mxuport_attach(struct usb_serial *serial)
/*
* All data from the ports is received on the first bulk in
* endpoint, with a multiplex header. The second bulk in is
- * used for events. Start to read serial data from the device
+ * used for events.
+ *
+ * Start to read from the device.
*/
err = usb_serial_generic_submit_read_urbs(port0, GFP_KERNEL);
if (err)
return err;
- /* Endpoints on Port1 is used for events */
err = usb_serial_generic_submit_read_urbs(port1, GFP_KERNEL);
if (err) {
usb_serial_generic_close(port0);
@@ -1304,11 +1312,8 @@ static void mxuport_break_ctl(struct tty_struct *tty, int break_state)
{
struct usb_serial_port *port = tty->driver_data;
struct usb_serial *serial = port->serial;
- struct mxuport_port *mxport;
int enable;
- mxport = usb_get_serial_port_data(port);
-
if (break_state == -1) {
enable = 1;
dev_dbg(&port->dev, "%s - sending break\n", __func__);
@@ -1384,11 +1389,6 @@ static struct usb_serial_driver *const serial_drivers[] = {
module_usb_serial_driver(serial_drivers, mxuport_idtable);
-/*
- * Module Information
- */
MODULE_AUTHOR("Andrew Lunn <andrew@xxxxxxx>");
MODULE_AUTHOR("<support@xxxxxxxx>");
MODULE_LICENSE("GPL");
-
-
--
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
