On Fri, Nov 04, 2016 at 06:33:29PM +0100, Andrew Jones wrote:
[...]
> > +static pci_cap_handler cap_handlers[0xff] = {
>
> Why 0xff? Shouldn't it be (PCI_CAP_ID_MAX + 1)?
Yes. Will fix.
[...]
> > +int pci_setup_msi(struct pci_dev *dev, uint64_t msi_addr, uint32_t msi_data)
> > +{
> > + uint16_t msi_control;
> > + uint16_t offset;
> > + pcidevaddr_t addr = dev->pci_bdf;
> > +
> > + assert(dev);
> > +
> > + if (!dev->msi_offset) {
> > + printf("MSI: dev 0x%x does not support MSI.\n", addr);
> > + return -1;
> > + }
> > +
> > + offset = dev->msi_offset;
> > + msi_control = pci_config_readw(addr, offset + PCI_MSI_FLAGS);
> > + pci_config_writel(addr, offset + PCI_MSI_ADDRESS_LO,
> > + msi_addr & 0xffffffff);
> > +
> > + if (msi_control & PCI_MSI_FLAGS_64BIT) {
> > + pci_config_writel(addr, offset + PCI_MSI_ADDRESS_HI,
> > + (uint32_t)(msi_addr >> 32));
> > + pci_config_writel(addr, offset + PCI_MSI_DATA_64, msi_data);
> > + printf("MSI: dev 0x%x init 64bit address: ", addr);
> > + } else {
> > + pci_config_writel(addr, offset + PCI_MSI_DATA_32, msi_data);
> > + printf("MSI: dev 0x%x init 32bit address: ", addr);
> > + }
> > + printf("addr=0x%lx, data=0x%x\n", msi_addr, msi_data);
> > +
> > + msi_control |= PCI_MSI_FLAGS_ENABLE;
> > + pci_config_writew(addr, offset + PCI_MSI_FLAGS, msi_control);
> > +
> > + return 0;
> > +}
>
> Again we only have 0/-1 for good/bad. I'd prefer true/false.
Will use bool as return code.
> Otherwise
>
> Reviewed-by: Andrew Jones <drjones@xxxxxxxxxx>
Thanks!
-- peterx
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
