Re: [PATCH v4 net-next] net: phy: mediatek: support MT7530 PHYs on EN71221 MCM

[Caleb James DeLisle]

On 15/09/2026 12:48, Daniel Golle wrote:
> On Tue, Sep 15, 2026 at 10:35:50AM +0000, Caleb James DeLisle wrote:
> > The EcoNet EN751221 multi-chip module implementation of the MT7530
> > requires some additional configuration of the PHYs on startup.
> > The reason for this is not known, but it is possible that it has
> > to do with the fact that the EN751221 MCM implementation of the
> > MT7530 runs at an abnormal PLL frequency (362.5Mhz).
> >
> > Detect whether the MT7530 PHY is attached to the MDIO bus of an
> > EcoNet EN751221 switch and if so, apply the necessary register
> > updates. Additionally, never attempt to configure an MT7530
> > identified PHY which does not have gigabit support because the same
> > ID is used for another (FE) PHY.
> >
> > Co-developed-by: Matheus Sampaio Queiroga <srherobrine20@xxxxxxxxx>
> > Signed-off-by: Matheus Sampaio Queiroga <srherobrine20@xxxxxxxxx>
> > Signed-off-by: Caleb James DeLisle <cjd@xxxxxxxx>
> > [...]
> > +static bool en751221_is_mcm_phy(struct phy_device *phydev)
> > +{
> > +	struct device *parent = phydev->mdio.bus->parent;
> > +
> > +	return parent && parent->of_node &&
> > +	       of_device_is_compatible(parent->of_node, "econet,en751221");
> I thought you had found better options than using the parent of_node...?


I documented my findings re registers that are different, but I don't 
have any documentation of those registers so to me they might as well be 
reading outside air temperature, so there is nothing I could safely do 
except the parent of_node.


> > +}
> > +
> > +/*
> > + * MTK_GPHY_ID_MT7530 ID is also used for an EcoNet SoC FE phy, but that PHY
> > + * does not advertise ESTATUS_1000_TFULL.
> > + */
> > +static int mt7530_phy_match(struct phy_device *phydev,
> > +			    const struct phy_driver *phydrv)
> > +{
> You miss the check for the PHY ID, see also below.
>
> I'd suggest something like
> 	int ret;
>
> 	ret = genphy_match_phy_device(phydev, phydrv);
> 	if (!ret)
> 		return 0;
>
> > +	return (phy_read(phydev, MII_ESTATUS) & ESTATUS_1000_TFULL) != 0 &&
> The above should be helper function used by both .match_phy_device ops.


Sounds good.


> > +		!en751221_is_mcm_phy(phydev);
> > +}
> > +
> > +static int en751221_phy_match(struct phy_device *phydev,
> > +			      const struct phy_driver *phydrv)
> > +{
> > +	return (phy_read(phydev, MII_ESTATUS) & ESTATUS_1000_TFULL) != 0 &&
> > +		en751221_is_mcm_phy(phydev);
> > +}
> > +
> >   static int mt7531_phy_config_init(struct phy_device *phydev)
> >   {
> >   	mtk_gephy_config_init(phydev);
> > @@ -135,6 +195,23 @@ static struct phy_driver mtk_gephy_driver[] = {
> >   		 */
> >   		.config_intr	= genphy_no_config_intr,
> >   		.handle_interrupt = genphy_handle_interrupt_no_ack,
> > +		.match_phy_device = mt7530_phy_match,
> > +		.suspend	= genphy_suspend,
> > +		.resume		= genphy_resume,
> > +		.read_page	= mtk_phy_read_page,
> > +		.write_page	= mtk_phy_write_page,
> > +	},
> > +	{
> > +		PHY_ID_MATCH_EXACT(MTK_GPHY_ID_MT7530),
> This assigns phy_id and phy_id_mask, which are used by
> genphy_match_phy_device(). However, you are setting a .match_phy_device
> op which doesn't call genphy_match_phy_device(), so this is ignored.


I see, I should have checked that more closely. Thanks, will fix.


Caleb

> > +		.name		= "EcoNet EN751221 MCM PHY",
> > +		.probe		= mt7530_phy_probe,
> > +		.config_init	= en751221_mcm_phy_config_init,
> > +		/* Interrupts are handled by the switch, not the PHY
> > +		 * itself.
> > +		 */
> > +		.config_intr	= genphy_no_config_intr,
> > +		.handle_interrupt = genphy_handle_interrupt_no_ack,
> > +		.match_phy_device = en751221_phy_match,
> >   		.suspend	= genphy_suspend,
> >   		.resume		= genphy_resume,
> >   		.read_page	= mtk_phy_read_page,
> > --
> > 2.39.5