From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755352AbcANRVF (ORCPT ); Thu, 14 Jan 2016 12:21:05 -0500 Received: from bhuna.collabora.co.uk ([46.235.227.227]:42478 "EHLO bhuna.collabora.co.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753261AbcANRVC (ORCPT ); Thu, 14 Jan 2016 12:21:02 -0500 Message-ID: <5697D8FB.7000709@collabora.co.uk> Date: Thu, 14 Jan 2016 17:20:59 +0000 From: Martyn Welch User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Icedove/31.8.0 MIME-Version: 1.0 To: Konstantin Shkolnyy , Konstantin Shkolnyy , "johan@kernel.org" CC: "linux-usb@vger.kernel.org" , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH v2 1/3 RESEND] USB: serial: cp210x: New 16-bit register access functions. References: <1450836867-27120-1-git-send-email-konstantin.shkolnyy@gmail.com> <5697CD37.4090504@collabora.co.uk> In-Reply-To: Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 14/01/16 17:10, Konstantin Shkolnyy wrote: >> -----Original Message----- >> From: linux-usb-owner@vger.kernel.org [mailto:linux-usb- >> owner@vger.kernel.org] On Behalf Of Martyn Welch >> Sent: Thursday, January 14, 2016 10:31 >> To: Konstantin Shkolnyy; johan@kernel.org >> Cc: linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org >> Subject: Re: [PATCH v2 1/3 RESEND] USB: serial: cp210x: New 16-bit register >> access functions. >> >> On 23/12/15 02:14, Konstantin Shkolnyy wrote: >>> cp210x_get_config and cp210x_set_config are cumbersome to use. This >> change >>> introduces new register access functions for 16-bit values, instead of >>> the above functions. >>> >>> Signed-off-by: Konstantin Shkolnyy >>> --- >>> drivers/usb/serial/cp210x.c | 155 >> +++++++++++++++++++++++++++++++------------- >>> 1 file changed, 111 insertions(+), 44 deletions(-) >>> >>> diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c >>> index fd67958..fd7c4f4 100644 >>> --- a/drivers/usb/serial/cp210x.c >>> +++ b/drivers/usb/serial/cp210x.c >>> @@ -422,14 +422,88 @@ static int cp210x_set_config(struct >> usb_serial_port *port, u8 request, >>> } >>> >>> /* >>> - * cp210x_set_config_single >>> - * Convenience function for calling cp210x_set_config on single data >> values >>> - * without requiring an integer pointer >>> + * Reads a variable-sized block of CP210X_ registers, identified by req. >>> + * Returns data into buf in native USB byte order. >>> */ >>> -static inline int cp210x_set_config_single(struct usb_serial_port *port, >>> - u8 request, unsigned int data) >>> +static int cp210x_read_reg_block(struct usb_serial_port *port, u8 req, >>> + void *buf, int bufsize) >>> { >>> - return cp210x_set_config(port, request, &data, 2); >>> + struct usb_serial *serial = port->serial; >>> + struct cp210x_port_private *port_priv = >> usb_get_serial_port_data(port); >>> + void *dmabuf; >>> + int result; >>> + >>> + dmabuf = kmalloc(bufsize, GFP_KERNEL); >>> + if (!dmabuf) { >>> + /* >>> + * FIXME Some callers don't bother to check for error, >>> + * at least give them consistent junk until they are fixed >>> + */ >>> + memset(buf, 0, bufsize); >>> + return -ENOMEM; >>> + } >>> + >>> + result = usb_control_msg(serial->dev, usb_rcvctrlpipe(serial->dev, >> 0), >>> + req, REQTYPE_INTERFACE_TO_HOST, 0, >>> + port_priv->bInterfaceNumber, dmabuf, bufsize, >>> + USB_CTRL_SET_TIMEOUT); >>> + if (result == bufsize) { >>> + memcpy(buf, dmabuf, bufsize); >>> + result = 0; >> >> I could be wrong, but isn't dmabuf a little-endian stream (hence the >> byte order swapping in the existing set and get functions). Won't this >> break on big endian systems due to the lack of byte swapping? > > This function is intended to not convert, as I tried to say in the header comment. > Ah, gotcha - conversion done in cp210x_read_u16_reg(). Martyn