From: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> To: Dongdong Liu <liudongdong3@huawei.com> Cc: Tomasz Nowicki <tn@semihalf.com>, 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@huawei.com, 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@huawei.com, 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 Date: Wed, 14 Sep 2016 13:40:16 +0100 [thread overview] Message-ID: <20160914124016.GA4123@red-moon> (raw) In-Reply-To: <57D7E53F.1000106@huawei.com> On Tue, Sep 13, 2016 at 07:38:39PM +0800, Dongdong Liu wrote: > Hi Tomasz > > ?? 2016/9/13 14:32, Tomasz Nowicki ????: > >Hi Liu, > > > >On 13.09.2016 04:36, Dongdong Liu wrote: > >>Hi Tomasz > >> > >>?? 2016/9/10 3:24, Tomasz Nowicki ????: > >>>Some platforms may not be fully compliant with generic set of PCI config > >>>accessors. For these cases we implement the way to overwrite CFG > >>>accessors > >>>set and configuration space range. > >>> > >>>In first place pci_mcfg_parse() saves machine's IDs and revision number > >>>(these come from MCFG header) in order to match against known quirk > >>>entries. > >>>Then the algorithm traverses available quirk list (static array), > >>>matches against <oem_id, oem_table_id, rev, domain, bus number range> and > >>>returns custom PCI config ops and/or CFG resource structure. > >>> > >>>When adding new quirk there are two possibilities: > >>>1. Override default pci_generic_ecam_ops ops but CFG resource comes > >>>from MCFG > >>>{ "OEM_ID", "OEM_TABLE_ID", <REV>, <DOMAIN>, <BUS_NR>, &foo_ops, > >>>MCFG_RES_EMPTY }, > >>>2. Override default pci_generic_ecam_ops ops and CFG resource. For > >>>this case > >>>it is also allowed get CFG resource from quirk entry w/o having it in > >>>MCFG. > >>>{ "OEM_ID", "OEM_TABLE_ID", <REV>, <DOMAIN>, <BUS_NR>, &boo_ops, > >>> DEFINE_RES_MEM(START, SIZE) }, > >>> > >>>pci_generic_ecam_ops and MCFG entries will be used for platforms > >>>free from quirks. > >>> > >>>Signed-off-by: Tomasz Nowicki <tn@semihalf.com> > >>>Signed-off-by: Dongdong Liu <liudongdong3@huawei.com> > >>>Signed-off-by: Christopher Covington <cov@codeaurora.org> > >>>--- > >>> drivers/acpi/pci_mcfg.c | 80 > >>>+++++++++++++++++++++++++++++++++++++++++++++---- > >>> 1 file changed, 74 insertions(+), 6 deletions(-) > >>> > >>>diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c > >>>index ffcc651..2b8acc7 100644 > >>>--- a/drivers/acpi/pci_mcfg.c > >>>+++ b/drivers/acpi/pci_mcfg.c > >>>@@ -32,6 +32,59 @@ struct mcfg_entry { > >>> u8 bus_start; > >>> u8 bus_end; > >>> }; > >>>+struct mcfg_fixup { > >>>+ char oem_id[ACPI_OEM_ID_SIZE + 1]; > >>>+ char oem_table_id[ACPI_OEM_TABLE_ID_SIZE + 1]; > >>>+ u32 oem_revision; > >>>+ u16 seg; > >>>+ struct resource bus_range; > >>>+ struct pci_ecam_ops *ops; > >>>+ struct resource cfgres; > >>>+}; > >>>+ > >>>+#define MCFG_DOM_ANY (-1) > >>>+#define MCFG_BUS_RANGE(start, end) DEFINE_RES_NAMED((start), \ > >>>+ ((end) - (start) + 1), \ > >>>+ NULL, IORESOURCE_BUS) > >>>+#define MCFG_BUS_ANY MCFG_BUS_RANGE(0x0, 0xff) > >>>+#define MCFG_RES_EMPTY DEFINE_RES_NAMED(0, 0, NULL, 0) > >>>+ > >>>+static struct mcfg_fixup mcfg_quirks[] = { > >>>+/* { OEM_ID, OEM_TABLE_ID, REV, DOMAIN, BUS_RANGE, cfgres, ops }, */ > >>>+}; > >>>+ > >>>+static char mcfg_oem_id[ACPI_OEM_ID_SIZE]; > >>>+static char mcfg_oem_table_id[ACPI_OEM_TABLE_ID_SIZE]; > >>>+static u32 mcfg_oem_revision; > >>>+ > >>>+static void pci_mcfg_match_quirks(struct acpi_pci_root *root, > >>>+ struct resource *cfgres, > >>>+ struct pci_ecam_ops **ecam_ops) > >>>+{ > >>>+ struct mcfg_fixup *f; > >>>+ int i; > >>>+ > >>>+ /* > >>>+ * First match against PCI topology <domain:bus> then use OEM ID, > >>>OEM > >>>+ * table ID, and OEM revision from MCFG table standard header. > >>>+ */ > >>>+ for (i = 0, f = mcfg_quirks; i < ARRAY_SIZE(mcfg_quirks); i++, > >>>f++) { > >>>+ if (f->seg == root->segment && > >> > >>why not use MCFG_DOM_RANGE, I think MCFG_DOM_RANGE is better. > >>if drop MCFG_DOM_RANGE, mcfg_quirks[] will be more complex. > >> > >>static struct mcfg_fixup mcfg_quirks[] = { > >>/* { OEM_ID, OEM_TABLE_ID, REV, DOMAIN, BUS_RANGE, cfgres, ops }, */ > >>#ifdef CONFIG_PCI_HOST_THUNDER_ECAM > >> /* SoC pass1.x */ > >> { "CAVIUM", "THUNDERX", 2, 0, MCFG_BUS_ANY, &pci_thunder_ecam_ops, > >> MCFG_RES_EMPTY}, > >> { "CAVIUM", "THUNDERX", 2, 1, MCFG_BUS_ANY, &pci_thunder_ecam_ops, > >> MCFG_RES_EMPTY}, > >> { "CAVIUM", "THUNDERX", 2, 2, MCFG_BUS_ANY, &pci_thunder_ecam_ops, > >> MCFG_RES_EMPTY}, > >> { "CAVIUM", "THUNDERX", 2, 3, MCFG_BUS_ANY, &pci_thunder_ecam_ops, > >> MCFG_RES_EMPTY}, > >> { "CAVIUM", "THUNDERX", 2, 10, MCFG_BUS_ANY, &pci_thunder_ecam_ops, > >> MCFG_RES_EMPTY}, > >> { "CAVIUM", "THUNDERX", 2, 11, MCFG_BUS_ANY, &pci_thunder_ecam_ops, > >> MCFG_RES_EMPTY}, > >> { "CAVIUM", "THUNDERX", 2, 12, MCFG_BUS_ANY, &pci_thunder_ecam_ops, > >> MCFG_RES_EMPTY}, > >> { "CAVIUM", "THUNDERX", 2, 13, MCFG_BUS_ANY, &pci_thunder_ecam_ops, > >> MCFG_RES_EMPTY}, > >>#endif > >>..... > >>}; > >> > >>As PATCH v5 we only need define mcfg_quirks as below, It looks better. > >>static struct pci_cfg_fixup mcfg_quirks[] __initconst = { > >>/* { OEM_ID, OEM_TABLE_ID, REV, DOMAIN, BUS_RANGE, pci_ops, init_hook > >>}, */ > >>#ifdef CONFIG_PCI_HOST_THUNDER_PEM > >> /* Pass2.0 */ > >> { "CAVIUM", "THUNDERX", 1, MCFG_DOM_RANGE(4, 9), MCFG_BUS_ANY, NULL, > >> thunder_pem_cfg_init }, > >> { "CAVIUM", "THUNDERX", 1, MCFG_DOM_RANGE(14, 19), MCFG_BUS_ANY, NULL, > >> thunder_pem_cfg_init }, > >>#endif > >>#ifdef CONFIG_PCI_HISI_ACPI > >> { "HISI ", "HIP05 ", 0, MCFG_DOM_RANGE(0, 3), MCFG_BUS_ANY, > >> NULL, hisi_pcie_acpi_hip05_init}, > >> { "HISI ", "HIP06 ", 0, MCFG_DOM_RANGE(0, 3), MCFG_BUS_ANY, > >> NULL, hisi_pcie_acpi_hip06_init}, > >> { "HISI ", "HIP07 ", 0, MCFG_DOM_RANGE(0, 15), MCFG_BUS_ANY, > >> NULL, hisi_pcie_acpi_hip07_init}, > >>#endif > >>}; > > > >Note this series disallow hisi_pcie_acpi_hip07_init() call. According to the Bjorn suggestion I rework quirk code to override ops and CFG resources only. Giving that I do not see the way to use MCFG_DOM_RANGE macro. For HISI you would need to get CFG range for each possible case: > > > >#ifdef CONFIG_PCI_HISI_ACPI > > { "HISI ", "HIP05 ", 0, 0, MCFG_BUS_ANY, &hisi_pcie_ops, > > DEFINE_RES_MEM(start0, size0)}, > > { "HISI ", "HIP05 ", 0, 1, MCFG_BUS_ANY, &hisi_pcie_ops, > > DEFINE_RES_MEM(start1, size1)}, > > { "HISI ", "HIP05 ", 0, 2, MCFG_BUS_ANY, &hisi_pcie_ops, > > DEFINE_RES_MEM(start2, size2)}, > > { "HISI ", "HIP05 ", 0, 3, MCFG_BUS_ANY, &hisi_pcie_ops, > > DEFINE_RES_MEM(start3, size3)}, > >[...] > >#endif > > > >Indeed there are more entries here but you do not have to define the same resource array in driver. > > 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 > This patch is based on RFC V5 quirk mechanism. > > Based on V6 quirk mechanism, we have to change it as below: That's because you are hacking around the quirk mechanism to define resources that have nothing to do with PCI config regions (that you ioremap and use to check the PCI link status) and of top of that you are *still* using the MCFG to describe config regions that are NOT ECAM compliant, which is the exact opposite of what this patchset is meant to achieve. If a config region is not ECAM compliant having it defined in the MCFG is a firmware bug and the way you are using this patchset is not what we designed it for, please correct me if I am wrong. Thanks, Lorenzo > > #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 > > > >. > > >
WARNING: multiple messages have this Message-ID (diff)
From: lorenzo.pieralisi@arm.com (Lorenzo Pieralisi) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH V6 2/5] PCI/ACPI: Check platform specific ECAM quirks Date: Wed, 14 Sep 2016 13:40:16 +0100 [thread overview] Message-ID: <20160914124016.GA4123@red-moon> (raw) In-Reply-To: <57D7E53F.1000106@huawei.com> On Tue, Sep 13, 2016 at 07:38:39PM +0800, Dongdong Liu wrote: > Hi Tomasz > > ?? 2016/9/13 14:32, Tomasz Nowicki ????: > >Hi Liu, > > > >On 13.09.2016 04:36, Dongdong Liu wrote: > >>Hi Tomasz > >> > >>?? 2016/9/10 3:24, Tomasz Nowicki ????: > >>>Some platforms may not be fully compliant with generic set of PCI config > >>>accessors. For these cases we implement the way to overwrite CFG > >>>accessors > >>>set and configuration space range. > >>> > >>>In first place pci_mcfg_parse() saves machine's IDs and revision number > >>>(these come from MCFG header) in order to match against known quirk > >>>entries. > >>>Then the algorithm traverses available quirk list (static array), > >>>matches against <oem_id, oem_table_id, rev, domain, bus number range> and > >>>returns custom PCI config ops and/or CFG resource structure. > >>> > >>>When adding new quirk there are two possibilities: > >>>1. Override default pci_generic_ecam_ops ops but CFG resource comes > >>>from MCFG > >>>{ "OEM_ID", "OEM_TABLE_ID", <REV>, <DOMAIN>, <BUS_NR>, &foo_ops, > >>>MCFG_RES_EMPTY }, > >>>2. Override default pci_generic_ecam_ops ops and CFG resource. For > >>>this case > >>>it is also allowed get CFG resource from quirk entry w/o having it in > >>>MCFG. > >>>{ "OEM_ID", "OEM_TABLE_ID", <REV>, <DOMAIN>, <BUS_NR>, &boo_ops, > >>> DEFINE_RES_MEM(START, SIZE) }, > >>> > >>>pci_generic_ecam_ops and MCFG entries will be used for platforms > >>>free from quirks. > >>> > >>>Signed-off-by: Tomasz Nowicki <tn@semihalf.com> > >>>Signed-off-by: Dongdong Liu <liudongdong3@huawei.com> > >>>Signed-off-by: Christopher Covington <cov@codeaurora.org> > >>>--- > >>> drivers/acpi/pci_mcfg.c | 80 > >>>+++++++++++++++++++++++++++++++++++++++++++++---- > >>> 1 file changed, 74 insertions(+), 6 deletions(-) > >>> > >>>diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c > >>>index ffcc651..2b8acc7 100644 > >>>--- a/drivers/acpi/pci_mcfg.c > >>>+++ b/drivers/acpi/pci_mcfg.c > >>>@@ -32,6 +32,59 @@ struct mcfg_entry { > >>> u8 bus_start; > >>> u8 bus_end; > >>> }; > >>>+struct mcfg_fixup { > >>>+ char oem_id[ACPI_OEM_ID_SIZE + 1]; > >>>+ char oem_table_id[ACPI_OEM_TABLE_ID_SIZE + 1]; > >>>+ u32 oem_revision; > >>>+ u16 seg; > >>>+ struct resource bus_range; > >>>+ struct pci_ecam_ops *ops; > >>>+ struct resource cfgres; > >>>+}; > >>>+ > >>>+#define MCFG_DOM_ANY (-1) > >>>+#define MCFG_BUS_RANGE(start, end) DEFINE_RES_NAMED((start), \ > >>>+ ((end) - (start) + 1), \ > >>>+ NULL, IORESOURCE_BUS) > >>>+#define MCFG_BUS_ANY MCFG_BUS_RANGE(0x0, 0xff) > >>>+#define MCFG_RES_EMPTY DEFINE_RES_NAMED(0, 0, NULL, 0) > >>>+ > >>>+static struct mcfg_fixup mcfg_quirks[] = { > >>>+/* { OEM_ID, OEM_TABLE_ID, REV, DOMAIN, BUS_RANGE, cfgres, ops }, */ > >>>+}; > >>>+ > >>>+static char mcfg_oem_id[ACPI_OEM_ID_SIZE]; > >>>+static char mcfg_oem_table_id[ACPI_OEM_TABLE_ID_SIZE]; > >>>+static u32 mcfg_oem_revision; > >>>+ > >>>+static void pci_mcfg_match_quirks(struct acpi_pci_root *root, > >>>+ struct resource *cfgres, > >>>+ struct pci_ecam_ops **ecam_ops) > >>>+{ > >>>+ struct mcfg_fixup *f; > >>>+ int i; > >>>+ > >>>+ /* > >>>+ * First match against PCI topology <domain:bus> then use OEM ID, > >>>OEM > >>>+ * table ID, and OEM revision from MCFG table standard header. > >>>+ */ > >>>+ for (i = 0, f = mcfg_quirks; i < ARRAY_SIZE(mcfg_quirks); i++, > >>>f++) { > >>>+ if (f->seg == root->segment && > >> > >>why not use MCFG_DOM_RANGE, I think MCFG_DOM_RANGE is better. > >>if drop MCFG_DOM_RANGE, mcfg_quirks[] will be more complex. > >> > >>static struct mcfg_fixup mcfg_quirks[] = { > >>/* { OEM_ID, OEM_TABLE_ID, REV, DOMAIN, BUS_RANGE, cfgres, ops }, */ > >>#ifdef CONFIG_PCI_HOST_THUNDER_ECAM > >> /* SoC pass1.x */ > >> { "CAVIUM", "THUNDERX", 2, 0, MCFG_BUS_ANY, &pci_thunder_ecam_ops, > >> MCFG_RES_EMPTY}, > >> { "CAVIUM", "THUNDERX", 2, 1, MCFG_BUS_ANY, &pci_thunder_ecam_ops, > >> MCFG_RES_EMPTY}, > >> { "CAVIUM", "THUNDERX", 2, 2, MCFG_BUS_ANY, &pci_thunder_ecam_ops, > >> MCFG_RES_EMPTY}, > >> { "CAVIUM", "THUNDERX", 2, 3, MCFG_BUS_ANY, &pci_thunder_ecam_ops, > >> MCFG_RES_EMPTY}, > >> { "CAVIUM", "THUNDERX", 2, 10, MCFG_BUS_ANY, &pci_thunder_ecam_ops, > >> MCFG_RES_EMPTY}, > >> { "CAVIUM", "THUNDERX", 2, 11, MCFG_BUS_ANY, &pci_thunder_ecam_ops, > >> MCFG_RES_EMPTY}, > >> { "CAVIUM", "THUNDERX", 2, 12, MCFG_BUS_ANY, &pci_thunder_ecam_ops, > >> MCFG_RES_EMPTY}, > >> { "CAVIUM", "THUNDERX", 2, 13, MCFG_BUS_ANY, &pci_thunder_ecam_ops, > >> MCFG_RES_EMPTY}, > >>#endif > >>..... > >>}; > >> > >>As PATCH v5 we only need define mcfg_quirks as below, It looks better. > >>static struct pci_cfg_fixup mcfg_quirks[] __initconst = { > >>/* { OEM_ID, OEM_TABLE_ID, REV, DOMAIN, BUS_RANGE, pci_ops, init_hook > >>}, */ > >>#ifdef CONFIG_PCI_HOST_THUNDER_PEM > >> /* Pass2.0 */ > >> { "CAVIUM", "THUNDERX", 1, MCFG_DOM_RANGE(4, 9), MCFG_BUS_ANY, NULL, > >> thunder_pem_cfg_init }, > >> { "CAVIUM", "THUNDERX", 1, MCFG_DOM_RANGE(14, 19), MCFG_BUS_ANY, NULL, > >> thunder_pem_cfg_init }, > >>#endif > >>#ifdef CONFIG_PCI_HISI_ACPI > >> { "HISI ", "HIP05 ", 0, MCFG_DOM_RANGE(0, 3), MCFG_BUS_ANY, > >> NULL, hisi_pcie_acpi_hip05_init}, > >> { "HISI ", "HIP06 ", 0, MCFG_DOM_RANGE(0, 3), MCFG_BUS_ANY, > >> NULL, hisi_pcie_acpi_hip06_init}, > >> { "HISI ", "HIP07 ", 0, MCFG_DOM_RANGE(0, 15), MCFG_BUS_ANY, > >> NULL, hisi_pcie_acpi_hip07_init}, > >>#endif > >>}; > > > >Note this series disallow hisi_pcie_acpi_hip07_init() call. According to the Bjorn suggestion I rework quirk code to override ops and CFG resources only. Giving that I do not see the way to use MCFG_DOM_RANGE macro. For HISI you would need to get CFG range for each possible case: > > > >#ifdef CONFIG_PCI_HISI_ACPI > > { "HISI ", "HIP05 ", 0, 0, MCFG_BUS_ANY, &hisi_pcie_ops, > > DEFINE_RES_MEM(start0, size0)}, > > { "HISI ", "HIP05 ", 0, 1, MCFG_BUS_ANY, &hisi_pcie_ops, > > DEFINE_RES_MEM(start1, size1)}, > > { "HISI ", "HIP05 ", 0, 2, MCFG_BUS_ANY, &hisi_pcie_ops, > > DEFINE_RES_MEM(start2, size2)}, > > { "HISI ", "HIP05 ", 0, 3, MCFG_BUS_ANY, &hisi_pcie_ops, > > DEFINE_RES_MEM(start3, size3)}, > >[...] > >#endif > > > >Indeed there are more entries here but you do not have to define the same resource array in driver. > > 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 > This patch is based on RFC V5 quirk mechanism. > > Based on V6 quirk mechanism, we have to change it as below: That's because you are hacking around the quirk mechanism to define resources that have nothing to do with PCI config regions (that you ioremap and use to check the PCI link status) and of top of that you are *still* using the MCFG to describe config regions that are NOT ECAM compliant, which is the exact opposite of what this patchset is meant to achieve. If a config region is not ECAM compliant having it defined in the MCFG is a firmware bug and the way you are using this patchset is not what we designed it for, please correct me if I am wrong. Thanks, Lorenzo > > #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 > > > >. > > >
next prev parent reply other threads:[~2016-09-14 12:40 UTC|newest] Thread overview: 234+ messages / expand[flat|nested] mbox.gz Atom feed top 2016-09-09 19:24 [PATCH V6 0/5] ECAM quirks handling for ARM64 platforms Tomasz Nowicki 2016-09-09 19:24 ` Tomasz Nowicki 2016-09-09 19:24 ` [PATCH V6 1/5] PCI/ACPI: Extend pci_mcfg_lookup() responsibilities Tomasz Nowicki 2016-09-09 19:24 ` Tomasz Nowicki 2016-09-09 19:24 ` [PATCH V6 2/5] PCI/ACPI: Check platform specific ECAM quirks Tomasz Nowicki 2016-09-09 19:24 ` Tomasz Nowicki 2016-09-12 22:24 ` Duc Dang 2016-09-12 22:24 ` Duc Dang 2016-09-12 22:24 ` Duc Dang 2016-09-12 22:47 ` Duc Dang 2016-09-12 22:47 ` Duc Dang 2016-09-12 22:47 ` Duc Dang 2016-09-13 5:58 ` Tomasz Nowicki 2016-09-13 5:58 ` Tomasz Nowicki 2016-09-13 5:58 ` Tomasz Nowicki 2016-09-13 6:37 ` Tomasz Nowicki 2016-09-13 6:37 ` Tomasz Nowicki 2016-09-13 6:37 ` Tomasz Nowicki 2016-09-13 2:36 ` Dongdong Liu 2016-09-13 2:36 ` Dongdong Liu 2016-09-13 2:36 ` Dongdong Liu 2016-09-13 6:32 ` Tomasz Nowicki 2016-09-13 6:32 ` Tomasz Nowicki 2016-09-13 11:38 ` Dongdong Liu 2016-09-13 11:38 ` Dongdong Liu 2016-09-13 11:38 ` Dongdong Liu 2016-09-14 12:40 ` Lorenzo Pieralisi [this message] 2016-09-14 12:40 ` Lorenzo Pieralisi 2016-09-15 10:58 ` Lorenzo Pieralisi 2016-09-15 10:58 ` Lorenzo Pieralisi 2016-09-16 9:02 ` Gabriele Paoloni 2016-09-16 9:02 ` Gabriele Paoloni 2016-09-16 9:02 ` Gabriele Paoloni 2016-09-16 9:02 ` Gabriele Paoloni 2016-09-16 12:27 ` Christopher Covington 2016-09-16 12:27 ` Christopher Covington 2016-09-16 12:27 ` Christopher Covington 2016-09-16 12:27 ` Christopher Covington 2016-09-16 13:42 ` Gabriele Paoloni 2016-09-16 13:42 ` Gabriele Paoloni 2016-09-16 13:42 ` Gabriele Paoloni 2016-09-16 13:42 ` Gabriele Paoloni 2016-09-09 19:24 ` [PATCH V6 3/5] PCI: thunder-pem: Allow to probe PEM-specific register range for ACPI case Tomasz Nowicki 2016-09-09 19:24 ` Tomasz Nowicki 2016-09-19 18:09 ` Bjorn Helgaas 2016-09-19 18:09 ` Bjorn Helgaas 2016-09-20 7:23 ` Tomasz Nowicki 2016-09-20 7:23 ` Tomasz Nowicki 2016-09-20 13:33 ` Bjorn Helgaas 2016-09-20 13:33 ` Bjorn Helgaas 2016-09-20 13:33 ` Bjorn Helgaas 2016-09-20 13:40 ` Ard Biesheuvel 2016-09-20 13:40 ` Ard Biesheuvel 2016-09-20 13:40 ` Ard Biesheuvel 2016-09-20 13:40 ` Ard Biesheuvel 2016-09-20 14:05 ` Bjorn Helgaas 2016-09-20 14:05 ` Bjorn Helgaas 2016-09-20 14:05 ` Bjorn Helgaas 2016-09-20 14:05 ` Bjorn Helgaas 2016-09-20 15:09 ` Ard Biesheuvel 2016-09-20 15:09 ` Ard Biesheuvel 2016-09-20 15:09 ` Ard Biesheuvel 2016-09-20 15:09 ` Ard Biesheuvel 2016-09-20 19:17 ` Bjorn Helgaas 2016-09-20 19:17 ` Bjorn Helgaas 2016-09-20 19:17 ` Bjorn Helgaas 2016-09-20 19:17 ` Bjorn Helgaas 2016-09-21 14:05 ` Lorenzo Pieralisi 2016-09-21 14:05 ` Lorenzo Pieralisi 2016-09-21 14:05 ` Lorenzo Pieralisi 2016-09-21 14:05 ` Lorenzo Pieralisi 2016-09-21 18:04 ` Bjorn Helgaas 2016-09-21 18:04 ` Bjorn Helgaas 2016-09-21 18:04 ` Bjorn Helgaas 2016-09-21 18:04 ` Bjorn Helgaas 2016-09-21 18:58 ` Duc Dang 2016-09-21 18:58 ` Duc Dang 2016-09-21 18:58 ` Duc Dang 2016-09-21 18:58 ` Duc Dang 2016-09-21 19:18 ` Bjorn Helgaas 2016-09-21 19:18 ` Bjorn Helgaas 2016-09-21 19:18 ` Bjorn Helgaas 2016-09-21 19:18 ` Bjorn Helgaas 2016-09-23 10:53 ` Tomasz Nowicki 2016-09-23 10:53 ` Tomasz Nowicki 2016-09-23 10:53 ` Tomasz Nowicki 2016-09-23 10:53 ` Tomasz Nowicki 2016-09-22 9:49 ` Lorenzo Pieralisi 2016-09-22 9:49 ` Lorenzo Pieralisi 2016-09-22 9:49 ` Lorenzo Pieralisi 2016-09-22 9:49 ` Lorenzo Pieralisi 2016-09-22 11:10 ` Gabriele Paoloni 2016-09-22 11:10 ` Gabriele Paoloni 2016-09-22 11:10 ` Gabriele Paoloni 2016-09-22 11:10 ` Gabriele Paoloni 2016-09-22 12:44 ` Lorenzo Pieralisi 2016-09-22 12:44 ` Lorenzo Pieralisi 2016-09-22 12:44 ` Lorenzo Pieralisi 2016-09-22 12:44 ` Lorenzo Pieralisi 2016-09-22 18:31 ` Bjorn Helgaas 2016-09-22 18:31 ` Bjorn Helgaas 2016-09-22 18:31 ` Bjorn Helgaas 2016-09-22 18:31 ` Bjorn Helgaas 2016-09-22 22:10 ` Bjorn Helgaas 2016-09-22 22:10 ` Bjorn Helgaas 2016-09-22 22:10 ` Bjorn Helgaas 2016-09-22 22:10 ` Bjorn Helgaas 2016-09-23 10:11 ` Lorenzo Pieralisi 2016-09-23 10:11 ` Lorenzo Pieralisi 2016-09-23 10:11 ` Lorenzo Pieralisi 2016-09-23 10:11 ` Lorenzo Pieralisi 2016-09-23 10:58 ` Gabriele Paoloni 2016-09-23 10:58 ` Gabriele Paoloni 2016-09-23 10:58 ` Gabriele Paoloni 2016-09-23 10:58 ` Gabriele Paoloni 2017-09-14 14:06 ` Ard Biesheuvel 2017-09-14 14:06 ` Ard Biesheuvel 2017-09-14 14:06 ` Ard Biesheuvel 2017-09-26 8:23 ` Gabriele Paoloni 2017-09-26 8:23 ` Gabriele Paoloni 2017-09-26 8:23 ` Gabriele Paoloni 2017-09-26 8:23 ` Gabriele Paoloni 2016-09-22 14:20 ` Christopher Covington 2016-09-22 14:20 ` Christopher Covington 2016-09-22 14:20 ` Christopher Covington 2016-09-22 14:20 ` Christopher Covington 2016-09-21 14:10 ` Gabriele Paoloni 2016-09-21 14:10 ` Gabriele Paoloni 2016-09-21 14:10 ` Gabriele Paoloni 2016-09-21 14:10 ` Gabriele Paoloni 2016-09-21 18:59 ` Bjorn Helgaas 2016-09-21 18:59 ` Bjorn Helgaas 2016-09-21 18:59 ` Bjorn Helgaas 2016-09-21 18:59 ` Bjorn Helgaas 2016-09-22 11:12 ` Gabriele Paoloni 2016-09-22 11:12 ` Gabriele Paoloni 2016-09-22 11:12 ` Gabriele Paoloni 2016-09-22 11:12 ` Gabriele Paoloni 2016-09-09 19:24 ` [PATCH V6 4/5] PCI: thunder: Enable ACPI PCI controller for ThunderX pass2.x silicon version Tomasz Nowicki 2016-09-09 19:24 ` Tomasz Nowicki 2016-09-19 15:45 ` Bjorn Helgaas 2016-09-19 15:45 ` Bjorn Helgaas 2016-09-20 7:06 ` Tomasz Nowicki 2016-09-20 7:06 ` Tomasz Nowicki 2016-09-20 13:08 ` Bjorn Helgaas 2016-09-20 13:08 ` Bjorn Helgaas 2016-09-20 13:08 ` Bjorn Helgaas 2016-09-21 8:05 ` Tomasz Nowicki 2016-09-21 8:05 ` Tomasz Nowicki 2016-09-09 19:24 ` [PATCH V6 5/5] PCI: thunder: Enable ACPI PCI controller for ThunderX pass1.x " Tomasz Nowicki 2016-09-09 19:24 ` Tomasz Nowicki 2016-09-09 19:30 ` [PATCH V6 0/5] ECAM quirks handling for ARM64 platforms Tomasz Nowicki 2016-09-09 19:30 ` Tomasz Nowicki 2016-09-20 19:26 ` Bjorn Helgaas 2016-09-20 19:26 ` Bjorn Helgaas 2016-09-21 1:15 ` cov 2016-09-21 1:15 ` cov at codeaurora.org 2016-09-21 13:11 ` Bjorn Helgaas 2016-09-21 13:11 ` Bjorn Helgaas 2016-09-21 14:07 ` Sinan Kaya 2016-09-21 14:07 ` Sinan Kaya 2016-09-21 17:31 ` Bjorn Helgaas 2016-09-21 17:31 ` Bjorn Helgaas 2016-09-21 17:31 ` Bjorn Helgaas 2016-09-21 17:34 ` Sinan Kaya 2016-09-21 17:34 ` Sinan Kaya 2016-09-21 22:38 ` [PATCHv2] PCI: QDF2432 32 bit config space accessors Christopher Covington 2016-09-21 22:38 ` Christopher Covington 2016-10-31 21:48 ` Bjorn Helgaas 2016-10-31 21:48 ` Bjorn Helgaas 2016-11-01 13:06 ` cov 2016-11-01 13:06 ` cov at codeaurora.org 2016-11-02 16:08 ` Bjorn Helgaas 2016-11-02 16:08 ` Bjorn Helgaas 2016-11-02 16:36 ` Sinan Kaya 2016-11-02 16:36 ` Sinan Kaya 2016-11-03 14:00 ` Bjorn Helgaas 2016-11-03 14:00 ` Bjorn Helgaas 2016-11-03 16:58 ` Sinan Kaya 2016-11-03 16:58 ` Sinan Kaya 2016-11-03 17:06 ` Sinan Kaya 2016-11-03 17:06 ` Sinan Kaya 2016-11-03 20:43 ` Bjorn Helgaas 2016-11-03 20:43 ` Bjorn Helgaas 2016-11-03 23:49 ` Sinan Kaya 2016-11-03 23:49 ` Sinan Kaya 2016-12-02 4:58 ` Jon Masters 2016-12-02 4:58 ` Jon Masters 2016-11-02 16:41 ` Bjorn Helgaas 2016-11-02 16:41 ` Bjorn Helgaas 2016-11-09 19:25 ` Christopher Covington 2016-11-09 19:25 ` Christopher Covington 2016-11-09 20:06 ` Bjorn Helgaas 2016-11-09 20:06 ` Bjorn Helgaas 2016-11-09 20:29 ` Ard Biesheuvel 2016-11-09 20:29 ` Ard Biesheuvel 2016-11-09 20:29 ` Ard Biesheuvel 2016-11-09 20:29 ` Ard Biesheuvel 2016-11-09 22:49 ` Bjorn Helgaas 2016-11-09 22:49 ` Bjorn Helgaas 2016-11-09 22:49 ` Bjorn Helgaas 2016-11-09 22:49 ` Bjorn Helgaas 2016-11-10 10:25 ` Ard Biesheuvel 2016-11-10 10:25 ` Ard Biesheuvel 2016-11-10 10:25 ` Ard Biesheuvel 2016-11-10 10:25 ` Ard Biesheuvel 2016-11-10 13:57 ` Lorenzo Pieralisi 2016-11-10 13:57 ` Lorenzo Pieralisi 2016-11-10 13:57 ` Lorenzo Pieralisi 2016-11-10 13:57 ` Lorenzo Pieralisi 2016-11-10 17:42 ` Bjorn Helgaas 2016-11-10 17:42 ` Bjorn Helgaas 2016-11-10 17:42 ` Bjorn Helgaas 2016-11-10 17:42 ` Bjorn Helgaas 2016-12-02 5:12 ` Jon Masters 2016-12-02 5:12 ` Jon Masters 2016-12-02 5:12 ` Jon Masters 2016-12-02 5:12 ` Jon Masters 2016-09-21 22:40 ` [PATCH V6 0/5] ECAM quirks handling for ARM64 platforms Christopher Covington 2016-09-21 22:40 ` Christopher Covington 2016-09-22 23:08 ` Bjorn Helgaas 2016-09-22 23:08 ` Bjorn Helgaas 2016-09-23 18:41 ` Christopher Covington 2016-09-23 18:41 ` Christopher Covington 2016-09-23 19:17 ` Bjorn Helgaas 2016-09-23 19:17 ` Bjorn Helgaas 2016-09-23 19:17 ` Bjorn Helgaas 2016-09-23 19:22 ` Christopher Covington 2016-09-23 19:22 ` Christopher Covington 2016-09-28 16:44 ` Christopher Covington 2016-11-24 11:05 ` [PATCH V6 1/1] ARM64/PCI: Manage controller-specific information on the host controller basis Tomasz Nowicki 2016-11-24 11:05 ` Tomasz Nowicki 2016-11-29 23:40 ` Bjorn Helgaas 2016-11-29 23:40 ` Bjorn Helgaas
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=20160914124016.GA4123@red-moon \ --to=lorenzo.pieralisi@arm.com \ --cc=Liviu.Dudau@arm.com \ --cc=andrea.gallo@linaro.org \ --cc=ard.biesheuvel@linaro.org \ --cc=arnd@arndb.de \ --cc=catalin.marinas@arm.com \ --cc=cov@codeaurora.org \ --cc=ddaney@caviumnetworks.com \ --cc=dhdang@apm.com \ --cc=gabriele.paoloni@huawei.com \ --cc=hanjun.guo@linaro.org \ --cc=helgaas@kernel.org \ --cc=jchandra@broadcom.com \ --cc=jcm@redhat.com \ --cc=jeremy.linton@arm.com \ --cc=jhugo@codeaurora.org \ --cc=linaro-acpi@lists.linaro.org \ --cc=linux-acpi@vger.kernel.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-pci@vger.kernel.org \ --cc=liudongdong3@huawei.com \ --cc=msalter@redhat.com \ --cc=mw@semihalf.com \ --cc=okaya@codeaurora.org \ --cc=rafael@kernel.org \ --cc=robert.richter@caviumnetworks.com \ --cc=tn@semihalf.com \ --cc=wangyijing@huawei.com \ --cc=will.deacon@arm.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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.