eeh@netbsd.org writes:
> 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.

At worst you can do so by traversing the alldevs after your device
struct.

I suggest that this be left off until the time it's known to be
needed.  It doesn't change API at all, and it seems a shame to
significantly expand the device struct before it's known to actually
have a concrete use.


> 	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 believe the point is, different sets of flags should _not_ be
#defined in the same set, _or_ passed together, _or_ stored together
in e.g. structures.

Why would it ever make sense to reserve a bit for PROP_WAIT in a
property structure, for instance?


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

Three sets of functions is basically what i've been suggesting pretty
much all along, yes.


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

I don't understand all the issues here.  However, the "device's
identification" should not depend on what's in the properties, it
should depend on what's in the device.  You may be able to construct
properties from what's in the device, certainly.

But the device should _not_ stick around after detach.

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

Why not?  How can you, absolutely, be certain that it's the same
device?

Further, there's the semantics issue: You remove a device and reattach
it, you (or at least I) _expect_ it to be reset.  Part of that is
properties, at least in my mind.

Anyway, as far as I'm concerned, as long as I have any say over what
happens in the config system, you will not prevail here.  When a
device is removed from the system, it is _removed_ from the system.


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

config_found calls multiple match routines to match a single device.
If a single buggy match routine modifies the properties, it can screw
up all subsequent match routines.

As far as I'm concerned, this _is_ necessary, if only for that
purpose.


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

Huh?  I don't get this.  If you go back and read what I have proposed
(repeatedly now) there should be no confusion whatsoever between 'aux'
and a device for properties.


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

This i would buy.  In this case, the code (all variants: old, old+new,
new) should recognize that no properties device has been passed, and
create one internally.


> 	> 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?

Go back and read the thing i've suggested you implement.

In short form, to answer your questions in order:

(1) new style drivers should _not_ be able to attach to busses which
haven't at least been converted for old+new compatibility.

(2) no, you only need to convert the bus to old+new compatibility,
then you can convert invididual drivers as need be.

(3) That's irrelevant, because if you do it right (or even the way i
suggested 8-) you can get seamless compatibility with existing
drivers.


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

That's what I suspected.  You should have documented it.


I forgot to bring up (again) one issue that you'd not addressed in
previous commentary:

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

The name of this is still bogus.  It should convey both dev_* and some
flavor of md-ness.



cgd