Bug 578616 - ITE887x serial port card not working
Summary: ITE887x serial port card not working
Keywords:
Status: CLOSED INSUFFICIENT_DATA
Alias: None
Product: Fedora
Classification: Fedora
Component: kernel
Version: 19
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Kernel Maintainer List
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2010-03-31 19:59 UTC by Joe Krahn
Modified: 2013-04-23 17:26 UTC (History)
8 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2013-04-23 17:26:21 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)
Datasheet for the IT8872F chip manufactured by ITE (1.01 MB, application/octet-stream)
2010-04-16 13:08 UTC, Niels de Vos
no flags Details

Description Joe Krahn 2010-03-31 19:59:05 UTC
Description of problem:
I have an ITE 2-serial no-parallel I/O card that is not working with the current kernel ITE887x support (initially posted as a patch on LKML aroung Jun 2007).

Version-Release number of selected component (if applicable):
2.6.32.9-70.fc12.i686

How reproducible:
always

Steps to Reproduce:
1. setserial -a /dev/ttyS2
2.
3.
  
Actual results:
output from setserial is followed by a broadcast kernel message:
Message from syslogd@XXX at Mar 30 19:04:09 ...
 kernel:Disabling IRQ #17

/var/log/messages says:
  irq 17: nobody cared (try booting with the "irqpoll" option)

Expected results:
IRQ 17 should be handled by the serial port driver

Additional info:
I don't think this card ever worked with a stock kernel, but it was working from a custom kernel with an ITE driver patch posted to LKML in 2003. The IT887x chip supports multiple configurations. The problem may be that this card has no parallel ports, and the parport driver side is not setting the configuration correctly. If I blacklist the parport_pc module, the serial port shows up, the IRQ error goes away, but the serial ports don't work.

The current kernel code was posted on LKML here:
http://lkml.indiana.edu/hypermail/linux/kernel/0708.1/1592.html

The kernel seems to configure the device correctly. The question is why the kernel serial driver is not handling the IRQs. However, it shows up twice in /proc/ioports:
02a0-02bf : ite887x
02c0-02df : it887x

The 'it887x' entry is for the parallel port driver, even though this card has no parallel port. (Note: IT887x is the chip ID, ITE887x is from the company abbreviation ITE.)

# lspci -vv -s 02:08.0
02:08.0 Serial controller: Integrated Technology Express, Inc. IT8874F
PCI Dual Serial Port Controller (rev 01) (prog-if 02 [16550])
        Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop-
ParErr- Stepping- SERR+ FastB2B- DisINTx-
        Status: Cap- 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort-
<TAbort- <MAbort- >SERR- <PERR- INTx-
        Latency: 0
        Interrupt: pin A routed to IRQ 17
        Region 0: I/O ports at ec00 [size=256]
        Region 1: I/O ports at e800 [size=256]
        Region 2: I/O ports at e400 [size=256]
        Region 3: I/O ports at e000 [size=256]
        Region 4: I/O ports at dc00 [size=256]
        Region 5: I/O ports at d800 [size=256]
        Kernel driver in use: serial
        Kernel modules: parport_pc


# setserial -a /dev/ttyS2
/dev/ttyS2, Line 2, UART: 16550A, Port: 0xe800, IRQ: 17
        Baud_base: 115200, close_delay: 50, divisor: 0
        closing_wait: 3000
        Flags: spd_normal skip_test

Comment 1 Joe Krahn 2010-04-01 20:30:14 UTC
There seems to be some conflicts with ITE887x code in drivers/serial/8250_pci.c and drivers/parport/parport_p.c. The both call request_region() to get ioports, then define the resulting port in pci config dwords 0x60 and 0x78. I don't think the serial and parallel functions can both work.

In the old ite887x patch from 2003 (http://lkml.org/lkml/2003/6/10/374), the ite887x serial code is contained in parport/parport_serial.c. It seems to me that parport_serial.c exists specifically for this sort of shared I/O card, and the ite887x serial code should be there rather than serial/8250_pci.c. It may be better to split code, and keep the serial-only versions in 8250_pci.c.

Also, I found a few discrepancies in parport_pc.c function sio_ite_8872_probe:

I think the inta_addr[] array should also include 0x200 and 0x280, as in 8250_pci.c.

Should it call "release_region(inta_addr[i], 0x8);" to release ioports when it finds this is a no-parallel card?

The printk message for the 2SP version is obscured with a debug macro, unlike all other versions of the card, and should be changed:
<<                DPRINTK(KERN_DEBUG "parport_pc: ITE8874 found (2S)\n");
>>                printk(KERN_INFO "parport_pc: ITE8874 found (2S)\n");

Comment 2 Niels de Vos 2010-04-15 11:28:50 UTC
Hi Joe,

My idea is to do the following:
1) attach the datasheet to BZ# 578616 (requested from one of the vendors)
2) check if the chip provides a register that says how many IO-ports are
   connected
3) use this value in the 8250_pci.c and parport_pc.c
4) (possibly) update parport_pc.c to only initialize the parport of IT887x chips


Background:
The super-IO chip can be connected to any number of IO-ports, it is a
choice the hardware manufacturer makes. Maybe it is possible to detect
the number of ports used, I'll have a look into that when I receive the
datasheet.

Your hardware (if not coming from Wincor-Nixdorf who tested the patches)
might not use the default number of IO-ports (32 iirc). A change that is
manufacturer/vendor specific might be needed in the worst case.


Please provide the following additional details of your system:

# cat /proc/tty/driver/serial
# cat /sys/module/8250/parameters/nr_uarts

Possibly you can change the behaviour when booting with 8250.nr_uarts=8
to increase the number of available UARTS.

Cheers,
Niels

Comment 3 Joe Krahn 2010-04-16 02:46:11 UTC
# cat /proc/tty/driver/serial
serinfo:1.0 driver revision:
0: uart:16550A port:000003F8 irq:4 tx:7492 rx:4948 RTS|CTS|DTR|DSR|CD
1: uart:16550A port:000002F8 irq:3 tx:445625 rx:665293 RTS|DTR
2: uart:16550A port:0000E800 irq:17 tx:0 rx:0
3: uart:16550A port:0000E400 irq:17 tx:0 rx:0

# cat /sys/module/8250/parameters/nr_uarts
4

The ioport configuration is incorrect, but not actually interfering with the serial ports, because the INTC port is only used to determine the device sub-type at device initialization. However, I think the right way to program the INTC port is to use the BAR 0, which is already PNP-configured to a unique address range:

        /* Map INTC control to BAR 0 */
        intc_base = pci_resource_start(pdev,0);


The actual problem with my device is that IRQ configuration is successful, but the UART IIR register does not clear the UART_IIR_NO_INT bit, so the serial interrupt handler ignores IRQs. At least for my hardware, the chip only sets the IIR status when set to serialized-IRQ mode, but the IRQ only works when set to standard INTA mode. Maybe this is why the current driver appears to be designed to disable interrupts:

        /* disable interrupts (UARTx_Routing[3:0]) */
        miscr &= ~(0xf << (12 - 4 * i));

The UART routing only applies to serialized IRQ, so my interrupts are enabled, but with IIR not reporting the interrupts status. It is probably a good idea to modify the autoconfig_irq in 8250.c to only accept a detected IRQ if the IIR register also works. For example:

        irq = probe_irq_off(irqs);
        if (irq && (serial_inp(up, UART_IIR) & UART_IIR_NO_INT) ) {
                printk(KERN_INFO "ttyS%d: interrupt did not set IIR status; IRQ disabled.\n",
                       serial_index(&up->port));
                irq = 0;
        }

This would cause my ITE UARTs to work by resorting to polled operation. The IIR value will still be incorrect, but serial8250_backup_timeout() has code to fake the IIR status bit in polled mode (look for "Diva" in 8250.c).

The above proposals will at least get the ITE887x working more reliably, if only in polled mode. It actually is not much harder to install a cutsom io_serial_in() function that communicates with the INTC directly, and fakes a normal IIR response.

Comment 4 Niels de Vos 2010-04-16 13:08:14 UTC
Created attachment 407093 [details]
Datasheet for the IT8872F chip manufactured by ITE

Comment 5 Joe Krahn 2010-04-17 17:07:41 UTC
Thanks for the data sheet!

I am testing a re-written driver, where the serial and parport drivers are merged into parport/parport_serial.c. I posted some info to LKML, and Alan Cox agreed that it makes sense to put the driver in one place, even though it can be either a combo card, or have only serial or parallel ports.

This turns out not to be as clean of a solution as it sounds. First, the parport_serial.c code most be updated to allow for no serial ports, and 8250_pci.c still needs some ITE-887x quirks code. IMHO, the 8250_pci driver should support external quirks function pointers, so that code for a non-standard serial port can be consolidated into a single module.

Since the ITE-887x chips are not that common, it is probably best to avoid global code changes. My current opinion is to update the current separate drivers as follows:

1) Use PCI BARs to map all I/O ports, including INTC.

2) Don't directly access PCI BARs from the config space (see Documentation/pci.txt)

3) Only map INTC I/O ports for the serial driver. The parport_pc driver code is designed to ignore ports that don't respond, so it is probably OK to simply assume 1 parallel port, and let the driver exit with ENODEV.

4) fix the autoconfig_irq() function to ignore UARTs where IIR does not respond correctly.

5) Maybe: Add a custom io_serial_in() function to map INTC IRQ status to the IIR, for ITE-887x chips that behave badly, like mine. The extra code to translate IRQs could be skipped, because it will work in polling mode. Or, this feature could be enabled by a module parameter.

Comment 6 Joe Krahn 2010-04-20 18:22:13 UTC
I have some results from testing updated driver code:

Bit 0 of INTC_base+0x18 may get set to one when using PCI BAR 0 for INTC. The default is for bits 1-4 set to GPIO_C, so bit 0 should not be used.

If the parport_pc driver probes for a parallel port, it always sees one, at leart for my hardware. So, the INTC type register needs to be checked.

The parport_pc driver depends on PCI device management to attach the driver. If the serial driver is already loaded, the parport_pc driver is ignored. In Fedora, the serial is built in and parport_pc is a module, so the current serial driver breaks the parport_pc driver.

It seems that the correct design is to use parport_serial. It is also not difficult to keep a separate 8250_pci driver for the serial-only chips. Does anyone know if this is beneficial? It could be relevant for a serial-port console, which may be why Fedora has serial code built in, but parport_serial is a module.

Comment 7 Niels de Vos 2010-04-29 11:51:24 UTC
(In reply to comment #6)
> If the parport_pc driver probes for a parallel port, it always sees one, at
> leart for my hardware. So, the INTC type register needs to be checked.

Yes, unfortunately the chip and it's available ports can not be detected easily. All chips seem to have the same PCI Vendor/Product ID's, so checking the type (not very well documented) is needed.


> It seems that the correct design is to use parport_serial. It is also not
> difficult to keep a separate 8250_pci driver for the serial-only chips. Does
> anyone know if this is beneficial? It could be relevant for a serial-port
> console, which may be why Fedora has serial code built in, but parport_serial
> is a module.    

Yes, for systems that have the IT887x on-board, it is wished that this functions during boot. Having the output of the console redirected to a serialport which is provided by a loadable module does not work. That said, it might/should be possible to compile the parport_serial into the kernel for these exceptions.

Comment 8 Bug Zapper 2010-11-03 18:10:08 UTC
This message is a reminder that Fedora 12 is nearing its end of life.
Approximately 30 (thirty) days from now Fedora will stop maintaining
and issuing updates for Fedora 12.  It is Fedora's policy to close all
bug reports from releases that are no longer maintained.  At that time
this bug will be closed as WONTFIX if it remains open with a Fedora 
'version' of '12'.

Package Maintainer: If you wish for this bug to remain open because you
plan to fix it in a currently maintained version, simply change the 'version' 
to a later Fedora version prior to Fedora 12's end of life.

Bug Reporter: Thank you for reporting this issue and we are sorry that 
we may not be able to fix it before Fedora 12 is end of life.  If you 
would still like to see this bug fixed and are able to reproduce it 
against a later version of Fedora please change the 'version' of this 
bug to the applicable version.  If you are unable to change the version, 
please add a comment here and someone will do it for you.

Although we aim to fix as many bugs as possible during every release's 
lifetime, sometimes those efforts are overtaken by events.  Often a 
more recent Fedora release includes newer upstream software that fixes 
bugs or makes them obsolete.

The process we are following is described here: 
http://fedoraproject.org/wiki/BugZappers/HouseKeeping

Comment 9 Niels de Vos 2010-11-06 21:00:04 UTC
Hi Joe,

what is the current status on this bug? I happily review any patches if you have something ready. Unfortunately I do not have access to the hardware any more, so I can not promise that I can find someone to do some testing.

Feel free to CC me on patches sent upstream.

Thanks!

Comment 10 Bug Zapper 2010-12-03 16:31:08 UTC
Fedora 12 changed to end-of-life (EOL) status on 2010-12-02. Fedora 12 is 
no longer maintained, which means that it will not receive any further 
security or bug fix updates. As a result we are closing this bug.

If you can reproduce this bug against a currently maintained version of 
Fedora please feel free to reopen this bug against that version.

Thank you for reporting this bug and we are sorry it could not be fixed.

Comment 11 Niels de Vos 2011-04-13 13:25:50 UTC
I received the following by email quite a while ago:

> We replaced the machine with that card, so I never got the code
> completed. The problem is that the kernel design for multifunction
> parallel/serial cards is not very good. This card is an anomaly
> because the same PCI id can be serial-only or parallel-only. There is
> really no way to create a completely proper driver without updating
> some of the main driver code. I hate to contribute an ugly quick-fix
> to the kernel, and a proper fix seems like too much work for a single
> card that is apparently not very common, or more comments would have
> shown up here.
> 
> The card has modifiable PCI IDs, so one possibility is to probe the
> card very early and modify the subsystem IDs according to the actual
> card type. This is the easiest solution, because standard ID-matching
> can be used, but it seems very ugly to hack PCI IDs.
> 
> Some of the core parallel and serial code could use an update, but it
> is a low priority because they are becoming a "legacy" hardware
> interface. Serial ports are still quite useful, but not so much for
> parallel ports, and the parallel code is the most incomplete. The
> multi driver code should probably have been primarily serial (i.e.
> serport-parallel) instead of primarily parallel (i.e.
> parport-serial).
> 
> If I can get advice on the correct approach, I will install the
> serial card, update the code for Fedora 13, and post a patch.
> 
> Thanks,
> Joe Krahn

I think this should work. It seems similar to devices that load a firmware-blob and get a new PCI/USB VendorID/ProductID.

Comment 12 Fedora End Of Life 2013-04-03 18:42:41 UTC
This bug appears to have been reported against 'rawhide' during the Fedora 19 development cycle.
Changing version to '19'.

(As we did not run this process for some time, it could affect also pre-Fedora 19 development
cycle bugs. We are very sorry. It will help us with cleanup during Fedora 19 End Of Life. Thank you.)

More information and reason for this action is here:
https://fedoraproject.org/wiki/BugZappers/HouseKeeping/Fedora19

Comment 13 Justin M. Forbes 2013-04-05 16:47:53 UTC
Is this still a problem with 3.9 based F19 kernels?

Comment 14 Justin M. Forbes 2013-04-23 17:26:21 UTC
This bug is being closed with INSUFFICIENT_DATA as there has not been a
response in 2 weeks.  If you are still experiencing this issue,
please reopen and attach the relevant data from the latest kernel you are
running and any data that might have been requested previously.


Note You need to log in before you can comment on or make changes to this bug.