2005-04-25 07:36:28

by Sune Mølgaard

[permalink] [raw]
Subject: [PATCH] 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


Attachments:
linux-2.4.30-pico.patch (1.57 kB)

2005-04-26 15:43:12

by Alessandro Amici

[permalink] [raw]
Subject: Re: [PATCH] 2.4.30 PicoPower IRQ router


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/

2005-04-26 19:00:56

by Sune Mølgaard

[permalink] [raw]
Subject: Re: [PATCH] 2.4.30 PicoPower IRQ router

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

2005-04-27 11:29:18

by Domen Puncer

[permalink] [raw]
Subject: Re: [PATCH] 2.4.30 PicoPower IRQ router

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

2005-04-27 18:24:07

by Sune Mølgaard

[permalink] [raw]
Subject: Re: [PATCH] 2.4.30 PicoPower IRQ router

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 [email protected]
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

2005-04-27 20:58:40

by Jesper Juhl

[permalink] [raw]
Subject: Re: [PATCH] 2.4.30 PicoPower IRQ router


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 [email protected]

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


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


2005-04-28 10:37:15

by Sune Mølgaard

[permalink] [raw]
Subject: Re: [PATCH] 2.4.30 PicoPower IRQ router

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 <[email protected]>

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

> 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

2005-04-30 12:34:10

by Sune Mølgaard

[permalink] [raw]
Subject: Re: [PATCH] 2.4.30 PicoPower IRQ router

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

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