[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <87618083B2453E4A8714035B62D679924FD8BC50@ORSMSX152.amr.corp.intel.com>
Date: Tue, 30 Apr 2013 18:12:22 +0000
From: "Tantilov, Emil S" <emil.s.tantilov@...el.com>
To: Ben Hutchings <bhutchings@...arflare.com>,
"Kirsher, Jeffrey T" <jeffrey.t.kirsher@...el.com>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"gospo@...hat.com" <gospo@...hat.com>,
"sassmann@...hat.com" <sassmann@...hat.com>
Subject: RE: [ethtool] ethtool: check mac type from ethtool_regs.version
>-----Original Message-----
>From: Ben Hutchings [mailto:bhutchings@...arflare.com]
>Sent: Tuesday, April 30, 2013 8:24 AM
>To: Kirsher, Jeffrey T; Tantilov, Emil S
>Cc: netdev@...r.kernel.org; gospo@...hat.com; sassmann@...hat.com
>Subject: Re: [ethtool] ethtool: check mac type from ethtool_regs.version
>
>On Thu, 2013-04-25 at 22:12 -0700, Jeff Kirsher wrote:
>> From: Emil Tantilov <emil.s.tantilov@...el.com>
>>
>> This patch cleans up the mac type checks by using ethtool_regs.version
>> provided by the driver. This change eliminates the need to add device IDs
>> every time they are added to the driver.
>>
>> This patch depends on a driver change that will add the mac_type to the
>> upper 8 bits of ethtool_regs.version.
>>
>> Note that when using ethtool with this patch with a version of ixgbe that
>> does not provide the mac_type in ethtool_regs.version the register dump
>> may be incomplete. However this issue would've existed previously for
>> device IDs that were not added to ethtool.
>[...]
>
>I don't think this is acceptable; ethtool should remain backward
>compatible if at all possible.
>
>It seems like this would work with both old and new drivers:
>
>diff --git a/ixgbe.c b/ixgbe.c
>index dae11d4..9b005f2 100644
>--- a/ixgbe.c
>+++ b/ixgbe.c
>@@ -133,10 +133,13 @@ ixgbe_dump_regs(struct ethtool_drvinfo *info, struct
>ethtool_regs *regs)
> u8 i;
> enum ixgbe_mac_type mac_type;
>
>- if (version != 1)
>+ if (version == 0)
> return -1;
>
>- mac_type = ixgbe_get_mac_type(hw_device_id);
>+ /* The current driver reports the MAC type, but older versions
>+ * only report the device ID so we have to infer the MAC type.
>+ */
>+ mac_type = version > 1 ? version : ixgbe_get_mac_type(hw_device_id);
>
> reg = regs_buff[1065];
> fprintf(stdout,
Yeah, this seems like a better way to go.
Thanks a lot,
Emil
>
>--
>Ben Hutchings, Staff Engineer, Solarflare
>Not speaking for my employer; that's the marketing department's job.
>They asked us to note that Solarflare product names are trademarked.
Powered by blists - more mailing lists