• Vladimir Oltean's avatar
    net: phy: continue searching for C45 MMDs even if first returned ffff:ffff · bba238ed
    Vladimir Oltean authored
    At the time of introduction, in commit bdeced75 ("net: dsa: felix:
    Add PCS operations for PHYLINK"), support for the Lynx PCS inside Felix
    was relying, for USXGMII support, on the fact that get_phy_device() is
    able to parse the Lynx PCS "device-in-package" registers for this C45
    MDIO device and identify it correctly.
    
    However, this was actually working somewhat by mistake (in the sense
    that, even though it was detected, it was detected for the wrong
    reasons).
    
    The get_phy_c45_ids() function works by iterating through all MMDs
    starting from 1 (MDIO_MMD_PMAPMD) and stops at the first one which
    returns a non-zero value in the "device-in-package" register pair,
    proceeding to see what that non-zero value is.
    
    For the Felix PCS, the first MMD (1, for the PMA/PMD) returns a non-zero
    value of 0xffffffff in the "device-in-package" registers. There is a
    code branch which is supposed to treat this case and flag it as wrong,
    and normally, this would have caught my attention when adding initial
    support for this PCS:
    
    	if ((devs_in_pkg & 0x1fffffff) == 0x1fffffff) {
    		/* If mostly Fs, there is no device there, then let's probe
    		 * MMD 0, as some 10G PHYs have zero Devices In package,
    		 * e.g. Cortina CS4315/CS4340 PHY.
    		 */
    
    However, this code never actually kicked in, it seems, because this
    snippet from get_phy_c45_devs_in_pkg() was basically sabotaging itself,
    by returning 0xfffffffe instead of 0xffffffff:
    
    	/* Bit 0 doesn't represent a device, it indicates c22 regs presence */
    	*devices_in_package &= ~BIT(0);
    
    Then the rest of the code just carried on thinking "ok, MMD 1 (PMA/PMD)
    says that there are 31 devices in that package, each having a device id
    of ffff:ffff, that's perfectly fine, let's go ahead and probe this PHY
    device".
    
    But after cleanup commit 320ed3bf ("net: phy: split
    devices_in_package"), this got "fixed", and now devs_in_pkg is no longer
    0xfffffffe, but 0xffffffff. So now, get_phy_device is returning -ENODEV
    for the Lynx PCS, because the semantics have remained mostly unchanged:
    the loop stops at the first MMD that returns a non-zero value, and that
    is MMD 1.
    
    But the Lynx PCS is simply a clause 37 PCS which implements the required
    MAC-side functionality for USXGMII (when operated in C45 mode, which is
    where C45 devices-in-package detection is relevant to). Of course it
    will fail the PMD/PMA test (MMD 1), since it is not a PHY. But it does
    implement detection for MDIO_MMD_PCS (3):
    
    - MDIO_DEVS1=0x008a, MDIO_DEVS2=0x0000,
    - MDIO_DEVID1=0x0083, MDIO_DEVID2=0xe400
    
    Let get_phy_c45_ids() continue searching for valid MMDs, and don't
    assume that every phy_device has a PMA/PMD MMD implemented.
    Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
    Reviewed-by: default avatarAndrew Lunn <andrew@lunn.ch>
    Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
    bba238ed
phy_device.c 80.2 KB