[PATCH] 2.4.30 PicoPower IRQ router

[PATCH] 2.4.30 PicoPower IRQ router

[Sune Mølgaard]

[-- Attachment #1: Type: text/plain, Size: 263 bytes --]

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

[-- Attachment #2: linux-2.4.30-pico.patch --]
[-- Type: text/x-patch, Size: 1612 bytes --]

--- 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 },

Re: [PATCH] 2.4.30 PicoPower IRQ router

[Alessandro Amici]

Alle 09:36, lunedì 25 aprile 2005, Sune Mølgaard ha scritto:
> 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.

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

Re: [PATCH] 2.4.30 PicoPower IRQ router

[Sune Mølgaard]

Alessandro Amici wrote:
> 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.

> 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--

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

Re: [PATCH] 2.4.30 PicoPower IRQ router

[Domen Puncer]

On 26/04/05 21:00 +0200, Sune Mølgaard wrote:
> Alessandro Amici wrote:
> >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.
> 
> >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--

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

> 
> --- 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);
> +}

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

> +
> +/*
>   * 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:

Use/define some PCI_DEVICE_ID_

> +      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

> 
>  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_?

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


	Domen

Re: [PATCH] 2.4.30 PicoPower IRQ router

[Sune Mølgaard]

Domen Puncer wrote:
> 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 sune@molgaard.org
Indentation was as per my own standard. Fixed below.

> 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.

> return 0; missing

Thanks. Fixed below.
> 
> PCI_VENDOR_ID_?
> 

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

> 
> 	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
--- 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-27 19:58:14.391991544 +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_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
  }

  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--

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

Re: [PATCH] 2.4.30 PicoPower IRQ router

[Jesper Juhl]

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

On Wed, 27 Apr 2005, Sune Mølgaard wrote:

> Domen Puncer wrote:
> > 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 sune@molgaard.org

Proper form would be:  Signed-off-by: Sune Molgaard <sune@molgaard.org>


> +static int pirq_pico_set(struct pci_dev *router, struct pci_dev *dev, int
> pirq, int irq)
> +{
> +        outb(0x10+((pirq-1)>>1), 0x24);
   ^^^^^^^^
   Proper indentation depth (8 chars)

> +       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)


> +        switch(device)
> +       {

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

See Documentation/CodingStyle Chapter 2


-- 
Jesper Juhl

Re: [PATCH] 2.4.30 PicoPower IRQ router

[Sune Mølgaard]

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

Let's try :-)

> Proper form would be:  Signed-off-by: Sune Molgaard <sune@molgaard.org>

Signed-off-by: Sune Mølgaard <sune@molgaard.org>

> 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?

>>+        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
--- 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_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
  }

  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

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

Re: [PATCH] 2.4.30 PicoPower IRQ router

[Sune Mølgaard]

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 <sune@molgaard.org>

--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

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