LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [GIT PATCH] PCI fixes for 2.6.25-rc2 git tree
@ 2008-02-21 23:46 Greg KH
       [not found] ` <1203637660-7247-3-git-send-email-gregkh@suse.de>
  0 siblings, 1 reply; 7+ messages in thread
From: Greg KH @ 2008-02-21 23:46 UTC (permalink / raw)
  To: Linus Torvalds, Andrew Morton; +Cc: linux-kernel, linux-pci, pcihpd-discuss

Here are some PCI patches against your 2.6.25-rc2 git tree.

They are a collection of PCI quirk additions, build fixes, and some PCI
hotplug fixes.


Please pull from:
	master.kernel.org:/pub/scm/linux/kernel/git/gregkh/pci-2.6.git/


The full patches will be sent to the linux-pci mailing list, if anyone
wants to see them.

thanks,

greg k-h


-------------

 arch/x86/kernel/acpi/boot.c        |    3 +-
 arch/x86/pci/irq.c                 |    4 ++
 drivers/pci/hotplug/acpiphp_core.c |    6 +++
 drivers/pci/hotplug/acpiphp_ibm.c  |   33 ++++++++-------
 drivers/pci/pci-acpi.c             |    1 +
 drivers/pci/proc.c                 |    2 +-
 drivers/pci/quirks.c               |   79 ++++++++++++++++++++++++++++++++---
 drivers/pci/setup-bus.c            |    6 +--
 drivers/pcmcia/i82092.c            |    7 +++-
 include/linux/pci_ids.h            |    6 +++
 10 files changed, 117 insertions(+), 30 deletions(-)

---------------

Andrew Morton (2):
      PCI: drivers/pcmcia/i82092.c: fix up after pci_bus_region changes
      PCI: fix up setup-bus.c #ifdef

Crane Cai (1):
      PCI: AMD SATA IDE mode quirk

Gary Hade (1):
      PCI: hotplug: acpiphp_ibm: Remove get device information

Jason Gaston (2):
      PCI: pci_ids: patch for Intel ICH10 DeviceID's
      PCI: irq: patch for Intel ICH10 DeviceID's

Kenji Kaneshige (1):
      PCI: Fix wrong reference counter check for proc_dir_entry

Peer Chen (1):
      PCI: quirks: set 'En' bit of MSI Mapping for devices onHT-based nvidia platform

Randy Dunlap (1):
      PCI: kernel-doc: fix pci-acpi warning

Yinghai Lu (1):
      PCI: don't load acpi_php when acpi is disabled


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 03/10] PCI: AMD SATA IDE mode quirk
       [not found] ` <1203637660-7247-3-git-send-email-gregkh@suse.de>
@ 2008-02-22  4:17   ` Matthew Wilcox
  2008-02-22  5:49     ` Cai, Crane
  0 siblings, 1 reply; 7+ messages in thread
From: Matthew Wilcox @ 2008-02-22  4:17 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: linux-pci, Crane Cai, linux-kernel, Linus Torvalds, Andrew Morton

On Thu, Feb 21, 2008 at 03:47:33PM -0800, Greg Kroah-Hartman wrote:
> +static void __devinit quirk_amd_ide_mode(struct pci_dev *pdev)
>  {
> -	/* set sb600 sata to ahci mode */
> -	if ((pdev->class >> 8) == PCI_CLASS_STORAGE_IDE) {
> -		u8 tmp;
> +	/* set sb600/sb700/sb800 sata to ahci mode */
> +	u8 tmp;
>  
> +	pci_read_config_byte(pdev, PCI_CLASS_DEVICE, &tmp);
> +	if (tmp == 0x01) {
>  		pci_read_config_byte(pdev, 0x40, &tmp);

This seems like a dis-improvement.  Why are we reading a config byte for
something we already have in the pci_dev?  Why are we now checking
against 0x01 instead of a symbolic constant?  Why are we no longer
checking that this is PCI_BASE_CLASS_STORAGE?

> -DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_ATI, PCI_DEVICE_ID_ATI_IXP600_SATA, quirk_sb600_sata);
> -DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_ATI, PCI_DEVICE_ID_ATI_IXP700_SATA, quirk_sb600_sata);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_ATI, PCI_DEVICE_ID_ATI_IXP600_SATA, quirk_amd_ide_mode);
> +DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_ATI, PCI_DEVICE_ID_ATI_IXP600_SATA, quirk_amd_ide_mode);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_ATI, PCI_DEVICE_ID_ATI_IXP700_SATA, quirk_amd_ide_mode);
> +DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_ATI, PCI_DEVICE_ID_ATI_IXP700_SATA, quirk_amd_ide_mode);

Nothing in the changelog entry suggests why we now need FIXUP_RESUME
entries when we didn't before.

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH 03/10] PCI: AMD SATA IDE mode quirk
  2008-02-22  4:17   ` [PATCH 03/10] PCI: AMD SATA IDE mode quirk Matthew Wilcox
@ 2008-02-22  5:49     ` Cai, Crane
  2008-02-22 11:11       ` Matthew Wilcox
  0 siblings, 1 reply; 7+ messages in thread
From: Cai, Crane @ 2008-02-22  5:49 UTC (permalink / raw)
  To: Matthew Wilcox, Greg Kroah-Hartman
  Cc: linux-pci, linux-kernel, Linus Torvalds, Andrew Morton

> On Thu, Feb 21, 2008 at 03:47:33PM -0800, Greg Kroah-Hartman wrote:
> > +static void __devinit quirk_amd_ide_mode(struct pci_dev *pdev)
> >  {
> > -	/* set sb600 sata to ahci mode */
> > -	if ((pdev->class >> 8) == PCI_CLASS_STORAGE_IDE) {
> > -		u8 tmp;
> > +	/* set sb600/sb700/sb800 sata to ahci mode */
> > +	u8 tmp;
> >  
> > +	pci_read_config_byte(pdev, PCI_CLASS_DEVICE, &tmp);
> > +	if (tmp == 0x01) {
> >  		pci_read_config_byte(pdev, 0x40, &tmp);
> 
> This seems like a dis-improvement.  Why are we reading a 
> config byte for something we already have in the pci_dev?  
> Why are we now checking against 0x01 instead of a symbolic 
> constant?  Why are we no longer checking that this is 
> PCI_BASE_CLASS_STORAGE?
It is a quirk. In pci_ids.h did have PCI_CLASS_STORAGE_IDE 
and PCI_BASE_CLASS_STORAGE, these can not represent 
the right situation we want to check. 0x01 represents 
PCI_CLASS_STORAGE_IDE last 2 bit. Also because it 
is a quirk, I do not think we need to change pci_ids.h. So 0x01 
used. 
> > -DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_ATI, 
> > PCI_DEVICE_ID_ATI_IXP600_SATA, quirk_sb600_sata); 
> > -DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_ATI, 
> > PCI_DEVICE_ID_ATI_IXP700_SATA, quirk_sb600_sata);
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_ATI, 
> > +PCI_DEVICE_ID_ATI_IXP600_SATA, quirk_amd_ide_mode); 
> > +DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_ATI, 
> > +PCI_DEVICE_ID_ATI_IXP600_SATA, quirk_amd_ide_mode); 
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_ATI, 
> > +PCI_DEVICE_ID_ATI_IXP700_SATA, quirk_amd_ide_mode); 
> > +DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_ATI, 
> > +PCI_DEVICE_ID_ATI_IXP700_SATA, quirk_amd_ide_mode);
> 
> Nothing in the changelog entry suggests why we now need 
> FIXUP_RESUME entries when we didn't before.
> 
PCI configuration space will be changed by BIOS and then in pci
init and restore. So resume also needed.
> --
> Intel are signing my paycheques ... these opinions are still 
> mine "Bill, look, we understand that you're interested in 
> selling us this operating system, but compare it to ours.  We 
> can't possibly take such a retrograde step."
> 
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 03/10] PCI: AMD SATA IDE mode quirk
  2008-02-22  5:49     ` Cai, Crane
@ 2008-02-22 11:11       ` Matthew Wilcox
  2008-02-25  1:43         ` Cai, Crane
  0 siblings, 1 reply; 7+ messages in thread
From: Matthew Wilcox @ 2008-02-22 11:11 UTC (permalink / raw)
  To: Cai, Crane
  Cc: Greg Kroah-Hartman, linux-pci, linux-kernel, Linus Torvalds,
	Andrew Morton

On Fri, Feb 22, 2008 at 01:49:20PM +0800, Cai, Crane wrote:
> > On Thu, Feb 21, 2008 at 03:47:33PM -0800, Greg Kroah-Hartman wrote:
> > > +static void __devinit quirk_amd_ide_mode(struct pci_dev *pdev)
> > >  {
> > > -	/* set sb600 sata to ahci mode */
> > > -	if ((pdev->class >> 8) == PCI_CLASS_STORAGE_IDE) {
> > > -		u8 tmp;
> > > +	/* set sb600/sb700/sb800 sata to ahci mode */
> > > +	u8 tmp;
> > >  
> > > +	pci_read_config_byte(pdev, PCI_CLASS_DEVICE, &tmp);
> > > +	if (tmp == 0x01) {
> > >  		pci_read_config_byte(pdev, 0x40, &tmp);
> > 
> > This seems like a dis-improvement.  Why are we reading a 
> > config byte for something we already have in the pci_dev?  
> > Why are we now checking against 0x01 instead of a symbolic 
> > constant?  Why are we no longer checking that this is 
> > PCI_BASE_CLASS_STORAGE?
> It is a quirk. In pci_ids.h did have PCI_CLASS_STORAGE_IDE 
> and PCI_BASE_CLASS_STORAGE, these can not represent 
> the right situation we want to check. 0x01 represents 
> PCI_CLASS_STORAGE_IDE last 2 bit. Also because it 
> is a quirk, I do not think we need to change pci_ids.h. So 0x01 
> used. 

You haven't explained what is wrong with the original code:

	if ((pdev->class >> 8) == PCI_CLASS_STORAGE_IDE) {

> > Nothing in the changelog entry suggests why we now need 
> > FIXUP_RESUME entries when we didn't before.
> > 
> PCI configuration space will be changed by BIOS and then in pci
> init and restore. So resume also needed.

That information needed to be in the changelog.

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH 03/10] PCI: AMD SATA IDE mode quirk
  2008-02-22 11:11       ` Matthew Wilcox
@ 2008-02-25  1:43         ` Cai, Crane
  2008-02-25  7:31           ` Grant Grundler
  0 siblings, 1 reply; 7+ messages in thread
From: Cai, Crane @ 2008-02-25  1:43 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Greg Kroah-Hartman, linux-pci, linux-kernel, Linus Torvalds,
	Andrew Morton

> On Fri, Feb 22, 2008 at 01:49:20PM +0800, Cai, Crane wrote:
> > > On Thu, Feb 21, 2008 at 03:47:33PM -0800, Greg 
> Kroah-Hartman wrote:
> > > > +static void __devinit quirk_amd_ide_mode(struct pci_dev *pdev)
> > > >  {
> > > > -	/* set sb600 sata to ahci mode */
> > > > -	if ((pdev->class >> 8) == PCI_CLASS_STORAGE_IDE) {
> > > > -		u8 tmp;
> > > > +	/* set sb600/sb700/sb800 sata to ahci mode */
> > > > +	u8 tmp;
> > > >  
> > > > +	pci_read_config_byte(pdev, PCI_CLASS_DEVICE, &tmp);
> > > > +	if (tmp == 0x01) {
> > > >  		pci_read_config_byte(pdev, 0x40, &tmp);
> > > 
> > > This seems like a dis-improvement.  Why are we reading a 
> config byte 
> > > for something we already have in the pci_dev?
> > > Why are we now checking against 0x01 instead of a 
> symbolic constant?  
> > > Why are we no longer checking that this is PCI_BASE_CLASS_STORAGE?
> > It is a quirk. In pci_ids.h did have PCI_CLASS_STORAGE_IDE and 
> > PCI_BASE_CLASS_STORAGE, these can not represent the right 
> situation we 
> > want to check. 0x01 represents PCI_CLASS_STORAGE_IDE last 2 
> bit. Also 
> > because it is a quirk, I do not think we need to change 
> pci_ids.h. So 
> > 0x01 used.
> 
> You haven't explained what is wrong with the original code:
> 
> 	if ((pdev->class >> 8) == PCI_CLASS_STORAGE_IDE) {
> 

When resume, this pdev->class is quirked, however BIOS has modified pci configuration too. Inconsistance occurs.

> > > Nothing in the changelog entry suggests why we now need 
> FIXUP_RESUME 
> > > entries when we didn't before.
> > > 
> > PCI configuration space will be changed by BIOS and then in 
> pci init 
> > and restore. So resume also needed.
> 
> That information needed to be in the changelog.

This info, is a normal info. If maintainer need us to added in source code. I preferred too. 
> --
> Intel are signing my paycheques ... these opinions are still 
> mine "Bill, look, we understand that you're interested in 
> selling us this operating system, but compare it to ours.  We 
> can't possibly take such a retrograde step."
> 
> 
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 03/10] PCI: AMD SATA IDE mode quirk
  2008-02-25  1:43         ` Cai, Crane
@ 2008-02-25  7:31           ` Grant Grundler
  2008-02-25  8:00             ` Cai, Crane
  0 siblings, 1 reply; 7+ messages in thread
From: Grant Grundler @ 2008-02-25  7:31 UTC (permalink / raw)
  To: Cai, Crane
  Cc: Matthew Wilcox, Greg Kroah-Hartman, linux-pci, linux-kernel,
	Linus Torvalds, Andrew Morton

On Mon, Feb 25, 2008 at 09:43:59AM +0800, Cai, Crane wrote:
> > On Fri, Feb 22, 2008 at 01:49:20PM +0800, Cai, Crane wrote:
> > > > On Thu, Feb 21, 2008 at 03:47:33PM -0800, Greg 
> > Kroah-Hartman wrote:
> > > > > +static void __devinit quirk_amd_ide_mode(struct pci_dev *pdev)
> > > > >  {
> > > > > -	/* set sb600 sata to ahci mode */
> > > > > -	if ((pdev->class >> 8) == PCI_CLASS_STORAGE_IDE) {
> > > > > -		u8 tmp;
> > > > > +	/* set sb600/sb700/sb800 sata to ahci mode */
> > > > > +	u8 tmp;
> > > > >  
> > > > > +	pci_read_config_byte(pdev, PCI_CLASS_DEVICE, &tmp);
> > > > > +	if (tmp == 0x01) {
> > > > >  		pci_read_config_byte(pdev, 0x40, &tmp);
> > > > 
> > > > This seems like a dis-improvement.  Why are we reading a 
> > config byte 
> > > > for something we already have in the pci_dev?
> > > > Why are we now checking against 0x01 instead of a 
> > symbolic constant?  
> > > > Why are we no longer checking that this is PCI_BASE_CLASS_STORAGE?
> > > It is a quirk. In pci_ids.h did have PCI_CLASS_STORAGE_IDE and 
> > > PCI_BASE_CLASS_STORAGE, these can not represent the right 
> > situation we 
> > > want to check. 0x01 represents PCI_CLASS_STORAGE_IDE last 2 
> > bit. Also 
> > > because it is a quirk, I do not think we need to change 
> > pci_ids.h. So 
> > > 0x01 used.
> > 
> > You haven't explained what is wrong with the original code:
> > 
> > 	if ((pdev->class >> 8) == PCI_CLASS_STORAGE_IDE) {
> > 
> 
> When resume, this pdev->class is quirked, however BIOS has
> modified pci configuration too. Inconsistance occurs.

Can you update pdev->class from the quirk?
It would be consistent then.
That would leave the code as-is except it's re-reading the field
from config space.

hth,
grant

> > > > Nothing in the changelog entry suggests why we now need 
> > FIXUP_RESUME 
> > > > entries when we didn't before.
> > > > 
> > > PCI configuration space will be changed by BIOS and then in 
> > pci init 
> > > and restore. So resume also needed.
> > 
> > That information needed to be in the changelog.
> 
> This info, is a normal info. If maintainer need us to added in source code. I preferred too. 
> > --
> > Intel are signing my paycheques ... these opinions are still 
> > mine "Bill, look, we understand that you're interested in 
> > selling us this operating system, but compare it to ours.  We 
> > can't possibly take such a retrograde step."
> > 
> > 
> > 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH 03/10] PCI: AMD SATA IDE mode quirk
  2008-02-25  7:31           ` Grant Grundler
@ 2008-02-25  8:00             ` Cai, Crane
  0 siblings, 0 replies; 7+ messages in thread
From: Cai, Crane @ 2008-02-25  8:00 UTC (permalink / raw)
  To: Grant Grundler
  Cc: Matthew Wilcox, Greg Kroah-Hartman, linux-pci, linux-kernel,
	Linus Torvalds, Andrew Morton

> On Mon, Feb 25, 2008 at 09:43:59AM +0800, Cai, Crane wrote:
> > > On Fri, Feb 22, 2008 at 01:49:20PM +0800, Cai, Crane wrote:
> > > > > On Thu, Feb 21, 2008 at 03:47:33PM -0800, Greg
> > > Kroah-Hartman wrote:
> > > > > > +static void __devinit quirk_amd_ide_mode(struct pci_dev 
> > > > > > +*pdev)
> > > > > >  {
> > > > > > -	/* set sb600 sata to ahci mode */
> > > > > > -	if ((pdev->class >> 8) == PCI_CLASS_STORAGE_IDE) {
> > > > > > -		u8 tmp;
> > > > > > +	/* set sb600/sb700/sb800 sata to ahci mode */
> > > > > > +	u8 tmp;
> > > > > >  
> > > > > > +	pci_read_config_byte(pdev, PCI_CLASS_DEVICE, &tmp);
> > > > > > +	if (tmp == 0x01) {
> > > > > >  		pci_read_config_byte(pdev, 0x40, &tmp);
> > > > > 
> > > > > This seems like a dis-improvement.  Why are we reading a
> > > config byte
> > > > > for something we already have in the pci_dev?
> > > > > Why are we now checking against 0x01 instead of a
> > > symbolic constant?  
> > > > > Why are we no longer checking that this is 
> PCI_BASE_CLASS_STORAGE?
> > > > It is a quirk. In pci_ids.h did have PCI_CLASS_STORAGE_IDE and 
> > > > PCI_BASE_CLASS_STORAGE, these can not represent the right
> > > situation we
> > > > want to check. 0x01 represents PCI_CLASS_STORAGE_IDE last 2
> > > bit. Also
> > > > because it is a quirk, I do not think we need to change
> > > pci_ids.h. So
> > > > 0x01 used.
> > > 
> > > You haven't explained what is wrong with the original code:
> > > 
> > > 	if ((pdev->class >> 8) == PCI_CLASS_STORAGE_IDE) {
> > > 
> > 
> > When resume, this pdev->class is quirked, however BIOS has modified 
> > pci configuration too. Inconsistance occurs.
> 
> Can you update pdev->class from the quirk?
I think we can, there are many places where pdev->class is changed in quirks.c.
> It would be consistent then.
> That would leave the code as-is except it's re-reading the 
> field from config space.
> 
> hth,
> grant

> > > > > Nothing in the changelog entry suggests why we now need
> > > FIXUP_RESUME
> > > > > entries when we didn't before.
> > > > > 
> > > > PCI configuration space will be changed by BIOS and then in
> > > pci init
> > > > and restore. So resume also needed.
> > > 
> > > That information needed to be in the changelog.
> > 
> > This info, is a normal info. If maintainer need us to added 
> in source code. I preferred too. 
> > > --
> > > Intel are signing my paycheques ... these opinions are still mine 
> > > "Bill, look, we understand that you're interested in 
> selling us this 
> > > operating system, but compare it to ours.  We can't possibly take 
> > > such a retrograde step."
> > > 
> > > 
> > > 
> 
> 
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2008-02-25  8:02 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-02-21 23:46 [GIT PATCH] PCI fixes for 2.6.25-rc2 git tree Greg KH
     [not found] ` <1203637660-7247-3-git-send-email-gregkh@suse.de>
2008-02-22  4:17   ` [PATCH 03/10] PCI: AMD SATA IDE mode quirk Matthew Wilcox
2008-02-22  5:49     ` Cai, Crane
2008-02-22 11:11       ` Matthew Wilcox
2008-02-25  1:43         ` Cai, Crane
2008-02-25  7:31           ` Grant Grundler
2008-02-25  8:00             ` Cai, Crane

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).