Skip to contentSkip to conversation

Please pull drivers/scsi/dec_esp.c from Linus' git

PATCH Martin Michlmayr 4 participants 10 messages Feb 13, 2006 – Feb 17, 2006

Subsystems (2)
SCSI MIPS
lore
Share & tools
lore

Martin Michlmayr

author
#0 lore raw

linux-mips git on DECstation currently fails to compile with:

CC drivers/scsi/dec_esp.o
drivers/scsi/dec_esp.c:137: error: static declaration of ‘dec_esp_detect’ follows non-static declaration drivers/scsi/dec_esp.c:101: error: previous declaration of ‘dec_esp_detect’ was here make[2]: *** [drivers/scsi/dec_esp.o] Error 1
make[1]: *** [drivers/scsi] Error 2

This declaration is correct in Linus' git tree but not in ours. Ralf, can you please compare the two again and sync.

The current delta is included below.

home/tbm/index.html?p=linux%2Fkernel%2Fgit%2Ftorvalds%2Flinux-2.6.git;a=blob_plain;h=a35ee43a48df78e1b627c63e33e6895035d0fe1a;f=drivers%2Fscsi%2Fdec_esp.c renamed from scsi/dec_esp.c +13 -13
@@ -55,7 +55,7 @@
 
 static int  dma_bytes_sent(struct NCR_ESP *esp, int fifo_count);
 static void dma_drain(struct NCR_ESP *esp);
-static int  dma_can_transfer(struct NCR_ESP *esp, Scsi_Cmnd * sp);
+static int  dma_can_transfer(struct NCR_ESP *esp, struct scsi_cmnd *sp);
 static void dma_dump_state(struct NCR_ESP *esp);
 static void dma_init_read(struct NCR_ESP *esp, u32 vaddress, int length);
 static void dma_init_write(struct NCR_ESP *esp, u32 vaddress, int length);

@@ -64,9 +64,9 @@
 static int  dma_irq_p(struct NCR_ESP *esp);
 static int  dma_ports_p(struct NCR_ESP *esp);
 static void dma_setup(struct NCR_ESP *esp, u32 addr, int count, int write);
-static void dma_mmu_get_scsi_one(struct NCR_ESP *esp, Scsi_Cmnd * sp);
-static void dma_mmu_get_scsi_sgl(struct NCR_ESP *esp, Scsi_Cmnd * sp);
-static void dma_advance_sg(Scsi_Cmnd * sp);
+static void dma_mmu_get_scsi_one(struct NCR_ESP *esp, struct scsi_cmnd * sp);
+static void dma_mmu_get_scsi_sgl(struct NCR_ESP *esp, struct scsi_cmnd * sp);
+static void dma_advance_sg(struct scsi_cmnd * sp);
 
 static void pmaz_dma_drain(struct NCR_ESP *esp);
 static void pmaz_dma_init_read(struct NCR_ESP *esp, u32 vaddress, int length);

@@ -74,7 +74,7 @@
 static void pmaz_dma_ints_off(struct NCR_ESP *esp);
 static void pmaz_dma_ints_on(struct NCR_ESP *esp);
 static void pmaz_dma_setup(struct NCR_ESP *esp, u32 addr, int count, int write);
-static void pmaz_dma_mmu_get_scsi_one(struct NCR_ESP *esp, Scsi_Cmnd * sp);
+static void pmaz_dma_mmu_get_scsi_one(struct NCR_ESP *esp, struct scsi_cmnd * sp);
 
 #define TC_ESP_RAM_SIZE 0x20000
 #define ESP_TGT_DMA_SIZE ((TC_ESP_RAM_SIZE/7) & ~(sizeof(int)-1))

@@ -98,7 +98,7 @@
 static irqreturn_t scsi_dma_err_int(int, void *, struct pt_regs *);
 static irqreturn_t scsi_dma_int(int, void *, struct pt_regs *);
 
-int dec_esp_detect(struct scsi_host_template * tpnt);
+static int dec_esp_detect(struct scsi_host_template * tpnt);
 
 static int dec_esp_release(struct Scsi_Host *shost)
 {

@@ -112,7 +112,7 @@
 
 static struct scsi_host_template driver_template = {
 	.proc_name		= "dec_esp",
-	.proc_info		= &esp_proc_info,
+	.proc_info		= esp_proc_info,
 	.name			= "NCR53C94",
 	.detect			= dec_esp_detect,
 	.slave_alloc		= esp_slave_alloc,

@@ -230,7 +230,7 @@
 			mem_start = get_tc_base_addr(slot);
 
 			/* Store base addr into esp struct */
-			esp->slot = mem_start;
+			esp->slot = CPHYSADDR(mem_start);
 
 			esp->dregs = 0;
 			esp->eregs = (void *)CKSEG1ADDR(mem_start +

@@ -379,7 +379,7 @@
 	}
 }
 
-static int dma_can_transfer(struct NCR_ESP *esp, Scsi_Cmnd * sp)
+static int dma_can_transfer(struct NCR_ESP *esp, struct scsi_cmnd * sp)
 {
 	return sp->SCp.this_residual;
 }

@@ -491,12 +491,12 @@
 		dma_init_write(esp, addr, count);
 }
 
-static void dma_mmu_get_scsi_one(struct NCR_ESP *esp, Scsi_Cmnd * sp)
+static void dma_mmu_get_scsi_one(struct NCR_ESP *esp, struct scsi_cmnd * sp)
 {
 	sp->SCp.ptr = (char *)virt_to_phys(sp->request_buffer);
 }
 
-static void dma_mmu_get_scsi_sgl(struct NCR_ESP *esp, Scsi_Cmnd * sp)
+static void dma_mmu_get_scsi_sgl(struct NCR_ESP *esp, struct scsi_cmnd * sp)
 {
 	int sz = sp->SCp.buffers_residual;
 	struct scatterlist *sg = sp->SCp.buffer;

@@ -508,7 +508,7 @@
 	sp->SCp.ptr = (char *)(sp->SCp.buffer->dma_address);
 }
 
-static void dma_advance_sg(Scsi_Cmnd * sp)
+static void dma_advance_sg(struct scsi_cmnd * sp)
 {
 	sp->SCp.ptr = (char *)(sp->SCp.buffer->dma_address);
 }

@@ -572,7 +572,7 @@
 		pmaz_dma_init_write(esp, addr, count);
 }
 
-static void pmaz_dma_mmu_get_scsi_one(struct NCR_ESP *esp, Scsi_Cmnd * sp)
+static void pmaz_dma_mmu_get_scsi_one(struct NCR_ESP *esp, struct scsi_cmnd * sp)
 {
 	sp->SCp.ptr = (char *)virt_to_phys(sp->request_buffer);
 }
Signature · 2 lines
Martin Michlmayr
http://www.cyrius.com/
#1 lore raw

home/tbm/index.html?p=linux%2Fkernel%2Fgit%2Ftorvalds%2Flinux-2.6.git;a=blob_plain;h=a35ee43a48df78e1b627c63e33e6895035d0fe1a;f=drivers%2Fscsi%2Fdec_esp.c line 236

Quoted 9 lines · Martin Michlmayr
home/tbm/index.html?p=linux%2Fkernel%2Fgit%2Ftorvalds%2Flinux-2.6.git;a=blob_plain;h=a35ee43a48df78e1b627c63e33e6895035d0fe1a;f=drivers%2Fscsi%2Fdec_esp.c renamed from scsi/dec_esp.c +1 -1
@@ -230,7 +230,7 @@
 			mem_start = get_tc_base_addr(slot);
 
 			/* Store base addr into esp struct */
-			esp->slot = mem_start;
+			esp->slot = CPHYSADDR(mem_start);
 
 			esp->dregs = 0;
 			esp->eregs = (void *)CKSEG1ADDR(mem_start +

I merged allmost all of the differences from mainline except this one.

Maciej, does this need the CPHYSADDR() op or not here?

Ralf

#2 lore raw
Quoted 7 lines · Ralf Baechle
Quoted 7 lines
home/tbm/index.html?p=linux%2Fkernel%2Fgit%2Ftorvalds%2Flinux-2.6.git;a=blob_plain;h=a35ee43a48df78e1b627c63e33e6895035d0fe1a;f=drivers%2Fscsi%2Fdec_esp.c renamed from scsi/dec_esp.c +1 -1
@@ -230,7 +230,7 @@
 			mem_start = get_tc_base_addr(slot);
 
 			/* Store base addr into esp struct */
-			esp->slot = mem_start;
+			esp->slot = CPHYSADDR(mem_start);
Quoted 6 lines

home/tbm/index.html?p=linux%2Fkernel%2Fgit%2Ftorvalds%2Flinux-2.6.git;a=blob_plain;h=a35ee43a48df78e1b627c63e33e6895035d0fe1a;f=drivers%2Fscsi%2Fdec_esp.c line 236

Quoted 2 lines
home/tbm/index.html?p=linux%2Fkernel%2Fgit%2Ftorvalds%2Flinux-2.6.git;a=blob_plain;h=a35ee43a48df78e1b627c63e33e6895035d0fe1a;f=drivers%2Fscsi%2Fdec_esp.c renamed from scsi/dec_esp.c +0 -0
 			esp->dregs = 0;
 			esp->eregs = (void *)CKSEG1ADDR(mem_start +

I merged allmost all of the differences from mainline except this one.

Maciej, does this need the CPHYSADDR() op or not here?

Of course not as get_tc_base_addr() returns a physical address these
days. Thanks for spotting this bit.

Maciej

#3 lore raw
Quoted 21 lines · Maciej W. Rozycki

Date: Thu, 16 Feb 2006 10:17:38 +0000 (GMT)
From: "Maciej W. Rozycki" <[email protected]>
To: Ralf Baechle <[email protected]>
cc: Martin Michlmayr <[email protected]>, [email protected]
Subject: Re: Please pull drivers/scsi/dec_esp.c from Linus' git
Content-Type: TEXT/PLAIN; charset=US-ASCII

Quoted 12 lines · Ralf Baechle

Review line 236

Quoted 9 lines
(quoted diff) +1 -1
@@ -230,7 +230,7 @@
 			mem_start = get_tc_base_addr(slot);
 
 			/* Store base addr into esp struct */
-			esp->slot = mem_start;
+			esp->slot = CPHYSADDR(mem_start);
 
 			esp->dregs = 0;
 			esp->eregs = (void *)CKSEG1ADDR(mem_start +

I merged allmost all of the differences from mainline except this one.

Quoted 4 lines
Quoted 1 line

Maciej, does this need the CPHYSADDR() op or not here?

Of course not as get_tc_base_addr() returns a physical address these
days. Thanks for spotting this bit.

Ok, I sent this bit upstream.

That still leaves below gem to sort out.

Ralf

drivers/scsi/NCR53C9x.h +1 -6
index 65a9b37..81d03d1 100644
@@ -145,12 +145,7 @@
 
 #ifndef MULTIPLE_PAD_SIZES
 
-#ifdef CONFIG_CPU_HAS_WB
-#include <asm/wbflush.h>
-#define esp_write(__reg, __val) do{(__reg) = (__val); wbflush();} while(0)
-#else
-#define esp_write(__reg, __val) ((__reg) = (__val))
-#endif
+#define esp_write(__reg, __val) do{(__reg) = (__val); iob();} while(0)
 #define esp_read(__reg) (__reg)
 
 struct ESP_regs {
#4 lore raw
Quoted 1 line · Ralf Baechle

That still leaves below gem to sort out.

Yeah -- there is that mmiowb() macro that is supposed to fit here, but
some MIPS-based hardware is ordered weakly (and strangely) enough for this single macro to be a bit insufficient. I think we should have at least mmiowb() and mmiob() (corresponding to wmb() and mb(), respectively) as there is a system we support that does writes in order, but snoops the writeback buffer (the R3220). Another one is worse yet as does all of that plus byte gathering (the R2020). At least the latter cannot have the NCR/Emulex SCSI chip and uses DEC's own design instead (a DC7061 gate array highly suspected to also support DSSI if appropriately configured).

I'm not sure if we really need mmiorb() -- probably not.

Maciej

#5 lore raw
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Maciej" == Maciej W Rozycki <[email protected]> writes:

Quoted 1 line · Ralf Baechle
Quoted 1 line

That still leaves below gem to sort out.

Maciej> Yeah -- there is that mmiowb() macro that is supposed to fit Maciej> here, but some MIPS-based hardware is ordered weakly (and Maciej> strangely) enough for this single macro to be a bit Maciej> insufficient. I think we should have at least mmiowb() and Maciej> mmiob() (corresponding to wmb() and mb(), respectively) as Maciej> there is a system we support that does writes in order, but Maciej> snoops the writeback buffer (the R3220). Another one is worse Maciej> yet as does all of that plus byte gathering (the R2020). At Maciej> least the latter cannot have the NCR/Emulex SCSI chip and uses Maciej> DEC's own design instead (a DC7061 gate array highly suspected Maciej> to also support DSSI if appropriately configured).

Maciej> I'm not sure if we really need mmiorb() -- probably not.

Just make mmiowb() strong enough on those platforms. There's really no reason to introduce yet another variation at this point.

Jes

#6 lore raw
Quoted 2 lines · Jes Sorensen

Just make mmiowb() strong enough on those platforms. There's really no reason to introduce yet another variation at this point.

It's just horribly slow and an overkill if it's only ordering of
consecutive writes rather than ordering of a read after a write that has to be guaranteed. But perhaps we could keep abusing wmb() for the former...

Maciej

#7 lore raw
Quoted 9 lines · Maciej W. Rozycki
Quoted 2 lines · Jes Sorensen

Just make mmiowb() strong enough on those platforms. There's really no reason to introduce yet another variation at this point.

It's just horribly slow and an overkill if it's only ordering of
consecutive writes rather than ordering of a read after a write that has to be guaranteed. But perhaps we could keep abusing wmb() for the former...

Depends on where it's happening. If it's the CPU doing it before it hits the bus then wmb() would probably be ok.

Jes

#8 lore raw
Quoted 6 lines · Jes Sorensen
Quoted 3 lines

It's just horribly slow and an overkill if it's only ordering of consecutive writes rather than ordering of a read after a write that has to be guaranteed. But perhaps we could keep abusing wmb() for the former...

Depends on where it's happening. If it's the CPU doing it before it hits the bus then wmb() would probably be ok.

These systems only allow the CPU to access devices other than RAM. But
consistency between platforms is important as the same drivers may be used for systems that have different access rules. Examples are currently the defxx and the tgafb drivers. They should work correctly with the R3220 writeback buffer as well as with PCI.

Maciej

#9 lore raw
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Maciej" == Maciej W Rozycki <[email protected]> writes:

Maciej> These systems only allow the CPU to access devices other than Maciej> RAM. But consistency between platforms is important as the Maciej> same drivers may be used for systems that have different Maciej> access rules. Examples are currently the defxx and the tgafb Maciej> drivers. They should work correctly with the R3220 writeback Maciej> buffer as well as with PCI.

So just change the implementation of mmiowb depending on whether it's on an R3220 or a PCI system.

Cheers,
Jes