Re: [PATCH v1 05/14] xen/arm: PCI host bridge discovery within XEN on ARM

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 13 Sep 2021, Oleksandr Andrushchenko wrote:
> Hi, Rahul!
> 
> On 19.08.21 15:02, Rahul Singh wrote:
> > XEN during boot will read the PCI device tree node “reg” property
> > and will map the PCI config space to the XEN memory.
> [snip]
> > +static struct pci_config_window *gen_pci_init(struct dt_device_node *dev,
> > +                                              int ecam_reg_idx)
> > +{
> > +    int err;
> > +    struct pci_config_window *cfg;
> > +    paddr_t addr, size;
> > +
> > +    cfg = xzalloc(struct pci_config_window);
> > +    if ( !cfg )
> > +        return NULL;
> > +
> > +    err = dt_pci_parse_bus_range(dev, cfg);
> > +    if ( !err ) {
> > +        cfg->busn_start = 0;
> > +        cfg->busn_end = 0xff;
> > +        printk(XENLOG_ERR "%s:No bus range found for pci controller\n",
> > +               dt_node_full_name(dev));
> > +    } else {
> > +        if ( cfg->busn_end > cfg->busn_start + 0xff )
> > +            cfg->busn_end = cfg->busn_start + 0xff;
> > +    }
> > +
> > +    /* Parse our PCI ecam register address*/
> > +    err = dt_device_get_address(dev, ecam_reg_idx, &addr, &size);
> 
> I am a bit worried here that we don't get the reg index from the device tree,
> 
> but for generic ECAM we use reg[0] and for Xilinx we use reg[2].
> 
> For example, for Xilinx we have
> 
> reg = <0x00 0xfd0e0000 0x00 0x1000 0x00 0xfd480000 0x00 0x1000 0x80 0x00 0x00 
> 0x1000000>;
> reg-names = "breg\0pcireg\0cfg";
> 
> so, we can parse the reg-names and understand that the configuration space is 
> the last in the reg property.
> 
> The same I think can be done for other device trees probably.

Well spotted!


> Rahul, do you know if reg-names "cfg" is vendor specific of used widely?

Unfortunately it seems to be vendor specific :-(
Which means that "dt_device_get_address" or similar should be moved to a
vendor specific function.