2.4.30 PicoPower IRQ router

PATCH In review Sune Mølgaard 4 participants 8 messages Apr 25, 2005 – Apr 30, 2005 PCI

Share & tools

Sune Mølgaard

author

2005-04-25 07:36 UTC #0 lore raw

PATCH 1/1 2.4.30 PicoPower IRQ router

The following patch fixes Picopower PT86C523 (found in e.g. Dell Latitude XPI P150CD and, so I've heard, IBM Thinkpad 760 series) getting irq 0, and thus not working with yenta. Patch is made from recommendations from David Hinds.

Best regards,

Sune Mølgaard

arch/i386/kernel/pci-irq.c +37 -0

@@ -157,6 +157,25 @@
 }
 
 /*
+ * PicoPower PT86C523
+ */
+
+static int pirq_pico_get(struct pci_dev *router, struct pci_dev *dev, int pirq)
+{
+  outb(0x10+((pirq-1)>>1), 0x24);
+  return ((pirq-1)&1) ? (inb(0x26)>>4) : (inb(0x26)&0xf);
+}
+
+static int pirq_pico_set(struct pci_dev *router, struct pci_dev *dev, int pirq, int irq) 
+{ 
+  outb(0x10+((pirq-1)>>1), 0x24);
+  unsigned int x;
+  x = inb(0x26); 
+  x = ((pirq-1)&1) ? ((x&0x0f)|(irq<<4)) : ((x&0xf0)|(irq)); 
+  outb(x,0x26); 
+} 
+
+/*
  * ALI pirq entries are damn ugly, and completely undocumented.
  * This has been figured out from pirq tables, and it's not a pretty
  * picture.

@@ -609,6 +628,23 @@
 
 #endif
 
+static __init int pico_router_probe(struct irq_router *r, struct pci_dev *router, u16 device)
+{
+  switch(device)
+  {
+    case 0x0002:
+      r->name = "PicoPower PT86C523";
+      r->get = pirq_pico_get;
+      r->set = pirq_pico_set;
+      return 1;
+
+    case 0x8002:
+      r->name = "PicoPower PT86C523 rev. BB+";
+      r->get = pirq_pico_get;
+      r->set = pirq_pico_set;
+      return 1;
+  }
+}
 
 static __init int intel_router_probe(struct irq_router *r, struct pci_dev *router, u16 device)
 {

@@ -814,6 +850,7 @@
 }
 		
 static __initdata struct irq_router_handler pirq_routers[] = {
+        { 0x1066, pico_router_probe },
 	{ PCI_VENDOR_ID_INTEL, intel_router_probe },
 	{ PCI_VENDOR_ID_AL, ali_router_probe },
 	{ PCI_VENDOR_ID_ITE, ite_router_probe },

Alessandro Amici

2005-04-26 15:43 UTC #1 lore raw

Alle 09:36, lunedì 25 aprile 2005, Sune Mølgaard ha scritto:

Quoted 1 line

Patch is made from recommendations from David Hinds.

just in case you didn't notice: your patch is empty :)

and try to gather info on someone actually in charge of the subsystem you are modifying and CC him. random patches on l-k may not get the needed attention.

Signature · 1 line

B-Open Solutions srl - http://www.bopen.it/

Sune Mølgaard

author

2005-04-26 19:00 UTC #2 lore raw

Quoted 1 line · Alessandro Amici

just in case you didn't notice: your patch is empty :)

How so? I see it fine in the mail that came back to me, but ok. I'll repost below.

Quoted 2 lines

and try to gather info on someone actually in charge of the subsystem you are modifying and CC him. random patches on l-k may not get the needed attention.

I thought of that and forwarded to Martin Mares, but thank you for the tip

Best regards,

Sune

--Start patch--

--- linux-2.4.30/arch/i386/kernel/pci-irq.c 2005-04-04
03:42:19.000000000 +0200
+++ linux/arch/i386/kernel/pci-irq.c 2005-04-25 08:43:02.501678464 +0200
@@ -157,6 +157,25 @@
}

/*
+ * PicoPower PT86C523
+ */
+
+static int pirq_pico_get(struct pci_dev *router, struct pci_dev *dev, int pirq)
+{
+ outb(0x10+((pirq-1)>>1), 0x24);
+ return ((pirq-1)&1) ? (inb(0x26)>>4) : (inb(0x26)&0xf); +}
+
+static int pirq_pico_set(struct pci_dev *router, struct pci_dev *dev, int pirq, int irq)
+{

+  outb(0x10+((pirq-1)>>1), 0x24);
+  unsigned int x;
+  x = inb(0x26);
+  x = ((pirq-1)&1) ? ((x&0x0f)|(irq<<4)) : ((x&0xf0)|(irq));
+  outb(x,0x26);

+}
+
+/*
* ALI pirq entries are damn ugly, and completely undocumented.
* This has been figured out from pirq tables, and it's not a pretty
* picture.
@@ -609,6 +628,23 @@

#endif

+static __init int pico_router_probe(struct irq_router *r, struct pci_dev *router, u16 device)
+{
+ switch(device)
+ {
+ case 0x0002:

+      r->name = "PicoPower PT86C523";
+      r->get = pirq_pico_get;
+      r->set = pirq_pico_set;
+      return 1;
+
+    case 0x8002:
+      r->name = "PicoPower PT86C523 rev. BB+";
+      r->get = pirq_pico_get;
+      r->set = pirq_pico_set;
+      return 1;

+ }
+}

static __init int intel_router_probe(struct irq_router *r, struct
pci_dev *router, u16 device)
{
@@ -814,6 +850,7 @@
}

static __initdata struct irq_router_handler pirq_routers[] = {
+ { 0x1066, pico_router_probe },
{ PCI_VENDOR_ID_INTEL, intel_router_probe }, { PCI_VENDOR_ID_AL, ali_router_probe },
{ PCI_VENDOR_ID_ITE, ite_router_probe },

--End patch--

Signature · 2 lines

The best way to accelerate a Macintosh is at 9.8m sec sec.
- Marcus Dolengo

Domen Puncer

2005-04-27 11:29 UTC #3 lore raw

Quoted 11 lines · Sune Mølgaard

Quoted 1 line · Alessandro Amici
just in case you didn't notice: your patch is empty :)
How so? I see it fine in the mail that came back to me, but ok. I'll repost below.
Quoted 4 lines
and try to gather info on someone actually in charge of the subsystem you are modifying and CC him. random patches on l-k may not get the needed attention.

Quoted 7 lines

I thought of that and forwarded to Martin Mares, but thank you for the tip
Best regards,
Sune
--Start patch--

Signed-off-by? And weird indentification, try to use tabs.

Quoted 17 lines

--- linux-2.4.30/arch/i386/kernel/pci-irq.c 2005-04-04
03:42:19.000000000 +0200
+++ linux/arch/i386/kernel/pci-irq.c 2005-04-25 08:43:02.501678464 +0200
arch/i386/kernel/pci-irq.c +8 -0
@@ -157,6 +157,25 @@
 }
 
 /*
+ * PicoPower PT86C523
+ */
+
+static int pirq_pico_get(struct pci_dev *router, struct pci_dev *dev,
 int pirq)
+{
+  outb(0x10+((pirq-1)>>1), 0x24);
+  return ((pirq-1)&1) ? (inb(0x26)>>4) : (inb(0x26)&0xf);
+}

arch/i386/kernel/pci-irq.c line 178

Quoted 10 lines

arch/i386/kernel/pci-irq.c +9 -0

+
+static int pirq_pico_set(struct pci_dev *router, struct pci_dev *dev,
 int pirq, int irq)
+{
+  outb(0x10+((pirq-1)>>1), 0x24);
+  unsigned int x;
+  x = inb(0x26);
+  x = ((pirq-1)&1) ? ((x&0x0f)|(irq<<4)) : ((x&0xf0)|(irq));
+  outb(x,0x26);
+}

I really don't know about this, but existing code (2.6.x) uses {read,write}_config_nybble which looks suspiciously similar.

Quoted 6 lines

arch/i386/kernel/pci-irq.c +2 -0
+
+/*
  * ALI pirq entries are damn ugly, and completely undocumented.
* This has been figured out from pirq tables, and it's not a pretty
* picture.

arch/i386/kernel/pci-irq.c line 636

Quoted 9 lines

arch/i386/kernel/pci-irq.c +5 -0

@@ -609,6 +628,23 @@
 
 #endif
 
+static __init int pico_router_probe(struct irq_router *r, struct
 pci_dev *router, u16 device)
+{
+  switch(device)
+  {
+    case 0x0002:

Use/define some PCI_DEVICE_ID_

arch/i386/kernel/pci-irq.c line 647

Quoted 12 lines

arch/i386/kernel/pci-irq.c +12 -0

+      r->name = "PicoPower PT86C523";
+      r->get = pirq_pico_get;
+      r->set = pirq_pico_set;
+      return 1;
+
+    case 0x8002:
+      r->name = "PicoPower PT86C523 rev. BB+";
+      r->get = pirq_pico_get;
+      r->set = pirq_pico_set;
+      return 1;
+  }
+}

return 0; missing

arch/i386/kernel/pci-irq.c line 853

Quoted 9 lines

(quoted diff) +0 -0

 static __init int intel_router_probe(struct irq_router *r, struct
 pci_dev *router, u16 device)

{

arch/i386/kernel/pci-irq.c +1 -0

@@ -814,6 +850,7 @@
 }
 
 static __initdata struct irq_router_handler pirq_routers[] = {
+        { 0x1066, pico_router_probe },

PCI_VENDOR_ID_?

arch/i386/kernel/pci-irq.c line 856

Quoted 4 lines

arch/i386/kernel/pci-irq.c +0 -0

 	{ PCI_VENDOR_ID_INTEL, intel_router_probe },
 	{ PCI_VENDOR_ID_AL, ali_router_probe },
 	{ PCI_VENDOR_ID_ITE, ite_router_probe },

Domen

Sune Mølgaard

author

2005-04-27 18:24 UTC #4 lore raw

Quoted 4 lines · Domen Puncer

On 26/04/05 21:00 +0200, Sune Mølgaard wrote:
Signed-off-by? And weird indentification, try to use tabs.

Signed-off-by Sune Molgaard [email protected] Indentation was as per my own standard. Fixed below.

Quoted 2 lines

I really don't know about this, but existing code (2.6.x) uses {read,write}_config_nybble which looks suspiciously similar.

As stated, David Hinds recommended this, and it works. My insight is limited.

Quoted 1 line

return 0; missing

Thanks. Fixed below.

Quoted 3 lines

PCI_VENDOR_ID_?

Sorry. I was lazy and hardcoded it in. Fixed below.

Quoted 2 lines

Domen

Thanks for the input. Below follows an updated patch.

/Sune

--Begin patch--
diff -Npru linux-2.4.30/arch/i386/kernel/pci-irq.c \
linux/arch/i386/kernel/pci-irq.c

arch/i386/kernel/pci-irq.c 2005-04-27 19:58:14.391991544 +0200 renamed from arch/i386/kernel/pci-irq.c 2005-04-04 03:42:19.000000000 +0200 +37 -0

@@ -157,6 +157,25 @@ static void write_config_nybble(struct p
 }
 
 /*
+ * PicoPower PT86C523
+ */
+
+static int pirq_pico_get(struct pci_dev *router, struct pci_dev *dev, int pirq)
+{
+        outb(0x10+((pirq-1)>>1), 0x24);
+       return ((pirq-1)&1) ? (inb(0x26)>>4) : (inb(0x26)&0xf);
+}
+
+static int pirq_pico_set(struct pci_dev *router, struct pci_dev *dev, int pirq, int irq)
+{
+        outb(0x10+((pirq-1)>>1), 0x24);
+       unsigned int x;
+       x = inb(0x26);
+       x = ((pirq-1)&1) ? ((x&0x0f)|(irq<<4)) : ((x&0xf0)|(irq));
+       outb(x,0x26);
+}
+
+/*
  * ALI pirq entries are damn ugly, and completely undocumented.
  * This has been figured out from pirq tables, and it's not a pretty
  * picture.

@@ -609,6 +628,24 @@ static int pirq_bios_set(struct pci_dev
 
 #endif
 
+static __init int pico_router_probe(struct irq_router *r, struct pci_dev *router, u16 device)
+{
+        switch(device)
+       {
+               case PCI_DEVICE_PICOPOWER_PT86C523:
+                       r->name = "PicoPower PT86C523";
+                       r->get = pirq_pico_get;
+                       r->set = pirq_pico_set;
+                       return 1;
+
+               case PCI_DEVICE_PICOPOWER_PT86C523BBP:
+                       r->name = "PicoPower PT86C523 rev. BB+";
+                       r->get = pirq_pico_get;
+                       r->set = pirq_pico_set;
+                       return 1;
+       }
+       return 0;
+}
 
 static __init int intel_router_probe(struct irq_router *r, struct

pci_dev *router, u16 device)
{
@@ -814,6 +851,7 @@ static __init int amd_router_probe(struc
}

include/linux/pci_ids.h 2005-04-27 20:04:12.195597128 +0200 renamed from include/linux/pci_ids.h 2005-04-04 03:42:20.000000000 +0200 +4 -0

@@ -119,6 +119,10 @@
 
 /* Vendors and devices.  Sort key: vendor first, device next. */
 
+#define PCI_VENDOR_ID_PICOPOWER             0x104c
+#define PCI_DEVICE_ID_PICOPOWER_PT86C523    0x0002
+#define PCI_DEVICE_ID_PICOPOWER_PT86C523BBP 0x8002
+
 #define PCI_VENDOR_ID_DYNALINK         0x0675
 #define PCI_DEVICE_ID_DYNALINK_IS64PH  0x1702

--End patch--

Signature · 2 lines

Thou shalt not commit adultery ... unless in the mood.
- W. C. Fields

Jesper Juhl

2005-04-27 20:58 UTC #5 lore raw

A few more style comments below. Nice cleanup compared to the first version though. Now let's get it perfect :-)

Quoted 7 lines · Sune Mølgaard

Quoted 4 lines · Domen Puncer
On 26/04/05 21:00 +0200, Sune Mølgaard wrote:
Signed-off-by? And weird indentification, try to use tabs.
Signed-off-by Sune Molgaard [email protected]

Proper form would be: Signed-off-by: Sune Molgaard <[email protected]>

Review

Quoted 4 lines

+static int pirq_pico_set(struct pci_dev *router, struct pci_dev *dev, int pirq, int irq)
(quoted diff) +2 -0
+{
+        outb(0x10+((pirq-1)>>1), 0x24);

^^^^^^^^
Proper indentation depth (8 chars)

Quoted 1 line

+ unsigned int x;

^^^^^^^
Only 7 char indentation.

This mismatch is found several places in the patch. Besides, the patch seems whitespace damaged - the lines seem to be indented with spaces, not tabs. Perhaps your email client mangled it?

(note: Chapter 1 of Documentation/CodingStyle describes proper
indentation)

arch/i386/kernel/pci-irq.c 2005-04-27 19:58:14.391991544 +0200 static int pirq_bios_set(struct pci_dev · line 634

Quoted 2 lines

arch/i386/kernel/pci-irq.c 2005-04-27 19:58:14.391991544 +0200 renamed from arch/i386/kernel/pci-irq.c 2005-04-04 03:42:19.000000000 +0200 +2 -0
+        switch(device)
+       {

The opening brace goes on the same line as the switch statement.

See Documentation/CodingStyle Chapter 2

Signature · 1 line

Jesper Juhl

Sune Mølgaard

author

2005-04-28 10:37 UTC #6 lore raw

Quoted 2 lines · Jesper Juhl

A few more style comments below. Nice cleanup compared to the first version though. Now let's get it perfect :-)

Let's try :-)

Quoted 1 line

Proper form would be: Signed-off-by: Sune Molgaard <[email protected]>

Signed-off-by Sune Mølgaard <[email protected]>

Quoted 2 lines

This mismatch is found several places in the patch. Besides, the patch seems whitespace damaged - the lines seem to be indented with spaces, not tabs. Perhaps your email client mangled it?

How do I tell?

Quoted 7 lines

Review
Quoted 2 lines
(quoted diff) +2 -0
+        switch(device)
+       {
The opening brace goes on the same line as the switch statement.
See Documentation/CodingStyle Chapter 2

The other switch statements in the file are bracketed like this. I think changing that would be up to that author. Correct me if I am wrong.

--Begin patch--
diff -Npru linux-2.4.30/arch/i386/kernel/pci-irq.c \
linux/arch/i386/kernel/pci-irq.c

arch/i386/kernel/pci-irq.c 2005-04-28 12:31:34.999771672 +0200 renamed from arch/i386/kernel/pci-irq.c 2005-04-04 03:42:19.000000000 +0200 +37 -0

@@ -157,6 +157,25 @@ static void write_config_nybble(struct p
 }
 
 /*
+ * PicoPower PT86C523
+ */
+
+static int pirq_pico_get(struct pci_dev *router, struct pci_dev *dev, int pirq)
+{
+       outb(0x10+((pirq-1)>>1), 0x24);
+       return ((pirq-1)&1) ? (inb(0x26)>>4) : (inb(0x26)&0xf);
+}
+
+static int pirq_pico_set(struct pci_dev *router, struct pci_dev *dev, int pirq, int irq)
+{
+       outb(0x10+((pirq-1)>>1), 0x24);
+       unsigned int x;
+       x = inb(0x26);
+       x = ((pirq-1)&1) ? ((x&0x0f)|(irq<<4)) : ((x&0xf0)|(irq));
+       outb(x,0x26);
+}
+
+/*
  * ALI pirq entries are damn ugly, and completely undocumented.
  * This has been figured out from pirq tables, and it's not a pretty
  * picture.

@@ -609,6 +628,24 @@ static int pirq_bios_set(struct pci_dev
 
 #endif
 
+static __init int pico_router_probe(struct irq_router *r, struct pci_dev *router, u16 device)
+{
+       switch(device)
+       {
+               case PCI_DEVICE_PICOPOWER_PT86C523:
+                       r->name = "PicoPower PT86C523";
+                       r->get = pirq_pico_get;
+                       r->set = pirq_pico_set;
+                       return 1;
+
+               case PCI_DEVICE_PICOPOWER_PT86C523BBP:
+                       r->name = "PicoPower PT86C523 rev. BB+";
+                       r->get = pirq_pico_get;
+                       r->set = pirq_pico_set;
+                       return 1;
+       }
+       return 0;
+}
 
 static __init int intel_router_probe(struct irq_router *r, struct

pci_dev *router, u16 device)
{
@@ -814,6 +851,7 @@ static __init int amd_router_probe(struc
}

include/linux/pci_ids.h 2005-04-27 20:04:12.195597128 +0200 renamed from include/linux/pci_ids.h 2005-04-04 03:42:20.000000000 +0200 +4 -0

@@ -119,6 +119,10 @@
 
 /* Vendors and devices.  Sort key: vendor first, device next. */
 
+#define PCI_VENDOR_ID_PICOPOWER             0x104c
+#define PCI_DEVICE_ID_PICOPOWER_PT86C523    0x0002
+#define PCI_DEVICE_ID_PICOPOWER_PT86C523BBP 0x8002
+
 #define PCI_VENDOR_ID_DYNALINK         0x0675
 #define PCI_DEVICE_ID_DYNALINK_IS64PH  0x1702

--End patch--

Best regards,

Sune Mølgaard

Signature · 2 lines

Now, suddenly, dynamic HTML makes the Web as interactive as my old Apple IIc.
- Peter Merholz

Sune Mølgaard

author

2005-04-30 12:34 UTC #7 lore raw

In reply to Jesper Juhl

One final but critical change. I accidentally used vendor ID for USR instead of Picopower in pci_ids.h. Plus I didn't refer to it properly in pci-irq.c

Signed-off-by Sune Mølgaard <[email protected]>

--Begin patch--
diff -Npru linux-2.4.30/arch/i386/kernel/pci-irq.c \
linux/arch/i386/kernel/pci-irq.c
--- linux-2.4.30/arch/i386/kernel/pci-irq.c 2005-04-04
03:42:19.000000000 +0200
+++ linux/arch/i386/kernel/pci-irq.c 2005-04-28 12:31:34.999771672 +0200
@@ -157,6 +157,25 @@ static void write_config_nybble(struct p
}

/*
+ * PicoPower PT86C523
+ */
+
+static int pirq_pico_get(struct pci_dev *router, struct pci_dev *dev, int pirq)
+{
+ outb(0x10+((pirq-1)>>1), 0x24);
+ return ((pirq-1)&1) ? (inb(0x26)>>4) : (inb(0x26)&0xf);
+}
+
+static int pirq_pico_set(struct pci_dev *router, struct pci_dev *dev, int pirq, int irq)
+{

+       outb(0x10+((pirq-1)>>1), 0x24);
+       unsigned int x;
+       x = inb(0x26);
+       x = ((pirq-1)&1) ? ((x&0x0f)|(irq<<4)) : ((x&0xf0)|(irq));
+       outb(x,0x26);

+}
+
+/*
* ALI pirq entries are damn ugly, and completely undocumented.
* This has been figured out from pirq tables, and it's not a pretty
* picture.
@@ -609,6 +628,24 @@ static int pirq_bios_set(struct pci_dev

#endif

+static __init int pico_router_probe(struct irq_router *r, struct pci_dev *router, u16 device)
+{
+ switch(device)
+ {
+ case PCI_DEVICE_ID_PICOPOWER_PT86C523:

+                       r->name = "PicoPower PT86C523";
+                       r->get = pirq_pico_get;
+                       r->set = pirq_pico_set;
+                       return 1;
+
+               case PCI_DEVICE_ID_PICOPOWER_PT86C523BBP:
+                       r->name = "PicoPower PT86C523 rev. BB+";
+                       r->get = pirq_pico_get;
+                       r->set = pirq_pico_set;
+                       return 1;

+ }
+ return 0;
+}

static __init int intel_router_probe(struct irq_router *r, struct
pci_dev *router, u16 device)
{
@@ -814,6 +851,7 @@ static __init int amd_router_probe(struc
}

static __initdata struct irq_router_handler pirq_routers[] = {
+ { PCI_VENDOR_ID_PICOPOWER, pico_router_probe },
{ PCI_VENDOR_ID_INTEL, intel_router_probe }, { PCI_VENDOR_ID_AL, ali_router_probe },
{ PCI_VENDOR_ID_ITE, ite_router_probe },
diff -Npru linux-2.4.30/include/linux/pci_ids.h \
linux/include/linux/pci_ids.h
--- linux-2.4.30/include/linux/pci_ids.h 2005-04-04
03:42:20.000000000 +0200
+++ linux/include/linux/pci_ids.h 2005-04-27 20:04:12.195597128 +0200
@@ -119,6 +119,10 @@

/* Vendors and devices. Sort key: vendor first, device next. */

+#define PCI_VENDOR_ID_PICOPOWER 0x104c
+#define PCI_DEVICE_ID_PICOPOWER_PT86C523 0x0002
+#define PCI_DEVICE_ID_PICOPOWER_PT86C523BBP 0x8002 +
#define PCI_VENDOR_ID_DYNALINK 0x0675
#define PCI_DEVICE_ID_DYNALINK_IS64PH 0x1702

--End patch--

Best regards,

Sune Mølgaard

Signature · 3 lines

Now, suddenly, dynamic HTML makes the Web as interactive as my old Apple
IIc.
- Peter Merholz