diff mbox

[v6,3/6] pci:host: Add Altera PCIe host controller driver

Message ID 1441103408-8433-4-git-send-email-lftan@altera.com (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show

Commit Message

Ley Foon Tan Sept. 1, 2015, 10:30 a.m. UTC
This patch adds the Altera PCIe host controller driver.

Signed-off-by: Ley Foon Tan <lftan@altera.com>
---
 drivers/pci/host/Kconfig       |   8 +
 drivers/pci/host/Makefile      |   1 +
 drivers/pci/host/pcie-altera.c | 593 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 602 insertions(+)
 create mode 100644 drivers/pci/host/pcie-altera.c

Comments

Lorenzo Pieralisi Sept. 1, 2015, 4:33 p.m. UTC | #1
On Tue, Sep 01, 2015 at 11:30:05AM +0100, Ley Foon Tan wrote:

[...]

> +static void altera_pcie_retrain(struct pci_dev *dev)
> +{
> +       u16 linkcap, linkstat;
> +
> +       /*
> +        * Set the retrain bit if the PCIe rootport support > 2.5GB/s, but
> +        * current speed is 2.5 GB/s.
> +        */
> +       pcie_capability_read_word(dev, PCI_EXP_LNKCAP, &linkcap);
> +
> +       if ((linkcap & PCI_EXP_LNKCAP_SLS) <= PCI_EXP_LNKCAP_SLS_2_5GB)
> +               return;
> +
> +       pcie_capability_read_word(dev, PCI_EXP_LNKSTA, &linkstat);
> +       if ((linkstat & PCI_EXP_LNKSTA_CLS) == PCI_EXP_LNKSTA_CLS_2_5GB)
> +               pcie_capability_set_word(dev, PCI_EXP_LNKCTL,
> +                                        PCI_EXP_LNKCTL_RL);
> +}
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_ALTERA, PCI_ANY_ID, altera_pcie_retrain);

This filtering is still arguable, since it unconditionally applies to
all Altera PCI devices (I guess there are not any apart from this
host controller).

> +
> +static void altera_pcie_fixup_res(struct pci_dev *dev)
> +{
> +       /*
> +        * Prevent enumeration of root port.
> +        */
> +       if (!dev->bus->parent && dev->devfn == 0) {
> +               int i;
> +
> +               for (i = 0; i < PCI_NUM_RESOURCES; i++) {
> +                       dev->resource[i].start = 0;
> +                       dev->resource[i].end   = 0;
> +                       dev->resource[i].flags   = 0;
> +               }
> +       }
> +}
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_ALTERA, PCI_ANY_ID,
> +                        altera_pcie_fixup_res);

Ditto, it is a quirk of your host controller, not a quirk for
all Altera PCI devices.

> +static inline void cra_writel(struct altera_pcie *pcie, u32 value, u32 reg)
> +{
> +       writel_relaxed(value, pcie->cra_base + reg);
> +}
> +
> +static inline u32 cra_readl(struct altera_pcie *pcie, u32 reg)
> +{
> +       return readl_relaxed(pcie->cra_base + reg);
> +}
> +
> +static void tlp_read_rx(struct altera_pcie *pcie,
> +                       struct tlp_rp_regpair_t *tlp_rp_regdata)
> +{
> +       tlp_rp_regdata->ctrl = cra_readl(pcie, RP_RXCPL_STATUS);
> +       tlp_rp_regdata->reg0 = cra_readl(pcie, RP_RXCPL_REG0);
> +       tlp_rp_regdata->reg1 = cra_readl(pcie, RP_RXCPL_REG1);
> +}
> +
> +static void tlp_write_tx(struct altera_pcie *pcie,
> +                        struct tlp_rp_regpair_t *tlp_rp_regdata)
> +{
> +       cra_writel(pcie, tlp_rp_regdata->reg0, RP_TX_REG0);
> +       cra_writel(pcie, tlp_rp_regdata->reg1, RP_TX_REG1);
> +       cra_writel(pcie, tlp_rp_regdata->ctrl, RP_TX_CNTRL);
> +}
> +
> +static bool altera_pcie_link_is_up(struct altera_pcie *pcie)
> +{
> +       return !!(cra_readl(pcie, RP_LTSSM) & LTSSM_L0);
> +}
> +
> +static bool altera_pcie_valid_config(struct altera_pcie *pcie,
> +                                    struct pci_bus *bus, int dev)
> +{
> +       /* If there is no link, then there is no device */
> +       if (bus->number != pcie->root_bus_nr) {
> +               if (!altera_pcie_link_is_up(pcie))
> +                       return false;
> +       }

Can you explain to pls me why you have to check this for every config
transaction ? Isn't it something that can prevent probing the
host controller altogether ?

> +
> +       /* access only one slot on each root port */
> +       if (bus->number == pcie->root_bus_nr && dev > 0)
> +               return false;
> +
> +       /*
> +        * Do not read more than one device on the bus directly attached
> +        * to RC.

You should also explain why.

> +        */
> +       if (bus->primary == pcie->root_bus_nr && dev > 0)
> +               return false;
> +
> +        return true;
> +}
> +
> +static int tlp_read_packet(struct altera_pcie *pcie, u32 *value)
> +{
> +       u8 loop;
> +       struct tlp_rp_regpair_t tlp_rp_regdata;
> +
> +       for (loop = 0; loop < TLP_LOOP; loop++) {
> +               tlp_read_rx(pcie, &tlp_rp_regdata);
> +               if (tlp_rp_regdata.ctrl & RP_RXCPL_EOP) {
> +                       if (value)
> +                               *value = tlp_rp_regdata.reg0;
> +                       return PCIBIOS_SUCCESSFUL;
> +               }
> +               udelay(5);

Could you comment please on the chosen udelay/TLP_LOOP values (ie how
did you come up with them) ?

> +       }
> +
> +       return -ENOENT;
> +}
> +
> +static void tlp_write_packet_unaligned(struct altera_pcie *pcie, u32 *headers,
> +                                      u32 data)
> +{
> +       struct tlp_rp_regpair_t tlp_rp_regdata;
> +
> +       tlp_rp_regdata.reg0 = headers[0];
> +       tlp_rp_regdata.reg1 = headers[1];
> +       tlp_rp_regdata.ctrl = RP_TX_SOP;
> +       tlp_write_tx(pcie, &tlp_rp_regdata);
> +
> +       tlp_rp_regdata.reg0 = headers[2];
> +       tlp_rp_regdata.reg1 = data;
> +       tlp_rp_regdata.ctrl = RP_TX_EOP;
> +       tlp_write_tx(pcie, &tlp_rp_regdata);
> +}
> +
> +static void tlp_write_packet_aligned(struct altera_pcie *pcie, u32 *headers,
> +                                    u32 data)
> +{
> +       struct tlp_rp_regpair_t tlp_rp_regdata;
> +
> +       tlp_rp_regdata.reg0 = headers[0];
> +       tlp_rp_regdata.reg1 = headers[1];
> +       tlp_rp_regdata.ctrl = RP_TX_SOP;
> +       tlp_write_tx(pcie, &tlp_rp_regdata);
> +
> +       tlp_rp_regdata.reg0 = headers[2];
> +       tlp_rp_regdata.reg1 = 0;
> +       tlp_rp_regdata.ctrl = 0;
> +       tlp_write_tx(pcie, &tlp_rp_regdata);
> +
> +       tlp_rp_regdata.reg0 = data;
> +       tlp_rp_regdata.reg1 = 0;
> +       tlp_rp_regdata.ctrl = RP_TX_EOP;
> +       tlp_write_tx(pcie, &tlp_rp_regdata);
> +}

I do not think you need two functions here, a flag can do.

> +static int tlp_cfg_dword_read(struct altera_pcie *pcie, u8 bus, u32 devfn,
> +                             int where, u32 *value)
> +{
> +       int ret;
> +       u32 headers[TLP_HDR_SIZE];
> +
> +       if (bus == pcie->root_bus_nr)
> +               headers[0] = TLP_CFG_DW0(TLP_FMTTYPE_CFGRD0);
> +       else
> +               headers[0] = TLP_CFG_DW0(TLP_FMTTYPE_CFGRD1);
> +
> +       headers[1] = TLP_CFG_DW1(TLP_REQ_ID(pcie->root_bus_nr, devfn),
> +                                       TLP_READ_TAG);
> +       headers[2] = TLP_CFG_DW2(bus, devfn, where);
> +
> +       tlp_write_packet_unaligned(pcie, headers, 0);
> +
> +       ret = tlp_read_packet(pcie, value);
> +       if (ret != PCIBIOS_SUCCESSFUL)
> +               *value = ~0UL;  /* return 0xFFFFFFFF if error */
> +
> +       return ret;
> +}
> +
> +static int tlp_cfg_dword_write(struct altera_pcie *pcie, u8 bus, u32 devfn,
> +                              int where, u32 value)
> +{
> +       u32 headers[TLP_HDR_SIZE];
> +       int ret;
> +
> +       if (bus == pcie->root_bus_nr)
> +               headers[0] = TLP_CFG_DW0(TLP_FMTTYPE_CFGWR0);
> +       else
> +               headers[0] = TLP_CFG_DW0(TLP_FMTTYPE_CFGWR1);
> +
> +       headers[1] = TLP_CFG_DW1(TLP_REQ_ID(pcie->root_bus_nr, devfn),
> +                                       TLP_WRITE_TAG);
> +       headers[2] = TLP_CFG_DW2(bus, devfn, where);
> +
> +       /* check alignment to Qword */
> +       if ((where & 0x7) == 0)
> +               tlp_write_packet_aligned(pcie, headers, value);
> +       else
> +               tlp_write_packet_unaligned(pcie, headers, value);
> +
> +       ret = tlp_read_packet(pcie, NULL);
> +       if (ret != PCIBIOS_SUCCESSFUL)
> +               return ret;
> +
> +       /* Keep an eye out for changes to the root bus number */
> +       if ((bus == pcie->root_bus_nr) && (where == PCI_PRIMARY_BUS))
> +               pcie->root_bus_nr = (u8)(value);

Essentially you are polling the rootport bridge programming here, correct ?
I think you should describe this a bit better along with the assumptions
you are making.

> +
> +       return PCIBIOS_SUCCESSFUL;
> +}
> +
> +static int altera_pcie_cfg_read(struct pci_bus *bus, unsigned int devfn,
> +                               int where, int size, u32 *value)
> +{
> +       struct altera_pcie *pcie = bus->sysdata;
> +       int ret;
> +       u32 data;
> +
> +       if (!altera_pcie_valid_config(pcie, bus, PCI_SLOT(devfn))) {
> +               *value = ~0UL;
> +               return PCIBIOS_DEVICE_NOT_FOUND;
> +       }
> +
> +       ret = tlp_cfg_dword_read(pcie, bus->number, devfn,
> +                                (where & ~DWORD_MASK), &data);
> +       if (ret != PCIBIOS_SUCCESSFUL)
> +               return ret;
> +
> +       switch (size) {
> +       case 1:
> +               *value = (data >> (8 * (where & 0x3))) & 0xff;
> +               break;
> +       case 2:
> +               *value = (data >> (8 * (where & 0x2))) & 0xffff;
> +               break;
> +       default:
> +               *value = data;
> +               break;
> +       }
> +
> +       return PCIBIOS_SUCCESSFUL;
> +}
> +
> +static int altera_pcie_cfg_write(struct pci_bus *bus, unsigned int devfn,
> +                                int where, int size, u32 value)
> +{
> +       struct altera_pcie *pcie = bus->sysdata;
> +       u32 data32;
> +       u32 shift = 8 * (where & 3);
> +       int ret;
> +
> +       if (!altera_pcie_valid_config(pcie, bus, PCI_SLOT(devfn)))
> +               return PCIBIOS_DEVICE_NOT_FOUND;
> +
> +       /* write partial */
> +       if (size != sizeof(u32)) {
> +               ret = tlp_cfg_dword_read(pcie, bus->number, devfn,
> +                                        where & ~DWORD_MASK, &data32);
> +               if (ret)
> +                       return ret;
> +       }
> +
> +       switch (size) {
> +       case 1:
> +               data32 = (data32 & ~(0xff << shift)) |
> +                               ((value & 0xff) << shift);
> +               break;
> +       case 2:
> +               data32 = (data32 & ~(0xffff << shift)) |
> +                               ((value & 0xffff) << shift);
> +               break;
> +       default:
> +               data32 = value;
> +               break;
> +       }
> +
> +       return tlp_cfg_dword_write(pcie, bus->number, devfn,
> +               (where & ~DWORD_MASK), data32);
> +}
> +
> +static struct pci_ops altera_pcie_ops = {
> +       .read = altera_pcie_cfg_read,
> +       .write = altera_pcie_cfg_write,
> +};
> +
> +static int altera_pcie_intx_map(struct irq_domain *domain, unsigned int irq,
> +                               irq_hw_number_t hwirq)
> +{
> +       irq_set_chip_and_handler(irq, &dummy_irq_chip, handle_simple_irq);
> +       irq_set_chip_data(irq, domain->host_data);
> +       set_irq_flags(irq, IRQF_VALID);
> +
> +       return 0;
> +}
> +
> +static const struct irq_domain_ops intx_domain_ops = {
> +       .map = altera_pcie_intx_map,
> +};
> +
> +static void altera_pcie_isr(unsigned int irq, struct irq_desc *desc)
> +{
> +       struct irq_chip *chip = irq_desc_get_chip(desc);
> +       struct altera_pcie *pcie;
> +       unsigned long status;
> +       u32 bit;
> +       u32 virq;
> +
> +       chained_irq_enter(chip, desc);
> +       pcie = irq_desc_get_handler_data(desc);
> +
> +       while ((status = cra_readl(pcie, P2A_INT_STATUS)
> +               & P2A_INT_STS_ALL) != 0) {
> +               for_each_set_bit(bit, &status, INTX_NUM) {
> +                       /* clear interrupts */
> +                       cra_writel(pcie, 1 << bit, P2A_INT_STATUS);
> +
> +                       virq = irq_find_mapping(pcie->irq_domain, bit + 1);
> +                       if (virq)
> +                               generic_handle_irq(virq);
> +                       else
> +                               dev_err(&pcie->pdev->dev, "unexpected IRQ\n");
> +               }
> +       }
> +
> +       chained_irq_exit(chip, desc);
> +}
> +
> +static void altera_pcie_release_of_pci_ranges(struct altera_pcie *pcie)
> +{
> +       pci_free_resource_list(&pcie->resources);
> +}
> +
> +static int altera_pcie_parse_request_of_pci_ranges(struct altera_pcie *pcie)
> +{
> +       int err, res_valid = 0;
> +       struct device *dev = &pcie->pdev->dev;
> +       struct device_node *np = dev->of_node;
> +       resource_size_t iobase;
> +       struct resource_entry *win;
> +
> +       err = of_pci_get_host_bridge_resources(np, 0, 0xff, &pcie->resources,
> +                                              &iobase);

Nit: I do not see the point of passing iobase. You do not support IO space,
if there is an IO address space in DT you want the function to barf,
you do not want the function to initialize iobase silently.

So, you should pass iobase as NULL, if there is an IO space in DT:

of_pci_get_host_bridge_resources()

will barf and that's what you want to flag FW misconfigurations up.

[...]

> +static int altera_pcie_probe(struct platform_device *pdev)
> +{
> +       struct altera_pcie *pcie;
> +       struct pci_bus *bus;
> +       int ret;
> +
> +       pcie = devm_kzalloc(&pdev->dev, sizeof(*pcie), GFP_KERNEL);
> +       if (!pcie)
> +               return -ENOMEM;
> +
> +       pcie->pdev = pdev;
> +
> +       ret = altera_pcie_parse_dt(pcie);
> +       if (ret) {
> +               dev_err(&pdev->dev, "Parsing DT failed\n");
> +               return ret;
> +       }
> +
> +       INIT_LIST_HEAD(&pcie->resources);
> +
> +       ret = altera_pcie_parse_request_of_pci_ranges(pcie);
> +       if (ret) {
> +               dev_err(&pdev->dev, "Failed add resources\n");
> +               return ret;
> +       }
> +
> +       ret = altera_pcie_init_irq_domain(pcie);
> +       if (ret) {
> +               dev_err(&pdev->dev, "Failed creating IRQ Domain\n");
> +               return ret;
> +       }
> +
> +       pcie->root_bus_nr = 0;

Nit: It is already 0.

> +
> +       /* clear all interrupts */
> +       cra_writel(pcie, P2A_INT_STS_ALL, P2A_INT_STATUS);
> +       /* enable all interrupts */
> +       cra_writel(pcie, P2A_INT_ENA_ALL, P2A_INT_ENABLE);
> +
> +       bus = pci_scan_root_bus(&pdev->dev, pcie->root_bus_nr, &altera_pcie_ops,
> +                               pcie, &pcie->resources);
> +       if (!bus)
> +               return -ENOMEM;
> +
> +       pci_fixup_irqs(pci_common_swizzle, of_irq_parse_and_map_pci);
> +       pci_assign_unassigned_bus_resources(bus);

I think you are missing a call to pcie_bus_configure_settings(),
see drivers/pci/host/pci-host-generic.c

Thanks,
Lorenzo

> +       pci_bus_add_devices(bus);

> +
> +       platform_set_drvdata(pdev, pcie);
> +       return ret;
> +}
> +
> +static int __exit altera_pcie_remove(struct platform_device *pdev)
> +{
> +       struct altera_pcie *pcie = platform_get_drvdata(pdev);
> +
> +       altera_pcie_free_irq_domain(pcie);
> +       platform_set_drvdata(pdev, NULL);
> +       return 0;
> +}
> +
> +static const struct of_device_id altera_pcie_of_match[] = {
> +       { .compatible = "altr,pcie-root-port-1.0", },
> +       {},
> +};
> +MODULE_DEVICE_TABLE(of, altera_pcie_of_match);
> +
> +static struct platform_driver altera_pcie_driver = {
> +       .probe          = altera_pcie_probe,
> +       .remove         = altera_pcie_remove,
> +       .driver = {
> +               .name   = "altera-pcie",
> +               .of_match_table = altera_pcie_of_match,
> +       },
> +};
> +
> +module_platform_driver(altera_pcie_driver);
> +
> +MODULE_AUTHOR("Ley Foon Tan <lftan@altera.com>");
> +MODULE_DESCRIPTION("Altera PCIe host controller driver");
> +MODULE_LICENSE("GPL v2");
> --
> 1.8.2.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ley Foon Tan Sept. 4, 2015, 8:29 a.m. UTC | #2
On Wed, Sep 2, 2015 at 12:33 AM, Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
>
> On Tue, Sep 01, 2015 at 11:30:05AM +0100, Ley Foon Tan wrote:
>
> [...]
>
> > +static void altera_pcie_retrain(struct pci_dev *dev)
> > +{
> > +       u16 linkcap, linkstat;
> > +
> > +       /*
> > +        * Set the retrain bit if the PCIe rootport support > 2.5GB/s, but
> > +        * current speed is 2.5 GB/s.
> > +        */
> > +       pcie_capability_read_word(dev, PCI_EXP_LNKCAP, &linkcap);
> > +
> > +       if ((linkcap & PCI_EXP_LNKCAP_SLS) <= PCI_EXP_LNKCAP_SLS_2_5GB)
> > +               return;
> > +
> > +       pcie_capability_read_word(dev, PCI_EXP_LNKSTA, &linkstat);
> > +       if ((linkstat & PCI_EXP_LNKSTA_CLS) == PCI_EXP_LNKSTA_CLS_2_5GB)
> > +               pcie_capability_set_word(dev, PCI_EXP_LNKCTL,
> > +                                        PCI_EXP_LNKCTL_RL);
> > +}
> > +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_ALTERA, PCI_ANY_ID, altera_pcie_retrain);
>
> This filtering is still arguable, since it unconditionally applies to
> all Altera PCI devices (I guess there are not any apart from this
> host controller).
This fixup is required for all Altera PCIe controller in all device families.
>
> > +
> > +static void altera_pcie_fixup_res(struct pci_dev *dev)
> > +{
> > +       /*
> > +        * Prevent enumeration of root port.
> > +        */
> > +       if (!dev->bus->parent && dev->devfn == 0) {
> > +               int i;
> > +
> > +               for (i = 0; i < PCI_NUM_RESOURCES; i++) {
> > +                       dev->resource[i].start = 0;
> > +                       dev->resource[i].end   = 0;
> > +                       dev->resource[i].flags   = 0;
> > +               }
> > +       }
> > +}
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_ALTERA, PCI_ANY_ID,
> > +                        altera_pcie_fixup_res);
>
> Ditto, it is a quirk of your host controller, not a quirk for
> all Altera PCI devices.
Same here.

>
> > +static inline void cra_writel(struct altera_pcie *pcie, u32 value, u32 reg)
> > +{
> > +       writel_relaxed(value, pcie->cra_base + reg);
> > +}
> > +
> > +static inline u32 cra_readl(struct altera_pcie *pcie, u32 reg)
> > +{
> > +       return readl_relaxed(pcie->cra_base + reg);
> > +}
> > +
> > +static void tlp_read_rx(struct altera_pcie *pcie,
> > +                       struct tlp_rp_regpair_t *tlp_rp_regdata)
> > +{
> > +       tlp_rp_regdata->ctrl = cra_readl(pcie, RP_RXCPL_STATUS);
> > +       tlp_rp_regdata->reg0 = cra_readl(pcie, RP_RXCPL_REG0);
> > +       tlp_rp_regdata->reg1 = cra_readl(pcie, RP_RXCPL_REG1);
> > +}
> > +
> > +static void tlp_write_tx(struct altera_pcie *pcie,
> > +                        struct tlp_rp_regpair_t *tlp_rp_regdata)
> > +{
> > +       cra_writel(pcie, tlp_rp_regdata->reg0, RP_TX_REG0);
> > +       cra_writel(pcie, tlp_rp_regdata->reg1, RP_TX_REG1);
> > +       cra_writel(pcie, tlp_rp_regdata->ctrl, RP_TX_CNTRL);
> > +}
> > +
> > +static bool altera_pcie_link_is_up(struct altera_pcie *pcie)
> > +{
> > +       return !!(cra_readl(pcie, RP_LTSSM) & LTSSM_L0);
> > +}
> > +
> > +static bool altera_pcie_valid_config(struct altera_pcie *pcie,
> > +                                    struct pci_bus *bus, int dev)
> > +{
> > +       /* If there is no link, then there is no device */
> > +       if (bus->number != pcie->root_bus_nr) {
> > +               if (!altera_pcie_link_is_up(pcie))
> > +                       return false;
> > +       }
>
> Can you explain to pls me why you have to check this for every config
> transaction ? Isn't it something that can prevent probing the
> host controller altogether ?
In our PCIe hardware spec, it stated that software should check the
link status before issuing a configuration request to downstream
ports.
BTW, other pci controllers have similar implementation as well, eg: dw
pci, mvebu pci.
>
> > +
> > +       /* access only one slot on each root port */
> > +       if (bus->number == pcie->root_bus_nr && dev > 0)
> > +               return false;
> > +
> > +       /*
> > +        * Do not read more than one device on the bus directly attached
> > +        * to RC.
>
> You should also explain why.
Okay.

>
> > +        */
> > +       if (bus->primary == pcie->root_bus_nr && dev > 0)
> > +               return false;
> > +
> > +        return true;
> > +}
> > +
> > +static int tlp_read_packet(struct altera_pcie *pcie, u32 *value)
> > +{
> > +       u8 loop;
> > +       struct tlp_rp_regpair_t tlp_rp_regdata;
> > +
> > +       for (loop = 0; loop < TLP_LOOP; loop++) {
> > +               tlp_read_rx(pcie, &tlp_rp_regdata);
> > +               if (tlp_rp_regdata.ctrl & RP_RXCPL_EOP) {
> > +                       if (value)
> > +                               *value = tlp_rp_regdata.reg0;
> > +                       return PCIBIOS_SUCCESSFUL;
> > +               }
> > +               udelay(5);
>
> Could you comment please on the chosen udelay/TLP_LOOP values (ie how
> did you come up with them) ?
For udelay value, we just want to have small delay between each read.
For TLP_LOOP value, minimum 2 loops to read tlp headers and 1 loop to
read data payload. So, we choose to poll 10 loops for maximum.

>
> > +       }
> > +
> > +       return -ENOENT;
> > +}
> > +
> > +static void tlp_write_packet_unaligned(struct altera_pcie *pcie, u32 *headers,
> > +                                      u32 data)
> > +{
> > +       struct tlp_rp_regpair_t tlp_rp_regdata;
> > +
> > +       tlp_rp_regdata.reg0 = headers[0];
> > +       tlp_rp_regdata.reg1 = headers[1];
> > +       tlp_rp_regdata.ctrl = RP_TX_SOP;
> > +       tlp_write_tx(pcie, &tlp_rp_regdata);
> > +
> > +       tlp_rp_regdata.reg0 = headers[2];
> > +       tlp_rp_regdata.reg1 = data;
> > +       tlp_rp_regdata.ctrl = RP_TX_EOP;
> > +       tlp_write_tx(pcie, &tlp_rp_regdata);
> > +}
> > +
> > +static void tlp_write_packet_aligned(struct altera_pcie *pcie, u32 *headers,
> > +                                    u32 data)
> > +{
> > +       struct tlp_rp_regpair_t tlp_rp_regdata;
> > +
> > +       tlp_rp_regdata.reg0 = headers[0];
> > +       tlp_rp_regdata.reg1 = headers[1];
> > +       tlp_rp_regdata.ctrl = RP_TX_SOP;
> > +       tlp_write_tx(pcie, &tlp_rp_regdata);
> > +
> > +       tlp_rp_regdata.reg0 = headers[2];
> > +       tlp_rp_regdata.reg1 = 0;
> > +       tlp_rp_regdata.ctrl = 0;
> > +       tlp_write_tx(pcie, &tlp_rp_regdata);
> > +
> > +       tlp_rp_regdata.reg0 = data;
> > +       tlp_rp_regdata.reg1 = 0;
> > +       tlp_rp_regdata.ctrl = RP_TX_EOP;
> > +       tlp_write_tx(pcie, &tlp_rp_regdata);
> > +}
>
> I do not think you need two functions here, a flag can do.
Okay, will merge these 2 functions.
>
> > +static int tlp_cfg_dword_read(struct altera_pcie *pcie, u8 bus, u32 devfn,
> > +                             int where, u32 *value)
> > +{
> > +       int ret;
> > +       u32 headers[TLP_HDR_SIZE];
> > +
> > +       if (bus == pcie->root_bus_nr)
> > +               headers[0] = TLP_CFG_DW0(TLP_FMTTYPE_CFGRD0);
> > +       else
> > +               headers[0] = TLP_CFG_DW0(TLP_FMTTYPE_CFGRD1);
> > +
> > +       headers[1] = TLP_CFG_DW1(TLP_REQ_ID(pcie->root_bus_nr, devfn),
> > +                                       TLP_READ_TAG);
> > +       headers[2] = TLP_CFG_DW2(bus, devfn, where);
> > +
> > +       tlp_write_packet_unaligned(pcie, headers, 0);
> > +
> > +       ret = tlp_read_packet(pcie, value);
> > +       if (ret != PCIBIOS_SUCCESSFUL)
> > +               *value = ~0UL;  /* return 0xFFFFFFFF if error */
> > +
> > +       return ret;
> > +}
> > +
> > +static int tlp_cfg_dword_write(struct altera_pcie *pcie, u8 bus, u32 devfn,
> > +                              int where, u32 value)
> > +{
> > +       u32 headers[TLP_HDR_SIZE];
> > +       int ret;
> > +
> > +       if (bus == pcie->root_bus_nr)
> > +               headers[0] = TLP_CFG_DW0(TLP_FMTTYPE_CFGWR0);
> > +       else
> > +               headers[0] = TLP_CFG_DW0(TLP_FMTTYPE_CFGWR1);
> > +
> > +       headers[1] = TLP_CFG_DW1(TLP_REQ_ID(pcie->root_bus_nr, devfn),
> > +                                       TLP_WRITE_TAG);
> > +       headers[2] = TLP_CFG_DW2(bus, devfn, where);
> > +
> > +       /* check alignment to Qword */
> > +       if ((where & 0x7) == 0)
> > +               tlp_write_packet_aligned(pcie, headers, value);
> > +       else
> > +               tlp_write_packet_unaligned(pcie, headers, value);
> > +
> > +       ret = tlp_read_packet(pcie, NULL);
> > +       if (ret != PCIBIOS_SUCCESSFUL)
> > +               return ret;
> > +
> > +       /* Keep an eye out for changes to the root bus number */
> > +       if ((bus == pcie->root_bus_nr) && (where == PCI_PRIMARY_BUS))
> > +               pcie->root_bus_nr = (u8)(value);
>
> Essentially you are polling the rootport bridge programming here, correct ?
> I think you should describe this a bit better along with the assumptions
> you are making.
Yes, you are right. Will update description here.
>
[...]

> > +
> > +static void altera_pcie_release_of_pci_ranges(struct altera_pcie *pcie)
> > +{
> > +       pci_free_resource_list(&pcie->resources);
> > +}
> > +
> > +static int altera_pcie_parse_request_of_pci_ranges(struct altera_pcie *pcie)
> > +{
> > +       int err, res_valid = 0;
> > +       struct device *dev = &pcie->pdev->dev;
> > +       struct device_node *np = dev->of_node;
> > +       resource_size_t iobase;
> > +       struct resource_entry *win;
> > +
> > +       err = of_pci_get_host_bridge_resources(np, 0, 0xff, &pcie->resources,
> > +                                              &iobase);
>
> Nit: I do not see the point of passing iobase. You do not support IO space,
> if there is an IO address space in DT you want the function to barf,
> you do not want the function to initialize iobase silently.
>
> So, you should pass iobase as NULL, if there is an IO space in DT:
>
> of_pci_get_host_bridge_resources()
>
> will barf and that's what you want to flag FW misconfigurations up.
Okay, will pass in NULL for iobase.
>
> [...]
>
> > +static int altera_pcie_probe(struct platform_device *pdev)
> > +{
> > +       struct altera_pcie *pcie;
> > +       struct pci_bus *bus;
> > +       int ret;
> > +
> > +       pcie = devm_kzalloc(&pdev->dev, sizeof(*pcie), GFP_KERNEL);
> > +       if (!pcie)
> > +               return -ENOMEM;
> > +
> > +       pcie->pdev = pdev;
> > +
> > +       ret = altera_pcie_parse_dt(pcie);
> > +       if (ret) {
> > +               dev_err(&pdev->dev, "Parsing DT failed\n");
> > +               return ret;
> > +       }
> > +
> > +       INIT_LIST_HEAD(&pcie->resources);
> > +
> > +       ret = altera_pcie_parse_request_of_pci_ranges(pcie);
> > +       if (ret) {
> > +               dev_err(&pdev->dev, "Failed add resources\n");
> > +               return ret;
> > +       }
> > +
> > +       ret = altera_pcie_init_irq_domain(pcie);
> > +       if (ret) {
> > +               dev_err(&pdev->dev, "Failed creating IRQ Domain\n");
> > +               return ret;
> > +       }
> > +
> > +       pcie->root_bus_nr = 0;
>
> Nit: It is already 0.
Okay. Will remove it.
>
> > +
> > +       /* clear all interrupts */
> > +       cra_writel(pcie, P2A_INT_STS_ALL, P2A_INT_STATUS);
> > +       /* enable all interrupts */
> > +       cra_writel(pcie, P2A_INT_ENA_ALL, P2A_INT_ENABLE);
> > +
> > +       bus = pci_scan_root_bus(&pdev->dev, pcie->root_bus_nr, &altera_pcie_ops,
> > +                               pcie, &pcie->resources);
> > +       if (!bus)
> > +               return -ENOMEM;
> > +
> > +       pci_fixup_irqs(pci_common_swizzle, of_irq_parse_and_map_pci);
> > +       pci_assign_unassigned_bus_resources(bus);
>
> I think you are missing a call to pcie_bus_configure_settings(),
> see drivers/pci/host/pci-host-generic.c
Other pci controller drivers like xgene and iproc don't call to this
function, but it call in
arch/arm/kernel/bios32.c:pci_common_init_dev().
Do we really need this?

Thanks for reviewing.

Regards
Ley Foon
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lorenzo Pieralisi Sept. 8, 2015, 9:19 a.m. UTC | #3
On Fri, Sep 04, 2015 at 09:29:14AM +0100, Ley Foon Tan wrote:
> On Wed, Sep 2, 2015 at 12:33 AM, Lorenzo Pieralisi
> <lorenzo.pieralisi@arm.com> wrote:

[...]

> > > +static bool altera_pcie_valid_config(struct altera_pcie *pcie,
> > > +                                    struct pci_bus *bus, int dev)
> > > +{
> > > +       /* If there is no link, then there is no device */
> > > +       if (bus->number != pcie->root_bus_nr) {
> > > +               if (!altera_pcie_link_is_up(pcie))
> > > +                       return false;
> > > +       }
> >
> > Can you explain to pls me why you have to check this for every config
> > transaction ? Isn't it something that can prevent probing the
> > host controller altogether ?
> In our PCIe hardware spec, it stated that software should check the
> link status before issuing a configuration request to downstream
> ports.
> BTW, other pci controllers have similar implementation as well, eg: dw
> pci, mvebu pci.

Understood, thanks.

[...]

> > > +static int tlp_read_packet(struct altera_pcie *pcie, u32 *value)
> > > +{
> > > +       u8 loop;
> > > +       struct tlp_rp_regpair_t tlp_rp_regdata;
> > > +
> > > +       for (loop = 0; loop < TLP_LOOP; loop++) {
> > > +               tlp_read_rx(pcie, &tlp_rp_regdata);
> > > +               if (tlp_rp_regdata.ctrl & RP_RXCPL_EOP) {
> > > +                       if (value)
> > > +                               *value = tlp_rp_regdata.reg0;
> > > +                       return PCIBIOS_SUCCESSFUL;
> > > +               }
> > > +               udelay(5);
> >
> > Could you comment please on the chosen udelay/TLP_LOOP values (ie how
> > did you come up with them) ?
> For udelay value, we just want to have small delay between each read.

I would explain how you chose the value, in particular if it can
affect next generation hosts sharing the same driver.

> For TLP_LOOP value, minimum 2 loops to read tlp headers and 1 loop to
> read data payload. So, we choose to poll 10 loops for maximum.

Add it to a comment.

[...]

> > > +static int altera_pcie_probe(struct platform_device *pdev)
> > > +{
> > > +       struct altera_pcie *pcie;
> > > +       struct pci_bus *bus;
> > > +       int ret;
> > > +
> > > +       pcie = devm_kzalloc(&pdev->dev, sizeof(*pcie), GFP_KERNEL);
> > > +       if (!pcie)
> > > +               return -ENOMEM;
> > > +
> > > +       pcie->pdev = pdev;
> > > +
> > > +       ret = altera_pcie_parse_dt(pcie);
> > > +       if (ret) {
> > > +               dev_err(&pdev->dev, "Parsing DT failed\n");
> > > +               return ret;
> > > +       }
> > > +
> > > +       INIT_LIST_HEAD(&pcie->resources);
> > > +
> > > +       ret = altera_pcie_parse_request_of_pci_ranges(pcie);
> > > +       if (ret) {
> > > +               dev_err(&pdev->dev, "Failed add resources\n");
> > > +               return ret;
> > > +       }
> > > +
> > > +       ret = altera_pcie_init_irq_domain(pcie);
> > > +       if (ret) {
> > > +               dev_err(&pdev->dev, "Failed creating IRQ Domain\n");
> > > +               return ret;
> > > +       }
> > > +
> > > +       pcie->root_bus_nr = 0;
> >
> > Nit: It is already 0.
> Okay. Will remove it.
> >
> > > +
> > > +       /* clear all interrupts */
> > > +       cra_writel(pcie, P2A_INT_STS_ALL, P2A_INT_STATUS);
> > > +       /* enable all interrupts */
> > > +       cra_writel(pcie, P2A_INT_ENA_ALL, P2A_INT_ENABLE);
> > > +
> > > +       bus = pci_scan_root_bus(&pdev->dev, pcie->root_bus_nr, &altera_pcie_ops,
> > > +                               pcie, &pcie->resources);
> > > +       if (!bus)
> > > +               return -ENOMEM;
> > > +
> > > +       pci_fixup_irqs(pci_common_swizzle, of_irq_parse_and_map_pci);
> > > +       pci_assign_unassigned_bus_resources(bus);
> >
> > I think you are missing a call to pcie_bus_configure_settings(),
> > see drivers/pci/host/pci-host-generic.c
> Other pci controller drivers like xgene and iproc don't call to this
> function, but it call in
> arch/arm/kernel/bios32.c:pci_common_init_dev().
> Do we really need this?

It is there to provide a way to configure the system MPS, through
command line parameters, it is always a good idea to have it and
it should be part of your driver so that you can tune it in case
the MPS is misconfigured or you want to apply a specific configuration
for a given system.

Have a look at pcie_bus_config and how it is used in
pcie_bus_configure_settings(), that would clarify further.

Thanks,
Lorenzo
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ley Foon Tan Sept. 9, 2015, 6:49 a.m. UTC | #4
On Tue, Sep 8, 2015 at 5:19 PM, Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
> On Fri, Sep 04, 2015 at 09:29:14AM +0100, Ley Foon Tan wrote:
>> On Wed, Sep 2, 2015 at 12:33 AM, Lorenzo Pieralisi
>> <lorenzo.pieralisi@arm.com> wrote:
>
> [...]
>
>> > > +static bool altera_pcie_valid_config(struct altera_pcie *pcie,
>> > > +                                    struct pci_bus *bus, int dev)
>> > > +{
>> > > +       /* If there is no link, then there is no device */
>> > > +       if (bus->number != pcie->root_bus_nr) {
>> > > +               if (!altera_pcie_link_is_up(pcie))
>> > > +                       return false;
>> > > +       }
>> >
>> > Can you explain to pls me why you have to check this for every config
>> > transaction ? Isn't it something that can prevent probing the
>> > host controller altogether ?
>> In our PCIe hardware spec, it stated that software should check the
>> link status before issuing a configuration request to downstream
>> ports.
>> BTW, other pci controllers have similar implementation as well, eg: dw
>> pci, mvebu pci.
>
> Understood, thanks.
>
> [...]
>
>> > > +static int tlp_read_packet(struct altera_pcie *pcie, u32 *value)
>> > > +{
>> > > +       u8 loop;
>> > > +       struct tlp_rp_regpair_t tlp_rp_regdata;
>> > > +
>> > > +       for (loop = 0; loop < TLP_LOOP; loop++) {
>> > > +               tlp_read_rx(pcie, &tlp_rp_regdata);
>> > > +               if (tlp_rp_regdata.ctrl & RP_RXCPL_EOP) {
>> > > +                       if (value)
>> > > +                               *value = tlp_rp_regdata.reg0;
>> > > +                       return PCIBIOS_SUCCESSFUL;
>> > > +               }
>> > > +               udelay(5);
>> >
>> > Could you comment please on the chosen udelay/TLP_LOOP values (ie how
>> > did you come up with them) ?
>> For udelay value, we just want to have small delay between each read.
>
> I would explain how you chose the value, in particular if it can
> affect next generation hosts sharing the same driver.
Actually, we don't have any calculation to choose the value. Just want
to give small relaxation between each read.
It is request from Marc in previous review.
>
>> For TLP_LOOP value, minimum 2 loops to read tlp headers and 1 loop to
>> read data payload. So, we choose to poll 10 loops for maximum.
>
> Add it to a comment.
Okay.
>
[...]
>> >
>> > > +
>> > > +       /* clear all interrupts */
>> > > +       cra_writel(pcie, P2A_INT_STS_ALL, P2A_INT_STATUS);
>> > > +       /* enable all interrupts */
>> > > +       cra_writel(pcie, P2A_INT_ENA_ALL, P2A_INT_ENABLE);
>> > > +
>> > > +       bus = pci_scan_root_bus(&pdev->dev, pcie->root_bus_nr, &altera_pcie_ops,
>> > > +                               pcie, &pcie->resources);
>> > > +       if (!bus)
>> > > +               return -ENOMEM;
>> > > +
>> > > +       pci_fixup_irqs(pci_common_swizzle, of_irq_parse_and_map_pci);
>> > > +       pci_assign_unassigned_bus_resources(bus);
>> >
>> > I think you are missing a call to pcie_bus_configure_settings(),
>> > see drivers/pci/host/pci-host-generic.c
>> Other pci controller drivers like xgene and iproc don't call to this
>> function, but it call in
>> arch/arm/kernel/bios32.c:pci_common_init_dev().
>> Do we really need this?
>
> It is there to provide a way to configure the system MPS, through
> command line parameters, it is always a good idea to have it and
> it should be part of your driver so that you can tune it in case
> the MPS is misconfigured or you want to apply a specific configuration
> for a given system.
>
> Have a look at pcie_bus_config and how it is used in
> pcie_bus_configure_settings(), that would clarify further.
Thanks for explanation. I will add it.

Thanks.

Regards
Ley Foon
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
index 675c2d1..43867d7 100644
--- a/drivers/pci/host/Kconfig
+++ b/drivers/pci/host/Kconfig
@@ -145,4 +145,12 @@  config PCIE_IPROC_BCMA
 	  Say Y here if you want to use the Broadcom iProc PCIe controller
 	  through the BCMA bus interface
 
+config PCIE_ALTERA
+	tristate "Altera PCIe controller"
+	depends on ARCH_SOCFPGA
+	select PCI_DOMAINS
+	help
+	  Say Y here if you want to enable PCIe controller support for Altera
+	  SoCFPGA family of SoCs.
+
 endmenu
diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
index 140d66f..6954f76 100644
--- a/drivers/pci/host/Makefile
+++ b/drivers/pci/host/Makefile
@@ -17,3 +17,4 @@  obj-$(CONFIG_PCI_VERSATILE) += pci-versatile.o
 obj-$(CONFIG_PCIE_IPROC) += pcie-iproc.o
 obj-$(CONFIG_PCIE_IPROC_PLATFORM) += pcie-iproc-platform.o
 obj-$(CONFIG_PCIE_IPROC_BCMA) += pcie-iproc-bcma.o
+obj-$(CONFIG_PCIE_ALTERA) += pcie-altera.o
diff --git a/drivers/pci/host/pcie-altera.c b/drivers/pci/host/pcie-altera.c
new file mode 100644
index 0000000..f73c499
--- /dev/null
+++ b/drivers/pci/host/pcie-altera.c
@@ -0,0 +1,593 @@ 
+/*
+ * Copyright Altera Corporation (C) 2013-2015. All rights reserved
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+#include <linux/delay.h>
+#include <linux/interrupt.h>
+#include <linux/irqchip/chained_irq.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/of_pci.h>
+#include <linux/pci.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+#define A2P_ADDR_MAP_LO0		0x1000
+#define A2P_ADDR_MAP_HI0		0x1004
+#define RP_TX_REG0			0x2000
+#define RP_TX_REG1			0x2004
+#define RP_TX_CNTRL			0x2008
+#define RP_TX_EOP			0x2
+#define RP_TX_SOP			0x1
+#define RP_RXCPL_STATUS		0x2010
+#define RP_RXCPL_EOP			0x2
+#define RP_RXCPL_SOP			0x1
+#define RP_RXCPL_REG0			0x2014
+#define RP_RXCPL_REG1			0x2018
+#define P2A_INT_STATUS			0x3060
+#define P2A_INT_STS_ALL			0xF
+#define P2A_INT_ENABLE			0x3070
+#define P2A_INT_ENA_ALL			0xF
+#define RP_LTSSM			0x3C64
+#define LTSSM_L0			0xF
+
+/* TLP configuration type 0 and 1 */
+#define TLP_FMTTYPE_CFGRD0		0x04	/* Configuration Read Type 0 */
+#define TLP_FMTTYPE_CFGWR0		0x44	/* Configuration Write Type 0 */
+#define TLP_FMTTYPE_CFGRD1		0x05	/* Configuration Read Type 1 */
+#define TLP_FMTTYPE_CFGWR1		0x45	/* Configuration Write Type 1 */
+#define TLP_PAYLOAD_SIZE		0x01
+#define TLP_READ_TAG			0x1D
+#define TLP_WRITE_TAG			0x10
+#define TLP_CFG_DW0(fmttype)		(((fmttype) << 24) | TLP_PAYLOAD_SIZE)
+#define TLP_CFG_DW1(reqid, tag)		(((reqid) << 16) | (tag << 8) | 0xF)
+#define TLP_CFG_DW2(bus, devfn, offset)	\
+				(((bus) << 24) | ((devfn) << 16) | (offset))
+#define TLP_REQ_ID(bus, devfn)		(((bus) << 8) | (devfn))
+#define TLP_COMPL_STATUS(hdr)		(((hdr) & 0xE0) >> 13)
+#define TLP_HDR_SIZE			3
+#define TLP_LOOP			10
+
+#define INTX_NUM		4
+
+#define DWORD_MASK		3
+
+struct altera_pcie {
+	struct platform_device	*pdev;
+	void __iomem		*cra_base;
+	int			irq;
+	u8			root_bus_nr;
+	struct irq_domain		*irq_domain;
+	struct resource		bus_range;
+	struct list_head		resources;
+};
+
+struct tlp_rp_regpair_t {
+	u32 ctrl;
+	u32 reg0;
+	u32 reg1;
+};
+
+static void altera_pcie_retrain(struct pci_dev *dev)
+{
+	u16 linkcap, linkstat;
+
+	/*
+	 * Set the retrain bit if the PCIe rootport support > 2.5GB/s, but
+	 * current speed is 2.5 GB/s.
+	 */
+	pcie_capability_read_word(dev, PCI_EXP_LNKCAP, &linkcap);
+
+	if ((linkcap & PCI_EXP_LNKCAP_SLS) <= PCI_EXP_LNKCAP_SLS_2_5GB)
+		return;
+
+	pcie_capability_read_word(dev, PCI_EXP_LNKSTA, &linkstat);
+	if ((linkstat & PCI_EXP_LNKSTA_CLS) == PCI_EXP_LNKSTA_CLS_2_5GB)
+		pcie_capability_set_word(dev, PCI_EXP_LNKCTL,
+					 PCI_EXP_LNKCTL_RL);
+}
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_ALTERA, PCI_ANY_ID, altera_pcie_retrain);
+
+static void altera_pcie_fixup_res(struct pci_dev *dev)
+{
+	/*
+	 * Prevent enumeration of root port.
+	 */
+	if (!dev->bus->parent && dev->devfn == 0) {
+		int i;
+
+		for (i = 0; i < PCI_NUM_RESOURCES; i++) {
+			dev->resource[i].start = 0;
+			dev->resource[i].end   = 0;
+			dev->resource[i].flags   = 0;
+		}
+	}
+}
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_ALTERA, PCI_ANY_ID,
+			 altera_pcie_fixup_res);
+
+static inline void cra_writel(struct altera_pcie *pcie, u32 value, u32 reg)
+{
+	writel_relaxed(value, pcie->cra_base + reg);
+}
+
+static inline u32 cra_readl(struct altera_pcie *pcie, u32 reg)
+{
+	return readl_relaxed(pcie->cra_base + reg);
+}
+
+static void tlp_read_rx(struct altera_pcie *pcie,
+			struct tlp_rp_regpair_t *tlp_rp_regdata)
+{
+	tlp_rp_regdata->ctrl = cra_readl(pcie, RP_RXCPL_STATUS);
+	tlp_rp_regdata->reg0 = cra_readl(pcie, RP_RXCPL_REG0);
+	tlp_rp_regdata->reg1 = cra_readl(pcie, RP_RXCPL_REG1);
+}
+
+static void tlp_write_tx(struct altera_pcie *pcie,
+			 struct tlp_rp_regpair_t *tlp_rp_regdata)
+{
+	cra_writel(pcie, tlp_rp_regdata->reg0, RP_TX_REG0);
+	cra_writel(pcie, tlp_rp_regdata->reg1, RP_TX_REG1);
+	cra_writel(pcie, tlp_rp_regdata->ctrl, RP_TX_CNTRL);
+}
+
+static bool altera_pcie_link_is_up(struct altera_pcie *pcie)
+{
+	return !!(cra_readl(pcie, RP_LTSSM) & LTSSM_L0);
+}
+
+static bool altera_pcie_valid_config(struct altera_pcie *pcie,
+				     struct pci_bus *bus, int dev)
+{
+	/* If there is no link, then there is no device */
+	if (bus->number != pcie->root_bus_nr) {
+		if (!altera_pcie_link_is_up(pcie))
+			return false;
+	}
+
+	/* access only one slot on each root port */
+	if (bus->number == pcie->root_bus_nr && dev > 0)
+		return false;
+
+	/*
+	 * Do not read more than one device on the bus directly attached
+	 * to RC.
+	 */
+	if (bus->primary == pcie->root_bus_nr && dev > 0)
+		return false;
+
+	 return true;
+}
+
+static int tlp_read_packet(struct altera_pcie *pcie, u32 *value)
+{
+	u8 loop;
+	struct tlp_rp_regpair_t tlp_rp_regdata;
+
+	for (loop = 0; loop < TLP_LOOP; loop++) {
+		tlp_read_rx(pcie, &tlp_rp_regdata);
+		if (tlp_rp_regdata.ctrl & RP_RXCPL_EOP) {
+			if (value)
+				*value = tlp_rp_regdata.reg0;
+			return PCIBIOS_SUCCESSFUL;
+		}
+		udelay(5);
+	}
+
+	return -ENOENT;
+}
+
+static void tlp_write_packet_unaligned(struct altera_pcie *pcie, u32 *headers,
+				       u32 data)
+{
+	struct tlp_rp_regpair_t tlp_rp_regdata;
+
+	tlp_rp_regdata.reg0 = headers[0];
+	tlp_rp_regdata.reg1 = headers[1];
+	tlp_rp_regdata.ctrl = RP_TX_SOP;
+	tlp_write_tx(pcie, &tlp_rp_regdata);
+
+	tlp_rp_regdata.reg0 = headers[2];
+	tlp_rp_regdata.reg1 = data;
+	tlp_rp_regdata.ctrl = RP_TX_EOP;
+	tlp_write_tx(pcie, &tlp_rp_regdata);
+}
+
+static void tlp_write_packet_aligned(struct altera_pcie *pcie, u32 *headers,
+				     u32 data)
+{
+	struct tlp_rp_regpair_t tlp_rp_regdata;
+
+	tlp_rp_regdata.reg0 = headers[0];
+	tlp_rp_regdata.reg1 = headers[1];
+	tlp_rp_regdata.ctrl = RP_TX_SOP;
+	tlp_write_tx(pcie, &tlp_rp_regdata);
+
+	tlp_rp_regdata.reg0 = headers[2];
+	tlp_rp_regdata.reg1 = 0;
+	tlp_rp_regdata.ctrl = 0;
+	tlp_write_tx(pcie, &tlp_rp_regdata);
+
+	tlp_rp_regdata.reg0 = data;
+	tlp_rp_regdata.reg1 = 0;
+	tlp_rp_regdata.ctrl = RP_TX_EOP;
+	tlp_write_tx(pcie, &tlp_rp_regdata);
+}
+
+static int tlp_cfg_dword_read(struct altera_pcie *pcie, u8 bus, u32 devfn,
+			      int where, u32 *value)
+{
+	int ret;
+	u32 headers[TLP_HDR_SIZE];
+
+	if (bus == pcie->root_bus_nr)
+		headers[0] = TLP_CFG_DW0(TLP_FMTTYPE_CFGRD0);
+	else
+		headers[0] = TLP_CFG_DW0(TLP_FMTTYPE_CFGRD1);
+
+	headers[1] = TLP_CFG_DW1(TLP_REQ_ID(pcie->root_bus_nr, devfn),
+					TLP_READ_TAG);
+	headers[2] = TLP_CFG_DW2(bus, devfn, where);
+
+	tlp_write_packet_unaligned(pcie, headers, 0);
+
+	ret = tlp_read_packet(pcie, value);
+	if (ret != PCIBIOS_SUCCESSFUL)
+		*value = ~0UL;	/* return 0xFFFFFFFF if error */
+
+	return ret;
+}
+
+static int tlp_cfg_dword_write(struct altera_pcie *pcie, u8 bus, u32 devfn,
+			       int where, u32 value)
+{
+	u32 headers[TLP_HDR_SIZE];
+	int ret;
+
+	if (bus == pcie->root_bus_nr)
+		headers[0] = TLP_CFG_DW0(TLP_FMTTYPE_CFGWR0);
+	else
+		headers[0] = TLP_CFG_DW0(TLP_FMTTYPE_CFGWR1);
+
+	headers[1] = TLP_CFG_DW1(TLP_REQ_ID(pcie->root_bus_nr, devfn),
+					TLP_WRITE_TAG);
+	headers[2] = TLP_CFG_DW2(bus, devfn, where);
+
+	/* check alignment to Qword */
+	if ((where & 0x7) == 0)
+		tlp_write_packet_aligned(pcie, headers, value);
+	else
+		tlp_write_packet_unaligned(pcie, headers, value);
+
+	ret = tlp_read_packet(pcie, NULL);
+	if (ret != PCIBIOS_SUCCESSFUL)
+		return ret;
+
+	/* Keep an eye out for changes to the root bus number */
+	if ((bus == pcie->root_bus_nr) && (where == PCI_PRIMARY_BUS))
+		pcie->root_bus_nr = (u8)(value);
+
+	return PCIBIOS_SUCCESSFUL;
+}
+
+static int altera_pcie_cfg_read(struct pci_bus *bus, unsigned int devfn,
+				int where, int size, u32 *value)
+{
+	struct altera_pcie *pcie = bus->sysdata;
+	int ret;
+	u32 data;
+
+	if (!altera_pcie_valid_config(pcie, bus, PCI_SLOT(devfn))) {
+		*value = ~0UL;
+		return PCIBIOS_DEVICE_NOT_FOUND;
+	}
+
+	ret = tlp_cfg_dword_read(pcie, bus->number, devfn,
+				 (where & ~DWORD_MASK), &data);
+	if (ret != PCIBIOS_SUCCESSFUL)
+		return ret;
+
+	switch (size) {
+	case 1:
+		*value = (data >> (8 * (where & 0x3))) & 0xff;
+		break;
+	case 2:
+		*value = (data >> (8 * (where & 0x2))) & 0xffff;
+		break;
+	default:
+		*value = data;
+		break;
+	}
+
+	return PCIBIOS_SUCCESSFUL;
+}
+
+static int altera_pcie_cfg_write(struct pci_bus *bus, unsigned int devfn,
+				 int where, int size, u32 value)
+{
+	struct altera_pcie *pcie = bus->sysdata;
+	u32 data32;
+	u32 shift = 8 * (where & 3);
+	int ret;
+
+	if (!altera_pcie_valid_config(pcie, bus, PCI_SLOT(devfn)))
+		return PCIBIOS_DEVICE_NOT_FOUND;
+
+	/* write partial */
+	if (size != sizeof(u32)) {
+		ret = tlp_cfg_dword_read(pcie, bus->number, devfn,
+					 where & ~DWORD_MASK, &data32);
+		if (ret)
+			return ret;
+	}
+
+	switch (size) {
+	case 1:
+		data32 = (data32 & ~(0xff << shift)) |
+				((value & 0xff) << shift);
+		break;
+	case 2:
+		data32 = (data32 & ~(0xffff << shift)) |
+				((value & 0xffff) << shift);
+		break;
+	default:
+		data32 = value;
+		break;
+	}
+
+	return tlp_cfg_dword_write(pcie, bus->number, devfn,
+		(where & ~DWORD_MASK), data32);
+}
+
+static struct pci_ops altera_pcie_ops = {
+	.read = altera_pcie_cfg_read,
+	.write = altera_pcie_cfg_write,
+};
+
+static int altera_pcie_intx_map(struct irq_domain *domain, unsigned int irq,
+				irq_hw_number_t hwirq)
+{
+	irq_set_chip_and_handler(irq, &dummy_irq_chip, handle_simple_irq);
+	irq_set_chip_data(irq, domain->host_data);
+	set_irq_flags(irq, IRQF_VALID);
+
+	return 0;
+}
+
+static const struct irq_domain_ops intx_domain_ops = {
+	.map = altera_pcie_intx_map,
+};
+
+static void altera_pcie_isr(unsigned int irq, struct irq_desc *desc)
+{
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	struct altera_pcie *pcie;
+	unsigned long status;
+	u32 bit;
+	u32 virq;
+
+	chained_irq_enter(chip, desc);
+	pcie = irq_desc_get_handler_data(desc);
+
+	while ((status = cra_readl(pcie, P2A_INT_STATUS)
+		& P2A_INT_STS_ALL) != 0) {
+		for_each_set_bit(bit, &status, INTX_NUM) {
+			/* clear interrupts */
+			cra_writel(pcie, 1 << bit, P2A_INT_STATUS);
+
+			virq = irq_find_mapping(pcie->irq_domain, bit + 1);
+			if (virq)
+				generic_handle_irq(virq);
+			else
+				dev_err(&pcie->pdev->dev, "unexpected IRQ\n");
+		}
+	}
+
+	chained_irq_exit(chip, desc);
+}
+
+static void altera_pcie_release_of_pci_ranges(struct altera_pcie *pcie)
+{
+	pci_free_resource_list(&pcie->resources);
+}
+
+static int altera_pcie_parse_request_of_pci_ranges(struct altera_pcie *pcie)
+{
+	int err, res_valid = 0;
+	struct device *dev = &pcie->pdev->dev;
+	struct device_node *np = dev->of_node;
+	resource_size_t iobase;
+	struct resource_entry *win;
+
+	err = of_pci_get_host_bridge_resources(np, 0, 0xff, &pcie->resources,
+					       &iobase);
+	if (err)
+		return err;
+
+	resource_list_for_each_entry(win, &pcie->resources) {
+		struct resource *parent, *res = win->res;
+
+		switch (resource_type(res)) {
+		case IORESOURCE_MEM:
+			parent = &iomem_resource;
+			res_valid |= !(res->flags & IORESOURCE_PREFETCH);
+			break;
+		default:
+			continue;
+		}
+
+		err = devm_request_resource(dev, parent, res);
+		if (err)
+			goto out_release_res;
+	}
+
+	if (!res_valid) {
+		dev_err(dev, "non-prefetchable memory resource required\n");
+		err = -EINVAL;
+		goto out_release_res;
+	}
+
+	return 0;
+
+out_release_res:
+	altera_pcie_release_of_pci_ranges(pcie);
+	return err;
+}
+
+static void altera_pcie_free_irq_domain(struct altera_pcie *pcie)
+{
+	int i;
+	u32 irq;
+
+	for (i = 0; i < INTX_NUM; i++) {
+		irq = irq_find_mapping(pcie->irq_domain, i + 1);
+		if (irq > 0)
+			irq_dispose_mapping(irq);
+	}
+
+	irq_domain_remove(pcie->irq_domain);
+}
+
+static int altera_pcie_init_irq_domain(struct altera_pcie *pcie)
+{
+	struct device *dev = &pcie->pdev->dev;
+	struct device_node *node = dev->of_node;
+
+	/* Setup INTx */
+	pcie->irq_domain = irq_domain_add_linear(node, INTX_NUM,
+					&intx_domain_ops, pcie);
+	if (!pcie->irq_domain) {
+		dev_err(dev, "Failed to get a INTx IRQ domain\n");
+		return -ENOMEM;
+	}
+
+	return 0;
+}
+
+static int altera_pcie_parse_dt(struct altera_pcie *pcie)
+{
+	struct resource *cra;
+	struct platform_device *pdev = pcie->pdev;
+
+	cra = platform_get_resource_byname(pdev, IORESOURCE_MEM, "Cra");
+	if (!cra) {
+		cra = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cra");
+		if (!cra) {
+			dev_err(&pdev->dev,
+				"no cra memory resource defined\n");
+			return -ENODEV;
+		}
+	}
+
+	pcie->cra_base = devm_ioremap_resource(&pdev->dev, cra);
+	if (IS_ERR(pcie->cra_base)) {
+		dev_err(&pdev->dev, "failed to map cra memory\n");
+		return PTR_ERR(pcie->cra_base);
+	}
+
+	/* setup IRQ */
+	pcie->irq = platform_get_irq(pdev, 0);
+	if (pcie->irq <= 0) {
+		dev_err(&pdev->dev, "failed to get IRQ: %d\n", pcie->irq);
+		return -EINVAL;
+	}
+
+	irq_set_chained_handler_and_data(pcie->irq, altera_pcie_isr, pcie);
+
+	return 0;
+}
+
+static int altera_pcie_probe(struct platform_device *pdev)
+{
+	struct altera_pcie *pcie;
+	struct pci_bus *bus;
+	int ret;
+
+	pcie = devm_kzalloc(&pdev->dev, sizeof(*pcie), GFP_KERNEL);
+	if (!pcie)
+		return -ENOMEM;
+
+	pcie->pdev = pdev;
+
+	ret = altera_pcie_parse_dt(pcie);
+	if (ret) {
+		dev_err(&pdev->dev, "Parsing DT failed\n");
+		return ret;
+	}
+
+	INIT_LIST_HEAD(&pcie->resources);
+
+	ret = altera_pcie_parse_request_of_pci_ranges(pcie);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed add resources\n");
+		return ret;
+	}
+
+	ret = altera_pcie_init_irq_domain(pcie);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed creating IRQ Domain\n");
+		return ret;
+	}
+
+	pcie->root_bus_nr = 0;
+
+	/* clear all interrupts */
+	cra_writel(pcie, P2A_INT_STS_ALL, P2A_INT_STATUS);
+	/* enable all interrupts */
+	cra_writel(pcie, P2A_INT_ENA_ALL, P2A_INT_ENABLE);
+
+	bus = pci_scan_root_bus(&pdev->dev, pcie->root_bus_nr, &altera_pcie_ops,
+				pcie, &pcie->resources);
+	if (!bus)
+		return -ENOMEM;
+
+	pci_fixup_irqs(pci_common_swizzle, of_irq_parse_and_map_pci);
+	pci_assign_unassigned_bus_resources(bus);
+	pci_bus_add_devices(bus);
+
+	platform_set_drvdata(pdev, pcie);
+	return ret;
+}
+
+static int __exit altera_pcie_remove(struct platform_device *pdev)
+{
+	struct altera_pcie *pcie = platform_get_drvdata(pdev);
+
+	altera_pcie_free_irq_domain(pcie);
+	platform_set_drvdata(pdev, NULL);
+	return 0;
+}
+
+static const struct of_device_id altera_pcie_of_match[] = {
+	{ .compatible = "altr,pcie-root-port-1.0", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, altera_pcie_of_match);
+
+static struct platform_driver altera_pcie_driver = {
+	.probe		= altera_pcie_probe,
+	.remove		= altera_pcie_remove,
+	.driver = {
+		.name	= "altera-pcie",
+		.of_match_table = altera_pcie_of_match,
+	},
+};
+
+module_platform_driver(altera_pcie_driver);
+
+MODULE_AUTHOR("Ley Foon Tan <lftan@altera.com>");
+MODULE_DESCRIPTION("Altera PCIe host controller driver");
+MODULE_LICENSE("GPL v2");