Hi Minghuan, On Tue, Oct 11, 2016 at 5:36 PM, M.H. Lian <minghuan.l...@nxp.com> wrote: > Hi Bin, > > Please see my comments inline. > > Thanks, > Minghuan > >> -----Original Message----- >> From: Bin Meng [mailto:bmeng...@gmail.com] >> Sent: Tuesday, October 11, 2016 3:43 PM >> To: M.H. Lian <minghuan.l...@nxp.com> >> Cc: Simon Glass <s...@chromium.org>; U-Boot Mailing List <u- >> b...@lists.denx.de>; Mingkai Hu <mingkai...@nxp.com>; Leo Li >> <leoyang...@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.l...@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. Regards, Bin _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot