Re: [PATCH 06/11] ide: make ide_do_drive_cmd return rq->errors

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Tue, Apr 22, 2008 at 09:26:37AM +0900, FUJITA Tomonori wrote:
> ide_do_drive_cmd forges an error value (-EIO) instead of
> rq->errors. idetape_queue_rw_tail wants rq->errors so this patch makes
> ide_do_drive_cmd return rq->errors.
> 
> For compatibility, this patch makes the users of ide_do_drive_cmd
> return -EIO instead of a return value of ide_do_drive_cmd
> (rq->errors).

i don't see the reason for it. In the end, the only error type that is being
handed to and fro is -EIO and nobody is interested in rq->errors. So, whether
ide_do_drive_cmd or its callers return -EIO is kinda unimportant. In the second
case, however, you have simply added more code (as per the diffstat below) with
no apparent functionality. It would only make sense, IMHO, if you would
differentiate behaviour based on rq->errors...

> Signed-off-by: FUJITA Tomonori <fujita.tomonori@xxxxxxxxxxxxx>
> Cc: Bartlomiej Zolnierkiewicz <bzolnier@xxxxxxxxx>
> ---
>  drivers/ide/ide-cd_ioctl.c |    3 ++-
>  drivers/ide/ide-floppy.c   |    4 +++-
>  drivers/ide/ide-io.c       |    2 +-
>  drivers/ide/ide-tape.c     |    5 ++++-
>  drivers/ide/ide-taskfile.c |    7 +++++--
>  drivers/ide/ide.c          |    2 ++
>  6 files changed, 17 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/ide/ide-cd_ioctl.c b/drivers/ide/ide-cd_ioctl.c
> index 9c044da..57dda1f 100644
> --- a/drivers/ide/ide-cd_ioctl.c
> +++ b/drivers/ide/ide-cd_ioctl.c
> @@ -302,7 +302,8 @@ int ide_cdrom_reset(struct cdrom_device_info *cdi)
>  	req.cmd_type = REQ_TYPE_SPECIAL;
>  	req.cmd_flags = REQ_QUIET;
>  	ret = ide_do_drive_cmd(drive, &req, ide_wait);
> -
> +	if (ret)
> +		ret = -EIO;
>  	/*
>  	 * A reset will unlock the door. If it was previously locked,
>  	 * lock it again.
> diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
> index d75b2aa..af6625a 100644
> --- a/drivers/ide/ide-floppy.c
> +++ b/drivers/ide/ide-floppy.c
> @@ -886,13 +886,15 @@ static int idefloppy_queue_pc_tail(ide_drive_t *drive, struct ide_atapi_pc *pc)
>  {
>  	struct ide_floppy_obj *floppy = drive->driver_data;
>  	struct request *rq;
> +	int ret;
>  
>  	rq = blk_get_request(drive->queue, READ, __GFP_WAIT);
>  	rq->buffer = (char *) pc;
>  	rq->cmd_type = REQ_TYPE_SPECIAL;
>  	rq->rq_disk = floppy->disk;
>  
> -	return ide_do_drive_cmd(drive, rq, ide_wait);
> +	ret = ide_do_drive_cmd(drive, rq, ide_wait);
> +	return ret ? -EIO : 0;
>  }
>  
>  /*
> diff --git a/drivers/ide/ide-io.c b/drivers/ide/ide-io.c
> index 31e5afa..ce09f70 100644
> --- a/drivers/ide/ide-io.c
> +++ b/drivers/ide/ide-io.c
> @@ -1667,7 +1667,7 @@ int ide_do_drive_cmd (ide_drive_t *drive, struct request *rq, ide_action_t actio
>  	if (must_wait) {
>  		wait_for_completion(&wait);
>  		if (rq->errors)
> -			err = -EIO;
> +			err = rq->errors;
>  
>  		blk_put_request(rq);
>  	}
> diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
> index f43fd07..02851b3 100644
> --- a/drivers/ide/ide-tape.c
> +++ b/drivers/ide/ide-tape.c
> @@ -1911,11 +1911,14 @@ static int __idetape_queue_pc_tail(ide_drive_t *drive, struct ide_atapi_pc *pc)
>  {
>  	struct ide_tape_obj *tape = drive->driver_data;
>  	struct request rq;
> +	int ret;
>  
>  	idetape_init_rq(&rq, REQ_IDETAPE_PC1);
>  	rq.buffer = (char *) pc;
>  	rq.rq_disk = tape->disk;
> -	return ide_do_drive_cmd(drive, &rq, ide_wait);
> +	ret = ide_do_drive_cmd(drive, &rq, ide_wait);
> +	return ret ? -EIO : 0;
> +
>  }
>  
>  static void idetape_create_load_unload_cmd(ide_drive_t *drive,
> diff --git a/drivers/ide/ide-taskfile.c b/drivers/ide/ide-taskfile.c
> index fac7fad..6d433c4 100644
> --- a/drivers/ide/ide-taskfile.c
> +++ b/drivers/ide/ide-taskfile.c
> @@ -531,6 +531,7 @@ static ide_startstop_t pre_task_out_intr(ide_drive_t *drive, struct request *rq)
>  int ide_raw_taskfile(ide_drive_t *drive, ide_task_t *task, u8 *buf, u16 nsect)
>  {
>  	struct request *rq;
> +	int ret;
>  
>  	rq = blk_get_request(drive->queue, READ, __GFP_WAIT);
>  	rq->cmd_type = REQ_TYPE_ATA_TASKFILE;
> @@ -551,7 +552,8 @@ int ide_raw_taskfile(ide_drive_t *drive, ide_task_t *task, u8 *buf, u16 nsect)
>  	rq->special = task;
>  	task->rq = rq;
>  
> -	return ide_do_drive_cmd(drive, rq, ide_wait);
> +	ret = ide_do_drive_cmd(drive, rq, ide_wait);
> +	return ret ? -EIO : 0;
>  }
>  
>  EXPORT_SYMBOL(ide_raw_taskfile);
> @@ -782,7 +784,8 @@ int ide_cmd_ioctl (ide_drive_t *drive, unsigned int cmd, unsigned long arg)
>  		rq = blk_get_request(drive->queue, READ, __GFP_WAIT);
>  		rq->cmd_type = REQ_TYPE_ATA_TASKFILE;
>  
> -		return ide_do_drive_cmd(drive, rq, ide_wait);
> +		err = ide_do_drive_cmd(drive, rq, ide_wait);
> +		return err ? -EIO : 0;
>  	}
>  
>  	if (copy_from_user(args, (void __user *)arg, 4))
> diff --git a/drivers/ide/ide.c b/drivers/ide/ide.c
> index c5b33f5..8970054 100644
> --- a/drivers/ide/ide.c
> +++ b/drivers/ide/ide.c
> @@ -738,6 +738,8 @@ static int generic_ide_resume(struct device *dev)
>  	rqpm.pm_state = PM_EVENT_ON;
>  
>  	err = ide_do_drive_cmd(drive, rq, ide_head_wait);
> +	if (err)
> +		err = -EIO;
>  
>  	if (err == 0 && dev->driver) {
>  		ide_driver_t *drv = to_ide_driver(dev->driver);
> -- 
> 1.5.4.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ide" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Regards/Gruß,
    Boris.
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Filesystems]     [Linux SCSI]     [Linux RAID]     [Git]     [Kernel Newbies]     [Linux Newbie]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Samba]     [Device Mapper]

  Powered by Linux