On Fri, Jul 24, 2015 at 08:48:09AM +0200, Petr Tesarik wrote:
> From: Petr Tesarik <ptesarik@xxxxxxx>
>
> There is a lot of overlap between the two functions (e.g. calculation
> of the buffer size), so this removes a bit of code duplication, but
> most importantly, a more generic function can be easily reused for
> other message types.
I'm not sure I consider this is an improvement yet.
> Signed-off-by: Petr Tesarik <ptesarik@xxxxxxxx>
> ---
> drivers/usb/serial/cp210x.c | 109 ++++++++++++++++++++------------------------
> 1 file changed, 49 insertions(+), 60 deletions(-)
>
> diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
> index 1bae015..69f03b6 100644
> --- a/drivers/usb/serial/cp210x.c
> +++ b/drivers/usb/serial/cp210x.c
> @@ -307,14 +307,17 @@ enum cp210x_request_type {
> #define CONTROL_WRITE_RTS 0x0200
>
> /*
> - * cp210x_get_config
> - * Reads from the CP210x configuration registers
> + * cp210x_control_msg
> + * Sends a generic control message, taking care of endianness
> + * and error messages.
> * 'size' is specified in bytes.
> - * 'data' is a pointer to a pre-allocated array of integers large
> - * enough to hold 'size' bytes (with 4 bytes to each integer)
> + * 'data' is a pointer to the input/output buffer. For output, it holds
> + * the data (in host order) to be sent. For input, it receives data from
> + * the device and must be big enough to hold 'size' bytes.
> */
> -static int cp210x_get_config(struct usb_serial_port *port, u8 request,
> - unsigned int *data, int size)
> +static int cp210x_control_msg(struct usb_serial_port *port, u8 request,
> + u8 requesttype, u16 value, u32 *data, int size,
> + int timeout)
Should you not use your new request type enum here?
> {
> struct usb_serial *serial = port->serial;
> struct cp210x_serial_private *spriv = usb_get_serial_data(serial);
> @@ -328,20 +331,22 @@ static int cp210x_get_config(struct usb_serial_port *port, u8 request,
> if (!buf)
> return -ENOMEM;
>
> - /* Issue the request, attempting to read 'size' bytes */
> - result = usb_control_msg(serial->dev, usb_rcvctrlpipe(serial->dev, 0),
> - request, REQTYPE_INTERFACE_TO_HOST, 0x0000,
> - spriv->bInterfaceNumber, buf, size,
> - USB_CTRL_GET_TIMEOUT);
> + if (!(requesttype & USB_DIR_IN)) {
> + for (i = 0; i < length; i++)
> + buf[i] = cpu_to_le32(data[i]);
> + }
>
> - /* Convert data into an array of integers */
> - for (i = 0; i < length; i++)
> - data[i] = le32_to_cpu(buf[i]);
> + result = usb_control_msg(serial->dev, usb_rcvctrlpipe(serial->dev, 0),
And this should be usb_sndctrlpipe for outgoing messages.
> + request, requesttype, value,
> + spriv->bInterfaceNumber, buf, size, timeout);
Please resend this when you start using your generalised function (for
the gpio work?).
I'll drop all four for now.
Thanks,
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
