Subject: Re: Device Properties: The Next Generation
To: None <eeh@netbsd.org>
From: Chris G. Demetriou <cgd@sibyte.com>
List: tech-kern
Date: 03/07/2001 13:53:53
eeh@netbsd.org writes:
> 	Looked at another way, CACHEABLE, LINEAR, etc., don't neceesarily have
> 	to be stored.  STRING and CONST do (as far as I can tell).
> 
> Both PROP_STRING and PROP_CONST are used to control how a
> property is created rather than stored in the property.

Huh?  PROP_STRING and PROP_CONST both affect storage.

PROP_STRING must at least be stored with the property, in some obvious
form.

PROP_CONST must also be stored, and also completely munges the
underlying storage mechanism...



> I disagree.  If it has a valid parent pointer it should be in
> the tree.  If it's not in the tree then it shouldn't have a
> valid parent pointer and we need to continue to pass in the
> `parent' device pointer to both the match and attach functions.
> Otherwise it's in a state of semi-insertion that really does
> not make sense.

So, there are a couple of things to say to this.

I think the big problem here is that several things are being confused
in this interface.

(1) access to parent.  In general it's not appropriate, but some
devices need it.  So they have to be accomodated in some manner.

(2) property search chain.

(3) timing / creation / use of new device.


Right now, a new 'struct device' is created early as a dummy, and is
the property container.  It's reasonably used for (1) and (2) if it's
got a link to the parent made.

However, until it's attached, it should _not_ be linked into the tree
of devices.  It's not a device.

You may call that 'semi-insertion,' but I don't really think it is
(any more than a separate property container making a reference to a
parent property container would be).


If you want to clean up this interface so that it doesn't do this
"semi-insertion" the right way to do it is:

(1) allocate a child only in config_attach().  There are advantages to
this anyway, it'll avoid some confusion that the changes you're
suggesting would introduce.

(2) _not_ pass a child to the match routine, do pass the parent.

(3) make property containers first-class objects.

(4) pass one in to the match routine (like aux is now, in addition to
aux), with the inheritance info pointing at the parent device's
property container.

Note that if you're trying to hang properties off of cfdata for
locators, it's fairly obvious how you'd get them into the various
functions, and use them as well.

> 	uh oh, how do you nuke the device?  You pretty much have to -- if you
> 	were doing a copy, the state is now ... not well defined.
> 
> 	dev_detach() isn't the right thing -- it's not an attached device yet.
> 
> It's in the tree in an unconfigured state,

Nothing that has not been fully "configured" (config_attach()ed)
should be (completely) linked into the tree.

> but if you want a
> separate dev_destroy() that would be simple to add.  But then
> dev_detach() should not be destroying the device node if you 
> want a nice orthogonal interface.

That's an interesting point.  It could substantially complicate
things.

I really think i'm leaning back towards devices only being created in
config_attach() or its equivalent, i.e. you don't pass devices around
until they're devices.




> 	> 	(2) the weird cases need to be shot in the head.
> 	> 
> 	> 	(3) the semantics for the seemingly-normal case of "things that don't
> 	> 	want to pass any aux info/properties" (which typically do all the work
> 	> 	in the cfmatch_t they provide to config_search()) needs to be figured
> 	> 	out.
> 	> 
> 	> Don't ask me.  I just provided the interface because some buses
> 	> use this directly.
> 
> 	The problem is, not all of the existing uses will map to it...  As
> 	part of a proposal like this, you really need to think about how
> 	problems like those get solved with the new system.
> 
> 	It's actually kinda disturbing that not only did you not seem to
> 	notice this stuff before, but you sound like you want to punt on it
> 	now...
> 
> If you use indirect config, you are taking responsibility
> for all the config details.  So when things change the
> drivers need to be chaged as appropriate.  
> 
> That's the reason to send all this to tech-kern.

Huh?  How does that statement _not_ apply to _direct_ config?

Both use a set of interfaces supplied by the configuration framework.
The set of functions provided by the configuration framework _should_
apply to both.  Maybe you don't call all functions for each, but the
majority of the underlying guts should be common.

If you're going to suggest seriously changing the configuration
architecture, it's your responsibility to provide examples of how
existing code should adapt to the new architecture.  That's true for
both direct and indirect config.  And, both must work reasonably well,
and not be significantly harder to implement than they are now.  (In
general, the gain/pain tradeoff can be applied, but there shouldn't be
much difference in pain to convert one type of bus or another.)


> 	> I think calling config_search() directly is
> 	> a Bad Thing.  I would be happy to make config_search() an internal
> 	> interface only.
> 
> 	uh... how would you implement indirect-config busses, except via
> 	config_search()?
> 
> I thought that the submatch function was added to config_found()
> specifically to support indirect-config through config_found().
> Or are there problems with doing that?

That is not correct.  Indirect config would work fine with the
interfaces that were originally designed/implemented by Chris T.

config_found() is _never_ used for indirect config devices.
submatch is used to do a check of the config-supplied locators against
the ones actually supplied by the bus.  You can think of submatch
functions for direct config as code which would otherwise have to be
in every single driver attached to a given direct config bus.

It just doesn't make sense to try to use config_found() for indirect
config.  direct config / config_found says "I have found this device,
find me a driver which matches it."

Indirect config says "Scan over the list of all devices specified in
the config file as children of this bus, checking to see if they're
there."

Certainly, there's common code that's shared (in particular,
config_search() 8-), but the end result and the level of control
that's needed are just different for each.


> 	This has to be figured out; a lot of busses rely on it.  Unless you're
> 	going to introduce a new mechanism that does the same type of thing we
> 	would lose support for, oh, ISA, VME, who knows what else -- those are
> 	the two that jump out at me.
> 
> So where are the ISA and VME experts?

Huh?  For the purposes of autoconfiguration, I definitely qualify for
the former, and can probably do the latter (since it's about the
same).



> 	> 	Also, this just occurred to me: how do cfmatch_t, cfprint_t, et
> 	> 	al. change? 8-)
> 	> 
> 	> Hm.  They may need changes to take a struct device* rather than
> 	> an `aux'.
> 
> 	or, in transition, both, as previously noted.  print doesn't have
> 	enough info to tell what it's been passed, otherwise...
> 
> cfprintt_t is caled from config_found() and config_attach().  Both
> of those routines know what the parent bus is and what form of 
> form of arguments should be appropriate to the print function.
> 
> Calling a *print() routine from a child driver is an extremely
> questionable practice.

So, say you're _in_ the print routine.

If all you get are a void * and a parent name pointer, how do you know
what that void * is (if it could be a 'struct device *' or an aux
ptr)?

Right now you know, because it's always an aux ptr.


> 	> 	does it make sense to have dev_prop_cmp to compare two named
> 	> 	properties?  also, possibly dev_prop_cmpdata to compare a property
> 	> 	agains known data?
> 	> 
> 	> No, I don't think so.  I can't see them ever being used. 
> 	> And it violates the definition of a property: (dev, name, data).
> 	> If any of the three is different, it's a different property. 
> 
> 	That makes it harder to do submatch fns...
> 
> For the most part, comparing a locator to a property on the
> device node could easily be delegated to MI code rather than
> the sbmatch function.

For the most part?  It's one of those things where, either you provide
the hook or you don't.

I think I agree, actually.  It's probably possible for MI code to do
the property sanity check that is normally done by direct-config
submatch functions.


> In fact, we self-identifying buses could
> provide a specific device identifier to the MI code to allow 
> config_search() to determine the exact match without calling
> submatch functions.  But the increased search speed gained
> by doing somethi like that is probably not worth the effort.

I'm not sure what you mean.

completely replace the device match routine, you can _almost_ do
that.  (cfdriver used to have an 'aux' member, which was used by BSDI
to point to e.g. EISA IDs).

However, match also (arguably) has the responsibility to check for
resource availability, etc.  A non-device-specific match routine can't
really satisfy that.


> 	I actually think the type checking thing is somewhat
> 	important... because e.g. you're changing the interfaces pretty
> 	heavily, you don't necessarily want unmodified old interfaces to drop
> 	in & compile when completely converted.
> 
> Since the transitional version will require casting the functions
> to stuff them in the cfattach structure, I don't think you will
> get strong typechecking until the old inface is removed.

Sure, but then you will.  (and the removal should be pretty easy, just
flip a switch and run unifdef, if people code carefully...)


> O.K.  No locators added automagically.  Should they be compared automagically?

There's something to be said for this.  I think i understand how this
might work (see earlier this mail or previous -- i got distracted by
other stuff, and had to stop responding for a bit).



> We're making `parent' go away?  O.K.  If we do that it should
> be handled in the initial conversion rather than later where
> it will cause more breakage.  If `parent' will not be needed
> in the future then it should not be needed right now.

(1) see comments in other mail,

(2) you still need backward compat for not-converted drivers, but I
think we all know that.  8-)



> 	the search semantics should be well-defined, and (i thought) were.
> 	That's the only way you can guarantee what "the correct property" is,
> 	in fact.
> 
> Well, if the property returned breaks search order semantics, it
> is, by definition, not "the correct property".  The search order
> semantics are well defined for properties that exist in the device
> tree.  dev_getmdprop should function a manner appropriate to those
> semantics.

I'll buy that.  It's an "as if" thing.  the md get prop routine
should work "as if" it searches only the given device's properties.
If it can optimize that while maintaining the search semantics, more
power to it.  8-)


chris