Re: [PATCH net-next 5/5] net: phy: mediatek: add calibration logic for AN7583

[Wayen Yan]

Hi Christian,

A few issues in an7583_phy_config_init():

1) Redundant double assignment of mdi_resister_type

    shared->mdi_resister_type = MDI_5R;

    shared->mdi_resister_type = MDI_5R;   /* duplicate */
    if (shared->mdi_resister_type == MDI_0R)
        shared->r50_cal_tbl = an7583_zcal_to_r50ohm_0R;
    if (shared->mdi_resister_type == MDI_5R)
        shared->r50_cal_tbl = an7583_zcal_to_r50ohm_5R;

The first assignment is immediately overwritten by the second
identical one. Also the two `if` blocks should be `else if` --
currently both conditions are checked independently and the
MDI_0R branch is dead code (since the value is hardcoded to
MDI_5R).

If the intent is to default to MDI_5R and later read from SoC,
the structure should be:

    shared->mdi_resister_type = MDI_5R;  /* FIXME: read from SCU */
    if (shared->mdi_resister_type == MDI_0R)
        shared->r50_cal_tbl = an7583_zcal_to_r50ohm_0R;
    else
        shared->r50_cal_tbl = an7583_zcal_to_r50ohm_5R;

2) Typo: mdi_resister_type -> mdi_resistor_type (same as patch 4/5)

Also the FIXME comment says "MDI Resister Type" -- should be
"Resistor Type".

3) Dependency on patch 4/5 probe fix

an7583_phy_config_init() uses shared->phydev_p0 which is set in
an7581_phy_probe(). If the probe function from patch 4/5 is not
fixed (missing shared = phy_package_get_priv(phydev)), this will
deref NULL/garbage:

    phydev_p0 = shared->phydev_p0;
    phy_offset = phydev->mdio.addr - phydev_p0->mdio.addr;

This highlights that patch 4/5's probe fix is a blocker for both
AN7581 and AN7583.

Best,
Wayen