[2.6.19 PATCH 4/7] ehea: ethtool interface
Signed-off-by: Jan-Bernd Themann [EMAIL PROTECTED]
drivers/net/ehea/ehea_ethtool.c | 294
1 file changed, 294 insertions(+)
--- linux-2.6.18-rc6-orig/drivers/net/ehea/ehea_ethtool.c 1970-01-01
01:00:00.0 +0100
+++ kernel/drivers/net/ehea/ehea_ethtool.c 2006-09-06 15:53:43.0
+0200
@@ -0,0 +1,294 @@
+/*
+ * linux/drivers/net/ehea/ehea_ethtool.c
+ *
+ * eHEA ethernet device driver for IBM eServer System p
+ *
+ * (C) Copyright IBM Corp. 2006
+ *
+ * Authors:
+ * Christoph Raisch [EMAIL PROTECTED]
+ * Jan-Bernd Themann [EMAIL PROTECTED]
+ * Thomas Klein [EMAIL PROTECTED]
+ *
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2, or (at your option)
+ * any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#include ehea.h
+#include ehea_phyp.h
+
+static int ehea_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
+{
+ struct ehea_port *port = netdev_priv(dev);
+ int ret;
+
+ ret = ehea_sense_port_attr(port);
+
+ if (ret)
+ return ret;
+
+ if (netif_carrier_ok(dev)) {
+ switch(port-port_speed) {
+ case EHEA_SPEED_10M: cmd-speed = SPEED_10; break;
+ case EHEA_SPEED_100M: cmd-speed = SPEED_100; break;
+ case EHEA_SPEED_1G: cmd-speed = SPEED_1000; break;
+ case EHEA_SPEED_10G: cmd-speed = SPEED_1; break;
+ }
+ cmd-duplex = port-full_duplex == 1 ?
+DUPLEX_FULL : DUPLEX_HALF;
+ } else {
+ cmd-speed = -1;
+ cmd-duplex = -1;
+ }
+
+ cmd-supported = (SUPPORTED_1baseT_Full | SUPPORTED_1000baseT_Full
+ | SUPPORTED_100baseT_Full | SUPPORTED_100baseT_Half
+ | SUPPORTED_10baseT_Full | SUPPORTED_10baseT_Half
+ | SUPPORTED_Autoneg | SUPPORTED_FIBRE);
+
+ cmd-advertising = (ADVERTISED_1baseT_Full | ADVERTISED_Autoneg
+| ADVERTISED_FIBRE);
+
+ cmd-port = PORT_FIBRE;
+ cmd-autoneg = port-autoneg == 1 ? AUTONEG_ENABLE : AUTONEG_DISABLE;
+
+ return 0;
+}
+
+static int ehea_set_settings(struct net_device *dev, struct ethtool_cmd *cmd)
+{
+ struct ehea_port *port = netdev_priv(dev);
+ int ret = 0;
+ u32 sp;
+
+ if (cmd-autoneg == AUTONEG_ENABLE) {
+ sp = EHEA_SPEED_AUTONEG;
+ goto doit;
+ }
+
+ switch(cmd-speed) {
+ case SPEED_10:
+ if (cmd-duplex == DUPLEX_FULL)
+ sp = H_SPEED_10M_F;
+ else
+ sp = H_SPEED_10M_H;
+ break;
+
+ case SPEED_100:
+ if (cmd-duplex == DUPLEX_FULL)
+ sp = H_SPEED_100M_F;
+ else
+ sp = H_SPEED_100M_H;
+ break;
+
+ case SPEED_1000:
+ if (cmd-duplex == DUPLEX_FULL)
+ sp = H_SPEED_1G_F;
+ else
+ ret = -EINVAL;
+ break;
+
+ case SPEED_1:
+ if (cmd-duplex == DUPLEX_FULL)
+ sp = H_SPEED_10G_F;
+ else
+ ret = -EINVAL;
+ break;
+
+ default:
+ ret = -EINVAL;
+ break;
+ }
+
+ if (ret)
+ goto out;
+doit:
+ ret = ehea_set_portspeed(port, sp);
+
+ if (!ret)
+ ehea_info(%s: Port speed succesfully set: %dMbps
+ %s Duplex,
+ port-netdev-name, port-port_speed,
+ port-full_duplex == 1 ? Full : Half);
+out:
+ return ret;
+}
+
+static int ehea_nway_reset(struct net_device *dev)
+{
+ struct ehea_port *port = netdev_priv(dev);
+ int ret;
+
+ ret = ehea_set_portspeed(port, EHEA_SPEED_AUTONEG);
+
+ if (!ret)
+ ehea_info(%s: Port speed succesfully set: %dMbps
+ %s Duplex,
+ port-netdev-name, port-port_speed,
+ port-full_duplex == 1 ? Full : Half);
+ return ret;
+}
+
+static void ehea_get_drvinfo(struct net_device *dev,
+ struct ethtool_drvinfo *info)
+{
+
[2.6.19 PATCH 4/7] ehea: ethtool interface
Signed-off-by: Jan-Bernd Themann [EMAIL PROTECTED]
drivers/net/ehea/ehea_ethtool.c | 244
1 file changed, 244 insertions(+)
--- linux-2.6.18-rc4-git1-orig/drivers/net/ehea/ehea_ethtool.c 1969-12-31
16:00:00.0 -0800
+++ kernel/drivers/net/ehea/ehea_ethtool.c 2006-08-23 02:01:00.363230269
-0700
@@ -0,0 +1,244 @@
+/*
+ * linux/drivers/net/ehea/ehea_ethtool.c
+ *
+ * eHEA ethernet device driver for IBM eServer System p
+ *
+ * (C) Copyright IBM Corp. 2006
+ *
+ * Authors:
+ * Christoph Raisch [EMAIL PROTECTED]
+ * Jan-Bernd Themann [EMAIL PROTECTED]
+ * Thomas Klein [EMAIL PROTECTED]
+ *
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2, or (at your option)
+ * any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#include ehea.h
+#include ehea_phyp.h
+
+
+static int netdev_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
+{
+ u64 hret;
+ struct ehea_port *port = netdev_priv(dev);
+ struct ehea_adapter *adapter = port-adapter;
+ struct hcp_ehea_port_cb4 *cb4;
+
+ cb4 = kzalloc(H_CB_ALIGNMENT, GFP_KERNEL);
+ if (!cb4) {
+ ehea_error(no mem for cb4);
+ return -ENOMEM;
+ }
+
+ hret = ehea_h_query_ehea_port(adapter-handle, port-logical_port_id,
+ H_PORT_CB4, H_PORT_CB4_ALL, cb4);
+ if (hret != H_SUCCESS) {
+ ehea_error(query_ehea_port failed);
+ kfree(cb4);
+ return -EIO;
+ }
+
+ if (netif_msg_hw(port))
+ ehea_dump(cb4, sizeof(*cb4), netdev_get_settings);
+
+ if (netif_carrier_ok(dev)) {
+ switch(cb4-port_speed){
+ case H_PORT_SPEED_10M_H:
+ cmd-speed = SPEED_10;
+ cmd-duplex = DUPLEX_HALF;
+ break;
+ case H_PORT_SPEED_10M_F:
+ cmd-speed = SPEED_10;
+ cmd-duplex = DUPLEX_FULL;
+ break;
+ case H_PORT_SPEED_100M_H:
+ cmd-speed = SPEED_100;
+ cmd-duplex = DUPLEX_HALF;
+ break;
+ case H_PORT_SPEED_100M_F:
+ cmd-speed = SPEED_100;
+ cmd-duplex = DUPLEX_FULL;
+ break;
+ case H_PORT_SPEED_1G_F:
+ cmd-speed = SPEED_1000;
+ cmd-duplex = DUPLEX_FULL;
+ break;
+ case H_PORT_SPEED_10G_F:
+ cmd-speed = SPEED_1;
+ cmd-duplex = DUPLEX_FULL;
+ break;
+ }
+ } else {
+ cmd-speed = -1;
+ cmd-duplex = -1;
+ }
+
+ cmd-supported = (SUPPORTED_1baseT_Full | SUPPORTED_1000baseT_Full
+ | SUPPORTED_100baseT_Full | SUPPORTED_100baseT_Half
+ | SUPPORTED_10baseT_Full | SUPPORTED_10baseT_Half
+ | SUPPORTED_Autoneg | SUPPORTED_FIBRE);
+
+ cmd-advertising = (ADVERTISED_1baseT_Full | ADVERTISED_Autoneg
+| ADVERTISED_FIBRE);
+
+ cmd-port = PORT_FIBRE;
+ cmd-autoneg = AUTONEG_ENABLE;
+
+ kfree(cb4);
+ return 0;
+}
+
+static void netdev_get_drvinfo(struct net_device *dev,
+ struct ethtool_drvinfo *info)
+{
+ strlcpy(info-driver, DRV_NAME, sizeof(info-driver) - 1);
+ strlcpy(info-version, DRV_VERSION, sizeof(info-version) - 1);
+}
+
+static u32 netdev_get_msglevel(struct net_device *dev)
+{
+ struct ehea_port *port = netdev_priv(dev);
+ return port-msg_enable;
+}
+
+static void netdev_set_msglevel(struct net_device *dev, u32 value)
+{
+ struct ehea_port *port = netdev_priv(dev);
+ port-msg_enable = value;
+}
+
+static char ehea_ethtool_stats_keys[][ETH_GSTRING_LEN] = {
+ {poll_max_processed},
+ {queue_stopped},
+ {min_swqe_avail},
+ {poll_receive_err},
+ {pkt_send},
+ {pkt_xmit},
+ {send_tasklet},
+ {ehea_poll},
+ {nwqe},
+ {swqe_available_0},
+ {sig_comp_iv},
+ {rxo},
+ {rx64},
+ {rx65},
+ {rx128},
+ {rx256},
+ {rx512},
+ {rx1024},
+ {txo},
+
[2.6.19 PATCH 4/7] ehea: ethtool interface
Signed-off-by: Jan-Bernd Themann [EMAIL PROTECTED]
drivers/net/ehea/ehea_ethtool.c | 244
1 file changed, 244 insertions(+)
--- linux-2.6.18-rc4-git1-orig/drivers/net/ehea/ehea_ethtool.c 1969-12-31
16:00:00.0 -0800
+++ kernel/drivers/net/ehea/ehea_ethtool.c 2006-08-22 06:05:29.197373636
-0700
@@ -0,0 +1,244 @@
+/*
+ * linux/drivers/net/ehea/ehea_ethtool.c
+ *
+ * eHEA ethernet device driver for IBM eServer System p
+ *
+ * (C) Copyright IBM Corp. 2006
+ *
+ * Authors:
+ * Christoph Raisch [EMAIL PROTECTED]
+ * Jan-Bernd Themann [EMAIL PROTECTED]
+ * Thomas Klein [EMAIL PROTECTED]
+ *
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2, or (at your option)
+ * any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#include ehea.h
+#include ehea_phyp.h
+
+
+static int netdev_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
+{
+ u64 hret;
+ struct ehea_port *port = netdev_priv(dev);
+ struct ehea_adapter *adapter = port-adapter;
+ struct hcp_ehea_port_cb4 *cb4;
+
+ cb4 = kzalloc(H_CB_ALIGNMENT, GFP_KERNEL);
+ if (!cb4) {
+ ehea_error(no mem for cb4);
+ return -ENOMEM;
+ }
+
+ hret = ehea_h_query_ehea_port(adapter-handle, port-logical_port_id,
+ H_PORT_CB4, H_PORT_CB4_ALL, cb4);
+ if (hret != H_SUCCESS) {
+ ehea_error(query_ehea_port failed);
+ kfree(cb4);
+ return -EIO;
+ }
+
+ if (netif_msg_hw(port))
+ ehea_dump(cb4, sizeof(*cb4), netdev_get_settings);
+
+ if (netif_carrier_ok(dev)) {
+ switch(cb4-port_speed){
+ case H_PORT_SPEED_10M_H:
+ cmd-speed = SPEED_10;
+ cmd-duplex = DUPLEX_HALF;
+ break;
+ case H_PORT_SPEED_10M_F:
+ cmd-speed = SPEED_10;
+ cmd-duplex = DUPLEX_FULL;
+ break;
+ case H_PORT_SPEED_100M_H:
+ cmd-speed = SPEED_100;
+ cmd-duplex = DUPLEX_HALF;
+ break;
+ case H_PORT_SPEED_100M_F:
+ cmd-speed = SPEED_100;
+ cmd-duplex = DUPLEX_FULL;
+ break;
+ case H_PORT_SPEED_1G_F:
+ cmd-speed = SPEED_1000;
+ cmd-duplex = DUPLEX_FULL;
+ break;
+ case H_PORT_SPEED_10G_F:
+ cmd-speed = SPEED_1;
+ cmd-duplex = DUPLEX_FULL;
+ break;
+ }
+ } else {
+ cmd-speed = -1;
+ cmd-duplex = -1;
+ }
+
+ cmd-supported = (SUPPORTED_1baseT_Full | SUPPORTED_1000baseT_Full
+ | SUPPORTED_100baseT_Full | SUPPORTED_100baseT_Half
+ | SUPPORTED_10baseT_Full | SUPPORTED_10baseT_Half
+ | SUPPORTED_Autoneg | SUPPORTED_FIBRE);
+
+ cmd-advertising = (ADVERTISED_1baseT_Full | ADVERTISED_Autoneg
+| ADVERTISED_FIBRE);
+
+ cmd-port = PORT_FIBRE;
+ cmd-autoneg = AUTONEG_ENABLE;
+
+ kfree(cb4);
+ return 0;
+}
+
+static void netdev_get_drvinfo(struct net_device *dev,
+ struct ethtool_drvinfo *info)
+{
+ strlcpy(info-driver, DRV_NAME, sizeof(info-driver) - 1);
+ strlcpy(info-version, DRV_VERSION, sizeof(info-version) - 1);
+}
+
+static u32 netdev_get_msglevel(struct net_device *dev)
+{
+ struct ehea_port *port = netdev_priv(dev);
+ return port-msg_enable;
+}
+
+static void netdev_set_msglevel(struct net_device *dev, u32 value)
+{
+ struct ehea_port *port = netdev_priv(dev);
+ port-msg_enable = value;
+}
+
+static char ehea_ethtool_stats_keys[][ETH_GSTRING_LEN] = {
+ {poll_max_processed},
+ {queue_stopped},
+ {min_swqe_avail},
+ {poll_receive_err},
+ {pkt_send},
+ {pkt_xmit},
+ {send_tasklet},
+ {ehea_poll},
+ {nwqe},
+ {swqe_available_0},
+ {sig_comp_iv},
+ {rxo},
+ {rx64},
+ {rx65},
+ {rx128},
+ {rx256},
+ {rx512},
+ {rx1024},
+ {txo},
+
Re: [2.6.19 PATCH 4/7] ehea: ethtool interface
Stephen Hemminger wrote: On Fri, 18 Aug 2006 17:41:26 +0200 Thomas Klein [EMAIL PROTECTED] wrote: Hi Alexey, first of all thanks a lot for the extensive review. Alexey Dobriyan wrote: + u64 hret = H_HARDWARE; Useless assignment here and everywhere. Initializing returncodes to errorstate is a cheap way to prevent accidentally returning (uninitalized) success returncodes which can lead to catastrophic misbehaviour. That is old thinking. Current compilers do live/dead analysis and tell you about this at compile time which is better than relying on default behavior at runtime. Understood. I reworked the returncode handling and removed the unnecessary initializations. Thanks for pointing this out. Thomas - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [2.6.19 PATCH 4/7] ehea: ethtool interface
Alexey Dobriyan wrote:
On Fri, Aug 18, 2006 at 01:33:22PM +0200, Jan-Bernd Themann wrote:
--- linux-2.6.18-rc4-orig/drivers/net/ehea/ehea_ethtool.c
+++ kernel/drivers/net/ehea/ehea_ethtool.c
+{
+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.
Done.
+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.
The kernel requires a structure member of that name.
+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.
Fixed.
+.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.
Agreed. Assignments removed.
Thanks again :-)
Thomas
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [2.6.19 PATCH 4/7] ehea: ethtool interface
On Fri, 2006-08-18 at 17:41 +0200, Thomas Klein wrote: Hi Alexey, first of all thanks a lot for the extensive review. Alexey Dobriyan wrote: + u64 hret = H_HARDWARE; Useless assignment here and everywhere. Initializing returncodes to errorstate is a cheap way to prevent accidentally returning (uninitalized) success returncodes which can lead to catastrophic misbehaviour. If you try to return an uninitialized value the compiler will warn you, you'll then look at the code and realise you missed a case, you might save yourself a bug. By unconditionally initialising you are lying to the compiler, and it can no longer help you. cheers -- Michael Ellerman IBM OzLabs wwweb: http://michael.ellerman.id.au phone: +61 2 6212 1183 (tie line 70 21183) We do not inherit the earth from our ancestors, we borrow it from our children. - S.M.A.R.T Person signature.asc Description: This is a digitally signed message part
Re: [2.6.19 PATCH 4/7] ehea: ethtool interface
On Sat, 2006-08-19 at 16:18 +1000, Michael Ellerman wrote: If you try to return an uninitialized value the compiler will warn you, you'll then look at the code and realise you missed a case, you might save yourself a bug. You *should* look at the code :) So should we be reporting these as bugs? [EMAIL PROTECTED]:~/linux/linux-2.6.17.6$ script make.script Script started, file is make.script [EMAIL PROTECTED]:~/linux/linux-2.6.17.6$ make ... Script done, file is make.script [EMAIL PROTECTED]:~/linux/linux-2.6.17.6$ fgrep warning make.script arch/i386/kernel/cpu/transmeta.c:12: warning: 'cpu_freq' may be used uninitialized in this function fs/bio.c:169: warning: 'idx' may be used uninitialized in this function fs/eventpoll.c:500: warning: 'fd' may be used uninitialized in this function fs/isofs/namei.c:162: warning: 'offset' may be used uninitialized in this function fs/isofs/namei.c:162: warning: 'block' may be used uninitialized in this function fs/nfsd/nfsctl.c:292: warning: 'maxsize' may be used uninitialized in this function fs/udf/balloc.c:751: warning: 'goal_eloc.logicalBlockNum' may be used uninitialized in this function fs/udf/super.c:1358: warning: 'ino.partitionReferenceNum' may be used uninitialized in this function fs/xfs/xfs_alloc_btree.c:611: warning: 'nkey.ar_startblock' may be used uninitialized in this function fs/xfs/xfs_alloc_btree.c:611: warning: 'nkey.ar_blockcount' may be used uninitialized in this function fs/xfs/xfs_bmap.c:2498: warning: 'rtx' is used uninitialized in this function fs/xfs/xfs_bmap_btree.c:753: warning: 'nkey.br_startoff' may be used uninitialized in this function fs/xfs/xfs_da_btree.c:151: warning: 'action' may be used uninitialized in this function fs/xfs/xfs_dir.c:363: warning: 'totallen' may be used uninitialized in this function fs/xfs/xfs_dir.c:363: warning: 'count' may be used uninitialized in this function fs/xfs/xfs_ialloc_btree.c:545: warning: 'nkey.ir_startino' may be used uninitialized in this function fs/xfs/xfs_inode.c:1958: warning: 'last_dip' may be used uninitialized in this function fs/xfs/xfs_inode.c:1960: warning: 'last_offset' may be used uninitialized in this function fs/xfs/xfs_log.c:1749: warning: 'iclog' may be used uninitialized in this function fs/xfs/xfs_log_recover.c:523: warning: 'first_blk' may be used uninitialized in this function ipc/msg.c:338: warning: 'setbuf.qbytes' may be used uninitialized in this function ipc/msg.c:338: warning: 'setbuf.uid' may be used uninitialized in this function ipc/msg.c:338: warning: 'setbuf.gid' may be used uninitialized in this function ipc/msg.c:338: warning: 'setbuf.mode' may be used uninitialized in this function ipc/sem.c:810: warning: 'setbuf.uid' may be used uninitialized in this function ipc/sem.c:810: warning: 'setbuf.gid' may be used uninitialized in this function ipc/sem.c:810: warning: 'setbuf.mode' may be used uninitialized in this function drivers/md/dm-table.c:431: warning: 'dev' may be used uninitialized in this function drivers/md/dm-ioctl.c:1388: warning: 'param' may be used uninitialized in this function net/sched/sch_cbq.c:409: warning: 'ret' may be used uninitialized in this function lib/zlib_inflate/inftrees.c:121: warning: 'r.base' may be used uninitialized in this function - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [2.6.19 PATCH 4/7] ehea: ethtool interface
On Sat, 2006-08-19 at 02:48 -0400, Andy Gay wrote:
On Sat, 2006-08-19 at 16:18 +1000, Michael Ellerman wrote:
If you try to return an uninitialized value the compiler will warn you,
you'll then look at the code and realise you missed a case, you might
save yourself a bug.
You *should* look at the code :)
So should we be reporting these as bugs?
No you're better off sending patches ;)
A lot of these have started appearing recently, which I think is due to
GCC becoming more vocal. Unfortunately many of them are false positives
caused by GCC not seeming to grok that this is ok:
void foo(int *x) { *x = 1; }
...
int x;
foo(x);
return x;
It's a pity because it creates noise, but still it's beside the point.
New code going into the kernel should be 100% warning free, and so if
the eHEA guys had missed an error case they'd spot the warning before
they submitted it.
Doing the initialise-to-some-value trick means you only spot the bug
via testing.
cheers
--
Michael Ellerman
IBM OzLabs
wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)
We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person
signature.asc
Description: This is a digitally signed message part
Re: [2.6.19 PATCH 4/7] ehea: ethtool interface
On Saturday 19 August 2006 10:41, Michael Ellerman wrote:
A lot of these have started appearing recently, which I think is due to
GCC becoming more vocal. Unfortunately many of them are false positives
caused by GCC not seeming to grok that this is ok:
void foo(int *x) { *x = 1; }
...
int x;
foo(x);
return x;
It's more subtle than this, gcc only gets it wrong when multiple
things come together, the most common one seems to be:
- it tries to inline foo()
- foo has a path where it initializes *x and another one where it
doesn't
- x is accessed after foo() returns, but only when foo indeed has
initialized it.
The problem is that gcc now is more aggressive about inlining
functions. It used to assume that all functions initialize their
pointer arguments, now it does some more checking, but not enough,
so there are lots of false positives. Every gcc-4.x release seems
to fix some of these cases, but a few others remain.
Arnd
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [2.6.19 PATCH 4/7] ehea: ethtool interface
Andy Gay wrote: fs/bio.c:169: warning: 'idx' may be used uninitialized in this function fs/eventpoll.c:500: warning: 'fd' may be used uninitialized in this function fs/isofs/namei.c:162: warning: 'offset' may be used uninitialized in this function fs/isofs/namei.c:162: warning: 'block' may be used uninitialized in this function fs/nfsd/nfsctl.c:292: warning: 'maxsize' may be used uninitialized in this function fs/udf/balloc.c:751: warning: 'goal_eloc.logicalBlockNum' may be used uninitialized in this function fs/udf/super.c:1358: warning: 'ino.partitionReferenceNum' may be used uninitialized in this function fs/xfs/xfs_alloc_btree.c:611: warning: 'nkey.ar_startblock' may be used uninitialized in this function fs/xfs/xfs_alloc_btree.c:611: warning: 'nkey.ar_blockcount' may be used uninitialized in this function fs/xfs/xfs_bmap.c:2498: warning: 'rtx' is used uninitialized in this function fs/xfs/xfs_bmap_btree.c:753: warning: 'nkey.br_startoff' may be used uninitialized in this function fs/xfs/xfs_da_btree.c:151: warning: 'action' may be used uninitialized in this function fs/xfs/xfs_dir.c:363: warning: 'totallen' may be used uninitialized in this function fs/xfs/xfs_dir.c:363: warning: 'count' may be used uninitialized in this function fs/xfs/xfs_ialloc_btree.c:545: warning: 'nkey.ir_startino' may be used uninitialized in this function fs/xfs/xfs_inode.c:1958: warning: 'last_dip' may be used uninitialized in this function fs/xfs/xfs_inode.c:1960: warning: 'last_offset' may be used uninitialized in this function fs/xfs/xfs_log.c:1749: warning: 'iclog' may be used uninitialized in this function fs/xfs/xfs_log_recover.c:523: warning: 'first_blk' may be used uninitialized in this function ipc/msg.c:338: warning: 'setbuf.qbytes' may be used uninitialized in this function ipc/msg.c:338: warning: 'setbuf.uid' may be used uninitialized in this function ipc/msg.c:338: warning: 'setbuf.gid' may be used uninitialized in this function ipc/msg.c:338: warning: 'setbuf.mode' may be used uninitialized in this function ipc/sem.c:810: warning: 'setbuf.uid' may be used uninitialized in this function ipc/sem.c:810: warning: 'setbuf.gid' may be used uninitialized in this function ipc/sem.c:810: warning: 'setbuf.mode' may be used uninitialized in this function drivers/md/dm-table.c:431: warning: 'dev' may be used uninitialized in this function drivers/md/dm-ioctl.c:1388: warning: 'param' may be used uninitialized in this function net/sched/sch_cbq.c:409: warning: 'ret' may be used uninitialized in this function lib/zlib_inflate/inftrees.c:121: warning: 'r.base' may be used uninitialized in this function These are gcc bugs. We don't patch the kernel for gcc bugs. Jeff - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[2.6.19 PATCH 4/7] ehea: ethtool interface
Signed-off-by: Jan-Bernd Themann [EMAIL PROTECTED]
drivers/net/ehea/ehea_ethtool.c | 264
1 file changed, 264 insertions(+)
--- linux-2.6.18-rc4-orig/drivers/net/ehea/ehea_ethtool.c 1969-12-31
16:00:00.0 -0800
+++ kernel/drivers/net/ehea/ehea_ethtool.c 2006-08-18 00:01:02.841367875
-0700
@@ -0,0 +1,264 @@
+/*
+ * linux/drivers/net/ehea/ehea_ethtool.c
+ *
+ * eHEA ethernet device driver for IBM eServer System p
+ *
+ * (C) Copyright IBM Corp. 2006
+ *
+ * Authors:
+ * Christoph Raisch [EMAIL PROTECTED]
+ * Jan-Bernd Themann [EMAIL PROTECTED]
+ * Thomas Klein [EMAIL PROTECTED]
+ *
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2, or (at your option)
+ * any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#include ehea.h
+#include ehea_phyp.h
+
+
+static int netdev_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
+{
+ u64 hret = H_HARDWARE;
+ 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);
+
+ if (hret != H_SUCCESS) {
+ ehea_error(query_ehea_port failed);
+ kfree(cb4);
+ goto get_settings_exit;
+ }
+
+ if (netif_msg_hw(port))
+ ehea_dump(cb4, sizeof(struct hcp_query_ehea_port_cb_4),
+ netdev_get_settings);
+
+ if (netif_carrier_ok(dev)) {
+ switch(cb4-port_speed){
+ case H_PORT_SPEED_10M_H:
+ cmd-speed = SPEED_10;
+ cmd-duplex = DUPLEX_HALF;
+ break;
+ case H_PORT_SPEED_10M_F:
+ cmd-speed = SPEED_10;
+ cmd-duplex = DUPLEX_FULL;
+ break;
+ case H_PORT_SPEED_100M_H:
+ cmd-speed = SPEED_100;
+ cmd-duplex = DUPLEX_HALF;
+ break;
+ case H_PORT_SPEED_100M_F:
+ cmd-speed = SPEED_100;
+ cmd-duplex = DUPLEX_FULL;
+ break;
+ case H_PORT_SPEED_1G_F:
+ cmd-speed = SPEED_1000;
+ cmd-duplex = DUPLEX_FULL;
+ break;
+ case H_PORT_SPEED_10G_F:
+ cmd-speed = SPEED_1;
+ cmd-duplex = DUPLEX_FULL;
+ break;
+ }
+ } else {
+ cmd-speed = -1;
+ cmd-duplex = -1;
+ }
+
+ cmd-supported = (SUPPORTED_1baseT_Full | SUPPORTED_1000baseT_Full
+ | SUPPORTED_100baseT_Full | SUPPORTED_100baseT_Half
+ | SUPPORTED_10baseT_Full | SUPPORTED_10baseT_Half
+ | SUPPORTED_Autoneg | SUPPORTED_FIBRE);
+
+ cmd-advertising = (ADVERTISED_1baseT_Full | ADVERTISED_Autoneg
+| ADVERTISED_FIBRE);
+
+ cmd-port = PORT_FIBRE;
+ cmd-autoneg = AUTONEG_ENABLE;
+
+ kfree(cb4);
+ return 0;
+
+get_settings_exit:
+ return 1;
+}
+
+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);
+}
+
+static u32 netdev_get_msglevel(struct net_device *dev)
+{
+ struct ehea_port *port = netdev_priv(dev);
+ return port-msg_enable;
+}
+
+static void netdev_set_msglevel(struct net_device *dev, u32 value)
+{
+ struct ehea_port *port = netdev_priv(dev);
+ port-msg_enable = value;
+}
+
+static char ehea_ethtool_stats_keys[][ETH_GSTRING_LEN] = {
+ {poll_max_processed},
+ {queue_stopped},
+ {min_swqe_avail},
+ {poll_receive_err},
+ {pkt_send},
+
Re: [2.6.19 PATCH 4/7] ehea: ethtool interface
On Fri, Aug 18, 2006 at 01:33:22PM +0200, Jan-Bernd Themann wrote:
--- linux-2.6.18-rc4-orig/drivers/net/ehea/ehea_ethtool.c
+++ kernel/drivers/net/ehea/ehea_ethtool.c
+static int netdev_get_settings(struct net_device *dev, struct ethtool_cmd
*cmd)
+{
+ u64 hret = H_HARDWARE;
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.
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [2.6.19 PATCH 4/7] ehea: ethtool interface
Hi Alexey,
first of all thanks a lot for the extensive review.
Alexey Dobriyan wrote:
+ u64 hret = H_HARDWARE;
Useless assignment here and everywhere.
Initializing returncodes to errorstate is a cheap way to prevent
accidentally returning (uninitalized) success returncodes which
can lead to catastrophic misbehaviour.
+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.
I agree.
Kind regards
Thomas
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [2.6.19 PATCH 4/7] ehea: ethtool interface
On Fri, 18 Aug 2006 17:41:26 +0200 Thomas Klein [EMAIL PROTECTED] wrote: Hi Alexey, first of all thanks a lot for the extensive review. Alexey Dobriyan wrote: + u64 hret = H_HARDWARE; Useless assignment here and everywhere. Initializing returncodes to errorstate is a cheap way to prevent accidentally returning (uninitalized) success returncodes which can lead to catastrophic misbehaviour. That is old thinking. Current compilers do live/dead analysis and tell you about this at compile time which is better than relying on default behavior at runtime. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
