On 13.10.2023 00:09, Volodymyr Babchuk wrote: > Previously pci_enable_msi() function obtained pdev pointer by itself, > but taking into account upcoming changes to PCI locking, it is better > when caller passes already acquired pdev pointer to the function.
For the patch to be understandable on its own, the "is better" wants explaining here. > Note that ns16550 driver does not check validity of obtained pdev > pointer because pci_enable_msi() already does this. I'm not convinced of this model. I'd rather see the caller do the check, and the callee - optionally - have a respective assertion. > --- a/xen/arch/x86/msi.c > +++ b/xen/arch/x86/msi.c > @@ -983,13 +983,13 @@ static int msix_capability_init(struct pci_dev *dev, > * irq or non-zero for otherwise. > **/ > > -static int __pci_enable_msi(struct msi_info *msi, struct msi_desc **desc) > +static int __pci_enable_msi(struct msi_info *msi, struct msi_desc **desc, > + struct pci_dev *pdev) In line with msi_capability_init() and ... > @@ -1038,13 +1038,13 @@ static void __pci_disable_msi(struct msi_desc *entry) > * of irqs available. Driver should use the returned value to re-send > * its request. > **/ > -static int __pci_enable_msix(struct msi_info *msi, struct msi_desc **desc) > +static int __pci_enable_msix(struct msi_info *msi, struct msi_desc **desc, > + struct pci_dev *pdev) ... msix_capability_init(), may I ask that the new parameter then become the first one, not the last (and hence even past output parameters)? Jan
