Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1300312 > unrolled thread

[PATCH 3/3] USB: mxu11x0: move firmware download and endpoint testing to probe callback

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 3/3] USB: mxu11x0: move firmware download and endpoint testing to probe callback Mathieu OTHACEHE <m.othacehe@gmail.com> - 2016-01-03 15:30 +0100
    Re: [PATCH 3/3] USB: mxu11x0: move firmware download and endpoint  testing to probe callback Johan Hovold <johan@kernel.org> - 2016-01-03 17:50 +0100

#1300312 — [PATCH 3/3] USB: mxu11x0: move firmware download and endpoint testing to probe callback

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>
---
 drivers/usb/serial/mxu11x0.c | 130 ++++++++++++++++++++++++++-----------------
 1 file changed, 78 insertions(+), 52 deletions(-)

diff --git a/drivers/usb/serial/mxu11x0.c b/drivers/usb/serial/mxu11x0.c
index 354fcb5..0392531 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);
@@ -336,16 +337,85 @@ static void mxu1_release(struct usb_serial *serial)
 	kfree(mxdev);
 }
 
-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;
@@ -389,16 +459,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);
@@ -407,42 +467,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 int mxu1_write_byte(struct usb_serial_port *port, u32 addr,
@@ -957,6 +982,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.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1300334 — Re: [PATCH 3/3] USB: mxu11x0: move firmware download and endpoint testing to probe callback

On Sun, Jan 03, 2016 at 03:26:01PM +0100, Mathieu OTHACEHE wrote:
> 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>
> ---
>  drivers/usb/serial/mxu11x0.c | 130 ++++++++++++++++++++++++++-----------------
>  1 file changed, 78 insertions(+), 52 deletions(-)
> 
> diff --git a/drivers/usb/serial/mxu11x0.c b/drivers/usb/serial/mxu11x0.c
> index 354fcb5..0392531 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);
> @@ -336,16 +337,85 @@ static void mxu1_release(struct usb_serial *serial)
>  	kfree(mxdev);
>  }
>  
> -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)) {

No need for inner parentheses.

> +
> +		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
> +		 */

Multi-line comments should be on the following format

	/*
	 * ...
	 */

Looks good otherwise.

Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web