introduce device_is_attached()
Hi,
I want to introduce a new function to sys/devices.h:
bool device_is_attached(device_t parent, cfdata_t cf);
The purpose is for bus drivers who wants to attach children
and ensure that only one instance of it will attach.
'parent' is the bus driver and 'cf' is the child device
as passed to the submatch callback via config_search_loc().
The return value is true if the child is already attached.
I implemented a reference usage of it in amdnb_misc.c to ensure
that amdtemp only attaches once on rescan.
Any comments to the patch?
Christoph
Index: sys/sys/device.h
===
RCS file: /cvsroot/src/sys/sys/device.h,v
retrieving revision 1.140
diff -u -p -r1.140 device.h
--- sys/sys/device.h13 Nov 2011 22:05:58 - 1.140
+++ sys/sys/device.h16 Apr 2012 16:38:11 -
@@ -528,6 +528,9 @@ voiddevice_active_deregister(device_t,
bool device_is_a(device_t, const char *);
+bool device_is_attached(device_t, cfdata_t);
+
+
device_t device_find_by_xname(const char *);
device_t device_find_by_driver_unit(const char *, int);
Index: sys/kern/subr_device.c
===
RCS file: /cvsroot/src/sys/kern/subr_device.c,v
retrieving revision 1.2
diff -u -p -r1.2 subr_device.c
--- sys/kern/subr_device.c 31 Jan 2010 15:10:12 - 1.2
+++ sys/kern/subr_device.c 16 Apr 2012 16:38:11 -
@@ -176,3 +176,32 @@ device_is_a(device_t dev, const char *dn
return strcmp(dev-dv_cfdriver-cd_name, dname) == 0;
}
+
+/*
+ * device_is_attached:
+ *
+ * Returns true if a driver already attached to a device.
+ * Useful for bus drivers to figure if driver already attached
+ * to prevent drivers attaching twice.
+ */
+bool
+device_is_attached(device_t parent, cfdata_t cf)
+{
+ device_t dev;
+ deviter_t di;
+ bool attached = false;
+
+ for (dev = deviter_first(di, DEVITER_F_LEAVES_FIRST); dev != NULL;
+ dev = deviter_next(di))
+ {
+ if (device_parent(dev) != parent)
+ continue;
+ if (device_is_a(dev, cf-cf_name)) {
+ attached = true;
+ break;
+ }
+ }
+ deviter_release(di);
+
+ return attached;
+}
Index: sys/arch/x86/pci/amdnb_misc.c
===
RCS file: /cvsroot/src/sys/arch/x86/pci/amdnb_misc.c,v
retrieving revision 1.2
diff -u -p -r1.2 amdnb_misc.c
--- sys/arch/x86/pci/amdnb_misc.c 16 Apr 2012 16:07:24 - 1.2
+++ sys/arch/x86/pci/amdnb_misc.c 16 Apr 2012 16:38:11 -
@@ -78,38 +78,13 @@ amdnb_misc_match(device_t parent, cfdata
static int
amdnb_misc_search(device_t parent, cfdata_t cf, const int *locs, void *aux)
{
- device_t dev;
- deviter_t di;
- bool attach;
-
if (!config_match(parent, cf, aux))
return 0;
- attach = true;
-
/* Figure out if found child 'cf' is already attached.
* No need to attach it twice.
*/
-
- /* XXX: I only want to iterate over the children of *this* device.
-* Can we introduce a
-* deviter_first_child(di, parent, DEVITER_F_LEAVES_ONLY)
-* or even better, can we introduce a query function that returns
-* if a child is already attached?
-*/
- for (dev = deviter_first(di, DEVITER_F_LEAVES_FIRST); dev != NULL;
- dev = deviter_next(di))
- {
- if (device_parent(dev) != parent)
- continue;
- if (device_is_a(dev, cf-cf_name)) {
- attach = false;
- break;
- }
- }
- deviter_release(di);
-
- if (!attach)
+ if (device_is_attached(parent, cf))
return 0;
config_attach_loc(parent, cf, locs, aux, NULL);
Re: introduce device_is_attached()
On Apr 16, 2012, at 9:52 AM, Christoph Egger wrote: Hi, I want to introduce a new function to sys/devices.h: bool device_is_attached(device_t parent, cfdata_t cf); I'd prefer device_is_attached_p The purpose is for bus drivers who wants to attach children and ensure that only one instance of it will attach. 'parent' is the bus driver and 'cf' is the child device as passed to the submatch callback via config_search_loc(). The return value is true if the child is already attached. Can it be used in driver match routines so they don't need to keep a local to prevent multiple matches?
Re: introduce device_is_attached()
Along these same lines, it might be nice if there were a way to determine if _any_ instances of a device were attached. This is particularly useful for device-driver modules, which should prevent themselves from being unloaded if there are active instances. A quick check of some device-driver modules show that they simply call config_fini_component() which updates autoconfig tables, but doesn't seem to check if a driver instance exists... On Mon, 16 Apr 2012, Christoph Egger wrote: Hi, I want to introduce a new function to sys/devices.h: bool device_is_attached(device_t parent, cfdata_t cf); The purpose is for bus drivers who wants to attach children and ensure that only one instance of it will attach. 'parent' is the bus driver and 'cf' is the child device as passed to the submatch callback via config_search_loc(). The return value is true if the child is already attached. I implemented a reference usage of it in amdnb_misc.c to ensure that amdtemp only attaches once on rescan. Any comments to the patch? Christoph !DSPAM:4f8c4e7e1986566716057! - | Paul Goyette | PGP Key fingerprint: | E-mail addresses: | | Customer Service | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com| | Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette at juniper.net | | Kernel Developer | | pgoyette at netbsd.org | -
Re: introduce device_is_attached()
On Mon, Apr 16, 2012 at 06:52:28PM +0200, Christoph Egger wrote:
Hi,
I want to introduce a new function to sys/devices.h:
bool device_is_attached(device_t parent, cfdata_t cf);
The purpose is for bus drivers who wants to attach children
and ensure that only one instance of it will attach.
'parent' is the bus driver and 'cf' is the child device
as passed to the submatch callback via config_search_loc().
The return value is true if the child is already attached.
I implemented a reference usage of it in amdnb_misc.c to ensure
that amdtemp only attaches once on rescan.
Don't add that function. Just use a small amdnb_misc softc to track
whether or not the amdtemp is attached or not:
struct amdnb_misc_softc {
device_t sc_amdtemp;
};
Don't pass a pci_attach_args to amdtemp. Just pass it the chipset tag
and PCI tag if that is all that it needs.
I'm not sure I fully understand the purpose of amdnb_miscbus.
Are all of the functions that do/will attach at amdnb_miscbus
configuration-space only functions, or are they something else? Please
explain what amdnb_miscbus is for.
Dave
--
David Young
dyo...@pobox.comUrbana, IL(217) 721-9981
Re: introduce device_is_attached()
On 16.04.12 19:12, Matt Thomas wrote: On Apr 16, 2012, at 9:52 AM, Christoph Egger wrote: Hi, I want to introduce a new function to sys/devices.h: bool device_is_attached(device_t parent, cfdata_t cf); I'd prefer device_is_attached_p Ok, I will rename it. The purpose is for bus drivers who wants to attach children and ensure that only one instance of it will attach. 'parent' is the bus driver and 'cf' is the child device as passed to the submatch callback via config_search_loc(). The return value is true if the child is already attached. Can it be used in driver match routines so they don't need to keep a local to prevent multiple matches? Oh, good point. I think this is possible and will make struct amdnb_misc_softc completely superflous. I will give this a try tomorrow. Christoph
Re: introduce device_is_attached()
On Mon, 16 Apr 2012, Christoph Egger wrote: On 16.04.12 19:12, Matt Thomas wrote: On Apr 16, 2012, at 9:52 AM, Christoph Egger wrote: Hi, I want to introduce a new function to sys/devices.h: bool device_is_attached(device_t parent, cfdata_t cf); I'd prefer device_is_attached_p Ok, I will rename it. The purpose is for bus drivers who wants to attach children and ensure that only one instance of it will attach. 'parent' is the bus driver and 'cf' is the child device as passed to the submatch callback via config_search_loc(). The return value is true if the child is already attached. Can it be used in driver match routines so they don't need to keep a local to prevent multiple matches? Oh, good point. I think this is possible and will make struct amdnb_misc_softc completely superflous. I will give this a try tomorrow. I'm kind of with David Young, surely this is what the softc is for.. so that the parent can keep track of which resources it has allocated (and by inference, not reallocate them to another device) but you could also use the device_properties to store such information? iain
re: introduce device_is_attached()
I want to introduce a new function to sys/devices.h:
bool device_is_attached(device_t parent, cfdata_t cf);
this doesn't seem to match what the function actually does, which
is answer the question does this parent bus have a child attached
with the same driver name as in cf-cf_name.
that doesn't handle the more interesting cases i can think of such
as does this pci bus have something at device 4 function 1, which
is a question i'd expect device_is_attached{,_p}() to answer. ie,
it is limited to fake busses AFAICT.
i don't think we should adopt this function in sys/device.h.
.mrg.
loadable verbose message modules (was Re: Kernel panic codes)
On Sun, Apr 15, 2012 at 10:57:54PM +1000, Nat Sloss wrote: Hi. I have been working on a program that uses bluetooth sco sockets and I am having frequent kernel panics relating to usb. I am receiving trap type 6 code 0 and trap type 6 code 2 errors. I've been thinking that it would be nice if there were more kernel modules that replaced or supplemented anonymous numbers with their name or description. Thus trap type 6 code 0 and trap type 6 code 2 would become something like trap type 6(T_PAGEFLT) code 0 and trap type 6(T_PAGEFLT) code 2PGEX_W if and only if the module was loaded. The existing printf() in trap_print() printf(trap type %d code %x ...\n, type, frame-tf_err, ...); would change (just for example) to printf(trap type %d%s code %x%s ...\n, type, trap_type_string(type), frame-tf_err, trap_code_string(type, frame-tf_err), ...); By default, the number - string conversion functions, const char *trap_type_string(int type); const char *trap_code_string(int type, int code); would be weak aliases for a single function that returns the empty string. The kernel module would override the defaults by providing strong aliases to actual implementations. For that weak/strong alias thing to work on a loadable module, I think that Someone(TM) will need to make the kernel linker DTRT when a modules with overriding strong aliases is added. If the module is not unloadable, Someone(TM)'s work is through. There are some gotchas making the kernel *un*loadable. BTW, I also desire this function in the kernel linker for Other Reasons. Dave -- David Young dyo...@pobox.comUrbana, IL(217) 721-9981
Re: CVS commit: src/tests/modules
On Mon, Mar 26, 2012 at 12:10:30AM -0700, Matt Thomas wrote: doesn't modctl/modload return some error which indicate the reason of failure? EPERM which isn't really useful. Oddly enough, it actually fails with EPERM on Sparc, but with ENOSYS on Xen. Manuel pointed out that it might be kobj_load_vfs(), kobj_load_mem(), or kobj_stat() that returns ENOSYS. - Julka.
