Skip to contentSkip to conversation

2.4: Preemption fixes for Broadcom DMA Page operations

PATCH In review Manish Lachwani 3 participants 4 messages Dec 2, 2004

Subsystems (2)
Networking MIPS
lore
Share & tools
lore

Manish Lachwani

author
#0 lore raw

PATCH 1/1 2.4: Preemption fixes for Broadcom DMA Page operations

Hello !

The attached patch implements preempt_disable/preempt_enable around the SB1 DMA page operations. Please review ...

Thanks
Manish Lachwani

mips/mm/pg-sb1.c renamed from mips/mm/pg-sb1.c.orig +31 -7
@@ -157,7 +157,11 @@
 
 void sb1_dma_init(void)
 {
-	int cpu = smp_processor_id();
+	int cpu;
+
+	preempt_disable();
+
+	cpu = smp_processor_id();
 	uint64_t base_val = PHYSADDR(&page_descr[cpu]) | V_DM_DSCR_BASE_RINGSZ(1);
 
 	out64(base_val,

@@ -166,15 +170,23 @@
 	      IO_SPACE_BASE + A_DM_REGISTER(cpu, R_DM_DSCR_BASE));
 	out64(base_val | M_DM_DSCR_BASE_ENABL,
 	      IO_SPACE_BASE + A_DM_REGISTER(cpu, R_DM_DSCR_BASE));
+
+	preempt_enable();
 }
 
 void sb1_clear_page_dma(void *page)
 {
-	int cpu = smp_processor_id();
+	int cpu;
+
+	preempt_disable();
+
+	cpu = smp_processor_id();
 
 	/* if the page is above Kseg0, use old way */
-	if (KSEGX(page) != K0BASE)
+	if (KSEGX(page) != K0BASE) {
+		preempt_enable();
 		return sb1_clear_page(page);
+	}
 
 	page_descr[cpu].dscr_a = PHYSADDR(page) | M_DM_DSCRA_ZERO_MEM | M_DM_DSCRA_L2C_DEST | M_DM_DSCRA_INTERRUPT;
 	page_descr[cpu].dscr_b = V_DM_DSCRB_SRC_LENGTH(PAGE_SIZE);

@@ -187,17 +199,27 @@
 	while (!(in64(IO_SPACE_BASE + A_DM_REGISTER(cpu, R_DM_DSCR_BASE_DEBUG)) & M_DM_DSCR_BASE_INTERRUPT))
 		;
 	in64(IO_SPACE_BASE + A_DM_REGISTER(cpu, R_DM_DSCR_BASE));
+
+	preempt_enable();
 }
 
 void sb1_copy_page_dma(void *to, void *from)
 {
-	unsigned long from_phys = PHYSADDR(from);
-	unsigned long to_phys = PHYSADDR(to);
-	int cpu = smp_processor_id();
+	unsigned long from_phys;
+	unsigned long to_phys;
+	int cpu;
+
+	preempt_disable();
+
+	from_phys = PHYSADDR(from);
+	to_phys = PHYSADDR(to);
+	cpu = smp_processor_id();
 
 	/* if either page is above Kseg0, use old way */
-	if ((KSEGX(to) != K0BASE) || (KSEGX(from) != K0BASE))
+	if ((KSEGX(to) != K0BASE) || (KSEGX(from) != K0BASE)) {
+		preempt_enable();
 		return sb1_copy_page(to, from);
+	}
 
 	page_descr[cpu].dscr_a = PHYSADDR(to_phys) | M_DM_DSCRA_L2C_DEST | M_DM_DSCRA_INTERRUPT;
 	page_descr[cpu].dscr_b = PHYSADDR(from_phys) | V_DM_DSCRB_SRC_LENGTH(PAGE_SIZE);

@@ -210,6 +232,8 @@
 	while (!(in64(IO_SPACE_BASE + A_DM_REGISTER(cpu, R_DM_DSCR_BASE_DEBUG)) & M_DM_DSCR_BASE_INTERRUPT))
 		;
 	in64(IO_SPACE_BASE + A_DM_REGISTER(cpu, R_DM_DSCR_BASE));
+
+	preempt_enable();
 }
 
 #endif
char/Config.in renamed from char/Config.in.orig +1 -1
@@ -178,7 +178,7 @@
          define_bool CONFIG_AU1X00_USB_DEVICE y
          fi
       fi
-      if [ "$CONFIG_SIBYTE_SB1250" = "y" ]; then
+      if [ "$CONFIG_SIBYTE_BOARD" = "y" ]; then
          bool '  Support for sb1250 onchip DUART' CONFIG_SIBYTE_SB1250_DUART
          if [ "$CONFIG_SIBYTE_SB1250_DUART" = "y" ]; then
             bool '  Console on SB1250 DUART' CONFIG_SIBYTE_SB1250_DUART_CONSOLE
net/Config.in renamed from net/Config.in.orig +1 -1
@@ -104,7 +104,7 @@
    if [ "$CONFIG_IDT_79EB434" = "y" ]; then
       bool '  IDT RC32434 Ethernet support' CONFIG_IDT_RC32434_ETH
    fi
-   if [ "$CONFIG_SIBYTE_SB1250" = "y" ]; then
+   if [ "$CONFIG_SIBYTE_BOARD" = "y" ]; then
       tristate '  SB1250 Ethernet support' CONFIG_NET_SB1250_MAC
    fi
    if [ "$CONFIG_SGI_IP27" = "y" ]; then
#1 lore raw
Quoted 2 lines · Manish Lachwani

The attached patch implements preempt_disable/preempt_enable around the SB1 DMA page operations. Please review ...

We don't support preemption in 2.4 -> bitbucket.

Ralf

#2 lore raw
In reply to Manish Lachwani
Quoted 2 lines · Manish Lachwani

The attached patch implements preempt_disable/preempt_enable around the SB1 DMA page operations. Please review ...

Why is it needed?

Also I think these:

- if [ "$CONFIG_SIBYTE_SB1250" = "y" ]; then
+ if [ "$CONFIG_SIBYTE_BOARD" = "y" ]; then

are unrelated and inaccurate. The devices are internal to the SoCs and not strictly board-dependent. How about:

+ if [ "$CONFIG_SIBYTE_SB1250" = "y" || "$CONFIG_SIBYTE_BCM112X" = "y"]; then

at the very least, or perhaps using reverse dependencies?

Maciej

author
#3 lore raw

Hi Maciej,

Quoted 14 lines · Maciej W. Rozycki
Quoted 2 lines · Manish Lachwani

The attached patch implements preempt_disable/preempt_enable around the SB1 DMA page operations. Please review ...

Why is it needed?

Also I think these:

(quoted diff) +1 -1
-   if [ "$CONFIG_SIBYTE_SB1250" = "y" ]; then
+   if [ "$CONFIG_SIBYTE_BOARD" = "y" ]; then
Quoted 3 lines

are unrelated and inaccurate. The devices are internal to the SoCs and not strictly board-dependent. How about:

+ if [ "$CONFIG_SIBYTE_SB1250" = "y" || "$CONFIG_SIBYTE_BCM112X" = "y"]; then

Actually, this makes more sense. I will send a new patch

Thanks
Manish Lachwani

Quoted 4 lines

at the very least, or perhaps using reverse dependencies?

Maciej