linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Greg KH <greg@kroah.com>
To: Matt_Domsch@Dell.com
Cc: alan@redhat.com, linux-kernel@vger.kernel.org, jgarzik@redhat.com
Subject: Re: [RFC][PATCH] Dynamic PCI Device IDs
Date: Fri, 2 May 2003 16:15:58 -0700	[thread overview]
Message-ID: <20030502231558.GA16209@kroah.com> (raw)
In-Reply-To: <1051749599.20870.234.camel@iguana.localdomain>

On Wed, Apr 30, 2003 at 07:39:57PM -0500, Matt_Domsch@Dell.com wrote:
> > First off, nice idea, but I think the implementation needs a bit of
> > work.
> 
> Thanks.  I didn't expect it to be perfect first-pass.
> Let me answer some questions out-of-order, maybe that will help.
> 
> > > 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.
> 
> That was my first idea, but Jeff said:
> http://marc.theaimsgroup.com/?l=linux-kernel&m=104681922317051&w=2
>         I think there is value in decoupling the two operations:
>         
>         	echo "0x0000 0x0000 0x0000 0x0000 0x0000 0x0000" >
> .../3c59x/table
>         	echo 1 > .../3c59x/probe_it
>         
>         Because you want the id table additions to be persistant in the face of
>         cardbus unplug/replug, and for the case where cardbus card may not be
>         present yet, but {will,may} be soon.

But by adding the device ids, they will be persistant, for that driver,
right?  Then when the device is plugged in, the core will iterate over
the static and dynamic ids, right?  If so, I don't see how a "probe_it"
file is needed.

> > >  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?
> 
> Because the one field I'm not filling in by default is the opaque
> unsigned long driver_data.  Most drivers don't need it, and those that
> do tend to come in two camps:  those that put a single integer there
> which is an internal table lookup for equivalancy, and those that put a
> pointer there to something (which definitely shouldn't be passed from
> userspace).  There aren't many of the latter (which is good), but I
> didn't want them to break with the introduction of this patch.  They
> should be recoded to to a table lookup, but that's beyond the scope I
> wanted to deal with today. :-)
> 
> That said, if drivers implement their own write routines, I wanted to
> give them a way to programatically expose what data should be written,
> and how.   I'll grant that the current help text isn't programatically
> helpful ATM.

Ah, can't you just not worry about that driver_data field somehow?  Like
say, "Any driver that depends on it, can't use the dynamic_id"? :)

I know, wishful thinking.  But I still don't think you should be
cluttering up the driver core with that, it should be able to be
localized within the pci code somehow.

> > Ick, don't put help files within the kernel image.  Didn't you take
> > them all out for the edd patch a while ago?  :)
> 
> If we resolve the above, I'll be happy to nuke them.

Either way, we shouldn't have help files within the kernel.  I'd say
nuke them, and write up some good documentation :)

> > 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...)
> 
> That's the only reason I know - so one can always write an app to
> retreive what IDs a given module has, both static and dynamic.  I don't
> think it's critical to keep.

I'd say drop it for now, it keeps the code simpler.  If people _really_
cry out for some way to retrieve them later, we can worry about that
then.

thanks,

greg k-h

  reply	other threads:[~2003-05-02 23:01 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2003-05-01  0:39 Matt_Domsch
2003-05-02 23:15 ` Greg KH [this message]
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
  -- strict thread matches above, loose matches on Subject: below --
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
2003-04-30 21:45 Matt Domsch
2003-04-30 21:53 ` Jeff Garzik
2003-04-30 22:24 ` Greg KH

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:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

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

  git send-email \
    --in-reply-to=20030502231558.GA16209@kroah.com \
    --to=greg@kroah.com \
    --cc=Matt_Domsch@Dell.com \
    --cc=alan@redhat.com \
    --cc=jgarzik@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --subject='Re: [RFC][PATCH] Dynamic PCI Device IDs' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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