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@xxxxxxxxx>
> ---
> 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-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
