making order in file2alias

RFC PATCH In review Alessandro Rubini 3 participants 8 messages Dec 1, 2011 – Dec 4, 2011 Kbuild

lore

Share & tools

lore

Alessandro Rubini

author

2011-12-01 23:45 UTC #0 lore raw

v2 cover making order in file2alias

(this message is the RFC, the patch itself is expected to be fine)

This is a repost of what I've sent on Nov 4th. Since Rusty asked to only do the first step, here it is. I rebased on next-20111201 and it still works fine.

As for the previous 2/2 (see https://lkml.org/lkml/2011/11/4/127 ), I still think it makes sense. And as Dave Martin suggested, we can do without the array.

Thus, I may prepare three smaller steps, if that's acceptable (greg?)

step 1: create the ELF section so ENTRY() lines can leave the array
(and change name accordingly).

step 2: each ENTRY() line can be moved just after the associated code
(this means that a new bus is just a single hunk, not 2 of them)

step 3: I create the headers needed to move code and ENTRY in separate
files. This is some movement around, not trivial so it may deserve a patch in itself.

step 4 and later ones: Individual busses may reach their own external file,
conditionally compiled per Kconfig rules.

If that's something worth evaluating, I can do that over the weekend.

thanks
/alessandro

Alessandro Rubini (1):
modpost: use table-lookup to build module aliases

 scripts/mod/file2alias.c |  166 +++++++++++++++++-----------------------------
 1 files changed, 60 insertions(+), 106 deletions(-)

Signature · 1 line

1.7.7.2

Alessandro Rubini

author

2011-12-01 23:45 UTC #1 lore raw

PATCH v2 1/1 modpost: use table-lookup to build module aliases

The function handle_moddevtable was a huge "else if" thing. This patch places the lookup in a table to reduce code and increase data.

Signed-off-by Alessandro Rubini <[email protected]>

Acked-by Samuel I. Gonsalvez <[email protected]>

Acked-by Manohar Vanga <[email protected]>

Cc Dave Martin <[email protected]>

---

Changes in V2:
- The macro is ENTRY() not E()
- All array members are split in two lines (most won't fit 80 cols)
- The array is sorted (but the associated code functions are not)

 scripts/mod/file2alias.c |  166 +++++++++++++++++-----------------------------
 1 files changed, 60 insertions(+), 106 deletions(-)

scripts/mod/file2alias.c +60 ~~-106~~

index f936d1f..1247bc9 100644
@@ -913,6 +913,51 @@ static void do_table(void *symval, unsigned long size,
 	}
 }
 
+/* This array collects all instances that use the generic do_table above */
+struct devtable_switch {
+	const char *device_id;
+	const char *symname;
+	unsigned long id_size;
+	void *function;
+};
+#define E(id, name, structname, f) {id, name, sizeof(struct structname), f}
+
+static struct devtable_switch devtable_switch[] = {
+	E("pci", "__mod_pci_device_table", pci_device_id, do_pci_entry),
+	E("hid", "__mod_hid_device_table", hid_device_id, do_hid_entry),
+	E("ieee1394", "__mod_ieee1394_device_table",
+	  ieee1394_device_id, do_ieee1394_entry),
+	E("ccw", "__mod_ccw_device_table", ccw_device_id, do_ccw_entry),
+	E("ap", "__mod_ap_device_table", ap_device_id, do_ap_entry),
+	E("css", "__mod_css_device_table", css_device_id, do_css_entry),
+	E("seio", "__mod_serio_device_table", serio_device_id, do_serio_entry),
+	E("acpi", "__mod_acpi_device_table", acpi_device_id, do_acpi_entry),
+	E("pcmcia", "__mod_pcmcia_device_table",
+	  pcmcia_device_id, do_pcmcia_entry),
+	E("of", "__mod_of_device_table", of_device_id, do_of_entry),
+	E("vio", "__mod_vio_device_table", vio_device_id, do_vio_entry),
+	E("input", "__mod_input_device_table", input_device_id, do_input_entry),
+	E("eisa", "__mod_eisa_device_table", eisa_device_id, do_eisa_entry),
+	E("parisc", "__mod_parisc_device_table",
+	  parisc_device_id, do_parisc_entry),
+	E("sdio", "__mod_sdio_device_table", sdio_device_id, do_sdio_entry),
+	E("ssb", "__mod_ssb_device_table", ssb_device_id, do_ssb_entry),
+	E("bcma", "__mod_bcma_device_table", bcma_device_id, do_bcma_entry),
+	E("virtio", "__mod_virtio_device_table",
+	  virtio_device_id, do_virtio_entry),
+	E("vmbus", "__mod_vmbus_device_table",
+	  hv_vmbus_device_id, do_vmbus_entry),
+	E("i2c", "__mod_i2c_device_table", i2c_device_id, do_i2c_entry),
+	E("spi", "__mod_spi_device_table", spi_device_id, do_spi_entry),
+	E("dmi", "__mod_dmi_device_table", dmi_system_id, do_dmi_entry),
+	E("platform", "__mod_platform_device_table",
+	  platform_device_id, do_platform_entry),
+	E("mdio", "__mod_mdio_device_table", mdio_device_id, do_mdio_entry),
+	E("zorro", "__mod_zorro_device_table", zorro_device_id, do_zorro_entry),
+	E("isa", "__mod_isapnp_device_table",
+	  isapnp_device_id, do_isapnp_entry)
+};
+
 /* Create MODULE_ALIAS() statements.
  * At this time, we cannot write the actual output C source yet,
  * so we write into the mod->dev_table_buf buffer. */

@@ -936,117 +981,26 @@ void handle_moddevtable(struct module *mod, struct elf_info *info,
 			+ sym->st_value;
 	}
 
-	if (sym_is(symname, "__mod_pci_device_table"))
-		do_table(symval, sym->st_size,
-			 sizeof(struct pci_device_id), "pci",
-			 do_pci_entry, mod);
-	else if (sym_is(symname, "__mod_usb_device_table"))
-		/* special case to handle bcdDevice ranges */
+	/* First handle the "special" cases */
+	if (sym_is(symname, "__mod_usb_device_table"))
 		do_usb_table(symval, sym->st_size, mod);
-	else if (sym_is(symname, "__mod_hid_device_table"))
-		do_table(symval, sym->st_size,
-			 sizeof(struct hid_device_id), "hid",
-			 do_hid_entry, mod);
-	else if (sym_is(symname, "__mod_ieee1394_device_table"))
-		do_table(symval, sym->st_size,
-			 sizeof(struct ieee1394_device_id), "ieee1394",
-			 do_ieee1394_entry, mod);
-	else if (sym_is(symname, "__mod_ccw_device_table"))
-		do_table(symval, sym->st_size,
-			 sizeof(struct ccw_device_id), "ccw",
-			 do_ccw_entry, mod);
-	else if (sym_is(symname, "__mod_ap_device_table"))
-		do_table(symval, sym->st_size,
-			 sizeof(struct ap_device_id), "ap",
-			 do_ap_entry, mod);
-	else if (sym_is(symname, "__mod_css_device_table"))
-		do_table(symval, sym->st_size,
-			 sizeof(struct css_device_id), "css",
-			 do_css_entry, mod);
-	else if (sym_is(symname, "__mod_serio_device_table"))
-		do_table(symval, sym->st_size,
-			 sizeof(struct serio_device_id), "serio",
-			 do_serio_entry, mod);
-	else if (sym_is(symname, "__mod_acpi_device_table"))
-		do_table(symval, sym->st_size,
-			 sizeof(struct acpi_device_id), "acpi",
-			 do_acpi_entry, mod);
 	else if (sym_is(symname, "__mod_pnp_device_table"))
 		do_pnp_device_entry(symval, sym->st_size, mod);
 	else if (sym_is(symname, "__mod_pnp_card_device_table"))
 		do_pnp_card_entries(symval, sym->st_size, mod);
-	else if (sym_is(symname, "__mod_pcmcia_device_table"))
-		do_table(symval, sym->st_size,
-			 sizeof(struct pcmcia_device_id), "pcmcia",
-			 do_pcmcia_entry, mod);
-        else if (sym_is(symname, "__mod_of_device_table"))
-		do_table(symval, sym->st_size,
-			 sizeof(struct of_device_id), "of",
-			 do_of_entry, mod);
-        else if (sym_is(symname, "__mod_vio_device_table"))
-		do_table(symval, sym->st_size,
-			 sizeof(struct vio_device_id), "vio",
-			 do_vio_entry, mod);
-	else if (sym_is(symname, "__mod_input_device_table"))
-		do_table(symval, sym->st_size,
-			 sizeof(struct input_device_id), "input",
-			 do_input_entry, mod);
-	else if (sym_is(symname, "__mod_eisa_device_table"))
-		do_table(symval, sym->st_size,
-			 sizeof(struct eisa_device_id), "eisa",
-			 do_eisa_entry, mod);
-	else if (sym_is(symname, "__mod_parisc_device_table"))
-		do_table(symval, sym->st_size,
-			 sizeof(struct parisc_device_id), "parisc",
-			 do_parisc_entry, mod);
-	else if (sym_is(symname, "__mod_sdio_device_table"))
-		do_table(symval, sym->st_size,
-			 sizeof(struct sdio_device_id), "sdio",
-			 do_sdio_entry, mod);
-	else if (sym_is(symname, "__mod_ssb_device_table"))
-		do_table(symval, sym->st_size,
-			 sizeof(struct ssb_device_id), "ssb",
-			 do_ssb_entry, mod);
-	else if (sym_is(symname, "__mod_bcma_device_table"))
-		do_table(symval, sym->st_size,
-			 sizeof(struct bcma_device_id), "bcma",
-			 do_bcma_entry, mod);
-	else if (sym_is(symname, "__mod_virtio_device_table"))
-		do_table(symval, sym->st_size,
-			 sizeof(struct virtio_device_id), "virtio",
-			 do_virtio_entry, mod);
-	else if (sym_is(symname, "__mod_vmbus_device_table"))
-		do_table(symval, sym->st_size,
-			 sizeof(struct hv_vmbus_device_id), "vmbus",
-			 do_vmbus_entry, mod);
-	else if (sym_is(symname, "__mod_i2c_device_table"))
-		do_table(symval, sym->st_size,
-			 sizeof(struct i2c_device_id), "i2c",
-			 do_i2c_entry, mod);
-	else if (sym_is(symname, "__mod_spi_device_table"))
-		do_table(symval, sym->st_size,
-			 sizeof(struct spi_device_id), "spi",
-			 do_spi_entry, mod);
-	else if (sym_is(symname, "__mod_dmi_device_table"))
-		do_table(symval, sym->st_size,
-			 sizeof(struct dmi_system_id), "dmi",
-			 do_dmi_entry, mod);
-	else if (sym_is(symname, "__mod_platform_device_table"))
-		do_table(symval, sym->st_size,
-			 sizeof(struct platform_device_id), "platform",
-			 do_platform_entry, mod);
-	else if (sym_is(symname, "__mod_mdio_device_table"))
-		do_table(symval, sym->st_size,
-			 sizeof(struct mdio_device_id), "mdio",
-			 do_mdio_entry, mod);
-	else if (sym_is(symname, "__mod_zorro_device_table"))
-		do_table(symval, sym->st_size,
-			 sizeof(struct zorro_device_id), "zorro",
-			 do_zorro_entry, mod);
-	else if (sym_is(symname, "__mod_isapnp_device_table"))
-		do_table(symval, sym->st_size,
-			sizeof(struct isapnp_device_id), "isa",
-			do_isapnp_entry, mod);
+	else {
+		struct devtable_switch *p = devtable_switch;
+		int i;
+
+		/* scan the array */
+		for (i = 0; i < ARRAY_SIZE(devtable_switch); i++, p++) {
+			if (sym_is(symname, p->symname)) {
+				do_table(symval, sym->st_size, p->id_size,
+					 p->device_id, p->function, mod);
+				break;
+			}
+		}
+	}
 	free(zeros);
 }

Signature · 1 line

1.7.7.2

Greg KH

2011-12-02 00:04 UTC #3 lore raw

In reply to Alessandro Rubini

Quoted 5 lines · Alessandro Rubini

(this message is the RFC, the patch itself is expected to be fine)
This is a repost of what I've sent on Nov 4th. Since Rusty asked to only do the first step, here it is. I rebased on next-20111201 and it still works fine.

Well, some of us disagree with Rusty :)

As he's still the maintainer of the module code, I'll defer to him though.

Quoted 12 lines

As for the previous 2/2 (see https://lkml.org/lkml/2011/11/4/127 ), I still think it makes sense. And as Dave Martin suggested, we can do without the array.
Thus, I may prepare three smaller steps, if that's acceptable (greg?)
step 1: create the ELF section so ENTRY() lines can leave the array
(and change name accordingly).
step 2: each ENTRY() line can be moved just after the associated code
(this means that a new bus is just a single hunk, not 2 of them)

Quoted 8 lines

step 3: I create the headers needed to move code and ENTRY in separate
files. This is some movement around, not trivial so it may deserve a patch in itself.
step 4 and later ones: Individual busses may reach their own external file,
conditionally compiled per Kconfig rules.
If that's something worth evaluating, I can do that over the weekend.

That sounds very reasonable to me.

thanks,

greg k-h

Greg KH

Acked-by Greg Kroah-Hartman on v2 1/1

2011-12-02 00:05 UTC #2 lore raw

Re: [PATCH V2 1/1] modpost: use table-lookup to build module aliases

In reply to Alessandro Rubini

Quoted 7 lines · Alessandro Rubini

The function handle_moddevtable was a huge "else if" thing. This patch places the lookup in a table to reduce code and increase data.
Signed-off-by Alessandro Rubini <[email protected]>
Acked-by Samuel I. Gonsalvez <[email protected]>
Acked-by Manohar Vanga <[email protected]>
Cc Dave Martin <[email protected]>

Rusty, do you want to take this one in your tree, or should I take it through mine? Either is fine with me, if you want it to go through yours, feel free to add:

Acked-by Greg Kroah-Hartman <[email protected]>

to it.

thanks,

greg k-h

Rusty Russell

2011-12-02 02:48 UTC #4 lore raw

In reply to Greg KH

Quoted 8 lines · Greg KH

Quoted 5 lines · Alessandro Rubini
(this message is the RFC, the patch itself is expected to be fine)
This is a repost of what I've sent on Nov 4th. Since Rusty asked to only do the first step, here it is. I rebased on next-20111201 and it still works fine.
Well, some of us disagree with Rusty :)

These patches are bikeshedding, but I am genuinely curious about your reasoning, especially since you invoked Linus and Andrew in support.

The first patch makes it more compact, but a bit less simple. Hard to care either way.

And notice that it broke serio. Big lose for a "tidyup".

But splitting 15-line functions into separate files? Less compact, less simple. And now you have to patch two files. The argument that this is symmetric with how we write new drivers is pretty bogus IMHO: adding a dozen lines to one file is definitely easier than a new file and adding all the boilerplate.

Quoted 2 lines

As he's still the maintainer of the module code, I'll defer to him though.

Well, modpost is kind of a no-mans-land, but I'm happy to own this.

If you want to get rid of the table, how's this (below, on top of my previous patch). A bit marginal, but keeps things together.

Quoted 12 lines

Quoted 12 lines
Thus, I may prepare three smaller steps, if that's acceptable (greg?)
step 1: create the ELF section so ENTRY() lines can leave the array
(and change name accordingly).
step 2: each ENTRY() line can be moved just after the associated code
(this means that a new bus is just a single hunk, not 2 of them)
step 3: I create the headers needed to move code and ENTRY in separate
files. This is some movement around, not trivial so it may deserve a patch in itself.

Quoted 6 lines

Quoted 4 lines
step 4 and later ones: Individual busses may reach their own external file,
conditionally compiled per Kconfig rules.
If that's something worth evaluating, I can do that over the weekend.
That sounds very reasonable to me.

If you *really* want to use separate files, then this patch will let you do it. You'll need to expose some stuff in a header though.

Thanks,
Rusty.

From: Rusty Russell <[email protected]>
Subject: modpost: use linker section to generate table.

This means (most) future busses need only have one hunk in their patch. Also took the opportunity to check that function matches the type.

Again, inspired by Alessandro's patch series.

Signed-off-by Rusty Russell <[email protected]>

Cc Alessandro Rubini <[email protected]>

scripts/mod/file2alias.c +60 ~~-44~~

@@ -39,6 +39,35 @@ typedef unsigned char	__u8;
  * we handle those differences explicitly below */
 #include "../../include/linux/mod_devicetable.h"
 
+/* This array collects all instances that use the generic do_table */
+struct devtable {
+	const char *device_id; /* name of table, __mod_<name>_device_table. */
+	unsigned long id_size;
+	void *function;
+};
+
+/* We construct a table of pointers in an ELF section (pointers generally
+ * go unpadded by gcc).  ld creates boundary syms for us. */
+extern struct devtable *__start___devtable[], *__stop___devtable[];
+#define ___cat(a,b) a ## b
+#define __cat(a,b) ___cat(a,b)
+
+#if __GNUC__ == 3 && __GNUC_MINOR__ < 3
+# define __used			__attribute__((__unused__))
+#else
+# define __used			__attribute__((__used__))
+#endif
+
+/* Add a table entry.  We test function type matches while we're here. */
+#define ADD_TO_DEVTABLE(device_id, type, function) \
+	static struct devtable __cat(devtable,__LINE__) = {	\
+		device_id + 0*sizeof((function)((const char *)NULL,	\
+						(type *)NULL,		\
+						(char *)NULL)),		\
+		sizeof(type), (function) };				\
+	static struct devtable *__attribute__((section("__devtable"))) \
+		__used __cat(devtable_ptr,__LINE__) = &__cat(devtable,__LINE__)
+
 #define ADD(str, sep, cond, field)                              \
 do {                                                            \
         strcat(str, sep);                                       \

@@ -290,6 +319,7 @@ static int do_hid_entry(const char *file
 
 	return 1;
 }
+ADD_TO_DEVTABLE("hid", struct hid_device_id, do_hid_entry);
 
 /* Looks like: ieee1394:venNmoNspNverN */
 static int do_ieee1394_entry(const char *filename,

@@ -314,6 +344,7 @@ static int do_ieee1394_entry(const char 
 	add_wildcard(alias);
 	return 1;
 }
+ADD_TO_DEVTABLE("ieee1394", struct ieee1394_device_id, do_ieee1394_entry);
 
 /* Looks like: pci:vNdNsvNsdNbcNscNiN. */
 static int do_pci_entry(const char *filename,

@@ -357,6 +388,7 @@ static int do_pci_entry(const char *file
 	add_wildcard(alias);
 	return 1;
 }
+ADD_TO_DEVTABLE("pci", struct pci_device_id, do_pci_entry);
 
 /* looks like: "ccw:tNmNdtNdmN" */
 static int do_ccw_entry(const char *filename,

@@ -380,6 +412,7 @@ static int do_ccw_entry(const char *file
 	add_wildcard(alias);
 	return 1;
 }
+ADD_TO_DEVTABLE("ccw", struct ccw_device_id, do_ccw_entry);
 
 /* looks like: "ap:tN" */
 static int do_ap_entry(const char *filename,

@@ -388,6 +421,7 @@ static int do_ap_entry(const char *filen
 	sprintf(alias, "ap:t%02X*", id->dev_type);
 	return 1;
 }
+ADD_TO_DEVTABLE("ap", struct ap_device_id, do_ap_entry);
 
 /* looks like: "css:tN" */
 static int do_css_entry(const char *filename,

@@ -396,6 +430,7 @@ static int do_css_entry(const char *file
 	sprintf(alias, "css:t%01X", id->type);
 	return 1;
 }
+ADD_TO_DEVTABLE("css", struct css_device_id, do_css_entry);
 
 /* Looks like: "serio:tyNprNidNexN" */
 static int do_serio_entry(const char *filename,

@@ -415,6 +450,7 @@ static int do_serio_entry(const char *fi
 	add_wildcard(alias);
 	return 1;
 }
+ADD_TO_DEVTABLE("serio", struct serio_device_id, do_serio_entry);
 
 /* looks like: "acpi:ACPI0003 or acpi:PNP0C0B" or "acpi:LNXVIDEO" */
 static int do_acpi_entry(const char *filename,

@@ -423,6 +459,7 @@ static int do_acpi_entry(const char *fil
 	sprintf(alias, "acpi*:%s:*", id->id);
 	return 1;
 }
+ADD_TO_DEVTABLE("acpi", struct acpi_device_id, do_acpi_entry);
 
 /* looks like: "pnp:dD" */
 static void do_pnp_device_entry(void *symval, unsigned long size,

@@ -545,8 +582,7 @@ static int do_pcmcia_entry(const char *f
 	add_wildcard(alias);
        return 1;
 }
-
-
+ADD_TO_DEVTABLE("pcmcia", struct pcmcia_device_id, do_pcmcia_entry);
 
 static int do_of_entry (const char *filename, struct of_device_id *of, char *alias)
 {

@@ -569,6 +605,7 @@ static int do_of_entry (const char *file
     add_wildcard(alias);
     return 1;
 }
+ADD_TO_DEVTABLE("of", struct of_device_id, do_of_entry);
 
 static int do_vio_entry(const char *filename, struct vio_device_id *vio,
 		char *alias)

@@ -586,6 +623,7 @@ static int do_vio_entry(const char *file
 	add_wildcard(alias);
 	return 1;
 }
+ADD_TO_DEVTABLE("vio", struct vio_device_id, do_vio_entry);
 
 #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))

@@ -641,6 +679,7 @@ static int do_input_entry(const char *fi
 		do_input(alias, id->swbit, 0, INPUT_DEVICE_ID_SW_MAX);
 	return 1;
 }
+ADD_TO_DEVTABLE("input", struct input_device_id, do_input_entry);
 
 static int do_eisa_entry(const char *filename, struct eisa_device_id *eisa,
 		char *alias)

@@ -651,6 +690,7 @@ static int do_eisa_entry(const char *fil
 		strcat(alias, "*");
 	return 1;
 }
+ADD_TO_DEVTABLE("eisa", struct eisa_device_id, do_eisa_entry);
 
 /* Looks like: parisc:tNhvNrevNsvN */
 static int do_parisc_entry(const char *filename, struct parisc_device_id *id,

@@ -670,6 +710,7 @@ static int do_parisc_entry(const char *f
 	add_wildcard(alias);
 	return 1;
 }
+ADD_TO_DEVTABLE("parisc", struct parisc_device_id, do_parisc_entry);
 
 /* Looks like: sdio:cNvNdN. */
 static int do_sdio_entry(const char *filename,

@@ -686,6 +727,7 @@ static int do_sdio_entry(const char *fil
 	add_wildcard(alias);
 	return 1;
 }
+ADD_TO_DEVTABLE("sdio", struct sdio_device_id, do_sdio_entry);
 
 /* Looks like: ssb:vNidNrevN. */
 static int do_ssb_entry(const char *filename,

@@ -702,6 +744,7 @@ static int do_ssb_entry(const char *file
 	add_wildcard(alias);
 	return 1;
 }
+ADD_TO_DEVTABLE("ssb", struct ssb_device_id, do_ssb_entry);
 
 /* Looks like: bcma:mNidNrevNclN. */
 static int do_bcma_entry(const char *filename,

@@ -720,6 +763,7 @@ static int do_bcma_entry(const char *fil
 	add_wildcard(alias);
 	return 1;
 }
+ADD_TO_DEVTABLE("bcma", struct bcma_device_id, do_bcma_entry);
 
 /* Looks like: virtio:dNvN */
 static int do_virtio_entry(const char *filename, struct virtio_device_id *id,

@@ -735,6 +779,7 @@ static int do_virtio_entry(const char *f
 	add_wildcard(alias);
 	return 1;
 }
+ADD_TO_DEVTABLE("virtio", struct virtio_device_id, do_virtio_entry);
 
 /*
  * Looks like: vmbus:guid

@@ -756,6 +801,7 @@ static int do_vmbus_entry(const char *fi
 
 	return 1;
 }
+ADD_TO_DEVTABLE("vmbus", struct hv_vmbus_device_id, do_vmbus_entry);
 
 /* Looks like: i2c:S */
 static int do_i2c_entry(const char *filename, struct i2c_device_id *id,

@@ -765,6 +811,7 @@ static int do_i2c_entry(const char *file
 
 	return 1;
 }
+ADD_TO_DEVTABLE("i2c", struct i2c_device_id, do_i2c_entry);
 
 /* Looks like: spi:S */
 static int do_spi_entry(const char *filename, struct spi_device_id *id,

@@ -774,6 +821,7 @@ static int do_spi_entry(const char *file
 
 	return 1;
 }
+ADD_TO_DEVTABLE("spi", struct spi_device_id, do_spi_entry);
 
 static const struct dmifield {
 	const char *prefix;

@@ -828,6 +876,7 @@ static int do_dmi_entry(const char *file
 	strcat(alias, ":");
 	return 1;
 }
+ADD_TO_DEVTABLE("dmi", struct dmi_system_id, do_dmi_entry);
 
 static int do_platform_entry(const char *filename,
 			     struct platform_device_id *id, char *alias)

@@ -835,6 +884,7 @@ static int do_platform_entry(const char 
 	sprintf(alias, PLATFORM_MODULE_PREFIX "%s", id->name);
 	return 1;
 }
+ADD_TO_DEVTABLE("platform", struct platform_device_id, do_platform_entry);
 
 static int do_mdio_entry(const char *filename,
 			 struct mdio_device_id *id, char *alias)

@@ -857,6 +907,7 @@ static int do_mdio_entry(const char *fil
 
 	return 1;
 }
+ADD_TO_DEVTABLE("mdio", struct mdio_device_id, do_mdio_entry);
 
 /* Looks like: zorro:iN. */
 static int do_zorro_entry(const char *filename, struct zorro_device_id *id,

@@ -867,6 +918,7 @@ static int do_zorro_entry(const char *fi
 	ADD(alias, "i", id->id != ZORRO_WILDCARD, id->id);
 	return 1;
 }
+ADD_TO_DEVTABLE("zorro", struct zorro_device_id, do_zorro_entry);
 
 /* looks like: "pnp:dD" */
 static int do_isapnp_entry(const char *filename,

@@ -880,6 +932,7 @@ static int do_isapnp_entry(const char *f
 		(id->function >> 12) & 0x0f, (id->function >> 8) & 0x0f);
 	return 1;
 }
+ADD_TO_DEVTABLE("isa", struct isapnp_device_id, do_isapnp_entry);
 
 /* Does namelen bytes of name exactly match the symbol? */
 static bool sym_is(const char *name, unsigned namelen, const char *symbol)

@@ -912,42 +965,6 @@ static void do_table(void *symval, unsig
 	}
 }
 
-/* This array collects all instances that use the generic do_table above */
-struct devtable_switch {
-	const char *device_id; /* name of table, __mod_<name>_device_table. */
-	unsigned long id_size;
-	void *function;
-};
-
-static const struct devtable_switch devtable_switch[] = {
-	{ "acpi", sizeof(struct acpi_device_id), do_acpi_entry },
-	{ "ap", sizeof(struct ap_device_id), do_ap_entry },
-	{ "bcma", sizeof(struct bcma_device_id), do_bcma_entry },
-	{ "ccw", sizeof(struct ccw_device_id), do_ccw_entry },
-	{ "css", sizeof(struct css_device_id), do_css_entry },
-	{ "dmi", sizeof(struct dmi_system_id), do_dmi_entry },
-	{ "eisa", sizeof(struct eisa_device_id), do_eisa_entry },
-	{ "hid", sizeof(struct hid_device_id), do_hid_entry },
-	{ "i2c", sizeof(struct i2c_device_id), do_i2c_entry },
-	{ "ieee1394", sizeof(struct ieee1394_device_id), do_ieee1394_entry },
-	{ "input", sizeof(struct input_device_id), do_input_entry },
-	{ "isa", sizeof(struct isapnp_device_id), do_isapnp_entry },
-	{ "mdio", sizeof(struct mdio_device_id), do_mdio_entry },
-	{ "of", sizeof(struct of_device_id), do_of_entry },
-	{ "parisc", sizeof(struct parisc_device_id), do_parisc_entry },
-	{ "pci", sizeof(struct pci_device_id), do_pci_entry },
-	{ "pcmcia", sizeof(struct pcmcia_device_id), do_pcmcia_entry },
-	{ "platform", sizeof(struct platform_device_id), do_platform_entry },
-	{ "sdio", sizeof(struct sdio_device_id), do_sdio_entry },
-	{ "serio", sizeof(struct serio_device_id), do_serio_entry },
-	{ "spi", sizeof(struct spi_device_id), do_spi_entry },
-	{ "ssb", sizeof(struct ssb_device_id), do_ssb_entry },
-	{ "vio", sizeof(struct vio_device_id), do_vio_entry },
-	{ "virtio", sizeof(struct virtio_device_id), do_virtio_entry },
-	{ "vmbus", sizeof(struct hv_vmbus_device_id), do_vmbus_entry },
-	{ "zorro", sizeof(struct zorro_device_id), do_zorro_entry },
-};
-
 /* Create MODULE_ALIAS() statements.
  * At this time, we cannot write the actual output C source yet,
  * so we write into the mod->dev_table_buf buffer. */

@@ -993,13 +1010,12 @@ void handle_moddevtable(struct module *m
 	else if (sym_is(name, namelen, "pnp_card"))
 		do_pnp_card_entries(symval, sym->st_size, mod);
 	else {
-		const struct devtable_switch *p = devtable_switch;
-		unsigned int i;
+		struct devtable **p;
 
-		for (i = 0; i < ARRAY_SIZE(devtable_switch); i++, p++) {
-			if (sym_is(name, namelen, p->device_id)) {
-				do_table(symval, sym->st_size, p->id_size,
-					 p->device_id, p->function, mod);
+		for (p = __start___devtable; p < __stop___devtable; p++) {
+			if (sym_is(name, namelen, (*p)->device_id)) {
+				do_table(symval, sym->st_size, (*p)->id_size,
+					 (*p)->device_id, (*p)->function, mod);
 				break;
 			}
 		}

Alessandro Rubini

author

2011-12-02 08:42 UTC #5 lore raw

Quoted 2 lines

But splitting 15-line functions into separate files? Less compact, less simple.

Personally, I'm more interested in the addition of new busses (as drop-in files) than the split up of current ones. But I see your point.

Quoted 6 lines

Quoted 6 lines
Quoted 6 lines
step 1: create the ELF section so ENTRY() lines can leave the array
(and change name accordingly).
step 2: each ENTRY() line can be moved just after the associated code
(this means that a new bus is just a single hunk, not 2 of them)

Quoted 6 lines

Quoted 3 lines
Quoted 3 lines
step 3: I create the headers needed to move code and ENTRY in separate
files. This is some movement around, not trivial so it may deserve a patch in itself.
If you *really* want to use separate files, then this patch will let you do it. You'll need to expose some stuff in a header though.

Fine with me. This is 1 and 2 of my list above. And the "need to expose" is my step 3. So I agree with this.

thanks
/alessandro

Greg KH

2011-12-02 19:31 UTC #6 lore raw

Quoted 5 lines · Alessandro Rubini

Quoted 2 lines
But splitting 15-line functions into separate files? Less compact, less simple.
Personally, I'm more interested in the addition of new busses (as drop-in files) than the split up of current ones. But I see your point.

Yes, that's what me and Linus have asked about having in the past, making things easier to merge and the like. Now admittedly, this file isn't all that hard to merge these days given that the addition of new busses is pretty rare, these complaints might have been from before git times, when merges were harder than they are now.

Anyway, I like your patch as well, as it seems to get us to that goal.

thanks,

greg k-h

Rusty Russell

2011-12-04 10:50 UTC #7 lore raw

Quoted 7 lines · Greg KH

Quoted 5 lines · Alessandro Rubini
Quoted 2 lines
But splitting 15-line functions into separate files? Less compact, less simple.
Personally, I'm more interested in the addition of new busses (as drop-in files) than the split up of current ones. But I see your point.

Quoted 7 lines

Yes, that's what me and Linus have asked about having in the past, making things easier to merge and the like. Now admittedly, this file isn't all that hard to merge these days given that the addition of new busses is pretty rare, these complaints might have been from before git times, when merges were harder than they are now.
Anyway, I like your patch as well, as it seems to get us to that goal.

But it doesn't :( Trivial conflict in file2alias.c becomes trivial conflict in Makefile.

We could use $(wildcard) in the Makefile, but that's not used much in the kernel so it'd seem a bit weird.

So honestly, I think we should reorder file2alias.c into alphabetical order. In practice, that'll prevent conflicts.

As I said, I'm happy for Alessandro to do the separate file thing *too*, but if that was the only point, there wasn't one.

Cheers,
Rusty.