| Message ID | 1438033652-30435-15-git-send-email-sjg@chromium.org |
|---|---|
| State | Superseded |
| Delegated to: | Simon Glass |
| Headers | show |
Hi Simon, On Tue, Jul 28, 2015 at 5:47 AM, Simon Glass <sjg@chromium.org> wrote: > At present minnowmax does not correct set up PCI interrupts. This should be > done in U-Boot so that devices work correctly in Linux. > > Note: This code needs to make use of the recent pirq_routing work. It does > not seem to support all the required features, so this RFC will hopefully What features are missing in the existing PIRQ codes? When I did the PIRQ support, I checked both TunnelCreek and BayTrail chipset datasheet, and found the only difference seems to be the pci configuration space vs. memory-mapped IBASE where the pci irq routing registers reside. > help figure this out. > > Signed-off-by: Simon Glass <sjg@chromium.org> > --- [snip] Regards, Bin
Hi Bin, On 28 July 2015 at 01:50, Bin Meng <bmeng.cn@gmail.com> wrote: > > Hi Simon, > > On Tue, Jul 28, 2015 at 5:47 AM, Simon Glass <sjg@chromium.org> wrote: > > At present minnowmax does not correct set up PCI interrupts. This should be > > done in U-Boot so that devices work correctly in Linux. > > > > Note: This code needs to make use of the recent pirq_routing work. It does > > not seem to support all the required features, so this RFC will hopefully > > What features are missing in the existing PIRQ codes? When I did the > PIRQ support, I checked both TunnelCreek and BayTrail chipset > datasheet, and found the only difference seems to be the pci > configuration space vs. memory-mapped IBASE where the pci irq routing > registers reside. I see that each PCI device can be assigned four routes, making up a 16-bit register. But the code I see in pirq_assign_req() only assigns a single one, using a byte register. Regards, Simon
Hi Simon, On Tue, Jul 28, 2015 at 7:58 PM, Simon Glass <sjg@chromium.org> wrote: > Hi Bin, > > On 28 July 2015 at 01:50, Bin Meng <bmeng.cn@gmail.com> wrote: >> >> Hi Simon, >> >> On Tue, Jul 28, 2015 at 5:47 AM, Simon Glass <sjg@chromium.org> wrote: >> > At present minnowmax does not correct set up PCI interrupts. This should be >> > done in U-Boot so that devices work correctly in Linux. >> > >> > Note: This code needs to make use of the recent pirq_routing work. It does >> > not seem to support all the required features, so this RFC will hopefully >> >> What features are missing in the existing PIRQ codes? When I did the >> PIRQ support, I checked both TunnelCreek and BayTrail chipset >> datasheet, and found the only difference seems to be the pci >> configuration space vs. memory-mapped IBASE where the pci irq routing >> registers reside. > > I see that each PCI device can be assigned four routes, making up a > 16-bit register. But the code I see in pirq_assign_req() only assigns > a single one, using a byte register. > No, the pci irq routing register is still a 8-bit register on BayTrail, where pirq_assign_irq() programs. The 16-bit register you mentioned should be done in the platform codes. See arch/x86/cpu/queensbay/tnc.c::cpu_irq_init(). By the way actually we can leave those register programmed as they have the optimized default values for all pci devices after power up, unless we intentionally want to change them. Regards, Bin
Hi Bin, On 28 July 2015 at 18:40, Bin Meng <bmeng.cn@gmail.com> wrote: > Hi Simon, > > On Tue, Jul 28, 2015 at 7:58 PM, Simon Glass <sjg@chromium.org> wrote: >> Hi Bin, >> >> On 28 July 2015 at 01:50, Bin Meng <bmeng.cn@gmail.com> wrote: >>> >>> Hi Simon, >>> >>> On Tue, Jul 28, 2015 at 5:47 AM, Simon Glass <sjg@chromium.org> wrote: >>> > At present minnowmax does not correct set up PCI interrupts. This should be >>> > done in U-Boot so that devices work correctly in Linux. >>> > >>> > Note: This code needs to make use of the recent pirq_routing work. It does >>> > not seem to support all the required features, so this RFC will hopefully >>> >>> What features are missing in the existing PIRQ codes? When I did the >>> PIRQ support, I checked both TunnelCreek and BayTrail chipset >>> datasheet, and found the only difference seems to be the pci >>> configuration space vs. memory-mapped IBASE where the pci irq routing >>> registers reside. >> >> I see that each PCI device can be assigned four routes, making up a >> 16-bit register. But the code I see in pirq_assign_req() only assigns >> a single one, using a byte register. >> > > No, the pci irq routing register is still a 8-bit register on > BayTrail, where pirq_assign_irq() programs. The 16-bit register you > mentioned should be done in the platform codes. See > arch/x86/cpu/queensbay/tnc.c::cpu_irq_init(). By the way actually we > can leave those register programmed as they have the optimized default > values for all pci devices after power up, unless we intentionally > want to change them. What do you mean by 'leave those register programmed'? Regards, Simon
Hi Simon, On Wed, Jul 29, 2015 at 8:42 AM, Simon Glass <sjg@chromium.org> wrote: > Hi Bin, > > On 28 July 2015 at 18:40, Bin Meng <bmeng.cn@gmail.com> wrote: >> Hi Simon, >> >> On Tue, Jul 28, 2015 at 7:58 PM, Simon Glass <sjg@chromium.org> wrote: >>> Hi Bin, >>> >>> On 28 July 2015 at 01:50, Bin Meng <bmeng.cn@gmail.com> wrote: >>>> >>>> Hi Simon, >>>> >>>> On Tue, Jul 28, 2015 at 5:47 AM, Simon Glass <sjg@chromium.org> wrote: >>>> > At present minnowmax does not correct set up PCI interrupts. This should be >>>> > done in U-Boot so that devices work correctly in Linux. >>>> > >>>> > Note: This code needs to make use of the recent pirq_routing work. It does >>>> > not seem to support all the required features, so this RFC will hopefully >>>> >>>> What features are missing in the existing PIRQ codes? When I did the >>>> PIRQ support, I checked both TunnelCreek and BayTrail chipset >>>> datasheet, and found the only difference seems to be the pci >>>> configuration space vs. memory-mapped IBASE where the pci irq routing >>>> registers reside. >>> >>> I see that each PCI device can be assigned four routes, making up a >>> 16-bit register. But the code I see in pirq_assign_req() only assigns >>> a single one, using a byte register. >>> >> >> No, the pci irq routing register is still a 8-bit register on >> BayTrail, where pirq_assign_irq() programs. The 16-bit register you >> mentioned should be done in the platform codes. See >> arch/x86/cpu/queensbay/tnc.c::cpu_irq_init(). By the way actually we >> can leave those register programmed as they have the optimized default >> values for all pci devices after power up, unless we intentionally >> want to change them. > > What do you mean by 'leave those register programmed'? > I mean their default value is normally OK, like INTA maps PIRQA, INTB maps PIRQB, INTC maps PIRQC and INTD maps PIRQD. Regards, Bin
Hi Bin, On 28 July 2015 at 18:46, Bin Meng <bmeng.cn@gmail.com> wrote: > Hi Simon, > > On Wed, Jul 29, 2015 at 8:42 AM, Simon Glass <sjg@chromium.org> wrote: >> Hi Bin, >> >> On 28 July 2015 at 18:40, Bin Meng <bmeng.cn@gmail.com> wrote: >>> Hi Simon, >>> >>> On Tue, Jul 28, 2015 at 7:58 PM, Simon Glass <sjg@chromium.org> wrote: >>>> Hi Bin, >>>> >>>> On 28 July 2015 at 01:50, Bin Meng <bmeng.cn@gmail.com> wrote: >>>>> >>>>> Hi Simon, >>>>> >>>>> On Tue, Jul 28, 2015 at 5:47 AM, Simon Glass <sjg@chromium.org> wrote: >>>>> > At present minnowmax does not correct set up PCI interrupts. This should be >>>>> > done in U-Boot so that devices work correctly in Linux. >>>>> > >>>>> > Note: This code needs to make use of the recent pirq_routing work. It does >>>>> > not seem to support all the required features, so this RFC will hopefully >>>>> >>>>> What features are missing in the existing PIRQ codes? When I did the >>>>> PIRQ support, I checked both TunnelCreek and BayTrail chipset >>>>> datasheet, and found the only difference seems to be the pci >>>>> configuration space vs. memory-mapped IBASE where the pci irq routing >>>>> registers reside. >>>> >>>> I see that each PCI device can be assigned four routes, making up a >>>> 16-bit register. But the code I see in pirq_assign_req() only assigns >>>> a single one, using a byte register. >>>> >>> >>> No, the pci irq routing register is still a 8-bit register on >>> BayTrail, where pirq_assign_irq() programs. The 16-bit register you >>> mentioned should be done in the platform codes. See >>> arch/x86/cpu/queensbay/tnc.c::cpu_irq_init(). By the way actually we >>> can leave those register programmed as they have the optimized default >>> values for all pci devices after power up, unless we intentionally >>> want to change them. >> >> What do you mean by 'leave those register programmed'? >> > > I mean their default value is normally OK, like INTA maps PIRQA, INTB > maps PIRQB, INTC maps PIRQC and INTD maps PIRQD. OK, so drop writing to the pirq registers? Should I program the 16-bit registers? If so, I will need to extend the device tree binding, won't I? Sorry my understanding is limited on this - and I'd like to use your generic code if possible. Regards, Simon
Hi Simon, On Wed, Jul 29, 2015 at 8:48 AM, Simon Glass <sjg@chromium.org> wrote: > Hi Bin, > > On 28 July 2015 at 18:46, Bin Meng <bmeng.cn@gmail.com> wrote: >> Hi Simon, >> >> On Wed, Jul 29, 2015 at 8:42 AM, Simon Glass <sjg@chromium.org> wrote: >>> Hi Bin, >>> >>> On 28 July 2015 at 18:40, Bin Meng <bmeng.cn@gmail.com> wrote: >>>> Hi Simon, >>>> >>>> On Tue, Jul 28, 2015 at 7:58 PM, Simon Glass <sjg@chromium.org> wrote: >>>>> Hi Bin, >>>>> >>>>> On 28 July 2015 at 01:50, Bin Meng <bmeng.cn@gmail.com> wrote: >>>>>> >>>>>> Hi Simon, >>>>>> >>>>>> On Tue, Jul 28, 2015 at 5:47 AM, Simon Glass <sjg@chromium.org> wrote: >>>>>> > At present minnowmax does not correct set up PCI interrupts. This should be >>>>>> > done in U-Boot so that devices work correctly in Linux. >>>>>> > >>>>>> > Note: This code needs to make use of the recent pirq_routing work. It does >>>>>> > not seem to support all the required features, so this RFC will hopefully >>>>>> >>>>>> What features are missing in the existing PIRQ codes? When I did the >>>>>> PIRQ support, I checked both TunnelCreek and BayTrail chipset >>>>>> datasheet, and found the only difference seems to be the pci >>>>>> configuration space vs. memory-mapped IBASE where the pci irq routing >>>>>> registers reside. >>>>> >>>>> I see that each PCI device can be assigned four routes, making up a >>>>> 16-bit register. But the code I see in pirq_assign_req() only assigns >>>>> a single one, using a byte register. >>>>> >>>> >>>> No, the pci irq routing register is still a 8-bit register on >>>> BayTrail, where pirq_assign_irq() programs. The 16-bit register you >>>> mentioned should be done in the platform codes. See >>>> arch/x86/cpu/queensbay/tnc.c::cpu_irq_init(). By the way actually we >>>> can leave those register programmed as they have the optimized default >>>> values for all pci devices after power up, unless we intentionally >>>> want to change them. >>> >>> What do you mean by 'leave those register programmed'? >>> >> >> I mean their default value is normally OK, like INTA maps PIRQA, INTB >> maps PIRQB, INTC maps PIRQC and INTD maps PIRQD. > > OK, so drop writing to the pirq registers? You might need check if BayTrail FSP does any programming on these registers. On Intel Crown Bay, I checked those values are the same values as documented in the TunnelCreek datasheet. > Should I program the 16-bit registers? If so, I will need to extend > the device tree binding, won't I? When I did the device tree binding changes, I did not convert these registers to use device tree, because their offsets, not like the routing registers as programmed in pirq_assign_irq(), is really platform specific thing and does not seem to have a good formula to calculate its offset (it varies from platform to platform). I previously wanted to create some device bindings like below, but I think they are really ugly so I did not go that way: irq-router@1f,0 { reg = <0x0000f800 0 0 0 0>; compatible = "intel,irq-router"; ...... dev0-intx-offset = <0x2040>; dev0-intx-value = <PIRQA PIRQB PIRQC PIRQD>; dev2-intx-offset = <0x2048>; dev2-intx-value = <PIRQA PIRQB PIRQC PIRQD>; dev4-intx-offset = <0x2060>; dev4-intx-value = <PIRQA PIRQB PIRQC PIRQD>; dev26-intx-offset = <0x20a0>; dev26-intx-value = <PIRQA PIRQB PIRQC PIRQD>; dev31-intx-offset = <0x20c0>; dev31-intx-value = <PIRQA PIRQB PIRQC PIRQD>; ...... Sorry I cannot find a better way to describe this in device tree, so I chose to implement directly in the cpu_irq_init() to program these registers. > > Sorry my understanding is limited on this - and I'd like to use your > generic code if possible. > Yes, let's try to use the existing codes. If needed, we can have some patches to support new platforms. Regards, Bin
Hi Simon, On Wed, Jul 29, 2015 at 9:08 AM, Bin Meng <bmeng.cn@gmail.com> wrote: > Hi Simon, > > On Wed, Jul 29, 2015 at 8:48 AM, Simon Glass <sjg@chromium.org> wrote: >> Hi Bin, >> >> On 28 July 2015 at 18:46, Bin Meng <bmeng.cn@gmail.com> wrote: >>> Hi Simon, >>> >>> On Wed, Jul 29, 2015 at 8:42 AM, Simon Glass <sjg@chromium.org> wrote: >>>> Hi Bin, >>>> >>>> On 28 July 2015 at 18:40, Bin Meng <bmeng.cn@gmail.com> wrote: >>>>> Hi Simon, >>>>> >>>>> On Tue, Jul 28, 2015 at 7:58 PM, Simon Glass <sjg@chromium.org> wrote: >>>>>> Hi Bin, >>>>>> >>>>>> On 28 July 2015 at 01:50, Bin Meng <bmeng.cn@gmail.com> wrote: >>>>>>> >>>>>>> Hi Simon, >>>>>>> >>>>>>> On Tue, Jul 28, 2015 at 5:47 AM, Simon Glass <sjg@chromium.org> wrote: >>>>>>> > At present minnowmax does not correct set up PCI interrupts. This should be >>>>>>> > done in U-Boot so that devices work correctly in Linux. >>>>>>> > >>>>>>> > Note: This code needs to make use of the recent pirq_routing work. It does >>>>>>> > not seem to support all the required features, so this RFC will hopefully >>>>>>> >>>>>>> What features are missing in the existing PIRQ codes? When I did the >>>>>>> PIRQ support, I checked both TunnelCreek and BayTrail chipset >>>>>>> datasheet, and found the only difference seems to be the pci >>>>>>> configuration space vs. memory-mapped IBASE where the pci irq routing >>>>>>> registers reside. >>>>>> >>>>>> I see that each PCI device can be assigned four routes, making up a >>>>>> 16-bit register. But the code I see in pirq_assign_req() only assigns >>>>>> a single one, using a byte register. >>>>>> >>>>> >>>>> No, the pci irq routing register is still a 8-bit register on >>>>> BayTrail, where pirq_assign_irq() programs. The 16-bit register you >>>>> mentioned should be done in the platform codes. See >>>>> arch/x86/cpu/queensbay/tnc.c::cpu_irq_init(). By the way actually we >>>>> can leave those register programmed as they have the optimized default >>>>> values for all pci devices after power up, unless we intentionally >>>>> want to change them. >>>> >>>> What do you mean by 'leave those register programmed'? >>>> >>> >>> I mean their default value is normally OK, like INTA maps PIRQA, INTB >>> maps PIRQB, INTC maps PIRQC and INTD maps PIRQD. >> >> OK, so drop writing to the pirq registers? > > You might need check if BayTrail FSP does any programming on these > registers. On Intel Crown Bay, I checked those values are the same > values as documented in the TunnelCreek datasheet. > >> Should I program the 16-bit registers? If so, I will need to extend >> the device tree binding, won't I? > > When I did the device tree binding changes, I did not convert these > registers to use device tree, because their offsets, not like the > routing registers as programmed in pirq_assign_irq(), is really > platform specific thing and does not seem to have a good formula to > calculate its offset (it varies from platform to platform). I > previously wanted to create some device bindings like below, but I > think they are really ugly so I did not go that way: > > irq-router@1f,0 { > reg = <0x0000f800 0 0 0 0>; > compatible = "intel,irq-router"; > ...... > dev0-intx-offset = <0x2040>; > dev0-intx-value = <PIRQA PIRQB PIRQC PIRQD>; > dev2-intx-offset = <0x2048>; > dev2-intx-value = <PIRQA PIRQB PIRQC PIRQD>; > dev4-intx-offset = <0x2060>; > dev4-intx-value = <PIRQA PIRQB PIRQC PIRQD>; > dev26-intx-offset = <0x20a0>; > dev26-intx-value = <PIRQA PIRQB PIRQC PIRQD>; > dev31-intx-offset = <0x20c0>; > dev31-intx-value = <PIRQA PIRQB PIRQC PIRQD>; > ...... > > Sorry I cannot find a better way to describe this in device tree, so I > chose to implement directly in the cpu_irq_init() to program these > registers. > >> >> Sorry my understanding is limited on this - and I'd like to use your >> generic code if possible. >> > > Yes, let's try to use the existing codes. If needed, we can have some > patches to support new platforms. > I just managed to get a BayTrail board (not MinnowMax) and will try to first port U-Boot (assume it won't take much time) then see how PIRQ looks like on this platform. Regards, Bin
Hi Simon, On Wed, Jul 29, 2015 at 5:01 PM, Bin Meng <bmeng.cn@gmail.com> wrote: > Hi Simon, > > On Wed, Jul 29, 2015 at 9:08 AM, Bin Meng <bmeng.cn@gmail.com> wrote: >> Hi Simon, >> >> On Wed, Jul 29, 2015 at 8:48 AM, Simon Glass <sjg@chromium.org> wrote: >>> Hi Bin, >>> >>> On 28 July 2015 at 18:46, Bin Meng <bmeng.cn@gmail.com> wrote: >>>> Hi Simon, >>>> >>>> On Wed, Jul 29, 2015 at 8:42 AM, Simon Glass <sjg@chromium.org> wrote: >>>>> Hi Bin, >>>>> >>>>> On 28 July 2015 at 18:40, Bin Meng <bmeng.cn@gmail.com> wrote: >>>>>> Hi Simon, >>>>>> >>>>>> On Tue, Jul 28, 2015 at 7:58 PM, Simon Glass <sjg@chromium.org> wrote: >>>>>>> Hi Bin, >>>>>>> >>>>>>> On 28 July 2015 at 01:50, Bin Meng <bmeng.cn@gmail.com> wrote: >>>>>>>> >>>>>>>> Hi Simon, >>>>>>>> >>>>>>>> On Tue, Jul 28, 2015 at 5:47 AM, Simon Glass <sjg@chromium.org> wrote: >>>>>>>> > At present minnowmax does not correct set up PCI interrupts. This should be >>>>>>>> > done in U-Boot so that devices work correctly in Linux. >>>>>>>> > >>>>>>>> > Note: This code needs to make use of the recent pirq_routing work. It does >>>>>>>> > not seem to support all the required features, so this RFC will hopefully >>>>>>>> >>>>>>>> What features are missing in the existing PIRQ codes? When I did the >>>>>>>> PIRQ support, I checked both TunnelCreek and BayTrail chipset >>>>>>>> datasheet, and found the only difference seems to be the pci >>>>>>>> configuration space vs. memory-mapped IBASE where the pci irq routing >>>>>>>> registers reside. >>>>>>> >>>>>>> I see that each PCI device can be assigned four routes, making up a >>>>>>> 16-bit register. But the code I see in pirq_assign_req() only assigns >>>>>>> a single one, using a byte register. >>>>>>> >>>>>> >>>>>> No, the pci irq routing register is still a 8-bit register on >>>>>> BayTrail, where pirq_assign_irq() programs. The 16-bit register you >>>>>> mentioned should be done in the platform codes. See >>>>>> arch/x86/cpu/queensbay/tnc.c::cpu_irq_init(). By the way actually we >>>>>> can leave those register programmed as they have the optimized default >>>>>> values for all pci devices after power up, unless we intentionally >>>>>> want to change them. >>>>> >>>>> What do you mean by 'leave those register programmed'? >>>>> >>>> >>>> I mean their default value is normally OK, like INTA maps PIRQA, INTB >>>> maps PIRQB, INTC maps PIRQC and INTD maps PIRQD. >>> >>> OK, so drop writing to the pirq registers? >> >> You might need check if BayTrail FSP does any programming on these >> registers. On Intel Crown Bay, I checked those values are the same >> values as documented in the TunnelCreek datasheet. >> >>> Should I program the 16-bit registers? If so, I will need to extend >>> the device tree binding, won't I? >> >> When I did the device tree binding changes, I did not convert these >> registers to use device tree, because their offsets, not like the >> routing registers as programmed in pirq_assign_irq(), is really >> platform specific thing and does not seem to have a good formula to >> calculate its offset (it varies from platform to platform). I >> previously wanted to create some device bindings like below, but I >> think they are really ugly so I did not go that way: >> >> irq-router@1f,0 { >> reg = <0x0000f800 0 0 0 0>; >> compatible = "intel,irq-router"; >> ...... >> dev0-intx-offset = <0x2040>; >> dev0-intx-value = <PIRQA PIRQB PIRQC PIRQD>; >> dev2-intx-offset = <0x2048>; >> dev2-intx-value = <PIRQA PIRQB PIRQC PIRQD>; >> dev4-intx-offset = <0x2060>; >> dev4-intx-value = <PIRQA PIRQB PIRQC PIRQD>; >> dev26-intx-offset = <0x20a0>; >> dev26-intx-value = <PIRQA PIRQB PIRQC PIRQD>; >> dev31-intx-offset = <0x20c0>; >> dev31-intx-value = <PIRQA PIRQB PIRQC PIRQD>; >> ...... >> >> Sorry I cannot find a better way to describe this in device tree, so I >> chose to implement directly in the cpu_irq_init() to program these >> registers. >> >>> >>> Sorry my understanding is limited on this - and I'd like to use your >>> generic code if possible. >>> >> >> Yes, let's try to use the existing codes. If needed, we can have some >> patches to support new platforms. >> > > I just managed to get a BayTrail board (not MinnowMax) and will try to > first port U-Boot (assume it won't take much time) then see how PIRQ > looks like on this platform. > It turns out the existing PIRQ codes works pretty well on BayTrail. Please check this patch: http://patchwork.ozlabs.org/patch/502058/ Regards, Bin
diff --git a/arch/x86/cpu/baytrail/valleyview.c b/arch/x86/cpu/baytrail/valleyview.c index f1c3578..efbb4a5 100644 --- a/arch/x86/cpu/baytrail/valleyview.c +++ b/arch/x86/cpu/baytrail/valleyview.c @@ -5,9 +5,14 @@ */ #include <common.h> +#include <dm.h> #include <mmc.h> #include <pci_ids.h> +#include <asm/interrupt.h> +#include <asm/irq.h> +#include <asm/ioapic.h> #include <asm/post.h> +#include <asm/fsp/fsp_support.h> static struct pci_device_id mmc_supported[] = { { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_VALLEYVIEW_SDIO }, @@ -37,3 +42,490 @@ int arch_cpu_init(void) return 0; } #endif + +/* + * TODO(sjg): This code comes from coreboot's fsp_baytrail/southcluster.c. It + * has been cleaned up a little but still has macro stuff like + * PCI_DEV_PIRQ_ROUTE and doesn't use device tree. It needs to be adjusted to + * fit with the existing pirq-routing code. However it isn't clear that that + * code has all the required features. + * + * PIC IRQ settings + */ +#define PIRQ_PIC_IRQ3 0x3 +#define PIRQ_PIC_IRQ4 0x4 +#define PIRQ_PIC_IRQ5 0x5 +#define PIRQ_PIC_IRQ6 0x6 +#define PIRQ_PIC_IRQ7 0x7 +#define PIRQ_PIC_IRQ9 0x9 +#define PIRQ_PIC_IRQ10 0xa +#define PIRQ_PIC_IRQ11 0xb +#define PIRQ_PIC_IRQ12 0xc +#define PIRQ_PIC_IRQ14 0xe +#define PIRQ_PIC_IRQ15 0xf +#define PIRQ_PIC_IRQDISABLE 0x80 + +#define PCI_DEV_PIRQ_ROUTE(dev_, a_, b_, c_, d_) \ + [dev_] = ((PIRQ ## d_) << 12) | ((PIRQ ## c_) << 8) | \ + ((PIRQ ## b_) << 4) | ((PIRQ ## a_) << 0) + +#define PIRQ_PIC(pirq_, pic_irq_) \ + [PIRQ ## pirq_] = PIRQ_PIC_IRQ ## pic_irq_ + +#define NUM_OF_PCI_DEVS 32 +#define NUM_PIRQS 8 + +struct baytrail_irq_route { + /* Per-device configuration */ + uint16_t pcidev[NUM_OF_PCI_DEVS]; + /* Route path for each internal PIRQx in PIC mode */ + uint8_t pic[NUM_PIRQS]; +}; + +static struct baytrail_irq_route irq_info = { + { + PCI_DEV_PIRQ_ROUTE(0x02, A, A, A, A), + PCI_DEV_PIRQ_ROUTE(0x10, D, E, F, G), + PCI_DEV_PIRQ_ROUTE(0x11, B, A, A, A), + PCI_DEV_PIRQ_ROUTE(0x12, C, A, A, A), + PCI_DEV_PIRQ_ROUTE(0x13, D, A, A, A), + PCI_DEV_PIRQ_ROUTE(0x14, E, A, A, A), + PCI_DEV_PIRQ_ROUTE(0x15, F, A, A, A), + PCI_DEV_PIRQ_ROUTE(0x17, F, A, A, A), + PCI_DEV_PIRQ_ROUTE(0x18, B, A, D, C), + PCI_DEV_PIRQ_ROUTE(0x1a, F, A, A, A), + PCI_DEV_PIRQ_ROUTE(0x1b, G, A, A, A), + PCI_DEV_PIRQ_ROUTE(0x1c, E, F, G, H), + PCI_DEV_PIRQ_ROUTE(0x1d, D, A, A, A), + PCI_DEV_PIRQ_ROUTE(0x1e, B, D, E, F), + PCI_DEV_PIRQ_ROUTE(0x1f, H, G, B, C), + }, { + /* + * Route each PIRQ[A-H] to a PIC IRQ[0-15] + * Reserved: 0, 1, 2, 8, 13 + * PS2 keyboard: 12 + * ACPI/SCI: 9 + * Floppy: 6 + */ + PIRQ_PIC(A, 4), + PIRQ_PIC(B, 5), + PIRQ_PIC(C, 7), + PIRQ_PIC(D, 10), + PIRQ_PIC(E, 11), + PIRQ_PIC(F, 12), + PIRQ_PIC(G, 14), + PIRQ_PIC(H, 15), + } +}; + +struct ibase_regs { + u32 actl; + u32 mc; + u8 pirq[8]; + u32 scnt; /* 0x10 */ + u32 kmc; + u32 fs; + u32 bc; + u16 ir[32]; /* 0x20 */ + u32 oic; /* 0x60 */ +}; + +enum { + /* ACTL */ + SCIS_MASK = 0x07, + SCIS_IRQ9 = 0, + SCIS_IRQ10, + SCIS_IRQ11, + SCIS_IRQ20, + SCIS_IRQ21, + SCIS_IRQ22, + SCIS_IRQ23, + + /* OIC */ + AEN = 1 << 8, + + /* SCNT */ + SCNT_CONTINUOUS_MODE = 1 << 7, + SCNT_QUIET_MODE = 0, + SIRQEN = 1 << 12, + + IBASE = 0x50, +}; + +static int valleyview_enable_ioapic(struct udevice *dev, + struct ibase_regs *ibase) +{ + u32 reg32; + int i; + + /* Enable ACPI I/O and power management. Set SCI IRQ to IRQ9 */ + writel(AEN, &ibase->oic); + reg32 = readl(&ibase->oic); /* Read back per BWG */ + writel(SCIS_IRQ9, &ibase->actl); + + io_apic_write(0, 1 << 25); + + /* affirm full set of redirection table entries ("write once") */ + reg32 = io_apic_read(1); + io_apic_write(1, reg32); + + reg32 = io_apic_read(0); + debug("Southbridge APIC ID = %x\n", (reg32 >> 24) & 0x0f); + if (reg32 != (1 << 25)) { + debug("APIC Error\n"); + return -EINVAL; + } + + debug("Dumping IOAPIC registers\n"); + for (i = 0; i < 3; i++) + debug(" reg 0x%04x: 0x%08x\n", i, io_apic_read(i)); + + /* + * 3 = Select Boot Configuration register + * 1 = Use Processor System Bus to deliver interrupts + */ + io_apic_write(3, 1); + + return 0; +} + +static void valleyview_enable_serial_irqs(struct udevice *dev, + struct ibase_regs *ibase) +{ +#ifdef SETUPSERIQ /* NOT defined. Remove when the TODO is done */ + /* + * TODO: SERIRQ seems to have a number of problems on baytrail. + * With it enabled, we get some spurious interrupts (ps2) + * in seabios. It also caused IOCHK# NMIs. Remove it + * until we understand how it needs to be configured. + */ + u8 reg; + + /* Disable the IOCHK# NMI. Let the NMI handler enable it if needed */ + reg = inb(0x61); + reg &= 0x0f; /* Higher Nibble must be 0 */ + reg |= 1 << 3; /* IOCHK# NMI Disable for now */ + outb(reg, 0x61); + + setbits_le32(&ibase->oic, SIRQEN); + writeb(SCNT_CONTINUOUS_MODE, &ibase->scnt); + +#if !IS_ENABLED(CONFIG_SERIRQ_CONTINUOUS_MODE) + /* + * SoC requires that the System BIOS first set the SERIRQ logic to + * continuous mode operation for at least one frame before switching + * it into quiet mode operation. + */ + outb(0x00, 0xed); /* I/O Delay to get the 1 frame */ + writeb(SCNT_QUIET_MODE, &ibase->scnt); +#endif +#endif /* DON'T SET UP IRQS */ +} + +/** + * Take an INT_PIN number (0, 1 - 4) and convert + * it to a string ("NO PIN", "PIN A" - "PIN D") + * + * @param pin PCI Interrupt Pin number (0, 1 - 4) + * @return A string corresponding to the pin number or "Invalid" + */ +static const char *pin_to_str(int pin) +{ + const char *str[5] = { + "NO PIN", + "PIN A", + "PIN B", + "PIN C", + "PIN D", + }; + + if (pin >= 0 && pin <= 4) + return str[pin]; + else + return "Invalid PIN, not 0 - 4"; +} + +/** + * Get the PCI INT_PIN swizzle for a device defined as: + * pin_parent = (pin_child + devn_child) % 4 + 1 + * where PIN A = 1 ... PIN_D = 4 + * + * Given a PCI device structure 'dev', find the interrupt pin + * that will be triggered on its parent bridge device when + * generating an interrupt. For example: Device 1:3.2 may + * use INT_PIN A but will trigger PIN D on its parent bridge + * device. In this case, this function will return 4 (PIN D). + * + * @param dev A PCI device structure to swizzle interrupt pins for + * @param *parent_bridge The PCI device structure for the bridge + * device 'dev' is attached to + * @return The interrupt pin number (1 - 4) that 'dev' will + * trigger when generating an interrupt + */ +static int swizzle_irq_pins(struct udevice *dev, + struct udevice **parent_bridgep) +{ + struct udevice *parent; /* Our current device's parent device */ + struct udevice *child; /* The child device of the parent */ + uint8_t swizzled_pin = 0; /* Pin swizzled across a bridge */ + + /* Start with PIN A = 0 ... D = 3 */ + dm_pci_read_config8(dev, PCI_INTERRUPT_PIN, &swizzled_pin); + swizzled_pin -= 1; + + /* While our current device has parent devices */ + child = dev; + for (parent = child->parent; parent; parent = parent->parent) { + struct pci_child_platdata *pplat, *cplat; + + cplat = dev_get_parent_platdata(child); + pplat = dev_get_parent_platdata(parent); + + /* Swizzle the INT_PIN for any bridges not on root bus */ + swizzled_pin = (PCI_DEV(cplat->devfn) + swizzled_pin) % 4; + debug("\tWith INT_PIN swizzled to %s\n" + "\tAttached to bridge device %01X:%02Xh.%02Xh\n", + pin_to_str(swizzled_pin + 1), parent->seq, + PCI_DEV(pplat->devfn), PCI_FUNC(pplat->devfn)); + + /* Continue until we find the root bus */ + if (device_get_uclass_id(parent->parent->parent) == + UCLASS_PCI) { + /* + * We will go on to the next parent so this parent + * becomes the child + */ + child = parent; + continue; + } else { + /* + * Found the root bridge device, + * fill in the structure and exit + */ + *parent_bridgep = parent; + break; + } + } + + /* End with PIN A = 1 ... D = 4 */ + return swizzled_pin + 1; +} + +/** + * get_pci_irq_pins() - Get interrupt pin number for a device + * + * Given a device structure 'dev', find its interrupt pin and its parent bridge + * 'parent_bdg' device structure. If it is behind a bridge, it will return the + * interrupt pin number (1 - 4) of the parent bridge that the device interrupt + * pin has been swizzled to, otherwise it will return the interrupt pin that is + * programmed into the PCI config space of the target device. If 'dev' is + * behind a bridge, it will fill in 'parent_bdg' with the device structure of + * the bridge it is behind, otherwise it will copy 'dev' into 'parent_bdg'. + * + * @param dev Device to check + * @param *parent_bdgp Parent bridge for device + * @return interrupt pin number (1 - 4) that @dev will trigger when + * generating an interrupt, or -EINVAL is the pin is invalid, or + * -ENOENT if there was no paremt bridge + */ +static int get_pci_irq_pins(struct udevice *dev, struct udevice **parent_bdgp) +{ + uint8_t bus = 0; /* The bus this device is on */ + uint16_t devfn = 0; /* This device's device and function numbers */ + uint8_t int_pin = 0; /* Interrupt pin used by the device */ + uint8_t target_pin = 0; /* Interrupt pin we want to assign an IRQ to */ + + bus = dev->parent->seq; + devfn = pci_get_bdf(dev); + + /* Get and validate the interrupt pin used. Only 1-4 are allowed */ + dm_pci_read_config8(dev, PCI_INTERRUPT_PIN, &int_pin); + if (int_pin < 1 || int_pin > 4) + return -EINVAL; + + debug("PCI IRQ: Found device %01X:%02X.%02X using %s\n", bus, + PCI_DEV(devfn), PCI_FUNC(devfn), pin_to_str(int_pin)); + + /* If this device is on a bridge, swizzle its INT_PIN */ + if (bus) { + /* Swizzle its INT_PINs */ + target_pin = swizzle_irq_pins(dev, parent_bdgp); + + /* Make sure the swizzle returned valid structures */ + if (*parent_bdgp == NULL) { + debug("Warning: Could not find parent bridge for this device!\n"); + return -ENOENT; + } + } else { /* Device is not behind a bridge */ + target_pin = int_pin; /* Return its own interrupt pin */ + *parent_bdgp = dev; /* Return its own structure */ + } + + /* Target pin is the interrupt pin we want to assign an IRQ to */ + return target_pin; +} + +/* + * Write PCI config space IRQ assignments. PCI devices have the INT_LINE + * (0x3C) and INT_PIN (0x3D) registers which report interrupt routing + * information to operating systems and drivers. The INT_PIN register is + * generally read only and reports which interrupt pin A - D it uses. The + * INT_LINE register is configurable and reports which IRQ (generally the + * PIC IRQs 1 - 15) it will use. This needs to take interrupt pin swizzling + * on devices that are downstream on a PCI bridge into account. + * + * This function will loop through all enabled PCI devices and program the + * INT_LINE register with the correct PIC IRQ number for the INT_PIN that it + * uses. It then configures each interrupt in the pic to be level triggered. + */ +static void write_pci_config_irqs(const struct baytrail_irq_route *ir) +{ + struct udevice *dev; + + for (pci_find_first_device(&dev); dev; pci_find_next_device(&dev)) { + pci_dev_t current_bdf = pci_get_bdf(dev); + struct udevice *targ_dev; + uint8_t original_int_pin; + pci_dev_t parent_bdf; + uint8_t new_int_pin; + uint8_t device_num; + uint8_t pirq; + uint8_t int_line; + + /* Get the INT_PIN and device structure to look for */ + targ_dev = NULL; + new_int_pin = get_pci_irq_pins(dev, &targ_dev); + if (targ_dev == NULL || new_int_pin < 1) + continue; + + /* Get the original INT_PIN for record keeping */ + dm_pci_read_config8(dev, PCI_INTERRUPT_PIN, &original_int_pin); + + parent_bdf = pci_get_bdf(targ_dev); + device_num = PCI_DEV(parent_bdf); + + if (ir->pcidev[device_num] == 0) { + debug("Warning: PCI Device %d does not have an IRQ entry, skipping it\n", + device_num); + continue; + } + + /* + * Find the PIRQ that is attached to the INT_PIN this device + * uses. + * TODO(sjg): Rationalise this with pirq_route_irqs() and + * pci_assign_irqs(). + */ + pirq = (ir->pcidev[device_num] >> ((new_int_pin - 1) * 4)) & + 0xf; + + /* Get the INT_LINE this device/function will use */ + int_line = ir->pic[pirq]; + + if (int_line != PIRQ_PIC_IRQDISABLE) { + /* + * Set this IRQ to level triggered since it is used by + * a PCI device + */ + configure_irq_trigger(int_line, true); + dm_pci_write_config8(dev, PCI_INTERRUPT_LINE, int_line); + } else { + dm_pci_write_config8(dev, PCI_INTERRUPT_LINE, + PCI_INTERRUPT_LINE_DISABLE); + } + + debug("\tINT_PIN\t\t: %d (%s)\n", + original_int_pin, pin_to_str(original_int_pin)); + if (parent_bdf != current_bdf) { + debug("\tSwizzled to\t: %d (%s)\n", new_int_pin, + pin_to_str(new_int_pin)); + } + debug("\tPIRQ\t\t: %c\n", 'A' + pirq); + debug("\tINT_LINE\t: 0x%X (IRQ %d)\n", int_line, int_line); + } +} + +static void valleyview_pirq_init(const struct baytrail_irq_route *ir, + struct ibase_regs *ibase) +{ + int i; + + /* Set up the PIRQ PIC routing based on static config */ + debug("Start writing IRQ assignments\n" + "PIRQ\tA \tB \tC \tD \tE \tF \tG \tH\n" + "IRQ "); + for (i = 0; i < NUM_PIRQS; i++) { + writeb(ir->pic[i], &ibase->pirq[i]); + debug("\t%d", ir->pic[i]); + } + debug("\n\n"); + + /* Set up the per device PIRQ routing based on static config */ + debug("\t\t\tPIRQ[A-H] routed to each INT_PIN[A-D]\n" + "Dev\tINTA (IRQ)\tINTB (IRQ)\tINTC (IRQ)\tINTD (IRQ)\n"); + + for (i = 0; i < NUM_OF_PCI_DEVS; i++) { + writew(ir->pcidev[i], &ibase->ir[i]); + + /* If the entry is more than just 0, print it out */ + if (ir->pcidev[i]) { + int j, pirq; + + debug(" %d: ", i); + for (j = 0; j < 4; j++) { + pirq = (ir->pcidev[i] >> (j * 4)) & 0xF; + debug("\t%-4c (%d)", 'A' + pirq, ir->pic[pirq]); + } + debug("\n"); + } + } + + /* Write IRQ assignments to PCI config space */ + write_pci_config_irqs(ir); +} + +static int valleyview_init(struct udevice *dev) +{ + int ret; + u32 addr; + struct ibase_regs *ibase; + + debug("%s\n", __func__); + + dm_pci_read_config32(dev, IBASE, &addr); + addr &= ~0xf; + ibase = (struct ibase_regs *)addr; + debug("ibase = %p\n", ibase); + + writeb(0, &ibase->mc); + + /* Enable special cycles */ + dm_pci_write_config16(dev, PCI_COMMAND, PCI_COMMAND_IO | + PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER | PCI_COMMAND_SPECIAL); + + /* IO APIC initialisation */ + ret = valleyview_enable_ioapic(dev, ibase); + if (ret) + return ret; + + valleyview_enable_serial_irqs(dev, ibase); + + /* Setup the PIRQ */ + valleyview_pirq_init(&irq_info, ibase); + + return fsp_init_phase_pci(); +} + +int arch_misc_init(void) +{ + struct udevice *dev; + int ret; + + ret = pci_bus_find_bdf(PCI_BDF(0, 0x1f, 0), &dev); + if (ret) { + debug("Cannot find Intel Legacy Block\n"); + return ret; + } + + return valleyview_init(dev); +} diff --git a/arch/x86/dts/minnowmax.dts b/arch/x86/dts/minnowmax.dts index 9527233..fb2510b 100644 --- a/arch/x86/dts/minnowmax.dts +++ b/arch/x86/dts/minnowmax.dts @@ -7,6 +7,7 @@ /dts-v1/; #include <dt-bindings/gpio/x86-gpio.h> +#include <dt-bindings/interrupt-router/intel-irq.h> /include/ "skeleton.dtsi" /include/ "serial.dtsi" @@ -120,6 +121,33 @@ ranges = <0x02000000 0x0 0xd0000000 0xd0000000 0 0x10000000 0x42000000 0x0 0xc0000000 0xc0000000 0 0x10000000 0x01000000 0x0 0x2000 0x2000 0 0xe000>; + + /* TODO(sjg@chromium.org): Currently unused */ + irq-router@1f,0 { + reg = <0x0000f800 0 0 0 0>; + compatible = "intel,irq-router"; + intel,pirq-config = "ibase"; + intel,ibase-offset = <0x50>; + intel,pirq-link = <0x20 32>; + intel,pirq-mask = <0xdcb0>; + intel,pirq-routing = < + PCI_BDF(0, 0x02, 0) INTB PIRQA + PCI_BDF(0, 0x10, 0) INTA PIRQD + PCI_BDF(0, 0x11, 0) INTA PIRQB + PCI_BDF(0, 0x12, 0) INTA PIRQC + PCI_BDF(0, 0x13, 0) INTA PIRQD + PCI_BDF(0, 0x14, 0) INTA PIRQE + PCI_BDF(0, 0x15, 0) INTC PIRQF + PCI_BDF(0, 0x17, 0) INTA PIRQF + PCI_BDF(0, 0x18, 0) INTA PIRQB + PCI_BDF(0, 0x1a, 0) INTA PIRQF + PCI_BDF(0, 0x1b, 0) INTA PIRQG + PCI_BDF(0, 0x1c, 0) INTD PIRQE + PCI_BDF(0, 0x1d, 0) INTA PIRQD + PCI_BDF(0, 0x1e, 0) INTB PIRQB + PCI_BDF(0, 0x1f, 0) INTC PIRQH + >; + }; }; spi { diff --git a/include/configs/minnowmax.h b/include/configs/minnowmax.h index cdbb65d..ffa5510 100644 --- a/include/configs/minnowmax.h +++ b/include/configs/minnowmax.h @@ -16,6 +16,7 @@ #define CONFIG_SYS_MONITOR_LEN (1 << 20) #define CONFIG_BOARD_EARLY_INIT_F #define CONFIG_ARCH_EARLY_INIT_R +#define CONFIG_ARCH_MISC_INIT #define CONFIG_SMSC_LPC47M
At present minnowmax does not correct set up PCI interrupts. This should be done in U-Boot so that devices work correctly in Linux. Note: This code needs to make use of the recent pirq_routing work. It does not seem to support all the required features, so this RFC will hopefully help figure this out. Signed-off-by: Simon Glass <sjg@chromium.org> --- arch/x86/cpu/baytrail/valleyview.c | 492 +++++++++++++++++++++++++++++++++++++ arch/x86/dts/minnowmax.dts | 28 +++ include/configs/minnowmax.h | 1 + 3 files changed, 521 insertions(+)