Messages in this thread | | | Date | Fri, 17 Dec 2021 13:38:06 +0100 | Subject | Re: [PATCH 1/2] PCI: Add support for VF Resizable Bar extended cap | From | Michał Winiarski <> |
| |
On 16.12.2021 01:21, Krzysztof Wilczyński wrote: > Hi Michał, > > [...] >> +static int pci_memory_decoding(struct pci_dev *dev) >> +{ >> + u16 cmd; >> + >> + pci_read_config_word(dev, PCI_COMMAND, &cmd); >> + if (cmd & PCI_COMMAND_MEMORY) >> + return -EBUSY; >> + >> + return 0; >> +} >> + >> +#ifdef CONFIG_PCI_IOV >> +static int pci_vf_memory_decoding(struct pci_dev *dev) >> +{ >> + u16 cmd; >> + >> + pci_read_config_word(dev, dev->sriov->pos + PCI_SRIOV_CTRL, &cmd); >> + if (cmd & PCI_SRIOV_CTRL_MSE) >> + return -EBUSY; >> + >> + return 0; >> +} >> +#endif >> + >> int pci_resize_resource(struct pci_dev *dev, int resno, int size) >> { >> struct resource *res = dev->resource + resno; >> struct pci_host_bridge *host; >> int old, ret; >> u32 sizes; >> - u16 cmd; >> >> /* Check if we must preserve the firmware's resource assignment */ >> host = pci_find_host_bridge(dev->bus); >> @@ -424,9 +447,14 @@ int pci_resize_resource(struct pci_dev *dev, int resno, int size) >> if (!(res->flags & IORESOURCE_UNSET)) >> return -EBUSY; >> >> - pci_read_config_word(dev, PCI_COMMAND, &cmd); >> - if (cmd & PCI_COMMAND_MEMORY) >> - return -EBUSY; >> +#ifdef CONFIG_PCI_IOV >> + if (resno >= PCI_IOV_RESOURCES) >> + ret = pci_vf_memory_decoding(dev); >> + else >> +#endif >> + ret = pci_memory_decoding(dev); >> + if (ret) >> + return ret; > > The above else works, however, it does trip our static analysis tools, and > lack of the indentation makes it slightly confusing to read, at least at > the first glance. Thus, I wonder whether it would be possible to combine > the pci_vf_memory_decoding() and pci_memory_decoding() somehow neatly into > a private helper that takes a boolean to indicate whether it's a VF or not. > Then, we could drop the if-statement, since some of the logic could live > within the helper. > > What do you think?
Sure - implementing a helper in a way that Christian suggested (the helper still takes resno though) should also take care of that.
Thanks -Michał
> > Krzysztof
| |