Subject: Re: More config overhaul -- name-based attachment lookup
To: None <tech-kern@netbsd.org>
From: Jason R Thorpe <thorpej@wasabisystems.com>
List: tech-kern
Date: 10/03/2002 09:04:42
--UlVJffcvxoiEqYs2
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Thu, Oct 03, 2002 at 08:53:37AM -0700, Jason R Thorpe wrote:

 > Now that all driver lookups in the autoconfig code are handled by string
 > matches, it's time to do the same thing with attachment lookups.  This
 > implements that.
 > 
 > It is worth noting that most of the config changes I'd had to deal with
 > have been related to the lack of a ctor/dtor facility in our kernel.  I
 > will probably address that in the near future.  With ctors/dtors, a lot
 > of this glue can go away.
 > 
 > I plan on checking this in fairly soon (I need to fix up the amiga/atari/x68k
 > console configuration code to use the new attachment lookup mechanism).

Uh... I guess I forgot to attach the patch.  Here it is.

-- 
        -- Jason R. Thorpe <thorpej@wasabisystems.com>

--UlVJffcvxoiEqYs2
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename=cfattach-patch

Index: sys/kern/subr_autoconf.c
===================================================================
RCS file: /cvsroot/syssrc/sys/kern/subr_autoconf.c,v
retrieving revision 1.75
diff -c -r1.75 subr_autoconf.c
*** sys/kern/subr_autoconf.c	2002/10/01 18:11:58	1.75
--- sys/kern/subr_autoconf.c	2002/10/03 15:38:53
***************
*** 119,124 ****
--- 119,129 ----
  extern struct cfdriver * const cfdriver_list_initial[];
  
  /*
+  * Initial list of cfattach's.
+  */
+ extern const struct cfattachinit cfattachinit[];
+ 
+ /*
   * List of cfdata tables.  We always have one such list -- the one
   * built statically when the kernel was configured.
   */
***************
*** 182,198 ****
  void
  config_init(void)
  {
! 	int i;
  
  	if (config_initialized)
  		return;
  
  	/* allcfdrivers is statically initialized. */
! 	for (i = 0; cfdriver_list_initial[i] != NULL; i++)
  		if (config_cfdriver_attach(cfdriver_list_initial[i]) != 0)
  			panic("configure: duplicate `%s' drivers",
  			    cfdriver_list_initial[i]->cd_name);
  
  	TAILQ_INIT(&allcftables);
  	initcftable.ct_cfdata = cfdata;
  	TAILQ_INSERT_TAIL(&allcftables, &initcftable, ct_list);
--- 187,216 ----
  void
  config_init(void)
  {
! 	const struct cfattachinit *cfai;
! 	int i, j;
  
  	if (config_initialized)
  		return;
  
  	/* allcfdrivers is statically initialized. */
! 	for (i = 0; cfdriver_list_initial[i] != NULL; i++) {
  		if (config_cfdriver_attach(cfdriver_list_initial[i]) != 0)
  			panic("configure: duplicate `%s' drivers",
  			    cfdriver_list_initial[i]->cd_name);
+ 	}
  
+ 	for (cfai = &cfattachinit[0]; cfai->cfai_name != NULL; cfai++) {
+ 		for (j = 0; cfai->cfai_list[j] != NULL; j++) {
+ 			if (config_cfattach_attach(cfai->cfai_name,
+ 						   cfai->cfai_list[j]) != 0)
+ 				panic("configure: duplicate `%s' attachment "
+ 				    "of `%s' driver",
+ 				    cfai->cfai_list[j]->ca_name,
+ 				    cfai->cfai_name);
+ 		}
+ 	}
+ 
  	TAILQ_INIT(&allcftables);
  	initcftable.ct_cfdata = cfdata;
  	TAILQ_INSERT_TAIL(&allcftables, &initcftable, ct_list);
***************
*** 257,262 ****
--- 275,281 ----
  			return (EEXIST);
  	}
  
+ 	LIST_INIT(&cd->cd_attach);
  	LIST_INSERT_HEAD(&allcfdrivers, cd, cd_list);
  
  	return (0);
***************
*** 276,281 ****
--- 295,304 ----
  			return (EBUSY);
  	}
  
+ 	/* ...and no attachments loaded. */
+ 	if (LIST_EMPTY(&cd->cd_attach) == 0)
+ 		return (EBUSY);
+ 
  	LIST_REMOVE(cd, cd_list);
  
  	KASSERT(cd->cd_devs == NULL);
***************
*** 291,311 ****
  {
  	struct cfdriver *cd;
  
- 	/*
- 	 * It is sometimes necessary to use the autoconfiguration
- 	 * framework quite early (e.g. to initialize the console).
- 	 * We support this by noticing an empty cfdriver list and
- 	 * searching the initial static list instead.
- 	 */
- 	if (LIST_EMPTY(&allcfdrivers)) {
- 		int i;
- 
- 		for (i = 0; cfdriver_list_initial[i] != NULL; i++) {
- 			if (STREQ(cfdriver_list_initial[i]->cd_name, name))
- 				return (cfdriver_list_initial[i]);
- 		}
- 	}
- 
  	LIST_FOREACH(cd, &allcfdrivers, cd_list) {
  		if (STREQ(cd->cd_name, name))
  			return (cd);
--- 314,319 ----
***************
*** 315,320 ****
--- 323,410 ----
  }
  
  /*
+  * Add a cfattach to the specified driver.
+  */
+ int
+ config_cfattach_attach(const char *driver, struct cfattach *ca)
+ {
+ 	struct cfattach *lca;
+ 	struct cfdriver *cd;
+ 
+ 	cd = config_cfdriver_lookup(driver);
+ 	if (cd == NULL)
+ 		return (ESRCH);
+ 
+ 	/* Make sure this attachment isn't already on this driver. */
+ 	LIST_FOREACH(lca, &cd->cd_attach, ca_list) {
+ 		if (STREQ(lca->ca_name, ca->ca_name))
+ 			return (EEXIST);
+ 	}
+ 
+ 	LIST_INSERT_HEAD(&cd->cd_attach, ca, ca_list);
+ 
+ 	return (0);
+ }
+ 
+ /*
+  * Remove a cfattach from the specified driver.
+  */
+ int
+ config_cfattach_detach(const char *driver, struct cfattach *ca)
+ {
+ 	struct cfdriver *cd;
+ 	struct device *dev;
+ 	int i;
+ 
+ 	cd = config_cfdriver_lookup(driver);
+ 	if (cd == NULL)
+ 		return (ESRCH);
+ 
+ 	/* Make sure there are no active instances. */
+ 	for (i = 0; i < cd->cd_ndevs; i++) {
+ 		if ((dev = cd->cd_devs[i]) == NULL)
+ 			continue;
+ 		if (STREQ(dev->dv_cfdata->cf_atname, ca->ca_name))
+ 			return (EBUSY);
+ 	}
+ 
+ 	LIST_REMOVE(ca, ca_list);
+ 
+ 	return (0);
+ }
+ 
+ /*
+  * Look up a cfattach by name.
+  */
+ static struct cfattach *
+ config_cfattach_lookup_cd(struct cfdriver *cd, const char *atname)
+ {
+ 	struct cfattach *ca;
+ 
+ 	LIST_FOREACH(ca, &cd->cd_attach, ca_list) {
+ 		if (STREQ(ca->ca_name, atname))
+ 			return (ca);
+ 	}
+ 
+ 	return (NULL);
+ }
+ 
+ /*
+  * Look up a cfattach by driver/attachment name.
+  */
+ struct cfattach *
+ config_cfattach_lookup(const char *name, const char *atname)
+ {
+ 	struct cfdriver *cd;
+ 
+ 	cd = config_cfdriver_lookup(name);
+ 	if (cd == NULL)
+ 		return (NULL);
+ 
+ 	return (config_cfattach_lookup_cd(cd, atname));
+ }
+ 
+ /*
   * Apply the matching function and choose the best.  This is used
   * a few times and we want to keep the code small.
   */
***************
*** 326,336 ****
  	if (m->fn != NULL)
  		pri = (*m->fn)(m->parent, cf, m->aux);
  	else {
! 	        if (cf->cf_attach->ca_match == NULL) {
! 			panic("mapply: no match function for '%s' device",
! 			    cf->cf_name);
  		}
! 		pri = (*cf->cf_attach->ca_match)(m->parent, cf, m->aux);
  	}
  	if (pri > m->pri) {
  		m->match = cf;
--- 416,433 ----
  	if (m->fn != NULL)
  		pri = (*m->fn)(m->parent, cf, m->aux);
  	else {
! 		struct cfattach *ca;
! 
! 		ca = config_cfattach_lookup(cf->cf_name, cf->cf_atname);
! 		if (ca == NULL) {
! 			/* No attachment for this entry, oh well. */
! 			return;
  		}
! 	        if (ca->ca_match == NULL) {
! 			panic("mapply: no match function for '%s' attachment "
! 			    "of '%s'", cf->cf_atname, cf->cf_name);
! 		}
! 		pri = (*ca->ca_match)(m->parent, cf, m->aux);
  	}
  	if (pri > m->pri) {
  		m->match = cf;
***************
*** 402,409 ****
  int
  config_match(struct device *parent, struct cfdata *cf, void *aux)
  {
  
! 	return ((*cf->cf_attach->ca_match)(parent, cf, aux));
  }
  
  /*
--- 499,513 ----
  int
  config_match(struct device *parent, struct cfdata *cf, void *aux)
  {
+ 	struct cfattach *ca;
+ 
+ 	ca = config_cfattach_lookup(cf->cf_name, cf->cf_atname);
+ 	if (ca == NULL) {
+ 		/* No attachment for this entry, oh well. */
+ 		return (0);
+ 	}
  
! 	return ((*ca->ca_match)(parent, cf, aux));
  }
  
  /*
***************
*** 579,585 ****
  	struct device *dev;
  	struct cftable *ct;
  	struct cfdriver *cd;
! 	const struct cfattach *ca;
  	size_t lname, lunit;
  	const char *xunit;
  	int myunit;
--- 683,689 ----
  	struct device *dev;
  	struct cftable *ct;
  	struct cfdriver *cd;
! 	struct cfattach *ca;
  	size_t lname, lunit;
  	const char *xunit;
  	int myunit;
***************
*** 587,593 ****
  
  	cd = config_cfdriver_lookup(cf->cf_name);
  	KASSERT(cd != NULL);
! 	ca = cf->cf_attach;
  	if (ca->ca_devsize < sizeof(struct device))
  		panic("config_attach");
  
--- 691,700 ----
  
  	cd = config_cfdriver_lookup(cf->cf_name);
  	KASSERT(cd != NULL);
! 
! 	ca = config_cfattach_lookup_cd(cd, cf->cf_atname);
! 	KASSERT(ca != NULL);
! 
  	if (ca->ca_devsize < sizeof(struct device))
  		panic("config_attach");
  
***************
*** 631,636 ****
--- 738,745 ----
  	TAILQ_INSERT_TAIL(&alldevs, dev, dv_list);	/* link up */
  	dev->dv_class = cd->cd_class;
  	dev->dv_cfdata = cf;
+ 	dev->dv_cfdriver = cd;
+ 	dev->dv_cfattach = ca;
  	dev->dv_unit = myunit;
  	memcpy(dev->dv_xname, cd->cd_name, lname);
  	memcpy(dev->dv_xname + lname, xunit, lunit);
***************
*** 708,714 ****
  #endif
  	cd = config_cfdriver_lookup(cf->cf_name);
  	KASSERT(cd != NULL);
! 	ca = cf->cf_attach;
  
  	/*
  	 * Ensure the device is deactivated.  If the device doesn't
--- 817,825 ----
  #endif
  	cd = config_cfdriver_lookup(cf->cf_name);
  	KASSERT(cd != NULL);
! 
! 	ca = config_cfattach_lookup_cd(cd, cf->cf_atname);
! 	KASSERT(ca != NULL);
  
  	/*
  	 * Ensure the device is deactivated.  If the device doesn't
***************
*** 816,822 ****
  int
  config_activate(struct device *dev)
  {
! 	const struct cfattach *ca = dev->dv_cfdata->cf_attach;
  	int rv = 0, oflags = dev->dv_flags;
  
  	if (ca->ca_activate == NULL)
--- 927,933 ----
  int
  config_activate(struct device *dev)
  {
! 	const struct cfattach *ca = dev->dv_cfattach;
  	int rv = 0, oflags = dev->dv_flags;
  
  	if (ca->ca_activate == NULL)
***************
*** 834,840 ****
  int
  config_deactivate(struct device *dev)
  {
! 	const struct cfattach *ca = dev->dv_cfdata->cf_attach;
  	int rv = 0, oflags = dev->dv_flags;
  
  	if (ca->ca_activate == NULL)
--- 945,951 ----
  int
  config_deactivate(struct device *dev)
  {
! 	const struct cfattach *ca = dev->dv_cfattach;
  	int rv = 0, oflags = dev->dv_flags;
  
  	if (ca->ca_activate == NULL)
Index: sys/kern/subr_userconf.c
===================================================================
RCS file: /cvsroot/syssrc/sys/kern/subr_userconf.c,v
retrieving revision 1.8
diff -c -r1.8 subr_userconf.c
*** sys/kern/subr_userconf.c	2002/09/27 02:24:34	1.8
--- sys/kern/subr_userconf.c	2002/10/03 15:38:54
***************
*** 580,586 ****
  
  	userconf_cnt = 0;
  
! 	while (cfdata[i].cf_attach != 0) {
  		if (userconf_more())
  			break;
  		userconf_pdev(i++);
--- 580,586 ----
  
  	userconf_cnt = 0;
  
! 	while (cfdata[i].cf_name != NULL) {
  		if (userconf_more())
  			break;
  		userconf_pdev(i++);
***************
*** 606,612 ****
  		break;
  	}
  
! 	while (cfdata[i].cf_attach != 0) {
  		if (strlen(cfdata[i].cf_name) == len) {
  
  			/*
--- 606,612 ----
  		break;
  	}
  
! 	while (cfdata[i].cf_name != NULL) {
  		if (strlen(cfdata[i].cf_name) == len) {
  
  			/*
Index: sys/sys/device.h
===================================================================
RCS file: /cvsroot/syssrc/sys/sys/device.h,v
retrieving revision 1.59
diff -c -r1.59 device.h
*** sys/sys/device.h	2002/10/02 16:49:31	1.59
--- sys/sys/device.h	2002/10/03 15:38:55
***************
*** 108,113 ****
--- 108,115 ----
  	enum	devclass dv_class;	/* this device's classification */
  	TAILQ_ENTRY(device) dv_list;	/* entry on list of all devices */
  	struct	cfdata *dv_cfdata;	/* config data that found us */
+ 	struct	cfdriver *dv_cfdriver;	/* our cfdriver */
+ 	struct	cfattach *dv_cfattach;	/* our cfattach */
  	int	dv_unit;		/* device unit number */
  	char	dv_xname[16];		/* external name (name + unit) */
  	struct	device *dv_parent;	/* pointer to parent device */
***************
*** 179,185 ****
   */
  struct cfdata {
  	const char *cf_name;		/* driver name */
! 	const struct cfattach *cf_attach;/* config attachment */
  	short	cf_unit;		/* unit number */
  	short	cf_fstate;		/* finding state (below) */
  	int	*cf_loc;		/* locators (machine dependent) */
--- 181,187 ----
   */
  struct cfdata {
  	const char *cf_name;		/* driver name */
! 	const char *cf_atname;		/* attachment name */
  	short	cf_unit;		/* unit number */
  	short	cf_fstate;		/* finding state (below) */
  	int	*cf_loc;		/* locators (machine dependent) */
***************
*** 221,236 ****
   * structure array will be shared.
   */
  struct cfattach {
  	size_t	  ca_devsize;		/* size of dev data (for malloc) */
  	cfmatch_t ca_match;		/* returns a match level */
  	void	(*ca_attach)(struct device *, struct device *, void *);
  	int	(*ca_detach)(struct device *, int);
  	int	(*ca_activate)(struct device *, enum devact);
  };
  
  #define	CFATTACH_DECL(name, ddsize, matfn, attfn, detfn, actfn)		\
! const struct cfattach __CONCAT(name,_ca) = {				\
! 	ddsize, matfn, attfn, detfn, actfn				\
  }
  
  /* Flags given to config_detach(), and the ca_detach function. */
--- 223,241 ----
   * structure array will be shared.
   */
  struct cfattach {
+ 	const char *ca_name;		/* name of attachment */
+ 	LIST_ENTRY(cfattach) ca_list;	/* link on cfdriver's list */
  	size_t	  ca_devsize;		/* size of dev data (for malloc) */
  	cfmatch_t ca_match;		/* returns a match level */
  	void	(*ca_attach)(struct device *, struct device *, void *);
  	int	(*ca_detach)(struct device *, int);
  	int	(*ca_activate)(struct device *, enum devact);
  };
+ LIST_HEAD(cfattachlist, cfattach);
  
  #define	CFATTACH_DECL(name, ddsize, matfn, attfn, detfn, actfn)		\
! struct cfattach __CONCAT(name,_ca) = {					\
! 	___STRING(name), { }, ddsize, matfn, attfn, detfn, actfn	\
  }
  
  /* Flags given to config_detach(), and the ca_detach function. */
***************
*** 239,244 ****
--- 244,250 ----
  
  struct cfdriver {
  	LIST_ENTRY(cfdriver) cd_list;	/* link on allcfdrivers */
+ 	struct cfattachlist cd_attach;	/* list of all attachments */
  	void	**cd_devs;		/* devices found */
  	const char *cd_name;		/* device name */
  	enum	devclass cd_class;	/* device classification */
***************
*** 249,258 ****
  
  #define	CFDRIVER_DECL(name, class, attrs)				\
  struct cfdriver __CONCAT(name,_cd) = {					\
! 	{ }, NULL, ___STRING(name), class, 0, attrs			\
  }
  
  /*
   * Configuration printing functions, and their return codes.  The second
   * argument is NULL if the device was configured; otherwise it is the name
   * of the parent device.  The return value is ignored if the device was
--- 255,273 ----
  
  #define	CFDRIVER_DECL(name, class, attrs)				\
  struct cfdriver __CONCAT(name,_cd) = {					\
! 	{ }, { }, NULL, ___STRING(name), class, 0, attrs		\
  }
  
  /*
+  * The cfattachinit is a data structure used to associate a list of
+  * cfattach's with cfdrivers as found in the static kernel configuration.
+  */
+ struct cfattachinit {
+ 	const char *cfai_name;		 /* driver name */
+ 	struct cfattach * const *cfai_list;/* list of attachments */
+ };
+ 
+ /*
   * Configuration printing functions, and their return codes.  The second
   * argument is NULL if the device was configured; otherwise it is the name
   * of the parent device.  The return value is ignored if the device was
***************
*** 286,291 ****
--- 301,311 ----
  
  int	config_cfdriver_attach(struct cfdriver *);
  int	config_cfdriver_detach(struct cfdriver *);
+ 
+ int	config_cfattach_attach(const char *, struct cfattach *);
+ int	config_cfattach_detach(const char *, struct cfattach *);
+ 
+ struct cfattach *config_cfattach_lookup(const char *, const char *);
  
  struct cfdata *config_search(cfmatch_t, struct device *, void *);
  struct cfdata *config_rootsearch(cfmatch_t, const char *, void *);
Index: usr.sbin/config/mkioconf.c
===================================================================
RCS file: /cvsroot/syssrc/usr.sbin/config/mkioconf.c,v
retrieving revision 1.67
diff -c -r1.67 mkioconf.c
*** usr.sbin/config/mkioconf.c	2002/10/02 16:49:03	1.67
--- usr.sbin/config/mkioconf.c	2002/10/03 15:38:56
***************
*** 59,64 ****
--- 59,65 ----
  static int emitcfdata(FILE *);
  static int emitcfdrivers(FILE *);
  static int emitexterns(FILE *);
+ static int emitcfattachinit(FILE *);
  static int emithdr(FILE *);
  static int emitloc(FILE *);
  static int emitpseudo(FILE *);
***************
*** 90,98 ****
  		return (1);
  	}
  	v = emithdr(fp);
! 	if (v != 0 || emitcfdrivers(fp) || emitexterns(fp) || emitloc(fp) ||
! 	    emitparents(fp) || emitcfdata(fp) || emitroots(fp) ||
! 	    emitpseudo(fp) || emitvfslist(fp) ||
  	    (do_devsw ? 0 : emitname2blk(fp))) {
  		if (v >= 0)
  			(void)fprintf(stderr,
--- 91,100 ----
  		return (1);
  	}
  	v = emithdr(fp);
! 	if (v != 0 || emitcfdrivers(fp) || emitexterns(fp) ||
! 	    emitcfattachinit(fp) || emitloc(fp) || emitparents(fp) ||
! 	    emitcfdata(fp) || emitroots(fp) || emitpseudo(fp) ||
! 	    emitvfslist(fp) ||
  	    (do_devsw ? 0 : emitname2blk(fp))) {
  		if (v >= 0)
  			(void)fprintf(stderr,
***************
*** 230,240 ****
  	TAILQ_FOREACH(da, &alldevas, d_next) {
  		if (!deva_has_instances(da, WILD))
  			continue;
! 		if (fprintf(fp, "extern const struct cfattach %s_ca;\n",
  			    da->d_name) < 0)
  			return (1);
  	}
  	NEWLINE;
  	return (0);
  }
  
--- 232,290 ----
  	TAILQ_FOREACH(da, &alldevas, d_next) {
  		if (!deva_has_instances(da, WILD))
  			continue;
! 		if (fprintf(fp, "extern struct cfattach %s_ca;\n",
  			    da->d_name) < 0)
  			return (1);
  	}
+ 	return (0);
+ }
+ 
+ static int
+ emitcfattachinit(FILE *fp)
+ {
+ 	struct devbase *d;
+ 	struct deva *da;
+ 
  	NEWLINE;
+ 	TAILQ_FOREACH(d, &allbases, d_next) {
+ 		if (!devbase_has_instances(d, WILD))
+ 			continue;
+ 		if (d->d_ahead == NULL)
+ 			continue;
+ 
+ 		if (fprintf(fp,
+ "static struct cfattach * const %s_cfattachinit[] = {\n\t",
+ 			    d->d_name) < 0)
+ 			return (1);
+ 		for (da = d->d_ahead; da != NULL; da = da->d_bsame) {
+ 			if (!deva_has_instances(da, WILD))
+ 				continue;
+ 			if (fprintf(fp, "&%s_ca, ", da->d_name) < 0)
+ 				return (1);
+ 		}
+ 		if (fprintf(fp, "NULL\n};\n") < 0)
+ 			return (1);
+ 	}
+ 
+ 	NEWLINE;
+ 	if (fprintf(fp,
+ "const struct cfattachinit cfattachinit[] = {\n") < 0)
+ 		return (1);
+ 
+ 	TAILQ_FOREACH(d, &allbases, d_next) {
+ 		if (!devbase_has_instances(d, WILD))
+ 			continue;
+ 		if (d->d_ahead == NULL)
+ 			continue;
+ 
+ 		if (fprintf(fp, "\t{ \"%s\", %s_cfattachinit },\n",
+ 			    d->d_name, d->d_name) < 0)
+ 			return (1);
+ 	}
+ 
+ 	if (fprintf(fp, "\t{ NULL, NULL }\n};\n") < 0)
+ 		return (1);
+ 
  	return (0);
  }
  
***************
*** 408,417 ****
  			loc = locbuf;
  		} else
  			loc = "loc";
! 		if (fprintf(fp, "    {\"%s\",%s&%s_ca,%s%2d, %s, %7s, %#6x, ",
  			    basename, strlen(basename) < 8 ? "\t\t"
  			    				   : "\t",
! 			    attachment, strlen(attachment) < 3 ? "\t\t"
  			    				       : "\t",
  			    unit, state, loc, i->i_cfflags) < 0)
  			return (1);
--- 458,467 ----
  			loc = locbuf;
  		} else
  			loc = "loc";
! 		if (fprintf(fp, "    {\"%s\",%s\"%s\",%s%2d, %s, %7s, %#6x, ",
  			    basename, strlen(basename) < 8 ? "\t\t"
  			    				   : "\t",
! 			    attachment, strlen(attachment) < 5 ? "\t\t"
  			    				       : "\t",
  			    unit, state, loc, i->i_cfflags) < 0)
  			return (1);

--UlVJffcvxoiEqYs2--