PCI devices with no PCI_CACHE_LINE_SIZE implemented

PCI devices with no PCI_CACHE_LINE_SIZE implemented

[Matt Domsch]

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

Greg,

Some PCI device functions, such as the EHCI portion of Intel ICH5 and
ICH6 chips, do not implement the PCI_CACHE_LINE_SIZE register (which
is legal to not implement per PCI spec as it is a busmaster that
cannot issue a MWI).  However, for each of these, the kernel tries to
set the value, fails, and prints a KERN_WARNING message about it.

a) need this be a warning, wouldn't KERN_DEBUG suffice, if a message
is needed at all?  This is printed in pci_generic_prep_mwi().

b) How might you prefer to handle such devices?

Per the PCI 2.3 spec, reading a value of 0 may mean several things:
1) setting the register at all isn't supported
   - this is what pci.c assumes now and returns -EINVAL.
2) setting the register to the value you tried isn't supported, but
   you can try again with another value until you find the right one.
   - but there are no hints as to what the right value for a device
     might be.

Personally, I'd be happy if the printk were gone completely, but
that's your call.  With the prints there, our support organization is
concerned it can cause tech support calls for (in this case) a non-issue. 

Thanks,
Matt

-- 
Matt Domsch
Sr. Software Engineer, Lead Engineer
Dell Linux Solutions linux.dell.com & www.dell.com/linux
Linux on Dell mailing lists @ http://lists.us.dell.com

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: PCI devices with no PCI_CACHE_LINE_SIZE implemented

[Greg KH]

On Thu, Apr 29, 2004 at 02:53:01PM -0500, Matt Domsch wrote:
> Greg,
> 
> Some PCI device functions, such as the EHCI portion of Intel ICH5 and
> ICH6 chips, do not implement the PCI_CACHE_LINE_SIZE register (which
> is legal to not implement per PCI spec as it is a busmaster that
> cannot issue a MWI).  However, for each of these, the kernel tries to
> set the value, fails, and prints a KERN_WARNING message about it.
> 
> a) need this be a warning, wouldn't KERN_DEBUG suffice, if a message
> is needed at all?  This is printed in pci_generic_prep_mwi().

Yes, we should make that KERN_DEBUG.  I don't have a problem with that.
Care to make a patch?

> b) How might you prefer to handle such devices?
> 
> Per the PCI 2.3 spec, reading a value of 0 may mean several things:
> 1) setting the register at all isn't supported
>    - this is what pci.c assumes now and returns -EINVAL.
> 2) setting the register to the value you tried isn't supported, but
>    you can try again with another value until you find the right one.
>    - but there are no hints as to what the right value for a device
>      might be.

I think we need to stick with 1, unless we get more info on what the
"proper" value should be.

thanks,

greg k-h

Re: PCI devices with no PCI_CACHE_LINE_SIZE implemented

[Matt Domsch]

On Wed, May 05, 2004 at 03:31:02PM -0700, Greg KH wrote:
> On Thu, Apr 29, 2004 at 02:53:01PM -0500, Matt Domsch wrote:
> > a) need this be a warning, wouldn't KERN_DEBUG suffice, if a message
> > is needed at all?  This is printed in pci_generic_prep_mwi().
> 
> Yes, we should make that KERN_DEBUG.  I don't have a problem with that.
> Care to make a patch?

Appended for 2.6.6-rc3.  I'll send a 2.4.x patch separately.

Thanks,
Matt

-- 
Matt Domsch
Sr. Software Engineer, Lead Engineer
Dell Linux Solutions linux.dell.com & www.dell.com/linux
Linux on Dell mailing lists @ http://lists.us.dell.com

===== drivers/pci/pci.c 1.65 vs edited =====
--- 1.65/drivers/pci/pci.c	Fri Mar 26 10:58:01 2004
+++ edited/drivers/pci/pci.c	Wed May  5 17:39:08 2004
@@ -640,7 +640,7 @@
 	if (cacheline_size == pci_cache_line_size)
 		return 0;
 
-	printk(KERN_WARNING "PCI: cache line size of %d is not supported "
+	printk(KERN_DEBUG "PCI: cache line size of %d is not supported "
 	       "by device %s\n", pci_cache_line_size << 2, pci_name(dev));
 
 	return -EINVAL;

Re: PCI devices with no PCI_CACHE_LINE_SIZE implemented

[Matt Domsch]

On Wed, May 05, 2004 at 03:31:02PM -0700, Greg KH wrote:
> On Thu, Apr 29, 2004 at 02:53:01PM -0500, Matt Domsch wrote:
> > a) need this be a warning, wouldn't KERN_DEBUG suffice, if a message
> > is needed at all?  This is printed in pci_generic_prep_mwi().
> 
> Yes, we should make that KERN_DEBUG.  I don't have a problem with that.
> Care to make a patch?

Patch for 2.4.27-pre1 appended.

Thanks,
Matt

-- 
Matt Domsch
Sr. Software Engineer, Lead Engineer
Dell Linux Solutions linux.dell.com & www.dell.com/linux
Linux on Dell mailing lists @ http://lists.us.dell.com

===== drivers/pci/pci.c 1.47 vs edited =====
--- 1.47/drivers/pci/pci.c	Mon Sep 22 07:27:35 2003
+++ edited/drivers/pci/pci.c	Wed May  5 17:49:13 2004
@@ -943,7 +943,7 @@
 	if (cacheline_size == pci_cache_line_size)
 		return 0;
 
-	printk(KERN_WARNING "PCI: cache line size of %d is not supported "
+	printk(KERN_DEBUG "PCI: cache line size of %d is not supported "
 	       "by device %s\n", pci_cache_line_size << 2, dev->slot_name);
 
 	return -EINVAL;

Re: PCI devices with no PCI_CACHE_LINE_SIZE implemented

[Greg KH]

On Wed, May 05, 2004 at 05:48:47PM -0500, Matt Domsch wrote:
> On Wed, May 05, 2004 at 03:31:02PM -0700, Greg KH wrote:
> > On Thu, Apr 29, 2004 at 02:53:01PM -0500, Matt Domsch wrote:
> > > a) need this be a warning, wouldn't KERN_DEBUG suffice, if a message
> > > is needed at all?  This is printed in pci_generic_prep_mwi().
> > 
> > Yes, we should make that KERN_DEBUG.  I don't have a problem with that.
> > Care to make a patch?
> 
> Appended for 2.6.6-rc3.  I'll send a 2.4.x patch separately.

I've applied this for 2.6.  I'm not the 2.4 pci maintainer, so I can't
help you out there, sorry.

greg k-h

Re: PCI devices with no PCI_CACHE_LINE_SIZE implemented

[Matt Domsch]

Martin, below is a patch to lower the severity of this printk to
KERN_DEBUG, as there are devices which (properly) don't implement the
PCI_CACHE_LINE_SIZE register, and it's not a bug, so don't print at a
WARNING level.

Thanks,
Matt


----- Forwarded message from Matt Domsch <Matt_Domsch@dell.com> -----

Date: Wed, 5 May 2004 17:50:53 -0500
From: Matt Domsch <Matt_Domsch@dell.com>
To: Greg KH <greg@kroah.com>
Cc: linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org
Subject: Re: PCI devices with no PCI_CACHE_LINE_SIZE implemented
In-Reply-To: <20040505223102.GF30003@kroah.com>

On Wed, May 05, 2004 at 03:31:02PM -0700, Greg KH wrote:
> On Thu, Apr 29, 2004 at 02:53:01PM -0500, Matt Domsch wrote:
> > a) need this be a warning, wouldn't KERN_DEBUG suffice, if a message
> > is needed at all?  This is printed in pci_generic_prep_mwi().
> 
> Yes, we should make that KERN_DEBUG.  I don't have a problem with that.
> Care to make a patch?

Patch for 2.4.27-pre1 appended.

Thanks,
Matt

-- 
Matt Domsch
Sr. Software Engineer, Lead Engineer
Dell Linux Solutions linux.dell.com & www.dell.com/linux
Linux on Dell mailing lists @ http://lists.us.dell.com

===== drivers/pci/pci.c 1.47 vs edited =====
--- 1.47/drivers/pci/pci.c	Mon Sep 22 07:27:35 2003
+++ edited/drivers/pci/pci.c	Wed May  5 17:49:13 2004
@@ -943,7 +943,7 @@
 	if (cacheline_size == pci_cache_line_size)
 		return 0;
 
-	printk(KERN_WARNING "PCI: cache line size of %d is not supported "
+	printk(KERN_DEBUG "PCI: cache line size of %d is not supported "
 	       "by device %s\n", pci_cache_line_size << 2, dev->slot_name);
 
 	return -EINVAL;

Re: PCI devices with no PCI_CACHE_LINE_SIZE implemented

[Jeff Garzik]

Matt Domsch wrote:
> Martin, below is a patch to lower the severity of this printk to
> KERN_DEBUG, as there are devices which (properly) don't implement the
> PCI_CACHE_LINE_SIZE register, and it's not a bug, so don't print at a
> WARNING level.
> 
> Thanks,
> Matt

I'd send it straight to Marcelo, saying that the 2.6 PCI maintainer 
approved it....  Martin hasn't hacked on kernel code in a while...

	Jeff

Re: PCI devices with no PCI_CACHE_LINE_SIZE implemented

[Martin Mares]

Hi!

> I'd send it straight to Marcelo, saying that the 2.6 PCI maintainer 
> approved it....  Martin hasn't hacked on kernel code in a while...

Anyway, looks OK for me :)

				Have a nice fortnight
-- 
Martin `MJ' Mares   <mj@ucw.cz>   http://atrey.karlin.mff.cuni.cz/~mj/
Faculty of Math and Physics, Charles University, Prague, Czech Rep., Earth
Next lecture on time travel will be held on previous Monday.