Message ID | ae4f76fd0903310344s2d46edf2l505a1d21a4778d54@mail.gmail.com (mailing list archive) |
---|---|
State | Not Applicable, archived |
Headers | show |
On Tue, 31 Mar 2009 12:44:15 +0200 Henk Stegeman <henk.stegeman@gmail.com> wrote: > Fix fec_mpc52xx driver to use net_device_ops and to be careful not to > dereference phy_device if a phy has not yet been connected. > > Signed-off-by: Henk Stegeman <henk.stegeman@gmail.com> > > diff --git a/drivers/net/fec_mpc52xx.c b/drivers/net/fec_mpc52xx.c > index cd8e98b..ca76b95 100644 > --- a/drivers/net/fec_mpc52xx.c > +++ b/drivers/net/fec_mpc52xx.c > @@ -847,24 +847,40 @@ static void mpc52xx_fec_get_drvinfo(struct > net_device *dev, > static int mpc52xx_fec_get_settings(struct net_device *dev, struct > ethtool_cmd *cmd) > { > struct mpc52xx_fec_priv *priv = netdev_priv(dev); > + > + if (!priv->phydev) > + return -ENODEV; > + > return phy_ethtool_gset(priv->phydev, cmd); > } > > static int mpc52xx_fec_set_settings(struct net_device *dev, struct > ethtool_cmd *cmd) > { > struct mpc52xx_fec_priv *priv = netdev_priv(dev); > + > + if (!priv->phydev) > + return -ENODEV; > + > return phy_ethtool_sset(priv->phydev, cmd); > } > > static u32 mpc52xx_fec_get_msglevel(struct net_device *dev) > { > struct mpc52xx_fec_priv *priv = netdev_priv(dev); > + > + if (!priv->phydev) > + return 0; > + > return priv->msg_enable; > } > > static void mpc52xx_fec_set_msglevel(struct net_device *dev, u32 level) > { > struct mpc52xx_fec_priv *priv = netdev_priv(dev); > + > + if (!priv->phydev) > + return; > + > priv->msg_enable = level; > } > > @@ -882,12 +898,31 @@ static int mpc52xx_fec_ioctl(struct net_device > *dev, struct ifreq *rq, int cmd) > { > struct mpc52xx_fec_priv *priv = netdev_priv(dev); > > + if (!priv->phydev) > + return -ENODEV; > + > return mpc52xx_fec_phy_mii_ioctl(priv, if_mii(rq), cmd); > } > > /* ======================================================================== */ > /* OF Driver */ > /* ======================================================================== */ > +static const struct net_device_ops mpc52xx_fec_netdev_ops = { > + .ndo_open = mpc52xx_fec_open, > + .ndo_stop = mpc52xx_fec_close, > + .ndo_start_xmit = mpc52xx_fec_hard_start_xmit, > + .ndo_tx_timeout = mpc52xx_fec_tx_timeout, > + .ndo_get_stats = mpc52xx_fec_get_stats, > + .ndo_set_multicast_list = mpc52xx_fec_set_multicast_list, > + .ndo_validate_addr = eth_validate_addr, > + .ndo_set_mac_address = mpc52xx_fec_set_mac_address, > + .ndo_do_ioctl = mpc52xx_fec_ioctl, > What about change_mtu? Don't you want: .ndo_change_mtu = eth_change_mtu,
On Tue, Mar 31, 2009 at 8:48 AM, Stephen Hemminger <shemminger@vyatta.com> wrote: > On Tue, 31 Mar 2009 12:44:15 +0200 > Henk Stegeman <henk.stegeman@gmail.com> wrote: > >> Fix fec_mpc52xx driver to use net_device_ops and to be careful not to >> dereference phy_device if a phy has not yet been connected. >> >> Signed-off-by: Henk Stegeman <henk.stegeman@gmail.com> Hi Henk, I hadn't heard from you for a while about the signed-off-by line, but I really needed to get those changes queued because the 2.6.30 merge window is open now, and some of my patches depend on it. I ended up rewriting your patch from scratch, testing it and posting it last night. I would really like to stick with the rewritten version since it is a little tighter, and it is the one I've got tested. However, now that I've heard from you, I'm happy to change the patch author credit to you. You should look on the list for the patches I sent out last night (labeled [PATCH 02/14] and [PATCH 03/14]) and reply to each of them with an "Acked-by:" line if they look good to you. g.
On Tue, Mar 31, 2009 at 8:48 AM, Stephen Hemminger <shemminger@vyatta.com> wrote: > On Tue, 31 Mar 2009 12:44:15 +0200 >> +static const struct net_device_ops mpc52xx_fec_netdev_ops = { >> + .ndo_open = mpc52xx_fec_open, >> + .ndo_stop = mpc52xx_fec_close, >> + .ndo_start_xmit = mpc52xx_fec_hard_start_xmit, >> + .ndo_tx_timeout = mpc52xx_fec_tx_timeout, >> + .ndo_get_stats = mpc52xx_fec_get_stats, >> + .ndo_set_multicast_list = mpc52xx_fec_set_multicast_list, >> + .ndo_validate_addr = eth_validate_addr, >> + .ndo_set_mac_address = mpc52xx_fec_set_mac_address, >> + .ndo_do_ioctl = mpc52xx_fec_ioctl, >> > What about change_mtu? Don't you want: > .ndo_change_mtu = eth_change_mtu, Yes, you're right. fixed. Thanks, g.
diff --git a/drivers/net/fec_mpc52xx.c b/drivers/net/fec_mpc52xx.c index cd8e98b..ca76b95 100644 --- a/drivers/net/fec_mpc52xx.c +++ b/drivers/net/fec_mpc52xx.c @@ -847,24 +847,40 @@ static void mpc52xx_fec_get_drvinfo(struct net_device *dev, static int mpc52xx_fec_get_settings(struct net_device *dev, struct ethtool_cmd *cmd) { struct mpc52xx_fec_priv *priv = netdev_priv(dev); + + if (!priv->phydev) + return -ENODEV; + return phy_ethtool_gset(priv->phydev, cmd); } static int mpc52xx_fec_set_settings(struct net_device *dev, struct ethtool_cmd *cmd) { struct mpc52xx_fec_priv *priv = netdev_priv(dev); + + if (!priv->phydev) + return -ENODEV; + return phy_ethtool_sset(priv->phydev, cmd); } static u32 mpc52xx_fec_get_msglevel(struct net_device *dev) { struct mpc52xx_fec_priv *priv = netdev_priv(dev); + + if (!priv->phydev) + return 0; + return priv->msg_enable; } static void mpc52xx_fec_set_msglevel(struct net_device *dev, u32 level) { struct mpc52xx_fec_priv *priv = netdev_priv(dev); + + if (!priv->phydev) + return; +
Fix fec_mpc52xx driver to use net_device_ops and to be careful not to dereference phy_device if a phy has not yet been connected. Signed-off-by: Henk Stegeman <henk.stegeman@gmail.com> priv->msg_enable = level; } @@ -882,12 +898,31 @@ static int mpc52xx_fec_ioctl(struct net_device *dev, struct ifreq *rq, int cmd) { struct mpc52xx_fec_priv *priv = netdev_priv(dev); + if (!priv->phydev) + return -ENODEV; + return mpc52xx_fec_phy_mii_ioctl(priv, if_mii(rq), cmd); } /* ======================================================================== */ /* OF Driver */ /* ======================================================================== */ +static const struct net_device_ops mpc52xx_fec_netdev_ops = { + .ndo_open = mpc52xx_fec_open, + .ndo_stop = mpc52xx_fec_close, + .ndo_start_xmit = mpc52xx_fec_hard_start_xmit, + .ndo_tx_timeout = mpc52xx_fec_tx_timeout, + .ndo_get_stats = mpc52xx_fec_get_stats, + .ndo_set_multicast_list = mpc52xx_fec_set_multicast_list, + .ndo_validate_addr = eth_validate_addr, + .ndo_set_mac_address = mpc52xx_fec_set_mac_address, + .ndo_do_ioctl = mpc52xx_fec_ioctl, + +#ifdef CONFIG_NET_POLL_CONTROLLER + .ndo_poll_controller = mpc52xx_fec_poll_controller, +#endif +}; + static int __devinit mpc52xx_fec_probe(struct of_device *op, const struct of_device_id *match) @@ -929,20 +964,10 @@ mpc52xx_fec_probe(struct of_device *op, const struct of_device_id *match) return -EBUSY; /* Init ether ndev with what we have */ - ndev->open = mpc52xx_fec_open; - ndev->stop = mpc52xx_fec_close; - ndev->hard_start_xmit = mpc52xx_fec_hard_start_xmit; - ndev->do_ioctl = mpc52xx_fec_ioctl; ndev->ethtool_ops = &mpc52xx_fec_ethtool_ops; - ndev->get_stats = mpc52xx_fec_get_stats; - ndev->set_mac_address = mpc52xx_fec_set_mac_address; - ndev->set_multicast_list = mpc52xx_fec_set_multicast_list; - ndev->tx_timeout = mpc52xx_fec_tx_timeout; ndev->watchdog_timeo = FEC_WATCHDOG_TIMEOUT; ndev->base_addr = mem.start; -#ifdef CONFIG_NET_POLL_CONTROLLER - ndev->poll_controller = mpc52xx_fec_poll_controller; -#endif + ndev->netdev_ops = &mpc52xx_fec_netdev_ops; priv->t_irq = priv->r_irq = ndev->irq = NO_IRQ; /* IRQ are free for now */