Messages in this thread |  | | Date | Mon, 30 Nov 2020 14:28:35 +0100 | From | Steen Hegelund <> | Subject | Re: [RFC PATCH 2/3] net: sparx5: Add Sparx5 switchdev driver |
| |
On 28.11.2020 20:03, Andrew Lunn wrote: >EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > >> +static int sparx5_port_open(struct net_device *ndev) >> +{ >> + struct sparx5_port *port = netdev_priv(ndev); >> + int err = 0; >> + >> + sparx5_port_enable(port, true); >> + if (port->conf.phy_mode != PHY_INTERFACE_MODE_NA) { >> + err = phylink_of_phy_connect(port->phylink, port->of_node, 0); >> + if (err) { >> + netdev_err(ndev, "Could not attach to PHY\n"); >> + return err; >> + } >> + } > >This looks a bit odd. PHY_INTERFACE_MODE_NA means don't touch, >something else has already configured the MAC-PHY mode in the PHY. >You should not not connect the PHY because of this.
Hmm. I will have to revisit this again. The intent was to be able to destinguish between regular PHYs and SFPs (as read from the DT). But maybe the phylink_of_phy_connect function handles this automatically... > >> +void sparx5_destroy_netdev(struct sparx5 *sparx5, struct sparx5_port *port) >> +{ >> + if (port->phylink) { >> + /* Disconnect the phy */ >> + if (rtnl_trylock()) { > >Why do you use rtnl_trylock()?
The sparx5_port_stop() in turn calls phylink_stop() that expects the lock to be taken. Should I rather just call rtnl_lock()?
Thanks for your comments
/Steen
> > Andrew
BR Steen
--------------------------------------- Steen Hegelund steen.hegelund@microchip.com
|  |