Re: [2.6.19 PATCH 4/7] ehea: ethtool interface

On Fri, Aug 18, 2006 at 01:33:22PM +0200, Jan-Bernd Themann wrote:

useless assignment;

+	struct ehea_port *port = netdev_priv(dev);
+	struct ehea_adapter *adapter = port->adapter;
+	struct hcp_query_ehea_port_cb_4 *cb4 = NULL;
+
+	cb4 = kzalloc(H_CB_ALIGNMENT, GFP_KERNEL);
+	if(!cb4) {
+		ehea_error("no mem for cb4");
+		goto get_settings_exit;
+	}
+
+	hret = ehea_h_query_ehea_port(adapter->handle,
+				      port->logical_port_id,
+				      H_PORT_CB4,
+				      H_PORT_CB4_ALL,
+				      cb4);

+static void netdev_get_drvinfo(struct net_device *dev,
+			       struct ethtool_drvinfo *info)
+{
+	strncpy(info->driver, DRV_NAME, sizeof(info->driver) - 1);
+	strncpy(info->version, DRV_VERSION, sizeof(info->version) - 1);

Use strlcpy() to not forget -1 accidently.

+static u32 netdev_get_msglevel(struct net_device *dev)

			 ^^^^^^^^

+{
+	struct ehea_port *port = netdev_priv(dev);
+	return port->msg_enable;

			 ^^^^^^

Something is mis-named here.

+}
+
+static void netdev_set_msglevel(struct net_device *dev, u32 value)
+{
+	struct ehea_port *port = netdev_priv(dev);
+	port->msg_enable = value;
+}

And here.

+static void netdev_get_ethtool_stats(struct net_device *dev,
+				     struct ethtool_stats *stats, u64 *data)
+{
+	int i = 0;
+	u64 hret = H_HARDWARE;

Useless assignment.

+	struct ehea_port *port = netdev_priv(dev);
+	struct ehea_adapter *adapter = port->adapter;
+	struct ehea_port_res *pr = &port->port_res[0];
+	struct port_state *p_state = &pr->p_state;
+	struct hcp_query_ehea_port_cb_6 *cb6 = NULL;

Ditto.

+	cb6 = kzalloc(H_CB_ALIGNMENT, GFP_KERNEL);
+	if(!cb6) {
+		ehea_error("no mem for cb6");
+		goto stats_exit;
+	}
+
+	hret = ehea_h_query_ehea_port(adapter->handle,
+				      port->logical_port_id,
+				      H_PORT_CB6,
+				      H_PORT_CB6_ALL,
+				      cb6);

+struct ethtool_ops ehea_ethtool_ops = {
+	.get_settings = netdev_get_settings,
+	.get_drvinfo = netdev_get_drvinfo,
+	.get_msglevel = netdev_get_msglevel,
+	.set_msglevel = netdev_set_msglevel,
+        .get_link = ethtool_op_get_link,
+        .get_tx_csum = ethtool_op_get_tx_csum,

Whitespace breakage.

+	.set_tx_csum = ethtool_op_set_tx_csum,
+	.get_sg = ethtool_op_get_sg,
+	.set_sg = ethtool_op_set_sg,
+	.get_tso = ethtool_op_get_tso,
+	.set_tso = ethtool_op_set_tso,
+	.get_strings = netdev_get_strings,
+	.get_stats_count = netdev_get_stats_count,
+	.get_ethtool_stats = netdev_get_ethtool_stats,

+	.set_settings = NULL,
+	.nway_reset = NULL,
+	.get_pauseparam = NULL,
+	.set_pauseparam = NULL,
+	.get_rx_csum = NULL,
+	.set_rx_csum = NULL,
+	.phys_id = NULL,
+	.self_test = NULL,
+	.self_test_count = NULL

If you don't use them, don't mention them at all. Compiler will DTRT.