Home » Mailing list archives » linux-netdev » 2009 » March » 24

Re: [PATCH] ucc_geth: Convert to net_device_ops

Previous thread: [PATCH] netlink: add NETLINK_NO_ENOBUFS socket flag by Pablo Neira Ayuso on Monday, March 23, 2009 - 2:33 am. (8 messages)

Next thread: ucc_geth: nf_conntrack: table full, dropping packet. by Joakim Tjernlund on Monday, March 23, 2009 - 3:42 am. (40 messages)
[view original message]
From: Joakim Tjernlund
Subject: [PATCH] ucc_geth: Convert to net_device_ops
Date: Monday, March 23, 2009 - 3:17 am

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(-)

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)
 {
        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;
        ...
[ message continues ]
[view original message]
From: David Miller
Subject: Re: [PATCH] ucc_geth: Convert to net_device_ops
Date: Monday, March 23, 2009 - 11:49 am

From: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>


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.

--


[ message continues ]
[view original message]
From: Joakim Tjernlund
Subject: Re: [PATCH] ucc_geth: Convert to net_device_ops
Date: Tuesday, March 24, 2009 - 3:37 am

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


 Jocke

[ message continues ]
[view original message]
From: David Miller
Subject: Re: [PATCH] ucc_geth: Convert to net_device_ops
Date: Tuesday, March 24, 2009 - 2:22 pm

From: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>

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.

--


[ message continues ]
[view original message]
From: Joakim Tjernlund
Subject: Re: [PATCH] ucc_geth: Convert to net_device_ops
Date: Tuesday, March 24, 2009 - 3:45 pm

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

I know, but my mail system is screwing up WS ATM so I can't. Working
on it though.
--


[ message continues ]
[view original message]
From: David Miller
Subject: Re: [PATCH] ucc_geth: Convert to net_device_ops
Date: Tuesday, March 24, 2009 - 3:49 pm

From: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>

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

Yawn...
--


[ message continues ]
[view original message]
From: Joakim Tjernlund
Subject: Re: [PATCH] ucc_geth: Convert to net_device_ops
Date: Tuesday, March 24, 2009 - 4:08 pm

Same here, getting tiered. Should have seen that. Here goes,
attatched as well as explained earlier.

    Jocke
From 651fcd7d1ba494f2e2f20da51196b1b3dccdcd68 Mon Sep 17 00:00:00 2001
From: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
Date: Fri, 20 Mar 2009 21:09:14 +0100
Subject: [PATCH] ucc_geth: Convert to net_device_ops

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

diff --git a/drivers/net/ucc_geth.c b/drivers/net/ucc_geth.c
index 7716239..097aed8 100644
--- a/drivers/net/ucc_geth.c
+++ b/drivers/net/ucc_geth.c
@@ -3501,6 +3501,20 @@ 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_mac_address    = eth_mac_addr,
+       .ndo_change_mtu         = eth_change_mtu,
+       .ndo_set_multicast_list = ucc_geth_set_multi,
+       .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)
 {
        struct device *device = &ofdev->dev;
@@ -3716,19 +3730,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 ...
[ message continues ]
[view original message]
From: David Miller
Subject: Re: [PATCH] ucc_geth: Convert to net_device_ops
Date: Tuesday, March 24, 2009 - 4:29 pm

From: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>

I'll apply this, thanks.

Please provide a proper "Signed-off-by:" line with future
patch submissions, thanks.
--


[ message continues ]
[view original message]
From: Joakim Tjernlund
Subject: Re: [PATCH] ucc_geth: Convert to net_device_ops
Date: Tuesday, March 24, 2009 - 4:35 pm

Yep, getting tiered ...
Hopefully I will have something more coming your way the next few days.


--


[ message continues ]
[view original message]
From: Timur Tabi
Subject: Re: [PATCH] ucc_geth: Convert to net_device_ops
Date: Friday, March 27, 2009 - 11:50 am

On Tue, Mar 24, 2009 at 6:08 PM, Joakim Tjernlund

I have an old patch for change_mtu support for ucc_geth which I've
been meaning to push upstream.  This patch uses this function instead
of eth_change_mtu():

+static int ucc_geth_change_mtu(struct net_device *dev, int new_mtu)
+{
+	int max_rx_buf_length;
+	struct ucc_geth_private *ugeth = netdev_priv(dev);
+	int frame_size = new_mtu + 18;
+	struct ucc_geth_info *ug_info;
+	struct ucc_fast_info *uf_info;
+
+	if (ugeth->p_rx_glbl_pram) {
+		printk("%s: Interface is active!\n", __FUNCTION__);
+	} else {
+
+		ug_info = ugeth->ug_info;
+		uf_info = &ug_info->uf_info;
+
+		if ((frame_size < 64) || (frame_size > JUMBO_FRAME_SIZE)) {
+			printk(KERN_ERR "%s: Invalid MTU setting\n", dev->name);
+			return -EINVAL;
+		}
+
+		max_rx_buf_length =
+		    (frame_size & ~(UCC_GETH_MRBLR_ALIGNMENT - 1)) +
+		    UCC_GETH_MRBLR_ALIGNMENT;
+
+		ugeth->ug_info->uf_info.max_rx_buf_length = max_rx_buf_length;
+		dev->mtu = new_mtu;
+
+		/* Set the max. rx buffer size (MRBLR) */
+		uf_info->max_rx_buf_length = max_rx_buf_length;
+
+		/* set the max. frame length (MFLR) */
+		ug_info->maxFrameLength = frame_size;
+
+		ugeth_info("%s: MTU = %d (frame size=%d)\n", dev->name,
+		       dev->mtu, frame_size);
+	}
+	return 0;
+}

Do you think this is how it should be done?  I don't know enough about
ucc_geth and MTUs to know off-hand.

-- 
Timur Tabi
Linux kernel developer at Freescale
--


[ message continues ]
[view original message]
From: Joakim Tjernlund
Subject: Re: [PATCH] ucc_geth: Convert to net_device_ops
Date: Saturday, March 28, 2009 - 4:27 am

Not quite so. I don't think you can support JUMBO frames with just one big 
RX buffer.
Also, maxframe & RX buffer should be somewhat bigger than the actual mtu 
so you can
support VLAN(4) and PPPOE(8) = 12 bytes.
You need to add 12 bytes to the default values too.

 Jocke 

--


[ message continues ]
Previous thread: [PATCH] netlink: add NETLINK_NO_ENOBUFS socket flag by Pablo Neira Ayuso on Monday, March 23, 2009 - 2:33 am. (8 messages)

Next thread: ucc_geth: nf_conntrack: table full, dropping packet. by Joakim Tjernlund on Monday, March 23, 2009 - 3:42 am. (40 messages)