| Message ID | 20240605081605.18769-3-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: > The current derves implementation uses manual shift operations to check > whether a bit in a mask is set. The code can be made more readable by > writing a small helper function for that. > > Implement mask_contains_bar() and use it where applicable. > > Signed-off-by: Philipp Stanner <pstanner@redhat.com> > --- > drivers/pci/devres.c | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/drivers/pci/devres.c b/drivers/pci/devres.c > index 2c562b9eaf80..f13edd4a3873 100644 > --- a/drivers/pci/devres.c > +++ b/drivers/pci/devres.c > @@ -161,6 +161,10 @@ int pcim_set_mwi(struct pci_dev *dev) > } > EXPORT_SYMBOL(pcim_set_mwi); > > +static inline bool mask_contains_bar(int mask, int bar) Why these are signed? Using & for signed values is an indication that the types should have been unsigned. The typing change has ripple effects to caller-side typing. > +{ > + return mask & BIT(bar); > +}
diff --git a/drivers/pci/devres.c b/drivers/pci/devres.c index 2c562b9eaf80..f13edd4a3873 100644 --- a/drivers/pci/devres.c +++ b/drivers/pci/devres.c @@ -161,6 +161,10 @@ int pcim_set_mwi(struct pci_dev *dev) } EXPORT_SYMBOL(pcim_set_mwi); +static inline bool mask_contains_bar(int mask, int bar) +{ + return mask & BIT(bar); +} static void pcim_release(struct device *gendev, void *res) { @@ -169,7 +173,7 @@ static void pcim_release(struct device *gendev, void *res) int i; for (i = 0; i < DEVICE_COUNT_RESOURCE; i++) - if (this->region_mask & (1 << i)) + if (mask_contains_bar(this->region_mask, i)) pci_release_region(dev, i); if (this->mwi) @@ -363,7 +367,7 @@ int pcim_iomap_regions(struct pci_dev *pdev, int mask, const char *name) for (i = 0; i < DEVICE_COUNT_RESOURCE; i++) { unsigned long len; - if (!(mask & (1 << i))) + if (!mask_contains_bar(mask, i)) continue; rc = -EINVAL; @@ -386,7 +390,7 @@ int pcim_iomap_regions(struct pci_dev *pdev, int mask, const char *name) pci_release_region(pdev, i); err_inval: while (--i >= 0) { - if (!(mask & (1 << i))) + if (!mask_contains_bar(mask, i)) continue; pcim_iounmap(pdev, iomap[i]); pci_release_region(pdev, i); @@ -438,7 +442,7 @@ void pcim_iounmap_regions(struct pci_dev *pdev, int mask) return; for (i = 0; i < PCIM_IOMAP_MAX; i++) { - if (!(mask & (1 << i))) + if (!mask_contains_bar(mask, i)) continue; pcim_iounmap(pdev, iomap[i]);
The current derves implementation uses manual shift operations to check whether a bit in a mask is set. The code can be made more readable by writing a small helper function for that. Implement mask_contains_bar() and use it where applicable. Signed-off-by: Philipp Stanner <pstanner@redhat.com> --- drivers/pci/devres.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)