From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 07445C433F5 for ; Tue, 5 Oct 2021 15:46:01 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id CE46F61131 for ; Tue, 5 Oct 2021 15:46:00 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org CE46F61131 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:Content-Type: Content-Transfer-Encoding:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date:Message-ID:From: References:Cc:To:Subject:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=Zl/Yhwzz/2kUOpElDGpyQx2/C1cRnKUKhoQTAlZ9j54=; b=B43DSclYPJUsNN8XorZdkWgAgO md/pZpms7zHfMXiUr6+xca8t13cTdhxAFKiAyhDayA2RxeupIkfODc2yfuAbtUZgaM3xJW3Zdp+yS Y2neyfm957X8cNH/FuWPaTelmq5wM3u3Odd3Qcnen1P77j3cubK3mPkoEYQz2ObxWHi3b2OBozi7H 63gcl7JKg4a3ylZ2KNIOYCldzmF4j7Noo9HSBlC+k+Z6VC1v0bDiceeDigqslyNEydbETVEkk+1G/ tDcSYwAPlC4stnAjCDucfOUj3s8ByHBtLmeYG1BBitCNJB72VszFbz5sfQtAe/F07hmJErTAf58Lt 9Vc7oUdg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mXmby-00B7Ip-2c; Tue, 05 Oct 2021 15:44:10 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mXmbt-00B7Gq-E9; Tue, 05 Oct 2021 15:44:07 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id F27E21FB; Tue, 5 Oct 2021 08:44:01 -0700 (PDT) Received: from [192.168.122.166] (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 1B48B3F70D; Tue, 5 Oct 2021 08:44:01 -0700 (PDT) Subject: Re: [PATCH v3 3/4] PCI/ACPI: Add Broadcom bcm2711 MCFG quirk To: Bjorn Helgaas Cc: linux-pci@vger.kernel.org, lorenzo.pieralisi@arm.com, nsaenz@kernel.org, bhelgaas@google.com, rjw@rjwysocki.net, lenb@kernel.org, robh@kernel.org, kw@linux.com, f.fainelli@gmail.com, bcm-kernel-feedback-list@broadcom.com, linux-acpi@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-rpi-kernel@lists.infradead.org, linux-kernel@vger.kernel.org References: <20211005151042.GA1083482@bhelgaas> From: Jeremy Linton Message-ID: <2bcc69b5-6659-0ec4-cd33-a9b8e61d1afe@arm.com> Date: Tue, 5 Oct 2021 10:43:32 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.10.1 MIME-Version: 1.0 In-Reply-To: <20211005151042.GA1083482@bhelgaas> Content-Language: en-US X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20211005_084405_638799_3773C519 X-CRM114-Status: GOOD ( 29.61 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi, Thanks for looking at this again. On 10/5/21 10:10 AM, Bjorn Helgaas wrote: > On Thu, Aug 26, 2021 at 02:15:56AM -0500, Jeremy Linton wrote: >> Now that there is a bcm2711 quirk, it needs to be enabled when the >> MCFG is missing. Use an ACPI namespace _DSD property >> "linux-ecam-quirk-id" as an alternative to the MCFG OEM. >> >> Signed-off-by: Jeremy Linton >> Acked-by: Florian Fainelli >> Acked-by: Bjorn Helgaas >> --- >> drivers/acpi/pci_mcfg.c | 17 +++++++++++++++++ >> 1 file changed, 17 insertions(+) >> >> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c >> index 53cab975f612..04c517418365 100644 >> --- a/drivers/acpi/pci_mcfg.c >> +++ b/drivers/acpi/pci_mcfg.c >> @@ -169,6 +169,9 @@ static struct mcfg_fixup mcfg_quirks[] = { >> ALTRA_ECAM_QUIRK(1, 13), >> ALTRA_ECAM_QUIRK(1, 14), >> ALTRA_ECAM_QUIRK(1, 15), >> + >> + { "bc2711", "", 0, 0, MCFG_BUS_ANY, &bcm2711_pcie_ops, >> + DEFINE_RES_MEM(0xFD500000, 0xA000) }, >> }; >> >> static char mcfg_oem_id[ACPI_OEM_ID_SIZE]; >> @@ -198,8 +201,22 @@ static void pci_mcfg_apply_quirks(struct acpi_pci_root *root, >> u16 segment = root->segment; >> struct resource *bus_range = &root->secondary; >> struct mcfg_fixup *f; >> + const char *soc; >> int i; >> >> + /* >> + * This may be a machine with a PCI/SMC conduit, which means it doesn't >> + * have an MCFG. Use an ACPI namespace definition instead. >> + */ >> + if (!fwnode_property_read_string(acpi_fwnode_handle(root->device), >> + "linux-ecam-quirk-id", &soc)) { >> + if (strlen(soc) != ACPI_OEM_ID_SIZE) > > From a reviewing perspective, it's not obvious why soc must be exactly > ACPI_OEM_ID_SIZE. Does that imply space-padding in the DT string or > something? This is at the moment an ACPI only DSD, and it must follow the MADT OEM_ID format for now because we are effectively just overriding that field. The rest of the code in this module is just treating it as a fixed 6 bytes. > > Is there any documentation for this DT property? Its not a DT property, and its unclear since its linux only if it belongs in previously linked ACPI registry. > > Also not obvious why strlen() is safe here. I mean, I looked a couple > levels deep in fwnode_property_read_string(), but whatever guarantees > null termination is buried pretty deep. I've not tracked down who, if anyone other than the AML compiler is guaranteeing a null. The spec says something to the effect "Most other string, however, are of variable-length and are automatically null terminated by the compiler". Not sure if that helps any. > > It seems a little weird to use an MCFG quirk mechanism when there's no > MCFG at all on this platform. Well its just a point to hook in a CFG space quirk, and since that is what most of the MCFG quirks are, it seemed reasonable to reuse it rather than recreate it. PS, had some offline convo about reposing with a simple rebase and the ACK's applied, will do that if it helps any, but when I checked a couple weeks back this applied to 5.15 automatically. > >> + dev_err(&root->device->dev, "ECAM quirk should be %d characters\n", >> + ACPI_OEM_ID_SIZE); >> + else >> + memcpy(mcfg_oem_id, soc, ACPI_OEM_ID_SIZE); >> + } >> + >> for (i = 0, f = mcfg_quirks; i < ARRAY_SIZE(mcfg_quirks); i++, f++) { >> if (pci_mcfg_quirk_matches(f, segment, bus_range)) { >> if (f->cfgres.start) >> -- >> 2.31.1 >> _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel