Messages in this thread | | | Date | Sun, 10 May 2020 17:04:11 +0200 | From | Michael Walle <> | Subject | Re: [PATCH net-next] net: phy: at803x: add cable diagnostics support |
| |
Am 2020-05-10 16:52, schrieb Andrew Lunn: > More of a note to self: > > Now we have three implementations, we start to see bits of common code > which could be pulled out and shared. > >> +static bool at803x_cdt_fault_length_valid(u16 status) >> +{ >> + switch (FIELD_GET(AT803X_CDT_STATUS_STAT_MASK, status)) { >> + case AT803X_CDT_STATUS_STAT_OPEN: >> + case AT803X_CDT_STATUS_STAT_SHORT: >> + return true; >> + } >> + return false; >> +} > > If we uses the netlink attribute values, not the PHY specific values, > this could be put in the core. > >> + >> +static int at803x_cdt_fault_length(u16 status) >> +{ >> + int dt; >> + >> + /* According to the datasheet the distance to the fault is >> + * DELTA_TIME * 0.824 meters. >> + * >> + * The author suspect the correct formula is: >> + * >> + * fault_distance = DELTA_TIME * (c * VF) / 125MHz / 2 >> + * >> + * where c is the speed of light, VF is the velocity factor of >> + * the twisted pair cable, 125MHz the counter frequency and >> + * we need to divide by 2 because the hardware will measure the >> + * round trip time to the fault and back to the PHY. >> + * >> + * With a VF of 0.69 we get the factor 0.824 mentioned in the >> + * datasheet. >> + */ >> + dt = FIELD_GET(AT803X_CDT_STATUS_DELTA_TIME_MASK, status); >> + >> + return (dt * 824) / 10; >> +} > > There seems to be a general consensus of 0.824 meters. So we could > have helpers to convert back and forth in the core.
Yeah, but I don't know why VF is 0.69 (and it differs per cable, so its just some kind of sensible default value). According to https://en.wikipedia.org/wiki/Velocity_factor it should be something around 0.64 or 0.65.
-michael
> >> +static int at803x_cable_test_start(struct phy_device *phydev) >> +{ >> + /* Enable auto-negotiation, but advertise no capabilities, no link >> + * will be established. A restart of the auto-negotiation is not >> + * required, because the cable test will automatically break the >> link. >> + */ >> + phy_write(phydev, MII_BMCR, BMCR_ANENABLE); >> + phy_write(phydev, MII_ADVERTISE, ADVERTISE_CSMA); >> + phy_write(phydev, MII_CTRL1000, 0); > > Could be a genphy_ helper. > > Lets get the code merged, when we can come back and do some > refactoring. > > Andrew
| |