Hi Martin,
On Mon, Jul 18, 2016 at 04:29:06PM +0200, Martin Kepplinger wrote:
> Signed-off-by: Martin Kepplinger <mart...@posteo.de>
> ---
> 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
