linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Matt_Domsch@Dell.com
To: greg@kroah.com
Cc: alan@redhat.com, linux-kernel@vger.kernel.org, jgarzik@redhat.com
Subject: Re: [RFC][PATCH] Dynamic PCI Device IDs
Date: Wed, 30 Apr 2003 19:39:57 -0500	[thread overview]
Message-ID: <1051749599.20870.234.camel@iguana.localdomain> (raw)

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

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

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

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


> Is that the exists() callback?

Yes.

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

I'll look into this.  I did something similar in the EDD code, so I
reused the idea again.

> 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:
>         http://bugme.osdl.org/show_bug.cgi?id=650
> Once that is in, this patch should clean up a whole lot.  I'd
> recommend waiting for that

Yes, that's fine.  I'd love to have that ability.  I did it the way Pat
wanted it a couple months ago.
http://marc.theaimsgroup.com/?l=linux-kernel&m=104678872809999&w=2

> (or if you want to tackle it, please do) before applying something
> like your patch.

I'm srue Pat will do a fine job. :-)

Thanks for the comments.
-Matt


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



             reply	other threads:[~2003-05-01  0:27 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2003-05-01  0:39 Matt_Domsch [this message]
2003-05-02 23:15 ` [RFC][PATCH] Dynamic PCI Device IDs 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
  -- 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=1051749599.20870.234.camel@iguana.localdomain \
    --to=matt_domsch@dell.com \
    --cc=alan@redhat.com \
    --cc=greg@kroah.com \
    --cc=jgarzik@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    /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
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).