On Mon, Jun 19, 2017 at 4:37 AM, Marek Vasut marex@denx.de wrote:
On 06/13/2017 06:28 PM, Joe Hershberger wrote:
On Tue, Jun 13, 2017 at 4:24 AM, Marek Vasut marex@denx.de wrote:
On 06/12/2017 10:20 PM, Joe Hershberger wrote:
Don't wait forever, Pass errors back, etc.
Signed-off-by: Joe Hershberger joe.hershberger@ni.com
This is a pass at improving the code quality. This has not been tested in any way.
drivers/net/ag7xxx.c | 63 +++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 50 insertions(+), 13 deletions(-)
diff --git a/drivers/net/ag7xxx.c b/drivers/net/ag7xxx.c index cf60d11..c8352d1 100644 --- a/drivers/net/ag7xxx.c +++ b/drivers/net/ag7xxx.c
[...] SNIP
@@ -723,10 +764,13 @@ static int ag933x_phy_setup_common(struct udevice *dev) return ret;
/* Read out link status */
ret = ag7xxx_mdio_read(priv->bus, phymax, 0, MII_MIPSCR);
ret = ag7xxx_mdio_read(priv->bus, phymax, 0, AG7XXX_PHY_PSSR); if (ret < 0) return ret;if (!(ret & AG7XXX_PHY_PSSR_LINK_UP))return -ENOLINK;Are you sure about this ?
It seems reasonable to me, but I don't have the HW to test against as noted above.
CCing Wills . I wouldn't be surprised if the hardware was somehow magically screwed up, so I'd prefer to stick with equivalent changes and maybe changes of the logic in a separate patch.
OK.
return 0; }@@ -743,13 +787,6 @@ static int ag933x_phy_setup_common(struct udevice *dev) return ret; }
for (i = 0; i < phymax; i++) {/* Read out link status */ret = ag7xxx_mdio_read(priv->bus, i, 0, MII_MIPSCR);if (ret < 0)return ret;}And this ?
This was based on your comment: "Actually, I think this is only for the switch ports, so we don't care about the link status."
Just so I understand it correctly, the code reads link status and does nothing with it ?
That's how I read it.
-- Best regards, Marek Vasut