Netdev List
 help / color / mirror / Atom feed
* [PATCH 07/22] net: phy: micrel: refactor broadcast disable
From: Johan Hovold @ 2014-11-11 17:37 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: David S. Miller, linux-kernel, netdev, Johan Hovold
In-Reply-To: <1415727460-20417-1-git-send-email-johan@kernel.org>

Refactor and clean up broadcast disable.

Some Micrel PHYs have a broadcast-off bit in the Operation Mode Strap
Override register which can be used to disable PHY address 0 as the
broadcast address, so that it can be used as a unique (non-broadcast)
address on a shared bus.

Note that the KSZPHY_OMSO_RMII_OVERRIDE bit is set by default on
KSZ8021/8031.

Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/net/phy/micrel.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index d962a2866bba..21abe5ade3df 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -184,6 +184,25 @@ static int kszphy_setup_led(struct phy_device *phydev,
 	return rc < 0 ? rc : 0;
 }
 
+/* Disable PHY address 0 as the broadcast address, so that it can be used as a
+ * unique (non-broadcast) address on a shared bus.
+ */
+static int kszphy_broadcast_disable(struct phy_device *phydev)
+{
+	int ret;
+
+	ret = phy_read(phydev, MII_KSZPHY_OMSO);
+	if (ret < 0)
+		goto out;
+
+	ret = phy_write(phydev, MII_KSZPHY_OMSO, ret | KSZPHY_OMSO_B_CAST_OFF);
+out:
+	if (ret)
+		dev_err(&phydev->dev, "failed to disable broadcast address\n");
+
+	return ret;
+}
+
 static int kszphy_config_init(struct phy_device *phydev)
 {
 	return 0;
@@ -197,7 +216,6 @@ static int kszphy_config_init_led8041(struct phy_device *phydev)
 
 static int ksz8021_config_init(struct phy_device *phydev)
 {
-	const u16 val = KSZPHY_OMSO_B_CAST_OFF | KSZPHY_OMSO_RMII_OVERRIDE;
 	int rc;
 
 	rc = kszphy_setup_led(phydev, 0x1f, 4);
@@ -207,7 +225,9 @@ static int ksz8021_config_init(struct phy_device *phydev)
 	rc = ksz_config_flags(phydev);
 	if (rc < 0)
 		return rc;
-	rc = phy_write(phydev, MII_KSZPHY_OMSO, val);
+
+	rc = kszphy_broadcast_disable(phydev);
+
 	return rc < 0 ? rc : 0;
 }
 
-- 
2.0.4

^ permalink raw reply related

* [PATCH 06/22] net: phy: micrel: use BIT macro
From: Johan Hovold @ 2014-11-11 17:37 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: David S. Miller, linux-kernel, netdev, Johan Hovold
In-Reply-To: <1415727460-20417-1-git-send-email-johan@kernel.org>

Use BIT macro for bitmask definitions.

Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/net/phy/micrel.c | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 62ca9613a514..d962a2866bba 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -30,30 +30,30 @@
 
 /* Operation Mode Strap Override */
 #define MII_KSZPHY_OMSO				0x16
-#define KSZPHY_OMSO_B_CAST_OFF			(1 << 9)
-#define KSZPHY_OMSO_RMII_OVERRIDE		(1 << 1)
-#define KSZPHY_OMSO_MII_OVERRIDE		(1 << 0)
+#define KSZPHY_OMSO_B_CAST_OFF			BIT(9)
+#define KSZPHY_OMSO_RMII_OVERRIDE		BIT(1)
+#define KSZPHY_OMSO_MII_OVERRIDE		BIT(0)
 
 /* general Interrupt control/status reg in vendor specific block. */
 #define MII_KSZPHY_INTCS			0x1B
-#define	KSZPHY_INTCS_JABBER			(1 << 15)
-#define	KSZPHY_INTCS_RECEIVE_ERR		(1 << 14)
-#define	KSZPHY_INTCS_PAGE_RECEIVE		(1 << 13)
-#define	KSZPHY_INTCS_PARELLEL			(1 << 12)
-#define	KSZPHY_INTCS_LINK_PARTNER_ACK		(1 << 11)
-#define	KSZPHY_INTCS_LINK_DOWN			(1 << 10)
-#define	KSZPHY_INTCS_REMOTE_FAULT		(1 << 9)
-#define	KSZPHY_INTCS_LINK_UP			(1 << 8)
+#define	KSZPHY_INTCS_JABBER			BIT(15)
+#define	KSZPHY_INTCS_RECEIVE_ERR		BIT(14)
+#define	KSZPHY_INTCS_PAGE_RECEIVE		BIT(13)
+#define	KSZPHY_INTCS_PARELLEL			BIT(12)
+#define	KSZPHY_INTCS_LINK_PARTNER_ACK		BIT(11)
+#define	KSZPHY_INTCS_LINK_DOWN			BIT(10)
+#define	KSZPHY_INTCS_REMOTE_FAULT		BIT(9)
+#define	KSZPHY_INTCS_LINK_UP			BIT(8)
 #define	KSZPHY_INTCS_ALL			(KSZPHY_INTCS_LINK_UP |\
 						KSZPHY_INTCS_LINK_DOWN)
 
 /* general PHY control reg in vendor specific block. */
 #define	MII_KSZPHY_CTRL			0x1F
 /* bitmap of PHY register to set interrupt mode */
-#define KSZPHY_CTRL_INT_ACTIVE_HIGH		(1 << 9)
-#define KSZ9021_CTRL_INT_ACTIVE_HIGH		(1 << 14)
-#define KS8737_CTRL_INT_ACTIVE_HIGH		(1 << 14)
-#define KSZ8051_RMII_50MHZ_CLK			(1 << 7)
+#define KSZPHY_CTRL_INT_ACTIVE_HIGH		BIT(9)
+#define KSZ9021_CTRL_INT_ACTIVE_HIGH		BIT(14)
+#define KS8737_CTRL_INT_ACTIVE_HIGH		BIT(14)
+#define KSZ8051_RMII_50MHZ_CLK			BIT(7)
 
 /* Write/read to/from extended registers */
 #define MII_KSZPHY_EXTREG                       0x0b
@@ -400,8 +400,8 @@ static int ksz9031_config_init(struct phy_device *phydev)
 }
 
 #define KSZ8873MLL_GLOBAL_CONTROL_4	0x06
-#define KSZ8873MLL_GLOBAL_CONTROL_4_DUPLEX	(1 << 6)
-#define KSZ8873MLL_GLOBAL_CONTROL_4_SPEED	(1 << 4)
+#define KSZ8873MLL_GLOBAL_CONTROL_4_DUPLEX	BIT(6)
+#define KSZ8873MLL_GLOBAL_CONTROL_4_SPEED	BIT(4)
 static int ksz8873mll_read_status(struct phy_device *phydev)
 {
 	int regval;
-- 
2.0.4

^ permalink raw reply related

* [PATCH 05/22] net: phy: micrel: fix config_intr error handling
From: Johan Hovold @ 2014-11-11 17:37 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: David S. Miller, linux-kernel, netdev, Johan Hovold
In-Reply-To: <1415727460-20417-1-git-send-email-johan@kernel.org>

Make sure never to update the control register with random data (an
error code) by checking the return value after reading it.

Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/net/phy/micrel.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index bcc6c0ea75fa..62ca9613a514 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -122,6 +122,8 @@ static int kszphy_config_intr(struct phy_device *phydev)
 
 	/* set the interrupt pin active low */
 	temp = phy_read(phydev, MII_KSZPHY_CTRL);
+	if (temp < 0)
+		return temp;
 	temp &= ~KSZPHY_CTRL_INT_ACTIVE_HIGH;
 	phy_write(phydev, MII_KSZPHY_CTRL, temp);
 	rc = kszphy_set_interrupt(phydev);
@@ -134,6 +136,8 @@ static int ksz9021_config_intr(struct phy_device *phydev)
 
 	/* set the interrupt pin active low */
 	temp = phy_read(phydev, MII_KSZPHY_CTRL);
+	if (temp < 0)
+		return temp;
 	temp &= ~KSZ9021_CTRL_INT_ACTIVE_HIGH;
 	phy_write(phydev, MII_KSZPHY_CTRL, temp);
 	rc = kszphy_set_interrupt(phydev);
@@ -146,6 +150,8 @@ static int ks8737_config_intr(struct phy_device *phydev)
 
 	/* set the interrupt pin active low */
 	temp = phy_read(phydev, MII_KSZPHY_CTRL);
+	if (temp < 0)
+		return temp;
 	temp &= ~KS8737_CTRL_INT_ACTIVE_HIGH;
 	phy_write(phydev, MII_KSZPHY_CTRL, temp);
 	rc = kszphy_set_interrupt(phydev);
-- 
2.0.4

^ permalink raw reply related

* [PATCH 04/22] net: phy: replace phy_drivers_register calls
From: Johan Hovold @ 2014-11-11 17:37 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: David S. Miller, linux-kernel, netdev, Johan Hovold
In-Reply-To: <1415727460-20417-1-git-send-email-johan@kernel.org>

Replace module init/exit which only calls phy_drivers_register with
module_phy_driver macro.

Tested using Micrel driver, and otherwise compile-tested only.

Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/net/phy/amd-xgbe-phy.c | 15 +--------------
 drivers/net/phy/at803x.c       | 14 +-------------
 drivers/net/phy/bcm63xx.c      | 15 +--------------
 drivers/net/phy/bcm7xxx.c      | 15 +--------------
 drivers/net/phy/bcm87xx.c      | 14 +-------------
 drivers/net/phy/broadcom.c     | 15 +--------------
 drivers/net/phy/cicada.c       | 15 +--------------
 drivers/net/phy/davicom.c      | 15 +--------------
 drivers/net/phy/icplus.c       | 15 +--------------
 drivers/net/phy/lxt.c          | 15 +--------------
 drivers/net/phy/marvell.c      | 15 +--------------
 drivers/net/phy/micrel.c       | 15 +--------------
 drivers/net/phy/realtek.c      | 13 +------------
 drivers/net/phy/smsc.c         | 14 +-------------
 drivers/net/phy/ste10Xp.c      | 15 +--------------
 drivers/net/phy/vitesse.c      | 14 +-------------
 16 files changed, 16 insertions(+), 218 deletions(-)

diff --git a/drivers/net/phy/amd-xgbe-phy.c b/drivers/net/phy/amd-xgbe-phy.c
index f3230eef41fd..52e7427c0a38 100644
--- a/drivers/net/phy/amd-xgbe-phy.c
+++ b/drivers/net/phy/amd-xgbe-phy.c
@@ -1435,20 +1435,7 @@ static struct phy_driver amd_xgbe_phy_driver[] = {
 	},
 };
 
-static int __init amd_xgbe_phy_init(void)
-{
-	return phy_drivers_register(amd_xgbe_phy_driver,
-				    ARRAY_SIZE(amd_xgbe_phy_driver));
-}
-
-static void __exit amd_xgbe_phy_exit(void)
-{
-	phy_drivers_unregister(amd_xgbe_phy_driver,
-			       ARRAY_SIZE(amd_xgbe_phy_driver));
-}
-
-module_init(amd_xgbe_phy_init);
-module_exit(amd_xgbe_phy_exit);
+module_phy_driver(amd_xgbe_phy_driver);
 
 static struct mdio_device_id __maybe_unused amd_xgbe_phy_ids[] = {
 	{ XGBE_PHY_ID, XGBE_PHY_MASK },
diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index fdc1b418fa6a..f80e19ac6704 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -352,19 +352,7 @@ static struct phy_driver at803x_driver[] = {
 	},
 } };
 
-static int __init atheros_init(void)
-{
-	return phy_drivers_register(at803x_driver,
-				    ARRAY_SIZE(at803x_driver));
-}
-
-static void __exit atheros_exit(void)
-{
-	phy_drivers_unregister(at803x_driver, ARRAY_SIZE(at803x_driver));
-}
-
-module_init(atheros_init);
-module_exit(atheros_exit);
+module_phy_driver(at803x_driver);
 
 static struct mdio_device_id __maybe_unused atheros_tbl[] = {
 	{ ATH8030_PHY_ID, 0xffffffef },
diff --git a/drivers/net/phy/bcm63xx.c b/drivers/net/phy/bcm63xx.c
index ac55b0807853..830ec31f952f 100644
--- a/drivers/net/phy/bcm63xx.c
+++ b/drivers/net/phy/bcm63xx.c
@@ -100,20 +100,7 @@ static struct phy_driver bcm63xx_driver[] = {
 	.driver		= { .owner = THIS_MODULE },
 } };
 
-static int __init bcm63xx_phy_init(void)
-{
-	return phy_drivers_register(bcm63xx_driver,
-		ARRAY_SIZE(bcm63xx_driver));
-}
-
-static void __exit bcm63xx_phy_exit(void)
-{
-	phy_drivers_unregister(bcm63xx_driver,
-		ARRAY_SIZE(bcm63xx_driver));
-}
-
-module_init(bcm63xx_phy_init);
-module_exit(bcm63xx_phy_exit);
+module_phy_driver(bcm63xx_driver);
 
 static struct mdio_device_id __maybe_unused bcm63xx_tbl[] = {
 	{ 0x00406000, 0xfffffc00 },
diff --git a/drivers/net/phy/bcm7xxx.c b/drivers/net/phy/bcm7xxx.c
index fdce1ea28790..f9de20f93cb8 100644
--- a/drivers/net/phy/bcm7xxx.c
+++ b/drivers/net/phy/bcm7xxx.c
@@ -337,20 +337,7 @@ static struct mdio_device_id __maybe_unused bcm7xxx_tbl[] = {
 	{ }
 };
 
-static int __init bcm7xxx_phy_init(void)
-{
-	return phy_drivers_register(bcm7xxx_driver,
-			ARRAY_SIZE(bcm7xxx_driver));
-}
-
-static void __exit bcm7xxx_phy_exit(void)
-{
-	phy_drivers_unregister(bcm7xxx_driver,
-			ARRAY_SIZE(bcm7xxx_driver));
-}
-
-module_init(bcm7xxx_phy_init);
-module_exit(bcm7xxx_phy_exit);
+module_phy_driver(bcm7xxx_driver);
 
 MODULE_DEVICE_TABLE(mdio, bcm7xxx_tbl);
 
diff --git a/drivers/net/phy/bcm87xx.c b/drivers/net/phy/bcm87xx.c
index 799789518e87..1eca20452f03 100644
--- a/drivers/net/phy/bcm87xx.c
+++ b/drivers/net/phy/bcm87xx.c
@@ -216,18 +216,6 @@ static struct phy_driver bcm87xx_driver[] = {
 	.driver		= { .owner = THIS_MODULE },
 } };
 
-static int __init bcm87xx_init(void)
-{
-	return phy_drivers_register(bcm87xx_driver,
-		ARRAY_SIZE(bcm87xx_driver));
-}
-module_init(bcm87xx_init);
-
-static void __exit bcm87xx_exit(void)
-{
-	phy_drivers_unregister(bcm87xx_driver,
-		ARRAY_SIZE(bcm87xx_driver));
-}
-module_exit(bcm87xx_exit);
+module_phy_driver(bcm87xx_driver);
 
 MODULE_LICENSE("GPL");
diff --git a/drivers/net/phy/broadcom.c b/drivers/net/phy/broadcom.c
index 34088d60da74..4e2abfb8552c 100644
--- a/drivers/net/phy/broadcom.c
+++ b/drivers/net/phy/broadcom.c
@@ -776,20 +776,7 @@ static struct phy_driver broadcom_drivers[] = {
 	.driver		= { .owner = THIS_MODULE },
 } };
 
-static int __init broadcom_init(void)
-{
-	return phy_drivers_register(broadcom_drivers,
-		ARRAY_SIZE(broadcom_drivers));
-}
-
-static void __exit broadcom_exit(void)
-{
-	phy_drivers_unregister(broadcom_drivers,
-		ARRAY_SIZE(broadcom_drivers));
-}
-
-module_init(broadcom_init);
-module_exit(broadcom_exit);
+module_phy_driver(broadcom_drivers);
 
 static struct mdio_device_id __maybe_unused broadcom_tbl[] = {
 	{ PHY_ID_BCM5411, 0xfffffff0 },
diff --git a/drivers/net/phy/cicada.c b/drivers/net/phy/cicada.c
index b57ce0cc9657..27f5464899d4 100644
--- a/drivers/net/phy/cicada.c
+++ b/drivers/net/phy/cicada.c
@@ -129,20 +129,7 @@ static struct phy_driver cis820x_driver[] = {
 	.driver		= { .owner = THIS_MODULE,},
 } };
 
-static int __init cicada_init(void)
-{
-	return phy_drivers_register(cis820x_driver,
-		ARRAY_SIZE(cis820x_driver));
-}
-
-static void __exit cicada_exit(void)
-{
-	phy_drivers_unregister(cis820x_driver,
-		ARRAY_SIZE(cis820x_driver));
-}
-
-module_init(cicada_init);
-module_exit(cicada_exit);
+module_phy_driver(cis820x_driver);
 
 static struct mdio_device_id __maybe_unused cicada_tbl[] = {
 	{ 0x000fc410, 0x000ffff0 },
diff --git a/drivers/net/phy/davicom.c b/drivers/net/phy/davicom.c
index d2c08f625a41..0d16c7d9e1bf 100644
--- a/drivers/net/phy/davicom.c
+++ b/drivers/net/phy/davicom.c
@@ -182,20 +182,7 @@ static struct phy_driver dm91xx_driver[] = {
 	.driver		= { .owner = THIS_MODULE,},
 } };
 
-static int __init davicom_init(void)
-{
-	return phy_drivers_register(dm91xx_driver,
-		ARRAY_SIZE(dm91xx_driver));
-}
-
-static void __exit davicom_exit(void)
-{
-	phy_drivers_unregister(dm91xx_driver,
-		ARRAY_SIZE(dm91xx_driver));
-}
-
-module_init(davicom_init);
-module_exit(davicom_exit);
+module_phy_driver(dm91xx_driver);
 
 static struct mdio_device_id __maybe_unused davicom_tbl[] = {
 	{ 0x0181b880, 0x0ffffff0 },
diff --git a/drivers/net/phy/icplus.c b/drivers/net/phy/icplus.c
index 97bf58bf4939..8644f039d922 100644
--- a/drivers/net/phy/icplus.c
+++ b/drivers/net/phy/icplus.c
@@ -253,20 +253,7 @@ static struct phy_driver icplus_driver[] = {
 	.driver		= { .owner = THIS_MODULE,},
 } };
 
-static int __init icplus_init(void)
-{
-	return phy_drivers_register(icplus_driver,
-		ARRAY_SIZE(icplus_driver));
-}
-
-static void __exit icplus_exit(void)
-{
-	phy_drivers_unregister(icplus_driver,
-		ARRAY_SIZE(icplus_driver));
-}
-
-module_init(icplus_init);
-module_exit(icplus_exit);
+module_phy_driver(icplus_driver);
 
 static struct mdio_device_id __maybe_unused icplus_tbl[] = {
 	{ 0x02430d80, 0x0ffffff0 },
diff --git a/drivers/net/phy/lxt.c b/drivers/net/phy/lxt.c
index 9108f3191701..a3a5a703635b 100644
--- a/drivers/net/phy/lxt.c
+++ b/drivers/net/phy/lxt.c
@@ -312,20 +312,7 @@ static struct phy_driver lxt97x_driver[] = {
 	.driver		= { .owner = THIS_MODULE,},
 } };
 
-static int __init lxt_init(void)
-{
-	return phy_drivers_register(lxt97x_driver,
-		ARRAY_SIZE(lxt97x_driver));
-}
-
-static void __exit lxt_exit(void)
-{
-	phy_drivers_unregister(lxt97x_driver,
-		ARRAY_SIZE(lxt97x_driver));
-}
-
-module_init(lxt_init);
-module_exit(lxt_exit);
+module_phy_driver(lxt97x_driver);
 
 static struct mdio_device_id __maybe_unused lxt_tbl[] = {
 	{ 0x78100000, 0xfffffff0 },
diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index bd37e45c89c0..708facd78612 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -1052,20 +1052,7 @@ static struct phy_driver marvell_drivers[] = {
 	},
 };
 
-static int __init marvell_init(void)
-{
-	return phy_drivers_register(marvell_drivers,
-		 ARRAY_SIZE(marvell_drivers));
-}
-
-static void __exit marvell_exit(void)
-{
-	phy_drivers_unregister(marvell_drivers,
-		 ARRAY_SIZE(marvell_drivers));
-}
-
-module_init(marvell_init);
-module_exit(marvell_exit);
+module_phy_driver(marvell_drivers);
 
 static struct mdio_device_id __maybe_unused marvell_tbl[] = {
 	{ MARVELL_PHY_ID_88E1101, MARVELL_PHY_ID_MASK },
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 8c2a29a9bd7f..bcc6c0ea75fa 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -657,20 +657,7 @@ static struct phy_driver ksphy_driver[] = {
 	.driver		= { .owner = THIS_MODULE, },
 } };
 
-static int __init ksphy_init(void)
-{
-	return phy_drivers_register(ksphy_driver,
-		ARRAY_SIZE(ksphy_driver));
-}
-
-static void __exit ksphy_exit(void)
-{
-	phy_drivers_unregister(ksphy_driver,
-		ARRAY_SIZE(ksphy_driver));
-}
-
-module_init(ksphy_init);
-module_exit(ksphy_exit);
+module_phy_driver(ksphy_driver);
 
 MODULE_DESCRIPTION("Micrel PHY driver");
 MODULE_AUTHOR("David J. Choi");
diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index 45483fdfbe06..96a0f0fab3ca 100644
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c
@@ -101,18 +101,7 @@ static struct phy_driver realtek_drvs[] = {
 	},
 };
 
-static int __init realtek_init(void)
-{
-	return phy_drivers_register(realtek_drvs, ARRAY_SIZE(realtek_drvs));
-}
-
-static void __exit realtek_exit(void)
-{
-	phy_drivers_unregister(realtek_drvs, ARRAY_SIZE(realtek_drvs));
-}
-
-module_init(realtek_init);
-module_exit(realtek_exit);
+module_phy_driver(realtek_drvs);
 
 static struct mdio_device_id __maybe_unused realtek_tbl[] = {
 	{ 0x001cc912, 0x001fffff },
diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
index a4b08198fb9f..c0f6479e19d4 100644
--- a/drivers/net/phy/smsc.c
+++ b/drivers/net/phy/smsc.c
@@ -250,24 +250,12 @@ static struct phy_driver smsc_phy_driver[] = {
 	.driver		= { .owner = THIS_MODULE, }
 } };
 
-static int __init smsc_init(void)
-{
-	return phy_drivers_register(smsc_phy_driver,
-		ARRAY_SIZE(smsc_phy_driver));
-}
-
-static void __exit smsc_exit(void)
-{
-	phy_drivers_unregister(smsc_phy_driver, ARRAY_SIZE(smsc_phy_driver));
-}
+module_phy_driver(smsc_phy_driver);
 
 MODULE_DESCRIPTION("SMSC PHY driver");
 MODULE_AUTHOR("Herbert Valerio Riedel");
 MODULE_LICENSE("GPL");
 
-module_init(smsc_init);
-module_exit(smsc_exit);
-
 static struct mdio_device_id __maybe_unused smsc_tbl[] = {
 	{ 0x0007c0a0, 0xfffffff0 },
 	{ 0x0007c0b0, 0xfffffff0 },
diff --git a/drivers/net/phy/ste10Xp.c b/drivers/net/phy/ste10Xp.c
index 5e1eb138916f..3fc199b773e6 100644
--- a/drivers/net/phy/ste10Xp.c
+++ b/drivers/net/phy/ste10Xp.c
@@ -112,20 +112,7 @@ static struct phy_driver ste10xp_pdriver[] = {
 	.driver = {.owner = THIS_MODULE,}
 } };
 
-static int __init ste10Xp_init(void)
-{
-	return phy_drivers_register(ste10xp_pdriver,
-		ARRAY_SIZE(ste10xp_pdriver));
-}
-
-static void __exit ste10Xp_exit(void)
-{
-	phy_drivers_unregister(ste10xp_pdriver,
-		ARRAY_SIZE(ste10xp_pdriver));
-}
-
-module_init(ste10Xp_init);
-module_exit(ste10Xp_exit);
+module_phy_driver(ste10xp_pdriver);
 
 static struct mdio_device_id __maybe_unused ste10Xp_tbl[] = {
 	{ STE101P_PHY_ID, 0xfffffff0 },
diff --git a/drivers/net/phy/vitesse.c b/drivers/net/phy/vitesse.c
index 5dc0935da99c..76cad712ddb2 100644
--- a/drivers/net/phy/vitesse.c
+++ b/drivers/net/phy/vitesse.c
@@ -311,19 +311,7 @@ static struct phy_driver vsc82xx_driver[] = {
 	.driver		= { .owner = THIS_MODULE,},
 } };
 
-static int __init vsc82xx_init(void)
-{
-	return phy_drivers_register(vsc82xx_driver,
-		ARRAY_SIZE(vsc82xx_driver));
-}
-
-static void __exit vsc82xx_exit(void)
-{
-	phy_drivers_unregister(vsc82xx_driver, ARRAY_SIZE(vsc82xx_driver));
-}
-
-module_init(vsc82xx_init);
-module_exit(vsc82xx_exit);
+module_phy_driver(vsc82xx_driver);
 
 static struct mdio_device_id __maybe_unused vitesse_tbl[] = {
 	{ PHY_ID_VSC8234, 0x000ffff0 },
-- 
2.0.4

^ permalink raw reply related

* [PATCH 03/22] net: phy: replace phy_driver_register calls
From: Johan Hovold @ 2014-11-11 17:37 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: David S. Miller, linux-kernel, netdev, Johan Hovold
In-Reply-To: <1415727460-20417-1-git-send-email-johan@kernel.org>

Replace module init/exit which only calls phy_driver_register with
module_phy_driver macro.

Compile tested only.

Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/net/phy/amd.c      | 17 +++--------------
 drivers/net/phy/et1011c.c  | 17 +++--------------
 drivers/net/phy/national.c | 17 +++--------------
 drivers/net/phy/qsemi.c    | 17 +++--------------
 4 files changed, 12 insertions(+), 56 deletions(-)

diff --git a/drivers/net/phy/amd.c b/drivers/net/phy/amd.c
index a3fb5ceb6487..65a488f82eb8 100644
--- a/drivers/net/phy/amd.c
+++ b/drivers/net/phy/amd.c
@@ -61,7 +61,7 @@ static int am79c_config_intr(struct phy_device *phydev)
 	return err;
 }
 
-static struct phy_driver am79c_driver = {
+static struct phy_driver am79c_driver[] = { {
 	.phy_id		= PHY_ID_AM79C874,
 	.name		= "AM79C874",
 	.phy_id_mask	= 0xfffffff0,
@@ -73,20 +73,9 @@ static struct phy_driver am79c_driver = {
 	.ack_interrupt	= am79c_ack_interrupt,
 	.config_intr	= am79c_config_intr,
 	.driver		= { .owner = THIS_MODULE,},
-};
-
-static int __init am79c_init(void)
-{
-	return phy_driver_register(&am79c_driver);
-}
-
-static void __exit am79c_exit(void)
-{
-	phy_driver_unregister(&am79c_driver);
-}
+} };
 
-module_init(am79c_init);
-module_exit(am79c_exit);
+module_phy_driver(am79c_driver);
 
 static struct mdio_device_id __maybe_unused amd_tbl[] = {
 	{ PHY_ID_AM79C874, 0xfffffff0 },
diff --git a/drivers/net/phy/et1011c.c b/drivers/net/phy/et1011c.c
index a8eb19ec3183..a907743816a8 100644
--- a/drivers/net/phy/et1011c.c
+++ b/drivers/net/phy/et1011c.c
@@ -87,7 +87,7 @@ static int et1011c_read_status(struct phy_device *phydev)
 	return ret;
 }
 
-static struct phy_driver et1011c_driver = {
+static struct phy_driver et1011c_driver[] = { {
 	.phy_id		= 0x0282f014,
 	.name		= "ET1011C",
 	.phy_id_mask	= 0xfffffff0,
@@ -96,20 +96,9 @@ static struct phy_driver et1011c_driver = {
 	.config_aneg	= et1011c_config_aneg,
 	.read_status	= et1011c_read_status,
 	.driver 	= { .owner = THIS_MODULE,},
-};
-
-static int __init et1011c_init(void)
-{
-	return phy_driver_register(&et1011c_driver);
-}
-
-static void __exit et1011c_exit(void)
-{
-	phy_driver_unregister(&et1011c_driver);
-}
+} };
 
-module_init(et1011c_init);
-module_exit(et1011c_exit);
+module_phy_driver(et1011c_driver);
 
 static struct mdio_device_id __maybe_unused et1011c_tbl[] = {
 	{ 0x0282f014, 0xfffffff0 },
diff --git a/drivers/net/phy/national.c b/drivers/net/phy/national.c
index 9a5f234d95b0..0a7b9c7f09a2 100644
--- a/drivers/net/phy/national.c
+++ b/drivers/net/phy/national.c
@@ -129,7 +129,7 @@ static int ns_config_init(struct phy_device *phydev)
 	return ns_ack_interrupt(phydev);
 }
 
-static struct phy_driver dp83865_driver = {
+static struct phy_driver dp83865_driver[] = { {
 	.phy_id = DP83865_PHY_ID,
 	.phy_id_mask = 0xfffffff0,
 	.name = "NatSemi DP83865",
@@ -141,25 +141,14 @@ static struct phy_driver dp83865_driver = {
 	.ack_interrupt = ns_ack_interrupt,
 	.config_intr = ns_config_intr,
 	.driver = {.owner = THIS_MODULE,}
-};
+} };
 
-static int __init ns_init(void)
-{
-	return phy_driver_register(&dp83865_driver);
-}
-
-static void __exit ns_exit(void)
-{
-	phy_driver_unregister(&dp83865_driver);
-}
+module_phy_driver(dp83865_driver);
 
 MODULE_DESCRIPTION("NatSemi PHY driver");
 MODULE_AUTHOR("Stuart Menefy");
 MODULE_LICENSE("GPL");
 
-module_init(ns_init);
-module_exit(ns_exit);
-
 static struct mdio_device_id __maybe_unused ns_tbl[] = {
 	{ DP83865_PHY_ID, 0xfffffff0 },
 	{ }
diff --git a/drivers/net/phy/qsemi.c b/drivers/net/phy/qsemi.c
index fe0d0a15d5e1..be4c6f7c3645 100644
--- a/drivers/net/phy/qsemi.c
+++ b/drivers/net/phy/qsemi.c
@@ -111,7 +111,7 @@ static int qs6612_config_intr(struct phy_device *phydev)
 
 }
 
-static struct phy_driver qs6612_driver = {
+static struct phy_driver qs6612_driver[] = { {
 	.phy_id		= 0x00181440,
 	.name		= "QS6612",
 	.phy_id_mask	= 0xfffffff0,
@@ -123,20 +123,9 @@ static struct phy_driver qs6612_driver = {
 	.ack_interrupt	= qs6612_ack_interrupt,
 	.config_intr	= qs6612_config_intr,
 	.driver 	= { .owner = THIS_MODULE,},
-};
-
-static int __init qs6612_init(void)
-{
-	return phy_driver_register(&qs6612_driver);
-}
-
-static void __exit qs6612_exit(void)
-{
-	phy_driver_unregister(&qs6612_driver);
-}
+} };
 
-module_init(qs6612_init);
-module_exit(qs6612_exit);
+module_phy_driver(qs6612_driver);
 
 static struct mdio_device_id __maybe_unused qs6612_tbl[] = {
 	{ 0x00181440, 0xfffffff0 },
-- 
2.0.4

^ permalink raw reply related

* [PATCH 02/22] net: phy: add module_phy_driver macro
From: Johan Hovold @ 2014-11-11 17:37 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: David S. Miller, linux-kernel, netdev, Johan Hovold
In-Reply-To: <1415727460-20417-1-git-send-email-johan@kernel.org>

Add helper macro for PHY drivers which do not do anything special in
module init/exit. This will allow us to eliminate a lot of boilerplate
code.

Signed-off-by: Johan Hovold <johan@kernel.org>
---
 include/linux/phy.h | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/include/linux/phy.h b/include/linux/phy.h
index ed39956b5613..c50e1f1f46e0 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -745,4 +745,28 @@ int __init mdio_bus_init(void);
 void mdio_bus_exit(void);
 
 extern struct bus_type mdio_bus_type;
+
+/**
+ * module_phy_driver() - Helper macro for registering PHY drivers
+ * @__phy_drivers: array of PHY drivers to register
+ *
+ * Helper macro for PHY drivers which do not do anything special in module
+ * init/exit. Each module may only use this macro once, and calling it
+ * replaces module_init() and module_exit().
+ */
+#define phy_module_driver(__phy_drivers, __count)			\
+static int __init phy_module_init(void)					\
+{									\
+	return phy_drivers_register(__phy_drivers, __count);		\
+}									\
+module_init(phy_module_init);						\
+static void __exit phy_module_exit(void)				\
+{									\
+	phy_drivers_unregister(__phy_drivers, __count);			\
+}									\
+module_exit(phy_module_exit)
+
+#define module_phy_driver(__phy_drivers)				\
+	phy_module_driver(__phy_drivers, ARRAY_SIZE(__phy_drivers))
+
 #endif /* __PHY_H */
-- 
2.0.4

^ permalink raw reply related

* [PATCH 01/22] dt/bindings: fix documentation of ethernet-phy compatible property
From: Johan Hovold @ 2014-11-11 17:37 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: David S. Miller, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA, Johan Hovold,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1415727460-20417-1-git-send-email-johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

A recent commit extended the documentation of the ethernet-phy
compatible property, but placed the new paragraph under the max-speed
property.

Fixes: f00e756ed12d ("dt: Document a compatible entry for MDIO ethernet
Phys")

Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Signed-off-by: Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 Documentation/devicetree/bindings/net/phy.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/phy.txt b/Documentation/devicetree/bindings/net/phy.txt
index 5b8c58903077..40831fbaff72 100644
--- a/Documentation/devicetree/bindings/net/phy.txt
+++ b/Documentation/devicetree/bindings/net/phy.txt
@@ -19,7 +19,6 @@ Optional Properties:
   specifications. If neither of these are specified, the default is to
   assume clause 22. The compatible list may also contain other
   elements.
-- max-speed: Maximum PHY supported speed (10, 100, 1000...)
 
   If the phy's identifier is known then the list may contain an entry
   of the form: "ethernet-phy-idAAAA.BBBB" where
@@ -29,6 +28,8 @@ Optional Properties:
             4 hex digits. This is the chip vendor OUI bits 19:24,
             followed by 10 bits of a vendor specific ID.
 
+- max-speed: Maximum PHY supported speed (10, 100, 1000...)
+
 Example:
 
 ethernet-phy@0 {
-- 
2.0.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH 00/22] net: phy: refactoring and Micrel features
From: Johan Hovold @ 2014-11-11 17:37 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: David S. Miller, linux-kernel, netdev, Johan Hovold,
	Bruno Thomsen

This series fixes a couple of minor issues, refactors phy-driver
registration, adds device and device-type abstractions to the micrel
driver, and adds support for broadcast disable, led-mode and
RMII-reference clock selection for KSZ8081 and KSZ8091 devices.

While adding support for more features for the Micrel PHYs mentioned
above, it became apparent that the configuration space is much too large
and that adding type-specific callbacks will simply not scale. Instead I
added a driver_data field to struct phy_device, which can be used to
store static device type data that can be parsed and acted on in
generic driver callbacks instead. This allows a lot of duplicated code
to be removed, and should make it much easier to add new features or
deal with device-type quirks in the future.

This series has been tested on a dual KSZ8081 setup (which wasn't
supported until now). Further testing on other Micrel PHYs would be much
appreciated.

Note that the final patch effectively reverts the recent commit
a95a18afe4c8 ("phy/micrel: KSZ8031RNL RMII clock reconfiguration bug"),
which claims that some KSZ8031 PHY  become unresponsive if the broadcast
disable flag is set before configuring the clock mode, even though
disabling the broadcast address is generally the first thing you want to
do (e.g. to avoid unnecessary reconfigurations).

Hopefully, the author of that commit (on CC) can confirm whether this is
indeed the case, and if so the final patch can be dropped unless it can
be worked around in a different way (which I suspect).

Thanks,
Johan


Johan Hovold (22):
  dt/bindings: fix documentation of ethernet-phy compatible property
  net: phy: add module_phy_driver macro
  net: phy: replace phy_driver_register calls
  net: phy: replace phy_drivers_register calls
  net: phy: micrel: fix config_intr error handling
  net: phy: micrel: use BIT macro
  net: phy: micrel: refactor broadcast disable
  net: phy: micrel: disable broadcast for KSZ8081/KSZ8091
  net: phy: micrel: add led-mode sanity check
  net: phy: micrel: refactor led-mode error handling
  net: phy: micrel: clean up led-mode setup
  net: phy: micrel: enable led-mode for KSZ8081/KSZ8091
  net: phy: add static data field to struct phy_driver
  net: phy: micrel: add device-type abstraction
  net: phy: micrel: parse of nodes at probe
  net: phy: micrel: add has-broadcast-disable flag to type data
  net: phy: micrel: add generic rmii-ref-clk-sel support
  net: phy: micrel: add support for rmii_ref_clk_sel to KSZ8081/KSZ8091
  dt/bindings: add micrel,rmii_ref_clk_sel_25_mhz to eth-phy binding
  net: phy: micrel: refactor interrupt config
  net: phy: micrel: add copyright entry
  net: phy: micrel: use generic config_init for KSZ8021/KSZ8031

 Documentation/devicetree/bindings/net/micrel.txt |  11 +-
 Documentation/devicetree/bindings/net/phy.txt    |   3 +-
 drivers/net/phy/amd-xgbe-phy.c                   |  15 +-
 drivers/net/phy/amd.c                            |  17 +-
 drivers/net/phy/at803x.c                         |  14 +-
 drivers/net/phy/bcm63xx.c                        |  15 +-
 drivers/net/phy/bcm7xxx.c                        |  15 +-
 drivers/net/phy/bcm87xx.c                        |  14 +-
 drivers/net/phy/broadcom.c                       |  15 +-
 drivers/net/phy/cicada.c                         |  15 +-
 drivers/net/phy/davicom.c                        |  15 +-
 drivers/net/phy/et1011c.c                        |  17 +-
 drivers/net/phy/icplus.c                         |  15 +-
 drivers/net/phy/lxt.c                            |  15 +-
 drivers/net/phy/marvell.c                        |  15 +-
 drivers/net/phy/micrel.c                         | 346 ++++++++++++++---------
 drivers/net/phy/national.c                       |  17 +-
 drivers/net/phy/qsemi.c                          |  17 +-
 drivers/net/phy/realtek.c                        |  13 +-
 drivers/net/phy/smsc.c                           |  14 +-
 drivers/net/phy/ste10Xp.c                        |  15 +-
 drivers/net/phy/vitesse.c                        |  14 +-
 include/linux/micrel_phy.h                       |   1 -
 include/linux/phy.h                              |  26 ++
 24 files changed, 278 insertions(+), 396 deletions(-)

-- 
2.0.4

^ permalink raw reply

* Re: [PATCH] brcmfmac: unlink URB when request timed out
From: Mathy Vanhoef @ 2014-11-11 17:35 UTC (permalink / raw)
  To: Arend van Spriel, brudley, frankyl, meuleman, linville, pieterpg,
	linux-wireless, brcm80211-dev-list, netdev, Oliver Neukum
In-Reply-To: <5461ED83.7070509@broadcom.com>

Using usb_kill_urb() instead of usb_unlink_urb() seems to work without any problems.

On 11/11/2014 06:05 AM, Arend van Spriel wrote:
> On 10-11-14 17:08, Mathy Vanhoef wrote:
>> On 11/10/2014 06:18 AM, Arend van Spriel wrote:
>>> On 09-11-14 19:10, Mathy Vanhoef wrote:
>>>> From: Mathy Vanhoef <vanhoefm@gmail.com>
>>>>
>>>> Unlink the submitted URB in brcmf_usb_dl_cmd if the request timed out. This
>>>> assures the URB is never submitted twice, preventing a driver crash.
>>>
>>> Hi Mathy,
>>>
>>> What driver crash are you referring to? The log only shows the WARNING
>>> ending in a USB disconnect but no actual crash. Does your patch get the
>>> driver running properly or does it only avoid the warning.
>>
>> Hi Arend,
>>
>> It shows a warning, after which the device doesn't work (but the computer is
>> still usable). But I've noticed that when *unplugging* the USB cable the OS may
>> freeze. This doesn't always happen though, sometimes unplugging works OK. The
>> patch both avoids the warning, and gets the device/driver running properly
>> (unplugging also works OK).
>>
>>>
>>> With that said, it seems there is some need for improvement, but I also
>>> notice you are running this on a virtual machine so could that affect
>>> the timeout to kick in before completion. Could you try to increase the
>>> timeout. Still when a timeout occurs this needs to be handled properly.
>>> Could you also try the following patch?
>>
>> I did a few additional tests:
>>
>> 1. When increasing IOCTL_RESP_TIMEOUT to 20000 (ten times the normal value) the
>>    timeout and warning still occur. Device/driver doesn't work.
>> 2. When increasing BRCMF_USB_RESET_GETVER_SPINWAIT to 1000 (ten timers the
>>    normal value) everything works. Device/driver works.
> 
> This means the ctl_urb completes on your system within 3sec, but not
> within 2.1sec. After discussing this with my colleague, we think you
> should use usb_kill_urb() instead of usb_unlink_urb() as it assures the
> completion handler is called. Could you retest that and let us know.
> 
> Regards,
> Arend
> 
>> 3. Quick test using backports-3.18-rc1-1 (aka unpatched driver) on a non-
>>    virtualized Linux install: In that case everything worked fine. So the bug
>>    may only be triggered in a virtualized environment / VMWare.
>> 4. When applying your patch, the driver stops early during initialization of
>>    the device. I included a WARN_ONCE before returning EINVAL and got the
>>    output below.
>>
>> Kind regards,
>> Mathy
>> ---
>> [  220.955647] usb 1-1: new high-speed USB device number 3 using ehci-pci
>> [  221.487797] usb 1-1: New USB device found, idVendor=043e, idProduct=3004
>> [  221.487802] usb 1-1: New USB device strings: Mfr=1, Product=2, SerialNumber=3
>> [  221.487804] usb 1-1: Product: Remote Download Wireless Adapter
>> [  221.487806] usb 1-1: Manufacturer: Broadcom
>> [  221.487808] usb 1-1: SerialNumber: 000000000001
>> [  221.490472] brcmfmac: brcmf_usb_probe Enter 0x043e:0x3004
>> [  221.490476] brcmfmac: brcmf_usb_probe Broadcom high speed USB WLAN interface detected
>> [  221.490477] brcmfmac: brcmf_usb_probe_cb Enter
>> [  221.490480] brcmfmac: brcmf_usb_attach Enter
>> [  221.490494] brcmfmac: brcmf_usb_dlneeded Enter
>> [  221.490495] ------------[ cut here ]------------
>> [  221.490503] WARNING: CPU: 0 PID: 100 at drivers/net/wireless/brcm80211/brcmfmac/usb.c:716 brcmf_usb_dl_cmd+0x75/0x1a0 [brcmfmac]()
>> [  221.490505] EINVAL devinfo=c0044000 ctl_rub=ef898380 completed=0
>> [  221.490506] Modules linked in: brcmfmac brcmutil vmw_pvscsi pcnet32 mptspi mptscsih mptbase
>> [  221.490514] CPU: 0 PID: 100 Comm: kworker/0:1 Not tainted 3.18.0-rc3-wl+ #2
>> [  221.490515] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 07/31/2013
>> [  221.490528] Workqueue: usb_hub_wq hub_event
>> [  221.490530]  00000000 00000000 eecffb58 c1711f4a eecffb98 eecffb88 c103edaf f11cbc58
>> [  221.490534]  eecffbb4 00000064 f11cbc84 000002cc f11c1595 f11c1595 c0044000 ffffffea
>> [  221.490537]  ef726000 eecffba0 c103ee4e 00000009 eecffb98 f11cbc58 eecffbb4 eecffbd0
>> [  221.490541] Call Trace:
>> [  221.490550]  [<c1711f4a>] dump_stack+0x41/0x52
>> [  221.490558]  [<c103edaf>] warn_slowpath_common+0x7f/0xa0
>> [  221.490563]  [<f11c1595>] ? brcmf_usb_dl_cmd+0x75/0x1a0 [brcmfmac]
>> [  221.490567]  [<f11c1595>] ? brcmf_usb_dl_cmd+0x75/0x1a0 [brcmfmac]
>> [  221.490570]  [<c103ee4e>] warn_slowpath_fmt+0x2e/0x30
>> [  221.490575]  [<f11c1595>] brcmf_usb_dl_cmd+0x75/0x1a0 [brcmfmac]
>> [  221.490580]  [<f11c2cd8>] brcmf_usb_probe+0x3c8/0x640 [brcmfmac]
>> [  221.490583]  [<c1717d53>] ? mutex_lock+0x13/0x32
>> [  221.490586]  [<c1493ae3>] usb_probe_interface+0xa3/0x180
>> [  221.490590]  [<c13f5690>] ? __driver_attach+0x90/0x90
>> [  221.490592]  [<c13f546e>] driver_probe_device+0x5e/0x1f0
>> [  221.490595]  [<c13f5690>] ? __driver_attach+0x90/0x90
>> [  221.490597]  [<c13f56c9>] __device_attach+0x39/0x50
>> [  221.490600]  [<c13f3d84>] bus_for_each_drv+0x34/0x70
>> [  221.490602]  [<c13f53db>] device_attach+0x7b/0x90
>> [  221.490604]  [<c13f5690>] ? __driver_attach+0x90/0x90
>> [  221.490607]  [<c13f4b8f>] bus_probe_device+0x6f/0x90
>> [  221.490609]  [<c13f3256>] device_add+0x426/0x520
>> [  221.490611]  [<c1491503>] ? usb_control_msg+0xb3/0xd0
>> [  221.490614]  [<c1717d53>] ? mutex_lock+0x13/0x32
>> [  221.490627]  [<c14922f8>] usb_set_configuration+0x3f8/0x700
>> [  221.490630]  [<c13f5690>] ? __driver_attach+0x90/0x90
>> [  221.490633]  [<c149ac7b>] generic_probe+0x2b/0x90
>> [  221.490637]  [<c1188bc0>] ? sysfs_create_link+0x20/0x40
>> [  221.490639]  [<c1492bec>] usb_probe_device+0xc/0x10
>> [  221.490641]  [<c13f546e>] driver_probe_device+0x5e/0x1f0
>> [  221.490644]  [<c13f5690>] ? __driver_attach+0x90/0x90
>> [  221.490646]  [<c13f56c9>] __device_attach+0x39/0x50
>> [  221.490649]  [<c13f3d84>] bus_for_each_drv+0x34/0x70
>> [  221.490651]  [<c13f53db>] device_attach+0x7b/0x90
>> [  221.490653]  [<c13f5690>] ? __driver_attach+0x90/0x90
>> [  221.490656]  [<c13f4b8f>] bus_probe_device+0x6f/0x90
>> [  221.490658]  [<c13f3256>] device_add+0x426/0x520
>> [  221.490661]  [<c148aa2e>] ? usb_new_device+0x16e/0x3a0
>> [  221.490663]  [<c148aad7>] usb_new_device+0x217/0x3a0
>> [  221.490666]  [<c148bff7>] hub_event+0xa17/0xda0
>> [  221.490668]  [<c1716918>] ? __schedule+0x2f8/0x710
>> [  221.490672]  [<c105127c>] ? pwq_dec_nr_in_flight+0x3c/0x90
>> [  221.490674]  [<c10513ee>] process_one_work+0x11e/0x360
>> [  221.490677]  [<c1051750>] worker_thread+0xf0/0x3c0
>> [  221.490680]  [<c106e14a>] ? __wake_up_locked+0x1a/0x20
>> [  221.490682]  [<c1051660>] ? process_scheduled_works+0x30/0x30
>> [  221.490685]  [<c1055b56>] kthread+0x96/0xb0
>> [  221.490687]  [<c1050000>] ? put_unbound_pool+0x110/0x170
>> [  221.490691]  [<c1719c81>] ret_from_kernel_thread+0x21/0x30
>> [  221.490693]  [<c1055ac0>] ? kthread_worker_fn+0x110/0x110
>> [  221.490695] ---[ end trace 9befd914693f3083 ]---
>> [  221.490697] brcmfmac: brcmf_usb_dlneeded chip 57005 rev 0xf11cfcec
>> [  221.490699] brcmfmac: brcmf_fw_get_firmwares enter: dev=1-1
>>
>> diff --git a/drivers/net/wireless/brcm80211/brcmfmac/usb.c b/drivers/net/wireless/brcm80211/brcmfmac/usb.c
>> index 5265aa7..15b1aa7 100644
>> --- a/drivers/net/wireless/brcm80211/brcmfmac/usb.c
>> +++ b/drivers/net/wireless/brcm80211/brcmfmac/usb.c
>> @@ -709,8 +709,13 @@ static int brcmf_usb_dl_cmd(struct brcmf_usbdev_info *devinfo, u8 cmd,
>>  	char *tmpbuf;
>>  	u16 size;
>>  
>> -	if ((!devinfo) || (devinfo->ctl_urb == NULL))
>> +	if (!devinfo || !devinfo->ctl_urb || !devinfo->ctl_completed) {
>> +		WARN_ONCE(1, "EINVAL devinfo=%p ctl_rub=%p completed=%d\n",
>> +			devinfo,
>> +			devinfo ? devinfo->ctl_urb : NULL,
>> +			devinfo ? devinfo->ctl_completed : -1);
>>  		return -EINVAL;
>> +	}
>>  
>>  	tmpbuf = kmalloc(buflen, GFP_ATOMIC);
>>  	if (!tmpbuf)
>>
>>
>>>
>>> Regards,
>>> Arend
>>> ---
>>>  drivers/net/wireless/brcm80211/brcmfmac/usb.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/wireless/brcm80211/brcmfmac/usb.c
>>> b/drivers/net/wireles
>>> index dc13591..786c40b 100644
>>> --- a/drivers/net/wireless/brcm80211/brcmfmac/usb.c
>>> +++ b/drivers/net/wireless/brcm80211/brcmfmac/usb.c
>>> @@ -640,7 +640,7 @@ static int brcmf_usb_dl_cmd(struct brcmf_usbdev_info
>>> *devinf
>>>         char *tmpbuf;
>>>         u16 size;
>>>
>>> -       if ((!devinfo) || (devinfo->ctl_urb == NULL))
>>> +       if (!devinfo || !devinfo->ctl_urb || !devinfo->ctl_completed)
>>>                 return -EINVAL;
>>>
>>>         tmpbuf = kmalloc(buflen, GFP_ATOMIC);
>>>
>>>> Signed-off-by: Mathy Vanhoef <vanhoefm@gmail.com>
>>>> ---
>>>> Currently brcmfmac may crash when a USB device is attached (tested with a LG
>>>> TWFM-B003D). In particular it fails on the second call to brcmf_usb_dl_cmd in
>>>> the while loop of brcmf_usb_resetcfg. The problem is that an URB is being
>>>> submitted twice:
>>>>
>>>> [  169.861800] brcmfmac: brcmf_usb_dl_writeimage Enter, fw f14db000, len 348160
>>>> [  171.787791] brcmfmac: brcmf_usb_dl_writeimage Exit, err=0
>>>> [  171.787797] brcmfmac: brcmf_usb_dlstart Exit, err=0
>>>> [  171.787799] brcmfmac: brcmf_usb_dlrun Enter
>>>> [  171.791794] brcmfmac: brcmf_usb_resetcfg Enter
>>>> [  173.988072] ------------[ cut here ]------------
>>>> [  173.988083] WARNING: CPU: 0 PID: 369 at drivers/usb/core/urb.c:339 usb_submit_urb+0x4e6/0x500()
>>>> [  173.988085] URB eaf45f00 submitted while active
>>>> [  173.988086] Modules linked in: brcmfmac brcmutil vmw_pvscsi pcnet32 mptspi mptscsih mptbase
>>>> [  173.988100] CPU: 0 PID: 369 Comm: kworker/0:2 Not tainted 3.18.0-rc3-wl #1
>>>> [  173.988102] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 07/31/2013
>>>> [  173.988106] Workqueue: events request_firmware_work_func
>>>> [  173.988108]  00000000 00000000 ee747db8 c1711f4a ee747df8 ee747de8 c103edaf c18d1e10
>>>> [  173.988112]  ee747e14 00000171 c18a8b29 00000153 c1490556 c1490556 eaf45f00 eafdc660
>>>> [  173.988115]  f14b8fa0 ee747e00 c103ee4e 00000009 ee747df8 c18d1e10 ee747e14 ee747e50
>>>> [  173.988119] Call Trace:
>>>> [  173.988129]  [<c1711f4a>] dump_stack+0x41/0x52
>>>> [  173.988136]  [<c103edaf>] warn_slowpath_common+0x7f/0xa0
>>>> [  173.988139]  [<c1490556>] ? usb_submit_urb+0x4e6/0x500
>>>> [  173.988141]  [<c1490556>] ? usb_submit_urb+0x4e6/0x500
>>>> [  173.988147]  [<f14b8fa0>] ? brcmf_usb_ioctl_resp_wake+0x40/0x40 [brcmfmac]
>>>> [  173.988150]  [<c103ee4e>] warn_slowpath_fmt+0x2e/0x30
>>>> [  173.988152]  [<c1490556>] usb_submit_urb+0x4e6/0x500
>>>> [  173.988156]  [<c1123de1>] ? __kmalloc+0x21/0x140
>>>> [  173.988161]  [<f14b91c3>] ? brcmf_usb_dl_cmd+0x33/0x120 [brcmfmac]
>>>> [  173.988166]  [<f14b9243>] brcmf_usb_dl_cmd+0xb3/0x120 [brcmfmac]
>>>> [  173.988170]  [<f14ba6c4>] brcmf_usb_probe_phase2+0x4e4/0x640 [brcmfmac]
>>>> [  173.988176]  [<f14b4900>] brcmf_fw_request_code_done+0xd0/0xf0 [brcmfmac]
>>>> [  173.988178]  [<c1400876>] request_firmware_work_func+0x26/0x50
>>>> [  173.988182]  [<c10513ee>] process_one_work+0x11e/0x360
>>>> [  173.988184]  [<c1051750>] worker_thread+0xf0/0x3c0
>>>> [  173.988205]  [<c106e14a>] ? __wake_up_locked+0x1a/0x20
>>>> [  173.988208]  [<c1051660>] ? process_scheduled_works+0x30/0x30
>>>> [  173.988211]  [<c1055b56>] kthread+0x96/0xb0
>>>> [  173.988214]  [<c1719c81>] ret_from_kernel_thread+0x21/0x30
>>>> [  173.988217]  [<c1055ac0>] ? kthread_worker_fn+0x110/0x110
>>>> [  173.988219] ---[ end trace 0c88bf46801de083 ]---
>>>> [  173.988221] brcmf_usb_dl_cmd: usb_submit_urb failed -16
>>>> [  173.988396] brcmfmac: brcmf_usb_probe_phase2 failed: dev=1-1, err=-19
>>>> [  173.989503] brcmfmac: brcmf_usb_disconnect Enter
>>>>
>>>> This patch fixes the brcmf_usb_dl_cmd function to prevent an URB from being
>>>> submitted twice. Tested using a LG TWFM-B003D, which now works properly.
>>>>
>>>>
>>>>  drivers/net/wireless/brcm80211/brcmfmac/usb.c |    6 ++++--
>>>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/net/wireless/brcm80211/brcmfmac/usb.c b/drivers/net/wireless/brcm80211/brcmfmac/usb.c
>>>> index 5265aa7..1bc7858 100644
>>>> --- a/drivers/net/wireless/brcm80211/brcmfmac/usb.c
>>>> +++ b/drivers/net/wireless/brcm80211/brcmfmac/usb.c
>>>> @@ -738,10 +738,12 @@ static int brcmf_usb_dl_cmd(struct brcmf_usbdev_info *devinfo, u8 cmd,
>>>>  		goto finalize;
>>>>  	}
>>>>  
>>>> -	if (!brcmf_usb_ioctl_resp_wait(devinfo))
>>>> +	if (!brcmf_usb_ioctl_resp_wait(devinfo)) {
>>>> +		usb_unlink_urb(devinfo->ctl_urb);
>>>>  		ret = -ETIMEDOUT;
>>>> -	else
>>>> +	} else {
>>>>  		memcpy(buffer, tmpbuf, buflen);
>>>> +	}
>>>>  
>>>>  finalize:
>>>>  	kfree(tmpbuf);
>>>>
>>>
> 

^ permalink raw reply

* Re: [net-next PATCH v3 4/5] fm10k/igb/ixgbe: Replace __skb_alloc_page with dev_alloc_page
From: Jeff Kirsher @ 2014-11-11 17:28 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: netdev, linux-usb, leedom, hariprasad, donald.c.skidmore, oliver,
	balbi, matthew.vick, mgorman, davem
In-Reply-To: <20141111172657.16460.60596.stgit@ahduyck-vm-fedora20>

[-- Attachment #1: Type: text/plain, Size: 796 bytes --]

On Tue, 2014-11-11 at 09:26 -0800, Alexander Duyck wrote:
> The Intel drivers were pretty much just using the plain vanilla GFP
> flags
> in their calls to __skb_alloc_page so this change makes it so that
> they use
> dev_alloc_page which just uses GFP_ATOMIC for the gfp_flags value.
> 
> Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Cc: Matthew Vick <matthew.vick@intel.com>
> Cc: Don Skidmore <donald.c.skidmore@intel.com>
> Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com>
> ---
>  drivers/net/ethernet/intel/fm10k/fm10k_main.c |    2 +-
>  drivers/net/ethernet/intel/igb/igb_main.c     |    2 +-
>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    3 +--
>  3 files changed, 3 insertions(+), 4 deletions(-)

Thanks Alex, I will add the patch to my queue.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* [net-next PATCH v3 5/5] net: Remove __skb_alloc_page and __skb_alloc_pages
From: Alexander Duyck @ 2014-11-11 17:27 UTC (permalink / raw)
  To: netdev, linux-usb
  Cc: leedom, hariprasad, donald.c.skidmore, oliver, balbi,
	matthew.vick, mgorman, davem, jeffrey.t.kirsher
In-Reply-To: <20141111172523.16460.16845.stgit@ahduyck-vm-fedora20>

Remove the two functions which are now dead code.

Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com>
---
 include/linux/skbuff.h |   43 -------------------------------------------
 1 file changed, 43 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 2e5221f..73c370e 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -2233,49 +2233,6 @@ static inline struct page *dev_alloc_page(void)
 }
 
 /**
- *	__skb_alloc_pages - allocate pages for ps-rx on a skb and preserve pfmemalloc data
- *	@gfp_mask: alloc_pages_node mask. Set __GFP_NOMEMALLOC if not for network packet RX
- *	@skb: skb to set pfmemalloc on if __GFP_MEMALLOC is used
- *	@order: size of the allocation
- *
- * 	Allocate a new page.
- *
- * 	%NULL is returned if there is no free memory.
-*/
-static inline struct page *__skb_alloc_pages(gfp_t gfp_mask,
-					      struct sk_buff *skb,
-					      unsigned int order)
-{
-	struct page *page;
-
-	gfp_mask |= __GFP_COLD;
-
-	if (!(gfp_mask & __GFP_NOMEMALLOC))
-		gfp_mask |= __GFP_MEMALLOC;
-
-	page = alloc_pages_node(NUMA_NO_NODE, gfp_mask, order);
-	if (skb && page && page->pfmemalloc)
-		skb->pfmemalloc = true;
-
-	return page;
-}
-
-/**
- *	__skb_alloc_page - allocate a page for ps-rx for a given skb and preserve pfmemalloc data
- *	@gfp_mask: alloc_pages_node mask. Set __GFP_NOMEMALLOC if not for network packet RX
- *	@skb: skb to set pfmemalloc on if __GFP_MEMALLOC is used
- *
- * 	Allocate a new page.
- *
- * 	%NULL is returned if there is no free memory.
- */
-static inline struct page *__skb_alloc_page(gfp_t gfp_mask,
-					     struct sk_buff *skb)
-{
-	return __skb_alloc_pages(gfp_mask, skb, 0);
-}
-
-/**
  *	skb_propagate_pfmemalloc - Propagate pfmemalloc if skb is allocated after RX page
  *	@page: The page that was allocated from skb_alloc_page
  *	@skb: The skb that may need pfmemalloc set

^ permalink raw reply related

* [net-next PATCH v3 2/5] cxgb4/cxgb4vf: Replace __skb_alloc_page with __dev_alloc_page
From: Alexander Duyck @ 2014-11-11 17:26 UTC (permalink / raw)
  To: netdev, linux-usb
  Cc: leedom, hariprasad, donald.c.skidmore, oliver, balbi,
	matthew.vick, mgorman, davem, jeffrey.t.kirsher
In-Reply-To: <20141111172523.16460.16845.stgit@ahduyck-vm-fedora20>

Drop the bloated use of __skb_alloc_page and replace it with
__dev_alloc_page.  In addition update the one other spot that is
allocating a page so that it allocates with the correct flags.

Cc: Hariprasad S <hariprasad@chelsio.com>
Cc: Casey Leedom <leedom@chelsio.com>
Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com>
---
 drivers/net/ethernet/chelsio/cxgb4/sge.c   |    6 +++---
 drivers/net/ethernet/chelsio/cxgb4vf/sge.c |    7 ++++---
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/chelsio/cxgb4/sge.c b/drivers/net/ethernet/chelsio/cxgb4/sge.c
index 5e1b314..20ee002 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/sge.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/sge.c
@@ -576,7 +576,7 @@ static unsigned int refill_fl(struct adapter *adap, struct sge_fl *q, int n,
 	__be64 *d = &q->desc[q->pidx];
 	struct rx_sw_desc *sd = &q->sdesc[q->pidx];
 
-	gfp |= __GFP_NOWARN | __GFP_COLD;
+	gfp |= __GFP_NOWARN;
 
 	if (s->fl_pg_order == 0)
 		goto alloc_small_pages;
@@ -585,7 +585,7 @@ static unsigned int refill_fl(struct adapter *adap, struct sge_fl *q, int n,
 	 * Prefer large buffers
 	 */
 	while (n) {
-		pg = alloc_pages(gfp | __GFP_COMP, s->fl_pg_order);
+		pg = __dev_alloc_pages(gfp, s->fl_pg_order);
 		if (unlikely(!pg)) {
 			q->large_alloc_failed++;
 			break;       /* fall back to single pages */
@@ -615,7 +615,7 @@ static unsigned int refill_fl(struct adapter *adap, struct sge_fl *q, int n,
 
 alloc_small_pages:
 	while (n--) {
-		pg = __skb_alloc_page(gfp, NULL);
+		pg = __dev_alloc_page(gfp);
 		if (unlikely(!pg)) {
 			q->alloc_failed++;
 			break;
diff --git a/drivers/net/ethernet/chelsio/cxgb4vf/sge.c b/drivers/net/ethernet/chelsio/cxgb4vf/sge.c
index 85036e6..9df40df 100644
--- a/drivers/net/ethernet/chelsio/cxgb4vf/sge.c
+++ b/drivers/net/ethernet/chelsio/cxgb4vf/sge.c
@@ -602,6 +602,8 @@ static unsigned int refill_fl(struct adapter *adapter, struct sge_fl *fl,
 	 */
 	BUG_ON(fl->avail + n > fl->size - FL_PER_EQ_UNIT);
 
+	gfp |= __GFP_NOWARN;
+
 	/*
 	 * If we support large pages, prefer large buffers and fail over to
 	 * small pages if we can't allocate large pages to satisfy the refill.
@@ -612,8 +614,7 @@ static unsigned int refill_fl(struct adapter *adapter, struct sge_fl *fl,
 		goto alloc_small_pages;
 
 	while (n) {
-		page = alloc_pages(gfp | __GFP_COMP | __GFP_NOWARN,
-				   FL_PG_ORDER);
+		page = __dev_alloc_pages(gfp, FL_PG_ORDER);
 		if (unlikely(!page)) {
 			/*
 			 * We've failed inour attempt to allocate a "large
@@ -657,7 +658,7 @@ static unsigned int refill_fl(struct adapter *adapter, struct sge_fl *fl,
 
 alloc_small_pages:
 	while (n--) {
-		page = __skb_alloc_page(gfp | __GFP_NOWARN, NULL);
+		page = __dev_alloc_page(gfp);
 		if (unlikely(!page)) {
 			fl->alloc_failed++;
 			break;

^ permalink raw reply related

* [net-next PATCH v3 4/5] fm10k/igb/ixgbe: Replace __skb_alloc_page with dev_alloc_page
From: Alexander Duyck @ 2014-11-11 17:26 UTC (permalink / raw)
  To: netdev, linux-usb
  Cc: leedom, hariprasad, donald.c.skidmore, oliver, balbi,
	matthew.vick, mgorman, davem, jeffrey.t.kirsher
In-Reply-To: <20141111172523.16460.16845.stgit@ahduyck-vm-fedora20>

The Intel drivers were pretty much just using the plain vanilla GFP flags
in their calls to __skb_alloc_page so this change makes it so that they use
dev_alloc_page which just uses GFP_ATOMIC for the gfp_flags value.

Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Cc: Matthew Vick <matthew.vick@intel.com>
Cc: Don Skidmore <donald.c.skidmore@intel.com>
Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com>
---
 drivers/net/ethernet/intel/fm10k/fm10k_main.c |    2 +-
 drivers/net/ethernet/intel/igb/igb_main.c     |    2 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    3 +--
 3 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_main.c b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
index e645af4..73457ed 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_main.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
@@ -83,7 +83,7 @@ static bool fm10k_alloc_mapped_page(struct fm10k_ring *rx_ring,
 		return true;
 
 	/* alloc new page for storage */
-	page = alloc_page(GFP_ATOMIC | __GFP_COLD);
+	page = dev_alloc_page();
 	if (unlikely(!page)) {
 		rx_ring->rx_stats.alloc_failed++;
 		return false;
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index a2d72a8..1e35fae 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -6988,7 +6988,7 @@ static bool igb_alloc_mapped_page(struct igb_ring *rx_ring,
 		return true;
 
 	/* alloc new page for storage */
-	page = __skb_alloc_page(GFP_ATOMIC | __GFP_COLD, NULL);
+	page = dev_alloc_page();
 	if (unlikely(!page)) {
 		rx_ring->rx_stats.alloc_failed++;
 		return false;
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index d2df4e3..7405478 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -1440,8 +1440,7 @@ static bool ixgbe_alloc_mapped_page(struct ixgbe_ring *rx_ring,
 
 	/* alloc new page for storage */
 	if (likely(!page)) {
-		page = __skb_alloc_pages(GFP_ATOMIC | __GFP_COLD | __GFP_COMP,
-					 bi->skb, ixgbe_rx_pg_order(rx_ring));
+		page = dev_alloc_pages(ixgbe_rx_pg_order(rx_ring));
 		if (unlikely(!page)) {
 			rx_ring->rx_stats.alloc_rx_page_failed++;
 			return false;

^ permalink raw reply related

* [net-next PATCH v3 3/5] phonet: Replace calls to __skb_alloc_page with __dev_alloc_page
From: Alexander Duyck @ 2014-11-11 17:26 UTC (permalink / raw)
  To: netdev, linux-usb
  Cc: leedom, hariprasad, donald.c.skidmore, oliver, balbi,
	matthew.vick, mgorman, davem, jeffrey.t.kirsher
In-Reply-To: <20141111172523.16460.16845.stgit@ahduyck-vm-fedora20>

Replace the calls to __skb_alloc_page that are passed NULL with calls to
__dev_alloc_page.

In addition remove __GFP_COLD flag from allocations as we only want it for
the Rx buffer which is taken care of by __dev_alloc_skb, not for any
secondary allocations such as the queue element transmit descriptors.

Cc: Oliver Neukum <oliver@neukum.org>
Cc: Felipe Balbi <balbi@ti.com>
Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com>
---
 drivers/net/usb/cdc-phonet.c           |    6 +++---
 drivers/usb/gadget/function/f_phonet.c |    6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/usb/cdc-phonet.c b/drivers/net/usb/cdc-phonet.c
index 2ec1500..415ce8b 100644
--- a/drivers/net/usb/cdc-phonet.c
+++ b/drivers/net/usb/cdc-phonet.c
@@ -130,7 +130,7 @@ static int rx_submit(struct usbpn_dev *pnd, struct urb *req, gfp_t gfp_flags)
 	struct page *page;
 	int err;
 
-	page = __skb_alloc_page(gfp_flags | __GFP_NOMEMALLOC, NULL);
+	page = __dev_alloc_page(gfp_flags | __GFP_NOMEMALLOC);
 	if (!page)
 		return -ENOMEM;
 
@@ -212,7 +212,7 @@ resubmit:
 	if (page)
 		put_page(page);
 	if (req)
-		rx_submit(pnd, req, GFP_ATOMIC | __GFP_COLD);
+		rx_submit(pnd, req, GFP_ATOMIC);
 }
 
 static int usbpn_close(struct net_device *dev);
@@ -231,7 +231,7 @@ static int usbpn_open(struct net_device *dev)
 	for (i = 0; i < rxq_size; i++) {
 		struct urb *req = usb_alloc_urb(0, GFP_KERNEL);
 
-		if (!req || rx_submit(pnd, req, GFP_KERNEL | __GFP_COLD)) {
+		if (!req || rx_submit(pnd, req, GFP_KERNEL)) {
 			usb_free_urb(req);
 			usbpn_close(dev);
 			return -ENOMEM;
diff --git a/drivers/usb/gadget/function/f_phonet.c b/drivers/usb/gadget/function/f_phonet.c
index b9cfc15..cde7397 100644
--- a/drivers/usb/gadget/function/f_phonet.c
+++ b/drivers/usb/gadget/function/f_phonet.c
@@ -303,7 +303,7 @@ pn_rx_submit(struct f_phonet *fp, struct usb_request *req, gfp_t gfp_flags)
 	struct page *page;
 	int err;
 
-	page = __skb_alloc_page(gfp_flags | __GFP_NOMEMALLOC, NULL);
+	page = __dev_alloc_page(gfp_flags | __GFP_NOMEMALLOC);
 	if (!page)
 		return -ENOMEM;
 
@@ -377,7 +377,7 @@ static void pn_rx_complete(struct usb_ep *ep, struct usb_request *req)
 	if (page)
 		put_page(page);
 	if (req)
-		pn_rx_submit(fp, req, GFP_ATOMIC | __GFP_COLD);
+		pn_rx_submit(fp, req, GFP_ATOMIC);
 }
 
 /*-------------------------------------------------------------------------*/
@@ -437,7 +437,7 @@ static int pn_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
 
 			netif_carrier_on(dev);
 			for (i = 0; i < phonet_rxq_size; i++)
-				pn_rx_submit(fp, fp->out_reqv[i], GFP_ATOMIC | __GFP_COLD);
+				pn_rx_submit(fp, fp->out_reqv[i], GFP_ATOMIC);
 		}
 		spin_unlock(&port->lock);
 		return 0;

^ permalink raw reply related

* [net-next PATCH v3 1/5] net: Add device Rx page allocation function
From: Alexander Duyck @ 2014-11-11 17:26 UTC (permalink / raw)
  To: netdev, linux-usb
  Cc: leedom, hariprasad, donald.c.skidmore, oliver, balbi,
	matthew.vick, mgorman, davem, jeffrey.t.kirsher
In-Reply-To: <20141111172523.16460.16845.stgit@ahduyck-vm-fedora20>

This patch implements __dev_alloc_pages and __dev_alloc_page.  These are
meant to replace the __skb_alloc_pages and __skb_alloc_page functions.  The
reason for doing this is that it occurred to me that __skb_alloc_page is
supposed to be passed an sk_buff pointer, but it is NULL in all cases where
it is used.  Worse is that in the case of ixgbe it is passed NULL via the
sk_buff pointer in the rx_buffer info structure which means the compiler is
not correctly stripping it out.

The naming for these functions is based on dev_alloc_skb and __dev_alloc_skb.
There was originally a netdev_alloc_page, however that was passed a
net_device pointer and this function is not so I thought it best to follow
that naming scheme since that is the same difference between dev_alloc_skb
and netdev_alloc_skb.

In the case of anything greater than order 0 it is assumed that we want a
compound page so __GFP_COMP is set for all allocations as we expect a
compound page when assigning a page frag.

The other change in this patch is to exploit the behaviors of the page
allocator in how it handles flags.  So for example we can always set
__GFP_COMP and __GFP_MEMALLOC since they are ignored if they are not
applicable or are overridden by another flag.

Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com>
---
 include/linux/skbuff.h |   48 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 103fbe8..2e5221f 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -2185,6 +2185,54 @@ static inline struct sk_buff *netdev_alloc_skb_ip_align(struct net_device *dev,
 }
 
 /**
+ * __dev_alloc_pages - allocate page for network Rx
+ * @gfp_mask: allocation priority. Set __GFP_NOMEMALLOC if not for network Rx
+ * @order: size of the allocation
+ *
+ * Allocate a new page.
+ *
+ * %NULL is returned if there is no free memory.
+*/
+static inline struct page *__dev_alloc_pages(gfp_t gfp_mask,
+					     unsigned int order)
+{
+	/* This piece of code contains several assumptions.
+	 * 1.  This is for device Rx, therefor a cold page is preferred.
+	 * 2.  The expectation is the user wants a compound page.
+	 * 3.  If requesting a order 0 page it will not be compound
+	 *     due to the check to see if order has a value in prep_new_page
+	 * 4.  __GFP_MEMALLOC is ignored if __GFP_NOMEMALLOC is set due to
+	 *     code in gfp_to_alloc_flags that should be enforcing this.
+	 */
+	gfp_mask |= __GFP_COLD | __GFP_COMP | __GFP_MEMALLOC;
+
+	return alloc_pages_node(NUMA_NO_NODE, gfp_mask, order);
+}
+
+static inline struct page *dev_alloc_pages(unsigned int order)
+{
+	return __dev_alloc_pages(GFP_ATOMIC, order);
+}
+
+/**
+ * __dev_alloc_page - allocate a page for network Rx
+ * @gfp_mask: allocation priority. Set __GFP_NOMEMALLOC if not for network Rx
+ *
+ * Allocate a new page.
+ *
+ * %NULL is returned if there is no free memory.
+ */
+static inline struct page *__dev_alloc_page(gfp_t gfp_mask)
+{
+	return __dev_alloc_pages(gfp_mask, 0);
+}
+
+static inline struct page *dev_alloc_page(void)
+{
+	return __dev_alloc_page(GFP_ATOMIC);
+}
+
+/**
  *	__skb_alloc_pages - allocate pages for ps-rx on a skb and preserve pfmemalloc data
  *	@gfp_mask: alloc_pages_node mask. Set __GFP_NOMEMALLOC if not for network packet RX
  *	@skb: skb to set pfmemalloc on if __GFP_MEMALLOC is used

^ permalink raw reply related

* [net-next PATCH v3 0/5] Replace __skb_alloc_pages with simpler function
From: Alexander Duyck @ 2014-11-11 17:26 UTC (permalink / raw)
  To: netdev, linux-usb
  Cc: leedom, hariprasad, donald.c.skidmore, oliver, balbi,
	matthew.vick, mgorman, davem, jeffrey.t.kirsher

This patch series replaces __skb_alloc_pages with a much simpler function,
__dev_alloc_pages.  The main difference between the two is that
__skb_alloc_pages had an sk_buff pointer that was being passed as NULL in
call places where it was called.  In a couple of cases the NULL was passed
by variable and this led to unnecessary code being run.

As such in order to simplify things the __dev_alloc_pages call only takes a
mask and the page order being requested.  In addition it takes advantage of
several behaviors already built into the page allocator so that it can just
set GFP flags unconditionally.

v2: Renamed functions to dev_alloc_page(s) instead of netdev_alloc_page(s)
    Removed __GFP_COLD flag from usb code as it was redundant
v3: Update patch descriptions and subjects to match changes in v2

---

Alexander Duyck (5):
      net: Add device Rx page allocation function
      cxgb4/cxgb4vf: Replace __skb_alloc_page with __dev_alloc_page
      phonet: Replace calls to __skb_alloc_page with __dev_alloc_page
      fm10k/igb/ixgbe: Replace __skb_alloc_page with dev_alloc_page
      net: Remove __skb_alloc_page and __skb_alloc_pages


 drivers/net/ethernet/chelsio/cxgb4/sge.c      |    6 +-
 drivers/net/ethernet/chelsio/cxgb4vf/sge.c    |    7 ++-
 drivers/net/ethernet/intel/fm10k/fm10k_main.c |    2 -
 drivers/net/ethernet/intel/igb/igb_main.c     |    2 -
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    3 -
 drivers/net/usb/cdc-phonet.c                  |    6 +-
 drivers/usb/gadget/function/f_phonet.c        |    6 +-
 include/linux/skbuff.h                        |   61 ++++++++++++++-----------
 8 files changed, 49 insertions(+), 44 deletions(-)

--

^ permalink raw reply

* Re: [net-next PATCH v2 4/5] fm10k/igb/ixgbe: Replace __skb_alloc_page with netdev_alloc_page
From: Alexander Duyck @ 2014-11-11 17:24 UTC (permalink / raw)
  To: Cong Wang
  Cc: netdev, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	leedom-ut6Up61K2wZBDgjK7y7TUQ, hariprasad-ut6Up61K2wZBDgjK7y7TUQ,
	donald.c.skidmore-ral2JQCrhuEAvxtiuMwx3w,
	oliver-GvhC2dPhHPQdnm+yROfE0A, balbi-l0cyMroinI0,
	matthew.vick-ral2JQCrhuEAvxtiuMwx3w, mgorman-l3A5Bk7waGM,
	David Miller, jeffrey.t.kirsher-ral2JQCrhuEAvxtiuMwx3w
In-Reply-To: <CAHA+R7P+GtA6xHxrPZOhxHM0LA8mGkmWugTqLeHtw5M6P3udvQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>


On 11/11/2014 09:15 AM, Cong Wang wrote:
> On Tue, Nov 11, 2014 at 9:11 AM, Alexander Duyck
> <alexander.h.duyck-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>> diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_main.c b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
>> index e645af4..73457ed 100644
>> --- a/drivers/net/ethernet/intel/fm10k/fm10k_main.c
>> +++ b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
>> @@ -83,7 +83,7 @@ static bool fm10k_alloc_mapped_page(struct fm10k_ring *rx_ring,
>>                  return true;
>>
>>          /* alloc new page for storage */
>> -       page = alloc_page(GFP_ATOMIC | __GFP_COLD);
>> +       page = dev_alloc_page();
> Doesn't match $subject.

Yeah, I just noticed that.  I missed the patch title and comments when I 
was doing the replacement.

v3 on the way.

Thanks,

Alex
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [patch net-next 1/2] net: move vlan pop/push functions into common code
From: Pravin Shelar @ 2014-11-11 17:24 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: netdev, David Miller, Jamal Hadi Salim, Tom Herbert, Eric Dumazet,
	willemb, Daniel Borkmann, mst, fw, Paul.Durrant, Thomas Graf
In-Reply-To: <1415700789-9171-1-git-send-email-jiri@resnulli.us>

On Tue, Nov 11, 2014 at 2:13 AM, Jiri Pirko <jiri@resnulli.us> wrote:
> So it can be used from out of openvswitch code.
> Did couple of cosmetic changes on the way, namely variable naming and
> adding support for 8021AD proto.
>
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
> ---
>  include/linux/skbuff.h    |  2 ++
>  net/core/skbuff.c         | 86 +++++++++++++++++++++++++++++++++++++++++++++++
>  net/openvswitch/actions.c | 76 ++---------------------------------------
>  3 files changed, 90 insertions(+), 74 deletions(-)
>
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 103fbe8..3b0445c 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -2673,6 +2673,8 @@ void skb_scrub_packet(struct sk_buff *skb, bool xnet);
>  unsigned int skb_gso_transport_seglen(const struct sk_buff *skb);
>  struct sk_buff *skb_segment(struct sk_buff *skb, netdev_features_t features);
>  struct sk_buff *skb_vlan_untag(struct sk_buff *skb);
> +int skb_vlan_pop(struct sk_buff *skb);
> +int skb_vlan_push(struct sk_buff *skb, __be16 vlan_proto, u16 vlan_tci);
>
>  struct skb_checksum_ops {
>         __wsum (*update)(const void *mem, int len, __wsum wsum);
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 7001896..75e53d4 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -4151,6 +4151,92 @@ err_free:
>  }
>  EXPORT_SYMBOL(skb_vlan_untag);
>
> +/* remove VLAN header from packet and update csum accordingly. */
> +static int __skb_vlan_pop(struct sk_buff *skb, u16 *vlan_tci)
> +{
> +       struct vlan_hdr *vhdr;
> +       int err;
> +
> +       if (!pskb_may_pull(skb, VLAN_ETH_HLEN))
> +               return -ENOMEM;
> +
> +       if (!skb_cloned(skb) || skb_clone_writable(skb, VLAN_ETH_HLEN))
> +               return 0;
> +
> +       err = pskb_expand_head(skb, 0, 0, GFP_ATOMIC);
> +       if (unlikely(err))
> +               return err;
> +
> +       if (skb->ip_summed == CHECKSUM_COMPLETE)
> +               skb->csum = csum_sub(skb->csum, csum_partial(skb->data
> +                                       + (2 * ETH_ALEN), VLAN_HLEN, 0));
> +
> +       vhdr = (struct vlan_hdr *)(skb->data + ETH_HLEN);
> +       *vlan_tci = ntohs(vhdr->h_vlan_TCI);
> +
> +       memmove(skb->data + VLAN_HLEN, skb->data, 2 * ETH_ALEN);
> +       __skb_pull(skb, VLAN_HLEN);
> +
> +       vlan_set_encap_proto(skb, vhdr);
> +       skb->mac_header += VLAN_HLEN;
> +       if (skb_network_offset(skb) < ETH_HLEN)
> +               skb_set_network_header(skb, ETH_HLEN);
> +       skb_reset_mac_len(skb);
> +
> +       return 0;
> +}
> +
> +int skb_vlan_pop(struct sk_buff *skb)
> +{
> +       u16 vlan_tci;
> +       __be16 vlan_proto;
> +       int err;
> +
> +       if (likely(vlan_tx_tag_present(skb))) {
> +               skb->vlan_tci = 0;
> +       } else {
> +               if (unlikely((skb->protocol != htons(ETH_P_8021Q) &&
> +                             skb->protocol != htons(ETH_P_8021AD)) ||
> +                            skb->len < VLAN_ETH_HLEN))
> +                       return 0;
> +
> +               err = __skb_vlan_pop(skb, &vlan_tci);
> +               if (err)
> +                       return err;
> +       }
> +       /* move next vlan tag to hw accel tag */
> +       if (likely((skb->protocol != htons(ETH_P_8021Q) &&
> +                   skb->protocol != htons(ETH_P_8021AD)) ||
> +                  skb->len < VLAN_ETH_HLEN))
> +               return 0;
> +
> +       vlan_proto = skb->protocol;
> +       err = __skb_vlan_pop(skb, &vlan_tci);
> +       if (unlikely(err))
> +               return err;
> +
> +       __vlan_hwaccel_put_tag(skb, vlan_proto, vlan_tci);
> +       return 0;
> +}
> +EXPORT_SYMBOL(skb_vlan_pop);
> +
> +int skb_vlan_push(struct sk_buff *skb, __be16 vlan_proto, u16 vlan_tci)
> +{
> +       if (vlan_tx_tag_present(skb)) {
> +               /* push down current VLAN tag */
> +               if (!__vlan_put_tag(skb, skb->vlan_proto,
> +                                   vlan_tx_tag_get(skb)))
> +                       return -ENOMEM;
> +
Since you are restructuring this code, can you also change
__vlan_put_tag() to not free skb on error. So that these two new
functions can have common error handling code.

> +               if (skb->ip_summed == CHECKSUM_COMPLETE)
> +                       skb->csum = csum_add(skb->csum, csum_partial(skb->data
> +                                       + (2 * ETH_ALEN), VLAN_HLEN, 0));
> +       }
> +       __vlan_hwaccel_put_tag(skb, vlan_proto, vlan_tci);
> +       return 0;
> +}
> +EXPORT_SYMBOL(skb_vlan_push);
> +
>  /**
>   * alloc_skb_with_frags - allocate skb with page frags
>   *
> diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
> index f7e5891..1b28110 100644
> --- a/net/openvswitch/actions.c
> +++ b/net/openvswitch/actions.c
> @@ -206,86 +206,14 @@ static int set_mpls(struct sk_buff *skb, const __be32 *mpls_lse)
>         return 0;
>  }
>
> -/* remove VLAN header from packet and update csum accordingly. */
> -static int __pop_vlan_tci(struct sk_buff *skb, __be16 *current_tci)
> -{
> -       struct vlan_hdr *vhdr;
> -       int err;
> -
> -       err = make_writable(skb, VLAN_ETH_HLEN);
> -       if (unlikely(err))
> -               return err;
> -
> -       if (skb->ip_summed == CHECKSUM_COMPLETE)
> -               skb->csum = csum_sub(skb->csum, csum_partial(skb->data
> -                                       + (2 * ETH_ALEN), VLAN_HLEN, 0));
> -
> -       vhdr = (struct vlan_hdr *)(skb->data + ETH_HLEN);
> -       *current_tci = vhdr->h_vlan_TCI;
> -
> -       memmove(skb->data + VLAN_HLEN, skb->data, 2 * ETH_ALEN);
> -       __skb_pull(skb, VLAN_HLEN);
> -
> -       vlan_set_encap_proto(skb, vhdr);
> -       skb->mac_header += VLAN_HLEN;
> -
> -       if (skb_network_offset(skb) < ETH_HLEN)
> -               skb_set_network_header(skb, ETH_HLEN);
> -
> -       /* Update mac_len for subsequent MPLS actions */
> -       skb_reset_mac_len(skb);
> -       return 0;
> -}
> -
>  static int pop_vlan(struct sk_buff *skb)
>  {
> -       __be16 tci;
> -       int err;
> -
> -       if (likely(vlan_tx_tag_present(skb))) {
> -               skb->vlan_tci = 0;
> -       } else {
> -               if (unlikely(skb->protocol != htons(ETH_P_8021Q) ||
> -                            skb->len < VLAN_ETH_HLEN))
> -                       return 0;
> -
> -               err = __pop_vlan_tci(skb, &tci);
> -               if (err)
> -                       return err;
> -       }
> -       /* move next vlan tag to hw accel tag */
> -       if (likely(skb->protocol != htons(ETH_P_8021Q) ||
> -                  skb->len < VLAN_ETH_HLEN))
> -               return 0;
> -
> -       err = __pop_vlan_tci(skb, &tci);
> -       if (unlikely(err))
> -               return err;
> -
> -       __vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), ntohs(tci));
> -       return 0;
> +       return skb_vlan_pop(skb);
>  }
>
>  static int push_vlan(struct sk_buff *skb, const struct ovs_action_push_vlan *vlan)
>  {
> -       if (unlikely(vlan_tx_tag_present(skb))) {
> -               u16 current_tag;
> -
> -               /* push down current VLAN tag */
> -               current_tag = vlan_tx_tag_get(skb);
> -
> -               if (!__vlan_put_tag(skb, skb->vlan_proto, current_tag))
> -                       return -ENOMEM;
> -               /* Update mac_len for subsequent MPLS actions */
> -               skb->mac_len += VLAN_HLEN;
> -
> -               if (skb->ip_summed == CHECKSUM_COMPLETE)
> -                       skb->csum = csum_add(skb->csum, csum_partial(skb->data
> -                                       + (2 * ETH_ALEN), VLAN_HLEN, 0));
> -
> -       }
> -       __vlan_hwaccel_put_tag(skb, vlan->vlan_tpid, ntohs(vlan->vlan_tci) & ~VLAN_TAG_PRESENT);
> -       return 0;
> +       return skb_vlan_push(skb, vlan->vlan_tpid, ntohs(vlan->vlan_tci) & ~VLAN_TAG_PRESENT);
>  }
>
>  static int set_eth_addr(struct sk_buff *skb,
> --
> 1.9.3
>

^ permalink raw reply

* Re: [net-next PATCH v2 4/5] fm10k/igb/ixgbe: Replace __skb_alloc_page with netdev_alloc_page
From: Cong Wang @ 2014-11-11 17:15 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: netdev, linux-usb, leedom, hariprasad, donald.c.skidmore, oliver,
	balbi, matthew.vick, mgorman, David Miller, jeffrey.t.kirsher
In-Reply-To: <20141111171117.15976.41609.stgit@ahduyck-vm-fedora20>

On Tue, Nov 11, 2014 at 9:11 AM, Alexander Duyck
<alexander.h.duyck@redhat.com> wrote:
> diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_main.c b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
> index e645af4..73457ed 100644
> --- a/drivers/net/ethernet/intel/fm10k/fm10k_main.c
> +++ b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
> @@ -83,7 +83,7 @@ static bool fm10k_alloc_mapped_page(struct fm10k_ring *rx_ring,
>                 return true;
>
>         /* alloc new page for storage */
> -       page = alloc_page(GFP_ATOMIC | __GFP_COLD);
> +       page = dev_alloc_page();

Doesn't match $subject.

^ permalink raw reply

* [net-next PATCH v2 5/5] net: Remove __skb_alloc_page and __skb_alloc_pages
From: Alexander Duyck @ 2014-11-11 17:11 UTC (permalink / raw)
  To: netdev, linux-usb
  Cc: leedom, hariprasad, donald.c.skidmore, oliver, balbi,
	matthew.vick, mgorman, davem, jeffrey.t.kirsher
In-Reply-To: <20141111170711.15976.44062.stgit@ahduyck-vm-fedora20>

Remove the two functions which are now dead code.

Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com>
---
 include/linux/skbuff.h |   43 -------------------------------------------
 1 file changed, 43 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 2e5221f..73c370e 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -2233,49 +2233,6 @@ static inline struct page *dev_alloc_page(void)
 }
 
 /**
- *	__skb_alloc_pages - allocate pages for ps-rx on a skb and preserve pfmemalloc data
- *	@gfp_mask: alloc_pages_node mask. Set __GFP_NOMEMALLOC if not for network packet RX
- *	@skb: skb to set pfmemalloc on if __GFP_MEMALLOC is used
- *	@order: size of the allocation
- *
- * 	Allocate a new page.
- *
- * 	%NULL is returned if there is no free memory.
-*/
-static inline struct page *__skb_alloc_pages(gfp_t gfp_mask,
-					      struct sk_buff *skb,
-					      unsigned int order)
-{
-	struct page *page;
-
-	gfp_mask |= __GFP_COLD;
-
-	if (!(gfp_mask & __GFP_NOMEMALLOC))
-		gfp_mask |= __GFP_MEMALLOC;
-
-	page = alloc_pages_node(NUMA_NO_NODE, gfp_mask, order);
-	if (skb && page && page->pfmemalloc)
-		skb->pfmemalloc = true;
-
-	return page;
-}
-
-/**
- *	__skb_alloc_page - allocate a page for ps-rx for a given skb and preserve pfmemalloc data
- *	@gfp_mask: alloc_pages_node mask. Set __GFP_NOMEMALLOC if not for network packet RX
- *	@skb: skb to set pfmemalloc on if __GFP_MEMALLOC is used
- *
- * 	Allocate a new page.
- *
- * 	%NULL is returned if there is no free memory.
- */
-static inline struct page *__skb_alloc_page(gfp_t gfp_mask,
-					     struct sk_buff *skb)
-{
-	return __skb_alloc_pages(gfp_mask, skb, 0);
-}
-
-/**
  *	skb_propagate_pfmemalloc - Propagate pfmemalloc if skb is allocated after RX page
  *	@page: The page that was allocated from skb_alloc_page
  *	@skb: The skb that may need pfmemalloc set

^ permalink raw reply related

* [net-next PATCH v2 4/5] fm10k/igb/ixgbe: Replace __skb_alloc_page with netdev_alloc_page
From: Alexander Duyck @ 2014-11-11 17:11 UTC (permalink / raw)
  To: netdev, linux-usb
  Cc: leedom, hariprasad, donald.c.skidmore, oliver, balbi,
	matthew.vick, mgorman, davem, jeffrey.t.kirsher
In-Reply-To: <20141111170711.15976.44062.stgit@ahduyck-vm-fedora20>

The Intel drivers were pretty much just using the plain vanilla GFP flags
in their calls to __skb_alloc_page so this change makes it so that they use
netdev_alloc_page which just uses GFP_ATOMIC for the gfp_flags value.

Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Cc: Matthew Vick <matthew.vick@intel.com>
Cc: Don Skidmore <donald.c.skidmore@intel.com>
Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com>
---
 drivers/net/ethernet/intel/fm10k/fm10k_main.c |    2 +-
 drivers/net/ethernet/intel/igb/igb_main.c     |    2 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    3 +--
 3 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_main.c b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
index e645af4..73457ed 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_main.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
@@ -83,7 +83,7 @@ static bool fm10k_alloc_mapped_page(struct fm10k_ring *rx_ring,
 		return true;
 
 	/* alloc new page for storage */
-	page = alloc_page(GFP_ATOMIC | __GFP_COLD);
+	page = dev_alloc_page();
 	if (unlikely(!page)) {
 		rx_ring->rx_stats.alloc_failed++;
 		return false;
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index a2d72a8..1e35fae 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -6988,7 +6988,7 @@ static bool igb_alloc_mapped_page(struct igb_ring *rx_ring,
 		return true;
 
 	/* alloc new page for storage */
-	page = __skb_alloc_page(GFP_ATOMIC | __GFP_COLD, NULL);
+	page = dev_alloc_page();
 	if (unlikely(!page)) {
 		rx_ring->rx_stats.alloc_failed++;
 		return false;
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index d2df4e3..7405478 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -1440,8 +1440,7 @@ static bool ixgbe_alloc_mapped_page(struct ixgbe_ring *rx_ring,
 
 	/* alloc new page for storage */
 	if (likely(!page)) {
-		page = __skb_alloc_pages(GFP_ATOMIC | __GFP_COLD | __GFP_COMP,
-					 bi->skb, ixgbe_rx_pg_order(rx_ring));
+		page = dev_alloc_pages(ixgbe_rx_pg_order(rx_ring));
 		if (unlikely(!page)) {
 			rx_ring->rx_stats.alloc_rx_page_failed++;
 			return false;

^ permalink raw reply related

* [net-next PATCH v2 3/5] phonet: Replace calls to __skb_alloc_page with __netdev_alloc_page
From: Alexander Duyck @ 2014-11-11 17:11 UTC (permalink / raw)
  To: netdev, linux-usb
  Cc: leedom, hariprasad, donald.c.skidmore, oliver, balbi,
	matthew.vick, mgorman, davem, jeffrey.t.kirsher
In-Reply-To: <20141111170711.15976.44062.stgit@ahduyck-vm-fedora20>

Replace the calls to __skb_alloc_page that are passed NULL with calls to
__netdev_alloc_page.

In addition remove __GFP_COLD flag from allocations as we only want it for
the Rx buffer which is taken care of by __dev_alloc_skb, not for any
secondary allocations such as the queue element transmit descriptors.

Cc: Oliver Neukum <oliver@neukum.org>
Cc: Felipe Balbi <balbi@ti.com>
Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com>
---
 drivers/net/usb/cdc-phonet.c           |    6 +++---
 drivers/usb/gadget/function/f_phonet.c |    6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/usb/cdc-phonet.c b/drivers/net/usb/cdc-phonet.c
index 2ec1500..415ce8b 100644
--- a/drivers/net/usb/cdc-phonet.c
+++ b/drivers/net/usb/cdc-phonet.c
@@ -130,7 +130,7 @@ static int rx_submit(struct usbpn_dev *pnd, struct urb *req, gfp_t gfp_flags)
 	struct page *page;
 	int err;
 
-	page = __skb_alloc_page(gfp_flags | __GFP_NOMEMALLOC, NULL);
+	page = __dev_alloc_page(gfp_flags | __GFP_NOMEMALLOC);
 	if (!page)
 		return -ENOMEM;
 
@@ -212,7 +212,7 @@ resubmit:
 	if (page)
 		put_page(page);
 	if (req)
-		rx_submit(pnd, req, GFP_ATOMIC | __GFP_COLD);
+		rx_submit(pnd, req, GFP_ATOMIC);
 }
 
 static int usbpn_close(struct net_device *dev);
@@ -231,7 +231,7 @@ static int usbpn_open(struct net_device *dev)
 	for (i = 0; i < rxq_size; i++) {
 		struct urb *req = usb_alloc_urb(0, GFP_KERNEL);
 
-		if (!req || rx_submit(pnd, req, GFP_KERNEL | __GFP_COLD)) {
+		if (!req || rx_submit(pnd, req, GFP_KERNEL)) {
 			usb_free_urb(req);
 			usbpn_close(dev);
 			return -ENOMEM;
diff --git a/drivers/usb/gadget/function/f_phonet.c b/drivers/usb/gadget/function/f_phonet.c
index b9cfc15..cde7397 100644
--- a/drivers/usb/gadget/function/f_phonet.c
+++ b/drivers/usb/gadget/function/f_phonet.c
@@ -303,7 +303,7 @@ pn_rx_submit(struct f_phonet *fp, struct usb_request *req, gfp_t gfp_flags)
 	struct page *page;
 	int err;
 
-	page = __skb_alloc_page(gfp_flags | __GFP_NOMEMALLOC, NULL);
+	page = __dev_alloc_page(gfp_flags | __GFP_NOMEMALLOC);
 	if (!page)
 		return -ENOMEM;
 
@@ -377,7 +377,7 @@ static void pn_rx_complete(struct usb_ep *ep, struct usb_request *req)
 	if (page)
 		put_page(page);
 	if (req)
-		pn_rx_submit(fp, req, GFP_ATOMIC | __GFP_COLD);
+		pn_rx_submit(fp, req, GFP_ATOMIC);
 }
 
 /*-------------------------------------------------------------------------*/
@@ -437,7 +437,7 @@ static int pn_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
 
 			netif_carrier_on(dev);
 			for (i = 0; i < phonet_rxq_size; i++)
-				pn_rx_submit(fp, fp->out_reqv[i], GFP_ATOMIC | __GFP_COLD);
+				pn_rx_submit(fp, fp->out_reqv[i], GFP_ATOMIC);
 		}
 		spin_unlock(&port->lock);
 		return 0;

^ permalink raw reply related

* [net-next PATCH v2 2/5] cxgb4/cxgb4vf: Replace __skb_alloc_page with __netdev_alloc_page
From: Alexander Duyck @ 2014-11-11 17:11 UTC (permalink / raw)
  To: netdev, linux-usb
  Cc: leedom, hariprasad, donald.c.skidmore, oliver, balbi,
	matthew.vick, mgorman, davem, jeffrey.t.kirsher
In-Reply-To: <20141111170711.15976.44062.stgit@ahduyck-vm-fedora20>

Drop the bloated use of __skb_alloc_page and replace it with
__netdev_alloc_page.  In addition update the one other spot that is
allocating a page so that it allocates with the correct flags.

Cc: Hariprasad S <hariprasad@chelsio.com>
Cc: Casey Leedom <leedom@chelsio.com>
Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com>
---
 drivers/net/ethernet/chelsio/cxgb4/sge.c   |    6 +++---
 drivers/net/ethernet/chelsio/cxgb4vf/sge.c |    7 ++++---
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/chelsio/cxgb4/sge.c b/drivers/net/ethernet/chelsio/cxgb4/sge.c
index 5e1b314..20ee002 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/sge.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/sge.c
@@ -576,7 +576,7 @@ static unsigned int refill_fl(struct adapter *adap, struct sge_fl *q, int n,
 	__be64 *d = &q->desc[q->pidx];
 	struct rx_sw_desc *sd = &q->sdesc[q->pidx];
 
-	gfp |= __GFP_NOWARN | __GFP_COLD;
+	gfp |= __GFP_NOWARN;
 
 	if (s->fl_pg_order == 0)
 		goto alloc_small_pages;
@@ -585,7 +585,7 @@ static unsigned int refill_fl(struct adapter *adap, struct sge_fl *q, int n,
 	 * Prefer large buffers
 	 */
 	while (n) {
-		pg = alloc_pages(gfp | __GFP_COMP, s->fl_pg_order);
+		pg = __dev_alloc_pages(gfp, s->fl_pg_order);
 		if (unlikely(!pg)) {
 			q->large_alloc_failed++;
 			break;       /* fall back to single pages */
@@ -615,7 +615,7 @@ static unsigned int refill_fl(struct adapter *adap, struct sge_fl *q, int n,
 
 alloc_small_pages:
 	while (n--) {
-		pg = __skb_alloc_page(gfp, NULL);
+		pg = __dev_alloc_page(gfp);
 		if (unlikely(!pg)) {
 			q->alloc_failed++;
 			break;
diff --git a/drivers/net/ethernet/chelsio/cxgb4vf/sge.c b/drivers/net/ethernet/chelsio/cxgb4vf/sge.c
index 85036e6..9df40df 100644
--- a/drivers/net/ethernet/chelsio/cxgb4vf/sge.c
+++ b/drivers/net/ethernet/chelsio/cxgb4vf/sge.c
@@ -602,6 +602,8 @@ static unsigned int refill_fl(struct adapter *adapter, struct sge_fl *fl,
 	 */
 	BUG_ON(fl->avail + n > fl->size - FL_PER_EQ_UNIT);
 
+	gfp |= __GFP_NOWARN;
+
 	/*
 	 * If we support large pages, prefer large buffers and fail over to
 	 * small pages if we can't allocate large pages to satisfy the refill.
@@ -612,8 +614,7 @@ static unsigned int refill_fl(struct adapter *adapter, struct sge_fl *fl,
 		goto alloc_small_pages;
 
 	while (n) {
-		page = alloc_pages(gfp | __GFP_COMP | __GFP_NOWARN,
-				   FL_PG_ORDER);
+		page = __dev_alloc_pages(gfp, FL_PG_ORDER);
 		if (unlikely(!page)) {
 			/*
 			 * We've failed inour attempt to allocate a "large
@@ -657,7 +658,7 @@ static unsigned int refill_fl(struct adapter *adapter, struct sge_fl *fl,
 
 alloc_small_pages:
 	while (n--) {
-		page = __skb_alloc_page(gfp | __GFP_NOWARN, NULL);
+		page = __dev_alloc_page(gfp);
 		if (unlikely(!page)) {
 			fl->alloc_failed++;
 			break;

^ permalink raw reply related

* [net-next PATCH v2 1/5] net: Add device Rx page allocation function
From: Alexander Duyck @ 2014-11-11 17:10 UTC (permalink / raw)
  To: netdev, linux-usb
  Cc: leedom, hariprasad, donald.c.skidmore, oliver, balbi,
	matthew.vick, mgorman, davem, jeffrey.t.kirsher
In-Reply-To: <20141111170711.15976.44062.stgit@ahduyck-vm-fedora20>

This patch implements __dev_alloc_pages and __dev_alloc_page.  These are
meant to replace the __skb_alloc_pages and __skb_alloc_page functions.  The
reason for doing this is that it occurred to me that __skb_alloc_page is
supposed to be passed an sk_buff pointer, but it is NULL in all cases where
it is used.  Worse is that in the case of ixgbe it is passed NULL via the
sk_buff pointer in the rx_buffer info structure which means the compiler is
not correctly stripping it out.

The naming for these functions is based on dev_alloc_skb and __dev_alloc_skb.
There was originally a netdev_alloc_page, however that was passed a
net_device pointer and this function is not so I thought it best to follow
that naming scheme since that is the same difference between dev_alloc_skb
and netdev_alloc_skb.

In the case of anything greater than order 0 it is assumed that we want a
compound page so __GFP_COMP is set for all allocations as we expect a
compound page when assigning a page frag.

The other change in this patch is to exploit the behaviors of the page
allocator in how it handles flags.  So for example we can always set
__GFP_COMP and __GFP_MEMALLOC since they are ignored if they are not
applicable or are overridden by another flag.

Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com>
---
 include/linux/skbuff.h |   48 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 103fbe8..2e5221f 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -2185,6 +2185,54 @@ static inline struct sk_buff *netdev_alloc_skb_ip_align(struct net_device *dev,
 }
 
 /**
+ * __dev_alloc_pages - allocate page for network Rx
+ * @gfp_mask: allocation priority. Set __GFP_NOMEMALLOC if not for network Rx
+ * @order: size of the allocation
+ *
+ * Allocate a new page.
+ *
+ * %NULL is returned if there is no free memory.
+*/
+static inline struct page *__dev_alloc_pages(gfp_t gfp_mask,
+					     unsigned int order)
+{
+	/* This piece of code contains several assumptions.
+	 * 1.  This is for device Rx, therefor a cold page is preferred.
+	 * 2.  The expectation is the user wants a compound page.
+	 * 3.  If requesting a order 0 page it will not be compound
+	 *     due to the check to see if order has a value in prep_new_page
+	 * 4.  __GFP_MEMALLOC is ignored if __GFP_NOMEMALLOC is set due to
+	 *     code in gfp_to_alloc_flags that should be enforcing this.
+	 */
+	gfp_mask |= __GFP_COLD | __GFP_COMP | __GFP_MEMALLOC;
+
+	return alloc_pages_node(NUMA_NO_NODE, gfp_mask, order);
+}
+
+static inline struct page *dev_alloc_pages(unsigned int order)
+{
+	return __dev_alloc_pages(GFP_ATOMIC, order);
+}
+
+/**
+ * __dev_alloc_page - allocate a page for network Rx
+ * @gfp_mask: allocation priority. Set __GFP_NOMEMALLOC if not for network Rx
+ *
+ * Allocate a new page.
+ *
+ * %NULL is returned if there is no free memory.
+ */
+static inline struct page *__dev_alloc_page(gfp_t gfp_mask)
+{
+	return __dev_alloc_pages(gfp_mask, 0);
+}
+
+static inline struct page *dev_alloc_page(void)
+{
+	return __dev_alloc_page(GFP_ATOMIC);
+}
+
+/**
  *	__skb_alloc_pages - allocate pages for ps-rx on a skb and preserve pfmemalloc data
  *	@gfp_mask: alloc_pages_node mask. Set __GFP_NOMEMALLOC if not for network packet RX
  *	@skb: skb to set pfmemalloc on if __GFP_MEMALLOC is used

^ permalink raw reply related

* [net-next PATCH v2 0/5] Replace __skb_alloc_pages with simpler function
From: Alexander Duyck @ 2014-11-11 17:10 UTC (permalink / raw)
  To: netdev-u79uwXL29TY76Z2rM5mHXA, linux-usb-u79uwXL29TY76Z2rM5mHXA
  Cc: leedom-ut6Up61K2wZBDgjK7y7TUQ, hariprasad-ut6Up61K2wZBDgjK7y7TUQ,
	donald.c.skidmore-ral2JQCrhuEAvxtiuMwx3w,
	oliver-GvhC2dPhHPQdnm+yROfE0A, balbi-l0cyMroinI0,
	matthew.vick-ral2JQCrhuEAvxtiuMwx3w, mgorman-l3A5Bk7waGM,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q,
	jeffrey.t.kirsher-ral2JQCrhuEAvxtiuMwx3w

This patch series replaces __skb_alloc_pages with a much simpler function,
__dev_alloc_pages.  The main difference between the two is that
__skb_alloc_pages had an sk_buff pointer that was being passed as NULL in
call places where it was called.  In a couple of cases the NULL was passed
by variable and this led to unnecessary code being run.

As such in order to simplify things the __dev_alloc_pages call only takes a
mask and the page order being requested.  In addition it takes advantage of
several behaviors already built into the page allocator so that it can just
set GFP flags unconditionally.

v2: Renamed functions to dev_alloc_page(s) instead of netdev_alloc_page(s)
    Removed __GFP_COLD flag from usb code as it was redundant

---

Alexander Duyck (5):
      net: Add device Rx page allocation function
      cxgb4/cxgb4vf: Replace __skb_alloc_page with __netdev_alloc_page
      phonet: Replace calls to __skb_alloc_page with __netdev_alloc_page
      fm10k/igb/ixgbe: Replace __skb_alloc_page with netdev_alloc_page
      net: Remove __skb_alloc_page and __skb_alloc_pages


 drivers/net/ethernet/chelsio/cxgb4/sge.c      |    6 +-
 drivers/net/ethernet/chelsio/cxgb4vf/sge.c    |    7 ++-
 drivers/net/ethernet/intel/fm10k/fm10k_main.c |    2 -
 drivers/net/ethernet/intel/igb/igb_main.c     |    2 -
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    3 -
 drivers/net/usb/cdc-phonet.c                  |    6 +-
 drivers/usb/gadget/function/f_phonet.c        |    6 +-
 include/linux/skbuff.h                        |   61 ++++++++++++++-----------
 8 files changed, 49 insertions(+), 44 deletions(-)

--
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox