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. > > > 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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot