Messages in this thread | | | From | Sean Anderson <> | Subject | Re: [RFC net-next PATCH 16/16] net: sfp: Add quirk to ignore PHYs | Date | Tue, 5 Oct 2021 14:10:17 -0400 |
| |
On 10/5/21 12:45 PM, Sean Anderson wrote: > > > On 10/5/21 6:33 AM, Russell King (Oracle) wrote: >> On Mon, Oct 04, 2021 at 03:15:27PM -0400, Sean Anderson wrote: >>> Some modules have something at SFP_PHY_ADDR which isn't a PHY. If we try to >>> probe it, we might attach genphy anyway if addresses 2 and 3 return >>> something other than all 1s. To avoid this, add a quirk for these modules >>> so that we do not probe their PHY. >>> >>> The particular module in this case is a Finisar SFP-GB-GE-T. This module is >>> also worked around in xgbe_phy_finisar_phy_quirks() by setting the support >>> manually. However, I do not believe that it has a PHY in the first place: >>> >>> $ i2cdump -y -r 0-31 $BUS 0x56 w >>> 0,8 1,9 2,a 3,b 4,c 5,d 6,e 7,f >>> 00: ff01 ff01 ff01 c20c 010c 01c0 0f00 0120 >>> 08: fc48 000e ff78 0000 0000 0000 0000 00f0 >>> 10: 7800 00bc 0000 401c 680c 0300 0000 0000 >>> 18: ff41 0000 0a00 8890 0000 0000 0000 0000 >> >> Actually, I think that is a PHY. It's byteswapped (which is normal using >> i2cdump in this way).The real contents of the registers are: >> >> 00: 01ff 01ff 01ff 0cc2 0c01 c001 000f 2001 >> 08: 48fc 0e00 78ff 0000 0000 0000 0000 f000 >> 10: 0078 bc00 0000 1c40 0c68 0003 0000 0000 >> 18: 41ff 0000 000a 9088 0000 0000 0000 0000 > > Ah, thanks for catching this. > >> It's advertising pause + asym pause, 1000BASE-T FD, link partner is also >> advertising 1000BASE-T FD but no pause abilities. >> >> When comparing this with a Marvell 88e1111: >> >> 00: 1140 7949 0141 0cc2 05e1 0000 0004 2001 >> 08: 0000 0e00 4000 0000 0000 0000 0000 f000 >> 10: 0078 8100 0000 0040 0568 0000 0000 0000 >> 18: 4100 0000 0002 8084 0000 0000 0000 0000 >> >> It looks remarkably similar. However, The first few reads seem to be >> corrupted with 0x01ff. It may be that the module is slow to allow the >> PHY to start responding - we've had similar with Champion One SFPs. > > Do you have an an example of how to work around this? Even reading one > register at a time I still get the bogus 0x01ff. Reading bytewise, a > reasonable-looking upper byte is returned every other read, but the > lower byte is 0xff every time.
Ok, upon further experimentation, I can read out something reasonable by using the "consecutive byte" mode
# i2cdump -y -r 0-0x3f 2 0x56 c 0 1 2 3 4 5 6 7 8 9 a b c d e f 0123456789abcdef 00: 01 40 01 6d 01 41 0c c2 0c 01 c5 e1 00 0d 20 01 ?@?m?A??????.? ? 10: 59 f9 0e 00 78 00 00 00 00 00 00 00 00 00 f0 00 Y??.x.........?. 20: 00 78 ac 40 00 00 00 00 0c 68 00 00 00 00 00 00 .x?@....?h...... 30: 41 00 00 00 00 0a 90 88 00 00 00 00 00 00 00 00 A....???........
I believe this is just doing i2c_smbus_write_byte+i2c_smbus_read_byte
S Addr Wr [A] Phyaddr [A] P S Addr Rd [A] [DataHigh] NA P S Addr Rd [A] [DataLow] NA P
as opposed to i2c_smbus_read_word_data
S Addr Wr [A] Phyaddr [A] S Addr Rd [A] [DataHigh] A [DataLow] NA P
or i2c_smbus_read_word_data
S Addr Wr [A] Phyaddr [A] S Addr Rd [A] [DataHigh] NA P S Addr Wr [A] Phyaddr [A] S Addr Rd [A] [DataLow] NA P
So now I suppose the question is whether replacing the existing i2c logic with something like
i2c_smbus_write_byte(i2c, i2c_mii_phy_addr(phy_id)); i2c_smbus_read_byte(i2c); i2c_smbus_read_byte(i2c);
will break everyone else's SFP phys. If it does, this could be difficult to do as a quirk because the MII-I2C bus is created before we read the SFP EEPROM.
--Sean
| |