[U-Boot] [PATCH 1/9] dm: pci: return the real controller in pci_bus_to_hose()

[M.H. Lian]

Hi Bin,

Please see my comments inline.

Thanks,
Minghuan

> -----Original Message-----
> From: Bin Meng [mailto:bmeng.cn at gmail.com]
> Sent: Tuesday, October 11, 2016 3:43 PM
> To: M.H. Lian <minghuan.lian at nxp.com>
> Cc: Simon Glass <sjg at chromium.org>; U-Boot Mailing List <u-
> boot at lists.denx.de>; Mingkai Hu <mingkai.hu at nxp.com>; Leo Li
> <leoyang.li at nxp.com>
> Subject: Re: [U-Boot] [PATCH 1/9] dm: pci: return the real controller in
> pci_bus_to_hose()
> 
> Hi Minghuan,
> 
> On Tue, Oct 11, 2016 at 3:12 PM, M.H. Lian <minghuan.lian at nxp.com> wrote:
> > Hi Bin,
> >
> > With the patches our Layerscape PCIe driver has been fully based on DM.
> > Ethernet driver E1000 needs to define "CONFIG_DM_ETH" to use PCIe DM
> API instead of legacy PCI API.
> > But our other Ethernet driver FM(drivers/net/fm/eth.c) is still not support
> DM. So we cannot define "CONFIG_DM_ETH"
> 
> For LS1021a ethernet, please pick up this patch to see if it works:
> http://patchwork.ozlabs.org/patch/566347/
[Minghuan Lian] fm.c is used by ls1043 and ls1046.  So if this patch is merged, I may remove " CONFIG_DM_PCI_COMPAT "
On LS1021a.
> 
> > Well, we must define "CONFIG_DM_PCI_COMPAT" to support e1000 and
> fm at the same time.
> > After FM driver is changed to support DM, we can define
> "CONFIG_DM_ETH" and remove  "CONFIG_DM_PCI_COMPAT "
> >
> > But the current DM driver has an issue.
> >
> > 1.
> >  pci_bus_to_hose(int busnum)  defined in driver/pci/pci_compat.c  is
> > to return the hose associated current busnum(PCIe device) instead of
> > PCIe controller (RC)
> >
> > pci_bus_to_hose(int bus) defined in driver/pci/pci.c for legacy PCI driver is
> to return the hose pointed to the PCIe controller(RC).
> >
> > My first patch is to keep consistency and return the hose pointer of the
> PCIe controller.
> > -       return dev_get_uclass_priv(bus);
> > +       return dev_get_uclass_priv(pci_get_controller(bus));
> >
> > 2
> >  In pci/pci_common.c phys_addr_t pci_hose_bus_to_phys()
> >
> > #ifdef CONFIG_DM_PCI
> >        /* The root controller has the region information */
> >        hose = pci_bus_to_hose(0);
> > #endif
> >
> > Is always to return hose of the bus0.
> >
> > But our SoC has more than one PCIe controllers(RC).
> >
> > For example:
> > PCI0 bus 0  --  e1000#0  bus1
> > PCI1 bus 2  --  e1000#1  bus3.
> 
> I got it. But this does not look that good to me. There are two controllers, and
> bus number should be relative to the controller itself, not system wide. It's
> definitely right to assign bus number 0 to both PCIe host controllers, as they
> forward the bus number on their own bus link. I am wondering we should
> add a controller number to the PCI command, like the storage device
> command. The first parameter is the controller number, while the second
> parameter is the bus number.
[Minghuan Lian] Yes.  Linux uses "PCI domain" to isolate PCIe controllers.
And config "CONFIG_PCI_DOMAINS" is to enable domain like controllers number.
But, under Linux if disable "CONFIG_PCI_DOMAINS ", all PCIe controllers will be assigned continuous  bus 
Number like the current uboot.

> 
> >
> > PCI1 is the second PCIe controller (RC) has different PCIe space to PCI0.
> > For E1000#1, we want to get the host pointed to PCI1 bus2 not bus0.
> >
> 
> Regards,
> Bin