Re: [PATCH -next] ieee802154: atusb: move to new USB API

[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@xxxxxxxxx/ [0]
> Signed-off-by: Pavel Skripkin <paskripkin@xxxxxxxxx>
> ---
>
> 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