On Wednesday 28 January 2009 18:57:49 Adrian McMenamin wrote:
> --- a/drivers/sh/maple/maple.c
> +++ b/drivers/sh/maple/maple.c
> -int maple_add_packet_sleeps(struct maple_device *mdev, u32 function,
> - u32 command, size_t length, void *data)
> -{
> - int locking, ret = 0;
> - void *sendbuf = NULL;
> -
> - locking = mutex_lock_interruptible(&mdev->mq->mutex);
> - if (locking) {
> - ret = -EIO;
> - goto out;
> - }
> + int ret = 0;
> + void *sendbuf;
>
> + sendbuf = kzalloc(length * 4, GFP_KERNEL);
> if (length) {
> - sendbuf = kmalloc(length * 4, GFP_KERNEL);
that looks wonky ... you do the alloc first and then check the len ?
> failed_nomem:
> + printk(KERN_INFO "maple: could not allocate memory\n");
should be a KERN_NOTICE or worse ... KERN_INFO doesnt seem appropriate
> + if (device_register(&mdev->dev)) {
> printk(KERN_INFO
> - "Maple bus: Attempt to register device"
> - " (%x, %x) failed.\n",
> + "Maple bus: Attempt to register device"
> + " (%x, %x) failed.\n",
same here ... KERN_INFO shouldnt be used in error handlers
> +static void maple_response_fileerr(struct maple_device *mdev, void
> *recvbuf) +{
> + if (mdev->fileerrhandler) {
> + mdev->fileerrhandler(mdev, recvbuf);
> + return;
> + } else
> + printk(KERN_INFO "maple: device (%d, %d) reports file error %d\n",
> + mdev->port, mdev->unit, ((int *)recvbuf)[1]);
> +}
and here ...
> case MAPLE_RESPONSE_AGAIN:
> case MAPLE_RESPONSE_BADCMD:
> case MAPLE_RESPONSE_BADFUNC:
> - printk(KERN_DEBUG
> + printk(KERN_INFO
> "Maple non-fatal error 0x%X\n",
> code);
and here ...
> /* Maple Bus command and response codes */
> enum maple_code {
> + MAPLE_RESPONSE_FILEERR = -5,
> + MAPLE_RESPONSE_AGAIN = -4, /* retransmit */
do you really need to set every value ? i would only bother when there's a
discontinuity ...
-mike
Attachment:
signature.asc
Description: This is a digitally signed message part.
