Messages in this thread | | | Date | Mon, 7 Apr 2014 12:38:23 -0400 | From | Murali Karicheri <> | Subject | Re: [RESEND: RFC PATCH 3/3] pcie: keystone: add pcie driver based on designware core driver |
| |
On 3/25/2014 12:54 PM, Jason Gunthorpe wrote: > On Tue, Mar 25, 2014 at 08:44:36AM +0100, Arnd Bergmann wrote: > >> I have no idea how this would work with the standard interrupt-map property, >> since the legacy interrupt host is now the same device as the pci host. >> >> Maybe it's better to move the legacy irqchip handling entirely out of >> the driver and use a separate device node for the registers so it can >> come with its own #interrupt-cells, and then refer to this irqchip from >> the interrupt-map. > The other DW PCI-E drivers are being fixed to use interrupt-map, so I > think this driver should be fixed before it goes in as well. Jason,
Could you send me a patch reference or branch that I can review to better understand changes required in my driver to use the bindings you have described below?
Murali > Other PCI-E hosts have handled this particular problem with a > construction like this: > > pcie@21800000 { > compatible = "ti,keystone2-pcie"; > device_type = "pci"; > #address-cells = <3>; > #size-cells = <2>; > #interrupt-cells = <1>; > reg = .. > > pcie_intc: interrupt-controller { > interrupt-controller; > #address-cells = <0>; > #interrupt-cells = <1>; > } > > interrupts = ... enough to decode INTx; > > interrtup-map = <0 0 0 1 &pcie_intc 1>, // INT A > <0 0 0 2 &pcie_intc 2>, // INT B > <0 0 0 3 &pcie_intc 3>, // INT C > <0 0 0 4 &pcie_intc 4>; // INT D > } > > When the legacy irq domain is created it should be bound to the > pcie_intc node, not to pcie node. > > ks_pcie_map_irq should be removed. The latest dw core driver still had map_irq() being provided by the driver. So any patch reference will help me understand that you describe here.
Thanks
Murali > >>> +#ifdef CONFIG_PCIE_KEYSTONE >>> +/* >>> + * The KeyStone PCIe controller has maximum read request size of 256 bytes. >>> + * Force this configuration for all EP including bridges. >>> + */ >>> +static void quirk_limit_readrequest(struct pci_dev *dev) >>> +{ >>> + pcie_set_readrq(dev, 256); >>> +} >>> +DECLARE_PCI_FIXUP_FINAL(PCI_ANY_ID, PCI_ANY_ID, quirk_limit_readrequest); >>> +#endif /* CONFIG_TI_KEYSTONE_PCIE */ >> You can't do this: >> >> A quirk for a specific hardware must not be enabled by compile-time options. >> You have to find a different way to do this. > This configuration should happen automatically by the PCI core if the > config spaces are correct. See pcie_write_mrrs, pcie_write_mps, > pcie_bus_configure_settings, etc. > > Your host will need to conform to the PCI spec and provide a root > port bridge header for this to work. > > Also, even in the keystone case the MRRS should not be globally forced > like this, there may be other bridges in the system with a 128 byte > MPS. > > Jason
| |