[+to Petr, -cc Jesse, bouncing]
On Mon, Aug 19, 2024 at 07:23:42AM +0200, Jiri Slaby wrote:
> On 19. 08. 24, 6:50, Jiri Slaby wrote:
> > CC e1000e guys + Jesse (due to 75a3f93b5383) + Bjorn (due to b2c289415b2b)
>
> Bjorn,
>
> I am confused by these changes:
> ==========================================
> @@ -291,16 +288,13 @@ static int e1000_set_link_ksettings(struct net_device
> *net
> dev,
> * duplex is forced.
> */
> if (cmd->base.eth_tp_mdix_ctrl) {
> - if (hw->phy.media_type != e1000_media_type_copper) {
> - ret_val = -EOPNOTSUPP;
> - goto out;
> - }
> + if (hw->phy.media_type != e1000_media_type_copper)
> + return -EOPNOTSUPP;
>
> if ((cmd->base.eth_tp_mdix_ctrl != ETH_TP_MDI_AUTO) &&
> (cmd->base.autoneg != AUTONEG_ENABLE)) {
> e_err("forcing MDI/MDI-X state is not supported when
> lin
> k speed and/or duplex are forced\n");
> - ret_val = -EINVAL;
> - goto out;
> + return -EINVAL;
> }
> }
>
> @@ -347,7 +341,6 @@ static int e1000_set_link_ksettings(struct net_device
> *netde
> v,
> }
>
> out:
> - pm_runtime_put_sync(netdev->dev.parent);
> clear_bit(__E1000_RESETTING, &adapter->state);
> return ret_val;
> }
> ==========================================
>
> So no more clear_bit(__E1000_RESETTING in the above fail paths. Is that
> intentional?
Not intentional. Petr, do you have the ability to test the patch
below? I'm not sure it's the correct fix, but it reverts the pieces
of b2c289415b2b that Jiri pointed out.
diff --git a/drivers/net/ethernet/intel/e1000e/ethtool.c b/drivers/net/ethernet/intel/e1000e/ethtool.c
index 9364bc2b4eb1..9db36ee71684 100644
--- a/drivers/net/ethernet/intel/e1000e/ethtool.c
+++ b/drivers/net/ethernet/intel/e1000e/ethtool.c
@@ -280,7 +280,8 @@ static int e1000_set_link_ksettings(struct net_device *netdev,
if (hw->phy.ops.check_reset_block &&
hw->phy.ops.check_reset_block(hw)) {
e_err("Cannot change link characteristics when SoL/IDER is active.\n");
- return -EINVAL;
+ ret_val = -EINVAL;
+ goto out;
}
/* MDI setting is only allowed when autoneg enabled because
@@ -288,13 +289,16 @@ static int e1000_set_link_ksettings(struct net_device *netdev,
* duplex is forced.
*/
if (cmd->base.eth_tp_mdix_ctrl) {
- if (hw->phy.media_type != e1000_media_type_copper)
- return -EOPNOTSUPP;
+ if (hw->phy.media_type != e1000_media_type_copper) {
+ ret_val = -EOPNOTSUPP;
+ goto out;
+ }
if ((cmd->base.eth_tp_mdix_ctrl != ETH_TP_MDI_AUTO) &&
(cmd->base.autoneg != AUTONEG_ENABLE)) {
e_err("forcing MDI/MDI-X state is not supported when link speed and/or duplex are forced\n");
- return -EINVAL;
+ ret_val = -EINVAL;
+ goto out;
}
}
