RE: [PATCH V6 2/5] PCI/ACPI: Check platform specific ECAM quirks

From	Gabriele Paoloni <gabriele.paoloni@huawei.com>
Newsgroups	linux.kernel
Subject	RE: [PATCH V6 2/5] PCI/ACPI: Check platform specific ECAM quirks
Date	2016-09-16 11:10 +0200
Message-ID	<shXjI-2Fs-7@gated-at.bofh.it> (permalink)
References	(1 earlier) <sfzER-6Yg-9@gated-at.bofh.it> <sgLND-3Sk-1@gated-at.bofh.it> <sgPxT-6xI-3@gated-at.bofh.it> <sgUed-1cs-19@gated-at.bofh.it> <shCyC-5VC-11@gated-at.bofh.it>
Organization	linux.* mail to news gateway

Show all headers | View raw

Hi Lorenzo and Tomasz

Many Thanks for looking at this

> -----Original Message-----
> From: Lorenzo Pieralisi [mailto:lorenzo.pieralisi@arm.com]
> Sent: 15 September 2016 11:59
> To: liudongdong (C)
> Cc: Tomasz Nowicki; helgaas@kernel.org; will.deacon@arm.com;
> catalin.marinas@arm.com; rafael@kernel.org; arnd@arndb.de;
> hanjun.guo@linaro.org; okaya@codeaurora.org; jchandra@broadcom.com;
> cov@codeaurora.org; dhdang@apm.com; ard.biesheuvel@linaro.org;
> robert.richter@caviumnetworks.com; mw@semihalf.com;
> Liviu.Dudau@arm.com; ddaney@caviumnetworks.com; Wangyijing;
> msalter@redhat.com; linux-pci@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; linaro-acpi@lists.linaro.org;
> jcm@redhat.com; andrea.gallo@linaro.org; jeremy.linton@arm.com;
> Gabriele Paoloni; jhugo@codeaurora.org; linux-acpi@vger.kernel.org;
> linux-kernel@vger.kernel.org
> 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

Thanks

Gab

> 
> Thanks,
> Lorenzo
> 
> > This patch is based on RFC V5 quirk mechanism.
> >
> > Based on V6 quirk mechanism, we have to change it as below:
> >
> > #ifdef CONFIG_PCI_HISI_ACPI
> > 	{ "HISI ", "HIP05 ", 0, 0, MCFG_BUS_ANY, &hisi_pcie_hip05_ops,
> > 	  MCFG_RES_EMPTY},
> > 	{ "HISI ", "HIP05 ", 0, 1, MCFG_BUS_ANY, &hisi_pcie_hip05_ops,
> > 	  MCFG_RES_EMPTY},
> > 	{ "HISI ", "HIP05 ", 0, 2, MCFG_BUS_ANY, &hisi_pcie_hip05_ops,
> > 	  MCFG_RES_EMPTY},
> > 	{ "HISI ", "HIP05 ", 0, 3, MCFG_BUS_ANY, &hisi_pcie_hip05_ops,
> > 	  MCFG_RES_EMPTY},
> > 	{ "HISI ", "HIP06 ", 0, 0, MCFG_BUS_ANY, &hisi_pcie_hip06_ops,
> > 	  MCFG_RES_EMPTY},
> > 	{ "HISI ", "HIP06 ", 0, 1, MCFG_BUS_ANY, &hisi_pcie_hip06_ops,
> > 	  MCFG_RES_EMPTY},
> > 	{ "HISI ", "HIP06 ", 0, 2, MCFG_BUS_ANY, &hisi_pcie_hip06_ops,
> > 	  MCFG_RES_EMPTY},
> > 	{ "HISI ", "HIP06 ", 0, 3, MCFG_BUS_ANY, &hisi_pcie_hip06_ops,
> >          MCFG_RES_EMPTY},
> > 	{ "HISI ", "HIP07 ", 0, 0, MCFG_BUS_ANY, &hisi_pcie_hip07_ops,
> > 	  MCFG_RES_EMPTY},
> > 	{ "HISI ", "HIP07 ", 0, 1, MCFG_BUS_ANY, &hisi_pcie_hip07_ops,
> > 	  MCFG_RES_EMPTY},
> > 	....
> >
> > 	{ "HISI ", "HIP07 ", 0, 15, MCFG_BUS_ANY, &hisi_pcie_hip07_ops,
> > 	  MCFG_RES_EMPTY},
> >
> > #endif
> >
> > struct pci_ecam_ops hisi_pci_hip05_ops = {
> > 	.bus_shift	= 20,
> > 	.init		=  hisi_pci_hip05_init,
> > 	.pci_ops	= {
> > 		.map_bus	= pci_ecam_map_bus,
> > 		.read		= hisi_pcie_acpi_rd_conf,
> > 		.write		= hisi_pcie_acpi_wr_conf,
> > 	}
> > };
> >
> > struct pci_ecam_ops hisi_pci_hip06_ops = {
> > 	.bus_shift = 20,
> > 	.init = hisi_pci_hip06_init,
> > 	.pci_ops = {
> > 		.map_bus = pci_ecam_map_bus,
> > 		.read = hisi_pcie_acpi_rd_conf,
> > 		.write = hisi_pcie_acpi_wr_conf,
> > 	}
> > };
> >
> > hisi_pci_hipxx_init function is used to get RC config resource with
> hardcode.
> > .....
> >
> > So I hope we can use MCFG_DOM_RANGE, Then I can change it as below.
> >
> > #ifdef CONFIG_PCI_HISI_ACPI
> > 	{ "HISI  ", "HIP05   ", 0, MCFG_DOM_RANGE(0, 3), MCFG_BUS_ANY,
> > 	 &hisi_pcie_hip05_ops, MCFG_RES_EMPTY},
> > 	{ "HISI  ", "HIP06   ", 0, MCFG_DOM_RANGE(0, 3), MCFG_BUS_ANY,
> > 	 &hisi_pcie_hip06_ops, MCFG_RES_EMPTY},
> > 	{ "HISI  ", "HIP07   ", 0, MCFG_DOM_RANGE(0, 15), MCFG_BUS_ANY,
> > 	  &hisi_pcie_hip07_ops, MCFG_RES_EMPTY},
> > #endif
> >
> > Thanks
> > Dongdong
> > >
> > >Thanks,
> > >Tomasz
> > >
> > >.
> > >
> >

Thread

[PATCH V6 2/5] PCI/ACPI: Check platform specific ECAM quirks Tomasz Nowicki <tn@semihalf.com> - 2016-09-09 21:30 +0200
  Re: [PATCH V6 2/5] PCI/ACPI: Check platform specific ECAM quirks Duc Dang <dhdang@apm.com> - 2016-09-13 00:30 +0200
    Re: [PATCH V6 2/5] PCI/ACPI: Check platform specific ECAM quirks Duc Dang <dhdang@apm.com> - 2016-09-13 00:50 +0200
      Re: [PATCH V6 2/5] PCI/ACPI: Check platform specific ECAM quirks Tomasz Nowicki <tn@semihalf.com> - 2016-09-13 08:00 +0200
    Re: [PATCH V6 2/5] PCI/ACPI: Check platform specific ECAM quirks Tomasz Nowicki <tn@semihalf.com> - 2016-09-13 08:40 +0200
  Re: [PATCH V6 2/5] PCI/ACPI: Check platform specific ECAM quirks Dongdong Liu <liudongdong3@huawei.com> - 2016-09-13 04:40 +0200
    Re: [PATCH V6 2/5] PCI/ACPI: Check platform specific ECAM quirks Tomasz Nowicki <tn@semihalf.com> - 2016-09-13 08:40 +0200
      Re: [PATCH V6 2/5] PCI/ACPI: Check platform specific ECAM quirks Dongdong Liu <liudongdong3@huawei.com> - 2016-09-13 13:40 +0200
        Re: [PATCH V6 2/5] PCI/ACPI: Check platform specific ECAM quirks Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> - 2016-09-14 14:50 +0200
        Re: [PATCH V6 2/5] PCI/ACPI: Check platform specific ECAM quirks Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> - 2016-09-15 13:00 +0200
          RE: [PATCH V6 2/5] PCI/ACPI: Check platform specific ECAM quirks Gabriele Paoloni <gabriele.paoloni@huawei.com> - 2016-09-16 11:10 +0200
            Re: [PATCH V6 2/5] PCI/ACPI: Check platform specific ECAM quirks Christopher Covington <cov@codeaurora.org> - 2016-09-16 14:30 +0200
              RE: [PATCH V6 2/5] PCI/ACPI: Check platform specific ECAM quirks Gabriele Paoloni <gabriele.paoloni@huawei.com> - 2016-09-16 15:50 +0200

csiph-web