Hi Cov > -----Original Message----- > From: Christopher Covington [mailto:cov@xxxxxxxxxxxxxx] > Sent: 16 September 2016 13:28 > To: Gabriele Paoloni; Lorenzo Pieralisi; liudongdong (C) > Cc: Tomasz Nowicki; helgaas@xxxxxxxxxx; will.deacon@xxxxxxx; > catalin.marinas@xxxxxxx; rafael@xxxxxxxxxx; arnd@xxxxxxxx; > hanjun.guo@xxxxxxxxxx; okaya@xxxxxxxxxxxxxx; jchandra@xxxxxxxxxxxx; > dhdang@xxxxxxx; ard.biesheuvel@xxxxxxxxxx; > robert.richter@xxxxxxxxxxxxxxxxxx; mw@xxxxxxxxxxxx; > Liviu.Dudau@xxxxxxx; ddaney@xxxxxxxxxxxxxxxxxx; Wangyijing; > msalter@xxxxxxxxxx; linux-pci@xxxxxxxxxxxxxxx; linux-arm- > kernel@xxxxxxxxxxxxxxxxxxx; linaro-acpi@xxxxxxxxxxxxxxxx; > jcm@xxxxxxxxxx; andrea.gallo@xxxxxxxxxx; jeremy.linton@xxxxxxx; > jhugo@xxxxxxxxxxxxxx; linux-acpi@xxxxxxxxxxxxxxx; linux- > kernel@xxxxxxxxxxxxxxx > Subject: Re: [PATCH V6 2/5] PCI/ACPI: Check platform specific ECAM > quirks > > On 09/16/2016 05:02 AM, Gabriele Paoloni wrote: > > Hi Lorenzo and Tomasz > > > > Many Thanks for looking at this > > > >> -----Original Message----- > >> From: Lorenzo Pieralisi [mailto:lorenzo.pieralisi@xxxxxxx] > >> Sent: 15 September 2016 11:59 > >> To: liudongdong (C) > >> Cc: Tomasz Nowicki; helgaas@xxxxxxxxxx; will.deacon@xxxxxxx; > >> catalin.marinas@xxxxxxx; rafael@xxxxxxxxxx; arnd@xxxxxxxx; > >> hanjun.guo@xxxxxxxxxx; okaya@xxxxxxxxxxxxxx; jchandra@xxxxxxxxxxxx; > >> cov@xxxxxxxxxxxxxx; dhdang@xxxxxxx; ard.biesheuvel@xxxxxxxxxx; > >> robert.richter@xxxxxxxxxxxxxxxxxx; mw@xxxxxxxxxxxx; > >> Liviu.Dudau@xxxxxxx; ddaney@xxxxxxxxxxxxxxxxxx; Wangyijing; > >> msalter@xxxxxxxxxx; linux-pci@xxxxxxxxxxxxxxx; linux-arm- > >> kernel@xxxxxxxxxxxxxxxxxxx; linaro-acpi@xxxxxxxxxxxxxxxx; > >> jcm@xxxxxxxxxx; andrea.gallo@xxxxxxxxxx; jeremy.linton@xxxxxxx; > >> Gabriele Paoloni; jhugo@xxxxxxxxxxxxxx; linux-acpi@xxxxxxxxxxxxxxx; > >> linux-kernel@xxxxxxxxxxxxxxx > >> Subject: Re: [PATCH V6 2/5] PCI/ACPI: Check platform specific ECAM > >> quirks > >> > >> On Tue, Sep 13, 2016 at 07:38:39PM +0800, Dongdong Liu wrote: > >> > >> [...] > >> > >>> Our host bridge is non ECAM only for the RC bus config space; > >>> for any other bus underneath the root bus we support ECAM access. > >>> > >>> RC config resource with hardcode as DEFINE_RES_MEM(0xb0070000, > >> SZ_4K), > >>> EP config resource we get it from MCFG table. > >>> So we need to override ops, but config resource we only need to > >> hardcode with RC config resource. > >>> > >>> Our host controller ACPI support patch can be found: > >>> https://lkml.org/lkml/2016/8/31/340 > >> > >> Sorry I misread your code. IIUC you create an array of resources > that > >> represent non-ECAM config space (and incidentally contain debug > >> registers to check the link status - that you need to check for > every > >> given config access (?)), but you still need to have an MCFG entry > that > > > > The link status check is inherited from the designware framework (see > > http://lxr.free-electrons.com/source/drivers/pci/host/pcie- > designware.c#L678) > > > > However I think that in this case we can just check the link status > > in our init function and return an error if the link is down > > > >> covers the bus number subject to quirk to make sure this mechanism > >> works. Correct ? > > > > Well we need the quirks for the root bus numbers but if read this v6 > > quirk mechanism correctly even if we do not specify an mcfg entry for > > bus 0 oci_mcfg_match_quirks() is called anyway and we can set our > > special configuration space addresses for the root buses (i.e. I > think > > we can have a clean MCFG table with entries only for those bus ranges > > that are really ECAM) > > > >> > >> This also means that, with the MCFG tables you have and current > >> mainline kernel you are able to probe a root bridge (because the > MCFG > >> table covers the bus number that is not ECAM), with enumeration > >> going haywire because it is trying to carry out ECAM accesses on > >> non-ECAM space. > > > > Yes correct, we cannot access the host controller configuration space > > with our current MCFG table and current Linux mainline > > > >> > >> Is my reading correct ? > >> > >> Anyway, that's not stricly related to this discussion, it is time we > >> converge on this patchset, we can add a domain range if that > >> simplifies things. > > > > IMO it would be better to have the domain range to avoid > > a very large and repetitive static quirk array > > The v6 framework requires what, 21 additional lines of quirk data? What > if you were to define some preprocessor macros to slim it down? I think > something like the following would bring it down to roughly 7 > additional > lines. Thanks, yes this would work as well. To be honest this is not a big issue. So either we go this way or we introduce domain range... Cheers Gab > > #define PCI_ACPI_QUIRK_QUAD_DOM(rev, dom, ops) \ > { "HISI ", rev, 0, dom+0, MCFG_BUS_ANY, ops, > MCFG_RES_EMPTY}, \ > { "HISI ", rev, 0, dom+1, MCFG_BUS_ANY, ops, > MCFG_RES_EMPTY}, \ > { "HISI ", rev, 0, dom+2, MCFG_BUS_ANY, ops, > MCFG_RES_EMPTY}, \ > { "HISI ", rev, 0, dom+3, MCFG_BUS_ANY, ops, > MCFG_RES_EMPTY}, > > PCI_ACPI_QUIRK_QUAD_DOM("HIP05 ", 0, &hisi_pcie_hip05_ops) > PCI_ACPI_QUIRK_QUAD_DOM("HIP06 ", 0, &hisi_pcie_hip05_ops) > PCI_ACPI_QUIRK_QUAD_DOM("HIP07 ", 0, &hisi_pcie_hip06_ops) > PCI_ACPI_QUIRK_QUAD_DOM("HIP07 ", 4, &hisi_pcie_hip07_ops) > PCI_ACPI_QUIRK_QUAD_DOM("HIP07 ", 8, &hisi_pcie_hip07_ops) > PCI_ACPI_QUIRK_QUAD_DOM("HIP07 ", 12, &hisi_pcie_hip07_ops) > > Cov > -- > Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm > Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code > Aurora Forum, a Linux Foundation Collaborative Project. -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html