link time error in drivers/mtd (240t13p2)

Hi.

Various files in drivers/mtd references cfi_probe (by way of do_cfi_probe). This function is static and thus not shared. The following patch removes the static declaration but if it is What Was Intended I do not know. It makes the kernel link, however.

@@ -17,7 +17,7 @@
 #include <linux/mtd/cfi.h>
 
 
-static struct mtd_info *cfi_probe(struct map_info *);
+struct mtd_info *cfi_probe(struct map_info *);
 
 static void print_cfi_ident(struct cfi_ident *);
 static void check_cmd_set(struct map_info *, int, unsigned long);

@@ -32,7 +32,7 @@
  * this module is non-zero, i.e. between inter_module_get and
  * inter_module_put.  Keith Owens <[email protected]> 29 Oct 2000.
  */
-static struct mtd_info *cfi_probe(struct map_info *map)
+struct mtd_info *cfi_probe(struct map_info *map)
 {
 	struct mtd_info *mtd = NULL;
 	struct cfi_private *cfi;

        Rasmus([email protected])

The Holocaust was an obscene period in our nation's history. I mean
in this century's history. But we all lived in this century. I didn't
live in this century.
                -- Senator Dan Quayle, 9/15/88
                   (reported in Esquire, 8/92, The New Yorker, 10/10/88, p.102)
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

Somebody changed include/linux/mtd/map.h between 2.4.0-test11 and test12. That change is wrong, it adds conditional complexity where it is not required - inter_module_xxx works even without CONFIG_MODULES. cfi_probe should still be static.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

No. Think about the link order. inter_module_xxx doesn't work reliably. get_module_symbol() did.

dwmw2


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

Messing about with conditional compilation because the link order is incorrect is the wrong fix. The mtd/Makefile must link the objects in the correct order.

cfi_probe.o needs to come after cfi_cmdset_000?.o. doc_probe.o needs to come after doc200?.o.
nora.o, octagon-5066.o, physmap.o, rpxlite.o, vmax301.o, pnc2000.o need to come after cfi_probe.o.
octagon-5066.o, vmax301.o need to come after jedec.o. octagon-5066.o, vmax301.o need to come after map_ram.o. octagon-5066.o, vmax301.o need to come after map_rom.o.

2.4.0-test13-pre2 almost does that, the only obvious problem is that cfi_probe appears before cfi_cmdset. Move cfi_probe to link after cfi_cmdset, do you still get link order problems with the 2.4.0-test11 version of include/linux/mtd.h?

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

The conditional compilation is far more obvious to people than subtle issues with link order. So I prefer to avoid the latter at all costs.

I have to have some conditional compilation in my tree to allow it to compile under 2.0 uClinux. Admittedly that doesn't have to get into 2.4, but I obviously prefer the code in 2.4 to be as close to my working copy as possible.

I'll poke at it and try to come up with a cleaner solution. It may be that I can shift all the conditional stuff off into the compatmac.h and leave the 'real' code path in a cleaner state than the current one.

I haven't had problems. But the possibility exists.

dwmw2


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

The rest of the kernel already depends totally on these "subtle" issues with link order. Why should mtd be different?

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

Because I maintain the MTD code and I want it to be. I think the link order dependencies are ugly, unnecessary and far more likely to be problematic then the alternatives. I'll code an alternative which is cleaner than the current code, and Linus can either accept it or not, as he sees fit.

dwmw2


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

Your choice. Just bear in mind that if CONFIG_MODULES=y but mtd objects are built into the kernel then mtd _must_ have a correct link order. Consider a config with CONFIG_MODULES=y but every mtd option is set to 'y', link order is critical. The moment you have two or more mtd modules built in then you are stuck with link order issues, no matter what you do. Of course you could invent and maintain your own unique method for controlling mtd initialisation order ...

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

Yep, I'd just noticed that one. The patch was actually put in by someone to fix 2.0 compilation - and I noticed that it made the link order problem go away for certain configs.

I'll try to find a clean way to make the MTD code work in all configurations without having to do that. If it really comes to doing the above, I'll probably just give up and let it stay 'broken' (IMO) along with the rest of the kernel code which as you say already has link order dependencies.

dwmw2


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

On Sunday 17 December 2000 12:04, David Woodhouse wrote to linux-kernel:

[cc'ed to mtd list as it might help a few people there]
heh.
I'm glad this has come up as I think I got bitten by this a couple of weeks
ago, but didn't realise it at the time. For various reasons to do with avoiding using initrd's I'd built a kernel that was modular, but had doc2000 and nftl loaded statically - other mtd stuff was loaded as modules. My Millenium chip behaved like the proverbial stick - data wouldn't stick to it, certainly not across reboots. Then I came in one morning to find the screen slowly scolling the following message:
NFTL - writeblock(): Cannot find block to write to
end_request: I/O error, dev 5d:01 (unknown), sector 256
Tell Dave he fucked up. LastFreeEUN = 1023, FirstEUN=10765
No Virtual Unit Chains available for folding. Failing request. [This will teach me not to run mke2fs from 5 miles away!]

I guess this must have been caused by incorrect ordering of the modules, as a recompilation of the kernel and linking everything as modular has left it working fine [This was using 2.2.17 + the merged doc2000 w/Millenium support from the 30/11 snapshot and from cvs since, although I've now jumped to 2.4.0-test12 without any problems]

If anyone else has had this problem I would suggest this as a solution

cheers
john

To unsubscribe, send "unsubscribe mtd" to [email protected]

That probably wasn't an init order problem. That was just broken code. It should now be fixed, and the printk has been sanitised too - it wasn't really supposed to get past the proof of concept stage with that precise wording in it.

Regardless of the wording used, that error shouldn't ever happen in the new code, if your NFTL has been reformatted since you broke it. If it does, we need to know about it.

--
dwmw2

To unsubscribe, send "unsubscribe mtd" to [email protected]

Why should mtd be broken just because the rest of the Makefiles are

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

Keith Owens <[email protected]> said:

[...]

Would tsort(1) perhaps help?

Horst von Brand                             [email protected]
Casilla 9G, Vin~a del Mar, Chile                               +56 32 672616
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

[Horst von Brand]

I'm betting Linus would never go for using tsort to resolve such issues -- unless tsort output is guaranteed to be stable (the docs for GNU textutils don't say). This would be for the same reason that he rejected the partial ordering in the LINK_FIRST patch -- because it was only partial ordering and he thinks total ordering is necessary. For me, BTW, that's still an article of faith -- I still do not see why total ordering *is* necessary, but <shrug> thus saith the penguin.

Peter
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

It is intended, thanks. Not only does it make the inter_module_xxx case work reliably, but it also allows you to compile the code at all under 2.0 uCLinux. The reason it was omitted from test12 is because there are a handful of other changes to the CFI code which I haven't yet tested as thoroughly as I want to. If you're using CFI flash, please could you test the latest version from my CVS tree and let me know the results?

dwmw2


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/