public inbox for linux-kernel@vger.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 4/4] USB: mxu11x0: move firmware download and endpoint testing to probe callback
Date: Mon,  4 Jan 2016 19:49:39 +0100	[thread overview]
Message-ID: <1451933379-21994-5-git-send-email-m.othacehe@gmail.com> (raw)
In-Reply-To: <1451933379-21994-1-git-send-email-m.othacehe@gmail.com>

Move interrupt in endpoint test and firmware download to a new probe
callback. This avoids unnecessary memory allocations done by core
before port_probe callback is called.

If the device has to be reseted (firmware downloaded) or if the
interface is incorrect (no interrupt in endpoint), the probe fails
by returning ENODEV.

Signed-off-by: Mathieu OTHACEHE <m.othacehe@gmail.com>
---

Changes in v2:
* Remove useless paranthesis
* Fix multi-line comment

 drivers/usb/serial/mxu11x0.c | 131 ++++++++++++++++++++++++++-----------------
 1 file changed, 79 insertions(+), 52 deletions(-)

diff --git a/drivers/usb/serial/mxu11x0.c b/drivers/usb/serial/mxu11x0.c
index 6326cda..b942aeb 100644
--- a/drivers/usb/serial/mxu11x0.c
+++ b/drivers/usb/serial/mxu11x0.c
@@ -272,7 +272,8 @@ static int mxu1_send_ctrl_urb(struct usb_serial *serial,
 }
 
 static int mxu1_download_firmware(struct usb_serial *serial,
-				  const struct firmware *fw_p)
+				  const struct firmware *fw_p,
+				  struct usb_endpoint_descriptor *endpoint)
 {
 	int status = 0;
 	int buffer_size;
@@ -285,7 +286,7 @@ static int mxu1_download_firmware(struct usb_serial *serial,
 	struct mxu1_firmware_header *header;
 	unsigned int pipe;
 
-	pipe = usb_sndbulkpipe(dev, serial->port[0]->bulk_out_endpointAddress);
+	pipe = usb_sndbulkpipe(dev, endpoint->bEndpointAddress);
 
 	buffer_size = fw_p->size + sizeof(*header);
 	buffer = kmalloc(buffer_size, GFP_KERNEL);
@@ -328,16 +329,86 @@ static int mxu1_download_firmware(struct usb_serial *serial,
 	return 0;
 }
 
-static int mxu1_port_probe(struct usb_serial_port *port)
+static int mxu1_probe(struct usb_serial *serial, const struct usb_device_id *id)
 {
-	struct mxu1_port *mxport;
-	struct mxu1_device *mxdev;
+	struct usb_host_interface *cur_altsetting;
+	char fw_name[32];
+	const struct firmware *fw_p = NULL;
+	struct usb_device *dev = serial->dev;
+	u16 model;
+	int err;
+	struct usb_endpoint_descriptor *endpoint, *interrupt_in, *bulk_out;
+	int i;
+
+	dev_dbg(&serial->interface->dev, "%s - product 0x%04X, num configurations %d, configuration value %d\n",
+		__func__, le16_to_cpu(dev->descriptor.idProduct),
+		dev->descriptor.bNumConfigurations,
+		dev->actconfig->desc.bConfigurationValue);
+
+	cur_altsetting = serial->interface->cur_altsetting;
+	interrupt_in = NULL;
+	bulk_out = NULL;
+
+	for (i = 0; i < cur_altsetting->desc.bNumEndpoints; i++) {
+		endpoint = &cur_altsetting->endpoint[i].desc;
+		if (usb_endpoint_is_bulk_out(endpoint)) {
+			dev_dbg(&serial->interface->dev,
+				"found bulk out on endpoint %d\n", i);
+			bulk_out = endpoint;
+		}
+
+		if (usb_endpoint_is_int_in(endpoint)) {
+			dev_dbg(&serial->interface->dev,
+				"found interrupt in on endpoint %d\n", i);
+			interrupt_in = endpoint;
+		}
+	}
+
+	/* if we have only 1 bulk out endpoint, download firmware */
+	if (bulk_out && cur_altsetting->desc.bNumEndpoints == 1) {
+
+		model = le16_to_cpu(dev->descriptor.idProduct);
+		snprintf(fw_name,
+			 sizeof(fw_name),
+			 "moxa/moxa-%04x.fw",
+			 model);
+
+		err = request_firmware(&fw_p, fw_name, &serial->interface->dev);
+		if (err) {
+			dev_err(&serial->interface->dev, "failed to request firmware: %d\n",
+				err);
+			return -ENODEV;
+		}
+
+		err = mxu1_download_firmware(serial, fw_p, bulk_out);
+		if (err)
+			goto err_release_firmware;
 
-	if (!port->interrupt_in_urb) {
-		dev_err(&port->dev, "no interrupt urb\n");
+		/* device is being reset */
+		err = -ENODEV;
+		goto err_release_firmware;
+
+	} else if (!interrupt_in) {
+		/*
+		 * firmware is already loaded but there is
+		 * no interrupt in endpoint available
+		 */
+		dev_err(&serial->interface->dev, "no interrupt endpoint\n");
 		return -ENODEV;
 	}
 
+	return 0;
+
+err_release_firmware:
+	release_firmware(fw_p);
+	return err;
+}
+
+static int mxu1_port_probe(struct usb_serial_port *port)
+{
+	struct mxu1_port *mxport;
+	struct mxu1_device *mxdev;
+
 	mxport = kzalloc(sizeof(struct mxu1_port), GFP_KERNEL);
 	if (!mxport)
 		return -ENOMEM;
@@ -381,16 +452,6 @@ static int mxu1_port_remove(struct usb_serial_port *port)
 static int mxu1_startup(struct usb_serial *serial)
 {
 	struct mxu1_device *mxdev;
-	struct usb_device *dev = serial->dev;
-	struct usb_host_interface *cur_altsetting;
-	char fw_name[32];
-	const struct firmware *fw_p = NULL;
-	int err;
-
-	dev_dbg(&serial->interface->dev, "%s - product 0x%04X, num configurations %d, configuration value %d\n",
-		__func__, le16_to_cpu(dev->descriptor.idProduct),
-		dev->descriptor.bNumConfigurations,
-		dev->actconfig->desc.bConfigurationValue);
 
 	/* create device structure */
 	mxdev = kzalloc(sizeof(struct mxu1_device), GFP_KERNEL);
@@ -399,42 +460,7 @@ static int mxu1_startup(struct usb_serial *serial)
 
 	usb_set_serial_data(serial, mxdev);
 
-	mxdev->mxd_model = le16_to_cpu(dev->descriptor.idProduct);
-
-	cur_altsetting = serial->interface->cur_altsetting;
-
-	/* if we have only 1 configuration, download firmware */
-	if (cur_altsetting->desc.bNumEndpoints == 1) {
-
-		snprintf(fw_name,
-			 sizeof(fw_name),
-			 "moxa/moxa-%04x.fw",
-			 mxdev->mxd_model);
-
-		err = request_firmware(&fw_p, fw_name, &serial->interface->dev);
-		if (err) {
-			dev_err(&serial->interface->dev, "failed to request firmware: %d\n",
-				err);
-			goto err_free_mxdev;
-		}
-
-		err = mxu1_download_firmware(serial, fw_p);
-		if (err)
-			goto err_release_firmware;
-
-		/* device is being reset */
-		err = -ENODEV;
-		goto err_release_firmware;
-	}
-
 	return 0;
-
-err_release_firmware:
-	release_firmware(fw_p);
-err_free_mxdev:
-	kfree(mxdev);
-
-	return err;
 }
 
 static void mxu1_release(struct usb_serial *serial)
@@ -964,6 +990,7 @@ static struct usb_serial_driver mxu11x0_device = {
 	.description		= "MOXA UPort 11x0",
 	.id_table		= mxu1_idtable,
 	.num_ports		= 1,
+	.probe                  = mxu1_probe,
 	.port_probe             = mxu1_port_probe,
 	.port_remove            = mxu1_port_remove,
 	.attach			= mxu1_startup,
-- 
2.6.4


      parent reply	other threads:[~2016-01-04 18:50 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-04 18:49 [PATCH v2 0/4] USB: mxu11x0: fixes and follow ups Mathieu OTHACEHE
2016-01-04 18:49 ` [PATCH v2 1/4] USB: mxu11x0: fix memory leak on usb_serial private data Mathieu OTHACEHE
2016-01-25 12:01   ` Johan Hovold
2016-01-30 17:40     ` Mathieu OTHACEHE
2016-02-28 12:20       ` Johan Hovold
2016-02-28 16:30         ` Mathieu OTHACEHE
2016-02-29  9:38           ` Johan Hovold
2016-01-04 18:49 ` [PATCH v2 2/4] USB: mxu11x0: clean device control commands Mathieu OTHACEHE
2016-01-04 18:49 ` [PATCH v2 3/4] USB: mxu11x0: remove duplicated set_termios call Mathieu OTHACEHE
2016-01-04 18:49 ` Mathieu OTHACEHE [this message]

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=1451933379-21994-5-git-send-email-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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox