Re: [PATCH net-next] net: phy: mediatek: add driver for EcoNet Fast Ethernet SoC PHYs
From: Caleb James DeLisle
Date: Thu Aug 27 2026 - 03:13:37 EST
On 26/08/2026 04:08, Andrew Lunn wrote:
On Wed, Aug 26, 2026 at 02:08:50AM +0200, Caleb James DeLisle wrote:Tested and it worked, re-sent.
On 25/08/2026 21:40, Andrew Lunn wrote:If it is as simple as that, i suggest you detect it at runtime.
Also provide support for the older EN7512 Fast Ethernet SoC PHYs foundIs there a way to tell them apart using other registers?
in EN751221 chips with do not have the MCM switch. That is chips which
do not have a "G" in the name. As these PHYs bear the ID 03a2.9412
which collides with MTK_GPHY_ID_MT7530 gigabit PHY, do not match them
and instead rely on the user to override the PHY ID in the device tree
if they wish to use this driver.
The reference code differentiates this PHY from the other because this one
does not advertise gigabit capability. I sent it like this because the idea
of a matcher made me nervous and this felt more conservative, but I'm open
to guidance about what is the most appropriate solution.
In the driver structure, you provide a match_phy_device()
function. This get called independent of what ID value you have in the
structure. So you first need to check if the ID matches. Then check
the 1G capability.
Ideally you want the code in the same driver, because module loading
happens based on the ID. I'm not sure user space will load two drivers
if they both indicate the same ID. Something you can experiment with.
Changed loop for readability, return -EIO, and added a comment as well.
Consider adding comments as well. Also consider replacing the infinite+ for (;;) {It is unusual to do loops like this. Can it be turned into a do while
loop? And without looking deep into it, it is not clear if this is
endless if the hardware stops responding.
You raise a good point in that it's hard to reason out the default exit
scenario, and I will figure out how to improve that. I'm not sure I can
actually get rid of the infinite loop without making the code worse, because
the default exit condition needs to assign ret and goto the error out label.
loop with a bounded loop, and return -EIO if the it goes around the
loop too many times. What you are trying to avoid is on hardware error
a CPU constantly spinning until power off.
Consider using phy_package_init_once() to configure the shared thingsIt doesn't need to be configured first but it must be probed because these+static int en751221_fephy_config_init(struct phy_device *phydev)What is the issue here? Why must port 0 be first?
+{
+ struct econet_socphy_shared *shared = phy_package_get_priv(phydev);
+ u16 l0r26_temp;
+ int ret;
+ int i;
+
+ if (!shared->phydev_p0) {
+ phydev_err(phydev, "Port zero must be configured\n");
+ return -EOPNOTSUPP;
+ }
PHYs are not independent, they exist as a group and some configuration must
be done on the first phy in the group. I will re-send with clarification in
a comment.
by the first PHY to probe, independent of what address it is.
That doesn't solve the problem, but I re-sent with much more effort put into explaining what the problem is. In short, these PHYs are not independent - they exist in a group, and there is a "master" PHY of the group which has extra registers that are used by all PHYs of the group. If the master is not configured, then calibration cannot proceed for any PHY of the group.
Thank you for your review and advice.
Caleb
Andrew