Re: [PATCH net-next v10 04/15] net: phy: Introduce PHY ports representation

[Maxime Chevallier <maxime.chevallier@bootlin.com>]

On Sat, 26 Jul 2025 22:33:33 +0200
Andrew Lunn <andrew@lunn.ch> wrote:

> > +
> > +/**
> > + * phy_caps_medium_get_supported() - Returns linkmodes supported on a given medium
> > + * @supported: After this call, contains all possible linkmodes on a given medium,
> > + *	       and with the given number of lanes.  
> 
> Maybe nit picking, but maybe append:
> 
> , or less.
> 
> > +		/* For most cases, min_lanes == lanes, except for 10/100BaseT that work
> > +		 * on 2 lanes but are compatible with 4 lanes mediums
> > +		 */
> > +		if (link_mode_params[i].mediums & BIT(medium) &&
> > +		    link_mode_params[i].lanes >= lanes &&
> > +		    link_mode_params[i].min_lanes <= lanes) {  
> 
> We should only care about min_lanes here. I don't think the
> link_mode_params[i].lanes >= lanes is needed.
> 
> Maybe you can add a BUILD_BUG_ON() into the macro to ensure
> min_lanes <= lanes?
> 
> > +struct phy_port *phy_of_parse_port(struct device_node *dn)
> > +{
> > +	struct fwnode_handle *fwnode = of_fwnode_handle(dn);
> > +	enum ethtool_link_medium medium;
> > +	struct phy_port *port;
> > +	const char *med_str;
> > +	u32 lanes, mediums = 0;
> > +	int ret;
> > +
> > +	ret = fwnode_property_read_u32(fwnode, "lanes", &lanes);
> > +	if (ret)
> > +		lanes = 0;  
> 
> The DT binding says that both properties are required. So i think this
> should be:
> 
> 		return ret;

Ah true indeed, let me fix that then :)

> 
> > + * phy_port_get_type() - get the PORT_* attribut for that port.  
> 
> attribut_e_
> 
> > +	 * If the port isn't initialized, the port->mediums and port->lanes
> > +	 * fields must be set, possibly according to stapping information.  
> 
> st_r_apping
> 
> 	Andrew

Sorry for all the typos, it's a weak excuse but lately my laptop's
keayboard has been acting up, and it either misses strokes or gets the
letters stuck in repeat-mode. Russell has similar issues, so I'm still
unsure if this is a hardware or software thing... I'll install and pass
spellcheck for the next iteration to avoid that :)

Maxime