archive mirror
 help / color / mirror / Atom feed
From: Greg KH <>
To: Matt Domsch <>
Subject: Re: [RFC][PATCH] Dynamic PCI Device IDs
Date: Wed, 30 Apr 2003 15:24:07 -0700	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

First off, nice idea, but I think the implementation needs a bit of

On Wed, Apr 30, 2003 at 04:45:14PM -0500, Matt Domsch wrote:
>   One may read the new_id file, which by default returns:
>   $ cat new_id
>   echo vendor device subvendor subdevice class classmask
>   where each field is a 32-bit value in ABCD (hex) format (no leading 0x).
>   Pass only as many fields as you need to override the defaults below.
>   Default vendor, device, subvendor, and subdevice fields
>   are set to FFFFFFFF (PCI_ANY_ID).
>   Default class and classmask fields are set to 0.

Ick, don't put help files within the kernel image.  Didn't you take them
all out for the edd patch a while ago?  :)

Just make it a write only file.

>   One can then cause the driver to probe for devices again.
>   echo 1 > probe_it

Why wouldn't the writing to the new_id file cause the probe to happen
immediatly?  Why wait?  So I think we can get rid of that file.

Also, do we really need to keep a list of id's visible to userspace (the
"0" file above?  We currently don't do that for the "static ids" (yeah I
know they are easily extracted from the module image...)

>   Individual device drivers may override the behavior of the new_id
>   file, for instance, if they need to also pass driver-specific
>   information.  Likewise, reading the individual dynamic ID files
>   can be overridden by the driver.

Why would a driver want to override these behaviors?

>   This also adds an existance test field to struct driver_attribute,
>   necessary because we only want the probe_it file to appear iff
>   struct pci_driver->probe is non-NULL.

Is that the exists() callback?  Is it really needed?  Can't the pci core
do this without needing to push that logic into the driver core?  After
all, it knows if the pci_driver->probe() call is non-NULL, the driver
core doesn't.

Also, we really need a generic way to easily create subdirectories
within the driver model, to keep you from having to dive down into
kobjects and the mess, like you had to do here.  Pat's said he will work
on this next, once he emerges from his OLS paper writing hole, and
there's even a bug assigned to him for this:
Once that is in, this patch should clean up a whole lot.  I'd recommend
waiting for that (or if you want to tackle it, please do) before
applying something like your patch.

Sound ok?


greg k-h

  parent reply	other threads:[~2003-04-30 22:09 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2003-04-30 21:45 [RFC][PATCH] Dynamic PCI Device IDs Matt Domsch
2003-04-30 21:53 ` Jeff Garzik
2003-04-30 22:24 ` Greg KH [this message]
2003-05-01  0:39 Matt_Domsch
2003-05-02 23:15 ` Greg KH
2003-05-05  5:37   ` Jeff Garzik
2003-05-06  0:17     ` Greg KH
2003-05-05 22:51   ` Matt Domsch
2003-05-06  0:15     ` Greg KH
2003-05-06  2:04 Matt_Domsch
2003-05-06  3:56 ` Greg KH
2003-05-06 16:35   ` Matt Domsch
2003-05-10  0:11     ` Greg KH
2003-05-13 21:28     ` Patrick Mochel
2003-05-13 21:33       ` Patrick Mochel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).