Re: [PATCH] PCI: Move various PCI IDs to header file

Re: [PATCH] PCI: Move various PCI IDs to header file

[Jeff Garzik]

Linux Kernel Mailing List wrote:
> diff --git a/drivers/scsi/sata_vsc.c b/drivers/scsi/sata_vsc.c
> index 8a29ce3..27d6587 100644
> --- a/drivers/scsi/sata_vsc.c
> +++ b/drivers/scsi/sata_vsc.c
> @@ -433,13 +433,14 @@ err_out:
>  
>  
>  /*
> - * 0x1725/0x7174 is the Vitesse VSC-7174
> - * 0x8086/0x3200 is the Intel 31244, which is supposed to be identical
> - * compatibility is untested as of yet
> + * Intel 31244 is supposed to be identical.
> + * Compatibility is untested as of yet.
>   */
>  static const struct pci_device_id vsc_sata_pci_tbl[] = {
> -	{ 0x1725, 0x7174, PCI_ANY_ID, PCI_ANY_ID, 0x10600, 0xFFFFFF, 0 },
> -	{ 0x8086, 0x3200, PCI_ANY_ID, PCI_ANY_ID, 0x10600, 0xFFFFFF, 0 },
> +	{ PCI_VENDOR_ID_VITESSE, PCI_DEVICE_ID_VITESSE_VSC7174,
> +	  PCI_ANY_ID, PCI_ANY_ID, 0x10600, 0xFFFFFF, 0 },
> +	{ PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_GD31244,
> +	  PCI_ANY_ID, PCI_ANY_ID, 0x10600, 0xFFFFFF, 0 },
>  	{ }
>  };
>  

WTF?  This is a REGRESSION from the repeatedly expressed desire -- clear 
throughout libata -- that single-use PCI device IDs should not ever 
receive PCI_DEVICE_ID_xxx constants.

I'm going to queue up a revert patch for this silliness.

Next time, please let the relevant maintainer(s) know when you are 
touching their driver, so they have a chance to filter out the crap.

This was -never- sent to me or linux-ide, or otherwise brought to the 
attention of the maintainers.

	Jeff

Re: [PATCH] PCI: Move various PCI IDs to header file

[Randy.Dunlap]

On Thu, 22 Jun 2006 21:29:47 -0400 Jeff Garzik wrote:

> Linux Kernel Mailing List wrote:
> > diff --git a/drivers/scsi/sata_vsc.c b/drivers/scsi/sata_vsc.c
> > index 8a29ce3..27d6587 100644
> > --- a/drivers/scsi/sata_vsc.c
> > +++ b/drivers/scsi/sata_vsc.c
> > @@ -433,13 +433,14 @@ err_out:
> >  
> >  
> >  /*
> > - * 0x1725/0x7174 is the Vitesse VSC-7174
> > - * 0x8086/0x3200 is the Intel 31244, which is supposed to be identical
> > - * compatibility is untested as of yet
> > + * Intel 31244 is supposed to be identical.
> > + * Compatibility is untested as of yet.
> >   */
> >  static const struct pci_device_id vsc_sata_pci_tbl[] = {
> > -	{ 0x1725, 0x7174, PCI_ANY_ID, PCI_ANY_ID, 0x10600, 0xFFFFFF, 0 },
> > -	{ 0x8086, 0x3200, PCI_ANY_ID, PCI_ANY_ID, 0x10600, 0xFFFFFF, 0 },
> > +	{ PCI_VENDOR_ID_VITESSE, PCI_DEVICE_ID_VITESSE_VSC7174,
> > +	  PCI_ANY_ID, PCI_ANY_ID, 0x10600, 0xFFFFFF, 0 },
> > +	{ PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_GD31244,
> > +	  PCI_ANY_ID, PCI_ANY_ID, 0x10600, 0xFFFFFF, 0 },
> >  	{ }
> >  };
> >  
> 
> WTF?  This is a REGRESSION from the repeatedly expressed desire -- clear 
> throughout libata -- that single-use PCI device IDs should not ever 
> receive PCI_DEVICE_ID_xxx constants.
> 
> I'm going to queue up a revert patch for this silliness.
> 
> Next time, please let the relevant maintainer(s) know when you are 
> touching their driver, so they have a chance to filter out the crap.
> 
> This was -never- sent to me or linux-ide, or otherwise brought to the 
> attention of the maintainers.

Well, if you could get IDE/ATA traffic moved to linux-ide
and networking traffic moved to netdev  and USB traffic
to linux-usb etc. etc. etc.,
I'd love it.  :)

lkml is like a "melting pot."

---
~Randy

Re: [PATCH] PCI: Move various PCI IDs to header file

[Jeff Garzik]

Randy.Dunlap wrote:
> lkml is like a "melting pot."

It's also easy to miss stuff, which is why Andrew manages to CC the 
relevant maintainers, when something pops into his tree...

	Jeff

Re: [PATCH] PCI: Move various PCI IDs to header file

[Brent Casavant]

Jeff,

I apologize for the incredibly late reply.  Your objection of course
came up mere hours after I left on a vacation of nearly two weeks.

I also apologize for failing to CC linux-ide.  I did, however,
originally CC the author of the Vitesse VSC driver (Jeremy Higdon).

The patch was submitted precisely because the PCI device ID was no
longer going to be single-use.  In fact, shortly after the message
you posted below, Andrew posted the following (which has been queued
up for a month or so):

	[patch 187/200] SGI IOC4: Detect IO card variant

In short, due to the unusual card design of the SGI IO10 card, and
the need to differentiate it from the SGI PCI-RT card, I was left
with no reasonable option other than to look for the existence of
the Vitesse device at a known PCI bus location.  As such I needed
to re-use the PCI ID of the Vitesse part, thereby making it dual-use
and qualifying it for assignment of a PCI_DEVICE_ID_xxx constant.

I did perhaps overstep by also creating a PCI_DEVICE_ID_xxx constant
for the purportedly identical Intel part.  Am I allowed a third apology
within a single email?

Brent

On Thu, 22 Jun 2006, Jeff Garzik wrote:

> WTF?  This is a REGRESSION from the repeatedly expressed desire -- clear
> throughout libata -- that single-use PCI device IDs should not ever receive
> PCI_DEVICE_ID_xxx constants.
> 
> I'm going to queue up a revert patch for this silliness.
> 
> Next time, please let the relevant maintainer(s) know when you are touching
> their driver, so they have a chance to filter out the crap.
> 
> This was -never- sent to me or linux-ide, or otherwise brought to the
> attention of the maintainers.

-- 
Brent Casavant                          All music is folk music.  I ain't
bcasavan@sgi.com                        never heard a horse sing a song.
Silicon Graphics, Inc.                    -- Louis Armstrong