Re: [PATCH v3 3/3] PCI: ARM: add support for generic PCI host controller

[Arnd Bergmann <arnd@arndb.de>]

On Tuesday 18 February 2014 12:20:43 Will Deacon wrote:

> +static int gen_pci_alloc_io_offset(u32 sz, resource_size_t *offset)
> +{
> +	static DECLARE_BITMAP(io_map, (IO_SPACE_LIMIT + 1) / SZ_64K);
> +	int idx, num_wins;
> +
> +	if (sz > SZ_64K)
> +		return -ENOSPC;
> +
> +	num_wins = (IO_SPACE_LIMIT + 1) / SZ_64K;
> +	idx = 0;
> +	do {
> +		idx = find_next_zero_bit(io_map, num_wins, idx);
> +		if (idx == num_wins)
> +			return -ENOSPC;
> +	} while (test_and_set_bit(idx, io_map));
> +
> +	*offset = idx * SZ_64K;
> +	return 0;
> +}

Sicne you're always starting the search at 0 and you never free
the map, this is essentially the same as remembering the last number
that was used and using the next one, right?

You should also pass the rang->pci_addr here to calculate the
offset in case the pci_addr is not zero.

> +	/* Register our I/O and Memory resources */
> +	res_valid = 0;
> +	list_for_each_entry(win, &pci->host.windows, list) {
> +		struct resource *parent;
> +
> +		if (resource_type(win->res) == IORESOURCE_IO) {
> +			parent = &ioport_resource;
> +			err = pci_ioremap_io(win->offset, win->res->start);

and consequently pass the pci_addr rather than the offset here. How about
moving the pci_ioremap_io() call into gen_pci_alloc_io_offset()?

	Arnd