Message ID | OF4B79F4DE.4DC6DEA7-ONC1257583.007ECBEA-C1257583.007F2805@transmode.se (mailing list archive) |
---|---|
State | Accepted, archived |
Headers | show |
From: Joakim Tjernlund <Joakim.Tjernlund@transmode.se> Date: Wed, 25 Mar 2009 00:08:53 +0100 > David Miller <davem@davemloft.net> wrote on 24/03/2009 23:49:02: > > > > 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... > > Same here, getting tiered. Should have seen that. Here goes, > attatched as well as explained earlier. I'll apply this, thanks. Please provide a proper "Signed-off-by:" line with future patch submissions, thanks.
David Miller <davem@davemloft.net> wrote on 25/03/2009 00:29:13: > From: Joakim Tjernlund <Joakim.Tjernlund@transmode.se> > Date: Wed, 25 Mar 2009 00:08:53 +0100 > > > David Miller <davem@davemloft.net> wrote on 24/03/2009 23:49:02: > > > > > > 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... > > > > Same here, getting tiered. Should have seen that. Here goes, > > attatched as well as explained earlier. > > I'll apply this, thanks. Thanks. > > Please provide a proper "Signed-off-by:" line with future > patch submissions, thanks. Yep, getting tiered ... Hopefully I will have something more coming your way the next few days.
On Tue, Mar 24, 2009 at 6:08 PM, Joakim Tjernlund
<Joakim.Tjernlund@transmode.se> wrote:
> + .ndo_change_mtu = eth_change_mtu,
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@gmail.com wrote on 27/03/2009 19:50:30: > > On Tue, Mar 24, 2009 at 6:08 PM, Joakim Tjernlund > <Joakim.Tjernlund@transmode.se> wrote: > > > + .ndo_change_mtu = eth_change_mtu, > > 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. 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
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)