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

[Daniel Golle <daniel@makrotopia.org>]

On Thu, Sep 17, 2026 at 05:11:50PM +0200, Caleb James DeLisle wrote:
> 
> On 17/09/2026 16:48, Daniel Golle wrote:
> > On Tue, Sep 15, 2026 at 02:01:29PM +0200, Caleb James DeLisle wrote:
> > > On 15/09/2026 13:47, Daniel Golle wrote:
> > > > On Tue, Sep 15, 2026 at 11:34:27AM +0000, Caleb James DeLisle wrote:
> > > > > @@ -135,6 +199,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),
> > > > I'd suggest to actually use phy_id and phy_id_mask assigned by the
> > > > PHY_ID_MATCH_EXACT macro by calling genphy_match_phy_device() in your
> > > > match functions above instead of open-coding the ID match.
> > > > Or drop PHY_ID_MATCH_EXACT from *both* drivers.
> > > I suppose the latter is easier because then I don't have to re-think
> > > mt7530_is_gphy() which would be lying if it wasn't actually checking ID is
> > > MTK_GPHY_ID_MT7530.
> > > 
> > I would have preferred to call genphy_match_phy_device() in your match
> > functions instead of open-coding phy_id_compare() which is best
> > reached via genphy_match_phy_device() in this situation -- that would
> > express the code intent in the most obvious way imho.
> 
> 
> I did it this way because the name mt7530_is_gphy() implies "Is this an
> MT7530 gigabit PHY?" which if it doesn't match on MTK_GPHY_ID_MT7530 then
> that's not what it does so there's a little bit more thought involved.
> 
> 
> If I'd have known this was really your preference I'd have done that, but I
> already just sent v6 so I guess I can send v7 tomorrow.

It's also fine for me like it is now, just a light preference, not
worth a resend just for that.

However, I'd also still like to see the OF parent walk replaced by
register evidence, which requires that you dump the registers more
than once to understand if any difference are things like counters or
temperature, or actually identify the PHY variant.

It can of course be that the actual silicon is really the exact same
MT7530 MCM as eg. on MT7623, just driven by a different clock
frequency and the observable register differences are things like
clock devidors -- if we manage to understand that and just apply the
resulting tuning accordingly, even better.