Re: [PATCH net-next 4/5] net: phy: mediatek: add calibration logic for AN7581
From: Wayen Yan <hidden>
Date: 2026-07-08 23:34:17
Also in:
linux-arm-kernel, linux-mediatek, lkml
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