From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stephen Warren <swarren@wwwdotorg.org>
Date: Wed, 21 Oct 2015 14:25:57 -0600
Subject: [U-Boot] [PATCH 4/8] dm: pci: Support decoding ranges with
 duplicate entries
In-Reply-To: <1445104205-4079-5-git-send-email-sjg@chromium.org>
References: <1445104205-4079-1-git-send-email-sjg@chromium.org>
 <1445104205-4079-5-git-send-email-sjg@chromium.org>
Message-ID: <5627F4D5.6070104@wwwdotorg.org>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On 10/17/2015 11:50 AM, Simon Glass wrote:
> At present we add a new resource entry for every range entry. But some range
> entries refer to configuration regions. To make this work, avoid adding two
> regions of the same time. The later ranges will overwrite the earlier
> (configuration) ones.

s/time/type/ in the last-but-one line.

What's wrong with having two regions of the same type? Equally, if we 
can "get away" with not storing some of the regions that happen to have 
a duplicate type, why not recast the function so that it only stores 
regions of specific (useful/desired) types, and simply dropping all of 
the other regions. That'd be a lot more consistent than only storing a 
somewhat arbitrary subset of the regions.

> There does not seem to be a way to distinguish the configuration ranges
> other than by ordering.

Well, they do have different addresses too. But yes, the DT binding is 
written so that the entries in ranges must appear in a specific order, 
so order is the correct way to index the entries.

> diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c

> @@ -720,9 +721,15 @@ static int decode_regions(struct pci_controller *hose, const void *blob,
>   		} else {
>   			continue;
>   		}
> -		debug(" - type=%d\n", type);
> -		pci_set_region(hose->regions + hose->region_count++, pci_addr,
> -			       addr, size, type);
> +		pos = -1;
> +		for (i = 0; i < hose->region_count; i++) {
> +			if (hose->regions[i].flags == type)
> +				pos = i;

and break too?

> +		}
> +		if (pos == -1)
> +			pos = hose->region_count++;
> +		debug(" - type=%d, pos=%d\n", type, pos);
> +		pci_set_region(hose->regions + pos, pci_addr, addr, size, type);
>   	}