| Message ID | 20240605081605.18769-11-pstanner@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Make PCI's devres API more consistent | expand |
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@redhat.com> > --- > 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()? > - dr->mwi = 1; > - return pci_set_mwi(dev); > + return ret; > } > EXPORT_SYMBOL(pcim_set_mwi);
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@redhat.com> > > --- > > 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); >
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); - dr->mwi = 1; - return pci_set_mwi(dev); + return ret; } EXPORT_SYMBOL(pcim_set_mwi); @@ -392,9 +402,6 @@ static void pcim_release(struct device *gendev, void *res) struct pci_dev *dev = to_pci_dev(gendev); struct pci_devres *this = res; - if (this->mwi) - pci_clear_mwi(dev); - if (this->restore_intx) pci_intx(dev, this->orig_intx); diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h index ff439dd05200..dbf6772aaaaf 100644 --- a/drivers/pci/pci.h +++ b/drivers/pci/pci.h @@ -825,7 +825,6 @@ static inline pci_power_t mid_pci_get_power_state(struct pci_dev *pdev) struct pci_devres { unsigned int orig_intx:1; unsigned int restore_intx:1; - unsigned int mwi:1; }; struct pci_devres *find_pci_dr(struct pci_dev *pdev);
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@redhat.com> --- drivers/pci/devres.c | 29 ++++++++++++++++++----------- drivers/pci/pci.h | 1 - 2 files changed, 18 insertions(+), 12 deletions(-)