On Wed, Jul 22, 2020 at 02:52:09PM +0300, Vladimir Oltean wrote: > On Mon, Jul 20, 2020 at 08:26:54PM +0300, Vladimir Oltean wrote: > > From: Vladimir Oltean <[email protected]> > > > > After the patch below, the iteration through the available MMDs is > > completely short-circuited, and devs_in_pkg remains set to the initial > > value of zero. > > > > Due to devs_in_pkg being zero, the rest of get_phy_c45_ids() is > > short-circuited too: the following loop never reaches below this point > > either (it executes "continue" for every device in package, failing to > > retrieve PHY ID for any of them): > > > > /* Now probe Device Identifiers for each device present. */ > > for (i = 1; i < num_ids; i++) { > > if (!(devs_in_pkg & (1 << i))) > > continue; > > > > So c45_ids->device_ids remains populated with zeroes. This causes an > > Aquantia AQR412 PHY (same as any C45 PHY would, in fact) to be probed by > > the Generic PHY driver. > > > > The issue seems to be a case of submitting partially committed work (and > > therefore testing something other than was submitted). > > > > The intention of the patch was to delay exiting the loop until one more > > condition is reached (the devs_in_pkg read from hardware is either 0, OR > > mostly f's). So fix the patch to reflect that. > > > > Tested with traffic on a LS1028A-QDS, the PHY is now probed correctly > > using the Aquantia driver. The devs_in_pkg bit field is set to > > 0xe000009a, and the MMDs that are present have the following IDs: > > > > [ 5.600772] libphy: get_phy_c45_ids: device_ids[1]=0x3a1b662 > > [ 5.618781] libphy: get_phy_c45_ids: device_ids[3]=0x3a1b662 > > [ 5.630797] libphy: get_phy_c45_ids: device_ids[4]=0x3a1b662 > > [ 5.654535] libphy: get_phy_c45_ids: device_ids[7]=0x3a1b662 > > [ 5.791723] libphy: get_phy_c45_ids: device_ids[29]=0x3a1b662 > > [ 5.804050] libphy: get_phy_c45_ids: device_ids[30]=0x3a1b662 > > [ 5.816375] libphy: get_phy_c45_ids: device_ids[31]=0x0 > > > > [ 7.690237] mscc_felix 0000:00:00.5: PHY [0.5:00] driver [Aquantia > > AQR412] (irq=POLL) > > [ 7.704739] mscc_felix 0000:00:00.5: PHY [0.5:01] driver [Aquantia > > AQR412] (irq=POLL) > > [ 7.718918] mscc_felix 0000:00:00.5: PHY [0.5:02] driver [Aquantia > > AQR412] (irq=POLL) > > [ 7.733044] mscc_felix 0000:00:00.5: PHY [0.5:03] driver [Aquantia > > AQR412] (irq=POLL) > > > > Fixes: bba238ed037c ("net: phy: continue searching for C45 MMDs even if > > first returned ffff:ffff") > > Reported-by: Colin King <[email protected]> > > Reported-by: Ioana Ciornei <[email protected]> > > Signed-off-by: Vladimir Oltean <[email protected]> > > --- > > This patch is repairing some pretty significant breakage. Could we > please get some review before there start appearing user reports? > > [ sorry for the breakage ]
I'm surprised it has not been merged, since the fix seems quite obvious. Reviewed-by: Andrew Lunn <[email protected]> Andrew
