Messages in this thread | | | Date | Mon, 3 Sep 2001 13:57:38 -0600 | From | Richard Gooch <> | Subject | Re: [PATCH] Added devfs support for i386 msr/cpuid driver |
| |
Per Niva writes: > On Mon, 27 Aug 2001, Richard Gooch wrote: > > The reason it's wrong is because he put #ifdef's in there. The > > functions should just be called unconditionally. The #ifdef's are in > > the header. > > I actually pondered a while on this, and settled on > the cut'n'paste-from-mtrr.c version. There is no error > check there, and I just overlooked it. > > The defence for the #ifdefs is that I didn't see > register_chrdev() being aware of devfs, and I thought > we'd be better off just not calling register_chrdev() > at all if we have devfs.
No, even if CONFIG_DEVFS_FS=y, doesn't mean the user wants to mount/use devfs, so you shouldn't disable register_chrdev().
> It's not like I personally like #ifdefs, but it seemed > justified to my inexperienced eyes at that point. And > there's a #ifdef CONFIG_DEVFS_FS around the call in > mtrr.c too, and I thought it safe to do like what's > already in the official tree.
The #ifdef in mtrr.c is there for historical reasons (at one point, neither the mtrr or devfs patches were in the kernel, but I wanted mtrr to use devfs if available, hence the #ifdef). The #ifdef can be safely taken out (and should be).
> In microcode.c however, is the new-style without #ifdef > (or rather with the #ifdef in the headers instead) > and with error checking, but microcode_init() doesn't > use register_chrdev() anyway, even if devfs is not > supported. > > Please enlighten me!
The microcode patch went in after devfs was in the kernel, IIRC, and thus could unconditionally reference devfs_register().
Regards,
Richard.... Permanent: rgooch@atnf.csiro.au Current: rgooch@ras.ucalgary.ca - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |