From: Zhou Wang <wangzhou1@hisilicon.com>
To: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: James Morse <James.Morse@arm.com>,
Bjorn Helgaas <bhelgaas@google.com>,
Jingoo Han <jg1.han@samsung.com>,
Pratyush Anand <pratyush.anand@gmail.com>,
Arnd Bergmann <arnd@arndb.de>,
"fabrice.gasnier@st.com" <fabrice.gasnier@st.com>,
Liviu Dudau <Liviu.Dudau@arm.com>,
"linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
"gabriele.paoloni@huawei.com" <gabriele.paoloni@huawei.com>,
"yuanzhichang@hisilicon.com" <yuanzhichang@hisilicon.com>,
"zhudacai@hisilicon.com" <zhudacai@hisilicon.com>,
"zhangjukuo@huawei.com" <zhangjukuo@huawei.com>,
"qiuzhenfa@hisilicon.com" <qiuzhenfa@hisilicon.com>,
"liguozhu@hisilicon.com" <liguozhu@hisilicon.com>
Subject: Re: [PATCH v2 2/4] PCI: designware: Add ARM64 support
Date: Thu, 11 Jun 2015 10:51:39 +0800 [thread overview]
Message-ID: <5578F7BB.7080003@hisilicon.com> (raw)
In-Reply-To: <20150609111507.GB20182@red-moon>
On 2015/6/9 19:15, Lorenzo Pieralisi wrote:
> On Fri, Jun 05, 2015 at 09:11:30AM +0100, Zhou Wang wrote:
>
> [...]
>
>>>> -int dw_pcie_host_init(struct pcie_port *pp)
>>>> +int __init dw_pcie_host_init(struct pcie_port *pp)
>>>> {
>>>> struct device_node *np = pp->dev->of_node;
>>>> struct platform_device *pdev = to_platform_device(pp->dev);
>>>> struct of_pci_range range;
>>>> struct of_pci_range_parser parser;
>>>> + struct pci_bus *bus;
>>>> struct resource *cfg_res;
>>>> + LIST_HEAD(res);
>>>> u32 val, na, ns;
>>>> const __be32 *addrp;
>>>> int i, index, ret;
>>>> @@ -502,15 +498,49 @@ int dw_pcie_host_init(struct pcie_port *pp)
>>>> val |= PORT_LOGIC_SPEED_CHANGE;
>>>> dw_pcie_wr_own_conf(pp, PCIE_LINK_WIDTH_SPEED_CONTROL, 4, val);
>>>>
>>>> -#ifdef CONFIG_PCI_MSI
>>>> - dw_pcie_msi_chip.dev = pp->dev;
>>>> - dw_pci.msi_ctrl = &dw_pcie_msi_chip;
>>>> +#ifdef CONFIG_ARM
>>>> + /*
>>>> + * FIXME: we should really be able to use
>>>> + * of_pci_get_host_bridge_resources on arm32 as well,
>>>> + * but the conversion needs some more testing
>>>> + */
>>>> + if (global_io_offset < SZ_1M && pp->io_size > 0) {
>>>> + pci_ioremap_io(global_io_offset, pp->io_base);
>>>> + global_io_offset += SZ_64K;
>>>> + pci_add_resource_offset(&res, &pp->io,
>>>> + global_io_offset - pp->io_bus_addr);
>>>> + }
>>>> + pci_add_resource_offset(&res, &pp->mem,
>>>> + pp->mem.start - pp->mem_bus_addr);
>>>> + pci_add_resource(&res, &pp->busn);
>>>
>>> I don't think this #ifdef is necessary. In the spirit of 'the conversion
>>> needs some more testing', I removed it leaving just the below arm64 code.
>>>
>>> This worked on my Freescale i.MX6 Quad SABRE Lite Board, I went as far as
>>> scanning for wireless access points.
>>>
>>
>> I think it depends on which kind of PCIe device you use, if we use a PCIe device
>> with a I/O Bar, it may not work well without above code. But so far, I have not
>> met a PCIe device which must work with a I/O Bar.
>
> There are two problems here:
>
> 1) the io_base address you get from of_pci_get_host_bridge_resources
> must be mapped using pci_remap_iospace. You are not doing this, so
> even if you had a PCIe card with I/O bar it would not work on arm64
> as the code stands. Remember that I/O space on arm/arm64 works as a
> memory access with offset from PCI_IOBASE and that's the value you get
> in the I/O resource. See:
>
> drivers/pci/host/pci-host-generic.c
> drivers/pci/host/pci-versatile.c
> drivers/pci/host/pci-xgene.c
>
> 2) (1) above would sort out I/O space access for both arm and arm64 so,
> as James said, the #ifdef is useless.
>
> I hope this makes things clearer.
>
> Thanks,
> Lorenzo
>
Hi Lorenzo,
Many thanks for your explanation. I have a better understanding now.
So I think we could remove #ifdef and add pci_remap_iospace after
of_pci_get_host_bridge_resources.
Regards,
Zhou
>>
>>>> +#else
>>>> + ret = of_pci_get_host_bridge_resources(np, 0, 0xff, &res, &pp->io_base);
>>>
>>> of_pci_get_host_bridge_resources expects &pp->io_base to be a
>>> resource_size_t*, but &io_base is u64*. This generates a warning on arm
>>> with the above change. Changing the the type in
>>> drivers/pci/host/pcie-designware.h fixes this.
>>>
>>>
>>> Thanks,
>>>
>>> James
>>>
>>
>> OK, will modify this in next version.
>>
>> Best Regards and thanks again for your test,
>> Zhou
>>
>>>> + if (ret)
>>>> + return ret;
>>>> +#endif
>>>> +
>>>> + bus = pci_create_root_bus(pp->dev, pp->root_bus_nr, &dw_pcie_ops,
>>>> + pp, &res);
>>>> + if (!bus)
>>>> + return -ENOMEM;
>>>> +
>>>> +#ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN
>>>> + bus->msi = container_of(&pp->irq_domain, struct msi_controller, domain);
>>>> +#else
>>>> + bus->msi = &dw_pcie_msi_chip;
>>>> #endif
>>>>
>>>> - dw_pci.nr_controllers = 1;
>>>> - dw_pci.private_data = (void **)&pp;
>>>> + pci_scan_child_bus(bus);
>>>> + if (pp->ops->scan_bus)
>>>> + pp->ops->scan_bus(pp);
>>>>
>>>> - pci_common_init_dev(pp->dev, &dw_pci);
>>>> +#ifdef CONFIG_ARM
>>>> + /* support old dtbs that incorrectly describe IRQs */
>>>> + pci_fixup_irqs(pci_common_swizzle, of_irq_parse_and_map_pci);
>>>> +#endif
>>>> +
>>>> + pci_assign_unassigned_bus_resources(bus);
>>>> + pci_bus_add_devices(bus);
>>>>
>>>> return 0;
>>>> }
>>>> @@ -653,7 +683,7 @@ static int dw_pcie_valid_config(struct pcie_port *pp,
>>>> static int dw_pcie_rd_conf(struct pci_bus *bus, u32 devfn, int where,
>>>> int size, u32 *val)
>>>> {
>>>> - struct pcie_port *pp = sys_to_pcie(bus->sysdata);
>>>> + struct pcie_port *pp = bus->sysdata;
>>>> int ret;
>>>>
>>>> if (dw_pcie_valid_config(pp, bus, PCI_SLOT(devfn)) == 0) {
>>>> @@ -677,7 +707,7 @@ static int dw_pcie_rd_conf(struct pci_bus *bus, u32 devfn, int where,
>>>> static int dw_pcie_wr_conf(struct pci_bus *bus, u32 devfn,
>>>> int where, int size, u32 val)
>>>> {
>>>> - struct pcie_port *pp = sys_to_pcie(bus->sysdata);
>>>> + struct pcie_port *pp = bus->sysdata;
>>>> int ret;
>>>>
>>>> if (dw_pcie_valid_config(pp, bus, PCI_SLOT(devfn)) == 0)
>>>> @@ -701,64 +731,6 @@ static struct pci_ops dw_pcie_ops = {
>>>> .write = dw_pcie_wr_conf,
>>>> };
>>>>
>>>> -static int dw_pcie_setup(int nr, struct pci_sys_data *sys)
>>>> -{
>>>> - struct pcie_port *pp;
>>>> -
>>>> - pp = sys_to_pcie(sys);
>>>> -
>>>> - if (global_io_offset < SZ_1M && pp->io_size > 0) {
>>>> - sys->io_offset = global_io_offset - pp->io_bus_addr;
>>>> - pci_ioremap_io(global_io_offset, pp->io_base);
>>>> - global_io_offset += SZ_64K;
>>>> - pci_add_resource_offset(&sys->resources, &pp->io,
>>>> - sys->io_offset);
>>>> - }
>>>> -
>>>> - sys->mem_offset = pp->mem.start - pp->mem_bus_addr;
>>>> - pci_add_resource_offset(&sys->resources, &pp->mem, sys->mem_offset);
>>>> - pci_add_resource(&sys->resources, &pp->busn);
>>>> -
>>>> - return 1;
>>>> -}
>>>> -
>>>> -static struct pci_bus *dw_pcie_scan_bus(int nr, struct pci_sys_data *sys)
>>>> -{
>>>> - struct pci_bus *bus;
>>>> - struct pcie_port *pp = sys_to_pcie(sys);
>>>> -
>>>> - pp->root_bus_nr = sys->busnr;
>>>> - bus = pci_create_root_bus(pp->dev, sys->busnr,
>>>> - &dw_pcie_ops, sys, &sys->resources);
>>>> - if (!bus)
>>>> - return NULL;
>>>> -
>>>> - pci_scan_child_bus(bus);
>>>> -
>>>> - if (bus && pp->ops->scan_bus)
>>>> - pp->ops->scan_bus(pp);
>>>> -
>>>> - return bus;
>>>> -}
>>>> -
>>>> -static int dw_pcie_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
>>>> -{
>>>> - struct pcie_port *pp = sys_to_pcie(dev->bus->sysdata);
>>>> - int irq;
>>>> -
>>>> - irq = of_irq_parse_and_map_pci(dev, slot, pin);
>>>> - if (!irq)
>>>> - irq = pp->irq;
>>>> -
>>>> - return irq;
>>>> -}
>>>> -
>>>> -static struct hw_pci dw_pci = {
>>>> - .setup = dw_pcie_setup,
>>>> - .scan = dw_pcie_scan_bus,
>>>> - .map_irq = dw_pcie_map_irq,
>>>> -};
>>>> -
>>>> void dw_pcie_setup_rc(struct pcie_port *pp)
>>>> {
>>>> u32 val;
>>>
>>>
>>> .
>>>
>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>
> .
>
next prev parent reply other threads:[~2015-06-11 2:53 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-06-03 8:35 [PATCH v2 0/4] PCI: hisi: Add PCIe host support for Hisilicon Soc Hip05 Zhou Wang
2015-06-03 8:35 ` [PATCH v2 1/4] ARM/PCI: remove align_resource callback in pcibios_align_resource Zhou Wang
2015-06-04 13:18 ` James Morse
2015-06-05 3:53 ` Zhou Wang
2015-06-03 8:35 ` [PATCH v2 2/4] PCI: designware: Add ARM64 support Zhou Wang
2015-06-04 13:19 ` James Morse
2015-06-05 8:11 ` Zhou Wang
2015-06-09 11:15 ` Lorenzo Pieralisi
2015-06-10 13:35 ` Gabriele Paoloni
2015-06-11 5:44 ` Zhou Wang
2015-06-11 14:32 ` Gabriele Paoloni
2015-06-16 11:30 ` Zhou Wang
2015-06-16 14:14 ` Gabriele Paoloni
2015-07-13 10:58 ` Lorenzo Pieralisi
2015-07-13 11:45 ` Zhou Wang
2015-07-13 14:17 ` Lorenzo Pieralisi
2015-07-17 10:45 ` Gabriele Paoloni
2015-06-11 2:51 ` Zhou Wang [this message]
2015-06-11 2:44 ` Zhou Wang
2015-06-14 19:18 ` Pratyush Anand
2015-06-16 10:14 ` Zhou Wang
2015-06-03 8:35 ` [PATCH v2 3/4] PCI: hisi: Add PCIe host support for Hisilicon Soc Hip05 Zhou Wang
2015-06-04 13:23 ` James Morse
2015-06-05 8:13 ` Zhou Wang
2015-06-03 8:35 ` [PATCH v2 4/4] Documentation: DT: Add Hisilicon PCIe host binding Zhou Wang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5578F7BB.7080003@hisilicon.com \
--to=wangzhou1@hisilicon.com \
--cc=James.Morse@arm.com \
--cc=Liviu.Dudau@arm.com \
--cc=arnd@arndb.de \
--cc=bhelgaas@google.com \
--cc=devicetree@vger.kernel.org \
--cc=fabrice.gasnier@st.com \
--cc=gabriele.paoloni@huawei.com \
--cc=jg1.han@samsung.com \
--cc=liguozhu@hisilicon.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-pci@vger.kernel.org \
--cc=lorenzo.pieralisi@arm.com \
--cc=pratyush.anand@gmail.com \
--cc=qiuzhenfa@hisilicon.com \
--cc=yuanzhichang@hisilicon.com \
--cc=zhangjukuo@huawei.com \
--cc=zhudacai@hisilicon.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).