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

From: Sebastian Reichel
Date: Tue Jan 16 2018 - 05:27:29 EST


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: signature.asc
Description: PGP signature