diff mbox series

[PATCHv2,net-next,8/8] net: ibm: emac: remove all waiting code

Message ID 20240903194312.12718-9-rosenp@gmail.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series [PATCHv2,net-next,1/8] net: ibm: emac: use devm for alloc_etherdev | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 7 this patch: 7
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 5 of 5 maintainers
netdev/build_clang success Errors and warnings before: 7 this patch: 7
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 7 this patch: 7
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 143 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2024-09-04--15-00 (tests: 718)

Commit Message

Rosen Penev Sept. 3, 2024, 7:42 p.m. UTC
EPROBE_DEFER, which probably wasn't available when this driver was
written, can be used instead of waiting manually.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/net/ethernet/ibm/emac/core.c | 77 ++++++++--------------------
 1 file changed, 20 insertions(+), 57 deletions(-)

Comments

Paolo Abeni Sept. 5, 2024, 10:27 a.m. UTC | #1
On 9/3/24 21:42, Rosen Penev wrote:
[...]
>   static int emac_wait_deps(struct emac_instance *dev)

Possibly rename the function to something else, as it does not wait anymore?

> @@ -2419,26 +2397,25 @@ static int emac_wait_deps(struct emac_instance *dev)
>   		deps[EMAC_DEP_MDIO_IDX].phandle = dev->mdio_ph;
>   	if (dev->blist && dev->blist > emac_boot_list)
>   		deps[EMAC_DEP_PREV_IDX].phandle = 0xffffffffu;
> -	bus_register_notifier(&platform_bus_type, &emac_of_bus_notifier);
> -	wait_event_timeout(emac_probe_wait,
> -			   emac_check_deps(dev, deps),
> -			   EMAC_PROBE_DEP_TIMEOUT);
> -	bus_unregister_notifier(&platform_bus_type, &emac_of_bus_notifier);
> -	err = emac_check_deps(dev, deps) ? 0 : -ENODEV;
> +
> +	err = emac_check_deps(dev, deps);
> +	if (err)
> +		return err;
> +
>   	for (i = 0; i < EMAC_DEP_COUNT; i++) {
>   		of_node_put(deps[i].node);
> -		if (err)
> -			platform_device_put(deps[i].ofdev);
> -	}
> -	if (err == 0) {
> -		dev->mal_dev = deps[EMAC_DEP_MAL_IDX].ofdev;
> -		dev->zmii_dev = deps[EMAC_DEP_ZMII_IDX].ofdev;
> -		dev->rgmii_dev = deps[EMAC_DEP_RGMII_IDX].ofdev;
> -		dev->tah_dev = deps[EMAC_DEP_TAH_IDX].ofdev;
> -		dev->mdio_dev = deps[EMAC_DEP_MDIO_IDX].ofdev;
> +		platform_device_put(deps[i].ofdev);

I'm likely lost, but AFAICS after the patch, on success 
platform_device_put() is invoked unconditionally on each deps[] entry, 
while before it was called only on failure?!?

/P
diff mbox series

Patch

diff --git a/drivers/net/ethernet/ibm/emac/core.c b/drivers/net/ethernet/ibm/emac/core.c
index 6db76eeb4d9b..21401a8cb32c 100644
--- a/drivers/net/ethernet/ibm/emac/core.c
+++ b/drivers/net/ethernet/ibm/emac/core.c
@@ -32,7 +32,6 @@ 
 #include <linux/ethtool.h>
 #include <linux/mii.h>
 #include <linux/bitops.h>
-#include <linux/workqueue.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
 #include <linux/of_irq.h>
@@ -96,11 +95,6 @@  MODULE_LICENSE("GPL");
 static u32 busy_phy_map;
 static DEFINE_MUTEX(emac_phy_map_lock);
 
-/* This is the wait queue used to wait on any event related to probe, that
- * is discovery of MALs, other EMACs, ZMII/RGMIIs, etc...
- */
-static DECLARE_WAIT_QUEUE_HEAD(emac_probe_wait);
-
 /* Having stable interface names is a doomed idea. However, it would be nice
  * if we didn't have completely random interface names at boot too :-) It's
  * just a matter of making everybody's life easier. Since we are doing
@@ -116,9 +110,6 @@  static DECLARE_WAIT_QUEUE_HEAD(emac_probe_wait);
 #define EMAC_BOOT_LIST_SIZE	4
 static struct device_node *emac_boot_list[EMAC_BOOT_LIST_SIZE];
 
-/* How long should I wait for dependent devices ? */
-#define EMAC_PROBE_DEP_TIMEOUT	(HZ * 5)
-
 /* I don't want to litter system log with timeout errors
  * when we have brain-damaged PHY.
  */
@@ -973,8 +964,6 @@  static void __emac_set_multicast_list(struct emac_instance *dev)
 	 * we need is just to stop RX channel. This seems to work on all
 	 * tested SoCs.                                                --ebs
 	 *
-	 * If we need the full reset, we might just trigger the workqueue
-	 * and do it async... a bit nasty but should work --BenH
 	 */
 	dev->mcast_pending = 0;
 	emac_rx_disable(dev);
@@ -2378,7 +2367,9 @@  static int emac_check_deps(struct emac_instance *dev,
 		if (deps[i].drvdata != NULL)
 			there++;
 	}
-	return there == EMAC_DEP_COUNT;
+	if (there != EMAC_DEP_COUNT)
+		return -EPROBE_DEFER;
+	return 0;
 }
 
 static void emac_put_deps(struct emac_instance *dev)
@@ -2390,19 +2381,6 @@  static void emac_put_deps(struct emac_instance *dev)
 	platform_device_put(dev->tah_dev);
 }
 
-static int emac_of_bus_notify(struct notifier_block *nb, unsigned long action,
-			      void *data)
-{
-	/* We are only intereted in device addition */
-	if (action == BUS_NOTIFY_BOUND_DRIVER)
-		wake_up_all(&emac_probe_wait);
-	return 0;
-}
-
-static struct notifier_block emac_of_bus_notifier = {
-	.notifier_call = emac_of_bus_notify
-};
-
 static int emac_wait_deps(struct emac_instance *dev)
 {
 	struct emac_depentry deps[EMAC_DEP_COUNT];
@@ -2419,26 +2397,25 @@  static int emac_wait_deps(struct emac_instance *dev)
 		deps[EMAC_DEP_MDIO_IDX].phandle = dev->mdio_ph;
 	if (dev->blist && dev->blist > emac_boot_list)
 		deps[EMAC_DEP_PREV_IDX].phandle = 0xffffffffu;
-	bus_register_notifier(&platform_bus_type, &emac_of_bus_notifier);
-	wait_event_timeout(emac_probe_wait,
-			   emac_check_deps(dev, deps),
-			   EMAC_PROBE_DEP_TIMEOUT);
-	bus_unregister_notifier(&platform_bus_type, &emac_of_bus_notifier);
-	err = emac_check_deps(dev, deps) ? 0 : -ENODEV;
+
+	err = emac_check_deps(dev, deps);
+	if (err)
+		return err;
+
 	for (i = 0; i < EMAC_DEP_COUNT; i++) {
 		of_node_put(deps[i].node);
-		if (err)
-			platform_device_put(deps[i].ofdev);
-	}
-	if (err == 0) {
-		dev->mal_dev = deps[EMAC_DEP_MAL_IDX].ofdev;
-		dev->zmii_dev = deps[EMAC_DEP_ZMII_IDX].ofdev;
-		dev->rgmii_dev = deps[EMAC_DEP_RGMII_IDX].ofdev;
-		dev->tah_dev = deps[EMAC_DEP_TAH_IDX].ofdev;
-		dev->mdio_dev = deps[EMAC_DEP_MDIO_IDX].ofdev;
+		platform_device_put(deps[i].ofdev);
 	}
+
+	dev->mal_dev = deps[EMAC_DEP_MAL_IDX].ofdev;
+	dev->zmii_dev = deps[EMAC_DEP_ZMII_IDX].ofdev;
+	dev->rgmii_dev = deps[EMAC_DEP_RGMII_IDX].ofdev;
+	dev->tah_dev = deps[EMAC_DEP_TAH_IDX].ofdev;
+	dev->mdio_dev = deps[EMAC_DEP_MDIO_IDX].ofdev;
+
 	platform_device_put(deps[EMAC_DEP_PREV_IDX].ofdev);
-	return err;
+
+	return 0;
 }
 
 static int emac_read_uint_prop(struct device_node *np, const char *name,
@@ -3084,12 +3061,8 @@  static int emac_probe(struct platform_device *ofdev)
 
 	/* Wait for dependent devices */
 	err = emac_wait_deps(dev);
-	if (err) {
-		printk(KERN_ERR
-		       "%pOF: Timeout waiting for dependent devices\n", np);
-		/*  display more info about what's missing ? */
+	if (err)
 		goto err_irq_unmap;
-	}
 	dev->mal = platform_get_drvdata(dev->mal_dev);
 	if (dev->mdio_dev != NULL)
 		dev->mdio_instance = platform_get_drvdata(dev->mdio_dev);
@@ -3189,10 +3162,6 @@  static int emac_probe(struct platform_device *ofdev)
 	wmb();
 	platform_set_drvdata(ofdev, dev);
 
-	/* There's a new kid in town ! Let's tell everybody */
-	wake_up_all(&emac_probe_wait);
-
-
 	printk(KERN_INFO "%s: EMAC-%d %pOF, MAC %pM\n",
 	       ndev->name, dev->cell_index, np, ndev->dev_addr);
 
@@ -3225,14 +3194,8 @@  static int emac_probe(struct platform_device *ofdev)
 	if (dev->wol_irq)
 		irq_dispose_mapping(dev->wol_irq);
  err_gone:
-	/* if we were on the bootlist, remove us as we won't show up and
-	 * wake up all waiters to notify them in case they were waiting
-	 * on us
-	 */
-	if (blist) {
+	if (blist)
 		*blist = NULL;
-		wake_up_all(&emac_probe_wait);
-	}
 	return err;
 }