eeh@netbsd.org writes:
	> 3	Description
	> 
	> A property is a tuple that consists of a pointer to a device node (struct
	> device *), string, and an arbitrary amount of data.  This tuple is established
	> by dev_setprop(), retrieved by dev_getprop(), and destroyed by dev_delprop().  In
	> addition, there is a machine dependent hook, md_getprop()[1] that is called if
	> attempting to retrieve a property fails.  md_getprop() can then use any
	> arbitrary method to generate property data if it so desires, or cause a failure
	> to be reported.
	> 
	> This functionality will be implemented in two phases.  The first phase will
	> add these functions, but retain the existing behavior for compatilbility.
	> dev_create(9) will generate a dummy device node that the parent can use to 
	> attach properties to.  config_attach*(9) functions will then create the real
	> device node and migrate properties to it.

	s/migrate/copy/ please.

Fine, `copy'.

	> The second phase will separate the `struct device' from the device's softc.
	> dev_create(9) will create the true device node, which config_found*(9) will
	> attach to the device tree.

	It's not clear that you want to go that far.  I still think you want:

	(1) fake device is mostly a container,

	(2) config_found or dev_config_found() or whatever makes a new one and
	attaches it.

	The problem is again one of handling of non-configured devices.  It's
	just easier for code be be written as:

		create properties container
		config_search(... properties container ...)
		destroy properties container

	or:

		create properties container
		set up properties that will never change
		loop {
			set up properties that change from dev to dev.
			set immutable
			config_found(... properties container ...)
			clear immutable
		}
		destroy properties container.

	than it is e.g.:

		create properties container
		if (config_search(... properties_container) == NULL)
			destroy_properties_container

	or similar for the second example.

	Having to worry about freeing depending on whether or not the device
	actually was attached is a pain, and pains mean that the code will get
	written wrong, and the code getting written wrong will mean probable
	memory leaks or data structure corruption.

	Yeah, reusing the storage may be a bit more efficient, but we've
	pretty much already said that for this interface efficiency is not a
	primary concern.  I'd rather have something which makes code
	correctness easier.


	To this end, I suggest you make the following changes to existing
	device stuff (in addition to adding the properties list).

		* add a new devclass, DV_PROPERTIES (for non-devices that are
		  solely property containers.)

		* add a new flag, DV_PROP_IMMUTABLE, which indiates that
		  a device's properties are immutable.

Actually, I spent some time thinking about this issue. 
If you don't trust your child nodes not to mess up your
properties, why should you trust your child nodes not to
remove the immutable bit?  Hence I added dev_copyprops(9),
so you can keep a private property container that nobody 
else has access to.

Also, dev_destroy(9) checks to see if the perperty container
is unused before deleting it.  That way you can do:

	create properties container
	config_search(... properties container ...)
	destroy properties container

or:

	create static properties container
	set up properties that will never change
	loop {
		create dynamic properties container
		copy properties from static properties container
			to dynamic properties container
		set up properties that change from dev to dev on
			dynamic properties container
		config_found(... dynamic properties container ... )
		destroy dynamic properties container
	}
	destroy static properties container


		
	> 3.1	New Functionality
	> 
	> int dev_setprop(struct device *dev, const char *name, void *val, size_t len, 
	> 			int flags);
	> 
	> Create a property `name' and attach it to `dev' with a `len' byte value 
	> copied from location `val'.  If PROP_WAIT is not specified then dev_setprop(9)
	> will not sleep for resource shortage.  Returns 0 on success or an error value.

	(1) if immutable, error.

	(2) if not found on device specified, adds it there (i.e. doesn't
	somehow traverse up-chain).

	(3) if found on device specified, replaces existing entry.

O.K.  I'll make this more explicit.

	> int dev_copyprops(struct device *source, struct device *dest, int flags);
	> 
	> Copy all properties associated with `source' device to `dest' device
	> structure.  If PROP_WAIT is not specified then dev_copyprops(9) will not sleep
	> for resource shortage.  Returns 0 on success or an error value.

	Is this a deep copy, or a shallow copy?  (i.e., are only the
	properties directly associated with the named device copied, or are
	those properties and all inherited properties also copied?)

	I suspect you mean 'only the properties directly associated,' but it
	should be speficied.

It only does `shallow copies'.  Only functions which specifically take
PROP_INHERIT ever traverse device trees, and only when that flag is
explicitly set.

	> size_t dev_getprop(struct device *dev, const char *name, void *val, 
	> 			size_t len, int flags);
	> 
	> Retrieve a property called `name' associated with `dev'.  If the property is
	> not found dev_getprop(9) will call md_getprop(9).  If the flag PROP_INHERIT is
	> set, and there is no property with that name associated with this device node,
	> it will try to retrieve the property from any parent device nodes.

	Is the ordering on this correct?  i'd expect it would be:

		(1) look at current device
		(2) if (PROP_INHERIT) look at parents for interhited devices,
		(3) call md_getprop() (or whatever it'll be called.

No, the order is slightly different:

	(1) look at current device
	(2) call md_getprop()
	(3) if (PROP_INHERIT) look at parents for interhited devices

The reason for this is that md_getprop() is specific for that particular
device, and when dev_getprop() is called on the parent, it will also
call md_getprop() on that device before going on to it's parent, etc.

	> int dev_delprop(struct device *dev, const char *name, int flags);
	> 
	> Remove a property from a device node.  If the flag PROP_INHERIT is set it will
	> look for that property in any parent device nodes.  If the flag PROP_ALL is
	> set it will remove the property from that device node and all parent device
	> nodes.  If a NULL is supplied for the name, dev_delprop(9) will remove all
	> properties from a device node.  If a NULL is supplied for the name and
	> PROP_ALL is set, dev_delprop(9) will remove ALL properties from a device node
	> and all its parents.  It returns 0 on success or an error value.

	I don't think it's right for there to be a mechanism by which a device
	can modify the properties of its parents.  (Under what circumstances
	do you think this is a valid thing to do?)

Frankly, I don't thinka device should ever delete properties
from parent nodes.  I would be quite happy to remove both
PROP_INHERIT and PROP_ALL functionality.

	There definitely needs to be a way to 'reset' the inheritance, though,
	i.e. terminate the inheritance chain.  Probably DVF_PROP_NOINHERIT,
	which says that this node doesn't inherit from its parents.

	That way, to clear a device's slate, you might do:

		(1) set NOINHERIT,
		(2) delete all.

	Actually makes me wonder if you want to have the equivalent of a
	'whiteout'.  (it might be useful to see how often that'd be used in
	practice, and leave the door open to implementing it later...)  If
	that type of thing isn't supported, it might be useful to say
	something like:

		(1) copy all inherited values down from parent,
		(2) set NOINHERIT.

Since you specifically select whether you want to PROP_INHERIT
or not when you do the query, none of that is really needed.
To clear a device's node all you need is:

	dev_delprop(dev, NULL, 0);

Whiteouts are an interesting idea but add complexity that we
won't want to worry about initially.  It's hard enough to
determine where a property's value comes from withoug having
to deal with possible whiteouts.  We could add that feature 
in the future if there is a need, but I fail to see it right 
now.

	> struct device *config_found_smd(struct device *dev, void *aux, 	cfprint_t print, 
	> 			cfmatch_t submatch, struct device *propdev);
	> 
	> In addition to config_found(9), and config_found_sm(9), config_found_smd(9)
	> can be used to supply device properties to child devices during probe and 
	> attach.  The `propdev' parameter is passed down to config_search(9) and
	> config_attach(9).
	> 
	> 
	> struct device *dev_config_found(struct device *dev, struct device *propdev,
	> 		 	cfprint_t print, cfmatch_t submatch);
	> 
	> dev_config_found(9) is intended as an interface for bus drivers that no longer
	> need to supply an `aux'.  The `propdev' parameter will initally be a dummy
	> container but later will be the real child device node.

	How does it make sense to provide both of these as public interfaces,
	assuing you're going to replace the 'aux' with the propdev for
	config_found_smd()?

	config_found_smd() can sanely be called either with:

		(1) aux set appropriately && propdev == NULL, or
		(2) aux to be ignored && propdev != NULL.

	(1) == config_found_sm().
	(2) == dev_config_found().

Hopefully, config_found_smd() would eventually be phased
out.  (Yeah, sure.)

	> struct cfdata *dev_config_search(cfmatch_t fn, struct device *parent, void *aux, 
	> 			struct device *propdev);
	> 
	> dev_config_search(9) takes an additional parameter `propdev' which is a
	> pointer to a device which provides a set of properties for devices to use
	> during probe.  If `propdev' is not NULL, locator data are attached to it as
	> properties, it is passed to the probe routine as the `aux' parameter and
	> the value passed in as `aux' is ignored.  The probe routine needs to know to
	> cast `aux' to a pointer to a device and extract properties rather than use it
	> as a pointer to configuration data.
	> 
	> struct device *dev_config_attach(struct device *parent, struct cfdata *cf, 
	> 			void *aux, cfprint_t print, struct device *propdev)
	> 
	> If a non-NULL `propdev' is passed in to dev_config_attach(9), all properties
	> attached to it are moved to the newly created device node for the device
	> being attached.

	Again, for providing public interfaces, why bother with the 'aux'
	parameter if it's just going to be replaced by propdev if the latter
	is supplied?

	It only makes sense to take both if you're going to provide both to
	underlying match/attach rtns (to allow backward compatiblity.)


	If you're going to mandate an all-or-nothing transition per bus, it
	makes sense to have two disjoint set of function entry points: one
	which takes aux, and one which takes propdev.

	If you're going to allow a smooth transition, then you need entry
	points which take both and wrappers around them to take either.


	To encourage smooth transition between your two phases, I suggest you
	do the following also at the time you introduce the properties stuff:

	(1) add a new fn/macro dev_private_date(struct device *), which
	returns a pointer to the private data.

	(2) provide macro CFATTACH_DEFINE(attach_name, private_size) (or
	better name) that would:

		provide struct cfattach attach_name##ca (for now using
			private_size + sizeof struct device as size)
		provide static protos for attach_name##match and
			attach_name##attach

	Aux vectors should be in the prototypes, but probably named specially
	so they can't easily be used.

	(3) provide macros CFATTACH_MATCH_DEFINE(attach_name) and
	CFATTACH_ATTACH_DEFINE(attach_name) (and i suppose one for detach)
	that can be used to head the function definitions and match the protos
	provided by (2).

	I'd say that initially they'd start out taking both aux and propdev,
	but eventually we could flip a switch and make them only take propdev.

	In the mean time, the 'cfattach' could still be defined to take only
	aux, etc., so we could transition drivers, etc.  But flip that switch,
	and the aux support goes away entirely, no editing required.

	The transition would go something like:

	(0) provide the infrastructure to do conversion, as described above,
	and to provide the new property interfaces.

	(1) convert busses to provide both properties and aux values.

	(2) convert drivers on those busses to use new CFATTACH declaration
	macros, and to exclusively use properties.

	(3) after a period of time when the new macros have been in place and
	all in-tree drivers have been converted, flip the switch to allow
	drivers to only take properties, and nuke the aux-only versions of the
	config_found, etc. functions.

	(4) deprecate the versions of the config functions that include both
	properties and aux values, and as time permits, rip the aux value code
	out of busses.

	(5) after a period of time has elapsed and all of the busses have been
	converted, rip out the aux-tolerant config_found, etc. functions.

	I say do that at time of addition of properties, because otherwise you
	need to worry about editing the cfattach() stuff in all the existing
	drivers, etc., multiple times.  This can allow you to hide the gross
	casts necessary for backward compatibility inside the macros.


Actually, we could get rid of this `aux' vs. `propdev' distinction in
the function signitures by basing it on a property associated with the
parent (bus) node.  If the property "device-properties" is set it casts
`aux' to a `struct device *' and uses it to pass propreties.  If that
property is not set it uses it at an opaque `aux' structure the way
it works now.

But that would preclude mixing old and new attachment schemes.  So
macros are probably a better way to go.  Unless there were some way
to provide this information in the cfattach strucure....

And add a new macro, DEV_PRIVATE(type, dev) that will initially
cast the `struct device' pointer to the softc, then later dereference
the pointer to the softc in `struct device' once the two are separate.

Eduardo