[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [RFC PATCH 01/13] PCI: Prepare removing devres from pci_intx()
On Thu, 2024-10-10 at 11:43 -0600, Alex Williamson wrote: > On Wed, 9 Oct 2024 10:35:07 +0200 > Philipp Stanner <pstanner@xxxxxxxxxx> wrote: > > > pci_intx() is a hybrid function which sometimes performs devres > > operations, depending on whether pcim_enable_device() has been used > > to > > enable the pci_dev. This sometimes-managed nature of the function > > is > > problematic. Notably, it causes the function to allocate under some > > circumstances which makes it unusable from interrupt context. > > > > To, ultimately, remove the hybrid nature from pci_intx(), it is > > first > > necessary to provide an always-managed and a never-managed version > > of that function. Then, all callers of pci_intx() can be ported to > > the > > version they need, depending whether they use pci_enable_device() > > or > > pcim_enable_device(). > > > > An always-managed function exists, namely pcim_intx(), for which > > __pcim_intx(), a never-managed version of pci_intx() had been > > implemented. > > > > Make __pcim_intx() a public function under the name > > pci_intx_unmanaged(). Make pcim_intx() a public function. > > > > Signed-off-by: Philipp Stanner <pstanner@xxxxxxxxxx> > > --- > > drivers/pci/devres.c | 24 +++--------------------- > > drivers/pci/pci.c | 26 ++++++++++++++++++++++++++ > > include/linux/pci.h | 2 ++ > > 3 files changed, 31 insertions(+), 21 deletions(-) > > > > diff --git a/drivers/pci/devres.c b/drivers/pci/devres.c > > index b133967faef8..475a3ae5c33f 100644 > > --- a/drivers/pci/devres.c > > +++ b/drivers/pci/devres.c > > @@ -411,31 +411,12 @@ static inline bool mask_contains_bar(int > > mask, int bar) > > return mask & BIT(bar); > > } > > > > -/* > > - * This is a copy of pci_intx() used to bypass the problem of > > recursive > > - * function calls due to the hybrid nature of pci_intx(). > > - */ > > -static void __pcim_intx(struct pci_dev *pdev, int enable) > > -{ > > - u16 pci_command, new; > > - > > - pci_read_config_word(pdev, PCI_COMMAND, &pci_command); > > - > > - if (enable) > > - new = pci_command & ~PCI_COMMAND_INTX_DISABLE; > > - else > > - new = pci_command | PCI_COMMAND_INTX_DISABLE; > > - > > - if (new != pci_command) > > - pci_write_config_word(pdev, PCI_COMMAND, new); > > -} > > - > > static void pcim_intx_restore(struct device *dev, void *data) > > { > > struct pci_dev *pdev = to_pci_dev(dev); > > struct pcim_intx_devres *res = data; > > > > - __pcim_intx(pdev, res->orig_intx); > > + pci_intx_unmanaged(pdev, res->orig_intx); > > } > > > > static struct pcim_intx_devres *get_or_create_intx_devres(struct > > device *dev) > > @@ -472,10 +453,11 @@ int pcim_intx(struct pci_dev *pdev, int > > enable) > > return -ENOMEM; > > > > res->orig_intx = !enable; > > - __pcim_intx(pdev, enable); > > + pci_intx_unmanaged(pdev, enable); > > > > return 0; > > } > > +EXPORT_SYMBOL(pcim_intx); > > What precludes this from _GPL? Also note that this is now calling a > GPL symbol, so by default I'd assume it should also be GPL. Thanks, Ah right, I overlooked that pci_intx() also has the _GPL version. Will make consistent. Thx, P. > > Alex > > > > > static void pcim_disable_device(void *pdev_raw) > > { > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > > index 7d85c04fbba2..318cfb5b5e15 100644 > > --- a/drivers/pci/pci.c > > +++ b/drivers/pci/pci.c > > @@ -4476,6 +4476,32 @@ void pci_disable_parity(struct pci_dev *dev) > > } > > } > > > > +/** > > + * pci_intx - enables/disables PCI INTx for device dev, unmanaged > > version > > + * @pdev: the PCI device to operate on > > + * @enable: boolean: whether to enable or disable PCI INTx > > + * > > + * Enables/disables PCI INTx for device @pdev > > + * > > + * This function behavios identically to pci_intx(), but is never > > managed with > > + * devres. > > + */ > > +void pci_intx_unmanaged(struct pci_dev *pdev, int enable) > > +{ > > + u16 pci_command, new; > > + > > + pci_read_config_word(pdev, PCI_COMMAND, &pci_command); > > + > > + if (enable) > > + new = pci_command & ~PCI_COMMAND_INTX_DISABLE; > > + else > > + new = pci_command | PCI_COMMAND_INTX_DISABLE; > > + > > + if (new != pci_command) > > + pci_write_config_word(pdev, PCI_COMMAND, new); > > +} > > +EXPORT_SYMBOL_GPL(pci_intx_unmanaged); > > + > > /** > > * pci_intx - enables/disables PCI INTx for device dev > > * @pdev: the PCI device to operate on > > diff --git a/include/linux/pci.h b/include/linux/pci.h > > index 573b4c4c2be6..6b8cde76d564 100644 > > --- a/include/linux/pci.h > > +++ b/include/linux/pci.h > > @@ -1353,6 +1353,7 @@ int __must_check pcim_set_mwi(struct pci_dev > > *dev); > > int pci_try_set_mwi(struct pci_dev *dev); > > void pci_clear_mwi(struct pci_dev *dev); > > void pci_disable_parity(struct pci_dev *dev); > > +void pci_intx_unmanaged(struct pci_dev *pdev, int enable); > > void pci_intx(struct pci_dev *dev, int enable); > > bool pci_check_and_mask_intx(struct pci_dev *dev); > > bool pci_check_and_unmask_intx(struct pci_dev *dev); > > @@ -2293,6 +2294,7 @@ static inline void pci_fixup_device(enum > > pci_fixup_pass pass, > > struct pci_dev *dev) { } > > #endif > > > > +int pcim_intx(struct pci_dev *pdev, int enabled); > > void __iomem *pcim_iomap(struct pci_dev *pdev, int bar, unsigned > > long maxlen); > > void __iomem *pcim_iomap_region(struct pci_dev *pdev, int bar, > > const char *name); >
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |