From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Luca Clementi <luca.clementi@gmail.com>
Cc: linux-kernel@vger.kernel.org, Brian Swetland <swetland@google.com>
Subject: Re: [PATCH] Staging: Android: logger: module_exit implementationg
Date: Fri, 2 Nov 2012 11:29:26 -0700 [thread overview]
Message-ID: <20121102182926.GA18824@kroah.com> (raw)
In-Reply-To: <1351836952-3389-1-git-send-email-luca.clementi@gmail.com>
On Thu, Nov 01, 2012 at 11:15:52PM -0700, Luca Clementi wrote:
> Created the module_exit for the android logger so that
> it can be loaded and unloaded as a module. Fixed
> module_init and some other minor issues.
That's doing more than one thing here at once, care to break it up?
Yeah, I know it seems funny for such a small patch, but it helps.
Also, now that you've added this, the logger driver still can't be built
as a module, as the build system isn't changed to let that happen,
right?
Also, why do you want to build this as a module?
> Signed-off-by: Luca Clementi <luca.clementi@gmail.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Brian Swetland <swetland@google.com>
> ---
> drivers/staging/android/logger.c | 30 +++++++++++++++++++++++++++++-
> 1 file changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/android/logger.c b/drivers/staging/android/logger.c
> index 1d5ed47..050be01 100644
> --- a/drivers/staging/android/logger.c
> +++ b/drivers/staging/android/logger.c
> @@ -676,4 +676,32 @@ static int __init logger_init(void)
> out:
> return ret;
> }
> -device_initcall(logger_init);
> +
> +static void __exit logger_exit(void)
> +{
> + struct logger_log *current_log, *next_log;
> +
> + list_for_each_entry_safe(current_log, next_log, &log_list, logs) {
> + /* we have to delete all the entry inside log_list */
> + ret = misc_deregister(¤t_log->misc);
> + if (unlikely(ret)) {
> + pr_err("failed to deregister misc device for log '%s'!\n",
> + current_log->misc.name);
> + }
> + pr_info("removed loggger '%s'\n", current_log->misc.name);
Is that message really needed?
> + vfree(current_log->buffer);
> + kfree(current_log->misc.name);
> + kfree(current_log);
> + }
> +
> + return;
> +}
> +
> +
> +module_init(logger_init);
Is module_init() the same "level" as device_initcall()? Did you test
this out in an Android system?
> +module_exit(logger_exit);
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Brian Swetland, <swetland@google.com>");
> +MODULE_DESCRIPTION("Android Logger");
> +
> +
What's with the unneeded trailing empty lines?
thanks,
greg k-h
next prev parent reply other threads:[~2012-11-02 18:29 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-11-02 6:15 [PATCH] Staging: Android: logger: module_exit implementation Luca Clementi
2012-11-02 18:29 ` Greg Kroah-Hartman [this message]
2012-11-03 5:40 ` [PATCH] Staging: Android: logger: module_exit implementationg Brian Swetland
2012-11-03 17:45 ` Luca Clementi
2012-11-05 0:03 ` Ryan Mallon
2012-11-04 23:57 ` [PATCH] Staging: Android: logger: module_exit implementation Ryan Mallon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20121102182926.GA18824@kroah.com \
--to=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=luca.clementi@gmail.com \
--cc=swetland@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.