quoted

+
+static u32 lio_get_link(struct net_device *dev)
+{
+	u32 ret;
+
+	ret = netif_carrier_ok(dev) ? 1 : 0;
+	return ret;
+}
+

This is unnecessary, this is what the default ethtool handler already does
if you give a NULL handler.

Okay

quoted

+	if (linfo->link.s.status) {
+		ecmd->speed = linfo->link.s.speed;
+		ecmd->duplex = linfo->link.s.duplex;
+	} else {
+		ecmd->speed = -1;
+		ecmd->duplex = -1;
+	}
+

You should SPEED_UNKNOWN and DUPLEX_UNKNOWN (not -1).
and don't set ecmd->speed directly, use
     ethtool_cmd_speed_sed(ecmd, speed)

Okay

quoted

+static void
+lio_get_ethtool_stats(struct net_device *netdev,
+		      struct ethtool_stats *stats, u64 *data)
+{
+	struct lio *lio = GET_LIO(netdev);
+	struct octeon_device *oct_dev = lio->oct_dev;
+	int i = 0, j;
+	
+	data[i++] = jiffies;
+	data[i++] = lio->stats.tx_packets;
+	data[i++] = lio->stats.tx_bytes;
+	data[i++] = lio->stats.rx_packets;
+	data[i++] = lio->stats.rx_bytes;
+	data[i++] = lio->stats.tx_errors;
+	data[i++] = lio->stats.tx_dropped;
+	data[i++] = lio->stats.rx_dropped;
+

Do not mirror existing netdevice statistics in ethtool.
The purpose of ethtool stats is to provide device specific information.

Okay.

quoted

+/** \brief Network device get stats
+ * @param netdev    pointer to network device
+ * @returns pointer stats structure
+ */
+static struct net_device_stats *liquidio_stats(struct net_device *netdev)
+{
+	return &(GET_LIO(netdev)->stats);
+}
+

Unnecessary function, this is what network core does already
if .ndo_get_stats is NULL;

Okay.

quoted

+static int liquidio_ioctl(struct net_device *netdev, struct ifreq *ifr, int

cmd)

quoted

+{
+	switch (cmd) {
+	case SIOCSHWTSTAMP:
+		return hwtstamp_ioctl(netdev, ifr, cmd);
+	default:
+		return -EINVAL

Standard error code for unsupported ioctl is -EOPNOTSUPP

Okay.

quoted

+static int __init liquidio_init(void)
+{
+	int i;
+	struct handshake *hs;
+
+	pr_info("LiquidIO: Starting Network module version %s\n",
+		LIQUIDIO_VERSION);
+

Do you really need to log this. Systems are already too chatty.

Okay. This is available with ethtool anyway.

quoted

+/**
+ * \brief Alloc net device
+ * @param size Size to allocate
+ * @param nq how many queues
+ * @returns pointer to net device structure
+ */	
+static inline struct net_device *liquidio_alloc_netdev(int size, int nq)
+{
+	if (nq > 1)
+		return alloc_netdev_mq(size, "lio%d",

NET_NAME_UNKNOWN,

quoted

+				       ether_setup, nq);
+	else
+		return alloc_netdev(size, "lio%d", NET_NAME_UNKNOWN,
+				    ether_setup);
+}

There is no need for the (nq > 1) test, just do:

quoted

+		return alloc_netdev_mq(size, "lio%d",

NET_NAME_UNKNOWN,

quoted

+				       ether_setup, nq);

Okay.

Also, why does this device not have standard ethernet naming?

No good reason. We will follow the p<slot_number>p<port_number> convention.

quoted

+void process_noresponse_list(struct octeon_device *oct,
+			     struct octeon_instr_queue *iq)
+{
+	uint32_t get_idx;
+	struct octeon_soft_command *sc;
+	struct octeon_instr_ih *ih;

All global function names must start with same prefix, driver may not
always be built as a module.

In this case oceteon_process_noresponse_list.

Okay. All global symbols will get an octeon_ or lio_ prefix.

We'll reply to the last couple issues separately.

Thanks for all the feedback.