Sym53c8xx_2 Odd Byte Data Transfer patches

Sym53c8xx_2 Odd Byte Data Transfer patches

[Stephens, Larry]

[-- Attachment #1: Type: text/plain, Size: 981 bytes --]


> These changes fix a problem with odd byte data transfers to a wide device
> (16 bits) in the LSI sym53c8xx_2 driver.  This issue is primarily seen
> only with tape drives although the code doesn't restrict the fix to any
> particular device.
> 
> These changes are nearly identical to those made in the sym53c8xx driver
> over a year ago.
> 
> The changed driver has been checked out by the LSI Logic developer and by
> the customer who requested that the sym53c8xx_2 be updated for the odd
> byte data transfer problem.  
> 
> File 2.4_sym53c8xx_2.2.1.18.patch is the patch file for the 2.4 kernel
> driver.  That driver version number has been bumped to 2.1.18.
> 
> File 2.6_sym53c8xx_2.2.2.1.patch is the patch file for the 2.6 kernel
> driver.  That driver version  number has been bumped to 2.2.1.
> 
> Thank you for accepting this request.  Please let us know if you need
> additional information.
> 
>  <<2.4_sym53c8xx_2.2.1.18.patch>>  <<2.6_sym53c8xx_2.2.2.1.patch>> 

[-- Attachment #2: 2.4_sym53c8xx_2.2.1.18.patch --]
[-- Type: application/octet-stream, Size: 2744 bytes --]

diff -uarN b/drivers/scsi/sym53c8xx_2/sym_glue.c a/drivers/scsi/sym53c8xx_2/sym_glue.c
--- b/drivers/scsi/sym53c8xx_2/sym_glue.c	2005-04-28 09:15:21.000000000 -0500
+++ a/drivers/scsi/sym53c8xx_2/sym_glue.c	2005-04-28 09:15:50.000000000 -0500
@@ -627,13 +627,22 @@
 {
 	struct sym_tblmove *data = &cp->phys.data[SYM_CONF_MAX_SG-1];
 	int segment;
+	unsigned int len = cmd->request_bufflen;
 
-	cp->data_len = cmd->request_bufflen;
-
-	if (cmd->request_bufflen) {
+	if (len) {
 		bus_addr_t baddr = map_scsi_single_data(np, cmd);
 		if (baddr) {
-			sym_build_sge(np, data, baddr, cmd->request_bufflen);
+			if ( len & 0x01 )  /* odd # of bytes? */
+			{
+				tcb_p tp = &np->target[cp->target];
+				if (tp->head.wval & EWS)  /* negotiated wide? */
+				{
+					len++;
+					cp->odd_byte_adjustment++;
+				}
+			}
+			cp->data_len = len;
+			sym_build_sge(np, data, baddr, len);
 			segment = 1;
 		}
 		else
@@ -665,6 +674,16 @@
 		for (segment = 0; segment < use_sg; segment++) {
 			bus_addr_t baddr = bus_sg_dma_address(&scatter[segment]);
 			unsigned int len = bus_sg_dma_len(&scatter[segment]);
+			if ( len & 0x01 )  /* odd # of bytes? */
+			{
+				tcb_p tp = &np->target[cp->target];
+				if (tp->head.wval & EWS)  /* negotiated wide? */
+				{
+					len++;
+					cp->odd_byte_adjustment++;
+				}
+			}
+
 
 			sym_build_sge(np, &data[segment], baddr, len);
 			cp->data_len += len;
diff -uarN b/drivers/scsi/sym53c8xx_2/sym_hipd.c a/drivers/scsi/sym53c8xx_2/sym_hipd.c
--- b/drivers/scsi/sym53c8xx_2/sym_hipd.c	2005-04-28 09:15:21.000000000 -0500
+++ a/drivers/scsi/sym53c8xx_2/sym_hipd.c	2005-04-28 09:15:50.000000000 -0500
@@ -50,7 +50,7 @@
  * SUCH DAMAGE.
  */
 
-#define SYM_DRIVER_NAME	"sym-2.1.17a"
+#define SYM_DRIVER_NAME	"sym-2.1.18"
 
 #ifdef __FreeBSD__
 #include <dev/sym/sym_glue.h>
@@ -3921,6 +3921,8 @@
 		resid += (tmp & 0xffffff);
 	}
 
+	resid -= cp->odd_byte_adjustment;
+
 	/*
 	 *  Hopefully, the result is not too wrong.
 	 */
@@ -4828,7 +4830,7 @@
 	/*
 	 *  Remember all informations needed to free this CCB.
 	 */
-	cp->to_abort = 0;
+	cp->to_abort = cp->odd_byte_adjustment = 0;
 	cp->tag	   = tag;
 	cp->order  = tag_order;
 	cp->target = tn;
diff -uarN b/drivers/scsi/sym53c8xx_2/sym_hipd.h a/drivers/scsi/sym53c8xx_2/sym_hipd.h
--- b/drivers/scsi/sym53c8xx_2/sym_hipd.h	2005-04-28 09:15:21.000000000 -0500
+++ a/drivers/scsi/sym53c8xx_2/sym_hipd.h	2005-04-28 09:15:50.000000000 -0500
@@ -862,6 +862,7 @@
 				/*  NO_TAG means no tag		*/
 	u_char	target;
 	u_char	lun;
+	u_char	odd_byte_adjustment;
 	ccb_p	link_ccbh;	/* Host adapter CCB hash chain	*/
 	SYM_QUEHEAD
 		link_ccbq;	/* Link to free/busy CCB queue	*/

[-- Attachment #3: 2.6_sym53c8xx_2.2.2.1.patch --]
[-- Type: application/octet-stream, Size: 3062 bytes --]

diff -uarN b/drivers/scsi/sym53c8xx_2/sym_defs.h a/drivers/scsi/sym53c8xx_2/sym_defs.h
--- b/drivers/scsi/sym53c8xx_2/sym_defs.h	2005-04-28 14:32:32.000000000 -0500
+++ a/drivers/scsi/sym53c8xx_2/sym_defs.h	2005-04-28 10:19:14.000000000 -0500
@@ -40,7 +40,7 @@
 #ifndef SYM_DEFS_H
 #define SYM_DEFS_H
 
-#define SYM_VERSION "2.2.0"
+#define SYM_VERSION "2.2.1"
 #define SYM_DRIVER_NAME	"sym-" SYM_VERSION
 
 /*
diff -uarN b/drivers/scsi/sym53c8xx_2/sym_glue.c a/drivers/scsi/sym53c8xx_2/sym_glue.c
--- b/drivers/scsi/sym53c8xx_2/sym_glue.c	2005-04-28 14:32:32.000000000 -0500
+++ a/drivers/scsi/sym53c8xx_2/sym_glue.c	2005-04-28 14:29:53.000000000 -0500
@@ -389,13 +389,22 @@
 {
 	struct sym_tblmove *data = &cp->phys.data[SYM_CONF_MAX_SG-1];
 	int segment;
+	unsigned int len = cmd->request_bufflen;
 
-	cp->data_len = cmd->request_bufflen;
-
-	if (cmd->request_bufflen) {
+	if (len) {
 		dma_addr_t baddr = map_scsi_single_data(np, cmd);
 		if (baddr) {
-			sym_build_sge(np, data, baddr, cmd->request_bufflen);
+			if ( len & 0x01 )  /* odd # of bytes? */
+			{
+				struct sym_tcb *tp = &np->target[cp->target];
+				if (tp->head.wval & EWS)  /* negotiated wide? */
+				{
+					len++;
+					cp->odd_byte_adjustment++;
+				}
+			}
+			cp->data_len = len;
+			sym_build_sge(np, data, baddr, len);
 			segment = 1;
 		} else {
 			segment = -2;
@@ -430,6 +439,15 @@
 		for (segment = 0; segment < use_sg; segment++) {
 			dma_addr_t baddr = sg_dma_address(&scatter[segment]);
 			unsigned int len = sg_dma_len(&scatter[segment]);
+			if ( len & 0x01 )  /* odd # of bytes? */
+			{
+				struct sym_tcb *tp = &np->target[cp->target];
+				if (tp->head.wval & EWS)  /* negotiated wide? */
+				{
+					len++;
+					cp->odd_byte_adjustment++;
+				}
+			}
 
 			sym_build_sge(np, &data[segment], baddr, len);
 			cp->data_len += len;
diff -uarN b/drivers/scsi/sym53c8xx_2/sym_hipd.c a/drivers/scsi/sym53c8xx_2/sym_hipd.c
--- b/drivers/scsi/sym53c8xx_2/sym_hipd.c	2005-04-28 14:32:32.000000000 -0500
+++ a/drivers/scsi/sym53c8xx_2/sym_hipd.c	2005-04-28 10:26:59.000000000 -0500
@@ -3877,6 +3877,8 @@
 		resid += (tmp & 0xffffff);
 	}
 
+	resid -= cp->odd_byte_adjustment;
+
 	/*
 	 *  Hopefully, the result is not too wrong.
 	 */
@@ -4761,7 +4763,7 @@
 	/*
 	 *  Remember all informations needed to free this CCB.
 	 */
-	cp->to_abort = 0;
+	cp->to_abort = cp->odd_byte_adjustment = 0;
 	cp->tag	   = tag;
 	cp->order  = tag_order;
 	cp->target = tn;
diff -uarN b/drivers/scsi/sym53c8xx_2/sym_hipd.h a/drivers/scsi/sym53c8xx_2/sym_hipd.h
--- b/drivers/scsi/sym53c8xx_2/sym_hipd.h	2005-04-28 14:32:32.000000000 -0500
+++ a/drivers/scsi/sym53c8xx_2/sym_hipd.h	2005-04-28 10:15:37.000000000 -0500
@@ -790,6 +790,7 @@
 				/*  NO_TAG means no tag		*/
 	u_char	target;
 	u_char	lun;
+	u_char	odd_byte_adjustment;
 	struct sym_ccb *link_ccbh;	/* Host adapter CCB hash chain	*/
 	SYM_QUEHEAD link_ccbq;	/* Link to free/busy CCB queue	*/
 	u32	startp;		/* Initial data pointer		*/

RE: Sym53c8xx_2 Odd Byte Data Transfer patches

[Tony Battersby]

I just gave 2.4_sym53c8xx_2.2.1.18.patch a quick test - it fixes the
problem and everything else still tests OK.

Thanks,
Anthony J. Battersby
Cybernetics

Re: Sym53c8xx_2 Odd Byte Data Transfer patches

[Matthew Wilcox]

On Mon, May 09, 2005 at 06:16:48AM -0600, Stephens, Larry wrote:
> > These changes fix a problem with odd byte data transfers to a wide device
> > (16 bits) in the LSI sym53c8xx_2 driver.  This issue is primarily seen
> > only with tape drives although the code doesn't restrict the fix to any
> > particular device.

While this patch does seem to solve the problem of transferring an odd
number of bytes to the device, I recently received a bug report saying
that we don't accept an odd number of bytes transferred from a device.
It seems to me that it's going to require modifying the scsi scripts in
order to do this.  Do you agree?  Have you looked into transfers in the
opposite direction as part of this work?

-- 
"Next the statesmen will invent cheap lies, putting the blame upon 
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince 
himself that the war is just, and will thank God for the better sleep 
he enjoys after this process of grotesque self-deception." -- Mark Twain

RE: Sym53c8xx_2 Odd Byte Data Transfer patches

[Tony Battersby]

> While this patch does seem to solve the problem of transferring an odd
> number of bytes to the device, I recently received a bug report saying
> that we don't accept an odd number of bytes transferred from a device.
> It seems to me that it's going to require modifying the scsi
> scripts in
> order to do this.  Do you agree?  Have you looked into
> transfers in the
> opposite direction as part of this work?

If a target sends an odd number of bytes during a wide DATA IN phase,
then it should send an IGNORE WIDE RESIDUE message immediately after
exiting the data phase to inform the initiator.  The sym53c8xx_2 driver
in the 2.4 kernel series handles this correctly except on a REQUEST
SENSE command issued for autosense (previously sym53c8xx_2 would reject
an IWR message for autosense; I submitted a patch a good while ago to
make it ignore IWR in this case instead of rejecting it).  Not
accounting for IWR on autosense is not a big deal though, so I consider
sym53c8xx_2 in 2.4 kernels to handle odd-length DATA IN phases
acceptably.  I haven't tested the 2.6 kernels.

Anthony J. Battersby
Cybernetics

RE: Sym53c8xx_2 Odd Byte Data Transfer patches

[Stephens, Larry]

This fix handles the case where an application requests that an odd number
of bytes of data be transferred either from or to a wide (16 bit) device
(usually a tape drive) on an LSI Logic adapter controlled by the sym53c8xx_2
driver.

Our customer has provided us with an application that allows a user to
specify how many bytes of data to write or read from the wide tape drive.
Baring an NDA agreement, I probably could provide the application if
necessary.

-----Original Message-----
From: Tony Battersby [mailto:tonyb@cybernetics.com]
Sent: Tuesday, June 14, 2005 3:01 PM
To: 'Matthew Wilcox'; 'Stephens, Larry'
Cc: 'Linux-Scsi@Vger. Kernel. Org (E-mail)'
Subject: RE: Sym53c8xx_2 Odd Byte Data Transfer patches


> While this patch does seem to solve the problem of transferring an odd
> number of bytes to the device, I recently received a bug report saying
> that we don't accept an odd number of bytes transferred from a device.
> It seems to me that it's going to require modifying the scsi
> scripts in
> order to do this.  Do you agree?  Have you looked into
> transfers in the
> opposite direction as part of this work?

If a target sends an odd number of bytes during a wide DATA IN phase,
then it should send an IGNORE WIDE RESIDUE message immediately after
exiting the data phase to inform the initiator.  The sym53c8xx_2 driver
in the 2.4 kernel series handles this correctly except on a REQUEST
SENSE command issued for autosense (previously sym53c8xx_2 would reject
an IWR message for autosense; I submitted a patch a good while ago to
make it ignore IWR in this case instead of rejecting it).  Not
accounting for IWR on autosense is not a big deal though, so I consider
sym53c8xx_2 in 2.4 kernels to handle odd-length DATA IN phases
acceptably.  I haven't tested the 2.6 kernels.

Anthony J. Battersby
Cybernetics