Re: [RFC 08/11] net: phy: Allow mdio buses to auto-probe c45 devices

[Jeremy Linton]

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,