All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rusty Russell <rusty@rustcorp.com.au>
To: linux-kernel@vger.kernel.org, virtualization@lists.linux-foundation.org
Cc: Peter Maydell <peter.maydell@linaro.org>,
	Sasha Levin <levinsasha928@gmail.com>,
	Pawel Moll <pawel.moll@arm.com>
Subject: Re: [PATCH] virtio-mmio: Devices parameter parsing
Date: Wed, 16 Nov 2011 11:12:05 +1030	[thread overview]
Message-ID: <87sjlooq3m.fsf@rustcorp.com.au> (raw)
In-Reply-To: <1321365185-2928-1-git-send-email-pawel.moll@arm.com>

On Tue, 15 Nov 2011 13:53:05 +0000, Pawel Moll <pawel.moll@arm.com> wrote:
> +static char *virtio_mmio_cmdline_devices;
> +module_param_named(devices, virtio_mmio_cmdline_devices, charp, 0);

This is the wrong way to do this.

Don't put things in a charp and process it later.  It's lazy.  You
should write parsers for it and call it straight from module_param.

And if you do it that way, multiple devices are simply multiple
arguments.

Like so:

static int virtio_mmio_set(const char *val, const struct kernel_param *kp)
{
        if (!virtio_mmio_cmdline_parent_initialized) {
        	err = device_register(&virtio_mmio_cmdline_parent);
        	if (err)
        		return err;
                virtio_mmio_cmdline_parent_initialized = true;
        }
        
        ... parse here, return -errno on fail, 0 on success.
}

static struct kernel_param_ops param_ops_virtio_mmio = {
        .set = virtio_mmio_set,
        .get = virtio_mmio_get,
};

module_param_cb(device, &param_ops_virtio_mmio, NULL, 0400);

Initialization and error handling is now done for you...

Cheers,
Rusty.

WARNING: multiple messages have this Message-ID (diff)
From: Rusty Russell <rusty@rustcorp.com.au>
To: Pawel Moll <pawel.moll@arm.com>,
	linux-kernel@vger.kernel.org,
	virtualization@lists.linux-foundation.org
Cc: Sasha Levin <levinsasha928@gmail.com>,
	Peter Maydell <peter.maydell@linaro.org>,
	Pawel Moll <pawel.moll@arm.com>
Subject: Re: [PATCH] virtio-mmio: Devices parameter parsing
Date: Wed, 16 Nov 2011 11:12:05 +1030	[thread overview]
Message-ID: <87sjlooq3m.fsf@rustcorp.com.au> (raw)
In-Reply-To: <1321365185-2928-1-git-send-email-pawel.moll@arm.com>

On Tue, 15 Nov 2011 13:53:05 +0000, Pawel Moll <pawel.moll@arm.com> wrote:
> +static char *virtio_mmio_cmdline_devices;
> +module_param_named(devices, virtio_mmio_cmdline_devices, charp, 0);

This is the wrong way to do this.

Don't put things in a charp and process it later.  It's lazy.  You
should write parsers for it and call it straight from module_param.

And if you do it that way, multiple devices are simply multiple
arguments.

Like so:

static int virtio_mmio_set(const char *val, const struct kernel_param *kp)
{
        if (!virtio_mmio_cmdline_parent_initialized) {
        	err = device_register(&virtio_mmio_cmdline_parent);
        	if (err)
        		return err;
                virtio_mmio_cmdline_parent_initialized = true;
        }
        
        ... parse here, return -errno on fail, 0 on success.
}

static struct kernel_param_ops param_ops_virtio_mmio = {
        .set = virtio_mmio_set,
        .get = virtio_mmio_get,
};

module_param_cb(device, &param_ops_virtio_mmio, NULL, 0400);

Initialization and error handling is now done for you...

Cheers,
Rusty.

  reply	other threads:[~2011-11-16  0:42 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-11-15 13:53 [PATCH] virtio-mmio: Devices parameter parsing Pawel Moll
2011-11-16  0:42 ` Rusty Russell [this message]
2011-11-16  0:42   ` Rusty Russell
2011-11-16 18:13   ` Pawel Moll
2011-11-16 18:13     ` Pawel Moll
2011-11-17 12:42     ` [PATCH v2] " Pawel Moll
2011-11-17 12:42       ` Pawel Moll
2011-11-21  3:32     ` [PATCH] " Rusty Russell
2011-11-21  3:32     ` Rusty Russell
2011-11-21 14:44       ` Pawel Moll
2011-11-21 14:44         ` Pawel Moll
2011-11-21 17:56         ` Pawel Moll
2011-11-21 17:56           ` Pawel Moll
2011-11-22  0:53           ` Rusty Russell
2011-11-22  0:53             ` Rusty Russell
2011-11-23 18:08             ` Pawel Moll
2011-11-28  0:31               ` Rusty Russell
2011-11-28  0:31                 ` Rusty Russell
2011-11-29 17:36                 ` Pawel Moll
2011-11-29 17:36                   ` Pawel Moll
2011-12-01  2:06                   ` Rusty Russell
2011-12-01  2:06                     ` Rusty Russell
2011-12-12 17:53                     ` Pawel Moll
2011-12-12 17:53                       ` Pawel Moll
2011-12-12 17:57                       ` [PATCH 1/2] params: <level>_initcall-like kernel parameters Pawel Moll
2011-12-12 17:57                         ` Pawel Moll
2011-12-12 17:57                         ` [PATCH 2/2] virtio-mmio: Devices parameter parsing Pawel Moll
2011-12-12 17:57                           ` Pawel Moll
2012-04-09 16:32                           ` Sasha Levin
2012-04-09 16:32                             ` Sasha Levin
2012-04-10 12:53                             ` Pawel Moll
2012-04-10 12:53                               ` Pawel Moll
2011-12-15  3:51                         ` [PATCH 1/2] params: <level>_initcall-like kernel parameters Rusty Russell
2011-12-15  3:51                           ` Rusty Russell
2011-12-15  9:38                           ` Pawel Moll
2011-12-15  9:38                             ` Pawel Moll
2011-11-22  0:44         ` [PATCH] virtio-mmio: Devices parameter parsing Rusty Russell
2011-11-22  0:44           ` Rusty Russell
  -- strict thread matches above, loose matches on Subject: below --
2011-11-15 13:53 Pawel Moll
2012-05-09 17:30 Pawel Moll
2012-05-09 17:30 ` Pawel Moll
2012-05-10  0:44 ` Rusty Russell
2012-05-10  0:44   ` Rusty Russell

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=87sjlooq3m.fsf@rustcorp.com.au \
    --to=rusty@rustcorp.com.au \
    --cc=levinsasha928@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pawel.moll@arm.com \
    --cc=peter.maydell@linaro.org \
    --cc=virtualization@lists.linux-foundation.org \
    /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.