On Thu, Feb 26, 2015 at 11:47:49AM -0500, Benjamin Romer wrote:
> diff --git a/drivers/staging/unisys/include/timskmod.h
> b/drivers/staging/unisys/include/timskmod.h
> index 7ad65cc..c23cde5 100644
> --- a/drivers/staging/unisys/include/timskmod.h
> +++ b/drivers/staging/unisys/include/timskmod.h
> @@ -68,8 +68,7 @@
> */
> #define ASSERT(cond) \
> do { if (!(cond)) \
> - HUHDRV("ASSERT failed - %s", \
> - __stringify(cond)); \
> + return cond;\
> } while (0)
>
Ugh... Don't do this.
There is only one place which calls this ASSERT() macro. It should
probably be changed to:
WARN_ON(!pw->is_scheduled);
or something.
> - if (debug_buf == NULL) {
> - LOGERR("failed to allocate buffer to provide proc
> data.\n");
> - return -ENOMEM;
> - }
> + if (debug_buf == NULL)
> + return -ENOMEM;
Double tab.
> @@ -1063,19 +1010,7 @@ do_scsi_linuxstat(struct uiscmdrsp *cmdrsp, struct
> scsi_cmnd *scsicmd)
>
> if (atomic_read(&vdisk->error_count) < VIRTHBA_ERROR_COUNT) {
> atomic_inc(&vdisk->error_count);
> - LOGERR("SCSICMD ****FAILED scsicmd:0x%p op:0x%x
> <%d:%d:%d:%llu> 0x%x-0x%x-0x%x-0x%x-0x%x.\n",
> - scsicmd, cmdrsp->scsi.cmnd[0],
> - scsidev->host->host_no, scsidev->id,
> - scsidev->channel, scsidev->lun,
> - cmdrsp->scsi.linuxstat, sd->valid, sd->sense_key,
> - sd->additional_sense_code,
> - sd->additional_sense_code_qualifier);
> - if (atomic_read(&vdisk->error_count) ==
> - VIRTHBA_ERROR_COUNT) {
> - LOGERR("Throtling SCSICMD errors disk
> <%d:%d:%d:%llu>\n",
> - scsidev->host->host_no, scsidev->id,
> - scsidev->channel, scsidev->lun);
> - }
> + atomic_read(&vdisk->error_count);
Is this really necessary?
> atomic_set(&vdisk->ios_threshold, IOS_ERROR_THRESHOLD);
> }
> }
> @@ -206,15 +204,12 @@ static int write_vbus_bus_info(struct
> spar_vbus_channel_protocol *chan,
> {
> int off;
>
> - if (!chan) {
> - LOGERR("vbus channel not present");
> + if (!chan)
> return -1;
> - }
> +
> off = sizeof(struct channel_header) + chan->hdr_info.bus_info_offset;
> - if (chan->hdr_info.bus_info_offset == 0) {
> - LOGERR("vbus channel not used, because bus_info_offset == 0");
> - return -1;
> - }
> + if (chan->hdr_info.bus_info_offset == 0)
> + return -1;
Double tab.
> memcpy(((u8 *)(chan)) + off, info, sizeof(*info));
> return 0;
> }
Gotta run... Enough reviewing for tonight.
regards,
dan carpenter
_______________________________________________
devel mailing list
[email protected]
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
