[bug report] wifi: rtw88: usb: fix priority queue to endpoint mapping

[bug report] wifi: rtw88: usb: fix priority queue to endpoint mapping

[Dan Carpenter]

Hello Sascha Hauer,

The patch a6f187f92bcc: "wifi: rtw88: usb: fix priority queue to
endpoint mapping" from Apr 17, 2023, leads to the following Smatch
static checker warning:

drivers/net/wireless/realtek/rtw88/usb.c:219 rtw_usb_parse() warn: assigning (-22) to unsigned variable 'rtwusb->qsel_to_ep[8]'
drivers/net/wireless/realtek/rtw88/usb.c:220 rtw_usb_parse() warn: assigning (-22) to unsigned variable 'rtwusb->qsel_to_ep[9]'
drivers/net/wireless/realtek/rtw88/usb.c:221 rtw_usb_parse() warn: assigning (-22) to unsigned variable 'rtwusb->qsel_to_ep[10]'
drivers/net/wireless/realtek/rtw88/usb.c:222 rtw_usb_parse() warn: assigning (-22) to unsigned variable 'rtwusb->qsel_to_ep[11]'
drivers/net/wireless/realtek/rtw88/usb.c:223 rtw_usb_parse() warn: assigning (-22) to unsigned variable 'rtwusb->qsel_to_ep[12]'
drivers/net/wireless/realtek/rtw88/usb.c:224 rtw_usb_parse() warn: assigning (-22) to unsigned variable 'rtwusb->qsel_to_ep[13]'
drivers/net/wireless/realtek/rtw88/usb.c:225 rtw_usb_parse() warn: assigning (-22) to unsigned variable 'rtwusb->qsel_to_ep[14]'
drivers/net/wireless/realtek/rtw88/usb.c:226 rtw_usb_parse() warn: assigning (-22) to unsigned variable 'rtwusb->qsel_to_ep[15]'

drivers/net/wireless/realtek/rtw88/usb.c
    137 static int rtw_usb_parse(struct rtw_dev *rtwdev,
    138                          struct usb_interface *interface)
    139 {
    140         struct rtw_usb *rtwusb = rtw_get_usb_priv(rtwdev);
    141         struct usb_host_interface *host_interface = &interface->altsetting[0];
    142         struct usb_interface_descriptor *interface_desc = &host_interface->desc;
    143         struct usb_endpoint_descriptor *endpoint;
    144         struct usb_device *usbd = interface_to_usbdev(interface);
    145         int num_out_pipes = 0;
    146         int i;
    147         u8 num;
    148         const struct rtw_chip_info *chip = rtwdev->chip;
    149         const struct rtw_rqpn *rqpn;
    150 
    151         for (i = 0; i < interface_desc->bNumEndpoints; i++) {
    152                 endpoint = &host_interface->endpoint[i].desc;
    153                 num = usb_endpoint_num(endpoint);
    154 
    155                 if (usb_endpoint_dir_in(endpoint) &&
    156                     usb_endpoint_xfer_bulk(endpoint)) {
    157                         if (rtwusb->pipe_in) {
    158                                 rtw_err(rtwdev, "IN pipes overflow\n");
    159                                 return -EINVAL;
    160                         }
    161 
    162                         rtwusb->pipe_in = num;
    163                 }
    164 
    165                 if (usb_endpoint_dir_in(endpoint) &&
    166                     usb_endpoint_xfer_int(endpoint)) {
    167                         if (rtwusb->pipe_interrupt) {
    168                                 rtw_err(rtwdev, "INT pipes overflow\n");
    169                                 return -EINVAL;
    170                         }
    171 
    172                         rtwusb->pipe_interrupt = num;
    173                 }
    174 
    175                 if (usb_endpoint_dir_out(endpoint) &&
    176                     usb_endpoint_xfer_bulk(endpoint)) {
    177                         if (num_out_pipes >= ARRAY_SIZE(rtwusb->out_ep)) {
    178                                 rtw_err(rtwdev, "OUT pipes overflow\n");
    179                                 return -EINVAL;
    180                         }
    181 
    182                         rtwusb->out_ep[num_out_pipes++] = num;
    183                 }
    184         }
    185 
    186         switch (usbd->speed) {
    187         case USB_SPEED_LOW:
    188         case USB_SPEED_FULL:
    189                 rtwusb->bulkout_size = RTW_USB_FULL_SPEED_BULK_SIZE;
    190                 break;
    191         case USB_SPEED_HIGH:
    192                 rtwusb->bulkout_size = RTW_USB_HIGH_SPEED_BULK_SIZE;
    193                 break;
    194         case USB_SPEED_SUPER:
    195                 rtwusb->bulkout_size = RTW_USB_SUPER_SPEED_BULK_SIZE;
    196                 break;
    197         default:
    198                 rtw_err(rtwdev, "failed to detect usb speed\n");
    199                 return -EINVAL;
    200         }
    201 
    202         rtwdev->hci.bulkout_num = num_out_pipes;
    203 
    204         if (num_out_pipes < 1 || num_out_pipes > 4) {
    205                 rtw_err(rtwdev, "invalid number of endpoints %d\n", num_out_pipes);
    206                 return -EINVAL;
    207         }
    208 
    209         rqpn = &chip->rqpn_table[num_out_pipes];
    210 
    211         rtwusb->qsel_to_ep[TX_DESC_QSEL_TID0] = dma_mapping_to_ep(rqpn->dma_map_be);
    212         rtwusb->qsel_to_ep[TX_DESC_QSEL_TID1] = dma_mapping_to_ep(rqpn->dma_map_bk);
    213         rtwusb->qsel_to_ep[TX_DESC_QSEL_TID2] = dma_mapping_to_ep(rqpn->dma_map_bk);
    214         rtwusb->qsel_to_ep[TX_DESC_QSEL_TID3] = dma_mapping_to_ep(rqpn->dma_map_be);
    215         rtwusb->qsel_to_ep[TX_DESC_QSEL_TID4] = dma_mapping_to_ep(rqpn->dma_map_vi);
    216         rtwusb->qsel_to_ep[TX_DESC_QSEL_TID5] = dma_mapping_to_ep(rqpn->dma_map_vi);
    217         rtwusb->qsel_to_ep[TX_DESC_QSEL_TID6] = dma_mapping_to_ep(rqpn->dma_map_vo);
    218         rtwusb->qsel_to_ep[TX_DESC_QSEL_TID7] = dma_mapping_to_ep(rqpn->dma_map_vo);
--> 219         rtwusb->qsel_to_ep[TX_DESC_QSEL_TID8] = -EINVAL;

Can't save negative error codes to a u8.

    220         rtwusb->qsel_to_ep[TX_DESC_QSEL_TID9] = -EINVAL;
    221         rtwusb->qsel_to_ep[TX_DESC_QSEL_TID10] = -EINVAL;
    222         rtwusb->qsel_to_ep[TX_DESC_QSEL_TID11] = -EINVAL;
    223         rtwusb->qsel_to_ep[TX_DESC_QSEL_TID12] = -EINVAL;
    224         rtwusb->qsel_to_ep[TX_DESC_QSEL_TID13] = -EINVAL;
    225         rtwusb->qsel_to_ep[TX_DESC_QSEL_TID14] = -EINVAL;
    226         rtwusb->qsel_to_ep[TX_DESC_QSEL_TID15] = -EINVAL;
    227         rtwusb->qsel_to_ep[TX_DESC_QSEL_BEACON] = dma_mapping_to_ep(rqpn->dma_map_hi);
    228         rtwusb->qsel_to_ep[TX_DESC_QSEL_HIGH] = dma_mapping_to_ep(rqpn->dma_map_hi);
    229         rtwusb->qsel_to_ep[TX_DESC_QSEL_MGMT] = dma_mapping_to_ep(rqpn->dma_map_mg);
    230         rtwusb->qsel_to_ep[TX_DESC_QSEL_H2C] = dma_mapping_to_ep(rqpn->dma_map_hi);
    231 
    232         return 0;
    233 }

regards,
dan carpenter

RE: [bug report] wifi: rtw88: usb: fix priority queue to endpoint mapping

[Ping-Ke Shih]

Hi Sascha,

> -----Original Message-----
> From: Dan Carpenter <dan.carpenter@linaro.org>
> Sent: Thursday, May 4, 2023 6:56 PM
> To: s.hauer@pengutronix.de
> Cc: linux-wireless@vger.kernel.org
> Subject: [bug report] wifi: rtw88: usb: fix priority queue to endpoint mapping
> 
> Hello Sascha Hauer,
> 
> The patch a6f187f92bcc: "wifi: rtw88: usb: fix priority queue to
> endpoint mapping" from Apr 17, 2023, leads to the following Smatch
> static checker warning:
> 
> drivers/net/wireless/realtek/rtw88/usb.c:219 rtw_usb_parse() warn: assigning (-22) to unsigned variable
> 'rtwusb->qsel_to_ep[8]'

[...]

>     218         rtwusb->qsel_to_ep[TX_DESC_QSEL_TID7] = dma_mapping_to_ep(rqpn->dma_map_vo);
> --> 219         rtwusb->qsel_to_ep[TX_DESC_QSEL_TID8] = -EINVAL;
> 
> Can't save negative error codes to a u8.
> 

return type of dma_mapping_to_ep() is 'int' and it also possibly returns -EINVAL, and
rtwusb->qsel_to_ep[] is used by qsel_to_ep() that also use 'int' as return type. 
Therefore, I would like to change type of qsel_to_ep[] from 'u8' to 'int'. Does it
work to you?

Ping-Ke

RE: [bug report] wifi: rtw88: usb: fix priority queue to endpoint mapping

[Ping-Ke Shih]

> -----Original Message-----
> From: Ping-Ke Shih <pkshih@realtek.com>
> Sent: Monday, May 8, 2023 10:39 AM
> To: Dan Carpenter <dan.carpenter@linaro.org>; s.hauer@pengutronix.de
> Cc: linux-wireless@vger.kernel.org
> Subject: RE: [bug report] wifi: rtw88: usb: fix priority queue to endpoint mapping
> 
> Hi Sascha,
> 
> > -----Original Message-----
> > From: Dan Carpenter <dan.carpenter@linaro.org>
> > Sent: Thursday, May 4, 2023 6:56 PM
> > To: s.hauer@pengutronix.de
> > Cc: linux-wireless@vger.kernel.org
> > Subject: [bug report] wifi: rtw88: usb: fix priority queue to endpoint mapping
> >
> > Hello Sascha Hauer,
> >
> > The patch a6f187f92bcc: "wifi: rtw88: usb: fix priority queue to
> > endpoint mapping" from Apr 17, 2023, leads to the following Smatch
> > static checker warning:
> >
> > drivers/net/wireless/realtek/rtw88/usb.c:219 rtw_usb_parse() warn: assigning (-22) to unsigned variable
> > 'rtwusb->qsel_to_ep[8]'
> 
> [...]
> 
> >     218         rtwusb->qsel_to_ep[TX_DESC_QSEL_TID7] = dma_mapping_to_ep(rqpn->dma_map_vo);
> > --> 219         rtwusb->qsel_to_ep[TX_DESC_QSEL_TID8] = -EINVAL;
> >
> > Can't save negative error codes to a u8.
> >
> 
> return type of dma_mapping_to_ep() is 'int' and it also possibly returns -EINVAL, and
> rtwusb->qsel_to_ep[] is used by qsel_to_ep() that also use 'int' as return type.
> Therefore, I would like to change type of qsel_to_ep[] from 'u8' to 'int'. Does it
> work to you?
> 

I have made a patch [1] along with above idea. 

[1] https://lore.kernel.org/linux-wireless/20230508085539.46795-1-pkshih@realtek.com/T/#u

Ping-Ke

Re: [bug report] wifi: rtw88: usb: fix priority queue to endpoint mapping

[s.hauer]

Hi Ping-Ke,

On Mon, May 08, 2023 at 02:39:19AM +0000, Ping-Ke Shih wrote:
> Hi Sascha,
> 
> > -----Original Message-----
> > From: Dan Carpenter <dan.carpenter@linaro.org>
> > Sent: Thursday, May 4, 2023 6:56 PM
> > To: s.hauer@pengutronix.de
> > Cc: linux-wireless@vger.kernel.org
> > Subject: [bug report] wifi: rtw88: usb: fix priority queue to endpoint mapping
> > 
> > Hello Sascha Hauer,
> > 
> > The patch a6f187f92bcc: "wifi: rtw88: usb: fix priority queue to
> > endpoint mapping" from Apr 17, 2023, leads to the following Smatch
> > static checker warning:
> > 
> > drivers/net/wireless/realtek/rtw88/usb.c:219 rtw_usb_parse() warn: assigning (-22) to unsigned variable
> > 'rtwusb->qsel_to_ep[8]'
> 
> [...]
> 
> >     218         rtwusb->qsel_to_ep[TX_DESC_QSEL_TID7] = dma_mapping_to_ep(rqpn->dma_map_vo);
> > --> 219         rtwusb->qsel_to_ep[TX_DESC_QSEL_TID8] = -EINVAL;
> > 
> > Can't save negative error codes to a u8.
> > 
> 
> return type of dma_mapping_to_ep() is 'int' and it also possibly returns -EINVAL, and
> rtwusb->qsel_to_ep[] is used by qsel_to_ep() that also use 'int' as return type. 
> Therefore, I would like to change type of qsel_to_ep[] from 'u8' to 'int'. Does it
> work to you?

Fine with me. I acked your patch.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [bug report] wifi: rtw88: usb: fix priority queue to endpoint mapping

[Larry Finger]

On 5/7/23 21:39, Ping-Ke Shih wrote:
> return type of dma_mapping_to_ep() is 'int' and it also possibly returns -EINVAL, and
> rtwusb->qsel_to_ep[] is used by qsel_to_ep() that also use 'int' as return type.
> Therefore, I would like to change type of qsel_to_ep[] from 'u8' to 'int'. Does it
> work to you?
> 
Sasha and Ping-Ke,

I have been testing using s8 rather than u8 for qsel_to_ep[], if you would like 
to keep the array the same size. All the values fit within the s8 limits.

Larry