[coreboot] proposed patch, notsigned off, comments welcome.

[Peter Stuge]

On Sun, May 11, 2008 at 12:33:01PM -0700, ron minnich wrote:
> This is a first try at a program to grok lxirqs.

I love utilities that help automate ports!


I'll start with a recap of my summary:

On Mon, May 05, 2008 at 02:06:31AM +0200, Peter Stuge wrote:
> The 5536 has GPIO interrupts, which are used for the external PCI
> bus interrupt signals. That makes sense since the PCI_INTx signals
> connect to 5536 GPIO balls.

..

> Hardwarewise, the USB module generates interrupts on what is called
> "Unrestricted Y Input 2" on the PIC. That input maps to one interrupt
> group between 1 and 15 per the PIC specific MSR at addr+20h bits 11:8.
> 
> The interrupt is OR:ed with any other interrupts mapped to the same
> group. The group number corresponds to the classic 8259 IRQ number.
> Interrupt group 10 generates IRQ 10. "Unrestricted" because the input
> can be mapped to any IRQ 1-15. MFGPT and GPIO interrupts are on the
> Unrestricted Z inputs.
> 
> Reset value for all interrupt group MSRs is 0, ie. disabled.

Unrestricted Z inputs to the PIC are also mapped to one interrupt
group between 1 and 15. Which GPIO interrupt input that goes to which
interrupt group is controlled by the PIC specific MSRs at addr+22h
and addr+23h.


Ok, back to Ron's message:

> Sanity check first, I'm not signing this off yet. Are the enable
> and invert tests correct below?
> 
> Long term goal is a program that will follow the IRQ through the
> magic rabbit hole and up to the CPU.

Why? When will the program be used?


> I don't know how to do vr reads in user mode, however.

VRs don't really matter. VRs _only_ purpose is to implement a
compatibility layer for software that does not understand GeodeLink.

I think our utilities should and must do that. They don't have to
care about VRs. The only caveat would be that if something is written
to GeodeLink then perhaps a VR needs to be updated to reflect the
change - but such changes may be taboo anyway because whoever is
reading the VRs may not be able to handle someone else changing them.
Another issue is that VRs are not guaranteed to be atomic.


> Here's the current output on alix1c, which is strange looking to me.
> IRQ A, GPIO pin 0
> Input Enabled and Not Inverted
> IRQ B, GPIO pin 7
> Input Enabled and Inverted
> IRQ C, GPIO pin 12
> Input Enabled and Inverted
> IRQ D, GPIO pin 13
> Input Disabled and Not Inverted

This does not look correct. I would expect all pins to be set up the
same way. Can you confirm using the MSR tool from the other day?


> +char *irqnames[] = {"invalid", "A", "B", "C", "D"};

Please do not mix IRQ with INT;

INT is an instruction which generates a software interrupt.

IRQ is Interrupt Request Channel and refers to hardware
generated interrupts, the number between 1 and 15. IRQs used to be
equivalent to electrical signals, but that's no longer true.
This is also called interrupt line, and is set in bits 7:0 in PCI
register 0x3c.

PCI_INTA, PCI_INTB, PCI_INTC and PCI_INTD are names of the electrical
signals in the PCI bus. The utility deals only with these so far.
This is also called the interrupt pin, and is set in bits 15:8 in PCI
register 0x3c.


> +void irq(struct pci_dev *cs5536, int irqno, unsigned char gpiopin)
> +{
> +	/* Use GPIOBASE register to find where the GPIO is mapped. */
> +	gpiobar = pci_read_word(cs5536, 0x14) & 0xfffc;
> +
> +	printf("IRQ %s, GPIO pin %d\n", irqnames[irqno], gpiopin);
> +
> +//	enable = inl(gpiobar + 0x20);
> +//	printf("input enable %08lx\n", enable);
> +	printf("Input %s and %s\n", 
> +		atomicval(0x20, gpiopin) ? "Enabled" : "Disabled", 
> +		atomicval(0x24, gpiopin) ? "Inverted" : "Not Inverted");
> +	
> +}
..
> +int main(int argc, char *argv[])
> +{
..
> +	introute = strtoul(argv[1], 0, 0);
..
> +	irq(cs5536, 1, introute);
> +	irq(cs5536, 2, introute>>8);
> +	irq(cs5536, 3, introute>>16);
> +	irq(cs5536, 4, introute>>24);

I think this is backwards.

One, there is no need for PCI, please use MSRs directly instead.

Two, I would expect a utility to _output_ the correct GPIO pins, not
require them as _input_.

If the current mapping of GPIO pins->PIC Unrestricted Z inputs is
supposed to be specifed then that can and should always be read
directly from MSRs PICbase+22h and +23h.

All GPIO settings are read with the generic MSR tool that was
recently committed, and I think a shell script would be good enough
to translate some bits to text.


I'm afraid I don't understand the motivation for this tool.

Please help me so I can contribute something more constructive! :)


//Peter