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

[Wayen Yan <win847@gmail.com>]

Hi Christian,

Thanks for working on this. One critical bug found that will crash
on probe, plus a couple of minor issues.

1) Uninitialized shared pointer in an7581_phy_probe()

The local variable `shared` is declared but never assigned before
use:

    static int an7581_phy_probe(struct phy_device *phydev)
    {
        struct airoha_socphy_shared *shared;   /* not initialized */
        ...
        ret = devm_phy_package_join(&phydev->mdio.dev, phydev, 0,
                                    sizeof(struct airoha_socphy_shared));
        if (ret)
            return ret;

        ...
        if (phydev->mdio.addr == AIROHA_DEFAULT_PORT0_ADDR)
            shared->phydev_p0 = phydev;   /* writing to uninitialized pointer */

devm_phy_package_join() allocates the shared priv data internally
(accessible via phydev->shared->priv), but the local variable
`shared` itself is never populated. You need to call
phy_package_get_priv(phydev) after the join succeeds before
accessing any shared fields.

Fix:

    shared = phy_package_get_priv(phydev);

should be added right after devm_phy_package_join() succeeds.

Without this fix, the first PHY to probe (addr == 0x9) will crash,
and all subsequent PHYs' config_init will dereference an
uninitialized phydev_p0 in every calibration function.

2) Typo: mdi_resister_type -> mdi_resistor_type

The field name "resister" appears in multiple places (enum, struct
field, config_init, FIXME comments). It should be "resistor".
This will be baked into the ABI once merged so worth fixing now:

- enum airoha_mdi_resister_type -> airoha_mdi_resistor_type
- shared->mdi_resister_type -> shared->mdi_resistor_type
- FIXME comment: "MDI Resister Type" -> "MDI Resistor Type"

3) Observation: mdi_resister_type is always MDI_5R but tables have
   MDI_0R data

Currently the code hardcodes mdi_resister_type = MDI_5R, so the
MDI_0R entries in an7581_tx_amp_compensation_tbl[] are dead data.
The FIXME suggests this should be read from SCU registers
eventually. Consider adding the MDI_0R branch now (or at minimum
an else) so the code structure is ready when the SCU read is
implemented, and avoid shipping dead table data.

Best,
Wayen