eeh@netbsd.org writes:

	> [ ... property flags ... ]

	still not keen on grouping the "operation" flags with the "property
	meaning" flags.

bus_dma* and bus_space* group flags like this.

	you don't see e.g. mmap() mixing 'prot' and 'flags' even though it
	could.  This isn't different than that.

This isn't like mmap where there needs to be a separate
set of protections and flags.

	> DEV_CFA_DECL(name, size, match, attach)	-- Declare cfattach structure

	You should derive match and attach function names from 'name'.
	Drivers also need to support detach, and perhaps more entry points at
	some time in the future.

Deriving the match and attach names from the `name' won't work
when different buses share the same match and attach functions.
This is done in many sparc device drivers.

	> DEV_PROTECT(dev)			-- Make properties on the device immutable.
	> DEV_UNPROTECT(dev)			-- Remove immutability from a device.

	maybe DEV_PROP_*()

O.K.

	> 
	> 2.3	Transitional Functions
	> 
	>    struct device *config_found_sad(struct device *dev, void *aux, 
	> 			cfprint_t print, cfmatch_t submatch, 
	> 			struct device *child);
	>    struct cfdata *config_search_ad(cfmatch_t fn, struct device *parent, 
	> 			void *aux, struct device *child);
	>    struct device *config_attach_ad(struct device *parent, struct cfdata *cf, 
	> 			void *aux, cfprint_t print, struct device *child);

	look good, I think.

	> 
	> 2.4	New Functions
	> 
	>    struct device *dev_config_create(struct device *parent, int flags);

	more discussion of this later.

	dev_create, no parent, opflags.

Needs to have WAIT/NOWAIT passed in, and needs parent so
it's put on the tree in the right place and depth searches
work.

	also, it occurs to me that we need a dev_destroy...

dev_detach() does that.


	>    struct device *dev_config_found(struct device *parent, struct device *child, 
	> 			cfmatch_t submatch, cfprint_t print);
	>    struct cfdata *dev_config_search(struct device *parent, struct device *child,
	> 			cfmatch_t fn);

	Here's a question: is config_search() as is ever called with a
	non-NULL aux ptr?  Given the way it's used in the places i'm aware of
	(e.g. ISA), that's not really necessary/appropriate.

	The vast majority of uses in this case are with NULL.  There are a few
	counterexamples:

	* config_found().

	* weird MD early-probe code.  This is done by amiga and atari, and to
	be honest I don't understand it and don't have time right now to
	understand.  Perhaps somebody with clue could tell us what exactly
	that is about.

	* cobalt and sgimips mainbus config.  I'd guess off-hand that this
	could/should probably be done via direct config.

	* a few really weird uses in hpcmips where the parent _print_ routine
	is passed down in the aux.

	* a few possibly correct uses (hpib, pnpbios -- but i'd have thought
	this would be direct config!!!, some ports obio, vme & similar).

	I think this needs a bit more study.

	In particular:

	(1) the MD early-probe code needs to be understood, to see if that's
	ameable to conversion.

	(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.  I think calling config_search() directly is
a Bad Thing.  I would be happy to make config_search() an internal
interface only.

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

	>    struct device *dev_config_attach(struct device *parent, struct device *child, 
	> 			struct cfdata *cf, cfprint_t print);

	These look OK.


	>    int dev_setprop(struct device *dev, const char *name, void *val, 
	> 			size_t len, int flags);

	This should take 'propflags' and 'opflags'.

	>    int dev_copyprops(struct device *source, struct device *dest, 
	> 			int flags);

	This is _only_ 'opflags'.

	>    size_t dev_getprop(struct device *dev, const char *name, void *val, 
	> 			size_t len, int flags);
	>    int dev_delprop(struct device *dev, const char *name, int flags);
	>
	>    size_t dev_mdgetprop(struct device *dev, const char *name, void *val, 
	> 			size_t len, int flags);

	I believe these are all only 'opflags' as well, right?

The flags each function takes is documented in the function
description.

	So, perhaps you'll hate me for this, but we have:

	DEV_PROP_PROTECT (assuming you change as suggested above),
	dev_config_*, etc....

	I would think that:

		dev_prop_set
		dev_prop_copyall
		dev_prop_get
		dev_prop_delete
		dev_prop_get_md

	would be better names.  Sorry, I know, you've changed (some of?) these
	once before at my request...  but with the rest of the dev_* functions
	it seems obvious and neither of us should have overlooked it.  trivial
	to change, too.

Fine.  I don't really care what they're called.

	It also occurs to me that config_detach needs at least a name
	remapping.

O.K.  Call it dev_config_detach() and map it with a macro.

	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. 

	The thought being, if you are going to go into attaching locators
	directoly, you need to be able to do something like:

		get property value passed by parent
		get property value from locator
		if (locator is not a wildcard &&
		    parent value != locator value)
			reject

	That's best accomplished via something like:

		int x = wildcard;

		if (dev_prop_cmpdata("locatorval", PROP_SEARCH or 0, x, sizeof x) &&
		    dev_prop_cmp("locatorval", PROP_SEARCH or 0,
				 "parentval", PROP_SEARCH or 0))
			reject;

Yeah, the locators are an issue.  We could have the MI code
do the locator comparison, taking in to account wildcards.
The only question is how you present wildcarded locators 
to a program trying to extract a config file from a running
kernel.  

This is a protocol issue that should be discussed separately.

	> 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, dev_mdgetprop() that is called if
	> attempting to retrieve a property fails.  dev_mdgetprop() 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_config_create(9) will generate a dummy device node that the parent can use
	> to attach properties to.  config_attach*(9) functions will consume the dummy
	> device node then create the real device node and migrate properties to it.

	not just config_attach*, right?  I'd suspect config_found(),

Hm.  Yes, I expect config_found is the one that consumes the device.

	> The second phase will separate the `struct device' from the device's softc.
	> dev_config_create(9) will create the true device node, which config_found*(9)
	> will attach to the device tree.  If the device probes successfully,
	> config_attach(9) will allocate a separate device softc structure and link it
	> to `struct device'.  This change will cause breakage in practically all
	> existing device drivers, so it will be relegated to some future date.

	s/it.*/the interfaces described here will allow smooth transition/  8-)


O.K.

	> 3.0	Changed Data Structures
	> 
	> Several fields will be added to `struct device' and some will be relocated to
	> compact the size of the structure on 64-bit platforms.
	> 
	> A singly linked list of properties will hang from the `dv_props' field.
	> 
	> A pointer to the device private data will be contained in `dv_private'.

	cool.


	> 3.1	Changed Functionality
	> 
	> A new device type `DV_NONE' will be added to signify that a device node is as
	> yet un-probed.
	> 
	> Drivers that expect properties during probe should:
	> 
	> 	o Define a softc that does not contain a `struct device *'
	> 
	> 	o Use CFA_DECL() to declare the softc.

	s/softc/cfattach structure/

	> By doing that, the `aux' parameter passed in to the device's probe function
	> will point to a `struct device *', with all all locators attached as
	> properties, in addition to any properties the parent driver has attached. 

	You also need to use macros to define & declare the match, attach,
	detach, etc., functions.

	The point here is to define them something like:

	int foomatch(__parent_dontuse, cfdata, __aux_dontuse, childdevnode)

	in such a way that the 'dontuse' params can magically be made to
	disappear later.

No, the child node is simply passed instead of the `aux'.  No
change of function signatures needed.  Ever.

	> DEV_CFA_DECL(name, size, match, attach)
	> 
	> Declare a `struct cfattach' for a new-style device driver.  The `size' is the
	> size of the softc, and the `match' and `attach' parameters should be pointers
	> to the match and attach functions for that device.  The `name' parameter should
	> match the name referenced by `ioconf.c'.

	additional comments above on this.


	> struct device *dev_config_create(struct device *parent, int flags);
	> 
	> dev_config_create() will trigger the creation of an un-configured device node
	> and attach it to the tree under `parent'.  This allows the parent driver to
	> attach properties to the device node before calling config_found*() so they
	> are available to the child device during the probe and attach steps.  If
	> PROP_WAIT is not specified then dev_config_create(9) will not sleep to resolve
	> resource shortages.  Returns a pointer to a `struct device' on success, and
	> NULL on error.

	s/_config// i think.

	this _should not_ take a parent, and should not attach into the device
	tree.

I thought you wanted it in the device tree so you could query 
recursively in the *probe() routine.

	These are opflags, and should probably be named along the form
	DEV_WAIT or DEV_OP_WAIT.

	They shold not be PROP_ flags!!!

O.K.  I'll change the flag to `int wait' which is true/false.

	> struct device *config_found_sad(struct device *parent, void *aux, cfprint_t print, 
	> 			cfmatch_t submatch, struct device *child);
	> 
	> In addition to config_found(9), and config_found_sm(9), config_found_sad(9)
	> can be used to supply device properties to child devices during probe and
	> attach.  The `child' parameter is passed down to config_search(9) and
	> config_attach(9).

	s/sad/sd/, s/sad/smd/ or similar, I think.  This follows from the
	previous convention of adding; you're replacing letters...  'aux' has
	always been there.

O.K.

	config_found() and config_found_sm() are wrappers around this
	function, right (which pass null submatch and child, and null child,
	respectively).

Yes.

	> struct cfdata *config_search_ad(cfmatch_t fn, struct device *parent, 
	> 			void *aux, struct device *child);
	> 
	> config_search_ad(9) takes an additional parameter `child' which is a pointer
	> to a device which provides a set of properties for devices to use during
	> probe.

	config_search_d.

	> If the device being probed used DEV_CFA_DECL() to declare it's `cfattach'
	> structure, a `child' device is created if it does not exist.  Locator data are
	> attached to the `child' as properties.

	It's not clear to me that this is actually desirable.  I think you
	want the bus-provided function that does the matchign to do it, just
	like right now that's the fn that fills in the aux.

O.K.  So if you use this function you need to provide a non-NULL
child?

	> It is then passed to the probe routine
	> as the `aux' parameter.  The child driver should cast the `aux' parameter to a
	> `struct device *' and use it to query for properties.

	_no_.  the parameters should be as strongly typed as possible, and
	'aux' should be nuked entirely.

	That means no aux, no casting.  The declare the match/attach/detach
	functions with their macros, and then use the 'child' device that's
	passed in.

	That's really the right way to do it.

I don't like the idea of having macros for all the 
match/attach/detach functions.  If we want to change
the function signature, I would handle it all inside
the DEV_CFA_DECL() macro: 

	1) Declare the match/attach functions with the
	new parameters.

	2) Cast them inside the the parameter list to
	the appropriate type.

That way when you use DEV_CFA_DECL() you will be forced
to change the calling sequence or have a conflicting
function declaration.

	> If the device did not use DEV_CFA_DECL(), the probe routine is called with the
	> `aux' passed in by the parent in the `aux' field and the `child' device with
	> locator properties is not available to the *_probe() routine.

	no.  The calling convention is the same either way.  Just old-style
	drivers don't notice that they've got a hidden extra argument.

No.  No hidden argument.  You either get a (parent, cf, aux) or
a (parent, cf, child).  No point in keeping vestiges of the old
interface around.

	> struct device *config_attach_ad(struct device *parent, struct cfdata *cf, 
	> 			void *aux, cfprint_t print, struct device *child)

	config_attach_d or similar.  oh, hell, i'm not going to argue over
	this trivial a name issue, these functions are going to die eventually
	anyway.


	> If a non-NULL `child' is passed in to config_attach_ad(9), a softc of the size
	> specified by DEV_CFA_DECL() will be allocated and a pointer to it placed in
	> the device private field of the `child' device.  This structure can be
	> accessed through `DEV_PRIVATE(dev)'.
	> 
	> If a non-NULL `child' is passed in to config_attach_ad(9), a new device node
	> is allocated of the appropriate size, all properties attached to it are moved
	> to the newly created device node for the device being attached.
	> 
	> If a NULL value is passed in as the `child', a device structure is allocated
	> of the appropriate size and locator data are associated with it.  If the
	> device used DEV_CFA_DECL(), then the softc will be accessible through
	> `DEV_PRIVATE(dev)'.

	No.  If NULL is passed, this is an invocation via a wrapper of
	config_attach or similar.

	the parent fills in the properties for the locators, if appropriate.

	In cases of direct config, the parent should also be doing
	'submatch'ing to ensure sanity.

	I think that this could probably be done different ways ... but it
	merits some more discussion up front since it begins to form the basis
	of the 'protocols' for property passing.

	_Exactly_ what do you have in mind here, if it's going to be done
	automatically?

I was thinking that the submatch could compare the locators
attached by the parent to the device structure against the
locators provided by config.  But this is open to revision.

	> struct device *dev_config_found(struct device *dev, struct device *child,
	> 		 	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 `child' parameter will initally be a dummy
	> container but later will be the real child device node.

	not a dummy, it contains useful properties.

So, what do you want to call it if it's never a real device?

	> struct cfdata *dev_config_search(cfmatch_t fn, struct device *parent, void *aux, 
	> 			struct device *child);
	> 
	> dev_config_search(9) is intended as an interface for bus drivers that no
	> longer need to supply an `aux'.  The `child' parameter will initally be a
	> dummy container but later will be the real child device node.

	same here.


	> 
	> struct device *dev_config_attach(struct device *parent, struct device *child, 
	> 			struct cfdata *cf, cfprint_t print);
	> 
	> dev_config_attach(9) is intended as an interface for bus drivers that no
	> longer need to supply an `aux'.  The `child' parameter will initally be a
	> dummy container but later will be the real child device node.

	same here.


	> 
	> 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.  If PROP_CONST is specified, no storage is
	> allocated for the value, and when the property is queried it will copy `len'
	> bytes from the location specified by `val', so that data cannot be freed or
	> the kernel may panic.  If PROP_STRING is specified then the property is marked
	> as being a NUL terminated ASCII string.

	does size include NUL in this case?

Yes.  Otherwise it will probably be lost.

	> Returns 0 on success or an error
	> value.  This will fail without modifying the device node if it is protected.

	This is fine, modulo the name and the issues about flags.

	> 
	> int dev_copyprops(struct device *source, struct device *dest, int flags);
	> 
	> Copy all properties associated with `source' device to `dest' device
	> structure.  It does not traverse the device tree or make any use of
	> dev_mdgetprop().  If PROP_WAIT is not specified then dev_copyprops(9) will not
	> sleep for resource shortage.  Returns 0 on success or an error value.  The
	> state of properties on the destination device is undefined if the operation
	> fails.  This will fail without modifying the destination device node if it is
	> protected.

	this seems fine.


	> 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 dev_mdgetprop(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.  Returns -1
	> if the property cannot be found, otherwise it returns the length of the value
	> data and if `val' is not NULL it copies up to `len' bytes of the property data
	> to the location pointed to by `val'.
	> 
	> 
	> int dev_delprop(struct device *dev, const char *name, int flags);
	> 
	> Remove a property from a device node.  If a NULL is supplied for the name,
	> dev_delprop(9) will remove all properties from a device node.  It returns 0 on
	> success or an error value.  This will fail without modifying the device node
	> if it is protected.

	these seem fine modulo name issues.


	> size_t dev_mdgetprop(struct device *dev, const char *name, void *val, 
	> 			size_t len, int flags);
	> 
	> If defined an a machine dependent header file, this function is called when
	> ever an attempt is made to retrieve a property from a device node but the
	> property is not found.  It allows machine dependent code to look up properties
	> from other locations.  It should be implemented to behave the same way as
	> dev_getprop(9) does.  It does not need to traverse parent device nodes.

	How do you tell if a function is defined in a MD header file?  Are you
	saying it's a macro?

Yes, although it may be a macro that calls a function of the same name.

	It _may not_ traverse parent device nodes; if it does, that'll totally
	screw up search order, right?

Well, I don't really care what it does as long as it returns the correct
property.

	> 4	Converting Drivers
	> 
	> There are essentially two types of drivers, bus drivers and device drivers.
	> Bus drivers are distinguished because they can attach other drivers to
	> themselves.  The conversion process consists of four steps.  First a protocol
	> must be established for the use of properties during probe and attach.  Then
	> the bus driver needs to be converted to use the transitional interfaces that
	> provides both an `aux' and properties.  Next the device drivers attaching to
	> it need to be converted to handle the new interface to use properties rather
	> than `aux'.  Finally, the bus driver needs to be converted to use the new
	> interface and only use properties instead of providing an `aux'.

	yes.


	> A protocol should be carefully defined to provide what ever information a
	> driver needs from a bus so it can probe and attach.  A detailed discussion of
	> the issues involved is well beyond the scope of this document, but the
	> protocol should be able to handle layering of bus drivers.

	yes.  The one concern I have here is that you've started to define the
	protocol, by specifying that locators will be passed automatically,
	without actually going into more details on that...

The original idea was to copy them from the ioconf structures
onto the device node.  But this has issues.

	> The bus driver is then changed to manage its child device node explicitly.
	> The driver calls call `dev_config_create()' to allocate a driver and attach
	> properties to it.  It also creates an `aux'.  It then calls
	> `config_found_sad()' with both the `aux' and the device it created with
	> `dev_config_create()'.
	> 
	> Child drivers can now be modified to the new interface.  The `struct device'
	> is removed from the softc, and `DEV_CFA_DECL()' is used to declare the
	> device's `cfattach' structure.  The device's `*_probe()' and `*_attach()'
	> routines are modified to cast the `aux' parameter to a `struct device *', and
	> `dev_getprop()' is used to retrieve properties from it.  After the probe,
	> `DEV_PRIVATE(dev)' is used to access the softc, rather than directly casting
	> the `struct device *'.

	Comments per the above.


	> Once all the child drivers for a particular bus are converted, the bus driver
	> can be modified not to create an `aux' at all, and the call to
	> `config_found_sad()' is changed to a call to `dev_config_found()'.
	> 
	> 
	> Appendix 1
	> 
	> Providing dev_mdsetprop() and dev_mddelprop() would be possible but make the
	> framework much more complicated.  If permanent storage of properties is
	> desired it should use some machine dependent method since non-volatile storage
	> is not necessarily available on all architectures.
	> 
	> 
	> Appendix 2
	> 
	> The config(8) application will eventually be extended to take typed values for
	> locators and for properties that can be specified in config files.
	> 
	> 
	> Appendix 3
	> 
	> A set of protocols need to be developed for the use of properties for
	> attaching device and bus nodes.  Required global properties should be defined,
	> as well as bus-specific properties.
	> 
	> 
	> Eduardo