Re: [PATCHv3 1/5] net: dsa: Support internal phy on 'cpu' port

[Sebastian Reichel <sebastian.reichel-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 1213 bytes --]

Hi,

On Mon, Jan 15, 2018 at 11:57:18PM +0100, Andrew Lunn wrote:
> >  int dsa_port_fixed_link_register_of(struct dsa_port *dp)
> >  {
> >  	struct device_node *dn = dp->dn;
> > @@ -305,6 +354,10 @@ int dsa_port_fixed_link_register_of(struct dsa_port *dp)
> >  			ds->ops->adjust_link(ds, port, phydev);
> >  
> >  		put_device(&phydev->mdio.dev);
> > +	} else {
> > +		err = dsa_port_setup_phy_of(dp, true);
> > +		if (err)
> > +			return err;
> 
> Hi Sebastian
> 
> First off, i tend to agree with Florian. I'm not sure how reliable
> this is. There is normally a state machine moving the PHY between
> different states. But in order to do that, i think you need a netdev.
> Have you tried multiple down/up of the other MAC/PHY? Does it always
> work?

I tested multiple down/up transitions and everything works as
expected.

> But, at the moment, we don't have much better.
> 
> What i don't like is having this code inside
> dsa_port_fixed_link_register_of(). This has nothing to do with a fixed
> link. Please export functions from port.c and call them directly from
> dsa_port_setup() and dsa_port_teardown().

I just sent PATCHv4 implementing this change.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]