Message ID | 1315291645.28564.20.camel@lb-tlvb-dmitry |
---|---|
State | Superseded, archived |
Delegated to: | David Miller |
Headers | show |
On Tue, 2011-09-06 at 09:47 +0300, Yaniv Rosner wrote: > Enable changing advertisement settings via ethtool and fix > flow-control advertisement when autoneg flow-control is disabled. [...] > diff --git a/drivers/net/bnx2x/bnx2x_main.c b/drivers/net/bnx2x/bnx2x_main.c > index f74582a..42c7be1 100644 > --- a/drivers/net/bnx2x/bnx2x_main.c > +++ b/drivers/net/bnx2x/bnx2x_main.c > @@ -2125,6 +2125,12 @@ static int bnx2x_set_spio(struct bnx2x *bp, int spio_num, u32 mode) > void bnx2x_calc_fc_adv(struct bnx2x *bp) > { > u8 cfg_idx = bnx2x_get_link_cfg_idx(bp); > + if (bp->link_params.req_flow_ctrl[cfg_idx] != BNX2X_FLOW_CTRL_AUTO) { > + bp->port.advertising[cfg_idx] &= ~(ADVERTISED_Asym_Pause | > + ADVERTISED_Pause); > + return; > + } [...] I think you should still advertise the flow control behaviour you want, even if you will override the result of autonegotiation. Ben.
On Tue, 2011-09-06 at 08:19 -0700, Ben Hutchings wrote: > On Tue, 2011-09-06 at 09:47 +0300, Yaniv Rosner wrote: > > Enable changing advertisement settings via ethtool and fix > > flow-control advertisement when autoneg flow-control is disabled. > [...] > > diff --git a/drivers/net/bnx2x/bnx2x_main.c b/drivers/net/bnx2x/bnx2x_main.c > > index f74582a..42c7be1 100644 > > --- a/drivers/net/bnx2x/bnx2x_main.c > > +++ b/drivers/net/bnx2x/bnx2x_main.c > > @@ -2125,6 +2125,12 @@ static int bnx2x_set_spio(struct bnx2x *bp, int spio_num, u32 mode) > > void bnx2x_calc_fc_adv(struct bnx2x *bp) > > { > > u8 cfg_idx = bnx2x_get_link_cfg_idx(bp); > > + if (bp->link_params.req_flow_ctrl[cfg_idx] != BNX2X_FLOW_CTRL_AUTO) { > > + bp->port.advertising[cfg_idx] &= ~(ADVERTISED_Asym_Pause | > > + ADVERTISED_Pause); > > + return; > > + } > [...] > > I think you should still advertise the flow control behaviour you want, > even if you will override the result of autonegotiation. > > Ben. > Ben, It's a gray area, so I don't have a strong opinion this way or another. I trust your senses, and will follow your suggestion to fix it. Thanks, Yaniv -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/bnx2x/bnx2x_ethtool.c b/drivers/net/bnx2x/bnx2x_ethtool.c index 2218630..6aa94d3 100644 --- a/drivers/net/bnx2x/bnx2x_ethtool.c +++ b/drivers/net/bnx2x/bnx2x_ethtool.c @@ -363,13 +363,50 @@ static int bnx2x_set_settings(struct net_device *dev, struct ethtool_cmd *cmd) } /* advertise the requested speed and duplex if supported */ - cmd->advertising &= bp->port.supported[cfg_idx]; + if (cmd->advertising & ~(bp->port.supported[cfg_idx])) { + DP(NETIF_MSG_LINK, "Advertisement parameters " + "are not supported\n"); + return -EINVAL; + } bp->link_params.req_line_speed[cfg_idx] = SPEED_AUTO_NEG; - bp->link_params.req_duplex[cfg_idx] = DUPLEX_FULL; - bp->port.advertising[cfg_idx] |= (ADVERTISED_Autoneg | + bp->link_params.req_duplex[cfg_idx] = cmd->duplex; + bp->port.advertising[cfg_idx] = (ADVERTISED_Autoneg | cmd->advertising); + if (cmd->advertising) { + + bp->link_params.speed_cap_mask[cfg_idx] = 0; + if (cmd->advertising & ADVERTISED_10baseT_Half) { + bp->link_params.speed_cap_mask[cfg_idx] |= + PORT_HW_CFG_SPEED_CAPABILITY_D0_10M_HALF; + } + if (cmd->advertising & ADVERTISED_10baseT_Full) + bp->link_params.speed_cap_mask[cfg_idx] |= + PORT_HW_CFG_SPEED_CAPABILITY_D0_10M_FULL; + if (cmd->advertising & ADVERTISED_100baseT_Full) + bp->link_params.speed_cap_mask[cfg_idx] |= + PORT_HW_CFG_SPEED_CAPABILITY_D0_100M_FULL; + + if (cmd->advertising & ADVERTISED_100baseT_Half) { + bp->link_params.speed_cap_mask[cfg_idx] |= + PORT_HW_CFG_SPEED_CAPABILITY_D0_100M_HALF; + } + if (cmd->advertising & ADVERTISED_1000baseT_Half) { + bp->link_params.speed_cap_mask[cfg_idx] |= + PORT_HW_CFG_SPEED_CAPABILITY_D0_1G; + } + if (cmd->advertising & (ADVERTISED_1000baseT_Full | + ADVERTISED_1000baseKX_Full)) + bp->link_params.speed_cap_mask[cfg_idx] |= + PORT_HW_CFG_SPEED_CAPABILITY_D0_1G; + + if (cmd->advertising & (ADVERTISED_10000baseT_Full | + ADVERTISED_10000baseKX4_Full | + ADVERTISED_10000baseKR_Full)) + bp->link_params.speed_cap_mask[cfg_idx] |= + PORT_HW_CFG_SPEED_CAPABILITY_D0_10G; + } } else { /* forced speed */ /* advertise the requested speed and duplex if supported */ switch (speed) { diff --git a/drivers/net/bnx2x/bnx2x_main.c b/drivers/net/bnx2x/bnx2x_main.c index f74582a..42c7be1 100644 --- a/drivers/net/bnx2x/bnx2x_main.c +++ b/drivers/net/bnx2x/bnx2x_main.c @@ -2125,6 +2125,12 @@ static int bnx2x_set_spio(struct bnx2x *bp, int spio_num, u32 mode) void bnx2x_calc_fc_adv(struct bnx2x *bp) { u8 cfg_idx = bnx2x_get_link_cfg_idx(bp); + if (bp->link_params.req_flow_ctrl[cfg_idx] != BNX2X_FLOW_CTRL_AUTO) { + bp->port.advertising[cfg_idx] &= ~(ADVERTISED_Asym_Pause | + ADVERTISED_Pause); + return; + } + switch (bp->link_vars.ieee_fc & MDIO_COMBO_IEEE0_AUTO_NEG_ADV_PAUSE_MASK) { case MDIO_COMBO_IEEE0_AUTO_NEG_ADV_PAUSE_NONE: