Re: [PATCH 1/6] ehea: interface to network stack

Hi Christian,

thanks for your comments, we'll send an updated patch set soon.

Jan-Bernd

Christian Borntraeger wrote:

Hi Jan-Bernd,

I had some minutes, here are some finding after a quick look.

On Wednesday 09 August 2006 10:38, you wrote:

quoted

+static struct net_device_stats *ehea_get_stats(struct net_device *dev)
+{
+	int i;
+	u64 hret = H_HARDWARE;
+	u64 rx_packets = 0;
+	struct ehea_port *port = (struct ehea_port*)dev->priv;

dev->priv is a void pointer, this cast is unnecessary. When we are at it, have 
you considered the netdev_priv macro? This will require some prep in 
alloc_netdev and might not always pe possible. 

good point, we'll use alloc_etherdev / netdev_priv

quoted

+
+	EDEB_DMP(7, (u8*)cb2,
+		 sizeof(struct hcp_query_ehea_port_cb_2), "After HCALL");
+
+	for (i = 0; i < port->num_def_qps; i++) {
+		rx_packets += port->port_res[i].rx_packets;
+	}
+
+	stats->tx_packets = cb2->txucp + cb2->txmcp + cb2->txbcp;
+	stats->multicast = cb2->rxmcp;
+	stats->rx_errors = cb2->rxuerr;
+	stats->rx_bytes = cb2->rxo;
+	stats->tx_bytes = cb2->txo;
+	stats->rx_packets = rx_packets;
+
+get_stat_exit:
+	EDEB_EX(7, "");
+	return stats;
+}

again, cb2 is not freed.
[...]

yep, done

quoted

+static inline u64 get_swqe_addr(u64 tmp_addr, int addr_seg)
+{
+	u64 addr;
+	addr = tmp_addr;
+	return addr;
+}

This is suppsed to change in the future? If not you can get rid of it. 

quoted

+
+static inline u64 get_rwqe_addr(u64 tmp_addr)
+{
+	return tmp_addr;
+}

same here. 

removed

quoted

+ ehea_poll()

The poll function seems too long and therefore hard to review. Please consider 
splitting it. 

done