Skip to contentSkip to conversation

VSC_GSC_HSC bus debugging

PATCH In review Roel Kluin 2 participants 5 messages Oct 21, 2007

Subsystems (2)
Kernel janitors PA-RISC architecture
lore
Share & tools
lore

Roel Kluin

author
#0 lore raw

PATCH 1/1 VSC_GSC_HSC bus debugging

VSC_GSC_HSC bus debugging

Signed-off-by Roel Kluin <[email protected]>
drivers/parisc/Kconfig +5 -0
index 1d3b84b..6121170 100644
@@ -10,6 +10,11 @@ config GSC
 	  are not found in B1000, C3000, J5000, A500, L1000, N4000 and upwards.
 	  If in doubt, say "Y".
 
+config GSC_DEBUG
+	bool "VSC_GSC_HSC bus debugging"
+	depends on GSC
+	default n
+
 config HPPB
 	bool "HP-PB bus support"
 	depends on GSC
drivers/parisc/gsc.c +7 -8
index 1b3e3fd..0e6a614 100644
@@ -26,14 +26,13 @@
 #include <asm/io.h>
 
 #include "gsc.h"
+#ifdef GSC_DEBUG
+	#define DEBUG 1
+#endif
+#include <linux/kernel.h>
 
 #undef DEBUG
 
-#ifdef DEBUG
-#define DEBPRINTK printk
-#else
-#define DEBPRINTK(x,...)
-#endif
 
 int gsc_alloc_irq(struct gsc_irq *i)
 {

@@ -82,7 +81,7 @@ irqreturn_t gsc_asic_intr(int gsc_asic_irq, void *dev)
 	if (irr = 0)
 		return IRQ_NONE;
 
-	DEBPRINTK("%s intr, mask=0x%x\n", gsc_asic->name, irr);
+	printk("%s intr, mask=0x%x\n", gsc_asic->name, irr);
 
 	do {
 		int local_irq = __ffs(irr);

@@ -112,7 +111,7 @@ static void gsc_asic_disable_irq(unsigned int irq)
 	int local_irq = gsc_find_local_irq(irq, irq_dev->global_irq, 32);
 	u32 imr;
 
-	DEBPRINTK(KERN_DEBUG "%s(%d) %s: IMR 0x%x\n", __FUNCTION__, irq,
+	printk(KERN_DEBUG "%s(%d) %s: IMR 0x%x\n", __FUNCTION__, irq,
 			irq_dev->name, imr);
 
 	/* Disable the IRQ line by clearing the bit in the IMR */

@@ -127,7 +126,7 @@ static void gsc_asic_enable_irq(unsigned int irq)
 	int local_irq = gsc_find_local_irq(irq, irq_dev->global_irq, 32);
 	u32 imr;
 
-	DEBPRINTK(KERN_DEBUG "%s(%d) %s: IMR 0x%x\n", __FUNCTION__, irq,
+	printk(KERN_DEBUG "%s(%d) %s: IMR 0x%x\n", __FUNCTION__, irq,
 			irq_dev->name, imr);
 
 	/* Enable the IRQ line by setting the bit in the IMR */
#1 lore raw

drivers/parisc/Kconfig config GSC · line 14

Quoted 2 lines · Roel Kluin
drivers/parisc/Kconfig +2 -0
+config GSC_DEBUG
+	bool "VSC_GSC_HSC bus debugging"

What an ugly prompt string ...

drivers/parisc/Kconfig config GSC · line 16

Quoted 2 lines
drivers/parisc/Kconfig +2 -0
+	depends on GSC
+	default n

default is 'n', by default. No need to specify it.

drivers/parisc/gsc.c line 31

Quoted 3 lines
drivers/parisc/gsc.c +3 -0
+#ifdef GSC_DEBUG
+	#define DEBUG 1
+#endif

Why the indent?

drivers/parisc/gsc.c irqreturn_t gsc_asic_intr(int gsc_asic_irq, void *dev) · line 84

Quoted 2 lines
drivers/parisc/gsc.c +1 -1
-	DEBPRINTK("%s intr, mask=0x%x\n", gsc_asic->name, irr);
+	printk("%s intr, mask=0x%x\n", gsc_asic->name, irr);

Presumably you meant pr_debug?

Signature · 4 lines
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."
author
#2 lore raw
Quoted 5 lines · Matthew Wilcox

Review

Quoted 2 lines · Roel Kluin
(quoted diff) +2 -0
+config GSC_DEBUG
+	bool "VSC_GSC_HSC bus debugging"

What an ugly prompt string ...

I wrote a script to make these changes, but apparently it's still not perfect I should have checked it more. I will make changes and resend previously sent patches. Below is the patch previously called [PATCH] VSC_GSC_HSC bus debugging, with corrections.

I used "VSC/GSC/HSC bus support" from "config GSC" in the Kconfig. the slashes were translated to underscores because they previously gave a bug. it's fixed now, but I think the name "VSC/GSC/HSC bus debugging" is more appropriate here.

Quoted 4 lines

Review

Quoted 2 lines
(quoted diff) +2 -0
+	depends on GSC
+	default n

default is 'n', by default. No need to specify it.

ok fixed that

Quoted 5 lines

Review

Quoted 3 lines
(quoted diff) +3 -0
+#ifdef GSC_DEBUG
+	#define DEBUG 1
+#endif

Why the indent?

I thought it was more clear. But have that changed now as well.

Quoted 4 lines

Review

Quoted 2 lines
(quoted diff) +1 -1
-	DEBPRINTK("%s intr, mask=0x%x\n", gsc_asic->name, irr);
+	printk("%s intr, mask=0x%x\n", gsc_asic->name, irr);

Presumably you meant pr_debug?

yes.

Thanks for your comments. How is the one below instead? It is a replacement for [PATCH] VSC_GSC_HSC bus debugging.

Now there's also a change in some printk's and the GSC_DEBUG option has a place lower in the Kconfig file. I also added a Kconfig help description.

---
Kconfig GSC bus debugging and more elaborate printks

Signed-off-by Roel Kluin <[email protected]>
drivers/parisc/Kconfig +6 -0
index 1d3b84b..8959141 100644
@@ -54,6 +54,12 @@ config EISA
 	  supports both the Mongoose & Wax EISA adapters.  It is sadly
 	  incomplete and lacks support for card-to-host DMA.
 
+config GSC_DEBUG
+	bool "VSC/GSC/HSC bus debugging"
+	depends on GSC
+	help
+	  Say Y here to enable debugging for VSC, GSC and HSC busses.
+
 source "drivers/eisa/Kconfig"
 
 config ISA
drivers/parisc/gsc.c +9 -11
index 1b3e3fd..8805abb 100644
@@ -27,19 +27,17 @@
 
 #include "gsc.h"
 
-#undef DEBUG
-
-#ifdef DEBUG
-#define DEBPRINTK printk
-#else
-#define DEBPRINTK(x,...)
+#ifdef GSC_DEBUG
+#define DEBUG 1
 #endif
+#include <linux/kernel.h>
+
 
 int gsc_alloc_irq(struct gsc_irq *i)
 {
 	int irq = txn_alloc_irq(GSC_EIM_WIDTH);
 	if (irq < 0) {
-		printk("cannot get irq\n");
+		printk("GSC: cannot get irq\n");
 		return irq;
 	}
 

@@ -58,7 +56,7 @@ int gsc_claim_irq(struct gsc_irq *i, int irq)
 
 	irq = txn_claim_irq(irq);
 	if (irq < 0) {
-		printk("cannot claim irq %d\n", c);
+		printk("GSC: cannot claim irq %d\n", c);
 		return irq;
 	}
 

@@ -82,7 +80,7 @@ irqreturn_t gsc_asic_intr(int gsc_asic_irq, void *dev)
 	if (irr == 0)
 		return IRQ_NONE;
 
-	DEBPRINTK("%s intr, mask=0x%x\n", gsc_asic->name, irr);
+	pr_debug("%s intr, mask=0x%x\n", gsc_asic->name, irr);
 
 	do {
 		int local_irq = __ffs(irr);

@@ -112,7 +110,7 @@ static void gsc_asic_disable_irq(unsigned int irq)
 	int local_irq = gsc_find_local_irq(irq, irq_dev->global_irq, 32);
 	u32 imr;
 
-	DEBPRINTK(KERN_DEBUG "%s(%d) %s: IMR 0x%x\n", __FUNCTION__, irq,
+	pr_debug("%s(%d) %s: IMR 0x%x\n", __FUNCTION__, irq,
 			irq_dev->name, imr);
 
 	/* Disable the IRQ line by clearing the bit in the IMR */

@@ -127,7 +125,7 @@ static void gsc_asic_enable_irq(unsigned int irq)
 	int local_irq = gsc_find_local_irq(irq, irq_dev->global_irq, 32);
 	u32 imr;
 
-	DEBPRINTK(KERN_DEBUG "%s(%d) %s: IMR 0x%x\n", __FUNCTION__, irq,
+	pr_debug("%s(%d) %s: IMR 0x%x\n", __FUNCTION__, irq,
 			irq_dev->name, imr);
 
 	/* Enable the IRQ line by setting the bit in the IMR */

_______________________________________________
parisc-linux mailing list
[email protected]
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

#3 lore raw
In reply to Roel Kluin

drivers/parisc/Kconfig config GSC · line 14

Quoted 2 lines · Roel Kluin
drivers/parisc/Kconfig +2 -0
+config GSC_DEBUG
+	bool "VSC_GSC_HSC bus debugging"

What an ugly prompt string ...

drivers/parisc/Kconfig config GSC · line 16

Quoted 2 lines
drivers/parisc/Kconfig +2 -0
+	depends on GSC
+	default n

default is 'n', by default. No need to specify it.

drivers/parisc/gsc.c line 31

Quoted 3 lines
drivers/parisc/gsc.c +3 -0
+#ifdef GSC_DEBUG
+	#define DEBUG 1
+#endif

Why the indent?

drivers/parisc/gsc.c irqreturn_t gsc_asic_intr(int gsc_asic_irq, void *dev) · line 84

Quoted 2 lines
drivers/parisc/gsc.c +1 -1
-	DEBPRINTK("%s intr, mask=0x%x\n", gsc_asic->name, irr);
+	printk("%s intr, mask=0x%x\n", gsc_asic->name, irr);

Presumably you meant pr_debug?

Signature · 8 lines
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."
_______________________________________________
parisc-linux mailing list
[email protected]
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux
author
#4 lore raw
In reply to Roel Kluin
Quoted 5 lines · Matthew Wilcox

Review

Quoted 2 lines · Roel Kluin
(quoted diff) +2 -0
+config GSC_DEBUG
+	bool "VSC_GSC_HSC bus debugging"

What an ugly prompt string ...

I wrote a script to make these changes, but apparently it's still not perfect I should have checked it more. I will make changes and resend previously sent patches. Below is the patch previously called [PATCH] VSC_GSC_HSC bus debugging, with corrections.

I used "VSC/GSC/HSC bus support" from "config GSC" in the Kconfig. the slashes were translated to underscores because they previously gave a bug. it's fixed now, but I think the name "VSC/GSC/HSC bus debugging" is more appropriate here.

Quoted 4 lines

Review

Quoted 2 lines
(quoted diff) +2 -0
+	depends on GSC
+	default n

default is 'n', by default. No need to specify it.

ok fixed that

Quoted 5 lines

Review

Quoted 3 lines
(quoted diff) +3 -0
+#ifdef GSC_DEBUG
+	#define DEBUG 1
+#endif

Why the indent?

I thought it was more clear. But have that changed now as well.

Quoted 4 lines

Review

Quoted 2 lines
(quoted diff) +1 -1
-	DEBPRINTK("%s intr, mask=0x%x\n", gsc_asic->name, irr);
+	printk("%s intr, mask=0x%x\n", gsc_asic->name, irr);

Presumably you meant pr_debug?

yes.

Thanks for your comments. How is the one below instead? It is a replacement for [PATCH] VSC_GSC_HSC bus debugging.

Now there's also a change in some printk's and the GSC_DEBUG option has a place lower in the Kconfig file. I also added a Kconfig help description.

---
Kconfig GSC bus debugging and more elaborate printks

Signed-off-by Roel Kluin <[email protected]>
drivers/parisc/Kconfig +6 -0
index 1d3b84b..8959141 100644
@@ -54,6 +54,12 @@ config EISA
 	  supports both the Mongoose & Wax EISA adapters.  It is sadly
 	  incomplete and lacks support for card-to-host DMA.
 
+config GSC_DEBUG
+	bool "VSC/GSC/HSC bus debugging"
+	depends on GSC
+	help
+	  Say Y here to enable debugging for VSC, GSC and HSC busses.
+
 source "drivers/eisa/Kconfig"
 
 config ISA
drivers/parisc/gsc.c +9 -11
index 1b3e3fd..8805abb 100644
@@ -27,19 +27,17 @@
 
 #include "gsc.h"
 
-#undef DEBUG
-
-#ifdef DEBUG
-#define DEBPRINTK printk
-#else
-#define DEBPRINTK(x,...)
+#ifdef GSC_DEBUG
+#define DEBUG 1
 #endif
+#include <linux/kernel.h>
+
 
 int gsc_alloc_irq(struct gsc_irq *i)
 {
 	int irq = txn_alloc_irq(GSC_EIM_WIDTH);
 	if (irq < 0) {
-		printk("cannot get irq\n");
+		printk("GSC: cannot get irq\n");
 		return irq;
 	}
 

@@ -58,7 +56,7 @@ int gsc_claim_irq(struct gsc_irq *i, int irq)
 
 	irq = txn_claim_irq(irq);
 	if (irq < 0) {
-		printk("cannot claim irq %d\n", c);
+		printk("GSC: cannot claim irq %d\n", c);
 		return irq;
 	}
 

@@ -82,7 +80,7 @@ irqreturn_t gsc_asic_intr(int gsc_asic_irq, void *dev)
 	if (irr = 0)
 		return IRQ_NONE;
 
-	DEBPRINTK("%s intr, mask=0x%x\n", gsc_asic->name, irr);
+	pr_debug("%s intr, mask=0x%x\n", gsc_asic->name, irr);
 
 	do {
 		int local_irq = __ffs(irr);

@@ -112,7 +110,7 @@ static void gsc_asic_disable_irq(unsigned int irq)
 	int local_irq = gsc_find_local_irq(irq, irq_dev->global_irq, 32);
 	u32 imr;
 
-	DEBPRINTK(KERN_DEBUG "%s(%d) %s: IMR 0x%x\n", __FUNCTION__, irq,
+	pr_debug("%s(%d) %s: IMR 0x%x\n", __FUNCTION__, irq,
 			irq_dev->name, imr);
 
 	/* Disable the IRQ line by clearing the bit in the IMR */

@@ -127,7 +125,7 @@ static void gsc_asic_enable_irq(unsigned int irq)
 	int local_irq = gsc_find_local_irq(irq, irq_dev->global_irq, 32);
 	u32 imr;
 
-	DEBPRINTK(KERN_DEBUG "%s(%d) %s: IMR 0x%x\n", __FUNCTION__, irq,
+	pr_debug("%s(%d) %s: IMR 0x%x\n", __FUNCTION__, irq,
 			irq_dev->name, imr);
 
 	/* Enable the IRQ line by setting the bit in the IMR */