Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757538AbcLAT6p (ORCPT ); Thu, 1 Dec 2016 14:58:45 -0500 Received: from mail-ua0-f174.google.com ([209.85.217.174]:34399 "EHLO mail-ua0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752078AbcLAT6n (ORCPT ); Thu, 1 Dec 2016 14:58:43 -0500 MIME-Version: 1.0 In-Reply-To: References: <1480549373-2123-1-git-send-email-dhdang@apm.com> <1480604901.4751.17.camel@redhat.com> From: Duc Dang Date: Thu, 1 Dec 2016 11:58:12 -0800 Message-ID: Subject: Re: [PATCH v3] PCI/ACPI: xgene: Add ECAM quirk for X-Gene PCIe controller To: Jon Masters Cc: Mark Salter , Bjorn Helgaas , Rafael Wysocki , Lorenzo Pieralisi , Arnd Bergmann , linux-pci@vger.kernel.org, linux-arm , Linux Kernel Mailing List , Tomasz Nowicki , patches Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1461 Lines: 41 On Thu, Dec 1, 2016 at 11:17 AM, Jon Masters wrote: > On 12/01/2016 10:08 AM, Mark Salter wrote: >> On Wed, 2016-11-30 at 15:42 -0800, Duc Dang wrote: > >>> +static int xgene_v1_pcie_ecam_init(struct pci_config_window *cfg) >>> +{ >>> + struct acpi_device *adev = to_acpi_device(cfg->parent); >>> + struct acpi_pci_root *root = acpi_driver_data(adev); >>> + struct device *dev = cfg->parent; >>> + struct xgene_pcie_port *port; >>> + struct resource *csr; >>> + >>> + port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL); >>> + if (!port) >>> + return -ENOMEM; >>> + >>> + csr = &xgene_v1_csr_res[root->segment]; >> >> This hard-coded assumption that segment N uses controller N breaks >> for m400 where segment 0 is using controller 3. I think the latest firmware released from us a few months back use segment 3 for PCIe controller 3 in MCFG table. > > This seems very fragile. So in addition to Bjorn's comment about not > trusting firmware provided data for the segment offset in the CSR list, > you will want to also determine the controller from the ACPI tree. The > existing code walks the ACPI hierarchy and finds the CSR that way. > Obviously, the goal is to avoid that in the latest incarnation, but you > could still determine which controller matches a given device. Yes, I will look into that more. > > Jon. > > -- > Computer Architect | Sent from my Fedora powered laptop > Regards, Duc Dang.