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,
