[PATCH 4/4] phylib: Allow reading and writing a mii bus from atomic context.

[PATCH 4/4] phylib: Allow reading and writing a mii bus from atomic context.

[Richard Cochran]

In order to support hardware time stamping from a PHY, it is necessary to
read from the PHY while running in_interrupt(). This patch allows a mii
bus to operate in an atomic context. An mii_bus driver may declare itself
capable for this mode. Drivers which do not do this will remain with the
default that bus operations may sleep.

Before commit 35b5f6b1a82b5c586e0b24c711dc6ba944e88ef1 mii bus
operations were protected with spin locks. That commit replaced the
locks with mutexs in order to accommodate i2c buses that need to
sleep. Thus, this patch restores the original behavior as a run time
option.

Signed-off-by: Richard Cochran <richard.cochran@omicron.at>
---
 drivers/net/fsl_pq_mdio.c  |    4 +-
 drivers/net/phy/mdio_bus.c |   45 +++++++++++++++++++++++++++++++++++++------
 include/linux/phy.h        |   15 ++++++++++++-
 3 files changed, 53 insertions(+), 11 deletions(-)

diff --git a/drivers/net/fsl_pq_mdio.c b/drivers/net/fsl_pq_mdio.c
index b4c41d7..0b34f50 100644
--- a/drivers/net/fsl_pq_mdio.c
+++ b/drivers/net/fsl_pq_mdio.c
@@ -145,7 +145,7 @@ static int fsl_pq_mdio_reset(struct mii_bus *bus)
 	struct fsl_pq_mdio __iomem *regs = fsl_pq_mdio_get_regs(bus);
 	int timeout = PHY_INIT_TIMEOUT;
 
-	mutex_lock(&bus->mdio_lock);
+	mdiobus_lock(bus);
 
 	/* Reset the management interface */
 	out_be32(&regs->miimcfg, MIIMCFG_RESET);
@@ -157,7 +157,7 @@ static int fsl_pq_mdio_reset(struct mii_bus *bus)
 	while ((in_be32(&regs->miimind) & MIIMIND_BUSY) && timeout--)
 		cpu_relax();
 
-	mutex_unlock(&bus->mdio_lock);
+	mdiobus_unlock(bus);
 
 	if (timeout < 0) {
 		printk(KERN_ERR "%s: The MII Bus is stuck!\n",
diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index 6a6b819..ad6bed8 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -37,6 +37,32 @@
 #include <asm/uaccess.h>
 
 /**
+ * mdiobus_lock - locks a given bus for read or write operations.
+ * @bus: target mii_bus
+ */
+void mdiobus_lock(struct mii_bus *bus)
+{
+	if (MDIOBUS_SLEEPS_RW == bus->locktype)
+		mutex_lock(&bus->lock.m);
+	else
+		spin_lock(&bus->lock.s);
+}
+EXPORT_SYMBOL(mdiobus_lock);
+
+/**
+ * mdiobus_unlock - unlocks a given bus for read or write operations.
+ * @bus: target mii_bus
+ */
+void mdiobus_unlock(struct mii_bus *bus)
+{
+	if (MDIOBUS_SLEEPS_RW == bus->locktype)
+		mutex_unlock(&bus->lock.m);
+	else
+		spin_unlock(&bus->lock.s);
+}
+EXPORT_SYMBOL(mdiobus_unlock);
+
+/**
  * mdiobus_alloc - allocate a mii_bus structure
  *
  * Description: called by a bus driver to allocate an mii_bus
@@ -107,7 +133,10 @@ int mdiobus_register(struct mii_bus *bus)
 		return -EINVAL;
 	}
 
-	mutex_init(&bus->mdio_lock);
+	if (MDIOBUS_SLEEPS_RW == bus->locktype)
+		mutex_init(&bus->lock.m);
+	else
+		spin_lock_init(&bus->lock.s);
 
 	if (bus->reset)
 		bus->reset(bus);
@@ -212,11 +241,12 @@ int mdiobus_read(struct mii_bus *bus, int addr, u32 regnum)
 {
 	int retval;
 
-	BUG_ON(in_interrupt());
+	if (MDIOBUS_SLEEPS_RW == bus->locktype)
+		BUG_ON(in_interrupt());
 
-	mutex_lock(&bus->mdio_lock);
+	mdiobus_lock(bus);
 	retval = bus->read(bus, addr, regnum);
-	mutex_unlock(&bus->mdio_lock);
+	mdiobus_unlock(bus);
 
 	return retval;
 }
@@ -237,11 +267,12 @@ int mdiobus_write(struct mii_bus *bus, int addr, u32 regnum, u16 val)
 {
 	int err;
 
-	BUG_ON(in_interrupt());
+	if (MDIOBUS_SLEEPS_RW == bus->locktype)
+		BUG_ON(in_interrupt());
 
-	mutex_lock(&bus->mdio_lock);
+	mdiobus_lock(bus);
 	err = bus->write(bus, addr, regnum, val);
-	mutex_unlock(&bus->mdio_lock);
+	mdiobus_unlock(bus);
 
 	return err;
 }
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 7a8caac..93ea55f 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -98,11 +98,20 @@ struct mii_bus {
 	int (*write)(struct mii_bus *bus, int phy_id, int regnum, u16 val);
 	int (*reset)(struct mii_bus *bus);
 
+	/* Indicates whether bus may be used from an atomic context. */
+	enum {
+		MDIOBUS_SLEEPS_RW,
+		MDIOBUS_ATOMIC_RW
+	} locktype;
+
 	/*
-	 * A lock to ensure that only one thing can read/write
+	 * A lock or mutex to ensure that only one thing can read/write
 	 * the MDIO bus at a time
 	 */
-	struct mutex mdio_lock;
+	union {
+		struct mutex m;
+		spinlock_t s;
+	} lock;
 
 	struct device *parent;
 	enum {
@@ -127,6 +136,8 @@ struct mii_bus {
 };
 #define to_mii_bus(d) container_of(d, struct mii_bus, dev)
 
+void mdiobus_lock(struct mii_bus *bus);
+void mdiobus_unlock(struct mii_bus *bus);
 struct mii_bus *mdiobus_alloc(void);
 int mdiobus_register(struct mii_bus *bus);
 void mdiobus_unregister(struct mii_bus *bus);
-- 
1.7.0.4

[PATCH v2 0/4] Extend Time Stamping

[Richard Cochran]

This patch set extends the packet time stamping capabilites of the
network stack in two ways.

1. The first patch presents a work-around for the TX software time
   stamping fallback problem cited in cd4d8fdad1f1. The idea is to add
   two inline functions into each MAC driver. The functions will act
   as hooks for current (and possible future) time stamping needs,
   once they are placed correctly within each MAC driver.

2. The other patches prepare the way for PHY drivers to offer time
   stamping.

I am preparing a new round of patches for PTP support, but it will
require the changes in this patch set in order to function. Thus I
would like to have this patch set reviewed (and hopefully merged) in
order to go forward.

Thanks,
Richard

* Patch ChangeLog
** v2
   Removed the CONFIG option for the driver hooks.


Richard Cochran (4):
  net: add driver hooks for time stamping.
  phylib: add a way to make PHY time stamps possible.
  phylib: preserve ifreq parameter when calling generic phy_mii_ioctl()
  phylib: Allow reading and writing a mii bus from atomic context.

 drivers/net/arm/ixp4xx_eth.c           |    3 +-
 drivers/net/au1000_eth.c               |    2 +-
 drivers/net/bcm63xx_enet.c             |    2 +-
 drivers/net/cpmac.c                    |    5 +--
 drivers/net/dnet.c                     |    2 +-
 drivers/net/ethoc.c                    |    2 +-
 drivers/net/fec.c                      |    2 +-
 drivers/net/fec_mpc52xx.c              |    2 +-
 drivers/net/fs_enet/fs_enet-main.c     |    3 +-
 drivers/net/fsl_pq_mdio.c              |    4 +-
 drivers/net/gianfar.c                  |    2 +-
 drivers/net/macb.c                     |    2 +-
 drivers/net/mv643xx_eth.c              |    2 +-
 drivers/net/octeon/octeon_mgmt.c       |    2 +-
 drivers/net/phy/mdio_bus.c             |   45 ++++++++++++++++++---
 drivers/net/phy/phy.c                  |    8 +++-
 drivers/net/sb1250-mac.c               |    2 +-
 drivers/net/sh_eth.c                   |    2 +-
 drivers/net/smsc911x.c                 |    2 +-
 drivers/net/smsc9420.c                 |    2 +-
 drivers/net/stmmac/stmmac_main.c       |   22 ++++-------
 drivers/net/tc35815.c                  |    2 +-
 drivers/net/tg3.c                      |    2 +-
 drivers/net/ucc_geth.c                 |    2 +-
 drivers/staging/octeon/ethernet-mdio.c |    2 +-
 include/linux/phy.h                    |   24 ++++++++++-
 include/linux/skbuff.h                 |   65 ++++++++++++++++++++++++++++++++
 net/Kconfig                            |   11 +++++
 net/dsa/slave.c                        |    3 +-
 29 files changed, 175 insertions(+), 54 deletions(-)

[PATCH 1/4] net: add driver hooks for time stamping.

[Richard Cochran]

This patch adds hooks for transmit and receive time stamps. The
transmit hook allows a software fallback for transmit time stamps,
for MACs lacking time stamping hardware. The receive hook does not yet
have any effect, but it prepares the way for hardware time stamping
in PHY devices. Using the hooks will still require adding two inline
function calls to each MAC driver.

Signed-off-by: Richard Cochran <richard.cochran@omicron.at>
---
 include/linux/skbuff.h |   36 ++++++++++++++++++++++++++++++++++++
 1 files changed, 36 insertions(+), 0 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index ac74ee0..75323fe 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -29,6 +29,7 @@
 #include <linux/rcupdate.h>
 #include <linux/dmaengine.h>
 #include <linux/hrtimer.h>
+#include <linux/phy.h>
 
 /* Don't change this without changing skb_csum_unnecessary! */
 #define CHECKSUM_NONE 0
@@ -1947,6 +1948,41 @@ static inline ktime_t net_invalid_timestamp(void)
 extern void skb_tstamp_tx(struct sk_buff *orig_skb,
 			struct skb_shared_hwtstamps *hwtstamps);
 
+static inline void sw_tx_timestamp(struct sk_buff *skb)
+{
+	union skb_shared_tx *shtx = skb_tx(skb);
+	if (shtx->software && !shtx->in_progress)
+		skb_tstamp_tx(skb, NULL);
+}
+
+/**
+ * skb_tx_timestamp() - Driver hook for transmit timestamping
+ *
+ * Ethernet MAC Drivers should call this function in their hard_xmit()
+ * function as soon as possible after giving the sk_buff to the MAC
+ * hardware, but before freeing the sk_buff.
+ *
+ * @phy: The port's phy_device. Pass NULL if this is not available.
+ * @skb: A socket buffer.
+ */
+static inline void skb_tx_timestamp(struct phy_device *phy, struct sk_buff *skb)
+{
+	sw_tx_timestamp(skb);
+}
+
+/**
+ * skb_rx_timestamp() - Driver hook for receive timestamping
+ *
+ * Ethernet MAC Drivers should call this function in their NAPI poll()
+ * function immediately before calling eth_type_trans().
+ *
+ * @phy: The port's phy_device. Pass NULL if this is not available.
+ * @skb: A socket buffer.
+ */
+static inline void skb_rx_timestamp(struct phy_device *phy, struct sk_buff *skb)
+{
+}
+
 extern __sum16 __skb_checksum_complete_head(struct sk_buff *skb, int len);
 extern __sum16 __skb_checksum_complete(struct sk_buff *skb);
 
-- 
1.7.0.4

[PATCH 2/4] phylib: add a way to make PHY time stamps possible.

[Richard Cochran]

This patch adds a new networking option to allow hardware time stamps
from PHY devices. Using PHY time stamps will still require adding two
inline function calls to each MAC driver. The CONFIG option makes
these calls safe to add, since the calls become NOOPs when the option
is disabled.

The patch also adds phylib driver methods for the SIOCSHWTSTAMP ioctl
and callbacks for transmit and receive time stamping. Drivers may
optionally implement these functions.

Signed-off-by: Richard Cochran <richard.cochran@omicron.at>
---
 include/linux/phy.h    |    7 +++++++
 include/linux/skbuff.h |   29 +++++++++++++++++++++++++++++
 net/Kconfig            |   11 +++++++++++
 3 files changed, 47 insertions(+), 0 deletions(-)

diff --git a/include/linux/phy.h b/include/linux/phy.h
index 987e111..3566bde 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -234,6 +234,8 @@ enum phy_state {
 	PHY_RESUMING
 };
 
+struct sk_buff;
+
 /* phy_device: An instance of a PHY
  *
  * drv: Pointer to the driver for this PHY instance
@@ -402,6 +404,11 @@ struct phy_driver {
 	/* Clears up any memory if needed */
 	void (*remove)(struct phy_device *phydev);
 
+	/* Handles SIOCSHWTSTAMP ioctl for hardware time stamping. */
+	int (*hwtstamp)(struct phy_device *phydev, struct ifreq *ifr);
+	int (*rxtstamp)(struct phy_device *phydev, struct sk_buff *skb);
+	int (*txtstamp)(struct phy_device *phydev, struct sk_buff *skb);
+
 	struct device_driver driver;
 };
 #define to_phy_driver(d) container_of(d, struct phy_driver, driver)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 75323fe..8905508 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -1955,6 +1955,33 @@ static inline void sw_tx_timestamp(struct sk_buff *skb)
 		skb_tstamp_tx(skb, NULL);
 }
 
+#ifdef CONFIG_NETWORK_PHY_TIMESTAMPING
+
+static inline void phy_tx_timestamp(struct phy_device *phy, struct sk_buff *skb)
+{
+	union skb_shared_tx *shtx = skb_tx(skb);
+	if (shtx->hardware && phy && phy->drv->txtstamp)
+		phy->drv->txtstamp(phy, skb);
+}
+
+static inline void phy_rx_timestamp(struct phy_device *phy, struct sk_buff *skb)
+{
+	if (phy && phy->drv->rxtstamp)
+		phy->drv->rxtstamp(phy, skb);
+}
+
+#else /* CONFIG_NETWORK_PHY_TIMESTAMPING */
+
+static inline void phy_tx_timestamp(struct phy_device *phy, struct sk_buff *skb)
+{
+}
+
+static inline void phy_rx_timestamp(struct phy_device *phy, struct sk_buff *skb)
+{
+}
+
+#endif /* !CONFIG_NETWORK_PHY_TIMESTAMPING */
+
 /**
  * skb_tx_timestamp() - Driver hook for transmit timestamping
  *
@@ -1967,6 +1994,7 @@ static inline void sw_tx_timestamp(struct sk_buff *skb)
  */
 static inline void skb_tx_timestamp(struct phy_device *phy, struct sk_buff *skb)
 {
+	phy_tx_timestamp(phy, skb);
 	sw_tx_timestamp(skb);
 }
 
@@ -1981,6 +2009,7 @@ static inline void skb_tx_timestamp(struct phy_device *phy, struct sk_buff *skb)
  */
 static inline void skb_rx_timestamp(struct phy_device *phy, struct sk_buff *skb)
 {
+	phy_rx_timestamp(phy, skb);
 }
 
 extern __sum16 __skb_checksum_complete_head(struct sk_buff *skb, int len);
diff --git a/net/Kconfig b/net/Kconfig
index 0d68b40..3fa7ae3 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -86,6 +86,17 @@ config NETWORK_SECMARK
 	  to nfmark, but designated for security purposes.
 	  If you are unsure how to answer this question, answer N.
 
+config NETWORK_PHY_TIMESTAMPING
+	bool "Timestamping in PHY devices"
+	depends on EXPERIMENTAL
+	help
+	  This allows timestamping of network packets by PHYs with
+	  hardware timestamping capabilities. This option adds some
+	  overhead in the transmit and receive paths. Note that this
+	  option also requires support in the MAC driver.
+
+	  If you are unsure how to answer this question, answer N.
+
 menuconfig NETFILTER
 	bool "Network packet filtering framework (Netfilter)"
 	---help---
-- 
1.7.0.4

[PATCH 3/4] phylib: preserve ifreq parameter when calling generic phy_mii_ioctl()

[Richard Cochran]

The phy_mii_ioctl() function unnecessarily throws away the original ifreq.
We need access to the ifreq in order to support PHYs that can perform
hardware time stamping.

Two maverick drivers filter the ioctl commands passed to phy_mii_ioctl().
This is unnecessary since phylib will check the command in any case.

Signed-off-by: Richard Cochran <richard.cochran@omicron.at>
---
 drivers/net/arm/ixp4xx_eth.c           |    3 ++-
 drivers/net/au1000_eth.c               |    2 +-
 drivers/net/bcm63xx_enet.c             |    2 +-
 drivers/net/cpmac.c                    |    5 +----
 drivers/net/dnet.c                     |    2 +-
 drivers/net/ethoc.c                    |    2 +-
 drivers/net/fec.c                      |    2 +-
 drivers/net/fec_mpc52xx.c              |    2 +-
 drivers/net/fs_enet/fs_enet-main.c     |    3 +--
 drivers/net/gianfar.c                  |    2 +-
 drivers/net/macb.c                     |    2 +-
 drivers/net/mv643xx_eth.c              |    2 +-
 drivers/net/octeon/octeon_mgmt.c       |    2 +-
 drivers/net/phy/phy.c                  |    8 +++++++-
 drivers/net/sb1250-mac.c               |    2 +-
 drivers/net/sh_eth.c                   |    2 +-
 drivers/net/smsc911x.c                 |    2 +-
 drivers/net/smsc9420.c                 |    2 +-
 drivers/net/stmmac/stmmac_main.c       |   22 ++++++++--------------
 drivers/net/tc35815.c                  |    2 +-
 drivers/net/tg3.c                      |    2 +-
 drivers/net/ucc_geth.c                 |    2 +-
 drivers/staging/octeon/ethernet-mdio.c |    2 +-
 include/linux/phy.h                    |    2 +-
 net/dsa/slave.c                        |    3 +--
 25 files changed, 39 insertions(+), 43 deletions(-)

diff --git a/drivers/net/arm/ixp4xx_eth.c b/drivers/net/arm/ixp4xx_eth.c
index ee2f842..4f1cc71 100644
--- a/drivers/net/arm/ixp4xx_eth.c
+++ b/drivers/net/arm/ixp4xx_eth.c
@@ -782,7 +782,8 @@ static int eth_ioctl(struct net_device *dev, struct ifreq *req, int cmd)
 
 	if (!netif_running(dev))
 		return -EINVAL;
-	return phy_mii_ioctl(port->phydev, if_mii(req), cmd);
+
+	return phy_mii_ioctl(port->phydev, req, cmd);
 }
 
 /* ethtool support */
diff --git a/drivers/net/au1000_eth.c b/drivers/net/au1000_eth.c
index ece6128..386d4fe 100644
--- a/drivers/net/au1000_eth.c
+++ b/drivers/net/au1000_eth.c
@@ -978,7 +978,7 @@ static int au1000_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
 	if (!aup->phy_dev)
 		return -EINVAL; /* PHY not controllable */
 
-	return phy_mii_ioctl(aup->phy_dev, if_mii(rq), cmd);
+	return phy_mii_ioctl(aup->phy_dev, rq, cmd);
 }
 
 static const struct net_device_ops au1000_netdev_ops = {
diff --git a/drivers/net/bcm63xx_enet.c b/drivers/net/bcm63xx_enet.c
index faf5add..0d2c5da 100644
--- a/drivers/net/bcm63xx_enet.c
+++ b/drivers/net/bcm63xx_enet.c
@@ -1496,7 +1496,7 @@ static int bcm_enet_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
 	if (priv->has_phy) {
 		if (!priv->phydev)
 			return -ENODEV;
-		return phy_mii_ioctl(priv->phydev, if_mii(rq), cmd);
+		return phy_mii_ioctl(priv->phydev, rq, cmd);
 	} else {
 		struct mii_if_info mii;
 
diff --git a/drivers/net/cpmac.c b/drivers/net/cpmac.c
index 1756d28..cdb05bb 100644
--- a/drivers/net/cpmac.c
+++ b/drivers/net/cpmac.c
@@ -846,11 +846,8 @@ static int cpmac_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
 		return -EINVAL;
 	if (!priv->phy)
 		return -EINVAL;
-	if ((cmd == SIOCGMIIPHY) || (cmd == SIOCGMIIREG) ||
-	    (cmd == SIOCSMIIREG))
-		return phy_mii_ioctl(priv->phy, if_mii(ifr), cmd);
 
-	return -EOPNOTSUPP;
+	return phy_mii_ioctl(priv->phy, ifr, cmd);
 }
 
 static int cpmac_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
diff --git a/drivers/net/dnet.c b/drivers/net/dnet.c
index 8b0f50b..4ea7141 100644
--- a/drivers/net/dnet.c
+++ b/drivers/net/dnet.c
@@ -797,7 +797,7 @@ static int dnet_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
 	if (!phydev)
 		return -ENODEV;
 
-	return phy_mii_ioctl(phydev, if_mii(rq), cmd);
+	return phy_mii_ioctl(phydev, rq, cmd);
 }
 
 static void dnet_get_drvinfo(struct net_device *dev,
diff --git a/drivers/net/ethoc.c b/drivers/net/ethoc.c
index 37ce8ac..d9f3106 100644
--- a/drivers/net/ethoc.c
+++ b/drivers/net/ethoc.c
@@ -732,7 +732,7 @@ static int ethoc_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
 		phy = priv->phy;
 	}
 
-	return phy_mii_ioctl(phy, mdio, cmd);
+	return phy_mii_ioctl(phy, ifr, cmd);
 }
 
 static int ethoc_config(struct net_device *dev, struct ifmap *map)
diff --git a/drivers/net/fec.c b/drivers/net/fec.c
index b4afd7a..1670866 100644
--- a/drivers/net/fec.c
+++ b/drivers/net/fec.c
@@ -828,7 +828,7 @@ static int fec_enet_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
 	if (!phydev)
 		return -ENODEV;
 
-	return phy_mii_ioctl(phydev, if_mii(rq), cmd);
+	return phy_mii_ioctl(phydev, rq, cmd);
 }
 
 static void fec_enet_free_buffers(struct net_device *dev)
diff --git a/drivers/net/fec_mpc52xx.c b/drivers/net/fec_mpc52xx.c
index 25e6cc6..fdbf148 100644
--- a/drivers/net/fec_mpc52xx.c
+++ b/drivers/net/fec_mpc52xx.c
@@ -826,7 +826,7 @@ static int mpc52xx_fec_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
 	if (!priv->phydev)
 		return -ENOTSUPP;
 
-	return phy_mii_ioctl(priv->phydev, if_mii(rq), cmd);
+	return phy_mii_ioctl(priv->phydev, rq, cmd);
 }
 
 static const struct net_device_ops mpc52xx_fec_netdev_ops = {
diff --git a/drivers/net/fs_enet/fs_enet-main.c b/drivers/net/fs_enet/fs_enet-main.c
index 309a0ea..f08cff9 100644
--- a/drivers/net/fs_enet/fs_enet-main.c
+++ b/drivers/net/fs_enet/fs_enet-main.c
@@ -963,12 +963,11 @@ static const struct ethtool_ops fs_ethtool_ops = {
 static int fs_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
 {
 	struct fs_enet_private *fep = netdev_priv(dev);
-	struct mii_ioctl_data *mii = (struct mii_ioctl_data *)&rq->ifr_data;
 
 	if (!netif_running(dev))
 		return -EINVAL;
 
-	return phy_mii_ioctl(fep->phydev, mii, cmd);
+	return phy_mii_ioctl(fep->phydev, rq, cmd);
 }
 
 extern int fs_mii_connect(struct net_device *dev);
diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
index fccb7a3..1d16d51 100644
--- a/drivers/net/gianfar.c
+++ b/drivers/net/gianfar.c
@@ -847,7 +847,7 @@ static int gfar_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
 	if (!priv->phydev)
 		return -ENODEV;
 
-	return phy_mii_ioctl(priv->phydev, if_mii(rq), cmd);
+	return phy_mii_ioctl(priv->phydev, rq, cmd);
 }
 
 static unsigned int reverse_bitmap(unsigned int bit_map, unsigned int max_qs)
diff --git a/drivers/net/macb.c b/drivers/net/macb.c
index 40797fb..ff2f158 100644
--- a/drivers/net/macb.c
+++ b/drivers/net/macb.c
@@ -1082,7 +1082,7 @@ static int macb_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
 	if (!phydev)
 		return -ENODEV;
 
-	return phy_mii_ioctl(phydev, if_mii(rq), cmd);
+	return phy_mii_ioctl(phydev, rq, cmd);
 }
 
 static const struct net_device_ops macb_netdev_ops = {
diff --git a/drivers/net/mv643xx_eth.c b/drivers/net/mv643xx_eth.c
index 82b720f..0561425 100644
--- a/drivers/net/mv643xx_eth.c
+++ b/drivers/net/mv643xx_eth.c
@@ -2457,7 +2457,7 @@ static int mv643xx_eth_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
 	struct mv643xx_eth_private *mp = netdev_priv(dev);
 
 	if (mp->phy != NULL)
-		return phy_mii_ioctl(mp->phy, if_mii(ifr), cmd);
+		return phy_mii_ioctl(mp->phy, ifr, cmd);
 
 	return -EOPNOTSUPP;
 }
diff --git a/drivers/net/octeon/octeon_mgmt.c b/drivers/net/octeon/octeon_mgmt.c
index f4a0f08..b264f0f 100644
--- a/drivers/net/octeon/octeon_mgmt.c
+++ b/drivers/net/octeon/octeon_mgmt.c
@@ -620,7 +620,7 @@ static int octeon_mgmt_ioctl(struct net_device *netdev,
 	if (!p->phydev)
 		return -EINVAL;
 
-	return phy_mii_ioctl(p->phydev, if_mii(rq), cmd);
+	return phy_mii_ioctl(p->phydev, rq, cmd);
 }
 
 static void octeon_mgmt_adjust_link(struct net_device *netdev)
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 64be466..5130db8 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -309,8 +309,9 @@ EXPORT_SYMBOL(phy_ethtool_gset);
  * current state.  Use at own risk.
  */
 int phy_mii_ioctl(struct phy_device *phydev,
-		struct mii_ioctl_data *mii_data, int cmd)
+		struct ifreq *ifr, int cmd)
 {
+	struct mii_ioctl_data *mii_data = if_mii(ifr);
 	u16 val = mii_data->val_in;
 
 	switch (cmd) {
@@ -360,6 +361,11 @@ int phy_mii_ioctl(struct phy_device *phydev,
 		}
 		break;
 
+	case SIOCSHWTSTAMP:
+		if (phydev->drv->hwtstamp)
+			return phydev->drv->hwtstamp(phydev, ifr);
+		/* fall through */
+
 	default:
 		return -EOPNOTSUPP;
 	}
diff --git a/drivers/net/sb1250-mac.c b/drivers/net/sb1250-mac.c
index 1f3acc3..e585c3f 100644
--- a/drivers/net/sb1250-mac.c
+++ b/drivers/net/sb1250-mac.c
@@ -2532,7 +2532,7 @@ static int sbmac_mii_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
 	if (!netif_running(dev) || !sc->phy_dev)
 		return -EINVAL;
 
-	return phy_mii_ioctl(sc->phy_dev, if_mii(rq), cmd);
+	return phy_mii_ioctl(sc->phy_dev, rq, cmd);
 }
 
 static int sbmac_close(struct net_device *dev)
diff --git a/drivers/net/sh_eth.c b/drivers/net/sh_eth.c
index 501a55f..8279f8e 100644
--- a/drivers/net/sh_eth.c
+++ b/drivers/net/sh_eth.c
@@ -1233,7 +1233,7 @@ static int sh_eth_do_ioctl(struct net_device *ndev, struct ifreq *rq,
 	if (!phydev)
 		return -ENODEV;
 
-	return phy_mii_ioctl(phydev, if_mii(rq), cmd);
+	return phy_mii_ioctl(phydev, rq, cmd);
 }
 
 #if defined(SH_ETH_HAS_TSU)
diff --git a/drivers/net/smsc911x.c b/drivers/net/smsc911x.c
index cc55974..56dc2ff 100644
--- a/drivers/net/smsc911x.c
+++ b/drivers/net/smsc911x.c
@@ -1538,7 +1538,7 @@ static int smsc911x_do_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
 	if (!netif_running(dev) || !pdata->phy_dev)
 		return -EINVAL;
 
-	return phy_mii_ioctl(pdata->phy_dev, if_mii(ifr), cmd);
+	return phy_mii_ioctl(pdata->phy_dev, ifr, cmd);
 }
 
 static int
diff --git a/drivers/net/smsc9420.c b/drivers/net/smsc9420.c
index 6cdee6a..b09ee1c 100644
--- a/drivers/net/smsc9420.c
+++ b/drivers/net/smsc9420.c
@@ -245,7 +245,7 @@ static int smsc9420_do_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
 	if (!netif_running(dev) || !pd->phy_dev)
 		return -EINVAL;
 
-	return phy_mii_ioctl(pd->phy_dev, if_mii(ifr), cmd);
+	return phy_mii_ioctl(pd->phy_dev, ifr, cmd);
 }
 
 static int smsc9420_ethtool_get_settings(struct net_device *dev,
diff --git a/drivers/net/stmmac/stmmac_main.c b/drivers/net/stmmac/stmmac_main.c
index a31d580..acf0616 100644
--- a/drivers/net/stmmac/stmmac_main.c
+++ b/drivers/net/stmmac/stmmac_main.c
@@ -1437,24 +1437,18 @@ static void stmmac_poll_controller(struct net_device *dev)
 static int stmmac_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
 {
 	struct stmmac_priv *priv = netdev_priv(dev);
-	int ret = -EOPNOTSUPP;
+	int ret;
 
 	if (!netif_running(dev))
 		return -EINVAL;
 
-	switch (cmd) {
-	case SIOCGMIIPHY:
-	case SIOCGMIIREG:
-	case SIOCSMIIREG:
-		if (!priv->phydev)
-			return -EINVAL;
-
-		spin_lock(&priv->lock);
-		ret = phy_mii_ioctl(priv->phydev, if_mii(rq), cmd);
-		spin_unlock(&priv->lock);
-	default:
-		break;
-	}
+	if (!priv->phydev)
+		return -EINVAL;
+
+	spin_lock(&priv->lock);
+	ret = phy_mii_ioctl(priv->phydev, rq, cmd);
+	spin_unlock(&priv->lock);
+
 	return ret;
 }
 
diff --git a/drivers/net/tc35815.c b/drivers/net/tc35815.c
index be08b75..99e423a 100644
--- a/drivers/net/tc35815.c
+++ b/drivers/net/tc35815.c
@@ -2066,7 +2066,7 @@ static int tc35815_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
 		return -EINVAL;
 	if (!lp->phy_dev)
 		return -ENODEV;
-	return phy_mii_ioctl(lp->phy_dev, if_mii(rq), cmd);
+	return phy_mii_ioctl(lp->phy_dev, rq, cmd);
 }
 
 static void tc35815_chip_reset(struct net_device *dev)
diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index 289cdc5..d4163f2 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -10954,7 +10954,7 @@ static int tg3_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
 		if (!(tp->tg3_flags3 & TG3_FLG3_PHY_CONNECTED))
 			return -EAGAIN;
 		phydev = tp->mdio_bus->phy_map[TG3_PHY_MII_ADDR];
-		return phy_mii_ioctl(phydev, data, cmd);
+		return phy_mii_ioctl(phydev, ifr, cmd);
 	}
 
 	switch (cmd) {
diff --git a/drivers/net/ucc_geth.c b/drivers/net/ucc_geth.c
index dc32a62..e17dd74 100644
--- a/drivers/net/ucc_geth.c
+++ b/drivers/net/ucc_geth.c
@@ -3714,7 +3714,7 @@ static int ucc_geth_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
 	if (!ugeth->phydev)
 		return -ENODEV;
 
-	return phy_mii_ioctl(ugeth->phydev, if_mii(rq), cmd);
+	return phy_mii_ioctl(ugeth->phydev, rq, cmd);
 }
 
 static const struct net_device_ops ucc_geth_netdev_ops = {
diff --git a/drivers/staging/octeon/ethernet-mdio.c b/drivers/staging/octeon/ethernet-mdio.c
index 7e0be8d..10a82ef 100644
--- a/drivers/staging/octeon/ethernet-mdio.c
+++ b/drivers/staging/octeon/ethernet-mdio.c
@@ -113,7 +113,7 @@ int cvm_oct_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
 	if (!priv->phydev)
 		return -EINVAL;
 
-	return phy_mii_ioctl(priv->phydev, if_mii(rq), cmd);
+	return phy_mii_ioctl(priv->phydev, rq, cmd);
 }
 
 static void cvm_oct_adjust_link(struct net_device *dev)
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 3566bde..7a8caac 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -505,7 +505,7 @@ void phy_stop_machine(struct phy_device *phydev);
 int phy_ethtool_sset(struct phy_device *phydev, struct ethtool_cmd *cmd);
 int phy_ethtool_gset(struct phy_device *phydev, struct ethtool_cmd *cmd);
 int phy_mii_ioctl(struct phy_device *phydev,
-		struct mii_ioctl_data *mii_data, int cmd);
+		struct ifreq *ifr, int cmd);
 int phy_start_interrupts(struct phy_device *phydev);
 void phy_print_status(struct phy_device *phydev);
 struct phy_device* phy_device_create(struct mii_bus *bus, int addr, int phy_id);
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 8fdca56..64ca2a6 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -164,10 +164,9 @@ out:
 static int dsa_slave_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
 {
 	struct dsa_slave_priv *p = netdev_priv(dev);
-	struct mii_ioctl_data *mii_data = if_mii(ifr);
 
 	if (p->phy != NULL)
-		return phy_mii_ioctl(p->phy, mii_data, cmd);
+		return phy_mii_ioctl(p->phy, ifr, cmd);
 
 	return -EOPNOTSUPP;
 }
-- 
1.7.0.4

[PATCH 4/4] phylib: Allow reading and writing a mii bus from atomic context.

[Richard Cochran]

In order to support hardware time stamping from a PHY, it is necessary to
read from the PHY while running in_interrupt(). This patch allows a mii
bus to operate in an atomic context. An mii_bus driver may declare itself
capable for this mode. Drivers which do not do this will remain with the
default that bus operations may sleep.

Before commit 35b5f6b1a82b5c586e0b24c711dc6ba944e88ef1 mii bus
operations were protected with spin locks. That commit replaced the
locks with mutexs in order to accommodate i2c buses that need to
sleep. Thus, this patch restores the original behavior as a run time
option.

Signed-off-by: Richard Cochran <richard.cochran@omicron.at>
---
 drivers/net/fsl_pq_mdio.c  |    4 +-
 drivers/net/phy/mdio_bus.c |   45 +++++++++++++++++++++++++++++++++++++------
 include/linux/phy.h        |   15 ++++++++++++-
 3 files changed, 53 insertions(+), 11 deletions(-)

diff --git a/drivers/net/fsl_pq_mdio.c b/drivers/net/fsl_pq_mdio.c
index b4c41d7..0b34f50 100644
--- a/drivers/net/fsl_pq_mdio.c
+++ b/drivers/net/fsl_pq_mdio.c
@@ -145,7 +145,7 @@ static int fsl_pq_mdio_reset(struct mii_bus *bus)
 	struct fsl_pq_mdio __iomem *regs = fsl_pq_mdio_get_regs(bus);
 	int timeout = PHY_INIT_TIMEOUT;
 
-	mutex_lock(&bus->mdio_lock);
+	mdiobus_lock(bus);
 
 	/* Reset the management interface */
 	out_be32(&regs->miimcfg, MIIMCFG_RESET);
@@ -157,7 +157,7 @@ static int fsl_pq_mdio_reset(struct mii_bus *bus)
 	while ((in_be32(&regs->miimind) & MIIMIND_BUSY) && timeout--)
 		cpu_relax();
 
-	mutex_unlock(&bus->mdio_lock);
+	mdiobus_unlock(bus);
 
 	if (timeout < 0) {
 		printk(KERN_ERR "%s: The MII Bus is stuck!\n",
diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index 6a6b819..ad6bed8 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -37,6 +37,32 @@
 #include <asm/uaccess.h>
 
 /**
+ * mdiobus_lock - locks a given bus for read or write operations.
+ * @bus: target mii_bus
+ */
+void mdiobus_lock(struct mii_bus *bus)
+{
+	if (MDIOBUS_SLEEPS_RW == bus->locktype)
+		mutex_lock(&bus->lock.m);
+	else
+		spin_lock(&bus->lock.s);
+}
+EXPORT_SYMBOL(mdiobus_lock);
+
+/**
+ * mdiobus_unlock - unlocks a given bus for read or write operations.
+ * @bus: target mii_bus
+ */
+void mdiobus_unlock(struct mii_bus *bus)
+{
+	if (MDIOBUS_SLEEPS_RW == bus->locktype)
+		mutex_unlock(&bus->lock.m);
+	else
+		spin_unlock(&bus->lock.s);
+}
+EXPORT_SYMBOL(mdiobus_unlock);
+
+/**
  * mdiobus_alloc - allocate a mii_bus structure
  *
  * Description: called by a bus driver to allocate an mii_bus
@@ -107,7 +133,10 @@ int mdiobus_register(struct mii_bus *bus)
 		return -EINVAL;
 	}
 
-	mutex_init(&bus->mdio_lock);
+	if (MDIOBUS_SLEEPS_RW == bus->locktype)
+		mutex_init(&bus->lock.m);
+	else
+		spin_lock_init(&bus->lock.s);
 
 	if (bus->reset)
 		bus->reset(bus);
@@ -212,11 +241,12 @@ int mdiobus_read(struct mii_bus *bus, int addr, u32 regnum)
 {
 	int retval;
 
-	BUG_ON(in_interrupt());
+	if (MDIOBUS_SLEEPS_RW == bus->locktype)
+		BUG_ON(in_interrupt());
 
-	mutex_lock(&bus->mdio_lock);
+	mdiobus_lock(bus);
 	retval = bus->read(bus, addr, regnum);
-	mutex_unlock(&bus->mdio_lock);
+	mdiobus_unlock(bus);
 
 	return retval;
 }
@@ -237,11 +267,12 @@ int mdiobus_write(struct mii_bus *bus, int addr, u32 regnum, u16 val)
 {
 	int err;
 
-	BUG_ON(in_interrupt());
+	if (MDIOBUS_SLEEPS_RW == bus->locktype)
+		BUG_ON(in_interrupt());
 
-	mutex_lock(&bus->mdio_lock);
+	mdiobus_lock(bus);
 	err = bus->write(bus, addr, regnum, val);
-	mutex_unlock(&bus->mdio_lock);
+	mdiobus_unlock(bus);
 
 	return err;
 }
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 7a8caac..93ea55f 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -98,11 +98,20 @@ struct mii_bus {
 	int (*write)(struct mii_bus *bus, int phy_id, int regnum, u16 val);
 	int (*reset)(struct mii_bus *bus);
 
+	/* Indicates whether bus may be used from an atomic context. */
+	enum {
+		MDIOBUS_SLEEPS_RW,
+		MDIOBUS_ATOMIC_RW
+	} locktype;
+
 	/*
-	 * A lock to ensure that only one thing can read/write
+	 * A lock or mutex to ensure that only one thing can read/write
 	 * the MDIO bus at a time
 	 */
-	struct mutex mdio_lock;
+	union {
+		struct mutex m;
+		spinlock_t s;
+	} lock;
 
 	struct device *parent;
 	enum {
@@ -127,6 +136,8 @@ struct mii_bus {
 };
 #define to_mii_bus(d) container_of(d, struct mii_bus, dev)
 
+void mdiobus_lock(struct mii_bus *bus);
+void mdiobus_unlock(struct mii_bus *bus);
 struct mii_bus *mdiobus_alloc(void);
 int mdiobus_register(struct mii_bus *bus);
 void mdiobus_unregister(struct mii_bus *bus);
-- 
1.7.0.4

Re: [PATCH 1/4] net: add driver hooks for time stamping.

[David Miller]

From: Richard Cochran <richardcochran@gmail.com>
Date: Mon, 5 Jul 2010 07:31:07 +0200

> This patch adds hooks for transmit and receive time stamps. The
> transmit hook allows a software fallback for transmit time stamps,
> for MACs lacking time stamping hardware. The receive hook does not yet
> have any effect, but it prepares the way for hardware time stamping
> in PHY devices. Using the hooks will still require adding two inline
> function calls to each MAC driver.
> 
> Signed-off-by: Richard Cochran <richard.cochran@omicron.at>

Applied.

Re: [PATCH 2/4] phylib: add a way to make PHY time stamps possible.

[David Miller]

From: Richard Cochran <richardcochran@gmail.com>
Date: Mon, 5 Jul 2010 07:32:52 +0200

> This patch adds a new networking option to allow hardware time stamps
> from PHY devices. Using PHY time stamps will still require adding two
> inline function calls to each MAC driver. The CONFIG option makes
> these calls safe to add, since the calls become NOOPs when the option
> is disabled.
> 
> The patch also adds phylib driver methods for the SIOCSHWTSTAMP ioctl
> and callbacks for transmit and receive time stamping. Drivers may
> optionally implement these functions.
> 
> Signed-off-by: Richard Cochran <richard.cochran@omicron.at>

Applied.

Re: [PATCH 3/4] phylib: preserve ifreq parameter when calling generic phy_mii_ioctl()

[David Miller]

From: Richard Cochran <richardcochran@gmail.com>
Date: Mon, 5 Jul 2010 07:33:03 +0200

> The phy_mii_ioctl() function unnecessarily throws away the original ifreq.
> We need access to the ifreq in order to support PHYs that can perform
> hardware time stamping.
> 
> Two maverick drivers filter the ioctl commands passed to phy_mii_ioctl().
> This is unnecessary since phylib will check the command in any case.
> 
> Signed-off-by: Richard Cochran <richard.cochran@omicron.at>

Applied.

Re: [PATCH 4/4] phylib: Allow reading and writing a mii bus from atomic context.

[David Miller]

From: Richard Cochran <richardcochran@gmail.com>
Date: Mon, 5 Jul 2010 07:33:14 +0200

> In order to support hardware time stamping from a PHY, it is necessary to
> read from the PHY while running in_interrupt(). This patch allows a mii
> bus to operate in an atomic context. An mii_bus driver may declare itself
> capable for this mode. Drivers which do not do this will remain with the
> default that bus operations may sleep.
> 
> Before commit 35b5f6b1a82b5c586e0b24c711dc6ba944e88ef1 mii bus
> operations were protected with spin locks. That commit replaced the
> locks with mutexs in order to accommodate i2c buses that need to
> sleep. Thus, this patch restores the original behavior as a run time
> option.
> 
> Signed-off-by: Richard Cochran <richard.cochran@omicron.at>

Ok, I'm not to happy about this one, to be honest.

Conditional locking is always a problem waiting to happen.

Question: What context can you call mdiobus_lock() in?

Answer: You absolutely cannot know.

Therefore every piece of code, except those with special knowledge
of what kind of device is behind the mdiobus object, have to assume
the worst which is that the interface can only be called in sleepable
contexts.

We need to find another way to accomodate this, I really don't want to
see this kind of situation where the locking facility type is
conditional upon the device sitting behind the interface.

Re: [PATCH 1/4] net: add driver hooks for time stamping.

[David Miller]

From: David Miller <davem@davemloft.net>
Date: Mon, 05 Jul 2010 19:03:01 -0700 (PDT)

> From: Richard Cochran <richardcochran@gmail.com>
> Date: Mon, 5 Jul 2010 07:31:07 +0200
> 
>> This patch adds hooks for transmit and receive time stamps. The
>> transmit hook allows a software fallback for transmit time stamps,
>> for MACs lacking time stamping hardware. The receive hook does not yet
>> have any effect, but it prepares the way for hardware time stamping
>> in PHY devices. Using the hooks will still require adding two inline
>> function calls to each MAC driver.
>> 
>> Signed-off-by: Richard Cochran <richard.cochran@omicron.at>
> 
> Applied.

Actually, this breaks the build.

There are drivers that never saw phylib.h included in them,
which use internal static functions named "phy_read()" and such
which match global interfaces defined in phylib.h

drivers/net/tulip/dmfe.c:334:12: error: conflicting types for 'phy_read'
include/linux/phy.h:437:19: note: previous definition of 'phy_read' was here
drivers/net/tulip/dmfe.c:335:13: error: conflicting types for 'phy_write'
include/linux/phy.h:452:19: note: previous definition of 'phy_write' was here

You're going to need to resolve this issue first, then make sure at a
minimum that an "allmodconfig" build fully passes with an
unconditional include of phylib.h in skbuff.h

I've reverted all of your patches.

Re: [PATCH 4/4] phylib: Allow reading and writing a mii bus from atomic context.

[Andy Fleming]

On Mon, Jul 5, 2010 at 12:33 AM, Richard Cochran
<richardcochran@gmail.com> wrote:
> In order to support hardware time stamping from a PHY, it is necessary to
> read from the PHY while running in_interrupt(). This patch allows a mii
> bus to operate in an atomic context. An mii_bus driver may declare itself
> capable for this mode. Drivers which do not do this will remain with the
> default that bus operations may sleep.
>
> Before commit 35b5f6b1a82b5c586e0b24c711dc6ba944e88ef1 mii bus
> operations were protected with spin locks. That commit replaced the
> locks with mutexs in order to accommodate i2c buses that need to
> sleep. Thus, this patch restores the original behavior as a run time
> option.


The reason we moved to mutexes was because we didn't want PHY
operations to be done in interrupt context.  They are too slow.  Also,
some MII busses will trigger an interrupt when the operation is done,
which means their driver has the option of sleeping.  As such, it was
an original principle of the PHY lib that MII transactions were not
allowed in interrupt context.  *Certainly*, once you *do* allow MII
transactions in interrupt context, you *cannot* use spin_lock().  You
at least have to use spin_lock_irq[save].

Also, I agree with David's comments, and Grant's.  There's got to be
another way to do this.

Andy

Re: [PATCH 4/4] phylib: Allow reading and writing a mii bus from atomic context.

[Richard Cochran]

On Tue, Jul 06, 2010 at 12:09:10PM -0500, Andy Fleming wrote:
> allowed in interrupt context.  *Certainly*, once you *do* allow MII
> transactions in interrupt context, you *cannot* use spin_lock().  You
> at least have to use spin_lock_irq[save].

Okay, I see. Before the change to mutexes the code used spin_lock_bh.

> Also, I agree with David's comments, and Grant's.  There's got to be
> another way to do this.

I understand what troubles you about the proposed changes. However, I
cannot see a better way to get a PHY time stamp other than to read it
out during the napi poll and hard_xmit functions. It is a chicken and
egg problem.

Consider the receive path:

1. PTP Packet passed through PHY. PHY recognizes it and stores a time
   stamp along with some UID from the packet.

2. Napi calls the MAC driver's poll function.

3. MAC driver acquires packet. At this point, if we want to have a
   hardware time stamp, we must read it out over the mdio bus, before
   handing the packet over to the stack via netif_receive_skb().

If we decide to defer the packet delivery (in step 3), we have to know
whether the PHY will have a time stamp for this packet. The only way
to do this is to compare the UIDs, but that requires reading it over
the mdio bus.

One could simply defer *all* packets, but that would really stink.

Possibly, we could defer all likely packets, for example all PTP
packets. Changing all the MAC driver to call a time stamping hook that
conditionally consumes the skbuffs would also stink. Could the packet
be deferred by the stack, early in netif_receive_skb? If so, a work
queue could read the PHY and then deliver the deferred packets at some
later time.

IMHO, it better just to check for a PHY time stamp immediately, and
live with the performance hit. I believe that PTP users will accept
this. Naturally, it requires allowing reading the mdio bus in two
non-sleeping contexts.

There are perhaps other ways to provide PHY time stamps, but I am
doubtful that they would work better, in general. The National
Semiconductor PHYTER can provide the time stamps as status frames, and
it can also insert time stamps directly into the incoming and
outgoing frames.

* Using status frames is an attractive idea, because it obviates the
  need to read over the mdio bus. However, you still have the problem
  of matching the status frames to the PTP packets.

* Inserting a time stamp into outgoing frames presents no problems
  (this is the so called "PTP one step" operation). However, for
  incoming frames the PHYTER inserts the time stamp into the PTP
  message at a programmable offset. This invalidates the UDP checksum
  for PTPv1 packets (PTPv2 checksums will be automatically corrected.)

Even if we implement one of these alternatives, I am not sure that
other PHYs will also offer the same capabilities as the PHYTER.

Well, there you have it. I welcome any ideas on how to go about
offering PHY time stamping.

Thanks,
Richard

Re: [PATCH 4/4] phylib: Allow reading and writing a mii bus from atomic context.

[Richard Cochran]

On Wed, Jul 07, 2010 at 09:18:32AM +0200, Richard Cochran wrote:
> Consider the receive path:
> 
> 1. PTP Packet passed through PHY. PHY recognizes it and stores a time
>    stamp along with some UID from the packet.
> 
> 2. Napi calls the MAC driver's poll function.
> 
> 3. MAC driver acquires packet. At this point, if we want to have a
>    hardware time stamp, we must read it out over the mdio bus, before
>    handing the packet over to the stack via netif_receive_skb().
> 
> If we decide to defer the packet delivery (in step 3), we have to know
> whether the PHY will have a time stamp for this packet. The only way
> to do this is to compare the UIDs, but that requires reading it over
> the mdio bus.

(Forwarding Andy's message to me that was missing a CC to the list)

On Thu, Jul 08, 2010 at 03:40:41PM -0500, Andy Fleming wrote:

Wait, is the intent for this mdio read to be done for *every* packet?
MDIO is spec'ed out to go up to 2.5MHz.  Each transaction takes 64
cycles.  And I see that reading the timestamp from the PHY you
submitted support for takes at *least* 2 transactions.  The fastest
you can process packets would then be under 20,000 packets per second.
Even on a 100Mb link with full-sized packets, you would be 40% done
receiving the next packet before you had passed the packet on to the
stack.  Each MDIO transaction takes 25,600 cycles on a gigahertz
processor.  It's just too long, IMO, and it looks like this code will
end up doing up to...10?

I wasn't able to find an example of how you were going to use the
time-stamp functions you provided.  Could you please go into a little
more detail about how you intended this to work?

Re: [PATCH 4/4] phylib: Allow reading and writing a mii bus from atomic context.

[Richard Cochran]

On Thu, Jul 08, 2010 at 03:40:41PM -0500, Andy Fleming wrote:
> 
> Wait, is the intent for this mdio read to be done for *every* packet?
> MDIO is spec'ed out to go up to 2.5MHz.  Each transaction takes 64
> cycles.  And I see that reading the timestamp from the PHY you
> submitted support for takes at *least* 2 transactions.  The fastest
> you can process packets would then be under 20,000 packets per second.
> Even on a 100Mb link with full-sized packets, you would be 40% done
> receiving the next packet before you had passed the packet on to the
> stack.  Each MDIO transaction takes 25,600 cycles on a gigahertz
> processor.  It's just too long, IMO, and it looks like this code will
> end up doing up to...10?

Well, it is actually worse than that. From my measurements, 64 cycles
at 2.5 MHz (25.6 usec) is too optimistic. On one Xscale IXP platform,
I get 35 to 40 usec per read. Also, the PHYTER needs six reads for a
Rx and four for a Tx time stamp.

So you are right, it is a very long time to leave interrupts off.

However, not every packet needs a time stamp. The PHY devices that I
know of all are selective. That is, they recognize PTP packets in
hardware and only provide time stamps for those special packets. The
packet rate is not that hight. For example, the "sync" message comes
once every two seconds in PTPv1 and up to ten times per second in
PTPv2.

I have been working on an alternative, which I will post soon. It goes
like this... (comments, please, before I get too far into it!)

* General
   - phy registers itself with net_device { opaque pointer }

   - ts_candidate(skb):
     function to detect likely packets, returns true if
       1. phy_device pointer exists in net_device
       2. data look like PTP, using a BPF

   - work queue:
     1. read out time stamps
     2. match times with queued skbs
     3. deliver skbs with time stamps (or without on timeout)

* Rx path
   - netif_receive_skb:
     if ts_candidate(skb), defer(skb).

* Tx path
   - hook in each MAC driver
   - if ts_candidate(skb), clone and defer(skb).


> I wasn't able to find an example of how you were going to use the
> time-stamp functions you provided.  Could you please go into a little
> more detail about how you intended this to work?

My last PTP series included the PHY stuff, too. There you can see how
it all fits together.

   http://marc.info/?l=linux-netdev&m=127661796627120&w=3


Thanks,
Richard