Subject: Re: More Device Properties
To: None <cgd@sibyte.com, eeh@netbsd.org>
From: None <eeh@netbsd.org>
List: tech-kern
Date: 02/20/2001 01:17:24
	eeh@netbsd.org writes:

	> 	CIRCLEQ_HEAD(dv_children, device) dv_child;	/* circular list of children */
	> 	CIRCLEQ_ENTRY(device) dv_sibling;	/* circular list sibling device */

	These haven't been brought up before, and I'm not at all convinced
	they're either a good idea or necessary.

Matt Thomas requested this functionality.  As far as I can tell,
you can only traverse the tree from child to parent, and cannot
go down.  I expect this will become necessary to support dynamic
reconfiguration, and I was changing the device node structure,
so I thought I'd throw it in.

	> 	SLIST_HEAD(devpropslist, devprop) dv_props;
	> 	void	*dv_private;
	> };
	> 
	> 
	> 2.1	Flags
	> 
	> PROP_WAIT		-- Specify that the call can sleep.
	> PROP_INHERIT		-- Inherit properties from parent.
	> PROP_STRING		-- Property is a string.
	> PROP_CONST		-- Property is constant do not allocate storage
	> 			   for the data.

	These are logically two different sets of flags.  They should be
	separated.

Well, they are being passed in in the same `flags' field in
dev_setprop() and dev_getprop() so it would be better if they
don't conflict with one another.

	I think if you have a solution which provide sane limitation of
	inheritance (which I think you need), there's no need for
	PROP_INHERIT.

No.  PROP_INHERIT is merely a convenience flag to search for
that particular property up the device tree.  If it's causing
confusion, maybe it should be renamed.

	> 2.2	Macros
	> 
	> DEVICE_PRIVATE(dev)	-- Access the private softc data of dev.
	> 
	> 
	> 2.3	Functions
	> 
	>    struct device *config_create(struct device *parent, int flags);
	>    struct device *config_found_smd(struct device *dev, void *aux, 
	> 			cfprint_t print, cfmatch_t submatch, 
	> 			struct device *child);
	>    struct device *dev_config_found(struct device *parent, 
	> 			struct device *child, cfprint_t print, 
	> 			cfmatch_t submatch);
	>    struct cfdata *dev_config_search(cfmatch_t fn, struct device *parent, 
	> 			void *aux, struct device *child);
	>    struct device *dev_config_attach(struct device *parent, struct cfdata *cf, 
	> 			void *aux, cfprint_t print, struct device *child);

	No.  The new-style name functions should _not_ take 'aux' at all.  If
	you want to provide functions which take both, to help
	backward-compatibility, they should be provided with names similar to
	the old functions (e.g. _d at the end).  You get this right for
	config_found_smd, but other than that, it's wrong.

	The goal here is to completely remove 'aux' right?  Well, if you do
	that, why should we have to go through and edit large numbers of
	drivers in the tree when it's time to do that?

The goal is to remove `aux' eventually, but if you don't
provide it to the dev_config*() routines, a parent bus
cannot have both old-style children and children that
have been converted.  Maybe we need three sets of functions:
old, transitional, and new.

	> If devices are detached, the `struct device' will remain
	> with all its properties but the softc will be destroyed.  This change will
	> cause breakage in practically all existing device drivers, so it will be
	> relegated to some future date.

	If devices are detached, the 'struct device' will remain attached to
	the device tree?  _NO_.  If a device is detached, it is _detached_.
	Removed.  Completely.  'struct device' gets completely destroyed.

This was found to be necessary on Solaris to support certain
types of devices which are reconfigured dynamically, particularly
SCSI and fibre-channel disks.  If you add a disk to a fibre-channel
loop, devices can change numbers, go away, come back, all by themselves.
You want to be able to keep around a device's identification (WWN)
so they don't silently migrate and change device nodes.

Similarly, if a user sets a property on a device, say disables
it, and the device is deconfigured and reconfigured, that 
property should not be lost in the process.

	> attach.  The `child' parameter is passed down to config_search(9) and
	> config_attach(9).

	This seems right enough, except you need to have some mechanism by
	which properties are made read-only, and that should be used by
	config_found_smd() and dev_config_found().

I don't think that is necessary.  If you really need the properties
to be read-only, create a dummy device node and copy them all there.


	> If the device being probed specifies a negative value in the `ca_devsize'
	> field of the `cfattach' structure, a `child' device is created if it does not
	> exist.

	I don't see any reason to do this.  If the parent didn't supply a
	device to hold properties, and the child requires one, then that's
	simply an error.

	the property-getting/setting code should probably recognize 'NULL'
	devices and panic.

No, it won't.  It will try to set properties on an `aux' and
all hell will break loose.

Anyway, if you don't need to pass any data to your children
other than locators, you shouldn't have to.

	> Locator data are attached to the `child' as properties, and the
	> `aux' parameter is placed in its device private data field.  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.  If it needs access to the aux data, use of `DEVICE_PRIVATE()'
	> on it will return the `aux' provided by the parent.

	No.  New-style drivers should never be able to get to aux.

This allows you to attach a new-stile driver to an old-style
bus.  Or isn't that desired functionality?  Do you really need
to convert a bus and all the drivers that can attach to it all
at the same time?  Even if some of those are in MD code scattered
across a dozen architectures?

	> 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
	> md_getprop().  If PROP_WAIT is not specified then dev_copyprops(9) will not
	> sleep for resource shortage.  Returns 0 on success or an error value.

	What will the state of 'dest' be on error?

Undefined.  Some of the properties may be copied and some
not.  Since these commands are relatively stateless, you can
either repeat the call and hope all the properties are copied,
or delete all the properties on that node to get to a known state.

Eduardo
..