In subject:
PCI: altera: Add Agilex support
to match style of history.
> #define TLP_CFG_DW1(pcie, tag, be) \
> - (((TLP_REQ_ID(pcie->root_bus_nr, RP_DEVFN)) << 16) | (tag << 8) | (be))
> + (((TLP_REQ_ID((pcie)->root_bus_nr, RP_DEVFN)) << 16) | ((tag) << 8) | (be))
Seems OK, but unrelated to adding Agilex support, so it should be a
separate patch.
> +#define AGLX_RP_CFG_ADDR(pcie, reg) \
> + (((pcie)->hip_base) + (reg))
Fits on one line.
> +#define AGLX_BDF_REG 0x00002004
> +#define AGLX_ROOT_PORT_IRQ_STATUS 0x14c
> +#define AGLX_ROOT_PORT_IRQ_ENABLE 0x150
> +#define CFG_AER BIT(4)
Indent values to match #defines above.
> static bool altera_pcie_hide_rc_bar(struct pci_bus *bus, unsigned int devfn,
> int offset)
> {
> - if (pci_is_root_bus(bus) && (devfn == 0) &&
> - (offset == PCI_BASE_ADDRESS_0))
> + if (pci_is_root_bus(bus) && devfn == 0 && offset == PCI_BASE_ADDRESS_0)
> return true;
OK, but again unrelated to Agilex.
> @@ -373,7 +422,7 @@ static int tlp_cfg_dword_write(struct altera_pcie *pcie, u8 bus, u32 devfn,
> * Monitor changes to PCI_PRIMARY_BUS register on root port
> * and update local copy of root bus number accordingly.
> */
> - if ((bus == pcie->root_bus_nr) && (where == PCI_PRIMARY_BUS))
> + if (bus == pcie->root_bus_nr && where == PCI_PRIMARY_BUS)
Ditto.
> @@ -577,7 +731,7 @@ static void altera_wait_link_retrain(struct altera_pcie *pcie)
> dev_err(dev, "link retrain timeout\n");
> break;
> }
> - udelay(100);
> + usleep_range(50, 150);
Where do these values come from? Needs a comment, ideally with a spec
citation.
How do we know a 50us delay is enough when we previously waited at
least 100us?
> @@ -590,7 +744,7 @@ static void altera_wait_link_retrain(struct altera_pcie *pcie)
> dev_err(dev, "link up timeout\n");
> break;
> }
> - udelay(100);
> + usleep_range(50, 150);
Ditto.
> +static void aglx_isr(struct irq_desc *desc)
> +{
> + struct irq_chip *chip = irq_desc_get_chip(desc);
> + struct altera_pcie *pcie;
> + struct device *dev;
> + u32 status;
> + int ret;
> +
> + chained_irq_enter(chip, desc);
> + pcie = irq_desc_get_handler_data(desc);
> + dev = &pcie->pdev->dev;
>
> + status = readl(pcie->hip_base + pcie->pcie_data->port_conf_offset +
> + pcie->pcie_data->port_irq_status_offset);
> + if (status & CFG_AER) {
> + ret = generic_handle_domain_irq(pcie->irq_domain, 0);
> + if (ret)
> + dev_err_ratelimited(dev, "unexpected IRQ,\n");
Was there supposed to be more data here, e.g., an IRQ %d or something?
Or is it just a spurious "," at the end of the line?
> pcie->irq_domain = irq_domain_add_linear(node, PCI_NUM_INTX,
> - &intx_domain_ops, pcie);
> + &intx_domain_ops, pcie);
Cleanup that should be in a separate patch. *This* patch should have
the absolute minimum required to enable Agilex to make it easier to
review/backport/revert/etc.
> +static const struct altera_pcie_data altera_pcie_3_0_f_tile_data = {
> + .ops = &altera_pcie_ops_3_0,
> + .version = ALTERA_PCIE_V3,
> + .cap_offset = 0x70,
It looks like this is where the PCIe Capability is? There's no way to
discover this offset, e.g., by following the capability list like
pci_find_capability() does?
Bjorn
