Subject: Re: More Device Properties
To: None <cgd@sibyte.com, eeh@netbsd.org>
From: None <eeh@netbsd.org>
List: tech-kern
Date: 02/14/2001 19:48:55
	> 2.1	Changed Functions
	> 
	>    struct device *config_found_smd(struct device *dev, void *aux, 
	> 			cfprint_t print, cfmatch_t submatch, 
	> 			struct device *propdev);

	Technically, this is a new function.  Presumably, config_found() and
	config_found_sm() would be macro wrappers around it, right?

Right.  See the accompanying patch.

	>    struct cfdata *config_search(cfmatch_t fn, struct device *parent, 
	> 			void *aux, struct device *propdev);
	>    struct device *config_attach(struct device *parent, struct cfdata *cf, 
	> 			void *aux, cfprint_t print, struct device *propdev)

	These should probably be done in ways that preserve
	backward-compatible APIs, as was done with config_found ->
	config_found_sm, etc.

Yes, I was considering doing that.

	> 2.2	New Functions
	> 
	>    struct device *devprop_create(void);
	>    void devprop_destroy(struct device *);

	I'm not convinced of the virtue of these.  I think if you're going to
	create a 'property container,' you should create a propery container,
	and embed that in 'struct device'.

	Using device structs for this purpose seems a little bit ... lacking.

	>    int set_devprop(struct device *dev, const char *name, void *val, 
	> 			size_t len, int flags);
	>    size_t get_devprop(struct device *dev, const char *name, void *val, 
	> 			size_t len, int flags);
	>    int del_devprop(struct device *dev, const char *name, int flags);
	> 
	>    size_t get_mdprop(struct device *dev, const char *name, void *val, 
	> 			size_t len, int flags);

	These should all be prefixed with a common prefix, just like config_*
	are now.

	I might suggest something like:

	struct devproplist *devprop_list_create();
	void *devprop_list_destroy(struct devproplist *);

	then use struct devproplist, rather than devices, in the devprop fns.
	embed a devproplist in struct device, and use it when appropriate.

I struggled with this a bit and decided that using `struct device *' in
some contexts and `struct devproplist *' in others broke orthogonality, 
encapsulation, and consistency.  

The interfaces could all be changed to operate on device property lists,
but that doesn't seem as clean an implementation to me.  That would also
preclude any possibility of implemnting as anything other than a linked
list in future, such as database table.

	I'm not sure I like the extra copying that get_devprop() requires,
	either, but I'm kinda ambivalent about that.

It simplifies memory management since you don't need to worry about
having dangling references to data that's been freed.

	It seems that you only intend to be passing 'simple values,'
	i.e. nothing where a copy actually has to transform an object
	(e.g. increment a reference count or similar)?

	If you're going to start passing all auxv info down this way, and want
	to be doing (un)loadable drivers, it may be a good thing to provide a
	way to mutate on copy...  Not really sure though.  Something to think
	about.

Those types of operations require strong typing on the data, which 
significantly complicates the system.  Solaris has different routines
for integer properties, string properties and array properties.  This
complicates the interfaces but still does not provide enough information
to do any sort of type manipulations during retrieval.

Anyway, if you plan to replace the aux information with properties, it is
best to encode them as separate proerties rather than a fixed `aux' structure.
That way the client can be sure that it is getting the particular value it
is requesting by name, rather than hoping some offset into the `aux' data
is being interpreted properly.

Eduardo