Data accessors more patches

PATCH New Boaz Harrosh 5 participants 17 messages Jul 4, 2007 – Jul 12, 2007

Subsystems (4)

SCSI USB libata Microtek X6 scanner

lore

Share & tools

lore

Boaz Harrosh

author

2007-07-04 18:20 UTC #0 lore raw

As a reply to this email I will send some more files converted to new data accessors, and attempts of cleanups
of: !use_sg code path's, use of scsi_for_each_sg and use of scsi_{,un}map_dma().

some of these files are in scsi-ml core but they can be converted now, and are not at all dependent on implementation.

libata-scsi.c - code will now work with scsi_sgtable but farther cleanup is needed.
scsi.c - some prints might as well use accessors now.
scsi_debug.c - This is actually a scsi LLD. Normal conversion.

The rest of the converted files I will send in the following days

Boaz

Boaz Harrosh

author

2007-07-04 18:24 UTC #1 lore raw

PATCH 1/1 libata-scsi.c: convert to use the data buffer accessors

This is a minimal patch needed if we want to use the new scsi_sgtable implementation, by using of the new data accessors. But it is not a complete clean up of the !use_sg path.
Libata-core still has the qc->flags & ATA_QCFLAG_SG and !qc->n_elem code paths. Perhaps an ata maintainer would have a go at it.

- use of new data accessors.
- clean some of the more obvious !use_sg path
- TODO: farther cleanup of qc->flags & ATA_QCFLAG_SG and !qc->n_elem code paths in libata-core
- TODO: Use of new scsi_dma_{map,unmap} if applicable.

---
 drivers/ata/libata-scsi.c |   39 +++++++++++++--------------------------
 1 files changed, 13 insertions(+), 26 deletions(-)

drivers/ata/libata-scsi.c +13 ~~-26~~

index 13603a0..c0af390 100644
@@ -450,13 +450,8 @@ static struct ata_queued_cmd *ata_scsi_qc_new(struct ata_device *dev,
 		qc->scsicmd = cmd;
 		qc->scsidone = done;
 
-		if (cmd->use_sg) {
-			qc->__sg = (struct scatterlist *) cmd->request_buffer;
-			qc->n_elem = cmd->use_sg;
-		} else if (cmd->request_bufflen) {
-			qc->__sg = &qc->sgent;
-			qc->n_elem = 1;
-		}
+		qc->__sg = scsi_sglist(cmd);
+		qc->n_elem = scsi_sg_count(cmd);
 	} else {
 		cmd->result = (DID_OK << 16) | (QUEUE_FULL << 1);
 		done(cmd);

@@ -1496,17 +1491,13 @@ static int ata_scsi_translate(struct ata_device *dev, struct scsi_cmnd *cmd,
 	/* data is present; dma-map it */
 	if (cmd->sc_data_direction == DMA_FROM_DEVICE ||
 	    cmd->sc_data_direction == DMA_TO_DEVICE) {
-		if (unlikely(cmd->request_bufflen < 1)) {
+		if (unlikely(scsi_bufflen(cmd) < 1)) {
 			ata_dev_printk(dev, KERN_WARNING,
 				       "WARNING: zero len r/w req\n");
 			goto err_did;
 		}
 
-		if (cmd->use_sg)
-			ata_sg_init(qc, cmd->request_buffer, cmd->use_sg);
-		else
-			ata_sg_init_one(qc, cmd->request_buffer,
-					cmd->request_bufflen);
+		ata_sg_init(qc, scsi_sglist(cmd), scsi_sg_count(cmd));
 
 		qc->dma_dir = cmd->sc_data_direction;
 	}

@@ -1560,15 +1551,14 @@ static unsigned int ata_scsi_rbuf_get(struct scsi_cmnd *cmd, u8 **buf_out)
 	u8 *buf;
 	unsigned int buflen;
 
-	if (cmd->use_sg) {
-		struct scatterlist *sg;
+	struct scatterlist *sg = scsi_sglist(cmd);
 
-		sg = (struct scatterlist *) cmd->request_buffer;
+	if (sg) {
 		buf = kmap_atomic(sg->page, KM_IRQ0) + sg->offset;
 		buflen = sg->length;
 	} else {
-		buf = cmd->request_buffer;
-		buflen = cmd->request_bufflen;
+		buf = NULL;
+		buflen = 0;
 	}
 
 	*buf_out = buf;

@@ -1588,12 +1578,9 @@ static unsigned int ata_scsi_rbuf_get(struct scsi_cmnd *cmd, u8 **buf_out)
 
 static inline void ata_scsi_rbuf_put(struct scsi_cmnd *cmd, u8 *buf)
 {
-	if (cmd->use_sg) {
-		struct scatterlist *sg;
-
-		sg = (struct scatterlist *) cmd->request_buffer;
+	struct scatterlist *sg = scsi_sglist(cmd);
+	if (sg)
 		kunmap_atomic(buf - sg->offset, KM_IRQ0);
-	}
 }
 
 /**

@@ -2420,7 +2407,7 @@ static unsigned int atapi_xlat(struct ata_queued_cmd *qc)
 			qc->tf.feature |= ATAPI_DMADIR;
 	}
 
-	qc->nbytes = scmd->request_bufflen;
+	qc->nbytes = scsi_bufflen(scmd);
 
 	return 0;
 }

@@ -2633,7 +2620,7 @@ static unsigned int ata_scsi_pass_thru(struct ata_queued_cmd *qc)
 	case ATA_CMD_WRITE_LONG_ONCE:
 		if (tf->protocol != ATA_PROT_PIO || tf->nsect != 1)
 			goto invalid_fld;
-		qc->sect_size = scmd->request_bufflen;
+		qc->sect_size = scsi_bufflen(scmd);
 	}
 
 	/*

@@ -2663,7 +2650,7 @@ static unsigned int ata_scsi_pass_thru(struct ata_queued_cmd *qc)
 	 * TODO: find out if we need to do more here to
 	 *       cover scatter/gather case.
 	 */
-	qc->nbytes = scmd->request_bufflen;
+	qc->nbytes = scsi_bufflen(scmd);
 
 	/* request result TF */
 	qc->flags |= ATA_QCFLAG_RESULT_TF;

Signature · 1 line

1.5.2.2.249.g45fd

Boaz Harrosh

author

2007-07-04 18:26 UTC #2 lore raw

PATCH 1/1 scsi.c: convert to use the data buffer accessors

In reply to Boaz Harrosh

- a couple of prints, they can use the accessors

Signed-off-by Boaz Harrosh <[email protected]>

---
 drivers/scsi/scsi.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

drivers/scsi/scsi.c +1 -1

index 4c1e313..1d1aba6 100644
@@ -368,7 +368,7 @@ void scsi_log_send(struct scsi_cmnd *cmd)
 			if (level > 3) {
 				printk(KERN_INFO "buffer = 0x%p, bufflen = %d,"
 				       " done = 0x%p, queuecommand 0x%p\n",
-					cmd->request_buffer, cmd->request_bufflen,
+					scsi_sglist(cmd), scsi_bufflen(cmd),
 					cmd->done,
 					cmd->device->host->hostt->queuecommand);

Signature · 1 line

1.5.2.2.249.g45fd

Boaz Harrosh

author

2007-07-04 18:28 UTC #3 lore raw

PATCH 1/1 scsi_debug: convert to use the data buffer accessors and !use_sg cleanup

In reply to Boaz Harrosh

scsi_debug is just a synthetic scsi target that simulate a scsi device and can inject errors to test the error handling paths in scsi-ml. As any scsi LLD it is converted to use the new data accessors.
- Use of new data accessors
- Clean of !use_sg code path.

Signed-off-by Boaz Harrosh <[email protected]>

---
 drivers/scsi/scsi_debug.c |   46 +++++++++++++++-----------------------------
 1 files changed, 16 insertions(+), 30 deletions(-)

drivers/scsi/scsi_debug.c +16 ~~-30~~

index 4cd9c58..1e2d6bb 100644
@@ -328,7 +328,7 @@ int scsi_debug_queuecommand(struct scsi_cmnd * SCpnt, done_funct_t done)
 	if (done == NULL)
 		return 0;	/* assume mid level reprocessing command */
 
-	SCpnt->resid = 0;
+	scsi_set_resid(SCpnt, 0);
 	if ((SCSI_DEBUG_OPT_NOISE & scsi_debug_opts) && cmd) {
 		printk(KERN_INFO "scsi_debug: cmd ");
 		for (k = 0, len = SCpnt->cmd_len; k < len; ++k)

@@ -597,31 +597,22 @@ static int check_readiness(struct scsi_cmnd * SCpnt, int reset_only,
 static int fill_from_dev_buffer(struct scsi_cmnd * scp, unsigned char * arr,
 				int arr_len)
 {
-	int k, req_len, act_len, len, active;
+	int k, req_len, act_len, len, active, sg_count;
 	void * kaddr;
 	void * kaddr_off;
 	struct scatterlist * sgpnt;
 
-	if (0 == scp->request_bufflen)
+	if (0 == scsi_bufflen(scp))
 		return 0;
-	if (NULL == scp->request_buffer)
+	if (NULL == scsi_sglist(scp))
 		return (DID_ERROR << 16);
 	if (! ((scp->sc_data_direction == DMA_BIDIRECTIONAL) ||
 	      (scp->sc_data_direction == DMA_FROM_DEVICE)))
 		return (DID_ERROR << 16);
-	if (0 == scp->use_sg) {
-		req_len = scp->request_bufflen;
-		act_len = (req_len < arr_len) ? req_len : arr_len;
-		memcpy(scp->request_buffer, arr, act_len);
-		if (scp->resid)
-			scp->resid -= act_len;
-		else
-			scp->resid = req_len - act_len;
-		return 0;
-	}
-	sgpnt = (struct scatterlist *)scp->request_buffer;
+	sg_count = scsi_sg_count(scp);
 	active = 1;
-	for (k = 0, req_len = 0, act_len = 0; k < scp->use_sg; ++k, ++sgpnt) {
+	req_len = act_len = 0;
+	scsi_for_each_sg(scp, sgpnt, sg_count, k) {
 		if (active) {
 			kaddr = (unsigned char *)
 				kmap_atomic(sgpnt->page, KM_USER0);

@@ -639,10 +630,10 @@ static int fill_from_dev_buffer(struct scsi_cmnd * scp, unsigned char * arr,
 		}
 		req_len += sgpnt->length;
 	}
-	if (scp->resid)
-		scp->resid -= act_len;
+	if (scsi_get_resid(scp))
+		scsi_set_resid(scp, scsi_get_resid(scp) - act_len);
 	else
-		scp->resid = req_len - act_len;
+		scsi_set_resid(scp, req_len - act_len);
 	return 0;
 }

@@ -650,26 +641,21 @@ static int fill_from_dev_buffer(struct scsi_cmnd * scp, unsigned char * arr,
 static int fetch_to_dev_buffer(struct scsi_cmnd * scp, unsigned char * arr,
 			       int max_arr_len)
 {
-	int k, req_len, len, fin;
+	int k, req_len, len, fin, sg_count;
 	void * kaddr;
 	void * kaddr_off;
 	struct scatterlist * sgpnt;
 
-	if (0 == scp->request_bufflen)
+	if (0 == scsi_bufflen(scp))
 		return 0;
-	if (NULL == scp->request_buffer)
+	if (NULL == scsi_sglist(scp))
 		return -1;
 	if (! ((scp->sc_data_direction == DMA_BIDIRECTIONAL) ||
 	      (scp->sc_data_direction == DMA_TO_DEVICE)))
 		return -1;
-	if (0 == scp->use_sg) {
-		req_len = scp->request_bufflen;
-		len = (req_len < max_arr_len) ? req_len : max_arr_len;
-		memcpy(arr, scp->request_buffer, len);
-		return len;
-	}
-	sgpnt = (struct scatterlist *)scp->request_buffer;
-	for (k = 0, req_len = 0, fin = 0; k < scp->use_sg; ++k, ++sgpnt) {
+	sg_count = scsi_sg_count(scp);
+	req_len = fin = 0;
+	scsi_for_each_sg(scp, sgpnt, sg_count, k) {
 		kaddr = (unsigned char *)kmap_atomic(sgpnt->page, KM_USER0);
 		if (NULL == kaddr)
 			return -1;

Signature · 1 line

1.5.2.2.249.g45fd

Boaz Harrosh

author

2007-07-12 13:11 UTC #4 lore raw

PATCH 1/1 microtek.c - use data accessors and !use_sg cleanup

In reply to Boaz Harrosh

- use scsi_cmnd data accessors
- Clean the !use_sg code paths

Signed-off-by Boaz Harrosh <[email protected]>

---
 drivers/usb/image/microtek.c |   32 ++++++++++++++------------------
 1 files changed, 14 insertions(+), 18 deletions(-)

drivers/usb/image/microtek.c +14 ~~-18~~

index 51bd80d..0a5c6e8 100644
@@ -447,7 +447,8 @@ static void mts_data_done( struct urb* transfer )
 	MTS_INT_INIT();
 
 	if ( context->data_length != transfer->actual_length ) {
-		context->srb->resid = context->data_length - transfer->actual_length;
+		scsi_set_resid(context->srb, context->data_length -
+		                                     transfer->actual_length);
 	} else if ( unlikely(transfer->status) ) {
 		context->srb->result = (transfer->status == -ENOENT ? DID_ABORT : DID_ERROR)<<16;
 	}

@@ -490,7 +491,8 @@ static void mts_command_done( struct urb *transfer )
 					   context->data_pipe,
 					   context->data,
 					   context->data_length,
-					   context->srb->use_sg > 1 ? mts_do_sg : mts_data_done);
+					   scsi_sg_count(context->srb) > 1 ?
+					           mts_do_sg : mts_data_done);
 		} else {
 			mts_get_status(transfer);
 		}

@@ -504,21 +506,23 @@ static void mts_do_sg (struct urb* transfer)
 	struct scatterlist * sg;
 	MTS_INT_INIT();
 
-	MTS_DEBUG("Processing fragment %d of %d\n", context->fragment,context->srb->use_sg);
+	MTS_DEBUG("Processing fragment %d of %d\n", context->fragment,
+	                                          scsi_sg_count(context->srb));
 
 	if (unlikely(transfer->status)) {
                 context->srb->result = (transfer->status == -ENOENT ? DID_ABORT : DID_ERROR)<<16;
 		mts_transfer_cleanup(transfer);
         }
 
-	sg = context->srb->request_buffer;
+	sg = scsi_sglist(context->srb);
 	context->fragment++;
 	mts_int_submit_urb(transfer,
 			   context->data_pipe,
 			   page_address(sg[context->fragment].page) +
 			   sg[context->fragment].offset,
 			   sg[context->fragment].length,
-			   context->fragment + 1 == context->srb->use_sg ? mts_data_done : mts_do_sg);
+			   context->fragment + 1 == scsi_sg_count(context->srb) ?
+			   mts_data_done : mts_do_sg);
 	return;
 }

@@ -546,20 +550,12 @@ mts_build_transfer_context(struct scsi_cmnd *srb, struct mts_desc* desc)
 	desc->context.srb = srb;
 	desc->context.fragment = 0;
 
-	if (!srb->use_sg) {
-		if ( !srb->request_bufflen ){
-			desc->context.data = NULL;
-			desc->context.data_length = 0;
-			return;
-		} else {
-			desc->context.data = srb->request_buffer;
-			desc->context.data_length = srb->request_bufflen;
-			MTS_DEBUG("length = %d or %d\n",
-				  srb->request_bufflen, srb->bufflen);
-		}
+	if (!scsi_bufflen(srb)) {
+		desc->context.data = NULL;
+		desc->context.data_length = 0;
+		return;
 	} else {
-		MTS_DEBUG("Using scatter/gather\n");
-		sg = srb->request_buffer;
+		sg = scsi_sglist(srb);
 		desc->context.data = page_address(sg[0].page) + sg[0].offset;
 		desc->context.data_length = sg[0].length;
 	}

Signature · 1 line

1.5.2.2.249.g45fd

Boaz Harrosh

author

2007-07-12 13:24 UTC #5 lore raw

Re: [PATCH] microtek.c - use data accessors and !use_sg cleanup

Quoted 75 lines · Boaz Harrosh

- use scsi_cmnd data accessors
- Clean the !use_sg code paths

Signed-off-by Boaz Harrosh <[email protected]>

---
 drivers/usb/image/microtek.c |   32 ++++++++++++++------------------
 1 files changed, 14 insertions(+), 18 deletions(-)

drivers/usb/image/microtek.c +10 ~~-16~~

index 51bd80d..0a5c6e8 100644
@@ -447,7 +447,8 @@ static void mts_data_done( struct urb* transfer )
 	MTS_INT_INIT();
 
 	if ( context->data_length != transfer->actual_length ) {
-		context->srb->resid = context->data_length - transfer->actual_length;
+		scsi_set_resid(context->srb, context->data_length -
+		                                     transfer->actual_length);
 	} else if ( unlikely(transfer->status) ) {
 		context->srb->result = (transfer->status == -ENOENT ? DID_ABORT : DID_ERROR)<<16;
 	}

@@ -490,7 +491,8 @@ static void mts_command_done( struct urb *transfer )
 					   context->data_pipe,
 					   context->data,
 					   context->data_length,
-					   context->srb->use_sg > 1 ? mts_do_sg : mts_data_done);
+					   scsi_sg_count(context->srb) > 1 ?
+					           mts_do_sg : mts_data_done);
 		} else {
 			mts_get_status(transfer);
 		}

@@ -504,21 +506,23 @@ static void mts_do_sg (struct urb* transfer)
 	struct scatterlist * sg;
 	MTS_INT_INIT();
 
-	MTS_DEBUG("Processing fragment %d of %d\n", context->fragment,context->srb->use_sg);
+	MTS_DEBUG("Processing fragment %d of %d\n", context->fragment,
+	                                          scsi_sg_count(context->srb));
 
 	if (unlikely(transfer->status)) {
                 context->srb->result = (transfer->status == -ENOENT ? DID_ABORT : DID_ERROR)<<16;
 		mts_transfer_cleanup(transfer);
         }
 
-	sg = context->srb->request_buffer;
+	sg = scsi_sglist(context->srb);
 	context->fragment++;
 	mts_int_submit_urb(transfer,
 			   context->data_pipe,
 			   page_address(sg[context->fragment].page) +
 			   sg[context->fragment].offset,
 			   sg[context->fragment].length,
-			   context->fragment + 1 == context->srb->use_sg ? mts_data_done : mts_do_sg);
+			   context->fragment + 1 == scsi_sg_count(context->srb) ?
+			   mts_data_done : mts_do_sg);
 	return;
 }

@@ -546,20 +550,12 @@ mts_build_transfer_context(struct scsi_cmnd *srb, struct mts_desc* desc)
 	desc->context.srb = srb;
 	desc->context.fragment = 0;
 
-	if (!srb->use_sg) {
-		if ( !srb->request_bufflen ){
-			desc->context.data = NULL;
-			desc->context.data_length = 0;
-			return;
-		} else {
-			desc->context.data = srb->request_buffer;
-			desc->context.data_length = srb->request_bufflen;
-			MTS_DEBUG("length = %d or %d\n",
-				  srb->request_bufflen, srb->bufflen);
-		}
+	if (!scsi_bufflen(srb)) {

drivers/usb/image/microtek.c mts_build_transfer_context(struct scsi_cmnd *srb, struct mts_desc* desc) · line 561

Quoted 10 lines

drivers/usb/image/microtek.c +4 -2

+		desc->context.data = NULL;
+		desc->context.data_length = 0;
+		return;
 	} else {
-		MTS_DEBUG("Using scatter/gather\n");
-		sg = srb->request_buffer;
+		sg = scsi_sglist(srb);
 		desc->context.data = page_address(sg[0].page) + sg[0].offset;
 		desc->context.data_length = sg[0].length;
 	}

I forgot TO: of maintainers

Oliver Neukum <[email protected]>
Greg Kroah-Hartman <[email protected]>

You seem to have signed some of the last patches
on this driver. Please review

Thanks
Boaz Harrosh

Greg KH

2007-07-12 16:59 UTC #6 lore raw

Re: [PATCH] microtek.c - use data accessors and !use_sg cleanup

Quoted 89 lines · Boaz Harrosh

Quoted 75 lines · Boaz Harrosh

- use scsi_cmnd data accessors
- Clean the !use_sg code paths

Signed-off-by Boaz Harrosh <[email protected]>

---
 drivers/usb/image/microtek.c |   32 ++++++++++++++------------------
 1 files changed, 14 insertions(+), 18 deletions(-)

drivers/usb/image/microtek.c +10 ~~-16~~

index 51bd80d..0a5c6e8 100644
@@ -447,7 +447,8 @@ static void mts_data_done( struct urb* transfer )
 	MTS_INT_INIT();
 
 	if ( context->data_length != transfer->actual_length ) {
-		context->srb->resid = context->data_length - transfer->actual_length;
+		scsi_set_resid(context->srb, context->data_length -
+		                                     transfer->actual_length);
 	} else if ( unlikely(transfer->status) ) {
 		context->srb->result = (transfer->status == -ENOENT ? DID_ABORT : DID_ERROR)<<16;
 	}

@@ -490,7 +491,8 @@ static void mts_command_done( struct urb *transfer )
 					   context->data_pipe,
 					   context->data,
 					   context->data_length,
-					   context->srb->use_sg > 1 ? mts_do_sg : mts_data_done);
+					   scsi_sg_count(context->srb) > 1 ?
+					           mts_do_sg : mts_data_done);
 		} else {
 			mts_get_status(transfer);
 		}

@@ -504,21 +506,23 @@ static void mts_do_sg (struct urb* transfer)
 	struct scatterlist * sg;
 	MTS_INT_INIT();
 
-	MTS_DEBUG("Processing fragment %d of %d\n", context->fragment,context->srb->use_sg);
+	MTS_DEBUG("Processing fragment %d of %d\n", context->fragment,
+	                                          scsi_sg_count(context->srb));
 
 	if (unlikely(transfer->status)) {
                 context->srb->result = (transfer->status == -ENOENT ? DID_ABORT : DID_ERROR)<<16;
 		mts_transfer_cleanup(transfer);
         }
 
-	sg = context->srb->request_buffer;
+	sg = scsi_sglist(context->srb);
 	context->fragment++;
 	mts_int_submit_urb(transfer,
 			   context->data_pipe,
 			   page_address(sg[context->fragment].page) +
 			   sg[context->fragment].offset,
 			   sg[context->fragment].length,
-			   context->fragment + 1 == context->srb->use_sg ? mts_data_done : mts_do_sg);
+			   context->fragment + 1 == scsi_sg_count(context->srb) ?
+			   mts_data_done : mts_do_sg);
 	return;
 }

@@ -546,20 +550,12 @@ mts_build_transfer_context(struct scsi_cmnd *srb, struct mts_desc* desc)
 	desc->context.srb = srb;
 	desc->context.fragment = 0;
 
-	if (!srb->use_sg) {
-		if ( !srb->request_bufflen ){
-			desc->context.data = NULL;
-			desc->context.data_length = 0;
-			return;
-		} else {
-			desc->context.data = srb->request_buffer;
-			desc->context.data_length = srb->request_bufflen;
-			MTS_DEBUG("length = %d or %d\n",
-				  srb->request_bufflen, srb->bufflen);
-		}
+	if (!scsi_bufflen(srb)) {

drivers/usb/image/microtek.c mts_build_transfer_context(struct scsi_cmnd *srb, struct mts_desc* desc) · line 561

Quoted 10 lines

drivers/usb/image/microtek.c +4 -2

+		desc->context.data = NULL;
+		desc->context.data_length = 0;
+		return;
 	} else {
-		MTS_DEBUG("Using scatter/gather\n");
-		sg = srb->request_buffer;
+		sg = scsi_sglist(srb);
 		desc->context.data = page_address(sg[0].page) + sg[0].offset;
 		desc->context.data_length = sg[0].length;
 	}

I forgot TO: of maintainers

Quoted 5 lines

Oliver Neukum <[email protected]>
Greg Kroah-Hartman <[email protected]>
You seem to have signed some of the last patches
on this driver. Please review

No objetion from me. Care to resend it to me, and CC: the linux-usb-devel list also so that it is in a format that I can apply to the tree?

thanks,

greg k-h

Boaz Harrosh

author

2007-07-12 17:04 UTC #7 lore raw

Re: [PATCH] microtek.c - use data accessors and !use_sg cleanup

Quoted 95 lines · Greg KH

Quoted 88 lines · Boaz Harrosh

Quoted 75 lines · Boaz Harrosh

- use scsi_cmnd data accessors
- Clean the !use_sg code paths

Signed-off-by Boaz Harrosh <[email protected]>

---
 drivers/usb/image/microtek.c |   32 ++++++++++++++------------------
 1 files changed, 14 insertions(+), 18 deletions(-)

drivers/usb/image/microtek.c +10 ~~-16~~

index 51bd80d..0a5c6e8 100644
@@ -447,7 +447,8 @@ static void mts_data_done( struct urb* transfer )
 	MTS_INT_INIT();
 
 	if ( context->data_length != transfer->actual_length ) {
-		context->srb->resid = context->data_length - transfer->actual_length;
+		scsi_set_resid(context->srb, context->data_length -
+		                                     transfer->actual_length);
 	} else if ( unlikely(transfer->status) ) {
 		context->srb->result = (transfer->status == -ENOENT ? DID_ABORT : DID_ERROR)<<16;
 	}

@@ -490,7 +491,8 @@ static void mts_command_done( struct urb *transfer )
 					   context->data_pipe,
 					   context->data,
 					   context->data_length,
-					   context->srb->use_sg > 1 ? mts_do_sg : mts_data_done);
+					   scsi_sg_count(context->srb) > 1 ?
+					           mts_do_sg : mts_data_done);
 		} else {
 			mts_get_status(transfer);
 		}

@@ -504,21 +506,23 @@ static void mts_do_sg (struct urb* transfer)
 	struct scatterlist * sg;
 	MTS_INT_INIT();
 
-	MTS_DEBUG("Processing fragment %d of %d\n", context->fragment,context->srb->use_sg);
+	MTS_DEBUG("Processing fragment %d of %d\n", context->fragment,
+	                                          scsi_sg_count(context->srb));
 
 	if (unlikely(transfer->status)) {
                 context->srb->result = (transfer->status == -ENOENT ? DID_ABORT : DID_ERROR)<<16;
 		mts_transfer_cleanup(transfer);
         }
 
-	sg = context->srb->request_buffer;
+	sg = scsi_sglist(context->srb);
 	context->fragment++;
 	mts_int_submit_urb(transfer,
 			   context->data_pipe,
 			   page_address(sg[context->fragment].page) +
 			   sg[context->fragment].offset,
 			   sg[context->fragment].length,
-			   context->fragment + 1 == context->srb->use_sg ? mts_data_done : mts_do_sg);
+			   context->fragment + 1 == scsi_sg_count(context->srb) ?
+			   mts_data_done : mts_do_sg);
 	return;
 }

@@ -546,20 +550,12 @@ mts_build_transfer_context(struct scsi_cmnd *srb, struct mts_desc* desc)
 	desc->context.srb = srb;
 	desc->context.fragment = 0;
 
-	if (!srb->use_sg) {
-		if ( !srb->request_bufflen ){
-			desc->context.data = NULL;
-			desc->context.data_length = 0;
-			return;
-		} else {
-			desc->context.data = srb->request_buffer;
-			desc->context.data_length = srb->request_bufflen;
-			MTS_DEBUG("length = %d or %d\n",
-				  srb->request_bufflen, srb->bufflen);
-		}
+	if (!scsi_bufflen(srb)) {

drivers/usb/image/microtek.c mts_build_transfer_context(struct scsi_cmnd *srb, struct mts_desc* desc) · line 561

Quoted 10 lines

drivers/usb/image/microtek.c +4 -2

+		desc->context.data = NULL;
+		desc->context.data_length = 0;
+		return;
 	} else {
-		MTS_DEBUG("Using scatter/gather\n");
-		sg = srb->request_buffer;
+		sg = scsi_sglist(srb);
 		desc->context.data = page_address(sg[0].page) + sg[0].offset;
 		desc->context.data_length = sg[0].length;
 	}

I forgot TO: of maintainers

Quoted 5 lines

Oliver Neukum <[email protected]>
Greg Kroah-Hartman <[email protected]>
You seem to have signed some of the last patches
on this driver. Please review

Quoted 7 lines

No objetion from me. Care to resend it to me, and CC: the linux-usb-devel list also so that it is in a format that I can apply to the tree?
thanks,
greg k-h

Will do. I hope I can post with out signing up.

James Bottomley

2007-07-12 17:05 UTC #8 lore raw

Re: [PATCH] microtek.c - use data accessors and !use_sg cleanup

In reply to Greg KH

Quoted 3 lines · Greg KH

No objetion from me. Care to resend it to me, and CC: the linux-usb-devel list also so that it is in a format that I can apply to the tree?

This will give you a cross tree dependency on scsi-misc-2.6 because of the new data accessors ... if that's what you want to do?

James

Greg KH

Acked-by

2007-07-12 18:06 UTC #9 lore raw

Re: [PATCH] microtek.c - use data accessors and !use_sg cleanup

Quoted 7 lines · James Bottomley

Quoted 3 lines · Greg KH
No objetion from me. Care to resend it to me, and CC: the linux-usb-devel list also so that it is in a format that I can apply to the tree?
This will give you a cross tree dependency on scsi-misc-2.6 because of the new data accessors ... if that's what you want to do?

Oops, sorry, I wasn't aware of that.

No, I don't want that at all, feel free to push this through your tree, I have no objection to it at all:

Acked-by Greg Kroah-Hartman <[email protected]>

thanks,

greg k-h

Oliver Neukum

maintainer

2007-07-12 13:40 UTC #10 lore raw

Re: [PATCH] microtek.c - use data accessors and !use_sg cleanup

In reply to Boaz Harrosh

Am Donnerstag, 12. Juli 2007 schrieb Boaz Harrosh:

Quoted 3 lines

- use scsi_cmnd data accessors
- Clean the !use_sg code paths

This doesn't compile, at least on x86_64, so NAK.

Regards
Oliver

CC [M] drivers/usb/image/microtek.o
drivers/usb/image/microtek.c: In function ‘mts_data_done’:
drivers/usb/image/microtek.c:450: warning: implicit declaration of function ‘scsi_set_resid’ drivers/usb/image/microtek.c: In function ‘mts_command_done’:
drivers/usb/image/microtek.c:494: warning: implicit declaration of function ‘scsi_sg_count’ drivers/usb/image/microtek.c: In function ‘mts_do_sg’:
drivers/usb/image/microtek.c:517: warning: implicit declaration of function ‘scsi_sglist’ drivers/usb/image/microtek.c:517: warning: assignment makes pointer from integer without a cast drivers/usb/image/microtek.c: In function ‘mts_build_transfer_context’:
drivers/usb/image/microtek.c:553: warning: implicit declaration of function ‘scsi_bufflen’ drivers/usb/image/microtek.c:558: warning: assignment makes pointer from integer without a cast
MODPOST vmlinux
Kernel: arch/x86_64/boot/bzImage is ready (#4)
Building modules, stage 2.
MODPOST 1705 modules
ERROR: "scsi_set_resid" [drivers/usb/image/microtek.ko] undefined!
ERROR: "scsi_sg_count" [drivers/usb/image/microtek.ko] undefined!
ERROR: "scsi_sglist" [drivers/usb/image/microtek.ko] undefined!
ERROR: "scsi_bufflen" [drivers/usb/image/microtek.ko] undefined!
make[1]: *** [__modpost] Fehler 1
make: *** [modules] Fehler 2
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

James Bottomley

2007-07-12 13:54 UTC #11 lore raw

Re: [PATCH] microtek.c - use data accessors and !use_sg cleanup

Quoted 10 lines · Oliver Neukum

Am Donnerstag, 12. Juli 2007 schrieb Boaz Harrosh:
Quoted 3 lines
- use scsi_cmnd data accessors
- Clean the !use_sg code paths
This doesn't compile, at least on x86_64, so NAK.
Regards
Oliver

Quoted 11 lines

CC [M] drivers/usb/image/microtek.o
drivers/usb/image/microtek.c: In function ‘mts_data_done’:
drivers/usb/image/microtek.c:450: warning: implicit declaration of function ‘scsi_set_resid’ drivers/usb/image/microtek.c: In function ‘mts_command_done’:
drivers/usb/image/microtek.c:494: warning: implicit declaration of function ‘scsi_sg_count’ drivers/usb/image/microtek.c: In function ‘mts_do_sg’:
drivers/usb/image/microtek.c:517: warning: implicit declaration of function ‘scsi_sglist’ drivers/usb/image/microtek.c:517: warning: assignment makes pointer from integer without a cast drivers/usb/image/microtek.c: In function ‘mts_build_transfer_context’:
drivers/usb/image/microtek.c:553: warning: implicit declaration of function ‘scsi_bufflen’ drivers/usb/image/microtek.c:558: warning: assignment makes pointer from integer without a cast

These would appear to be because you're not building against scsi-misc rather than because of a failure in the patch.

James

-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Oliver Neukum

maintainer

2007-07-12 13:57 UTC #12 lore raw

Re: [PATCH] microtek.c - use data accessors and !use_sg cleanup

Am Donnerstag, 12. Juli 2007 schrieb James Bottomley:

Quoted 23 lines · Oliver Neukum

Quoted 10 lines
Am Donnerstag, 12. Juli 2007 schrieb Boaz Harrosh:
Quoted 3 lines
- use scsi_cmnd data accessors
- Clean the !use_sg code paths
This doesn't compile, at least on x86_64, so NAK.
Regards
Oliver
Quoted 11 lines
CC [M] drivers/usb/image/microtek.o
drivers/usb/image/microtek.c: In function ‘mts_data_done’:
drivers/usb/image/microtek.c:450: warning: implicit declaration of function ‘scsi_set_resid’ drivers/usb/image/microtek.c: In function ‘mts_command_done’:
drivers/usb/image/microtek.c:494: warning: implicit declaration of function ‘scsi_sg_count’ drivers/usb/image/microtek.c: In function ‘mts_do_sg’:
drivers/usb/image/microtek.c:517: warning: implicit declaration of function ‘scsi_sglist’ drivers/usb/image/microtek.c:517: warning: assignment makes pointer from integer without a cast drivers/usb/image/microtek.c: In function ‘mts_build_transfer_context’:
drivers/usb/image/microtek.c:553: warning: implicit declaration of function ‘scsi_bufflen’ drivers/usb/image/microtek.c:558: warning: assignment makes pointer from integer without a cast

Quoted 2 lines

These would appear to be because you're not building against scsi-misc rather than because of a failure in the patch.

Thanks. Which tree should I test against? Where do I get it?

Regards
Oliver

-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Boaz Harrosh

author

2007-07-12 14:01 UTC #13 lore raw

Re: [PATCH] microtek.c - use data accessors and !use_sg cleanup

Quoted 6 lines · Oliver Neukum

Thanks. Which tree should I test against? Where do I get it?
Regards
Oliver

git-clone git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi-misc-2.6.git scsi-misc-2.6

Boaz

Boaz Harrosh

author

2007-07-12 13:58 UTC #14 lore raw

Re: [PATCH] microtek.c - use data accessors and !use_sg cleanup

In reply to Oliver Neukum

Quoted 22 lines · Oliver Neukum

Am Donnerstag, 12. Juli 2007 schrieb Boaz Harrosh:
Quoted 2 lines
- use scsi_cmnd data accessors
- Clean the !use_sg code paths
This doesn't compile, at least on x86_64, so NAK.
Regards
Oliver
CC [M] drivers/usb/image/microtek.o
drivers/usb/image/microtek.c: In function ‘mts_data_done’:
drivers/usb/image/microtek.c:450: warning: implicit declaration of function ‘scsi_set_resid’ drivers/usb/image/microtek.c: In function ‘mts_command_done’:
drivers/usb/image/microtek.c:494: warning: implicit declaration of function ‘scsi_sg_count’ drivers/usb/image/microtek.c: In function ‘mts_do_sg’:
drivers/usb/image/microtek.c:517: warning: implicit declaration of function ‘scsi_sglist’ drivers/usb/image/microtek.c:517: warning: assignment makes pointer from integer without a cast drivers/usb/image/microtek.c: In function ‘mts_build_transfer_context’:
drivers/usb/image/microtek.c:553: warning: implicit declaration of function ‘scsi_bufflen’ drivers/usb/image/microtek.c:558: warning: assignment makes pointer from integer without a cast
MODPOST vmlinux
Kernel: arch/x86_64/boot/bzImage is ready (#4)

Quoted 8 lines

Building modules, stage 2.
MODPOST 1705 modules
ERROR: "scsi_set_resid" [drivers/usb/image/microtek.ko] undefined!
ERROR: "scsi_sg_count" [drivers/usb/image/microtek.ko] undefined!
ERROR: "scsi_sglist" [drivers/usb/image/microtek.ko] undefined!
ERROR: "scsi_bufflen" [drivers/usb/image/microtek.ko] undefined! make[1]: *** [__modpost] Fehler 1
make: *** [modules] Fehler 2

This is over current scsi-2.6-misc git tree, and the infrastructure needed for this is going into the linux 2.6.23 merging window. The proposed changes above are for probably 2.6.24 merge window.

It is all a part of a clean sweep of every scsi driver in the tree. Most drivers have already been converted and accepted into current scsi-2.6-misc and are do into the current merge window.

The microtek.c driver is sitting in USB land but it is a scsi driver and should be maintained through the scsi-2.6-misc tree? No?

Boaz
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Boaz Harrosh

author

2007-07-12 13:14 UTC #15 lore raw

In reply to Boaz Harrosh

Quoted 12 lines · Boaz Harrosh

As a reply to this email I will send some more files converted to new data accessors, and attempts of cleanups
of: !use_sg code path's, use of scsi_for_each_sg and use of scsi_{,un}map_dma().
some of these files are in scsi-ml core but they can be converted now, and are not at all dependent on implementation.
libata-scsi.c - code will now work with scsi_sgtable but farther cleanup is needed.
scsi.c - some prints might as well use accessors now.
scsi_debug.c - This is actually a scsi LLD. Normal conversion.

Quoted 8 lines

The rest of the converted files I will send in the following days
Boaz
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Tomo hi.

I would like your Signed-off-by: on all of these. Since you have more experience with these cleanups.

Thanks

FUJITA Tomonori

2007-07-12 22:50 UTC #16 lore raw

From: Boaz Harrosh <[email protected]>
Subject: Re: Data accessors more patches
Date: Thu, 12 Jul 2007 16:13:52 +0300

Quoted 17 lines

Quoted 16 lines · Boaz Harrosh
As a reply to this email I will send some more files converted to new data accessors, and attempts of cleanups
of: !use_sg code path's, use of scsi_for_each_sg and use of scsi_{,un}map_dma().
some of these files are in scsi-ml core but they can be converted now, and are not at all dependent on implementation.
libata-scsi.c - code will now work with scsi_sgtable but farther cleanup is needed.
scsi.c - some prints might as well use accessors now.
scsi_debug.c - This is actually a scsi LLD. Normal conversion.
The rest of the converted files I will send in the following days
Boaz

Quoted 8 lines

Quoted 4 lines
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Tomo hi.
I would like your Signed-off-by: on all of these. Since you have more experience with these cleanups.

aha152x, ide-scsi and libata-scsi?

Well, when I tried to convert them, I thought that it would be better to leave them to the maintainers. They look broken or too complicated.

I'd like to save 'just do grep and replace' and 'convert without confidence' options.