[U-Boot] [PATCHv2 03/15] dm: pci: remove pci_bus_to_hose(0) calling

[Simon Glass]

Hi,

On 14 November 2016 at 00:22, Z.Q. Hou <zhiqiang.hou at nxp.com> wrote:
> Hi Simon,
>
> Thanks for your comments!
>
>> -----Original Message-----
>> From: sjg at google.com [mailto:sjg at google.com] On Behalf Of Simon Glass
>> Sent: 2016年11月12日 0:18
>> To: Z.Q. Hou <zhiqiang.hou at nxp.com>
>> Cc: U-Boot Mailing List <u-boot at lists.denx.de>; Albert ARIBAUD
>> <albert.u.boot at aribaud.net>; Prabhakar Kushwaha
>> <prabhakar.kushwaha at nxp.com>; Huan Wang-B18965
>> <alison.wang at freescale.com>; Sumit Garg <sumit.garg at nxp.com>; Ruchika
>> Gupta <ruchika.gupta at nxp.com>; Saksham Jain
>> <saksham.jain at nxp.freescale.com>; york sun <york.sun at nxp.com>; M.H. Lian
>> <minghuan.lian at nxp.com>; Bin Meng <bmeng.cn at gmail.com>; Mingkai Hu
>> <mingkai.hu at nxp.com>
>> Subject: Re: [PATCHv2 03/15] dm: pci: remove pci_bus_to_hose(0) calling
>>
>> Hi,
>>
>> On 10 November 2016 at 03:58, Zhiqiang Hou <Zhiqiang.Hou at nxp.com>
>> wrote:
>> > From: Minghuan Lian <Minghuan.Lian at nxp.com>
>> >
>> > There may be multiple PCIe controllers in a SoC.
>> > It is not correct that always calling pci_bus_to_hose(0) to get the
>> > first PCIe controller for the PCIe device connected other controllers.
>> > We just remove this calling because hose always point the correct PCIe
>> > controller.
>> >
>> > Signed-off-by: Minghuan Lian <Minghuan.Lian at nxp.com>
>> > Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou at nxp.com>
>> > ---
>> > V2:
>> >  - No change
>> >
>> >  drivers/pci/pci_common.c | 10 ----------
>> >  1 file changed, 10 deletions(-)
>>
>> So is 'hose' always a root PCI controller now? If so, can you comment these in
>> the header? I'm a bit confused by this
>
> Yes, the patch 02 of this patchset statement that the function pci_bus_to_hose() will return the root pci controller.
> And will add the comment for the function.
>
>>
>> We should perhaps move the regions[] array into its own struct separate from
>> pci_controller.
>
> Could you let this patch set in first? Because I think this is a big task, and so far we have no effort to do that.

Yes - please add a comment as above, and a TODO for yourself.

Reviewed-by: Simon Glass <sjg at chromium.org>

>
>>
>> >
>> > diff --git a/drivers/pci/pci_common.c b/drivers/pci/pci_common.c index
>> > 1755914..448e814 100644
>> > --- a/drivers/pci/pci_common.c
>> > +++ b/drivers/pci/pci_common.c
>> > @@ -181,11 +181,6 @@ phys_addr_t pci_hose_bus_to_phys(struct
>> pci_controller *hose,
>> >                 return phys_addr;
>> >         }
>> >
>> > -#ifdef CONFIG_DM_PCI
>> > -       /* The root controller has the region information */
>> > -       hose = pci_bus_to_hose(0);
>> > -#endif
>> > -
>> >         /*
>> >          * if PCI_REGION_MEM is set we do a two pass search with
>> preference
>> >          * on matches that don't have PCI_REGION_SYS_MEMORY set
>> @@
>> > -248,11 +243,6 @@ pci_addr_t pci_hose_phys_to_bus(struct pci_controller
>> *hose,
>> >                 return bus_addr;
>> >         }
>> >
>> > -#ifdef CONFIG_DM_PCI
>> > -       /* The root controller has the region information */
>> > -       hose = pci_bus_to_hose(0);
>> > -#endif
>> > -
>> >         /*
>> >          * if PCI_REGION_MEM is set we do a two pass search with
>> preference
>> >          * on matches that don't have PCI_REGION_SYS_MEMORY set
>> > --
>> > 2.1.0.27.g96db324
>> >
>
> Thanks,
> Zhiqiang

Regards,
Simon