Re: [trivial PATCH 1/7] ixgb: use PCI_VENDOR_ID_*

Re: [trivial PATCH 1/7] ixgb: use PCI_VENDOR_ID_*

[Jeff Kirsher]

[-- Attachment #1: Type: text/plain, Size: 4241 bytes --]

On Tue, 2012-07-10 at 15:31 -0700, Jon Mason wrote:
> Use PCI_VENDOR_ID_* from pci_ids.h instead of creating #define locally.
> 
> Signed-off-by: Jon Mason <jdmason@kudzu.us>
> Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Cc: Bruce Allan <bruce.w.allan@intel.com>
> Cc: Carolyn Wyborny <carolyn.wyborny@intel.com>
> Cc: Don Skidmore <donald.c.skidmore@intel.com>
> Cc: Greg Rose <gregory.v.rose@intel.com>
> Cc: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
> Cc: Alex Duyck <alexander.h.duyck@intel.com>
> Cc: John Ronciak <john.ronciak@intel.com>
> ---
>  drivers/net/ethernet/intel/ixgb/ixgb_hw.c   |    5 +++--
>  drivers/net/ethernet/intel/ixgb/ixgb_ids.h  |    5 -----
>  drivers/net/ethernet/intel/ixgb/ixgb_main.c |   10 +++++-----
>  3 files changed, 8 insertions(+), 12 deletions(-)

This should go through David Miller's networking tree's.  Adding netdev
mailing list.

Acked-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

> 
> diff --git a/drivers/net/ethernet/intel/ixgb/ixgb_hw.c b/drivers/net/ethernet/intel/ixgb/ixgb_hw.c
> index 99b69ad..bf9a220 100644
> --- a/drivers/net/ethernet/intel/ixgb/ixgb_hw.c
> +++ b/drivers/net/ethernet/intel/ixgb/ixgb_hw.c
> @@ -32,6 +32,7 @@
>  
>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>  
> +#include <linux/pci_ids.h>
>  #include "ixgb_hw.h"
>  #include "ixgb_ids.h"
>  
> @@ -96,7 +97,7 @@ static u32 ixgb_mac_reset(struct ixgb_hw *hw)
>  	ASSERT(!(ctrl_reg & IXGB_CTRL0_RST));
>  #endif
>  
> -	if (hw->subsystem_vendor_id == SUN_SUBVENDOR_ID) {
> +	if (hw->subsystem_vendor_id == PCI_VENDOR_ID_SUN) {
>  		ctrl_reg =  /* Enable interrupt from XFP and SerDes */
>  			   IXGB_CTRL1_GPI0_EN |
>  			   IXGB_CTRL1_SDP6_DIR |
> @@ -271,7 +272,7 @@ ixgb_identify_phy(struct ixgb_hw *hw)
>  	}
>  
>  	/* update phy type for sun specific board */
> -	if (hw->subsystem_vendor_id == SUN_SUBVENDOR_ID)
> +	if (hw->subsystem_vendor_id == PCI_VENDOR_ID_SUN)
>  		phy_type = ixgb_phy_type_bcm;
>  
>  	return phy_type;
> diff --git a/drivers/net/ethernet/intel/ixgb/ixgb_ids.h b/drivers/net/ethernet/intel/ixgb/ixgb_ids.h
> index 2a58847..32c1b30 100644
> --- a/drivers/net/ethernet/intel/ixgb/ixgb_ids.h
> +++ b/drivers/net/ethernet/intel/ixgb/ixgb_ids.h
> @@ -33,11 +33,6 @@
>  ** The Device and Vendor IDs for 10 Gigabit MACs
>  **********************************************************************/
>  
> -#define INTEL_VENDOR_ID             0x8086
> -#define INTEL_SUBVENDOR_ID          0x8086
> -#define SUN_VENDOR_ID               0x108E
> -#define SUN_SUBVENDOR_ID            0x108E
> -
>  #define IXGB_DEVICE_ID_82597EX      0x1048
>  #define IXGB_DEVICE_ID_82597EX_SR   0x1A48
>  #define IXGB_DEVICE_ID_82597EX_LR   0x1B48
> diff --git a/drivers/net/ethernet/intel/ixgb/ixgb_main.c b/drivers/net/ethernet/intel/ixgb/ixgb_main.c
> index 5fce363..4e5a060 100644
> --- a/drivers/net/ethernet/intel/ixgb/ixgb_main.c
> +++ b/drivers/net/ethernet/intel/ixgb/ixgb_main.c
> @@ -54,13 +54,13 @@ MODULE_PARM_DESC(copybreak,
>   *   Class, Class Mask, private data (not used) }
>   */
>  static DEFINE_PCI_DEVICE_TABLE(ixgb_pci_tbl) = {
> -	{INTEL_VENDOR_ID, IXGB_DEVICE_ID_82597EX,
> +	{PCI_VENDOR_ID_INTEL, IXGB_DEVICE_ID_82597EX,
>  	 PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
> -	{INTEL_VENDOR_ID, IXGB_DEVICE_ID_82597EX_CX4,
> +	{PCI_VENDOR_ID_INTEL, IXGB_DEVICE_ID_82597EX_CX4,
>  	 PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
> -	{INTEL_VENDOR_ID, IXGB_DEVICE_ID_82597EX_SR,
> +	{PCI_VENDOR_ID_INTEL, IXGB_DEVICE_ID_82597EX_SR,
>  	 PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
> -	{INTEL_VENDOR_ID, IXGB_DEVICE_ID_82597EX_LR,
> +	{PCI_VENDOR_ID_INTEL, IXGB_DEVICE_ID_82597EX_LR,
>  	 PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
>  
>  	/* required last entry */
> @@ -195,7 +195,7 @@ ixgb_irq_enable(struct ixgb_adapter *adapter)
>  {
>  	u32 val = IXGB_INT_RXT0 | IXGB_INT_RXDMT0 |
>  		  IXGB_INT_TXDW | IXGB_INT_LSC;
> -	if (adapter->hw.subsystem_vendor_id == SUN_SUBVENDOR_ID)
> +	if (adapter->hw.subsystem_vendor_id == PCI_VENDOR_ID_SUN)
>  		val |= IXGB_INT_GPI0;
>  	IXGB_WRITE_REG(&adapter->hw, IMS, val);
>  	IXGB_WRITE_FLUSH(&adapter->hw);



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [trivial PATCH 2/7] ixgbe: remove unused #define

[Jeff Kirsher]

[-- Attachment #1: Type: text/plain, Size: 1397 bytes --]

On Tue, 2012-07-10 at 15:31 -0700, Jon Mason wrote:
> Remove unused IXGBE_INTEL_VENDOR_ID #define
> 
> Signed-off-by: Jon Mason <jdmason@kudzu.us>
> Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Cc: Bruce Allan <bruce.w.allan@intel.com>
> Cc: Carolyn Wyborny <carolyn.wyborny@intel.com>
> Cc: Don Skidmore <donald.c.skidmore@intel.com>
> Cc: Greg Rose <gregory.v.rose@intel.com>
> Cc: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
> Cc: Alex Duyck <alexander.h.duyck@intel.com>
> Cc: John Ronciak <john.ronciak@intel.com>
> ---
>  drivers/net/ethernet/intel/ixgbe/ixgbe_type.h |    3 ---
>  1 file changed, 3 deletions(-)

This should also go through David Miller's networking tree's.  Adding
netdev mailing list.

Acked-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

> 
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> index 204848d..c8d8040 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> @@ -32,9 +32,6 @@
>  #include <linux/mdio.h>
>  #include <linux/netdevice.h>
>  
> -/* Vendor ID */
> -#define IXGBE_INTEL_VENDOR_ID   0x8086
> -
>  /* Device IDs */
>  #define IXGBE_DEV_ID_82598               0x10B6
>  #define IXGBE_DEV_ID_82598_BX            0x1508



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [trivial PATCH 2/7] ixgbe: remove unused #define

[Jeff Kirsher]

[-- Attachment #1: Type: text/plain, Size: 1699 bytes --]

On Tue, 2012-07-10 at 15:43 -0700, Jeff Kirsher wrote:
> On Tue, 2012-07-10 at 15:31 -0700, Jon Mason wrote:
> > Remove unused IXGBE_INTEL_VENDOR_ID #define
> > 
> > Signed-off-by: Jon Mason <jdmason@kudzu.us>
> > Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> > Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
> > Cc: Bruce Allan <bruce.w.allan@intel.com>
> > Cc: Carolyn Wyborny <carolyn.wyborny@intel.com>
> > Cc: Don Skidmore <donald.c.skidmore@intel.com>
> > Cc: Greg Rose <gregory.v.rose@intel.com>
> > Cc: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
> > Cc: Alex Duyck <alexander.h.duyck@intel.com>
> > Cc: John Ronciak <john.ronciak@intel.com>
> > ---
> >  drivers/net/ethernet/intel/ixgbe/ixgbe_type.h |    3 ---
> >  1 file changed, 3 deletions(-)
> 
> This should also go through David Miller's networking tree's.  Adding
> netdev mailing list.
> 
> Acked-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

After looking at this further, I rescind my ACK.

NAK, this define is used in several places in ixgbe_main.c and the
driver will not compile with this patch.

> 
> > 
> > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> > index 204848d..c8d8040 100644
> > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> > @@ -32,9 +32,6 @@
> >  #include <linux/mdio.h>
> >  #include <linux/netdevice.h>
> >  
> > -/* Vendor ID */
> > -#define IXGBE_INTEL_VENDOR_ID   0x8086
> > -
> >  /* Device IDs */
> >  #define IXGBE_DEV_ID_82598               0x10B6
> >  #define IXGBE_DEV_ID_82598_BX            0x1508
> 
> 


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [trivial PATCH 2/7] ixgbe: remove unused #define

[Jon Mason]

On Thu, Jul 12, 2012 at 11:15 AM, Jeff Kirsher
<jeffrey.t.kirsher@intel.com> wrote:
> On Tue, 2012-07-10 at 15:43 -0700, Jeff Kirsher wrote:
>> On Tue, 2012-07-10 at 15:31 -0700, Jon Mason wrote:
>> > Remove unused IXGBE_INTEL_VENDOR_ID #define
>> >
>> > Signed-off-by: Jon Mason <jdmason@kudzu.us>
>> > Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>> > Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
>> > Cc: Bruce Allan <bruce.w.allan@intel.com>
>> > Cc: Carolyn Wyborny <carolyn.wyborny@intel.com>
>> > Cc: Don Skidmore <donald.c.skidmore@intel.com>
>> > Cc: Greg Rose <gregory.v.rose@intel.com>
>> > Cc: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
>> > Cc: Alex Duyck <alexander.h.duyck@intel.com>
>> > Cc: John Ronciak <john.ronciak@intel.com>
>> > ---
>> >  drivers/net/ethernet/intel/ixgbe/ixgbe_type.h |    3 ---
>> >  1 file changed, 3 deletions(-)
>>
>> This should also go through David Miller's networking tree's.  Adding
>> netdev mailing list.
>>
>> Acked-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>
> After looking at this further, I rescind my ACK.
>
> NAK, this define is used in several places in ixgbe_main.c and the
> driver will not compile with this patch.

My apologies, I was a bit sloppy and simply rebased my old patches
without retesting them.  The code has changed since I originally
pushed the patch (http://www.spinics.net/lists/netdev/msg171523.html).
 I will fix and resubmit.

Thanks,
Jon


>>
>> >
>> > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
>> > index 204848d..c8d8040 100644
>> > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
>> > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
>> > @@ -32,9 +32,6 @@
>> >  #include <linux/mdio.h>
>> >  #include <linux/netdevice.h>
>> >
>> > -/* Vendor ID */
>> > -#define IXGBE_INTEL_VENDOR_ID   0x8086
>> > -
>> >  /* Device IDs */
>> >  #define IXGBE_DEV_ID_82598               0x10B6
>> >  #define IXGBE_DEV_ID_82598_BX            0x1508
>>
>>
>

Re: [trivial PATCH 2/7] ixgbe: remove unused #define

[Jeff Kirsher]

[-- Attachment #1: Type: text/plain, Size: 1847 bytes --]

On Thu, 2012-07-12 at 12:49 -0700, Jon Mason wrote:
> On Thu, Jul 12, 2012 at 11:15 AM, Jeff Kirsher
> <jeffrey.t.kirsher@intel.com> wrote:
> > On Tue, 2012-07-10 at 15:43 -0700, Jeff Kirsher wrote:
> >> On Tue, 2012-07-10 at 15:31 -0700, Jon Mason wrote:
> >> > Remove unused IXGBE_INTEL_VENDOR_ID #define
> >> >
> >> > Signed-off-by: Jon Mason <jdmason@kudzu.us>
> >> > Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> >> > Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
> >> > Cc: Bruce Allan <bruce.w.allan@intel.com>
> >> > Cc: Carolyn Wyborny <carolyn.wyborny@intel.com>
> >> > Cc: Don Skidmore <donald.c.skidmore@intel.com>
> >> > Cc: Greg Rose <gregory.v.rose@intel.com>
> >> > Cc: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
> >> > Cc: Alex Duyck <alexander.h.duyck@intel.com>
> >> > Cc: John Ronciak <john.ronciak@intel.com>
> >> > ---
> >> >  drivers/net/ethernet/intel/ixgbe/ixgbe_type.h |    3 ---
> >> >  1 file changed, 3 deletions(-)
> >>
> >> This should also go through David Miller's networking tree's.
> Adding
> >> netdev mailing list.
> >>
> >> Acked-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> >
> > After looking at this further, I rescind my ACK.
> >
> > NAK, this define is used in several places in ixgbe_main.c and the
> > driver will not compile with this patch.
> 
> My apologies, I was a bit sloppy and simply rebased my old patches
> without retesting them.  The code has changed since I originally
> pushed the patch (http://www.spinics.net/lists/netdev/msg171523.html).
>  I will fix and resubmit.
> 
> Thanks,
> Jon 

Also note, that when you fix up the patch and resend it, could you
resend patch 1 (ixgb) as well to netdev@vger.kernel.org.  That way
netdev patchwork will pickup the patches and I can get them into my
queue.

Thanks!
Jeff



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]