On Wed, 2011-08-10 at 17:31 -0600, Kenneth Heitke wrote:
> From: Sagar Dharia <sdharia@xxxxxxxxxxxxxx>
Just trivia:
> create mode 100755 Documentation/slimbus/slimbus-framework.txt
644?
> +++ b/drivers/slimbus/slimbus.c
> @@ -0,0 +1,2629 @@
[]
> +static int slim_register_controller(struct slim_controller *ctrl)
> + if (ctrl->nports) {
> + ctrl->ports = kzalloc(ctrl->nports * sizeof(struct slim_port),
> + GFP_KERNEL);
Many of these kzalloc with multiplies could be kcalloc.
> + ctrl->chans = kzalloc(ctrl->nchans * sizeof(struct slim_ich),
> + GFP_KERNEL);
[]
> +void slim_msg_response(struct slim_controller *ctrl, u8 *reply, u8 tid, u8 len)
> +{
[]
> + dev_err(&ctrl->dev, "Got response to invalid TID:%d, len:%d",
> + tid, len);
Many of the dev_<level> uses are missing terminating "\n".
It might be reasonable to have slim specific macros/functions
for message logging. That might allow slim specific prefixes.
For functions, you could look at netdev_<level>:
slim_<level>(struct slim_controller *ctrl, const char *fmt, ...)
or macros like:
#define slim_printk(level, ctrl, fmt, ...) \
dev_printk(level, &(ctrl)->dev, fmt, ##__VA_ARGS__)
#define slim_<level>(level, ctrl, fmt, ...) \
slim_printk(KERN_<LEVEL>, ctrl, fmt, ##__VA_ARGS__)
etc.
[]
> +static int slim_processtxn(struct slim_controller *ctrl, u8 dt, u8 mc, u16 ec,
> + u8 mt, u8 *rbuf, const u8 *wbuf, u8 len, u8 mlen,
> + struct completion *comp, u8 la, u8 *tid)
> +{
[]
> + ctrl->txnt = krealloc(ctrl->txnt,
> + (i + 1) * sizeof(struct slim_msg_txn *),
> + GFP_KERNEL);
realloc's to the same var are generally unwise.
Is anything that was pointed to important?
If so, and the return was NULL, you just lost
the important stuff.
[]
> +int slim_assign_laddr(struct slim_controller *ctrl, const u8 *e_addr,
> + u8 e_len, u8 *laddr)
[]
> + ctrl->addrt = krealloc(ctrl->addrt,
> + (ctrl->num_dev + 1) *
> + sizeof(struct slim_addrt),
> + GFP_KERNEL);
Same potential realloc issue here too.
[]
> +static u16 slim_slicecodefromsize(u32 req)
> +{
> + u8 codetosize[8] = {1, 2, 3, 4, 6, 8, 12, 16};
static const
> + if (req >= 8)
> + return 0;
> + else
> + return codetosize[req];
Should this be u8 or u16? It's somewhat odd to
have the array a different size than the return.
> +static u16 slim_slicesize(u32 code)
> +{
> + u8 sizetocode[16] = {0, 1, 2, 3, 3, 4, 4, 5, 5, 5, 5, 6, 6, 6, 6, 7};
static const
> + if (code == 0)
> + code = 1;
> + if (code > 16)
> + code = 16;
clamp
> + return sizetocode[code - 1];
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
