> 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- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Wed Apr 30 2003 - 22:00:37 EST