Subject: Re: Device Properties: The Next Generation
To: None <cgd@sibyte.com, eeh@netbsd.org>
From: None <eeh@netbsd.org>
List: tech-kern
Date: 03/07/2001 20:18:02
[...]

	Anyway, as should have been obvious, Chris Torek's 4.4BSD
	autoconfiguration documentation is interesting background, but does
	_not_ describe the state of autoconfiguration in NetBSD.

	At minimum, we've added a bunch of things and changed a bunch of
	interfaces, as we adapted the system to conditions that, as far as I
	can tell, were either not thought of, or thought of and not handled in
	the original design.  (I think one of the reasons for that may be
	because of the ... limited configuration environment provided on
	sparcs.  Everything looks the same, pretty much.  It's not a good
	example, as far as demonstration of configuration on many system
	types, but for sparc it probably makes sense.)

Then maybe this should be documented in, say, section 9.

[...]

	> 	(This kinda concerns me: you're not planning to go down the OFW-style
	> 	multi-value properties path, are you?  I could see some minor desire
	> 	for that, but it's not the path that we've taken with locators in the
	> 	config files up to this point... and you shouldn't start randomly
	> 	diverging from what the existing configuration code does.)
	> 
	> There's nothing to preclude multi-valued properties.  A few
	> multi-values properties are faster and easier to handle than
	> lots of single-values properties.  But I was not planning
	> to use multi-valued properties for locators. 

	Right, so, once you go down that road, you start having to worry about
	typing (especially if you want to represent them to users in some sane
	manner.)

You always need to worry about typing.  The easiet way to do that is
to have the user program that tries to extract the properties and present
them to the user know what the format is.  Creating self-descriptive
data structures in the kernel is a recipe for extreme kernel bloat.

	> 	> It may also make sense to combine all the locators as (name,
	> 	> value) pairs in a single property called `locators' so they
	> 	> can be easily extracted.
	> 
	> 	_easily_?  what would make that easy to extract?  8-)
	> 
	> How do you distingush a locator property from a non-locator 
	> property when trying to dump a device tree to a kernel config
	> file?

	Hmm.  Yes, good point.

	It seems that it would probably be easier and less work to implement
	to provide a 'locators' or 'locnames' property which included the
	_names_ of the locators (requiring whatever used that to dig them out
	on its own).

That would work.

	Another issue here is, if you're going to do this, you need to have
	some real correspondence between locator names and names in the config
	file.  It's not clear to me that the way multi-value locators are
	currently done makes sense if you're really trying to use locators as
	names.  (It's a bit difficult to use in code, too, since you don't
	really have a loop bound you can use...)

Huh?

	> My plan was to attach the properties to the cfdata the same
	> way locators are, only from different fields of the cfdata
	> structure so you can distinguish locators from other properites.  
	> Then they can be easily attached to the device node inside 
	> config_attach().  (Can't think of any reasonn the *match() 
	> routines would need access to properties.)

	I can, but doing property attachment for match is pretty painful (how
	many times do you copy the bloody things?).

The number of copies depends on the number of probes.  The number
of probes could be reduced if self-identifying buses would check
the device they found with the devices supported by the driver rather
than relying on locators and calling the device's match function.

	A couple of things that pop into mind, which may not actually apply
	but probably merit thought...

	* debug spew.  It may be desirable to turn on extra buff debugging in
	  a driver via a property.  If there's some desire to allow that
	  debugging output/logging/whatever to be turned on during probe...
	  (I'm not sure that this is kosher, but i can imagine that it'd be
	  useful.)

We're only talking about the *match() routine.  Anyway, since
changing a property requires re-running config, and recompiling
large parts of the kernel, it is much better to use a patchable
variable to control debugging.

	* better "locators."  In the fullness of time, it would be good if
	  locators ceased being 'int's.  In particular, at minimum, it can be
	  useful to represent integer values at least as wide as bus_addr_t.
	  (These might be real locators, described by parent bus being
	  attached to.  Absence of a 'real' locator in could file would mean
	  no property, i.e. doesn't use that resource at all, and wildcarded
	  value could be handled by e.g. 0-length.)

	  Also, strings would be good (for matching textual
	  strings, e.g. device serial number, product name string, ethernet
	  address, etc.  (These would probably be attached to the device
	  directly or because of non-attachment-related attributes,
	  e.g. 'ether', and control only that device -- so, really, they'd be
	  property-ish.  For these, absence from config file is a wildcard,
	  since they'll always be meaningful for that device, but it's
	  probably best to represent that 'no property.')

	  There's some question in my mind about how "real" locators "should"
	  be represented, i.e. as a set of properties, the current int-based
	  scheme with some adaptatation glue, or some migration of the current
	  int-based scheme to something slightly better (e.g. that can handle
	  wider addresses).  But it seems fairly obvious that the "property-ish"
	  set of things are reasonably represented as properties.

Changing the type of locators could easily be done with appropriate
modifications to config and the resulting ioconf.c.

The real issue is how to distinguish locators, which describe a 
device's location, from other properties which may describe a
device's capabilities, physical aspects (name), or control a
device's behavior.  If you are building a config file you want
some of them but not others.

	This, and the issue of "What's a locator" from above make me go back
	to different ideas...

	E.g., devices that have multiple sets of properties, one for
	'locators', and one for 'other stuff'.  (or two for locators: one for
	real ones, one for device-local ones, plus the rest...) Maybe
	distinguished by a flag (rather than some kind of separate
	container)...

Adding flags complicates things.  Although, I suppose the data
could have that information either prepended or appended to it.

	in the case of 'real locators' (which are defined by the bus being
	attached to), the parent bus would know to put them in the right set.
	In the case of property-ish config-specified values, the _child_ would
	know to put them in the right set.


	Thinking about the way different busses work, I think you'd want
	behaviour that worked something like:

You need to work on your terminology here.  It is confusing.

	direct config:

	    for each device found:

		parent sets up properties for found device, calls
			config_found().  (This attaches "normal properties"
			and "locators" to device.)

I expect you mean that the parent associates the location
of the device with the device node as properties, along with
other properties to control device behavior, etc.

		parent submatch compares properties for found device with
			"real" locators for device currently being examined.
			If "compatible," calls device match rtn.  (This
			examines properties of device, and "real" locators.)

By `"real" locators' I presume you mean the ones pointed
to by the cfdata struct.  (You know, there really is no
reason why dev_getprop() couldn't also search through
those as well when hunting for properties.  But that
would get complicated when trying to compare the
real value with the default value since the default
value would be hidden.)

		device match routine does whatever it wants with "real"
			locators, otherwise pokes at device, and
			uses "property-ish" locators to verify that the
			device-specific properties (e.g. enet addr)
			match.  (This examines properties of device,
			and property-ish locators.)

I think you're saying that the match routine compares the
properties attached to the device with the values found
through the cfdata struct.

		If device match succeeds, config_attach() is called.

		config_attach sets up the device.  It probably makes sense
			to have it copy the property-ish locators to the
			device (otherwise, attach would have to do it).

Wouldn't those have been set at the beginning?

	    done

	indirect config:

	    for each device in the config file (via config_search()):

		parent submatch/search fn sets up some properties for found
			device,	and copies real locators from cfdata.  it
			invokes child device match.  (attaches properties and
			locators to device, uses cfdata's "real" locators.)

		device match looks at those locators, possibly modifying some
			of them or adding some.  Uses property-ish locators
			to verify that device-specific properties match.

If the *match() routine modifies properties then the device
node cannot be immutable....

		if device match succeeds, config_attach() is called.  It does
			the same stuff as it does above.

	    done


	Some notes:

	* you could have config_found attach 'property-ish' locators to
	devices, if you delete them after every match.  That seems really
	painful in terms of efficiency, and code still has to be able to look
	up 'real' locators.

	* Indirect config is actually easy, since each device struct allocated
	basically has a lifetime of one device match fn...  no big inner loops
	here.


	I'd like to understand how you'd like this scenario to work.

	I consider this fairly important, because it's the obvious next
	extension to the properties framework (and it's been brought up
	previously).  You don't necessarily have to make the code do the right
	thing right now (though it's better to overhaul things once rather
	than twice), but it's important to understand how this goal would
	impact the existing design.

	If it doesn't fit without significant modification (code or
	conceptual), then that to me is an indication that something's
	wrong...

My current thoughts are as follows:

Direct config:

	Bus driver identifies a bus location.

	Bus driver allocates a device node.

	Bus driver attaches location information to the 
		device node as properties (`slot', 
		`device', `function', etc.)

	Bus driver attaches other necessary information 
		to the device node as properties such as 
		`bus_tag', etc.

	Bus driver calls config_found().

	config_search() attaches a property to the device
		node called `locator-names' that lists
		the names of all locators valid on that
		bus.

	config_search() calls the submatch routine, which
		compares the properties to what you can
		find in cfdata, and if they match calls
		the device match routine.

-or-

	config_search() calls the device match routine.

	The device match routine then compares properties
		to verify it supports that device and returns
		success or failure.

	If the match fails, config_search() removes the 
		`locator-names' property and tries again

	If all matches fail, config_found() destroys the
		device node and returns.

	If a match succeeds, config_attach() allocates a
		softc for the device (or allocates a new
		device w/softc and copies all properties
		to it and deletes the old device node).
		
	config_attach() attaches all the non-locator
		properties (currently flags) to the device
		node and calls the attach routine.

done.

Indirect config

	for each device in the config file (via config_search()):

	parent submatch/search fn adds properties to the
		device to describe or limit where the 
		child driver should search.

	device's match function looks at those properties,
		and maybe cfdata, and determines whether 
		the device exists at the specified location.
		If so, it attaches properties to describe the
		device and its location.

	if device match succeeds, config_attach() is called.  It does
		the same stuff as it does above.

	if all matches fail, the parent should destroy the
		child device node.

done

Eduardo