All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mathieu OTHACEHE <m.othacehe@gmail.com>
To: johan@kernel.org, gregkh@linuxfoundation.org
Cc: linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org,
	Mathieu OTHACEHE <m.othacehe@gmail.com>
Subject: [PATCH v2 17/22] usb: serial: ti_usb_3410_5052: Fix firmware downloading
Date: Tue, 26 Jul 2016 19:59:57 +0200	[thread overview]
Message-ID: <20160726180002.2398-18-m.othacehe@gmail.com> (raw)
In-Reply-To: <20160726180002.2398-1-m.othacehe@gmail.com>

The buffer used to store firmware is allocated to maximum firmware
size (TI_FIRMWARE_BUF_SIZE) + header size.
This buffer is filled with requested firmware (fw_p->size) and padded
with 0xff bytes. The header is written over the 3 first bytes of the
buffer (overwritting the 3 first bytes of the requested firmware)

Then, ti_do_download function is called to send only the first
fw_p->size bytes from the buffer to the device.
The rest of the buffer is never used.

So it is sufficient to allocate a buffer of fw_p->size bytes to store
only the requested firmware. There is no need to do any padding.

This patch also move firmware buffer manipulation to ti_do_download
function.

Signed-off-by: Mathieu OTHACEHE <m.othacehe@gmail.com>
---
 drivers/usb/serial/ti_usb_3410_5052.c | 61 +++++++++++++++++++----------------
 1 file changed, 33 insertions(+), 28 deletions(-)

diff --git a/drivers/usb/serial/ti_usb_3410_5052.c b/drivers/usb/serial/ti_usb_3410_5052.c
index 472971b..b46a511 100644
--- a/drivers/usb/serial/ti_usb_3410_5052.c
+++ b/drivers/usb/serial/ti_usb_3410_5052.c
@@ -1639,46 +1639,65 @@ static int ti_write_byte(struct usb_serial_port *port, u32 addr,
 	return status;
 }
 
-static int ti_do_download(struct usb_device *dev, int pipe,
-			  u8 *buffer, int size)
+static int ti_do_download(struct usb_serial *serial,
+			  const struct firmware *fw_p)
 {
 	int pos;
 	u8 cs = 0;
 	int done;
+	struct usb_device *dev = serial->dev;
 	struct ti_firmware_header *header;
 	int status = 0;
+	u8 *buffer;
+	int buffer_size;
 	int len;
+	unsigned int pipe;
+
+	pipe = usb_sndbulkpipe(dev, serial->port[0]->bulk_out_endpointAddress);
+
+	buffer_size = fw_p->size;
+	buffer = kmalloc(buffer_size, GFP_KERNEL);
+	if (!buffer)
+		return -ENOMEM;
 
-	for (pos = sizeof(struct ti_firmware_header); pos < size; pos++)
+	memcpy(buffer, fw_p->data, fw_p->size);
+
+	for (pos = sizeof(*header); pos < buffer_size; pos++)
 		cs = (u8)(cs + buffer[pos]);
 
 	header = (struct ti_firmware_header *)buffer;
-	header->wLength = cpu_to_le16(size - sizeof(*header));
+	header->wLength = cpu_to_le16(buffer_size - sizeof(*header));
 	header->bCheckSum = cs;
 
 	dev_dbg(&dev->dev, "%s - downloading firmware\n", __func__);
-	for (pos = 0; pos < size; pos += done) {
-		len = min(size - pos, TI_DOWNLOAD_MAX_PACKET_SIZE);
+	for (pos = 0; pos < buffer_size; pos += done) {
+		len = min(buffer_size - pos, TI_DOWNLOAD_MAX_PACKET_SIZE);
 		status = usb_bulk_msg(dev, pipe, buffer + pos, len, &done,
 				      TI_DOWNLOAD_TIMEOUT);
 		if (status)
 			break;
 	}
-	return status;
+
+	kfree(buffer);
+
+	if (status) {
+		dev_err(&dev->dev, "failed to download firmware: %d\n", status);
+		return status;
+	}
+
+	dev_dbg(&dev->dev, "%s - download successful\n", __func__);
+
+	return 0;
 }
 
 static int ti_download_firmware(struct usb_serial *serial)
 {
 	int status;
-	int buffer_size;
-	u8 *buffer;
 	struct usb_device *dev = serial->dev;
 	struct ti_device *tdev = usb_get_serial_data(serial);
-	unsigned int pipe;
 	const struct firmware *fw_p;
 	char buf[32];
 
-	pipe = usb_sndbulkpipe(dev, serial->port[0]->bulk_out_endpointAddress);
 
 	if (le16_to_cpu(dev->descriptor.idVendor) == MXU1_VENDOR_ID) {
 		snprintf(buf,
@@ -1733,30 +1752,16 @@ check_firmware:
 		dev_err(&dev->dev, "%s - firmware not found\n", __func__);
 		return -ENOENT;
 	}
+
 	if (fw_p->size > TI_FIRMWARE_BUF_SIZE) {
 		dev_err(&dev->dev, "%s - firmware too large %zu\n", __func__, fw_p->size);
 		release_firmware(fw_p);
 		return -ENOENT;
 	}
 
-	buffer_size = TI_FIRMWARE_BUF_SIZE + sizeof(struct ti_firmware_header);
-	buffer = kmalloc(buffer_size, GFP_KERNEL);
-	if (buffer) {
-		memcpy(buffer, fw_p->data, fw_p->size);
-		memset(buffer + fw_p->size, 0xff, buffer_size - fw_p->size);
-		status = ti_do_download(dev, pipe, buffer, fw_p->size);
-		kfree(buffer);
-	} else {
-		status = -ENOMEM;
-	}
-	release_firmware(fw_p);
-	if (status) {
-		dev_err(&dev->dev, "%s - error downloading firmware, %d\n",
-							__func__, status);
-		return status;
-	}
+	ti_do_download(serial, fw_p);
 
-	dev_dbg(&dev->dev, "%s - download successful\n", __func__);
+	release_firmware(fw_p);
 
 	return 0;
 }
-- 
2.9.0

  parent reply	other threads:[~2016-07-26 18:06 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-26 17:59 [PATCH v2 00/22] usb: serial: ti_usb_3410_5052: clean driver Mathieu OTHACEHE
2016-07-26 17:59 ` [PATCH v2 01/22] usb: serial: ti_usb_3410_5052: Do not use __uX types Mathieu OTHACEHE
2016-08-23  7:44   ` Johan Hovold
2016-07-26 17:59 ` [PATCH v2 02/22] usb: serial: ti_usb_3410_5052: Remove useless dev_dbg messages Mathieu OTHACEHE
2016-08-23  7:52   ` Johan Hovold
2016-07-26 17:59 ` [PATCH v2 03/22] usb: serial: ti_usb_3410_5052: Use kzalloc instead of kmalloc Mathieu OTHACEHE
2016-07-27  8:09   ` Oliver Neukum
2016-07-27 12:46     ` Mathieu OTHACEHE
2016-08-23  7:54       ` Johan Hovold
2016-07-27 11:21   ` Sergei Shtylyov
2016-07-26 17:59 ` [PATCH v2 04/22] usb: serial: ti_usb_3410_5052: Remove useless NULL-testing Mathieu OTHACEHE
2016-08-23  8:04   ` Johan Hovold
2016-07-26 17:59 ` [PATCH v2 05/22] usb: serial: ti_usb_3410_5052: Use C_X macros instead of c_cflag manipulation Mathieu OTHACEHE
2016-07-26 17:59 ` [PATCH v2 06/22] usb: serial: ti_usb_3410_5052: Remove unused variables Mathieu OTHACEHE
2016-08-23  8:15   ` Johan Hovold
2016-07-26 17:59 ` [PATCH v2 07/22] usb: serial: ti_usb_3410_5052: Use macros instead of magic values Mathieu OTHACEHE
2016-08-23  8:19   ` Johan Hovold
2016-07-26 17:59 ` [PATCH v2 08/22] usb: serial: ti_usb_3410_5052: Remove in_sync and out_sync functions Mathieu OTHACEHE
2016-07-26 17:59 ` [PATCH v2 09/22] usb: serial: ti_usb_3410_5052: Remove useless tty_wakeup Mathieu OTHACEHE
2016-07-26 17:59 ` [PATCH v2 10/22] usb: serial: ti_usb_3410_5052: Change ti_write_byte function arguments Mathieu OTHACEHE
2016-07-27  8:13   ` Oliver Neukum
2016-07-27 16:08     ` Mathieu OTHACEHE
2016-07-27 19:10       ` Oliver Neukum
2016-07-26 17:59 ` [PATCH v2 11/22] usb: serial: ti_usb_3410_5052: Do not modify interrupt context Mathieu OTHACEHE
2016-07-26 17:59 ` [PATCH v2 12/22] usb: serial: ti_usb_3410_5052: Remove usb_serial pointer in ti_port Mathieu OTHACEHE
2016-07-26 17:59 ` [PATCH v2 13/22] usb: serial: ti_usb_3410_5052: Change ti_get/set_serial_info function arguments Mathieu OTHACEHE
2016-07-26 17:59 ` [PATCH v2 14/22] usb: serial: ti_usb_3410_5052: Do not set shadow mcr in open callback Mathieu OTHACEHE
2016-07-26 17:59 ` [PATCH v2 15/22] usb: serial: ti_usb_3410_5052: Check old_termios parameter in set_termios Mathieu OTHACEHE
2016-07-26 17:59 ` [PATCH v2 16/22] usb: serial: ti_usb_3410_5052: Raise DTR and RTS flags if speed is not null anymore Mathieu OTHACEHE
2016-07-26 17:59 ` Mathieu OTHACEHE [this message]
2016-07-26 17:59 ` [PATCH v2 18/22] usb: serial: ti_usb_3410_5052: Standardize debug and error messages Mathieu OTHACEHE
2016-07-26 17:59 ` [PATCH v2 19/22] usb: serial: ti_usb_3410_5052: Use variables for vendor and product Mathieu OTHACEHE
2016-07-26 18:00 ` [PATCH v2 20/22] usb: serial: ti_usb_3410_5052: Set shadow msr before waking up waiters Mathieu OTHACEHE
2016-07-26 18:00 ` [PATCH v2 21/22] usb: serial: ti_usb_3410_5052: Add CMSPAR support Mathieu OTHACEHE
2016-07-26 18:00 ` [PATCH v2 22/22] usb: serial: ti_usb_3410_5052: Fix indentation problems Mathieu OTHACEHE

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160726180002.2398-18-m.othacehe@gmail.com \
    --to=m.othacehe@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=johan@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.