From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754182AbbJNV5d (ORCPT ); Wed, 14 Oct 2015 17:57:33 -0400 Received: from mout.kundenserver.de ([212.227.126.130]:54509 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752157AbbJNV5a (ORCPT ); Wed, 14 Oct 2015 17:57:30 -0400 From: Arnd Bergmann To: linux-arm-kernel@lists.infradead.org Cc: Zhou Wang , "Wangkefeng (Kevin)" , Gabriele Paoloni , "linux-pci@vger.kernel.org" , "Liviu.Dudau@arm.com" , qiujiang , "lorenzo.pieralisi@arm.com" , "linux@arm.linux.org.uk" , "pratyush.anand@gmail.com" , "xuwei (O)" , Bjorn Helgaas , "gabriel.fernandez@linaro.org" , "liudongdong (C)" , zhangjukuo , qiuzhenfa , "devicetree@vger.kernel.org" , "jason@lakedaemon.net" , Rob Herring , Bjorn Helgaas , "thomas.petazzoni@free-electrons.com" , "jingoohan1@gmail.com" , "linux-kernel@vger.kernel.org" , "Minghuan.Lian@freescale.com" , "james.morse@arm.com" , "Liguozhu (Kenneth)" Subject: Re: [PATCH v10 4/6] PCI: hisi: Add PCIe host support for HiSilicon SoC Hip05 Date: Wed, 14 Oct 2015 23:56:33 +0200 Message-ID: <5572871.GecUyqk59W@wuerfel> User-Agent: KMail/4.11.5 (Linux/3.16.0-10-generic; KDE/4.11.5; x86_64; ; ) In-Reply-To: <561E23EB.7050504@hisilicon.com> References: <1444445957-239522-1-git-send-email-wangzhou1@hisilicon.com> <30775300.COZ4nEMWXC@wuerfel> <561E23EB.7050504@hisilicon.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Provags-ID: V03:K0:WM1NP4mzuuRdSBClt9bHrDN0E2Yu/Kw7eucRtlddDCj5r2pvk4O MH62aLZauzcW+Cp51Y27RUnBZ7Rmtpv47CEJzeLFRnC5S6lLG5iHp55yTv30cvGFbEwKcBn oijtSFMJtzrNo1JwMsvQFSQerGC2kqv67MfcH0ByTU2fUqEWMk8KuyCx1xwLhrqJdon861v Sr2KLdhFbQWhgv9vXECCA== X-UI-Out-Filterresults: notjunk:1;V01:K0:SzeEWynfWrk=:9sa091vA8uLYDAgr7BaSiX ZPgwgyt1f+Z7iA+ErlGKZRX3DC+fKTe2591X+jYlA212xXWamEBY9HS1qc8nVg7R0jbOMbFOc Db0oqHVrz6ElVmd7p3unOf/x4zNmrHuXcJxy/r3DkJBP4DqyiF2lBHM0aTcb4gSrWl/s7E8wO PIwzyoNdxhnnyXBEmt35UYVeRq3NF5RpVQU9YL0O40MIwYeTnHl9mcoTLx2C31Py+rzCI743M wBXx+2UQ3M6EK2J45Yj36ZS6pDVBuLInnthKUhSFh4TjI0kZsxGUYi4ITT52avPhcEW2LAxlG i1KEa7PH21fgCAaW392z4EIZJ4nmMKFkdg9bMNvkccQ6BArPU5HVxb5fOfowyahMAv28AoNCD LSOaH4ryjqz7c47sOIYdSyul1dO41l2M9a0WYnQc5U4JufmkM8kkzmp1sbFJYZ4x8oO/a0L58 WmALxLJ3coFlySNPYYd+zyVZngcQJ0x81vFcp12sZpVhwRRbn7gH/ZwHaS6X8NYO9On7fodhl y3/vSOO+P2+t9VReZm5wc6ZrHBJu/La+mluvX2cr/Zc8+DU2UQ6G4T9WSr4QPJqF1GCqi28g5 Pw/vDN+Zg+B/XTF8xDXXNBA2Zx7kdI3pQ1KMfS56LM/lM9cKx8/Bv0DUnxwGOreNuV5KyD6Hf c0buhOld5x1hzLgsva6/Ay0XAjAspqaL6tBwlPGxBmQlPOZaaYQcR+i6S1yW0R8DwjxRokkrr xlHgRT+5JCz5z+/D Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wednesday 14 October 2015 17:44:11 Zhou Wang wrote: > On 2015/10/14 17:06, Arnd Bergmann wrote: > > On Wednesday 14 October 2015 16:59:03 Zhou Wang wrote: > >> > >> Hi Arnd, > >> > >> In Hip05 PCIe host, it uses GITS_TRANSLATER's address to get TLP package > >> which contains MSI address and MSI data, and then combine BDF and MSI data > >> to a 32 bit data which will be writen to GITS_TRANSLATER register of ITS. > >> > >> I think maybe this is a defect of our PCIe controller. > > > > I'd consider it a bug in the firmware if this is not set up correctly > > before boot. > > > >>> I don't think what you do here is safe because the 'reg' property > >>> of the MSI controller might point to the address that is used for > >>> the message directly. > >> > >> I see your point, however we must get address of GITS_TRANSLATER and > >> set it to PCIe host. How about adding necessary comments here? > > > > This seems to just be static setup that should be done before Linux > > is even loaded. Any reason you can't do it that way? > > > > There are some ITSs in Hip05-D02 platform, in fact, we can use any of them > as a msi-controller, which we can configure in dts. I am afraid that > hard-setting the value in BIOS would lead to restrictions in terms of flexibility, > as with the current implementation the same BIOS-driver can fit different > DTS structures. The dtb generally should be expected to match whatever the firmware sets up, so if there is one reasonable setting here, I see no problem with hardcoding it that way. In particular on server systems, we usually expect the firmware to configure almost everything in advance and just tell us how it is configured, while on embedded systems we can't trust the bootload and usually set it all up in the kernel from scratch. What would be a reason to pick one ITS over another? On a related note, don't you also need to describe in DT how PCI B/D/F function numbers get turned into addresses in the ITS? Does that also require configuration in the driver? I see this code here: + hisi_pcie_apb_writel(pcie, PCIE_MSI_ASID_ENABLE | PCIE_MSI_ASID_VALUE, + PCIE_SLV_MSI_ASID); + hisi_pcie_apb_writel(pcie, PCIE_MSI_TRANS_ENABLE, PCIE_MSI_TRANS_REG); + hisi_pcie_change_apb_mode(pcie, PCIE_SLV_DBI_MODE); plus all of hisi_pcie_config_context(). This looks like it will change the way the MSI is interpreted. This also seems like something that could be done in the firmware in advance, and just get reported in DT. Arnd