Re: [PATCH] [SERIAL] add TP560 data/fax/modem support
On Mon, Feb 07, 2005 at 12:39:42PM -0700, Bjorn Helgaas wrote: > Claim Topic TP560 data/fax/voice modem. This device reports as class 0x0780, > so we don't claim it by default: Applied, thanks. -- Russell King Linux kernel2.6 ARM Linux - http://www.arm.linux.org.uk/ maintainer of: 2.6 PCMCIA - http://pcmcia.arm.linux.org.uk/ 2.6 Serial core - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [SERIAL] add TP560 data/fax/modem support
On Tue, 2005-02-08 at 07:25 -0500, linux-os wrote: > On Mon, 7 Feb 2005, Bjorn Helgaas wrote: > > > On Mon, 2005-02-07 at 15:12 -0500, linux-os wrote: > >> I thought somebody promised to add a pci_route_irq(dev) or some > >> such so that the device didn't have to be enabled before > >> the IRQ was correct. > >> > >> I first reported this bad IRQ problem back in December of 2004. > >> Has the new function been added? > > > > That's a completely different problem. The point here is that > > the serial driver currently doesn't do anything with the TP560 > > (no pci_enable_device(), no pci_route_irq(), no nothing). Then > > when setserial comes along and force-feeds the driver with the > > IO and IRQ info, there's nothing at that point that does anything > > to enable the device or route its interrupt either. > > > > I did raise the idea of adding a pci_route_irq() interface, but > > to be honest, I was never convinced of its general usefulness. > > There have been two PCI drivers in the past three days where > users have reported that the IRQ was wrong. If you've got pointers to these two reports, please provide them. > One patch was > provided to enable the device before requesting the IRQ, this > being inherently dangerous. So, if you are the one who > re-wrote the PCI stuff so that the IRQ is wrong when the > device is claimed, then I think you have a duty to fix the > code that you have broken. Most drivers in the tree call pci_enable_device() before request_irq(). That was the case even before my changes. Dangerous? Maybe, but I didn't make it more so. > The correct way to fix the broken code is to make sure that > the IRQ, reported in the structure, is correct. Alternate > methods might be a pci_route_irq() function. Or perhaps, the IRQ could be routed in pci_setup_device(). But, by the principle of "if you don't use it, don't touch it", there *is* something to be said for the fact that the current code doesn't touch IRQ routing unless a driver actually claims the device. > The existing PCI code is broken. The fact that an invalid > IRQ is reported in the structure is PROOF that it is broken > and requires no further discussion. Then I'm sure your patch to fix it will be accepted without much dissent. > Many of us have to > use this stuff in a professional environment, we can't > enable devices without an interrupt handler in place > because it is not allowed in code that is subject to > peer review. We also can't use code when such problems > as invalid values in returned data are discovered. Nobody's forcing you to use "this stuff." > Two months ago I discovered this problem and reported it, hoping > that the person who broke existing code would fix it. It has > not been fixed. The fact is, yours is the only report of an issue with the structure of the new code. And it apparently only concerns an out-of-tree driver. Still, I agree that it might be good to do something about it. But you haven't provided pointers to the hardware specs or driver involved. So I'm not going to go too much out of my way to fix it. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [SERIAL] add TP560 data/fax/modem support
On Mon, 7 Feb 2005, Bjorn Helgaas wrote: On Mon, 2005-02-07 at 15:12 -0500, linux-os wrote: I thought somebody promised to add a pci_route_irq(dev) or some such so that the device didn't have to be enabled before the IRQ was correct. I first reported this bad IRQ problem back in December of 2004. Has the new function been added? That's a completely different problem. The point here is that the serial driver currently doesn't do anything with the TP560 (no pci_enable_device(), no pci_route_irq(), no nothing). Then when setserial comes along and force-feeds the driver with the IO and IRQ info, there's nothing at that point that does anything to enable the device or route its interrupt either. I did raise the idea of adding a pci_route_irq() interface, but to be honest, I was never convinced of its general usefulness. There have been two PCI drivers in the past three days where users have reported that the IRQ was wrong. One patch was provided to enable the device before requesting the IRQ, this being inherently dangerous. So, if you are the one who re-wrote the PCI stuff so that the IRQ is wrong when the device is claimed, then I think you have a duty to fix the code that you have broken. The correct way to fix the broken code is to make sure that the IRQ, reported in the structure, is correct. Alternate methods might be a pci_route_irq() function. The existing PCI code is broken. The fact that an invalid IRQ is reported in the structure is PROOF that it is broken and requires no further discussion. Many of us have to use this stuff in a professional environment, we can't enable devices without an interrupt handler in place because it is not allowed in code that is subject to peer review. We also can't use code when such problems as invalid values in returned data are discovered. Two months ago I discovered this problem and reported it, hoping that the person who broke existing code would fix it. It has not been fixed. Cheers, Dick Johnson Penguin : Linux version 2.6.10 on an i686 machine (5537.79 BogoMips). Notice : All mail here is now cached for review by Dictator Bush. 98.36% of all statistics are fiction. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [SERIAL] add TP560 data/fax/modem support
On Mon, 2005-02-07 at 15:12 -0500, linux-os wrote: > I thought somebody promised to add a pci_route_irq(dev) or some > such so that the device didn't have to be enabled before > the IRQ was correct. > > I first reported this bad IRQ problem back in December of 2004. > Has the new function been added? That's a completely different problem. The point here is that the serial driver currently doesn't do anything with the TP560 (no pci_enable_device(), no pci_route_irq(), no nothing). Then when setserial comes along and force-feeds the driver with the IO and IRQ info, there's nothing at that point that does anything to enable the device or route its interrupt either. I did raise the idea of adding a pci_route_irq() interface, but to be honest, I was never convinced of its general usefulness. I haven't heard of any driver in the tree that requires it, so it's not clear that it would be accepted even if I (or you) wrote it. I think you mentioned a specific PCI interface chip that was susceptible to the problem; is there a public reference that would help explicate the situation? - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] [SERIAL] add TP560 data/fax/modem support
I thought somebody promised to add a pci_route_irq(dev) or some
such so that the device didn't have to be enabled before
the IRQ was correct.
I first reported this bad IRQ problem back in December of 2004.
Has the new function been added?
On Mon, 7 Feb 2005, Bjorn Helgaas wrote:
Claim Topic TP560 data/fax/voice modem. This device reports as class 0x0780,
so we don't claim it by default:
00:0d.0 Class 0780: 151f:
Subsystem: 151f:
Interrupt: pin A routed to IRQ 11
Region 0: I/O ports at a400 [size=8]
00: 1f 15 00 00 01 00 00 02 00 00 80 07 00 00 00 00
10: 01 a4 00 00 00 00 00 00 00 00 00 00 00 00 00 00
20: 00 00 00 00 00 00 00 00 00 00 00 00 1f 15 00 00
30: 00 00 00 00 00 00 00 00 00 00 00 00 0b 01 00 00
Some rc.serial scripts extract IRQ and I/O port information from
/proc/pci and stuff it into an unused port using setserial. That
doesn't work reliably anymore because pci_enable_device() is never
called, so the IRQ may not be enabled.
Thanks to Evan Clarke for reporting and helping debug this problem.
Signed-off-by: Bjorn Helgaas <[EMAIL PROTECTED]>
= drivers/serial/8250_pci.c 1.48 vs edited =
--- 1.48/drivers/serial/8250_pci.c 2004-11-21 23:42:29 -07:00
+++ edited/drivers/serial/8250_pci.c2005-02-07 12:00:32 -07:00
@@ -2212,6 +2212,13 @@
0, pbn_exar_XR17C158 },
/*
+* Topic TP560 Data/Fax/Voice 56k modem (reported by Evan Clarke)
+*/
+ { PCI_VENDOR_ID_TOPIC, PCI_DEVICE_ID_TOPIC_TP560,
+ PCI_ANY_ID, PCI_ANY_ID, 0, 0,
+ pbn_b0_1_115200 },
+
+ /*
* These entries match devices with class COMMUNICATION_SERIAL,
* COMMUNICATION_MODEM or COMMUNICATION_MULTISERIAL
*/
= include/linux/pci_ids.h 1.200 vs edited =
--- 1.200/include/linux/pci_ids.h 2005-01-30 23:33:43 -07:00
+++ edited/include/linux/pci_ids.h 2005-02-07 11:56:14 -07:00
@@ -1972,6 +1972,9 @@
#define PCI_DEVICE_ID_BCM4401 0x4401
#define PCI_DEVICE_ID_BCM4401B0 0x4402
+#define PCI_VENDOR_ID_TOPIC0x151f
+#define PCI_DEVICE_ID_TOPIC_TP560 0x
+
#define PCI_VENDOR_ID_ENE 0x1524
#define PCI_DEVICE_ID_ENE_1211 0x1211
#define PCI_DEVICE_ID_ENE_1225 0x1225
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Cheers,
Dick Johnson
Penguin : Linux version 2.6.10 on an i686 machine (5537.79 BogoMips).
Notice : All mail here is now cached for review by Dictator Bush.
98.36% of all statistics are fiction.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] [SERIAL] add TP560 data/fax/modem support
Claim Topic TP560 data/fax/voice modem. This device reports as class 0x0780,
so we don't claim it by default:
00:0d.0 Class 0780: 151f:
Subsystem: 151f:
Interrupt: pin A routed to IRQ 11
Region 0: I/O ports at a400 [size=8]
00: 1f 15 00 00 01 00 00 02 00 00 80 07 00 00 00 00
10: 01 a4 00 00 00 00 00 00 00 00 00 00 00 00 00 00
20: 00 00 00 00 00 00 00 00 00 00 00 00 1f 15 00 00
30: 00 00 00 00 00 00 00 00 00 00 00 00 0b 01 00 00
Some rc.serial scripts extract IRQ and I/O port information from
/proc/pci and stuff it into an unused port using setserial. That
doesn't work reliably anymore because pci_enable_device() is never
called, so the IRQ may not be enabled.
Thanks to Evan Clarke for reporting and helping debug this problem.
Signed-off-by: Bjorn Helgaas <[EMAIL PROTECTED]>
= drivers/serial/8250_pci.c 1.48 vs edited =
--- 1.48/drivers/serial/8250_pci.c 2004-11-21 23:42:29 -07:00
+++ edited/drivers/serial/8250_pci.c2005-02-07 12:00:32 -07:00
@@ -2212,6 +2212,13 @@
0, pbn_exar_XR17C158 },
/*
+* Topic TP560 Data/Fax/Voice 56k modem (reported by Evan Clarke)
+*/
+ { PCI_VENDOR_ID_TOPIC, PCI_DEVICE_ID_TOPIC_TP560,
+ PCI_ANY_ID, PCI_ANY_ID, 0, 0,
+ pbn_b0_1_115200 },
+
+ /*
* These entries match devices with class COMMUNICATION_SERIAL,
* COMMUNICATION_MODEM or COMMUNICATION_MULTISERIAL
*/
= include/linux/pci_ids.h 1.200 vs edited =
--- 1.200/include/linux/pci_ids.h 2005-01-30 23:33:43 -07:00
+++ edited/include/linux/pci_ids.h 2005-02-07 11:56:14 -07:00
@@ -1972,6 +1972,9 @@
#define PCI_DEVICE_ID_BCM4401 0x4401
#define PCI_DEVICE_ID_BCM4401B00x4402
+#define PCI_VENDOR_ID_TOPIC0x151f
+#define PCI_DEVICE_ID_TOPIC_TP560 0x
+
#define PCI_VENDOR_ID_ENE 0x1524
#define PCI_DEVICE_ID_ENE_1211 0x1211
#define PCI_DEVICE_ID_ENE_1225 0x1225
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
