Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754535AbbG3RBd (ORCPT ); Thu, 30 Jul 2015 13:01:33 -0400 Received: from mail-lb0-f170.google.com ([209.85.217.170]:34549 "EHLO mail-lb0-f170.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753111AbbG3RB3 (ORCPT ); Thu, 30 Jul 2015 13:01:29 -0400 Date: Thu, 30 Jul 2015 19:01:27 +0200 From: Johan Hovold To: Petr Tesarik Cc: Johan Hovold , Greg Kroah-Hartman , "open list:USB SERIAL SUBSYSTEM" , open list , Petr Tesarik Subject: Re: [PATCH 2/4] cp210x: Unify code for set/get config control messages Message-ID: <20150730170127.GF28535@localhost> References: <1437720491-28702-1-git-send-email-ptesarik@suse.com> <1437720491-28702-3-git-send-email-ptesarik@suse.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1437720491-28702-3-git-send-email-ptesarik@suse.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3124 Lines: 82 On Fri, Jul 24, 2015 at 08:48:09AM +0200, Petr Tesarik wrote: > From: Petr Tesarik > > 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 > --- > 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-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/