| Message ID | 20180924091853.452055-1-lkundrak@v3.sk (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | libertas: return errno from lbs_add_card() | expand |
Hi Lubomir,
I love your patch! Yet something to improve:
[auto build test ERROR on wireless-drivers-next/master]
[also build test ERROR on v4.19-rc5 next-20180924]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Lubomir-Rintel/libertas-return-errno-from-lbs_add_card/20180925-105034
base: https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers-next.git master
config: sh-allmodconfig (attached as .config)
compiler: sh4-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.2.0 make.cross ARCH=sh
All errors (new ones prefixed by >>):
drivers/net//wireless/marvell/libertas/if_usb.c: In function 'if_usb_probe':
>> drivers/net//wireless/marvell/libertas/if_usb.c:259:3: error: 'ret' undeclared (first use in this function); did you mean 'net'?
ret = PTR_ERR(priv);
^~~
net
drivers/net//wireless/marvell/libertas/if_usb.c:259:3: note: each undeclared identifier is reported only once for each function it appears in
--
drivers/net//wireless/marvell/libertas/if_spi.c: In function 'if_spi_probe':
>> drivers/net//wireless/marvell/libertas/if_spi.c:1150:3: error: 'ret' undeclared (first use in this function); did you mean 'net'?
ret = PTR_ERR(priv);
^~~
net
drivers/net//wireless/marvell/libertas/if_spi.c:1150:3: note: each undeclared identifier is reported only once for each function it appears in
vim +259 drivers/net//wireless/marvell/libertas/if_usb.c
184
185 /**
186 * if_usb_probe - sets the configuration values
187 * @intf: &usb_interface pointer
188 * @id: pointer to usb_device_id
189 * returns: 0 on success, error code on failure
190 */
191 static int if_usb_probe(struct usb_interface *intf,
192 const struct usb_device_id *id)
193 {
194 struct usb_device *udev;
195 struct usb_host_interface *iface_desc;
196 struct usb_endpoint_descriptor *endpoint;
197 struct lbs_private *priv;
198 struct if_usb_card *cardp;
199 int r = -ENOMEM;
200 int i;
201
202 udev = interface_to_usbdev(intf);
203
204 cardp = kzalloc(sizeof(struct if_usb_card), GFP_KERNEL);
205 if (!cardp)
206 goto error;
207
208 timer_setup(&cardp->fw_timeout, if_usb_fw_timeo, 0);
209 init_waitqueue_head(&cardp->fw_wq);
210
211 cardp->udev = udev;
212 cardp->model = (uint32_t) id->driver_info;
213 iface_desc = intf->cur_altsetting;
214
215 lbs_deb_usbd(&udev->dev, "bcdUSB = 0x%X bDeviceClass = 0x%X"
216 " bDeviceSubClass = 0x%X, bDeviceProtocol = 0x%X\n",
217 le16_to_cpu(udev->descriptor.bcdUSB),
218 udev->descriptor.bDeviceClass,
219 udev->descriptor.bDeviceSubClass,
220 udev->descriptor.bDeviceProtocol);
221
222 for (i = 0; i < iface_desc->desc.bNumEndpoints; ++i) {
223 endpoint = &iface_desc->endpoint[i].desc;
224 if (usb_endpoint_is_bulk_in(endpoint)) {
225 cardp->ep_in_size = le16_to_cpu(endpoint->wMaxPacketSize);
226 cardp->ep_in = usb_endpoint_num(endpoint);
227
228 lbs_deb_usbd(&udev->dev, "in_endpoint = %d\n", cardp->ep_in);
229 lbs_deb_usbd(&udev->dev, "Bulk in size is %d\n", cardp->ep_in_size);
230
231 } else if (usb_endpoint_is_bulk_out(endpoint)) {
232 cardp->ep_out_size = le16_to_cpu(endpoint->wMaxPacketSize);
233 cardp->ep_out = usb_endpoint_num(endpoint);
234
235 lbs_deb_usbd(&udev->dev, "out_endpoint = %d\n", cardp->ep_out);
236 lbs_deb_usbd(&udev->dev, "Bulk out size is %d\n", cardp->ep_out_size);
237 }
238 }
239 if (!cardp->ep_out_size || !cardp->ep_in_size) {
240 lbs_deb_usbd(&udev->dev, "Endpoints not found\n");
241 goto dealloc;
242 }
243 if (!(cardp->rx_urb = usb_alloc_urb(0, GFP_KERNEL))) {
244 lbs_deb_usbd(&udev->dev, "Rx URB allocation failed\n");
245 goto dealloc;
246 }
247 if (!(cardp->tx_urb = usb_alloc_urb(0, GFP_KERNEL))) {
248 lbs_deb_usbd(&udev->dev, "Tx URB allocation failed\n");
249 goto dealloc;
250 }
251 cardp->ep_out_buf = kmalloc(MRVDRV_ETH_TX_PACKET_BUFFER_SIZE, GFP_KERNEL);
252 if (!cardp->ep_out_buf) {
253 lbs_deb_usbd(&udev->dev, "Could not allocate buffer\n");
254 goto dealloc;
255 }
256
257 priv = lbs_add_card(cardp, &intf->dev);
258 if (IS_ERR(priv)) {
> 259 ret = PTR_ERR(priv);
260 goto err_add_card;
261 }
262
263 cardp->priv = priv;
264
265 priv->hw_host_to_card = if_usb_host_to_card;
266 priv->enter_deep_sleep = NULL;
267 priv->exit_deep_sleep = NULL;
268 priv->reset_deep_sleep_wakeup = NULL;
269 priv->is_polling = false;
270 #ifdef CONFIG_OLPC
271 if (machine_is_olpc())
272 priv->reset_card = if_usb_reset_olpc_card;
273 #endif
274
275 cardp->boot2_version = udev->descriptor.bcdDevice;
276
277 usb_get_dev(udev);
278 usb_set_intfdata(intf, cardp);
279
280 r = lbs_get_firmware_async(priv, &udev->dev, cardp->model,
281 fw_table, if_usb_prog_firmware);
282 if (r)
283 goto err_get_fw;
284
285 return 0;
286
287 err_get_fw:
288 lbs_remove_card(priv);
289 err_add_card:
290 if_usb_reset_device(cardp);
291 dealloc:
292 if_usb_free(cardp);
293
294 error:
295 return r;
296 }
297
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi Lubomir,
I love your patch! Yet something to improve:
[auto build test ERROR on wireless-drivers-next/master]
[also build test ERROR on v4.19-rc7 next-20181008]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Lubomir-Rintel/libertas-return-errno-from-lbs_add_card/20180925-105034
base: https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers-next.git master
config: i386-randconfig-a0-10081439 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
drivers/net/wireless/marvell/libertas/if_spi.c: In function 'if_spi_probe':
>> drivers/net/wireless/marvell/libertas/if_spi.c:1150:3: error: 'ret' undeclared (first use in this function)
ret = PTR_ERR(priv);
^
drivers/net/wireless/marvell/libertas/if_spi.c:1150:3: note: each undeclared identifier is reported only once for each function it appears in
vim +/ret +1150 drivers/net/wireless/marvell/libertas/if_spi.c
1103
1104 static int if_spi_probe(struct spi_device *spi)
1105 {
1106 struct if_spi_card *card;
1107 struct lbs_private *priv = NULL;
1108 struct libertas_spi_platform_data *pdata = dev_get_platdata(&spi->dev);
1109 int err = 0;
1110
1111 if (!pdata) {
1112 err = -EINVAL;
1113 goto out;
1114 }
1115
1116 if (pdata->setup) {
1117 err = pdata->setup(spi);
1118 if (err)
1119 goto out;
1120 }
1121
1122 /* Allocate card structure to represent this specific device */
1123 card = kzalloc(sizeof(struct if_spi_card), GFP_KERNEL);
1124 if (!card) {
1125 err = -ENOMEM;
1126 goto teardown;
1127 }
1128 spi_set_drvdata(spi, card);
1129 card->pdata = pdata;
1130 card->spi = spi;
1131 card->prev_xfer_time = jiffies;
1132
1133 INIT_LIST_HEAD(&card->cmd_packet_list);
1134 INIT_LIST_HEAD(&card->data_packet_list);
1135 spin_lock_init(&card->buffer_lock);
1136
1137 /* Initialize the SPI Interface Unit */
1138
1139 /* Firmware load */
1140 err = if_spi_init_card(card);
1141 if (err)
1142 goto free_card;
1143
1144 /*
1145 * Register our card with libertas.
1146 * This will call alloc_etherdev.
1147 */
1148 priv = lbs_add_card(card, &spi->dev);
1149 if (IS_ERR(priv)) {
> 1150 ret = PTR_ERR(priv);
1151 goto free_card;
1152 }
1153 card->priv = priv;
1154 priv->setup_fw_on_resume = 1;
1155 priv->card = card;
1156 priv->hw_host_to_card = if_spi_host_to_card;
1157 priv->enter_deep_sleep = NULL;
1158 priv->exit_deep_sleep = NULL;
1159 priv->reset_deep_sleep_wakeup = NULL;
1160 priv->fw_ready = 1;
1161
1162 /* Initialize interrupt handling stuff. */
1163 card->workqueue = alloc_workqueue("libertas_spi", WQ_MEM_RECLAIM, 0);
1164 if (!card->workqueue) {
1165 err = -ENOMEM;
1166 goto remove_card;
1167 }
1168 INIT_WORK(&card->packet_work, if_spi_host_to_card_worker);
1169 INIT_WORK(&card->resume_work, if_spi_resume_worker);
1170
1171 err = request_irq(spi->irq, if_spi_host_interrupt,
1172 IRQF_TRIGGER_FALLING, "libertas_spi", card);
1173 if (err) {
1174 pr_err("can't get host irq line-- request_irq failed\n");
1175 goto terminate_workqueue;
1176 }
1177
1178 /*
1179 * Start the card.
1180 * This will call register_netdev, and we'll start
1181 * getting interrupts...
1182 */
1183 err = lbs_start_card(priv);
1184 if (err)
1185 goto release_irq;
1186
1187 lbs_deb_spi("Finished initializing WLAN module.\n");
1188
1189 /* successful exit */
1190 goto out;
1191
1192 release_irq:
1193 free_irq(spi->irq, card);
1194 terminate_workqueue:
1195 destroy_workqueue(card->workqueue);
1196 remove_card:
1197 lbs_remove_card(priv); /* will call free_netdev */
1198 free_card:
1199 free_if_spi_card(card);
1200 teardown:
1201 if (pdata->teardown)
1202 pdata->teardown(spi);
1203 out:
1204 return err;
1205 }
1206
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
diff --git a/drivers/net/wireless/marvell/libertas/if_cs.c b/drivers/net/wireless/marvell/libertas/if_cs.c index 7d88223f890b..cebf03c6a622 100644 --- a/drivers/net/wireless/marvell/libertas/if_cs.c +++ b/drivers/net/wireless/marvell/libertas/if_cs.c @@ -900,8 +900,8 @@ static int if_cs_probe(struct pcmcia_device *p_dev) /* Make this card known to the libertas driver */ priv = lbs_add_card(card, &p_dev->dev); - if (!priv) { - ret = -ENOMEM; + if (IS_ERR(priv)) { + ret = PTR_ERR(priv); goto out2; } diff --git a/drivers/net/wireless/marvell/libertas/if_sdio.c b/drivers/net/wireless/marvell/libertas/if_sdio.c index 43743c26c071..2fd54a0aeb5a 100644 --- a/drivers/net/wireless/marvell/libertas/if_sdio.c +++ b/drivers/net/wireless/marvell/libertas/if_sdio.c @@ -1206,8 +1206,8 @@ static int if_sdio_probe(struct sdio_func *func, priv = lbs_add_card(card, &func->dev); - if (!priv) { - ret = -ENOMEM; + if (IS_ERR(priv)) { + ret = PTR_ERR(priv); goto free; } diff --git a/drivers/net/wireless/marvell/libertas/if_spi.c b/drivers/net/wireless/marvell/libertas/if_spi.c index e9aec6cb1105..47cee494eb62 100644 --- a/drivers/net/wireless/marvell/libertas/if_spi.c +++ b/drivers/net/wireless/marvell/libertas/if_spi.c @@ -1146,8 +1146,8 @@ static int if_spi_probe(struct spi_device *spi) * This will call alloc_etherdev. */ priv = lbs_add_card(card, &spi->dev); - if (!priv) { - err = -ENOMEM; + if (IS_ERR(priv)) { + ret = PTR_ERR(priv); goto free_card; } card->priv = priv; diff --git a/drivers/net/wireless/marvell/libertas/if_usb.c b/drivers/net/wireless/marvell/libertas/if_usb.c index c67a8e7be310..66dc6aeeadbc 100644 --- a/drivers/net/wireless/marvell/libertas/if_usb.c +++ b/drivers/net/wireless/marvell/libertas/if_usb.c @@ -254,8 +254,11 @@ static int if_usb_probe(struct usb_interface *intf, goto dealloc; } - if (!(priv = lbs_add_card(cardp, &intf->dev))) + priv = lbs_add_card(cardp, &intf->dev); + if (IS_ERR(priv)) { + ret = PTR_ERR(priv); goto err_add_card; + } cardp->priv = priv; diff --git a/drivers/net/wireless/marvell/libertas/main.c b/drivers/net/wireless/marvell/libertas/main.c index f22e1c220cba..f7db60bc7c7f 100644 --- a/drivers/net/wireless/marvell/libertas/main.c +++ b/drivers/net/wireless/marvell/libertas/main.c @@ -907,25 +907,29 @@ struct lbs_private *lbs_add_card(void *card, struct device *dmdev) struct net_device *dev; struct wireless_dev *wdev; struct lbs_private *priv = NULL; + int err; /* Allocate an Ethernet device and register it */ wdev = lbs_cfg_alloc(dmdev); if (IS_ERR(wdev)) { + err = PTR_ERR(wdev); pr_err("cfg80211 init failed\n"); - goto done; + goto err_cfg; } wdev->iftype = NL80211_IFTYPE_STATION; priv = wdev_priv(wdev); priv->wdev = wdev; - if (lbs_init_adapter(priv)) { + err = lbs_init_adapter(priv); + if (err) { pr_err("failed to initialize adapter structure\n"); goto err_wdev; } dev = alloc_netdev(0, "wlan%d", NET_NAME_UNKNOWN, ether_setup); if (!dev) { + err = -ENOMEM; dev_err(dmdev, "no memory for network device instance\n"); goto err_adapter; } @@ -949,6 +953,7 @@ struct lbs_private *lbs_add_card(void *card, struct device *dmdev) init_waitqueue_head(&priv->waitq); priv->main_thread = kthread_run(lbs_thread, dev, "lbs_main"); if (IS_ERR(priv->main_thread)) { + err = PTR_ERR(priv->main_thread); lbs_deb_thread("Error creating main thread.\n"); goto err_ndev; } @@ -961,7 +966,7 @@ struct lbs_private *lbs_add_card(void *card, struct device *dmdev) priv->wol_gap = 20; priv->ehs_remove_supported = true; - goto done; + return priv; err_ndev: free_netdev(dev); @@ -972,10 +977,8 @@ struct lbs_private *lbs_add_card(void *card, struct device *dmdev) err_wdev: lbs_cfg_free(priv); - priv = NULL; - -done: - return priv; + err_cfg: + return ERR_PTR(err); } EXPORT_SYMBOL_GPL(lbs_add_card);
Makes lbs_add_card() longer throw away the errno and lets its callers propagate it instead. Basicaly a cosmetical thing. Used to be a part of a patchset that I discarded, but it still make sense to apply this one, because it makes the error handling somewhat cleaner. Signed-off-by: Lubomir Rintel <lkundrak@v3.sk> --- drivers/net/wireless/marvell/libertas/if_cs.c | 4 ++-- drivers/net/wireless/marvell/libertas/if_sdio.c | 4 ++-- drivers/net/wireless/marvell/libertas/if_spi.c | 4 ++-- drivers/net/wireless/marvell/libertas/if_usb.c | 5 ++++- drivers/net/wireless/marvell/libertas/main.c | 17 ++++++++++------- 5 files changed, 20 insertions(+), 14 deletions(-)