libertas: return errno from lbs_add_card()
diff mbox series

Message ID 20180924091853.452055-1-lkundrak@v3.sk
State Changes Requested
Delegated to: Kalle Valo
Headers show
Series
  • libertas: return errno from lbs_add_card()
Related show

Commit Message

Lubomir Rintel Sept. 24, 2018, 9:18 a.m. UTC
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(-)

Comments

kernel test robot Sept. 25, 2018, 4:15 a.m. UTC | #1
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
kernel test robot Oct. 8, 2018, 7:16 p.m. UTC | #2
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

Patch
diff mbox series

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);