On Thu, 2024-06-13 at 20:19 +0300, Ilpo Järvinen wrote: > On Wed, 5 Jun 2024, Philipp Stanner wrote: > > > Managing pci_set_mwi() with devres can easily be done with its own > > callback, without the necessity to store any state about it in a > > device-related struct. > > > > Remove the MWI state from struct pci_devres. > > Give pcim_set_mwi() a separate devres-callback. > > > > Signed-off-by: Philipp Stanner <pstanner@xxxxxxxxxx> > > --- > > drivers/pci/devres.c | 29 ++++++++++++++++++----------- > > drivers/pci/pci.h | 1 - > > 2 files changed, 18 insertions(+), 12 deletions(-) > > > > diff --git a/drivers/pci/devres.c b/drivers/pci/devres.c > > index 936369face4b..0bafb67e1886 100644 > > --- a/drivers/pci/devres.c > > +++ b/drivers/pci/devres.c > > @@ -361,24 +361,34 @@ void __iomem > > *devm_pci_remap_cfg_resource(struct device *dev, > > } > > EXPORT_SYMBOL(devm_pci_remap_cfg_resource); > > > > +static void __pcim_clear_mwi(void *pdev_raw) > > +{ > > + struct pci_dev *pdev = pdev_raw; > > + > > + pci_clear_mwi(pdev); > > +} > > + > > /** > > * pcim_set_mwi - a device-managed pci_set_mwi() > > - * @dev: the PCI device for which MWI is enabled > > + * @pdev: the PCI device for which MWI is enabled > > * > > * Managed pci_set_mwi(). > > * > > * RETURNS: An appropriate -ERRNO error value on error, or zero > > for success. > > */ > > -int pcim_set_mwi(struct pci_dev *dev) > > +int pcim_set_mwi(struct pci_dev *pdev) > > { > > - struct pci_devres *dr; > > + int ret; > > > > - dr = find_pci_dr(dev); > > - if (!dr) > > - return -ENOMEM; > > + ret = devm_add_action(&pdev->dev, __pcim_clear_mwi, pdev); > > + if (ret != 0) > > + return ret; > > + > > + ret = pci_set_mwi(pdev); > > + if (ret != 0) > > + devm_remove_action(&pdev->dev, __pcim_clear_mwi, > > pdev); > > I'm sorry if this is a stupid question but why this cannot use > devm_add_action_or_reset()? For MWI that could be done. This is basically just consistent with the new pcim_enable_device() in patch No.11 where devm_add_action_or_reset() could collide with pcim_pin_device(). We could squash usage of devm_add_action_or_reset() in here. I don't care. P. > > > - dr->mwi = 1; > > - return pci_set_mwi(dev); > > + return ret; > > } > > EXPORT_SYMBOL(pcim_set_mwi); >