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
Hi Sascha,
> -----Original Message-----
> From: Dan Carpenter <[email protected]>
> Sent: Thursday, May 4, 2023 6:56 PM
> To: [email protected]
> Cc: [email protected]
> 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
> -----Original Message-----
> From: Ping-Ke Shih <[email protected]>
> Sent: Monday, May 8, 2023 10:39 AM
> To: Dan Carpenter <[email protected]>; [email protected]
> Cc: [email protected]
> Subject: RE: [bug report] wifi: rtw88: usb: fix priority queue to endpoint mapping
>
> Hi Sascha,
>
> > -----Original Message-----
> > From: Dan Carpenter <[email protected]>
> > Sent: Thursday, May 4, 2023 6:56 PM
> > To: [email protected]
> > Cc: [email protected]
> > 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/[email protected]/T/#u
Ping-Ke
Hi Ping-Ke,
On Mon, May 08, 2023 at 02:39:19AM +0000, Ping-Ke Shih wrote:
> Hi Sascha,
>
> > -----Original Message-----
> > From: Dan Carpenter <[email protected]>
> > Sent: Thursday, May 4, 2023 6:56 PM
> > To: [email protected]
> > Cc: [email protected]
> > 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 |
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