| Message ID | 1498506010-12920-3-git-send-email-joe.hershberger@ni.com |
|---|---|
| State | RFC |
| Delegated to: | Joe Hershberger |
| Headers | show |
On 06/26/2017 09:40 PM, Joe Hershberger wrote: > In the case of the WAN port, pay attention to the link status. > In the case of LAN ports, stop reading the link status since we don't > care. > > Signed-off-by: Joe Hershberger <joe.hershberger@ni.com> Well, let's see how that works, ev. I'll have to bisect. btw I hope all this is post-2017.07 material. > --- > This is a pass at improving the code quality. > This has not been tested in any way. > > Changes in v2: > - New - Split link status change into its own patch (so it can be dropped if it affects behavior) > > drivers/net/ag7xxx.c | 18 ++++++++++-------- > 1 file changed, 10 insertions(+), 8 deletions(-) > > diff --git a/drivers/net/ag7xxx.c b/drivers/net/ag7xxx.c > index 00e6806..c8352d1 100644 > --- a/drivers/net/ag7xxx.c > +++ b/drivers/net/ag7xxx.c > @@ -100,6 +100,12 @@ enum ag7xxx_model { > /* Rx Status */ > #define AG7XXX_ETH_DMA_RX_STATUS 0x194 > > +/* PHY Control Registers */ > + > +/* PHY Specific Status Register */ > +#define AG7XXX_PHY_PSSR 0x11 > +#define AG7XXX_PHY_PSSR_LINK_UP BIT(10) > + > /* Custom register at 0x18070000 */ > #define AG7XXX_GMAC_ETH_CFG 0x00 > #define AG7XXX_ETH_CFG_SW_PHY_ADDR_SWAP BIT(8) > @@ -758,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; > + > return 0; > } > > @@ -778,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; > - } > - > return 0; > } > >
On Tue, Jun 27, 2017 at 4:32 AM, Marek Vasut <marex@denx.de> wrote: > On 06/26/2017 09:40 PM, Joe Hershberger wrote: >> In the case of the WAN port, pay attention to the link status. >> In the case of LAN ports, stop reading the link status since we don't >> care. >> >> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com> > > Well, let's see how that works, ev. I'll have to bisect. > btw I hope all this is post-2017.07 material. I'm not intending to apply any of these until you sign off it's functionality. -Joe
On 06/27/2017 05:46 PM, Joe Hershberger wrote: > On Tue, Jun 27, 2017 at 4:32 AM, Marek Vasut <marex@denx.de> wrote: >> On 06/26/2017 09:40 PM, Joe Hershberger wrote: >>> In the case of the WAN port, pay attention to the link status. >>> In the case of LAN ports, stop reading the link status since we don't >>> care. >>> >>> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com> >> >> Well, let's see how that works, ev. I'll have to bisect. >> btw I hope all this is post-2017.07 material. > > I'm not intending to apply any of these until you sign off it's functionality. Hm, maybe Wills can help with that ... CCed
On Tue, Jun 27, 2017 at 11:10 AM, Marek Vasut <marex@denx.de> wrote: > On 06/27/2017 05:46 PM, Joe Hershberger wrote: >> On Tue, Jun 27, 2017 at 4:32 AM, Marek Vasut <marex@denx.de> wrote: >>> On 06/26/2017 09:40 PM, Joe Hershberger wrote: >>>> In the case of the WAN port, pay attention to the link status. >>>> In the case of LAN ports, stop reading the link status since we don't >>>> care. >>>> >>>> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com> >>> >>> Well, let's see how that works, ev. I'll have to bisect. >>> btw I hope all this is post-2017.07 material. >> >> I'm not intending to apply any of these until you sign off it's functionality. > > Hm, maybe Wills can help with that ... CCed So... still functional? Thanks, -Joe
diff --git a/drivers/net/ag7xxx.c b/drivers/net/ag7xxx.c index 00e6806..c8352d1 100644 --- a/drivers/net/ag7xxx.c +++ b/drivers/net/ag7xxx.c @@ -100,6 +100,12 @@ enum ag7xxx_model { /* Rx Status */ #define AG7XXX_ETH_DMA_RX_STATUS 0x194 +/* PHY Control Registers */ + +/* PHY Specific Status Register */ +#define AG7XXX_PHY_PSSR 0x11 +#define AG7XXX_PHY_PSSR_LINK_UP BIT(10) + /* Custom register at 0x18070000 */ #define AG7XXX_GMAC_ETH_CFG 0x00 #define AG7XXX_ETH_CFG_SW_PHY_ADDR_SWAP BIT(8) @@ -758,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; + return 0; } @@ -778,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; - } - return 0; }
In the case of the WAN port, pay attention to the link status. In the case of LAN ports, stop reading the link status since we don't care. 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. Changes in v2: - New - Split link status change into its own patch (so it can be dropped if it affects behavior) drivers/net/ag7xxx.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-)