On Sun, Jun 28, 2015 at 01:28:19PM -0500, Peter E. Berger wrote:
> From: "Peter E. Berger" <pber...@brimson.com>
>
> The io_ti driver fails to download firmware to Edgeport
> devices such as the EP/416, even when the on-disk firmware image
> (/lib/firmware/edgeport/down3.bin) is more current than the version
> on the EP/416. The current download code is broken in a few ways.
> Notably it mis-uses global variables OperationalMajorVersion and
> OperationalMinorVersion (reading their values before they've been
> properly initialized and subsequently initializing them multiple times
> without synchronization). This patch drops the global variables and
> replaces the redundant calls to request_firmware()/release_firmware()
> in download_fw() with a single call pair in edge_startup(); the firmware
> image pointer is then passed to download_fw() and build_i2c_fw_hdr().
This looks good now apart from one small issue I mention below.
Could you also please rename the patch summary (Subject) to something
more descriptive such as
USB: io_ti: fix firmware version handling
or similar?
> Signed-off-by: Peter E. Berger <pber...@brimson.com>
> ---
> drivers/usb/serial/io_ti.c | 91
> +++++++++++++++++++++-------------------------
> 1 file changed, 42 insertions(+), 49 deletions(-)
>
> diff --git a/drivers/usb/serial/io_ti.c b/drivers/usb/serial/io_ti.c
> index 69378a7..4492c17 100644
> --- a/drivers/usb/serial/io_ti.c
> +++ b/drivers/usb/serial/io_ti.c
> @@ -943,6 +925,13 @@ static int download_fw(struct edgeport_serial *serial)
> struct usb_interface_descriptor *interface;
> int download_cur_ver;
> int download_new_ver;
> + u8 fw_major_version;
> + u8 fw_minor_version;
> +
I think you should add minimal sanity checks on the firmware length here
as part of this patch. That is, make sure that fw->size >= 4, or if you
prefer, fw->size >= FW_HEADER_SIZE (i.e. 8).
> + fw_major_version = fw->data[0];
> + fw_minor_version = fw->data[1];
Thanks,
Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
