Re: [PATCH 3/6] PCI legacy I/O port free driver (take2) - Add device_flags into pci_device_id

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Greg KH wrote:
On Tue, Feb 21, 2006 at 07:09:08PM -0500, Jeff Garzik wrote:

Andi Kleen wrote:

On Tuesday 21 February 2006 22:55, Jeff Garzik wrote:



It doesn't matter how easily its added, it is the wrong place to add such things.

This is what the various functions called during pci_driver::probe() do...


The problem is that at least on the e1000 it only applies to some of the many PCI-IDs it supports. So the original patch had an long ugly switch
with PCI IDs to check it. I suggested to use driver_data for it then,
but Kenji-San ended up with this new field.  I actually like the idea
of the new field because it would allow to add such things very easily
without adding lots of code.

it's not an uncommon situation. e.g. consider driver A which supports
a lot of PCI-IDs but MSI only works on a few of them. How do you
handle this? Add an ugly switch that will bitrot? Or put all the information into a single place which is the pci_device_id array.

You do what tons of other drivers do, and indicate this via driver_data. An enumerated type in driver_data can be used to uniquely identify any device or set of devices.

No need to add anything.


Yes, I agree, use driver_data, it's simpler, and keeps the PCI core
clean.

thanks,

greg k-h


Hi,

Thank you very much for all of the comments.

The reason why I added the new field into struct pci_device_id is
as follows, though these are already explained completely by Andi.

   - By adding the new field instead of using driver_data, drivers
     can pass the flags to the kernel directly. That is, drivers
     don't need to call any new APIs to tell something to the kernel
     after checking their own driver_data in their .probe().

   - This new field can be used not only for ioport but also
     other purpose (e.g. MSI).

But according to the discussion, it doesn't look a good idea to add
the new field into pci_device_id this time. So I'll update my patches
to use the driver_data field instead.

Thanks,
Kenji Kaneshige
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux