Capitalize subject line similarly. s/suport/support/ On Wed, Sep 06, 2023 at 04:39:18PM +0530, sharath.kumar.d.m@xxxxxxxxx wrote: > From: D M Sharath Kumar <sharath.kumar.d.m@xxxxxxxxx> Needs a commit log. It's ok to repeat the subject line. > +#define AGLX_BDF_REG 0x00002004 > +#define AGLX_ROOT_PORT_IRQ_STATUS 0x14c > +#define AGLX_ROOT_PORT_IRQ_ENABLE 0x150 > +#define CFG_AER (1<<4) This seems to be AGLX-specific so maybe should have a prefix? > +static u32 port_conf_off; port_conf_off looks like something that should be per-controller. > +static int aglx_rp_read_cfg(struct altera_pcie *pcie, u8 busno, u32 devfn, > + int where, int size, u32 *value) > +{ > + void __iomem *addr = AGLX_RP_CFG_ADDR(pcie, where); > + > + switch (size) { > + case 1: > + *value = readb(addr); > + break; > + case 2: > + *value = readw(addr); > + break; > + default: > + *value = readl(addr); > + break; > + } > + > + /* interrupt pin not programmed in hardware > + */ Use single-line comment style: /* interrupt pin not programmed in hardware */ > + if (where == 0x3d) > + *value = 0x01; > + if (where == 0x3c) > + *value |= 0x0100; Use PCI_INTERRUPT_LINE and PCI_INTERRUPT_PIN. > + return PCIBIOS_SUCCESSFUL; > +} > +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 + port_conf_off > + + AGLX_ROOT_PORT_IRQ_STATUS)); > + if (status & CFG_AER) { > + ret = generic_handle_domain_irq(pcie->irq_domain, 0); > + if (ret) > + dev_err_ratelimited(dev, "unexpected IRQ,\n"); Remove the comma at end (or maybe you meant to add something else?) Looks like the place it was copied from had "unexpected IRQ, INT%d". > + if (pcie->pcie_data->version == ALTERA_PCIE_V3) { > + pcie->cs_base = > + devm_platform_ioremap_resource_byname(pdev, "Cs"); > + if (IS_ERR(pcie->cs_base)) > + return PTR_ERR(pcie->cs_base); > + of_property_read_u32(pcie->pdev->dev.of_node, "port_conf_stat", > + &port_conf_off); > + dev_info(&pcie->pdev->dev, "port_conf_stat_off =%x\n", port_conf_off); Is this a debug message? Doesn't look like something we need all the time. If you want it all the time, use %#x so it's clear that it's hex. > +static const struct altera_pcie_data altera_pcie_3_0_data = { > + .ops = &altera_pcie_ops_3_0, > + .version = ALTERA_PCIE_V3, > + .cap_offset = 0x70, > + .cfgrd0 = 0, > + .cfgrd1 = 0, > + .cfgwr0 = 0, > + .cfgwr1 = 0, cfgrd0, ..., cfgwr1 aren't used here, so no need to initialize them.