Patchwork ucc_geth: Convert to net_device_ops

login
register
mail settings
Submitter Joakim Tjernlund
Date March 23, 2009, 10:17 a.m.
Message ID <OF2EE08546.DF677136-ONC1257582.0037C87E-C1257582.00388BCE@transmode.se>
Download mbox | patch
Permalink /patch/24911/
State Accepted, archived
Delegated to: Kumar Gala
Headers show

Comments

Joakim Tjernlund - March 23, 2009, 10:17 a.m.
Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
---

Sorry for the WS damaged patch, but my current company mailer cannot
handle inline patches. Due to this we are setting up a new mail system
but it will be a few days before this is ready.
Therefore I am attaching this patch as well, use that one
to apply instaed of the inline variant.


 drivers/net/ucc_geth.c |   28 +++++++++++++++++++---------
 1 files changed, 19 insertions(+), 9 deletions(-)

 {
        struct device *device = &ofdev->dev;
@@ -3715,19 +3733,11 @@ static int ucc_geth_probe(struct of_device* ofdev, 
const struct of_device_id *ma
 
        /* Fill in the dev structure */
        uec_set_ethtool_ops(dev);
-       dev->open = ucc_geth_open;
-       dev->hard_start_xmit = ucc_geth_start_xmit;
-       dev->tx_timeout = ucc_geth_timeout;
+       dev->netdev_ops = &ucc_geth_netdev_ops;
        dev->watchdog_timeo = TX_TIMEOUT;
        INIT_WORK(&ugeth->timeout_work, ucc_geth_timeout_work);
        netif_napi_add(dev, &ugeth->napi, ucc_geth_poll, 
UCC_GETH_DEV_WEIGHT);
-#ifdef CONFIG_NET_POLL_CONTROLLER
-       dev->poll_controller = ucc_netpoll;
-#endif
-       dev->stop = ucc_geth_close;
-//    dev->change_mtu = ucc_geth_change_mtu;
        dev->mtu = 1500;
-       dev->set_multicast_list = ucc_geth_set_multi;
 
        ugeth->msg_enable = netif_msg_init(debug.msg_enable, 
UGETH_MSG_DEFAULT);
        ugeth->phy_interface = phy_interface;
David Miller - March 23, 2009, 6:49 p.m.
From: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
Date: Mon, 23 Mar 2009 11:17:39 +0100

> Sorry for the WS damaged patch, but my current company mailer cannot
> handle inline patches. Due to this we are setting up a new mail system
> but it will be a few days before this is ready.
> Therefore I am attaching this patch as well, use that one
> to apply instaed of the inline variant.

This patch has other problems, anyways.

> +#if 0
> +/* not yet */
> +       .ndo_set_mac_address    = ucc_geth_set_mac_address,
> +       .ndo_change_mtu         = ucc_geth_change_mtu,
> +       .ndo_do_ioctl           = ucc_geth_do_ioctl,
> +#endif

Don't do stuff like this, please.  Even the code you are
copying from doesn't have the set_mac_address method
assignment proceted by #if 0 stuff.

Just get rid of all the unused stuff completely.
Joakim Tjernlund - March 24, 2009, 10:37 a.m.
David Miller <davem@davemloft.net> wrote on 23/03/2009 19:49:28:
> 
> From: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
> Date: Mon, 23 Mar 2009 11:17:39 +0100
> 
> > Sorry for the WS damaged patch, but my current company mailer cannot
> > handle inline patches. Due to this we are setting up a new mail system
> > but it will be a few days before this is ready.
> > Therefore I am attaching this patch as well, use that one
> > to apply instaed of the inline variant.
> 
> This patch has other problems, anyways.
> 
> > +#if 0
> > +/* not yet */
> > +       .ndo_set_mac_address    = ucc_geth_set_mac_address,
> > +       .ndo_change_mtu         = ucc_geth_change_mtu,
> > +       .ndo_do_ioctl           = ucc_geth_do_ioctl,
> > +#endif
> 
> Don't do stuff like this, please.  Even the code you are
> copying from doesn't have the set_mac_address method
> assignment proceted by #if 0 stuff.
> 
> Just get rid of all the unused stuff completely.

OK, here is a new version. The #if 0 ..#endif has been removed.


 Jocke
David Miller - March 24, 2009, 9:22 p.m.
From: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
Date: Tue, 24 Mar 2009 11:37:11 +0100

> David Miller <davem@davemloft.net> wrote on 23/03/2009 19:49:28:
> > 
> > From: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
> > Date: Mon, 23 Mar 2009 11:17:39 +0100
> > 
> > > Sorry for the WS damaged patch, but my current company mailer cannot
> > > handle inline patches. Due to this we are setting up a new mail system
> > > but it will be a few days before this is ready.
> > > Therefore I am attaching this patch as well, use that one
> > > to apply instaed of the inline variant.
> > 
> > This patch has other problems, anyways.
> > 
> > > +#if 0
> > > +/* not yet */
> > > +       .ndo_set_mac_address    = ucc_geth_set_mac_address,
> > > +       .ndo_change_mtu         = ucc_geth_change_mtu,
> > > +       .ndo_do_ioctl           = ucc_geth_do_ioctl,
> > > +#endif
> > 
> > Don't do stuff like this, please.  Even the code you are
> > copying from doesn't have the set_mac_address method
> > assignment proceted by #if 0 stuff.
> > 
> > Just get rid of all the unused stuff completely.
> 
> OK, here is a new version. The #if 0 ..#endif has been removed.

Missing ".ndo_set_mac_address = eth_set_mac_address," and
also ".ndo_change_mtu = eth_change_mtu".

Also please inline your patches so that people can quote them in
replies.
Joakim Tjernlund - March 24, 2009, 10:45 p.m.
David Miller <davem@davemloft.net> wrote on 24/03/2009 22:22:09:
> 
> From: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
> Date: Tue, 24 Mar 2009 11:37:11 +0100
> 
> > David Miller <davem@davemloft.net> wrote on 23/03/2009 19:49:28:
> > > 
> > > From: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
> > > Date: Mon, 23 Mar 2009 11:17:39 +0100
> > > 
> > > > Sorry for the WS damaged patch, but my current company mailer 
cannot
> > > > handle inline patches. Due to this we are setting up a new mail 
system
> > > > but it will be a few days before this is ready.
> > > > Therefore I am attaching this patch as well, use that one
> > > > to apply instaed of the inline variant.
> > > 
> > > This patch has other problems, anyways.
> > > 
> > > > +#if 0
> > > > +/* not yet */
> > > > +       .ndo_set_mac_address    = ucc_geth_set_mac_address,
> > > > +       .ndo_change_mtu         = ucc_geth_change_mtu,
> > > > +       .ndo_do_ioctl           = ucc_geth_do_ioctl,
> > > > +#endif
> > > 
> > > Don't do stuff like this, please.  Even the code you are
> > > copying from doesn't have the set_mac_address method
> > > assignment proceted by #if 0 stuff.
> > > 
> > > Just get rid of all the unused stuff completely.
> > 
> > OK, here is a new version. The #if 0 ..#endif has been removed.
> 
> Missing ".ndo_set_mac_address = eth_set_mac_address," and
> also ".ndo_change_mtu = eth_change_mtu".

I don't follow. Are these mandatory now? They were not in the old
impl. either.
> 
> Also please inline your patches so that people can quote them in
> replies.

I know, but my mail system is screwing up WS ATM so I can't. Working
on it though.
David Miller - March 24, 2009, 10:49 p.m.
From: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
Date: Tue, 24 Mar 2009 23:45:13 +0100

> I don't follow. Are these mandatory now? They were not in the old
> impl. either.

See ether_setup() which is called indirectly via alloc_etherdev().

Yawn...

Patch

diff --git a/drivers/net/ucc_geth.c b/drivers/net/ucc_geth.c
index 12e5c3d..d3f6eee 100644
--- a/drivers/net/ucc_geth.c
+++ b/drivers/net/ucc_geth.c
@@ -3503,6 +3503,24 @@  static phy_interface_t to_phy_interface(const char 
*phy_connection_type)
        return PHY_INTERFACE_MODE_MII;
 }
 
+static const struct net_device_ops ucc_geth_netdev_ops = {
+       .ndo_open               = ucc_geth_open,
+       .ndo_stop               = ucc_geth_close,
+       .ndo_start_xmit         = ucc_geth_start_xmit,
+       .ndo_validate_addr      = eth_validate_addr,
+       .ndo_set_multicast_list = ucc_geth_set_multi,
+#if 0
+/* not yet */
+       .ndo_set_mac_address    = ucc_geth_set_mac_address,
+       .ndo_change_mtu         = ucc_geth_change_mtu,
+       .ndo_do_ioctl           = ucc_geth_do_ioctl,
+#endif
+       .ndo_tx_timeout         = ucc_geth_timeout,
+#ifdef CONFIG_NET_POLL_CONTROLLER
+       .ndo_poll_controller    = ucc_netpoll,
+#endif
+};
+
 static int ucc_geth_probe(struct of_device* ofdev, const struct 
of_device_id *match)