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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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:
+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

+
+	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


+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.
+
+static inline u64 get_rwqe_addr(u64 tmp_addr)
+{
+	return tmp_addr;
+}

same here.

removed


+ ehea_poll()

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


done

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux