[PATCH] PCI: nVidia's MCP61 ethernet card needs quirk for wrong class

[PATCH] PCI: nVidia's MCP61 ethernet card needs quirk for wrong class

[Jiri Kosina]

From: Jiri Kosina <jkosina@suse.cz>

PCI: nVidia's MCP61 ethernet card needs quirk for wrong class

The MCP61 ethernet controller from nVidia (NVENET_19) contains wrong
PCI class:

	00:07.0 Bridge [0680]: nVidia Corporation MCP61 Ethernet [10de:03ef] (rev a2)

i.e. it identifies itself as a bridge. Fix this.

Signed-off-by: Jiri Kosina <jkosina@suse.cz>

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 50f2dd9..fbf6a89 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -1511,6 +1511,13 @@ static void __devinit fixup_rev1_53c810(struct pci_dev* dev)
 }
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_NCR, PCI_DEVICE_ID_NCR_53C810, fixup_rev1_53c810);
 
+/* nVidia's MCP61 ethernet card has wrong PCI class (0x0680) */
+static void __devinit fixup_nvenet_19(struct pci_dev *dev)
+{
+	dev->class = PCI_CLASS_NETWORK_ETHERNET << 8;
+}
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_NVIDIA, PCI_DEVICE_ID_NVIDIA_NVENET_19, fixup_nvenet_19);
+
 static void pci_do_fixups(struct pci_dev *dev, struct pci_fixup *f, struct pci_fixup *end)
 {
 	while (f < end) {

Re: [PATCH] PCI: nVidia's MCP61 ethernet card needs quirk for wrong class

[Jeff Garzik]

Jiri Kosina wrote:
> From: Jiri Kosina <jkosina@suse.cz>
> 
> PCI: nVidia's MCP61 ethernet card needs quirk for wrong class
> 
> The MCP61 ethernet controller from nVidia (NVENET_19) contains wrong
> PCI class:
> 
> 	00:07.0 Bridge [0680]: nVidia Corporation MCP61 Ethernet [10de:03ef] (rev a2)
> 
> i.e. it identifies itself as a bridge. Fix this.
> 
> Signed-off-by: Jiri Kosina <jkosina@suse.cz>

What is the problem that is present without this patch?

	Jeff

Re: [PATCH] PCI: nVidia's MCP61 ethernet card needs quirk for wrong class

[Jiri Kosina]

On Mon, 1 Oct 2007, Jeff Garzik wrote:

> > PCI: nVidia's MCP61 ethernet card needs quirk for wrong class
> > The MCP61 ethernet controller from nVidia (NVENET_19) contains wrong
> > PCI class:
> >  00:07.0 Bridge [0680]: nVidia Corporation MCP61 Ethernet [10de:03ef] (rev
> >  a2)
> > i.e. it identifies itself as a bridge. Fix this.
> > Signed-off-by: Jiri Kosina <jkosina@suse.cz>
> What is the problem that is present without this patch?

Userspace tools that are used to configure network are probable not to 
detect this device as a network card and therefore not provide means to 
configure the device (this is a case at least with yast, I don't know what 
is the situation with other configurators).

There might be also other situations, I don't know. Userspace really 
should know the proper class of the device, shouldn't it?

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH] PCI: nVidia's MCP61 ethernet card needs quirk for wrong class

[Jeff Garzik]

Jiri Kosina wrote:
> On Mon, 1 Oct 2007, Jeff Garzik wrote:
> 
>>> PCI: nVidia's MCP61 ethernet card needs quirk for wrong class
>>> The MCP61 ethernet controller from nVidia (NVENET_19) contains wrong
>>> PCI class:
>>>  00:07.0 Bridge [0680]: nVidia Corporation MCP61 Ethernet [10de:03ef] (rev
>>>  a2)
>>> i.e. it identifies itself as a bridge. Fix this.
>>> Signed-off-by: Jiri Kosina <jkosina@suse.cz>
>> What is the problem that is present without this patch?
> 
> Userspace tools that are used to configure network are probable not to 
> detect this device as a network card and therefore not provide means to 
> configure the device (this is a case at least with yast, I don't know what 
> is the situation with other configurators).
> 
> There might be also other situations, I don't know. Userspace really 
> should know the proper class of the device, shouldn't it?

There are other network devices that do not claim 
PCI_CLASS_NETWORK_ETHERNET either.  Since this is a purely cosmetic 
issue -- said userland tools would need to support weird cases _anyway_ 
  -- I am not inclined to apply the patch.

The kernel could do a lot to make things "prettier," but that would lead 
to lots of additional code bloat.  It's easier to export the world as it 
is, and let the chips fall where they may.

	Jeff

Re: [PATCH] PCI: nVidia's MCP61 ethernet card needs quirk for wrong class

[Jiri Kosina]

On Mon, 1 Oct 2007, Jeff Garzik wrote:

> There are other network devices that do not claim 
> PCI_CLASS_NETWORK_ETHERNET either.  

Nor any other PCI_CLASS_NETWORK subclass?

> Since this is a purely cosmetic issue -- said userland tools would need 
> to support weird cases _anyway_ -- I am not inclined to apply the patch.

What would you recommend as a method for userspace to detect whether given 
hardware is a network card, supposing that there is no driver bound to it 
yet?

> The kernel could do a lot to make things "prettier," but that would lead 
> to lots of additional code bloat.  It's easier to export the world as it 
> is, and let the chips fall where they may.

OK, I don't seem to agree with you on this matter, let's wait for Greg's 
oppinion.

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH] PCI: nVidia's MCP61 ethernet card needs quirk for wrong class

[Jeff Garzik]

Jiri Kosina wrote:
> On Mon, 1 Oct 2007, Jeff Garzik wrote:
> 
>> There are other network devices that do not claim 
>> PCI_CLASS_NETWORK_ETHERNET either.  
> 
> Nor any other PCI_CLASS_NETWORK subclass?

Correct.


>> Since this is a purely cosmetic issue -- said userland tools would need 
>> to support weird cases _anyway_ -- I am not inclined to apply the patch.
> 
> What would you recommend as a method for userspace to detect whether given 
> hardware is a network card, supposing that there is no driver bound to it 
> yet?

Note the mention of "weird cases"

You come up with a general method, then you have to deal with exceptions 
to that general method.

Another example:  8139too and 8139cp both claim to support 
(0x10EC,0x8139) PCI IDs.  The distro installer needs additional 
information to know to read the PCI revision, which is what determines 
whether or not to use 8139cp driver.

Sometimes life is just not as simple as you would like it to be :)

	Jeff

Re: [PATCH] PCI: nVidia's MCP61 ethernet card needs quirk for wrong class

[Jiri Kosina]

On Mon, 1 Oct 2007, Jeff Garzik wrote:

> > > There are other network devices that do not claim 
> > > PCI_CLASS_NETWORK_ETHERNET either.
> > Nor any other PCI_CLASS_NETWORK subclass?
> Correct.

BTW do you have some examples off the top of your head?

> > What would you recommend as a method for userspace to detect whether 
> > given hardware is a network card, supposing that there is no driver 
> > bound to it yet?
> Note the mention of "weird cases" You come up with a general method, 
> then you have to deal with exceptions to that general method.

Sure. Still don't see why to do it in kernel, when it is hardware-related 
though.

> Another example:  8139too and 8139cp both claim to support 
> (0x10EC,0x8139) PCI IDs.  The distro installer needs additional 
> information to know to read the PCI revision, which is what determines 
> whether or not to use 8139cp driver.

That's also pretty ugly, sure.

> Sometimes life is just not as simple as you would like it to be :)

You tell me :) There is a load of quirks for individual devices that claim 
themselves to be HID-compliant, but break the specs in various different 
creative ways :)

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH] PCI: nVidia's MCP61 ethernet card needs quirk for wrong class

[Greg KH]

On Mon, Oct 01, 2007 at 05:24:37PM +0200, Jiri Kosina wrote:
> On Mon, 1 Oct 2007, Jeff Garzik wrote:
> > > What would you recommend as a method for userspace to detect whether 
> > > given hardware is a network card, supposing that there is no driver 
> > > bound to it yet?
> > Note the mention of "weird cases" You come up with a general method, 
> > then you have to deal with exceptions to that general method.
> 
> Sure. Still don't see why to do it in kernel, when it is hardware-related 
> though.

I'll defer to Jeff's judgement here.  If the device is broken such that
it does not know to return the correct class, then we need to be able to
have a "quirk" table in the userspace tools to handle the list of
devices that are broken in that way.

For config items, we don't want to modify them in any way, we just want
to pass-through the data direct from the device.  Look at the sysfs and
proc files for pci config spaces.  They are returning binary data stored
in the device directly.  We do not want to have to start modifying that
data directly just to handle messed up devices like this.

So this should be handled by yast or other userspace programs properly,
not by changing the kernel to lie.

> > Another example:  8139too and 8139cp both claim to support 
> > (0x10EC,0x8139) PCI IDs.  The distro installer needs additional 
> > information to know to read the PCI revision, which is what determines 
> > whether or not to use 8139cp driver.
> 
> That's also pretty ugly, sure.
> 
> > Sometimes life is just not as simple as you would like it to be :)
> 
> You tell me :) There is a load of quirks for individual devices that claim 
> themselves to be HID-compliant, but break the specs in various different 
> creative ways :)

That is done purely due to the way that Microsoft wrote their operating
system.  The only way to write a userspace USB driver on windows is to
have it claim to be a HID device.  Then the kernel driver correctly
grabs it, and allows userspace programs to take over control of it.

That is why we have such a huge blacklist of HID devices in our kernel,
and so does Apple.  All because Windows can not easily handle something
we have been doing for over 7 years now.

And yes, you can now write userspace drivers for all USB devices on
Windows now, but you still have to get your userspace-driver signed, and
the interface is so horrible it's not even funny, so companies are still
avoiding it and using the old "make it claim to be a hid device" hack
for new devices.

So the HID table examples really have nothing to do with the broken
network device, as the HID devices are purposfully created that way.
For this network device, someone just messed up at the manufacturer :)

thanks,

greg k-h

Re: [PATCH] PCI: nVidia's MCP61 ethernet card needs quirk for wrong class

[Matthew Wilcox]

On Mon, Oct 01, 2007 at 09:25:46AM -0700, Greg KH wrote:
> I'll defer to Jeff's judgement here.  If the device is broken such that
> it does not know to return the correct class, then we need to be able to
> have a "quirk" table in the userspace tools to handle the list of
> devices that are broken in that way.

I think Jeff is right on this particular point -- better to have this
worked around in the userspace tool than in the kernel.

> For config items, we don't want to modify them in any way, we just want
> to pass-through the data direct from the device.  Look at the sysfs and
> proc files for pci config spaces.  They are returning binary data stored
> in the device directly.  We do not want to have to start modifying that
> data directly just to handle messed up devices like this.

Nobody's talking about changing the raw pci data.  What we're talking
about is changing the value read out of

$ cat /sys/bus/pci/devices/0000\:00\:00.0/class 
0x060000

We do tweak at least some of the other similar values (vendor, for sure)
to work around bugs; not sure we have an example for changing the class
value yet.

pciutils knows that the kernel has potentially fixed up the values in
the individual files, and to trust those over the raw data.

-- 
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."

Re: [PATCH] PCI: nVidia's MCP61 ethernet card needs quirk for wrong class

[Greg KH]

On Mon, Oct 01, 2007 at 11:03:38AM -0600, Matthew Wilcox wrote:
> On Mon, Oct 01, 2007 at 09:25:46AM -0700, Greg KH wrote:
> > For config items, we don't want to modify them in any way, we just want
> > to pass-through the data direct from the device.  Look at the sysfs and
> > proc files for pci config spaces.  They are returning binary data stored
> > in the device directly.  We do not want to have to start modifying that
> > data directly just to handle messed up devices like this.
> 
> Nobody's talking about changing the raw pci data.  What we're talking
> about is changing the value read out of
> 
> $ cat /sys/bus/pci/devices/0000\:00\:00.0/class 
> 0x060000

I know, but the next request will be to also fix up those raw values as
someone will have a tool that only uses them :)

> We do tweak at least some of the other similar values (vendor, for sure)
> to work around bugs; not sure we have an example for changing the class
> value yet.
> 
> pciutils knows that the kernel has potentially fixed up the values in
> the individual files, and to trust those over the raw data.

Ah, cool, I didn't realize that pciutils handled this kind of mismatch.
That's good to know.

thanks,

greg k-h