Re: [PATCH] allow gpiolib to be a module

[Linus Walleij]

On Mon, Sep 10, 2012 at 2:16 PM, Jan Beulich <JBeulich@xxxxxxxx> wrote:

> Without ARCH_REQUIRE_GPIOLIB there's no reason to force this code, when
> enabled, to always be built into the kernel, which requires only minor
> Makefile and source code adjustments.
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

I *really* want Grant's opinion on this patch before merging
it...

> +++ 3.6-rc5-gpiolib-module/drivers/gpio/gpiolib.c
> @@ -1003,7 +1003,9 @@ static int __init gpiolib_sysfs_init(voi
>
>         return status;
>  }
> +#ifndef MODULE
>  postcore_initcall(gpiolib_sysfs_init);
> +#endif

#ifdefs are ugly but allright I don't know if there are any better
ways to do this.

(...)

> +#ifndef MODULE
>  subsys_initcall(gpiolib_debugfs_init);
> +#endif
>
>  #endif /* DEBUG_FS */
> +
> +#ifdef MODULE
> +int __init gpiolib_init(void)
> +{
> +       return gpiolib_sysfs_init() ?: gpiolib_debugfs_init();

I can't parse this, sorry the gpio subsystem maintainer is too bad coder.
What about something more readable?

int ret;

ret = gpiolib_sysfs_init();
if (ret)
   return ret;
return gpiolib_debugfs_init();

I know it doesn't look as cool but it's easier for me to understand.

There is a bug too: I don't think this compiles if you compile as
a module but disable debugfs. Try it out.

> +}
> +module_init(gpiolib_init);
> +
> +MODULE_DESCRIPTION("GPIO library");
> +MODULE_LICENSE("GPL");

These two things don't really need to be inside an #ifdef
right?

> +#endif

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/