Hi Emil,

I was looking through a set of ixgbe patches and came across this commit

    commit 410a494902777c11f95031d9ed757d7f8f09c5c6
    ixgbe: add write flush when configuring CS4223/7

and am wondering about the setting of reg_phy_ext in the middle of ixgbe_setup_mac_link_sfp_x550a(). It looks like it is read from the PHY, modified to remove the CX1 and SR mode bits, but then those bits are overwritten in the "if (setup_linear)" block immediately following, and that is what gets written back out.

        ret_val = hw->phy.ops.read_reg(hw, reg_slice,
                                       IXGBE_MDIO_ZERO_DEV_TYPE, &reg_phy_ext);
        if (ret_val)
                return ret_val;

        reg_phy_ext &= ~((IXGBE_CS4227_EDC_MODE_CX1 << 1) |
                         (IXGBE_CS4227_EDC_MODE_SR << 1));

        if (setup_linear)
                reg_phy_ext = (IXGBE_CS4227_EDC_MODE_CX1 << 1) | 1;
        else
                reg_phy_ext = (IXGBE_CS4227_EDC_MODE_SR << 1) | 1;

        ret_val = hw->phy.ops.write_reg(hw, reg_slice,
                                        IXGBE_MDIO_ZERO_DEV_TYPE, reg_phy_ext);
        if (ret_val)
                return ret_val;


The assignments to reg_phy_ext look wrong to me - perhaps those should be '|=' rather than '='?

sln

--
==================================================
Shannon Nelson           shannon.nel...@oracle.com
Parents can't afford to be squeamish

Reply via email to