Hi Juergen: Thanks for your review. > -----Original Message----- > From: Jürgen Beisert [mailto:jbe@xxxxxxxxxxxxxx] > Sent: Monday, December 09, 2013 7:01 PM > To: Richard Zhu > Cc: marex@xxxxxxx; hrhaan@xxxxxxxxx; shawn.guo@xxxxxxxxxx; jgarzik@xxxxxxxxx; > tj@xxxxxxxxxx; linux-ide@xxxxxxxxxxxxxxx; Zhu Richard-R65037 > Subject: Re: [PATCH] pci: imx: enable pcie msi support > > Hi Richard, > > On Monday 09 December 2013 10:25:19 Richard Zhu wrote: > > [...] > > +static int dw_msi_check_device(struct msi_chip *chip, struct pci_dev *pdev, > > + int nvec, int type) > > +{ > > + struct pcie_port *pp = sys_to_pcie(pdev->bus->sysdata); > > + u32 val; > > + > > + if (pp->quirks & DW_PCIE_QUIRK_MSI_SELF_EN) { > > + if ((type == PCI_CAP_ID_MSI) || (type == PCI_CAP_ID_MSIX)) { > > + /* Set MSI enable of RC here */ > > + val = readl(pp->dbi_base + 0x50); > > + if ((val & (PCI_MSI_FLAGS_ENABLE << 16)) == 0) { > > + val |= PCI_MSI_FLAGS_ENABLE << 16; > > + writel(val, pp->dbi_base + 0x50); > > + } > > + } > > + } > > + > > + return 0; > > +} > > Maybe I'm wrong: but does setting this bit not already happen in function > msi_set_enable() in 'drivers/pci/msi.c' (just to a 16 bit register, so there's > no bit shift required)? > [Richard] yes, this bit would be set in msi_set_enable invoked by msi_capability_init. But I can't monitor that this bit is set when e1000e is probed. :(. So I had to set this bit explicitly here. > Regards, > Juergen > > -- > Pengutronix e.K. | Juergen Beisert | > Linux Solutions for Science and Industry | http://www.pengutronix.de/ | > Best Regards Richard Zhu -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html