Skip to content

Commit

Permalink
Merge branch 'ftgmac100-fixes'
Browse files Browse the repository at this point in the history
Heyi Guo says:

====================
drivers/net/ftgmac100: fix occasional DHCP failure

This patch set is to fix the issues discussed in the mail thread:
https://lore.kernel.org/netdev/51f5b7a7-330f-6b3c-253d-10e45cdb6805@linux.alibaba.com/
and follows the advice from Andrew Lunn.

The first 2 patches refactors the code to enable adjust_link calling reset
function directly.
====================

Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
David S. Miller committed Feb 23, 2022
2 parents ecf4a24 + 1baf2e5 commit 0228d37
Showing 1 changed file with 129 additions and 114 deletions.
243 changes: 129 additions & 114 deletions drivers/net/ethernet/faraday/ftgmac100.c
Original file line number Diff line number Diff line change
Expand Up @@ -989,117 +989,6 @@ static int ftgmac100_alloc_rx_buffers(struct ftgmac100 *priv)
return 0;
}

static void ftgmac100_adjust_link(struct net_device *netdev)
{
struct ftgmac100 *priv = netdev_priv(netdev);
struct phy_device *phydev = netdev->phydev;
bool tx_pause, rx_pause;
int new_speed;

/* We store "no link" as speed 0 */
if (!phydev->link)
new_speed = 0;
else
new_speed = phydev->speed;

/* Grab pause settings from PHY if configured to do so */
if (priv->aneg_pause) {
rx_pause = tx_pause = phydev->pause;
if (phydev->asym_pause)
tx_pause = !rx_pause;
} else {
rx_pause = priv->rx_pause;
tx_pause = priv->tx_pause;
}

/* Link hasn't changed, do nothing */
if (phydev->speed == priv->cur_speed &&
phydev->duplex == priv->cur_duplex &&
rx_pause == priv->rx_pause &&
tx_pause == priv->tx_pause)
return;

/* Print status if we have a link or we had one and just lost it,
* don't print otherwise.
*/
if (new_speed || priv->cur_speed)
phy_print_status(phydev);

priv->cur_speed = new_speed;
priv->cur_duplex = phydev->duplex;
priv->rx_pause = rx_pause;
priv->tx_pause = tx_pause;

/* Link is down, do nothing else */
if (!new_speed)
return;

/* Disable all interrupts */
iowrite32(0, priv->base + FTGMAC100_OFFSET_IER);

/* Reset the adapter asynchronously */
schedule_work(&priv->reset_task);
}

static int ftgmac100_mii_probe(struct net_device *netdev)
{
struct ftgmac100 *priv = netdev_priv(netdev);
struct platform_device *pdev = to_platform_device(priv->dev);
struct device_node *np = pdev->dev.of_node;
struct phy_device *phydev;
phy_interface_t phy_intf;
int err;

/* Default to RGMII. It's a gigabit part after all */
err = of_get_phy_mode(np, &phy_intf);
if (err)
phy_intf = PHY_INTERFACE_MODE_RGMII;

/* Aspeed only supports these. I don't know about other IP
* block vendors so I'm going to just let them through for
* now. Note that this is only a warning if for some obscure
* reason the DT really means to lie about it or it's a newer
* part we don't know about.
*
* On the Aspeed SoC there are additionally straps and SCU
* control bits that could tell us what the interface is
* (or allow us to configure it while the IP block is held
* in reset). For now I chose to keep this driver away from
* those SoC specific bits and assume the device-tree is
* right and the SCU has been configured properly by pinmux
* or the firmware.
*/
if (priv->is_aspeed && !(phy_interface_mode_is_rgmii(phy_intf))) {
netdev_warn(netdev,
"Unsupported PHY mode %s !\n",
phy_modes(phy_intf));
}

phydev = phy_find_first(priv->mii_bus);
if (!phydev) {
netdev_info(netdev, "%s: no PHY found\n", netdev->name);
return -ENODEV;
}

phydev = phy_connect(netdev, phydev_name(phydev),
&ftgmac100_adjust_link, phy_intf);

if (IS_ERR(phydev)) {
netdev_err(netdev, "%s: Could not attach to PHY\n", netdev->name);
return PTR_ERR(phydev);
}

/* Indicate that we support PAUSE frames (see comment in
* Documentation/networking/phy.rst)
*/
phy_support_asym_pause(phydev);

/* Display what we found */
phy_attached_info(phydev);

return 0;
}

static int ftgmac100_mdiobus_read(struct mii_bus *bus, int phy_addr, int regnum)
{
struct net_device *netdev = bus->priv;
Expand Down Expand Up @@ -1410,10 +1299,8 @@ static int ftgmac100_init_all(struct ftgmac100 *priv, bool ignore_alloc_err)
return err;
}

static void ftgmac100_reset_task(struct work_struct *work)
static void ftgmac100_reset(struct ftgmac100 *priv)
{
struct ftgmac100 *priv = container_of(work, struct ftgmac100,
reset_task);
struct net_device *netdev = priv->netdev;
int err;

Expand Down Expand Up @@ -1459,6 +1346,134 @@ static void ftgmac100_reset_task(struct work_struct *work)
rtnl_unlock();
}

static void ftgmac100_reset_task(struct work_struct *work)
{
struct ftgmac100 *priv = container_of(work, struct ftgmac100,
reset_task);

ftgmac100_reset(priv);
}

static void ftgmac100_adjust_link(struct net_device *netdev)
{
struct ftgmac100 *priv = netdev_priv(netdev);
struct phy_device *phydev = netdev->phydev;
bool tx_pause, rx_pause;
int new_speed;

/* We store "no link" as speed 0 */
if (!phydev->link)
new_speed = 0;
else
new_speed = phydev->speed;

/* Grab pause settings from PHY if configured to do so */
if (priv->aneg_pause) {
rx_pause = tx_pause = phydev->pause;
if (phydev->asym_pause)
tx_pause = !rx_pause;
} else {
rx_pause = priv->rx_pause;
tx_pause = priv->tx_pause;
}

/* Link hasn't changed, do nothing */
if (phydev->speed == priv->cur_speed &&
phydev->duplex == priv->cur_duplex &&
rx_pause == priv->rx_pause &&
tx_pause == priv->tx_pause)
return;

/* Print status if we have a link or we had one and just lost it,
* don't print otherwise.
*/
if (new_speed || priv->cur_speed)
phy_print_status(phydev);

priv->cur_speed = new_speed;
priv->cur_duplex = phydev->duplex;
priv->rx_pause = rx_pause;
priv->tx_pause = tx_pause;

/* Link is down, do nothing else */
if (!new_speed)
return;

/* Disable all interrupts */
iowrite32(0, priv->base + FTGMAC100_OFFSET_IER);

/* Release phy lock to allow ftgmac100_reset to aquire it, keeping lock
* order consistent to prevent dead lock.
*/
if (netdev->phydev)
mutex_unlock(&netdev->phydev->lock);

ftgmac100_reset(priv);

if (netdev->phydev)
mutex_lock(&netdev->phydev->lock);

}

static int ftgmac100_mii_probe(struct net_device *netdev)
{
struct ftgmac100 *priv = netdev_priv(netdev);
struct platform_device *pdev = to_platform_device(priv->dev);
struct device_node *np = pdev->dev.of_node;
struct phy_device *phydev;
phy_interface_t phy_intf;
int err;

/* Default to RGMII. It's a gigabit part after all */
err = of_get_phy_mode(np, &phy_intf);
if (err)
phy_intf = PHY_INTERFACE_MODE_RGMII;

/* Aspeed only supports these. I don't know about other IP
* block vendors so I'm going to just let them through for
* now. Note that this is only a warning if for some obscure
* reason the DT really means to lie about it or it's a newer
* part we don't know about.
*
* On the Aspeed SoC there are additionally straps and SCU
* control bits that could tell us what the interface is
* (or allow us to configure it while the IP block is held
* in reset). For now I chose to keep this driver away from
* those SoC specific bits and assume the device-tree is
* right and the SCU has been configured properly by pinmux
* or the firmware.
*/
if (priv->is_aspeed && !(phy_interface_mode_is_rgmii(phy_intf))) {
netdev_warn(netdev,
"Unsupported PHY mode %s !\n",
phy_modes(phy_intf));
}

phydev = phy_find_first(priv->mii_bus);
if (!phydev) {
netdev_info(netdev, "%s: no PHY found\n", netdev->name);
return -ENODEV;
}

phydev = phy_connect(netdev, phydev_name(phydev),
&ftgmac100_adjust_link, phy_intf);

if (IS_ERR(phydev)) {
netdev_err(netdev, "%s: Could not attach to PHY\n", netdev->name);
return PTR_ERR(phydev);
}

/* Indicate that we support PAUSE frames (see comment in
* Documentation/networking/phy.rst)
*/
phy_support_asym_pause(phydev);

/* Display what we found */
phy_attached_info(phydev);

return 0;
}

static int ftgmac100_open(struct net_device *netdev)
{
struct ftgmac100 *priv = netdev_priv(netdev);
Expand Down

0 comments on commit 0228d37

Please sign in to comment.