[U-Boot] [PATCH 11/11] dm: x86: baytrail: Correct PCI region 3 when driver model is used

[Simon Glass]

Hi,

On 8 June 2015 at 06:32, Andrew Bradford <andrew at bradfordembedded.com> wrote:
> Hi Bin / Simon,
>
> On 06/08 10:57, Bin Meng wrote:
>> Hi Simon,
>>
>> On Sun, Jun 7, 2015 at 10:50 PM, Simon Glass <sjg at chromium.org> wrote:
>> > Commit afbbd413a fixed this for non-driver-model. Make sure that the driver
>> > model code handles this also.
>> >
>> > Signed-off-by: Simon Glass <sjg at chromium.org>
>> > ---
>> >
>> >  drivers/pci/pci-uclass.c | 3 ++-
>> >  1 file changed, 2 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
>> > index edec93f..4255c02 100644
>> > --- a/drivers/pci/pci-uclass.c
>> > +++ b/drivers/pci/pci-uclass.c
>> > @@ -495,7 +495,8 @@ static int decode_regions(struct pci_controller *hose, const void *blob,
>> >
>> >         /* Add a region for our local memory */
>> >         pci_set_region(hose->regions + hose->region_count++, 0, 0,
>> > -                      gd->ram_size, PCI_REGION_MEM | PCI_REGION_SYS_MEMORY);
>> > +                      gd->ram_size < 0x80000000U ? gd->ram_size : 0x80000000U,
>> > +                      PCI_REGION_MEM | PCI_REGION_SYS_MEMORY);
>> >
>> >         return 0;
>> >  }
>> > --
>>
>> I think this is specific to baytrail fsp configuration. It should not
>> be put into a common driver.
>
> Yes, I agree with Bin, this is likely only a Bay Trail (or maybe a very
> small number of other processors) configuration.  I believe your change
> here will impact all PCI hosts which have > 2 GiB of RAM.
>
> It might be a bit ugly to have an #ifdef in this file, it all seems like
> nice clean generic code.  But maybe it's not a big deal to limit all
> u-boot to 2 GiB of RAM for this region?

Yes, this will bite us when something else moves PCI to driver model
and we may as well fix it now.

What is the best option? We could add a pci_max_addr to global_data
perhaps? This could be set to ram_size for most machines, and adjusted
for x86. I'd prefer to avoid #ifdef CONFIG_X86.

Regards,
Simon