From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.osdl.org (fire.osdl.org [65.172.181.4]) (using TLSv1 with cipher EDH-RSA-DES-CBC3-SHA (168/168 bits)) (Client CN "smtp.osdl.org", Issuer "OSDL Hostmaster" (not verified)) by ozlabs.org (Postfix) with ESMTP id F28A7679F7 for ; Fri, 3 Jun 2005 08:05:51 +1000 (EST) Date: Thu, 2 Jun 2005 15:05:43 -0700 From: Stephen Hemminger To: Matt Porter Message-ID: <20050602150543.7e4326b6@dxpl.pdx.osdl.net> In-Reply-To: <20050602143404.F24818@cox.net> References: <20050602140359.B24818@cox.net> <20050602141247.C24818@cox.net> <20050602141946.D24818@cox.net> <20050602142509.E24818@cox.net> <20050602143404.F24818@cox.net> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Cc: akpm@osdl.org, netdev@oss.sgi.com, linux-kernel@vger.kernel.org, torvalds@osdl.org, linuxppc-embedded@ozlabs.org, jgarzik@pobox.com Subject: Re: [PATCH][5/5] RapidIO support: net driver over messaging List-Id: Linux on Embedded PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , How much is this like ethernet? does it still do ARP? Can it do promiscious receive? > +LIST_HEAD(rionet_peers); Does this have to be global? Not sure about the locking of this stuff, are you relying on the RTNL? > + > +static int rionet_change_mtu(struct net_device *ndev, int new_mtu) > +{ > + struct rionet_private *rnet = ndev->priv; > + > + if (netif_msg_drv(rnet)) > + printk(KERN_WARNING > + "%s: rionet_change_mtu(): not implemented\n", DRV_NAME); > + > + return 0; > +} If you can allow any mtu then don't need this at all. Or if you are limited then better return an error for bad values. > +static void rionet_set_multicast_list(struct net_device *ndev) > +{ > + struct rionet_private *rnet = ndev->priv; > + > + if (netif_msg_drv(rnet)) > + printk(KERN_WARNING > + "%s: rionet_set_multicast_list(): not implemented\n", > + DRV_NAME); > +} If you can't handle it then just leave dev->set_multicast_list as NULL and all attempts to add or delete will get -EINVAL > + > +static int rionet_open(struct net_device *ndev) > +{ > + /* Initialize inbound message ring */ > + for (i = 0; i < RIONET_RX_RING_SIZE; i++) > + rnet->rx_skb[i] = NULL; > + rnet->rx_slot = 0; > + rionet_rx_fill(ndev, 0); > + > + rnet->tx_slot = 0; > + rnet->tx_cnt = 0; > + rnet->ack_slot = 0; > + > + spin_lock_init(&rnet->lock); > + > + rnet->msg_enable = RIONET_DEFAULT_MSGLEVEL; Better to do all initialization of the per device data in the place it is allocated (rio_setup_netdev) > + > +static int rionet_ioctl(struct net_device *ndev, struct ifreq *rq, int cmd) > +{ > + return -EOPNOTSUPP; > +} Unneeded, if dev->do_ioctl is NULL, then all private ioctl's will return -EINVAL that is what you want. > + > +static u32 rionet_get_link(struct net_device *ndev) > +{ > + return netif_carrier_ok(ndev); > +} Use ethtool_op_get_link > + > +static int rionet_setup_netdev(struct rio_mport *mport) > +{ > + int rc = 0; > + struct net_device *ndev = NULL; > + struct rionet_private *rnet; > + u16 device_id; > + > + /* Allocate our net_device structure */ > + ndev = alloc_etherdev(sizeof(struct rionet_private)); > + if (ndev == NULL) { > + printk(KERN_INFO "%s: could not allocate ethernet device.\n", > + DRV_NAME); > + rc = -ENOMEM; > + goto out; > + } > + > + /* > + * XXX hack, store point a static at ndev so we can get it... > + * Perhaps need an array of these that the handler can > + * index via the mbox number. > + */ > + sndev = ndev; > + > + /* Set up private area */ > + rnet = (struct rionet_private *)ndev->priv; > + rnet->mport = mport; > + > + /* Set the default MAC address */ > + device_id = rio_local_get_device_id(mport); > + ndev->dev_addr[0] = 0x00; > + ndev->dev_addr[1] = 0x01; > + ndev->dev_addr[2] = 0x00; > + ndev->dev_addr[3] = 0x01; > + ndev->dev_addr[4] = device_id >> 8; > + ndev->dev_addr[5] = device_id & 0xff; > + > + /* Fill in the driver function table */ > + ndev->open = &rionet_open; > + ndev->hard_start_xmit = &rionet_start_xmit; > + ndev->stop = &rionet_close; > + ndev->get_stats = &rionet_stats; > + ndev->change_mtu = &rionet_change_mtu; > + ndev->set_mac_address = &rionet_set_mac_address; > + ndev->set_multicast_list = &rionet_set_multicast_list; > + ndev->do_ioctl = &rionet_ioctl; > + SET_ETHTOOL_OPS(ndev, &rionet_ethtool_ops); > + > + ndev->mtu = RIO_MAX_MSG_SIZE - 14; > + > + SET_MODULE_OWNER(ndev); Can you set any ndev->features to get better performance. Can you take >32bit data addresses? then set HIGHDMA You are doing your on locking, can you use LLTX? Does the hardware support scatter gather?