On Mon, Jun 22, 2020 at 02:36:19PM +0100, Russell King - ARM Linux admin wrote: > On Mon, Jun 22, 2020 at 06:46:52PM +0530, Calvin Johnson wrote: > > On Mon, Jun 22, 2020 at 09:29:17AM +0000, Madalin Bucur (OSS) wrote: > > > > -----Original Message----- > > > > From: Calvin Johnson (OSS) <calvin.john...@oss.nxp.com> > > > > Sent: Monday, June 22, 2020 11:19 AM > > > > To: Jeremy Linton <jeremy.lin...@arm.com>; Russell King - ARM Linux > > > > admin > > > > <li...@armlinux.org.uk>; Jon <j...@solid-run.com>; Cristi Sovaiala > > > > <cristian.sovai...@nxp.com>; Ioana Ciornei <ioana.cior...@nxp.com>; > > > > Andrew > > > > Lunn <and...@lunn.ch>; Andy Shevchenko <andy.shevche...@gmail.com>; > > > > Florian Fainelli <f.faine...@gmail.com>; Madalin Bucur (OSS) > > > > <madalin.bu...@oss.nxp.com> > > > > Cc: linux...@gmail.com; netdev@vger.kernel.org; Calvin Johnson (OSS) > > > > <calvin.john...@oss.nxp.com> > > > > Subject: [net-next PATCH v2 1/3] net: phy: Allow mdio buses to > > > > auto-probe > > > > c45 devices > > > > > > > > From: Jeremy Linton <jeremy.lin...@arm.com> > > > > > > > > The mdiobus_scan logic is currently hardcoded to only > > > > work with c22 devices. This works fairly well in most > > > > cases, but its possible that a c45 device doesn't respond > > > > despite being a standard phy. If the parent hardware > > > > is capable, it makes sense to scan for c22 devices before > > > > falling back to c45. > > > > > > > > As we want this to reflect the capabilities of the STA, > > > > lets add a field to the mii_bus structure to represent > > > > the capability. That way devices can opt into the extended > > > > scanning. Existing users should continue to default to c22 > > > > only scanning as long as they are zero'ing the structure > > > > before use. > > > > > > How is this default working for existing users, the code below does not > > > seem > > > to do anything for a zeroed struct, as there is no default in the switch? > > > > Looking further into this, I think MDIOBUS_C22 = 0, was correct. Prior to > > this patch, get_phy_device() was executed for C22 in this path. I'll discuss > > with Russell and Andrew on this and get back. > > It is not correct for the reasons I stated when I made the comment. > When you introduce "probe_capabilities", every MDIO bus will have > that field as zero. > > In your original patch, that means the bus only supports clause 22. > However, we have buses today that _that_ is factually incorrect. > Therefore, introducing probe_capabilities with zero meaning MDIOBUS_C22 > is wrong. It means we can _never_ assume that bus->probe_capabilities > means the bus does not support Clause 45. > > Now, as per your patch below, that is better. It means we're able to > identify those drivers that have not declared which bus access methods > are supported, while we can positively identify those which have. > > All that's needed is for your switch() statement to maintain today's > behaviour where no declared probe_capabilities means that the bus > should be probed for clause 22 PHYs. > > This means we can later introduce the ability to prevent clause 45 > probing for PHYs that declare themselves as explicitly only supporting > clause 22 if we need to without having been backed into a corner, and > left wondering whether the lack of probe_capabilities is because someone > decided "it's zero, so doesn't need to be initialised" and didn't bother > explicitly stating .probe_capabilities = MDIOBUS_C22.
Got it. I'll add the MDIOBUS_NO_CAP case also. Thanks Calvin
