On Tue, Oct 22, 2019 at 03:06:49AM +0200, Andrew Lunn wrote:
> Hi Ioana
>
> > +static int dpaa2_eth_connect_mac(struct dpaa2_eth_priv *priv)
> > +{
> > + struct fsl_mc_device *dpni_dev, *dpmac_dev;
> > + struct dpaa2_mac *mac;
> > + int err;
> > +
> > + dpni_dev = to_fsl_mc_device(priv->net_dev->dev.parent);
> > + dpmac_dev = fsl_mc_get_endpoint(dpni_dev);
> > + if (!dpmac_dev || dpmac_dev->dev.type != &fsl_mc_bus_dpmac_type)
> > + return 0;
> > +
> > + if (dpaa2_mac_is_type_fixed(dpmac_dev, priv->mc_io))
> > + return 0;
> > +
> > + mac = kzalloc(sizeof(struct dpaa2_mac), GFP_KERNEL);
> > + if (!mac)
> > + return -ENOMEM;
> > +
> > + mac->mc_dev = dpmac_dev;
> > + mac->mc_io = priv->mc_io;
> > + mac->net_dev = priv->net_dev;
> > +
> > + err = dpaa2_mac_connect(mac);
> > + if (err) {
> > + netdev_err(priv->net_dev, "Error connecting to the MAC
> > endpoint\n");
> > + kfree(mac);
> > + return err;
> > + }
> > + priv->mac = mac;
> > +
> > + return 0;
> > +}
> > +
> > +static void dpaa2_eth_disconnect_mac(struct dpaa2_eth_priv *priv)
> > +{
> > + if (!priv->mac)
> > + return;
> > +
> > + rtnl_lock();
> > + dpaa2_mac_disconnect(priv->mac);
> > + kfree(priv->mac);
> > + priv->mac = NULL;
> > + rtnl_unlock();
> > +}
>
> dpaa2_eth_connect_mac() does not take the rtnl lock.
> dpaa2_eth_disconnect_mac() does. This asymmetry makes me think
> something is wrong. But it could be correct....
The way the driver is written, it's fine.
dpaa2_eth_connect_mac() is called prior to the netdev being registered.
At that point, nothing is published.
dpaa2_eth_disconnect_mac() is called _prior_ to the netdev being
unregistered, so there could be live accesses happening to the phy
and phylink.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
