Re: [PATCH] Added devfs support for i386 msr/cpuid driver

Richard Gooch (rgooch@ras.ucalgary.ca)
Mon, 3 Sep 2001 13:57:38 -0600


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/