Re: [PATCH 01/16] ehca: module infrastructure
From: Jörn Engel
Date: Thu May 18 2006 - 13:40:48 EST
On Mon, 15 May 2006 19:41:13 +0200, Heiko J Schick wrote:
> + * This source code is distributed under a dual license of GPL v2.0 and
> OpenIB
Your mailer is still mangling long lines, it seems. If you need a
quick solution, I could offer you a gmail invite.
> +
> + EDEB_EX(7, "ret=%x", ret);
> +
> + return ret;
> +
> +create_aqp1:
> + ib_destroy_cq(sport->ibcq_aqp1);
> +
> + EDEB_EX(7, "ret=%x", ret);
> +
> + return ret;
> +}
Those two cases could be combined with a goto. Saves a tiny amount of
rodata.
> +#define EHCA_RESOURCE_ATTR(name)
> \
> +static ssize_t ehca_show_##name(struct device *dev,
> \
You have spaces instead of tabs in the lines the mailer mangled.
> +
> \
> + data = rblock->name; \
> + kfree(rblock); \
> + \
> + if ((strcmp(#name, "num_ports") == 0) && (ehca_nr_ports == 1)) \
> + return snprintf(buf, 256, "1\n"); \
> + else \
> + return snprintf(buf, 256, "%d\n", data); \
> + \
Is rblock->num_ports uninitialized when (ehca_nr_ports == 1)? Looks
rather odd.
> + shca = (struct ehca_shca *)ib_alloc_device(sizeof(*shca));
A quick grep showed that every single return value of
ib_alloc_device() has a cast.
Roland, can't you just change ib_alloc_device() to return void*?
> +static struct of_device_id ehca_device_table[] =
> +{
> + {
> + .name = "lhca",
> + .compatible = "IBM,lhca",
> + },
> + {},
> +};
Is the extra element needed?
> + if ((ret = ehca_create_slab_caches(&ehca_module))) {
> + EDEB_ERR(4, "Cannot create SLAB caches");
> + ret = -ENOMEM;
> + goto module_init1;
> + }
ret = try_something()
if (ret) {
...
}
> + ehca_module.timer.data = (unsigned long)(void*)&ehca_module;
Why the double cast?
Jörn
--
Fancy algorithms are slow when n is small, and n is usually small.
Fancy algorithms have big constants. Until you know that n is
frequently going to be big, don't get fancy.
-- Rob Pike
-
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/