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

[Bin Meng]

Hi Simon,

On Wed, Jun 24, 2015 at 11:23 AM, Simon Glass <sjg at chromium.org> wrote:
> 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?
>

Yep, maybe not a big deal, but it may also break some boards if the
allocated buffer for PCI device happens to be above 2 GiB, but that
case might be rare (ie: like 3 GiB memory is installed where on most
cases we only see boards with 1 GiB, 2 GiB, 4 GiB memory)

> 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.

Adding pci_max_addr to global_data sounds good if we want to avoid
#ifdefs. It would be good to hear more comments from other domains?

Regards,
Bin