* [PATCH 1/4] net: phy: introduce phy_aneg_done
2014-09-18 7:11 net: phy updates Sascha Hauer
@ 2014-09-18 7:11 ` Sascha Hauer
2014-09-18 7:11 ` [PATCH 2/4] net: phy: Use xzalloc for small allocations Sascha Hauer
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Sascha Hauer @ 2014-09-18 7:11 UTC (permalink / raw)
To: barebox
phy_wait_aneg_done() is directly called by the network code, so it
should not read phy registers directly. Introduce phy_aneg_done to
give phy drivers the chance to do something different to poll for
autonegotiation completion.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
drivers/net/phy/phy.c | 57 +++++++++++++++++++++++++++++++++++++++++----------
include/linux/phy.h | 4 ++++
2 files changed, 50 insertions(+), 11 deletions(-)
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index cad4cf5..7604e1d 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -31,6 +31,21 @@
static struct phy_driver genphy_driver;
static int genphy_config_init(struct phy_device *phydev);
+/**
+ * phy_aneg_done - return auto-negotiation status
+ * @phydev: target phy_device struct
+ *
+ * Description: Return the auto-negotiation status from this @phydev
+ * Returns > 0 on success or < 0 on error. 0 means that auto-negotiation
+ * is still pending.
+ */
+static int phy_aneg_done(struct phy_device *phydev)
+{
+ struct phy_driver *drv = to_phy_driver(phydev->dev.driver);
+
+ return drv->aneg_done(phydev);
+}
+
int phy_update_status(struct phy_device *dev)
{
struct phy_driver *drv = to_phy_driver(dev->dev.driver);
@@ -477,25 +492,15 @@ int genphy_setup_forced(struct phy_device *phydev)
int phy_wait_aneg_done(struct phy_device *phydev)
{
uint64_t start = get_time_ns();
- int ctl;
if (phydev->autoneg == AUTONEG_DISABLE)
return 0;
while (!is_timeout(start, PHY_AN_TIMEOUT * SECOND)) {
- ctl = phy_read(phydev, MII_BMSR);
- if (ctl & BMSR_ANEGCOMPLETE) {
+ if (phy_aneg_done(phydev) > 0) {
phydev->link = 1;
return 0;
}
-
- /* Restart auto-negotiation if remote fault */
- if (ctl & BMSR_RFAULT) {
- puts("PHY remote fault detected\n"
- "PHY restarting auto-negotiation\n");
- phy_write(phydev, MII_BMCR,
- BMCR_ANENABLE | BMCR_ANRESTART);
- }
}
phydev->link = 0;
@@ -572,6 +577,33 @@ int genphy_config_aneg(struct phy_device *phydev)
}
/**
+ * genphy_aneg_done - return auto-negotiation status
+ * @phydev: target phy_device struct
+ *
+ * Description: Reads the status register and returns 0 either if
+ * auto-negotiation is incomplete, or if there was an error.
+ * Returns BMSR_ANEGCOMPLETE if auto-negotiation is done.
+ */
+int genphy_aneg_done(struct phy_device *phydev)
+{
+ int bmsr = phy_read(phydev, MII_BMSR);
+
+ if (bmsr < 0)
+ return bmsr;
+
+ /* Restart auto-negotiation if remote fault */
+ if (bmsr & BMSR_RFAULT) {
+ puts("PHY remote fault detected\n"
+ "PHY restarting auto-negotiation\n");
+ phy_write(phydev, MII_BMCR,
+ BMCR_ANENABLE | BMCR_ANRESTART);
+ }
+
+ return bmsr & BMSR_ANEGCOMPLETE;
+}
+EXPORT_SYMBOL(genphy_aneg_done);
+
+/**
* genphy_update_link - update link status in @phydev
* @phydev: target phy_device struct
*
@@ -825,6 +857,9 @@ int phy_driver_register(struct phy_driver *phydrv)
if (!phydrv->config_aneg)
phydrv->config_aneg = genphy_config_aneg;
+ if (!phydrv->aneg_done)
+ phydrv->aneg_done = genphy_aneg_done;
+
if (!phydrv->read_status)
phydrv->read_status = genphy_read_status;
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 7c5d754..c0fd4ff 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -234,6 +234,9 @@ struct phy_driver {
*/
int (*config_aneg)(struct phy_device *phydev);
+ /* Determines the auto negotiation result */
+ int (*aneg_done)(struct phy_device *phydev);
+
/* Determines the negotiated speed and duplex */
int (*read_status)(struct phy_device *phydev);
@@ -295,6 +298,7 @@ int phy_wait_aneg_done(struct phy_device *phydev);
/* Generic PHY support and helper functions */
int genphy_restart_aneg(struct phy_device *phydev);
int genphy_config_aneg(struct phy_device *phydev);
+int genphy_aneg_done(struct phy_device *phydev);
int genphy_update_link(struct phy_device *phydev);
int genphy_read_status(struct phy_device *phydev);
int genphy_config_advert(struct phy_device *phydev);
--
2.1.0
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 2/4] net: phy: Use xzalloc for small allocations
2014-09-18 7:11 net: phy updates Sascha Hauer
2014-09-18 7:11 ` [PATCH 1/4] net: phy: introduce phy_aneg_done Sascha Hauer
@ 2014-09-18 7:11 ` Sascha Hauer
2014-09-18 7:11 ` [PATCH 3/4] net: phy: don't use 'dev' as name for variables of type struct phy_device Sascha Hauer
2014-09-18 7:11 ` [PATCH 4/4] net: phy: Use poller for periodic link check Sascha Hauer
3 siblings, 0 replies; 6+ messages in thread
From: Sascha Hauer @ 2014-09-18 7:11 UTC (permalink / raw)
To: barebox
No need to call kzalloc for small allocations, xzalloc will do without
the need for an additional check.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
drivers/net/phy/phy.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 7604e1d..7813c49 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -86,9 +86,7 @@ int phy_register_fixup(const char *bus_id, u32 phy_uid, u32 phy_uid_mask,
{
struct phy_fixup *fixup;
- fixup = kzalloc(sizeof(struct phy_fixup), GFP_KERNEL);
- if (!fixup)
- return -ENOMEM;
+ fixup = xzalloc(sizeof(struct phy_fixup));
strlcpy(fixup->bus_id, bus_id, sizeof(fixup->bus_id));
fixup->phy_uid = phy_uid;
@@ -157,10 +155,7 @@ static struct phy_device *phy_device_create(struct mii_bus *bus, int addr, int p
/* We allocate the device, and initialize the
* default values */
- dev = kzalloc(sizeof(*dev), GFP_KERNEL);
-
- if (NULL == dev)
- return (struct phy_device*) PTR_ERR((void*)-ENOMEM);
+ dev = xzalloc(sizeof(*dev));
dev->speed = 0;
dev->duplex = -1;
--
2.1.0
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 3/4] net: phy: don't use 'dev' as name for variables of type struct phy_device
2014-09-18 7:11 net: phy updates Sascha Hauer
2014-09-18 7:11 ` [PATCH 1/4] net: phy: introduce phy_aneg_done Sascha Hauer
2014-09-18 7:11 ` [PATCH 2/4] net: phy: Use xzalloc for small allocations Sascha Hauer
@ 2014-09-18 7:11 ` Sascha Hauer
2014-09-18 7:11 ` [PATCH 4/4] net: phy: Use poller for periodic link check Sascha Hauer
3 siblings, 0 replies; 6+ messages in thread
From: Sascha Hauer @ 2014-09-18 7:11 UTC (permalink / raw)
To: barebox
Using 'dev' as name for variables which are not of type
struct device(_d) is bad habit and leads to confusions. Use phydev
instead.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
drivers/net/phy/phy.c | 78 +++++++++++++++++++++++++--------------------------
1 file changed, 39 insertions(+), 39 deletions(-)
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 7813c49..f3dffca 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -46,26 +46,26 @@ static int phy_aneg_done(struct phy_device *phydev)
return drv->aneg_done(phydev);
}
-int phy_update_status(struct phy_device *dev)
+int phy_update_status(struct phy_device *phydev)
{
- struct phy_driver *drv = to_phy_driver(dev->dev.driver);
- struct eth_device *edev = dev->attached_dev;
+ struct phy_driver *drv = to_phy_driver(phydev->dev.driver);
+ struct eth_device *edev = phydev->attached_dev;
int ret;
- int oldspeed = dev->speed, oldduplex = dev->duplex;
+ int oldspeed = phydev->speed, oldduplex = phydev->duplex;
- ret = drv->read_status(dev);
+ ret = drv->read_status(phydev);
if (ret)
return ret;
- if (dev->speed == oldspeed && dev->duplex == oldduplex)
+ if (phydev->speed == oldspeed && phydev->duplex == oldduplex)
return 0;
- if (dev->adjust_link)
- dev->adjust_link(edev);
+ if (phydev->adjust_link)
+ phydev->adjust_link(edev);
- if (dev->link)
- dev_info(&edev->dev, "%dMbps %s duplex link detected\n", dev->speed,
- dev->duplex ? "full" : "half");
+ if (phydev->link)
+ dev_info(&edev->dev, "%dMbps %s duplex link detected\n",
+ phydev->speed, phydev->duplex ? "full" : "half");
return 0;
}
@@ -151,28 +151,28 @@ int phy_scan_fixups(struct phy_device *phydev)
static struct phy_device *phy_device_create(struct mii_bus *bus, int addr, int phy_id)
{
- struct phy_device *dev;
+ struct phy_device *phydev;
/* We allocate the device, and initialize the
* default values */
- dev = xzalloc(sizeof(*dev));
+ phydev = xzalloc(sizeof(*phydev));
- dev->speed = 0;
- dev->duplex = -1;
- dev->pause = dev->asym_pause = 0;
- dev->link = 1;
- dev->autoneg = AUTONEG_ENABLE;
+ phydev->speed = 0;
+ phydev->duplex = -1;
+ phydev->pause = phydev->asym_pause = 0;
+ phydev->link = 1;
+ phydev->autoneg = AUTONEG_ENABLE;
- dev->addr = addr;
- dev->phy_id = phy_id;
+ phydev->addr = addr;
+ phydev->phy_id = phy_id;
- dev->bus = bus;
- dev->dev.bus = &mdio_bus_type;
+ phydev->bus = bus;
+ phydev->dev.bus = &mdio_bus_type;
- strcpy(dev->dev.name, "phy");
- dev->dev.id = DEVICE_ID_DYNAMIC;
+ strcpy(phydev->dev.name, "phy");
+ phydev->dev.id = DEVICE_ID_DYNAMIC;
- return dev;
+ return phydev;
}
/**
* get_phy_id - reads the specified addr for its ID.
@@ -217,7 +217,7 @@ int get_phy_id(struct mii_bus *bus, int addr, u32 *phy_id)
*/
struct phy_device *get_phy_device(struct mii_bus *bus, int addr)
{
- struct phy_device *dev = NULL;
+ struct phy_device *phydev = NULL;
u32 phy_id = 0;
int r;
@@ -229,9 +229,9 @@ struct phy_device *get_phy_device(struct mii_bus *bus, int addr)
if ((phy_id & 0x1fffffff) == 0x1fffffff)
return ERR_PTR(-ENODEV);
- dev = phy_device_create(bus, addr, phy_id);
+ phydev = phy_device_create(bus, addr, phy_id);
- return dev;
+ return phydev;
}
static void phy_config_aneg(struct phy_device *phydev)
@@ -242,31 +242,31 @@ static void phy_config_aneg(struct phy_device *phydev)
drv->config_aneg(phydev);
}
-int phy_register_device(struct phy_device* dev)
+int phy_register_device(struct phy_device *phydev)
{
int ret;
- if (dev->registered)
+ if (phydev->registered)
return -EBUSY;
- dev->dev.parent = &dev->bus->dev;
+ phydev->dev.parent = &phydev->bus->dev;
- ret = register_device(&dev->dev);
+ ret = register_device(&phydev->dev);
if (ret)
return ret;
- dev->bus->phy_map[dev->addr] = dev;
+ phydev->bus->phy_map[phydev->addr] = phydev;
- dev->registered = 1;
+ phydev->registered = 1;
- if (dev->dev.driver)
+ if (phydev->dev.driver)
return 0;
- dev->dev.driver = &genphy_driver.drv;
- ret = device_probe(&dev->dev);
+ phydev->dev.driver = &genphy_driver.drv;
+ ret = device_probe(&phydev->dev);
if (ret) {
- unregister_device(&dev->dev);
- dev->registered = 0;
+ unregister_device(&phydev->dev);
+ phydev->registered = 0;
}
return ret;
--
2.1.0
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 4/4] net: phy: Use poller for periodic link check
2014-09-18 7:11 net: phy updates Sascha Hauer
` (2 preceding siblings ...)
2014-09-18 7:11 ` [PATCH 3/4] net: phy: don't use 'dev' as name for variables of type struct phy_device Sascha Hauer
@ 2014-09-18 7:11 ` Sascha Hauer
2014-09-24 5:09 ` Sascha Hauer
3 siblings, 1 reply; 6+ messages in thread
From: Sascha Hauer @ 2014-09-18 7:11 UTC (permalink / raw)
To: barebox
This continuously updates the link status in the background. The networking
code no longer has to periodically update the link status itself but instead
can only check for phydev->link.
With this we also always have link status changes printed to the console.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
drivers/net/Kconfig | 1 +
drivers/net/phy/phy.c | 57 +++++++++++++++++++++++++++++++++++++++++----------
net/eth.c | 23 +++++++--------------
3 files changed, 54 insertions(+), 27 deletions(-)
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index c99fcc8..b4dda53 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -20,6 +20,7 @@ config HAS_NETX_ETHER
bool
config PHYLIB
+ select POLLER
bool
menu "Network drivers"
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index f3dffca..980d81d 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -22,6 +22,7 @@
#include <init.h>
#include <net.h>
#include <malloc.h>
+#include <poller.h>
#include <linux/phy.h>
#include <linux/phy.h>
#include <linux/err.h>
@@ -50,22 +51,29 @@ int phy_update_status(struct phy_device *phydev)
{
struct phy_driver *drv = to_phy_driver(phydev->dev.driver);
struct eth_device *edev = phydev->attached_dev;
+ struct device_d *dev;
int ret;
int oldspeed = phydev->speed, oldduplex = phydev->duplex;
+ int oldlink = phydev->link;
ret = drv->read_status(phydev);
if (ret)
return ret;
- if (phydev->speed == oldspeed && phydev->duplex == oldduplex)
+ if (phydev->speed == oldspeed && phydev->duplex == oldduplex &&
+ phydev->link == oldlink)
return 0;
- if (phydev->adjust_link)
+ if (phydev->adjust_link && edev)
phydev->adjust_link(edev);
+ dev = edev ? &edev->dev : &phydev->dev;
+
if (phydev->link)
- dev_info(&edev->dev, "%dMbps %s duplex link detected\n",
- phydev->speed, phydev->duplex ? "full" : "half");
+ dev_info(dev, "%dMbps %s duplex link detected\n", phydev->speed,
+ phydev->duplex ? "full" : "half");
+ else
+ dev_info(dev, "link down\n");
return 0;
}
@@ -492,14 +500,13 @@ int phy_wait_aneg_done(struct phy_device *phydev)
return 0;
while (!is_timeout(start, PHY_AN_TIMEOUT * SECOND)) {
- if (phy_aneg_done(phydev) > 0) {
- phydev->link = 1;
- return 0;
- }
+ if (phy_aneg_done(phydev) > 0)
+ break;
}
- phydev->link = 0;
- return -ETIMEDOUT;
+ phy_update_status(phydev);
+
+ return phydev->link ? 0 : -ETIMEDOUT;
}
/**
@@ -650,8 +657,10 @@ int genphy_read_status(struct phy_device *phydev)
int lpagb = 0;
/* if force the status and link are set */
- if (phydev->force)
+ if (phydev->force) {
+ phydev->link = 1;
return 0;
+ }
/* Update the link, but return if there
* was an error */
@@ -897,6 +906,32 @@ static struct phy_driver genphy_driver = {
SUPPORTED_BNC,
};
+static void phy_poll(struct poller_struct *poller)
+{
+ static uint64_t to;
+ struct device_d *dev;
+ struct phy_device *phy;
+
+ if (!to || is_timeout(to, 2 * SECOND)) {
+ bus_for_each_device(&mdio_bus_type, dev) {
+ phy = container_of(dev, struct phy_device, dev);
+ phy_update_status(phy);
+ }
+
+ to = get_time_ns();
+ }
+}
+
+static struct poller_struct phy_poller = {
+ .func = phy_poll,
+};
+
+static int phy_poller_register(void)
+{
+ return poller_register(&phy_poller);
+}
+late_initcall(phy_poller_register);
+
static int generic_phy_register(void)
{
return phy_driver_register(&genphy_driver);
diff --git a/net/eth.c b/net/eth.c
index 89bddba..79a0c92 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -29,7 +29,6 @@
#include <malloc.h>
static struct eth_device *eth_current;
-static uint64_t last_link_check;
static LIST_HEAD(netdev_list);
@@ -173,26 +172,18 @@ int eth_complete(struct string_list *sl, char *instr)
/*
* Check for link if we haven't done so for longer.
*/
-static int eth_carrier_check(struct eth_device *edev, int force)
+static int eth_carrier_check(struct eth_device *edev)
{
- int ret;
-
if (!IS_ENABLED(CONFIG_PHYLIB))
return 0;
if (!edev->phydev)
return 0;
- if (force)
- phy_wait_aneg_done(edev->phydev);
+ if (edev->phydev->link)
+ return 0;
- if (force || is_timeout(last_link_check, 5 * SECOND) ||
- !edev->phydev->link) {
- ret = phy_update_status(edev->phydev);
- if (ret)
- return ret;
- last_link_check = get_time_ns();
- }
+ phy_wait_aneg_done(edev->phydev);
return edev->phydev->link ? 0 : -ENETDOWN;
}
@@ -214,7 +205,7 @@ static int eth_check_open(struct eth_device *edev)
edev->active = 1;
- return eth_carrier_check(edev, 1);
+ return eth_carrier_check(edev);
}
int eth_send(struct eth_device *edev, void *packet, int length)
@@ -225,7 +216,7 @@ int eth_send(struct eth_device *edev, void *packet, int length)
if (ret)
return ret;
- ret = eth_carrier_check(edev, 0);
+ ret = eth_carrier_check(edev);
if (ret)
return ret;
@@ -242,7 +233,7 @@ static int __eth_rx(struct eth_device *edev)
if (ret)
return ret;
- ret = eth_carrier_check(edev, 0);
+ ret = eth_carrier_check(edev);
if (ret)
return ret;
--
2.1.0
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 4/4] net: phy: Use poller for periodic link check
2014-09-18 7:11 ` [PATCH 4/4] net: phy: Use poller for periodic link check Sascha Hauer
@ 2014-09-24 5:09 ` Sascha Hauer
0 siblings, 0 replies; 6+ messages in thread
From: Sascha Hauer @ 2014-09-24 5:09 UTC (permalink / raw)
To: barebox
On Thu, Sep 18, 2014 at 09:11:15AM +0200, Sascha Hauer wrote:
> This continuously updates the link status in the background. The networking
> code no longer has to periodically update the link status itself but instead
> can only check for phydev->link.
> With this we also always have link status changes printed to the console.
Nice try, but this doesn't work, so I'll drop this one.
The problem comes with USB network adapters (SPI would be the same).
pollers get called in is_timeout() loops like USB host drivers use
to poll for urb completion. So when network transfers happen and
the network driver issues USB requests it can happen that in the
USB host driver a poller triggers which with the periodic link check
now triggers a USB request itself. barebox can't (and shouldn't) handle
this reentrancy.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 6+ messages in thread