[U-Boot] [PATCH 1/2] dm: pci: make ranges dt property optional

[Christian Gmeiner]

Hi Bin,

Am Mi., 23. Mai 2018 um 15:10 Uhr schrieb Bin Meng <bmeng.cn at gmail.com>:

> Hi Christian,

> On Wed, May 23, 2018 at 8:00 PM, Christian Gmeiner
> <christian.gmeiner at gmail.com> wrote:
> > If we use u-boot as coreboot payload with a generic dts without
> > any ranges specified we fail in pci pre_probe and our pci bus
> > is not usable.
> >

> What do you mean by "a generic dts"?


> The coreboot payload needs to specify a dedicated dts for that board,
> for example to build a coreboot payload for minnowmax, we need specify
> minnowmax.dts in the Kconfig. README.x86 documents this.


Yeah.. so in my eyes a "generic dts" contains only this part regarding pci:

         pci {
                 compatible = "pci-x86";
                 #address-cells = <0x3>;
                 #size-cells = <0x2>;
         };

The important part is that it does not contain any ranges. Coreboot is the
one
who does the pci bus setup (assigning memory windows for devices etc). So I
do not want to know in u-boot at build time (ranges thing for pci) how the
pci
bus looks regarding addresses. My end goal is one generic u-boot payload
that
gets used for different FSP 2.0 based boards.

> > So convert decode_regions(..) into a void function and do the simple
> > error handling there.
> >
> > Signed-off-by: Christian Gmeiner <christian.gmeiner at gmail.com>
> > ---
> >  drivers/pci/pci-uclass.c | 21 +++++++++------------
> >  1 file changed, 9 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
> > index 49be1ebdd7..416444a230 100644
> > --- a/drivers/pci/pci-uclass.c
> > +++ b/drivers/pci/pci-uclass.c
> > @@ -810,7 +810,7 @@ error:
> >         return ret;
> >  }
> >
> > -static int decode_regions(struct pci_controller *hose, ofnode
parent_node,
> > +static void decode_regions(struct pci_controller *hose, ofnode
parent_node,
> >                           ofnode node)
> >  {
> >         int pci_addr_cells, addr_cells, size_cells;
> > @@ -820,8 +820,11 @@ static int decode_regions(struct pci_controller
*hose, ofnode parent_node,
> >         int i;
> >
> >         prop = ofnode_get_property(node, "ranges", &len);
> > -       if (!prop)
> > -               return -EINVAL;
> > +       if (!prop) {
> > +               debug("%s: Cannot decode regions\n", __func__);
> > +               return;
> > +       }
> > +
> >         pci_addr_cells = ofnode_read_simple_addr_cells(node);
> >         addr_cells = ofnode_read_simple_addr_cells(parent_node);
> >         size_cells = ofnode_read_simple_size_cells(node);
> > @@ -876,7 +879,7 @@ static int decode_regions(struct pci_controller
*hose, ofnode parent_node,
> >         bd_t *bd = gd->bd;
> >
> >         if (!bd)
> > -               return 0;
> > +               return;
> >
> >         for (i = 0; i < CONFIG_NR_DRAM_BANKS; ++i) {
> >                 if (bd->bi_dram[i].size) {
> > @@ -901,13 +904,12 @@ static int decode_regions(struct pci_controller
*hose, ofnode parent_node,
> >                         base, size, PCI_REGION_MEM |
PCI_REGION_SYS_MEMORY);
> >  #endif
> >
> > -       return 0;
> > +       return;
> >  }
> >
> >  static int pci_uclass_pre_probe(struct udevice *bus)
> >  {
> >         struct pci_controller *hose;
> > -       int ret;
> >
> >         debug("%s, bus=%d/%s, parent=%s\n", __func__, bus->seq,
bus->name,
> >               bus->parent->name);
> > @@ -916,12 +918,7 @@ static int pci_uclass_pre_probe(struct udevice
*bus)
> >         /* For bridges, use the top-level PCI controller */
> >         if (!device_is_on_pci_bus(bus)) {
> >                 hose->ctlr = bus;
> > -               ret = decode_regions(hose, dev_ofnode(bus->parent),
> > -                                    dev_ofnode(bus));
> > -               if (ret) {
> > -                       debug("%s: Cannot decode regions\n", __func__);
> > -                       return ret;
> > -               }
> > +               decode_regions(hose, dev_ofnode(bus->parent),
dev_ofnode(bus));
> >         } else {
> >                 struct pci_controller *parent_hose;
> >
> > --
> >

> Regards,
> Bin



-- 
greets
--
Christian Gmeiner, MSc

https://christian-gmeiner.info