From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932205AbcARRmn (ORCPT ); Mon, 18 Jan 2016 12:42:43 -0500 Received: from mail-lf0-f67.google.com ([209.85.215.67]:36192 "EHLO mail-lf0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755981AbcARRmj (ORCPT ); Mon, 18 Jan 2016 12:42:39 -0500 Date: Mon, 18 Jan 2016 18:42:36 +0100 From: Johan Hovold To: Konstantin Shkolnyy Cc: Martyn Welch , Konstantin Shkolnyy , "johan@kernel.org" , "linux-usb@vger.kernel.org" , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH v3 2/4] USB: serial: cp210x: Switch to new 16-bit register access functions. Message-ID: <20160118174236.GG3169@localhost> References: <1451704323-11941-1-git-send-email-konstantin.shkolnyy@gmail.com> <5697DE45.7000204@collabora.co.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: 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 On Thu, Jan 14, 2016 at 06:22:29PM +0000, 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 11:44 > > To: Konstantin Shkolnyy; johan@kernel.org > > Cc: linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org > > Subject: Re: [PATCH v3 2/4] USB: serial: cp210x: Switch to new 16-bit register > > access functions. > ... > > > > @@ -697,14 +685,11 @@ static unsigned int > > cp210x_quantise_baudrate(unsigned int baud) > > > > > > static int cp210x_open(struct tty_struct *tty, struct usb_serial_port *port) > > > { > > > - int result; > > > + int err; Why rename the return value? I prefer ret over err, but there's no need to change such things for code that's already in place. > > > > > > - result = cp210x_set_config_single(port, CP210X_IFC_ENABLE, > > > - > > UART_ENABLE); > > > - if (result) { > > > - dev_err(&port->dev, "%s - Unable to enable UART\n", > > __func__); > > > - return result; > > > - } > > > + err = cp210x_write_u16_reg(port, CP210X_IFC_ENABLE, > > UART_ENABLE); > > > + if (err) > > > + return err; > > > > Any reason for dropping the error message? > > I already print a message if the underlying usb_control_msg fails, so > it should be covered there. Yes, but it's an unrelated change. The previous error message was more informative. Thanks, Johan