[PATCH] yenta: irq-routing for TI bridges...again

[Daniel Ritz]

On Tuesday 16 March 2004 07:36, Pavel Roskin wrote:
> On Mon, 15 Mar 2004, Daniel Ritz wrote:
> > you undid moving around yenta_probe_irq(), plus added the prototypes on
> > top somewhere. ok. anything else i might have missed?
>
> I also undid removal of commented out parts in ti_override() and
> ti1250_override().  Anyway, the new patch is quite readable - diff is not
> confused anymore.

ah ok. it was too late to see it :) 

>
> > ok, just nuke it completely. we use PCI interrupts anyway if ISAs are not
> > working.
>
> Very good.
>
> > nice. this means your chip has the INTRTIE already set. the 1221 has INTB
> > only in MFUNC1 which is zero. looking at the correct value you have
> > mfunc1 set to isa irq 9. if you like you can clear that bit in your
> > make-it-non-working code to see the func1 code in action :)
>
> I'm not sure I understand you correctly but I tried, and the func1 code
> has failed miserably.
>
> New patch without changes:
>
> Yenta: CardBus bridge found at 0000:00:08.0 [133f:1233]
> Yenta: Using CSCINT to route CSC interrupts to PCI
> Yenta: Routing CardBus interrupts to PCI
> Yenta TI: socket 0000:00:08.0, mfunc 0x0cc07d92, devctl 0x60
> Yenta: ISA IRQ mask 0x0000, PCI irq 12
> Socket status: 30000010
> Yenta: CardBus bridge found at 0000:00:08.1 [133f:1233]
> Yenta: Using CSCINT to route CSC interrupts to PCI
> Yenta: Routing CardBus interrupts to PCI
> Yenta TI: socket 0000:00:08.1, mfunc 0x0cc07d92, devctl 0x60
> Yenta: ISA IRQ mask 0x0000, PCI irq 12
> Socket status: 30000010
>
> Everything is fine.
>
> Now testing with the original crippling code in the
> beginning of ti12xx_override()
>
> if (PCI_FUNC(socket->dev->devfn) == 0) {
>         config_writel(socket, TI122X_MFUNC, 0);
>         config_writeb(socket, TI113X_DEVICE_CONTROL, 0x66);
> }
>
> Yenta: CardBus bridge found at 0000:00:08.0 [133f:1233]
> Yenta: Using CSCINT to route CSC interrupts to PCI
> Yenta: Routing CardBus interrupts to PCI
> Yenta TI: socket 0000:00:08.0, mfunc 0x00000000, devctl 0x66
> Yenta TI: socket 0000:00:08.0 probing PCI interrupt failed, trying to fix
> Yenta TI: socket 0000:00:08.0 falling back to parallel PCI interrupts
> Yenta TI: socket 0000:00:08.0 parallel PCI interrupts ok
> Yenta: ISA IRQ mask 0x0000, PCI irq 12
> Socket status: 30000010
> Yenta: CardBus bridge found at 0000:00:08.1 [133f:1233]
> Yenta: Using CSCINT to route CSC interrupts to PCI
> Yenta: Routing CardBus interrupts to PCI
> Yenta TI: socket 0000:00:08.1, mfunc 0x00000002, devctl 0x64
> Yenta: ISA IRQ mask 0x0000, PCI irq 12
> Socket status: 30000010
>
> Also everything is fine.  I'm actually surprised that devctl was 0x64 when
> the second socket was probed.  That would be TI113X_DCR_IMODE_SERIAL.

yes, of course. i removed the ISA stuff on your request completly. no fall back to
parallel PCI only. we now only clear out the all-serial, ie. the PCI relevant part.

>
> New crippling code.  The INTRTIE bit is inverted (actually unset):
>
> if (PCI_FUNC(socket->dev->devfn) == 0) {
>         u32 sysctl = config_readl(socket, TI113X_SYSTEM_CONTROL);
>         config_writel(socket, TI122X_MFUNC, 0);
>         config_writeb(socket, TI113X_DEVICE_CONTROL, 0x66);
>         printk("sysctl = 0x%08x\n", sysctl);
>         sysctl ^= TI122X_SCR_INTRTIE;
>         config_writel(socket, TI113X_SYSTEM_CONTROL, sysctl);
> }

please use sysctl &= ~TI122X_SCR_INTRTIE; to clear bits out. anyway, it was set
so it got cleared.

>
> Yenta: CardBus bridge found at 0000:00:08.0 [133f:1233]
> sysctl = 0x2844f060
> Yenta: Using CSCINT to route CSC interrupts to PCI
> Yenta: Routing CardBus interrupts to PCI
> Yenta TI: socket 0000:00:08.0, mfunc 0x00000000, devctl 0x66
> Yenta TI: socket 0000:00:08.0 probing PCI interrupt failed, trying to fix
> Yenta TI: socket 0000:00:08.0 falling back to parallel PCI interrupts
> Yenta TI: socket 0000:00:08.0 parallel PCI interrupts ok
> Yenta: ISA IRQ mask 0x0000, PCI irq 12
> Socket status: 30000010
> Yenta: CardBus bridge found at 0000:00:08.1 [133f:1233]
> Yenta: Using CSCINT to route CSC interrupts to PCI
> Yenta: Routing CardBus interrupts to PCI
> Yenta TI: socket 0000:00:08.1, mfunc 0x00000002, devctl 0x64
> Yenta TI: socket 0000:00:08.1 probing PCI interrupt failed, trying to fix

so w/o intrtie no interrupt delivered

> irq 12: nobody cared!
> Call Trace:
>  [<c01098f7>] __report_bad_irq+0x27/0x80
>  [<f88b97bd>] yenta_interrupt+0xd/0x30 [yenta_socket]
>  [<c01099dc>] note_interrupt+0x5c/0x90
>  [<c0109fe3>] do_IRQ+0x293/0x380
>  [<c0107fdc>] common_interrupt+0x18/0x20
>  [<c0109896>] handle_IRQ_event+0x26/0x60
>  [<c0109e86>] do_IRQ+0x136/0x380
>  [<c0107fdc>] common_interrupt+0x18/0x20
>  [<c0107fdc>] common_interrupt+0x18/0x20
>  [<c0125e44>] do_softirq+0x44/0xa0
>  [<c0109f8e>] do_IRQ+0x23e/0x380
>  [<c0107fdc>] common_interrupt+0x18/0x20
>  [<c022798d>] acpi_processor_idle+0xe8/0x1e3
>  [<c0103000>] _stext+0x0/0xf0
>  [<c0104f94>] cpu_idle+0x34/0x40
>  [<c0462770>] start_kernel+0x1b0/0x220
>  [<c0462480>] unknown_bootoption+0x0/0x110
>
> handlers:
> [<f88b97b0>] (yenta_interrupt+0x0/0x30 [yenta_socket])
> Disabling IRQ #12

this is strange. there's the probe handler missing. which of the two
yenta_probe_cb_irq() is responsible? can you add some printk's?

also the probe handler clears out interrupts before deregistring
the handler.

> Yenta TI: socket 0000:00:08.1 no PCI interrupts. Fish. Please report.
> Yenta: ISA IRQ mask 0x0000, PCI irq 0
> Socket status: 30000010
>
> It looks like we sent an interrupt to func0, which didn't accept it.

yeah, but i don't really understand why.

>
> Perhaps we should not probe interrupts in ti12xx_irqroute_func1() without
> the INTRTIE bit being in a known state.  I even think that the same may be

what is known state??
the reason why i probe for INTB first is that it could be correct. in this case it
could be wrong to set intrtie 'cos we get the interrupt on the wrong line.
it's problematic if not both, inta and intb, are routed to the same interrupt.
what we should do is too look at the value of mfunc1 first, and
only try to set INTB only if it's zero. ('cos in your setup it was isa irq 9 originally)

> true for ti12xx_irqroute_func0() as well.

i think func0 is less problematic.

>
> If we generate an interrupt, we must make sure that we don't say it's not
> ours. There may be other devices already active on that interrupt, and
> they will lose their interrupt too if nobody claims it.
>
> Also, mfunc is restored to mfunc_old after "write, probe" and never used
> after "still nothing: set INTRTIE", so we are testing the interrupts with
> INTRTIE and the old mfunc.  I'm afraid it's wrong.

well, it is in the config_writel() on the following line. it could have been mfunc_old
there directly, but it's copy-paste...

can you try two things:
- change the delay in yenta_probe_cb_irq() (try smaller/larger)
- add the following line to the disabling code in yenta_probe_cb_irq()
	cb_writel(socket, CB_SOCKET_FORCE, 0);

another idea. what does the lspci -vvv of your (working) 1221 look like?
we could look up the PCI config to see which pin is routed to which interrupt...

rgds
-daniel