On Thursday 12 June 2008, Borislav Petkov wrote:
> This is the first one in the series attempting to phase out ide_atapi_pc and use
> block request structs. Also, the pc request type is now REQ_TYPE_BLOCK_PC. As a result,
> idefloppy_blockpc_cmd becomes unused and can go.
Do not attack too many dragons at once or they will slay you... ;)
IOW this patch mixes too many changes (some _really_ non-trivial)
in one go which results in rather bad outcome...
[...]
> diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
> index b368943..ffebf83 100644
> --- a/drivers/ide/ide-floppy.c
> +++ b/drivers/ide/ide-floppy.c
> @@ -217,7 +217,7 @@ static int idefloppy_end_request(ide_drive_t *drive, int uptodate, int nsecs)
> /* Why does this happen? */
> if (!rq)
> return 0;
> - if (!blk_special_request(rq)) {
> + if (!blk_pc_request(rq)) {
> /* our real local end request function */
> ide_end_request(drive, uptodate, nsecs);
> return 0;
> @@ -282,13 +282,14 @@ static void idefloppy_update_buffers(ide_drive_t *drive,
> * pass through the driver.
> */
> static void idefloppy_queue_pc_head(ide_drive_t *drive, struct ide_atapi_pc *pc,
> - struct request *rq)
> + struct request *rq, unsigned char *cmd)
> {
> struct ide_floppy_obj *floppy = drive->driver_data;
>
> blk_rq_init(NULL, rq);
> rq->buffer = (char *) pc;
> - rq->cmd_type = REQ_TYPE_SPECIAL;
> + rq->cmd_type = REQ_TYPE_BLOCK_PC;
> + memcpy(rq->cmd, cmd, 12);
> rq->cmd_flags |= REQ_PREEMPT;
> rq->rq_disk = floppy->disk;
> ide_do_drive_cmd(drive, rq);
REQUEST SENSE command is switched from REQ_TYPE_SPECIAL to REQ_TYPE_BLOCK_PC
which should belong to a separate patch and is premature IMHO (the other
ATAPI drivers seem to still use REQ_TYPE_SPECIAL so probably it makes sense
to unify/move REQUEST_SENSE handling to ide-atapi.c first).
> @@ -323,10 +324,10 @@ static void ide_floppy_callback(ide_drive_t *drive)
> if (floppy->failed_pc == pc)
> floppy->failed_pc = NULL;
>
> - if (pc->c[0] == GPCMD_READ_10 || pc->c[0] == GPCMD_WRITE_10 ||
> - (pc->rq && blk_pc_request(pc->rq)))
> + if (pc->rq->cmd[0] == GPCMD_READ_10 || pc->rq->cmd[0] == GPCMD_WRITE_10
> + || (pc->rq && blk_pc_request(pc->rq)))
> uptodate = 1; /* FIXME */
This actually seems to break REQUEST SENSE handling
(please note the blk_pc_request() check above)... :(
> - else if (pc->c[0] == GPCMD_REQUEST_SENSE) {
> + else if (pc->rq->cmd[0] == GPCMD_REQUEST_SENSE) {
[...]
> @@ -592,25 +601,6 @@ static void idefloppy_create_rw_cmd(idefloppy_floppy_t *floppy,
> pc->flags |= PC_FLAG_DMA_OK;
> }
>
> -static void idefloppy_blockpc_cmd(idefloppy_floppy_t *floppy,
> - struct ide_atapi_pc *pc, struct request *rq)
> -{
> - idefloppy_init_pc(pc);
> - memcpy(pc->c, rq->cmd, sizeof(pc->c));
> - pc->rq = rq;
> - pc->b_count = rq->data_len;
> - if (rq->data_len && rq_data_dir(rq) == WRITE)
> - pc->flags |= PC_FLAG_WRITING;
> - pc->buf = rq->data;
> - if (rq->bio)
> - pc->flags |= PC_FLAG_DMA_OK;
> - /*
> - * possibly problematic, doesn't look like ide-floppy correctly
> - * handled scattered requests if dma fails...
> - */
> - pc->req_xfer = pc->buf_size = rq->data_len;
> -}
[...]
> @@ -644,12 +634,9 @@ static ide_startstop_t idefloppy_do_request(ide_drive_t *drive,
> }
> pc = idefloppy_next_pc_storage(drive);
> idefloppy_create_rw_cmd(floppy, pc, rq, block);
> - } else if (blk_special_request(rq)) {
> + } else if (blk_pc_request(rq))
> pc = (struct ide_atapi_pc *) rq->buffer;
> - } else if (blk_pc_request(rq)) {
> - pc = idefloppy_next_pc_storage(drive);
> - idefloppy_blockpc_cmd(floppy, pc, rq);
> - } else {
> + else {
> blk_dump_rq_flags(rq,
> "ide-floppy: unsupported command in queue");
> idefloppy_end_request(drive, 0, 0);
Also the above changes seem to break handling of blk_pc_request() requests
(i.e. SG_IO ioctl) because they will be no longer initialized properly.
OTOH the main change (pc->c to rq->cmd) looks OK (only minor complaint is
that 'u8' is both shorter and more readable than 'unsigned short')...
[ I had to also skip patches #16-17 because of skipping this patch. ]
--
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
