MMD's in the device list sometimes return 0 for their id.
When that happens lets reset the id back to 0xfffffff so
that we don't get a stub device created for it.
This is a questionable commit, but i'm tossing it out
there along with the comment that reading the spec
seems to indicate that maybe there are further registers
that could be probed in an attempt to resolve some futher
"bad" phys. It sort of comes down to do we want unused phy
devices floating around (potentially unmatched in dt) or
do we want to cut them off early and let DT create them
directly.
For the ACPI case, we don't really have a good way to match
them, and since it hasn't been working I think its perfectly
reasonable at this point to expect phy's to implement enough
of the standard that we can detect them and attach a phy
specific driver.
Signed-off-by: Jeremy Linton <[email protected]>
---
drivers/net/phy/phy_device.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index b2cd22d6315c..acdada865864 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -780,6 +780,10 @@ static int get_phy_c45_ids(struct mii_bus *bus, int addr, u32 *phy_id,
return ret;
if (valid_phy_id(c45_ids->device_ids[i]))
valid_id = true;
+ else
+ c45_ids->device_ids[i] = 0xffffffff;
+
+ /* consider probing PKGID per 45.2.12.2.1? */
}
if (!valid_id && c22_present)
--
2.26.2
On Fri, May 22, 2020 at 04:30:55PM -0500, Jeremy Linton wrote:
> MMD's in the device list sometimes return 0 for their id.
> When that happens lets reset the id back to 0xfffffff so
> that we don't get a stub device created for it.
>
> This is a questionable commit, but i'm tossing it out
> there along with the comment that reading the spec
> seems to indicate that maybe there are further registers
> that could be probed in an attempt to resolve some futher
> "bad" phys. It sort of comes down to do we want unused phy
> devices floating around (potentially unmatched in dt) or
> do we want to cut them off early and let DT create them
> directly.
I'm not sure what you mean "stub device" or "unused phy devices
floating around" - the individual MMDs are not treated as separate
"phy devices", but as one PHY device as a whole.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC for 0.8m (est. 1762m) line in suburbia: sync at 13.1Mbps down 424kbps up
Hi,
On 5/23/20 1:44 PM, Russell King - ARM Linux admin wrote:
> On Fri, May 22, 2020 at 04:30:55PM -0500, Jeremy Linton wrote:
>> MMD's in the device list sometimes return 0 for their id.
>> When that happens lets reset the id back to 0xfffffff so
>> that we don't get a stub device created for it.
>>
>> This is a questionable commit, but i'm tossing it out
>> there along with the comment that reading the spec
>> seems to indicate that maybe there are further registers
>> that could be probed in an attempt to resolve some futher
>> "bad" phys. It sort of comes down to do we want unused phy
>> devices floating around (potentially unmatched in dt) or
>> do we want to cut them off early and let DT create them
>> directly.
>
> I'm not sure what you mean "stub device" or "unused phy devices
> floating around" - the individual MMDs are not treated as separate
> "phy devices", but as one PHY device as a whole.
>
Well, I guess its clearer to say phy/mmd devices with a phy_id=0. Which
is a problem if we don't have DT overriding the phy_id for a given
address. Although AFAIK given a couple of the /sys/bus/mdio_bus/devices
lists I've seen, and after studying this code for a while now, I think
"bogus" phy's might be getting created*. I was far to easy, to upset the
cart when I was hacking on this set, and end up with a directory chuck
full of phys.
So this gets close to one of the questions I asked in the cover letter.
This patch and 09/11 serve to cut off possibly valid phy's which are
failing to identify themselves using the standard registers. Which per
the 802.3 spec there is a blurb about 0 in the id registers for some
cases. Its not really a critical problem for ACPI machines to have these
phys around (OTOH, there might be issues with c22 phys on c45 electrical
buses that respond to c45 reg requests but don't set the c22 regs flag,
I haven't seen that yet.). I considered dropping this patch, and 9/11
was a last minute addition. I kept it because I was worried all those
extra "reserved" MMDs would end up with id = 0's in there and break
something.
* In places where there isn't actually a phy, likely a large part of the
problem was clearing the c22 bit, which allowed 0xFFFFFFFF returns to
slip through the devices list.
On Sun, May 24, 2020 at 11:20:01PM -0500, Jeremy Linton wrote:
> Hi,
>
> On 5/23/20 1:44 PM, Russell King - ARM Linux admin wrote:
> > On Fri, May 22, 2020 at 04:30:55PM -0500, Jeremy Linton wrote:
> > > MMD's in the device list sometimes return 0 for their id.
> > > When that happens lets reset the id back to 0xfffffff so
> > > that we don't get a stub device created for it.
> > >
> > > This is a questionable commit, but i'm tossing it out
> > > there along with the comment that reading the spec
> > > seems to indicate that maybe there are further registers
> > > that could be probed in an attempt to resolve some futher
> > > "bad" phys. It sort of comes down to do we want unused phy
> > > devices floating around (potentially unmatched in dt) or
> > > do we want to cut them off early and let DT create them
> > > directly.
> >
> > I'm not sure what you mean "stub device" or "unused phy devices
> > floating around" - the individual MMDs are not treated as separate
> > "phy devices", but as one PHY device as a whole.
> >
>
> Well, I guess its clearer to say phy/mmd devices with a phy_id=0. Which is a
> problem if we don't have DT overriding the phy_id for a given address.
> Although AFAIK given a couple of the /sys/bus/mdio_bus/devices lists I've
> seen, and after studying this code for a while now, I think "bogus" phy's
> might be getting created*. I was far to easy, to upset the cart when I was
> hacking on this set, and end up with a directory chuck full of phys.
>
> So this gets close to one of the questions I asked in the cover letter. This
> patch and 09/11 serve to cut off possibly valid phy's which are failing to
> identify themselves using the standard registers. Which per the 802.3 spec
> there is a blurb about 0 in the id registers for some cases. Its not really
> a critical problem for ACPI machines to have these phys around (OTOH, there
> might be issues with c22 phys on c45 electrical buses that respond to c45
> reg requests but don't set the c22 regs flag, I haven't seen that yet.).
If you have a classical clause 22 PHY on a clause 45 bus, it isn't
going to respond to clause 45 cycles, so it isn't going to respond to
a request to read the devices-in-package register, so there is no
"c22 regs" flag.
> I
> considered dropping this patch, and 9/11 was a last minute addition. I kept
> it because I was worried all those extra "reserved" MMDs would end up with
> id = 0's in there and break something.
>
> * In places where there isn't actually a phy, likely a large part of the
> problem was clearing the c22 bit, which allowed 0xFFFFFFFF returns to slip
> through the devices list.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC for 0.8m (est. 1762m) line in suburbia: sync at 13.1Mbps down 424kbps up