Messages in this thread | | | Subject | Re: [RFC 08/11] net: phy: Allow mdio buses to auto-probe c45 devices | From | Jeremy Linton <> | Date | Sun, 24 May 2020 23:28:52 -0500 |
| |
Hi,
On 5/24/20 9:44 AM, Andrew Lunn wrote: >> +++ b/include/linux/phy.h >> @@ -275,6 +275,11 @@ struct mii_bus { >> int reset_delay_us; >> /* RESET GPIO descriptor pointer */ >> struct gpio_desc *reset_gpiod; >> + /* bus capabilities, used for probing */ >> + enum { >> + MDIOBUS_C22_ONLY = 0, >> + MDIOBUS_C45_FIRST, >> + } probe_capabilities; >> }; > > > I'm not so keen on _FIRST. It suggest _LAST would also be valid. But > that then suggests this is not a bus property, but a PHY property, and > some PHYs might need _FIRST and other phys need _LAST, and then you > have a bus which has both sorts of PHY on it, and you have a problem. > > So i think it would be better to have > > enum { > MDIOBUS_UNKNOWN = 0, > MDIOBUS_C22, > MDIOBUS_C45, > MDIOBUS_C45_C22, > } bus_capabilities; > > Describe just what the bus master can support.
Yes, the naming is reasonable and I will update it in the next patch. I went around a bit myself with this naming early on, and the problem I saw was that a C45 capable master, can have C45 electrical phy's that only respond to c22 requests (AFAIK). So the MDIOBUS_C45 (I think I was calling it C45_ONLY) is an invalid selection. Not, that it wouldn't be helpful to have a C45_ONLY case, but that the assumption is that you wouldn't try and probe c22 registers, which I thought was a mistake.
Thanks,
| |