Messages in this thread | | | Date | Wed, 5 Jan 2022 21:27:43 +0100 | Subject | Re: [PATCH -next] ieee802154: atusb: move to new USB API | From | Stefan Schmidt <> |
| |
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
| |