On Thu, Jun 13, 2024 at 07:54:38PM +0300, Ilpo Järvinen wrote:
> On Wed, 5 Jun 2024, Philipp Stanner wrote:
>
> > When the original PCI devres API was implemented, priority was given to
> > the creation of a set of "plural functions" such as
> > pcim_request_regions(). These functions have bit masks as parameters to
> > specify which BARs shall get mapped. Most users, however, only use those
> > to map 1-3 BARs.
> > +static int __pcim_request_region_range(struct pci_dev *pdev, int bar,
> > + unsigned long offset, unsigned long maxlen,
> > + const char *name, int req_flags)
> > +{
> > + resource_size_t start = pci_resource_start(pdev, bar);
> > + resource_size_t len = pci_resource_len(pdev, bar);
> > + unsigned long dev_flags = pci_resource_flags(pdev, bar);
> > +
> > + if (start == 0 || len == 0) /* That's an unused BAR. */
> > + return 0;
> > + if (len <= offset)
> > + return -EINVAL;
>
> Extra space.
Thanks for reviewing this. I dropped the space locally in the v9
series.
> > void pcim_iounmap_regions(struct pci_dev *pdev, int mask)
> > {
> > - void __iomem * const *iomap;
> > - int i;
> > -
> > - iomap = pcim_iomap_table(pdev);
> > - if (!iomap)
> > - return;
> > + short bar;
>
> The current best practice is to use unsigned for loop vars that will never
> be negative.
>
> I don't entirely follow what is reasoning behind making it short instead
> of unsigned int?
Existing interfaces like pcim_iomap() take "int bar". I locally
changed all the BAR indices to "int". We can make everything unsigned
later if worthwhile.
> > - for (i = 0; i < PCIM_IOMAP_MAX; i++) {
> > - if (!mask_contains_bar(mask, i))
> > + for (bar = 0; bar < PCI_STD_NUM_BARS; bar++) {
>
> Is this change minimal if it contains variable renames like this?
> Was "i" not "bar" even if it was given as a parameter to
> mask_contains_bar()?
Replaced locally with "i".
Bjorn
