* [PATCH 2/3] tmscsim: remove bogus endianness conversions
@ 2007-05-04 20:59 Guennadi Liakhovetski
2007-05-05 9:37 ` Christoph Hellwig
0 siblings, 1 reply; 4+ messages in thread
From: Guennadi Liakhovetski @ 2007-05-04 20:59 UTC (permalink / raw)
To: linux-scsi, James Bottomley
cpu_to_le32 endianness conversions in tmscsim.c, followed by
arithmetic operations don't look correct. Besides, {in,out}[wl]
already perform the necessary conversions. Further, bus addresses
of request buffers are guaranteed to be (mapped) under 4G by
current scsi- and block-layer defaults. This could be explicitly
enforced by using blk_queue_bounce_limit(), which, however,
doesn't seem to be the common practice among SCSI drivers.
Signed-off-by: G. Liakhovetski <g.liakhovetski@gmx.de>
diff --git a/drivers/scsi/tmscsim.c b/drivers/scsi/tmscsim.c
index d0e171c..be869d0 100644
--- a/drivers/scsi/tmscsim.c
+++ b/drivers/scsi/tmscsim.c
@@ -778,8 +778,8 @@ dc390_DataOut_0( struct dc390_acb* pACB, struct dc390_srb* pSRB, u8 *psstatus)
pSRB->pSegmentList++;
psgl = pSRB->pSegmentList;
- pSRB->SGBusAddr = cpu_to_le32(pci_dma_lo32(sg_dma_address(psgl)));
- pSRB->SGToBeXferLen = cpu_to_le32(sg_dma_len(psgl));
+ pSRB->SGBusAddr = sg_dma_address(psgl);
+ pSRB->SGToBeXferLen = sg_dma_len(psgl);
}
else
pSRB->SGToBeXferLen = 0;
@@ -842,7 +842,7 @@ dc390_DataIn_0( struct dc390_acb* pACB, struct dc390_srb* pSRB, u8 *psstatus)
DEBUG1(ResidCnt = ((unsigned long) DC390_read8 (CtcReg_High) << 16) \
+ ((unsigned long) DC390_read8 (CtcReg_Mid) << 8) \
+ ((unsigned long) DC390_read8 (CtcReg_Low)));
- DEBUG1(printk (KERN_DEBUG "Count_2_Zero (ResidCnt=%i,ToBeXfer=%li),", ResidCnt, pSRB->SGToBeXferLen));
+ DEBUG1(printk (KERN_DEBUG "Count_2_Zero (ResidCnt=%u,ToBeXfer=%lu),", ResidCnt, pSRB->SGToBeXferLen));
DC390_write8 (DMA_Cmd, READ_DIRECTION+DMA_IDLE_CMD);
@@ -853,8 +853,8 @@ dc390_DataIn_0( struct dc390_acb* pACB, struct dc390_srb* pSRB, u8 *psstatus)
pSRB->pSegmentList++;
psgl = pSRB->pSegmentList;
- pSRB->SGBusAddr = cpu_to_le32(pci_dma_lo32(sg_dma_address(psgl)));
- pSRB->SGToBeXferLen = cpu_to_le32(sg_dma_len(psgl));
+ pSRB->SGBusAddr = sg_dma_address(psgl);
+ pSRB->SGToBeXferLen = sg_dma_len(psgl);
}
else
pSRB->SGToBeXferLen = 0;
@@ -921,11 +921,12 @@ din_1:
ptr = (u8 *) bus_to_virt( pSRB->SGBusAddr );
*ptr = bval;
- pSRB->SGBusAddr++; xferCnt++;
+ pSRB->SGBusAddr++;
+ xferCnt++;
pSRB->TotalXferredLen++;
pSRB->SGToBeXferLen--;
}
- DEBUG1(printk (KERN_DEBUG "Xfered: %li, Total: %li, Remaining: %li\n", xferCnt,\
+ DEBUG1(printk (KERN_DEBUG "Xfered: %lu, Total: %lu, Remaining: %lu\n", xferCnt,\
pSRB->TotalXferredLen, pSRB->SGToBeXferLen));
}
@@ -1157,14 +1158,14 @@ dc390_restore_ptr (struct dc390_acb* pACB, struct dc390_srb* pSRB)
{
pSRB->pSegmentList++;
psgl = pSRB->pSegmentList;
- pSRB->SGBusAddr = cpu_to_le32(pci_dma_lo32(sg_dma_address(psgl)));
- pSRB->SGToBeXferLen = cpu_to_le32(sg_dma_len(psgl));
+ pSRB->SGBusAddr = sg_dma_address(psgl);
+ pSRB->SGToBeXferLen = sg_dma_len(psgl);
}
else
pSRB->SGToBeXferLen = 0;
}
- pSRB->SGToBeXferLen -= (pSRB->Saved_Ptr - pSRB->TotalXferredLen);
- pSRB->SGBusAddr += (pSRB->Saved_Ptr - pSRB->TotalXferredLen);
+ pSRB->SGToBeXferLen -= pSRB->Saved_Ptr - pSRB->TotalXferredLen;
+ pSRB->SGBusAddr += pSRB->Saved_Ptr - pSRB->TotalXferredLen;
printk (KERN_INFO "DC390: Pointer restored. Segment %i, Total %li, Bus %08lx\n",
pSRB->SGIndex, pSRB->Saved_Ptr, pSRB->SGBusAddr);
@@ -1315,8 +1316,8 @@ dc390_DataIO_Comm( struct dc390_acb* pACB, struct dc390_srb* pSRB, u8 ioDir)
if( !pSRB->SGToBeXferLen )
{
psgl = pSRB->pSegmentList;
- pSRB->SGBusAddr = cpu_to_le32(pci_dma_lo32(sg_dma_address(psgl)));
- pSRB->SGToBeXferLen = cpu_to_le32(sg_dma_len(psgl));
+ pSRB->SGBusAddr = sg_dma_address(psgl);
+ pSRB->SGToBeXferLen = sg_dma_len(psgl);
DEBUG1(printk (KERN_DEBUG " DC390: Next SG segment."));
}
lval = pSRB->SGToBeXferLen;
diff --git a/drivers/scsi/tmscsim.h b/drivers/scsi/tmscsim.h
index 9b66fa8..c3d8c80 100644
--- a/drivers/scsi/tmscsim.h
+++ b/drivers/scsi/tmscsim.h
@@ -19,14 +19,6 @@
#define SEL_TIMEOUT 153 /* 250 ms selection timeout (@ 40 MHz) */
-#define pci_dma_lo32(a) (a & 0xffffffff)
-
-typedef u8 UCHAR; /* 8 bits */
-typedef u16 USHORT; /* 16 bits */
-typedef u32 UINT; /* 32 bits */
-typedef unsigned long ULONG; /* 32/64 bits */
-
-
/*
;-----------------------------------------------------------------------
; SCSI Request Block
@@ -43,7 +35,9 @@ struct scatterlist *pSegmentList;
struct scatterlist Segmentx; /* make a one entry of S/G list table */
-unsigned long SGBusAddr; /*;a segment starting address as seen by AM53C974A*/
+unsigned long SGBusAddr; /*;a segment starting address as seen by AM53C974A
+ in CPU endianness. We're only getting 32-bit bus
+ addresses by default */
unsigned long SGToBeXferLen; /*; to be xfer length */
unsigned long TotalXferredLen;
unsigned long SavedTotXLen;
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH 2/3] tmscsim: remove bogus endianness conversions
2007-05-04 20:59 [PATCH 2/3] tmscsim: remove bogus endianness conversions Guennadi Liakhovetski
@ 2007-05-05 9:37 ` Christoph Hellwig
2007-05-05 21:18 ` Guennadi Liakhovetski
0 siblings, 1 reply; 4+ messages in thread
From: Christoph Hellwig @ 2007-05-05 9:37 UTC (permalink / raw)
To: Guennadi Liakhovetski; +Cc: linux-scsi, James Bottomley
On Fri, May 04, 2007 at 10:59:40PM +0200, Guennadi Liakhovetski wrote:
> cpu_to_le32 endianness conversions in tmscsim.c, followed by
> arithmetic operations don't look correct. Besides, {in,out}[wl]
> already perform the necessary conversions. Further, bus addresses
> of request buffers are guaranteed to be (mapped) under 4G by
> current scsi- and block-layer defaults. This could be explicitly
> enforced by using blk_queue_bounce_limit(), which, however,
> doesn't seem to be the common practice among SCSI drivers.
Looks good to me. If there are structures in dma memory in the driver
it would help to give the __le annotation to get the whole endianess
handling completely right.
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH 2/3] tmscsim: remove bogus endianness conversions
2007-05-05 9:37 ` Christoph Hellwig
@ 2007-05-05 21:18 ` Guennadi Liakhovetski
2007-05-06 11:20 ` Christoph Hellwig
0 siblings, 1 reply; 4+ messages in thread
From: Guennadi Liakhovetski @ 2007-05-05 21:18 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-scsi, James Bottomley
On Sat, 5 May 2007, Christoph Hellwig wrote:
> On Fri, May 04, 2007 at 10:59:40PM +0200, Guennadi Liakhovetski wrote:
> > cpu_to_le32 endianness conversions in tmscsim.c, followed by
> > arithmetic operations don't look correct. Besides, {in,out}[wl]
> > already perform the necessary conversions. Further, bus addresses
> > of request buffers are guaranteed to be (mapped) under 4G by
> > current scsi- and block-layer defaults. This could be explicitly
> > enforced by using blk_queue_bounce_limit(), which, however,
> > doesn't seem to be the common practice among SCSI drivers.
>
> Looks good to me. If there are structures in dma memory in the driver
> it would help to give the __le annotation to get the whole endianess
> handling completely right.
Yeah, but these variables are not like that. Actually, I don't think
there are any variables in tmscsim that get passed to the controller over
DMA.
Thanks
Guennadi
---
Guennadi Liakhovetski
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2007-05-06 11:20 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-05-04 20:59 [PATCH 2/3] tmscsim: remove bogus endianness conversions Guennadi Liakhovetski
2007-05-05 9:37 ` Christoph Hellwig
2007-05-05 21:18 ` Guennadi Liakhovetski
2007-05-06 11:20 ` Christoph Hellwig
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox