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

[M.H. Lian]

Hi Bin,

I noticed Sandbox architecture has added PCI menu to the arch/sandbox/Kconfig
Could I move this PCI menu to the driver/pci/kconfig?
So I could add all PCI related config to defconfig files.

Please see my comments inline.

Thanks,
Minghuan


> -----Original Message-----
> From: Bin Meng [mailto:bmeng.cn at gmail.com]
> Sent: Tuesday, October 11, 2016 6:35 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 5:36 PM, M.H. Lian <minghuan.lian at nxp.com> wrote:
> > 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.
> >>
> 
> Yep, I think so.
> 
> >> > 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.
> >
> 
> Yep, I guess you will need introduce CONFIG_PCI_DOMAINS to U-Boot DM
> PCI codes.
> 
[Minghuan Lian] For Linux, "DOMAINS" is an option, not a must.
And we need to change many PCIe code carefully to compatible with all PCIe driver.
It will take plenty of time. And there are still very pressing jobs on hand. So I will add this to my todo list.

> Regards,
> Bin