From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755071AbcGTVHC (ORCPT ); Wed, 20 Jul 2016 17:07:02 -0400 Received: from mail-pa0-f68.google.com ([209.85.220.68]:33978 "EHLO mail-pa0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754731AbcGTVG6 (ORCPT ); Wed, 20 Jul 2016 17:06:58 -0400 Date: Wed, 20 Jul 2016 14:06:53 -0700 From: Dmitry Torokhov To: Martin Kepplinger Cc: linux-kernel@vger.kernel.org, linux-input@vger.kernel.org Subject: Re: [PATCH 1/4] input: tablet: pegasus_notetaker: Track usb control msg errors Message-ID: <20160720210653.GF25655@dtor-ws> References: <1468852149-2614-1-git-send-email-martink@posteo.de> <1468852149-2614-2-git-send-email-martink@posteo.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1468852149-2614-2-git-send-email-martink@posteo.de> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Martin, On Mon, Jul 18, 2016 at 04:29:06PM +0200, Martin Kepplinger wrote: > Signed-off-by: Martin Kepplinger > --- > drivers/input/tablet/pegasus_notetaker.c | 26 ++++++++++++++++++-------- > 1 file changed, 18 insertions(+), 8 deletions(-) > > diff --git a/drivers/input/tablet/pegasus_notetaker.c b/drivers/input/tablet/pegasus_notetaker.c > index 83aa583..27cb352 100644 > --- a/drivers/input/tablet/pegasus_notetaker.c > +++ b/drivers/input/tablet/pegasus_notetaker.c > @@ -79,7 +79,7 @@ struct pegasus { > struct work_struct init; > }; > > -static void pegasus_control_msg(struct pegasus *pegasus, u8 *data, int len) > +static int pegasus_control_msg(struct pegasus *pegasus, u8 *data, int len) > { > const int sizeof_buf = len + 2; > int result; > @@ -87,7 +87,7 @@ static void pegasus_control_msg(struct pegasus *pegasus, u8 *data, int len) > > cmd_buf = kmalloc(sizeof_buf, GFP_KERNEL); > if (!cmd_buf) > - return; > + return -ENOMEM; > > cmd_buf[0] = NOTETAKER_REPORT_ID; > cmd_buf[1] = len; > @@ -100,17 +100,23 @@ static void pegasus_control_msg(struct pegasus *pegasus, u8 *data, int len) > 0, 0, cmd_buf, sizeof_buf, > USB_CTRL_SET_TIMEOUT); > > - if (result != sizeof_buf) > - dev_err(&pegasus->usbdev->dev, "control msg error\n"); > + if (result != sizeof_buf) { > + if (result >= 0) > + result = -EIO; > + dev_err(&pegasus->usbdev->dev, "control msg error: %d\n", > + result); > + } > > kfree(cmd_buf); > + > + return result; Your 4th patch relies on pegasus_control_msg() returning 0 on success, otherwise you will not restart the URB when doing reset resume. I rearranged the code here so we free cmd_buf right after calling usb_control_msg() and the explicitly returnig 0 in success branch. > } > > -static void pegasus_set_mode(struct pegasus *pegasus, u8 mode, u8 led) > +static int pegasus_set_mode(struct pegasus *pegasus, u8 mode, u8 led) > { > u8 cmd[] = {NOTETAKER_SET_CMD, NOTETAKER_SET_MODE, led, mode}; > > - pegasus_control_msg(pegasus, cmd, sizeof(cmd)); > + return pegasus_control_msg(pegasus, cmd, sizeof(cmd)); > } > > static void pegasus_parse_packet(struct pegasus *pegasus) > @@ -184,8 +190,12 @@ static void pegasus_irq(struct urb *urb) > static void pegasus_init(struct work_struct *work) > { > struct pegasus *pegasus = container_of(work, struct pegasus, init); > + int retval; > > - pegasus_set_mode(pegasus, PEN_MODE_XY, NOTETAKER_LED_MOUSE); > + retval = pegasus_set_mode(pegasus, PEN_MODE_XY, NOTETAKER_LED_MOUSE); > + if (retval < 0) > + dev_err(&pegasus->usbdev->dev, "pegasus_set_mode error: %d\n", > + retval); > } > > static int pegasus_open(struct input_dev *dev) > @@ -201,7 +211,7 @@ static int pegasus_open(struct input_dev *dev) > if (usb_submit_urb(pegasus->irq, GFP_KERNEL)) > retval = -EIO; > > - pegasus_set_mode(pegasus, PEN_MODE_XY, NOTETAKER_LED_MOUSE); > + retval = pegasus_set_mode(pegasus, PEN_MODE_XY, NOTETAKER_LED_MOUSE); > > usb_autopm_put_interface(pegasus->intf); > Thanks. -- Dmitry