lkml.org 
[lkml]   [2022]   [Jan]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH -next] ieee802154: atusb: move to new USB API
From

Hello.

On 05.01.22 15:49, Pavel Skripkin wrote:
> Old USB API is prone to uninit value bugs if error handling is not
> correct. Let's move atusb to use new USB API to
>
> 1) Make code more simple, since new API does not require memory
> to be allocates via kmalloc()
>
> 2) Defend driver from usb-related uninit value bugs.
>
> 3) Make code more modern and simple
>
> This patch removes atusb usb wrappers as Greg suggested [0], this will make
> code more obvious and easier to understand over time, and replaces old
> API calls with new ones.
>
> Also this patch adds and updates usb related error handling to prevent
> possible uninit value bugs in future
>
> Link: https://lore.kernel.org/all/YdL0GPxy4TdGDzOO@kroah.com/ [0]
> Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
> ---
>
> Only build tested.

Gave it a first quick run on real hardware here. Besides one small bug
(see below) it looked good.

Will give it a bit more testing over the next days.



> @@ -881,14 +819,27 @@ static int atusb_get_and_conf_chip(struct atusb *atusb)
> u8 man_id_0, man_id_1, part_num, version_num;
> const char *chip;
> struct ieee802154_hw *hw = atusb->hw;
> + int ret;
>
> - man_id_0 = atusb_read_reg(atusb, RG_MAN_ID_0);
> - man_id_1 = atusb_read_reg(atusb, RG_MAN_ID_1);
> - part_num = atusb_read_reg(atusb, RG_PART_NUM);
> - version_num = atusb_read_reg(atusb, RG_VERSION_NUM);
> + ret = usb_control_msg_recv(usb_dev, 0, ATUSB_REG_READ, ATUSB_REQ_FROM_DEV,
> + 0, RG_MAN_ID_0, &man_id_0, 1, 1000, GFP_KERNEL);
> + if (ret < 0)
> + return ret;
>
> - if (atusb->err)
> - return atusb->err;
> + ret = usb_control_msg_recv(usb_dev, 0, ATUSB_REG_READ, ATUSB_REQ_FROM_DEV,
> + 0, RG_MAN_ID_1, &man_id_1, 1, 1000, GFP_KERNEL);
> + if (ret < 0)
> + return ret;
> +
> + ret = usb_control_msg_recv(usb_dev, 0, ATUSB_REG_READ, ATUSB_REQ_FROM_DEV,
> + 0, RG_PART_NUM, &atusb, 1, 1000, GFP_KERNEL);

This needs to be written to &part_num and not &atusb.

Pretty nice for a first blind try without hardware. Thanks.

Will let you know if I find anything else from testing.

regards
Stefan Schmidt

\
 
 \ /
  Last update: 2022-01-05 21:29    [W:0.080 / U:0.084 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site