* [PATCH net-next v2 1/7] net: mdiobus: add unlocked accessors
2018-01-02 10:52 [PATCH net-next v2 0/7] Resolve races in phy accessors Russell King - ARM Linux
@ 2018-01-02 10:58 ` Russell King
2018-01-02 10:58 ` [PATCH net-next v2 2/7] net: phy: use unlocked accessors for indirect MMD accesses Russell King
` (6 subsequent siblings)
7 siblings, 0 replies; 16+ messages in thread
From: Russell King @ 2018-01-02 10:58 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli; +Cc: netdev
Add unlocked versions of the bus accessors, which allows access to the
bus with all the tracing. These accessors validate that the bus mutex
is held, which is a basic requirement for all mii bus accesses.
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
drivers/net/phy/mdio_bus.c | 65 +++++++++++++++++++++++++++++++++++++---------
include/linux/mdio.h | 3 +++
2 files changed, 56 insertions(+), 12 deletions(-)
diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index 54d00a1d2bef..75be8be0d169 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -494,6 +494,55 @@ struct phy_device *mdiobus_scan(struct mii_bus *bus, int addr)
EXPORT_SYMBOL(mdiobus_scan);
/**
+ * __mdiobus_read - Unlocked version of the mdiobus_read function
+ * @bus: the mii_bus struct
+ * @addr: the phy address
+ * @regnum: register number to read
+ *
+ * Read a MDIO bus register. Caller must hold the mdio bus lock.
+ *
+ * NOTE: MUST NOT be called from interrupt context.
+ */
+int __mdiobus_read(struct mii_bus *bus, int addr, u32 regnum)
+{
+ int retval;
+
+ WARN_ON_ONCE(!mutex_is_locked(&bus->mdio_lock));
+
+ retval = bus->read(bus, addr, regnum);
+
+ trace_mdio_access(bus, 1, addr, regnum, retval, retval);
+
+ return retval;
+}
+EXPORT_SYMBOL(__mdiobus_read);
+
+/**
+ * __mdiobus_write - Unlocked version of the mdiobus_write function
+ * @bus: the mii_bus struct
+ * @addr: the phy address
+ * @regnum: register number to write
+ * @val: value to write to @regnum
+ *
+ * Write a MDIO bus register. Caller must hold the mdio bus lock.
+ *
+ * NOTE: MUST NOT be called from interrupt context.
+ */
+int __mdiobus_write(struct mii_bus *bus, int addr, u32 regnum, u16 val)
+{
+ int err;
+
+ WARN_ON_ONCE(!mutex_is_locked(&bus->mdio_lock));
+
+ err = bus->write(bus, addr, regnum, val);
+
+ trace_mdio_access(bus, 0, addr, regnum, val, err);
+
+ return err;
+}
+EXPORT_SYMBOL(__mdiobus_write);
+
+/**
* mdiobus_read_nested - Nested version of the mdiobus_read function
* @bus: the mii_bus struct
* @addr: the phy address
@@ -513,11 +562,9 @@ int mdiobus_read_nested(struct mii_bus *bus, int addr, u32 regnum)
BUG_ON(in_interrupt());
mutex_lock_nested(&bus->mdio_lock, MDIO_MUTEX_NESTED);
- retval = bus->read(bus, addr, regnum);
+ retval = __mdiobus_read(bus, addr, regnum);
mutex_unlock(&bus->mdio_lock);
- trace_mdio_access(bus, 1, addr, regnum, retval, retval);
-
return retval;
}
EXPORT_SYMBOL(mdiobus_read_nested);
@@ -539,11 +586,9 @@ int mdiobus_read(struct mii_bus *bus, int addr, u32 regnum)
BUG_ON(in_interrupt());
mutex_lock(&bus->mdio_lock);
- retval = bus->read(bus, addr, regnum);
+ retval = __mdiobus_read(bus, addr, regnum);
mutex_unlock(&bus->mdio_lock);
- trace_mdio_access(bus, 1, addr, regnum, retval, retval);
-
return retval;
}
EXPORT_SYMBOL(mdiobus_read);
@@ -569,11 +614,9 @@ int mdiobus_write_nested(struct mii_bus *bus, int addr, u32 regnum, u16 val)
BUG_ON(in_interrupt());
mutex_lock_nested(&bus->mdio_lock, MDIO_MUTEX_NESTED);
- err = bus->write(bus, addr, regnum, val);
+ err = __mdiobus_write(bus, addr, regnum, val);
mutex_unlock(&bus->mdio_lock);
- trace_mdio_access(bus, 0, addr, regnum, val, err);
-
return err;
}
EXPORT_SYMBOL(mdiobus_write_nested);
@@ -596,11 +639,9 @@ int mdiobus_write(struct mii_bus *bus, int addr, u32 regnum, u16 val)
BUG_ON(in_interrupt());
mutex_lock(&bus->mdio_lock);
- err = bus->write(bus, addr, regnum, val);
+ err = __mdiobus_write(bus, addr, regnum, val);
mutex_unlock(&bus->mdio_lock);
- trace_mdio_access(bus, 0, addr, regnum, val, err);
-
return err;
}
EXPORT_SYMBOL(mdiobus_write);
diff --git a/include/linux/mdio.h b/include/linux/mdio.h
index ca08ab16ecdc..34796e29c90c 100644
--- a/include/linux/mdio.h
+++ b/include/linux/mdio.h
@@ -257,6 +257,9 @@ static inline u16 ethtool_adv_to_mmd_eee_adv_t(u32 adv)
return reg;
}
+int __mdiobus_read(struct mii_bus *bus, int addr, u32 regnum);
+int __mdiobus_write(struct mii_bus *bus, int addr, u32 regnum, u16 val);
+
int mdiobus_read(struct mii_bus *bus, int addr, u32 regnum);
int mdiobus_read_nested(struct mii_bus *bus, int addr, u32 regnum);
int mdiobus_write(struct mii_bus *bus, int addr, u32 regnum, u16 val);
--
2.7.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH net-next v2 2/7] net: phy: use unlocked accessors for indirect MMD accesses
2018-01-02 10:52 [PATCH net-next v2 0/7] Resolve races in phy accessors Russell King - ARM Linux
2018-01-02 10:58 ` [PATCH net-next v2 1/7] net: mdiobus: add unlocked accessors Russell King
@ 2018-01-02 10:58 ` Russell King
2018-01-02 10:58 ` [PATCH net-next v2 3/7] net: phy: add unlocked accessors Russell King
` (5 subsequent siblings)
7 siblings, 0 replies; 16+ messages in thread
From: Russell King @ 2018-01-02 10:58 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli; +Cc: netdev
Use unlocked accessors for indirect MMD accesses to clause 22 PHYs.
This permits tracing of these accesses.
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
drivers/net/phy/phy-core.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
index 21f75ae244b3..83d32644cb4d 100644
--- a/drivers/net/phy/phy-core.c
+++ b/drivers/net/phy/phy-core.c
@@ -193,13 +193,14 @@ static void mmd_phy_indirect(struct mii_bus *bus, int phy_addr, int devad,
u16 regnum)
{
/* Write the desired MMD Devad */
- bus->write(bus, phy_addr, MII_MMD_CTRL, devad);
+ __mdiobus_write(bus, phy_addr, MII_MMD_CTRL, devad);
/* Write the desired MMD register address */
- bus->write(bus, phy_addr, MII_MMD_DATA, regnum);
+ __mdiobus_write(bus, phy_addr, MII_MMD_DATA, regnum);
/* Select the Function : DATA with no post increment */
- bus->write(bus, phy_addr, MII_MMD_CTRL, devad | MII_MMD_CTRL_NOINCR);
+ __mdiobus_write(bus, phy_addr, MII_MMD_CTRL,
+ devad | MII_MMD_CTRL_NOINCR);
}
/**
@@ -232,7 +233,7 @@ int phy_read_mmd(struct phy_device *phydev, int devad, u32 regnum)
mmd_phy_indirect(bus, phy_addr, devad, regnum);
/* Read the content of the MMD's selected register */
- val = bus->read(bus, phy_addr, MII_MMD_DATA);
+ val = __mdiobus_read(bus, phy_addr, MII_MMD_DATA);
mutex_unlock(&bus->mdio_lock);
}
return val;
@@ -271,7 +272,7 @@ int phy_write_mmd(struct phy_device *phydev, int devad, u32 regnum, u16 val)
mmd_phy_indirect(bus, phy_addr, devad, regnum);
/* Write the data into MMD's selected register */
- bus->write(bus, phy_addr, MII_MMD_DATA, val);
+ __mdiobus_write(bus, phy_addr, MII_MMD_DATA, val);
mutex_unlock(&bus->mdio_lock);
ret = 0;
--
2.7.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH net-next v2 3/7] net: phy: add unlocked accessors
2018-01-02 10:52 [PATCH net-next v2 0/7] Resolve races in phy accessors Russell King - ARM Linux
2018-01-02 10:58 ` [PATCH net-next v2 1/7] net: mdiobus: add unlocked accessors Russell King
2018-01-02 10:58 ` [PATCH net-next v2 2/7] net: phy: use unlocked accessors for indirect MMD accesses Russell King
@ 2018-01-02 10:58 ` Russell King
2018-01-02 15:49 ` Andrew Lunn
2018-01-02 10:58 ` [PATCH net-next v2 4/7] net: phy: add paged phy register accessors Russell King
` (4 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Russell King @ 2018-01-02 10:58 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli; +Cc: netdev
Add unlocked versions of the bus accessors, which allows access to the
bus with all the tracing. These accessors validate that the bus mutex
is held, which is a basic requirement for all mii bus accesses.
Also added is a read-modify-write unlocked accessor with the same
locking requirements.
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
drivers/net/phy/phy-core.c | 25 +++++++++++++++++++++++++
include/linux/phy.h | 28 ++++++++++++++++++++++++++++
2 files changed, 53 insertions(+)
diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
index 83d32644cb4d..37c039da0c16 100644
--- a/drivers/net/phy/phy-core.c
+++ b/drivers/net/phy/phy-core.c
@@ -280,3 +280,28 @@ int phy_write_mmd(struct phy_device *phydev, int devad, u32 regnum, u16 val)
return ret;
}
EXPORT_SYMBOL(phy_write_mmd);
+
+/**
+ * __phy_modify() - Convenience function for modifying a PHY register
+ * @phydev: a pointer to a &struct phy_device
+ * @regnum: register number
+ * @mask: bit mask of bits to clear
+ * @set: bit mask of bits to set
+ *
+ * Unlocked helper function which allows a PHY register to be modified as
+ * new register value = (old register value & mask) | set
+ */
+int __phy_modify(struct phy_device *phydev, u32 regnum, u16 mask, u16 set)
+{
+ int ret, res;
+
+ ret = __phy_read(phydev, regnum);
+ if (ret >= 0) {
+ res = __phy_write(phydev, regnum, (ret & ~mask) | set);
+ if (res < 0)
+ ret = res;
+ }
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(__phy_modify);
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 71d777fe6c3d..3dae5b7408b4 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -716,6 +716,18 @@ static inline int phy_read(struct phy_device *phydev, u32 regnum)
}
/**
+ * __phy_read - convenience function for reading a given PHY register
+ * @phydev: the phy_device struct
+ * @regnum: register number to read
+ *
+ * The caller must have taken the MDIO bus lock.
+ */
+static inline int __phy_read(struct phy_device *phydev, u32 regnum)
+{
+ return __mdiobus_read(phydev->mdio.bus, phydev->mdio.addr, regnum);
+}
+
+/**
* phy_write - Convenience function for writing a given PHY register
* @phydev: the phy_device struct
* @regnum: register number to write
@@ -731,6 +743,22 @@ static inline int phy_write(struct phy_device *phydev, u32 regnum, u16 val)
}
/**
+ * __phy_write - Convenience function for writing a given PHY register
+ * @phydev: the phy_device struct
+ * @regnum: register number to write
+ * @val: value to write to @regnum
+ *
+ * The caller must have taken the MDIO bus lock.
+ */
+static inline int __phy_write(struct phy_device *phydev, u32 regnum, u16 val)
+{
+ return __mdiobus_write(phydev->mdio.bus, phydev->mdio.addr, regnum,
+ val);
+}
+
+int __phy_modify(struct phy_device *phydev, u32 regnum, u16 mask, u16 set);
+
+/**
* phy_interrupt_is_valid - Convenience function for testing a given PHY irq
* @phydev: the phy_device struct
*
--
2.7.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH net-next v2 3/7] net: phy: add unlocked accessors
2018-01-02 10:58 ` [PATCH net-next v2 3/7] net: phy: add unlocked accessors Russell King
@ 2018-01-02 15:49 ` Andrew Lunn
0 siblings, 0 replies; 16+ messages in thread
From: Andrew Lunn @ 2018-01-02 15:49 UTC (permalink / raw)
To: Russell King; +Cc: Florian Fainelli, netdev
On Tue, Jan 02, 2018 at 10:58:37AM +0000, Russell King wrote:
> Add unlocked versions of the bus accessors, which allows access to the
> bus with all the tracing. These accessors validate that the bus mutex
> is held, which is a basic requirement for all mii bus accesses.
>
> Also added is a read-modify-write unlocked accessor with the same
> locking requirements.
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH net-next v2 4/7] net: phy: add paged phy register accessors
2018-01-02 10:52 [PATCH net-next v2 0/7] Resolve races in phy accessors Russell King - ARM Linux
` (2 preceding siblings ...)
2018-01-02 10:58 ` [PATCH net-next v2 3/7] net: phy: add unlocked accessors Russell King
@ 2018-01-02 10:58 ` Russell King
2018-01-02 15:52 ` Andrew Lunn
2018-01-02 10:58 ` [PATCH net-next v2 5/7] net: phy: marvell: fix paged access races Russell King
` (3 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Russell King @ 2018-01-02 10:58 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli; +Cc: netdev
Add a set of paged phy register accessors which are inherently safe in
their design against other accesses interfering with the paged access.
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
drivers/net/phy/phy-core.c | 157 +++++++++++++++++++++++++++++++++++++++++++++
include/linux/phy.h | 11 ++++
2 files changed, 168 insertions(+)
diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
index 37c039da0c16..412ed8ff50e2 100644
--- a/drivers/net/phy/phy-core.c
+++ b/drivers/net/phy/phy-core.c
@@ -305,3 +305,160 @@ int __phy_modify(struct phy_device *phydev, u32 regnum, u16 mask, u16 set)
return ret;
}
EXPORT_SYMBOL_GPL(__phy_modify);
+
+static int __phy_read_page(struct phy_device *phydev)
+{
+ return phydev->drv->read_page(phydev);
+}
+
+static int __phy_write_page(struct phy_device *phydev, int page)
+{
+ return phydev->drv->write_page(phydev, page);
+}
+
+/**
+ * phy_save_page() - take the bus lock and save the current page
+ * @phydev: a pointer to a &struct phy_device
+ *
+ * Take the MDIO bus lock, and return the current page number. On error,
+ * returns a negative errno. phy_restore_page() must always be called
+ * after this, irrespective of success or failure of this call.
+ */
+int phy_save_page(struct phy_device *phydev)
+{
+ mutex_lock(&phydev->mdio.bus->mdio_lock);
+ return __phy_read_page(phydev);
+}
+EXPORT_SYMBOL_GPL(phy_save_page);
+
+/**
+ * phy_select_page() - take the bus lock, save the current page, and set a page
+ * @phydev: a pointer to a &struct phy_device
+ * @page: desired page
+ *
+ * Take the MDIO bus lock to protect against concurrent access, save the
+ * current PHY page, and set the current page. On error, returns a
+ * negative errno, otherwise returns the previous page number.
+ * phy_restore_page() must always be called after this, irrespective
+ * of success or failure of this call.
+ */
+int phy_select_page(struct phy_device *phydev, int page)
+{
+ int ret, oldpage;
+
+ oldpage = ret = phy_save_page(phydev);
+ if (ret < 0)
+ return ret;
+
+ if (oldpage != page) {
+ ret = __phy_write_page(phydev, page);
+ if (ret < 0)
+ return ret;
+ }
+
+ return oldpage;
+}
+EXPORT_SYMBOL_GPL(phy_select_page);
+
+/**
+ * phy_restore_page() - restore the page register and release the bus lock
+ * @phydev: a pointer to a &struct phy_device
+ * @oldpage: the old page, return value from phy_save_page() or phy_select_page()
+ * @ret: operation's return code
+ *
+ * Release the MDIO bus lock, restoring @oldpage if it is a valid page.
+ * This function propagates the earliest error code from the group of
+ * operations.
+ *
+ * Returns:
+ * @oldpage if it was a negative value, otherwise
+ * @ret if it was a negative errno value, otherwise
+ * phy_write_page()'s negative value if it were in error, otherwise
+ * @ret.
+ */
+int phy_restore_page(struct phy_device *phydev, int oldpage, int ret)
+{
+ int r;
+
+ if (oldpage >= 0) {
+ r = __phy_write_page(phydev, oldpage);
+
+ /* Propagate the operation return code if the page write
+ * was successful.
+ */
+ if (ret >= 0 && r < 0)
+ ret = r;
+ } else {
+ /* Propagate the phy page selection error code */
+ ret = oldpage;
+ }
+
+ mutex_unlock(&phydev->mdio.bus->mdio_lock);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(phy_restore_page);
+
+/**
+ * phy_read_paged() - Convenience function for reading a paged register
+ * @phydev: a pointer to a &struct phy_device
+ * @page: the page for the phy
+ * @regnum: register number
+ *
+ * Same rules as for phy_read().
+ */
+int phy_read_paged(struct phy_device *phydev, int page, u32 regnum)
+{
+ int ret = 0, oldpage;
+
+ oldpage = phy_select_page(phydev, page);
+ if (oldpage >= 0)
+ ret = __phy_read(phydev, regnum);
+
+ return phy_restore_page(phydev, oldpage, ret);
+}
+EXPORT_SYMBOL(phy_read_paged);
+
+/**
+ * phy_write_paged() - Convenience function for writing a paged register
+ * @phydev: a pointer to a &struct phy_device
+ * @page: the page for the phy
+ * @regnum: register number
+ * @val: value to write
+ *
+ * Same rules as for phy_write().
+ */
+int phy_write_paged(struct phy_device *phydev, int page, u32 regnum, u16 val)
+{
+ int ret = 0, oldpage;
+
+ oldpage = phy_select_page(phydev, page);
+ if (oldpage >= 0)
+ ret = __phy_write(phydev, regnum, val);
+
+ return phy_restore_page(phydev, oldpage, ret);
+}
+EXPORT_SYMBOL(phy_write_paged);
+
+/**
+ * phy_modify_paged() - Convenience function for modifying a paged register
+ * @phydev: a pointer to a &struct phy_device
+ * @page: the page for the phy
+ * @regnum: register number
+ * @mask: bit mask of bits to clear
+ * @set: bit mask of bits to set
+ *
+ * Same rules as for phy_read() and phy_write().
+ */
+int phy_modify_paged(struct phy_device *phydev, int page, u32 regnum,
+ u16 mask, u16 set)
+{
+ int ret = 0, oldpage;
+
+ oldpage = phy_select_page(phydev, page);
+ if (oldpage >= 0)
+ ret = __phy_modify(phydev, regnum, mask, set);
+
+ return phy_restore_page(phydev, oldpage, ret);
+}
+EXPORT_SYMBOL(phy_modify_paged);
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 3dae5b7408b4..f408220d95ec 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -634,6 +634,9 @@ struct phy_driver {
int (*write_mmd)(struct phy_device *dev, int devnum, u16 regnum,
u16 val);
+ int (*read_page)(struct phy_device *dev);
+ int (*write_page)(struct phy_device *dev, int page);
+
/* Get the size and type of the eeprom contained within a plug-in
* module */
int (*module_info)(struct phy_device *dev,
@@ -836,6 +839,14 @@ static inline bool phy_is_pseudo_fixed_link(struct phy_device *phydev)
*/
int phy_write_mmd(struct phy_device *phydev, int devad, u32 regnum, u16 val);
+int phy_save_page(struct phy_device *phydev);
+int phy_select_page(struct phy_device *phydev, int page);
+int phy_restore_page(struct phy_device *phydev, int oldpage, int ret);
+int phy_read_paged(struct phy_device *phydev, int page, u32 regnum);
+int phy_write_paged(struct phy_device *phydev, int page, u32 regnum, u16 val);
+int phy_modify_paged(struct phy_device *phydev, int page, u32 regnum,
+ u16 mask, u16 set);
+
struct phy_device *phy_device_create(struct mii_bus *bus, int addr, int phy_id,
bool is_c45,
struct phy_c45_device_ids *c45_ids);
--
2.7.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH net-next v2 5/7] net: phy: marvell: fix paged access races
2018-01-02 10:52 [PATCH net-next v2 0/7] Resolve races in phy accessors Russell King - ARM Linux
` (3 preceding siblings ...)
2018-01-02 10:58 ` [PATCH net-next v2 4/7] net: phy: add paged phy register accessors Russell King
@ 2018-01-02 10:58 ` Russell King
2018-01-02 16:00 ` Andrew Lunn
2018-01-02 10:58 ` [PATCH net-next v2 6/7] net: phy: add phy_modify() accessor Russell King
` (2 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Russell King @ 2018-01-02 10:58 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli; +Cc: netdev
For paged accesses to be truely safe, we need to hold the bus lock to
prevent anyone else gaining access to the registers while we modify
them.
The phydev->lock mutex does not do this: userspace via the MII ioctl
can still sneak in and read or write any register while we are on a
different page, and the suspend/resume methods can be called by a
thread different to the thread polling the phy status.
Races have been observed with mvneta on SolidRun Clearfog with phylink,
particularly between the phylib worker reading the PHYs status, and
the thread resuming mvneta, calling phy_start() which then calls
through to m88e1121_config_aneg_rgmii_delays(), which tries to
read-modify-write the MSCR register:
CPU0 CPU1
marvell_read_status_page()
marvell_set_page(phydev, MII_MARVELL_FIBER_PAGE)
...
m88e1121_config_aneg_rgmii_delays()
set_page(MII_MARVELL_MSCR_PAGE)
phy_read(phydev, MII_88E1121_PHY_MSCR_REG)
marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE);
...
phy_write(phydev, MII_88E1121_PHY_MSCR_REG)
The result of this is we end up writing the copper page register 21,
which causes the copper PHY to be disabled, and the link partner sees
the link immediately go down.
Solve this by taking the bus lock instead of the PHY lock, thereby
preventing other accesses to the PHY while we are accessing other PHY
pages.
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
drivers/net/phy/marvell.c | 354 ++++++++++++++++++----------------------------
1 file changed, 137 insertions(+), 217 deletions(-)
diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index 82104edca393..1cb84064d658 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -83,7 +83,7 @@
#define MII_88E1121_PHY_MSCR_REG 21
#define MII_88E1121_PHY_MSCR_RX_DELAY BIT(5)
#define MII_88E1121_PHY_MSCR_TX_DELAY BIT(4)
-#define MII_88E1121_PHY_MSCR_DELAY_MASK (~(BIT(5) | BIT(4)))
+#define MII_88E1121_PHY_MSCR_DELAY_MASK (BIT(5) | BIT(4))
#define MII_88E1121_MISC_TEST 0x1a
#define MII_88E1510_MISC_TEST_TEMP_THRESHOLD_MASK 0x1f00
@@ -177,27 +177,19 @@ struct marvell_priv {
struct device *hwmon_dev;
};
-static int marvell_get_page(struct phy_device *phydev)
+static int marvell_read_page(struct phy_device *phydev)
{
- return phy_read(phydev, MII_MARVELL_PHY_PAGE);
+ return __phy_read(phydev, MII_MARVELL_PHY_PAGE);
}
-static int marvell_set_page(struct phy_device *phydev, int page)
+static int marvell_write_page(struct phy_device *phydev, int page)
{
- return phy_write(phydev, MII_MARVELL_PHY_PAGE, page);
+ return __phy_write(phydev, MII_MARVELL_PHY_PAGE, page);
}
-static int marvell_get_set_page(struct phy_device *phydev, int page)
+static int marvell_set_page(struct phy_device *phydev, int page)
{
- int oldpage = marvell_get_page(phydev);
-
- if (oldpage < 0)
- return oldpage;
-
- if (page != oldpage)
- return marvell_set_page(phydev, page);
-
- return 0;
+ return phy_write(phydev, MII_MARVELL_PHY_PAGE, page);
}
static int marvell_ack_interrupt(struct phy_device *phydev)
@@ -399,7 +391,7 @@ static int m88e1111_config_aneg(struct phy_device *phydev)
static int marvell_of_reg_init(struct phy_device *phydev)
{
const __be32 *paddr;
- int len, i, saved_page, current_page, ret;
+ int len, i, saved_page, current_page, ret = 0;
if (!phydev->mdio.dev.of_node)
return 0;
@@ -409,12 +401,11 @@ static int marvell_of_reg_init(struct phy_device *phydev)
if (!paddr || len < (4 * sizeof(*paddr)))
return 0;
- saved_page = marvell_get_page(phydev);
+ saved_page = phy_save_page(phydev);
if (saved_page < 0)
- return saved_page;
+ goto err;
current_page = saved_page;
- ret = 0;
len /= sizeof(*paddr);
for (i = 0; i < len - 3; i += 4) {
u16 page = be32_to_cpup(paddr + i);
@@ -425,14 +416,14 @@ static int marvell_of_reg_init(struct phy_device *phydev)
if (page != current_page) {
current_page = page;
- ret = marvell_set_page(phydev, page);
+ ret = marvell_write_page(phydev, page);
if (ret < 0)
goto err;
}
val = 0;
if (mask) {
- val = phy_read(phydev, reg);
+ val = __phy_read(phydev, reg);
if (val < 0) {
ret = val;
goto err;
@@ -441,17 +432,12 @@ static int marvell_of_reg_init(struct phy_device *phydev)
}
val |= val_bits;
- ret = phy_write(phydev, reg, val);
+ ret = __phy_write(phydev, reg, val);
if (ret < 0)
goto err;
}
err:
- if (current_page != saved_page) {
- i = marvell_set_page(phydev, saved_page);
- if (ret == 0)
- ret = i;
- }
- return ret;
+ return phy_restore_page(phydev, saved_page, ret);
}
#else
static int marvell_of_reg_init(struct phy_device *phydev)
@@ -462,34 +448,21 @@ static int marvell_of_reg_init(struct phy_device *phydev)
static int m88e1121_config_aneg_rgmii_delays(struct phy_device *phydev)
{
- int err, oldpage, mscr;
-
- oldpage = marvell_get_set_page(phydev, MII_MARVELL_MSCR_PAGE);
- if (oldpage < 0)
- return oldpage;
-
- mscr = phy_read(phydev, MII_88E1121_PHY_MSCR_REG);
- if (mscr < 0) {
- err = mscr;
- goto out;
- }
-
- mscr &= MII_88E1121_PHY_MSCR_DELAY_MASK;
+ int mscr;
if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)
- mscr |= (MII_88E1121_PHY_MSCR_RX_DELAY |
- MII_88E1121_PHY_MSCR_TX_DELAY);
+ mscr = MII_88E1121_PHY_MSCR_RX_DELAY |
+ MII_88E1121_PHY_MSCR_TX_DELAY;
else if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID)
- mscr |= MII_88E1121_PHY_MSCR_RX_DELAY;
+ mscr = MII_88E1121_PHY_MSCR_RX_DELAY;
else if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID)
- mscr |= MII_88E1121_PHY_MSCR_TX_DELAY;
-
- err = phy_write(phydev, MII_88E1121_PHY_MSCR_REG, mscr);
-
-out:
- marvell_set_page(phydev, oldpage);
+ mscr = MII_88E1121_PHY_MSCR_TX_DELAY;
+ else
+ mscr = 0;
- return err;
+ return phy_modify_paged(phydev, MII_MARVELL_MSCR_PAGE,
+ MII_88E1121_PHY_MSCR_REG,
+ MII_88E1121_PHY_MSCR_DELAY_MASK, mscr);
}
static int m88e1121_config_aneg(struct phy_device *phydev)
@@ -515,20 +488,11 @@ static int m88e1121_config_aneg(struct phy_device *phydev)
static int m88e1318_config_aneg(struct phy_device *phydev)
{
- int err, oldpage, mscr;
-
- oldpage = marvell_get_set_page(phydev, MII_MARVELL_MSCR_PAGE);
- if (oldpage < 0)
- return oldpage;
-
- mscr = phy_read(phydev, MII_88E1318S_PHY_MSCR1_REG);
- mscr |= MII_88E1318S_PHY_MSCR1_PAD_ODD;
-
- err = phy_write(phydev, MII_88E1318S_PHY_MSCR1_REG, mscr);
- if (err < 0)
- return err;
+ int err;
- err = marvell_set_page(phydev, oldpage);
+ err = phy_modify_paged(phydev, MII_MARVELL_MSCR_PAGE,
+ MII_88E1318S_PHY_MSCR1_REG,
+ 0, MII_88E1318S_PHY_MSCR1_PAD_ODD);
if (err < 0)
return err;
@@ -854,20 +818,15 @@ static int m88e1111_config_init(struct phy_device *phydev)
static int m88e1121_config_init(struct phy_device *phydev)
{
- int err, oldpage;
-
- oldpage = marvell_get_set_page(phydev, MII_MARVELL_LED_PAGE);
- if (oldpage < 0)
- return oldpage;
+ int err;
/* Default PHY LED config: LED[0] .. Link, LED[1] .. Activity */
- err = phy_write(phydev, MII_88E1121_PHY_LED_CTRL,
- MII_88E1121_PHY_LED_DEF);
+ err = phy_write_paged(phydev, MII_MARVELL_LED_PAGE,
+ MII_88E1121_PHY_LED_CTRL,
+ MII_88E1121_PHY_LED_DEF);
if (err < 0)
return err;
- marvell_set_page(phydev, oldpage);
-
/* Set marvell,reg-init configuration from device tree */
return marvell_config_init(phydev);
}
@@ -1398,100 +1357,98 @@ static int m88e1121_did_interrupt(struct phy_device *phydev)
static void m88e1318_get_wol(struct phy_device *phydev,
struct ethtool_wolinfo *wol)
{
+ int oldpage, ret = 0;
+
wol->supported = WAKE_MAGIC;
wol->wolopts = 0;
- if (marvell_set_page(phydev, MII_MARVELL_WOL_PAGE) < 0)
- return;
+ oldpage = phy_select_page(phydev, MII_MARVELL_WOL_PAGE);
+ if (oldpage < 0)
+ goto error;
- if (phy_read(phydev, MII_88E1318S_PHY_WOL_CTRL) &
- MII_88E1318S_PHY_WOL_CTRL_MAGIC_PACKET_MATCH_ENABLE)
+ ret = __phy_read(phydev, MII_88E1318S_PHY_WOL_CTRL);
+ if (ret & MII_88E1318S_PHY_WOL_CTRL_MAGIC_PACKET_MATCH_ENABLE)
wol->wolopts |= WAKE_MAGIC;
- if (marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE) < 0)
- return;
+error:
+ phy_restore_page(phydev, oldpage, ret);
}
static int m88e1318_set_wol(struct phy_device *phydev,
struct ethtool_wolinfo *wol)
{
- int err, oldpage, temp;
+ int err = 0, oldpage;
- oldpage = marvell_get_page(phydev);
+ oldpage = phy_save_page(phydev);
+ if (oldpage < 0)
+ goto error;
if (wol->wolopts & WAKE_MAGIC) {
/* Explicitly switch to page 0x00, just to be sure */
- err = marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE);
+ err = marvell_write_page(phydev, MII_MARVELL_COPPER_PAGE);
if (err < 0)
- return err;
+ goto error;
/* Enable the WOL interrupt */
- temp = phy_read(phydev, MII_88E1318S_PHY_CSIER);
- temp |= MII_88E1318S_PHY_CSIER_WOL_EIE;
- err = phy_write(phydev, MII_88E1318S_PHY_CSIER, temp);
+ err = __phy_modify(phydev, MII_88E1318S_PHY_CSIER, 0,
+ MII_88E1318S_PHY_CSIER_WOL_EIE);
if (err < 0)
- return err;
+ goto error;
- err = marvell_set_page(phydev, MII_MARVELL_LED_PAGE);
+ err = marvell_write_page(phydev, MII_MARVELL_LED_PAGE);
if (err < 0)
- return err;
+ goto error;
/* Setup LED[2] as interrupt pin (active low) */
- temp = phy_read(phydev, MII_88E1318S_PHY_LED_TCR);
- temp &= ~MII_88E1318S_PHY_LED_TCR_FORCE_INT;
- temp |= MII_88E1318S_PHY_LED_TCR_INTn_ENABLE;
- temp |= MII_88E1318S_PHY_LED_TCR_INT_ACTIVE_LOW;
- err = phy_write(phydev, MII_88E1318S_PHY_LED_TCR, temp);
+ err = __phy_modify(phydev, MII_88E1318S_PHY_LED_TCR,
+ (u16)~MII_88E1318S_PHY_LED_TCR_FORCE_INT,
+ MII_88E1318S_PHY_LED_TCR_INTn_ENABLE |
+ MII_88E1318S_PHY_LED_TCR_INT_ACTIVE_LOW);
if (err < 0)
- return err;
+ goto error;
- err = marvell_set_page(phydev, MII_MARVELL_WOL_PAGE);
+ err = marvell_write_page(phydev, MII_MARVELL_WOL_PAGE);
if (err < 0)
- return err;
+ goto error;
/* Store the device address for the magic packet */
- err = phy_write(phydev, MII_88E1318S_PHY_MAGIC_PACKET_WORD2,
+ err = __phy_write(phydev, MII_88E1318S_PHY_MAGIC_PACKET_WORD2,
((phydev->attached_dev->dev_addr[5] << 8) |
phydev->attached_dev->dev_addr[4]));
if (err < 0)
- return err;
- err = phy_write(phydev, MII_88E1318S_PHY_MAGIC_PACKET_WORD1,
+ goto error;
+ err = __phy_write(phydev, MII_88E1318S_PHY_MAGIC_PACKET_WORD1,
((phydev->attached_dev->dev_addr[3] << 8) |
phydev->attached_dev->dev_addr[2]));
if (err < 0)
- return err;
- err = phy_write(phydev, MII_88E1318S_PHY_MAGIC_PACKET_WORD0,
+ goto error;
+ err = __phy_write(phydev, MII_88E1318S_PHY_MAGIC_PACKET_WORD0,
((phydev->attached_dev->dev_addr[1] << 8) |
phydev->attached_dev->dev_addr[0]));
if (err < 0)
- return err;
+ goto error;
/* Clear WOL status and enable magic packet matching */
- temp = phy_read(phydev, MII_88E1318S_PHY_WOL_CTRL);
- temp |= MII_88E1318S_PHY_WOL_CTRL_CLEAR_WOL_STATUS;
- temp |= MII_88E1318S_PHY_WOL_CTRL_MAGIC_PACKET_MATCH_ENABLE;
- err = phy_write(phydev, MII_88E1318S_PHY_WOL_CTRL, temp);
+ err = __phy_modify(phydev, MII_88E1318S_PHY_WOL_CTRL, 0,
+ MII_88E1318S_PHY_WOL_CTRL_CLEAR_WOL_STATUS |
+ MII_88E1318S_PHY_WOL_CTRL_MAGIC_PACKET_MATCH_ENABLE);
if (err < 0)
- return err;
+ goto error;
} else {
- err = marvell_set_page(phydev, MII_MARVELL_WOL_PAGE);
+ err = marvell_write_page(phydev, MII_MARVELL_WOL_PAGE);
if (err < 0)
- return err;
+ goto error;
/* Clear WOL status and disable magic packet matching */
- temp = phy_read(phydev, MII_88E1318S_PHY_WOL_CTRL);
- temp |= MII_88E1318S_PHY_WOL_CTRL_CLEAR_WOL_STATUS;
- temp &= ~MII_88E1318S_PHY_WOL_CTRL_MAGIC_PACKET_MATCH_ENABLE;
- err = phy_write(phydev, MII_88E1318S_PHY_WOL_CTRL, temp);
+ err = __phy_modify(phydev, MII_88E1318S_PHY_WOL_CTRL,
+ (u16)~MII_88E1318S_PHY_WOL_CTRL_MAGIC_PACKET_MATCH_ENABLE,
+ MII_88E1318S_PHY_WOL_CTRL_CLEAR_WOL_STATUS);
if (err < 0)
- return err;
+ goto error;
}
- err = marvell_set_page(phydev, oldpage);
- if (err < 0)
- return err;
-
- return 0;
+error:
+ return phy_restore_page(phydev, oldpage, err);
}
static int marvell_get_sset_count(struct phy_device *phydev)
@@ -1519,14 +1476,10 @@ static u64 marvell_get_stat(struct phy_device *phydev, int i)
{
struct marvell_hw_stat stat = marvell_hw_stats[i];
struct marvell_priv *priv = phydev->priv;
- int oldpage, val;
+ int val;
u64 ret;
- oldpage = marvell_get_set_page(phydev, stat.page);
- if (oldpage < 0)
- return UINT64_MAX;
-
- val = phy_read(phydev, stat.reg);
+ val = phy_read_paged(phydev, stat.page, stat.reg);
if (val < 0) {
ret = UINT64_MAX;
} else {
@@ -1535,8 +1488,6 @@ static u64 marvell_get_stat(struct phy_device *phydev, int i)
ret = priv->stats[i];
}
- marvell_set_page(phydev, oldpage);
-
return ret;
}
@@ -1553,51 +1504,44 @@ static void marvell_get_stats(struct phy_device *phydev,
static int m88e1121_get_temp(struct phy_device *phydev, long *temp)
{
int oldpage;
- int ret;
+ int ret = 0;
int val;
*temp = 0;
- mutex_lock(&phydev->lock);
-
- oldpage = marvell_get_set_page(phydev, MII_MARVELL_MISC_TEST_PAGE);
- if (oldpage < 0) {
- mutex_unlock(&phydev->lock);
- return oldpage;
- }
+ oldpage = phy_select_page(phydev, MII_MARVELL_MISC_TEST_PAGE);
+ if (oldpage < 0)
+ goto error;
/* Enable temperature sensor */
- ret = phy_read(phydev, MII_88E1121_MISC_TEST);
+ ret = __phy_read(phydev, MII_88E1121_MISC_TEST);
if (ret < 0)
goto error;
- ret = phy_write(phydev, MII_88E1121_MISC_TEST,
- ret | MII_88E1121_MISC_TEST_TEMP_SENSOR_EN);
+ ret = __phy_write(phydev, MII_88E1121_MISC_TEST,
+ ret | MII_88E1121_MISC_TEST_TEMP_SENSOR_EN);
if (ret < 0)
goto error;
/* Wait for temperature to stabilize */
usleep_range(10000, 12000);
- val = phy_read(phydev, MII_88E1121_MISC_TEST);
+ val = __phy_read(phydev, MII_88E1121_MISC_TEST);
if (val < 0) {
ret = val;
goto error;
}
/* Disable temperature sensor */
- ret = phy_write(phydev, MII_88E1121_MISC_TEST,
- ret & ~MII_88E1121_MISC_TEST_TEMP_SENSOR_EN);
+ ret = __phy_write(phydev, MII_88E1121_MISC_TEST,
+ ret & ~MII_88E1121_MISC_TEST_TEMP_SENSOR_EN);
if (ret < 0)
goto error;
*temp = ((val & MII_88E1121_MISC_TEST_TEMP_MASK) - 5) * 5000;
error:
- marvell_set_page(phydev, oldpage);
- mutex_unlock(&phydev->lock);
-
- return ret;
+ return phy_restore_page(phydev, oldpage, ret);
}
static int m88e1121_hwmon_read(struct device *dev,
@@ -1671,118 +1615,64 @@ static const struct hwmon_chip_info m88e1121_hwmon_chip_info = {
static int m88e1510_get_temp(struct phy_device *phydev, long *temp)
{
- int oldpage;
int ret;
*temp = 0;
- mutex_lock(&phydev->lock);
-
- oldpage = marvell_get_set_page(phydev, MII_MARVELL_MISC_TEST_PAGE);
- if (oldpage < 0) {
- mutex_unlock(&phydev->lock);
- return oldpage;
- }
-
- ret = phy_read(phydev, MII_88E1510_TEMP_SENSOR);
+ ret = phy_read_paged(phydev, MII_MARVELL_MISC_TEST_PAGE,
+ MII_88E1510_TEMP_SENSOR);
if (ret < 0)
- goto error;
+ return ret;
*temp = ((ret & MII_88E1510_TEMP_SENSOR_MASK) - 25) * 1000;
-error:
- marvell_set_page(phydev, oldpage);
- mutex_unlock(&phydev->lock);
-
- return ret;
+ return 0;
}
static int m88e1510_get_temp_critical(struct phy_device *phydev, long *temp)
{
- int oldpage;
int ret;
*temp = 0;
- mutex_lock(&phydev->lock);
-
- oldpage = marvell_get_set_page(phydev, MII_MARVELL_MISC_TEST_PAGE);
- if (oldpage < 0) {
- mutex_unlock(&phydev->lock);
- return oldpage;
- }
-
- ret = phy_read(phydev, MII_88E1121_MISC_TEST);
+ ret = phy_read_paged(phydev, MII_MARVELL_MISC_TEST_PAGE,
+ MII_88E1121_MISC_TEST);
if (ret < 0)
- goto error;
+ return ret;
*temp = (((ret & MII_88E1510_MISC_TEST_TEMP_THRESHOLD_MASK) >>
MII_88E1510_MISC_TEST_TEMP_THRESHOLD_SHIFT) * 5) - 25;
/* convert to mC */
*temp *= 1000;
-error:
- marvell_set_page(phydev, oldpage);
- mutex_unlock(&phydev->lock);
-
- return ret;
+ return 0;
}
static int m88e1510_set_temp_critical(struct phy_device *phydev, long temp)
{
- int oldpage;
- int ret;
-
- mutex_lock(&phydev->lock);
-
- oldpage = marvell_get_set_page(phydev, MII_MARVELL_MISC_TEST_PAGE);
- if (oldpage < 0) {
- mutex_unlock(&phydev->lock);
- return oldpage;
- }
-
- ret = phy_read(phydev, MII_88E1121_MISC_TEST);
- if (ret < 0)
- goto error;
-
temp = temp / 1000;
temp = clamp_val(DIV_ROUND_CLOSEST(temp, 5) + 5, 0, 0x1f);
- ret = phy_write(phydev, MII_88E1121_MISC_TEST,
- (ret & ~MII_88E1510_MISC_TEST_TEMP_THRESHOLD_MASK) |
- (temp << MII_88E1510_MISC_TEST_TEMP_THRESHOLD_SHIFT));
-
-error:
- marvell_set_page(phydev, oldpage);
- mutex_unlock(&phydev->lock);
- return ret;
+ return phy_modify_paged(phydev, MII_MARVELL_MISC_TEST_PAGE,
+ MII_88E1121_MISC_TEST,
+ MII_88E1510_MISC_TEST_TEMP_THRESHOLD_MASK,
+ temp << MII_88E1510_MISC_TEST_TEMP_THRESHOLD_SHIFT);
}
static int m88e1510_get_temp_alarm(struct phy_device *phydev, long *alarm)
{
- int oldpage;
int ret;
*alarm = false;
- mutex_lock(&phydev->lock);
-
- oldpage = marvell_get_set_page(phydev, MII_MARVELL_MISC_TEST_PAGE);
- if (oldpage < 0) {
- mutex_unlock(&phydev->lock);
- return oldpage;
- }
-
- ret = phy_read(phydev, MII_88E1121_MISC_TEST);
+ ret = phy_read_paged(phydev, MII_MARVELL_MISC_TEST_PAGE,
+ MII_88E1121_MISC_TEST);
if (ret < 0)
- goto error;
- *alarm = !!(ret & MII_88E1510_MISC_TEST_TEMP_IRQ);
+ return ret;
-error:
- marvell_set_page(phydev, oldpage);
- mutex_unlock(&phydev->lock);
+ *alarm = !!(ret & MII_88E1510_MISC_TEST_TEMP_IRQ);
- return ret;
+ return 0;
}
static int m88e1510_hwmon_read(struct device *dev,
@@ -1979,6 +1869,8 @@ static struct phy_driver marvell_drivers[] = {
.config_intr = &marvell_config_intr,
.resume = &genphy_resume,
.suspend = &genphy_suspend,
+ .read_page = marvell_read_page,
+ .write_page = marvell_write_page,
.get_sset_count = marvell_get_sset_count,
.get_strings = marvell_get_strings,
.get_stats = marvell_get_stats,
@@ -1997,6 +1889,8 @@ static struct phy_driver marvell_drivers[] = {
.config_intr = &marvell_config_intr,
.resume = &genphy_resume,
.suspend = &genphy_suspend,
+ .read_page = marvell_read_page,
+ .write_page = marvell_write_page,
.get_sset_count = marvell_get_sset_count,
.get_strings = marvell_get_strings,
.get_stats = marvell_get_stats,
@@ -2015,6 +1909,8 @@ static struct phy_driver marvell_drivers[] = {
.config_intr = &marvell_config_intr,
.resume = &genphy_resume,
.suspend = &genphy_suspend,
+ .read_page = marvell_read_page,
+ .write_page = marvell_write_page,
.get_sset_count = marvell_get_sset_count,
.get_strings = marvell_get_strings,
.get_stats = marvell_get_stats,
@@ -2033,6 +1929,8 @@ static struct phy_driver marvell_drivers[] = {
.config_intr = &marvell_config_intr,
.resume = &genphy_resume,
.suspend = &genphy_suspend,
+ .read_page = marvell_read_page,
+ .write_page = marvell_write_page,
.get_sset_count = marvell_get_sset_count,
.get_strings = marvell_get_strings,
.get_stats = marvell_get_stats,
@@ -2052,6 +1950,8 @@ static struct phy_driver marvell_drivers[] = {
.did_interrupt = &m88e1121_did_interrupt,
.resume = &genphy_resume,
.suspend = &genphy_suspend,
+ .read_page = marvell_read_page,
+ .write_page = marvell_write_page,
.get_sset_count = marvell_get_sset_count,
.get_strings = marvell_get_strings,
.get_stats = marvell_get_stats,
@@ -2073,6 +1973,8 @@ static struct phy_driver marvell_drivers[] = {
.set_wol = &m88e1318_set_wol,
.resume = &genphy_resume,
.suspend = &genphy_suspend,
+ .read_page = marvell_read_page,
+ .write_page = marvell_write_page,
.get_sset_count = marvell_get_sset_count,
.get_strings = marvell_get_strings,
.get_stats = marvell_get_stats,
@@ -2091,6 +1993,8 @@ static struct phy_driver marvell_drivers[] = {
.config_intr = &marvell_config_intr,
.resume = &genphy_resume,
.suspend = &genphy_suspend,
+ .read_page = marvell_read_page,
+ .write_page = marvell_write_page,
.get_sset_count = marvell_get_sset_count,
.get_strings = marvell_get_strings,
.get_stats = marvell_get_stats,
@@ -2109,6 +2013,8 @@ static struct phy_driver marvell_drivers[] = {
.config_intr = &marvell_config_intr,
.resume = &genphy_resume,
.suspend = &genphy_suspend,
+ .read_page = marvell_read_page,
+ .write_page = marvell_write_page,
.get_sset_count = marvell_get_sset_count,
.get_strings = marvell_get_strings,
.get_stats = marvell_get_stats,
@@ -2127,6 +2033,8 @@ static struct phy_driver marvell_drivers[] = {
.config_intr = &marvell_config_intr,
.resume = &genphy_resume,
.suspend = &genphy_suspend,
+ .read_page = marvell_read_page,
+ .write_page = marvell_write_page,
.get_sset_count = marvell_get_sset_count,
.get_strings = marvell_get_strings,
.get_stats = marvell_get_stats,
@@ -2145,6 +2053,8 @@ static struct phy_driver marvell_drivers[] = {
.config_intr = &marvell_config_intr,
.resume = &genphy_resume,
.suspend = &genphy_suspend,
+ .read_page = marvell_read_page,
+ .write_page = marvell_write_page,
.get_sset_count = marvell_get_sset_count,
.get_strings = marvell_get_strings,
.get_stats = marvell_get_stats,
@@ -2166,6 +2076,8 @@ static struct phy_driver marvell_drivers[] = {
.set_wol = &m88e1318_set_wol,
.resume = &marvell_resume,
.suspend = &marvell_suspend,
+ .read_page = marvell_read_page,
+ .write_page = marvell_write_page,
.get_sset_count = marvell_get_sset_count,
.get_strings = marvell_get_strings,
.get_stats = marvell_get_stats,
@@ -2186,6 +2098,8 @@ static struct phy_driver marvell_drivers[] = {
.did_interrupt = &m88e1121_did_interrupt,
.resume = &genphy_resume,
.suspend = &genphy_suspend,
+ .read_page = marvell_read_page,
+ .write_page = marvell_write_page,
.get_sset_count = marvell_get_sset_count,
.get_strings = marvell_get_strings,
.get_stats = marvell_get_stats,
@@ -2205,6 +2119,8 @@ static struct phy_driver marvell_drivers[] = {
.did_interrupt = &m88e1121_did_interrupt,
.resume = &genphy_resume,
.suspend = &genphy_suspend,
+ .read_page = marvell_read_page,
+ .write_page = marvell_write_page,
.get_sset_count = marvell_get_sset_count,
.get_strings = marvell_get_strings,
.get_stats = marvell_get_stats,
@@ -2225,6 +2141,8 @@ static struct phy_driver marvell_drivers[] = {
.did_interrupt = &m88e1121_did_interrupt,
.resume = &genphy_resume,
.suspend = &genphy_suspend,
+ .read_page = marvell_read_page,
+ .write_page = marvell_write_page,
.get_sset_count = marvell_get_sset_count,
.get_strings = marvell_get_strings,
.get_stats = marvell_get_stats,
@@ -2244,6 +2162,8 @@ static struct phy_driver marvell_drivers[] = {
.did_interrupt = &m88e1121_did_interrupt,
.resume = &genphy_resume,
.suspend = &genphy_suspend,
+ .read_page = marvell_read_page,
+ .write_page = marvell_write_page,
.get_sset_count = marvell_get_sset_count,
.get_strings = marvell_get_strings,
.get_stats = marvell_get_stats,
--
2.7.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH net-next v2 5/7] net: phy: marvell: fix paged access races
2018-01-02 10:58 ` [PATCH net-next v2 5/7] net: phy: marvell: fix paged access races Russell King
@ 2018-01-02 16:00 ` Andrew Lunn
0 siblings, 0 replies; 16+ messages in thread
From: Andrew Lunn @ 2018-01-02 16:00 UTC (permalink / raw)
To: Russell King; +Cc: Florian Fainelli, netdev
On Tue, Jan 02, 2018 at 10:58:48AM +0000, Russell King wrote:
> For paged accesses to be truely safe, we need to hold the bus lock to
> prevent anyone else gaining access to the registers while we modify
> them.
>
> The phydev->lock mutex does not do this: userspace via the MII ioctl
> can still sneak in and read or write any register while we are on a
> different page, and the suspend/resume methods can be called by a
> thread different to the thread polling the phy status.
>
> Races have been observed with mvneta on SolidRun Clearfog with phylink,
> particularly between the phylib worker reading the PHYs status, and
> the thread resuming mvneta, calling phy_start() which then calls
> through to m88e1121_config_aneg_rgmii_delays(), which tries to
> read-modify-write the MSCR register:
>
> CPU0 CPU1
> marvell_read_status_page()
> marvell_set_page(phydev, MII_MARVELL_FIBER_PAGE)
> ...
> m88e1121_config_aneg_rgmii_delays()
> set_page(MII_MARVELL_MSCR_PAGE)
> phy_read(phydev, MII_88E1121_PHY_MSCR_REG)
> marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE);
> ...
> phy_write(phydev, MII_88E1121_PHY_MSCR_REG)
>
> The result of this is we end up writing the copper page register 21,
> which causes the copper PHY to be disabled, and the link partner sees
> the link immediately go down.
>
> Solve this by taking the bus lock instead of the PHY lock, thereby
> preventing other accesses to the PHY while we are accessing other PHY
> pages.
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH net-next v2 6/7] net: phy: add phy_modify() accessor
2018-01-02 10:52 [PATCH net-next v2 0/7] Resolve races in phy accessors Russell King - ARM Linux
` (4 preceding siblings ...)
2018-01-02 10:58 ` [PATCH net-next v2 5/7] net: phy: marvell: fix paged access races Russell King
@ 2018-01-02 10:58 ` Russell King
2018-01-02 16:01 ` Andrew Lunn
2018-01-02 10:58 ` [PATCH net-next v2 7/7] net: phy: convert read-modify-write to phy_modify() Russell King
2018-01-03 16:04 ` [PATCH net-next v2 0/7] Resolve races in phy accessors David Miller
7 siblings, 1 reply; 16+ messages in thread
From: Russell King @ 2018-01-02 10:58 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli; +Cc: netdev
Add phy_modify() convenience accessor to complement the mdiobus
counterpart.
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
drivers/net/phy/phy-core.c | 23 +++++++++++++++++++++++
include/linux/phy.h | 1 +
2 files changed, 24 insertions(+)
diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
index 412ed8ff50e2..a2168adea81f 100644
--- a/drivers/net/phy/phy-core.c
+++ b/drivers/net/phy/phy-core.c
@@ -306,6 +306,29 @@ int __phy_modify(struct phy_device *phydev, u32 regnum, u16 mask, u16 set)
}
EXPORT_SYMBOL_GPL(__phy_modify);
+/**
+ * phy_modify - Convenience function for modifying a given PHY register
+ * @phydev: the phy_device struct
+ * @regnum: register number to write
+ * @mask: bit mask of bits to clear
+ * @set: new value of bits set in mask to write to @regnum
+ *
+ * NOTE: MUST NOT be called from interrupt context,
+ * because the bus read/write functions may wait for an interrupt
+ * to conclude the operation.
+ */
+int phy_modify(struct phy_device *phydev, u32 regnum, u16 mask, u16 set)
+{
+ int ret;
+
+ mutex_lock(&phydev->mdio.bus->mdio_lock);
+ ret = __phy_modify(phydev, regnum, mask, set);
+ mutex_unlock(&phydev->mdio.bus->mdio_lock);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(phy_modify);
+
static int __phy_read_page(struct phy_device *phydev)
{
return phydev->drv->read_page(phydev);
diff --git a/include/linux/phy.h b/include/linux/phy.h
index f408220d95ec..0ee4ece312da 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -760,6 +760,7 @@ static inline int __phy_write(struct phy_device *phydev, u32 regnum, u16 val)
}
int __phy_modify(struct phy_device *phydev, u32 regnum, u16 mask, u16 set);
+int phy_modify(struct phy_device *phydev, u32 regnum, u16 mask, u16 set);
/**
* phy_interrupt_is_valid - Convenience function for testing a given PHY irq
--
2.7.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH net-next v2 7/7] net: phy: convert read-modify-write to phy_modify()
2018-01-02 10:52 [PATCH net-next v2 0/7] Resolve races in phy accessors Russell King - ARM Linux
` (5 preceding siblings ...)
2018-01-02 10:58 ` [PATCH net-next v2 6/7] net: phy: add phy_modify() accessor Russell King
@ 2018-01-02 10:58 ` Russell King
2018-01-02 16:05 ` Andrew Lunn
2018-01-03 16:04 ` [PATCH net-next v2 0/7] Resolve races in phy accessors David Miller
7 siblings, 1 reply; 16+ messages in thread
From: Russell King @ 2018-01-02 10:58 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli; +Cc: netdev
Convert read-modify-write sequences in at803x, Marvell and core phylib
to use phy_modify() to ensure safety.
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
drivers/net/phy/at803x.c | 20 ++--------
drivers/net/phy/marvell.c | 88 +++++++++++++++++---------------------------
drivers/net/phy/phy_device.c | 50 +++++--------------------
3 files changed, 46 insertions(+), 112 deletions(-)
diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index e911e4990b20..e86c1b8b1b51 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -216,34 +216,22 @@ static int at803x_suspend(struct phy_device *phydev)
int value;
int wol_enabled;
- mutex_lock(&phydev->lock);
-
value = phy_read(phydev, AT803X_INTR_ENABLE);
wol_enabled = value & AT803X_INTR_ENABLE_WOL;
- value = phy_read(phydev, MII_BMCR);
-
if (wol_enabled)
- value |= BMCR_ISOLATE;
+ value = BMCR_ISOLATE;
else
- value |= BMCR_PDOWN;
+ value = BMCR_PDOWN;
- phy_write(phydev, MII_BMCR, value);
-
- mutex_unlock(&phydev->lock);
+ phy_modify(phydev, MII_BMCR, 0, value);
return 0;
}
static int at803x_resume(struct phy_device *phydev)
{
- int value;
-
- value = phy_read(phydev, MII_BMCR);
- value &= ~(BMCR_PDOWN | BMCR_ISOLATE);
- phy_write(phydev, MII_BMCR, value);
-
- return 0;
+ return phy_modify(phydev, MII_BMCR, ~(BMCR_PDOWN | BMCR_ISOLATE), 0);
}
static int at803x_probe(struct phy_device *phydev)
diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index 1cb84064d658..8212c2fd7fe1 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -471,7 +471,7 @@ static int m88e1121_config_aneg(struct phy_device *phydev)
if (phy_interface_is_rgmii(phydev)) {
err = m88e1121_config_aneg_rgmii_delays(phydev);
- if (err)
+ if (err < 0)
return err;
}
@@ -664,19 +664,14 @@ static int m88e1116r_config_init(struct phy_device *phydev)
static int m88e3016_config_init(struct phy_device *phydev)
{
- int reg;
+ int ret;
/* Enable Scrambler and Auto-Crossover */
- reg = phy_read(phydev, MII_88E3016_PHY_SPEC_CTRL);
- if (reg < 0)
- return reg;
-
- reg &= ~MII_88E3016_DISABLE_SCRAMBLER;
- reg |= MII_88E3016_AUTO_MDIX_CROSSOVER;
-
- reg = phy_write(phydev, MII_88E3016_PHY_SPEC_CTRL, reg);
- if (reg < 0)
- return reg;
+ ret = phy_modify(phydev, MII_88E3016_PHY_SPEC_CTRL,
+ ~MII_88E3016_DISABLE_SCRAMBLER,
+ MII_88E3016_AUTO_MDIX_CROSSOVER);
+ if (ret < 0)
+ return ret;
return marvell_config_init(phydev);
}
@@ -685,42 +680,34 @@ static int m88e1111_config_init_hwcfg_mode(struct phy_device *phydev,
u16 mode,
int fibre_copper_auto)
{
- int temp;
-
- temp = phy_read(phydev, MII_M1111_PHY_EXT_SR);
- if (temp < 0)
- return temp;
-
- temp &= ~(MII_M1111_HWCFG_MODE_MASK |
- MII_M1111_HWCFG_FIBER_COPPER_AUTO |
- MII_M1111_HWCFG_FIBER_COPPER_RES);
- temp |= mode;
-
if (fibre_copper_auto)
- temp |= MII_M1111_HWCFG_FIBER_COPPER_AUTO;
+ mode |= MII_M1111_HWCFG_FIBER_COPPER_AUTO;
- return phy_write(phydev, MII_M1111_PHY_EXT_SR, temp);
+ return phy_modify(phydev, MII_M1111_PHY_EXT_SR,
+ (u16)~(MII_M1111_HWCFG_MODE_MASK |
+ MII_M1111_HWCFG_FIBER_COPPER_AUTO |
+ MII_M1111_HWCFG_FIBER_COPPER_RES),
+ mode);
}
static int m88e1111_config_init_rgmii_delays(struct phy_device *phydev)
{
- int temp;
-
- temp = phy_read(phydev, MII_M1111_PHY_EXT_CR);
- if (temp < 0)
- return temp;
+ int delay;
if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) {
- temp |= (MII_M1111_RGMII_RX_DELAY | MII_M1111_RGMII_TX_DELAY);
+ delay = MII_M1111_RGMII_RX_DELAY | MII_M1111_RGMII_TX_DELAY;
} else if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID) {
- temp &= ~MII_M1111_RGMII_TX_DELAY;
- temp |= MII_M1111_RGMII_RX_DELAY;
+ delay = MII_M1111_RGMII_RX_DELAY;
} else if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID) {
- temp &= ~MII_M1111_RGMII_RX_DELAY;
- temp |= MII_M1111_RGMII_TX_DELAY;
+ delay = MII_M1111_RGMII_TX_DELAY;
+ } else {
+ delay = 0;
}
- return phy_write(phydev, MII_M1111_PHY_EXT_CR, temp);
+ return phy_modify(phydev, MII_M1111_PHY_EXT_CR,
+ (u16)~(MII_M1111_RGMII_RX_DELAY |
+ MII_M1111_RGMII_TX_DELAY),
+ delay);
}
static int m88e1111_config_init_rgmii(struct phy_device *phydev)
@@ -766,7 +753,7 @@ static int m88e1111_config_init_rtbi(struct phy_device *phydev)
int err;
err = m88e1111_config_init_rgmii_delays(phydev);
- if (err)
+ if (err < 0)
return err;
err = m88e1111_config_init_hwcfg_mode(
@@ -793,7 +780,7 @@ static int m88e1111_config_init(struct phy_device *phydev)
if (phy_interface_is_rgmii(phydev)) {
err = m88e1111_config_init_rgmii(phydev);
- if (err)
+ if (err < 0)
return err;
}
@@ -834,7 +821,6 @@ static int m88e1121_config_init(struct phy_device *phydev)
static int m88e1510_config_init(struct phy_device *phydev)
{
int err;
- int temp;
/* SGMII-to-Copper mode initialization */
if (phydev->interface == PHY_INTERFACE_MODE_SGMII) {
@@ -846,16 +832,15 @@ static int m88e1510_config_init(struct phy_device *phydev)
return err;
/* In reg 20, write MODE[2:0] = 0x1 (SGMII to Copper) */
- temp = phy_read(phydev, MII_88E1510_GEN_CTRL_REG_1);
- temp &= ~MII_88E1510_GEN_CTRL_REG_1_MODE_MASK;
- temp |= MII_88E1510_GEN_CTRL_REG_1_MODE_SGMII;
- err = phy_write(phydev, MII_88E1510_GEN_CTRL_REG_1, temp);
+ err = phy_modify(phydev, MII_88E1510_GEN_CTRL_REG_1,
+ ~MII_88E1510_GEN_CTRL_REG_1_MODE_MASK,
+ MII_88E1510_GEN_CTRL_REG_1_MODE_SGMII);
if (err < 0)
return err;
/* PHY reset is necessary after changing MODE[2:0] */
- temp |= MII_88E1510_GEN_CTRL_REG_1_RESET;
- err = phy_write(phydev, MII_88E1510_GEN_CTRL_REG_1, temp);
+ err = phy_modify(phydev, MII_88E1510_GEN_CTRL_REG_1, 0,
+ MII_88E1510_GEN_CTRL_REG_1_RESET);
if (err < 0)
return err;
@@ -961,7 +946,6 @@ static int m88e1149_config_init(struct phy_device *phydev)
static int m88e1145_config_init_rgmii(struct phy_device *phydev)
{
- int temp;
int err;
err = m88e1111_config_init_rgmii_delays(phydev);
@@ -973,15 +957,9 @@ static int m88e1145_config_init_rgmii(struct phy_device *phydev)
if (err < 0)
return err;
- temp = phy_read(phydev, 0x1e);
- if (temp < 0)
- return temp;
-
- temp &= 0xf03f;
- temp |= 2 << 9; /* 36 ohm */
- temp |= 2 << 6; /* 39 ohm */
-
- err = phy_write(phydev, 0x1e, temp);
+ err = phy_modify(phydev, 0x1e, 0xf03f,
+ 2 << 9 | /* 36 ohm */
+ 2 << 6); /* 39 ohm */
if (err < 0)
return err;
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index b15b31ca2618..e5ddc5ae56d1 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1328,9 +1328,8 @@ static int genphy_config_eee_advert(struct phy_device *phydev)
*/
int genphy_setup_forced(struct phy_device *phydev)
{
- int ctl = phy_read(phydev, MII_BMCR);
+ u16 ctl = 0;
- ctl &= BMCR_LOOPBACK | BMCR_ISOLATE | BMCR_PDOWN;
phydev->pause = 0;
phydev->asym_pause = 0;
@@ -1342,7 +1341,8 @@ int genphy_setup_forced(struct phy_device *phydev)
if (DUPLEX_FULL == phydev->duplex)
ctl |= BMCR_FULLDPLX;
- return phy_write(phydev, MII_BMCR, ctl);
+ return phy_modify(phydev, MII_BMCR,
+ BMCR_LOOPBACK | BMCR_ISOLATE | BMCR_PDOWN, ctl);
}
EXPORT_SYMBOL(genphy_setup_forced);
@@ -1352,17 +1352,9 @@ EXPORT_SYMBOL(genphy_setup_forced);
*/
int genphy_restart_aneg(struct phy_device *phydev)
{
- int ctl = phy_read(phydev, MII_BMCR);
-
- if (ctl < 0)
- return ctl;
-
- ctl |= BMCR_ANENABLE | BMCR_ANRESTART;
-
/* Don't isolate the PHY if we're negotiating */
- ctl &= ~BMCR_ISOLATE;
-
- return phy_write(phydev, MII_BMCR, ctl);
+ return phy_modify(phydev, MII_BMCR, ~BMCR_ISOLATE,
+ BMCR_ANENABLE | BMCR_ANRESTART);
}
EXPORT_SYMBOL(genphy_restart_aneg);
@@ -1628,44 +1620,20 @@ EXPORT_SYMBOL(genphy_config_init);
int genphy_suspend(struct phy_device *phydev)
{
- int value;
-
- mutex_lock(&phydev->lock);
-
- value = phy_read(phydev, MII_BMCR);
- phy_write(phydev, MII_BMCR, value | BMCR_PDOWN);
-
- mutex_unlock(&phydev->lock);
-
- return 0;
+ return phy_modify(phydev, MII_BMCR, 0, BMCR_PDOWN);
}
EXPORT_SYMBOL(genphy_suspend);
int genphy_resume(struct phy_device *phydev)
{
- int value;
-
- value = phy_read(phydev, MII_BMCR);
- phy_write(phydev, MII_BMCR, value & ~BMCR_PDOWN);
-
- return 0;
+ return phy_modify(phydev, MII_BMCR, ~BMCR_PDOWN, 0);
}
EXPORT_SYMBOL(genphy_resume);
int genphy_loopback(struct phy_device *phydev, bool enable)
{
- int value;
-
- value = phy_read(phydev, MII_BMCR);
- if (value < 0)
- return value;
-
- if (enable)
- value |= BMCR_LOOPBACK;
- else
- value &= ~BMCR_LOOPBACK;
-
- return phy_write(phydev, MII_BMCR, value);
+ return phy_modify(phydev, MII_BMCR, ~BMCR_LOOPBACK,
+ enable ? BMCR_LOOPBACK : 0);
}
EXPORT_SYMBOL(genphy_loopback);
--
2.7.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH net-next v2 0/7] Resolve races in phy accessors
2018-01-02 10:52 [PATCH net-next v2 0/7] Resolve races in phy accessors Russell King - ARM Linux
` (6 preceding siblings ...)
2018-01-02 10:58 ` [PATCH net-next v2 7/7] net: phy: convert read-modify-write to phy_modify() Russell King
@ 2018-01-03 16:04 ` David Miller
2018-01-03 17:32 ` Russell King - ARM Linux
7 siblings, 1 reply; 16+ messages in thread
From: David Miller @ 2018-01-03 16:04 UTC (permalink / raw)
To: linux; +Cc: andrew, f.fainelli, netdev
From: Russell King - ARM Linux <linux@armlinux.org.uk>
Date: Tue, 2 Jan 2018 10:52:18 +0000
> This series resolves races with various accesses to PHY registers.
> The first five patches are necessary before we add phylink support
> to mvneta, the remaining three are merely cleanups for unobserved
> races, and hence are less critical.
Series applied.
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH net-next v2 0/7] Resolve races in phy accessors
2018-01-03 16:04 ` [PATCH net-next v2 0/7] Resolve races in phy accessors David Miller
@ 2018-01-03 17:32 ` Russell King - ARM Linux
2018-01-03 18:38 ` David Miller
0 siblings, 1 reply; 16+ messages in thread
From: Russell King - ARM Linux @ 2018-01-03 17:32 UTC (permalink / raw)
To: David Miller; +Cc: andrew, f.fainelli, netdev
On Wed, Jan 03, 2018 at 11:04:31AM -0500, David Miller wrote:
> From: Russell King - ARM Linux <linux@armlinux.org.uk>
> Date: Tue, 2 Jan 2018 10:52:18 +0000
>
> > This series resolves races with various accesses to PHY registers.
> > The first five patches are necessary before we add phylink support
> > to mvneta, the remaining three are merely cleanups for unobserved
> > races, and hence are less critical.
>
> Series applied.
aieee, this should have been applied before the mvneta patch set (as
mentioned in the covering email to that set, as well as the cover
message you quoted above) because the mvneta patch set makes these
races visible. Well, I guess folk are going to have to put up with
bisect issues if they hit a commit between the two merges in your tree.
Well, I guess what's done is done, I did my best to avoid it being
visible.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next v2 0/7] Resolve races in phy accessors
2018-01-03 17:32 ` Russell King - ARM Linux
@ 2018-01-03 18:38 ` David Miller
0 siblings, 0 replies; 16+ messages in thread
From: David Miller @ 2018-01-03 18:38 UTC (permalink / raw)
To: linux; +Cc: andrew, f.fainelli, netdev
From: Russell King - ARM Linux <linux@armlinux.org.uk>
Date: Wed, 3 Jan 2018 17:32:55 +0000
> On Wed, Jan 03, 2018 at 11:04:31AM -0500, David Miller wrote:
>> From: Russell King - ARM Linux <linux@armlinux.org.uk>
>> Date: Tue, 2 Jan 2018 10:52:18 +0000
>>
>> > This series resolves races with various accesses to PHY registers.
>> > The first five patches are necessary before we add phylink support
>> > to mvneta, the remaining three are merely cleanups for unobserved
>> > races, and hence are less critical.
>>
>> Series applied.
>
> aieee, this should have been applied before the mvneta patch set (as
> mentioned in the covering email to that set, as well as the cover
> message you quoted above) because the mvneta patch set makes these
> races visible. Well, I guess folk are going to have to put up with
> bisect issues if they hit a commit between the two merges in your tree.
>
> Well, I guess what's done is done, I did my best to avoid it being
> visible.
Sorry for not catching this, my bad.
^ permalink raw reply [flat|nested] 16+ messages in thread