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

[Bin Meng]

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.

Regards,
Bin